[PATCH] drm/radeon: bypass no_64bit_msi with new msi64 parameter

Han Gao posted 1 patch 1 month, 2 weeks ago
drivers/gpu/drm/radeon/radeon.h         | 1 +
drivers/gpu/drm/radeon/radeon_drv.c     | 4 ++++
drivers/gpu/drm/radeon/radeon_irq_kms.c | 2 +-
3 files changed, 6 insertions(+), 1 deletion(-)
[PATCH] drm/radeon: bypass no_64bit_msi with new msi64 parameter
Posted by Han Gao 1 month, 2 weeks ago
Sophgo SG2042's MSI driver lacks 32-bit MSI support.
Added a msi64 parameter to skip the limitation and force 64-bit MSI.

Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
---
 drivers/gpu/drm/radeon/radeon.h         | 1 +
 drivers/gpu/drm/radeon/radeon_drv.c     | 4 ++++
 drivers/gpu/drm/radeon/radeon_irq_kms.c | 2 +-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 527b9d19d730..7207e3156c28 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -122,6 +122,7 @@ extern int radeon_uvd;
 extern int radeon_vce;
 extern int radeon_si_support;
 extern int radeon_cik_support;
+extern int radeon_msi64;
 
 /*
  * Copy from radeon_drv.h so we don't have to include both and have conflicting
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 87fd6255c114..53af28494c03 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -249,6 +249,10 @@ int radeon_cik_support = -1;
 MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled, -1 = default)");
 module_param_named(cik_support, radeon_cik_support, int, 0444);
 
+int radeon_msi64;
+MODULE_PARM_DESC(msi64, "MSI64 support (1 = enabled, 0 = disabled)");
+module_param_named(msi64, radeon_msi64, int, 0444);
+
 static const struct pci_device_id pciidlist[] = {
 	radeon_PCI_IDS
 };
diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c
index 9961251b44ba..62eb5a6968ff 100644
--- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
@@ -250,7 +250,7 @@ static bool radeon_msi_ok(struct radeon_device *rdev)
 	 * of address for "64-bit" MSIs which breaks on some platforms, notably
 	 * IBM POWER servers, so we limit them
 	 */
-	if (rdev->family < CHIP_BONAIRE) {
+	if (rdev->family < CHIP_BONAIRE && !radeon_msi64) {
 		dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
 		rdev->pdev->no_64bit_msi = 1;
 	}
-- 
2.47.3
Re: [PATCH] drm/radeon: bypass no_64bit_msi with new msi64 parameter
Posted by Arnd Bergmann 1 month, 2 weeks ago
On Sat, Dec 20, 2025, at 17:33, Han Gao wrote:
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 87fd6255c114..53af28494c03 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -249,6 +249,10 @@ int radeon_cik_support = -1;
>  MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled, 
> -1 = default)");
>  module_param_named(cik_support, radeon_cik_support, int, 0444);
> 
> +int radeon_msi64;
> +MODULE_PARM_DESC(msi64, "MSI64 support (1 = enabled, 0 = disabled)");
> +module_param_named(msi64, radeon_msi64, int, 0444);
> +

As with the hda-intel patch, this should not be a module argument,
but we should have the kernel figure out what to do itself.

> diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c 
> b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> index 9961251b44ba..62eb5a6968ff 100644
> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> @@ -250,7 +250,7 @@ static bool radeon_msi_ok(struct radeon_device 
> *rdev)
>  	 * of address for "64-bit" MSIs which breaks on some platforms, 
> notably
>  	 * IBM POWER servers, so we limit them
>  	 */
> -	if (rdev->family < CHIP_BONAIRE) {
> +	if (rdev->family < CHIP_BONAIRE && !radeon_msi64) {
>  		dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
>  		rdev->pdev->no_64bit_msi = 1;

According to the comment above it, the device can apparently
do 40-bit addressing but not use the entire 64-bit space.

I assume the SG2042 chip has the irqchip somewhere above the
32-bit line but below the 40-bit line, so it ends up working.

I wonder if the msi_verify_entries() function should check
against dev->coherent_dma_mask instead of checking the
upper 32 bits for being nonzero, that probably gives you
the desired behavior.

     Arnd
Re: [PATCH] drm/radeon: bypass no_64bit_msi with new msi64 parameter
Posted by Christian König 1 month, 2 weeks ago
On 12/22/25 22:32, Arnd Bergmann wrote:
> On Sat, Dec 20, 2025, at 17:33, Han Gao wrote:
>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
>> b/drivers/gpu/drm/radeon/radeon_drv.c
>> index 87fd6255c114..53af28494c03 100644
>> --- a/drivers/gpu/drm/radeon/radeon_drv.c
>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
>> @@ -249,6 +249,10 @@ int radeon_cik_support = -1;
>>  MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled, 
>> -1 = default)");
>>  module_param_named(cik_support, radeon_cik_support, int, 0444);
>>
>> +int radeon_msi64;
>> +MODULE_PARM_DESC(msi64, "MSI64 support (1 = enabled, 0 = disabled)");
>> +module_param_named(msi64, radeon_msi64, int, 0444);
>> +
> 
> As with the hda-intel patch, this should not be a module argument,
> but we should have the kernel figure out what to do itself.

Yeah, completely agree. This is basically just a workaround (and a bit ugly one).

>> diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c 
>> b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> index 9961251b44ba..62eb5a6968ff 100644
>> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> @@ -250,7 +250,7 @@ static bool radeon_msi_ok(struct radeon_device 
>> *rdev)
>>  	 * of address for "64-bit" MSIs which breaks on some platforms, 
>> notably
>>  	 * IBM POWER servers, so we limit them
>>  	 */
>> -	if (rdev->family < CHIP_BONAIRE) {
>> +	if (rdev->family < CHIP_BONAIRE && !radeon_msi64) {
>>  		dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
>>  		rdev->pdev->no_64bit_msi = 1;
> 
> According to the comment above it, the device can apparently
> do 40-bit addressing but not use the entire 64-bit space.
> 
> I assume the SG2042 chip has the irqchip somewhere above the
> 32-bit line but below the 40-bit line, so it ends up working.
> 
> I wonder if the msi_verify_entries() function should check
> against dev->coherent_dma_mask instead of checking the
> upper 32 bits for being nonzero, that probably gives you
> the desired behavior.

Again completely agree, that sounds like a plan to me.

IIRC the modified code here is basically just a workaround because the MSI control dword on older radeon HW was not setup correctly.

Regards,
Christian.

> 
>      Arnd
Re: [PATCH] drm/radeon: bypass no_64bit_msi with new msi64 parameter
Posted by Vivian Wang 1 month, 2 weeks ago
Hi Christian,

I have a question about this 40-bit restriction.

On 12/23/25 22:55, Christian König wrote:
> On 12/22/25 22:32, Arnd Bergmann wrote:
>> On Sat, Dec 20, 2025, at 17:33, Han Gao wrote:
>> [...]
>>> diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c 
>>> b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>>> index 9961251b44ba..62eb5a6968ff 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>>> @@ -250,7 +250,7 @@ static bool radeon_msi_ok(struct radeon_device 
>>> *rdev)
>>>  	 * of address for "64-bit" MSIs which breaks on some platforms, 
>>> notably
>>>  	 * IBM POWER servers, so we limit them
>>>  	 */
>>> -	if (rdev->family < CHIP_BONAIRE) {
>>> +	if (rdev->family < CHIP_BONAIRE && !radeon_msi64) {
>>>  		dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
>>>  		rdev->pdev->no_64bit_msi = 1;
>> According to the comment above it, the device can apparently
>> do 40-bit addressing but not use the entire 64-bit space.
>>
>> I assume the SG2042 chip has the irqchip somewhere above the
>> 32-bit line but below the 40-bit line, so it ends up working.
>>
>> I wonder if the msi_verify_entries() function should check
>> against dev->coherent_dma_mask instead of checking the
>> upper 32 bits for being nonzero, that probably gives you
>> the desired behavior.
> Again completely agree, that sounds like a plan to me.
>
> IIRC the modified code here is basically just a workaround because the MSI control dword on older radeon HW was not setup correctly.

Does this mean that on Bonaire and onwards, MSI can reach full 64-bit
space, while DMA still only does 40-bit?
(drivers/gpu/drm/radeon/radeon_device.c sets DMA mask to at most 40 bits.)

If so, checking coherent_dma_mask would be wrong for those devices.

Do you think maybe it would be safer to introduce a msi_addr_mask for
occasions like these? We can have msi_addr_mask = DMA_BIT_MASK(40) for
pre-Bonaire, and then the ppc PCI stuff can check the mask and see if
it's usable. Probably something similar for hda.

Vivian "dramforever" Wang

Re: [PATCH] drm/radeon: bypass no_64bit_msi with new msi64 parameter
Posted by Christian König 1 month ago
Hi Vivian,

adding Bjorn as well.

On 12/23/25 16:31, Vivian Wang wrote:
> Hi Christian,
> 
> I have a question about this 40-bit restriction.
> 
> On 12/23/25 22:55, Christian König wrote:
>> On 12/22/25 22:32, Arnd Bergmann wrote:
>>> On Sat, Dec 20, 2025, at 17:33, Han Gao wrote:
>>> [...]
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c 
>>>> b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>>>> index 9961251b44ba..62eb5a6968ff 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>>>> @@ -250,7 +250,7 @@ static bool radeon_msi_ok(struct radeon_device 
>>>> *rdev)
>>>>  	 * of address for "64-bit" MSIs which breaks on some platforms, 
>>>> notably
>>>>  	 * IBM POWER servers, so we limit them
>>>>  	 */
>>>> -	if (rdev->family < CHIP_BONAIRE) {
>>>> +	if (rdev->family < CHIP_BONAIRE && !radeon_msi64) {
>>>>  		dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
>>>>  		rdev->pdev->no_64bit_msi = 1;
>>> According to the comment above it, the device can apparently
>>> do 40-bit addressing but not use the entire 64-bit space.
>>>
>>> I assume the SG2042 chip has the irqchip somewhere above the
>>> 32-bit line but below the 40-bit line, so it ends up working.
>>>
>>> I wonder if the msi_verify_entries() function should check
>>> against dev->coherent_dma_mask instead of checking the
>>> upper 32 bits for being nonzero, that probably gives you
>>> the desired behavior.
>> Again completely agree, that sounds like a plan to me.
>>
>> IIRC the modified code here is basically just a workaround because the MSI control dword on older radeon HW was not setup correctly.
> 
> Does this mean that on Bonaire and onwards, MSI can reach full 64-bit
> space, while DMA still only does 40-bit?
> (drivers/gpu/drm/radeon/radeon_device.c sets DMA mask to at most 40 bits.)

I need to double check with the HW guys and/or documentation, but I don't think so.

As far as I know the bus interface of the HW can only handle 40bits of address space. Later HW generations extend that to 44 or 48bits, but never the full 64bits.

I could be that the interrupt handler block for the MSI functionality has a special handling, but I strongly doubt that.

> 
> If so, checking coherent_dma_mask would be wrong for those devices.
> 
> Do you think maybe it would be safer to introduce a msi_addr_mask for
> occasions like these? We can have msi_addr_mask = DMA_BIT_MASK(40) for
> pre-Bonaire, and then the ppc PCI stuff can check the mask and see if
> it's usable. Probably something similar for hda.

That sounds like it would be rather clean, but it might be overkill.

As far as I know we have exactly one PCIe device (the pre-Bonaire HW generation) which messed up the bit in the MSI descriptor and so can only do 32bit MSI while the rest of the device can do 64bit accesses.

A good part of the confusion is comes because the PCIe spec is a bit unspecific what that 64bit support actually means. The original intent was probably to indicate 64bit address space support to operating systems.

But in reality devices only indicate that they can issue 64bit read and write requests but can only handle 40/44/48 or 57 bit addresses.

It's a bit messed up but that's what it is.

Regards,
Christian.

> 
> Vivian "dramforever" Wang
>