On Thu, 08 May 2025, Jouni Högander <jouni.hogan...@intel.com> wrote:
> We want to enable sink ALPM from PSR code. Make intel_alpm_enable_sink
> available for PSR.
>
> Signed-off-by: Jouni Högander <jouni.hogan...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_alpm.c | 11 +++++++++--
> drivers/gpu/drm/i915/display/intel_alpm.h | 2 ++
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 1bf08b80c23f..9442483058d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -426,8 +426,15 @@ void intel_alpm_pre_plane_update(struct
> intel_atomic_state *state,
> }
> }
>
> -static void intel_alpm_enable_sink(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *crtc_state)
> +/**
> + * intel_alpm_enable_sink - Enable ALPM on sink
> + * @intel_dp: Intel DP struct
> + * @crtc_state: Intel CRTC struct
> + *
> + * This function is enabling DPCD on sink based on information from
> crtc_state.
> + */
Perhaps surprisingly I'm not a big fan of kernel-doc for all the simple
little functions like this.
The function name already says what it does, the parameters are
self-explanatory.
The kernel-doc is not even pulled into the Sphinx build... and why would
it be, nobody reading the kernel documentation would be interested in
this small detail.
I might add a small regular comment about writing DPCD as needed... but
might not.
The documentation comments we absolutely need more are the high level
descriptions at the top of files, which is a glaring omission in
intel_alpm.c (and many other places). What is ALPM, what does it do,
how, why, etc.
See commit b031ef5ea8b1 ("drm/i915/mst: add beginnings of DP MST
documentation") or intel_dp_mst.c for the kind of comments I'd love to
have more.
I'm not insisting you do that now, but perhaps consider dropping the
kernel-doc.
BR,
Jani.
> +void intel_alpm_enable_sink(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state)
> {
> u8 val;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h
> b/drivers/gpu/drm/i915/display/intel_alpm.h
> index d7126d65b60f..c9fe21e3e72c 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> @@ -23,6 +23,8 @@ void intel_alpm_lobf_compute_config(struct intel_dp
> *intel_dp,
> struct drm_connector_state *conn_state);
> void intel_alpm_configure(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state);
> +void intel_alpm_enable_sink(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state);
> void intel_alpm_pre_plane_update(struct intel_atomic_state *state,
> struct intel_crtc *crtc);
> void intel_alpm_post_plane_update(struct intel_atomic_state *state,
--
Jani Nikula, Intel
