Re: [PATCH 01/13] drm/i915: Add skl+ plane name aliases to enum plane_id

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, May 17, 2024 at 06:33:46PM +0300, Jani Nikula wrote:
> On Thu, 16 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Using PLANE_PRIMARY + PLANE_SPRITE? on skl+ results in a bunch
> > of unnecessary head scratching. Add aliases using the skl+ plane
> > names.
> > And for pre-skl we only need to keep PRIMARY,SPRITE0,SPRITE1
> > as we only ever have 0-2 sprites per pipe on those platforms.
> 
> Should these be changed too?
> 
> - intel_plane_set_ckey()

I suppose one could consider splitting this to pre-skl
vs. skl+ variants and using the appropriate names
in each. But the whole ckey uapi is really designed
around the pre-skl single primary + single sprite world
view, so using the PLANE_PRIMARY name there seems OK.

> - for_each_plane_id_on_crtc()

There's not really a right answer here I guess. As 
long as it's 0 where we start this will work.


> I'm not sure. But there's one real issue:
> 
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 32d10e62b2b9..d0bfee2ca643 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -238,9 +238,9 @@ int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
> >  static u8 icl_nv12_y_plane_mask(struct drm_i915_private *i915)
> >  {
> >  	if (DISPLAY_VER(i915) >= 13 || HAS_D12_PLANE_MINIMIZATION(i915))
> > -		return BIT(PLANE_SPRITE2) | BIT(PLANE_SPRITE3);
> > +		return BIT(PLANE_6) | BIT(PLANE_7);
> >  	else
> > -		return BIT(PLANE_SPRITE4) | BIT(PLANE_SPRITE5);
> > +		return BIT(PLANE_4) | BIT(PLANE_5);
> 
> The if branches got swapped?

Yeah. Good catch. I suspect my brain was in the
"newer platforms surely have more things" mindset.

-- 
Ville Syrjälä
Intel