Re: [Intel-gfx] [PATCH 08/37] drm/i915: make intel_{uncore, de}_rmw() more useful

2020-05-21 Thread Lucas De Marchi 

On Thu, May 21, 2020 at 10:24:49AM -0700, Jose Souza wrote:

On Wed, 2020-05-20 at 17:37 -0700, Lucas De Marchi wrote:

Return the old value read so some places of the code can still do the
rmw but add warnings/errors about the value it read.

Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/display/intel_de.h |  4 ++--
 drivers/gpu/drm/i915/intel_uncore.h | 10 +++---
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_de.h 
b/drivers/gpu/drm/i915/display/intel_de.h
index 00da10bf35f5..d5441b1ba2fe 100644
--- a/drivers/gpu/drm/i915/display/intel_de.h
+++ b/drivers/gpu/drm/i915/display/intel_de.h
@@ -42,10 +42,10 @@ intel_de_write_fw(struct drm_i915_private *i915, i915_reg_t 
reg, u32 val)
intel_uncore_write_fw(>uncore, reg, val);
 }



Maybe add function documentation with this new information about the return?


yeah... just not sure about the usefulness since the only place I
intended to use it I ended up doing something different. So I'm still
not sure. Maybe wait a user to appear.

thanks

Lucas De Marchi



With that:

Reviewed-by: José Roberto de Souza 



-static inline void
+static inline u32
 intel_de_rmw(struct drm_i915_private *i915, i915_reg_t reg, u32 clear, u32 set)
 {
-   intel_uncore_rmw(>uncore, reg, clear, set);
+   return intel_uncore_rmw(>uncore, reg, clear, set);
 }

 static inline int
diff --git a/drivers/gpu/drm/i915/intel_uncore.h 
b/drivers/gpu/drm/i915/intel_uncore.h
index 8d3aa8b9acf9..5da43b56fa11 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -379,8 +379,8 @@ intel_uncore_read64_2x32(struct intel_uncore *uncore,
 #define intel_uncore_write64_fw(...) __raw_uncore_write64(__VA_ARGS__)
 #define intel_uncore_posting_read_fw(...) 
((void)intel_uncore_read_fw(__VA_ARGS__))

-static inline void intel_uncore_rmw(struct intel_uncore *uncore,
-   i915_reg_t reg, u32 clear, u32 set)
+static inline u32 intel_uncore_rmw(struct intel_uncore *uncore,
+  i915_reg_t reg, u32 clear, u32 set)
 {
u32 old, val;

@@ -388,9 +388,11 @@ static inline void intel_uncore_rmw(struct intel_uncore 
*uncore,
val = (old & ~clear) | set;
if (val != old)
intel_uncore_write(uncore, reg, val);
+
+   return old;
 }

-static inline void intel_uncore_rmw_fw(struct intel_uncore *uncore,
+static inline u32 intel_uncore_rmw_fw(struct intel_uncore *uncore,
   i915_reg_t reg, u32 clear, u32 set)
 {
u32 old, val;
@@ -399,6 +401,8 @@ static inline void intel_uncore_rmw_fw(struct intel_uncore 
*uncore,
val = (old & ~clear) | set;
if (val != old)
intel_uncore_write_fw(uncore, reg, val);
+
+   return old;
 }

 static inline int intel_uncore_write_and_verify(struct intel_uncore *uncore,

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/37] drm/i915: make intel_{uncore, de}_rmw() more useful

2020-05-21 Thread Souza, Jose 
On Wed, 2020-05-20 at 17:37 -0700, Lucas De Marchi wrote:
> Return the old value read so some places of the code can still do the
> rmw but add warnings/errors about the value it read.
> 
> Signed-off-by: Lucas De Marchi 
> ---
>  drivers/gpu/drm/i915/display/intel_de.h |  4 ++--
>  drivers/gpu/drm/i915/intel_uncore.h | 10 +++---
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_de.h 
> b/drivers/gpu/drm/i915/display/intel_de.h
> index 00da10bf35f5..d5441b1ba2fe 100644
> --- a/drivers/gpu/drm/i915/display/intel_de.h
> +++ b/drivers/gpu/drm/i915/display/intel_de.h
> @@ -42,10 +42,10 @@ intel_de_write_fw(struct drm_i915_private *i915, 
> i915_reg_t reg, u32 val)
>   intel_uncore_write_fw(>uncore, reg, val);
>  }
> 

Maybe add function documentation with this new information about the return?

With that:

Reviewed-by: José Roberto de Souza 

>  
> -static inline void
> +static inline u32
>  intel_de_rmw(struct drm_i915_private *i915, i915_reg_t reg, u32 clear, u32 
> set)
>  {
> - intel_uncore_rmw(>uncore, reg, clear, set);
> + return intel_uncore_rmw(>uncore, reg, clear, set);
>  }
>  
>  static inline int
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h 
> b/drivers/gpu/drm/i915/intel_uncore.h
> index 8d3aa8b9acf9..5da43b56fa11 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -379,8 +379,8 @@ intel_uncore_read64_2x32(struct intel_uncore *uncore,
>  #define intel_uncore_write64_fw(...) __raw_uncore_write64(__VA_ARGS__)
>  #define intel_uncore_posting_read_fw(...) 
> ((void)intel_uncore_read_fw(__VA_ARGS__))
>  
> -static inline void intel_uncore_rmw(struct intel_uncore *uncore,
> - i915_reg_t reg, u32 clear, u32 set)
> +static inline u32 intel_uncore_rmw(struct intel_uncore *uncore,
> +i915_reg_t reg, u32 clear, u32 set)
>  {
>   u32 old, val;
>  
> @@ -388,9 +388,11 @@ static inline void intel_uncore_rmw(struct intel_uncore 
> *uncore,
>   val = (old & ~clear) | set;
>   if (val != old)
>   intel_uncore_write(uncore, reg, val);
> +
> + return old;
>  }
>  
> -static inline void intel_uncore_rmw_fw(struct intel_uncore *uncore,
> +static inline u32 intel_uncore_rmw_fw(struct intel_uncore *uncore,
>  i915_reg_t reg, u32 clear, u32 set)
>  {
>   u32 old, val;
> @@ -399,6 +401,8 @@ static inline void intel_uncore_rmw_fw(struct 
> intel_uncore *uncore,
>   val = (old & ~clear) | set;
>   if (val != old)
>   intel_uncore_write_fw(uncore, reg, val);
> +
> + return old;
>  }
>  
>  static inline int intel_uncore_write_and_verify(struct intel_uncore *uncore,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 08/37] drm/i915: make intel_{uncore, de}_rmw() more useful

2020-05-20 Thread Lucas De Marchi 
Return the old value read so some places of the code can still do the
rmw but add warnings/errors about the value it read.

Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/display/intel_de.h |  4 ++--
 drivers/gpu/drm/i915/intel_uncore.h | 10 +++---
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_de.h 
b/drivers/gpu/drm/i915/display/intel_de.h
index 00da10bf35f5..d5441b1ba2fe 100644
--- a/drivers/gpu/drm/i915/display/intel_de.h
+++ b/drivers/gpu/drm/i915/display/intel_de.h
@@ -42,10 +42,10 @@ intel_de_write_fw(struct drm_i915_private *i915, i915_reg_t 
reg, u32 val)
intel_uncore_write_fw(>uncore, reg, val);
 }
 
-static inline void
+static inline u32
 intel_de_rmw(struct drm_i915_private *i915, i915_reg_t reg, u32 clear, u32 set)
 {
-   intel_uncore_rmw(>uncore, reg, clear, set);
+   return intel_uncore_rmw(>uncore, reg, clear, set);
 }
 
 static inline int
diff --git a/drivers/gpu/drm/i915/intel_uncore.h 
b/drivers/gpu/drm/i915/intel_uncore.h
index 8d3aa8b9acf9..5da43b56fa11 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -379,8 +379,8 @@ intel_uncore_read64_2x32(struct intel_uncore *uncore,
 #define intel_uncore_write64_fw(...) __raw_uncore_write64(__VA_ARGS__)
 #define intel_uncore_posting_read_fw(...) 
((void)intel_uncore_read_fw(__VA_ARGS__))
 
-static inline void intel_uncore_rmw(struct intel_uncore *uncore,
-   i915_reg_t reg, u32 clear, u32 set)
+static inline u32 intel_uncore_rmw(struct intel_uncore *uncore,
+  i915_reg_t reg, u32 clear, u32 set)
 {
u32 old, val;
 
@@ -388,9 +388,11 @@ static inline void intel_uncore_rmw(struct intel_uncore 
*uncore,
val = (old & ~clear) | set;
if (val != old)
intel_uncore_write(uncore, reg, val);
+
+   return old;
 }
 
-static inline void intel_uncore_rmw_fw(struct intel_uncore *uncore,
+static inline u32 intel_uncore_rmw_fw(struct intel_uncore *uncore,
   i915_reg_t reg, u32 clear, u32 set)
 {
u32 old, val;
@@ -399,6 +401,8 @@ static inline void intel_uncore_rmw_fw(struct intel_uncore 
*uncore,
val = (old & ~clear) | set;
if (val != old)
intel_uncore_write_fw(uncore, reg, val);
+
+   return old;
 }
 
 static inline int intel_uncore_write_and_verify(struct intel_uncore *uncore,
-- 
2.26.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx