Quoting Jani Nikula (2024-12-31 13:27:37-03:00)
>In preparation for making struct intel_pmdemand_state an opaque type,
>convert to_intel_pmdemand_state() to a function.
>
>Cc: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
>Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
This looks good to me, so
Reviewed-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
, but I'm also taking this opportunity to reply to your comment below.
>
>---
>
>This is the simplest change. There could be other alternatives.
>
>Outside of intel_pmdemand.c, this is only used to convert
>display.pmdemand.obj.state to struct intel_pmdemand_state *. Maybe we
>could just pass the global object or state pointer instead? Or we could
>have a function to get the current state from, say, struct
>intel_display? What we currently have is a bit cumbersome.
I like the idea of the exposed interface receiving only pointers to the
generic types and that we make the necessary conversion internally.
We currently are only using to_intel_pmdemand_state() to be able to pass
the correct argument to other functions exposed by the pmdemand header.
Not sure there is much benefit in doing that except for some level of
compile-time type-safety?
So, I would generally say:
- For functions that can operate directly on the display.*.obj member
(e.g. hardware state readout), we just ask for the display struct
pointer as a parameter.
- For functions that potentially add the global state to the atomic
state, we also ask for the pointer to the atomic state.
- For functions that operate only on the state bits and that could be
called for a state instance that could either be the current one (or
old) or some new state during a commit, we ask for a pointer to the
intel_global_state struct.
--
Gustavo Sousa
>---
> drivers/gpu/drm/i915/display/intel_pmdemand.c | 5 +++++
> drivers/gpu/drm/i915/display/intel_pmdemand.h | 3 +--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
>index cdd314956a31..69b40b3735b3 100644
>--- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
>+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
>@@ -15,6 +15,11 @@
> #include "intel_pmdemand.h"
> #include "skl_watermark.h"
>
>+struct intel_pmdemand_state *to_intel_pmdemand_state(struct intel_global_state *obj_state)
>+{
>+ return container_of(obj_state, struct intel_pmdemand_state, base);
>+}
>+
> static struct intel_global_state *
> intel_pmdemand_duplicate_state(struct intel_global_obj *obj)
> {
>diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.h b/drivers/gpu/drm/i915/display/intel_pmdemand.h
>index a1c49efdc493..89296364ec3b 100644
>--- a/drivers/gpu/drm/i915/display/intel_pmdemand.h
>+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.h
>@@ -43,8 +43,7 @@ struct intel_pmdemand_state {
> struct pmdemand_params params;
> };
>
>-#define to_intel_pmdemand_state(global_state) \
>- container_of_const((global_state), struct intel_pmdemand_state, base)
>+struct intel_pmdemand_state *to_intel_pmdemand_state(struct intel_global_state *obj_state);
>
> void intel_pmdemand_init_early(struct drm_i915_private *i915);
> int intel_pmdemand_init(struct drm_i915_private *i915);
>--
>2.39.5
>
