On Thu, Dec 09, 2021 at 06:51:23PM +0200, Jani Nikula wrote:
> Hide the details better.
>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 18 ++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_cdclk.h | 13 ++++++++-----
> 2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index a216a350006d..84674a4f7226 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2630,6 +2630,24 @@ intel_atomic_get_cdclk_state(struct intel_atomic_state *state)
> return to_intel_cdclk_state(cdclk_state);
> }
>
> +struct intel_cdclk_state *
> +to_intel_cdclk_state(struct intel_global_state *cdclk_state)
> +{
> + return container_of(cdclk_state, struct intel_cdclk_state, base);
> +}
> +
> +struct intel_cdclk_state *
> +intel_atomic_get_old_cdclk_state(struct intel_atomic_state *state)
> +{
> + return to_intel_cdclk_state(intel_atomic_get_old_global_obj_state(state, &to_i915(state->base.dev)->cdclk.obj));
> +}
> +
> +struct intel_cdclk_state *
> +intel_atomic_get_new_cdclk_state(struct intel_atomic_state *state)
> +{
> + return to_intel_cdclk_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->cdclk.obj));
> +}
> +
Not really sure about this one. We don't do this for any other similar
cases, and I think the macro versions are needed if we have any kind of
const vs. non-const funny business going on. I guess in this particular
case we don't, but pretty sure that was a real thing for some other
atomic states when I was pondering about using functions rather than
macros for those.
So I'm tempted to say we should stick to a common pattern across the
board if possible.
> int intel_cdclk_atomic_check(struct intel_atomic_state *state,
> bool *need_cdclk_calc)
> {
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index bb3a778c506b..77e8c8e1708f 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -76,11 +76,14 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state,
> struct intel_cdclk_state *
> intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
>
> -#define to_intel_cdclk_state(x) container_of((x), struct intel_cdclk_state, base)
> -#define intel_atomic_get_old_cdclk_state(state) \
> - to_intel_cdclk_state(intel_atomic_get_old_global_obj_state(state, &to_i915(state->base.dev)->cdclk.obj))
> -#define intel_atomic_get_new_cdclk_state(state) \
> - to_intel_cdclk_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->cdclk.obj))
> +struct intel_cdclk_state *
> +to_intel_cdclk_state(struct intel_global_state *cdclk_state);
> +
> +struct intel_cdclk_state *
> +intel_atomic_get_old_cdclk_state(struct intel_atomic_state *state);
> +
> +struct intel_cdclk_state *
> +intel_atomic_get_new_cdclk_state(struct intel_atomic_state *state);
>
> int intel_cdclk_init(struct drm_i915_private *dev_priv);
>
> --
> 2.30.2
--
Ville Syrjälä
Intel
