Re: [PATCH v2 4/4] drm/msm/dp: Add support for LTTPR handling

[Johan Hovold <johan@kernel.org>]

On Wed, Dec 11, 2024 at 03:04:15PM +0200, Abel Vesa wrote:
 
> +static void msm_dp_display_lttpr_init(struct msm_dp_display_private *dp)
> +{
> +	int lttpr_count;
> +
> +	if (drm_dp_read_lttpr_common_caps(dp->aux, dp->panel->dpcd,
> +					  dp->lttpr_caps))
> +		return;
> +
> +	lttpr_count = drm_dp_lttpr_count(dp->lttpr_caps);

I was gonna say shouldn't you handle errors here, but that explains the
non-negative check I commented on the first patch in the series.

This looks error prone, but I think you should at least update the
kernel doc comment to drm_dp_lttpr_init() in the first patch so that
it's clear that you pass in the number of LTTPRs *or* an errno.

> +
> +	drm_dp_lttpr_init(dp->aux, lttpr_count);
> +}
> +
>  static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
>  {
>  	struct drm_connector *connector = dp->msm_dp_display.connector;
>  	const struct drm_display_info *info = &connector->display_info;
>  	int rc = 0;
>  
> +	msm_dp_display_lttpr_init(dp);

It looks like you ignore errors on purpose so I guess that's fine.

> +
>  	rc = msm_dp_panel_read_sink_caps(dp->panel, connector);
>  	if (rc)
>  		goto end;

Either way, this is needed for external display on my x1e80100 machines,
while not breaking the X13s:

Tested-by: Johan Hovold <johan+linaro@kernel.org>

Johan