On Fri, May 17, 2024 at 06:33:46PM +0300, Jani Nikula wrote:
> On Thu, 16 May 2024, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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
