[Intel-gfx] [PATCH v2 06/11] drm/i915: Fix unclaimed register access due to delayed VGA memroy disable

[Paulo Zanoni]

2013/9/20  <ville.syrjala at linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> VGA registers live inside the power well on HSW, so in order to write
> the VGA MSR register we need the power well to be on.
>
> We really must write to the register to properly clear the
> VGA_MSR_MEM_EN enable bit, even if all VGA registers get zeroed when
> the power well is down. It seems that the implicit zeroing done by
> the power well is not enough to propagate the VGA_MSR_MEM_EN bit to
> whomever is actually responsible for the memory decode ranges.
>
> If we leave VGA memory decode enabled, and then turn off the power well,
> all VGA memory reads will return zeroes. But if we first disable VGA
> memory deocde and then turn off the power well, VGA memory reads
> return all ones, indicating that the access wasn't claimed by anyone.
> For the vga arbiter to function correctly the IGD must not claim the
> VGA memory accesses.
>
> Previously we were doing the VGA_MSR register access while the power well
> was excplicitly powered up during driver init. But ever since
>
>  commit 6e1b4fdad5157bb9e88777d525704aba24389bee
>  Author: Ville Syrjälä <ville.syrjala at linux.intel.com>
>  Date:   Thu Sep 5 20:40:52 2013 +0300
>
>     drm/i915: Delay disabling of VGA memory until vgacon->fbcon handoff is done
>
> we delay the VGA memory disable until fbcon has initialized, and so
> there's a possibility that the power well got turned off during the
> fbcon modeset. Also vgacon_save_screen() will need the power well to be
> on to be able to read the VGA memory.
>
> So immediately after enabling the power well during init grab a refence
> for VGA purposes, and after all the VGA handling is done, release it.
>
> v2: Add intel_display_power_put() for the num_pipes==0 case

Even though the patch was applied, the issue didn't go away. It looks
like the conflict-merging went wrong. Look:
http://cgit.freedesktop.org/~danvet/drm-intel/commit/?h=drm-intel-next-queued&id=971b0597b3a13576e1af1e0a90379aa308f498e5

The new _put function is not after i915_disable_vga_mem().


>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index e5c7b10..956d143 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1326,13 +1326,18 @@ static int i915_load_modeset_init(struct drm_device *dev)
>
>         intel_init_power_well(dev);
>
> +       /* Keep VGA alive until i915_disable_vga_mem() */
> +       intel_display_power_get(dev, POWER_DOMAIN_VGA);
> +
>         intel_modeset_gem_init(dev);
>
>         /* Always safe in the mode setting case. */
>         /* FIXME: do pre/post-mode set stuff in core KMS code */
>         dev->vblank_disable_allowed = 1;
> -       if (INTEL_INFO(dev)->num_pipes == 0)
> +       if (INTEL_INFO(dev)->num_pipes == 0) {
> +               intel_display_power_put(dev, POWER_DOMAIN_VGA);
>                 return 0;
> +       }
>
>         ret = intel_fbdev_init(dev);
>         if (ret)
> @@ -1358,6 +1363,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>          * vgacon_save_screen() works during the handover.
>          */
>         i915_disable_vga_mem(dev);
> +       intel_display_power_put(dev, POWER_DOMAIN_VGA);
>
>         /* Only enable hotplug handling once the fbdev is fully set up. */
>         dev_priv->enable_hotplug_processing = true;
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni