On Wed, 18 Jun 2025, Imre Deak <imre.d...@intel.com> wrote:
> On Thu, Jun 12, 2025 at 03:12:10PM +0300, Jani Nikula wrote:
>> Add intel_cdclk_actual() and intel_cdclk_actual_voltage_level() helpers
>> to avoid looking at struct intel_cdclk_state internals outside of
>> intel_cdclk.c.
>>
>> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_cdclk.c | 10 ++++++++++
>> drivers/gpu/drm/i915/display/intel_cdclk.h | 2 ++
>> drivers/gpu/drm/i915/display/intel_pmdemand.c | 4 ++--
>> 3 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index 994be1d0e20c..2e8abf237bd1 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> @@ -3884,3 +3884,13 @@ void intel_cdclk_read_hw(struct intel_display
>> *display)
>> cdclk_state->actual = display->cdclk.hw;
>> cdclk_state->logical = display->cdclk.hw;
>> }
>> +
>> +int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state)
>> +{
>> + return cdclk_state->actual.cdclk;
>> +}
>> +
>> +int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state
>> *cdclk_state)
>> +{
>> + return cdclk_state->actual.voltage_level;
>> +}
>
> These could've been grouped better after intel_cdclk_logical().
Yes, changing that.
> I wondered if it'd make sense to use
> intel_cdclk_{logical,actual}_cdclk() instead of
> intel_cdclk_{logical,actual}().
Mmh. I dislike the repetition, "cdclk logical cdclk"...
> Or *_clock() instead of *_cdclk() in the above and other helpers.
...so I set out to consistently use "clock", but then it didn't feel
right for things like "intel_cdclk_min_cdclk" because it's then compared
against min_cdclk in a number of places.
I don't know, leave it as it is now in the patches?
BR,
Jani.
>
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> index 0d5ee1826168..f38605c6ab72 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> @@ -103,5 +103,7 @@ int intel_cdclk_bw_min_cdclk(const struct
>> intel_cdclk_state *cdclk_state);
>> bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state);
>> void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int
>> force_min_cdclk);
>> void intel_cdclk_read_hw(struct intel_display *display);
>> +int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state);
>> +int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state
>> *cdclk_state);
>>
>> #endif /* __INTEL_CDCLK_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> b/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> index 16ef68ef4041..d806c15db7ce 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> @@ -360,9 +360,9 @@ int intel_pmdemand_atomic_check(struct
>> intel_atomic_state *state)
>> return PTR_ERR(new_cdclk_state);
>>
>> new_pmdemand_state->params.voltage_index =
>> - new_cdclk_state->actual.voltage_level;
>> + intel_cdclk_actual_voltage_level(new_cdclk_state);
>> new_pmdemand_state->params.cdclk_freq_mhz =
>> - DIV_ROUND_UP(new_cdclk_state->actual.cdclk, 1000);
>> + DIV_ROUND_UP(intel_cdclk_actual(new_cdclk_state), 1000);
>>
>> intel_pmdemand_update_max_ddiclk(display, state, new_pmdemand_state);
>>
>> --
>> 2.39.5
>>
--
Jani Nikula, Intel
