diff mbox series

[15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

Message ID 20211015113713.630119-16-cssk@net-c.es (mailing list archive)
State New, archived
Headers show
Series replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi | expand

Commit Message

Claudio Suarez Oct. 15, 2021, 11:37 a.m. UTC
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi where possible.

This is a TODO task in Documentation/gpu/todo.rst

Signed-off-by: Claudio Suarez <cssk@net-c.es>
---
 drivers/gpu/drm/i915/display/intel_connector.c | 5 +++++
 drivers/gpu/drm/i915/display/intel_connector.h | 1 +
 drivers/gpu/drm/i915/display/intel_hdmi.c      | 2 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c      | 3 ++-
 4 files changed, 9 insertions(+), 2 deletions(-)

Comments

Ville Syrjala Oct. 15, 2021, 12:30 p.m. UTC | #1
On Fri, Oct 15, 2021 at 01:37:13PM +0200, Claudio Suarez wrote:
> Once EDID is parsed, the monitor HDMI support information is available
> through drm_display_info.is_hdmi. Retriving the same information with
> drm_detect_hdmi_monitor() is less efficient. Change to
> drm_display_info.is_hdmi where possible.
> 
> This is a TODO task in Documentation/gpu/todo.rst
> 
> Signed-off-by: Claudio Suarez <cssk@net-c.es>
> ---
>  drivers/gpu/drm/i915/display/intel_connector.c | 5 +++++
>  drivers/gpu/drm/i915/display/intel_connector.h | 1 +
>  drivers/gpu/drm/i915/display/intel_hdmi.c      | 2 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c      | 3 ++-
>  4 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
> index 9bed1ccecea0..3346b55df6e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.c
> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
>  	return ret;
>  }
>  
> +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> +{
> +	return connector->display_info.is_hdmi;
> +}
> +
>  static const struct drm_prop_enum_list force_audio_names[] = {
>  	{ HDMI_AUDIO_OFF_DVI, "force-dvi" },
>  	{ HDMI_AUDIO_OFF, "off" },
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h
> index 661a37a3c6d8..ceda6e72ece6 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.h
> +++ b/drivers/gpu/drm/i915/display/intel_connector.h
> @@ -27,6 +27,7 @@ enum pipe intel_connector_get_pipe(struct intel_connector *connector);
>  int intel_connector_update_modes(struct drm_connector *connector,
>  				 struct edid *edid);
>  int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
> +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
>  void intel_attach_force_audio_property(struct drm_connector *connector);
>  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
>  void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index b04685bb6439..2b1d7c5bebdd 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>  	to_intel_connector(connector)->detect_edid = edid;
>  	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
>  		intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> -		intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
> +		intel_hdmi->has_hdmi_sink = intel_connector_is_hdmi_monitor(connector);

Hmm. Have we parse the EDID by this point actually? I don't think that
was the case in the past but maybe it changed at some point.

>  
>  		connected = true;
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 6cb27599ea03..a32279e4fee8 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
>  		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
>  			status = connector_status_connected;
>  			if (intel_sdvo_connector->is_hdmi) {
> -				intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid);
>  				intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid);
> +				intel_sdvo->has_hdmi_monitor =
> +					intel_connector_is_hdmi_monitor(connector);

FYI there's a third copy of this in intel_dp.c

>  			}
>  		} else
>  			status = connector_status_disconnected;
> -- 
> 2.33.0
>
Jani Nikula Oct. 15, 2021, 12:44 p.m. UTC | #2
On Fri, 15 Oct 2021, Claudio Suarez <cssk@net-c.es> wrote:
> Once EDID is parsed, the monitor HDMI support information is available
> through drm_display_info.is_hdmi. Retriving the same information with
> drm_detect_hdmi_monitor() is less efficient. Change to
> drm_display_info.is_hdmi where possible.
>
> This is a TODO task in Documentation/gpu/todo.rst
>
> Signed-off-by: Claudio Suarez <cssk@net-c.es>
> ---
>  drivers/gpu/drm/i915/display/intel_connector.c | 5 +++++
>  drivers/gpu/drm/i915/display/intel_connector.h | 1 +
>  drivers/gpu/drm/i915/display/intel_hdmi.c      | 2 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c      | 3 ++-
>  4 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
> index 9bed1ccecea0..3346b55df6e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.c
> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
>  	return ret;
>  }
>  
> +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> +{
> +	return connector->display_info.is_hdmi;
> +}
> +

A helper like this belongs in drm, not i915. Seems useful in other
drivers too.

BR,
Jani.

>  static const struct drm_prop_enum_list force_audio_names[] = {
>  	{ HDMI_AUDIO_OFF_DVI, "force-dvi" },
>  	{ HDMI_AUDIO_OFF, "off" },
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h
> index 661a37a3c6d8..ceda6e72ece6 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.h
> +++ b/drivers/gpu/drm/i915/display/intel_connector.h
> @@ -27,6 +27,7 @@ enum pipe intel_connector_get_pipe(struct intel_connector *connector);
>  int intel_connector_update_modes(struct drm_connector *connector,
>  				 struct edid *edid);
>  int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
> +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
>  void intel_attach_force_audio_property(struct drm_connector *connector);
>  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
>  void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index b04685bb6439..2b1d7c5bebdd 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>  	to_intel_connector(connector)->detect_edid = edid;
>  	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
>  		intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> -		intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
> +		intel_hdmi->has_hdmi_sink = intel_connector_is_hdmi_monitor(connector);
>  
>  		connected = true;
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 6cb27599ea03..a32279e4fee8 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
>  		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
>  			status = connector_status_connected;
>  			if (intel_sdvo_connector->is_hdmi) {
> -				intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid);
>  				intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid);
> +				intel_sdvo->has_hdmi_monitor =
> +					intel_connector_is_hdmi_monitor(connector);
>  			}
>  		} else
>  			status = connector_status_disconnected;
Ville Syrjala Oct. 15, 2021, 12:58 p.m. UTC | #3
On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote:
> On Fri, 15 Oct 2021, Claudio Suarez <cssk@net-c.es> wrote:
> > Once EDID is parsed, the monitor HDMI support information is available
> > through drm_display_info.is_hdmi. Retriving the same information with
> > drm_detect_hdmi_monitor() is less efficient. Change to
> > drm_display_info.is_hdmi where possible.
> >
> > This is a TODO task in Documentation/gpu/todo.rst
> >
> > Signed-off-by: Claudio Suarez <cssk@net-c.es>
> > ---
> >  drivers/gpu/drm/i915/display/intel_connector.c | 5 +++++
> >  drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> >  drivers/gpu/drm/i915/display/intel_hdmi.c      | 2 +-
> >  drivers/gpu/drm/i915/display/intel_sdvo.c      | 3 ++-
> >  4 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
> > index 9bed1ccecea0..3346b55df6e1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
> >  	return ret;
> >  }
> >  
> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> > +{
> > +	return connector->display_info.is_hdmi;
> > +}
> > +
> 
> A helper like this belongs in drm, not i915. Seems useful in other
> drivers too.

Not sure it's actually helpful for i915. We end up having to root around
in the display_info in a lot of places anyway. So a helper for single
boolean seems a bit out of place perhaps.
Jani Nikula Oct. 15, 2021, 3:18 p.m. UTC | #4
On Fri, 15 Oct 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote:
>> On Fri, 15 Oct 2021, Claudio Suarez <cssk@net-c.es> wrote:
>> > Once EDID is parsed, the monitor HDMI support information is available
>> > through drm_display_info.is_hdmi. Retriving the same information with
>> > drm_detect_hdmi_monitor() is less efficient. Change to
>> > drm_display_info.is_hdmi where possible.
>> >
>> > This is a TODO task in Documentation/gpu/todo.rst
>> >
>> > Signed-off-by: Claudio Suarez <cssk@net-c.es>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_connector.c | 5 +++++
>> >  drivers/gpu/drm/i915/display/intel_connector.h | 1 +
>> >  drivers/gpu/drm/i915/display/intel_hdmi.c      | 2 +-
>> >  drivers/gpu/drm/i915/display/intel_sdvo.c      | 3 ++-
>> >  4 files changed, 9 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
>> > index 9bed1ccecea0..3346b55df6e1 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
>> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
>> >  	return ret;
>> >  }
>> >  
>> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
>> > +{
>> > +	return connector->display_info.is_hdmi;
>> > +}
>> > +
>> 
>> A helper like this belongs in drm, not i915. Seems useful in other
>> drivers too.
>
> Not sure it's actually helpful for i915. We end up having to root around
> in the display_info in a lot of places anyway. So a helper for single
> boolean seems a bit out of place perhaps.

*shrug*

Maybe it's just my frustration at the lack of interfaces and poking
around in the depths of nested structs and pointer chasing that's coming
through. You just need to change so many things if you want to later
refactor where "is hdmi" comes from and is stored.

Anyway, if a helper is being added like in this series, I think it
should be one helper in drm, not redundant copies in multiple
drivers. Or we should not have the helper(s) at all. One or the other,
not the worst of both worlds.


BR,
Jani.
Claudio Suarez Oct. 15, 2021, 6:18 p.m. UTC | #5
On Fri, Oct 15, 2021 at 03:30:49PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 15, 2021 at 01:37:13PM +0200, Claudio Suarez wrote:
> > Once EDID is parsed, the monitor HDMI support information is available
> > through drm_display_info.is_hdmi. Retriving the same information with
> > drm_detect_hdmi_monitor() is less efficient. Change to
> > drm_display_info.is_hdmi where possible.
> > 
> > This is a TODO task in Documentation/gpu/todo.rst
> > 
> > Signed-off-by: Claudio Suarez <cssk@net-c.es>
> > ---
> >  drivers/gpu/drm/i915/display/intel_connector.c | 5 +++++
> >  drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> >  drivers/gpu/drm/i915/display/intel_hdmi.c      | 2 +-
> >  drivers/gpu/drm/i915/display/intel_sdvo.c      | 3 ++-
> >  4 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
> > index 9bed1ccecea0..3346b55df6e1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
> >  	return ret;
> >  }
> >  
> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> > +{
> > +	return connector->display_info.is_hdmi;
> > +}
> > +
> >  static const struct drm_prop_enum_list force_audio_names[] = {
> >  	{ HDMI_AUDIO_OFF_DVI, "force-dvi" },
> >  	{ HDMI_AUDIO_OFF, "off" },
> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h
> > index 661a37a3c6d8..ceda6e72ece6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_connector.h
> > +++ b/drivers/gpu/drm/i915/display/intel_connector.h
> > @@ -27,6 +27,7 @@ enum pipe intel_connector_get_pipe(struct intel_connector *connector);
> >  int intel_connector_update_modes(struct drm_connector *connector,
> >  				 struct edid *edid);
> >  int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
> >  void intel_attach_force_audio_property(struct drm_connector *connector);
> >  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> >  void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index b04685bb6439..2b1d7c5bebdd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> >  	to_intel_connector(connector)->detect_edid = edid;
> >  	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
> >  		intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> > -		intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
> > +		intel_hdmi->has_hdmi_sink = intel_connector_is_hdmi_monitor(connector);
> 
> Hmm. Have we parse the EDID by this point actually? I don't think that
> was the case in the past but maybe it changed at some point.

Yes, I think so. The complete code is:

----
        edid = drm_get_edid(connector, i2c);

        if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
                drm_dbg_kms(&dev_priv->drm,
                            "HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
                intel_gmbus_force_bit(i2c, true);
                edid = drm_get_edid(connector, i2c);
                intel_gmbus_force_bit(i2c, false);
        }

        intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);

        intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);

        to_intel_connector(connector)->detect_edid = edid;
        if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
                intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
                intel_hdmi->has_hdmi_sink = intel_connector_is_hdmi_monitor(connector);
----
The edid value comes from drm_get_edid(), first or second.
drm_get_edid() internally calls drm_connector_update_edid_property()
drm_connector_update_edid_property() calls drm_add_display_info() and parses the edid.
So, the edid is parsed.
I checked this and I read the docs many times because at the first time I felt something
was wrong. But that is the sequence of calls.

> 
> >  
> >  		connected = true;
> >  	}
> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > index 6cb27599ea03..a32279e4fee8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > @@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
> >  		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
> >  			status = connector_status_connected;
> >  			if (intel_sdvo_connector->is_hdmi) {
> > -				intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid);
> >  				intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid);
> > +				intel_sdvo->has_hdmi_monitor =
> > +					intel_connector_is_hdmi_monitor(connector);
> 
> FYI there's a third copy of this in intel_dp.c

Yes. But in this case the edid comes from intel_dp_get_edid().
In intel_dp_get_edid(), there is a if. One of the branches calls drm_get_edid(),
so no problem here. But the other branch gets the edid from drm_edid_duplicate().
I haven't seen any guarantee that display_info is updated in this case.

I didn't change this file for that reason.

Thank you for your comments :)

BR
Claudio Suarez.
Claudio Suarez Oct. 15, 2021, 6:44 p.m. UTC | #6
On Fri, Oct 15, 2021 at 06:18:34PM +0300, Jani Nikula wrote:
> On Fri, 15 Oct 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote:
> >> On Fri, 15 Oct 2021, Claudio Suarez <cssk@net-c.es> wrote:
> >> > Once EDID is parsed, the monitor HDMI support information is available
> >> > through drm_display_info.is_hdmi. Retriving the same information with
> >> > drm_detect_hdmi_monitor() is less efficient. Change to
> >> > drm_display_info.is_hdmi where possible.
> >> >
> >> > This is a TODO task in Documentation/gpu/todo.rst
> >> >
> >> > Signed-off-by: Claudio Suarez <cssk@net-c.es>
> >> > ---
> >> >  drivers/gpu/drm/i915/display/intel_connector.c | 5 +++++
> >> >  drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> >> >  drivers/gpu/drm/i915/display/intel_hdmi.c      | 2 +-
> >> >  drivers/gpu/drm/i915/display/intel_sdvo.c      | 3 ++-
> >> >  4 files changed, 9 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
> >> > index 9bed1ccecea0..3346b55df6e1 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> >> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
> >> >  	return ret;
> >> >  }
> >> >  
> >> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> >> > +{
> >> > +	return connector->display_info.is_hdmi;
> >> > +}
> >> > +
> >> 
> >> A helper like this belongs in drm, not i915. Seems useful in other
> >> drivers too.
> >
> > Not sure it's actually helpful for i915. We end up having to root around
> > in the display_info in a lot of places anyway. So a helper for single
> > boolean seems a bit out of place perhaps.
> 
> *shrug*
> 
> Maybe it's just my frustration at the lack of interfaces and poking
> around in the depths of nested structs and pointer chasing that's coming
> through. You just need to change so many things if you want to later
> refactor where "is hdmi" comes from and is stored.
> 
> Anyway, if a helper is being added like in this series, I think it
> should be one helper in drm, not redundant copies in multiple
> drivers. Or we should not have the helper(s) at all. One or the other,
> not the worst of both worlds.

Thank you all for your comments :)
The big work here was to figure out which drm_detect_hdmi_monitor() can be
replaced. Changing a helper isn't a problem.
I'll send a new patch in a few hours.

BR
Claudio Suarez.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
index 9bed1ccecea0..3346b55df6e1 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -213,6 +213,11 @@  int intel_ddc_get_modes(struct drm_connector *connector,
 	return ret;
 }
 
+bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
+{
+	return connector->display_info.is_hdmi;
+}
+
 static const struct drm_prop_enum_list force_audio_names[] = {
 	{ HDMI_AUDIO_OFF_DVI, "force-dvi" },
 	{ HDMI_AUDIO_OFF, "off" },
diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h
index 661a37a3c6d8..ceda6e72ece6 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.h
+++ b/drivers/gpu/drm/i915/display/intel_connector.h
@@ -27,6 +27,7 @@  enum pipe intel_connector_get_pipe(struct intel_connector *connector);
 int intel_connector_update_modes(struct drm_connector *connector,
 				 struct edid *edid);
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
+bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
 void intel_attach_aspect_ratio_property(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..2b1d7c5bebdd 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2355,7 +2355,7 @@  intel_hdmi_set_edid(struct drm_connector *connector)
 	to_intel_connector(connector)->detect_edid = edid;
 	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
 		intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
-		intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
+		intel_hdmi->has_hdmi_sink = intel_connector_is_hdmi_monitor(connector);
 
 		connected = true;
 	}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..a32279e4fee8 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2060,8 +2060,9 @@  intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
 		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
 			status = connector_status_connected;
 			if (intel_sdvo_connector->is_hdmi) {
-				intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid);
 				intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid);
+				intel_sdvo->has_hdmi_monitor =
+					intel_connector_is_hdmi_monitor(connector);
 			}
 		} else
 			status = connector_status_disconnected;