On Mon, 2025-05-12 at 13:33 +0300, Ville Syrjala wrote: > From: Ville Syrjälä <[email protected]> > > We have two copies of the code to generate the "disable this event" > value for the DMC_EVT_CTL registers. Extract to a helper. > > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_dmc.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c > b/drivers/gpu/drm/i915/display/intel_dmc.c > index 49cbb83b2bbe..ec940f837427 100644 > --- a/drivers/gpu/drm/i915/display/intel_dmc.c > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c > @@ -536,6 +536,14 @@ void intel_dmc_disable_pipe(struct intel_display > *display, enum pipe pipe) > } > } > > +static u32 dmc_evt_ctl_disable(void) > +{ > + return REG_FIELD_PREP(DMC_EVT_CTL_TYPE_MASK, > + DMC_EVT_CTL_TYPE_EDGE_0_1) | > + REG_FIELD_PREP(DMC_EVT_CTL_EVENT_ID_MASK, > + DMC_EVENT_FALSE); > +} > +
This could even be a macro. And the name seems a bit misleading to me, because it sounds like "disable" is an action, but the function only returns the values used to disable. > /** > * intel_dmc_block_pkgc() - block PKG C-state > * @display: display instance > @@ -575,10 +583,7 @@ void > intel_dmc_start_pkgc_exit_at_start_of_undelayed_vblank(struct intel_display > REG_FIELD_PREP(DMC_EVT_CTL_EVENT_ID_MASK, > PIPEDMC_EVENT_VBLANK); > else > - val = REG_FIELD_PREP(DMC_EVT_CTL_EVENT_ID_MASK, > - DMC_EVENT_FALSE) | > - REG_FIELD_PREP(DMC_EVT_CTL_TYPE_MASK, > - DMC_EVT_CTL_TYPE_EDGE_0_1); > + val = dmc_evt_ctl_disable(); > > intel_de_write(display, MTL_PIPEDMC_EVT_CTL_4(pipe), > val); > @@ -635,10 +640,7 @@ static u32 dmc_mmiodata(struct intel_display *display, > if (disable_dmc_evt(display, dmc_id, > dmc->dmc_info[dmc_id].mmioaddr[i], > dmc->dmc_info[dmc_id].mmiodata[i])) > - return REG_FIELD_PREP(DMC_EVT_CTL_TYPE_MASK, > - DMC_EVT_CTL_TYPE_EDGE_0_1) | > - REG_FIELD_PREP(DMC_EVT_CTL_EVENT_ID_MASK, > - DMC_EVENT_FALSE); > + return dmc_evt_ctl_disable(); > else > return dmc->dmc_info[dmc_id].mmiodata[i]; > } ...but that's a nitpick, so: Reviewed-by: Luca Coelho <[email protected]> -- Cheers, Luca.
