[PATCH] nouveau/svm: Fix to migrate all requested pages
Users may request that pages from an OpenCL SVM allocation be migrated
to the GPU with clEnqueueSVMMigrateMem(). In Nouveau this will call into
nouveau_dmem_migrate_vma() to do the migration. If the total range to be
migrated exceeds SG_MAX_SINGLE_ALLOC the pages will be migrated in
chunks of size SG_MAX_SINGLE_ALLOC. However a typo in updating the
starting address means that only the first chunk will get migrated.
Fix the calculation so that the entire range will get migrated if
possible.
Signed-off-by: Alistair Popple
Fixes: e3d8b0890469 ("drm/nouveau/svm: map pages after migration")
---
drivers/gpu/drm/nouveau/nouveau_dmem.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c
b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 7ba66ad68a8a..16356611b5b9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -680,7 +680,11 @@ nouveau_dmem_migrate_vma(struct nouveau_drm *drm,
goto out_free_dma;
for (i = 0; i < npages; i += max) {
- args.end = start + (max << PAGE_SHIFT);
+ if (args.start + (max << PAGE_SHIFT) > end)
+ args.end = end;
+ else
+ args.end = args.start + (max << PAGE_SHIFT);
+
ret = migrate_vma_setup(&args);
if (ret)
goto out_free_pfns;
--
2.35.1
Re: [Freedreno] [PATCH v2 5/7] arm64: dts: qcom: sc7280: Update gpu register list
On 7/19/2022 3:26 PM, Rajendra Nayak wrote: On 7/19/2022 12:49 PM, Stephen Boyd wrote: Quoting Akhil P Oommen (2022-07-18 23:37:16) On 7/19/2022 11:19 AM, Stephen Boyd wrote: Quoting Akhil P Oommen (2022-07-18 21:07:05) On 7/14/2022 11:10 AM, Akhil P Oommen wrote: IIUC, qcom gdsc driver doesn't ensure hardware is collapsed since they are vote-able switches. Ideally, we should ensure that the hw has collapsed for gpu recovery because there could be transient votes from other subsystems like hypervisor using their vote register. I am not sure how complex the plumbing to gpucc driver would be to allow gpu driver to check hw status. OTOH, with this patch, gpu driver does a read operation on a gpucc register which is in always-on domain. That means we don't need to vote any resource to access this register. Reading between the lines here, you're saying that you have to read the gdsc register to make sure that the gdsc is in some state? Can you clarify exactly what you're doing? And how do you know that something else in the kernel can't cause the register to change after it is read? It certainly seems like we can't be certain because there is voting involved. From gpu driver, cx_gdscr.bit[31] (power off status) register can be polled to ensure that it *collapsed at least once*. We don't need to care if something turns ON gdsc after that. yes, this looks like the best case effort to get the gpu to recover, but the kernel driver really has no control to make sure this condition can always be met (because it depends on other entities like hyp, trustzone etc right?) Why not just put a worst case polling delay? I didn't get you entirely. Where do you mean to keep the polling delay? Stephen/Rajendra/Taniya, any suggestion? Why can't you assert a gpu reset signal with the reset APIs? This series seems to jump through a bunch of hoops to get the gdsc and power domain to "reset" when I don't know why any of that is necessary. Can't we simply assert a reset to the hardware after recovery completes so the device is back into a good known POR (power on reset) state? That is because there is no register interface to reset GPU CX domain. The recommended sequence from HW design folks is to collapse both cx and gx gdsc to properly reset gpu/gmu. Ok. One knee jerk reaction is to treat the gdsc as a reset then and possibly mux that request along with any power domain on/off so that if the reset is requested and the power domain is off nothing happens. Otherwise if the power domain is on then it manually sequences and controls the two gdscs so that the GPU is reset and then restores the enable state of the power domain. It would be fatal to asynchronously pull the plug on CX gdsc forcefully because there might be another gpu/smmu driver thread accessing registers in cx domain. -Akhil.
[PATCH v2] drm/panel-edp: add IVO M133NW4J-R3 panel entry
Add an eDP panel entry for IVO M133NW4J-R3.
Due to lack of documentation, use the delay_200_500_e50 timings for now.
Signed-off-by: Steev Klimaszewski
---
v2 - actually mark it as R3 in the change, not R2...
I'm basing my information gathering off what I could find for the IVO
M133NW4J panels on panelook.com. R0 is glossy, and mine is not. R2
says it is discontinued, and I am just guessing that I have the R3 as
the Thinkpad X13s just came out, roughly a month ago.
Signed-off-by: Steev Klimaszewski
---
drivers/gpu/drm/panel/panel-edp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 5024ba690abf..870b98041c60 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1883,6 +1883,7 @@ static const struct edp_panel_entry edp_panels[] = {
EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay,
"N116BCA-EA1"),
+ EDP_PANEL_ENTRY('I', 'V', 'O', 0x854b, &delay_200_500_p2e100,
"M133NW4J-R3"),
EDP_PANEL_ENTRY('K', 'D', 'B', 0x0624,
&kingdisplay_kd116n21_30nv_a010.delay, "116N21-30NV-A010"),
EDP_PANEL_ENTRY('K', 'D', 'B', 0x1120, &delay_200_500_e80_d50,
"116N29-30NK-C007"),
--
2.30.2
[PATCH] drm/panel-edp: add IVO M133NW4J-R3 panel entry
Add an eDP panel entry for IVO M133NW4J-R3.
Due to lack of documentation, use the delay_200_500_e50 timings for now.
Signed-off-by: Steev Klimaszewski
---
I'm basing my information gathering off what I could find for the IVO
M133NW4J panels on panelook.com. R0 is glossy, and mine is not. R2
says it is discontinued, and I am just guessing that I have the R3 as
the Thinkpad X13s just came out, roughly a month ago.
---
drivers/gpu/drm/panel/panel-edp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 5024ba690abf..8f89226e9db5 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1883,6 +1883,7 @@ static const struct edp_panel_entry edp_panels[] = {
EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay,
"N116BCA-EA1"),
+ EDP_PANEL_ENTRY('I', 'V', 'O', 0x854b, &delay_200_500_p2e100,
"M133NW4J-R2"),
EDP_PANEL_ENTRY('K', 'D', 'B', 0x0624,
&kingdisplay_kd116n21_30nv_a010.delay, "116N21-30NV-A010"),
EDP_PANEL_ENTRY('K', 'D', 'B', 0x1120, &delay_200_500_e80_d50,
"116N29-30NK-C007"),
--
2.30.2
[PATCH] drm/amdgpu: use native mode for dp aux transfer
When using amdgpu for e8860, the monitor sometimes haven't any signal, and the kernel reports some errors: [ 17.317302][ 2] [ T1045] [drm:amdgpu_atombios_dp_link_train [amdgpu]] *ERROR* channel eq failed: 5 tries [ 17.326963][ 2] [ T1045] [drm:amdgpu_atombios_dp_link_train [amdgpu]] *ERROR* channel eq failed But if I use radeon for e8860, everything are always normal, the reason is that radeon use native mode and amdgpu use atombios mode when init dp aux, so when I use native mode for amdgpu, everything are always normal. Signed-off-by: Zhenneng Li Change-Id: Ia9a2be3ab03e56b1c8337fdbf713461196fbc58f --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 5 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dp_auxch.c | 273 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 + drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 + drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 5 +- 7 files changed, 290 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_dp_auxch.c diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index 3e0e2eb7e235..2913cf46f848 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -58,7 +58,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \ amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \ amdgpu_fw_attestation.o amdgpu_securedisplay.o \ - amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o + amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o amdgpu_dp_auxch.o amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 30ce6bb6fa77..15e0288b1997 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -238,6 +238,8 @@ extern int amdgpu_num_kcq; #define AMDGPU_VCNFW_LOG_SIZE (32 * 1024) extern int amdgpu_vcnfw_log; +extern int amdgpu_auxch; + #define AMDGPU_VM_MAX_NUM_CTX 4096 #define AMDGPU_SG_THRESHOLD(256*1024*1024) #define AMDGPU_DEFAULT_GTT_SIZE_MB 3072ULL /* 3GB by default */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c index 9ba4817a9148..68c8d79e2937 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c @@ -49,7 +49,10 @@ static struct amdgpu_i2c_bus_rec amdgpu_atombios_get_bus_rec_for_i2c_gpio(ATOM_G memset(&i2c, 0, sizeof(struct amdgpu_i2c_bus_rec)); - i2c.mask_clk_reg = le16_to_cpu(gpio->usClkMaskRegisterIndex); + if (amdgpu_auxch) + i2c.mask_clk_reg = le16_to_cpu(gpio->usClkMaskRegisterIndex) * 4; + else + i2c.mask_clk_reg = le16_to_cpu(gpio->usClkMaskRegisterIndex); i2c.mask_data_reg = le16_to_cpu(gpio->usDataMaskRegisterIndex); i2c.en_clk_reg = le16_to_cpu(gpio->usClkEnRegisterIndex); i2c.en_data_reg = le16_to_cpu(gpio->usDataEnRegisterIndex); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dp_auxch.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dp_auxch.c new file mode 100644 index ..22078f1ca936 --- /dev/null +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dp_auxch.c @@ -0,0 +1,273 @@ +/* + * Copyright 2015 Red Hat Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * Authors: Dave Airlie + */ + +#include "amdgpu.h" + +#defineAUX_SW_RX_OVERFLOW (1 << 8) +#defineAUX_SW_RX_HPD_DISCON(1 << 9) +#defineAUX_SW_RX_PARTIAL_BYTE (1 << 10) +#defineAUX_SW_NON_AUX_MODE (1 << 11) +#defineAUX_SW_RX_SYNC_IN
[PATCH AUTOSEL 5.4 08/16] drm: panel-orientation-quirks: Add quirk for the Lenovo Yoga Tablet 2 830
From: Hans de Goede
[ Upstream commit 144248515246e52a3706de1ee928af29a63794b8 ]
The Lenovo Yoga Tablet 2 830F / 830L use a panel which has been mounted
90 degrees rotated. Add a quirk for this.
Signed-off-by: Hans de Goede
Reviewed-by: Javier Martinez Canillas
Link:
https://patchwork.freedesktop.org/patch/msgid/20220623112710.15693-1-hdego...@redhat.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/drm_panel_orientation_quirks.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index f5ab891731d0..ad75e7712ebb 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -266,6 +266,21 @@ static const struct dmi_system_id orientation_data[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X9"),
},
.driver_data = (void *)&lcd1200x1920_rightside_up,
+ }, {/* Lenovo Yoga Tablet 2 830F / 830L */
+ .matches = {
+/*
+ * Note this also matches the Lenovo Yoga Tablet 2 1050F/L
+ * since that uses the same mainboard. The resolution match
+ * will limit this to only matching on the 830F/L. Neither has
+ * any external video outputs so those are not a concern.
+ */
+DMI_MATCH(DMI_SYS_VENDOR, "Intel Corp."),
+DMI_MATCH(DMI_PRODUCT_NAME, "VALLEYVIEW C0 PLATFORM"),
+DMI_MATCH(DMI_BOARD_NAME, "BYT-T FFD8"),
+/* Partial match on beginning of BIOS version */
+DMI_MATCH(DMI_BIOS_VERSION, "BLADE_21"),
+ },
+ .driver_data = (void *)&lcd1200x1920_rightside_up,
}, {/* OneGX1 Pro */
.matches = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "SYSTEM_MANUFACTURER"),
--
2.35.1
[PATCH AUTOSEL 5.10 13/25] drm: panel-orientation-quirks: Add quirk for the Lenovo Yoga Tablet 2 830
From: Hans de Goede
[ Upstream commit 144248515246e52a3706de1ee928af29a63794b8 ]
The Lenovo Yoga Tablet 2 830F / 830L use a panel which has been mounted
90 degrees rotated. Add a quirk for this.
Signed-off-by: Hans de Goede
Reviewed-by: Javier Martinez Canillas
Link:
https://patchwork.freedesktop.org/patch/msgid/20220623112710.15693-1-hdego...@redhat.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/drm_panel_orientation_quirks.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index f5ab891731d0..ad75e7712ebb 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -266,6 +266,21 @@ static const struct dmi_system_id orientation_data[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X9"),
},
.driver_data = (void *)&lcd1200x1920_rightside_up,
+ }, {/* Lenovo Yoga Tablet 2 830F / 830L */
+ .matches = {
+/*
+ * Note this also matches the Lenovo Yoga Tablet 2 1050F/L
+ * since that uses the same mainboard. The resolution match
+ * will limit this to only matching on the 830F/L. Neither has
+ * any external video outputs so those are not a concern.
+ */
+DMI_MATCH(DMI_SYS_VENDOR, "Intel Corp."),
+DMI_MATCH(DMI_PRODUCT_NAME, "VALLEYVIEW C0 PLATFORM"),
+DMI_MATCH(DMI_BOARD_NAME, "BYT-T FFD8"),
+/* Partial match on beginning of BIOS version */
+DMI_MATCH(DMI_BIOS_VERSION, "BLADE_21"),
+ },
+ .driver_data = (void *)&lcd1200x1920_rightside_up,
}, {/* OneGX1 Pro */
.matches = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "SYSTEM_MANUFACTURER"),
--
2.35.1
[PATCH AUTOSEL 5.15 26/42] drm: panel-orientation-quirks: Add quirk for the Lenovo Yoga Tablet 2 830
From: Hans de Goede
[ Upstream commit 144248515246e52a3706de1ee928af29a63794b8 ]
The Lenovo Yoga Tablet 2 830F / 830L use a panel which has been mounted
90 degrees rotated. Add a quirk for this.
Signed-off-by: Hans de Goede
Reviewed-by: Javier Martinez Canillas
Link:
https://patchwork.freedesktop.org/patch/msgid/20220623112710.15693-1-hdego...@redhat.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/drm_panel_orientation_quirks.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index f5ab891731d0..ad75e7712ebb 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -266,6 +266,21 @@ static const struct dmi_system_id orientation_data[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X9"),
},
.driver_data = (void *)&lcd1200x1920_rightside_up,
+ }, {/* Lenovo Yoga Tablet 2 830F / 830L */
+ .matches = {
+/*
+ * Note this also matches the Lenovo Yoga Tablet 2 1050F/L
+ * since that uses the same mainboard. The resolution match
+ * will limit this to only matching on the 830F/L. Neither has
+ * any external video outputs so those are not a concern.
+ */
+DMI_MATCH(DMI_SYS_VENDOR, "Intel Corp."),
+DMI_MATCH(DMI_PRODUCT_NAME, "VALLEYVIEW C0 PLATFORM"),
+DMI_MATCH(DMI_BOARD_NAME, "BYT-T FFD8"),
+/* Partial match on beginning of BIOS version */
+DMI_MATCH(DMI_BIOS_VERSION, "BLADE_21"),
+ },
+ .driver_data = (void *)&lcd1200x1920_rightside_up,
}, {/* OneGX1 Pro */
.matches = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "SYSTEM_MANUFACTURER"),
--
2.35.1
[PATCH AUTOSEL 5.18 35/54] drm/ssd130x: Fix pre-charge period setting
From: Ezequiel Garcia [ Upstream commit b68277f19e31a25312c4acccadb5cf1502e52e84 ] Fix small typo which causes the mask for the 'precharge1' setting to be used with the 'precharge2' value. Signed-off-by: Ezequiel Garcia Acked-by: Javier Martinez Canillas Signed-off-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20220706184133.210888-1-ezequ...@vanguardiasur.com.ar Signed-off-by: Sasha Levin --- drivers/gpu/drm/solomon/ssd130x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c index 38b6c2c14f53..9a8a8f5606fd 100644 --- a/drivers/gpu/drm/solomon/ssd130x.c +++ b/drivers/gpu/drm/solomon/ssd130x.c @@ -293,7 +293,7 @@ static int ssd130x_init(struct ssd130x_device *ssd130x) /* Set precharge period in number of ticks from the internal clock */ precharge = (SSD130X_SET_PRECHARGE_PERIOD1_SET(ssd130x->prechargep1) | -SSD130X_SET_PRECHARGE_PERIOD1_SET(ssd130x->prechargep2)); +SSD130X_SET_PRECHARGE_PERIOD2_SET(ssd130x->prechargep2)); ret = ssd130x_write_cmd(ssd130x, 2, SSD130X_SET_PRECHARGE_PERIOD, precharge); if (ret < 0) return ret; -- 2.35.1
[PATCH AUTOSEL 5.18 32/54] drm: panel-orientation-quirks: Add quirk for the Lenovo Yoga Tablet 2 830
From: Hans de Goede
[ Upstream commit 144248515246e52a3706de1ee928af29a63794b8 ]
The Lenovo Yoga Tablet 2 830F / 830L use a panel which has been mounted
90 degrees rotated. Add a quirk for this.
Signed-off-by: Hans de Goede
Reviewed-by: Javier Martinez Canillas
Link:
https://patchwork.freedesktop.org/patch/msgid/20220623112710.15693-1-hdego...@redhat.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/drm_panel_orientation_quirks.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index 4e853acfd1e8..cc8556ac673b 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -280,6 +280,21 @@ static const struct dmi_system_id orientation_data[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X9"),
},
.driver_data = (void *)&lcd1200x1920_rightside_up,
+ }, {/* Lenovo Yoga Tablet 2 830F / 830L */
+ .matches = {
+/*
+ * Note this also matches the Lenovo Yoga Tablet 2 1050F/L
+ * since that uses the same mainboard. The resolution match
+ * will limit this to only matching on the 830F/L. Neither has
+ * any external video outputs so those are not a concern.
+ */
+DMI_MATCH(DMI_SYS_VENDOR, "Intel Corp."),
+DMI_MATCH(DMI_PRODUCT_NAME, "VALLEYVIEW C0 PLATFORM"),
+DMI_MATCH(DMI_BOARD_NAME, "BYT-T FFD8"),
+/* Partial match on beginning of BIOS version */
+DMI_MATCH(DMI_BIOS_VERSION, "BLADE_21"),
+ },
+ .driver_data = (void *)&lcd1200x1920_rightside_up,
}, {/* OneGX1 Pro */
.matches = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "SYSTEM_MANUFACTURER"),
--
2.35.1
[PATCH v4 0/2] Remove the VFIO_IOMMU_NOTIFY_DMA_UNMAP notifier
This is the last notifier toward the drivers, replace it with a simple op callback in the vfio_device_ops. v4: - Rebase over the CCW series v3: https://lore.kernel.org/r/0-v3-7593f297c43f+56ce-vfio_unmap_notif_...@nvidia.com - Remove 'nb' doc string from ccw - Rebase on extern removal patch - Check that register_device/unregister_device are either both defined or not - Remove check of dma_unmap during vfio_register_iommu_driver() as it would break the drivers that don't use pin_pages - Don't change VFIO_IOMMU_NOTIFY_DMA_UNMAP to an enum since we are not keeping it anyhow v2: https://lore.kernel.org/r/0-v2-80aa110d03ce+24b-vfio_unmap_notif_...@nvidia.com - Declare and initialize variables in intel_vgpu_dma_unmap() - Remove 'vendor' when touching comments - Remove kdoc for vfio dma_unmap notifier - Add WARN_ON to vfio_register_emulated_iommu_dev() since dma_unmap is mandatory - Move dma_unmap call loop to vfio_notify_dma_unmap() - Document why the double mutex is being used and why the mutex lock is dropped when calling dma_unmap v1: https://lore.kernel.org/r/0-v1-896844109f36+a-vfio_unmap_notif_...@nvidia.com Signed-off-by: Jason Gunthorpe Jason Gunthorpe (2): vfio: Replace the DMA unmapping notifier with a callback vfio: Replace the iommu notifier with a device list drivers/gpu/drm/i915/gvt/gvt.h| 1 - drivers/gpu/drm/i915/gvt/kvmgt.c | 75 +- drivers/s390/cio/vfio_ccw_ops.c | 39 ++ drivers/s390/cio/vfio_ccw_private.h | 2 - drivers/s390/crypto/vfio_ap_ops.c | 53 ++--- drivers/s390/crypto/vfio_ap_private.h | 3 - drivers/vfio/vfio.c | 108 ++ drivers/vfio/vfio.h | 9 +-- drivers/vfio/vfio_iommu_type1.c | 103 +++- include/linux/vfio.h | 21 + 10 files changed, 132 insertions(+), 282 deletions(-) base-commit: 2a8ed7ef00b939fbcc98b948f780bd03bafed227 -- 2.37.1
[PATCH v4 2/2] vfio: Replace the iommu notifier with a device list
Instead of bouncing the function call to the driver op through a blocking
notifier just have the iommu layer call it directly.
Register each device that is being attached to the iommu with the lower
driver which then threads them on a linked list and calls the appropriate
driver op at the right time.
Currently the only use is if dma_unmap() is defined.
Also, fully lock all the debugging tests on the pinning path that a
dma_unmap is registered.
Reviewed-by: Christoph Hellwig
Reviewed-by: Kevin Tian
Signed-off-by: Jason Gunthorpe
---
drivers/vfio/vfio.c | 41 -
drivers/vfio/vfio.h | 12 ++--
drivers/vfio/vfio_iommu_type1.c | 103
include/linux/vfio.h| 2 +-
4 files changed, 81 insertions(+), 77 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 83c375fa242121..b3ce8073cfb1fe 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -231,7 +231,7 @@ int vfio_register_iommu_driver(const struct
vfio_iommu_driver_ops *ops)
{
struct vfio_iommu_driver *driver, *tmp;
- if (WARN_ON(!ops->register_notifier != !ops->unregister_notifier))
+ if (WARN_ON(!ops->register_device != !ops->unregister_device))
return -EINVAL;
driver = kzalloc(sizeof(*driver), GFP_KERNEL);
@@ -1082,17 +1082,6 @@ static void vfio_device_unassign_container(struct
vfio_device *device)
up_write(&device->group->group_rwsem);
}
-static int vfio_iommu_notifier(struct notifier_block *nb, unsigned long action,
- void *data)
-{
- struct vfio_device *vfio_device =
- container_of(nb, struct vfio_device, iommu_nb);
- struct vfio_iommu_type1_dma_unmap *unmap = data;
-
- vfio_device->ops->dma_unmap(vfio_device, unmap->iova, unmap->size);
- return NOTIFY_OK;
-}
-
static struct file *vfio_device_open(struct vfio_device *device)
{
struct vfio_iommu_driver *iommu_driver;
@@ -1128,15 +1117,9 @@ static struct file *vfio_device_open(struct vfio_device
*device)
}
iommu_driver = device->group->container->iommu_driver;
- if (device->ops->dma_unmap && iommu_driver &&
- iommu_driver->ops->register_notifier) {
- unsigned long events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
-
- device->iommu_nb.notifier_call = vfio_iommu_notifier;
- iommu_driver->ops->register_notifier(
- device->group->container->iommu_data, &events,
- &device->iommu_nb);
- }
+ if (iommu_driver && iommu_driver->ops->register_device)
+ iommu_driver->ops->register_device(
+ device->group->container->iommu_data, device);
up_read(&device->group->group_rwsem);
}
@@ -1176,11 +1159,9 @@ static struct file *vfio_device_open(struct vfio_device
*device)
device->ops->close_device(device);
iommu_driver = device->group->container->iommu_driver;
- if (device->ops->dma_unmap && iommu_driver &&
- iommu_driver->ops->unregister_notifier)
- iommu_driver->ops->unregister_notifier(
- device->group->container->iommu_data,
- &device->iommu_nb);
+ if (iommu_driver && iommu_driver->ops->unregister_device)
+ iommu_driver->ops->unregister_device(
+ device->group->container->iommu_data, device);
}
err_undo_count:
up_read(&device->group->group_rwsem);
@@ -1385,11 +1366,9 @@ static int vfio_device_fops_release(struct inode *inode,
struct file *filep)
device->ops->close_device(device);
iommu_driver = device->group->container->iommu_driver;
- if (device->ops->dma_unmap && iommu_driver &&
- iommu_driver->ops->unregister_notifier)
- iommu_driver->ops->unregister_notifier(
- device->group->container->iommu_data,
- &device->iommu_nb);
+ if (iommu_driver && iommu_driver->ops->unregister_device)
+ iommu_driver->ops->unregister_device(
+ device->group->container->iommu_data, device);
up_read(&device->group->group_rwsem);
device->open_count--;
if (device->open_count == 0)
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index 25da02ca1568fc..4a7db1f3c33e7e 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -33,9 +33,6 @@ enum vfio_iommu_notify_type {
VFIO_IOMMU_CONTAINER_CLOSE = 0,
};
-/* events for register_notifier() */
-#define VFIO_IOMMU_NOTIFY_DMA_UNMAP BIT(0)
-
/**
* struct vfio_iommu_driver_ops - VFIO IOMMU driver callbacks
*/
@@ -58,11 +55,10 @@ struct vfio_iommu_driver_ops
[PATCH v4 1/2] vfio: Replace the DMA unmapping notifier with a callback
Instead of having drivers register the notifier with explicit code just
have them provide a dma_unmap callback op in their driver ops and rely on
the core code to wire it up.
Suggested-by: Christoph Hellwig
Reviewed-by: Christoph Hellwig
Reviewed-by: Kevin Tian
Reviewed-by: Tony Krowiak
Reviewed-by: Eric Farman
Reviewed-by: Zhenyu Wang
Signed-off-by: Jason Gunthorpe
---
drivers/gpu/drm/i915/gvt/gvt.h| 1 -
drivers/gpu/drm/i915/gvt/kvmgt.c | 75 ---
drivers/s390/cio/vfio_ccw_ops.c | 39 ++--
drivers/s390/cio/vfio_ccw_private.h | 2 -
drivers/s390/crypto/vfio_ap_ops.c | 53 ++-
drivers/s390/crypto/vfio_ap_private.h | 3 -
drivers/vfio/vfio.c | 129 +-
drivers/vfio/vfio.h | 3 +
include/linux/vfio.h | 21 +
9 files changed, 86 insertions(+), 240 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index aee1a45da74bcb..705689e6401197 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -226,7 +226,6 @@ struct intel_vgpu {
unsigned long nr_cache_entries;
struct mutex cache_lock;
- struct notifier_block iommu_notifier;
atomic_t released;
struct kvm_page_track_notifier_node track_node;
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index e2f6c56ab3420c..ecd5bb37b63a2a 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -729,34 +729,25 @@ int intel_gvt_set_edid(struct intel_vgpu *vgpu, int
port_num)
return ret;
}
-static int intel_vgpu_iommu_notifier(struct notifier_block *nb,
-unsigned long action, void *data)
+static void intel_vgpu_dma_unmap(struct vfio_device *vfio_dev, u64 iova,
+u64 length)
{
- struct intel_vgpu *vgpu =
- container_of(nb, struct intel_vgpu, iommu_notifier);
-
- if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) {
- struct vfio_iommu_type1_dma_unmap *unmap = data;
- struct gvt_dma *entry;
- unsigned long iov_pfn, end_iov_pfn;
+ struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
+ struct gvt_dma *entry;
+ u64 iov_pfn = iova >> PAGE_SHIFT;
+ u64 end_iov_pfn = iov_pfn + length / PAGE_SIZE;
- iov_pfn = unmap->iova >> PAGE_SHIFT;
- end_iov_pfn = iov_pfn + unmap->size / PAGE_SIZE;
+ mutex_lock(&vgpu->cache_lock);
+ for (; iov_pfn < end_iov_pfn; iov_pfn++) {
+ entry = __gvt_cache_find_gfn(vgpu, iov_pfn);
+ if (!entry)
+ continue;
- mutex_lock(&vgpu->cache_lock);
- for (; iov_pfn < end_iov_pfn; iov_pfn++) {
- entry = __gvt_cache_find_gfn(vgpu, iov_pfn);
- if (!entry)
- continue;
-
- gvt_dma_unmap_page(vgpu, entry->gfn, entry->dma_addr,
- entry->size);
- __gvt_cache_remove_entry(vgpu, entry);
- }
- mutex_unlock(&vgpu->cache_lock);
+ gvt_dma_unmap_page(vgpu, entry->gfn, entry->dma_addr,
+ entry->size);
+ __gvt_cache_remove_entry(vgpu, entry);
}
-
- return NOTIFY_OK;
+ mutex_unlock(&vgpu->cache_lock);
}
static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
@@ -783,36 +774,20 @@ static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
{
struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
- unsigned long events;
- int ret;
-
- vgpu->iommu_notifier.notifier_call = intel_vgpu_iommu_notifier;
- events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
- ret = vfio_register_notifier(vfio_dev, VFIO_IOMMU_NOTIFY, &events,
-&vgpu->iommu_notifier);
- if (ret != 0) {
- gvt_vgpu_err("vfio_register_notifier for iommu failed: %d\n",
- ret);
- goto out;
- }
-
- ret = -EEXIST;
if (vgpu->attached)
- goto undo_iommu;
+ return -EEXIST;
- ret = -ESRCH;
if (!vgpu->vfio_device.kvm ||
vgpu->vfio_device.kvm->mm != current->mm) {
gvt_vgpu_err("KVM is required to use Intel vGPU\n");
- goto undo_iommu;
+ return -ESRCH;
}
kvm_get_kvm(vgpu->vfio_device.kvm);
- ret = -EEXIST;
if (__kvmgt_vgpu_exist(vgpu))
- goto undo_iommu;
+ return -EEXIST;
vgpu->attached = true;
@@ -831,12 +806,6 @@ static int intel_vgpu_open_device(struct vfio_device
*vfio_dev)
atomic_set(&vgpu->released, 0
Re: [PATCH v3 1/2] vfio: Replace the DMA unmapping notifier with a callback
On Thu, Jul 07, 2022 at 03:37:16PM -0600, Alex Williamson wrote:
> On Mon, 4 Jul 2022 21:59:03 -0300
> Jason Gunthorpe wrote:
> > diff --git a/drivers/s390/cio/vfio_ccw_ops.c
> > b/drivers/s390/cio/vfio_ccw_ops.c
> > index b49e2e9db2dc6f..09e0ce7b72324c 100644
> > --- a/drivers/s390/cio/vfio_ccw_ops.c
> > +++ b/drivers/s390/cio/vfio_ccw_ops.c
> > @@ -44,31 +44,19 @@ static int vfio_ccw_mdev_reset(struct vfio_ccw_private
> > *private)
> > return ret;
> > }
> >
> > -static int vfio_ccw_mdev_notifier(struct notifier_block *nb,
> > - unsigned long action,
> > - void *data)
> > +static void vfio_ccw_dma_unmap(struct vfio_device *vdev, u64 iova, u64
> > length)
> > {
> > struct vfio_ccw_private *private =
> > - container_of(nb, struct vfio_ccw_private, nb);
> > -
> > - /*
> > -* Vendor drivers MUST unpin pages in response to an
> > -* invalidation.
> > -*/
> > - if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) {
> > - struct vfio_iommu_type1_dma_unmap *unmap = data;
> > -
> > - if (!cp_iova_pinned(&private->cp, unmap->iova))
> > - return NOTIFY_OK;
> > + container_of(vdev, struct vfio_ccw_private, vdev);
> >
> > - if (vfio_ccw_mdev_reset(private))
> > - return NOTIFY_BAD;
> > + /* Drivers MUST unpin pages in response to an invalidation. */
> > + if (!cp_iova_pinned(&private->cp, iova))
> > + return;
> >
> > - cp_free(&private->cp);
> > - return NOTIFY_OK;
> > - }
> > + if (vfio_ccw_mdev_reset(private))
> > + return;
> >
> > - return NOTIFY_DONE;
> > + cp_free(&private->cp);
> > }
>
>
> The cp_free() call is gone here with [1], so I think this function now
> just ends with:
>
> ...
> vfio_ccw_mdev_reset(private);
> }
>
> There are also minor contextual differences elsewhere from that series,
> so a quick respin to record the changes on list would be appreciated.
>
> However the above kind of highlights that NOTIFY_BAD that silently gets
> dropped here. I realize we weren't testing the return value of the
> notifier call chain and really we didn't intend that notifiers could
> return a failure here, but does this warrant some logging or suggest
> future work to allow a device to go offline here? Thanks.
It looks like no.
If the FSM trapped in a bad state here, such as
VFIO_CCW_STATE_NOT_OPER, then it means it should have already unpinned
the pages and this is considered a success for this purpose
The return code here exists only to return to userspace so it can
detect during a VFIO_DEVICE_RESET that the device has crashed
irrecoverably.
Thus just continuing to silently ignore it seems like the best thing.
Jason
Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT
Hi,
On Tue, Jul 19, 2022 at 3:45 PM Doug Anderson wrote:
>
> Hi,
>
> On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> wrote:
> >
> > The value read from the "hpd-absent-delay-ms" property in DT was being
> > saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> > typo.
> >
> > Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed
> > by EDID")
> > Signed-off-by: Nícolas F. R. A. Prado
> > ---
> >
> > drivers/gpu/drm/panel/panel-edp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c
> > b/drivers/gpu/drm/panel/panel-edp.c
> > index 152e00eb846f..b3536d8600f4 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -738,7 +738,7 @@ static int generic_edp_panel_probe(struct device *dev,
> > struct panel_edp *panel)
> > of_property_read_u32(dev->of_node, "hpd-reliable-delay-ms",
> > &reliable_ms);
> > desc->delay.hpd_reliable = reliable_ms;
> > of_property_read_u32(dev->of_node, "hpd-absent-delay-ms",
> > &absent_ms);
> > - desc->delay.hpd_reliable = absent_ms;
> > + desc->delay.hpd_absent = absent_ms;
>
> Well that's embarrassing. In the end I never used any of these
> properties for anything shipping since HPD was always hooked up on
> later boards and the only board that needed "hpd_reliable" never ended
> up switching to the generic "edp-panel".
>
> Reviewed-by: Douglas Anderson
>
> I'll apply this right away to drm-misc-fixes.
ef2084a8388b drm/panel-edp: Fix variable typo when saving hpd absent
delay from DT
Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT
Hi,
On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
wrote:
>
> The value read from the "hpd-absent-delay-ms" property in DT was being
> saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> typo.
>
> Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by
> EDID")
> Signed-off-by: Nícolas F. R. A. Prado
> ---
>
> drivers/gpu/drm/panel/panel-edp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c
> b/drivers/gpu/drm/panel/panel-edp.c
> index 152e00eb846f..b3536d8600f4 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -738,7 +738,7 @@ static int generic_edp_panel_probe(struct device *dev,
> struct panel_edp *panel)
> of_property_read_u32(dev->of_node, "hpd-reliable-delay-ms",
> &reliable_ms);
> desc->delay.hpd_reliable = reliable_ms;
> of_property_read_u32(dev->of_node, "hpd-absent-delay-ms", &absent_ms);
> - desc->delay.hpd_reliable = absent_ms;
> + desc->delay.hpd_absent = absent_ms;
Well that's embarrassing. In the end I never used any of these
properties for anything shipping since HPD was always hooked up on
later boards and the only board that needed "hpd_reliable" never ended
up switching to the generic "edp-panel".
Reviewed-by: Douglas Anderson
I'll apply this right away to drm-misc-fixes.
Re: [PATCH 2/3] drm/panel-edp: Add panel entry for B120XAN01.0
Hi,
On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
wrote:
>
> Add panel identification entry for the AUO B120XAN01.0 (product ID:
> 0x1062) panel.
>
> Signed-off-by: Nícolas F. R. A. Prado
> ---
>
> drivers/gpu/drm/panel/panel-edp.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c
> b/drivers/gpu/drm/panel/panel-edp.c
> index 675d793d925e..152e00eb846f 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1879,6 +1879,7 @@ static const struct edp_panel_entry edp_panels[] = {
> EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay,
> "B116XAK01"),
> EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50,
> "B116XAN06.1"),
> EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50,
> "B133UAN01.0"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0x1062, &delay_200_500_e50,
> "B120XAN01.0"),
* Sort first by vendor, then by product ID.
0x1062 sorts at the top of the AUO panels, not at the bottom.
-Doug
Re: [PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH
Hi,
On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
wrote:
>
> Add panel identification entry for the IVO R140NWF5 RH (product ID:
> 0x057d) panel.
>
> Signed-off-by: Nícolas F. R. A. Prado
>
> ---
> The comments on the driver indicate that the T3 timing should be set on
> hpd_absent, while hpd_reliable would have a shorter time just while the
> HPD line stabilizes on low after power is supplied.
Right. Ideally hpd_reliable is 0 unless you've got a badly-designed panel.
> But can we really assume that the HPD line will be reliable at all
> before the DDIC is done booting up, at which point the HPD line is
> brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
> T3), since only then we're really sure that the DDIC is done setting up
> and the HPD line is reliable?
If the panel is hooked up properly, then the HPD pin should be pulled
low at the start and then should only go high after the panel is ready
for us to talk to it, right? So it's not like the DDIC has to boot up
and actively init the state. I would assume that the initial state of
the "HPD output" from the panel's IC would be one of the following:
* A floating input.
* A pulled down input.
* An output driven low.
In any of those cases just adding a pull down on the line would be
enough to ensure that the HPD line is reliable until the panel comes
around and actively drives the line high.
Remember, this is eDP and it's not something that's hot-plugged, so
there's no debouncing involved and in a properly designed system there
should be no time needed for the signal to stabilize. I would also
point out that on the oficial eDP docs the eDP timing diagram doesn't
show the initial state of "HPD" as "unknown". It shows it as low.
Now, that all being said, I have seen at least one panel that glitched
itself at bootup. After you powered it on it would blip its HPD line
high before it had actually finished booting. Then the HPD would go
low again before finally going high after the panel finished booting.
This is the reason for "hpd_reliable".
If you've got a board with a well-designed panel but the hookup
between the panel and the board is wrong (maybe the board is missing a
pulldown on the HPD line?) then you can just set the "no-hpd" property
for your board. That will tell the kernel to just always delay the
"hpd-absent" delay.
> I've set the T3 delay to hpd_absent in this series, following what's
> instructed in the comments, but I'd like to discuss whether we shouldn't
> be setting T3 on hpd_reliable instead, for all panels, to be on the
> safer side.
The way it's specified right now is more flexible, though, isn't it?
This way if you're on a board where the HPD truly _isn't_ stable then
you can just set the "no-hpd" and it will automatically use the
"hpd_absent" delay, right?
> drivers/gpu/drm/panel/panel-edp.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c
> b/drivers/gpu/drm/panel/panel-edp.c
> index 3626469c4cc2..675d793d925e 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
> .enable = 200,
> };
>
> +static const struct panel_delay delay_200_500_e200 = {
> + .hpd_absent = 200,
> + .unprepare = 500,
> + .enable = 200,
> +};
> +
> #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id,
> _delay, _name) \
> { \
> .name = _name, \
> @@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
>
> EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay,
> "N116BCA-EA1"),
>
> + EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5
> RH"),
> +
This looks fine to me:
Reviewed-by: Douglas Anderson
I'm happy to apply this in a day or two assuming you're OK with my
explanation above.
Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT
Às 17:38 de 19/07/22, Nícolas F. R. A. Prado escreveu:
> The value read from the "hpd-absent-delay-ms" property in DT was being
> saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> typo.
>
> Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by
> EDID")
> Signed-off-by: Nícolas F. R. A. Prado
Reviewed-by: André Almeida
[PATCH] dt-bindings: panel: raydium, rm67191: Add missing type to 'video-mode'
'video-mode' is missing a type definition and is not a common property. The type is 'uint32'. Signed-off-by: Rob Herring --- .../devicetree/bindings/display/panel/raydium,rm67191.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.yaml b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.yaml index 617aa8c8c03a..d62fd692bf10 100644 --- a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.yaml +++ b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.yaml @@ -38,6 +38,7 @@ properties: 0 - burst-mode 1 - non-burst with sync event 2 - non-burst with sync pulse +$ref: /schemas/types.yaml#/definitions/uint32 enum: [0, 1, 2] required: -- 2.34.1
[PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH
Add panel identification entry for the IVO R140NWF5 RH (product ID:
0x057d) panel.
Signed-off-by: Nícolas F. R. A. Prado
---
The comments on the driver indicate that the T3 timing should be set on
hpd_absent, while hpd_reliable would have a shorter time just while the
HPD line stabilizes on low after power is supplied.
But can we really assume that the HPD line will be reliable at all
before the DDIC is done booting up, at which point the HPD line is
brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
T3), since only then we're really sure that the DDIC is done setting up
and the HPD line is reliable?
I've set the T3 delay to hpd_absent in this series, following what's
instructed in the comments, but I'd like to discuss whether we shouldn't
be setting T3 on hpd_reliable instead, for all panels, to be on the
safer side.
drivers/gpu/drm/panel/panel-edp.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 3626469c4cc2..675d793d925e 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
.enable = 200,
};
+static const struct panel_delay delay_200_500_e200 = {
+ .hpd_absent = 200,
+ .unprepare = 500,
+ .enable = 200,
+};
+
#define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id,
_delay, _name) \
{ \
.name = _name, \
@@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay,
"N116BCA-EA1"),
+ EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5
RH"),
+
EDP_PANEL_ENTRY('K', 'D', 'B', 0x0624,
&kingdisplay_kd116n21_30nv_a010.delay, "116N21-30NV-A010"),
EDP_PANEL_ENTRY('K', 'D', 'B', 0x1120, &delay_200_500_e80_d50,
"116N29-30NK-C007"),
--
2.37.0
[PATCH 0/3] New eDP panels and a bugfix
This series adds two new eDP panel entries in the first two patches, and fixes a typo in the third one that prevented usage of the DT properties. Please see the note in patch 1 for a question on hpd_reliable vs. hpd_absent. Nícolas F. R. A. Prado (3): drm/panel-edp: Add panel entry for R140NWF5 RH drm/panel-edp: Add panel entry for B120XAN01.0 drm/panel-edp: Fix variable typo when saving hpd absent delay from DT drivers/gpu/drm/panel/panel-edp.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) -- 2.37.0
[PATCH 2/3] drm/panel-edp: Add panel entry for B120XAN01.0
Add panel identification entry for the AUO B120XAN01.0 (product ID:
0x1062) panel.
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/panel/panel-edp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 675d793d925e..152e00eb846f 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1879,6 +1879,7 @@ static const struct edp_panel_entry edp_panels[] = {
EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay,
"B116XAK01"),
EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50,
"B116XAN06.1"),
EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50,
"B133UAN01.0"),
+ EDP_PANEL_ENTRY('A', 'U', 'O', 0x1062, &delay_200_500_e50,
"B120XAN01.0"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x0786, &delay_200_500_p2e80,
"NV116WHM-T01"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x07d1, &boe_nv133fhm_n61.delay,
"NV133FHM-N61"),
--
2.37.0
[PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT
The value read from the "hpd-absent-delay-ms" property in DT was being
saved to the wrong variable, overriding the hpd_reliable delay. Fix the
typo.
Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by
EDID")
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/panel/panel-edp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 152e00eb846f..b3536d8600f4 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -738,7 +738,7 @@ static int generic_edp_panel_probe(struct device *dev,
struct panel_edp *panel)
of_property_read_u32(dev->of_node, "hpd-reliable-delay-ms",
&reliable_ms);
desc->delay.hpd_reliable = reliable_ms;
of_property_read_u32(dev->of_node, "hpd-absent-delay-ms", &absent_ms);
- desc->delay.hpd_reliable = absent_ms;
+ desc->delay.hpd_absent = absent_ms;
/* Power the panel on so we can read the EDID */
ret = pm_runtime_get_sync(dev);
--
2.37.0
Re: [PATCH v1 4/6] dma-buf: Acquire wait-wound context on attachment
On 7/15/22 09:59, Dmitry Osipenko wrote:
> On 7/15/22 09:50, Christian König wrote:
>> Am 15.07.22 um 02:52 schrieb Dmitry Osipenko:
>>> Intel i915 GPU driver uses wait-wound mutex to lock multiple GEMs on the
>>> attachment to the i915 dma-buf. In order to let all drivers utilize
>>> shared
>>> wait-wound context during attachment in a general way, make dma-buf
>>> core to
>>> acquire the ww context internally for the attachment operation and update
>>> i915 driver to use the importer's ww context instead of the internal one.
>>>
>>> From now on all dma-buf exporters shall use the importer's ww context
>>> for
>>> the attachment operation.
>>>
>>> Signed-off-by: Dmitry Osipenko
>>> ---
>>> drivers/dma-buf/dma-buf.c | 8 +-
>>> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +-
>>> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
>>> drivers/gpu/drm/i915/gem/i915_gem_object.h | 6 ++---
>>> drivers/gpu/drm/i915/i915_gem_evict.c | 2 +-
>>> drivers/gpu/drm/i915/i915_gem_ww.c | 26 +++
>>> drivers/gpu/drm/i915/i915_gem_ww.h | 15 +--
>>> 7 files changed, 47 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index 0ee588276534..37545ecb845a 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -807,6 +807,8 @@ static struct sg_table * __map_dma_buf(struct
>>> dma_buf_attachment *attach,
>>> * Optionally this calls &dma_buf_ops.attach to allow
>>> device-specific attach
>>> * functionality.
>>> *
>>> + * Exporters shall use ww_ctx acquired by this function.
>>> + *
>>> * Returns:
>>> *
>>> * A pointer to newly created &dma_buf_attachment on success, or a
>>> negative
>>> @@ -822,6 +824,7 @@ dma_buf_dynamic_attach_unlocked(struct dma_buf
>>> *dmabuf, struct device *dev,
>>> void *importer_priv)
>>> {
>>> struct dma_buf_attachment *attach;
>>> + struct ww_acquire_ctx ww_ctx;
>>> int ret;
>>> if (WARN_ON(!dmabuf || !dev))
>>> @@ -841,7 +844,8 @@ dma_buf_dynamic_attach_unlocked(struct dma_buf
>>> *dmabuf, struct device *dev,
>>> attach->importer_ops = importer_ops;
>>> attach->importer_priv = importer_priv;
>>> - dma_resv_lock(dmabuf->resv, NULL);
>>> + ww_acquire_init(&ww_ctx, &reservation_ww_class);
>>> + dma_resv_lock(dmabuf->resv, &ww_ctx);
>>
>> That won't work like this. The core property of a WW context is that you
>> need to unwind all the locks and re-quire them with the contended one
>> first.
>>
>> When you statically lock the imported one here you can't do that any more.
>
> You're right. I felt that something is missing here, but couldn't
> notice. I'll think more about this and enable
> CONFIG_DEBUG_WW_MUTEX_SLOWPATH. Thank you!
>
Christian, do you think we could make an excuse for the attach()
callback and make the exporter responsible for taking the resv lock? It
will be inconsistent with the rest of the callbacks, where importer
takes the lock, but it will be the simplest and least invasive solution.
It's very messy to do a cross-driver ww locking, I don't think it's the
right approach.
--
Best regards,
Dmitry
Re: [PATCH v2] drm/i915/guc: support v69 in parallel to v70
On 7/18/2022 16:07, Daniele Ceraolo Spurio wrote:
This patch re-introduces support for GuC v69 in parallel to v70. As this
is a quick fix, v69 has been re-introduced as the single "fallback" guc
version in case v70 is not available on disk and only for platforms that
are out of force_probe and require the GuC by default. All v69 specific
code has been labeled as such for easy identification, and the same was
done for all v70 functions for which there is a separate v69 version,
to avoid accidentally calling the wrong version via the unlabeled name.
When the fallback mode kicks in, a drm_notice message is printed in
dmesg to inform the user of the required update. The existing
logging of the fetch function has also been updated so that we no
longer complain immediately if we can't find a fw and we only throw an
error if the fetch of both the base and fallback blobs fails.
The plan is to follow this up with a more complex rework to allow for
multiple different GuC versions to be supported at the same time.
v2: reduce the fallback to platform that require it, switch to
firmware_request_nowarn(), improve logs.
Fixes: 2584b3549f4c ("drm/i915/guc: Update to GuC version 70.1.1")
Link: https://lists.freedesktop.org/archives/intel-gfx/2022-July/301640.html
Signed-off-by: Daniele Ceraolo Spurio
Cc: John Harrison
Cc: Matthew Brost
Cc: Matt Roper
Cc: Dave Airlie
Cc: Michal Wajdeczko
Acked-by: Rodrigo Vivi
Reviewed-by: John Harrison
---
drivers/gpu/drm/i915/gt/intel_context_types.h | 11 +-
.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 3 +
drivers/gpu/drm/i915/gt/uc/intel_guc.h| 5 +
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 45 +++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 352 +++---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 56 ++-
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 7 +
7 files changed, 417 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index d2d75d9c0c8d..04eacae1aca5 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -275,10 +275,17 @@ struct intel_context {
u8 child_index;
/** @guc: GuC specific members for parallel submission */
struct {
- /** @wqi_head: head pointer in work queue */
+ /** @wqi_head: cached head pointer in work queue */
u16 wqi_head;
- /** @wqi_tail: tail pointer in work queue */
+ /** @wqi_tail: cached tail pointer in work queue */
u16 wqi_tail;
+ /** @wq_head: pointer to the actual head in work queue
*/
+ u32 *wq_head;
+ /** @wq_tail: pointer to the actual head in work queue
*/
+ u32 *wq_tail;
+ /** @wq_status: pointer to the status in work queue */
+ u32 *wq_status;
+
/**
* @parent_page: page in context state (ce->state) used
* by parent for work queue, process descriptor
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
index 4ef9990ed7f8..29ef8afc8c2e 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -122,6 +122,9 @@ enum intel_guc_action {
INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_DONE = 0x1002,
INTEL_GUC_ACTION_SCHED_ENGINE_MODE_SET = 0x1003,
INTEL_GUC_ACTION_SCHED_ENGINE_MODE_DONE = 0x1004,
+ INTEL_GUC_ACTION_V69_SET_CONTEXT_PRIORITY = 0x1005,
+ INTEL_GUC_ACTION_V69_SET_CONTEXT_EXECUTION_QUANTUM = 0x1006,
+ INTEL_GUC_ACTION_V69_SET_CONTEXT_PREEMPTION_TIMEOUT = 0x1007,
INTEL_GUC_ACTION_CONTEXT_RESET_NOTIFICATION = 0x1008,
INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
INTEL_GUC_ACTION_HOST2GUC_UPDATE_CONTEXT_POLICIES = 0x100B,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index d0d99f178f2d..a7acffbf15d1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -170,6 +170,11 @@ struct intel_guc {
/** @ads_engine_usage_size: size of engine usage in the ADS */
u32 ads_engine_usage_size;
+ /** @lrc_desc_pool_v69: object allocated to hold the GuC LRC descriptor pool */
+ struct i915_vma *lrc_desc_pool_v69;
+ /** @lrc_desc_pool_vaddr_v69: contents of the GuC LRC descriptor pool */
+ void *lrc_desc_pool_vaddr_v69;
+
/**
* @context_lookup: used to resolve intel_context from guc_id, if a
* context is present in this structure it is registered with the GuC
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
b/drivers
Re: [PATCH] drm/amdgpu: Fix comment typo
Applied. Thanks! On Tue, Jul 19, 2022 at 8:34 AM Christian König wrote: > > Am 16.07.22 um 06:28 schrieb Jason Wang: > > The double `to' is duplicated in the comment, remove one. > > > > Signed-off-by: Jason Wang > > Reviewed-by: Christian König > > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > index e3d139708160..b45cd7cbbea8 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > @@ -80,7 +80,7 @@ > >* - 3.24.0 - Add high priority compute support for gfx9 > >* - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk). > >* - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE. > > - * - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation. > > + * - 3.27.0 - Add new chunk to AMDGPU_CS to enable BO_LIST creation. > >* - 3.28.0 - Add AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES > >* - 3.29.0 - Add AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID > >* - 3.30.0 - Add AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE. >
Re: [PATCH] drm/radeon: Fix comment typo
Applied. Thanks! On Tue, Jul 19, 2022 at 8:33 AM Christian König wrote: > > Am 16.07.22 um 05:57 schrieb Jason Wang: > > The double `have' is duplicated in line 696, remove one. > > > > Signed-off-by: Jason Wang > > Reviewed-by: Christian König > > > --- > > drivers/gpu/drm/radeon/radeon_gem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c > > b/drivers/gpu/drm/radeon/radeon_gem.c > > index 84843b3b3aef..261fcbae88d7 100644 > > --- a/drivers/gpu/drm/radeon/radeon_gem.c > > +++ b/drivers/gpu/drm/radeon/radeon_gem.c > > @@ -693,7 +693,7 @@ int radeon_gem_va_ioctl(struct drm_device *dev, void > > *data, > > } > > > > /* !! DONT REMOVE !! > > - * We don't support vm_id yet, to be sure we don't have have broken > > + * We don't support vm_id yet, to be sure we don't have broken > >* userspace, reject anyone trying to use non 0 value thus moving > >* forward we can use those fields without breaking existant userspace > >*/ >
Re: [PATCH v2 09/13] drm/gem: Add LRU/shrinker helper
On 7/19/22 20:18, Rob Clark wrote:
> +void
> +drm_gem_lru_move_tail_locked(struct drm_gem_lru *lru, struct drm_gem_object
> *obj)
> +{
> + WARN_ON(!mutex_is_locked(lru->lock));
Nit: What about lockdep_assert_held_once(&lru->lock->base)) ?
Otherwise, looks good! I'll use it for the DRM-SHMEM shrinker after
completing the work on the dma-buf locks.
Reviewed-by: Dmitry Osipenko
--
Best regards,
Dmitry
Re: [RFC PATCH v3 1/2] drm/bridge: ti-sn65dsi86: fetch bpc using drm_atomic_state
Hi, On Mon, Jul 11, 2022 at 2:21 AM Dmitry Baryshkov wrote: > > Rather than reading the pdata->connector directly, fetch the connector > using drm_atomic_state. This allows us to make pdata->connector optional > (and thus supporting DRM_BRIDGE_ATTACH_NO_CONNECTOR). > > Reviewed-by: Sam Ravnborg > Reviewed-by: Laurent Pinchart > Signed-off-by: Dmitry Baryshkov > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 22 -- > 1 file changed, 16 insertions(+), 6 deletions(-) Landed on drm-misc-next: 2dbeef82d14f drm/bridge: ti-sn65dsi86: fetch bpc using drm_atomic_state
[PATCH v2 08/13] drm/msm/gem: Remove active refcnt
From: Rob Clark
At this point the pinned refcnt is sufficient, and the shrinker is
already prepared to encounter objects which are still active according
to fences attached to the resv.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem.c| 45 ++--
drivers/gpu/drm/msm/msm_gem.h| 14 ++---
drivers/gpu/drm/msm/msm_gem_submit.c | 22 ++
3 files changed, 8 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 407b18a24dc4..209438744bab 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -734,8 +734,7 @@ int msm_gem_madvise(struct drm_gem_object *obj, unsigned
madv)
/* If the obj is inactive, we might need to move it
* between inactive lists
*/
- if (msm_obj->active_count == 0)
- update_lru(obj);
+ update_lru(obj);
msm_gem_unlock(obj);
@@ -788,7 +787,6 @@ void msm_gem_evict(struct drm_gem_object *obj)
GEM_WARN_ON(!msm_gem_is_locked(obj));
GEM_WARN_ON(is_unevictable(msm_obj));
GEM_WARN_ON(!msm_obj->evictable);
- GEM_WARN_ON(msm_obj->active_count);
/* Get rid of any iommu mapping(s): */
put_iova_spaces(obj, false);
@@ -813,37 +811,6 @@ void msm_gem_vunmap(struct drm_gem_object *obj)
msm_obj->vaddr = NULL;
}
-void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu)
-{
- struct msm_gem_object *msm_obj = to_msm_bo(obj);
- struct msm_drm_private *priv = obj->dev->dev_private;
-
- might_sleep();
- GEM_WARN_ON(!msm_gem_is_locked(obj));
- GEM_WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED);
- GEM_WARN_ON(msm_obj->dontneed);
-
- if (msm_obj->active_count++ == 0) {
- mutex_lock(&priv->mm_lock);
- if (msm_obj->evictable)
- mark_unevictable(msm_obj);
- list_move_tail(&msm_obj->mm_list, &gpu->active_list);
- mutex_unlock(&priv->mm_lock);
- }
-}
-
-void msm_gem_active_put(struct drm_gem_object *obj)
-{
- struct msm_gem_object *msm_obj = to_msm_bo(obj);
-
- might_sleep();
- GEM_WARN_ON(!msm_gem_is_locked(obj));
-
- if (--msm_obj->active_count == 0) {
- update_lru(obj);
- }
-}
-
static void update_lru(struct drm_gem_object *obj)
{
struct msm_drm_private *priv = obj->dev->dev_private;
@@ -851,9 +818,6 @@ static void update_lru(struct drm_gem_object *obj)
GEM_WARN_ON(!msm_gem_is_locked(&msm_obj->base));
- if (msm_obj->active_count != 0)
- return;
-
mutex_lock(&priv->mm_lock);
if (msm_obj->dontneed)
@@ -926,7 +890,7 @@ void msm_gem_describe(struct drm_gem_object *obj, struct
seq_file *m,
stats->all.count++;
stats->all.size += obj->size;
- if (is_active(msm_obj)) {
+ if (msm_gem_active(obj)) {
stats->active.count++;
stats->active.size += obj->size;
}
@@ -954,7 +918,7 @@ void msm_gem_describe(struct drm_gem_object *obj, struct
seq_file *m,
}
seq_printf(m, "%08x: %c %2d (%2d) %08llx %p",
- msm_obj->flags, is_active(msm_obj) ? 'A' : 'I',
+ msm_obj->flags, msm_gem_active(obj) ? 'A' : 'I',
obj->name, kref_read(&obj->refcount),
off, msm_obj->vaddr);
@@ -1037,9 +1001,6 @@ static void msm_gem_free_object(struct drm_gem_object
*obj)
list_del(&msm_obj->mm_list);
mutex_unlock(&priv->mm_lock);
- /* object should not be on active list: */
- GEM_WARN_ON(is_active(msm_obj));
-
put_iova_spaces(obj, true);
if (obj->import_attach) {
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 6fe521ccda45..420ba49bf21a 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -138,7 +138,6 @@ struct msm_gem_object {
char name[32]; /* Identifier to print for the debugfs files */
- int active_count;
int pin_count;
};
#define to_msm_bo(x) container_of(x, struct msm_gem_object, base)
@@ -171,8 +170,6 @@ void *msm_gem_get_vaddr_active(struct drm_gem_object *obj);
void msm_gem_put_vaddr_locked(struct drm_gem_object *obj);
void msm_gem_put_vaddr(struct drm_gem_object *obj);
int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv);
-void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu);
-void msm_gem_active_put(struct drm_gem_object *obj);
bool msm_gem_active(struct drm_gem_object *obj);
int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t
*timeout);
int msm_gem_cpu_fini(struct drm_gem_object *obj);
@@ -245,12 +242,6 @@ msm_gem_is_locked(struct drm_gem_object *obj)
return dma_resv_is_locked(obj->resv) || (kref_read(&obj->refcount) ==
0);
}
-static inline bool is_active(struct msm_gem_ob
[PATCH v2 12/13] drm/msm/gem: Consolidate shrinker trace
From: Rob Clark
Combine separate trace events for purge vs evict into one. When we add
support for purging/evicting active buffers we'll just add more info
into this one trace event, rather than adding a bunch more events.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem_shrinker.c | 19 ++-
drivers/gpu/drm/msm/msm_gpu_trace.h| 32 +++---
2 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c
b/drivers/gpu/drm/msm/msm_gem_shrinker.c
index 530b1102b46d..5cc05d669a08 100644
--- a/drivers/gpu/drm/msm/msm_gem_shrinker.c
+++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c
@@ -71,25 +71,20 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct
shrink_control *sc)
struct msm_drm_private *priv =
container_of(shrinker, struct msm_drm_private, shrinker);
long nr = sc->nr_to_scan;
- unsigned long freed;
+ unsigned long freed, purged, evicted = 0;
- freed = drm_gem_lru_scan(&priv->lru.dontneed, nr, purge);
- nr -= freed;
-
- if (freed > 0)
- trace_msm_gem_purge(freed << PAGE_SHIFT);
+ purged = drm_gem_lru_scan(&priv->lru.dontneed, nr, purge);
+ nr -= purged;
if (can_swap() && nr > 0) {
- unsigned long evicted;
-
evicted = drm_gem_lru_scan(&priv->lru.willneed, nr, evict);
nr -= evicted;
+ }
- if (evicted > 0)
- trace_msm_gem_evict(evicted << PAGE_SHIFT);
+ freed = purged + evicted;
- freed += evicted;
- }
+ if (freed)
+ trace_msm_gem_shrink(sc->nr_to_scan, purged, evicted);
return (freed > 0) ? freed : SHRINK_STOP;
}
diff --git a/drivers/gpu/drm/msm/msm_gpu_trace.h
b/drivers/gpu/drm/msm/msm_gpu_trace.h
index ca0b08d7875b..8867fa0a0306 100644
--- a/drivers/gpu/drm/msm/msm_gpu_trace.h
+++ b/drivers/gpu/drm/msm/msm_gpu_trace.h
@@ -115,29 +115,23 @@ TRACE_EVENT(msm_gmu_freq_change,
);
-TRACE_EVENT(msm_gem_purge,
- TP_PROTO(u32 bytes),
- TP_ARGS(bytes),
+TRACE_EVENT(msm_gem_shrink,
+ TP_PROTO(u32 nr_to_scan, u32 purged, u32 evicted),
+ TP_ARGS(nr_to_scan, purged, evicted),
TP_STRUCT__entry(
- __field(u32, bytes)
+ __field(u32, nr_to_scan)
+ __field(u32, purged)
+ __field(u32, evicted)
),
TP_fast_assign(
- __entry->bytes = bytes;
+ __entry->nr_to_scan = nr_to_scan;
+ __entry->purged = purged;
+ __entry->evicted = evicted;
),
- TP_printk("Purging %u bytes", __entry->bytes)
-);
-
-
-TRACE_EVENT(msm_gem_evict,
- TP_PROTO(u32 bytes),
- TP_ARGS(bytes),
- TP_STRUCT__entry(
- __field(u32, bytes)
- ),
- TP_fast_assign(
- __entry->bytes = bytes;
- ),
- TP_printk("Evicting %u bytes", __entry->bytes)
+ TP_printk("nr_to_scan=%u pages, purged=%u pages, evicted=%u
pages",
+ __entry->nr_to_scan,
+ __entry->purged,
+ __entry->evicted)
);
--
2.36.1
[PATCH v2 09/13] drm/gem: Add LRU/shrinker helper
From: Rob Clark
Add a simple LRU helper to assist with driver's shrinker implementation.
It handles tracking the number of backing pages associated with a given
LRU, and provides a helper to implement shrinker_scan.
A driver can use multiple LRU instances to track objects in various
states, for example a dontneed LRU for purgeable objects, a willneed LRU
for evictable objects, and an unpinned LRU for objects without backing
pages.
All LRUs that the object can be moved between must share a single lock.
Cc: Daniel Vetter
Cc: Thomas Zimmermann
Cc: Dmitry Osipenko
Signed-off-by: Rob Clark
---
drivers/gpu/drm/drm_gem.c | 183 ++
include/drm/drm_gem.h | 56
2 files changed, 239 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index eb0c2d041f13..684db28cc71c 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -165,6 +165,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
obj->resv = &obj->_resv;
drm_vma_node_reset(&obj->vma_node);
+ INIT_LIST_HEAD(&obj->lru_node);
}
EXPORT_SYMBOL(drm_gem_private_object_init);
@@ -951,6 +952,7 @@ drm_gem_object_release(struct drm_gem_object *obj)
dma_resv_fini(&obj->_resv);
drm_gem_free_mmap_offset(obj);
+ drm_gem_lru_remove(obj);
}
EXPORT_SYMBOL(drm_gem_object_release);
@@ -1274,3 +1276,184 @@ drm_gem_unlock_reservations(struct drm_gem_object
**objs, int count,
ww_acquire_fini(acquire_ctx);
}
EXPORT_SYMBOL(drm_gem_unlock_reservations);
+
+/**
+ * drm_gem_lru_init - initialize a LRU
+ *
+ * @lru: The LRU to initialize
+ * @lock: The lock protecting the LRU
+ */
+void
+drm_gem_lru_init(struct drm_gem_lru *lru, struct mutex *lock)
+{
+ lru->lock = lock;
+ lru->count = 0;
+ INIT_LIST_HEAD(&lru->list);
+}
+EXPORT_SYMBOL(drm_gem_lru_init);
+
+static void
+lru_remove(struct drm_gem_object *obj)
+{
+ obj->lru->count -= obj->size >> PAGE_SHIFT;
+ WARN_ON(obj->lru->count < 0);
+ list_del(&obj->lru_node);
+ obj->lru = NULL;
+}
+
+/**
+ * drm_gem_lru_remove - remove object from whatever LRU it is in
+ *
+ * If the object is currently in any LRU, remove it.
+ *
+ * @obj: The GEM object to remove from current LRU
+ */
+void
+drm_gem_lru_remove(struct drm_gem_object *obj)
+{
+ struct drm_gem_lru *lru = obj->lru;
+
+ if (!lru)
+ return;
+
+ mutex_lock(lru->lock);
+ lru_remove(obj);
+ mutex_unlock(lru->lock);
+}
+EXPORT_SYMBOL(drm_gem_lru_remove);
+
+/**
+ * drm_gem_lru_move_tail - move the object to the tail of the LRU
+ *
+ * If the object is already in this LRU it will be moved to the
+ * tail. Otherwise it will be removed from whichever other LRU
+ * it is in (if any) and moved into this LRU.
+ *
+ * @lru: The LRU to move the object into.
+ * @obj: The GEM object to move into this LRU
+ */
+void
+drm_gem_lru_move_tail(struct drm_gem_lru *lru, struct drm_gem_object *obj)
+{
+ mutex_lock(lru->lock);
+ drm_gem_lru_move_tail_locked(lru, obj);
+ mutex_unlock(lru->lock);
+}
+EXPORT_SYMBOL(drm_gem_lru_move_tail);
+
+/**
+ * drm_gem_lru_move_tail_locked - move the object to the tail of the LRU
+ *
+ * If the object is already in this LRU it will be moved to the
+ * tail. Otherwise it will be removed from whichever other LRU
+ * it is in (if any) and moved into this LRU.
+ *
+ * Call with LRU lock held.
+ *
+ * @lru: The LRU to move the object into.
+ * @obj: The GEM object to move into this LRU
+ */
+void
+drm_gem_lru_move_tail_locked(struct drm_gem_lru *lru, struct drm_gem_object
*obj)
+{
+ WARN_ON(!mutex_is_locked(lru->lock));
+
+ if (obj->lru)
+ lru_remove(obj);
+
+ lru->count += obj->size >> PAGE_SHIFT;
+ list_add_tail(&obj->lru_node, &lru->list);
+ obj->lru = lru;
+}
+EXPORT_SYMBOL(drm_gem_lru_move_tail_locked);
+
+/**
+ * drm_gem_lru_scan - helper to implement shrinker.scan_objects
+ *
+ * If the shrink callback succeeds, it is expected that the driver
+ * move the object out of this LRU.
+ *
+ * If the LRU possibly contain active buffers, it is the responsibility
+ * of the shrink callback to check for this (ie. dma_resv_test_signaled())
+ * or if necessary block until the buffer becomes idle.
+ *
+ * @lru: The LRU to scan
+ * @nr_to_scan: The number of pages to try to reclaim
+ * @shrink: Callback to try to shrink/reclaim the object.
+ */
+unsigned long
+drm_gem_lru_scan(struct drm_gem_lru *lru, unsigned nr_to_scan,
+bool (*shrink)(struct drm_gem_object *obj))
+{
+ struct drm_gem_lru still_in_lru;
+ struct drm_gem_object *obj;
+ unsigned freed = 0;
+
+ drm_gem_lru_init(&still_in_lru, lru->lock);
+
+ mutex_lock(lru->lock);
+
+ while (freed < nr_to_scan) {
+ obj = list_first_entry_or_null(&lru->list, typeof(*obj),
lru_node);
+
+ if (!obj)
+ br
[PATCH v2 13/13] drm/msm/gem: Evict active GEM objects when necessary
From: Rob Clark
If we are under enough memory pressure, we should stall waiting for
active buffers to become idle in order to evict.
v2: Check for __GFP_ATOMIC before blocking
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem_shrinker.c | 70 +-
drivers/gpu/drm/msm/msm_gpu_trace.h| 16 +++---
2 files changed, 68 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c
b/drivers/gpu/drm/msm/msm_gem_shrinker.c
index 5cc05d669a08..f31054d25314 100644
--- a/drivers/gpu/drm/msm/msm_gem_shrinker.c
+++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c
@@ -24,6 +24,13 @@ static bool can_swap(void)
return enable_eviction && get_nr_swap_pages() > 0;
}
+static bool can_block(struct shrink_control *sc)
+{
+ if (sc->gfp_mask & __GFP_ATOMIC)
+ return false;
+ return current_is_kswapd() || (sc->gfp_mask & __GFP_RECLAIM);
+}
+
static unsigned long
msm_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
{
@@ -65,26 +72,65 @@ evict(struct drm_gem_object *obj)
return true;
}
+static bool
+wait_for_idle(struct drm_gem_object *obj)
+{
+ enum dma_resv_usage usage = dma_resv_usage_rw(true);
+ return dma_resv_wait_timeout(obj->resv, usage, false, 1000) > 0;
+}
+
+static bool
+active_purge(struct drm_gem_object *obj)
+{
+ if (!wait_for_idle(obj))
+ return false;
+
+ return purge(obj);
+}
+
+static bool
+active_evict(struct drm_gem_object *obj)
+{
+ if (!wait_for_idle(obj))
+ return false;
+
+ return evict(obj);
+}
+
static unsigned long
msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
{
struct msm_drm_private *priv =
container_of(shrinker, struct msm_drm_private, shrinker);
+ struct {
+ struct drm_gem_lru *lru;
+ bool (*shrink)(struct drm_gem_object *obj);
+ bool cond;
+ unsigned long freed;
+ } stages[] = {
+ /* Stages of progressively more aggressive/expensive reclaim: */
+ { &priv->lru.dontneed, purge,true },
+ { &priv->lru.willneed, evict,can_swap() },
+ { &priv->lru.dontneed, active_purge, can_block(sc) },
+ { &priv->lru.willneed, active_evict, can_swap() &&
can_block(sc) },
+ };
long nr = sc->nr_to_scan;
- unsigned long freed, purged, evicted = 0;
-
- purged = drm_gem_lru_scan(&priv->lru.dontneed, nr, purge);
- nr -= purged;
-
- if (can_swap() && nr > 0) {
- evicted = drm_gem_lru_scan(&priv->lru.willneed, nr, evict);
- nr -= evicted;
+ unsigned long freed = 0;
+
+ for (unsigned i = 0; (nr > 0) && (i < ARRAY_SIZE(stages)); i++) {
+ if (!stages[i].cond)
+ continue;
+ stages[i].freed =
+ drm_gem_lru_scan(stages[i].lru, nr, stages[i].shrink);
+ nr -= stages[i].freed;
+ freed += stages[i].freed;
}
- freed = purged + evicted;
-
- if (freed)
- trace_msm_gem_shrink(sc->nr_to_scan, purged, evicted);
+ if (freed) {
+ trace_msm_gem_shrink(sc->nr_to_scan, stages[0].freed,
+stages[1].freed, stages[2].freed,
+stages[3].freed);
+ }
return (freed > 0) ? freed : SHRINK_STOP;
}
diff --git a/drivers/gpu/drm/msm/msm_gpu_trace.h
b/drivers/gpu/drm/msm/msm_gpu_trace.h
index 8867fa0a0306..ac40d857bc45 100644
--- a/drivers/gpu/drm/msm/msm_gpu_trace.h
+++ b/drivers/gpu/drm/msm/msm_gpu_trace.h
@@ -116,22 +116,26 @@ TRACE_EVENT(msm_gmu_freq_change,
TRACE_EVENT(msm_gem_shrink,
- TP_PROTO(u32 nr_to_scan, u32 purged, u32 evicted),
- TP_ARGS(nr_to_scan, purged, evicted),
+ TP_PROTO(u32 nr_to_scan, u32 purged, u32 evicted,
+u32 active_purged, u32 active_evicted),
+ TP_ARGS(nr_to_scan, purged, evicted, active_purged,
active_evicted),
TP_STRUCT__entry(
__field(u32, nr_to_scan)
__field(u32, purged)
__field(u32, evicted)
+ __field(u32, active_purged)
+ __field(u32, active_evicted)
),
TP_fast_assign(
__entry->nr_to_scan = nr_to_scan;
__entry->purged = purged;
__entry->evicted = evicted;
+ __entry->active_purged = active_purged;
+ __entry->active_evicted = active_evicted;
),
- TP_printk("nr_to_scan=%u pages, purged=%u pages, evicted=%u
pages",
- __entry->nr_to_scan,
- __entry->purged,
-
[PATCH v2 07/13] drm/msm/gem: Consolidate pin/unpin paths
From: Rob Clark
Avoid having multiple spots where we increment/decrement pin_count (and
associated LRU updating)
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 3da64c7f65a2..407b18a24dc4 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -190,7 +190,7 @@ static struct page **msm_gem_pin_pages_locked(struct
drm_gem_object *obj)
p = get_pages(obj);
if (!IS_ERR(p)) {
- msm_obj->pin_count++;
+ to_msm_bo(obj)->pin_count++;
update_lru(obj);
}
@@ -213,9 +213,7 @@ void msm_gem_unpin_pages(struct drm_gem_object *obj)
struct msm_gem_object *msm_obj = to_msm_bo(obj);
msm_gem_lock(obj);
- msm_obj->pin_count--;
- GEM_WARN_ON(msm_obj->pin_count < 0);
- update_lru(obj);
+ msm_gem_unpin_locked(obj);
msm_gem_unlock(obj);
}
@@ -436,14 +434,13 @@ int msm_gem_pin_vma_locked(struct drm_gem_object *obj,
struct msm_gem_vma *vma)
if (GEM_WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED))
return -EBUSY;
- pages = get_pages(obj);
+ pages = msm_gem_pin_pages_locked(obj);
if (IS_ERR(pages))
return PTR_ERR(pages);
ret = msm_gem_map_vma(vma->aspace, vma, prot, msm_obj->sgt, obj->size);
-
- if (!ret)
- msm_obj->pin_count++;
+ if (ret)
+ msm_gem_unpin_locked(obj);
return ret;
}
--
2.36.1
[PATCH v2 10/13] drm/msm/gem: Convert to using drm_gem_lru
From: Rob Clark
This converts over to use the shared GEM LRU/shrinker helpers. Note
that it means we are no longer tracking purgeable or willneed buffers
that are active separately. But the most recently pinned buffers should
be at the tail of the various LRUs, and the shrinker is already prepared
to encounter objects which are still active.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_drv.c | 14 +--
drivers/gpu/drm/msm/msm_drv.h | 70 +++
drivers/gpu/drm/msm/msm_gem.c | 58
drivers/gpu/drm/msm/msm_gem.h | 93
drivers/gpu/drm/msm/msm_gem_shrinker.c | 117 ++---
drivers/gpu/drm/msm/msm_gpu.c | 3 -
drivers/gpu/drm/msm/msm_gpu.h | 6 --
7 files changed, 104 insertions(+), 257 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index d7ca025457b6..1ca4a92ba96e 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -418,14 +418,18 @@ static int msm_drm_init(struct device *dev, const struct
drm_driver *drv)
INIT_LIST_HEAD(&priv->objects);
mutex_init(&priv->obj_lock);
- INIT_LIST_HEAD(&priv->inactive_willneed);
- INIT_LIST_HEAD(&priv->inactive_dontneed);
- INIT_LIST_HEAD(&priv->inactive_unpinned);
- mutex_init(&priv->mm_lock);
+ /*
+* Initialize the LRUs:
+*/
+ mutex_init(&priv->lru.lock);
+ drm_gem_lru_init(&priv->lru.unbacked, &priv->lru.lock);
+ drm_gem_lru_init(&priv->lru.pinned, &priv->lru.lock);
+ drm_gem_lru_init(&priv->lru.willneed, &priv->lru.lock);
+ drm_gem_lru_init(&priv->lru.dontneed, &priv->lru.lock);
/* Teach lockdep about lock ordering wrt. shrinker: */
fs_reclaim_acquire(GFP_KERNEL);
- might_lock(&priv->mm_lock);
+ might_lock(&priv->lru.lock);
fs_reclaim_release(GFP_KERNEL);
drm_mode_config_init(ddev);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index b3689a2d27d7..208ae5bc5e6b 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -142,28 +142,60 @@ struct msm_drm_private {
struct mutex obj_lock;
/**
-* LRUs of inactive GEM objects. Every bo is either in one of the
-* inactive lists (depending on whether or not it is shrinkable) or
-* gpu->active_list (for the gpu it is active on[1]), or transiently
-* on a temporary list as the shrinker is running.
+* lru:
*
-* Note that inactive_willneed also contains pinned and vmap'd bos,
-* but the number of pinned-but-not-active objects is small (scanout
-* buffers, ringbuffer, etc).
+* The various LRU's that a GEM object is in at various stages of
+* it's lifetime. Objects start out in the unbacked LRU. When
+* pinned (for scannout or permanently mapped GPU buffers, like
+* ringbuffer, memptr, fw, etc) it moves to the pinned LRU. When
+* unpinned, it moves into willneed or dontneed LRU depending on
+* madvise state. When backing pages are evicted (willneed) or
+* purged (dontneed) it moves back into the unbacked LRU.
*
-* These lists are protected by mm_lock (which should be acquired
-* before per GEM object lock). One should *not* hold mm_lock in
-* get_pages()/vmap()/etc paths, as they can trigger the shrinker.
-*
-* [1] if someone ever added support for the old 2d cores, there could
be
-* more than one gpu object
+* The dontneed LRU is considered by the shrinker for objects
+* that are candidate for purging, and the willneed LRU is
+* considered for objects that could be evicted.
*/
- struct list_head inactive_willneed; /* inactive + potentially
unpin/evictable */
- struct list_head inactive_dontneed; /* inactive + shrinkable */
- struct list_head inactive_unpinned; /* inactive + purged or unpinned */
- long shrinkable_count; /* write access under mm_lock */
- long evictable_count;/* write access under mm_lock */
- struct mutex mm_lock;
+ struct {
+ /**
+* unbacked:
+*
+* The LRU for GEM objects without backing pages allocated.
+* This mostly exists so that objects are always is one
+* LRU.
+*/
+ struct drm_gem_lru unbacked;
+
+ /**
+* pinned:
+*
+* The LRU for pinned GEM objects
+*/
+ struct drm_gem_lru pinned;
+
+ /**
+* willneed:
+*
+* The LRU for unpinned GEM objects which are in madvise
+* WILLNEED state (ie. can be evicted)
+
[PATCH v2 11/13] drm/msm/gem: Unpin buffers earlier
From: Rob Clark
We've already attached the fences, so obj->resv (which shrinker checks)
tells us whether they are still active. So we can unpin sooner, before
we drop the queue lock.
This also avoids the need to grab the obj lock in the retire path,
avoiding potential for lock contention between submit and retire.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem_submit.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index adf358fb8e9d..5599d93ec0d2 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -501,11 +501,11 @@ static int submit_reloc(struct msm_gem_submit *submit,
struct msm_gem_object *ob
*/
static void submit_cleanup(struct msm_gem_submit *submit, bool error)
{
- unsigned cleanup_flags = BO_LOCKED;
+ unsigned cleanup_flags = BO_LOCKED | BO_OBJ_PINNED;
unsigned i;
if (error)
- cleanup_flags |= BO_VMA_PINNED | BO_OBJ_PINNED;
+ cleanup_flags |= BO_VMA_PINNED;
for (i = 0; i < submit->nr_bos; i++) {
struct msm_gem_object *msm_obj = submit->bos[i].obj;
@@ -522,10 +522,6 @@ void msm_submit_retire(struct msm_gem_submit *submit)
for (i = 0; i < submit->nr_bos; i++) {
struct drm_gem_object *obj = &submit->bos[i].obj->base;
- msm_gem_lock(obj);
- /* Note, VMA already fence-unpinned before submit: */
- submit_cleanup_bo(submit, i, BO_OBJ_PINNED);
- msm_gem_unlock(obj);
drm_gem_object_put(obj);
}
}
--
2.36.1
[PATCH v2 06/13] drm/msm/gem: Rename to pin/unpin_pages
From: Rob Clark
Since that is what these fxns actually do.. they are getting *pinned*
pages (as opposed to cases where we need pages, but don't need them
pinned, like CPU mappings).
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem.c | 18 +-
drivers/gpu/drm/msm/msm_gem.h | 4 ++--
drivers/gpu/drm/msm/msm_gem_prime.c | 4 ++--
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 97467364dc0a..3da64c7f65a2 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -177,30 +177,38 @@ static void put_pages(struct drm_gem_object *obj)
}
}
-struct page **msm_gem_get_pages(struct drm_gem_object *obj)
+static struct page **msm_gem_pin_pages_locked(struct drm_gem_object *obj)
{
struct msm_gem_object *msm_obj = to_msm_bo(obj);
struct page **p;
- msm_gem_lock(obj);
+ GEM_WARN_ON(!msm_gem_is_locked(obj));
if (GEM_WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED)) {
- msm_gem_unlock(obj);
return ERR_PTR(-EBUSY);
}
p = get_pages(obj);
-
if (!IS_ERR(p)) {
msm_obj->pin_count++;
update_lru(obj);
}
+ return p;
+}
+
+struct page **msm_gem_pin_pages(struct drm_gem_object *obj)
+{
+ struct page **p;
+
+ msm_gem_lock(obj);
+ p = msm_gem_pin_pages_locked(obj);
msm_gem_unlock(obj);
+
return p;
}
-void msm_gem_put_pages(struct drm_gem_object *obj)
+void msm_gem_unpin_pages(struct drm_gem_object *obj)
{
struct msm_gem_object *msm_obj = to_msm_bo(obj);
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 0ab0dc4f8c25..6fe521ccda45 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -159,8 +159,8 @@ int msm_gem_get_and_pin_iova(struct drm_gem_object *obj,
struct msm_gem_address_space *aspace, uint64_t *iova);
void msm_gem_unpin_iova(struct drm_gem_object *obj,
struct msm_gem_address_space *aspace);
-struct page **msm_gem_get_pages(struct drm_gem_object *obj);
-void msm_gem_put_pages(struct drm_gem_object *obj);
+struct page **msm_gem_pin_pages(struct drm_gem_object *obj);
+void msm_gem_unpin_pages(struct drm_gem_object *obj);
int msm_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
struct drm_mode_create_dumb *args);
int msm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
diff --git a/drivers/gpu/drm/msm/msm_gem_prime.c
b/drivers/gpu/drm/msm/msm_gem_prime.c
index dcc8a573bc76..c1d91863df05 100644
--- a/drivers/gpu/drm/msm/msm_gem_prime.c
+++ b/drivers/gpu/drm/msm/msm_gem_prime.c
@@ -63,12 +63,12 @@ struct drm_gem_object *msm_gem_prime_import_sg_table(struct
drm_device *dev,
int msm_gem_prime_pin(struct drm_gem_object *obj)
{
if (!obj->import_attach)
- msm_gem_get_pages(obj);
+ msm_gem_pin_pages(obj);
return 0;
}
void msm_gem_prime_unpin(struct drm_gem_object *obj)
{
if (!obj->import_attach)
- msm_gem_put_pages(obj);
+ msm_gem_unpin_pages(obj);
}
--
2.36.1
[PATCH v2 05/13] drm/msm/gem: Rename update_inactive
From: Rob Clark
Really what this is doing is updating various LRU lists.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index b55d252aef17..97467364dc0a 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -19,7 +19,7 @@
#include "msm_gpu.h"
#include "msm_mmu.h"
-static void update_inactive(struct msm_gem_object *msm_obj);
+static void update_lru(struct drm_gem_object *obj);
static dma_addr_t physaddr(struct drm_gem_object *obj)
{
@@ -132,7 +132,7 @@ static struct page **get_pages(struct drm_gem_object *obj)
if (msm_obj->flags & MSM_BO_WC)
sync_for_device(msm_obj);
- update_inactive(msm_obj);
+ update_lru(obj);
}
return msm_obj->pages;
@@ -193,7 +193,7 @@ struct page **msm_gem_get_pages(struct drm_gem_object *obj)
if (!IS_ERR(p)) {
msm_obj->pin_count++;
- update_inactive(msm_obj);
+ update_lru(obj);
}
msm_gem_unlock(obj);
@@ -207,7 +207,7 @@ void msm_gem_put_pages(struct drm_gem_object *obj)
msm_gem_lock(obj);
msm_obj->pin_count--;
GEM_WARN_ON(msm_obj->pin_count < 0);
- update_inactive(msm_obj);
+ update_lru(obj);
msm_gem_unlock(obj);
}
@@ -449,7 +449,7 @@ void msm_gem_unpin_locked(struct drm_gem_object *obj)
msm_obj->pin_count--;
GEM_WARN_ON(msm_obj->pin_count < 0);
- update_inactive(msm_obj);
+ update_lru(obj);
}
struct msm_gem_vma *msm_gem_get_vma_locked(struct drm_gem_object *obj,
@@ -658,7 +658,7 @@ static void *get_vaddr(struct drm_gem_object *obj, unsigned
madv)
goto fail;
}
- update_inactive(msm_obj);
+ update_lru(obj);
}
return msm_obj->vaddr;
@@ -730,7 +730,7 @@ int msm_gem_madvise(struct drm_gem_object *obj, unsigned
madv)
* between inactive lists
*/
if (msm_obj->active_count == 0)
- update_inactive(msm_obj);
+ update_lru(obj);
msm_gem_unlock(obj);
@@ -757,7 +757,7 @@ void msm_gem_purge(struct drm_gem_object *obj)
put_iova_vmas(obj);
msm_obj->madv = __MSM_MADV_PURGED;
- update_inactive(msm_obj);
+ update_lru(obj);
drm_gem_free_mmap_offset(obj);
@@ -792,7 +792,7 @@ void msm_gem_evict(struct drm_gem_object *obj)
put_pages(obj);
- update_inactive(msm_obj);
+ update_lru(obj);
}
void msm_gem_vunmap(struct drm_gem_object *obj)
@@ -835,13 +835,14 @@ void msm_gem_active_put(struct drm_gem_object *obj)
GEM_WARN_ON(!msm_gem_is_locked(obj));
if (--msm_obj->active_count == 0) {
- update_inactive(msm_obj);
+ update_lru(obj);
}
}
-static void update_inactive(struct msm_gem_object *msm_obj)
+static void update_lru(struct drm_gem_object *obj)
{
- struct msm_drm_private *priv = msm_obj->base.dev->dev_private;
+ struct msm_drm_private *priv = obj->dev->dev_private;
+ struct msm_gem_object *msm_obj = to_msm_bo(obj);
GEM_WARN_ON(!msm_gem_is_locked(&msm_obj->base));
--
2.36.1
[PATCH v2 04/13] drm/msm/gem: Check for active in shrinker path
From: Rob Clark
Currently in our shrinker path we shouldn't be encountering anything
that is active, but this will change in subsequent patches. So check
if there are unsignaled fences.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem.c | 10 ++
drivers/gpu/drm/msm/msm_gem.h | 1 +
drivers/gpu/drm/msm/msm_gem_shrinker.c | 6 ++
3 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 8ddbd2e001d4..b55d252aef17 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -870,6 +870,16 @@ static void update_inactive(struct msm_gem_object *msm_obj)
mutex_unlock(&priv->mm_lock);
}
+bool msm_gem_active(struct drm_gem_object *obj)
+{
+ GEM_WARN_ON(!msm_gem_is_locked(obj));
+
+ if (to_msm_bo(obj)->pin_count)
+ return true;
+
+ return !dma_resv_test_signaled(obj->resv, dma_resv_usage_rw(true));
+}
+
int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t *timeout)
{
bool write = !!(op & MSM_PREP_WRITE);
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 432032ad4aed..0ab0dc4f8c25 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -173,6 +173,7 @@ void msm_gem_put_vaddr(struct drm_gem_object *obj);
int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv);
void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu);
void msm_gem_active_put(struct drm_gem_object *obj);
+bool msm_gem_active(struct drm_gem_object *obj);
int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t
*timeout);
int msm_gem_cpu_fini(struct drm_gem_object *obj);
int msm_gem_new_handle(struct drm_device *dev, struct drm_file *file,
diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c
b/drivers/gpu/drm/msm/msm_gem_shrinker.c
index 6e39d959b9f0..ea8ed74982c1 100644
--- a/drivers/gpu/drm/msm/msm_gem_shrinker.c
+++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c
@@ -43,6 +43,9 @@ purge(struct msm_gem_object *msm_obj)
if (!is_purgeable(msm_obj))
return false;
+ if (msm_gem_active(&msm_obj->base))
+ return false;
+
/*
* This will move the obj out of still_in_list to
* the purged list
@@ -58,6 +61,9 @@ evict(struct msm_gem_object *msm_obj)
if (is_unevictable(msm_obj))
return false;
+ if (msm_gem_active(&msm_obj->base))
+ return false;
+
msm_gem_evict(&msm_obj->base);
return true;
--
2.36.1
[PATCH v2 03/13] drm/msm: Split out idr_lock
From: Rob Clark
Otherwise if we hit reclaim pinning objects in the submit path, we'll be
blocking retire_worker trying to free a submit.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_drv.c | 4 ++--
drivers/gpu/drm/msm/msm_gem_submit.c | 10 --
drivers/gpu/drm/msm/msm_gpu.h | 4 +++-
drivers/gpu/drm/msm/msm_submitqueue.c | 1 +
4 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 1ed4cd09dbf8..d7ca025457b6 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -883,13 +883,13 @@ static int wait_fence(struct msm_gpu_submitqueue *queue,
uint32_t fence_id,
* retired, so if the fence is not found it means there is nothing
* to wait for
*/
- ret = mutex_lock_interruptible(&queue->lock);
+ ret = mutex_lock_interruptible(&queue->idr_lock);
if (ret)
return ret;
fence = idr_find(&queue->fence_idr, fence_id);
if (fence)
fence = dma_fence_get_rcu(fence);
- mutex_unlock(&queue->lock);
+ mutex_unlock(&queue->idr_lock);
if (!fence)
return 0;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index c7819781879c..16c662808522 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -72,9 +72,9 @@ void __msm_gem_submit_destroy(struct kref *kref)
unsigned i;
if (submit->fence_id) {
- mutex_lock(&submit->queue->lock);
+ mutex_lock(&submit->queue->idr_lock);
idr_remove(&submit->queue->fence_idr, submit->fence_id);
- mutex_unlock(&submit->queue->lock);
+ mutex_unlock(&submit->queue->idr_lock);
}
dma_fence_put(submit->user_fence);
@@ -881,6 +881,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
submit->nr_cmds = i;
+ mutex_lock(&queue->idr_lock);
+
/*
* If using userspace provided seqno fence, validate that the id
* is available before arming sched job. Since access to fence_idr
@@ -889,6 +891,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
*/
if ((args->flags & MSM_SUBMIT_FENCE_SN_IN) &&
idr_find(&queue->fence_idr, args->fence)) {
+ mutex_unlock(&queue->idr_lock);
ret = -EINVAL;
goto out;
}
@@ -921,6 +924,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
submit->user_fence, 1,
INT_MAX, GFP_KERNEL);
}
+
+ mutex_unlock(&queue->idr_lock);
+
if (submit->fence_id < 0) {
ret = submit->fence_id;
submit->fence_id = 0;
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 4d935fedd2ac..962d2070bcdf 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -466,7 +466,8 @@ static inline int msm_gpu_convert_priority(struct msm_gpu
*gpu, int prio,
* @node: node in the context's list of submitqueues
* @fence_idr: maps fence-id to dma_fence for userspace visible fence
* seqno, protected by submitqueue lock
- * @lock: submitqueue lock
+ * @idr_lock: for serializing access to fence_idr
+ * @lock: submitqueue lock for serializing submits on a queue
* @ref: reference count
* @entity:the submit job-queue
*/
@@ -479,6 +480,7 @@ struct msm_gpu_submitqueue {
struct msm_file_private *ctx;
struct list_head node;
struct idr fence_idr;
+ struct mutex idr_lock;
struct mutex lock;
struct kref ref;
struct drm_sched_entity *entity;
diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c
b/drivers/gpu/drm/msm/msm_submitqueue.c
index f486a3cd4e55..c6929e205b51 100644
--- a/drivers/gpu/drm/msm/msm_submitqueue.c
+++ b/drivers/gpu/drm/msm/msm_submitqueue.c
@@ -200,6 +200,7 @@ int msm_submitqueue_create(struct drm_device *drm, struct
msm_file_private *ctx,
*id = queue->id;
idr_init(&queue->fence_idr);
+ mutex_init(&queue->idr_lock);
mutex_init(&queue->lock);
list_add_tail(&queue->node, &ctx->submitqueues);
--
2.36.1
[PATCH v2 02/13] drm/msm: Small submit cleanup
From: Rob Clark
Move more initialization into submit_create().
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem_submit.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index b7c61a99d274..c7819781879c 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -26,6 +26,7 @@ static struct msm_gem_submit *submit_create(struct drm_device
*dev,
struct msm_gpu_submitqueue *queue, uint32_t nr_bos,
uint32_t nr_cmds)
{
+ static atomic_t ident = ATOMIC_INIT(0);
struct msm_gem_submit *submit;
uint64_t sz;
int ret;
@@ -52,9 +53,13 @@ static struct msm_gem_submit *submit_create(struct
drm_device *dev,
submit->gpu = gpu;
submit->cmd = (void *)&submit->bos[nr_bos];
submit->queue = queue;
+ submit->pid = get_pid(task_pid(current));
submit->ring = gpu->rb[queue->ring_nr];
submit->fault_dumped = false;
+ /* Get a unique identifier for the submission for logging purposes */
+ submit->ident = atomic_inc_return(&ident) - 1;
+
INIT_LIST_HEAD(&submit->node);
return submit;
@@ -718,7 +723,6 @@ static void msm_process_post_deps(struct
msm_submit_post_dep *post_deps,
int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
struct drm_file *file)
{
- static atomic_t ident = ATOMIC_INIT(0);
struct msm_drm_private *priv = dev->dev_private;
struct drm_msm_gem_submit *args = data;
struct msm_file_private *ctx = file->driver_priv;
@@ -729,10 +733,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
*data,
struct msm_submit_post_dep *post_deps = NULL;
struct drm_syncobj **syncobjs_to_reset = NULL;
int out_fence_fd = -1;
- struct pid *pid = get_pid(task_pid(current));
bool has_ww_ticket = false;
unsigned i;
- int ret, submitid;
+ int ret;
if (!gpu)
return -ENXIO;
@@ -764,12 +767,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
*data,
if (!queue)
return -ENOENT;
- /* Get a unique identifier for the submission for logging purposes */
- submitid = atomic_inc_return(&ident) - 1;
-
ring = gpu->rb[queue->ring_nr];
- trace_msm_gpu_submit(pid_nr(pid), ring->id, submitid,
- args->nr_bos, args->nr_cmds);
if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
@@ -783,13 +781,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
*data,
if (IS_ERR(submit))
return PTR_ERR(submit);
+ trace_msm_gpu_submit(pid_nr(submit->pid), ring->id, submit->ident,
+ args->nr_bos, args->nr_cmds);
+
ret = mutex_lock_interruptible(&queue->lock);
if (ret)
goto out_post_unlock;
- submit->pid = pid;
- submit->ident = submitid;
-
if (args->flags & MSM_SUBMIT_SUDO)
submit->in_rb = true;
--
2.36.1
[PATCH v2 01/13] drm/msm: Reorder lock vs submit alloc
From: Rob Clark
This lets us drop the NORETRY.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem_submit.c | 24 ++--
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index c9e4aeb14f4a..b7c61a99d274 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -36,7 +36,7 @@ static struct msm_gem_submit *submit_create(struct drm_device
*dev,
if (sz > SIZE_MAX)
return ERR_PTR(-ENOMEM);
- submit = kzalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
+ submit = kzalloc(sz, GFP_KERNEL);
if (!submit)
return ERR_PTR(-ENOMEM);
@@ -771,25 +771,21 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
*data,
trace_msm_gpu_submit(pid_nr(pid), ring->id, submitid,
args->nr_bos, args->nr_cmds);
- ret = mutex_lock_interruptible(&queue->lock);
- if (ret)
- goto out_post_unlock;
-
if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
if (out_fence_fd < 0) {
ret = out_fence_fd;
- goto out_unlock;
+ return ret;
}
}
- submit = submit_create(dev, gpu, queue, args->nr_bos,
- args->nr_cmds);
- if (IS_ERR(submit)) {
- ret = PTR_ERR(submit);
- submit = NULL;
- goto out_unlock;
- }
+ submit = submit_create(dev, gpu, queue, args->nr_bos, args->nr_cmds);
+ if (IS_ERR(submit))
+ return PTR_ERR(submit);
+
+ ret = mutex_lock_interruptible(&queue->lock);
+ if (ret)
+ goto out_post_unlock;
submit->pid = pid;
submit->ident = submitid;
@@ -965,9 +961,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
if (ret && (out_fence_fd >= 0))
put_unused_fd(out_fence_fd);
mutex_unlock(&queue->lock);
+out_post_unlock:
if (submit)
msm_gem_submit_put(submit);
-out_post_unlock:
if (!IS_ERR_OR_NULL(post_deps)) {
for (i = 0; i < args->nr_out_syncobjs; ++i) {
kfree(post_deps[i].chain);
--
2.36.1
[PATCH v2 00/13] drm+msm: Shrinker and LRU rework
From: Rob Clark Mostly a resend with a small fix in the last patch (and a couple early patches drop out when rebasing). original description below: This is mostly motivated by getting drm/msm to pass an i-g-t shrinker test that I've been working on. In particular the test creates and cycles between more GEM buffers than what will fit in RAM to force eviction and re-pin. (There are sub-tests that cover this case both single threaded and with many child processes in parallel.) Getting this test to pass necessitated a few improvements: 1. Re-ordering submit path to get rid of __GFP_NORETRY (in the common case, doing this for syncobjs is still TODO) 2. Decoupling locks needed in the retire path from locks that could be held while hitting reclaim in the submit path 3. If necessary, allow stalling on active BOs for reclaim. The latter point is because we pin objects in the synchronous part of the submit path (before queuing on the drm gpu-scheduler), which means in the parallel variant of the i-g-t test, we need to be able to block in the reclaim path until some queued work has completed/retired. In the process of re-working how drm/msm tracks buffer state in it's various LRU lists, I refactored out a drm_gem_lru helper which, in theory, should be usable by other drivers and drm shmem helpers for implementing LRU tracking and shrinker. Rob Clark (13): drm/msm: Reorder lock vs submit alloc drm/msm: Small submit cleanup drm/msm: Split out idr_lock drm/msm/gem: Check for active in shrinker path drm/msm/gem: Rename update_inactive drm/msm/gem: Rename to pin/unpin_pages drm/msm/gem: Consolidate pin/unpin paths drm/msm/gem: Remove active refcnt drm/gem: Add LRU/shrinker helper drm/msm/gem: Convert to using drm_gem_lru drm/msm/gem: Unpin buffers earlier drm/msm/gem: Consolidate shrinker trace drm/msm/gem: Evict active GEM objects when necessary drivers/gpu/drm/drm_gem.c | 183 + drivers/gpu/drm/msm/msm_drv.c | 18 ++- drivers/gpu/drm/msm/msm_drv.h | 70 +++--- drivers/gpu/drm/msm/msm_gem.c | 149 +++- drivers/gpu/drm/msm/msm_gem.h | 112 +-- drivers/gpu/drm/msm/msm_gem_prime.c| 4 +- drivers/gpu/drm/msm/msm_gem_shrinker.c | 164 ++ drivers/gpu/drm/msm/msm_gem_submit.c | 78 --- drivers/gpu/drm/msm/msm_gpu.c | 3 - drivers/gpu/drm/msm/msm_gpu.h | 10 +- drivers/gpu/drm/msm/msm_gpu_trace.h| 36 +++-- drivers/gpu/drm/msm/msm_submitqueue.c | 1 + include/drm/drm_gem.h | 56 13 files changed, 483 insertions(+), 401 deletions(-) -- 2.36.1
Re: [RFC PATCH v3 2/2] drm/bridge: ti-sn65dsi86: support DRM_BRIDGE_ATTACH_NO_CONNECTOR
Hi, On Mon, Jul 11, 2022 at 10:23 AM Doug Anderson wrote: > > Hi, > > On Mon, Jul 11, 2022 at 2:21 AM Dmitry Baryshkov > wrote: > > > > Now as the driver does not depend on pdata->connector, add support for > > attaching the bridge with DRM_BRIDGE_ATTACH_NO_CONNECTOR. > > > > Reviewed-by: Sam Ravnborg > > Reviewed-by: Laurent Pinchart > > Signed-off-by: Dmitry Baryshkov > > --- > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 18 -- > > 1 file changed, 8 insertions(+), 10 deletions(-) > > This has been on my list of annoyances for quite some time now. Most > excellent to have it fixed, thanks! > > Reviewed-by: Douglas Anderson > > Tested together with patch #1. > > Tested-by: Douglas Anderson > > > Unless someone yells that there's a problem or someone beats me to it, > I'll plan to land in drm-misc-next sometime next week. Landed on drm-misc-next: 6e2dc7ac7141 drm/bridge: ti-sn65dsi86: support DRM_BRIDGE_ATTACH_NO_CONNECTOR
Re: [Intel-gfx] [PATCH] drm/i915/guc: support v69 in parallel to v70
On 7/19/2022 12:34 AM, Tvrtko Ursulin wrote:
On 18/07/2022 17:41, Ceraolo Spurio, Daniele wrote:
On 7/18/2022 3:02 AM, Tvrtko Ursulin wrote:
Hi,
On 15/07/2022 23:54, Daniele Ceraolo Spurio wrote:
This patch re-introduces support for GuC v69 in parallel to v70. As
this
is a quick fix, v69 has been re-introduced as the single "fallback"
guc
version in case v70 is not available on disk. All v69 specific code
has
been labeled as such for easy identification, and the same was done
for
all v70 functions for which there is a separate v69 version, to avoid
accidentally calling the wrong version via the unlabeled name.
When the fallback mode kicks in, a drm_warn message is printed in
dmesg
to warn the user of the required update.
The plan is to follow this up with a more complex rework to allow for
multiple different GuC versions to be supported at the same time.
Fixes: 2584b3549f4c ("drm/i915/guc: Update to GuC version 70.1.1")
Please check if I got this right:
* ADL-P was out of "force probe" starting from 5.17.
* GuC fw got bumped from v62 to v69 in 5.18.
Does this mean you would also need to handle v62 to avoid regressing
ADL-P from 5.17 to 5.18? I couldn't figure out when ADL-P switched
from execlists to GuC due a bit convoluted supported/wanted/needed
macros etc, so not entirely sure.
I haven't checked about previous GuC versions because the report from
Dave was on the 69->70 transition and about re-introducing v69
support, so I just focused on that. Let me dig on the versions and on
what would be needed to support all 3 revs (if it is required).
Secondly, my concern with the approach like in this patch is that it
would grow the i915 code base *if* there is no incentive to keep the
compatiblity breaking firware updates in check.
The grow of the i915 code is inevitable. Even without changes to
existing platforms, new features for new platforms will require new
GuC interfaces. Sometimes the GuC team also refactors an existing
interface so that it can include a new aspect of an existing feature.
We'll have to discuss with them how to minimize breakages in such
scenarios.
To think about in tandem with this is the question of whether many
more fallback versions need to be handled, even for platforms which
only use GuC to load HuC? Those would also regress in the media
encoding side of things, even if they don't use GuC submission, right?
The only HuC-only platform is ADL-S and that went out of force probe
when we were on GuC 62, so definitely nothing older than that will be
needed.
I was referring to platforms where HuC is used for some encoding
types. List on
https://github.com/intel/media-driver/blob/master/docs/media_features.md#media-features-summary.
It is not entirely clear to me from that list - you are saying the HuC
is actually used only on ADL-S? I was going by the existence of HuC
firmware files only so might be wrong just as well.
Like GuC, HuC can be enabled via modparam on anything gen11+, but it is
only enabled by default on a subset of platforms, which are all the
platforms for which we enable GuC submission, plus ADL-S. Of those, the
only ones out of force probe are the ADL variants and their derivatives,
so they're the only ones we need to guarantee backwards compatibility for.
See uc_expand_default_options() in intel_uc.c for further details.
Daniele
Regards,
Tvrtko
If this is so, the approach from this patch would feel rushed in my
view.
It totally is, no argument there. As mentioned in the commit message,
the plan is to replace the whole thing with a more flexible and
cleaner mechanism, but we do need something for the upcoming 5.19
release so there is no time to do this properly from the get-go.
There is also the question of being able to automatically load the
latest _compatible_ (same major version) GuC fw found on disk. Aka
allowing a bugfix firmware update which does not require a kernel
update. In theory should be possible but we don't have that
implemented either, right?
We do not. Something like this was actually shot down when GuC first
came around. We used to have simlinks for the GuC binary to be able
to drop in a replacement like you said, but there were concerns about
how to validate all the possible kernel:fw combinations this could
cause, hence why in the end we went with the exact match model. Note
that at the time we didn't have a patch number for bugfix tracking in
GuC, so the decision made more sense back then than it does now.
We've already restarted the discussion internally.
Daniele
Regards,
Tvrtko
Link:
https://lists.freedesktop.org/archives/intel-gfx/2022-July/301640.html
Signed-off-by: Daniele Ceraolo Spurio
Cc: John Harrison
Cc: Matthew Brost
Cc: Matt Roper
Cc: Dave Airlie
---
drivers/gpu/drm/i915/gt/intel_context_types.h | 11 +-
.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 3 +
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 5 +
drive
Re: [PATCH v2] drm/bridge: it6505: Add i2c api power on check
On Wed, 13 Jul 2022 at 05:16, allen wrote:
>
> From: allen chen
>
> Use i2c bus to read/write when it6505 power off will occur i2c error.
> Add this check will prevent i2c error when it6505 power off.
>
> Signed-off-by: Pin-Yen Lin
> Signed-off-by: Allen Chen
> Reviewed-by: Robert Foss
> ---
> drivers/gpu/drm/bridge/ite-it6505.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
> b/drivers/gpu/drm/bridge/ite-it6505.c
> index aa5e0aa1af85..cfd2c3275dc5 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -518,6 +518,9 @@ static int it6505_read(struct it6505 *it6505, unsigned
> int reg_addr)
> int err;
> struct device *dev = &it6505->client->dev;
>
> + if (!it6505->powered)
> + return -ENODEV;
> +
> err = regmap_read(it6505->regmap, reg_addr, &value);
> if (err < 0) {
> dev_err(dev, "read failed reg[0x%x] err: %d", reg_addr, err);
> @@ -533,6 +536,9 @@ static int it6505_write(struct it6505 *it6505, unsigned
> int reg_addr,
> int err;
> struct device *dev = &it6505->client->dev;
>
> + if (!it6505->powered)
> + return -ENODEV;
> +
> err = regmap_write(it6505->regmap, reg_addr, reg_val);
>
> if (err < 0) {
> @@ -550,6 +556,9 @@ static int it6505_set_bits(struct it6505 *it6505,
> unsigned int reg,
> int err;
> struct device *dev = &it6505->client->dev;
>
> + if (!it6505->powered)
> + return -ENODEV;
> +
> err = regmap_update_bits(it6505->regmap, reg, mask, value);
> if (err < 0) {
> dev_err(dev, "write reg[0x%x] = 0x%x mask = 0x%x failed err
> %d",
> @@ -2553,13 +2562,12 @@ static int it6505_poweron(struct it6505 *it6505)
> usleep_range(1, 2);
> }
>
> + it6505->powered = true;
> it6505_reset_logic(it6505);
> it6505_int_mask_enable(it6505);
> it6505_init(it6505);
> it6505_lane_off(it6505);
>
> - it6505->powered = true;
> -
> return 0;
> }
>
> --
> 2.25.1
>
This patch no longer applies to the drm-misc-next tree, could you
rebase it and send out a v3?
Re: [PATCH 3/3] drm/gud: Fix endianness in gud_xrgb8888_to_color() helper
Den 08.07.2022 20.21, skrev Geert Uytterhoeven:
> DRM formats are defined to be little-endian, unless the
> DRM_FORMAT_BIG_ENDIAN flag is set. Hence when converting from one
> format to another, multi-byte pixel values loaded from memory must be
> converted from little-endian to host-endian. Conversely, multi-byte
> pixel values written to memory must be converted from host-endian to
> little-endian. Currently only drm_fb_xrgb_to_rgb332_line() includes
> endianness handling.
>
> Fix gud_xrgb_to_color() on big-endian platforms by adding the
> missing endianness handling.
>
> Signed-off-by: Geert Uytterhoeven
> ---
> Compile-tested only.
>
> Interestingly, drm_fb_xrgb_to_rgb332() was introduced for GUD,
> and always had correct endiannes handling...
RGB332 support was added later and by that time I had understood that
the framebuffer was little endian and not host endian as I first assumed
(there's a fixme comment in gud_pipe.c that BE is probably broken but I
haven't got any hw to test on so I haven't tried to fix it).
Thanks for fixing this, pathces 2 and 3 tested on drm/gud and applied to
drm-misc-next.
Noralf.
> ---
> drivers/gpu/drm/gud/gud_pipe.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
> index 4873f9799f412e04..d42592f6daab8b2a 100644
> --- a/drivers/gpu/drm/gud/gud_pipe.c
> +++ b/drivers/gpu/drm/gud/gud_pipe.c
> @@ -105,7 +105,8 @@ static size_t gud_xrgb_to_color(u8 *dst, const struct
> drm_format_info *forma
> unsigned int bits_per_pixel = 8 / block_width;
> u8 r, g, b, pix, *block = dst; /* Assign to silence compiler warning */
> unsigned int x, y, width;
> - u32 *pix32;
> + __le32 *sbuf32;
> + u32 pix32;
> size_t len;
>
> /* Start on a byte boundary */
> @@ -114,8 +115,8 @@ static size_t gud_xrgb_to_color(u8 *dst, const struct
> drm_format_info *forma
> len = drm_format_info_min_pitch(format, 0, width) *
> drm_rect_height(rect);
>
> for (y = rect->y1; y < rect->y2; y++) {
> - pix32 = src + (y * fb->pitches[0]);
> - pix32 += rect->x1;
> + sbuf32 = src + (y * fb->pitches[0]);
> + sbuf32 += rect->x1;
>
> for (x = 0; x < width; x++) {
> unsigned int pixpos = x % block_width; /* within byte
> from the left */
> @@ -126,9 +127,10 @@ static size_t gud_xrgb_to_color(u8 *dst, const
> struct drm_format_info *forma
> *block = 0;
> }
>
> - r = *pix32 >> 16;
> - g = *pix32 >> 8;
> - b = *pix32++;
> + pix32 = le32_to_cpu(*sbuf32++);
> + r = pix32 >> 16;
> + g = pix32 >> 8;
> + b = pix32;
>
> switch (format->format) {
> case GUD_DRM_FORMAT_XRGB:
Re: [PATCH V2 0/3] DSI host and peripheral initialisation ordering
Hi Lucas On Fri, 10 Jun 2022 at 08:52, Lucas Stach wrote: > > Hi, > > Am Mittwoch, dem 11.05.2022 um 16:58 +0200 schrieb Marek Szyprowski: > > Hi Dave, > > > > On 05.04.2022 13:43, Dave Stevenson wrote: > > > On Fri, 18 Mar 2022 at 12:25, Dave Stevenson > > > wrote: > > > > On Fri, 4 Mar 2022 at 15:18, Dave Stevenson > > > > wrote: > > > > > Hi All > > > > A gentle ping on this series. Any comments on the approach? > > > > Thanks. > > > I realise the merge window has just closed and therefore folks have > > > been busy, but no responses on this after a month? > > > > > > Do I give up and submit a patch to document that DSI is broken and no one > > > cares? > > > > Thanks for pointing this patchset in the 'drm: bridge: Add Samsung MIPI > > DSIM bridge' thread, otherwise I would miss it since I'm not involved > > much in the DRM development. > > > > This resolves most of the issues in the Exynos DSI and its recent > > conversion to the drm bridge framework. I've added the needed > > prepare_upstream_first flags to the panels and everything works fine > > without the bridge chain order hacks. > > > > Feel free to add: > > > > Tested-by: Marek Szyprowski > > > > > > The only remaining thing to resolve is the moment of enabling DSI host. > > The proper sequence is: > > > > 1. host power on, 2. device power on, 3. host init, 4. device init, 5. > > video enable. > > > > #1 is done in dsi's pre_enable, #2 is done in panel's prepare. #3 was so > > far done in the first host transfer call, which usually happens in > > panel's prepare, then the #4 happens. Then video enable is done in the > > enable callbacks. > > > > Jagan wants to move it to the dsi host pre_enable() to let it work with > > DSI bridges controlled over different interfaces > > (https://lore.kernel.org/all/20220504114021.33265-6-ja...@amarulasolutions.com/ > > ). This however fails on Exynos with DSI panels, because when dsi's > > pre_enable is called, the dsi device is not yet powered. I've discussed > > this with Andrzej Hajda and we came to the conclusion that this can be > > resolved by adding the init() callback to the struct mipi_dsi_host_ops. > > Then DSI client (next bridge or panel) would call it after powering self > > on, but before sending any DSI commands in its pre_enable/prepare functions. > > > > I've prepared a prototype of such solution. This approach finally > > resolved all the initialization issues! The bridge chain finally matches > > the hardware, no hack are needed, and everything is controlled by the > > DRM core. This prototype also includes the Jagan's patches, which add > > IMX support to Samsung DSIM. If one is interested, here is my git repo > > with all the PoC patches: > > > > https://github.com/mszyprow/linux/tree/v5.18-next-20220511-dsi-rework > > While this needs rework on the bridge chip side, I fear that we need > something like that to allow the bridge to control the sequencing of > the DSI host init. While most bridges that aren't controlled via the > DSI channel might be fine with just initializing the host right before > a video signal is driven, there are some that need a different > sequencing. > > The chip I'm currently looking at is a TC368767, where the datasheet > states that the DSI lanes must be in LP-11 before the reset is > released. While the datasheet doesn't specify what happens if that > sequence is violated, Marek Vasut found that the chip enters a test > mode if the lanes are not in LP-11 at that point and I can confirm this > observation. > Now with the TC358767 being a DSI to (e)DP converter, we need to > release the chip from reset pretty early to establish the DP AUX > connection to communicate with the display, in order to find out which > video modes we can drive. As the chip is controlled via i2c in my case, > initializing the DSI host on first DSI command transaction is out and > doing so before the bridge pre_enable is way too late. > > What I would need for this chip to work properly is an explicit call, > like the mipi_dsi_host_init() added in the PoC above, to allow the > bridge driver to kick the DSI host initialization before releasing the > chip from reset state. This is going off on a slight tangent from the original patch set, but a thought has just come to mind for this use case. For this sort of bridge device where you want to power up early (either just for LP-11 state, or for HS on the clock lane), is power up at mipi_dsi_attach, and power down at mipi_dsi_detach sufficient? We have mode_flags in struct mipi_dsi_device passed in mipi_dsi_attach, so an extra flag in there (eg MIPI_DSI_EARLY_POWER_ON) would be very easy and be a simple way to signal an alternate DSI host behaviour. It still leaves the configuration of link frequency as an open question, but potentially solves the sequencing issue. Just a thought. Perhaps dsi_attach() is still too late in the process. Dave
Re: [PATCH v1 11/12] drm/bridge: Drop drm_bridge_funcs.mode_fixup
On Sun, 17 Jul 2022 at 18:58, Sam Ravnborg wrote:
>
> All users are converted over to drm_bridge_funcs.atomic_check()
> so it is safe to drop the mode_fixup support.
>
> Update the comment for atomic_check with relevant parts from mode_fixup.
>
> Signed-off-by: Sam Ravnborg
Reviewed-by: Dave Stevenson
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/drm_bridge.c | 7 +
> include/drm/drm_bridge.h | 60 ++--
> 2 files changed, 17 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index b6f56d8f3547..3f5acb19957c 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -685,10 +685,6 @@ static int drm_atomic_bridge_check(struct drm_bridge
> *bridge,
> crtc_state, conn_state);
> if (ret)
> return ret;
> - } else if (bridge->funcs->mode_fixup) {
> - if (!bridge->funcs->mode_fixup(bridge, &crtc_state->mode,
> - &crtc_state->adjusted_mode))
> - return -EINVAL;
> }
>
> return 0;
> @@ -934,8 +930,7 @@ drm_atomic_bridge_propagate_bus_flags(struct drm_bridge
> *bridge,
> * @conn_state: new connector state
> *
> * First trigger a bus format negotiation before calling
> - * &drm_bridge_funcs.atomic_check() (falls back on
> - * &drm_bridge_funcs.mode_fixup()) op for all the bridges in the encoder
> chain,
> + * &drm_bridge_funcs.atomic_check() op for all the bridges in the encoder
> chain,
> * starting from the last bridge to the first. These are called before
> calling
> * &drm_encoder_helper_funcs.atomic_check()
> *
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 7496f41535b1..8c93369bcc74 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -106,7 +106,7 @@ struct drm_bridge_funcs {
> * to look at anything else but the passed-in mode, and validate it
> * against configuration-invariant hardward constraints. Any further
> * limits which depend upon the configuration can only be checked in
> -* @mode_fixup.
> +* @atomic_check.
> *
> * RETURNS:
> *
> @@ -116,46 +116,6 @@ struct drm_bridge_funcs {
>const struct drm_display_info
> *info,
>const struct drm_display_mode
> *mode);
>
> - /**
> -* @mode_fixup:
> -*
> -* This callback is used to validate and adjust a mode. The parameter
> -* mode is the display mode that should be fed to the next element in
> -* the display chain, either the final &drm_connector or the next
> -* &drm_bridge. The parameter adjusted_mode is the input mode the
> bridge
> -* requires. It can be modified by this callback and does not need to
> -* match mode. See also &drm_crtc_state.adjusted_mode for more
> details.
> -*
> -* This is the only hook that allows a bridge to reject a modeset. If
> -* this function passes all other callbacks must succeed for this
> -* configuration.
> -*
> -* The mode_fixup callback is optional. &drm_bridge_funcs.mode_fixup()
> -* is not called when &drm_bridge_funcs.atomic_check() is implemented,
> -* so only one of them should be provided.
> -*
> -* NOTE:
> -*
> -* This function is called in the check phase of atomic modesets,
> which
> -* can be aborted for any reason (including on userspace's request to
> -* just check whether a configuration would be possible). Drivers MUST
> -* NOT touch any persistent state (hardware or software) or data
> -* structures except the passed in @state parameter.
> -*
> -* Also beware that userspace can request its own custom modes,
> neither
> -* core nor helpers filter modes to the list of probe modes reported
> by
> -* the GETCONNECTOR IOCTL and stored in &drm_connector.modes. To
> ensure
> -* that modes are filtered consistently put any bridge constraints and
> -* limits checks into @mode_valid.
> -*
> -* RETURNS:
> -*
> -* True if an acceptable configuration is possible, false if the
> modeset
> -* operation should be rejected.
> -*/
> - bool (*mode_fixup)(struct drm_bridge *bridge,
> - const struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode);
> /**
> * @disable:
> *
> @@ -466,9 +426,7 @@ struct drm_bridge_funcs {
> * &drm_bridge_funcs.atomic_check() hooks are called in reverse
> *
Re: [PATCH v1 09/12] drm/rcar-du: lvds: Use drm_bridge_funcs.atomic_check
On Sun, 17 Jul 2022 at 18:58, Sam Ravnborg wrote:
>
> Replace the deprecated drm_bridge_funcs.mode_fixup() with
> drm_bridge_funcs.atomic_check().
> The driver implements the state operations, so no other changes
> are required for the replacement.
>
> Signed-off-by: Sam Ravnborg
Reviewed-by: Dave Stevenson
> Cc: Laurent Pinchart
> Cc: Kieran Bingham
> Cc: linux-renesas-...@vger.kernel.org
> ---
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 830aac0a2cb4..c4adbcede090 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -554,10 +554,12 @@ static void rcar_lvds_atomic_disable(struct drm_bridge
> *bridge,
> clk_disable_unprepare(lvds->clocks.mod);
> }
>
> -static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
> -const struct drm_display_mode *mode,
> -struct drm_display_mode *adjusted_mode)
> +static int rcar_lvds_atomic_check(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> {
> + struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> int min_freq;
>
> @@ -569,7 +571,7 @@ static bool rcar_lvds_mode_fixup(struct drm_bridge
> *bridge,
> min_freq = lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL ? 5000 :
> 31000;
> adjusted_mode->clock = clamp(adjusted_mode->clock, min_freq, 148500);
>
> - return true;
> + return 0;
> }
>
> static int rcar_lvds_attach(struct drm_bridge *bridge,
> @@ -591,7 +593,7 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops
> = {
> .atomic_reset = drm_atomic_helper_bridge_reset,
> .atomic_enable = rcar_lvds_atomic_enable,
> .atomic_disable = rcar_lvds_atomic_disable,
> - .mode_fixup = rcar_lvds_mode_fixup,
> + .atomic_check = rcar_lvds_atomic_check,
> };
>
> bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> --
> 2.34.1
>
Re: [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
On Mon, 18 Jul 2022 22:30:50 +0100, Caleb Connolly wrote: > From: Sumit Semwal > > LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel. > > Signed-off-by: Vinod Koul > Signed-off-by: Sumit Semwal > [caleb: convert to yaml] > Signed-off-by: Caleb Connolly > --- > .../bindings/display/panel/lg,43408.yaml | 41 +++ > .../display/panel/panel-simple-dsi.yaml | 2 + > 2 files changed, 43 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/lg,43408.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: ./Documentation/devicetree/bindings/display/panel/lg,43408.yaml: $id: relative path/filename doesn't match actual path or filename expected: http://devicetree.org/schemas/display/panel/lg,43408.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/lg,43408.yaml: duplicate '$id' value 'http://devicetree.org/schemas/display/panel/panel-lvds.yaml#' doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
Re: [PATCH v1 08/12] drm/mediatek: Drop mtk_hdmi_bridge_mode_fixup
On Sun, 17 Jul 2022 at 18:58, Sam Ravnborg wrote:
>
> The implementation of drm_bridge_funcs.mode_fixup is optional
> so there is no need to provide an empty implementation.
> Drop mtk_hdmi_bridge_mode_fixup() so the driver no longer uses the
> deprecated drm_bridge_funcs.mode_fixup() operation.
>
> Signed-off-by: Sam Ravnborg
Reviewed-by: Dave Stevenson
> Cc: Chun-Kuang Hu
> Cc: Philipp Zabel
> Cc: Matthias Brugger
> Cc: linux-media...@lists.infradead.org
> Cc: linux-arm-ker...@lists.infradead.org
> ---
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index a63b76055f81..7321aa1ee6f0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1293,13 +1293,6 @@ static int mtk_hdmi_bridge_attach(struct drm_bridge
> *bridge,
> return 0;
> }
>
> -static bool mtk_hdmi_bridge_mode_fixup(struct drm_bridge *bridge,
> - const struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> -{
> - return true;
> -}
> -
> static void mtk_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
>struct drm_bridge_state
> *old_bridge_state)
> {
> @@ -1399,7 +1392,6 @@ static const struct drm_bridge_funcs
> mtk_hdmi_bridge_funcs = {
> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> .atomic_reset = drm_atomic_helper_bridge_reset,
> .attach = mtk_hdmi_bridge_attach,
> - .mode_fixup = mtk_hdmi_bridge_mode_fixup,
> .atomic_disable = mtk_hdmi_bridge_atomic_disable,
> .atomic_post_disable = mtk_hdmi_bridge_atomic_post_disable,
> .mode_set = mtk_hdmi_bridge_mode_set,
> --
> 2.34.1
>
Re: [PATCH v1 07/12] drm/bridge: tc358767: Use drm_bridge_funcs.atomic_check
On Sun, 17 Jul 2022 at 18:45, Sam Ravnborg wrote:
>
> When atomic_check() is defined, then mode_fixup() is ignored,
> so it had no effect that drm_bridge_funcs.mode_fixup was assigned.
> Embed the original implementation in the caller and drop the function.
>
> Signed-off-by: Sam Ravnborg
Reviewed-by: Dave Stevenson
(There was a point when drm_bridge_chain_mode_fixup still existed, but
that's gone/going now).
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> ---
> drivers/gpu/drm/bridge/tc358767.c | 21 ++---
> 1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c
> b/drivers/gpu/drm/bridge/tc358767.c
> index 02bd757a8987..b2ab967504af 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1496,26 +1496,18 @@ tc_edp_bridge_atomic_disable(struct drm_bridge
> *bridge,
> dev_err(tc->dev, "main link disable error: %d\n", ret);
> }
>
> -static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
> -const struct drm_display_mode *mode,
> -struct drm_display_mode *adj)
> -{
> - /* Fixup sync polarities, both hsync and vsync are active low */
> - adj->flags = mode->flags;
> - adj->flags |= (DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
> - adj->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
> -
> - return true;
> -}
> -
> static int tc_common_atomic_check(struct drm_bridge *bridge,
> struct drm_bridge_state *bridge_state,
> struct drm_crtc_state *crtc_state,
> struct drm_connector_state *conn_state,
> const unsigned int max_khz)
> {
> - tc_bridge_mode_fixup(bridge, &crtc_state->mode,
> -&crtc_state->adjusted_mode);
> + struct drm_display_mode *adj = &crtc_state->adjusted_mode;
> +
> + /* Fixup sync polarities, both hsync and vsync are active low */
> + adj->flags = crtc_state->mode.flags;
> + adj->flags |= (DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
> + adj->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
>
> if (crtc_state->adjusted_mode.clock > max_khz)
> return -EINVAL;
> @@ -1783,7 +1775,6 @@ static const struct drm_bridge_funcs
> tc_edp_bridge_funcs = {
> .atomic_check = tc_edp_atomic_check,
> .atomic_enable = tc_edp_bridge_atomic_enable,
> .atomic_disable = tc_edp_bridge_atomic_disable,
> - .mode_fixup = tc_bridge_mode_fixup,
> .detect = tc_bridge_detect,
> .get_edid = tc_get_edid,
> .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> --
> 2.34.1
>
[PATCH v4 4/7] drm/i915: Check for integer truncation on the configuration of ttm place
There is an impedance mismatch between the first/last valid page
frame number of ttm place in unsigned and our memory/page accounting in
unsigned long.
As the object size is under the control of userspace, we have to be prudent
and catch the conversion errors.
To catch the implicit truncation as we switch from unsigned long to
unsigned, we use overflows_type check and report E2BIG or overflow_type
prior to the operation.
v3: Not to change execution inside a macro. (Mauro)
Add safe_conversion_gem_bug_on() macro and remove temporal
SAFE_CONVERSION() macro.
v4: Fix unhandled GEM_BUG_ON() macro call from safe_conversion_gem_bug_on()
Signed-off-by: Gwan-gyeong Mun
Cc: Chris Wilson
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
Reported-by: kernel test robot
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 6 +++---
drivers/gpu/drm/i915/i915_gem.h | 4
drivers/gpu/drm/i915/intel_region_ttm.c | 20 +---
3 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 9f2be1892b6c..88f2887627dc 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -140,14 +140,14 @@ i915_ttm_place_from_region(const struct
intel_memory_region *mr,
if (flags & I915_BO_ALLOC_CONTIGUOUS)
place->flags |= TTM_PL_FLAG_CONTIGUOUS;
if (offset != I915_BO_INVALID_OFFSET) {
- place->fpfn = offset >> PAGE_SHIFT;
- place->lpfn = place->fpfn + (size >> PAGE_SHIFT);
+ safe_conversion_gem_bug_on(&place->fpfn, offset >> PAGE_SHIFT);
+ safe_conversion_gem_bug_on(&place->lpfn, place->fpfn + (size >>
PAGE_SHIFT));
} else if (mr->io_size && mr->io_size < mr->total) {
if (flags & I915_BO_ALLOC_GPU_ONLY) {
place->flags |= TTM_PL_FLAG_TOPDOWN;
} else {
place->fpfn = 0;
- place->lpfn = mr->io_size >> PAGE_SHIFT;
+ safe_conversion_gem_bug_on(&place->lpfn, mr->io_size >>
PAGE_SHIFT);
}
}
}
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 68d8d52bd541..327dacedd5d1 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -83,5 +83,9 @@ struct drm_i915_private;
#endif
#define I915_GEM_IDLE_TIMEOUT (HZ / 5)
+#define safe_conversion_gem_bug_on(ptr, value) !({ \
+ safe_conversion(ptr, value) ? 0 \
+ : (({ GEM_BUG_ON(overflows_type(value, *ptr)); }), 1); \
+})
#endif /* __I915_GEM_H__ */
diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
b/drivers/gpu/drm/i915/intel_region_ttm.c
index 575d67bc6ffe..f0d143948725 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -209,14 +209,26 @@ intel_region_ttm_resource_alloc(struct
intel_memory_region *mem,
if (flags & I915_BO_ALLOC_CONTIGUOUS)
place.flags |= TTM_PL_FLAG_CONTIGUOUS;
if (offset != I915_BO_INVALID_OFFSET) {
- place.fpfn = offset >> PAGE_SHIFT;
- place.lpfn = place.fpfn + (size >> PAGE_SHIFT);
+ if (!safe_conversion_gem_bug_on(&place.fpfn,
+ offset >> PAGE_SHIFT)) {
+ ret = -E2BIG;
+ goto out;
+ }
+ if (!safe_conversion_gem_bug_on(&place.lpfn,
+ place.fpfn + (size >>
PAGE_SHIFT))) {
+ ret = -E2BIG;
+ goto out;
+ }
} else if (mem->io_size && mem->io_size < mem->total) {
if (flags & I915_BO_ALLOC_GPU_ONLY) {
place.flags |= TTM_PL_FLAG_TOPDOWN;
} else {
place.fpfn = 0;
- place.lpfn = mem->io_size >> PAGE_SHIFT;
+ if (!safe_conversion_gem_bug_on(&place.lpfn,
+ mem->io_size >>
PAGE_SHIFT)) {
+ ret = -E2BIG;
+ goto out;
+ }
}
}
@@ -224,6 +236,8 @@ intel_region_ttm_resource_alloc(struct intel_memory_region
*mem,
mock_bo.bdev = &mem->i915->bdev;
ret = man->func->alloc(man, &mock_bo, &place, &res);
+
+out:
if (ret == -ENOSPC)
ret = -ENXIO;
if (!ret)
--
2.34.1
[PATCH v4 7/7] drm/i915: Remove truncation warning for large objects
From: Chris Wilson
Having addressed the issues surrounding incorrect types for local
variables and potential integer truncation in using the scatterlist API,
we have closed all the loop holes we had previously identified with
dangerously large object creation. As such, we can eliminate the warning
put in place to remind us to complete the review.
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Brian Welty
Cc: Matthew Auld
Cc: Thomas Hellström
Testcase: igt@gem_create@create-massive
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4991
Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
---
drivers/gpu/drm/i915/gem/i915_gem_object.h | 15 ---
1 file changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index d5f823cc1c2e..ae97811e8ff9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -20,25 +20,10 @@
enum intel_region_id;
-/*
- * XXX: There is a prevalence of the assumption that we fit the
- * object's page count inside a 32bit _signed_ variable. Let's document
- * this and catch if we ever need to fix it. In the meantime, if you do
- * spot such a local variable, please consider fixing!
- *
- * We can check for invalidly typed locals with typecheck(), see for example
- * i915_gem_object_get_sg().
- */
-#define GEM_CHECK_SIZE_OVERFLOW(sz) \
- GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
-
static inline bool i915_gem_object_size_2big(u64 size)
{
struct drm_i915_gem_object *obj;
- if (GEM_CHECK_SIZE_OVERFLOW(size))
- return true;
-
if (overflows_type(size, obj->base.size))
return true;
--
2.34.1
[PATCH v4 6/7] drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large
The ttm_bo_init_reserved() functions returns -ENOSPC if the size is too big to add vma. The direct function that returns -ENOSPC is drm_mm_insert_node_in_range(). To handle the same error as other code returning -E2BIG when the size is too large, it converts return value to -E2BIG. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 88f2887627dc..4d478bf325be 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1249,6 +1249,17 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, ret = ttm_bo_init_reserved(&i915->bdev, i915_gem_to_ttm(obj), bo_type, &i915_sys_placement, page_size >> PAGE_SHIFT, &ctx, NULL, NULL, i915_ttm_bo_destroy); + + /* +* XXX: The ttm_bo_init_reserved() functions returns -ENOSPC if the size +* is too big to add vma. The direct function that returns -ENOSPC is +* drm_mm_insert_node_in_range(). To handle the same error as other code +* that returns -E2BIG when the size is too large, it converts -ENOSPC to +* -E2BIG. +*/ + if (size >> PAGE_SHIFT > INT_MAX && ret == -ENOSPC) + ret = -E2BIG; + if (ret) return i915_ttm_err_to_gem(ret); -- 2.34.1
[PATCH v4 3/7] drm/i915: Check for integer truncation on scatterlist creation
From: Chris Wilson
There is an impedance mismatch between the scatterlist API using unsigned
int and our memory/page accounting in unsigned long. That is we may try
to create a scatterlist for a large object that overflows returning a
small table into which we try to fit very many pages. As the object size
is under control of userspace, we have to be prudent and catch the
conversion errors.
To catch the implicit truncation as we switch from unsigned long into the
scatterlist's unsigned int, we use overflows_type check and report
E2BIG prior to the operation. This is already used in our create ioctls to
indicate if the uABI request is simply too large for the backing store.
Failing that type check, we have a second check at sg_alloc_table time
to make sure the values we are passing into the scatterlist API are not
truncated.
It uses pgoff_t for locals that are dealing with page indices, in this
case, the page count is the limit of the page index.
And it uses safe_conversion() macro which performs a type conversion (cast)
of an integer value into a new variable, checking that the destination is
large enough to hold the source value.
v2: Move added i915_utils's macro into drm_util header (Jani N)
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Brian Welty
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
---
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 --
drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ---
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4
drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 5 -
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 4
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 -
drivers/gpu/drm/i915/gvt/dmabuf.c| 9 +
drivers/gpu/drm/i915/i915_scatterlist.h | 8
8 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
index c698f95af15f..ff2e6e780631 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
@@ -37,10 +37,13 @@ static int i915_gem_object_get_pages_internal(struct
drm_i915_gem_object *obj)
struct sg_table *st;
struct scatterlist *sg;
unsigned int sg_page_sizes;
- unsigned int npages;
+ pgoff_t npages; /* restricted by sg_alloc_table */
int max_order;
gfp_t gfp;
+ if (!safe_conversion(&npages, obj->base.size >> PAGE_SHIFT))
+ return -E2BIG;
+
max_order = MAX_ORDER;
#ifdef CONFIG_SWIOTLB
if (is_swiotlb_active(obj->base.dev->dev)) {
@@ -67,7 +70,6 @@ static int i915_gem_object_get_pages_internal(struct
drm_i915_gem_object *obj)
if (!st)
return -ENOMEM;
- npages = obj->base.size / PAGE_SIZE;
if (sg_alloc_table(st, npages, GFP_KERNEL)) {
kfree(st);
return -ENOMEM;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 7913f5402f56..d5f823cc1c2e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -26,9 +26,6 @@ enum intel_region_id;
* this and catch if we ever need to fix it. In the meantime, if you do
* spot such a local variable, please consider fixing!
*
- * Aside from our own locals (for which we have no excuse!):
- * - sg_table embeds unsigned int for nents
- *
* We can check for invalidly typed locals with typecheck(), see for example
* i915_gem_object_get_sg().
*/
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 0d0e46dae559..88ba7266a3a5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -28,6 +28,10 @@ static int i915_gem_object_get_pages_phys(struct
drm_i915_gem_object *obj)
void *dst;
int i;
+ /* Contiguous chunk, with a single scatterlist element */
+ if (overflows_type(obj->base.size, sg->length))
+ return -E2BIG;
+
if (GEM_WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 4eed3dd90ba8..604e8829e8ea 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -193,13 +193,16 @@ static int shmem_get_pages(struct drm_i915_gem_object
*obj)
struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct intel_memory_region *mem = obj->mm.region;
struct address_space *mapping = obj->base.filp->f_mapping;
- const unsigned long page_count = obj->base.size / PAGE_SIZE;
unsigned int max_segment = i915_sg_segment_size();
struct sg_table *st;
struct sgt_iter sgt_iter;
+ pgoff_t page_cou
[PATCH v4 5/7] drm/i915: Check if the size is too big while creating shmem file
The __shmem_file_setup() function returns -EINVAL if size is greater than MAX_LFS_FILESIZE. To handle the same error as other code that returns -E2BIG when the size is too large, it add a code that returns -E2BIG when the size is larger than the size that can be handled. v4: If BITS_PER_LONG is 32, size > MAX_LFS_FILESIZE is always false, so it checks only when BITS_PER_LONG is 64. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab Reported-by: kernel test robot --- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 604e8829e8ea..d41569ca6999 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -541,6 +541,20 @@ static int __create_shmem(struct drm_i915_private *i915, drm_gem_private_object_init(&i915->drm, obj, size); + /* XXX: The __shmem_file_setup() function returns -EINVAL if size is +* greater than MAX_LFS_FILESIZE. +* To handle the same error as other code that returns -E2BIG when +* the size is too large, we add a code that returns -E2BIG when the +* size is larger than the size that can be handled. +* If BITS_PER_LONG is 32, size > MAX_LFS_FILESIZE is always false, +* so we only needs to check when BITS_PER_LONG is 64. +* If BITS_PER_LONG is 32, E2BIG checks are processed when +* i915_gem_object_size_2big() is called before init_object() callback +* is called. +*/ + if (BITS_PER_LONG == 64 && size > MAX_LFS_FILESIZE) + return -E2BIG; + if (i915->mm.gemfs) filp = shmem_file_setup_with_mnt(i915->mm.gemfs, "i915", size, flags); -- 2.34.1
[PATCH v4 2/7] drm/i915/gem: Typecheck page lookups
From: Chris Wilson
We need to check that we avoid integer overflows when looking up a page,
and so fix all the instances where we have mistakenly used a plain
integer instead of a more suitable long. Be pedantic and add integer
typechecking to the lookup so that we can be sure that we are safe.
And it also uses pgoff_t as our page lookups must remain compatible with
the page cache, pgoff_t is currently exactly unsigned long.
v2: Move added i915_utils's macro into drm_util header (Jani N)
v3: Make not use the same macro name on a function. (Mauro)
For kernel-doc, macros and functions are handled in the same namespace,
the same macro name on a function prevents ever adding documentation
for it.
v4: Add kernel-doc markups to the kAPI functions and macros (Mauoro)
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
---
drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +-
drivers/gpu/drm/i915/gem/i915_gem_object.h| 293 --
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 27 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
.../drm/i915/gem/selftests/i915_gem_context.c | 12 +-
.../drm/i915/gem/selftests/i915_gem_mman.c| 8 +-
.../drm/i915/gem/selftests/i915_gem_object.c | 8 +-
drivers/gpu/drm/i915/i915_gem.c | 18 +-
drivers/gpu/drm/i915/i915_vma.c | 8 +-
9 files changed, 322 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index ccec4055fde3..90996fe8ad45 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -421,10 +421,11 @@ void __i915_gem_object_invalidate_frontbuffer(struct
drm_i915_gem_object *obj,
static void
i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ pgoff_t idx = offset >> PAGE_SHIFT;
void *src_map;
void *src_ptr;
- src_map = kmap_atomic(i915_gem_object_get_page(obj, offset >>
PAGE_SHIFT));
+ src_map = kmap_atomic(i915_gem_object_get_page(obj, idx));
src_ptr = src_map + offset_in_page(offset);
if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
@@ -437,9 +438,10 @@ i915_gem_object_read_from_page_kmap(struct
drm_i915_gem_object *obj, u64 offset,
static void
i915_gem_object_read_from_page_iomap(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ pgoff_t idx = offset >> PAGE_SHIFT;
+ dma_addr_t dma = i915_gem_object_get_dma_address(obj, idx);
void __iomem *src_map;
void __iomem *src_ptr;
- dma_addr_t dma = i915_gem_object_get_dma_address(obj, offset >>
PAGE_SHIFT);
src_map = io_mapping_map_wc(&obj->mm.region->iomap,
dma - obj->mm.region->region.start,
@@ -468,6 +470,7 @@ i915_gem_object_read_from_page_iomap(struct
drm_i915_gem_object *obj, u64 offset
*/
int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ GEM_BUG_ON(overflows_type(offset >> PAGE_SHIFT, pgoff_t));
GEM_BUG_ON(offset >= obj->base.size);
GEM_BUG_ON(offset_in_page(offset) > PAGE_SIZE - size);
GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 6f0a3ce35567..7913f5402f56 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -27,8 +27,10 @@ enum intel_region_id;
* spot such a local variable, please consider fixing!
*
* Aside from our own locals (for which we have no excuse!):
- * - sg_table embeds unsigned int for num_pages
- * - get_user_pages*() mixed ints with longs
+ * - sg_table embeds unsigned int for nents
+ *
+ * We can check for invalidly typed locals with typecheck(), see for example
+ * i915_gem_object_get_sg().
*/
#define GEM_CHECK_SIZE_OVERFLOW(sz) \
GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
@@ -363,44 +365,289 @@ i915_gem_object_get_tile_row_size(const struct
drm_i915_gem_object *obj)
int i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
unsigned int tiling, unsigned int stride);
+/**
+ * __i915_gem_object_page_iter_get_sg - helper to find the target scatterlist
+ * pointer and the target page position using pgoff_t n input argument and
+ * i915_gem_object_page_iter
+ * @obj: i915 GEM buffer object
+ * @iter: i915 GEM buffer object page iterator
+ * @n: page offset
+ * @offset: searched physical offset,
+ * it will be used for returning physical page offset value
+ *
+ * Context: Takes and releases the mutex lock of the i915_gem_object_page_iter.
+ * Takes and releases the RCU lock to search the radix_tree of
+ *
[PATCH v4 1/7] drm: Move and add a few utility macros into drm util header
It moves overflows_type utility macro into drm util header from i915_utils
header. The overflows_type can be used to catch the truncation between data
types. And it adds safe_conversion() macro which performs a type conversion
(cast) of an source value into a new variable, checking that the
destination is large enough to hold the source value.
And it adds exact_type and exactly_pgoff_t macro to catch type mis-match
while compiling.
v3: Add is_type_unsigned() macro (Mauro)
Modify overflows_type() macro to consider signed data types (Mauro)
Fix the problem that safe_conversion() macro always returns true
v4: Fix kernel-doc markups
Signed-off-by: Gwan-gyeong Mun
Cc: Thomas Hellström
Cc: Matthew Auld
Cc: Nirmoy Das
Cc: Jani Nikula
Reviewed-by: Mauro Carvalho Chehab
---
drivers/gpu/drm/i915/i915_utils.h | 5 +-
include/drm/drm_util.h| 77 +++
2 files changed, 78 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_utils.h
b/drivers/gpu/drm/i915/i915_utils.h
index c10d68cdc3ca..345e5b2dc1cd 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_X86
#include
@@ -111,10 +112,6 @@ bool i915_error_injected(void);
#define range_overflows_end_t(type, start, size, max) \
range_overflows_end((type)(start), (type)(size), (type)(max))
-/* Note we don't consider signbits :| */
-#define overflows_type(x, T) \
- (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T))
-
#define ptr_mask_bits(ptr, n) ({ \
unsigned long __v = (unsigned long)(ptr); \
(typeof(ptr))(__v & -BIT(n)); \
diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
index 79952d8c4bba..1de9ee5704fa 100644
--- a/include/drm/drm_util.h
+++ b/include/drm/drm_util.h
@@ -62,6 +62,83 @@
*/
#define for_each_if(condition) if (!(condition)) {} else
+/**
+ * is_type_unsigned - helper for checking data type which is an unsigned data
+ * type or not
+ * @x: The data type to check
+ *
+ * Returns:
+ * True if the data type is an unsigned data type, false otherwise.
+ */
+#define is_type_unsigned(x) ((typeof(x))-1 >= (typeof(x))0)
+
+/**
+ * overflows_type - helper for checking the truncation between data types
+ * @x: Source for overflow type comparison
+ * @T: Destination for overflow type comparison
+ *
+ * It compares the values and size of each data type between the first and
+ * second argument to check whether truncation can occur when assigning the
+ * first argument to the variable of the second argument.
+ * Source and Destination can be used with or without sign bit.
+ * Composite data structures such as union and structure are not considered.
+ * Enum data types are not considered.
+ * Floating point data types are not considered.
+ *
+ * Returns:
+ * True if truncation can occur, false otherwise.
+ */
+
+#define overflows_type(x, T) \
+ (is_type_unsigned(x) ? \
+ is_type_unsigned(T) ? \
+ (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T)) ? 1
: 0 \
+ : (sizeof(x) >= sizeof(T) && (x) >> (BITS_PER_TYPE(T) -
1)) ? 1 : 0 \
+ : is_type_unsigned(T) ? \
+ ((x) < 0) ? 1 : (sizeof(x) > sizeof(T) && (x) >>
BITS_PER_TYPE(T)) ? 1 : 0 \
+ : (sizeof(x) > sizeof(T)) ? \
+ ((x) < 0) ? (((x) * -1) >> BITS_PER_TYPE(T)) ? 1 : 0 \
+ : ((x) >> BITS_PER_TYPE(T)) ? 1 : 0 \
+ : 0)
+
+/**
+ * exact_type - break compile if source type and destination value's type are
+ * not the same
+ * @T: Source type
+ * @n: Destination value
+ *
+ * It is a helper macro for a poor man's -Wconversion: only allow variables of
+ * an exact type. It determines whether the source type and destination value's
+ * type are the same while compiling, and it breaks compile if two types are
+ * not the same
+ */
+#define exact_type(T, n) \
+ BUILD_BUG_ON(!__builtin_constant_p(n) &&
!__builtin_types_compatible_p(T, typeof(n)))
+
+/**
+ * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
+ * @n: value to compare pgoff_t type
+ *
+ * It breaks compile if the argument value's type is not pgoff_t type.
+ */
+#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
+
+/**
+ * safe_conversion - perform a type conversion (cast) of an source value into
+ * a new variable, checking that the destination is large enough to hold the
+ * source value.
+ * @ptr: Destination pointer address
+ * @value: Source value
+ *
+ * Returns:
+ * If the value would overflow the destination, it returns false.
+ */
+#define safe_conversion(ptr, value) ({ \
+ typeof(value) __v = (value); \
+ typeof(ptr) __ptr = (ptr); \
+ overflows_type(__v, *__ptr) ? 0 : ((*__ptr = (typeof(*__ptr))__v), 1); \
+})
+
/**
* drm_ca
[PATCH v4 0/7] Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation
This patch series fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation, etc. We need to check that we avoid integer overflows when looking up a page, and so fix all the instances where we have mistakenly used a plain integer instead of a more suitable long. And there is an impedance mismatch between the scatterlist API using unsigned int and our memory/page accounting in unsigned long. That is we may try to create a scatterlist for a large object that overflows returning a small table into which we try to fit very many pages. As the object size is under the control of userspace, we have to be prudent and catch the conversion errors. To catch the implicit truncation as we switch from unsigned long into the scatterlist's unsigned int, we use our overflows_type check and report E2BIG prior to the operation. This is already used in our create ioctls to indicate if the uABI request is simply too large for the backing store. And ttm place also has the same problem with scatterlist creation, and we fix the integer truncation problem with the way approached by scatterlist creation. And It corrects the error code to return -E2BIG when creating gem objects using ttm or shmem, if the size is too large in each case. In order to provide a common macro, it moves and adds a few utility macros into drm util header v3: Modify overflows_type() macro to consider signed data types and add is_type_unsigned() macro (Mauro) Make not use the same macro name on a function. (Mauro) For kernel-doc, macros and functions are handled in the same namespace, the same macro name on a function prevents ever adding documentation for it. Not to change execution inside a macro. (Mauro) Fix the problem that safe_conversion() macro always returns true (G.G) Add safe_conversion_gem_bug_on() macro and remove temporal SAFE_CONVERSION() macro. (G.G.) v4: Fix build warnins that reported by kernel test robot. (kernel test robot ) Add kernel-doc markups to the kAPI functions and macros (Mauoro) Testcase: igt@gem_create@create-massive Testcase: igt@gem_userptr_blits@input-checking Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4991 Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5411 Cc: Mauro Carvalho Chehab Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Cc: Nirmoy Das Cc: Jani Nikula Cc: David Airlie Cc: Daniel Vetter Chris Wilson (3): drm/i915/gem: Typecheck page lookups drm/i915: Check for integer truncation on scatterlist creation drm/i915: Remove truncation warning for large objects Gwan-gyeong Mun (4): drm: Move and add a few utility macros into drm util header drm/i915: Check for integer truncation on the configuration of ttm place drm/i915: Check if the size is too big while creating shmem file drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 +- drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +- drivers/gpu/drm/i915/gem/i915_gem_object.h| 303 +++--- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 27 +- drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4 + drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 19 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 23 +- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 +- .../drm/i915/gem/selftests/i915_gem_context.c | 12 +- .../drm/i915/gem/selftests/i915_gem_mman.c| 8 +- .../drm/i915/gem/selftests/i915_gem_object.c | 8 +- drivers/gpu/drm/i915/gvt/dmabuf.c | 9 +- drivers/gpu/drm/i915/i915_gem.c | 18 +- drivers/gpu/drm/i915/i915_gem.h | 4 + drivers/gpu/drm/i915/i915_scatterlist.h | 8 + drivers/gpu/drm/i915/i915_utils.h | 5 +- drivers/gpu/drm/i915/i915_vma.c | 8 +- drivers/gpu/drm/i915/intel_region_ttm.c | 20 +- include/drm/drm_util.h| 77 + 19 files changed, 478 insertions(+), 93 deletions(-) -- 2.34.1
Re: [PATCH v1 06/12] drm/bridge: cros-ec-anx7688: Use drm_bridge_funcs.atomic_check
On Sun, 17 Jul 2022 at 18:45, Sam Ravnborg wrote:
>
> Replace the deprecated drm_bridge_funcs.mode_fixup() with
> drm_bridge_funcs.atomic_check().
>
> drm_bridge_funcs.atomic_check() requires the atomic state operations,
> update these to the default implementations.
>
> Signed-off-by: Sam Ravnborg
Reviewed-by: Dave Stevenson
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> Cc: Benson Leung
> Cc: Guenter Roeck
> Cc: chrome-platf...@lists.linux.dev
> ---
> drivers/gpu/drm/bridge/cros-ec-anx7688.c | 28 +++-
> 1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/cros-ec-anx7688.c
> b/drivers/gpu/drm/bridge/cros-ec-anx7688.c
> index 0f6d907432e3..fc19ea87926f 100644
> --- a/drivers/gpu/drm/bridge/cros-ec-anx7688.c
> +++ b/drivers/gpu/drm/bridge/cros-ec-anx7688.c
> @@ -5,6 +5,7 @@
> * Copyright 2020 Google LLC
> */
>
> +#include
> #include
> #include
> #include
> @@ -45,9 +46,10 @@ bridge_to_cros_ec_anx7688(struct drm_bridge *bridge)
> return container_of(bridge, struct cros_ec_anx7688, bridge);
> }
>
> -static bool cros_ec_anx7688_bridge_mode_fixup(struct drm_bridge *bridge,
> - const struct drm_display_mode
> *mode,
> - struct drm_display_mode
> *adjusted_mode)
> +static int cros_ec_anx7688_bridge_atomic_check(struct drm_bridge *bridge,
> + struct drm_bridge_state
> *bridge_state,
> + struct drm_crtc_state
> *crtc_state,
> + struct drm_connector_state
> *conn_state)
> {
> struct cros_ec_anx7688 *anx = bridge_to_cros_ec_anx7688(bridge);
> int totalbw, requiredbw;
> @@ -56,13 +58,13 @@ static bool cros_ec_anx7688_bridge_mode_fixup(struct
> drm_bridge *bridge,
> int ret;
>
> if (!anx->filter)
> - return true;
> + return 0;
>
> /* Read both regs 0x85 (bandwidth) and 0x86 (lane count). */
> ret = regmap_bulk_read(anx->regmap, ANX7688_DP_BANDWIDTH_REG, regs,
> 2);
> if (ret < 0) {
> DRM_ERROR("Failed to read bandwidth/lane count\n");
> - return false;
> + return ret;
> }
> dpbw = regs[0];
> lanecount = regs[1];
> @@ -71,28 +73,34 @@ static bool cros_ec_anx7688_bridge_mode_fixup(struct
> drm_bridge *bridge,
> if (dpbw > 0x19 || lanecount > 2) {
> DRM_ERROR("Invalid bandwidth/lane count (%02x/%d)\n", dpbw,
> lanecount);
> - return false;
> + return -EINVAL;
> }
>
> /* Compute available bandwidth (kHz) */
> totalbw = dpbw * lanecount * 27 * 8 / 10;
>
> /* Required bandwidth (8 bpc, kHz) */
> - requiredbw = mode->clock * 8 * 3;
> + requiredbw = crtc_state->mode.clock * 8 * 3;
>
> DRM_DEBUG_KMS("DP bandwidth: %d kHz (%02x/%d); mode requires %d
> Khz\n",
> totalbw, dpbw, lanecount, requiredbw);
>
> if (totalbw == 0) {
> DRM_ERROR("Bandwidth/lane count are 0, not rejecting
> modes\n");
> - return true;
> + return 0;
> }
>
> - return totalbw >= requiredbw;
> + if (totalbw < requiredbw)
> + return -EINVAL;
> +
> + return 0;
> }
>
> static const struct drm_bridge_funcs cros_ec_anx7688_bridge_funcs = {
> - .mode_fixup = cros_ec_anx7688_bridge_mode_fixup,
> + .atomic_check = cros_ec_anx7688_bridge_atomic_check,
> + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> + .atomic_reset = drm_atomic_helper_bridge_reset,
> };
>
> static int cros_ec_anx7688_bridge_probe(struct i2c_client *client)
> --
> 2.34.1
>
Re: [PATCH v1 05/12] drm/bridge: sii8620: Use drm_bridge_funcs.atomic_check
On Sun, 17 Jul 2022 at 18:45, Sam Ravnborg wrote:
>
> Replace the deprecated drm_bridge_funcs.mode_fixup() with
> drm_bridge_funcs.atomic_check().
>
> drm_bridge_funcs.atomic_check() requires the atomic state operations,
> update these to the default implementations.
>
> Signed-off-by: Sam Ravnborg
Reviewed-by: Dave Stevenson
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> ---
> drivers/gpu/drm/bridge/sil-sii8620.c | 17 +++--
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> b/drivers/gpu/drm/bridge/sil-sii8620.c
> index ab0bce4a988c..b6e5c285c8ea 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -8,6 +8,7 @@
>
> #include
>
> +#include
> #include
> #include
> #include
> @@ -2262,26 +2263,30 @@ static enum drm_mode_status sii8620_mode_valid(struct
> drm_bridge *bridge,
> }
> }
>
> -static bool sii8620_mode_fixup(struct drm_bridge *bridge,
> - const struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> +static int sii8620_atomic_check(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> {
> struct sii8620 *ctx = bridge_to_sii8620(bridge);
>
> mutex_lock(&ctx->lock);
>
> - ctx->use_packed_pixel = sii8620_is_packing_required(ctx,
> adjusted_mode);
> + ctx->use_packed_pixel = sii8620_is_packing_required(ctx,
> &crtc_state->adjusted_mode);
>
> mutex_unlock(&ctx->lock);
>
> - return true;
> + return 0;
> }
>
> static const struct drm_bridge_funcs sii8620_bridge_funcs = {
> .attach = sii8620_attach,
> .detach = sii8620_detach,
> - .mode_fixup = sii8620_mode_fixup,
> + .atomic_check = sii8620_atomic_check,
> .mode_valid = sii8620_mode_valid,
> + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> + .atomic_reset = drm_atomic_helper_bridge_reset,
> };
>
> static int sii8620_probe(struct i2c_client *client,
> --
> 2.34.1
>
Re: [PATCH V2 0/3] DSI host and peripheral initialisation ordering
Hi Sam On Mon, 18 Jul 2022 at 21:52, Sam Ravnborg wrote: > > Hi Dave, > > a long overdue reply on this series. > > On Fri, Mar 04, 2022 at 03:17:55PM +, Dave Stevenson wrote: > > Hi All > > > > Changes from v1: > > - New patch to refactor drm_bridge_chain_post_disable and > > drm_bridge_chain_pre_enable > > to reuse drm_atomic_bridge_chain_post_disable / > > drm_atomic_bridge_chain_pre_enable > > but with a NULL state. > > - New patch that adds a pre_enable_upstream_first to drm_panel. > > - changed from an OPS flag to a bool "pre_enable_upstream_first" in > > drm_bridge. > > - Followed Andrzej's suggestion of using continue in the main loop to avoid > > needing 2 additional loops (one forward to find the last bridge wanting > > upstream first, and the second backwards again). > > - Actioned Laurent's review comments on docs patch. > > > > Original cover letter: > > > > Hopefully I've cc'ed all those that have bashed this problem around > > previously, > > or are otherwise linked to DRM bridges. > > > > There have been numerous discussions around how DSI support is currently > > broken > > as it doesn't support initialising the PHY to LP-11 and potentially the > > clock > > lane to HS prior to configuring the DSI peripheral. There is no op where the > > interface is initialised but HS video isn't also being sent. > > Currently you have: > > - peripheral pre_enable (host not initialised yet) > > - host pre_enable > > - encoder enable > > - host enable > > - peripheral enable (video already running) > > > > vc4 and exynos currently implement the DSI host as an encoder, and split the > > bridge_chain. This fails if you want to switch to being a bridge and/or use > > atomic calls as the state of all the elements split off are not added by > > drm_atomic_add_encoder_bridges. > > A typically chain looks like this: > > CRTC => Encoder => Bridge A => Bridge B > > We have in DRM bridges established what is the "next" bridge - indicated > with the direction of the arrows in the drawing. > > This set of patches introduces the concept of "upstream" bridges. > > pre_enable_prev_bridge_first would be easier to understand as it uses > the current terminology. > I get that "upstream" is used in the DSI specification - but we are > dealing with bridges that happens to support DSI and more, and mixing > the two terminologies is not good. > > Note: Upstream is also used in a bridge doc section - here it should > most likely be updated too. Sure, I have no issues with switching to prev/next from upstream/downstream. To the outsider it can be confusing - in pre_enable and disable, the next bridge to be called is the previous one. At least it is documented. > The current approach set a flag that magically makes the core do something > else. Have you considered a much more explicit approach? > > A few helpers like: > > drm_bridge_pre_enable_prev_bridge() > drm_bridge_enable_prev_bridge() > drm_bridge_disable_prev_bridge() > drm_bridge_post_disable_prev_bridge() No point in drm_bridge_enable_prev_bridge() and drm_bridge_post_disable_prev_bridge() as the call order down the chain will mean that they have already been called. drm_bridge_enable_next_bridge() and drm_bridge_post_disable_next_bridge() possibly. > And then update the core so the relevant function is only called once > for a bridge. > Then the need for DSI lanes in LP-11 can be archived by a call to > > drm_bridge_pre_enable_prev_bridge() Unfortunately it gets ugly with post_disable. The DSI host controller post_disable will have been called before the DSI peripheral's post_disable, and there are conditions where the peripheral needs to send DSI commands in post_disable (eg panel-asus-z00t-tm5p5-n35596 [1]). Changing all DSI hosts to call drm_bridge_post_disable_next_bridge feels like the wrong thing to do. There are currently hacks in dw-mipi-dsi that do call the next panel/bridge post_disable [2] and it would be nice to get rid of them. Currently the calls aren't tracked for state, so you end up with post_disable being called twice, and panels having to track state (eg jdi_lt070me05 [3]). [1] tm5p5_nt35596_unprepare() calls tm5p5_nt35596_off() https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c#L107 [2] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L889 [3] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c#L44 > This is more explicit than a flag that triggers some magic behaviour. > It may even see uses we have not realised yet. Personally it feels like more boilerplate in almost all DSI drivers, and generally I see a push to remove boilerplate. > It is late here - so maybe the above is not a good idea tomorrow - but > right now I like the simplicity of it. > > Other than the above I read that a mipi_dsi_host_init() is planned, > which is also explicit
Re: [PATCH v2 1/2] drm/bridge: anx7625: Fix refcount bug in anx7625_parse_dt()
On Tue, 19 Jul 2022 at 08:55, Liang He wrote:
>
> In anx7625_parse_dt(), 'pdata->mipi_host_node' will be assigned a
> new reference with of_graph_get_remote_node() which will increase
> the refcount of the object, correspondingly, we should call
> of_node_put() for the old reference stored in the 'pdata->mipi_host_node'.
>
> Fixes: 8bdfc5dae4e3 ("drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP")
> Signed-off-by: Liang He
> ---
> changelog:
>
> v2: (1) rebase with drm-misc-next advised by Robert Foss
> (2) use proper title
> (3) remove the v1's second bug ('ep0'), fixed recently
> v1: https://lore.kernel.org/all/20220707012330.305646-1-win...@126.com/
>
> drivers/gpu/drm/bridge/analogix/anx7625.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index d1f1d525aeb6..79fc7a50b497 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1642,6 +1642,7 @@ static int anx7625_parse_dt(struct device *dev,
> anx7625_get_swing_setting(dev, pdata);
>
> pdata->is_dpi = 0; /* default dsi mode */
> + of_node_put(pdata->mipi_host_node);
> pdata->mipi_host_node = of_graph_get_remote_node(np, 0, 0);
> if (!pdata->mipi_host_node) {
> DRM_DEV_ERROR(dev, "fail to get internal panel.\n");
> --
> 2.25.1
>
This series has my r-b.
Reviewed-by: Robert Foss
Applied series to drm-misc-next.
Re: [PATCH] drm/amdgpu: Fix comment typo
Am 16.07.22 um 06:28 schrieb Jason Wang: The double `to' is duplicated in the comment, remove one. Signed-off-by: Jason Wang Reviewed-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index e3d139708160..b45cd7cbbea8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -80,7 +80,7 @@ * - 3.24.0 - Add high priority compute support for gfx9 * - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk). * - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE. - * - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation. + * - 3.27.0 - Add new chunk to AMDGPU_CS to enable BO_LIST creation. * - 3.28.0 - Add AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES * - 3.29.0 - Add AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID * - 3.30.0 - Add AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE.
Re: [PATCH] drm/radeon: Fix comment typo
Am 16.07.22 um 05:57 schrieb Jason Wang: The double `have' is duplicated in line 696, remove one. Signed-off-by: Jason Wang Reviewed-by: Christian König --- drivers/gpu/drm/radeon/radeon_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index 84843b3b3aef..261fcbae88d7 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -693,7 +693,7 @@ int radeon_gem_va_ioctl(struct drm_device *dev, void *data, } /* !! DONT REMOVE !! -* We don't support vm_id yet, to be sure we don't have have broken +* We don't support vm_id yet, to be sure we don't have broken * userspace, reject anyone trying to use non 0 value thus moving * forward we can use those fields without breaking existant userspace */
Re: [PATCH 1/9] drm/panel/panel-sitronix-st7701: Make DSI mode flags common to ST7701
On 7/19/22 10:39, Linus Walleij wrote: On Sun, Jul 10, 2022 at 9:44 PM Marek Vasut wrote: The ST7701 and ST7701S are TFT matrix drivers with integrated multi protocol decoder capable of DSI/DPI/SPI input and 480x360...864 line TFT matrix output. Currently the only supported input is DSI. The protocol decoder is separate from the TFT matrix driver and is always capable of handling all of DSI non-burst mode with sync pulses or sync events as well as DSI burst mode. Move the DSI mode configuration from TFT matrix driver properties to common ST7701 code, because this is common to all TFT matrices. Signed-off-by: Marek Vasut Cc: Guido Günther Cc: Jagan Teki Cc: Laurent Pinchart Cc: Linus Walleij Cc: Sam Ravnborg Cc: Thierry Reding All 9 patches applied and pushed for drm-misc-next. Nice, and I got cleared to submit another panel support which prompted this rework, so that's coming soon.
Re: [PATCH v2 07/12] drm: bridge: samsung-dsim: Add module init, exit
On Mon, May 9, 2022 at 5:35 PM Marek Szyprowski
wrote:
>
> On 04.05.2022 13:40, Jagan Teki wrote:
> > Add module init and exit functions for the bridge to register
> > and unregister dsi_driver.
> >
> > Exynos drm driver stack will register the platform_driver separately
> > in the common of it's exynos_drm_drv.c including dsi_driver.
> >
> > Register again would return -EBUSY, so return 0 for such cases as
> > dsi_driver is already registered.
> >
> > v2, v1:
> > * none
> >
> > Signed-off-by: Jagan Teki
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 22 ++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index 8f9ae16d45bc..b618e52d0ee3 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -1740,6 +1740,28 @@ struct platform_driver dsi_driver = {
> > },
> > };
> >
> > +static int __init samsung_mipi_dsim_init(void)
> > +{
> > + int ret;
> > +
> > + ret = platform_driver_register(&dsi_driver);
> > +
> > + /**
> > + * Exynos drm driver stack will register the platform_driver
> > + * separately in the common of it's exynos_drm_drv.c including
> > + * dsi_driver. Register again would return -EBUSY, so return 0
> > + * for such cases as dsi_driver is already registered.
> > + */
> > + return ret == -EBUSY ? 0 : ret;
> > +}
> > +module_init(samsung_mipi_dsim_init);
>
> I've just noticed this. The above approach is really a bad pattern:
> registering the same driver 2 times and relying on the error.
If it tries to register 2nd time, then it returns EBUSY so we are
returning 0 for that case. not sure why it registers 2nd time again.
Jagan.
Re: [PATCH v7 7/9] drm/virtio: Improve DMA API usage for shmem BOs
On 7/19/22 13:31, Gerd Hoffmann wrote: > On Wed, Jul 06, 2022 at 10:22:52AM +0300, Dmitry Osipenko wrote: >> On 7/6/22 10:13, Gerd Hoffmann wrote: >>> Hi, >>> Gerd, thank you very much! It's was indeed unclear to me how to test the MMIO GPU, but yours variant with microvm works! I was looking for trying aarch64 in the past, but it also was unclear how to do it since there is no DT support for the VirtIO-GPU, AFAICS. >>> >>> aarch64 uses acpi by default (can be disabled via 'qemu -no-acpi'). >>> Not fully sure about arm(v7). >>> >>> Even with DT it should work because DT only describes the virtio-mmio >>> 'slots', not the actual virtio devices. >>> There is no virgl support because it's a virtio-gpu-device and not virtio-gpu-device-gl that is PCI-only in Qemu. Hence everything seems good. >>> >>> It's named 'virtio-gpu-gl-device' >> >> Ah, thanks again! Just quickly tested virtio-gpu-gl-device and >> everything works too for MMIO GPU on microvm, including virgl and Xorg >> (glamor). >> >> [drm] features: +virgl +edid -resource_blob -host_visible >> [drm] features: -context_init >> [drm] number of scanouts: 1 >> [drm] number of cap sets: 2 >> [drm] cap set 0: id 1, max-version 1, max-size 308 >> [drm] cap set 1: id 2, max-version 2, max-size 696 >> [drm] Initialized virtio_gpu 0.1.0 0 for LNRO0005:01 on minor 0 >> virtio-mmio LNRO0005:01: [drm] drm_plane_enable_fb_damage_clips() not called > > Cool. Havn't found the time to try s390x, so I'm taking that as good > enough test that we don't have any unnoticed dependencies on pci. > > Queued up. I'll go over a few more pending patches, and assuming no > issues show up in testing this should land in drm-misc-next in a few > hours. Great, thank you. -- Best regards, Dmitry
Re: [PATCH v1 12/12] drm/todo: Add bridge related todo items
Hi Sam On Mon, 18 Jul 2022 at 19:00, Sam Ravnborg wrote: > > Hi Dave, > > On Mon, Jul 18, 2022 at 11:27:37AM +0100, Dave Stevenson wrote: > > Hi Sam > > > > On Sun, 17 Jul 2022 at 18:58, Sam Ravnborg wrote: > > > > > > Add todo in the hope someone will help updating the bridge drivers. > > > > > > v2: > > > - Updated descriptions in todo.rst > > > > > > Signed-off-by: Sam Ravnborg > > > Acked-by: Maxime Ripard > > > Cc: Laurent Pinchart > > > Cc: Maarten Lankhorst > > > Cc: Maxime Ripard > > > Cc: Thomas Zimmermann > > > Cc: David Airlie > > > Cc: Daniel Vetter > > > --- > > > Documentation/gpu/todo.rst | 20 > > > 1 file changed, 20 insertions(+) > > > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > > index 10bfb50908d1..fbcc232e0bc1 100644 > > > --- a/Documentation/gpu/todo.rst > > > +++ b/Documentation/gpu/todo.rst > > > @@ -480,6 +480,26 @@ Contact: Thomas Zimmermann > > > > > > Level: Starter > > > > > > +Drop use of deprecated operations in bridge drivers > > > +-- > > > + > > > +&struct drm_bridge_funcs contains a number of deprecated operations > > > +which can be replaced by the atomic variants. > > > + > > > +The following is more or less 1:1 replacements with the arguments > > > +and names adjusted: > > > +* pre_enable => atomic_pre_enable > > > +* enable => atomic_enable > > > +* disable => atomic_disable > > > +* post_disable => atomic_post_disable > > > + > > > +* mode_set is no longer required and the implementation shall be merged > > > + with atomic_enable. > > > > The dw-mipi-dsi and msm DSI host controller bridge drivers are > > currently relying on mode_set as a convenient hook to power up the DSI > > host prior to pre_enable of the bridge chain. Removing it is therefore > > going to break those. > > > > There is a proposed mechanism to allow for the removal of this hack > > [1], but it's still waiting on R-B tags and/or discussion from bridge > > maintainers (gentle nudge as you are one of those maintainers). > > I have over time gained some knowledge of how bridges works and have > applied a few patches - but the maintainer role belongs to others. > I just try to help a bit. Apologies, I'd misread the text in this patch +Contact: bridge maintainers, Sam Ravnborg , + Laurent Pinchart as being that you and Laurent were the bridge maintainers. Colon instead of comma :-/ > I will review the referenced series - could you then in return > review this series? Sure, these look to be within my levels of knowledge. > > > > And do you mean merge with atomic_enable, or merge with > > atomic_pre_enable? atomic_pre_enable would be more applicable for > > almost all the bridges I'm aware of as they want to be configured > > before video starts. > Thanks, yes you are right. I will update the text to refer to > pre_enable as this is often the right choice. Looking at how many > bridges who implements mode_set, or are not yet atomic, this will > take a while before we can drop it. Thanks. That makes more logical sense to me for the majority of cases. As to timescales, it always takes longer than ideal to migrate older drivers. Thanks Dave
Re: [PATCH v7 7/9] drm/virtio: Improve DMA API usage for shmem BOs
On Wed, Jul 06, 2022 at 10:22:52AM +0300, Dmitry Osipenko wrote: > On 7/6/22 10:13, Gerd Hoffmann wrote: > > Hi, > > > >> Gerd, thank you very much! It's was indeed unclear to me how to test the > >> MMIO GPU, but yours variant with microvm works! I was looking for trying > >> aarch64 in the past, but it also was unclear how to do it since there is > >> no DT support for the VirtIO-GPU, AFAICS. > > > > aarch64 uses acpi by default (can be disabled via 'qemu -no-acpi'). > > Not fully sure about arm(v7). > > > > Even with DT it should work because DT only describes the virtio-mmio > > 'slots', not the actual virtio devices. > > > >> There is no virgl support because it's a virtio-gpu-device and not > >> virtio-gpu-device-gl that is PCI-only in Qemu. Hence everything seems good. > > > > It's named 'virtio-gpu-gl-device' > > Ah, thanks again! Just quickly tested virtio-gpu-gl-device and > everything works too for MMIO GPU on microvm, including virgl and Xorg > (glamor). > > [drm] features: +virgl +edid -resource_blob -host_visible > [drm] features: -context_init > [drm] number of scanouts: 1 > [drm] number of cap sets: 2 > [drm] cap set 0: id 1, max-version 1, max-size 308 > [drm] cap set 1: id 2, max-version 2, max-size 696 > [drm] Initialized virtio_gpu 0.1.0 0 for LNRO0005:01 on minor 0 > virtio-mmio LNRO0005:01: [drm] drm_plane_enable_fb_damage_clips() not called Cool. Havn't found the time to try s390x, so I'm taking that as good enough test that we don't have any unnoticed dependencies on pci. Queued up. I'll go over a few more pending patches, and assuming no issues show up in testing this should land in drm-misc-next in a few hours. take care, Gerd
Re: [Intel-gfx] [PATCH 0/2] drm/i915/gt: Expose per gt defaults in sysfs
On Mon, Jul 18, 2022 at 06:07:06PM -0700, Ashutosh Dixit wrote: > Create a gt/gtN/.defaults/ directory (similar to > engine//.defaults/) to expose default parameter values for each > gt in sysfs. This allows userspace to restore default parameter values > after they may have changed. > > Patch 1: Creates the gt/gtN/.defaults/ directory > Patch 2: Adds per-gt RPS defaults (rps_max_freq_mhz and rps_min_freq_mhz) >to gt/gtN/.defaults/ > > An approved Level-0/oneAPI UMD pull request which consumes the exposed > defaults can be seen here: > https://github.com/intel/compute-runtime/pull/552 > The UMD pull request will be merged if/after this series is merged to i915. Pushed to drm-intel-gt-next. Thanks for the patches. > > Previous discussion on these patches can be seen here: > https://patchwork.freedesktop.org/patch/484238/?series=102665&rev=4 > https://patchwork.freedesktop.org/patch/483988/?series=102665&rev=3 > > Cc: Matt Roper > Cc: Tvrtko Ursulin > Cc: Andi Shyti > Signed-off-by: Ashutosh Dixit > > Ashutosh Dixit (2): > drm/i915/gt: Create gt/gtN/.defaults/ for per gt sysfs defaults > drm/i915/gt: Expose per-gt RPS defaults in sysfs > > drivers/gpu/drm/i915/gt/intel_gt_sysfs.c| 10 +++--- > drivers/gpu/drm/i915/gt/intel_gt_sysfs.h| 6 > drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 34 + > drivers/gpu/drm/i915/gt/intel_gt_types.h| 9 ++ > drivers/gpu/drm/i915/gt/intel_rps.c | 2 ++ > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 12 +--- > 6 files changed, 64 insertions(+), 9 deletions(-) > > -- > 2.34.1 >
Re: [PATCH v1 0/11] drm: move dri1 drivers to drm/dri1/
Hi
Am 19.07.22 um 11:52 schrieb Daniel Vetter:
On Mon, 18 Jul 2022 at 08:56, Thomas Zimmermann wrote:
Hi
Am 16.07.22 um 20:17 schrieb Sam Ravnborg:
While discussing the way forward for the via driver
Javier came up with the proposal to move all DRI1 drivers
to their own folder.
The idea is to move the old DRI1 drivers so one do not
accidentally consider them modern drivers.
This set of patches implements this idea.
To prepare the move, DRIVER_LEGACY and CONFIG_DRM_LEGACY
are both renamed to *_DRI1. This makes it more obvious
that we are dealing with DRI1 drivers, as we have
a lot of other legacy support.
The drivers continue to have their own sub-directory
so the driver files are not mixed with the core files
which are copied in the last commit.
The DRI1 specific part of drm/Kconfig is likewise pulled
out and located in the dri1/ folder.
Feedback welcome!
To be honest, I still don't like this rename. Especially in the case of
via, which has a KMS driver coming up. It will now have an include
statement that crosses several levels in the directory hierarchy. And
what about the other DRI1 drivers? If we ever get KMS drivers for those,
do we want to move some header files back into their original locations?
Patches 1 and 2 look reasonable to me. The other driver patches have
basically zero upside IMHO.
Imo transitional drivers with both legacy dri1 and kms+gem support
made some sense 10+ years ago when all this infrastructure was still
being built. Now I really don't see much point.
For via imo make it a clean new driver, and copypaste anything from
the old one (like register headers) it needs. That will also make
review a ton easier I think. There has not been any actual via work,
just general refactoring, in that driver for 10+ years, so "bugfix
sharing" is really not an argument.
At some point, I suggested to turn the VIA KMS driver into a new
'unichrome' driver, but everyone wants to keep 'via' instead.
In the case of moving the core files into dri1/, the resulting Makefile
rule looks really ugly. I'd suggest to move all code into a separate
file drm_dri1.c and be done with it. For something more elaborate,
there could by drm_dri1.c and drm_dri1_helper.c, where the latter
contains all DRI1 code that is only used by the drivers.
Ugly Makefile for dri1 might be a feature :-) But personally no stake
on this bikeshed.
It's the core DRM makefile that would look ugly. :/
Best regards
Thomas
-Daniel
Best regards
Thomsa
Sam
Sam Ravnborg (11):
drm: rename DRIVER_LEGACY to DRIVER_DRI1
drm: Rename CONFIG_DRM_LEGACY to CONFIG_DRM_DRI1
drm/tdfx: Move the tdfx driver to drm/dri1/
drm/r128: Move the r128 driver to drm/dri1/
drm/i810: Move the i810 driver to drm/dri1/
drm/mga: Move the mga driver to drm/dri1/
drm/sis: Move the sis driver to drm/dri1/
drm/via: Move the via driver to drm/dri1/
drm/savage: Move the savage driver to drm/dri1/
drm/dri1: Move Kconfig logic to drm/dri1
drm: Move dri1 core files to drm/dri1
arch/powerpc/configs/pmac32_defconfig | 2 +-
arch/powerpc/configs/ppc6xx_defconfig | 2 +-
drivers/char/agp/Makefile | 2 +-
drivers/char/agp/agp.h | 2 +-
drivers/gpu/drm/Kconfig| 79
+-
drivers/gpu/drm/Makefile | 18 +++--
drivers/gpu/drm/dri1/Kconfig | 79
++
drivers/gpu/drm/dri1/Makefile | 11 +++
drivers/gpu/drm/{ => dri1}/drm_agpsupport.c| 4 +-
drivers/gpu/drm/{ => dri1}/drm_bufs.c | 22 +++---
drivers/gpu/drm/{ => dri1}/drm_context.c | 24 +++
drivers/gpu/drm/{ => dri1}/drm_dma.c | 4 +-
drivers/gpu/drm/{ => dri1}/drm_hashtab.c | 0
drivers/gpu/drm/{ => dri1}/drm_irq.c | 6 +-
drivers/gpu/drm/{ => dri1}/drm_legacy_misc.c | 2 +-
drivers/gpu/drm/{ => dri1}/drm_lock.c | 6 +-
drivers/gpu/drm/{ => dri1}/drm_memory.c| 0
drivers/gpu/drm/{ => dri1}/drm_scatter.c | 6 +-
drivers/gpu/drm/{ => dri1}/drm_vm.c| 2 +-
drivers/gpu/drm/{ => dri1}/i810/Makefile | 0
drivers/gpu/drm/{ => dri1}/i810/i810_dma.c | 0
drivers/gpu/drm/{ => dri1}/i810/i810_drv.c | 2 +-
drivers/gpu/drm/{ => dri1}/i810/i810_drv.h | 0
drivers/gpu/drm/{ => dri1}/mga/Makefile| 0
drivers/gpu/drm/{ => dri1}/mga/mga_dma.c | 0
drivers/gpu/drm/{ => dri1}/mga/mga_drv.c | 2 +-
drivers/gpu/drm/{ => dri1}/mga/mga_drv.h | 0
drivers/gpu/drm/{ => dri1}/mga/mga_ioc32.c | 0
drivers/gpu/drm/{ => dri1}/mga/mga_irq.c | 0
drivers/gpu/drm/{ => dri1}/mga/mga_state.c | 0
drivers/gpu/drm/{ => d
Re: [Intel-gfx] [PATCH 02/12] drm/i915/guc: Don't call ring_is_idle in GuC submission
On 19/07/2022 10:49, Tvrtko Ursulin wrote:
On 19/07/2022 01:09, John Harrison wrote:
On 7/18/2022 05:26, Tvrtko Ursulin wrote:
On 13/07/2022 00:31, john.c.harri...@intel.com wrote:
From: Matthew Brost
The engine registers really shouldn't be touched during GuC submission
as the GuC owns the registers. Don't call ring_is_idle and tie
Touch being just read and it is somehow harmful?
The registers are meaningless when GuC is controlling the submission.
The i915 driver has no knowledge of what context is or is not
executing on any given engine at any given time. So reading reading
the ring registers is incorrect - it can lead to bad assumptions about
what state the hardware is in.
Same is actually true with the execlists backend. The code in
ring_is_idle is not concerning itself with which context is running or
not. Just that the head/tail/ctl appear idle.
Problem/motivation appears to be on a higher than simply ring registers.
I am not claiming it makes sense with Guc and that it has to remain but
just suggesting for as a minimum clearer commit message.
intel_engine_is_idle strictly to the engine pm.
Strictly seems wrong - it is just ring_is_idle check that is replaced
and not the whole implementation of intel_engine_is_idle.
Because intel_engine_is_idle tied to the engine pm, retire requests
before checking intel_engines_are_idle in gt_drop_caches, and lastly
Why is re-ordering important? I at least can't understand it. I hope
it's not working around IGT failures.
If requests are physically completed but not retired then they will be
holding unnecessary PM references. So we need to flush those out
before checking for idle.
And if they are not as someone passes in DROP_RESET_ACTIVE? They will
not retire and code still enters intel_engines_are_idle so that has to
work, no? Something does not align for me still.
With "not retire" I meant potentially not retire within
I915_IDLE_ENGINES_TIMEOUT. I guess hack happens to work for some or all
IGTs which use DROP_RESET_ACTIVE.
Does it also mean patch would fix that problem without touching
intel_engine_is_idle/ring_is_idle - with just the re-ordering in
gt_drop_caches?
Regards,
Tvrtko
increase the timeout in gt_drop_caches for the intel_engines_are_idle
check.
Same here - why?
@Matthew Brost - do you recall which particular tests were hitting an
issue? I'm guessing gem_ctx_create? I believe that's the one that
creates and destroys thousands of contexts. That is much slower with
GuC (GuC communication required) than with execlists (i915 internal
state change only).
And if that is a logically separate change please split the patch up.
Regards,
Tvrtko
John.
Regards,
Tvrtko
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 13 +
drivers/gpu/drm/i915/i915_debugfs.c | 6 +++---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 283870c659911..959a7c92e8f4d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1602,6 +1602,9 @@ static bool ring_is_idle(struct
intel_engine_cs *engine)
{
bool idle = true;
+ /* GuC submission shouldn't access HEAD & TAIL via MMIO */
+ GEM_BUG_ON(intel_engine_uses_guc(engine));
+
if (I915_SELFTEST_ONLY(!engine->mmio_base))
return true;
@@ -1668,6 +1671,16 @@ bool intel_engine_is_idle(struct
intel_engine_cs *engine)
if (!i915_sched_engine_is_empty(engine->sched_engine))
return false;
+ /*
+ * We shouldn't touch engine registers with GuC submission as
the GuC
+ * owns the registers. Let's tie the idle to engine pm, at
worst this
+ * function sometimes will falsely report non-idle when idle
during the
+ * delay to retire requests or with virtual engines and a request
+ * running on another instance within the same class / submit
mask.
+ */
+ if (intel_engine_uses_guc(engine))
+ return false;
+
/* Ring stopped? */
return ring_is_idle(engine);
}
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 94e5c29d2ee3a..ee5334840e9cb 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -654,13 +654,13 @@ gt_drop_caches(struct intel_gt *gt, u64 val)
{
int ret;
+ if (val & DROP_RETIRE || val & DROP_RESET_ACTIVE)
+ intel_gt_retire_requests(gt);
+
if (val & DROP_RESET_ACTIVE &&
wait_for(intel_engines_are_idle(gt),
I915_IDLE_ENGINES_TIMEOUT))
intel_gt_set_wedged(gt);
- if (val & DROP_RETIRE)
- intel_gt_retire_requests(gt);
-
if (val & (DROP_IDLE | DROP_ACTIVE)) {
ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
if (ret)
diff --git a/drivers/gpu/
[PATCH v3 3/3] drm/i915: Use luminance range calculated during edid parsing
Instead of using fixed 0 - 512 range use luminance range calculated
as a part of edid parsing. As a backup fall back to static 0 - 512.
v3: Clean-ups suggested by Jani Nikula
v2: Use values calculated during edid parsing
Cc: Lyude Paul
Cc: Mika Kahola
Cc: Jani Nikula
Cc: Manasi Navare
Signed-off-by: Jouni Högander
---
.../gpu/drm/i915/display/intel_dp_aux_backlight.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index c92d5bb2326a..83af95bce98d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -278,6 +278,8 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector
*connector, enum pipe pi
{
struct drm_i915_private *i915 = to_i915(connector->base.dev);
struct intel_panel *panel = &connector->panel;
+ struct drm_luminance_range_info *luminance_range =
+ &connector->base.display_info.luminance_range;
int ret;
if (panel->backlight.edp.intel.sdr_uses_aux) {
@@ -293,8 +295,17 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector
*connector, enum pipe pi
}
}
- panel->backlight.max = 512;
- panel->backlight.min = 0;
+ if (luminance_range->max_luminance) {
+ panel->backlight.max = luminance_range->max_luminance;
+ panel->backlight.min = luminance_range->min_luminance;
+ } else {
+ panel->backlight.max = 512;
+ panel->backlight.min = 0;
+ }
+
+ drm_dbg_kms(&i915->drm, "Using backlight range %d..%d\n",
panel->backlight.min,
+ panel->backlight.max);
+
panel->backlight.level = intel_dp_aux_hdr_get_backlight(connector,
pipe);
panel->backlight.enabled = panel->backlight.level != 0;
--
2.25.1
[PATCH v3 2/3] drm/amdgpu_dm: Rely on split out luminance calculation function
Luminance range calculation was split out into drm_edid.c and is now
part of edid parsing. Rely on values calculated during edid parsing and
use these for caps->aux_max_input_signal and caps->aux_min_input_signal.
v2: Use values calculated during edid parsing
Cc: Roman Li
Cc: Rodrigo Siqueira
Cc: Harry Wentland
Cc: Lyude Paul
Cc: Mika Kahola
Cc: Jani Nikula
Cc: Manasi Navare
Signed-off-by: Jouni Högander
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 35 +++
1 file changed, 4 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 3e83fed540e8..eb7abdeb8653 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2903,15 +2903,12 @@ static struct drm_mode_config_helper_funcs
amdgpu_dm_mode_config_helperfuncs = {
static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector)
{
- u32 max_avg, min_cll, max, min, q, r;
struct amdgpu_dm_backlight_caps *caps;
struct amdgpu_display_manager *dm;
struct drm_connector *conn_base;
struct amdgpu_device *adev;
struct dc_link *link = NULL;
- static const u8 pre_computed_values[] = {
- 50, 51, 52, 53, 55, 56, 57, 58, 59, 61, 62, 63, 65, 66, 68, 69,
- 71, 72, 74, 75, 77, 79, 81, 82, 84, 86, 88, 90, 92, 94, 96, 98};
+ struct drm_luminance_range_info *luminance_range;
int i;
if (!aconnector || !aconnector->dc_link)
@@ -2933,8 +2930,6 @@ static void update_connector_ext_caps(struct
amdgpu_dm_connector *aconnector)
caps = &dm->backlight_caps[i];
caps->ext_caps = &aconnector->dc_link->dpcd_sink_ext_caps;
caps->aux_support = false;
- max_avg = conn_base->hdr_sink_metadata.hdmi_type1.max_fall;
- min_cll = conn_base->hdr_sink_metadata.hdmi_type1.min_cll;
if (caps->ext_caps->bits.oled == 1 /*||
caps->ext_caps->bits.sdr_aux_backlight_control == 1 ||
@@ -2946,31 +2941,9 @@ static void update_connector_ext_caps(struct
amdgpu_dm_connector *aconnector)
else if (amdgpu_backlight == 1)
caps->aux_support = true;
- /* From the specification (CTA-861-G), for calculating the maximum
-* luminance we need to use:
-* Luminance = 50*2**(CV/32)
-* Where CV is a one-byte value.
-* For calculating this expression we may need float point precision;
-* to avoid this complexity level, we take advantage that CV is divided
-* by a constant. From the Euclids division algorithm, we know that CV
-* can be written as: CV = 32*q + r. Next, we replace CV in the
-* Luminance expression and get 50*(2**q)*(2**(r/32)), hence we just
-* need to pre-compute the value of r/32. For pre-computing the values
-* We just used the following Ruby line:
-* (0...32).each {|cv| puts (50*2**(cv/32.0)).round}
-* The results of the above expressions can be verified at
-* pre_computed_values.
-*/
- q = max_avg >> 5;
- r = max_avg % 32;
- max = (1 << q) * pre_computed_values[r];
-
- // min luminance: maxLum * (CV/255)^2 / 100
- q = DIV_ROUND_CLOSEST(min_cll, 255);
- min = max * DIV_ROUND_CLOSEST((q * q), 100);
-
- caps->aux_max_input_signal = max;
- caps->aux_min_input_signal = min;
+ luminance_range = &conn_base->display_info.luminance_range;
+ caps->aux_min_input_signal = luminance_range->min_luminance;
+ caps->aux_max_input_signal = luminance_range->max_luminance;
}
void amdgpu_dm_update_connector_after_detect(
--
2.25.1
[PATCH v3 1/3] drm: New function to get luminance range based on static hdr metadata
Split luminance min/max calculation using static hdr metadata from
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:update_connector_ext_caps
into drm/drm_edid.c and use it during edid parsing. Calculated range is
stored into connector->display_info->luminance_range.
Add new data structure (drm_luminance_range_inf) to store luminance range
calculated using data from EDID's static hdr metadata block. Add this new
struct as a part of drm_display_info struct.
v3: Squashed adding drm_luminance_range_info patch here
v2: Calculate range during edid parsing
Cc: Roman Li
Cc: Rodrigo Siqueira
Cc: Harry Wentland
Cc: Lyude Paul
Cc: Mika Kahola
Cc: Jani Nikula
Cc: Manasi Navare
Signed-off-by: Jouni Högander
Acked-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 52 -
include/drm/drm_connector.h | 21 +++
2 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bbc25e3b7220..90a5e26eafa8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5165,6 +5165,51 @@ static void fixup_detailed_cea_mode_clock(struct
drm_display_mode *mode)
mode->clock = clock;
}
+static void drm_calculate_luminance_range(struct drm_connector *connector)
+{
+ struct hdr_static_metadata *hdr_metadata =
&connector->hdr_sink_metadata.hdmi_type1;
+ struct drm_luminance_range_info *luminance_range =
+ &connector->display_info.luminance_range;
+ static const u8 pre_computed_values[] = {
+ 50, 51, 52, 53, 55, 56, 57, 58, 59, 61, 62, 63, 65, 66, 68, 69,
+ 71, 72, 74, 75, 77, 79, 81, 82, 84, 86, 88, 90, 92, 94, 96, 98
+ };
+ u32 max_avg, min_cll, max, min, q, r;
+
+ if (!(hdr_metadata->metadata_type & BIT(HDMI_STATIC_METADATA_TYPE1)))
+ return;
+
+ max_avg = hdr_metadata->max_fall;
+ min_cll = hdr_metadata->min_cll;
+
+ /*
+* From the specification (CTA-861-G), for calculating the maximum
+* luminance we need to use:
+* Luminance = 50*2**(CV/32)
+* Where CV is a one-byte value.
+* For calculating this expression we may need float point precision;
+* to avoid this complexity level, we take advantage that CV is divided
+* by a constant. From the Euclids division algorithm, we know that CV
+* can be written as: CV = 32*q + r. Next, we replace CV in the
+* Luminance expression and get 50*(2**q)*(2**(r/32)), hence we just
+* need to pre-compute the value of r/32. For pre-computing the values
+* We just used the following Ruby line:
+* (0...32).each {|cv| puts (50*2**(cv/32.0)).round}
+* The results of the above expressions can be verified at
+* pre_computed_values.
+*/
+ q = max_avg >> 5;
+ r = max_avg % 32;
+ max = (1 << q) * pre_computed_values[r];
+
+ /* min luminance: maxLum * (CV/255)^2 / 100 */
+ q = DIV_ROUND_CLOSEST(min_cll, 255);
+ min = max * DIV_ROUND_CLOSEST((q * q), 100);
+
+ luminance_range->min_luminance = min;
+ luminance_range->max_luminance = max;
+}
+
static uint8_t eotf_supported(const u8 *edid_ext)
{
return edid_ext[2] &
@@ -5196,8 +5241,12 @@ drm_parse_hdr_metadata_block(struct drm_connector
*connector, const u8 *db)
connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4];
if (len >= 5)
connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5];
- if (len >= 6)
+ if (len >= 6) {
connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6];
+
+ /* Calculate only when all values are available */
+ drm_calculate_luminance_range(connector);
+ }
}
static void
@@ -6101,6 +6150,7 @@ static void drm_reset_display_info(struct drm_connector
*connector)
info->non_desktop = 0;
memset(&info->monitor_range, 0, sizeof(info->monitor_range));
+ memset(&info->luminance_range, 0, sizeof(info->luminance_range));
info->mso_stream_count = 0;
info->mso_pixel_overlap = 0;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 2c6fa746efac..248206bbd975 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -323,6 +323,22 @@ struct drm_monitor_range_info {
u8 max_vfreq;
};
+/**
+ * struct drm_luminance_range_info - Panel's luminance range for
+ * &drm_display_info. Calculated using data in EDID
+ *
+ * This struct is used to store a luminance range supported by panel
+ * as calculated using data from EDID's static hdr metadata.
+ *
+ * @min_luminance: This is the min supported luminance value
+ *
+ * @max_luminance: This is the max supported luminance value
+ */
+struct drm_luminance_range_info {
+ u32 min_luminance;
+ u32 max_luminance;
+};
+
/**
* enum drm_privacy_screen_status - privacy screen
[PATCH v3 0/3] HDR aux backlight range calculation
This patch set splits out static hdr metadata backlight range parsing from gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c into gpu/drm/drm-edid.c as a new function. This new function is then used during edid parsing when HDR static metadata block parsing. Calculated values are stored in a new struct drm_luminance_range introduced into display_info. Amdgpu_dm.c and intel_dp_aux_backlight.c are using this new data. v3: Some clean-ups suggested by Jani Nikula v2: Calculate the range during edid parsing and store into display_info Cc: Roman Li Cc: Maarten Lankhorst Cc: Rodrigo Siqueira Cc: Harry Wentland Cc: Lyude Paul Cc: Mika Kahola Cc: Jani Nikula Cc: Manasi Navare Jouni Högander (3): drm: New function to get luminance range based on static hdr metadata drm/amdgpu_dm: Rely on split out luminance calculation function drm/i915: Use luminance range calculated during edid parsing .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 35 ++--- drivers/gpu/drm/drm_edid.c| 52 ++- .../drm/i915/display/intel_dp_aux_backlight.c | 15 +- include/drm/drm_connector.h | 21 4 files changed, 89 insertions(+), 34 deletions(-) -- 2.25.1
Re: [Intel-gfx] [PATCH 10/12] drm/i915/guc: Support larger contexts on newer hardware
On 19/07/2022 01:13, John Harrison wrote:
On 7/18/2022 05:35, Tvrtko Ursulin wrote:
On 13/07/2022 00:31, john.c.harri...@intel.com wrote:
From: Matthew Brost
The GuC needs a copy of a golden context for implementing watchdog
resets (aka media resets). This context is larger on newer platforms.
So adjust the size being allocated/copied accordingly.
What were the consequences of this being too small? Media watchdog
reset broken impacting userspace? Platforms? Do we have an IGT
testcase? Do we need a Fixes: tag? Copy stable?
Yes. Not sure if we have an IGT for the media watchdog. I recall writing
something a long time back but I don't think it ever got merged due to
push back that I don't recall right now. And no because it only affects
DG2 onwards which is still forceprobed.
Right, hm, I don't know if the MBD SKU promise for DG2 relies on force
probe removal or not. My impression certainly was that a bunch of uapi
we recently merged made people happy in that respect - that we satisfied
the commit to deliver that support with 5.19. Maybe I am wrong, or
perhaps to err on the side of safety you could add the right Fixes: tag
regardless? Pick some patch which enables GuC for DG2 if there isn't
anything better I guess. Or you could check with James.
Regards,
Tvrtko
John.
Regards,
Tvrtko
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
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 ba7541f3ca610..74cbe8eaf5318 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
@@ -464,7 +464,11 @@ static void fill_engine_enable_masks(struct
intel_gt *gt,
}
#define LR_HW_CONTEXT_SIZE (80 * sizeof(u32))
-#define LRC_SKIP_SIZE (LRC_PPHWSP_SZ * PAGE_SIZE + LR_HW_CONTEXT_SIZE)
+#define XEHP_LR_HW_CONTEXT_SIZE (96 * sizeof(u32))
+#define LR_HW_CONTEXT_SZ(i915) (GRAPHICS_VER_FULL(i915) >=
IP_VER(12, 50) ? \
+ XEHP_LR_HW_CONTEXT_SIZE : \
+ LR_HW_CONTEXT_SIZE)
+#define LRC_SKIP_SIZE(i915) (LRC_PPHWSP_SZ * PAGE_SIZE +
LR_HW_CONTEXT_SZ(i915))
static int guc_prep_golden_context(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
@@ -525,7 +529,7 @@ static int guc_prep_golden_context(struct
intel_guc *guc)
* on all engines).
*/
ads_blob_write(guc, ads.eng_state_size[guc_class],
- real_size - LRC_SKIP_SIZE);
+ real_size - LRC_SKIP_SIZE(gt->i915));
ads_blob_write(guc, ads.golden_context_lrca[guc_class],
addr_ggtt);
@@ -599,7 +603,7 @@ static void guc_init_golden_context(struct
intel_guc *guc)
}
GEM_BUG_ON(ads_blob_read(guc,
ads.eng_state_size[guc_class]) !=
- real_size - LRC_SKIP_SIZE);
+ real_size - LRC_SKIP_SIZE(gt->i915));
GEM_BUG_ON(ads_blob_read(guc,
ads.golden_context_lrca[guc_class]) != addr_ggtt);
addr_ggtt += alloc_size;
Re: [PATCH v1 0/11] drm: move dri1 drivers to drm/dri1/
On Mon, 18 Jul 2022 at 08:56, Thomas Zimmermann wrote:
>
> Hi
>
> Am 16.07.22 um 20:17 schrieb Sam Ravnborg:
> > While discussing the way forward for the via driver
> > Javier came up with the proposal to move all DRI1 drivers
> > to their own folder.
> >
> > The idea is to move the old DRI1 drivers so one do not
> > accidentally consider them modern drivers.
> >
> > This set of patches implements this idea.
> >
> > To prepare the move, DRIVER_LEGACY and CONFIG_DRM_LEGACY
> > are both renamed to *_DRI1. This makes it more obvious
> > that we are dealing with DRI1 drivers, as we have
> > a lot of other legacy support.
> >
> > The drivers continue to have their own sub-directory
> > so the driver files are not mixed with the core files
> > which are copied in the last commit.
> >
> > The DRI1 specific part of drm/Kconfig is likewise pulled
> > out and located in the dri1/ folder.
> >
> > Feedback welcome!
>
> To be honest, I still don't like this rename. Especially in the case of
> via, which has a KMS driver coming up. It will now have an include
> statement that crosses several levels in the directory hierarchy. And
> what about the other DRI1 drivers? If we ever get KMS drivers for those,
> do we want to move some header files back into their original locations?
> Patches 1 and 2 look reasonable to me. The other driver patches have
> basically zero upside IMHO.
Imo transitional drivers with both legacy dri1 and kms+gem support
made some sense 10+ years ago when all this infrastructure was still
being built. Now I really don't see much point.
For via imo make it a clean new driver, and copypaste anything from
the old one (like register headers) it needs. That will also make
review a ton easier I think. There has not been any actual via work,
just general refactoring, in that driver for 10+ years, so "bugfix
sharing" is really not an argument.
> In the case of moving the core files into dri1/, the resulting Makefile
> rule looks really ugly. I'd suggest to move all code into a separate
> file drm_dri1.c and be done with it. For something more elaborate,
> there could by drm_dri1.c and drm_dri1_helper.c, where the latter
> contains all DRI1 code that is only used by the drivers.
Ugly Makefile for dri1 might be a feature :-) But personally no stake
on this bikeshed.
-Daniel
>
> Best regards
> Thomsa
>
> >
> > Sam
> >
> > Sam Ravnborg (11):
> >drm: rename DRIVER_LEGACY to DRIVER_DRI1
> >drm: Rename CONFIG_DRM_LEGACY to CONFIG_DRM_DRI1
> >drm/tdfx: Move the tdfx driver to drm/dri1/
> >drm/r128: Move the r128 driver to drm/dri1/
> >drm/i810: Move the i810 driver to drm/dri1/
> >drm/mga: Move the mga driver to drm/dri1/
> >drm/sis: Move the sis driver to drm/dri1/
> >drm/via: Move the via driver to drm/dri1/
> >drm/savage: Move the savage driver to drm/dri1/
> >drm/dri1: Move Kconfig logic to drm/dri1
> >drm: Move dri1 core files to drm/dri1
> >
> > arch/powerpc/configs/pmac32_defconfig | 2 +-
> > arch/powerpc/configs/ppc6xx_defconfig | 2 +-
> > drivers/char/agp/Makefile | 2 +-
> > drivers/char/agp/agp.h | 2 +-
> > drivers/gpu/drm/Kconfig| 79
> > +-
> > drivers/gpu/drm/Makefile | 18 +++--
> > drivers/gpu/drm/dri1/Kconfig | 79
> > ++
> > drivers/gpu/drm/dri1/Makefile | 11 +++
> > drivers/gpu/drm/{ => dri1}/drm_agpsupport.c| 4 +-
> > drivers/gpu/drm/{ => dri1}/drm_bufs.c | 22 +++---
> > drivers/gpu/drm/{ => dri1}/drm_context.c | 24 +++
> > drivers/gpu/drm/{ => dri1}/drm_dma.c | 4 +-
> > drivers/gpu/drm/{ => dri1}/drm_hashtab.c | 0
> > drivers/gpu/drm/{ => dri1}/drm_irq.c | 6 +-
> > drivers/gpu/drm/{ => dri1}/drm_legacy_misc.c | 2 +-
> > drivers/gpu/drm/{ => dri1}/drm_lock.c | 6 +-
> > drivers/gpu/drm/{ => dri1}/drm_memory.c| 0
> > drivers/gpu/drm/{ => dri1}/drm_scatter.c | 6 +-
> > drivers/gpu/drm/{ => dri1}/drm_vm.c| 2 +-
> > drivers/gpu/drm/{ => dri1}/i810/Makefile | 0
> > drivers/gpu/drm/{ => dri1}/i810/i810_dma.c | 0
> > drivers/gpu/drm/{ => dri1}/i810/i810_drv.c | 2 +-
> > drivers/gpu/drm/{ => dri1}/i810/i810_drv.h | 0
> > drivers/gpu/drm/{ => dri1}/mga/Makefile| 0
> > drivers/gpu/drm/{ => dri1}/mga/mga_dma.c | 0
> > drivers/gpu/drm/{ => dri1}/mga/mga_drv.c | 2 +-
> > drivers/gpu/drm/{ => dri1}/mga/mga_drv.h | 0
> > drivers/gpu/drm/{ => dri1}/mga/mga_ioc32.c | 0
> > drivers/gpu/drm/{ => dri1}/mga/mga_irq.c | 0
> > drivers/gpu/drm/{ => dri1}/mga/mga_state.c | 0
> > drivers/gpu/drm/{ =>
Re: [Intel-gfx] [PATCH 02/12] drm/i915/guc: Don't call ring_is_idle in GuC submission
On 19/07/2022 01:09, John Harrison wrote:
On 7/18/2022 05:26, Tvrtko Ursulin wrote:
On 13/07/2022 00:31, john.c.harri...@intel.com wrote:
From: Matthew Brost
The engine registers really shouldn't be touched during GuC submission
as the GuC owns the registers. Don't call ring_is_idle and tie
Touch being just read and it is somehow harmful?
The registers are meaningless when GuC is controlling the submission.
The i915 driver has no knowledge of what context is or is not executing
on any given engine at any given time. So reading reading the ring
registers is incorrect - it can lead to bad assumptions about what state
the hardware is in.
Same is actually true with the execlists backend. The code in
ring_is_idle is not concerning itself with which context is running or
not. Just that the head/tail/ctl appear idle.
Problem/motivation appears to be on a higher than simply ring registers.
I am not claiming it makes sense with Guc and that it has to remain but
just suggesting for as a minimum clearer commit message.
intel_engine_is_idle strictly to the engine pm.
Strictly seems wrong - it is just ring_is_idle check that is replaced
and not the whole implementation of intel_engine_is_idle.
Because intel_engine_is_idle tied to the engine pm, retire requests
before checking intel_engines_are_idle in gt_drop_caches, and lastly
Why is re-ordering important? I at least can't understand it. I hope
it's not working around IGT failures.
If requests are physically completed but not retired then they will be
holding unnecessary PM references. So we need to flush those out before
checking for idle.
And if they are not as someone passes in DROP_RESET_ACTIVE? They will
not retire and code still enters intel_engines_are_idle so that has to
work, no? Something does not align for me still.
increase the timeout in gt_drop_caches for the intel_engines_are_idle
check.
Same here - why?
@Matthew Brost - do you recall which particular tests were hitting an
issue? I'm guessing gem_ctx_create? I believe that's the one that
creates and destroys thousands of contexts. That is much slower with GuC
(GuC communication required) than with execlists (i915 internal state
change only).
And if that is a logically separate change please split the patch up.
Regards,
Tvrtko
John.
Regards,
Tvrtko
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 13 +
drivers/gpu/drm/i915/i915_debugfs.c | 6 +++---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 283870c659911..959a7c92e8f4d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1602,6 +1602,9 @@ static bool ring_is_idle(struct intel_engine_cs
*engine)
{
bool idle = true;
+ /* GuC submission shouldn't access HEAD & TAIL via MMIO */
+ GEM_BUG_ON(intel_engine_uses_guc(engine));
+
if (I915_SELFTEST_ONLY(!engine->mmio_base))
return true;
@@ -1668,6 +1671,16 @@ bool intel_engine_is_idle(struct
intel_engine_cs *engine)
if (!i915_sched_engine_is_empty(engine->sched_engine))
return false;
+ /*
+ * We shouldn't touch engine registers with GuC submission as
the GuC
+ * owns the registers. Let's tie the idle to engine pm, at worst
this
+ * function sometimes will falsely report non-idle when idle
during the
+ * delay to retire requests or with virtual engines and a request
+ * running on another instance within the same class / submit mask.
+ */
+ if (intel_engine_uses_guc(engine))
+ return false;
+
/* Ring stopped? */
return ring_is_idle(engine);
}
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 94e5c29d2ee3a..ee5334840e9cb 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -654,13 +654,13 @@ gt_drop_caches(struct intel_gt *gt, u64 val)
{
int ret;
+ if (val & DROP_RETIRE || val & DROP_RESET_ACTIVE)
+ intel_gt_retire_requests(gt);
+
if (val & DROP_RESET_ACTIVE &&
wait_for(intel_engines_are_idle(gt),
I915_IDLE_ENGINES_TIMEOUT))
intel_gt_set_wedged(gt);
- if (val & DROP_RETIRE)
- intel_gt_retire_requests(gt);
-
if (val & (DROP_IDLE | DROP_ACTIVE)) {
ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
if (ret)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index c22f29c3faa0e..53c7474dde495 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -278,7 +278,7 @@ struct i915_gem_mm {
u32 shrink_count;
};
-#define I915_IDLE_ENGINES_TIMEOUT (200) /* in ms */
+#define I915_IDLE_ENGINES_TIMEOUT (500) /* in ms */
unsigned long i915_fence_c
[PATCH] drm/vmwgfx: clean up some error pointer checking
The vmw_user_bo_noref_lookup() function cannot return NULL. If it
could, then this function would return PTR_ERR(NULL) which is success.
Returning success without initializing "*vmw_bo_p = vmw_bo;" would
lead to an uninitialized variable bug in the caller. Smatch complains
about this:
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c:1177 vmw_translate_mob_ptr() warn:
passing zero to 'PTR_ERR'
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c:1314 vmw_cmd_dx_bind_query() error:
uninitialized symbol 'vmw_bo'.
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index d49de4905efa..f085dbd4736d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -1172,7 +1172,7 @@ static int vmw_translate_mob_ptr(struct vmw_private
*dev_priv,
vmw_validation_preload_bo(sw_context->ctx);
vmw_bo = vmw_user_bo_noref_lookup(sw_context->filp, handle);
- if (IS_ERR_OR_NULL(vmw_bo)) {
+ if (IS_ERR(vmw_bo)) {
VMW_DEBUG_USER("Could not find or use MOB buffer.\n");
return PTR_ERR(vmw_bo);
}
@@ -1226,7 +1226,7 @@ static int vmw_translate_guest_ptr(struct vmw_private
*dev_priv,
vmw_validation_preload_bo(sw_context->ctx);
vmw_bo = vmw_user_bo_noref_lookup(sw_context->filp, handle);
- if (IS_ERR_OR_NULL(vmw_bo)) {
+ if (IS_ERR(vmw_bo)) {
VMW_DEBUG_USER("Could not find or use GMR region.\n");
return PTR_ERR(vmw_bo);
}
--
2.35.1
Re: [Intel-gfx] [PATCH 01/12] drm/i915: Remove bogus GEM_BUG_ON in unpark
On 19/07/2022 01:05, John Harrison wrote: On 7/18/2022 05:15, Tvrtko Ursulin wrote: On 13/07/2022 00:31, john.c.harri...@intel.com wrote: From: Matthew Brost Remove bogus GEM_BUG_ON which compared kernel context timeline seqno to seqno in memory on engine PM unpark. If a GT reset occurred these values might not match as a kernel context could be skipped. This bug was hidden by always switching to a kernel context on park (execlists requirement). Reset of the kernel context? Under which circumstances does that happen? As per description, the issue is with full GT reset. It is unclear if the claim is this to be a general problem or the assert is only invalid with the GuC. Lack of a CI reported issue suggests it is not a generic problem? Currently it is not an issue because we always switch to the kernel context because that's how execlists works and the entire driver is fundamentally based on execlist operation. When we stop using the kernel context as a (non-functional) barrier when using GuC submission, then you would see an issue without this fix. Issue is with GuC, GuC and full reset, or with full reset regardless of the backend? If issue is only with GuC patch should have drm/i915/guc prefix as minimum. But if it actually only becomes a problem when GuC backend stops parking with the kernel context when I think the whole unpark code should be refactored in a cleaner way than just removing the one assert. Otherwise what is the point of leaving everything else in there? Or if the issue is backend agnostic, *if* full reset happens to hit during parking, then it is different. Wouldn't that be a race with parking and reset which probably shouldn't happen to start with. Regards, Tvrtko John. Regards, Tvrtko Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_engine_pm.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index b0a4a2dbe3ee9..fb3e1599d04ec 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -68,8 +68,6 @@ static int __engine_unpark(struct intel_wakeref *wf) ce->timeline->seqno, READ_ONCE(*ce->timeline->hwsp_seqno), ce->ring->emit); - GEM_BUG_ON(ce->timeline->seqno != - READ_ONCE(*ce->timeline->hwsp_seqno)); } if (engine->unpark)
Re: [PATCH v1 0/6] Move all drivers to a common dma-buf locking convention
On Fri, Jul 15, 2022 at 9:53 AM Dmitry Osipenko wrote: > > Hello, > > This series moves all drivers to a dynamic dma-buf locking specification. > From now on all dma-buf importers are made responsible for holding > dma-buf's reservation lock around all operations performed over dma-bufs. > This common locking convention allows us to utilize reservation lock more > broadly around kernel without fearing of potential dead locks. > > This patchset passes all i915 selftests. It was also tested using VirtIO, > Panfrost, Lima and Tegra drivers. I tested cases of display+GPU, > display+V4L and GPU+V4L dma-buf sharing, which covers majority of kernel > drivers since rest of the drivers share same or similar code paths. > > This is a continuation of [1] where Christian König asked to factor out > the dma-buf locking changes into separate series. > > [1] > https://lore.kernel.org/dri-devel/20220526235040.678984-1-dmitry.osipe...@collabora.com/ > > Dmitry Osipenko (6): > dma-buf: Add _unlocked postfix to function names > drm/gem: Take reservation lock for vmap/vunmap operations > dma-buf: Move all dma-bufs to dynamic locking specification > dma-buf: Acquire wait-wound context on attachment > media: videobuf2: Stop using internal dma-buf lock > dma-buf: Remove internal lock > > drivers/dma-buf/dma-buf.c | 198 +++--- > drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +- > drivers/gpu/drm/armada/armada_gem.c | 14 +- > drivers/gpu/drm/drm_client.c | 4 +- > drivers/gpu/drm/drm_gem.c | 28 +++ > drivers/gpu/drm/drm_gem_cma_helper.c | 6 +- > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +- > drivers/gpu/drm/drm_gem_shmem_helper.c| 6 +- > drivers/gpu/drm/drm_prime.c | 12 +- > drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 6 +- > drivers/gpu/drm/exynos/exynos_drm_gem.c | 2 +- > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c| 20 +- > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 2 +- > drivers/gpu/drm/i915/gem/i915_gem_object.h| 6 +- > .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 20 +- > drivers/gpu/drm/i915/i915_gem_evict.c | 2 +- > drivers/gpu/drm/i915/i915_gem_ww.c| 26 ++- > drivers/gpu/drm/i915/i915_gem_ww.h| 15 +- > drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 8 +- > drivers/gpu/drm/qxl/qxl_object.c | 17 +- > drivers/gpu/drm/qxl/qxl_prime.c | 4 +- > drivers/gpu/drm/tegra/gem.c | 27 +-- > drivers/infiniband/core/umem_dmabuf.c | 11 +- > .../common/videobuf2/videobuf2-dma-contig.c | 26 +-- > .../media/common/videobuf2/videobuf2-dma-sg.c | 23 +- > .../common/videobuf2/videobuf2-vmalloc.c | 17 +- For the videobuf2 changes: Acked-by: Tomasz Figa Best regards, Tomasz
Re: [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3)
On 18/07/2022 23:30, Caleb Connolly wrote:
> From: Amit Pundir
>
> This adds an initial dts for the Blueline (Pixel 3). Supported
> functionality includes display, Debug UART, UFS, USB-C (peripheral), WiFi,
> Bluetooth and modem.
>
Thank you for your patch. There is something to discuss/improve.
(...)
> + volume-keys {
> + compatible = "gpio-keys";
> + label = "Volume keys";
> + autorepeat;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&volume_up_gpio>;
> +
> + vol-up {
key-vol-up
(DT schema requires it now)
> + label = "Volume Up";
> + linux,code = ;
> + gpios = <&pm8998_gpio 6 GPIO_ACTIVE_LOW>;
> + debounce-interval = <15>;
> + };
> + };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + mpss_region: memory@8e00 {
> + reg = <0 0x8e00 0 0x980>;
> + no-map;
> + };
> +
> + venus_mem: venus@9780 {
> + reg = <0 0x9780 0 0x50>;
> + no-map;
> + };
> +
> + cdsp_mem: cdsp-mem@97D0 {
> + reg = <0 0x97D0 0 0x80>;
> + no-map;
> + };
> +
> + mba_region: mba@9850 {
> + reg = <0 0x9850 0 0x20>;
> + no-map;
> + };
> +
> + slpi_mem: slpi@9870 {
> + reg = <0 0x9870 0 0x140>;
> + no-map;
> + };
> +
> + spss_mem: spss@99B0 {
> + reg = <0 0x99B0 0 0x10>;
> + no-map;
> + };
> +
> + /* rmtfs lower guard */
> + memory@f270 {
> + reg = <0 0xf270 0 0x1000>;
> + no-map;
> + };
> +
> + rmtfs_mem: memory@f2701000 {
> + compatible = "qcom,rmtfs-mem";
> + reg = <0 0xf2701000 0 0x20>;
> + no-map;
> +
> + qcom,client-id = <1>;
> + qcom,vmid = <15>;
> + };
> +
> + /* rmtfs upper guard */
> + memory@f2901000 {
> + reg = <0 0xf2901000 0 0x1000>;
> + no-map;
> + };
> + };
> +
> + vph_pwr: vph-pwr-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vph_pwr";
> + regulator-min-microvolt = <370>;
> + regulator-max-microvolt = <370>;
> + };
> +
> + vreg_s4a_1p8: vreg-s4a-1p8 {
Please use consistent naming, so if previous was "xxx-regulator", keep
similar pattern here.
> + compatible = "regulator-fixed";
> + regulator-name = "vreg_s4a_1p8";
> +
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> + regulator-always-on;
> + regulator-boot-on;
> +
> + vin-supply = <&vph_pwr>;
> + };
> +};
> +
> +&adsp_pas {
(...)
> +
> +&pm8998_gpio {
> + volume_up_gpio: vol-up-active {
The bindings require node name to finish with "-state"
> + pins = "gpio6";
> + function = "normal";
> + input-enable;
> + bias-pull-up;
> + qcom,drive-strength = <0>;
> + };
> +
> + panel_pmgpio_pins: panel-pmgpio-active {
Ditto.
> + pins = "gpio2", "gpio5";
> + function = "normal";
> + input-enable;
> + bias-disable;
> + power-source = <0>;
> + };
> +};
> +
> +&pm8998_pon {
> + resin {
> + compatible = "qcom,pm8941-resin";
> + interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> + debounce = <15625>;
> + bias-pull-up;
> + linux,code = ;
> + };
> +};
> +
Best regards,
Krzysztof
Re: [PATCH v9 06/14] mm/gup: migrate device coherent pages when pinning instead of failing
On 18.07.22 22:32, Andrew Morton wrote: > On Mon, 18 Jul 2022 12:56:29 +0200 David Hildenbrand wrote: > >>> /* >>> * Try to move out any movable page before pinning the range. >>> */ >>> @@ -1919,7 +1948,8 @@ static long check_and_migrate_movable_pages(unsigned >>> long nr_pages, >>> folio_nr_pages(folio)); >>> } >>> >>> - if (!list_empty(&movable_page_list) || isolation_error_count) >>> + if (!list_empty(&movable_page_list) || isolation_error_count >>> + || coherent_pages) >> >> The common style is to >> >> a) add the || to the end of the previous line >> b) indent such the we have a nice-to-read alignment >> >> if (!list_empty(&movable_page_list) || isolation_error_count || >> coherent_pages) >> > > I missed that. This series is now in mm-stable so any fix will need to > be a standalone followup patch, please. > >> Apart from that lgtm. >> >> Reviewed-by: David Hildenbrand > > And your reviewed-by's will be lost. Stupid git. I know, I already raised my concerns regarding the new workflow, so I won't repeat that. I can understand (too some degree) that we don't want code to change just before the new merge window opens. But I do wonder if we really don't even want to do subject+description updates. Sure, the commit IDs will change. Who cares? Anyhow, it is what it is. -- Thanks, David / dhildenb
Re: [PATCH 1/9] drm/panel/panel-sitronix-st7701: Make DSI mode flags common to ST7701
On Sun, Jul 10, 2022 at 9:44 PM Marek Vasut wrote: > The ST7701 and ST7701S are TFT matrix drivers with integrated multi > protocol decoder capable of DSI/DPI/SPI input and 480x360...864 line > TFT matrix output. Currently the only supported input is DSI. > > The protocol decoder is separate from the TFT matrix driver and is > always capable of handling all of DSI non-burst mode with sync pulses > or sync events as well as DSI burst mode. > > Move the DSI mode configuration from TFT matrix driver properties to > common ST7701 code, because this is common to all TFT matrices. > > Signed-off-by: Marek Vasut > Cc: Guido Günther > Cc: Jagan Teki > Cc: Laurent Pinchart > Cc: Linus Walleij > Cc: Sam Ravnborg > Cc: Thierry Reding All 9 patches applied and pushed for drm-misc-next. Yours, Linus Walleij
Re: [PATCH 1/4] Documentation: dt-bindings: arm: qcom: add google,blueline
On 18/07/2022 23:30, Caleb Connolly wrote: > Document the bindings for the Pixel 3 > > Based on > https://lore.kernel.org/all/20220521164550.91115-7-krzysztof.kozlow...@linaro.org/ Thanks for mention dependency. However this should not go to the final commit, thus please put such references after '---' marker. With that change: Acked-by: Krzysztof Kozlowski > > Signed-off-by: Caleb Connolly > --- Best regards, Krzysztof
Re: [PATCH v1 0/11] drm: move dri1 drivers to drm/dri1/
On 7/19/22 10:05, Thomas Zimmermann wrote: >>> [...] >> >> I will update the series with the following: >> - Drop drm/dri1/ >> - Keep the CONFIG_DRM_* change and keep the DRIVER_DRI1 change >> - All config options for DRI1 drivers will get a CONFIG_DRM_DRI1_* >>prefix >> - Convert at least some of the drivers to single file drivers named >>foo_dri1. >> - I may add SPDX for licenses when I am touching the files >> - I may try to concatenate all dri1 specific drm core files to drm_dri1. >>It is easy to do but I will take a look at the result before posting >>anything. >> >> When I have posted the above let's see what we all agree on. >> May take a couple of days before I get back to it. > > Sounds like a plan to me. > Sounds good to me as well. -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
[PATCH] docs: driver-api: firmware: add driver firmware guidelines. (v2)
From: Dave Airlie A recent snafu where Intel ignored upstream feedback on a firmware change, led to a late rc6 fix being required. In order to avoid this in the future we should document some expectations around linux-firmware. I was originally going to write this for drm, but it seems quite generic advice. v2: rewritten with suggestions from Thorsten Leemhuis. Acked-by: Luis Chamberlain Acked-by: Rodrigo Vivi Signed-off-by: Dave Airlie --- Documentation/driver-api/firmware/core.rst| 1 + .../firmware/firmware-usage-guidelines.rst| 34 +++ 2 files changed, 35 insertions(+) create mode 100644 Documentation/driver-api/firmware/firmware-usage-guidelines.rst diff --git a/Documentation/driver-api/firmware/core.rst b/Documentation/driver-api/firmware/core.rst index 1d1688cbc078..803cd574bbd7 100644 --- a/Documentation/driver-api/firmware/core.rst +++ b/Documentation/driver-api/firmware/core.rst @@ -13,4 +13,5 @@ documents these features. direct-fs-lookup fallback-mechanisms lookup-order + firmware-usage-guidelines diff --git a/Documentation/driver-api/firmware/firmware-usage-guidelines.rst b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst new file mode 100644 index ..34d2412e78c6 --- /dev/null +++ b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst @@ -0,0 +1,34 @@ +=== +Firmware Guidelines +=== + +Drivers that use firmware from linux-firmware should attempt to follow +the rules in this guide. + +* Firmware should be versioned with at least a major/minor version. It + is suggested that the firmware files in linux-firmware be named with + some device specific name, and just the major version. The + major/minor/patch versions should be stored in a header in the + firmware file for the driver to detect any non-ABI fixes/issues. The + firmware files in linux-firmware should be overwritten with the newest + compatible major version. Newer major version firmware should remain + compatible with all kernels that load that major number. + +* Users should *not* have to install newer firmware to use existing + hardware when they install a newer kernel. If the hardware isn't + enabled by default or under development, this can be ignored, until + the first kernel release that enables that hardware. This means no + major version bumps without the kernel retaining backwards + compatibility for the older major versions. Minor version bumps + should not introduce new features that newer kernels depend on + non-optionally. + +* If a security fix needs lockstep firmware and kernel fixes in order to + be successful, then all supported major versions in the linux-firmware + repo should be updated with the security fix, and the kernel patches + should detect if the firmware is new enough to declare if the security + issue is fixed. All communications around security fixes should point + at both the firmware and kernel fixes. If a security fix requires + deprecating old major versions, then this should only be done as a + last option, and be stated clearly in all communications. + -- 2.36.1 -- ___ Dri-devel mailing list dri-de...@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] docs: driver-api: firmware: add driver firmware guidelines.
> > +* Firmware should be versioned with at least a major/minor version. It > > + is suggested that the firmware files in linux-firmware be named with > > + some device specific name, and just the major version. The > > + major/minor/patch versions should be stored in a header in the > > + firmware file for the driver to detect any non-ABI fixes/issues. The > > + firmware files in linux-firmware should be overwritten with the newest > > + compatible major version. Newer major version firmware should remain > > + compatible with all kernels that load that major number. > > would symbolic links be acceptable in the linux-firmware.git where > the _.bin is a sym link to _..bin > > or having the _.bin really to be the overwritten every minor > update? I don't think providing multiple minor versions of fw in linux-firmware is that interesting. Like if the major is the same, surely you always want the newer ones. As long as the ABI doesn't break. Otherwise we are just wasting disk space with fws nobody will be using. Dave. -- ___ Dri-devel mailing list dri-de...@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] docs: driver-api: firmware: add driver firmware guidelines.
On Tue, 19 Jul 2022 at 08:04, Jakub Kicinski wrote: > > On Mon, 18 Jul 2022 11:33:11 +0200 Thorsten Leemhuis wrote: > > > If the hardware isn't > > > + enabled by default or under development, > > > > Wondering if it might be better to drop the "or under development", as > > the "enabled by default" is the main part afaics. Maybe something like > > "If support for the hardware is normally inactive (e.g. has to be > > enabled manually by a kernel parameter)" would be better anyway. > > It's a tricky one, I'd say something like you can break the FW ABI > "until HW becomes available for public consumption" or such. > I'm guessing what we're after is letting people break the compatibility > in early stages of the product development cycles. Pre-silicon and > bring up, but not after there are products on the market? I'll stick with enabled by default I think, "public consumption" invites efforts to describe corners of the cloud or other places where hw has shipped but is not technically "public", Dave. -- ___ Dri-devel mailing list dri-de...@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH v1 0/11] drm: move dri1 drivers to drm/dri1/
Hi Sam Am 19.07.22 um 09:55 schrieb Sam Ravnborg: Hi Javier, Thomas, On Mon, Jul 18, 2022 at 02:18:13PM +0200, Javier Martinez Canillas wrote: On 7/18/22 12:50, Thomas Zimmermann wrote: [...] To be honest, I still don't like this rename. Especially in the case of via, which has a KMS driver coming up. It will now have an include statement that crosses several levels in the directory hierarchy. And That could be avoided by moving drivers/gpu/drm/via/via_3d_reg.h and other header files to include/drm/via_3d_reg.h for example. Other drivers (i.e: i915) do the same for headers that are shared across different subsystems. what about the other DRI1 drivers? If we ever get KMS drivers for those, do we want to move some header files back into their original locations? I believe for these we could also move them to include/drm/ if needed. That pollutes these shared directories at the expense of everyone else. If anything, we want to move files out of the shared include paths. Yes, every option has a different set of trade offs. It would make sense to me if we'd have two distinct drivers. But here, the new and the old driver is really just one DRM driver with badly organized source code. I see. I haven't looked at the via drivers in detail. Patches 1 and 2 look reasonable to me. The other driver patches have basically zero upside IMHO. I disagree with the zero upside. It may be that the trade offs are not worth it but there are upsides of having all DRI1 drivers and core DRI1 bits in the same place. It makes grep-ing and reading files easier if one doesn't care about legacy DRI1 drivers. The grep-ability is a minor point. It does come up, but is usually sorted out easily. If we want to improve grep output, we should consider applying Sam's via_dri1 changes to all DRI1 drivers. So we'd end up with a single mga_dri1.c, tdfx_dri1.c, savage_dri1.c and so on. If the core/helper code is also in a _dri1-named source file, grep runs can filter out those filenames. Having everything with a _dri1 suffix would be an improvement I agree and if that's the consensus then I'm OK with that approach as well. [...] I will update the series with the following: - Drop drm/dri1/ - Keep the CONFIG_DRM_* change and keep the DRIVER_DRI1 change - All config options for DRI1 drivers will get a CONFIG_DRM_DRI1_* prefix - Convert at least some of the drivers to single file drivers named foo_dri1. - I may add SPDX for licenses when I am touching the files - I may try to concatenate all dri1 specific drm core files to drm_dri1. It is easy to do but I will take a look at the result before posting anything. When I have posted the above let's see what we all agree on. May take a couple of days before I get back to it. Sounds like a plan to me. Best regards Thomas Sam -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
Hi Caleb, On Tue, Jul 19, 2022 at 08:10:18AM +0200, Sam Ravnborg wrote: > Hi Caleb, > > On Mon, Jul 18, 2022 at 10:30:50PM +0100, Caleb Connolly wrote: > > From: Sumit Semwal > > > > LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel. > A few things to improve to this binding. > > Sam > > > > Signed-off-by: Vinod Koul > > Signed-off-by: Sumit Semwal > > [caleb: convert to yaml] > > Signed-off-by: Caleb Connolly > > --- > > .../bindings/display/panel/lg,43408.yaml | 41 +++ > > .../display/panel/panel-simple-dsi.yaml | 2 + > > 2 files changed, 43 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/panel/lg,43408.yaml > > > > diff --git a/Documentation/devicetree/bindings/display/panel/lg,43408.yaml > > b/Documentation/devicetree/bindings/display/panel/lg,43408.yaml > > new file mode 100644 > > index ..0529a3aa2692 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/lg,43408.yaml > > @@ -0,0 +1,41 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/panel/panel-lvds.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: LG SW43408 1080x2160 DSI panel > > + > > +maintainers: > > + - Caleb Connolly > > + > > +description: | > > + This panel is used on the Pixel 3, it is a 60hz OLED panel which > > + required DSC (Display Stream Compression) and has rounded corners. > > + > > +allOf: > > + - $ref: panel-common.yaml# > > + > > +properties: > > + compatible: > > +items: > > + - const: lg,sw43408 > > + > > + vddi-supply: true > > + vpnl-supply: true > > + reset-gpios: true > > + > > + backlight: false > > + power-supply: false > No need to say anything is false, this is covered by the statement below. > Also, the driver uses backlight, so it should be true? The driver do not use backlight from the DT so disregard the last comment. Sam
Re: [PATCH v1 0/11] drm: move dri1 drivers to drm/dri1/
Hi Javier, Thomas, On Mon, Jul 18, 2022 at 02:18:13PM +0200, Javier Martinez Canillas wrote: > On 7/18/22 12:50, Thomas Zimmermann wrote: > > [...] > > >>> To be honest, I still don't like this rename. Especially in the case of > >>> via, which has a KMS driver coming up. It will now have an include > >>> statement that crosses several levels in the directory hierarchy. And > >> > >> That could be avoided by moving drivers/gpu/drm/via/via_3d_reg.h and other > >> header files to include/drm/via_3d_reg.h for example. Other drivers (i.e: > >> i915) do the same for headers that are shared across different subsystems. > >> > >>> what about the other DRI1 drivers? If we ever get KMS drivers for those, > >>> do we want to move some header files back into their original locations? > >> > >> I believe for these we could also move them to include/drm/ if needed. > > > > That pollutes these shared directories at the expense of everyone else. > > If anything, we want to move files out of the shared include paths. > > > > Yes, every option has a different set of trade offs. > > > It would make sense to me if we'd have two distinct drivers. But here, > > the new and the old driver is really just one DRM driver with badly > > organized source code. > > > > I see. I haven't looked at the via drivers in detail. > > >> > >>> Patches 1 and 2 look reasonable to me. The other driver patches have > >>> basically zero upside IMHO. > >>> > >> > >> I disagree with the zero upside. It may be that the trade offs are not > >> worth it but there are upsides of having all DRI1 drivers and core DRI1 > >> bits in the same place. It makes grep-ing and reading files easier if > >> one doesn't care about legacy DRI1 drivers. > > > > The grep-ability is a minor point. It does come up, but is usually > > sorted out easily. > > > > If we want to improve grep output, we should consider applying Sam's > > via_dri1 changes to all DRI1 drivers. So we'd end up with a single > > mga_dri1.c, tdfx_dri1.c, savage_dri1.c and so on. If the core/helper > > code is also in a _dri1-named source file, grep runs can filter out > > those filenames. > > > > Having everything with a _dri1 suffix would be an improvement I agree > and if that's the consensus then I'm OK with that approach as well. > > [...] I will update the series with the following: - Drop drm/dri1/ - Keep the CONFIG_DRM_* change and keep the DRIVER_DRI1 change - All config options for DRI1 drivers will get a CONFIG_DRM_DRI1_* prefix - Convert at least some of the drivers to single file drivers named foo_dri1. - I may add SPDX for licenses when I am touching the files - I may try to concatenate all dri1 specific drm core files to drm_dri1. It is easy to do but I will take a look at the result before posting anything. When I have posted the above let's see what we all agree on. May take a couple of days before I get back to it. Sam
Re: [PATCH 4/4] drm: panel: Add lg sw43408 panel driver
Hi Caleb,
On Mon, Jul 18, 2022 at 10:30:51PM +0100, Caleb Connolly wrote:
> From: Sumit Semwal
>
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel, used in some Pixel3
> phones.
Thanks for submitting this.
When reading the code it is obvious that this was based on an older
panel and there is a few things to improve to get it on on same level as
the other panel drivers today.
I will comment in the following.
Sam
>
> Whatever init sequence we have for this panel isn't capable of
> initialising it completely, toggling the reset gpio ever causes the
> panel to die. Until this is resolved we avoid resetting the panel. The
> disable/unprepare functions only put the panel to sleep mode and
> disable the backlight.
>
> Signed-off-by: Sumit Semwal
> [vinod: Add DSC support]
> Signed-off-by: Vinod Koul
> [caleb: cleanup and support turning off the panel]
> Signed-off-by: Caleb Connolly
> ---
> MAINTAINERS | 8 +
> drivers/gpu/drm/panel/Kconfig| 11 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-lg-sw43408.c | 586 +++
> 4 files changed, 606 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-lg-sw43408.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f679152bdbad..8a2b954ad140 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6376,6 +6376,14 @@ S: Orphan / Obsolete
> F: drivers/gpu/drm/i810/
> F: include/uapi/drm/i810_drm.h
>
> +DRM DRIVER FOR LG SW43408 PANELS
> +M: Sumit Semwal
> +M: Caleb Connolly
> +S: Maintained
m +T: git git://anongit.freedesktop.org/drm/drm-misc
> +F: Documentation/devicetree/bindings/display/panel/lg,sw43408-panel.txt
> +F: drivers/gpu/drm/panel/panel-lg-sw43408.c
> +
> DRM DRIVER FOR LVDS PANELS
> M: Laurent Pinchart
> L: dri-devel@lists.freedesktop.org
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 38799effd00a..706b112794b9 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -256,6 +256,17 @@ config DRM_PANEL_LEADTEK_LTK500HD1829
> 24 bit RGB per pixel. It provides a MIPI DSI interface to
> the host and has a built-in LED backlight.
>
> +config DRM_PANEL_LG_SW43408
> + tristate "LG SW43408 panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to enable support for LG sw43408 panel.
> + The panel has a 1080x2160 resolution and uses
> + 24 bit RGB per pixel. It provides a MIPI DSI interface to
> + the host and has a built-in LED backlight.
> +
Hrmpf, the DRM_PANEL_SAMSUNG_LD9040 config entry is not placed in
alphabetic order. Can you move it or put you config optiosn with the
other LG config options?
> config DRM_PANEL_SAMSUNG_LD9040
> tristate "Samsung LD9040 RGB/SPI panel"
> depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 42a7ab54234b..ba26a69b74e7 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) +=
> panel-leadtek-ltk050h3146w.o
> obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
> obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
> obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> +obj-$(CONFIG_DRM_PANEL_LG_SW43408) += panel-lg-sw43408.o
> obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
> obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
> obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
> diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c
> b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> new file mode 100644
> index ..c7b8ec7b970d
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Linaro Ltd
Update to 2022?
> + * Author: Sumit Semwal
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +struct panel_cmd {
> + size_t len;
> + const char *data;
> +};
> +
> +#define _INIT_CMD(...) \
> + {\
> + .len = sizeof((char[]){ __VA_ARGS__ }), .data = (char[]) \
> + {\
> + __VA_ARGS__ \
> + }\
> + }
Other panel drivers do:
#define _INIT_DCS_CMD(...) { \
.type = INIT_DCS_CMD, \
.len = sizeof((char[]){__V
RE: [Intel-gfx] [PATCH v2 01/21] drm/i915/gt: Ignore TLB invalidations on idle engines
From: Tvrtko Ursulin > Sent: 19 July 2022 08:25 ... > > It's not only the TLB flushes that cause grief. > > > > There is a loop that forces a write-back of all the frame buffer pages. > > With a large display and some cpu (like my Ivy bridge one) that > > takes long enough with pre-emption disabled that wakeup of RT processes > > (and any pinned to the cpu) takes far longer than one might have > > wished for. > > > > Since some X servers request a flush every few seconds this makes > > the system unusable for some workloads. > > Ok TLB invalidations as discussed in this patch does not apply to > Ivybridge. But what is the write back loop you mention which is causing > you grief? What size frame buffers are we talking about here? If they > don't fit in the mappable area recently we merged a patch* which > improves things in that situation but not sure you are hitting exactly that. I found the old email: What I've found is that the Intel i915 graphics driver uses the 'events_unbound' kernel worker thread to periodically execute drm_cflush_sg(). (see https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/drm_cache.c) I'm guessing this is to ensure that any writes to graphics memory become visible is a semi-timely manner. This loop takes about 1us per iteration split fairly evenly between whatever is in for_each_sg_page() and drm_cflush_page(). With a 2560x1440 display the loop count is 3600 (4 bytes/pixel) and the whole function takes around 3.3ms. IIRC the first few page flushes are quick (I bet they go into a fifo) and then they all get slow. The flushes are actually requested from userspace. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: linux-next: manual merge of the drm tree with the drm-misc-fixes tree
Hi Stephen,
On Mon, Jul 18, 2022 at 1:49 AM Stephen Rothwell wrote:
> On Mon, 11 Jul 2022 10:05:45 +0200 Christian König
> wrote:
> > Am 11.07.22 um 04:47 schrieb Stephen Rothwell:
> > >
> > > Today's linux-next merge of the drm tree got a conflict in:
> > >
> > >drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > >
> > > between commit:
> > >
> > >925b6e59138c ("Revert "drm/amdgpu: add drm buddy support to amdgpu"")
> > >
> > > from the drm-misc-fixes tree and commit:
> > >
> > >5e3f1e7729ec ("drm/amdgpu: fix start calculation in
> > > amdgpu_vram_mgr_new")
> > >
> > > from the drm tree.
> > >
> > > This is a mess :-( I have just reverted the above revert before mergin
> > > the drm tree for today, please fix it up.
> >
> > Sorry for the noise, the patch "5e3f1e7729ec ("drm/amdgpu: fix start
> > calculation in amdgpu_vram_mgr_new")" and another one is going to be
> > reverted from the drm tree as well.
> >
> > It's just that -fixes patches where faster than -next patches.
>
> Here we are a week later, -rc7 has been released and as far as I can
> tell (though I may have missed it), this is still a problem :-(
>
> I am still reverting 925b6e59138c (which is now in Linus' tree).
Thanks for the hint! After reverting that commit, drm-next (sort of[1])
merges cleanly into upstream again.
[1] There's still a small conflict due to the removal of
force_dpms_off, cfr. the difference between commits
3283c83eb6fcfbda and cc79950bf0904f58 ("drm/amd/display:
Ensure valid event timestamp for cursor-only commits") in
v5.19-rc7 resp. drm-next.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
