Re: [Intel-gfx] [PATCH 4/6] drm/cec: add drm_dp_cec_attach() as the non-edid version of set edid

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

On Wed, 30 Aug 2023, Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 24/08/2023 15:46, Jani Nikula wrote:
>> Connectors have source physical address available in display
>> info. There's no need to parse the EDID again for this. Add
>> drm_dp_cec_attach() to do this.
>> 
>> Seems like the set_edid/unset_edid naming is a bit specific now that
>> there's no need to pass the EDID at all, so aim for attach/detach going
>> forward.
>> 
>> Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> Cc: linux-media@vger.kernel.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/display/drm_dp_cec.c | 22 +++++++++++++++++++---
>>  include/drm/display/drm_dp_helper.h  |  6 ++++++
>>  2 files changed, 25 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/display/drm_dp_cec.c b/drivers/gpu/drm/display/drm_dp_cec.c
>> index ae39dc794190..da7a7d357446 100644
>> --- a/drivers/gpu/drm/display/drm_dp_cec.c
>> +++ b/drivers/gpu/drm/display/drm_dp_cec.c
>> @@ -297,7 +297,7 @@ static void drm_dp_cec_unregister_work(struct work_struct *work)
>>   * were unchanged and just update the CEC physical address. Otherwise
>>   * unregister the old CEC adapter and create a new one.
>>   */
>> -void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>> +void drm_dp_cec_attach(struct drm_dp_aux *aux, u16 source_physical_address)
>>  {
>>  	struct drm_connector *connector = aux->cec.connector;
>>  	u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD |
>> @@ -339,7 +339,7 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>>  		if (aux->cec.adap->capabilities == cec_caps &&
>>  		    aux->cec.adap->available_log_addrs == num_las) {
>>  			/* Unchanged, so just set the phys addr */
>> -			cec_s_phys_addr_from_edid(aux->cec.adap, edid);
>> +			cec_s_phys_addr(adap, source_physical_address, false);
>
> As the kernel test robot indicated, this does not compile, this should
> be aux->cec.adap.
>
>>  			goto unlock;
>>  		}
>>  		/*
>> @@ -370,11 +370,27 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>>  		 * from drm_dp_cec_register_connector() edid == NULL, so in
>>  		 * that case the phys addr is just invalidated.
>>  		 */
>> -		cec_s_phys_addr_from_edid(aux->cec.adap, edid);
>> +		cec_s_phys_addr(adap, source_physical_address, false);
>>  	}
>>  unlock:
>>  	mutex_unlock(&aux->cec.lock);
>>  }
>> +EXPORT_SYMBOL(drm_dp_cec_attach);
>> +
>> +/*
>> + * Note: Prefer calling drm_dp_cec_attach() with
>> + * connector->display_info.source_physical_address if possible.
>> + */
>> +void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>> +{
>> +	u16 source_physical_address = CEC_PHYS_ADDR_INVALID;
>> +
>> +	if (edid && edid->extensions)
>
> And this source needs to include <drm/drm_edid.h>, also as found by
> the kernel test robot.

Yes, very embarrassing, I sent the v2 in reply [1].

BR,
Jani.

[1] https://patchwork.freedesktop.org/patch/msgid/20230825130120.1250089-1-jani.nikula@intel.com


>
> Regards,
>
> 	Hans
>
>> +		pa = cec_get_edid_phys_addr((const u8 *)edid,
>> +					    EDID_LENGTH * (edid->extensions + 1), NULL);
>> +
>> +	drm_dp_cec_attach(aux, source_physical_address);
>> +}
>>  EXPORT_SYMBOL(drm_dp_cec_set_edid);
>>  
>>  /*
>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
>> index 86f24a759268..3369104e2d25 100644
>> --- a/include/drm/display/drm_dp_helper.h
>> +++ b/include/drm/display/drm_dp_helper.h
>> @@ -699,6 +699,7 @@ void drm_dp_cec_irq(struct drm_dp_aux *aux);
>>  void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
>>  				   struct drm_connector *connector);
>>  void drm_dp_cec_unregister_connector(struct drm_dp_aux *aux);
>> +void drm_dp_cec_attach(struct drm_dp_aux *aux, u16 source_physical_address);
>>  void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid);
>>  void drm_dp_cec_unset_edid(struct drm_dp_aux *aux);
>>  #else
>> @@ -716,6 +717,11 @@ static inline void drm_dp_cec_unregister_connector(struct drm_dp_aux *aux)
>>  {
>>  }
>>  
>> +static inline void drm_dp_cec_attach(struct drm_dp_aux *aux,
>> +				     u16 source_physical_address)
>> +{
>> +}
>> +
>>  static inline void drm_dp_cec_set_edid(struct drm_dp_aux *aux,
>>  				       const struct edid *edid)
>>  {
>

-- 
Jani Nikula, Intel Open Source Graphics Center