* [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift
@ 2020-09-13 18:16 Lad Prabhakar
2020-09-14 23:45 ` Laurent Pinchart
2020-09-17 8:57 ` Hans Verkuil
0 siblings, 2 replies; 8+ messages in thread
From: Lad Prabhakar @ 2020-09-13 18:16 UTC (permalink / raw)
To: Niklas Söderlund, Laurent Pinchart, Geert Uytterhoeven,
Mauro Carvalho Chehab, linux-media, linux-renesas-soc
Cc: linux-kernel, Lad Prabhakar, Prabhakar
Enable YDS bit if bus_width and data_shift is set to 8 in parallel mode
for MEDIA_BUS_FMT_UYVY8_2X8 format.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
Changes for v3:
* Dropped BIT macro
* Introduced struct v4l2_fwnode_bus_parallel
Changes for v2:
* Dropped DT binding documentation patch
* Select the data pins depending on bus-width and data-shift
v1 -
https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=323799
---
drivers/media/platform/rcar-vin/rcar-core.c | 9 ++++-----
drivers/media/platform/rcar-vin/rcar-dma.c | 17 ++++++++++++++---
drivers/media/platform/rcar-vin/rcar-vin.h | 5 +++--
3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 7440c8965d27..1149ab76cf5c 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -626,12 +626,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
switch (vin->parallel->mbus_type) {
case V4L2_MBUS_PARALLEL:
- vin_dbg(vin, "Found PARALLEL media bus\n");
- vin->parallel->mbus_flags = vep->bus.parallel.flags;
- break;
case V4L2_MBUS_BT656:
- vin_dbg(vin, "Found BT656 media bus\n");
- vin->parallel->mbus_flags = 0;
+ vin_dbg(vin, "Found %s media bus\n",
+ vin->parallel->mbus_type == V4L2_MBUS_PARALLEL ?
+ "PARALLEL" : "BT656");
+ vin->parallel->bus = vep->bus.parallel;
break;
default:
vin_err(vin, "Unknown media bus type\n");
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index a5dbb90c5210..d067439b0b0d 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -125,6 +125,7 @@
#define VNDMR2_VPS (1 << 30)
#define VNDMR2_HPS (1 << 29)
#define VNDMR2_CES (1 << 28)
+#define VNDMR2_YDS (1 << 22)
#define VNDMR2_FTEV (1 << 17)
#define VNDMR2_VLV(n) ((n & 0xf) << 12)
@@ -698,16 +699,26 @@ static int rvin_setup(struct rvin_dev *vin)
if (!vin->is_csi) {
/* Hsync Signal Polarity Select */
- if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
+ if (!(vin->parallel->bus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_HPS;
/* Vsync Signal Polarity Select */
- if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
+ if (!(vin->parallel->bus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_VPS;
/* Data Enable Polarity Select */
- if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
+ if (vin->parallel->bus.flags & V4L2_MBUS_DATA_ENABLE_LOW)
dmr2 |= VNDMR2_CES;
+
+ switch (vin->mbus_code) {
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ if (vin->parallel->bus.bus_width == 8 &&
+ vin->parallel->bus.data_shift == 8)
+ dmr2 |= VNDMR2_YDS;
+ break;
+ default:
+ break;
+ }
}
/*
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index c19d077ce1cb..8396e0e45478 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -19,6 +19,7 @@
#include <media/v4l2-ctrls.h>
#include <media/v4l2-dev.h>
#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
#include <media/videobuf2-v4l2.h>
/* Number of HW buffers */
@@ -92,7 +93,7 @@ struct rvin_video_format {
* @asd: sub-device descriptor for async framework
* @subdev: subdevice matched using async framework
* @mbus_type: media bus type
- * @mbus_flags: media bus configuration flags
+ * @bus: media bus parallel configuration
* @source_pad: source pad of remote subdevice
* @sink_pad: sink pad of remote subdevice
*
@@ -102,7 +103,7 @@ struct rvin_parallel_entity {
struct v4l2_subdev *subdev;
enum v4l2_mbus_type mbus_type;
- unsigned int mbus_flags;
+ struct v4l2_fwnode_bus_parallel bus;
unsigned int source_pad;
unsigned int sink_pad;
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift
2020-09-13 18:16 [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift Lad Prabhakar
@ 2020-09-14 23:45 ` Laurent Pinchart
2020-09-15 0:01 ` Niklas Söderlund
2020-09-17 8:57 ` Hans Verkuil
1 sibling, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2020-09-14 23:45 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Niklas Söderlund, Geert Uytterhoeven, Mauro Carvalho Chehab,
linux-media, linux-renesas-soc, linux-kernel, Prabhakar
Hi Prabhakar,
Thank you for the patch.
On Sun, Sep 13, 2020 at 07:16:08PM +0100, Lad Prabhakar wrote:
> Enable YDS bit if bus_width and data_shift is set to 8 in parallel mode
> for MEDIA_BUS_FMT_UYVY8_2X8 format.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> Changes for v3:
> * Dropped BIT macro
> * Introduced struct v4l2_fwnode_bus_parallel
>
> Changes for v2:
> * Dropped DT binding documentation patch
> * Select the data pins depending on bus-width and data-shift
>
> v1 -
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=323799
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 9 ++++-----
> drivers/media/platform/rcar-vin/rcar-dma.c | 17 ++++++++++++++---
> drivers/media/platform/rcar-vin/rcar-vin.h | 5 +++--
> 3 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 7440c8965d27..1149ab76cf5c 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -626,12 +626,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
>
> switch (vin->parallel->mbus_type) {
> case V4L2_MBUS_PARALLEL:
> - vin_dbg(vin, "Found PARALLEL media bus\n");
> - vin->parallel->mbus_flags = vep->bus.parallel.flags;
> - break;
> case V4L2_MBUS_BT656:
> - vin_dbg(vin, "Found BT656 media bus\n");
> - vin->parallel->mbus_flags = 0;
> + vin_dbg(vin, "Found %s media bus\n",
> + vin->parallel->mbus_type == V4L2_MBUS_PARALLEL ?
> + "PARALLEL" : "BT656");
I'd write "parallel" and "BT.656".
> + vin->parallel->bus = vep->bus.parallel;
> break;
> default:
> vin_err(vin, "Unknown media bus type\n");
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index a5dbb90c5210..d067439b0b0d 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -125,6 +125,7 @@
> #define VNDMR2_VPS (1 << 30)
> #define VNDMR2_HPS (1 << 29)
> #define VNDMR2_CES (1 << 28)
> +#define VNDMR2_YDS (1 << 22)
> #define VNDMR2_FTEV (1 << 17)
> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
>
> @@ -698,16 +699,26 @@ static int rvin_setup(struct rvin_dev *vin)
>
> if (!vin->is_csi) {
> /* Hsync Signal Polarity Select */
> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> + if (!(vin->parallel->bus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> dmr2 |= VNDMR2_HPS;
>
> /* Vsync Signal Polarity Select */
> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> + if (!(vin->parallel->bus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> dmr2 |= VNDMR2_VPS;
>
> /* Data Enable Polarity Select */
> - if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
> + if (vin->parallel->bus.flags & V4L2_MBUS_DATA_ENABLE_LOW)
> dmr2 |= VNDMR2_CES;
> +
> + switch (vin->mbus_code) {
> + case MEDIA_BUS_FMT_UYVY8_2X8:
> + if (vin->parallel->bus.bus_width == 8 &&
You can possibly drop this check, as UYVY8_2X8 implies a bus_width equal
to 8. Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + vin->parallel->bus.data_shift == 8)
> + dmr2 |= VNDMR2_YDS;
> + break;
> + default:
> + break;
> + }
> }
>
> /*
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index c19d077ce1cb..8396e0e45478 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -19,6 +19,7 @@
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-dev.h>
> #include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> #include <media/videobuf2-v4l2.h>
>
> /* Number of HW buffers */
> @@ -92,7 +93,7 @@ struct rvin_video_format {
> * @asd: sub-device descriptor for async framework
> * @subdev: subdevice matched using async framework
> * @mbus_type: media bus type
> - * @mbus_flags: media bus configuration flags
> + * @bus: media bus parallel configuration
> * @source_pad: source pad of remote subdevice
> * @sink_pad: sink pad of remote subdevice
> *
> @@ -102,7 +103,7 @@ struct rvin_parallel_entity {
> struct v4l2_subdev *subdev;
>
> enum v4l2_mbus_type mbus_type;
> - unsigned int mbus_flags;
> + struct v4l2_fwnode_bus_parallel bus;
>
> unsigned int source_pad;
> unsigned int sink_pad;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift
2020-09-14 23:45 ` Laurent Pinchart
@ 2020-09-15 0:01 ` Niklas Söderlund
2020-09-15 0:02 ` Laurent Pinchart
0 siblings, 1 reply; 8+ messages in thread
From: Niklas Söderlund @ 2020-09-15 0:01 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Lad Prabhakar, Geert Uytterhoeven, Mauro Carvalho Chehab,
linux-media, linux-renesas-soc, linux-kernel, Prabhakar
Hi Lad,
Thanks for your work.
On 2020-09-15 02:45:32 +0300, Laurent Pinchart wrote:
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Sun, Sep 13, 2020 at 07:16:08PM +0100, Lad Prabhakar wrote:
> > Enable YDS bit if bus_width and data_shift is set to 8 in parallel mode
> > for MEDIA_BUS_FMT_UYVY8_2X8 format.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > Changes for v3:
> > * Dropped BIT macro
> > * Introduced struct v4l2_fwnode_bus_parallel
> >
> > Changes for v2:
> > * Dropped DT binding documentation patch
> > * Select the data pins depending on bus-width and data-shift
> >
> > v1 -
> > https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=323799
> > ---
> > drivers/media/platform/rcar-vin/rcar-core.c | 9 ++++-----
> > drivers/media/platform/rcar-vin/rcar-dma.c | 17 ++++++++++++++---
> > drivers/media/platform/rcar-vin/rcar-vin.h | 5 +++--
> > 3 files changed, 21 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> > index 7440c8965d27..1149ab76cf5c 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -626,12 +626,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
> >
> > switch (vin->parallel->mbus_type) {
> > case V4L2_MBUS_PARALLEL:
> > - vin_dbg(vin, "Found PARALLEL media bus\n");
> > - vin->parallel->mbus_flags = vep->bus.parallel.flags;
> > - break;
> > case V4L2_MBUS_BT656:
> > - vin_dbg(vin, "Found BT656 media bus\n");
> > - vin->parallel->mbus_flags = 0;
> > + vin_dbg(vin, "Found %s media bus\n",
> > + vin->parallel->mbus_type == V4L2_MBUS_PARALLEL ?
> > + "PARALLEL" : "BT656");
>
> I'd write "parallel" and "BT.656".
I agree with this change.
>
> > + vin->parallel->bus = vep->bus.parallel;
> > break;
> > default:
> > vin_err(vin, "Unknown media bus type\n");
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index a5dbb90c5210..d067439b0b0d 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -125,6 +125,7 @@
> > #define VNDMR2_VPS (1 << 30)
> > #define VNDMR2_HPS (1 << 29)
> > #define VNDMR2_CES (1 << 28)
> > +#define VNDMR2_YDS (1 << 22)
> > #define VNDMR2_FTEV (1 << 17)
> > #define VNDMR2_VLV(n) ((n & 0xf) << 12)
> >
> > @@ -698,16 +699,26 @@ static int rvin_setup(struct rvin_dev *vin)
> >
> > if (!vin->is_csi) {
> > /* Hsync Signal Polarity Select */
> > - if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> > + if (!(vin->parallel->bus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> > dmr2 |= VNDMR2_HPS;
> >
> > /* Vsync Signal Polarity Select */
> > - if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> > + if (!(vin->parallel->bus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> > dmr2 |= VNDMR2_VPS;
> >
> > /* Data Enable Polarity Select */
> > - if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
> > + if (vin->parallel->bus.flags & V4L2_MBUS_DATA_ENABLE_LOW)
> > dmr2 |= VNDMR2_CES;
> > +
> > + switch (vin->mbus_code) {
> > + case MEDIA_BUS_FMT_UYVY8_2X8:
> > + if (vin->parallel->bus.bus_width == 8 &&
>
> You can possibly drop this check, as UYVY8_2X8 implies a bus_width equal
> to 8. Apart from that,
I agree here as well, I think the check for UYVY8_2X8 may be dropped.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > + vin->parallel->bus.data_shift == 8)
> > + dmr2 |= VNDMR2_YDS;
> > + break;
> > + default:
> > + break;
> > + }
> > }
> >
> > /*
> > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> > index c19d077ce1cb..8396e0e45478 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> > @@ -19,6 +19,7 @@
> > #include <media/v4l2-ctrls.h>
> > #include <media/v4l2-dev.h>
> > #include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > #include <media/videobuf2-v4l2.h>
> >
> > /* Number of HW buffers */
> > @@ -92,7 +93,7 @@ struct rvin_video_format {
> > * @asd: sub-device descriptor for async framework
> > * @subdev: subdevice matched using async framework
> > * @mbus_type: media bus type
> > - * @mbus_flags: media bus configuration flags
> > + * @bus: media bus parallel configuration
> > * @source_pad: source pad of remote subdevice
> > * @sink_pad: sink pad of remote subdevice
> > *
> > @@ -102,7 +103,7 @@ struct rvin_parallel_entity {
> > struct v4l2_subdev *subdev;
> >
> > enum v4l2_mbus_type mbus_type;
> > - unsigned int mbus_flags;
> > + struct v4l2_fwnode_bus_parallel bus;
I think you could break this change (and the fallout) out to a separate
patch to make the functional change clearer.
> >
> > unsigned int source_pad;
> > unsigned int sink_pad;
>
> --
> Regards,
>
> Laurent Pinchart
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift
2020-09-15 0:01 ` Niklas Söderlund
@ 2020-09-15 0:02 ` Laurent Pinchart
0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2020-09-15 0:02 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Lad Prabhakar, Geert Uytterhoeven, Mauro Carvalho Chehab,
linux-media, linux-renesas-soc, linux-kernel, Prabhakar
Hi Niklas,
On Tue, Sep 15, 2020 at 02:01:40AM +0200, Niklas Söderlund wrote:
> On 2020-09-15 02:45:32 +0300, Laurent Pinchart wrote:
> > On Sun, Sep 13, 2020 at 07:16:08PM +0100, Lad Prabhakar wrote:
> > > Enable YDS bit if bus_width and data_shift is set to 8 in parallel mode
> > > for MEDIA_BUS_FMT_UYVY8_2X8 format.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > > Changes for v3:
> > > * Dropped BIT macro
> > > * Introduced struct v4l2_fwnode_bus_parallel
> > >
> > > Changes for v2:
> > > * Dropped DT binding documentation patch
> > > * Select the data pins depending on bus-width and data-shift
> > >
> > > v1 -
> > > https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=323799
> > > ---
> > > drivers/media/platform/rcar-vin/rcar-core.c | 9 ++++-----
> > > drivers/media/platform/rcar-vin/rcar-dma.c | 17 ++++++++++++++---
> > > drivers/media/platform/rcar-vin/rcar-vin.h | 5 +++--
> > > 3 files changed, 21 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> > > index 7440c8965d27..1149ab76cf5c 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > > @@ -626,12 +626,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
> > >
> > > switch (vin->parallel->mbus_type) {
> > > case V4L2_MBUS_PARALLEL:
> > > - vin_dbg(vin, "Found PARALLEL media bus\n");
> > > - vin->parallel->mbus_flags = vep->bus.parallel.flags;
> > > - break;
> > > case V4L2_MBUS_BT656:
> > > - vin_dbg(vin, "Found BT656 media bus\n");
> > > - vin->parallel->mbus_flags = 0;
> > > + vin_dbg(vin, "Found %s media bus\n",
> > > + vin->parallel->mbus_type == V4L2_MBUS_PARALLEL ?
> > > + "PARALLEL" : "BT656");
> >
> > I'd write "parallel" and "BT.656".
>
> I agree with this change.
>
> > > + vin->parallel->bus = vep->bus.parallel;
> > > break;
> > > default:
> > > vin_err(vin, "Unknown media bus type\n");
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > > index a5dbb90c5210..d067439b0b0d 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > > @@ -125,6 +125,7 @@
> > > #define VNDMR2_VPS (1 << 30)
> > > #define VNDMR2_HPS (1 << 29)
> > > #define VNDMR2_CES (1 << 28)
> > > +#define VNDMR2_YDS (1 << 22)
> > > #define VNDMR2_FTEV (1 << 17)
> > > #define VNDMR2_VLV(n) ((n & 0xf) << 12)
> > >
> > > @@ -698,16 +699,26 @@ static int rvin_setup(struct rvin_dev *vin)
> > >
> > > if (!vin->is_csi) {
> > > /* Hsync Signal Polarity Select */
> > > - if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> > > + if (!(vin->parallel->bus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> > > dmr2 |= VNDMR2_HPS;
> > >
> > > /* Vsync Signal Polarity Select */
> > > - if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> > > + if (!(vin->parallel->bus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> > > dmr2 |= VNDMR2_VPS;
> > >
> > > /* Data Enable Polarity Select */
> > > - if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
> > > + if (vin->parallel->bus.flags & V4L2_MBUS_DATA_ENABLE_LOW)
> > > dmr2 |= VNDMR2_CES;
> > > +
> > > + switch (vin->mbus_code) {
> > > + case MEDIA_BUS_FMT_UYVY8_2X8:
> > > + if (vin->parallel->bus.bus_width == 8 &&
> >
> > You can possibly drop this check, as UYVY8_2X8 implies a bus_width equal
> > to 8. Apart from that,
>
> I agree here as well, I think the check for UYVY8_2X8 may be dropped.
>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > > + vin->parallel->bus.data_shift == 8)
> > > + dmr2 |= VNDMR2_YDS;
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > }
> > >
> > > /*
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> > > index c19d077ce1cb..8396e0e45478 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> > > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> > > @@ -19,6 +19,7 @@
> > > #include <media/v4l2-ctrls.h>
> > > #include <media/v4l2-dev.h>
> > > #include <media/v4l2-device.h>
> > > +#include <media/v4l2-fwnode.h>
> > > #include <media/videobuf2-v4l2.h>
> > >
> > > /* Number of HW buffers */
> > > @@ -92,7 +93,7 @@ struct rvin_video_format {
> > > * @asd: sub-device descriptor for async framework
> > > * @subdev: subdevice matched using async framework
> > > * @mbus_type: media bus type
> > > - * @mbus_flags: media bus configuration flags
> > > + * @bus: media bus parallel configuration
> > > * @source_pad: source pad of remote subdevice
> > > * @sink_pad: sink pad of remote subdevice
> > > *
> > > @@ -102,7 +103,7 @@ struct rvin_parallel_entity {
> > > struct v4l2_subdev *subdev;
> > >
> > > enum v4l2_mbus_type mbus_type;
> > > - unsigned int mbus_flags;
> > > + struct v4l2_fwnode_bus_parallel bus;
>
> I think you could break this change (and the fallout) out to a separate
> patch to make the functional change clearer.
You're often on the side of breaking patches in too small pieces :-) In
this case the patch is small, so I don't think breaking it in two is
required. It would be different if the changes were larger and more
intertwined.
> > >
> > > unsigned int source_pad;
> > > unsigned int sink_pad;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift
2020-09-13 18:16 [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift Lad Prabhakar
2020-09-14 23:45 ` Laurent Pinchart
@ 2020-09-17 8:57 ` Hans Verkuil
2020-09-17 9:58 ` Hans Verkuil
1 sibling, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2020-09-17 8:57 UTC (permalink / raw)
To: Lad Prabhakar, Niklas Söderlund, Laurent Pinchart,
Geert Uytterhoeven, Mauro Carvalho Chehab, linux-media,
linux-renesas-soc
Cc: linux-kernel, Prabhakar
Hi Prabhakar,
Can you rebase this patch? It no longer applies.
Regards,
Hans
On 13/09/2020 20:16, Lad Prabhakar wrote:
> Enable YDS bit if bus_width and data_shift is set to 8 in parallel mode
> for MEDIA_BUS_FMT_UYVY8_2X8 format.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> Changes for v3:
> * Dropped BIT macro
> * Introduced struct v4l2_fwnode_bus_parallel
>
> Changes for v2:
> * Dropped DT binding documentation patch
> * Select the data pins depending on bus-width and data-shift
>
> v1 -
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=323799
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 9 ++++-----
> drivers/media/platform/rcar-vin/rcar-dma.c | 17 ++++++++++++++---
> drivers/media/platform/rcar-vin/rcar-vin.h | 5 +++--
> 3 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 7440c8965d27..1149ab76cf5c 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -626,12 +626,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
>
> switch (vin->parallel->mbus_type) {
> case V4L2_MBUS_PARALLEL:
> - vin_dbg(vin, "Found PARALLEL media bus\n");
> - vin->parallel->mbus_flags = vep->bus.parallel.flags;
> - break;
> case V4L2_MBUS_BT656:
> - vin_dbg(vin, "Found BT656 media bus\n");
> - vin->parallel->mbus_flags = 0;
> + vin_dbg(vin, "Found %s media bus\n",
> + vin->parallel->mbus_type == V4L2_MBUS_PARALLEL ?
> + "PARALLEL" : "BT656");
> + vin->parallel->bus = vep->bus.parallel;
> break;
> default:
> vin_err(vin, "Unknown media bus type\n");
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index a5dbb90c5210..d067439b0b0d 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -125,6 +125,7 @@
> #define VNDMR2_VPS (1 << 30)
> #define VNDMR2_HPS (1 << 29)
> #define VNDMR2_CES (1 << 28)
> +#define VNDMR2_YDS (1 << 22)
> #define VNDMR2_FTEV (1 << 17)
> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
>
> @@ -698,16 +699,26 @@ static int rvin_setup(struct rvin_dev *vin)
>
> if (!vin->is_csi) {
> /* Hsync Signal Polarity Select */
> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> + if (!(vin->parallel->bus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> dmr2 |= VNDMR2_HPS;
>
> /* Vsync Signal Polarity Select */
> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> + if (!(vin->parallel->bus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> dmr2 |= VNDMR2_VPS;
>
> /* Data Enable Polarity Select */
> - if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
> + if (vin->parallel->bus.flags & V4L2_MBUS_DATA_ENABLE_LOW)
> dmr2 |= VNDMR2_CES;
> +
> + switch (vin->mbus_code) {
> + case MEDIA_BUS_FMT_UYVY8_2X8:
> + if (vin->parallel->bus.bus_width == 8 &&
> + vin->parallel->bus.data_shift == 8)
> + dmr2 |= VNDMR2_YDS;
> + break;
> + default:
> + break;
> + }
> }
>
> /*
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index c19d077ce1cb..8396e0e45478 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -19,6 +19,7 @@
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-dev.h>
> #include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> #include <media/videobuf2-v4l2.h>
>
> /* Number of HW buffers */
> @@ -92,7 +93,7 @@ struct rvin_video_format {
> * @asd: sub-device descriptor for async framework
> * @subdev: subdevice matched using async framework
> * @mbus_type: media bus type
> - * @mbus_flags: media bus configuration flags
> + * @bus: media bus parallel configuration
> * @source_pad: source pad of remote subdevice
> * @sink_pad: sink pad of remote subdevice
> *
> @@ -102,7 +103,7 @@ struct rvin_parallel_entity {
> struct v4l2_subdev *subdev;
>
> enum v4l2_mbus_type mbus_type;
> - unsigned int mbus_flags;
> + struct v4l2_fwnode_bus_parallel bus;
>
> unsigned int source_pad;
> unsigned int sink_pad;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift
2020-09-17 8:57 ` Hans Verkuil
@ 2020-09-17 9:58 ` Hans Verkuil
2020-09-17 10:19 ` Niklas Söderlund
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2020-09-17 9:58 UTC (permalink / raw)
To: Lad Prabhakar, Niklas Söderlund, Laurent Pinchart,
Geert Uytterhoeven, Mauro Carvalho Chehab, linux-media,
linux-renesas-soc
Cc: linux-kernel, Prabhakar
On 17/09/2020 10:57, Hans Verkuil wrote:
> Hi Prabhakar,
>
> Can you rebase this patch? It no longer applies.
Never mind, my mistake. When I was preparing this patch I also had this patch
applied: https://patchwork.linuxtv.org/project/linux-media/patch/1595602732-25582-3-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com/
And that caused the conflict with this YDS patch.
However, I backed out the two renesas-vin-ycbcr-8b-g patches since there were a
bunch comments for https://patchwork.linuxtv.org/project/linux-media/patch/1595602732-25582-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com/
So after dropping those two patches this YDS patch now applies fine and will
be included in the PR.
Sorry for the confusion,
Hans
>
> Regards,
>
> Hans
>
> On 13/09/2020 20:16, Lad Prabhakar wrote:
>> Enable YDS bit if bus_width and data_shift is set to 8 in parallel mode
>> for MEDIA_BUS_FMT_UYVY8_2X8 format.
>>
>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
>> ---
>> Changes for v3:
>> * Dropped BIT macro
>> * Introduced struct v4l2_fwnode_bus_parallel
>>
>> Changes for v2:
>> * Dropped DT binding documentation patch
>> * Select the data pins depending on bus-width and data-shift
>>
>> v1 -
>> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=323799
>> ---
>> drivers/media/platform/rcar-vin/rcar-core.c | 9 ++++-----
>> drivers/media/platform/rcar-vin/rcar-dma.c | 17 ++++++++++++++---
>> drivers/media/platform/rcar-vin/rcar-vin.h | 5 +++--
>> 3 files changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
>> index 7440c8965d27..1149ab76cf5c 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-core.c
>> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
>> @@ -626,12 +626,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
>>
>> switch (vin->parallel->mbus_type) {
>> case V4L2_MBUS_PARALLEL:
>> - vin_dbg(vin, "Found PARALLEL media bus\n");
>> - vin->parallel->mbus_flags = vep->bus.parallel.flags;
>> - break;
>> case V4L2_MBUS_BT656:
>> - vin_dbg(vin, "Found BT656 media bus\n");
>> - vin->parallel->mbus_flags = 0;
>> + vin_dbg(vin, "Found %s media bus\n",
>> + vin->parallel->mbus_type == V4L2_MBUS_PARALLEL ?
>> + "PARALLEL" : "BT656");
>> + vin->parallel->bus = vep->bus.parallel;
>> break;
>> default:
>> vin_err(vin, "Unknown media bus type\n");
>> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
>> index a5dbb90c5210..d067439b0b0d 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
>> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
>> @@ -125,6 +125,7 @@
>> #define VNDMR2_VPS (1 << 30)
>> #define VNDMR2_HPS (1 << 29)
>> #define VNDMR2_CES (1 << 28)
>> +#define VNDMR2_YDS (1 << 22)
>> #define VNDMR2_FTEV (1 << 17)
>> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
>>
>> @@ -698,16 +699,26 @@ static int rvin_setup(struct rvin_dev *vin)
>>
>> if (!vin->is_csi) {
>> /* Hsync Signal Polarity Select */
>> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
>> + if (!(vin->parallel->bus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
>> dmr2 |= VNDMR2_HPS;
>>
>> /* Vsync Signal Polarity Select */
>> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
>> + if (!(vin->parallel->bus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
>> dmr2 |= VNDMR2_VPS;
>>
>> /* Data Enable Polarity Select */
>> - if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
>> + if (vin->parallel->bus.flags & V4L2_MBUS_DATA_ENABLE_LOW)
>> dmr2 |= VNDMR2_CES;
>> +
>> + switch (vin->mbus_code) {
>> + case MEDIA_BUS_FMT_UYVY8_2X8:
>> + if (vin->parallel->bus.bus_width == 8 &&
>> + vin->parallel->bus.data_shift == 8)
>> + dmr2 |= VNDMR2_YDS;
>> + break;
>> + default:
>> + break;
>> + }
>> }
>>
>> /*
>> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
>> index c19d077ce1cb..8396e0e45478 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
>> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
>> @@ -19,6 +19,7 @@
>> #include <media/v4l2-ctrls.h>
>> #include <media/v4l2-dev.h>
>> #include <media/v4l2-device.h>
>> +#include <media/v4l2-fwnode.h>
>> #include <media/videobuf2-v4l2.h>
>>
>> /* Number of HW buffers */
>> @@ -92,7 +93,7 @@ struct rvin_video_format {
>> * @asd: sub-device descriptor for async framework
>> * @subdev: subdevice matched using async framework
>> * @mbus_type: media bus type
>> - * @mbus_flags: media bus configuration flags
>> + * @bus: media bus parallel configuration
>> * @source_pad: source pad of remote subdevice
>> * @sink_pad: sink pad of remote subdevice
>> *
>> @@ -102,7 +103,7 @@ struct rvin_parallel_entity {
>> struct v4l2_subdev *subdev;
>>
>> enum v4l2_mbus_type mbus_type;
>> - unsigned int mbus_flags;
>> + struct v4l2_fwnode_bus_parallel bus;
>>
>> unsigned int source_pad;
>> unsigned int sink_pad;
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift
2020-09-17 9:58 ` Hans Verkuil
@ 2020-09-17 10:19 ` Niklas Söderlund
2020-09-17 10:42 ` Hans Verkuil
0 siblings, 1 reply; 8+ messages in thread
From: Niklas Söderlund @ 2020-09-17 10:19 UTC (permalink / raw)
To: Hans Verkuil
Cc: Lad Prabhakar, Laurent Pinchart, Geert Uytterhoeven,
Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
linux-kernel, Prabhakar
Hi Hans,
On 2020-09-17 11:58:16 +0200, Hans Verkuil wrote:
> On 17/09/2020 10:57, Hans Verkuil wrote:
> > Hi Prabhakar,
> >
> > Can you rebase this patch? It no longer applies.
>
> Never mind, my mistake. When I was preparing this patch I also had this patch
> applied: https://patchwork.linuxtv.org/project/linux-media/patch/1595602732-25582-3-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com/
>
> And that caused the conflict with this YDS patch.
>
> However, I backed out the two renesas-vin-ycbcr-8b-g patches since there were a
> bunch comments for https://patchwork.linuxtv.org/project/linux-media/patch/1595602732-25582-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com/
>
> So after dropping those two patches this YDS patch now applies fine and will
> be included in the PR.
Thanks for dropping the renesas-vin-ycbcr-8b-g patches from the PR, they
really should not have been picked up as this patch address the issue in
a nicer way. To ease your workload would you like me to collect VIN
patches and send PR to you for that driver?
>
> Sorry for the confusion,
>
> Hans
>
> >
> > Regards,
> >
> > Hans
> >
> > On 13/09/2020 20:16, Lad Prabhakar wrote:
> >> Enable YDS bit if bus_width and data_shift is set to 8 in parallel mode
> >> for MEDIA_BUS_FMT_UYVY8_2X8 format.
> >>
> >> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> >> ---
> >> Changes for v3:
> >> * Dropped BIT macro
> >> * Introduced struct v4l2_fwnode_bus_parallel
> >>
> >> Changes for v2:
> >> * Dropped DT binding documentation patch
> >> * Select the data pins depending on bus-width and data-shift
> >>
> >> v1 -
> >> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=323799
> >> ---
> >> drivers/media/platform/rcar-vin/rcar-core.c | 9 ++++-----
> >> drivers/media/platform/rcar-vin/rcar-dma.c | 17 ++++++++++++++---
> >> drivers/media/platform/rcar-vin/rcar-vin.h | 5 +++--
> >> 3 files changed, 21 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> >> index 7440c8965d27..1149ab76cf5c 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> >> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> >> @@ -626,12 +626,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
> >>
> >> switch (vin->parallel->mbus_type) {
> >> case V4L2_MBUS_PARALLEL:
> >> - vin_dbg(vin, "Found PARALLEL media bus\n");
> >> - vin->parallel->mbus_flags = vep->bus.parallel.flags;
> >> - break;
> >> case V4L2_MBUS_BT656:
> >> - vin_dbg(vin, "Found BT656 media bus\n");
> >> - vin->parallel->mbus_flags = 0;
> >> + vin_dbg(vin, "Found %s media bus\n",
> >> + vin->parallel->mbus_type == V4L2_MBUS_PARALLEL ?
> >> + "PARALLEL" : "BT656");
> >> + vin->parallel->bus = vep->bus.parallel;
> >> break;
> >> default:
> >> vin_err(vin, "Unknown media bus type\n");
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> >> index a5dbb90c5210..d067439b0b0d 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> >> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> >> @@ -125,6 +125,7 @@
> >> #define VNDMR2_VPS (1 << 30)
> >> #define VNDMR2_HPS (1 << 29)
> >> #define VNDMR2_CES (1 << 28)
> >> +#define VNDMR2_YDS (1 << 22)
> >> #define VNDMR2_FTEV (1 << 17)
> >> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
> >>
> >> @@ -698,16 +699,26 @@ static int rvin_setup(struct rvin_dev *vin)
> >>
> >> if (!vin->is_csi) {
> >> /* Hsync Signal Polarity Select */
> >> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> >> + if (!(vin->parallel->bus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> >> dmr2 |= VNDMR2_HPS;
> >>
> >> /* Vsync Signal Polarity Select */
> >> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> >> + if (!(vin->parallel->bus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> >> dmr2 |= VNDMR2_VPS;
> >>
> >> /* Data Enable Polarity Select */
> >> - if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
> >> + if (vin->parallel->bus.flags & V4L2_MBUS_DATA_ENABLE_LOW)
> >> dmr2 |= VNDMR2_CES;
> >> +
> >> + switch (vin->mbus_code) {
> >> + case MEDIA_BUS_FMT_UYVY8_2X8:
> >> + if (vin->parallel->bus.bus_width == 8 &&
> >> + vin->parallel->bus.data_shift == 8)
> >> + dmr2 |= VNDMR2_YDS;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> }
> >>
> >> /*
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> >> index c19d077ce1cb..8396e0e45478 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> >> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> >> @@ -19,6 +19,7 @@
> >> #include <media/v4l2-ctrls.h>
> >> #include <media/v4l2-dev.h>
> >> #include <media/v4l2-device.h>
> >> +#include <media/v4l2-fwnode.h>
> >> #include <media/videobuf2-v4l2.h>
> >>
> >> /* Number of HW buffers */
> >> @@ -92,7 +93,7 @@ struct rvin_video_format {
> >> * @asd: sub-device descriptor for async framework
> >> * @subdev: subdevice matched using async framework
> >> * @mbus_type: media bus type
> >> - * @mbus_flags: media bus configuration flags
> >> + * @bus: media bus parallel configuration
> >> * @source_pad: source pad of remote subdevice
> >> * @sink_pad: sink pad of remote subdevice
> >> *
> >> @@ -102,7 +103,7 @@ struct rvin_parallel_entity {
> >> struct v4l2_subdev *subdev;
> >>
> >> enum v4l2_mbus_type mbus_type;
> >> - unsigned int mbus_flags;
> >> + struct v4l2_fwnode_bus_parallel bus;
> >>
> >> unsigned int source_pad;
> >> unsigned int sink_pad;
> >>
> >
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift
2020-09-17 10:19 ` Niklas Söderlund
@ 2020-09-17 10:42 ` Hans Verkuil
0 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2020-09-17 10:42 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Lad Prabhakar, Laurent Pinchart, Geert Uytterhoeven,
Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
linux-kernel, Prabhakar
On 17/09/2020 12:19, Niklas Söderlund wrote:
> Hi Hans,
>
> On 2020-09-17 11:58:16 +0200, Hans Verkuil wrote:
>> On 17/09/2020 10:57, Hans Verkuil wrote:
>>> Hi Prabhakar,
>>>
>>> Can you rebase this patch? It no longer applies.
>>
>> Never mind, my mistake. When I was preparing this patch I also had this patch
>> applied: https://patchwork.linuxtv.org/project/linux-media/patch/1595602732-25582-3-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com/
>>
>> And that caused the conflict with this YDS patch.
>>
>> However, I backed out the two renesas-vin-ycbcr-8b-g patches since there were a
>> bunch comments for https://patchwork.linuxtv.org/project/linux-media/patch/1595602732-25582-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com/
>>
>> So after dropping those two patches this YDS patch now applies fine and will
>> be included in the PR.
>
> Thanks for dropping the renesas-vin-ycbcr-8b-g patches from the PR, they
> really should not have been picked up as this patch address the issue in
> a nicer way. To ease your workload would you like me to collect VIN
> patches and send PR to you for that driver?
I don't think that's necessary. It's not all that much.
But please don't hesitate to ping/mail me if it takes too long for me to pick up
rcar patches, in case I missed some.
Regards,
Hans
>
>>
>> Sorry for the confusion,
>>
>> Hans
>>
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>> On 13/09/2020 20:16, Lad Prabhakar wrote:
>>>> Enable YDS bit if bus_width and data_shift is set to 8 in parallel mode
>>>> for MEDIA_BUS_FMT_UYVY8_2X8 format.
>>>>
>>>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>> ---
>>>> Changes for v3:
>>>> * Dropped BIT macro
>>>> * Introduced struct v4l2_fwnode_bus_parallel
>>>>
>>>> Changes for v2:
>>>> * Dropped DT binding documentation patch
>>>> * Select the data pins depending on bus-width and data-shift
>>>>
>>>> v1 -
>>>> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=323799
>>>> ---
>>>> drivers/media/platform/rcar-vin/rcar-core.c | 9 ++++-----
>>>> drivers/media/platform/rcar-vin/rcar-dma.c | 17 ++++++++++++++---
>>>> drivers/media/platform/rcar-vin/rcar-vin.h | 5 +++--
>>>> 3 files changed, 21 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
>>>> index 7440c8965d27..1149ab76cf5c 100644
>>>> --- a/drivers/media/platform/rcar-vin/rcar-core.c
>>>> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
>>>> @@ -626,12 +626,11 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
>>>>
>>>> switch (vin->parallel->mbus_type) {
>>>> case V4L2_MBUS_PARALLEL:
>>>> - vin_dbg(vin, "Found PARALLEL media bus\n");
>>>> - vin->parallel->mbus_flags = vep->bus.parallel.flags;
>>>> - break;
>>>> case V4L2_MBUS_BT656:
>>>> - vin_dbg(vin, "Found BT656 media bus\n");
>>>> - vin->parallel->mbus_flags = 0;
>>>> + vin_dbg(vin, "Found %s media bus\n",
>>>> + vin->parallel->mbus_type == V4L2_MBUS_PARALLEL ?
>>>> + "PARALLEL" : "BT656");
>>>> + vin->parallel->bus = vep->bus.parallel;
>>>> break;
>>>> default:
>>>> vin_err(vin, "Unknown media bus type\n");
>>>> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
>>>> index a5dbb90c5210..d067439b0b0d 100644
>>>> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
>>>> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
>>>> @@ -125,6 +125,7 @@
>>>> #define VNDMR2_VPS (1 << 30)
>>>> #define VNDMR2_HPS (1 << 29)
>>>> #define VNDMR2_CES (1 << 28)
>>>> +#define VNDMR2_YDS (1 << 22)
>>>> #define VNDMR2_FTEV (1 << 17)
>>>> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
>>>>
>>>> @@ -698,16 +699,26 @@ static int rvin_setup(struct rvin_dev *vin)
>>>>
>>>> if (!vin->is_csi) {
>>>> /* Hsync Signal Polarity Select */
>>>> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
>>>> + if (!(vin->parallel->bus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
>>>> dmr2 |= VNDMR2_HPS;
>>>>
>>>> /* Vsync Signal Polarity Select */
>>>> - if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
>>>> + if (!(vin->parallel->bus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
>>>> dmr2 |= VNDMR2_VPS;
>>>>
>>>> /* Data Enable Polarity Select */
>>>> - if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
>>>> + if (vin->parallel->bus.flags & V4L2_MBUS_DATA_ENABLE_LOW)
>>>> dmr2 |= VNDMR2_CES;
>>>> +
>>>> + switch (vin->mbus_code) {
>>>> + case MEDIA_BUS_FMT_UYVY8_2X8:
>>>> + if (vin->parallel->bus.bus_width == 8 &&
>>>> + vin->parallel->bus.data_shift == 8)
>>>> + dmr2 |= VNDMR2_YDS;
>>>> + break;
>>>> + default:
>>>> + break;
>>>> + }
>>>> }
>>>>
>>>> /*
>>>> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
>>>> index c19d077ce1cb..8396e0e45478 100644
>>>> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
>>>> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
>>>> @@ -19,6 +19,7 @@
>>>> #include <media/v4l2-ctrls.h>
>>>> #include <media/v4l2-dev.h>
>>>> #include <media/v4l2-device.h>
>>>> +#include <media/v4l2-fwnode.h>
>>>> #include <media/videobuf2-v4l2.h>
>>>>
>>>> /* Number of HW buffers */
>>>> @@ -92,7 +93,7 @@ struct rvin_video_format {
>>>> * @asd: sub-device descriptor for async framework
>>>> * @subdev: subdevice matched using async framework
>>>> * @mbus_type: media bus type
>>>> - * @mbus_flags: media bus configuration flags
>>>> + * @bus: media bus parallel configuration
>>>> * @source_pad: source pad of remote subdevice
>>>> * @sink_pad: sink pad of remote subdevice
>>>> *
>>>> @@ -102,7 +103,7 @@ struct rvin_parallel_entity {
>>>> struct v4l2_subdev *subdev;
>>>>
>>>> enum v4l2_mbus_type mbus_type;
>>>> - unsigned int mbus_flags;
>>>> + struct v4l2_fwnode_bus_parallel bus;
>>>>
>>>> unsigned int source_pad;
>>>> unsigned int sink_pad;
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-17 10:45 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-13 18:16 [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift Lad Prabhakar
2020-09-14 23:45 ` Laurent Pinchart
2020-09-15 0:01 ` Niklas Söderlund
2020-09-15 0:02 ` Laurent Pinchart
2020-09-17 8:57 ` Hans Verkuil
2020-09-17 9:58 ` Hans Verkuil
2020-09-17 10:19 ` Niklas Söderlund
2020-09-17 10:42 ` Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.