On Mon, 08 Apr 2024, Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx> 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 <ankit.k.nautiyal@xxxxxxxxx>
> ---
> 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 <jani.nikula@xxxxxxxxx>
> }
>
> 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
