[PATCH] drm/ast: use managed devres functions

[Jocelyn Falempe]

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 at suse.de>
> Signed-off-by: Philipp Stanner <pstanner at 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)) {

-- 

Jocelyn