Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 22 Aug 2019 15:08:29 +0200
From:   Daniel Vetter <daniel@ffwll.ch>
To:     Gerd Hoffmann <kraxel@redhat.com>
Cc:     dri-devel@lists.freedesktop.org,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        "open list:FRAMEBUFFER LAYER" <linux-fbdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/3] fbdev: drop res_id parameter from
 remove_conflicting_pci_framebuffers
Message-ID: <20190822130829.GV11147@phenom.ffwll.local>
Mail-Followup-To: Gerd Hoffmann <kraxel@redhat.com>,
        dri-devel@lists.freedesktop.org,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
        "open list:FRAMEBUFFER LAYER" <linux-fbdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
References: <20190822090645.25410-1-kraxel@redhat.com>
 <20190822090645.25410-2-kraxel@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190822090645.25410-2-kraxel@redhat.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Aug 22, 2019 at 11:06:43AM +0200, Gerd Hoffmann wrote:
> Since commit b0e999c95581 ("fbdev: list all pci memory bars as
> conflicting apertures") the parameter was used for some sanity checks
> only, to make sure we detect any issues with the new approach to just
> list all memory bars as apertures.
> 
> No issues turned up so far, so continue to cleanup:  Drop the res_id
> parameter, drop the sanity checks.  Also downgrade the logging from
> "info" level to "debug" level and update documentation.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  include/drm/drm_fb_helper.h      |  2 +-
>  include/linux/fb.h               |  2 +-
>  drivers/video/fbdev/core/fbmem.c | 17 +++++------------
>  3 files changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index c8a8ae2a678a..5a5f4b1d8241 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -560,7 +560,7 @@ drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
>  	 * otherwise the vga fbdev driver falls over.
>  	 */
>  #if IS_REACHABLE(CONFIG_FB)
> -	ret = remove_conflicting_pci_framebuffers(pdev, resource_id, name);
> +	ret = remove_conflicting_pci_framebuffers(pdev, name);
>  #endif
>  	if (ret == 0)
>  		ret = vga_remove_vgacon(pdev);
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 756706b666a1..41e0069eca0a 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -607,7 +607,7 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
>  extern int register_framebuffer(struct fb_info *fb_info);
>  extern void unregister_framebuffer(struct fb_info *fb_info);
>  extern void unlink_framebuffer(struct fb_info *fb_info);
> -extern int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, int res_id,
> +extern int remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
>  					       const char *name);
>  extern int remove_conflicting_framebuffers(struct apertures_struct *a,
>  					   const char *name, bool primary);
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index e6a1c805064f..95c32952fa8a 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1758,21 +1758,19 @@ EXPORT_SYMBOL(remove_conflicting_framebuffers);
>  /**
>   * remove_conflicting_pci_framebuffers - remove firmware-configured framebuffers for PCI devices
>   * @pdev: PCI device
> - * @res_id: index of PCI BAR configuring framebuffer memory
>   * @name: requesting driver name
>   *
>   * This function removes framebuffer devices (eg. initialized by firmware)
> - * using memory range configured for @pdev's BAR @res_id.
> + * using memory range configured for any of @pdev's memory bars.
>   *
>   * The function assumes that PCI device with shadowed ROM drives a primary
>   * display and so kicks out vga16fb.
>   */
> -int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, int res_id, const char *name)
> +int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, const char *name)
>  {
>  	struct apertures_struct *ap;
>  	bool primary = false;
>  	int err, idx, bar;
> -	bool res_id_found = false;
>  
>  	for (idx = 0, bar = 0; bar < PCI_ROM_RESOURCE; bar++) {
>  		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
> @@ -1789,16 +1787,11 @@ int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, int res_id, const
>  			continue;
>  		ap->ranges[idx].base = pci_resource_start(pdev, bar);
>  		ap->ranges[idx].size = pci_resource_len(pdev, bar);
> -		pci_info(pdev, "%s: bar %d: 0x%lx -> 0x%lx\n", __func__, bar,
> -			 (unsigned long)pci_resource_start(pdev, bar),
> -			 (unsigned long)pci_resource_end(pdev, bar));
> +		pci_dbg(pdev, "%s: bar %d: 0x%lx -> 0x%lx\n", __func__, bar,
> +			(unsigned long)pci_resource_start(pdev, bar),
> +			(unsigned long)pci_resource_end(pdev, bar));
>  		idx++;
> -		if (res_id == bar)
> -			res_id_found = true;
>  	}
> -	if (!res_id_found)
> -		pci_warn(pdev, "%s: passed res_id (%d) is not a memory bar\n",
> -			 __func__, res_id);
>  
>  #ifdef CONFIG_X86
>  	primary = pdev->resource[PCI_ROM_RESOURCE].flags &
> -- 
> 2.18.1
> 

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