On Tue, Nov 08, 2016 at 03:26:31PM +0000, Chris Wilson wrote: > On Tue, Nov 08, 2016 at 04:47:12PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > 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.roper@xxxxxxxxx> > > Cc: Daniel Vetter <daniel@xxxxxxxx> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx