[PATCH 7/8] ALSA: hda: Fix single byte writing issue for Hygon family 18h model 5h

Fu Hao posted 8 patches 6 days, 8 hours ago
[PATCH 7/8] ALSA: hda: Fix single byte writing issue for Hygon family 18h model 5h
Posted by Fu Hao 6 days, 8 hours ago
On Hygon family 18h model 5h controller, some registers such as
GCTL, SD_CTL and SD_CTL_3B should be accessed in dword, or the
writing will fail.

Signed-off-by: Fu Hao <fuhao@open-hieco.net>
---
 include/sound/hdaudio.h       |  1 +
 sound/hda/controllers/intel.c |  4 ++++
 sound/hda/core/controller.c   | 10 +++++++--
 sound/hda/core/stream.c       | 42 ++++++++++++++++++++++++++---------
 4 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index f11bfc6b9..57a144fec 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -352,6 +352,7 @@ struct hdac_bus {
 	bool not_use_interrupts:1;	/* prohibiting the RIRB IRQ */
 	bool access_sdnctl_in_dword:1;	/* accessing the sdnctl register by dword */
 	bool use_pio_for_commands:1;	/* Use PIO instead of CORB for commands */
+	bool hygon_dword_access:1;
 
 	int poll_count;
 
diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
index eb5d48d90..6b27248dc 100644
--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -1885,6 +1885,10 @@ static int azx_first_init(struct azx *chip)
 	if (chip->driver_type == AZX_DRIVER_ZHAOXINHDMI)
 		bus->polling_mode = 1;
 
+	if (chip->driver_type == AZX_DRIVER_HYGON &&
+	    chip->pci->device == PCI_DEVICE_ID_HYGON_18H_M05H_HDA)
+		bus->hygon_dword_access = 1;
+
 	bus->remap_addr = pcim_iomap_region(pci, 0, "ICH HD audio");
 	if (IS_ERR(bus->remap_addr))
 		return PTR_ERR(bus->remap_addr);
diff --git a/sound/hda/core/controller.c b/sound/hda/core/controller.c
index 69e11d62b..bfe817045 100644
--- a/sound/hda/core/controller.c
+++ b/sound/hda/core/controller.c
@@ -511,7 +511,10 @@ void snd_hdac_bus_exit_link_reset(struct hdac_bus *bus)
 {
 	unsigned long timeout;
 
-	snd_hdac_chip_updateb(bus, GCTL, AZX_GCTL_RESET, AZX_GCTL_RESET);
+	if (bus->hygon_dword_access)
+		snd_hdac_chip_updatel(bus, GCTL, AZX_GCTL_RESET, AZX_GCTL_RESET);
+	else
+		snd_hdac_chip_updateb(bus, GCTL, AZX_GCTL_RESET, AZX_GCTL_RESET);
 
 	timeout = jiffies + msecs_to_jiffies(100);
 	while (!snd_hdac_chip_readb(bus, GCTL) && time_before(jiffies, timeout))
@@ -576,7 +579,10 @@ static void azx_int_disable(struct hdac_bus *bus)
 
 	/* disable interrupts in stream descriptor */
 	list_for_each_entry(azx_dev, &bus->stream_list, list)
-		snd_hdac_stream_updateb(azx_dev, SD_CTL, SD_INT_MASK, 0);
+		if (bus->hygon_dword_access)
+			snd_hdac_stream_updatel(azx_dev, SD_CTL, SD_INT_MASK, 0);
+		else
+			snd_hdac_stream_updateb(azx_dev, SD_CTL, SD_INT_MASK, 0);
 
 	/* disable SIE for all streams & disable controller CIE and GIE */
 	snd_hdac_chip_writel(bus, INTCTL, 0);
diff --git a/sound/hda/core/stream.c b/sound/hda/core/stream.c
index b471a038b..ccc0003a8 100644
--- a/sound/hda/core/stream.c
+++ b/sound/hda/core/stream.c
@@ -146,11 +146,15 @@ void snd_hdac_stream_start(struct hdac_stream *azx_dev)
 			stripe_ctl = snd_hdac_get_stream_stripe_ctl(bus, azx_dev->substream);
 		else
 			stripe_ctl = 0;
-		snd_hdac_stream_updateb(azx_dev, SD_CTL_3B, SD_CTL_STRIPE_MASK,
-					stripe_ctl);
+		if (bus->hygon_dword_access)
+			snd_hdac_stream_updatel(azx_dev, SD_CTL_3B, SD_CTL_STRIPE_MASK,
+						stripe_ctl);
+		else
+			snd_hdac_stream_updateb(azx_dev, SD_CTL_3B, SD_CTL_STRIPE_MASK,
+						stripe_ctl);
 	}
 	/* set DMA start and interrupt mask */
-	if (bus->access_sdnctl_in_dword)
+	if (bus->access_sdnctl_in_dword || bus->hygon_dword_access)
 		snd_hdac_stream_updatel(azx_dev, SD_CTL,
 				0, SD_CTL_DMA_START | SD_INT_MASK);
 	else
@@ -166,11 +170,22 @@ EXPORT_SYMBOL_GPL(snd_hdac_stream_start);
  */
 static void snd_hdac_stream_clear(struct hdac_stream *azx_dev)
 {
-	snd_hdac_stream_updateb(azx_dev, SD_CTL,
-				SD_CTL_DMA_START | SD_INT_MASK, 0);
-	snd_hdac_stream_writeb(azx_dev, SD_STS, SD_INT_MASK); /* to be sure */
-	if (azx_dev->stripe)
-		snd_hdac_stream_updateb(azx_dev, SD_CTL_3B, SD_CTL_STRIPE_MASK, 0);
+	struct hdac_bus *bus = azx_dev->bus;
+
+	if (bus->hygon_dword_access) {
+		snd_hdac_stream_updatel(azx_dev, SD_CTL,
+					SD_CTL_DMA_START | SD_INT_MASK, 0);
+		snd_hdac_stream_writeb(azx_dev, SD_STS, SD_INT_MASK); /* to be sure */
+		if (azx_dev->stripe)
+			snd_hdac_stream_updatel(azx_dev, SD_CTL_3B, SD_CTL_STRIPE_MASK, 0);
+	} else {
+		snd_hdac_stream_updateb(azx_dev, SD_CTL,
+					SD_CTL_DMA_START | SD_INT_MASK, 0);
+		snd_hdac_stream_writeb(azx_dev, SD_STS, SD_INT_MASK); /* to be sure */
+		if (azx_dev->stripe)
+			snd_hdac_stream_updateb(azx_dev, SD_CTL_3B, SD_CTL_STRIPE_MASK, 0);
+	}
+
 	azx_dev->running = false;
 }
 
@@ -225,12 +240,16 @@ void snd_hdac_stream_reset(struct hdac_stream *azx_dev)
 {
 	unsigned char val;
 	int dma_run_state;
+	struct hdac_bus *bus = azx_dev->bus;
 
 	snd_hdac_stream_clear(azx_dev);
 
 	dma_run_state = snd_hdac_stream_readb(azx_dev, SD_CTL) & SD_CTL_DMA_START;
 
-	snd_hdac_stream_updateb(azx_dev, SD_CTL, 0, SD_CTL_STREAM_RESET);
+	if (bus->hygon_dword_access)
+		snd_hdac_stream_updatel(azx_dev, SD_CTL, 0, SD_CTL_STREAM_RESET);
+	else
+		snd_hdac_stream_updateb(azx_dev, SD_CTL, 0, SD_CTL_STREAM_RESET);
 
 	/* wait for hardware to report that the stream entered reset */
 	snd_hdac_stream_readb_poll(azx_dev, SD_CTL, val, (val & SD_CTL_STREAM_RESET), 3, 300);
@@ -238,7 +257,10 @@ void snd_hdac_stream_reset(struct hdac_stream *azx_dev)
 	if (azx_dev->bus->dma_stop_delay && dma_run_state)
 		udelay(azx_dev->bus->dma_stop_delay);
 
-	snd_hdac_stream_updateb(azx_dev, SD_CTL, SD_CTL_STREAM_RESET, 0);
+	if (bus->hygon_dword_access)
+		snd_hdac_stream_updatel(azx_dev, SD_CTL, SD_CTL_STREAM_RESET, 0);
+	else
+		snd_hdac_stream_updateb(azx_dev, SD_CTL, SD_CTL_STREAM_RESET, 0);
 
 	/* wait for hardware to report that the stream is out of reset */
 	snd_hdac_stream_readb_poll(azx_dev, SD_CTL, val, !(val & SD_CTL_STREAM_RESET), 3, 300);
-- 
2.34.1
Re: [PATCH 7/8] ALSA: hda: Fix single byte writing issue for Hygon family 18h model 5h
Posted by David Laight 6 days, 3 hours ago
On Fri, 27 Mar 2026 16:11:13 +0800
Fu Hao <fuhao@open-hieco.net> wrote:

> On Hygon family 18h model 5h controller, some registers such as
> GCTL, SD_CTL and SD_CTL_3B should be accessed in dword, or the
> writing will fail.

What's a 'dword' ?
It is short for 'double word', a 64bit cpu has a word size of
64 bits, so a dword would be 128 bits.

On Hygon family 18h model 5h controller 32bit accesses must be
used to access registers such as CTL, SD_CTL and SD_CTL_3B.

	David
Re: [PATCH 7/8] ALSA: hda: Fix single byte writing issue for Hygon family 18h model 5h
Posted by Fu Hao 5 days, 14 hours ago
On 2026/3/27 21:02, David Laight wrote:
> On Fri, 27 Mar 2026 16:11:13 +0800
> Fu Hao <fuhao@open-hieco.net> wrote:
> 
>> On Hygon family 18h model 5h controller, some registers such as
>> GCTL, SD_CTL and SD_CTL_3B should be accessed in dword, or the
>> writing will fail.
> 
> What's a 'dword' ?
> It is short for 'double word', a 64bit cpu has a word size of
> 64 bits, so a dword would be 128 bits.
> 
> On Hygon family 18h model 5h controller 32bit accesses must be
> used to access registers such as CTL, SD_CTL and SD_CTL_3B.
> 
> 	David
> 

Accessing in dword alignment means performing register read and write
operations in 32-bit increments. In the current driver, some HDA
registers are accessed by default in byte increments. However, for
certain registers of the Hygon HD-audio controller, such as CTL, SD_CTL,
and SD_CTL_3B, they need to be accessed in 32-bit increments during
initialization, in accordance with the 32-bit nature of the registers
themselves.

If you access it using dword, the API snd_hdac_stream_updatel(writel,
32bit) will be used.

-- 
Regards,
Fu Hao
Re: [PATCH 7/8] ALSA: hda: Fix single byte writing issue for Hygon family 18h model 5h
Posted by Takashi Iwai 6 days, 4 hours ago
On Fri, 27 Mar 2026 09:11:13 +0100,
Fu Hao wrote:
> 
> On Hygon family 18h model 5h controller, some registers such as
> GCTL, SD_CTL and SD_CTL_3B should be accessed in dword, or the
> writing will fail.
> 
> Signed-off-by: Fu Hao <fuhao@open-hieco.net>
> ---
>  include/sound/hdaudio.h       |  1 +
>  sound/hda/controllers/intel.c |  4 ++++
>  sound/hda/core/controller.c   | 10 +++++++--
>  sound/hda/core/stream.c       | 42 ++++++++++++++++++++++++++---------
>  4 files changed, 45 insertions(+), 12 deletions(-)
> 
> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> index f11bfc6b9..57a144fec 100644
> --- a/include/sound/hdaudio.h
> +++ b/include/sound/hdaudio.h
> @@ -352,6 +352,7 @@ struct hdac_bus {
>  	bool not_use_interrupts:1;	/* prohibiting the RIRB IRQ */
>  	bool access_sdnctl_in_dword:1;	/* accessing the sdnctl register by dword */
>  	bool use_pio_for_commands:1;	/* Use PIO instead of CORB for commands */
> +	bool hygon_dword_access:1;
>  
>  	int poll_count;
>  

I hesitate to add another flag just for this platform.
And looking at the changes, all appear to be about the dword access
for sdnctl writes.  So I believe we can simply extend
access_sdnctl_in_dword to do the dword access in each write access,
instead?  The performance shouldn't matter as it's no hot path.

Alternatively, you can enable bus->aligned_mmio flag, but this needs
to enable CONFIG_SND_HDA_ALIGNED_MMIO.  That's a visible cost for
other platforms, though.


thanks,

Takashi
Re: [PATCH 7/8] ALSA: hda: Fix single byte writing issue for Hygon family 18h model 5h
Posted by Fu Hao 6 days, 4 hours ago
On 2026/3/27 20:11, Takashi Iwai wrote:
> On Fri, 27 Mar 2026 09:11:13 +0100,
> Fu Hao wrote:
>>
>> On Hygon family 18h model 5h controller, some registers such as
>> GCTL, SD_CTL and SD_CTL_3B should be accessed in dword, or the
>> writing will fail.
>>
>> Signed-off-by: Fu Hao <fuhao@open-hieco.net>
>> ---
>>   include/sound/hdaudio.h       |  1 +
>>   sound/hda/controllers/intel.c |  4 ++++
>>   sound/hda/core/controller.c   | 10 +++++++--
>>   sound/hda/core/stream.c       | 42 ++++++++++++++++++++++++++---------
>>   4 files changed, 45 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
>> index f11bfc6b9..57a144fec 100644
>> --- a/include/sound/hdaudio.h
>> +++ b/include/sound/hdaudio.h
>> @@ -352,6 +352,7 @@ struct hdac_bus {
>>   	bool not_use_interrupts:1;	/* prohibiting the RIRB IRQ */
>>   	bool access_sdnctl_in_dword:1;	/* accessing the sdnctl register by dword */
>>   	bool use_pio_for_commands:1;	/* Use PIO instead of CORB for commands */
>> +	bool hygon_dword_access:1;
>>   
>>   	int poll_count;
>>   
> 
> I hesitate to add another flag just for this platform.
> And looking at the changes, all appear to be about the dword access
> for sdnctl writes.  So I believe we can simply extend
> access_sdnctl_in_dword to do the dword access in each write access,
> instead?  The performance shouldn't matter as it's no hot path.
> 
> Alternatively, you can enable bus->aligned_mmio flag, but this needs
> to enable CONFIG_SND_HDA_ALIGNED_MMIO.  That's a visible cost for
> other platforms, though.
> 
> 
> thanks,
> 
> Takashi
> 

OK, thanks!I will try to use access_sdnctl_in_dword in
next patch series.

-- 
Regards,
Fu Hao