<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Agree. That's much better actually.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Yong</div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Christian König <ckoenig.leichtzumerken@gmail.com> Sent: Monday, February 25, 2019 10:44 AM To: Zhao, Yong; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: Differentiate two set_pte_pde function pointers <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">Am 21.02.19 um 23:58 schrieb Zhao, Yong: > There were two set_pte_pde function pointers in amdgpu_gmc_funcs and > amdgpu_vm_pte_funcs respectively. Because they are so similar, sometimes > it is confusing. So Rename the one in amdgpu_vm_pte_funcs to > write_pte_pde. NAK, we also already have a write_pte function. So this only increases the confusion. Instead we should rather just completely remove the gmc_*_set_pte_pde() functions because they actually do exactly the same on all generations. Christian. > > Signed-off-by: Yong Zhao <Yong.Zhao@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++--- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++-- > drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 6 +++--- > drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 6 +++--- > drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 6 +++--- > drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 +++--- > drivers/gpu/drm/amd/amdgpu/si_dma.c | 6 +++--- > 7 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 942b5ebc6dc2..fa86be694e48 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -806,7 +806,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev, > if (level != AMDGPU_VM_PTB) > ats_value |= AMDGPU_PDE_PTE; > > - amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0, > + amdgpu_vm_write_pte_pde(adev, &job->ibs[0], addr, 0, > ats_entries, 0, ats_value); > addr += ats_entries * 8; > } > @@ -818,7 +818,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev, > if (level == AMDGPU_VM_PTB && adev->asic_type >= CHIP_VEGA10) > value = AMDGPU_PTE_EXECUTABLE; > > - amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0, > + amdgpu_vm_write_pte_pde(adev, &job->ibs[0], addr, 0, > entries, 0, value); > } > > @@ -1235,7 +1235,7 @@ static void amdgpu_vm_do_set_ptes(struct amdgpu_pte_update_params *params, > addr | flags, count, incr); > > } else { > - amdgpu_vm_set_pte_pde(params->adev, params->ib, pe, addr, > + amdgpu_vm_write_pte_pde(params->adev, params->ib, pe, addr, > count, incr, flags); > } > } > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > index 81ff8177f092..7e345ec2671f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > @@ -161,7 +161,7 @@ struct amdgpu_vm_pte_funcs { > uint64_t value, unsigned count, > uint32_t incr); > /* for linear pte/pde updates without addr mapping */ > - void (*set_pte_pde)(struct amdgpu_ib *ib, > + void (*write_pte_pde)(struct amdgpu_ib *ib, > uint64_t pe, > uint64_t addr, unsigned count, > uint32_t incr, uint64_t flags); > @@ -287,7 +287,7 @@ struct amdgpu_vm_manager { > > #define amdgpu_vm_copy_pte(adev, ib, pe, src, count) ((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count))) > #define amdgpu_vm_write_pte(adev, ib, pe, value, count, incr) ((adev)->vm_manager.vm_pte_funcs->write_pte((ib), (pe), (value), (count), (incr))) > -#define amdgpu_vm_set_pte_pde(adev, ib, pe, addr, count, incr, flags) ((adev)->vm_manager.vm_pte_funcs->set_pte_pde((ib), (pe), (addr), (count), (incr), (flags))) > +#define amdgpu_vm_write_pte_pde(adev, ib, pe, addr, count, incr, flags) ((adev)->vm_manager.vm_pte_funcs->write_pte_pde((ib), (pe), (addr), (count), (incr), (flags))) > > void amdgpu_vm_manager_init(struct amdgpu_device *adev); > void amdgpu_vm_manager_fini(struct amdgpu_device *adev); > diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c > index 189599b694e8..07b874a44ddf 100644 > --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c > +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c > @@ -769,7 +769,7 @@ static void cik_sdma_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > } > > /** > - * cik_sdma_vm_set_pages - update the page tables using sDMA > + * cik_sdma_vm_write_pte_pde - update the page tables using sDMA > * > * @ib: indirect buffer to fill with commands > * @pe: addr of the page entry > @@ -780,7 +780,7 @@ static void cik_sdma_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > * > * Update the page tables using sDMA (CIK). > */ > -static void cik_sdma_vm_set_pte_pde(struct amdgpu_ib *ib, uint64_t pe, > +static void cik_sdma_vm_write_pte_pde(struct amdgpu_ib *ib, uint64_t pe, > uint64_t addr, unsigned count, > uint32_t incr, uint64_t flags) > { > @@ -1365,7 +1365,7 @@ static const struct amdgpu_vm_pte_funcs cik_sdma_vm_pte_funcs = { > .copy_pte = cik_sdma_vm_copy_pte, > > .write_pte = cik_sdma_vm_write_pte, > - .set_pte_pde = cik_sdma_vm_set_pte_pde, > + .write_pte_pde = cik_sdma_vm_write_pte_pde, > }; > > static void cik_sdma_set_vm_pte_funcs(struct amdgpu_device *adev) > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c > index cca3552b36ed..3161871c1a57 100644 > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c > @@ -707,7 +707,7 @@ static void sdma_v2_4_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > } > > /** > - * sdma_v2_4_vm_set_pte_pde - update the page tables using sDMA > + * sdma_v2_4_vm_write_pte_pde - update the page tables using sDMA > * > * @ib: indirect buffer to fill with commands > * @pe: addr of the page entry > @@ -718,7 +718,7 @@ static void sdma_v2_4_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > * > * Update the page tables using sDMA (CIK). > */ > -static void sdma_v2_4_vm_set_pte_pde(struct amdgpu_ib *ib, uint64_t pe, > +static void sdma_v2_4_vm_write_pte_pde(struct amdgpu_ib *ib, uint64_t pe, > uint64_t addr, unsigned count, > uint32_t incr, uint64_t flags) > { > @@ -1252,7 +1252,7 @@ static const struct amdgpu_vm_pte_funcs sdma_v2_4_vm_pte_funcs = { > .copy_pte = sdma_v2_4_vm_copy_pte, > > .write_pte = sdma_v2_4_vm_write_pte, > - .set_pte_pde = sdma_v2_4_vm_set_pte_pde, > + .write_pte_pde = sdma_v2_4_vm_write_pte_pde, > }; > > static void sdma_v2_4_set_vm_pte_funcs(struct amdgpu_device *adev) > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c > index 0ce8331baeb2..e369013a6bce 100644 > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c > @@ -978,7 +978,7 @@ static void sdma_v3_0_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > } > > /** > - * sdma_v3_0_vm_set_pte_pde - update the page tables using sDMA > + * sdma_v3_0_vm_write_pte_pde - update the page tables using sDMA > * > * @ib: indirect buffer to fill with commands > * @pe: addr of the page entry > @@ -989,7 +989,7 @@ static void sdma_v3_0_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > * > * Update the page tables using sDMA (CIK). > */ > -static void sdma_v3_0_vm_set_pte_pde(struct amdgpu_ib *ib, uint64_t pe, > +static void sdma_v3_0_vm_write_pte_pde(struct amdgpu_ib *ib, uint64_t pe, > uint64_t addr, unsigned count, > uint32_t incr, uint64_t flags) > { > @@ -1690,7 +1690,7 @@ static const struct amdgpu_vm_pte_funcs sdma_v3_0_vm_pte_funcs = { > .copy_pte = sdma_v3_0_vm_copy_pte, > > .write_pte = sdma_v3_0_vm_write_pte, > - .set_pte_pde = sdma_v3_0_vm_set_pte_pde, > + .write_pte_pde = sdma_v3_0_vm_write_pte_pde, > }; > > static void sdma_v3_0_set_vm_pte_funcs(struct amdgpu_device *adev) > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > index c816e55d43a9..e5e9eac1758d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > @@ -1343,7 +1343,7 @@ static void sdma_v4_0_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > } > > /** > - * sdma_v4_0_vm_set_pte_pde - update the page tables using sDMA > + * sdma_v4_0_vm_write_pte_pde - update the page tables using sDMA > * > * @ib: indirect buffer to fill with commands > * @pe: addr of the page entry > @@ -1354,7 +1354,7 @@ static void sdma_v4_0_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > * > * Update the page tables using sDMA (VEGA10). > */ > -static void sdma_v4_0_vm_set_pte_pde(struct amdgpu_ib *ib, > +static void sdma_v4_0_vm_write_pte_pde(struct amdgpu_ib *ib, > uint64_t pe, > uint64_t addr, unsigned count, > uint32_t incr, uint64_t flags) > @@ -2088,7 +2088,7 @@ static const struct amdgpu_vm_pte_funcs sdma_v4_0_vm_pte_funcs = { > .copy_pte = sdma_v4_0_vm_copy_pte, > > .write_pte = sdma_v4_0_vm_write_pte, > - .set_pte_pde = sdma_v4_0_vm_set_pte_pde, > + .write_pte_pde = sdma_v4_0_vm_write_pte_pde, > }; > > static void sdma_v4_0_set_vm_pte_funcs(struct amdgpu_device *adev) > diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c > index f15f196684ba..4c1e1ce895f7 100644 > --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c > +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c > @@ -353,7 +353,7 @@ static void si_dma_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > } > > /** > - * si_dma_vm_set_pte_pde - update the page tables using sDMA > + * si_dma_vm_write_pte_pde - update the page tables using sDMA > * > * @ib: indirect buffer to fill with commands > * @pe: addr of the page entry > @@ -364,7 +364,7 @@ static void si_dma_vm_write_pte(struct amdgpu_ib *ib, uint64_t pe, > * > * Update the page tables using sDMA (CIK). > */ > -static void si_dma_vm_set_pte_pde(struct amdgpu_ib *ib, > +static void si_dma_vm_write_pte_pde(struct amdgpu_ib *ib, > uint64_t pe, > uint64_t addr, unsigned count, > uint32_t incr, uint64_t flags) > @@ -829,7 +829,7 @@ static const struct amdgpu_vm_pte_funcs si_dma_vm_pte_funcs = { > .copy_pte = si_dma_vm_copy_pte, > > .write_pte = si_dma_vm_write_pte, > - .set_pte_pde = si_dma_vm_set_pte_pde, > + .write_pte_pde = si_dma_vm_write_pte_pde, > }; > > static void si_dma_set_vm_pte_funcs(struct amdgpu_device *adev) </div> </div> </body> </html>