On Fri, 16 Sep 2022, Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx> wrote:
> The struct has the action to be performed - squash, crawl
> or modeset and the corresponding cdclk which is the desired
> cdclk. This is the structure that gets populated during
> atomic check once it is determined what the cdclk change looks
> like
>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index c674879a84a5..3869f93e8ad2 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -11,13 +11,27 @@
> #include "intel_display.h"
> #include "intel_global_state.h"
>
> +#define MAX_CDCLK_ACTIONS 1
Okay, this review is just nitpicks, but they'll need to get fixed
eventually so here goes.
No tab after #define.
> +
> struct drm_i915_private;
> struct intel_atomic_state;
> struct intel_crtc_state;
>
> +enum cdclk_sequence {
Needs to be named intel_ something.
> + CDCLK_INVALID_ACTION = -1,
> +
> + CDCLK_SQUASH_ONLY = 0,
> + CDCLK_CRAWL_ONLY,
> + CDCLK_LEGACY,
> +};
> +
> struct intel_cdclk_config {
> unsigned int cdclk, vco, ref, bypass;
> u8 voltage_level;
> + struct cdclk_step {
Needs to be named intel_ something.
Since this is used independently, I'd prefer it to be defined outside of
struct intel_cdclk_config.
> + enum cdclk_sequence action;
> + u32 cdclk;
> + } steps[MAX_CDCLK_ACTIONS];
> };
>
> struct intel_cdclk_state {
--
Jani Nikula, Intel Open Source Graphics Center
