| Message ID | ddd89d5e49a0cd40c18f12567da7fb9605999fcd.1734099220.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/display: handle hdmi connector init failures, and no HDMI/DP cases | expand |
> -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: Friday, December 13, 2024 7:46 PM > To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org > Cc: Nikula, Jani <jani.nikula@intel.com>; Sergey Senozhatsky > <senozhatsky@chromium.org>; Ville Syrjala <ville.syrjala@linux.intel.com>; > Kandpal, Suraj <suraj.kandpal@intel.com> > Subject: [PATCH v3 1/6] drm/i915/ddi: change > intel_ddi_init_{dp,hdmi}_connector() return type > > The caller doesn't actually need the returned struct intel_connector; it's > stored in the ->attached_connector of intel_dp ad intel_hdmi. Switch to Typo : *and Otherwise LGTM, Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> > returning an int with 0 for success and negative errors codes to be able to > indicate success even when we don't have a connector. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 20 +++++++++----------- > 1 file changed, 9 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 4f9c50996446..21277cf8afef 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -4542,8 +4542,7 @@ static const struct drm_encoder_funcs > intel_ddi_funcs = { > .late_register = intel_ddi_encoder_late_register, }; > > -static struct intel_connector * > -intel_ddi_init_dp_connector(struct intel_digital_port *dig_port) > +static int intel_ddi_init_dp_connector(struct intel_digital_port > +*dig_port) > { > struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > struct intel_connector *connector; > @@ -4551,7 +4550,7 @@ intel_ddi_init_dp_connector(struct > intel_digital_port *dig_port) > > connector = intel_connector_alloc(); > if (!connector) > - return NULL; > + return -ENOMEM; > > dig_port->dp.output_reg = DDI_BUF_CTL(port); > if (DISPLAY_VER(i915) >= 14) > @@ -4566,7 +4565,7 @@ intel_ddi_init_dp_connector(struct > intel_digital_port *dig_port) > > if (!intel_dp_init_connector(dig_port, connector)) { > kfree(connector); > - return NULL; > + return -EINVAL; > } > > if (dig_port->base.type == INTEL_OUTPUT_EDP) { @@ -4582,7 > +4581,7 @@ intel_ddi_init_dp_connector(struct intel_digital_port *dig_port) > } > } > > - return connector; > + return 0; > } > > static int intel_hdmi_reset_link(struct intel_encoder *encoder, @@ -4748,20 > +4747,19 @@ static bool bdw_digital_port_connected(struct intel_encoder > *encoder) > return intel_de_read(dev_priv, GEN8_DE_PORT_ISR) & bit; } > > -static struct intel_connector * > -intel_ddi_init_hdmi_connector(struct intel_digital_port *dig_port) > +static int intel_ddi_init_hdmi_connector(struct intel_digital_port > +*dig_port) > { > struct intel_connector *connector; > enum port port = dig_port->base.port; > > connector = intel_connector_alloc(); > if (!connector) > - return NULL; > + return -ENOMEM; > > dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port); > intel_hdmi_init_connector(dig_port, connector); > > - return connector; > + return 0; > } > > static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dig_port) @@ > -5306,7 +5304,7 @@ void intel_ddi_init(struct intel_display *display, > intel_infoframe_init(dig_port); > > if (init_dp) { > - if (!intel_ddi_init_dp_connector(dig_port)) > + if (intel_ddi_init_dp_connector(dig_port)) > goto err; > > dig_port->hpd_pulse = intel_dp_hpd_pulse; @@ -5320,7 > +5318,7 @@ void intel_ddi_init(struct intel_display *display, > * but leave it just in case we have some really bad VBTs... > */ > if (encoder->type != INTEL_OUTPUT_EDP && init_hdmi) { > - if (!intel_ddi_init_hdmi_connector(dig_port)) > + if (intel_ddi_init_hdmi_connector(dig_port)) > goto err; > } > > -- > 2.39.5
On Fri, 13 Dec 2024, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote: >> -----Original Message----- >> From: Nikula, Jani <jani.nikula@intel.com> >> Sent: Friday, December 13, 2024 7:46 PM >> To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org >> Cc: Nikula, Jani <jani.nikula@intel.com>; Sergey Senozhatsky >> <senozhatsky@chromium.org>; Ville Syrjala <ville.syrjala@linux.intel.com>; >> Kandpal, Suraj <suraj.kandpal@intel.com> >> Subject: [PATCH v3 1/6] drm/i915/ddi: change >> intel_ddi_init_{dp,hdmi}_connector() return type >> >> The caller doesn't actually need the returned struct intel_connector; it's >> stored in the ->attached_connector of intel_dp ad intel_hdmi. Switch to > > Typo : *and > Otherwise LGTM, > Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> Thanks, care to look at patch 4 as well, it's changed slightly? BR, Jani. > >> returning an int with 0 for success and negative errors codes to be able to >> indicate success even when we don't have a connector. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_ddi.c | 20 +++++++++----------- >> 1 file changed, 9 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c >> b/drivers/gpu/drm/i915/display/intel_ddi.c >> index 4f9c50996446..21277cf8afef 100644 >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c >> @@ -4542,8 +4542,7 @@ static const struct drm_encoder_funcs >> intel_ddi_funcs = { >> .late_register = intel_ddi_encoder_late_register, }; >> >> -static struct intel_connector * >> -intel_ddi_init_dp_connector(struct intel_digital_port *dig_port) >> +static int intel_ddi_init_dp_connector(struct intel_digital_port >> +*dig_port) >> { >> struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); >> struct intel_connector *connector; >> @@ -4551,7 +4550,7 @@ intel_ddi_init_dp_connector(struct >> intel_digital_port *dig_port) >> >> connector = intel_connector_alloc(); >> if (!connector) >> - return NULL; >> + return -ENOMEM; >> >> dig_port->dp.output_reg = DDI_BUF_CTL(port); >> if (DISPLAY_VER(i915) >= 14) >> @@ -4566,7 +4565,7 @@ intel_ddi_init_dp_connector(struct >> intel_digital_port *dig_port) >> >> if (!intel_dp_init_connector(dig_port, connector)) { >> kfree(connector); >> - return NULL; >> + return -EINVAL; >> } >> >> if (dig_port->base.type == INTEL_OUTPUT_EDP) { @@ -4582,7 >> +4581,7 @@ intel_ddi_init_dp_connector(struct intel_digital_port *dig_port) >> } >> } >> >> - return connector; >> + return 0; >> } >> >> static int intel_hdmi_reset_link(struct intel_encoder *encoder, @@ -4748,20 >> +4747,19 @@ static bool bdw_digital_port_connected(struct intel_encoder >> *encoder) >> return intel_de_read(dev_priv, GEN8_DE_PORT_ISR) & bit; } >> >> -static struct intel_connector * >> -intel_ddi_init_hdmi_connector(struct intel_digital_port *dig_port) >> +static int intel_ddi_init_hdmi_connector(struct intel_digital_port >> +*dig_port) >> { >> struct intel_connector *connector; >> enum port port = dig_port->base.port; >> >> connector = intel_connector_alloc(); >> if (!connector) >> - return NULL; >> + return -ENOMEM; >> >> dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port); >> intel_hdmi_init_connector(dig_port, connector); >> >> - return connector; >> + return 0; >> } >> >> static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dig_port) @@ >> -5306,7 +5304,7 @@ void intel_ddi_init(struct intel_display *display, >> intel_infoframe_init(dig_port); >> >> if (init_dp) { >> - if (!intel_ddi_init_dp_connector(dig_port)) >> + if (intel_ddi_init_dp_connector(dig_port)) >> goto err; >> >> dig_port->hpd_pulse = intel_dp_hpd_pulse; @@ -5320,7 >> +5318,7 @@ void intel_ddi_init(struct intel_display *display, >> * but leave it just in case we have some really bad VBTs... >> */ >> if (encoder->type != INTEL_OUTPUT_EDP && init_hdmi) { >> - if (!intel_ddi_init_hdmi_connector(dig_port)) >> + if (intel_ddi_init_hdmi_connector(dig_port)) >> goto err; >> } >> >> -- >> 2.39.5 >
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 4f9c50996446..21277cf8afef 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -4542,8 +4542,7 @@ static const struct drm_encoder_funcs intel_ddi_funcs = { .late_register = intel_ddi_encoder_late_register, }; -static struct intel_connector * -intel_ddi_init_dp_connector(struct intel_digital_port *dig_port) +static int intel_ddi_init_dp_connector(struct intel_digital_port *dig_port) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); struct intel_connector *connector; @@ -4551,7 +4550,7 @@ intel_ddi_init_dp_connector(struct intel_digital_port *dig_port) connector = intel_connector_alloc(); if (!connector) - return NULL; + return -ENOMEM; dig_port->dp.output_reg = DDI_BUF_CTL(port); if (DISPLAY_VER(i915) >= 14) @@ -4566,7 +4565,7 @@ intel_ddi_init_dp_connector(struct intel_digital_port *dig_port) if (!intel_dp_init_connector(dig_port, connector)) { kfree(connector); - return NULL; + return -EINVAL; } if (dig_port->base.type == INTEL_OUTPUT_EDP) { @@ -4582,7 +4581,7 @@ intel_ddi_init_dp_connector(struct intel_digital_port *dig_port) } } - return connector; + return 0; } static int intel_hdmi_reset_link(struct intel_encoder *encoder, @@ -4748,20 +4747,19 @@ static bool bdw_digital_port_connected(struct intel_encoder *encoder) return intel_de_read(dev_priv, GEN8_DE_PORT_ISR) & bit; } -static struct intel_connector * -intel_ddi_init_hdmi_connector(struct intel_digital_port *dig_port) +static int intel_ddi_init_hdmi_connector(struct intel_digital_port *dig_port) { struct intel_connector *connector; enum port port = dig_port->base.port; connector = intel_connector_alloc(); if (!connector) - return NULL; + return -ENOMEM; dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port); intel_hdmi_init_connector(dig_port, connector); - return connector; + return 0; } static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dig_port) @@ -5306,7 +5304,7 @@ void intel_ddi_init(struct intel_display *display, intel_infoframe_init(dig_port); if (init_dp) { - if (!intel_ddi_init_dp_connector(dig_port)) + if (intel_ddi_init_dp_connector(dig_port)) goto err; dig_port->hpd_pulse = intel_dp_hpd_pulse; @@ -5320,7 +5318,7 @@ void intel_ddi_init(struct intel_display *display, * but leave it just in case we have some really bad VBTs... */ if (encoder->type != INTEL_OUTPUT_EDP && init_hdmi) { - if (!intel_ddi_init_hdmi_connector(dig_port)) + if (intel_ddi_init_hdmi_connector(dig_port)) goto err; }
The caller doesn't actually need the returned struct intel_connector; it's stored in the ->attached_connector of intel_dp ad intel_hdmi. Switch to returning an int with 0 for success and negative errors codes to be able to indicate success even when we don't have a connector. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/intel_ddi.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)