2024-05-02 09:03:47

by Hsin-Te Yuan

[permalink] [raw]
Subject: [PATCH 0/2] Add TDM setting support

The anx7625 supports two different TDM settings, which determine whether
or not the first audio data bit should be shifted. This series adds the
support for configuring TDM setting through a property in the device
tree.

Signed-off-by: Hsin-Te Yuan <[email protected]>
---
Hsin-Te Yuan (2):
dt-bindings: drm/bridge: anx7625: Add a perporty to change TDM setting
drm/bridge: anx7625: Change TDM setting accroding to dt property

.../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++
drivers/gpu/drm/bridge/analogix/anx7625.c | 8 ++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
3 files changed, 13 insertions(+)
---
base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
change-id: 20240430-anx-tdm-7ce41a0a901d

Best regards,
--
Hsin-Te Yuan <[email protected]>



2024-05-02 09:04:02

by Hsin-Te Yuan

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: drm/bridge: anx7625: Add a perporty to change TDM setting

Add a perporty to indicate whether anx7625 should shift the first audio
data bit. The default TDM setting is to shift the first audio data bit.

Signed-off-by: Hsin-Te Yuan <[email protected]>
---
.../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index a1ed1004651b9..915d5d54a2160 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -82,6 +82,10 @@ properties:
type: boolean
description: let the driver enable audio HDMI codec function or not.

+ no-shift-audio-data:
+ type: boolean
+ description: Disable the first audio data bit shift in the TDM settings.
+
aux-bus:
$ref: /schemas/display/dp-aux-bus.yaml#


--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-02 09:09:58

by Hsin-Te Yuan

[permalink] [raw]
Subject: [PATCH 2/2] drm/bridge: anx7625: Change TDM setting accroding to dt property

For some SoCs, the TDM setting is not to shift the first audio data bit,
which is not the default setting of anx7625. In such cases, the TDM
setting should be changed according to the device tree property.

Signed-off-by: Hsin-Te Yuan <[email protected]>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 8 ++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 29d91493b101a..538edddf313c9 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1709,6 +1709,9 @@ static int anx7625_parse_dt(struct device *dev,
if (of_property_read_bool(np, "analogix,audio-enable"))
pdata->audio_en = 1;

+ if(!of_property_read_bool(np, "no-shift-audio-data"))
+ pdata->shift_audio_data = 1;
+
return 0;
}

@@ -1866,6 +1869,11 @@ static int anx7625_audio_hw_params(struct device *dev, void *data,
~TDM_SLAVE_MODE,
I2S_SLAVE_MODE);

+ if (!ctx->pdata.shift_audio_data)
+ ret |= anx7625_write_or(ctx, ctx->i2c.tx_p2_client,
+ AUDIO_CONTROL_REGISTER,
+ TDM_TIMING_MODE);
+
/* Word length */
switch (params->sample_width) {
case 16:
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 39ed35d338363..41b395725913a 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -441,6 +441,7 @@ struct anx7625_platform_data {
u8 lane1_reg_data[DP_TX_SWING_REG_CNT];
u32 low_power_mode;
struct device_node *mipi_host_node;
+ int shift_audio_data;
};

struct anx7625_i2c_client {

--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-02 09:34:05

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 0/2] Add TDM setting support

On Thu, May 2, 2024 at 5:03 PM Hsin-Te Yuan <[email protected]> wrote:
>
> The anx7625 supports two different TDM settings, which determine whether
> or not the first audio data bit should be shifted. This series adds the
> support for configuring TDM setting through a property in the device
> tree.

As mentioned offline, this shouldn't need a DT property. Instead the
machine driver dictates which format is used to the DAIs, and the
DAIs set their respective settings accordingly. It would seem that
one of the drivers is misbehaving.

ChenYu

> Signed-off-by: Hsin-Te Yuan <[email protected]>
> ---
> Hsin-Te Yuan (2):
> dt-bindings: drm/bridge: anx7625: Add a perporty to change TDM setting
> drm/bridge: anx7625: Change TDM setting accroding to dt property
>
> .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++
> drivers/gpu/drm/bridge/analogix/anx7625.c | 8 ++++++++
> drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
> 3 files changed, 13 insertions(+)
> ---
> base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
> change-id: 20240430-anx-tdm-7ce41a0a901d
>
> Best regards,
> --
> Hsin-Te Yuan <[email protected]>
>

2024-05-02 14:50:28

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: drm/bridge: anx7625: Add a perporty to change TDM setting

On Thu, May 02, 2024 at 09:03:31AM +0000, Hsin-Te Yuan wrote:
> Add a perporty to indicate whether anx7625 should shift the first audio
> data bit. The default TDM setting is to shift the first audio data bit.
>
> Signed-off-by: Hsin-Te Yuan <[email protected]>
> ---
> .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index a1ed1004651b9..915d5d54a2160 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -82,6 +82,10 @@ properties:
> type: boolean
> description: let the driver enable audio HDMI codec function or not.
>
> + no-shift-audio-data:
> + type: boolean
> + description: Disable the first audio data bit shift in the TDM settings.

This just looks like software policy, since there's no mention in the
commit message or description as to what property of the hardware causes
this to be required. Can you please explain why this property is needed?

You're also missing a vendor prefix.

Thanks,
Conor.

> +
> aux-bus:
> $ref: /schemas/display/dp-aux-bus.yaml#
>
>
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>


Attachments:
(No filename) (1.41 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-03 06:59:07

by Hsin-Te Yuan

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: drm/bridge: anx7625: Add a perporty to change TDM setting

On Thu, May 2, 2024 at 10:47 PM Conor Dooley <[email protected]> wrote:
>
> On Thu, May 02, 2024 at 09:03:31AM +0000, Hsin-Te Yuan wrote:
> > Add a perporty to indicate whether anx7625 should shift the first audio
> > data bit. The default TDM setting is to shift the first audio data bit.
> >
> > Signed-off-by: Hsin-Te Yuan <[email protected]>
> > ---
> > .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > index a1ed1004651b9..915d5d54a2160 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625yaml
> > @@ -82,6 +82,10 @@ properties:
> > type: boolean
> > description: let the driver enable audio HDMI codec function or not.
> >
> > + no-shift-audio-data:
> > + type: boolean
> > + description: Disable the first audio data bit shift in the TDM settings.
>
> This just looks like software policy, since there's no mention in the
> commit message or description as to what property of the hardware causes
> this to be required. Can you please explain why this property is needed?
>
> You're also missing a vendor prefix.
>
> Thanks,
> Conor.
>
> > +
> > aux-bus:
> > $ref: /schemas/display/dp-aux-bus.yaml#
> >
> >
> > --
> > 2.45.0.rc1.225.g2a3ae87e7f-goog
> >

Sorry, I found this feature in the datasheet originally, but after
deeper investigation, it seems that this feature should be used to
support i2s dsp mode b instead of being used this way. Note that the
difference between i2s dsp mode a and b is whether or not to shift the
audio data by 1 clock cycle.

Regards,
Hsin-Te

2024-05-03 16:45:57

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: drm/bridge: anx7625: Add a perporty to change TDM setting

On Fri, May 03, 2024 at 02:58:16PM +0800, Hsin-Te Yuan wrote:
> On Thu, May 2, 2024 at 10:47 PM Conor Dooley <[email protected]> wrote:
> >
> > On Thu, May 02, 2024 at 09:03:31AM +0000, Hsin-Te Yuan wrote:
> > > Add a perporty to indicate whether anx7625 should shift the first audio
> > > data bit. The default TDM setting is to shift the first audio data bit.
> > >
> > > Signed-off-by: Hsin-Te Yuan <[email protected]>
> > > ---
> > > .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > index a1ed1004651b9..915d5d54a2160 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > @@ -82,6 +82,10 @@ properties:
> > > type: boolean
> > > description: let the driver enable audio HDMI codec function or not.
> > >
> > > + no-shift-audio-data:
> > > + type: boolean
> > > + description: Disable the first audio data bit shift in the TDM settings.
> >
> > This just looks like software policy, since there's no mention in the
> > commit message or description as to what property of the hardware causes
> > this to be required. Can you please explain why this property is needed?
> >
> > You're also missing a vendor prefix.
>
> Sorry, I found this feature in the datasheet originally, but after
> deeper investigation, it seems that this feature should be used to
> support i2s dsp mode b instead of being used this way. Note that the
> difference between i2s dsp mode a and b is whether or not to shift the
> audio data by 1 clock cycle.

Are you trying to say that this patch is not needed? I'm not really
sure.


Attachments:
(No filename) (1.89 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-03 17:13:12

by Hsin-Te Yuan

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: drm/bridge: anx7625: Add a perporty to change TDM setting

On Sat, May 4, 2024 at 12:45 AM Conor Dooley <[email protected]> wrote:
>
> On Fri, May 03, 2024 at 02:58:16PM +0800, Hsin-Te Yuan wrote:
> > On Thu, May 2, 2024 at 10:47 PM Conor Dooley <[email protected]> wrote:
> > >
> > > On Thu, May 02, 2024 at 09:03:31AM +0000, Hsin-Te Yuan wrote:
> > > > Add a perporty to indicate whether anx7625 should shift the first audio
> > > > data bit. The default TDM setting is to shift the first audio data bit.
> > > >
> > > > Signed-off-by: Hsin-Te Yuan <[email protected]>
> > > > ---
> > > > .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > index a1ed1004651b9..915d5d54a2160 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > @@ -82,6 +82,10 @@ properties:
> > > > type: boolean
> > > > description: let the driver enable audio HDMI codec function or not.
> > > >
> > > > + no-shift-audio-data:
> > > > + type: boolean
> > > > + description: Disable the first audio data bit shift in the TDM settings.
> > >
> > > This just looks like software policy, since there's no mention in the
> > > commit message or description as to what property of the hardware causes
> > > this to be required. Can you please explain why this property is needed?
> > >
> > > You're also missing a vendor prefix.
> >
> > Sorry, I found this feature in the datasheet originally, but after
> > deeper investigation, it seems that this feature should be used to
> > support i2s dsp mode b instead of being used this way. Note that the
> > difference between i2s dsp mode a and b is whether or not to shift the
> > audio data by 1 clock cycle.
>
> Are you trying to say that this patch is not needed? I'm not really
> sure.

Yes!

Regards,
Hsin-Te

2024-05-06 12:17:02

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH 2/2] drm/bridge: anx7625: Change TDM setting accroding to dt property

On Thu, May 2, 2024 at 11:03 AM Hsin-Te Yuan <[email protected]> wrote:
>
> For some SoCs, the TDM setting is not to shift the first audio data bit,
> which is not the default setting of anx7625. In such cases, the TDM
> setting should be changed according to the device tree property.
>
> Signed-off-by: Hsin-Te Yuan <[email protected]>
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 8 ++++++++
> drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 29d91493b101a..538edddf313c9 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1709,6 +1709,9 @@ static int anx7625_parse_dt(struct device *dev,
> if (of_property_read_bool(np, "analogix,audio-enable"))
> pdata->audio_en = 1;
>
> + if(!of_property_read_bool(np, "no-shift-audio-data"))
> + pdata->shift_audio_data = 1;

checkpatch --strict reports this:

ERROR: space required before the open parenthesis '('
#27: FILE: drivers/gpu/drm/bridge/analogix/anx7625.c:1712:
+ if(!of_property_read_bool(np, "no-shift-audio-data"))


> +
> return 0;
> }
>
> @@ -1866,6 +1869,11 @@ static int anx7625_audio_hw_params(struct device *dev, void *data,
> ~TDM_SLAVE_MODE,
> I2S_SLAVE_MODE);
>
> + if (!ctx->pdata.shift_audio_data)
> + ret |= anx7625_write_or(ctx, ctx->i2c.tx_p2_client,
> + AUDIO_CONTROL_REGISTER,
> + TDM_TIMING_MODE);
> +
> /* Word length */
> switch (params->sample_width) {
> case 16:
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
> index 39ed35d338363..41b395725913a 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.h
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
> @@ -441,6 +441,7 @@ struct anx7625_platform_data {
> u8 lane1_reg_data[DP_TX_SWING_REG_CNT];
> u32 low_power_mode;
> struct device_node *mipi_host_node;
> + int shift_audio_data;
> };
>
> struct anx7625_i2c_client {
>
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>
>