Re: [Intel-gfx] [RFC v4 09/11] drm/i915/dpcd_bl: Print return codes for VESA backlight failures
On Mon, Feb 08, 2021 at 06:38:59PM -0500, Lyude Paul wrote:
> Also, stop printing the DPCD register that failed, and just describe it
> instead. Saves us from having to look up each register offset when reading
> through kernel logs (plus, DPCD dumping with drm.debug |= 0x100 will give
> us that anyway).
>
> Signed-off-by: Lyude Paul
Reviewed-by: Rodrigo Vivi
> ---
> .../drm/i915/display/intel_dp_aux_backlight.c | 101 +-
> 1 file changed, 52 insertions(+), 49 deletions(-)
>
> 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 a139f0e08839..a98d9bd4b0ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -274,14 +274,12 @@ static bool
> intel_dp_aux_vesa_backlight_dpcd_mode(struct intel_connector *connec
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + int ret;
> u8 mode_reg;
>
> - if (drm_dp_dpcd_readb(_dp->aux,
> - DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
> - _reg) != 1) {
> - drm_dbg_kms(>drm,
> - "Failed to read the DPCD register 0x%x\n",
> - DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
> + ret = drm_dp_dpcd_readb(_dp->aux,
> DP_EDP_BACKLIGHT_MODE_SET_REGISTER, _reg);
> + if (ret != 1) {
> + drm_dbg_kms(>drm, "Failed to read backlight mode: %d\n",
> ret);
> return false;
> }
>
> @@ -297,6 +295,7 @@ static u32 intel_dp_aux_vesa_get_backlight(struct
> intel_connector *connector, en
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + int ret;
> u8 read_val[2] = { 0x0 };
> u16 level = 0;
>
> @@ -307,10 +306,10 @@ static u32 intel_dp_aux_vesa_get_backlight(struct
> intel_connector *connector, en
> if (!intel_dp_aux_vesa_backlight_dpcd_mode(connector))
> return connector->panel.backlight.max;
>
> - if (drm_dp_dpcd_read(_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
> _val,
> - sizeof(read_val)) != sizeof(read_val)) {
> - drm_dbg_kms(>drm, "Failed to read DPCD register 0x%x\n",
> - DP_EDP_BACKLIGHT_BRIGHTNESS_MSB);
> + ret = drm_dp_dpcd_read(_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
> _val,
> +sizeof(read_val));
> + if (ret != sizeof(read_val)) {
> + drm_dbg_kms(>drm, "Failed to read brightness level:
> %d\n", ret);
> return 0;
> }
>
> @@ -333,6 +332,7 @@ intel_dp_aux_vesa_set_backlight(const struct
> drm_connector_state *conn_state,
> struct intel_connector *connector =
> to_intel_connector(conn_state->connector);
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + int ret;
> u8 vals[2] = { 0x0 };
>
> /* Write the MSB and/or LSB */
> @@ -343,10 +343,10 @@ intel_dp_aux_vesa_set_backlight(const struct
> drm_connector_state *conn_state,
> vals[0] = level;
> }
>
> - if (drm_dp_dpcd_write(_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
> vals,
> - sizeof(vals)) != sizeof(vals)) {
> - drm_dbg_kms(>drm,
> - "Failed to write aux backlight level\n");
> + ret = drm_dp_dpcd_write(_dp->aux,
> DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, vals,
> + sizeof(vals));
> + if (ret != sizeof(vals)) {
> + drm_dbg_kms(>drm, "Failed to write aux backlight level:
> %d\n", ret);
> return;
> }
> }
> @@ -355,26 +355,28 @@ static void set_vesa_backlight_enable(struct
> intel_connector *connector, bool en
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + int ret;
> u8 reg_val = 0;
>
> /* Early return when display use other mechanism to enable backlight. */
> if (!connector->panel.backlight.edp.vesa.aux_enable)
> return;
>
> - if (drm_dp_dpcd_readb(_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
> _val) != 1) {
> - drm_dbg_kms(>drm, "Failed to read DPCD register 0x%x\n",
> - DP_EDP_DISPLAY_CONTROL_REGISTER);
> + ret = drm_dp_dpcd_readb(_dp->aux,
> DP_EDP_DISPLAY_CONTROL_REGISTER, _val);
> + if (ret != 1) {
> + drm_dbg_kms(>drm, "Failed to read eDP display control
> register: %d\n", ret);
> return;
> }
> +
> if (enable)
> reg_val |= DP_EDP_BACKLIGHT_ENABLE;
> else
> reg_val &= ~(DP_EDP_BACKLIGHT_ENABLE);
>
> - if (drm_dp_dpcd_writeb(_dp->aux,
[Intel-gfx] [RFC v4 09/11] drm/i915/dpcd_bl: Print return codes for VESA backlight failures
Also, stop printing the DPCD register that failed, and just describe it
instead. Saves us from having to look up each register offset when reading
through kernel logs (plus, DPCD dumping with drm.debug |= 0x100 will give
us that anyway).
Signed-off-by: Lyude Paul
---
.../drm/i915/display/intel_dp_aux_backlight.c | 101 +-
1 file changed, 52 insertions(+), 49 deletions(-)
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 a139f0e08839..a98d9bd4b0ed 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -274,14 +274,12 @@ static bool intel_dp_aux_vesa_backlight_dpcd_mode(struct
intel_connector *connec
{
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ int ret;
u8 mode_reg;
- if (drm_dp_dpcd_readb(_dp->aux,
- DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
- _reg) != 1) {
- drm_dbg_kms(>drm,
- "Failed to read the DPCD register 0x%x\n",
- DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
+ ret = drm_dp_dpcd_readb(_dp->aux,
DP_EDP_BACKLIGHT_MODE_SET_REGISTER, _reg);
+ if (ret != 1) {
+ drm_dbg_kms(>drm, "Failed to read backlight mode: %d\n",
ret);
return false;
}
@@ -297,6 +295,7 @@ static u32 intel_dp_aux_vesa_get_backlight(struct
intel_connector *connector, en
{
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ int ret;
u8 read_val[2] = { 0x0 };
u16 level = 0;
@@ -307,10 +306,10 @@ static u32 intel_dp_aux_vesa_get_backlight(struct
intel_connector *connector, en
if (!intel_dp_aux_vesa_backlight_dpcd_mode(connector))
return connector->panel.backlight.max;
- if (drm_dp_dpcd_read(_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
_val,
-sizeof(read_val)) != sizeof(read_val)) {
- drm_dbg_kms(>drm, "Failed to read DPCD register 0x%x\n",
- DP_EDP_BACKLIGHT_BRIGHTNESS_MSB);
+ ret = drm_dp_dpcd_read(_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
_val,
+ sizeof(read_val));
+ if (ret != sizeof(read_val)) {
+ drm_dbg_kms(>drm, "Failed to read brightness level:
%d\n", ret);
return 0;
}
@@ -333,6 +332,7 @@ intel_dp_aux_vesa_set_backlight(const struct
drm_connector_state *conn_state,
struct intel_connector *connector =
to_intel_connector(conn_state->connector);
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ int ret;
u8 vals[2] = { 0x0 };
/* Write the MSB and/or LSB */
@@ -343,10 +343,10 @@ intel_dp_aux_vesa_set_backlight(const struct
drm_connector_state *conn_state,
vals[0] = level;
}
- if (drm_dp_dpcd_write(_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
vals,
- sizeof(vals)) != sizeof(vals)) {
- drm_dbg_kms(>drm,
- "Failed to write aux backlight level\n");
+ ret = drm_dp_dpcd_write(_dp->aux,
DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, vals,
+ sizeof(vals));
+ if (ret != sizeof(vals)) {
+ drm_dbg_kms(>drm, "Failed to write aux backlight level:
%d\n", ret);
return;
}
}
@@ -355,26 +355,28 @@ static void set_vesa_backlight_enable(struct
intel_connector *connector, bool en
{
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ int ret;
u8 reg_val = 0;
/* Early return when display use other mechanism to enable backlight. */
if (!connector->panel.backlight.edp.vesa.aux_enable)
return;
- if (drm_dp_dpcd_readb(_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
_val) != 1) {
- drm_dbg_kms(>drm, "Failed to read DPCD register 0x%x\n",
- DP_EDP_DISPLAY_CONTROL_REGISTER);
+ ret = drm_dp_dpcd_readb(_dp->aux,
DP_EDP_DISPLAY_CONTROL_REGISTER, _val);
+ if (ret != 1) {
+ drm_dbg_kms(>drm, "Failed to read eDP display control
register: %d\n", ret);
return;
}
+
if (enable)
reg_val |= DP_EDP_BACKLIGHT_ENABLE;
else
reg_val &= ~(DP_EDP_BACKLIGHT_ENABLE);
- if (drm_dp_dpcd_writeb(_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
- reg_val) != 1) {
- drm_dbg_kms(>drm, "Failed to %s aux backlight\n",
- enable ? "enable" : "disable");
+
