[PATCH] drm/i915: Fix VLV analog output shivers

[PATCH] drm/i915: Fix VLV analog output shivers

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The current PLL settings produce a rather unstable picture when
I hook up a VLV to my HP ZR24w display via a VGA cable. Switching
the PLL to hybrid mode makes the picture a lot more stable. No
idea if this is truly wise though...

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 29f2c0d..8c30a4c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4412,8 +4412,14 @@ static void vlv_update_pll(struct intel_crtc *crtc)
 		vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
 				 0x00d0000f);
 
-	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP) ||
-	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)) {
+	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_ANALOG)) {
+		/* XXX this gives a less shivery picture */
+		if (!pipe)
+			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x08f70000);
+		else
+			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x08f40000);
+	} else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP) ||
+		   intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)) {
 		/* Use SSC source */
 		if (!pipe)
 			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
@@ -4421,7 +4427,7 @@ static void vlv_update_pll(struct intel_crtc *crtc)
 		else
 			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
 					 0x0df70000);
-	} else { /* HDMI or VGA */
+	} else { /* HDMI */
 		/* Use bend source */
 		if (!pipe)
 			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
-- 
1.8.1.5

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

Re: [PATCH] drm/i915: Fix VLV analog output shivers

[Daniel Vetter]

On Tue, Jun 11, 2013 at 11:08:16PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The current PLL settings produce a rather unstable picture when
> I hook up a VLV to my HP ZR24w display via a VGA cable. Switching
> the PLL to hybrid mode makes the picture a lot more stable. No
> idea if this is truly wise though...

Ok, you've just slipped up here and mentioned that the changed bit is for
"hybrid mode". Can I have real register defines for this magic now please?

Apparently Jesse just weaseled out of real work claiming that it's not
documented at all ;-)

Cheers, Daniel

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 29f2c0d..8c30a4c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4412,8 +4412,14 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>  		vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
>  				 0x00d0000f);
>  
> -	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP) ||
> -	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)) {
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_ANALOG)) {
> +		/* XXX this gives a less shivery picture */
> +		if (!pipe)
> +			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x08f70000);
> +		else
> +			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x08f40000);
> +	} else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP) ||
> +		   intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)) {
>  		/* Use SSC source */
>  		if (!pipe)
>  			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
> @@ -4421,7 +4427,7 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>  		else
>  			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
>  					 0x0df70000);
> -	} else { /* HDMI or VGA */
> +	} else { /* HDMI */
>  		/* Use bend source */
>  		if (!pipe)
>  			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Fix VLV analog output shivers

[Jesse Barnes]

On Tue, 11 Jun 2013 23:06:59 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Jun 11, 2013 at 11:08:16PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The current PLL settings produce a rather unstable picture when
> > I hook up a VLV to my HP ZR24w display via a VGA cable. Switching
> > the PLL to hybrid mode makes the picture a lot more stable. No
> > idea if this is truly wise though...
> 
> Ok, you've just slipped up here and mentioned that the changed bit is for
> "hybrid mode". Can I have real register defines for this magic now please?
> 
> Apparently Jesse just weaseled out of real work claiming that it's not
> documented at all ;-)
> 

There are some bits we could use, but we'd be making up the name.  On
top of that, the hex value is used in the docs, so if we make up bit
field names, we'll end up double taking everytime we look at these bits.

So there's no good answer here... :/

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix VLV analog output shivers

[Ville Syrjälä]

On Tue, Jun 11, 2013 at 04:18:46PM -0700, Jesse Barnes wrote:
> On Tue, 11 Jun 2013 23:06:59 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Tue, Jun 11, 2013 at 11:08:16PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > The current PLL settings produce a rather unstable picture when
> > > I hook up a VLV to my HP ZR24w display via a VGA cable. Switching
> > > the PLL to hybrid mode makes the picture a lot more stable. No
> > > idea if this is truly wise though...
> > 
> > Ok, you've just slipped up here and mentioned that the changed bit is for
> > "hybrid mode". Can I have real register defines for this magic now please?
> > 
> > Apparently Jesse just weaseled out of real work claiming that it's not
> > documented at all ;-)
> > 
> 
> There are some bits we could use, but we'd be making up the name.  On
> top of that, the hex value is used in the docs, so if we make up bit
> field names, we'll end up double taking everytime we look at these bits.
> 
> So there's no good answer here... :/

For this particular register I've not seen any documented "correct"
value. I have no idea where the values used in the code came from. If
you have a document that has them, I'd like to see it.

There are certainly some register values that are specified as raw hex
numbers in some docs, but I've noticed that in some cases even those
don't agree with our code :(

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Fix VLV analog output shivers

[Daniel Vetter]

On Wed, Jun 12, 2013 at 10:14 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Tue, Jun 11, 2013 at 04:18:46PM -0700, Jesse Barnes wrote:
>> On Tue, 11 Jun 2013 23:06:59 +0200
>> Daniel Vetter <daniel@ffwll.ch> wrote:
>>
>> > On Tue, Jun 11, 2013 at 11:08:16PM +0300, ville.syrjala@linux.intel.com wrote:
>> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > >
>> > > The current PLL settings produce a rather unstable picture when
>> > > I hook up a VLV to my HP ZR24w display via a VGA cable. Switching
>> > > the PLL to hybrid mode makes the picture a lot more stable. No
>> > > idea if this is truly wise though...
>> >
>> > Ok, you've just slipped up here and mentioned that the changed bit is for
>> > "hybrid mode". Can I have real register defines for this magic now please?
>> >
>> > Apparently Jesse just weaseled out of real work claiming that it's not
>> > documented at all ;-)
>> >
>>
>> There are some bits we could use, but we'd be making up the name.  On
>> top of that, the hex value is used in the docs, so if we make up bit
>> field names, we'll end up double taking everytime we look at these bits.
>>
>> So there's no good answer here... :/
>
> For this particular register I've not seen any documented "correct"
> value. I have no idea where the values used in the code came from. If
> you have a document that has them, I'd like to see it.
>
> There are certainly some register values that are specified as raw hex
> numbers in some docs, but I've noticed that in some cases even those
> don't agree with our code :(

Yeah, I'd prefer if we can slowly move to less magic. It's more work
to fill updates from the validation guys into patches, otoh it should
help greatly to reduce accidental breakage. And if regressions start
to hit us we might have a stab at a sensible revert.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch