[PATCH] drm: rcar-du: Fix pitch handling for fully planar YUV formats

[Kieran Bingham]

Hi Ville, Laurent,

On 09/09/2020 13:08, Ville Syrjälä wrote:
> On Tue, Sep 08, 2020 at 05:05:48PM +0100, Kieran Bingham wrote:
>> Hi Laurent,
>>
>> On 08/09/2020 16:52, Laurent Pinchart wrote:
>>> Hi Kieran,
>>>
>>> On Tue, Sep 08, 2020 at 04:42:58PM +0100, Kieran Bingham wrote:
>>>> On 06/08/2020 03:26, Laurent Pinchart wrote:
>>>>> When creating a frame buffer, the driver verifies that the pitches for
>>>>> the chroma planes match the luma plane. This is done incorrectly for
>>>>> fully planar YUV formats, without taking horizontal subsampling into
>>>>> account. Fix it.
>>>>>
>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
>>>>> ---
> <snip>
>>>>>  	}, {
>>>>>  		.fourcc = DRM_FORMAT_YVU444,
>>>>>  		.v4l2 = V4L2_PIX_FMT_YVU444M,
>>>>>  		.bpp = 24,
>>>>>  		.planes = 3,
>>>>> +		.hsub = 1,
>>>>>  	},
>>>>>  };
>>>>>  
>>>>
>>>> I wonder when we can have a global/generic set of format tables so that
>>>> all of this isn't duplicated on a per-driver basis.
>>>
>>> Note that this table also contains register values, so at least that
>>> part will need to be kept. For the rest, do you mean a 4CC library that
>>
>> Yes, the driver specific mappings of course need to be driver specific.
>>
>>
>>> would be shared between DRM/KMS and V4L2 ? That's a great idea. Too bad
>>> it has been shot down when patches were submitted :-S
>>
>>
>>  /o\ ... It just seems like so much data replication that must be used
>> by many drivers.
>>
>> Even without mapping the DRM/V4L2 fourccs - even a common table in each
>> subsystem would be beneficial wouldn't it?
>>
>> I mean - RCar-DU isn't the only device that needs to know how many
>> planes DRM_FORMAT_YUV422 has, or what horizontal subsampling it uses?
>>
>> Anyway, that's not an issue with this patch, it just seems glaring to me
>> that these entries are common across all hardware that use them ...
>>
>> (the bpp/planes/subsampling of course, not the hardware specific registers).
> 
> See drm_format_info() & co.

Aha perfect, That's what I was looking for.
I'm glad to see that's common (at least for the DRM parts).

The question is - why aren't we using it in RCar-DU.

Laurent, would you see any issue in obtaining the struct drm_format_info
rather than re-encoding all the data in these tables?

And if not - would you prefer to convert on top of this patch, or
preceding this patch?

Or simply prefer to keep the existing tables ?

Given that this fixes a bug - I'd say getting this patch in now is
probably best.

-- 
Regards
--
Kieran