Hi Jacopo,
Thank you for the patch.
On Tuesday, 9 January 2018 18:25:23 EET Jacopo Mondi wrote:
> Add bindings documentation for Renesas Capture Engine Unit (CEU).
>
> Signed-off-by: Jacopo Mondi
Reviewed-by: Laurent Pinchart
> ---
> .../devicetree/bindings/media/renesas,ce
Hi Niklas,
On Monday, 8 January 2018 19:24:47 EET Niklas Söderlund wrote:
> On 2017-12-08 22:12:56 +0200, Laurent Pinchart wrote:
> > On Friday, 8 December 2017 03:08:35 EET Niklas Söderlund wrote:
> >> In media controller mode all VIN instances needs to be part of the sam
Hi Niklas,
On Monday, 8 January 2018 18:42:05 EET Niklas Söderlund wrote:
> On 2018-01-08 18:35:23 +0200, Laurent Pinchart wrote:
> > On Wednesday, 20 December 2017 17:20:55 EET Niklas Söderlund wrote:
> >> On 2017-12-14 17:50:24 +0200, Laurent Pinchart wrote:
> >>>
Hi Niklas,
On Wednesday, 20 December 2017 17:20:55 EET Niklas Söderlund wrote:
> On 2017-12-14 17:50:24 +0200, Laurent Pinchart wrote:
> > On Thursday, 14 December 2017 16:25:00 EET Sakari Ailus wrote:
> >> On Fri, Dec 08, 2017 at 10:17:36AM +0200, Laurent Pinchart wrote:
reg = <0x21>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&vio_pins>;
> +
> + reset-gpios = <&port3 11 GPIO_ACTIVE_LOW>;
> + powerdown-gpios = <&port3 12 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + ov7670_out: endpoint {
> + remote-endpoint = <&ceu_in>;
> +
> + hsync-active = <1>;
> + vsync-active = <0>;
> + };
> + };
> + };
> +};
--
Regards,
Laurent Pinchart
to driver interface header file
> - Adjust build system
>
> This commit does not remove the original soc_camera based driver as long
> as other platforms depends on soc_camera-based CEU driver.
>
> Signed-off-by: Jacopo Mondi
Reviewed-by: Laurent Pinchart
> ---
> d
irq_mask;
> +
> + ceudev->notifier.v4l2_dev = &ceudev->v4l2_dev;
> + ceudev->notifier.subdevs= ceudev->asds;
> + ceudev->notifier.num_subdevs= num_subdevs;
> + ceudev->notifier.ops= &ceu_notify_ops;
> + ret = v4l2_async_notifier_register(&ceudev->v4l2_dev,
> +&ceudev->notifier);
> + if (ret)
> + goto error_v4l2_unregister;
> +
> + dev_info(dev, "Renesas Capture Engine Unit %s\n", dev_name(dev));
> +
> + return 0;
> +
> +error_v4l2_unregister:
> + v4l2_device_unregister(&ceudev->v4l2_dev);
> +error_pm_disable:
> + pm_runtime_disable(dev);
> +error_free_ceudev:
> + kfree(ceudev);
> +
> + return ret;
> +}
--
Regards,
Laurent Pinchart
for CEU video buffers is now reserved with membocks APIs,
> and need to be declared as dma_coherent during machine initialization to
> remove that architecture specific part from CEU driver.
>
> Signed-off-by: Jacopo Mondi
Reviewed-by: Laurent Pinchart
Hi Jacopo,
On Wednesday, 3 January 2018 17:44:58 EET jacopo mondi wrote:
> On Tue, Jan 02, 2018 at 05:44:03PM +0200, Laurent Pinchart wrote:
> > On Thursday, 28 December 2017 16:01:19 EET Jacopo Mondi wrote:
> >> Remove soc_camera framework dependencies from ov772x sensor dri
Would you like to give it a go ?
> [1]
> http://www.iwavesystems.com/product/development-platform/qseven-evaluation->
> board/rz-g1m-qseven-development-kit-49/rz-g1m-qseven-development-kit.html
--
Regards,
Laurent Pinchart
Hi Jacopo,
On Wednesday, 3 January 2018 10:49:52 EET jacopo mondi wrote:
> On Tue, Jan 02, 2018 at 01:45:30PM +0200, Laurent Pinchart wrote:
> > On Thursday, 28 December 2017 16:01:13 EET Jacopo Mondi wrote:
> >> Add bindings documentation for Renesas Capture Engine Unit (CEU).
Hi Jacopo,
On Wednesday, 3 January 2018 12:47:48 EET jacopo mondi wrote:
> On Tue, Jan 02, 2018 at 03:46:27PM +0200, Laurent Pinchart wrote:
> >> +/*
> >> + * ceu_device - CEU device instance
> >> + */
> >> +struct ceu_device {
> >> + struct de
gt; Signed-off-by: Geert Uytterhoeven
I can't verify the parent clock, but apart from that,
Acked-by: Laurent Pinchart
> ---
> This gets rid of the following error messages during boot and system
> resume:
>
> renesas-cpg-mssr e615.clock-controller: Cannot get mo
to error_gpio_put;
> +
> + ret = v4l2_async_register_subdev(&priv->subdev);
> + if (ret)
> + goto error_gpio_put;
> +
> + return ret;
> +
> +error_gpio_put:
> + if (priv->pdn_gpio)
> + gpiod_put(priv->pdn_gpio);
> +error_clk_put:
> + clk_put(priv->clk);
>
> return ret;
> }
[snip]
With these small issues fixed, and the comments to ov7725 that apply to tw9910
addressed,
Reviewed-by: Laurent Pinchart
--
Regards,
Laurent Pinchart
o get GPIO \"rstb\"");
> + return PTR_ERR(priv->rstb_gpio);
> + }
> +
> + if (priv->rstb_gpio) {
> + gpiod_set_value(priv->rstb_gpio, 0);
> + usleep_range(500, 1000);
> + gpiod_set_value(priv->rstb_gpio, 1);
> + usleep_range(500, 1000);
> +
> + gpiod_put(priv->rstb_gpio);
> + }
> +
> + return 0;
> +}
[snip]
--
Regards,
Laurent Pinchart
ble(&ov7725_gpios);
> + gpiod_add_lookup_table(&tw9910_gpios);
> +
> i2c_register_board_info(0, migor_i2c_devices,
> ARRAY_SIZE(migor_i2c_devices));
>
> + /* Initialize CEU platform device separately to map memory first */
> + device_initialize(&migor_ceu_device.dev);
> + arch_setup_pdev_archdata(&migor_ceu_device);
> + dma_declare_coherent_memory(&migor_ceu_device.dev,
> + ceu_dma_membase, ceu_dma_membase,
> + ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE -
> 1,
> + DMA_MEMORY_EXCLUSIVE);
> +
> + platform_device_add(&migor_ceu_device);
> +
> return platform_add_devices(migor_devices, ARRAY_SIZE(migor_devices));
> }
> arch_initcall(migor_devices_setup);
[snip]
With all these minor issues fixed,
Reviewed-by: Laurent Pinchart
--
Regards,
Laurent Pinchart
};
>
> mstp7_clks: mstp7_clks@fcfe0430 {
> @@ -667,4 +667,13 @@
> power-domains = <&cpg_clocks>;
> status = "disabled";
> };
> +
> + ceu: ceu@e821 {
> + reg = <0xe821 0x209c>;
With
+ if (IS_ERR(ceudev->base))
ceudev is leaked here (and in all error paths below, including the error block
at the end of the function).
> + return PTR_ERR(ceudev->base);
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0) {
> + dev_err(dev, "failed to get irq: %d\n", ret);
> + return ret;
> + }
> + irq = ret;
> +
> + ret = devm_request_irq(dev, irq, ceu_irq,
> +0, dev_name(dev), ceudev);
> + if (ret) {
> + dev_err(&pdev->dev, "Unable to register CEU interrupt.\n");
s/register/request/
> + return ret;
> + }
> +
> + pm_suspend_ignore_children(dev, true);
Does the CEU have child devices ?
> + pm_runtime_enable(dev);
> +
> + ret = v4l2_device_register(dev, &ceudev->v4l2_dev);
> + if (ret)
> + goto error_pm_disable;
> +
> + if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> + const struct of_device_id *match =
> + of_match_device(ceu_of_match, dev);
> + struct ceu_data *ceu_data = (struct ceu_data *) match->data;
You can make this const, no space needed before match->data, and match->data
is a void pointer so there's no need to cast it explicitly.
const struct ceu_data *ceu_data =
of_match_device(ceu_of_match, dev)->data;
could do.
> +
> + ceudev->irq_mask = ceu_data->irq_mask;
> + num_subdevs = ceu_parse_dt(ceudev);
> + } else if (dev->platform_data) {
> + /* Assume SH4 if booting with platform data. */
> + ceudev->irq_mask = CEU_CETCR_ALL_IRQS_SH4;
Another option to prepare for this would be to define ceu_data_sh4 already,
move the ceu_data variable to the top of this function, assign
ceu_data = &ceu_data_sh4;
here, and move the ceudev->irq_mask = ceu_data->irq_mask; line after the
num_subdevs < 0 check below.
> + num_subdevs = ceu_parse_platform_data(ceudev,
> + dev->platform_data);
> + } else {
> + num_subdevs = -EINVAL;
> + }
> +
> + if (num_subdevs < 0) {
> + ret = num_subdevs;
> + goto error_v4l2_unregister;
> + }
> +
> + ceudev->notifier.v4l2_dev = &ceudev->v4l2_dev;
> + ceudev->notifier.subdevs= ceudev->asds;
> + ceudev->notifier.num_subdevs= num_subdevs;
> + ceudev->notifier.ops= &ceu_notify_ops;
> + ret = v4l2_async_notifier_register(&ceudev->v4l2_dev,
> +&ceudev->notifier);
> + if (ret)
> + goto error_v4l2_unregister;
> +
> + dev_info(dev, "Renesas Capture Engine Unit %s\n", dev_name(dev));
> +
> + return 0;
> +
> +error_v4l2_unregister:
> + v4l2_device_unregister(&ceudev->v4l2_dev);
> +error_pm_disable:
> + pm_runtime_disable(dev);
> +
> + return ret;
> +}
> +
> +static int ceu_remove(struct platform_device *pdev)
> +{
> + struct ceu_device *ceudev = platform_get_drvdata(pdev);
> +
> + pm_runtime_disable(ceudev->dev);
> +
> + v4l2_async_notifier_unregister(&ceudev->notifier);
> +
> + v4l2_device_unregister(&ceudev->v4l2_dev);
> +
> + video_unregister_device(&ceudev->vdev);
> +
> + return 0;
> +}
[snip]
--
Regards,
Laurent Pinchart
ing it ceu_platform_data ?
> + unsigned int num_subdevs;
> + struct ceu_async_subdev subdevs[CEU_MAX_SUBDEVS];
> +};
> +
> +#endif /* __ASM_RENESAS_CEU_H__ */
Don't forget to update the comment here too.
Apart from that,
Reviewed-by: Laurent Pinchart
--
Regards,
Laurent Pinchart
0";
> + reg = <0x21>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&vio_pins>;
> +
> + reset-gpios = <&port3 11 GPIO_ACTIVE_LOW>;
> + powerdown-gpios = <&port3 12 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + ov7670_out: endpoint {
> + remote-endpoint = <&ceu_in>;
> +
> + hsync-active = <1>;
> + vsync-active = <0>;
> + };
> + };
> + };
> +};
--
Regards,
Laurent Pinchart
(3 << 0)/* Gen2 only */
> -#define LVDCR1_CLKSTBY_GEN3 (1 << 0)/* Gen3 only */
> +#define LVDCR1_CHSTBY(n) (3 << (2 + (n) * 2))
> +#define LVDCR1_CLKSTBY (3 << 0)
>
> #define LVDPLLCR 0x0008
> #define LVDPLLCR_CEEN(1 << 14)
--
Regards,
Laurent Pinchart
Hi Jacopo,
On Friday, 22 December 2017 16:40:16 EET jacopo mondi wrote:
> On Fri, Dec 22, 2017 at 02:03:41PM +0200, Laurent Pinchart wrote:
> > On Thursday, 21 December 2017 18:27:02 EET jacopo mondi wrote:
> >> On Mon, Dec 18, 2017 at 05:28:43PM +0200, Laurent Pinchart wrote:
&
Hi Jacopo,
On Thursday, 21 December 2017 18:27:02 EET jacopo mondi wrote:
> On Mon, Dec 18, 2017 at 05:28:43PM +0200, Laurent Pinchart wrote:
> > On Monday, 18 December 2017 14:25:12 EET jacopo mondi wrote:
> >> On Mon, Dec 11, 2017 at 06:15:23PM +0200, Laurent Pinchart w
Hi Jacopo,
On Thursday, 21 December 2017 22:31:25 EET jacopo mondi wrote:
> On Tue, Dec 12, 2017 at 12:00:43PM +0200, Laurent Pinchart wrote:
> > On Wednesday, 15 November 2017 12:55:58 EET Jacopo Mondi wrote:
> >> Migo-R platform uses sh_mobile_ceu camera driver, which is now b
m/rcar-vin/rcar-csi2.c index 2793efb..90c4062
> >> 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> >> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> >> @@ -928,8 +928,8 @@ static int rcar_csi2_parse_dt(struct rcar_csi2
> >> *priv)
> >>
> >>ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> >>if (!ep) {
> >> - dev_err(priv->dev, "Not connected to subdevice\n");
> >> - return -EINVAL;
> >> + dev_dbg(priv->dev, "Not connected to subdevice\n");
> >> + return 0;
> >>}
> >>
> >>ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);
--
Regards,
Laurent Pinchart
Hi Sakari,
On Tuesday, 19 December 2017 15:28:55 EET Sakari Ailus wrote:
> On Tue, Dec 19, 2017 at 03:07:41PM +0200, Laurent Pinchart wrote:
> > On Tuesday, 19 December 2017 13:57:42 EET jacopo mondi wrote:
[snip]
> >> Ok, actually parse_dt() and parse_platform_data() b
Hi Neil,
On Tuesday, 19 December 2017 11:00:28 EET Neil Armstrong wrote:
> On 17/12/2017 01:17, Laurent Pinchart wrote:
> > Color keying is the action of replacing pixels matching a given color
> > (or range of colors) with transparent pixels in an overlay when
> > performin
Hi Jacopo,
(CC'ing Sakari)
On Tuesday, 19 December 2017 13:57:42 EET jacopo mondi wrote:
> On Mon, Dec 11, 2017 at 06:15:23PM +0200, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > [snip]
> >
> >> +s
Hi Tomi,
On Monday, 18 December 2017 14:04:45 EET Tomi Valkeinen wrote:
> On 18/12/17 13:50, Laurent Pinchart wrote:
> > That's an option too. I had a look at the code once to find out how
> > ImageMagick was performing scaling and gave up with a headache soon
> > afterw
Hi Jacopo,
On Monday, 18 December 2017 14:25:12 EET jacopo mondi wrote:
> On Mon, Dec 11, 2017 at 06:15:23PM +0200, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
>
> [snip]
>
> >> +
> >> +/**
> >> + * ceu_
Hi Tomi,
On Monday, 18 December 2017 13:46:22 EET Tomi Valkeinen wrote:
> On 18/12/17 13:36, Laurent Pinchart wrote:
> > The problem with PNG (or any other format really) is that you not only
> > need to encode the image into the target format (PNG or JPG would require
> &g
s/pykmsutil.cpp
> @@ -64,4 +64,6 @@ void init_pykmstest(py::module &m)
> m.def("save_raw_frame", [](Framebuffer& fb, const char * filename) {
> save_raw_frame(fb, filename);
> });
> + m.def("compare_framebuffers", [](Framebuffer& a, Framebuffer& b) {
> + return compare_framebuffers(a, b); } );
> }
--
Regards,
Laurent Pinchart
saw it as a better implementation than my previous
> 'C' version. (I don't spend a lot of time in C++ land)
>
> > And I'm not fond of the function name, "frame" doesn't sound good. Maybe
> > rather save_raw_fb(). Or save_fb_raw(), so in the future we might have
> > also save_fb_png().
>
> Agreed, sounds fine. I'm definitely looking forward to a save_fb_png().
>
> >> +
> >> +}
> >> diff --git a/py/pykms/pykmsutil.cpp b/py/pykms/pykmsutil.cpp
> >> index 518d5eaa88f0..2d741751ba75 100644
> >> --- a/py/pykms/pykmsutil.cpp
> >> +++ b/py/pykms/pykmsutil.cpp
> >> @@ -59,4 +59,9 @@ void init_pykmstest(py::module &m)
> >> } );
> >> m.def("draw_text", [](Framebuffer& fb, uint32_t x, uint32_t y,
> >> const
> >> string& str, RGB color) {
> >> draw_text(fb, x, y, str, color); } );
> >> +
> >> +// Verification and Validation
> >> +m.def("save_raw_frame", [](Framebuffer& fb, const char * filename) {
> >> +save_raw_frame(fb, filename);
> >> +});
> >> }
--
Regards,
Laurent Pinchart
t stream control through a single function, but that's a
matter of taste. Feel free to give it a go if you want, otherwise
Acked-by: Laurent Pinchart
> ;
> }
--
Regards,
Laurent Pinchart
Hi Kieran,
Thank you for the patch.
On Thursday, 14 December 2017 01:10:09 EET Kieran Bingham wrote:
> From: Kieran Bingham
>
> Provide a space between the return type and the function definition
>
> Signed-off-by: Kieran Bingham
Acked-by: Laurent Pinchart
> --
about this, but I thought it makes
> user confuse. Thus, I kept original number.
>
> I'm happy if compiler can adjust it automatically,
> if not, I have no objection to modify it but we want to have such comment ?
> Because above comment/explain mentions about "fvco", not "fout".
Sure, I'll add a comment, it's a good point.
> > If you agree with these small changes there's no need to resubmit the
> > patch, I'll modify it when applying, and
> >
> > Reviewed-by: Laurent Pinchart
>
> Thank you for your help
Thank you for the code :-)
--
Regards,
Laurent Pinchart
Hi Sakari,
On Monday, 18 December 2017 01:33:56 EET Sakari Ailus wrote:
> On Sun, Dec 17, 2017 at 07:03:17PM +0200, Laurent Pinchart wrote:
> > On Wednesday, 13 December 2017 20:26:19 EET Jacopo Mondi wrote:
> >> Currently, subdevice notifiers are tested against all available
fvco= finnm * P
> + * fclkout = finnm / FDPLL
> + */
> + unsigned long finnm = input * (n + 1) / (m + 1);
> + unsigned long fvco = finnm * 2;
> +
> + if (fvco < 2000 || fv
().
>
> Signed-off-by: Kuninori Morimoto
Reviewed-by: Laurent Pinchart
> ---
> v3 -> v4
>
> - no change
>
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.
4l2-async.c index c13a781..307e1a5 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -8,6 +8,10 @@
> > * published by the Free Software Foundation.
> > */
> >
> > +#if defined(CONFIG_VIDEO_V4L2_ASYNC_DEBUG)
> > +#define DEBUG
>
> Do you need this?
No this isn't needed. Debugging can be enabled through dynamic debug without
requiring the Kconfig option. A Kconfig option might be useful to avoid
compiling the debug code in kernels that have dynamic debug enabled, but those
are large already and the amount of debug code here is limited, so I don't
think it's worth it.
> > +#endif
> > +
> >
> > #include
> > #include
> > #include
[snip]
--
Regards,
Laurent Pinchart
_notifier->owner = NULL;
> + return ret;
> + }
> + }
This is the part I like the least in this patch set. The
v4l2_subdev::subdev_notifier field should really disappear, there's no reason
to limit subdevs to a single notifier. Implicit registration of notifiers is a
dirty hack in my opinion.
> mutex_lock(&list_lock);
>
> INIT_LIST_HEAD(&sd->async_list);
[snip]
--
Regards,
Laurent Pinchart
>
> You'll have struct device either through the v4l2_device or v4l2_subdev. Do
> you need an additional field for this?
I agree with this comment. If there's a reason to add a new field, its life
time constraints should be documented. The fwnodes are refcounted and you&#x
t; index f6189a3..0fc464f 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -78,6 +78,7 @@ int fwnode_property_get_reference_args(const struct
> fwnode_handle *fwnode, unsigned int nargs, unsigned int index,
> struct fwnode_reference_args *args);
>
> +const char *fwnode_get_name(const struct fwnode_handle *fwnode);
> struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle
> *fwnode);
> struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode);
--
Regards,
Laurent Pinchart
; +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -539,8 +539,12 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>* (struct v4l2_subdev.dev), and async sub-device does not
> * exist independently of the device at any point of time.
>*/
> - if (!sd->fwnode && sd->dev)
> - sd->fwnode = dev_fwnode(sd->dev);
> + if (!sd->fwnode && sd->dev) {
> + sd->fwnode = fwnode_graph_get_next_endpoint(
> + dev_fwnode(sd->dev), NULL);
> + if (!sd->fwnode)
> + sd->fwnode = dev_fwnode(sd->dev);
Shouldn't you drop the reference to the fwnode here, as the framework doesn't
release it (see the comment just above this piece of code) ? You'll have to
update the comment as well to explain the new mechanism.
> + }
>
> mutex_lock(&list_lock);
>
[snip]
--
Regards,
Laurent Pinchart
Gen3 hardware supports color keying with replacement of the pixel value.
Implement it using the standard KMS colorkey properties.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/rcar
/vsp1_rwpf.h | 5 ++
include/drm/drm_blend.h | 4 ++
include/drm/drm_plane.h | 28 ++-
include/media/vsp1.h| 5 ++
11 files changed, 235 insertions(+), 23 deletions(-)
--
Regards,
Laurent Pinchart
properties can be defined by drivers to
expose device-specific features.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/drm_atomic.c | 16 +++
drivers/gpu/drm/drm_blend.c | 108 +++
include/drm/drm_blend.h | 4 ++
include/drm/drm_plane.h
. I have no use case for that mode at the moment but I'm fairly certain
that someone will come up with one, at least if not for the R-Car for a
different display engine that provides similarly exotic features. I believe
this can for now be left for implementation through driver-specific
prope
: Alexandru Gheorghe
[Group all color key parameters in a structure]
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c | 3 +++
drivers/media/platform/vsp1/vsp1_rpf.c | 10 --
drivers/media/platform/vsp1/vsp1_rwpf.h | 5 +
include/media/vsp1.h
colorkey.mode, colorkey.min and colorkey.max properties.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 60 +++--
drivers/gpu/drm/rcar-du/rcar_du_plane.h | 2 --
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 1 -
3 files changed, 43
The test will display an overlay with large colored boxes on top of the
root plane and turn the boxes translucent or transparent in sequence
using color keying.
Signed-off-by: Laurent Pinchart
---
tests/kms-test-colorkey.py | 111 +
1 file changed
u to fetch changes up to cef620afa09ca165cbcdbdedbff0120e409ad946:
drm: rcar-du: Add support for CRC computation (2017-12-14 01:11:50 +0200)
This will pull in the -next branches of the Linux media and DRM trees.
--------
Laurent Pinchart (9):
Hi Sakari,
On Thursday, 14 December 2017 16:25:00 EET Sakari Ailus wrote:
> On Fri, Dec 08, 2017 at 10:17:36AM +0200, Laurent Pinchart wrote:
> > On Friday, 8 December 2017 03:08:21 EET Niklas Söderlund wrote:
> > > The rcar-vin driver needs to be part of a media controller to
ite it 400 * 1000 * 1000 :-)
> >>for (fdpll = 1; fdpll < 32; fdpll++) {
> >> unsigned long output;
> >
> > The output frequency on the line below can be calculated with
> >
> > output = fvco / 2 / (fdpll + 1)
> >
> > to avoid the multiplication by (n + 1) and division by (m + 1).
>
> It is nice idea to avoid extra calculation.
> I will use this idea, and add extrate comment to avoid confusion
Thank you.
--
Regards,
Laurent Pinchart
number of scatterlist entries.
Signed-off-by: Laurent Pinchart
---
Changes since v1:
- Duplicate the imported scatter gather table in
rcar_du_vsp_plane_prepare_fb()
This patch fixes a bug of the previous version and is posted in case anyone
would like to test the implementation. I still plan to giv
values that will produce an output frequency within the
tolerance with M as small as possible. This can be done as a separate patch.
And while we're discussing PLL calculation, the three nested loops will run a
total of 10044 iterations :-/ That's a lot, and should be optimized if
possible. With the outer loop operating on N an easy optimization would have
been to compute fin * N in a local variable to avoid redoing the
multiplication for every value of M, but that's not possible anymore with the
outer loop operating on M.
--
Regards,
Laurent Pinchart
Hi Mauro,
On Tuesday, 12 December 2017 14:39:32 EET Mauro Carvalho Chehab wrote:
> Em Thu, 23 Nov 2017 15:21:01 +0100 Greg Kroah-Hartman escreveu:
> > On Thu, Nov 23, 2017 at 11:07:51AM -0200, Mauro Carvalho Chehab wrote:
> >> Em Thu, 16 Nov 2017 02:33:48 +0200 Laurent
Hi Greg and Mauro,
On Thursday, 23 November 2017 16:21:01 EET Greg Kroah-Hartman wrote:
> On Thu, Nov 23, 2017 at 11:07:51AM -0200, Mauro Carvalho Chehab wrote:
> > Em Thu, 16 Nov 2017 02:33:48 +0200 Laurent Pinchart escreveu:
> >> Device unplug being asynchronous, it na
Hi Mauro,
On Thursday, 23 November 2017 15:07:51 EET Mauro Carvalho Chehab wrote:
> Em Thu, 16 Nov 2017 02:33:48 +0200 Laurent Pinchart escreveu:
> > Device unplug being asynchronous, it naturally races with operations
> > performed by userspace through ioctls or other file oper
Hi Hans,
On Friday, 17 November 2017 13:09:20 EET Hans Verkuil wrote:
> On 16/11/17 01:33, Laurent Pinchart wrote:
> > Device unplug being asynchronous, it naturally races with operations
> > performed by userspace through ioctls or other file operations on video
> > device
anaging the issue.
>
> One potential thing spotted on top of Sakari's review inline below, of
> course I suspect this was more of a prototype/consideration patch.
>
> > On Thu, Nov 16, 2017 at 02:33:48AM +0200, Laurent Pinchart wrote:
> >> Device unplug being asynchr
Hi Sakari,
On Thursday, 16 November 2017 14:32:37 EET Sakari Ailus wrote:
> On Thu, Nov 16, 2017 at 02:33:48AM +0200, Laurent Pinchart wrote:
> > Device unplug being asynchronous, it naturally races with operations
> > performed by userspace through ioctls or other file oper
(SUSP_SH_STANDBY | SUSP_SH_SF,
> &migor_sdram_enter_start,
> @@ -651,16 +636,19 @@ static int __init migor_devices_setup(void)
>*/
> __raw_writew(__raw_readw(PORT_MSELCRA) | 1, PORT_MSELCRA);
>
> - /* Setup additional memory resource for CEU video buffers */
> - r = &migor_ceu_device.resource[2];
> - r->flags = IORESOURCE_MEM;
> - r->start = ceu_dma_membase;
> - r->end = r->start + CEU_BUFFER_MEMORY_SIZE - 1;
> - r->name = "ceu";
> -
> i2c_register_board_info(0, migor_i2c_devices,
> ARRAY_SIZE(migor_i2c_devices));
>
> + /* Initialize CEU platform device separately to map memory first */
> + device_initialize(&migor_ceu_device.dev);
> + arch_setup_pdev_archdata(&migor_ceu_device);
> + dma_declare_coherent_memory(&migor_ceu_device.dev,
> + ceu_dma_membase, ceu_dma_membase,
> + ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE -
> 1,
> + DMA_MEMORY_EXCLUSIVE);
> +
> + platform_device_add(&migor_ceu_device);
> +
> return platform_add_devices(migor_devices, ARRAY_SIZE(migor_devices));
> }
> arch_initcall(migor_devices_setup);
--
Regards,
Laurent Pinchart
Hi Kieran,
I found this e-mail already written and sitting in my outbox, so even if it's
quite outdated I decided to still send it.
On Tuesday 01 Aug 2017 15:06:20 Kieran Bingham wrote:
> On 26/06/17 19:12, Laurent Pinchart wrote:
> > The H3 ES1.x exhibits dot clock duty cycle st
v->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
> + KBUILD_MODNAME, dev_name(&pdev->dev));
> + priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> + priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + priv->subdev.entity.ops = &rcar_csi2_entity_ops;
> +
> + priv->pads[RCAR_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> + for (i = RCAR_CSI2_SOURCE_VC0; i < NR_OF_RCAR_CSI2_PAD; i++)
> + priv->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&priv->subdev.entity, NR_OF_RCAR_CSI2_PAD,
> + priv->pads);
> + if (ret)
> + goto error;
> +
> + ret = v4l2_async_register_subdev(&priv->subdev);
> + if (ret < 0)
> + goto error;
> +
> + pm_runtime_enable(&pdev->dev);
I'd enable runtime PM before registering the subdev, as the subdev API could
be called as soon as the subdev is registered.
> + dev_info(priv->dev, "%d lanes found\n", priv->lanes);
> +
> + return 0;
> +
> +error:
> + v4l2_async_notifier_unregister(&priv->notifier);
> + v4l2_async_notifier_cleanup(&priv->notifier);
> +
> + return ret;
> +}
[snip]
> +static struct platform_driver __refdata rcar_csi2_pdrv = {
> + .remove = rcar_csi2_remove,
> + .probe = rcar_csi2_probe,
> + .driver = {
> + .name = "rcar-csi2",
> + .of_match_table = of_match_ptr(rcar_csi2_of_table),
OF is required, so you could drop of_match_ptr().
> + },
> +};
> +
> +module_platform_driver(rcar_csi2_pdrv);
> +
> +MODULE_AUTHOR("Niklas Söderlund ");
> +MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
> +MODULE_LICENSE("GPL v2");
This contradicts the copyright header at the beginning of the file that states
GPL v2+ (which would be encoded in MODULE_LICENSE as just "GPL").
--
Regards,
Laurent Pinchart
i20>;
> + };
> + csi20vin7: endpoint@7 {
> + reg = <7>;
> + remote-endpoint = <&vin7csi20>;
> + };
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aa71ab52fd76d160..4737de9f41bff570 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8652,6 +8652,7 @@ L: linux-me...@vger.kernel.org
> L: linux-renesas-soc@vger.kernel.org
> T: git git://linuxtv.org/media_tree.git
> S: Supported
> +F: Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> F: Documentation/devicetree/bindings/media/rcar_vin.txt
> F: drivers/media/platform/rcar-vin/
--
Regards,
Laurent Pinchart
> + return 0;
> +}
[snip]
> +static int ceu_sensor_bound(struct v4l2_asyn
t;>> +#include
> >>
> >> Do you need this header? There would seem some that I wouldn't expect to
> >> be needed below, such as linux/init.h.
> >
> > It's probably a leftover, I'll remove it...
> >
> > [snip]
> >
> >>> +#if IS_ENABLED(CONFIG_OF)
> >>> +static const struct of_device_id ceu_of_match[] = {
> >>> + { .compatible = "renesas,renesas-ceu" },
> >>
> >> Even if you add support for new hardware, shouldn't you maintain support
> >> for renesas,sh-mobile-ceu?
> >
> > As you noticed already, the old driver did not support OF, so there
> > are no compatibility issues here
>
> Yeah, I realised that only after reviewing this patch.
>
> It'd be Super-cool if someone did the DT conversion. Perhaps Laurent? ;-)
Or you ? :-)
--
Regards,
Laurent Pinchart
2c/tw9910.h
> +++ b/include/media/i2c/tw9910.h
> @@ -18,6 +18,9 @@
>
> #include
>
> +#define TW9910_DATAWIDTH_8 BIT(0)
> +#define TW9910_DATAWIDTH_16 BIT(1)
> +
> enum tw9910_mpout_pin {
> TW9910_MPO_VLOSS,
> TW9910_MPO_HLOCK,
> @@ -32,6 +35,9 @@ enum tw9910_mpout_pin {
> struct tw9910_video_info {
> unsigned long buswidth;
> enum tw9910_mpout_pin mpout;
How about storing that as an unsigned int that takes values 8 or 16 ? It would
be more explicit (and a bit of kerneldoc for the tw9910_video_info structure
would make that even better).
> + int (*platform_enable)(void);
> + void (*platform_disable)(void);
> };
--
Regards,
Laurent Pinchart
#x27;re not patching the
Kconfig and Makefile here, that is because you will first convert the driver
away from soc-camera in the next commit.
Apart from that,
Acked-by: Laurent Pinchart
> Signed-off-by: Jacopo Mondi
> ---
> drivers/medi
the Kconfig and Makefile
here, that is because you will first convert the driver away from soc-camera
in the next commit.
Apart from that,
Acked-by: Laurent Pinchart
> Signed-off-by: Jacopo Mondi
> ---
> drivers/media/i2c/ov772x.c | 1124 +
>
ta(client));
>
> - v4l2_clk_put(priv->clk);
> + if (priv->clk)
> + clk_put(priv->clk);
Same here.
> v4l2_device_unregister_subdev(&priv->subdev);
> v4l2_ctrl_handler_free(&priv->hdl);
> return 0;
> diff --git a/include/media/i2c/ov772x.h b/include/media/i2c/ov772x.h
> index 00dbb7c..5896dff 100644
> --- a/include/media/i2c/ov772x.h
> +++ b/include/media/i2c/ov772x.h
> @@ -54,6 +54,9 @@ struct ov772x_edge_ctrl {
> struct ov772x_camera_info {
> unsigned long flags;
> struct ov772x_edge_ctrl edgectrl;
> +
> + int (*platform_enable)(void);
> + void (*platform_disable)(void);
> };
>
> #endif /* __OV772X_H__ */
--
Regards,
Laurent Pinchart
&migor_sdram_enter_start,
> @@ -651,16 +636,19 @@ static int __init migor_devices_setup(void)
>*/
> __raw_writew(__raw_readw(PORT_MSELCRA) | 1, PORT_MSELCRA);
>
> - /* Setup additional memory resource for CEU video buffers */
> - r = &migor_ceu_device.resource[2];
> - r->flags = IORESOURCE_MEM;
> - r->start = ceu_dma_membase;
> - r->end = r->start + CEU_BUFFER_MEMORY_SIZE - 1;
> - r->name = "ceu";
> -
> i2c_register_board_info(0, migor_i2c_devices,
> ARRAY_SIZE(migor_i2c_devices));
>
> + /* Initialize CEU platform device separately to map memory first */
> + device_initialize(&migor_ceu_device.dev);
> + arch_setup_pdev_archdata(&migor_ceu_device);
> + dma_declare_coherent_memory(&migor_ceu_device.dev,
> + ceu_dma_membase, ceu_dma_membase,
> + ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE -
> 1,
> + DMA_MEMORY_EXCLUSIVE);
> +
> + platform_device_add(&migor_ceu_device);
> +
> return platform_add_devices(migor_devices, ARRAY_SIZE(migor_devices));
> }
> arch_initcall(migor_devices_setup);
--
Regards,
Laurent Pinchart
Hi Jacopo,
On Monday, 11 December 2017 16:26:27 EET Laurent Pinchart wrote:
> On Wednesday, 15 November 2017 14:36:33 EET Sakari Ailus wrote:
> > On Wed, Nov 15, 2017 at 11:55:55AM +0100, Jacopo Mondi wrote:
> >> Add renesas-ceu header file.
> >>
> >> Do not
int i2c_address;
> > +};
> > +
> > +struct ceu_info {
> > + unsigned int num_subdevs;
> > + struct ceu_async_subdev subdevs[CEU_MAX_SENS];
> > +};
> > +
> > +#endif /* __ASM_RENESAS_CEU_H__ */
> > --
> > 2.7.4
--
Regards,
Laurent Pinchart
-gpios = <&port3 12 GPIO_ACTIVE_HIGH>;
> > +
> > + clocks = <&xclk>;
> > + clock-names = "xclk";
> > +
> > + xclk: fixed_clk {
> > + compatible = "fixed-clock";
> > +
qrestore(&vin->qlock, flags);
> @@ -1115,7 +1215,6 @@ static int rvin_start_streaming(struct vb2_queue *vq,
> unsigned int count) static void rvin_stop_streaming(struct vb2_queue *vq)
> {
> struct rvin_dev *vin = vb2_get_drv_priv(vq);
> - struct v4l2_subdev *sd;
> unsigned long flags;
> int retries = 0;
>
> @@ -1154,8 +1253,7 @@ static void rvin_stop_streaming(struct vb2_queue *vq)
>
> spin_unlock_irqrestore(&vin->qlock, flags);
>
> - sd = vin_to_source(vin);
> - v4l2_subdev_call(sd, video, s_stream, 0);
> + rvin_set_stream(vin, 0);
>
> /* disable interrupts */
> rvin_disable_interrupts(vin);
--
Regards,
Laurent Pinchart
ID, the CSI
receiver output channel, and the VIN ID.
Furthermore, shouldn't the CSI receiver ID be specified in DT using the
renesas,id property like we do for the VIN instances, instead of through the
endpoint number ? I know this will require a bit of refactoring, but I think
it would stimplify both the DT bindings and the code as we wouldn't hardcode a
fixed set of CSI receivers.
> };
>
> /**
--
Regards,
Laurent Pinchart
ce
> *
> + * @group: Gen3 CSI group
> * @pad: pad for media controller
> *
> * @lock:protects @queue
> @@ -134,6 +151,7 @@ struct rvin_dev {
> struct v4l2_async_notifier notifier;
> struct rvin_graph_entity *digital;
>
> + struct rvin_group *group;
> struct media_pad pad;
>
> struct mutex lock;
> @@ -162,6 +180,26 @@ struct rvin_dev {
> #define vin_warn(d, fmt, arg...) dev_warn(d->dev, fmt, ##arg)
> #define vin_err(d, fmt, arg...) dev_err(d->dev, fmt, ##arg)
>
> +/**
> + * struct rvin_group - VIN CSI2 group information
> + * @refcount:number of VIN instances using the group
> + *
> + * @mdev:media device which represents the group
> + *
> + * @lock:protects the vin and csi members
> + * @vin: VIN instances which are part of the group
> + * @csi: CSI-2 entities that are part of the group
> + */
> +struct rvin_group {
> + struct kref refcount;
> +
> + struct media_device mdev;
> +
> + struct mutex lock;
> + struct rvin_dev *vin[RCAR_VIN_NUM];
> + struct rvin_graph_entity csi[RVIN_CSI_MAX];
Given that the number and types of CSI receivers varies quite a bit between
SoCs I wonder whether this couldn't be a linked list. If csi was an array of
pointers it would be less of an issue, but an array of rvin_graph_entity can
grow large.
> +};
> +
> int rvin_dma_register(struct rvin_dev *vin, int irq);
> void rvin_dma_unregister(struct rvin_dev *vin);
--
Regards,
Laurent Pinchart
c :-) It shouldn't be a big
deal as MC-enabled applications running with an MC-enabled driver don't use
the input API anyway.
--
Regards,
Laurent Pinchart
Hi Niklas,
On Friday, 8 December 2017 16:06:58 EET Niklas Söderlund wrote:
> On 2017-12-08 11:35:18 +0200, Laurent Pinchart wrote:
> > On Friday, 8 December 2017 03:08:27 EET Niklas Söderlund wrote:
> >> There was never proper support in the VIN driver to deliver ALTERNATING
&g
Hi Niklas,
On Friday, 8 December 2017 16:14:23 EET Niklas Söderlund wrote:
> On 2017-12-08 11:04:26 +0200, Laurent Pinchart wrote:
> > On Friday, 8 December 2017 03:08:25 EET Niklas Söderlund wrote:
> >> It is possible on Gen2 to change the registers controlling composing and
Hi Niklas,
On Friday, 8 December 2017 15:09:21 EET Niklas Söderlund wrote:
> On 2017-12-08 09:54:31 +0200, Laurent Pinchart wrote:
> > On Friday, 8 December 2017 03:08:17 EET Niklas Söderlund wrote:
> >> If the video device was registered by the complete() callback it should
&g
3 },
> + },
> + },
> +};
> +
> static const struct of_device_id rvin_of_id_table[] = {
> {
> .compatible = "renesas,vin-r8a7778",
> @@ -1118,6 +1178,10 @@ static const struct of_device_id rvin_of_id_table[] =
> { .compatible = "renesas,vin-r8a7795",
> .data = &rcar_info_r8a7795,
> },
> + {
> + .compatible = "renesas,vin-r8a7796",
> + .data = &rcar_info_r8a7796,
> + },
> { },
> };
> MODULE_DEVICE_TABLE(of, rvin_of_id_table);
--
Regards,
Laurent Pinchart
rrect. Apart from that,
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/Kconfig | 2 +-
> drivers/media/platform/rcar-vin/rcar-core.c | 150 +
> 2 files changed, 151 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/
ased on the comment I made regarding the commit
message.
> * @lock:protects @queue
> * @queue: vb2 buffers queue
> *
> @@ -132,6 +134,8 @@ struct rvin_dev {
> struct v4l2_async_notifier notifier;
> struct rvin_graph_entity *digital;
>
> + struct media_pad pad;
> +
> struct mutex lock;
> struct vb2_queue queue;
--
Regards,
Laurent Pinchart
IN_DEFAULT_COLORSPACE;
> +
> + if (vin->info->use_mc) {
> + vdev->fops = &rvin_mc_fops;
> + vdev->ioctl_ops = &rvin_mc_ioctl_ops;
> + } else {
> + vdev->fops = &rvin_fops;
> + vdev->ioctl_ops = &rvin_ioctl_ops;
> + rvin_reset_format(vin);
> + }
> +
> + ret = rvin_format_align(vin, &vin->format);
> + if (ret)
> + return ret;
>
> ret = video_register_device(&vin->vdev, VFL_TYPE_GRABBER, -1);
> if (ret) {
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index
> 0747873c2b9cb74c..fd3cd781be0ab1cf 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
Can't you include this header in the .c files instead ? Minimizing the headers
included by other headers lowers (pre)compilation time.
> #include
>
> /* Number of HW buffers */
--
Regards,
Laurent Pinchart
> - &pix->height, 4, vin->info->max_height, 2, 0);
> -
> - pix->bytesperline = max_t(u32, pix->bytesperline,
> - rvin_format_bytesperline(pix));
> - pix->sizeimage = max_t(u32, pix->sizeimage,
> -rvin_format_sizeimage(pix));
> -
> - if (vin->info->chip == RCAR_M1 &&
> - pix->pixelformat == V4L2_PIX_FMT_XBGR32) {
> - vin_err(vin, "pixel format XBGR32 not supported on M1\n");
> - return -EINVAL;
> - }
> -
> - vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n",
> - pix->width, pix->height, pix->bytesperline, pix->sizeimage);
> -
> - return 0;
> + return rvin_format_align(vin, pix);
> }
>
> static int rvin_querycap(struct file *file, void *priv,
--
Regards,
Laurent Pinchart
and will continue to be the mode of
> operation on Gen2.
>
> Prepare for these two modes of operation by adding a flag to struct
> rvin_info which will control which mode to use.
>
> Signed-off-by: Niklas Söderlund
> Reviewed-by: Hans Verkuil
Reviewed-by: Laurent Pinchart
&
in.h index
> a440effe4b86af31..7819c760c2c13422 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -163,4 +163,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin);
>
> const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat);
>
> +void rvin_set_chsel(struct rvin_dev *vin, u8 chsel);
> +
> #endif
--
Regards,
Laurent Pinchart
e(vin, ALIGN(vin->format.width, 0x20), VNIS_REG);
> - else
> - rvin_write(vin, ALIGN(vin->format.width, 0x10), VNIS_REG);
> -
> vin_dbg(vin,
> "Pre-Clip: %ux%u@%u:%u YS: %d XS: %d Post-Clip: %ux%u@%u:%u\n",
> vin-&g
ntains.
> * @format: active V4L2 pixel format
> *
> * @crop:active cropping
> @@ -141,6 +138,8 @@ struct rvin_dev {
> unsigned int sequence;
> enum rvin_dma_state state;
>
> + struct v4l2_mbus_config mbus_cfg;
> + u32 code;
> struct v4l2_pix_format format;
>
> struct v4l2_rect crop;
--
Regards,
Laurent Pinchart
break;
> - }
> -
> vin->crop.top = vin->crop.left = 0;
> vin->crop.width = vin->format.width;
> vin->crop.height = vin->format.height;
> @@ -226,9 +217,6 @@ static int __rvin_try_format(struct rvin_dev *vin,
> switch (pix->field) {
> case V4L2_FIELD_TOP:
> case V4L2_FIELD_BOTTOM:
> - case V4L2_FIELD_ALTERNATE:
> - pix->height /= 2;
> - break;
> case V4L2_FIELD_NONE:
> case V4L2_FIELD_INTERLACED_TB:
> case V4L2_FIELD_INTERLACED_BT:
--
Regards,
Laurent Pinchart
-git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index
> 67541b483ee43c52..f8e0e7cedeaa6c38 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -48,16 +48,6 @@ enum rvin_dma_state {
> STOPPING,
> };
>
> -/**
> - * struct rvin_source_fmt - Source information
> - * @width: Width from source
> - * @height: Height from source
> - */
> -struct rvin_source_fmt {
> - u32 width;
> - u32 height;
> -};
> -
> /**
> * struct rvin_video_format - Data format stored in memory
> * @fourcc: Pixelformat
> @@ -125,7 +115,6 @@ struct rvin_info {
> * @sequence:V4L2 buffers sequence number
> * @state: keeps track of operation state
> *
> - * @source: active format from the video source
> * @format: active V4L2 pixel format
> *
> * @crop:active cropping
> @@ -152,7 +141,6 @@ struct rvin_dev {
> unsigned int sequence;
> enum rvin_dma_state state;
>
> - struct rvin_source_fmt source;
> struct v4l2_pix_format format;
>
> struct v4l2_rect crop;
--
Regards,
Laurent Pinchart
dex
> 36d0f0cc4ce01a6e..67541b483ee43c52 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -175,7 +175,4 @@ void rvin_v4l2_unregister(struct rvin_dev *vin);
>
> const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat);
>
> -/* Cropping, composing and scaling */
> -void rvin_crop_scale_comp(struct rvin_dev *vin);
> -
> #endif
--
Regards,
Laurent Pinchart
Hi Hans,
On Friday, 8 December 2017 10:46:34 EET Hans Verkuil wrote:
> On 12/08/2017 08:54 AM, Laurent Pinchart wrote:
> > On Friday, 8 December 2017 03:08:17 EET Niklas Söderlund wrote:
> >> If the video device was registered by the complete() callback it should
> >&g
->field;
> + width = pix->width;
> + height = pix->height;
>
> ret = v4l2_subdev_call(sd, pad, set_fmt, pad_cfg, &format);
> if (ret < 0 && ret != -ENOIOCTLCMD)
> @@ -191,6 +195,9 @@ static int __rvin_try_format_source(struct rvin_dev
> *vin, s
p a lot.
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 806 ++
> 1 file changed, 405 insertions(+), 401 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-
that V4L2 has never standardized a naming
scheme for the devices. It wouldn't be fair to ask you to fix that as a
prerequisite to get the VIN driver merged, but we clearly have to work on that
at some point.
> vdev->release = video_device_release_empty;
> vdev->ioctl_ops = &rvin_ioctl_ops;
> vdev->lock = &vin->lock;
--
Regards,
Laurent Pinchart
Nitpicking, there's no need for a blank line here.
> + * max_width:max input width the VIN supports
> + * max_height: max input height the VIN supports
And you're missing the @ before the field names. Please make sure to compile
the documentation to check for kerneldoc errors.
With this fixed,
Reviewed-by: Laurent Pinchart
> */
> struct rvin_info {
> enum chip_id chip;
> +
> + unsigned int max_width;
> + unsigned int max_height;
> };
>
> /**
--
Regards,
Laurent Pinchart
901 - 1000 of 2657 matches
Mail list logo
