[Bug 80531] 3.16-rc2 hdmi output resolution out of range Cape Verde PRO [Radeon HD 7750]
https://bugs.freedesktop.org/show_bug.cgi?id=80531 Martin Peres changed: What|Removed |Added Resolution|--- |MOVED Status|NEW |RESOLVED --- Comment #14 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/507. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 75992] Display freezes & corruption with an r7 260x on 3.14-rc6
https://bugs.freedesktop.org/show_bug.cgi?id=75992 Martin Peres changed: What|Removed |Added Resolution|--- |MOVED Status|NEW |RESOLVED --- Comment #60 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/460. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107296] WARNING: CPU: 0 PID: 370 at drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:1355 dcn_bw_update_from_pplib+0x16b/0x280 [amdgpu]
https://bugs.freedesktop.org/show_bug.cgi?id=107296 Martin Peres changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |MOVED --- Comment #25 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/963. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108521] RX 580 / Vega56 as eGPU amdgpu: gpu post error!
https://bugs.freedesktop.org/show_bug.cgi?id=108521 Martin Peres changed: What|Removed |Added Resolution|--- |MOVED Status|REOPENED|RESOLVED --- Comment #57 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/566. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109955] amdgpu [RX Vega 64] system freeze while gaming (VSYNC enabled)
https://bugs.freedesktop.org/show_bug.cgi?id=109955 Martin Peres changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |MOVED --- Comment #137 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/716. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108934] [Dualscreen] "No signal" issues when booting with DVI and HDMI screens both plugged in
https://bugs.freedesktop.org/show_bug.cgi?id=108934 Martin Peres changed: What|Removed |Added Resolution|--- |MOVED Status|NEW |RESOLVED --- Comment #3 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/624. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/4] drm: imx: Add i.MX 6 MIPI DSI host driver
On Tue, 19 Nov 2019, Fabio Estevam wrote: Hi Adrian, Hi Fabio, On Mon, Nov 18, 2019 at 12:25 PM Adrian Ratiu wrote: Some nitpicks: + +config DRM_IMX_MIPI_DSI + tristate "Freescale i.MX DRM MIPI DSI" This text seems too generic as there are i.MX SoCs that use different MIPI DSI IP. Maybe "Freescale i.MX6 DRM MIPI DSI" instead? Yes, this is a good idea, will update in a newer version to make it more specific. I'll let this version sit a little more on review so others also have time to review. Thank you! +module_platform_driver(imx_mipi_dsi_driver); + +MODULE_DESCRIPTION("i.MX MIPI DSI host controller driver"); i.MX6 MIPI DSI, please. ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107465] amdgpu.dc=1 triggers graphic card not recognizing native screen resolution
https://bugs.freedesktop.org/show_bug.cgi?id=107465 Martin Peres changed: What|Removed |Added Resolution|--- |MOVED Status|NEW |RESOLVED --- Comment #4 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/471. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107456] WARNING: CPU: 11 PID: 1137 at drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:88 dm_dp_aux_transfer+0xa5/0xc0 [amdgpu]
https://bugs.freedesktop.org/show_bug.cgi?id=107456 Martin Peres changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |MOVED --- Comment #2 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/470. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107376] Raven: `flush_delayed_work()` takes 500 ms
https://bugs.freedesktop.org/show_bug.cgi?id=107376 Martin Peres changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |MOVED --- Comment #1 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/464. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107278] Raven: pci_pm_resume takes over 1 second
https://bugs.freedesktop.org/show_bug.cgi?id=107278 Bug 107278 depends on bug 107376, which changed state. Bug 107376 Summary: Raven: `flush_delayed_work()` takes 500 ms https://bugs.freedesktop.org/show_bug.cgi?id=107376 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |MOVED -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105433] Unreliable Modesetting on Tonga with two DVI Screens
https://bugs.freedesktop.org/show_bug.cgi?id=105433 Martin Peres changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |MOVED --- Comment #12 from Martin Peres --- -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/320. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/crc-debugfs: fix crtc_crc_poll()'s return type
crtc_crc_poll() is defined as returning 'unsigned int' but the .poll method is declared as returning '__poll_t', a bitwise type. Fix this by using the proper return type and using the EPOLL constants instead of the POLL ones, as required for __poll_t. CC: Maarten Lankhorst CC: Maxime Ripard CC: Sean Paul CC: David Airlie CC: Daniel Vetter CC: dri-devel@lists.freedesktop.org Signed-off-by: Luc Van Oostenryck --- drivers/gpu/drm/drm_debugfs_crc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c index be1b7ba92ffe..0bb0aa0ebbca 100644 --- a/drivers/gpu/drm/drm_debugfs_crc.c +++ b/drivers/gpu/drm/drm_debugfs_crc.c @@ -334,17 +334,17 @@ static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf, return LINE_LEN(crc->values_cnt); } -static unsigned int crtc_crc_poll(struct file *file, poll_table *wait) +static __poll_t crtc_crc_poll(struct file *file, poll_table *wait) { struct drm_crtc *crtc = file->f_inode->i_private; struct drm_crtc_crc *crc = >crc; - unsigned ret; + __poll_t ret; poll_wait(file, >wq, wait); spin_lock_irq(>lock); if (crc->source && crtc_crc_data_count(crc)) - ret = POLLIN | POLLRDNORM; + ret = EPOLLIN | EPOLLRDNORM; else ret = 0; spin_unlock_irq(>lock); -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] video: hyperv_fb: Fix hibernation for the deferred IO feature
fb_deferred_io_work() can access the vmbus ringbuffer by calling fbdefio->deferred_io() -> synthvid_deferred_io() -> synthvid_update(). Because the vmbus ringbuffer is inaccessible between hvfb_suspend() and hvfb_resume(), we must cancel info->deferred_work before calling vmbus_close() and then reschedule it after we reopen the channel in hvfb_resume(). Fixes: a4ddb11d297e ("video: hyperv: hyperv_fb: Support deferred IO for Hyper-V frame buffer driver") Fixes: 824946a8b6fb ("video: hyperv_fb: Add the support of hibernation") Signed-off-by: Dexuan Cui --- This patch fixes the 2 aforementioned patches on Sasha Levin's Hyper-V tree's hyperv-next branch: https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git/log/?h=hyperv-next The 2 aforementioned patches have not appeared in the mainline yet, so please pick up this patch onto he same hyperv-next branch. drivers/video/fbdev/hyperv_fb.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index 4cd27e5172a1..08bc0dfb5ce7 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -1194,6 +1194,7 @@ static int hvfb_suspend(struct hv_device *hdev) fb_set_suspend(info, 1); cancel_delayed_work_sync(>dwork); + cancel_delayed_work_sync(>deferred_work); par->update_saved = par->update; par->update = false; @@ -1227,6 +1228,7 @@ static int hvfb_resume(struct hv_device *hdev) par->fb_ready = true; par->update = par->update_saved; + schedule_delayed_work(>deferred_work, info->fbdefio->delay); schedule_delayed_work(>dwork, HVFB_UPDATE_DELAY); /* 0 means do resume */ -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFCv1 33/42] drm/omap: dsi: use atomic helper for dirtyfb
* Tomi Valkeinen [191119 15:56]: > Afaik, Weston and X both handle page flips and/or dirtying the fb, so they > should work. Are there applications that do not work, and cannot be made to > work, except the few SGX test apps? I'm not sure sure yet what all it affects, I'll do some more tests on it. Regards, Tony ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 17/24] mm/gup: track FOLL_PIN pages
On 11/19/19 3:37 AM, Jan Kara wrote: > On Tue 19-11-19 00:16:36, John Hubbard wrote: >> @@ -2025,6 +2149,20 @@ static int __record_subpages(struct page *page, >> unsigned long addr, >> return nr; >> } >> >> +static bool __pin_compound_head(struct page *head, int refs, unsigned int >> flags) >> +{ > > I don't quite like the proliferation of names starting with __. I don't > think there's a good reason for that, particularly in this case. Also 'pin' > here is somewhat misleading as we already use term "pin" for the particular > way of pinning the page. We could have grab_compound_head() or maybe > nail_compound_head() :), but you're native speaker so you may come up with > better word. Yes, it is ugly naming, I'll change these as follows: __pin_compound_head() --> grab_compound_head() __record_subpages() --> record_subpages() I loved the "nail_compound_head()" suggestion, it just seems very vivid, but in the end, I figured I'd better keep it relatively drab and colorless. :) > >> +if (flags & FOLL_PIN) { >> +if (unlikely(!try_pin_compound_head(head, refs))) >> +return false; >> +} else { >> +head = try_get_compound_head(head, refs); >> +if (!head) >> +return false; >> +} >> + >> +return true; >> +} >> + >> static void put_compound_head(struct page *page, int refs) >> { >> /* Do a get_page() first, in case refs == page->_refcount */ > > put_compound_head() needs similar treatment as undo_dev_pagemap(), doesn't > it? > Yes, will fix that up. >> @@ -968,7 +973,18 @@ struct page *follow_devmap_pmd(struct vm_area_struct >> *vma, unsigned long addr, >> if (!*pgmap) >> return ERR_PTR(-EFAULT); >> page = pfn_to_page(pfn); >> -get_page(page); >> + >> +if (flags & FOLL_GET) >> +get_page(page); >> +else if (flags & FOLL_PIN) { >> +/* >> + * try_pin_page() is not actually expected to fail here because >> + * we hold the pmd lock so no one can unmap the pmd and free the >> + * page that it points to. >> + */ >> +if (unlikely(!try_pin_page(page))) >> +page = ERR_PTR(-EFAULT); >> +} > > This pattern is rather common. So maybe I'd add a helper grab_page(page, > flags) doing > > if (flags & FOLL_GET) > get_page(page); > else if (flags & FOLL_PIN) > return try_pin_page(page); > return true; > OK. > Otherwise the patch looks good to me now. > > Honza Great! I thought I'd have a v7 out today, but fate decided to have me repair my test machine instead. So, soon. ha. :) thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 15/24] fs/io_uring: set FOLL_PIN via pin_user_pages()
On 11/19/19 8:10 AM, Jens Axboe wrote: > On 11/19/19 1:16 AM, John Hubbard wrote: >> Convert fs/io_uring to use the new pin_user_pages() call, which sets >> FOLL_PIN. Setting FOLL_PIN is now required for code that requires >> tracking of pinned pages, and therefore for any code that calls >> put_user_page(). >> >> In partial anticipation of this work, the io_uring code was already >> calling put_user_page() instead of put_page(). Therefore, in order to >> convert from the get_user_pages()/put_page() model, to the >> pin_user_pages()/put_user_page() model, the only change required >> here is to change get_user_pages() to pin_user_pages(). >> >> Reviewed-by: Jan Kara >> Signed-off-by: John Hubbard > > You dropped my reviewed-by now... Given the file, you'd probably want > to keep that. Hi Jens, Yes, I was being too conservative I guess. I changed the patch somewhat and dropped the reviewed-by because of those changes...I'm adding it back for v7 based on this, thanks! thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/9] drm/nouveau: Various fixes for GP10B
On Mon, Nov 18, 2019 at 11:45 PM Thierry Reding wrote: > > On Sat, Nov 02, 2019 at 06:56:28PM +0100, Thierry Reding wrote: > > From: Thierry Reding > > > > Hi Ben, > > > > here's a revised subset of the patches I had sent out a couple of weeks > > ago. I've reworked the BAR2 accesses in the way that you had suggested, > > which at least for GP10B turned out to be fairly trivial to do. I have > > not looked in detail at this for GV11B yet, but a cursory look showed > > that BAR2 is accessed in more places, so the equivalent for GV11B might > > be a bit more involved. > > > > Other than that, not a lot has changed since then. I've added a couple > > of precursory patches to add IOMMU helper dummies for the case where > > IOMMU is disabled (as suggested by Ben Dooks). > > > > Joerg, it'd be great if you could give an Acked-by on those two patches > > so that Ben can pick them both up into the Nouveau tree. Alternatively I > > can put them both into a stable branch and send a pull request to both > > of you. Or yet another alternative would be for Joerg to apply them now > > and Ben to wait for v5.5-rc1 until he picks up the rest. All of those > > work for me. > > Hi Joerg, Ben, > > do you guys have any further comments on this series? I've got an > updated patch to silence the warning that the kbuild robot flagged, so > if there are no other comments I can send a final v3 of the series. I'm on leave at the moment. But the nouveau fixes look fine to me and I'm happy to pick them up once you send a v3, and allow Jeorg to apply the rest through his tree. Thanks, Ben. > > Thierry ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V13 4/6] mdev: introduce mediated virtio bus
On 2019/11/19 下午10:14, Jason Gunthorpe wrote: On Tue, Nov 19, 2019 at 10:02:08PM +0800, Jason Wang wrote: On 2019/11/19 下午8:38, Jason Gunthorpe wrote: On Tue, Nov 19, 2019 at 10:41:31AM +0800, Jason Wang wrote: On 2019/11/19 上午4:28, Jason Gunthorpe wrote: On Mon, Nov 18, 2019 at 03:27:13PM -0500, Michael S. Tsirkin wrote: On Mon, Nov 18, 2019 at 01:41:00PM +, Jason Gunthorpe wrote: On Mon, Nov 18, 2019 at 06:59:21PM +0800, Jason Wang wrote: +struct bus_type mdev_virtio_bus_type; + +struct mdev_virtio_device { + struct mdev_device mdev; + const struct mdev_virtio_ops *ops; + u16 class_id; +}; This seems to share nothing with mdev (ie mdev-vfio), why is it on the same bus? I must be missing something - which bus do they share? mdev_bus_type ? Jason Note: virtio has its own bus: mdev_virtio_bus_type. So they are not the same bus. That is even worse, why involve struct mdev_device at all then? Jason I don't quite get the question here. In the driver model the bus_type and foo_device are closely linked. I don't get the definition of "closely linked" here. Do you think the bus and device implement virtual bus series are closely linked? If yes, how did they achieve that? Creating 'mdev_device' instances and overriding the bus_type is a very abusive thing to do. Ok, mdev_device (without this series) had: struct mdev_device { struct device dev; struct mdev_parent *parent; guid_t uuid; void *driver_data; struct list_head next; struct kobject *type_kobj; struct device *iommu_device; bool active; }; So it's nothing bus or VFIO specific. And what virtual bus had is: struct virtbus_device { const char *name; int id; const struct virtbus_dev_id *dev_id; struct device dev; void *data; }; Are there any fundamental issues that you think mdev_device is abused? I won't expect the answers are generic objects like kobj, iommu device pointer etc. My understanding for mdev is that it was a mediator between the driver and physical device when it's hard to let them talk directly due to the complexity of refactoring and maintenance. Really, mdev is to support vfio with a backend other than PCI, nothing more. That partially explain why it was called mdev. So for virito, we want standard virtio driver to talk with a backend other than virtio. For the issue of PCI, actually the API is generic enough to support device other than PCI, e.g AP bus. Abusing it for other things is not appropriate. ie creating an instance and not filling in most of the vfio focused ops is an abusive thing to do. Well, it's only half of the mdev_parent_ops in mdev_parent, various methods could be done do be more generic to avoid duplication of codes. No? hardware that can offload virtio datapath but not control path. We want to present a unified interface (standard virtio) instead of a vendor specific interface, so a mediator level in the middle is a must. For virtio driver, mediator present a full virtio compatible device. For hardware, mediator will mediate the difference between the behavior defined by virtio spec and real hardware. If you need to bind to the VFIO driver then mdev is the right thing to use, otherwise it is not. It certainly should not be used to bind to random kernel drivers. This problem is what this virtual bus idea Intel is working on might solve. What do you mean by random here? With this series, we have dedicated bus and dedicated driver with matching method to make sure the binding is correct. It seems the only thing people care about with mdev is the GUID lifecycle stuff, but at the same time folks like Parav are saying they don't want to use that lifecycle stuff and prefer devlink instead. I'm sure you will need to handle other issues besides GUID which had been settled by mdev e.g IOMMU and types when starting to write a real hardware driver. Most likely, at least for virtio-net, everyone else will be able to use devlink as well, making it much less clear if that GUID lifecycle stuff is a good idea or not. This assumption is wrong, we hard already had at least two concrete examples of vDPA device that doesn't use devlink: - Intel IFC where virtio is done at VF level - Ali Cloud ECS instance where virtio is done at PF level Again, the device slicing is only part of our goal. The major goal is to have a mediator level that can take over the virtio control path between a standard virtio driver and a hardware who datapath is virtio compatible but not control path. Thanks Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/msm: Don't init ww_mutec acquire ctx before needed
On Tue, Nov 19, 2019 at 1:08 PM Daniel Vetter wrote: > > For locking semantics it really doesn't matter when we grab the > ticket. But for lockdep validation it does: the acquire ctx is a fake > lockdep. Since other drivers might want to do a full multi-lock dance > in their fault-handler, not just lock a single dma_resv. Therefore we > must init the acquire_ctx only after we've done all the copy_*_user or > anything else that might trigger a pagefault. For msm this means we > need to move it past submit_lookup_objects. seems reasonable.. but maybe a comment would be useful to prevent future-me from "cleaning-this-up" back to the way it was with that, r-b > > Aside: Why is msm still using struct_mutex, it seems to be using > dma_resv_lock for general buffer state protection? only because I (or anyone else) hasn't had time to revisit the struct_mutex usage since we moved to per-object-locks.. the downside, I suppose, of kernel generally working ok and this not being a big enough fire to bubble up to the top of my todo list BR, -R > > Signed-off-by: Daniel Vetter > Cc: Rob Clark > Cc: Sean Paul > Cc: linux-arm-...@vger.kernel.org > Cc: freedr...@lists.freedesktop.org > --- > drivers/gpu/drm/msm/msm_gem_submit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > b/drivers/gpu/drm/msm/msm_gem_submit.c > index fb1fdd7fa3ef..126b2f62bfe7 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -54,7 +54,6 @@ static struct msm_gem_submit *submit_create(struct > drm_device *dev, > > INIT_LIST_HEAD(>node); > INIT_LIST_HEAD(>bo_list); > - ww_acquire_init(>ticket, _ww_class); > > return submit; > } > @@ -489,6 +488,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > *data, > if (ret) > goto out; > > + ww_acquire_init(>ticket, _ww_class); > ret = submit_lock_objects(submit); > if (ret) > goto out; > -- > 2.24.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: power: Fix path to power-domain.txt bindings
On Wed, 20 Nov 2019 at 01:02, Rob Herring wrote: > > On Tue, Nov 19, 2019 at 8:43 AM Krzysztof Kozlowski wrote: > > > > With split of power domain controller bindings to power-domain.yaml, the > > consumer part was renamed to power-domain.txt. Update the references in > > other bindings. > > > > Reported-by: Geert Uytterhoeven > > Fixes: abb4805e343a ("dt-bindings: power: Convert Samsung Exynos Power > > Domain bindings to json-schema") > > Signed-off-by: Krzysztof Kozlowski > > --- > > Documentation/devicetree/bindings/clock/clk-exynos-audss.txt | 2 +- > > Documentation/devicetree/bindings/clock/exynos5433-clock.txt | 2 +- > > .../devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt | 2 +- > > .../devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt | 2 +- > > .../bindings/clock/renesas,rcar-gen2-cpg-clocks.txt | 2 +- > > .../devicetree/bindings/clock/renesas,rz-cpg-clocks.txt | 2 +- > > .../devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 2 +- > > Documentation/devicetree/bindings/display/msm/dpu.txt | 2 +- > > Documentation/devicetree/bindings/display/msm/mdp5.txt| 2 +- > > Documentation/devicetree/bindings/dsp/fsl,dsp.yaml| 2 +- > > Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt| 2 +- > > .../devicetree/bindings/media/mediatek-jpeg-decoder.txt | 2 +- > > Documentation/devicetree/bindings/media/mediatek-mdp.txt | 2 +- > > Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 2 +- > > Documentation/devicetree/bindings/pci/pci-keystone.txt| 2 +- > > Documentation/devicetree/bindings/phy/ti,phy-am654-serdes.txt | 2 +- > > Documentation/devicetree/bindings/power/qcom,rpmpd.txt| 2 +- > > Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt | 2 +- > > .../devicetree/bindings/usb/nvidia,tegra124-xusb.txt | 4 ++-- > > 19 files changed, 20 insertions(+), 20 deletions(-) > > Please no. Can you just undo the renaming back to power_domain.txt The renaming was done to make it consistent with yaml and other bindings but indeed it creates some churn... I'll send rename-undo then. Best regards, Krzysztof ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205585] New: [Regression] [amdgpu] AMD Vega 64 GPU invalid access and EEH under load
https://bugzilla.kernel.org/show_bug.cgi?id=205585 Bug ID: 205585 Summary: [Regression] [amdgpu] AMD Vega 64 GPU invalid access and EEH under load Product: Drivers Version: 2.5 Kernel Version: 5.4-rc7 Hardware: PPC-64 OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: tpear...@raptorengineering.com Regression: No Created attachment 285977 --> https://bugzilla.kernel.org/attachment.cgi?id=285977=edit backtrace When using amdgpu with kernel 5.4-rc5 through 5.4-rc7, we are seeing invalid DMA under load with the Vega 64. This issue did not occur on 5.3 or earlier. The invalid DMA causes an EEH and knocks the GPU offline until a reboot. Trace attached. -- 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: [RFCv1 33/42] drm/omap: dsi: use atomic helper for dirtyfb
Hi, On Tue, Nov 19, 2019 at 07:46:28PM +0100, Andreas Kemnade wrote: > On Tue, 19 Nov 2019 17:55:57 +0200 > Tomi Valkeinen wrote: > > > On 19/11/2019 17:06, Tony Lindgren wrote: > > > > >> The userspace apps need to do this. If they're using single-buffering, > > >> then > > >> using the dirtyfb ioctl should do the trick, after the SGX has finished > > >> drawing. > > > > > > Sounds like that's not happening. > > > > > > But why would the userspace app need to know this might be needed for > > > a DSI command mode display and that it's not needed for HDMI? > > > > When double-buffering, the userspace doesn't need to care, as the > > page-flip ioctl explicitly tells that the buffer is ready. > > > > When single buffering, either the userspace has to tell that the buffer > > is now ready, or the kernel has to guess based on something. But afaics, > > the latter is always a guess, and may not be a good guess. > > > > The kernel could trigger a delayed update based on, say, page fault if > > drawing with CPU. But how much delayed... And with this scenario, we > > would somehow need to find a way to catch the writes from any IP to the > > buffer, and afaik there's no such thing. > > > > >> It's probably somewhat difficult if EGL is controlling the display, as, > > >> afaik, SGX EGL is closed source, and that's probably where it should be > > >> done. > > >> > > >> But adding back the hacky omap gem sync stuff, and then somehow guessing > > >> from the sync finish that some display needs to be updated... It just > > >> does > > >> not sound right to me. > > > > > > Right it's ugly. Still sounds like we need something in the kernel > > > that knows "this is a DSI command mode LCD and needs to be updated". > > > well, if we look broader around and not only at the remaining displays > to be converted from omapdrm to generic and look at more displays, there > are also EPDs. > > > I think one option is to refresh the command mode display all the time. > > Either using a timer, or trigger updates based on the previous update > > being finished. > > > > Of course, that's kind of against the whole point of manual update > > display, but then it should effectively behave like a conventional > > always-updating display (except your HW is more expensive and consumes > > more power than a conventional display). > > > And again as an extreme example about power consumption: EPDs. > So I guess we need a generic interface for userspace-triggered > refreshes anyways. And in that case that are only partly refreshes. You can do exactly this using the dirtyfb ioctl, which tells the kernel, that some parts of the framebuffer are "dirty" and should be send to the hardware. This is being used by the kernel console and X.org. For omapdrm this triggers a full refresh of DSI command mode panels. The second method (which only supports full framebuffer for obvious reasons) is double buffering. If you toggle from first to second framebuffer, the driver will send the framebuffer to hardware. This is being used by Weston when the DRM backend is selected. -- Sebastian signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFCv1 32/42] drm/omap: dsi: convert to drm_panel
Hi Sebastian, > Am 18.11.2019 um 15:51 schrieb H. Nikolaus Schaller : > >> Ok, I tried not to break video mode support, but I do not have any >> hardware. Make sure to set the MIPI_DSI_MODE_VIDEO flag in the panel >> driver. > > Indeed, this may be missing (can't look into the code at the moment)... > Or I did something wrong when refactoring the driver. > We will find out. Yes, MIPI_DSI_MODE_VIDEO was indeed missing/wrongly applied and some more bugs. But I still wasn't able to make it work. I also tried to fake the panel-orisetech-otm8009a.c DSI driver into my setup. It should not properly program the panel by DCS command but it should try to. Result is the same: I can see it being probed and calling get_modes but then: [drm] Cannot find any crtc or sizes And I don't see calls to .enable or .prepare where DCS commands would be sent. BR and thanks, Nikolaus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dpu: Mark various data tables as const
These structures look like a bunch of data tables that aren't going to change after boot. Let's move them to the const RO section of memory so that they can't be modified at runtime on modern machines. Signed-off-by: Stephen Boyd --- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 30 +-- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 26 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c| 8 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 8 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 10 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 2 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 8 ++--- .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 4 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 6 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 6 ++-- 14 files changed, 58 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 04c8c44f5b9c..dd096b6b7bfa 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -60,7 +60,7 @@ static const struct dpu_caps sdm845_dpu_caps = { .has_idle_pc = true, }; -static struct dpu_mdp_cfg sdm845_mdp[] = { +static const struct dpu_mdp_cfg sdm845_mdp[] = { { .name = "top_0", .id = MDP_TOP, .base = 0x0, .len = 0x45C, @@ -88,7 +88,7 @@ static struct dpu_mdp_cfg sdm845_mdp[] = { /* * CTL sub blocks config */ -static struct dpu_ctl_cfg sdm845_ctl[] = { +static const struct dpu_ctl_cfg sdm845_ctl[] = { { .name = "ctl_0", .id = CTL_0, .base = 0x1000, .len = 0xE4, @@ -184,7 +184,7 @@ static const struct dpu_sspp_sub_blks sdm845_dma_sblk_3 = _DMA_SBLK("11", 4); .clk_ctrl = _clkctrl \ } -static struct dpu_sspp_cfg sdm845_sspp[] = { +static const struct dpu_sspp_cfg sdm845_sspp[] = { SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SDM845_MASK, sdm845_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0), SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SDM845_MASK, @@ -225,7 +225,7 @@ static const struct dpu_lm_sub_blks sdm845_lm_sblk = { .lm_pair_mask = (1 << _lmpair) \ } -static struct dpu_lm_cfg sdm845_lm[] = { +static const struct dpu_lm_cfg sdm845_lm[] = { LM_BLK("lm_0", LM_0, 0x44000, PINGPONG_0, LM_1), LM_BLK("lm_1", LM_1, 0x45000, PINGPONG_1, LM_0), LM_BLK("lm_2", LM_2, 0x46000, PINGPONG_2, LM_5), @@ -264,7 +264,7 @@ static const struct dpu_pingpong_sub_blks sdm845_pp_sblk = { .sblk = _pp_sblk \ } -static struct dpu_pingpong_cfg sdm845_pp[] = { +static const struct dpu_pingpong_cfg sdm845_pp[] = { PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7), PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800), PP_BLK("pingpong_2", PINGPONG_2, 0x71000), @@ -283,7 +283,7 @@ static struct dpu_pingpong_cfg sdm845_pp[] = { .prog_fetch_lines_worst_case = 24 \ } -static struct dpu_intf_cfg sdm845_intf[] = { +static const struct dpu_intf_cfg sdm845_intf[] = { INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0), INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0), INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1), @@ -294,10 +294,10 @@ static struct dpu_intf_cfg sdm845_intf[] = { * VBIF sub blocks config */ /* VBIF QOS remap */ -static u32 sdm845_rt_pri_lvl[] = {3, 3, 4, 4, 5, 5, 6, 6}; -static u32 sdm845_nrt_pri_lvl[] = {3, 3, 3, 3, 3, 3, 3, 3}; +static const u32 sdm845_rt_pri_lvl[] = {3, 3, 4, 4, 5, 5, 6, 6}; +static const u32 sdm845_nrt_pri_lvl[] = {3, 3, 3, 3, 3, 3, 3, 3}; -static struct dpu_vbif_cfg sdm845_vbif[] = { +static const struct dpu_vbif_cfg sdm845_vbif[] = { { .name = "vbif_0", .id = VBIF_0, .base = 0, .len = 0x1040, @@ -316,7 +316,7 @@ static struct dpu_vbif_cfg sdm845_vbif[] = { }, }; -static struct dpu_reg_dma_cfg sdm845_regdma = { +static const struct dpu_reg_dma_cfg sdm845_regdma = { .base = 0x0, .version = 0x1, .trigger_sel_off = 0x119c }; @@ -325,7 +325,7 @@ static struct dpu_reg_dma_cfg sdm845_regdma = { */ /* SSPP QOS LUTs */ -static struct dpu_qos_lut_entry sdm845_qos_linear[] = { +static const struct dpu_qos_lut_entry sdm845_qos_linear[] = { {.fl = 4, .lut = 0x357}, {.fl = 5, .lut = 0x3357}, {.fl = 6, .lut = 0x23357}, @@ -340,7 +340,7 @@ static struct dpu_qos_lut_entry sdm845_qos_linear[] = { {.fl = 0, .lut =
Re: [RFCv1 11/42] ARM: dts: omap: add channel to DSI panels
Hi Tony, On Mon, Nov 18, 2019 at 02:52:09PM -0800, Tony Lindgren wrote: > * Sebastian Reichel [191118 15:03]: > > On Mon, Nov 18, 2019 at 03:37:12PM +0100, H. Nikolaus Schaller wrote: > > > > Am 18.11.2019 um 15:33 schrieb Sebastian Reichel > > > > : > > > > On Mon, Nov 18, 2019 at 03:05:07PM +0200, Tomi Valkeinen wrote: > > > >> On 17/11/2019 04:39, Sebastian Reichel wrote: > > > >>> The standard binding for DSI requires, that the channel number > > > >>> of the panel is encoded in DT. This adds the channel number in > > > >>> all OMAP3-5 boards, in preparation for using common infrastructure. > > > >>> > > > >>> Signed-off-by: Sebastian Reichel > > > >>> --- > > > >>> .../devicetree/bindings/display/panel/panel-dsi-cm.txt | 4 +++- > > > >>> arch/arm/boot/dts/omap3-n950.dts| 3 ++- > > > >>> arch/arm/boot/dts/omap3.dtsi| 3 +++ > > > >>> arch/arm/boot/dts/omap4-droid4-xt894.dts| 3 ++- > > > >>> arch/arm/boot/dts/omap4-sdp.dts | 6 > > > >>> -- > > > >>> arch/arm/boot/dts/omap4.dtsi| 6 > > > >>> ++ > > > >>> arch/arm/boot/dts/omap5.dtsi| 6 > > > >>> ++ > > > >>> 7 files changed, 26 insertions(+), 5 deletions(-) > > > >> > > > >> Is this required only in the .txt, or also by the driver? This does > > > >> break > > > >> backward compatibility with the dtbs, and there's always someone who > > > >> won't > > > >> like it. > > > > > > > > I add a compatible string for the Droid 4 panel in addition to the > > > > generic one, which is not really required and just a precaution in > > > > case we need some quirks in the future. > > > > > > > > But I had to add the DSI channel to DT, which is required to follow > > > > the standard DSI bindings. We cannot use the generic infrastructure > > > > without this change. Technically it should have been there all the > > > > time, it is only working because it is currently hardcoded to 0 in > > > > the panel driver. > > > > > > Is it possible to change it to default to channel <0> if reg is not > > > specified? > > > > Currently nodes without reg property are skipped by of_mipi_dsi_device_add() > > and of_mipi_dsi_device_add() fails if reg node is missing. Technically > > it should be possible to default to channel 0 there. That affects all > > platforms, though. Considering the small amount of boards affected, I think > > its better to just fix the DT. Also the fixed DT does not make problems > > with older kernels and can be backported. > > You might be able to do a local fixup at driver probe time using > of_add_property(). See for example pcs_quirk_missing_pinctrl_cells() > I added earlier because of similar issues. That sounds like a good plan. I suppose it could be added for some kernel releases with a WARN() asking the user to update their DT. -- Sebastian signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: Tree for Nov 19 (i915)
On 11/19/19 12:46 AM, Stephen Rothwell wrote: > Hi all, > > Changes since 20191118: on x86_64: ERROR: "pm_suspend_target_state" [drivers/gpu/drm/i915/i915.ko] undefined! # CONFIG_SUSPEND is not set -- ~Randy Reported-by: Randy Dunlap ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 02/29] dt-bindings: memory: tegra20: emc: Document new interconnect property
19.11.2019 09:21, Thierry Reding пишет: > On Mon, Nov 18, 2019 at 11:02:20PM +0300, Dmitry Osipenko wrote: >> External memory controller is interconnected with memory controller and >> with external memory. Document new interconnect property which designates >> external memory controller as interconnect provider. >> >> Signed-off-by: Dmitry Osipenko >> --- >> .../bindings/memory-controllers/nvidia,tegra20-emc.txt| 4 >> 1 file changed, 4 insertions(+) > > Do we really want to describe this particular connection? It's pretty > static and the only real connection here is the EMC frequency, so the > whole interconnect infrastructure seems a bit overkill. > > Sounds to me like we could piggyback on top of the existing > nvidia,memory-controller property of the EMC to make the connection. Ultimately each memory client talks to EMEM through MC and EMC, although it should be okay to ignore the EMEM from a driver's / software perspective. [snip] ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/modes: remove unused variables
When compiling with W=1 few warnings about unused variables show up. This patch removes all the involved variables. Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/drm_modes.c | 22 +++--- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 88232698d7a0..aca901aff042 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -233,7 +233,7 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay, /* 3) Nominal HSync width (% of line period) - default 8 */ #define CVT_HSYNC_PERCENTAGE 8 unsigned int hblank_percentage; - int vsyncandback_porch, vback_porch, hblank; + int vsyncandback_porch, hblank; /* estimated the horizontal period */ tmp1 = HV_FACTOR * 100 - @@ -249,7 +249,6 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay, else vsyncandback_porch = tmp1; /* 10. Find number of lines in back porch */ - vback_porch = vsyncandback_porch - vsync; drm_mode->vtotal = vdisplay_rnd + 2 * vmargin + vsyncandback_porch + CVT_MIN_V_PORCH; /* 5) Definition of Horizontal blanking time limitation */ @@ -386,9 +385,8 @@ drm_gtf_mode_complex(struct drm_device *dev, int hdisplay, int vdisplay, int top_margin, bottom_margin; int interlace; unsigned int hfreq_est; - int vsync_plus_bp, vback_porch; - unsigned int vtotal_lines, vfieldrate_est, hperiod; - unsigned int vfield_rate, vframe_rate; + int vsync_plus_bp; + unsigned int vtotal_lines; int left_margin, right_margin; unsigned int total_active_pixels, ideal_duty_cycle; unsigned int hblank, total_pixels, pixel_freq; @@ -451,23 +449,9 @@ drm_gtf_mode_complex(struct drm_device *dev, int hdisplay, int vdisplay, /* [V SYNC+BP] = RINT(([MIN VSYNC+BP] * hfreq_est / 100)) */ vsync_plus_bp = MIN_VSYNC_PLUS_BP * hfreq_est / 1000; vsync_plus_bp = (vsync_plus_bp + 500) / 1000; - /* 9. Find the number of lines in V back porch alone: */ - vback_porch = vsync_plus_bp - V_SYNC_RQD; /* 10. Find the total number of lines in Vertical field period: */ vtotal_lines = vdisplay_rnd + top_margin + bottom_margin + vsync_plus_bp + GTF_MIN_V_PORCH; - /* 11. Estimate the Vertical field frequency: */ - vfieldrate_est = hfreq_est / vtotal_lines; - /* 12. Find the actual horizontal period: */ - hperiod = 100 / (vfieldrate_rqd * vtotal_lines); - - /* 13. Find the actual Vertical field frequency: */ - vfield_rate = hfreq_est / vtotal_lines; - /* 14. Find the Vertical frame frequency: */ - if (interlaced) - vframe_rate = vfield_rate / 2; - else - vframe_rate = vfield_rate; /* 15. Find number of pixels in left margin: */ if (margins) left_margin = (hdisplay_rnd * GTF_MARGIN_PERCENTAGE + 500) / -- 2.15.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] msm:disp:dpu1: add support to access hw irqs regs depending on revision
Current code assumes that all the irqs registers offsets can be accessed in all the hw revisions; this is not the case for some targets that should not access some of the irq registers. This change adds the support to selectively remove the irqs that are not supported in some of the hw revisions. Changes in v1: - Add support to selectively remove the hw irqs that are not not supported. Changes in v2: - Remove unrelated changes. Changes in v3: - Remove change-id (Stephen Boyd). - Add colon in variable description to match kernel-doc (Stephen Boyd). - Change macro-y way of variable description (Jordon Crouse). - Remove unnecessary if checks (Jordon Crouse). - Remove extra blank line (Jordon Crouse). Signed-off-by: Shubhashree Dhar --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 22 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 1 + 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 04c8c44..88f2664 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -421,6 +421,7 @@ static void sdm845_cfg_init(struct dpu_mdss_cfg *dpu_cfg) .reg_dma_count = 1, .dma_cfg = sdm845_regdma, .perf = sdm845_perf_data, + .mdss_irqs = 0x3ff, }; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index ec76b868..0fd3f50 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -646,6 +646,7 @@ struct dpu_perf_cfg { * @dma_formatsSupported formats for dma pipe * @cursor_formats Supported formats for cursor pipe * @vig_formatsSupported formats for vig pipe + * @mdss_irqs: Bitmap with the irqs supported by the target */ struct dpu_mdss_cfg { u32 hwversion; @@ -684,6 +685,8 @@ struct dpu_mdss_cfg { struct dpu_format_extended *dma_formats; struct dpu_format_extended *cursor_formats; struct dpu_format_extended *vig_formats; + + unsigned long mdss_irqs; }; struct dpu_mdss_hw_cfg_handler { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 8bfa7d0..0f28f27 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -800,8 +800,8 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr *intr, start_idx = reg_idx * 32; end_idx = start_idx + 32; - if (start_idx >= ARRAY_SIZE(dpu_irq_map) || - end_idx > ARRAY_SIZE(dpu_irq_map)) + if (!test_bit(reg_idx, >irq_mask) || + start_idx >= ARRAY_SIZE(dpu_irq_map)) continue; /* @@ -955,8 +955,11 @@ static int dpu_hw_intr_clear_irqs(struct dpu_hw_intr *intr) if (!intr) return -EINVAL; - for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) - DPU_REG_WRITE(>hw, dpu_intr_set[i].clr_off, 0x); + for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) { + if(test_bit(i, >irq_mask)) + DPU_REG_WRITE(>hw, + dpu_intr_set[i].clr_off, 0x); + } /* ensure register writes go through */ wmb(); @@ -971,8 +974,11 @@ static int dpu_hw_intr_disable_irqs(struct dpu_hw_intr *intr) if (!intr) return -EINVAL; - for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) - DPU_REG_WRITE(>hw, dpu_intr_set[i].en_off, 0x); + for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) { + if(test_bit(i, >irq_mask)) + DPU_REG_WRITE(>hw, + dpu_intr_set[i].en_off, 0x); + } /* ensure register writes go through */ wmb(); @@ -991,6 +997,9 @@ static void dpu_hw_intr_get_interrupt_statuses(struct dpu_hw_intr *intr) spin_lock_irqsave(>irq_lock, irq_flags); for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) { + if(!test_bit(i, >irq_mask)) + continue; + /* Read interrupt status */ intr->save_irq_status[i] = DPU_REG_READ(>hw, dpu_intr_set[i].status_off); @@ -1115,6 +1124,7 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr, return ERR_PTR(-ENOMEM); } + intr->irq_mask = m->mdss_irqs; spin_lock_init(>irq_lock); return intr; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
RE: [PATCH v4 08/13] dt-bindings: display: bridge: Repurpose lvds-encoder
Hi Laurent, Thank you for your feedback! > From: Laurent Pinchart > Sent: 19 November 2019 00:08 > Subject: Re: [PATCH v4 08/13] dt-bindings: display: bridge: Repurpose > lvds-encoder > > Hi Fabrizio, > > Thank you for the patch. > > On Wed, Nov 13, 2019 at 03:51:27PM +, Fabrizio Castro wrote: > > In an effort to repurpose lvds-encoder.c to also serve the > > function of LVDS decoders, we ended up defining a new "generic" > > compatible string ("lvds-decoder"), therefore adapt the dt schema > > to allow for the new compatible string. > > > > Signed-off-by: Fabrizio Castro > > > > --- > > v3->v4: > > * Improved title and description according to Laurent's comments > > * Reworked definition of the compatible property > > v2->v3: > > * Extracted conversion to lvds-codec as per Rob's comment > > v1->v2: > > * Converted to dt-schema as per Neil's comment > > --- > > .../{lvds-transmitter.yaml => lvds-codec.yaml} | 54 > > +- > > 1 file changed, 42 insertions(+), 12 deletions(-) > > rename > > Documentation/devicetree/bindings/display/bridge/{lvds-transmitter.yaml => > > lvds-codec.yaml} (61%) > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.yaml > b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > similarity index 61% > > rename from > > Documentation/devicetree/bindings/display/bridge/lvds-transmitter.yaml > > rename to Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > index 27de616..0ecc8a4 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.yaml > > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > @@ -1,17 +1,17 @@ > > # SPDX-License-Identifier: GPL-2.0 > > %YAML 1.2 > > --- > > -$id: http://devicetree.org/schemas/display/bridge/lvds-transmitter.yaml# > > +$id: http://devicetree.org/schemas/display/bridge/lvds-codec.yaml# > > $schema: http://devicetree.org/meta-schemas/core.yaml# > > > > -title: Parallel to LVDS Encoder > > +title: Transparent LVDS encoders and decoders > > > > maintainers: > >- Laurent Pinchart > > > > description: | > > - This binding supports the parallel to LVDS encoders that don't require > > any > > - configuration. > > + This binding supports transparent LVDS encoders and decoders that don't > > + require any configuration. > > > >LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. > > Multiple > >incompatible data link layers have been used over time to transmit image > > data > > @@ -33,12 +33,14 @@ properties: > > description: | > >Must list the device specific compatible string first, followed by > > the > >generic compatible string. > > -items: > > - - enum: > > -- ti,ds90c185 # For the TI DS90C185 FPD-Link Serializer > > -- ti,ds90c187 # For the TI DS90C187 FPD-Link Serializer > > -- ti,sn75lvds83 # For the TI SN75LVDS83 FlatLink transmitter > > - - const: lvds-encoder # Generic LVDS encoder compatible fallback > > +oneOf: > > + - items: > > +- enum: > > + - ti,ds90c185 # For the TI DS90C185 FPD-Link Serializer > > + - ti,ds90c187 # For the TI DS90C187 FPD-Link Serializer > > + - ti,sn75lvds83 # For the TI SN75LVDS83 FlatLink transmitter > > +- const: lvds-encoder # Generic LVDS encoder compatible fallback > > + - const: lvds-decoder # Generic LVDS decoders compatible fallback > > > >ports: > > type: object > > @@ -49,12 +51,14 @@ properties: > >port@0: > > type: object > > description: | > > - Port 0 is for parallel input > > + With LVDS encoders port 0 is for parallel input > > + With LVDS decoders port 0 is for LVDS input > > How about > > For LVDS encoders, port 0 is the parallel input > For LVDS decoders, port 0 is the LVDS input It's ok with me > > > > >port@1: > > type: object > > description: | > > - Port 1 is for LVDS output > > + With LVDS encoders port 1 is for LVDS output > > + With LVDS decoders port 1 is for parallel output > > And similarly here ? Here too > > If you're fine with this change there's no need to resubmit, I'll change > this when applying, and Yes please. Thanks, Fab > > Reviewed-by: Laurent Pinchart > > > > > required: > >- port@0 > > @@ -96,4 +100,30 @@ examples: > >}; > > }; > > > > + - | > > +lvds-decoder { > > + compatible = "lvds-decoder"; > > + > > + ports { > > +#address-cells = <1>; > > +#size-cells = <0>; > > + > > +port@0 { > > + reg = <0>; > > + > > + lvds_dec_in: endpoint { > > +remote-endpoint = <_out_lvds>; > > + }; > > +}; > > + > > +port@1 { > > + reg = <1>; > > + > > +
[PATCH 3/6] gpu/drm: ingenic: Use the plane's src_[x, y] to configure DMA length
Instead of obtaining the width/height of the framebuffer from the CRTC state, obtain it from the current plane state. Signed-off-by: Paul Cercueil --- drivers/gpu/drm/ingenic/ingenic-drm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c index 6dc4b06e7e68..7a172271bd63 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c @@ -374,8 +374,8 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, struct drm_framebuffer *fb = state->fb; if (fb) { - width = state->crtc->state->adjusted_mode.hdisplay; - height = state->crtc->state->adjusted_mode.vdisplay; + width = state->src_w >> 16; + height = state->src_h >> 16; cpp = fb->format->cpp[plane->index]; priv->dma_hwdesc->addr = drm_fb_cma_get_gem_addr(fb, state, 0); -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 12/29] interconnect: Add memory interconnection providers for NVIDIA Tegra SoCs
19.11.2019 09:31, Thierry Reding пишет: > On Mon, Nov 18, 2019 at 11:02:30PM +0300, Dmitry Osipenko wrote: > [...] >> diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h >> index 1238e35653d1..593954324259 100644 >> --- a/include/soc/tegra/mc.h >> +++ b/include/soc/tegra/mc.h >> @@ -141,6 +141,11 @@ struct tegra_mc_reset_ops { >> const struct tegra_mc_reset *rst); >> }; >> >> +struct tegra_mc_icc_node { >> +const char *name; >> +unsigned int id; >> +}; >> + >> struct tegra_mc_soc { >> const struct tegra_mc_client *clients; >> unsigned int num_clients; >> @@ -160,6 +165,9 @@ struct tegra_mc_soc { >> const struct tegra_mc_reset_ops *reset_ops; >> const struct tegra_mc_reset *resets; >> unsigned int num_resets; >> + >> +const struct tegra_mc_icc_node *icc_nodes; >> +unsigned int num_icc_nodes; >> }; >> >> struct tegra_mc { >> @@ -184,4 +192,22 @@ struct tegra_mc { >> int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long >> rate); >> unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc); >> >> +#ifdef CONFIG_INTERCONNECT_TEGRA >> +int tegra_icc_mc_setup_interconnect(struct tegra_mc *mc); >> +int tegra_icc_emc_setup_interconnect(struct device *emc_dev, >> + unsigned int dram_data_bus_width_bytes); >> +#else >> +static inline int tegra_icc_mc_setup_interconnect(struct tegra_mc *mc); >> +{ >> +return 0; >> +} >> + >> +static inline int >> +tegra_icc_emc_setup_interconnect(struct device *emc_dev, >> + unsigned int dram_data_bus_width_bytes) >> +{ >> +return 0; >> +} >> +#endif > > Is there really any reason why we should make this support optional? It > seems to me like we would always want this enabled once it's tested and > known to work. There is always some room for bugs, should be better to have an option to disable ICC entirely (IMO). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6] dt-bindings: display/ingenic: Add compatible string for JZ4770
Add a compatible string for the LCD controller found in the JZ4770 SoC. Signed-off-by: Paul Cercueil --- Documentation/devicetree/bindings/display/ingenic,lcd.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/ingenic,lcd.txt b/Documentation/devicetree/bindings/display/ingenic,lcd.txt index 7b536c8c6dde..01e3261defb6 100644 --- a/Documentation/devicetree/bindings/display/ingenic,lcd.txt +++ b/Documentation/devicetree/bindings/display/ingenic,lcd.txt @@ -4,6 +4,7 @@ Required properties: - compatible: one of: * ingenic,jz4740-lcd * ingenic,jz4725b-lcd + * ingenic,jz4770-lcd - reg: LCD registers location and length - clocks: LCD pixclock and device clock specifiers. The device clock is only required on the JZ4740. -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/crtc-helper: drm_connector_get_single_encoder prototype is missing
On 11/19/19 7:53 PM, Souza, Jose wrote: > On Tue, 2019-11-19 at 13:58 +0100, Benjamin Gaignard wrote: >> Include drm_crtc_helper_internal.h to provide >> drm_connector_get_single_encoder >> prototype. >> >> Fixes: a92462d6bf493 ("drm/connector: Share with non-atomic drivers >> the function to get the single encoder") > drm_connector_get_single_encoder() is implemented before the use in > this file so it is not broken, no need of a fixes tag. > > Reviewed-by: José Roberto de Souza I will remove fixe tag before push it. Thanks, Benjamin > >> Cc: José Roberto de Souza >> >> Signed-off-by: Benjamin Gaignard >> --- >> drivers/gpu/drm/drm_crtc_helper.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c >> b/drivers/gpu/drm/drm_crtc_helper.c >> index 499b05aaccfc..93a4eec429e8 100644 >> --- a/drivers/gpu/drm/drm_crtc_helper.c >> +++ b/drivers/gpu/drm/drm_crtc_helper.c >> @@ -48,6 +48,8 @@ >> #include >> #include >> >> +#include "drm_crtc_helper_internal.h" >> + >> /** >>* DOC: overview >>* ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/rect: remove useless call to clamp_t
Clamping a value between INT_MIN and INT_MAX always return the value itself and generate warnings when compiling with W=1. Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/drm_rect.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c index b8363aaa9032..681f1fd09357 100644 --- a/drivers/gpu/drm/drm_rect.c +++ b/drivers/gpu/drm/drm_rect.c @@ -89,7 +89,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst, u32 new_src_w = clip_scaled(drm_rect_width(src), drm_rect_width(dst), diff); - src->x1 = clamp_t(int64_t, src->x2 - new_src_w, INT_MIN, INT_MAX); + src->x1 = src->x2 - new_src_w; dst->x1 = clip->x1; } diff = clip->y1 - dst->y1; @@ -97,7 +97,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst, u32 new_src_h = clip_scaled(drm_rect_height(src), drm_rect_height(dst), diff); - src->y1 = clamp_t(int64_t, src->y2 - new_src_h, INT_MIN, INT_MAX); + src->y1 = src->y2 - new_src_h; dst->y1 = clip->y1; } diff = dst->x2 - clip->x2; @@ -105,7 +105,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst, u32 new_src_w = clip_scaled(drm_rect_width(src), drm_rect_width(dst), diff); - src->x2 = clamp_t(int64_t, src->x1 + new_src_w, INT_MIN, INT_MAX); + src->x2 = src->x1 + new_src_w; dst->x2 = clip->x2; } diff = dst->y2 - clip->y2; @@ -113,7 +113,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst, u32 new_src_h = clip_scaled(drm_rect_height(src), drm_rect_height(dst), diff); - src->y2 = clamp_t(int64_t, src->y1 + new_src_h, INT_MIN, INT_MAX); + src->y2 = src->y1 + new_src_h; dst->y2 = clip->y2; } -- 2.15.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 23/29] memory: tegra124-emc: Register as interconnect provider
18.11.2019 23:02, Dmitry Osipenko пишет: > EMC now provides MC with memory bandwidth using interconnect API. > > Signed-off-by: Dmitry Osipenko > --- > drivers/memory/tegra/tegra124-emc.c | 24 > 1 file changed, 24 insertions(+) > > diff --git a/drivers/memory/tegra/tegra124-emc.c > b/drivers/memory/tegra/tegra124-emc.c > index 2c73260654ba..c9478dcbeece 100644 > --- a/drivers/memory/tegra/tegra124-emc.c > +++ b/drivers/memory/tegra/tegra124-emc.c > @@ -25,6 +25,7 @@ > #define EMC_FBIO_CFG50x104 > #define EMC_FBIO_CFG5_DRAM_TYPE_MASK0x3 > #define EMC_FBIO_CFG5_DRAM_TYPE_SHIFT 0 > +#define EMC_FBIO_CFG5_DRAM_WIDTH_X64 BIT(4) > > #define EMC_INTSTATUS0x0 > #define EMC_INTSTATUS_CLKCHANGE_COMPLETE BIT(4) > @@ -1080,11 +1081,28 @@ static void emc_debugfs_init(struct device *dev, > struct tegra_emc *emc) > dev_err(dev, "failed to create debugfs entry\n"); > } > > +static unsigned int emc_dram_data_bus_width_bytes(struct tegra_emc *emc) > +{ > + unsigned int bus_width; > + u32 emc_cfg; > + > + emc_cfg = readl_relaxed(emc->regs + EMC_FBIO_CFG5); > + if (emc_cfg & EMC_FBIO_CFG5_DRAM_WIDTH_X64) > + bus_width = 64; > + else > + bus_width = 32; Looks like I got a bit confused while was looking at TRMs before, seems this width is unrelated to the EMC channel at all. I'll try to revisit this again. > + dev_info(emc->dev, "DRAM data-bus width: %ubit\n", bus_width); > + > + return bus_width / 8; > +} > + > static int tegra_emc_probe(struct platform_device *pdev) > { > struct platform_device *mc; > struct device_node *np; > struct tegra_emc *emc; > + unsigned int bus_width; > u32 ram_code; > int err; > > @@ -1146,6 +1164,12 @@ static int tegra_emc_probe(struct platform_device > *pdev) > if (IS_ENABLED(CONFIG_DEBUG_FS)) > emc_debugfs_init(>dev, emc); > > + bus_width = emc_dram_data_bus_width_bytes(emc); > + > + err = tegra_icc_emc_setup_interconnect(>dev, bus_width); > + if (err) > + dev_err(>dev, "failed to initialize ICC: %d\n", err); > + > return 0; > }; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] gpu/drm: ingenic: Set max FB height to 4095
While the LCD controller can effectively only support a maximum resolution of 800x600, the framebuffer's height can be much higher, since we can change the Y start offset. Signed-off-by: Paul Cercueil --- drivers/gpu/drm/ingenic/ingenic-drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c index 7a172271bd63..4538b081b0c5 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c @@ -635,7 +635,7 @@ static int ingenic_drm_probe(struct platform_device *pdev) drm->mode_config.min_width = 0; drm->mode_config.min_height = 0; drm->mode_config.max_width = 800; - drm->mode_config.max_height = 600; + drm->mode_config.max_height = 4095; drm->mode_config.funcs = _drm_mode_config_funcs; base = devm_platform_ioremap_resource(pdev, 0); -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v4 12/13] [HACK] drm/bridge: lvds-codec: Enforce device specific compatible strings
Hi Laurent, Thank you for your feedback! > From: Laurent Pinchart > Sent: 19 November 2019 00:16 > Subject: Re: [PATCH v4 12/13] [HACK] drm/bridge: lvds-codec: Enforce device > specific compatible strings > > Hi Fabrizio, > > Thank you for the patch. > > On Wed, Nov 13, 2019 at 03:51:31PM +, Fabrizio Castro wrote: > > The lvds-codec driver is a generic stub for transparent LVDS > > encoders and decoders. > > It's good practice to list a device specific compatible string > > s/good practice/mandatory/ Will fix > > > before the generic fallback (if any) in the DT node for the relevant > > LVDS encoder/decoder, and it's also required by the dt-bindings. > > A notable exception to the generic fallback mechanism is the case > > of "thine,thc63lvdm83d", as documented in: > > Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt > > This patch enforces the adoption of a device specific compatible > > string (as fist string in the list), by using markers for the > > s/fist/first/ Well spotted > > > compatible string we match against and the index of the matching > > compatible string in the list. > > > > Signed-off-by: Fabrizio Castro > > > > --- > > Hi Laurent, > > > > I don't think we need to do anything in the driver to address your > > comment, as we can "enforce" this with the bindings (please see the > > next patch, as it would help with the "enforcing" of the compatible > > string for the thine device). > > I am sending this patch only so that you can see what a possible > > solution in the driver could look like. > > > > v3->v4: > > * New patch addressing the below comment from Laurent: > > "I think the lvds-decoder driver should error out at probe time if only > > one compatible string is listed." > > --- > > drivers/gpu/drm/bridge/lvds-codec.c | 55 > > + > > 1 file changed, 49 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/lvds-codec.c > > b/drivers/gpu/drm/bridge/lvds-codec.c > > index 784bbd3..145c25d 100644 > > --- a/drivers/gpu/drm/bridge/lvds-codec.c > > +++ b/drivers/gpu/drm/bridge/lvds-codec.c > > @@ -14,11 +14,16 @@ > > #include > > #include > > > > +struct lvds_codec_data { > > + u32 connector_type; > > + bool device_specific; > > +}; > > + > > struct lvds_codec { > > struct drm_bridge bridge; > > struct drm_bridge *panel_bridge; > > struct gpio_desc *powerdown_gpio; > > - u32 connector_type; > > + const struct lvds_codec_data *data; > > }; > > > > static int lvds_codec_attach(struct drm_bridge *bridge) > > @@ -65,7 +70,30 @@ static int lvds_codec_probe(struct platform_device *pdev) > > if (!lvds_codec) > > return -ENOMEM; > > > > - lvds_codec->connector_type = (u32)of_device_get_match_data(>dev); > > + lvds_codec->data = of_device_get_match_data(>dev); > > + if (!lvds_codec->data) > > + return -EINVAL; > > + > > + /* > > +* If we haven't matched a device specific compatible string, we need > > +* to work out if the generic compatible string we matched against was > > +* listed first in the compatible property. > > +*/ > > Can't we do this unconditionally, and thus drop the lvds_codec_data > structure ? I don't think so, and the reason for this is that we have a corner case for thine,thc63lvdm83d. Here is what's allowed (according to the documentation) from what's supported upstream (+ this series): "ti,ds90c185", "lvds-encoder" "ti,ds90c187", "lvds-encoder" "ti,sn75lvds83", "lvds-encoder" "ti,ds90cf384a", "lvds-decoder" "thine,thc63lvdm83d" As you can see from the examples above, in most cases it's enough to say it's all good when we match a compatible string with index > 0, but for the thine device you _have_ to match the string with index 0 as that's what's currently documented (please see thine,thc63lvdm83d.txt) and that's what's supported by device trees already (please see arch/arm/boot/dts/r8a7779-marzen.dts). This patch "classifies" compatible strings, and it considers a good match device specific compatible strings, or generic compatible strings as long as they are not listed first. These days you can leverage the yaml files to validate the device trees, therefore we should be focusing on writing yaml files in such a way we only pass the checks we mean to, and by checks I mean: make dtbs_check or more specifically, for this series: make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml and that's of course on top of make dt_binding_check. It's a very common requirement to have a part number specific compatible string first followed by a generic (fallback) compatible string in the device trees, most drivers for Renesas SoCs have similar requirements. If we start doing this here, we'll end up doing it elsewhere as well, and I really think we shouldn't, but others may see things differently, so I'll wait for others (and yourself with further comments) to
[PATCH 5/6] gpu/drm: ingenic: Check for display size in CRTC atomic check
Check that the requested display size isn't above the limits supported by the CRTC. - JZ4750 and older support up to 800x600; - JZ4755 supports up to 1024x576; - JZ4760 and JZ4770 support up to 720p; - JZ4780 supports up to 2k. Signed-off-by: Paul Cercueil --- drivers/gpu/drm/ingenic/ingenic-drm.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c index 4538b081b0c5..d578c4cb6009 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c @@ -152,6 +152,7 @@ struct ingenic_dma_hwdesc { struct jz_soc_info { bool needs_dev_clk; + unsigned int max_width, max_height; }; struct ingenic_drm { @@ -163,6 +164,7 @@ struct ingenic_drm { struct device *dev; struct regmap *map; struct clk *lcd_clk, *pix_clk; + const struct jz_soc_info *soc_info; struct ingenic_dma_hwdesc *dma_hwdesc; dma_addr_t dma_hwdesc_phys; @@ -325,6 +327,10 @@ static int ingenic_drm_crtc_atomic_check(struct drm_crtc *crtc, if (!drm_atomic_crtc_needs_modeset(state)) return 0; + if (state->mode.hdisplay > priv->soc_info->max_height || + state->mode.vdisplay > priv->soc_info->max_width) + return -EINVAL; + rate = clk_round_rate(priv->pix_clk, state->adjusted_mode.clock * 1000); if (rate < 0) @@ -619,6 +625,7 @@ static int ingenic_drm_probe(struct platform_device *pdev) if (!priv) return -ENOMEM; + priv->soc_info = soc_info; priv->dev = dev; drm = >drm; drm->dev_private = priv; @@ -634,7 +641,7 @@ static int ingenic_drm_probe(struct platform_device *pdev) drm_mode_config_init(drm); drm->mode_config.min_width = 0; drm->mode_config.min_height = 0; - drm->mode_config.max_width = 800; + drm->mode_config.max_width = soc_info->max_width; drm->mode_config.max_height = 4095; drm->mode_config.funcs = _drm_mode_config_funcs; @@ -812,10 +819,14 @@ static int ingenic_drm_remove(struct platform_device *pdev) static const struct jz_soc_info jz4740_soc_info = { .needs_dev_clk = true, + .max_width = 800, + .max_height = 600, }; static const struct jz_soc_info jz4725b_soc_info = { .needs_dev_clk = false, + .max_width = 800, + .max_height = 600, }; static const struct of_device_id ingenic_drm_of_match[] = { -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFCv1 33/42] drm/omap: dsi: use atomic helper for dirtyfb
* Tomi Valkeinen [191119 05:42]: > On 19/11/2019 01:05, Tony Lindgren wrote: > > * Sebastian Reichel [191117 07:11]: > > > We can simply use the atomic helper for > > > handling the dirtyfb callback. > > ... > > > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c > > > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c > > > -void omap_crtc_flush(struct drm_crtc *crtc) > > > +static void omap_crtc_flush(struct drm_crtc *crtc) > > > { > > > struct omap_crtc *omap_crtc = to_omap_crtc(crtc); > > > - struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state); > > > - > > > - if (!omap_state->manually_updated) > > > - return; > > > if (!delayed_work_pending(_crtc->update_work)) > > > schedule_delayed_work(_crtc->update_work, 0); > > > > It would be nice if omap_crtc_flush() would become just some generic > > void function with no need to pass it a crtc. I guess for that it > > should know what panels are in manual command mode to refresh them. Proably still cannot be a void function, but seems like we need to call something outside omap_crtc.c. > > The reason I'm bringing this up is because it looks like we need > > to also flush DSI command mode panels from omap_gem_op_finish() > > for gles and the gem code probably should not need to know anything > > about crtc, right? > > We haven't had omap_gem_op_finish() in the kernel for some years now... > > Shouldn't a normal page flip, or if doing single-buffering, using the > dirtyfb ioctl, do the job? It does not seem to happen with the old pvr-omap4 related patches and whatever gles related tests at least. If you have some idea where it should get called I can take a look at some point. Regards, Tony ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] gpu/drm: ingenic: Avoid null pointer deference in plane atomic update
It is possible that there is no drm_framebuffer associated with a given plane state. Signed-off-by: Paul Cercueil --- drivers/gpu/drm/ingenic/ingenic-drm.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c index 2e2ed653e9c6..6dc4b06e7e68 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c @@ -371,14 +371,17 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, struct ingenic_drm *priv = drm_plane_get_priv(plane); struct drm_plane_state *state = plane->state; unsigned int width, height, cpp; + struct drm_framebuffer *fb = state->fb; - width = state->crtc->state->adjusted_mode.hdisplay; - height = state->crtc->state->adjusted_mode.vdisplay; - cpp = state->fb->format->cpp[plane->index]; + if (fb) { + width = state->crtc->state->adjusted_mode.hdisplay; + height = state->crtc->state->adjusted_mode.vdisplay; + cpp = fb->format->cpp[plane->index]; - priv->dma_hwdesc->addr = drm_fb_cma_get_gem_addr(state->fb, state, 0); - priv->dma_hwdesc->cmd = width * height * cpp / 4; - priv->dma_hwdesc->cmd |= JZ_LCD_CMD_EOF_IRQ; + priv->dma_hwdesc->addr = drm_fb_cma_get_gem_addr(fb, state, 0); + priv->dma_hwdesc->cmd = width * height * cpp / 4; + priv->dma_hwdesc->cmd |= JZ_LCD_CMD_EOF_IRQ; + } } static void ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder, -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6] gpu/drm: ingenic: Add support for the JZ4770
The LCD controller in the JZ4770 supports up to 720p. While there has been many new features added since the old JZ4740, which are not yet handled here, this driver still works fine. Signed-off-by: Paul Cercueil --- drivers/gpu/drm/ingenic/ingenic-drm.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c index d578c4cb6009..46d3ce763bb9 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c @@ -829,9 +829,16 @@ static const struct jz_soc_info jz4725b_soc_info = { .max_height = 600, }; +static const struct jz_soc_info jz4770_soc_info = { + .needs_dev_clk = false, + .max_width = 1280, + .max_height = 720, +}; + static const struct of_device_id ingenic_drm_of_match[] = { { .compatible = "ingenic,jz4740-lcd", .data = _soc_info }, { .compatible = "ingenic,jz4725b-lcd", .data = _soc_info }, + { .compatible = "ingenic,jz4770-lcd", .data = _soc_info }, { /* sentinel */ }, }; -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm/i915 / LPSS / mfd: Select correct PWM controller to use based on VBT
On Tue, Nov 19, 2019 at 04:18:15PM +0100, Hans de Goede wrote: > Hi All, > > This series needs to be merged through a single tree, to keep things > bisectable. I have even considered just squashing all 3 patches into 1, > but having separate commits seems better, but that does lead to an > intermediate state where the backlight sysfs interface will be broken > (and fixed 2 commits later). See below for some background info. > > The changes to drivers/acpi/acpi_lpss.c and drivers/mfd/intel_soc_pmic_core.c > are quite small and should not lead to any conflicts, so I believe that > it would be best to merge this entire series through the drm-intel tree. > > Lee, may I have your Acked-by for merging the mfd change through the > drm-intel tree? > > Rafael, may I have your Acked-by for merging the acpi_lpss change through the > drm-intel tree? > Entire series (or a single patch) makes sense to me. Thanks for fixing this old hardware! FWIW, Reviewed-by: Andy Shevchenko > Regards, > > Hans > > p.s. > > The promised background info: > > We have this long standing issue where instead of looking in the i915 > VBT (Video BIOS Table) to see if we should use the PWM block of the SoC > or of the PMIC to control the backlight of a DSI panel, we rely on > drivers/acpi/acpi_lpss.c and/or drivers/mfd/intel_soc_pmic_core.c > registering a pwm with the generic name of "pwm_backlight" and then the > i915 panel code does a pwm_get(dev, "pwm_backlight"). > > We have some heuristics in drivers/acpi/acpi_lpss.c to not register the > lookup if a Crystal Cove PMIC is presend and the mfd/intel_soc_pmic_core.c > code simply assumes that since there is a PMIC the PMIC PWM block will > be used. Basically we are winging it. > > Recently I've learned about 2 different BYT devices: > Point of View MOBII TAB-P800W > Acer Switch 10 SW5-012 > > Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS > PWM controller (and the VBT correctly indicates this), so here our old > heuristics fail. > > This series renams the PWM lookups registered by the LPSS / > intel_soc_pmic_core.c code from "pwm_backlight" to "pwm_soc_backlight" resp. > "pwm_pmic_backlight" and in the LPSS case also dropping the heuristics when > to register the lookup. This combined with teaching the i915 panel to call > pwm_get for the right lookup-name depending on the VBT bits resolves this. > -- With Best Regards, Andy Shevchenko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/crtc-helper: drm_connector_get_single_encoder prototype is missing
Include drm_crtc_helper_internal.h to provide drm_connector_get_single_encoder prototype. Fixes: a92462d6bf493 ("drm/connector: Share with non-atomic drivers the function to get the single encoder") Cc: José Roberto de Souza Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/drm_crtc_helper.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 499b05aaccfc..93a4eec429e8 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -48,6 +48,8 @@ #include #include +#include "drm_crtc_helper_internal.h" + /** * DOC: overview * -- 2.15.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFCv1 32/42] drm/omap: dsi: convert to drm_panel
Hi Nikolaus, On Tue, Nov 19, 2019 at 10:42:55AM +0100, H. Nikolaus Schaller wrote: > > Am 18.11.2019 um 15:51 schrieb H. Nikolaus Schaller : > > > >> Ok, I tried not to break video mode support, but I do not have any > >> hardware. Make sure to set the MIPI_DSI_MODE_VIDEO flag in the panel > >> driver. > > > > Indeed, this may be missing (can't look into the code at the moment)... > > Or I did something wrong when refactoring the driver. > > We will find out. > > Yes, MIPI_DSI_MODE_VIDEO was indeed missing/wrongly applied and some > more bugs. But I still wasn't able to make it work. > > I also tried to fake the panel-orisetech-otm8009a.c DSI driver into > my setup. It should not properly program the panel by DCS command > but it should try to. > > Result is the same: I can see it being probed and calling get_modes > but then: > > [drm] Cannot find any crtc or sizes > > And I don't see calls to .enable or .prepare where DCS commands would > be sent. You probably want to enable all kind of drm debugging to get more information. I guess, that some timing check fails, e.g. dsi_check_timings(). If the timing checks fail, the mode will not be added as valid mode. When no valid mode is found, the panel will not be enabled. -- Sebastian signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/fb-cma-helpers: Fix include issue
Exported functions prototypes are missing in drm_fb_cma_helper.c Include drm_fb_cma_helper to fix that issue. Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/drm_fb_cma_helper.c | 1 + include/drm/drm_fb_cma_helper.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index c0b0f603af63..9801c0333eca 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -9,6 +9,7 @@ * Copyright (C) 2012 Red Hat */ +#include #include #include #include diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h index 4becb09975a4..795aea1d0a25 100644 --- a/include/drm/drm_fb_cma_helper.h +++ b/include/drm/drm_fb_cma_helper.h @@ -2,6 +2,8 @@ #ifndef __DRM_FB_CMA_HELPER_H__ #define __DRM_FB_CMA_HELPER_H__ +#include + struct drm_framebuffer; struct drm_plane_state; -- 2.15.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 08/29] dt-bindings: interconnect: tegra: Add initial IDs
19.11.2019 09:25, Thierry Reding пишет: > On Mon, Nov 18, 2019 at 11:02:26PM +0300, Dmitry Osipenko wrote: >> Define interconnect IDs for memory controller (MC), external memory >> controller (EMC), external memory (EMEM) and memory clients of display >> controllers (DC). >> >> Signed-off-by: Dmitry Osipenko >> --- >> include/dt-bindings/interconnect/tegra-icc.h | 11 +++ >> 1 file changed, 11 insertions(+) >> create mode 100644 include/dt-bindings/interconnect/tegra-icc.h Hello Thierry, > There was a bit of discussion regarding this for a recent patch that I > was working on, see: > > http://patchwork.ozlabs.org/project/linux-tegra/list/?series=140318 Thank you very much for the link. > I'd rather not use an additional set of definitions for this. The memory > controller already has a set of native IDs for memory clients that I > think we can reuse for this. I missed that it's fine to have multiple ICC connections defined per-path, at quick glance looks like indeed it should be fine to re-use MC IDs. > I've only added these client IDs for Tegra194 because that's where we > need it to actually describe a specific hardware quirk, but I can come > up with the equivalent for older chips as well. Older Tegra SoCs have hardware units connected to MC through AHB bus, like USB for example. These units do not have MC client IDs and there is no MC ID defined for the AHB bus either, but probably it won't be a problem to define IDs for them if will be necessary. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFCv1 33/42] drm/omap: dsi: use atomic helper for dirtyfb
On Tue, 19 Nov 2019 17:55:57 +0200 Tomi Valkeinen wrote: > On 19/11/2019 17:06, Tony Lindgren wrote: > > >> The userspace apps need to do this. If they're using single-buffering, then > >> using the dirtyfb ioctl should do the trick, after the SGX has finished > >> drawing. > > > > Sounds like that's not happening. > > > > But why would the userspace app need to know this might be needed for > > a DSI command mode display and that it's not needed for HDMI? > > When double-buffering, the userspace doesn't need to care, as the > page-flip ioctl explicitly tells that the buffer is ready. > > When single buffering, either the userspace has to tell that the buffer > is now ready, or the kernel has to guess based on something. But afaics, > the latter is always a guess, and may not be a good guess. > > The kernel could trigger a delayed update based on, say, page fault if > drawing with CPU. But how much delayed... And with this scenario, we > would somehow need to find a way to catch the writes from any IP to the > buffer, and afaik there's no such thing. > > >> It's probably somewhat difficult if EGL is controlling the display, as, > >> afaik, SGX EGL is closed source, and that's probably where it should be > >> done. > >> > >> But adding back the hacky omap gem sync stuff, and then somehow guessing > >> from the sync finish that some display needs to be updated... It just does > >> not sound right to me. > > > > Right it's ugly. Still sounds like we need something in the kernel > > that knows "this is a DSI command mode LCD and needs to be updated". > well, if we look broader around and not only at the remaining displays to be converted from omapdrm to generic and look at more displays, there are also EPDs. > I think one option is to refresh the command mode display all the time. > Either using a timer, or trigger updates based on the previous update > being finished. > > Of course, that's kind of against the whole point of manual update > display, but then it should effectively behave like a conventional > always-updating display (except your HW is more expensive and consumes > more power than a conventional display). > And again as an extreme example about power consumption: EPDs. So I guess we need a generic interface for userspace-triggered refreshes anyways. And in that case that are only partly refreshes. Regards, Andreas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFCv1 33/42] drm/omap: dsi: use atomic helper for dirtyfb
* Tomi Valkeinen [191119 14:54]: > On 19/11/2019 16:32, Tony Lindgren wrote: > > > > We haven't had omap_gem_op_finish() in the kernel for some years now... > > > > > > Shouldn't a normal page flip, or if doing single-buffering, using the > > > dirtyfb ioctl, do the job? > > > > It does not seem to happen with the old pvr-omap4 related patches > > and whatever gles related tests at least. If you have some idea > > where it should get called I can take a look at some point. > > The userspace apps need to do this. If they're using single-buffering, then > using the dirtyfb ioctl should do the trick, after the SGX has finished > drawing. Sounds like that's not happening. But why would the userspace app need to know this might be needed for a DSI command mode display and that it's not needed for HDMI? > It's probably somewhat difficult if EGL is controlling the display, as, > afaik, SGX EGL is closed source, and that's probably where it should be > done. > > But adding back the hacky omap gem sync stuff, and then somehow guessing > from the sync finish that some display needs to be updated... It just does > not sound right to me. Right it's ugly. Still sounds like we need something in the kernel that knows "this is a DSI command mode LCD and needs to be updated". Regards, Tony ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v2] msm: disp: dpu1: add support to access hw irqs regs depending on revision
On 2019-11-14 22:59, Stephen Boyd wrote: Quoting Shubhashree Dhar (2019-11-13 21:56:16) Current code assumes that all the irqs registers offsets can be accessed in all the hw revisions; this is not the case for some targets that should not access some of the irq registers. What happens if we read the irq registers that we "should not access"? Does the system reset? It would be easier to make those registers return 0 when read indicating no interrupt and ignore writes so that everything keeps working without having to skip registers. In some of the hw revisions, the whole hw block is absent and trying to access those registers causes system panic(bus noc error). This change adds the support to selectively remove the irqs that are not supported in some of the hw revisions. Change-Id: I6052b8237b703a1a9edd53893e04f7bd72223da1 Please remove these before sending upstream. Signed-off-by: Shubhashree Dhar --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 22 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 1 + 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index ec76b868..def8a3f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -646,6 +646,7 @@ struct dpu_perf_cfg { * @dma_formatsSupported formats for dma pipe * @cursor_formats Supported formats for cursor pipe * @vig_formatsSupported formats for vig pipe + * @mdss_irqs Bitmap with the irqs supported by the target Hmm pretty sure there needs to be a colon so that kernel-doc can match this but maybe I'm wrong. */ struct dpu_mdss_cfg { u32 hwversion; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 12/29] interconnect: Add memory interconnection providers for NVIDIA Tegra SoCs
19.11.2019 09:30, Thierry Reding пишет: > On Mon, Nov 18, 2019 at 11:02:30PM +0300, Dmitry Osipenko wrote: >> All NVIDIA Tegra SoCs have identical topology in regards to memory >> interconnection between memory clients and memory controllers. >> The memory controller (MC) and external memory controller (EMC) are >> providing memory clients with required memory bandwidth. The memory >> controller performs arbitration between memory clients, while the >> external memory controller transfers data from/to DRAM and pipes that >> data from/to memory controller. Memory controller interconnect provider >> aggregates bandwidth requests from memory clients and sends the aggregated >> request to EMC provider that scales DRAM frequency in order to satisfy the >> bandwidth requirement. Memory controller provider could adjust hardware >> configuration for a more optimal arbitration depending on bandwidth >> requirements from memory clients, but this is unimplemented for now. >> >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/interconnect/Kconfig | 1 + >> drivers/interconnect/Makefile | 1 + >> drivers/interconnect/tegra/Kconfig | 6 + >> drivers/interconnect/tegra/Makefile| 4 + >> drivers/interconnect/tegra/tegra-icc-emc.c | 138 + >> drivers/interconnect/tegra/tegra-icc-mc.c | 130 +++ >> include/soc/tegra/mc.h | 26 >> 7 files changed, 306 insertions(+) >> create mode 100644 drivers/interconnect/tegra/Kconfig >> create mode 100644 drivers/interconnect/tegra/Makefile >> create mode 100644 drivers/interconnect/tegra/tegra-icc-emc.c >> create mode 100644 drivers/interconnect/tegra/tegra-icc-mc.c > > Why does this have to be separate from the memory controller driver in > drivers/memory/tegra? It seems like this requires a bunch of boilerplate > just so that this code can live in the drivers/interconnect directory. It fits with the IOMMU separation. To me that it's a bit nicer to have the separation for the ICC as well, but having ICC within memory controller driver also will be fine. Indeed it looks like there is not much in the MC's provider code right now, but maybe more stuff will be added later on. > If Georgi doesn't insist, I'd prefer if we carried this code directly in > the drivers/memory/tegra directory so that we don't have so many > indirections. > > Also, and I already briefly mentioned this in another reply, I think we > don't need two providers here. The only one we're really interested in > is the memory-client to memory-controller paths. The MC to EMC path is > static. Perhaps it is fine to drop EMC path, I'll revisit it. [snip] ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
[+cc Daniel, Vidya, Thierry; thread starts at https://lore.kernel.org/r/20191017121901.13699-1-kher...@redhat.com] On Tue, Nov 19, 2019 at 11:26:45PM +0100, Karol Herbst wrote: > On Tue, Nov 19, 2019 at 10:50 PM Bjorn Helgaas wrote: > > On Thu, Oct 17, 2019 at 02:19:01PM +0200, Karol Herbst wrote: > > > Fixes state transitions of Nvidia Pascal GPUs from D3cold into > > > higher device states. > > > ... > > > + * - kernel crashes, as all pci reads return -1, which most code > > > + *isn't able to handle well enough. > > > > More details about these crashes would be useful as we look at > > places that *should* be able to handle errors like this. > > makes sense, I could ,orthogonal to this, make the code more robust > if we hit issues like this in the future. What I am mostly wondering > about is, why pci core doesn't give up if the device doesn't come > back from D3cold? It sounds like, that the most sane thing to do > here is to just give up and fail runtime_resume and report errors > back to userspace trying to make use of the devices. It's possible there's something the core could do here. It's interesting that we have at least three issues in this area in this release: 1) This NVIDIA GPU issue 2) Daniel's issue with AMD Ryzen5/7 XHCI power-on (https://lore.kernel.org/r/20191014061355.29072-1-dr...@endlessm.com) 3) Vidya's issue with Intel 750 NVMe power-on (https://lore.kernel.org/r/20191118172310.21373-1-vid...@nvidia.com) Vidya's current patch is the most generic (calling pci_dev_wait() from pci_power_up()). That will print a warning if the device doesn't respond, but we still don't go as far as returning errors to runtime_resume. This is definitely something we should consider improving in the future. > > > + * - sudden shutdowns, as the kernel identified an unrecoverable error > > > after > > > + *userspace tries to access the GPU. > > > > This doesn't fit with the others and more details might be > > informative here as well. > > yeah.. I try to get more infos on that. But at least for me (and it > might be a distribution thing) if I execute lspci, the system shuts > down, or at least tries to and might fail. The lspci doesn't need to be after the failure occurs. You can do it immediately after boot. If you can capture any part of the dmesg or console log of these sudden shutdowns, that's all I'm interested in at this point. Bjorn ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Tue, Nov 19, 2019 at 10:50 PM Bjorn Helgaas wrote: > > [+cc Dave] > > On Thu, Oct 17, 2019 at 02:19:01PM +0200, Karol Herbst wrote: > > Fixes state transitions of Nvidia Pascal GPUs from D3cold into higher device > > states. > > > > v2: convert to pci_dev quirk > > put a proper technical explanation of the issue as a in-code comment > > v3: disable it only for certain combinations of intel and nvidia hardware > > v4: simplify quirk by setting flag on the GPU itself > > I have zero confidence that we understand the real problem, but we do > need to do something with this. I'll merge it for v5.5 if we get the > minor procedural stuff below straightened out. > Thanks, and I agree with your statement, but at this point I think only Intel can help out digging deeper as I see no way to debug this further. > > Signed-off-by: Karol Herbst > > Cc: Bjorn Helgaas > > Cc: Lyude Paul > > Cc: Rafael J. Wysocki > > Cc: Mika Westerberg > > Cc: linux-...@vger.kernel.org > > Cc: linux...@vger.kernel.org > > Cc: dri-devel@lists.freedesktop.org > > Cc: nouv...@lists.freedesktop.org > > --- > > drivers/pci/pci.c| 7 ++ > > drivers/pci/quirks.c | 53 > > include/linux/pci.h | 1 + > > 3 files changed, 61 insertions(+) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index b97d9e10c9cc..02e71e0bcdd7 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -850,6 +850,13 @@ static int pci_raw_set_power_state(struct pci_dev > > *dev, pci_power_t state) > > || (state == PCI_D2 && !dev->d2_support)) > > return -EIO; > > > > + /* > > + * check if we have a bad combination of bridge controller and nvidia > > + * GPU, see quirk_broken_nv_runpm for more info > > Whitespace damage. Capitalized incorrectly (see other comments > nearby). > > > + */ > > + if (state != PCI_D0 && dev->broken_nv_runpm) > > + return 0; > > + > > pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, ); > > > > /* > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index 44c4ae1abd00..0006c9e37b6f 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -5268,3 +5268,56 @@ static void > > quirk_reset_lenovo_thinkpad_p50_nvgpu(struct pci_dev *pdev) > > DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > > PCI_CLASS_DISPLAY_VGA, 8, > > quirk_reset_lenovo_thinkpad_p50_nvgpu); > > + > > +/* > > + * Some Intel PCIe bridges cause devices to disappear from the PCIe bus > > after > > + * those were put into D3cold state if they were put into a non D0 PCI PM > > + * device state before doing so. > > A device in D3cold is off the bus by definition. > > IIUC the problem is that the sequence D0 -> D3hot -> D3cold -> D0 for > the GPU fails in the transition back to D0, while D0 -> D3cold -> D0 > works fine. > > So I guess the problem is that we can put the device in D3cold with no > problem, but if we put in D3hot before going to D3cold, the device > never comes back to D0. Right? > correct. It By the way, it doesn't matter if I put the device into D1 instead, as long as the device is not in D0 state before putting it into D3cold, it fails. > > + * This leads to various issue different issues which all manifest > > differently, > > s/issue different// > > Actually, I think there's only one underlying issue with several > manifestations. > > > + * but have the same root cause: > > + * - AIML code execution hits an infinite loop (as the coe waits on device > > + *memory to change). > > s/AIML/AML/ > s/coe/code/ > > > + * - kernel crashes, as all pci reads return -1, which most code isn't > > able > > + *to handle well enough. > > s/pci/PCI/ > > More details about these crashes would be useful as we look at places > that *should* be able to handle errors like this. > makes sense, I could ,orthogonal to this, make the code more robust if we hit issues like this in the future. What I am mostly wondering about is, why pci core doesn't give up if the device doesn't come back from D3cold? It sounds like, that the most sane thing to do here is to just give up and fail runtime_resume and report errors back to userspace trying to make use of the devices. > > + * - sudden shutdowns, as the kernel identified an unrecoverable error > > after > > + *userspace tries to access the GPU. > > This doesn't fit with the others and more details might be > informative here as well. > yeah.. I try to get more infos on that. But at least for me (and it might be a distribution thing) if I execute lspci, the system shuts down, or at least tries to and might fail. > > + * In all cases dmesg will contain at least one line like this: > > + * 'nouveau :01:00.0: Refused to change power state, currently in D3' > > + * followed by a lot of nouveau timeouts. > > + * > > + * ACPI code writes bit 0x80 to the not
Re: [PATCH v4 12/13] [HACK] drm/bridge: lvds-codec: Enforce device specific compatible strings
Hi Fabrizio, On Tue, Nov 19, 2019 at 11:17:34AM +, Fabrizio Castro wrote: > On 19 November 2019 00:16 Laurent Pinchart wrote: > > On Wed, Nov 13, 2019 at 03:51:31PM +, Fabrizio Castro wrote: > > > The lvds-codec driver is a generic stub for transparent LVDS > > > encoders and decoders. > > > It's good practice to list a device specific compatible string > > > > s/good practice/mandatory/ > > Will fix > > > > before the generic fallback (if any) in the DT node for the relevant > > > LVDS encoder/decoder, and it's also required by the dt-bindings. > > > A notable exception to the generic fallback mechanism is the case > > > of "thine,thc63lvdm83d", as documented in: > > > Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt > > > This patch enforces the adoption of a device specific compatible > > > string (as fist string in the list), by using markers for the > > > > s/fist/first/ > > Well spotted > > > > > > compatible string we match against and the index of the matching > > > compatible string in the list. > > > > > > Signed-off-by: Fabrizio Castro > > > > > > --- > > > Hi Laurent, > > > > > > I don't think we need to do anything in the driver to address your > > > comment, as we can "enforce" this with the bindings (please see the > > > next patch, as it would help with the "enforcing" of the compatible > > > string for the thine device). > > > I am sending this patch only so that you can see what a possible > > > solution in the driver could look like. > > > > > > v3->v4: > > > * New patch addressing the below comment from Laurent: > > > "I think the lvds-decoder driver should error out at probe time if only > > > one compatible string is listed." > > > --- > > > drivers/gpu/drm/bridge/lvds-codec.c | 55 > > > + > > > 1 file changed, 49 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/lvds-codec.c > > > b/drivers/gpu/drm/bridge/lvds-codec.c > > > index 784bbd3..145c25d 100644 > > > --- a/drivers/gpu/drm/bridge/lvds-codec.c > > > +++ b/drivers/gpu/drm/bridge/lvds-codec.c > > > @@ -14,11 +14,16 @@ > > > #include > > > #include > > > > > > +struct lvds_codec_data { > > > + u32 connector_type; > > > + bool device_specific; > > > +}; > > > + > > > struct lvds_codec { > > > struct drm_bridge bridge; > > > struct drm_bridge *panel_bridge; > > > struct gpio_desc *powerdown_gpio; > > > - u32 connector_type; > > > + const struct lvds_codec_data *data; > > > }; > > > > > > static int lvds_codec_attach(struct drm_bridge *bridge) > > > @@ -65,7 +70,30 @@ static int lvds_codec_probe(struct platform_device > > > *pdev) > > > if (!lvds_codec) > > > return -ENOMEM; > > > > > > - lvds_codec->connector_type = (u32)of_device_get_match_data(>dev); > > > + lvds_codec->data = of_device_get_match_data(>dev); > > > + if (!lvds_codec->data) > > > + return -EINVAL; > > > + > > > + /* > > > + * If we haven't matched a device specific compatible string, we need > > > + * to work out if the generic compatible string we matched against was > > > + * listed first in the compatible property. > > > + */ > > > > Can't we do this unconditionally, and thus drop the lvds_codec_data > > structure ? > > I don't think so, and the reason for this is that we have a corner case for > thine,thc63lvdm83d. Here is what's allowed (according to the documentation) > from what's supported upstream (+ this series): > "ti,ds90c185", "lvds-encoder" > "ti,ds90c187", "lvds-encoder" > "ti,sn75lvds83", "lvds-encoder" > "ti,ds90cf384a", "lvds-decoder" > "thine,thc63lvdm83d" > > As you can see from the examples above, in most cases it's enough to say it's > all good when we match a compatible string with index > 0, but for the thine > device you _have_ to match the string with index 0 as that's what's currently > documented (please see thine,thc63lvdm83d.txt) and that's what's supported > by device trees already (please see arch/arm/boot/dts/r8a7779-marzen.dts). How about the following logic ? if (match_index("lvds-encoder") == 0 || match_index("lvds-decoder") == 0) return -EINVAL; ? > This patch "classifies" compatible strings, and it considers a good match > device specific compatible strings, or generic compatible strings as long > as they are not listed first. > > These days you can leverage the yaml files to validate the device trees, > therefore we should be focusing on writing yaml files in such a way we only > pass the checks we mean to, and by checks I mean: > make dtbs_check > > or more specifically, for this series: > make dtbs_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > and that's of course on top of make dt_binding_check. Sure, but that doesn't prevent anyone ignoring the validation. > It's a very common requirement to have a part number specific compatible > string first followed by a generic (fallback)
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
[+cc Dave] On Thu, Oct 17, 2019 at 02:19:01PM +0200, Karol Herbst wrote: > Fixes state transitions of Nvidia Pascal GPUs from D3cold into higher device > states. > > v2: convert to pci_dev quirk > put a proper technical explanation of the issue as a in-code comment > v3: disable it only for certain combinations of intel and nvidia hardware > v4: simplify quirk by setting flag on the GPU itself I have zero confidence that we understand the real problem, but we do need to do something with this. I'll merge it for v5.5 if we get the minor procedural stuff below straightened out. > Signed-off-by: Karol Herbst > Cc: Bjorn Helgaas > Cc: Lyude Paul > Cc: Rafael J. Wysocki > Cc: Mika Westerberg > Cc: linux-...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: nouv...@lists.freedesktop.org > --- > drivers/pci/pci.c| 7 ++ > drivers/pci/quirks.c | 53 > include/linux/pci.h | 1 + > 3 files changed, 61 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index b97d9e10c9cc..02e71e0bcdd7 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -850,6 +850,13 @@ static int pci_raw_set_power_state(struct pci_dev *dev, > pci_power_t state) > || (state == PCI_D2 && !dev->d2_support)) > return -EIO; > > + /* > + * check if we have a bad combination of bridge controller and nvidia > + * GPU, see quirk_broken_nv_runpm for more info Whitespace damage. Capitalized incorrectly (see other comments nearby). > + */ > + if (state != PCI_D0 && dev->broken_nv_runpm) > + return 0; > + > pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, ); > > /* > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 44c4ae1abd00..0006c9e37b6f 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -5268,3 +5268,56 @@ static void > quirk_reset_lenovo_thinkpad_p50_nvgpu(struct pci_dev *pdev) > DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > PCI_CLASS_DISPLAY_VGA, 8, > quirk_reset_lenovo_thinkpad_p50_nvgpu); > + > +/* > + * Some Intel PCIe bridges cause devices to disappear from the PCIe bus after > + * those were put into D3cold state if they were put into a non D0 PCI PM > + * device state before doing so. A device in D3cold is off the bus by definition. IIUC the problem is that the sequence D0 -> D3hot -> D3cold -> D0 for the GPU fails in the transition back to D0, while D0 -> D3cold -> D0 works fine. So I guess the problem is that we can put the device in D3cold with no problem, but if we put in D3hot before going to D3cold, the device never comes back to D0. Right? > + * This leads to various issue different issues which all manifest > differently, s/issue different// Actually, I think there's only one underlying issue with several manifestations. > + * but have the same root cause: > + * - AIML code execution hits an infinite loop (as the coe waits on device > + *memory to change). s/AIML/AML/ s/coe/code/ > + * - kernel crashes, as all pci reads return -1, which most code isn't able > + *to handle well enough. s/pci/PCI/ More details about these crashes would be useful as we look at places that *should* be able to handle errors like this. > + * - sudden shutdowns, as the kernel identified an unrecoverable error after > + *userspace tries to access the GPU. This doesn't fit with the others and more details might be informative here as well. > + * In all cases dmesg will contain at least one line like this: > + * 'nouveau :01:00.0: Refused to change power state, currently in D3' > + * followed by a lot of nouveau timeouts. > + * > + * ACPI code writes bit 0x80 to the not documented PCI register 0x248 of the > + * PCIe bridge controller in order to power down the GPU. > + * Nonetheless, there are other code paths inside the ACPI firmware which use > + * other registers, which seem to work fine: > + * - 0xbc bit 0x20 (publicly available documentation claims 'reserved') > + * - 0xb0 bit 0x10 (link disable) All these register addresses are device-specific, so they're useless without identifying the device. "lspci -vvnn" output would let us at least connect this with something. It would be nice to have that info archived along with your acpidump and python repro scripts in a bugzilla with the URL in the commit log. These are likely in PCI capabilities. If I make the leap of assuming the "link disable" bit is PCI_EXP_LNKCTL_LD, that would mean the Link Control register is at 0xb0 and the register at 0xbc would be the Root Control register, and indeed 0x20 in Root Control is reserved. I don't know what the relevance of all this is, though. It's not remarkable that accesses to these registers work. Unless you mean you can access these registers *after* trying to put the device back in D0, but other
[PATCH] drm/mcde: Support using DSI in LP mode
It is possible to set a flag in the struct mipi_dsi_device so the panel is handled in low power (LP) mode. Some displays only support this mode and it is also good for testing. Cc: Stephan Gerhold Signed-off-by: Linus Walleij --- drivers/gpu/drm/mcde/mcde_dsi.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c index 889c39efa80c..d4a12fe7ff01 100644 --- a/drivers/gpu/drm/mcde/mcde_dsi.c +++ b/drivers/gpu/drm/mcde/mcde_dsi.c @@ -648,10 +648,11 @@ static void mcde_dsi_start(struct mcde_dsi *d) /* Command mode, clear IF1 ID */ val = readl(d->regs + DSI_CMD_MODE_CTL); /* -* If we enable low-power mode here, with -* val |= DSI_CMD_MODE_CTL_IF1_LP_EN +* If we enable low-power mode here, * then display updates become really slow. */ + if (d->mdsi->mode_flags & MIPI_DSI_MODE_LPM) + val |= DSI_CMD_MODE_CTL_IF1_LP_EN; val &= ~DSI_CMD_MODE_CTL_IF1_ID_MASK; writel(val, d->regs + DSI_CMD_MODE_CTL); @@ -740,10 +741,11 @@ static void mcde_dsi_bridge_pre_enable(struct drm_bridge *bridge) /* Command mode, clear IF1 ID */ val = readl(d->regs + DSI_CMD_MODE_CTL); /* -* If we enable low-power mode here with -* val |= DSI_CMD_MODE_CTL_IF1_LP_EN +* If we enable low-power mode here * the display updates become really slow. */ + if (d->mdsi->mode_flags & MIPI_DSI_MODE_LPM) + val |= DSI_CMD_MODE_CTL_IF1_LP_EN; val &= ~DSI_CMD_MODE_CTL_IF1_ID_MASK; writel(val, d->regs + DSI_CMD_MODE_CTL); } -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/8] drm/xen: Simplify fb_create
On Fri, Nov 15, 2019 at 10:33:24AM +, Oleksandr Andrushchenko wrote: > On 11/15/19 11:21 AM, Daniel Vetter wrote: > > The current code is a pretty good wtf moment, since we drop the > > reference before we use it. It's not a big deal, because a) we only > > use the pointer, so doesn't blow up and the real reason b) fb->obj[0] > > already holds a full reference for us. > > > > Might as well take the real pointer ins't of complicated games that > > baffle. > > > > Signed-off-by: Daniel Vetter > > Cc: Oleksandr Andrushchenko > > Cc: xen-de...@lists.xenproject.org > Reviewed-by: Oleksandr Andrushchenko Thanks for taking a look, pushed to drm-misc-next. -Daniel > > --- > > drivers/gpu/drm/xen/xen_drm_front_kms.c | 9 + > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c > > b/drivers/gpu/drm/xen/xen_drm_front_kms.c > > index ff506bc99414..4f34c5208180 100644 > > --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c > > +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c > > @@ -63,14 +63,7 @@ fb_create(struct drm_device *dev, struct drm_file *filp, > > if (IS_ERR_OR_NULL(fb)) > > return fb; > > > > - gem_obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]); > > - if (!gem_obj) { > > - DRM_ERROR("Failed to lookup GEM object\n"); > > - ret = -ENOENT; > > - goto fail; > > - } > > - > > - drm_gem_object_put_unlocked(gem_obj); > > + gem_obj = fb->obj[0]; > > > > ret = xen_drm_front_fb_attach(drm_info->front_info, > > xen_drm_front_dbuf_to_cookie(gem_obj), -- 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/8] drm/tilcdc: Drop drm_gem_fb_create wrapper
On Fri, Nov 15, 2019 at 03:21:20PM +0200, Jyri Sarha wrote: > On 15/11/2019 11:21, Daniel Vetter wrote: > > Doesn't do anything. > > > > Signed-off-by: Daniel Vetter > > Cc: Jyri Sarha > > Cc: Tomi Valkeinen > > Acked-by: Jyri Sarha Thanks for taking a look, pushed to drm-misc-next. -Daniel > > > --- > > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 8 +--- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > index 2a9e67597375..a160880bea0a 100644 > > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > @@ -64,12 +64,6 @@ void tilcdc_module_cleanup(struct tilcdc_module *mod) > > > > static struct of_device_id tilcdc_of_match[]; > > > > -static struct drm_framebuffer *tilcdc_fb_create(struct drm_device *dev, > > - struct drm_file *file_priv, const struct drm_mode_fb_cmd2 > > *mode_cmd) > > -{ > > - return drm_gem_fb_create(dev, file_priv, mode_cmd); > > -} > > - > > static int tilcdc_atomic_check(struct drm_device *dev, > >struct drm_atomic_state *state) > > { > > @@ -140,7 +134,7 @@ static int tilcdc_commit(struct drm_device *dev, > > } > > > > static const struct drm_mode_config_funcs mode_config_funcs = { > > - .fb_create = tilcdc_fb_create, > > + .fb_create = drm_gem_fb_create, > > .atomic_check = tilcdc_atomic_check, > > .atomic_commit = tilcdc_commit, > > }; > > > > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- 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 2/8] drm/atmel: ditch fb_create wrapper
On Fri, Nov 15, 2019 at 10:33:24AM +0100, Boris Brezillon wrote: > On Fri, 15 Nov 2019 10:21:14 +0100 > Daniel Vetter wrote: > > > Spotted while looking through them all. > > > > Signed-off-by: Daniel Vetter > > Cc: Sam Ravnborg > > Cc: Boris Brezillon > > Acked-by: Boris Brezillon Merged, thanks for taking a look. -Daniel > > > Cc: Nicolas Ferre > > Cc: Alexandre Belloni > > Cc: Ludovic Desroches > > Cc: linux-arm-ker...@lists.infradead.org > > --- > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 8 +--- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > > b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > > index 92640298ad41..8dc917a1270b 100644 > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > > @@ -557,12 +557,6 @@ static irqreturn_t atmel_hlcdc_dc_irq_handler(int irq, > > void *data) > > return IRQ_HANDLED; > > } > > > > -static struct drm_framebuffer *atmel_hlcdc_fb_create(struct drm_device > > *dev, > > - struct drm_file *file_priv, const struct drm_mode_fb_cmd2 > > *mode_cmd) > > -{ > > - return drm_gem_fb_create(dev, file_priv, mode_cmd); > > -} > > - > > struct atmel_hlcdc_dc_commit { > > struct work_struct work; > > struct drm_device *dev; > > @@ -657,7 +651,7 @@ static int atmel_hlcdc_dc_atomic_commit(struct > > drm_device *dev, > > } > > > > static const struct drm_mode_config_funcs mode_config_funcs = { > > - .fb_create = atmel_hlcdc_fb_create, > > + .fb_create = drm_gem_fb_create, > > .atomic_check = drm_atomic_helper_check, > > .atomic_commit = atmel_hlcdc_dc_atomic_commit, > > }; > -- 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] drm/dsc: Return unsigned long on compute offset
On 19/11/2019 16:09, Mikita Lipski wrote: On 19/11/2019 12:11, Ville Syrjälä wrote: On Tue, Nov 19, 2019 at 04:59:40PM +, Cornij, Nikola wrote: If you're going to make all of them the same, then u64, please. This is because I'm not sure if calculations require 64-bit at some stage. If it does then it's already broken. Someone should probably figure out what's actally needed instead of shooting ducks with an icbm. Sorry made a type below. Supposed to be "I don't think it is broken" I don't think it is not broken, cause I'm currently testing DSC. The patch I sent early simply fixes the error of comparing signed and unsigned variables. We can then submit a second patch addressing the issue of using unsigned long int instead of u32. Also, since the variables in drm_dsc_config structure are all of type u8 and u16, the calculation values shouldn't exceed the size of u32. Thanks -Original Message- From: Lipski, Mikita Sent: November 19, 2019 10:08 AM To: Ville Syrjälä ; Lipski, Mikita Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Cornij, Nikola Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset On 19/11/2019 09:56, Ville Syrjälä wrote: On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote: From: Mikita Lipski We shouldn't compare int with unsigned long to find the max value and since we are not expecting negative value returned from compute_offset we should make this function return unsigned long so we can compare the values when computing rc parameters. Why are there other unsigned longs in dsc parameter computation in the first place? I believe it was initially set to be unsigned long for variable consistency, when we ported intel_compute_rc_parameters into drm_dsc_compute_rc_parameters. But now that I look at it, we can actually just set them to u32 or u64, as nothing should exceed that. Cc: Nikola Cornij Cc: Harry Wentland Signed-off-by: Mikita Lipski --- drivers/gpu/drm/drm_dsc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c index 74f3527f567d..ec40604ab6a2 100644 --- a/drivers/gpu/drm/drm_dsc.c +++ b/drivers/gpu/drm/drm_dsc.c @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload, } EXPORT_SYMBOL(drm_dsc_pps_payload_pack); -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group, +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group, int groups_per_line, int grpcnt) { - int offset = 0; - int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group); + unsigned long offset = 0; + unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group); if (grpcnt <= grpcnt_id) offset = DIV_ROUND_UP(grpcnt * pixels_per_group * vdsc_cfg->bits_per_pixel, 16); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Thanks, Mikita Lipski Software Engineer 2, AMD mikita.lip...@amd.com -- Thanks, Mikita Lipski Software Engineer 2, AMD mikita.lip...@amd.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] more dma-buf lockdep priming
Hi all, While looking at (dynamic) dma-buf issues and ideas I've spotted a bit more room for locking down the cross-driver dma_resv rules. Only functional fallout is a tiny patch for msm, assuming I didn't botch anything in the auditing of all relevant code. Comments, review and testing very much appreciated, as usual. Cheers, Daniel Daniel Vetter (3): drm/modeset: Prime modeset lock vs dma_resv dma-resv: Also prime acquire ctx for lockdep drm/msm: Don't init ww_mutec acquire ctx before needed drivers/dma-buf/dma-resv.c | 8 +++- drivers/gpu/drm/drm_mode_config.c| 28 drivers/gpu/drm/msm/msm_gem_submit.c | 2 +- 3 files changed, 36 insertions(+), 2 deletions(-) -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dsc: Return unsigned long on compute offset
On 19/11/2019 12:11, Ville Syrjälä wrote: On Tue, Nov 19, 2019 at 04:59:40PM +, Cornij, Nikola wrote: If you're going to make all of them the same, then u64, please. This is because I'm not sure if calculations require 64-bit at some stage. If it does then it's already broken. Someone should probably figure out what's actally needed instead of shooting ducks with an icbm. I don't think it is not broken, cause I'm currently testing DSC. The patch I sent early simply fixes the error of comparing signed and unsigned variables. We can then submit a second patch addressing the issue of using unsigned long int instead of u32. Also, since the variables in drm_dsc_config structure are all of type u8 and u16, the calculation values shouldn't exceed the size of u32. Thanks -Original Message- From: Lipski, Mikita Sent: November 19, 2019 10:08 AM To: Ville Syrjälä ; Lipski, Mikita Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Cornij, Nikola Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset On 19/11/2019 09:56, Ville Syrjälä wrote: On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote: From: Mikita Lipski We shouldn't compare int with unsigned long to find the max value and since we are not expecting negative value returned from compute_offset we should make this function return unsigned long so we can compare the values when computing rc parameters. Why are there other unsigned longs in dsc parameter computation in the first place? I believe it was initially set to be unsigned long for variable consistency, when we ported intel_compute_rc_parameters into drm_dsc_compute_rc_parameters. But now that I look at it, we can actually just set them to u32 or u64, as nothing should exceed that. Cc: Nikola Cornij Cc: Harry Wentland Signed-off-by: Mikita Lipski --- drivers/gpu/drm/drm_dsc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c index 74f3527f567d..ec40604ab6a2 100644 --- a/drivers/gpu/drm/drm_dsc.c +++ b/drivers/gpu/drm/drm_dsc.c @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload, } EXPORT_SYMBOL(drm_dsc_pps_payload_pack); -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group, +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group, int groups_per_line, int grpcnt) { - int offset = 0; - int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group); + unsigned long offset = 0; + unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group); if (grpcnt <= grpcnt_id) offset = DIV_ROUND_UP(grpcnt * pixels_per_group * vdsc_cfg->bits_per_pixel, 16); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Thanks, Mikita Lipski Software Engineer 2, AMD mikita.lip...@amd.com -- Thanks, Mikita Lipski Software Engineer 2, AMD mikita.lip...@amd.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] dma-resv: Also prime acquire ctx for lockdep
Semnatically it really doesn't matter where we grab the ticket. But since the ticket is a fake lockdep lock, it matters for lockdep validation purposes. This means stuff like grabbing a ticket and then doing copy_from/to_user isn't allowed anymore. This is a changed compared to the current ttm fault handler, which doesn't bother with having a full reservation. Since I'm looking into fixing the TODO entry in ttm_mem_evict_wait_busy() I think that'll have to change sooner or later anyway, better get started. A bit more context on why I'm looking into this: For backwards compat with existing i915 gem code I think we'll have to do full slowpath locking in the i915 equivalent of the eviction code. And with dynamic dma-buf that will leak across drivers, so another thing we need to standardize and make sure it's done the same way everyway. Unfortunately this means another full audit of all drivers: - gem helpers: acquire_init is done right before taking locks, so no problem. Same for acquire_fini and unlocking, which means nothing that's not already covered by the dma_resv_lock rules will be caught with this extension here to the acquire_ctx. - etnaviv: An absolute massive amount of code is run between the acquire_init and the first lock acquisition in submit_lock_objects. But nothing that would touch user memory and could cause a fault. Furthermore nothing that uses the ticket, so even if I missed something, it would be easy to fix by pushing the acquire_init right before the first use. Similar on the unlock/acquire_fini side. - i915: Right now (and this will likely change a lot rsn) the acquire ctx and actual locks are right next to each another. No problem. - msm has a problem: submit_create calls acquire_init, but then submit_lookup_objects() has a bunch of copy_from_user to do the object lookups. That's the only thing before submit_lock_objects call dma_resv_lock(). Despite all the copypasta to etnaviv, etnaviv does not have this issue since it copies all the userspace structs earlier. submit_cleanup does not have any such issues. With the prep patch to pull out the acquire_ctx and reorder it msm is going to be safe too. - nouveau: acquire_init is right next to ttm_bo_reserve, so all good. Similar on the acquire_fini/ttm_bo_unreserve side. - ttm execbuf utils: acquire context and locking are even in the same functions here (one function to reserve everything, the other to unreserve), so all good. - vc4: Another case where acquire context and locking are handled in the same functions (one function to lock everything, the other to unlock). Cc: Maarten Lankhorst Cc: Chris Wilson Cc: Christian König Cc: Sumit Semwal Cc: linux-me...@vger.kernel.org Cc: linaro-mm-...@lists.linaro.org Cc: Huang Rui Cc: Eric Anholt Cc: Ben Skeggs Cc: Alex Deucher Cc: Rob Herring Cc: Lucas Stach Cc: Russell King Cc: Christian Gmeiner Cc: Rob Clark Cc: Sean Paul Signed-off-by: Daniel Vetter --- drivers/dma-buf/dma-resv.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index d3c760e19991..079e38fde33a 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -100,7 +100,9 @@ static void dma_resv_list_free(struct dma_resv_list *list) static void __init dma_resv_lockdep(void) { struct mm_struct *mm = mm_alloc(); + struct ww_acquire_ctx ctx; struct dma_resv obj; + int ret; if (!mm) return; @@ -108,10 +110,14 @@ static void __init dma_resv_lockdep(void) dma_resv_init(); down_read(>mmap_sem); - ww_mutex_lock(, NULL); + ww_acquire_init(, _ww_class); + ret = dma_resv_lock(, ); + if (ret == -EDEADLK) + dma_resv_lock_slow(, ); fs_reclaim_acquire(GFP_KERNEL); fs_reclaim_release(GFP_KERNEL); ww_mutex_unlock(); + ww_acquire_fini(); up_read(>mmap_sem); mmput(mm); -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/msm: Don't init ww_mutec acquire ctx before needed
For locking semantics it really doesn't matter when we grab the ticket. But for lockdep validation it does: the acquire ctx is a fake lockdep. Since other drivers might want to do a full multi-lock dance in their fault-handler, not just lock a single dma_resv. Therefore we must init the acquire_ctx only after we've done all the copy_*_user or anything else that might trigger a pagefault. For msm this means we need to move it past submit_lookup_objects. Aside: Why is msm still using struct_mutex, it seems to be using dma_resv_lock for general buffer state protection? Signed-off-by: Daniel Vetter Cc: Rob Clark Cc: Sean Paul Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org --- drivers/gpu/drm/msm/msm_gem_submit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index fb1fdd7fa3ef..126b2f62bfe7 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -54,7 +54,6 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, INIT_LIST_HEAD(>node); INIT_LIST_HEAD(>bo_list); - ww_acquire_init(>ticket, _ww_class); return submit; } @@ -489,6 +488,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, if (ret) goto out; + ww_acquire_init(>ticket, _ww_class); ret = submit_lock_objects(submit); if (ret) goto out; -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/modeset: Prime modeset lock vs dma_resv
It's kinda really hard to get this wrong on a driver with both display and dma_resv locking. But who ever knows, so better to make sure that really all drivers nest these the same way. For actual lock semantics the acquire context nesting doesn't matter. But to teach lockdep what's going on with ww_mutex the acquire ctx is a fake lockdep lock, hence from a lockdep pov it does matter. That's why I figured better to include it. Cc: Maarten Lankhorst Cc: Chris Wilson Cc: Christian König Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_mode_config.c | 28 1 file changed, 28 insertions(+) diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 3b570a404933..08e6eff6a179 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -27,6 +27,7 @@ #include #include #include +#include #include "drm_crtc_internal.h" #include "drm_internal.h" @@ -415,6 +416,33 @@ void drm_mode_config_init(struct drm_device *dev) dev->mode_config.num_crtc = 0; dev->mode_config.num_encoder = 0; dev->mode_config.num_total_plane = 0; + + if (IS_ENABLED(CONFIG_LOCKDEP)) { + struct drm_modeset_acquire_ctx modeset_ctx; + struct ww_acquire_ctx resv_ctx; + struct dma_resv resv; + int ret; + + dma_resv_init(); + + drm_modeset_acquire_init(_ctx, 0); + ret = drm_modeset_lock(>mode_config.connection_mutex, + _ctx); + if (ret == -EDEADLK) + ret = drm_modeset_backoff(_ctx); + + ww_acquire_init(_ctx, _ww_class); + ret = dma_resv_lock(, _ctx); + if (ret == -EDEADLK) + dma_resv_lock_slow(, _ctx); + + dma_resv_unlock(); + ww_acquire_fini(_ctx); + + drm_modeset_drop_locks(_ctx); + drm_modeset_acquire_fini(_ctx); + dma_resv_fini(); + } } EXPORT_SYMBOL(drm_mode_config_init); -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/gma500: Fixup fbdev stolen size usage evaluation
On Tue, Nov 19, 2019 at 3:11 PM Paul Kocialkowski wrote: > > Hi, > > On Wed 13 Nov 19, 11:04, Patrik Jakobsson wrote: > > On Tue, Nov 12, 2019 at 4:50 PM Paul Kocialkowski > > wrote: > > > > > > Hi, > > > > > > On Tue 12 Nov 19, 16:11, Paul Kocialkowski wrote: > > > > Hi, > > > > > > > > On Tue 12 Nov 19, 11:20, Patrik Jakobsson wrote: > > > > > On Thu, Nov 7, 2019 at 4:30 PM Paul Kocialkowski > > > > > wrote: > > > > > > > > > > > > psbfb_probe performs an evaluation of the required size from the > > > > > > stolen > > > > > > GTT memory, but gets it wrong in two distinct ways: > > > > > > - The resulting size must be page-size-aligned; > > > > > > - The size to allocate is derived from the surface dimensions, not > > > > > > the fb > > > > > > dimensions. > > > > > > > > > > > > When two connectors are connected with different modes, the > > > > > > smallest will > > > > > > be stored in the fb dimensions, but the size that needs to be > > > > > > allocated must > > > > > > match the largest (surface) dimensions. This is what is used in the > > > > > > actual > > > > > > allocation code. > > > > > > > > > > > > Fix this by correcting the evaluation to conform to the two points > > > > > > above. > > > > > > It allows correctly switching to 16bpp when one connector is e.g. > > > > > > 1920x1080 > > > > > > and the other is 1024x768. > > > > > > > > > > > > Signed-off-by: Paul Kocialkowski > > > > > > --- > > > > > > drivers/gpu/drm/gma500/framebuffer.c | 8 ++-- > > > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/gma500/framebuffer.c > > > > > > b/drivers/gpu/drm/gma500/framebuffer.c > > > > > > index 218f3bb15276..90237abee088 100644 > > > > > > --- a/drivers/gpu/drm/gma500/framebuffer.c > > > > > > +++ b/drivers/gpu/drm/gma500/framebuffer.c > > > > > > @@ -462,6 +462,7 @@ static int psbfb_probe(struct drm_fb_helper > > > > > > *helper, > > > > > > container_of(helper, struct psb_fbdev, > > > > > > psb_fb_helper); > > > > > > struct drm_device *dev = psb_fbdev->psb_fb_helper.dev; > > > > > > struct drm_psb_private *dev_priv = dev->dev_private; > > > > > > + unsigned int fb_size; > > > > > > int bytespp; > > > > > > > > > > > > bytespp = sizes->surface_bpp / 8; > > > > > > @@ -471,8 +472,11 @@ static int psbfb_probe(struct drm_fb_helper > > > > > > *helper, > > > > > > /* If the mode will not fit in 32bit then switch to 16bit > > > > > > to get > > > > > >a console on full resolution. The X mode setting server > > > > > > will > > > > > >allocate its own 32bit GEM framebuffer */ > > > > > > - if (ALIGN(sizes->fb_width * bytespp, 64) * sizes->fb_height > > > > > > > > > > > > > - dev_priv->vram_stolen_size) { > > > > > > + fb_size = ALIGN(sizes->surface_width * bytespp, 64) * > > > > > > + sizes->surface_height; > > > > > > + fb_size = ALIGN(fb_size, PAGE_SIZE); > > > > > > + > > > > > > + if (fb_size > dev_priv->vram_stolen_size) { > > > > > > > > > > psb_gtt_alloc_range() already aligns by PAGE_SIZE for us. Looks like > > > > > we align a couple of times extra for luck. This needs cleaning up > > > > > instead of adding even more aligns. > > > > > > > > I'm not sure this is really for luck. As far as I can see, we need to > > > > do it > > > > properly for this size estimation since it's the final size that will be > > > > allocated (and thus needs to be available in whole). > > > > Ok now I understand what you meant. Actually vram_stolen_size is > > always page aligned so fb_size doesn't need any page alignment here. > > I'm a bit confused here, what about the case where: > unaligned fb_size < dev_priv->vram_stolen_size but > aligned fb_size > dev_priv->vram_stolen_size ? That can never happen since aligning fb_size will never cross a page boundary, and stolen_size is always on a page boundary. Not sure how to explain it on a good way but: If stolen_size = 4096, then fb_size is only more than stolen_size if it is actually > 4096 regardless if we align it or not. > > Granted, it's a corner case, but I don't follow the logic of comparing aligned > and unaligned sizes: it feels a bit like comparing two values of different > units. We can keep it if you think it makes the code more readable. > > > There is also no need to align for psbfb_create() since it also takes > > care of this. > > > > > > > > > > For the other times there is explicit alignment, they seem justified > > > > too: > > > > - in psb_gem_create: it is common to pass the aligned size when > > > > creating the > > > > associated GEM object with drm_gem_object_init, even though it's > > > > probably not > > > > crucial given that this is not where allocation actually happens; > > > > - in psbfb_create: the full size is apparently only really used to > > > > memset 0 > > > > the allocated buffer. I
Re: [PATCH v4 1/2] dt-bindings: pwm: Convert PWM bindings to json-schema
On Mon, Oct 21, 2019 at 06:02:06PM +0200, Krzysztof Kozlowski wrote: > Convert generic PWM controller bindings to DT schema format using > json-schema. The consumer bindings are provided by dt-schema. > > Signed-off-by: Krzysztof Kozlowski > Acked-by: Stephen Boyd > Acked-by: Paul Walmsley Looks like I missed this one somehow. I've applied the series now. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, 17 Oct 2019 at 22:19, Karol Herbst wrote: > > Fixes state transitions of Nvidia Pascal GPUs from D3cold into higher device > states. Can we get this acked/committed? At this stage I think we've done all we can unless Intel actually escalate this internally and work out how the hw is broken. Dave. > > v2: convert to pci_dev quirk > put a proper technical explanation of the issue as a in-code comment > v3: disable it only for certain combinations of intel and nvidia hardware > v4: simplify quirk by setting flag on the GPU itself > > Signed-off-by: Karol Herbst > Cc: Bjorn Helgaas > Cc: Lyude Paul > Cc: Rafael J. Wysocki > Cc: Mika Westerberg > Cc: linux-...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: nouv...@lists.freedesktop.org > --- > drivers/pci/pci.c| 7 ++ > drivers/pci/quirks.c | 53 > include/linux/pci.h | 1 + > 3 files changed, 61 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index b97d9e10c9cc..02e71e0bcdd7 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -850,6 +850,13 @@ static int pci_raw_set_power_state(struct pci_dev *dev, > pci_power_t state) >|| (state == PCI_D2 && !dev->d2_support)) > return -EIO; > > + /* > +* check if we have a bad combination of bridge controller and nvidia > + * GPU, see quirk_broken_nv_runpm for more info > +*/ > + if (state != PCI_D0 && dev->broken_nv_runpm) > + return 0; > + > pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, ); > > /* > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 44c4ae1abd00..0006c9e37b6f 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -5268,3 +5268,56 @@ static void > quirk_reset_lenovo_thinkpad_p50_nvgpu(struct pci_dev *pdev) > DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > PCI_CLASS_DISPLAY_VGA, 8, > quirk_reset_lenovo_thinkpad_p50_nvgpu); > + > +/* > + * Some Intel PCIe bridges cause devices to disappear from the PCIe bus after > + * those were put into D3cold state if they were put into a non D0 PCI PM > + * device state before doing so. > + * > + * This leads to various issue different issues which all manifest > differently, > + * but have the same root cause: > + * - AIML code execution hits an infinite loop (as the coe waits on device > + *memory to change). > + * - kernel crashes, as all pci reads return -1, which most code isn't able > + *to handle well enough. > + * - sudden shutdowns, as the kernel identified an unrecoverable error after > + *userspace tries to access the GPU. > + * > + * In all cases dmesg will contain at least one line like this: > + * 'nouveau :01:00.0: Refused to change power state, currently in D3' > + * followed by a lot of nouveau timeouts. > + * > + * ACPI code writes bit 0x80 to the not documented PCI register 0x248 of the > + * PCIe bridge controller in order to power down the GPU. > + * Nonetheless, there are other code paths inside the ACPI firmware which use > + * other registers, which seem to work fine: > + * - 0xbc bit 0x20 (publicly available documentation claims 'reserved') > + * - 0xb0 bit 0x10 (link disable) > + * Changing the conditions inside the firmware by poking into the relevant > + * addresses does resolve the issue, but it seemed to be ACPI private memory > + * and not any device accessible memory at all, so there is no portable way > of > + * changing the conditions. > + * > + * The only systems where this behavior can be seen are hybrid graphics > laptops > + * with a secondary Nvidia Pascal GPU. It cannot be ruled out that this issue > + * only occurs in combination with listed Intel PCIe bridge controllers and > + * the mentioned GPUs or if it's only a hw bug in the bridge controller. > + * > + * But because this issue was NOT seen on laptops with an Nvidia Pascal GPU > + * and an Intel Coffee Lake SoC, there is a higher chance of there being a > bug > + * in the bridge controller rather than in the GPU. > + * > + * This issue was not able to be reproduced on non laptop systems. > + */ > + > +static void quirk_broken_nv_runpm(struct pci_dev *dev) > +{ > + struct pci_dev *bridge = pci_upstream_bridge(dev); > + > + if (bridge->vendor == PCI_VENDOR_ID_INTEL && > + bridge->device == 0x1901) > + dev->broken_nv_runpm = 1; > +} > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, > + PCI_BASE_CLASS_DISPLAY, 16, > + quirk_broken_nv_runpm); > diff --git a/include/linux/pci.h b/include/linux/pci.h > index ac8a6c4e1792..903a0b3a39ec 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -416,6 +416,7 @@ struct pci_dev { > unsigned int__aer_firmware_first_valid:1; >
[Bug 205049] garbled graphics
https://bugzilla.kernel.org/show_bug.cgi?id=205049 Pierre-Eric Pelloux-Prayer (pierre-eric.pelloux-pra...@amd.com) changed: What|Removed |Added CC||pierre-eric.pelloux-prayer@ ||amd.com --- Comment #14 from Pierre-Eric Pelloux-Prayer (pierre-eric.pelloux-pra...@amd.com) --- This bug looks similar to https://bugs.freedesktop.org/show_bug.cgi?id=22 - so indeed using R600_DEBUG=nodcc might be a workaround. -- 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: [PATCH v3 12/14] drm/amdgpu: Use mmu_interval_notifier instead of hmm_mirror
I test v3 and it works fine. Regards, Philip On 2019-11-12 3:22 p.m., Jason Gunthorpe wrote: From: Jason Gunthorpe Convert the collision-retry lock around hmm_range_fault to use the one now provided by the mmu_interval notifier. Although this driver does not seem to use the collision retry lock that hmm provides correctly, it can still be converted over to use the mmu_interval_notifier api instead of hmm_mirror without too much trouble. This also deletes another place where a driver is associating additional data (struct amdgpu_mn) with a mmu_struct. Signed-off-by: Philip Yang Reviewed-by: Philip Yang Tested-by: Philip Yang Signed-off-by: Jason Gunthorpe --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 + drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 14 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c| 148 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h| 49 -- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 116 -- 5 files changed, 94 insertions(+), 237 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 47700302a08b7f..1bcedb9b477dce 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -1738,6 +1738,10 @@ static int update_invalid_user_pages(struct amdkfd_process_info *process_info, return ret; } + /* +* FIXME: Cannot ignore the return code, must hold +* notifier_lock +*/ amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm); /* Mark the BO as valid unless it was invalidated diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 82823d9a8ba887..22c989bca7514c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -603,8 +603,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, e->tv.num_shared = 2; amdgpu_bo_list_get_list(p->bo_list, >validated); - if (p->bo_list->first_userptr != p->bo_list->num_entries) - p->mn = amdgpu_mn_get(p->adev, AMDGPU_MN_TYPE_GFX); INIT_LIST_HEAD(); amdgpu_vm_get_pd_bo(>vm, >validated, >vm_pd); @@ -1287,11 +1285,11 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, if (r) goto error_unlock; - /* No memory allocation is allowed while holding the mn lock. -* p->mn is hold until amdgpu_cs_submit is finished and fence is added -* to BOs. + /* No memory allocation is allowed while holding the notifier lock. +* The lock is held until amdgpu_cs_submit is finished and fence is +* added to BOs. */ - amdgpu_mn_lock(p->mn); + mutex_lock(>adev->notifier_lock); /* If userptr are invalidated after amdgpu_cs_parser_bos(), return * -EAGAIN, drmIoctl in libdrm will restart the amdgpu_cs_ioctl. @@ -1334,13 +1332,13 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, amdgpu_vm_move_to_lru_tail(p->adev, >vm); ttm_eu_fence_buffer_objects(>ticket, >validated, p->fence); - amdgpu_mn_unlock(p->mn); + mutex_unlock(>adev->notifier_lock); return 0; error_abort: drm_sched_job_cleanup(>base); - amdgpu_mn_unlock(p->mn); + mutex_unlock(>adev->notifier_lock); error_unlock: amdgpu_job_free(job); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index 9fe1c31ce17a30..828b5167ff128f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -50,28 +50,6 @@ #include "amdgpu.h" #include "amdgpu_amdkfd.h" -/** - * amdgpu_mn_lock - take the write side lock for this notifier - * - * @mn: our notifier - */ -void amdgpu_mn_lock(struct amdgpu_mn *mn) -{ - if (mn) - down_write(>lock); -} - -/** - * amdgpu_mn_unlock - drop the write side lock for this notifier - * - * @mn: our notifier - */ -void amdgpu_mn_unlock(struct amdgpu_mn *mn) -{ - if (mn) - up_write(>lock); -} - /** * amdgpu_mn_invalidate_gfx - callback to notify about mm change * @@ -94,6 +72,9 @@ static bool amdgpu_mn_invalidate_gfx(struct mmu_interval_notifier *mni, return false; mutex_lock(>notifier_lock); + + mmu_interval_set_seq(mni, cur_seq); + r = dma_resv_wait_timeout_rcu(bo->tbo.base.resv, true, false, MAX_SCHEDULE_TIMEOUT); mutex_unlock(>notifier_lock); @@ -127,6 +108,9 @@ static bool amdgpu_mn_invalidate_hsa(struct mmu_interval_notifier *mni, return false; mutex_lock(>notifier_lock); + + mmu_interval_set_seq(mni, cur_seq); + amdgpu_amdkfd_evict_userptr(bo->kfd_bo, bo->notifier.mm); mutex_unlock(>notifier_lock); @@
Re: [PATCH] drm/crtc-helper: drm_connector_get_single_encoder prototype is missing
On Tue, 2019-11-19 at 13:58 +0100, Benjamin Gaignard wrote: > Include drm_crtc_helper_internal.h to provide > drm_connector_get_single_encoder > prototype. > > Fixes: a92462d6bf493 ("drm/connector: Share with non-atomic drivers > the function to get the single encoder") drm_connector_get_single_encoder() is implemented before the use in this file so it is not broken, no need of a fixes tag. Reviewed-by: José Roberto de Souza > > Cc: José Roberto de Souza > > Signed-off-by: Benjamin Gaignard > --- > drivers/gpu/drm/drm_crtc_helper.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > b/drivers/gpu/drm/drm_crtc_helper.c > index 499b05aaccfc..93a4eec429e8 100644 > --- a/drivers/gpu/drm/drm_crtc_helper.c > +++ b/drivers/gpu/drm/drm_crtc_helper.c > @@ -48,6 +48,8 @@ > #include > #include > > +#include "drm_crtc_helper_internal.h" > + > /** > * DOC: overview > * ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/4] drm: imx: Add i.MX 6 MIPI DSI host driver
Hi Adrian, On Mon, Nov 18, 2019 at 12:25 PM Adrian Ratiu wrote: Some nitpicks: > + > +config DRM_IMX_MIPI_DSI > + tristate "Freescale i.MX DRM MIPI DSI" This text seems too generic as there are i.MX SoCs that use different MIPI DSI IP. Maybe "Freescale i.MX6 DRM MIPI DSI" instead? > +module_platform_driver(imx_mipi_dsi_driver); > + > +MODULE_DESCRIPTION("i.MX MIPI DSI host controller driver"); i.MX6 MIPI DSI, please. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205049] garbled graphics
https://bugzilla.kernel.org/show_bug.cgi?id=205049 --- Comment #13 from Alex Deucher (alexdeuc...@gmail.com) --- I'm not able to reproduce this on my raven systems. Could be a mesa issue. Does setting the R600_DEBUG=nodcc environment variable help? -- 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 205049] garbled graphics
https://bugzilla.kernel.org/show_bug.cgi?id=205049 --- Comment #12 from le...@onet.pl --- Is anyone actually working on this problem? I just upgraded from Fedora 30 to 31, hoping that the upgrade might include other miscellaneous changes that fix the problem, but the problem is EXACTLY THE SAME. Nothing has changed. I'm starting to lose my mind with this chronic lack of progress. Support for the Lenovo Ideapad 330 has been been severely broken for at least the last 6-7 kernel versions! Can you bisect the people supposedly working on the kernel??? -- 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: [PATCH v1] msm:disp:dpu1: add support for display for SC7180 target
Quoting Kalyan Thota (2019-11-18 02:47:43) > Add display hw catalog changes for SC7180 target. > > Changes in v1: > > 1) Configure register offsets and capabilities for the > display hw blocks. > > This patch has dependency on the below series > > https://patchwork.kernel.org/patch/11243111/ > > Signed-off-by: Kalyan Thota > Signed-off-by: Shubhashree Dhar > Signed-off-by: Raviteja Tamatam Your signoff chain looks wrong. Probably should have some Co-developed-by tags here, and then your SoB should be last. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dsc: Return unsigned long on compute offset
On Tue, Nov 19, 2019 at 04:59:40PM +, Cornij, Nikola wrote: > If you're going to make all of them the same, then u64, please. > > This is because I'm not sure if calculations require 64-bit at some stage. If it does then it's already broken. Someone should probably figure out what's actally needed instead of shooting ducks with an icbm. > > -Original Message- > From: Lipski, Mikita > Sent: November 19, 2019 10:08 AM > To: Ville Syrjälä ; Lipski, Mikita > > Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Cornij, > Nikola > Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset > > > > On 19/11/2019 09:56, Ville Syrjälä wrote: > > On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote: > >> From: Mikita Lipski > >> > >> We shouldn't compare int with unsigned long to find the max value and > >> since we are not expecting negative value returned from > >> compute_offset we should make this function return unsigned long so > >> we can compare the values when computing rc parameters. > > > > Why are there other unsigned longs in dsc parameter computation in the > > first place? > > I believe it was initially set to be unsigned long for variable consistency, > when we ported intel_compute_rc_parameters into > drm_dsc_compute_rc_parameters. But now that I look at it, we can actually > just set them to u32 or u64, as nothing should exceed that. > > > >> > >> Cc: Nikola Cornij > >> Cc: Harry Wentland > >> Signed-off-by: Mikita Lipski > >> --- > >> drivers/gpu/drm/drm_dsc.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c > >> index 74f3527f567d..ec40604ab6a2 100644 > >> --- a/drivers/gpu/drm/drm_dsc.c > >> +++ b/drivers/gpu/drm/drm_dsc.c > >> @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct > >> drm_dsc_picture_parameter_set *pps_payload, > >> } > >> EXPORT_SYMBOL(drm_dsc_pps_payload_pack); > >> > >> -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int > >> pixels_per_group, > >> +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int > >> pixels_per_group, > >>int groups_per_line, int grpcnt) > >> { > >> - int offset = 0; > >> - int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, > >> pixels_per_group); > >> + unsigned long offset = 0; > >> + unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, > >> pixels_per_group); > >> > >>if (grpcnt <= grpcnt_id) > >>offset = DIV_ROUND_UP(grpcnt * pixels_per_group * > >> vdsc_cfg->bits_per_pixel, 16); > >> -- > >> 2.17.1 > >> > >> ___ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Thanks, > Mikita Lipski > Software Engineer 2, AMD > mikita.lip...@amd.com -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: power: Fix path to power-domain.txt bindings
On Tue, Nov 19, 2019 at 8:43 AM Krzysztof Kozlowski wrote: > > With split of power domain controller bindings to power-domain.yaml, the > consumer part was renamed to power-domain.txt. Update the references in > other bindings. > > Reported-by: Geert Uytterhoeven > Fixes: abb4805e343a ("dt-bindings: power: Convert Samsung Exynos Power Domain > bindings to json-schema") > Signed-off-by: Krzysztof Kozlowski > --- > Documentation/devicetree/bindings/clock/clk-exynos-audss.txt | 2 +- > Documentation/devicetree/bindings/clock/exynos5433-clock.txt | 2 +- > .../devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt | 2 +- > .../devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt | 2 +- > .../bindings/clock/renesas,rcar-gen2-cpg-clocks.txt | 2 +- > .../devicetree/bindings/clock/renesas,rz-cpg-clocks.txt | 2 +- > .../devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 2 +- > Documentation/devicetree/bindings/display/msm/dpu.txt | 2 +- > Documentation/devicetree/bindings/display/msm/mdp5.txt| 2 +- > Documentation/devicetree/bindings/dsp/fsl,dsp.yaml| 2 +- > Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt| 2 +- > .../devicetree/bindings/media/mediatek-jpeg-decoder.txt | 2 +- > Documentation/devicetree/bindings/media/mediatek-mdp.txt | 2 +- > Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 2 +- > Documentation/devicetree/bindings/pci/pci-keystone.txt| 2 +- > Documentation/devicetree/bindings/phy/ti,phy-am654-serdes.txt | 2 +- > Documentation/devicetree/bindings/power/qcom,rpmpd.txt| 2 +- > Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt | 2 +- > .../devicetree/bindings/usb/nvidia,tegra124-xusb.txt | 4 ++-- > 19 files changed, 20 insertions(+), 20 deletions(-) Please no. Can you just undo the renaming back to power_domain.txt Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm/dsc: Return unsigned long on compute offset
If you're going to make all of them the same, then u64, please. This is because I'm not sure if calculations require 64-bit at some stage. -Original Message- From: Lipski, Mikita Sent: November 19, 2019 10:08 AM To: Ville Syrjälä ; Lipski, Mikita Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Cornij, Nikola Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset On 19/11/2019 09:56, Ville Syrjälä wrote: > On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote: >> From: Mikita Lipski >> >> We shouldn't compare int with unsigned long to find the max value and >> since we are not expecting negative value returned from >> compute_offset we should make this function return unsigned long so >> we can compare the values when computing rc parameters. > > Why are there other unsigned longs in dsc parameter computation in the > first place? I believe it was initially set to be unsigned long for variable consistency, when we ported intel_compute_rc_parameters into drm_dsc_compute_rc_parameters. But now that I look at it, we can actually just set them to u32 or u64, as nothing should exceed that. > >> >> Cc: Nikola Cornij >> Cc: Harry Wentland >> Signed-off-by: Mikita Lipski >> --- >> drivers/gpu/drm/drm_dsc.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c >> index 74f3527f567d..ec40604ab6a2 100644 >> --- a/drivers/gpu/drm/drm_dsc.c >> +++ b/drivers/gpu/drm/drm_dsc.c >> @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct >> drm_dsc_picture_parameter_set *pps_payload, >> } >> EXPORT_SYMBOL(drm_dsc_pps_payload_pack); >> >> -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int >> pixels_per_group, >> +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int >> pixels_per_group, >> int groups_per_line, int grpcnt) >> { >> -int offset = 0; >> -int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, >> pixels_per_group); >> +unsigned long offset = 0; >> +unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, >> pixels_per_group); >> >> if (grpcnt <= grpcnt_id) >> offset = DIV_ROUND_UP(grpcnt * pixels_per_group * >> vdsc_cfg->bits_per_pixel, 16); >> -- >> 2.17.1 >> >> ___ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thanks, Mikita Lipski Software Engineer 2, AMD mikita.lip...@amd.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/i915: DSI: select correct PWM controller to use based on the VBT
Hi, On 19-11-2019 16:47, Ville Syrjälä wrote: On Tue, Nov 19, 2019 at 04:18:18PM +0100, Hans de Goede wrote: At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2 different PWM controllers for controlling the LCD's backlight brightness. Either the one integrated into the PMIC or the one integrated into the SoC (the 1st LPSS PWM controller). So far in the LPSS code on BYT we have skipped registering the LPSS PWM controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is present, assuming that in this case the PMIC PWM controller will be used. On CHT we have been relying on only 1 of the 2 PWM controllers being enabled in the DSDT at the same time; and always registered the lookup. So far this has been working, but the correct way to determine which PWM controller needs to be used is by checking a bit in the VBT table and recently I've learned about 2 different BYT devices: Point of View MOBII TAB-P800W Acer Switch 10 SW5-012 Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS PWM controller (and the VBT correctly indicates this), so here our old heuristics fail. This commit fixes using the wrong PWM controller on these devices by calling pwm_get() for the right PWM controller based on the VBT dsi.config.pwm_blc bit. Note this is part of a series which contains 2 other patches which renames the PWM lookup for the 1st SoC/LPSS PWM from "pwm_backlight" to "pwm_pmic_backlight" and the PWM lookup for the Crystal Cove PMIC PWM from "pwm_backlight" to "pwm_pmic_backlight". Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/display/intel_panel.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index bc14e9c0285a..ddcf311d1114 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -1840,13 +1840,22 @@ static int pwm_setup_backlight(struct intel_connector *connector, enum pipe pipe) { struct drm_device *dev = connector->base.dev; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_panel *panel = >panel; + const char *desc; int retval; - /* Get the PWM chip for backlight control */ - panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight"); + /* Get the right PWM chip for DSI backlight according to VBT */ + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) { + panel->backlight.pwm = pwm_get(dev->dev, "pwm_pmic_backlight"); + desc = "PMIC"; + } else { + panel->backlight.pwm = pwm_get(dev->dev, "pwm_soc_backlight"); + desc = "SoC"; + } Might we want the same thing for the panel enable gpio? TL;DR: yes but that is for a separate series, which currently only exists in my head. Longer story: It looks like on BYT we need to control both VLV_GPIO_NC_10_PANEL1_BKLTEN and VLV_GPIO_NC_11_PANEL1_BKLTCTL from vlv_dsi.c when the LPSS is used for PWM. With BKLTCTL working as a panel_enable (needs to be driven high early on when initializing the panel) and BKLTEN is just a backlight enable/disable GPIO. Without this DSI panels will not light-up on BYT when a HDMI monitor is connected and the GOP chooses to initialize the HDMI rather then the panel, since then these 2 pins stay low. On CHT the MIPI power on/off sequences seem to take care of this themselves. I still want to run some more tests. Currently if I export the 2 gpios in question in sysfs (since their not claimed yet) and read them they always read 0. I have the feeling this is caused by the input-buffer not being enabled on these GPIOs, and that they really are high. So I want to do a little hack to enable the input buffer and then see if indeed they are high when the GOP has initialized the panel. Testing has already shown that driving them high manualy before loading i915 when the GOP did not init the panel fixes the panel not lighting up. So I'm pretty sure that this is the case, but I want to verify this before writing a series for that. Regards, Hans + if (IS_ERR(panel->backlight.pwm)) { - DRM_ERROR("Failed to own the pwm chip\n"); + DRM_ERROR("Failed to get the %s PWM chip\n", desc); panel->backlight.pwm = NULL; return -ENODEV; } @@ -1873,6 +1882,7 @@ static int pwm_setup_backlight(struct intel_connector *connector, CRC_PMIC_PWM_PERIOD_NS); panel->backlight.enabled = panel->backlight.level != 0; + DRM_INFO("Using %s PWM for LCD backlight control\n", desc); return 0; } -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: power: Fix path to power-domain.txt bindings
Hi Krzysztof, On Tue, Nov 19, 2019 at 3:43 PM Krzysztof Kozlowski wrote: > With split of power domain controller bindings to power-domain.yaml, the > consumer part was renamed to power-domain.txt. Update the references in > other bindings. > > Reported-by: Geert Uytterhoeven > Fixes: abb4805e343a ("dt-bindings: power: Convert Samsung Exynos Power Domain > bindings to json-schema") > Signed-off-by: Krzysztof Kozlowski Thanks for your patch! Reviewed-by: Geert Uytterhoeven > .../bindings/clock/renesas,rcar-gen2-cpg-clocks.txt | 2 +- Please note this file is no longer present in next. But robh/for-next still has it. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFCv1 33/42] drm/omap: dsi: use atomic helper for dirtyfb
On 19/11/2019 17:06, Tony Lindgren wrote: The userspace apps need to do this. If they're using single-buffering, then using the dirtyfb ioctl should do the trick, after the SGX has finished drawing. Sounds like that's not happening. But why would the userspace app need to know this might be needed for a DSI command mode display and that it's not needed for HDMI? When double-buffering, the userspace doesn't need to care, as the page-flip ioctl explicitly tells that the buffer is ready. When single buffering, either the userspace has to tell that the buffer is now ready, or the kernel has to guess based on something. But afaics, the latter is always a guess, and may not be a good guess. The kernel could trigger a delayed update based on, say, page fault if drawing with CPU. But how much delayed... And with this scenario, we would somehow need to find a way to catch the writes from any IP to the buffer, and afaik there's no such thing. It's probably somewhat difficult if EGL is controlling the display, as, afaik, SGX EGL is closed source, and that's probably where it should be done. But adding back the hacky omap gem sync stuff, and then somehow guessing from the sync finish that some display needs to be updated... It just does not sound right to me. Right it's ugly. Still sounds like we need something in the kernel that knows "this is a DSI command mode LCD and needs to be updated". I think one option is to refresh the command mode display all the time. Either using a timer, or trigger updates based on the previous update being finished. Of course, that's kind of against the whole point of manual update display, but then it should effectively behave like a conventional always-updating display (except your HW is more expensive and consumes more power than a conventional display). There's this Panel Self Refresh feature in DisplayPort (which I think is implemented in drm_self_refresh_helper.c), which has some similarities to this case. But if I read it right, that also expects some kind of trigger from userspace (any DRM commit) to start the refresh. Afaik, Weston and X both handle page flips and/or dirtying the fb, so they should work. Are there applications that do not work, and cannot be made to work, except the few SGX test apps? Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/i915: DSI: select correct PWM controller to use based on the VBT
On Tue, Nov 19, 2019 at 04:18:18PM +0100, Hans de Goede wrote: > At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2 > different PWM controllers for controlling the LCD's backlight brightness. > Either the one integrated into the PMIC or the one integrated into the > SoC (the 1st LPSS PWM controller). > > So far in the LPSS code on BYT we have skipped registering the LPSS PWM > controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is > present, assuming that in this case the PMIC PWM controller will be used. > > On CHT we have been relying on only 1 of the 2 PWM controllers being > enabled in the DSDT at the same time; and always registered the lookup. > > So far this has been working, but the correct way to determine which PWM > controller needs to be used is by checking a bit in the VBT table and > recently I've learned about 2 different BYT devices: > Point of View MOBII TAB-P800W > Acer Switch 10 SW5-012 > > Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS > PWM controller (and the VBT correctly indicates this), so here our old > heuristics fail. > > This commit fixes using the wrong PWM controller on these devices by > calling pwm_get() for the right PWM controller based on the > VBT dsi.config.pwm_blc bit. > > Note this is part of a series which contains 2 other patches which renames > the PWM lookup for the 1st SoC/LPSS PWM from "pwm_backlight" to > "pwm_pmic_backlight" and the PWM lookup for the Crystal Cove PMIC PWM > from "pwm_backlight" to "pwm_pmic_backlight". > > Signed-off-by: Hans de Goede > --- > drivers/gpu/drm/i915/display/intel_panel.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_panel.c > b/drivers/gpu/drm/i915/display/intel_panel.c > index bc14e9c0285a..ddcf311d1114 100644 > --- a/drivers/gpu/drm/i915/display/intel_panel.c > +++ b/drivers/gpu/drm/i915/display/intel_panel.c > @@ -1840,13 +1840,22 @@ static int pwm_setup_backlight(struct intel_connector > *connector, > enum pipe pipe) > { > struct drm_device *dev = connector->base.dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_panel *panel = >panel; > + const char *desc; > int retval; > > - /* Get the PWM chip for backlight control */ > - panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight"); > + /* Get the right PWM chip for DSI backlight according to VBT */ > + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) { > + panel->backlight.pwm = pwm_get(dev->dev, "pwm_pmic_backlight"); > + desc = "PMIC"; > + } else { > + panel->backlight.pwm = pwm_get(dev->dev, "pwm_soc_backlight"); > + desc = "SoC"; > + } Might we want the same thing for the panel enable gpio? > + > if (IS_ERR(panel->backlight.pwm)) { > - DRM_ERROR("Failed to own the pwm chip\n"); > + DRM_ERROR("Failed to get the %s PWM chip\n", desc); > panel->backlight.pwm = NULL; > return -ENODEV; > } > @@ -1873,6 +1882,7 @@ static int pwm_setup_backlight(struct intel_connector > *connector, >CRC_PMIC_PWM_PERIOD_NS); > panel->backlight.enabled = panel->backlight.level != 0; > > + DRM_INFO("Using %s PWM for LCD backlight control\n", desc); > return 0; > } > > -- > 2.23.0 -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm/i915 / LPSS / mfd: Select correct PWM controller to use based on VBT
On Tue, 19 Nov 2019, Hans de Goede wrote: > Hi All, > > This series needs to be merged through a single tree, to keep things > bisectable. I have even considered just squashing all 3 patches into 1, > but having separate commits seems better, but that does lead to an > intermediate state where the backlight sysfs interface will be broken > (and fixed 2 commits later). See below for some background info. > > The changes to drivers/acpi/acpi_lpss.c and drivers/mfd/intel_soc_pmic_core.c > are quite small and should not lead to any conflicts, so I believe that > it would be best to merge this entire series through the drm-intel tree. > > Lee, may I have your Acked-by for merging the mfd change through the > drm-intel tree? > > Rafael, may I have your Acked-by for merging the acpi_lpss change through the > drm-intel tree? > > Regards, > > Hans > > p.s. > > The promised background info: > > We have this long standing issue where instead of looking in the i915 > VBT (Video BIOS Table) to see if we should use the PWM block of the SoC > or of the PMIC to control the backlight of a DSI panel, we rely on > drivers/acpi/acpi_lpss.c and/or drivers/mfd/intel_soc_pmic_core.c > registering a pwm with the generic name of "pwm_backlight" and then the > i915 panel code does a pwm_get(dev, "pwm_backlight"). > > We have some heuristics in drivers/acpi/acpi_lpss.c to not register the > lookup if a Crystal Cove PMIC is presend and the mfd/intel_soc_pmic_core.c > code simply assumes that since there is a PMIC the PMIC PWM block will > be used. Basically we are winging it. > > Recently I've learned about 2 different BYT devices: > Point of View MOBII TAB-P800W > Acer Switch 10 SW5-012 > > Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS > PWM controller (and the VBT correctly indicates this), so here our old > heuristics fail. > > This series renams the PWM lookups registered by the LPSS / > intel_soc_pmic_core.c code from "pwm_backlight" to "pwm_soc_backlight" resp. > "pwm_pmic_backlight" and in the LPSS case also dropping the heuristics when > to register the lookup. This combined with teaching the i915 panel to call > pwm_get for the right lookup-name depending on the VBT bits resolves this. Hans, thanks for your continued efforts in digging into the bottom of this! I'm sure there are a number of related bugs still open at fdo bugzilla. It all makes sense, Acked-by: Jani Nikula for merging through whichever tree. Thanks, 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
[PATCH 3/3] drm/i915: DSI: select correct PWM controller to use based on the VBT
At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2 different PWM controllers for controlling the LCD's backlight brightness. Either the one integrated into the PMIC or the one integrated into the SoC (the 1st LPSS PWM controller). So far in the LPSS code on BYT we have skipped registering the LPSS PWM controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is present, assuming that in this case the PMIC PWM controller will be used. On CHT we have been relying on only 1 of the 2 PWM controllers being enabled in the DSDT at the same time; and always registered the lookup. So far this has been working, but the correct way to determine which PWM controller needs to be used is by checking a bit in the VBT table and recently I've learned about 2 different BYT devices: Point of View MOBII TAB-P800W Acer Switch 10 SW5-012 Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS PWM controller (and the VBT correctly indicates this), so here our old heuristics fail. This commit fixes using the wrong PWM controller on these devices by calling pwm_get() for the right PWM controller based on the VBT dsi.config.pwm_blc bit. Note this is part of a series which contains 2 other patches which renames the PWM lookup for the 1st SoC/LPSS PWM from "pwm_backlight" to "pwm_pmic_backlight" and the PWM lookup for the Crystal Cove PMIC PWM from "pwm_backlight" to "pwm_pmic_backlight". Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/display/intel_panel.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index bc14e9c0285a..ddcf311d1114 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -1840,13 +1840,22 @@ static int pwm_setup_backlight(struct intel_connector *connector, enum pipe pipe) { struct drm_device *dev = connector->base.dev; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_panel *panel = >panel; + const char *desc; int retval; - /* Get the PWM chip for backlight control */ - panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight"); + /* Get the right PWM chip for DSI backlight according to VBT */ + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) { + panel->backlight.pwm = pwm_get(dev->dev, "pwm_pmic_backlight"); + desc = "PMIC"; + } else { + panel->backlight.pwm = pwm_get(dev->dev, "pwm_soc_backlight"); + desc = "SoC"; + } + if (IS_ERR(panel->backlight.pwm)) { - DRM_ERROR("Failed to own the pwm chip\n"); + DRM_ERROR("Failed to get the %s PWM chip\n", desc); panel->backlight.pwm = NULL; return -ENODEV; } @@ -1873,6 +1882,7 @@ static int pwm_setup_backlight(struct intel_connector *connector, CRC_PMIC_PWM_PERIOD_NS); panel->backlight.enabled = panel->backlight.level != 0; + DRM_INFO("Using %s PWM for LCD backlight control\n", desc); return 0; } -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] ACPI / LPSS: Rename pwm_backlight pwm-lookup to pwm_soc_backlight
At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2 different PWM controllers for controlling the LCD's backlight brightness. Either the one integrated into the PMIC or the one integrated into the SoC (the 1st LPSS PWM controller). So far in the LPSS code on BYT we have skipped registering the LPSS PWM controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is present, assuming that in this case the PMIC PWM controller will be used. On CHT we have been relying on only 1 of the 2 PWM controllers being enabled in the DSDT at the same time; and always registered the lookup. So far this has been working, but the correct way to determine which PWM controller needs to be used is by checking a bit in the VBT table and recently I've learned about 2 different BYT devices: Point of View MOBII TAB-P800W Acer Switch 10 SW5-012 Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS PWM controller (and the VBT correctly indicates this), so here our old heuristics fail. Since only the i915 driver has access to the VBT, this commit renames the "pwm_backlight" lookup entries for the 1st BYT/CHT LPSS PWM controller to "pwm_soc_backlight" so that the i915 driver can do a pwm_get() for the right controller depending on the VBT bit, instead of the i915 driver relying on a "pwm_backlight" lookup getting registered which magically points to the right controller. Signed-off-by: Hans de Goede --- drivers/acpi/acpi_lpss.c | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c index 751ed38f2a10..63e81d8e675b 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -69,10 +69,6 @@ ACPI_MODULE_NAME("acpi_lpss"); #define LPSS_SAVE_CTX BIT(4) #define LPSS_NO_D3_DELAY BIT(5) -/* Crystal Cove PMIC shares same ACPI ID between different platforms */ -#define BYT_CRC_HRV2 -#define CHT_CRC_HRV3 - struct lpss_private_data; struct lpss_device_desc { @@ -158,7 +154,7 @@ static void lpss_deassert_reset(struct lpss_private_data *pdata) */ static struct pwm_lookup byt_pwm_lookup[] = { PWM_LOOKUP_WITH_MODULE("80860F09:00", 0, ":00:02.0", - "pwm_backlight", 0, PWM_POLARITY_NORMAL, + "pwm_soc_backlight", 0, PWM_POLARITY_NORMAL, "pwm-lpss-platform"), }; @@ -170,8 +166,7 @@ static void byt_pwm_setup(struct lpss_private_data *pdata) if (!adev->pnp.unique_id || strcmp(adev->pnp.unique_id, "1")) return; - if (!acpi_dev_present("INT33FD", NULL, BYT_CRC_HRV)) - pwm_add_table(byt_pwm_lookup, ARRAY_SIZE(byt_pwm_lookup)); + pwm_add_table(byt_pwm_lookup, ARRAY_SIZE(byt_pwm_lookup)); } #define LPSS_I2C_ENABLE0x6c @@ -204,7 +199,7 @@ static void byt_i2c_setup(struct lpss_private_data *pdata) /* BSW PWM used for backlight control by the i915 driver */ static struct pwm_lookup bsw_pwm_lookup[] = { PWM_LOOKUP_WITH_MODULE("80862288:00", 0, ":00:02.0", - "pwm_backlight", 0, PWM_POLARITY_NORMAL, + "pwm_soc_backlight", 0, PWM_POLARITY_NORMAL, "pwm-lpss-platform"), }; -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight
At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2 different PWM controllers for controlling the LCD's backlight brightness. Either the one integrated into the PMIC or the one integrated into the SoC (the 1st LPSS PWM controller). So far in the LPSS code on BYT we have skipped registering the LPSS PWM controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is present, assuming that in this case the PMIC PWM controller will be used. On CHT we have been relying on only 1 of the 2 PWM controllers being enabled in the DSDT at the same time; and always registered the lookup. So far this has been working, but the correct way to determine which PWM controller needs to be used is by checking a bit in the VBT table and recently I've learned about 2 different BYT devices: Point of View MOBII TAB-P800W Acer Switch 10 SW5-012 Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS PWM controller (and the VBT correctly indicates this), so here our old heuristics fail. Since only the i915 driver has access to the VBT, this commit renames the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM controller to "pwm_pmic_backlight" so that the i915 driver can do a pwm_get() for the right controller depending on the VBT bit, instead of the i915 driver relying on a "pwm_backlight" lookup getting registered which magically points to the right controller. Signed-off-by: Hans de Goede --- drivers/mfd/intel_soc_pmic_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mfd/intel_soc_pmic_core.c b/drivers/mfd/intel_soc_pmic_core.c index c9f35378d391..47188df3080d 100644 --- a/drivers/mfd/intel_soc_pmic_core.c +++ b/drivers/mfd/intel_soc_pmic_core.c @@ -38,7 +38,7 @@ static struct gpiod_lookup_table panel_gpio_table = { /* PWM consumed by the Intel GFX */ static struct pwm_lookup crc_pwm_lookup[] = { - PWM_LOOKUP("crystal_cove_pwm", 0, ":00:02.0", "pwm_backlight", 0, PWM_POLARITY_NORMAL), + PWM_LOOKUP("crystal_cove_pwm", 0, ":00:02.0", "pwm_pmic_backlight", 0, PWM_POLARITY_NORMAL), }; static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c, -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] drm/i915 / LPSS / mfd: Select correct PWM controller to use based on VBT
Hi All, This series needs to be merged through a single tree, to keep things bisectable. I have even considered just squashing all 3 patches into 1, but having separate commits seems better, but that does lead to an intermediate state where the backlight sysfs interface will be broken (and fixed 2 commits later). See below for some background info. The changes to drivers/acpi/acpi_lpss.c and drivers/mfd/intel_soc_pmic_core.c are quite small and should not lead to any conflicts, so I believe that it would be best to merge this entire series through the drm-intel tree. Lee, may I have your Acked-by for merging the mfd change through the drm-intel tree? Rafael, may I have your Acked-by for merging the acpi_lpss change through the drm-intel tree? Regards, Hans p.s. The promised background info: We have this long standing issue where instead of looking in the i915 VBT (Video BIOS Table) to see if we should use the PWM block of the SoC or of the PMIC to control the backlight of a DSI panel, we rely on drivers/acpi/acpi_lpss.c and/or drivers/mfd/intel_soc_pmic_core.c registering a pwm with the generic name of "pwm_backlight" and then the i915 panel code does a pwm_get(dev, "pwm_backlight"). We have some heuristics in drivers/acpi/acpi_lpss.c to not register the lookup if a Crystal Cove PMIC is presend and the mfd/intel_soc_pmic_core.c code simply assumes that since there is a PMIC the PMIC PWM block will be used. Basically we are winging it. Recently I've learned about 2 different BYT devices: Point of View MOBII TAB-P800W Acer Switch 10 SW5-012 Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS PWM controller (and the VBT correctly indicates this), so here our old heuristics fail. This series renams the PWM lookups registered by the LPSS / intel_soc_pmic_core.c code from "pwm_backlight" to "pwm_soc_backlight" resp. "pwm_pmic_backlight" and in the LPSS case also dropping the heuristics when to register the lookup. This combined with teaching the i915 panel to call pwm_get for the right lookup-name depending on the VBT bits resolves this. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dsc: Return unsigned long on compute offset
On 19/11/2019 09:56, Ville Syrjälä wrote: On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote: From: Mikita Lipski We shouldn't compare int with unsigned long to find the max value and since we are not expecting negative value returned from compute_offset we should make this function return unsigned long so we can compare the values when computing rc parameters. Why are there other unsigned longs in dsc parameter computation in the first place? I believe it was initially set to be unsigned long for variable consistency, when we ported intel_compute_rc_parameters into drm_dsc_compute_rc_parameters. But now that I look at it, we can actually just set them to u32 or u64, as nothing should exceed that. Cc: Nikola Cornij Cc: Harry Wentland Signed-off-by: Mikita Lipski --- drivers/gpu/drm/drm_dsc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c index 74f3527f567d..ec40604ab6a2 100644 --- a/drivers/gpu/drm/drm_dsc.c +++ b/drivers/gpu/drm/drm_dsc.c @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload, } EXPORT_SYMBOL(drm_dsc_pps_payload_pack); -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group, +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group, int groups_per_line, int grpcnt) { - int offset = 0; - int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group); + unsigned long offset = 0; + unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group); if (grpcnt <= grpcnt_id) offset = DIV_ROUND_UP(grpcnt * pixels_per_group * vdsc_cfg->bits_per_pixel, 16); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Thanks, Mikita Lipski Software Engineer 2, AMD mikita.lip...@amd.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Bugzilla to Gitlabs migration - No email updates ?
Does gitlab not generate email when a comment is added to a ticket ? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dsc: Return unsigned long on compute offset
On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote: > From: Mikita Lipski > > We shouldn't compare int with unsigned long to find the max value > and since we are not expecting negative value returned from > compute_offset we should make this function return unsigned long > so we can compare the values when computing rc parameters. Why are there other unsigned longs in dsc parameter computation in the first place? > > Cc: Nikola Cornij > Cc: Harry Wentland > Signed-off-by: Mikita Lipski > --- > drivers/gpu/drm/drm_dsc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c > index 74f3527f567d..ec40604ab6a2 100644 > --- a/drivers/gpu/drm/drm_dsc.c > +++ b/drivers/gpu/drm/drm_dsc.c > @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct > drm_dsc_picture_parameter_set *pps_payload, > } > EXPORT_SYMBOL(drm_dsc_pps_payload_pack); > > -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int > pixels_per_group, > +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int > pixels_per_group, > int groups_per_line, int grpcnt) > { > - int offset = 0; > - int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, > pixels_per_group); > + unsigned long offset = 0; > + unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, > pixels_per_group); > > if (grpcnt <= grpcnt_id) > offset = DIV_ROUND_UP(grpcnt * pixels_per_group * > vdsc_cfg->bits_per_pixel, 16); > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFCv1 33/42] drm/omap: dsi: use atomic helper for dirtyfb
On 19/11/2019 16:32, Tony Lindgren wrote: We haven't had omap_gem_op_finish() in the kernel for some years now... Shouldn't a normal page flip, or if doing single-buffering, using the dirtyfb ioctl, do the job? It does not seem to happen with the old pvr-omap4 related patches and whatever gles related tests at least. If you have some idea where it should get called I can take a look at some point. The userspace apps need to do this. If they're using single-buffering, then using the dirtyfb ioctl should do the trick, after the SGX has finished drawing. It's probably somewhat difficult if EGL is controlling the display, as, afaik, SGX EGL is closed source, and that's probably where it should be done. But adding back the hacky omap gem sync stuff, and then somehow guessing from the sync finish that some display needs to be updated... It just does not sound right to me. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] DRM: ARC: PGU: fix framebuffer format switching
Current implementation don't switch to RGB565 format if BGR888 was previously used. Fix that. Signed-off-by: Eugeniy Paltsev --- drivers/gpu/drm/arc/arcpgu_crtc.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c index dfaddbb7da0d..31d9824c46cc 100644 --- a/drivers/gpu/drm/arc/arcpgu_crtc.c +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -32,6 +32,7 @@ static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) uint32_t pixel_format = fb->format->format; struct simplefb_format *format = NULL; int i; + u32 reg_ctrl; for (i = 0; i < ARRAY_SIZE(supported_formats); i++) { if (supported_formats[i].fourcc == pixel_format) @@ -41,11 +42,12 @@ static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) if (WARN_ON(!format)) return; - if (format->fourcc == DRM_FORMAT_RGB888) - arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, - arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) | - ARCPGU_MODE_RGB888_MASK); - + reg_ctrl = arc_pgu_read(arcpgu, ARCPGU_REG_CTRL); + if (format->fourcc == DRM_FORMAT_RGB565) + reg_ctrl &= ~ARCPGU_MODE_RGB888_MASK; + else + reg_ctrl |= ARCPGU_MODE_RGB888_MASK; + arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, reg_ctrl); } static const struct drm_crtc_funcs arc_pgu_crtc_funcs = { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/4] DRM: PGU: ARC: fixies related to framebuffer format
Eugeniy Paltsev (4): DRM: ARC: PGU: fix framebuffer format switching DRM: ARC: PGU: cleanup supported format list code DRM: ARC: PGU: replace unsupported by HW RGB888 format by XRGB888 DRM: ARC: PGU: add ARGB format to supported format list drivers/gpu/drm/arc/arcpgu_crtc.c | 36 +++ drivers/gpu/drm/arc/arcpgu_regs.h | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] DRM: ARC: PGU: cleanup supported format list code
Get rid of 'simplefb_format' structure usage as we only use its 'fourcc' field. Signed-off-by: Eugeniy Paltsev --- drivers/gpu/drm/arc/arcpgu_crtc.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c index 31d9824c46cc..5473b19a52ee 100644 --- a/drivers/gpu/drm/arc/arcpgu_crtc.c +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -20,9 +20,9 @@ #define ENCODE_PGU_XY(x, y)x) - 1) << 16) | ((y) - 1)) -static struct simplefb_format supported_formats[] = { - { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0}, DRM_FORMAT_RGB565 }, - { "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, +static const u32 arc_pgu_supported_formats[] = { + DRM_FORMAT_RGB565, + DRM_FORMAT_RGB888, }; static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) @@ -30,20 +30,20 @@ static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); const struct drm_framebuffer *fb = crtc->primary->state->fb; uint32_t pixel_format = fb->format->format; - struct simplefb_format *format = NULL; + u32 format = DRM_FORMAT_INVALID; int i; u32 reg_ctrl; - for (i = 0; i < ARRAY_SIZE(supported_formats); i++) { - if (supported_formats[i].fourcc == pixel_format) - format = _formats[i]; + for (i = 0; i < ARRAY_SIZE(arc_pgu_supported_formats); i++) { + if (arc_pgu_supported_formats[i] == pixel_format) + format = arc_pgu_supported_formats[i]; } - if (WARN_ON(!format)) + if (WARN_ON(format == DRM_FORMAT_INVALID)) return; reg_ctrl = arc_pgu_read(arcpgu, ARCPGU_REG_CTRL); - if (format->fourcc == DRM_FORMAT_RGB565) + if (format == DRM_FORMAT_RGB565) reg_ctrl &= ~ARCPGU_MODE_RGB888_MASK; else reg_ctrl |= ARCPGU_MODE_RGB888_MASK; @@ -195,18 +195,15 @@ static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm) { struct arcpgu_drm_private *arcpgu = drm->dev_private; struct drm_plane *plane = NULL; - u32 formats[ARRAY_SIZE(supported_formats)], i; int ret; plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL); if (!plane) return ERR_PTR(-ENOMEM); - for (i = 0; i < ARRAY_SIZE(supported_formats); i++) - formats[i] = supported_formats[i].fourcc; - ret = drm_universal_plane_init(drm, plane, 0xff, _pgu_plane_funcs, - formats, ARRAY_SIZE(formats), + arc_pgu_supported_formats, + ARRAY_SIZE(arc_pgu_supported_formats), NULL, DRM_PLANE_TYPE_PRIMARY, NULL); if (ret) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] DRM: ARC: PGU: replace unsupported by HW RGB888 format by XRGB888
ARC PGU doesn't support RGB888 (24 bit) format but supports XRGB888 (32 bit) format. Fix incorrect format list in a driver. Signed-off-by: Eugeniy Paltsev --- drivers/gpu/drm/arc/arcpgu_crtc.c | 6 +++--- drivers/gpu/drm/arc/arcpgu_regs.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c index 5473b19a52ee..980e00180e6f 100644 --- a/drivers/gpu/drm/arc/arcpgu_crtc.c +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -22,7 +22,7 @@ static const u32 arc_pgu_supported_formats[] = { DRM_FORMAT_RGB565, - DRM_FORMAT_RGB888, + DRM_FORMAT_XRGB, }; static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) @@ -44,9 +44,9 @@ static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) reg_ctrl = arc_pgu_read(arcpgu, ARCPGU_REG_CTRL); if (format == DRM_FORMAT_RGB565) - reg_ctrl &= ~ARCPGU_MODE_RGB888_MASK; + reg_ctrl &= ~ARCPGU_MODE_XRGB; else - reg_ctrl |= ARCPGU_MODE_RGB888_MASK; + reg_ctrl |= ARCPGU_MODE_XRGB; arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, reg_ctrl); } diff --git a/drivers/gpu/drm/arc/arcpgu_regs.h b/drivers/gpu/drm/arc/arcpgu_regs.h index dab2c380f7f3..b689a382d556 100644 --- a/drivers/gpu/drm/arc/arcpgu_regs.h +++ b/drivers/gpu/drm/arc/arcpgu_regs.h @@ -25,7 +25,7 @@ #define ARCPGU_CTRL_VS_POL_OFST0x3 #define ARCPGU_CTRL_HS_POL_MASK0x1 #define ARCPGU_CTRL_HS_POL_OFST0x4 -#define ARCPGU_MODE_RGB888_MASK0x04 +#define ARCPGU_MODE_XRGB BIT(2) #define ARCPGU_STAT_BUSY_MASK 0x02 #endif -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] DRM: ARC: PGU: add ARGB8888 format to supported format list
As we ignore first 8 bit of 32 bit pixel value we can add ARGB format as alias of XRGB. Signed-off-by: Eugeniy Paltsev --- drivers/gpu/drm/arc/arcpgu_crtc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c index 980e00180e6f..8ae1e1f97a73 100644 --- a/drivers/gpu/drm/arc/arcpgu_crtc.c +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -23,6 +23,7 @@ static const u32 arc_pgu_supported_formats[] = { DRM_FORMAT_RGB565, DRM_FORMAT_XRGB, + DRM_FORMAT_ARGB, }; static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dsc: Return unsigned long on compute offset
From: Mikita Lipski We shouldn't compare int with unsigned long to find the max value and since we are not expecting negative value returned from compute_offset we should make this function return unsigned long so we can compare the values when computing rc parameters. Cc: Nikola Cornij Cc: Harry Wentland Signed-off-by: Mikita Lipski --- drivers/gpu/drm/drm_dsc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c index 74f3527f567d..ec40604ab6a2 100644 --- a/drivers/gpu/drm/drm_dsc.c +++ b/drivers/gpu/drm/drm_dsc.c @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload, } EXPORT_SYMBOL(drm_dsc_pps_payload_pack); -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group, +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group, int groups_per_line, int grpcnt) { - int offset = 0; - int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group); + unsigned long offset = 0; + unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group); if (grpcnt <= grpcnt_id) offset = DIV_ROUND_UP(grpcnt * pixels_per_group * vdsc_cfg->bits_per_pixel, 16); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dt-bindings: power: Fix path to power-domain.txt bindings
With split of power domain controller bindings to power-domain.yaml, the consumer part was renamed to power-domain.txt. Update the references in other bindings. Reported-by: Geert Uytterhoeven Fixes: abb4805e343a ("dt-bindings: power: Convert Samsung Exynos Power Domain bindings to json-schema") Signed-off-by: Krzysztof Kozlowski --- Documentation/devicetree/bindings/clock/clk-exynos-audss.txt | 2 +- Documentation/devicetree/bindings/clock/exynos5433-clock.txt | 2 +- .../devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt | 2 +- .../devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt | 2 +- .../bindings/clock/renesas,rcar-gen2-cpg-clocks.txt | 2 +- .../devicetree/bindings/clock/renesas,rz-cpg-clocks.txt | 2 +- .../devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 2 +- Documentation/devicetree/bindings/display/msm/dpu.txt | 2 +- Documentation/devicetree/bindings/display/msm/mdp5.txt| 2 +- Documentation/devicetree/bindings/dsp/fsl,dsp.yaml| 2 +- Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt| 2 +- .../devicetree/bindings/media/mediatek-jpeg-decoder.txt | 2 +- Documentation/devicetree/bindings/media/mediatek-mdp.txt | 2 +- Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 2 +- Documentation/devicetree/bindings/pci/pci-keystone.txt| 2 +- Documentation/devicetree/bindings/phy/ti,phy-am654-serdes.txt | 2 +- Documentation/devicetree/bindings/power/qcom,rpmpd.txt| 2 +- Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt | 2 +- .../devicetree/bindings/usb/nvidia,tegra124-xusb.txt | 4 ++-- 19 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt b/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt index 6030afb10b5c..e6c6b43e9770 100644 --- a/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +++ b/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt @@ -36,7 +36,7 @@ Required Properties: Optional Properties: - power-domains: a phandle to respective power domain node as described by -generic PM domain bindings (see power/power_domain.txt for more +generic PM domain bindings (see power/power-domain.txt for more information). The following is the list of clocks generated by the controller. Each clock is diff --git a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt index 183c327a7d6b..972d4e45f8c1 100644 --- a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt +++ b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt @@ -178,7 +178,7 @@ Required Properties: Optional properties: - power-domains: a phandle to respective power domain node as described by - generic PM domain bindings (see power/power_domain.txt for more + generic PM domain bindings (see power/power-domain.txt for more information). Each clock is assigned an identifier and client nodes can use this identifier diff --git a/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt index 7cc4c0330b53..46ecbbce277c 100644 --- a/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt +++ b/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt @@ -17,7 +17,7 @@ Required Properties: SoC devices that are part of the CPG/MSTP Clock Domain and can be power-managed through an MSTP clock should refer to the CPG device node in their "power-domains" property, as documented by the generic PM domain bindings in -Documentation/devicetree/bindings/power/power_domain.txt. +Documentation/devicetree/bindings/power/power-domain.txt. Examples diff --git a/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt index 8c81547c29f5..cb32b4f41046 100644 --- a/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt +++ b/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt @@ -19,7 +19,7 @@ Required Properties: SoC devices that are part of the CPG/MSTP Clock Domain and can be power-managed through an MSTP clock should refer to the CPG device node in their "power-domains" property, as documented by the generic PM domain bindings in -Documentation/devicetree/bindings/power/power_domain.txt. +Documentation/devicetree/bindings/power/power-domain.txt. Examples diff --git a/Documentation/devicetree/bindings/clock/renesas,rcar-gen2-cpg-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,rcar-gen2-cpg-clocks.txt index f8c05bb4116e..58f9054704c2 100644 --- a/Documentation/devicetree/bindings/clock/renesas,rcar-gen2-cpg-clocks.txt +++ b/Documentation/devicetree/bindings/clock/renesas,rcar-gen2-cpg-clocks.txt @@ -28,7 +28,7
Re: [PATCH] drm/gma500: Fixup fbdev stolen size usage evaluation
Hi, On Wed 13 Nov 19, 11:04, Patrik Jakobsson wrote: > On Tue, Nov 12, 2019 at 4:50 PM Paul Kocialkowski > wrote: > > > > Hi, > > > > On Tue 12 Nov 19, 16:11, Paul Kocialkowski wrote: > > > Hi, > > > > > > On Tue 12 Nov 19, 11:20, Patrik Jakobsson wrote: > > > > On Thu, Nov 7, 2019 at 4:30 PM Paul Kocialkowski > > > > wrote: > > > > > > > > > > psbfb_probe performs an evaluation of the required size from the > > > > > stolen > > > > > GTT memory, but gets it wrong in two distinct ways: > > > > > - The resulting size must be page-size-aligned; > > > > > - The size to allocate is derived from the surface dimensions, not > > > > > the fb > > > > > dimensions. > > > > > > > > > > When two connectors are connected with different modes, the smallest > > > > > will > > > > > be stored in the fb dimensions, but the size that needs to be > > > > > allocated must > > > > > match the largest (surface) dimensions. This is what is used in the > > > > > actual > > > > > allocation code. > > > > > > > > > > Fix this by correcting the evaluation to conform to the two points > > > > > above. > > > > > It allows correctly switching to 16bpp when one connector is e.g. > > > > > 1920x1080 > > > > > and the other is 1024x768. > > > > > > > > > > Signed-off-by: Paul Kocialkowski > > > > > --- > > > > > drivers/gpu/drm/gma500/framebuffer.c | 8 ++-- > > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/gma500/framebuffer.c > > > > > b/drivers/gpu/drm/gma500/framebuffer.c > > > > > index 218f3bb15276..90237abee088 100644 > > > > > --- a/drivers/gpu/drm/gma500/framebuffer.c > > > > > +++ b/drivers/gpu/drm/gma500/framebuffer.c > > > > > @@ -462,6 +462,7 @@ static int psbfb_probe(struct drm_fb_helper > > > > > *helper, > > > > > container_of(helper, struct psb_fbdev, psb_fb_helper); > > > > > struct drm_device *dev = psb_fbdev->psb_fb_helper.dev; > > > > > struct drm_psb_private *dev_priv = dev->dev_private; > > > > > + unsigned int fb_size; > > > > > int bytespp; > > > > > > > > > > bytespp = sizes->surface_bpp / 8; > > > > > @@ -471,8 +472,11 @@ static int psbfb_probe(struct drm_fb_helper > > > > > *helper, > > > > > /* If the mode will not fit in 32bit then switch to 16bit to > > > > > get > > > > >a console on full resolution. The X mode setting server > > > > > will > > > > >allocate its own 32bit GEM framebuffer */ > > > > > - if (ALIGN(sizes->fb_width * bytespp, 64) * sizes->fb_height > > > > > > - dev_priv->vram_stolen_size) { > > > > > + fb_size = ALIGN(sizes->surface_width * bytespp, 64) * > > > > > + sizes->surface_height; > > > > > + fb_size = ALIGN(fb_size, PAGE_SIZE); > > > > > + > > > > > + if (fb_size > dev_priv->vram_stolen_size) { > > > > > > > > psb_gtt_alloc_range() already aligns by PAGE_SIZE for us. Looks like > > > > we align a couple of times extra for luck. This needs cleaning up > > > > instead of adding even more aligns. > > > > > > I'm not sure this is really for luck. As far as I can see, we need to do > > > it > > > properly for this size estimation since it's the final size that will be > > > allocated (and thus needs to be available in whole). > > Ok now I understand what you meant. Actually vram_stolen_size is > always page aligned so fb_size doesn't need any page alignment here. I'm a bit confused here, what about the case where: unaligned fb_size < dev_priv->vram_stolen_size but aligned fb_size > dev_priv->vram_stolen_size ? Granted, it's a corner case, but I don't follow the logic of comparing aligned and unaligned sizes: it feels a bit like comparing two values of different units. > There is also no need to align for psbfb_create() since it also takes > care of this. > > > > > > > For the other times there is explicit alignment, they seem justified too: > > > - in psb_gem_create: it is common to pass the aligned size when creating > > > the > > > associated GEM object with drm_gem_object_init, even though it's > > > probably not > > > crucial given that this is not where allocation actually happens; > > > - in psbfb_create: the full size is apparently only really used to memset > > > 0 > > > the allocated buffer. I think this makes sense for security reasons > > > (and not > > > leak previous contents in the additional space required for alignment). > > What I would prefer is to have a single place where the alignment is > made so any hardware requirements would be transparent to the rest of > the code. Mhh, I thought that psbfb_create needs to be aware of the alignment in the form of the pitch_lines variable to decide which 2d accel method can be used or not (depending on associated alignment requirements). I guess this makes for another reason to ditch the accelerated 2d accel support. > Best would be if alignment is only made in psb_gtt_alloc_range()