[Intel-gfx] [PATCH] drm/i915: Turn off hsync and vsync on ADPA when disabling crt

[Daniel Vetter]

On Tue, Mar 05, 2013 at 05:41:04PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 05, 2013 at 04:23:59PM +0100, Daniel Vetter wrote:
> > On Tue, Mar 05, 2013 at 04:59:12PM +0200, Ville Syrjälä wrote:
> > > On Tue, Mar 05, 2013 at 03:33:26PM +0100, Patrik Jakobsson wrote:
> > > > > Accroding to the docs these bits don't exist on PCH platforms.
> > > > > intel_crt_dpms() already has a check for this, so I suppose
> > > > > intel_disable_crt() should have one too.
> > > > >
> > > > > Also I noticed that we seem to have the hsync and vsync disable
> > > > > bits reversed. At least that's what the docs are telling me.
> > > > 
> > > > The PCH check just forces suspend and standby to off and we're only doing dpms
> > > > off in intel_disable_crt() so no need to check it there.
> > > 
> > > You're right. I assumed that the check would somehow avoid setting
> > > these bits too, but it doesn't. So I guess we don't really care
> > > that they don't exist.
> > 
> > The dpms state gets clamped to the values support by the hw in
> > intel_crt_dpms. So I think we should care also in intel_crt_disable.
> 
> The point was that in intel_crt_dpms() we don't care whether the
> hsync/vsync disable bits actually exist. We just set them whenever
> the dpms mode warrants it. So for "off" we always set both bits, and
> "off" is always supported. And intel_crt_disable() is equal to
> intel_crt_dpms(DRM_MODE_DPMS_OFF) so the behaviour is consistent
> across the board.
> 
> Whether or not there could be side effects from setting those bits
> on PCH plaforms is another matter. If there are, then the clamping
> stuff is not enough and we need to add PCH checks to both functions.

Oh right, I've missed that when reworking that code - we set the bits in
the OFF case ... I guess that'd be worthwile to amend, if just to keep out
any future surprises.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch