Re: [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state

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

On Tue, Jan 13, 2015 at 12:34:06AM +0100, Daniel Vetter wrote:
> On Mon, Jan 12, 2015 at 05:36:52PM +0200, Ander Conselvan de Oliveira wrote:
> > Otherwise setting the rotation property will cause the primary plane to
> > be disabled, caused by having a 0x0 initial value.
> >
> > Cc: stable@vger.kernel.org
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I guess long term we need to consolidate all our plane state related
> readout functions, atm it's splattered all over. But that's maybe
> something for after all the atomic work has stablized a bit.

We would also need to consider the user requested vs. current state
issue during resume. If someone suspends with a bunch of planes
enabled we should restore them during resume rather than overwrite
the user requested state with the current hardware state.

So the patch isn't entirely correct in that sense, but given that
we overwrite these values again based on the crtc->x/y and crtc->mode
when we restore the mode, so it should come out ok in this case. I
suppose we don't yet track the current vs. user state sufficiently
to do things in an entirely correct way.


> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d1a4de8..fdea96c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13322,6 +13322,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> >   return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
> >  }
> >
> > +static void primary_update_size(struct intel_crtc *crtc)
> > +{
> > + struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> > +
> > + if (!crtc->primary_enabled)
> > + return;
> > +
> > + primary->crtc_x = 0;
> > + primary->crtc_y = 0;
> > + primary->crtc_w = crtc->config.pipe_src_w;
> > + primary->crtc_h = crtc->config.pipe_src_h;
> > + primary->src_x = 0;
> > + primary->src_y = 0;
> > + primary->src_w = crtc->config.pipe_src_w << 16;
> > + primary->src_h = crtc->config.pipe_src_h << 16;
> > +}
> > +
> >  static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >  {
> >   struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -13332,6 +13349,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >   int i;
> >
> >   for_each_intel_crtc(dev, crtc) {
> > +
> >   memset(&crtc->config, 0, sizeof(crtc->config));
> >
> >   crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
> > @@ -13341,6 +13359,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >
> >   crtc->base.enabled = crtc->active;
> >   crtc->primary_enabled = primary_get_hw_state(crtc);
> > + primary_update_size(crtc);
> >
> >   DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> >        crtc->base.base.id,
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx