On Fri, Aug 04, 2023 at 06:52:51PM +0300, Laurent Pinchart wrote:
> 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:
> > >>> 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.
I'll fix this locally.
> > >>>> 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@xxxxxxxxxx>
> > >>>> ---
> > >>>> 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 ?
Replying to myself, this is something we could test, but let's not delay
this series, new formats can always be added on top.
> > >> 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
