[PATCH v1 0/4] drm/hyperv: A fix and a few cleanups

[PATCH v1 0/4] drm/hyperv: A fix and a few cleanups

[Uwe Kleine-König (The Capable Hub)]

Hello,

while working on a tree-wide cleanup I found a few issues in the
drm/hyperv driver that are addressed here.

While the first patch is a fix, the issue is so old (from 2021, included
in v5.14-rc1) that applying the series during the next merge window is
probably the right choice.

Best regards
Uwe

Uwe Kleine-König (The Capable Hub) (4):
  drm/hyperv: Unregister pci driver in error path before module unload
  drm/hyperv: Explicitly set subvendor and subdevice for pci match array
  drm/hyperv: Drop useless empty remove callback
  drm/hyperv: Move MODULE_DEVICE_TABLE to the device_id arrays

 drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)


base-commit: be5c93fa674f0fc3c8f359c2143abce6bbb422e6
-- 
2.55.0.11.g153666a7d9bb

[PATCH v1 1/4] drm/hyperv: Unregister pci driver in error path before module unload

[Uwe Kleine-König (The Capable Hub)]

The pci driver must not kept registered if the module is unloaded after
vmbus_driver_register() fails. So check the return value of
vmbus_driver_register() and unregister the pci driver on failure.

Fixes: 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device")
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
 drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index 20f35c48c0b8..2e75fb793495 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -249,7 +249,11 @@ static int __init hv_drm_init(void)
 	if (ret != 0)
 		return ret;
 
-	return vmbus_driver_register(&hv_drm_hv_driver);
+	ret = vmbus_driver_register(&hv_drm_hv_driver);
+	if (ret)
+		pci_unregister_driver(&hv_drm_pci_driver);
+
+	return ret;
 }
 
 static void __exit hv_drm_exit(void)
-- 
2.55.0.11.g153666a7d9bb

[PATCH v1 2/4] drm/hyperv: Explicitly set subvendor and subdevice for pci match array

[Uwe Kleine-König (The Capable Hub)]

.subvendor and .subdevice were set to 0 implicitly, so only devices with
these two values set to 0 in hardware can probe automatically. Make this
requirement explicit.

While touching this array item, also make use of the pci macro designed
for that case.

Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
 drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index 2e75fb793495..e766d87b7a9d 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -51,8 +51,8 @@ static void hv_drm_pci_remove(struct pci_dev *pdev)
 
 static const struct pci_device_id hv_drm_pci_tbl[] = {
 	{
-		.vendor = PCI_VENDOR_ID_MICROSOFT,
-		.device = PCI_DEVICE_ID_HYPERV_VIDEO,
+		PCI_VDEVICE_SUB(MICROSOFT, PCI_DEVICE_ID_HYPERV_VIDEO,
+				0, 0),
 	},
 	{ /* end of list */ }
 };
-- 
2.55.0.11.g153666a7d9bb

[PATCH v1 3/4] drm/hyperv: Drop useless empty remove callback

[Uwe Kleine-König (The Capable Hub)]

Having an empty remove callback is equivalent to no remove callback.
(The only minor difference is that with an empty remove callback
pm_runtime_get_sync() and pm_runtime_put_noidle() are called.)

Drop this useless function.

Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
 drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index e766d87b7a9d..e3f41336a831 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -45,10 +45,6 @@ static int hv_drm_pci_probe(struct pci_dev *pdev,
 	return 0;
 }
 
-static void hv_drm_pci_remove(struct pci_dev *pdev)
-{
-}
-
 static const struct pci_device_id hv_drm_pci_tbl[] = {
 	{
 		PCI_VDEVICE_SUB(MICROSOFT, PCI_DEVICE_ID_HYPERV_VIDEO,
@@ -64,7 +60,6 @@ static struct pci_driver hv_drm_pci_driver = {
 	.name =		KBUILD_MODNAME,
 	.id_table =	hv_drm_pci_tbl,
 	.probe =	hv_drm_pci_probe,
-	.remove =	hv_drm_pci_remove,
 };
 
 static int hv_drm_setup_vram(struct hv_drm_device *hv,
-- 
2.55.0.11.g153666a7d9bb

[PATCH v1 4/4] drm/hyperv: Move MODULE_DEVICE_TABLE to the device_id arrays

[Uwe Kleine-König (The Capable Hub)]

It matches the usual coding style to have the MODULE_DEVICE_TABLE macro
directly after the respective arrays.

Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
 drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index e3f41336a831..6a28048f687b 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -52,6 +52,7 @@ static const struct pci_device_id hv_drm_pci_tbl[] = {
 	},
 	{ /* end of list */ }
 };
+MODULE_DEVICE_TABLE(pci, hv_drm_pci_tbl);
 
 /*
  * PCI stub to support gen1 VM.
@@ -219,6 +220,7 @@ static const struct hv_vmbus_device_id hv_drm_vmbus_tbl[] = {
 	{HV_SYNTHVID_GUID},
 	{}
 };
+MODULE_DEVICE_TABLE(vmbus, hv_drm_vmbus_tbl);
 
 static struct hv_driver hv_drm_hv_driver = {
 	.name = KBUILD_MODNAME,
@@ -260,8 +262,6 @@ static void __exit hv_drm_exit(void)
 module_init(hv_drm_init);
 module_exit(hv_drm_exit);
 
-MODULE_DEVICE_TABLE(pci, hv_drm_pci_tbl);
-MODULE_DEVICE_TABLE(vmbus, hv_drm_vmbus_tbl);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Deepak Rawat <drawat.floss@gmail.com>");
 MODULE_DESCRIPTION("DRM driver for Hyper-V synthetic video device");
-- 
2.55.0.11.g153666a7d9bb

Re: [PATCH v1 1/4] drm/hyperv: Unregister pci driver in error path before module unload

[Thomas Zimmermann]

Am 01.07.26 um 19:05 schrieb Uwe Kleine-König (The Capable Hub):
> The pci driver must not kept registered if the module is unloaded after
> vmbus_driver_register() fails. So check the return value of
> vmbus_driver_register() and unregister the pci driver on failure.
>
> Fixes: 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device")
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 20f35c48c0b8..2e75fb793495 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -249,7 +249,11 @@ static int __init hv_drm_init(void)
>   	if (ret != 0)
>   		return ret;
>   
> -	return vmbus_driver_register(&hv_drm_hv_driver);
> +	ret = vmbus_driver_register(&hv_drm_hv_driver);
> +	if (ret)
> +		pci_unregister_driver(&hv_drm_pci_driver);
> +
> +	return ret;
>   }
>   
>   static void __exit hv_drm_exit(void)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH v1 2/4] drm/hyperv: Explicitly set subvendor and subdevice for pci match array

[Thomas Zimmermann]

Hi

Am 01.07.26 um 19:05 schrieb Uwe Kleine-König (The Capable Hub):
> .subvendor and .subdevice were set to 0 implicitly, so only devices with
> these two values set to 0 in hardware can probe automatically. Make this
> requirement explicit.
>
> While touching this array item, also make use of the pci macro designed
> for that case.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
>   drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 2e75fb793495..e766d87b7a9d 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -51,8 +51,8 @@ static void hv_drm_pci_remove(struct pci_dev *pdev)
>   
>   static const struct pci_device_id hv_drm_pci_tbl[] = {
>   	{
> -		.vendor = PCI_VENDOR_ID_MICROSOFT,
> -		.device = PCI_DEVICE_ID_HYPERV_VIDEO,
> +		PCI_VDEVICE_SUB(MICROSOFT, PCI_DEVICE_ID_HYPERV_VIDEO,
> +				0, 0),

IDK, but it looks like an oversight to me.  Setting the sub-fields to 
ANY seems like the better fix.

Best regards
Thomas

>   	},
>   	{ /* end of list */ }
>   };

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH v1 3/4] drm/hyperv: Drop useless empty remove callback

[Thomas Zimmermann]

Hi

Am 01.07.26 um 19:05 schrieb Uwe Kleine-König (The Capable Hub):
> Having an empty remove callback is equivalent to no remove callback.
> (The only minor difference is that with an empty remove callback
> pm_runtime_get_sync() and pm_runtime_put_noidle() are called.)
>
> Drop this useless function.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
>   drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index e766d87b7a9d..e3f41336a831 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -45,10 +45,6 @@ static int hv_drm_pci_probe(struct pci_dev *pdev,
>   	return 0;
>   }
>   
> -static void hv_drm_pci_remove(struct pci_dev *pdev)
> -{

It would be better to call drm_dev_unplug() from here.  With a bit more 
work, the driver can have hot-unplug functionality.

Best regards
Thomas

> -}
> -
>   static const struct pci_device_id hv_drm_pci_tbl[] = {
>   	{
>   		PCI_VDEVICE_SUB(MICROSOFT, PCI_DEVICE_ID_HYPERV_VIDEO,
> @@ -64,7 +60,6 @@ static struct pci_driver hv_drm_pci_driver = {
>   	.name =		KBUILD_MODNAME,
>   	.id_table =	hv_drm_pci_tbl,
>   	.probe =	hv_drm_pci_probe,
> -	.remove =	hv_drm_pci_remove,
>   };
>   
>   static int hv_drm_setup_vram(struct hv_drm_device *hv,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH v1 4/4] drm/hyperv: Move MODULE_DEVICE_TABLE to the device_id arrays

[Thomas Zimmermann]

Am 01.07.26 um 19:05 schrieb Uwe Kleine-König (The Capable Hub):
> It matches the usual coding style to have the MODULE_DEVICE_TABLE macro
> directly after the respective arrays.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index e3f41336a831..6a28048f687b 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -52,6 +52,7 @@ static const struct pci_device_id hv_drm_pci_tbl[] = {
>   	},
>   	{ /* end of list */ }
>   };
> +MODULE_DEVICE_TABLE(pci, hv_drm_pci_tbl);
>   
>   /*
>    * PCI stub to support gen1 VM.
> @@ -219,6 +220,7 @@ static const struct hv_vmbus_device_id hv_drm_vmbus_tbl[] = {
>   	{HV_SYNTHVID_GUID},
>   	{}
>   };
> +MODULE_DEVICE_TABLE(vmbus, hv_drm_vmbus_tbl);
>   
>   static struct hv_driver hv_drm_hv_driver = {
>   	.name = KBUILD_MODNAME,
> @@ -260,8 +262,6 @@ static void __exit hv_drm_exit(void)
>   module_init(hv_drm_init);
>   module_exit(hv_drm_exit);
>   
> -MODULE_DEVICE_TABLE(pci, hv_drm_pci_tbl);
> -MODULE_DEVICE_TABLE(vmbus, hv_drm_vmbus_tbl);
>   MODULE_LICENSE("GPL");
>   MODULE_AUTHOR("Deepak Rawat <drawat.floss@gmail.com>");
>   MODULE_DESCRIPTION("DRM driver for Hyper-V synthetic video device");

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)