1. Patchew
  2. linux
  3. [PATCH v5 0/7] drm/bridge: imx: Add HDMI PAI driver on i.MX8MP
  4. View patch

[PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format

Shengjiu Wang posted 7 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format
Posted by Shengjiu Wang 1 month, 1 week ago 
Add API dw_hdmi_set_sample_iec958() for IEC958 format because audio device
driver needs IEC958 information to configure this specific setting.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Liu Ying <victor.liu@nxp.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c |  5 +++++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c          | 12 +++++++++++-
 include/drm/bridge/dw_hdmi.h                       |  3 ++-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c
index ab18f9a3bf23..df7a37eb47f4 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c
@@ -90,6 +90,11 @@ static int audio_hw_params(struct device *dev,  void *data,
 				   params->iec.status[0] & IEC958_AES0_NONAUDIO);
 	dw_hdmi_set_sample_width(dw->data.hdmi, params->sample_width);
 
+	if (daifmt->bit_fmt == SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE)
+		dw_hdmi_set_sample_iec958(dw->data.hdmi, 1);
+	else
+		dw_hdmi_set_sample_iec958(dw->data.hdmi, 0);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 8d096b569cf1..3b77e73ac0ea 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -177,6 +177,7 @@ struct dw_hdmi {
 
 	spinlock_t audio_lock;
 	struct mutex audio_mutex;
+	unsigned int sample_iec958;
 	unsigned int sample_non_pcm;
 	unsigned int sample_width;
 	unsigned int sample_rate;
@@ -718,6 +719,14 @@ void dw_hdmi_set_sample_non_pcm(struct dw_hdmi *hdmi, unsigned int non_pcm)
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_non_pcm);
 
+void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958)
+{
+	mutex_lock(&hdmi->audio_mutex);
+	hdmi->sample_iec958 = iec958;
+	mutex_unlock(&hdmi->audio_mutex);
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_iec958);
+
 void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
 {
 	mutex_lock(&hdmi->audio_mutex);
@@ -849,7 +858,8 @@ static void dw_hdmi_gp_audio_enable(struct dw_hdmi *hdmi)
 				    hdmi->channels,
 				    hdmi->sample_width,
 				    hdmi->sample_rate,
-				    hdmi->sample_non_pcm);
+				    hdmi->sample_non_pcm,
+				    hdmi->sample_iec958);
 }
 
 static void dw_hdmi_gp_audio_disable(struct dw_hdmi *hdmi)
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index b8fc4fdf5a21..095cdd9b7424 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -145,7 +145,7 @@ struct dw_hdmi_plat_data {
 
 	/* Platform-specific audio enable/disable (optional) */
 	void (*enable_audio)(struct dw_hdmi *hdmi, int channel,
-			     int width, int rate, int non_pcm);
+			     int width, int rate, int non_pcm, int iec958);
 	void (*disable_audio)(struct dw_hdmi *hdmi);
 
 	/* Vendor PHY support */
@@ -179,6 +179,7 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
 int dw_hdmi_set_plugged_cb(struct dw_hdmi *hdmi, hdmi_codec_plugged_cb fn,
 			   struct device *codec_dev);
 void dw_hdmi_set_sample_non_pcm(struct dw_hdmi *hdmi, unsigned int non_pcm);
+void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958);
 void dw_hdmi_set_sample_width(struct dw_hdmi *hdmi, unsigned int width);
 void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
 void dw_hdmi_set_channel_count(struct dw_hdmi *hdmi, unsigned int cnt);
-- 
2.34.1
Re: [PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format
Posted by Luca Ceresoli 1 month ago 
Hello Shengjiu,

On Thu, 21 Aug 2025 15:31:28 +0800
Shengjiu Wang <shengjiu.wang@nxp.com> wrote:

> Add API dw_hdmi_set_sample_iec958() for IEC958 format because audio device
> driver needs IEC958 information to configure this specific setting.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> Acked-by: Liu Ying <victor.liu@nxp.com>

[...]

> +void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958)
> +{
> +	mutex_lock(&hdmi->audio_mutex);
> +	hdmi->sample_iec958 = iec958;
> +	mutex_unlock(&hdmi->audio_mutex);
> +}

