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

From: Lucas De Marchi <lucas.demarchi@intel.com>
To: "Souza, Jose" <jose.souza@intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Auld, Matthew" <matthew.auld@intel.com>,
	"fernando.pacheco@intel.com" <fernando.pacheco@intel.com>
Subject: Re: [Intel-gfx] [PATCH 08/37] drm/i915: make intel_{uncore, de}_rmw() more useful
Date: Thu, 21 May 2020 10:30:58 -0700	[thread overview]
Message-ID: <20200521173058.lsncnnkw343bsofw@ldmartin-desk1> (raw)
In-Reply-To: <7a5b61a80cd1839cf4cbf0891805d620b9bbdc96.camel@intel.com>

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 <lucas.demarchi@intel.com>
>> ---
>>  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(&i915->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 <jose.souza@intel.com>
>
>>
>> -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(&i915->uncore, reg, clear, set);
>> +	return intel_uncore_rmw(&i915->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