On Fri, 04 Jun 2021 16:08:26 +0800,
Christian König wrote:
>
>
>
> Am 04.06.21 um 09:53 schrieb Chen Li:
> > I met a gpu addr bug recently and the kernel log
> > tells me the pc is memcpy/memset and link register is
> > radeon_uvd_resume.
> >
> > As we know, in some architectures, optimized memcpy/memset
> > may not work well on device memory. Trival memcpy_toio/memset_io
> > can fix this problem.
> >
> > BTW, amdgpu has already done it in:
> > commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> > that's why it has no this issue on the same gpu and platform.
> >
> > Signed-off-by: Chen Li <chenli@xxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> > index 85a1f2c31749..55abf9a9623b 100644
> > --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> > +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> > @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> > if (rdev->uvd.vcpu_bo == NULL)
> > return -EINVAL;
> > - memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> > + memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> > + rdev->uvd_fw->data,
> > + rdev->uvd_fw->size);
>
> The coding style still looks wrong here, e.g. it is indented to far to the right
> and data/size can be on one line.
It's really werid that the patch before being replyed has not this coding style issue and do always indent the same with previous memcpy(in all of v1, v2 and v3),
you can check at https://patchwork.kernel.org/project/dri-devel/patch/87im2ufhyz.wl-chenli@xxxxxxxxxxxxx/ Cannot figure out what happened, sorry.
I'll take merge them in single line in the next series, thanks.
>
> Apart from that the patch is Reviewed-by: Christian König
> <christian.koenig@xxxxxxx>
>
> Regards,
> Christian.
>
> > size = radeon_bo_size(rdev->uvd.vcpu_bo);
> > size -= rdev->uvd_fw->size;
> > @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
> > ptr = rdev->uvd.cpu_addr;
> > ptr += rdev->uvd_fw->size;
> > - memset(ptr, 0, size);
> > + memset_io((void __iomem *)ptr, 0, size);
> > return 0;
> > }
>
>
>
