[BISECTED, -next] drm/i915: blurred HDMI output

[BISECTED, -next] drm/i915: blurred HDMI output

[Arnd Bergmann]

Commit e9e331a8ab "drm/i915/lvds: Ensure panel is unlocked for Ironlake or
the panel fitter" broke my HDMI output on GMA4500HD, it seemed to be driving
the output with the wrong resolution, while still showing the native panel
resolution of 1680x1050 being in use. This caused a very irritating blur
on the screen.

After bisecting the problem, I managed to do a partial revert which fixes
the problem and applies on top of linux-next. I have no idea what this patch
actually does or if it can break any other configuration, but it solves the
problem for me.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9109c00..2d47161 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3921,6 +3921,10 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 	pipeconf |= PIPECONF_ENABLE;
 	dpll |= DPLL_VCO_ENABLE;
 
+	/* Disable the panel fitter if it was on our pipe */
+	if (!HAS_PCH_SPLIT(dev) && intel_panel_fitter_pipe(dev) == pipe)
+		I915_WRITE(PFIT_CONTROL, 0);
+
 	DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe == 0 ? 'A' : 'B');
 	drm_mode_debug_printmodeline(mode);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 40e99bf..de21a82 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -224,6 +224,7 @@ extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
 extern u32 intel_panel_get_backlight(struct drm_device *dev);
 extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
 
+extern int intel_panel_fitter_pipe (struct drm_device *dev);
 extern void intel_crtc_load_lut(struct drm_crtc *crtc);
 extern void intel_encoder_prepare (struct drm_encoder *encoder);
 extern void intel_encoder_commit (struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 375316a..da65d95 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1053,7 +1053,7 @@ static int check_overlay_src(struct drm_device *dev,
  * Return the pipe currently connected to the panel fitter,
  * or -1 if the panel fitter is not present or not in use
  */
-static int intel_panel_fitter_pipe(struct drm_device *dev)
+int intel_panel_fitter_pipe(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32  pfit_control;

Re: [BISECTED, -next] drm/i915: blurred HDMI output

[Jesse Barnes]

On Sun, 17 Oct 2010 19:37:13 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> Commit e9e331a8ab "drm/i915/lvds: Ensure panel is unlocked for Ironlake or
> the panel fitter" broke my HDMI output on GMA4500HD, it seemed to be driving
> the output with the wrong resolution, while still showing the native panel
> resolution of 1680x1050 being in use. This caused a very irritating blur
> on the screen.
> 
> After bisecting the problem, I managed to do a partial revert which fixes
> the problem and applies on top of linux-next. I have no idea what this patch
> actually does or if it can break any other configuration, but it solves the
> problem for me.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9109c00..2d47161 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3921,6 +3921,10 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
>  	pipeconf |= PIPECONF_ENABLE;
>  	dpll |= DPLL_VCO_ENABLE;
>  
> +	/* Disable the panel fitter if it was on our pipe */
> +	if (!HAS_PCH_SPLIT(dev) && intel_panel_fitter_pipe(dev) == pipe)
> +		I915_WRITE(PFIT_CONTROL, 0);
> +
>  	DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe == 0 ? 'A' : 'B');
>  	drm_mode_debug_printmodeline(mode);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 40e99bf..de21a82 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -224,6 +224,7 @@ extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
>  extern u32 intel_panel_get_backlight(struct drm_device *dev);
>  extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
>  
> +extern int intel_panel_fitter_pipe (struct drm_device *dev);
>  extern void intel_crtc_load_lut(struct drm_crtc *crtc);
>  extern void intel_encoder_prepare (struct drm_encoder *encoder);
>  extern void intel_encoder_commit (struct drm_encoder *encoder);
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 375316a..da65d95 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -1053,7 +1053,7 @@ static int check_overlay_src(struct drm_device *dev,
>   * Return the pipe currently connected to the panel fitter,
>   * or -1 if the panel fitter is not present or not in use
>   */
> -static int intel_panel_fitter_pipe(struct drm_device *dev)
> +int intel_panel_fitter_pipe(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32  pfit_control;
> 

Hm, the LVDS code should take care of panel fitting in
the !HAS_PCH_SPLIT case.  Does this patch achieve the same thing as
yours?  Maybe we were leaving a stale PFIT value in place...

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds
index f1a6499..bc1e1c1 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1000,6 +1000,8 @@ void intel_lvds_init(struct drm_device *dev)
        if (!intel_lvds->fixed_mode)
                goto failed;
 
+       intel_lvds->pfit_dirty = true;
+
 out:
        if (HAS_PCH_SPLIT(dev)) {
                u32 pwm;

Re: [BISECTED, -next] drm/i915: blurred HDMI output

[Arnd Bergmann]

On Monday 18 October 2010 20:56:48 Jesse Barnes wrote:
> Hm, the LVDS code should take care of panel fitting in
> the !HAS_PCH_SPLIT case.  Does this patch achieve the same thing as
> yours?  Maybe we were leaving a stale PFIT value in place...

No, your patch doesn't have any effect for me.

	Arnd

Re: [BISECTED, -next] drm/i915: blurred HDMI output

[Jesse Barnes]

On Mon, 18 Oct 2010 21:13:59 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Monday 18 October 2010 20:56:48 Jesse Barnes wrote:
> > Hm, the LVDS code should take care of panel fitting in
> > the !HAS_PCH_SPLIT case.  Does this patch achieve the same thing as
> > yours?  Maybe we were leaving a stale PFIT value in place...
> 
> No, your patch doesn't have any effect for me.

Oh right, you were complaining about HMDI. :)

Yes we still need to shut off panel fitting elsewhere.  Do you see the
blurriness at boot or just after turning on HDMI while leaving the
laptop panel enabled?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [BISECTED, -next] drm/i915: blurred HDMI output

[Arnd Bergmann]

On Monday 18 October 2010 21:28:01 Jesse Barnes wrote:
> 
> On Mon, 18 Oct 2010 21:13:59 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > On Monday 18 October 2010 20:56:48 Jesse Barnes wrote:
> > > Hm, the LVDS code should take care of panel fitting in
> > > the !HAS_PCH_SPLIT case.  Does this patch achieve the same thing as
> > > yours?  Maybe we were leaving a stale PFIT value in place...
> > 
> > No, your patch doesn't have any effect for me.
> 
> Oh right, you were complaining about HMDI. :)
> 
> Yes we still need to shut off panel fitting elsewhere.  Do you see the
> blurriness at boot or just after turning on HDMI while leaving the
> laptop panel enabled?

It's a G45 desktop machine, not a laptop.

i915 is a module with kms enabled, the blur seems to show up on the
text console the moment that the module gets loaded during boot.
Before that, the console is in VGA text mode, which is always
slightly blurred because the screen has a different native resolution.

	Arnd

Re: [BISECTED, -next] drm/i915: blurred HDMI output

[Jesse Barnes]

On Mon, 18 Oct 2010 21:57:05 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Monday 18 October 2010 21:28:01 Jesse Barnes wrote:
> > 
> > On Mon, 18 Oct 2010 21:13:59 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> > 
> > > On Monday 18 October 2010 20:56:48 Jesse Barnes wrote:
> > > > Hm, the LVDS code should take care of panel fitting in
> > > > the !HAS_PCH_SPLIT case.  Does this patch achieve the same thing as
> > > > yours?  Maybe we were leaving a stale PFIT value in place...
> > > 
> > > No, your patch doesn't have any effect for me.
> > 
> > Oh right, you were complaining about HMDI. :)
> > 
> > Yes we still need to shut off panel fitting elsewhere.  Do you see the
> > blurriness at boot or just after turning on HDMI while leaving the
> > laptop panel enabled?
> 
> It's a G45 desktop machine, not a laptop.
> 
> i915 is a module with kms enabled, the blur seems to show up on the
> text console the moment that the module gets loaded during boot.
> Before that, the console is in VGA text mode, which is always
> slightly blurred because the screen has a different native resolution.

Arg why would the BIOS set the panel fitting bit at all in this
config?  Any attached outputs (VGA, HDMI) will have their own scalers...

Obviously we need to clear it somewhere, but we should be able to avoid
clearing it on every mode set.  We should probably shut down all panel
related bits in intel_lvds.c if we fail to detect an LVDS output; that
should save a little power and avoid problems like this.

Maybe the below patch which does that will help?

-- 
Jesse Barnes, Intel Open Source Technology Center

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds
index f1a6499..da31963 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1024,6 +1024,9 @@ out:
 
 failed:
        DRM_DEBUG_KMS("No LVDS modes found, disabling.\n");
+       intel_lvds_set_power(intel_lvds, false);
+       I915_WRITE(PFIT_CONTROL, 0);
+       I915_WRITE(LVDS, 0);
        drm_connector_cleanup(connector);
        drm_encoder_cleanup(encoder);
        kfree(intel_lvds);

Re: [BISECTED, -next] drm/i915: blurred HDMI output

[Arnd Bergmann]

On Monday 18 October 2010 22:15:23 Jesse Barnes wrote:
> On Mon, 18 Oct 2010 21:57:05 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
> > i915 is a module with kms enabled, the blur seems to show up on the
> > text console the moment that the module gets loaded during boot.
> > Before that, the console is in VGA text mode, which is always
> > slightly blurred because the screen has a different native resolution.
> 
> Arg why would the BIOS set the panel fitting bit at all in this
> config?  Any attached outputs (VGA, HDMI) will have their own scalers...

I didn't think the BIOS was setting it. Text mode appears to be blurred
differently, but that's hard to tell...

> Obviously we need to clear it somewhere, but we should be able to avoid
> clearing it on every mode set.  We should probably shut down all panel
> related bits in intel_lvds.c if we fail to detect an LVDS output; that
> should save a little power and avoid problems like this.
> 
> Maybe the below patch which does that will help?

No luck here either.

I don't think the code path you patch here actually gets used, since
intel_lvds_init gets called by intel_setup_outputs only for mobile devices.

	Arnd

Re: [BISECTED, -next] drm/i915: blurred HDMI output

[Arnd Bergmann]

On Monday 18 October 2010 22:30:17 Arnd Bergmann wrote:

> I don't think the code path you patch here actually gets used, since
> intel_lvds_init gets called by intel_setup_outputs only for mobile devices.

FWIW, the patch below does work.

	Arnd

--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5311,6 +5311,9 @@ static void intel_setup_outputs(struct drm_device *dev)
 	if (IS_MOBILE(dev) && !IS_I830(dev))
 		intel_lvds_init(dev);
 
+       I915_WRITE(PFIT_CONTROL, 0);
+       I915_WRITE(LVDS, 0);
+
 	if (HAS_PCH_SPLIT(dev)) {
 		dpd_is_edp = intel_dpd_is_edp(dev);

Re: [BISECTED, -next] drm/i915: blurred HDMI output

[Jesse Barnes]

On Mon, 18 Oct 2010 22:37:43 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Monday 18 October 2010 22:30:17 Arnd Bergmann wrote:
> 
> > I don't think the code path you patch here actually gets used, since
> > intel_lvds_init gets called by intel_setup_outputs only for mobile devices.
> 
> FWIW, the patch below does work.
> 
> 	Arnd
> 
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5311,6 +5311,9 @@ static void intel_setup_outputs(struct drm_device *dev)
>  	if (IS_MOBILE(dev) && !IS_I830(dev))
>  		intel_lvds_init(dev);
>  
> +       I915_WRITE(PFIT_CONTROL, 0);
> +       I915_WRITE(LVDS, 0);
> +
>  	if (HAS_PCH_SPLIT(dev)) {
>  		dpd_is_edp = intel_dpd_is_edp(dev);

Oh of course, I was thinking the output functions did detection, but we
short circuit it before that.

So we should probably do it in setup_outputs or init_display once we've
figured out there's no LVDS.  It's cool that the panel fitter still has
an effect even on non-LVDS platforms though, maybe we really should
treat it as a part of the CRTC rather than the LVDS encoder after all.

-- 
Jesse Barnes, Intel Open Source Technology Center