On Wed, 30 Aug 2023, Hans Verkuil <hverkuil-cisco@xxxxxxxxx> 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@xxxxxxxxx>
>> Cc: linux-media@xxxxxxxxxxxxxxx
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxx
>
> 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
