[0/3] drm/i915/display: handle hdmi connector init failures

Message

Jani Nikula Nov. 14, 2024, 3:52 p.m. UTC

This is an attempt to resolve [1].

BR,
Jani.


[1] https://lore.kernel.org/r/[e-mail]


Cc: Sergey Senozhatsky <[e-mail]>
Cc: Ville Syrjala <[e-mail]>

Jani Nikula (3):
  drm/i915/hdmi: propagate errors from intel_hdmi_init_connector()
  drm/i915/hdmi: add error handling in g4x_hdmi_init()
  drm/i915/ddi: propagate errors from intel_ddi_init_hdmi_connector()

 drivers/gpu/drm/i915/display/g4x_hdmi.c   | 35 +++++++++++++++--------
 drivers/gpu/drm/i915/display/g4x_hdmi.h   |  5 ++--
 drivers/gpu/drm/i915/display/intel_ddi.c  |  6 +++-
 drivers/gpu/drm/i915/display/intel_hdmi.c | 10 ++++---
 drivers/gpu/drm/i915/display/intel_hdmi.h |  4 +--
 5 files changed, 39 insertions(+), 21 deletions(-)

Comments

Sergey Senozhatsky Nov. 15, 2024, 5 a.m. UTC | #1

On (24/11/14 17:52), Jani Nikula wrote:
> This is an attempt to resolve [1].
> 
> [1] https://lore.kernel.org/r/[e-mail]
> 
> Jani Nikula (3):
>   drm/i915/hdmi: propagate errors from intel_hdmi_init_connector()
>   drm/i915/hdmi: add error handling in g4x_hdmi_init()
>   drm/i915/ddi: propagate errors from intel_ddi_init_hdmi_connector()

So these look reasonable and good to me, and I think landing them
makes a lot of sense.  For that part feel free to add:

Reported-and-tested-by: Sergey Senozhatsky <[e-mail]>
and FWIW
Reviewed-by: Sergey Senozhatsky <[e-mail]>



Now, regarding the issue in question - we are getting there, but not
quite there yet.

I'm still seeing multiple NULL-ptr derefs, namely on

 DDI A/PHY A
 DDI TC1/PHY TC1
 DDI TC2/PHY TC2
 DDI TC3/PHY TC3
 DDI TC4/PHY TC4

The thing is, all of them are DP connectors, we only ever call
intel_ddi_init_dp_connector() on them, so dig_port->hdmi
is not init-ed, wheres intel_hdmi_encoder_shutdown() doesn't
distinguish and treats everyone as HDMI connector, dereferencing
->hdmi.

With the small addition I don't see null-ptr derefs anymore:

---

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 1abf080b4a20..0c6bb378b708 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2377,6 +2377,11 @@ void intel_hdmi_encoder_shutdown(struct intel_encoder *encoder)
 {
        struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 
+       if (intel_encoder_is_dp(encoder)) {
+               pr_err(":: oops another day ::\n");
+               return;
+       }
+
        /*
         * Give a hand to buggy BIOSen which forget to turn
         * the TMDS output buffers back on after a reboot.