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

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

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

-- 
Jani Nikula, Intel Open Source Graphics Center