| Message ID | 20240530-bridge-hdmi-connector-v3-1-a1d184d68fe3@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm: make use of the HDMI connector infrastructure | expand |
Hi, On Thu, May 30, 2024 at 02:12:24AM GMT, Dmitry Baryshkov wrote: > Allow passing NULL as audio infoframe as a way to disable Audio > Infoframe generation. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > index ce96837eea65..5356723d21f5 100644 > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > @@ -681,7 +681,7 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_update_infoframes); > /** > * drm_atomic_helper_connector_hdmi_update_audio_infoframe - Update the Audio Infoframe > * @connector: A pointer to the HDMI connector > - * @frame: A pointer to the audio infoframe to write > + * @frame: A pointer to the audio infoframe to write or NULL to disable sending the frame > * > * This function is meant for HDMI connector drivers to update their > * audio infoframe. It will typically be used in one of the ALSA hooks > @@ -704,10 +704,16 @@ drm_atomic_helper_connector_hdmi_update_audio_infoframe(struct drm_connector *co > > mutex_lock(&connector->hdmi.infoframes.lock); > > - memcpy(&infoframe->data, frame, sizeof(infoframe->data)); > - infoframe->set = true; > + if (frame) { > + memcpy(&infoframe->data, frame, sizeof(infoframe->data)); > + infoframe->set = true; > + > + ret = write_infoframe(connector, infoframe); > + } else { > + infoframe->set = false; > > - ret = write_infoframe(connector, infoframe); > + ret = clear_infoframe(connector, infoframe); > + } I'm not entirely sure your commit matches your commit log? It looks like you follow the same pattern than the other infoframes and call write_infoframe if there's one, or clear_infoframe if there isn't. So we're never passing NULL to disable? clear_infoframe is called. Maxime
On Thu, May 30, 2024 at 10:49:26AM +0200, Maxime Ripard wrote: > Hi, > > On Thu, May 30, 2024 at 02:12:24AM GMT, Dmitry Baryshkov wrote: > > Allow passing NULL as audio infoframe as a way to disable Audio > > Infoframe generation. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > > index ce96837eea65..5356723d21f5 100644 > > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > > @@ -681,7 +681,7 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_update_infoframes); > > /** > > * drm_atomic_helper_connector_hdmi_update_audio_infoframe - Update the Audio Infoframe > > * @connector: A pointer to the HDMI connector > > - * @frame: A pointer to the audio infoframe to write > > + * @frame: A pointer to the audio infoframe to write or NULL to disable sending the frame > > * > > * This function is meant for HDMI connector drivers to update their > > * audio infoframe. It will typically be used in one of the ALSA hooks > > @@ -704,10 +704,16 @@ drm_atomic_helper_connector_hdmi_update_audio_infoframe(struct drm_connector *co > > > > mutex_lock(&connector->hdmi.infoframes.lock); > > > > - memcpy(&infoframe->data, frame, sizeof(infoframe->data)); > > - infoframe->set = true; > > + if (frame) { > > + memcpy(&infoframe->data, frame, sizeof(infoframe->data)); > > + infoframe->set = true; > > + > > + ret = write_infoframe(connector, infoframe); > > + } else { > > + infoframe->set = false; > > > > - ret = write_infoframe(connector, infoframe); > > + ret = clear_infoframe(connector, infoframe); > > + } > > I'm not entirely sure your commit matches your commit log? It looks like > you follow the same pattern than the other infoframes and call > write_infoframe if there's one, or clear_infoframe if there isn't. > > So we're never passing NULL to disable? clear_infoframe is called. This function is being called from the driver, so I want to be able to call drm_atomic_helper_connector_hdmi_update_audio_infoframe(connector, NULL) in order to disable sending of the Audio InfoFrame.
diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c index ce96837eea65..5356723d21f5 100644 --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c @@ -681,7 +681,7 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_update_infoframes); /** * drm_atomic_helper_connector_hdmi_update_audio_infoframe - Update the Audio Infoframe * @connector: A pointer to the HDMI connector - * @frame: A pointer to the audio infoframe to write + * @frame: A pointer to the audio infoframe to write or NULL to disable sending the frame * * This function is meant for HDMI connector drivers to update their * audio infoframe. It will typically be used in one of the ALSA hooks @@ -704,10 +704,16 @@ drm_atomic_helper_connector_hdmi_update_audio_infoframe(struct drm_connector *co mutex_lock(&connector->hdmi.infoframes.lock); - memcpy(&infoframe->data, frame, sizeof(infoframe->data)); - infoframe->set = true; + if (frame) { + memcpy(&infoframe->data, frame, sizeof(infoframe->data)); + infoframe->set = true; + + ret = write_infoframe(connector, infoframe); + } else { + infoframe->set = false; - ret = write_infoframe(connector, infoframe); + ret = clear_infoframe(connector, infoframe); + } mutex_unlock(&connector->hdmi.infoframes.lock);
Allow passing NULL as audio infoframe as a way to disable Audio Infoframe generation. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/display/drm_hdmi_state_helper.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)