From: Jani Nikula <jani.nikula@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Make backlight setup debugs consistent
Date: Wed, 15 Feb 2023 16:11:02 +0200 [thread overview]
Message-ID: <87lekzqart.fsf@intel.com> (raw)
In-Reply-To: <20230215135616.30411-1-ville.syrjala@linux.intel.com>
On Wed, 15 Feb 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> It's confusing to debug backlight issues when one can't
> easily even tell what kind of backlight control was
> selected. Sprinkle uniform debug messages to all the
> backlight setup functions.
>
> Also the one that was already there (ext_pwm) was
> using drm_info() for some reason. I don't think that's
> warranted so switch it to drm_dbg_kms() as well.
>
> v2: Deal with AUX backlights too (Jani)
> Move the VLV/CHV initial pipe debug there too (Jani)
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> .../gpu/drm/i915/display/intel_backlight.c | 36 +++++++++++++++++--
> drivers/gpu/drm/i915/display/intel_dp.c | 5 ---
> .../drm/i915/display/intel_dp_aux_backlight.c | 29 +++++++++++----
> .../i915/display/intel_dsi_dcs_backlight.c | 5 +++
> 4 files changed, 62 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index a4e4b7f79e4d..cb1e4423decb 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -1270,6 +1270,10 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
> cpu_ctl2 & ~BLM_PWM_ENABLE);
> }
>
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] Using native PCH PWM for backlight control\n",
> + connector->base.base.id, connector->base.name);
> +
> return 0;
> }
>
> @@ -1297,6 +1301,10 @@ static int pch_setup_backlight(struct intel_connector *connector, enum pipe unus
> panel->backlight.pwm_enabled = (cpu_ctl2 & BLM_PWM_ENABLE) &&
> (pch_ctl1 & BLM_PCH_PWM_ENABLE);
>
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] Using native PCH PWM for backlight control\n",
> + connector->base.base.id, connector->base.name);
> +
> return 0;
> }
>
> @@ -1335,6 +1343,10 @@ static int i9xx_setup_backlight(struct intel_connector *connector, enum pipe unu
>
> panel->backlight.pwm_enabled = val != 0;
>
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] Using native PWM for backlight control\n",
> + connector->base.base.id, connector->base.name);
> +
> return 0;
> }
>
> @@ -1364,6 +1376,10 @@ static int i965_setup_backlight(struct intel_connector *connector, enum pipe unu
>
> panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;
>
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] Using native PWM for backlight control\n",
> + connector->base.base.id, connector->base.name);
> +
> return 0;
> }
>
> @@ -1392,6 +1408,10 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe
>
> panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;
>
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] Using native PWM for backlight control (on pipe %c)\n",
> + connector->base.base.id, connector->base.name, pipe_name(pipe));
> +
> return 0;
> }
>
> @@ -1428,6 +1448,11 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused)
>
> panel->backlight.pwm_enabled = pwm_ctl & BXT_BLC_PWM_ENABLE;
>
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] Using native PWM for backlight control (controller=%d)\n",
> + connector->base.base.id, connector->base.name,
> + panel->backlight.controller);
> +
> return 0;
> }
>
> @@ -1490,6 +1515,11 @@ cnp_setup_backlight(struct intel_connector *connector, enum pipe unused)
>
> panel->backlight.pwm_enabled = pwm_ctl & BXT_BLC_PWM_ENABLE;
>
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] Using native PCH PWM for backlight control (controller=%d)\n",
> + connector->base.base.id, connector->base.name,
> + panel->backlight.controller);
> +
> return 0;
> }
>
> @@ -1538,8 +1568,10 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector,
> NSEC_PER_SEC / get_vbt_pwm_freq(connector);
> }
>
> - drm_info(&i915->drm, "Using %s PWM for LCD backlight control\n",
> - desc);
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] Using %s PWM for backlight control\n",
> + connector->base.base.id, connector->base.name, desc);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index b92e0b0f5369..717be9a9ef5b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5256,11 +5256,6 @@ static void intel_edp_backlight_setup(struct intel_dp *intel_dp,
>
> if (pipe != PIPE_A && pipe != PIPE_B)
> pipe = PIPE_A;
> -
> - drm_dbg_kms(&i915->drm,
> - "[CONNECTOR:%d:%s] using pipe %c for initial backlight setup\n",
> - connector->base.base.id, connector->base.name,
> - pipe_name(pipe));
> }
>
> intel_backlight_setup(connector, pipe);
> 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..8670b6956fee 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -273,6 +273,11 @@ intel_dp_aux_hdr_disable_backlight(const struct drm_connector_state *conn_state,
> panel->backlight.pwm_funcs->disable(conn_state, intel_backlight_invert_pwm_level(connector, 0));
> }
>
> +static const char *dpcd_vs_pwm_str(bool aux)
> +{
> + return aux ? "DPCD" : "PWM";
> +}
> +
> static int
> intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pipe)
> {
> @@ -282,11 +287,11 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi
> &connector->base.display_info.luminance_range;
> int ret;
>
> - if (panel->backlight.edp.intel.sdr_uses_aux) {
> - drm_dbg_kms(&i915->drm, "SDR backlight is controlled through DPCD\n");
> - } else {
> - drm_dbg_kms(&i915->drm, "SDR backlight is controlled through PWM\n");
> + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] SDR backlight is controlled through %s\n",
> + connector->base.base.id, connector->base.name,
> + dpcd_vs_pwm_str(panel->backlight.edp.intel.sdr_uses_aux));
>
> + if (!panel->backlight.edp.intel.sdr_uses_aux) {
> ret = panel->backlight.pwm_funcs->setup(connector, pipe);
> if (ret < 0) {
> drm_err(&i915->drm,
> @@ -303,8 +308,10 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi
> panel->backlight.min = 0;
> }
>
> - drm_dbg_kms(&i915->drm, "Using backlight range %d..%d\n", panel->backlight.min,
> - panel->backlight.max);
> + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Using AUX HDR interface for backlight control (range %d..%d)\n",
> + connector->base.base.id, connector->base.name,
> + panel->backlight.min, panel->backlight.max);
> +
>
> panel->backlight.level = intel_dp_aux_hdr_get_backlight(connector, pipe);
> panel->backlight.enabled = panel->backlight.level != 0;
> @@ -386,6 +393,13 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
> if (ret < 0)
> return ret;
>
> + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] AUX VESA backlight enable is controlled through %s\n",
> + connector->base.base.id, connector->base.name,
> + dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_enable));
> + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] AUX VESA backlight level is controlled through %s\n",
> + connector->base.base.id, connector->base.name,
> + dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_set));
> +
> if (!panel->backlight.edp.vesa.info.aux_set || !panel->backlight.edp.vesa.info.aux_enable) {
> ret = panel->backlight.pwm_funcs->setup(connector, pipe);
> if (ret < 0) {
> @@ -418,6 +432,9 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
> }
> }
>
> + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Using AUX VESA interface for backlight control\n",
> + connector->base.base.id, connector->base.name);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c
> index 20e466d843ce..049443245310 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c
> @@ -162,6 +162,7 @@ static void dcs_enable_backlight(const struct intel_crtc_state *crtc_state,
> static int dcs_setup_backlight(struct intel_connector *connector,
> enum pipe unused)
> {
> + struct drm_i915_private *i915 = to_i915(connector->base.dev);
> struct intel_panel *panel = &connector->panel;
>
> if (panel->vbt.backlight.brightness_precision_bits > 8)
> @@ -171,6 +172,10 @@ static int dcs_setup_backlight(struct intel_connector *connector,
>
> panel->backlight.level = panel->backlight.max;
>
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] Using DCS for backlight control\n",
> + connector->base.base.id, connector->base.name);
> +
> return 0;
> }
--
Jani Nikula, Intel Open Source Graphics Center
next prev parent reply other threads:[~2023-02-15 14:11 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-14 13:47 [Intel-gfx] [PATCH 1/3] drm/i915: Make backlight setup debugs consistent Ville Syrjala
2023-02-14 13:47 ` [Intel-gfx] [PATCH 2/3] drm/i915: Don't hide function calls with side effects Ville Syrjala
2023-02-15 10:15 ` Jani Nikula
2023-02-14 13:47 ` [Intel-gfx] [PATCH 3/3] drm/i915: Clean up g4x+ sprite TILEOFF programming Ville Syrjala
2023-02-15 10:16 ` Jani Nikula
2023-02-14 16:25 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Make backlight setup debugs consistent Patchwork
2023-02-15 3:28 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2023-02-15 10:14 ` [Intel-gfx] [PATCH 1/3] " Jani Nikula
2023-02-15 10:41 ` Ville Syrjälä
2023-02-15 13:56 ` [Intel-gfx] [PATCH v2 " Ville Syrjala
2023-02-15 14:11 ` Jani Nikula [this message]
2023-02-15 14:58 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Make backlight setup debugs consistent (rev2) Patchwork
2023-02-16 6:54 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87lekzqart.fsf@intel.com \
--to=jani.nikula@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox