diff mbox series

[1/6] drm/edid: add drm_edid_is_digital()

Message ID 4bdb407bf189fd922be022eb2f9564692377c81d.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 
Checking edid->input & DRM_EDID_INPUT_DIGITAL is common enough to
deserve a helper that also lets us abstract the raw EDID a bit better.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 17 +++++++++++++++--
 include/drm/drm_edid.h     |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

Comments

Ville Syrjala Aug. 31, 2023, 5:23 p.m. UTC | #1 
On Thu, Aug 24, 2023 at 04:46:02PM +0300, Jani Nikula wrote:
> Checking edid->input & DRM_EDID_INPUT_DIGITAL is common enough to
> deserve a helper that also lets us abstract the raw EDID a bit better.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/drm_edid.c | 17 +++++++++++++++--
>  include/drm/drm_edid.h     |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 340da8257b51..1dbb15439468 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3110,7 +3110,7 @@ drm_monitor_supports_rb(const struct drm_edid *drm_edid)
>  		return ret;
>  	}
>  
> -	return ((drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
> +	return drm_edid_is_digital(drm_edid);
>  }
>  
>  static void
> @@ -6519,7 +6519,7 @@ static void update_display_info(struct drm_connector *connector,
>  	if (edid->revision < 3)
>  		goto out;
>  
> -	if (!(edid->input & DRM_EDID_INPUT_DIGITAL))
> +	if (!drm_edid_is_digital(drm_edid))
>  		goto out;
>  
>  	info->color_formats |= DRM_COLOR_FORMAT_RGB444;
> @@ -7335,3 +7335,16 @@ static void _drm_update_tile_info(struct drm_connector *connector,
>  		connector->tile_group = NULL;
>  	}
>  }
> +
> +/**
> + * drm_edid_is_digital - is digital?
> + * @drm_edid: The EDID
> + *
> + * Return true if input is digital.
> + */
> +bool drm_edid_is_digital(const struct drm_edid *drm_edid)
> +{
> +	return drm_edid && drm_edid->edid &&
> +		drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL;
> +}
> +EXPORT_SYMBOL(drm_edid_is_digital);
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 48e93f909ef6..882d2638708e 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -612,6 +612,7 @@ const struct drm_edid *drm_edid_read_switcheroo(struct drm_connector *connector,
>  int drm_edid_connector_update(struct drm_connector *connector,
>  			      const struct drm_edid *edid);
>  int drm_edid_connector_add_modes(struct drm_connector *connector);
> +bool drm_edid_is_digital(const struct drm_edid *drm_edid);
>  
>  const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
>  				  int ext_id, int *ext_index);
> -- 
> 2.39.2
Alex Deucher Sept. 1, 2023, 7:26 p.m. UTC | #2 
On Thu, Aug 24, 2023 at 9:46 AM Jani Nikula <jani.nikula@intel.com> wrote:
>
> Checking edid->input & DRM_EDID_INPUT_DIGITAL is common enough to
> deserve a helper that also lets us abstract the raw EDID a bit better.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Seems to be a few additional users of this that could be converted:

drivers/gpu/drm/i915/display/intel_sdvo.c:        if (edid &&
edid->input & DRM_EDID_INPUT_DIGITAL)
drivers/gpu/drm/i915/display/intel_sdvo.c:    bool monitor_is_digital
= !!(edid->input & DRM_EDID_INPUT_DIGITAL);
drivers/gpu/drm/i915/display/intel_crt.c:        bool is_digital =
edid->input & DRM_EDID_INPUT_DIGITAL;
drivers/gpu/drm/i915/display/intel_hdmi.c:    if (edid && edid->input
& DRM_EDID_INPUT_DIGITAL) {
drivers/gpu/drm/gma500/psb_intel_sdvo.c:        if (edid->input &
DRM_EDID_INPUT_DIGITAL) {
drivers/gpu/drm/gma500/psb_intel_sdvo.c:            if (edid->input &
DRM_EDID_INPUT_DIGITAL)
drivers/gpu/drm/gma500/psb_intel_sdvo.c:        bool
monitor_is_digital = !!(edid->input & DRM_EDID_INPUT_DIGITAL);
drivers/gpu/drm/gma500/psb_intel_sdvo.c:    if (edid != NULL &&
edid->input & DRM_EDID_INPUT_DIGITAL)
drivers/gpu/drm/gma500/cdv_intel_hdmi.c:        if (edid->input &
DRM_EDID_INPUT_DIGITAL) {
drivers/gpu/drm/display/drm_dp_helper.c:        edid->input &
DRM_EDID_INPUT_DIGITAL &&
drivers/gpu/drm/nouveau/nouveau_connector.c:            if
(nv_connector->edid->input & DRM_EDID_INPUT_DIGITAL)
drivers/gpu/drm/radeon/radeon_connectors.c:
!!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
drivers/gpu/drm/radeon/radeon_connectors.c:
!!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c:
!!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c:
!!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL);




> ---
>  drivers/gpu/drm/drm_edid.c | 17 +++++++++++++++--
>  include/drm/drm_edid.h     |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 340da8257b51..1dbb15439468 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3110,7 +3110,7 @@ drm_monitor_supports_rb(const struct drm_edid *drm_edid)
>                 return ret;
>         }
>
> -       return ((drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
> +       return drm_edid_is_digital(drm_edid);
>  }
>
>  static void
> @@ -6519,7 +6519,7 @@ static void update_display_info(struct drm_connector *connector,
>         if (edid->revision < 3)
>                 goto out;
>
> -       if (!(edid->input & DRM_EDID_INPUT_DIGITAL))
> +       if (!drm_edid_is_digital(drm_edid))
>                 goto out;
>
>         info->color_formats |= DRM_COLOR_FORMAT_RGB444;
> @@ -7335,3 +7335,16 @@ static void _drm_update_tile_info(struct drm_connector *connector,
>                 connector->tile_group = NULL;
>         }
>  }
> +
> +/**
> + * drm_edid_is_digital - is digital?
> + * @drm_edid: The EDID
> + *
> + * Return true if input is digital.
> + */
> +bool drm_edid_is_digital(const struct drm_edid *drm_edid)
> +{
> +       return drm_edid && drm_edid->edid &&
> +               drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL;
> +}
> +EXPORT_SYMBOL(drm_edid_is_digital);
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 48e93f909ef6..882d2638708e 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -612,6 +612,7 @@ const struct drm_edid *drm_edid_read_switcheroo(struct drm_connector *connector,
>  int drm_edid_connector_update(struct drm_connector *connector,
>                               const struct drm_edid *edid);
>  int drm_edid_connector_add_modes(struct drm_connector *connector);
> +bool drm_edid_is_digital(const struct drm_edid *drm_edid);
>
>  const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
>                                   int ext_id, int *ext_index);
> --
> 2.39.2
>
Jani Nikula Sept. 4, 2023, 10:43 a.m. UTC | #3 
On Fri, 01 Sep 2023, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Aug 24, 2023 at 9:46 AM Jani Nikula <jani.nikula@intel.com> wrote:
>>
>> Checking edid->input & DRM_EDID_INPUT_DIGITAL is common enough to
>> deserve a helper that also lets us abstract the raw EDID a bit better.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Thanks; I'm afraid this got merged already.

> Seems to be a few additional users of this that could be converted:
>
> drivers/gpu/drm/i915/display/intel_sdvo.c:        if (edid &&
> edid->input & DRM_EDID_INPUT_DIGITAL)
> drivers/gpu/drm/i915/display/intel_sdvo.c:    bool monitor_is_digital
> = !!(edid->input & DRM_EDID_INPUT_DIGITAL);
> drivers/gpu/drm/i915/display/intel_crt.c:        bool is_digital =
> edid->input & DRM_EDID_INPUT_DIGITAL;
> drivers/gpu/drm/i915/display/intel_hdmi.c:    if (edid && edid->input
> & DRM_EDID_INPUT_DIGITAL) {

The next patch takes care of these.

> drivers/gpu/drm/gma500/psb_intel_sdvo.c:        if (edid->input &
> DRM_EDID_INPUT_DIGITAL) {
> drivers/gpu/drm/gma500/psb_intel_sdvo.c:            if (edid->input &
> DRM_EDID_INPUT_DIGITAL)
> drivers/gpu/drm/gma500/psb_intel_sdvo.c:        bool
> monitor_is_digital = !!(edid->input & DRM_EDID_INPUT_DIGITAL);
> drivers/gpu/drm/gma500/psb_intel_sdvo.c:    if (edid != NULL &&
> edid->input & DRM_EDID_INPUT_DIGITAL)
> drivers/gpu/drm/gma500/cdv_intel_hdmi.c:        if (edid->input &
> DRM_EDID_INPUT_DIGITAL) {
> drivers/gpu/drm/display/drm_dp_helper.c:        edid->input &
> DRM_EDID_INPUT_DIGITAL &&
> drivers/gpu/drm/nouveau/nouveau_connector.c:            if
> (nv_connector->edid->input & DRM_EDID_INPUT_DIGITAL)
> drivers/gpu/drm/radeon/radeon_connectors.c:
> !!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
> drivers/gpu/drm/radeon/radeon_connectors.c:
> !!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c:
> !!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c:
> !!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL);

drm_edid_is_digital() operates on struct drm_edid.

The drivers would first need to be converted to use struct drm_edid
instead of struct edid, and I'm not really taking that on.

IMO adding helpers to operate on struct edid would be
counter-productive.

BR,
Jani.

>
>
>
>
>> ---
>>  drivers/gpu/drm/drm_edid.c | 17 +++++++++++++++--
>>  include/drm/drm_edid.h     |  1 +
>>  2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 340da8257b51..1dbb15439468 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3110,7 +3110,7 @@ drm_monitor_supports_rb(const struct drm_edid *drm_edid)
>>                 return ret;
>>         }
>>
>> -       return ((drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
>> +       return drm_edid_is_digital(drm_edid);
>>  }
>>
>>  static void
>> @@ -6519,7 +6519,7 @@ static void update_display_info(struct drm_connector *connector,
>>         if (edid->revision < 3)
>>                 goto out;
>>
>> -       if (!(edid->input & DRM_EDID_INPUT_DIGITAL))
>> +       if (!drm_edid_is_digital(drm_edid))
>>                 goto out;
>>
>>         info->color_formats |= DRM_COLOR_FORMAT_RGB444;
>> @@ -7335,3 +7335,16 @@ static void _drm_update_tile_info(struct drm_connector *connector,
>>                 connector->tile_group = NULL;
>>         }
>>  }
>> +
>> +/**
>> + * drm_edid_is_digital - is digital?
>> + * @drm_edid: The EDID
>> + *
>> + * Return true if input is digital.
>> + */
>> +bool drm_edid_is_digital(const struct drm_edid *drm_edid)
>> +{
>> +       return drm_edid && drm_edid->edid &&
>> +               drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL;
>> +}
>> +EXPORT_SYMBOL(drm_edid_is_digital);
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> index 48e93f909ef6..882d2638708e 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -612,6 +612,7 @@ const struct drm_edid *drm_edid_read_switcheroo(struct drm_connector *connector,
>>  int drm_edid_connector_update(struct drm_connector *connector,
>>                               const struct drm_edid *edid);
>>  int drm_edid_connector_add_modes(struct drm_connector *connector);
>> +bool drm_edid_is_digital(const struct drm_edid *drm_edid);
>>
>>  const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
>>                                   int ext_id, int *ext_index);
>> --
>> 2.39.2
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 340da8257b51..1dbb15439468 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3110,7 +3110,7 @@  drm_monitor_supports_rb(const struct drm_edid *drm_edid)
 		return ret;
 	}
 
-	return ((drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
+	return drm_edid_is_digital(drm_edid);
 }
 
 static void
@@ -6519,7 +6519,7 @@  static void update_display_info(struct drm_connector *connector,
 	if (edid->revision < 3)
 		goto out;
 
-	if (!(edid->input & DRM_EDID_INPUT_DIGITAL))
+	if (!drm_edid_is_digital(drm_edid))
 		goto out;
 
 	info->color_formats |= DRM_COLOR_FORMAT_RGB444;
@@ -7335,3 +7335,16 @@  static void _drm_update_tile_info(struct drm_connector *connector,
 		connector->tile_group = NULL;
 	}
 }
+
+/**
+ * drm_edid_is_digital - is digital?
+ * @drm_edid: The EDID
+ *
+ * Return true if input is digital.
+ */
+bool drm_edid_is_digital(const struct drm_edid *drm_edid)
+{
+	return drm_edid && drm_edid->edid &&
+		drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL;
+}
+EXPORT_SYMBOL(drm_edid_is_digital);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 48e93f909ef6..882d2638708e 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -612,6 +612,7 @@  const struct drm_edid *drm_edid_read_switcheroo(struct drm_connector *connector,
 int drm_edid_connector_update(struct drm_connector *connector,
 			      const struct drm_edid *edid);
 int drm_edid_connector_add_modes(struct drm_connector *connector);
+bool drm_edid_is_digital(const struct drm_edid *drm_edid);
 
 const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
 				  int ext_id, int *ext_index);