Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <42658701-fa7d-1206-abae-5a1e01c060e3@loongson.cn>
Date:   Mon, 24 Jul 2023 20:28:50 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v3 4/9] PCI/VGA: Improve the default VGA device selection
Content-Language: en-US
To:     Bjorn Helgaas <helgaas@kernel.org>,
        Sui Jingfeng <sui.jingfeng@linux.dev>
Cc:     David Airlie <airlied@gmail.com>, amd-gfx@lists.freedesktop.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        intel-gfx@lists.freedesktop.org, kvm@vger.kernel.org,
        linux-pci@vger.kernel.org, linux-fbdev@vger.kernel.org,
        Alex Deucher <alexander.deucher@amd.com>,
        Christian Konig <christian.koenig@amd.com>,
        Pan Xinhui <Xinhui.Pan@amd.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        Ben Skeggs <bskeggs@redhat.com>,
        Karol Herbst <kherbst@redhat.com>,
        Lyude Paul <lyude@redhat.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Alex Williamson <alex.williamson@redhat.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Hawking Zhang <Hawking.Zhang@amd.com>,
        Mario Limonciello <mario.limonciello@amd.com>,
        Lijo Lazar <lijo.lazar@amd.com>,
        YiPeng Chai <YiPeng.Chai@amd.com>,
        Bokun Zhang <Bokun.Zhang@amd.com>,
        Likun Gao <Likun.Gao@amd.com>,
        Ville Syrjala <ville.syrjala@linux.intel.com>,
        Jason Gunthorpe <jgg@ziepe.ca>,
        Kevin Tian <kevin.tian@intel.com>,
        Cornelia Huck <cohuck@redhat.com>,
        Yishai Hadas <yishaih@nvidia.com>,
        Abhishek Sahu <abhsahu@nvidia.com>,
        Yi Liu <yi.l.liu@intel.com>,
        Jani Nikula <jani.nikula@intel.com>
References: <20230719193233.GA511659@bhelgaas>
From:   suijingfeng <suijingfeng@loongson.cn>
In-Reply-To: <20230719193233.GA511659@bhelgaas>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,


Thanks for you noticed my change.


On 2023/7/20 03:32, Bjorn Helgaas wrote:
>> @@ -1509,13 +1543,24 @@ static int pci_notify(struct notifier_block *nb, unsigned long action,
>>   	 * cases of hotplugable vga cards.
>>   	 */
>>   
>> -	if (action == BUS_NOTIFY_ADD_DEVICE)
>> +	switch (action) {
>> +	case BUS_NOTIFY_ADD_DEVICE:
>>   		notify = vga_arbiter_add_pci_device(pdev);
>> -	else if (action == BUS_NOTIFY_DEL_DEVICE)
>> +		if (notify)
>> +			vga_arbiter_notify_clients();
>> +		break;
>> +	case BUS_NOTIFY_DEL_DEVICE:
>>   		notify = vga_arbiter_del_pci_device(pdev);
>> +		if (notify)
>> +			vga_arbiter_notify_clients();
>> +		break;
>> +	case BUS_NOTIFY_BOUND_DRIVER:
>> +		vga_arbiter_do_arbitration(pdev);
>> +		break;
>> +	default:
>> +		break;
>> +	}
> Changing from if/else to switch makes the patch bigger than necessary
> for no real benefit and obscures what is really changing.

Actually, the logic become more clear after this patch applied.

```

     switch (action) {
     case BUS_NOTIFY_ADD_DEVICE:
         notify = vga_arbiter_add_pci_device(pdev);
         if (notify)
             vga_arbiter_notify_clients();
         break;
     case BUS_NOTIFY_DEL_DEVICE:
         notify = vga_arbiter_del_pci_device(pdev);
         if (notify)
             vga_arbiter_notify_clients();
         break;
     case BUS_NOTIFY_BOUND_DRIVER:
         vga_arbiter_do_arbitration(pdev);
         break;
     default:
         break;
     }

```


Because we only need call vga_arbiter_notify_clients() when action == 
BUS_NOTIFY_ADD_DEVICE or action == BUS_NOTIFY_DEL_DEVICE,

But *NOT* when the action equals to  BUS_NOTIFY_BOUND_DRIVER.