diff mbox series

[4/4] drm/i915/cdclk: Document CDCLK update methods

Message ID 20240207013334.29606-5-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/cdclk: More hardcoded cd2x divider nukage | expand

Commit Message

Ville Syrjälä Feb. 7, 2024, 1:33 a.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a bit of documentation to briefly explain the methods
by which we can change the CDCLK frequency.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Gustavo Sousa Feb. 16, 2024, 12:51 p.m. UTC | #1
Quoting Ville Syrjala (2024-02-06 22:33:34-03:00)
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>Add a bit of documentation to briefly explain the methods
>by which we can change the CDCLK frequency.
>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>index ca00586fdbc8..30dae4fef6cb 100644
>--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>@@ -63,6 +63,15 @@
>  * DMC will not change the active CDCLK frequency however, so that part
>  * will still be performed by the driver directly.
>  *
>+ * Several methods exist to change the CDCLK frequency, which ones are
>+ * supported depends on the platform:
>+ * - Full PLL disable + re-enable with new VCO frequency. Pipes must be inactive.
>+ * - CD2X divider update. Single pipe can be active as the divider update
>+ *   can be synchronized with the pipe's start of vblank.
>+ * - Crawl the PLL smoothly to the new VCO frequency. Pipes can be active.
>+ * - Squash waveform update. Pipes can be active.
>+ * - Crawl and squash can also be done back to back. Pipes can be active.
>+ *

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

I think it would also be nice to have some explanation of the components
involved in the generation of the CDCLK. I would prepend this paragraph
with something like:

    The are multiple components involved in the generation of the CDCLK
    frequency:

    - We have the CDCLK PLL, which generates an output clock
      based on a reference clock.

    - The CD2X Divider, which divides the output of the PLL based on a
      divisor selected from a set of pre-defined choices.

    - The CD2X Squasher, which further divides the output based on a
      waveform represented as a sequence of bits where each zero
      "squashes out" a clock cycle.

    - And finally a fixed divider that divides the output frequency by
      2.

    As such, the resulting CDCLK frequency can be calculated with the
    following formula:

        cdclk = vco / cd2x_div / (sq_len / sq_div) / 2

    , where vco is the frequency output from the PLL; cd2x_div
    represents the CD2X Divider; sq_len and sq_div are the bit length
    and the number of high bits for the CD2X Squasher waveform; and 2
    represents the fixed divider.

    Note that some older platforms do not contain the CD2X Divider
    and/or CD2X Squasher, in which case we can ignore their respective
    factors in the formula above.

In case you like it, we could either add it to this patch or I could send as
a separate patch. Your call.

--
Gustavo Sousa

>  * RAWCLK is a fixed frequency clock, often used by various auxiliary
>  * blocks such as AUX CH or backlight PWM. Hence the only thing we
>  * really need to know about RAWCLK is its frequency so that various
>-- 
>2.43.0
>
Ville Syrjälä Feb. 16, 2024, 4 p.m. UTC | #2
On Fri, Feb 16, 2024 at 09:51:36AM -0300, Gustavo Sousa wrote:
> Quoting Ville Syrjala (2024-02-06 22:33:34-03:00)
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >Add a bit of documentation to briefly explain the methods
> >by which we can change the CDCLK frequency.
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> > drivers/gpu/drm/i915/display/intel_cdclk.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >index ca00586fdbc8..30dae4fef6cb 100644
> >--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> >+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >@@ -63,6 +63,15 @@
> >  * DMC will not change the active CDCLK frequency however, so that part
> >  * will still be performed by the driver directly.
> >  *
> >+ * Several methods exist to change the CDCLK frequency, which ones are
> >+ * supported depends on the platform:
> >+ * - Full PLL disable + re-enable with new VCO frequency. Pipes must be inactive.
> >+ * - CD2X divider update. Single pipe can be active as the divider update
> >+ *   can be synchronized with the pipe's start of vblank.
> >+ * - Crawl the PLL smoothly to the new VCO frequency. Pipes can be active.
> >+ * - Squash waveform update. Pipes can be active.
> >+ * - Crawl and squash can also be done back to back. Pipes can be active.
> >+ *
> 
> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

Thanks. Pushed the series.

> 
> I think it would also be nice to have some explanation of the components
> involved in the generation of the CDCLK. I would prepend this paragraph
> with something like:
> 
>     The are multiple components involved in the generation of the CDCLK
>     frequency:
> 
>     - We have the CDCLK PLL, which generates an output clock
>       based on a reference clock.
> 
>     - The CD2X Divider, which divides the output of the PLL based on a
>       divisor selected from a set of pre-defined choices.
> 
>     - The CD2X Squasher, which further divides the output based on a
>       waveform represented as a sequence of bits where each zero
>       "squashes out" a clock cycle.
> 
>     - And finally a fixed divider that divides the output frequency by
>       2.
> 
>     As such, the resulting CDCLK frequency can be calculated with the
>     following formula:
> 
>         cdclk = vco / cd2x_div / (sq_len / sq_div) / 2
> 
>     , where vco is the frequency output from the PLL; cd2x_div
>     represents the CD2X Divider; sq_len and sq_div are the bit length
>     and the number of high bits for the CD2X Squasher waveform; and 2
>     represents the fixed divider.
> 
>     Note that some older platforms do not contain the CD2X Divider
>     and/or CD2X Squasher, in which case we can ignore their respective
>     factors in the formula above.
> 
> In case you like it, we could either add it to this patch or I could send as
> a separate patch. Your call.

Looks reasonable. Please send it as a separate patch.

> 
> --
> Gustavo Sousa
> 
> >  * RAWCLK is a fixed frequency clock, often used by various auxiliary
> >  * blocks such as AUX CH or backlight PWM. Hence the only thing we
> >  * really need to know about RAWCLK is its frequency so that various
> >-- 
> >2.43.0
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index ca00586fdbc8..30dae4fef6cb 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -63,6 +63,15 @@ 
  * DMC will not change the active CDCLK frequency however, so that part
  * will still be performed by the driver directly.
  *
+ * Several methods exist to change the CDCLK frequency, which ones are
+ * supported depends on the platform:
+ * - Full PLL disable + re-enable with new VCO frequency. Pipes must be inactive.
+ * - CD2X divider update. Single pipe can be active as the divider update
+ *   can be synchronized with the pipe's start of vblank.
+ * - Crawl the PLL smoothly to the new VCO frequency. Pipes can be active.
+ * - Squash waveform update. Pipes can be active.
+ * - Crawl and squash can also be done back to back. Pipes can be active.
+ *
  * RAWCLK is a fixed frequency clock, often used by various auxiliary
  * blocks such as AUX CH or backlight PWM. Hence the only thing we
  * really need to know about RAWCLK is its frequency so that various