[PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Damian Hobson-Garcia]

Hi Laurent,

Thank you for the review.

On 2023/08/03 20:06, Laurent Pinchart wrote:
> On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
>> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
>>> Hi Damian,
>>>
>>> Thank you for the patch.
>>>
>>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
>>>> Add additional pixel formats for which blending is disabling when
>>> Did you mean "disabled" instead of "disabling" ?


Oops.  Yes, that's exactly what I meant.


>>>
>>>> DRM_MODE_BLEND_PIXEL_NONE is set.
>>>>
>>>> Refactor the fourcc selection into a separate function to handle the
>>>> increased number of formats.
>>>>
>>>> Signed-off-by: Damian Hobson-Garcia <dhobsong at igel.co.jp>
>>>> ---
>>>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
>>>>   1 file changed, 32 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>> index 45c05d0ffc70..96241c03b60f 100644
>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
>>>>   	DRM_FORMAT_Y212,
>>>>   };
>>>>   
>>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
>>>> +{
>>>> +	u32 fourcc = state->format->fourcc;
>>>> +
>>>> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
>>>> +		switch (fourcc) {
>>>> +		case DRM_FORMAT_ARGB1555:
>>>> +			fourcc = DRM_FORMAT_XRGB1555;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_ARGB4444:
>>>> +			fourcc = DRM_FORMAT_XRGB4444;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_ARGB8888:
>>>> +			fourcc = DRM_FORMAT_XRGB8888;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_BGRA8888:
>>>> +			fourcc = DRM_FORMAT_BGRX8888;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_RGBA1010102:
>>>> +			fourcc = DRM_FORMAT_RGBX1010102;
>>>> +			break;
>>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
>>> intentionally ?

Yes, it was intentionally left out for the time being for the
reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not
being handled by the DU driver).

>> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
>> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
>> Let's do so with a patch on top of this series.
Yes, I was thinking the same thing.
> Replying to myself again, the datasheet doesn't explicitly list
> DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
> specify the location of the components should work fine for that format.
> Is this something you would be able to test ?

Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 
formats with.
I will double-check with the office in Japan, but I don't think that 
they have one either.

>
>> There's no need to send
>> a v2, I can handle the simple change in the commit message if you let me
>> know whether my comment is right or wrong.


Thank you,
Damian