[git pull] drm fixes for 5.12-rc5
Hi Linus, As expected last week things were overly quiet so this week things seem to have caught up. It still isn't too major. msm and amdgpu lead the size here, the msm fixes are pretty varied across the driver, the amdgpu one is mostly the S0ix fixes with some other minor ones. Otherwise there are a few i915 fixes and one each for nouveau, etnaviv and rcar-du. Dave. drm-fixes-2021-03-26: drm fixes for 5.12-rc5 msm: - pll fixes - shutdown hook fix - runtime resume fix - clear_oob fix - kms locking fix - display aux retry fix rcar-du: - warn_on in encoder init fix etnaviv: - Use FOLL_FORCE and FOLL_LONGTERM i915: - DisplayPort LTTPR fixes around link training and limiting it according to supported spec version. - Fix enabled_planes bitmask to really represent only logically enabled planes. - Fix DSS CTL registers for ICL DSI transcoders - Fix the GT fence revocation runtime PM logic. nouveau: - cursor size regression fix amdgpu: - S0ix fixes - Add PCI ID - Polaris PCIe DPM fix - Display fix for high refresh rate monitors The following changes since commit 0d02ec6b3136c73c09e7859f0d0e4e2c4c07b49b: Linux 5.12-rc4 (2021-03-21 14:56:43 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-03-26 for you to fetch changes up to 09d78dde88ef95a27b54a6e450ee700ccabdf39d: Merge tag 'drm-msm-fixes-2021-02-25' of https://gitlab.freedesktop.org/drm/msm into drm-fixes (2021-03-26 13:04:17 +1000) drm fixes for 5.12-rc5 msm: - pll fixes - shutdown hook fix - runtime resume fix - clear_oob fix - kms locking fix - display aux retry fix rcar-du: - warn_on in encoder init fix etnaviv: - Use FOLL_FORCE and FOLL_LONGTERM i915: - DisplayPort LTTPR fixes around link training and limiting it according to supported spec version. - Fix enabled_planes bitmask to really represent only logically enabled planes. - Fix DSS CTL registers for ICL DSI transcoders - Fix the GT fence revocation runtime PM logic. nouveau: - cursor size regression fix amdgpu: - S0ix fixes - Add PCI ID - Polaris PCIe DPM fix - Display fix for high refresh rate monitors Alex Deucher (11): drm/amdgpu: rework S3/S4/S0ix state handling drm/amdgpu: don't evict vram on APUs for suspend to ram (v4) drm/amdgpu: clean up non-DC suspend/resume handling drm/amdgpu: move s0ix check into amdgpu_device_ip_suspend_phase2 (v3) drm/amdgpu: re-enable suspend phase 2 for S0ix drm/amdgpu/swsmu: skip gfx cgpg on s0ix suspend drm/amdgpu: update comments about s0ix suspend/resume drm/amdgpu: drop S0ix checks around CG/PG in suspend drm/amdgpu: skip kfd suspend/resume for S0ix drm/amdgpu: Add additional Sienna Cichlid PCI ID drm/amdgpu/display: restore AUX_DPHY_TX_CONTROL for DCN2.x Daniel Vetter (2): drm/etnaviv: Use FOLL_FORCE for userptr drm/etnaviv: User FOLL_LONGTERM in userptr Dave Airlie (6): Merge tag 'du-fixes-20210316' of git://linuxtv.org/pinchartl/media into drm-fixes Merge tag 'drm-misc-fixes-2021-03-25' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'drm-intel-fixes-2021-03-25-1' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge branch 'linux-5.12' of git://github.com/skeggsb/linux into drm-fixes Merge tag 'amd-drm-fixes-5.12-2021-03-24' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Merge tag 'drm-msm-fixes-2021-02-25' of https://gitlab.freedesktop.org/drm/msm into drm-fixes Dmitry Baryshkov (4): drm/msm/dsi: fix check-before-set in the 7nm dsi_pll code drm/msm/dsi_pll_7nm: Solve TODO for multiplier frac_bits assignment drm/msm/dsi_pll_7nm: Fix variable usage for pll_lockdet_rate drm/msm: fix shutdown hook in case GPU components failed to bind Douglas Anderson (1): drm/msm: Fix speed-bin support not to access outside valid memory Fabio Estevam (1): drm/msm: Fix suspend/resume on i.MX5 Imre Deak (4): drm/i915/ilk-glk: Fix link training on links with LTTPRs drm/i915: Disable LTTPR support when the DPCD rev < 1.4 drm/i915: Disable LTTPR support when the LTTPR rev < 1.4 drm/i915: Fix the GT fence revocation runtime PM logic Jani Nikula (1): drm/i915/dsc: fix DSS CTL register usage for ICL DSI transcoders Jonathan Marek (1): drm/msm: fix a6xx_gmu_clear_oob Jordan Crouse (1): drm/msm: a6xx: Make sure the SQE microcode is safe Kalyan Thota (1): drm/msm/disp/dpu1: icc path needs to be set before dpu runtime resume Kenneth Feng (1): drm/amd/pm: workaround for audio noise issue Kieran Bingham (1): drm: rcar-du: Use drmm_encoder_alloc() to manage encoder Konrad Dybcio (1): drm/msm/adreno: a5xx_power: Don't apply A540 lm_setup to other GPUs Lyude Paul (1):
[PATCH v2] drm/imx: ipuv3-plane: Remove two unnecessary export symbols
The ipu_plane_disable_deferred() and ipu_planes_assign_pre() functions have not been used by any other modules but only imxdrm itself internally since imxdrm and imx-ipuv3-crtc were merged in one module. So, this patch removes export symbols for the two functions. Fixes: 3d1df96ad468 (drm/imx: merge imx-drm-core and ipuv3-crtc in one module) Signed-off-by: Liu Ying --- v2: * Fix commit message typo - s/ipu_plane_assign_pre/ipu_planes_assign_pre/ drivers/gpu/drm/imx/ipuv3-plane.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index fa50097..35681f9 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -264,7 +264,6 @@ void ipu_plane_disable_deferred(struct drm_plane *plane) ipu_plane_disable(ipu_plane, false); } } -EXPORT_SYMBOL_GPL(ipu_plane_disable_deferred); static void ipu_plane_state_reset(struct drm_plane *plane) { @@ -821,7 +820,6 @@ int ipu_planes_assign_pre(struct drm_device *dev, return 0; } -EXPORT_SYMBOL_GPL(ipu_planes_assign_pre); struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, int dma, int dp, unsigned int possible_crtcs, -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Update MST First Link Slot Information Based on Encoding Format
8b/10b encoding format requires to reserve the first slot for recording metadata. Real data transmission starts from the second slot, with a total of available 63 slots available. In 128b/132b encoding format, metadata is transmitted separately in LLCP packet before MTP. Real data transmission starts from the first slot, with a total of 64 slots available. Update the slot information after link detect. Signed-off-by: Fangzhi Zuo --- drivers/gpu/drm/drm_dp_mst_topology.c | 48 ++- include/drm/drm_dp_mst_helper.h | 8 + 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 42a0c6888c33..577ed4224778 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3382,7 +3382,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr) struct drm_dp_payload req_payload; struct drm_dp_mst_port *port; int i, j; - int cur_slots = 1; + int cur_slots = mgr->first_link_start_slot; mutex_lock(>payload_lock); for (i = 0; i < mgr->max_payloads; i++) { @@ -4302,8 +4302,13 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); - /* max. time slots - one slot for MTP header */ - if (num_slots > 63) + /** +* first_link_total_avail_slots: max. time slots +* first slot reserved for MTP header in 8b/10b, +* but not required for 128b/132b +*/ + + if (num_slots > mgr->first_link_total_avail_slots) return -ENOSPC; return num_slots; } @@ -4314,8 +4319,12 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr, { int ret; - /* max. time slots - one slot for MTP header */ - if (slots > 63) + /** +* first_link_total_avail_slots: max. time slots +* first slot reserved for MTP header in 8b/10b, +* but not required for 128b/132b +*/ + if (slots > mgr->first_link_total_avail_slots) return -ENOSPC; vcpi->pbn = pbn; @@ -4488,6 +4497,25 @@ int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); +/* + * drm_dp_mst_update_first_link_slot_info() + * update the first link's total available slots and starting slot + * @mgr: manager to store the slot info. + * @encoding_format: detected link encoding format + */ +void drm_dp_mst_update_first_link_slot_info( + struct drm_dp_mst_topology_mgr *mgr, uint8_t encoding_format) +{ + if (encoding_format == DP_CAP_ANSI_128B132B) { + mgr->first_link_total_avail_slots = 64; + mgr->first_link_start_slot = 0; + } + DRM_DEBUG_KMS("%s encoding format on 0x%p -> total %d slots, start at slot %d\n", + (encoding_format == DP_CAP_ANSI_128B132B) ? "128b/132b":"8b/10b", + mgr, mgr->first_link_total_avail_slots, mgr->first_link_start_slot); +} +EXPORT_SYMBOL(drm_dp_mst_update_first_link_slot_info); + /** * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel * @mgr: manager for this port @@ -4518,8 +4546,8 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, ret = drm_dp_init_vcpi(mgr, >vcpi, pbn, slots); if (ret) { - DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n", - DIV_ROUND_UP(pbn, mgr->pbn_div), ret); + DRM_DEBUG_KMS("failed to init vcpi slots=%d max=%d ret=%d\n", + DIV_ROUND_UP(pbn, mgr->pbn_div), mgr->first_link_total_avail_slots, ret); drm_dp_mst_topology_put_port(port); goto out; } @@ -5162,7 +5190,7 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state) { struct drm_dp_vcpi_allocation *vcpi; - int avail_slots = 63, payload_count = 0; + int avail_slots = mgr->first_link_total_avail_slots, payload_count = 0; list_for_each_entry(vcpi, _state->vcpis, next) { /* Releasing VCPI is always OK-even if the port is gone */ @@ -5191,7 +5219,7 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct drm_dp_mst_topology_mgr *mgr, } DRM_DEBUG_ATOMIC("[MST MGR:%p] mst state %p VCPI avail=%d used=%d\n", mgr, mst_state, avail_slots, -63 - avail_slots); +mgr->first_link_total_avail_slots - avail_slots); return 0; } @@ -5455,6 +5483,8 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, if (!mgr->proposed_vcpis) return -ENOMEM; set_bit(0, >payload_mask); + mgr->first_link_total_avail_slots = 63; +
Re: [PATCH] drm/amdkfd: Fix cat debugfs hang_hws file causes system crash bug
Am 2021-03-23 um 10:56 a.m. schrieb Alex Deucher: > Applied. Thanks! Thanks. I thought we fixed this before by making the file write-only. But I guess that's not sufficient to stop root from reading it: commit 2bdac179e217a0c0b548a8c60524977586621b19 Author: Felix Kuehling Date: Thu Dec 19 22:36:55 2019 -0500 drm/amdkfd: Fix permissions of hang_hws Reading from /sys/kernel/debug/kfd/hang_hws would cause a kernel oops because we didn't implement a read callback. Set the permission to write-only to prevent that. Signed-off-by: Felix Kuehling Reviewed-by: shaoyunl Signed-off-by: Alex Deucher Now that we have a sensible message in the file, I guess we should officially make it readable again. Regards, Felix > > Alex > > On Sun, Mar 21, 2021 at 5:33 AM Qu Huang wrote: >> Here is the system crash log: >> [ 1272.884438] BUG: unable to handle kernel NULL pointer dereference at >> (null) >> [ 1272.88] IP: [< (null)>] (null) >> [ 1272.884447] PGD 825b09067 PUD 8267c8067 PMD 0 >> [ 1272.884452] Oops: 0010 [#1] SMP >> [ 1272.884509] CPU: 13 PID: 3485 Comm: cat Kdump: loaded Tainted: G >> [ 1272.884515] task: 9a38dbd4d140 ti: 9a37cd3b8000 task.ti: >> 9a37cd3b8000 >> [ 1272.884517] RIP: 0010:[<>] [< (null)>] >> (null) >> [ 1272.884520] RSP: 0018:9a37cd3bbe68 EFLAGS: 00010203 >> [ 1272.884522] RAX: RBX: RCX: >> 00014d5f >> [ 1272.884524] RDX: fff4 RSI: 0001 RDI: >> 9a38aca4d200 >> [ 1272.884526] RBP: 9a37cd3bbed0 R08: 9a38dcd5f1a0 R09: >> 9a31ffc07300 >> [ 1272.884527] R10: 9a31ffc07300 R11: addd5e9d R12: >> 9a38b4e0fb00 >> [ 1272.884529] R13: 0001 R14: 9a37cd3bbf18 R15: >> 9a38aca4d200 >> [ 1272.884532] FS: 7feccaa67740() GS:9a38dcd4() >> knlGS: >> [ 1272.884534] CS: 0010 DS: ES: CR0: 80050033 >> [ 1272.884536] CR2: CR3: 0008267c CR4: >> 003407e0 >> [ 1272.884537] Call Trace: >> [ 1272.884544] [] ? seq_read+0x130/0x440 >> [ 1272.884548] [] vfs_read+0x9f/0x170 >> [ 1272.884552] [] SyS_read+0x7f/0xf0 >> [ 1272.884557] [] system_call_fastpath+0x22/0x27 >> [ 1272.884558] Code: Bad RIP value. >> [ 1272.884562] RIP [< (null)>] (null) >> [ 1272.884564] RSP >> [ 1272.884566] CR2: >> >> Signed-off-by: Qu Huang >> --- >> drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c | 7 ++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c >> b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c >> index 511712c..673d5e3 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c >> @@ -33,6 +33,11 @@ static int kfd_debugfs_open(struct inode *inode, struct >> file *file) >> >> return single_open(file, show, NULL); >> } >> +static int kfd_debugfs_hang_hws_read(struct seq_file *m, void *data) >> +{ >> + seq_printf(m, "echo gpu_id > hang_hws\n"); >> + return 0; >> +} >> >> static ssize_t kfd_debugfs_hang_hws_write(struct file *file, >> const char __user *user_buf, size_t size, loff_t *ppos) >> @@ -94,7 +99,7 @@ void kfd_debugfs_init(void) >> debugfs_create_file("rls", S_IFREG | 0444, debugfs_root, >> kfd_debugfs_rls_by_device, _debugfs_fops); >> debugfs_create_file("hang_hws", S_IFREG | 0200, debugfs_root, >> - NULL, _debugfs_hang_hws_fops); >> + kfd_debugfs_hang_hws_read, >> _debugfs_hang_hws_fops); >> } >> >> void kfd_debugfs_fini(void) >> -- >> 1.8.3.1 >> >> ___ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v3] drm/scheduler re-insert Bailing job to avoid memleak
[AMD Official Use Only - Internal Distribution Only] Hi, Andrey, >>how u handle non guilty singnaled jobs in drm_sched_stop, currently looks >>like you don't call put for them and just explicitly free them as before Good point, I missed that place. Will cover that in my next patch. >>Also sched->free_guilty seems useless with the new approach. Yes, I agree. >>Do we even need the cleanup mechanism at drm_sched_get_cleanup_job with this >>approach... I am not quite sure about that for now, let me think about this topic today. Hi, Christian, should I add a fence and get/put to that fence rather than using an explicit refcount? And another concerns? Thanks, Jack -Original Message- From: Grodzovsky, Andrey Sent: Friday, March 26, 2021 12:32 AM To: Zhang, Jack (Jian) ; Christian König ; dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; Koenig, Christian ; Liu, Monk ; Deng, Emily ; Rob Herring ; Tomeu Vizoso ; Steven Price Subject: Re: [PATCH v3] drm/scheduler re-insert Bailing job to avoid memleak There are a few issues here like - how u handle non guilty singnaled jobs in drm_sched_stop, currently looks like you don't call put for them and just explicitly free them as before. Also sched->free_guilty seems useless with the new approach. Do we even need the cleanup mechanism at drm_sched_get_cleanup_job with this approach... But first - We need Christian to express his opinion on this since I think he opposed refcounting jobs and that we should concentrate on fences instead. Christian - can you chime in here ? Andrey On 2021-03-25 5:51 a.m., Zhang, Jack (Jian) wrote: > [AMD Official Use Only - Internal Distribution Only] > > > Hi, Andrey > > Thank you for your good opinions. > > I literally agree with you that the refcount could solve the > get_clean_up_up cocurrent job gracefully, and no need to re-insert the > > job back anymore. > > I quickly made a draft for this idea as follows: > > How do you like it? I will start implement to it after I got your > acknowledge. > > Thanks, > > Jack > > +void drm_job_get(struct drm_sched_job *s_job) > > +{ > > + kref_get(_job->refcount); > > +} > > + > > +void drm_job_do_release(struct kref *ref) > > +{ > > + struct drm_sched_job *s_job; > > + struct drm_gpu_scheduler *sched; > > + > > + s_job = container_of(ref, struct drm_sched_job, refcount); > > + sched = s_job->sched; > > + sched->ops->free_job(s_job); > > +} > > + > > +void drm_job_put(struct drm_sched_job *s_job) > > +{ > > + kref_put(_job->refcount, drm_job_do_release); > > +} > > + > > static void drm_sched_job_begin(struct drm_sched_job *s_job) > > { > > struct drm_gpu_scheduler *sched = s_job->sched; > > + kref_init(_job->refcount); > > + drm_job_get(s_job); > > spin_lock(>job_list_lock); > > list_add_tail(_job->node, >ring_mirror_list); > > drm_sched_start_timeout(sched); > > @@ -294,17 +316,16 @@ static void drm_sched_job_timedout(struct > work_struct *work) > > * drm_sched_cleanup_jobs. It will be reinserted back > after sched->thread > > * is parked at which point it's safe. > > */ > > - list_del_init(>node); > > + drm_job_get(job); > > spin_unlock(>job_list_lock); > > job->sched->ops->timedout_job(job); > > - > > + drm_job_put(job); > > /* > > * Guilty job did complete and hence needs to be > manually removed > > * See drm_sched_stop doc. > > */ > > if (sched->free_guilty) { > > - job->sched->ops->free_job(job); > > sched->free_guilty = false; > > } > > } else { > > @@ -355,20 +376,6 @@ void drm_sched_stop(struct drm_gpu_scheduler > *sched, struct drm_sched_job *bad) > > - /* > > -* Reinsert back the bad job here - now it's safe as > > -* drm_sched_get_cleanup_job cannot race against us and > release the > > -* bad job at this point - we parked (waited for) any in > progress > > -* (earlier) cleanups and drm_sched_get_cleanup_job will not > be called > > -* now until the scheduler thread is unparked. > > -*/ > > - if (bad && bad->sched == sched) > > - /* > > -* Add at the head of the queue to reflect it was the > earliest > > -* job extracted. > > -*/ > > - list_add(>node, >ring_mirror_list); > > - > > /* > > * Iterate the job list from later to earlier one and either > deactive > > * their HW callbacks or remove them from mirror list if they > already > > @@ -774,7 +781,7 @@ static int drm_sched_main(void *param) > > kthread_should_stop()); > > if (cleanup_job) { >
RE: [PATCH v3] drm/scheduler re-insert Bailing job to avoid memleak
[AMD Official Use Only - Internal Distribution Only] Hi, Steve, Thank you for your detailed comments. But currently the patch is not finalized. We found some potential race condition even with this patch. The solution is under discussion and hopefully we could find an ideal one. After that, I will start to consider other drm-driver if it will influence other drivers(except for amdgpu). Best, Jack -Original Message- From: Steven Price Sent: Monday, March 22, 2021 11:29 PM To: Zhang, Jack (Jian) ; dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; Koenig, Christian ; Grodzovsky, Andrey ; Liu, Monk ; Deng, Emily ; Rob Herring ; Tomeu Vizoso Subject: Re: [PATCH v3] drm/scheduler re-insert Bailing job to avoid memleak On 15/03/2021 05:23, Zhang, Jack (Jian) wrote: > [AMD Public Use] > > Hi, Rob/Tomeu/Steven, > > Would you please help to review this patch for panfrost driver? > > Thanks, > Jack Zhang > > -Original Message- > From: Jack Zhang > Sent: Monday, March 15, 2021 1:21 PM > To: dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; > Koenig, Christian ; Grodzovsky, Andrey > ; Liu, Monk ; Deng, Emily > > Cc: Zhang, Jack (Jian) > Subject: [PATCH v3] drm/scheduler re-insert Bailing job to avoid > memleak > > re-insert Bailing jobs to avoid memory leak. > > V2: move re-insert step to drm/scheduler logic > V3: add panfrost's return value for bailing jobs in case it hits the > memleak issue. This commit message could do with some work - it's really hard to decipher what the actual problem you're solving is. > > Signed-off-by: Jack Zhang > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 +++- > drivers/gpu/drm/amd/amdgpu/amdgpu_job.c| 8 ++-- > drivers/gpu/drm/panfrost/panfrost_job.c| 4 ++-- > drivers/gpu/drm/scheduler/sched_main.c | 8 +++- > include/drm/gpu_scheduler.h| 1 + > 5 files changed, 19 insertions(+), 6 deletions(-) > [...] > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c > b/drivers/gpu/drm/panfrost/panfrost_job.c > index 6003cfeb1322..e2cb4f32dae1 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -444,7 +444,7 @@ static enum drm_gpu_sched_stat > panfrost_job_timedout(struct drm_sched_job >* spurious. Bail out. >*/ > if (dma_fence_is_signaled(job->done_fence)) > -return DRM_GPU_SCHED_STAT_NOMINAL; > +return DRM_GPU_SCHED_STAT_BAILING; > > dev_err(pfdev->dev, "gpu sched timeout, js=%d, config=0x%x, status=0x%x, > head=0x%x, tail=0x%x, sched_job=%p", > js, > @@ -456,7 +456,7 @@ static enum drm_gpu_sched_stat > panfrost_job_timedout(struct drm_sched_job > > /* Scheduler is already stopped, nothing to do. */ > if (!panfrost_scheduler_stop(>js->queue[js], sched_job)) > -return DRM_GPU_SCHED_STAT_NOMINAL; > +return DRM_GPU_SCHED_STAT_BAILING; > > /* Schedule a reset if there's no reset in progress. */ > if (!atomic_xchg(>reset.pending, 1)) This looks correct to me - in these two cases drm_sched_stop() is not called on the sched_job, so it looks like currently the job will be leaked. > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > b/drivers/gpu/drm/scheduler/sched_main.c > index 92d8de24d0a1..a44f621fb5c4 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -314,6 +314,7 @@ static void drm_sched_job_timedout(struct work_struct > *work) > { > struct drm_gpu_scheduler *sched; > struct drm_sched_job *job; > +int ret; > > sched = container_of(work, struct drm_gpu_scheduler, > work_tdr.work); > > @@ -331,8 +332,13 @@ static void drm_sched_job_timedout(struct work_struct > *work) > list_del_init(>list); > spin_unlock(>job_list_lock); > > -job->sched->ops->timedout_job(job); > +ret = job->sched->ops->timedout_job(job); > > +if (ret == DRM_GPU_SCHED_STAT_BAILING) { > +spin_lock(>job_list_lock); > +list_add(>node, >ring_mirror_list); > +spin_unlock(>job_list_lock); > +} I think we could really do with a comment somewhere explaining what "bailing" means in this context. For the Panfrost case we have two cases: * The GPU job actually finished while the timeout code was running (done_fence is signalled). * The GPU is already in the process of being reset (Panfrost has multiple queues, so mostly like a bad job in another queue). I'm also not convinced that (for Panfrost) it makes sense to be adding the jobs back to the list. For the first case above clearly the job could just be freed (it's complete). The second case is more interesting and Panfrost currently doesn't handle this well. In theory the driver could try to rescue the job ('soft stop' in Mali language) so that it could be resubmitted. Panfrost doesn't currently support that, so attempting to resubmit the job is almost certainly going to fail. It's on my TODO list to look at improving Panfrost in this regard, but sadly still quite far down. Steve > /* >* Guilty
Re: [RFC PATCH 08/11] drm/bridge: ti-sn65dsi86: Implement bridge connector operations
Hi Doug, On Wed, Mar 24, 2021 at 03:46:28PM -0700, Doug Anderson wrote: > On Sun, Mar 21, 2021 at 8:02 PM Laurent Pinchart wrote: > > > > Implement the bridge connector-related .get_edid() operation, and report > > the related bridge capabilities and type. The .get_edid() operation is > > implemented with the same backend as the EDID retrieval from the > > connector .get_modes() operation. > > > > Signed-off-by: Laurent Pinchart > > --- > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 30 ++- > > 1 file changed, 25 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > index dc300fab4319..6f6e075544e8 100644 > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > @@ -261,6 +261,18 @@ static void ti_sn_debugfs_remove(struct ti_sn_bridge > > *pdata) > > pdata->debugfs = NULL; > > } > > > > +static struct edid *__ti_sn_bridge_get_edid(struct ti_sn_bridge *pdata, > > + struct drm_connector *connector) > > +{ > > + struct edid *edid; > > + > > + pm_runtime_get_sync(pdata->dev); > > + edid = drm_get_edid(connector, >aux.ddc); > > + pm_runtime_put(pdata->dev); > > As mentioned in my patch [1], the above is a bit iffy for eDP. > Specifically just doing a pm_runtime_get_sync() isn't enough to > actually be able to read the EDID. We also need to do any panel power > sequencing and potentially set the "ignore HPD" bit in the bridge to > actually read the EDID. > > I'll try to find some time to make this better--let's see if I get > distracted before I manage it. I've replied to your review of 05/11 with a possible solution. Fingers crossed :-) > > + > > + return edid; > > +} > > + > > /* > > - > > * DRM Connector Operations > > */ > > @@ -277,11 +289,8 @@ static int ti_sn_bridge_connector_get_modes(struct > > drm_connector *connector) > > struct edid *edid = pdata->edid; > > int num, ret; > > > > - if (!edid) { > > - pm_runtime_get_sync(pdata->dev); > > - edid = pdata->edid = drm_get_edid(connector, > > >aux.ddc); > > - pm_runtime_put(pdata->dev); > > - } > > + if (!edid) > > + edid = pdata->edid = __ti_sn_bridge_get_edid(pdata, > > connector); > > It feels weird to me that we are now exposing the get_edid() function > directly but we still need to implement get_modes(). I guess this is > because we might need to fallback to the hardcoded modes? ...but that > seems like it would be a common pattern for eDP bridges... Bridges are moving from creating the connector internally to providing a set of bridge operations to support connector creation externally (by the drm_bridge_connector helper, or by display drivers directly if needed). During the transition, both need to be implemented, hence the bridge .get_edid() operation for the new model, and the connector .get_modes() operation for the old model. > > if (edid && drm_edid_is_valid(edid)) { > > ret = drm_connector_update_edid_property(connector, edid); > > @@ -871,12 +880,21 @@ static void ti_sn_bridge_post_disable(struct > > drm_bridge *bridge) > > pm_runtime_put_sync(pdata->dev); > > } > > > > +static struct edid *ti_sn_bridge_get_edid(struct drm_bridge *bridge, > > + struct drm_connector *connector) > > +{ > > + struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge); > > + > > + return __ti_sn_bridge_get_edid(pdata, connector); > > +} > > + > > static const struct drm_bridge_funcs ti_sn_bridge_funcs = { > > .attach = ti_sn_bridge_attach, > > .pre_enable = ti_sn_bridge_pre_enable, > > .enable = ti_sn_bridge_enable, > > .disable = ti_sn_bridge_disable, > > .post_disable = ti_sn_bridge_post_disable, > > + .get_edid = ti_sn_bridge_get_edid, > > }; > > > > /* > > - > > @@ -1335,6 +1353,8 @@ static int ti_sn_bridge_probe(struct i2c_client > > *client, > > > > pdata->bridge.funcs = _sn_bridge_funcs; > > pdata->bridge.of_node = client->dev.of_node; > > + pdata->bridge.ops = DRM_BRIDGE_OP_EDID; > > + pdata->bridge.type = DRM_MODE_CONNECTOR_eDP; > > Even with the few comments above, I think this is still fine to move > us in the right direction. Unless I manage to fix up the EDID reading > (in which case your patch would conflict and would need to be > tweaked), then: > > Reviewed-by: Douglas Anderson > > > [1] > https://lore.kernel.org/r/20210304155144.3.I60a7fb23ce4589006bc95c64ab8d15c74b876e68@changeid/ -- Regards, Laurent Pinchart ___ dri-devel mailing list
[PATCH] [v2] drm/i915: Remove repeated declaration
struct drm_i915_private, struct intel_crtc_state and struct intel_crtc is declared twice. Remove the duplicate. Reviewed-by: José Roberto de Souza Signed-off-by: Wan Jiabing --- Changelog: v2: - Modify subject line. - Delete trailing whitespace in commit log. --- drivers/gpu/drm/i915/display/intel_crt.h | 1 - drivers/gpu/drm/i915/display/intel_display.h | 1 - drivers/gpu/drm/i915/display/intel_vrr.h | 1 - 3 files changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_crt.h b/drivers/gpu/drm/i915/display/intel_crt.h index 1b3fba359efc..6c5c44600cbd 100644 --- a/drivers/gpu/drm/i915/display/intel_crt.h +++ b/drivers/gpu/drm/i915/display/intel_crt.h @@ -11,7 +11,6 @@ enum pipe; struct drm_encoder; struct drm_i915_private; -struct drm_i915_private; bool intel_crt_port_enabled(struct drm_i915_private *dev_priv, i915_reg_t adpa_reg, enum pipe *pipe); diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h index 76f8a805b0a3..29cb6d84ed70 100644 --- a/drivers/gpu/drm/i915/display/intel_display.h +++ b/drivers/gpu/drm/i915/display/intel_display.h @@ -48,7 +48,6 @@ struct i915_ggtt_view; struct intel_atomic_state; struct intel_crtc; struct intel_crtc_state; -struct intel_crtc_state; struct intel_digital_port; struct intel_dp; struct intel_encoder; diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h index fac01bf4ab50..96f9c9c27ab9 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.h +++ b/drivers/gpu/drm/i915/display/intel_vrr.h @@ -15,7 +15,6 @@ struct intel_crtc; struct intel_crtc_state; struct intel_dp; struct intel_encoder; -struct intel_crtc; bool intel_vrr_is_capable(struct drm_connector *connector); void intel_vrr_check_modeset(struct intel_atomic_state *state); -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/msm/dp: delete unnecessary debugfs error handling
Quoting Abhinav Kumar (2021-03-05 11:17:19) > Currently the error checking logic in the dp_debug module could > pass zero to PTR_ERR and it causes the below kbot warnings: > > drivers/gpu/drm/msm/dp/dp_debug.c:378 dp_debug_init() > warn: passing zero to 'PTR_ERR' > drivers/gpu/drm/msm/dp/dp_debug.c:387 dp_debug_init() > warn: passing zero to 'PTR_ERR' > drivers/gpu/drm/msm/dp/dp_debug.c:396 dp_debug_init() > warn: passing zero to 'PTR_ERR' > drivers/gpu/drm/msm/dp/dp_debug.c:405 dp_debug_init() > warn: passing zero to 'PTR_ERR' > > Debugfs functions are not supposed to be checked in the normal > case so delete this code. Also it silences the above Smatch > warnings that we're checking for NULL when these functions only > return error pointers. > > Reported-by: kernel test robot > Reported-by: Dan Carpenter > Signed-off-by: Abhinav Kumar > --- Reviewed-by: Stephen Boyd Nice cleanup! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/msm/dp: Fix incorrect NULL check kbot warnings in DP driver
Quoting Abhinav Kumar (2021-03-05 11:17:18) > Fix an incorrect NULL check reported by kbot in the MSM DP driver > > smatch warnings: > drivers/gpu/drm/msm/dp/dp_hpd.c:37 dp_hpd_connect() > error: we previously assumed 'hpd_priv->dp_cb' could be null > (see line 37) > > Reported-by: kernel test robot > Reported-by: Dan Carpenter > Signed-off-by: Abhinav Kumar > --- Reviewed-by: Stephen Boyd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/msm/dp: Fix indentation kbot warnings in DP driver
Quoting Abhinav Kumar (2021-03-05 11:17:17) > Fix a couple of indentation warnings reported by > kbot across MSM DP driver: > > New smatch warnings: > drivers/gpu/drm/msm/dp/dp_debug.c:229 dp_test_data_show() > warn: inconsistent indenting > > drivers/gpu/drm/msm/dp/dp_power.c:203 dp_power_clk_enable() > warn: inconsistent indenting > > Reported-by: kernel test robot > Reported-by: Dan Carpenter > Signed-off-by: Abhinav Kumar > --- Reviewed-by: Stephen Boyd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 05/11] drm/bridge: ti-sn65dsi86: Wrap panel with panel-bridge
Hi Doug, On Wed, Mar 24, 2021 at 03:44:39PM -0700, Doug Anderson wrote: > On Sun, Mar 21, 2021 at 8:02 PM Laurent Pinchart wrote: > > > > To simplify interfacing with the panel, wrap it in a panel-bridge and > > let the DRM bridge helpers handle chaining of operations. > > > > This also prepares for support of DRM_BRIDGE_ATTACH_NO_CONNECTOR, which > > requires all components in the display pipeline to be represented by > > bridges. > > > > Signed-off-by: Laurent Pinchart > > --- > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 30 +++ > > 1 file changed, 21 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > index 1d1be791d5ba..c21a7f7d452b 100644 > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > @@ -124,6 +124,7 @@ > > * @edid: Detected EDID of eDP panel. > > * @refclk: Our reference clock. > > * @panel:Our panel. > > + * @next_bridge: The next bridge. > > To make it easier for folks who don't work with DRM all day, could you > somehow clarify which direction "next" is talking about. AKA the next > "outward" (towards the sink) or the next "inward" (toward the source)? Sure, I'll expand the comment. > > * @enable_gpio: The GPIO we toggle to enable the bridge. > > * @supplies: Data for bulk enabling/disabling our regulators. > > * @dp_lanes: Count of dp_lanes we're using. > > @@ -152,6 +153,7 @@ struct ti_sn_bridge { > > struct mipi_dsi_device *dsi; > > struct clk *refclk; > > struct drm_panel*panel; > > + struct drm_bridge *next_bridge; > > There's no reason to store the "panel" pointer anymore, right? It can > just be a local variable in probe? Good point, I'll fix that. > > @@ -850,8 +856,6 @@ static void ti_sn_bridge_pre_enable(struct drm_bridge > > *bridge) > > */ > > regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE, > >HPD_DISABLE); > > - > > - drm_panel_prepare(pdata->panel); > > Ugh, I guess conflicts with my EDID patch [1] which assumes that this > function will directly turn the panel on. I'll see if I can find some > solution... > > [1] > https://lore.kernel.org/r/20210304155144.3.I60a7fb23ce4589006bc95c64ab8d15c74b876e68@changeid/ Would using the drm_bridge_connector help ? It's a helper that creates a connector based on a chain of bridges. It implements the .get_modes() connector operation (see drm_bridge_connector_get_modes()), based on the .get_edid() operation provided by the bridges. As it has a full view of the chain, it could enable bridges prior to reading the EDID, and then power them off, including the panel-bridge. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 03/11] drm/bridge: ti-sn65dsi86: Unregister AUX adapter in remove()
Hi Doug, On Thu, Mar 25, 2021 at 05:43:22PM -0700, Doug Anderson wrote: > On Tue, Mar 23, 2021 at 4:03 PM Laurent Pinchart wrote: > > On Tue, Mar 23, 2021 at 03:55:05PM -0700, Doug Anderson wrote: > > > On Tue, Mar 23, 2021 at 2:42 PM Laurent Pinchart wrote: > > > > On Tue, Mar 23, 2021 at 02:08:42PM -0700, Doug Anderson wrote: > > > > > On Sun, Mar 21, 2021 at 8:02 PM Laurent Pinchart wrote: > > > > > > > > > > > > The AUX adapter registered in probe() need to be unregistered in > > > > > > remove(). Do so. > > > > > > > > > > > > Fixes: b814ec6d4535 ("drm/bridge: ti-sn65dsi86: Implement AUX > > > > > > channel") > > > > > > Signed-off-by: Laurent Pinchart > > > > > > > > > > > > --- > > > > > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +++ > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > > > > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > > > > > index da78a12e58b5..c45420a50e73 100644 > > > > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > > > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > > > > > @@ -1307,6 +1307,9 @@ static int ti_sn_bridge_remove(struct > > > > > > i2c_client *client) > > > > > > return -EINVAL; > > > > > > > > > > > > kfree(pdata->edid); > > > > > > + > > > > > > + drm_dp_aux_unregister(>aux); > > > > > > + > > > > > > ti_sn_debugfs_remove(pdata); > > > > > > > > > > > > of_node_put(pdata->host_node); > > > > > > > > > > Good catch. One question, though. I know DRM sometimes has different > > > > > conventions than the rest of the kernel, but I always look for the > > > > > "remove" to be backwards of probe. That means that your code (and > > > > > probably most of the remove function) should come _after_ the > > > > > drm_bridge_remove(), right? ...since drm_bridge_add() was the last > > > > > thing in probe then drm_bridge_remove() should be the first thing in > > > > > remove? > > > > > > > > I agree in theory, yes. However, in practice, if you remove a bridge > > > > that is currently in use, all hell will break lose. And if the bridge > > > > isn't being used, it makes no difference. Still, it's worth changing the > > > > order of operations to move drm_bridge_remove() first, as it won't hurt > > > > in any case and is logically better. It's not an issue introduced by > > > > this series though, so how how about it on top, or in parallel ? > > > > > > Sure, it can be a separate patch. I'd kinda prefer it be a patch > > > _before_ ${SUBJECT} patch, though. Specifically it's harder for me to > > > reason about whether your new function call is in the right place and > > > won't cause any problems with the order being all jumbled. If we fix > > > the order first then it's easy to reason about your patch. > > > > > > > You can > > > > even submit a patch if you want :-) > > > > > > Happy to post it up if it won't cause more confusion w/ you posting > > > your next version and trying to figure out what to base it on (since > > > it will definitely conflict with your series). > > > > I'll need quite a bit of time before v2, as I'd like to test it, and > > that requires finishing support for the DSI bridge and the display > > controller :-) Please feel free to post a patch if you have time, I > > think it could get merged in drm-misc quite quickly. > > I haven't forgotten about this and I've got it written, but I'm trying > to put it together with the work I'm doing to fix EDID reading and > that's still going to take me a while longer. I'm out tomorrow but > _hoping_ that I'll be able to at least get a new patch series (at > least RFC quality) next week... No worries at all, it will take a few weeks at least before I get the display controller and DSI working on my board, so you're not blocking me :-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: adv7511: fix support for large EDIDs
Hi Hans, Thank you for the patch. On Wed, Mar 24, 2021 at 09:53:32AM +0100, Hans Verkuil wrote: > While testing support for large (> 256 bytes) EDIDs on the Renesas > Koelsch board I noticed that the adv7511 bridge driver only read the > first two blocks. > > The media V4L2 version for the adv7511 (drivers/media/i2c/adv7511-v4l2.c) > handled this correctly. > > Besides a simple bug when setting the segment register (it was set to the > block number instead of block / 2), the logic of the code was also weird. > In particular reading the DDC_STATUS is odd: this is unrelated to EDID > reading. Bits 3:0 of DDC_STATUS report the DDC controller state, which can be used to wait until the DDC controller is idle (it reports, among other possible states, if an EDID read is in progress). Other options are possible of course, including waiting for ADV7511_INT0_EDID_READY as done in adv7511_wait_for_edid(), but I wonder if the !irq case in adv7511_wait_for_edid() wouldn't be better of busy-looping on the DDC status instead of running the interrupt handler manually. That's unrelated to this patch though. > The reworked code just waits for any EDID segment reads to finish (this > does nothing if the a segment is already read), checks if the desired > segment matches the read segment, and if not, then it requests the new > segment and waits again for the EDID segment to be read. > > Finally it checks if the currently buffered EDID segment contains the > desired EDID block, and if not it will update the EDID buffer from > the adv7511. > > Tested with my Koelsch board and with EDIDs of 1, 2, 3 and 4 blocks. > > Signed-off-by: Hans Verkuil > --- > Testing on the Renesas board also requires these two adv7604 patches > if you want to test with an HDMI cable between the HDMI input and output: > > https://patchwork.linuxtv.org/project/linux-media/patch/00882808-472a-d429-c565-a701da579...@xs4all.nl/ > https://patchwork.linuxtv.org/project/linux-media/patch/c7093e76-ffb4-b19c-f576-b264f935a...@xs4all.nl/ > --- > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index 76555ae64e9c..9e8db1c60167 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -328,6 +328,7 @@ static void adv7511_set_link_config(struct adv7511 > *adv7511, > static void __adv7511_power_on(struct adv7511 *adv7511) > { > adv7511->current_edid_segment = -1; > + adv7511->edid_read = false; > > regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > ADV7511_POWER_POWER_DOWN, 0); > @@ -529,29 +530,35 @@ static int adv7511_get_edid_block(void *data, u8 *buf, > unsigned int block, > struct adv7511 *adv7511 = data; > struct i2c_msg xfer[2]; > uint8_t offset; > + unsigned int cur_segment; > unsigned int i; > int ret; > > if (len > 128) > return -EINVAL; > > - if (adv7511->current_edid_segment != block / 2) { > - unsigned int status; > + /* wait for any EDID segment reads to finish */ > + adv7511_wait_for_edid(adv7511, 200); Why do we need to wait for the EDID read to complete here ? Does the ADV7511 initiate an EDID read by itself that we need to wait for it to complete ? > > - ret = regmap_read(adv7511->regmap, ADV7511_REG_DDC_STATUS, > - ); > + ret = regmap_read(adv7511->regmap, ADV7511_REG_EDID_SEGMENT, > _segment); > + if (ret < 0) > + return ret; Do we need to read this from the device, can't we instead use current_edid_segment ? > + > + /* > + * If the current read segment does not match what we need, then > + * write the new segment and wait for it to be read. > + */ > + if (cur_segment != block / 2) { > + adv7511->edid_read = false; > + cur_segment = block / 2; > + regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT, > + cur_segment); > + ret = adv7511_wait_for_edid(adv7511, 200); > if (ret < 0) > return ret; > + } > > - if (status != 2) { > - adv7511->edid_read = false; > - regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT, > - block); > - ret = adv7511_wait_for_edid(adv7511, 200); > - if (ret < 0) > - return ret; > - } > - > + if (adv7511->current_edid_segment != cur_segment) { Do you need to test this condition separately from the previous one ? > /* Break this apart, hopefully more I2C controllers will >* support 64 byte transfers than 256 byte transfers >*/ > @@ -579,7 +586,7 @@ static int adv7511_get_edid_block(void *data, u8 *buf, > unsigned int block, >
[pull] drm/msm: drm-msm-fixes-2021-02-25 for v5.12-rc5
Hi Dave & Daniel, (resending one more time without git tag copy/paste fail) A few fixes for v5.12 The following changes since commit 182b4a2d251305201b6f9cae29067f7112f05835: drm/msm/dp: Add a missing semi-colon (2021-02-07 09:57:04 -0800) are available in the Git repository at: https://gitlab.freedesktop.org/drm/msm.git drm-msm-fixes-2021-02-25 for you to fetch changes up to 627dc55c273dab308303a5217bd3e767d7083ddb: drm/msm/disp/dpu1: icc path needs to be set before dpu runtime resume (2021-03-22 18:52:34 -0700) Dmitry Baryshkov (4): drm/msm/dsi: fix check-before-set in the 7nm dsi_pll code drm/msm/dsi_pll_7nm: Solve TODO for multiplier frac_bits assignment drm/msm/dsi_pll_7nm: Fix variable usage for pll_lockdet_rate drm/msm: fix shutdown hook in case GPU components failed to bind Douglas Anderson (1): drm/msm: Fix speed-bin support not to access outside valid memory Fabio Estevam (1): drm/msm: Fix suspend/resume on i.MX5 Jonathan Marek (1): drm/msm: fix a6xx_gmu_clear_oob Jordan Crouse (1): drm/msm: a6xx: Make sure the SQE microcode is safe Kalyan Thota (1): drm/msm/disp/dpu1: icc path needs to be set before dpu runtime resume Konrad Dybcio (1): drm/msm/adreno: a5xx_power: Don't apply A540 lm_setup to other GPUs Rob Clark (1): drm/msm: Ratelimit invalid-fence message Stephen Boyd (2): drm/msm/kms: Use nested locking for crtc lock instead of custom classes drm/msm/dp: Restore aux retry tuning logic drivers/gpu/drm/msm/adreno/a5xx_power.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 108 -- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 12 ++-- drivers/gpu/drm/msm/dp/dp_aux.c | 7 ++ drivers/gpu/drm/msm/dsi/pll/dsi_pll.c | 2 +- drivers/gpu/drm/msm/dsi/pll/dsi_pll.h | 6 +- drivers/gpu/drm/msm/dsi/pll/dsi_pll_7nm.c | 11 +-- drivers/gpu/drm/msm/msm_atomic.c | 7 +- drivers/gpu/drm/msm/msm_drv.c | 12 drivers/gpu/drm/msm/msm_fence.c | 2 +- drivers/gpu/drm/msm/msm_kms.h | 8 +-- 12 files changed, 119 insertions(+), 60 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 03/11] drm/bridge: ti-sn65dsi86: Unregister AUX adapter in remove()
Hi, On Tue, Mar 23, 2021 at 4:03 PM Laurent Pinchart wrote: > > Hi Doug, > > On Tue, Mar 23, 2021 at 03:55:05PM -0700, Doug Anderson wrote: > > On Tue, Mar 23, 2021 at 2:42 PM Laurent Pinchart wrote: > > > On Tue, Mar 23, 2021 at 02:08:42PM -0700, Doug Anderson wrote: > > > > On Sun, Mar 21, 2021 at 8:02 PM Laurent Pinchart wrote: > > > > > > > > > > The AUX adapter registered in probe() need to be unregistered in > > > > > remove(). Do so. > > > > > > > > > > Fixes: b814ec6d4535 ("drm/bridge: ti-sn65dsi86: Implement AUX > > > > > channel") > > > > > Signed-off-by: Laurent Pinchart > > > > > > > > > > --- > > > > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > > > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > > > > index da78a12e58b5..c45420a50e73 100644 > > > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > > > > @@ -1307,6 +1307,9 @@ static int ti_sn_bridge_remove(struct > > > > > i2c_client *client) > > > > > return -EINVAL; > > > > > > > > > > kfree(pdata->edid); > > > > > + > > > > > + drm_dp_aux_unregister(>aux); > > > > > + > > > > > ti_sn_debugfs_remove(pdata); > > > > > > > > > > of_node_put(pdata->host_node); > > > > > > > > Good catch. One question, though. I know DRM sometimes has different > > > > conventions than the rest of the kernel, but I always look for the > > > > "remove" to be backwards of probe. That means that your code (and > > > > probably most of the remove function) should come _after_ the > > > > drm_bridge_remove(), right? ...since drm_bridge_add() was the last > > > > thing in probe then drm_bridge_remove() should be the first thing in > > > > remove? > > > > > > I agree in theory, yes. However, in practice, if you remove a bridge > > > that is currently in use, all hell will break lose. And if the bridge > > > isn't being used, it makes no difference. Still, it's worth changing the > > > order of operations to move drm_bridge_remove() first, as it won't hurt > > > in any case and is logically better. It's not an issue introduced by > > > this series though, so how how about it on top, or in parallel ? > > > > Sure, it can be a separate patch. I'd kinda prefer it be a patch > > _before_ ${SUBJECT} patch, though. Specifically it's harder for me to > > reason about whether your new function call is in the right place and > > won't cause any problems with the order being all jumbled. If we fix > > the order first then it's easy to reason about your patch. > > > > > You can > > > even submit a patch if you want :-) > > > > Happy to post it up if it won't cause more confusion w/ you posting > > your next version and trying to figure out what to base it on (since > > it will definitely conflict with your series). > > I'll need quite a bit of time before v2, as I'd like to test it, and > that requires finishing support for the DSI bridge and the display > controller :-) Please feel free to post a patch if you have time, I > think it could get merged in drm-misc quite quickly. I haven't forgotten about this and I've got it written, but I'm trying to put it together with the work I'm doing to fix EDID reading and that's still going to take me a while longer. I'm out tomorrow but _hoping_ that I'll be able to at least get a new patch series (at least RFC quality) next week... -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/7] drm/i915: Allow configuring default request expiry via modparam
On Wed, Mar 24, 2021 at 12:13:35PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Module parameter is added (request_timeout_ms) to allow configuring the > default request/fence expiry. > > Default value is inherited from CONFIG_DRM_I915_REQUEST_TIMEOUT. > > Signed-off-by: Tvrtko Ursulin > Cc: Daniel Vetter > Acked-by: Daniel Vetter Entire series merged, thanks for the patches. -Daniel > --- > drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 +++-- > drivers/gpu/drm/i915/i915_params.c | 5 + > drivers/gpu/drm/i915/i915_params.h | 1 + > 3 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c > b/drivers/gpu/drm/i915/gem/i915_gem_context.c > index 33ff1a6a7724..0e8f0476e01f 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > @@ -845,11 +845,12 @@ static void __set_default_fence_expiry(struct > i915_gem_context *ctx) > struct drm_i915_private *i915 = ctx->i915; > int ret; > > - if (!IS_ACTIVE(CONFIG_DRM_I915_REQUEST_TIMEOUT)) > + if (!IS_ACTIVE(CONFIG_DRM_I915_REQUEST_TIMEOUT) || > + !i915->params.request_timeout_ms) > return; > > /* Default expiry for user fences. */ > - ret = __set_watchdog(ctx, CONFIG_DRM_I915_REQUEST_TIMEOUT * 1000); > + ret = __set_watchdog(ctx, i915->params.request_timeout_ms * 1000); > if (ret) > drm_notice(>drm, > "Failed to configure default fence expiry! (%d)", > diff --git a/drivers/gpu/drm/i915/i915_params.c > b/drivers/gpu/drm/i915/i915_params.c > index 6939634e56ed..0320878d96b0 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -197,6 +197,11 @@ i915_param_named_unsafe(fake_lmem_start, ulong, 0400, > "Fake LMEM start offset (default: 0)"); > #endif > > +#if CONFIG_DRM_I915_REQUEST_TIMEOUT > +i915_param_named_unsafe(request_timeout_ms, uint, 0600, > + "Default request/fence/batch buffer expiration > timeout."); > +#endif > + > static __always_inline void _print_param(struct drm_printer *p, >const char *name, >const char *type, > diff --git a/drivers/gpu/drm/i915/i915_params.h > b/drivers/gpu/drm/i915/i915_params.h > index 48f47e44e848..34ebb0662547 100644 > --- a/drivers/gpu/drm/i915/i915_params.h > +++ b/drivers/gpu/drm/i915/i915_params.h > @@ -72,6 +72,7 @@ struct drm_printer; > param(int, enable_dpcd_backlight, -1, 0600) \ > param(char *, force_probe, CONFIG_DRM_I915_FORCE_PROBE, 0400) \ > param(unsigned long, fake_lmem_start, 0, 0400) \ > + param(unsigned int, request_timeout_ms, > CONFIG_DRM_I915_REQUEST_TIMEOUT, 0600) \ > /* leave bools at the end to not create holes */ \ > param(bool, enable_hangcheck, true, 0600) \ > param(bool, load_detect_test, false, 0600) \ > -- > 2.27.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom
On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > The sc7180-trogdor-pompom board might be attached to any number of a > pile of eDP panels. At the moment I'm told that the list might include: > - KD KD116N21-30NV-A010 > - KD KD116N09-30NH-A016 > - Starry 2081116HHD028001-51D > - Sharp LQ116M1JW10 > > It should be noted that while the EDID programmed in the first 3 > panels indicates that they should run with exactly the same timing (to > keep things simple), the 4th panel not only needs different timing but > has a different resolution. > > As is true in general with eDP panels, we can figure out which panel > we have and all the info needed to drive its pixel clock by reading > the EDID. However, we can do this only after we've powered the panel > on. Powering on the panels requires following the timing diagram in > each panel's datasheet which specifies delays between certain > actions. This means that, while we can be quite dynamic about handling > things we can't just totally skip out on describing the panel like we > could do if it was connected to an external-facing DP port. Is this a 'standard' eDP connector? AFAICT, there does seem to be such a thing. I've said in the past I'd be okay with a edp-connector node. If that needs just the "HPD absent delay" property, I think that would be okay. It's just a never ending stream of new properties with each new panel that I don't want to see. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 8/8] nouveau/svm: Implement atomic SVM access
Some NVIDIA GPUs do not support direct atomic access to system memory via PCIe. Instead this must be emulated by granting the GPU exclusive access to the memory. This is achieved by replacing CPU page table entries with special swap entries that fault on userspace access. The driver then grants the GPU permission to update the page undergoing atomic access via the GPU page tables. When CPU access to the page is required a CPU fault is raised which calls into the device driver via MMU notifiers to revoke the atomic access. The original page table entries are then restored allowing CPU access to proceed. Signed-off-by: Alistair Popple --- v7: * Removed magic values for fault access levels * Improved readability of fault comparison code v4: * Check that page table entries haven't changed before mapping on the device --- drivers/gpu/drm/nouveau/include/nvif/if000c.h | 1 + drivers/gpu/drm/nouveau/nouveau_svm.c | 126 -- drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h | 1 + .../drm/nouveau/nvkm/subdev/mmu/vmmgp100.c| 6 + 4 files changed, 123 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/nouveau/include/nvif/if000c.h b/drivers/gpu/drm/nouveau/include/nvif/if000c.h index d6dd40f21eed..9c7ff56831c5 100644 --- a/drivers/gpu/drm/nouveau/include/nvif/if000c.h +++ b/drivers/gpu/drm/nouveau/include/nvif/if000c.h @@ -77,6 +77,7 @@ struct nvif_vmm_pfnmap_v0 { #define NVIF_VMM_PFNMAP_V0_APER 0x00f0ULL #define NVIF_VMM_PFNMAP_V0_HOST 0xULL #define NVIF_VMM_PFNMAP_V0_VRAM 0x0010ULL +#define NVIF_VMM_PFNMAP_V0_A 0x0004ULL #define NVIF_VMM_PFNMAP_V0_W 0x0002ULL #define NVIF_VMM_PFNMAP_V0_V 0x0001ULL #define NVIF_VMM_PFNMAP_V0_NONE 0xULL diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index a195e48c9aee..81526d65b4e2 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -35,6 +35,7 @@ #include #include #include +#include struct nouveau_svm { struct nouveau_drm *drm; @@ -67,6 +68,11 @@ struct nouveau_svm { } buffer[1]; }; +#define FAULT_ACCESS_READ 0 +#define FAULT_ACCESS_WRITE 1 +#define FAULT_ACCESS_ATOMIC 2 +#define FAULT_ACCESS_PREFETCH 3 + #define SVM_DBG(s,f,a...) NV_DEBUG((s)->drm, "svm: "f"\n", ##a) #define SVM_ERR(s,f,a...) NV_WARN((s)->drm, "svm: "f"\n", ##a) @@ -411,6 +417,24 @@ nouveau_svm_fault_cancel_fault(struct nouveau_svm *svm, fault->client); } +static int +nouveau_svm_fault_priority(u8 fault) +{ + switch (fault) { + case FAULT_ACCESS_PREFETCH: + return 0; + case FAULT_ACCESS_READ: + return 1; + case FAULT_ACCESS_WRITE: + return 2; + case FAULT_ACCESS_ATOMIC: + return 3; + default: + WARN_ON_ONCE(1); + return -1; + } +} + static int nouveau_svm_fault_cmp(const void *a, const void *b) { @@ -421,9 +445,8 @@ nouveau_svm_fault_cmp(const void *a, const void *b) return ret; if ((ret = (s64)fa->addr - fb->addr)) return ret; - /*XXX: atomic? */ - return (fa->access == 0 || fa->access == 3) - - (fb->access == 0 || fb->access == 3); + return nouveau_svm_fault_priority(fa->access) - + nouveau_svm_fault_priority(fb->access); } static void @@ -487,6 +510,10 @@ static bool nouveau_svm_range_invalidate(struct mmu_interval_notifier *mni, struct svm_notifier *sn = container_of(mni, struct svm_notifier, notifier); + if (range->event == MMU_NOTIFY_EXCLUSIVE && + range->owner == sn->svmm->vmm->cli->drm->dev) + return true; + /* * serializes the update to mni->invalidate_seq done by caller and * prevents invalidation of the PTE from progressing while HW is being @@ -555,6 +582,71 @@ static void nouveau_hmm_convert_pfn(struct nouveau_drm *drm, args->p.phys[0] |= NVIF_VMM_PFNMAP_V0_W; } +static int nouveau_atomic_range_fault(struct nouveau_svmm *svmm, + struct nouveau_drm *drm, + struct nouveau_pfnmap_args *args, u32 size, + struct svm_notifier *notifier) +{ + unsigned long timeout = + jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT); + struct mm_struct *mm = svmm->notifier.mm; + struct page *page; + unsigned long start = args->p.addr; + unsigned long notifier_seq; + int ret = 0; + + ret = mmu_interval_notifier_insert(>notifier, mm, +
[PATCH v7 7/8] nouveau/svm: Refactor nouveau_range_fault
Call mmu_interval_notifier_insert() as part of nouveau_range_fault(). This doesn't introduce any functional change but makes it easier for a subsequent patch to alter the behaviour of nouveau_range_fault() to support GPU atomic operations. Signed-off-by: Alistair Popple --- drivers/gpu/drm/nouveau/nouveau_svm.c | 34 --- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index 94f841026c3b..a195e48c9aee 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -567,18 +567,27 @@ static int nouveau_range_fault(struct nouveau_svmm *svmm, unsigned long hmm_pfns[1]; struct hmm_range range = { .notifier = >notifier, - .start = notifier->notifier.interval_tree.start, - .end = notifier->notifier.interval_tree.last + 1, .default_flags = hmm_flags, .hmm_pfns = hmm_pfns, .dev_private_owner = drm->dev, }; - struct mm_struct *mm = notifier->notifier.mm; + struct mm_struct *mm = svmm->notifier.mm; int ret; + ret = mmu_interval_notifier_insert(>notifier, mm, + args->p.addr, args->p.size, + _svm_mni_ops); + if (ret) + return ret; + + range.start = notifier->notifier.interval_tree.start; + range.end = notifier->notifier.interval_tree.last + 1; + while (true) { - if (time_after(jiffies, timeout)) - return -EBUSY; + if (time_after(jiffies, timeout)) { + ret = -EBUSY; + goto out; + } range.notifier_seq = mmu_interval_read_begin(range.notifier); mmap_read_lock(mm); @@ -587,7 +596,7 @@ static int nouveau_range_fault(struct nouveau_svmm *svmm, if (ret) { if (ret == -EBUSY) continue; - return ret; + goto out; } mutex_lock(>mutex); @@ -606,6 +615,9 @@ static int nouveau_range_fault(struct nouveau_svmm *svmm, svmm->vmm->vmm.object.client->super = false; mutex_unlock(>mutex); +out: + mmu_interval_notifier_remove(>notifier); + return ret; } @@ -727,14 +739,8 @@ nouveau_svm_fault(struct nvif_notify *notify) } notifier.svmm = svmm; - ret = mmu_interval_notifier_insert(, mm, - args.i.p.addr, args.i.p.size, - _svm_mni_ops); - if (!ret) { - ret = nouveau_range_fault(svmm, svm->drm, , - sizeof(args), hmm_flags, ); - mmu_interval_notifier_remove(); - } + ret = nouveau_range_fault(svmm, svm->drm, , + sizeof(args), hmm_flags, ); mmput(mm); limit = args.i.p.addr + args.i.p.size; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 5/8] mm: Device exclusive memory access
Some devices require exclusive write access to shared virtual memory (SVM) ranges to perform atomic operations on that memory. This requires CPU page tables to be updated to deny access whilst atomic operations are occurring. In order to do this introduce a new swap entry type (SWP_DEVICE_EXCLUSIVE). When a SVM range needs to be marked for exclusive access by a device all page table mappings for the particular range are replaced with device exclusive swap entries. This causes any CPU access to the page to result in a fault. Faults are resovled by replacing the faulting entry with the original mapping. This results in MMU notifiers being called which a driver uses to update access permissions such as revoking atomic access. After notifiers have been called the device will no longer have exclusive access to the region. Signed-off-by: Alistair Popple Reviewed-by: Christoph Hellwig --- v7: * Added Christoph's Reviewed-by. * Minor cosmetic cleanups suggested by Christoph. * Replace mmu_notifier_range_init_migrate/exclusive with mmu_notifier_range_init_owner as suggested by Christoph. * Replaced lock_page() with lock_page_retry() when handling faults. * Restrict to anonymous pages for now. v6: * Fixed a bisectablity issue due to incorrectly applying the rename of migrate_pgmap_owner to the wrong patches for Nouveau and hmm_test. v5: * Renamed range->migrate_pgmap_owner to range->owner. * Added MMU_NOTIFY_EXCLUSIVE to allow passing of a driver cookie which allows notifiers called as a result of make_device_exclusive_range() to be ignored. * Added a check to try_to_protect_one() to detect if the pages originally returned from get_user_pages() have been unmapped or not. * Removed check_device_exclusive_range() as it is no longer required with the other changes. * Documentation update. v4: * Add function to check that mappings are still valid and exclusive. * s/long/unsigned long/ in make_device_exclusive_entry(). --- Documentation/vm/hmm.rst | 19 ++- drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- include/linux/mmu_notifier.h | 26 ++-- include/linux/rmap.h | 4 + include/linux/swap.h | 4 +- include/linux/swapops.h | 44 +- lib/test_hmm.c| 2 +- mm/hmm.c | 5 + mm/memory.c | 108 - mm/migrate.c | 10 +- mm/mprotect.c | 8 + mm/page_vma_mapped.c | 9 +- mm/rmap.c | 210 ++ 13 files changed, 426 insertions(+), 25 deletions(-) diff --git a/Documentation/vm/hmm.rst b/Documentation/vm/hmm.rst index 09e28507f5b2..a14c2938e7af 100644 --- a/Documentation/vm/hmm.rst +++ b/Documentation/vm/hmm.rst @@ -332,7 +332,7 @@ between device driver specific code and shared common code: walks to fill in the ``args->src`` array with PFNs to be migrated. The ``invalidate_range_start()`` callback is passed a ``struct mmu_notifier_range`` with the ``event`` field set to - ``MMU_NOTIFY_MIGRATE`` and the ``migrate_pgmap_owner`` field set to + ``MMU_NOTIFY_MIGRATE`` and the ``owner`` field set to the ``args->pgmap_owner`` field passed to migrate_vma_setup(). This is allows the device driver to skip the invalidation callback and only invalidate device private MMU mappings that are actually migrating. @@ -405,6 +405,23 @@ between device driver specific code and shared common code: The lock can now be released. +Exclusive access memory +=== + +Some devices have features such as atomic PTE bits that can be used to implement +atomic access to system memory. To support atomic operations to a shared virtual +memory page such a device needs access to that page which is exclusive of any +userspace access from the CPU. The ``make_device_exclusive_range()`` function +can be used to make a memory range inaccessible from userspace. + +This replaces all mappings for pages in the given range with special swap +entries. Any attempt to access the swap entry results in a fault which is +resovled by replacing the entry with the original mapping. A driver gets +notified that the mapping has been changed by MMU notifiers, after which point +it will no longer have exclusive access to the page. Exclusive access is +guranteed to last until the driver drops the page lock and page reference, at +which point any CPU faults on the page may proceed as described. + Memory cgroup (memcg) and rss accounting diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index f18bd53da052..94f841026c3b 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -265,7 +265,7 @@ nouveau_svmm_invalidate_range_start(struct mmu_notifier *mn, * the invalidation is handled as part of
[PATCH v7 6/8] mm: Selftests for exclusive device memory
Adds some selftests for exclusive device memory. Signed-off-by: Alistair Popple Acked-by: Jason Gunthorpe Tested-by: Ralph Campbell Reviewed-by: Ralph Campbell --- lib/test_hmm.c | 124 +++ lib/test_hmm_uapi.h| 2 + tools/testing/selftests/vm/hmm-tests.c | 158 + 3 files changed, 284 insertions(+) diff --git a/lib/test_hmm.c b/lib/test_hmm.c index 5c9f5a020c1d..305a9d9e2b4c 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "test_hmm_uapi.h" @@ -46,6 +47,7 @@ struct dmirror_bounce { unsigned long cpages; }; +#define DPT_XA_TAG_ATOMIC 1UL #define DPT_XA_TAG_WRITE 3UL /* @@ -619,6 +621,54 @@ static void dmirror_migrate_alloc_and_copy(struct migrate_vma *args, } } +static int dmirror_check_atomic(struct dmirror *dmirror, unsigned long start, +unsigned long end) +{ + unsigned long pfn; + + for (pfn = start >> PAGE_SHIFT; pfn < (end >> PAGE_SHIFT); pfn++) { + void *entry; + struct page *page; + + entry = xa_load(>pt, pfn); + page = xa_untag_pointer(entry); + if (xa_pointer_tag(entry) == DPT_XA_TAG_ATOMIC) + return -EPERM; + } + + return 0; +} + +static int dmirror_atomic_map(unsigned long start, unsigned long end, + struct page **pages, struct dmirror *dmirror) +{ + unsigned long pfn, mapped = 0; + int i; + + /* Map the migrated pages into the device's page tables. */ + mutex_lock(>mutex); + + for (i = 0, pfn = start >> PAGE_SHIFT; pfn < (end >> PAGE_SHIFT); pfn++, i++) { + void *entry; + + if (!pages[i]) + continue; + + entry = pages[i]; + entry = xa_tag_pointer(entry, DPT_XA_TAG_ATOMIC); + entry = xa_store(>pt, pfn, entry, GFP_ATOMIC); + if (xa_is_err(entry)) { + mutex_unlock(>mutex); + return xa_err(entry); + } + + mapped++; + } + + mutex_unlock(>mutex); + return mapped; +} + static int dmirror_migrate_finalize_and_map(struct migrate_vma *args, struct dmirror *dmirror) { @@ -661,6 +711,71 @@ static int dmirror_migrate_finalize_and_map(struct migrate_vma *args, return 0; } +static int dmirror_exclusive(struct dmirror *dmirror, +struct hmm_dmirror_cmd *cmd) +{ + unsigned long start, end, addr; + unsigned long size = cmd->npages << PAGE_SHIFT; + struct mm_struct *mm = dmirror->notifier.mm; + struct page *pages[64]; + struct dmirror_bounce bounce; + unsigned long next; + int ret; + + start = cmd->addr; + end = start + size; + if (end < start) + return -EINVAL; + + /* Since the mm is for the mirrored process, get a reference first. */ + if (!mmget_not_zero(mm)) + return -EINVAL; + + mmap_read_lock(mm); + for (addr = start; addr < end; addr = next) { + int i, mapped; + + if (end < addr + (ARRAY_SIZE(pages) << PAGE_SHIFT)) + next = end; + else + next = addr + (ARRAY_SIZE(pages) << PAGE_SHIFT); + + ret = make_device_exclusive_range(mm, addr, next, pages, NULL); + mapped = dmirror_atomic_map(addr, next, pages, dmirror); + for (i = 0; i < ret; i++) { + if (pages[i]) { + unlock_page(pages[i]); + put_page(pages[i]); + } + } + + if (addr + (mapped << PAGE_SHIFT) < next) { + mmap_read_unlock(mm); + mmput(mm); + return -EBUSY; + } + } + mmap_read_unlock(mm); + mmput(mm); + + /* Return the migrated data for verification. */ + ret = dmirror_bounce_init(, start, size); + if (ret) + return ret; + mutex_lock(>mutex); + ret = dmirror_do_read(dmirror, start, end, ); + mutex_unlock(>mutex); + if (ret == 0) { + if (copy_to_user(u64_to_user_ptr(cmd->ptr), bounce.ptr, +bounce.size)) + ret = -EFAULT; + } + + cmd->cpages = bounce.cpages; + dmirror_bounce_fini(); + return ret; +} + static int dmirror_migrate(struct dmirror *dmirror, struct hmm_dmirror_cmd *cmd) { @@ -949,6 +1064,15 @@ static long dmirror_fops_unlocked_ioctl(struct file *filp, ret = dmirror_migrate(dmirror, ); break; +
[PATCH v7 4/8] mm/rmap: Split migration into its own function
Migration is currently implemented as a mode of operation for try_to_unmap_one() generally specified by passing the TTU_MIGRATION flag or in the case of splitting a huge anonymous page TTU_SPLIT_FREEZE. However it does not have much in common with the rest of the unmap functionality of try_to_unmap_one() and thus splitting it into a separate function reduces the complexity of try_to_unmap_one() making it more readable. Several simplifications can also be made in try_to_migrate_one() based on the following observations: - All users of TTU_MIGRATION also set TTU_IGNORE_MLOCK. - No users of TTU_MIGRATION ever set TTU_IGNORE_HWPOISON. - No users of TTU_MIGRATION ever set TTU_BATCH_FLUSH. TTU_SPLIT_FREEZE is a special case of migration used when splitting an anonymous page. This is most easily dealt with by calling the correct function from unmap_page() in mm/huge_memory.c - either try_to_migrate() for PageAnon or try_to_unmap(). Signed-off-by: Alistair Popple Reviewed-by: Christoph Hellwig Reviewed-by: Ralph Campbell --- v5: * Added comments about how PMD splitting works for migration vs. unmapping * Tightened up the flag check in try_to_migrate() to be explicit about which TTU_XXX flags are supported. --- include/linux/rmap.h | 4 +- mm/huge_memory.c | 15 +- mm/migrate.c | 9 +- mm/rmap.c| 358 --- 4 files changed, 280 insertions(+), 106 deletions(-) diff --git a/include/linux/rmap.h b/include/linux/rmap.h index e26ac2d71346..6062e0cfca2d 100644 --- a/include/linux/rmap.h +++ b/include/linux/rmap.h @@ -86,8 +86,6 @@ struct anon_vma_chain { }; enum ttu_flags { - TTU_MIGRATION = 0x1, /* migration mode */ - TTU_SPLIT_HUGE_PMD = 0x4, /* split huge PMD if any */ TTU_IGNORE_MLOCK= 0x8, /* ignore mlock */ TTU_IGNORE_HWPOISON = 0x20, /* corrupted page is recoverable */ @@ -96,7 +94,6 @@ enum ttu_flags { * do a final flush if necessary */ TTU_RMAP_LOCKED = 0x80, /* do not grab rmap lock: * caller holds it */ - TTU_SPLIT_FREEZE= 0x100,/* freeze pte under splitting thp */ }; #ifdef CONFIG_MMU @@ -193,6 +190,7 @@ static inline void page_dup_rmap(struct page *page, bool compound) int page_referenced(struct page *, int is_locked, struct mem_cgroup *memcg, unsigned long *vm_flags); +bool try_to_migrate(struct page *page, enum ttu_flags flags); bool try_to_unmap(struct page *, enum ttu_flags flags); /* Avoid racy checks */ diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 89af065cea5b..eab004331b97 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2357,16 +2357,21 @@ void vma_adjust_trans_huge(struct vm_area_struct *vma, static void unmap_page(struct page *page) { - enum ttu_flags ttu_flags = TTU_IGNORE_MLOCK | - TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD; + enum ttu_flags ttu_flags = TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD; bool unmap_success; VM_BUG_ON_PAGE(!PageHead(page), page); if (PageAnon(page)) - ttu_flags |= TTU_SPLIT_FREEZE; - - unmap_success = try_to_unmap(page, ttu_flags); + unmap_success = try_to_migrate(page, ttu_flags); + else + /* +* Don't install migration entries for file backed pages. This +* helps handle cases when i_size is in the middle of the page +* as there is no need to unmap pages beyond i_size manually. +*/ + unmap_success = try_to_unmap(page, ttu_flags | + TTU_IGNORE_MLOCK); VM_BUG_ON_PAGE(!unmap_success, page); } diff --git a/mm/migrate.c b/mm/migrate.c index b752543adb64..cc4612e2a246 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -1130,7 +1130,7 @@ static int __unmap_and_move(struct page *page, struct page *newpage, /* Establish migration ptes */ VM_BUG_ON_PAGE(PageAnon(page) && !PageKsm(page) && !anon_vma, page); - try_to_unmap(page, TTU_MIGRATION|TTU_IGNORE_MLOCK); + try_to_migrate(page, 0); page_was_mapped = 1; } @@ -1332,7 +1332,7 @@ static int unmap_and_move_huge_page(new_page_t get_new_page, if (page_mapped(hpage)) { bool mapping_locked = false; - enum ttu_flags ttu = TTU_MIGRATION|TTU_IGNORE_MLOCK; + enum ttu_flags ttu = 0; if (!PageAnon(hpage)) { /* @@ -1349,7 +1349,7 @@ static int unmap_and_move_huge_page(new_page_t get_new_page, ttu |= TTU_RMAP_LOCKED; } - try_to_unmap(hpage, ttu); + try_to_migrate(hpage, ttu); page_was_mapped
[PATCH v7 3/8] mm/rmap: Split try_to_munlock from try_to_unmap
The behaviour of try_to_unmap_one() is difficult to follow because it performs different operations based on a fairly large set of flags used in different combinations. TTU_MUNLOCK is one such flag. However it is exclusively used by try_to_munlock() which specifies no other flags. Therefore rather than overload try_to_unmap_one() with unrelated behaviour split this out into it's own function and remove the flag. Signed-off-by: Alistair Popple Reviewed-by: Ralph Campbell Reviewed-by: Christoph Hellwig --- v7: * Added Christoph's Reviewed-by v4: * Removed redundant check for VM_LOCKED --- include/linux/rmap.h | 1 - mm/rmap.c| 40 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/include/linux/rmap.h b/include/linux/rmap.h index def5c62c93b3..e26ac2d71346 100644 --- a/include/linux/rmap.h +++ b/include/linux/rmap.h @@ -87,7 +87,6 @@ struct anon_vma_chain { enum ttu_flags { TTU_MIGRATION = 0x1, /* migration mode */ - TTU_MUNLOCK = 0x2, /* munlock mode */ TTU_SPLIT_HUGE_PMD = 0x4, /* split huge PMD if any */ TTU_IGNORE_MLOCK= 0x8, /* ignore mlock */ diff --git a/mm/rmap.c b/mm/rmap.c index 977e70803ed8..d02bade5245b 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -1405,10 +1405,6 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma, struct mmu_notifier_range range; enum ttu_flags flags = (enum ttu_flags)(long)arg; - /* munlock has nothing to gain from examining un-locked vmas */ - if ((flags & TTU_MUNLOCK) && !(vma->vm_flags & VM_LOCKED)) - return true; - if (IS_ENABLED(CONFIG_MIGRATION) && (flags & TTU_MIGRATION) && is_zone_device_page(page) && !is_device_private_page(page)) return true; @@ -1469,8 +1465,6 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma, page_vma_mapped_walk_done(); break; } - if (flags & TTU_MUNLOCK) - continue; } /* Unexpected PMD-mapped THP? */ @@ -1784,6 +1778,37 @@ bool try_to_unmap(struct page *page, enum ttu_flags flags) return !page_mapcount(page) ? true : false; } +static bool try_to_munlock_one(struct page *page, struct vm_area_struct *vma, +unsigned long address, void *arg) +{ + struct page_vma_mapped_walk pvmw = { + .page = page, + .vma = vma, + .address = address, + }; + + /* munlock has nothing to gain from examining un-locked vmas */ + if (!(vma->vm_flags & VM_LOCKED)) + return true; + + while (page_vma_mapped_walk()) { + /* PTE-mapped THP are never mlocked */ + if (!PageTransCompound(page)) { + /* +* Holding pte lock, we do *not* need +* mmap_lock here +*/ + mlock_vma_page(page); + } + page_vma_mapped_walk_done(); + + /* found a mlocked page, no point continuing munlock check */ + return false; + } + + return true; +} + /** * try_to_munlock - try to munlock a page * @page: the page to be munlocked @@ -1796,8 +1821,7 @@ bool try_to_unmap(struct page *page, enum ttu_flags flags) void try_to_munlock(struct page *page) { struct rmap_walk_control rwc = { - .rmap_one = try_to_unmap_one, - .arg = (void *)TTU_MUNLOCK, + .rmap_one = try_to_munlock_one, .done = page_not_mapped, .anon_lock = page_lock_anon_vma_read, -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 2/8] mm/swapops: Rework swap entry manipulation code
Both migration and device private pages use special swap entries that are manipluated by a range of inline functions. The arguments to these are somewhat inconsitent so rework them to remove flag type arguments and to make the arguments similar for both read and write entry creation. Signed-off-by: Alistair Popple Reviewed-by: Christoph Hellwig Reviewed-by: Jason Gunthorpe Reviewed-by: Ralph Campbell --- include/linux/swapops.h | 56 ++--- mm/debug_vm_pgtable.c | 12 - mm/hmm.c| 2 +- mm/huge_memory.c| 26 +-- mm/hugetlb.c| 10 +--- mm/memory.c | 10 +--- mm/migrate.c| 26 ++- mm/mprotect.c | 10 +--- mm/rmap.c | 10 +--- 9 files changed, 100 insertions(+), 62 deletions(-) diff --git a/include/linux/swapops.h b/include/linux/swapops.h index 139be8235ad2..4dfd807ae52a 100644 --- a/include/linux/swapops.h +++ b/include/linux/swapops.h @@ -100,35 +100,35 @@ static inline void *swp_to_radix_entry(swp_entry_t entry) } #if IS_ENABLED(CONFIG_DEVICE_PRIVATE) -static inline swp_entry_t make_device_private_entry(struct page *page, bool write) +static inline swp_entry_t make_readable_device_private_entry(pgoff_t offset) { - return swp_entry(write ? SWP_DEVICE_WRITE : SWP_DEVICE_READ, -page_to_pfn(page)); + return swp_entry(SWP_DEVICE_READ, offset); } -static inline bool is_device_private_entry(swp_entry_t entry) +static inline swp_entry_t make_writable_device_private_entry(pgoff_t offset) { - int type = swp_type(entry); - return type == SWP_DEVICE_READ || type == SWP_DEVICE_WRITE; + return swp_entry(SWP_DEVICE_WRITE, offset); } -static inline void make_device_private_entry_read(swp_entry_t *entry) +static inline bool is_device_private_entry(swp_entry_t entry) { - *entry = swp_entry(SWP_DEVICE_READ, swp_offset(*entry)); + int type = swp_type(entry); + return type == SWP_DEVICE_READ || type == SWP_DEVICE_WRITE; } -static inline bool is_write_device_private_entry(swp_entry_t entry) +static inline bool is_writable_device_private_entry(swp_entry_t entry) { return unlikely(swp_type(entry) == SWP_DEVICE_WRITE); } #else /* CONFIG_DEVICE_PRIVATE */ -static inline swp_entry_t make_device_private_entry(struct page *page, bool write) +static inline swp_entry_t make_readable_device_private_entry(pgoff_t offset) { return swp_entry(0, 0); } -static inline void make_device_private_entry_read(swp_entry_t *entry) +static inline swp_entry_t make_writable_device_private_entry(pgoff_t offset) { + return swp_entry(0, 0); } static inline bool is_device_private_entry(swp_entry_t entry) @@ -136,35 +136,32 @@ static inline bool is_device_private_entry(swp_entry_t entry) return false; } -static inline bool is_write_device_private_entry(swp_entry_t entry) +static inline bool is_writable_device_private_entry(swp_entry_t entry) { return false; } #endif /* CONFIG_DEVICE_PRIVATE */ #ifdef CONFIG_MIGRATION -static inline swp_entry_t make_migration_entry(struct page *page, int write) -{ - BUG_ON(!PageLocked(compound_head(page))); - - return swp_entry(write ? SWP_MIGRATION_WRITE : SWP_MIGRATION_READ, - page_to_pfn(page)); -} - static inline int is_migration_entry(swp_entry_t entry) { return unlikely(swp_type(entry) == SWP_MIGRATION_READ || swp_type(entry) == SWP_MIGRATION_WRITE); } -static inline int is_write_migration_entry(swp_entry_t entry) +static inline int is_writable_migration_entry(swp_entry_t entry) { return unlikely(swp_type(entry) == SWP_MIGRATION_WRITE); } -static inline void make_migration_entry_read(swp_entry_t *entry) +static inline swp_entry_t make_readable_migration_entry(pgoff_t offset) { - *entry = swp_entry(SWP_MIGRATION_READ, swp_offset(*entry)); + return swp_entry(SWP_MIGRATION_READ, offset); +} + +static inline swp_entry_t make_writable_migration_entry(pgoff_t offset) +{ + return swp_entry(SWP_MIGRATION_WRITE, offset); } extern void __migration_entry_wait(struct mm_struct *mm, pte_t *ptep, @@ -174,21 +171,28 @@ extern void migration_entry_wait(struct mm_struct *mm, pmd_t *pmd, extern void migration_entry_wait_huge(struct vm_area_struct *vma, struct mm_struct *mm, pte_t *pte); #else +static inline swp_entry_t make_readable_migration_entry(pgoff_t offset) +{ + return swp_entry(0, 0); +} + +static inline swp_entry_t make_writable_migration_entry(pgoff_t offset) +{ + return swp_entry(0, 0); +} -#define make_migration_entry(page, write) swp_entry(0, 0) static inline int is_migration_entry(swp_entry_t swp) { return 0; } -static inline void make_migration_entry_read(swp_entry_t *entryp) { } static inline void __migration_entry_wait(struct mm_struct *mm,
[PATCH v7 1/8] mm: Remove special swap entry functions
Remove multiple similar inline functions for dealing with different types of special swap entries. Both migration and device private swap entries use the swap offset to store a pfn. Instead of multiple inline functions to obtain a struct page for each swap entry type use a common function pfn_swap_entry_to_page(). Also open-code the various entry_to_pfn() functions as this results is shorter code that is easier to understand. Signed-off-by: Alistair Popple Reviewed-by: Ralph Campbell Reviewed-by: Christoph Hellwig --- v7: * Reworded commit message to include pfn_swap_entry_to_page() * Added Christoph's Reviewed-by v6: * Removed redundant compound_page() call from inside PageLocked() * Fixed a minor build issue for s390 reported by kernel test bot v4: * Added pfn_swap_entry_to_page() * Reinstated check that migration entries point to locked pages * Removed #define swapcache_prepare which isn't needed for CONFIG_SWAP=0 builds --- arch/s390/mm/pgtable.c | 2 +- fs/proc/task_mmu.c | 23 +- include/linux/swap.h| 4 +-- include/linux/swapops.h | 69 ++--- mm/hmm.c| 5 ++- mm/huge_memory.c| 4 +-- mm/memcontrol.c | 2 +- mm/memory.c | 10 +++--- mm/migrate.c| 6 ++-- mm/page_vma_mapped.c| 6 ++-- 10 files changed, 50 insertions(+), 81 deletions(-) diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c index 18205f851c24..eec3a9d7176e 100644 --- a/arch/s390/mm/pgtable.c +++ b/arch/s390/mm/pgtable.c @@ -691,7 +691,7 @@ static void ptep_zap_swap_entry(struct mm_struct *mm, swp_entry_t entry) if (!non_swap_entry(entry)) dec_mm_counter(mm, MM_SWAPENTS); else if (is_migration_entry(entry)) { - struct page *page = migration_entry_to_page(entry); + struct page *page = pfn_swap_entry_to_page(entry); dec_mm_counter(mm, mm_counter(page)); } diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index 3cec6fbef725..08ee59d945c0 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -514,10 +514,8 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr, } else { mss->swap_pss += (u64)PAGE_SIZE << PSS_SHIFT; } - } else if (is_migration_entry(swpent)) - page = migration_entry_to_page(swpent); - else if (is_device_private_entry(swpent)) - page = device_private_entry_to_page(swpent); + } else if (is_pfn_swap_entry(swpent)) + page = pfn_swap_entry_to_page(swpent); } else if (unlikely(IS_ENABLED(CONFIG_SHMEM) && mss->check_shmem_swap && pte_none(*pte))) { page = xa_load(>vm_file->f_mapping->i_pages, @@ -549,7 +547,7 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr, swp_entry_t entry = pmd_to_swp_entry(*pmd); if (is_migration_entry(entry)) - page = migration_entry_to_page(entry); + page = pfn_swap_entry_to_page(entry); } if (IS_ERR_OR_NULL(page)) return; @@ -691,10 +689,8 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask, } else if (is_swap_pte(*pte)) { swp_entry_t swpent = pte_to_swp_entry(*pte); - if (is_migration_entry(swpent)) - page = migration_entry_to_page(swpent); - else if (is_device_private_entry(swpent)) - page = device_private_entry_to_page(swpent); + if (is_pfn_swap_entry(swpent)) + page = pfn_swap_entry_to_page(swpent); } if (page) { int mapcount = page_mapcount(page); @@ -1383,11 +1379,8 @@ static pagemap_entry_t pte_to_pagemap_entry(struct pagemapread *pm, frame = swp_type(entry) | (swp_offset(entry) << MAX_SWAPFILES_SHIFT); flags |= PM_SWAP; - if (is_migration_entry(entry)) - page = migration_entry_to_page(entry); - - if (is_device_private_entry(entry)) - page = device_private_entry_to_page(entry); + if (is_pfn_swap_entry(entry)) + page = pfn_swap_entry_to_page(entry); } if (page && !PageAnon(page)) @@ -1444,7 +1437,7 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned long addr, unsigned long end, if (pmd_swp_soft_dirty(pmd)) flags |= PM_SOFT_DIRTY; VM_BUG_ON(!is_pmd_migration_entry(pmd)); - page = migration_entry_to_page(entry); + page = pfn_swap_entry_to_page(entry); } #endif
[PATCH v7 0/8] Add support for SVM atomics in Nouveau
This is the seventh version of a series to add support to Nouveau for atomic memory operations on OpenCL shared virtual memory (SVM) regions. This version primarily improves readability of the Nouveau fault priority calculation code along with other minor functional and cosmetic improvements listed in the changelogs. Exclusive device access is implemented by adding a new swap entry type (SWAP_DEVICE_EXCLUSIVE) which is similar to a migration entry. The main difference is that on fault the original entry is immediately restored by the fault handler instead of waiting. Restoring the entry triggers calls to MMU notifers which allows a device driver to revoke the atomic access permission from the GPU prior to the CPU finalising the entry. Patches 1 & 2 refactor existing migration and device private entry functions. Patches 3 & 4 rework try_to_unmap_one() by splitting out unrelated functionality into separate functions - try_to_migrate_one() and try_to_munlock_one(). These should not change any functionality, but any help testing would be much appreciated as I have not been able to test every usage of try_to_unmap_one(). Patch 5 contains the bulk of the implementation for device exclusive memory. Patch 6 contains some additions to the HMM selftests to ensure everything works as expected. Patch 7 is a cleanup for the Nouveau SVM implementation. Patch 8 contains the implementation of atomic access for the Nouveau driver. This has been tested using the latest upstream Mesa userspace with a simple OpenCL test program which checks the results of atomic GPU operations on a SVM buffer whilst also writing to the same buffer from the CPU. Alistair Popple (8): mm: Remove special swap entry functions mm/swapops: Rework swap entry manipulation code mm/rmap: Split try_to_munlock from try_to_unmap mm/rmap: Split migration into its own function mm: Device exclusive memory access mm: Selftests for exclusive device memory nouveau/svm: Refactor nouveau_range_fault nouveau/svm: Implement atomic SVM access Documentation/vm/hmm.rst | 19 +- arch/s390/mm/pgtable.c| 2 +- drivers/gpu/drm/nouveau/include/nvif/if000c.h | 1 + drivers/gpu/drm/nouveau/nouveau_svm.c | 156 - drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h | 1 + .../drm/nouveau/nvkm/subdev/mmu/vmmgp100.c| 6 + fs/proc/task_mmu.c| 23 +- include/linux/mmu_notifier.h | 26 +- include/linux/rmap.h | 9 +- include/linux/swap.h | 8 +- include/linux/swapops.h | 123 ++-- lib/test_hmm.c| 126 +++- lib/test_hmm_uapi.h | 2 + mm/debug_vm_pgtable.c | 12 +- mm/hmm.c | 12 +- mm/huge_memory.c | 45 +- mm/hugetlb.c | 10 +- mm/memcontrol.c | 2 +- mm/memory.c | 128 +++- mm/migrate.c | 51 +- mm/mprotect.c | 18 +- mm/page_vma_mapped.c | 15 +- mm/rmap.c | 604 +++--- tools/testing/selftests/vm/hmm-tests.c| 158 + 24 files changed, 1282 insertions(+), 275 deletions(-) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Restrict sentinel requests further
On Wed, Mar 24, 2021 at 03:25:55PM +, Matthew Auld wrote: > On Wed, 24 Mar 2021 at 12:14, Tvrtko Ursulin > wrote: > > > > From: Tvrtko Ursulin > > > > Disallow sentinel requests follow previous sentinels to make request > > cancellation work better when faced with a chain of requests which have > > all been marked as in error. > > > > Because in cases where we end up with a stream of cancelled requests we > > want to turn of request coalescing so they each will get individually > > turn off Fixed while applying. -Daniel > > > skipped by the execlists_schedule_in (which is called per ELSP port, not > > per request). > > > > Signed-off-by: Tvrtko Ursulin > Reviewed-by: Matthew Auld > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/7] drm/i915: Request watchdog infrastructure
On Wed, Mar 24, 2021 at 12:13:33PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Prepares the plumbing for setting request/fence expiration time. All code > is put in place but is never activated due yet missing ability to actually > configure the timer. > > Outline of the basic operation: > > A timer is started when request is ready for execution. If the request > completes (retires) before the timer fires, timer is cancelled and nothing > further happens. > > If the timer fires request is added to a lockless list and worker queued. > Purpose of this is twofold: a) It allows request cancellation from a more > friendly context and b) coalesces multiple expirations into a single event > of consuming the list. > > Worker locklessly consumes the list of expired requests and cancels them > all using previous added i915_request_cancel(). > > Associated timeout value is stored in rq->context.watchdog.timeout_us. > > v2: > * Log expiration. > > v3: > * Include more information about user timeline in the log message. > > v4: > * Remove obsolete comment and fix formatting. (Matt) > > Signed-off-by: Tvrtko Ursulin > Cc: Daniel Vetter > Reviewed-by: Matthew Auld > --- > drivers/gpu/drm/i915/gt/intel_context_types.h | 4 ++ > .../drm/i915/gt/intel_execlists_submission.h | 2 + > drivers/gpu/drm/i915/gt/intel_gt.c| 3 ++ > drivers/gpu/drm/i915/gt/intel_gt.h| 2 + > drivers/gpu/drm/i915/gt/intel_gt_requests.c | 28 ++ > drivers/gpu/drm/i915/gt/intel_gt_types.h | 7 +++ > drivers/gpu/drm/i915/i915_request.c | 52 +++ > drivers/gpu/drm/i915/i915_request.h | 8 +++ > 8 files changed, 106 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h > b/drivers/gpu/drm/i915/gt/intel_context_types.h > index 0ea18c9e2aca..65a5730a4f5b 100644 > --- a/drivers/gpu/drm/i915/gt/intel_context_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h > @@ -99,6 +99,10 @@ struct intel_context { > #define CONTEXT_FORCE_SINGLE_SUBMISSION 7 > #define CONTEXT_NOPREEMPT8 > > + struct { > + u64 timeout_us; > + } watchdog; > + > u32 *lrc_reg_state; > union { > struct { > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.h > b/drivers/gpu/drm/i915/gt/intel_execlists_submission.h > index f7bd3fccfee8..4ca9b475e252 100644 > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.h > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.h > @@ -6,6 +6,7 @@ > #ifndef __INTEL_EXECLISTS_SUBMISSION_H__ > #define __INTEL_EXECLISTS_SUBMISSION_H__ > > +#include > #include > > struct drm_printer; > @@ -13,6 +14,7 @@ struct drm_printer; > struct i915_request; > struct intel_context; > struct intel_engine_cs; > +struct intel_gt; > > enum { > INTEL_CONTEXT_SCHEDULE_IN = 0, > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c > b/drivers/gpu/drm/i915/gt/intel_gt.c > index ca76f93bc03d..8d77dcbad059 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -31,6 +31,9 @@ void intel_gt_init_early(struct intel_gt *gt, struct > drm_i915_private *i915) > INIT_LIST_HEAD(>closed_vma); > spin_lock_init(>closed_lock); > > + init_llist_head(>watchdog.list); > + INIT_WORK(>watchdog.work, intel_gt_watchdog_work); > + > intel_gt_init_buffer_pool(gt); > intel_gt_init_reset(gt); > intel_gt_init_requests(gt); > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h > b/drivers/gpu/drm/i915/gt/intel_gt.h > index a17bd8b3195f..7ec395cace69 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt.h > @@ -78,4 +78,6 @@ static inline bool intel_gt_is_wedged(const struct intel_gt > *gt) > void intel_gt_info_print(const struct intel_gt_info *info, >struct drm_printer *p); > > +void intel_gt_watchdog_work(struct work_struct *work); > + > #endif /* __INTEL_GT_H__ */ > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c > b/drivers/gpu/drm/i915/gt/intel_gt_requests.c > index 36ec97f79174..fbfd19b2e5f2 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c > @@ -8,6 +8,7 @@ > #include "i915_drv.h" /* for_each_engine() */ > #include "i915_request.h" > #include "intel_engine_heartbeat.h" > +#include "intel_execlists_submission.h" > #include "intel_gt.h" > #include "intel_gt_pm.h" > #include "intel_gt_requests.h" > @@ -242,4 +243,31 @@ void intel_gt_fini_requests(struct intel_gt *gt) > { > /* Wait until the work is marked as finished before unloading! */ > cancel_delayed_work_sync(>requests.retire_work); > + > + flush_work(>watchdog.work); > +} > + > +void intel_gt_watchdog_work(struct work_struct *work) > +{ > + struct intel_gt *gt = > + container_of(work, typeof(*gt), watchdog.work); > + struct i915_request *rq,
[PATCH] drm/doc: Add RFC section
Motivated by the pre-review process for i915 gem/gt features, but probably useful in general for complex stuff. v2: Add reminder to not forget userspace projects in the discussion (Simon, Jason) v3: Actually put this into a folder, so we have it all (.rst files and headers for kerneldoc) contained somewhere separate (Jason) Cc: Simon Ser Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: Jason Ekstrand Cc: Dave Airlie Acked-by: Jason Ekstrand Acked-by: Simon Ser Acked-by: Rodrigo Vivi Acked-by: Dave Airlie Signed-off-by: Daniel Vetter --- Documentation/gpu/index.rst | 1 + Documentation/gpu/rfc/index.rst | 17 + 2 files changed, 18 insertions(+) create mode 100644 Documentation/gpu/rfc/index.rst diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst index c9a51e3bfb5a..ec4bc72438e4 100644 --- a/Documentation/gpu/index.rst +++ b/Documentation/gpu/index.rst @@ -16,6 +16,7 @@ Linux GPU Driver Developer's Guide vga-switcheroo vgaarbiter todo + rfc/index .. only:: subproject and html diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst new file mode 100644 index ..a8621f7dab8b --- /dev/null +++ b/Documentation/gpu/rfc/index.rst @@ -0,0 +1,17 @@ +=== +GPU RFC Section +=== + +For complex work, especially new uapi, it is often good to nail the high level +design issues before getting lost in the code details. This section is meant to +host such documentation: + +* Each RFC should be a section in this file, explaining the goal and main design + considerations. Especially for uapi make sure you Cc: all relevant project + mailing lists and involved people outside of dri-devel. + +* For uapi structures add a file to this directory with and then pull the + kerneldoc in like with real uapi headers. + +* Once the code has landed move all the documentation to the right places in + the main core, helper or driver sections. -- 2.31.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/mst: Enhance MST topology logging
Reviewed-by: Lyude Paul Let me know if you need me to push this to drm-misc-next for you On Thu, 2021-03-25 at 14:06 -0400, Eryk Brol wrote: > [why] > MST topology print was missing fec logging and pdt printed > as an int wasn't clear. vcpi and payload info was printed as an > arbitrary series of ints which requires user to know the ordering > of the prints, making the logs difficult to use. > > [how] > -add fec logging > -add pdt parsing into strings > -format vcpi and payload info into tables with headings > -clean up topology prints > > --- > > v2: Addressed Lyude's comments > -made helper function return const > -fixed indentation and spacing issues > > Signed-off-by: Eryk Brol > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 59 ++- > 1 file changed, 48 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 932c4641ec3e..de5124ce42cb 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -4720,6 +4720,28 @@ static void drm_dp_mst_kick_tx(struct > drm_dp_mst_topology_mgr *mgr) > queue_work(system_long_wq, >tx_work); > } > > +/* > + * Helper function for parsing DP device types into convenient strings > + * for use with dp_mst_topology > + */ > +static const char *pdt_to_string(u8 pdt) > +{ > + switch (pdt) { > + case DP_PEER_DEVICE_NONE: > + return "NONE"; > + case DP_PEER_DEVICE_SOURCE_OR_SST: > + return "SOURCE OR SST"; > + case DP_PEER_DEVICE_MST_BRANCHING: > + return "MST BRANCHING"; > + case DP_PEER_DEVICE_SST_SINK: > + return "SST SINK"; > + case DP_PEER_DEVICE_DP_LEGACY_CONV: > + return "DP LEGACY CONV"; > + default: > + return "ERR"; > + } > +} > + > static void drm_dp_mst_dump_mstb(struct seq_file *m, > struct drm_dp_mst_branch *mstb) > { > @@ -4732,9 +4754,20 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m, > prefix[i] = '\t'; > prefix[i] = '\0'; > > - seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports); > + seq_printf(m, "%smstb - [%p]: num_ports: %d\n", prefix, mstb, mstb- > >num_ports); > list_for_each_entry(port, >ports, next) { > - seq_printf(m, "%sport: %d: input: %d: pdt: %d, ddps: %d ldps: > %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->input, port- > >pdt, port->ddps, port->ldps, port->num_sdp_streams, port- > >num_sdp_stream_sinks, port, port->connector); > + seq_printf(m, "%sport %d - [%p] (%s - %s): ddps: %d, ldps: %d, > sdp: %d/%d, fec: %s, conn: %p\n", > + prefix, > + port->port_num, > + port, > + port->input ? "input" : "output", > + pdt_to_string(port->pdt), > + port->ddps, > + port->ldps, > + port->num_sdp_streams, > + port->num_sdp_stream_sinks, > + port->fec_capable ? "true" : "false", > + port->connector); > if (port->mstb) > drm_dp_mst_dump_mstb(m, port->mstb); > } > @@ -4787,33 +4820,37 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > mutex_unlock(>lock); > > mutex_lock(>payload_lock); > - seq_printf(m, "vcpi: %lx %lx %d\n", mgr->payload_mask, mgr->vcpi_mask, > - mgr->max_payloads); > + seq_printf(m, "\n*** VCPI Info ***\n"); > + seq_printf(m, "payload_mask: %lx, vcpi_mask: %lx, max_payloads: %d\n", > mgr->payload_mask, mgr->vcpi_mask, mgr->max_payloads); > > + seq_printf(m, "\n| idx | port # | vcp_id | # slots | sink > name |\n"); > for (i = 0; i < mgr->max_payloads; i++) { > if (mgr->proposed_vcpis[i]) { > char name[14]; > > port = container_of(mgr->proposed_vcpis[i], struct > drm_dp_mst_port, vcpi); > fetch_monitor_name(mgr, port, name, sizeof(name)); > - seq_printf(m, "vcpi %d: %d %d %d sink name: %s\n", i, > - port->port_num, port->vcpi.vcpi, > + seq_printf(m, "%10d%10d%10d%10d%20s\n", > + i, > + port->port_num, > + port->vcpi.vcpi, > port->vcpi.num_slots, > - (*name != 0) ? name : "Unknown"); > + (*name != 0) ? name : "Unknown"); > } else > - seq_printf(m, "vcpi %d:unused\n", i); > + seq_printf(m,
[PATCH 4/4] drm/i915: Implement SINGLE_TIMELINE with a syncobj (v3)
This API is entirely unnecessary and I'd love to get rid of it. If userspace wants a single timeline across multiple contexts, they can either use implicit synchronization or a syncobj, both of which existed at the time this feature landed. The justification given at the time was that it would help GL drivers which are inherently single-timeline. However, neither of our GL drivers actually wanted the feature. i965 was already in maintenance mode at the time and iris uses syncobj for everything. Unfortunately, as much as I'd love to get rid of it, it is used by the media driver so we can't do that. We can, however, do the next-best thing which is to embed a syncobj in the context and do exactly what we'd expect from userspace internally. This isn't an entirely identical implementation because it's no longer atomic if userspace races with itself by calling execbuffer2 twice simultaneously from different threads. It won't crash in that case; it just doesn't guarantee any ordering between those two submits. Moving SINGLE_TIMELINE to a syncobj emulation has a couple of technical advantages beyond mere annoyance. One is that intel_timeline is no longer an api-visible object and can remain entirely an implementation detail. This may be advantageous as we make scheduler changes going forward. Second is that, together with deleting the CLONE_CONTEXT API, we should now have a 1:1 mapping between intel_context and intel_timeline which may help us reduce locking. v2 (Jason Ekstrand): - Update the comment on i915_gem_context::syncobj to mention that it's an emulation and the possible race if userspace calls execbuffer2 twice on the same context concurrently. - Wrap the checks for eb.gem_context->syncobj in unlikely() - Drop the dma_fence reference - Improved commit message v3 (Jason Ekstrand): - Move the dma_fence_put() to before the error exit Signed-off-by: Jason Ekstrand Cc: Maarten Lankhorst Cc: Matthew Brost --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 47 --- .../gpu/drm/i915/gem/i915_gem_context_types.h | 14 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 16 +++ 3 files changed, 39 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index ea449be2672c2..87d1715e354fe 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -67,6 +67,8 @@ #include #include +#include + #include "gt/gen6_ppgtt.h" #include "gt/intel_context.h" #include "gt/intel_engine_heartbeat.h" @@ -225,10 +227,6 @@ static void intel_context_set_gem(struct intel_context *ce, ce->vm = vm; } - GEM_BUG_ON(ce->timeline); - if (ctx->timeline) - ce->timeline = intel_timeline_get(ctx->timeline); - if (ctx->sched.priority >= I915_PRIORITY_NORMAL && intel_engine_has_timeslices(ce->engine)) __set_bit(CONTEXT_USE_SEMAPHORES, >flags); @@ -365,8 +363,8 @@ void i915_gem_context_release(struct kref *ref) if (ctx->client) i915_drm_client_put(ctx->client); - if (ctx->timeline) - intel_timeline_put(ctx->timeline); + if (ctx->syncobj) + drm_syncobj_put(ctx->syncobj); mutex_destroy(>engines_mutex); mutex_destroy(>lut_mutex); @@ -820,33 +818,11 @@ static void __assign_ppgtt(struct i915_gem_context *ctx, i915_vm_close(vm); } -static void __set_timeline(struct intel_timeline **dst, - struct intel_timeline *src) -{ - struct intel_timeline *old = *dst; - - *dst = src ? intel_timeline_get(src) : NULL; - - if (old) - intel_timeline_put(old); -} - -static void __apply_timeline(struct intel_context *ce, void *timeline) -{ - __set_timeline(>timeline, timeline); -} - -static void __assign_timeline(struct i915_gem_context *ctx, - struct intel_timeline *timeline) -{ - __set_timeline(>timeline, timeline); - context_apply_all(ctx, __apply_timeline, timeline); -} - static struct i915_gem_context * i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags) { struct i915_gem_context *ctx; + int ret; if (flags & I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE && !HAS_EXECLISTS(i915)) @@ -875,16 +851,13 @@ i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags) } if (flags & I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE) { - struct intel_timeline *timeline; - - timeline = intel_timeline_create(>gt); - if (IS_ERR(timeline)) { + ret = drm_syncobj_create(>syncobj, +DRM_SYNCOBJ_CREATE_SIGNALED, +NULL); + if (ret) { context_close(ctx); -
Re: [PATCH] drm/i915: Implement SINGLE_TIMELINE with a syncobj (v2)
On Thu, Mar 25, 2021 at 4:21 PM Matthew Brost wrote: > > On Tue, Mar 23, 2021 at 12:51:49PM -0500, Jason Ekstrand wrote: > > This API is entirely unnecessary and I'd love to get rid of it. If > > userspace wants a single timeline across multiple contexts, they can > > either use implicit synchronization or a syncobj, both of which existed > > at the time this feature landed. The justification given at the time > > was that it would help GL drivers which are inherently single-timeline. > > However, neither of our GL drivers actually wanted the feature. i965 > > was already in maintenance mode at the time and iris uses syncobj for > > everything. > > > > Unfortunately, as much as I'd love to get rid of it, it is used by the > > media driver so we can't do that. We can, however, do the next-best > > thing which is to embed a syncobj in the context and do exactly what > > we'd expect from userspace internally. This isn't an entirely identical > > implementation because it's no longer atomic if userspace races with > > itself by calling execbuffer2 twice simultaneously from different > > threads. It won't crash in that case; it just doesn't guarantee any > > ordering between those two submits. > > > > Moving SINGLE_TIMELINE to a syncobj emulation has a couple of technical > > advantages beyond mere annoyance. One is that intel_timeline is no > > longer an api-visible object and can remain entirely an implementation > > detail. This may be advantageous as we make scheduler changes going > > forward. Second is that, together with deleting the CLONE_CONTEXT API, > > we should now have a 1:1 mapping between intel_context and > > intel_timeline which may help us reduce locking. > > > > I'm not going to dive into the philosophical debate that this patch has > pigeonholed into, but a 1:1 mapping between intel_context and > intel_timeline is huge win IMO - these are the types of cleanups I can > get really get behind. > > > v2 (Jason Ekstrand): > > - Update the comment on i915_gem_context::syncobj to mention that it's > >an emulation and the possible race if userspace calls execbuffer2 > >twice on the same context concurrently. > > - Wrap the checks for eb.gem_context->syncobj in unlikely() > > - Drop the dma_fence reference > > - Improved commit message > > > > Signed-off-by: Jason Ekstrand > > Cc: Maarten Lankhorst > > Cc: Matthew Brost > > --- > > drivers/gpu/drm/i915/gem/i915_gem_context.c | 47 --- > > .../gpu/drm/i915/gem/i915_gem_context_types.h | 14 +- > > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 16 +++ > > 3 files changed, 39 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > index f88bac19333ec..e094f4a1ca4cd 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > @@ -67,6 +67,8 @@ > > #include > > #include > > > > +#include > > + > > #include "gt/gen6_ppgtt.h" > > #include "gt/intel_context.h" > > #include "gt/intel_engine_heartbeat.h" > > @@ -224,10 +226,6 @@ static void intel_context_set_gem(struct intel_context > > *ce, > > ce->vm = vm; > > } > > > > - GEM_BUG_ON(ce->timeline); > > - if (ctx->timeline) > > - ce->timeline = intel_timeline_get(ctx->timeline); > > - > > if (ctx->sched.priority >= I915_PRIORITY_NORMAL && > > intel_engine_has_timeslices(ce->engine)) > > __set_bit(CONTEXT_USE_SEMAPHORES, >flags); > > @@ -344,8 +342,8 @@ void i915_gem_context_release(struct kref *ref) > > mutex_destroy(>engines_mutex); > > mutex_destroy(>lut_mutex); > > > > - if (ctx->timeline) > > - intel_timeline_put(ctx->timeline); > > + if (ctx->syncobj) > > + drm_syncobj_put(ctx->syncobj); > > > > put_pid(ctx->pid); > > mutex_destroy(>mutex); > > @@ -790,33 +788,11 @@ static void __assign_ppgtt(struct i915_gem_context > > *ctx, > > i915_vm_close(vm); > > } > > > > -static void __set_timeline(struct intel_timeline **dst, > > -struct intel_timeline *src) > > -{ > > - struct intel_timeline *old = *dst; > > - > > - *dst = src ? intel_timeline_get(src) : NULL; > > - > > - if (old) > > - intel_timeline_put(old); > > -} > > - > > -static void __apply_timeline(struct intel_context *ce, void *timeline) > > -{ > > - __set_timeline(>timeline, timeline); > > -} > > - > > -static void __assign_timeline(struct i915_gem_context *ctx, > > - struct intel_timeline *timeline) > > -{ > > - __set_timeline(>timeline, timeline); > > - context_apply_all(ctx, __apply_timeline, timeline); > > -} > > - > > static struct i915_gem_context * > > i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags) > > { > > struct i915_gem_context *ctx; > > + int ret; > > > >
Re: [PATCH] drm/atomic: Add the crtc to affected crtc only if uapi.enable = true
On Fri, Mar 19, 2021 at 11:27:59PM +0200, Ville Syrjälä wrote: > On Fri, Mar 19, 2021 at 02:26:24PM -0700, Navare, Manasi wrote: > > On Fri, Mar 19, 2021 at 11:12:41PM +0200, Ville Syrjälä wrote: > > > On Fri, Mar 19, 2021 at 01:54:13PM -0700, Navare, Manasi wrote: > > > > On Fri, Mar 19, 2021 at 04:56:24PM +0200, Ville Syrjälä wrote: > > > > > On Thu, Mar 18, 2021 at 04:01:26PM -0700, Navare, Manasi wrote: > > > > > > So basically we see this warning only in case of bigjoiner when > > > > > > drm_atomic_check gets called without setting the > > > > > > state->allow_modeset flag. > > > > > > > > > > Considering the code is 'WARN(!state->allow_modeset, ...' that > > > > > fact should be rather obvious. > > > > > > > > > > > > > > > > > So do you think that in i915, in intel_atomic_check_bigjoiner() we > > > > > > should only > > > > > > steal the crtc when allow_modeset flag is set in state? > > > > > > > > > > No. If you fully read drm_atomic_check_only() you will observe > > > > > that it will reject any commit w/ allow_modeset==false which > > > > > needs a modeset. And it does that before the WARN. > > > > > > > > > > So you're barking up the wrong tree here. The problem I think > > > > > is that you're just computing requested_crtcs wrong. > > > > > > > > So here in this case, requested CRTC = 0x1 since it requests modeset on > > > > CRTC 0 > > > > Now in teh atomic check, it steals the slave CRTC 1 and hence affected > > > > CRTC comes out > > > > as 0x3 and hence the mismatch. > > > > > > Hmm. How can it be 0x3 if we filtered out the uapi.enable==false case? > > > > > > > Yes if I add that condition like in this patch then it correctly calculates > > the affected crtc bitmask as only 0x1 since it doesnt include the slave > > crtc. > > So with this patch, requested crtc = 0x 1, affected crtc = 0x1 > > > > If this looks good then this fixes our bigjoiner warnings. > > Does this patch look good to you as is then? > > I think you still need to fix the requested_crtcs calculation. We calculate requested crtc at the beginning : for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) requested_crtc |= drm_crtc_mask(crtc); Are you suggesting adding this to after: if (config->funcs->atomic_check) { ret = config->funcs->atomic_check(state->dev, state); if (ret) { DRM_DEBUG_ATOMIC("atomic driver check for %p failed: %d\n", state, ret); return ret; } requested_crtc |= drm_crtc_mask(crtc);// Here it will have requested crtc = 0x11 } in this case here the state should already have master crtc 0 and slave crtc 1 and that requested crtc should already be 0x11 Then in that case we dont need any special check for calculating affected crtc, that also will be 0x11 Manasi > > -- > Ville Syrjälä > Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] mm: unexport follow_pfn
On Wed, Mar 24, 2021 at 08:17:10PM +0100, Daniel Vetter wrote: > On Wed, Mar 24, 2021 at 09:52:11AM -0300, Jason Gunthorpe wrote: > > On Tue, Mar 16, 2021 at 04:33:03PM +0100, Daniel Vetter wrote: > > > Both kvm (in bd2fae8da794 ("KVM: do not assume PTE is writable after > > > follow_pfn")) and vfio (in 07956b6269d3 ("vfio/type1: Use > > > follow_pte()")) have lost their callsites of follow_pfn(). All the > > > other ones have been switched over to unsafe_follow_pfn because they > > > cannot be fixed without breaking userspace api. > > > > > > Argueably the vfio code is still racy, but that's kinda a bigger > > > picture. But since it does leak the pte beyond where it drops the pt > > > lock, without anything else like an mmu notifier guaranteeing > > > coherence, the problem is at least clearly visible in the vfio code. > > > So good enough with me. > > > > > > I've decided to keep the explanation that after dropping the pt lock > > > you must have an mmu notifier if you keep using the pte somehow by > > > adjusting it and moving it into the kerneldoc for the new follow_pte() > > > function. > > > > > > Cc: 3...@google.com > > > Cc: Jann Horn > > > Cc: Paolo Bonzini > > > Cc: Jason Gunthorpe > > > Cc: Cornelia Huck > > > Cc: Peter Xu > > > Cc: Alex Williamson > > > Cc: linux...@kvack.org > > > Cc: linux-arm-ker...@lists.infradead.org > > > Cc: linux-samsung-...@vger.kernel.org > > > Cc: linux-me...@vger.kernel.org > > > Cc: k...@vger.kernel.org > > > Signed-off-by: Daniel Vetter > > > --- > > > include/linux/mm.h | 2 -- > > > mm/memory.c| 26 +- > > > mm/nommu.c | 13 + > > > 3 files changed, 6 insertions(+), 35 deletions(-) > > > > I think this is the right thing to do. > > Was just about to smash this into the topic branch for testing in > linux-next. Feel like an ack on the series, or at least the two mm > patches? Pushed them to my topic branch for a bit of testing in linux-next, hopefully goes all fine for a pull for 5.13. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm: Set drvdata to NULL when msm_drm_init() fails
We should set the platform device's driver data to NULL here so that code doesn't assume the struct drm_device pointer is valid when it could have been destroyed. The lifetime of this pointer is managed by a kref but when msm_drm_init() fails we call drm_dev_put() on the pointer which will free the pointer's memory. This driver uses the component model, so there's sort of two "probes" in this file, one for the platform device i.e. msm_pdev_probe() and one for the component i.e. msm_drm_bind(). The msm_drm_bind() code is using the platform device's driver data to store struct drm_device so the two functions are intertwined. This relationship becomes a problem for msm_pdev_shutdown() when it tests the NULL-ness of the pointer to see if it should call drm_atomic_helper_shutdown(). The NULL test is a proxy check for if the pointer has been freed by kref_put(). If the drm_device has been destroyed, then we shouldn't call the shutdown helper, and we know that is the case if msm_drm_init() failed, therefore set the driver data to NULL so that this pointer liveness is tracked properly. Fixes: 9d5cbf5fe46e ("drm/msm: add shutdown support for display platform_driver") Cc: Dmitry Baryshkov Cc: Fabio Estevam Cc: Krishna Manikandan Signed-off-by: Stephen Boyd --- drivers/gpu/drm/msm/msm_drv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index a5c6b8c23336..196907689c82 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -570,6 +570,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) kfree(priv); err_put_drm_dev: drm_dev_put(ddev); + platform_set_drvdata(pdev, NULL); return ret; } -- https://chromeos.dev ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Implement SINGLE_TIMELINE with a syncobj (v2)
On Tue, Mar 23, 2021 at 12:51:49PM -0500, Jason Ekstrand wrote: > This API is entirely unnecessary and I'd love to get rid of it. If > userspace wants a single timeline across multiple contexts, they can > either use implicit synchronization or a syncobj, both of which existed > at the time this feature landed. The justification given at the time > was that it would help GL drivers which are inherently single-timeline. > However, neither of our GL drivers actually wanted the feature. i965 > was already in maintenance mode at the time and iris uses syncobj for > everything. > > Unfortunately, as much as I'd love to get rid of it, it is used by the > media driver so we can't do that. We can, however, do the next-best > thing which is to embed a syncobj in the context and do exactly what > we'd expect from userspace internally. This isn't an entirely identical > implementation because it's no longer atomic if userspace races with > itself by calling execbuffer2 twice simultaneously from different > threads. It won't crash in that case; it just doesn't guarantee any > ordering between those two submits. > > Moving SINGLE_TIMELINE to a syncobj emulation has a couple of technical > advantages beyond mere annoyance. One is that intel_timeline is no > longer an api-visible object and can remain entirely an implementation > detail. This may be advantageous as we make scheduler changes going > forward. Second is that, together with deleting the CLONE_CONTEXT API, > we should now have a 1:1 mapping between intel_context and > intel_timeline which may help us reduce locking. > I'm not going to dive into the philosophical debate that this patch has pigeonholed into, but a 1:1 mapping between intel_context and intel_timeline is huge win IMO - these are the types of cleanups I can get really get behind. > v2 (Jason Ekstrand): > - Update the comment on i915_gem_context::syncobj to mention that it's >an emulation and the possible race if userspace calls execbuffer2 >twice on the same context concurrently. > - Wrap the checks for eb.gem_context->syncobj in unlikely() > - Drop the dma_fence reference > - Improved commit message > > Signed-off-by: Jason Ekstrand > Cc: Maarten Lankhorst > Cc: Matthew Brost > --- > drivers/gpu/drm/i915/gem/i915_gem_context.c | 47 --- > .../gpu/drm/i915/gem/i915_gem_context_types.h | 14 +- > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 16 +++ > 3 files changed, 39 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c > b/drivers/gpu/drm/i915/gem/i915_gem_context.c > index f88bac19333ec..e094f4a1ca4cd 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > @@ -67,6 +67,8 @@ > #include > #include > > +#include > + > #include "gt/gen6_ppgtt.h" > #include "gt/intel_context.h" > #include "gt/intel_engine_heartbeat.h" > @@ -224,10 +226,6 @@ static void intel_context_set_gem(struct intel_context > *ce, > ce->vm = vm; > } > > - GEM_BUG_ON(ce->timeline); > - if (ctx->timeline) > - ce->timeline = intel_timeline_get(ctx->timeline); > - > if (ctx->sched.priority >= I915_PRIORITY_NORMAL && > intel_engine_has_timeslices(ce->engine)) > __set_bit(CONTEXT_USE_SEMAPHORES, >flags); > @@ -344,8 +342,8 @@ void i915_gem_context_release(struct kref *ref) > mutex_destroy(>engines_mutex); > mutex_destroy(>lut_mutex); > > - if (ctx->timeline) > - intel_timeline_put(ctx->timeline); > + if (ctx->syncobj) > + drm_syncobj_put(ctx->syncobj); > > put_pid(ctx->pid); > mutex_destroy(>mutex); > @@ -790,33 +788,11 @@ static void __assign_ppgtt(struct i915_gem_context *ctx, > i915_vm_close(vm); > } > > -static void __set_timeline(struct intel_timeline **dst, > -struct intel_timeline *src) > -{ > - struct intel_timeline *old = *dst; > - > - *dst = src ? intel_timeline_get(src) : NULL; > - > - if (old) > - intel_timeline_put(old); > -} > - > -static void __apply_timeline(struct intel_context *ce, void *timeline) > -{ > - __set_timeline(>timeline, timeline); > -} > - > -static void __assign_timeline(struct i915_gem_context *ctx, > - struct intel_timeline *timeline) > -{ > - __set_timeline(>timeline, timeline); > - context_apply_all(ctx, __apply_timeline, timeline); > -} > - > static struct i915_gem_context * > i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags) > { > struct i915_gem_context *ctx; > + int ret; > > if (flags & I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE && > !HAS_EXECLISTS(i915)) > @@ -845,16 +821,13 @@ i915_gem_create_context(struct drm_i915_private *i915, > unsigned int flags) > } > > if (flags & I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE) { > -
Re: [igt-dev] [PATCH i-g-t 2/5] tests: Build gem_concurrent_all with meson
On Thu, Mar 25, 2021 at 08:59:12PM +0200, Arkadiusz Hiler wrote: > ...and add it to test-list-full.txt just like we do when building with > autotools. > > Signed-off-by: Arkadiusz Hiler tbh it might be time to sunset this. I kinda started it, it grew to astronomical combinatorial combinations, it's been on the skiplist since ages, and there's even more stuff on the skiplist. Looking at the history it started very benign to tests some pwrite/pread vs rendering synchronization issues. But it's definitely not doing just that anymore. But maybe in a next patch series. On the series: Acked-by: Daniel Vetter > --- > tests/meson.build | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/tests/meson.build b/tests/meson.build > index 54a1a3c7..8e3cd390 100644 > --- a/tests/meson.build > +++ b/tests/meson.build > @@ -402,6 +402,19 @@ test_list_target = custom_target('testlist', > install : true, > install_dir : libexecdir) > > +test_executables += executable('gem_concurrent_all', > 'i915/gem_concurrent_all.c', > +dependencies : test_deps + [ libatomic ], > +install_dir : libexecdir, > +install_rpath : libexecdir_rpathdir, > +install : true) > +test_list += 'gem_concurrent_all' > + > +test_list_full_target = custom_target('testlist-full', > + output : 'test-list-full.txt', > + command : [ gen_testlist, '@OUTPUT@', test_list ], > + install : true, > + install_dir : libexecdir) > + > test_script = find_program('igt_command_line.sh') > foreach prog : test_list > test('testcase check ' + prog, test_script, args : prog) > -- > 2.31.0 > > ___ > igt-dev mailing list > igt-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] drm/msm: drm-msm-next-2021-02-07 for v5.12-rc5
Hi Dave & Daniel, (resend without missing list cc's) A few fixes for v5.12 The following changes since commit 182b4a2d251305201b6f9cae29067f7112f05835: drm/msm/dp: Add a missing semi-colon (2021-02-07 09:57:04 -0800) are available in the Git repository at: https://gitlab.freedesktop.org/drm/msm.git drm-msm-next-2021-02-07 for you to fetch changes up to 627dc55c273dab308303a5217bd3e767d7083ddb: drm/msm/disp/dpu1: icc path needs to be set before dpu runtime resume (2021-03-22 18:52:34 -0700) Dmitry Baryshkov (4): drm/msm/dsi: fix check-before-set in the 7nm dsi_pll code drm/msm/dsi_pll_7nm: Solve TODO for multiplier frac_bits assignment drm/msm/dsi_pll_7nm: Fix variable usage for pll_lockdet_rate drm/msm: fix shutdown hook in case GPU components failed to bind Douglas Anderson (1): drm/msm: Fix speed-bin support not to access outside valid memory Fabio Estevam (1): drm/msm: Fix suspend/resume on i.MX5 Jonathan Marek (1): drm/msm: fix a6xx_gmu_clear_oob Jordan Crouse (1): drm/msm: a6xx: Make sure the SQE microcode is safe Kalyan Thota (1): drm/msm/disp/dpu1: icc path needs to be set before dpu runtime resume Konrad Dybcio (1): drm/msm/adreno: a5xx_power: Don't apply A540 lm_setup to other GPUs Rob Clark (1): drm/msm: Ratelimit invalid-fence message Stephen Boyd (2): drm/msm/kms: Use nested locking for crtc lock instead of custom classes drm/msm/dp: Restore aux retry tuning logic drivers/gpu/drm/msm/adreno/a5xx_power.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 108 -- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 12 ++-- drivers/gpu/drm/msm/dp/dp_aux.c | 7 ++ drivers/gpu/drm/msm/dsi/pll/dsi_pll.c | 2 +- drivers/gpu/drm/msm/dsi/pll/dsi_pll.h | 6 +- drivers/gpu/drm/msm/dsi/pll/dsi_pll_7nm.c | 11 +-- drivers/gpu/drm/msm/msm_atomic.c | 7 +- drivers/gpu/drm/msm/msm_drv.c | 12 drivers/gpu/drm/msm/msm_fence.c | 2 +- drivers/gpu/drm/msm/msm_kms.h | 8 +-- 12 files changed, 119 insertions(+), 60 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/mediatek: Switch the hdmi bridge ops to the atomic versions
Hi Dafna, Thank you for your patch. It'd be nice if you can cc the linux-mediatek ML for next version, so Mediatek people is more aware of this change. IMHO cc'ing the lkml is also a good practice. On 24/3/21 20:12, Dafna Hirschfeld wrote: > The bridge operation '.enable' and the audio cb '.get_eld' > access hdmi->conn. In the future we will want to support > the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR and then we will > not have direct access to the connector. > The atomic version '.atomic_enable' allows accessing the > current connector from the state. > This patch switches the bridge to the atomic version and > saves the current connector in a new field 'curr_conn'. > > Signed-off-by: Dafna Hirschfeld > --- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 41 - > 1 file changed, 29 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c > b/drivers/gpu/drm/mediatek/mtk_hdmi.c > index 8ee55f9e2954..9f415c508b33 100644 > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c > @@ -154,6 +154,7 @@ struct mtk_hdmi { > struct drm_bridge bridge; > struct drm_bridge *next_bridge; > struct drm_connector conn; > + struct drm_connector *curr_conn; Adding a new drm_connector (curr_conn) is something that surprised me at the beginning, then I saw that in the second patch you remove the first drm_connector (conn). I didn't test it yet, but did you test this patch alone? without the second one, maybe I'm missing something but looks to me that this patch alone breaks more functionalities of the driver? (and I know that the driver is broken right now) Is really needed to create a new drm_connector to switch to the atomic versions? > struct device *dev; > const struct mtk_hdmi_conf *conf; > struct phy *phy; > @@ -974,7 +975,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi > *hdmi, > ssize_t err; > > err = drm_hdmi_avi_infoframe_from_display_mode(, > ->conn, mode); > +hdmi->curr_conn, mode); > if (err < 0) { > dev_err(hdmi->dev, > "Failed to get AVI infoframe from mode: %zd\n", err); > @@ -1054,7 +1055,7 @@ static int > mtk_hdmi_setup_vendor_specific_infoframe(struct mtk_hdmi *hdmi, > ssize_t err; > > err = drm_hdmi_vendor_infoframe_from_display_mode(, > - >conn, mode); > + hdmi->curr_conn, > mode); > if (err) { > dev_err(hdmi->dev, > "Failed to get vendor infoframe from mode: %zd\n", err); > @@ -1357,7 +1358,8 @@ static bool mtk_hdmi_bridge_mode_fixup(struct > drm_bridge *bridge, > return true; > } > > -static void mtk_hdmi_bridge_disable(struct drm_bridge *bridge) > +static void mtk_hdmi_bridge_atomic_disable(struct drm_bridge *bridge, > +struct drm_bridge_state > *old_bridge_state) > { > struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge); > > @@ -1368,10 +1370,13 @@ static void mtk_hdmi_bridge_disable(struct drm_bridge > *bridge) > clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_HDMI_PIXEL]); > clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_HDMI_PLL]); > > + hdmi->curr_conn = NULL; > + > hdmi->enabled = false; > } > > -static void mtk_hdmi_bridge_post_disable(struct drm_bridge *bridge) > +static void mtk_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge, > + struct drm_bridge_state > *old_state) > { > struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge); > > @@ -1406,7 +1411,8 @@ static void mtk_hdmi_bridge_mode_set(struct drm_bridge > *bridge, > drm_mode_copy(>mode, adjusted_mode); > } > > -static void mtk_hdmi_bridge_pre_enable(struct drm_bridge *bridge) > +static void mtk_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, > + struct drm_bridge_state > *old_state) > { > struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge); > > @@ -1426,10 +1432,16 @@ static void mtk_hdmi_send_infoframe(struct mtk_hdmi > *hdmi, > mtk_hdmi_setup_vendor_specific_infoframe(hdmi, mode); > } > > -static void mtk_hdmi_bridge_enable(struct drm_bridge *bridge) > +static void mtk_hdmi_bridge_atomic_enable(struct drm_bridge *bridge, > + struct drm_bridge_state *old_state) > { > + struct drm_atomic_state *state = old_state->base.state; > struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge); > > + /* Retrieve the connector through the atomic state. */ > + hdmi->curr_conn = drm_atomic_get_new_connector_for_encoder(state, > + >
[PATCH i-g-t 2/5] tests: Build gem_concurrent_all with meson
...and add it to test-list-full.txt just like we do when building with autotools. Signed-off-by: Arkadiusz Hiler --- tests/meson.build | 13 + 1 file changed, 13 insertions(+) diff --git a/tests/meson.build b/tests/meson.build index 54a1a3c7..8e3cd390 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -402,6 +402,19 @@ test_list_target = custom_target('testlist', install : true, install_dir : libexecdir) +test_executables += executable('gem_concurrent_all', 'i915/gem_concurrent_all.c', + dependencies : test_deps + [ libatomic ], + install_dir : libexecdir, + install_rpath : libexecdir_rpathdir, + install : true) +test_list += 'gem_concurrent_all' + +test_list_full_target = custom_target('testlist-full', + output : 'test-list-full.txt', + command : [ gen_testlist, '@OUTPUT@', test_list ], + install : true, + install_dir : libexecdir) + test_script = find_program('igt_command_line.sh') foreach prog : test_list test('testcase check ' + prog, test_script, args : prog) -- 2.31.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH i-g-t 5/5] Get rid of GNU Autotools
Autotools have been deprecated in favor of Meson since early 2019. Cc: Daniel Vetter Cc: Petri Latvala Cc: Tomi Sarvela Signed-off-by: Arkadiusz Hiler --- Makefile.am | 44 --- autogen.sh | 17 - benchmarks/Makefile.am | 28 -- benchmarks/Makefile.sources | 28 -- configure.ac | 343 -- lib/Makefile.am | 226 lib/Makefile.sources | 195 --- m4/as-compiler-flag.m4 | 62 m4/ax_gcc_func_attribute.m4 | 226 overlay/Makefile.am | 70 scripts/Makefile.am | 2 - tests/Makefile.am| 153 tests/Makefile.sources | 581 --- tests/intel-ci/Makefile.am | 5 - tools/Makefile.am| 31 -- tools/Makefile.sources | 80 - tools/i915-perf/Makefile.am | 29 -- tools/meson.build| 5 +- tools/null_state_gen/Makefile.am | 31 -- tools/registers/Makefile.am | 2 - 20 files changed, 1 insertion(+), 2157 deletions(-) delete mode 100644 Makefile.am delete mode 100755 autogen.sh delete mode 100644 benchmarks/Makefile.am delete mode 100644 benchmarks/Makefile.sources delete mode 100644 configure.ac delete mode 100644 lib/Makefile.am delete mode 100644 lib/Makefile.sources delete mode 100644 m4/as-compiler-flag.m4 delete mode 100644 m4/ax_gcc_func_attribute.m4 delete mode 100644 overlay/Makefile.am delete mode 100644 scripts/Makefile.am delete mode 100644 tests/Makefile.am delete mode 100644 tests/Makefile.sources delete mode 100644 tests/intel-ci/Makefile.am delete mode 100644 tools/Makefile.am delete mode 100644 tools/Makefile.sources delete mode 100644 tools/i915-perf/Makefile.am delete mode 100644 tools/null_state_gen/Makefile.am delete mode 100644 tools/registers/Makefile.am diff --git a/Makefile.am b/Makefile.am deleted file mode 100644 index 94250964.. --- a/Makefile.am +++ /dev/null @@ -1,44 +0,0 @@ -# Copyright © 2005 Adam Jackson. -# Copyright © 2009,2013 Intel Corporation -# -# Permission is hereby granted, free of charge, to any person obtaining a -# copy of this software and associated documentation files (the "Software"), -# to deal in the Software without restriction, including without limitation -# on the rights to use, copy, modify, merge, publish, distribute, sub -# license, and/or sell copies of the Software, and to permit persons to whom -# the Software is furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice (including the next -# paragraph) shall be included in all copies or substantial portions of the -# Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL -# ADAM JACKSON BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER -# IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN -# CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - -ACLOCAL_AMFLAGS = ${ACLOCAL_FLAGS} -I m4 - -SUBDIRS = lib tools scripts benchmarks - -if BUILD_TESTS -SUBDIRS += tests -endif - -if BUILD_X86 -SUBDIRS += overlay benchmarks -endif - -MAINTAINERCLEANFILES = ChangeLog INSTALL - -.PHONY: ChangeLog INSTALL - -INSTALL: - $(INSTALL_CMD) - -ChangeLog: - $(CHANGELOG_CMD) - -dist-hook: ChangeLog INSTALL diff --git a/autogen.sh b/autogen.sh deleted file mode 100755 index 65fcab2f.. --- a/autogen.sh +++ /dev/null @@ -1,17 +0,0 @@ -#! /bin/sh - -srcdir=`dirname $0` -test -z "$srcdir" && srcdir=. - -ORIGDIR=`pwd` -cd $srcdir - -autoreconf -v --install || exit 1 -cd $ORIGDIR || exit $? - -git config --local --get format.subjectPrefix >/dev/null 2>&1 || -git config --local format.subjectPrefix "PATCH i-g-t" - -if test -z "$NOCONFIGURE"; then -$srcdir/configure "$@" -fi diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am deleted file mode 100644 index 45b923eb.. --- a/benchmarks/Makefile.am +++ /dev/null @@ -1,28 +0,0 @@ -include Makefile.sources - -benchmarks_PROGRAMS = $(benchmarks_prog_list) - -if HAVE_LIBDRM_INTEL -benchmarks_PROGRAMS += $(LIBDRM_INTEL_BENCHMARKS) -endif - -AM_CPPFLAGS = \ - -I$(top_srcdir) \ - -I$(top_srcdir)/include/drm-uapi \ - -I$(top_srcdir)/lib \ - -I$(top_srcdir)/lib/stubs/syscalls - -AM_CFLAGS = -I$(top_srcdir)/include/drm-uapi \ - $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) \ - $(WERROR_CFLAGS) -D_GNU_SOURCE -LDADD = $(top_builddir)/lib/libintel_tools.la - -benchmarks_LTLIBRARIES = gem_exec_tracer.la -gem_exec_tracer_la_LDFLAGS = -module -avoid-version -no-undefined -gem_exec_tracer_la_LIBADD = -ldl -
[PATCH i-g-t 3/5] tests: Remove ddx_intel_after_fbdev
It's not a even a proper test. Suggested-by: Petri Latvala Signed-off-by: Arkadiusz Hiler --- tests/Makefile.sources | 4 -- tests/ddx_intel_after_fbdev | 73 - 2 files changed, 77 deletions(-) delete mode 100755 tests/ddx_intel_after_fbdev diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 4f24fb3a..ffc7878a 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -550,10 +550,6 @@ kernel_tests_full = \ $(extra_kernel_tests) \ $(NULL) -scripts = \ - ddx_intel_after_fbdev \ - $(NULL) - IMAGES = pass.png 1080p-left.png 1080p-right.png testdisplay_SOURCES = \ diff --git a/tests/ddx_intel_after_fbdev b/tests/ddx_intel_after_fbdev deleted file mode 100755 index f068209d.. --- a/tests/ddx_intel_after_fbdev +++ /dev/null @@ -1,73 +0,0 @@ -#!/bin/bash -# -# Testcase: Load Intel DDX after fbdev was loaded -# - -whoami | grep -q root || { - echo "ERROR: not running as root" - exit 1 -} - -# no other X session should be running -find /tmp/ -name .X*lock 2>/dev/null | grep -q X && { - echo "ERROR: X session already running" - exit 1 -} - -TMPDIR=$(mktemp -d /tmp/igt.) || { - echo "ERROR: Failed to create temp dir" - exit 1 -} - -cat > $TMPDIR/xorg.conf.fbdev << EOF -Section "Device" - Driver "fbdev" - Identifier "Device[fbdev]" -EndSection -EOF - -cat > $TMPDIR/xorg.conf.intel << EOF -Section "Device" - Driver "intel" - Identifier "Device[intel]" -EndSection -EOF - -# log before fbdev -dmesg -c > $TMPDIR/dmesg.1.before.fbdev -cp /var/log/Xorg.0.log $TMPDIR/Xorg.0.log.1.before.fbdev - -# run fbdev -xinit -- /usr/bin/X -config $TMPDIR/xorg.conf.fbdev & -sleep 5 -if [ -f `which intel_reg` ]; then -`which intel_reg` dump > $TMPDIR/intel_reg_dump.1.fbdev -fi -killall X - -# log after fbdev & before intel -dmesg -c > $TMPDIR/dmesg.2.after.fbdev.before.intel -cp /var/log/Xorg.0.log $TMPDIR/Xorg.0.log.2.after.fbdev.before.intel - -sleep 5 - -# run intel -xinit -- /usr/bin/X -config $TMPDIR/xorg.conf.intel & -sleep 5 -if [ -f `which intel_reg` ]; then -`which intel_reg` dump > $TMPDIR/intel_reg_dump.2.intel -fi -killall X - -# log after intel -dmesg -c > $TMPDIR/dmesg.3.after.intel -cp /var/log/Xorg.0.log $TMPDIR/Xorg.0.log.3.after.intel - -cp $0 $TMPDIR/ - -tar czf $TMPDIR.tar.gz $TMPDIR/* -if [ -f $TMPDIR.tar.gz ]; then - echo $TMPDIR.tar.gz contains this script, all configs and logs generated on this tests -fi - -exit 0 -- 2.31.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH i-g-t 4/5] .gitlab-ci: Don't test Autotools
Signed-off-by: Arkadiusz Hiler --- .gitlab-ci.yml | 18 -- Dockerfile.build-debian | 8 2 files changed, 26 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e226d9d7..2b03fc98 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -154,17 +154,6 @@ build:tests-debian-meson-mips: paths: - build -build:tests-debian-autotools: - image: $CI_REGISTRY/$CI_PROJECT_PATH/build-debian:commit-$CI_COMMIT_SHA - stage: build - script: -- ./autogen.sh --enable-{chamelium,audio,intel,amdgpu,nouveau,tests,runner} -- make -j $(nproc) || make -j 1 -- cp tests/test-list.txt autotools-test-list.txt - artifacts: -paths: - - autotools-test-list.txt - TEST ## test:ninja-test: @@ -236,13 +225,6 @@ test:ninja-test-mips: - build when: on_failure -test:test-list-diff: - dependencies: -- build:tests-debian-autotools -- build:tests-debian-meson - stage: test - script: diff <(sed "s/ /\n/g" meson-test-list.txt| grep -v 'vc4\|v3d\|panfrost\|nouveau' | sort) <(sed "s/ /\n/g" autotools-test-list.txt | sort) - test:list-undocumented-tests: dependencies: - build:tests-fedora diff --git a/Dockerfile.build-debian b/Dockerfile.build-debian index b143a532..454f4bce 100644 --- a/Dockerfile.build-debian +++ b/Dockerfile.build-debian @@ -20,12 +20,4 @@ RUN apt-get install -y \ peg \ libdrm-intel1 -# autotools build deps -RUN apt-get install -y \ - autoconf \ - automake \ - xutils-dev \ - libtool \ - make - RUN apt-get clean -- 2.31.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH i-g-t 1/5] benchmarks: Build gem_exec_tracer with meson
Seems it has been overlooked during mesonification. It's a shared module that's meant to be LD_PRELOAD-ed to intercept EXECBUFFER2 calls for the purpose of replaying them later. Signed-off-by: Arkadiusz Hiler --- benchmarks/meson.build | 8 1 file changed, 8 insertions(+) diff --git a/benchmarks/meson.build b/benchmarks/meson.build index c70e1aac..bede51dc 100644 --- a/benchmarks/meson.build +++ b/benchmarks/meson.build @@ -35,3 +35,11 @@ foreach prog : benchmark_progs install_dir : benchmarksdir, dependencies : igt_deps) endforeach + +lib_gem_exec_tracer = shared_module( + 'gem_exec_tracer', + 'gem_exec_tracer.c', + dependencies : dlsym, + include_directories : inc, + install_dir : benchmarksdir, + install: true) -- 2.31.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On 3/25/21 7:24 PM, Jason Gunthorpe wrote: On Thu, Mar 25, 2021 at 07:13:33PM +0100, Thomas Hellström (Intel) wrote: On 3/25/21 6:55 PM, Jason Gunthorpe wrote: On Thu, Mar 25, 2021 at 06:51:26PM +0100, Thomas Hellström (Intel) wrote: On 3/24/21 9:25 PM, Dave Hansen wrote: On 3/24/21 1:22 PM, Thomas Hellström (Intel) wrote: We also have not been careful at *all* about how _PAGE_BIT_SOFTW* are used. It's quite possible we can encode another use even in the existing bits. Personally, I'd just try: #define _PAGE_BIT_SOFTW5 57 /* available for programmer */ OK, I'll follow your advise here. FWIW I grepped for SW1 and it seems used in a selftest, but only for PTEs AFAICT. Oh, and we don't care about 32-bit much anymore? On x86, we have 64-bit PTEs when running 32-bit kernels if PAE is enabled. IOW, we can handle the majority of 32-bit CPUs out there. But, yeah, we don't care about 32-bit. :) Hmm, Actually it makes some sense to use SW1, to make it end up in the same dword as the PSE bit, as from what I can tell, reading of a 64-bit pmd_t on 32-bit PAE is not atomic, so in theory a huge pmd could be modified while reading the pmd_t making the dwords inconsistent How does that work with fast gup anyway? It loops to get an atomic 64 bit value if the arch can't provide an atomic 64 bit load Hmm, ok, I see a READ_ONCE() in gup_pmd_range(), and then the resulting pmd is dereferenced either in try_grab_compound_head() or __gup_device_huge(), before the pmd is compared to the value the pointer is currently pointing to. Couldn't those dereferences be on invalid pointers? Uh.. That does look questionable, yes. Unless there is some tricky reason why a 64 bit pmd entry on a 32 bit arch either can't exist or has a stable upper 32 bits.. The pte does it with ptep_get_lockless(), we probably need the same for the other levels too instead of open coding a READ_ONCE? Jason Yes, unless that comment before local_irq_disable() means some magic is done to prevent bad things happening, but I guess if it's needed for ptes, it's probably needed for pmds and puds as well. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] iommu/io-pgtable-arm: Add IOMMU_LLC page protection flag
On Wed, Mar 17, 2021 at 2:33 AM Sai Prakash Ranjan wrote: > > Hi Rob, > > On 2021-03-16 22:46, Rob Clark wrote: > > ... > > >> > > > >> > > When the GPU has a buffer mapped with IOMMU_LLC, is the buffer also > >> > > mapped > >> > > into the CPU and with what attributes? Rob said "writecombine for > >> > > everything" -- does that mean ioremap_wc() / MEMREMAP_WC? > >> > > >> > Currently userspace asks for everything WC, so pgprot_writecombine() > >> > > >> > The kernel doesn't enforce this, but so far provides no UAPI to do > >> > anything useful with non-coherent cached mappings (although there is > >> > interest to support this) > >> > > >> > >> btw, I'm looking at a benchmark (gl_driver2_off) where (after some > >> other in-flight optimizations land) we end up bottlenecked on writing > >> to WC cmdstream buffers. I assume in the current state, WC goes all > >> the way to main memory rather than just to system cache? > >> > > > > oh, I guess this (mentioned earlier in thread) is what I really want > > for this benchmark: > > > > https://android-review.googlesource.com/c/kernel/common/+/1549097/3 > > > > You can also check if the system cache lines are allocated for GPU > or not with patch in https://crrev.com/c/2766723 > > With the above patch applied, > cat /sys/kernel/debug/llcc_stats/llcc_scid_status > > The SCIDs for GPU are listed in include/linux/soc/qcom/llcc-qcom.h > Actually for the benchmark I was referring to, it is the *CPU* bottlenecked on writes to writecombine mappings.. so I think what I want is for CPU mappings to be able to use systemcache.. BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On Thu, Mar 25, 2021 at 07:13:33PM +0100, Thomas Hellström (Intel) wrote: > > On 3/25/21 6:55 PM, Jason Gunthorpe wrote: > > On Thu, Mar 25, 2021 at 06:51:26PM +0100, Thomas Hellström (Intel) wrote: > > > On 3/24/21 9:25 PM, Dave Hansen wrote: > > > > On 3/24/21 1:22 PM, Thomas Hellström (Intel) wrote: > > > > > > We also have not been careful at *all* about how _PAGE_BIT_SOFTW* > > > > > > are > > > > > > used. It's quite possible we can encode another use even in the > > > > > > existing bits. > > > > > > > > > > > > Personally, I'd just try: > > > > > > > > > > > > #define _PAGE_BIT_SOFTW5 57 /* available for programmer > > > > > > */ > > > > > > > > > > > OK, I'll follow your advise here. FWIW I grepped for SW1 and it seems > > > > > used in a selftest, but only for PTEs AFAICT. > > > > > > > > > > Oh, and we don't care about 32-bit much anymore? > > > > On x86, we have 64-bit PTEs when running 32-bit kernels if PAE is > > > > enabled. IOW, we can handle the majority of 32-bit CPUs out there. > > > > > > > > But, yeah, we don't care about 32-bit. :) > > > Hmm, > > > > > > Actually it makes some sense to use SW1, to make it end up in the same > > > dword > > > as the PSE bit, as from what I can tell, reading of a 64-bit pmd_t on > > > 32-bit > > > PAE is not atomic, so in theory a huge pmd could be modified while reading > > > the pmd_t making the dwords inconsistent How does that work with fast > > > gup anyway? > > It loops to get an atomic 64 bit value if the arch can't provide an > > atomic 64 bit load > > Hmm, ok, I see a READ_ONCE() in gup_pmd_range(), and then the resulting pmd > is dereferenced either in try_grab_compound_head() or __gup_device_huge(), > before the pmd is compared to the value the pointer is currently pointing > to. Couldn't those dereferences be on invalid pointers? Uh.. That does look questionable, yes. Unless there is some tricky reason why a 64 bit pmd entry on a 32 bit arch either can't exist or has a stable upper 32 bits.. The pte does it with ptep_get_lockless(), we probably need the same for the other levels too instead of open coding a READ_ONCE? Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 4/5] drm/bridge: anx7625: add HDCP support
On Fri, Mar 19, 2021 at 2:35 AM Xin Ji wrote: > > Add HDCP feature, enable HDCP function through chip internal key > and downstream's capability. > > Signed-off-by: Xin Ji > --- > drivers/gpu/drm/bridge/analogix/anx7625.c | 147 ++ > drivers/gpu/drm/bridge/analogix/anx7625.h | 36 ++ > 2 files changed, 183 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > b/drivers/gpu/drm/bridge/analogix/anx7625.c > index 8c514b46d361..b424a570effa 100644 > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > @@ -633,6 +633,150 @@ static int anx7625_dpi_config(struct anx7625_data *ctx) > return ret; > } > > +static int anx7625_aux_dpcd_read(struct anx7625_data *ctx, > +u8 addrh, u8 addrm, u8 addrl, > +u8 len, u8 *buf) > +{ > + struct device *dev = >client->dev; > + int ret; > + u8 cmd; > + > + if (len > MAX_DPCD_BUFFER_SIZE) { > + DRM_DEV_ERROR(dev, "exceed aux buffer len.\n"); > + return -E2BIG; > + } > + > + cmd = ((len - 1) << 4) | 0x09; > + > + /* Set command and length */ > + ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + AP_AUX_COMMAND, cmd); > + > + /* Set aux access address */ > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > +AP_AUX_ADDR_7_0, addrl); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > +AP_AUX_ADDR_15_8, addrm); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > +AP_AUX_ADDR_19_16, addrh); > + > + /* Enable aux access */ > + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client, > + AP_AUX_CTRL_STATUS, AP_AUX_CTRL_OP_EN); > + > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "cannot access aux related register.\n"); > + return -EIO; > + } > + > + usleep_range(2000, 2100); > + > + ret = wait_aux_op_finish(ctx); > + if (ret) { > + DRM_DEV_ERROR(dev, "aux IO error: wait aux op finish.\n"); > + return ret; > + } > + > + ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client, > +AP_AUX_BUFF_START, len, buf); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "read dpcd register failed\n"); > + return -EIO; > + } > + > + return 0; > +} > + > +static int anx7625_read_flash_status(struct anx7625_data *ctx) > +{ > + return anx7625_reg_read(ctx, ctx->i2c.rx_p0_client, R_RAM_CTRL); > +} > + > +static int anx7625_hdcp_key_probe(struct anx7625_data *ctx) > +{ > + int ret, val; > + struct device *dev = >client->dev; > + u8 ident[32]; > + > + ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + FLASH_ADDR_HIGH, 0x91); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > +FLASH_ADDR_LOW, 0xA0); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "IO error : set key flash address.\n"); > + return ret; > + } > + > + ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + FLASH_LEN_HIGH, (FLASH_BUF_LEN - 1) >> 8); > + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > +FLASH_LEN_LOW, (FLASH_BUF_LEN - 1) & 0xFF); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "IO error : set key flash len.\n"); > + return ret; > + } > + > + ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, > + R_FLASH_RW_CTRL, FLASH_READ); > + ret |= readx_poll_timeout(anx7625_read_flash_status, > + ctx, val, > + ((val & FLASH_DONE) || (val < 0)), > + 2000, > + 2000 * 150); > + if (ret) { > + DRM_DEV_ERROR(dev, "flash read access fail!\n"); > + return -EIO; > + } > + > + ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client, > +FLASH_BUF_BASE_ADDR, > +FLASH_BUF_LEN, ident); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "read flash data fail!\n"); > + return -EIO; > + } > + > + if (ident[29] == 0xFF && ident[30] == 0xFF && ident[31] == 0xFF) > + return -EINVAL; > + > + return 0; > +} > + > +static int anx7625_hdcp_setting(struct anx7625_data *ctx) > +{ > + u8 bcap; > + int ret; > + struct device *dev = >client->dev; > + > + ret = anx7625_hdcp_key_probe(ctx); > + if (ret) { > +
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On 3/25/21 6:55 PM, Jason Gunthorpe wrote: On Thu, Mar 25, 2021 at 06:51:26PM +0100, Thomas Hellström (Intel) wrote: On 3/24/21 9:25 PM, Dave Hansen wrote: On 3/24/21 1:22 PM, Thomas Hellström (Intel) wrote: We also have not been careful at *all* about how _PAGE_BIT_SOFTW* are used. It's quite possible we can encode another use even in the existing bits. Personally, I'd just try: #define _PAGE_BIT_SOFTW5 57 /* available for programmer */ OK, I'll follow your advise here. FWIW I grepped for SW1 and it seems used in a selftest, but only for PTEs AFAICT. Oh, and we don't care about 32-bit much anymore? On x86, we have 64-bit PTEs when running 32-bit kernels if PAE is enabled. IOW, we can handle the majority of 32-bit CPUs out there. But, yeah, we don't care about 32-bit. :) Hmm, Actually it makes some sense to use SW1, to make it end up in the same dword as the PSE bit, as from what I can tell, reading of a 64-bit pmd_t on 32-bit PAE is not atomic, so in theory a huge pmd could be modified while reading the pmd_t making the dwords inconsistent How does that work with fast gup anyway? It loops to get an atomic 64 bit value if the arch can't provide an atomic 64 bit load Hmm, ok, I see a READ_ONCE() in gup_pmd_range(), and then the resulting pmd is dereferenced either in try_grab_compound_head() or __gup_device_huge(), before the pmd is compared to the value the pointer is currently pointing to. Couldn't those dereferences be on invalid pointers? /Thomas Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/mst: Enhance MST topology logging
[why] MST topology print was missing fec logging and pdt printed as an int wasn't clear. vcpi and payload info was printed as an arbitrary series of ints which requires user to know the ordering of the prints, making the logs difficult to use. [how] -add fec logging -add pdt parsing into strings -format vcpi and payload info into tables with headings -clean up topology prints --- v2: Addressed Lyude's comments -made helper function return const -fixed indentation and spacing issues Signed-off-by: Eryk Brol --- drivers/gpu/drm/drm_dp_mst_topology.c | 59 ++- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 932c4641ec3e..de5124ce42cb 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4720,6 +4720,28 @@ static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr) queue_work(system_long_wq, >tx_work); } +/* + * Helper function for parsing DP device types into convenient strings + * for use with dp_mst_topology + */ +static const char *pdt_to_string(u8 pdt) +{ + switch (pdt) { + case DP_PEER_DEVICE_NONE: + return "NONE"; + case DP_PEER_DEVICE_SOURCE_OR_SST: + return "SOURCE OR SST"; + case DP_PEER_DEVICE_MST_BRANCHING: + return "MST BRANCHING"; + case DP_PEER_DEVICE_SST_SINK: + return "SST SINK"; + case DP_PEER_DEVICE_DP_LEGACY_CONV: + return "DP LEGACY CONV"; + default: + return "ERR"; + } +} + static void drm_dp_mst_dump_mstb(struct seq_file *m, struct drm_dp_mst_branch *mstb) { @@ -4732,9 +4754,20 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m, prefix[i] = '\t'; prefix[i] = '\0'; - seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports); + seq_printf(m, "%smstb - [%p]: num_ports: %d\n", prefix, mstb, mstb->num_ports); list_for_each_entry(port, >ports, next) { - seq_printf(m, "%sport: %d: input: %d: pdt: %d, ddps: %d ldps: %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->input, port->pdt, port->ddps, port->ldps, port->num_sdp_streams, port->num_sdp_stream_sinks, port, port->connector); + seq_printf(m, "%sport %d - [%p] (%s - %s): ddps: %d, ldps: %d, sdp: %d/%d, fec: %s, conn: %p\n", + prefix, + port->port_num, + port, + port->input ? "input" : "output", + pdt_to_string(port->pdt), + port->ddps, + port->ldps, + port->num_sdp_streams, + port->num_sdp_stream_sinks, + port->fec_capable ? "true" : "false", + port->connector); if (port->mstb) drm_dp_mst_dump_mstb(m, port->mstb); } @@ -4787,33 +4820,37 @@ void drm_dp_mst_dump_topology(struct seq_file *m, mutex_unlock(>lock); mutex_lock(>payload_lock); - seq_printf(m, "vcpi: %lx %lx %d\n", mgr->payload_mask, mgr->vcpi_mask, - mgr->max_payloads); + seq_printf(m, "\n*** VCPI Info ***\n"); + seq_printf(m, "payload_mask: %lx, vcpi_mask: %lx, max_payloads: %d\n", mgr->payload_mask, mgr->vcpi_mask, mgr->max_payloads); + seq_printf(m, "\n| idx | port # | vcp_id | # slots | sink name |\n"); for (i = 0; i < mgr->max_payloads; i++) { if (mgr->proposed_vcpis[i]) { char name[14]; port = container_of(mgr->proposed_vcpis[i], struct drm_dp_mst_port, vcpi); fetch_monitor_name(mgr, port, name, sizeof(name)); - seq_printf(m, "vcpi %d: %d %d %d sink name: %s\n", i, - port->port_num, port->vcpi.vcpi, + seq_printf(m, "%10d%10d%10d%10d%20s\n", + i, + port->port_num, + port->vcpi.vcpi, port->vcpi.num_slots, - (*name != 0) ? name : "Unknown"); + (*name != 0) ? name : "Unknown"); } else - seq_printf(m, "vcpi %d:unused\n", i); + seq_printf(m, "%6d - Unused\n", i); } + seq_printf(m, "\n*** Payload Info ***\n"); + seq_printf(m, "| idx | state | start slot | # slots |\n"); for (i = 0; i < mgr->max_payloads; i++) { - seq_printf(m, "payload %d: %d, %d, %d\n", + seq_printf(m, "%10d%10d%15d%10d\n", i,
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On Thu, Mar 25, 2021 at 06:51:26PM +0100, Thomas Hellström (Intel) wrote: > > On 3/24/21 9:25 PM, Dave Hansen wrote: > > On 3/24/21 1:22 PM, Thomas Hellström (Intel) wrote: > > > > We also have not been careful at *all* about how _PAGE_BIT_SOFTW* are > > > > used. It's quite possible we can encode another use even in the > > > > existing bits. > > > > > > > > Personally, I'd just try: > > > > > > > > #define _PAGE_BIT_SOFTW5 57 /* available for programmer */ > > > > > > > OK, I'll follow your advise here. FWIW I grepped for SW1 and it seems > > > used in a selftest, but only for PTEs AFAICT. > > > > > > Oh, and we don't care about 32-bit much anymore? > > On x86, we have 64-bit PTEs when running 32-bit kernels if PAE is > > enabled. IOW, we can handle the majority of 32-bit CPUs out there. > > > > But, yeah, we don't care about 32-bit. :) > > Hmm, > > Actually it makes some sense to use SW1, to make it end up in the same dword > as the PSE bit, as from what I can tell, reading of a 64-bit pmd_t on 32-bit > PAE is not atomic, so in theory a huge pmd could be modified while reading > the pmd_t making the dwords inconsistent How does that work with fast > gup anyway? It loops to get an atomic 64 bit value if the arch can't provide an atomic 64 bit load Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On 3/24/21 9:25 PM, Dave Hansen wrote: On 3/24/21 1:22 PM, Thomas Hellström (Intel) wrote: We also have not been careful at *all* about how _PAGE_BIT_SOFTW* are used. It's quite possible we can encode another use even in the existing bits. Personally, I'd just try: #define _PAGE_BIT_SOFTW5 57 /* available for programmer */ OK, I'll follow your advise here. FWIW I grepped for SW1 and it seems used in a selftest, but only for PTEs AFAICT. Oh, and we don't care about 32-bit much anymore? On x86, we have 64-bit PTEs when running 32-bit kernels if PAE is enabled. IOW, we can handle the majority of 32-bit CPUs out there. But, yeah, we don't care about 32-bit. :) Hmm, Actually it makes some sense to use SW1, to make it end up in the same dword as the PSE bit, as from what I can tell, reading of a 64-bit pmd_t on 32-bit PAE is not atomic, so in theory a huge pmd could be modified while reading the pmd_t making the dwords inconsistent How does that work with fast gup anyway? In any case, what would be the best cause of action here? Use SW1 or disable completely for 32-bit? /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] iommu/io-pgtable-arm: Add IOMMU_LLC page protection flag
On Tue, Mar 09, 2021 at 12:10:44PM +0530, Sai Prakash Ranjan wrote: > On 2021-02-05 17:38, Sai Prakash Ranjan wrote: > > On 2021-02-04 03:16, Will Deacon wrote: > > > On Tue, Feb 02, 2021 at 11:56:27AM +0530, Sai Prakash Ranjan wrote: > > > > On 2021-02-01 23:50, Jordan Crouse wrote: > > > > > On Mon, Feb 01, 2021 at 08:20:44AM -0800, Rob Clark wrote: > > > > > > On Mon, Feb 1, 2021 at 3:16 AM Will Deacon wrote: > > > > > > > On Fri, Jan 29, 2021 at 03:12:59PM +0530, Sai Prakash Ranjan > > > > > > > wrote: > > > > > > > > On 2021-01-29 14:35, Will Deacon wrote: > > > > > > > > > On Mon, Jan 11, 2021 at 07:45:04PM +0530, Sai Prakash Ranjan > > > > > > > > > wrote: > > > > > > > > > > +#define IOMMU_LLC(1 << 6) > > > > > > > > > > > > > > > > > > On reflection, I'm a bit worried about exposing this because > > > > > > > > > I think it > > > > > > > > > will > > > > > > > > > introduce a mismatched virtual alias with the CPU (we don't > > > > > > > > > even have a > > > > > > > > > MAIR > > > > > > > > > set up for this memory type). Now, we also have that issue > > > > > > > > > for the PTW, > > > > > > > > > but > > > > > > > > > since we always use cache maintenance (i.e. the streaming > > > > > > > > > API) for > > > > > > > > > publishing the page-tables to a non-coheren walker, it works > > > > > > > > > out. > > > > > > > > > However, > > > > > > > > > if somebody expects IOMMU_LLC to be coherent with a DMA API > > > > > > > > > coherent > > > > > > > > > allocation, then they're potentially in for a nasty surprise > > > > > > > > > due to the > > > > > > > > > mismatched outer-cacheability attributes. > > > > > > > > > > > > > > > > > > > > > > > > > Can't we add the syscached memory type similar to what is done > > > > > > > > on android? > > > > > > > > > > > > > > Maybe. How does the GPU driver map these things on the CPU side? > > > > > > > > > > > > Currently we use writecombine mappings for everything, although > > > > > > there > > > > > > are some cases that we'd like to use cached (but have not merged > > > > > > patches that would give userspace a way to flush/invalidate) > > > > > > > > > > > > > > > > LLC/system cache doesn't have a relationship with the CPU cache. Its > > > > > just a > > > > > little accelerator that sits on the connection from the GPU to DDR and > > > > > caches > > > > > accesses. The hint that Sai is suggesting is used to mark the buffers > > > > > as > > > > > 'no-write-allocate' to prevent GPU write operations from being cached > > > > > in > > > > > the LLC > > > > > which a) isn't interesting and b) takes up cache space for read > > > > > operations. > > > > > > > > > > Its easiest to think of the LLC as a bonus accelerator that has no > > > > > cost > > > > > for > > > > > us to use outside of the unfortunate per buffer hint. > > > > > > > > > > We do have to worry about the CPU cache w.r.t I/O coherency (which is > > > > > a > > > > > different hint) and in that case we have all of concerns that Will > > > > > identified. > > > > > > > > > > > > > For mismatched outer cacheability attributes which Will > > > > mentioned, I was > > > > referring to [1] in android kernel. > > > > > > I've lost track of the conversation here :/ > > > > > > When the GPU has a buffer mapped with IOMMU_LLC, is the buffer also > > > mapped > > > into the CPU and with what attributes? Rob said "writecombine for > > > everything" -- does that mean ioremap_wc() / MEMREMAP_WC? > > > > > > > Rob answered this. > > > > > Finally, we need to be careful when we use the word "hint" as > > > "allocation > > > hint" has a specific meaning in the architecture, and if we only > > > mismatch on > > > those then we're actually ok. But I think IOMMU_LLC is more than > > > just a > > > hint, since it actually drives eviction policy (i.e. it enables > > > writeback). > > > > > > Sorry for the pedantry, but I just want to make sure we're all talking > > > about the same things! > > > > > > > Sorry for the confusion which probably was caused by my mentioning of > > android, NWA(no write allocate) is an allocation hint which we can > > ignore > > for now as it is not introduced yet in upstream. > > > > Any chance of taking this forward? We do not want to miss out on small fps > gain when the product gets released. Do we have a solution to the mismatched virtual alias? Will ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd: Fix a typo in two different sentences
Applied. Thanks! Alex On Mon, Mar 22, 2021 at 6:45 PM Randy Dunlap wrote: > > On 3/22/21 2:06 PM, Bhaskar Chowdhury wrote: > > > > s/defintion/definition/ .two different places. > > > > Signed-off-by: Bhaskar Chowdhury > > Acked-by: Randy Dunlap > > > --- > > drivers/gpu/drm/amd/include/atombios.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/include/atombios.h > > b/drivers/gpu/drm/amd/include/atombios.h > > index c1d7b1d0b952..47eb84598b96 100644 > > --- a/drivers/gpu/drm/amd/include/atombios.h > > +++ b/drivers/gpu/drm/amd/include/atombios.h > > @@ -1987,9 +1987,9 @@ typedef struct _PIXEL_CLOCK_PARAMETERS_V6 > > #define PIXEL_CLOCK_V6_MISC_HDMI_BPP_MASK 0x0c > > #define PIXEL_CLOCK_V6_MISC_HDMI_24BPP 0x00 > > #define PIXEL_CLOCK_V6_MISC_HDMI_36BPP 0x04 > > -#define PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6 0x08//for V6, the > > correct defintion for 36bpp should be 2 for 36bpp(2:1) > > +#define PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6 0x08//for V6, the > > correct definition for 36bpp should be 2 for 36bpp(2:1) > > #define PIXEL_CLOCK_V6_MISC_HDMI_30BPP 0x08 > > -#define PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6 0x04//for V6, the > > correct defintion for 30bpp should be 1 for 36bpp(5:4) > > +#define PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6 0x04//for V6, the > > correct definition for 30bpp should be 1 for 36bpp(5:4) > > #define PIXEL_CLOCK_V6_MISC_HDMI_48BPP 0x0c > > #define PIXEL_CLOCK_V6_MISC_REF_DIV_SRC 0x10 > > #define PIXEL_CLOCK_V6_MISC_GEN_DPREFCLK0x40 > > -- > > > -- > ~Randy > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/amdgpu/gfx_v7_0: Trivial typo fixes
Applied. Thanks! Alex On Thu, Mar 25, 2021 at 5:26 AM Nirmoy wrote: > > > Reviewed-by: Nirmoy Das > > On 3/25/21 9:53 AM, Bhaskar Chowdhury wrote: > > s/acccess/access/ > > s/inferface/interface/ > > s/sequnce/sequence/ .two different places. > > s/retrive/retrieve/ > > s/sheduling/scheduling/ > > s/independant/independent/ > > s/wether/whether/ ..two different places. > > s/emmit/emit/ > > s/synce/sync/ > > > > > > Signed-off-by: Bhaskar Chowdhury > > --- > > drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 22 +++--- > > 1 file changed, 11 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > > index a368724c3dfc..4502b95ddf6b 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > > @@ -1877,7 +1877,7 @@ static void gfx_v7_0_init_compute_vmid(struct > > amdgpu_device *adev) > > mutex_unlock(>srbm_mutex); > > > > /* Initialize all compute VMIDs to have no GDS, GWS, or OA > > -acccess. These should be enabled by FW for target VMIDs. */ > > +access. These should be enabled by FW for target VMIDs. */ > > for (i = adev->vm_manager.first_kfd_vmid; i < AMDGPU_NUM_VMID; i++) { > > WREG32(amdgpu_gds_reg_offset[i].mem_base, 0); > > WREG32(amdgpu_gds_reg_offset[i].mem_size, 0); > > @@ -2058,7 +2058,7 @@ static void gfx_v7_0_constants_init(struct > > amdgpu_device *adev) > >* @adev: amdgpu_device pointer > >* > >* Set up the number and offset of the CP scratch registers. > > - * NOTE: use of CP scratch registers is a legacy inferface and > > + * NOTE: use of CP scratch registers is a legacy interface and > >* is not used by default on newer asics (r6xx+). On newer asics, > >* memory buffers are used for fences rather than scratch regs. > >*/ > > @@ -2172,7 +2172,7 @@ static void gfx_v7_0_ring_emit_vgt_flush(struct > > amdgpu_ring *ring) > >* @seq: sequence number > >* @flags: fence related flags > >* > > - * Emits a fence sequnce number on the gfx ring and flushes > > + * Emits a fence sequence number on the gfx ring and flushes > >* GPU caches. > >*/ > > static void gfx_v7_0_ring_emit_fence_gfx(struct amdgpu_ring *ring, u64 > > addr, > > @@ -2215,7 +2215,7 @@ static void gfx_v7_0_ring_emit_fence_gfx(struct > > amdgpu_ring *ring, u64 addr, > >* @seq: sequence number > >* @flags: fence related flags > >* > > - * Emits a fence sequnce number on the compute ring and flushes > > + * Emits a fence sequence number on the compute ring and flushes > >* GPU caches. > >*/ > > static void gfx_v7_0_ring_emit_fence_compute(struct amdgpu_ring *ring, > > @@ -2245,14 +2245,14 @@ static void gfx_v7_0_ring_emit_fence_compute(struct > > amdgpu_ring *ring, > >* gfx_v7_0_ring_emit_ib - emit an IB (Indirect Buffer) on the ring > >* > >* @ring: amdgpu_ring structure holding ring information > > - * @job: job to retrive vmid from > > + * @job: job to retrieve vmid from > >* @ib: amdgpu indirect buffer object > >* @flags: options (AMDGPU_HAVE_CTX_SWITCH) > >* > >* Emits an DE (drawing engine) or CE (constant engine) IB > >* on the gfx ring. IBs are usually generated by userspace > >* acceleration drivers and submitted to the kernel for > > - * sheduling on the ring. This function schedules the IB > > + * scheduling on the ring. This function schedules the IB > >* on the gfx ring for execution by the GPU. > >*/ > > static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring, > > @@ -2402,7 +2402,7 @@ static int gfx_v7_0_ring_test_ib(struct amdgpu_ring > > *ring, long timeout) > > > > /* > >* CP. > > - * On CIK, gfx and compute now have independant command processors. > > + * On CIK, gfx and compute now have independent command processors. > >* > >* GFX > >* Gfx consists of a single ring and can process both gfx jobs and > > @@ -2630,7 +2630,7 @@ static int gfx_v7_0_cp_gfx_resume(struct > > amdgpu_device *adev) > > ring->wptr = 0; > > WREG32(mmCP_RB0_WPTR, lower_32_bits(ring->wptr)); > > > > - /* set the wb address wether it's enabled or not */ > > + /* set the wb address whether it's enabled or not */ > > rptr_addr = adev->wb.gpu_addr + (ring->rptr_offs * 4); > > WREG32(mmCP_RB0_RPTR_ADDR, lower_32_bits(rptr_addr)); > > WREG32(mmCP_RB0_RPTR_ADDR_HI, upper_32_bits(rptr_addr) & 0xFF); > > @@ -2985,7 +2985,7 @@ static void gfx_v7_0_mqd_init(struct amdgpu_device > > *adev, > > mqd->cp_hqd_pq_wptr_poll_addr_lo = wb_gpu_addr & 0xfffc; > > mqd->cp_hqd_pq_wptr_poll_addr_hi = upper_32_bits(wb_gpu_addr) & > > 0x; > > > > - /* set the wb address wether it's enabled or not */ > > + /* set the wb address whether it's enabled or not */ > > wb_gpu_addr = adev->wb.gpu_addr + (ring->rptr_offs * 4); > >
Re: [PATCH V2] drm/radeon/r600_cs: Few typo fixes
Applied. Thanks! Alex On Wed, Mar 24, 2021 at 7:46 PM Randy Dunlap wrote: > > On 3/24/21 4:29 PM, Bhaskar Chowdhury wrote: > > s/miror/mirror/ > > s/needind/needing/ > > s/informations/information/ > > > > Signed-off-by: Bhaskar Chowdhury > > Acked-by: Randy Dunlap > > Thanks. > > > --- > > Changes from V1: > > Randy's finding incorporated ,i.e in one place,informations->information > > Adjusted the subject line accordingly > > > > drivers/gpu/drm/radeon/r600_cs.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/radeon/r600_cs.c > > b/drivers/gpu/drm/radeon/r600_cs.c > > index 34b7c6f16479..8be4799a98ef 100644 > > --- a/drivers/gpu/drm/radeon/r600_cs.c > > +++ b/drivers/gpu/drm/radeon/r600_cs.c > > @@ -38,7 +38,7 @@ extern void r600_cs_legacy_get_tiling_conf(struct > > drm_device *dev, u32 *npipes, > > > > > > struct r600_cs_track { > > - /* configuration we miror so that we use same code btw kms/ums */ > > + /* configuration we mirror so that we use same code btw kms/ums */ > > u32 group_size; > > u32 nbanks; > > u32 npipes; > > @@ -963,7 +963,7 @@ static int r600_cs_parse_packet0(struct > > radeon_cs_parser *p, > > * > > * This function will test against r600_reg_safe_bm and return 0 > > * if register is safe. If register is not flag as safe this function > > - * will test it against a list of register needind special handling. > > + * will test it against a list of register needing special handling. > > */ > > static int r600_cs_check_reg(struct radeon_cs_parser *p, u32 reg, u32 idx) > > { > > @@ -2336,7 +2336,7 @@ int r600_cs_parse(struct radeon_cs_parser *p) > > /** > > * r600_dma_cs_next_reloc() - parse next reloc > > * @p: parser structure holding parsing context. > > - * @cs_reloc:reloc informations > > + * @cs_reloc:reloc information > > * > > * Return the next reloc, do bo validation and compute > > * GPU offset using the provided start. > > -- > > > -- > ~Randy > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] amdgpu: securedisplay: simplify i2c hexdump output
Applied. Thanks! Alex On Wed, Mar 24, 2021 at 9:37 AM Arnd Bergmann wrote: > > From: Arnd Bergmann > > A previous fix I did left a rather complicated loop in > amdgpu_securedisplay_debugfs_write() for what could be expressed in a > simple sprintf, as Rasmus pointed out. > > This drops the leading 0x for each byte, but is otherwise > much nicer. > > Suggested-by: Rasmus Villemoes > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c | 11 +++ > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c > index 69d7f6bff5d4..fc3ddd7aa6f0 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c > @@ -92,9 +92,7 @@ static ssize_t amdgpu_securedisplay_debugfs_write(struct > file *f, const char __u > struct drm_device *dev = adev_to_drm(adev); > uint32_t phy_id; > uint32_t op; > - int i; > char str[64]; > - char i2c_output[256]; > int ret; > > if (*pos || size > sizeof(str) - 1) > @@ -136,12 +134,9 @@ static ssize_t amdgpu_securedisplay_debugfs_write(struct > file *f, const char __u > ret = psp_securedisplay_invoke(psp, > TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC); > if (!ret) { > if (securedisplay_cmd->status == > TA_SECUREDISPLAY_STATUS__SUCCESS) { > - int pos = 0; > - memset(i2c_output, 0, sizeof(i2c_output)); > - for (i = 0; i < > TA_SECUREDISPLAY_I2C_BUFFER_SIZE; i++) > - pos += sprintf(i2c_output + pos, " > 0x%X", > - > securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf[i]); > - dev_info(adev->dev, "SECUREDISPLAY: I2C > buffer out put is :%s\n", i2c_output); > + dev_info(adev->dev, "SECUREDISPLAY: I2C > buffer out put is: %*ph\n", > +TA_SECUREDISPLAY_I2C_BUFFER_SIZE, > + > securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf); > } else { > psp_securedisplay_parse_resp_status(psp, > securedisplay_cmd->status); > } > -- > 2.29.2 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/amdgpu: Ensure that the modifier requested is supported by plane.
On Wed, Mar 24, 2021 at 4:16 PM Mark Yacoub wrote: > > From: Mark Yacoub > > On initializing the framebuffer, call drm_any_plane_has_format to do a > check if the modifier is supported. drm_any_plane_has_format calls > dm_plane_format_mod_supported which is extended to validate that the > modifier is on the list of the plane's supported modifiers. > > The bug was caught using igt-gpu-tools test: > kms_addfb_basic.addfb25-bad-modifier > > Tested on ChromeOS Zork by turning on the display, running an overlay > test, and running a YT video. > > === Changes from v1 === > Explicitly handle DRM_FORMAT_MOD_INVALID modifier. > > Cc: Alex Deucher > Cc: Bas Nieuwenhuizen > Signed-off-by: default avatarMark Yacoub Applied. Thanks! Alex > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c| 13 + > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 18 +++--- > 2 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > index afa5f8ad0f563..a947b5aa420d2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > @@ -908,6 +908,19 @@ int amdgpu_display_gem_fb_verify_and_init( > _fb_funcs); > if (ret) > goto err; > + /* Verify that the modifier is supported. */ > + if (!drm_any_plane_has_format(dev, mode_cmd->pixel_format, > + mode_cmd->modifier[0])) { > + struct drm_format_name_buf format_name; > + drm_dbg_kms(dev, > + "unsupported pixel format %s / modifier 0x%llx\n", > + drm_get_format_name(mode_cmd->pixel_format, > + _name), > + mode_cmd->modifier[0]); > + > + ret = -EINVAL; > + goto err; > + } > > ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj); > 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 961abf1cf040c..6fc746996c24f 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3939,6 +3939,7 @@ static bool dm_plane_format_mod_supported(struct > drm_plane *plane, > { > struct amdgpu_device *adev = drm_to_adev(plane->dev); > const struct drm_format_info *info = drm_format_info(format); > + int i; > > enum dm_micro_swizzle microtile = > modifier_gfx9_swizzle_mode(modifier) & 3; > > @@ -3946,11 +3947,22 @@ static bool dm_plane_format_mod_supported(struct > drm_plane *plane, > return false; > > /* > -* We always have to allow this modifier, because core DRM still > -* checks LINEAR support if userspace does not provide modifers. > +* We always have to allow these modifiers: > +* 1. Core DRM checks for LINEAR support if userspace does not > provide modifiers. > +* 2. Not passing any modifiers is the same as explicitly passing > INVALID. > */ > - if (modifier == DRM_FORMAT_MOD_LINEAR) > + if (modifier == DRM_FORMAT_MOD_LINEAR || > + modifier == DRM_FORMAT_MOD_INVALID) { > return true; > + } > + > + /* Check that the modifier is on the list of the plane's supported > modifiers. */ > + for (i = 0; i < plane->modifier_count; i++) { > + if (modifier == plane->modifiers[i]) > + break; > + } > + if (i == plane->modifier_count) > + return false; > > /* > * The arbitrary tiling support for multiplane formats has not been > hooked > -- > 2.31.0.291.g576ba9dcdaf-goog > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/2] dt-bindings: display: bridge: Add Chipone ICN6211 bindings
Pushed to drm-misc-next https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a42e37db23b88120aea9fa31f9c0952accb39296 On Mon, 22 Mar 2021 at 11:33, Jagan Teki wrote: > > ICN6211 is MIPI-DSI to RGB Converter bridge from Chipone. > > It has a flexible configuration of MIPI DSI signal input and > produces RGB565, RGB666, RGB888 output format. > > Add dt-bingings for it. > > Signed-off-by: Jagan Teki > Reviewed-by: Robert Foss > Reviewed-by: Rob Herring > --- > Changes for v5: > - rebase drm-misc-next > - collect Rob, Robert review tags > Changes for v4: > - fixed Laurent comments > - added regulators > - replace reset with EN > - fixed warnings pointed by Robert > Changes for v3: > - updated to new dt-bindings style > > .../display/bridge/chipone,icn6211.yaml | 99 +++ > MAINTAINERS | 5 + > 2 files changed, 104 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml > b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml > new file mode 100644 > index ..62c3bd4cb28d > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml > @@ -0,0 +1,99 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/chipone,icn6211.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Chipone ICN6211 MIPI-DSI to RGB Converter bridge > + > +maintainers: > + - Jagan Teki > + > +description: | > + ICN6211 is MIPI-DSI to RGB Converter bridge from chipone. > + > + It has a flexible configuration of MIPI DSI signal input and > + produce RGB565, RGB666, RGB888 output format. > + > +properties: > + compatible: > +enum: > + - chipone,icn6211 > + > + reg: > +maxItems: 1 > +description: virtual channel number of a DSI peripheral > + > + enable-gpios: > +description: Bridge EN pin, chip is reset when EN is low. > + > + vdd1-supply: > +description: A 1.8V/2.5V/3.3V supply that power the MIPI RX. > + > + vdd2-supply: > +description: A 1.8V/2.5V/3.3V supply that power the PLL. > + > + vdd3-supply: > +description: A 1.8V/2.5V/3.3V supply that power the RGB output. > + > + ports: > +$ref: /schemas/graph.yaml#/properties/ports > + > +properties: > + port@0: > +$ref: /schemas/graph.yaml#/properties/port > +description: > + Video port for MIPI DSI input > + > + port@1: > +$ref: /schemas/graph.yaml#/properties/port > +description: > + Video port for MIPI DPI output (panel or connector). > + > +required: > + - port@0 > + - port@1 > + > +required: > + - compatible > + - reg > + - enable-gpios > + - ports > + > +additionalProperties: false > + > +examples: > + - | > +#include > + > +dsi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + bridge@0 { > +compatible = "chipone,icn6211"; > +reg = <0>; > +enable-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5 */ > + > +ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > +reg = <0>; > + > +bridge_in_dsi: endpoint { > + remote-endpoint = <_out_bridge>; > +}; > + }; > + > + port@1 { > +reg = <1>; > + > +bridge_out_panel: endpoint { > + remote-endpoint = <_out_bridge>; > +}; > + }; > +}; > + }; > +}; > diff --git a/MAINTAINERS b/MAINTAINERS > index 4b705ba51c54..b9d11101d060 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5568,6 +5568,11 @@ S: Maintained > F: Documentation/devicetree/bindings/display/panel/boe,himax8279d.yaml > F: drivers/gpu/drm/panel/panel-boe-himax8279d.c > > +DRM DRIVER FOR CHIPONE ICN6211 MIPI-DSI to RGB CONVERTER BRIDGE > +M: Jagan Teki > +S: Maintained > +F: Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml > + > DRM DRIVER FOR FARADAY TVE200 TV ENCODER > M: Linus Walleij > S: Maintained > -- > 2.25.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/scheduler re-insert Bailing job to avoid memleak
There are a few issues here like - how u handle non guilty singnaled jobs in drm_sched_stop, currently looks like you don't call put for them and just explicitly free them as before. Also sched->free_guilty seems useless with the new approach. Do we even need the cleanup mechanism at drm_sched_get_cleanup_job with this approach... But first - We need Christian to express his opinion on this since I think he opposed refcounting jobs and that we should concentrate on fences instead. Christian - can you chime in here ? Andrey On 2021-03-25 5:51 a.m., Zhang, Jack (Jian) wrote: [AMD Official Use Only - Internal Distribution Only] Hi, Andrey Thank you for your good opinions. I literally agree with you that the refcount could solve the get_clean_up_up cocurrent job gracefully, and no need to re-insert the job back anymore. I quickly made a draft for this idea as follows: How do you like it? I will start implement to it after I got your acknowledge. Thanks, Jack +void drm_job_get(struct drm_sched_job *s_job) +{ + kref_get(_job->refcount); +} + +void drm_job_do_release(struct kref *ref) +{ + struct drm_sched_job *s_job; + struct drm_gpu_scheduler *sched; + + s_job = container_of(ref, struct drm_sched_job, refcount); + sched = s_job->sched; + sched->ops->free_job(s_job); +} + +void drm_job_put(struct drm_sched_job *s_job) +{ + kref_put(_job->refcount, drm_job_do_release); +} + static void drm_sched_job_begin(struct drm_sched_job *s_job) { struct drm_gpu_scheduler *sched = s_job->sched; + kref_init(_job->refcount); + drm_job_get(s_job); spin_lock(>job_list_lock); list_add_tail(_job->node, >ring_mirror_list); drm_sched_start_timeout(sched); @@ -294,17 +316,16 @@ static void drm_sched_job_timedout(struct work_struct *work) * drm_sched_cleanup_jobs. It will be reinserted back after sched->thread * is parked at which point it's safe. */ - list_del_init(>node); + drm_job_get(job); spin_unlock(>job_list_lock); job->sched->ops->timedout_job(job); - + drm_job_put(job); /* * Guilty job did complete and hence needs to be manually removed * See drm_sched_stop doc. */ if (sched->free_guilty) { - job->sched->ops->free_job(job); sched->free_guilty = false; } } else { @@ -355,20 +376,6 @@ void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job *bad) - /* - * Reinsert back the bad job here - now it's safe as - * drm_sched_get_cleanup_job cannot race against us and release the - * bad job at this point - we parked (waited for) any in progress - * (earlier) cleanups and drm_sched_get_cleanup_job will not be called - * now until the scheduler thread is unparked. - */ - if (bad && bad->sched == sched) - /* - * Add at the head of the queue to reflect it was the earliest - * job extracted. - */ - list_add(>node, >ring_mirror_list); - /* * Iterate the job list from later to earlier one and either deactive * their HW callbacks or remove them from mirror list if they already @@ -774,7 +781,7 @@ static int drm_sched_main(void *param) kthread_should_stop()); if (cleanup_job) { - sched->ops->free_job(cleanup_job); + drm_job_put(cleanup_job); /* queue timeout for next job */ drm_sched_start_timeout(sched); } diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 5a1f068af1c2..b80513eec90f 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -188,6 +188,7 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f); * to schedule the job. */ struct drm_sched_job { + struct kref refcount; struct spsc_node queue_node; struct drm_gpu_scheduler *sched; struct drm_sched_fence *s_fence; @@ -198,6 +199,7 @@ struct drm_sched_job { enum drm_sched_priority s_priority; struct drm_sched_entity *entity; struct dma_fence_cb cb; + }; *From:* Grodzovsky, Andrey *Sent:* Friday, March 19, 2021 12:17 AM *To:* Zhang, Jack (Jian) ; Christian König ; dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; Koenig, Christian ; Liu, Monk ; Deng, Emily ; Rob Herring ; Tomeu Vizoso ; Steven Price *Subject:* Re: [PATCH v3]
Re: [PATCH] drm: bridge: convert sysfs sprintf/snprintf family to sysfs_emit
Pushed to drm-misc-next https://cgit.freedesktop.org/drm/drm-misc/commit/?id=fffa69aa6b1c89853cd00dea969e4754633596d7 On Sun, 7 Feb 2021 at 10:12, Jiapeng Chong wrote: > > Fix the following coccicheck warning: > > drivers/gpu/drm/bridge/lontium-lt9611uxc.c:858:8-16: WARNING: use > scnprintf or sprintf. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > index fee2795..3cac16d 100644 > --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > @@ -855,7 +855,7 @@ static ssize_t lt9611uxc_firmware_show(struct device > *dev, struct device_attribu > { > struct lt9611uxc *lt9611uxc = dev_get_drvdata(dev); > > - return snprintf(buf, PAGE_SIZE, "%02x\n", lt9611uxc->fw_version); > + return sysfs_emit(buf, "%02x\n", lt9611uxc->fw_version); > } > > static DEVICE_ATTR_RW(lt9611uxc_firmware); > -- > 1.8.3.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH drm/amdgpu 0/2] Convert sysfs sprintf/snprintf family to sysfs_emit
Applied the series. Thanks! Alex On Wed, Mar 24, 2021 at 5:17 AM Tian Tao wrote: > > Use the generic sysfs_emit() function to take place of > snprintf/scnprintf, to avoid buffer overrun. > > Tian Tao (2): > drm/amdgpu: Convert sysfs sprintf/snprintf family to sysfs_emit > drm/amd/pm: Convert sysfs sprintf/snprintf family to sysfs_emit > > drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 +-- > drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 6 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 8 +-- > drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 32 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 4 +- > drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 2 +- > drivers/gpu/drm/amd/pm/amdgpu_pm.c | 88 > ++-- > 9 files changed, 73 insertions(+), 79 deletions(-) > > -- > 2.7.4 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Color mode exposed to user space?
+ dri-devel I don't think it's currently exposed anywhere. Alex On Wed, Mar 24, 2021 at 5:11 AM Werner Sembach wrote: > > Hello, > > is the information which color mode is currently in used for a display (RGB, > YCbCr444, or YCbCr420) exposed to user space somewhere? > > If no: Where would be the best place to put code to expose it to sysfs? > > Thanks in advance, > > Werner Sembach > > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon/radeon_pm: Convert sysfs sprintf/snprintf family to sysfs_emit
On Wed, Mar 24, 2021 at 2:47 AM Tian Tao wrote: > > Fix the following coccicheck warning: > drivers/gpu//drm/radeon/radeon_pm.c:521:9-17: WARNING: use scnprintf or > sprintf > drivers/gpu//drm/radeon/radeon_pm.c:475:8-16: WARNING: use scnprintf or > sprintf > drivers/gpu//drm/radeon/radeon_pm.c:418:8-16: WARNING: use scnprintf or > sprintf > drivers/gpu//drm/radeon/radeon_pm.c:363:8-16: WARNING: use scnprintf or > sprintf > drivers/gpu//drm/radeon/radeon_pm.c:734:8-16: WARNING: use scnprintf or > sprintf > drivers/gpu//drm/radeon/radeon_pm.c:688:8-16: WARNING: use scnprintf or > sprintf > drivers/gpu//drm/radeon/radeon_pm.c:704:8-16: WARNING: use scnprintf or > sprintf > drivers/gpu//drm/radeon/radeon_pm.c:755:8-16: WARNING: use scnprintf or > sprintf > > Signed-off-by: Tian Tao Applied. Thanks! Alex > --- > drivers/gpu/drm/radeon/radeon_pm.c | 36 +--- > 1 file changed, 17 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_pm.c > b/drivers/gpu/drm/radeon/radeon_pm.c > index 1995dad..dd56fcd 100644 > --- a/drivers/gpu/drm/radeon/radeon_pm.c > +++ b/drivers/gpu/drm/radeon/radeon_pm.c > @@ -361,11 +361,10 @@ static ssize_t radeon_get_pm_profile(struct device *dev, > struct radeon_device *rdev = ddev->dev_private; > int cp = rdev->pm.profile; > > - return snprintf(buf, PAGE_SIZE, "%s\n", > - (cp == PM_PROFILE_AUTO) ? "auto" : > - (cp == PM_PROFILE_LOW) ? "low" : > - (cp == PM_PROFILE_MID) ? "mid" : > - (cp == PM_PROFILE_HIGH) ? "high" : "default"); > + return sysfs_emit(buf, "%s\n", (cp == PM_PROFILE_AUTO) ? "auto" : > + (cp == PM_PROFILE_LOW) ? "low" : > + (cp == PM_PROFILE_MID) ? "mid" : > + (cp == PM_PROFILE_HIGH) ? "high" : "default"); > } > > static ssize_t radeon_set_pm_profile(struct device *dev, > @@ -416,9 +415,8 @@ static ssize_t radeon_get_pm_method(struct device *dev, > struct radeon_device *rdev = ddev->dev_private; > int pm = rdev->pm.pm_method; > > - return snprintf(buf, PAGE_SIZE, "%s\n", > - (pm == PM_METHOD_DYNPM) ? "dynpm" : > - (pm == PM_METHOD_PROFILE) ? "profile" : "dpm"); > + return sysfs_emit(buf, "%s\n", (pm == PM_METHOD_DYNPM) ? "dynpm" : > + (pm == PM_METHOD_PROFILE) ? "profile" : "dpm"); > } > > static ssize_t radeon_set_pm_method(struct device *dev, > @@ -473,9 +471,9 @@ static ssize_t radeon_get_dpm_state(struct device *dev, > struct radeon_device *rdev = ddev->dev_private; > enum radeon_pm_state_type pm = rdev->pm.dpm.user_state; > > - return snprintf(buf, PAGE_SIZE, "%s\n", > - (pm == POWER_STATE_TYPE_BATTERY) ? "battery" : > - (pm == POWER_STATE_TYPE_BALANCED) ? "balanced" : > "performance"); > + return sysfs_emit(buf, "%s\n", > + (pm == POWER_STATE_TYPE_BATTERY) ? "battery" : > + (pm == POWER_STATE_TYPE_BALANCED) ? "balanced" : > "performance"); > } > > static ssize_t radeon_set_dpm_state(struct device *dev, > @@ -519,11 +517,11 @@ static ssize_t > radeon_get_dpm_forced_performance_level(struct device *dev, > > if ((rdev->flags & RADEON_IS_PX) && > (ddev->switch_power_state != DRM_SWITCH_POWER_ON)) > - return snprintf(buf, PAGE_SIZE, "off\n"); > + return sysfs_emit(buf, "off\n"); > > - return snprintf(buf, PAGE_SIZE, "%s\n", > - (level == RADEON_DPM_FORCED_LEVEL_AUTO) ? "auto" : > - (level == RADEON_DPM_FORCED_LEVEL_LOW) ? "low" : > "high"); > + return sysfs_emit(buf, "%s\n", > + (level == RADEON_DPM_FORCED_LEVEL_AUTO) ? "auto" : > + (level == RADEON_DPM_FORCED_LEVEL_LOW) ? "low" : > "high"); > } > > static ssize_t radeon_set_dpm_forced_performance_level(struct device *dev, > @@ -686,7 +684,7 @@ static ssize_t radeon_hwmon_show_temp(struct device *dev, > else > temp = 0; > > - return snprintf(buf, PAGE_SIZE, "%d\n", temp); > + return sysfs_emit(buf, "%d\n", temp); > } > > static ssize_t radeon_hwmon_show_temp_thresh(struct device *dev, > @@ -702,7 +700,7 @@ static ssize_t radeon_hwmon_show_temp_thresh(struct > device *dev, > else > temp = rdev->pm.dpm.thermal.max_temp; > > - return snprintf(buf, PAGE_SIZE, "%d\n", temp); > + return sysfs_emit(buf, "%d\n", temp); > } > > static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, radeon_hwmon_show_temp, > NULL, 0); > @@ -732,7 +730,7 @@ static ssize_t radeon_hwmon_show_sclk(struct device *dev, >for hwmon */ > sclk *= 1; > > - return snprintf(buf, PAGE_SIZE, "%u\n", sclk); > + return sysfs_emit(buf,
Re: [PATCH] drm/ttm: fix invalid NULL deref
On 25/03/2021 15:27, Christian König wrote: > The BO might be NULL in this function, use the bdev directly. > > Signed-off-by: Christian König > Reported-by: Colin Ian King > Fixes: a1f091f8ef2b ("drm/ttm: switch to per device LRU lock") > --- > drivers/gpu/drm/ttm/ttm_bo.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 2d2ac532987e..6ab7b66ce36d 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -625,7 +625,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev, > unsigned i; > int ret; > > - spin_lock(>bdev->lru_lock); > + spin_lock(>lru_lock); > for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { > list_for_each_entry(bo, >lru[i], lru) { > bool busy; > @@ -662,7 +662,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev, > if (!bo) { > if (busy_bo && !ttm_bo_get_unless_zero(busy_bo)) > busy_bo = NULL; > - spin_unlock(>bdev->lru_lock); > + spin_unlock(>lru_lock); > ret = ttm_mem_evict_wait_busy(busy_bo, ctx, ticket); > if (busy_bo) > ttm_bo_put(busy_bo); > @@ -676,7 +676,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev, > return ret; > } > > - spin_unlock(>bdev->lru_lock); > + spin_unlock(>lru_lock); > > ret = ttm_bo_evict(bo, ctx); > if (locked) > Looks good to me. Thanks. Reviewed-by: Colin Ian King ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: fix invalid NULL deref
The BO might be NULL in this function, use the bdev directly. Signed-off-by: Christian König Reported-by: Colin Ian King Fixes: a1f091f8ef2b ("drm/ttm: switch to per device LRU lock") --- drivers/gpu/drm/ttm/ttm_bo.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 2d2ac532987e..6ab7b66ce36d 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -625,7 +625,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev, unsigned i; int ret; - spin_lock(>bdev->lru_lock); + spin_lock(>lru_lock); for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { list_for_each_entry(bo, >lru[i], lru) { bool busy; @@ -662,7 +662,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev, if (!bo) { if (busy_bo && !ttm_bo_get_unless_zero(busy_bo)) busy_bo = NULL; - spin_unlock(>bdev->lru_lock); + spin_unlock(>lru_lock); ret = ttm_mem_evict_wait_busy(busy_bo, ctx, ticket); if (busy_bo) ttm_bo_put(busy_bo); @@ -676,7 +676,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev, return ret; } - spin_unlock(>bdev->lru_lock); + spin_unlock(>lru_lock); ret = ttm_bo_evict(bo, ctx); if (locked) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: switch back to static allocation limits for now
The shrinker based approach still has some flaws. Especially that we need temporary pages to free up the pages allocated to the driver is problematic in a shrinker. Signed-off-by: Christian König Acked-by: Daniel Vetter --- drivers/gpu/drm/ttm/ttm_device.c | 14 ++-- drivers/gpu/drm/ttm/ttm_tt.c | 112 --- include/drm/ttm/ttm_tt.h | 3 +- 3 files changed, 53 insertions(+), 76 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 8e821cddf81c..9b787b3caeb5 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -53,7 +53,6 @@ static void ttm_global_release(void) goto out; ttm_pool_mgr_fini(); - ttm_tt_mgr_fini(); __free_page(glob->dummy_read_page); memset(glob, 0, sizeof(*glob)); @@ -64,7 +63,7 @@ static void ttm_global_release(void) static int ttm_global_init(void) { struct ttm_global *glob = _glob; - unsigned long num_pages; + unsigned long num_pages, num_dma32; struct sysinfo si; int ret = 0; @@ -78,8 +77,15 @@ static int ttm_global_init(void) * system memory. */ num_pages = ((u64)si.totalram * si.mem_unit) >> PAGE_SHIFT; - ttm_pool_mgr_init(num_pages * 50 / 100); - ttm_tt_mgr_init(); + num_pages /= 2; + + /* But for DMA32 we limit ourself to only use 2GiB maximum. */ + num_dma32 = (u64)(si.totalram - si.totalhigh) * si.mem_unit + >> PAGE_SHIFT; + num_dma32 = min(num_dma32, 2UL << (30 - PAGE_SHIFT)); + + ttm_pool_mgr_init(num_pages); + ttm_tt_mgr_init(num_pages, num_dma32); glob->dummy_read_page = alloc_page(__GFP_ZERO | GFP_DMA32); diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 95b5cff25f4c..7dcd3fb69495 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -40,8 +40,18 @@ #include "ttm_module.h" -static struct shrinker mm_shrinker; -static atomic_long_t swapable_pages; +static unsigned long ttm_pages_limit; + +MODULE_PARM_DESC(pages_limit, "Limit for the allocated pages"); +module_param_named(pages_limit, ttm_pages_limit, ulong, 0644); + +static unsigned long ttm_dma32_pages_limit; + +MODULE_PARM_DESC(dma32_pages_limit, "Limit for the allocated DMA32 pages"); +module_param_named(dma32_pages_limit, ttm_dma32_pages_limit, ulong, 0644); + +static atomic_long_t ttm_pages_allocated; +static atomic_long_t ttm_dma32_pages_allocated; /* * Allocates a ttm structure for the given BO. @@ -294,8 +304,6 @@ static void ttm_tt_add_mapping(struct ttm_device *bdev, struct ttm_tt *ttm) for (i = 0; i < ttm->num_pages; ++i) ttm->pages[i]->mapping = bdev->dev_mapping; - - atomic_long_add(ttm->num_pages, _pages); } int ttm_tt_populate(struct ttm_device *bdev, @@ -309,12 +317,25 @@ int ttm_tt_populate(struct ttm_device *bdev, if (ttm_tt_is_populated(ttm)) return 0; + atomic_long_add(ttm->num_pages, _pages_allocated); + if (bdev->pool.use_dma32) + atomic_long_add(ttm->num_pages, _dma32_pages_allocated); + + while (atomic_long_read(_pages_allocated) > ttm_pages_limit || + atomic_long_read(_dma32_pages_allocated) > + ttm_dma32_pages_limit) { + + ret = ttm_global_swapout(ctx, GFP_KERNEL); + if (ret) + goto error; + } + if (bdev->funcs->ttm_tt_populate) ret = bdev->funcs->ttm_tt_populate(bdev, ttm, ctx); else ret = ttm_pool_alloc(>pool, ttm, ctx); if (ret) - return ret; + goto error; ttm_tt_add_mapping(bdev, ttm); ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED; @@ -327,6 +348,12 @@ int ttm_tt_populate(struct ttm_device *bdev, } return 0; + +error: + atomic_long_sub(ttm->num_pages, _pages_allocated); + if (bdev->pool.use_dma32) + atomic_long_sub(ttm->num_pages, _dma32_pages_allocated); + return ret; } EXPORT_SYMBOL(ttm_tt_populate); @@ -342,12 +369,9 @@ static void ttm_tt_clear_mapping(struct ttm_tt *ttm) (*page)->mapping = NULL; (*page++)->index = 0; } - - atomic_long_sub(ttm->num_pages, _pages); } -void ttm_tt_unpopulate(struct ttm_device *bdev, - struct ttm_tt *ttm) +void ttm_tt_unpopulate(struct ttm_device *bdev, struct ttm_tt *ttm) { if (!ttm_tt_is_populated(ttm)) return; @@ -357,76 +381,24 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, bdev->funcs->ttm_tt_unpopulate(bdev, ttm); else ttm_pool_free(>pool, ttm); - ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED; -} - -/* As long as pages are available make sure to release at least one */ -static unsigned long
[PATCH] drm/ttm: switch back to static allocation limits for now
The shrinker based approach still has some flaws. Especially that we need temporary pages to free up the pages allocated to the driver is problematic in a shrinker. Signed-off-by: Christian König Acked-by: Daniel Vetter --- drivers/gpu/drm/ttm/ttm_device.c | 14 ++-- drivers/gpu/drm/ttm/ttm_tt.c | 112 --- include/drm/ttm/ttm_tt.h | 3 +- 3 files changed, 53 insertions(+), 76 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 8e821cddf81c..9b787b3caeb5 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -53,7 +53,6 @@ static void ttm_global_release(void) goto out; ttm_pool_mgr_fini(); - ttm_tt_mgr_fini(); __free_page(glob->dummy_read_page); memset(glob, 0, sizeof(*glob)); @@ -64,7 +63,7 @@ static void ttm_global_release(void) static int ttm_global_init(void) { struct ttm_global *glob = _glob; - unsigned long num_pages; + unsigned long num_pages, num_dma32; struct sysinfo si; int ret = 0; @@ -78,8 +77,15 @@ static int ttm_global_init(void) * system memory. */ num_pages = ((u64)si.totalram * si.mem_unit) >> PAGE_SHIFT; - ttm_pool_mgr_init(num_pages * 50 / 100); - ttm_tt_mgr_init(); + num_pages /= 2; + + /* But for DMA32 we limit ourself to only use 2GiB maximum. */ + num_dma32 = (u64)(si.totalram - si.totalhigh) * si.mem_unit + >> PAGE_SHIFT; + num_dma32 = min(num_dma32, 2UL << (30 - PAGE_SHIFT)); + + ttm_pool_mgr_init(num_pages); + ttm_tt_mgr_init(num_pages, num_dma32); glob->dummy_read_page = alloc_page(__GFP_ZERO | GFP_DMA32); diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 95b5cff25f4c..7dcd3fb69495 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -40,8 +40,18 @@ #include "ttm_module.h" -static struct shrinker mm_shrinker; -static atomic_long_t swapable_pages; +static unsigned long ttm_pages_limit; + +MODULE_PARM_DESC(pages_limit, "Limit for the allocated pages"); +module_param_named(pages_limit, ttm_pages_limit, ulong, 0644); + +static unsigned long ttm_dma32_pages_limit; + +MODULE_PARM_DESC(dma32_pages_limit, "Limit for the allocated DMA32 pages"); +module_param_named(dma32_pages_limit, ttm_dma32_pages_limit, ulong, 0644); + +static atomic_long_t ttm_pages_allocated; +static atomic_long_t ttm_dma32_pages_allocated; /* * Allocates a ttm structure for the given BO. @@ -294,8 +304,6 @@ static void ttm_tt_add_mapping(struct ttm_device *bdev, struct ttm_tt *ttm) for (i = 0; i < ttm->num_pages; ++i) ttm->pages[i]->mapping = bdev->dev_mapping; - - atomic_long_add(ttm->num_pages, _pages); } int ttm_tt_populate(struct ttm_device *bdev, @@ -309,12 +317,25 @@ int ttm_tt_populate(struct ttm_device *bdev, if (ttm_tt_is_populated(ttm)) return 0; + atomic_long_add(ttm->num_pages, _pages_allocated); + if (bdev->pool.use_dma32) + atomic_long_add(ttm->num_pages, _dma32_pages_allocated); + + while (atomic_long_read(_pages_allocated) > ttm_pages_limit || + atomic_long_read(_dma32_pages_allocated) > + ttm_dma32_pages_limit) { + + ret = ttm_global_swapout(ctx, GFP_KERNEL); + if (ret) + goto error; + } + if (bdev->funcs->ttm_tt_populate) ret = bdev->funcs->ttm_tt_populate(bdev, ttm, ctx); else ret = ttm_pool_alloc(>pool, ttm, ctx); if (ret) - return ret; + goto error; ttm_tt_add_mapping(bdev, ttm); ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED; @@ -327,6 +348,12 @@ int ttm_tt_populate(struct ttm_device *bdev, } return 0; + +error: + atomic_long_sub(ttm->num_pages, _pages_allocated); + if (bdev->pool.use_dma32) + atomic_long_sub(ttm->num_pages, _dma32_pages_allocated); + return ret; } EXPORT_SYMBOL(ttm_tt_populate); @@ -342,12 +369,9 @@ static void ttm_tt_clear_mapping(struct ttm_tt *ttm) (*page)->mapping = NULL; (*page++)->index = 0; } - - atomic_long_sub(ttm->num_pages, _pages); } -void ttm_tt_unpopulate(struct ttm_device *bdev, - struct ttm_tt *ttm) +void ttm_tt_unpopulate(struct ttm_device *bdev, struct ttm_tt *ttm) { if (!ttm_tt_is_populated(ttm)) return; @@ -357,76 +381,24 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, bdev->funcs->ttm_tt_unpopulate(bdev, ttm); else ttm_pool_free(>pool, ttm); - ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED; -} - -/* As long as pages are available make sure to release at least one */ -static unsigned long
Re: drm/ttm: switch to per device LRU lock
Thanks! Just a copy issue. Patch to fix this is on the mailing list. Christian. Am 25.03.21 um 16:00 schrieb Colin Ian King: Hi, Static analysis with Coverity in linux-next has detected an issue in drivers/gpu/drm/ttm/ttm_bo.c with the follow commit: commit a1f091f8ef2b680a5184db065527612247cb4cae Author: Christian König Date: Tue Oct 6 17:26:42 2020 +0200 drm/ttm: switch to per device LRU lock Instead of having a global lock for potentially less contention. The analysis is as follows: 617 int ttm_mem_evict_first(struct ttm_device *bdev, 618struct ttm_resource_manager *man, 619const struct ttm_place *place, 620struct ttm_operation_ctx *ctx, 621struct ww_acquire_ctx *ticket) 622 { 1. assign_zero: Assigning: bo = NULL. 623struct ttm_buffer_object *bo = NULL, *busy_bo = NULL; 624bool locked = false; 625unsigned i; 626int ret; 627 Explicit null dereferenced (FORWARD_NULL)2. var_deref_op: Dereferencing null pointer bo. 628spin_lock(>bdev->lru_lock); 629for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { The spin_lock on bo is dereferencing a null bo pointer. Colin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
re: drm/ttm: switch to per device LRU lock
Hi, Static analysis with Coverity in linux-next has detected an issue in drivers/gpu/drm/ttm/ttm_bo.c with the follow commit: commit a1f091f8ef2b680a5184db065527612247cb4cae Author: Christian König Date: Tue Oct 6 17:26:42 2020 +0200 drm/ttm: switch to per device LRU lock Instead of having a global lock for potentially less contention. The analysis is as follows: 617 int ttm_mem_evict_first(struct ttm_device *bdev, 618struct ttm_resource_manager *man, 619const struct ttm_place *place, 620struct ttm_operation_ctx *ctx, 621struct ww_acquire_ctx *ticket) 622 { 1. assign_zero: Assigning: bo = NULL. 623struct ttm_buffer_object *bo = NULL, *busy_bo = NULL; 624bool locked = false; 625unsigned i; 626int ret; 627 Explicit null dereferenced (FORWARD_NULL)2. var_deref_op: Dereferencing null pointer bo. 628spin_lock(>bdev->lru_lock); 629for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { The spin_lock on bo is dereferencing a null bo pointer. Colin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] MAINTAINERS: Update Maintainers of DRM Bridge Drivers
Add myself as co-maintainer of DRM Bridge Drivers. Repository commit access has already been granted. https://gitlab.freedesktop.org/freedesktop/freedesktop/-/issues/338 Cc: Neil Armstrong Cc: Laurent Pinchart Cc: Jonas Karlman Cc: Andrzej Hajda Cc: Jernej Škrabec Cc: Daniel Vetter Signed-off-by: Robert Foss Acked-by: Neil Armstrong Acked-by: Maxime Ripard Acked-by: Laurent Pinchart Acked-by: Andrzej Hajda --- Add maintainer Acks. Submitted to follow proper dim workflow. MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 4b705ba51c54..16ace8f58649 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5902,6 +5902,7 @@ F:drivers/gpu/drm/atmel-hlcdc/ DRM DRIVERS FOR BRIDGE CHIPS M: Andrzej Hajda M: Neil Armstrong +M: Robert Foss R: Laurent Pinchart R: Jonas Karlman R: Jernej Skrabec -- 2.31.0.30.g398dba342d.dirty ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: Add linux-mediatek ML for drm Mediatek drivers
Hi, Dafna: Dafna Hirschfeld 於 2021年3月25日 週四 下午9:07寫道: > > Add the linux-mediatek mailing list to drm Mediatek drivers > Applied to mediatek-drm-next [1], thanks. [1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next Regards, Chun-Kuang. > Signed-off-by: Dafna Hirschfeld > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9e876927c60d..8260bc5afe66 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5963,6 +5963,7 @@ DRM DRIVERS FOR MEDIATEK > M: Chun-Kuang Hu > M: Philipp Zabel > L: dri-devel@lists.freedesktop.org > +L: linux-media...@lists.infradead.org (moderated for non-subscribers) > S: Supported > F: Documentation/devicetree/bindings/display/mediatek/ > F: drivers/gpu/drm/mediatek/ > -- > 2.17.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/11] [RFC] drm/i915/dp: fix array overflow warning
On 3/25/21 3:53 AM, Arnd Bergmann wrote: On Thu, Mar 25, 2021 at 9:05 AM Jani Nikula wrote: Clearly something is wrong here, but I can't quite figure out what. Changing the array size to 16 bytes avoids the warning, but is probably the wrong solution here. Ugh. drm_dp_channel_eq_ok() does not actually require more than DP_LINK_STATUS_SIZE - 2 elements in the link_status. It's some other related functions that do, and in most cases it's convenient to read all those DP_LINK_STATUS_SIZE bytes. However, here the case is slightly different for DP MST, and the change causes reserved DPCD addresses to be read. Not sure it matters, but really I think the problem is what drm_dp_channel_eq_ok() advertizes. I also don't like the array notation with sizes in function parameters in general, because I think it's misleading. Would gcc-11 warn if a function actually accesses the memory out of bounds of the size? Yes, that is the point of the warning. Using an explicit length in an array argument type tells gcc that the function will never access beyond the end of that bound, and that passing a short array is a bug. I don't know if this /only/ means triggering a warning, or if gcc is also able to make optimizations after classifying this as undefined behavior that it would not make for an unspecified length. GCC uses the array parameter notation as a hint for warnings but it doesn't optimize on this basis and never will be able to because code that accesses more elements from the array isn't invalid. Adding static to the bound, as in void f (int[static N]) does imply that the function won't access more than N elements and C intends for optimizers to rely on it, although GCC doesn't yet. The warning for the array notation is a more portable alternative to explicitly annotating functions with attribute access, and to -Wvla-parameter for VLA parameters. The latter seem to be used relatively rarely, sometimes deliberately because of the bad rap of VLA objects, even though VLA parameters don't suffer from the same problems. Martin Anyway. I don't think we're going to get rid of the array notation anytime soon, if ever, no matter how much I dislike it, so I think the right fix would be to at least state the correct required size in drm_dp_channel_eq_ok(). Ok. Just to confirm: Changing the declaration to an unspecified length avoids the warnings, as does the patch below: diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index eedbb48815b7..6ebeec3d88a7 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -46,12 +46,12 @@ */ /* Helpers for DP link training */ -static u8 dp_link_status(const u8 link_status[DP_LINK_STATUS_SIZE], int r) +static u8 dp_link_status(const u8 link_status[DP_LINK_STATUS_SIZE - 2], int r) { return link_status[r - DP_LANE0_1_STATUS]; } -static u8 dp_get_lane_status(const u8 link_status[DP_LINK_STATUS_SIZE], +static u8 dp_get_lane_status(const u8 link_status[DP_LINK_STATUS_SIZE - 2], int lane) { int i = DP_LANE0_1_STATUS + (lane >> 1); @@ -61,7 +61,7 @@ static u8 dp_get_lane_status(const u8 link_status[DP_LINK_STATUS_SIZE], return (l >> s) & 0xf; } -bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE], +bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE - 2], int lane_count) { u8 lane_align; diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index edffd1dcca3e..160f7fd127b1 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1456,7 +1456,7 @@ enum drm_dp_phy { #define DP_LINK_CONSTANT_N_VALUE 0x8000 #define DP_LINK_STATUS_SIZE 6 -bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE], +bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE - 2], int lane_count); bool drm_dp_clock_recovery_ok(const u8 link_status[DP_LINK_STATUS_SIZE], int lane_count); This obviously needs a good explanation in the code and the changelog text, which I don't have, but if the above is what you had in mind, please take that and add Reported-by/Tested-by: Arnd Bergmann . Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/6] mfd: rt4831: Adds support for Richtek RT4831 core
On Thu, 25 Mar 2021, ChiYuan Huang wrote: > HI, Lee: > > ChiYuan Huang 於 2021年1月13日 週三 下午10:09寫道: > > > > Lee Jones 於 2021年1月13日 週三 下午8:21寫道: > > > > > > On Thu, 17 Dec 2020, cy_huang wrote: > > > > > > > From: ChiYuan Huang > > > > > > > > This adds support Richtek RT4831 core. It includes four channel WLED > > > > driver > > > > and Display Bias Voltage outputs. > > > > > > > > Signed-off-by: ChiYuan Huang > > > > --- > > > > since v5 > > > > - Rename file name from rt4831-core.c to rt4831.c > > > > - Change RICHTEK_VID to RICHTEK_VENDOR_ID. > > > > - Change gpio_desc nameing from 'enable' to 'enable_gpio' in probe. > > > > - Change variable 'val' to the meaningful name 'chip_id'. > > > > - Refine the error log when vendor id is not matched. > > > > - Remove of_match_ptr. > > > > > > > > since v2 > > > > - Refine Kconfig descriptions. > > > > - Add copyright. > > > > - Refine error logs in probe. > > > > - Refine comment lines in remove and shutdown. > > > > --- > > > > drivers/mfd/Kconfig | 10 + > > > > drivers/mfd/Makefile | 1 + > > > > drivers/mfd/rt4831.c | 124 > > > > +++ > > > > 3 files changed, 135 insertions(+) > > > > create mode 100644 drivers/mfd/rt4831.c > > > > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > > > index 8b99a13..dfb2640 100644 > > > > --- a/drivers/mfd/Kconfig > > > > +++ b/drivers/mfd/Kconfig > > > > @@ -1088,6 +1088,16 @@ config MFD_RDC321X > > > > southbridge which provides access to GPIOs and Watchdog using > > > > the > > > > southbridge PCI device configuration space. > > > > > > > > +config MFD_RT4831 > > > > + tristate "Richtek RT4831 four channel WLED and Display Bias > > > > Voltage" > > > > + depends on I2C > > > > + select MFD_CORE > > > > + select REGMAP_I2C > > > > + help > > > > + This enables support for the Richtek RT4831 that includes 4 > > > > channel > > > > + WLED driving and Display Bias Voltage. It's commonly used to > > > > provide > > > > + power to the LCD display and LCD backlight. > > > > + > > > > config MFD_RT5033 > > > > tristate "Richtek RT5033 Power Management IC" > > > > depends on I2C > > > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > > > index 1780019..28d247b 100644 > > > > --- a/drivers/mfd/Makefile > > > > +++ b/drivers/mfd/Makefile > > > > @@ -235,6 +235,7 @@ obj-$(CONFIG_MFD_MENF21BMC) += menf21bmc.o > > > > obj-$(CONFIG_MFD_HI6421_PMIC)+= hi6421-pmic-core.o > > > > obj-$(CONFIG_MFD_HI655X_PMIC) += hi655x-pmic.o > > > > obj-$(CONFIG_MFD_DLN2) += dln2.o > > > > +obj-$(CONFIG_MFD_RT4831) += rt4831.o > > > > obj-$(CONFIG_MFD_RT5033) += rt5033.o > > > > obj-$(CONFIG_MFD_SKY81452) += sky81452.o > > > > > > > > diff --git a/drivers/mfd/rt4831.c b/drivers/mfd/rt4831.c > > > > new file mode 100644 > > > > index ..2bf8364 > > > > --- /dev/null > > > > +++ b/drivers/mfd/rt4831.c > > > > @@ -0,0 +1,124 @@ > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > +/* > > > > + * Copyright (c) 2020 Richtek Technology Corp. > > > > > > Nit: If you respin this, please bump the date. > > > > > Okay. > > > > + * Author: ChiYuan Huang > > > > + */ > > > > + > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > + > > > > +#define RT4831_REG_REVISION 0x01 > > > > +#define RT4831_REG_ENABLE0x08 > > > > +#define RT4831_REG_I2CPROT 0x15 > > > > + > > > > +#define RICHTEK_VENDOR_ID0x03 > > > > +#define RT4831_VID_MASK GENMASK(1, 0) > > > > +#define RT4831_RESET_MASKBIT(7) > > > > +#define RT4831_I2CSAFETMR_MASK BIT(0) > > > > + > > > > +static const struct mfd_cell rt4831_subdevs[] = { > > > > + OF_MFD_CELL("rt4831-backlight", NULL, NULL, 0, 0, > > > > "richtek,rt4831-backlight"), > > > > + MFD_CELL_NAME("rt4831-regulator") > > > > +}; > > > > + > > > > +static bool rt4831_is_accessible_reg(struct device *dev, unsigned int > > > > reg) > > > > +{ > > > > + if (reg >= RT4831_REG_REVISION && reg <= RT4831_REG_I2CPROT) > > > > + return true; > > > > + return false; > > > > +} > > > > + > > > > +static const struct regmap_config rt4831_regmap_config = { > > > > + .reg_bits = 8, > > > > + .val_bits = 8, > > > > + .max_register = RT4831_REG_I2CPROT, > > > > + > > > > + .readable_reg = rt4831_is_accessible_reg, > > > > + .writeable_reg = rt4831_is_accessible_reg, > > > > +}; > > > > + > > > > +static int rt4831_probe(struct i2c_client *client) > > > > +{ > > > > + struct gpio_desc *enable_gpio; > > > > + struct regmap *regmap; > > > > + unsigned int chip_id; > > > > + int ret; > > > > + > > > > + enable_gpio = devm_gpiod_get_optional(>dev, "enable", > > > > GPIOD_OUT_HIGH); > > > > + if (IS_ERR(enable_gpio)) { > > > > + dev_err(>dev, "Failed to get 'enable'
[Bug 212427] [AMDGPU] Multiple ttm_bo_release warnings
https://bugzilla.kernel.org/show_bug.cgi?id=212427 --- Comment #3 from Alex Deucher (alexdeuc...@gmail.com) --- Reverted in stable: commit bec771b5e0901f4b0bc861bcb58056de5151ae3a Author: Greg Kroah-Hartman Date: Thu Mar 25 09:52:40 2021 +0100 Revert "drm/ttm: Warn on pinning without holding a reference" This reverts commit 7d09e9725b5dcc8d14e101de931e4969d033a6ad which is commit 57fcd550eb15bce14a7154736379dfd4ed60ae81 upstream. It is causing too many warnings on 5.11.y, so should be dropped for now. Cc: Huang Rui Cc: Christian König Cc: Daniel Vetter Cc: Christian Koenig Cc: Huang Rui Cc: Sasha Levin Reported-by: Oleksandr Natalenko Link: https://lore.kernel.org/r/8c3da8bc-0bf3-496f-1fd6-4f65a07b2...@amd.com Signed-off-by: Greg Kroah-Hartman -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 212425] Kernel warning at drivers/gpu/drm/ttm/ttm_bo.c:517
https://bugzilla.kernel.org/show_bug.cgi?id=212425 --- Comment #4 from Alex Deucher (alexdeuc...@gmail.com) --- Reverted in stable: commit bec771b5e0901f4b0bc861bcb58056de5151ae3a Author: Greg Kroah-Hartman Date: Thu Mar 25 09:52:40 2021 +0100 Revert "drm/ttm: Warn on pinning without holding a reference" This reverts commit 7d09e9725b5dcc8d14e101de931e4969d033a6ad which is commit 57fcd550eb15bce14a7154736379dfd4ed60ae81 upstream. It is causing too many warnings on 5.11.y, so should be dropped for now. Cc: Huang Rui Cc: Christian König Cc: Daniel Vetter Cc: Christian Koenig Cc: Huang Rui Cc: Sasha Levin Reported-by: Oleksandr Natalenko Link: https://lore.kernel.org/r/8c3da8bc-0bf3-496f-1fd6-4f65a07b2...@amd.com Signed-off-by: Greg Kroah-Hartman -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 203905] amdgpu:actual_brightness has unreal/wrong value
https://bugzilla.kernel.org/show_bug.cgi?id=203905 --- Comment #27 from kurmi...@libero.it --- For me the only workaround to avoid this bug is disabling systemd backlight restoration. Then use brillo and enable a specific service to restore the brightness at startup. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 203905] amdgpu:actual_brightness has unreal/wrong value
https://bugzilla.kernel.org/show_bug.cgi?id=203905 --- Comment #26 from Paulo Nascimento (paulo.ul...@googlemail.com) --- laptop: Lenovo Legion 5, amd ryzen 5 4600h Graphics: Device-1: NVIDIA TU116M [GeForce GTX 1660 Ti Mobile] driver: nouveau Device-2: Advanced Micro Devices [AMD/ATI] Renoir driver: amdgpu Kernel 5.11.6-1 Manjaro 21.1 I have tried all suggested workarounds: - systemctl start systemd-backlight@backlight:amdgpu_bl0 - mkinitcpio with MODULES=(amdgpu) - acpi_backlight=vendor - acpi_backlight=video - /usr/lib/systemd/systemd-backlight save backlight:amdgpu_bl0 /usr/lib/systemd/systemd-backlight load backlight:amdgpu_bl0 Even echo 10 >/sys/class/backlight/amdgpu_bl0/brightness doesn't change the brightness. Any suggestions? -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On Thu, Mar 25, 2021 at 02:54:31PM +0100, Christian König wrote: > > The goal is to optimize large page size usage in the page tables. > > > > There are three critera that impact this: > > 1) The possible CPU page table sizes > > 2) The useful contiguity the device can create in its iomemory > > 3) The VA's alignment, as this sets an upper bound on 1 and 2 > > > > If a device has 256k pages and the arch supports 2M and 4k then the VA > > should align to somewhere between 4k and 256k. The ideal alignment > > would be to optimize PTE usage when stuffing 256k blocks by fully > > populating PTEs and depends on the arch's # of PTE's per page. > > Ah! So you want to also avoid that we only halve populate a PTEs as well! > That rather nifty. > > But you don't need the device page size for this. Just looking at the size > of the mapping should be enough. Well, kind of, at a certain point we start to over-align things which is a bit harmful too, it is best to cap it at what the device could actually use, IMHO. Keep in mind address space is not free, and 32 bit in particular needs to be efficient. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
Am 25.03.21 um 14:33 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 02:26:50PM +0100, Christian König wrote: Am 25.03.21 um 14:17 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 02:05:14PM +0100, Christian König wrote: Am 25.03.21 um 13:42 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 01:09:14PM +0100, Christian König wrote: Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) wrote: Nope. The point here was that in this case, to make sure mmap uses the correct VA to give us a reasonable chance of alignement, the driver might need to be aware of and do trickery with the huge page-table-entry sizes anyway, although I think in most cases a standard helper for this can be supplied. Of course the driver needs some way to influence the VA mmap uses, gernally it should align to the natural page size of the device Well a mmap() needs to be aligned to the page size of the CPU, but not necessarily to the one of the device. So I'm pretty sure the device driver should not be involved in any way the choosing of the VA for the CPU mapping. No, if the device wants to use huge pages it must influence the mmap VA or it can't form huge pgaes. No, that's the job of the core MM and not of the individual driver. The core mm doesn't know the page size of the device, only which of several page levels the arch supports. The device must be involevd here. Why? See you can have a device which has for example 256KiB pages, but it should perfectly work that the CPU mapping is aligned to only 4KiB. The goal is to optimize large page size usage in the page tables. There are three critera that impact this: 1) The possible CPU page table sizes 2) The useful contiguity the device can create in its iomemory 3) The VA's alignment, as this sets an upper bound on 1 and 2 If a device has 256k pages and the arch supports 2M and 4k then the VA should align to somewhere between 4k and 256k. The ideal alignment would be to optimize PTE usage when stuffing 256k blocks by fully populating PTEs and depends on the arch's # of PTE's per page. Ah! So you want to also avoid that we only halve populate a PTEs as well! That rather nifty. But you don't need the device page size for this. Just looking at the size of the mapping should be enough. In other words we would align the VA so that it tries to avoid crossing page table boundaries. But to be honest I'm really wondering why the heck we don't already do this in vm_unmapped_area(). That should be beneficial for basically every slightly larger mapping. Christian. If a device has 256k pages and the arch supports 256k pages then the VA should align to 256k. The device should never be touching any of this, it should simply inform what its operating page size is and the MM should use that to align the VA. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2 0/2] HDCP 2.2 DP errata
On Wed, 24 Mar 2021, Anshuman Gupta wrote: > HDCP DP 2.2 errata is part of HDCP DP 2.3 specs > as well. > > Anshuman Gupta (2): > drm/i915/hdcp: Add DP HDCP2.2 timeout to read entire msg > drm/hdcp: DP HDCP2.2 errata LC_Send_L_Prime=16 > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 45 ++-- > include/drm/drm_hdcp.h | 5 ++- > 2 files changed, 36 insertions(+), 14 deletions(-) Maarten, Maxime, Thomas - Can I get an ack for merging this via drm-intel-next, please? BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On Thu, Mar 25, 2021 at 02:26:50PM +0100, Christian König wrote: > Am 25.03.21 um 14:17 schrieb Jason Gunthorpe: > > On Thu, Mar 25, 2021 at 02:05:14PM +0100, Christian König wrote: > > > > > > Am 25.03.21 um 13:42 schrieb Jason Gunthorpe: > > > > On Thu, Mar 25, 2021 at 01:09:14PM +0100, Christian König wrote: > > > > > Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: > > > > > > On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) > > > > > > wrote: > > > > > > > > > > > > > Nope. The point here was that in this case, to make sure mmap > > > > > > > uses the > > > > > > > correct VA to give us a reasonable chance of alignement, the > > > > > > > driver might > > > > > > > need to be aware of and do trickery with the huge > > > > > > > page-table-entry sizes > > > > > > > anyway, although I think in most cases a standard helper for this > > > > > > > can be > > > > > > > supplied. > > > > > > Of course the driver needs some way to influence the VA mmap uses, > > > > > > gernally it should align to the natural page size of the device > > > > > Well a mmap() needs to be aligned to the page size of the CPU, but not > > > > > necessarily to the one of the device. > > > > > > > > > > So I'm pretty sure the device driver should not be involved in any > > > > > way the > > > > > choosing of the VA for the CPU mapping. > > > > No, if the device wants to use huge pages it must influence the mmap > > > > VA or it can't form huge pgaes. > > > No, that's the job of the core MM and not of the individual driver. > > The core mm doesn't know the page size of the device, only which of > > several page levels the arch supports. The device must be involevd > > here. > > Why? See you can have a device which has for example 256KiB pages, but it > should perfectly work that the CPU mapping is aligned to only 4KiB. The goal is to optimize large page size usage in the page tables. There are three critera that impact this: 1) The possible CPU page table sizes 2) The useful contiguity the device can create in its iomemory 3) The VA's alignment, as this sets an upper bound on 1 and 2 If a device has 256k pages and the arch supports 2M and 4k then the VA should align to somewhere between 4k and 256k. The ideal alignment would be to optimize PTE usage when stuffing 256k blocks by fully populating PTEs and depends on the arch's # of PTE's per page. If a device has 256k pages and the arch supports 256k pages then the VA should align to 256k. The device should never be touching any of this, it should simply inform what its operating page size is and the MM should use that to align the VA. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
Hi, On 3/25/21 2:02 PM, Christian König wrote: Am 25.03.21 um 13:36 schrieb Thomas Hellström (Intel): On 3/25/21 1:09 PM, Christian König wrote: Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) wrote: Nope. The point here was that in this case, to make sure mmap uses the correct VA to give us a reasonable chance of alignement, the driver might need to be aware of and do trickery with the huge page-table-entry sizes anyway, although I think in most cases a standard helper for this can be supplied. Of course the driver needs some way to influence the VA mmap uses, gernally it should align to the natural page size of the device Well a mmap() needs to be aligned to the page size of the CPU, but not necessarily to the one of the device. So I'm pretty sure the device driver should not be involved in any way the choosing of the VA for the CPU mapping. Christian. We've had this discussion before and at that time I managed to convince you by pointing to the shmem helper for this, shmem_get_umapped_area(). No, you didn't convinced me. I was just surprised that this is something under driver control. Basically there are two ways to do this. Either use a standard helper similar to shmem's, and then the driver needs to align physical (device) huge page boundaries to address space offset huge page boundaries. If you don't do that you can just as well use a custom function that adjusts for you not doing that (drm_get_unmapped_area()). Both require driver knowledge of the size of huge pages. And once more, at least for GPU drivers that looks like the totally wrong approach to me. Aligning the VMA so that huge page allocations become possible is the job of the MM subsystem and not that of the drivers. Previous discussion here https://www.spinics.net/lists/linux-mm/msg205291.html Without a function to adjust, mmap will use it's default (16 byte?) alignment and chance of alignment becomes very small. Well it's 4KiB at least. Yes :/ ... Regards, Christian. Thanks, Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: WARNING: AMDGPU DRM warning in 5.11.9
Hello. On Thu, Mar 25, 2021 at 07:57:33AM +0200, Ilkka Prusi wrote: > On 24.3.2021 16.16, Chris Rankin wrote: > > Hi, > > > > Theee warnings ares not present in my dmesg log from 5.11.8: > > > > [ 43.390159] [ cut here ] > > [ 43.393574] WARNING: CPU: 2 PID: 1268 at > > drivers/gpu/drm/ttm/ttm_bo.c:517 ttm_bo_release+0x172/0x282 [ttm] > > [ 43.401940] Modules linked in: nf_nat_ftp nf_conntrack_ftp cfg80211 > > Changing WARN_ON to WARN_ON_ONCE in drivers/gpu/drm/ttm/ttm_bo.c > ttm_bo_release() reduces the flood of messages into single splat. > > This warning appears to come from 57fcd550eb15bce ("drm/ttm: Warn on pinning > without holding a reference)" and reverting it might be one choice. > > > > > > There are others, but I am assuming there is a common cause here. > > > > Cheers, > > Chris > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index a76eb2c14e8c..50b53355b265 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -514,7 +514,7 @@ static void ttm_bo_release(struct kref *kref) > * shrinkers, now that they are queued for > * destruction. > */ > - if (WARN_ON(bo->pin_count)) { > + if (WARN_ON_ONCE(bo->pin_count)) { > bo->pin_count = 0; > ttm_bo_del_from_lru(bo); > ttm_bo_add_mem_to_lru(bo, >mem); > > > > -- > - Ilkka > WARN_ON_ONCE() will just hide the underlying problem. Do we know why this happens at all? Same for me, BTW, with v5.11.9: ``` [~]> lspci | grep VGA 0a:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Lexa PRO [Radeon 540/540X/550/550X / RX 540X/550/550X] (rev c7) [ 3676.033140] [ cut here ] [ 3676.033153] WARNING: CPU: 7 PID: 1318 at drivers/gpu/drm/ttm/ttm_bo.c:517 ttm_bo_release+0x375/0x500 [ttm] … [ 3676.033340] Hardware name: ASUS System Product Name/Pro WS X570-ACE, BIOS 3302 03/05/2021 … [ 3676.033469] Call Trace: [ 3676.033473] ttm_bo_move_accel_cleanup+0x1ab/0x3a0 [ttm] [ 3676.033478] amdgpu_bo_move+0x334/0x860 [amdgpu] [ 3676.033580] ttm_bo_validate+0x1f1/0x2d0 [ttm] [ 3676.033585] amdgpu_cs_bo_validate+0x9b/0x1c0 [amdgpu] [ 3676.033665] amdgpu_cs_list_validate+0x115/0x150 [amdgpu] [ 3676.033743] amdgpu_cs_ioctl+0x873/0x20a0 [amdgpu] [ 3676.033960] drm_ioctl_kernel+0xb8/0x140 [drm] [ 3676.033977] drm_ioctl+0x222/0x3c0 [drm] [ 3676.034071] amdgpu_drm_ioctl+0x49/0x80 [amdgpu] [ 3676.034145] __x64_sys_ioctl+0x83/0xb0 [ 3676.034149] do_syscall_64+0x33/0x40 … [ 3676.034171] ---[ end trace 66e9865b027112f3 ]--- ``` Thanks. -- Oleksandr Natalenko (post-factum) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] video: hyperv_fb: Fix a double free in hvfb_probe
On Wed, Mar 24, 2021 at 01:46:39PM +, Michael Kelley wrote: > From: Lv Yunlong Sent: Wednesday, March 24, 2021 > 3:37 AM > > > > In function hvfb_probe in hyperv_fb.c, it calls hvfb_getmem(hdev, info) > > and return err when info->apertures is freed. > > > > In the error1 label of hvfb_probe, info->apertures will be freed for the > > second time in framebuffer_release(info). > > > > My patch removes all kfree(info->apertures) instead of set info->apertures > > to NULL. It is because that let framebuffer_release() handle freeing the > > memory flows the fbdev pattern, and less code overall. > > Let me suggest some clarifications in the commit message. It's probably > better not to reference the initial approach of setting info->apertures to > NULL, since there won't be any record of that approach in the commit > history. Here's what I would suggest: > > Function hvfb_probe() calls hvfb_getmem(), expecting upon return that > info->apertures is either NULL or points to memory that should be freed > by framebuffer_release(). But hvfb_getmem() is freeing the memory and > leaving the pointer non-NULL, resulting in a double free if an error > occurs or later if hvfb_remove() is called. > > Fix this by removing all kfree(info->apertures) calls in hvfb_getmem(). > This will allow framebuffer_release() to free the memory, which follows > the pattern of other fbdev drivers. > > Modulo this revision to the commit message, which Wei Liu can > probably incorporate, > Yes. I surely can incorporate the changes. I will also add the Fixes tag. > Reviewed-by: Michael Kelley ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drivers: gpu: drm: xen_drm_front_drm_info is declared twice
Hi, Daniel! On 3/25/21 11:16 AM, Daniel Vetter wrote: > On Thu, Mar 25, 2021 at 7:53 AM Oleksandr Andrushchenko > wrote: >> Hi, >> >> On 3/25/21 8:19 AM, Wan Jiabing wrote: >>> struct xen_drm_front_drm_info has been declared. >>> Remove the duplicate. >>> >>> Signed-off-by: Wan Jiabing >> Thank you for the patch, >> >> Reviewed-by: Oleksandr Andrushchenko >> >> Will apply to drm-misc-next-fixes > drm-misc-next-fixes is the wrong tree, bugfixes outside of the merge > window belong into drm-misc-fixes. Please consult > > https://urldefense.com/v3/__https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html*where-do-i-apply-my-patch__;Iw!!GF_29dbcQIUBPA!nfKNXrB-yHqaxeH6nC3mEw28HFFI1p5fc5CZKEFeoQPWXEhZCpvMqvW8EtFfTqtHPiNgpY4S-g$ > [drm[.]pages[.]freedesktop[.]org] > > We need to hard-reset drm-misc-next-fixes back, please re-apply the > patches (both of them) to drm-misc-fixes. Also adding drm-misc > maintainers. Sorry for screwing things up, will re-apply both patches to drm-misc-fixes > -Daniel > >> Thank you, >> >> Oleksandr >> >>> --- >>>drivers/gpu/drm/xen/xen_drm_front_conn.h | 1 - >>>1 file changed, 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_conn.h >>> b/drivers/gpu/drm/xen/xen_drm_front_conn.h >>> index 3adacba9a23b..e5f4314899ee 100644 >>> --- a/drivers/gpu/drm/xen/xen_drm_front_conn.h >>> +++ b/drivers/gpu/drm/xen/xen_drm_front_conn.h >>> @@ -16,7 +16,6 @@ >>>struct drm_connector; >>>struct xen_drm_front_drm_info; >>> >>> -struct xen_drm_front_drm_info; >>> >>>int xen_drm_front_conn_init(struct xen_drm_front_drm_info *drm_info, >>>struct drm_connector *connector); > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
Am 25.03.21 um 14:17 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 02:05:14PM +0100, Christian König wrote: Am 25.03.21 um 13:42 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 01:09:14PM +0100, Christian König wrote: Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) wrote: Nope. The point here was that in this case, to make sure mmap uses the correct VA to give us a reasonable chance of alignement, the driver might need to be aware of and do trickery with the huge page-table-entry sizes anyway, although I think in most cases a standard helper for this can be supplied. Of course the driver needs some way to influence the VA mmap uses, gernally it should align to the natural page size of the device Well a mmap() needs to be aligned to the page size of the CPU, but not necessarily to the one of the device. So I'm pretty sure the device driver should not be involved in any way the choosing of the VA for the CPU mapping. No, if the device wants to use huge pages it must influence the mmap VA or it can't form huge pgaes. No, that's the job of the core MM and not of the individual driver. The core mm doesn't know the page size of the device, only which of several page levels the arch supports. The device must be involevd here. Why? See you can have a device which has for example 256KiB pages, but it should perfectly work that the CPU mapping is aligned to only 4KiB. As long as you don't do things like shared virtual memory between device and CPU the VA addresses used on the CPU should be completely irrelevant for the device. Regards, Christian. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On Thu, Mar 25, 2021 at 02:05:14PM +0100, Christian König wrote: > > > Am 25.03.21 um 13:42 schrieb Jason Gunthorpe: > > On Thu, Mar 25, 2021 at 01:09:14PM +0100, Christian König wrote: > > > Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: > > > > On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) > > > > wrote: > > > > > > > > > Nope. The point here was that in this case, to make sure mmap uses the > > > > > correct VA to give us a reasonable chance of alignement, the driver > > > > > might > > > > > need to be aware of and do trickery with the huge page-table-entry > > > > > sizes > > > > > anyway, although I think in most cases a standard helper for this can > > > > > be > > > > > supplied. > > > > Of course the driver needs some way to influence the VA mmap uses, > > > > gernally it should align to the natural page size of the device > > > Well a mmap() needs to be aligned to the page size of the CPU, but not > > > necessarily to the one of the device. > > > > > > So I'm pretty sure the device driver should not be involved in any way the > > > choosing of the VA for the CPU mapping. > > No, if the device wants to use huge pages it must influence the mmap > > VA or it can't form huge pgaes. > > No, that's the job of the core MM and not of the individual driver. The core mm doesn't know the page size of the device, only which of several page levels the arch supports. The device must be involevd here. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] MAINTAINERS: Add linux-mediatek ML for drm Mediatek drivers
Add the linux-mediatek mailing list to drm Mediatek drivers Signed-off-by: Dafna Hirschfeld --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9e876927c60d..8260bc5afe66 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5963,6 +5963,7 @@ DRM DRIVERS FOR MEDIATEK M: Chun-Kuang Hu M: Philipp Zabel L: dri-devel@lists.freedesktop.org +L: linux-media...@lists.infradead.org (moderated for non-subscribers) S: Supported F: Documentation/devicetree/bindings/display/mediatek/ F: drivers/gpu/drm/mediatek/ -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [v2] drivers: gpu: drm: Remove duplicate declaration
Hi Wan, Thank you for the patch. On Thu, Mar 25, 2021 at 07:10:24PM +0800, Wan Jiabing wrote: > struct dss_device has been declared. Remove the duplicate. > And sort these forward declarations alphabetically. > > Signed-off-by: Wan Jiabing Reviewed-by: Laurent Pinchart Tomi, I assume you'll handle this patch, please let me know if you don't plan to do so. > --- > Changelog: > v2: > - Sort forward declarations alphabetically. > --- > drivers/gpu/drm/omapdrm/dss/omapdss.h | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h > b/drivers/gpu/drm/omapdrm/dss/omapdss.h > index a40abeafd2e9..040d5a3e33d6 100644 > --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h > +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h > @@ -48,16 +48,15 @@ > #define DISPC_IRQ_ACBIAS_COUNT_STAT3 (1 << 29) > #define DISPC_IRQ_FRAMEDONE3 (1 << 30) > > -struct dss_device; > -struct omap_drm_private; > -struct omap_dss_device; > struct dispc_device; > +struct drm_connector; > struct dss_device; > struct dss_lcd_mgr_config; > +struct hdmi_avi_infoframe; > +struct omap_drm_private; > +struct omap_dss_device; > struct snd_aes_iec958; > struct snd_cea_861_aud_if; > -struct hdmi_avi_infoframe; > -struct drm_connector; > > enum omap_display_type { > OMAP_DISPLAY_TYPE_NONE = 0, -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
Am 25.03.21 um 13:42 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 01:09:14PM +0100, Christian König wrote: Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) wrote: Nope. The point here was that in this case, to make sure mmap uses the correct VA to give us a reasonable chance of alignement, the driver might need to be aware of and do trickery with the huge page-table-entry sizes anyway, although I think in most cases a standard helper for this can be supplied. Of course the driver needs some way to influence the VA mmap uses, gernally it should align to the natural page size of the device Well a mmap() needs to be aligned to the page size of the CPU, but not necessarily to the one of the device. So I'm pretty sure the device driver should not be involved in any way the choosing of the VA for the CPU mapping. No, if the device wants to use huge pages it must influence the mmap VA or it can't form huge pgaes. No, that's the job of the core MM and not of the individual driver. In other words current->mm->get_unmapped_area should already return a properly aligned VA. Messing with that inside file->f_op->get_unmapped_area is utterly nonsense as far as I can see. It happens to be this way currently, but that is not even remotely good design. Christian. It is the same reason why mmap returns 2M stuff these days to make THP possible Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
Am 25.03.21 um 13:36 schrieb Thomas Hellström (Intel): On 3/25/21 1:09 PM, Christian König wrote: Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) wrote: Nope. The point here was that in this case, to make sure mmap uses the correct VA to give us a reasonable chance of alignement, the driver might need to be aware of and do trickery with the huge page-table-entry sizes anyway, although I think in most cases a standard helper for this can be supplied. Of course the driver needs some way to influence the VA mmap uses, gernally it should align to the natural page size of the device Well a mmap() needs to be aligned to the page size of the CPU, but not necessarily to the one of the device. So I'm pretty sure the device driver should not be involved in any way the choosing of the VA for the CPU mapping. Christian. We've had this discussion before and at that time I managed to convince you by pointing to the shmem helper for this, shmem_get_umapped_area(). No, you didn't convinced me. I was just surprised that this is something under driver control. Basically there are two ways to do this. Either use a standard helper similar to shmem's, and then the driver needs to align physical (device) huge page boundaries to address space offset huge page boundaries. If you don't do that you can just as well use a custom function that adjusts for you not doing that (drm_get_unmapped_area()). Both require driver knowledge of the size of huge pages. And once more, at least for GPU drivers that looks like the totally wrong approach to me. Aligning the VMA so that huge page allocations become possible is the job of the MM subsystem and not that of the drivers. Without a function to adjust, mmap will use it's default (16 byte?) alignment and chance of alignment becomes very small. Well it's 4KiB at least. Regards, Christian. /Thomas Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/mediatek: Switch the hdmi bridge ops to the atomic versions
Hi, On 25.03.21 11:43, Enric Balletbo i Serra wrote: Hi Dafna, Thank you for your patch. It'd be nice if you can cc the linux-mediatek ML for next version, so Mediatek people is more aware of this change. IMHO cc'ing the lkml is also a good practice. ok, I added all the mails from get_maintainer.pl. linux-mediatek was not there, I can add it. On 24/3/21 20:12, Dafna Hirschfeld wrote: The bridge operation '.enable' and the audio cb '.get_eld' access hdmi->conn. In the future we will want to support the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR and then we will not have direct access to the connector. The atomic version '.atomic_enable' allows accessing the current connector from the state. This patch switches the bridge to the atomic version and saves the current connector in a new field 'curr_conn'. Signed-off-by: Dafna Hirschfeld --- drivers/gpu/drm/mediatek/mtk_hdmi.c | 41 - 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 8ee55f9e2954..9f415c508b33 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c @@ -154,6 +154,7 @@ struct mtk_hdmi { struct drm_bridge bridge; struct drm_bridge *next_bridge; struct drm_connector conn; + struct drm_connector *curr_conn; Adding a new drm_connector (curr_conn) is something that surprised me at the beginning, then I saw that in the second patch you remove the first drm_connector (conn). I didn't test it yet, but did you test this patch alone? without the second one, maybe I'm missing something but looks to me that this patch alone breaks more functionalities of the driver? (and I know that the driver is broken right now) Hi, I didn't test it alone, but indeed this patch was wirrten with the intention that 'conn' will be removed next patch. But I don't think that patch should break functionality. Is really needed to create a new drm_connector to switch to the atomic versions? No, indeed this is the wrong patch to add curr_conn, I should add curr_conn to the second patch or maybe to a seperate last patch so not to overload the patch. Thanks, Dafna struct device *dev; const struct mtk_hdmi_conf *conf; struct phy *phy; @@ -974,7 +975,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi, ssize_t err; err = drm_hdmi_avi_infoframe_from_display_mode(, - >conn, mode); + hdmi->curr_conn, mode); if (err < 0) { dev_err(hdmi->dev, "Failed to get AVI infoframe from mode: %zd\n", err); @@ -1054,7 +1055,7 @@ static int mtk_hdmi_setup_vendor_specific_infoframe(struct mtk_hdmi *hdmi, ssize_t err; err = drm_hdmi_vendor_infoframe_from_display_mode(, - >conn, mode); + hdmi->curr_conn, mode); if (err) { dev_err(hdmi->dev, "Failed to get vendor infoframe from mode: %zd\n", err); @@ -1357,7 +1358,8 @@ static bool mtk_hdmi_bridge_mode_fixup(struct drm_bridge *bridge, return true; } -static void mtk_hdmi_bridge_disable(struct drm_bridge *bridge) +static void mtk_hdmi_bridge_atomic_disable(struct drm_bridge *bridge, + struct drm_bridge_state *old_bridge_state) { struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge); @@ -1368,10 +1370,13 @@ static void mtk_hdmi_bridge_disable(struct drm_bridge *bridge) clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_HDMI_PIXEL]); clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_HDMI_PLL]); + hdmi->curr_conn = NULL; + hdmi->enabled = false; } -static void mtk_hdmi_bridge_post_disable(struct drm_bridge *bridge) +static void mtk_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge, + struct drm_bridge_state *old_state) { struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge); @@ -1406,7 +1411,8 @@ static void mtk_hdmi_bridge_mode_set(struct drm_bridge *bridge, drm_mode_copy(>mode, adjusted_mode); } -static void mtk_hdmi_bridge_pre_enable(struct drm_bridge *bridge) +static void mtk_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, + struct drm_bridge_state *old_state) { struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge); @@ -1426,10 +1432,16 @@ static void mtk_hdmi_send_infoframe(struct mtk_hdmi *hdmi, mtk_hdmi_setup_vendor_specific_infoframe(hdmi, mode); } -static void mtk_hdmi_bridge_enable(struct drm_bridge *bridge) +static void mtk_hdmi_bridge_atomic_enable(struct drm_bridge *bridge, +
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On Thu, Mar 25, 2021 at 01:09:14PM +0100, Christian König wrote: > Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: > > On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) wrote: > > > > > Nope. The point here was that in this case, to make sure mmap uses the > > > correct VA to give us a reasonable chance of alignement, the driver might > > > need to be aware of and do trickery with the huge page-table-entry sizes > > > anyway, although I think in most cases a standard helper for this can be > > > supplied. > > Of course the driver needs some way to influence the VA mmap uses, > > gernally it should align to the natural page size of the device > > Well a mmap() needs to be aligned to the page size of the CPU, but not > necessarily to the one of the device. > > So I'm pretty sure the device driver should not be involved in any way the > choosing of the VA for the CPU mapping. No, if the device wants to use huge pages it must influence the mmap VA or it can't form huge pgaes. It is the same reason why mmap returns 2M stuff these days to make THP possible Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On 3/25/21 1:09 PM, Christian König wrote: Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) wrote: Nope. The point here was that in this case, to make sure mmap uses the correct VA to give us a reasonable chance of alignement, the driver might need to be aware of and do trickery with the huge page-table-entry sizes anyway, although I think in most cases a standard helper for this can be supplied. Of course the driver needs some way to influence the VA mmap uses, gernally it should align to the natural page size of the device Well a mmap() needs to be aligned to the page size of the CPU, but not necessarily to the one of the device. So I'm pretty sure the device driver should not be involved in any way the choosing of the VA for the CPU mapping. Christian. We've had this discussion before and at that time I managed to convince you by pointing to the shmem helper for this, shmem_get_umapped_area(). Basically there are two ways to do this. Either use a standard helper similar to shmem's, and then the driver needs to align physical (device) huge page boundaries to address space offset huge page boundaries. If you don't do that you can just as well use a custom function that adjusts for you not doing that (drm_get_unmapped_area()). Both require driver knowledge of the size of huge pages. Without a function to adjust, mmap will use it's default (16 byte?) alignment and chance of alignment becomes very small. /Thomas Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/etnaviv: Remove useless error message
Fix the following coccicheck report: drivers/gpu/drm/etnaviv/etnaviv_gpu.c:1775:2-9: line 1775 is redundant because platform_get_irq() already prints an error Remove dev_err() messages after platform_get_irq() failures. Signed-off-by: Tian Tao Signed-off-by: Zihao Tang Signed-off-by: Jay Fang --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index c6404b8..4645349 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1771,10 +1771,8 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev) /* Get Interrupt: */ gpu->irq = platform_get_irq(pdev, 0); - if (gpu->irq < 0) { - dev_err(dev, "failed to get irq: %d\n", gpu->irq); + if (gpu->irq < 0) return gpu->irq; - } err = devm_request_irq(>dev, gpu->irq, irq_handler, 0, dev_name(gpu->dev), gpu); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
Am 25.03.21 um 13:01 schrieb Jason Gunthorpe: On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) wrote: Nope. The point here was that in this case, to make sure mmap uses the correct VA to give us a reasonable chance of alignement, the driver might need to be aware of and do trickery with the huge page-table-entry sizes anyway, although I think in most cases a standard helper for this can be supplied. Of course the driver needs some way to influence the VA mmap uses, gernally it should align to the natural page size of the device Well a mmap() needs to be aligned to the page size of the CPU, but not necessarily to the one of the device. So I'm pretty sure the device driver should not be involved in any way the choosing of the VA for the CPU mapping. Christian. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/2] mm,drm/ttm: Block fast GUP to TTM huge pages
On Thu, Mar 25, 2021 at 12:53:15PM +0100, Thomas Hellström (Intel) wrote: > Nope. The point here was that in this case, to make sure mmap uses the > correct VA to give us a reasonable chance of alignement, the driver might > need to be aware of and do trickery with the huge page-table-entry sizes > anyway, although I think in most cases a standard helper for this can be > supplied. Of course the driver needs some way to influence the VA mmap uses, gernally it should align to the natural page size of the device Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vkms: fix misuse of WARN_ON
On 03/25, Dmitry Vyukov wrote: > On Wed, Mar 24, 2021 at 11:00 PM Melissa Wen wrote: > > > > On 03/20, Dmitry Vyukov wrote: > > > vkms_vblank_simulate() uses WARN_ON for timing-dependent condition > > > (timer overrun). This is a mis-use of WARN_ON, WARN_ON must be used > > > to denote kernel bugs. Use pr_warn() instead. > > > > > > Signed-off-by: Dmitry Vyukov > > > Reported-by: syzbot+4fc21a003c8332eb0...@syzkaller.appspotmail.com > > > Cc: Rodrigo Siqueira > > > Cc: Melissa Wen > > > Cc: Haneen Mohammed > > > Cc: Daniel Vetter > > > Cc: David Airlie > > > Cc: dri-devel@lists.freedesktop.org > > > Cc: linux-ker...@vger.kernel.org > > > Change-Id: I7f01c288092bc7e472ec63af198f93ce3d8c49f7 > > > --- > > > drivers/gpu/drm/vkms/vkms_crtc.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c > > > b/drivers/gpu/drm/vkms/vkms_crtc.c > > > index 0443b7deeaef6..758d8a98d96b3 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > > > @@ -18,7 +18,8 @@ static enum hrtimer_restart vkms_vblank_simulate(struct > > > hrtimer *timer) > > > > > > ret_overrun = hrtimer_forward_now(>vblank_hrtimer, > > > output->period_ns); > > > - WARN_ON(ret_overrun != 1); > > > + if (ret_overrun != 1) > > > + pr_warn("%s: vblank timer overrun\n", __func__); > > > > Hi Dmitry, > > > > Thanks for your patch. > > > > Looks good to me. > > The Change-Id tag just seems a little noisy to me, but can be > > fixed while applying. > > Yes, please drop Change-Id when applying if possible. Sorry for that. > Thanks for the review, Melissa. Applied to drm-misc-next. Thanks, Melissa > > > Acked-by: Melissa Wen > > > > > > > > spin_lock(>lock); > > > ret = drm_crtc_handle_vblank(crtc); > > > > > > base-commit: e94c55b8e0a0bbe9a026250cf31e2fa45957d776 > > > -- > > > 2.31.0.291.g576ba9dcdaf-goog > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel