On Wed, 30 Aug 2023, Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote:
> On 24/08/2023 15:46, Jani Nikula wrote:
>> In the drm subsystem, the source physical address is, in most cases,
>> available without having to parse the EDID again. Add notes about
>> preferring to use the pre-parsed address instead.
>>
>> Cc: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
>> Cc: linux-media@xxxxxxxxxxxxxxx
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/media/cec/core/cec-adap.c | 4 ++++
>> drivers/media/cec/core/cec-notifier.c | 4 ++++
>> 2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
>> index 241b1621b197..2c627ed611ed 100644
>> --- a/drivers/media/cec/core/cec-adap.c
>> +++ b/drivers/media/cec/core/cec-adap.c
>> @@ -1688,6 +1688,10 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
>> }
>> EXPORT_SYMBOL_GPL(cec_s_phys_addr);
>>
>> +/*
>> + * Note: In the drm subsystem, prefer calling cec_s_phys_addr() with
>> + * connector->display_info.source_physical_address if possible.
>> + */
>
> I would rephrase this:
>
> /*
> * Note: in the drm subsystem, prefer calling (if possible):
> *
> * cec_s_phys_addr(adap, connector->display_info.source_physical_address, false);
> */
>
> I think it is important to indicate that the last argument should be 'false'.
Agreed.
>
>> void cec_s_phys_addr_from_edid(struct cec_adapter *adap,
>> const struct edid *edid)
>> {
>> diff --git a/drivers/media/cec/core/cec-notifier.c b/drivers/media/cec/core/cec-notifier.c
>> index 389dc664b211..13f043b3025b 100644
>> --- a/drivers/media/cec/core/cec-notifier.c
>> +++ b/drivers/media/cec/core/cec-notifier.c
>> @@ -195,6 +195,10 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa)
>> }
>> EXPORT_SYMBOL_GPL(cec_notifier_set_phys_addr);
>>
>> +/*
>> + * Note: In the drm subsystem, prefer calling cec_notifier_set_phys_addr() with
>> + * connector->display_info.source_physical_address if possible.
>> + */
>
> This comment is fine, there is no similar last argument here. But perhaps
> it is good to use a similar format as above. Up to you.
Thanks, rephrased both in v2.
BR,
Jani.
>
>> void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n,
>> const struct edid *edid)
>> {
>
> Regards,
>
> Hans
--
Jani Nikula, Intel Open Source Graphics Center
