Re: Performance drop due to alloc_workqueue() misuse and recent change
Hello, again. On Mon, Dec 04, 2023 at 04:03:47PM +, Naohiro Aota wrote: ... > In summary, we misuse max_active, considering it is a global limit. And, > the recent commit introduced a huge performance drop in some cases. We > need to review alloc_workqueue() usage to check if its max_active setting > is proper or not. Can you please test the following branch? https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git unbound-system-wide-max_active Thanks. -- tejun
Re: [PATCH] drm/i915: Piggyback opregion vbt to store vbt read from flash/oprom
On Tue, Dec 19, 2023 at 05:49:52PM -0800, Radhakrishna Sripada wrote: > Discrete cards do not have ACPI opregion. The vbt is stored in a special > flash accessible by the display controller. In order to access the vbt > in such cases, re-use the vbt, vbt_size fields in the opregion structure. Why? > > We should move away from storing the vbt in the opregion and store it, > if required in the vbt structure. > > Signed-off-by: Radhakrishna Sripada > --- > drivers/gpu/drm/i915/display/intel_bios.c | 44 ++- > drivers/gpu/drm/i915/display/intel_opregion.c | 7 ++- > 2 files changed, 29 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > b/drivers/gpu/drm/i915/display/intel_bios.c > index 736499a6e2c7..cbfbc56ff5b2 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -2930,12 +2930,11 @@ static u32 intel_spi_read(struct intel_uncore > *uncore, u32 offset) > return intel_uncore_read(uncore, PRIMARY_SPI_TRIGGER); > } > > -static struct vbt_header *spi_oprom_get_vbt(struct drm_i915_private *i915) > +static struct vbt_header *spi_oprom_get_vbt(struct drm_i915_private *i915, > u16 *vbt_size) > { > u32 count, data, found, store = 0; > u32 static_region, oprom_offset; > u32 oprom_size = 0x20; > - u16 vbt_size; > u32 *vbt; > > static_region = intel_uncore_read(>uncore, SPI_STATIC_REGIONS); > @@ -2957,18 +2956,18 @@ static struct vbt_header *spi_oprom_get_vbt(struct > drm_i915_private *i915) > goto err_not_found; > > /* Get VBT size and allocate space for the VBT */ > - vbt_size = intel_spi_read(>uncore, > + *vbt_size = intel_spi_read(>uncore, > found + offsetof(struct vbt_header, > vbt_size)); > - vbt_size &= 0x; > + *vbt_size &= 0x; > > - vbt = kzalloc(round_up(vbt_size, 4), GFP_KERNEL); > + vbt = kzalloc(round_up(*vbt_size, 4), GFP_KERNEL); > if (!vbt) > goto err_not_found; > > - for (count = 0; count < vbt_size; count += 4) > + for (count = 0; count < *vbt_size; count += 4) > *(vbt + store++) = intel_spi_read(>uncore, found + count); > > - if (!intel_bios_is_valid_vbt(vbt, vbt_size)) > + if (!intel_bios_is_valid_vbt(vbt, *vbt_size)) > goto err_free_vbt; > > drm_dbg_kms(>drm, "Found valid VBT in SPI flash\n"); > @@ -2977,16 +2976,16 @@ static struct vbt_header *spi_oprom_get_vbt(struct > drm_i915_private *i915) > > err_free_vbt: > kfree(vbt); > + *vbt_size = 0; > err_not_found: > return NULL; > } > > -static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915) > +static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915, u16 > *vbt_size) > { > struct pci_dev *pdev = to_pci_dev(i915->drm.dev); > void __iomem *p = NULL, *oprom; > struct vbt_header *vbt; > - u16 vbt_size; > size_t i, size; > > oprom = pci_map_rom(pdev, ); > @@ -3011,21 +3010,21 @@ static struct vbt_header *oprom_get_vbt(struct > drm_i915_private *i915) > goto err_unmap_oprom; > } > > - vbt_size = ioread16(p + offsetof(struct vbt_header, vbt_size)); > - if (vbt_size > size) { > + *vbt_size = ioread16(p + offsetof(struct vbt_header, vbt_size)); > + if (*vbt_size > size) { > drm_dbg(>drm, > - "VBT incomplete (vbt_size overflows)\n"); > + "VBT incomplete (*vbt_size overflows)\n"); > goto err_unmap_oprom; > } > > /* The rest will be validated by intel_bios_is_valid_vbt() */ > - vbt = kmalloc(vbt_size, GFP_KERNEL); > + vbt = kmalloc(*vbt_size, GFP_KERNEL); > if (!vbt) > goto err_unmap_oprom; > > - memcpy_fromio(vbt, p, vbt_size); > + memcpy_fromio(vbt, p, *vbt_size); > > - if (!intel_bios_is_valid_vbt(vbt, vbt_size)) > + if (!intel_bios_is_valid_vbt(vbt, *vbt_size)) > goto err_free_vbt; > > pci_unmap_rom(pdev, oprom); > @@ -3036,6 +3035,7 @@ static struct vbt_header *oprom_get_vbt(struct > drm_i915_private *i915) > > err_free_vbt: > kfree(vbt); > + *vbt_size = 0; > err_unmap_oprom: > pci_unmap_rom(pdev, oprom); > > @@ -3052,8 +3052,10 @@ static struct vbt_header *oprom_get_vbt(struct > drm_i915_private *i915) > */ > void intel_bios_init(struct drm_i915_private *i915) > { > + struct intel_opregion *opregion = >display.opregion; > const struct vbt_header *vbt = i915->display.opregion.vbt; > struct vbt_header *oprom_vbt = NULL; > + u16 vbt_size; > const struct bdb_header *bdb; > > INIT_LIST_HEAD(>display.vbt.display_devices); > @@ -3072,13 +3074,15 @@ void intel_bios_init(struct drm_i915_private *i915) >* PCI mapping >*/ > if (!vbt && IS_DGFX(i915)) { > -
[PATCH] drm/i915: Piggyback opregion vbt to store vbt read from flash/oprom
Discrete cards do not have ACPI opregion. The vbt is stored in a special flash accessible by the display controller. In order to access the vbt in such cases, re-use the vbt, vbt_size fields in the opregion structure. We should move away from storing the vbt in the opregion and store it, if required in the vbt structure. Signed-off-by: Radhakrishna Sripada --- drivers/gpu/drm/i915/display/intel_bios.c | 44 ++- drivers/gpu/drm/i915/display/intel_opregion.c | 7 ++- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index 736499a6e2c7..cbfbc56ff5b2 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -2930,12 +2930,11 @@ static u32 intel_spi_read(struct intel_uncore *uncore, u32 offset) return intel_uncore_read(uncore, PRIMARY_SPI_TRIGGER); } -static struct vbt_header *spi_oprom_get_vbt(struct drm_i915_private *i915) +static struct vbt_header *spi_oprom_get_vbt(struct drm_i915_private *i915, u16 *vbt_size) { u32 count, data, found, store = 0; u32 static_region, oprom_offset; u32 oprom_size = 0x20; - u16 vbt_size; u32 *vbt; static_region = intel_uncore_read(>uncore, SPI_STATIC_REGIONS); @@ -2957,18 +2956,18 @@ static struct vbt_header *spi_oprom_get_vbt(struct drm_i915_private *i915) goto err_not_found; /* Get VBT size and allocate space for the VBT */ - vbt_size = intel_spi_read(>uncore, + *vbt_size = intel_spi_read(>uncore, found + offsetof(struct vbt_header, vbt_size)); - vbt_size &= 0x; + *vbt_size &= 0x; - vbt = kzalloc(round_up(vbt_size, 4), GFP_KERNEL); + vbt = kzalloc(round_up(*vbt_size, 4), GFP_KERNEL); if (!vbt) goto err_not_found; - for (count = 0; count < vbt_size; count += 4) + for (count = 0; count < *vbt_size; count += 4) *(vbt + store++) = intel_spi_read(>uncore, found + count); - if (!intel_bios_is_valid_vbt(vbt, vbt_size)) + if (!intel_bios_is_valid_vbt(vbt, *vbt_size)) goto err_free_vbt; drm_dbg_kms(>drm, "Found valid VBT in SPI flash\n"); @@ -2977,16 +2976,16 @@ static struct vbt_header *spi_oprom_get_vbt(struct drm_i915_private *i915) err_free_vbt: kfree(vbt); + *vbt_size = 0; err_not_found: return NULL; } -static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915) +static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915, u16 *vbt_size) { struct pci_dev *pdev = to_pci_dev(i915->drm.dev); void __iomem *p = NULL, *oprom; struct vbt_header *vbt; - u16 vbt_size; size_t i, size; oprom = pci_map_rom(pdev, ); @@ -3011,21 +3010,21 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915) goto err_unmap_oprom; } - vbt_size = ioread16(p + offsetof(struct vbt_header, vbt_size)); - if (vbt_size > size) { + *vbt_size = ioread16(p + offsetof(struct vbt_header, vbt_size)); + if (*vbt_size > size) { drm_dbg(>drm, - "VBT incomplete (vbt_size overflows)\n"); + "VBT incomplete (*vbt_size overflows)\n"); goto err_unmap_oprom; } /* The rest will be validated by intel_bios_is_valid_vbt() */ - vbt = kmalloc(vbt_size, GFP_KERNEL); + vbt = kmalloc(*vbt_size, GFP_KERNEL); if (!vbt) goto err_unmap_oprom; - memcpy_fromio(vbt, p, vbt_size); + memcpy_fromio(vbt, p, *vbt_size); - if (!intel_bios_is_valid_vbt(vbt, vbt_size)) + if (!intel_bios_is_valid_vbt(vbt, *vbt_size)) goto err_free_vbt; pci_unmap_rom(pdev, oprom); @@ -3036,6 +3035,7 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915) err_free_vbt: kfree(vbt); + *vbt_size = 0; err_unmap_oprom: pci_unmap_rom(pdev, oprom); @@ -3052,8 +3052,10 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915) */ void intel_bios_init(struct drm_i915_private *i915) { + struct intel_opregion *opregion = >display.opregion; const struct vbt_header *vbt = i915->display.opregion.vbt; struct vbt_header *oprom_vbt = NULL; + u16 vbt_size; const struct bdb_header *bdb; INIT_LIST_HEAD(>display.vbt.display_devices); @@ -3072,13 +3074,15 @@ void intel_bios_init(struct drm_i915_private *i915) * PCI mapping */ if (!vbt && IS_DGFX(i915)) { - oprom_vbt = spi_oprom_get_vbt(i915); - vbt = oprom_vbt; + oprom_vbt = spi_oprom_get_vbt(i915, _size); + opregion->vbt = vbt = oprom_vbt; +
[PATCH 0/3] TC phy check cleanup
We are relying on end-less if-else ladders for a type-c phy capabilities check. Though it made sense when platforms supported legacy type-c support, modern platforms rely on the information passed by vbt. This cleanup restricts the if-else ladder to the platforms supporting legacy type-c phys and relies on vbt info for modern client and discrete platforms. Radhakrishna Sripada (3): drm/i915: Rename intel_bios_encoder_data_lookup as a port variant drm/i915: Introduce intel_encoder_phy_data_lookup drm/i915: Separate tc check for legacy and non legacy tc phys drivers/gpu/drm/i915/display/g4x_dp.c | 2 +- drivers/gpu/drm/i915/display/g4x_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_bios.c | 15 +- drivers/gpu/drm/i915/display/intel_bios.h | 5 +++- drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- drivers/gpu/drm/i915/display/intel_display.c | 29 --- .../drm/i915/display/intel_display_device.h | 1 + drivers/gpu/drm/i915/display/intel_dp.c | 2 +- 8 files changed, 42 insertions(+), 16 deletions(-) -- 2.34.1
[PATCH 1/3] drm/i915: Rename intel_bios_encoder_data_lookup as a port variant
intel_bios_encoder_data_lookup takes enum port as an argument. A variant based out of phy based lookup is required to be used out of vbt code which will be introduced in a later patch. Hence indicate the current variant as intel_bios_encoder_port_data_lookup. Signed-off-by: Radhakrishna Sripada --- drivers/gpu/drm/i915/display/g4x_dp.c | 2 +- drivers/gpu/drm/i915/display/g4x_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_bios.c | 2 +- drivers/gpu/drm/i915/display/intel_bios.h | 2 +- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c index dfe0b07a122d..ecfa55d6f9f5 100644 --- a/drivers/gpu/drm/i915/display/g4x_dp.c +++ b/drivers/gpu/drm/i915/display/g4x_dp.c @@ -1298,7 +1298,7 @@ bool g4x_dp_init(struct drm_i915_private *dev_priv, if (!assert_port_valid(dev_priv, port)) return false; - devdata = intel_bios_encoder_data_lookup(dev_priv, port); + devdata = intel_bios_encoder_port_data_lookup(dev_priv, port); /* FIXME bail? */ if (!devdata) diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c b/drivers/gpu/drm/i915/display/g4x_hdmi.c index 8096492b3fad..3aedbb6f13c8 100644 --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c @@ -697,7 +697,7 @@ void g4x_hdmi_init(struct drm_i915_private *dev_priv, if (!assert_hdmi_port_valid(dev_priv, port)) return; - devdata = intel_bios_encoder_data_lookup(dev_priv, port); + devdata = intel_bios_encoder_port_data_lookup(dev_priv, port); /* FIXME bail? */ if (!devdata) diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index aa169b0055e9..febc607267f0 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -3636,7 +3636,7 @@ bool intel_bios_encoder_hpd_invert(const struct intel_bios_encoder_data *devdata } const struct intel_bios_encoder_data * -intel_bios_encoder_data_lookup(struct drm_i915_private *i915, enum port port) +intel_bios_encoder_port_data_lookup(struct drm_i915_private *i915, enum port port) { struct intel_bios_encoder_data *devdata; diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h index 49e24b7cf675..a296aad7f545 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.h +++ b/drivers/gpu/drm/i915/display/intel_bios.h @@ -255,7 +255,7 @@ bool intel_bios_port_supports_typec_usb(struct drm_i915_private *i915, enum port bool intel_bios_port_supports_tbt(struct drm_i915_private *i915, enum port port); const struct intel_bios_encoder_data * -intel_bios_encoder_data_lookup(struct drm_i915_private *i915, enum port port); +intel_bios_encoder_port_data_lookup(struct drm_i915_private *i915, enum port port); bool intel_bios_encoder_supports_dvi(const struct intel_bios_encoder_data *devdata); bool intel_bios_encoder_supports_hdmi(const struct intel_bios_encoder_data *devdata); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 3dfca8d88a6f..76f1b8828df8 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -6155,7 +6155,7 @@ static bool _intel_dp_is_port_edp(struct drm_i915_private *dev_priv, bool intel_dp_is_port_edp(struct drm_i915_private *i915, enum port port) { const struct intel_bios_encoder_data *devdata = - intel_bios_encoder_data_lookup(i915, port); + intel_bios_encoder_port_data_lookup(i915, port); return _intel_dp_is_port_edp(i915, devdata, port); } -- 2.34.1
[PATCH 3/3] drm/i915: Separate tc check for legacy and non legacy tc phys
Starting MTL and DG2 if a phy is not marked as USB-typeC or TBT capable by vbt we should not consider it as a Legacy type-c phy. The concept of Legacy-tc existed in platforms from Icelake to Alder lake where an external FIA can be routed to one of the phy's thus making the phy tc capable without being marked in the vbt. Discrete cards have native ports and client products post MTL have a 1:1 mapping with type-C subsystem which is advertised by the bios. So rely on the vbt info regarding type-c capability. Signed-off-by: Radhakrishna Sripada --- drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- drivers/gpu/drm/i915/display/intel_display.c | 29 --- .../drm/i915/display/intel_display_device.h | 1 + 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 12a29363e5df..7d5b95f97d5f 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -5100,7 +5100,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, } if (intel_phy_is_tc(dev_priv, phy)) { - bool is_legacy = + bool is_legacy = HAS_LEGACY_TC(dev_priv) && !intel_bios_encoder_supports_typec_usb(devdata) && !intel_bios_encoder_supports_tbt(devdata); diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b10aad15a63d..03006c9af824 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1854,17 +1854,9 @@ bool intel_phy_is_combo(struct drm_i915_private *dev_priv, enum phy phy) return false; } -bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy phy) +static bool intel_phy_is_legacy_tc(struct drm_i915_private *dev_priv, enum phy phy) { - /* -* DG2's "TC1", although TC-capable output, doesn't share the same flow -* as other platforms on the display engine side and rather rely on the -* SNPS PHY, that is programmed separately -*/ - if (IS_DG2(dev_priv)) - return false; - - if (DISPLAY_VER(dev_priv) >= 13) + if (DISPLAY_VER(dev_priv) == 13) return phy >= PHY_F && phy <= PHY_I; else if (IS_TIGERLAKE(dev_priv)) return phy >= PHY_D && phy <= PHY_I; @@ -1874,6 +1866,23 @@ bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy phy) return false; } +static bool intel_phy_is_vbt_tc(struct drm_i915_private *dev_priv, enum phy phy) +{ + const struct intel_bios_encoder_data *data = + intel_bios_encoder_phy_data_lookup(dev_priv, phy); + + return intel_bios_encoder_supports_typec_usb(data) && + intel_bios_encoder_supports_tbt(data); +} + +bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy phy) +{ + if (!HAS_LEGACY_TC(dev_priv)) + return intel_phy_is_vbt_tc(dev_priv, phy); + else + return intel_phy_is_legacy_tc(dev_priv, phy); +} + bool intel_phy_is_snps(struct drm_i915_private *dev_priv, enum phy phy) { /* diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h index fe4268813786..9450e263c873 100644 --- a/drivers/gpu/drm/i915/display/intel_display_device.h +++ b/drivers/gpu/drm/i915/display/intel_display_device.h @@ -58,6 +58,7 @@ struct drm_printer; #define HAS_IPS(i915) (IS_HASWELL_ULT(i915) || IS_BROADWELL(i915)) #define HAS_LRR(i915) (DISPLAY_VER(i915) >= 12) #define HAS_LSPCON(i915) (IS_DISPLAY_VER(i915, 9, 10)) +#define HAS_LEGACY_TC(i915)(IS_DISPLAY_VER(i915, 11, 13) && !IS_DGFX(i915)) #define HAS_MBUS_JOINING(i915) (IS_ALDERLAKE_P(i915) || DISPLAY_VER(i915) >= 14) #define HAS_MSO(i915) (DISPLAY_VER(i915) >= 12) #define HAS_OVERLAY(i915) (DISPLAY_INFO(i915)->has_overlay) -- 2.34.1
[PATCH 2/3] drm/i915: Introduce intel_encoder_phy_data_lookup
This patch introduces phy version of intel_encoder_port_data_lookup. Port based variant is dependent on vbt child data extraction and conversion to port data to be used further. Port data is not immediately available and is difficult to be determined from phy info. Signed-off-by: Radhakrishna Sripada --- drivers/gpu/drm/i915/display/intel_bios.c | 13 + drivers/gpu/drm/i915/display/intel_bios.h | 3 +++ 2 files changed, 16 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index febc607267f0..0b21364b2bdf 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -3648,6 +3648,19 @@ intel_bios_encoder_port_data_lookup(struct drm_i915_private *i915, enum port por return NULL; } +const struct intel_bios_encoder_data * +intel_bios_encoder_phy_data_lookup(struct drm_i915_private *i915, enum phy phy) +{ + struct intel_bios_encoder_data *devdata; + + list_for_each_entry(devdata, >display.vbt.display_devices, node) { + if (intel_port_to_phy(i915, intel_bios_encoder_port(devdata)) == phy) + return devdata; + } + + return NULL; +} + void intel_bios_for_each_encoder(struct drm_i915_private *i915, void (*func)(struct drm_i915_private *i915, const struct intel_bios_encoder_data *devdata)) diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h index a296aad7f545..2861ebb13909 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.h +++ b/drivers/gpu/drm/i915/display/intel_bios.h @@ -39,6 +39,7 @@ struct intel_crtc_state; struct intel_encoder; struct intel_panel; enum aux_ch; +enum phy; enum port; enum intel_backlight_type { @@ -254,6 +255,8 @@ bool intel_bios_get_dsc_params(struct intel_encoder *encoder, bool intel_bios_port_supports_typec_usb(struct drm_i915_private *i915, enum port port); bool intel_bios_port_supports_tbt(struct drm_i915_private *i915, enum port port); +const struct intel_bios_encoder_data * +intel_bios_encoder_phy_data_lookup(struct drm_i915_private *i915, enum phy phy); const struct intel_bios_encoder_data * intel_bios_encoder_port_data_lookup(struct drm_i915_private *i915, enum port port); -- 2.34.1
[PATCH] drm/i915/guc: Avoid circular locking issue on busyness flush
From: John Harrison Avoid the following lockdep complaint: <4> [298.856498] == <4> [298.856500] WARNING: possible circular locking dependency detected <4> [298.856503] 6.7.0-rc5-CI_DRM_14017-g58ac4ffc75b6+ #1 Tainted: G N <4> [298.856505] -- <4> [298.856507] kworker/4:1H/190 is trying to acquire lock: <4> [298.856509] 8881103e9978 (>reset.backoff_srcu){}-{0:0}, at: _intel_gt_reset_lock+0x35/0x380 [i915] <4> [298.856661] but task is already holding lock: <4> [298.856663] c900013f7e58 ((work_completion)(&(>timestamp.work)->work)){+.+.}-{0:0}, at: process_scheduled_works+0x264/0x530 <4> [298.856671] which lock already depends on the new lock. The complaint is not actually valid. The busyness worker thread does indeed hold the worker lock and then attempt to acquire the reset lock (which may have happened in reverse order elsewhere). However, it does so with a trylock that exits if the reset lock is not available (specifically to prevent this and other similar deadlocks). Unfortunately, lockdep does not understand the trylock semantics (the lock is an i915 specific custom implementation for resets). Not doing a synchronous flush of the worker thread when a reset is in progress resolves the lockdep splat by never even attempting to grab the lock in this particular scenario. There are situatons where a synchronous cancel is required, however. So, always do the synchronous cancel if not in reset. And add an extra synchronous cancel to the end of the reset flow to account for when a reset is occurring at driver shutdown and the cancel is no longer synchronous but could lead to unallocated memory accesses if the worker is not stopped. Signed-off-by: Zhanjun Dong Signed-off-by: John Harrison Cc: Andi Shyti Cc: Daniel Vetter --- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 48 ++- drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 +- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index a259f1118c5ab..0228a77d456ed 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1362,7 +1362,45 @@ static void guc_enable_busyness_worker(struct intel_guc *guc) static void guc_cancel_busyness_worker(struct intel_guc *guc) { - cancel_delayed_work_sync(>timestamp.work); + /* +* There are many different call stacks that can get here. Some of them +* hold the reset mutex. The busyness worker also attempts to acquire the +* reset mutex. Synchronously flushing a worker thread requires acquiring +* the worker mutex. Lockdep sees this as a conflict. It thinks that the +* flush can deadlock because it holds the worker mutex while waiting for +* the reset mutex, but another thread is holding the reset mutex and might +* attempt to use other worker functions. +* +* In practice, this scenario does not exist because the busyness worker +* does not block waiting for the reset mutex. It does a try-lock on it and +* immediately exits if the lock is already held. Unfortunately, the mutex +* in question (I915_RESET_BACKOFF) is an i915 implementation which has lockdep +* annotation but not to the extent of explaining the 'might lock' is also a +* 'does not need to lock'. So one option would be to add more complex lockdep +* annotations to ignore the issue (if at all possible). A simpler option is to +* just not flush synchronously when a rest in progress. Given that the worker +* will just early exit and re-schedule itself anyway, there is no advantage +* to running it immediately. +* +* If a reset is not in progress, then the synchronous flush may be required. +* As noted many call stacks lead here, some during suspend and driver unload +* which do require a synchronous flush to make sure the worker is stopped +* before memory is freed. +* +* Trying to pass a 'need_sync' or 'in_reset' flag all the way down through +* every possible call stack is unfeasible. It would be too intrusive to many +* areas that really don't care about the GuC backend. However, there is the +* 'reset_in_progress' flag available, so just use that. +* +* And note that in the case of a reset occurring during driver unload +* (wedge_on_fini), skipping the cancel in _prepare (when the reset flag is set +* is fine because there is another cancel in _finish (when the reset flag is +* not). +*/ + if (guc_to_gt(guc)->uc.reset_in_progress) + cancel_delayed_work(>timestamp.work); + else +
✓ Fi.CI.BAT: success for drm/i915/mtl: Add fake PCH for Meteor Lake (rev2)
== Series Details == Series: drm/i915/mtl: Add fake PCH for Meteor Lake (rev2) URL : https://patchwork.freedesktop.org/series/127963/ State : success == Summary == CI Bug Log - changes from CI_DRM_14045 -> Patchwork_127963v2 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v2/index.html Participating hosts (38 -> 37) -- Additional (1): fi-pnv-d510 Missing(2): bat-rpls-2 fi-snb-2520m Known issues Here are the changes found in Patchwork_127963v2 that come from known issues: ### IGT changes ### Issues hit * igt@gem_lmem_swapping@basic: - fi-pnv-d510:NOTRUN -> [SKIP][1] ([fdo#109271]) +28 other tests skip [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v2/fi-pnv-d510/igt@gem_lmem_swapp...@basic.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 Build changes - * Linux: CI_DRM_14045 -> Patchwork_127963v2 CI-20190529: 20190529 CI_DRM_14045: e786d8c6347b8b304e82c6bcebc0d38401792b16 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7648: 5c7b44f13c9c5bc15af0cb2b6a5ea10a8a468ac0 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_127963v2: e786d8c6347b8b304e82c6bcebc0d38401792b16 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 22a222ed2423 drm/i915/mtl: Add fake PCH for Meteor Lake == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v2/index.html
✗ Fi.CI.SPARSE: warning for drm/i915/mtl: Add fake PCH for Meteor Lake (rev2)
== Series Details == Series: drm/i915/mtl: Add fake PCH for Meteor Lake (rev2) URL : https://patchwork.freedesktop.org/series/127963/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
Re: [PATCH v4 1/3] drm/i915/dp: Use LINK_QUAL_PATTERN_* Phy test pattern names
On Wed, 13 Dec 2023, Khaled Almahallawy wrote: > Starting from DP2.0 specs, DPCD 248h is renamed > LINK_QUAL_PATTERN_SELECT and it has the same values of registers > DPCD 10Bh-10Eh. > Use the PHY pattern names defined for DPCD 10Bh-10Eh in order to add > CP2520 Pattern 3 (TPS4) phy pattern support in the next > patch of this series and DP2.1 PHY patterns for future series. > > v2: rebase > > Cc: Jani Nikula > Cc: Imre Deak > Cc: Lee Shawn C > Signed-off-by: Khaled Almahallawy > Reviewed-by: Jani Nikula All three pushed to drm-intel-next, thanks for the patches, and sorry it took so long. BR, Jani. > --- > drivers/gpu/drm/i915/display/intel_dp.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 3b2482bf683f..a1e63ab5761b 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4683,27 +4683,27 @@ static void intel_dp_phy_pattern_update(struct > intel_dp *intel_dp, > u32 pattern_val; > > switch (data->phy_pattern) { > - case DP_PHY_TEST_PATTERN_NONE: > + case DP_LINK_QUAL_PATTERN_DISABLE: > drm_dbg_kms(_priv->drm, "Disable Phy Test Pattern\n"); > intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), 0x0); > break; > - case DP_PHY_TEST_PATTERN_D10_2: > + case DP_LINK_QUAL_PATTERN_D10_2: > drm_dbg_kms(_priv->drm, "Set D10.2 Phy Test Pattern\n"); > intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), > DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2); > break; > - case DP_PHY_TEST_PATTERN_ERROR_COUNT: > + case DP_LINK_QUAL_PATTERN_ERROR_RATE: > drm_dbg_kms(_priv->drm, "Set Error Count Phy Test > Pattern\n"); > intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), > DDI_DP_COMP_CTL_ENABLE | > DDI_DP_COMP_CTL_SCRAMBLED_0); > break; > - case DP_PHY_TEST_PATTERN_PRBS7: > + case DP_LINK_QUAL_PATTERN_PRBS7: > drm_dbg_kms(_priv->drm, "Set PRBS7 Phy Test Pattern\n"); > intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), > DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7); > break; > - case DP_PHY_TEST_PATTERN_80BIT_CUSTOM: > + case DP_LINK_QUAL_PATTERN_80BIT_CUSTOM: > /* >* FIXME: Ideally pattern should come from DPCD 0x250. As >* current firmware of DPR-100 could not set it, so hardcoding > @@ -4721,7 +4721,7 @@ static void intel_dp_phy_pattern_update(struct intel_dp > *intel_dp, > DDI_DP_COMP_CTL_ENABLE | > DDI_DP_COMP_CTL_CUSTOM80); > break; > - case DP_PHY_TEST_PATTERN_CP2520: > + case DP_LINK_QUAL_PATTERN_CP2520_PAT_1: > /* >* FIXME: Ideally pattern should come from DPCD 0x24A. As >* current firmware of DPR-100 could not set it, so hardcoding -- Jani Nikula, Intel
[PATCH v2] drm/i915/mtl: Add fake PCH for Meteor Lake
Correct the implementation trying to detect MTL PCH with the MTL fake PCH id. On MTL, both the North Display (NDE) and South Display (SDE) functionality reside on the same die (the SoC die in this case), unlike many past platforms where the SDE was on a separate PCH die. The code is (badly) structured today in a way that assumes the SDE is always on the PCH for modern platforms, so on platforms where we don't actually need to identify the PCH to figure out how the SDE behaves (i.e., all DG1/2 GPUs as well as MTL and LNL),we've been assigning a "fake PCH" as a quickhack that allows us to avoid restructuring a bunch of the code.we've been assigning a "fake PCH" as a quick hack that allows us to avoid restructuring a bunch of the code. Removed unused macros of LNL amd MTL as well. v2: Reorder PCH_MTL conditional check (Matt Roper) Reverting to PCH_MTL for PICA interrupt(Matt Roper) Signed-off-by: Haridhar Kalvala --- drivers/gpu/drm/i915/display/intel_backlight.c | 2 +- drivers/gpu/drm/i915/display/intel_bios.c| 3 +-- drivers/gpu/drm/i915/display/intel_cdclk.c | 6 +++--- drivers/gpu/drm/i915/display/intel_display_irq.c | 2 +- drivers/gpu/drm/i915/display/intel_gmbus.c | 5 + drivers/gpu/drm/i915/display/intel_hotplug_irq.c | 6 ++ drivers/gpu/drm/i915/display/intel_pps.c | 2 +- drivers/gpu/drm/i915/soc/intel_pch.c | 16 drivers/gpu/drm/i915/soc/intel_pch.h | 6 +- 9 files changed, 19 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c index 612d4cd9dacb..696ae59874a9 100644 --- a/drivers/gpu/drm/i915/display/intel_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_backlight.c @@ -1465,7 +1465,7 @@ static bool cnp_backlight_controller_is_valid(struct drm_i915_private *i915, int if (controller == 1 && INTEL_PCH_TYPE(i915) >= PCH_ICP && - INTEL_PCH_TYPE(i915) < PCH_MTP) + INTEL_PCH_TYPE(i915) <= PCH_ADP) return intel_de_read(i915, SOUTH_CHICKEN1) & ICP_SECOND_PPS_IO_SELECT; return true; diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index aa169b0055e9..0e61e424802e 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -2204,8 +2204,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin) if (IS_DGFX(i915)) return vbt_pin; - if (INTEL_PCH_TYPE(i915) >= PCH_LNL || HAS_PCH_MTP(i915) || - IS_ALDERLAKE_P(i915)) { + if (INTEL_PCH_TYPE(i915) >= PCH_MTL || IS_ALDERLAKE_P(i915)) { ddc_pin_map = adlp_ddc_pin_map; n_entries = ARRAY_SIZE(adlp_ddc_pin_map); } else if (IS_ALDERLAKE_S(i915)) { diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index c985ebb6831a..b251a71092dd 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -3467,15 +3467,15 @@ u32 intel_read_rawclk(struct drm_i915_private *dev_priv) { u32 freq; - if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) - freq = dg1_rawclk(dev_priv); - else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTP) + if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTL) /* * MTL always uses a 38.4 MHz rawclk. The bspec tells us * "RAWCLK_FREQ defaults to the values for 38.4 and does * not need to be programmed." */ freq = 38400; + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) + freq = dg1_rawclk(dev_priv); else if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) freq = cnp_rawclk(dev_priv); else if (HAS_PCH_SPLIT(dev_priv)) diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c index a7d8f3fc98de..6964f4b95865 100644 --- a/drivers/gpu/drm/i915/display/intel_display_irq.c +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c @@ -986,7 +986,7 @@ static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_i * their flags both in the PICA and SDE IIR. */ if (*pch_iir & SDE_PICAINTERRUPT) { - drm_WARN_ON(>drm, INTEL_PCH_TYPE(i915) < PCH_MTP); + drm_WARN_ON(>drm, INTEL_PCH_TYPE(i915) < PCH_MTL); pica_ier = intel_de_rmw(i915, PICAINTERRUPT_IER, ~0, 0); *pica_iir = intel_de_read(i915, PICAINTERRUPT_IIR); diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 40d7b6f3f489..854566ba5414 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -155,7 +155,7 @@ static const struct gmbus_pin *get_gmbus_pin(struct
Re: [PATCH] drm/i915/mtl: Add fake PCH for Meteor Lake
On 12/19/2023 10:39 PM, Matt Roper wrote: On Tue, Dec 19, 2023 at 02:58:00PM +0530, Haridhar Kalvala wrote: Correct the implementation trying to detect MTL PCH with the MTL fake PCH id. On MTL, both the North Display (NDE) and South Display (SDE) functionality reside on the same die (the SoC die in this case), unlike many past platforms where the SDE was on a separate PCH die. The code is (badly) structured today in a way that assumes the SDE is always on the PCH for modern platforms, so on platforms where we don't actually need to identify the PCH to figure out how the SDE behaves (i.e., all DG1/2 GPUs as well as MTL and LNL),we've been assigning a "fake PCH" as a quickhack that allows us to avoid restructuring a bunch of the code.we've been assigning a "fake PCH" as a quick hack that allows us to avoid restructuring a bunch of the code. Removed unused macros of LNL amd MTL as well. In the future, make sure you generate your patches with "-v<#>" and add a changelog to the commit message so it's clear what's changed since the previous revision. sure. With commit title changed, this will take new series so started as initial patches taking previous comments into account. I will add version to next patchset. Signed-off-by: Haridhar Kalvala --- drivers/gpu/drm/i915/display/intel_backlight.c | 2 +- drivers/gpu/drm/i915/display/intel_bios.c| 3 +-- drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +- drivers/gpu/drm/i915/display/intel_display_irq.c | 2 +- drivers/gpu/drm/i915/display/intel_gmbus.c | 5 + drivers/gpu/drm/i915/display/intel_hotplug_irq.c | 6 ++ drivers/gpu/drm/i915/display/intel_pps.c | 2 +- drivers/gpu/drm/i915/soc/intel_pch.c | 16 drivers/gpu/drm/i915/soc/intel_pch.h | 6 +- 9 files changed, 17 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c index 612d4cd9dacb..696ae59874a9 100644 --- a/drivers/gpu/drm/i915/display/intel_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_backlight.c @@ -1465,7 +1465,7 @@ static bool cnp_backlight_controller_is_valid(struct drm_i915_private *i915, int if (controller == 1 && INTEL_PCH_TYPE(i915) >= PCH_ICP && - INTEL_PCH_TYPE(i915) < PCH_MTP) + INTEL_PCH_TYPE(i915) <= PCH_ADP) return intel_de_read(i915, SOUTH_CHICKEN1) & ICP_SECOND_PPS_IO_SELECT; return true; diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index aa169b0055e9..0e61e424802e 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -2204,8 +2204,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin) if (IS_DGFX(i915)) return vbt_pin; - if (INTEL_PCH_TYPE(i915) >= PCH_LNL || HAS_PCH_MTP(i915) || - IS_ALDERLAKE_P(i915)) { + if (INTEL_PCH_TYPE(i915) >= PCH_MTL || IS_ALDERLAKE_P(i915)) { ddc_pin_map = adlp_ddc_pin_map; n_entries = ARRAY_SIZE(adlp_ddc_pin_map); } else if (IS_ALDERLAKE_S(i915)) { diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index c985ebb6831a..2e6e55d3e885 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -3469,7 +3469,7 @@ u32 intel_read_rawclk(struct drm_i915_private *dev_priv) if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) freq = dg1_rawclk(dev_priv); - else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTP) + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTL) Since PCH_MTL > PCH_DG1, this condition can never be reached. You need to re-order the conditions here so that they go from highest to lowest: if ( >= MTL ) else if ( >= DG1 ) else if ( >= CNP ) ... Once you do this, it looks like it will also solve a pre-existing LNL bug that was causing LNL to (incorrectly) take the DG1 path instead of the MTL path. Bspec 68969 confirms that LNL should be inheriting MTL's behavior, not DG1's. Thank you. Will reorder. /* * MTL always uses a 38.4 MHz rawclk. The bspec tells us * "RAWCLK_FREQ defaults to the values for 38.4 and does diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c index a7d8f3fc98de..e318e24d1efd 100644 --- a/drivers/gpu/drm/i915/display/intel_display_irq.c +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c @@ -986,7 +986,7 @@ static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_i * their flags both in the PICA and SDE IIR. */ if (*pch_iir & SDE_PICAINTERRUPT) { - drm_WARN_ON(>drm, INTEL_PCH_TYPE(i915) < PCH_MTP); + drm_WARN_ON(>drm,
Re: [PATCH] drm/i915/guc: Add MCR type check for wa registers
On Mon, Dec 18, 2023 at 11:46:44AM +, Shuicheng Lin wrote: > Some of the wa registers are MCR registers, which have different > read/write process with normal MMIO registers. > Add function intel_gt_is_mcr_reg to check whether it is mcr register > or not. > > Signed-off-by: Shuicheng Lin > Cc: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 27 ++ > drivers/gpu/drm/i915/gt/intel_gt_mcr.h | 2 ++ > drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 6 - > 3 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > index e253750a51c5..b3ee9d152dbe 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c > @@ -81,6 +81,11 @@ static const struct intel_mmio_range > dg2_lncf_steering_table[] = { > {}, > }; > > +static const struct intel_mmio_range dg2_dss_steering_table[] = { > + { 0x00E400, 0x00E7FF }, > + {}, > +}; This is incomplete. There are a _lot_ more DSS MCR ranges than this. > + > /* > * We have several types of MCR registers on PVC where steering to (0,0) > * will always provide us with a non-terminated value. We'll stick them > @@ -190,6 +195,7 @@ void intel_gt_mcr_init(struct intel_gt *gt) > } else if (IS_DG2(i915)) { > gt->steering_table[MSLICE] = xehpsdv_mslice_steering_table; > gt->steering_table[LNCF] = dg2_lncf_steering_table; > + gt->steering_table[DSS] = dg2_dss_steering_table; Why are you making this change only on DG2? There are DSS steering ranges that we've been implicitly steering on many platforms. Switching to explicit steering is something that should probably happen on all platforms or no platforms. > /* >* No need to hook up the GAM table since it has a dedicated >* steering control register on DG2 and can use implicit > @@ -715,6 +721,27 @@ void intel_gt_mcr_get_nonterminated_steering(struct > intel_gt *gt, > *instance = gt->default_steering.instanceid; > } > > +/* > + * intel_gt_is_mcr_reg - check whether it is a mcr register or not > + * @gt: GT structure > + * @reg: the register to check > + * > + * Returns true if @reg belong to a register range of any steering type, > + * otherwise, return false. > + */ > +bool intel_gt_is_mcr_reg(struct intel_gt *gt, i915_reg_t reg) > +{ > + int type; > + i915_mcr_reg_t mcr_reg = {.reg = reg.reg}; > + > + for (type = 0; type < NUM_STEERING_TYPES; type++) { > + if (reg_needs_read_steering(gt, mcr_reg, type)) { > + return true; > + } > + } > + return false; > +} We don't need this function; see below. > + > /** > * intel_gt_mcr_read_any_fw - reads one instance of an MCR register > * @gt: GT structure > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.h > b/drivers/gpu/drm/i915/gt/intel_gt_mcr.h > index 01ac565a56a4..1ac5e6e2814e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.h > @@ -30,6 +30,8 @@ void intel_gt_mcr_multicast_write_fw(struct intel_gt *gt, > u32 intel_gt_mcr_multicast_rmw(struct intel_gt *gt, i915_mcr_reg_t reg, > u32 clear, u32 set); > > +bool intel_gt_is_mcr_reg(struct intel_gt *gt, i915_reg_t reg); > + > void intel_gt_mcr_get_nonterminated_steering(struct intel_gt *gt, >i915_mcr_reg_t reg, >u8 *group, u8 *instance); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > index 63724e17829a..6c3910c9dce5 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > @@ -377,8 +377,12 @@ static int guc_mmio_regset_init(struct temp_regset > *regset, > CCS_MASK(engine->gt)) > ret |= GUC_MMIO_REG_ADD(gt, regset, GEN12_RCU_MODE, true); > > + /* Check whether there is MCR register or not */ > for (i = 0, wa = wal->list; i < wal->count; i++, wa++) > - ret |= GUC_MMIO_REG_ADD(gt, regset, wa->reg, wa->masked_reg); > + if (intel_gt_is_mcr_reg(gt, wa->reg)) The proper condition here would be wa->is_mcr. Of course that assumes that you actually define all of the workarounds appropriately (using the MCR variants when appropriate) and also the registers properly (using MCR_REG() instead of _MMIO). Converting all of the implicit steering over to explicit steering is a lot of invasive changes to the code, and I'm not sure it's really worth it at this point. A much simpler and equally effective fix for the GuC not steering DSS (and GSLICE) ranges properly would be to just add the steering flags inside guc_mmio_reg_add() so that it gets added to all registers (both MCR and non-MCR). That's basically what we used to do (and we even
Re: [PATCH] drm/i915/mtl: Add fake PCH for Meteor Lake
On Tue, Dec 19, 2023 at 02:58:00PM +0530, Haridhar Kalvala wrote: > Correct the implementation trying to detect MTL PCH with > the MTL fake PCH id. > > On MTL, both the North Display (NDE) and South Display (SDE) functionality > reside on the same die (the SoC die in this case), unlike many past > platforms where the SDE was on a separate PCH die. The code is (badly) > structured today in a way that assumes the SDE is always on the PCH for > modern platforms, so on platforms where we don't actually need to identify > the PCH to figure out how the SDE behaves (i.e., all DG1/2 GPUs as well as > MTL and LNL),we've been assigning a "fake PCH" as a quickhack that allows > us to avoid restructuring a bunch of the code.we've been assigning a > "fake PCH" as a quick hack that allows us to avoid restructuring a bunch > of the code. > > Removed unused macros of LNL amd MTL as well. In the future, make sure you generate your patches with "-v<#>" and add a changelog to the commit message so it's clear what's changed since the previous revision. > > Signed-off-by: Haridhar Kalvala > --- > drivers/gpu/drm/i915/display/intel_backlight.c | 2 +- > drivers/gpu/drm/i915/display/intel_bios.c| 3 +-- > drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +- > drivers/gpu/drm/i915/display/intel_display_irq.c | 2 +- > drivers/gpu/drm/i915/display/intel_gmbus.c | 5 + > drivers/gpu/drm/i915/display/intel_hotplug_irq.c | 6 ++ > drivers/gpu/drm/i915/display/intel_pps.c | 2 +- > drivers/gpu/drm/i915/soc/intel_pch.c | 16 > drivers/gpu/drm/i915/soc/intel_pch.h | 6 +- > 9 files changed, 17 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c > b/drivers/gpu/drm/i915/display/intel_backlight.c > index 612d4cd9dacb..696ae59874a9 100644 > --- a/drivers/gpu/drm/i915/display/intel_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_backlight.c > @@ -1465,7 +1465,7 @@ static bool cnp_backlight_controller_is_valid(struct > drm_i915_private *i915, int > > if (controller == 1 && > INTEL_PCH_TYPE(i915) >= PCH_ICP && > - INTEL_PCH_TYPE(i915) < PCH_MTP) > + INTEL_PCH_TYPE(i915) <= PCH_ADP) > return intel_de_read(i915, SOUTH_CHICKEN1) & > ICP_SECOND_PPS_IO_SELECT; > > return true; > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > b/drivers/gpu/drm/i915/display/intel_bios.c > index aa169b0055e9..0e61e424802e 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -2204,8 +2204,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 > vbt_pin) > if (IS_DGFX(i915)) > return vbt_pin; > > - if (INTEL_PCH_TYPE(i915) >= PCH_LNL || HAS_PCH_MTP(i915) || > - IS_ALDERLAKE_P(i915)) { > + if (INTEL_PCH_TYPE(i915) >= PCH_MTL || IS_ALDERLAKE_P(i915)) { > ddc_pin_map = adlp_ddc_pin_map; > n_entries = ARRAY_SIZE(adlp_ddc_pin_map); > } else if (IS_ALDERLAKE_S(i915)) { > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index c985ebb6831a..2e6e55d3e885 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -3469,7 +3469,7 @@ u32 intel_read_rawclk(struct drm_i915_private *dev_priv) > > if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) > freq = dg1_rawclk(dev_priv); > - else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTP) > + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTL) Since PCH_MTL > PCH_DG1, this condition can never be reached. You need to re-order the conditions here so that they go from highest to lowest: if ( >= MTL ) else if ( >= DG1 ) else if ( >= CNP ) ... Once you do this, it looks like it will also solve a pre-existing LNL bug that was causing LNL to (incorrectly) take the DG1 path instead of the MTL path. Bspec 68969 confirms that LNL should be inheriting MTL's behavior, not DG1's. > /* >* MTL always uses a 38.4 MHz rawclk. The bspec tells us >* "RAWCLK_FREQ defaults to the values for 38.4 and does > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c > b/drivers/gpu/drm/i915/display/intel_display_irq.c > index a7d8f3fc98de..e318e24d1efd 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c > @@ -986,7 +986,7 @@ static void gen8_read_and_ack_pch_irqs(struct > drm_i915_private *i915, u32 *pch_i >* their flags both in the PICA and SDE IIR. >*/ > if (*pch_iir & SDE_PICAINTERRUPT) { > - drm_WARN_ON(>drm, INTEL_PCH_TYPE(i915) < PCH_MTP); > + drm_WARN_ON(>drm, INTEL_PCH_TYPE(i915) <= PCH_ADP); I think you can keep this one "< PCH_MTL." It's a bug if we ever see a PICA interrupt on
[PATCH v10 1/6] pwm: Rename pwm_apply_state() to pwm_apply_might_sleep()
In order to introduce a pwm api which can be used from atomic context, we will need two functions for applying pwm changes: int pwm_apply_might_sleep(struct pwm *, struct pwm_state *); int pwm_apply_atomic(struct pwm *, struct pwm_state *); This commit just deals with renaming pwm_apply_state(), a following commit will introduce the pwm_apply_atomic() function. Acked-by: Uwe Kleine-König Acked-by: Guenter Roeck Acked-by: Mark Brown Acked-by: Dmitry Torokhov # for input Acked-by: Hans de Goede Acked-by: Jani Nikula Acked-by: Lee Jones Signed-off-by: Sean Young --- Documentation/driver-api/pwm.rst | 8 +++--- MAINTAINERS | 2 +- .../gpu/drm/i915/display/intel_backlight.c| 6 ++-- drivers/gpu/drm/solomon/ssd130x.c | 2 +- drivers/hwmon/pwm-fan.c | 8 +++--- drivers/input/misc/da7280.c | 4 +-- drivers/input/misc/pwm-beeper.c | 4 +-- drivers/input/misc/pwm-vibra.c| 8 +++--- drivers/leds/leds-pwm.c | 2 +- drivers/leds/rgb/leds-pwm-multicolor.c| 4 +-- drivers/media/rc/pwm-ir-tx.c | 4 +-- drivers/platform/x86/lenovo-yogabook.c| 2 +- drivers/pwm/core.c| 18 ++-- drivers/pwm/pwm-twl-led.c | 2 +- drivers/pwm/pwm-vt8500.c | 2 +- drivers/pwm/sysfs.c | 10 +++ drivers/regulator/pwm-regulator.c | 4 +-- drivers/video/backlight/lm3630a_bl.c | 2 +- drivers/video/backlight/lp855x_bl.c | 2 +- drivers/video/backlight/pwm_bl.c | 12 drivers/video/fbdev/ssd1307fb.c | 2 +- include/linux/pwm.h | 28 +-- 22 files changed, 68 insertions(+), 68 deletions(-) diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst index bb264490a87a..f1d8197c8c43 100644 --- a/Documentation/driver-api/pwm.rst +++ b/Documentation/driver-api/pwm.rst @@ -41,7 +41,7 @@ the getter, devm_pwm_get() and devm_fwnode_pwm_get(), also exist. After being requested, a PWM has to be configured using:: - int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state); + int pwm_apply_might_sleep(struct pwm_device *pwm, struct pwm_state *state); This API controls both the PWM period/duty_cycle config and the enable/disable state. @@ -57,13 +57,13 @@ If supported by the driver, the signal can be optimized, for example to improve EMI by phase shifting the individual channels of a chip. The pwm_config(), pwm_enable() and pwm_disable() functions are just wrappers -around pwm_apply_state() and should not be used if the user wants to change +around pwm_apply_might_sleep() and should not be used if the user wants to change several parameter at once. For example, if you see pwm_config() and pwm_{enable,disable}() calls in the same function, this probably means you -should switch to pwm_apply_state(). +should switch to pwm_apply_might_sleep(). The PWM user API also allows one to query the PWM state that was passed to the -last invocation of pwm_apply_state() using pwm_get_state(). Note this is +last invocation of pwm_apply_might_sleep() using pwm_get_state(). Note this is different to what the driver has actually implemented if the request cannot be satisfied exactly with the hardware in use. There is currently no way for consumers to get the actually implemented settings. diff --git a/MAINTAINERS b/MAINTAINERS index 97f51d5ec1cf..c58480595220 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17576,7 +17576,7 @@ F: drivers/video/backlight/pwm_bl.c F: include/dt-bindings/pwm/ F: include/linux/pwm.h F: include/linux/pwm_backlight.h -K: pwm_(config|apply_state|ops) +K: pwm_(config|apply_might_sleep|ops) PXA GPIO DRIVER M: Robert Jarzmik diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c index 2e8f17c04522..ff9b9918b0a1 100644 --- a/drivers/gpu/drm/i915/display/intel_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_backlight.c @@ -274,7 +274,7 @@ static void ext_pwm_set_backlight(const struct drm_connector_state *conn_state, struct intel_panel *panel = _intel_connector(conn_state->connector)->panel; pwm_set_relative_duty_cycle(>backlight.pwm_state, level, 100); - pwm_apply_state(panel->backlight.pwm, >backlight.pwm_state); + pwm_apply_might_sleep(panel->backlight.pwm, >backlight.pwm_state); } static void @@ -427,7 +427,7 @@ static void ext_pwm_disable_backlight(const struct drm_connector_state *old_conn intel_backlight_set_pwm_level(old_conn_state, level); panel->backlight.pwm_state.enabled = false; - pwm_apply_state(panel->backlight.pwm, >backlight.pwm_state); +
Re: [PATCH 2/2] drm/i915/hdcp: fix intel_hdcp_get_repeater_ctl() error return value
On Tue, 19 Dec 2023, Ville Syrjälä wrote: > On Tue, Dec 19, 2023 at 12:47:46PM +0200, Jani Nikula wrote: >> intel_hdcp_get_repeater_ctl() is supposed to return unsigned register >> contents. Returning negative error values is unexpected, and none of the >> callers check for that. >> >> Sort of fix the error cases by returning 0. I don't think we should hit >> these cases anyway, and using 0 for the registers is safer than >> 0xffea (-EINVAL). >> >> Signed-off-by: Jani Nikula >> --- >> drivers/gpu/drm/i915/display/intel_hdcp.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c >> b/drivers/gpu/drm/i915/display/intel_hdcp.c >> index f9010094ff29..ee29fcb860e4 100644 >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c >> @@ -347,7 +347,7 @@ u32 intel_hdcp_get_repeater_ctl(struct drm_i915_private >> *i915, >> default: >> drm_err(>drm, "Unknown transcoder %d\n", >> cpu_transcoder); > > These should probably be MISSING_CASE() as well. Yeah, maybe. For another day... > > Anyways, > Reviewed-by: Ville Syrjälä Thanks, both pushed. BR, Jani. > >> -return -EINVAL; >> +return 0; >> } >> } >> >> @@ -364,7 +364,7 @@ u32 intel_hdcp_get_repeater_ctl(struct drm_i915_private >> *i915, >> return HDCP_DDIE_REP_PRESENT | HDCP_DDIE_SHA1_M0; >> default: >> drm_err(>drm, "Unknown port %d\n", port); >> -return -EINVAL; >> +return 0; >> } >> } >> >> -- >> 2.39.2 -- Jani Nikula, Intel
Re: [Intel-gfx] [PATCH] drm/i915/gem: Atomically invalidate userptr on mmu-notifier
Hi Jonathan, On Mon, Dec 18, 2023 at 06:33:44PM +, Cavitt, Jonathan wrote: ... > > On Tue, Nov 28, 2023 at 08:25:05AM -0800, Jonathan Cavitt wrote: > > > Never block for outstanding work on userptr object upon receipt of a > > > mmu-notifier. The reason we originally did so was to immediately unbind > > > the userptr and unpin its pages, but since that has been dropped in > > > commit b4b9731b02c3c ("drm/i915: Simplify userptr locking"), we never > > > return the pages to the system i.e. never drop our page->mapcount and so > > > do not allow the page and CPU PTE to be revoked. Based on this history, > > > we know we are safe to drop the wait entirely. > > > > > > Upon return from mmu-notifier, we will still have the userptr pages > > > pinned preventing the following PTE operation (such as try_to_unmap) > > > adjusting the vm_area_struct, so it is safe to keep the pages around for > > > as long as we still have i/o pending. > > > > > > We do not have any means currently to asynchronously revalidate the > > > userptr pages, that is always prior to next use. > > > > > > Signed-off-by: Chris Wilson > > > Signed-off-by: Jonathan Cavitt > > > > Reviewed-by: Andi Shyti > > Thank you for the review! I don't think I have permission to push this > upstream, > though, so you or someone else will have to complete the push. pushed in drm-intel-gt-next. Thanks and sorry for the late merge. Andi
Re: [RFC] drm/i915: Allow dmabuf mmap forwarding
On 13/12/2023 14:13, Christian König wrote: Am 13.12.23 um 12:46 schrieb Tvrtko Ursulin: Hi, On 12/12/2023 14:10, Christian König wrote: Hi Tvrtko, Thanks for pointing this mail out once more, I've totally missed it. That's okay, if it was really urgent I would have re-raised the thread earlier. :) As it stands so far it is only about acceptance test suites failing and no known real use cases affected. Am 12.12.23 um 11:37 schrieb Tvrtko Ursulin: On 25/09/2023 14:16, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Allow mmap forwarding for imported buffers in order to allow minigbm mmap to work on aperture-less platforms such as Meteorlake. So far i915 did not allow mmap on imported buffers but from minigbm perspective that worked because of the DRM_IOCTL_I915_GEM_MMAP_GTT fall- back would then be attempted, and would be successful. This stops working on Meteorlake since there is no aperture. Allow i915 to mmap imported buffers using forwarding via dma_buf_mmap(), which allows the primary minigbm path of DRM_IOCTL_I915_GEM_MMAP_OFFSET / I915_MMAP_OFFSET_WB to work. Signed-off-by: Tvrtko Ursulin Cc: Daniel Vetter Cc: Christian König Cc: Matthew Auld Cc: Nirmoy Das --- 1) It is unclear to me if any real userspace depends on this, but there are certainly compliance suites which fail. Well that is actually intentional, but see below. 2) It is also a bit unclear to me if dma_buf_mmap() is exactly intended for this kind of use. It seems that it is, but I also found some old mailing list discussions suggesting there might be some unresolved questions around VMA revocation. I actually solved those a few years back by introducing the vma_set_file() function which standardized the dance necessary for the dma_buf_mmap() function. 1 + 2 = RFC for now. Daniel and Christian were involved in 2) in the past so comments would be appreciated. Any comments on this one? I don't have all the historical knowledge of when this was maybe attempted before and what problems were hit, or something. So would there be downsides or it is fine to forward it. It works technically inside the kernel and Thomas Zimmerman suggested a patch set which made it possible to use for all DRM drivers. But IIRC this patch set was rejected with the rational that while doing an mmap() on an imported DMA-buf works when userspace actually does this then there is a bug in userspace. The UMD doesn't seems to be aware of the fact that the buffer is imported and so for example needs to call dma_buf_begin_cpu_access() and dma_buf_end_cpu_access(). UMDs can trivially work around this by doing the mmap() on the DMA-buf file descriptor instead (potentially after re-exporting it), but the kernel really shouldn't help hide userspace bugs. Hm right, however why does drm_gem_shmem_mmap: if (obj->import_attach) { ret = dma_buf_mmap(obj->dma_buf, vma, 0); Honestly I have absolutely no idea. Isn't that allowing drivers which use the helper to to forward to dma_buf_mmap? Yes, Daniel mentioned that some drivers did this before we found that it's actually not a good idea. It could be that this code piece was meant with that and we only allow it to avoid breaking UAPI. Never the less I think we should add documentation for this. Maybe I am getting lost in the forest of callbacks in this area.. Because it is supposed to be about shmem objects, but drivers which use the helper and rely on common prime import look and also use drm_gem_shmem_prime_import_sg_table can get there. I don't fully understand it either of hand. Right, I will leave untangling the jungle of callbacks and helpers in my mind for later too. For now I think the rationale that the API does not allow correctnes via dma_buf_begin_cpu_access/end is I think good enough to pass on as will-not-implement. Thanks for the clarifications! Regards, Tvrtko Regards, Christian. Regards, Tvrtko Test-with: 20230925131539.32743-1-tvrtko.ursu...@linux.intel.com --- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 78 +++ .../gpu/drm/i915/gem/i915_gem_object_types.h | 1 + 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index aa4d842d4c5a..78c84c0a8b08 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -664,6 +665,7 @@ insert_mmo(struct drm_i915_gem_object *obj, struct i915_mmap_offset *mmo) static struct i915_mmap_offset * mmap_offset_attach(struct drm_i915_gem_object *obj, enum i915_mmap_type mmap_type, + bool forward_mmap, struct drm_file *file) { struct drm_i915_private *i915 = to_i915(obj->base.dev); @@ -682,6 +684,7 @@ mmap_offset_attach(struct drm_i915_gem_object *obj, mmo->obj = obj;
✓ Fi.CI.BAT: success for drm/i915: Rework global state serialization
== Series Details == Series: drm/i915: Rework global state serialization URL : https://patchwork.freedesktop.org/series/127968/ State : success == Summary == CI Bug Log - changes from CI_DRM_14042 -> Patchwork_127968v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/index.html Participating hosts (35 -> 33) -- Additional (2): bat-kbl-2 bat-adlp-11 Missing(4): bat-dg2-9 fi-bsw-nick fi-snb-2520m fi-bsw-n3050 Known issues Here are the changes found in Patchwork_127968v1 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - bat-adlp-11:NOTRUN -> [SKIP][1] ([i915#9318]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@debugfs_t...@basic-hwmon.html * igt@fbdev@info: - bat-adlp-11:NOTRUN -> [SKIP][2] ([i915#1849] / [i915#2582]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@fb...@info.html - bat-kbl-2: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1849]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-kbl-2/igt@fb...@info.html * igt@fbdev@nullptr: - bat-adlp-11:NOTRUN -> [SKIP][4] ([i915#2582]) +3 other tests skip [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@fb...@nullptr.html * igt@gem_lmem_swapping@parallel-random-engines: - bat-kbl-2: NOTRUN -> [SKIP][5] ([fdo#109271]) +36 other tests skip [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@gem_lmem_swapping@verify-random: - bat-adlp-11:NOTRUN -> [SKIP][6] ([i915#4613]) +3 other tests skip [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@gem_lmem_swapp...@verify-random.html * igt@gem_tiled_pread_basic: - bat-adlp-11:NOTRUN -> [SKIP][7] ([i915#3282]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@gem_tiled_pread_basic.html * igt@i915_module_load@reload: - fi-apl-guc: [PASS][8] -> [DMESG-WARN][9] ([i915#180] / [i915#1982] / [i915#8585]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-apl-guc/igt@i915_module_l...@reload.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/fi-apl-guc/igt@i915_module_l...@reload.html * igt@i915_pm_rpm@module-reload: - fi-apl-guc: [PASS][10] -> [DMESG-WARN][11] ([i915#180] / [i915#8585]) +1 other test dmesg-warn [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-apl-guc/igt@i915_pm_...@module-reload.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/fi-apl-guc/igt@i915_pm_...@module-reload.html * igt@i915_pm_rps@basic-api: - bat-adlp-11:NOTRUN -> [SKIP][12] ([i915#6621]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@i915_pm_...@basic-api.html * igt@i915_selftest@live@reset: - fi-apl-guc: [PASS][13] -> [DMESG-WARN][14] ([i915#9730]) +36 other tests dmesg-warn [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-apl-guc/igt@i915_selftest@l...@reset.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/fi-apl-guc/igt@i915_selftest@l...@reset.html * igt@i915_suspend@basic-s3-without-i915: - bat-atsm-1: NOTRUN -> [SKIP][15] ([i915#6645]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-atsm-1/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - bat-adlp-11:NOTRUN -> [SKIP][16] ([i915#5608] / [i915#9866]) +1 other test skip [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy: - bat-adlp-11:NOTRUN -> [SKIP][17] ([i915#9824] / [i915#9866]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@kms_cursor_leg...@basic-flip-before-cursor-legacy.html * igt@kms_flip@basic-flip-vs-dpms: - bat-adlp-11:NOTRUN -> [SKIP][18] ([i915#3637]) +3 other tests skip [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@kms_f...@basic-flip-vs-dpms.html * igt@kms_force_connector_basic@prune-stale-modes: - bat-adlp-11:NOTRUN -> [SKIP][19] ([i915#4093]) +3 other tests skip [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v1/bat-adlp-11/igt@kms_force_connector_ba...@prune-stale-modes.html * igt@kms_frontbuffer_tracking@basic: - bat-adlp-11:NOTRUN -> [SKIP][20] ([i915#4342] / [i915#5354]) [20]:
✗ Fi.CI.IGT: failure for drm/i915: Cursor vblank evasion
== Series Details == Series: drm/i915: Cursor vblank evasion URL : https://patchwork.freedesktop.org/series/127744/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14017_full -> Patchwork_127744v1_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_127744v1_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_127744v1_full, please notify your bug team (i915-ci-in...@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (7 -> 8) -- Additional (1): shard-glk-0 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_127744v1_full: ### IGT changes ### Possible regressions * igt@kms_cursor_legacy@single-move@all-pipes: - shard-mtlp: [PASS][1] -> [DMESG-WARN][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14017/shard-mtlp-7/igt@kms_cursor_legacy@single-m...@all-pipes.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-mtlp-5/igt@kms_cursor_legacy@single-m...@all-pipes.html Known issues Here are the changes found in Patchwork_127744v1_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_balancer@bonded-chain: - shard-rkl: NOTRUN -> [ABORT][3] ([i915#9856]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-rkl-5/igt@gem_exec_balan...@bonded-chain.html - shard-dg1: NOTRUN -> [ABORT][4] ([i915#9856]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-dg1-13/igt@gem_exec_balan...@bonded-chain.html * igt@gem_exec_balancer@full: - shard-dg2: NOTRUN -> [ABORT][5] ([i915#9855] / [i915#9856]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-dg2-7/igt@gem_exec_balan...@full.html - shard-rkl: NOTRUN -> [INCOMPLETE][6] ([i915#9856]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-rkl-4/igt@gem_exec_balan...@full.html * igt@gem_exec_balancer@indices: - shard-dg2: NOTRUN -> [INCOMPLETE][7] ([i915#9856]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-dg2-6/igt@gem_exec_balan...@indices.html * igt@gem_exec_balancer@parallel-balancer: - shard-dg1: NOTRUN -> [ABORT][8] ([i915#9855]) +1 other test abort [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-dg1-13/igt@gem_exec_balan...@parallel-balancer.html * igt@gem_exec_reloc@basic-write-gtt: - shard-mtlp: NOTRUN -> [SKIP][9] ([i915#3281]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-mtlp-7/igt@gem_exec_re...@basic-write-gtt.html * igt@gem_exec_suspend@basic-s4-devices@lmem0: - shard-dg2: [PASS][10] -> [ABORT][11] ([i915#7975] / [i915#8213]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14017/shard-dg2-11/igt@gem_exec_suspend@basic-s4-devi...@lmem0.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-dg2-5/igt@gem_exec_suspend@basic-s4-devi...@lmem0.html * igt@gem_exec_whisper@basic-contexts: - shard-tglu: NOTRUN -> [INCOMPLETE][12] ([i915#6755] / [i915#9857]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-tglu-2/igt@gem_exec_whis...@basic-contexts.html * igt@gem_mmap_wc@write-gtt-read-wc: - shard-mtlp: NOTRUN -> [SKIP][13] ([i915#4083]) +2 other tests skip [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-mtlp-6/igt@gem_mmap...@write-gtt-read-wc.html * igt@gem_readwrite@new-obj: - shard-mtlp: NOTRUN -> [SKIP][14] ([i915#3282]) +1 other test skip [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-mtlp-6/igt@gem_readwr...@new-obj.html * igt@gem_render_copy@y-tiled-to-vebox-y-tiled: - shard-mtlp: NOTRUN -> [SKIP][15] ([i915#8428]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-mtlp-6/igt@gem_render_c...@y-tiled-to-vebox-y-tiled.html * igt@gem_tiled_partial_pwrite_pread@reads: - shard-mtlp: NOTRUN -> [SKIP][16] ([i915#4077]) +1 other test skip [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-mtlp-6/igt@gem_tiled_partial_pwrite_pr...@reads.html * igt@gem_userptr_blits@vma-merge: - shard-tglu: NOTRUN -> [FAIL][17] ([i915#3318]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/shard-tglu-2/igt@gem_userptr_bl...@vma-merge.html * igt@gen9_exec_parse@basic-rejected: - shard-mtlp: NOTRUN -> [SKIP][18] ([i915#2856]) [18]:
✗ Fi.CI.SPARSE: warning for drm/i915: Rework global state serialization
== Series Details == Series: drm/i915: Rework global state serialization URL : https://patchwork.freedesktop.org/series/127968/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
✗ Fi.CI.BAT: failure for drm/i915/display: Skip C10 state verification in case of fastset
== Series Details == Series: drm/i915/display: Skip C10 state verification in case of fastset URL : https://patchwork.freedesktop.org/series/127966/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14042 -> Patchwork_127966v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_127966v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_127966v1, please notify your bug team (i915-ci-in...@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/index.html Participating hosts (35 -> 38) -- Additional (4): bat-dg2-8 bat-adlp-11 bat-mtlp-8 fi-pnv-d510 Missing(1): fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_127966v1: ### IGT changes ### Possible regressions * igt@i915_suspend@basic-s3-without-i915: - fi-kbl-7567u: [PASS][1] -> [DMESG-WARN][2] +1 other test dmesg-warn [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-kbl-7567u/igt@i915_susp...@basic-s3-without-i915.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/fi-kbl-7567u/igt@i915_susp...@basic-s3-without-i915.html Known issues Here are the changes found in Patchwork_127966v1 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - bat-mtlp-8: NOTRUN -> [SKIP][3] ([i915#9318]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html - bat-adlp-11:NOTRUN -> [SKIP][4] ([i915#9318]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-adlp-11/igt@debugfs_t...@basic-hwmon.html * igt@fbdev@info: - bat-adlp-11:NOTRUN -> [SKIP][5] ([i915#1849] / [i915#2582]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-adlp-11/igt@fb...@info.html * igt@fbdev@nullptr: - bat-adlp-11:NOTRUN -> [SKIP][6] ([i915#2582]) +3 other tests skip [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-adlp-11/igt@fb...@nullptr.html * igt@gem_exec_suspend@basic-s0@smem: - bat-dg2-9: [PASS][7] -> [INCOMPLETE][8] ([i915#9275]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/bat-dg2-9/igt@gem_exec_suspend@basic...@smem.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-dg2-9/igt@gem_exec_suspend@basic...@smem.html * igt@gem_lmem_swapping@basic: - fi-pnv-d510:NOTRUN -> [SKIP][9] ([fdo#109271]) +28 other tests skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/fi-pnv-d510/igt@gem_lmem_swapp...@basic.html * igt@gem_lmem_swapping@verify-random: - bat-adlp-11:NOTRUN -> [SKIP][10] ([i915#4613]) +3 other tests skip [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-adlp-11/igt@gem_lmem_swapp...@verify-random.html - bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#4613]) +3 other tests skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_mmap@basic: - bat-mtlp-8: NOTRUN -> [SKIP][12] ([i915#4083]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-mtlp-8/igt@gem_m...@basic.html - bat-dg2-8: NOTRUN -> [SKIP][13] ([i915#4083]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-dg2-8/igt@gem_m...@basic.html * igt@gem_mmap_gtt@basic: - bat-mtlp-8: NOTRUN -> [SKIP][14] ([i915#4077]) +2 other tests skip [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-mtlp-8/igt@gem_mmap_...@basic.html - bat-dg2-8: NOTRUN -> [SKIP][15] ([i915#4077]) +2 other tests skip [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-dg2-8/igt@gem_mmap_...@basic.html * igt@gem_render_tiled_blits@basic: - bat-mtlp-8: NOTRUN -> [SKIP][16] ([i915#4079]) +1 other test skip [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-dg2-8: NOTRUN -> [SKIP][17] ([i915#4079]) +1 other test skip [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-dg2-8/igt@gem_tiled_pread_basic.html - bat-adlp-11:NOTRUN -> [SKIP][18] ([i915#3282]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127966v1/bat-adlp-11/igt@gem_tiled_pread_basic.html * igt@i915_pm_rpm@module-reload: - fi-kbl-7567u: [PASS][19] -> [DMESG-WARN][20] ([i915#8585]) [19]:
[PATCH 3/3] drm/i915: Extract intel_atomic_swap_state()
From: Ville Syrjälä Pull all the state swap stuff into its own function to declutter intel_atomic_commit() a bit. Note that currently the state swap is spread across both sides of the unprepare branch in intel_atomic_commit(), but we can pull all of it ahead a bit since we bail on the first error, and thus there is no change in behaviour from the reordering. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 23 +++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 46eff0ee5519..bf37bfcf350e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7311,6 +7311,23 @@ static int intel_atomic_setup_commit(struct intel_atomic_state *state, bool nonb return 0; } +static int intel_atomic_swap_state(struct intel_atomic_state *state) +{ + int ret; + + ret = drm_atomic_helper_swap_state(>base, true); + if (ret) + return ret; + + intel_atomic_swap_global_state(state); + + intel_shared_dpll_swap_state(state); + + intel_atomic_track_fbs(state); + + return 0; +} + int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *_state, bool nonblock) { @@ -7358,9 +7375,7 @@ int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *_state, ret = intel_atomic_setup_commit(state, nonblock); if (!ret) - ret = drm_atomic_helper_swap_state(>base, true); - if (!ret) - intel_atomic_swap_global_state(state); + ret = intel_atomic_swap_state(state); if (ret) { struct intel_crtc_state *new_crtc_state; @@ -7374,8 +7389,6 @@ int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *_state, intel_runtime_pm_put(_priv->runtime_pm, state->wakeref); return ret; } - intel_shared_dpll_swap_state(state); - intel_atomic_track_fbs(state); drm_atomic_state_get(>base); INIT_WORK(>base.commit_work, intel_atomic_commit_work); -- 2.41.0
[PATCH 2/3] drm/i915: Rework global state serializaiton
From: Ville Syrjälä Instead of injecting extra crtc commits to serialize the global state let's hand roll a but of commit machinery to take care of the hardware synchronization. Rather than basing everything on the crtc commits we track these as their own thing. I think this makes more sense as the hardware blocks we are working with are not in any way tied to the pipes, so the completion should not be tied in with the vblank machinery either. The difference to the old behaviour is that: - we no longer pull extra crtcs into the commit which should make drm_atomic_check_only() happier - since those crtcs don't get pulled in we also don't end up reprogamming them and thus don't need to wait their vblanks to pass/etc. So this should be tad faster as well. TODO: perhaps have each global object complete its own commit once the post-plane update phase is done? Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6728 Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 19 ++- .../gpu/drm/i915/display/intel_global_state.c | 137 -- .../gpu/drm/i915/display/intel_global_state.h | 9 +- 3 files changed, 152 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b10aad15a63d..46eff0ee5519 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7068,6 +7068,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) drm_atomic_helper_wait_for_dependencies(>base); drm_dp_mst_atomic_wait_for_dependencies(>base); + intel_atomic_global_state_wait_for_dependencies(state); /* * During full modesets we write a lot of registers, wait @@ -7244,6 +7245,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) intel_pmdemand_post_plane_update(state); drm_atomic_helper_commit_hw_done(>base); + intel_atomic_global_state_commit_done(state); if (state->modeset) { /* As one of the primary mmio accessors, KMS has a high @@ -7294,6 +7296,21 @@ static void intel_atomic_track_fbs(struct intel_atomic_state *state) plane->frontbuffer_bit); } +static int intel_atomic_setup_commit(struct intel_atomic_state *state, bool nonblock) +{ + int ret; + + ret = drm_atomic_helper_setup_commit(>base, nonblock); + if (ret) + return ret; + + ret = intel_atomic_global_state_setup_commit(state); + if (ret) + return ret; + + return 0; +} + int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *_state, bool nonblock) { @@ -7339,7 +7356,7 @@ int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *_state, return ret; } - ret = drm_atomic_helper_setup_commit(>base, nonblock); + ret = intel_atomic_setup_commit(state, nonblock); if (!ret) ret = drm_atomic_helper_swap_state(>base, true); if (!ret) diff --git a/drivers/gpu/drm/i915/display/intel_global_state.c b/drivers/gpu/drm/i915/display/intel_global_state.c index e8e8be54143b..cbcd1e91b7be 100644 --- a/drivers/gpu/drm/i915/display/intel_global_state.c +++ b/drivers/gpu/drm/i915/display/intel_global_state.c @@ -10,12 +10,55 @@ #include "intel_display_types.h" #include "intel_global_state.h" +struct intel_global_commit { + struct kref ref; + struct completion done; +}; + +static struct intel_global_commit *commit_new(void) +{ + struct intel_global_commit *commit; + + commit = kzalloc(sizeof(*commit), GFP_KERNEL); + if (!commit) + return NULL; + + init_completion(>done); + kref_init(>ref); + + return commit; +} + +static void __commit_free(struct kref *kref) +{ + struct intel_global_commit *commit = + container_of(kref, typeof(*commit), ref); + + kfree(commit); +} + +static struct intel_global_commit *commit_get(struct intel_global_commit *commit) +{ + if (commit) + kref_get(>ref); + + return commit; +} + +static void commit_put(struct intel_global_commit *commit) +{ + if (commit) + kref_put(>ref, __commit_free); +} + static void __intel_atomic_global_state_free(struct kref *kref) { struct intel_global_state *obj_state = container_of(kref, struct intel_global_state, ref); struct intel_global_obj *obj = obj_state->obj; + commit_put(obj_state->commit); + obj->funcs->atomic_destroy_state(obj, obj_state); } @@ -127,6 +170,8 @@ intel_atomic_get_global_obj_state(struct intel_atomic_state *state, obj_state->obj = obj; obj_state->changed = false; + obj_state->serialized = false; + obj_state->commit = NULL;
[PATCH 1/3] drm/i915: Compute use_sagv_wm differently
From: Ville Syrjälä drm_atomic_check_only() gets upset if we try to add extra crtcs to any commit that isn't flagged with DRM_MODE_ATOMIC_ALLOW_MODESET. This conflicts with how SAGV watermarks work on pre-ADL as we need to manually switch over the SAGV watermarks before we can safely enable SAGV. So in order to make SAGV usage possible we need to compute each pipe's use of SAGV watermarks as if there aren't any other active pipes. Ie. if the current pipe isn't the one blocking SAGV then we make it use the SAGV watermarks, even if some other pipe prevents SAGV from actually being used. Otherwise we could end up with a pipes using the normal watermarks (but not blocking SAGV), and some other pipe in parallel enabling SAGV, which would likely cause underruns. The alternative approach of preventing SAGV usage until all pipes simultanously end up using SAGV watermarks would only really work if userspace always adds all pipes to every commits, which isn't the case typically. The downside of this is that we will end up using the less optimal SAGV watermarks even if some other pipe prevents SAGV from actually being enabled. In which case the system won't achieve the minimum possible power consumption. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 38 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 56588d6e24ae..9cee19cbe253 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -443,12 +443,35 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state) for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + struct skl_pipe_wm *pipe_wm = _crtc_state->wm.skl.optimal; + new_bw_state = intel_atomic_get_bw_state(state); if (IS_ERR(new_bw_state)) return PTR_ERR(new_bw_state); old_bw_state = intel_atomic_get_old_bw_state(state); + /* +* We store use_sagv_wm in the crtc state rather than relying on +* that bw state since we have no convenient way to get at the +* latter from the plane commit hooks (especially in the legacy +* cursor case). +* +* drm_atomic_check_only() gets upset if we pull more crtcs +* into the state, so we have to calculate this based on the +* individual intel_crtc_can_enable_sagv() rather than +* the overall intel_crtc_can_enable_sagv(). Otherwise the +* crtcs not included in the commit would not switch to the +* SAGV watermarks when we are about to enable SAGV, and that +* would lead to underruns. This does mean extra power draw +* when only a subset of the crtcs are blocking SAGV as the +* other crtcs can't be allowed to use the more optimal +* normal (ie. non-SAGV) watermarks. +*/ + pipe_wm->use_sagv_wm = !HAS_HW_SAGV_WM(i915) && + DISPLAY_VER(i915) >= 12 && + intel_crtc_can_enable_sagv(new_crtc_state); + if (intel_crtc_can_enable_sagv(new_crtc_state)) new_bw_state->pipe_sagv_reject &= ~BIT(crtc->pipe); else @@ -478,21 +501,6 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state) return ret; } - for_each_new_intel_crtc_in_state(state, crtc, -new_crtc_state, i) { - struct skl_pipe_wm *pipe_wm = _crtc_state->wm.skl.optimal; - - /* -* We store use_sagv_wm in the crtc state rather than relying on -* that bw state since we have no convenient way to get at the -* latter from the plane commit hooks (especially in the legacy -* cursor case) -*/ - pipe_wm->use_sagv_wm = !HAS_HW_SAGV_WM(i915) && - DISPLAY_VER(i915) >= 12 && - intel_can_enable_sagv(i915, new_bw_state); - } - return 0; } -- 2.41.0
[PATCH 0/3] drm/i915: Rework global state serialization
From: Ville Syrjälä Rework the way we do the hardware serialization with global states. This avoids angering drm_atomic_check_only(), and I suppose it's a bit more efficient as well. Ville Syrjälä (3): drm/i915: Compute use_sagv_wm differently drm/i915: Rework global state serializaiton drm/i915: Extract intel_atomic_swap_state() drivers/gpu/drm/i915/display/intel_display.c | 42 +- .../gpu/drm/i915/display/intel_global_state.c | 137 -- .../gpu/drm/i915/display/intel_global_state.h | 9 +- drivers/gpu/drm/i915/display/skl_watermark.c | 38 +++-- 4 files changed, 193 insertions(+), 33 deletions(-) -- 2.41.0
Re: [PATCH 2/2] drm/i915/hdcp: fix intel_hdcp_get_repeater_ctl() error return value
On Tue, Dec 19, 2023 at 12:47:46PM +0200, Jani Nikula wrote: > intel_hdcp_get_repeater_ctl() is supposed to return unsigned register > contents. Returning negative error values is unexpected, and none of the > callers check for that. > > Sort of fix the error cases by returning 0. I don't think we should hit > these cases anyway, and using 0 for the registers is safer than > 0xffea (-EINVAL). > > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > b/drivers/gpu/drm/i915/display/intel_hdcp.c > index f9010094ff29..ee29fcb860e4 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -347,7 +347,7 @@ u32 intel_hdcp_get_repeater_ctl(struct drm_i915_private > *i915, > default: > drm_err(>drm, "Unknown transcoder %d\n", > cpu_transcoder); These should probably be MISSING_CASE() as well. Anyways, Reviewed-by: Ville Syrjälä > - return -EINVAL; > + return 0; > } > } > > @@ -364,7 +364,7 @@ u32 intel_hdcp_get_repeater_ctl(struct drm_i915_private > *i915, > return HDCP_DDIE_REP_PRESENT | HDCP_DDIE_SHA1_M0; > default: > drm_err(>drm, "Unknown port %d\n", port); > - return -EINVAL; > + return 0; > } > } > > -- > 2.39.2 -- Ville Syrjälä Intel
✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: Skip C10 state verification in case of fastset
== Series Details == Series: drm/i915/display: Skip C10 state verification in case of fastset URL : https://patchwork.freedesktop.org/series/127966/ State : warning == Summary == Error: dim checkpatch failed 9b8ec3f52711 drm/i915/display: Skip C10 state verification in case of fastset -:8: WARNING:TYPO_SPELLING: 'verfication' may be misspelled - perhaps 'verification'? #8: verfication compares bios programmed PLL values against ^^^ total: 0 errors, 1 warnings, 0 checks, 9 lines checked
✗ Fi.CI.BUILD: warning for drm/i915/display: Skip C10 state verification in case of fastset
== Series Details == Series: drm/i915/display: Skip C10 state verification in case of fastset URL : https://patchwork.freedesktop.org/series/127966/ State : warning == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/127966/revisions/1/mbox/ not found
Re: [PATCH 1/2] drm/i915/hdcp: unify connector logging format
On Tue, Dec 19, 2023 at 12:47:45PM +0200, Jani Nikula wrote: > It's customary to debug log connectors using [CONNECTOR:%d:%s] > format. Make the HDCP code follow suit. > > Signed-off-by: Jani Nikula Reviewed-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 68 +++ > 1 file changed, 34 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > b/drivers/gpu/drm/i915/display/intel_hdcp.c > index 39b3f7c0c77c..f9010094ff29 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -853,8 +853,8 @@ static int intel_hdcp_auth(struct intel_connector > *connector) > if (shim->stream_encryption) { > ret = shim->stream_encryption(connector, true); > if (ret) { > - drm_err(>drm, "[%s:%d] Failed to enable HDCP 1.4 > stream enc\n", > - connector->base.name, connector->base.base.id); > + drm_err(>drm, "[CONNECTOR:%d:%s] Failed to enable > HDCP 1.4 stream enc\n", > + connector->base.base.id, connector->base.name); > return ret; > } > drm_dbg_kms(>drm, "HDCP 1.4 transcoder: %s stream > encrypted\n", > @@ -878,14 +878,14 @@ static int _intel_hdcp_disable(struct intel_connector > *connector) > u32 repeater_ctl; > int ret; > > - drm_dbg_kms(>drm, "[%s:%d] HDCP is being disabled...\n", > - connector->base.name, connector->base.base.id); > + drm_dbg_kms(>drm, "[CONNECTOR:%d:%s] HDCP is being disabled...\n", > + connector->base.base.id, connector->base.name); > > if (hdcp->shim->stream_encryption) { > ret = hdcp->shim->stream_encryption(connector, false); > if (ret) { > - drm_err(>drm, "[%s:%d] Failed to disable HDCP 1.4 > stream enc\n", > - connector->base.name, connector->base.base.id); > + drm_err(>drm, "[CONNECTOR:%d:%s] Failed to > disable HDCP 1.4 stream enc\n", > + connector->base.base.id, connector->base.name); > return ret; > } > drm_dbg_kms(>drm, "HDCP 1.4 transcoder: %s stream > encryption disabled\n", > @@ -929,8 +929,8 @@ static int intel_hdcp1_enable(struct intel_connector > *connector) > struct intel_hdcp *hdcp = >hdcp; > int i, ret, tries = 3; > > - drm_dbg_kms(>drm, "[%s:%d] HDCP is being enabled...\n", > - connector->base.name, connector->base.base.id); > + drm_dbg_kms(>drm, "[CONNECTOR:%d:%s] HDCP is being enabled...\n", > + connector->base.base.id, connector->base.name); > > if (!hdcp_key_loadable(i915)) { > drm_err(>drm, "HDCP key Load is not possible\n"); > @@ -1027,8 +1027,8 @@ static int intel_hdcp_check_link(struct intel_connector > *connector) > if (drm_WARN_ON(>drm, > !intel_hdcp_in_use(i915, cpu_transcoder, port))) { > drm_err(>drm, > - "%s:%d HDCP link stopped encryption,%x\n", > - connector->base.name, connector->base.base.id, > + "[CONNECTOR:%d:%s] HDCP link stopped encryption,%x\n", > + connector->base.base.id, connector->base.name, > intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, > port))); > ret = -ENXIO; > intel_hdcp_update_value(connector, > @@ -1046,8 +1046,8 @@ static int intel_hdcp_check_link(struct intel_connector > *connector) > } > > drm_dbg_kms(>drm, > - "[%s:%d] HDCP link failed, retrying authentication\n", > - connector->base.name, connector->base.base.id); > + "[CONNECTOR:%d:%s] HDCP link failed, retrying > authentication\n", > + connector->base.base.id, connector->base.name); > > ret = _intel_hdcp_disable(connector); > if (ret) { > @@ -1731,8 +1731,8 @@ static int hdcp2_enable_stream_encryption(struct > intel_connector *connector) > > if (!(intel_de_read(i915, HDCP2_STATUS(i915, cpu_transcoder, port)) & > LINK_ENCRYPTION_STATUS)) { > - drm_err(>drm, "[%s:%d] HDCP 2.2 Link is not encrypted\n", > - connector->base.name, connector->base.base.id); > + drm_err(>drm, "[CONNECTOR:%d:%s] HDCP 2.2 Link is not > encrypted\n", > + connector->base.base.id, connector->base.name); > ret = -EPERM; > goto link_recover; > } > @@ -1740,8 +1740,8 @@ static int hdcp2_enable_stream_encryption(struct > intel_connector *connector) > if (hdcp->shim->stream_2_2_encryption) { > ret = hdcp->shim->stream_2_2_encryption(connector, true); > if (ret)
Re: [PATCH v2 11/15] drm/i915: Split the smem and lmem plane readout apart
On Tue, Dec 19, 2023 at 11:55:01AM +0100, Andrzej Hajda wrote: > On 15.12.2023 11:59, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Declutter initial_plane_vma() a bit by pulling the lmem and smem > > readout paths into their own functions. > > > > TODO: the smem path should still be fixed to get and validate > >the dma address from the pte as well > > > > Cc: Paz Zcharya > > Signed-off-by: Ville Syrjälä > > I am not sure about this split, split suggests the paths significantly > differs, but they differ just by 2 things: > - mem region, > - assumption about 1:1 mapping for older platforms. That last one is a fairly major difference which needs actual work since the PTEs will have a different layouts. There's the obvious 8byte vs. 4byte difference for gen8+ vs. older, and also there are several variants of 4byte PTEs (don't recall off the top of my head how many exactly). > > Btw I was wondering if wouldn't be good to abstract out pte retrieval, > as the pattern "ggtt->gsm + offset / I915_GTT_PAGE_SIZE" is present in > multiple places and depends on hw gen, but maybe it is another patch. Yeah, I think I'd just prefer the gem/gt code provide a function we can call with the ggtt address, and it'll spit out the dma address for us. And if we want to go for belts and suspenders we could also have it validate that the dma addresses are contiguous. But once we have that we should probably be able to collapse this into just the single codepath. > > No strong feelings. > Reviewed-by: Andrzej Hajda > > Regards > Andrzej > > > Regards > Andrzej > > > > > > --- > > .../drm/i915/display/intel_display_types.h| 2 + > > .../drm/i915/display/intel_plane_initial.c| 145 +++--- > > 2 files changed, 95 insertions(+), 52 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index 341d80c2b9de..d2b0cc754667 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -782,6 +782,8 @@ struct intel_plane_state { > > > > struct intel_initial_plane_config { > > struct intel_framebuffer *fb; > > + struct intel_memory_region *mem; > > + resource_size_t phys_base; > > struct i915_vma *vma; > > unsigned int tiling; > > int size; > > diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c > > b/drivers/gpu/drm/i915/display/intel_plane_initial.c > > index 48b74319f45c..78bff6181ceb 100644 > > --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c > > +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c > > @@ -44,6 +44,93 @@ intel_reuse_initial_plane_obj(struct drm_i915_private > > *i915, > > return false; > > } > > > > +static bool > > +initial_plane_phys_lmem(struct drm_i915_private *i915, > > + struct intel_initial_plane_config *plane_config) > > +{ > > + gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm; > > + struct intel_memory_region *mem; > > + dma_addr_t dma_addr; > > + gen8_pte_t pte; > > + u32 base; > > + > > + base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT); > > + > > + gte += base / I915_GTT_PAGE_SIZE; > > + > > + pte = ioread64(gte); > > + if (!(pte & GEN12_GGTT_PTE_LM)) { > > + drm_err(>drm, > > + "Initial plane programming missing PTE_LM bit\n"); > > + return false; > > + } > > + > > + dma_addr = pte & GEN12_GGTT_PTE_ADDR_MASK; > > + > > + if (IS_DGFX(i915)) > > + mem = i915->mm.regions[INTEL_REGION_LMEM_0]; > > + else > > + mem = i915->mm.stolen_region; > > + if (!mem) { > > + drm_dbg_kms(>drm, > > + "Initial plane memory region not initialized\n"); > > + return false; > > + } > > + > > + /* > > +* On lmem we don't currently expect this to > > +* ever be placed in the stolen portion. > > +*/ > > + if (dma_addr < mem->region.start || dma_addr > mem->region.end) { > > + drm_err(>drm, > > + "Initial plane programming using invalid range, > > dma_addr=%pa (%s [%pa-%pa])\n", > > + _addr, mem->region.name, >region.start, > > >region.end); > > + return false; > > + } > > + > > + drm_dbg(>drm, > > + "Using dma_addr=%pa, based on initial plane programming\n", > > + _addr); > > + > > + plane_config->phys_base = dma_addr - mem->region.start; > > + plane_config->mem = mem; > > + > > + return true; > > +} > > + > > +static bool > > +initial_plane_phys_smem(struct drm_i915_private *i915, > > + struct intel_initial_plane_config *plane_config) > > +{ > > + struct intel_memory_region *mem; > > + u32 base; > > + > > + base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT); > > + > > + mem = i915->mm.stolen_region; > > + if (!mem) { > > +
[PATCH] drm/i915/display: Skip C10 state verification in case of fastset
PLL's are not programmed in case of fastset so the state verfication compares bios programmed PLL values against sw PLL values. To overcome this limitation, we can skip the state verification for C10 in fastset case as the driver is not writing PLL values. Signed-off-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c index 884a1da36089..3ef54eaca9e4 100644 --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c @@ -3016,6 +3016,9 @@ static void intel_c10pll_state_verify(const struct intel_crtc_state *state, const struct intel_c10pll_state *mpllb_sw_state = >cx0pll_state.c10; int i; + if (intel_crtc_needs_fastset(state)) + return; + for (i = 0; i < ARRAY_SIZE(mpllb_sw_state->pll); i++) { u8 expected = mpllb_sw_state->pll[i]; -- 2.34.1
Re: [PATCH v5 00/20] remove I2C_CLASS_DDC support
> I created an immutable branch for this which the buildbots will > hopefully check over night. I will reply with comments tomorrow when I > got the buildbot results. Applied to for-next, thanks! signature.asc Description: PGP signature
✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/hdcp: unify connector logging format
== Series Details == Series: series starting with [1/2] drm/i915/hdcp: unify connector logging format URL : https://patchwork.freedesktop.org/series/127965/ State : success == Summary == CI Bug Log - changes from CI_DRM_14042 -> Patchwork_127965v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/index.html Participating hosts (35 -> 37) -- Additional (3): bat-adlp-11 bat-mtlp-8 fi-pnv-d510 Missing(1): fi-snb-2520m Known issues Here are the changes found in Patchwork_127965v1 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - bat-mtlp-8: NOTRUN -> [SKIP][1] ([i915#9318]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html - bat-adlp-11:NOTRUN -> [SKIP][2] ([i915#9318]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-adlp-11/igt@debugfs_t...@basic-hwmon.html * igt@fbdev@info: - bat-adlp-11:NOTRUN -> [SKIP][3] ([i915#1849] / [i915#2582]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-adlp-11/igt@fb...@info.html * igt@fbdev@nullptr: - bat-adlp-11:NOTRUN -> [SKIP][4] ([i915#2582]) +3 other tests skip [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-adlp-11/igt@fb...@nullptr.html * igt@gem_exec_suspend@basic-s0@lmem0: - bat-dg2-9: [PASS][5] -> [INCOMPLETE][6] ([i915#9275]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/bat-dg2-9/igt@gem_exec_suspend@basic...@lmem0.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-dg2-9/igt@gem_exec_suspend@basic...@lmem0.html * igt@gem_lmem_swapping@basic: - fi-pnv-d510:NOTRUN -> [SKIP][7] ([fdo#109271]) +28 other tests skip [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/fi-pnv-d510/igt@gem_lmem_swapp...@basic.html * igt@gem_lmem_swapping@verify-random: - bat-adlp-11:NOTRUN -> [SKIP][8] ([i915#4613]) +3 other tests skip [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-adlp-11/igt@gem_lmem_swapp...@verify-random.html - bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#4613]) +3 other tests skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_mmap@basic: - bat-mtlp-8: NOTRUN -> [SKIP][10] ([i915#4083]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-mtlp-8/igt@gem_m...@basic.html * igt@gem_mmap_gtt@basic: - bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#4077]) +2 other tests skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-mtlp-8/igt@gem_mmap_...@basic.html * igt@gem_render_tiled_blits@basic: - bat-mtlp-8: NOTRUN -> [SKIP][12] ([i915#4079]) +1 other test skip [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-adlp-11:NOTRUN -> [SKIP][13] ([i915#3282]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-adlp-11/igt@gem_tiled_pread_basic.html * igt@i915_pm_rps@basic-api: - bat-adlp-11:NOTRUN -> [SKIP][14] ([i915#6621]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-adlp-11/igt@i915_pm_...@basic-api.html - bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#6621]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-mtlp-8/igt@i915_pm_...@basic-api.html * igt@i915_suspend@basic-s3-without-i915: - bat-atsm-1: NOTRUN -> [SKIP][16] ([i915#6645]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-atsm-1/igt@i915_susp...@basic-s3-without-i915.html - bat-mtlp-8: NOTRUN -> [SKIP][17] ([i915#6645]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-mtlp-8/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy: - bat-mtlp-8: NOTRUN -> [SKIP][18] ([i915#5190]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-mtlp-8/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html * igt@kms_addfb_basic@basic-y-tiled-legacy: - bat-mtlp-8: NOTRUN -> [SKIP][19] ([i915#4212]) +8 other tests skip [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127965v1/bat-mtlp-8/igt@kms_addfb_ba...@basic-y-tiled-legacy.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - bat-adlp-11:NOTRUN -> [SKIP][20] ([i915#5608] / [i915#9866]) +1 other test skip [20]:
✓ Fi.CI.BAT: success for drm/i915: Cursor vblank evasion
== Series Details == Series: drm/i915: Cursor vblank evasion URL : https://patchwork.freedesktop.org/series/127744/ State : success == Summary == CI Bug Log - changes from CI_DRM_14017 -> Patchwork_127744v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/index.html Participating hosts (37 -> 37) -- Additional (1): bat-rpls-2 Missing(1): fi-snb-2520m Known issues Here are the changes found in Patchwork_127744v1 that come from known issues: ### CI changes ### Issues hit * boot: - bat-jsl-1: [PASS][1] -> [FAIL][2] ([i915#8293]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14017/bat-jsl-1/boot.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/bat-jsl-1/boot.html ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - bat-rpls-2: NOTRUN -> [SKIP][3] ([i915#9318]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/bat-rpls-2/igt@debugfs_t...@basic-hwmon.html * igt@gem_tiled_pread_basic: - bat-rpls-2: NOTRUN -> [SKIP][4] ([i915#3282]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/bat-rpls-2/igt@gem_tiled_pread_basic.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - bat-rpls-2: NOTRUN -> [SKIP][5] ([i915#4103]) +1 other test skip [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/bat-rpls-2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html * igt@kms_dsc@dsc-basic: - bat-rpls-2: NOTRUN -> [SKIP][6] ([i915#3555] / [i915#3840]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/bat-rpls-2/igt@kms_...@dsc-basic.html * igt@kms_force_connector_basic@force-load-detect: - bat-rpls-2: NOTRUN -> [SKIP][7] ([fdo#109285]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/bat-rpls-2/igt@kms_force_connector_ba...@force-load-detect.html * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1: - bat-rplp-1: [PASS][8] -> [ABORT][9] ([i915#8668]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14017/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html * igt@kms_pm_backlight@basic-brightness: - bat-rpls-2: NOTRUN -> [SKIP][10] ([i915#5354]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/bat-rpls-2/igt@kms_pm_backli...@basic-brightness.html * igt@kms_pm_rpm@basic-rte: - bat-rpls-2: NOTRUN -> [ABORT][11] ([i915#8668]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v1/bat-rpls-2/igt@kms_pm_...@basic-rte.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285 [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282 [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555 [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840 [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103 [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354 [i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359 [i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293 [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668 [i915#8981]: https://gitlab.freedesktop.org/drm/intel/issues/8981 [i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318 Build changes - * Linux: CI_DRM_14017 -> Patchwork_127744v1 CI-20190529: 20190529 CI_DRM_14017: 58ac4ffc75b62e6007e85ae6777820e77c113b01 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7639: 18afc09e362b605a3c88c000331708f105d2c23a @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_127744v1: 58ac4ffc75b62e6007e85ae6777820e77c113b01 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits e1c277f9ae55 Revert "drm/i915/xe2lpd: Treat cursor plane as regular plane for DDB allocation" b72675c28b05 drm/i915: Perform vblank evasion around legacy cursor updates 3aaf80b59334 drm/i915: Move intel_vblank_evade() & co. into intel_vblank.c 1f183fe092b6 drm/i915: Move the min/max scanline sanity check into intel_vblank_evade() 2af5d0bdcc34 drm/i915: Extract intel_vblank_evade() fa583f9032ef drm/i915: Include need_vlv_dsi_wa in intel_vblank_evade_ctx 235770e8ab3a drm/i915: Introduce struct intel_vblank_evade_ctx 74deb7624175 drm/i915: Reorder drm_vblank_put() vs. need_vlv_dsi_wa b2859e2d5714 drm/i915: Decouple intel_crtc_vblank_evade_scanlines() from
✗ Fi.CI.BAT: failure for drm/i915/mtl: Add fake PCH for Meteor Lake
== Series Details == Series: drm/i915/mtl: Add fake PCH for Meteor Lake URL : https://patchwork.freedesktop.org/series/127963/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14042 -> Patchwork_127963v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_127963v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_127963v1, please notify your bug team (i915-ci-in...@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/index.html Participating hosts (35 -> 37) -- Additional (3): bat-kbl-2 bat-adlp-11 bat-mtlp-8 Missing(1): fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_127963v1: ### IGT changes ### Possible regressions * igt@i915_suspend@basic-s2idle-without-i915: - fi-kbl-7567u: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-kbl-7567u/igt@i915_susp...@basic-s2idle-without-i915.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/fi-kbl-7567u/igt@i915_susp...@basic-s2idle-without-i915.html Known issues Here are the changes found in Patchwork_127963v1 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - bat-mtlp-8: NOTRUN -> [SKIP][3] ([i915#9318]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html - bat-adlp-11:NOTRUN -> [SKIP][4] ([i915#9318]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-adlp-11/igt@debugfs_t...@basic-hwmon.html * igt@fbdev@info: - bat-adlp-11:NOTRUN -> [SKIP][5] ([i915#1849] / [i915#2582]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-adlp-11/igt@fb...@info.html - bat-kbl-2: NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#1849]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-kbl-2/igt@fb...@info.html * igt@fbdev@nullptr: - bat-adlp-11:NOTRUN -> [SKIP][7] ([i915#2582]) +3 other tests skip [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-adlp-11/igt@fb...@nullptr.html * igt@gem_lmem_swapping@parallel-random-engines: - bat-kbl-2: NOTRUN -> [SKIP][8] ([fdo#109271]) +36 other tests skip [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@gem_lmem_swapping@verify-random: - bat-adlp-11:NOTRUN -> [SKIP][9] ([i915#4613]) +3 other tests skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-adlp-11/igt@gem_lmem_swapp...@verify-random.html - bat-mtlp-8: NOTRUN -> [SKIP][10] ([i915#4613]) +3 other tests skip [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_mmap@basic: - bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#4083]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-mtlp-8/igt@gem_m...@basic.html * igt@gem_mmap_gtt@basic: - bat-mtlp-8: NOTRUN -> [SKIP][12] ([i915#4077]) +2 other tests skip [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-mtlp-8/igt@gem_mmap_...@basic.html * igt@gem_render_tiled_blits@basic: - bat-mtlp-8: NOTRUN -> [SKIP][13] ([i915#4079]) +1 other test skip [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-adlp-11:NOTRUN -> [SKIP][14] ([i915#3282]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-adlp-11/igt@gem_tiled_pread_basic.html * igt@i915_pm_rps@basic-api: - bat-adlp-11:NOTRUN -> [SKIP][15] ([i915#6621]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-adlp-11/igt@i915_pm_...@basic-api.html - bat-mtlp-8: NOTRUN -> [SKIP][16] ([i915#6621]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-mtlp-8/igt@i915_pm_...@basic-api.html * igt@i915_suspend@basic-s3-without-i915: - bat-atsm-1: NOTRUN -> [SKIP][17] ([i915#6645]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-atsm-1/igt@i915_susp...@basic-s3-without-i915.html - bat-mtlp-8: NOTRUN -> [SKIP][18] ([i915#6645]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127963v1/bat-mtlp-8/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy: -
✗ Fi.CI.SPARSE: warning for drm/i915/mtl: Add fake PCH for Meteor Lake
== Series Details == Series: drm/i915/mtl: Add fake PCH for Meteor Lake URL : https://patchwork.freedesktop.org/series/127963/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
✗ Fi.CI.BAT: failure for Enable Adaptive Sync SDP Support for DP (rev5)
== Series Details == Series: Enable Adaptive Sync SDP Support for DP (rev5) URL : https://patchwork.freedesktop.org/series/126829/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14042 -> Patchwork_126829v5 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_126829v5 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_126829v5, please notify your bug team (i915-ci-in...@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/index.html Participating hosts (35 -> 35) -- Additional (2): bat-kbl-2 fi-pnv-d510 Missing(2): bat-rpls-2 fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_126829v5: ### IGT changes ### Possible regressions * igt@gem_ctx_create@basic-files: - fi-rkl-11600: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-rkl-11600/igt@gem_ctx_cre...@basic-files.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/fi-rkl-11600/igt@gem_ctx_cre...@basic-files.html * igt@i915_module_load@load: - fi-bsw-n3050: [PASS][3] -> [ABORT][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-bsw-n3050/igt@i915_module_l...@load.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/fi-bsw-n3050/igt@i915_module_l...@load.html - fi-elk-e7500: [PASS][5] -> [INCOMPLETE][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-elk-e7500/igt@i915_module_l...@load.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/fi-elk-e7500/igt@i915_module_l...@load.html - fi-bsw-nick:[PASS][7] -> [INCOMPLETE][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-bsw-nick/igt@i915_module_l...@load.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/fi-bsw-nick/igt@i915_module_l...@load.html * igt@kms_hdmi_inject@inject-audio: - fi-ilk-650: [PASS][9] -> [INCOMPLETE][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/fi-ilk-650/igt@kms_hdmi_inj...@inject-audio.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/fi-ilk-650/igt@kms_hdmi_inj...@inject-audio.html Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_module_load@load: - {bat-rpls-3}: [PASS][11] -> [ABORT][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/bat-rpls-3/igt@i915_module_l...@load.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/bat-rpls-3/igt@i915_module_l...@load.html Known issues Here are the changes found in Patchwork_126829v5 that come from known issues: ### CI changes ### Issues hit * boot: - bat-jsl-1: [PASS][13] -> [FAIL][14] ([i915#8293]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/bat-jsl-1/boot.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/bat-jsl-1/boot.html ### IGT changes ### Issues hit * igt@fbdev@info: - bat-kbl-2: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#1849]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/bat-kbl-2/igt@fb...@info.html * igt@gem_exec_suspend@basic-s0@smem: - bat-dg2-9: [PASS][16] -> [INCOMPLETE][17] ([i915#9275]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14042/bat-dg2-9/igt@gem_exec_suspend@basic...@smem.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/bat-dg2-9/igt@gem_exec_suspend@basic...@smem.html * igt@gem_lmem_swapping@basic: - fi-pnv-d510:NOTRUN -> [SKIP][18] ([fdo#109271]) +28 other tests skip [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/fi-pnv-d510/igt@gem_lmem_swapp...@basic.html * igt@gem_lmem_swapping@parallel-random-engines: - bat-kbl-2: NOTRUN -> [SKIP][19] ([fdo#109271]) +36 other tests skip [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@i915_suspend@basic-s3-without-i915: - bat-atsm-1: NOTRUN -> [SKIP][20] ([i915#6645]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/bat-atsm-1/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_pipe_crc_basic@suspend-read-crc: - bat-atsm-1: NOTRUN -> [SKIP][21] ([i915#1836]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v5/bat-atsm-1/igt@kms_pipe_crc_ba...@suspend-read-crc.html
Re: [PATCH v2 12/15] drm/i915: Simplify intel_initial_plane_config() calling convention
On 15.12.2023 11:59, Ville Syrjala wrote: From: Ville Syrjälä There's no reason the caller of intel_initial_plane_config() should have to loop over the CRTCs. Pull the loop into the function to make life simpler for the caller. Cc: Paz Zcharya Signed-off-by: Ville Syrjälä Reviewed-by: Andrzej Hajda Regards Andrzej --- .../drm/i915/display/intel_display_driver.c | 7 +--- .../drm/i915/display/intel_plane_initial.c| 40 +++ .../drm/i915/display/intel_plane_initial.h| 4 +- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c index 62f7b10484be..2fe0f4ad359c 100644 --- a/drivers/gpu/drm/i915/display/intel_display_driver.c +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c @@ -285,7 +285,6 @@ int intel_display_driver_probe_nogem(struct drm_i915_private *i915) { struct drm_device *dev = >drm; enum pipe pipe; - struct intel_crtc *crtc; int ret; if (!HAS_DISPLAY(i915)) @@ -335,11 +334,7 @@ int intel_display_driver_probe_nogem(struct drm_i915_private *i915) intel_acpi_assign_connector_fwnodes(i915); drm_modeset_unlock_all(dev); - for_each_intel_crtc(dev, crtc) { - if (!to_intel_crtc_state(crtc->base.state)->uapi.active) - continue; - intel_crtc_initial_plane_config(crtc); - } + intel_initial_plane_config(i915); /* * Make sure hardware watermarks really match the state we read out. diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c index 78bff6181ceb..b7e12b60d68b 100644 --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c @@ -357,25 +357,31 @@ static void plane_config_fini(struct intel_initial_plane_config *plane_config) i915_vma_put(plane_config->vma); } -void intel_crtc_initial_plane_config(struct intel_crtc *crtc) +void intel_initial_plane_config(struct drm_i915_private *i915) { - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - struct intel_initial_plane_config plane_config = {}; + struct intel_crtc *crtc; - /* -* Note that reserving the BIOS fb up front prevents us -* from stuffing other stolen allocations like the ring -* on top. This prevents some ugliness at boot time, and -* can even allow for smooth boot transitions if the BIOS -* fb is large enough for the active pipe configuration. -*/ - dev_priv->display.funcs.display->get_initial_plane_config(crtc, _config); + for_each_intel_crtc(>drm, crtc) { + struct intel_initial_plane_config plane_config = {}; - /* -* If the fb is shared between multiple heads, we'll -* just get the first one. -*/ - intel_find_initial_plane_obj(crtc, _config); + if (!to_intel_crtc_state(crtc->base.state)->uapi.active) + continue; - plane_config_fini(_config); + /* +* Note that reserving the BIOS fb up front prevents us +* from stuffing other stolen allocations like the ring +* on top. This prevents some ugliness at boot time, and +* can even allow for smooth boot transitions if the BIOS +* fb is large enough for the active pipe configuration. +*/ + i915->display.funcs.display->get_initial_plane_config(crtc, _config); + + /* +* If the fb is shared between multiple heads, we'll +* just get the first one. +*/ + intel_find_initial_plane_obj(crtc, _config); + + plane_config_fini(_config); + } } diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.h b/drivers/gpu/drm/i915/display/intel_plane_initial.h index c7e35ab3182b..64ab95239cd4 100644 --- a/drivers/gpu/drm/i915/display/intel_plane_initial.h +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.h @@ -6,8 +6,8 @@ #ifndef __INTEL_PLANE_INITIAL_H__ #define __INTEL_PLANE_INITIAL_H__ -struct intel_crtc; +struct drm_i915_private; -void intel_crtc_initial_plane_config(struct intel_crtc *crtc); +void intel_initial_plane_config(struct drm_i915_private *i915); #endif
Re: [PATCH v2 09/15] drm/i915: Fix MTL initial plane readout
On 15.12.2023 11:59, Ville Syrjala wrote: From: Ville Syrjälä MTL stolen memory looks more like local memory, so use the (now fixed) lmem path when doing the initial plane readout. Cc: Paz Zcharya Signed-off-by: Ville Syrjälä --- .../drm/i915/display/intel_plane_initial.c| 25 +-- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c index db594ccf0323..c72d4cacf631 100644 --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c @@ -59,7 +59,7 @@ initial_plane_vma(struct drm_i915_private *i915, return NULL; base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT); - if (IS_DGFX(i915)) { + if (IS_DGFX(i915) || HAS_LMEMBAR_SMEM_STOLEN(i915)) { gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm; gen8_pte_t pte; @@ -73,11 +73,20 @@ initial_plane_vma(struct drm_i915_private *i915, } phys_base = pte & GEN12_GGTT_PTE_ADDR_MASK; - mem = i915->mm.regions[INTEL_REGION_LMEM_0]; + + if (IS_DGFX(i915)) + mem = i915->mm.regions[INTEL_REGION_LMEM_0]; + else + mem = i915->mm.stolen_region; + if (!mem) { + drm_dbg_kms(>drm, + "Initial plane memory region not initialized\n"); + return NULL; + } /* -* We don't currently expect this to ever be placed in the -* stolen portion. +* On lmem we don't currently expect this to +* ever be placed in the stolen portion. */ if (phys_base < mem->region.start || phys_base > mem->region.end) { drm_err(>drm, @@ -94,11 +103,13 @@ initial_plane_vma(struct drm_i915_private *i915, } else { phys_base = base; mem = i915->mm.stolen_region; + if (!mem) { + drm_dbg_kms(>drm, + "Initial plane memory region not initialized\n"); + return NULL; + } Code duplication suggests, we could try to move this out ifs. The extra check should be harmless in case of 1:1. Regards Andrzej } - if (!mem) - return NULL; - size = round_up(plane_config->base + plane_config->size, mem->min_page_size); size -= base;
Re: [PATCH v2 10/15] drm/i915: s/phys_base/dma_addr/
On 15.12.2023 11:59, Ville Syrjala wrote: From: Ville Syrjälä The address we read from the PTE is a dma address, not a physical address. Rename the variable to say so. Cc: Paz Zcharya Signed-off-by: Ville Syrjälä Reviewed-by: Andrzej Hajda Regards Andrzej --- .../gpu/drm/i915/display/intel_plane_initial.c| 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c index c72d4cacf631..48b74319f45c 100644 --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c @@ -61,6 +61,7 @@ initial_plane_vma(struct drm_i915_private *i915, base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT); if (IS_DGFX(i915) || HAS_LMEMBAR_SMEM_STOLEN(i915)) { gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm; + dma_addr_t dma_addr; gen8_pte_t pte; gte += base / I915_GTT_PAGE_SIZE; @@ -72,7 +73,7 @@ initial_plane_vma(struct drm_i915_private *i915, return NULL; } - phys_base = pte & GEN12_GGTT_PTE_ADDR_MASK; + dma_addr = pte & GEN12_GGTT_PTE_ADDR_MASK; if (IS_DGFX(i915)) mem = i915->mm.regions[INTEL_REGION_LMEM_0]; @@ -88,18 +89,18 @@ initial_plane_vma(struct drm_i915_private *i915, * On lmem we don't currently expect this to * ever be placed in the stolen portion. */ - if (phys_base < mem->region.start || phys_base > mem->region.end) { + if (dma_addr < mem->region.start || dma_addr > mem->region.end) { drm_err(>drm, - "Initial plane programming using invalid range, phys_base=%pa (%s [%pa-%pa])\n", - _base, mem->region.name, >region.start, >region.end); + "Initial plane programming using invalid range, dma_addr=%pa (%s [%pa-%pa])\n", + _addr, mem->region.name, >region.start, >region.end); return NULL; } drm_dbg(>drm, - "Using phys_base=%pa, based on initial plane programming\n", - _base); + "Using dma_addr=%pa, based on initial plane programming\n", + _addr); - phys_base -= mem->region.start; + phys_base = dma_addr - mem->region.start; } else { phys_base = base; mem = i915->mm.stolen_region;
Re: [PATCH v2 11/15] drm/i915: Split the smem and lmem plane readout apart
On 15.12.2023 11:59, Ville Syrjala wrote: From: Ville Syrjälä Declutter initial_plane_vma() a bit by pulling the lmem and smem readout paths into their own functions. TODO: the smem path should still be fixed to get and validate the dma address from the pte as well Cc: Paz Zcharya Signed-off-by: Ville Syrjälä I am not sure about this split, split suggests the paths significantly differs, but they differ just by 2 things: - mem region, - assumption about 1:1 mapping for older platforms. Btw I was wondering if wouldn't be good to abstract out pte retrieval, as the pattern "ggtt->gsm + offset / I915_GTT_PAGE_SIZE" is present in multiple places and depends on hw gen, but maybe it is another patch. No strong feelings. Reviewed-by: Andrzej Hajda Regards Andrzej Regards Andrzej --- .../drm/i915/display/intel_display_types.h| 2 + .../drm/i915/display/intel_plane_initial.c| 145 +++--- 2 files changed, 95 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 341d80c2b9de..d2b0cc754667 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -782,6 +782,8 @@ struct intel_plane_state { struct intel_initial_plane_config { struct intel_framebuffer *fb; + struct intel_memory_region *mem; + resource_size_t phys_base; struct i915_vma *vma; unsigned int tiling; int size; diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c index 48b74319f45c..78bff6181ceb 100644 --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c @@ -44,6 +44,93 @@ intel_reuse_initial_plane_obj(struct drm_i915_private *i915, return false; } +static bool +initial_plane_phys_lmem(struct drm_i915_private *i915, + struct intel_initial_plane_config *plane_config) +{ + gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm; + struct intel_memory_region *mem; + dma_addr_t dma_addr; + gen8_pte_t pte; + u32 base; + + base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT); + + gte += base / I915_GTT_PAGE_SIZE; + + pte = ioread64(gte); + if (!(pte & GEN12_GGTT_PTE_LM)) { + drm_err(>drm, + "Initial plane programming missing PTE_LM bit\n"); + return false; + } + + dma_addr = pte & GEN12_GGTT_PTE_ADDR_MASK; + + if (IS_DGFX(i915)) + mem = i915->mm.regions[INTEL_REGION_LMEM_0]; + else + mem = i915->mm.stolen_region; + if (!mem) { + drm_dbg_kms(>drm, + "Initial plane memory region not initialized\n"); + return false; + } + + /* +* On lmem we don't currently expect this to +* ever be placed in the stolen portion. +*/ + if (dma_addr < mem->region.start || dma_addr > mem->region.end) { + drm_err(>drm, + "Initial plane programming using invalid range, dma_addr=%pa (%s [%pa-%pa])\n", + _addr, mem->region.name, >region.start, >region.end); + return false; + } + + drm_dbg(>drm, + "Using dma_addr=%pa, based on initial plane programming\n", + _addr); + + plane_config->phys_base = dma_addr - mem->region.start; + plane_config->mem = mem; + + return true; +} + +static bool +initial_plane_phys_smem(struct drm_i915_private *i915, + struct intel_initial_plane_config *plane_config) +{ + struct intel_memory_region *mem; + u32 base; + + base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT); + + mem = i915->mm.stolen_region; + if (!mem) { + drm_dbg_kms(>drm, + "Initial plane memory region not initialized\n"); + return false; + } + + /* FIXME get and validate the dma_addr from the PTE */ + plane_config->phys_base = base; + plane_config->mem = mem; + + return true; +} + +static bool +initial_plane_phys(struct drm_i915_private *i915, + struct intel_initial_plane_config *plane_config) +{ + if (IS_DGFX(i915) || HAS_LMEMBAR_SMEM_STOLEN(i915)) + return initial_plane_phys_lmem(i915, plane_config); + else + return initial_plane_phys_smem(i915, plane_config); +} + static struct i915_vma * initial_plane_vma(struct drm_i915_private *i915, struct intel_initial_plane_config *plane_config) @@ -58,59 +145,13 @@ initial_plane_vma(struct drm_i915_private *i915, if (plane_config->size == 0) return NULL; + if (!initial_plane_phys(i915,
[PATCH 2/2] drm/i915/hdcp: fix intel_hdcp_get_repeater_ctl() error return value
intel_hdcp_get_repeater_ctl() is supposed to return unsigned register contents. Returning negative error values is unexpected, and none of the callers check for that. Sort of fix the error cases by returning 0. I don't think we should hit these cases anyway, and using 0 for the registers is safer than 0xffea (-EINVAL). Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_hdcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index f9010094ff29..ee29fcb860e4 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -347,7 +347,7 @@ u32 intel_hdcp_get_repeater_ctl(struct drm_i915_private *i915, default: drm_err(>drm, "Unknown transcoder %d\n", cpu_transcoder); - return -EINVAL; + return 0; } } @@ -364,7 +364,7 @@ u32 intel_hdcp_get_repeater_ctl(struct drm_i915_private *i915, return HDCP_DDIE_REP_PRESENT | HDCP_DDIE_SHA1_M0; default: drm_err(>drm, "Unknown port %d\n", port); - return -EINVAL; + return 0; } } -- 2.39.2
✗ Fi.CI.SPARSE: warning for Enable Adaptive Sync SDP Support for DP (rev5)
== Series Details == Series: Enable Adaptive Sync SDP Support for DP (rev5) URL : https://patchwork.freedesktop.org/series/126829/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
[PATCH 1/2] drm/i915/hdcp: unify connector logging format
It's customary to debug log connectors using [CONNECTOR:%d:%s] format. Make the HDCP code follow suit. Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_hdcp.c | 68 +++ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 39b3f7c0c77c..f9010094ff29 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -853,8 +853,8 @@ static int intel_hdcp_auth(struct intel_connector *connector) if (shim->stream_encryption) { ret = shim->stream_encryption(connector, true); if (ret) { - drm_err(>drm, "[%s:%d] Failed to enable HDCP 1.4 stream enc\n", - connector->base.name, connector->base.base.id); + drm_err(>drm, "[CONNECTOR:%d:%s] Failed to enable HDCP 1.4 stream enc\n", + connector->base.base.id, connector->base.name); return ret; } drm_dbg_kms(>drm, "HDCP 1.4 transcoder: %s stream encrypted\n", @@ -878,14 +878,14 @@ static int _intel_hdcp_disable(struct intel_connector *connector) u32 repeater_ctl; int ret; - drm_dbg_kms(>drm, "[%s:%d] HDCP is being disabled...\n", - connector->base.name, connector->base.base.id); + drm_dbg_kms(>drm, "[CONNECTOR:%d:%s] HDCP is being disabled...\n", + connector->base.base.id, connector->base.name); if (hdcp->shim->stream_encryption) { ret = hdcp->shim->stream_encryption(connector, false); if (ret) { - drm_err(>drm, "[%s:%d] Failed to disable HDCP 1.4 stream enc\n", - connector->base.name, connector->base.base.id); + drm_err(>drm, "[CONNECTOR:%d:%s] Failed to disable HDCP 1.4 stream enc\n", + connector->base.base.id, connector->base.name); return ret; } drm_dbg_kms(>drm, "HDCP 1.4 transcoder: %s stream encryption disabled\n", @@ -929,8 +929,8 @@ static int intel_hdcp1_enable(struct intel_connector *connector) struct intel_hdcp *hdcp = >hdcp; int i, ret, tries = 3; - drm_dbg_kms(>drm, "[%s:%d] HDCP is being enabled...\n", - connector->base.name, connector->base.base.id); + drm_dbg_kms(>drm, "[CONNECTOR:%d:%s] HDCP is being enabled...\n", + connector->base.base.id, connector->base.name); if (!hdcp_key_loadable(i915)) { drm_err(>drm, "HDCP key Load is not possible\n"); @@ -1027,8 +1027,8 @@ static int intel_hdcp_check_link(struct intel_connector *connector) if (drm_WARN_ON(>drm, !intel_hdcp_in_use(i915, cpu_transcoder, port))) { drm_err(>drm, - "%s:%d HDCP link stopped encryption,%x\n", - connector->base.name, connector->base.base.id, + "[CONNECTOR:%d:%s] HDCP link stopped encryption,%x\n", + connector->base.base.id, connector->base.name, intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, port))); ret = -ENXIO; intel_hdcp_update_value(connector, @@ -1046,8 +1046,8 @@ static int intel_hdcp_check_link(struct intel_connector *connector) } drm_dbg_kms(>drm, - "[%s:%d] HDCP link failed, retrying authentication\n", - connector->base.name, connector->base.base.id); + "[CONNECTOR:%d:%s] HDCP link failed, retrying authentication\n", + connector->base.base.id, connector->base.name); ret = _intel_hdcp_disable(connector); if (ret) { @@ -1731,8 +1731,8 @@ static int hdcp2_enable_stream_encryption(struct intel_connector *connector) if (!(intel_de_read(i915, HDCP2_STATUS(i915, cpu_transcoder, port)) & LINK_ENCRYPTION_STATUS)) { - drm_err(>drm, "[%s:%d] HDCP 2.2 Link is not encrypted\n", - connector->base.name, connector->base.base.id); + drm_err(>drm, "[CONNECTOR:%d:%s] HDCP 2.2 Link is not encrypted\n", + connector->base.base.id, connector->base.name); ret = -EPERM; goto link_recover; } @@ -1740,8 +1740,8 @@ static int hdcp2_enable_stream_encryption(struct intel_connector *connector) if (hdcp->shim->stream_2_2_encryption) { ret = hdcp->shim->stream_2_2_encryption(connector, true); if (ret) { - drm_err(>drm, "[%s:%d] Failed to enable HDCP 2.2 stream enc\n", - connector->base.name, connector->base.base.id); +
[PATCH] drm/i915/mtl: Add fake PCH for Meteor Lake
Correct the implementation trying to detect MTL PCH with the MTL fake PCH id. On MTL, both the North Display (NDE) and South Display (SDE) functionality reside on the same die (the SoC die in this case), unlike many past platforms where the SDE was on a separate PCH die. The code is (badly) structured today in a way that assumes the SDE is always on the PCH for modern platforms, so on platforms where we don't actually need to identify the PCH to figure out how the SDE behaves (i.e., all DG1/2 GPUs as well as MTL and LNL),we've been assigning a "fake PCH" as a quickhack that allows us to avoid restructuring a bunch of the code.we've been assigning a "fake PCH" as a quick hack that allows us to avoid restructuring a bunch of the code. Removed unused macros of LNL amd MTL as well. Signed-off-by: Haridhar Kalvala --- drivers/gpu/drm/i915/display/intel_backlight.c | 2 +- drivers/gpu/drm/i915/display/intel_bios.c| 3 +-- drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +- drivers/gpu/drm/i915/display/intel_display_irq.c | 2 +- drivers/gpu/drm/i915/display/intel_gmbus.c | 5 + drivers/gpu/drm/i915/display/intel_hotplug_irq.c | 6 ++ drivers/gpu/drm/i915/display/intel_pps.c | 2 +- drivers/gpu/drm/i915/soc/intel_pch.c | 16 drivers/gpu/drm/i915/soc/intel_pch.h | 6 +- 9 files changed, 17 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c index 612d4cd9dacb..696ae59874a9 100644 --- a/drivers/gpu/drm/i915/display/intel_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_backlight.c @@ -1465,7 +1465,7 @@ static bool cnp_backlight_controller_is_valid(struct drm_i915_private *i915, int if (controller == 1 && INTEL_PCH_TYPE(i915) >= PCH_ICP && - INTEL_PCH_TYPE(i915) < PCH_MTP) + INTEL_PCH_TYPE(i915) <= PCH_ADP) return intel_de_read(i915, SOUTH_CHICKEN1) & ICP_SECOND_PPS_IO_SELECT; return true; diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index aa169b0055e9..0e61e424802e 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -2204,8 +2204,7 @@ static u8 map_ddc_pin(struct drm_i915_private *i915, u8 vbt_pin) if (IS_DGFX(i915)) return vbt_pin; - if (INTEL_PCH_TYPE(i915) >= PCH_LNL || HAS_PCH_MTP(i915) || - IS_ALDERLAKE_P(i915)) { + if (INTEL_PCH_TYPE(i915) >= PCH_MTL || IS_ALDERLAKE_P(i915)) { ddc_pin_map = adlp_ddc_pin_map; n_entries = ARRAY_SIZE(adlp_ddc_pin_map); } else if (IS_ALDERLAKE_S(i915)) { diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index c985ebb6831a..2e6e55d3e885 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -3469,7 +3469,7 @@ u32 intel_read_rawclk(struct drm_i915_private *dev_priv) if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) freq = dg1_rawclk(dev_priv); - else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTP) + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTL) /* * MTL always uses a 38.4 MHz rawclk. The bspec tells us * "RAWCLK_FREQ defaults to the values for 38.4 and does diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c index a7d8f3fc98de..e318e24d1efd 100644 --- a/drivers/gpu/drm/i915/display/intel_display_irq.c +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c @@ -986,7 +986,7 @@ static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_i * their flags both in the PICA and SDE IIR. */ if (*pch_iir & SDE_PICAINTERRUPT) { - drm_WARN_ON(>drm, INTEL_PCH_TYPE(i915) < PCH_MTP); + drm_WARN_ON(>drm, INTEL_PCH_TYPE(i915) <= PCH_ADP); pica_ier = intel_de_rmw(i915, PICAINTERRUPT_IER, ~0, 0); *pica_iir = intel_de_read(i915, PICAINTERRUPT_IIR); diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 40d7b6f3f489..854566ba5414 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -155,7 +155,7 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915, const struct gmbus_pin *pins; size_t size; - if (INTEL_PCH_TYPE(i915) >= PCH_LNL) { + if (INTEL_PCH_TYPE(i915) >= PCH_MTL) { pins = gmbus_pins_mtp; size = ARRAY_SIZE(gmbus_pins_mtp); } else if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { @@ -164,9 +164,6 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915, } else if (INTEL_PCH_TYPE(i915) >=
Re: [PATCH] drm/i915/xelpg: Add fake PCH for xelpg
On 12/18/2023 9:24 PM, Matt Roper wrote: Oh, and one more thing I forgot to mention before hitting send...the title for this patch doesn't make sense. Xe_LPG is the graphics IP used by MTL; that's completely unrelated to the display IP (which is Xe_LPD+). Since we're assigning the fake PCH value based off the platform (IS_METEORLAKE) rather than the display version, this should just be "drm/i915/mtl: Add fake PCH for Meteor Lake" Matt Sure, I will update. On Mon, Dec 18, 2023 at 07:52:12AM -0800, Matt Roper wrote: On Mon, Dec 18, 2023 at 04:33:13PM +0530, Haridhar Kalvala wrote: Correct the implementation trying to detect MTL PCH with the MTL fake PCH id. On MTL, both the North Display (NDE) and South Display (SDE) functionality reside on the same die (the SoC die in this case), unlike many past platforms where the SDE was on a separate PCH die. The code is (badly) structured today in a way that assumes the SDE is always on the PCH for modern platforms, so on platforms where we don't actually need to identify the PCH to figure out how the SDE behaves (i.e., all DG1/2 GPUs as well as MTL and LNL),we've been assigning a "fake PCH" as a quickhack that allows us to avoid restructuring a bunch of the code.we've been assigning a "fake PCH" as a quick hack that allows us to avoid restructuring a bunch of the code. Signed-off-by: Haridhar Kalvala --- drivers/gpu/drm/i915/display/intel_backlight.c | 2 +- drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +- drivers/gpu/drm/i915/display/intel_display_irq.c | 2 +- drivers/gpu/drm/i915/display/intel_gmbus.c | 6 ++ drivers/gpu/drm/i915/display/intel_hotplug_irq.c | 7 +++ drivers/gpu/drm/i915/display/intel_pps.c | 2 +- drivers/gpu/drm/i915/soc/intel_pch.c | 12 +++- drivers/gpu/drm/i915/soc/intel_pch.h | 4 ++-- 8 files changed, 18 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c index 612d4cd9dacb..696ae59874a9 100644 --- a/drivers/gpu/drm/i915/display/intel_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_backlight.c @@ -1465,7 +1465,7 @@ static bool cnp_backlight_controller_is_valid(struct drm_i915_private *i915, int if (controller == 1 && INTEL_PCH_TYPE(i915) >= PCH_ICP && - INTEL_PCH_TYPE(i915) < PCH_MTP) + INTEL_PCH_TYPE(i915) <= PCH_ADP) return intel_de_read(i915, SOUTH_CHICKEN1) & ICP_SECOND_PPS_IO_SELECT; return true; diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index c985ebb6831a..2e6e55d3e885 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -3469,7 +3469,7 @@ u32 intel_read_rawclk(struct drm_i915_private *dev_priv) if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) freq = dg1_rawclk(dev_priv); - else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTP) + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MTL) /* * MTL always uses a 38.4 MHz rawclk. The bspec tells us * "RAWCLK_FREQ defaults to the values for 38.4 and does diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c index a7d8f3fc98de..e318e24d1efd 100644 --- a/drivers/gpu/drm/i915/display/intel_display_irq.c +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c @@ -986,7 +986,7 @@ static void gen8_read_and_ack_pch_irqs(struct drm_i915_private *i915, u32 *pch_i * their flags both in the PICA and SDE IIR. */ if (*pch_iir & SDE_PICAINTERRUPT) { - drm_WARN_ON(>drm, INTEL_PCH_TYPE(i915) < PCH_MTP); + drm_WARN_ON(>drm, INTEL_PCH_TYPE(i915) <= PCH_ADP); pica_ier = intel_de_rmw(i915, PICAINTERRUPT_IER, ~0, 0); *pica_iir = intel_de_read(i915, PICAINTERRUPT_IIR); diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 40d7b6f3f489..2d9c740ba17e 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -155,7 +155,8 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915, const struct gmbus_pin *pins; size_t size; - if (INTEL_PCH_TYPE(i915) >= PCH_LNL) { + if ((INTEL_PCH_TYPE(i915) >= PCH_LNL) || + (INTEL_PCH_TYPE(i915) >= PCH_MTL)) { You only need the MTL condition here. The LNL check becomes redundant. yeah, removed. pins = gmbus_pins_mtp; size = ARRAY_SIZE(gmbus_pins_mtp); } else if (INTEL_PCH_TYPE(i915) >= PCH_DG2) { @@ -164,9 +165,6 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *i915, } else if (INTEL_PCH_TYPE(i915) >= PCH_DG1) { pins = gmbus_pins_dg1; size =
Re: [PATCH 1/4] drm/i915/alpm: Add ALPM register definitions
On Tue, 19 Dec 2023, Jouni Högander wrote: Commit message goes here. > Signed-off-by: Jouni Högander > --- > drivers/gpu/drm/i915/display/intel_psr_regs.h | 103 ++ > 1 file changed, 103 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h > b/drivers/gpu/drm/i915/display/intel_psr_regs.h > index efe4306b37e0..9410a43e901b 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h > +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h > @@ -290,4 +290,107 @@ > _SEL_FETCH_PLANE_OFFSET_1_A - > \ > _SEL_FETCH_PLANE_BASE_1_A) > > +#define _ALPM_CTL_A 0x60950 > +#define ALPM_CTL(tran) _MMIO_TRANS2(tran, _ALPM_CTL_A) > +#define ALPM_CTL_ALPM_ENABLEBIT(31) > +#define ALPM_CTL_ALPM_AUX_LESS_ENABLE BIT(30) > +#define ALPM_CTL_LOBF_ENABLEBIT(29) > +#define ALPM_CTL_EXTENDED_FAST_WAKE_ENABLE BIT(28) > +#define ALPM_CTL_KEEP_FEC_ENABLE_FOR_AUX_WAKE_SLEEP BIT(27) > +#define ALPM_CTL_RESTORE_OCCUREDBIT(26) > +#define ALPM_CTL_RESTORE_TO_SLEEP BIT(25) > +#define ALPM_CTL_RESTORE_TO_DEEP_SLEEP BIT(24) Please use REG_BIT() throughout. > +#define ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_MASK REG_GENMASK(23, > 21) > +#define ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS > REG_FIELD_PREP(ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_MASK, 0) > +#define ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_128_SYMBOLS > REG_FIELD_PREP(ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_MASK, 1) > +#define ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_256_SYMBOLS > REG_FIELD_PREP(ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_MASK, 2) > +#define ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_512_SYMBOLS > REG_FIELD_PREP(ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_MASK, 3) > +#define ALPM_CTL_AUX_WAKE_SLEEP_HOLD_ENABLE BIT(20) > +#define ALPM_CTL_ALPM_ENTRY_CHECK_MASK REG_GENMASK(19, > 16) > +#define ALPM_CTL_ALPM_ENTRY_CHECK(val) > REG_FIELD_PREP(ALPM_CTL_ALPM_ENTRY_CHECK_MASK, val) > +#define ALPM_CTL_EXTENDED_FAST_WAKE_TIME_MASK REG_GENMASK(13, > 8) > +#define ALPM_CTL_EXTENDED_FAST_WAKE_MIN_LINES 5 > +#define ALPM_CTL_EXTENDED_FAST_WAKE_TIME(lines) > REG_FIELD_PREP(ALPM_CTL_EXTENDED_FAST_WAKE_TIME_MASK, (lines) - > ALPM_CTL_EXTENDED_FAST_WAKE_MIN_LINES) > +#define ALPM_CTL_AUX_LESS_WAKE_TIME_MASKREG_GENMASK(5, 0) > +#define ALPM_CTL_AUX_LESS_WAKE_TIME(val) > REG_FIELD_PREP(ALPM_CTL_AUX_LESS_WAKE_TIME_MASK, val) > + > +#define _ALPM_CTL2_A 0x60950 > +#define ALPM_CTL2(tran) _MMIO_TRANS2(tran, _ALPM_CTL2_A) > +#define ALPM_CTL2_SWITCH_TO_ACTIVE_LATENCY_MASK REG_GENMASK(28, > 24) > +#define ALPM_CTL2_SWITCH_TO_ACTIVE_LATENCY(val) > REG_FIELD_PREP(ALPM_CTL2_SWITCH_TO_ACTIVE_LATENCY_MASK, val) > +#define ALPM_CTL2_AUX_LESS_WAKE_TIME_EXTENSION_MASK REG_GENMASK(19, > 16) > +#define ALPM_CTL2_AUX_LESS_WAKE_TIME_EXTENSION(val) > REG_FIELD_PREP(ALPM_CTL2_AUX_LESS_WAKE_TIME_EXTENSION_MASK, val) > +#define ALPM_CTL2_NUMBER_OF_LTTPR_MASK > REG_GENMASK(15, 12) > +#define ALPM_CTL2_NUMBER_OF_LTTPR(val) > REG_FIELD_PREP(ALPM_CTL2_NUMBER_OF_LTTPR_MASK, val) > +#define ALPM_CTL2_LTTPR_AUX_LESS_SLEEP_HOLD_TIME_MASK > REG_GENMASK(10, 8) > +#define ALPM_CTL2_LTTPR_AUX_LESS_SLEEP_HOLD_TIME(val) > REG_FIELD_PREP(ALPM_CTL2_LTTPR_AUX_LESS_SLEEP_HOLD_TIME_MASK, val) > +#define ALPM_CTL2_FEC_DECODE_EN_POSITION_AFTER_WAKE_SR BIT(4) > +#define ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES_MASK > REG_GENMASK(2, 0) > +#define ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES(val) > REG_FIELD_PREP(ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES_MASK, val) > + > +#define _PORT_ALPM_CTL_A 0x16fa2c > +#define PORT_ALPM_CTL(tran) _MMIO_TRANS2(tran, > _PORT_ALPM_CTL_A) > +#define PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE BIT(31) > +#define PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK REG_GENMASK(23, 20) > +#define PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val) > REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val) > +#define PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK REG_GENMASK(19, 16) > +#define PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val) > REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK, val) > +#define PORT_ALPM_CTL_SILENCE_PERIOD_MASK REG_GENMASK(7, 0) > +#define PORT_ALPM_CTL_SILENCE_PERIOD(val) > REG_FIELD_PREP(PORT_ALPM_CTL_SILENCE_PERIOD_MASK, val) > + > +#define _PORT_ALPM_LFPS_CTL_A0x16fa30 > +#define PORT_ALPM_LFPS_CTL(tran) > _MMIO_TRANS2(tran, _PORT_ALPM_LFPS_CTL_A) > +#define
[PATCH 3/3] drm/i915/display: Compute and Enable AS SDP
Add necessary functions definitions to enable and compute AS SDP data. The new `intel_dp_compute_as_sdp` function computes AS SDP values based on the display configuration, ensuring proper handling of Variable Refresh Rate (VRR). --v2: - Add DP_SDP_ADAPTIVE_SYNC to infoframe_type_to_idx().[Ankit] - separate patch for intel_read/write_dp_sdp [Ankit]. - _HSW_VIDEO_DIP_ASYNC_DATA_A should be from ADL onward [Ankit] - To fix indentation [Ankit] --v3: - Add VIDEO_DIP_ENABLE_AS_HSW flag to intel_dp_set_infoframes. --v4: - Add HAS_VRR check before write as sdp. --v5: - Add missed HAS_VRR check before read as sdp. Signed-off-by: Mitul Golani --- drivers/gpu/drm/i915/display/intel_ddi.c | 3 +++ drivers/gpu/drm/i915/display/intel_dp.c | 23 +++ 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 38f28c480b38..ed5afaa0ce5d 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3918,6 +3918,9 @@ static void intel_ddi_get_config(struct intel_encoder *encoder, intel_read_dp_sdp(encoder, pipe_config, HDMI_PACKET_TYPE_GAMUT_METADATA); intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); + if ((DISPLAY_VER(dev_priv) >= 13) && HAS_VRR(dev_priv)) + intel_read_dp_sdp(encoder, pipe_config, DP_SDP_ADAPTIVE_SYNC); + intel_psr_get_config(encoder, pipe_config); intel_audio_codec_get_config(encoder, pipe_config); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 5dee50f812c6..0747b5bda5c8 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2630,6 +2630,25 @@ static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp, _state->infoframes.vsc); } +static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp, + struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state) +{ + struct drm_dp_as_sdp *as_sdp = _state->infoframes.as_sdp; + struct intel_connector *connector = intel_dp->attached_connector; + const struct drm_display_mode *adjusted_mode = + _state->hw.adjusted_mode; + int vrefresh = drm_mode_vrefresh(adjusted_mode); + + if (!intel_vrr_is_in_range(connector, vrefresh)) + return; + + crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC); + as_sdp->sdp_type = DP_SDP_ADAPTIVE_SYNC; + as_sdp->length = 0x9; + as_sdp->vtotal = adjusted_mode->vtotal; +} + void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state, const struct drm_connector_state *conn_state, @@ -2956,6 +2975,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, g4x_dp_set_clock(encoder, pipe_config); intel_vrr_compute_config(pipe_config, conn_state); + intel_dp_compute_as_sdp(intel_dp, pipe_config, conn_state); intel_psr_compute_config(intel_dp, pipe_config, conn_state); intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16); intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); @@ -4368,6 +4388,9 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, if (!crtc_state->has_psr) intel_write_dp_sdp(encoder, crtc_state, DP_SDP_VSC); + if ((DISPLAY_VER(dev_priv) >= 13) && HAS_VRR(dev_priv)) + intel_write_dp_sdp(encoder, crtc_state, DP_SDP_ADAPTIVE_SYNC); + intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA); } -- 2.25.1
[PATCH 2/3] drm/i915/dp: Add Read/Write support for Adaptive Sync SDP
Add the necessary structures and functions to handle reading and unpacking Adaptive Sync Secondary Data Packets. Also add support to write and pack AS SDP. --v2: - Correct use of REG_BIT and REG_GENMASK. [Jani] - Use as_sdp instead of async. [Jani] - Remove unrelated comments and changes. [Jani] - Correct code indent. [Jani] Signed-off-by: Mitul Golani --- drivers/gpu/drm/i915/display/intel_dp.c | 95 ++- drivers/gpu/drm/i915/display/intel_hdmi.c | 12 ++- drivers/gpu/drm/i915/i915_reg.h | 6 ++ include/drm/display/drm_dp_helper.h | 3 + 4 files changed, 112 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 3b2482bf683f..5dee50f812c6 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -94,7 +94,6 @@ #define INTEL_DP_RESOLUTION_STANDARD (2 << INTEL_DP_RESOLUTION_SHIFT_MASK) #define INTEL_DP_RESOLUTION_FAILSAFE (3 << INTEL_DP_RESOLUTION_SHIFT_MASK) - /* Constants for DP DSC configurations */ static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15}; @@ -4110,6 +4109,34 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state, return false; } +static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *as_sdp, + struct dp_sdp *sdp, size_t size) +{ + size_t length = sizeof(struct dp_sdp); + + if (size < length) + return -ENOSPC; + + memset(sdp, 0, size); + + /* Prepare AS (Adaptive Sync) SDP Header */ + sdp->sdp_header.HB0 = 0; + sdp->sdp_header.HB1 = as_sdp->sdp_type; + sdp->sdp_header.HB2 = 0x02; + sdp->sdp_header.HB3 = as_sdp->length; + + /* Fill AS (Adaptive Sync) SDP Payload */ + sdp->db[1] = 0x0; + sdp->db[1] = as_sdp->vtotal & 0xFF; + sdp->db[2] = (as_sdp->vtotal >> 8) & 0xF; + sdp->db[3] = 0x0; + sdp->db[4] = 0x0; + sdp->db[7] = 0x0; + sdp->db[8] = 0x0; + + return length; +} + static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct dp_sdp *sdp, size_t size) { @@ -4277,6 +4304,10 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder, _state->infoframes.drm.drm, , sizeof(sdp)); break; + case DP_SDP_ADAPTIVE_SYNC: + len = intel_dp_as_sdp_pack(_state->infoframes.as_sdp, , + sizeof(sdp)); + break; default: MISSING_CASE(type); return; @@ -4315,7 +4346,8 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, i915_reg_t reg = HSW_TVIDEO_DIP_CTL(crtc_state->cpu_transcoder); u32 dip_enable = VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW | -VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK; +VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK | +VIDEO_DIP_ENABLE_AS_HSW; u32 val = intel_de_read(dev_priv, reg) & ~dip_enable; /* TODO: Sanitize DSC enabling wrt. intel_dsc_dp_pps_write(). */ @@ -4339,6 +4371,40 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA); } +static +int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *as_sdp, + const void *buffer, size_t size) +{ + const struct dp_sdp *sdp = buffer; + + if (size < sizeof(struct dp_sdp)) + return -EINVAL; + + memset(as_sdp, 0, sizeof(*as_sdp)); + + if (sdp->sdp_header.HB0 != 0) + return -EINVAL; + + if (sdp->sdp_header.HB1 != DP_SDP_ADAPTIVE_SYNC) + return -EINVAL; + + if (sdp->sdp_header.HB2 != 0x02) + return -EINVAL; + + if ((sdp->sdp_header.HB3 & 0x3F) != 9) + return -EINVAL; + + if ((sdp->db[0] & AS_SDP_OP_MODE) != 0x0) + return -EINVAL; + + as_sdp->vtotal = ((u64)sdp->db[2] << 32) | (u64)sdp->db[1]; + as_sdp->target_rr = 0; + as_sdp->duration_incr_ms = 0; + as_sdp->duration_decr_ms = 0; + + return 0; +} + static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc, const void *buffer, size_t size) { @@ -4409,6 +4475,27 @@ static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc, return 0; } +static int +intel_read_dp_metadata_infoframe_as_sdp(struct intel_encoder *encoder, + struct intel_crtc_state *crtc_state, + struct drm_dp_as_sdp *as_sdp) +{ + struct
[PATCH 1/3] drm: Add Adaptive Sync SDP logging
Add structure representing Adaptive Sync Secondary Data Packet (AS SDP). Also, add Adaptive Sync SDP logging in drm_dp_helper.c to facilitate debugging. --v2: - Update logging. [Jani, Ankit] - use as_sdp instead of async [Ankit] - Correct define placeholders to where it is being actually used. [Jani] - Update members in as_sdp structure and make it uniform. [Jani] Signed-off-by: Mitul Golani --- drivers/gpu/drm/display/drm_dp_helper.c | 12 .../drm/i915/display/intel_crtc_state_dump.c | 12 .../drm/i915/display/intel_display_types.h| 1 + include/drm/display/drm_dp.h | 2 ++ include/drm/display/drm_dp_helper.h | 30 +++ 5 files changed, 57 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index d72b6f9a352c..8edd328b6bb8 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -2917,6 +2917,18 @@ void drm_dp_vsc_sdp_log(const char *level, struct device *dev, } EXPORT_SYMBOL(drm_dp_vsc_sdp_log); +void drm_dp_as_sdp_log(struct drm_printer *p, const struct drm_dp_as_sdp *as_sdp) +{ + drm_printf(p, "DP SDP: AS_SDP, revision %u, length %u\n", + as_sdp->revision, as_sdp->length); + drm_printf(p, " vtotal: %d\n", as_sdp->vtotal); + drm_printf(p, "target_rr: %d\n", as_sdp->target_rr); + drm_printf(p, "duration_incr_ms: %d\n", as_sdp->duration_incr_ms); + drm_printf(p, "duration_decr_ms: %d\n", as_sdp->duration_decr_ms); + drm_printf(p, "operation_mode: %d\n", as_sdp->operation_mode); +} +EXPORT_SYMBOL(drm_dp_as_sdp_log); + /** * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON * @dpcd: DisplayPort configuration data diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c index 49fd100ec98a..2b40dee19bfb 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c @@ -61,6 +61,15 @@ intel_dump_dp_vsc_sdp(struct drm_i915_private *i915, drm_dp_vsc_sdp_log(KERN_DEBUG, i915->drm.dev, vsc); } +static void +intel_dump_dp_as_sdp(struct drm_i915_private *i915, +const struct drm_dp_as_sdp *as_sdp) +{ + struct drm_printer p = drm_debug_printer("AS_SDP"); + + drm_dp_as_sdp_log(, as_sdp); +} + static void intel_dump_buffer(struct drm_i915_private *i915, const char *prefix, const u8 *buf, size_t len) @@ -300,6 +309,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config, if (pipe_config->infoframes.enable & intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA)) intel_dump_infoframe(i915, _config->infoframes.drm); + if (pipe_config->infoframes.enable & + intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC)) + intel_dump_dp_as_sdp(i915, _config->infoframes.as_sdp); if (pipe_config->infoframes.enable & intel_hdmi_infoframe_enable(DP_SDP_VSC)) intel_dump_dp_vsc_sdp(i915, _config->infoframes.vsc); diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index b3e942f2eeb0..0c430baefbeb 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1331,6 +1331,7 @@ struct intel_crtc_state { union hdmi_infoframe hdmi; union hdmi_infoframe drm; struct drm_dp_vsc_sdp vsc; + struct drm_dp_as_sdp as_sdp; } infoframes; u8 eld[MAX_ELD_BYTES]; diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 3731828825bd..051f75e09920 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -1577,10 +1577,12 @@ enum drm_dp_phy { #define DP_SDP_AUDIO_COPYMANAGEMENT0x05 /* DP 1.2 */ #define DP_SDP_ISRC0x06 /* DP 1.2 */ #define DP_SDP_VSC 0x07 /* DP 1.2 */ +#define DP_SDP_ADAPTIVE_SYNC0x22 /* DP 1.4 */ #define DP_SDP_CAMERA_GENERIC(i) (0x08 + (i)) /* 0-7, DP 1.3 */ #define DP_SDP_PPS 0x10 /* DP 1.4 */ #define DP_SDP_VSC_EXT_VESA0x20 /* DP 1.4 */ #define DP_SDP_VSC_EXT_CEA 0x21 /* DP 1.4 */ + /* 0x80+ CEA-861 infoframe types */ #define DP_SDP_AUDIO_INFOFRAME_HB2 0x1b diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h index 863b2e7add29..ab75b421fdf8 100644 --- a/include/drm/display/drm_dp_helper.h +++ b/include/drm/display/drm_dp_helper.h @@ -98,6 +98,36 @@ struct drm_dp_vsc_sdp { enum dp_content_type content_type; }; +/** + * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP + * + * This structure represents a DP AS SDP of drm + * It is based on DP 2.1 spec [Table
[PATCH 0/3] Enable Adaptive Sync SDP Support for DP
An Adaptive Sync SDP allows a DP protocol converter to forward Adaptive Sync video with minimal buffering overhead within the converter. An Adaptive-Sync-capable DP protocol converter indicates its support by setting the related bit in the DPCD register. Computes AS SDP values based on the display configuration, ensuring proper handling of Variable Refresh Rate (VRR) in the context of Adaptive Sync. --v2: - Update logging to Patch-1 - use as_sdp instead of async - Put definitions to correct placeholders from where it is defined. - Update member types of as_sdp for uniformity. - Correct use of REG_BIT and REG_GENMASK. - Remove unrelated comments and changes. - Correct code indents. - separate out patch changes for intel_read/write_dp_sdp. --v3: - Add VIDEO_DIP_ASYNC_DATA_SIZE definition and comment in as_sdp_pack function to patch 2 as originally used there. [Patch 2]. - Add VIDEO_DIP_ENABLE_AS_HSW flag to intel_dp_set_infoframes [Patch 3]. --v4: - Add check for HAS_VRR before writing AS SDP. [Patch 3]. --v5: - Add missing check for HAS_VRR before reading AS SDP as well [Patch 3]. Mitul Golani (3): drm: Add Adaptive Sync SDP logging drm/i915/dp: Add Read/Write support for Adaptive Sync SDP drm/i915/display: Compute and Enable AS SDP drivers/gpu/drm/display/drm_dp_helper.c | 12 ++ .../drm/i915/display/intel_crtc_state_dump.c | 12 ++ drivers/gpu/drm/i915/display/intel_ddi.c | 3 + .../drm/i915/display/intel_display_types.h| 1 + drivers/gpu/drm/i915/display/intel_dp.c | 118 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +- drivers/gpu/drm/i915/i915_reg.h | 6 + include/drm/display/drm_dp.h | 2 + include/drm/display/drm_dp_helper.h | 33 + 9 files changed, 195 insertions(+), 4 deletions(-) -- 2.25.1
[PATCH 2/3] drm/i915/dp: Add Read/Write support for Adaptive Sync SDP
Add the necessary structures and functions to handle reading and unpacking Adaptive Sync Secondary Data Packets. Also add support to write and pack AS SDP. --v2: - Correct use of REG_BIT and REG_GENMASK. [Jani] - Use as_sdp instead of async. [Jani] - Remove unrelated comments and changes. [Jani] - Correct code indent. [Jani] Signed-off-by: Mitul Golani --- drivers/gpu/drm/i915/display/intel_dp.c | 95 ++- drivers/gpu/drm/i915/display/intel_hdmi.c | 12 ++- drivers/gpu/drm/i915/i915_reg.h | 6 ++ include/drm/display/drm_dp_helper.h | 3 + 4 files changed, 112 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 3b2482bf683f..5dee50f812c6 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -94,7 +94,6 @@ #define INTEL_DP_RESOLUTION_STANDARD (2 << INTEL_DP_RESOLUTION_SHIFT_MASK) #define INTEL_DP_RESOLUTION_FAILSAFE (3 << INTEL_DP_RESOLUTION_SHIFT_MASK) - /* Constants for DP DSC configurations */ static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15}; @@ -4110,6 +4109,34 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state, return false; } +static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *as_sdp, + struct dp_sdp *sdp, size_t size) +{ + size_t length = sizeof(struct dp_sdp); + + if (size < length) + return -ENOSPC; + + memset(sdp, 0, size); + + /* Prepare AS (Adaptive Sync) SDP Header */ + sdp->sdp_header.HB0 = 0; + sdp->sdp_header.HB1 = as_sdp->sdp_type; + sdp->sdp_header.HB2 = 0x02; + sdp->sdp_header.HB3 = as_sdp->length; + + /* Fill AS (Adaptive Sync) SDP Payload */ + sdp->db[1] = 0x0; + sdp->db[1] = as_sdp->vtotal & 0xFF; + sdp->db[2] = (as_sdp->vtotal >> 8) & 0xF; + sdp->db[3] = 0x0; + sdp->db[4] = 0x0; + sdp->db[7] = 0x0; + sdp->db[8] = 0x0; + + return length; +} + static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct dp_sdp *sdp, size_t size) { @@ -4277,6 +4304,10 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder, _state->infoframes.drm.drm, , sizeof(sdp)); break; + case DP_SDP_ADAPTIVE_SYNC: + len = intel_dp_as_sdp_pack(_state->infoframes.as_sdp, , + sizeof(sdp)); + break; default: MISSING_CASE(type); return; @@ -4315,7 +4346,8 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, i915_reg_t reg = HSW_TVIDEO_DIP_CTL(crtc_state->cpu_transcoder); u32 dip_enable = VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW | -VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK; +VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK | +VIDEO_DIP_ENABLE_AS_HSW; u32 val = intel_de_read(dev_priv, reg) & ~dip_enable; /* TODO: Sanitize DSC enabling wrt. intel_dsc_dp_pps_write(). */ @@ -4339,6 +4371,40 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA); } +static +int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *as_sdp, + const void *buffer, size_t size) +{ + const struct dp_sdp *sdp = buffer; + + if (size < sizeof(struct dp_sdp)) + return -EINVAL; + + memset(as_sdp, 0, sizeof(*as_sdp)); + + if (sdp->sdp_header.HB0 != 0) + return -EINVAL; + + if (sdp->sdp_header.HB1 != DP_SDP_ADAPTIVE_SYNC) + return -EINVAL; + + if (sdp->sdp_header.HB2 != 0x02) + return -EINVAL; + + if ((sdp->sdp_header.HB3 & 0x3F) != 9) + return -EINVAL; + + if ((sdp->db[0] & AS_SDP_OP_MODE) != 0x0) + return -EINVAL; + + as_sdp->vtotal = ((u64)sdp->db[2] << 32) | (u64)sdp->db[1]; + as_sdp->target_rr = 0; + as_sdp->duration_incr_ms = 0; + as_sdp->duration_decr_ms = 0; + + return 0; +} + static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc, const void *buffer, size_t size) { @@ -4409,6 +4475,27 @@ static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc, return 0; } +static int +intel_read_dp_metadata_infoframe_as_sdp(struct intel_encoder *encoder, + struct intel_crtc_state *crtc_state, + struct drm_dp_as_sdp *as_sdp) +{ + struct
[PATCH 3/3] drm/i915/display: Compute and Enable AS SDP
Add necessary functions definitions to enable and compute AS SDP data. The new `intel_dp_compute_as_sdp` function computes AS SDP values based on the display configuration, ensuring proper handling of Variable Refresh Rate (VRR). --v2: - Add DP_SDP_ADAPTIVE_SYNC to infoframe_type_to_idx().[Ankit] - separate patch for intel_read/write_dp_sdp [Ankit]. - _HSW_VIDEO_DIP_ASYNC_DATA_A should be from ADL onward [Ankit] - To fix indentation [Ankit] --v3: - Add VIDEO_DIP_ENABLE_AS_HSW flag to intel_dp_set_infoframes. --v4: - Add HAS_VRR check before write as sdp. --v5: - Add missed HAS_VRR check before read as sdp. Signed-off-by: Mitul Golani --- drivers/gpu/drm/i915/display/intel_ddi.c | 3 +++ drivers/gpu/drm/i915/display/intel_dp.c | 23 +++ 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 38f28c480b38..ed5afaa0ce5d 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3918,6 +3918,9 @@ static void intel_ddi_get_config(struct intel_encoder *encoder, intel_read_dp_sdp(encoder, pipe_config, HDMI_PACKET_TYPE_GAMUT_METADATA); intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); + if ((DISPLAY_VER(dev_priv) >= 13) && HAS_VRR(dev_priv)) + intel_read_dp_sdp(encoder, pipe_config, DP_SDP_ADAPTIVE_SYNC); + intel_psr_get_config(encoder, pipe_config); intel_audio_codec_get_config(encoder, pipe_config); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 5dee50f812c6..0747b5bda5c8 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2630,6 +2630,25 @@ static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp, _state->infoframes.vsc); } +static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp, + struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state) +{ + struct drm_dp_as_sdp *as_sdp = _state->infoframes.as_sdp; + struct intel_connector *connector = intel_dp->attached_connector; + const struct drm_display_mode *adjusted_mode = + _state->hw.adjusted_mode; + int vrefresh = drm_mode_vrefresh(adjusted_mode); + + if (!intel_vrr_is_in_range(connector, vrefresh)) + return; + + crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC); + as_sdp->sdp_type = DP_SDP_ADAPTIVE_SYNC; + as_sdp->length = 0x9; + as_sdp->vtotal = adjusted_mode->vtotal; +} + void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state, const struct drm_connector_state *conn_state, @@ -2956,6 +2975,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, g4x_dp_set_clock(encoder, pipe_config); intel_vrr_compute_config(pipe_config, conn_state); + intel_dp_compute_as_sdp(intel_dp, pipe_config, conn_state); intel_psr_compute_config(intel_dp, pipe_config, conn_state); intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16); intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); @@ -4368,6 +4388,9 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, if (!crtc_state->has_psr) intel_write_dp_sdp(encoder, crtc_state, DP_SDP_VSC); + if ((DISPLAY_VER(dev_priv) >= 13) && HAS_VRR(dev_priv)) + intel_write_dp_sdp(encoder, crtc_state, DP_SDP_ADAPTIVE_SYNC); + intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA); } -- 2.25.1
[PATCH 1/3] drm: Add Adaptive Sync SDP logging
Add structure representing Adaptive Sync Secondary Data Packet (AS SDP). Also, add Adaptive Sync SDP logging in drm_dp_helper.c to facilitate debugging. --v2: - Update logging. [Jani, Ankit] - use as_sdp instead of async [Ankit] - Correct define placeholders to where it is being actually used. [Jani] - Update members in as_sdp structure and make it uniform. [Jani] Signed-off-by: Mitul Golani --- drivers/gpu/drm/display/drm_dp_helper.c | 12 .../drm/i915/display/intel_crtc_state_dump.c | 12 .../drm/i915/display/intel_display_types.h| 1 + include/drm/display/drm_dp.h | 2 ++ include/drm/display/drm_dp_helper.h | 30 +++ 5 files changed, 57 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index d72b6f9a352c..8edd328b6bb8 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -2917,6 +2917,18 @@ void drm_dp_vsc_sdp_log(const char *level, struct device *dev, } EXPORT_SYMBOL(drm_dp_vsc_sdp_log); +void drm_dp_as_sdp_log(struct drm_printer *p, const struct drm_dp_as_sdp *as_sdp) +{ + drm_printf(p, "DP SDP: AS_SDP, revision %u, length %u\n", + as_sdp->revision, as_sdp->length); + drm_printf(p, " vtotal: %d\n", as_sdp->vtotal); + drm_printf(p, "target_rr: %d\n", as_sdp->target_rr); + drm_printf(p, "duration_incr_ms: %d\n", as_sdp->duration_incr_ms); + drm_printf(p, "duration_decr_ms: %d\n", as_sdp->duration_decr_ms); + drm_printf(p, "operation_mode: %d\n", as_sdp->operation_mode); +} +EXPORT_SYMBOL(drm_dp_as_sdp_log); + /** * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON * @dpcd: DisplayPort configuration data diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c index 49fd100ec98a..2b40dee19bfb 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c @@ -61,6 +61,15 @@ intel_dump_dp_vsc_sdp(struct drm_i915_private *i915, drm_dp_vsc_sdp_log(KERN_DEBUG, i915->drm.dev, vsc); } +static void +intel_dump_dp_as_sdp(struct drm_i915_private *i915, +const struct drm_dp_as_sdp *as_sdp) +{ + struct drm_printer p = drm_debug_printer("AS_SDP"); + + drm_dp_as_sdp_log(, as_sdp); +} + static void intel_dump_buffer(struct drm_i915_private *i915, const char *prefix, const u8 *buf, size_t len) @@ -300,6 +309,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config, if (pipe_config->infoframes.enable & intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA)) intel_dump_infoframe(i915, _config->infoframes.drm); + if (pipe_config->infoframes.enable & + intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC)) + intel_dump_dp_as_sdp(i915, _config->infoframes.as_sdp); if (pipe_config->infoframes.enable & intel_hdmi_infoframe_enable(DP_SDP_VSC)) intel_dump_dp_vsc_sdp(i915, _config->infoframes.vsc); diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index b3e942f2eeb0..0c430baefbeb 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1331,6 +1331,7 @@ struct intel_crtc_state { union hdmi_infoframe hdmi; union hdmi_infoframe drm; struct drm_dp_vsc_sdp vsc; + struct drm_dp_as_sdp as_sdp; } infoframes; u8 eld[MAX_ELD_BYTES]; diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 3731828825bd..051f75e09920 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -1577,10 +1577,12 @@ enum drm_dp_phy { #define DP_SDP_AUDIO_COPYMANAGEMENT0x05 /* DP 1.2 */ #define DP_SDP_ISRC0x06 /* DP 1.2 */ #define DP_SDP_VSC 0x07 /* DP 1.2 */ +#define DP_SDP_ADAPTIVE_SYNC0x22 /* DP 1.4 */ #define DP_SDP_CAMERA_GENERIC(i) (0x08 + (i)) /* 0-7, DP 1.3 */ #define DP_SDP_PPS 0x10 /* DP 1.4 */ #define DP_SDP_VSC_EXT_VESA0x20 /* DP 1.4 */ #define DP_SDP_VSC_EXT_CEA 0x21 /* DP 1.4 */ + /* 0x80+ CEA-861 infoframe types */ #define DP_SDP_AUDIO_INFOFRAME_HB2 0x1b diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h index 863b2e7add29..ab75b421fdf8 100644 --- a/include/drm/display/drm_dp_helper.h +++ b/include/drm/display/drm_dp_helper.h @@ -98,6 +98,36 @@ struct drm_dp_vsc_sdp { enum dp_content_type content_type; }; +/** + * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP + * + * This structure represents a DP AS SDP of drm + * It is based on DP 2.1 spec [Table
[PATCH 0/3]
An Adaptive Sync SDP allows a DP protocol converter to forward Adaptive Sync video with minimal buffering overhead within the converter. An Adaptive-Sync-capable DP protocol converter indicates its support by setting the related bit in the DPCD register. Computes AS SDP values based on the display configuration, ensuring proper handling of Variable Refresh Rate (VRR) in the context of Adaptive Sync. --v2: - Update logging to Patch-1 - use as_sdp instead of async - Put definitions to correct placeholders from where it is defined. - Update member types of as_sdp for uniformity. - Correct use of REG_BIT and REG_GENMASK. - Remove unrelated comments and changes. - Correct code indents. - separate out patch changes for intel_read/write_dp_sdp. --v3: - Add VIDEO_DIP_ASYNC_DATA_SIZE definition and comment in as_sdp_pack function to patch 2 as originally used there. [Patch 2]. - Add VIDEO_DIP_ENABLE_AS_HSW flag to intel_dp_set_infoframes [Patch 3]. --v4: - Add check for HAS_VRR before writing AS SDP. [Patch 3]. --v5: - Add missing check for HAS_VRR before reading AS SDP as well [Patch 3]. Mitul Golani (3): drm: Add Adaptive Sync SDP logging drm/i915/dp: Add Read/Write support for Adaptive Sync SDP drm/i915/display: Compute and Enable AS SDP drivers/gpu/drm/display/drm_dp_helper.c | 12 ++ .../drm/i915/display/intel_crtc_state_dump.c | 12 ++ drivers/gpu/drm/i915/display/intel_ddi.c | 3 + .../drm/i915/display/intel_display_types.h| 1 + drivers/gpu/drm/i915/display/intel_dp.c | 118 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +- drivers/gpu/drm/i915/i915_reg.h | 6 + include/drm/display/drm_dp.h | 2 + include/drm/display/drm_dp_helper.h | 33 + 9 files changed, 195 insertions(+), 4 deletions(-) -- 2.25.1
[PATCH 3/3] drm/i915/display: Compute and Enable AS SDP
Add necessary functions definitions to enable and compute AS SDP data. The new `intel_dp_compute_as_sdp` function computes AS SDP values based on the display configuration, ensuring proper handling of Variable Refresh Rate (VRR). --v2: - Add DP_SDP_ADAPTIVE_SYNC to infoframe_type_to_idx().[Ankit] - separate patch for intel_read/write_dp_sdp [Ankit]. - _HSW_VIDEO_DIP_ASYNC_DATA_A should be from ADL onward [Ankit] - To fix indentation [Ankit] Signed-off-by: Mitul Golani --- drivers/gpu/drm/i915/display/intel_ddi.c | 3 +++ drivers/gpu/drm/i915/display/intel_dp.c | 23 +++ 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 38f28c480b38..628725611dbe 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3918,6 +3918,9 @@ static void intel_ddi_get_config(struct intel_encoder *encoder, intel_read_dp_sdp(encoder, pipe_config, HDMI_PACKET_TYPE_GAMUT_METADATA); intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); + if (DISPLAY_VER(dev_priv) >= 13) + intel_read_dp_sdp(encoder, pipe_config, DP_SDP_ADAPTIVE_SYNC); + intel_psr_get_config(encoder, pipe_config); intel_audio_codec_get_config(encoder, pipe_config); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 5dee50f812c6..0747b5bda5c8 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2630,6 +2630,25 @@ static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp, _state->infoframes.vsc); } +static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp, + struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state) +{ + struct drm_dp_as_sdp *as_sdp = _state->infoframes.as_sdp; + struct intel_connector *connector = intel_dp->attached_connector; + const struct drm_display_mode *adjusted_mode = + _state->hw.adjusted_mode; + int vrefresh = drm_mode_vrefresh(adjusted_mode); + + if (!intel_vrr_is_in_range(connector, vrefresh)) + return; + + crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC); + as_sdp->sdp_type = DP_SDP_ADAPTIVE_SYNC; + as_sdp->length = 0x9; + as_sdp->vtotal = adjusted_mode->vtotal; +} + void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state, const struct drm_connector_state *conn_state, @@ -2956,6 +2975,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, g4x_dp_set_clock(encoder, pipe_config); intel_vrr_compute_config(pipe_config, conn_state); + intel_dp_compute_as_sdp(intel_dp, pipe_config, conn_state); intel_psr_compute_config(intel_dp, pipe_config, conn_state); intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16); intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); @@ -4368,6 +4388,9 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, if (!crtc_state->has_psr) intel_write_dp_sdp(encoder, crtc_state, DP_SDP_VSC); + if ((DISPLAY_VER(dev_priv) >= 13) && HAS_VRR(dev_priv)) + intel_write_dp_sdp(encoder, crtc_state, DP_SDP_ADAPTIVE_SYNC); + intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA); } -- 2.25.1
[PATCH 1/3] drm: Add Adaptive Sync SDP logging
Add structure representing Adaptive Sync Secondary Data Packet (AS SDP). Also, add Adaptive Sync SDP logging in drm_dp_helper.c to facilitate debugging. --v2: - Update logging. [Jani, Ankit] - use as_sdp instead of async [Ankit] - Correct define placeholders to where it is being actually used. [Jani] - Update members in as_sdp structure and make it uniform. [Jani] Signed-off-by: Mitul Golani --- drivers/gpu/drm/display/drm_dp_helper.c | 12 .../drm/i915/display/intel_crtc_state_dump.c | 12 .../drm/i915/display/intel_display_types.h| 1 + include/drm/display/drm_dp.h | 2 ++ include/drm/display/drm_dp_helper.h | 30 +++ 5 files changed, 57 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index d72b6f9a352c..8edd328b6bb8 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -2917,6 +2917,18 @@ void drm_dp_vsc_sdp_log(const char *level, struct device *dev, } EXPORT_SYMBOL(drm_dp_vsc_sdp_log); +void drm_dp_as_sdp_log(struct drm_printer *p, const struct drm_dp_as_sdp *as_sdp) +{ + drm_printf(p, "DP SDP: AS_SDP, revision %u, length %u\n", + as_sdp->revision, as_sdp->length); + drm_printf(p, " vtotal: %d\n", as_sdp->vtotal); + drm_printf(p, "target_rr: %d\n", as_sdp->target_rr); + drm_printf(p, "duration_incr_ms: %d\n", as_sdp->duration_incr_ms); + drm_printf(p, "duration_decr_ms: %d\n", as_sdp->duration_decr_ms); + drm_printf(p, "operation_mode: %d\n", as_sdp->operation_mode); +} +EXPORT_SYMBOL(drm_dp_as_sdp_log); + /** * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON * @dpcd: DisplayPort configuration data diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c index 49fd100ec98a..2b40dee19bfb 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c @@ -61,6 +61,15 @@ intel_dump_dp_vsc_sdp(struct drm_i915_private *i915, drm_dp_vsc_sdp_log(KERN_DEBUG, i915->drm.dev, vsc); } +static void +intel_dump_dp_as_sdp(struct drm_i915_private *i915, +const struct drm_dp_as_sdp *as_sdp) +{ + struct drm_printer p = drm_debug_printer("AS_SDP"); + + drm_dp_as_sdp_log(, as_sdp); +} + static void intel_dump_buffer(struct drm_i915_private *i915, const char *prefix, const u8 *buf, size_t len) @@ -300,6 +309,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config, if (pipe_config->infoframes.enable & intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA)) intel_dump_infoframe(i915, _config->infoframes.drm); + if (pipe_config->infoframes.enable & + intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC)) + intel_dump_dp_as_sdp(i915, _config->infoframes.as_sdp); if (pipe_config->infoframes.enable & intel_hdmi_infoframe_enable(DP_SDP_VSC)) intel_dump_dp_vsc_sdp(i915, _config->infoframes.vsc); diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index b3e942f2eeb0..0c430baefbeb 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1331,6 +1331,7 @@ struct intel_crtc_state { union hdmi_infoframe hdmi; union hdmi_infoframe drm; struct drm_dp_vsc_sdp vsc; + struct drm_dp_as_sdp as_sdp; } infoframes; u8 eld[MAX_ELD_BYTES]; diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 3731828825bd..051f75e09920 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -1577,10 +1577,12 @@ enum drm_dp_phy { #define DP_SDP_AUDIO_COPYMANAGEMENT0x05 /* DP 1.2 */ #define DP_SDP_ISRC0x06 /* DP 1.2 */ #define DP_SDP_VSC 0x07 /* DP 1.2 */ +#define DP_SDP_ADAPTIVE_SYNC0x22 /* DP 1.4 */ #define DP_SDP_CAMERA_GENERIC(i) (0x08 + (i)) /* 0-7, DP 1.3 */ #define DP_SDP_PPS 0x10 /* DP 1.4 */ #define DP_SDP_VSC_EXT_VESA0x20 /* DP 1.4 */ #define DP_SDP_VSC_EXT_CEA 0x21 /* DP 1.4 */ + /* 0x80+ CEA-861 infoframe types */ #define DP_SDP_AUDIO_INFOFRAME_HB2 0x1b diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h index 863b2e7add29..ab75b421fdf8 100644 --- a/include/drm/display/drm_dp_helper.h +++ b/include/drm/display/drm_dp_helper.h @@ -98,6 +98,36 @@ struct drm_dp_vsc_sdp { enum dp_content_type content_type; }; +/** + * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP + * + * This structure represents a DP AS SDP of drm + * It is based on DP 2.1 spec [Table
[PATCH 2/3] drm/i915/dp: Add Read/Write support for Adaptive Sync SDP
Add the necessary structures and functions to handle reading and unpacking Adaptive Sync Secondary Data Packets. Also add support to write and pack AS SDP. --v2: - Correct use of REG_BIT and REG_GENMASK. [Jani] - Use as_sdp instead of async. [Jani] - Remove unrelated comments and changes. [Jani] - Correct code indent. [Jani] Signed-off-by: Mitul Golani --- drivers/gpu/drm/i915/display/intel_dp.c | 95 ++- drivers/gpu/drm/i915/display/intel_hdmi.c | 12 ++- drivers/gpu/drm/i915/i915_reg.h | 6 ++ include/drm/display/drm_dp_helper.h | 3 + 4 files changed, 112 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 3b2482bf683f..5dee50f812c6 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -94,7 +94,6 @@ #define INTEL_DP_RESOLUTION_STANDARD (2 << INTEL_DP_RESOLUTION_SHIFT_MASK) #define INTEL_DP_RESOLUTION_FAILSAFE (3 << INTEL_DP_RESOLUTION_SHIFT_MASK) - /* Constants for DP DSC configurations */ static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15}; @@ -4110,6 +4109,34 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state, return false; } +static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *as_sdp, + struct dp_sdp *sdp, size_t size) +{ + size_t length = sizeof(struct dp_sdp); + + if (size < length) + return -ENOSPC; + + memset(sdp, 0, size); + + /* Prepare AS (Adaptive Sync) SDP Header */ + sdp->sdp_header.HB0 = 0; + sdp->sdp_header.HB1 = as_sdp->sdp_type; + sdp->sdp_header.HB2 = 0x02; + sdp->sdp_header.HB3 = as_sdp->length; + + /* Fill AS (Adaptive Sync) SDP Payload */ + sdp->db[1] = 0x0; + sdp->db[1] = as_sdp->vtotal & 0xFF; + sdp->db[2] = (as_sdp->vtotal >> 8) & 0xF; + sdp->db[3] = 0x0; + sdp->db[4] = 0x0; + sdp->db[7] = 0x0; + sdp->db[8] = 0x0; + + return length; +} + static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct dp_sdp *sdp, size_t size) { @@ -4277,6 +4304,10 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder, _state->infoframes.drm.drm, , sizeof(sdp)); break; + case DP_SDP_ADAPTIVE_SYNC: + len = intel_dp_as_sdp_pack(_state->infoframes.as_sdp, , + sizeof(sdp)); + break; default: MISSING_CASE(type); return; @@ -4315,7 +4346,8 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, i915_reg_t reg = HSW_TVIDEO_DIP_CTL(crtc_state->cpu_transcoder); u32 dip_enable = VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW | -VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK; +VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK | +VIDEO_DIP_ENABLE_AS_HSW; u32 val = intel_de_read(dev_priv, reg) & ~dip_enable; /* TODO: Sanitize DSC enabling wrt. intel_dsc_dp_pps_write(). */ @@ -4339,6 +4371,40 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder, intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA); } +static +int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *as_sdp, + const void *buffer, size_t size) +{ + const struct dp_sdp *sdp = buffer; + + if (size < sizeof(struct dp_sdp)) + return -EINVAL; + + memset(as_sdp, 0, sizeof(*as_sdp)); + + if (sdp->sdp_header.HB0 != 0) + return -EINVAL; + + if (sdp->sdp_header.HB1 != DP_SDP_ADAPTIVE_SYNC) + return -EINVAL; + + if (sdp->sdp_header.HB2 != 0x02) + return -EINVAL; + + if ((sdp->sdp_header.HB3 & 0x3F) != 9) + return -EINVAL; + + if ((sdp->db[0] & AS_SDP_OP_MODE) != 0x0) + return -EINVAL; + + as_sdp->vtotal = ((u64)sdp->db[2] << 32) | (u64)sdp->db[1]; + as_sdp->target_rr = 0; + as_sdp->duration_incr_ms = 0; + as_sdp->duration_decr_ms = 0; + + return 0; +} + static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc, const void *buffer, size_t size) { @@ -4409,6 +4475,27 @@ static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc, return 0; } +static int +intel_read_dp_metadata_infoframe_as_sdp(struct intel_encoder *encoder, + struct intel_crtc_state *crtc_state, + struct drm_dp_as_sdp *as_sdp) +{ + struct
[PATCH 0/3] Enable Adaptive Sync SDP Support for DP
An Adaptive Sync SDP allows a DP protocol converter to forward Adaptive Sync video with minimal buffering overhead within the converter. An Adaptive-Sync-capable DP protocol converter indicates its support by setting the related bit in the DPCD register. Computes AS SDP values based on the display configuration, ensuring proper handling of Variable Refresh Rate (VRR) in the context of Adaptive Sync. --v2: - Update logging to Patch-1 - use as_sdp instead of async - Put definitions to correct placeholders from where it is defined. - Update member types of as_sdp for uniformity. - Correct use of REG_BIT and REG_GENMASK. - Remove unrelated comments and changes. - Correct code indents. - separate out patch changes for intel_read/write_dp_sdp. --v3: - Add VIDEO_DIP_ASYNC_DATA_SIZE definition and comment in as_sdp_pack function to patch 2 as originally used there. [Patch 2]. - Add VIDEO_DIP_ENABLE_AS_HSW flag to intel_dp_set_infoframes [Patch 3]. --v4: - Add check for HAS_VRR [Patch 4] Mitul Golani (3): drm: Add Adaptive Sync SDP logging drm/i915/dp: Add Read/Write support for Adaptive Sync SDP drm/i915/display: Compute and Enable AS SDP drivers/gpu/drm/display/drm_dp_helper.c | 12 ++ .../drm/i915/display/intel_crtc_state_dump.c | 12 ++ drivers/gpu/drm/i915/display/intel_ddi.c | 3 + .../drm/i915/display/intel_display_types.h| 1 + drivers/gpu/drm/i915/display/intel_dp.c | 118 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +- drivers/gpu/drm/i915/i915_reg.h | 6 + include/drm/display/drm_dp.h | 2 + include/drm/display/drm_dp_helper.h | 33 + 9 files changed, 195 insertions(+), 4 deletions(-) -- 2.25.1