Linux-Next Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
@ 2019-08-07  2:56 Tao Zhou
  2019-08-07  3:09 ` Jisheng Zhang
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Tao Zhou @ 2019-08-07  2:56 UTC (permalink / raw)
  To: amd-gfx, alexander.deucher, hawking.zhang, dennis.li, broonie,
	akpm, christian.koenig
  Cc: kernel-build-reports, linux-arm-kernel, linux-next, Tao Zhou

readq/writeq are not supported on all architectures

Signed-off-by: Tao Zhou <tao.zhou1@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 558fe6d091ed..7eb9e0b9235a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -272,14 +272,10 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
  */
 uint64_t amdgpu_mm_rreg64(struct amdgpu_device *adev, uint32_t reg)
 {
-	uint64_t ret;
-
 	if ((reg * 4) < adev->rmmio_size)
-		ret = readq(((void __iomem *)adev->rmmio) + (reg * 4));
+		return atomic64_read((atomic64_t *)(adev->rmmio + (reg * 4)));
 	else
 		BUG();
-
-	return ret;
 }
 
 /**
@@ -294,7 +290,7 @@ uint64_t amdgpu_mm_rreg64(struct amdgpu_device *adev, uint32_t reg)
 void amdgpu_mm_wreg64(struct amdgpu_device *adev, uint32_t reg, uint64_t v)
 {
 	if ((reg * 4) < adev->rmmio_size)
-		writeq(v, ((void __iomem *)adev->rmmio) + (reg * 4));
+		atomic64_set((atomic64_t *)(adev->rmmio + (reg * 4)), v);
 	else
 		BUG();
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07  2:56 [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations Tao Zhou
@ 2019-08-07  3:09 ` Jisheng Zhang
  2019-08-07  4:02   ` Alex Deucher
  2019-08-07  4:03 ` Alex Deucher
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Jisheng Zhang @ 2019-08-07  3:09 UTC (permalink / raw)
  To: Tao Zhou
  Cc: amd-gfx, alexander.deucher, hawking.zhang, dennis.li, broonie,
	akpm, christian.koenig, linux-next, linux-arm-kernel,
	kernel-build-reports

On Wed, 7 Aug 2019 10:56:40 +0800 Tao Zhou wrote:


> 
> 
> readq/writeq are not supported on all architectures
> 
> Signed-off-by: Tao Zhou <tao.zhou1@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 558fe6d091ed..7eb9e0b9235a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -272,14 +272,10 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
>   */
>  uint64_t amdgpu_mm_rreg64(struct amdgpu_device *adev, uint32_t reg)
>  {
> -       uint64_t ret;
> -
>         if ((reg * 4) < adev->rmmio_size)
> -               ret = readq(((void __iomem *)adev->rmmio) + (reg * 4));
> +               return atomic64_read((atomic64_t *)(adev->rmmio + (reg * 4)));

atomic64_read doesn't equal to readq on some architectures..

>         else
>                 BUG();
> -
> -       return ret;
>  }
> 
>  /**
> @@ -294,7 +290,7 @@ uint64_t amdgpu_mm_rreg64(struct amdgpu_device *adev, uint32_t reg)
>  void amdgpu_mm_wreg64(struct amdgpu_device *adev, uint32_t reg, uint64_t v)
>  {
>         if ((reg * 4) < adev->rmmio_size)
> -               writeq(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> +               atomic64_set((atomic64_t *)(adev->rmmio + (reg * 4)), v);
>         else
>                 BUG();
>  }
> --
> 2.17.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07  3:09 ` Jisheng Zhang
@ 2019-08-07  4:02   ` Alex Deucher
  0 siblings, 0 replies; 17+ messages in thread
From: Alex Deucher @ 2019-08-07  4:02 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: Tao Zhou, linux-arm-kernel, kernel-build-reports, amd-gfx,
	broonie, linux-next, alexander.deucher, akpm, christian.koenig,
	dennis.li, hawking.zhang

On Tue, Aug 6, 2019 at 11:31 PM Jisheng Zhang
<Jisheng.Zhang@synaptics.com> wrote:
>
> On Wed, 7 Aug 2019 10:56:40 +0800 Tao Zhou wrote:
>
>
> >
> >
> > readq/writeq are not supported on all architectures
> >
> > Signed-off-by: Tao Zhou <tao.zhou1@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 558fe6d091ed..7eb9e0b9235a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -272,14 +272,10 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
> >   */
> >  uint64_t amdgpu_mm_rreg64(struct amdgpu_device *adev, uint32_t reg)
> >  {
> > -       uint64_t ret;
> > -
> >         if ((reg * 4) < adev->rmmio_size)
> > -               ret = readq(((void __iomem *)adev->rmmio) + (reg * 4));
> > +               return atomic64_read((atomic64_t *)(adev->rmmio + (reg * 4)));
>
> atomic64_read doesn't equal to readq on some architectures..

What we really wanted originally was atomic64.  We basically want a
read or write that is guaranteed to be 64 bits at a time.

Alex

>
> >         else
> >                 BUG();
> > -
> > -       return ret;
> >  }
> >
> >  /**
> > @@ -294,7 +290,7 @@ uint64_t amdgpu_mm_rreg64(struct amdgpu_device *adev, uint32_t reg)
> >  void amdgpu_mm_wreg64(struct amdgpu_device *adev, uint32_t reg, uint64_t v)
> >  {
> >         if ((reg * 4) < adev->rmmio_size)
> > -               writeq(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> > +               atomic64_set((atomic64_t *)(adev->rmmio + (reg * 4)), v);
> >         else
> >                 BUG();
> >  }
> > --
> > 2.17.1
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07  2:56 [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations Tao Zhou
  2019-08-07  3:09 ` Jisheng Zhang
@ 2019-08-07  4:03 ` Alex Deucher
  2019-08-07  7:08 ` Christoph Hellwig
  2019-08-08 19:25 ` Guenter Roeck
  3 siblings, 0 replies; 17+ messages in thread
From: Alex Deucher @ 2019-08-07  4:03 UTC (permalink / raw)
  To: Tao Zhou
  Cc: amd-gfx list, Deucher, Alexander, Hawking Zhang, Dennis Li,
	Mark Brown, Andrew Morton, Christian Koenig,
	Linux-Next Mailing List, linux-arm-kernel, kernel-build-reports

On Tue, Aug 6, 2019 at 10:56 PM Tao Zhou <tao.zhou1@amd.com> wrote:
>
> readq/writeq are not supported on all architectures
>
> Signed-off-by: Tao Zhou <tao.zhou1@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 558fe6d091ed..7eb9e0b9235a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -272,14 +272,10 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
>   */
>  uint64_t amdgpu_mm_rreg64(struct amdgpu_device *adev, uint32_t reg)
>  {
> -       uint64_t ret;
> -
>         if ((reg * 4) < adev->rmmio_size)
> -               ret = readq(((void __iomem *)adev->rmmio) + (reg * 4));
> +               return atomic64_read((atomic64_t *)(adev->rmmio + (reg * 4)));
>         else
>                 BUG();
> -
> -       return ret;
>  }
>
>  /**
> @@ -294,7 +290,7 @@ uint64_t amdgpu_mm_rreg64(struct amdgpu_device *adev, uint32_t reg)
>  void amdgpu_mm_wreg64(struct amdgpu_device *adev, uint32_t reg, uint64_t v)
>  {
>         if ((reg * 4) < adev->rmmio_size)
> -               writeq(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> +               atomic64_set((atomic64_t *)(adev->rmmio + (reg * 4)), v);
>         else
>                 BUG();
>  }
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07  2:56 [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations Tao Zhou
  2019-08-07  3:09 ` Jisheng Zhang
  2019-08-07  4:03 ` Alex Deucher
@ 2019-08-07  7:08 ` Christoph Hellwig
  2019-08-07  8:53   ` Koenig, Christian
  2019-08-08 19:25 ` Guenter Roeck
  3 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2019-08-07  7:08 UTC (permalink / raw)
  To: Tao Zhou
  Cc: amd-gfx, alexander.deucher, hawking.zhang, dennis.li, broonie,
	akpm, christian.koenig, linux-next, linux-arm-kernel,
	kernel-build-reports

On Wed, Aug 07, 2019 at 10:56:40AM +0800, Tao Zhou wrote:
> readq/writeq are not supported on all architectures

NAK.  You must not use atomic_* on __iomem (MMIO) memory.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07  7:08 ` Christoph Hellwig
@ 2019-08-07  8:53   ` Koenig, Christian
  2019-08-07 10:41     ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Koenig, Christian @ 2019-08-07  8:53 UTC (permalink / raw)
  To: Christoph Hellwig, Zhou1, Tao
  Cc: amd-gfx, Deucher, Alexander, Zhang, Hawking, Li, Dennis, broonie,
	akpm, linux-next, linux-arm-kernel, kernel-build-reports

Am 07.08.19 um 09:08 schrieb Christoph Hellwig:
> On Wed, Aug 07, 2019 at 10:56:40AM +0800, Tao Zhou wrote:
>> readq/writeq are not supported on all architectures
> NAK.  You must not use atomic_* on __iomem (MMIO) memory.

Well then what's the right thing to do here?

Essentially writeq/readq doesn't seems to be available on all 
architectures either.

Regards,
Christian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07  8:53   ` Koenig, Christian
@ 2019-08-07 10:41     ` Christoph Hellwig
  2019-08-07 10:55       ` Koenig, Christian
  0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2019-08-07 10:41 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Christoph Hellwig, Zhou1, Tao, amd-gfx, Deucher, Alexander,
	Zhang, Hawking, Li, Dennis, broonie, akpm, linux-next,
	linux-arm-kernel, kernel-build-reports

On Wed, Aug 07, 2019 at 08:53:25AM +0000, Koenig, Christian wrote:
> Am 07.08.19 um 09:08 schrieb Christoph Hellwig:
> > On Wed, Aug 07, 2019 at 10:56:40AM +0800, Tao Zhou wrote:
> >> readq/writeq are not supported on all architectures
> > NAK.  You must not use atomic_* on __iomem (MMIO) memory.
> 
> Well then what's the right thing to do here?
> 
> Essentially writeq/readq doesn't seems to be available on all 
> architectures either.

writeq/readq are provided whenever the CPU actually supports 64-bit
atomic loads and stores.  If it doesn't provide them atomic64* is
not going to be atomic vs the I/O device either.  And that is on top
of the fact that for various architectures you can't simply use
plain loads and stores on MMIO memory to start with, which is why
we have the special accessors and the __iomem annotation.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07 10:41     ` Christoph Hellwig
@ 2019-08-07 10:55       ` Koenig, Christian
  2019-08-07 12:59         ` Mark Brown
  2019-08-07 13:00         ` Christoph Hellwig
  0 siblings, 2 replies; 17+ messages in thread
From: Koenig, Christian @ 2019-08-07 10:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Zhou1, Tao, amd-gfx, Deucher, Alexander, Zhang, Hawking, Li,
	Dennis, broonie, akpm, linux-next, linux-arm-kernel,
	kernel-build-reports

Am 07.08.19 um 12:41 schrieb Christoph Hellwig:
> On Wed, Aug 07, 2019 at 08:53:25AM +0000, Koenig, Christian wrote:
>> Am 07.08.19 um 09:08 schrieb Christoph Hellwig:
>>> On Wed, Aug 07, 2019 at 10:56:40AM +0800, Tao Zhou wrote:
>>>> readq/writeq are not supported on all architectures
>>> NAK.  You must not use atomic_* on __iomem (MMIO) memory.
>> Well then what's the right thing to do here?
>>
>> Essentially writeq/readq doesn't seems to be available on all
>> architectures either.
> writeq/readq are provided whenever the CPU actually supports 64-bit
> atomic loads and stores.

Is there a config option which we can make the driver depend on?

I mean that ARM doesn't support 64bit atomic loads and stores on MMIO is 
quite a boomer for us.

Toa do you of hand know what we actually need the 64bit atomic stores 
for? E.g. what is the hardware background?

Regards,
Christian.

> If it doesn't provide them atomic64* is
> not going to be atomic vs the I/O device either.  And that is on top
> of the fact that for various architectures you can't simply use
> plain loads and stores on MMIO memory to start with, which is why
> we have the special accessors and the __iomem annotation.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07 10:55       ` Koenig, Christian
@ 2019-08-07 12:59         ` Mark Brown
  2019-08-07 13:00           ` Koenig, Christian
  2019-08-07 13:00         ` Christoph Hellwig
  1 sibling, 1 reply; 17+ messages in thread
From: Mark Brown @ 2019-08-07 12:59 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Christoph Hellwig, Zhou1, Tao, amd-gfx, Deucher, Alexander,
	Zhang, Hawking, Li, Dennis, akpm, linux-next, linux-arm-kernel,
	kernel-build-reports

[-- Attachment #1: Type: text/plain, Size: 315 bytes --]

On Wed, Aug 07, 2019 at 10:55:01AM +0000, Koenig, Christian wrote:
> Am 07.08.19 um 12:41 schrieb Christoph Hellwig:

> > writeq/readq are provided whenever the CPU actually supports 64-bit
> > atomic loads and stores.

> Is there a config option which we can make the driver depend on?

64BIT should do the trick.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07 10:55       ` Koenig, Christian
  2019-08-07 12:59         ` Mark Brown
@ 2019-08-07 13:00         ` Christoph Hellwig
  2019-08-07 13:03           ` Koenig, Christian
  1 sibling, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2019-08-07 13:00 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Christoph Hellwig, Zhou1, Tao, amd-gfx, Deucher, Alexander,
	Zhang, Hawking, Li, Dennis, broonie, akpm, linux-next,
	linux-arm-kernel, kernel-build-reports

On Wed, Aug 07, 2019 at 10:55:01AM +0000, Koenig, Christian wrote:
> >> Essentially writeq/readq doesn't seems to be available on all
> >> architectures either.
> > writeq/readq are provided whenever the CPU actually supports 64-bit
> > atomic loads and stores.
> 
> Is there a config option which we can make the driver depend on?
> 
> I mean that ARM doesn't support 64bit atomic loads and stores on MMIO is 
> quite a boomer for us.

The model is to cheack if readq/writeq are defined, and if not to
include the one of io-64-nonatomic-hi-lo.h or io-64-nonatomic-lo-hi.h.
The reason for that is that hardware is supposed to be able to deal with
two 32-bit writes, but it depends on the hardware if the lower or upper
half is what commits the write.

The only 32-bit platform that claims support for readq/writeq is sh,
and I have doubts if that actually works as expected.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07 12:59         ` Mark Brown
@ 2019-08-07 13:00           ` Koenig, Christian
  2019-08-07 13:07             ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Koenig, Christian @ 2019-08-07 13:00 UTC (permalink / raw)
  To: Mark Brown
  Cc: Christoph Hellwig, Zhou1, Tao, amd-gfx, Deucher, Alexander,
	Zhang, Hawking, Li, Dennis, akpm, linux-next, linux-arm-kernel,
	kernel-build-reports

Am 07.08.19 um 14:59 schrieb Mark Brown:
> On Wed, Aug 07, 2019 at 10:55:01AM +0000, Koenig, Christian wrote:
>> Am 07.08.19 um 12:41 schrieb Christoph Hellwig:
>>> writeq/readq are provided whenever the CPU actually supports 64-bit
>>> atomic loads and stores.
>> Is there a config option which we can make the driver depend on?
> 64BIT should do the trick.

That doesn't work because 32bit x86 does support writeq/readq as far as 
I know.

Christian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07 13:00         ` Christoph Hellwig
@ 2019-08-07 13:03           ` Koenig, Christian
  2019-08-07 18:00             ` Alex Deucher
  0 siblings, 1 reply; 17+ messages in thread
From: Koenig, Christian @ 2019-08-07 13:03 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Zhou1, Tao, amd-gfx, Deucher, Alexander, Zhang, Hawking, Li,
	Dennis, broonie, akpm, linux-next, linux-arm-kernel,
	kernel-build-reports

Am 07.08.19 um 15:00 schrieb Christoph Hellwig:
> On Wed, Aug 07, 2019 at 10:55:01AM +0000, Koenig, Christian wrote:
>>>> Essentially writeq/readq doesn't seems to be available on all
>>>> architectures either.
>>> writeq/readq are provided whenever the CPU actually supports 64-bit
>>> atomic loads and stores.
>> Is there a config option which we can make the driver depend on?
>>
>> I mean that ARM doesn't support 64bit atomic loads and stores on MMIO is
>> quite a boomer for us.
> The model is to cheack if readq/writeq are defined, and if not to
> include the one of io-64-nonatomic-hi-lo.h or io-64-nonatomic-lo-hi.h.
> The reason for that is that hardware is supposed to be able to deal with
> two 32-bit writes, but it depends on the hardware if the lower or upper
> half is what commits the write.

Read, but as I understood Tao change this is not the case here. 
Otherwise we would just use our WREG32/RREG32 macros in the driver.

Tao, please explain why exactly we need the WREG64/RREG64 change which 
caused this.

Christian.

>
> The only 32-bit platform that claims support for readq/writeq is sh,
> and I have doubts if that actually works as expected.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07 13:00           ` Koenig, Christian
@ 2019-08-07 13:07             ` Christoph Hellwig
  0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2019-08-07 13:07 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Mark Brown, Christoph Hellwig, Zhou1, Tao, amd-gfx, Deucher,
	Alexander, Zhang, Hawking, Li, Dennis, akpm, linux-next,
	linux-arm-kernel, kernel-build-reports

On Wed, Aug 07, 2019 at 01:00:48PM +0000, Koenig, Christian wrote:
> Am 07.08.19 um 14:59 schrieb Mark Brown:
> > On Wed, Aug 07, 2019 at 10:55:01AM +0000, Koenig, Christian wrote:
> >> Am 07.08.19 um 12:41 schrieb Christoph Hellwig:
> >>> writeq/readq are provided whenever the CPU actually supports 64-bit
> >>> atomic loads and stores.
> >> Is there a config option which we can make the driver depend on?
> > 64BIT should do the trick.
> 
> That doesn't work because 32bit x86 does support writeq/readq as far as 
> I know.

I also had a vague memory that x86-32 did support it with SSE, but
I can't actually find any traces of that support.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07 13:03           ` Koenig, Christian
@ 2019-08-07 18:00             ` Alex Deucher
  0 siblings, 0 replies; 17+ messages in thread
From: Alex Deucher @ 2019-08-07 18:00 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Christoph Hellwig, linux-arm-kernel, kernel-build-reports, Zhou1,
	Tao, amd-gfx, broonie, linux-next, Deucher, Alexander, akpm, Li,
	Dennis, Zhang, Hawking

On Wed, Aug 7, 2019 at 9:03 AM Koenig, Christian
<Christian.Koenig@amd.com> wrote:
>
> Am 07.08.19 um 15:00 schrieb Christoph Hellwig:
> > On Wed, Aug 07, 2019 at 10:55:01AM +0000, Koenig, Christian wrote:
> >>>> Essentially writeq/readq doesn't seems to be available on all
> >>>> architectures either.
> >>> writeq/readq are provided whenever the CPU actually supports 64-bit
> >>> atomic loads and stores.
> >> Is there a config option which we can make the driver depend on?
> >>
> >> I mean that ARM doesn't support 64bit atomic loads and stores on MMIO is
> >> quite a boomer for us.
> > The model is to cheack if readq/writeq are defined, and if not to
> > include the one of io-64-nonatomic-hi-lo.h or io-64-nonatomic-lo-hi.h.
> > The reason for that is that hardware is supposed to be able to deal with
> > two 32-bit writes, but it depends on the hardware if the lower or upper
> > half is what commits the write.
>
> Read, but as I understood Tao change this is not the case here.
> Otherwise we would just use our WREG32/RREG32 macros in the driver.
>
> Tao, please explain why exactly we need the WREG64/RREG64 change which
> caused this.

We use this for doorbells as well which is also MMIO.  Basically we
have the requirement to read or write the full 64 bits in one
operation.  E.g., for 64-bit doorbells, the entire register is the
trigger so if we do it as two writes, we'll miss half the update.  In
the case of some error counter registers, reading the register will
clear the value so we need to read out the full value or we lose the
half the value.  This works properly on x86 and AMD64.

Alex

>
> Christian.
>
> >
> > The only 32-bit platform that claims support for readq/writeq is sh,
> > and I have doubts if that actually works as expected.
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-07  2:56 [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations Tao Zhou
                   ` (2 preceding siblings ...)
  2019-08-07  7:08 ` Christoph Hellwig
@ 2019-08-08 19:25 ` Guenter Roeck
  2019-08-08 19:33   ` Alex Deucher
  3 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2019-08-08 19:25 UTC (permalink / raw)
  To: Tao Zhou
  Cc: amd-gfx, alexander.deucher, hawking.zhang, dennis.li, broonie,
	akpm, christian.koenig, linux-next, linux-arm-kernel,
	kernel-build-reports

On Wed, Aug 07, 2019 at 10:56:40AM +0800, Tao Zhou wrote:
> readq/writeq are not supported on all architectures
> 
> Signed-off-by: Tao Zhou <tao.zhou1@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Regarding the claim that this would work for 32-bit x86 builds:

make ARCH=i386 allmodconfig
make ARCH=i386 drivers/gpu/drm/amd/amdgpu/amdgpu_device.o

results in:

  ...
  CC [M]  drivers/gpu/drm/amd/amdgpu/amdgpu_device.o
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: In function ‘amdgpu_mm_rreg64’:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:279:9: error: implicit declaration of function ‘readq’;

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: In function ‘amdgpu_mm_wreg64’:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:298:3: error: implicit declaration of function ‘writeq’

This is with next-20190808.

Guenter

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-08 19:25 ` Guenter Roeck
@ 2019-08-08 19:33   ` Alex Deucher
  2019-08-09  9:04     ` Koenig, Christian
  0 siblings, 1 reply; 17+ messages in thread
From: Alex Deucher @ 2019-08-08 19:33 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Tao Zhou, linux-arm-kernel, kernel-build-reports, amd-gfx list,
	Mark Brown, Linux-Next Mailing List, Deucher, Alexander,
	Andrew Morton, Christian Koenig, Dennis Li, Hawking Zhang

On Thu, Aug 8, 2019 at 3:31 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Wed, Aug 07, 2019 at 10:56:40AM +0800, Tao Zhou wrote:
> > readq/writeq are not supported on all architectures
> >
> > Signed-off-by: Tao Zhou <tao.zhou1@amd.com>
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Regarding the claim that this would work for 32-bit x86 builds:

I wasn't talking about readq/writeq, I was talking about the atomic64
interfaces.

Alex

>
> make ARCH=i386 allmodconfig
> make ARCH=i386 drivers/gpu/drm/amd/amdgpu/amdgpu_device.o
>
> results in:
>
>   ...
>   CC [M]  drivers/gpu/drm/amd/amdgpu/amdgpu_device.o
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: In function ‘amdgpu_mm_rreg64’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:279:9: error: implicit declaration of function ‘readq’;
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: In function ‘amdgpu_mm_wreg64’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:298:3: error: implicit declaration of function ‘writeq’
>
> This is with next-20190808.
>
> Guenter
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: drm/amdgpu: replace readq/writeq with atomic64 operations
  2019-08-08 19:33   ` Alex Deucher
@ 2019-08-09  9:04     ` Koenig, Christian
  0 siblings, 0 replies; 17+ messages in thread
From: Koenig, Christian @ 2019-08-09  9:04 UTC (permalink / raw)
  To: Alex Deucher, Guenter Roeck
  Cc: Zhou1, Tao, linux-arm-kernel, kernel-build-reports, amd-gfx list,
	Mark Brown, Linux-Next Mailing List, Deucher, Alexander,
	Andrew Morton, Li, Dennis, Zhang, Hawking

Am 08.08.19 um 21:33 schrieb Alex Deucher:
> On Thu, Aug 8, 2019 at 3:31 PM Guenter Roeck <linux@roeck-us.net> wrote:
>> On Wed, Aug 07, 2019 at 10:56:40AM +0800, Tao Zhou wrote:
>>> readq/writeq are not supported on all architectures
>>>
>>> Signed-off-by: Tao Zhou <tao.zhou1@amd.com>
>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> Regarding the claim that this would work for 32-bit x86 builds:
> I wasn't talking about readq/writeq, I was talking about the atomic64
> interfaces.

On quite a bunch of architectures atomic64 operations are actually 
implemented with spinlocks or other architecture depended special handling.

So the approach of casting an iomem pointer to an atomic64 and then hope 
for the best is actually completely nonsense.

If the hardware really needs a single 64bit write for doorbells or 
registers, then we absolutely need to limit the driver to 64bit 
architectures.

If the hardware doesn't need 64bit writes we should actually always use 
two 32bit writes to not run into random and hard to debug problems 
because of this.

Christian.

>
> Alex
>
>> make ARCH=i386 allmodconfig
>> make ARCH=i386 drivers/gpu/drm/amd/amdgpu/amdgpu_device.o
>>
>> results in:
>>
>>    ...
>>    CC [M]  drivers/gpu/drm/amd/amdgpu/amdgpu_device.o
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: In function ‘amdgpu_mm_rreg64’:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:279:9: error: implicit declaration of function ‘readq’;
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: In function ‘amdgpu_mm_wreg64’:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:298:3: error: implicit declaration of function ‘writeq’
>>
>> This is with next-20190808.
>>
>> Guenter
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, back to index

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07  2:56 [PATCH] drm/amdgpu: replace readq/writeq with atomic64 operations Tao Zhou
2019-08-07  3:09 ` Jisheng Zhang
2019-08-07  4:02   ` Alex Deucher
2019-08-07  4:03 ` Alex Deucher
2019-08-07  7:08 ` Christoph Hellwig
2019-08-07  8:53   ` Koenig, Christian
2019-08-07 10:41     ` Christoph Hellwig
2019-08-07 10:55       ` Koenig, Christian
2019-08-07 12:59         ` Mark Brown
2019-08-07 13:00           ` Koenig, Christian
2019-08-07 13:07             ` Christoph Hellwig
2019-08-07 13:00         ` Christoph Hellwig
2019-08-07 13:03           ` Koenig, Christian
2019-08-07 18:00             ` Alex Deucher
2019-08-08 19:25 ` Guenter Roeck
2019-08-08 19:33   ` Alex Deucher
2019-08-09  9:04     ` Koenig, Christian

Linux-Next Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-next/0 linux-next/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-next linux-next/ https://lore.kernel.org/linux-next \
		linux-next@vger.kernel.org linux-next@archiver.kernel.org
	public-inbox-index linux-next


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-next


AGPL code for this site: git clone https://public-inbox.org/ public-inbox