From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B1088C433E6 for ; Mon, 8 Feb 2021 11:27:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 67A5F64E42 for ; Mon, 8 Feb 2021 11:27:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231483AbhBHL1L (ORCPT ); Mon, 8 Feb 2021 06:27:11 -0500 Received: from mga17.intel.com ([192.55.52.151]:44579 "EHLO mga17.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232670AbhBHLMa (ORCPT ); Mon, 8 Feb 2021 06:12:30 -0500 IronPort-SDR: 8DsPo0kFK3zbbjOBYrwXBRq5+UkfBGTsMOcuKF7eArlSgfbSw+p6DvYpYU5emd9YPeIFCyI/cw 3qGClCNZMMpg== X-IronPort-AV: E=McAfee;i="6000,8403,9888"; a="161445974" X-IronPort-AV: E=Sophos;i="5.81,161,1610438400"; d="scan'208";a="161445974" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Feb 2021 03:11:49 -0800 IronPort-SDR: e/2aMYPsgB0iavTaYJAI7kdF/a2ofXD2ykrHQjpC7e5as5C4lWp0UOYJlJIeqW9a8akiivd9NP lVD8gImj68UQ== X-IronPort-AV: E=Sophos;i="5.81,161,1610438400"; d="scan'208";a="418743050" Received: from anveshag-mobl1.amr.corp.intel.com (HELO intel.com) ([10.209.119.193]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Feb 2021 03:11:47 -0800 Date: Mon, 8 Feb 2021 06:11:45 -0500 From: Rodrigo Vivi To: Lyude Paul Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, David Airlie , open list , Sean Paul Subject: Re: [RFC v3 04/10] drm/i915/dpcd_bl: Handle drm_dpcd_read/write() return values correctly Message-ID: <20210208111145.GA4798@intel.com> References: <20210205234515.1216538-1-lyude@redhat.com> <20210205234515.1216538-5-lyude@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210205234515.1216538-5-lyude@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 05, 2021 at 06:45:08PM -0500, Lyude Paul wrote: > 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 Reviewed-by: Rodrigo Vivi > --- > .../drm/i915/display/intel_dp_aux_backlight.c | 41 +++++++++---------- > 1 file changed, 19 insertions(+), 22 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 62294967f430..c37ccc8538cb 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > @@ -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)) > @@ -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; > } > @@ -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; > } > @@ -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"); > } > > @@ -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; > } > @@ -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"); > } > > @@ -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, > - ®_val) < 0) { > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, ®_val) != 1) { > drm_dbg_kms(&i915->drm, "Failed to read DPCD register 0x%x\n", > DP_EDP_DISPLAY_CONTROL_REGISTER); > return; > @@ -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; > @@ -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; > @@ -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"); > > @@ -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); > @@ -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; > -- > 2.29.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel