[PATCH 2/2] drm/i915/lnl: Program PKGC_LATENCY register

[Jani Nikula]

On Mon, 05 Feb 2024, "Kandpal, Suraj" <suraj.kandpal at intel.com> wrote:
>> On Mon, 05 Feb 2024, Suraj Kandpal <suraj.kandpal at intel.com> wrote:
>> > +		if (wm_latency[i] == 0)
>> > +			break;
>> > +		else if (wm_latency[i] > max_value)
>> > +			max_value = wm_latency[i];
>> > +	}
>> > +
>> > +	if (max_value == 0)
>> > +		max_value = ~0 & LNL_PKG_C_LATENCY_MASK;
>> 
>> What does "~0 &" gain you here?
>> 
>
> So max value is 0 for all bits except 0-12 as we need to set them as all 1's to disable deep pkgc State

How is ~0 & LNL_PKG_C_LATENCY_MASK different from
LNL_PKG_C_LATENCY_MASK?

>> > +
>> > +	clear |= LNL_ADDED_WAKE_TIME_MASK |
>> LNL_PKG_C_LATENCY_MASK;
>> > +	val |= max_value;
>> 
>> If you have fields defined for the register, why not use it for setting max value
>> too?
>
> Sorry I didn't get you here .

val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, max_value);

>
>> 
>> > +	intel_uncore_rmw(&i915->uncore, LNL_PKG_C_LATENCY, clear, val); }
>> > +
>> >  static void skl_setup_wm_latency(struct drm_i915_private *i915)  {
>> >  	if (HAS_HW_SAGV_WM(i915))
>> > @@ -3407,6 +3435,9 @@ static void skl_setup_wm_latency(struct
>> drm_i915_private *i915)
>> >  		skl_read_wm_latency(i915, i915->display.wm.skl_latency);
>> >
>> >  	intel_print_wm_latency(i915, "Gen9 Plane",
>> > i915->display.wm.skl_latency);
>> > +
>> > +	if (DISPLAY_VER(i915) >= 20)
>> > +		intel_program_pkgc_latency(i915, i915-
>> >display.wm.skl_latency);
>> 
>> Before this, nothing in the skl_wm_init() path actually writes any registers, it's
>> all readout. Is this the right place to be doing this?
>> 
>
> Yes since all latency values are all ready and available for use which
> we can program in the deep pkgc register.

Is that a good reason to change a function that only reads hardware to
something writes the hardware?

BR,
Jani.

>
> Regards,
> Suraj Kandpal
>> >  }
>> >
>> >  static const struct intel_wm_funcs skl_wm_funcs = {
>> 
>> --
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel