On Thu, Jul 28, 2016 at 04:38:55PM +0100, Chris Wilson wrote:
> On Thu, Jul 28, 2016 at 05:50:44PM +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > 
> > Supposedly nothing should have change in the DPCD/EDID deparment unless
> > there was a long HPD. Let's skip the work in ->detect() in that case.
> > We'll still want to do the link status check thouhg.
> > 
> > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.olive...@intel.com>
> > Cc: Jim Bride <jim.br...@linux.intel.com>
> > Cc: Manasi D Navare <manasi.d.nav...@intel.com>
> > Cc: Durgadoss R <durgados...@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  | 19 +++++++++++++++----
> >  drivers/gpu/drm/i915/intel_drv.h |  2 +-
> >  2 files changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 675b83f57a07..d1dd351682d2 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4315,12 +4315,15 @@ intel_dp_detect(struct drm_connector *connector, 
> > bool force)
> >  {
> >     struct intel_dp *intel_dp = intel_attached_dp(connector);
> >     struct intel_connector *intel_connector = to_intel_connector(connector);
> > -   enum drm_connector_status status;
> > +   enum drm_connector_status status = connector->status;
> >  
> >     DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> >                   connector->base.id, connector->name);
> >  
> > -   status = intel_dp_long_pulse(intel_connector);
> > +   if (intel_dp->long_hpd_pending) {
> 
> Set from a worker unlocked, consumed unlocked. At the very least you
> want a WRITE_ONCE() to document this should be unset before
> intel_dp_long_pulse(), if (cmpxchg(long_hpd_pending, 1, 0) {} and a
> smp_store_mb() when setting for belt-and-braces approach.

Yeah, I guess we should do something like that. There is other junk
also getting set/checked unlocked in this work as well, and I
haven't had the energy to figure it all out yet.

> 
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 7fef18288aa2..20cf7ad26357 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -854,7 +854,7 @@ struct intel_dp {
> >     uint8_t sink_count;
> >     bool link_mst;
> >     bool has_audio;
> > -   bool detect_done;
> 
> Unrelated cleanup?

That one should be part of the previous patch, I think.

> 
> > +   bool long_hpd_pending;
> >     enum hdmi_force_audio force_audio;
> >     bool limited_color_range;
> >     bool color_range_auto;
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to