dri-devel September 2019

dri-devel@lists.freedesktop.org

315 participants
2278 discussions

[PATCH v3] drm/ioctl: Add a ioctl to label GEM objects
by Rohan Garg 27 Sep '19

27 Sep '19

DRM_IOCTL_BO_SET_LABEL lets you label GEM objects, making it easier to debug issues in userspace applications. Changes in v2: - Hoist the IOCTL up into the drm_driver framework Changes in v3: - Introduce a drm_gem_set_label for drivers to use internally in order to label a GEM object - Hoist string copying up into the IOCTL - Fix documentation - Move actual gem labeling into drm_gem_adopt_label Signed-off-by: Rohan Garg <rohan.garg(a)collabora.com> --- drivers/gpu/drm/drm_gem.c | 69 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_internal.h | 3 ++ drivers/gpu/drm/drm_ioctl.c | 1 + include/drm/drm_drv.h | 13 +++++++ include/drm/drm_gem.h | 7 ++++ include/uapi/drm/drm.h | 20 ++++++++++ 6 files changed, 113 insertions(+) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 6854f5867d51..24ab463e3a33 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -941,6 +941,70 @@ drm_gem_release(struct drm_device *dev, struct drm_file *file_private) idr_destroy(&file_private->object_idr); } +int drm_gem_set_label_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_gem_set_label_object *args = data; + struct drm_gem_object *gem_obj; + int ret = 0; + char *label = NULL; + + if (!args->len || !args->name) + return -EINVAL; + + if (!dev->driver->label) + return -EOPNOTSUPP; + + if (args->len > 0) + label = strndup_user(u64_to_user_ptr(args->name), args->len); + + if (IS_ERR(label)) { + ret = PTR_ERR(label); + goto err; + } + + gem_obj = drm_gem_object_lookup(file_priv, args->handle); + if (!gem_obj) { + DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle); + ret = -ENOENT; + goto err; + } + + dev->driver->label(gem_obj, label, args->len); + drm_gem_object_put_unlocked(gem_obj); + + return 0; +err: + kfree(label); + return ret; +} + +static void drm_gem_adopt_label(struct drm_gem_object *gem_obj, char *label, int len) +{ + if ((len == 0 && label == NULL) || gem_obj->label) { + kfree(gem_obj->label); + gem_obj->label = NULL; + } + + gem_obj->label = label; +} +EXPORT_SYMBOL(drm_gem_adopt_label); + +int drm_gem_set_label(struct drm_gem_object *gem_obj, const char *label, int len) +{ + char *new_label = NULL; + + if (label) { + new_label = kstrndup(label, len, GFP_KERNEL); + if (IS_ERR(new_label)) + return PTR_ERR(new_label); + } + + drm_gem_adopt_label(gem_obj, new_label, len); + return 0; +} +EXPORT_SYMBOL(drm_gem_set_label); + /** * drm_gem_object_release - release GEM buffer object resources * @obj: GEM buffer object @@ -958,6 +1022,11 @@ drm_gem_object_release(struct drm_gem_object *obj) dma_resv_fini(&obj->_resv); drm_gem_free_mmap_offset(obj); + + if (obj->label) { + kfree(obj->label); + obj->label = NULL; + } } EXPORT_SYMBOL(drm_gem_object_release); diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 51a2055c8f18..c4d6e8bcd34f 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -137,6 +137,9 @@ int drm_gem_pin(struct drm_gem_object *obj); void drm_gem_unpin(struct drm_gem_object *obj); void *drm_gem_vmap(struct drm_gem_object *obj); void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr); +int drm_gem_set_label_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv); +int drm_gem_set_label(struct drm_gem_object *gem_obj, const char *label, int len); /* drm_debugfs.c drm_debugfs_crc.c */ #if defined(CONFIG_DEBUG_FS) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index f675a3bb2c88..3316e6a12a26 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -709,6 +709,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER), DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER), + DRM_IOCTL_DEF(DRM_IOCTL_GEM_SET_LABEL, drm_gem_set_label_ioctl, DRM_RENDER_ALLOW), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 8976afe48c1c..5150ddf5031c 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -715,6 +715,19 @@ struct drm_driver { struct drm_device *dev, uint32_t handle); + /** + * @label: + * + * This label's a buffer object. + * + * Called by the user via ioctl. + * + * Returns: + * + * Zero on success, negative errno on failure. + */ + int (*label)(struct drm_gem_object *gem_obj, const char *label, int len); + /** * @gem_vm_ops: Driver private ops for this object * diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 6aaba14f5972..f801c054e972 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -237,6 +237,13 @@ struct drm_gem_object { */ int name; + /** + * @label: + * + * Label for this object, should be a human readable string. + */ + char *label; + /** * @dma_buf: * diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 8a5b2f8f8eb9..56fa009e9be6 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -626,6 +626,25 @@ struct drm_gem_open { __u64 size; }; +/** struct drm_gem_set_label_object - ioctl argument for labelling BOs. + * + * This label's a BO with a userspace label + * + */ +struct drm_gem_set_label_object { + /** Handle for the object being labelled. */ + __u32 handle; + + /** Label and label length. + * len includes the trailing NUL. + */ + __u32 len; + __u64 name; + + /** Flags */ + int flags; +}; + #define DRM_CAP_DUMB_BUFFER 0x1 #define DRM_CAP_VBLANK_HIGH_CRTC 0x2 #define DRM_CAP_DUMB_PREFERRED_DEPTH 0x3 @@ -946,6 +965,7 @@ extern "C" { #define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct drm_syncobj_timeline_array) #define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct drm_syncobj_transfer) #define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, struct drm_syncobj_timeline_array) +#define DRM_IOCTL_GEM_SET_LABEL DRM_IOWR(0xCE, struct drm_gem_set_label_object) /** * Device specific ioctls should only be in their respective headers -- 2.17.1

2 1

[Bug 111651] [CI][RESUME] igt@gem_ctx_shared@q-independent-bsd[12] - Skip - mmio base not known
by bugzilla-daemon＠freedesktop.org 27 Sep '19

27 Sep '19

1 0

[PATCH v7 0/5] Support mipitx for mt8183
by Jitao Shi 27 Sep '19

27 Sep '19

Changes since v6: - add the mipitx driving control in device tree. - config mipitx impedance with calibration data. Changes since v5: - remove mipi_tx->ref_clk - remove mt8183 pll prepare unprepare Changes since v4: - fine tune the mipi_tx->ref_clk and mipi_tx->pll sequence 1. Prepare mipi_tx->ref_clk 2. Prepare mipi_tx->pll 3. Enable mipi_tx->ref_clk 4. Enable mipi_tx->pll Changes since v3: - turn off PLL before setting PLL parameters. Changes since v2: - update Acked-by: Rob Herring <robh(a)kernel.org> - update mt8183 max bit rate support Changes since v1: - update dt-bindings document for mt8183 mipitx. - remove mtk_mipitx_clk_get_ops and assign clk_ops in probe. - fix the lincence - remove txdiv1 from mtk_mipi_tx_pll_prepare Jitao Shi (5): dt-bindings: display: mediatek: update dsi supported chips drm/mediatek: separate mipi_tx to different file drm/mediatek: add mipi_tx driver for mt8183 drm/mediatek: add the mipitx driving control in device tree drm/mediatek: config mipitx impedance with calibration data .../display/mediatek/mediatek,dsi.txt | 15 +- drivers/gpu/drm/mediatek/Makefile | 2 + drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 344 ++---------------- drivers/gpu/drm/mediatek/mtk_mipi_tx.h | 51 +++ drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c | 289 +++++++++++++++ drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c | 220 +++++++++++ 6 files changed, 603 insertions(+), 318 deletions(-) create mode 100644 drivers/gpu/drm/mediatek/mtk_mipi_tx.h create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c -- 2.21.0

2 8

[PATCH v3 10/11] drm/amdgpu: job is secure iff CS is secure (v4)
by Huang, Ray 26 Sep '19

26 Sep '19

Mark a job as secure, if and only if the command submission flag has the secure flag set. v2: fix the null job pointer while in vmid 0 submission. v3: Context --> Command submission. v4: filling cs parser with cs->in.flags Signed-off-by: Huang Rui <ray.huang(a)amd.com> Co-developed-by: Luben Tuikov <luben.tuikov(a)amd.com> Signed-off-by: Luben Tuikov <luben.tuikov(a)amd.com> Reviewed-by: Alex Deucher <alexander.deucher(a)amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 11 ++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 2 ++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 697e8e5..fd60695 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -485,6 +485,9 @@ struct amdgpu_cs_parser { uint64_t bytes_moved; uint64_t bytes_moved_vis; + /* secure cs */ + bool is_secure; + /* user fence */ struct amdgpu_bo_list_entry uf_entry; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 51f3db0..9038dc1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -133,6 +133,14 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs goto free_chunk; } + /** + * The command submission (cs) is a union, so an assignment to + * 'out' is destructive to the cs (at least the first 8 + * bytes). For this reason, inquire about the flags before the + * assignment to 'out'. + */ + p->is_secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE; + /* get chunks */ chunk_array_user = u64_to_user_ptr(cs->in.chunks); if (copy_from_user(chunk_array, chunk_array_user, @@ -1252,8 +1260,9 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, p->ctx->preamble_presented = true; } - cs->out.handle = seq; + job->secure = p->is_secure; job->uf_sequence = seq; + cs->out.handle = seq; amdgpu_job_free_resources(job); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c index e1dc229..cb9b650 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c @@ -210,7 +210,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs, if (job && ring->funcs->emit_cntxcntl) { status |= job->preamble_status; status |= job->preemption_status; - amdgpu_ring_emit_cntxcntl(ring, status, false); + amdgpu_ring_emit_cntxcntl(ring, status, job->secure); } for (i = 0; i < num_ibs; ++i) { @@ -229,7 +229,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs, } if (ring->funcs->emit_tmz) - amdgpu_ring_emit_tmz(ring, false, false); + amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false); #ifdef CONFIG_X86_64 if (!(adev->flags & AMD_IS_APU)) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h index dc7ee93..aa0e375 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h @@ -63,6 +63,8 @@ struct amdgpu_job { uint64_t uf_addr; uint64_t uf_sequence; + /* the job is due to a secure command submission */ + bool secure; }; int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs, -- 2.7.4

3 5

[PATCH v2] dt-bindings: gpu: Convert Samsung Image Scaler to dt-schema
by Marek Szyprowski 26 Sep '19

26 Sep '19

From: Maciej Falkowski <m.falkowski(a)samsung.com> Convert Samsung Image Scaler to newer dt-schema format. Signed-off-by: Maciej Falkowski <m.falkowski(a)samsung.com> Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com> --- v2: - Removed quotation marks from string in 'compatible' property - Added if-then statement for 'clocks' and 'clock-names' property - Added include directive to example - Added GIC_SPI macro to example Best regards, Maciej Falkowski --- .../bindings/gpu/samsung-scaler.txt | 27 ------- .../bindings/gpu/samsung-scaler.yaml | 71 +++++++++++++++++++ 2 files changed, 71 insertions(+), 27 deletions(-) delete mode 100644 Documentation/devicetree/bindings/gpu/samsung-scaler.txt create mode 100644 Documentation/devicetree/bindings/gpu/samsung-scaler.yaml diff --git a/Documentation/devicetree/bindings/gpu/samsung-scaler.txt b/Documentation/devicetree/bindings/gpu/samsung-scaler.txt deleted file mode 100644 index 9c3d98105dfd..000000000000 --- a/Documentation/devicetree/bindings/gpu/samsung-scaler.txt +++ /dev/null @@ -1,27 +0,0 @@ -* Samsung Exynos Image Scaler - -Required properties: - - compatible : value should be one of the following: - (a) "samsung,exynos5420-scaler" for Scaler IP in Exynos5420 - (b) "samsung,exynos5433-scaler" for Scaler IP in Exynos5433 - - - reg : Physical base address of the IP registers and length of memory - mapped region. - - - interrupts : Interrupt specifier for scaler interrupt, according to format - specific to interrupt parent. - - - clocks : Clock specifier for scaler clock, according to generic clock - bindings. (See Documentation/devicetree/bindings/clock/exynos*.txt) - - - clock-names : Names of clocks. For exynos scaler, it should be "mscl" - on 5420 and "pclk", "aclk" and "aclk_xiu" on 5433. - -Example: - scaler@12800000 { - compatible = "samsung,exynos5420-scaler"; - reg = <0x12800000 0x1294>; - interrupts = <0 220 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clock CLK_MSCL0>; - clock-names = "mscl"; - }; diff --git a/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml b/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml new file mode 100644 index 000000000000..af19930d052e --- /dev/null +++ b/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml @@ -0,0 +1,71 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/gpu/samsung-scaler.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Samsung Exynos SoC Image Scaler + +maintainers: + - Inki Dae <inki.dae(a)samsung.com> + +properties: + compatible: + enum: + - samsung,exynos5420-scaler + - samsung,exynos5433-scaler + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +if: + properties: + compatible: + contains: + const: samsung,exynos5420-scaler +then: + properties: + clocks: + items: + - description: mscl clock + + clock-names: + items: + - const: mscl +else: + properties: + clocks: + items: + - description: mscl clock + - description: aclk clock + - description: aclk_xiu clock + + clock-names: + items: + - const: pclk + - const: aclk + - const: aclk_xiu + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + +examples: + - | + #include <dt-bindings/clock/exynos5420.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + scaler@12800000 { + compatible = "samsung,exynos5420-scaler"; + reg = <0x12800000 0x1294>; + interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clock CLK_MSCL0>; + clock-names = "mscl"; + }; + -- 2.17.1

