On Thu, Dec 15, 2016 at 05:05:05PM -0200, Paulo Zanoni wrote: > Em Ter, 2016-11-22 às 18:02 +0200, ville.syrjala@xxxxxxxxxxxxxxx > escreveu: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > With plane->plane now purely reserved for the primary planes, let's > > not even populate it for cursors and sprites. Let's switch the type > > to enum plane as well since it's no longer being abused for anything > > else. > > Since it's a primary plane thing, I think it makes more sense to just > leave it at struct intel_crtc instead of having a field that's unused > and doesn't make sense in some cases. Primary plane aren't really primary planes on old platforms though. That's just a software concept that has no bearing on the hardware. > The crtc struct already includes > some fields that are specific to primary planes, so I think it's a good > place. Or we could create a new class: struct intel_primary_plane { > struct intel_plane base; enum plane legacy_plane }; > > Following is a patch suggestion (probably impossible to apply due to my > editor breaking long lines). Any arguments against it? > > --8<---------------------------------------------------------- > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index bc1af87..c54b1a7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15065,14 +15065,6 @@ intel_primary_plane_create(struct > drm_i915_private *dev_priv, enum pipe pipe) > state->scaler_id = -1; > } > primary->pipe = pipe; > - /* > - * On gen2/3 only plane A can do FBC, but the panel fitter and > LVDS > - * port is hooked to pipe B. Hence we want plane A feeding > pipe B. > - */ > - if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4) > - primary->plane = (enum plane) !pipe; > - else > - primary->plane = (enum plane) pipe; > primary->id = PLANE_PRIMARY; > primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe); > primary->check_plane = intel_check_primary_plane; > @@ -15120,7 +15112,7 @@ intel_primary_plane_create(struct > drm_i915_private *dev_priv, enum pipe pipe) > 0, &intel_plane_funcs, > intel_primary_formats, > num_formats, > DRM_PLANE_TYPE_PRIMARY, > - "plane %c", > plane_name(primary->plane)); > + "plane %c", > pipe_name(pipe)); /* FIXME */ > if (ret) > goto fail; > > @@ -15272,7 +15264,6 @@ intel_cursor_plane_create(struct > drm_i915_private *dev_priv, enum pipe pipe) > cursor->can_scale = false; > cursor->max_downscale = 1; > cursor->pipe = pipe; > - cursor->plane = pipe; > cursor->id = PLANE_CURSOR; > cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe); > cursor->check_plane = intel_check_cursor_plane; > @@ -15390,7 +15381,14 @@ static int intel_crtc_init(struct > drm_i915_private *dev_priv, enum pipe pipe) > goto fail; > > intel_crtc->pipe = pipe; > - intel_crtc->plane = primary->plane; > + /* > + * On gen2/3 only plane A can do FBC, but the panel fitter and > LVDS > + * port is hooked to pipe B. Hence we want plane A feeding > pipe B. > + */ > + if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4) > + intel_crtc->plane = (enum plane) !pipe; > + else > + intel_crtc->plane = (enum plane) pipe; > > intel_crtc->cursor_base = ~0; > intel_crtc->cursor_cntl = ~0; > @@ -16866,11 +16864,11 @@ void i915_redisable_vga(struct > drm_i915_private *dev_priv) > intel_display_power_put(dev_priv, POWER_DOMAIN_VGA); > } > > -static bool primary_get_hw_state(struct intel_plane *plane) > +static bool primary_get_hw_state(struct intel_crtc *crtc) > { > - struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > - return I915_READ(DSPCNTR(plane->plane)) & > DISPLAY_PLANE_ENABLE; > + return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE; > } > > /* FIXME read out full plane state for all planes */ > @@ -16880,8 +16878,7 @@ static void readout_plane_state(struct > intel_crtc *crtc) > struct intel_plane_state *plane_state = > to_intel_plane_state(primary->state); > > - plane_state->base.visible = crtc->active && > - primary_get_hw_state(to_intel_plane(primary)); > + plane_state->base.visible = crtc->active && > primary_get_hw_state(crtc); > > if (plane_state->base.visible) > crtc->base.state->plane_mask |= 1 << > drm_plane_index(primary); > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 362f698..b2bdd49 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -767,7 +767,6 @@ struct intel_plane_wm_parameters { > > struct intel_plane { > struct drm_plane base; > - u8 plane; > enum plane_id id; > enum pipe pipe; > bool can_scale; > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > b/drivers/gpu/drm/i915/intel_sprite.c > index 63154a5..1044095 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1111,7 +1111,6 @@ intel_sprite_plane_create(struct drm_i915_private > *dev_priv, > } > > intel_plane->pipe = pipe; > - intel_plane->plane = plane; > intel_plane->id = PLANE_SPRITE0 + plane; > intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, > plane); > intel_plane->check_plane = intel_check_sprite_plane; > > --8<---------------------------------------------------------- > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_display.c | 1 - > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_sprite.c | 1 - > > 3 files changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index b6d5d5e5cc99..f180f14fcf3a 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -15188,7 +15188,6 @@ intel_cursor_plane_create(struct > > drm_i915_private *dev_priv, enum pipe pipe) > > cursor->can_scale = false; > > cursor->max_downscale = 1; > > cursor->pipe = pipe; > > - cursor->plane = pipe; > > cursor->id = PLANE_CURSOR; > > cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe); > > cursor->check_plane = intel_check_cursor_plane; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index c1b245853ba9..d14718e09911 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -767,7 +767,7 @@ struct intel_plane_wm_parameters { > > > > struct intel_plane { > > struct drm_plane base; > > - u8 plane; > > + enum plane plane; > > enum plane_id id; > > enum pipe pipe; > > bool can_scale; > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > > b/drivers/gpu/drm/i915/intel_sprite.c > > index 63154a5a9305..1044095d0084 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -1111,7 +1111,6 @@ intel_sprite_plane_create(struct > > drm_i915_private *dev_priv, > > } > > > > intel_plane->pipe = pipe; > > - intel_plane->plane = plane; > > intel_plane->id = PLANE_SPRITE0 + plane; > > intel_plane->frontbuffer_bit = > > INTEL_FRONTBUFFER_SPRITE(pipe, plane); > > intel_plane->check_plane = intel_check_sprite_plane; -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx