Re: drm/ast: Do not enable PCI resources multiple times

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 25.07.23 um 03:12 schrieb suijingfeng:
> Hi,
>
> On 2023/7/25 02:34, Thomas Zimmermann wrote:
> > Hi
> >
> > Am 18.07.23 um 07:40 schrieb suijingfeng:
> > > Hi,
> > >
> > >
> > > Actually,  I'm only a little bit worry about the ast_pm_thaw() code 
> > > path.
> > >
> > > |- ast_pm_thaw()
> > >
> > > |-- ast_drm_thaw()
> > >
> > > |--- ast_post_gpu()
>
> > I'm not quite sure what mean here, because the post-GPU code is not 
> > involved in this patch.
>
>
> Ah, I'm get confused with a previous patch.

Ok, no problem. I've now merged that patch.

Best regards
Thomas

> Previously, the drm/ast driver call ast_init_pci_config() function in 
> ast_post_gpu().
>
> I also don't know why it need to do so.
>
> Ok then, just remove it.
>
>
> > All this patch does is to remove duplicated code.
>
> Yes, this patch has nothing to do with the ast_post_gpu() function.
>
>
> > Is there a bug in the post-GPU handling?
>
> No, I'm not find one so far.
>
>
> > Best regards
> > Thomas
> >
> > > Except this, all other code path have pci_enable_device() or 
> > > pcim_enable_device() called.
> > >
> > > So, this patch seems OK.
> > >
> > >
> > > On 2023/7/12 21:08, Thomas Zimmermann wrote:
> > > > Remove ast_init_pci_config() as the ast driver already enables the PCI
> > > > resources by calling pcim_enable_device().
> > > >
> > > > Suggested-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
> > > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > > > Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>
> > > > ---
> > > >   drivers/gpu/drm/ast/ast_main.c | 21 ---------------------
> > > >   1 file changed, 21 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/ast/ast_main.c 
> > > > b/drivers/gpu/drm/ast/ast_main.c
> > > > index 8bfbdfd86d77..dae365ed3969 100644
> > > > --- a/drivers/gpu/drm/ast/ast_main.c
> > > > +++ b/drivers/gpu/drm/ast/ast_main.c
> > > > @@ -35,23 +35,6 @@
> > > >   #include "ast_drv.h"
> > > > -static int ast_init_pci_config(struct pci_dev *pdev)
> > > > -{
> > > > -    int err;
> > > > -    u16 pcis04;
> > > > -
> > > > -    err = pci_read_config_word(pdev, PCI_COMMAND, &pcis04);
> > > > -    if (err)
> > > > -        goto out;
> > > > -
> > > > -    pcis04 |= PCI_COMMAND_MEMORY | PCI_COMMAND_IO;
> > > > -
> > > > -    err = pci_write_config_word(pdev, PCI_COMMAND, pcis04);
> > > > -
> > > > -out:
> > > > -    return pcibios_err_to_errno(err);
> > > > -}
> > > > -
> > > >   static bool ast_is_vga_enabled(struct drm_device *dev)
> > > >   {
> > > >       struct ast_device *ast = to_ast_device(dev);
> > > > @@ -483,10 +466,6 @@ struct ast_device *ast_device_create(const 
> > > > struct drm_driver *drv,
> > > >               return ERR_PTR(-EIO);
> > > >       }
> > > > -    ret = ast_init_pci_config(pdev);
> > > > -    if (ret)
> > > > -        return ERR_PTR(ret);
> > > > -
> > > >       if (!ast_is_vga_enabled(dev)) {
> > > >           drm_info(dev, "VGA not enabled on entry, requesting chip 
> > > > POST\n");
> > > >           need_post = true;

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