[PATCH 0/2] Add TDM setting support

[PATCH 0/2] Add TDM setting support

[Hsin-Te Yuan]

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 <yuanhsinte@chromium.org>
---
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 <yuanhsinte@chromium.org>

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

[Hsin-Te Yuan]

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 <yuanhsinte@chromium.org>
---
 .../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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]

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 <yuanhsinte@chromium.org>
> ---
>  .../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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Hsin-Te Yuan]

On Thu, May 2, 2024 at 10:47 PM Conor Dooley <conor@kernel.org> 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 <yuanhsinte@chromium.org>
> > ---
> >  .../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
> >

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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]

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 <conor@kernel.org> 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 <yuanhsinte@chromium.org>
> > > ---
> > >  .../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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Hsin-Te Yuan]

On Sat, May 4, 2024 at 12:45 AM Conor Dooley <conor@kernel.org> 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 <conor@kernel.org> 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 <yuanhsinte@chromium.org>
> > > > ---
> > > >  .../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

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

[Hsin-Te Yuan]

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 <yuanhsinte@chromium.org>
---
 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

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

[Robert Foss]

On Thu, May 2, 2024 at 11:03 AM Hsin-Te Yuan <yuanhsinte@chromium.org> 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 <yuanhsinte@chromium.org>
> ---
>  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
>
>

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

[Chen-Yu Tsai]

On Thu, May 2, 2024 at 5:03 PM Hsin-Te Yuan <yuanhsinte@chromium.org> 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 <yuanhsinte@chromium.org>
> ---
> 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 <yuanhsinte@chromium.org>
>