On Tue, Nov 08, 2016 at 03:26:31PM +0000, Chris Wilson wrote:
> On Tue, Nov 08, 2016 at 04:47:12PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> >
> > As I told people in [1] we really should not be confusing enum plane
> > as a per-pipe plane identifier. Looks like that happened nonetheless, so
> > let's fix it up by splitting the two into two enums.
> >
> > We'll also want something we just directly pass to various register
> > offset macros and whatnot on SKL+. So let's make this new thing work for
> > that.
> > Currently we pass intel_plane->plane for the "sprites" and just a
> > hardcoded zero for the "primary" planes. We want to get rid of that
> > hardocoding so that we can share the same code for all planes (apart
> > from the legacy cursor of course).
> >
> > [1]
> > https://lists.freedesktop.org/archives/intel-gfx/2015-September/076082.html
> >
> > Cc: Matt Roper <matthew.d.ro...@intel.com>
> > Cc: Daniel Vetter <dan...@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 28 +++++++++++++++++++++-------
> > drivers/gpu/drm/i915/intel_display.c | 2 ++
> > drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> > drivers/gpu/drm/i915/intel_sprite.c | 1 +
> > 4 files changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 30777dee3f9c..2451b88b1e82 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -171,22 +171,36 @@ static inline bool transcoder_is_dsi(enum transcoder
> > transcoder)
> > }
> >
> > /*
> > - * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
> > - * number of planes per CRTC. Not all platforms really have this many
> > planes,
> > - * which means some arrays of size I915_MAX_PLANES may have unused entries
> > - * between the topmost sprite plane and the cursor plane.
> > + * Global legacy plane identifier. Valid only for primary/sprite
> > + * planes on pre-g4x, and only for primary planes on g4x+.
> > */
> > enum plane {
> > - PLANE_A = 0,
> > + PLANE_A,
> > PLANE_B,
> > PLANE_C,
> > - PLANE_CURSOR,
> > - I915_MAX_PLANES,
> > };
> > #define plane_name(p) ((p) + 'A')
>
> And make then non-interchangeable with PLANE_A = 'A', PLANE_B = 'B' etc?
>
> > #define sprite_name(p, s) ((p) * INTEL_INFO(dev_priv)->num_sprites[(p)] +
> > (s) + 'A')
>
> But that makes that macro even worse. It was an idea.
Yeah, would be nice to make it harder to mix things up somehow. But I
must admit I didn't spend much time thinking what would be the best way
to achieve that.
>
> > struct intel_plane {
> > struct drm_plane base;
> > - int plane;
> > + u8 plane;
>
> enum plane plane ?
A bit later ;)
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
