All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/hyperv : Removing the restruction of VRAM allocation with PCI bar size
@ 2022-05-20  7:20 Saurabh Sengar
  2022-05-20 16:04   ` Wei Liu
  2022-05-20 16:12   ` Deepak Rawat
  0 siblings, 2 replies; 5+ messages in thread
From: Saurabh Sengar @ 2022-05-20  7:20 UTC (permalink / raw)
  To: ssengar, drawat.floss, airlied, daniel, linux-hyperv, dri-devel,
	linux-kernel, decui, haiyangz

There were two different approaches getting used in this driver to
allocate vram:
	1. VRAM allocation from PCI region for Gen1
	2. VRAM alloaction from MMIO region for Gen2
First approach limilts the vram to PCI BAR size, which is 64 MB in most
legacy systems. This limits the maximum resolution to be restricted to
64 MB size, and with recent conclusion on fbdev issue its concluded to have
similar allocation strategy for both Gen1 and Gen2. This patch unifies
the Gen1 and Gen2 vram allocation strategy.

Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
FBdev patch Ref :
https://lore.kernel.org/lkml/20220428143746.sya775ro5zi3kgm3@liuwe-devbox-debian-v2/T/

 drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 76 ++-----------------------
 1 file changed, 4 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index 4a8941fa0815..a32afd84f361 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -69,57 +69,8 @@ static struct pci_driver hyperv_pci_driver = {
 	.remove =	hyperv_pci_remove,
 };
 
-static int hyperv_setup_gen1(struct hyperv_drm_device *hv)
-{
-	struct drm_device *dev = &hv->dev;
-	struct pci_dev *pdev;
-	int ret;
-
-	pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
-			      PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
-	if (!pdev) {
-		drm_err(dev, "Unable to find PCI Hyper-V video\n");
-		return -ENODEV;
-	}
-
-	ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &hyperv_driver);
-	if (ret) {
-		drm_err(dev, "Not able to remove boot fb\n");
-		return ret;
-	}
-
-	if (pci_request_region(pdev, 0, DRIVER_NAME) != 0)
-		drm_warn(dev, "Cannot request framebuffer, boot fb still active?\n");
-
-	if ((pdev->resource[0].flags & IORESOURCE_MEM) == 0) {
-		drm_err(dev, "Resource at bar 0 is not IORESOURCE_MEM\n");
-		ret = -ENODEV;
-		goto error;
-	}
-
-	hv->fb_base = pci_resource_start(pdev, 0);
-	hv->fb_size = pci_resource_len(pdev, 0);
-	if (!hv->fb_base) {
-		drm_err(dev, "Resource not available\n");
-		ret = -ENODEV;
-		goto error;
-	}
-
-	hv->fb_size = min(hv->fb_size,
-			  (unsigned long)(hv->mmio_megabytes * 1024 * 1024));
-	hv->vram = devm_ioremap(&pdev->dev, hv->fb_base, hv->fb_size);
-	if (!hv->vram) {
-		drm_err(dev, "Failed to map vram\n");
-		ret = -ENOMEM;
-	}
-
-error:
-	pci_dev_put(pdev);
-	return ret;
-}
-
-static int hyperv_setup_gen2(struct hyperv_drm_device *hv,
-			     struct hv_device *hdev)
+static int hyperv_setup_gen(struct hyperv_drm_device *hv,
+			    struct hv_device *hdev)
 {
 	struct drm_device *dev = &hv->dev;
 	int ret;
@@ -181,10 +132,7 @@ static int hyperv_vmbus_probe(struct hv_device *hdev,
 		goto err_hv_set_drv_data;
 	}
 
-	if (efi_enabled(EFI_BOOT))
-		ret = hyperv_setup_gen2(hv, hdev);
-	else
-		ret = hyperv_setup_gen1(hv);
+	ret = hyperv_setup_gen(hv, hdev);
 
 	if (ret)
 		goto err_vmbus_close;
@@ -225,29 +173,13 @@ static int hyperv_vmbus_remove(struct hv_device *hdev)
 {
 	struct drm_device *dev = hv_get_drvdata(hdev);
 	struct hyperv_drm_device *hv = to_hv(dev);
-	struct pci_dev *pdev;
 
 	drm_dev_unplug(dev);
 	drm_atomic_helper_shutdown(dev);
 	vmbus_close(hdev->channel);
 	hv_set_drvdata(hdev, NULL);
 
-	/*
-	 * Free allocated MMIO memory only on Gen2 VMs.
-	 * On Gen1 VMs, release the PCI device
-	 */
-	if (efi_enabled(EFI_BOOT)) {
-		vmbus_free_mmio(hv->mem->start, hv->fb_size);
-	} else {
-		pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
-				      PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
-		if (!pdev) {
-			drm_err(dev, "Unable to find PCI Hyper-V video\n");
-			return -ENODEV;
-		}
-		pci_release_region(pdev, 0);
-		pci_dev_put(pdev);
-	}
+	vmbus_free_mmio(hv->mem->start, hv->fb_size);
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/hyperv : Removing the restruction of VRAM allocation with PCI bar size
  2022-05-20  7:20 [PATCH] drm/hyperv : Removing the restruction of VRAM allocation with PCI bar size Saurabh Sengar
@ 2022-05-20 16:04   ` Wei Liu
  2022-05-20 16:12   ` Deepak Rawat
  1 sibling, 0 replies; 5+ messages in thread
From: Wei Liu @ 2022-05-20 16:04 UTC (permalink / raw)
  To: Saurabh Sengar
  Cc: ssengar, drawat.floss, airlied, daniel, linux-hyperv, dri-devel,
	linux-kernel, decui, haiyangz, Wei Liu

On Fri, May 20, 2022 at 12:20:40AM -0700, Saurabh Sengar wrote:
> There were two different approaches getting used in this driver to
> allocate vram:
> 	1. VRAM allocation from PCI region for Gen1
> 	2. VRAM alloaction from MMIO region for Gen2
> First approach limilts the vram to PCI BAR size, which is 64 MB in most
> legacy systems. This limits the maximum resolution to be restricted to
> 64 MB size, and with recent conclusion on fbdev issue its concluded to have
> similar allocation strategy for both Gen1 and Gen2. This patch unifies
> the Gen1 and Gen2 vram allocation strategy.
> 
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
> FBdev patch Ref :
> https://lore.kernel.org/lkml/20220428143746.sya775ro5zi3kgm3@liuwe-devbox-debian-v2/T/
> 
[...]
> -static int hyperv_setup_gen2(struct hyperv_drm_device *hv,
> -			     struct hv_device *hdev)
> +static int hyperv_setup_gen(struct hyperv_drm_device *hv,
> +			    struct hv_device *hdev)

There is no need to have "_gen" suffix anymore.

I don't know much about the rest so cannot really make any meaningful
comment here.

Thanks,
Wei.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/hyperv : Removing the restruction of VRAM allocation with PCI bar size
@ 2022-05-20 16:04   ` Wei Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Wei Liu @ 2022-05-20 16:04 UTC (permalink / raw)
  To: Saurabh Sengar
  Cc: linux-hyperv, airlied, haiyangz, ssengar, decui, linux-kernel,
	dri-devel, drawat.floss, Wei Liu

On Fri, May 20, 2022 at 12:20:40AM -0700, Saurabh Sengar wrote:
> There were two different approaches getting used in this driver to
> allocate vram:
> 	1. VRAM allocation from PCI region for Gen1
> 	2. VRAM alloaction from MMIO region for Gen2
> First approach limilts the vram to PCI BAR size, which is 64 MB in most
> legacy systems. This limits the maximum resolution to be restricted to
> 64 MB size, and with recent conclusion on fbdev issue its concluded to have
> similar allocation strategy for both Gen1 and Gen2. This patch unifies
> the Gen1 and Gen2 vram allocation strategy.
> 
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
> FBdev patch Ref :
> https://lore.kernel.org/lkml/20220428143746.sya775ro5zi3kgm3@liuwe-devbox-debian-v2/T/
> 
[...]
> -static int hyperv_setup_gen2(struct hyperv_drm_device *hv,
> -			     struct hv_device *hdev)
> +static int hyperv_setup_gen(struct hyperv_drm_device *hv,
> +			    struct hv_device *hdev)

There is no need to have "_gen" suffix anymore.

I don't know much about the rest so cannot really make any meaningful
comment here.

Thanks,
Wei.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/hyperv : Removing the restruction of VRAM allocation with PCI bar size
  2022-05-20  7:20 [PATCH] drm/hyperv : Removing the restruction of VRAM allocation with PCI bar size Saurabh Sengar
@ 2022-05-20 16:12   ` Deepak Rawat
  2022-05-20 16:12   ` Deepak Rawat
  1 sibling, 0 replies; 5+ messages in thread
From: Deepak Rawat @ 2022-05-20 16:12 UTC (permalink / raw)
  To: Saurabh Sengar
  Cc: ssengar, David Airlie, Daniel Vetter, linux-hyperv, dri-devel,
	linux-kernel, Dexuan Cui, Haiyang Zhang

On Fri, May 20, 2022 at 12:20 AM Saurabh Sengar
<ssengar@linux.microsoft.com> wrote:
>
> There were two different approaches getting used in this driver to
> allocate vram:
>         1. VRAM allocation from PCI region for Gen1
>         2. VRAM alloaction from MMIO region for Gen2
> First approach limilts the vram to PCI BAR size, which is 64 MB in most
> legacy systems. This limits the maximum resolution to be restricted to
> 64 MB size, and with recent conclusion on fbdev issue its concluded to have
> similar allocation strategy for both Gen1 and Gen2. This patch unifies
> the Gen1 and Gen2 vram allocation strategy.
>
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
> FBdev patch Ref :
> https://lore.kernel.org/lkml/20220428143746.sya775ro5zi3kgm3@liuwe-devbox-debian-v2/T/
>
>  drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 76 ++-----------------------
>  1 file changed, 4 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 4a8941fa0815..a32afd84f361 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -69,57 +69,8 @@ static struct pci_driver hyperv_pci_driver = {
>         .remove =       hyperv_pci_remove,
>  };
>
> -static int hyperv_setup_gen1(struct hyperv_drm_device *hv)
> -{
> -       struct drm_device *dev = &hv->dev;
> -       struct pci_dev *pdev;
> -       int ret;
> -
> -       pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> -                             PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> -       if (!pdev) {
> -               drm_err(dev, "Unable to find PCI Hyper-V video\n");
> -               return -ENODEV;
> -       }
> -
> -       ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &hyperv_driver);
> -       if (ret) {
> -               drm_err(dev, "Not able to remove boot fb\n");
> -               return ret;
> -       }
> -
> -       if (pci_request_region(pdev, 0, DRIVER_NAME) != 0)
> -               drm_warn(dev, "Cannot request framebuffer, boot fb still active?\n");
> -
> -       if ((pdev->resource[0].flags & IORESOURCE_MEM) == 0) {
> -               drm_err(dev, "Resource at bar 0 is not IORESOURCE_MEM\n");
> -               ret = -ENODEV;
> -               goto error;
> -       }
> -
> -       hv->fb_base = pci_resource_start(pdev, 0);
> -       hv->fb_size = pci_resource_len(pdev, 0);
> -       if (!hv->fb_base) {
> -               drm_err(dev, "Resource not available\n");
> -               ret = -ENODEV;
> -               goto error;
> -       }
> -
> -       hv->fb_size = min(hv->fb_size,
> -                         (unsigned long)(hv->mmio_megabytes * 1024 * 1024));
> -       hv->vram = devm_ioremap(&pdev->dev, hv->fb_base, hv->fb_size);
> -       if (!hv->vram) {
> -               drm_err(dev, "Failed to map vram\n");
> -               ret = -ENOMEM;
> -       }
> -
> -error:
> -       pci_dev_put(pdev);
> -       return ret;
> -}
> -
> -static int hyperv_setup_gen2(struct hyperv_drm_device *hv,
> -                            struct hv_device *hdev)
> +static int hyperv_setup_gen(struct hyperv_drm_device *hv,
> +                           struct hv_device *hdev)
>  {

nit: This can be renamed to hyperv_setup_vram instead.

Reviewed-by: Deepak Rawat <drawat.floss@gmail.com>

>         struct drm_device *dev = &hv->dev;
>         int ret;
> @@ -181,10 +132,7 @@ static int hyperv_vmbus_probe(struct hv_device *hdev,
>                 goto err_hv_set_drv_data;
>         }
>
> -       if (efi_enabled(EFI_BOOT))
> -               ret = hyperv_setup_gen2(hv, hdev);
> -       else
> -               ret = hyperv_setup_gen1(hv);
> +       ret = hyperv_setup_gen(hv, hdev);
>
>         if (ret)
>                 goto err_vmbus_close;
> @@ -225,29 +173,13 @@ static int hyperv_vmbus_remove(struct hv_device *hdev)
>  {
>         struct drm_device *dev = hv_get_drvdata(hdev);
>         struct hyperv_drm_device *hv = to_hv(dev);
> -       struct pci_dev *pdev;
>
>         drm_dev_unplug(dev);
>         drm_atomic_helper_shutdown(dev);
>         vmbus_close(hdev->channel);
>         hv_set_drvdata(hdev, NULL);
>
> -       /*
> -        * Free allocated MMIO memory only on Gen2 VMs.
> -        * On Gen1 VMs, release the PCI device
> -        */
> -       if (efi_enabled(EFI_BOOT)) {
> -               vmbus_free_mmio(hv->mem->start, hv->fb_size);
> -       } else {
> -               pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> -                                     PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> -               if (!pdev) {
> -                       drm_err(dev, "Unable to find PCI Hyper-V video\n");
> -                       return -ENODEV;
> -               }
> -               pci_release_region(pdev, 0);
> -               pci_dev_put(pdev);
> -       }
> +       vmbus_free_mmio(hv->mem->start, hv->fb_size);
>
>         return 0;
>  }
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/hyperv : Removing the restruction of VRAM allocation with PCI bar size
@ 2022-05-20 16:12   ` Deepak Rawat
  0 siblings, 0 replies; 5+ messages in thread
From: Deepak Rawat @ 2022-05-20 16:12 UTC (permalink / raw)
  To: Saurabh Sengar
  Cc: linux-hyperv, David Airlie, Haiyang Zhang, ssengar, Dexuan Cui,
	linux-kernel, dri-devel

On Fri, May 20, 2022 at 12:20 AM Saurabh Sengar
<ssengar@linux.microsoft.com> wrote:
>
> There were two different approaches getting used in this driver to
> allocate vram:
>         1. VRAM allocation from PCI region for Gen1
>         2. VRAM alloaction from MMIO region for Gen2
> First approach limilts the vram to PCI BAR size, which is 64 MB in most
> legacy systems. This limits the maximum resolution to be restricted to
> 64 MB size, and with recent conclusion on fbdev issue its concluded to have
> similar allocation strategy for both Gen1 and Gen2. This patch unifies
> the Gen1 and Gen2 vram allocation strategy.
>
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
> FBdev patch Ref :
> https://lore.kernel.org/lkml/20220428143746.sya775ro5zi3kgm3@liuwe-devbox-debian-v2/T/
>
>  drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 76 ++-----------------------
>  1 file changed, 4 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 4a8941fa0815..a32afd84f361 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -69,57 +69,8 @@ static struct pci_driver hyperv_pci_driver = {
>         .remove =       hyperv_pci_remove,
>  };
>
> -static int hyperv_setup_gen1(struct hyperv_drm_device *hv)
> -{
> -       struct drm_device *dev = &hv->dev;
> -       struct pci_dev *pdev;
> -       int ret;
> -
> -       pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> -                             PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> -       if (!pdev) {
> -               drm_err(dev, "Unable to find PCI Hyper-V video\n");
> -               return -ENODEV;
> -       }
> -
> -       ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &hyperv_driver);
> -       if (ret) {
> -               drm_err(dev, "Not able to remove boot fb\n");
> -               return ret;
> -       }
> -
> -       if (pci_request_region(pdev, 0, DRIVER_NAME) != 0)
> -               drm_warn(dev, "Cannot request framebuffer, boot fb still active?\n");
> -
> -       if ((pdev->resource[0].flags & IORESOURCE_MEM) == 0) {
> -               drm_err(dev, "Resource at bar 0 is not IORESOURCE_MEM\n");
> -               ret = -ENODEV;
> -               goto error;
> -       }
> -
> -       hv->fb_base = pci_resource_start(pdev, 0);
> -       hv->fb_size = pci_resource_len(pdev, 0);
> -       if (!hv->fb_base) {
> -               drm_err(dev, "Resource not available\n");
> -               ret = -ENODEV;
> -               goto error;
> -       }
> -
> -       hv->fb_size = min(hv->fb_size,
> -                         (unsigned long)(hv->mmio_megabytes * 1024 * 1024));
> -       hv->vram = devm_ioremap(&pdev->dev, hv->fb_base, hv->fb_size);
> -       if (!hv->vram) {
> -               drm_err(dev, "Failed to map vram\n");
> -               ret = -ENOMEM;
> -       }
> -
> -error:
> -       pci_dev_put(pdev);
> -       return ret;
> -}
> -
> -static int hyperv_setup_gen2(struct hyperv_drm_device *hv,
> -                            struct hv_device *hdev)
> +static int hyperv_setup_gen(struct hyperv_drm_device *hv,
> +                           struct hv_device *hdev)
>  {

nit: This can be renamed to hyperv_setup_vram instead.

Reviewed-by: Deepak Rawat <drawat.floss@gmail.com>

>         struct drm_device *dev = &hv->dev;
>         int ret;
> @@ -181,10 +132,7 @@ static int hyperv_vmbus_probe(struct hv_device *hdev,
>                 goto err_hv_set_drv_data;
>         }
>
> -       if (efi_enabled(EFI_BOOT))
> -               ret = hyperv_setup_gen2(hv, hdev);
> -       else
> -               ret = hyperv_setup_gen1(hv);
> +       ret = hyperv_setup_gen(hv, hdev);
>
>         if (ret)
>                 goto err_vmbus_close;
> @@ -225,29 +173,13 @@ static int hyperv_vmbus_remove(struct hv_device *hdev)
>  {
>         struct drm_device *dev = hv_get_drvdata(hdev);
>         struct hyperv_drm_device *hv = to_hv(dev);
> -       struct pci_dev *pdev;
>
>         drm_dev_unplug(dev);
>         drm_atomic_helper_shutdown(dev);
>         vmbus_close(hdev->channel);
>         hv_set_drvdata(hdev, NULL);
>
> -       /*
> -        * Free allocated MMIO memory only on Gen2 VMs.
> -        * On Gen1 VMs, release the PCI device
> -        */
> -       if (efi_enabled(EFI_BOOT)) {
> -               vmbus_free_mmio(hv->mem->start, hv->fb_size);
> -       } else {
> -               pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> -                                     PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> -               if (!pdev) {
> -                       drm_err(dev, "Unable to find PCI Hyper-V video\n");
> -                       return -ENODEV;
> -               }
> -               pci_release_region(pdev, 0);
> -               pci_dev_put(pdev);
> -       }
> +       vmbus_free_mmio(hv->mem->start, hv->fb_size);
>
>         return 0;
>  }
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-05-20 16:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-20  7:20 [PATCH] drm/hyperv : Removing the restruction of VRAM allocation with PCI bar size Saurabh Sengar
2022-05-20 16:04 ` Wei Liu
2022-05-20 16:04   ` Wei Liu
2022-05-20 16:12 ` Deepak Rawat
2022-05-20 16:12   ` Deepak Rawat

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.