Re: [PATCH v2 4/6] drm/i915/lspcon: change signature of lspcon_wait_pcon_mode()

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

On Tue, 04 Feb 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Tue, Feb 04, 2025 at 03:58:41PM +0200, Jani Nikula wrote:
>> Switch from struct intel_lspcon to struct intel_digital_port to unify.
>
> in the previous patch and in this one I'm asking myself if we
> are not going to the opposite direction that we should...
> and make the lspcon component to use lspcon structs as entry
> but looking to the patch itself the code gets cleaner because
> digport is an information that is broadly available. So, let's
> continue:

I had mostly similar thoughts, and conclusions, and settled on this.

> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Thanks, pushed the series to din.

BR,
Jani.

>
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp.c     | 3 +--
>>  drivers/gpu/drm/i915/display/intel_lspcon.c | 4 +++-
>>  drivers/gpu/drm/i915/display/intel_lspcon.h | 4 +---
>>  3 files changed, 5 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 2db7741c25bd..14d81538c912 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -3501,7 +3501,6 @@ void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode)
>>  		ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, mode);
>>  	} else {
>>  		struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> -		struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
>>  
>>  		lspcon_resume(dig_port);
>>  
>> @@ -3520,7 +3519,7 @@ void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode)
>>  		}
>>  
>>  		if (ret == 1 && intel_lspcon_active(dig_port))
>> -			lspcon_wait_pcon_mode(lspcon);
>> +			lspcon_wait_pcon_mode(dig_port);
>>  	}
>>  
>>  	if (ret != 1)
>> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
>> index 2dd2b700b0f3..45b63fea3642 100644
>> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
>> @@ -656,8 +656,10 @@ u32 lspcon_infoframes_enabled(struct intel_encoder *encoder,
>>  	return val;
>>  }
>>  
>> -void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon)
>> +void lspcon_wait_pcon_mode(struct intel_digital_port *dig_port)
>>  {
>> +	struct intel_lspcon *lspcon = &dig_port->lspcon;
>> +
>>  	lspcon_wait_mode(lspcon, DRM_LSPCON_MODE_PCON);
>>  }
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.h b/drivers/gpu/drm/i915/display/intel_lspcon.h
>> index 18cc15e5221d..20581af1ddb2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_lspcon.h
>> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.h
>> @@ -8,18 +8,16 @@
>>  
>>  #include <linux/types.h>
>>  
>> -struct drm_connector;
>>  struct drm_connector_state;
>>  struct intel_crtc_state;
>>  struct intel_digital_port;
>>  struct intel_encoder;
>> -struct intel_lspcon;
>>  
>>  bool lspcon_init(struct intel_digital_port *dig_port);
>>  bool intel_lspcon_active(struct intel_digital_port *dig_port);
>>  bool lspcon_detect_hdr_capability(struct intel_digital_port *dig_port);
>>  void lspcon_resume(struct intel_digital_port *dig_port);
>> -void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
>> +void lspcon_wait_pcon_mode(struct intel_digital_port *dig_port);
>>  void lspcon_write_infoframe(struct intel_encoder *encoder,
>>  			    const struct intel_crtc_state *crtc_state,
>>  			    unsigned int type,
>> -- 
>> 2.39.5
>> 

-- 
Jani Nikula, Intel