On Mon, May 29, 2023 at 11:45:04PM +0800, Sui Jingfeng wrote:
> The vga_is_firmware_default() function will work on non-x86 architectures
> as long as the arch has UEFI GOP support, which passes the firmware
> framebuffer base address and size.
>
> This patch makes the vga_is_firmware_default() function arch-independent.
> This could help the VGAARB subsystem make the right choice for multiple
> GPU systems. Usually an integrated one and a discrete one for desktop
> computers. Depending on the firmware framebuffer being put into which
> GPU's VRAM, VGAARB could inherit the firmware's choice, which in turn,
> is the exact choice of the user.
Is there a system that needs this change? If so, the commit log
should mention it.
It's definitely nice to remove #ifdefs, but it's better if we have an
actual reason and some testing of another arch that makes use of this.
Also, take a look at the git history and match the subject line and
commit log style (prefix, capitalization, imperative voice).
> Signed-off-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
> ---
> drivers/pci/vgaarb.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index 5a696078b382..f81b6c54e327 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -61,7 +61,6 @@ static bool vga_arbiter_used;
> static DEFINE_SPINLOCK(vga_lock);
> static DECLARE_WAIT_QUEUE_HEAD(vga_wait_queue);
>
> -
> static const char *vga_iostate_to_str(unsigned int iostate)
> {
> /* Ignore VGA_RSRC_IO and VGA_RSRC_MEM */
> @@ -545,7 +544,6 @@ EXPORT_SYMBOL(vga_put);
>
> static bool vga_is_firmware_default(struct pci_dev *pdev)
> {
> -#if defined(CONFIG_X86) || defined(CONFIG_IA64)
> u64 base = screen_info.lfb_base;
> u64 size = screen_info.lfb_size;
> struct resource *r;
> @@ -571,7 +569,7 @@ static bool vga_is_firmware_default(struct pci_dev *pdev)
>
> return true;
> }
> -#endif
> +
> return false;
> }
>
> @@ -865,8 +863,7 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
> }
>
> /* this is called with the lock */
> -static inline void vga_update_device_decodes(struct vga_device *vgadev,
> - int new_decodes)
> +static void vga_update_device_decodes(struct vga_device *vgadev, int new_decodes)
I don't mind removing the "inline" here, but it shouldn't be combined
with the rest of the patch. When it's combined, I can't tell whether
there's a reason we need this change or if it's just a cleanup.
> {
> struct device *dev = &vgadev->pdev->dev;
> int old_decodes, decodes_removed, decodes_unlocked;
> --
> 2.25.1
>
