Re: [PATCH] media: platform: Renesas IMR driver

[Magnus Damm]

Hi Sergei,

Thanks for your efforts with this driver. Nice to see that V2 is
getting in better shape.

In the future, would it be possible for you to include the patch
version number in the [PATCH] tag somehow?

On Fri, Mar 3, 2017 at 9:03 PM, Sergei Shtylyov
 wrote:
> On 03/03/2017 02:58 PM, Geert Uytterhoeven wrote:
>
> +  - "renesas,imr-lx4-v3m" for R-Car V3M.



 "renesas,-EPROBE_DEFER-imr-lx4"
>>>
>>>
>>>
>>>Huh? :-)
>>
>>
>> Do you know the part number of V3M?
>
>
>No, but using the names from the manual I don't need it.

NAK, like Geert says, please follow the same style as other upstream
drivers. DT compat strings is not a place for random polices.

Thanks,

/ magnus

[PATCH/RFC] v4l: vsp1: Use FCP device for DisplayList and VB2 queue

[Magnus Damm]

From: Magnus Damm <damm+rene...@opensource.se>

Incrementally fix up parts of the code not yet covered by the
IOMMU patches by Laurent:

[PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

This patch simply uses the recently introduced function
rcar_fcp_get_device() on the VSP device to retrieve the
FCP device that needs to be used with the DMA MAP API
when IOMMU is enabled.

Without this patch the DU/VSP/FCP devices on R-Car Gen3 will
generate traps during boot when IOMMU is enabled.

Signed-off-by: Magnus Damm <damm+rene...@opensource.se>
---

 drivers/media/platform/vsp1/vsp1_dl.c|   13 +
 drivers/media/platform/vsp1/vsp1_video.c |6 +-
 2 files changed, 14 insertions(+), 5 deletions(-)

--- 0001/drivers/media/platform/vsp1/vsp1_dl.c
+++ work/drivers/media/platform/vsp1/vsp1_dl.c  2016-09-01 06:18:17.140607110 
+0900
@@ -17,6 +17,8 @@
 #include 
 #include 
 
+#include 
+
 #include "vsp1.h"
 #include "vsp1_dl.h"
 
@@ -130,12 +132,12 @@ static int vsp1_dl_body_init(struct vsp1
 size_t extra_size)
 {
size_t size = num_entries * sizeof(*dlb->entries) + extra_size;
+   struct device *fcp = rcar_fcp_get_device(vsp1->fcp);
 
dlb->vsp1 = vsp1;
dlb->size = size;
-
-   dlb->entries = dma_alloc_wc(vsp1->dev, dlb->size, >dma,
-   GFP_KERNEL);
+   dlb->entries = dma_alloc_wc(fcp ? fcp : vsp1->dev,
+   dlb->size, >dma, GFP_KERNEL);
if (!dlb->entries)
return -ENOMEM;
 
@@ -147,7 +149,10 @@ static int vsp1_dl_body_init(struct vsp1
  */
 static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb)
 {
-   dma_free_wc(dlb->vsp1->dev, dlb->size, dlb->entries, dlb->dma);
+   struct device *fcp = rcar_fcp_get_device(dlb->vsp1->fcp);
+
+   dma_free_wc(fcp ? fcp : dlb->vsp1->dev,
+   dlb->size, dlb->entries, dlb->dma);
 }
 
 /**
--- 0001/drivers/media/platform/vsp1/vsp1_video.c
+++ work/drivers/media/platform/vsp1/vsp1_video.c   2016-09-01 
06:20:02.940607110 +0900
@@ -27,6 +27,8 @@
 #include 
 #include 
 
+#include 
+
 #include "vsp1.h"
 #include "vsp1_bru.h"
 #include "vsp1_dl.h"
@@ -939,6 +941,7 @@ struct vsp1_video *vsp1_video_create(str
 {
struct vsp1_video *video;
const char *direction;
+   struct device *fcp;
int ret;
 
video = devm_kzalloc(vsp1->dev, sizeof(*video), GFP_KERNEL);
@@ -996,7 +999,8 @@ struct vsp1_video *vsp1_video_create(str
video->queue.ops = _video_queue_qops;
video->queue.mem_ops = _dma_contig_memops;
video->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
-   video->queue.dev = video->vsp1->dev;
+   fcp = rcar_fcp_get_device(vsp1->fcp);
+   video->queue.dev = fcp ? fcp : video->vsp1->dev;
ret = vb2_queue_init(>queue);
if (ret < 0) {
dev_err(video->vsp1->dev, "failed to initialize vb2 queue\n");
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

[Magnus Damm]

Hi Laurent,

Thanks for your help with this. Good to see that the DU driver is
getting closer to work with the IPMMU hardware! Please see below for
some feedback from me.

On Fri, Aug 19, 2016 at 5:39 PM, Laurent Pinchart
 wrote:
> Hello,
>
> This patch series fixes the rcar-du-drm driver to support VSP plane sources
> with an IOMMU. It is available for convenience at
>
> git://linuxtv.org/pinchartl/media.git iommu/devel/du
>
> On R-Car Gen3 the DU has no direct memory access but sources planes through
> VSP instances. When an IOMMU is inserted between the VSP and memory, the DU
> framebuffers need to be DMA mapped using the VSP device, not the DU device as
> currently done. The same situation can also be reproduced on Gen2 hardware by
> linking the VSP to the DU in DT [1], effectively disabling direct memory
> access by the DU.
>
> The situation is made quite complex by the fact that different planes can be
> connected to different DU instances, and thus served by different IOMMUs (or,
> in practice on existing hardware, by the same IOMMU but through different
> micro-TLBs). We thus can't allocate and map buffers to the right device in a
> single dma_alloc_wc() operation as done in the DRM CMA GEM helpers.
>
> However, on such setups, the DU DT node doesn't reference IOMMUs as the DU
> does not perform any direct memory access. We can thus keep the GEM object
> allocation unchanged, and the DMA addresses that we receive in the DU driver
> will be physical addresses. Those buffers then need to be mapped to the VSP
> device when they are associated with planes. Fortunately the atomic framework
> provides two plane helper operations, .prepare_fb() and .cleanup_fb() that we
> can use for this purpose.
>
> The reality is slightly more complex than this on Gen3, as an FCP device
> instance sits between VSP instances and memory. It is the FCP devices that are
> connected to the IOMMUs, and buffer mapping thus need to be performed using
> the FCP devices. This isn't required on Gen2 as the platforms don't have any
> FCPs.
>
> Patches 1/6 and 2/6 unconstify the state argument to the .prepare_fb() and
> .cleanup_fb() operations, to allow storing the mapped buffer addresses in the
> state. Patches 3/6 and 4/6 then extend the rcar-fcp driver API to expose the
> FCP struct device. Patch 5/6 extends the vsp1 driver API to allow mapping a
> scatter-gather list to the VSP, with the implementation using the FCP devices
> instead when available. Patch 6/6 then use the vsp1 mapping API in the
> rcar-du-drm driver to map and unmap buffers when needed.
>
> The series has been tested on Gen2 (Lager) only as the Gen3 IOMMU is known to
> be broken.

Slight clarification, the R-Car Gen3 family as a whole does not have
broken IPMMU hardware. Early R-Car H3 revisions do require some errata
handling though, but M3-W and later ES versions and MP of H3 will be
fine. Given the early R-Car H3 errata I agree it makes sense to
develop and test this series on R-Car Gen2 though.

> A possible improvement is to modify the GEM object allocation mechanism to use
> non-contiguous memory when the DU driver detects that all the VSP instances it
> is connected to use an IOMMU (possibly through FCP devices).
>
> An issue has been noticed with synchronization between page flip and VSP
> operation. Buffers get unmapped (and possibly freed) before the VSP is done
> reading them. The problem isn't new, but is much more noticeable with IOMMU
> support enabled as any hardware access to unmapped memory generates an IOMMU
> page fault immediately.
>
> The series unfortunately contain a dependency between DRM and V4L2 patches,
> complicating upstream merge. As there's no urgency to merge patch 6/6 due to
> the IOMMU being broken on Gen3 at the moment, I propose merging patches
> 1/6-2/6 and 3/6-5/6 independently for the next kernel release.
>
> I would particularly appreciate feedback on the APIs introduced by patches 4/6
> and 5/6.

The code in general looks fine to me. The APIs introduced by patches
4/6 and 5/6 seem quite straightforward. Is there something I can do to
help with those?

> [1] 
> https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg06589.html
>
> Laurent Pinchart (6):
>   drm: Don't implement empty prepare_fb()/cleanup_fb()
>   drm: Unconstify state argument to prepare_fb()/cleanup_fb()
>   v4l: rcar-fcp: Don't get/put module reference
>   v4l: rcar-fcp: Add an API to retrieve the FCP device
>   v4l: vsp1: Add API to map and unmap DRM buffers through the VSP
>   drm: rcar-du: Map memory through the VSP device
>
>  drivers/gpu/drm/arc/arcpgu_crtc.c   |  2 -
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 15 -
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 -
>  drivers/gpu/drm/i915/intel_display.c|  4 +-
>  drivers/gpu/drm/i915/intel_drv.h|  4 +-
>  

Re: [PATCH 14/15] ARM: shmobile: Remove AP4EVB board support

[Magnus Damm]

On Mon, Jun 17, 2013 at 3:12 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Mon, 17 Jun 2013, Magnus Damm wrote:

 [snip]

 So Guennadi, if you want to keep this board then you have to step up
 and fix things. If not then there is no point in keeping it.

 Ok, after a private discussion we agreed to remove the board, which will
 also make the drivers for the Renesas sh-/r-mobile CSI2 interface and for
 the Sony IMX074 sensor untestable and susceptible to removal. Also
 multi-subdevice support in soc-camera now will lose its only use and can
 become broken. I will also drop CSI2 and AP4EVB patches from my V4L2 clock
 / async probing series.

Thanks for writing this summary. It matches my understanding.

It is unfortunate, but it seems to me that the camera sensor has to be
tested on another platform. Regarding the CSI2 interface, as we
discussed, this IP still exists in newer SoCs so because of that I
recommend you to try to request newer hardware for future testing.

About multi-subdevice and your ongoing work with V4L2 clock / async
probing, please select a more recent hardware platform.

Thanks,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] V4L2: soc_camera: Renesas R-Car VIN driver

[Magnus Damm]

Hi Sergei,

On Fri, Apr 19, 2013 at 11:31 PM, Sergei Shtylyov
sergei.shtyl...@cogentembedded.com wrote:
 From: Vladimir Barinov vladimir.bari...@cogentembedded.com

 Add Renesas R-Car VIN (Video In) V4L2 driver.

 Based on the patch by Phil Edworthy phil.edwor...@renesas.com.

 Signed-off-by: Vladimir Barinov vladimir.bari...@cogentembedded.com
 [Sergei: removed deprecated IRQF_DISABLED flag.]
 Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com

 ---
 Changes since the original posting:
 - added IRQF_SHARED flag in devm_request_irq() call (since on R8A7778 VIN0/1
   share the same IRQ) and removed deprecated IRQF_DISABLED flag.

  drivers/media/platform/soc_camera/Kconfig|7
  drivers/media/platform/soc_camera/Makefile   |1
  drivers/media/platform/soc_camera/rcar_vin.c | 1784 
 +++
  include/linux/platform_data/camera-rcar.h|   25
  4 files changed, 1817 insertions(+)

 Index: renesas/drivers/media/platform/soc_camera/Kconfig
 ===
 --- renesas.orig/drivers/media/platform/soc_camera/Kconfig
 +++ renesas/drivers/media/platform/soc_camera/Kconfig
 @@ -45,6 +45,13 @@ config VIDEO_PXA27x
 ---help---
   This is a v4l2 driver for the PXA27x Quick Capture Interface

 +config VIDEO_RCAR_VIN
 +   tristate R-Car Video Input (VIN) support
 +   depends on VIDEO_DEV  SOC_CAMERA  (ARCH_R8A7778 || ARCH_R8A7779)
 +   select VIDEOBUF2_DMA_CONTIG
 +   ---help---
 + This is a v4l2 driver for the R-Car VIN Interface

Thanks for your work on this. I believe there are multiple SoCs
containing VIN hardware, so limiting to r8a7778 and r8a7779 doesn't
make sense to me. Actually, our other drivers do not have this kind of
detailed dependency control.

So based on that, would it be possible for you to change the above
dependency to:

depends on VIDEO_DEV  SOC_CAMERA

Thanks,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: acks needed

[Magnus Damm]

Hi Guennadi,

On Wed, Sep 28, 2011 at 5:11 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 Hi Paul, Magnus

 The following patches need your acks to allow the whole stack to go on
 time into 3.2 without breaking platforms even intermittently.

 [20/59] ARM: ap4evb: switch imx074 configuration to default number of lanes
 http://patchwork.linuxtv.org/patch/7514/
 [27/59] ARM: mach-shmobile: convert mackerel to mediabus flags
 http://patchwork.linuxtv.org/patch/7506/
 [28/59] sh: convert ap325rxa to mediabus flags
 http://patchwork.linuxtv.org/patch/7513/
 [49/59] sh: ap3rxa: remove redundant soc-camera platform data fields
 http://patchwork.linuxtv.org/patch/7517/
 [50/59] sh: migor: remove unused ov772x buswidth flag
 http://patchwork.linuxtv.org/patch/7516/
 [56/59] ARM: mach-shmobile: mackerel doesn't need legacy SOCAM_* flags anymore
 http://patchwork.linuxtv.org/patch/7523/

They all look fine to me, thanks. Please note that I have not
performed any kind of testing of the patches above, but as usual I
expect you to fix up eventual issues by yourself. =)

Acked-by: Magnus Damm d...@opensource.se
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC v2 3/3] fbdev: sh_mobile_lcdc: Support FOURCC-based format API

[Magnus Damm]

On Fri, Aug 19, 2011 at 6:37 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 ---
  arch/arm/mach-shmobile/board-ag5evm.c   |    2 +-
  arch/arm/mach-shmobile/board-ap4evb.c   |    4 +-
  arch/arm/mach-shmobile/board-mackerel.c |    4 +-
  drivers/video/sh_mobile_lcdcfb.c        |  342 
 ---
  include/video/sh_mobile_lcdc.h          |    4 +-
  5 files changed, 230 insertions(+), 126 deletions(-)

Hi Laurent, thanks for the patch!

Since you're changing the LCDC platform data please make sure you also
update the 5 boards using the LCDC under arch/sh.

Thanks,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: switch mackerel to dynamically manage the platform camera

[Magnus Damm]

On Mon, Mar 21, 2011 at 10:22 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Wed, 16 Mar 2011, Magnus Damm wrote:

 On Tue, Feb 22, 2011 at 6:57 PM, Guennadi Liakhovetski
 g.liakhovet...@gmx.de wrote:
  Use soc_camera_platform helper functions to dynamically manage the
  camera device.
 
  Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
  ---
   arch/arm/mach-shmobile/board-mackerel.c |   28 
  +++-
   1 files changed, 7 insertions(+), 21 deletions(-)

 I just tested patch 1/3 and patch 3/3 on my Mackerel board.

 Thanks for testing!

 Unfortunately I get this printout on the console:

 sh_mobile_ceu sh_mobile_ceu.0: SuperH Mobile CEU driver attached to camera 0
 soc_camera_platform soc_camera_platform.0: Platform has not set
 soc_camera_device pointer!
 soc_camera_platform: probe of soc_camera_platform.0 failed with error -22
 sh_mobile_ceu sh_mobile_ceu.0: SuperH Mobile CEU driver detached from camera  0

 Without these two patches everything work just fine. Any ideas on how
 to fix it? I'd be happy to test V2. =)

 Hm, yes, looks like I'm initialising the pointer too late. Could you,
 please, test the patch below on top, if it helps, I'll send v2.

Yes, this fix incremental change solves the problem. Thanks!

Please post V2 and add:

Acked-by: Magnus Damm d...@opensource.se
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: switch mackerel to dynamically manage the platform camera

[Magnus Damm]

On Mon, Mar 21, 2011 at 10:22 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Wed, 16 Mar 2011, Magnus Damm wrote:

 On Tue, Feb 22, 2011 at 6:57 PM, Guennadi Liakhovetski
 g.liakhovet...@gmx.de wrote:
  Use soc_camera_platform helper functions to dynamically manage the
  camera device.
 
  Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
  ---
   arch/arm/mach-shmobile/board-mackerel.c |   28 
  +++-
   1 files changed, 7 insertions(+), 21 deletions(-)

 I just tested patch 1/3 and patch 3/3 on my Mackerel board.

 Thanks for testing!

 Unfortunately I get this printout on the console:

 sh_mobile_ceu sh_mobile_ceu.0: SuperH Mobile CEU driver attached to camera 0
 soc_camera_platform soc_camera_platform.0: Platform has not set
 soc_camera_device pointer!
 soc_camera_platform: probe of soc_camera_platform.0 failed with error -22
 sh_mobile_ceu sh_mobile_ceu.0: SuperH Mobile CEU driver detached from camera  0

 Without these two patches everything work just fine. Any ideas on how
 to fix it? I'd be happy to test V2. =)

 Hm, yes, looks like I'm initialising the pointer too late. Could you,
 please, test the patch below on top, if it helps, I'll send v2.

Yes, this incremental change solves the problem. Thanks!

Please post V2 and add:

Acked-by: Magnus Damm d...@opensource.se
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: switch mackerel to dynamically manage the platform camera

[Magnus Damm]

On Tue, Feb 22, 2011 at 6:57 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 Use soc_camera_platform helper functions to dynamically manage the
 camera device.

 Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
 ---
  arch/arm/mach-shmobile/board-mackerel.c |   28 +++-
  1 files changed, 7 insertions(+), 21 deletions(-)

Hi Guennadi,

Thanks for your work on this. The soc_camera_platform interface has
become much much nicer with these patches.

I just tested patch 1/3 and patch 3/3 on my Mackerel board.
Unfortunately I get this printout on the console:

sh_mobile_ceu sh_mobile_ceu.0: SuperH Mobile CEU driver attached to camera 0
soc_camera_platform soc_camera_platform.0: Platform has not set
soc_camera_device pointer!
soc_camera_platform: probe of soc_camera_platform.0 failed with error -22
sh_mobile_ceu sh_mobile_ceu.0: SuperH Mobile CEU driver detached from camera 0

Without these two patches everything work just fine. Any ideas on how
to fix it? I'd be happy to test V2. =)

Thanks,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] V4L/DVB: soc-camera: module_put() fix

[Magnus Damm]

From: Magnus Damm d...@opensource.se

Avoid calling module_put() if try_module_get() has
been skipped.

Signed-off-by: Magnus Damm d...@opensource.se
---

 drivers/media/video/soc_camera.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- 0001/drivers/media/video/soc_camera.c
+++ work/drivers/media/video/soc_camera.c   2010-06-23 13:43:05.0 
+0900
@@ -1034,7 +1034,8 @@ eiufmt:
soc_camera_free_i2c(icd);
} else {
icl-del_device(icl);
-   module_put(control-driver-owner);
+   if (control  control-driver  control-driver-owner)
+   module_put(control-driver-owner);
}
 enodrv:
 eadddev:
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3 v2] V4L: SuperH Video Output Unit (VOU) driver

[Magnus Damm]

Hey Guennadi,

On Thu, Mar 18, 2010 at 7:28 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 A number of SuperH SoCs, including sh7724, include a Video Output Unit. This
 patch adds a video (V4L2) output driver for it. The driver uses v4l2-subdev 
 and
 mediabus APIs to interface to TV encoders.

 Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
 ---

Thanks for your work on this. The VOU block is actually used by the
SH-Mobile series of SoCs. So you may want to throw in a mobile there
in you description to avoid future name space collisions.

I'll make sure that people test your V2 patch.

Is both NTSC and PAL known to be ok with v2?

Thanks,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] soc-camera: return -ENODEV is sensor is missing

[Magnus Damm]

From: Magnus Damm d...@opensource.se

Update the soc-camera i2c code to return -ENODEV if
a camera sensor is missing instead of -ENOMEM.

Signed-off-by: Magnus Damm d...@opensource.se
---

 drivers/media/video/soc_camera.c |8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

--- 0001/drivers/media/video/soc_camera.c
+++ work/drivers/media/video/soc_camera.c   2010-02-09 17:32:58.0 
+0900
@@ -846,10 +846,8 @@ static int soc_camera_init_i2c(struct so
struct soc_camera_host *ici = to_soc_camera_host(icd-dev.parent);
struct i2c_adapter *adap = i2c_get_adapter(icl-i2c_adapter_id);
struct v4l2_subdev *subdev;
-   int ret;
 
if (!adap) {
-   ret = -ENODEV;
dev_err(icd-dev, Cannot get I2C adapter #%d. No driver?\n,
icl-i2c_adapter_id);
goto ei2cga;
@@ -859,10 +857,8 @@ static int soc_camera_init_i2c(struct so
 
subdev = v4l2_i2c_new_subdev_board(ici-v4l2_dev, adap,
icl-module_name, icl-board_info, NULL);
-   if (!subdev) {
-   ret = -ENOMEM;
+   if (!subdev)
goto ei2cnd;
-   }
 
client = subdev-priv;
 
@@ -873,7 +869,7 @@ static int soc_camera_init_i2c(struct so
 ei2cnd:
i2c_put_adapter(adap);
 ei2cga:
-   return ret;
+   return -ENODEV;
 }
 
 static void soc_camera_free_i2c(struct soc_camera_device *icd)
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] sh_mobile_ceu_camera: Add physical address alignment checks V2

[Magnus Damm]

From: Magnus Damm d...@opensource.se

Make sure physical addresses are 32-bit aligned in the SuperH
Mobile CEU driver V2. The lowest two bits of the frame address
registers are fixed to zero so frame buffers have to be 32-bit
aligned. The V4L2 mmap() case is using dma_alloc_coherent() for
this driver which will return already aligned addresses, but in
the USERPTR case we must make sure that the user space pointer
is valid.

Signed-off-by: Magnus Damm d...@opensource.se
---

 Tested with a hacked up capture.c on a sh7722 Migo-R board.

 V2 moves the checks to sh_mobile_ceu_videobuf_prepare()

 drivers/media/video/sh_mobile_ceu_camera.c |9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

--- 0010/drivers/media/video/sh_mobile_ceu_camera.c
+++ work/drivers/media/video/sh_mobile_ceu_camera.c 2009-12-11 
16:52:19.0 +0900
@@ -339,7 +339,7 @@ static int sh_mobile_ceu_videobuf_prepar
}
 
vb-size = vb-width * vb-height * ((buf-fmt-depth + 7)  3);
-   if (0 != vb-baddr  vb-bsize  vb-size) {
+   if (0 != vb-baddr  vb-bsize  vb-size  !(vb-width  3)) {
ret = -EINVAL;
goto out;
}
@@ -348,6 +348,13 @@ static int sh_mobile_ceu_videobuf_prepar
ret = videobuf_iolock(vq, vb, NULL);
if (ret)
goto fail;
+
+   /* the physical address must be 32-bit aligned (USERPTR) */
+   if (videobuf_to_dma_contig(vb)  3) {
+   ret = -EINVAL;
+   goto fail;
+   }
+
vb-state = VIDEOBUF_PREPARED;
}
 
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sh_mobile_ceu_camera: Remove frame size page alignment

[Magnus Damm]

On Thu, Dec 10, 2009 at 10:06 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Wed, 9 Dec 2009, Magnus Damm wrote:

 From: Magnus Damm d...@opensource.se

 This patch updates the SuperH Mobile CEU driver to
 not page align the frame size. Useful in the case of
 USERPTR with non-page aligned frame sizes and offsets.

 Signed-off-by: Magnus Damm d...@opensource.se

 Please, correct me if I'm wrong. Currently most (all?) sh platforms, using
 this driver, and wishing to use V4L2_MEMORY_MMAP, reserve contiguous
 memory in their platform code. In this case pcdev-video_limit is set to
 the size of that area. videobuf-dma-contig.c::__videobuf_mmap_mapper()
 will anyway allocate page-aligned buffers for V4L2_MEMORY_MMAP, so, even
 for the case of a platform, not reserving RAM at boot-time, it should
 work. Similarly it should work for the V4L2_MEMORY_USERPTR case. So, looks
 ok to me, queued, thanks.

Correct. On SuperH Mobile the amount of reserved physically contiguous
memory for the CEU can be overridden on the kernel command line, and
in the case of systems only using USERPTR it is wise to set it to the
reserved memory to zero since the memory will be unused anyway when
the V4L2 buffers come from elsewhere.

If there is no physically contiguous memory reserved for the CEU and
V4L2 MMAP is used then there is a risk of physically contiguous memory
allocation failure due to memory fragmentation. Nothing out of the
ordinary, to play it safe just reserve physically contiguous memory
during boot up time and be done with it.

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH - v1] V4L-Fix videobuf_dma_contig_user_get() for non-aligned offsets

[Magnus Damm]

On Wed, Dec 9, 2009 at 6:36 AM,  m-kariche...@ti.com wrote:
 From: Muralidharan Karicheri m-kariche...@ti.com

 If a USERPTR address that is not aligned to page boundary is passed to the
 videobuf_dma_contig_user_get() function, it saves a page aligned address to
 the dma_handle. This is not correct. This issue is observed when using USERPTR
 IO machism for buffer exchange.

 Updates from last version:-

 Adding offset for size calculation as per comment from Magnus Damm. This
 ensures the last page is also included for checking if memory is
 contiguous.

 Signed-off-by: Muralidharan Karicheri m-kariche...@ti.com

Hi Murali,

I've spent some time testing this patch with the SuperH CEU driver in
USERPTR mode. My test case is based on capture.c with places a bunch
of QVGA frames directly after each other. The size of each QVGA frame
is not an even multiple of 4k page size, so some of the frames will
use a non-aligned start addresses. Currently the CEU driver page
aligns the size of each frame, but I'll fix that in an upcoming patch.
Thank you!

Acked-by: Magnus Damm d...@opensource.se
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] sh_mobile_ceu_camera: Add physical address alignment checks

[Magnus Damm]

From: Magnus Damm d...@opensource.se

Make sure physical addresses are 32-bit aligned in the
SuperH Mobile CEU driver. The lowest two bits of the
address registers are fixed to zero so frame buffers
have to bit 32-bit aligned. The V4L2 mmap() case is
using dma_alloc_coherent() for this driver which will
return aligned addresses, but in the USERPTR case we
must make sure that the user space pointer is valid.

Signed-off-by: Magnus Damm d...@opensource.se
---

 drivers/media/video/sh_mobile_ceu_camera.c |   16 
 1 file changed, 12 insertions(+), 4 deletions(-)

--- 0001/drivers/media/video/sh_mobile_ceu_camera.c
+++ work/drivers/media/video/sh_mobile_ceu_camera.c 2009-12-09 
17:16:47.0 +0900
@@ -278,9 +278,14 @@ static int sh_mobile_ceu_capture(struct 
 
phys_addr_top = videobuf_to_dma_contig(pcdev-active);
ceu_write(pcdev, CDAYR, phys_addr_top);
+   if (phys_addr_top  3)
+   return -EINVAL;
+
if (pcdev-is_interlaced) {
phys_addr_bottom = phys_addr_top + icd-user_width;
ceu_write(pcdev, CDBYR, phys_addr_bottom);
+   if (phys_addr_bottom  3)
+   return -EINVAL;
}
 
switch (icd-current_fmt-fourcc) {
@@ -288,13 +293,16 @@ static int sh_mobile_ceu_capture(struct 
case V4L2_PIX_FMT_NV21:
case V4L2_PIX_FMT_NV16:
case V4L2_PIX_FMT_NV61:
-   phys_addr_top += icd-user_width *
-   icd-user_height;
+   phys_addr_top += icd-user_width * icd-user_height;
ceu_write(pcdev, CDACR, phys_addr_top);
+   if (phys_addr_top  3)
+   return -EINVAL;
+
if (pcdev-is_interlaced) {
-   phys_addr_bottom = phys_addr_top +
-   icd-user_width;
+   phys_addr_bottom = phys_addr_top + icd-user_width;
ceu_write(pcdev, CDBCR, phys_addr_bottom);
+   if (phys_addr_bottom  3)
+   return -EINVAL;
}
}
 
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] sh_mobile_ceu_camera: Remove frame size page alignment

[Magnus Damm]

From: Magnus Damm d...@opensource.se

This patch updates the SuperH Mobile CEU driver to
not page align the frame size. Useful in the case of
USERPTR with non-page aligned frame sizes and offsets.

Signed-off-by: Magnus Damm d...@opensource.se
---

 drivers/media/video/sh_mobile_ceu_camera.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--- 0010/drivers/media/video/sh_mobile_ceu_camera.c
+++ work/drivers/media/video/sh_mobile_ceu_camera.c 2009-12-09 
17:54:37.0 +0900
@@ -199,14 +199,13 @@ static int sh_mobile_ceu_videobuf_setup(
struct sh_mobile_ceu_dev *pcdev = ici-priv;
int bytes_per_pixel = (icd-current_fmt-depth + 7)  3;
 
-   *size = PAGE_ALIGN(icd-user_width * icd-user_height *
-  bytes_per_pixel);
+   *size = icd-user_width * icd-user_height * bytes_per_pixel;
 
if (0 == *count)
*count = 2;
 
if (pcdev-video_limit) {
-   while (*size * *count  pcdev-video_limit)
+   while (PAGE_ALIGN(*size) * *count  pcdev-video_limit)
(*count)--;
}
 
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] V4L - Fix videobuf_dma_contig_user_get() getting page aligned physical address

[Magnus Damm]

Hi again Murali,

Thanks for your work on this.

On Thu, Dec 3, 2009 at 12:48 AM, Karicheri, Muralidharan
m-kariche...@ti.com wrote:
 Magnus,

Thanks for the patch. For non-page aligned user space pointers I agree
that a fix is needed. Don't you think the while loop in
videobuf_dma_contig_user_get() also needs to be adjusted to include
the last page? I think the while loop checks one page too little in
the non-aligned case today.

 Thanks for reviewing my patch. It had worked for non-aligned address in
 my testing. If I understand this code correctly, the physical address of
 the user page start is determined in the first loop (pages_done == 0)
 and additional loops are run to make sure the memory is physically
 contiguous. Initially the mem-size is set to number of pages aligned to
 page size.

 Assume we pass 4097 bytes as size.

 mem-size = PAGE_ALIGN(vb-size); = 2

 Inside the loop, iteration is done for 0 to pages-1.

 pages_done  (mem-size  12) = pages_done  2 = iterate 2 times

 For size of 4096, we iterate once.
 For size of 4095, we iterate once.

 So IMO the loop is already iterate one more time when we pass non-aligned 
 address since size is aligned to include the last page. So based on this
 could you ack my patch so that we could ask Mauro to merge it with priority?

I think your observations are correct, but I also think there is one
more hidden issue. In the case where the offset within the page is
other than 0 then we should loop once more to also check the final
page. Right now no one is checking if the last page is contiguous or
not in the case on non-page-aligned offset..

So in your case with a 4096 or 4095 size, but if the offset withing
the page is non-zero then we should loop twice to make sure the pages
really are physically contiguous. Today we only loop once based on the
size. We should also include the offset in the calculation of number
of pages to check.

If you can include that fix in your patch that would be great. If not
then i'll fix it up myself.

Thanks!

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] soc-camera: Add mt9t112 camera support

[Magnus Damm]

On Thu, Nov 19, 2009 at 6:15 PM, Kuninori Morimoto
morimoto.kunin...@renesas.com wrote:
 Signed-off-by: Kuninori Morimoto morimoto.kunin...@renesas.com
 ---
 Guennadi

 I add new number in v4l2-chip-ident.h
 Is it OK for you ?

 This camera is very picky.
 So, it have a lot of constant value.

 The register of mt9t112 and mt9t111 are same.
 But I have mt9t112 only.
 mt9t111 should also work, but I can not check.

 This patch is based on your 20091105 patches.

  drivers/media/video/Kconfig     |    6 +
  drivers/media/video/Makefile    |    1 +
  drivers/media/video/mt9t112.c   | 1158 
 +++
  include/media/mt9t112.h         |   32 ++
  include/media/v4l2-chip-ident.h |    2 +
  5 files changed, 1199 insertions(+), 0 deletions(-)
  create mode 100644 drivers/media/video/mt9t112.c
  create mode 100644 include/media/mt9t112.h

Hi Morimoto-san,

Do you have any mt9t112 platform data for the ecovec board? I'd like
to try out this patch but I don't know which board specific parts that
are missing!

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] soc-camera: Add mt9t112 camera support

[Magnus Damm]

Hey Morimoto-san,

On Thu, Nov 26, 2009 at 9:50 AM, Kuninori Morimoto
morimoto.kunin...@renesas.com wrote:
 Do you have any mt9t112 platform data for the ecovec board? I'd like
 to try out this patch but I don't know which board specific parts that
 are missing!

 Yes I have.
 I attached it.
 This platform patch is based on Guennadi's latest patches.

 I also attached tw9910 platform patch.
 Please apply in order of tw9910 - mt9t112.

Thanks for the patches.

So now I've done some testing of the mt9t112 sensor hooked up to CEU0
on the ecovec board. I tried 16-bit RGB and NV12 in various
resolutions with mplayer. My only comment is that it seems to take a
bit of time to setup the sensor initially, but that may be something
related to the camera sensor itself.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Linux Plumbers Conference 2009: V4L2 API discussions

[Magnus Damm]

On Wed, Aug 5, 2009 at 5:14 AM, Karicheri,
Muralidharanm-kariche...@ti.com wrote:
 2) Previewer  Resizer driver. I am working with Vaibhav who had worked on an 
 RFC for this. The previewer and resizer devices are doing memory to memory 
 operations. Also should be flexible to use these hardware with capture driver 
 to do on the fly preview and resize. The TI hardware is parameter intensive. 
 We believe these parameters are to be exported to user space through IOCTLs 
 and would require addition of new IOCTLs and extension of control IDs. We 
 will be working with you on this as well.

FWIW, for our SuperH Mobile devices we make use of UIO and user space
libraries to support for our on-chip multimedia blocks. These blocks
do scaling, rotation, color space conversion and hardware
encode/decode of various formats including h264 and mpeg4 in HD
resolution.

Apart from UIO we use V4L2 for the camera capture interface driver
sh_mobile_ceu_camera.c. It has support for on the fly color space
conversion and scaling/cropping. The CEU driver is making use of
videobuf-dma-contig.c and the USERPTR changes included in 2.6.31-rc
gives the driver zero copy frame capture support.

All of this is of course available upstream.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] adding support for setting bus parameters in sub device

[Magnus Damm]

On Wed, Jun 17, 2009 at 5:33 PM, Hans Verkuilhverk...@xs4all.nl wrote:
 I think automatic negotiation is a good thing if it is implemented
 correctly.

 Actually, i think modelling software after hardware is a good thing
 and from that perspective the soc_camera was (and still is) a very
 good fit for our on-chip SoC. Apart from host/sensor separation, the
 main benefits in my mind are autonegotiation and separate
 configuration for camera sensor, capture interface and board.

 I don't mind doing the same outside soc_camera, and I agree with Hans
 that in some cases it's nice to hard code and skip the magic
 negotiation. I'm however pretty sure the soc_camera allows hard coding
 though, so in that case you get the best of two worlds.

 It is my strong opinion that while autonegotiation is easy to use, it is
 not a wise choice to make. Filling in a single struct with the bus
 settings to use for each board-subdev combination (usually there is only
 one) is simple, straight-forward and unambiguous. And I really don't see
 why that should take much time at all. And I consider it a very good point
 that the programmer is forced to think about this for a bit.

I agree that it's good to force the programmer to think. In this case
I assume you are talking about the board support engineer or at least
the person writing software to attach a camera sensor with capture
hardware.

You are not against letting drivers export their capabilites at least?
I'd like to see drivers that exports capabilites about which signals
that are supported and which states that are valid. So for instance,
the SuperH CEU driver supports both active high and active low HSYNC
and VSYNC signals. I'd like to make sure that the driver writers are
forced to think and export a bitmap of capabilites describing all
valid pin states. A little bit in the same way that i2c drivers use
-functionality() to export a bitmap of capabilites. Then if the
assignment of the pin states is automatic or hard coded I don't care
about.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] adding support for setting bus parameters in sub device

[Magnus Damm]

On Mon, Jun 15, 2009 at 12:33 AM, Guennadi
Liakhovetskig.liakhovet...@gmx.de wrote:
 On Fri, 12 Jun 2009, Hans Verkuil wrote:

 On Friday 12 June 2009 14:59:03 Guennadi Liakhovetski wrote:
  On Fri, 12 Jun 2009, Hans Verkuil wrote:
 
1. it is very unusual that the board designer has to mandate what 
signal
polarity has to be used - only when there's additional logic between 
the
capture device and the host. So, we shouldn't overload all boards with
this information. Board-code authors will be grateful to us!
  
   I talked to my colleague who actually designs boards like that about what
   he would prefer. His opinion is that he wants to set this himself, rather
   than leave it as the result of a software negotiation. It simplifies
   verification and debugging the hardware, and in addition there may be
   cases where subtle timing differences between e.g. sampling on a falling
   edge vs rising edge can actually become an important factor, particularly
   on high frequencies.

Let me guess, your coworker is a hardware designer? Letting hardware
people do hardware design is usually a good idea, but I'm yet to see
good software written by hardware people. =)

  I'd say this is different. You're talking about cases where you _want_ to
  be able to configure it explicitly, I am saying you do not have to _force_
  all to do this. Now, this selection only makes sense if both are
  configurable, right? In this case, e.g., pxa270 driver does support
  platform-specified preference. So, if both the host and the client can
  configure either polarity in the software you _can_ still specify the
  preferred one in platform data and it will be used.
 
  I think, the ability to specify inverters and the preferred polarity
  should cover all possible cases.

 In my opinion you should always want to set this explicitly. This is not
 something you want to leave to chance. Say you autoconfigure this. Now
 someone either changes the autoconf algorithm, or a previously undocumented
 register was discovered for the i2c device and it can suddenly configure the
 polarity of some signal that was previously thought to be fixed, or something
 else happens causing a different polarity to be negotiated.

 TBH, the argumentation like someone changes the autoconf algorithm or
 previously undocumented register is discovered doesn't convince me. In
 any case, I am adding authors, maintainers and major contributors to
 various soc-camera host drivers to CC and asking them to express their
 opinion on this matter. I will not add anything else here to avoid any
 unfair competition:-) they will have to go a couple emails back in this
 thread to better understand what is being discussed here.

I think automatic negotiation is a good thing if it is implemented correctly.

Actually, i think modelling software after hardware is a good thing
and from that perspective the soc_camera was (and still is) a very
good fit for our on-chip SoC. Apart from host/sensor separation, the
main benefits in my mind are autonegotiation and separate
configuration for camera sensor, capture interface and board.

I don't mind doing the same outside soc_camera, and I agree with Hans
that in some cases it's nice to hard code and skip the magic
negotiation. I'm however pretty sure the soc_camera allows hard coding
though, so in that case you get the best of two worlds.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] adding support for setting bus parameters in sub device

[Magnus Damm]

On Tue, Jun 16, 2009 at 11:33 PM, Karicheri,
Muralidharanm-kariche...@ti.com wrote:

 snip

 [MK]In that case can't the driver just ignore the field polarity? I
assume that drivers implement the parameter that has support in hardware.
So it is not an issue.

No, because the same driver runs on hardware that also has the field
signal. So we need to be able to give information about which signals
that the board actually implement. We already do this with the
soc_camera framework and it is working just fine.


 Hardware with field signal used (driver use polarity from platform data and 
 set it in the hardware)
 Hardware with field signal not used (In this case, even though the driver 
 sets it in the hardware, it is not really used in the hardware design and 
 hence is a don't care. right?

 So I don't see why it matters.

Maybe I'm misunderstanding what you are trying to do. But how can the
camera sensor driver check if the field signal is present or not? The
camera sensor driver may need information if a pin is present or not
for some decision? Perhaps for hardware configuration?

A good example IMO is the tw9910 driver and the mpout signal. Right
now the mpout configuration is part of the platform data, but maybe it
would make more sense to allow the driver to check if field is used on
the platform and if so configure the pin accordingly.

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] adding support for setting bus parameters in sub device

[Magnus Damm]

On Wed, Jun 10, 2009 at 5:55 AM, m-kariche...@ti.com wrote:
 From: Muralidharan Karicheri a0868...@gt516km11.gt.design.ti.com

 re-sending with RFC in the header

 This patch adds support for setting bus parameters such as bus type
 (BT.656, BT.1120 etc), width (example 10 bit raw image data bus)
 and polarities (vsync, hsync, field etc) in sub device. This allows
 bridge driver to configure the sub device for a specific set of bus
 parameters through s_bus() function call.

 Reviewed By Hans Verkuil.
 Signed-off-by: Muralidharan Karicheri m-kariche...@ti.com
 ---
 Applies to v4l-dvb repository

  include/media/v4l2-subdev.h |   36 
  1 files changed, 36 insertions(+), 0 deletions(-)

 diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
 index 1785608..c1cfb3b 100644
 --- a/include/media/v4l2-subdev.h
 +++ b/include/media/v4l2-subdev.h
 @@ -37,6 +37,41 @@ struct v4l2_decode_vbi_line {
        u32 type;               /* VBI service type (V4L2_SLICED_*). 0 if no 
 service found */
  };

 +/*
 + * Some sub-devices are connected to the bridge device through a bus that
 + * carries * the clock, vsync, hsync and data. Some interfaces such as BT.656
 + * carries the sync embedded in the data where as others have separate line
 + * carrying the sync signals. The structure below is used by bridge driver to
 + * set the desired bus parameters in the sub device to work with it.
 + */
 +enum v4l2_subdev_bus_type {
 +       /* BT.656 interface. Embedded sync */
 +       V4L2_SUBDEV_BUS_BT_656,
 +       /* BT.1120 interface. Embedded sync */
 +       V4L2_SUBDEV_BUS_BT_1120,
 +       /* 8 bit muxed YCbCr bus, separate sync and field signals */
 +       V4L2_SUBDEV_BUS_YCBCR_8,
 +       /* 16 bit YCbCr bus, separate sync and field signals */
 +       V4L2_SUBDEV_BUS_YCBCR_16,
 +       /* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */
 +       V4L2_SUBDEV_BUS_RAW_BAYER
 +};
 +
 +struct v4l2_subdev_bus {
 +       enum v4l2_subdev_bus_type type;
 +       u8 width;
 +       /* 0 - active low, 1 - active high */
 +       unsigned pol_vsync:1;
 +       /* 0 - active low, 1 - active high */
 +       unsigned pol_hsync:1;
 +       /* 0 - low to high , 1 - high to low */
 +       unsigned pol_field:1;
 +       /* 0 - sample at falling edge , 1 - sample at rising edge */
 +       unsigned pol_pclock:1;
 +       /* 0 - active low , 1 - active high */
 +       unsigned pol_data:1;
 +};

As for the pins/signals, I wonder if per-signal polarity/edge is
enough. If this is going to be used by/replace the soc_camera
interface then we also need to know if the signal is present or not.
For instance, I have a SuperH board using my CEU driver together with
one OV7725 camera or one TW9910 video decoder. Some revisions of the
board do not route the field signal between the SuperH on-chip CEU and
the TW9910. Both the CEU and the TW9910 support this signal, it just
happen to be missing. I think we need a way to include this board
specific property somehow.

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] Convert SuperH camera-enabled platforms to soc-camera as platform_device

[Magnus Damm]

On Wed, May 13, 2009 at 12:13 AM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 Now that soc-camera compatibility patch is in the mainline, we can convert
 all platforms to the new scheme. This patch series converts SuperH boards.
 Unfortunately, the first patch has to also (slightly) modify two camera
 drivers, but that looks like a minor inconvenience to me, at least when
 compared to my original convert-all-at-once mega-patch.

I've tried capturing with these patches applied to next-20090520.
Works fine on Migo-R. The boot output looks ok on ap325 as well
(ov772x detected), have not tried capturing with that board though.
Thanks for your help!

Acked-by: Magnus Damm d...@igel.co.jp
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] videobuf-dma-contig: zero copy USERPTR V3 comments

[Magnus Damm]

From: Magnus Damm d...@igel.co.jp

This patch adds function descriptions to V3 of the V4L2
videobuf-dma-contig USERPTR zero copy patch.

Signed-off-by: Magnus Damm d...@igel.co.jp
---

 drivers/media/video/videobuf-dma-contig.c |   16 
 1 file changed, 16 insertions(+)

--- 0005/drivers/media/video/videobuf-dma-contig.c
+++ work/drivers/media/video/videobuf-dma-contig.c  2009-05-12 
21:14:40.0 +0900
@@ -110,6 +110,12 @@ static struct vm_operations_struct video
.close= videobuf_vm_close,
 };
 
+/**
+ * videobuf_dma_contig_user_put() - reset pointer to user space buffer
+ * @mem: per-buffer private videobuf-dma-contig data
+ *
+ * This function resets the user space pointer 
+ */
 static void videobuf_dma_contig_user_put(struct videobuf_dma_contig_memory 
*mem)
 {
mem-is_userptr = 0;
@@ -117,6 +123,16 @@ static void videobuf_dma_contig_user_put
mem-size = 0;
 }
 
+/**
+ * videobuf_dma_contig_user_get() - setup user space memory pointer
+ * @mem: per-buffer private videobuf-dma-contig data
+ * @vb: video buffer to map
+ *
+ * This function validates and sets up a pointer to user space memory.
+ * Only physically contiguous pfn-mapped memory is accepted.
+ *
+ * Returns 0 if successful.
+ */
 static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem,
struct videobuf_buffer *vb)
 {
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] videobuf-dma-contig: zero copy USERPTR support V3

[Magnus Damm]

On Mon, May 11, 2009 at 10:36 PM, Mauro Carvalho Chehab
mche...@infradead.org wrote:
 Em Fri, 8 May 2009 13:06:58 -0700
 Andrew Morton a...@linux-foundation.org escreveu:

 On Fri, 08 May 2009 17:53:10 +0900
 Magnus Damm magnus.d...@gmail.com wrote:

  From: Magnus Damm d...@igel.co.jp
 
  This is V3 of the V4L2 videobuf-dma-contig USERPTR zero copy patch.
 
  Since videobuf-dma-contig is designed to handle physically contiguous
  memory, this patch modifies the videobuf-dma-contig code to only accept
  a user space pointer to physically contiguous memory. For now only
  VM_PFNMAP vmas are supported, so forget hotplug.
 
  On SuperH Mobile we use this with our sh_mobile_ceu_camera driver
  together with various multimedia accelerator blocks that are exported to
  user space using UIO. The UIO kernel code exports physically contiguous
  memory to user space and lets the user space application mmap() this memory
  and pass a pointer using the USERPTR interface for V4L2 zero copy 
  operation.
 
  With this approach we support zero copy capture, hardware scaling and
  various forms of hardware encoding and decoding.
 
  Signed-off-by: Magnus Damm d...@igel.co.jp

 Acked-by: Mauro Carvalho Chehab mche...@redhat.com

Thank you!

 What does it do, how does it do it and why does it do it?

 A good documentation is a really good idea here. There videobuf internals are
 very complex. A good documentation for it is very important to keep it 
 updated.

I've just posted a little patch that adds function descriptions,
hopefully that is one step in the right direction.

 I would also suggest if you could also take a look at videobuf-vmalloc and 
 implement a
 similar method to provide USERPTR. The vmalloc flavor can easily be tested 
 with
 the virtual (vivi) video driver, so it helps people to better understand how
 videobuf works. It will also help the USB drivers that use videobuf to use 
 USERPTR.

Yeah, supporting USERPTR with vivi sounds like a good plan. I'm not
sure how much work it involves though. The comment in the
videobuf-vmalloc header says that the buffer code assumes that the
driver does not touch the data, but I think that's exactly how vivi
generates the frame data for us. =)

I need to figure out the best way to grab references to user space
pages and map them virtually contiguous like vmalloc does. This will
take a bit of time, so don't expect anything submitted in time for
v2.6.31. I've put it fairly high on my TODO list.

Thanks for your help!

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/3] mm: introduce follow_pte()

[Magnus Damm]

On Wed, May 6, 2009 at 6:21 AM, Johannes Weiner han...@cmpxchg.org wrote:
 On Tue, May 05, 2009 at 02:05:17PM -0700, Andrew Morton wrote:
 On Tue, 5 May 2009 22:38:07 +0200
 Johannes Weiner han...@cmpxchg.org wrote:
  On Tue, May 05, 2009 at 12:24:42PM -0700, Andrew Morton wrote:
   On Mon,  4 May 2009 11:54:32 +0200
   Johannes Weiner han...@cmpxchg.org wrote:
  
A generic readonly page table lookup helper to map an address space
and an address from it to a pte.
  
   umm, OK.
  
   Is there actually some point to these three patches?  If so, what is it?
 
  Magnus needs to check for physical contiguity of a VMAs backing pages
  to support zero-copy exportation of video data to userspace.
 
  This series implements follow_pfn() so he can walk the VMA backing
  pages and ensure their PFNs are in linear order.
 
  [ This patch can be collapsed with 2/3, I just thought it would be
    easier to read the diffs when having them separate. ]
 
  1/3 and 2/3: factor out the page table walk from follow_phys() into
  follow_pte().
 
  3/3: implement follow_pfn() on top of follow_pte().

 So we could bundle these patches with Magnus's patchset, or we could
 consider these three patches as a cleanup or something.

 Given that 3/3 introduces an unused function, I'm inclined to sit tight
 and await Magnus's work.

 Yeah, I didn't see the video guys responding on Magnus' patch yet, so
 let's wait for them.

 Magnus, the actual conversion of your code should be trivial, could
 you respin it on top of these three patches using follow_pfn() then?

So I tested the patches in -mm (1/3, 2/3, 3/3) together with the zero
copy patch and everything seems fine. Feel free to add acks from me,
least for patch 1/3 and 3/3 - i know too little about the generic case
to say anything about 2/3.

Acked-by: Magnus Damm d...@igel.co.jp

I'll send V3 of my zero copy patch in a little while. Thanks a lot for the help!

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] videobuf-dma-contig: zero copy USERPTR support V3

[Magnus Damm]

From: Magnus Damm d...@igel.co.jp

This is V3 of the V4L2 videobuf-dma-contig USERPTR zero copy patch.

Since videobuf-dma-contig is designed to handle physically contiguous
memory, this patch modifies the videobuf-dma-contig code to only accept
a user space pointer to physically contiguous memory. For now only
VM_PFNMAP vmas are supported, so forget hotplug.

On SuperH Mobile we use this with our sh_mobile_ceu_camera driver
together with various multimedia accelerator blocks that are exported to
user space using UIO. The UIO kernel code exports physically contiguous
memory to user space and lets the user space application mmap() this memory
and pass a pointer using the USERPTR interface for V4L2 zero copy operation.

With this approach we support zero copy capture, hardware scaling and
various forms of hardware encoding and decoding.

Signed-off-by: Magnus Damm d...@igel.co.jp
---

 Needs the following patches (Thanks to Johannes Weiner and akpm):
 - mm-introduce-follow_pte.patch
 - mm-use-generic-follow_pte-in-follow_phys.patch
 - mm-introduce-follow_pfn.patch
 
 Tested on SH7722 Migo-R with a hacked up capture.c

 Changes since V2:
 - use follow_pfn(), drop mm/memory.c changes

 Changes since V1:
 - minor cleanups and formatting changes
 - use follow_phys() in videobuf-dma-contig instead of duplicating code
 - since videobuf-dma-contig can be a module: EXPORT_SYMBOL(follow_phys)
 - move CONFIG_HAVE_IOREMAP_PROT to always build follow_phys()

 drivers/media/video/videobuf-dma-contig.c |   78 +++--
 1 file changed, 73 insertions(+), 5 deletions(-)

--- 0013/drivers/media/video/videobuf-dma-contig.c
+++ work/drivers/media/video/videobuf-dma-contig.c  2009-05-08 
15:57:21.0 +0900
@@ -17,6 +17,7 @@
 #include linux/init.h
 #include linux/module.h
 #include linux/mm.h
+#include linux/pagemap.h
 #include linux/dma-mapping.h
 #include media/videobuf-dma-contig.h
 
@@ -25,6 +26,7 @@ struct videobuf_dma_contig_memory {
void *vaddr;
dma_addr_t dma_handle;
unsigned long size;
+   int is_userptr;
 };
 
 #define MAGIC_DC_MEM 0x0733ac61
@@ -108,6 +110,66 @@ static struct vm_operations_struct video
.close= videobuf_vm_close,
 };
 
+static void videobuf_dma_contig_user_put(struct videobuf_dma_contig_memory 
*mem)
+{
+   mem-is_userptr = 0;
+   mem-dma_handle = 0;
+   mem-size = 0;
+}
+
+static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem,
+   struct videobuf_buffer *vb)
+{
+   struct mm_struct *mm = current-mm;
+   struct vm_area_struct *vma;
+   unsigned long prev_pfn, this_pfn;
+   unsigned long pages_done, user_address;
+   int ret;
+
+   mem-size = PAGE_ALIGN(vb-size);
+   mem-is_userptr = 0;
+   ret = -EINVAL;
+
+   down_read(mm-mmap_sem);
+
+   vma = find_vma(mm, vb-baddr);
+   if (!vma)
+   goto out_up;
+
+   if ((vb-baddr + mem-size)  vma-vm_end)
+   goto out_up;
+
+   pages_done = 0;
+   prev_pfn = 0; /* kill warning */
+   user_address = vb-baddr;
+
+   while (pages_done  (mem-size  PAGE_SHIFT)) {
+   ret = follow_pfn(vma, user_address, this_pfn);
+   if (ret)
+   break;
+
+   if (pages_done == 0)
+   mem-dma_handle = this_pfn  PAGE_SHIFT;
+   else if (this_pfn != (prev_pfn + 1))
+   ret = -EFAULT;
+
+   if (ret)
+   break;
+
+   prev_pfn = this_pfn;
+   user_address += PAGE_SIZE;
+   pages_done++;
+   }
+
+   if (!ret)
+   mem-is_userptr = 1;
+
+ out_up:
+   up_read(current-mm-mmap_sem);
+
+   return ret;
+}
+
 static void *__videobuf_alloc(size_t size)
 {
struct videobuf_dma_contig_memory *mem;
@@ -154,12 +216,11 @@ static int __videobuf_iolock(struct vide
case V4L2_MEMORY_USERPTR:
dev_dbg(q-dev, %s memory method USERPTR\n, __func__);
 
-   /* The only USERPTR currently supported is the one needed for
-  read() method.
-*/
+   /* handle pointer from user space */
if (vb-baddr)
-   return -EINVAL;
+   return videobuf_dma_contig_user_get(mem, vb);
 
+   /* allocate memory for the read() method */
mem-size = PAGE_ALIGN(vb-size);
mem-vaddr = dma_alloc_coherent(q-dev, mem-size,
mem-dma_handle, GFP_KERNEL);
@@ -386,7 +447,7 @@ void videobuf_dma_contig_free(struct vid
   So, it should free memory only if the memory were allocated for
   read() operation.
 */
-   if ((buf-memory != V4L2_MEMORY_USERPTR) || buf-baddr)
+   if (buf-memory != V4L2_MEMORY_USERPTR)
return;
 
if (!mem)
@@ -394,6 +455,13

Re: [PATCH] videobuf-dma-contig: zero copy USERPTR support V2

[Magnus Damm]

On Tue, Apr 28, 2009 at 6:01 PM, Magnus Damm magnus.d...@gmail.com wrote:
 This is V2 of the V4L2 videobuf-dma-contig USERPTR zero copy patch.

I guess the V4L2 specific bits are pretty simple.

As for the minor mm modifications below,

 --- 0001/mm/memory.c
 +++ work/mm/memory.c    2009-04-28 14:56:43.0 +0900
 @@ -3009,7 +3009,6 @@ int in_gate_area_no_task(unsigned long a

  #endif /* __HAVE_ARCH_GATE_AREA */

 -#ifdef CONFIG_HAVE_IOREMAP_PROT
  int follow_phys(struct vm_area_struct *vma,
                unsigned long address, unsigned int flags,
                unsigned long *prot, resource_size_t *phys)

Is it ok with the memory management guys to always build follow_phys()?

 @@ -3063,7 +3062,9 @@ unlock:
  out:
        return ret;
  }
 +EXPORT_SYMBOL(follow_phys);

 +#ifdef CONFIG_HAVE_IOREMAP_PROT
  int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
                        void *buf, int len, int write)
  {

How about exporting follow_phys()? This because the user
videobuf-dma-contig.c can be built as a module.

Should I use EXPORT_SYMBOL_GPL() instead of EXPORT_SYMBOL()?

Any comments?

Thanks,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] videobuf-dma-contig: remove sync operation

[Magnus Damm]

From: Magnus Damm d...@igel.co.jp

Remove the videobuf-dma-contig sync operation. Sync is only needed
for noncoherent buffers, and since videobuf-dma-contig is built on
coherent memory allocators the memory is by definition always in sync.

Reported-by: Matthieu CASTET matthieu.cas...@parrot.com
Signed-off-by: Magnus Damm d...@igel.co.jp
---

 Thanks to Mattieu, Paul and Paulius for all the help!
 Tested on SH7722 Migo-R with CEU and ov7725.

 drivers/media/video/videobuf-dma-contig.c |   14 --
 1 file changed, 14 deletions(-)

--- 0001/drivers/media/video/videobuf-dma-contig.c
+++ work/drivers/media/video/videobuf-dma-contig.c  2009-04-28 
13:09:37.0 +0900
@@ -182,19 +182,6 @@ static int __videobuf_iolock(struct vide
return 0;
 }
 
-static int __videobuf_sync(struct videobuf_queue *q,
-  struct videobuf_buffer *buf)
-{
-   struct videobuf_dma_contig_memory *mem = buf-priv;
-
-   BUG_ON(!mem);
-   MAGIC_CHECK(mem-magic, MAGIC_DC_MEM);
-
-   dma_sync_single_for_cpu(q-dev, mem-dma_handle, mem-size,
-   DMA_FROM_DEVICE);
-   return 0;
-}
-
 static int __videobuf_mmap_free(struct videobuf_queue *q)
 {
unsigned int i;
@@ -356,7 +343,6 @@ static struct videobuf_qtype_ops qops = 
 
.alloc= __videobuf_alloc,
.iolock   = __videobuf_iolock,
-   .sync = __videobuf_sync,
.mmap_free= __videobuf_mmap_free,
.mmap_mapper  = __videobuf_mmap_mapper,
.video_copy_to_user = __videobuf_copy_to_user,
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] videobuf-dma-contig: zero copy USERPTR support V2

[Magnus Damm]

From: Magnus Damm d...@igel.co.jp

This is V2 of the V4L2 videobuf-dma-contig USERPTR zero copy patch.

Since videobuf-dma-contig is designed to handle physically contiguous
memory, this patch modifies the videobuf-dma-contig code to only accept
a pointer to physically contiguous memory. For now only VM_PFNMAP vmas
are supported, so forget hotplug.

On SuperH Mobile we use this approach for our sh_mobile_ceu_camera driver
together with various multimedia accelerator blocks that are exported to
user space using UIO. The UIO kernel code exports physically contiugous
memory to user space and lets the user space application mmap() this memory
and pass a pointer using the USERPTR interface for V4L2 zero copy operation.

With this approach we support zero copy capture, hardware scaling and
various forms of hardware encoding and decoding.

Signed-off-by: Magnus Damm d...@igel.co.jp
---

 Many thanks to Hannes for the feedback!
 Tested on SH7722 Migo-R with a hacked up capture.c

 Changes since V1:
 - minor cleanups and formatting changes
 - use follow_phys() in videobuf-dma-contig instead of duplicating code
 - since videobuf-dma-contig can be a module: EXPORT_SYMBOL(follow_phys)
 - move CONFIG_HAVE_IOREMAP_PROT to always build follow_phys()

 drivers/media/video/videobuf-dma-contig.c |   82 +++--
 mm/memory.c   |3 -
 2 files changed, 79 insertions(+), 6 deletions(-)

--- 0005/drivers/media/video/videobuf-dma-contig.c
+++ work/drivers/media/video/videobuf-dma-contig.c  2009-04-28 
14:59:23.0 +0900
@@ -17,6 +17,7 @@
 #include linux/init.h
 #include linux/module.h
 #include linux/mm.h
+#include linux/pagemap.h
 #include linux/dma-mapping.h
 #include media/videobuf-dma-contig.h
 
@@ -25,6 +26,7 @@ struct videobuf_dma_contig_memory {
void *vaddr;
dma_addr_t dma_handle;
unsigned long size;
+   int is_userptr;
 };
 
 #define MAGIC_DC_MEM 0x0733ac61
@@ -108,6 +110,70 @@ static struct vm_operations_struct video
.close= videobuf_vm_close,
 };
 
+static void videobuf_dma_contig_user_put(struct videobuf_dma_contig_memory 
*mem)
+{
+   mem-is_userptr = 0;
+   mem-dma_handle = 0;
+   mem-size = 0;
+}
+
+static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem,
+   struct videobuf_buffer *vb)
+{
+   struct mm_struct *mm = current-mm;
+   struct vm_area_struct *vma;
+   unsigned long prev_pfn, this_pfn;
+   unsigned long pages_done, user_address;
+   unsigned long prot;
+   resource_size_t phys;
+   int ret;
+
+   mem-size = PAGE_ALIGN(vb-size);
+   mem-is_userptr = 0;
+   ret = -EINVAL;
+
+   down_read(mm-mmap_sem);
+
+   vma = find_vma(mm, vb-baddr);
+   if (!vma)
+   goto out_up;
+
+   if ((vb-baddr + mem-size)  vma-vm_end)
+   goto out_up;
+
+   pages_done = 0;
+   prev_pfn = 0; /* kill warning */
+   user_address = vb-baddr;
+
+   while (pages_done  (mem-size  PAGE_SHIFT)) {
+   ret = follow_phys(vma, user_address, 0, prot, phys);
+   if (ret)
+   break;
+
+   this_pfn = phys  PAGE_SHIFT;
+
+   if (pages_done == 0)
+   mem-dma_handle = phys;
+   else if (this_pfn != (prev_pfn + 1))
+   ret = -EFAULT;
+
+   if (ret)
+   break;
+
+   prev_pfn = this_pfn;
+   user_address += PAGE_SIZE;
+   pages_done++;
+   }
+
+   if (!ret)
+   mem-is_userptr = 1;
+
+ out_up:
+   up_read(current-mm-mmap_sem);
+
+   return ret;
+}
+
 static void *__videobuf_alloc(size_t size)
 {
struct videobuf_dma_contig_memory *mem;
@@ -154,12 +220,11 @@ static int __videobuf_iolock(struct vide
case V4L2_MEMORY_USERPTR:
dev_dbg(q-dev, %s memory method USERPTR\n, __func__);
 
-   /* The only USERPTR currently supported is the one needed for
-  read() method.
-*/
+   /* handle pointer from user space */
if (vb-baddr)
-   return -EINVAL;
+   return videobuf_dma_contig_user_get(mem, vb);
 
+   /* allocate memory for the read() method */
mem-size = PAGE_ALIGN(vb-size);
mem-vaddr = dma_alloc_coherent(q-dev, mem-size,
mem-dma_handle, GFP_KERNEL);
@@ -386,7 +451,7 @@ void videobuf_dma_contig_free(struct vid
   So, it should free memory only if the memory were allocated for
   read() operation.
 */
-   if ((buf-memory != V4L2_MEMORY_USERPTR) || buf-baddr)
+   if (buf-memory != V4L2_MEMORY_USERPTR)
return;
 
if (!mem)
@@ -394,6 +459,13 @@ void videobuf_dma_contig_free(struct vid

Re: [PATCH 0/5] soc-camera: convert to platform device

[Magnus Damm]

On Mon, Apr 20, 2009 at 4:22 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Mon, 20 Apr 2009, Magnus Damm wrote:
 On Fri, Apr 17, 2009 at 7:43 PM, Guennadi Liakhovetski
 g.liakhovet...@gmx.de wrote:
  On Fri, 17 Apr 2009, Magnus Damm wrote:
  On Fri, Apr 17, 2009 at 4:51 PM, Guennadi Liakhovetski
  g.liakhovet...@gmx.de wrote:
   On Fri, 17 Apr 2009, Magnus Damm wrote:
   On Wed, Apr 15, 2009 at 9:17 PM, Guennadi Liakhovetski
   g.liakhovet...@gmx.de wrote:
This patch series is a preparation for the v4l2-subdev conversion. 
Please,
review and test. My current patch-stack in the form of a
(manually-created) quilt-series is at
http://www.open-technology.de/download/20090415/ based on linux-next
history branch, commit ID in -base file. Don't be surprised, that
patch-set also contains a few not directly related patches.

 Today I've tested the following on top of linux-2.6 (stable 2.6.30-rc)
 d91dfbb41bb2e9bdbfbd2cc7078ed7436eab027a

 0033-soc-camera-host-driver-cleanup.patch
 0034-soc-camera-remove-an-extra-device-generation-from-s.patch
 0035-soc-camera-simplify-register-access-routines-in-mul.patch

 So far OK.
 However, applying 0036- or v2 from the mailing list results in rejects
 (probably because linux-next vs linux-2.6 differences) but the files
 should not affect Migo-R.

 So with 0036 or v2 applied it builds ok for Migo-R, but I can't open
 /dev/video0 for some reason. Reverting the patch and only using
 0033-0035 above results in ok /dev/video0 opening.

 Did you have video drivers build in or as modules? If modules - in which
 order did you load them? Unfortunately, at the moment it might be
 important:-( What's in dmesg after loading all drivers?

They are compiled-in. No modules.

 What are your dependencies?

 I'll try out your 20090415/series on top of the matching linux-next for now.

So linux-next fa169db2b277ebafa466d625ed2d16b2d2a4bc82 with
20090415/series applies without any rejects and compiles just fine for
Migo-R. However, during runtime I experience the same problem as with
2.6.30-rc plus 0033-0035 + 0036 or v2:

/ # /mplayer -flip -vf mirror -quiet tv://
MPlayer dev-SVN-rUNKNOWN-4.2-SH4-LINUX_v0701 (C) 2000-2008 MPlayer Team

Playing tv://.
TV file format detected.
Selected driver: v4l2
 name: Video 4 Linux 2 input
 author: Martin Olschewski olschew...@zpr.uni-koeln.de
 comment: first try, more to come ;-)
v4l2: unable to open '/dev/video0': No such device or address
v4l2: ioctl set mute failed: Bad file descriptor
v4l2: 0 frames successfully processed, 0 frames dropped.


Exiting... (End of file)
/ #

Removing 0036 unbreaks the code and mplayer/capture.c works as expected.

I also tried out v2 of your soc-camera-platform patch but it still
does not work.

Can you please test on your Migo-R board? I'd be happy to assist you
in setting up your environment.

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] soc-camera: convert to platform device

[Magnus Damm]

On Mon, Apr 20, 2009 at 5:14 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Mon, 20 Apr 2009, Magnus Damm wrote:
 Can you please test on your Migo-R board? I'd be happy to assist you
 in setting up your environment.

 I did test it and it worked - exactly as you say - with the entire patch
 stack + v2 of soc-camera: convert to platform device, the only
 difference that I can see so far, is that I used modules. So, you can
 either look in dmesg for driver initialisation whether ov772x and tw9910
 have found theit i2c chips, or just wait until I test a monolitic build
 myself. It can be problematic if the i2c-host driver initialises too
 late... If you want to test a modular build it would be enough to just
 have sh_mobile_ceu_camera.ko as a module, the rest can stay built in.

I prefer to wait then. Please consider the built-in case broken.

Usually I get some output similar to this during boot:

camera 0-0: SuperH Mobile CEU driver attached to camera 0
camera 0-0: ov7725 Product ID 77:21 Manufacturer ID 7f:a2
camera 0-0: SuperH Mobile CEU driver detached from camera 0

But with the convert to platform device patch appied I see nothing like that.

The migor_defconfig should give you the static non-module
configuration that is broken.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH][RFC] videobuf-dma-config: zero copy USERPTR support

[Magnus Damm]

From: Magnus Damm d...@igel.co.jp

Zero copy video frame capture from user space using V4L2 USERPTR.

This patch adds USERPTR support to the videobuf-dma-contig buffer code.
Since videobuf-dma-contig is designed to handle physically contiguous
memory, this patch modifies the videobuf-dma-contig code to only accept
a pointer physically contiguous memory. For now only VM_PFNMAP vmas are
supported, so forget hotplug.

On SuperH Mobile we use this approach for our V4L2 CEU driver together
with various multimedia accelerator blocks that are exported to user 
space using UIO. The UIO kernel code exports physically contiugous memory
to user space and lets the user space application mmap() this memory and
pass a pointer using the USERPTR interface for V4L2 zero copy operation.

With this approach we support zero copy capture, hardware scaling and
various forms of hardware encoding.

Hopefully this patch is useful for other SoCs. For user space example
code I suggest having a look at the USERPTR implementation in capture.c.

Any comments? Does anyone need to use memory backed by struct page?

Signed-off-by: Magnus Damm d...@igel.co.jp
---

 drivers/media/video/videobuf-dma-contig.c |  138 +++--
 1 file changed, 131 insertions(+), 7 deletions(-)

--- 0001/drivers/media/video/videobuf-dma-contig.c
+++ work/drivers/media/video/videobuf-dma-contig.c  2009-04-20 
18:04:32.0 +0900
@@ -17,6 +17,8 @@
 #include linux/init.h
 #include linux/module.h
 #include linux/mm.h
+#include linux/hugetlb.h
+#include linux/pagemap.h
 #include linux/dma-mapping.h
 #include media/videobuf-dma-contig.h
 
@@ -25,6 +27,7 @@ struct videobuf_dma_contig_memory {
void *vaddr;
dma_addr_t dma_handle;
unsigned long size;
+   int is_userptr;
 };
 
 #define MAGIC_DC_MEM 0x0733ac61
@@ -108,6 +111,117 @@ static struct vm_operations_struct video
.close= videobuf_vm_close,
 };
 
+static void videobuf_dma_contig_user_put(struct videobuf_dma_contig_memory 
*mem)
+{
+   mem-is_userptr = 0;
+   mem-dma_handle = 0;
+   mem-size = 0;
+}
+
+/* modelled after follow_phys() in mm/memory.c */
+static int get_pfn(struct vm_area_struct *vma,
+  unsigned long address, unsigned long *pfnp)
+{
+   struct mm_struct *mm = vma-vm_mm;
+   pgd_t *pgd;
+   pud_t *pud;
+   pmd_t *pmd;
+   pte_t *ptep, pte;
+   spinlock_t *ptl;
+
+   if (!(vma-vm_flags  (VM_IO | VM_PFNMAP)))
+   goto no_page_table;
+
+   pgd = pgd_offset(mm, address);
+   if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
+   goto no_page_table;
+
+   pud = pud_offset(pgd, address);
+   if (pud_none(*pud) || unlikely(pud_bad(*pud)))
+   goto no_page_table;
+
+   pmd = pmd_offset(pud, address);
+   if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
+   goto no_page_table;
+
+   /* We cannot handle huge page PFN maps. Luckily they don't exist. */
+   if (pmd_huge(*pmd))
+   goto no_page_table;
+
+   ptep = pte_offset_map_lock(mm, pmd, address, ptl);
+   if (!ptep)
+   goto no_page_table;
+
+   pte = *ptep;
+   if (!pte_present(pte))
+   goto unlock;
+
+   *pfnp = pte_pfn(pte);
+   pte_unmap_unlock(ptep, ptl);
+   return 0;
+unlock:
+   pte_unmap_unlock(ptep, ptl);
+no_page_table:
+   return -EINVAL;
+}
+
+
+static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem,
+   struct videobuf_buffer *vb)
+{
+   struct mm_struct *mm = current-mm;
+   struct vm_area_struct *vma;
+   unsigned long prev_pfn, this_pfn;
+   unsigned long pages_done, user_address;
+   int ret;
+
+   mem-size = PAGE_ALIGN(vb-size);
+   mem-is_userptr = 0;
+   ret = -EINVAL;
+
+   down_read(mm-mmap_sem);
+
+   vma = find_vma(mm, vb-baddr);
+   if (!vma)
+   goto out_up;
+
+   if ((vb-baddr + mem-size)  vma-vm_end)
+   goto out_up;
+
+   pages_done = 0;
+   prev_pfn = 0; /* kill warning */
+   user_address = vb-baddr;
+
+   while (pages_done  (mem-size  PAGE_SHIFT)) {
+   ret = get_pfn(vma, user_address, this_pfn);
+   if (ret)
+   break;
+
+   if (pages_done == 0) {
+   prev_pfn = this_pfn;
+   mem-dma_handle = this_pfn  PAGE_SHIFT;
+   } else {
+   if (this_pfn != (prev_pfn + 1))
+   ret = -EFAULT;
+   }
+
+   if (ret)
+   break;
+
+   prev_pfn = this_pfn;
+   user_address += PAGE_SIZE;
+   pages_done++;
+   }
+
+   if (!ret  pages_done)
+   mem-is_userptr = 1;
+
+ out_up:
+   up_read(current-mm-mmap_sem);
+
+   return ret;
+}
+
 static void *__videobuf_alloc(size_t size

Re: [PATCH 0/5] soc-camera: convert to platform device

[Magnus Damm]

Hi Guennadi,

On Wed, Apr 15, 2009 at 9:17 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 This patch series is a preparation for the v4l2-subdev conversion. Please,
 review and test. My current patch-stack in the form of a
 (manually-created) quilt-series is at
 http://www.open-technology.de/download/20090415/ based on linux-next
 history branch, commit ID in -base file. Don't be surprised, that
 patch-set also contains a few not directly related patches.

Testing on Migo-R board with 2.6.30-rc2-git-something and the
following cherry-picked patches:

0007-driver-core-fix-driver_match_device.patch
0033-soc-camera-host-driver-cleanup.patch
0034-soc-camera-remove-an-extra-device-generation-from-s.patch
0035-soc-camera-simplify-register-access-routines-in-mul.patch
and part of 0036 (avoiding rejects, ap325 seems broken btw)

It compiles just fine, boots up allright, but I can't open /dev/video0 anymore.

It's still supposed to work with all drivers compiled-in, right?

I'll investigate a bit more and update to latest linux-2.6 git.

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] soc-camera: convert to platform device

[Magnus Damm]

On Fri, Apr 17, 2009 at 4:51 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Fri, 17 Apr 2009, Magnus Damm wrote:
 On Wed, Apr 15, 2009 at 9:17 PM, Guennadi Liakhovetski
 g.liakhovet...@gmx.de wrote:
  This patch series is a preparation for the v4l2-subdev conversion. Please,
  review and test. My current patch-stack in the form of a
  (manually-created) quilt-series is at
  http://www.open-technology.de/download/20090415/ based on linux-next
  history branch, commit ID in -base file. Don't be surprised, that
  patch-set also contains a few not directly related patches.

 Testing on Migo-R board with 2.6.30-rc2-git-something and the
 following cherry-picked patches:

 0007-driver-core-fix-driver_match_device.patch
 0033-soc-camera-host-driver-cleanup.patch
 0034-soc-camera-remove-an-extra-device-generation-from-s.patch
 0035-soc-camera-simplify-register-access-routines-in-mul.patch
 and part of 0036 (avoiding rejects, ap325 seems broken btw)

 Have I broken it or is it unrelated?

2.6.30-rc seems broken on Migo-R. A quick check suggests the following:

V4L/DVB (10141): OK
V4L/DVB (10672): BAD
V4L/DVB (11024): BAD

OK means mplayer capture works as excepted with CEU and ov772x.
BAD means failure to open() /dev/video0 in the case of CEU. vivi works fine.

Morimoto-san and/or Guennadi, do you see the same thing?

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] sh_mobile_ceu: Add field signal operation

[Magnus Damm]

On Thu, Feb 5, 2009 at 5:04 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Tue, 3 Feb 2009, Kuninori Morimoto wrote:

 sh_mobile_ceu can support field signal from external module.
 To support this operation, SH_CEU_FLAG_USE_FLDID_{HIGH, LOW}
 are added to header.

 I never dealt with interlaced video, so, I probably just don't understand
 something, please explain. I understand the Field ID signal is optional,
 and, if used, it can be either active high or low. But who gets to decide
 which polarity is applicable in a specific case? The platform? Is it not
 like with other control signals, where if both partners are freely
 configurable, then any polarity can be used; if one is configurable and
 another is hard-wired, only one polarity can be used. And as long as the
 signal is available (connected), the platform has no further influence on
 its use? Ok, there can be an inverter there, but that we can handle too.

I believe you are correct. It is just yet another signal.

 So, wouldn't something like

 +       if (pcdev-pdata-flags  SH_CEU_FLAG_USE_FLDID)
 +               flags |= SOCAM_FIELD_ID_ACTIVE_HIGH | 
 SOCAM_FIELD_ID_ACTIVE_LOW;
 +

 ...

 +       if (common_flags  (SOCAM_FIELD_ID_ACTIVE_HIGH | 
 SOCAM_FIELD_ID_ACTIVE_LOW) ==
 +           SOCAM_FIELD_ID_ACTIVE_LOW)
 +               /* The client only supports active low field ID */
 +               value |= 1  16;
 +       /* Otherwise we are free to choose, leave default active high */

 Or does Field ID work differently?

Nope, it works just like that. I guess what makes this confusing is
that we have some boards that don't have the FLD signal. So far the
CEU driver has had it's configuration passed as platform data, but
making it more generic and fit better to the soc-camera framework is
of course a good idea. But we may need a way to specify the actual
configuration of our board.

So the TV decoder chip has a field signal. So does the CEU. But not
early versions of the board.

 And what do you do, if the platform doesn't specify SH_CEU_FLAG_USE_FLDID,
 i.e., it is not connected, but the client does? Or the other way round? In
 other words, is it a working configuration, when one of the partners
 provides this signal and the other one doesn't? I guess it is, because,
 you say, it is optional. So we shouldn't test it in
 soc_camera_bus_param_compatible()?

I made a hack half a year ago that worked around the interlaced
640x480 video with missing field signal to 640x240 progressive.
Basically exactly the same as interlace for the TV decoder but the CEU
is configured in 640x240 progressive mode and gets the double amount
of frames per second.

Not sure if we want such a feature upstream though, so maybe this may
be a non-issue. =)

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: About the specific setting for lens

[Magnus Damm]

On Tue, Feb 24, 2009 at 10:17 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Mon, 23 Feb 2009, morimoto.kunin...@renesas.com wrote:


 Dear Guennadi.

 Now MigoR and AP325 board have ov772x camera.
 However, the lens used is different.

 And I have a specific good setting value
 for the lens of AP325.

 So, I would like to add new function for
 specific lens value.
 meybe like this.
 Can I add it ?

 -- board-ap325 ---
 static const struct regval_list ov772x_lens[] = {
        { 0xAA, 0xBB }, { 0xCC, 0xDD }, { 0xEE, 0xFF },
        ...
        ENDMARKER,
 }

 static struct ov772x_camera_info ov772x_info = {
        ...
        .lenssetting = ov772x_lens,
 }

 Hm, lenses can be replaced in principle, right? Does it really make sense
 to hard-code them in platform code? Maybe better as module parameter? Or
 are these parameters really board-specific?

I'd say that having such information with the platform data is as good
as it gets. =) In theory it's possible to replace the lens - it's
actually possible to remove the ap325 camera module as well. Using a
module parameter doesn't really make it any better since the same
driver may be used by multiple instances with different lens setups.
At least platform data makes the configuration per-camera instead of
per-driver.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] soc-camera: configure drivers with a default format on open

[Magnus Damm]

On Fri, Feb 20, 2009 at 12:20 AM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 Currently soc-camera doesn't set up any image format without an explicit 
 S_FMT.
 It seems this should be supported, since, for example, capture-example.c from
 v4l2-apps by default doesn't issue an S_FMT. This patch configures a default
 image format on open().

 Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
 ---

I like the idea behind this patch, but I wonder if it is compatible
with standard V4L2 behaviour. Please double check against the  open()
comment in section 4.1.3. Image Format Negotiation below:

http://v4l2spec.bytesex.org/spec/c6488.htm#AEN6520

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sh_mobile_ceu_camera: NV12/21/16/61 are added only once.

[Magnus Damm]

On Fri, Feb 6, 2009 at 8:21 AM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 On Fri, 23 Jan 2009, Magnus Damm wrote:
 On Fri, Jan 23, 2009 at 9:28 AM, Kuninori Morimoto
 morimoto.kunin...@renesas.com wrote:
  NV12/21/16/61 had been added every time
  UYVY/VYUY/YUYV/YVYU appears on get_formats.
  This patch modify this problem.

 That's one way to do it. Every similar driver has to do the same thing. Yuck.

 Or we could have a better translation framework that does OR for us,
 using for instance bitmaps.

 This has been on my list for a while now, but I'm quite busy these days,
 but I think I now have an idea how to fix this problem in a less
 destructive way, withoug undermining the soc-camera algorithms:-) Please,
 have a look at the patch below. Does it fix the problem for you? If not -
 how can we modify it to work for you? Notice - not even completely compile
 tested:-)

Thanks for your effort on this. From a quick glance your solution is
fine with me, from the fixing the bug perspective at least. In
practice I don't think your solution is very different from
Morimoto-sans patch though. Maybe it's a bit cleaner.

But since the number of pixel formats are finite I still think a
bitmap would be useful to represent which formats that are supported.
Setting a bit in a bitmap is a much easier than walking lists to check
for duplicate modes.

OTOH, there are not that many drivers that need this format
translation at this point so the best may be just closing this issue
with and solve it in a more generic fashion later on if there are more
users.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sh_mobile_ceu_camera: NV12/21/16/61 are added only once.

[Magnus Damm]

On Fri, Jan 23, 2009 at 9:28 AM, Kuninori Morimoto
morimoto.kunin...@renesas.com wrote:
 NV12/21/16/61 had been added every time
 UYVY/VYUY/YUYV/YVYU appears on get_formats.
 This patch modify this problem.

That's one way to do it. Every similar driver has to do the same thing. Yuck.

Or we could have a better translation framework that does OR for us,
using for instance bitmaps.

/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html