On Wed, 15 Feb 2023, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > With multi panel machines becoming more prominent it's also > important to know which connector's backlight we're talking > about. Include that information in all the backlight debug/error > messages. > > Signed-off-by: Ville Syrjälä <[email protected]>
Reviewed-by: Jani Nikula <[email protected]> > --- > .../gpu/drm/i915/display/intel_backlight.c | 60 ++++++++++++------- > .../drm/i915/display/intel_dp_aux_backlight.c | 55 ++++++++++------- > 2 files changed, 72 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c > b/drivers/gpu/drm/i915/display/intel_backlight.c > index a4e4b7f79e4d..7e076bd5f07c 100644 > --- a/drivers/gpu/drm/i915/display/intel_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_backlight.c > @@ -105,7 +105,8 @@ void intel_backlight_set_pwm_level(const struct > drm_connector_state *conn_state, > struct drm_i915_private *i915 = to_i915(connector->base.dev); > struct intel_panel *panel = &connector->panel; > > - drm_dbg_kms(&i915->drm, "set backlight PWM = %d\n", val); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] set backlight PWM = %d\n", > + connector->base.base.id, connector->base.name, val); > panel->backlight.pwm_funcs->set(conn_state, val); > } > > @@ -283,7 +284,8 @@ intel_panel_actually_set_backlight(const struct > drm_connector_state *conn_state, > struct drm_i915_private *i915 = to_i915(connector->base.dev); > struct intel_panel *panel = &connector->panel; > > - drm_dbg_kms(&i915->drm, "set backlight level = %d\n", level); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] set backlight level = %d\n", > + connector->base.base.id, connector->base.name, level); > > panel->backlight.funcs->set(conn_state, level); > } > @@ -345,7 +347,8 @@ static void lpt_disable_backlight(const struct > drm_connector_state *old_conn_sta > */ > tmp = intel_de_read(i915, BLC_PWM_CPU_CTL2); > if (tmp & BLM_PWM_ENABLE) { > - drm_dbg_kms(&i915->drm, "cpu backlight was enabled, > disabling\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] CPU backlight was > enabled, disabling\n", > + connector->base.base.id, connector->base.name); > intel_de_write(i915, BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE); > } > > @@ -458,7 +461,8 @@ void intel_backlight_disable(const struct > drm_connector_state *old_conn_state) > * another client is not activated. > */ > if (i915->drm.switch_power_state == DRM_SWITCH_POWER_CHANGING) { > - drm_dbg_kms(&i915->drm, "Skipping backlight disable on vga > switch\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Skipping backlight > disable on vga switch\n", > + connector->base.base.id, connector->base.name); > return; > } > > @@ -482,7 +486,8 @@ static void lpt_enable_backlight(const struct > intel_crtc_state *crtc_state, > > pch_ctl1 = intel_de_read(i915, BLC_PWM_PCH_CTL1); > if (pch_ctl1 & BLM_PCH_PWM_ENABLE) { > - drm_dbg_kms(&i915->drm, "pch backlight already enabled\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] PCH backlight > already enabled\n", > + connector->base.base.id, connector->base.name); > pch_ctl1 &= ~BLM_PCH_PWM_ENABLE; > intel_de_write(i915, BLC_PWM_PCH_CTL1, pch_ctl1); > } > @@ -533,14 +538,16 @@ static void pch_enable_backlight(const struct > intel_crtc_state *crtc_state, > > cpu_ctl2 = intel_de_read(i915, BLC_PWM_CPU_CTL2); > if (cpu_ctl2 & BLM_PWM_ENABLE) { > - drm_dbg_kms(&i915->drm, "cpu backlight already enabled\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] CPU backlight > already enabled\n", > + connector->base.base.id, connector->base.name); > cpu_ctl2 &= ~BLM_PWM_ENABLE; > intel_de_write(i915, BLC_PWM_CPU_CTL2, cpu_ctl2); > } > > pch_ctl1 = intel_de_read(i915, BLC_PWM_PCH_CTL1); > if (pch_ctl1 & BLM_PCH_PWM_ENABLE) { > - drm_dbg_kms(&i915->drm, "pch backlight already enabled\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] PCH backlight > already enabled\n", > + connector->base.base.id, connector->base.name); > pch_ctl1 &= ~BLM_PCH_PWM_ENABLE; > intel_de_write(i915, BLC_PWM_PCH_CTL1, pch_ctl1); > } > @@ -578,7 +585,8 @@ static void i9xx_enable_backlight(const struct > intel_crtc_state *crtc_state, > > ctl = intel_de_read(i915, BLC_PWM_CTL); > if (ctl & BACKLIGHT_DUTY_CYCLE_MASK_PNV) { > - drm_dbg_kms(&i915->drm, "backlight already enabled\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] backlight already > enabled\n", > + connector->base.base.id, connector->base.name); > intel_de_write(i915, BLC_PWM_CTL, 0); > } > > @@ -618,7 +626,8 @@ static void i965_enable_backlight(const struct > intel_crtc_state *crtc_state, > > ctl2 = intel_de_read(i915, BLC_PWM_CTL2); > if (ctl2 & BLM_PWM_ENABLE) { > - drm_dbg_kms(&i915->drm, "backlight already enabled\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] backlight already > enabled\n", > + connector->base.base.id, connector->base.name); > ctl2 &= ~BLM_PWM_ENABLE; > intel_de_write(i915, BLC_PWM_CTL2, ctl2); > } > @@ -653,7 +662,8 @@ static void vlv_enable_backlight(const struct > intel_crtc_state *crtc_state, > > ctl2 = intel_de_read(i915, VLV_BLC_PWM_CTL2(pipe)); > if (ctl2 & BLM_PWM_ENABLE) { > - drm_dbg_kms(&i915->drm, "backlight already enabled\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] backlight already > enabled\n", > + connector->base.base.id, connector->base.name); > ctl2 &= ~BLM_PWM_ENABLE; > intel_de_write(i915, VLV_BLC_PWM_CTL2(pipe), ctl2); > } > @@ -685,7 +695,8 @@ static void bxt_enable_backlight(const struct > intel_crtc_state *crtc_state, > if (panel->backlight.controller == 1) { > val = intel_de_read(i915, UTIL_PIN_CTL); > if (val & UTIL_PIN_ENABLE) { > - drm_dbg_kms(&i915->drm, "util pin already enabled\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] utility pin > already enabled\n", > + connector->base.base.id, > connector->base.name); > val &= ~UTIL_PIN_ENABLE; > intel_de_write(i915, UTIL_PIN_CTL, val); > } > @@ -699,7 +710,8 @@ static void bxt_enable_backlight(const struct > intel_crtc_state *crtc_state, > > pwm_ctl = intel_de_read(i915, > BXT_BLC_PWM_CTL(panel->backlight.controller)); > if (pwm_ctl & BXT_BLC_PWM_ENABLE) { > - drm_dbg_kms(&i915->drm, "backlight already enabled\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] backlight already > enabled\n", > + connector->base.base.id, connector->base.name); > pwm_ctl &= ~BXT_BLC_PWM_ENABLE; > intel_de_write(i915, > BXT_BLC_PWM_CTL(panel->backlight.controller), > pwm_ctl); > @@ -1468,7 +1480,8 @@ cnp_setup_backlight(struct intel_connector *connector, > enum pipe unused) > */ > panel->backlight.controller = connector->panel.vbt.backlight.controller; > if (!cnp_backlight_controller_is_valid(i915, > panel->backlight.controller)) { > - drm_dbg_kms(&i915->drm, "Invalid backlight controller %d, > assuming 0\n", > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Invalid backlight > controller %d, assuming 0\n", > + connector->base.base.id, connector->base.name, > panel->backlight.controller); > panel->backlight.controller = 0; > } > @@ -1511,8 +1524,8 @@ static int ext_pwm_setup_backlight(struct > intel_connector *connector, > } > > if (IS_ERR(panel->backlight.pwm)) { > - drm_err(&i915->drm, "Failed to get the %s PWM chip\n", > - desc); > + drm_err(&i915->drm, "[CONNECTOR:%d:%s] Failed to get the %s PWM > chip\n", > + connector->base.base.id, connector->base.name, desc); > panel->backlight.pwm = NULL; > return -ENODEV; > } > @@ -1529,7 +1542,8 @@ static int ext_pwm_setup_backlight(struct > intel_connector *connector, > level = intel_backlight_invert_pwm_level(connector, level); > panel->backlight.pwm_enabled = true; > > - drm_dbg_kms(&i915->drm, "PWM already enabled at freq %ld, VBT > freq %d, level %d\n", > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] PWM already enabled > at freq %ld, VBT freq %d, level %d\n", > + connector->base.base.id, connector->base.name, > NSEC_PER_SEC / (unsigned > long)panel->backlight.pwm_state.period, > get_vbt_pwm_freq(connector), level); > } else { > @@ -1623,10 +1637,12 @@ int intel_backlight_setup(struct intel_connector > *connector, enum pipe pipe) > if (!connector->panel.vbt.backlight.present) { > if (intel_has_quirk(i915, QUIRK_BACKLIGHT_PRESENT)) { > drm_dbg_kms(&i915->drm, > - "no backlight present per VBT, but present > per quirk\n"); > + "[CONNECTOR:%d:%s] no backlight present per > VBT, but present per quirk\n", > + connector->base.base.id, > connector->base.name); > } else { > drm_dbg_kms(&i915->drm, > - "no backlight present per VBT\n"); > + "[CONNECTOR:%d:%s] no backlight present per > VBT\n", > + connector->base.base.id, > connector->base.name); > return 0; > } > } > @@ -1642,16 +1658,16 @@ int intel_backlight_setup(struct intel_connector > *connector, enum pipe pipe) > > if (ret) { > drm_dbg_kms(&i915->drm, > - "failed to setup backlight for connector %s\n", > - connector->base.name); > + "[CONNECTOR:%d:%s] failed to setup backlight\n", > + connector->base.base.id, connector->base.name); > return ret; > } > > panel->backlight.present = true; > > drm_dbg_kms(&i915->drm, > - "Connector %s backlight initialized, %s, brightness > %u/%u\n", > - connector->base.name, > + "[CONNECTOR:%d:%s] backlight initialized, %s, brightness > %u/%u\n", > + connector->base.base.id, connector->base.name, > str_enabled_disabled(panel->backlight.enabled), > panel->backlight.level, panel->backlight.max); > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > index 83af95bce98d..aaecd1c38172 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > @@ -105,6 +105,11 @@ enum intel_dp_aux_backlight_modparam { > INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL = 3, > }; > > +static bool is_intel_tcon_cap(const u8 tcon_cap[4]) > +{ > + return tcon_cap[0] >= 1; > +} > + > /* Intel EDP backlight callbacks */ > static bool > intel_dp_aux_supports_hdr_backlight(struct intel_connector *connector) > @@ -125,14 +130,12 @@ intel_dp_aux_supports_hdr_backlight(struct > intel_connector *connector) > if (!(tcon_cap[1] & INTEL_EDP_HDR_TCON_BRIGHTNESS_NITS_CAP)) > return false; > > - if (tcon_cap[0] >= 1) { > - drm_dbg_kms(&i915->drm, "Detected Intel HDR backlight interface > version %d\n", > - tcon_cap[0]); > - } else { > - drm_dbg_kms(&i915->drm, "Detected unsupported HDR backlight > interface version %d\n", > - tcon_cap[0]); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Detected %s HDR backlight > interface version %d\n", > + connector->base.base.id, connector->base.name, > + is_intel_tcon_cap(tcon_cap) ? "Intel" : "unsupported", > tcon_cap[0]); > + > + if (!is_intel_tcon_cap(tcon_cap)) > return false; > - } > > /* > * If we don't have HDR static metadata there is no way to > @@ -147,7 +150,8 @@ intel_dp_aux_supports_hdr_backlight(struct > intel_connector *connector) > !(connector->base.hdr_sink_metadata.hdmi_type1.metadata_type & > BIT(HDMI_STATIC_METADATA_TYPE1))) { > drm_info(&i915->drm, > - "Panel is missing HDR static metadata. Possible > support for Intel HDR backlight interface is not used. If your backlight > controls don't work try booting with i915.enable_dpcd_backlight=%d. needs > this, please file a _new_ bug report on drm/i915, see " FDO_BUG_URL " for > details.\n", > + "[CONNECTOR:%d:%s] Panel is missing HDR static > metadata. Possible support for Intel HDR backlight interface is not used. If > your backlight controls don't work try booting with > i915.enable_dpcd_backlight=%d. needs this, please file a _new_ bug report on > drm/i915, see " FDO_BUG_URL " for details.\n", > + connector->base.base.id, connector->base.name, > INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL); > return false; > } > @@ -168,7 +172,8 @@ intel_dp_aux_hdr_get_backlight(struct intel_connector > *connector, enum pipe pipe > u8 buf[2] = { 0 }; > > if (drm_dp_dpcd_readb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, > &tmp) != 1) { > - drm_err(&i915->drm, "Failed to read current backlight mode from > DPCD\n"); > + drm_err(&i915->drm, "[CONNECTOR:%d:%s] Failed to read current > backlight mode from DPCD\n", > + connector->base.base.id, connector->base.name); > return 0; > } > > @@ -185,7 +190,8 @@ intel_dp_aux_hdr_get_backlight(struct intel_connector > *connector, enum pipe pipe > > if (drm_dp_dpcd_read(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB, buf, > sizeof(buf)) != sizeof(buf)) { > - drm_err(&i915->drm, "Failed to read brightness from DPCD\n"); > + drm_err(&i915->drm, "[CONNECTOR:%d:%s] Failed to read > brightness from DPCD\n", > + connector->base.base.id, connector->base.name); > return 0; > } > > @@ -205,7 +211,8 @@ intel_dp_aux_hdr_set_aux_backlight(const struct > drm_connector_state *conn_state, > > if (drm_dp_dpcd_write(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB, > buf, > sizeof(buf)) != sizeof(buf)) > - drm_err(dev, "Failed to write brightness level to DPCD\n"); > + drm_err(dev, "[CONNECTOR:%d:%s] Failed to write brightness > level to DPCD\n", > + connector->base.base.id, connector->base.name); > } > > static void > @@ -238,7 +245,8 @@ intel_dp_aux_hdr_enable_backlight(const struct > intel_crtc_state *crtc_state, > > ret = drm_dp_dpcd_readb(&intel_dp->aux, > INTEL_EDP_HDR_GETSET_CTRL_PARAMS, &old_ctrl); > if (ret != 1) { > - drm_err(&i915->drm, "Failed to read current backlight control > mode: %d\n", ret); > + drm_err(&i915->drm, "[CONNECTOR:%d:%s] Failed to read current > backlight control mode: %d\n", > + connector->base.base.id, connector->base.name, ret); > return; > } > > @@ -254,9 +262,10 @@ intel_dp_aux_hdr_enable_backlight(const struct > intel_crtc_state *crtc_state, > ctrl &= ~INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE; > } > > - if (ctrl != old_ctrl) > - if (drm_dp_dpcd_writeb(&intel_dp->aux, > INTEL_EDP_HDR_GETSET_CTRL_PARAMS, ctrl) != 1) > - drm_err(&i915->drm, "Failed to configure DPCD > brightness controls\n"); > + if (ctrl != old_ctrl && > + drm_dp_dpcd_writeb(&intel_dp->aux, > INTEL_EDP_HDR_GETSET_CTRL_PARAMS, ctrl) != 1) > + drm_err(&i915->drm, "[CONNECTOR:%d:%s] Failed to configure DPCD > brightness controls\n", > + connector->base.base.id, connector->base.name); > } > > static void > @@ -290,7 +299,8 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector > *connector, enum pipe pi > ret = panel->backlight.pwm_funcs->setup(connector, pipe); > if (ret < 0) { > drm_err(&i915->drm, > - "Failed to setup SDR backlight controls through > PWM: %d\n", ret); > + "[CONNECTOR:%d:%s] Failed to setup SDR > backlight controls through PWM: %d\n", > + connector->base.base.id, connector->base.name, > ret); > return ret; > } > } > @@ -390,8 +400,8 @@ static int intel_dp_aux_vesa_setup_backlight(struct > intel_connector *connector, > ret = panel->backlight.pwm_funcs->setup(connector, pipe); > if (ret < 0) { > drm_err(&i915->drm, > - "Failed to setup PWM backlight controls for eDP > backlight: %d\n", > - ret); > + "[CONNECTOR:%d:%s] Failed to setup PWM > backlight controls for eDP backlight: %d\n", > + connector->base.base.id, connector->base.name, > ret); > return ret; > } > } > @@ -428,7 +438,8 @@ intel_dp_aux_supports_vesa_backlight(struct > intel_connector *connector) > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > if (drm_edp_backlight_supported(intel_dp->edp_dpcd)) { > - drm_dbg_kms(&i915->drm, "AUX Backlight Control Supported!\n"); > + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] AUX Backlight > Control Supported!\n", > + connector->base.base.id, connector->base.name); > return true; > } > return false; > @@ -504,13 +515,15 @@ int intel_dp_aux_init_backlight_funcs(struct > intel_connector *connector) > * interfaces is to probe for Intel's first, and VESA's second. > */ > if (try_intel_interface && > intel_dp_aux_supports_hdr_backlight(connector)) { > - drm_dbg_kms(dev, "Using Intel proprietary eDP backlight > controls\n"); > + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Using Intel proprietary eDP > backlight controls\n", > + connector->base.base.id, connector->base.name); > panel->backlight.funcs = &intel_dp_hdr_bl_funcs; > return 0; > } > > if (try_vesa_interface && > intel_dp_aux_supports_vesa_backlight(connector)) { > - drm_dbg_kms(dev, "Using VESA eDP backlight controls\n"); > + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Using VESA eDP backlight > controls\n", > + connector->base.base.id, connector->base.name); > panel->backlight.funcs = &intel_dp_vesa_bl_funcs; > return 0; > } -- Jani Nikula, Intel Open Source Graphics Center
