On Tue, 14 Dec 2021, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote: > On Tue, Dec 14, 2021 at 11:42:41AM +0200, Jani Nikula wrote: >>On Fri, 17 Sep 2021, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote: >>> From: Clint Taylor <clinton.a.taylor@xxxxxxxxx> >>> >>> Read OPROM SPI through MMIO and find VBT entry since we can't use >>> OpRegion and PCI mapping may not work on some systems due to most BIOSes >>> not leaving the Option ROM mapped. >> >>What happened here, still not merged? :o > > I don't understand neither. I got nacks, because of the other approach > to get the VBT from opregion. In that case reading via spi > controller directly would not be needed. However the other approach is > still not applied and meanwhile DG1 and DG2 have to fallback to our fake > vbt. > > So I actually think we should go ahead and just merge this. Agreed. This has been posted a few times with an accompanying "drm/i915/oprom: Basic sanitization" patch [1]. I don't like the idea of posting a series with one patch adding a function and the next one completely rewriting the same function. However, cleanup of that combo has not happened, and IIUC as a standalone patch this moves things forward and does no harm. This seems to still apply fine. I've hit the retest button to get current test results, and I suggest we merge this, and let's iterate from there. BR, Jani. [1] https://lore.kernel.org/all/20210412090526.30547-15-matthew.auld@xxxxxxxxx/ > > Lucas De Marchi > >> >>BR, >>Jani. >> >> >> >>> >>> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >>> Cc: Tomas Winkler <tomas.winkler@xxxxxxxxx> >>> Signed-off-by: Clint Taylor <clinton.a.taylor@xxxxxxxxx> >>> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> >>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >>> --- >>> drivers/gpu/drm/i915/display/intel_bios.c | 80 +++++++++++++++++++++-- >>> drivers/gpu/drm/i915/i915_reg.h | 8 +++ >>> 2 files changed, 82 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c >>> index 3c25926092de..7f179dbdec1b 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_bios.c >>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c >>> @@ -2280,6 +2280,66 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size) >>> return vbt; >>> } >>> >>> +static struct vbt_header *spi_oprom_get_vbt(struct drm_i915_private *i915) >>> +{ >>> + u32 count, data, found, store = 0; >>> + u32 static_region, oprom_offset; >>> + u32 oprom_size = 0x200000; >>> + u16 vbt_size; >>> + u32 *vbt; >>> + >>> + static_region = intel_uncore_read(&i915->uncore, SPI_STATIC_REGIONS); >>> + static_region &= OPTIONROM_SPI_REGIONID_MASK; >>> + intel_uncore_write(&i915->uncore, PRIMARY_SPI_REGIONID, static_region); >>> + >>> + oprom_offset = intel_uncore_read(&i915->uncore, OROM_OFFSET); >>> + oprom_offset &= OROM_OFFSET_MASK; >>> + >>> + for (count = 0; count < oprom_size; count += 4) { >>> + intel_uncore_write(&i915->uncore, PRIMARY_SPI_ADDRESS, oprom_offset + count); >>> + data = intel_uncore_read(&i915->uncore, PRIMARY_SPI_TRIGGER); >>> + >>> + if (data == *((const u32 *)"$VBT")) { >>> + found = oprom_offset + count; >>> + break; >>> + } >>> + } >>> + >>> + if (count >= oprom_size) >>> + goto err_not_found; >>> + >>> + /* Get VBT size and allocate space for the VBT */ >>> + intel_uncore_write(&i915->uncore, PRIMARY_SPI_ADDRESS, found + >>> + offsetof(struct vbt_header, vbt_size)); >>> + vbt_size = intel_uncore_read(&i915->uncore, PRIMARY_SPI_TRIGGER); >>> + vbt_size &= 0xffff; >>> + >>> + vbt = kzalloc(vbt_size, GFP_KERNEL); >>> + if (!vbt) { >>> + drm_err(&i915->drm, "Unable to allocate %u bytes for VBT storage\n", >>> + vbt_size); >>> + goto err_not_found; >>> + } >>> + >>> + for (count = 0; count < vbt_size; count += 4) { >>> + intel_uncore_write(&i915->uncore, PRIMARY_SPI_ADDRESS, found + count); >>> + data = intel_uncore_read(&i915->uncore, PRIMARY_SPI_TRIGGER); >>> + *(vbt + store++) = data; >>> + } >>> + >>> + if (!intel_bios_is_valid_vbt(vbt, vbt_size)) >>> + goto err_free_vbt; >>> + >>> + drm_dbg_kms(&i915->drm, "Found valid VBT in SPI flash\n"); >>> + >>> + return (struct vbt_header *)vbt; >>> + >>> +err_free_vbt: >>> + kfree(vbt); >>> +err_not_found: >>> + return NULL; >>> +} >>> + >>> static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915) >>> { >>> struct pci_dev *pdev = to_pci_dev(i915->drm.dev); >>> @@ -2329,6 +2389,8 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915) >>> >>> pci_unmap_rom(pdev, oprom); >>> >>> + drm_dbg_kms(&i915->drm, "Found valid VBT in PCI ROM\n"); >>> + >>> return vbt; >>> >>> err_free_vbt: >>> @@ -2363,17 +2425,23 @@ void intel_bios_init(struct drm_i915_private *i915) >>> >>> init_vbt_defaults(i915); >>> >>> - /* If the OpRegion does not have VBT, look in PCI ROM. */ >>> + /* >>> + * If the OpRegion does not have VBT, look in SPI flash through MMIO or >>> + * PCI mapping >>> + */ >>> + if (!vbt && IS_DGFX(i915)) { >>> + oprom_vbt = spi_oprom_get_vbt(i915); >>> + vbt = oprom_vbt; >>> + } >>> + >>> if (!vbt) { >>> oprom_vbt = oprom_get_vbt(i915); >>> - if (!oprom_vbt) >>> - goto out; >>> - >>> vbt = oprom_vbt; >>> - >>> - drm_dbg_kms(&i915->drm, "Found valid VBT in PCI ROM\n"); >>> } >>> >>> + if (!vbt) >>> + goto out; >>> + >>> bdb = get_bdb_header(vbt); >>> i915->vbt.version = bdb->version; >>> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >>> index c3a21f7c003d..fd3fee090412 100644 >>> --- a/drivers/gpu/drm/i915/i915_reg.h >>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>> @@ -12771,6 +12771,14 @@ enum skl_power_gate { >>> #define TCSS_DDI_STATUS_HPD_LIVE_STATUS_TBT REG_BIT(1) >>> #define TCSS_DDI_STATUS_HPD_LIVE_STATUS_ALT REG_BIT(0) >>> >>> +#define PRIMARY_SPI_TRIGGER _MMIO(0x102040) >>> +#define PRIMARY_SPI_ADDRESS _MMIO(0x102080) >>> +#define PRIMARY_SPI_REGIONID _MMIO(0x102084) >>> +#define SPI_STATIC_REGIONS _MMIO(0x102090) >>> +#define OPTIONROM_SPI_REGIONID_MASK REG_GENMASK(7, 0) >>> +#define OROM_OFFSET _MMIO(0x1020c0) >>> +#define OROM_OFFSET_MASK REG_GENMASK(20, 16) >>> + >>> /* This register controls the Display State Buffer (DSB) engines. */ >>> #define _DSBSL_INSTANCE_BASE 0x70B00 >>> #define DSBSL_INSTANCE(pipe, id) (_DSBSL_INSTANCE_BASE + \ >> >>-- >>Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center
