Re: [PATCH v2 1/2] drm: Add GPU reset sysfs event
Hi, Maybe it would be a good idea to state the intended use-case in the commit message? And explain why the current (driver-specific IIRC) APIs aren't enough? Since this introduces new uAPI, can you point to a user-space patch which uses the new uAPI? See this link for more info on DRM user-space requirements: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements Please use drm_dbg_* and drm_warn_* helpers instead of DRM_DEBUG and DRM_WARN. This allows identifying on which device the uevent happens. On Tuesday, March 8th, 2022 at 19:04, Shashank Sharma wrote: > +void drm_sysfs_reset_event(struct drm_device *dev, struct drm_reset_event > *reset_info) reset_info can be const.
Re: [Intel-gfx] [PATCH] drm/i915/gtt: reduce overzealous alignment constraints for GGTT
|Acked-by: Nirmoy Das |
On 03/03/2022 11:02, Matthew Auld wrote:
Currently this will enforce both 2M alignment and padding for any LMEM
pages inserted into the GGTT. However, this was only meant to be applied
to the compact-pt layout with the ppGTT. For the GGTT we can reduce the
alignment and padding to 64K.
Bspec: 45015
Fixes: 87bd701ee268 ("drm/i915: enforce min GTT alignment for discrete cards")
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Robert Beckett
Cc: Ramalingam C
---
drivers/gpu/drm/i915/gt/intel_gtt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 4bcdfcab3642..a5f5b2dda332 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -234,7 +234,8 @@ void i915_address_space_init(struct i915_address_space *vm,
int subclass)
memset64(vm->min_alignment, I915_GTT_MIN_ALIGNMENT,
ARRAY_SIZE(vm->min_alignment));
- if (HAS_64K_PAGES(vm->i915) && NEEDS_COMPACT_PT(vm->i915)) {
+ if (HAS_64K_PAGES(vm->i915) && NEEDS_COMPACT_PT(vm->i915) &&
+ subclass == VM_CLASS_PPGTT) {
vm->min_alignment[INTEL_MEMORY_LOCAL] = I915_GTT_PAGE_SIZE_2M;
vm->min_alignment[INTEL_MEMORY_STOLEN_LOCAL] =
I915_GTT_PAGE_SIZE_2M;
} else if (HAS_64K_PAGES(vm->i915)) {
[PATCH v2 1/2] drm: Add GPU reset sysfs event
From: Shashank Sharma
This patch adds a new sysfs event, which will indicate
the userland about a GPU reset, and can also provide
some information like:
- process ID of the process involved with the GPU reset
- process name of the involved process
- the GPU status info (using flags)
This patch also introduces the first flag of the flags
bitmap, which can be appended as and when required.
V2: Addressed review comments from Christian and Amar
- move the reset information structure to DRM layer
- drop _ctx from struct name
- make pid 32 bit(than 64)
- set flag when VRAM invalid (than valid)
- add process name as well (Amar)
Cc: Alexandar Deucher
Cc: Christian Koenig
Cc: Amaranath Somalapuram
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/drm_sysfs.c | 31 +++
include/drm/drm_sysfs.h | 10 ++
2 files changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 430e00b16eec..840994810910 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -409,6 +409,37 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_sysfs_hotplug_event);
+/**
+ * drm_sysfs_reset_event - generate a DRM uevent to indicate GPU reset
+ * @dev: DRM device
+ * @reset_info: The contextual information about the reset (like PID, flags)
+ *
+ * Send a uevent for the DRM device specified by @dev. This informs
+ * user that a GPU reset has occurred, so that an interested client
+ * can take any recovery or profiling measure.
+ */
+void drm_sysfs_reset_event(struct drm_device *dev, struct drm_reset_event
*reset_info)
+{
+ unsigned char pid_str[13];
+ unsigned char flags_str[15];
+ unsigned char pname_str[TASK_COMM_LEN + 6];
+ unsigned char reset_str[] = "RESET=1";
+ char *envp[] = { reset_str, pid_str, pname_str, flags_str, NULL };
+
+ if (!reset_info) {
+ DRM_WARN("No reset info, not sending the event\n");
+ return;
+ }
+
+ DRM_DEBUG("generating reset event\n");
+
+ snprintf(pid_str, ARRAY_SIZE(pid_str), "PID=%u", reset_info->pid);
+ snprintf(pname_str, ARRAY_SIZE(pname_str), "NAME=%s",
reset_info->pname);
+ snprintf(flags_str, ARRAY_SIZE(flags_str), "FLAGS=%u",
reset_info->flags);
+ kobject_uevent_env(>primary->kdev->kobj, KOBJ_CHANGE, envp);
+}
+EXPORT_SYMBOL(drm_sysfs_reset_event);
+
/**
* drm_sysfs_connector_hotplug_event - generate a DRM uevent for any connector
* change
diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
index 6273cac44e47..5ba11c760619 100644
--- a/include/drm/drm_sysfs.h
+++ b/include/drm/drm_sysfs.h
@@ -1,16 +1,26 @@
/* SPDX-License-Identifier: GPL-2.0 */
#ifndef _DRM_SYSFS_H_
#define _DRM_SYSFS_H_
+#include
+
+#define DRM_GPU_RESET_FLAG_VRAM_INVALID (1 << 0)
struct drm_device;
struct device;
struct drm_connector;
struct drm_property;
+struct drm_reset_event {
+ uint32_t pid;
+ uint32_t flags;
+ char pname[TASK_COMM_LEN];
+};
+
int drm_class_device_register(struct device *dev);
void drm_class_device_unregister(struct device *dev);
void drm_sysfs_hotplug_event(struct drm_device *dev);
+void drm_sysfs_reset_event(struct drm_device *dev, struct drm_reset_event
*reset_info);
void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
void drm_sysfs_connector_status_event(struct drm_connector *connector,
struct drm_property *property);
--
2.32.0
[PATCH v2 2/2] drm/amdgpu: add work function for GPU reset event
From: Shashank Sharma
This patch adds a work function, which sends a GPU reset
uevent and some contextual infomration, like the PID and
some status flags. This work should be scheduled during
a GPU reset.
The userspace can do some recovery and post-processing work
based on this event and information.
V2: Addressed review comments from Christian
- Changed the name of the work to gpu_reset_event_work
- Added a structure to accommodate some additional information
(like a PID and some flags)
- Do not add new structure in amdgpu.h
Cc: Alexander Deucher
Cc: Christian Koenig
Cc: Amaranath Somalapuram
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h| 4
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 13 +
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index d8b854fcbffa..6a97c585bdfd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -61,6 +61,7 @@
#include
#include
#include
+#include
#include
#include "dm_pp_interface.h"
@@ -813,6 +814,7 @@ struct amd_powerplay {
#define AMDGPU_RESET_MAGIC_NUM 64
#define AMDGPU_MAX_DF_PERFMONS 4
#define AMDGPU_PRODUCT_NAME_LEN 64
+
struct amdgpu_device {
struct device *dev;
struct pci_dev *pdev;
@@ -1063,6 +1065,7 @@ struct amdgpu_device {
int asic_reset_res;
struct work_struct xgmi_reset_work;
+ struct work_struct gpu_reset_event_work;
struct list_headreset_list;
longgfx_timeout;
@@ -1097,6 +1100,7 @@ struct amdgpu_device {
pci_channel_state_t pci_channel_state;
struct amdgpu_reset_control *reset_cntl;
+ struct drm_reset_event reset_event_info;
uint32_t
ip_versions[MAX_HWIP][HWIP_MAX_INSTANCE];
boolram_is_direct_mapped;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ed077de426d9..1aef07fd0dff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -73,6 +73,7 @@
#include
#include
+#include
MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin");
MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin");
@@ -3277,6 +3278,17 @@ bool amdgpu_device_has_dc_support(struct amdgpu_device
*adev)
return amdgpu_device_asic_has_dc_support(adev->asic_type);
}
+static void amdgpu_device_reset_event_func(struct work_struct *__work)
+{
+ struct amdgpu_device *adev = container_of(__work, struct amdgpu_device,
+ gpu_reset_event_work);
+ /*
+* A GPU reset has happened, inform the userspace and pass the
+* reset related information.
+*/
+ drm_sysfs_reset_event(>ddev, >reset_event_info);
+}
+
static void amdgpu_device_xgmi_reset_func(struct work_struct *__work)
{
struct amdgpu_device *adev =
@@ -3525,6 +3537,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
amdgpu_device_delay_enable_gfx_off);
INIT_WORK(>xgmi_reset_work, amdgpu_device_xgmi_reset_func);
+ INIT_WORK(>gpu_reset_event_work, amdgpu_device_reset_event_func);
adev->gfx.gfx_off_req_count = 1;
adev->pm.ac_power = power_supply_is_system_supplied() > 0;
--
2.32.0
[PATCH] dt-bindings: gpu: mali-bifrost: Document RZ/V2L SoC
The Renesas RZ/V2L SoC (a.k.a R9A07G054) has a Bifrost Mali-G31 GPU, add a compatible string for it. Signed-off-by: Lad Prabhakar Reviewed-by: Biju Das --- Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 4d6bfae0653c..85f8d4764740 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -20,6 +20,7 @@ properties: - mediatek,mt8183-mali - realtek,rtd1619-mali - renesas,r9a07g044-mali + - renesas,r9a07g054-mali - rockchip,px30-mali - rockchip,rk3568-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -109,7 +110,9 @@ allOf: properties: compatible: contains: -const: renesas,r9a07g044-mali +enum: + - renesas,r9a07g044-mali + - renesas,r9a07g054-mali then: properties: interrupts: -- 2.17.1
Re: [PATCH v7 22/24] drm: rockchip: Add VOP2 driver
Hi Daniel: On 3/9/22 10:03, Andy Yan wrote: Hi Daniel: On 3/8/22 22:04, Daniel Stone wrote: On Tue, 8 Mar 2022 at 08:42, Andy Yan wrote: On 3/7/22 21:09, Daniel Stone wrote: On Mon, 7 Mar 2022 at 12:18, Andy Yan wrote: When run a weston 10.0.0: # export XDG_RUNTIME_DIR=/tmp # weston --backend=drm-backend.so --use-pixma --tty=2 --continue=without-input I got the following error: drm_atomic_check_only [PLANE:31:Smart0-win0] CRTC set but no FB Can you please start Weston with --logger-scopes=log,drm-backend and attach the output? Please see the weston ouput here[0] Are you running with musl perhaps? Do you mean the C library? I chose uClib-ng in buildroot, not use musl. Either way, please make sure your libdrm build includes commit 79fa377c8bdc84fde99c6a6ac17e554971c617be. The upstream buildroot use libdrm2.4.109, this commit[0] if from libdrm2.4.110 I cherry-pick this patch to my local libdrm, but has no effect, still has "atomic: couldn't commit new state" error. I have do a search in libdrm and weston, but find no one call drmModeAtomicMerge, is that right? [0]https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/167 With your patch applied from libdrm2.4.110, I do a make clean for buidlroot, than build it again, That's take effect. I can see only the second value (non-zero FB) of plane 31 commit to the kernel. So this is works. Maybe the buidroot should update libdrm package. Thank you. Cheers, Daniel ___ Linux-rockchip mailing list linux-rockc...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH v2, 0/4] Cooperate with DSI RX devices to modify dsi funcs and delay mipi high to cooperate with panel sequence
On Tue, Mar 8, 2022 at 6:00 PM Benjamin Gaignard wrote: > > > Le 08/03/2022 à 10:12, Hsin-Yi Wang a écrit : > > On Fri, Mar 4, 2022 at 7:25 PM Benjamin Gaignard > > wrote: > >> > >> Le 04/03/2022 à 11:15, xinlei@mediatek.com a écrit : > >>> From: Xinlei Lee > >>> > >>> In upstream-v5.8, dsi_enable will operate panel_enable, but this > >>> modification has been moved in v5.9. In order to ensure the timing of > >>> dsi_power_on/off and the timing of pulling up/down the MIPI signal, > >>> the modification of v5.9 is synchronized in this series of patches. > >> Hello, > >> > >> I'm trying to debug an issue on mt8183 kukui krane sku176 device. > >> The problem is that boe-tv101wum-nl6 panel isn't working. > >> At boot time I can see these logs: > >> panel-boe-tv101wum-nl6 14014000.dsi.0: failed to write command 1 > >> panel-boe-tv101wum-nl6 14014000.dsi.0: failed to init panel: -62 > >> and a DSI interrupt time out. > >> > >> Since I believe the problem is link to DSI/panel enabling sequence > >> I have try this series but that doesn't solve the issue. > >> I notice that when going out of deep sleep mode panel is functional. > >> > >> May you have any idea to debug/solve this problem ? > >> > > Hi Benjamin, > > > > I think this might not be related to this series. Which kernel are you > > using? > > I tried the krane sku176 with linux-next 5.17-rc6 > > (519dd6c19986696f59847ff8bf930436ccffd9a1 (tag: next-20220307, > > linux-next/master) with or without this series, both can get the display on. > > > > dsi related message: > > [0.206330] mediatek-drm mediatek-drm.1.auto: Adding component > > match for /soc/dsi@14014000 > > [4.567577] panel-boe-tv101wum-nl6 14014000.dsi.0: supply pp3300 > > not found, using dummy regulator > > [4.567732] panel-boe-tv101wum-nl6 14014000.dsi.0: GPIO lookup for > > consumer enable > > [4.567738] panel-boe-tv101wum-nl6 14014000.dsi.0: using device > > tree for GPIO lookup > > [4.567757] of_get_named_gpiod_flags: parsed 'enable-gpios' > > property of node '/soc/dsi@14014000/panel@0[0]' - status (0) > > [4.585884] panel-boe-tv101wum-nl6 14014000.dsi.0: supply pp3300 > > not found, using dummy regulator > > [4.586037] panel-boe-tv101wum-nl6 14014000.dsi.0: GPIO lookup for > > consumer enable > > [4.586042] panel-boe-tv101wum-nl6 14014000.dsi.0: using device > > tree for GPIO lookup > > [4.586059] of_get_named_gpiod_flags: parsed 'enable-gpios' > > property of node '/soc/dsi@14014000/panel@0[0]' - status (0) > > [4.587430] mediatek-drm mediatek-drm.1.auto: bound 14014000.dsi > > (ops 0xffd369a752b8) > > > > > > Maybe some config is not enabled? > > I using 5.17.0-rc1-next-20220127 kernel tag. > The configs look similar. > > I have the follow log at boot time: > Hi Xinlei, Can you resend this patch to base on the latest rc? This patch no longer applies to rc6, I have to resolve conflict locally. Thanks > [1.533384] phy phy-11e5.dsi-phy.2: Looking up phy-supply from device > tree > [1.533402] phy phy-11e5.dsi-phy.2: Looking up phy-supply property in > node /soc/dsi-phy@11e5 failed > [3.173068] mediatek-drm mediatek-drm.1.auto: Adding component match for > /soc/dsi@14014000 > [4.671806] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up avdd-supply > from device tree > [4.680348] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up avee-supply > from device tree > [4.688784] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > pp3300-supply from device tree > [4.697816] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > pp1800-supply from device tree > [4.842346] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up avdd-supply > from device tree > [4.854573] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up avee-supply > from device tree > [4.862976] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > pp3300-supply from device tree > [4.871568] panel-boe-tv101wum-nl6 14014000.dsi.0: Looking up > pp1800-supply from device tree > [4.964021] mediatek-drm mediatek-drm.1.auto: bound 14014000.dsi (ops > mtk_dsi_component_ops) > ... > [ 38.273437] [drm] Wait DSI IRQ(0x0002) Timeout > [ 38.281584] panel-boe-tv101wum-nl6 14014000.dsi.0: failed to write command > 1 > [ 38.288651] panel-boe-tv101wum-nl6 14014000.dsi.0: failed to init panel: > -62 > ... > [ 70.113674] mediatek-drm mediatek-drm.1.auto: [drm] *ERROR* flip_done > timed out > [ 70.121054] mediatek-drm mediatek-drm.1.auto: [drm] *ERROR* > [CRTC:45:crtc-0] commit wait timed out > [ 70.130037] [drm:mtk_drm_crtc_atomic_begin] *ERROR* new event while there > is still a pending event > [ 70.241222] [ cut here ] > [ 70.245898] [CRTC:45:crtc-0] vblank wait timed out > [ 70.250729] WARNING: CPU: 7 PID: 397 at > drivers/gpu/drm/drm_atomic_helper.c:1529 > drm_atomic_helper_wait_for_vblanks.part.0+0x290/0x24 > [ 70.262815] Modules linked in: hci_uart btqca btbcm bluetooth
[PATCH 8/8] drm/i915/xehpsdv/dg1/tgl: Fix issue with LRI relative addressing
From: Akeem G Abodunrin
When bit 19 of MI_LOAD_REGISTER_IMM instruction opcode is set on devices
of tgl+, HW does not care about certain register address offsets, but
instead check the following for valid address ranges on specific engines:
RCS && CCS: BITS(0 - 10)
BCS: BITS(0 - 11)
VECS && VCS: BITS(0 - 13)
Also, tgl+ now support relative addressing for BCS engine - So, this
patch fixes issue with live_gt_lrc selftest that is failing where there is
mismatch between LRC register layout generated during init and HW
default register offsets.
Bspec: 45728
Cc: Kumar Valsan, Prathap
Signed-off-by: Akeem G Abodunrin
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 36 +-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 63fd508fea49..5b2a205ab372 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -131,6 +131,27 @@ static int context_flush(struct intel_context *ce, long
timeout)
return err;
}
+static int get_lri_mask(struct intel_engine_cs *engine, u32 lri)
+{
+ if ((lri & MI_LRI_LRM_CS_MMIO) == 0)
+ return ~0u;
+
+ if (GRAPHICS_VER(engine->i915) < 12)
+ return 0xfff;
+
+ switch (engine->class) {
+ default:
+ case RENDER_CLASS:
+ case COMPUTE_CLASS:
+ return 0x07ff;
+ case COPY_ENGINE_CLASS:
+ return 0x0fff;
+ case VIDEO_DECODE_CLASS:
+ case VIDEO_ENHANCEMENT_CLASS:
+ return 0x3fff;
+ }
+}
+
static int live_lrc_layout(void *arg)
{
struct intel_gt *gt = arg;
@@ -170,6 +191,7 @@ static int live_lrc_layout(void *arg)
dw = 0;
do {
u32 lri = READ_ONCE(hw[dw]);
+ u32 lri_mask;
if (lri == 0) {
dw++;
@@ -197,6 +219,18 @@ static int live_lrc_layout(void *arg)
break;
}
+ /*
+* When bit 19 of MI_LOAD_REGISTER_IMM instruction
+* opcode is set on Gen12+ devices, HW does not
+* care about certain register address offsets, and
+* instead check the following for valid address
+* ranges on specific engines:
+* RCS && CCS: BITS(0 - 10)
+* BCS: BITS(0 - 11)
+* VECS && VCS: BITS(0 - 13)
+*/
+ lri_mask = get_lri_mask(engine, lri);
+
lri &= 0x7f;
lri++;
dw++;
@@ -204,7 +238,7 @@ static int live_lrc_layout(void *arg)
while (lri) {
u32 offset = READ_ONCE(hw[dw]);
- if (offset != lrc[dw]) {
+ if ((offset ^ lrc[dw]) & lri_mask) {
pr_err("%s: Different registers found
at dword %d, expected %x, found %x\n",
engine->name, dw, offset,
lrc[dw]);
err = -EINVAL;
--
2.20.1
[PATCH 7/8] drm/i915/selftest: Always cancel semaphore on error
From: Chris Wilson
Ensure that we always signal the semaphore when timing out, so that if it
happens to be stuck waiting for the semaphore we will quickly recover
without having to wait for a reset.
Reported-by: CQ Tang
Signed-off-by: Chris Wilson
cc: Joonas Lahtinen
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index d8face764ee4..63fd508fea49 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -1530,20 +1530,17 @@ __lrc_isolation(struct intel_engine_cs *engine, u32
poison, bool relative)
usleep_range(100, 500);
err = poison_registers(B, engine, poison, relative, );
- if (err) {
- WRITE_ONCE(*sema.va, -1);
- i915_request_put(rq);
- goto err_result1;
- }
-
- if (i915_request_wait(rq, 0, HZ / 2) < 0) {
+ if (err == 0 && i915_request_wait(rq, 0, HZ / 2) < 0) {
pr_err("%s(%s): wait for results timed out\n",
__func__, engine->name);
- i915_request_put(rq);
err = -ETIME;
- goto err_result1;
}
+
+ /* Always cancel the semaphore wait, just in case the GPU gets stuck */
+ WRITE_ONCE(*sema.va, -1);
i915_request_put(rq);
+ if (err)
+ goto err_result1;
err = compare_isolation(engine, ref, result, A, poison, relative);
--
2.20.1
[PATCH 4/8] drm/i915/gt: Explicitly clear BB_OFFSET for new contexts
From: Chris Wilson
Even though the initial protocontext we load onto HW has the register
cleared, by the time we save it into the default image, BB_OFFSET has
had the enable bit set. Reclear BB_OFFSET for each new context.
Testcase: igt/i915_selftests/gt_lrc
Signed-off-by: Chris Wilson
Cc: Mika Kuoppala
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/gt/intel_engine_regs.h | 1 +
drivers/gpu/drm/i915/gt/intel_lrc.c | 17 +
drivers/gpu/drm/i915/gt/selftest_lrc.c | 5 +
3 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_regs.h
b/drivers/gpu/drm/i915/gt/intel_engine_regs.h
index 0bf8b45c9319..d6da3bbf66f8 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_regs.h
@@ -109,6 +109,7 @@
#define RING_SBBSTATE(base)_MMIO((base) + 0x118) /* hsw+ */
#define RING_SBBADDR_UDW(base) _MMIO((base) + 0x11c) /* gen8+
*/
#define RING_BBADDR(base) _MMIO((base) + 0x140)
+#define RING_BB_OFFSET(base) _MMIO((base) + 0x158)
#define RING_BBADDR_UDW(base) _MMIO((base) + 0x168) /* gen8+
*/
#define CCID(base) _MMIO((base) + 0x180)
#define CCID_EN BIT(0)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 07bef7128fdb..f673bae97a03 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -662,6 +662,18 @@ static int lrc_ring_mi_mode(const struct intel_engine_cs
*engine)
return -1;
}
+static int lrc_ring_bb_offset(const struct intel_engine_cs *engine)
+{
+ if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50))
+ return 0x80;
+ else if (GRAPHICS_VER(engine->i915) >= 12)
+ return 0x70;
+ else if (GRAPHICS_VER(engine->i915) >= 9)
+ return 0x64;
+ else
+ return -1;
+}
+
static int lrc_ring_gpr0(const struct intel_engine_cs *engine)
{
if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50))
@@ -768,6 +780,7 @@ static void init_common_regs(u32 * const regs,
bool inhibit)
{
u32 ctl;
+ int loc;
ctl = _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH);
ctl |= _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
@@ -779,6 +792,10 @@ static void init_common_regs(u32 * const regs,
regs[CTX_CONTEXT_CONTROL] = ctl;
regs[CTX_TIMESTAMP] = ce->runtime.last;
+
+ loc = lrc_ring_bb_offset(engine);
+ if (loc != -1)
+ regs[loc + 1] = 0;
}
static void init_wa_bb_regs(u32 * const regs,
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index b064e824053f..2149b2c92793 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -323,6 +323,11 @@ static int live_lrc_fixed(void *arg)
lrc_ring_cmd_buf_cctl(engine),
"RING_CMD_BUF_CCTL"
},
+ {
+
i915_mmio_reg_offset(RING_BB_OFFSET(engine->mmio_base)),
+ lrc_ring_bb_offset(engine),
+ "RING_BB_OFFSET"
+ },
{ },
}, *t;
u32 *hw;
--
2.20.1
[PATCH 5/8] drm/i915/selftests: Check for incomplete LRI from the context image
From: Chris Wilson
In order to keep the context image parser simple, we assume that all
commands follow a similar format. A few, especially not MI commands on
the render engines, have fixed lengths not encoded in a length field.
This caused us to incorrectly skip over 3D state commands, and start
interpretting context data as instructions. Eventually, as Daniele
discovered, this would lead us to find addition LRI as part of the data
and mistakenly add invalid LRI commands to the context probes.
Stop parsing after we see the first !MI command, as we know we will have
seen all the context registers by that point. (Mostly true for all gen
so far, though the render context does have LRI after the first page that
we have been ignoring so far. It would be useful to extract those as well
so that we have the full list of user accesisble registers.)
Similarly, emit a warning if we do try to emit an invalid zero-length
LRI.
Reported-by: Daniele Ceraolo Spurio
Signed-off-by: Chris Wilson
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 63 ++
1 file changed, 55 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 2149b2c92793..6717ecaed178 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -27,6 +27,9 @@
#define NUM_GPR 16
#define NUM_GPR_DW (NUM_GPR * 2) /* each GPR is 2 dwords */
+#define LRI_HEADER MI_INSTR(0x22, 0)
+#define LRI_LENGTH_MASK GENMASK(7, 0)
+
static struct i915_vma *create_scratch(struct intel_gt *gt)
{
return __vm_create_scratch_for_read_pinned(>ggtt->vm, PAGE_SIZE);
@@ -180,7 +183,7 @@ static int live_lrc_layout(void *arg)
continue;
}
- if ((lri & GENMASK(31, 23)) != MI_INSTR(0x22, 0)) {
+ if ((lri & GENMASK(31, 23)) != LRI_HEADER) {
pr_err("%s: Expected LRI command at dword %d,
found %08x\n",
engine->name, dw, lri);
err = -EINVAL;
@@ -948,21 +951,43 @@ store_context(struct intel_context *ce,
hw = defaults;
hw += LRC_STATE_OFFSET / sizeof(*hw);
do {
- u32 len = hw[dw] & 0x7f;
+ u32 len = hw[dw] & LRI_LENGTH_MASK;
u32 cmd = MI_STORE_REGISTER_MEM_GEN8;
u32 offset = 0;
u32 mask = ~0;
+ /*
+* Keep it simple, skip parsing complex commands
+*
+* At present, there are no more MI_LOAD_REGISTER_IMM
+* commands after the first 3D state command. Rather
+* than include a table (see i915_cmd_parser.c) of all
+* the possible commands and their instruction lengths
+* (or mask for variable length instructions), assume
+* we have gathered the complete list of registers and
+* bail out.
+*/
+ if ((hw[dw] >> INSTR_CLIENT_SHIFT) != INSTR_MI_CLIENT)
+ break;
+
if (hw[dw] == 0) {
dw++;
continue;
}
- if ((hw[dw] & GENMASK(31, 23)) != MI_INSTR(0x22, 0)) {
+ if ((hw[dw] & GENMASK(31, 23)) != LRI_HEADER) {
+ /* Assume all other MI commands match LRI length mask */
dw += len + 2;
continue;
}
+ if (!len) {
+ pr_err("%s: invalid LRI found in context image\n",
+ engine->name);
+ igt_hexdump(defaults, PAGE_SIZE);
+ break;
+ }
+
if (hw[dw] & MI_LRI_LRM_CS_MMIO) {
mask = 0xfff;
if (relative)
@@ -1162,21 +1187,32 @@ load_context(struct intel_context *ce,
hw = defaults;
hw += LRC_STATE_OFFSET / sizeof(*hw);
do {
- u32 cmd = MI_INSTR(0x22, 0);
- u32 len = hw[dw] & 0x7f;
+ u32 len = hw[dw] & LRI_LENGTH_MASK;
+ u32 cmd = LRI_HEADER;
u32 offset = 0;
u32 mask = ~0;
+ /* For simplicity, break parsing at the first complex command */
+ if ((hw[dw] >> INSTR_CLIENT_SHIFT) != INSTR_MI_CLIENT)
+ break;
+
if (hw[dw] == 0) {
dw++;
continue;
}
- if ((hw[dw] & GENMASK(31, 23)) != MI_INSTR(0x22, 0)) {
+ if ((hw[dw] & GENMASK(31, 23)) != LRI_HEADER) {
dw += len + 2;
continue;
}
+ if (!len) {
+
[PATCH 3/8] drm/i915/selftests: Flush the submission for lrc_isolation
From: Chris Wilson
The lrc_isolation test uses two contexts in order to read from one
context while poisoning from a second. The test verifies that the
writes of the second context do not leak into the first context. This is
done by first recording the register state from context A, forcing a
preemption to context B, and only then switching back to context A to
re-read the register state to see if anything changed. The sequence is
important (and internally controlled by semaphores), but it does require
that context A is submitted *before* context B, as context B has higher
priority to force the preemption.
Signed-off-by: Chris Wilson
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 998e561694be..b064e824053f 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -1416,8 +1416,10 @@ __lrc_isolation(struct intel_engine_cs *engine, u32
poison, bool relative)
}
if (i915_request_wait(rq, 0, HZ / 2) < 0) {
+ pr_err("%s(%s): wait for reference results timed out\n",
+ __func__, engine->name);
i915_request_put(rq);
- err = -ETIME;
+ err = -EIO;
goto err_ref1;
}
i915_request_put(rq);
@@ -1440,6 +1442,17 @@ __lrc_isolation(struct intel_engine_cs *engine, u32
poison, bool relative)
goto err_result1;
}
+ /* Wait until we record the register state before allowing preemption */
+ if (wait_for_submit(engine, rq, HZ / 5)) {
+ pr_err("%s(%s): wait for submission timed out\n",
+ __func__, engine->name);
+ i915_request_put(rq);
+ err = -EIO;
+ goto err_result1;
+ }
+ while (READ_ONCE(*sema.va) && !signal_pending(current))
+ usleep_range(100, 500);
+
err = poison_registers(B, engine, poison, relative, );
if (err) {
WRITE_ONCE(*sema.va, -1);
@@ -1448,6 +1461,8 @@ __lrc_isolation(struct intel_engine_cs *engine, u32
poison, bool relative)
}
if (i915_request_wait(rq, 0, HZ / 2) < 0) {
+ pr_err("%s(%s): wait for results timed out\n",
+ __func__, engine->name);
i915_request_put(rq);
err = -ETIME;
goto err_result1;
--
2.20.1
[PATCH 6/8] drm/i915/selftest: Clear the output buffers before GPU writes
From: Chris Wilson
When testing whether we can get the GPU to leak information about
non-privileged state, we first need to ensure that the output buffer is
set to a known value as the HW may opt to skip the write into memory for
a non-privileged read of a sensitive register. We chose POISON_INUSE (0x5a)
so that is both non-zero and distinct from the poison values used during
the test.
Reported-by: CQ Tang
Signed-off-by: Chris Wilson
cc: Joonas Lahtinen
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 32 ++
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 6717ecaed178..d8face764ee4 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -1430,6 +1430,30 @@ static int compare_isolation(struct intel_engine_cs
*engine,
return err;
}
+static struct i915_vma *
+create_result_vma(struct i915_address_space *vm, unsigned long sz)
+{
+ struct i915_vma *vma;
+ void *ptr;
+
+ vma = create_user_vma(vm, sz);
+ if (IS_ERR(vma))
+ return vma;
+
+ /* Set the results to a known value distinct from the poison */
+ ptr = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
+ if (IS_ERR(ptr)) {
+ i915_vma_put(vma);
+ return ERR_CAST(ptr);
+ }
+
+ memset(ptr, POISON_INUSE, vma->size);
+ i915_gem_object_flush_map(vma->obj);
+ i915_gem_object_unpin_map(vma->obj);
+
+ return vma;
+}
+
static int
__lrc_isolation(struct intel_engine_cs *engine, u32 poison, bool relative)
{
@@ -1449,13 +1473,13 @@ __lrc_isolation(struct intel_engine_cs *engine, u32
poison, bool relative)
goto err_A;
}
- ref[0] = create_user_vma(A->vm, SZ_64K);
+ ref[0] = create_result_vma(A->vm, SZ_64K);
if (IS_ERR(ref[0])) {
err = PTR_ERR(ref[0]);
goto err_B;
}
- ref[1] = create_user_vma(A->vm, SZ_64K);
+ ref[1] = create_result_vma(A->vm, SZ_64K);
if (IS_ERR(ref[1])) {
err = PTR_ERR(ref[1]);
goto err_ref0;
@@ -1476,13 +1500,13 @@ __lrc_isolation(struct intel_engine_cs *engine, u32
poison, bool relative)
}
i915_request_put(rq);
- result[0] = create_user_vma(A->vm, SZ_64K);
+ result[0] = create_result_vma(A->vm, SZ_64K);
if (IS_ERR(result[0])) {
err = PTR_ERR(result[0]);
goto err_ref1;
}
- result[1] = create_user_vma(A->vm, SZ_64K);
+ result[1] = create_result_vma(A->vm, SZ_64K);
if (IS_ERR(result[1])) {
err = PTR_ERR(result[1]);
goto err_result0;
--
2.20.1
[PATCH 2/8] drm/i915/selftests: Exercise cross-process context isolation
From: Chris Wilson
Verify that one context running on engine A cannot manipulate another
client's context concurrently running on engine B using unprivileged
access.
Signed-off-by: Chris Wilson
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 275 +
1 file changed, 238 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 073c9795f42f..998e561694be 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -913,6 +913,7 @@ create_user_vma(struct i915_address_space *vm, unsigned
long size)
static struct i915_vma *
store_context(struct intel_context *ce,
+ struct intel_engine_cs *engine,
struct i915_vma *scratch,
bool relative)
{
@@ -930,7 +931,7 @@ store_context(struct intel_context *ce,
return ERR_CAST(cs);
}
- defaults = shmem_pin_map(ce->engine->default_state);
+ defaults = shmem_pin_map(engine->default_state);
if (!defaults) {
i915_gem_object_unpin_map(batch->obj);
i915_vma_put(batch);
@@ -962,7 +963,7 @@ store_context(struct intel_context *ce,
if (relative)
cmd |= MI_LRI_LRM_CS_MMIO;
else
- offset = ce->engine->mmio_base;
+ offset = engine->mmio_base;
}
dw++;
@@ -981,7 +982,7 @@ store_context(struct intel_context *ce,
*cs++ = MI_BATCH_BUFFER_END;
- shmem_unpin_map(ce->engine->default_state, defaults);
+ shmem_unpin_map(engine->default_state, defaults);
i915_gem_object_flush_map(batch->obj);
i915_gem_object_unpin_map(batch->obj);
@@ -1004,23 +1005,48 @@ static int move_to_active(struct i915_request *rq,
return err;
}
+struct hwsp_semaphore {
+ u32 ggtt;
+ u32 *va;
+};
+
+static struct hwsp_semaphore hwsp_semaphore(struct intel_engine_cs *engine)
+{
+ struct hwsp_semaphore s;
+
+ s.va = memset32(engine->status_page.addr + 1000, 0, 1);
+ s.ggtt = (i915_ggtt_offset(engine->status_page.vma) +
+ offset_in_page(s.va));
+
+ return s;
+}
+
+static u32 *emit_noops(u32 *cs, int count)
+{
+ while (count--)
+ *cs++ = MI_NOOP;
+
+ return cs;
+}
+
static struct i915_request *
record_registers(struct intel_context *ce,
+struct intel_engine_cs *engine,
struct i915_vma *before,
struct i915_vma *after,
bool relative,
-u32 *sema)
+const struct hwsp_semaphore *sema)
{
struct i915_vma *b_before, *b_after;
struct i915_request *rq;
u32 *cs;
int err;
- b_before = store_context(ce, before, relative);
+ b_before = store_context(ce, engine, before, relative);
if (IS_ERR(b_before))
return ERR_CAST(b_before);
- b_after = store_context(ce, after, relative);
+ b_after = store_context(ce, engine, after, relative);
if (IS_ERR(b_after)) {
rq = ERR_CAST(b_after);
goto err_before;
@@ -1046,7 +1072,7 @@ record_registers(struct intel_context *ce,
if (err)
goto err_rq;
- cs = intel_ring_begin(rq, 14);
+ cs = intel_ring_begin(rq, 18);
if (IS_ERR(cs)) {
err = PTR_ERR(cs);
goto err_rq;
@@ -1057,16 +1083,28 @@ record_registers(struct intel_context *ce,
*cs++ = lower_32_bits(b_before->node.start);
*cs++ = upper_32_bits(b_before->node.start);
- *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
- *cs++ = MI_SEMAPHORE_WAIT |
- MI_SEMAPHORE_GLOBAL_GTT |
- MI_SEMAPHORE_POLL |
- MI_SEMAPHORE_SAD_NEQ_SDD;
- *cs++ = 0;
- *cs++ = i915_ggtt_offset(ce->engine->status_page.vma) +
- offset_in_page(sema);
- *cs++ = 0;
- *cs++ = MI_NOOP;
+ if (sema) {
+ WRITE_ONCE(*sema->va, -1);
+
+ /* Signal the poisoner */
+ *cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
+ *cs++ = sema->ggtt;
+ *cs++ = 0;
+ *cs++ = 0;
+
+ /* Then wait for the poison to settle */
+ *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
+ *cs++ = MI_SEMAPHORE_WAIT |
+ MI_SEMAPHORE_GLOBAL_GTT |
+ MI_SEMAPHORE_POLL |
+ MI_SEMAPHORE_SAD_NEQ_SDD;
+ *cs++ = 0;
+ *cs++ = sema->ggtt;
+ *cs++ = 0;
+ *cs++ = MI_NOOP;
+ } else {
+ cs = emit_noops(cs, 10);
+ }
*cs++ = MI_ARB_ON_OFF | MI_ARB_DISABLE;
*cs++ = MI_BATCH_BUFFER_START_GEN8 |
[PATCH 1/8] drm/i915/selftests: Exercise relative mmio paths to non-privileged registers
From: Chris Wilson
Verify that context isolation is also preserved when accessing
context-local registers with relative-mmio commands.
Signed-off-by: Chris Wilson
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 88 --
1 file changed, 67 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 21c29d315cc0..073c9795f42f 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -912,7 +912,9 @@ create_user_vma(struct i915_address_space *vm, unsigned
long size)
}
static struct i915_vma *
-store_context(struct intel_context *ce, struct i915_vma *scratch)
+store_context(struct intel_context *ce,
+ struct i915_vma *scratch,
+ bool relative)
{
struct i915_vma *batch;
u32 dw, x, *cs, *hw;
@@ -941,6 +943,9 @@ store_context(struct intel_context *ce, struct i915_vma
*scratch)
hw += LRC_STATE_OFFSET / sizeof(*hw);
do {
u32 len = hw[dw] & 0x7f;
+ u32 cmd = MI_STORE_REGISTER_MEM_GEN8;
+ u32 offset = 0;
+ u32 mask = ~0;
if (hw[dw] == 0) {
dw++;
@@ -952,11 +957,19 @@ store_context(struct intel_context *ce, struct i915_vma
*scratch)
continue;
}
+ if (hw[dw] & MI_LRI_LRM_CS_MMIO) {
+ mask = 0xfff;
+ if (relative)
+ cmd |= MI_LRI_LRM_CS_MMIO;
+ else
+ offset = ce->engine->mmio_base;
+ }
+
dw++;
len = (len + 1) / 2;
while (len--) {
- *cs++ = MI_STORE_REGISTER_MEM_GEN8;
- *cs++ = hw[dw];
+ *cs++ = cmd;
+ *cs++ = (hw[dw] & mask) + offset;
*cs++ = lower_32_bits(scratch->node.start + x);
*cs++ = upper_32_bits(scratch->node.start + x);
@@ -995,6 +1008,7 @@ static struct i915_request *
record_registers(struct intel_context *ce,
struct i915_vma *before,
struct i915_vma *after,
+bool relative,
u32 *sema)
{
struct i915_vma *b_before, *b_after;
@@ -1002,11 +1016,11 @@ record_registers(struct intel_context *ce,
u32 *cs;
int err;
- b_before = store_context(ce, before);
+ b_before = store_context(ce, before, relative);
if (IS_ERR(b_before))
return ERR_CAST(b_before);
- b_after = store_context(ce, after);
+ b_after = store_context(ce, after, relative);
if (IS_ERR(b_after)) {
rq = ERR_CAST(b_after);
goto err_before;
@@ -1076,7 +1090,8 @@ record_registers(struct intel_context *ce,
goto err_after;
}
-static struct i915_vma *load_context(struct intel_context *ce, u32 poison)
+static struct i915_vma *
+load_context(struct intel_context *ce, u32 poison, bool relative)
{
struct i915_vma *batch;
u32 dw, *cs, *hw;
@@ -1103,7 +1118,10 @@ static struct i915_vma *load_context(struct
intel_context *ce, u32 poison)
hw = defaults;
hw += LRC_STATE_OFFSET / sizeof(*hw);
do {
+ u32 cmd = MI_INSTR(0x22, 0);
u32 len = hw[dw] & 0x7f;
+ u32 offset = 0;
+ u32 mask = ~0;
if (hw[dw] == 0) {
dw++;
@@ -1115,11 +1133,19 @@ static struct i915_vma *load_context(struct
intel_context *ce, u32 poison)
continue;
}
+ if (hw[dw] & MI_LRI_LRM_CS_MMIO) {
+ mask = 0xfff;
+ if (relative)
+ cmd |= MI_LRI_LRM_CS_MMIO;
+ else
+ offset = ce->engine->mmio_base;
+ }
+
dw++;
+ *cs++ = cmd | len;
len = (len + 1) / 2;
- *cs++ = MI_LOAD_REGISTER_IMM(len);
while (len--) {
- *cs++ = hw[dw];
+ *cs++ = (hw[dw] & mask) + offset;
*cs++ = poison;
dw += 2;
}
@@ -1136,14 +1162,18 @@ static struct i915_vma *load_context(struct
intel_context *ce, u32 poison)
return batch;
}
-static int poison_registers(struct intel_context *ce, u32 poison, u32 *sema)
+static int
+poison_registers(struct intel_context *ce,
+u32 poison,
+bool relative,
+u32 *sema)
{
struct i915_request *rq;
struct i915_vma *batch;
u32 *cs;
int err;
- batch = load_context(ce, poison);
+ batch = load_context(ce, poison,
[PATCH 0/8] Patches for selftest_lrc
Patches that fix and enhance the selftest_lrc Akeem G Abodunrin (1): drm/i915/xehpsdv/dg1/tgl: Fix issue with LRI relative addressing Chris Wilson (7): drm/i915/selftests: Exercise relative mmio paths to non-privileged registers drm/i915/selftests: Exercise cross-process context isolation drm/i915/selftests: Flush the submission for lrc_isolation drm/i915/gt: Explicitly clear BB_OFFSET for new contexts drm/i915/selftests: Check for incomplete LRI from the context image drm/i915/selftest: Clear the output buffers before GPU writes drm/i915/selftest: Always cancel semaphore on error drivers/gpu/drm/i915/gt/intel_engine_regs.h | 1 + drivers/gpu/drm/i915/gt/intel_lrc.c | 17 + drivers/gpu/drm/i915/gt/selftest_lrc.c | 499 +--- 3 files changed, 452 insertions(+), 65 deletions(-) -- 2.20.1
Re: [PATCH v7 22/24] drm: rockchip: Add VOP2 driver
Hi Daniel: On 3/8/22 22:04, Daniel Stone wrote: On Tue, 8 Mar 2022 at 08:42, Andy Yan wrote: On 3/7/22 21:09, Daniel Stone wrote: On Mon, 7 Mar 2022 at 12:18, Andy Yan wrote: When run a weston 10.0.0: # export XDG_RUNTIME_DIR=/tmp # weston --backend=drm-backend.so --use-pixma --tty=2 --continue=without-input I got the following error: drm_atomic_check_only [PLANE:31:Smart0-win0] CRTC set but no FB Can you please start Weston with --logger-scopes=log,drm-backend and attach the output? Please see the weston ouput here[0] Are you running with musl perhaps? Do you mean the C library? I chose uClib-ng in buildroot, not use musl. Either way, please make sure your libdrm build includes commit 79fa377c8bdc84fde99c6a6ac17e554971c617be. The upstream buildroot use libdrm2.4.109, this commit[0] if from libdrm2.4.110 I cherry-pick this patch to my local libdrm, but has no effect, still has "atomic: couldn't commit new state" error. I have do a search in libdrm and weston, but find no one call drmModeAtomicMerge, is that right? [0]https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/167 Cheers, Daniel
[pull] drm/msm: drm-msm-next-2022-03-08 for 5.18
Hi Dave & Daniel, Follow-up pull req for v5.18 to pull in some important fixes. The following changes since commit afab9d91d872819f98a792c32c302d9e3261f1a1: drm/msm/adreno: Expose speedbin to userspace (2022-02-25 13:29:57 -0800) are available in the Git repository at: https://gitlab.freedesktop.org/drm/msm.git drm-msm-next-2022-03-08 for you to fetch changes up to 05afd57f4d34602a652fdaf58e0a2756b3c20fd4: drm/msm/gpu: Fix crash on devices without devfreq support (v2) (2022-03-08 13:55:23 -0800) Dan Carpenter (1): drm/msm/adreno: fix cast in adreno_get_param() Dmitry Baryshkov (1): dt-bindings: display/msm: add missing brace in dpu-qcm2290.yaml Rob Clark (8): drm/msm: Update generated headers drm/msm: Add SET_PARAM ioctl drm/msm: Add SYSPROF param (v2) drm/msm/a6xx: Zap counters across context switch drm/msm: Add MSM_SUBMIT_FENCE_SN_IN drm/msm/a6xx: Fix missing ARRAY_SIZE() check drm/msm: Fix dirtyfb refcounting drm/msm/gpu: Fix crash on devices without devfreq support (v2) Rob Herring (1): dt-bindings: display/msm: Drop bogus interrupt flags cell on MDSS nodes .../bindings/display/msm/dpu-msm8998.yaml | 4 +- .../bindings/display/msm/dpu-qcm2290.yaml | 5 +- drivers/gpu/drm/msm/adreno/a2xx.xml.h | 26 +- drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 1 + drivers/gpu/drm/msm/adreno/a3xx.xml.h | 30 +- drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 1 + drivers/gpu/drm/msm/adreno/a4xx.xml.h | 112 +++- drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 1 + drivers/gpu/drm/msm/adreno/a5xx.xml.h | 63 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 1 + drivers/gpu/drm/msm/adreno/a6xx.xml.h | 674 + drivers/gpu/drm/msm/adreno/a6xx_gmu.xml.h | 26 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 42 +- drivers/gpu/drm/msm/adreno/adreno_common.xml.h | 31 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c| 22 +- drivers/gpu/drm/msm/adreno/adreno_gpu.h| 2 + drivers/gpu/drm/msm/adreno/adreno_pm4.xml.h| 46 +- drivers/gpu/drm/msm/disp/mdp4/mdp4.xml.h | 37 +- drivers/gpu/drm/msm/disp/mdp5/mdp5.xml.h | 37 +- drivers/gpu/drm/msm/disp/mdp_common.xml.h | 37 +- drivers/gpu/drm/msm/dsi/dsi.xml.h | 37 +- drivers/gpu/drm/msm/dsi/dsi_phy_10nm.xml.h | 37 +- drivers/gpu/drm/msm/dsi/dsi_phy_14nm.xml.h | 37 +- drivers/gpu/drm/msm/dsi/dsi_phy_20nm.xml.h | 37 +- drivers/gpu/drm/msm/dsi/dsi_phy_28nm.xml.h | 37 +- drivers/gpu/drm/msm/dsi/dsi_phy_28nm_8960.xml.h| 37 +- drivers/gpu/drm/msm/dsi/dsi_phy_5nm.xml.h | 480 --- drivers/gpu/drm/msm/dsi/dsi_phy_7nm.xml.h | 43 +- drivers/gpu/drm/msm/dsi/mmss_cc.xml.h | 37 +- drivers/gpu/drm/msm/dsi/sfpb.xml.h | 37 +- drivers/gpu/drm/msm/hdmi/hdmi.xml.h| 37 +- drivers/gpu/drm/msm/hdmi/qfprom.xml.h | 37 +- drivers/gpu/drm/msm/msm_drv.c | 31 +- drivers/gpu/drm/msm/msm_fb.c | 4 +- drivers/gpu/drm/msm/msm_gem_submit.c | 42 +- drivers/gpu/drm/msm/msm_gpu.c | 2 + drivers/gpu/drm/msm/msm_gpu.h | 29 + drivers/gpu/drm/msm/msm_gpu_devfreq.c | 30 +- drivers/gpu/drm/msm/msm_submitqueue.c | 39 ++ include/uapi/drm/msm_drm.h | 32 +- 40 files changed, 1144 insertions(+), 1156 deletions(-) delete mode 100644 drivers/gpu/drm/msm/dsi/dsi_phy_5nm.xml.h
Re: [PATCH] drm/bridge: anx7625: check the return on anx7625_aux_trans
On 3/8/22 2:57 PM, Nick Desaulniers wrote:
On Thu, Mar 3, 2022 at 12:19 PM wrote:
From: Tom Rix
Clang static analysis reports this issue
anx7625.c:876:13: warning: The left operand of '&' is
a garbage value
if (!(bcap & 0xOA01)) {
^
bcap is only set by a successful call to
anx7625_aux_trans(). So check.
Fixes: cd1637c7e480 ("drm/bridge: anx7625: add HDCP support")
Is this the correct Fixes tag?
yes
I think it should be
Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid
through aux channel")
This one changes the name of the function
- anx7625_aux_dpcd_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, );
+ anx7625_aux_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, );
A return check from the earlier commit, when this block of code came
into existence, is when it was first needed.
Tom
instead.
Signed-off-by: Tom Rix
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 633618bafd75d..f02ac079ed2ec 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -872,7 +872,10 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx)
}
/* Read downstream capability */
- anx7625_aux_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, );
+ ret = anx7625_aux_trans(ctx, DP_AUX_NATIVE_READ, 0x68028, 1, );
+ if (ret < 0)
+ return ret;
+
if (!(bcap & 0x01)) {
pr_warn("downstream not support HDCP 1.4, cap(%x).\n", bcap);
return 0;
--
2.26.3
Re: [PATCH v1 5/5] drm/virtio: Add memory shrinker
On Tue, Mar 8, 2022 at 5:17 AM Dmitry Osipenko
wrote:
>
> Add memory shrinker and new madvise IOCTL to the VirtIO-GPU driver.
> Userspace (BO cache manager of Mesa driver) will mark BOs as "don't need"
> using the new IOCTL to let shrinker purge the marked BOs on OOM, thus
> shrinker will lower memory pressure and prevent OOM kills.
>
> Signed-off-by: Daniel Almeida
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/gpu/drm/virtio/Makefile | 3 +-
> drivers/gpu/drm/virtio/virtgpu_drv.h | 26 +++-
> drivers/gpu/drm/virtio/virtgpu_gem.c | 84
> drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c | 124 ++
> drivers/gpu/drm/virtio/virtgpu_ioctl.c| 37 ++
> drivers/gpu/drm/virtio/virtgpu_kms.c | 10 ++
> drivers/gpu/drm/virtio/virtgpu_object.c | 7 +
> drivers/gpu/drm/virtio/virtgpu_plane.c| 17 ++-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 15 +++
> include/uapi/drm/virtgpu_drm.h| 14 ++
> 10 files changed, 333 insertions(+), 4 deletions(-)
> create mode 100644 drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c
>
[snip]
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c
> b/drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c
> new file mode 100644
> index ..39eb9a3e7e4a
> --- /dev/null
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Collabora Ltd.
> + */
> +
> +#include
> +#include
> +
> +#include "virtgpu_drv.h"
> +
> +static unsigned long
> +virtio_gpu_gem_shrinker_count_objects(struct shrinker *shrinker,
> + struct shrink_control *sc)
> +{
> + struct drm_gem_shmem_object *shmem;
> + struct virtio_gpu_device *vgdev;
> + unsigned long count = 0;
> + bool empty = true;
> +
> + vgdev = container_of(shrinker, struct virtio_gpu_device,
> +vgshrinker.shrinker);
> +
> + if (!mutex_trylock(>mm_lock))
> + return 0;
One bit of advice from previously dealing with shrinker and heavy
memory pressure situations (turns out 4GB chromebooks can be pretty
much under *constant* memory pressure):
You *really* want to make shrinker->count_objects lockless.. and
minimize the lock contention on shrinker->scan_objects (ie. The
problem is you can end up with shrinking going on on all CPU cores in
parallel, you want to not funnel that thru one lock as much as
possible.
See in particular:
25ed38b3ed26 ("drm/msm: Drop mm_lock in scan loop")
cc8a4d5a1bd8 ("drm/msm: Avoid mutex in shrinker_count()")
BR,
-R
> + list_for_each_entry(shmem, >vgshrinker.list, madv_list) {
> + empty = false;
> +
> + if (!mutex_trylock(>pages_lock))
> + continue;
> +
> + if (drm_gem_shmem_is_purgeable(shmem))
> + count += shmem->base.size >> PAGE_SHIFT;
> +
> + mutex_unlock(>pages_lock);
> + }
> +
> + mutex_unlock(>mm_lock);
> +
> + return empty ? SHRINK_EMPTY : count;
> +}
> +
> +static bool virtio_gpu_gem_shrinker_purge(struct virtio_gpu_device *vgdev,
> + struct drm_gem_object *obj)
> +{
> + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
> + struct drm_gem_shmem_object *shmem = >base;
> + int err;
> +
> + if (!dma_resv_test_signaled(obj->resv, true) ||
> + !drm_gem_shmem_is_purgeable(shmem) ||
> + refcount_read(>pin_count))
> + return false;
> +
> + /*
> +* Release host's memory before guest's memory is gone to ensure that
> +* host won't touch released memory of the guest.
> +*/
> + err = virtio_gpu_gem_host_mem_release(bo);
> + if (err)
> + return false;
> +
> + list_del_init(>madv_list);
> + drm_gem_shmem_purge_locked(shmem);
> +
> + return true;
> +}
> +
> +static unsigned long
> +virtio_gpu_gem_shrinker_scan_objects(struct shrinker *shrinker,
> +struct shrink_control *sc)
> +{
> + struct drm_gem_shmem_object *shmem, *tmp;
> + struct virtio_gpu_device *vgdev;
> + unsigned long freed = 0;
> +
> + vgdev = container_of(shrinker, struct virtio_gpu_device,
> +vgshrinker.shrinker);
> +
> + if (!mutex_trylock(>mm_lock))
> + return SHRINK_STOP;
> +
> + list_for_each_entry_safe(shmem, tmp, >vgshrinker.list,
> madv_list) {
> + if (freed >= sc->nr_to_scan)
> + break;
> +
> + if (!dma_resv_trylock(shmem->base.resv))
> + continue;
> +
> + if (!mutex_trylock(>pages_lock))
> + goto resv_unlock;
> +
> + if (virtio_gpu_gem_shrinker_purge(vgdev, >base))
> +
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On Tue, Mar 8, 2022 at 3:36 PM Dmitry Osipenko wrote: > > On 3/9/22 01:24, Rob Clark wrote: > > On Tue, Mar 8, 2022 at 11:28 AM Dmitry Osipenko > > wrote: > >> > >> On 3/8/22 19:29, Rob Clark wrote: > >>> On Tue, Mar 8, 2022 at 5:17 AM Dmitry Osipenko > >>> wrote: > > Hello, > > This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. > During OOM, the shrinker will release BOs that are marked as "not needed" > by userspace using the new madvise IOCTL. The userspace in this case is > the Mesa VirGL driver, it will mark the cached BOs as "not needed", > allowing kernel driver to release memory of the cached shmem BOs on > lowmem > situations, preventing OOM kills. > >>> > >>> Will host memory pressure already trigger shrinker in guest? > >> > >> The host memory pressure won't trigger shrinker in guest here. This > >> series will help only with the memory pressure within the guest using a > >> usual "virgl context". > >> > >> Having a host shrinker in a case of "virgl contexts" should be a > >> difficult problem to solve. > > > > Hmm, I think we just need the balloon driver to trigger the shrinker > > in the guest kernel? I suppose a driver like drm/virtio might want to > > differentiate between host and guest pressure (ie. consider only > > objects that have host vs guest storage), but even without that, > > freeing up memory in the guest when host is under memory pressure > > seems worthwhile. Maybe I'm over-simplifying? > > Might be the opposite, i.e. me over-complicating :) The variant with > memory ballooning actually could be good and will work for all kinds of > virtio contexts universally. There will be some back-n-forth between > host and guest, but perhaps it will work okay. Thank you for the suggestion. > > >>> This is > >>> something I'm quite interested in for "virtgpu native contexts" (ie. > >>> native guest driver with new context type sitting on top of virtgpu), > >> > >> In a case of "native contexts" it should be doable, at least I can't see > >> any obvious problems. The madvise invocations could be passed to the > >> host using a new virtio-gpu command by the guest's madvise IOCTL > >> handler, instead-of/in-addition-to handling madvise in the guest's > >> kernel, and that's it. > > > > I think we don't want to do that, because MADV:WILLNEED would be by > > far the most frequent guest<->host synchronous round trip. So from > > that perspective tracking madvise state in guest kernel seems quite > > attractive. > > This is a valid concern. I'd assume that the overhead should be > tolerable, but I don't have any actual perf numbers. jfwiw, MADV:WILLNEED is a *very* hot path for gl drivers, based on some measurements I did a while back with various apps/benchmarks.. easily more than 10x the next most frequent ioctl (for MADV:WONTNEED and MADV:WILLNEED each, so more than 20x combined.. but MADV:WONTNEED can be async). But if the balloon triggering shrinker approach works out, that would be pretty great.. it seems like the easy option and doesn't require adding new host kernel uabi :-) BR, -R > > If we really can't track madvise state in the guest for dealing with > > host memory pressure, I think the better option is to introduce > > MADV:WILLNEED_REPLACE, ie. something to tell the host kernel that the > > buffer is needed but the previous contents are not (as long as the GPU > > VA remains the same). With this the host could allocate new pages if > > needed, and the guest would not need to wait for a reply from host. > > If variant with the memory ballooning will work, then it will be > possible to track the state within guest-only. Let's consider the > simplest variant for now. > > I'll try to implement the balloon driver support in the v2 and will get > back to you. > > >>> since that isn't using host storage > >> > >> s/host/guest ? > > > > Yes, sorry, I meant that it is not using guest storage. > > Thank you for the clarification.
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On 3/9/22 01:24, Rob Clark wrote: > On Tue, Mar 8, 2022 at 11:28 AM Dmitry Osipenko > wrote: >> >> On 3/8/22 19:29, Rob Clark wrote: >>> On Tue, Mar 8, 2022 at 5:17 AM Dmitry Osipenko >>> wrote: Hello, This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. During OOM, the shrinker will release BOs that are marked as "not needed" by userspace using the new madvise IOCTL. The userspace in this case is the Mesa VirGL driver, it will mark the cached BOs as "not needed", allowing kernel driver to release memory of the cached shmem BOs on lowmem situations, preventing OOM kills. >>> >>> Will host memory pressure already trigger shrinker in guest? >> >> The host memory pressure won't trigger shrinker in guest here. This >> series will help only with the memory pressure within the guest using a >> usual "virgl context". >> >> Having a host shrinker in a case of "virgl contexts" should be a >> difficult problem to solve. > > Hmm, I think we just need the balloon driver to trigger the shrinker > in the guest kernel? I suppose a driver like drm/virtio might want to > differentiate between host and guest pressure (ie. consider only > objects that have host vs guest storage), but even without that, > freeing up memory in the guest when host is under memory pressure > seems worthwhile. Maybe I'm over-simplifying? Might be the opposite, i.e. me over-complicating :) The variant with memory ballooning actually could be good and will work for all kinds of virtio contexts universally. There will be some back-n-forth between host and guest, but perhaps it will work okay. Thank you for the suggestion. >>> This is >>> something I'm quite interested in for "virtgpu native contexts" (ie. >>> native guest driver with new context type sitting on top of virtgpu), >> >> In a case of "native contexts" it should be doable, at least I can't see >> any obvious problems. The madvise invocations could be passed to the >> host using a new virtio-gpu command by the guest's madvise IOCTL >> handler, instead-of/in-addition-to handling madvise in the guest's >> kernel, and that's it. > > I think we don't want to do that, because MADV:WILLNEED would be by > far the most frequent guest<->host synchronous round trip. So from > that perspective tracking madvise state in guest kernel seems quite > attractive. This is a valid concern. I'd assume that the overhead should be tolerable, but I don't have any actual perf numbers. > If we really can't track madvise state in the guest for dealing with > host memory pressure, I think the better option is to introduce > MADV:WILLNEED_REPLACE, ie. something to tell the host kernel that the > buffer is needed but the previous contents are not (as long as the GPU > VA remains the same). With this the host could allocate new pages if > needed, and the guest would not need to wait for a reply from host. If variant with the memory ballooning will work, then it will be possible to track the state within guest-only. Let's consider the simplest variant for now. I'll try to implement the balloon driver support in the v2 and will get back to you. >>> since that isn't using host storage >> >> s/host/guest ? > > Yes, sorry, I meant that it is not using guest storage. Thank you for the clarification.
Re: [PATCH v2 1/3] dt-bindings: media: Add macros for video interface bus types
On Sun, Mar 06, 2022 at 08:04:49PM +0200, Laurent Pinchart wrote: > On Sun, Mar 06, 2022 at 07:39:03PM +0200, Laurent Pinchart wrote: > > Add a new dt-bindings/media/video-interfaces.h header that defines > > macros corresponding to the bus types from media/video-interfaces.yaml. > > This allows avoiding hardcoded constants in device tree sources. > > > > Signed-off-by: Laurent Pinchart > > --- > > Changes since v1: > > > > - Dual-license under GPL-2.0-only or MIT > > - Rename PARALLEL TO BT601 > > Contrary to popular belief, further investigation revealed that BT.601 > doesn't define VSYNC and HSYNC (or HREF, as it is also commonly called) > signals. MEDIA_BUS_TYPE_BT601 is thus likely not a good name. I haven't > been able to find a standard for parallel camera interfaces that would > be a good match here. On the display side there's MIPI DPI, but on the > camera side it seems things have evolved quite organically. I may have > missed something though. So keep 'PARALLEL' and anything less ambiguous will be a new type. Rob
Re: [v3,4/5] fbdev: Improve performance of cfb_imageblit()
Hi Thomas,
On 23.02.2022 20:38, Thomas Zimmermann wrote:
> Improve the performance of cfb_imageblit() by manually unrolling
> the inner blitting loop and moving some invariants out. The compiler
> failed to do this automatically. This change keeps cfb_imageblit()
> in sync with sys_imagebit().
>
> A microbenchmark measures the average number of CPU cycles
> for cfb_imageblit() after a stabilizing period of a few minutes
> (i7-4790, FullHD, simpledrm, kernel with debugging).
>
> cfb_imageblit(), new: 15724 cycles
> cfb_imageblit(): old: 30566 cycles
>
> In the optimized case, cfb_imageblit() is now ~2x faster than before.
>
> v3:
> * fix commit description (Pekka)
>
> Signed-off-by: Thomas Zimmermann
> Acked-by: Sam Ravnborg
> Reviewed-by: Javier Martinez Canillas
This patch landed recently in linux next-20220308 as commit 0d03011894d2
("fbdev: Improve performance of cfb_imageblit()"). Sadly it causes a
freeze after DRM and emulated fbdev initialization on various Samsung
Exynos ARM 32bit based boards. This happens when kernel is compiled from
exynos_defconfig. Surprisingly when kernel is compiled from
multi_v7_defconfig all those boards boot fine, so this is a matter of
one of the debugging options enabled in the exynos_defconfig. I will try
to analyze this further and share the results. Reverting $subject on top
of next-20220308 fixes the boot issue.
> ---
> drivers/video/fbdev/core/cfbimgblt.c | 51 +++-
> 1 file changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/cfbimgblt.c
> b/drivers/video/fbdev/core/cfbimgblt.c
> index 01b01a279681..7361cfabdd85 100644
> --- a/drivers/video/fbdev/core/cfbimgblt.c
> +++ b/drivers/video/fbdev/core/cfbimgblt.c
> @@ -218,23 +218,29 @@ static inline void fast_imageblit(const struct fb_image
> *image, struct fb_info *
> {
> u32 fgx = fgcolor, bgx = bgcolor, bpp = p->var.bits_per_pixel;
> u32 ppw = 32/bpp, spitch = (image->width + 7)/8;
> - u32 bit_mask, end_mask, eorx, shift;
> + u32 bit_mask, eorx;
> const char *s = image->data, *src;
> u32 __iomem *dst;
> const u32 *tab = NULL;
> + size_t tablen;
> + u32 colortab[16];
> int i, j, k;
>
> switch (bpp) {
> case 8:
> tab = fb_be_math(p) ? cfb_tab8_be : cfb_tab8_le;
> + tablen = 16;
> break;
> case 16:
> tab = fb_be_math(p) ? cfb_tab16_be : cfb_tab16_le;
> + tablen = 4;
> break;
> case 32:
> - default:
> tab = cfb_tab32;
> + tablen = 2;
> break;
> + default:
> + return;
> }
>
> for (i = ppw-1; i--; ) {
> @@ -248,15 +254,42 @@ static inline void fast_imageblit(const struct fb_image
> *image, struct fb_info *
> eorx = fgx ^ bgx;
> k = image->width/ppw;
>
> - for (i = image->height; i--; ) {
> - dst = (u32 __iomem *) dst1, shift = 8; src = s;
> + for (i = 0; i < tablen; ++i)
> + colortab[i] = (tab[i] & eorx) ^ bgx;
>
> - for (j = k; j--; ) {
> - shift -= ppw;
> - end_mask = tab[(*src >> shift) & bit_mask];
> - FB_WRITEL((end_mask & eorx)^bgx, dst++);
> - if (!shift) { shift = 8; src++; }
> + for (i = image->height; i--; ) {
> + dst = (u32 __iomem *)dst1;
> + src = s;
> +
> + switch (ppw) {
> + case 4: /* 8 bpp */
> + for (j = k; j; j -= 2, ++src) {
> + FB_WRITEL(colortab[(*src >> 4) & bit_mask],
> dst++);
> + FB_WRITEL(colortab[(*src >> 0) & bit_mask],
> dst++);
> + }
> + break;
> + case 2: /* 16 bpp */
> + for (j = k; j; j -= 4, ++src) {
> + FB_WRITEL(colortab[(*src >> 6) & bit_mask],
> dst++);
> + FB_WRITEL(colortab[(*src >> 4) & bit_mask],
> dst++);
> + FB_WRITEL(colortab[(*src >> 2) & bit_mask],
> dst++);
> + FB_WRITEL(colortab[(*src >> 0) & bit_mask],
> dst++);
> + }
> + break;
> + case 1: /* 32 bpp */
> + for (j = k; j; j -= 8, ++src) {
> + FB_WRITEL(colortab[(*src >> 7) & bit_mask],
> dst++);
> + FB_W
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On Tue, Mar 8, 2022 at 11:28 AM Dmitry Osipenko wrote: > > On 3/8/22 19:29, Rob Clark wrote: > > On Tue, Mar 8, 2022 at 5:17 AM Dmitry Osipenko > > wrote: > >> > >> Hello, > >> > >> This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. > >> During OOM, the shrinker will release BOs that are marked as "not needed" > >> by userspace using the new madvise IOCTL. The userspace in this case is > >> the Mesa VirGL driver, it will mark the cached BOs as "not needed", > >> allowing kernel driver to release memory of the cached shmem BOs on lowmem > >> situations, preventing OOM kills. > > > > Will host memory pressure already trigger shrinker in guest? > > The host memory pressure won't trigger shrinker in guest here. This > series will help only with the memory pressure within the guest using a > usual "virgl context". > > Having a host shrinker in a case of "virgl contexts" should be a > difficult problem to solve. Hmm, I think we just need the balloon driver to trigger the shrinker in the guest kernel? I suppose a driver like drm/virtio might want to differentiate between host and guest pressure (ie. consider only objects that have host vs guest storage), but even without that, freeing up memory in the guest when host is under memory pressure seems worthwhile. Maybe I'm over-simplifying? > > This is > > something I'm quite interested in for "virtgpu native contexts" (ie. > > native guest driver with new context type sitting on top of virtgpu), > > In a case of "native contexts" it should be doable, at least I can't see > any obvious problems. The madvise invocations could be passed to the > host using a new virtio-gpu command by the guest's madvise IOCTL > handler, instead-of/in-addition-to handling madvise in the guest's > kernel, and that's it. I think we don't want to do that, because MADV:WILLNEED would be by far the most frequent guest<->host synchronous round trip. So from that perspective tracking madvise state in guest kernel seems quite attractive. If we really can't track madvise state in the guest for dealing with host memory pressure, I think the better option is to introduce MADV:WILLNEED_REPLACE, ie. something to tell the host kernel that the buffer is needed but the previous contents are not (as long as the GPU VA remains the same). With this the host could allocate new pages if needed, and the guest would not need to wait for a reply from host. > > since that isn't using host storage > > s/host/guest ? Yes, sorry, I meant that it is not using guest storage. BR, -R
Re: [PATCH] drm/amdgpu: Add support for drm_privacy_screen
Hi,
On 3/8/22 23:07, Harry Wentland wrote:
>
>
> On 2022-03-08 17:02, Hans de Goede wrote:
>> Hi,
>>
>> On 3/8/22 21:56, Sean Paul wrote:
>>> From: Sean Paul
>>>
>>> This patch adds the necessary hooks to make amdgpu aware of privacy
>>> screens. On devices with privacy screen drivers (such as thinkpad-acpi),
>>> the amdgpu driver will defer probe until it's ready and then sync the sw
>>> and hw state on each commit the connector is involved and enabled.
>>>
>>> Signed-off-by: Sean Paul
>
> The amdgpu_dm portion looks fine to me with Hans' comment
> addressed.
>
> I don't know what the impact of the EPROBE_DEFER in amdgpu_pci_probe
> is.
Since it happens first thing in the probe function and the GPU
has not been touched yet (and thus cannot have been put in
a "bad" state), this should be fine.
The kernel will just keep retrying each time some other drivers
have successfully probed, until the privacy screen is available
and then the probe will continue normally.
Regards,
Hans
>
> Harry
>
>>> ---
>>>
>>> I tested this locally, but am not super confident everything is in the
>>> correct place. Hopefully the intent of the patch is clear and we can
>>> tweak positioning if needed.
>>
>> Thank you for working on this, from a drm-privacy screen
>> pov this looks good, one small remark below.
>>
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
>>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 3 +++
>>> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 16 +++-
>>> 3 files changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index 2ab675123ae3..e2cfae56c020 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -26,6 +26,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include "amdgpu_drv.h"
>>> @@ -1988,6 +1989,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>>> {
>>> struct drm_device *ddev;
>>> struct amdgpu_device *adev;
>>> + struct drm_privacy_screen *privacy_screen;
>>> unsigned long flags = ent->driver_data;
>>> int ret, retry = 0, i;
>>> bool supports_atomic = false;
>>> @@ -2063,6 +2065,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>>> size = pci_resource_len(pdev, 0);
>>> is_fw_fb = amdgpu_is_fw_framebuffer(base, size);
>>>
>>> + /* If the LCD panel has a privacy screen, defer probe until its ready */
>>> + privacy_screen = drm_privacy_screen_get(>dev, NULL);
>>> + if (IS_ERR(privacy_screen) && PTR_ERR(privacy_screen) == -EPROBE_DEFER)
>>> + return -EPROBE_DEFER;
>>> +
>>> + drm_privacy_screen_put(privacy_screen);
>>> +
>>> /* Get rid of things like offb */
>>> ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev,
>>> _kms_driver);
>>> if (ret)
>>> 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 e1d3db3fe8de..9e2bb6523add 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -9781,6 +9781,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
>>> drm_atomic_state *state)
>>> if (acrtc) {
>>> new_crtc_state = drm_atomic_get_new_crtc_state(state,
>>> >base);
>>> old_crtc_state = drm_atomic_get_old_crtc_state(state,
>>> >base);
>>> +
>>> + /* Sync the privacy screen state between hw and sw */
>>> + drm_connector_update_privacy_screen(new_con_state);
>>> }
>>>
>>> /* Skip any modesets/resets */
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>>> index 740435ae3997..e369fc95585e 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>>> @@ -27,6 +27,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include "dm_services.h"
>>> #include "amdgpu.h"
>>> #include "amdgpu_dm.h"
>>> @@ -506,6 +507,7 @@ void amdgpu_dm_initialize_dp_connector(struct
>>> amdgpu_display_manager *dm,
>>>struct amdgpu_dm_connector *aconnector,
>>>int link_index)
>>> {
>>> + struct drm_device *dev = dm->ddev;
>>> struct dc_link_settings max_link_enc_cap = {0};
>>>
>>> aconnector->dm_dp_aux.aux.name =
>>> @@ -519,8 +521,20 @@ void amdgpu_dm_initialize_dp_connector(struct
>>> amdgpu_display_manager *dm,
>>> drm_dp_cec_register_connector(>dm_dp_aux.aux,
>>> >base);
>>>
>>> - if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
>>> + if (aconnector->base.connector_type ==
Re: [PATCH] drm/amdgpu: Add support for drm_privacy_screen
On 2022-03-08 17:02, Hans de Goede wrote:
> Hi,
>
> On 3/8/22 21:56, Sean Paul wrote:
>> From: Sean Paul
>>
>> This patch adds the necessary hooks to make amdgpu aware of privacy
>> screens. On devices with privacy screen drivers (such as thinkpad-acpi),
>> the amdgpu driver will defer probe until it's ready and then sync the sw
>> and hw state on each commit the connector is involved and enabled.
>>
>> Signed-off-by: Sean Paul
The amdgpu_dm portion looks fine to me with Hans' comment
addressed.
I don't know what the impact of the EPROBE_DEFER in amdgpu_pci_probe
is.
Harry
>> ---
>>
>> I tested this locally, but am not super confident everything is in the
>> correct place. Hopefully the intent of the patch is clear and we can
>> tweak positioning if needed.
>
> Thank you for working on this, from a drm-privacy screen
> pov this looks good, one small remark below.
>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 3 +++
>> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 16 +++-
>> 3 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 2ab675123ae3..e2cfae56c020 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -26,6 +26,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include "amdgpu_drv.h"
>> @@ -1988,6 +1989,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>> {
>> struct drm_device *ddev;
>> struct amdgpu_device *adev;
>> +struct drm_privacy_screen *privacy_screen;
>> unsigned long flags = ent->driver_data;
>> int ret, retry = 0, i;
>> bool supports_atomic = false;
>> @@ -2063,6 +2065,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>> size = pci_resource_len(pdev, 0);
>> is_fw_fb = amdgpu_is_fw_framebuffer(base, size);
>>
>> +/* If the LCD panel has a privacy screen, defer probe until its ready */
>> +privacy_screen = drm_privacy_screen_get(>dev, NULL);
>> +if (IS_ERR(privacy_screen) && PTR_ERR(privacy_screen) == -EPROBE_DEFER)
>> +return -EPROBE_DEFER;
>> +
>> +drm_privacy_screen_put(privacy_screen);
>> +
>> /* Get rid of things like offb */
>> ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev,
>> _kms_driver);
>> if (ret)
>> 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 e1d3db3fe8de..9e2bb6523add 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -9781,6 +9781,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
>> drm_atomic_state *state)
>> if (acrtc) {
>> new_crtc_state = drm_atomic_get_new_crtc_state(state,
>> >base);
>> old_crtc_state = drm_atomic_get_old_crtc_state(state,
>> >base);
>> +
>> +/* Sync the privacy screen state between hw and sw */
>> +drm_connector_update_privacy_screen(new_con_state);
>> }
>>
>> /* Skip any modesets/resets */
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> index 740435ae3997..e369fc95585e 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> @@ -27,6 +27,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include "dm_services.h"
>> #include "amdgpu.h"
>> #include "amdgpu_dm.h"
>> @@ -506,6 +507,7 @@ void amdgpu_dm_initialize_dp_connector(struct
>> amdgpu_display_manager *dm,
>> struct amdgpu_dm_connector *aconnector,
>> int link_index)
>> {
>> +struct drm_device *dev = dm->ddev;
>> struct dc_link_settings max_link_enc_cap = {0};
>>
>> aconnector->dm_dp_aux.aux.name =
>> @@ -519,8 +521,20 @@ void amdgpu_dm_initialize_dp_connector(struct
>> amdgpu_display_manager *dm,
>> drm_dp_cec_register_connector(>dm_dp_aux.aux,
>>>base);
>>
>> -if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
>> +if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP) {
>> +struct drm_privacy_screen *privacy_screen;
>> +
>> +/* Reference given up in drm_connector_cleanup() */
>> +privacy_screen = drm_privacy_screen_get(dev->dev, NULL);
>> +if (!IS_ERR(privacy_screen)) {
>> +
>> drm_connector_attach_privacy_screen_provider(>base,
>> +
>> privacy_screen);
>> +} else {
>> +
Re: [PATCH] drm/amdgpu: Add support for drm_privacy_screen
Hi,
On 3/8/22 21:56, Sean Paul wrote:
> From: Sean Paul
>
> This patch adds the necessary hooks to make amdgpu aware of privacy
> screens. On devices with privacy screen drivers (such as thinkpad-acpi),
> the amdgpu driver will defer probe until it's ready and then sync the sw
> and hw state on each commit the connector is involved and enabled.
>
> Signed-off-by: Sean Paul
> ---
>
> I tested this locally, but am not super confident everything is in the
> correct place. Hopefully the intent of the patch is clear and we can
> tweak positioning if needed.
Thank you for working on this, from a drm-privacy screen
pov this looks good, one small remark below.
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 3 +++
> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 16 +++-
> 3 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 2ab675123ae3..e2cfae56c020 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -26,6 +26,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include "amdgpu_drv.h"
> @@ -1988,6 +1989,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> {
> struct drm_device *ddev;
> struct amdgpu_device *adev;
> + struct drm_privacy_screen *privacy_screen;
> unsigned long flags = ent->driver_data;
> int ret, retry = 0, i;
> bool supports_atomic = false;
> @@ -2063,6 +2065,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> size = pci_resource_len(pdev, 0);
> is_fw_fb = amdgpu_is_fw_framebuffer(base, size);
>
> + /* If the LCD panel has a privacy screen, defer probe until its ready */
> + privacy_screen = drm_privacy_screen_get(>dev, NULL);
> + if (IS_ERR(privacy_screen) && PTR_ERR(privacy_screen) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + drm_privacy_screen_put(privacy_screen);
> +
> /* Get rid of things like offb */
> ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev,
> _kms_driver);
> if (ret)
> 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 e1d3db3fe8de..9e2bb6523add 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -9781,6 +9781,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
> drm_atomic_state *state)
> if (acrtc) {
> new_crtc_state = drm_atomic_get_new_crtc_state(state,
> >base);
> old_crtc_state = drm_atomic_get_old_crtc_state(state,
> >base);
> +
> + /* Sync the privacy screen state between hw and sw */
> + drm_connector_update_privacy_screen(new_con_state);
> }
>
> /* Skip any modesets/resets */
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 740435ae3997..e369fc95585e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
> #include "dm_services.h"
> #include "amdgpu.h"
> #include "amdgpu_dm.h"
> @@ -506,6 +507,7 @@ void amdgpu_dm_initialize_dp_connector(struct
> amdgpu_display_manager *dm,
> struct amdgpu_dm_connector *aconnector,
> int link_index)
> {
> + struct drm_device *dev = dm->ddev;
> struct dc_link_settings max_link_enc_cap = {0};
>
> aconnector->dm_dp_aux.aux.name =
> @@ -519,8 +521,20 @@ void amdgpu_dm_initialize_dp_connector(struct
> amdgpu_display_manager *dm,
> drm_dp_cec_register_connector(>dm_dp_aux.aux,
> >base);
>
> - if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
> + if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP) {
> + struct drm_privacy_screen *privacy_screen;
> +
> + /* Reference given up in drm_connector_cleanup() */
> + privacy_screen = drm_privacy_screen_get(dev->dev, NULL);
> + if (!IS_ERR(privacy_screen)) {
> +
> drm_connector_attach_privacy_screen_provider(>base,
> +
> privacy_screen);
> + } else {
> + drm_err(dev, "Error getting privacy screen, ret=%d\n",
> + PTR_ERR(privacy_screen));
This will now log a warning on all devices without a privacy-screen. The i915
code uses this instead:
} else if (PTR_ERR(privacy_screen) !=
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On 3/8/22 17:21, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 03:47:22PM +0100, Marek Vasut wrote:
On 3/8/22 14:49, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 02:27:40PM +0100, Marek Vasut wrote:
On 3/8/22 13:51, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
On 3/8/22 11:07, Jagan Teki wrote:
On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
On 3/8/22 09:03, Jagan Teki wrote:
Hi,
[...]
@@ -314,7 +321,9 @@ static const struct drm_bridge_funcs chipone_bridge_funcs =
{
static int chipone_parse_dt(struct chipone *icn)
{
struct device *dev = icn->dev;
+ struct device_node *endpoint;
struct drm_panel *panel;
+ int dsi_lanes;
int ret;
icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
@@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone *icn)
return PTR_ERR(icn->enable_gpio);
}
+ endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
+ dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
+ icn->host_node = of_graph_get_remote_port_parent(endpoint);
+ of_node_put(endpoint);
+
+ if (!icn->host_node)
+ return -ENODEV;
The non-ports-based OF graph returns a -19 example on the Allwinner
Display pipeline in R16 [1].
We need to have a helper to return host_node for non-ports as I have
done it for drm_of_find_bridge.
[1] https://patchwork.amarulasolutions.com/patch/1805/
The link points to a patch marked "DO NOT MERGE", maybe that patch is
missing the DSI host port@0 OF graph link ? Both port@0 and port@1 are
required, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
What is "non-ports-based OF graph" ?
I don't see drm_of_find_bridge() in linux-next , what is that ?
port@0 is optional as some of the DSI host OF-graph represent the
bridge or panel as child nodes instead of ports. (i think dt-binding
has to fix it to make port@0 optional)
The current upstream DT binding document says:
required:
- port@0
- port@1
So port@0 is mandatory.
In the binding, sure, but fundamentally the DT excerpt Jagan provided is
correct. If the bridge supports DCS, there's no reason to use the OF
graph in the first place: the bridge node will be a child node of the
MIPI-DSI controller (and there's no obligation to use the OF-graph for a
MIPI-DSI controller).
I believe port@0 should be made optional (or downright removed if
MIPI-DCS in the only control bus).
That's out of scope of this series anyway, so Jagan can implement patches
for that mode if needed.
Not really? You can't count on the port@0 being there generally
speaking, so you can't count on data-lanes being there either, which
exactly what you're doing in this patch.
I can because the upstream DT bindings currently say port@0 must be present,
see above. If that requirement should be relaxed, sure, but that's a
separate series.
And another upstream DT bindings say that you don't need them at all.
Which "another upstream DT bindings" do you refer to ?
Yes, there's a conflict. Yes, it's unfortunate. But the generic DSI
binding is far more relevant than a single bridge driver.
That seems to be the wrong way around, how can generic subsystem-wide
binding take precedence over specific driver binding ?
So figuring it out is very much a prerequisite to that series,
especially since those patches effectively make the OF-graph mandatory
in some situations, while it was purely aesthetics before.
The OF-graph is mandatory per the DT bindings of this driver. If you
implement invalid DT which does not contain port@0, it will fail DT
validation.
If this requirement should be relaxed, sure, it can and I don't think it
would be hard to do, but I don't see why that should be part of this
series, which follows the upstream DT binding document for this driver.
If I cannot trust the driver DT bindings to indicate what is and is not
mandatory, what other document can I trust then ...
Re: [PATCH v2] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
Am 2022-03-08 um 16:08 schrieb David Yat Sin:
Export dmabuf handles for GTT BOs so that their contents can be accessed
using SDMA during checkpoint/restore.
Signed-off-by: David Yat Sin
Looks good to me. Please also post a link to the user mode change for this.
Note that the user mode code has not been merged upstream yet. I think
this should be the final cleanup before the user mode CRIU plugin can be
merged with the updated KFD version dependency.
Thanks,
Felix
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12
include/uapi/linux/kfd_ioctl.h | 3 ++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 2c7d76e67ddb..e1e2362841f8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1759,7 +1759,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
goto exit;
}
}
- if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret =
criu_get_prime_handle(_bo->tbo.base,
bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? DRM_RDWR : 0,
@@ -1812,7 +1813,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
exit:
while (ret && bo_index--) {
- if (bo_buckets[bo_index].alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[bo_index].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[bo_index].dmabuf_fd);
}
@@ -2211,7 +2213,8 @@ static int criu_restore_bo(struct kfd_process *p,
pr_debug("map memory was successful for the BO\n");
/* create the dmabuf object and export the bo */
- if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM | KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret = criu_get_prime_handle(_mem->bo->tbo.base, DRM_RDWR,
_bucket->dmabuf_fd);
if (ret)
@@ -2281,7 +2284,8 @@ static int criu_restore_bos(struct kfd_process *p,
exit:
while (ret && i--) {
- if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[i].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[i].dmabuf_fd);
}
kvfree(bo_buckets);
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index b40687bf1014..eb9ff85f8556 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -33,9 +33,10 @@
* - 1.5 - Add SVM API
* - 1.6 - Query clear flags in SVM get_attr API
* - 1.7 - Checkpoint Restore (CRIU) API
+ * - 1.8 - CRIU - Support for SDMA transfers with GTT BOs
*/
#define KFD_IOCTL_MAJOR_VERSION 1
-#define KFD_IOCTL_MINOR_VERSION 7
+#define KFD_IOCTL_MINOR_VERSION 8
struct kfd_ioctl_get_version_args {
__u32 major_version;/* from KFD */
[PATCH v2] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
Export dmabuf handles for GTT BOs so that their contents can be accessed
using SDMA during checkpoint/restore.
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12
include/uapi/linux/kfd_ioctl.h | 3 ++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 2c7d76e67ddb..e1e2362841f8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1759,7 +1759,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
goto exit;
}
}
- if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret =
criu_get_prime_handle(_bo->tbo.base,
bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? DRM_RDWR : 0,
@@ -1812,7 +1813,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
exit:
while (ret && bo_index--) {
- if (bo_buckets[bo_index].alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[bo_index].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[bo_index].dmabuf_fd);
}
@@ -2211,7 +2213,8 @@ static int criu_restore_bo(struct kfd_process *p,
pr_debug("map memory was successful for the BO\n");
/* create the dmabuf object and export the bo */
- if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM | KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret = criu_get_prime_handle(_mem->bo->tbo.base, DRM_RDWR,
_bucket->dmabuf_fd);
if (ret)
@@ -2281,7 +2284,8 @@ static int criu_restore_bos(struct kfd_process *p,
exit:
while (ret && i--) {
- if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[i].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[i].dmabuf_fd);
}
kvfree(bo_buckets);
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index b40687bf1014..eb9ff85f8556 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -33,9 +33,10 @@
* - 1.5 - Add SVM API
* - 1.6 - Query clear flags in SVM get_attr API
* - 1.7 - Checkpoint Restore (CRIU) API
+ * - 1.8 - CRIU - Support for SDMA transfers with GTT BOs
*/
#define KFD_IOCTL_MAJOR_VERSION 1
-#define KFD_IOCTL_MINOR_VERSION 7
+#define KFD_IOCTL_MINOR_VERSION 8
struct kfd_ioctl_get_version_args {
__u32 major_version;/* from KFD */
--
2.17.1
Re: [PATCH] drm/msm/gpu: Fix crash on devices without devfreq support (v2)
On Tue, 8 Mar 2022 at 21:48, Rob Clark wrote: > > From: Rob Clark > > Avoid going down devfreq paths on devices where devfreq is not > initialized. > > v2: Change has_devfreq() logic [Dmitry] > > Reported-by: Linux Kernel Functional Testing > Reported-by: Anders Roxell > Signed-off-by: Rob Clark Reviewed-by: Dmitry Baryshkov > --- > drivers/gpu/drm/msm/msm_gpu_devfreq.c | 30 ++- > 1 file changed, 25 insertions(+), 5 deletions(-) > -- With best wishes Dmitry
[PATCH] drm/amdgpu: Add support for drm_privacy_screen
From: Sean Paul
This patch adds the necessary hooks to make amdgpu aware of privacy
screens. On devices with privacy screen drivers (such as thinkpad-acpi),
the amdgpu driver will defer probe until it's ready and then sync the sw
and hw state on each commit the connector is involved and enabled.
Signed-off-by: Sean Paul
---
I tested this locally, but am not super confident everything is in the
correct place. Hopefully the intent of the patch is clear and we can
tweak positioning if needed.
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 3 +++
.../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 16 +++-
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 2ab675123ae3..e2cfae56c020 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#include
#include "amdgpu_drv.h"
@@ -1988,6 +1989,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
{
struct drm_device *ddev;
struct amdgpu_device *adev;
+ struct drm_privacy_screen *privacy_screen;
unsigned long flags = ent->driver_data;
int ret, retry = 0, i;
bool supports_atomic = false;
@@ -2063,6 +2065,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
size = pci_resource_len(pdev, 0);
is_fw_fb = amdgpu_is_fw_framebuffer(base, size);
+ /* If the LCD panel has a privacy screen, defer probe until its ready */
+ privacy_screen = drm_privacy_screen_get(>dev, NULL);
+ if (IS_ERR(privacy_screen) && PTR_ERR(privacy_screen) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ drm_privacy_screen_put(privacy_screen);
+
/* Get rid of things like offb */
ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev,
_kms_driver);
if (ret)
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 e1d3db3fe8de..9e2bb6523add 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9781,6 +9781,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
if (acrtc) {
new_crtc_state = drm_atomic_get_new_crtc_state(state,
>base);
old_crtc_state = drm_atomic_get_old_crtc_state(state,
>base);
+
+ /* Sync the privacy screen state between hw and sw */
+ drm_connector_update_privacy_screen(new_con_state);
}
/* Skip any modesets/resets */
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 740435ae3997..e369fc95585e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "dm_services.h"
#include "amdgpu.h"
#include "amdgpu_dm.h"
@@ -506,6 +507,7 @@ void amdgpu_dm_initialize_dp_connector(struct
amdgpu_display_manager *dm,
struct amdgpu_dm_connector *aconnector,
int link_index)
{
+ struct drm_device *dev = dm->ddev;
struct dc_link_settings max_link_enc_cap = {0};
aconnector->dm_dp_aux.aux.name =
@@ -519,8 +521,20 @@ void amdgpu_dm_initialize_dp_connector(struct
amdgpu_display_manager *dm,
drm_dp_cec_register_connector(>dm_dp_aux.aux,
>base);
- if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
+ if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP) {
+ struct drm_privacy_screen *privacy_screen;
+
+ /* Reference given up in drm_connector_cleanup() */
+ privacy_screen = drm_privacy_screen_get(dev->dev, NULL);
+ if (!IS_ERR(privacy_screen)) {
+
drm_connector_attach_privacy_screen_provider(>base,
+
privacy_screen);
+ } else {
+ drm_err(dev, "Error getting privacy screen, ret=%d\n",
+ PTR_ERR(privacy_screen));
+ }
return;
+ }
dc_link_dp_get_max_link_enc_cap(aconnector->dc_link, _link_enc_cap);
aconnector->mst_mgr.cbs = _mst_cbs;
--
Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH v5 3/5] drm/msm/dp: set stream_pixel rate directly
Quoting Dmitry Baryshkov (2022-03-03 23:58:58)
> On Fri, 4 Mar 2022 at 07:31, Stephen Boyd wrote:
> >
> > Quoting Dmitry Baryshkov (2022-03-03 20:23:06)
> > > On Fri, 4 Mar 2022 at 01:32, Stephen Boyd wrote:
> > > >
> > > > Quoting Dmitry Baryshkov (2022-02-16 21:55:27)
> > > > > The only clock for which we set the rate is the "stream_pixel". Rather
> > > > > than storing the rate and then setting it by looping over all the
> > > > > clocks, set the clock rate directly.
> > > > >
> > > > > Signed-off-by: Dmitry Baryshkov
> > > > [...]
> > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> > > > > b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> > > > > index 07f6bf7e1acb..8e6361dedd77 100644
> > > > > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> > > > > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> > > > > @@ -1315,7 +1315,7 @@ static void dp_ctrl_set_clock_rate(struct
> > > > > dp_ctrl_private *ctrl,
> > > > > DRM_DEBUG_DP("setting rate=%lu on clk=%s\n", rate, name);
> > > > >
> > > > > if (num)
> > > > > - cfg->rate = rate;
> > > > > + clk_set_rate(cfg->clk, rate);
> > > >
> > > > This looks bad. From what I can tell we set the rate of the pixel clk
> > > > after enabling the phy and configuring it. See the order of operations
> > > > in dp_ctrl_enable_mainlink_clocks() and note how dp_power_clk_enable()
> > > > is the one that eventually sets a rate through dp_power_clk_set_rate()
> > > >
> > > > dp_ctrl_set_clock_rate(ctrl, DP_CTRL_PM, "ctrl_link",
> > > > ctrl->link->link_params.rate *
> > > > 1000);
> > > >
> > > > phy_configure(phy, _io->phy_opts);
> > > > phy_power_on(phy);
> > > >
> > > > ret = dp_power_clk_enable(ctrl->power, DP_CTRL_PM, true);
> > >
> > > This code has been changed in the previous patch.
> > >
> > > Let's get back a bit.
> > > Currently dp_ctrl_set_clock_rate() doesn't change the clock rate. It
> > > just stores the rate in the config so that later the sequence of
> > > dp_power_clk_enable() -> dp_power_clk_set_rate() ->
> > > [dp_power_clk_set_link_rate() -> dev_pm_opp_set_rate() or
> > > msm_dss_clk_set_rate() -> clk_set_rate()] will use that.
> > >
> > > There are only two users of dp_ctrl_set_clock_rate():
> > > - dp_ctrl_enable_mainlink_clocks(), which you have quoted above.
> > > This case is handled in the patch 1 from this series. It makes
> >
> > Patch 1 form this series says DP is unaffected. Huh?
> >
> > > dp_ctrl_enable_mainlink_clocks() call dev_pm_opp_set_rate() directly
> > > without storing (!) the rate in the config, calling
> > > phy_configure()/phy_power_on() and then setting the opp via the
> > > sequence of calls specified above
>
> Note, this handles the "ctrl_link" clock.
>
> > >
> > > - dp_ctrl_enable_stream_clocks(), which calls dp_power_clk_enable()
> > > immediately afterwards. This call would set the stream_pixel rate
> > > while enabling stream clocks. As far as I can see, the stream_pixel is
> > > the only stream clock. So this patch sets the clock rate without
> > > storing in the interim configuration data.
> > >
> > > Could you please clarify, what exactly looks bad to you?
> > >
>
> Note, this handles the "stream_pixel" clock.
>
> >
> > I'm concerned about the order of operations changing between the
> > phy being powered on and the pixel clk frequency being set. From what I
> > recall the pixel clk rate operations depend on the phy frequency being
> > set (which is done through phy_configure?) so if we call clk_set_rate()
> > on the pixel clk before the phy is set then the clk frequency will be
> > calculated badly and probably be incorrect.
>
> But the order of operations is mostly unchanged. The only major change
> is that the opp point is now set before calling the
> phy_configure()/phy_power_on()
Yes that's my concern. The qmp phy driver has a couple clk_set_rate()
calls in the .configure_dp_phy callback. That is called from
phy_power_on() (see qcom_qmp_phy_power_on() and qcom_qmp_phy_dp_ops).
Looking at qcom_qmp_v3_phy_configure_dp_phy() it does
clk_set_rate(dp_clks->dp_link_hw.clk, dp_opts->link_rate * 10);
clk_set_rate(dp_clks->dp_pixel_hw.clk, pixel_freq);
and I believe the child of dp_pixel_hw is find_clock("stream_pixel").
Looks like that is DISP_CC_MDSS_DP_PIXEL_CLK which is
disp_cc_mdss_dp_pixel_clk_src for the rate settable part. That has
clk_dp_ops which is clk_rcg2_dp_set_rate() for the set rate part. That
wants the parent clk frequency to be something non-zero to use in
rational_best_approximation(). If the clk_set_rate("stream_pixel") call
is made before phy_power_on() then the parent_rate in
clk_rcg2_dp_set_rate() won't be valid and the pixel clk frequency will
be wrong.
>
> For the pixel clock the driver has:
> static int dp_ctrl_enable_stream_clocks(struct dp_ctrl_private *ctrl)
> {
> int ret = 0;
>
> dp_ctrl_set_clock_rate(ctrl, DP_STREAM_PM, "stream_pixel",
>
Re: [PATCH 2/2] drm: ssd130x: Always apply segment remap setting
On 3/8/22 17:07, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai
>
> Currently the ssd130x driver only sets the segment remap setting when
> the device tree requests it; it however does not clear the setting if
> it is not requested. This leads to the setting incorrectly persisting
> if the hardware is always on and has no reset GPIO wired. This might
> happen when a developer is trying to find the correct settings for an
> unknown module, and cause the developer to get confused because the
> settings from the device tree are not consistently applied.
>
> Make the driver apply the segment remap setting consistently, setting
> the value correctly based on the device tree setting. This also makes
> this setting's behavior consistent with the other settings, which are
> always applied.
>
Nice catch. This is certainly much better. Thanks!
> Fixes: a61732e80867 ("drm: Add driver for Solomon SSD130x OLED displays")
> Signed-off-by: Chen-Yu Tsai
> ---
Acked-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v3 5/5] drm/msm: allow compile time selection of driver components
Quoting Dmitry Baryshkov (2022-03-03 19:21:06) > MSM DRM driver already allows one to compile out the DP or DSI support. > Add support for disabling other features like MDP4/MDP5/DPU drivers or > direct HDMI output support. > > Suggested-by: Stephen Boyd > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v3 4/5] drm/msm: stop using device's match data pointer
Quoting Dmitry Baryshkov (2022-03-03 19:21:05) > Let's make the match's data pointer a (sub-)driver's private data. The > only user currently is the msm_drm_init() function, using this data to > select kms_init callback. Pass this callback through the driver's > private data instead. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v3 3/5] drm/msm: split the main platform driver
Quoting Dmitry Baryshkov (2022-03-03 19:21:04)
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 857eefbb8649..c89de88ed2d1 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -255,3 +258,170 @@ struct msm_mdss *msm_mdss_init(struct platform_device
> *pdev, bool is_mdp5)
[...]
> +
> +static int mdss_probe(struct platform_device *pdev)
> +{
> + struct msm_mdss *mdss;
> + struct msm_drm_private *priv;
> + int mdp_ver = get_mdp_ver(pdev);
> + struct device *mdp_dev;
> + struct device *dev = >dev;
> + int ret;
> +
> + priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
Is this set here so that msm_mdss_init() can use it? Can we pass 'priv'
as an argument to that function if so?
> +
> + mdss = msm_mdss_init(pdev, mdp_ver == KMS_MDP5);
> + if (IS_ERR(mdss)) {
> + ret = PTR_ERR(mdss);
> + platform_set_drvdata(pdev, NULL);
Is this platform_set_drvdata to NULL really necessary? It would be nice
to skip it so the mental load of this probe is lower.
> +
> + return ret;
> + }
> +
Re: [PATCH 1/2] drm: ssd130x: Fix COM scan direction register mask
Hello Chen-Yu,
Thanks a lot for your patch.
On 3/8/22 17:07, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai
>
> The SSD130x's command to toggle COM scan direction uses bit 3 and only
> bit 3 to set the direction of the scanout. The driver has an incorrect
> GENMASK(3, 2), causing the setting to be set on bit 2, rendering it
> ineffective.
>
> Fix the mask to only bit 3, so that the requested setting is applied
> correctly.
>
Sigh, you are correct. I thought that triple checked the datasheet
when writing this but I got it wrong anyways...
> Fixes: a61732e80867 ("drm: Add driver for Solomon SSD130x OLED displays")
> Signed-off-by: Chen-Yu Tsai
> ---
Acked-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v3 2/5] drm/msm: remove extra indirection for msm_mdss
Quoting Dmitry Baryshkov (2022-03-03 19:21:03) > Since now there is just one mdss subdriver, drop all the indirection, > make msm_mdss struct completely opaque (and defined inside msm_mdss.c) > and call mdss functions directly. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v3 1/5] drm/msm: unify MDSS drivers
Quoting Dmitry Baryshkov (2022-03-03 19:21:02) > MDP5 and DPU1 both provide the driver handling the MDSS region, which > handles the irq domain and (incase of DPU1) adds some init for the UBWC > controller. Unify those two pieces of code into a common driver. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH] drm/msm/adreno: fix cast in adreno_get_param()
Quoting Dan Carpenter (2022-03-07 05:31:05)
> These casts need to happen before the shift. The only time it would
> matter would be if "rev.core" is >= 128. In that case the sign bit
> would be extended and we do not want that.
>
> Fixes: afab9d91d872 ("drm/msm/adreno: Expose speedbin to userspace")
> Signed-off-by: Dan Carpenter
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
Am 2022-03-08 um 14:11 schrieb David Yat Sin:
Export dmabuf handles for GTT BOs so that their contents can be accessed
using SDMA during checkpoint/restore.
This deserves a minor version bump. The plugin should depend on that
bumped version when it starts using dmabuf handles for GTT BOs.
Regards,
Felix
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 2c7d76e67ddb..e1e2362841f8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1759,7 +1759,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
goto exit;
}
}
- if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret =
criu_get_prime_handle(_bo->tbo.base,
bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? DRM_RDWR : 0,
@@ -1812,7 +1813,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
exit:
while (ret && bo_index--) {
- if (bo_buckets[bo_index].alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[bo_index].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[bo_index].dmabuf_fd);
}
@@ -2211,7 +2213,8 @@ static int criu_restore_bo(struct kfd_process *p,
pr_debug("map memory was successful for the BO\n");
/* create the dmabuf object and export the bo */
- if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM | KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret = criu_get_prime_handle(_mem->bo->tbo.base, DRM_RDWR,
_bucket->dmabuf_fd);
if (ret)
@@ -2281,7 +2284,8 @@ static int criu_restore_bos(struct kfd_process *p,
exit:
while (ret && i--) {
- if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[i].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[i].dmabuf_fd);
}
kvfree(bo_buckets);
[PATCH] video: fbdev: kyro: make read-only array ODValues static const
Don't populate the read-only array ODValues on the stack but
instead make it static const. Also makes the object code a little
smaller.
Signed-off-by: Colin Ian King
---
drivers/video/fbdev/kyro/STG4000InitDevice.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/kyro/STG4000InitDevice.c
b/drivers/video/fbdev/kyro/STG4000InitDevice.c
index 21875d3c2dc2..ffeb355c8b50 100644
--- a/drivers/video/fbdev/kyro/STG4000InitDevice.c
+++ b/drivers/video/fbdev/kyro/STG4000InitDevice.c
@@ -124,7 +124,7 @@ u32 ProgramClock(u32 refClock,
u32 ulScore, ulPhaseScore, ulVcoScore;
u32 ulTmp = 0, ulVCO;
u32 ulScaleClockReq, ulMinClock, ulMaxClock;
- u32 ODValues[] = { 1, 2, 0 };
+ static const u32 ODValues[] = { 1, 2, 0 };
/* Translate clock in Hz */
coreClock *= 100; /* in Hz */
--
2.35.1
Re: [PATCH 1/3] drm/bridge: Add MAINTAINERS entry for DRM drivers for bridge chip bindings
Hi Doug, Thank you for the patch. On Tue, Mar 08, 2022 at 11:06:57AM -0800, Douglas Anderson wrote: > The bindings for bridge chips should also get the same maintainers > entry so the right people get notified about bindings changes. > > Signed-off-by: Douglas Anderson Reviewed-by: Laurent Pinchart > --- > > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0216d2ffe728..a73179d55d00 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6403,6 +6403,7 @@ R: Jonas Karlman > R: Jernej Skrabec > S: Maintained > T: git git://anongit.freedesktop.org/drm/drm-misc > +F: Documentation/devicetree/bindings/display/bridge/ > F: drivers/gpu/drm/bridge/ > > DRM DRIVERS FOR EXYNOS -- Regards, Laurent Pinchart
Re: [PATCH v5 1/5] arm64/dts/qcom/sc7280: remove assigned-clock-rate property for mdp clk
Quoting Vinod Polimera (2022-03-08 08:54:56)
> Kernel clock driver assumes that initial rate is the
> max rate for that clock and was not allowing it to scale
> beyond the assigned clock value.
How? I see ftbl_disp_cc_mdss_mdp_clk_src[] has multiple frequencies and
clk_rcg2_shared_ops so it doesn't look like anything in the clk driver
is preventing the frequency from changing beyond the assigned value.
>
> Drop the assigned clock rate property and vote on the mdp clock as per
> calculated value during the usecase.
>
> Changes in v2:
> - Remove assigned-clock-rate property and set mdp clk during resume sequence.
> - Add fixes tag.
>
> Changes in v3:
> - Remove extra line after fixes tag.(Stephen Boyd)
This changelog should be removed.
>
> Fixes: 62fbdce91("arm64: dts: qcom: sc7280: add display dt nodes")
I thought folks were saying that this is bad to keep? I don't really
mind either way, but I guess it's better to drop the fixes tag because
this is largely a performance improvement?
> Signed-off-by: Vinod Polimera
> Reviewed-by: Stephen Boyd
[PATCH v2 02/12] drm/gma500: Acquire reservation lock for GEM objects
Protect concurrent access to struct psb_gem_object by acquiring the GEM object's reservation lock; as it's supposed to be. The use of the GTT mutex can now be moved into GTT code. Signed-off-by: Thomas Zimmermann Acked-by: Patrik Jakobsson --- drivers/gpu/drm/gma500/gem.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c index fe7d242567c0..026ce11f7460 100644 --- a/drivers/gpu/drm/gma500/gem.c +++ b/drivers/gpu/drm/gma500/gem.c @@ -31,6 +31,10 @@ int psb_gem_pin(struct psb_gem_object *pobj) unsigned int npages; int ret; + ret = dma_resv_lock(obj->resv, NULL); + if (drm_WARN_ONCE(dev, ret, "dma_resv_lock() failed, ret=%d\n", ret)) + return ret; + mutex_lock(_priv->gtt_mutex); if (pobj->in_gart || pobj->stolen) @@ -56,11 +60,13 @@ int psb_gem_pin(struct psb_gem_object *pobj) out: ++pobj->in_gart; mutex_unlock(_priv->gtt_mutex); + dma_resv_unlock(obj->resv); return 0; err_mutex_unlock: mutex_unlock(_priv->gtt_mutex); + dma_resv_unlock(obj->resv); return ret; } @@ -71,6 +77,11 @@ void psb_gem_unpin(struct psb_gem_object *pobj) struct drm_psb_private *dev_priv = to_drm_psb_private(dev); u32 gpu_base = dev_priv->gtt.gatt_start; unsigned long npages; + int ret; + + ret = dma_resv_lock(obj->resv, NULL); + if (drm_WARN_ONCE(dev, ret, "dma_resv_lock() failed, ret=%d\n", ret)) + return; mutex_lock(_priv->gtt_mutex); @@ -95,6 +106,7 @@ void psb_gem_unpin(struct psb_gem_object *pobj) out: mutex_unlock(_priv->gtt_mutex); + dma_resv_unlock(obj->resv); } static vm_fault_t psb_gem_fault(struct vm_fault *vmf); -- 2.35.1
[PATCH v2 03/12] drm/gma500: Move GTT locking into GTT helpers
Acquire the GTT mutex in psb_gtt_{insert,remove}_pages(). Remove
locking from callers. Also remove the GTT locking around the resume
code. Resume does not run concurrently with other GTT operations.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gem.c | 11 ++-
drivers/gpu/drm/gma500/gtt.c | 24 +++-
2 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 026ce11f7460..2f44535f58e7 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -35,15 +35,13 @@ int psb_gem_pin(struct psb_gem_object *pobj)
if (drm_WARN_ONCE(dev, ret, "dma_resv_lock() failed, ret=%d\n", ret))
return ret;
- mutex_lock(_priv->gtt_mutex);
-
if (pobj->in_gart || pobj->stolen)
goto out; /* already mapped */
pages = drm_gem_get_pages(obj);
if (IS_ERR(pages)) {
ret = PTR_ERR(pages);
- goto err_mutex_unlock;
+ goto err_dma_resv_unlock;
}
npages = obj->size / PAGE_SIZE;
@@ -59,13 +57,11 @@ int psb_gem_pin(struct psb_gem_object *pobj)
out:
++pobj->in_gart;
- mutex_unlock(_priv->gtt_mutex);
dma_resv_unlock(obj->resv);
return 0;
-err_mutex_unlock:
- mutex_unlock(_priv->gtt_mutex);
+err_dma_resv_unlock:
dma_resv_unlock(obj->resv);
return ret;
}
@@ -83,8 +79,6 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
if (drm_WARN_ONCE(dev, ret, "dma_resv_lock() failed, ret=%d\n", ret))
return;
- mutex_lock(_priv->gtt_mutex);
-
WARN_ON(!pobj->in_gart);
--pobj->in_gart;
@@ -105,7 +99,6 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
pobj->pages = NULL;
out:
- mutex_unlock(_priv->gtt_mutex);
dma_resv_unlock(obj->resv);
}
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 309ffe921bfd..4202e88e5f84 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -74,11 +74,7 @@ static u32 __iomem *psb_gtt_entry(struct drm_psb_private
*pdev, const struct res
return pdev->gtt_map + (offset >> PAGE_SHIFT);
}
-/*
- * Take our preallocated GTT range and insert the GEM object into
- * the GTT. This is protected via the gtt mutex which the caller
- * must hold.
- */
+/* Acquires GTT mutex internally. */
void psb_gtt_insert_pages(struct drm_psb_private *pdev, const struct resource
*res,
struct page **pages)
{
@@ -86,6 +82,8 @@ void psb_gtt_insert_pages(struct drm_psb_private *pdev, const
struct resource *r
u32 __iomem *gtt_slot;
u32 pte;
+ mutex_lock(>gtt_mutex);
+
/* Write our page entries into the GTT itself */
npages = resource_size(res) >> PAGE_SHIFT;
@@ -98,19 +96,19 @@ void psb_gtt_insert_pages(struct drm_psb_private *pdev,
const struct resource *r
/* Make sure all the entries are set before we return */
ioread32(gtt_slot - 1);
+
+ mutex_unlock(>gtt_mutex);
}
-/*
- * Remove a preallocated GTT range from the GTT. Overwrite all the
- * page table entries with the dummy page. This is protected via the gtt
- * mutex which the caller must hold.
- */
+/* Acquires GTT mutex internally. */
void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource
*res)
{
resource_size_t npages, i;
u32 __iomem *gtt_slot;
u32 pte;
+ mutex_lock(>gtt_mutex);
+
/* Install scratch page for the resource */
pte = psb_gtt_mask_pte(page_to_pfn(pdev->scratch_page),
PSB_MMU_CACHED_MEMORY);
@@ -123,6 +121,8 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev,
const struct resource *r
/* Make sure all the entries are set before we return */
ioread32(gtt_slot - 1);
+
+ mutex_unlock(>gtt_mutex);
}
static void psb_gtt_alloc(struct drm_device *dev)
@@ -306,8 +306,6 @@ int psb_gtt_restore(struct drm_device *dev)
struct psb_gem_object *pobj;
unsigned int restored = 0, total = 0, size = 0;
- /* On resume, the gtt_mutex is already initialized */
- mutex_lock(_priv->gtt_mutex);
psb_gtt_init(dev, 1);
while (r != NULL) {
@@ -325,7 +323,7 @@ int psb_gtt_restore(struct drm_device *dev)
r = r->sibling;
total++;
}
- mutex_unlock(_priv->gtt_mutex);
+
DRM_DEBUG_DRIVER("Restored %u of %u gtt ranges (%u KB)", restored,
total, (size / 1024));
--
2.35.1
[PATCH v2 12/12] drm/gma500: Move GTT memory-range setup into helper
Move the setup code for GTT/GATT memory ranges into a new helper and
call the function from psb_gtt_init() and psb_gtt_resume(). Removes
code duplication.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/gtt.c | 153 +++
1 file changed, 64 insertions(+), 89 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 83d9a9f7c73c..379bc218aa6b 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -182,68 +182,91 @@ static void psb_gtt_clear(struct drm_psb_private *pdev)
(void)ioread32(pdev->gtt_map + i - 1);
}
-int psb_gtt_init(struct drm_device *dev)
+static void psb_gtt_init_ranges(struct drm_psb_private *dev_priv)
{
- struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct drm_device *dev = _priv->dev;
struct pci_dev *pdev = to_pci_dev(dev->dev);
struct psb_gtt *pg = _priv->gtt;
- unsigned gtt_pages;
- int ret;
-
- mutex_init(_priv->gtt_mutex);
-
- ret = psb_gtt_enable(dev_priv);
- if (ret)
- goto err_mutex_destroy;
+ resource_size_t gtt_phys_start, mmu_gatt_start, gtt_start, gtt_pages,
+ gatt_start, gatt_pages;
+ struct resource *gtt_mem;
/* The root resource we allocate address space from */
- pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
+ gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
/*
-* The video mmu has a hw bug when accessing 0x0D000.
-* Make gatt start at 0x0e000,. This doesn't actually
-* matter for us but may do if the video acceleration ever
-* gets opened up.
+* The video MMU has a HW bug when accessing 0x0d000. Make
+* GATT start at 0x0e000. This doesn't actually matter for
+* us now, but maybe will if the video acceleration ever gets
+* opened up.
*/
- pg->mmu_gatt_start = 0xE000;
+ mmu_gatt_start = 0xe000;
+
+ gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
+ gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE) >> PAGE_SHIFT;
- pg->gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
- gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE)
- >> PAGE_SHIFT;
/* CDV doesn't report this. In which case the system has 64 gtt pages */
- if (pg->gtt_start == 0 || gtt_pages == 0) {
+ if (!gtt_start || !gtt_pages) {
dev_dbg(dev->dev, "GTT PCI BAR not initialized.\n");
gtt_pages = 64;
- pg->gtt_start = dev_priv->pge_ctl;
+ gtt_start = dev_priv->pge_ctl;
}
- pg->gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
- pg->gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE)
- >> PAGE_SHIFT;
- dev_priv->gtt_mem = >resource[PSB_GATT_RESOURCE];
+ gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
+ gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE) >> PAGE_SHIFT;
- if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
+ if (!gatt_pages || !gatt_start) {
static struct resource fudge; /* Preferably peppermint */
- /* This can occur on CDV systems. Fudge it in this case.
- We really don't care what imaginary space is being allocated
- at this point */
+
+ /*
+* This can occur on CDV systems. Fudge it in this case. We
+* really don't care what imaginary space is being allocated
+* at this point.
+*/
dev_dbg(dev->dev, "GATT PCI BAR not initialized.\n");
- pg->gatt_start = 0x4000;
- pg->gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
- /* This is a little confusing but in fact the GTT is providing
- a view from the GPU into memory and not vice versa. As such
- this is really allocating space that is not the same as the
- CPU address space on CDV */
+ gatt_start = 0x4000;
+ gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
+
+ /*
+* This is a little confusing but in fact the GTT is providing
+* a view from the GPU into memory and not vice versa. As such
+* this is really allocating space that is not the same as the
+* CPU address space on CDV.
+*/
fudge.start = 0x4000;
fudge.end = 0x4000 + 128 * 1024 * 1024 - 1;
fudge.name = "fudge";
fudge.flags = IORESOURCE_MEM;
- dev_priv->gtt_mem =
+
+ gtt_mem =
+ } else {
+ gtt_mem
[PATCH v2 04/12] drm/gma500: Remove struct psb_gtt.sem sempahore
The semaphore at struct psb_mmu_driver.sem protects access to the MMU
fields. Additional locking with struct psb_gtt.sem is unnecessary. Remove
the field and related code.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gtt.c | 7 ---
drivers/gpu/drm/gma500/gtt.h | 1 -
drivers/gpu/drm/gma500/psb_drv.c | 4
3 files changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 4202e88e5f84..c7b7cb1f2d13 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,12 +125,6 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev,
const struct resource *r
mutex_unlock(>gtt_mutex);
}
-static void psb_gtt_alloc(struct drm_device *dev)
-{
- struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
- init_rwsem(_priv->gtt.sem);
-}
-
void psb_gtt_takedown(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
@@ -166,7 +160,6 @@ int psb_gtt_init(struct drm_device *dev, int resume)
if (!resume) {
mutex_init(_priv->gtt_mutex);
mutex_init(_priv->mmap_mutex);
- psb_gtt_alloc(dev);
}
pg = _priv->gtt;
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index ff1dcdd1ff52..31500533ac45 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -22,7 +22,6 @@ struct psb_gtt {
unsigned gatt_pages;
unsigned long stolen_size;
unsigned long vram_stolen_size;
- struct rw_semaphore sem;
};
/* Exported functions */
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index eeb681be9c95..7227a8e44d23 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -184,13 +184,11 @@ static void psb_driver_unload(struct drm_device *dev)
if (dev_priv->mmu) {
struct psb_gtt *pg = _priv->gtt;
- down_read(>sem);
psb_mmu_remove_pfn_sequence(
psb_mmu_get_default_pd
(dev_priv->mmu),
pg->mmu_gatt_start,
dev_priv->vram_stolen_size >> PAGE_SHIFT);
- up_read(>sem);
psb_mmu_driver_takedown(dev_priv->mmu);
dev_priv->mmu = NULL;
}
@@ -345,12 +343,10 @@ static int psb_driver_load(struct drm_device *dev,
unsigned long flags)
return ret;
/* Add stolen memory to SGX MMU */
- down_read(>sem);
ret = psb_mmu_insert_pfn_sequence(psb_mmu_get_default_pd(dev_priv->mmu),
dev_priv->stolen_base >> PAGE_SHIFT,
pg->gatt_start,
pg->stolen_size >> PAGE_SHIFT, 0);
- up_read(>sem);
psb_mmu_set_pd_context(psb_mmu_get_default_pd(dev_priv->mmu), 0);
psb_mmu_set_pd_context(dev_priv->pf_pd, 1);
--
2.35.1
[PATCH v2 08/12] drm/gma500: Split GTT init/resume/fini into GTT and GEM functions
The GTT init, fini and resume functions contain both, GTT and GEM,
code. Split each into a separate GTT and a GEM function. The GEM
code is responsible for mmap_mutex and the stolen memory area. The
rest of the functionality is left in GTT functions.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gtt.c | 118 ---
drivers/gpu/drm/gma500/gtt.h | 3 +
drivers/gpu/drm/gma500/psb_drv.c | 4 ++
3 files changed, 83 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 99c644a5c5cb..0e99774c7e59 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,19 +125,26 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev,
const struct resource *r
mutex_unlock(>gtt_mutex);
}
+void psb_gem_mm_fini(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+
+ iounmap(dev_priv->vram_addr);
+
+ mutex_destroy(_priv->mmap_mutex);
+}
+
void psb_gtt_fini(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
- iounmap(dev_priv->vram_addr);
iounmap(dev_priv->gtt_map);
pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
(void)PSB_RVDC32(PSB_PGETBL_CTL);
- mutex_destroy(_priv->mmap_mutex);
mutex_destroy(_priv->gtt_mutex);
}
@@ -211,12 +218,10 @@ int psb_gtt_init(struct drm_device *dev)
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
unsigned gtt_pages;
- unsigned long stolen_size, vram_stolen_size;
struct psb_gtt *pg;
int ret = 0;
mutex_init(_priv->gtt_mutex);
- mutex_init(_priv->mmap_mutex);
pg = _priv->gtt;
@@ -274,22 +279,8 @@ int psb_gtt_init(struct drm_device *dev)
dev_priv->gtt_mem =
}
- pci_read_config_dword(pdev, PSB_BSM, _priv->stolen_base);
- vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base
- - PAGE_SIZE;
-
- stolen_size = vram_stolen_size;
-
- dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
- dev_priv->stolen_base, vram_stolen_size / 1024);
-
pg->gtt_pages = gtt_pages;
- pg->stolen_size = stolen_size;
- dev_priv->vram_stolen_size = vram_stolen_size;
- /*
-* Map the GTT and the stolen memory area
-*/
dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages <<
PAGE_SHIFT);
if (!dev_priv->gtt_map) {
dev_err(dev->dev, "Failure to map gtt.\n");
@@ -297,26 +288,54 @@ int psb_gtt_init(struct drm_device *dev)
goto err_gtt_disable;
}
+ psb_gtt_clear(dev_priv);
+
+ return 0;
+
+err_gtt_disable:
+ pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+ PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+ (void)PSB_RVDC32(PSB_PGETBL_CTL);
+ mutex_destroy(_priv->gtt_mutex);
+ return ret;
+}
+
+int psb_gem_mm_init(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
+ unsigned long stolen_size, vram_stolen_size;
+ struct psb_gtt *pg;
+ int ret;
+
+ mutex_init(_priv->mmap_mutex);
+
+ pg = _priv->gtt;
+
+ pci_read_config_dword(pdev, PSB_BSM, _priv->stolen_base);
+ vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base -
PAGE_SIZE;
+
+ stolen_size = vram_stolen_size;
+
+ dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
+ dev_priv->stolen_base, vram_stolen_size / 1024);
+
+ pg->stolen_size = stolen_size;
+ dev_priv->vram_stolen_size = vram_stolen_size;
+
dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
if (!dev_priv->vram_addr) {
dev_err(dev->dev, "Failure to map stolen base.\n");
ret = -ENOMEM;
- goto err_iounmap;
+ goto err_mutex_destroy;
}
- psb_gtt_clear(dev_priv);
psb_gtt_populate_stolen(dev_priv);
return 0;
-err_iounmap:
- iounmap(dev_priv->gtt_map);
-err_gtt_disable:
- pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
- PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
- (void)PSB_RVDC32(PSB_PGETBL_CTL);
+err_mutex_destroy:
mutex_destroy(_priv->mmap_mutex);
- mutex_destroy(_priv->gtt_mutex);
return ret;
}
@@ -325,9 +344,8 @@ static int psb_gtt_resume(struct drm_device *dev)
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev =
[PATCH v2 09/12] drm/gma500: Inline psb_gtt_restore()
Inline psb_gtt_restore() into its only caller in power.c.
Perform the GTT restoration in psb_gem_mm_resume(). The restoration
step is part of GEM anyway and will be moved over at some point.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gtt.c | 15 ++-
drivers/gpu/drm/gma500/gtt.h | 3 ++-
drivers/gpu/drm/gma500/power.c | 4 +++-
3 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 0e99774c7e59..9e1b19fcea28 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -339,7 +339,7 @@ int psb_gem_mm_init(struct drm_device *dev)
return ret;
}
-static int psb_gtt_resume(struct drm_device *dev)
+int psb_gtt_resume(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -422,7 +422,7 @@ static int psb_gtt_resume(struct drm_device *dev)
return ret;
}
-static int psb_gem_mm_resume(struct drm_device *dev)
+int psb_gem_mm_resume(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -445,17 +445,6 @@ static int psb_gem_mm_resume(struct drm_device *dev)
}
psb_gtt_populate_stolen(dev_priv);
-
- return 0;
-}
-
-int psb_gtt_restore(struct drm_device *dev)
-{
- struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-
- psb_gtt_resume(dev);
- psb_gem_mm_resume(dev);
-
psb_gtt_populate_resources(dev_priv);
return 0;
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index 45e1926dfce1..9a6d79200dce 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -27,7 +27,7 @@ struct psb_gtt {
/* Exported functions */
int psb_gtt_init(struct drm_device *dev);
void psb_gtt_fini(struct drm_device *dev);
-extern int psb_gtt_restore(struct drm_device *dev);
+int psb_gtt_resume(struct drm_device *dev);
int psb_gtt_allocate_resource(struct drm_psb_private *pdev, struct resource
*res,
const char *name, resource_size_t size,
resource_size_t align,
@@ -39,5 +39,6 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev, const
struct resource *r
int psb_gem_mm_init(struct drm_device *dev);
void psb_gem_mm_fini(struct drm_device *dev);
+int psb_gem_mm_resume(struct drm_device *dev);
#endif
diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
index d2a46d96e746..28e472b31698 100644
--- a/drivers/gpu/drm/gma500/power.c
+++ b/drivers/gpu/drm/gma500/power.c
@@ -112,7 +112,9 @@ static void gma_resume_display(struct pci_dev *pdev)
pci_write_config_word(pdev, PSB_GMCH_CTRL,
dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
- psb_gtt_restore(dev); /* Rebuild our GTT mappings */
+ /* Rebuild our GTT mappings */
+ psb_gtt_resume(dev);
+ psb_gem_mm_resume(dev);
dev_priv->ops->restore_regs(dev);
}
--
2.35.1
[PATCH v2 01/12] drm/gma500: Remove struct psb_gem_object.npage
Calculate the number of pages in the BO's backing storage from
the size. Remove the npage field.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gem.c | 9 +
drivers/gpu/drm/gma500/gem.h | 1 -
drivers/gpu/drm/gma500/gma_display.c | 8 +++-
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 8d65af80bb08..fe7d242567c0 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -51,7 +51,6 @@ int psb_gem_pin(struct psb_gem_object *pobj)
(gpu_base + pobj->offset), npages, 0, 0,
PSB_MMU_CACHED_MEMORY);
- pobj->npage = npages;
pobj->pages = pages;
out:
@@ -71,6 +70,7 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
struct drm_device *dev = obj->dev;
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
u32 gpu_base = dev_priv->gtt.gatt_start;
+ unsigned long npages;
mutex_lock(_priv->gtt_mutex);
@@ -81,16 +81,17 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
if (pobj->in_gart || pobj->stolen)
goto out;
+ npages = obj->size / PAGE_SIZE;
+
psb_mmu_remove_pages(psb_mmu_get_default_pd(dev_priv->mmu),
-(gpu_base + pobj->offset), pobj->npage, 0, 0);
+(gpu_base + pobj->offset), npages, 0, 0);
psb_gtt_remove_pages(dev_priv, >resource);
/* Reset caching flags */
- set_pages_array_wb(pobj->pages, pobj->npage);
+ set_pages_array_wb(pobj->pages, npages);
drm_gem_put_pages(obj, pobj->pages, true, false);
pobj->pages = NULL;
- pobj->npage = 0;
out:
mutex_unlock(_priv->gtt_mutex);
diff --git a/drivers/gpu/drm/gma500/gem.h b/drivers/gpu/drm/gma500/gem.h
index 79cced40c87f..96ef864b4bf1 100644
--- a/drivers/gpu/drm/gma500/gem.h
+++ b/drivers/gpu/drm/gma500/gem.h
@@ -23,7 +23,6 @@ struct psb_gem_object {
bool stolen;/* Backed from stolen RAM */
bool mmapping; /* Is mmappable */
struct page **pages;/* Backing pages if present */
- int npage; /* Number of backing pages */
};
static inline struct psb_gem_object *to_psb_gem_object(struct drm_gem_object
*obj)
diff --git a/drivers/gpu/drm/gma500/gma_display.c
b/drivers/gpu/drm/gma500/gma_display.c
index 60ba7de59139..dd801404cf99 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -391,11 +391,9 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
goto unref_cursor;
}
- /* Prevent overflow */
- if (pobj->npage > 4)
- cursor_pages = 4;
- else
- cursor_pages = pobj->npage;
+ cursor_pages = obj->size / PAGE_SIZE;
+ if (cursor_pages > 4)
+ cursor_pages = 4; /* Prevent overflow */
/* Copy the cursor to cursor mem */
tmp_dst = dev_priv->vram_addr + cursor_pobj->offset;
--
2.35.1
[PATCH v2 11/12] drm/gma500: Move GTT enable and disable code into helpers
Move the code for enabling and disabling the GTT into helpers and call
the functions in psb_gtt_init(), psb_gtt_fini() and psb_gtt_resume().
Removes code duplication.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/gtt.c | 81
1 file changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index b03feec64f01..83d9a9f7c73c 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,17 +125,44 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev,
const struct resource *r
mutex_unlock(>gtt_mutex);
}
-void psb_gtt_fini(struct drm_device *dev)
+static int psb_gtt_enable(struct drm_psb_private *dev_priv)
{
- struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct drm_device *dev = _priv->dev;
struct pci_dev *pdev = to_pci_dev(dev->dev);
+ int ret;
- iounmap(dev_priv->gtt_map);
+ ret = pci_read_config_word(pdev, PSB_GMCH_CTRL, _priv->gmch_ctrl);
+ if (ret)
+ return pcibios_err_to_errno(ret);
+ ret = pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl |
_PSB_GMCH_ENABLED);
+ if (ret)
+ return pcibios_err_to_errno(ret);
+
+ dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
+ PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
+
+ (void)PSB_RVDC32(PSB_PGETBL_CTL);
+
+ return 0;
+}
+
+static void psb_gtt_disable(struct drm_psb_private *dev_priv)
+{
+ struct drm_device *dev = _priv->dev;
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+
(void)PSB_RVDC32(PSB_PGETBL_CTL);
+}
+void psb_gtt_fini(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+
+ iounmap(dev_priv->gtt_map);
+ psb_gtt_disable(dev_priv);
mutex_destroy(_priv->gtt_mutex);
}
@@ -159,22 +186,15 @@ int psb_gtt_init(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
+ struct psb_gtt *pg = _priv->gtt;
unsigned gtt_pages;
- struct psb_gtt *pg;
- int ret = 0;
+ int ret;
mutex_init(_priv->gtt_mutex);
- pg = _priv->gtt;
-
- /* Enable the GTT */
- pci_read_config_word(pdev, PSB_GMCH_CTRL, _priv->gmch_ctrl);
- pci_write_config_word(pdev, PSB_GMCH_CTRL,
- dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
-
- dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
- PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
- (void) PSB_RVDC32(PSB_PGETBL_CTL);
+ ret = psb_gtt_enable(dev_priv);
+ if (ret)
+ goto err_mutex_destroy;
/* The root resource we allocate address space from */
pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
@@ -227,17 +247,16 @@ int psb_gtt_init(struct drm_device *dev)
if (!dev_priv->gtt_map) {
dev_err(dev->dev, "Failure to map gtt.\n");
ret = -ENOMEM;
- goto err_gtt_disable;
+ goto err_psb_gtt_disable;
}
psb_gtt_clear(dev_priv);
return 0;
-err_gtt_disable:
- pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
- PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
- (void)PSB_RVDC32(PSB_PGETBL_CTL);
+err_psb_gtt_disable:
+ psb_gtt_disable(dev_priv);
+err_mutex_destroy:
mutex_destroy(_priv->gtt_mutex);
return ret;
}
@@ -246,20 +265,14 @@ int psb_gtt_resume(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
+ struct psb_gtt *pg = _priv->gtt;
unsigned int gtt_pages;
- struct psb_gtt *pg;
int ret;
- pg = _priv->gtt;
-
/* Enable the GTT */
- pci_read_config_word(pdev, PSB_GMCH_CTRL, _priv->gmch_ctrl);
- pci_write_config_word(pdev, PSB_GMCH_CTRL,
- dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
-
- dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
- PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
- (void) PSB_RVDC32(PSB_PGETBL_CTL);
+ ret = psb_gtt_enable(dev_priv);
+ if (ret)
+ return ret;
/* The root resource we allocate address space from */
pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
@@ -311,16 +324,14 @@ int psb_gtt_resume(struct drm_device *dev)
if (gtt_pages != pg->gtt_pages) {
dev_err(dev->dev, "GTT resume error.\n");
ret = -EINVAL;
- goto err_gtt_disable;
+ goto err_psb_gtt_disable;
}
[PATCH v2 07/12] drm/gma500: Cleanup GTT uninit and error handling
Replace psb_gtt_takedown() with finalizer function that is only called
for unloading the driver. Use roll-back pattern for error handling in
psb_gtt_init() and _resume(). Also fixes a bug where vmap_addr was never
unmapped.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gtt.c | 49
drivers/gpu/drm/gma500/gtt.h | 2 +-
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
drivers/gpu/drm/gma500/psb_drv.h | 1 -
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 43ad3ec38c80..99c644a5c5cb 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,23 +125,20 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev,
const struct resource *r
mutex_unlock(>gtt_mutex);
}
-void psb_gtt_takedown(struct drm_device *dev)
+void psb_gtt_fini(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
- if (dev_priv->gtt_map) {
- iounmap(dev_priv->gtt_map);
- dev_priv->gtt_map = NULL;
- }
- if (dev_priv->gtt_initialized) {
- pci_write_config_word(pdev, PSB_GMCH_CTRL,
- dev_priv->gmch_ctrl);
- PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
- (void) PSB_RVDC32(PSB_PGETBL_CTL);
- }
- if (dev_priv->vram_addr)
- iounmap(dev_priv->gtt_map);
+ iounmap(dev_priv->vram_addr);
+ iounmap(dev_priv->gtt_map);
+
+ pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+ PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+ (void)PSB_RVDC32(PSB_PGETBL_CTL);
+
+ mutex_destroy(_priv->mmap_mutex);
+ mutex_destroy(_priv->gtt_mutex);
}
/* Clear GTT. Use a scratch page to avoid accidents or scribbles. */
@@ -233,8 +230,6 @@ int psb_gtt_init(struct drm_device *dev)
(void) PSB_RVDC32(PSB_PGETBL_CTL);
/* The root resource we allocate address space from */
- dev_priv->gtt_initialized = 1;
-
pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
/*
@@ -299,14 +294,14 @@ int psb_gtt_init(struct drm_device *dev)
if (!dev_priv->gtt_map) {
dev_err(dev->dev, "Failure to map gtt.\n");
ret = -ENOMEM;
- goto out_err;
+ goto err_gtt_disable;
}
dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
if (!dev_priv->vram_addr) {
dev_err(dev->dev, "Failure to map stolen base.\n");
ret = -ENOMEM;
- goto out_err;
+ goto err_iounmap;
}
psb_gtt_clear(dev_priv);
@@ -314,8 +309,14 @@ int psb_gtt_init(struct drm_device *dev)
return 0;
-out_err:
- psb_gtt_takedown(dev);
+err_iounmap:
+ iounmap(dev_priv->gtt_map);
+err_gtt_disable:
+ pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+ PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+ (void)PSB_RVDC32(PSB_PGETBL_CTL);
+ mutex_destroy(_priv->mmap_mutex);
+ mutex_destroy(_priv->gtt_mutex);
return ret;
}
@@ -340,8 +341,6 @@ static int psb_gtt_resume(struct drm_device *dev)
(void) PSB_RVDC32(PSB_PGETBL_CTL);
/* The root resource we allocate address space from */
- dev_priv->gtt_initialized = 1;
-
pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
/*
@@ -398,7 +397,7 @@ static int psb_gtt_resume(struct drm_device *dev)
if ((gtt_pages != pg->gtt_pages) && (stolen_size != pg->stolen_size)) {
dev_err(dev->dev, "GTT resume error.\n");
ret = -EINVAL;
- goto out_err;
+ goto err_gtt_disable;
}
pg->gtt_pages = gtt_pages;
@@ -410,8 +409,10 @@ static int psb_gtt_resume(struct drm_device *dev)
return 0;
-out_err:
- psb_gtt_takedown(dev);
+err_gtt_disable:
+ pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+ PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+ (void)PSB_RVDC32(PSB_PGETBL_CTL);
return ret;
}
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index cb270ea40794..5839d5d06adc 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -26,7 +26,7 @@ struct psb_gtt {
/* Exported functions */
int psb_gtt_init(struct drm_device *dev);
-extern void psb_gtt_takedown(struct drm_device *dev);
+void psb_gtt_fini(struct drm_device *dev);
extern int psb_gtt_restore(struct drm_device *dev);
int psb_gtt_allocate_resource(struct drm_psb_private *pdev, struct resource
*res,
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 2891a3dc8d2e..41be6e1ac7f9 100644
---
[PATCH v2 10/12] drm/gma500: Move GEM memory management functions to gem.c
Move GEM functions from gtt.c to gem.c. Adapt some names. No
functional changes.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gem.c | 133 +++
drivers/gpu/drm/gma500/gem.h | 12 +++
drivers/gpu/drm/gma500/gtt.c | 127 +
drivers/gpu/drm/gma500/gtt.h | 5 +-
drivers/gpu/drm/gma500/power.c | 1 +
drivers/gpu/drm/gma500/psb_drv.c | 1 +
6 files changed, 149 insertions(+), 130 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 2f44535f58e7..dffe37490206 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -21,6 +21,10 @@
#include "gem.h"
#include "psb_drv.h"
+/*
+ * PSB GEM object
+ */
+
int psb_gem_pin(struct psb_gem_object *pobj)
{
struct drm_gem_object *obj = >base;
@@ -296,3 +300,132 @@ static vm_fault_t psb_gem_fault(struct vm_fault *vmf)
return ret;
}
+
+/*
+ * Memory management
+ */
+
+/* Insert vram stolen pages into the GTT. */
+static void psb_gem_mm_populate_stolen(struct drm_psb_private *pdev)
+{
+ struct drm_device *dev = >dev;
+ unsigned int pfn_base;
+ unsigned int i, num_pages;
+ uint32_t pte;
+
+ pfn_base = pdev->stolen_base >> PAGE_SHIFT;
+ num_pages = pdev->vram_stolen_size >> PAGE_SHIFT;
+
+ drm_dbg(dev, "Set up %u stolen pages starting at 0x%08x, GTT offset
%dK\n",
+ num_pages, pfn_base << PAGE_SHIFT, 0);
+
+ for (i = 0; i < num_pages; ++i) {
+ pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
+ iowrite32(pte, pdev->gtt_map + i);
+ }
+
+ (void)ioread32(pdev->gtt_map + i - 1);
+}
+
+int psb_gem_mm_init(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
+ unsigned long stolen_size, vram_stolen_size;
+ struct psb_gtt *pg;
+ int ret;
+
+ mutex_init(_priv->mmap_mutex);
+
+ pg = _priv->gtt;
+
+ pci_read_config_dword(pdev, PSB_BSM, _priv->stolen_base);
+ vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base -
PAGE_SIZE;
+
+ stolen_size = vram_stolen_size;
+
+ dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
+ dev_priv->stolen_base, vram_stolen_size / 1024);
+
+ pg->stolen_size = stolen_size;
+ dev_priv->vram_stolen_size = vram_stolen_size;
+
+ dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
+ if (!dev_priv->vram_addr) {
+ dev_err(dev->dev, "Failure to map stolen base.\n");
+ ret = -ENOMEM;
+ goto err_mutex_destroy;
+ }
+
+ psb_gem_mm_populate_stolen(dev_priv);
+
+ return 0;
+
+err_mutex_destroy:
+ mutex_destroy(_priv->mmap_mutex);
+ return ret;
+}
+
+void psb_gem_mm_fini(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+
+ iounmap(dev_priv->vram_addr);
+
+ mutex_destroy(_priv->mmap_mutex);
+}
+
+/* Re-insert all pinned GEM objects into GTT. */
+static void psb_gem_mm_populate_resources(struct drm_psb_private *pdev)
+{
+ unsigned int restored = 0, total = 0, size = 0;
+ struct resource *r = pdev->gtt_mem->child;
+ struct drm_device *dev = >dev;
+ struct psb_gem_object *pobj;
+
+ while (r) {
+ /*
+* TODO: GTT restoration needs a refactoring, so that we don't
have to touch
+* struct psb_gem_object here. The type represents a GEM
object and is
+* not related to the GTT itself.
+*/
+ pobj = container_of(r, struct psb_gem_object, resource);
+ if (pobj->pages) {
+ psb_gtt_insert_pages(pdev, >resource,
pobj->pages);
+ size += resource_size(>resource);
+ ++restored;
+ }
+ r = r->sibling;
+ ++total;
+ }
+
+ drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total,
(size / 1024));
+}
+
+int psb_gem_mm_resume(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
+ unsigned long stolen_size, vram_stolen_size;
+ struct psb_gtt *pg;
+
+ pg = _priv->gtt;
+
+ pci_read_config_dword(pdev, PSB_BSM, _priv->stolen_base);
+ vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base -
PAGE_SIZE;
+
+ stolen_size = vram_stolen_size;
+
+ dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
dev_priv->stolen_base,
+ vram_stolen_size / 1024);
+
+ if (stolen_size != pg->stolen_size) {
+ dev_err(dev->dev, "GTT resume error.\n");
+ return -EINVAL;
+ }
+
+
[PATCH v2 06/12] drm/gma500: Move GTT resume logic out of psb_gtt_init()
The current implementation of psb_gtt_init() also does resume
handling. Move the resume code into its own helper.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gtt.c | 122 ++-
drivers/gpu/drm/gma500/gtt.h | 2 +-
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
3 files changed, 104 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index acd50ee26b03..43ad3ec38c80 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -209,7 +209,7 @@ static void psb_gtt_populate_resources(struct
drm_psb_private *pdev)
drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total,
(size / 1024));
}
-int psb_gtt_init(struct drm_device *dev, int resume)
+int psb_gtt_init(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -218,10 +218,8 @@ int psb_gtt_init(struct drm_device *dev, int resume)
struct psb_gtt *pg;
int ret = 0;
- if (!resume) {
- mutex_init(_priv->gtt_mutex);
- mutex_init(_priv->mmap_mutex);
- }
+ mutex_init(_priv->gtt_mutex);
+ mutex_init(_priv->mmap_mutex);
pg = _priv->gtt;
@@ -290,13 +288,6 @@ int psb_gtt_init(struct drm_device *dev, int resume)
dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
dev_priv->stolen_base, vram_stolen_size / 1024);
- if (resume && (gtt_pages != pg->gtt_pages) &&
- (stolen_size != pg->stolen_size)) {
- dev_err(dev->dev, "GTT resume error.\n");
- ret = -EINVAL;
- goto out_err;
- }
-
pg->gtt_pages = gtt_pages;
pg->stolen_size = stolen_size;
dev_priv->vram_stolen_size = vram_stolen_size;
@@ -304,19 +295,14 @@ int psb_gtt_init(struct drm_device *dev, int resume)
/*
* Map the GTT and the stolen memory area
*/
- if (!resume)
- dev_priv->gtt_map = ioremap(pg->gtt_phys_start,
- gtt_pages << PAGE_SHIFT);
+ dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages <<
PAGE_SHIFT);
if (!dev_priv->gtt_map) {
dev_err(dev->dev, "Failure to map gtt.\n");
ret = -ENOMEM;
goto out_err;
}
- if (!resume)
- dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base,
-stolen_size);
-
+ dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
if (!dev_priv->vram_addr) {
dev_err(dev->dev, "Failure to map stolen base.\n");
ret = -ENOMEM;
@@ -333,11 +319,107 @@ int psb_gtt_init(struct drm_device *dev, int resume)
return ret;
}
+static int psb_gtt_resume(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
+ unsigned int gtt_pages;
+ unsigned long stolen_size, vram_stolen_size;
+ struct psb_gtt *pg;
+ int ret = 0;
+
+ pg = _priv->gtt;
+
+ /* Enable the GTT */
+ pci_read_config_word(pdev, PSB_GMCH_CTRL, _priv->gmch_ctrl);
+ pci_write_config_word(pdev, PSB_GMCH_CTRL,
+ dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
+
+ dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
+ PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
+ (void) PSB_RVDC32(PSB_PGETBL_CTL);
+
+ /* The root resource we allocate address space from */
+ dev_priv->gtt_initialized = 1;
+
+ pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
+
+ /*
+* The video mmu has a hw bug when accessing 0x0D000.
+* Make gatt start at 0x0e000,. This doesn't actually
+* matter for us but may do if the video acceleration ever
+* gets opened up.
+*/
+ pg->mmu_gatt_start = 0xE000;
+
+ pg->gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
+ gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE) >> PAGE_SHIFT;
+ /* CDV doesn't report this. In which case the system has 64 gtt pages */
+ if (pg->gtt_start == 0 || gtt_pages == 0) {
+ dev_dbg(dev->dev, "GTT PCI BAR not initialized.\n");
+ gtt_pages = 64;
+ pg->gtt_start = dev_priv->pge_ctl;
+ }
+
+ pg->gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
+ pg->gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE) >>
PAGE_SHIFT;
+ dev_priv->gtt_mem = >resource[PSB_GATT_RESOURCE];
+
+ if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
+ static struct resource fudge; /* Preferably peppermint */
+ /*
+*
[PATCH v2 05/12] drm/gma500: Move GTT setup and restoration into helper funtions
The GTT init and restore functions contain logic to populate the
GTT entries. Move the code into helper functions.
Signed-off-by: Thomas Zimmermann
Acked-by: Patrik Jakobsson
---
drivers/gpu/drm/gma500/gtt.c | 115 +--
1 file changed, 68 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index c7b7cb1f2d13..acd50ee26b03 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -144,18 +144,79 @@ void psb_gtt_takedown(struct drm_device *dev)
iounmap(dev_priv->gtt_map);
}
+/* Clear GTT. Use a scratch page to avoid accidents or scribbles. */
+static void psb_gtt_clear(struct drm_psb_private *pdev)
+{
+ resource_size_t pfn_base;
+ unsigned long i;
+ uint32_t pte;
+
+ pfn_base = page_to_pfn(pdev->scratch_page);
+ pte = psb_gtt_mask_pte(pfn_base, PSB_MMU_CACHED_MEMORY);
+
+ for (i = 0; i < pdev->gtt.gtt_pages; ++i)
+ iowrite32(pte, pdev->gtt_map + i);
+
+ (void)ioread32(pdev->gtt_map + i - 1);
+}
+
+/* Insert vram stolen pages into the GTT. */
+static void psb_gtt_populate_stolen(struct drm_psb_private *pdev)
+{
+ struct drm_device *dev = >dev;
+ unsigned int pfn_base;
+ unsigned int i, num_pages;
+ uint32_t pte;
+
+ pfn_base = pdev->stolen_base >> PAGE_SHIFT;
+ num_pages = pdev->vram_stolen_size >> PAGE_SHIFT;
+
+ drm_dbg(dev, "Set up %u stolen pages starting at 0x%08x, GTT offset
%dK\n",
+ num_pages, pfn_base << PAGE_SHIFT, 0);
+
+ for (i = 0; i < num_pages; ++i) {
+ pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
+ iowrite32(pte, pdev->gtt_map + i);
+ }
+
+ (void)ioread32(pdev->gtt_map + i - 1);
+}
+
+/* Re-insert all pinned GEM objects into GTT. */
+static void psb_gtt_populate_resources(struct drm_psb_private *pdev)
+{
+ unsigned int restored = 0, total = 0, size = 0;
+ struct resource *r = pdev->gtt_mem->child;
+ struct drm_device *dev = >dev;
+ struct psb_gem_object *pobj;
+
+ while (r) {
+ /*
+* TODO: GTT restoration needs a refactoring, so that we don't
have to touch
+* struct psb_gem_object here. The type represents a GEM
object and is
+* not related to the GTT itself.
+*/
+ pobj = container_of(r, struct psb_gem_object, resource);
+ if (pobj->pages) {
+ psb_gtt_insert_pages(pdev, >resource,
pobj->pages);
+ size += resource_size(>resource);
+ ++restored;
+ }
+ r = r->sibling;
+ ++total;
+ }
+
+ drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total,
(size / 1024));
+}
+
int psb_gtt_init(struct drm_device *dev, int resume)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
unsigned gtt_pages;
unsigned long stolen_size, vram_stolen_size;
- unsigned i, num_pages;
- unsigned pfn_base;
struct psb_gtt *pg;
-
int ret = 0;
- uint32_t pte;
if (!resume) {
mutex_init(_priv->gtt_mutex);
@@ -262,29 +323,9 @@ int psb_gtt_init(struct drm_device *dev, int resume)
goto out_err;
}
- /*
-* Insert vram stolen pages into the GTT
-*/
-
- pfn_base = dev_priv->stolen_base >> PAGE_SHIFT;
- num_pages = vram_stolen_size >> PAGE_SHIFT;
- dev_dbg(dev->dev, "Set up %d stolen pages starting at 0x%08x, GTT
offset %dK\n",
- num_pages, pfn_base << PAGE_SHIFT, 0);
- for (i = 0; i < num_pages; ++i) {
- pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
- iowrite32(pte, dev_priv->gtt_map + i);
- }
-
- /*
-* Init rest of GTT to the scratch page to avoid accidents or scribbles
-*/
-
- pfn_base = page_to_pfn(dev_priv->scratch_page);
- pte = psb_gtt_mask_pte(pfn_base, PSB_MMU_CACHED_MEMORY);
- for (; i < gtt_pages; ++i)
- iowrite32(pte, dev_priv->gtt_map + i);
+ psb_gtt_clear(dev_priv);
+ psb_gtt_populate_stolen(dev_priv);
- (void) ioread32(dev_priv->gtt_map + i - 1);
return 0;
out_err:
@@ -295,30 +336,10 @@ int psb_gtt_init(struct drm_device *dev, int resume)
int psb_gtt_restore(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
- struct resource *r = dev_priv->gtt_mem->child;
- struct psb_gem_object *pobj;
- unsigned int restored = 0, total = 0, size = 0;
psb_gtt_init(dev, 1);
- while (r != NULL) {
- /*
-* TODO: GTT restoration needs a refactoring, so that we don't
have to touch
-
[PATCH v2 00/12] drm/gma500: Various cleanups to GEM code
Refactor and simplify various parts of the memory management. This
includes locking, initialization and finalizer functions, and code
organization.
Tested on Atom N2800 hardware.
v2:
* put common code in psb_gtt_{init,fini,resume}() into
helpers (Sam, Patrik)
Thomas Zimmermann (12):
drm/gma500: Remove struct psb_gem_object.npage
drm/gma500: Acquire reservation lock for GEM objects
drm/gma500: Move GTT locking into GTT helpers
drm/gma500: Remove struct psb_gtt.sem sempahore
drm/gma500: Move GTT setup and restoration into helper funtions
drm/gma500: Move GTT resume logic out of psb_gtt_init()
drm/gma500: Cleanup GTT uninit and error handling
drm/gma500: Split GTT init/resume/fini into GTT and GEM functions
drm/gma500: Inline psb_gtt_restore()
drm/gma500: Move GEM memory management functions to gem.c
drm/gma500: Move GTT enable and disable code into helpers
drm/gma500: Move GTT memory-range setup into helper
drivers/gpu/drm/gma500/gem.c | 161 ++-
drivers/gpu/drm/gma500/gem.h | 13 +-
drivers/gpu/drm/gma500/gma_display.c | 8 +-
drivers/gpu/drm/gma500/gtt.c | 295 +--
drivers/gpu/drm/gma500/gtt.h | 8 +-
drivers/gpu/drm/gma500/power.c | 5 +-
drivers/gpu/drm/gma500/psb_drv.c | 13 +-
drivers/gpu/drm/gma500/psb_drv.h | 1 -
8 files changed, 317 insertions(+), 187 deletions(-)
base-commit: 710a019ad85e96e66f7d75ee7f4733cdd8a2b0d0
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: 6e1032c6302461624f33194c8b8f37103a3fa6ef
--
2.35.1
Re: [PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Hi Suraj On 3/8/2022 6:30 AM, Kandpal, Suraj wrote: Hi Abhinav, Hi, Hi, Hi Abhinav, Hi Laurent Ok sure, I can take this up but I need to understand the proposal a little bit more before proceeding on this. So we will discuss this in another email where we specifically talk about the connector changes. Also, I am willing to take this up once the encoder part is done and merged so that atleast we keep moving on that as MSM WB implementation can proceed with that first. Hi Jani and Suraj Any concerns with splitting up the series into encoder and connector OR re- arranging them? Let me know if you can do this OR I can also split this up keeping Suraj's name in the Co-developed by tag. I was actually thinking of floating a new patch series with both encoder and connector pointer changes but with a change in initialization functions wherein we allocate a connector and encoder incase the driver doesn’t have its own this should minimize the effect on other drivers already using current drm writeback framework and also allow i915 to create its own connector. I was proposing to split up the encoder and connector because the comments from Laurent meant we were in agreement about the encoder but not about the connector. I am afraid another attempt with the similar idea is only going to stall the series again and hence i gave this option. Seems like this patch series currently won't be able to move forward with the connector pointer change so I guess you can split the series to encoder pointer change where we optionally create encoder if required ,keeping my name in co-developed tag so that msm can move forward with its implementation and then we can work to see how the connector issue can be bypassed. Best Regards, Suraj Kandpal Thanks, let me re-spin this and will keep your name as co-developer. Should be able to get it on the list in a week. Thanks Abhinav Eventually its upto Laurent and the other maintainers to guide us forward on this as this series has been stalled for weeks now. I thought that Laurent was quite strict about the connector. So I'd suggest targeting drm_writeback_connector with an optionally created encoder and the builtin connector In that case even if we target that i915 will not be able to move forward with its implementation of writeback as builtin connector does not work for us right now as explained earlier, optionally creating encoder and connector will help everyone move forward and once done we can actually sit and work on how to side step this issue using Laurent's suggestion This is up to Laurent & other DRM maintainers to decide whether this approach would be accepted or not. So far unfortunately I have mostly seen the pushback and a strong suggestion to stop treating each drm_connector as i915_connector. I haven't checked the exact details, but IMO adding a branch if the connector's type is DRM_CONNECTOR_VIRTUAL should be doable. Bringing in the change where we don’t treat each drm_connector as an intel_connector or even adding a branch which checks if drm_connector is DRM_CONNECTOR_VIRTUAL and conditionally cast it to intel_connector is quite a lot of work for i915. That's why I was suggesting if for now we could move forward with optionally creating both encoder and connector minimizing affects on other drivers as well as allowing us to move forward. What do you think, Launrent? We can work on Laurent's suggestion but that would first require the initial RFC patches and then a rework from both of our drivers side to see if they gel well with our current code which will take a considerable amount of time. We can for now however float the patch series up which minimally affects the current drivers and also allows i915 and msm to move forward with its writeback implementation off course with a proper documentation stating new drivers shouldn't try to make their own connectors and encoders and that drm_writeback will do that for them. Once that's done and merged we can work with Laurent on his proposal to improve the drm writeback framework so that this issue can be side stepped entirely in future. For now I would like to keep the encoder and connector pointer changes together.
Re: [PATCH 06/10] drm/gma500: Move GTT resume logic out of psb_gtt_init()
Hi
Am 08.03.22 um 13:07 schrieb Patrik Jakobsson:
On Tue, Mar 8, 2022 at 9:48 AM Thomas Zimmermann wrote:
Hi Sam and Patrik
Am 07.03.22 um 22:02 schrieb Patrik Jakobsson:
On Mon, Mar 7, 2022 at 8:07 PM Sam Ravnborg wrote:
Hi Thomas,
One comment below.
On Sun, Mar 06, 2022 at 09:36:15PM +0100, Thomas Zimmermann wrote:
The current implementation of psb_gtt_init() also does resume
handling. Move the resume code into its own helper.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gma500/gtt.c | 122 ++-
drivers/gpu/drm/gma500/gtt.h | 2 +-
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
3 files changed, 104 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index acd50ee26b03..43ad3ec38c80 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -209,7 +209,7 @@ static void psb_gtt_populate_resources(struct
drm_psb_private *pdev)
drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total,
(size / 1024));
}
-int psb_gtt_init(struct drm_device *dev, int resume)
+int psb_gtt_init(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -218,10 +218,8 @@ int psb_gtt_init(struct drm_device *dev, int resume)
struct psb_gtt *pg;
int ret = 0;
- if (!resume) {
- mutex_init(_priv->gtt_mutex);
- mutex_init(_priv->mmap_mutex);
- }
+ mutex_init(_priv->gtt_mutex);
+ mutex_init(_priv->mmap_mutex);
pg = _priv->gtt;
@@ -290,13 +288,6 @@ int psb_gtt_init(struct drm_device *dev, int resume)
dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
dev_priv->stolen_base, vram_stolen_size / 1024);
- if (resume && (gtt_pages != pg->gtt_pages) &&
- (stolen_size != pg->stolen_size)) {
- dev_err(dev->dev, "GTT resume error.\n");
- ret = -EINVAL;
- goto out_err;
- }
-
pg->gtt_pages = gtt_pages;
pg->stolen_size = stolen_size;
dev_priv->vram_stolen_size = vram_stolen_size;
@@ -304,19 +295,14 @@ int psb_gtt_init(struct drm_device *dev, int resume)
/*
* Map the GTT and the stolen memory area
*/
- if (!resume)
- dev_priv->gtt_map = ioremap(pg->gtt_phys_start,
- gtt_pages << PAGE_SHIFT);
+ dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages << PAGE_SHIFT);
if (!dev_priv->gtt_map) {
dev_err(dev->dev, "Failure to map gtt.\n");
ret = -ENOMEM;
goto out_err;
}
- if (!resume)
- dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base,
- stolen_size);
-
+ dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
if (!dev_priv->vram_addr) {
dev_err(dev->dev, "Failure to map stolen base.\n");
ret = -ENOMEM;
@@ -333,11 +319,107 @@ int psb_gtt_init(struct drm_device *dev, int resume)
return ret;
}
The below is a lot of duplicated complex code.
Can you add one more helper for this?
That makes a lot of sense.
I was thinking the same but figured it would be an easy follow up
patch. But sure, why not fix it here already.
While at it, the gtt enable/disable code could be put in helpers as well.
Patrik, do you know why the code re-reads all these values during
resume? Do the values change? The driver never seems to do anything
with the updated values. For example, it doesn't update the GTT mapping
if gtt_phys_start changes. Can we remove all that code from the resume
function?
In theory these values can change if you hibernate, make changes in
the bios and then resume. I think I've seen bioses that can change the
stolen size and/or aperture size. Not sure if that would also change
the value in eg. pge_ctl or the size of the gtt. I would have to do
some testing on real hardware to figure this out.
But as you say, the above scenario is already broken. For the time
being, can we perhaps just fail gracefully?
Ah, thanks for the explanation. I'll leave the current logic as-is.
Best regards
Thomas
--
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 9/9] drm/virtio: Implement dumb_create_fbdev with GEM SHMEM helpers
On 3/3/22 21:58, Thomas Zimmermann wrote:
> Implement struct drm_driver.dumb_create_fbdev with the helpers
> provided by GEM SHMEM. Fbdev deferred I/O will now work without
> an intermediate shadow buffer for mmap.
>
> As the virtio driver replaces several of the regular GEM SHMEM
> functions with its own implementation, some additional code is
> necessary make fbdev optimization work. Especially, the driver
> has to provide buffer-object functions for fbdev. Add the hook
> struct drm_driver.gem_create_object_fbdev, which is similar to
> struct drm_driver.gem_create_object and allows for the creation
> of dedicated fbdev buffer objects. Wire things up within GEM
> SHMEM.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 7 +++-
> drivers/gpu/drm/virtio/virtgpu_drv.c| 2 +
> drivers/gpu/drm/virtio/virtgpu_drv.h| 2 +
> drivers/gpu/drm/virtio/virtgpu_object.c | 54 +++--
> include/drm/drm_drv.h | 10 +
> 5 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index ab7cb7d896c3..225aa17895bd 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -71,7 +71,12 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t
> size, bool private, bool f
>
> size = PAGE_ALIGN(size);
>
> - if (dev->driver->gem_create_object) {
> + if (dev->driver->gem_create_object_fbdev && fbdev) {
Same comment here to check if fbdev emulation is enabled or maybe is not
worht it ? But I think this hints the compiler to optimize the if branch.
[snip]
> +}
> +#else
> +struct drm_gem_object *virtio_gpu_create_object_fbdev(struct drm_device *dev,
> + size_t size)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
As mentioned, I believe this should be ERR_PTR(-ENOTSUPP) instead.
Feel free to ignore all this nits if you consider that are not applicable.
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [RESEND PATCH] dt-bindings: display/msm: add missing brace in dpu-qcm2290.yaml
On Tue, Mar 1, 2022 at 6:14 PM Dmitry Baryshkov wrote: > > Add missing brace in dpu-qcm2290.yaml. While we are at it, also fix > indentation for another brace, so it matches the corresponding line. > > Reported-by: Rob Herring > Cc: Loic Poulain > Reviewed-by: Bjorn Andersson > Signed-off-by: Dmitry Baryshkov > --- > Didn't include freedreno@ in the first email, so resending. > --- > Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Now that the example actually builds, we get just schema warnings: /builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.example.dt.yaml: mdss@5e0: compatible: ['qcom,qcm2290-mdss', 'qcom,mdss'] is too long >From schema: >/builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml /builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.example.dt.yaml: mdss@5e0: 'mdp@5e01000' does not match any of the regexes: '^display-controller@[0-9a-f]+$', 'pinctrl-[0-9]+' >From schema: >/builds/robherring/linux-dt/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml I would have assumed upon reporting errors with 'make dt_binding_check' that the fixes would be tested with 'make dt_binding_check'... Rob
Re: [PATCH 8/9] drm/gem-shmem: Implement fbdev dumb buffer and mmap helpers
On 3/3/22 21:58, Thomas Zimmermann wrote:
> Implement struct drm_driver.dumb_create_fbdev for GEM SHMEM. The
> created buffer object returned by this function implements deferred
> I/O for its mmap operation.
>
> Add this feature to a number of drivers that use GEM SHMEM helpers
> as shadow planes over their regular video memory. The new macro
> DRM_GEM_SHMEM_DRIVER_OPS_WITH_SHADOW_PLANES sets the regular GEM
> functions and dumb_create_fbdev in struct drm_driver. Fbdev emulation
> on these drivers will now mmap the GEM SHMEM pages directly with
> deferred I/O without an intermediate shadow buffer.
>
> Signed-off-by: Thomas Zimmermann
> ---
[snip]
> @@ -49,8 +50,20 @@ static const struct drm_gem_object_funcs
> drm_gem_shmem_funcs = {
> .vm_ops = _gem_shmem_vm_ops,
> };
>
> +static const struct drm_gem_object_funcs drm_gem_shmem_funcs_fbdev = {
> + .free = drm_gem_shmem_object_free,
> + .print_info = drm_gem_shmem_object_print_info,
> + .pin = drm_gem_shmem_object_pin,
> + .unpin = drm_gem_shmem_object_unpin,
> + .get_sg_table = drm_gem_shmem_object_get_sg_table,
> + .vmap = drm_gem_shmem_object_vmap,
> + .vunmap = drm_gem_shmem_object_vunmap,
> + .mmap = drm_gem_shmem_object_mmap_fbdev,
> + .vm_ops = _gem_shmem_vm_ops_fbdev,
The drm_gem_shmem_funcs_fbdev is the same than drm_gem_shmem_funcs but
.mmap and .vm_ops callbacks. Maybe adding a macro to declare these two
struct drm_gem_object_funcs could make easier to maintain it long term ?
> +};
> +
> static struct drm_gem_shmem_object *
> -__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)
> +__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
> bool fbdev)
> {
> struct drm_gem_shmem_object *shmem;
> struct drm_gem_object *obj;
> @@ -70,8 +83,12 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t
> size, bool private)
> obj = >base;
> }
>
> - if (!obj->funcs)
> - obj->funcs = _gem_shmem_funcs;
> + if (!obj->funcs) {
> + if (fbdev)
Same question than in patch #6, maybe
if (defined(CONFIG_DRM_FBDEV_EMULATION) && fbdev) ?
[snip]
> + */
> +int drm_gem_shmem_dumb_create_fbdev(struct drm_file *file, struct drm_device
> *dev,
> + struct drm_mode_create_dumb *args)
> +{
> +#if defined(CONFIG_DRM_FBDEV_EMULATION)
> + return __drm_gem_shmem_dumb_create(file, dev, true, args);
> +#else
> + return -ENOSYS;
> +#endif
> +}
I believe the correct errno code here should be -ENOTSUPP.
[snip]
> + /* We don't use vmf->pgoff since that has the fake offset */
> + page_offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
I see this (vmf->address - vma->vm_start) calculation in many places of
this series. Maybe we could add a macro to calculate the offset instead ?
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On 3/8/22 19:29, Rob Clark wrote: > On Tue, Mar 8, 2022 at 5:17 AM Dmitry Osipenko > wrote: >> >> Hello, >> >> This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. >> During OOM, the shrinker will release BOs that are marked as "not needed" >> by userspace using the new madvise IOCTL. The userspace in this case is >> the Mesa VirGL driver, it will mark the cached BOs as "not needed", >> allowing kernel driver to release memory of the cached shmem BOs on lowmem >> situations, preventing OOM kills. > > Will host memory pressure already trigger shrinker in guest? The host memory pressure won't trigger shrinker in guest here. This series will help only with the memory pressure within the guest using a usual "virgl context". Having a host shrinker in a case of "virgl contexts" should be a difficult problem to solve. > This is > something I'm quite interested in for "virtgpu native contexts" (ie. > native guest driver with new context type sitting on top of virtgpu), In a case of "native contexts" it should be doable, at least I can't see any obvious problems. The madvise invocations could be passed to the host using a new virtio-gpu command by the guest's madvise IOCTL handler, instead-of/in-addition-to handling madvise in the guest's kernel, and that's it. > since that isn't using host storage s/host/guest ?
[PATCH] drm/amdkfd: CRIU export dmabuf handles for GTT BOs
Export dmabuf handles for GTT BOs so that their contents can be accessed
using SDMA during checkpoint/restore.
Signed-off-by: David Yat Sin
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 2c7d76e67ddb..e1e2362841f8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1759,7 +1759,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
goto exit;
}
}
- if (bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret =
criu_get_prime_handle(_bo->tbo.base,
bo_bucket->alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? DRM_RDWR : 0,
@@ -1812,7 +1813,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
exit:
while (ret && bo_index--) {
- if (bo_buckets[bo_index].alloc_flags &
KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[bo_index].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[bo_index].dmabuf_fd);
}
@@ -2211,7 +2213,8 @@ static int criu_restore_bo(struct kfd_process *p,
pr_debug("map memory was successful for the BO\n");
/* create the dmabuf object and export the bo */
- if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (bo_bucket->alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM | KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
ret = criu_get_prime_handle(_mem->bo->tbo.base, DRM_RDWR,
_bucket->dmabuf_fd);
if (ret)
@@ -2281,7 +2284,8 @@ static int criu_restore_bos(struct kfd_process *p,
exit:
while (ret && i--) {
- if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
+ if (bo_buckets[i].alloc_flags
+ & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM |
KFD_IOC_ALLOC_MEM_FLAGS_GTT))
close_fd(bo_buckets[i].dmabuf_fd);
}
kvfree(bo_buckets);
--
2.17.1
[PATCH 3/3] drm/bridge: Add myself as a reviewer for the Parade PS8640 bridge chip
Though the parade bridge chip is a little bit of a black box, I'm at least interested in hearing about changes to the driver since this bridge chip is used on some Chromebooks that I'm involved with. Signed-off-by: Douglas Anderson --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 7d25d0b4dccc..db7fe53643c2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6171,6 +6171,11 @@ S: Maintained F: Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino.yaml F: drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c +DRM DRIVER FOR PARADE PS8640 BRIDGE CHIP +R: Douglas Anderson +F: Documentation/devicetree/bindings/display/bridge/ps8640.yaml +F: drivers/gpu/drm/bridge/parade-ps8640.c + DRM DRIVER FOR PERVASIVE DISPLAYS REPAPER PANELS M: Noralf Trønnes S: Maintained -- 2.35.1.616.g0bdcbb4464-goog
[PATCH 2/3] drm/bridge: Add myself as a reviewer for the TI SN65DSI86 bridge chip
I've spent quite a bit of time poking at this driver and it's used on several Chromebooks I'm involved with. I'd like to get notified about patches. Add myself as a reviewer. It's expected that changes will still be landed through drm-misc as they always have been. Signed-off-by: Douglas Anderson --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a73179d55d00..7d25d0b4dccc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6284,6 +6284,11 @@ DRM DRIVER FOR TDFX VIDEO CARDS S: Orphan / Obsolete F: drivers/gpu/drm/tdfx/ +DRM DRIVER FOR TI SN65DSI86 BRIDGE CHIP +R: Douglas Anderson +F: Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml +F: drivers/gpu/drm/bridge/ti-sn65dsi86.c + DRM DRIVER FOR TPO TPG110 PANELS M: Linus Walleij S: Maintained -- 2.35.1.616.g0bdcbb4464-goog
[PATCH 1/3] drm/bridge: Add MAINTAINERS entry for DRM drivers for bridge chip bindings
The bindings for bridge chips should also get the same maintainers entry so the right people get notified about bindings changes. Signed-off-by: Douglas Anderson --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0216d2ffe728..a73179d55d00 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6403,6 +6403,7 @@ R:Jonas Karlman R: Jernej Skrabec S: Maintained T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/bridge/ F: drivers/gpu/drm/bridge/ DRM DRIVERS FOR EXYNOS -- 2.35.1.616.g0bdcbb4464-goog
Re: [PATCH] drm/msm/gpu: Fix crash on devices without devfreq support (v2)
On Tue, Mar 8, 2022 at 10:53 AM Fabio Estevam wrote:
>
> On Tue, Mar 8, 2022 at 3:48 PM Rob Clark wrote:
> >
> > From: Rob Clark
> >
> > Avoid going down devfreq paths on devices where devfreq is not
> > initialized.
> >
> > v2: Change has_devfreq() logic [Dmitry]
> >
> > Reported-by: Linux Kernel Functional Testing
> > Reported-by: Anders Roxell
Fixes: 6aa89ae1fb04 ("drm/msm/gpu: Cancel idle/boost work on suspend")
> > Signed-off-by: Rob Clark
>
> Does this need a Fixes tag?
Yes, sorry, patchwork had picked up the fixes tag from previous
version but I'd forgot to add it locally
BR,
-R
Re: [PATCH 7/9] drm/fb-helper: Provide fbdev deferred-I/O helpers
On 3/3/22 21:58, Thomas Zimmermann wrote: > Add drm_fb_helper_vm_page_mkwrite(), a helper to track pages written > by fbdev userspace. DRM drivers should use this function to implement > fbdev deferred I/O. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
RE: [RFC 18/28] drm: rcar-du: Add RZ/G2L LCDC Support
Hi Laurent,
Thanks for the feedback.
> Subject: Re: [RFC 18/28] drm: rcar-du: Add RZ/G2L LCDC Support
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Wed, Jan 12, 2022 at 05:46:02PM +, Biju Das wrote:
> > The LCD controller is composed of Frame Compression Processor (FCPVD),
> > Video Signal Processor (VSPD), and Display Unit (DU).
> >
> > It has DPI/DSI interfaces and supports a maximum resolution of 1080p
> > along with 2 rpf's to support blending of two picture layers and
> > raster operations (ROPs).
> >
> > A feature bit for RZ/G2L SoC is introduced to support RZ/G2L with the
> > rest of the SoC supported by this driver.
>
> The RZ/G2L DU seems to be a completely different IP core than the DU in R-
> Car and other RZ SoCs. I think it should be supported by a separate
> driver, or at least with a different (and modularized) CRTC
> implementation. This patch has too many RCAR_DU_FEATURE_RZG2L checks.
Agreed, May be will create separate RZ/G2L specific DU driver based on RCar-DU
driver removing the plane, Group, CMM, LVDS, HDMI and CRTC adaptation for
RZ/G2L
and will send next version for feedback.
Regards,
Biju
>
> > Signed-off-by: Biju Das
> > ---
> > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 148 ++--
> > drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 +
> > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 23
> > drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 +
> > drivers/gpu/drm/rcar-du/rcar_du_group.c | 5 +
> > drivers/gpu/drm/rcar-du/rcar_du_regs.h | 52 +
> > 6 files changed, 195 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > index 521446890d3d..aea9178f3e7d 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > @@ -10,6 +10,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include
> > @@ -219,6 +220,42 @@ static void rcar_du_crtc_set_display_timing(struct
> rcar_du_crtc *rcrtc)
> > u32 dsmr;
> > u32 escr;
> >
> > + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + u32 ditr0, ditr1, ditr2, ditr3, ditr4, ditr5, pbcr0;
> > +
> > + clk_set_rate(rcrtc->extclock, mode_clock);
> > +
> > + ditr0 = (DU_DITR0_DEMD_HIGH
> > + | ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DU_DITR0_VSPOL : 0)
> > + | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DU_DITR0_HSPOL :
> 0));
> > +
> > + ditr1 = DU_DITR1_VSA(mode->vsync_end - mode->vsync_start)
> > + | DU_DITR1_VACTIVE(mode->vdisplay);
> > +
> > + ditr2 = DU_DITR2_VBP(mode->vtotal - mode->vsync_end)
> > + | DU_DITR2_VFP(mode->vsync_start - mode->vdisplay);
> > +
> > + ditr3 = DU_DITR3_HSA(mode->hsync_end - mode->hsync_start)
> > + | DU_DITR3_HACTIVE(mode->hdisplay);
> > +
> > + ditr4 = DU_DITR4_HBP(mode->htotal - mode->hsync_end)
> > + | DU_DITR4_HFP(mode->hsync_start - mode->hdisplay);
> > +
> > + ditr5 = DU_DITR5_VSFT(0) | DU_DITR5_HSFT(0);
> > +
> > + pbcr0 = DU_PBCR0_PB_DEP(0x1F);
> > +
> > + rcar_du_write(rcdu, DU_DITR0, ditr0);
> > + rcar_du_write(rcdu, DU_DITR1, ditr1);
> > + rcar_du_write(rcdu, DU_DITR2, ditr2);
> > + rcar_du_write(rcdu, DU_DITR3, ditr3);
> > + rcar_du_write(rcdu, DU_DITR4, ditr4);
> > + rcar_du_write(rcdu, DU_DITR5, ditr5);
> > + rcar_du_write(rcdu, DU_PBCR0, pbcr0);
> > +
> > + return;
> > + }
> > +
> > if (rcdu->info->dpll_mask & (1 << rcrtc->index)) {
> > unsigned long target = mode_clock;
> > struct dpll_info dpll = { 0 };
> > @@ -531,16 +568,23 @@ static void rcar_du_cmm_setup(struct drm_crtc
> > *crtc)
> >
> > static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) {
> > - /* Set display off and background to black */
> > - rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0));
> > - rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0));
> > + struct rcar_du_device *rcdu = rcrtc->dev;
> >
> > - /* Configure display timings and output routing */
> > - rcar_du_crtc_set_display_timing(rcrtc);
> > - rcar_du_group_set_routing(rcrtc->group);
> > + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + /* Set display off and background to black */
> > + rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0));
> > + rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0));
> >
> > - /* Start with all planes disabled. */
> > - rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR,
> 0);
> > + /* Configure display timings and output routing */
> > + rcar_du_crtc_set_display_timing(rcrtc);
> > + rcar_du_group_set_routing(rcrtc->group);
> > +
> > + /* Start with all planes disabled. */
> > + rcar_du_group_write(rcrtc->group,
Re: [PATCH] drm/msm/gpu: Fix crash on devices without devfreq support (v2)
On Tue, Mar 8, 2022 at 3:48 PM Rob Clark wrote: > > From: Rob Clark > > Avoid going down devfreq paths on devices where devfreq is not > initialized. > > v2: Change has_devfreq() logic [Dmitry] > > Reported-by: Linux Kernel Functional Testing > Reported-by: Anders Roxell > Signed-off-by: Rob Clark Does this need a Fixes tag?
[PATCH] drm/msm/gpu: Fix crash on devices without devfreq support (v2)
From: Rob Clark
Avoid going down devfreq paths on devices where devfreq is not
initialized.
v2: Change has_devfreq() logic [Dmitry]
Reported-by: Linux Kernel Functional Testing
Reported-by: Anders Roxell
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 30 ++-
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index 9bf319be11f6..12641616acd3 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -83,6 +83,12 @@ static struct devfreq_dev_profile msm_devfreq_profile = {
static void msm_devfreq_boost_work(struct kthread_work *work);
static void msm_devfreq_idle_work(struct kthread_work *work);
+static bool has_devfreq(struct msm_gpu *gpu)
+{
+ struct msm_gpu_devfreq *df = >devfreq;
+ return !!df->devfreq;
+}
+
void msm_devfreq_init(struct msm_gpu *gpu)
{
struct msm_gpu_devfreq *df = >devfreq;
@@ -149,6 +155,9 @@ void msm_devfreq_cleanup(struct msm_gpu *gpu)
{
struct msm_gpu_devfreq *df = >devfreq;
+ if (!has_devfreq(gpu))
+ return;
+
devfreq_cooling_unregister(gpu->cooling);
dev_pm_qos_remove_request(>boost_freq);
dev_pm_qos_remove_request(>idle_freq);
@@ -156,16 +165,24 @@ void msm_devfreq_cleanup(struct msm_gpu *gpu)
void msm_devfreq_resume(struct msm_gpu *gpu)
{
- gpu->devfreq.busy_cycles = 0;
- gpu->devfreq.time = ktime_get();
+ struct msm_gpu_devfreq *df = >devfreq;
- devfreq_resume_device(gpu->devfreq.devfreq);
+ if (!has_devfreq(gpu))
+ return;
+
+ df->busy_cycles = 0;
+ df->time = ktime_get();
+
+ devfreq_resume_device(df->devfreq);
}
void msm_devfreq_suspend(struct msm_gpu *gpu)
{
struct msm_gpu_devfreq *df = >devfreq;
+ if (!has_devfreq(gpu))
+ return;
+
devfreq_suspend_device(df->devfreq);
cancel_idle_work(df);
@@ -185,6 +202,9 @@ void msm_devfreq_boost(struct msm_gpu *gpu, unsigned factor)
struct msm_gpu_devfreq *df = >devfreq;
uint64_t freq;
+ if (!has_devfreq(gpu))
+ return;
+
freq = get_freq(gpu);
freq *= factor;
@@ -207,7 +227,7 @@ void msm_devfreq_active(struct msm_gpu *gpu)
struct devfreq_dev_status status;
unsigned int idle_time;
- if (!df->devfreq)
+ if (!has_devfreq(gpu))
return;
/*
@@ -253,7 +273,7 @@ void msm_devfreq_idle(struct msm_gpu *gpu)
{
struct msm_gpu_devfreq *df = >devfreq;
- if (!df->devfreq)
+ if (!has_devfreq(gpu))
return;
msm_hrtimer_queue_work(>idle_work, ms_to_ktime(1),
--
2.35.1
Re: [Intel-gfx] [PATCH] drm/i915/xehp: Drop aux table invalidation on FlatCCS platforms
On Mon, Feb 28, 2022 at 09:29:52PM -0800, Matt Roper wrote:
Platforms with FlatCCS do not use auxiliary planes for compression
control data and thus do not need traditional aux table invalidation
(and the registers no longer even exist).
Original-author: CQ Tang
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 28
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 1f8cf4f790b2..13bbbf5d9737 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -231,7 +231,7 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
if (mode & EMIT_INVALIDATE) {
u32 flags = 0;
- u32 *cs;
+ u32 *cs, count;
flags |= PIPE_CONTROL_COMMAND_CACHE_INVALIDATE;
flags |= PIPE_CONTROL_TLB_INVALIDATE;
@@ -246,7 +246,12 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
flags |= PIPE_CONTROL_CS_STALL;
- cs = intel_ring_begin(rq, 8 + 4);
+ if (!HAS_FLAT_CCS(rq->engine->i915))
+ count = 8 + 4;
+ else
+ count = 8;
u32 count = 8;
...
if (!HAS_FLAT_CCS(rq->engine->i915))
count += 4;
would probably be shorter, or even
cs = intel_ring_begin(rq, HAS_FLAT_CCS(...) ? 12 : 8)
but doesn't really matter
Reviewed-by: Lucas De Marchi
Lucas De Marchi
Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/gem: Remove logic for wbinvd_on_all_cpus
On Tue, Feb 22, 2022 at 08:24:31PM +0100, Thomas Hellström (Intel) wrote:
Hi, Michael,
On 2/22/22 18:26, Michael Cheng wrote:
This patch removes logic for wbinvd_on_all_cpus and brings in
drm_cache.h. This header has the logic that outputs a warning
when wbinvd_on_all_cpus when its being used on a non-x86 platform.
Signed-off-by: Michael Cheng
Linus has been pretty clear that he won't accept patches that add
macros that works on one arch and warns on others anymore in i915 and
I figure even less so in drm code.
So we shouldn't try to move this out to drm. Instead we should
restrict the wbinvd() inside our driver to integrated and X86 only.
For discrete on all architectures we should be coherent and hence not
be needing wbinvd().
the warn is there to guarantee we don't forget a code path. However
simply adding the warning is the real issue: we should rather guarantee
we can't take that code path. I.e., as you said refactor the code to
guarantee it works on discrete without that logic.
$ git grep wbinvd_on_all_cpus -- drivers/gpu/drm/
drivers/gpu/drm/drm_cache.c:if (wbinvd_on_all_cpus())
drivers/gpu/drm/drm_cache.c:if (wbinvd_on_all_cpus())
drivers/gpu/drm/drm_cache.c:if (wbinvd_on_all_cpus())
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c: * Currently we just do
a heavy handed wbinvd_on_all_cpus() here since
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c:
wbinvd_on_all_cpus();
It looks like we actually go through this on other discrete graphics. Is
this missing an update like s/IS_DG1/IS_DGFX/? Or should we be doing
something else?
drivers/gpu/drm/i915/gem/i915_gem_pm.c:#define wbinvd_on_all_cpus() \
drivers/gpu/drm/i915/gem/i915_gem_pm.c: wbinvd_on_all_cpus();
Those are for suspend. Revert ac05a22cd07a ("drm/i915/gem: Almagamate clflushes on
suspend")
or extract that part to a helper function and implement it differently
for arches != x86?
drivers/gpu/drm/i915/gem/i915_gem_pm.c: wbinvd_on_all_cpus();
Probably take a similar approach to the suspend case?
drivers/gpu/drm/i915/gt/intel_ggtt.c: wbinvd_on_all_cpus();
This one comes from 64b95df91f44 ("drm/i915: Assume exclusive access to objects
inside resume")
Shouldn't that be doing the invalidate if the write domain is
I915_GEM_DOMAIN_CPU
In the end I think the warning would be ok if it was the cherry on top,
to guarantee we don't take those paths. We should probably have a
warn_once() to avoid spamming the console. But we also have to rework
the code to guarantee we are the only ones who may eventually get that
warning, and not the end user.
Lucas De Marchi
Thanks,
/Thomas
Re: [PATCH 6/9] drm/fb-helper: Provide callback to create fbdev dumb buffers
On 3/3/22 21:58, Thomas Zimmermann wrote:
> Provide struct drm_driver.dumb_create_fbdev, a callback hook for
> fbdev dumb buffers. Wire up fbdev and client helpers to use the new
> interface, if present.
>
> This acknowledges the fact that fbdev buffers are different. The most
> significant difference to regular GEM BOs is in mmap semantics. Fbdev
> userspace treats the pages as video memory, which makes it impossible
> to ever move the mmap'ed buffer. Hence, drivers ussually have to pin
> the BO permanently or install an intermediate shadow buffer for mmap.
>
> So far, fbdev memory came from dumb buffers and DRM drivers had no
> means of detecting this without reimplementing a good part of the fbdev
> code. With the new callback, drivers can perma-pin fbdev buffer objects
> if needed.
>
> Several drivers also require damage handling, which fbdev implements
> with its deferred I/O helpers. The new callback allows a driver's memory
> manager to set up a suitable mmap.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_client.c| 14 +++
> drivers/gpu/drm/drm_crtc_internal.h | 3 +++
> drivers/gpu/drm/drm_dumb_buffers.c | 36 +
> drivers/gpu/drm/drm_fb_helper.c | 26 +
> include/drm/drm_client.h| 3 ++-
> include/drm/drm_drv.h | 17 ++
> 6 files changed, 84 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index af3b7395bf69..c964064651d5 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -247,7 +247,8 @@ static void drm_client_buffer_delete(struct
> drm_client_buffer *buffer)
> }
>
> static struct drm_client_buffer *
> -drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32
> height, u32 format)
> +drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32
> height, u32 format,
> + bool fbdev)
> {
> const struct drm_format_info *info = drm_format_info(format);
> struct drm_mode_create_dumb dumb_args = { };
> @@ -265,7 +266,10 @@ drm_client_buffer_create(struct drm_client_dev *client,
> u32 width, u32 height, u
> dumb_args.width = width;
> dumb_args.height = height;
> dumb_args.bpp = info->cpp[0] * 8;
> - ret = drm_mode_create_dumb(dev, _args, client->file);
> + if (fbdev)
Maybe if (defined(CONFIG_DRM_FBDEV_EMULATION) && fbdev) ?
> + ret = drm_mode_create_dumb_fbdev(dev, _args, client->file);
And drm_mode_create_dumb_fbdev() could just be made a stub if
CONFIG_DRM_FBDEV_EMULATION isn't enabled.
I believe the only usage of the DRM client API currently is the fbdev
emulation layer anyways? But still makes sense I think to condtionally
compile that since drm_client.o is built in the drm.ko module and the
drm_fb_helper.o only included if fbdev emulation has been enabled.
> + else
> + ret = drm_mode_create_dumb(dev, _args, client->file);
> if (ret)
> goto err_delete;
>
> @@ -402,6 +406,7 @@ static int drm_client_buffer_addfb(struct
> drm_client_buffer *buffer,
> * @width: Framebuffer width
> * @height: Framebuffer height
> * @format: Buffer format
> + * @fbdev: True if the client provides an fbdev device, or false otherwise
> *
An emulated fbdev device ?
Other than those small nit,
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
[PATCH v2] drm/mgag200: Fix PLL setup for g200wb and g200ew
commit f86c3ed55920 ("drm/mgag200: Split PLL setup into compute and
update functions") introduced a regression for g200wb and g200ew.
The PLLs are not set up properly, and VGA screen stays
black, or displays "out of range" message.
MGA1064_WB_PIX_PLLC_N/M/P was mistakenly replaced with
MGA1064_PIX_PLLC_N/M/P which have different addresses.
Patch tested on a Dell T310 with g200wb
Fixes: f86c3ed55920 ("drm/mgag200: Split PLL setup into compute and update
functions")
Cc: sta...@vger.kernel.org
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/mgag200/mgag200_pll.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_pll.c
b/drivers/gpu/drm/mgag200/mgag200_pll.c
index e9ae22b4f813..52be08b744ad 100644
--- a/drivers/gpu/drm/mgag200/mgag200_pll.c
+++ b/drivers/gpu/drm/mgag200/mgag200_pll.c
@@ -404,9 +404,9 @@ mgag200_pixpll_update_g200wb(struct mgag200_pll *pixpll,
const struct mgag200_pl
udelay(50);
/* program pixel pll register */
- WREG_DAC(MGA1064_PIX_PLLC_N, xpixpllcn);
- WREG_DAC(MGA1064_PIX_PLLC_M, xpixpllcm);
- WREG_DAC(MGA1064_PIX_PLLC_P, xpixpllcp);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_N, xpixpllcn);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_M, xpixpllcm);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_P, xpixpllcp);
udelay(50);
--
2.35.1
Re: [PATCH] drm/mgag200: Fix PLL setup for g200wb and g200ew
On 08/03/2022 18:31, Greg KH wrote:
On Tue, Mar 08, 2022 at 06:11:11PM +0100, Jocelyn Falempe wrote:
commit f86c3ed55920ca1d874758cc290890902a6cffc4 ("drm/mgag200: Split PLL
setup into compute and update functions") introduced a regression for
g200wb and g200ew.
No need for all those digits in the sha1, see below:
The PLLs are not set up properly, and VGA screen stays
black, or displays "out of range" message.
MGA1064_WB_PIX_PLLC_N/M/P was mistakenly replaced with
MGA1064_PIX_PLLC_N/M/P which have different addresses.
Patch tested on a Dell T310 with g200wb
Fixes: f86c3ed55920ca1d874758cc290890902a6cffc4
As per the documentation that line should read:
Fixes: f86c3ed55920 ("drm/mgag200: Split PLL setup into compute and update
functions")
Sorry, I will send a v2 shortly.
thanks,
greg k-h
Re: [PATCH] drm/mgag200: Fix PLL setup for g200wb and g200ew
On Tue, Mar 08, 2022 at 06:11:11PM +0100, Jocelyn Falempe wrote:
> commit f86c3ed55920ca1d874758cc290890902a6cffc4 ("drm/mgag200: Split PLL
> setup into compute and update functions") introduced a regression for
> g200wb and g200ew.
No need for all those digits in the sha1, see below:
> The PLLs are not set up properly, and VGA screen stays
> black, or displays "out of range" message.
>
> MGA1064_WB_PIX_PLLC_N/M/P was mistakenly replaced with
> MGA1064_PIX_PLLC_N/M/P which have different addresses.
>
> Patch tested on a Dell T310 with g200wb
>
> Fixes: f86c3ed55920ca1d874758cc290890902a6cffc4
As per the documentation that line should read:
Fixes: f86c3ed55920 ("drm/mgag200: Split PLL setup into compute and update
functions")
thanks,
greg k-h
Re: [PATCH 5/9] drm/fb-helper: Separate deferred I/O from shadow buffers
On 3/3/22 21:58, Thomas Zimmermann wrote: > DRM drivers will be able to handle deferred I/O by themselves. So > a driver will be able to use deferred I/O without an intermediate > shadow buffer. > > Prepare fbdev emulation by separating shadow buffers and deferred > I/O from each other. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 4/9] fbdev: Export helper for implementing page_mkwrite
On 3/3/22 21:58, Thomas Zimmermann wrote: > Refactor the page-write handler and export it as helper function > fb_deferred_io_page_mkwrite(). Drivers that implement struct > vm_operations_struct.page_mkwrite for deferred I/O should use the > function to let fbdev track written pages of mmap'ed framebuffer > memory. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
[PATCH] drm/mgag200: Fix PLL setup for g200wb and g200ew
commit f86c3ed55920ca1d874758cc290890902a6cffc4 ("drm/mgag200: Split PLL
setup into compute and update functions") introduced a regression for
g200wb and g200ew.
The PLLs are not set up properly, and VGA screen stays
black, or displays "out of range" message.
MGA1064_WB_PIX_PLLC_N/M/P was mistakenly replaced with
MGA1064_PIX_PLLC_N/M/P which have different addresses.
Patch tested on a Dell T310 with g200wb
Fixes: f86c3ed55920ca1d874758cc290890902a6cffc4
Cc: sta...@vger.kernel.org
Signed-off-by: Jocelyn Falempe
---
drivers/gpu/drm/mgag200/mgag200_pll.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_pll.c
b/drivers/gpu/drm/mgag200/mgag200_pll.c
index e9ae22b4f813..52be08b744ad 100644
--- a/drivers/gpu/drm/mgag200/mgag200_pll.c
+++ b/drivers/gpu/drm/mgag200/mgag200_pll.c
@@ -404,9 +404,9 @@ mgag200_pixpll_update_g200wb(struct mgag200_pll *pixpll,
const struct mgag200_pl
udelay(50);
/* program pixel pll register */
- WREG_DAC(MGA1064_PIX_PLLC_N, xpixpllcn);
- WREG_DAC(MGA1064_PIX_PLLC_M, xpixpllcm);
- WREG_DAC(MGA1064_PIX_PLLC_P, xpixpllcp);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_N, xpixpllcn);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_M, xpixpllcm);
+ WREG_DAC(MGA1064_WB_PIX_PLLC_P, xpixpllcp);
udelay(50);
--
2.35.1
Re: [PATCH v5 5/5] drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table during probe
On Tue, 8 Mar 2022 at 19:55, Vinod Polimera wrote: > > use max clock during probe/bind sequence from the opp table. > The clock will be scaled down when framework sends an update. > > Signed-off-by: Vinod Polimera > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index d550f90..d9922b9 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -1221,6 +1221,7 @@ static int dpu_bind(struct device *dev, struct device > *master, void *data) > struct dpu_kms *dpu_kms; > struct dss_module_power *mp; > int ret = 0; > + unsigned long max_freq = ULONG_MAX; > > dpu_kms = devm_kzalloc(>dev, sizeof(*dpu_kms), GFP_KERNEL); > if (!dpu_kms) > @@ -1243,6 +1244,8 @@ static int dpu_bind(struct device *dev, struct device > *master, void *data) > return ret; > } > > + dev_pm_opp_find_freq_floor(dev, _freq); You leak a reference to the opp here. The function returns a value, which should be dev_pm_opp_put(). Moreover judging from the dev_pm_opp_set_rate() code I think you don't have to find an exact frequency, as it will call clk_round_rate()/_find_freq_ceil() anyway. Could you please check that it works? > + dev_pm_opp_set_rate(dev, max_freq); > platform_set_drvdata(pdev, dpu_kms); > > ret = msm_kms_init(_kms->base, _funcs); > -- > 2.7.4 > -- With best wishes Dmitry
Re: [PATCH] drm: remove min_order BUG_ON check
On 08/03/2022 13:59, Arunpravin wrote:
On 07/03/22 10:11 pm, Matthew Auld wrote:
On 07/03/2022 14:37, Arunpravin wrote:
place BUG_ON(order < min_order) outside do..while
loop as it fails Unigine Heaven benchmark.
Unigine Heaven has buffer allocation requests for
example required pages are 161 and alignment request
is 128. To allocate the remaining 33 pages, continues
the iteration to find the order value which is 5 and
when it compares with min_order = 7, enables the
BUG_ON(). To avoid this problem, placed the BUG_ON
check outside of do..while loop.
Signed-off-by: Arunpravin
---
drivers/gpu/drm/drm_buddy.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 72f52f293249..ed94c56b720f 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -669,10 +669,11 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
order = fls(pages) - 1;
min_order = ilog2(min_page_size) - ilog2(mm->chunk_size);
+ BUG_ON(order < min_order);
Isn't the issue that we are allowing a size that is not aligned to the
requested min_page_size? Should we not fix the caller(and throw a normal
error here), or perhaps add the round_up() here instead?
CASE 1:
when size is not aligned to the requested min_page_size, for instance,
required size = 161 pages, min_page_size = 128 pages, here we have 3
possible options,
a. AFAIK,This kind of situation is common in any workload,the first
allocation (i.e) 128 pages is aligned to min_page_size, Should we just
allocate the left over 33 pages (2 pow 5, 2 pow 0) since the caller does
know the left over pages are not in min_page_size alignment?
So IIUC looking at amdgpu_gem_create_ioctl(), userspace can specify some
arbitrary physical alignment for an object? Is that not meant to apply
to every page/chunk? The above example would only have the correct
physical alignment guaranteed for the first chunk, or so, is this the
expected ABI behaviour?
Also looking at this some more, the other related bug here is the
order-- == min_order check, since it now won't bail when order == 0,
leading to order = -1, if we are unlucky...
Originally, if asking for min_page_size > chunk_size, then the
allocation was meant to fail if it can't fill the resource request with
pages of at least that size(and also alignment). Or at least that was
the original meaning in i915 IIRC.
b. There are many such instances in unigine heaven workload (there would
be many such workloads), throwing a normal error would lower the FPS? is
it possible to fix at caller application?
c. adding the round_up() is possible, but in every such instances we end
up allocating extra unused memory. For example, if required pages = 1028
and min_page_size = 1024 pages, we end up round up of left over 4 pages
to the min_page_size, so the total size would be 2048 pages.
i.e if someone does:
alloc_blocks(mm, 0, end, 4096, 1<<16, , flags);
CASE 2:
I think this case should be detected (i.e) when min_page_size > size,
should we return -EINVAL?
This will still trigger the BUG_ON() even if we move it out of the loop,
AFAICT.
Should we just allow the CASE 1 proceed for the allocation and return
-EINVAL for the CASE 2?
+
do {
order = min(order, (unsigned int)fls(pages) - 1);
BUG_ON(order > mm->max_order);
- BUG_ON(order < min_order);
do {
if (flags & DRM_BUDDY_RANGE_ALLOCATION)
base-commit: 8025c79350b90e5a8029234d433578f12abbae2b
[PATCH v5 0/5] Update mdp clk to max supported value to support higher refresh rates
Kernel clock driver assumes that initial rate is the max rate for that clock and was not allowing it to scale beyond the assigned clock value. Drop the assigned clock rate property and vote on the mdp clock as per calculated value during the usecase. Changes in v2: - Remove assigned-clock-rate property and set mdp clk during resume sequence. - Add fixes tag. Changes in v3: - Remove extra line after fixes tag.(Stephen Boyd) - Add similar changes for sc7180, sdm845 which uses opp table for voting mdp clk.(Stephen Boyd) - Drop patch: "drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table" Changes in v4: - Add similar change for sm8250.(Dmitry) Changes in v5: - Add change to set mdp clk to max frequency in opp table during mdp probe/bind. Vinod Polimera (5): arm64/dts/qcom/sc7280: remove assigned-clock-rate property for mdp clk arm64/dts/qcom/sc7180: remove assigned-clock-rate property for mdp clk arm64/dts/qcom/sdm845: remove assigned-clock-rate property for mdp clk arm64/dts/qcom/sm8250: remove assigned-clock-rate property for mdp clk drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table during probe arch/arm64/boot/dts/qcom/sc7180.dtsi| 9 ++--- arch/arm64/boot/dts/qcom/sc7280.dtsi| 9 ++--- arch/arm64/boot/dts/qcom/sdm845.dtsi| 9 ++--- arch/arm64/boot/dts/qcom/sm8250.dtsi| 9 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ 5 files changed, 11 insertions(+), 28 deletions(-) -- 2.7.4
[PATCH v5 3/5] arm64/dts/qcom/sdm845: remove assigned-clock-rate property for mdp clk
Kernel clock driver assumes that initial rate is the
max rate for that clock and was not allowing it to scale
beyond the assigned clock value.
Drop the assigned clock rate property and vote on the mdp clock as per
calculated value during the usecase.
Fixes: 08c2a076d1("arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file")
Signed-off-by: Vinod Polimera
Reviewed-by: Stephen Boyd
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0d6286d..80dc486 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -4181,9 +4181,6 @@
< DISP_CC_MDSS_MDP_CLK>;
clock-names = "iface", "core";
- assigned-clocks = < DISP_CC_MDSS_MDP_CLK>;
- assigned-clock-rates = <3>;
-
interrupts = ;
interrupt-controller;
#interrupt-cells = <1>;
@@ -4214,10 +4211,8 @@
< DISP_CC_MDSS_VSYNC_CLK>;
clock-names = "gcc-bus", "iface", "bus",
"core", "vsync";
- assigned-clocks = <
DISP_CC_MDSS_MDP_CLK>,
- <
DISP_CC_MDSS_VSYNC_CLK>;
- assigned-clock-rates = <3>,
- <1920>;
+ assigned-clocks = <
DISP_CC_MDSS_VSYNC_CLK>;
+ assigned-clock-rates = <1920>;
operating-points-v2 = <_opp_table>;
power-domains = < SDM845_CX>;
--
2.7.4
[PATCH v5 2/5] arm64/dts/qcom/sc7180: remove assigned-clock-rate property for mdp clk
Kernel clock driver assumes that initial rate is the
max rate for that clock and was not allowing it to scale
beyond the assigned clock value.
Drop the assigned clock rate property and vote on the mdp clock as per
calculated value during the usecase.
Fixes: a3db7ad1af("arm64: dts: qcom: sc7180: add display dt nodes")
Signed-off-by: Vinod Polimera
Reviewed-by: Stephen Boyd
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index e1c46b8..eaab746 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2900,9 +2900,6 @@
< DISP_CC_MDSS_MDP_CLK>;
clock-names = "iface", "ahb", "core";
- assigned-clocks = < DISP_CC_MDSS_MDP_CLK>;
- assigned-clock-rates = <3>;
-
interrupts = ;
interrupt-controller;
#interrupt-cells = <1>;
@@ -2932,12 +2929,10 @@
< DISP_CC_MDSS_VSYNC_CLK>;
clock-names = "bus", "iface", "rot", "lut",
"core",
"vsync";
- assigned-clocks = <
DISP_CC_MDSS_MDP_CLK>,
- <
DISP_CC_MDSS_VSYNC_CLK>,
+ assigned-clocks = <
DISP_CC_MDSS_VSYNC_CLK>,
<
DISP_CC_MDSS_ROT_CLK>,
<
DISP_CC_MDSS_AHB_CLK>;
- assigned-clock-rates = <3>,
- <1920>,
+ assigned-clock-rates = <1920>,
<1920>,
<1920>;
operating-points-v2 = <_opp_table>;
--
2.7.4
[PATCH v5 4/5] arm64/dts/qcom/sm8250: remove assigned-clock-rate property for mdp clk
Kernel clock driver assumes that initial rate is the
max rate for that clock and was not allowing it to scale
beyond the assigned clock value.
Drop the assigned clock rate property and vote on the mdp clock as per
calculated value during the usecase.
Fixes: 7c1dffd471("arm64: dts: qcom: sm8250.dtsi: add display system nodes")
Signed-off-by: Vinod Polimera
Reviewed-by: Stephen Boyd
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index fdaf303..2105eb7 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3164,9 +3164,6 @@
< DISP_CC_MDSS_MDP_CLK>;
clock-names = "iface", "bus", "nrt_bus", "core";
- assigned-clocks = < DISP_CC_MDSS_MDP_CLK>;
- assigned-clock-rates = <46000>;
-
interrupts = ;
interrupt-controller;
#interrupt-cells = <1>;
@@ -3191,10 +3188,8 @@
< DISP_CC_MDSS_VSYNC_CLK>;
clock-names = "iface", "bus", "core", "vsync";
- assigned-clocks = <
DISP_CC_MDSS_MDP_CLK>,
- <
DISP_CC_MDSS_VSYNC_CLK>;
- assigned-clock-rates = <46000>,
- <1920>;
+ assigned-clocks = <
DISP_CC_MDSS_VSYNC_CLK>;
+ assigned-clock-rates = <1920>;
operating-points-v2 = <_opp_table>;
power-domains = < SM8250_MMCX>;
--
2.7.4
[PATCH v5 5/5] drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table during probe
use max clock during probe/bind sequence from the opp table. The clock will be scaled down when framework sends an update. Signed-off-by: Vinod Polimera --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index d550f90..d9922b9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1221,6 +1221,7 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) struct dpu_kms *dpu_kms; struct dss_module_power *mp; int ret = 0; + unsigned long max_freq = ULONG_MAX; dpu_kms = devm_kzalloc(>dev, sizeof(*dpu_kms), GFP_KERNEL); if (!dpu_kms) @@ -1243,6 +1244,8 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) return ret; } + dev_pm_opp_find_freq_floor(dev, _freq); + dev_pm_opp_set_rate(dev, max_freq); platform_set_drvdata(pdev, dpu_kms); ret = msm_kms_init(_kms->base, _funcs); -- 2.7.4
[PATCH v5 1/5] arm64/dts/qcom/sc7280: remove assigned-clock-rate property for mdp clk
Kernel clock driver assumes that initial rate is the
max rate for that clock and was not allowing it to scale
beyond the assigned clock value.
Drop the assigned clock rate property and vote on the mdp clock as per
calculated value during the usecase.
Changes in v2:
- Remove assigned-clock-rate property and set mdp clk during resume sequence.
- Add fixes tag.
Changes in v3:
- Remove extra line after fixes tag.(Stephen Boyd)
Fixes: 62fbdce91("arm64: dts: qcom: sc7280: add display dt nodes")
Signed-off-by: Vinod Polimera
Reviewed-by: Stephen Boyd
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index baf1653..408cf6c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2856,9 +2856,6 @@
"ahb",
"core";
- assigned-clocks = < DISP_CC_MDSS_MDP_CLK>;
- assigned-clock-rates = <3>;
-
interrupts = ;
interrupt-controller;
#interrupt-cells = <1>;
@@ -2892,11 +2889,9 @@
"lut",
"core",
"vsync";
- assigned-clocks = <
DISP_CC_MDSS_MDP_CLK>,
- <
DISP_CC_MDSS_VSYNC_CLK>,
+ assigned-clocks = <
DISP_CC_MDSS_VSYNC_CLK>,
< DISP_CC_MDSS_AHB_CLK>;
- assigned-clock-rates = <3>,
- <1920>,
+ assigned-clock-rates = <1920>,
<1920>;
operating-points-v2 = <_opp_table>;
power-domains = < SC7280_CX>;
--
2.7.4
[PATCH] drm/i915/guc: Use iosys_map interface to update lrc_desc
This patch is continuation of the effort to move all pointers in i915,
which at any point may be pointing to device memory or system memory, to
iosys_map interface.
More details about the need of this change is explained in the patch
series which initiated this task
https://patchwork.freedesktop.org/series/99711/
This patch converts all access to the lrc_desc through iosys_map
interfaces.
Cc: Lucas De Marchi
Cc: John Harrison
Cc: Matthew Brost
Cc: Umesh Nerlige Ramappa
Signed-off-by: Balasubramani Vivekanandan
---
drivers/gpu/drm/i915/gt/uc/intel_guc.h| 2 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 68 ---
2 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index e439e6c1ac8b..cbbc24dbaf0f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -168,7 +168,7 @@ struct intel_guc {
/** @lrc_desc_pool: object allocated to hold the GuC LRC descriptor
pool */
struct i915_vma *lrc_desc_pool;
/** @lrc_desc_pool_vaddr: contents of the GuC LRC descriptor pool */
- void *lrc_desc_pool_vaddr;
+ struct iosys_map lrc_desc_pool_vaddr;
/**
* @context_lookup: used to resolve intel_context from guc_id, if a
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 9ec03234d2c2..84b17ded886a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -467,13 +467,14 @@ static u32 *get_wq_pointer(struct guc_process_desc *desc,
return &__get_parent_scratch(ce)->wq[ce->parallel.guc.wqi_tail /
sizeof(u32)];
}
-static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index)
+static void __write_lrc_desc(struct intel_guc *guc, u32 index,
+struct guc_lrc_desc *desc)
{
- struct guc_lrc_desc *base = guc->lrc_desc_pool_vaddr;
+ unsigned int size = sizeof(struct guc_lrc_desc);
GEM_BUG_ON(index >= GUC_MAX_CONTEXT_ID);
- return [index];
+ iosys_map_memcpy_to(>lrc_desc_pool_vaddr, index * size, desc,
size);
}
static inline struct intel_context *__get_context(struct intel_guc *guc, u32
id)
@@ -489,20 +490,28 @@ static int guc_lrc_desc_pool_create(struct intel_guc *guc)
{
u32 size;
int ret;
+ void *addr;
size = PAGE_ALIGN(sizeof(struct guc_lrc_desc) *
GUC_MAX_CONTEXT_ID);
ret = intel_guc_allocate_and_map_vma(guc, size, >lrc_desc_pool,
-(void
**)>lrc_desc_pool_vaddr);
+);
+
if (ret)
return ret;
+ if (i915_gem_object_is_lmem(guc->lrc_desc_pool->obj))
+ iosys_map_set_vaddr_iomem(>lrc_desc_pool_vaddr,
+ (void __iomem *)addr);
+ else
+ iosys_map_set_vaddr(>lrc_desc_pool_vaddr, addr);
+
return 0;
}
static void guc_lrc_desc_pool_destroy(struct intel_guc *guc)
{
- guc->lrc_desc_pool_vaddr = NULL;
+ iosys_map_clear(>lrc_desc_pool_vaddr);
i915_vma_unpin_and_release(>lrc_desc_pool, I915_VMA_RELEASE_MAP);
}
@@ -513,9 +522,11 @@ static inline bool guc_submission_initialized(struct
intel_guc *guc)
static inline void _reset_lrc_desc(struct intel_guc *guc, u32 id)
{
- struct guc_lrc_desc *desc = __get_lrc_desc(guc, id);
+ unsigned int size = sizeof(struct guc_lrc_desc);
- memset(desc, 0, sizeof(*desc));
+ GEM_BUG_ON(id >= GUC_MAX_CONTEXT_ID);
+
+ iosys_map_memset(>lrc_desc_pool_vaddr, id * size, 0, size);
}
static inline bool ctx_id_mapped(struct intel_guc *guc, u32 id)
@@ -2233,7 +2244,7 @@ static void prepare_context_registration_info(struct
intel_context *ce)
struct intel_engine_cs *engine = ce->engine;
struct intel_guc *guc = >gt->uc.guc;
u32 ctx_id = ce->guc_id.id;
- struct guc_lrc_desc *desc;
+ struct guc_lrc_desc desc;
struct intel_context *child;
GEM_BUG_ON(!engine->mask);
@@ -2245,13 +2256,13 @@ static void prepare_context_registration_info(struct
intel_context *ce)
GEM_BUG_ON(i915_gem_object_is_lmem(guc->ct.vma->obj) !=
i915_gem_object_is_lmem(ce->ring->vma->obj));
- desc = __get_lrc_desc(guc, ctx_id);
- desc->engine_class = engine_class_to_guc_class(engine->class);
- desc->engine_submit_mask = engine->logical_mask;
- desc->hw_context_desc = ce->lrc.lrca;
- desc->priority = ce->guc_state.prio;
- desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
- guc_context_policy_init(engine, desc);
+ memset(, 0, sizeof(desc));
+ desc.engine_class = engine_class_to_guc_class(engine->class);
+ desc.engine_submit_mask =
Re: [PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
Hi Javier,
On Mon, Feb 14, 2022 at 2:37 PM Javier Martinez Canillas
wrote:
> This adds a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon
> OLED display controllers.
>
> It's only the core part of the driver and a bus specific driver is needed
> for each transport interface supported by the display controllers.
>
> Signed-off-by: Javier Martinez Canillas
Thanks for your patch, which is now commit a61732e808672cfa ("drm:
Add driver for Solomon SSD130x OLED displays") in drm/drm-next
Sorry for the delay, but finally I gave it a try on my Adafruit
FeatherWing 128x32 OLED.
Some of the weird issues (cursor disappears after printing some text,
more text also doesn't appear until I clear the display) are still there.
Unfortunately a regression was introduced since your v3: printed
text is mirrored upside-down. I.e. "E" is rendered correctly, but "L"
turns into "Γ" (Greek Gamma).
I suspect something went wrong with the display initialization
sequence.
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
Re: [PATCH v1 0/5] Add memory shrinker to VirtIO-GPU DRM driver
On Tue, Mar 8, 2022 at 5:17 AM Dmitry Osipenko wrote: > > Hello, > > This patchset introduces memory shrinker for the VirtIO-GPU DRM driver. > During OOM, the shrinker will release BOs that are marked as "not needed" > by userspace using the new madvise IOCTL. The userspace in this case is > the Mesa VirGL driver, it will mark the cached BOs as "not needed", > allowing kernel driver to release memory of the cached shmem BOs on lowmem > situations, preventing OOM kills. Will host memory pressure already trigger shrinker in guest? This is something I'm quite interested in for "virtgpu native contexts" (ie. native guest driver with new context type sitting on top of virtgpu), since that isn't using host storage BR, -R > This patchset includes couple fixes for problems I found while was working > on the shrinker, it also includes prerequisite DMA API usage improvement > needed by the shrinker. > > The Mesa and IGT patches will be kept on hold until this kernel series > will be approved and applied. > > This patchset was tested using Qemu and crosvm, including both cases of > IOMMU off/on. > > Mesa: https://gitlab.freedesktop.org/digetx/mesa/-/commits/virgl-madvise > IGT: > https://gitlab.freedesktop.org/digetx/igt-gpu-tools/-/tree/virtio-madvise > > Dmitry Osipenko (5): > drm/virtio: Correct drm_gem_shmem_get_sg_table() error handling > drm/virtio: Check whether transferred 2D BO is shmem > drm/virtio: Unlock GEM reservations in error code path > drm/virtio: Improve DMA API usage for shmem BOs > drm/virtio: Add memory shrinker > > drivers/gpu/drm/virtio/Makefile | 3 +- > drivers/gpu/drm/virtio/virtgpu_drv.c | 22 +++- > drivers/gpu/drm/virtio/virtgpu_drv.h | 31 - > drivers/gpu/drm/virtio/virtgpu_gem.c | 84 > drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c | 124 ++ > drivers/gpu/drm/virtio/virtgpu_ioctl.c| 37 ++ > drivers/gpu/drm/virtio/virtgpu_kms.c | 17 ++- > drivers/gpu/drm/virtio/virtgpu_object.c | 63 +++-- > drivers/gpu/drm/virtio/virtgpu_plane.c| 17 ++- > drivers/gpu/drm/virtio/virtgpu_vq.c | 30 +++-- > include/uapi/drm/virtgpu_drm.h| 14 ++ > 11 files changed, 373 insertions(+), 69 deletions(-) > create mode 100644 drivers/gpu/drm/virtio/virtgpu_gem_shrinker.c > > -- > 2.35.1 >
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On Tue, Mar 08, 2022 at 03:47:22PM +0100, Marek Vasut wrote:
> On 3/8/22 14:49, Maxime Ripard wrote:
> > On Tue, Mar 08, 2022 at 02:27:40PM +0100, Marek Vasut wrote:
> > > On 3/8/22 13:51, Maxime Ripard wrote:
> > > > On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
> > > > > On 3/8/22 11:07, Jagan Teki wrote:
> > > > > > On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
> > > > > > >
> > > > > > > On 3/8/22 09:03, Jagan Teki wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > [...]
> > > > > > >
> > > > > > > > > @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs
> > > > > > > > > chipone_bridge_funcs = {
> > > > > > > > > static int chipone_parse_dt(struct chipone *icn)
> > > > > > > > > {
> > > > > > > > > struct device *dev = icn->dev;
> > > > > > > > > + struct device_node *endpoint;
> > > > > > > > > struct drm_panel *panel;
> > > > > > > > > + int dsi_lanes;
> > > > > > > > > int ret;
> > > > > > > > >
> > > > > > > > > icn->vdd1 = devm_regulator_get_optional(dev,
> > > > > > > > > "vdd1");
> > > > > > > > > @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct
> > > > > > > > > chipone *icn)
> > > > > > > > > return PTR_ERR(icn->enable_gpio);
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > + endpoint =
> > > > > > > > > of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
> > > > > > > > > + dsi_lanes = of_property_count_u32_elems(endpoint,
> > > > > > > > > "data-lanes");
> > > > > > > > > + icn->host_node =
> > > > > > > > > of_graph_get_remote_port_parent(endpoint);
> > > > > > > > > + of_node_put(endpoint);
> > > > > > > > > +
> > > > > > > > > + if (!icn->host_node)
> > > > > > > > > + return -ENODEV;
> > > > > > > >
> > > > > > > > The non-ports-based OF graph returns a -19 example on the
> > > > > > > > Allwinner
> > > > > > > > Display pipeline in R16 [1].
> > > > > > > >
> > > > > > > > We need to have a helper to return host_node for non-ports as I
> > > > > > > > have
> > > > > > > > done it for drm_of_find_bridge.
> > > > > > > >
> > > > > > > > [1] https://patchwork.amarulasolutions.com/patch/1805/
> > > > > > >
> > > > > > > The link points to a patch marked "DO NOT MERGE", maybe that
> > > > > > > patch is
> > > > > > > missing the DSI host port@0 OF graph link ? Both port@0 and
> > > > > > > port@1 are
> > > > > > > required, see:
> > > > > > >
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
> > > > > > >
> > > > > > > What is "non-ports-based OF graph" ?
> > > > > > >
> > > > > > > I don't see drm_of_find_bridge() in linux-next , what is that ?
> > > > > >
> > > > > > port@0 is optional as some of the DSI host OF-graph represent the
> > > > > > bridge or panel as child nodes instead of ports. (i think dt-binding
> > > > > > has to fix it to make port@0 optional)
> > > > >
> > > > > The current upstream DT binding document says:
> > > > >
> > > > > required:
> > > > > - port@0
> > > > > - port@1
> > > > >
> > > > > So port@0 is mandatory.
> > > >
> > > > In the binding, sure, but fundamentally the DT excerpt Jagan provided is
> > > > correct. If the bridge supports DCS, there's no reason to use the OF
> > > > graph in the first place: the bridge node will be a child node of the
> > > > MIPI-DSI controller (and there's no obligation to use the OF-graph for a
> > > > MIPI-DSI controller).
> > > >
> > > > I believe port@0 should be made optional (or downright removed if
> > > > MIPI-DCS in the only control bus).
> > >
> > > That's out of scope of this series anyway, so Jagan can implement patches
> > > for that mode if needed.
> >
> > Not really? You can't count on the port@0 being there generally
> > speaking, so you can't count on data-lanes being there either, which
> > exactly what you're doing in this patch.
>
> I can because the upstream DT bindings currently say port@0 must be present,
> see above. If that requirement should be relaxed, sure, but that's a
> separate series.
And another upstream DT bindings say that you don't need them at all.
Yes, there's a conflict. Yes, it's unfortunate. But the generic DSI
binding is far more relevant than a single bridge driver.
So figuring it out is very much a prerequisite to that series,
especially since those patches effectively make the OF-graph mandatory
in some situations, while it was purely aesthetics before.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v6 2/6] drm/format-helper: Add drm_fb_xrgb8888_to_mono_reversed()
Hi Javier,
On Mon, Feb 14, 2022 at 2:37 PM Javier Martinez Canillas
wrote:
> Add support to convert from XR24 to reversed monochrome for drivers that
> control monochromatic display panels, that only have 1 bit per pixel.
>
> The function does a line-by-line conversion doing an intermediate step
> first from XR24 to 8-bit grayscale and then to reversed monochrome.
>
> The drm_fb_gray8_to_mono_reversed_line() helper was based on code from
> drivers/gpu/drm/tiny/repaper.c driver.
>
> Signed-off-by: Javier Martinez Canillas
> Reviewed-by: Thomas Zimmermann
> Reviewed-by: Andy Shevchenko
Thanks for your patch, which is now commit bcf8b616deb87941
("drm/format-helper: Add drm_fb_xrgb_to_mono_reversed()")
in drm/drm-next.
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -12,9 +12,11 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> +#include
> #include
>
> static unsigned int clip_offset(const struct drm_rect *clip, unsigned int
> pitch, unsigned int cpp)
> @@ -591,3 +593,111 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int
> dst_pitch, uint32_t dst_for
> return -EINVAL;
> }
> EXPORT_SYMBOL(drm_fb_blit_toio);
> +
> +static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src,
> unsigned int pixels,
"pixels" is not the number of pixels to process, but the number of
bytes in the monochrome destination buffer.
> + unsigned int start_offset,
> unsigned int end_len)
> +{
> + unsigned int xb, i;
> +
> + for (xb = 0; xb < pixels; xb++) {
> + unsigned int start = 0, end = 8;
> + u8 byte = 0x00;
> +
> + if (xb == 0 && start_offset)
> + start = start_offset;
> +
> + if (xb == pixels - 1 && end_len)
> + end = end_len;
> +
> + for (i = start; i < end; i++) {
> + unsigned int x = xb * 8 + i;
> +
> + byte >>= 1;
> + if (src[x] >> 7)
> + byte |= BIT(7);
> + }
> + *dst++ = byte;
> + }
The above is IMHO very hard to read.
I think it can be made simpler by passing the total number of pixels
to process instead of "pixels" (which is bytes) and "end_len".
> +}
> +
> +/**
> + * drm_fb_xrgb_to_mono_reversed - Convert XRGB to reversed monochrome
> + * @dst: reversed monochrome destination buffer
> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
> + * @src: XRGB source buffer
> + * @fb: DRM framebuffer
> + * @clip: Clip rectangle area to copy
> + *
> + * DRM doesn't have native monochrome support.
> + * Such drivers can announce the commonly supported XR24 format to userspace
> + * and use this function to convert to the native format.
> + *
> + * This function uses drm_fb_xrgb_to_gray8() to convert to grayscale and
> + * then the result is converted from grayscale to reversed monohrome.
> + */
> +void drm_fb_xrgb_to_mono_reversed(void *dst, unsigned int dst_pitch,
> const void *vaddr,
> + const struct drm_framebuffer *fb, const
> struct drm_rect *clip)
> +{
> + unsigned int linepixels = drm_rect_width(clip);
> + unsigned int lines = clip->y2 - clip->y1;
drm_rect_height(clip)?
> + unsigned int cpp = fb->format->cpp[0];
> + unsigned int len_src32 = linepixels * cpp;
> + struct drm_device *dev = fb->dev;
> + unsigned int start_offset, end_len;
> + unsigned int y;
> + u8 *mono = dst, *gray8;
> + u32 *src32;
> +
> + if (drm_WARN_ON(dev, fb->format->format != DRM_FORMAT_XRGB))
> + return;
> +
> + /*
> +* The reversed mono destination buffer contains 1 bit per pixel
> +* and destination scanlines have to be in multiple of 8 pixels.
> +*/
> + if (!dst_pitch)
> + dst_pitch = DIV_ROUND_UP(linepixels, 8);
This is not correct when clip->x1 is not a multiple of 8 pixels.
Should be:
DIV_ROUND_UP(linepixels + clip->x1 % 8, 8);
> +
> + drm_WARN_ONCE(dev, dst_pitch % 8 != 0, "dst_pitch is not a multiple
> of 8\n");
This triggers for me: dst_pitch = 1.
Which is perfectly fine, when flashing an 8-pixel wide cursor ;-)
> +
> + /*
> +* The cma memory is write-combined so reads are uncached.
> +* Speed up by fetching one line at a time.
> +*
> +* Also, format conversion from XR24 to reversed monochrome
> +* are done line-by-line but are converted to 8-bit grayscale
> +* as an intermediate step.
> +*
> +* Allocate a buffer to be used for both copying from the cma
> +* memory and to store the intermediate grayscale line pixels.
> +*/
> + src32 = kmalloc(len_src32 + linepixels, GFP_KERNEL);
> + if (!src32)
> +
Re: [PATCH 0/3] Fix MediaTek display dt-bindings issues
On Fri, Mar 4, 2022 at 5:49 PM Chun-Kuang Hu wrote: > > Hi, Rob: > > Would you like to take this series into your tree, or I take this > series into my tree? I can't. I don't have the broken commits in my tree. Rob
[PATCH 2/2] drm: ssd130x: Always apply segment remap setting
From: Chen-Yu Tsai
Currently the ssd130x driver only sets the segment remap setting when
the device tree requests it; it however does not clear the setting if
it is not requested. This leads to the setting incorrectly persisting
if the hardware is always on and has no reset GPIO wired. This might
happen when a developer is trying to find the correct settings for an
unknown module, and cause the developer to get confused because the
settings from the device tree are not consistently applied.
Make the driver apply the segment remap setting consistently, setting
the value correctly based on the device tree setting. This also makes
this setting's behavior consistent with the other settings, which are
always applied.
Fixes: a61732e80867 ("drm: Add driver for Solomon SSD130x OLED displays")
Signed-off-by: Chen-Yu Tsai
---
drivers/gpu/drm/solomon/ssd130x.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x.c
b/drivers/gpu/drm/solomon/ssd130x.c
index ccd378135589..d08d86ef07bc 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -48,7 +48,7 @@
#define SSD130X_CONTRAST 0x81
#define SSD130X_SET_LOOKUP_TABLE 0x91
#define SSD130X_CHARGE_PUMP0x8d
-#define SSD130X_SEG_REMAP_ON 0xa1
+#define SSD130X_SET_SEG_REMAP 0xa0
#define SSD130X_DISPLAY_OFF0xae
#define SSD130X_SET_MULTIPLEX_RATIO0xa8
#define SSD130X_DISPLAY_ON 0xaf
@@ -61,6 +61,8 @@
#define SSD130X_SET_COM_PINS_CONFIG0xda
#define SSD130X_SET_VCOMH 0xdb
+#define SSD130X_SET_SEG_REMAP_MASK GENMASK(0, 0)
+#define SSD130X_SET_SEG_REMAP_SET(val)
FIELD_PREP(SSD130X_SET_SEG_REMAP_MASK, (val))
#define SSD130X_SET_COM_SCAN_DIR_MASK GENMASK(3, 3)
#define SSD130X_SET_COM_SCAN_DIR_SET(val)
FIELD_PREP(SSD130X_SET_COM_SCAN_DIR_MASK, (val))
#define SSD130X_SET_CLOCK_DIV_MASK GENMASK(3, 0)
@@ -235,7 +237,7 @@ static void ssd130x_power_off(struct ssd130x_device
*ssd130x)
static int ssd130x_init(struct ssd130x_device *ssd130x)
{
- u32 precharge, dclk, com_invdir, compins, chargepump;
+ u32 precharge, dclk, com_invdir, compins, chargepump, seg_remap;
int ret;
/* Set initial contrast */
@@ -244,11 +246,11 @@ static int ssd130x_init(struct ssd130x_device *ssd130x)
return ret;
/* Set segment re-map */
- if (ssd130x->seg_remap) {
- ret = ssd130x_write_cmd(ssd130x, 1, SSD130X_SEG_REMAP_ON);
- if (ret < 0)
- return ret;
- }
+ seg_remap = (SSD130X_SET_SEG_REMAP |
+SSD130X_SET_SEG_REMAP_SET(ssd130x->seg_remap));
+ ret = ssd130x_write_cmd(ssd130x, 1, seg_remap);
+ if (ret < 0)
+ return ret;
/* Set COM direction */
com_invdir = (SSD130X_SET_COM_SCAN_DIR |
--
2.34.1
[PATCH 1/2] drm: ssd130x: Fix COM scan direction register mask
From: Chen-Yu Tsai
The SSD130x's command to toggle COM scan direction uses bit 3 and only
bit 3 to set the direction of the scanout. The driver has an incorrect
GENMASK(3, 2), causing the setting to be set on bit 2, rendering it
ineffective.
Fix the mask to only bit 3, so that the requested setting is applied
correctly.
Fixes: a61732e80867 ("drm: Add driver for Solomon SSD130x OLED displays")
Signed-off-by: Chen-Yu Tsai
---
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 ce4dc20412e0..ccd378135589 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -61,7 +61,7 @@
#define SSD130X_SET_COM_PINS_CONFIG0xda
#define SSD130X_SET_VCOMH 0xdb
-#define SSD130X_SET_COM_SCAN_DIR_MASK GENMASK(3, 2)
+#define SSD130X_SET_COM_SCAN_DIR_MASK GENMASK(3, 3)
#define SSD130X_SET_COM_SCAN_DIR_SET(val)
FIELD_PREP(SSD130X_SET_COM_SCAN_DIR_MASK, (val))
#define SSD130X_SET_CLOCK_DIV_MASK GENMASK(3, 0)
#define SSD130X_SET_CLOCK_DIV_SET(val)
FIELD_PREP(SSD130X_SET_CLOCK_DIV_MASK, (val))
--
2.34.1
Re: [PATCH v1 5/5] drm/virtio: Add memory shrinker
On 3/8/22 16:17, Dmitry Osipenko wrote: > @@ -246,20 +246,28 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane > *plane, > struct virtio_gpu_device *vgdev = dev->dev_private; > struct virtio_gpu_framebuffer *vgfb; > struct virtio_gpu_object *bo; > + int err; > > if (!new_state->fb) > return 0; > > vgfb = to_virtio_gpu_framebuffer(new_state->fb); > bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); > - if (!bo || (plane->type == DRM_PLANE_TYPE_PRIMARY && !bo->guest_blob)) > + > + err = virtio_gpu_gem_pin(bo); > + if (err) > + return err; I just noticed that this produces a refcount debug warning because I missed to initialize the refcount when BO is created. That warning splat was hidden by a huge lockdep splat produced by drm_aperture_remove_conflicting_pci_framebuffers(), which probably should be fixed. I'll correct it in v2.
