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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Damian,

(CC'ing Tomi)

On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote:
> 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@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.

Tomi, is this something you could test ?

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

-- 
Regards,

Laurent Pinchart