Re: [PATCH 01/18] drm/amdgpu: abstract gart table initialization for gfxhub/mmhub

[Huang Rui <ray.huang-5C7GfCeVMHo@public.gmane.org>]

On Thu, Jun 01, 2017 at 01:02:18AM +0800, Koenig, Christian wrote:
> Am 31.05.2017 um 18:14 schrieb Huang Rui:
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 33
> +++++++++++++++++++-------------
> >   drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  | 33
> +++++++++++++++++++-------------
> >   2 files changed, 40 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/
> amdgpu/gfxhub_v1_0.c
> > index 7c430c4..8cf30b7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> > @@ -31,6 +31,24 @@
> >  
> >   #include "soc15_common.h"
> >  
> > +static void gfxhub_v1_0_init_pt_regs(struct amdgpu_device *adev)
> > +{
> > +     uint64_t value;
> > +
> > +     BUG_ON(adev->gart.table_addr & (~0x0000FFFFFFFFF000ULL));
> > +     value = adev->gart.table_addr - adev->mc.vram_start
> > +             + adev->vm_manager.vram_base_offset;
> > +     value &= 0x0000FFFFFFFFF000ULL;
> > +     value |= 0x1; /*valid bit*/
> > +
> > +     WREG32(SOC15_REG_OFFSET(GC, 0,
> > +                             mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32),
> > +             (u32)value);
> > +     WREG32(SOC15_REG_OFFSET(GC, 0,
> > +                             mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32),
> > +             (u32)(value >> 32));
> > +}
> > +
> >   int gfxhub_v1_0_gart_enable(struct amdgpu_device *adev)
> >   {
> >        u32 tmp;
> > @@ -38,6 +56,8 @@ int gfxhub_v1_0_gart_enable(struct amdgpu_device *adev)
> >        u32 i;
> >  
> >        /* Program MC. */
> > +     gfxhub_v1_0_init_pt_regs(adev);
> > +
> >        /* Update configuration */
> >        WREG32(SOC15_REG_OFFSET(GC, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR),
> >                adev->mc.vram_start >> 18);
> > @@ -154,19 +174,6 @@ int gfxhub_v1_0_gart_enable(struct amdgpu_device *adev)
> >                                mmVM_CONTEXT0_PAGE_TABLE_END_ADDR_HI32),
> >                (u32)(adev->mc.gtt_end >> 44));
> >  
> > -     BUG_ON(adev->gart.table_addr & (~0x0000FFFFFFFFF000ULL));
> > -     value = adev->gart.table_addr - adev->mc.vram_start
> > -             + adev->vm_manager.vram_base_offset;
> > -     value &= 0x0000FFFFFFFFF000ULL;
> > -     value |= 0x1; /*valid bit*/
> > -
> > -     WREG32(SOC15_REG_OFFSET(GC, 0,
> > -                             mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32),
> > -             (u32)value);
> > -     WREG32(SOC15_REG_OFFSET(GC, 0,
> > -                             mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32),
> > -             (u32)(value >> 32));
> > -
> >        WREG32(SOC15_REG_OFFSET(GC, 0,
> >                                mmVM_L2_PROTECTION_FAULT_DEFAULT_ADDR_LO32),
> >                (u32)(adev->dummy_page.addr >> 12));
> > diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/
> amdgpu/mmhub_v1_0.c
> > index afd9d85..84eb3a3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > @@ -44,6 +44,24 @@ u64 mmhub_v1_0_get_fb_location(struct amdgpu_device *adev)
> >        return base;
> >   }
> >  
> > +static void mmhub_v1_0_init_pt_regs(struct amdgpu_device *adev)
> > +{
> > +     uint64_t value;
> > +
> > +     BUG_ON(adev->gart.table_addr & (~0x0000FFFFFFFFF000ULL));
> > +     value = adev->gart.table_addr - adev->mc.vram_start +
> > +             adev->vm_manager.vram_base_offset;
> > +     value &= 0x0000FFFFFFFFF000ULL;
> > +     value |= 0x1; /* valid bit */
> > +
> > +     WREG32(SOC15_REG_OFFSET(MMHUB, 0,
> > +                             mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32),
> > +             (u32)value);
> > +     WREG32(SOC15_REG_OFFSET(MMHUB, 0,
> > +                             mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32),
> > +             (u32)(value >> 32));
> 
> While at it please use the upper_32_bits() and lower_32_bits() macros
> for this.
> 
> Additional to that since you cleanup the code anyway (which is very
> appreciated!) please use the WREG32_SOC15() macro instead.
> 

My pleasure. I found original mmhub/gfxhub programming sequence was really
hard to read when I was debugging S3 issue. So I have to break it down to
make it clearly. 

Could I add one more patches at top of patch set to use
WREG32_SOC15/REG32_SOC15 instead for the whole GMC, GFXHUB, and MMHUB
blocks.

Thanks,
Ray
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx