Re: [PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Arnd Bergmann]

On Mon, Jun 26, 2017 at 6:26 PM, Logan Gunthorpe  wrote:
> Hi Jyri,
>
> Thanks for the ack. However, I'm reworking this patch set to use the
> include/linux/io-64-nonatomic* headers which will explicitly devolve
> into two 32-bit transfers. It's not clear whether this is appropriate
> for the tilcdc driver as it was never setup to use 32-bit transfers
> (unlike the others I had patched).
>
> If you think it's ok, I can still patch this driver to use the
> non-atomic headers. Otherwise I can leave it out. Please let me know.

You'd have to first figure out whether this device is of the lo-hi
or the hi-lo variant, or doesn't allow the I/O to be split at all.

Note that we could theoretically define ARM to use strd/ldrd
for writeq/readq, but I would expect that to be wrong with many
other devices that can use the existing io-64-nonatomic headers.

The comment in set_scanout() suggests that we actually do rely
on the write64 to be atomic, so we probably don't want to change
this driver.

 Arnd

Re: [PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Logan Gunthorpe]

Hi Jyri,

Thanks for the ack. However, I'm reworking this patch set to use the
include/linux/io-64-nonatomic* headers which will explicitly devolve
into two 32-bit transfers. It's not clear whether this is appropriate
for the tilcdc driver as it was never setup to use 32-bit transfers
(unlike the others I had patched).

If you think it's ok, I can still patch this driver to use the
non-atomic headers. Otherwise I can leave it out. Please let me know.

Thanks,

Logan


On 26/06/17 02:55 AM, Jyri Sarha wrote:
> Acked-by: Jyri Sarha 
> 
> And thanks!
> 
>> ---
>>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 6 --
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h 
>> b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
>> index e9ce725698a9..0b901405f30a 100644
>> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
>> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
>> @@ -133,13 +133,7 @@ static inline void tilcdc_write64(struct drm_device 
>> *dev, u32 reg, u64 data)
>>  struct tilcdc_drm_private *priv = dev->dev_private;
>>  void __iomem *addr = priv->mmio + reg;
>>  
>> -#ifdef iowrite64
>>  iowrite64(data, addr);
>> -#else
>> -__iowmb();
>> -/* This compiles to strd (=64-bit write) on ARM7 */
>> -*(u64 __force *)addr = __cpu_to_le64(data);
>> -#endif
>>  }
>>  
>>  static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
>>
> 

Re: [PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Jyri Sarha]

On 06/22/17 19:48, Logan Gunthorpe wrote:
> Now that we can expect iowrite64 to always exist the hack is no longer
> necessary so we just call iowrite64 directly.
> 
> Signed-off-by: Logan Gunthorpe 
> Cc: Jyri Sarha 
> Cc: Tomi Valkeinen 
> Cc: David Airlie 

Acked-by: Jyri Sarha 

And thanks!

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h 
> b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index e9ce725698a9..0b901405f30a 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -133,13 +133,7 @@ static inline void tilcdc_write64(struct drm_device 
> *dev, u32 reg, u64 data)
>   struct tilcdc_drm_private *priv = dev->dev_private;
>   void __iomem *addr = priv->mmio + reg;
>  
> -#ifdef iowrite64
>   iowrite64(data, addr);
> -#else
> - __iowmb();
> - /* This compiles to strd (=64-bit write) on ARM7 */
> - *(u64 __force *)addr = __cpu_to_le64(data);
> -#endif
>  }
>  
>  static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
> 

[PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Logan Gunthorpe]

Now that we can expect iowrite64 to always exist the hack is no longer
necessary so we just call iowrite64 directly.

Signed-off-by: Logan Gunthorpe 
Cc: Jyri Sarha 
Cc: Tomi Valkeinen 
Cc: David Airlie 
---
 drivers/gpu/drm/tilcdc/tilcdc_regs.h | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h 
b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index e9ce725698a9..0b901405f30a 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -133,13 +133,7 @@ static inline void tilcdc_write64(struct drm_device *dev, 
u32 reg, u64 data)
struct tilcdc_drm_private *priv = dev->dev_private;
void __iomem *addr = priv->mmio + reg;
 
-#ifdef iowrite64
iowrite64(data, addr);
-#else
-   __iowmb();
-   /* This compiles to strd (=64-bit write) on ARM7 */
-   *(u64 __force *)addr = __cpu_to_le64(data);
-#endif
 }
 
 static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
-- 
2.11.0