[Intel-gfx] [PATCH] drm/i915: Be defensive and don't assume PSR has any commit to sync against

Chris Wilson chris at chris-wilson.co.uk
Tue Sep 4 17:44:14 UTC 2018


Quoting Ville Syrjälä (2018-09-04 18:39:53)
> On Tue, Sep 04, 2018 at 05:29:02PM +0100, Chris Wilson wrote:
> > If the previous modeset commit has completed and is no longer part of
> > the crtc state, skip waiting for it.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107792
> > Fixes: c44301fce614 ("drm/i915: Allow control of PSR at runtime through debugfs, v6")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 21984d4c08ed..bddc9c7c681e 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -834,6 +834,7 @@ int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
> >       struct drm_device *dev = &dev_priv->drm;
> >       struct drm_connector_state *conn_state;
> >       struct intel_crtc_state *crtc_state = NULL;
> > +     struct drm_crtc_commit *commit = NULL;
> >       struct drm_crtc *crtc;
> >       struct intel_dp *dp;
> >       int ret;
> > @@ -860,12 +861,15 @@ int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
> >                       return ret;
> >  
> >               crtc_state = to_intel_crtc_state(crtc->state);
> > -             ret = wait_for_completion_interruptible(&crtc_state->base.commit->hw_done);
> > -     } else
> > -             ret = wait_for_completion_interruptible(&conn_state->commit->hw_done);
> > -
> > -     if (ret)
> > -             return ret;
> > +             commit = crtc_state->base.commit;
> > +     } else {
> > +             commit = conn_state->commit;
> 
> I can't even find where we clear state->commit after its done.
> Do we just leave it pointing at freed memory or something? Also I
> can't figure out why drm_atomic_helper_commit_hw_done() copies
> the commit also to the old state.

Let me be the messenger then ;) commit is NULL at this point, I just
presumed it was intentional.
-Chris


More information about the Intel-gfx mailing list