From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v2] drm/i915: State readout and cross-checking for dp_m2_n2 Date: Mon, 12 May 2014 13:27:25 +0300 Message-ID: <20140512102725.GD18465@intel.com> References: <20140422191526.GA10722@phenom.ffwll.local> <1399277971-2561-1-git-send-email-vandana.kannan@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 799D06E6E3 for ; Mon, 12 May 2014 03:27:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1399277971-2561-1-git-send-email-vandana.kannan@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Vandana Kannan Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, May 05, 2014 at 01:49:31PM +0530, Vandana Kannan wrote: > Adding relevant read out comparison code, in check_crtc_state, for the new > member of crtc_config, dp_m2_n2, which was introduced to store link_m_n > values for a DP downclock mode (if available). Suggested by Daniel. > = > v2: Changed patch title. > Daniel's review comments incorporated. > Added relevant state readout code for M2_N2. dp_m2_n2 comparison to be do= ne > only when high RR is not in use (This is because alternate m_n register > programming will be done only when low RR is being used). > = > Signed-off-by: Vandana Kannan > Cc: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_ddi.c | 1 + > drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++++++++++++++++++= +++--- > drivers/gpu/drm/i915/intel_dp.c | 2 + > drivers/gpu/drm/i915/intel_drv.h | 2 + > 4 files changed, 72 insertions(+), 5 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/inte= l_ddi.c > index 0ad4e96..6784f0b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1587,6 +1587,7 @@ void intel_ddi_get_config(struct intel_encoder *enc= oder, > case TRANS_DDI_MODE_SELECT_DP_MST: > pipe_config->has_dp_encoder =3D true; > intel_dp_get_m_n(intel_crtc, pipe_config); > + intel_dp_get_m2_n2(intel_crtc, pipe_config); > break; > default: > break; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index 797f01c..2e625eb 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6670,6 +6670,34 @@ void intel_dp_get_m_n(struct intel_crtc *crtc, > &pipe_config->dp_m_n); > } > = > +void intel_dp_get_m2_n2(struct intel_crtc *crtc, > + struct intel_crtc_config *pipe_config) > +{ > + struct drm_device *dev =3D crtc->base.dev; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + enum transcoder transcoder =3D pipe_config->cpu_transcoder; > + > + if (INTEL_INFO(dev)->gen >=3D 8) { > + intel_cpu_transcoder_get_m_n(crtc, transcoder, > + &pipe_config->dp_m_n); dm_m2_n2 surely? And why do we even want to do this? > + } else if (INTEL_INFO(dev)->gen > 6) { > + pipe_config->dp_m2_n2.link_m =3D > + I915_READ(PIPE_LINK_M2(transcoder)); > + pipe_config->dp_m2_n2.link_n =3D > + I915_READ(PIPE_LINK_N2(transcoder)); > + pipe_config->dp_m2_n2.gmch_m =3D > + I915_READ(PIPE_DATA_M2(transcoder)) > + & ~TU_SIZE_MASK; > + pipe_config->dp_m2_n2.gmch_n =3D > + I915_READ(PIPE_DATA_N2(transcoder)); > + pipe_config->dp_m2_n2.tu =3D > + ((I915_READ(PIPE_DATA_M2(transcoder)) > + & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1; > + } > + > +} > > + > static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc, > struct intel_crtc_config *pipe_config) > { > @@ -9169,6 +9197,15 @@ static void intel_dump_pipe_config(struct intel_cr= tc *crtc, > pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n, > pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n, > pipe_config->dp_m_n.tu); > + > + DRM_DEBUG_KMS("dp: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2: = %u, tu2: %u\n", > + pipe_config->has_dp_encoder, > + pipe_config->dp_m2_n2.gmch_m, > + pipe_config->dp_m2_n2.gmch_n, > + pipe_config->dp_m2_n2.link_m, > + pipe_config->dp_m2_n2.link_n, > + pipe_config->dp_m2_n2.tu); > + > DRM_DEBUG_KMS("requested mode:\n"); > drm_mode_debug_printmodeline(&pipe_config->requested_mode); > DRM_DEBUG_KMS("adjusted mode:\n"); > @@ -9533,6 +9570,14 @@ intel_pipe_config_compare(struct drm_device *dev, > struct intel_crtc_config *current_config, > struct intel_crtc_config *pipe_config) > { > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + struct intel_connector *intel_connector =3D dev_priv->drrs.connector; > + struct intel_encoder *encoder =3D (intel_connector !=3D NULL) ? > + intel_attached_encoder(&intel_connector->base) : > + NULL; > + struct intel_dp *intel_dp =3D (encoder !=3D NULL) ? > + enc_to_intel_dp(&encoder->base) : NULL; > + > #define PIPE_CONF_CHECK_X(name) \ > if (current_config->name !=3D pipe_config->name) { \ > DRM_ERROR("mismatch in " #name " " \ > @@ -9583,11 +9628,28 @@ intel_pipe_config_compare(struct drm_device *dev, > PIPE_CONF_CHECK_I(fdi_m_n.tu); > = > PIPE_CONF_CHECK_I(has_dp_encoder); > - PIPE_CONF_CHECK_I(dp_m_n.gmch_m); > - PIPE_CONF_CHECK_I(dp_m_n.gmch_n); > - PIPE_CONF_CHECK_I(dp_m_n.link_m); > - PIPE_CONF_CHECK_I(dp_m_n.link_n); > - PIPE_CONF_CHECK_I(dp_m_n.tu); > + > + > + /* DP M1_N1 registers are used when DRRS is disabled or when high RR > + * is used. Else, DP M2_N2 registers are used. The following check > + * has been added to make sure a mismatch (if any) is displayed only > + * for a real difference and not because of DRRS state. > + */ > + if ((dev_priv->vbt.drrs_type =3D=3D DRRS_NOT_SUPPORTED) || > + (dev_priv->vbt.drrs_type !=3D DRRS_NOT_SUPPORTED && intel_dp && > + intel_dp->drrs_state.refresh_rate_type =3D=3D DRRS_HIGH_RR)) { > + PIPE_CONF_CHECK_I(dp_m_n.gmch_m); > + PIPE_CONF_CHECK_I(dp_m_n.gmch_n); > + PIPE_CONF_CHECK_I(dp_m_n.link_m); > + PIPE_CONF_CHECK_I(dp_m_n.link_n); > + PIPE_CONF_CHECK_I(dp_m_n.tu); > + } else { > + PIPE_CONF_CHECK_I(dp_m2_n2.gmch_m); > + PIPE_CONF_CHECK_I(dp_m2_n2.gmch_n); > + PIPE_CONF_CHECK_I(dp_m2_n2.link_m); > + PIPE_CONF_CHECK_I(dp_m2_n2.link_n); > + PIPE_CONF_CHECK_I(dp_m2_n2.tu); > + } Can't we just check both dp_m_n and dp_m2_n2 always? > = > PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay); > PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal); > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel= _dp.c > index 34ed143..9aa4dcd 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1511,6 +1511,8 @@ static void intel_dp_get_config(struct intel_encode= r *encoder, > = > intel_dp_get_m_n(crtc, pipe_config); > = > + intel_dp_get_m2_n2(crtc, pipe_config); > + > if (port =3D=3D PORT_A) { > if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) =3D=3D DP_PLL_FREQ_160MHZ) > pipe_config->port_clock =3D 162000; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/inte= l_drv.h > index d8b540b..1013f70 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -763,6 +763,8 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv= ); > void hsw_disable_pc8(struct drm_i915_private *dev_priv); > void intel_dp_get_m_n(struct intel_crtc *crtc, > struct intel_crtc_config *pipe_config); > +void intel_dp_get_m2_n2(struct intel_crtc *crtc, > + struct intel_crtc_config *pipe_config); > int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n = *m_n); > void > ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_con= fig, > -- = > 1.9.2 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC