[PATCH] drm/i915: Do not dereference NULL crtc or fb until after checking

[PATCH] drm/i915: Do not dereference NULL crtc or fb until after checking

[Chris Wilson]

Fixes regression from
commit 4906557eb37b7fef84fad4304acef6dedf919880
Author: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Date:   Thu Jul 11 18:45:05 2013 -0300

    drm/i915: Hook PSR functionality

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67526
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 928b42f..7e16c6e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1514,10 +1514,10 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc = dig_port->base.base.crtc;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_i915_gem_object *obj = to_intel_framebuffer(crtc->fb)->obj;
-	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
+	struct drm_crtc *crtc;
+	struct intel_crtc *intel_crtc;
+	struct drm_i915_gem_object *obj;
+	struct intel_encoder *intel_encoder;
 
 	if (!IS_HASWELL(dev)) {
 		DRM_DEBUG_KMS("PSR not supported on this platform\n");
@@ -1544,12 +1544,21 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 		return false;
 	}
 
+	crtc = dig_port->base.base.crtc;
+	if (crtc == NULL) {
+		DRM_DEBUG_KMS("crtc not active for PSR\n");
+		dev_priv->no_psr_reason = PSR_CRTC_NOT_ACTIVE;
+		return false;
+	}
+
+	intel_crtc = to_intel_crtc(crtc);
 	if (!intel_crtc->active || !crtc->fb || !crtc->mode.clock) {
 		DRM_DEBUG_KMS("crtc not active for PSR\n");
 		dev_priv->no_psr_reason = PSR_CRTC_NOT_ACTIVE;
 		return false;
 	}
 
+	obj = to_intel_framebuffer(crtc->fb)->obj;
 	if (obj->tiling_mode != I915_TILING_X ||
 	    obj->fence_reg == I915_FENCE_REG_NONE) {
 		DRM_DEBUG_KMS("PSR condition failed: fb not tiled or fenced\n");
-- 
1.8.4.rc0

Re: [PATCH] drm/i915: Do not dereference NULL crtc or fb until after checking

[Chris Wilson]

On Fri, Aug 02, 2013 at 08:39:49PM +0100, Chris Wilson wrote:
> Fixes regression from
> commit 4906557eb37b7fef84fad4304acef6dedf919880
> Author: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> Date:   Thu Jul 11 18:45:05 2013 -0300
> 
>     drm/i915: Hook PSR functionality
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67526
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 928b42f..7e16c6e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1514,10 +1514,10 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_crtc *crtc = dig_port->base.base.crtc;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_i915_gem_object *obj = to_intel_framebuffer(crtc->fb)->obj;
> -	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> +	struct drm_crtc *crtc;
> +	struct intel_crtc *intel_crtc;
> +	struct drm_i915_gem_object *obj;
> +	struct intel_encoder *intel_encoder;

Sigh. I didn't mean to move the intel_encoder in the end. That can be
still be set up during the init as it not part of the bug fix.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Do not dereference NULL crtc or fb until after checking

[Ben Widawsky]

On Fri, Aug 02, 2013 at 09:16:47PM +0100, Chris Wilson wrote:
> On Fri, Aug 02, 2013 at 08:39:49PM +0100, Chris Wilson wrote:
> > Fixes regression from
> > commit 4906557eb37b7fef84fad4304acef6dedf919880
> > Author: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > Date:   Thu Jul 11 18:45:05 2013 -0300
> > 
> >     drm/i915: Hook PSR functionality
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67526
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > Cc: Ben Widawsky <ben@bwidawsk.net>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 928b42f..7e16c6e 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1514,10 +1514,10 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
> >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> >  	struct drm_device *dev = dig_port->base.base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	struct drm_crtc *crtc = dig_port->base.base.crtc;
> > -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > -	struct drm_i915_gem_object *obj = to_intel_framebuffer(crtc->fb)->obj;
> > -	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> > +	struct drm_crtc *crtc;
> > +	struct intel_crtc *intel_crtc;
> > +	struct drm_i915_gem_object *obj;
> > +	struct intel_encoder *intel_encoder;
> 
> Sigh. I didn't mean to move the intel_encoder in the end. That can be
> still be set up during the init as it not part of the bug fix.
> -Chris
> 

Merged to the Daniel temp tree.

-- 
Ben Widawsky, Intel Open Source Technology Center