Re: [PATCH v7 4/4] drm/i915: switch to drm_fb_helper_remove_conflicting_pci_framebuffers

From: Daniel Vetter <daniel@ffwll.ch>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	daniel@ffwll.ch, Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	David Airlie <airlied@linux.ie>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v7 4/4] drm/i915: switch to drm_fb_helper_remove_conflicting_pci_framebuffers
Date: Mon, 11 Mar 2019 13:43:05 +0100	[thread overview]
Message-ID: <20190311124305.GB2665@phenom.ffwll.local> (raw)
In-Reply-To: <20190301092502.30948-5-kraxel@redhat.com>

On Fri, Mar 01, 2019 at 10:25:02AM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 38 ++------------------------------------
>  1 file changed, 2 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9df65d386d11..c6f1e0dbdd58 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -732,31 +732,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  	return ret;
>  }
>  
> -static int i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
> -{
> -	struct apertures_struct *ap;
> -	struct pci_dev *pdev = dev_priv->drm.pdev;
> -	struct i915_ggtt *ggtt = &dev_priv->ggtt;
> -	bool primary;
> -	int ret;
> -
> -	ap = alloc_apertures(1);
> -	if (!ap)
> -		return -ENOMEM;
> -
> -	ap->ranges[0].base = ggtt->gmadr.start;
> -	ap->ranges[0].size = ggtt->mappable_end;
> -
> -	primary =
> -		pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> -
> -	ret = drm_fb_helper_remove_conflicting_framebuffers(ap, "inteldrmfb", primary);
> -
> -	kfree(ap);
> -
> -	return ret;
> -}
> -
>  static void intel_init_dpio(struct drm_i915_private *dev_priv)
>  {
>  	/*
> @@ -1377,22 +1352,13 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  	if (ret)
>  		goto err_perf;
>  
> -	/*
> -	 * WARNING: Apparently we must kick fbdev drivers before vgacon,
> -	 * otherwise the vga fbdev driver falls over.
> -	 */
> -	ret = i915_kick_out_firmware_fb(dev_priv);
> +	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 2,
> +								"inteldrmfb");

Ok I reviewed this more carefully, it moves around:

- gen2: bar 0
- gen3-5: bar 1
- gen6+: bar 2

I think the 100% future proof version would be to change the helper in the
fb core to just have apertures mapping all bars of the passed-in pci
device, and remove the bar number. Then we could apply this patch without
having to duplicate the knowledge about which pci bar the fb can sit in.

If you don't feel like typing this (since quite a bit of scope creep with
my comments already ...) a patch to add it to todo.rst would be great.

Thanks, Daniel

>  	if (ret) {
>  		DRM_ERROR("failed to remove conflicting framebuffer drivers\n");
>  		goto err_ggtt;
>  	}
>  
> -	ret = vga_remove_vgacon(pdev);
> -	if (ret) {
> -		DRM_ERROR("failed to remove conflicting VGA console\n");
> -		goto err_ggtt;
> -	}
> -
>  	ret = i915_ggtt_init_hw(dev_priv);
>  	if (ret)
>  		goto err_ggtt;
> -- 
> 2.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch