[Intel-gfx] [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Daniel Vetter]

On Mon, Mar 30, 2015 at 05:39:54PM +0200, Daniel Vetter wrote:
> On Mon, Mar 30, 2015 at 03:19:29PM +0530, Ramalingam C wrote:
> > Looks good to me..
> > 
> > Reviewed-by: Ramalingam C <ramalingam.c at intel.com>
> > 
> > On Friday 27 March 2015 12:51 AM, Rodrigo Vivi wrote:
> > >With PSR enabled being pre computed on pipe_config we can now
> > >prevent DRRS to be enabled along with PSR.
> > >
> > >v2: Rebase after changing previous patch
> > >
> > >Cc: Ramalingam C <ramalingam.c at intel.com>
> > >Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> 
> Both patches applied, thanks.

Both dropped again since it blows up - in compute_config functions you are
not allowed to look at the current configuration ever. And you do that in
two places with this code:
- deref drm_encoder->crtc, which results in oopses
- deref intel_crtc->config, which is just semantically wrong.

See Chris patch for how to fix this (although it's slightly incomplete
since it doesn't replace all access to intel_crtc->config with the
explicitly passed-in pipe_config pointer).
-Daniel

> -Daniel
> 
> > >---
> > >  drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > >diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > >index e6b1c42..5551b3c 100644
> > >--- a/drivers/gpu/drm/i915/intel_dp.c
> > >+++ b/drivers/gpu/drm/i915/intel_dp.c
> > >@@ -4994,11 +4994,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
> > >  		return;
> > >  	}
> > >-	/*
> > >-	 * FIXME: This needs proper synchronization with psr state for some
> > >-	 * platforms that cannot have PSR and DRRS enabled at the same time.
> > >-	 */
> > >-
> > >  	dig_port = dp_to_dig_port(intel_dp);
> > >  	encoder = &dig_port->base;
> > >  	intel_crtc = encoder->new_crtc;
> > >@@ -5084,6 +5079,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
> > >  		return;
> > >  	}
> > >+	if (intel_crtc->config->psr_ready) {
> > >+		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
> > >+		return;
> > >+	}
> > >+
> > >  	mutex_lock(&dev_priv->drrs.mutex);
> > >  	if (WARN_ON(dev_priv->drrs.dp)) {
> > >  		DRM_ERROR("DRRS already enabled\n");
> > 
> > -- 
> > Thanks,
> > --Ram
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch