Re: [PATCH v3] media: rcar-vin: Enable YDS bit depending on bus_width and data_shift

[Hans Verkuil <hverkuil@xs4all.nl>]

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;
>>>>
>>>
>>
>