4 5

[PATCH 0/2] drm/i915: Acquire locks with interrupts disabled
by Sebastian Andrzej Siewior 26 Sep '19

26 Sep '19

Two locking related changed which popped up on PREEMPT_RT. Sebastian

2 4

[PATCH v4] drm: Don't free jobs in wait_event_interruptible()
by Steven Price 26 Sep '19

26 Sep '19

drm_sched_cleanup_jobs() attempts to free finished jobs, however because it is called as the condition of wait_event_interruptible() it must not sleep. Unfortuantly some free callbacks (notibly for Panfrost) do sleep. Instead let's rename drm_sched_cleanup_jobs() to drm_sched_get_cleanup_job() and simply return a job for processing if there is one. The caller can then call the free_job() callback outside the wait_event_interruptible() where sleeping is possible before re-checking and returning to sleep if necessary. Signed-off-by: Steven Price <steven.price(a)arm.com> --- Changes from v3: * drm_sched_main() re-arms the timeout for the next job after calling free_job() drivers/gpu/drm/scheduler/sched_main.c | 45 +++++++++++++++----------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 9a0ee74d82dc..148468447ba9 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -622,43 +622,41 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb) } /** - * drm_sched_cleanup_jobs - destroy finished jobs + * drm_sched_get_cleanup_job - fetch the next finished job to be destroyed * * @sched: scheduler instance * - * Remove all finished jobs from the mirror list and destroy them. + * Returns the next finished job from the mirror list (if there is one) + * ready for it to be destroyed. */ -static void drm_sched_cleanup_jobs(struct drm_gpu_scheduler *sched) +static struct drm_sched_job * +drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched) { + struct drm_sched_job *job = NULL; unsigned long flags; /* Don't destroy jobs while the timeout worker is running */ if (sched->timeout != MAX_SCHEDULE_TIMEOUT && !cancel_delayed_work(&sched->work_tdr)) - return; - + return NULL; - while (!list_empty(&sched->ring_mirror_list)) { - struct drm_sched_job *job; + spin_lock_irqsave(&sched->job_list_lock, flags); - job = list_first_entry(&sched->ring_mirror_list, + job = list_first_entry_or_null(&sched->ring_mirror_list, struct drm_sched_job, node); - if (!dma_fence_is_signaled(&job->s_fence->finished)) - break; - spin_lock_irqsave(&sched->job_list_lock, flags); + if (job && dma_fence_is_signaled(&job->s_fence->finished)) { /* remove job from ring_mirror_list */ list_del_init(&job->node); - spin_unlock_irqrestore(&sched->job_list_lock, flags); - - sched->ops->free_job(job); + } else { + job = NULL; + /* queue timeout for next job */ + drm_sched_start_timeout(sched); } - /* queue timeout for next job */ - spin_lock_irqsave(&sched->job_list_lock, flags); - drm_sched_start_timeout(sched); spin_unlock_irqrestore(&sched->job_list_lock, flags); + return job; } /** @@ -698,12 +696,21 @@ static int drm_sched_main(void *param) struct drm_sched_fence *s_fence; struct drm_sched_job *sched_job; struct dma_fence *fence; + struct drm_sched_job *cleanup_job = NULL; wait_event_interruptible(sched->wake_up_worker, - (drm_sched_cleanup_jobs(sched), + (cleanup_job = drm_sched_get_cleanup_job(sched)) || (!drm_sched_blocked(sched) && (entity = drm_sched_select_entity(sched))) || - kthread_should_stop())); + kthread_should_stop()); + + while (cleanup_job) { + sched->ops->free_job(cleanup_job); + /* queue timeout for next job */ + drm_sched_start_timeout(sched); + + cleanup_job = drm_sched_get_cleanup_job(sched); + } if (!entity) continue; -- 2.20.1

