| Message ID | 20231030095024.9808-2-pstanner@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ast: use managed devres functions | expand |
On 30/10/2023 10:50, Philipp Stanner wrote: > Currently, tha ast-driver just maps the PCI-dev's regions with > pcim_iomap(). It does not actually reserve the regions exclusively > with, e.g., pci_request_regions(). > > Replace the calls to pcim_iomap() with ones to pcim_iomap_regions() to > reserve and map the regions simultaneously. > > Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > --- > ¡Hola! > I picked up the memory-region-request-task from the DRM-TODO-List [1] > and began with this driver. > > Please have a first look. I wasn't entirely sure about -ENOMEM... for > example, as far as my understanding goes, it should not be able to fail > anyways in the second call. Yes, you can remove these checks, other drivers don't do it: https://elixir.bootlin.com/linux/latest/source/arch/x86/platform/intel-mid/pwr.c#L372 > > I don't have the server-hardware, thus, can't test it on a physical > machine. I've done a quick check on an AST2600, and it works. > > Please tell me what you think. That's a good patch, thanks for your contribution. I'll wait for Thomas review, and with the checks removed, I can help push it to drm-misc-next. > > P. > > [1] https://dri.freedesktop.org/docs/drm/gpu/todo.html#request-memory-regions-in-all-drivers > --- > drivers/gpu/drm/ast/ast_main.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c > index dae365ed3969..1004c6628938 100644 > --- a/drivers/gpu/drm/ast/ast_main.c > +++ b/drivers/gpu/drm/ast/ast_main.c > @@ -444,9 +444,13 @@ struct ast_device *ast_device_create(const struct drm_driver *drv, > if (ret) > return ERR_PTR(ret); > > - ast->regs = pcim_iomap(pdev, 1, 0); > + ret = pcim_iomap_regions(pdev, BIT(1), 0); > + if (ret) > + return ERR_PTR(ret); > + > + ast->regs = pcim_iomap_table(pdev)[1]; > if (!ast->regs) > - return ERR_PTR(-EIO); > + return ERR_PTR(-ENOMEM); You can remove this check. > > /* > * After AST2500, MMIO is enabled by default, and it should be adopted > @@ -461,9 +465,12 @@ struct ast_device *ast_device_create(const struct drm_driver *drv, > > /* "map" IO regs if the above hasn't done so already */ > if (!ast->ioregs) { > - ast->ioregs = pcim_iomap(pdev, 2, 0); > + ret = pcim_iomap_regions(pdev, BIT(2), 0); > + if (ret) > + return ERR_PTR(ret); > + ast->ioregs = pcim_iomap_table(pdev)[2]; > if (!ast->ioregs) > - return ERR_PTR(-EIO); > + return ERR_PTR(-ENOMEM); you can remove this check too. > } > > if (!ast_is_vga_enabled(dev)) {
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index dae365ed3969..1004c6628938 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -444,9 +444,13 @@ struct ast_device *ast_device_create(const struct drm_driver *drv, if (ret) return ERR_PTR(ret); - ast->regs = pcim_iomap(pdev, 1, 0); + ret = pcim_iomap_regions(pdev, BIT(1), 0); + if (ret) + return ERR_PTR(ret); + + ast->regs = pcim_iomap_table(pdev)[1]; if (!ast->regs) - return ERR_PTR(-EIO); + return ERR_PTR(-ENOMEM); /* * After AST2500, MMIO is enabled by default, and it should be adopted @@ -461,9 +465,12 @@ struct ast_device *ast_device_create(const struct drm_driver *drv, /* "map" IO regs if the above hasn't done so already */ if (!ast->ioregs) { - ast->ioregs = pcim_iomap(pdev, 2, 0); + ret = pcim_iomap_regions(pdev, BIT(2), 0); + if (ret) + return ERR_PTR(ret); + ast->ioregs = pcim_iomap_table(pdev)[2]; if (!ast->ioregs) - return ERR_PTR(-EIO); + return ERR_PTR(-ENOMEM); } if (!ast_is_vga_enabled(dev)) {
Currently, tha ast-driver just maps the PCI-dev's regions with pcim_iomap(). It does not actually reserve the regions exclusively with, e.g., pci_request_regions(). Replace the calls to pcim_iomap() with ones to pcim_iomap_regions() to reserve and map the regions simultaneously. Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> Signed-off-by: Philipp Stanner <pstanner@redhat.com> --- ¡Hola! I picked up the memory-region-request-task from the DRM-TODO-List [1] and began with this driver. Please have a first look. I wasn't entirely sure about -ENOMEM... for example, as far as my understanding goes, it should not be able to fail anyways in the second call. I don't have the server-hardware, thus, can't test it on a physical machine. Please tell me what you think. P. [1] https://dri.freedesktop.org/docs/drm/gpu/todo.html#request-memory-regions-in-all-drivers --- drivers/gpu/drm/ast/ast_main.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)