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

[Daniel Vetter]

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.

> > I'm looking at the 965/G35 PRM and the "sync disable" are defined correctly but
> > used incorrectly in intel_disable_crt(). That's what my patch fixes. I haven't
> > checked the other PRMs. Is it different on newer hardware?
> 
> This is what the docs say:
> 
> 11:10 Monitor DPMS: (for CRT port) ...
> ...
> 00 = ... (will not affect sync pulses)
> 01 = ... (HSYNC pulses, VSYNC does not)
> 10 = ... (VSYNC pulses, HSYNC does not)
> 11 = ... (Neither HSYNC nor VSYNC pulses)
> 
> These are our definintions:
> 
> #define   ADPA_VSYNC_CNTL_DISABLE (1<<11)
> #define   ADPA_HSYNC_CNTL_DISABLE (1<<10)
> 
> As you can see they don't match.

I've checked gen2/3 docs and they agree with this, so we need to update
the #define.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch