diff mbox series

[6/6] media: cec: core: add note about *_from_edid() function usage in drm

Message ID 7cebfea8f999d2d0d49533f9849d109830c5d1b6.1692884619.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm, cec and edid updates | expand

Commit Message

Jani Nikula Aug. 24, 2023, 1:46 p.m. UTC
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@xs4all.nl>
Cc: linux-media@vger.kernel.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/media/cec/core/cec-adap.c     | 4 ++++
 drivers/media/cec/core/cec-notifier.c | 4 ++++
 2 files changed, 8 insertions(+)

Comments

Hans Verkuil Aug. 30, 2023, 10:03 a.m. UTC | #1
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@xs4all.nl>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  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'.

>  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.

>  void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n,
>  					  const struct edid *edid)
>  {

Regards,

	Hans
Jani Nikula Aug. 31, 2023, 10:52 a.m. UTC | #2
On Wed, 30 Aug 2023, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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@xs4all.nl>
>> Cc: linux-media@vger.kernel.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  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
diff mbox series

Patch

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.
+ */
 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.
+ */
 void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n,
 					  const struct edid *edid)
 {