On Thu, Jul 28, 2016 at 06:30:47PM +0100, Chris Wilson wrote:
> On Thu, Jul 28, 2016 at 05:50:38PM +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > 
> > Currently we re-read a bunch of static eDP panel caps from the DPCD
> > over and over again. Let's do it only once to save some time and effort.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk>
> 
> >  static bool
> >  intel_dp_get_dpcd(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 = to_i915(dev);
> > +   bool init_edp_dpcd = is_edp(intel_dp) &&
> > +           intel_dp->dpcd[DP_DPCD_REV] == 0;
> >  
> >     if (drm_dp_dpcd_read(&intel_dp->aux, 0x000, intel_dp->dpcd,
> >                          sizeof(intel_dp->dpcd)) < 0)
> 
> > +   if (init_edp_dpcd)
> > +           intel_edp_get_dpcd(intel_dp);
> 
> I have to admit it took me a few passes to realise how this achieves
> init once.
> 
>       bool intel_dp_dpcd_is_unset(intel_dp)
>       {
>               return intel_dp->dpcd[DP_DPCD_REV] == 0;
>       }
> 
> Not really sure how else to make it clear to the causal reader that test
> is only true on the first call.

First I thought I'd just call the eDP specific function from the
connector init, but then I realized it needs to happen after reading the
regular DPCD stuff, but before printing out the rates/etc. So in the end
I just left it where it was and added the bool. I suppose it should be
possible to refactor the thing somehow to make it less confusing.

I guess I can give it one more try, and if I should fail, I'll suck in
your intel_dp_dpcd_is_unset() at least...

-- 
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