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

[Jernej Škrabec]

Hi!

Dne četrtek, 02. januar 2020 ob 17:32:07 CET je Roman Stratiienko napisal(a):
> чт, 2 янв. 2020 г., 12:08 Maxime Ripard <mripard at kernel.org>:
> > Hi,
> > 
> > On Wed, Jan 01, 2020 at 10:47:50PM +0200, 
roman.stratiienko at globallogic.com wrote:
> > > From: Roman Stratiienko <roman.stratiienko at globallogic.com>
> > > 
> > > 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 at globallogic.com>
> > 
> > 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