Re: [PATCH 1/2] drm/amdgpu: handle asics with 1 SDMA instance

[Huang Rui <ray.huang@amd.com>]

On Thu, May 05, 2022 at 10:52:31AM +0800, Alex Deucher wrote:
> On Wed, May 4, 2022 at 2:28 AM Christian König <christian.koenig@amd.com> wrote:
> >
> > Am 03.05.22 um 22:38 schrieb Alex Deucher:
> > > From: Xiaojian Du <Xiaojian.Du@amd.com>
> > >
> > > This patch will handle asics with 1 SDMA instance.
> > >
> > > Signed-off-by: Xiaojian Du <Xiaojian.Du@amd.com>
> > > Reviewed-by: Huang Rui <ray.huang@amd.com>
> > > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/soc21.c | 5 +++--
> > >   1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> > > index 307a1da13557..29acc5573f56 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> > > @@ -252,8 +252,9 @@ static int soc21_read_register(struct amdgpu_device *adev, u32 se_num,
> > >       *value = 0;
> > >       for (i = 0; i < ARRAY_SIZE(soc21_allowed_read_registers); i++) {
> > >               en = &soc21_allowed_read_registers[i];
> > > -             if (reg_offset !=
> > > -                 (adev->reg_offset[en->hwip][en->inst][en->seg] + en->reg_offset))
> > > +             if ((i == 7 && (adev->sdma.num_instances == 1)) || /* some asics don't have SDMA1 */
> > > +                     reg_offset !=
> > > +                     (adev->reg_offset[en->hwip][en->inst][en->seg] + en->reg_offset))
> >
> > Hui what? Why do we filter out register reads in the low level function?
> 
> It was copied from nv.c.  The proper solution is to handle it like we
> do in soc15.c:
> if (adev->reg_offset[en->hwip][en->inst] &&
>                     reg_offset != (adev->reg_offset[en->hwip][en->inst][en->seg]
>                                         + en->reg_offset))
>                     continue;
> 

The SDMA1_BASE are not contained in GC V11.0.1 APUs, so cannot refer this
pointer. Above check is better than before, thanks!

Thanks,
Ray