Re: [PATCH 11/11] drm/i915/ddi: simplify intel_ddi_get_encoder_pipes() slightly

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 19 Nov 2024, Imre Deak <imre.deak@intel.com> wrote:
> On Mon, Nov 18, 2024 at 04:49:06PM +0200, Jani Nikula wrote:
>> Use a temporary variable for DDI mode to simplify the conditions. This
>> is in line with the other places that read DDI mode.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_ddi.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index 33628cbc0f72..e25b712bf03b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -818,7 +818,7 @@ static void intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
>>  	mst_pipe_mask = 0;
>>  	for_each_pipe(dev_priv, p) {
>>  		enum transcoder cpu_transcoder = (enum transcoder)p;
>> -		unsigned int port_mask, ddi_select;
>> +		u32 port_mask, ddi_select, ddi_mode;
>>  		intel_wakeref_t trans_wakeref;
>>  
>>  		trans_wakeref = intel_display_power_get_if_enabled(dev_priv,
>> @@ -842,9 +842,10 @@ static void intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
>>  		if ((tmp & port_mask) != ddi_select)
>>  			continue;
>>  
>> -		if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST ||
>> -		    (HAS_DP20(display) &&
>> -		     (tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_FDI_OR_128B132B))
>> +		ddi_mode = tmp & TRANS_DDI_MODE_SELECT_MASK;
>> +
>> +		if (ddi_mode == TRANS_DDI_MODE_SELECT_DP_MST ||
>> +		    (ddi_mode == TRANS_DDI_MODE_SELECT_FDI_OR_128B132B && HAS_DP20(display)))
>
> nit: the above condition and the fdi counterpart is used elsewhere too,
> so could use a helper. The patchset looks ok regardless:
>
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Hum, so was this for the entire series? Also how about v2 of patch 6?

Please consider either replying to the cover letter or for each
individual patch, so that b4 gather the trailers automagically.

BR,
Jani.


>
>>  			mst_pipe_mask |= BIT(p);
>>  
>>  		*pipe_mask |= BIT(p);
>> -- 
>> 2.39.5
>> 

-- 
Jani Nikula, Intel