On Mon, 08 Apr 2024, Ankit Nautiyal <[email protected]> wrote: > Currently lspcon_resume calls lspcon_init and in case of failure we get > error messages from lspcon_init and then again from lspcon_resume. > > Just have a single error message in lspcon_init and convert all other > errors as dbg messages. > > Signed-off-by: Ankit Nautiyal <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_lspcon.c | 27 +++++++++++---------- > 1 file changed, 14 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c > b/drivers/gpu/drm/i915/display/intel_lspcon.c > index 16ee0dc179f7..3c3bc80e32f0 100644 > --- a/drivers/gpu/drm/i915/display/intel_lspcon.c > +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c > @@ -680,24 +680,30 @@ bool lspcon_init(struct intel_digital_port *dig_port) > return false; > > if (!lspcon_set_pcon_mode(lspcon)) { > - drm_err(&i915->drm, "LSPCON mode change to PCON failed\n"); > - return false; > + drm_dbg_kms(&i915->drm, "LSPCON mode change to PCON failed\n"); > + goto lspcon_init_failed; > } > > if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd) != 0) { > - drm_err(&i915->drm, "LSPCON DPCD read failed\n"); > - return false; > + drm_dbg_kms(&i915->drm, "LSPCON DPCD read failed\n"); > + goto lspcon_init_failed; > } > > if (!lspcon_detect_vendor(lspcon)) { > - drm_err(&i915->drm, "LSPCON vendor detection failed\n"); > - return false; > + drm_dbg_kms(&i915->drm, "LSPCON vendor detection failed\n"); > + goto lspcon_init_failed;
Why not just keep all of the above as drm_err(), adding all the relevant info there... > } > > connector->ycbcr_420_allowed = true; > lspcon->active = true; > drm_dbg_kms(&i915->drm, "Success: LSPCON init\n"); > return true; > + > +lspcon_init_failed: > + drm_err(&i915->drm, "LSPCON init failed on port %c\n", > + port_name(dig_port->base.port)); > + > + return false; And dropping this change altogheter? Why would we need both the debug message and the error? Just have one error message? With that change, Reviewed-by: Jani Nikula <[email protected]> > } > > u32 intel_lspcon_infoframes_enabled(struct intel_encoder *encoder, > @@ -718,13 +724,8 @@ void lspcon_resume(struct intel_digital_port *dig_port) > if (!intel_bios_encoder_is_lspcon(dig_port->base.devdata)) > return; > > - if (!lspcon->active) { > - if (!lspcon_init(dig_port)) { > - drm_err(&i915->drm, "LSPCON init failed on port %c\n", > - port_name(dig_port->base.port)); > - return; > - } > - } > + if (!lspcon->active && !lspcon_init(dig_port)) > + return; > > expected_mode = lspcon_get_expected_mode(lspcon); > if (expected_mode == DRM_LSPCON_MODE_PCON) -- Jani Nikula, Intel
