On Tue, Sep 04, 2018 at 06:44:14PM +0100, Chris Wilson wrote: > 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@xxxxxxxxxxxxxxxxxx> > > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> > > > --- > > > 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. My expectation would be that it gets cleared somewhere, but I simply can't find any such code. So the only explanation I have now is that there has never been any commits on that crtc/connector before and that's why it's NULL. Based on that theory this does seem sane, so Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
