Skip to content

Commit

Permalink
drm/i915/dpcd_bl: Handle drm_dpcd_read/write() return values correctly
Browse files Browse the repository at this point in the history
This is kind of an annoying aspect of DRM's DP helpers:
drm_dp_dpcd_readb/writeb() return the size of bytes read/written on
success, thus we want to check against that instead of checking if the
return value is less than 0.

I'll probably be fixing this in the near future once I start doing DP work
again, also because I'd rather not mix a tree-wide refactor like that in
with a patch series intended to be around introducing DP backlight helpers.
So, for now let's just handle the return values from each function
correctly.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210514181504.565252-3-lyude@redhat.com
  • Loading branch information
Lyude Paul committed Jun 9, 2021
1 parent 4154fa0 commit 3faea99
Showing 1 changed file with 19 additions and 22 deletions.
41 changes: 19 additions & 22 deletions drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ intel_dp_aux_supports_hdr_backlight(struct intel_connector *connector)
u8 tcon_cap[4];

ret = drm_dp_dpcd_read(aux, INTEL_EDP_HDR_TCON_CAP0, tcon_cap, sizeof(tcon_cap));
if (ret < 0)
if (ret != sizeof(tcon_cap))
return false;

if (!(tcon_cap[1] & INTEL_EDP_HDR_TCON_BRIGHTNESS_NITS_CAP))
Expand Down Expand Up @@ -137,7 +137,7 @@ intel_dp_aux_hdr_get_backlight(struct intel_connector *connector, enum pipe pipe
u8 tmp;
u8 buf[2] = { 0 };

if (drm_dp_dpcd_readb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, &tmp) < 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");
return 0;
}
Expand All @@ -153,7 +153,8 @@ intel_dp_aux_hdr_get_backlight(struct intel_connector *connector, enum pipe pipe
return panel->backlight.max;
}

if (drm_dp_dpcd_read(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB, buf, sizeof(buf)) < 0) {
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");
return 0;
}
Expand All @@ -172,7 +173,8 @@ intel_dp_aux_hdr_set_aux_backlight(const struct drm_connector_state *conn_state,
buf[0] = level & 0xFF;
buf[1] = (level & 0xFF00) >> 8;

if (drm_dp_dpcd_write(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB, buf, 4) < 0)
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");
}

Expand Down Expand Up @@ -203,7 +205,7 @@ intel_dp_aux_hdr_enable_backlight(const struct intel_crtc_state *crtc_state,
u8 old_ctrl, ctrl;

ret = drm_dp_dpcd_readb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, &old_ctrl);
if (ret < 0) {
if (ret != 1) {
drm_err(&i915->drm, "Failed to read current backlight control mode: %d\n", ret);
return;
}
Expand All @@ -221,7 +223,7 @@ intel_dp_aux_hdr_enable_backlight(const struct intel_crtc_state *crtc_state,
}

if (ctrl != old_ctrl)
if (drm_dp_dpcd_writeb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, ctrl) < 0)
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");
}

Expand Down Expand Up @@ -277,8 +279,7 @@ static void set_vesa_backlight_enable(struct intel_dp *intel_dp, bool enable)
if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP))
return;

if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
&reg_val) < 0) {
if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, &reg_val) != 1) {
drm_dbg_kms(&i915->drm, "Failed to read DPCD register 0x%x\n",
DP_EDP_DISPLAY_CONTROL_REGISTER);
return;
Expand Down Expand Up @@ -332,8 +333,8 @@ 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(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
&read_val, sizeof(read_val)) < 0) {
if (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, &read_val,
sizeof(read_val)) != sizeof(read_val)) {
drm_dbg_kms(&i915->drm, "Failed to read DPCD register 0x%x\n",
DP_EDP_BACKLIGHT_BRIGHTNESS_MSB);
return 0;
Expand Down Expand Up @@ -365,8 +366,8 @@ intel_dp_aux_vesa_set_backlight(const struct drm_connector_state *conn_state,
vals[0] = (level & 0xFF00) >> 8;
vals[1] = (level & 0xFF);
}
if (drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
vals, sizeof(vals)) < 0) {
if (drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, vals,
sizeof(vals)) != sizeof(vals)) {
drm_dbg_kms(&i915->drm,
"Failed to write aux backlight level\n");
return;
Expand Down Expand Up @@ -401,9 +402,8 @@ intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state,
new_dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
new_dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;

if (drm_dp_dpcd_writeb(&intel_dp->aux,
DP_EDP_PWMGEN_BIT_COUNT,
pwmgen_bit_count) < 0)
if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_PWMGEN_BIT_COUNT,
pwmgen_bit_count) != 1)
drm_dbg_kms(&i915->drm,
"Failed to write aux pwmgen bit count\n");

Expand All @@ -424,11 +424,9 @@ intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state,
}

if (new_dpcd_buf != dpcd_buf) {
if (drm_dp_dpcd_writeb(&intel_dp->aux,
DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < 0) {
drm_dbg_kms(&i915->drm,
"Failed to write aux backlight mode\n");
}
if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
new_dpcd_buf) != 1)
drm_dbg_kms(&i915->drm, "Failed to write aux backlight mode\n");
}

intel_dp_aux_vesa_set_backlight(conn_state, level);
Expand Down Expand Up @@ -519,8 +517,7 @@ static u32 intel_dp_aux_vesa_calc_max_backlight(struct intel_connector *connecto
}

drm_dbg_kms(&i915->drm, "Using eDP pwmgen bit count of %d\n", pn);
if (drm_dp_dpcd_writeb(&intel_dp->aux,
DP_EDP_PWMGEN_BIT_COUNT, pn) < 0) {
if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_PWMGEN_BIT_COUNT, pn) != 1) {
drm_dbg_kms(&i915->drm,
"Failed to write aux pwmgen bit count\n");
return max_backlight;
Expand Down

0 comments on commit 3faea99

Please sign in to comment.