| Message ID | 20190805134102.24173-9-jbrunet@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | drm/bridge: dw-hdmi: improve i2s support | expand |
On 2019-08-05 15:41, Jerome Brunet wrote: > Provide the eld to the generic hdmi-codec driver. > This will let the driver enforce the maximum channel number and set the > channel allocation depending on the hdmi sink. > > Cc: Jonas Karlman <jonas@kwiboo.se> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h | 1 + > drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 10 ++++++++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 + > 3 files changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h > index 63b5756f463b..cb07dc0da5a7 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h > @@ -14,6 +14,7 @@ struct dw_hdmi_audio_data { > > struct dw_hdmi_i2s_audio_data { > struct dw_hdmi *hdmi; > + u8 *eld; > > void (*write)(struct dw_hdmi *hdmi, u8 val, int offset); > u8 (*read)(struct dw_hdmi *hdmi, int offset); > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c > index b8ece9c1ba2c..14d499b344c0 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c > @@ -121,6 +121,15 @@ static void dw_hdmi_i2s_audio_shutdown(struct device *dev, void *data) > dw_hdmi_audio_disable(hdmi); > } > > +static int dw_hdmi_i2s_get_eld(struct device *dev, void *data, uint8_t *buf, > + size_t len) > +{ > + struct dw_hdmi_i2s_audio_data *audio = data; > + > + memcpy(buf, audio->eld, min(sizeof(audio->eld), len)); Above sizeof does not work as intended, sizeof(audio->eld) is probably the size of a pointer and not MAX_ELD_BYTES (128), resulting in only part of the ELD gets copied to buf. Thanks for reworking dw-hdmi multi channel lpcm support!, this works on Rockchip for most parts. Without the [1] patch (reset cts+n after clock is enabled) audio sometime stay silent when switching between e.g. 2ch 44.1khz and 6ch 48khz tracks. It is not fully clear to me why reset cts+n helps, if that have negative affects on other platforms or if there is another way to solve loosing audio. I also have a small issue with the channel allocation being selected by hdmi-codec, my soundbar reports LPCM 6.1ch instead of LPCM 7.1ch when I play a 7.1ch speaker test clip. I have a hdmi-codec patch to fix selection of channel allocation in queue. For patch 1-7: Reviewed-by: Jonas Karlman <jonas@kwiboo.se> [1] https://github.com/Kwiboo/linux-rockchip/commit/c0839e874f843ad173ddde958303d6878394ef92 Regards, Jonas > + return 0; > +} > + > static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component, > struct device_node *endpoint) > { > @@ -144,6 +153,7 @@ static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component, > static struct hdmi_codec_ops dw_hdmi_i2s_ops = { > .hw_params = dw_hdmi_i2s_hw_params, > .audio_shutdown = dw_hdmi_i2s_audio_shutdown, > + .get_eld = dw_hdmi_i2s_get_eld, > .get_dai_id = dw_hdmi_i2s_get_dai_id, > }; > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index bed4bb017afd..8df69c9dbfad 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -2797,6 +2797,7 @@ __dw_hdmi_probe(struct platform_device *pdev, > struct dw_hdmi_i2s_audio_data audio; > > audio.hdmi = hdmi; > + audio.eld = hdmi->connector.eld; > audio.write = hdmi_writeb; > audio.read = hdmi_readb; > hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
On Wed 07 Aug 2019 at 14:57, Jonas Karlman <jonas@kwiboo.se> wrote: > On 2019-08-05 15:41, Jerome Brunet wrote: >> Provide the eld to the generic hdmi-codec driver. >> This will let the driver enforce the maximum channel number and set the >> channel allocation depending on the hdmi sink. >> >> Cc: Jonas Karlman <jonas@kwiboo.se> >> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> >> --- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h | 1 + >> drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 10 ++++++++++ >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 + >> 3 files changed, 12 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h >> index 63b5756f463b..cb07dc0da5a7 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h >> @@ -14,6 +14,7 @@ struct dw_hdmi_audio_data { >> >> struct dw_hdmi_i2s_audio_data { >> struct dw_hdmi *hdmi; >> + u8 *eld; >> >> void (*write)(struct dw_hdmi *hdmi, u8 val, int offset); >> u8 (*read)(struct dw_hdmi *hdmi, int offset); >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c >> index b8ece9c1ba2c..14d499b344c0 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c >> @@ -121,6 +121,15 @@ static void dw_hdmi_i2s_audio_shutdown(struct device *dev, void *data) >> dw_hdmi_audio_disable(hdmi); >> } >> >> +static int dw_hdmi_i2s_get_eld(struct device *dev, void *data, uint8_t *buf, >> + size_t len) >> +{ >> + struct dw_hdmi_i2s_audio_data *audio = data; >> + >> + memcpy(buf, audio->eld, min(sizeof(audio->eld), len)); > > Above sizeof does not work as intended, sizeof(audio->eld) is probably the size of a pointer and not MAX_ELD_BYTES (128), > resulting in only part of the ELD gets copied to buf. Silly ... thanks for pointing this out. I'll respin > > > Thanks for reworking dw-hdmi multi channel lpcm support!, this works on Rockchip for most parts. > Without the [1] patch (reset cts+n after clock is enabled) audio sometime stay silent when switching between e.g. 2ch 44.1khz and 6ch 48khz tracks. > It is not fully clear to me why reset cts+n helps, if that have > negative affects on other platforms or if there is another way to > solve loosing audio. I did not see that issue my self but I guess could propose this change ? > > I also have a small issue with the channel allocation being selected by hdmi-codec, my soundbar reports LPCM 6.1ch instead of LPCM 7.1ch when I play a 7.1ch speaker test clip. > I have a hdmi-codec patch to fix selection of channel allocation in > queue. Yes, I know there is still a few problems around that and stale eld. But those problem are not really coming from the i2s interface of the dw-hdmi itself and should be dealt with separatly. This is just the beginning ;) > > > For patch 1-7: > > Reviewed-by: Jonas Karlman <jonas@kwiboo.se> > > > [1] https://github.com/Kwiboo/linux-rockchip/commit/c0839e874f843ad173ddde958303d6878394ef92 > > Regards, > Jonas > >> + return 0; >> +} >> + >> static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component, >> struct device_node *endpoint) >> { >> @@ -144,6 +153,7 @@ static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component, >> static struct hdmi_codec_ops dw_hdmi_i2s_ops = { >> .hw_params = dw_hdmi_i2s_hw_params, >> .audio_shutdown = dw_hdmi_i2s_audio_shutdown, >> + .get_eld = dw_hdmi_i2s_get_eld, >> .get_dai_id = dw_hdmi_i2s_get_dai_id, >> }; >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index bed4bb017afd..8df69c9dbfad 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -2797,6 +2797,7 @@ __dw_hdmi_probe(struct platform_device *pdev, >> struct dw_hdmi_i2s_audio_data audio; >> >> audio.hdmi = hdmi; >> + audio.eld = hdmi->connector.eld; >> audio.write = hdmi_writeb; >> audio.read = hdmi_readb; >> hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h index 63b5756f463b..cb07dc0da5a7 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h @@ -14,6 +14,7 @@ struct dw_hdmi_audio_data { struct dw_hdmi_i2s_audio_data { struct dw_hdmi *hdmi; + u8 *eld; void (*write)(struct dw_hdmi *hdmi, u8 val, int offset); u8 (*read)(struct dw_hdmi *hdmi, int offset); diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c index b8ece9c1ba2c..14d499b344c0 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c @@ -121,6 +121,15 @@ static void dw_hdmi_i2s_audio_shutdown(struct device *dev, void *data) dw_hdmi_audio_disable(hdmi); } +static int dw_hdmi_i2s_get_eld(struct device *dev, void *data, uint8_t *buf, + size_t len) +{ + struct dw_hdmi_i2s_audio_data *audio = data; + + memcpy(buf, audio->eld, min(sizeof(audio->eld), len)); + return 0; +} + static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component, struct device_node *endpoint) { @@ -144,6 +153,7 @@ static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component, static struct hdmi_codec_ops dw_hdmi_i2s_ops = { .hw_params = dw_hdmi_i2s_hw_params, .audio_shutdown = dw_hdmi_i2s_audio_shutdown, + .get_eld = dw_hdmi_i2s_get_eld, .get_dai_id = dw_hdmi_i2s_get_dai_id, }; diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index bed4bb017afd..8df69c9dbfad 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2797,6 +2797,7 @@ __dw_hdmi_probe(struct platform_device *pdev, struct dw_hdmi_i2s_audio_data audio; audio.hdmi = hdmi; + audio.eld = hdmi->connector.eld; audio.write = hdmi_writeb; audio.read = hdmi_readb; hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
Provide the eld to the generic hdmi-codec driver. This will let the driver enforce the maximum channel number and set the channel allocation depending on the hdmi sink. Cc: Jonas Karlman <jonas@kwiboo.se> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 10 ++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 + 3 files changed, 12 insertions(+)