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
