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.barysh...@linaro.org>
> ---
>  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

Reply via email to