| Message ID | 10324781f9f7eae5a92506aaa7a40403efd345dd.1735662324.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/pmdemand: cleanups | expand |
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@intel.com> >Signed-off-by: Jani Nikula <jani.nikula@intel.com> This looks good to me, so Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> , 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 >
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);
In preparation for making struct intel_pmdemand_state an opaque type, convert to_intel_pmdemand_state() to a function. Cc: Gustavo Sousa <gustavo.sousa@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- 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. --- 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(-)