Apologies for jumping in the discussion as late as in v5, but I noticed
this patch and I was wondering whether this mutex_lock/unlock() is
really needed, as you're copying an int.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format
Posted by Shengjiu Wang 1 month ago 
On Tue, Sep 2, 2025 at 12:52 AM Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
>
> Hello Shengjiu,
>
> On Thu, 21 Aug 2025 15:31:28 +0800
> Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
>
> > Add API dw_hdmi_set_sample_iec958() for IEC958 format because audio device
> > driver needs IEC958 information to configure this specific setting.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > Acked-by: Liu Ying <victor.liu@nxp.com>
>
> [...]
>
> > +void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958)
> > +{
> > +     mutex_lock(&hdmi->audio_mutex);
> > +     hdmi->sample_iec958 = iec958;
> > +     mutex_unlock(&hdmi->audio_mutex);
> > +}
>
> Apologies for jumping in the discussion as late as in v5, but I noticed
> this patch and I was wondering whether this mutex_lock/unlock() is
> really needed, as you're copying an int.

Thanks for your comments.

Seems it is not necessary to add mutex here. I just follow the code as
other similar functions.  I will send a new version to update it.

Best regards
Shengjiu Wang

Shengjiu Wang
>
> Luca
>
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Re: [PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format
Posted by Maxime Ripard 3 weeks, 3 days ago 
Hi,

On Wed, Sep 03, 2025 at 06:41:05PM +0800, Shengjiu Wang wrote:
> On Tue, Sep 2, 2025 at 12:52 AM Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
> >
> > Hello Shengjiu,
> >
> > On Thu, 21 Aug 2025 15:31:28 +0800
> > Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
> >
> > > Add API dw_hdmi_set_sample_iec958() for IEC958 format because audio device
> > > driver needs IEC958 information to configure this specific setting.
> > >
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > Acked-by: Liu Ying <victor.liu@nxp.com>
> >
> > [...]
> >
> > > +void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958)
> > > +{
> > > +     mutex_lock(&hdmi->audio_mutex);
> > > +     hdmi->sample_iec958 = iec958;
> > > +     mutex_unlock(&hdmi->audio_mutex);
> > > +}
> >
> > Apologies for jumping in the discussion as late as in v5, but I noticed
> > this patch and I was wondering whether this mutex_lock/unlock() is
> > really needed, as you're copying an int.
> 
> Thanks for your comments.
> 
> Seems it is not necessary to add mutex here. I just follow the code as
> other similar functions.  I will send a new version to update it.

Let's not be smart about it. Next thing you know, someone will add
another field in there that would absolutely require a mutex and now
you're not race free anymore.

Unless there's a real concern, the mutex must stay.

Maxime
Re: [PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format
Posted by Shengjiu Wang 3 weeks, 2 days ago 
Hi

On Tue, Sep 9, 2025 at 2:39 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> Hi,
>
> On Wed, Sep 03, 2025 at 06:41:05PM +0800, Shengjiu Wang wrote:
> > On Tue, Sep 2, 2025 at 12:52 AM Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
> > >
> > > Hello Shengjiu,
> > >
> > > On Thu, 21 Aug 2025 15:31:28 +0800
> > > Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
> > >
> > > > Add API dw_hdmi_set_sample_iec958() for IEC958 format because audio device
> > > > driver needs IEC958 information to configure this specific setting.
> > > >
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > Acked-by: Liu Ying <victor.liu@nxp.com>
> > >
> > > [...]
> > >
> > > > +void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958)
> > > > +{
> > > > +     mutex_lock(&hdmi->audio_mutex);
> > > > +     hdmi->sample_iec958 = iec958;
> > > > +     mutex_unlock(&hdmi->audio_mutex);
> > > > +}
> > >
> > > Apologies for jumping in the discussion as late as in v5, but I noticed
> > > this patch and I was wondering whether this mutex_lock/unlock() is
> > > really needed, as you're copying an int.
> >
> > Thanks for your comments.
> >
> > Seems it is not necessary to add mutex here. I just follow the code as
> > other similar functions.  I will send a new version to update it.
>
> Let's not be smart about it. Next thing you know, someone will add
> another field in there that would absolutely require a mutex and now
> you're not race free anymore.
>
> Unless there's a real concern, the mutex must stay.
>

Ok, thanks for comments.  Then Patch v6 need to be dropped.

Is there any other comments for this Patch v5?
If no, can this series be accepted?

