Re: [08/14] drm/ast: Set up release action right after enabling MMIO

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Am 19.06.23 um 03:57 schrieb Sui Jingfeng:
> Hi,
>
>
> Tested with ast2400
>
>
> On 2023/6/16 21:52, Thomas Zimmermann wrote:
> > Ast sets up a managed release of the MMIO access flags. Move this
> > code next to the MMIO access code, so that it runs if other errors
> > occur during the device initialization.
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>
> Tested-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>

Which model do you test on?

> Reviewed-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
>
> > ---
> >   drivers/gpu/drm/ast/ast_main.c | 38 +++++++++++++++++-----------------
> >   1 file changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/ast/ast_main.c 
> > b/drivers/gpu/drm/ast/ast_main.c
> > index 3295876c09b35..6ff4b837e64d7 100644
> > --- a/drivers/gpu/drm/ast/ast_main.c
> > +++ b/drivers/gpu/drm/ast/ast_main.c
> > @@ -71,11 +71,25 @@ static void ast_enable_vga(struct drm_device *dev)
> >       ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
> >   }
> > -static void ast_enable_mmio(struct drm_device *dev)
> > +/*
> > + * Run this function as part of the HW device cleanup; not
> > + * when the DRM device gets released.
> > + */
>
>
> > +static void ast_enable_mmio_release(void *data)
> >   {
> > -    struct ast_device *ast = to_ast_device(dev);
> > +    struct ast_device *ast = data;
> > +
> > +    /* enable standard VGA decode */
> > +    ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
> > +}
> > +
> > +static int ast_enable_mmio(struct ast_device *ast)
> > +{
> > +    struct drm_device *dev = &ast->base;
> >       ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
> > +
> > +    return devm_add_action_or_reset(dev->dev, 
> > ast_enable_mmio_release, ast);
> >   }
> >   static void ast_open_key(struct ast_device *ast)
> > @@ -392,18 +406,6 @@ static int ast_get_dram_info(struct drm_device *dev)
> >       return 0;
> >   }
> > -/*
> > - * Run this function as part of the HW device cleanup; not
> > - * when the DRM device gets released.
> > - */
> > -static void ast_device_release(void *data)
> > -{
> > -    struct ast_device *ast = data;
> > -
> > -    /* enable standard VGA decode */
> > -    ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
> > -}
> > -
> >   struct ast_device *ast_device_create(const struct drm_driver *drv,
> >                        struct pci_dev *pdev,
> >                        unsigned long flags)
> > @@ -465,7 +467,9 @@ struct ast_device *ast_device_create(const struct 
> > drm_driver *drv,
> >       /* Enable extended register access */
> >       ast_open_key(ast);
> > -    ast_enable_mmio(dev);
> > +    ret = ast_enable_mmio(ast);
> > +    if (ret)
> > +        return ERR_PTR(ret);
> >       /* Find out whether P2A works or whether to use device-tree */
> >       ast_detect_config_mode(dev, &scu_rev);
> > @@ -498,9 +502,5 @@ struct ast_device *ast_device_create(const struct 
> > drm_driver *drv,
> >       if (ret)
> >           return ERR_PTR(ret);
> > -    ret = devm_add_action_or_reset(dev->dev, ast_device_release, ast);
> > -    if (ret)
> > -        return ERR_PTR(ret);
> > -
> >       return ast;
> >   }

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Attachment: OpenPGP_signature
Description: OpenPGP digital signature