[PATCH v2 04/11] PCI/VGA: Drop the inline in the vga_update_device_decodes() function.

Sui Jingfeng posted 11 patches 2 years, 6 months ago
[PATCH v2 04/11] PCI/VGA: Drop the inline in the vga_update_device_decodes() function.
Posted by Sui Jingfeng 2 years, 6 months ago
From: Sui Jingfeng <suijingfeng@loongson.cn>

The vga_update_device_decodes() function is not performance-critical.
So drop the inline. This patch also makes the parameter consistent with
the argument, using the 'unsigned int' type instead of the 'signed' type
to store the decode.

Change the second argument of the vga_update_device_decodes() function
to 'unsigned int' type.

Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
---
 drivers/pci/vgaarb.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 8742a51d450f..dc10a262fb5e 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -860,24 +860,24 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
 	return ret;
 }
 
-/* this is called with the lock */
-static inline void vga_update_device_decodes(struct vga_device *vgadev,
-					     int new_decodes)
+/* This is called with the lock */
+static void vga_update_device_decodes(struct vga_device *vgadev,
+				      unsigned int new_decodes)
 {
 	struct device *dev = &vgadev->pdev->dev;
-	int old_decodes, decodes_removed, decodes_unlocked;
+	unsigned int old_decodes = vgadev->decodes;
+	unsigned int decodes_removed = ~new_decodes & old_decodes;
+	unsigned int decodes_unlocked = vgadev->locks & decodes_removed;
 
-	old_decodes = vgadev->decodes;
-	decodes_removed = ~new_decodes & old_decodes;
-	decodes_unlocked = vgadev->locks & decodes_removed;
 	vgadev->decodes = new_decodes;
 
-	vgaarb_info(dev, "changed VGA decodes: olddecodes=%s,decodes=%s:owns=%s\n",
-		vga_iostate_to_str(old_decodes),
-		vga_iostate_to_str(vgadev->decodes),
-		vga_iostate_to_str(vgadev->owns));
+	vgaarb_info(dev,
+		    "VGA decodes changed: olddecodes=%s,decodes=%s:owns=%s\n",
+		    vga_iostate_to_str(old_decodes),
+		    vga_iostate_to_str(vgadev->decodes),
+		    vga_iostate_to_str(vgadev->owns));
 
-	/* if we removed locked decodes, lock count goes to zero, and release */
+	/* If we removed locked decodes, lock count goes to zero, and release */
 	if (decodes_unlocked) {
 		if (decodes_unlocked & VGA_RSRC_LEGACY_IO)
 			vgadev->io_lock_cnt = 0;
-- 
2.34.1
Re: [PATCH v2 04/11] PCI/VGA: Drop the inline in the vga_update_device_decodes() function.
Posted by Ilpo Järvinen 2 years, 6 months ago
On Wed, 9 Aug 2023, Sui Jingfeng wrote:

> From: Sui Jingfeng <suijingfeng@loongson.cn>
> 
> The vga_update_device_decodes() function is not performance-critical.
> So drop the inline. This patch also makes the parameter consistent with
> the argument, using the 'unsigned int' type instead of the 'signed' type
> to store the decode.

Use imperative form, Don't start with "This patch" but directly what 
follows after those words.
 
> Change the second argument of the vga_update_device_decodes() function
> to 'unsigned int' type.

Somehow it feels all these 3 changes should be in separate patches, I 
don't see how they're related at all with each other other than they touch 
roughly the same area.

-- 
 i.

> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
> ---
>  drivers/pci/vgaarb.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index 8742a51d450f..dc10a262fb5e 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -860,24 +860,24 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
>  	return ret;
>  }
>  
> -/* this is called with the lock */
> -static inline void vga_update_device_decodes(struct vga_device *vgadev,
> -					     int new_decodes)
> +/* This is called with the lock */
> +static void vga_update_device_decodes(struct vga_device *vgadev,
> +				      unsigned int new_decodes)
>  {
>  	struct device *dev = &vgadev->pdev->dev;
> -	int old_decodes, decodes_removed, decodes_unlocked;
> +	unsigned int old_decodes = vgadev->decodes;
> +	unsigned int decodes_removed = ~new_decodes & old_decodes;
> +	unsigned int decodes_unlocked = vgadev->locks & decodes_removed;
>  
> -	old_decodes = vgadev->decodes;
> -	decodes_removed = ~new_decodes & old_decodes;
> -	decodes_unlocked = vgadev->locks & decodes_removed;
>  	vgadev->decodes = new_decodes;
>  
> -	vgaarb_info(dev, "changed VGA decodes: olddecodes=%s,decodes=%s:owns=%s\n",
> -		vga_iostate_to_str(old_decodes),
> -		vga_iostate_to_str(vgadev->decodes),
> -		vga_iostate_to_str(vgadev->owns));
> +	vgaarb_info(dev,
> +		    "VGA decodes changed: olddecodes=%s,decodes=%s:owns=%s\n",
> +		    vga_iostate_to_str(old_decodes),
> +		    vga_iostate_to_str(vgadev->decodes),
> +		    vga_iostate_to_str(vgadev->owns));
>  
> -	/* if we removed locked decodes, lock count goes to zero, and release */
> +	/* If we removed locked decodes, lock count goes to zero, and release */
>  	if (decodes_unlocked) {
>  		if (decodes_unlocked & VGA_RSRC_LEGACY_IO)
>  			vgadev->io_lock_cnt = 0;
>