Best regards
Shengjiu Wang
Re: [PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format
Posted by Liu Ying 1 week, 4 days ago 
On 09/10/2025, Shengjiu Wang wrote:
> Hi
> 
> On Tue, Sep 9, 2025 at 2:39 PM Maxime Ripard <mripard@kernel.org> wrote:
>>
>> Hi,
>>
>> On Wed, Sep 03, 2025 at 06:41:05PM +0800, Shengjiu Wang wrote:
>>> On Tue, Sep 2, 2025 at 12:52 AM Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
>>>>
>>>> Hello Shengjiu,
>>>>
>>>> On Thu, 21 Aug 2025 15:31:28 +0800
>>>> Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
>>>>
>>>>> Add API dw_hdmi_set_sample_iec958() for IEC958 format because audio device
>>>>> driver needs IEC958 information to configure this specific setting.
>>>>>
>>>>> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
>>>>> Acked-by: Liu Ying <victor.liu@nxp.com>
>>>>
>>>> [...]
>>>>
>>>>> +void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958)
>>>>> +{
>>>>> +     mutex_lock(&hdmi->audio_mutex);
>>>>> +     hdmi->sample_iec958 = iec958;
>>>>> +     mutex_unlock(&hdmi->audio_mutex);
>>>>> +}
>>>>
>>>> Apologies for jumping in the discussion as late as in v5, but I noticed
>>>> this patch and I was wondering whether this mutex_lock/unlock() is
>>>> really needed, as you're copying an int.
>>>
>>> Thanks for your comments.
>>>
>>> Seems it is not necessary to add mutex here. I just follow the code as
>>> other similar functions.  I will send a new version to update it.
>>
>> Let's not be smart about it. Next thing you know, someone will add
>> another field in there that would absolutely require a mutex and now
>> you're not race free anymore.
>>
>> Unless there's a real concern, the mutex must stay.
>>
> 
> Ok, thanks for comments.  Then Patch v6 need to be dropped.

To properly track the changelog with patchwork, can you send v7 to add
the mutex?

> 
> Is there any other comments for this Patch v5?
> If no, can this series be accepted?
> 
> Best regards
> Shengjiu Wang


-- 
Regards,
Liu Ying
Re: [PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format
Posted by Shengjiu Wang 1 week, 4 days ago 
On Mon, Sep 22, 2025 at 11:01 AM Liu Ying <victor.liu@nxp.com> wrote:
>
> On 09/10/2025, Shengjiu Wang wrote:
> > Hi
> >
> > On Tue, Sep 9, 2025 at 2:39 PM Maxime Ripard <mripard@kernel.org> wrote:
> >>
> >> Hi,
> >>
> >> On Wed, Sep 03, 2025 at 06:41:05PM +0800, Shengjiu Wang wrote:
> >>> On Tue, Sep 2, 2025 at 12:52 AM Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
> >>>>
> >>>> Hello Shengjiu,
> >>>>
> >>>> On Thu, 21 Aug 2025 15:31:28 +0800
> >>>> Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
> >>>>
> >>>>> Add API dw_hdmi_set_sample_iec958() for IEC958 format because audio device
> >>>>> driver needs IEC958 information to configure this specific setting.
> >>>>>
> >>>>> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> >>>>> Acked-by: Liu Ying <victor.liu@nxp.com>
> >>>>
> >>>> [...]
> >>>>
> >>>>> +void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958)
> >>>>> +{
> >>>>> +     mutex_lock(&hdmi->audio_mutex);
> >>>>> +     hdmi->sample_iec958 = iec958;
> >>>>> +     mutex_unlock(&hdmi->audio_mutex);
> >>>>> +}
> >>>>
> >>>> Apologies for jumping in the discussion as late as in v5, but I noticed
> >>>> this patch and I was wondering whether this mutex_lock/unlock() is
> >>>> really needed, as you're copying an int.
> >>>
> >>> Thanks for your comments.
> >>>
> >>> Seems it is not necessary to add mutex here. I just follow the code as
> >>> other similar functions.  I will send a new version to update it.
> >>
> >> Let's not be smart about it. Next thing you know, someone will add
> >> another field in there that would absolutely require a mutex and now
> >> you're not race free anymore.
> >>
> >> Unless there's a real concern, the mutex must stay.
> >>
> >
> > Ok, thanks for comments.  Then Patch v6 need to be dropped.
>
> To properly track the changelog with patchwork, can you send v7 to add
> the mutex?

Yes, will send it.

Best regards

Shengjiu wang

>
> >
> > Is there any other comments for this Patch v5?
> > If no, can this series be accepted?
> >
> > Best regards
> > Shengjiu Wang
>
>
> --
> Regards,
> Liu Ying
Re: [PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format
Posted by Shengjiu Wang 2 weeks ago 
Hi Luca

On Wed, Sep 10, 2025 at 6:29 PM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
>
> Hi
>
> On Tue, Sep 9, 2025 at 2:39 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > Hi,
> >
> > On Wed, Sep 03, 2025 at 06:41:05PM +0800, Shengjiu Wang wrote:
> > > On Tue, Sep 2, 2025 at 12:52 AM Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
> > > >
> > > > Hello Shengjiu,
> > > >
> > > > On Thu, 21 Aug 2025 15:31:28 +0800
> > > > Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
> > > >
> > > > > Add API dw_hdmi_set_sample_iec958() for IEC958 format because audio device
> > > > > driver needs IEC958 information to configure this specific setting.
> > > > >
> > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > Acked-by: Liu Ying <victor.liu@nxp.com>
> > > >
> > > > [...]
> > > >
> > > > > +void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958)
> > > > > +{
> > > > > +     mutex_lock(&hdmi->audio_mutex);
> > > > > +     hdmi->sample_iec958 = iec958;
> > > > > +     mutex_unlock(&hdmi->audio_mutex);
> > > > > +}
> > > >
> > > > Apologies for jumping in the discussion as late as in v5, but I noticed
> > > > this patch and I was wondering whether this mutex_lock/unlock() is
> > > > really needed, as you're copying an int.
> > >
> > > Thanks for your comments.
> > >
> > > Seems it is not necessary to add mutex here. I just follow the code as
> > > other similar functions.  I will send a new version to update it.
> >
> > Let's not be smart about it. Next thing you know, someone will add
> > another field in there that would absolutely require a mutex and now
> > you're not race free anymore.
> >
> > Unless there's a real concern, the mutex must stay.
> >
>
> Ok, thanks for comments.  Then Patch v6 need to be dropped.
>
> Is there any other comments for this Patch v5?
> If no, can this series be accepted?
>

Can we have a conclusion that keeps the mutex as Maxime's comments?

Best regards
Shengjiu Wang
Re: [PATCH v5 4/7] drm/bridge: dw-hdmi: Add API dw_hdmi_set_sample_iec958() for iec958 format
Posted by Luca Ceresoli 2 weeks ago 
Hello Shengjiu,

On Fri, 19 Sep 2025 11:43:05 +0800
Shengjiu Wang <shengjiu.wang@gmail.com> wrote:

> Hi Luca
> 
> On Wed, Sep 10, 2025 at 6:29 PM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
> >
> > Hi
> >
> > On Tue, Sep 9, 2025 at 2:39 PM Maxime Ripard <mripard@kernel.org> wrote:  
> > >
> > > Hi,
> > >
> > > On Wed, Sep 03, 2025 at 06:41:05PM +0800, Shengjiu Wang wrote:  
> > > > On Tue, Sep 2, 2025 at 12:52 AM Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:  
> > > > >
> > > > > Hello Shengjiu,
> > > > >
> > > > > On Thu, 21 Aug 2025 15:31:28 +0800
> > > > > Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
> > > > >  
> > > > > > Add API dw_hdmi_set_sample_iec958() for IEC958 format because audio device
> > > > > > driver needs IEC958 information to configure this specific setting.
> > > > > >
> > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > > Acked-by: Liu Ying <victor.liu@nxp.com>  
> > > > >
> > > > > [...]
> > > > >  
> > > > > > +void dw_hdmi_set_sample_iec958(struct dw_hdmi *hdmi, unsigned int iec958)
> > > > > > +{
> > > > > > +     mutex_lock(&hdmi->audio_mutex);
> > > > > > +     hdmi->sample_iec958 = iec958;
> > > > > > +     mutex_unlock(&hdmi->audio_mutex);
> > > > > > +}  
> > > > >
> > > > > Apologies for jumping in the discussion as late as in v5, but I noticed
> > > > > this patch and I was wondering whether this mutex_lock/unlock() is
> > > > > really needed, as you're copying an int.  
> > > >
> > > > Thanks for your comments.
> > > >
> > > > Seems it is not necessary to add mutex here. I just follow the code as
> > > > other similar functions.  I will send a new version to update it.  
> > >
> > > Let's not be smart about it. Next thing you know, someone will add
> > > another field in there that would absolutely require a mutex and now
> > > you're not race free anymore.
> > >
> > > Unless there's a real concern, the mutex must stay.
> > >  
> >
> > Ok, thanks for comments.  Then Patch v6 need to be dropped.
> >
> > Is there any other comments for this Patch v5?
> > If no, can this series be accepted?
> >  
> 
> Can we have a conclusion that keeps the mutex as Maxime's comments?

If Maxime wants the mutex to be kept, it's fine.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.