[PATCH 0/2] drm/radeon: Sun XVR-300 support on sparc64

[PATCH 0/2] drm/radeon: Sun XVR-300 support on sparc64

[Imre Kaloz]

These two patches get radeon KMS working on the Sun XVR-300, a
FireMV 2200 PCIe (RV370) board, in sparc64 hosts.

Patch 1 fixes an Oops that any posted radeon card on sparc64 hits in
igp_read_bios_from_vram(), which dereferences an ioremap() cookie to
check the VBIOS signature. It is tagged for stable.

Patch 2 adds a legacy connector table for the XVR-300, which has no
COMBIOS for the driver to parse, so both DVI-I outputs are found.
The XVR-300 needs both.

Tested on a Sun Ultra 45 with an XVR-300.

Imre Kaloz (2):
  drm/radeon: Check the VBIOS signature after copying it from VRAM
  drm/radeon: add a connector table for the Sun XVR-300

 drivers/gpu/drm/radeon/radeon_bios.c    | 10 +++--
 drivers/gpu/drm/radeon/radeon_combios.c | 51 +++++++++++++++++++++++++
 drivers/gpu/drm/radeon/radeon_mode.h    |  3 +-
 3 files changed, 59 insertions(+), 5 deletions(-)


base-commit: fe2ec83746e501645709761605c2464a44fd2929
-- 
2.47.3

[PATCH 1/2] drm/radeon: Check the VBIOS signature after copying it from VRAM

[Imre Kaloz]

On sparc64 a plain load through an ioremap() cookie is not a valid
access and faults; igp_read_bios_from_vram() does exactly that to check
the VBIOS signature, so radeon KMS never gets past this BIOS-fetch
attempt. Check the memcpy_fromio() copy instead, matching the file's
other paths.

Fixes: b442962a9e82 ("drm/radeon/kms: add support for "Surround View"")
Cc: stable@vger.kernel.org
Signed-off-by: Imre Kaloz <kaloz@kernel.org>
---
 drivers/gpu/drm/radeon/radeon_bios.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index 8595f4c6e2e5..a8367e481182 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -63,10 +63,6 @@ static bool igp_read_bios_from_vram(struct radeon_device *rdev)
 		return false;
 	}
 
-	if (size == 0 || bios[0] != 0x55 || bios[1] != 0xaa) {
-		iounmap(bios);
-		return false;
-	}
 	rdev->bios = kmalloc(size, GFP_KERNEL);
 	if (rdev->bios == NULL) {
 		iounmap(bios);
@@ -74,6 +70,12 @@ static bool igp_read_bios_from_vram(struct radeon_device *rdev)
 	}
 	memcpy_fromio(rdev->bios, bios, size);
 	iounmap(bios);
+
+	if (size == 0 || rdev->bios[0] != 0x55 || rdev->bios[1] != 0xaa) {
+		kfree(rdev->bios);
+		rdev->bios = NULL;
+		return false;
+	}
 	return true;
 }
 
-- 
2.47.3

[PATCH 2/2] drm/radeon: add a connector table for the Sun XVR-300

[Imre Kaloz]

The Sun XVR-300 is a FireMV 2200 PCIe (RV370) board with an
FCode-only Open Firmware ROM and no COMBIOS to parse, so the
generic connector table leaves both DVI-I outputs undetected. Both
XVR-300 SKUs report device ID 5b64 with an ATI subsystem ID rather
than Sun's, so match the OF node name prefix instead. The table
comes from the board's FCode.

Signed-off-by: Imre Kaloz <kaloz@kernel.org>
---
 drivers/gpu/drm/radeon/radeon_combios.c | 51 +++++++++++++++++++++++++
 drivers/gpu/drm/radeon/radeon_mode.h    |  3 +-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_combios.c b/drivers/gpu/drm/radeon/radeon_combios.c
index b3e7a747f11e..096f953c67be 100644
--- a/drivers/gpu/drm/radeon/radeon_combios.c
+++ b/drivers/gpu/drm/radeon/radeon_combios.c
@@ -25,6 +25,7 @@
  *          Alex Deucher
  */
 
+#include <linux/of.h>
 #include <linux/pci.h>
 
 #include <drm/drm_device.h>
@@ -1512,6 +1513,14 @@ bool radeon_get_legacy_connector_info_from_table(struct drm_device *dev)
 			rdev->mode_info.connector_table = CT_RN50_POWER;
 		else
 #endif
+		if (of_node_name_prefix(rdev->pdev->dev.of_node, "SUNW,XVR-300"))
+			/* FireMV 2200 PCIe with FCode-only ROM, no COMBIOS
+			 * to parse; both XVR-300 SKUs share device ID 5b64
+			 * with an ATI subsystem ID, so match the OF node
+			 * name instead.
+			 */
+			rdev->mode_info.connector_table = CT_SUN_XVR300;
+		else
 			rdev->mode_info.connector_table = CT_GENERIC;
 	}
 
@@ -2187,6 +2196,48 @@ bool radeon_get_legacy_connector_info_from_table(struct drm_device *dev)
 					    CONNECTOR_OBJECT_ID_SVIDEO,
 					    &hpd);
 		break;
+	case CT_SUN_XVR300:
+		DRM_INFO("Connector Table: %d (sun xvr-300)\n",
+			 rdev->mode_info.connector_table);
+		/* DVI-I - internal tmds, tv dac */
+		ddc_i2c = combios_setup_i2c_bus(rdev, DDC_DVI, 0, 0);
+		hpd.hpd = RADEON_HPD_1;
+		radeon_add_legacy_encoder(dev,
+					  radeon_get_encoder_enum(dev,
+								ATOM_DEVICE_DFP1_SUPPORT,
+								0),
+					  ATOM_DEVICE_DFP1_SUPPORT);
+		radeon_add_legacy_encoder(dev,
+					  radeon_get_encoder_enum(dev,
+								ATOM_DEVICE_CRT2_SUPPORT,
+								2),
+					  ATOM_DEVICE_CRT2_SUPPORT);
+		radeon_add_legacy_connector(dev, 0,
+					    ATOM_DEVICE_DFP1_SUPPORT |
+					    ATOM_DEVICE_CRT2_SUPPORT,
+					    DRM_MODE_CONNECTOR_DVII, &ddc_i2c,
+					    CONNECTOR_OBJECT_ID_SINGLE_LINK_DVI_I,
+					    &hpd);
+		/* DVI-I - sil164 dvo, primary dac */
+		ddc_i2c = combios_setup_i2c_bus(rdev, DDC_VGA, 0, 0);
+		hpd.hpd = RADEON_HPD_2;
+		radeon_add_legacy_encoder(dev,
+					  radeon_get_encoder_enum(dev,
+								ATOM_DEVICE_DFP2_SUPPORT,
+								0),
+					  ATOM_DEVICE_DFP2_SUPPORT);
+		radeon_add_legacy_encoder(dev,
+					  radeon_get_encoder_enum(dev,
+								ATOM_DEVICE_CRT1_SUPPORT,
+								1),
+					  ATOM_DEVICE_CRT1_SUPPORT);
+		radeon_add_legacy_connector(dev, 1,
+					    ATOM_DEVICE_DFP2_SUPPORT |
+					    ATOM_DEVICE_CRT1_SUPPORT,
+					    DRM_MODE_CONNECTOR_DVII, &ddc_i2c,
+					    CONNECTOR_OBJECT_ID_SINGLE_LINK_DVI_I,
+					    &hpd);
+		break;
 	default:
 		DRM_INFO("Connector table: %d (invalid)\n",
 			 rdev->mode_info.connector_table);
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 031a3bf6fe0a..6a16d821915d 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -226,7 +226,8 @@ enum radeon_connector_table {
 	CT_MAC_X800,
 	CT_MAC_G5_9600,
 	CT_SAM440EP,
-	CT_MAC_G4_SILVER
+	CT_MAC_G4_SILVER,
+	CT_SUN_XVR300
 };
 
 enum radeon_dvo_chip {
-- 
2.47.3

Re: [PATCH 1/2] drm/radeon: Check the VBIOS signature after copying it from VRAM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] radeon: inefficient 256KB MMIO copy and allocation before VBIOS signature check
--

commit e7cbdfb764010827fd7b2b511a029cf30846ddbe
Author: Imre Kaloz <kaloz@kernel.org>

drm/radeon: Check the VBIOS signature after copying it from VRAM

This commit modifies the VBIOS reading logic to check the signature after
copying the data from VRAM. This is done to avoid a sparc64 fault caused
by a plain load through an ioremap() cookie.

> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
> index 8595f4c6e2e5c..a8367e4811821 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -63,10 +63,6 @@ static bool igp_read_bios_from_vram(struct radeon_device *rdev)
>  		return false;
>  	}
>  
> -	if (size == 0 || bios[0] != 0x55 || bios[1] != 0xaa) {
> -		iounmap(bios);
> -		return false;
> -	}
>  	rdev->bios = kmalloc(size, GFP_KERNEL);
>  	if (rdev->bios == NULL) {
>  		iounmap(bios);
> @@ -74,6 +70,12 @@ static bool igp_read_bios_from_vram(struct radeon_device *rdev)
>  	}
>  	memcpy_fromio(rdev->bios, bios, size);
>  	iounmap(bios);
> +
> +	if (size == 0 || rdev->bios[0] != 0x55 || rdev->bios[1] != 0xaa) {

[Severity: Medium]
Does this sequence perform an expensive unconditional 256KB kmalloc() and
PCIe MMIO read (memcpy_fromio()) for discrete GPUs before validating the
VBIOS header signature?

During device probe, radeon_get_bios() tries multiple fallback methods and
calls igp_read_bios_from_vram(). This allows posted discrete GPUs to proceed.
The new logic maps VRAM, immediately allocates 256KB, copies it, and only
then checks if the first two bytes match the expected 0x55 and 0xaa
signature. This wastes allocation overhead and introduces a noticeable boot
delay, as reading 256KB of un-cached MMIO over PCIe takes significant time.

To fix the sparc64 __iomem access fault without creating a performance
regression, could this code use readb(&bios[0]) for the signature validation
before performing the 256KB allocation and copy, matching the pattern
established in radeon_read_bios()?

> +		kfree(rdev->bios);
> +		rdev->bios = NULL;
> +		return false;
> +	}
>  	return true;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927105555.8771-1-kaloz@kernel.org?part=1