Re: [PATCH 2/2] drm/i915: take a power domain reference while checking the HDMI live status

[Imre Deak <imre.deak@intel.com>]

On pe, 2015-11-20 at 13:13 +0200, Jani Nikula wrote:
> On Fri, 20 Nov 2015, Imre Deak <imre.deak@intel.com> wrote:
> > On Fri, 2015-11-20 at 11:54 +0200, Imre Deak wrote:
> > > On Thu, 2015-11-19 at 21:17 +0200, Ville Syrjälä wrote:
> > > > On Thu, Nov 19, 2015 at 09:13:04PM +0200, Imre Deak wrote:
> > > > > On Thu, 2015-11-19 at 21:08 +0200, Ville Syrjälä wrote:
> > > > > > On Thu, Nov 19, 2015 at 08:55:01PM +0200, Imre Deak wrote:
> > > > > > > There are platforms that don't need the full GMBUS power
> > > > > > > domain
> > > > > > > (PCH, BXT) while others do (VLV/CHV). For optimizing this
> > > > > > > we
> > > > > > > would need to add a new power domain, but it's not clear
> > > > > > > how
> > > > > > > much
> > > > > > > we
> > > > > > > would benefit given the short time we hold the reference.
> > > > > > > So
> > > > > > > for
> > > > > > > now
> > > > > > > let's keep things simple.
> > > > > > 
> > > > > > Actually on PCH platforms the gmbus domain means just just
> > > > > > an
> > > > > > rpm ref since the gmbus hw lives in the PCH.
> > > > > 
> > > > > Ah right.
> > > > > 
> > > > > > And IIRC on BXT gmbus lives in pw0 so same deal really.
> > > > > 
> > > > > It's in PW2 there. I'll fix the commit message.
> > > > 
> > > > Doh. Not sure where I got the PW0 zero idea. Maybe I confused
> > > > gmbus
> > > > with hpd. But yes, you're right about that.
> > > > 
> > > > > 
> > > > > > And for vlv/chv we should just need the disp2d well, which
> > > > > > is exactly what we get with the gmbus domain.
> > > > > > 
> > > > > > So I don't think there's actually anything to optimize here
> > > > > > with current platforms.
> > > > > > 
> > > > > > Both patches look fine to me:
> > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Pushed 1/2 and 2/2 with the fixed commit message to dinq. Thanks
> > > for
> > > the review.
> > 
> > I just noticed that 2/2 fixes the following commit which is in
> > v4.4-rc1 
> > already:
> > 
> > commit 237ed86c693d8a8e4db476976aeb30df4deac74b
> > Author: Sonika Jindal <sonika.jindal@intel.com>
> > Date:   Tue Sep 15 09:44:20 2015 +0530
> > 
> >     drm/i915: Check live status before reading edid
> > 
> > so AFAIU this should've gone through -fixes. Jani can you still
> > apply
> > it there?
> 
> Otherwise no problem but this has a dependency on
> 
> commit f0ab43e6c338896cadee64ced3fc30a5343890d9
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Mon Nov 9 16:48:19 2015 +0100
> 
>     drm/i915: Introduce a gmbus power domain
> 
> and didn't bother figuring out how deep the rabbit hole goes.
> 
> If you want this in -fixes, please either send a -fixes specific
> patch,
> or see how many commits would need to be backported.

Ok. I backported the 5 patches we'd need. The only issue with that
AFAICS is to resolve any conflict between drm-intel-fixes and dinq, but
that looks manageable. I did have to resolve one conflict in one of the
patches while backporting, so I pushed them to
https://github.com/ideak/linux/commits/gmbus-fix-backport

I can also send the rebased versions to the list if needed.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx