From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: add fast boot support for Haswell Date: Mon, 05 Aug 2013 22:42:24 +0200 Message-ID: <52000E30.6000901@intel.com> References: <1375391542-21936-1-git-send-email-furquan@google.com> <20130805072421.GV22035@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1364735569==" Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Furquan Shaikh Cc: Josh Triplett , Paulo Zanoni , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Ronald Minnich List-Id: dri-devel@lists.freedesktop.org This is a multi-part message in MIME format. --===============1364735569== Content-Type: multipart/alternative; boundary="------------000806000306070506000501" This is a multi-part message in MIME format. --------------000806000306070506000501 MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; format="flowed" Content-Transfer-Encoding: quoted-printable On 05/08/2013 22:24, Furquan Shaikh wrote: > We tested the submitted patch on several systems here and it seems to = > be working fine. So, I'm not sure I understand your comment. Can you = > please provide more details? You check for the bit DP_PLL_FREQ_160MHZ in the register DP_A when = calculating the reference frequence for port A eDP panels (i.e. the = assignement of pipe_config->port_clock). This bit is valid on port A for = ivb, snb & ilk, but not on hsw. Haswell has a complete new way for = assigning the pll to the port. For solid fastboot support I think we = need to support them all (i.e. also the hdmi wrpll clocks and while at = it we might as well write the fdi dotclock readout code, it should be = almost the same as the DP dotclock computation). -Daniel > > Thanks, > Furquan > > > On Mon, Aug 5, 2013 at 12:24 AM, Daniel Vetter > wrote: > > On Thu, Aug 01, 2013 at 02:12:22PM -0700, Furquan Shaikh wrote: > > Enables getting correct mode clock when reading pipe config > > > > Signed-off-by: Furquan Shaikh > > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 8 ++++++++ > > drivers/gpu/drm/i915/intel_display.c | 9 ++++++++- > > 2 files changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > > index 931b4bb..fa0af9b 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1269,6 +1269,7 @@ static void intel_ddi_get_config(struct > intel_encoder *encoder, > > struct drm_i915_private *dev_priv =3D > encoder->base.dev->dev_private; > > struct intel_crtc *intel_crtc =3D > to_intel_crtc(encoder->base.crtc); > > enum transcoder cpu_transcoder =3D > intel_crtc->config.cpu_transcoder; > > + int port =3D intel_ddi_get_encoder_port(encoder); > > u32 temp, flags =3D 0; > > > > temp =3D I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder)); > > @@ -1282,6 +1283,13 @@ static void intel_ddi_get_config(struct > intel_encoder *encoder, > > flags |=3D DRM_MODE_FLAG_NVSYNC; > > > > pipe_config->adjusted_mode.flags |=3D flags; > > + > > + 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; > > + else > > + pipe_config->port_clock =3D 270000; > > + } > > I don't think this works correctly since for DP we have new clocks on > haswell, see intel_ddi_pll_mode_set. Also I think it'd be good to > go right > ahead and implement clock readout support for all hsw clock > sources, not > just DP. > -Daniel > > > } > > > > static void intel_ddi_destroy(struct drm_encoder *encoder) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > > index 3e66f05..681c99a 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -7176,6 +7176,8 @@ static void i9xx_crtc_clock_get(struct > intel_crtc *crtc, > > pipe_config->pixel_multiplier; > > } > > > > +#define div_ceil(A, B) ((A)/(B) + ((A)%(B) ? 1 : 0)) > > + > > static void ironlake_crtc_clock_get(struct intel_crtc *crtc, > > struct intel_crtc_config > *pipe_config) > > { > > @@ -7218,7 +7220,11 @@ static void > ironlake_crtc_clock_get(struct intel_crtc *crtc, > > return; > > > > clock =3D ((u64)link_m * (u64)link_freq * (u64)repeat); > > - do_div(clock, link_n); > > + /* This is required because the value comes out to be in > fraction > > + (eg. 70699.54). Need to round it up since values are > compared in > > + drm_mode_equal > > + */ > > + clock =3D div_ceil(clock, link_n); > > > > pipe_config->adjusted_mode.clock =3D clock; > > } > > @@ -9588,6 +9594,7 @@ static void intel_init_display(struct > drm_device *dev) > > > > if (HAS_DDI(dev)) { > > dev_priv->display.get_pipe_config =3D haswell_get_pipe_config; > > + dev_priv->display.get_clock =3D ironlake_crtc_clock_g= et; > > dev_priv->display.crtc_mode_set =3D > haswell_crtc_mode_set; > > dev_priv->display.crtc_enable =3D haswell_crtc_enable; > > dev_priv->display.crtc_disable =3D haswell_crtc_disab= le; > > -- > > 1.8.3 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - > http://blog.ffwll.ch > > Intel Semiconductor AG Registered No. 020.30.913.786-7 Registered Office: World Trade Center, Leutschenbachstrasse 95, 8050 Zurich= , Switzerland This e-mail and any attachments may contain confidential material for the s= ole use of the intended recipient(s). Any review or distribution by others = is strictly prohibited. If you are not the intended recipient, please conta= ct the sender and delete all copies. --------------000806000306070506000501 MIME-Version: 1.0 Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable
On 05/08/2013 22:24, Furquan Shaikh wrote:
We tested the submitted patch on several systems here and it seems to be working fine. So, I'm not sure I understand your comment. Can you please provide more details?

You check for the bit DP_PLL_FREQ_160MHZ in the register DP_A when calculating the reference frequence for port A eDP panels (i.e. the assignement of pipe_config->port_clock). This bit is valid on port A for ivb, snb & ilk, but not on hsw. Haswell has a complete new way for assigning the pll to the port. For solid fastboot support I think we need to support them all (i.e. also the hdmi wrpll clocks and while at it we might as well write the fdi dotclock readout code, it should be almost the same as the DP dotclock computation).
-Daniel

Thanks,<= /div>
Furquan<= /div>


On Mon, Aug 5, 2013 at 12:24 AM, Daniel Vetter <daniel@ff= wll.ch> wrote:
On Thu, Aug 01, 2013 at 02:12:22PM -0700, Furquan Shaikh wrote:
> Enables getting correct mode clock when reading pipe config
>
> Signed-off-by: Furquan Shaikh <furquan@google.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c    = | 8 ++++++++
>  drivers/gpu/drm/i915/intel_display.c | 9 ++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(= -)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 931b4bb..fa0af9b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1269,6 +1269,7 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
>       struct drm_i915_private *dev_pr= iv =3D encoder->base.dev->dev_private;
>       struct intel_crtc *intel_crtc = =3D to_intel_crtc(encoder->base.crtc);
>       enum transcoder cpu_transcoder = =3D intel_crtc->config.cpu_transcoder;
> +     int port =3D intel_ddi_get_encoder_port(encoder);
>       u32 temp, flags =3D 0;
>
>       temp =3D I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> @@ -1282,6 +1283,13 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
>               fla= gs |=3D DRM_MODE_FLAG_NVSYNC;
>
>       pipe_config->adjusted_mode.f= lags |=3D flags;
> +
> +     if (port =3D=3D PORT_A) {
> +             if ((I91= 5_READ(DP_A) & DP_PLL_FREQ_MASK) =3D=3D DP_PLL_FREQ_160MHZ)
> +               &= nbsp;     pipe_config->port_clock =3D 162000;
> +             else
> +               &= nbsp;     pipe_config->port_clock =3D 270000;
> +     }

I don't think this works correctly since for DP we have new clocks on
haswell, see intel_ddi_pll_mode_set. Also I think it'd be good to go right
ahead and implement clock readout support for all hsw clock sources, not
just DP.
-Daniel

>  }
>
>  static void intel_ddi_destroy(struct drm_encod= er *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3e66f05..681c99a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7176,6 +7176,8 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>               pip= e_config->pixel_multiplier;
>  }
>
> +#define div_ceil(A, B) ((A)/(B) + ((A)%(B) ? 1 : 0))
> +
>  static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>               &nb= sp;                   struct intel_crtc_config *pipe_config)
>  {
> @@ -7218,7 +7220,11 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>               ret= urn;
>
>       clock =3D ((u64)link_m * (u64)l= ink_freq * (u64)repeat);
> -     do_div(clock, link_n);
> +     /* This is required because the valu= e comes out to be in fraction
> +        (eg. 70699.54). Need to= round it up since values are compared in
> +        drm_mode_equal
> +     */
> +     clock =3D div_ceil(clock, link_n); >
>       pipe_config->adjusted_mode.c= lock =3D clock;
>  }
> @@ -9588,6 +9594,7 @@ static void intel_init_display(struct drm_device *dev)
>
>       if (HAS_DDI(dev)) {
>               dev_priv->display.get_pipe_config =3D haswell_get_pipe_config;
> +             dev_priv= ->display.get_clock =3D ironlake_crtc_clock_get;
>               dev= _priv->display.crtc_mode_set =3D haswell_crtc_mode_set;
>               dev= _priv->display.crtc_enable =3D haswell_crtc_enable;
>               dev= _priv->display.crtc_disable =3D haswell_crtc_disable;
> --
> 1.8.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@l= ists.freedesktop.org
> http://lists.freedesktop.org/mailman/list= info/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - = http://blog.ffwll.ch


Intel Semiconductor AG
Registered No. 020.30.913.786-7
Registered Office: World Trade Center, Leutschenbachstrasse 95, 8050 Zurich= , Switzerland

This e-mail and any attachments may contain confidential material for th= e sole use of the intended recipient(s). Any review or distribution by othe= rs is strictly prohibited. If you are not the intended recipient, please co= ntact the sender and delete all copies.

--------------000806000306070506000501-- --===============1364735569== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1364735569==--