Re: [Intel-gfx] [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
On Mon, 2019-06-24 at 15:39 +0300, Ville Syrjälä wrote:
> On Thu, Jun 20, 2019 at 11:33:27PM +, Souza, Jose wrote:
> > On Wed, 2019-06-19 at 20:47 +0300, Ville Syrjälä wrote:
> > > On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> > > wrote:
> > > > Now 180, 172.8 and 192 MHz are supported.
> > > >
> > > > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > > > enabled as state by BSpec and implemented in
> > > > intel_crtc_compute_min_cdclk(), CD clock must be at least twice
> > > > of
> > > > Azalia BCLK and BCLK by default is 96 MHz, it could be set to
> > > > 48
> > > > MHz
> > > > but we are not reading it.
> > >
> > > Do we know whether this thing still suffers from the glk/cnl
> > > issue
> > > where
> > > we can't even talk to the codec if cdclk is too low? If it does
> > > we
> > > need
> > > to adjust the platform check for the "force min cdclk for audio"
> > > thing.
> > > Also that is going to suck on icl due to the cd2x divider always
> > > being
> > > 1 and hence we can't do the optimized cdclk change around audio
> > > power
> > > requests.
> >
> > Not sure how to test if we can talk to the codec when cdclock is
> > too
> > low(you mean lower than 192000khz(96MHz*2)?) but this workflow
> > worked:
> >
> > 1- only full HD eDP panel connected | cdclock = 172800khz
> > 2- connected a full HD DP monitor with audio | cdclock = 192000khz
> > * audio works
> > 3- disconnected full HD DP monitor | cdclock = 172800khz
> > 4- connected again a full HD DP monitor with audio | cdclock =
> > 192000khz
> > * audio works
> >
> > Do you have any other test in mind?
>
> Try to load/resume the audio driver when cdclk is low maybe?
Oh good idea.
It worked, snd_hda_intel loaded without errors while cdclk was 172800
kHz and then connecting a monitor with audio the cdclk was set to
192000 kHz and audio was send to speakers.
>
> > > > BSpec: 20598
> > > > BSpec: 15729
> > > > Cc: Clint Taylor
> > > > Signed-off-by: José Roberto de Souza
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_cdclk.c | 29
> > > > +++---
> > > >
> > > > 1 file changed, 20 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > index 8993ab283562..d560e25d3fb5 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > > > drm_i915_private *dev_priv)
> > > >
> > > > static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > > > {
> > > > - int ranges_24[] = { 312000, 552000, 648000 };
> > > > - int ranges_19_38[] = { 307200, 556800, 652800 };
> > > > - int *ranges;
> > > > + const int ranges_24[] = { 18, 192000, 312000,
> > > > 552000,
> > > > 648000 };
> > > > + const int ranges_19_38[] = { 172800, 192000, 307200,
> > > > 556800,
> > > > 652800 };
> > > > + const int *ranges;
> > > > + unsigned int len, i;
> > > >
> > > > switch (ref) {
> > > > default:
> > > > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int
> > > > min_cdclk,
> > > > unsigned int ref)
> > > > /* fall through */
> > > > case 24000:
> > > > ranges = ranges_24;
> > > > + len = ARRAY_SIZE(ranges_24);
> > > > break;
> > > > case 19200:
> > > > case 38400:
> > > > ranges = ranges_19_38;
> > > > + len = ARRAY_SIZE(ranges_19_38);
> > > > break;
> > > > }
> > > >
> > > > - if (min_cdclk > ranges[1])
> > > > - return ranges[2];
> > > > - else if (min_cdclk > ranges[0])
> > > > - return ranges[1];
> > > > - else
> > > > - return ranges[0];
> > > > + for (i = 0; i < len; i++) {
> > > > + if (min_cdclk <= ranges[i])
> > > > + return ranges[i];
> > > > + }
> > > > +
> > > > + WARN_ON(min_cdclk > ranges[len - 1]);
> > > > + return ranges[len - 1];
> > > > }
> > > >
> > > > static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > > > *dev_priv, int cdclk)
> > > > @@ -1792,16 +1796,23 @@ static int
> > > > icl_calc_cdclk_pll_vco(struct
> > > > drm_i915_private *dev_priv, int cdclk)
> > > > default:
> > > > MISSING_CASE(cdclk);
> > > > /* fall through */
> > > > + case 172800:
> > > > case 307200:
> > > > case 556800:
> > > > case 652800:
> > > > WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > > dev_priv->cdclk.hw.ref != 38400);
> > > > break;
> > > > + case 18:
> > > > case 312000:
> > > > case 552000:
> > > > case 648000:
> > > >
Re: [Intel-gfx] [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
On Thu, Jun 20, 2019 at 11:33:27PM +, Souza, Jose wrote:
> On Wed, 2019-06-19 at 20:47 +0300, Ville Syrjälä wrote:
> > On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> > wrote:
> > > Now 180, 172.8 and 192 MHz are supported.
> > >
> > > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > > enabled as state by BSpec and implemented in
> > > intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> > > Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48
> > > MHz
> > > but we are not reading it.
> >
> > Do we know whether this thing still suffers from the glk/cnl issue
> > where
> > we can't even talk to the codec if cdclk is too low? If it does we
> > need
> > to adjust the platform check for the "force min cdclk for audio"
> > thing.
> > Also that is going to suck on icl due to the cd2x divider always
> > being
> > 1 and hence we can't do the optimized cdclk change around audio power
> > requests.
>
> Not sure how to test if we can talk to the codec when cdclock is too
> low(you mean lower than 192000khz(96MHz*2)?) but this workflow worked:
>
> 1- only full HD eDP panel connected | cdclock = 172800khz
> 2- connected a full HD DP monitor with audio | cdclock = 192000khz
> * audio works
> 3- disconnected full HD DP monitor | cdclock = 172800khz
> 4- connected again a full HD DP monitor with audio | cdclock =
> 192000khz
> * audio works
>
> Do you have any other test in mind?
Try to load/resume the audio driver when cdclk is low maybe?
>
> >
> > > BSpec: 20598
> > > BSpec: 15729
> > > Cc: Clint Taylor
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++---
> > >
> > > 1 file changed, 20 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > index 8993ab283562..d560e25d3fb5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > > drm_i915_private *dev_priv)
> > >
> > > static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > > {
> > > - int ranges_24[] = { 312000, 552000, 648000 };
> > > - int ranges_19_38[] = { 307200, 556800, 652800 };
> > > - int *ranges;
> > > + const int ranges_24[] = { 18, 192000, 312000, 552000,
> > > 648000 };
> > > + const int ranges_19_38[] = { 172800, 192000, 307200, 556800,
> > > 652800 };
> > > + const int *ranges;
> > > + unsigned int len, i;
> > >
> > > switch (ref) {
> > > default:
> > > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk,
> > > unsigned int ref)
> > > /* fall through */
> > > case 24000:
> > > ranges = ranges_24;
> > > + len = ARRAY_SIZE(ranges_24);
> > > break;
> > > case 19200:
> > > case 38400:
> > > ranges = ranges_19_38;
> > > + len = ARRAY_SIZE(ranges_19_38);
> > > break;
> > > }
> > >
> > > - if (min_cdclk > ranges[1])
> > > - return ranges[2];
> > > - else if (min_cdclk > ranges[0])
> > > - return ranges[1];
> > > - else
> > > - return ranges[0];
> > > + for (i = 0; i < len; i++) {
> > > + if (min_cdclk <= ranges[i])
> > > + return ranges[i];
> > > + }
> > > +
> > > + WARN_ON(min_cdclk > ranges[len - 1]);
> > > + return ranges[len - 1];
> > > }
> > >
> > > static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > > *dev_priv, int cdclk)
> > > @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> > > drm_i915_private *dev_priv, int cdclk)
> > > default:
> > > MISSING_CASE(cdclk);
> > > /* fall through */
> > > + case 172800:
> > > case 307200:
> > > case 556800:
> > > case 652800:
> > > WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > dev_priv->cdclk.hw.ref != 38400);
> > > break;
> > > + case 18:
> > > case 312000:
> > > case 552000:
> > > case 648000:
> > > WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > > + break;
> > > + case 192000:
> > > + WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > + dev_priv->cdclk.hw.ref != 38400 &&
> > > + dev_priv->cdclk.hw.ref != 24000);
> > > }
> > >
> > > ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > > --
> > > 2.22.0
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
On Wed, 2019-06-19 at 20:47 +0300, Ville Syrjälä wrote:
> On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> wrote:
> > Now 180, 172.8 and 192 MHz are supported.
> >
> > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > enabled as state by BSpec and implemented in
> > intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> > Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48
> > MHz
> > but we are not reading it.
>
> Do we know whether this thing still suffers from the glk/cnl issue
> where
> we can't even talk to the codec if cdclk is too low? If it does we
> need
> to adjust the platform check for the "force min cdclk for audio"
> thing.
> Also that is going to suck on icl due to the cd2x divider always
> being
> 1 and hence we can't do the optimized cdclk change around audio power
> requests.
Not sure how to test if we can talk to the codec when cdclock is too
low(you mean lower than 192000khz(96MHz*2)?) but this workflow worked:
1- only full HD eDP panel connected | cdclock = 172800khz
2- connected a full HD DP monitor with audio | cdclock = 192000khz
* audio works
3- disconnected full HD DP monitor | cdclock = 172800khz
4- connected again a full HD DP monitor with audio | cdclock =
192000khz
* audio works
Do you have any other test in mind?
>
> > BSpec: 20598
> > BSpec: 15729
> > Cc: Clint Taylor
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++---
> >
> > 1 file changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 8993ab283562..d560e25d3fb5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > drm_i915_private *dev_priv)
> >
> > static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > {
> > - int ranges_24[] = { 312000, 552000, 648000 };
> > - int ranges_19_38[] = { 307200, 556800, 652800 };
> > - int *ranges;
> > + const int ranges_24[] = { 18, 192000, 312000, 552000,
> > 648000 };
> > + const int ranges_19_38[] = { 172800, 192000, 307200, 556800,
> > 652800 };
> > + const int *ranges;
> > + unsigned int len, i;
> >
> > switch (ref) {
> > default:
> > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk,
> > unsigned int ref)
> > /* fall through */
> > case 24000:
> > ranges = ranges_24;
> > + len = ARRAY_SIZE(ranges_24);
> > break;
> > case 19200:
> > case 38400:
> > ranges = ranges_19_38;
> > + len = ARRAY_SIZE(ranges_19_38);
> > break;
> > }
> >
> > - if (min_cdclk > ranges[1])
> > - return ranges[2];
> > - else if (min_cdclk > ranges[0])
> > - return ranges[1];
> > - else
> > - return ranges[0];
> > + for (i = 0; i < len; i++) {
> > + if (min_cdclk <= ranges[i])
> > + return ranges[i];
> > + }
> > +
> > + WARN_ON(min_cdclk > ranges[len - 1]);
> > + return ranges[len - 1];
> > }
> >
> > static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > *dev_priv, int cdclk)
> > @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> > drm_i915_private *dev_priv, int cdclk)
> > default:
> > MISSING_CASE(cdclk);
> > /* fall through */
> > + case 172800:
> > case 307200:
> > case 556800:
> > case 652800:
> > WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > dev_priv->cdclk.hw.ref != 38400);
> > break;
> > + case 18:
> > case 312000:
> > case 552000:
> > case 648000:
> > WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > + break;
> > + case 192000:
> > + WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > + dev_priv->cdclk.hw.ref != 38400 &&
> > + dev_priv->cdclk.hw.ref != 24000);
> > }
> >
> > ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > --
> > 2.22.0
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
On Thu, Jun 20, 2019 at 12:36:57AM +, Souza, Jose wrote:
> On Wed, 2019-06-19 at 14:48 +0300, Ville Syrjälä wrote:
> > On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> > wrote:
> > > Now 180, 172.8 and 192 MHz are supported.
> > >
> > > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > > enabled as state by BSpec and implemented in
> > > intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> > > Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48
> > > MHz
> > > but we are not reading it.
> > >
> > > BSpec: 20598
> > > BSpec: 15729
> > > Cc: Clint Taylor
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++---
> > >
> > > 1 file changed, 20 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > index 8993ab283562..d560e25d3fb5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > > drm_i915_private *dev_priv)
> > >
> > > static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > > {
> > > - int ranges_24[] = { 312000, 552000, 648000 };
> > > - int ranges_19_38[] = { 307200, 556800, 652800 };
> > > - int *ranges;
> > > + const int ranges_24[] = { 18, 192000, 312000, 552000,
> > > 648000 };
> > > + const int ranges_19_38[] = { 172800, 192000, 307200, 556800,
> > > 652800 };
> >
> > static?
>
> We don't need to keep this in memory all the time.
It'll be in memory either way. It's going to end up
as either text or bss. bss seems better.
>
> >
> > > + const int *ranges;
> > > + unsigned int len, i;
> >
> > just 'int' please
>
> Ok
>
> >
> > >
> > > switch (ref) {
> > > default:
> > > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk,
> > > unsigned int ref)
> > > /* fall through */
> > > case 24000:
> > > ranges = ranges_24;
> > > + len = ARRAY_SIZE(ranges_24);
> > > break;
> > > case 19200:
> > > case 38400:
> > > ranges = ranges_19_38;
> > > + len = ARRAY_SIZE(ranges_19_38);
> > > break;
> > > }
> > >
> > > - if (min_cdclk > ranges[1])
> > > - return ranges[2];
> > > - else if (min_cdclk > ranges[0])
> > > - return ranges[1];
> > > - else
> > > - return ranges[0];
> > > + for (i = 0; i < len; i++) {
> > > + if (min_cdclk <= ranges[i])
> > > + return ranges[i];
> > > + }
> > > +
> > > + WARN_ON(min_cdclk > ranges[len - 1]);
> > > + return ranges[len - 1];
> > > }
> > >
> > > static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > > *dev_priv, int cdclk)
> > > @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> > > drm_i915_private *dev_priv, int cdclk)
> > > default:
> > > MISSING_CASE(cdclk);
> > > /* fall through */
> > > + case 172800:
> > > case 307200:
> > > case 556800:
> > > case 652800:
> > > WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > dev_priv->cdclk.hw.ref != 38400);
> > > break;
> > > + case 18:
> > > case 312000:
> > > case 552000:
> > > case 648000:
> > > WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > > + break;
> > > + case 192000:
> > > + WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > + dev_priv->cdclk.hw.ref != 38400 &&
> > > + dev_priv->cdclk.hw.ref != 24000);
> >
> > I'd probably put a break here too for symmetry.
>
> Ok
>
> >
> > > }
> > >
> > > ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > > --
> > > 2.22.0
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
On Wed, 2019-06-19 at 14:48 +0300, Ville Syrjälä wrote:
> On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> wrote:
> > Now 180, 172.8 and 192 MHz are supported.
> >
> > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > enabled as state by BSpec and implemented in
> > intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> > Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48
> > MHz
> > but we are not reading it.
> >
> > BSpec: 20598
> > BSpec: 15729
> > Cc: Clint Taylor
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++---
> >
> > 1 file changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 8993ab283562..d560e25d3fb5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > drm_i915_private *dev_priv)
> >
> > static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > {
> > - int ranges_24[] = { 312000, 552000, 648000 };
> > - int ranges_19_38[] = { 307200, 556800, 652800 };
> > - int *ranges;
> > + const int ranges_24[] = { 18, 192000, 312000, 552000,
> > 648000 };
> > + const int ranges_19_38[] = { 172800, 192000, 307200, 556800,
> > 652800 };
>
> static?
We don't need to keep this in memory all the time.
>
> > + const int *ranges;
> > + unsigned int len, i;
>
> just 'int' please
Ok
>
> >
> > switch (ref) {
> > default:
> > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk,
> > unsigned int ref)
> > /* fall through */
> > case 24000:
> > ranges = ranges_24;
> > + len = ARRAY_SIZE(ranges_24);
> > break;
> > case 19200:
> > case 38400:
> > ranges = ranges_19_38;
> > + len = ARRAY_SIZE(ranges_19_38);
> > break;
> > }
> >
> > - if (min_cdclk > ranges[1])
> > - return ranges[2];
> > - else if (min_cdclk > ranges[0])
> > - return ranges[1];
> > - else
> > - return ranges[0];
> > + for (i = 0; i < len; i++) {
> > + if (min_cdclk <= ranges[i])
> > + return ranges[i];
> > + }
> > +
> > + WARN_ON(min_cdclk > ranges[len - 1]);
> > + return ranges[len - 1];
> > }
> >
> > static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > *dev_priv, int cdclk)
> > @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> > drm_i915_private *dev_priv, int cdclk)
> > default:
> > MISSING_CASE(cdclk);
> > /* fall through */
> > + case 172800:
> > case 307200:
> > case 556800:
> > case 652800:
> > WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > dev_priv->cdclk.hw.ref != 38400);
> > break;
> > + case 18:
> > case 312000:
> > case 552000:
> > case 648000:
> > WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > + break;
> > + case 192000:
> > + WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > + dev_priv->cdclk.hw.ref != 38400 &&
> > + dev_priv->cdclk.hw.ref != 24000);
>
> I'd probably put a break here too for symmetry.
Ok
>
> > }
> >
> > ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > --
> > 2.22.0
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza wrote:
> Now 180, 172.8 and 192 MHz are supported.
>
> 180 and 172.8 MHz CD clocks will only be used when audio is not
> enabled as state by BSpec and implemented in
> intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48 MHz
> but we are not reading it.
Do we know whether this thing still suffers from the glk/cnl issue where
we can't even talk to the codec if cdclk is too low? If it does we need
to adjust the platform check for the "force min cdclk for audio" thing.
Also that is going to suck on icl due to the cd2x divider always being
1 and hence we can't do the optimized cdclk change around audio power
requests.
>
> BSpec: 20598
> BSpec: 15729
> Cc: Clint Taylor
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++---
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 8993ab283562..d560e25d3fb5 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct drm_i915_private
> *dev_priv)
>
> static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> {
> - int ranges_24[] = { 312000, 552000, 648000 };
> - int ranges_19_38[] = { 307200, 556800, 652800 };
> - int *ranges;
> + const int ranges_24[] = { 18, 192000, 312000, 552000, 648000 };
> + const int ranges_19_38[] = { 172800, 192000, 307200, 556800, 652800 };
> + const int *ranges;
> + unsigned int len, i;
>
> switch (ref) {
> default:
> @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int
> ref)
> /* fall through */
> case 24000:
> ranges = ranges_24;
> + len = ARRAY_SIZE(ranges_24);
> break;
> case 19200:
> case 38400:
> ranges = ranges_19_38;
> + len = ARRAY_SIZE(ranges_19_38);
> break;
> }
>
> - if (min_cdclk > ranges[1])
> - return ranges[2];
> - else if (min_cdclk > ranges[0])
> - return ranges[1];
> - else
> - return ranges[0];
> + for (i = 0; i < len; i++) {
> + if (min_cdclk <= ranges[i])
> + return ranges[i];
> + }
> +
> + WARN_ON(min_cdclk > ranges[len - 1]);
> + return ranges[len - 1];
> }
>
> static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int
> cdclk)
> @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> drm_i915_private *dev_priv, int cdclk)
> default:
> MISSING_CASE(cdclk);
> /* fall through */
> + case 172800:
> case 307200:
> case 556800:
> case 652800:
> WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> dev_priv->cdclk.hw.ref != 38400);
> break;
> + case 18:
> case 312000:
> case 552000:
> case 648000:
> WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> + break;
> + case 192000:
> + WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> + dev_priv->cdclk.hw.ref != 38400 &&
> + dev_priv->cdclk.hw.ref != 24000);
> }
>
> ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> --
> 2.22.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza wrote:
> Now 180, 172.8 and 192 MHz are supported.
>
> 180 and 172.8 MHz CD clocks will only be used when audio is not
> enabled as state by BSpec and implemented in
> intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48 MHz
> but we are not reading it.
>
> BSpec: 20598
> BSpec: 15729
> Cc: Clint Taylor
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++---
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 8993ab283562..d560e25d3fb5 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct drm_i915_private
> *dev_priv)
>
> static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> {
> - int ranges_24[] = { 312000, 552000, 648000 };
> - int ranges_19_38[] = { 307200, 556800, 652800 };
> - int *ranges;
> + const int ranges_24[] = { 18, 192000, 312000, 552000, 648000 };
> + const int ranges_19_38[] = { 172800, 192000, 307200, 556800, 652800 };
static?
> + const int *ranges;
> + unsigned int len, i;
just 'int' please
>
> switch (ref) {
> default:
> @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int
> ref)
> /* fall through */
> case 24000:
> ranges = ranges_24;
> + len = ARRAY_SIZE(ranges_24);
> break;
> case 19200:
> case 38400:
> ranges = ranges_19_38;
> + len = ARRAY_SIZE(ranges_19_38);
> break;
> }
>
> - if (min_cdclk > ranges[1])
> - return ranges[2];
> - else if (min_cdclk > ranges[0])
> - return ranges[1];
> - else
> - return ranges[0];
> + for (i = 0; i < len; i++) {
> + if (min_cdclk <= ranges[i])
> + return ranges[i];
> + }
> +
> + WARN_ON(min_cdclk > ranges[len - 1]);
> + return ranges[len - 1];
> }
>
> static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int
> cdclk)
> @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> drm_i915_private *dev_priv, int cdclk)
> default:
> MISSING_CASE(cdclk);
> /* fall through */
> + case 172800:
> case 307200:
> case 556800:
> case 652800:
> WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> dev_priv->cdclk.hw.ref != 38400);
> break;
> + case 18:
> case 312000:
> case 552000:
> case 648000:
> WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> + break;
> + case 192000:
> + WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> + dev_priv->cdclk.hw.ref != 38400 &&
> + dev_priv->cdclk.hw.ref != 24000);
I'd probably put a break here too for symmetry.
> }
>
> ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> --
> 2.22.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
Now 180, 172.8 and 192 MHz are supported.
180 and 172.8 MHz CD clocks will only be used when audio is not
enabled as state by BSpec and implemented in
intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48 MHz
but we are not reading it.
BSpec: 20598
BSpec: 15729
Cc: Clint Taylor
Signed-off-by: José Roberto de Souza
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++---
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 8993ab283562..d560e25d3fb5 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct drm_i915_private
*dev_priv)
static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
{
- int ranges_24[] = { 312000, 552000, 648000 };
- int ranges_19_38[] = { 307200, 556800, 652800 };
- int *ranges;
+ const int ranges_24[] = { 18, 192000, 312000, 552000, 648000 };
+ const int ranges_19_38[] = { 172800, 192000, 307200, 556800, 652800 };
+ const int *ranges;
+ unsigned int len, i;
switch (ref) {
default:
@@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int
ref)
/* fall through */
case 24000:
ranges = ranges_24;
+ len = ARRAY_SIZE(ranges_24);
break;
case 19200:
case 38400:
ranges = ranges_19_38;
+ len = ARRAY_SIZE(ranges_19_38);
break;
}
- if (min_cdclk > ranges[1])
- return ranges[2];
- else if (min_cdclk > ranges[0])
- return ranges[1];
- else
- return ranges[0];
+ for (i = 0; i < len; i++) {
+ if (min_cdclk <= ranges[i])
+ return ranges[i];
+ }
+
+ WARN_ON(min_cdclk > ranges[len - 1]);
+ return ranges[len - 1];
}
static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
@@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
drm_i915_private *dev_priv, int cdclk)
default:
MISSING_CASE(cdclk);
/* fall through */
+ case 172800:
case 307200:
case 556800:
case 652800:
WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
dev_priv->cdclk.hw.ref != 38400);
break;
+ case 18:
case 312000:
case 552000:
case 648000:
WARN_ON(dev_priv->cdclk.hw.ref != 24000);
+ break;
+ case 192000:
+ WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
+ dev_priv->cdclk.hw.ref != 38400 &&
+ dev_priv->cdclk.hw.ref != 24000);
}
ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
--
2.22.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
