Re: [PATCH v5 8/9] drm/vc4: hdmi: switch to using generic HDMI Codec infrastructure

[Maxime Ripard <mripard@xxxxxxxxxx>]

Hi,

Sorry, I've been drowning under work and couldn't review that series before.

I'll review the driver API for now, and we can focus on the exact
implementation later on.

On Sun, Dec 01, 2024 at 02:44:12AM +0200, Dmitry Baryshkov wrote:
> Drop driver-specific implementation and use the generic HDMI Codec
> framework in order to implement the HDMI audio support.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 68 ++++++++++--------------------------------
>  drivers/gpu/drm/vc4/vc4_hdmi.h |  2 --
>  2 files changed, 15 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 7295834e75fb1ab0cd241ed274e675567e66870b..d0a9aff7ad43016647493263c00d593296a1e3ad 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -595,6 +595,9 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
>  	if (vc4_hdmi->variant->supports_hdr)
>  		max_bpc = 12;
>  
> +	connector->hdmi_codec.max_i2s_channels = 8;
> +	connector->hdmi_codec.i2s = 1;
> +

I guess it's a similar discussion than we had with HDMI2.0+ earlier
today, but I don't really like initializing by structs. Struct fields
are easy to miss, and can be easily uninitialized by mistake.

I think I'd prefer to have them as argument to the init function. And if
they are optional, we can explicitly mark them as unused.

Like, it looks like the get_dai_id implementation relies on it being set
to < 0 for it to be ignored, but it's not here, so I'd assume it's used
with an ID of 0, even though the driver didn't support get_dai_id so
far?

Maxime

Attachment: signature.asc
Description: PGP signature