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;
MIME-Version: 1.0
References: <1653031240-13623-1-git-send-email-ssengar@linux.microsoft.com>
In-Reply-To: <1653031240-13623-1-git-send-email-ssengar@linux.microsoft.com>
From:   Deepak Rawat <drawat.floss@gmail.com>
Date:   Fri, 20 May 2022 09:12:04 -0700
Message-ID: <CAHFnvW01rOJq2yuFQ2u692XKz9uNfoDoYmXYShpkS+rjeZSyUg@mail.gmail.com>
Subject: Re: [PATCH] drm/hyperv : Removing the restruction of VRAM allocation
 with PCI bar size
To:     Saurabh Sengar <ssengar@linux.microsoft.com>
Cc:     ssengar@microsoft.com, David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>, linux-hyperv@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        Dexuan Cui <decui@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>