On Tue, 15 Sep 2026, Animesh Manna <[email protected]> wrote:
> Abstract ALPM DPCD initialization into its own function.
>
> v2:
> - Improve commit description. [Suraj, Jouni]
>
> Cc: Jouni Högander <[email protected]>
> Signed-off-by: Animesh Manna <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_alpm.c | 11 +++++++++++
> drivers/gpu/drm/i915/display/intel_alpm.h | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 6 +-----
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 10943539bc7c..a6743fe62d48 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -43,6 +43,17 @@ bool intel_alpm_is_alpm_aux_less(struct intel_dp *intel_dp,
> (crtc_state->has_lobf &&
> intel_alpm_aux_less_wake_supported(intel_dp));
> }
>
> +bool intel_alpm_init_dpcd(struct intel_dp *intel_dp)
> +{
> + u8 dpcd;
> +
> + if (drm_dp_dpcd_read_byte(&intel_dp->aux, DP_RECEIVER_ALPM_CAP, &dpcd)
> < 0)
> + return false;
> +
> + intel_dp->alpm_dpcd = dpcd;
> + return true;
> +}
Why not propagate the original error instead of flattening to a boolean?
What does boolean true/false mean for a function that's not a predicate
function but "init"?
>From a maintenance perspective, the mixing of bool vs. int return values
is a problem because you never know what is being checked for in the
caller side:
if (!intel_alpm_init_dpcd())
Does that return bool and false means failure? Or does that return int
and false means success?!
BR,
Jani.
> +
> void intel_alpm_init(struct intel_dp *intel_dp)
> {
> mutex_init(&intel_dp->alpm.lock);
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h
> b/drivers/gpu/drm/i915/display/intel_alpm.h
> index f8f605d94f96..56c3e1482e37 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> @@ -15,6 +15,7 @@ struct intel_connector;
> struct intel_atomic_state;
> struct intel_crtc;
>
> +bool intel_alpm_init_dpcd(struct intel_dp *intel_dp);
> void intel_alpm_init(struct intel_dp *intel_dp);
> bool intel_alpm_compute_params(struct intel_dp *intel_dp,
> struct intel_crtc_state *crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0cd5e6b5034c..650c8b39270b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4783,7 +4783,6 @@ static bool
> intel_edp_init_dpcd(struct intel_dp *intel_dp, struct intel_connector
> *connector)
> {
> struct intel_display *display = to_intel_display(intel_dp);
> - int ret;
> u8 dprx;
>
> /* this function is meant to be called only once */
> @@ -4829,10 +4828,7 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp, struct
> intel_connector *connector
> */
> intel_dp_init_source_oui(intel_dp);
>
> - /* Read the ALPM DPCD caps */
> - ret = drm_dp_dpcd_read_byte(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> - &intel_dp->alpm_dpcd);
> - if (ret < 0)
> + if (!intel_alpm_init_dpcd(intel_dp))
> return false;
>
> /*
--
Jani Nikula, Intel
