Re: [PATCH v3 2/2] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame.

From: Jernej Åkrabec
Date: Thu Jan 02 2020 - 12:25:11 EST


Hi!

Dne Äetrtek, 02. januar 2020 ob 17:32:07 CET je Roman Stratiienko napisal(a):
> ÑÑ, 2 ÑÐÐ. 2020 Ð., 12:08 Maxime Ripard <mripard@xxxxxxxxxx>:
> > Hi,
> >
> > On Wed, Jan 01, 2020 at 10:47:50PM +0200,
roman.stratiienko@xxxxxxxxxxxxxxx wrote:
> > > From: Roman Stratiienko <roman.stratiienko@xxxxxxxxxxxxxxx>
> > >
> > > According to DRM documentation the only difference between PRIMARY
> > > and OVERLAY plane is that each CRTC must have PRIMARY plane and
> > > OVERLAY are optional.
> > >
> > > Allow PRIMARY plane to have dimension different from full-screen.
> > >
> > > Fixes: 5bb5f5dafa1a ("drm/sun4i: Reorganize UI layer code in DE2")
> > > Signed-off-by: Roman Stratiienko <roman.stratiienko@xxxxxxxxxxxxxxx>
> >
> > So it applies to all the 4 patches you've sent, but this lacks some
> > context.
> >
> > There's a few questions that should be answered here:
> > - Which situation is it fixing?
>
> Setting primary plane size less than crtc breaks composition. Also
> shifting top left corner also breaks it.

True, HW doesn't have notion of primary plane. It's just one plane which is
marked as primary, but otherwise it has same capabilities as others, like x,y
coordinates, size, etc.

>
> > - What tool / userspace stack is it fixing?
>
> I am using Android userspace and drm_hwcomposer HAL.
>
> @Jernej, you've said that you observed similar issue. Could you share
> what userspace have you used?

I observed it with DE1, but it has exactly the same issue. I noticed this
problem on Kodi (gbm version). Kodi first searches for plane capable of
displaying NV12 format (for video) and after that a plane which is capable of
displaying RGB888 format (for GUI). In DE1 case, first plane is primary and
also capable of displaying NV12 format. So when video is displayed which
doesn't cover whole screen, display output is corrupted. However, with such
fix, video playback is correct. Luc Verhaegen make equivalent fix for DE1, where
he also claims primary plane doesn't have to be same size as CRTC output:
https://github.com/libv/fosdem-video-linux/commit/
ae3215d37ca2a55642bcae6c83c3612e26275711

>
> > - What happens with your fix? Do you set the plane at coordinates
> >
> > 0,0 (meaning you'll crop the top-lef corner), do you center it? If
> > the plane is smaller than the CTRC size, what is set on the edges?
>
> You can put primary plane to any part of the screen and make it as
> small as 8x8 (according to the datasheet) . Background would be filled
> with black color, that is default, but it also could be overridden by
> setting corresponding registers.

Correct, same logic as for overlay planes applies.

Best regards,
Jernej

>
> > Thanks!
> > Maxime