2 5

[Bug 111836] gmail>>attachment button is not working
by bugzilla-daemon＠freedesktop.org 26 Sep '19

26 Sep '19

https://bugs.freedesktop.org/show_bug.cgi?id=111836 Bug ID: 111836 Summary: gmail>>attachment button is not working Product: DRI Version: unspecified Hardware: Other OS: Windows (All) Status: NEW Severity: normal Priority: not set Component: General Assignee: dri-devel(a)lists.freedesktop.org Reporter: ramitha.pr1993(a)gmail.com login to gmail click on compose mail cannot attach files -- You are receiving this mail because: You are the assignee for the bug.

1 2

[PATCH] drm: Don't free jobs in wait_event_interruptible()
by Steven Price 26 Sep '19

26 Sep '19

drm_sched_cleanup_jobs() attempts to free finished jobs, however because it is called as the condition of wait_event_interruptible() it must not sleep. Unfortunately some free callbacks (notably for Panfrost) do sleep. Instead let's rename drm_sched_cleanup_jobs() to drm_sched_get_cleanup_job() and simply return a job for processing if there is one. The caller can then call the free_job() callback outside the wait_event_interruptible() where sleeping is possible before re-checking and returning to sleep if necessary. Signed-off-by: Steven Price <steven.price(a)arm.com> --- drivers/gpu/drm/scheduler/sched_main.c | 44 ++++++++++++++------------ 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 9a0ee74d82dc..0ed4aaa4e6d1 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -622,43 +622,41 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb) } /** - * drm_sched_cleanup_jobs - destroy finished jobs + * drm_sched_get_cleanup_job - fetch the next finished job to be destroyed * * @sched: scheduler instance * - * Remove all finished jobs from the mirror list and destroy them. + * Returns the next finished job from the mirror list (if there is one) + * ready for it to be destroyed. */ -static void drm_sched_cleanup_jobs(struct drm_gpu_scheduler *sched) +static struct drm_sched_job * +drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched) { + struct drm_sched_job *job = NULL; unsigned long flags; /* Don't destroy jobs while the timeout worker is running */ if (sched->timeout != MAX_SCHEDULE_TIMEOUT && !cancel_delayed_work(&sched->work_tdr)) - return; - - - while (!list_empty(&sched->ring_mirror_list)) { - struct drm_sched_job *job; + return NULL; - job = list_first_entry(&sched->ring_mirror_list, + job = list_first_entry_or_null(&sched->ring_mirror_list, struct drm_sched_job, node); - if (!dma_fence_is_signaled(&job->s_fence->finished)) - break; - spin_lock_irqsave(&sched->job_list_lock, flags); + spin_lock_irqsave(&sched->job_list_lock, flags); + + if (job && dma_fence_is_signaled(&job->s_fence->finished)) { /* remove job from ring_mirror_list */ list_del_init(&job->node); - spin_unlock_irqrestore(&sched->job_list_lock, flags); - - sched->ops->free_job(job); + } else { + job = NULL; + /* queue timeout for next job */ + drm_sched_start_timeout(sched); } - /* queue timeout for next job */ - spin_lock_irqsave(&sched->job_list_lock, flags); - drm_sched_start_timeout(sched); spin_unlock_irqrestore(&sched->job_list_lock, flags); + return job; } /** @@ -698,12 +696,18 @@ static int drm_sched_main(void *param) struct drm_sched_fence *s_fence; struct drm_sched_job *sched_job; struct dma_fence *fence; + struct drm_sched_job *cleanup_job = NULL; wait_event_interruptible(sched->wake_up_worker, - (drm_sched_cleanup_jobs(sched), + (cleanup_job = drm_sched_get_cleanup_job(sched)) || (!drm_sched_blocked(sched) && (entity = drm_sched_select_entity(sched))) || - kthread_should_stop())); + kthread_should_stop()); + + while (cleanup_job) { + sched->ops->free_job(cleanup_job); + cleanup_job = drm_sched_get_cleanup_job(sched); + } if (!entity) continue; -- 2.20.1

3 10

[PATCH v3] drm: Don't free jobs in wait_event_interruptible()
by Steven Price 26 Sep '19

26 Sep '19

drm_sched_cleanup_jobs() attempts to free finished jobs, however because it is called as the condition of wait_event_interruptible() it must not sleep. Unfortuantly some free callbacks (notibly for Panfrost) do sleep. Instead let's rename drm_sched_cleanup_jobs() to drm_sched_get_cleanup_job() and simply return a job for processing if there is one. The caller can then call the free_job() callback outside the wait_event_interruptible() where sleeping is possible before re-checking and returning to sleep if necessary. Signed-off-by: Steven Price <steven.price(a)arm.com> --- Changes from v2: * Actually move list_first_entry_or_null() within the lock drivers/gpu/drm/scheduler/sched_main.c | 42 ++++++++++++++------------ 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 9a0ee74d82dc..e4bd792f2b29 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -622,43 +622,41 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb) } /** - * drm_sched_cleanup_jobs - destroy finished jobs + * drm_sched_get_cleanup_job - fetch the next finished job to be destroyed * * @sched: scheduler instance * - * Remove all finished jobs from the mirror list and destroy them. + * Returns the next finished job from the mirror list (if there is one) + * ready for it to be destroyed. */ -static void drm_sched_cleanup_jobs(struct drm_gpu_scheduler *sched) +static struct drm_sched_job * +drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched) { + struct drm_sched_job *job = NULL; unsigned long flags; /* Don't destroy jobs while the timeout worker is running */ if (sched->timeout != MAX_SCHEDULE_TIMEOUT && !cancel_delayed_work(&sched->work_tdr)) - return; - + return NULL; - while (!list_empty(&sched->ring_mirror_list)) { - struct drm_sched_job *job; + spin_lock_irqsave(&sched->job_list_lock, flags); - job = list_first_entry(&sched->ring_mirror_list, + job = list_first_entry_or_null(&sched->ring_mirror_list, struct drm_sched_job, node); - if (!dma_fence_is_signaled(&job->s_fence->finished)) - break; - spin_lock_irqsave(&sched->job_list_lock, flags); + if (job && dma_fence_is_signaled(&job->s_fence->finished)) { /* remove job from ring_mirror_list */ list_del_init(&job->node); - spin_unlock_irqrestore(&sched->job_list_lock, flags); - - sched->ops->free_job(job); + } else { + job = NULL; + /* queue timeout for next job */ + drm_sched_start_timeout(sched); } - /* queue timeout for next job */ - spin_lock_irqsave(&sched->job_list_lock, flags); - drm_sched_start_timeout(sched); spin_unlock_irqrestore(&sched->job_list_lock, flags); + return job; } /** @@ -698,12 +696,18 @@ static int drm_sched_main(void *param) struct drm_sched_fence *s_fence; struct drm_sched_job *sched_job; struct dma_fence *fence; + struct drm_sched_job *cleanup_job = NULL; wait_event_interruptible(sched->wake_up_worker, - (drm_sched_cleanup_jobs(sched), + (cleanup_job = drm_sched_get_cleanup_job(sched)) || (!drm_sched_blocked(sched) && (entity = drm_sched_select_entity(sched))) || - kthread_should_stop())); + kthread_should_stop()); + + while (cleanup_job) { + sched->ops->free_job(cleanup_job); + cleanup_job = drm_sched_get_cleanup_job(sched); + } if (!entity) continue; -- 2.20.1

2 3

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel September 2019