On Mon, Jun 16, 2014 at 05:10:14PM -0700, Rodrigo Vivi wrote: > On Mon, Jun 16, 2014 at 10:51 AM, Daniel Vetter <daniel.vetter@xxxxxxxx> > wrote: > > > Trying to fish that one out through looping is a bit a locking > > nightmare. So just set it and use it in the work struct. > > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > > drivers/gpu/drm/i915/intel_dp.c | 21 +++++++-------------- > > 2 files changed, 9 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index f5db29428406..759f7c6d1622 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -633,10 +633,11 @@ struct i915_drrs { > > struct intel_connector *connector; > > }; > > > > +struct intel_dp; > > struct i915_psr { > > bool sink_support; > > bool source_ok; > > - bool enabled; > > + struct intel_dp *enabled; > > bool active; > > struct delayed_work work; > > }; > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index ac1cef459a7b..10bcc052df4b 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -1826,7 +1826,7 @@ static void intel_edp_psr_do_enable(struct intel_dp > > *intel_dp) > > /* Enable PSR on the host */ > > intel_edp_psr_enable_source(intel_dp); > > > > - dev_priv->psr.enabled = true; > > + dev_priv->psr.enabled = intel_dp; > > > > I liked this very much... > > > > dev_priv->psr.active = true; > > } > > > > @@ -1867,26 +1867,19 @@ void intel_edp_psr_disable(struct intel_dp > > *intel_dp) > > EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10)) > > DRM_ERROR("Timed out waiting for PSR Idle State\n"); > > > > - dev_priv->psr.enabled = false; > > + dev_priv->psr.enabled = NULL; > > } > > > > static void intel_edp_psr_work(struct work_struct *work) > > { > > struct drm_i915_private *dev_priv = > > container_of(work, typeof(*dev_priv), psr.work.work); > > - struct drm_device *dev = dev_priv->dev; > > - struct intel_encoder *encoder; > > - struct intel_dp *intel_dp = NULL; > > - > > - list_for_each_entry(encoder, &dev->mode_config.encoder_list, > > base.head) > > - if (encoder->type == INTEL_OUTPUT_EDP) { > > - intel_dp = enc_to_intel_dp(&encoder->base); > > + struct intel_dp *intel_dp = dev_priv->psr.enabled; > > > > > but I'm afraid to give NULL to below functions... > Shouldn't we add a if(!intel_dp) return; at least just to be on the safe > side? Good catch, I'll put that into the series since it breaks bisect. With the fixup from last night the end result is safe again. -Daniel > > - if (!intel_edp_psr_match_conditions(intel_dp)) > > - intel_edp_psr_disable(intel_dp); > > - else > > - intel_edp_psr_do_enable(intel_dp); > > - } > > + if (!intel_edp_psr_match_conditions(intel_dp)) > > + intel_edp_psr_disable(intel_dp); > > + else > > + intel_edp_psr_do_enable(intel_dp); > > } > > > > static void intel_edp_psr_inactivate(struct drm_device *dev) > > -- > > 2.0.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx