Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753685AbcC3O0x (ORCPT ); Wed, 30 Mar 2016 10:26:53 -0400 Received: from mga02.intel.com ([134.134.136.20]:8331 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752055AbcC3O0w (ORCPT ); Wed, 30 Mar 2016 10:26:52 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,416,1455004800"; d="scan'208";a="948063215" From: Jani Nikula To: Lyude , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: arthur.j.runyan@intel.com, Ville =?utf-8?B?U3lyasOkbMOk?= , Lyude , Daniel Vetter , David Airlie , open list Subject: Re: [PATCH v4 RESEND 5/5] drm/i915: Get rid of intel_dp_dpcd_read_wake() In-Reply-To: <1459175606-13875-6-git-send-email-cpaul@redhat.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <1459175606-13875-1-git-send-email-cpaul@redhat.com> <1459175606-13875-6-git-send-email-cpaul@redhat.com> User-Agent: Notmuch/0.21+91~gada1e33 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Wed, 30 Mar 2016 17:26:47 +0300 Message-ID: <87lh5011p4.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7223 Lines: 192 On Mon, 28 Mar 2016, Lyude wrote: > Since we've fixed up drm_dp_dpcd_read() to allow for retries when things > timeout, there's no use for having this function anymore. Good riddens. > > Signed-off-by: Lyude > --- > drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- > 1 file changed, 22 insertions(+), 57 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index f069a82..43c2933 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3184,47 +3184,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) > } > > /* > - * Native read with retry for link status and receiver capability reads for > - * cases where the sink may still be asleep. > - * > - * Sinks are *supposed* to come up within 1ms from an off state, but we're also > - * supposed to retry 3 times per the spec. > - */ > -static ssize_t > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, > - void *buffer, size_t size) > -{ > - ssize_t ret; > - int i; > - > - /* > - * Sometime we just get the same incorrect byte repeated > - * over the entire buffer. Doing just one throw away read > - * initially seems to "solve" it. > - */ > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); Ville, care to check this series (except patch 1) with your display to see if it still works? BR, Jani. > - > - for (i = 0; i < 3; i++) { > - ret = drm_dp_dpcd_read(aux, offset, buffer, size); > - if (ret == size) > - return ret; > - msleep(1); > - } > - > - return ret; > -} > - > -/* > * Fetch AUX CH registers 0x202 - 0x207 which contain > * link status information > */ > bool > intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) > { > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > - DP_LANE0_1_STATUS, > - link_status, > - DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > + return drm_dp_dpcd_read(&intel_dp->aux, DP_LANE0_1_STATUS, link_status, > + DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > } > > /* These are source-specific values. */ > @@ -3859,8 +3826,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > struct drm_i915_private *dev_priv = dev->dev_private; > uint8_t rev; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, > - sizeof(intel_dp->dpcd)) < 0) > + if (drm_dp_dpcd_read(&intel_dp->aux, 0x000, intel_dp->dpcd, > + sizeof(intel_dp->dpcd)) < 0) > return false; /* aux transfer failed */ > > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); > @@ -3871,9 +3838,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > /* Check if the panel supports PSR */ > memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd)); > if (is_edp(intel_dp)) { > - intel_dp_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT, > - intel_dp->psr_dpcd, > - sizeof(intel_dp->psr_dpcd)); > + drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, > + intel_dp->psr_dpcd, > + sizeof(intel_dp->psr_dpcd)); > if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) { > dev_priv->psr.sink_support = true; > DRM_DEBUG_KMS("Detected EDP PSR Panel.\n"); > @@ -3884,9 +3851,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > uint8_t frame_sync_cap; > > dev_priv->psr.sink_support = true; > - intel_dp_dpcd_read_wake(&intel_dp->aux, > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > - &frame_sync_cap, 1); > + drm_dp_dpcd_read(&intel_dp->aux, > + DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > + &frame_sync_cap, 1); > dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false; > /* PSR2 needs frame sync as well */ > dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync; > @@ -3902,15 +3869,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > /* Intermediate frequency support */ > if (is_edp(intel_dp) && > (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE) && > - (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > + (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > (rev >= 0x03)) { /* eDp v1.4 or higher */ > __le16 sink_rates[DP_MAX_SUPPORTED_RATES]; > int i; > > - intel_dp_dpcd_read_wake(&intel_dp->aux, > - DP_SUPPORTED_LINK_RATES, > - sink_rates, > - sizeof(sink_rates)); > + drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES, > + sink_rates, sizeof(sink_rates)); > > for (i = 0; i < ARRAY_SIZE(sink_rates); i++) { > int val = le16_to_cpu(sink_rates[i]); > @@ -3933,9 +3898,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > if (intel_dp->dpcd[DP_DPCD_REV] == 0x10) > return true; /* no per-port downstream info */ > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > - intel_dp->downstream_ports, > - DP_MAX_DOWNSTREAM_PORTS) < 0) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > + intel_dp->downstream_ports, > + DP_MAX_DOWNSTREAM_PORTS) < 0) > return false; /* downstream port status fetch failed */ > > return true; > @@ -3949,11 +3914,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) > if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT)) > return; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", > buf[0], buf[1], buf[2]); > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", > buf[0], buf[1], buf[2]); > } > @@ -3969,7 +3934,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp) > if (intel_dp->dpcd[DP_DPCD_REV] < 0x12) > return false; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > if (buf[0] & DP_MST_CAP) { > DRM_DEBUG_KMS("Sink is MST capable\n"); > intel_dp->is_mst = true; > @@ -4106,7 +4071,7 @@ stop: > static bool > intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) > { > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > + return drm_dp_dpcd_read(&intel_dp->aux, > DP_DEVICE_SERVICE_IRQ_VECTOR, > sink_irq_vector, 1) == 1; > } > @@ -4116,7 +4081,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) > { > int ret; > > - ret = intel_dp_dpcd_read_wake(&intel_dp->aux, > + ret = drm_dp_dpcd_read(&intel_dp->aux, > DP_SINK_COUNT_ESI, > sink_irq_vector, 14); > if (ret != 14) > @@ -4377,7 +4342,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { > uint8_t reg; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT, > ®, 1) < 0) > return connector_status_unknown; -- Jani Nikula, Intel Open Source Technology Center