Linux Media Controller development
 help / color / mirror / Atom feed
* RE: [PATCH v4 1/4] [media] s5p-mfc: Update MFCv5 driver for callback based architecture
@ 2012-08-24  4:25 Arun Kumar K
  0 siblings, 0 replies; 3+ messages in thread
From: Arun Kumar K @ 2012-08-24  4:25 UTC (permalink / raw)
  To: Kamil Debski, linux-media@vger.kernel.org
  Cc: Jeongtae Park, Jang-Hyuck Kim, peter Oh, NAVEEN KRISHNA CHATRADHI,
	Marek Szyprowski, kmpark@infradead.org, SUNIL JOSHI

Hi Kamil,
Thank you for the review comments.
Will post v5 patches incorporating your comments.

Regards
Arun

------- Original Message -------
Sender : Kamil Debski<k.debski@samsung.com>  Software Engineer/SPRC-Linux Platform (SSD)/Samsung Electronics
Date   : Aug 22, 2012 21:48 (GMT+05:30)
Title  : RE: [PATCH v4 1/4] [media] s5p-mfc: Update MFCv5 driver for callback
 based architecture

Hi Arun,

First of all - thank you for all the patches, they are getting better and
better
with every version.

Two things: 
1) A minor one - I suggest that the choice of the default pixel format should
be
done in a different manner. I suggest using a V4L2_PIX_FMT_* in the
DEF_SRC_FMT_DEC
define and then using find_format in s5p_mfc_dec_init. Same goes for encoding.

2) A major one - the ops mechanism could be improved. I see that there are
many functions that only call an ops. Such as:

> int s5p_mfc_alloc_dec_temp_buffers(struct s5p_mfc_ctx *ctx)
> {
>        return s5p_mfc_ops->s5p_mfc_alloc_dec_temp_buffers(ctx);
> }

I suggest dropping the s5p_mfc_ prefix in all the fields of s5p_mfc_hw_ops and
using a macro to call the ops. Like this:

+#define fimc_pipeline_call(f, op, p, args...)                          \r
+       (!(f) ? -ENODEV : (((f)->pipeline_ops && (f)->pipeline_ops->op) ? \r
+                           (f)->pipeline_ops->op((p), ##args) :
-ENOIOCTLCMD))
(from commit by Sylwester Nawrocki http://goo.gl/Q657j)

In addition, your code does not check whether the ops pointer is null and I
think that it should be done.

Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center

[snip the code]

<p>&nbsp;</p><p>&nbsp;</p>

^ permalink raw reply	[flat|nested] 3+ messages in thread
* [PATCH v4 0/4] Update MFC v4l2 driver to support MFC6.x
@ 2012-08-09 10:28 Arun Kumar K
  2012-08-09 10:28 ` [PATCH v4 1/4] [media] s5p-mfc: Update MFCv5 driver for callback based architecture Arun Kumar K
  0 siblings, 1 reply; 3+ messages in thread
From: Arun Kumar K @ 2012-08-09 10:28 UTC (permalink / raw)
  To: linux-media
  Cc: jtp.park, janghyuck.kim, jaeryul.oh, ch.naveen, arun.kk,
	m.szyprowski, k.debski, kmpark, joshi

The patchset adds support for MFCv6 firmware in s5p-mfc driver.
The first two patches will update the existing MFCv5 driver framework
for making it suitable for supporting co-existence with a newer
hardware version. The last two patches add support for MFCv6 firmware.
This patchset have to be applied on patches [1] and [2] posted
earlier which adds the required v4l2 controls.

Changelog:
- Separate patch for callback based architecture.
- Patches divided to enable incremental compilation.
- Working MFCv6 encoder and decoder.
- Addressed review comments given for v3 patchset.

[1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg48972.html
[2] http://www.mail-archive.com/linux-media@vger.kernel.org/msg48973.html

Arun Kumar K (1):
  [media] s5p-mfc: Update MFCv5 driver for callback based architecture

Jeongtae Park (3):
  [media] s5p-mfc: Add MFC variant data to device context
  [media] s5p-mfc: MFCv6 register definitions
  [media] s5p-mfc: Update MFC v4l2 driver to support MFC6.x

 drivers/media/video/Kconfig                  |    4 +-
 drivers/media/video/s5p-mfc/Makefile         |    7 +-
 drivers/media/video/s5p-mfc/regs-mfc-v6.h    |  429 ++++++
 drivers/media/video/s5p-mfc/regs-mfc.h       |   29 +
 drivers/media/video/s5p-mfc/s5p_mfc.c        |  224 ++--
 drivers/media/video/s5p-mfc/s5p_mfc_cmd.c    |   98 +-
 drivers/media/video/s5p-mfc/s5p_mfc_cmd.h    |   13 +
 drivers/media/video/s5p-mfc/s5p_mfc_cmd_v5.c |  164 +++
 drivers/media/video/s5p-mfc/s5p_mfc_cmd_v5.h |   20 +
 drivers/media/video/s5p-mfc/s5p_mfc_cmd_v6.c |  155 ++
 drivers/media/video/s5p-mfc/s5p_mfc_cmd_v6.h |   20 +
 drivers/media/video/s5p-mfc/s5p_mfc_common.h |  156 ++-
 drivers/media/video/s5p-mfc/s5p_mfc_ctrl.c   |  188 ++-
 drivers/media/video/s5p-mfc/s5p_mfc_ctrl.h   |    1 +
 drivers/media/video/s5p-mfc/s5p_mfc_dec.c    |  223 ++-
 drivers/media/video/s5p-mfc/s5p_mfc_dec.h    |    1 +
 drivers/media/video/s5p-mfc/s5p_mfc_enc.c    |  205 ++--
 drivers/media/video/s5p-mfc/s5p_mfc_enc.h    |    1 +
 drivers/media/video/s5p-mfc/s5p_mfc_intr.c   |   11 +-
 drivers/media/video/s5p-mfc/s5p_mfc_opr.c    | 1405 ++-----------------
 drivers/media/video/s5p-mfc/s5p_mfc_opr.h    |  178 ++-
 drivers/media/video/s5p-mfc/s5p_mfc_opr_v5.c | 1761 +++++++++++++++++++++++
 drivers/media/video/s5p-mfc/s5p_mfc_opr_v5.h |   85 ++
 drivers/media/video/s5p-mfc/s5p_mfc_opr_v6.c | 1944 ++++++++++++++++++++++++++
 drivers/media/video/s5p-mfc/s5p_mfc_opr_v6.h |   50 +
 drivers/media/video/s5p-mfc/s5p_mfc_pm.c     |    3 +-
 drivers/media/video/s5p-mfc/s5p_mfc_shm.c    |   47 -
 drivers/media/video/s5p-mfc/s5p_mfc_shm.h    |   90 --
 28 files changed, 5645 insertions(+), 1867 deletions(-)
 create mode 100644 drivers/media/video/s5p-mfc/regs-mfc-v6.h
 create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_cmd_v5.c
 create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_cmd_v5.h
 create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_cmd_v6.c
 create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_cmd_v6.h
 create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr_v5.c
 create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr_v5.h
 create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr_v6.c
 create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr_v6.h
 delete mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_shm.c
 delete mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_shm.h


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-08-24  4:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-24  4:25 [PATCH v4 1/4] [media] s5p-mfc: Update MFCv5 driver for callback based architecture Arun Kumar K
  -- strict thread matches above, loose matches on Subject: below --
2012-08-09 10:28 [PATCH v4 0/4] Update MFC v4l2 driver to support MFC6.x Arun Kumar K
2012-08-09 10:28 ` [PATCH v4 1/4] [media] s5p-mfc: Update MFCv5 driver for callback based architecture Arun Kumar K
2012-08-22 16:18   ` Kamil Debski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox