Re: [Nouveau] [PATCH v2 06/13] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

On Wed, Oct 27, 2021 at 07:28:45AM +0900, Inki Dae wrote:
> Hi,
> 
> 21. 10. 17. 오전 3:42에 Claudio Suarez 이(가) 쓴 글:
> > 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
> > 
> > Signed-off-by: Claudio Suarez 
> > ---
> >  drivers/gpu/drm/exynos/exynos_hdmi.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(
> > 
> > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
> > b/drivers/gpu/drm/exynos/exynos_hdmi.c
> > index 7655142a4651..a563d6386abe 100644
> > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> > @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector 
> > *connector)
> > if (!edid)
> > return -ENODEV;
> >  
> > -   hdata->dvi_mode = !drm_detect_hdmi_monitor(edid);
> > +   /* This updates connector->display_info */
> > +   drm_connector_update_edid_property(connector, edid);
> > +
> > +   hdata->dvi_mode = !connector->display_info.is_hdmi;
> 
> Thanks for correcting this. Yeah, we should use drm_display_info.is_hdmi 
> parsed from EDID.
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/drm_edid.c?h=v5.14.14#n4725
> 
> Signed-off-by: Inki Dae 


Thank you, Inki.

Best regards
Claudio Suarez

Re: [Nouveau] [PATCH v4 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

Commit a92d083d08b0 created the new flag is_hdmi in drm_display_info
which is set when sink compliant with CEA-861 (EDID) will be treated
as an HDMI sink.

>From that day, this value can be used in some cases instead of
calling drm_detect_hdmi_monitor() and a second parse is avoided
because drm_detect_hdmi_monitor() parses. A TODO task was
registered in Documentation/gpu/todo.rst to perform that task in
the future.

The flag drm_display_info.is_hdmi is set in the function
drm_add_display_info(), which is called from
drm_connector_update_edid_property(). Since commit 5186421cbfe2,
drm_get_edid() calls drm_connector_update_edid_property() when
reading the edid data from an i2c adapter. Therefore, in these
cases drm_display_info.is_hdmi is updated to its correct
value when returning from drm_get_edid().

Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
in the cases when drm_detect_hdmi_monitor() is called after a
read from an i2c adapter using drm_get_edid() in the i915 driver.

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..008e5b0ba408 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 = connector->display_info.is_hdmi;
 
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c 
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..b4065e4df644 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 =
+   
connector->display_info.is_hdmi;
}
} else
status = connector_status_disconnected;
-- 
2.33.0

Re: [Nouveau] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

On Mon, Oct 25, 2021 at 12:17:37AM +0200, Claudio Suarez wrote:
[...]

No new comments about this, I suppose everything is fine.

I'm going to send the patch with this changes. Thanks to all and
special thanks to you, Ville. Hope this helps the kernel.
Don't hesitate to ask new changes if necessary.

Best regards
Claudio Suarez

Re: [Nouveau] [Intel-gfx] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

On Fri, Oct 22, 2021 at 03:22:57PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 22, 2021 at 03:01:52PM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 22, 2021 at 12:25:33PM +0200, Claudio Suarez wrote:
> > > On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
> > > > > drm_get_edid() internally calls to 
> > > > > drm_connector_update_edid_property()
> > > > > and then drm_add_display_info(), which parses the EDID.
> > > > > This happens in the function intel_hdmi_set_edid() and
> > > > > intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
> > > > > 
> > > > > 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
> > > > 
> > > > I meant we need to examine all call chains that can lead to
> > > > .detect() to make sure all of them do in fact update the
> > > > display_info beforehand.
> > > 
> > > Well, I studied it carefully and, yes, all call chains that can lead to
> > > drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info
> > > beforehand. In the case that this doesn't happen, the code is unchanged.
> > > 
> > > Do you want I explain the changes in the code here again ? Or do you want
> > > to me change the commit message to be more clear ? In the first case, I 
> > > can
> > > write here a detailed explanation. In the second case I can make a longer 
> > > commit
> > > message.
> > > 
> > > Or both?
> > 
> > I want all those call chains explained in the commit message,
> > otherwise I have no easy way to confirm whether the change
> > is correct or not.
> 
> Hmm. OK, so I had a bit of a dig around and seems that what we do now
> .detect()->drm_get_edid()->drm_connector_update_edid_property()->drm_add_display_info()

Yes. I said before that I felt something was wrong when I read the
documentation and then the code. To be more explicit now, I expected that
drm_connector_update_edid_property() will be done in the
fill_modes/get_modes phase instead of when reading the edid.
The documentation suggests that but the code reads the edid in the
detect phase.
Now, since drm_connector_update_edid_property() is called in the detect
phase, it is not necessary to keep the edid data in the private connector
struct. It is in struct drm_connector from the beginning.
But this is topic for another patch.

> Now the question is when did that start happening? Looks like it was
> commit 4b4df570b41d ("drm: Update edid-derived drm_display_info fields
> at edid property set [v2]") that started to call drm_add_display_info()
> from drm_connector_update_edid_property(), and then commit 5186421cbfe2
> ("drm: Introduce epoch counter to drm_connector") started to call
> drm_connector_update_edid_property() from drm_get_edid(). Before both
> of those commits were in place display_info would still contain
> some stale garbage during .detect().
>
> That is the story I think we want in these commit messages since it
> a) explains why the old code was directly parsing the edid instead
> b) why it's now safe to change this

--commit-message?

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

Commit a92d083d08b0 created the new flag is_hdmi in drm_display_info
which is set when sink compliant with CEA-861 (EDID) shall be treated
as an HDMI sink.

>From that day, this value can be used in some cases instead of
calling drm_detect_hdmi_monitor() and a second parse is avoided
because drm_detect_hdmi_monitor() parses. A TODO task was
registered in Documentation/gpu/todo.rst to perform that task in
the future.

The flag drm_display_info.is_hdmi is set in the function
drm_add_display_info(), which is called from
drm_connector_update_edid_property(). Since commit 5186421cbfe2,
drm_get_edid() calls drm_connector_update_edid_property() when
reading the edid data from an i2c adapter. Therefore, in these
cases drm_display_info.is_hdmi is updated to its correct
value when returning from drm_get_edid().

Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
in the cases when drm_detect_hdmi_monitor() is called after a
read from an i2c adapter using drm_get_edid() in the i915 driver.
---

> 
> PS. connector->force handling in drm_get_edid() looks a bit busted
> since it doesn't call drm_connec

Re: [Nouveau] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
> > drm_get_edid() internally calls to drm_connector_update_edid_property()
> > and then drm_add_display_info(), which parses the EDID.
> > This happens in the function intel_hdmi_set_edid() and
> > intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
> > 
> > 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
> 
> I meant we need to examine all call chains that can lead to
> .detect() to make sure all of them do in fact update the
> display_info beforehand.

Well, I studied it carefully and, yes, all call chains that can lead to
drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info
beforehand. In the case that this doesn't happen, the code is unchanged.

Do you want I explain the changes in the code here again ? Or do you want
to me change the commit message to be more clear ? In the first case, I can
write here a detailed explanation. In the second case I can make a longer commit
message.

Or both?

Best Regards,
Claudio Suarez.

Re: [Nouveau] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

drm_get_edid() internally calls to drm_connector_update_edid_property()
and then drm_add_display_info(), which parses the EDID.
This happens in the function intel_hdmi_set_edid() and
intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).

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

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..008e5b0ba408 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 = connector->display_info.is_hdmi;
 
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c 
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..b4065e4df644 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 =
+   
connector->display_info.is_hdmi;
}
} else
status = connector_status_disconnected;
-- 
2.33.0

Re: [Nouveau] [PATCH v3 01/13] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info

[Claudio Suarez]

According to the documentation, drm_add_edid_modes
"... Also fills out the _display_info structure and ELD in @connector
with any information which can be derived from the edid."

drm_add_edid_modes accepts a struct edid *edid parameter which may have a
value or may be null. When it is not null, connector->display_info and
connector->eld are updated according to the edid. When edid=NULL, only
connector->eld is reset. Reset connector->display_info to be consistent
and accurate.

Since drm_edid_is_valid() considers NULL as an invalid EDID, simplify the
code to avoid duplicating code in the case of NULL/error.

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/drm_edid.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6325877c5fd6..a019a26ede7a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5356,14 +5356,14 @@ int drm_add_edid_modes(struct drm_connector *connector, 
struct edid *edid)
int num_modes = 0;
u32 quirks;
 
-   if (edid == NULL) {
-   clear_eld(connector);
-   return 0;
-   }
if (!drm_edid_is_valid(edid)) {
+   /* edid == NULL or invalid here */
clear_eld(connector);
-   drm_warn(connector->dev, "%s: EDID invalid.\n",
-connector->name);
+   drm_reset_display_info(connector);
+   if (edid)
+   drm_warn(connector->dev,
+"[CONNECTOR:%d:%s] EDID invalid.\n",
+connector->base.id, connector->name);
return 0;
}
 
-- 
2.33.0

Re: [Nouveau] [PATCH v2 01/13] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info

[Claudio Suarez]

On Tue, Oct 19, 2021 at 09:35:08PM +0300, Ville Syrjälä wrote:
> On Sat, Oct 16, 2021 at 08:42:14PM +0200, Claudio Suarez wrote:
> > According to the documentation, drm_add_edid_modes
> > "... Also fills out the _display_info structure and ELD in @connector
> > with any information which can be derived from the edid."
> > 
> > drm_add_edid_modes accepts a struct edid *edid parameter which may have a
> > value or may be null. When it is not null, connector->display_info and
> > connector->eld are updated according to the edid. When edid=NULL, only
> > connector->eld is reset. Reset connector->display_info to be consistent
> > and accurate.
> > 
> > Signed-off-by: Claudio Suarez 
> > ---
> >  drivers/gpu/drm/drm_edid.c | 11 +--
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 6325877c5fd6..c643db17782c 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector 
> > *connector, struct edid *edid)
> > int num_modes = 0;
> > u32 quirks;
> >  
> > -   if (edid == NULL) {
> > -   clear_eld(connector);
> > -   return 0;
> > -   }
> > if (!drm_edid_is_valid(edid)) {
> 
> OK, so drm_edid_is_valid() will happily accept NULL and considers
> it invalid. You may want to mention that explicitly in the commit
> message.

Thank you for your comments, I appreciate :)
I'm sending new mails with the new commit messages.

> > +   /* edid == NULL or invalid here */
> > clear_eld(connector);
> > -   drm_warn(connector->dev, "%s: EDID invalid.\n",
> > -connector->name);
> > +   drm_reset_display_info(connector);
> > +   if (edid)
> > +   drm_warn(connector->dev, "%s: EDID invalid.\n",
> > +connector->name);
> 
> Could you respin this to use the standard [CONNECTOR:%d:%s] form
> while at it? Or I guess a patch to mass convert the whole drm_edid.c
> might be another option.

Good point.
I like the idea of a new patch. I'll start working on it. I can change
this drm_warn here to avoid merge conflicts.

> Patch looks good.
> Reviewed-by: Ville Syrjälä 

Thanks!

BR
Claudio Suarez.

[Nouveau] [PATCH 07/15] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 7655142a4651..a563d6386abe 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector *connector)
if (!edid)
return -ENODEV;
 
-   hdata->dvi_mode = !drm_detect_hdmi_monitor(edid);
+   /* This updates connector->display_info */
+   drm_connector_update_edid_property(connector, edid);
+
+   hdata->dvi_mode = !connector->display_info.is_hdmi;
DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n",
  (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
  edid->width_cm, edid->height_cm);
 
-   drm_connector_update_edid_property(connector, edid);
cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);
 
ret = drm_add_edid_modes(connector, edid);
-- 
2.33.0

[Nouveau] [PATCH 05/15] drm/tegra: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/tegra/hdmi.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index e5d2a4026028..21571221b49b 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -831,14 +831,10 @@ static void tegra_hdmi_setup_tmds(struct tegra_hdmi *hdmi,
 
 static bool tegra_output_is_hdmi(struct tegra_output *output)
 {
-   struct edid *edid;
-
if (!output->connector.edid_blob_ptr)
return false;
 
-   edid = (struct edid *)output->connector.edid_blob_ptr->data;
-
-   return drm_detect_hdmi_monitor(edid);
+   return output->connector.display_info.is_hdmi;
 }
 
 static enum drm_connector_status
-- 
2.33.0

[Nouveau] [PATCH 14/15] drm/nouveau: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++--
 drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +-
 drivers/gpu/drm/nouveau/nouveau_connector.h | 6 ++
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d7b9f7f8c9e3..fadd58b015d6 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -844,7 +844,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct 
nouveau_crtc *nv_crtc,
int ret;
int size;
 
-   if (!drm_detect_hdmi_monitor(nv_connector->edid))
+   if (nouveau_connector_is_hdmi_monitor(_connector->base))
return;
 
hdmi = _connector->base.display_info.hdmi;
@@ -1745,7 +1745,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *sta
 */
if (mode->clock >= 165000 &&
nv_encoder->dcb->duallink_possible &&
-   !drm_detect_hdmi_monitor(nv_connector->edid))
+   
!nouveau_connector_is_hdmi_monitor(_connector->base))
proto = 
NV507D_SOR_SET_CONTROL_PROTOCOL_DUAL_TMDS;
} else {
proto = NV507D_SOR_SET_CONTROL_PROTOCOL_SINGLE_TMDS_B;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c 
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index d66f97280282..0a138bfb8f32 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -127,14 +127,8 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
struct drm_display_mode *omode = >state.adjusted_mode;
struct drm_display_mode *umode = >state.mode;
int mode = asyc->scaler.mode;
-   struct edid *edid;
int umode_vdisplay, omode_hdisplay, omode_vdisplay;
 
-   if (connector->edid_blob_ptr)
-   edid = (struct edid *)connector->edid_blob_ptr->data;
-   else
-   edid = NULL;
-
if (!asyc->scaler.full) {
if (mode == DRM_MODE_SCALE_NONE)
omode = umode;
@@ -162,7 +156,7 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
 */
if ((asyc->scaler.underscan.mode == UNDERSCAN_ON ||
(asyc->scaler.underscan.mode == UNDERSCAN_AUTO &&
-drm_detect_hdmi_monitor(edid {
+nouveau_connector_is_hdmi_monitor(connector {
u32 bX = asyc->scaler.underscan.hborder;
u32 bY = asyc->scaler.underscan.vborder;
u32 r = (asyh->view.oH << 19) / asyh->view.oW;
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 22b83a6577eb..211543373b72 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1010,7 +1010,7 @@ get_tmds_link_bandwidth(struct drm_connector *connector)
unsigned duallink_scale =
nouveau_duallink && nv_encoder->dcb->duallink_possible ? 2 : 1;
 
-   if (drm_detect_hdmi_monitor(nv_connector->edid)) {
+   if (nouveau_connector_is_hdmi_monitor(connector)) {
info = _connector->base.display_info;
duallink_scale = 1;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h 
b/drivers/gpu/drm/nouveau/nouveau_connector.h
index 40f90e353540..299f3a3b2331 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -167,6 +167,12 @@ nouveau_connector_is_mst(struct drm_connector *connector)
return encoder->encoder_type == DRM_MODE_ENCODER_DPMST;
 }
 
+static inline bool
+nouveau_connector_is_hdmi_monitor(struct drm_connector *connector)
+{
+   return connector->display_info.is_hdmi;
+}
+
 #define nouveau_for_each_non_mst_connector_iter(connector, iter) \
drm_for_each_connector_iter(connector, iter) \
for_each_if(!nouveau_connector_is_mst(connector))
-- 
2.33.0

[Nouveau] [PATCH 08/15] drm/msm: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index 58707a1f3878..07585092f919 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -364,8 +364,8 @@ static int msm_hdmi_connector_get_modes(struct 
drm_connector *connector)
 
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl);
 
-   hdmi->hdmi_mode = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_mode = connector->display_info.is_hdmi;
 
if (edid) {
ret = drm_add_edid_modes(connector, edid);
-- 
2.33.0

[Nouveau] [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver

[Claudio Suarez]

a) Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. The amdgpu driver still calls
drm_detect_hdmi_monitor() to retrieve the same information, which
is less efficient. Change to drm_display_info.is_hdmi

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

b) drm_display_info is updated by drm_get_edid() or
drm_connector_update_edid_property(). In the amdgpu driver it is almost
always updated when the edid is read in amdgpu_connector_get_edid(),
but not always.  Change amdgpu_connector_get_edid() and
amdgpu_connector_free_edid() to keep drm_display_info updated. This allows a)
to work properly.

c) Use drm_edid_get_monitor_name() instead of duplicating the code that
parses the EDID in dm_helpers_parse_edid_caps()

Also, remove the unused "struct dc_context *ctx" parameter in
dm_helpers_parse_edid_caps()

Signed-off-by: Claudio Suarez 
---
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 23 +++
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.h|  2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c  |  4 +-
 .../gpu/drm/amd/amdgpu/atombios_encoders.c|  6 +--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  3 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 39 ++-
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  2 +-
 drivers/gpu/drm/amd/display/dc/dm_helpers.h   |  2 +-
 9 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index b9c11c2b2885..7b41a1120b70 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -25,6 +25,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -108,7 +109,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector 
*connector)
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_HDMIB:
if (amdgpu_connector->use_digital) {
-   if 
(drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+   if (amdgpu_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -116,7 +117,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector 
*connector)
break;
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_HDMIA:
-   if (drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+   if (amdgpu_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -125,7 +126,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector 
*connector)
dig_connector = amdgpu_connector->con_priv;
if ((dig_connector->dp_sink_type == 
CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
(dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) ||
-   drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+   (amdgpu_connector_is_hdmi_monitor(connector))) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -149,7 +150,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector 
*connector)
break;
}
 
-   if (drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+   if (amdgpu_connector_is_hdmi_monitor(connector)) {
/*
 * Pre DCE-8 hw can't handle > 12 bpc, and more than 12 bpc 
doesn't make
 * much sense without support for > 12 bpc framebuffers. RGB 
4:4:4 at
@@ -315,8 +316,10 @@ static void amdgpu_connector_get_edid(struct drm_connector 
*connector)
if (!amdgpu_connector->edid) {
/* some laptops provide a hardcoded edid in rom for LCDs */
if (((connector->connector_type == DRM_MODE_CONNECTOR_LVDS) ||
-(connector->connector_type == DRM_MODE_CONNECTOR_eDP)))
+(connector->connector_type == DRM_MODE_CONNECTOR_eDP))) {
amdgpu_connector->edid = 
amdgpu_connector_get_hardcoded_edid(adev);
+   drm_connector_update_edid_property(connector, 
amdgpu_connector->edid);
+   }
}
 }
 
@@ -326,6 +329,7 @@ static void amdgpu_connector_free_edid(struct drm_connector 
*connector)
 
kfree(amdgpu_connector->edid);
amdgpu_connector->edid = NULL;
+   drm_connector_update_edid_property(connector, NULL);
 }
 
 static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector)
@@ -1170,7 +1174,7 @@ static enum

[Nouveau] [PATCH 00/15] replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

Copy from the TODO document Documentation/gpu/todo.rst 

===
Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
---

Once EDID is parsed, the monitor HDMI support information is available through
drm_display_info.is_hdmi. Many drivers still call drm_detect_hdmi_monitor() to
retrieve the same information, which is less efficient.

Audit each individual driver calling drm_detect_hdmi_monitor() and switch to
drm_display_info.is_hdmi if applicable.
=

I did it in two steps:
- check that drm_display_info has a correct value.
- in that case, replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

Almost all occurrences of drm_detect_hdmi_monitor() could be changed. Some
small inconsistencies have been solved.

Stats:
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c| 23 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.h|  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c  |  4 ++--
 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c|  6 +++---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  3 +--
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 39 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  2 +-
 drivers/gpu/drm/amd/display/dc/dm_helpers.h   |  2 +-
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c  |  2 +-
 drivers/gpu/drm/bridge/sii902x.c  |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
 drivers/gpu/drm/drm_edid.c|  2 ++
 drivers/gpu/drm/exynos/exynos_hdmi.c  |  6 --
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c   |  3 ++-
 drivers/gpu/drm/gma500/psb_intel_sdvo.c   |  6 --
 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 ++-
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c   |  4 ++--
 drivers/gpu/drm/nouveau/dispnv50/head.c   |  8 +---
 drivers/gpu/drm/nouveau/nouveau_connector.c   |  2 +-
 drivers/gpu/drm/nouveau/nouveau_connector.h   |  6 ++
 drivers/gpu/drm/radeon/atombios_encoders.c|  6 +++---
 drivers/gpu/drm/radeon/radeon_connectors.c| 20 
++--
 drivers/gpu/drm/radeon/radeon_display.c   |  2 +-
 drivers/gpu/drm/radeon/radeon_encoders.c  |  4 ++--
 drivers/gpu/drm/radeon/radeon_mode.h  |  1 +
 drivers/gpu/drm/rockchip/inno_hdmi.c  |  4 ++--
 drivers/gpu/drm/rockchip/rk3066_hdmi.c|  2 +-
 drivers/gpu/drm/sti/sti_hdmi.c| 10 ++
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c|  4 ++--
 drivers/gpu/drm/tegra/hdmi.c  |  6 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c|  6 +++---
 drivers/gpu/drm/zte/zx_hdmi.c |  4 ++--
 37 files changed, 112 insertions(+), 96 deletions(-)

Best regards.
Claudio Suarez

[Nouveau] [PATCH 06/15] drm/gma500: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++-
 drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 --
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c 
b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
index e525689f84f0..d9db5d52d52e 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
@@ -130,6 +130,7 @@ static enum drm_connector_status cdv_hdmi_detect(
struct edid *edid = NULL;
enum drm_connector_status status = connector_status_disconnected;
 
+   /* This updates connector->display_info */
edid = drm_get_edid(connector, _encoder->i2c_bus->adapter);
 
hdmi_priv->has_hdmi_sink = false;
@@ -138,7 +139,7 @@ static enum drm_connector_status cdv_hdmi_detect(
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
hdmi_priv->has_hdmi_sink =
-   drm_detect_hdmi_monitor(edid);
+   connector->display_info.is_hdmi;
hdmi_priv->has_hdmi_audio =
drm_detect_monitor_audio(edid);
}
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c 
b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 355da2856389..5ef49d17de98 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -1266,8 +1266,10 @@ psb_intel_sdvo_hdmi_sink_detect(struct drm_connector 
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (psb_intel_sdvo->is_hdmi) {
-   psb_intel_sdvo->has_hdmi_monitor = 
drm_detect_hdmi_monitor(edid);
-   psb_intel_sdvo->has_hdmi_audio = 
drm_detect_monitor_audio(edid);
+   psb_intel_sdvo->has_hdmi_monitor =
+   connector->display_info.is_hdmi;
+   psb_intel_sdvo->has_hdmi_audio =
+   drm_detect_monitor_audio(edid);
}
} else
status = connector_status_disconnected;
-- 
2.33.0

[Nouveau] [PATCH 10/15] drm/sti: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/sti/sti_hdmi.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f3ace11209dd..3f8b04a1407e 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -984,14 +984,16 @@ static int sti_hdmi_connector_get_modes(struct 
drm_connector *connector)
if (!edid)
goto fail;
 
-   hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
-   DRM_DEBUG_KMS("%s : %dx%d cm\n",
- (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
- edid->width_cm, edid->height_cm);
cec_notifier_set_phys_addr_from_edid(hdmi->notifier, edid);
 
count = drm_add_edid_modes(connector, edid);
+
+   /* This updates connector->display_info */
drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_monitor = connector->display_info.is_hdmi;
+   DRM_DEBUG_KMS("%s : %dx%d cm\n",
+ (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
+ edid->width_cm, edid->height_cm);
 
kfree(edid);
return count;
-- 
2.33.0

[Nouveau] [PATCH 11/15] drm/zte: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/zte/zx_hdmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
index cd79ca0a92a9..7df682d90723 100644
--- a/drivers/gpu/drm/zte/zx_hdmi.c
+++ b/drivers/gpu/drm/zte/zx_hdmi.c
@@ -265,9 +265,9 @@ static int zx_hdmi_connector_get_modes(struct drm_connector 
*connector)
if (!edid)
return 0;
 
-   hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
-   hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
drm_connector_update_edid_property(connector, edid);
+   hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
+   hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
 
-- 
2.33.0

[Nouveau] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info

[Claudio Suarez]

According to the documentation, drm_add_edid_modes
"... Also fills out the _display_info structure and ELD in @connector
with any information which can be derived from the edid."

drm_add_edid_modes accepts a struct edid *edid parameter which may have a
value or may be null. When it is not null, connector->display_info and
connector->eld are updated according to the edid. When edid=NULL, only
connector->eld is reset. Reset connector->display_info to be consistent
and accurate.

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/drm_edid.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6325877c5fd6..6cbe09b2357c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector *connector, 
struct edid *edid)
 
if (edid == NULL) {
clear_eld(connector);
+   drm_reset_display_info(connector);
return 0;
}
if (!drm_edid_is_valid(edid)) {
clear_eld(connector);
+   drm_reset_display_info(connector);
drm_warn(connector->dev, "%s: EDID invalid.\n",
 connector->name);
return 0;
-- 
2.33.0

[Nouveau] [PATCH 12/15] drm/rockchip: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/rockchip/inno_hdmi.c   | 4 ++--
 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c 
b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 7afdc54eb3ec..d479f230833e 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -553,9 +553,9 @@ static int inno_hdmi_connector_get_modes(struct 
drm_connector *connector)
 
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
-   hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
-   hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
+   hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c 
b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index 1c546c3a8998..03aaae39cf61 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -472,8 +472,8 @@ static int rk3066_hdmi_connector_get_modes(struct 
drm_connector *connector)
 
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
-   hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
-- 
2.33.0

[Nouveau] [PATCH 03/15] drm/vc4: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Use this value instead of calling
drm_detect_hdmi_monitor() to avoid a second parse.

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index b4b4653fe301..d531e4c501eb 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -182,7 +182,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, 
bool force)
 
if (edid) {
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, 
edid);
-   vc4_hdmi->encoder.hdmi_monitor = 
drm_detect_hdmi_monitor(edid);
+   vc4_hdmi->encoder.hdmi_monitor =
+   connector->display_info.is_hdmi;
kfree(edid);
}
}
@@ -212,10 +213,9 @@ static int vc4_hdmi_connector_get_modes(struct 
drm_connector *connector)
if (!edid)
return -ENODEV;
 
-   vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
-
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
+   vc4_encoder->hdmi_monitor = connector->display_info.is_hdmi;
kfree(edid);
 
if (vc4_hdmi->disable_4kp60) {
-- 
2.33.0

[Nouveau] [PATCH 09/15] drm/sun4i: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c 
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 2f2c9f0a1071..f57bedbbeeb8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -215,11 +215,11 @@ static int sun4i_hdmi_get_modes(struct drm_connector 
*connector)
if (!edid)
return 0;
 
-   hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
+   drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_monitor = connector->display_info.is_hdmi;
DRM_DEBUG_DRIVER("Monitor is %s monitor\n",
 hdmi->hdmi_monitor ? "an HDMI" : "a DVI");
 
-   drm_connector_update_edid_property(connector, edid);
cec_s_phys_addr_from_edid(hdmi->cec_adap, edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
-- 
2.33.0

[Nouveau] [PATCH 04/15] drm/radeon: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information is less
efficient. Change to drm_display_info.is_hdmi

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

Also, correct an inacurracy or bug in
radeon_connector_get_edid()/radeon_connector_free_edid(). Two variables
have EDID data:
- struct radeon_connector.edid
- struct drm_connector.edid_blob_ptr
The second is updated by calling drm_connector_update_edid_property() or
drm_get_edid() - which internally calls drm_connector_update_edid_property().
drm_display_info.is_hdmi is updated when this function is called.
radeon_connector_get_edid() calls drm_get_edid() to update
drm_connector.edid_blob_ptr/drm_display_info only in some cases. Change it
to be always up to date, so drm_display_info is always correct.
As counterpart, reset these values in radeon_connector_free_edid().

This second change is necessary for the previous one to work properly.

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/radeon/atombios_encoders.c |  6 +++---
 drivers/gpu/drm/radeon/radeon_connectors.c | 20 ++--
 drivers/gpu/drm/radeon/radeon_display.c|  2 +-
 drivers/gpu/drm/radeon/radeon_encoders.c   |  4 ++--
 drivers/gpu/drm/radeon/radeon_mode.h   |  1 +
 5 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c 
b/drivers/gpu/drm/radeon/atombios_encoders.c
index 0fce73b9a646..29a140732f71 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -713,7 +713,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_connector->use_digital &&
(radeon_connector->audio == RADEON_AUDIO_ENABLE))
return ATOM_ENCODER_MODE_HDMI;
-   else if 
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+   else if (radeon_connector_is_hdmi_monitor(connector) &&
 (radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else if (radeon_connector->use_digital)
@@ -732,7 +732,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
-   else if 
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+   else if (radeon_connector_is_hdmi_monitor(connector) &&
 (radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
@@ -756,7 +756,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
} else if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
-   else if 
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+   else if (radeon_connector_is_hdmi_monitor(connector) &&
 (radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 607ad5620bd9..0200f094467c 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -130,7 +130,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_HDMIB:
if (radeon_connector->use_digital) {
-   if 
(drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+   if (radeon_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -138,7 +138,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
break;
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_HDMIA:
-   if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+   if (radeon_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -147,7 +147,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
dig_connector = radeon_connector->con_priv;
if ((dig_

[Nouveau] [PATCH 13/15] drm/bridge: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
 drivers/gpu/drm/bridge/sii902x.c | 2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 76555ae64e9c..f6891280a58d 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -617,7 +617,7 @@ static struct edid *adv7511_get_edid(struct adv7511 
*adv7511,
__adv7511_power_off(adv7511);
 
adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
-  drm_detect_hdmi_monitor(edid));
+  connector->display_info.is_hdmi);
 
cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
 
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 89558e581530..5719be0a03c7 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -283,7 +283,7 @@ static int sii902x_get_modes(struct drm_connector 
*connector)
edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
drm_connector_update_edid_property(connector, edid);
if (edid) {
-   if (drm_detect_hdmi_monitor(edid))
+   if (connector->display_info.is_hdmi)
output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
 
num = drm_add_edid_modes(connector, edid);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f08d0fded61f..33f0afb6b646 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2359,7 +2359,7 @@ static struct edid *dw_hdmi_get_edid(struct dw_hdmi *hdmi,
dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
edid->width_cm, edid->height_cm);
 
-   hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
+   hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
 
return edid;
-- 
2.33.0

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

[Claudio Suarez]

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 
---
 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);
 
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;
-- 
2.33.0

Re: [Nouveau] [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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 
> > ---
> >  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(_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 = t

[Nouveau] [PATCH v2 12/13] drm/nouveau: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++--
 drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +---
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d7b9f7f8c9e3..b3c7199aa63d 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -844,7 +844,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct 
nouveau_crtc *nv_crtc,
int ret;
int size;
 
-   if (!drm_detect_hdmi_monitor(nv_connector->edid))
+   if (!nv_connector->base.display_info.is_hdmi)
return;
 
hdmi = _connector->base.display_info.hdmi;
@@ -1745,7 +1745,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *sta
 */
if (mode->clock >= 165000 &&
nv_encoder->dcb->duallink_possible &&
-   !drm_detect_hdmi_monitor(nv_connector->edid))
+   !nv_connector->base.display_info.is_hdmi)
proto = 
NV507D_SOR_SET_CONTROL_PROTOCOL_DUAL_TMDS;
} else {
proto = NV507D_SOR_SET_CONTROL_PROTOCOL_SINGLE_TMDS_B;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c 
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index d66f97280282..29d6c010ac13 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -127,14 +127,8 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
struct drm_display_mode *omode = >state.adjusted_mode;
struct drm_display_mode *umode = >state.mode;
int mode = asyc->scaler.mode;
-   struct edid *edid;
int umode_vdisplay, omode_hdisplay, omode_vdisplay;
 
-   if (connector->edid_blob_ptr)
-   edid = (struct edid *)connector->edid_blob_ptr->data;
-   else
-   edid = NULL;
-
if (!asyc->scaler.full) {
if (mode == DRM_MODE_SCALE_NONE)
omode = umode;
@@ -162,7 +156,7 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
 */
if ((asyc->scaler.underscan.mode == UNDERSCAN_ON ||
(asyc->scaler.underscan.mode == UNDERSCAN_AUTO &&
-drm_detect_hdmi_monitor(edid {
+connector->display_info.is_hdmi))) {
u32 bX = asyc->scaler.underscan.hborder;
u32 bY = asyc->scaler.underscan.vborder;
u32 r = (asyh->view.oH << 19) / asyh->view.oW;
-- 
2.33.0

[Nouveau] [PATCH v2 08/13] drm/sun4i: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c 
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 2f2c9f0a1071..f57bedbbeeb8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -215,11 +215,11 @@ static int sun4i_hdmi_get_modes(struct drm_connector 
*connector)
if (!edid)
return 0;
 
-   hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
+   drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_monitor = connector->display_info.is_hdmi;
DRM_DEBUG_DRIVER("Monitor is %s monitor\n",
 hdmi->hdmi_monitor ? "an HDMI" : "a DVI");
 
-   drm_connector_update_edid_property(connector, edid);
cec_s_phys_addr_from_edid(hdmi->cec_adap, edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
-- 
2.33.0

[Nouveau] [PATCH v2 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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 
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..008e5b0ba408 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 = connector->display_info.is_hdmi;
 
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c 
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..b4065e4df644 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 =
+   
connector->display_info.is_hdmi;
}
} else
status = connector_status_disconnected;
-- 
2.33.0

[Nouveau] [PATCH v2 05/13] drm/gma500: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++-
 drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 --
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c 
b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
index e525689f84f0..d9db5d52d52e 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
@@ -130,6 +130,7 @@ static enum drm_connector_status cdv_hdmi_detect(
struct edid *edid = NULL;
enum drm_connector_status status = connector_status_disconnected;
 
+   /* This updates connector->display_info */
edid = drm_get_edid(connector, _encoder->i2c_bus->adapter);
 
hdmi_priv->has_hdmi_sink = false;
@@ -138,7 +139,7 @@ static enum drm_connector_status cdv_hdmi_detect(
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
hdmi_priv->has_hdmi_sink =
-   drm_detect_hdmi_monitor(edid);
+   connector->display_info.is_hdmi;
hdmi_priv->has_hdmi_audio =
drm_detect_monitor_audio(edid);
}
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c 
b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 355da2856389..5ef49d17de98 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -1266,8 +1266,10 @@ psb_intel_sdvo_hdmi_sink_detect(struct drm_connector 
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (psb_intel_sdvo->is_hdmi) {
-   psb_intel_sdvo->has_hdmi_monitor = 
drm_detect_hdmi_monitor(edid);
-   psb_intel_sdvo->has_hdmi_audio = 
drm_detect_monitor_audio(edid);
+   psb_intel_sdvo->has_hdmi_monitor =
+   connector->display_info.is_hdmi;
+   psb_intel_sdvo->has_hdmi_audio =
+   drm_detect_monitor_audio(edid);
}
} else
status = connector_status_disconnected;
-- 
2.33.0

[Nouveau] [PATCH v2 09/13] drm/sti: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/sti/sti_hdmi.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f3ace11209dd..3f8b04a1407e 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -984,14 +984,16 @@ static int sti_hdmi_connector_get_modes(struct 
drm_connector *connector)
if (!edid)
goto fail;
 
-   hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
-   DRM_DEBUG_KMS("%s : %dx%d cm\n",
- (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
- edid->width_cm, edid->height_cm);
cec_notifier_set_phys_addr_from_edid(hdmi->notifier, edid);
 
count = drm_add_edid_modes(connector, edid);
+
+   /* This updates connector->display_info */
drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_monitor = connector->display_info.is_hdmi;
+   DRM_DEBUG_KMS("%s : %dx%d cm\n",
+ (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
+ edid->width_cm, edid->height_cm);
 
kfree(edid);
return count;
-- 
2.33.0

[Nouveau] [PATCH v2 06/13] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 7655142a4651..a563d6386abe 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector *connector)
if (!edid)
return -ENODEV;
 
-   hdata->dvi_mode = !drm_detect_hdmi_monitor(edid);
+   /* This updates connector->display_info */
+   drm_connector_update_edid_property(connector, edid);
+
+   hdata->dvi_mode = !connector->display_info.is_hdmi;
DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n",
  (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
  edid->width_cm, edid->height_cm);
 
-   drm_connector_update_edid_property(connector, edid);
cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);
 
ret = drm_add_edid_modes(connector, edid);
-- 
2.33.0

[Nouveau] [PATCH v2 07/13] drm/msm: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index 58707a1f3878..07585092f919 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -364,8 +364,8 @@ static int msm_hdmi_connector_get_modes(struct 
drm_connector *connector)
 
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl);
 
-   hdmi->hdmi_mode = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_mode = connector->display_info.is_hdmi;
 
if (edid) {
ret = drm_add_edid_modes(connector, edid);
-- 
2.33.0

[Nouveau] [PATCH v2 10/13] drm/rockchip: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/rockchip/inno_hdmi.c   | 4 ++--
 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c 
b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 7afdc54eb3ec..d479f230833e 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -553,9 +553,9 @@ static int inno_hdmi_connector_get_modes(struct 
drm_connector *connector)
 
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
-   hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
-   hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
+   hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c 
b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index 1c546c3a8998..03aaae39cf61 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -472,8 +472,8 @@ static int rk3066_hdmi_connector_get_modes(struct 
drm_connector *connector)
 
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
-   hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+   hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
-- 
2.33.0

[Nouveau] [PATCH v2 11/13] drm/bridge: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
 drivers/gpu/drm/bridge/sii902x.c | 2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 76555ae64e9c..f6891280a58d 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -617,7 +617,7 @@ static struct edid *adv7511_get_edid(struct adv7511 
*adv7511,
__adv7511_power_off(adv7511);
 
adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
-  drm_detect_hdmi_monitor(edid));
+  connector->display_info.is_hdmi);
 
cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
 
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 89558e581530..5719be0a03c7 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -283,7 +283,7 @@ static int sii902x_get_modes(struct drm_connector 
*connector)
edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
drm_connector_update_edid_property(connector, edid);
if (edid) {
-   if (drm_detect_hdmi_monitor(edid))
+   if (connector->display_info.is_hdmi)
output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
 
num = drm_add_edid_modes(connector, edid);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f08d0fded61f..33f0afb6b646 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2359,7 +2359,7 @@ static struct edid *dw_hdmi_get_edid(struct dw_hdmi *hdmi,
dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
edid->width_cm, edid->height_cm);
 
-   hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
+   hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
 
return edid;
-- 
2.33.0

[Nouveau] [PATCH v2 04/13] drm/tegra: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/tegra/hdmi.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index e5d2a4026028..21571221b49b 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -831,14 +831,10 @@ static void tegra_hdmi_setup_tmds(struct tegra_hdmi *hdmi,
 
 static bool tegra_output_is_hdmi(struct tegra_output *output)
 {
-   struct edid *edid;
-
if (!output->connector.edid_blob_ptr)
return false;
 
-   edid = (struct edid *)output->connector.edid_blob_ptr->data;
-
-   return drm_detect_hdmi_monitor(edid);
+   return output->connector.display_info.is_hdmi;
 }
 
 static enum drm_connector_status
-- 
2.33.0

[Nouveau] [PATCH v2 03/13] drm/radeon: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information is less
efficient. Change to drm_display_info.is_hdmi

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

Also, correct an inacurracy or bug in
radeon_connector_get_edid()/radeon_connector_free_edid(). Two variables
have EDID data:
- struct radeon_connector.edid
- struct drm_connector.edid_blob_ptr
The second is updated by calling drm_connector_update_edid_property() or
drm_get_edid() - which internally calls drm_connector_update_edid_property().
drm_display_info.is_hdmi is updated when this function is called.
radeon_connector_get_edid() calls drm_get_edid() to update
drm_connector.edid_blob_ptr/drm_display_info only in some cases. Change it
to be always up to date, so drm_display_info is always correct.
As counterpart, reset these values in radeon_connector_free_edid().

This second change is necessary for the previous one to work properly.

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/radeon/atombios_encoders.c |  6 +++---
 drivers/gpu/drm/radeon/radeon_connectors.c | 15 +--
 drivers/gpu/drm/radeon/radeon_display.c|  2 +-
 drivers/gpu/drm/radeon/radeon_encoders.c   |  4 ++--
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c 
b/drivers/gpu/drm/radeon/atombios_encoders.c
index 0fce73b9a646..844f61a36f20 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -713,7 +713,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_connector->use_digital &&
(radeon_connector->audio == RADEON_AUDIO_ENABLE))
return ATOM_ENCODER_MODE_HDMI;
-   else if 
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+   else if (connector->display_info.is_hdmi &&
 (radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else if (radeon_connector->use_digital)
@@ -732,7 +732,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
-   else if 
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+   else if (connector->display_info.is_hdmi &&
 (radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
@@ -756,7 +756,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
} else if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
-   else if 
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+   else if (connector->display_info.is_hdmi &&
 (radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 607ad5620bd9..deaae181ac59 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -130,7 +130,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_HDMIB:
if (radeon_connector->use_digital) {
-   if 
(drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+   if (connector->display_info.is_hdmi) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -138,7 +138,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
break;
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_HDMIA:
-   if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+   if (connector->display_info.is_hdmi) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -147,7 +147,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
dig_connector = radeon_connector->con_priv;
if ((dig_connector->dp_sink_type == 
CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
(dig_connector->

[Nouveau] [PATCH v2 00/13] replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

Changelog:
v2:
- no helper function
- A separate patch is made for amdgpu
- zte patch is removed because that driver no longer exists

[Why]
Copy from Documentation/gpu/todo.rst 
===
Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
---

Once EDID is parsed, the monitor HDMI support information is available through
drm_display_info.is_hdmi. Many drivers still call drm_detect_hdmi_monitor() to
retrieve the same information, which is less efficient.

Audit each individual driver calling drm_detect_hdmi_monitor() and switch to
drm_display_info.is_hdmi if applicable.
=

[How]
I did it in two steps:
- check that drm_display_info has a correct value.
- in that case, replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

Almost all occurrences of drm_detect_hdmi_monitor() could be changed. Some
small inconsistencies have been solved.

Stats:
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c |  2 +-
 drivers/gpu/drm/bridge/sii902x.c |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c|  2 +-
 drivers/gpu/drm/drm_edid.c   | 11 +--
 drivers/gpu/drm/exynos/exynos_hdmi.c |  6 --
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c  |  3 ++-
 drivers/gpu/drm/gma500/psb_intel_sdvo.c  |  6 --
 drivers/gpu/drm/i915/display/intel_hdmi.c|  2 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c|  3 ++-
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c|  2 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c  |  4 ++--
 drivers/gpu/drm/nouveau/dispnv50/head.c  |  8 +---
 drivers/gpu/drm/radeon/atombios_encoders.c   |  6 +++---
 drivers/gpu/drm/radeon/radeon_connectors.c   | 15 +--
 drivers/gpu/drm/radeon/radeon_display.c  |  2 +-
 drivers/gpu/drm/radeon/radeon_encoders.c |  4 ++--
 drivers/gpu/drm/rockchip/inno_hdmi.c |  4 ++--
 drivers/gpu/drm/rockchip/rk3066_hdmi.c   |  2 +-
 drivers/gpu/drm/sti/sti_hdmi.c   | 10 ++
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c   |  4 ++--
 drivers/gpu/drm/tegra/hdmi.c |  6 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c   |  6 +++---
 22 files changed, 55 insertions(+), 55 deletions(-)

Best regards.
Claudio Suarez

[Nouveau] [PATCH v2 02/13] drm/vc4: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Use this value instead of calling
drm_detect_hdmi_monitor() to avoid a second parse.

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

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index b4b4653fe301..d531e4c501eb 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -182,7 +182,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, 
bool force)
 
if (edid) {
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, 
edid);
-   vc4_hdmi->encoder.hdmi_monitor = 
drm_detect_hdmi_monitor(edid);
+   vc4_hdmi->encoder.hdmi_monitor =
+   connector->display_info.is_hdmi;
kfree(edid);
}
}
@@ -212,10 +213,9 @@ static int vc4_hdmi_connector_get_modes(struct 
drm_connector *connector)
if (!edid)
return -ENODEV;
 
-   vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
-
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
+   vc4_encoder->hdmi_monitor = connector->display_info.is_hdmi;
kfree(edid);
 
if (vc4_hdmi->disable_4kp60) {
-- 
2.33.0

[Nouveau] [PATCH v2 01/13] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info

[Claudio Suarez]

According to the documentation, drm_add_edid_modes
"... Also fills out the _display_info structure and ELD in @connector
with any information which can be derived from the edid."

drm_add_edid_modes accepts a struct edid *edid parameter which may have a
value or may be null. When it is not null, connector->display_info and
connector->eld are updated according to the edid. When edid=NULL, only
connector->eld is reset. Reset connector->display_info to be consistent
and accurate.

Signed-off-by: Claudio Suarez 
---
 drivers/gpu/drm/drm_edid.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6325877c5fd6..c643db17782c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector *connector, 
struct edid *edid)
int num_modes = 0;
u32 quirks;
 
-   if (edid == NULL) {
-   clear_eld(connector);
-   return 0;
-   }
if (!drm_edid_is_valid(edid)) {
+   /* edid == NULL or invalid here */
clear_eld(connector);
-   drm_warn(connector->dev, "%s: EDID invalid.\n",
-connector->name);
+   drm_reset_display_info(connector);
+   if (edid)
+   drm_warn(connector->dev, "%s: EDID invalid.\n",
+connector->name);
return 0;
}
 
-- 
2.33.0

Re: [Nouveau] [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver

[Claudio Suarez]

On Fri, Oct 15, 2021 at 11:14:54AM -0400, Harry Wentland wrote:
> 
> 
> On 2021-10-15 07:37, Claudio Suarez wrote:
> > a) Once EDID is parsed, the monitor HDMI support information is available
> > through drm_display_info.is_hdmi. The amdgpu driver still calls
> > drm_detect_hdmi_monitor() to retrieve the same information, which
> > is less efficient. Change to drm_display_info.is_hdmi
> > 
> > This is a TODO task in Documentation/gpu/todo.rst
> > 
> > b) drm_display_info is updated by drm_get_edid() or
> > drm_connector_update_edid_property(). In the amdgpu driver it is almost
> > always updated when the edid is read in amdgpu_connector_get_edid(),
> > but not always.  Change amdgpu_connector_get_edid() and
> > amdgpu_connector_free_edid() to keep drm_display_info updated. This allows 
> > a)
> > to work properly.
> > 
> > c) Use drm_edid_get_monitor_name() instead of duplicating the code that
> > parses the EDID in dm_helpers_parse_edid_caps()
> > 
> > Also, remove the unused "struct dc_context *ctx" parameter in
> > dm_helpers_parse_edid_caps()
> > 
> 
> Thanks for this work.
> 
> The fact that you listed three separate changes in this commit
> is a clear indication that this patch should be three separate
> patches instead. Separating the functional bits from the straight
> refactor will help with bisection if this leads to a regression.
> 
> All changes look reasonable to me, though. With this patch split
> into three patches in the sequence (b), (c), then (a) this is
> Reviewed-by: Harry Wentland 

Ok, thanks. I'll send three patches.

BR
Claudio Suarez

Re: [Nouveau] [Freedreno] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info

[Claudio Suarez]

On Sat, Oct 16, 2021 at 10:25:03AM +0200, Claudio Suarez wrote:
> On Fri, Oct 15, 2021 at 10:33:29PM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 15, 2021 at 09:24:06PM +0200, Claudio Suarez wrote:
> > > On Fri, Oct 15, 2021 at 03:03:13PM +0300, Ville Syrjälä wrote:
> > > > On Fri, Oct 15, 2021 at 01:36:59PM +0200, Claudio Suarez wrote:
> > > > > According to the documentation, drm_add_edid_modes
> > > > > "... Also fills out the _display_info structure and ELD in 
> > > > > @connector
> > > > > with any information which can be derived from the edid."
> > > > > 
> > > > > drm_add_edid_modes accepts a struct edid *edid parameter which may 
> > > > > have a
> > > > > value or may be null. When it is not null, connector->display_info and
> > > > > connector->eld are updated according to the edid. When edid=NULL, only
> > > > > connector->eld is reset. Reset connector->display_info to be 
> > > > > consistent
> > > > > and accurate.
> > > > > 
> > > > > Signed-off-by: Claudio Suarez 
> > > > > ---
> > > > >  drivers/gpu/drm/drm_edid.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > > index 6325877c5fd6..6cbe09b2357c 100644
> > > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > > @@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector 
> > > > > *connector, struct edid *edid)
> > > > >  
> > > > >   if (edid == NULL) {
> > > > >   clear_eld(connector);
> > > > > + drm_reset_display_info(connector);
> > > > >   return 0;
> > > > >   }
> > > > >   if (!drm_edid_is_valid(edid)) {
> > > > >   clear_eld(connector);
> > > > > + drm_reset_display_info(connector);
> > > > 
> > > > Looks easier if you pull both of those out from these branches and
> > > > just call them unconditionally at the start.
> > > 
> > > After looking at the full code, I am not sure. This is the code:
> > > ==
> > > int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> > > {
> > > int num_modes = 0;
> > > u32 quirks;
> > > 
> > > if (edid == NULL) {
> > > clear_eld(connector);
> > > drm_reset_display_info(connector); <--- added by me
> > > return 0;
> > > }
> > > if (!drm_edid_is_valid(edid)) {
> > > clear_eld(connector);
> > > drm_reset_display_info(connector); <--- added by me
> > > drm_warn(connector->dev, "%s: EDID invalid.\n",
> > >  connector->name);
> > > return 0;
> > > }
> > > 
> > > drm_edid_to_eld(connector, edid);
> > > 
> > > quirks = drm_add_display_info(connector, edid);
> > >   etc...
> > > =
> > > 
> > > If we move those out of these branches and edid != NULL, we are executing 
> > > an
> > > unnecessary clear_eld(connector) and an unnecessary 
> > > drm_reset_display_info(connector)
> > > because the fields will be set in the next drm_edid_to_eld(connector, 
> > > edid) and
> > > drm_add_display_info(connector, edid)
> > > 
> > > Do we want this ?
> > 
> > Seems fine by me. And maybe we could nuke the second
> > drm_reset_display_info() from deeper inside drm_add_display_info()?
> > Not sure if drm_add_display_info() still has to be able to operate
> > standalone or not.
> > 
> > Hmm. Another option is to just move all these NULL/invalid edid
> > checks into drm_edid_to_eld() and drm_add_display_info().
> 
> I was thinking about this. We can use a boolean variable:
> ===
> bool edid_is_invalid;
> 
> edid_is_invalid = !drm_edid_is_valid(edid);
> 
> if (edid == NULL || edid_is_invalid) {
> clear_eld(connector);
> drm_reset_display_info(connector);
> if (edid_is_invalid)
>  drm_warn(connector->dev, "%s: EDID invalid.\n",
>   connector->name);
> return 0;
> }
> 
> drm_edid_to_eld(connector, edid);
> ...
> ===
> Internally, drm_edid_is_valid() handles NULL pointers properly.
> It is a quite elegant solution with a small change in the original
> design, and it improves this part in the way you pointed out.

I'll send a patch with this idea and we can talk about the new code.
Thanks!

Best regards,
Claudio Suarez.

Re: [Nouveau] [Freedreno] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info

[Claudio Suarez]

On Fri, Oct 15, 2021 at 10:33:29PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 15, 2021 at 09:24:06PM +0200, Claudio Suarez wrote:
> > On Fri, Oct 15, 2021 at 03:03:13PM +0300, Ville Syrjälä wrote:
> > > On Fri, Oct 15, 2021 at 01:36:59PM +0200, Claudio Suarez wrote:
> > > > According to the documentation, drm_add_edid_modes
> > > > "... Also fills out the _display_info structure and ELD in 
> > > > @connector
> > > > with any information which can be derived from the edid."
> > > > 
> > > > drm_add_edid_modes accepts a struct edid *edid parameter which may have 
> > > > a
> > > > value or may be null. When it is not null, connector->display_info and
> > > > connector->eld are updated according to the edid. When edid=NULL, only
> > > > connector->eld is reset. Reset connector->display_info to be consistent
> > > > and accurate.
> > > > 
> > > > Signed-off-by: Claudio Suarez 
> > > > ---
> > > >  drivers/gpu/drm/drm_edid.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > index 6325877c5fd6..6cbe09b2357c 100644
> > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > @@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector 
> > > > *connector, struct edid *edid)
> > > >  
> > > > if (edid == NULL) {
> > > > clear_eld(connector);
> > > > +   drm_reset_display_info(connector);
> > > > return 0;
> > > > }
> > > > if (!drm_edid_is_valid(edid)) {
> > > > clear_eld(connector);
> > > > +   drm_reset_display_info(connector);
> > > 
> > > Looks easier if you pull both of those out from these branches and
> > > just call them unconditionally at the start.
> > 
> > After looking at the full code, I am not sure. This is the code:
> > ==
> > int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> > {
> > int num_modes = 0;
> > u32 quirks;
> > 
> > if (edid == NULL) {
> > clear_eld(connector);
> > drm_reset_display_info(connector); <--- added by me
> > return 0;
> > }
> > if (!drm_edid_is_valid(edid)) {
> > clear_eld(connector);
> > drm_reset_display_info(connector); <--- added by me
> > drm_warn(connector->dev, "%s: EDID invalid.\n",
> >  connector->name);
> > return 0;
> > }
> > 
> > drm_edid_to_eld(connector, edid);
> > 
> > quirks = drm_add_display_info(connector, edid);
> > etc...
> > =
> > 
> > If we move those out of these branches and edid != NULL, we are executing an
> > unnecessary clear_eld(connector) and an unnecessary 
> > drm_reset_display_info(connector)
> > because the fields will be set in the next drm_edid_to_eld(connector, edid) 
> > and
> > drm_add_display_info(connector, edid)
> > 
> > Do we want this ?
> 
> Seems fine by me. And maybe we could nuke the second
> drm_reset_display_info() from deeper inside drm_add_display_info()?
> Not sure if drm_add_display_info() still has to be able to operate
> standalone or not.
> 
> Hmm. Another option is to just move all these NULL/invalid edid
> checks into drm_edid_to_eld() and drm_add_display_info().

I was thinking about this. We can use a boolean variable:
=======
bool edid_is_invalid;

edid_is_invalid = !drm_edid_is_valid(edid);

if (edid == NULL || edid_is_invalid) {
clear_eld(connector);
drm_reset_display_info(connector);
if (edid_is_invalid)
 drm_warn(connector->dev, "%s: EDID invalid.\n",
  connector->name);
return 0;
}

drm_edid_to_eld(connector, edid);
...
===
Internally, drm_edid_is_valid() handles NULL pointers properly.
It is a quite elegant solution with a small change in the original
design, and it improves this part in the way you pointed out.

Best regards,
Claudio Suarez

Re: [Nouveau] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info

[Claudio Suarez]

On Fri, Oct 15, 2021 at 03:03:13PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 15, 2021 at 01:36:59PM +0200, Claudio Suarez wrote:
> > According to the documentation, drm_add_edid_modes
> > "... Also fills out the _display_info structure and ELD in @connector
> > with any information which can be derived from the edid."
> > 
> > drm_add_edid_modes accepts a struct edid *edid parameter which may have a
> > value or may be null. When it is not null, connector->display_info and
> > connector->eld are updated according to the edid. When edid=NULL, only
> > connector->eld is reset. Reset connector->display_info to be consistent
> > and accurate.
> > 
> > Signed-off-by: Claudio Suarez 
> > ---
> >  drivers/gpu/drm/drm_edid.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 6325877c5fd6..6cbe09b2357c 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector 
> > *connector, struct edid *edid)
> >  
> > if (edid == NULL) {
> > clear_eld(connector);
> > +   drm_reset_display_info(connector);
> > return 0;
> > }
> > if (!drm_edid_is_valid(edid)) {
> > clear_eld(connector);
> > +   drm_reset_display_info(connector);
> 
> Looks easier if you pull both of those out from these branches and
> just call them unconditionally at the start.

After looking at the full code, I am not sure. This is the code:
==
int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
{
int num_modes = 0;
u32 quirks;

if (edid == NULL) {
clear_eld(connector);
drm_reset_display_info(connector); <--- added by me
return 0;
}
if (!drm_edid_is_valid(edid)) {
clear_eld(connector);
drm_reset_display_info(connector); <--- added by me
drm_warn(connector->dev, "%s: EDID invalid.\n",
 connector->name);
return 0;
}

drm_edid_to_eld(connector, edid);

quirks = drm_add_display_info(connector, edid);
etc...
=

If we move those out of these branches and edid != NULL, we are executing an
unnecessary clear_eld(connector) and an unnecessary 
drm_reset_display_info(connector)
because the fields will be set in the next drm_edid_to_eld(connector, edid) and
drm_add_display_info(connector, edid)

Do we want this ?

BR
Claudio Suarez

Re: [Nouveau] [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Claudio Suarez]

On Fri, Oct 15, 2021 at 06:18:34PM +0300, Jani Nikula wrote:
> On Fri, 15 Oct 2021, Ville Syrjälä  wrote:
> > On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote:
> >> On Fri, 15 Oct 2021, 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 
> >> > ---
> >> >  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.