On Wed, 2015-01-21 at 17:43 +0530, Ramalingam C wrote:
> Hi
>
> On Friday 16 January 2015 04:41 AM, Rodrigo Vivi wrote:
>
> > On Fri, Jan 9, 2015 at 12:56 PM, Vandana Kannan
> > <vandana.kan...@intel.com> wrote:
> > > From: Durgadoss R <durgados...@intel.com>
> > >
> > > This patch enables eDP DRRS for CHV by adding the
> > > required IS_CHERRYVIEW() checks.
> > > CHV uses the same register bit as VLV.
> > >
> > > [Vandana]: Since CHV has 2 sets of M_N registers, it will follow the same
> > > code
> > > path as gen < 8. Added CHV check in dp_set_m_n()
> > >
> > > Signed-off-by: Durgadoss R <durgados...@intel.com>
> > > Signed-off-by: Vandana Kannan <vandana.kan...@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 25596ca..bb44fb9 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5810,8 +5810,8 @@ void intel_cpu_transcoder_set_m_n(struct intel_crtc
> > > *crtc,
> > > * for gen < 8) and if DRRS is supported (to make sure the
> > > * registers are not unnecessarily accessed).
> > > */
> > > - if (m2_n2 && INTEL_INFO(dev)->gen < 8 &&
> > > - crtc->config.has_drrs) {
> > > + if (m2_n2 && (IS_CHERRYVIEW(dev) || INTEL_INFO(dev)->gen
> > > < 8)
> > > + && crtc->config.has_drrs) {
> > This change here doesn't seem safe. As I told on previous comment I'd
> > prefer changing intel_dp_set_m_n instead of re-using this intel_cpu
> > one...
> Though I am rewriting the intel_dp_set_m_n() that will reuse
> intel_cpu_transcoder_set_m_n() within.
> But as a result, i am going to avoid the parallel usage of
> intel_cpu_transcoder_set_m_n() and intel_dp_set_m_n().
> So I am afraid this check for inclusion of cherryview for m2_n2
> programming will be part of the newer code also.
>
> Appending the RFC for the newer intel_dp_set_m_n() implementation
> below. Please review.
>
> RFC starts here:
>
> drivers/gpu/drm/i915/intel_display.c | 19
> ++++++++++++++++---
> drivers/gpu/drm/i915/intel_dp.c | 6
> ++----
> drivers/gpu/drm/i915/intel_drv.h | 8
> +++++++-
> 3 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 061a253..59cc87f
> 100644
> ---
> a/drivers/gpu/drm/i915/intel_display.c
> +++
> b/drivers/gpu/drm/i915/intel_display.c
> @@ -5829,13 +5829,26 @@ void intel_cpu_transcoder_set_m_n(struct
> intel_crtc *crtc,
> }
>
> }
>
>
>
> -void intel_dp_set_m_n(struct intel_crtc
> *crtc)
> +void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set set
> = M1_N1)
> {
>
> + struct intel_link_m_n *dp_m_n,
> *dp_m2_n2;
> +
>
> + if (set == M1_N1)
> {
> + dp_m_n =
> &crtc->config.dp_m_n;
> + dp_m2_n2 =
> &crtc->config.dp_m2_n2;
> + } else if (set == M2_N2)
> {
> + /* Only one register programming is supported
> */
> + dp_m_n =
> &crtc->config.dp_m_n;
> + dp_m2_n2 =
> NULL;
> + } else
> {
> + DRM_ERROR("Unsupported divider value
> \n");
> +
> return;
> + }
>
> +
>
> if
> (crtc->config.has_pch_encoder)
> intel_pch_transcoder_set_m_n(crtc,
> &crtc->config.dp_m_n);
>
> else
> - intel_cpu_transcoder_set_m_n(crtc,
> &crtc->config.dp_m_n,
> -
> &crtc->config.dp_m2_n2);
> + intel_cpu_transcoder_set_m_n(crtc, dp_m_n,
> dp_m2_n2);
> }
>
>
>
> static void vlv_update_pll(struct intel_crtc
> *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index b315292..784b8dd
> 100644
> ---
> a/drivers/gpu/drm/i915/intel_dp.c
> +++
> b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4817,11 +4817,10 @@ static void intel_dp_set_drrs_state(struct
> drm_device *dev, int refresh_rate)
> if (INTEL_INFO(dev)->gen >= 8 && !IS_CHERRYVIEW(dev))
> {
> switch(index)
> {
> case
> DRRS_HIGH_RR:
> -
> intel_dp_set_m_n(intel_crtc);
> + intel_dp_set_m_n(intel_crtc,
> M1_N1);
>
> break;
> case
> DRRS_LOW_RR:
> -
> intel_cpu_transcoder_set_m_n(intel_crtc,
> - &intel_crtc->config.dp_m2_n2,
> NULL);
> + intel_dp_set_m_n(intel_crtc,
> M2_N2);
>
> break;
> case
> DRRS_MAX_RR:
>
> default:
> @@ -4835,7 +4834,6 @@ static void intel_dp_set_drrs_state(struct
> drm_device *dev, int refresh_rate)
> val |=
> PIPECONF_EDP_RR_MODE_SWITCH_VLV;
>
> else
> val |=
> PIPECONF_EDP_RR_MODE_SWITCH;
> -
> intel_dp_set_m_n(intel_crtc);
> } else
> {
> if
> (IS_VALLEYVIEW(dev))
> val &=
> ~PIPECONF_EDP_RR_MODE_SWITCH_VLV;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 86d31f2..910e613
> 100644
> ---
> a/drivers/gpu/drm/i915/intel_drv.h
> +++
> b/drivers/gpu/drm/i915/intel_drv.h
> @@ -595,6 +595,12 @@ struct intel_hdmi
> {
> struct
> intel_dp_mst_encoder;
> #define DP_MAX_DOWNSTREAM_PORTS
> 0x10
>
>
> +enum link_m_n_set
> {
> + M1_N1 =
> 0,
> +
> M2_N2,
> +
> DIVIDER_MAX
do we need this divider_max here?
> +};
>
> +
>
> struct intel_dp
> {
> uint32_t
> output_reg;
> uint32_t
> aux_ch_ctl_reg;
> @@ -983,7 +989,7 @@ 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_set_m_n(struct intel_crtc
> *crtc);
> +void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set set
> = M1_N1);
> void intel_cpu_transcoder_set_m_n(struct intel_crtc
> *crtc,
> struct intel_link_m_n
> *m_n,
> struct intel_link_m_n
> *m2_n2);
> --
>
> 1.7.9.5
> >
> > > I915_WRITE(PIPE_DATA_M2(transcoder),
> > > TU_SIZE(m2_n2->tu) |
> > > m2_n2->gmch_m);
> > > I915_WRITE(PIPE_DATA_N2(transcoder),
> > > m2_n2->gmch_n);
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 3362d93..42195fe 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4802,7 +4802,7 @@ static void intel_dp_set_drrs_state(struct
> > > drm_device *dev, int refresh_rate)
> > > return;
> > > }
> > >
> > > - if (INTEL_INFO(dev)->gen >= 8) {
> > > + if (INTEL_INFO(dev)->gen >= 8 && !IS_CHERRYVIEW(dev)) {
> > > switch(index) {
> > > case DRRS_HIGH_RR:
> > > intel_dp_set_m_n(intel_crtc);
> > > --
> > > 2.0.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> --Ram
But overall it is better and much more clear than previous version.
Thanks
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
