diff mbox series

[v2,05/11] PCI/VGA: Move the new_state assignment out of the loop

Message ID 20230808223412.1743176-6-sui.jingfeng@linux.dev (mailing list archive)
State Changes Requested
Delegated to: Bjorn Helgaas
Headers show
Series Fix typos, comments and copyright | expand

Commit Message

Sui Jingfeng Aug. 8, 2023, 10:34 p.m. UTC
From: Sui Jingfeng <suijingfeng@loongson.cn>

In the vga_arbiter_notify_clients() function, the value of the 'new_state'
variable will be 'false' on systems that have more than one VGA device.
The value will be 'true' if there is only one VGA device or no VGA device
at all. Hence, its value is not relevant to the iteration of the loop.

So move the assignment clause out of the loop. For a system with multiple
video cards, this patch saves unnecessary assignment.

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

Comments

Ilpo Järvinen Aug. 9, 2023, 1:55 p.m. UTC | #1
On Wed, 9 Aug 2023, Sui Jingfeng wrote:

> From: Sui Jingfeng <suijingfeng@loongson.cn>
> 
> In the vga_arbiter_notify_clients() function, the value of the 'new_state'
> variable will be 'false' on systems that have more than one VGA device.
> The value will be 'true' if there is only one VGA device or no VGA device
> at all. Hence, its value is not relevant to the iteration of the loop.
> 
> So move the assignment clause out of the loop. For a system with multiple
> video cards, this patch saves unnecessary assignment.
> 
> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
> ---
>  drivers/pci/vgaarb.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index dc10a262fb5e..6883067a802a 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -1468,22 +1468,20 @@ static void vga_arbiter_notify_clients(void)
>  {
>  	struct vga_device *vgadev;
>  	unsigned long flags;
> -	uint32_t new_decodes;
> -	bool new_state;
> +	bool state;
>  
>  	if (!vga_arbiter_used)
>  		return;
>  
> +	state = (vga_count > 1) ? false : true;
> +

Is it safe to move this outside of the lock?

This would be enough (no need for ?: construct):

state = vga_count <= 1;

Or if you want to keep it as > 1:

state = !(vga_count > 1);

>  	spin_lock_irqsave(&vga_lock, flags);
>  	list_for_each_entry(vgadev, &vga_list, list) {
> -		if (vga_count > 1)
> -			new_state = false;
> -		else
> -			new_state = true;
>  		if (vgadev->set_decode) {
> -			new_decodes = vgadev->set_decode(vgadev->pdev,
> -							 new_state);
> -			vga_update_device_decodes(vgadev, new_decodes);
> +			unsigned int decodes;
> +
> +			decodes = vgadev->set_decode(vgadev->pdev, state);
> +			vga_update_device_decodes(vgadev, decodes);
>  		}
>  	}
>  	spin_unlock_irqrestore(&vga_lock, flags);
>
diff mbox series

Patch

diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index dc10a262fb5e..6883067a802a 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -1468,22 +1468,20 @@  static void vga_arbiter_notify_clients(void)
 {
 	struct vga_device *vgadev;
 	unsigned long flags;
-	uint32_t new_decodes;
-	bool new_state;
+	bool state;
 
 	if (!vga_arbiter_used)
 		return;
 
+	state = (vga_count > 1) ? false : true;
+
 	spin_lock_irqsave(&vga_lock, flags);
 	list_for_each_entry(vgadev, &vga_list, list) {
-		if (vga_count > 1)
-			new_state = false;
-		else
-			new_state = true;
 		if (vgadev->set_decode) {
-			new_decodes = vgadev->set_decode(vgadev->pdev,
-							 new_state);
-			vga_update_device_decodes(vgadev, new_decodes);
+			unsigned int decodes;
+
+			decodes = vgadev->set_decode(vgadev->pdev, state);
+			vga_update_device_decodes(vgadev, decodes);
 		}
 	}
 	spin_unlock_irqrestore(&vga_lock, flags);