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

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Am 19.06.23 um 10:22 schrieb Thomas Zimmermann:
> 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?

Oh, you mentioned the AST2400.

> > 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