[Bug 110137] drirc adaptive sync, vrr, freesync blacklisting documentation
https://bugs.freedesktop.org/show_bug.cgi?id=110137 Bug ID: 110137 Summary: drirc adaptive sync, vrr, freesync blacklisting documentation Product: Mesa Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: gr...@sub.red QA Contact: dri-devel@lists.freedesktop.org Hi, using qutebrowser in fullscreen mode breaks the video output with VRR enabled because it refreshes very unsteadily, which is typical for a browser. Some browsers are blacklisted in the drirc. I've added the following lines to the drirc, but it doesn't disable VRR for a fullscreen qutebrowser window.: Is there any documentation about how the matching works? -- 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 5/9] drm/syncobj: use the timeline point in drm_syncobj_find_fence v3
Hi Chunming, I love your patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on next-20190306] [cannot apply to v5.0] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Chunming-Zhou/dma-buf-add-new-dma_fence_chain-container-v5/20190316-055452 reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/drm_syncobj.c:239:42: sparse: constant 50 is so big >> it is long include/linux/slab.h:664:13: sparse: undefined identifier '__builtin_mul_overflow' include/linux/slab.h:664:13: sparse: not a function include/linux/slab.h:664:13: sparse: not a function include/linux/slab.h:664:13: sparse: not a function include/linux/slab.h:664:13: sparse: call with no type! vim +239 drivers/gpu/drm/drm_syncobj.c 215 216 /* 5s */ 217 #define DRM_SYNCOBJ_WAIT_FOR_SUBMIT_TIMEOUT 50 218 /** 219 * drm_syncobj_find_fence - lookup and reference the fence in a sync object 220 * @file_private: drm file private pointer 221 * @handle: sync object handle to lookup. 222 * @point: timeline point 223 * @flags: DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT or not 224 * @fence: out parameter for the fence 225 * 226 * This is just a convenience function that combines drm_syncobj_find() and 227 * drm_syncobj_fence_get(). 228 * 229 * Returns 0 on success or a negative error value on failure. On success @fence 230 * contains a reference to the fence, which must be released by calling 231 * dma_fence_put(). 232 */ 233 int drm_syncobj_find_fence(struct drm_file *file_private, 234 u32 handle, u64 point, u64 flags, 235 struct dma_fence **fence) 236 { 237 struct drm_syncobj *syncobj = drm_syncobj_find(file_private, handle); 238 struct syncobj_wait_entry wait; > 239 u64 timeout = > nsecs_to_jiffies64(DRM_SYNCOBJ_WAIT_FOR_SUBMIT_TIMEOUT); 240 int ret; 241 242 if (!syncobj) 243 return -ENOENT; 244 245 *fence = drm_syncobj_fence_get(syncobj); 246 drm_syncobj_put(syncobj); 247 248 if (*fence) { 249 ret = dma_fence_chain_find_seqno(fence, point); 250 if (!ret) 251 return 0; 252 dma_fence_put(*fence); 253 } else { 254 ret = -EINVAL; 255 } 256 257 if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) 258 return ret; 259 260 memset(, 0, sizeof(wait)); 261 wait.task = current; 262 wait.point = point; 263 drm_syncobj_fence_add_wait(syncobj, ); 264 265 do { 266 set_current_state(TASK_INTERRUPTIBLE); 267 if (wait.fence) { 268 ret = 0; 269 break; 270 } 271 if (timeout == 0) { 272 ret = -ETIME; 273 break; 274 } 275 276 if (signal_pending(current)) { 277 ret = -ERESTARTSYS; 278 break; 279 } 280 281 timeout = schedule_timeout(timeout); 282 } while (1); 283 284 __set_current_state(TASK_RUNNING); 285 *fence = wait.fence; 286 287 if (wait.node.next) 288 drm_syncobj_remove_wait(syncobj, ); 289 290 return ret; 291 } 292 EXPORT_SYMBOL(drm_syncobj_find_fence); 293 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 0/5 v2] DMA-BUF Heaps (destaging ION)
On Fri, Mar 15, 2019 at 4:15 PM Jerome Glisse wrote: > On Tue, Mar 05, 2019 at 12:54:28PM -0800, John Stultz wrote: > > Here is a initial RFC of the dma-buf heaps patchset Andrew and I > > have been working on which tries to destage a fair chunk of ION > > functionality. > > > > The patchset implements per-heap devices which can be opened > > directly and then an ioctl is used to allocate a dmabuf from the > > heap. > > > > The interface is similar, but much simpler then IONs, only > > providing an ALLOC ioctl. > > > > Also, I've provided simple system and cma heaps. The system > > heap in particular is missing the page-pool optimizations ION > > had, but works well enough to validate the interface. > > > > I've booted and tested these patches with AOSP on the HiKey960 > > using the kernel tree here: > > > > https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/dma-buf-heap > > > > And the userspace changes here: > > https://android-review.googlesource.com/c/device/linaro/hikey/+/909436 > > What upstream driver will use this eventualy ? And why is it > needed ? So, its sort of a complicated answer, as we don't have a fully open pipeline just yet. The HiKey board's upstream kirin drm driver uses this, as it needs contiguous buffers for its framebuffers. So in Android the HiKey gralloc (opensource userspace) allocates the HW_FB buffers from the CMA heap. Other graphics buffers are then allocated by gralloc out of the system heap and SurfaceFlinger and drm_hwc (both also opensource userspace) coordinate squashing those other buffers down through the mali utgard gpu (proprietary GL blob) onto the target HW_FB buffer. (All of the above is the same for the HiKey960, but we're still working the drm driver into shape for upstreaming). That said, I know the Lima driver is starting to shape up, and I'm hoping to give it a whirl to replace the proprietary utgard driver. Everything else would stay the same, which would give us a fully open pipeline. I know for other dev boards like the db410c w/ freedreno, the Android pipeline gets away with using the gbmgralloc implementation, but my understanding in that case is the rendering/display pipeline doesn't require contiguous buffers, so the allocation logic can be simpler, and doesn't use ION heaps. But its possible a gralloc could be implemented to use the system heap for allocations on that device. thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/selftest:- Adding a new format(whose cpp=0) to be tested by igt_check_drm_format_min_pitch
Hi Ayan, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v5.0 next-20190306] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Ayan-Halder/drm-selftest-Adding-a-new-format-whose-cpp-0-to-be-tested-by-igt_check_drm_format_min_pitch/20190316-050437 config: x86_64-allmodconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/gpu//drm/selftests/test-drm_format.c: In function 'igt_check_drm_format_min_pitch': >> drivers/gpu//drm/selftests/test-drm_format.c:280:25: error: >> 'DRM_FORMAT_VUY101010' undeclared (first use in this function); did you mean >> 'DRM_FORMAT_ARGB2101010'? info = drm_format_info(DRM_FORMAT_VUY101010); ^~~~ DRM_FORMAT_ARGB2101010 drivers/gpu//drm/selftests/test-drm_format.c:280:25: note: each undeclared identifier is reported only once for each function it appears in vim +280 drivers/gpu//drm/selftests/test-drm_format.c 100 101 int igt_check_drm_format_min_pitch(void *ignored) 102 { 103 const struct drm_format_info *info = NULL; 104 105 /* Test invalid arguments */ 106 FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0); 107 FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0); 108 FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0); 109 110 /* Test 1 plane 8 bits per pixel format */ 111 info = drm_format_info(DRM_FORMAT_RGB332); 112 FAIL_ON(!info); 113 FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0); 114 FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0); 115 FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0); 116 117 FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 1); 118 FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 2); 119 FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 640); 120 FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 1024); 121 FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 1920); 122 FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 4096); 123 FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 671); 124 FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) != 125 (uint64_t)UINT_MAX); 126 FAIL_ON(drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)) != 127 (uint64_t)(UINT_MAX - 1)); 128 129 /* Test 1 plane 16 bits per pixel format */ 130 info = drm_format_info(DRM_FORMAT_XRGB); 131 FAIL_ON(!info); 132 FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0); 133 FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0); 134 FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0); 135 136 FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 2); 137 FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 4); 138 FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 1280); 139 FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 2048); 140 FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 3840); 141 FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 8192); 142 FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 1342); 143 FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) != 144 (uint64_t)UINT_MAX * 2); 145 FAIL_ON(drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)) != 146 (uint64_t)(UINT_MAX - 1) * 2); 147 148 /* Test 1 plane 24 bits per pixel format */ 149 info = drm_format_info(DRM_FORMAT_RGB888); 150 FAIL_ON(!info); 151 FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0); 152 FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0); 153 FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0); 154 155 FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 3); 156 FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 6); 157 FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 1920); 158 FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 3072); 159 FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 5760); 160 FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 12288); 161 FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 2013); 162 FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) != 163 (uint64_t)UINT_MAX * 3); 164
Re: [PATCH v4 4/6] dt-bindings: display: sii902x: Remove trailing white space
On Thu, 14 Mar 2019 13:27:50 +0200, Jyri Sarha wrote: > Remove trailing white space from sii902x display bridge binding. > > Signed-off-by: Jyri Sarha > Reviewed-by: Laurent Pinchart > --- > Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 0/5 v2] DMA-BUF Heaps (destaging ION)
On Tue, Mar 05, 2019 at 12:54:28PM -0800, John Stultz wrote: > Here is a initial RFC of the dma-buf heaps patchset Andrew and I > have been working on which tries to destage a fair chunk of ION > functionality. > > The patchset implements per-heap devices which can be opened > directly and then an ioctl is used to allocate a dmabuf from the > heap. > > The interface is similar, but much simpler then IONs, only > providing an ALLOC ioctl. > > Also, I've provided simple system and cma heaps. The system > heap in particular is missing the page-pool optimizations ION > had, but works well enough to validate the interface. > > I've booted and tested these patches with AOSP on the HiKey960 > using the kernel tree here: > > https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/dma-buf-heap > > And the userspace changes here: > https://android-review.googlesource.com/c/device/linaro/hikey/+/909436 What upstream driver will use this eventualy ? And why is it needed ? Cheers, Jérôme ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109281] [CI][SHARDS] igt@gem_ctx_isolation@* - skip - WARNING: GEN not recognized!
https://bugs.freedesktop.org/show_bug.cgi?id=109281 Chris Wilson changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #4 from Chris Wilson --- commit 67c72249d963a30a681c204b5aad1563dc98d92c Author: Dale B Stimson Date: Mon Mar 4 17:03:08 2019 -0800 gem_ctx_isolation.c - Gen11 enabling for context isolation test -- 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: [RFC][PATCH 1/5 v2] dma-buf: Add dma-buf heaps framework
On 3/15/19 2:29 PM, John Stultz wrote: On Fri, Mar 15, 2019 at 1:18 PM Laura Abbott wrote: On 3/5/19 12:54 PM, John Stultz wrote: +DMA-BUF HEAPS FRAMEWORK +M: Laura Abbott +R: Liam Mark +R: Brian Starkey +R: "Andrew F. Davis" +R: John Stultz +S: Maintained +L: linux-me...@vger.kernel.org +L: dri-devel@lists.freedesktop.org +L: linaro-mm-...@lists.linaro.org (moderated for non-subscribers) +F: include/uapi/linux/dma-heap.h +F: include/linux/dma-heap.h +F: drivers/dma-buf/dma-heap.c +F: drivers/dma-buf/heaps/* +T: git git://anongit.freedesktop.org/drm/drm-misc So I talked about this with Sumit privately but I think it might make sense to have me step down as maintainer when this goes out of staging. I mostly worked on Ion at my previous position and anything I do now is mostly a side project. I still want to see it succeed which is why I took on the maintainer role but I don't want to become blocking for people who have a stronger vision about where this needs to go (see also, I'm not working with this on a daily basis). If you just want someone to help review or take patches to be pulled, I'm happy to do so but I'd hate to become the bottleneck on getting things done for people who are attempting to do real work. I worry this will make everyone to touch the side of their nose and yell "NOT IT!" :) First of all, thank you so much for your efforts maintaining ION along with your attempts to drag out requirements from interested parties and the numerous attempts to get collaborative discussion going at countless conferences! Your persistence and continual nudging in the face of apathetic private users of the code probably cannot be appreciated enough! Your past practical experience with ION and active work with the upstream community made you a stand out pick for this, but I understand not wanting to be eternally stuck with a maintainership if your not active in the area. I'm happy to volunteer as a neutral party, but I worry my limited experience with some of the more complicated usage would make my opinions less informed then they probably need to be. Further, as a neutral party, Sumit would probably be a better pick since he's already maintaining the dmabuf core. Honestly if you're doing the work to re-write everything, I think you're more than qualified to be the maintainer. I would also support Sumit as well. So I'd nominate Andrew, Liam or Benjamin (or all three?) as they all have more practical experience enabling past ION heaps on real devices and have demonstrated active interest in working in the community. I do think this would benefit both from multiple maintainers and from maintainers who are actively using the framework. Like I said, I can still be a maintainer but I think having some comaintainers would be very helpful (and I'd support any of the names you've suggested) So, in other words... NOT IT! :) I think you have to shout "Noes goes" first. :) -john Thanks, Laura P.S. For the benefit of anyone who's confused, https://en.wikipedia.org/wiki/Nose_goes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 202799] amd/dc: right monitor sometimes never comes back up on dual-display setup after locking session
https://bugzilla.kernel.org/show_bug.cgi?id=202799 --- Comment #7 from Clément Guérin (li...@protonmail.com) --- Created attachment 281855 --> https://bugzilla.kernel.org/attachment.cgi?id=281855=edit dmesg drm.debug=4 Here's the log as requested. -- 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] drm/selftest:- Adding a new format(whose cpp=0) to be tested by igt_check_drm_format_min_pitch
Hi Ayan, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v5.0 next-20190306] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Ayan-Halder/drm-selftest-Adding-a-new-format-whose-cpp-0-to-be-tested-by-igt_check_drm_format_min_pitch/20190316-050437 config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 8.1.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=8.1.0 make.cross ARCH=xtensa All errors (new ones prefixed by >>): drivers/gpu//drm/selftests/test-drm_format.c: In function 'igt_check_drm_format_min_pitch': >> drivers/gpu//drm/selftests/test-drm_format.c:280:25: error: >> 'DRM_FORMAT_VUY101010' undeclared (first use in this function); did you mean >> 'DRM_FORMAT_BGRA1010102'? info = drm_format_info(DRM_FORMAT_VUY101010); ^~~~ DRM_FORMAT_BGRA1010102 drivers/gpu//drm/selftests/test-drm_format.c:280:25: note: each undeclared identifier is reported only once for each function it appears in vim +280 drivers/gpu//drm/selftests/test-drm_format.c 100 101 int igt_check_drm_format_min_pitch(void *ignored) 102 { 103 const struct drm_format_info *info = NULL; 104 105 /* Test invalid arguments */ 106 FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0); 107 FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0); 108 FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0); 109 110 /* Test 1 plane 8 bits per pixel format */ 111 info = drm_format_info(DRM_FORMAT_RGB332); 112 FAIL_ON(!info); 113 FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0); 114 FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0); 115 FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0); 116 117 FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 1); 118 FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 2); 119 FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 640); 120 FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 1024); 121 FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 1920); 122 FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 4096); 123 FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 671); 124 FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) != 125 (uint64_t)UINT_MAX); 126 FAIL_ON(drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)) != 127 (uint64_t)(UINT_MAX - 1)); 128 129 /* Test 1 plane 16 bits per pixel format */ 130 info = drm_format_info(DRM_FORMAT_XRGB); 131 FAIL_ON(!info); 132 FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0); 133 FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0); 134 FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0); 135 136 FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 2); 137 FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 4); 138 FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 1280); 139 FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 2048); 140 FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 3840); 141 FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 8192); 142 FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 1342); 143 FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) != 144 (uint64_t)UINT_MAX * 2); 145 FAIL_ON(drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)) != 146 (uint64_t)(UINT_MAX - 1) * 2); 147 148 /* Test 1 plane 24 bits per pixel format */ 149 info = drm_format_info(DRM_FORMAT_RGB888); 150 FAIL_ON(!info); 151 FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0); 152 FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0); 153 FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0); 154 155 FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 3); 156 FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 6); 157 FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 1920); 158 FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 3072); 159 FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 5760); 160 FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 12288); 161
Re: [git pull] drm fixes for 5.1-rc1
The pull request you sent on Fri, 15 Mar 2019 10:35:19 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-next-2019-03-15 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/8264fd046a0884d6bf475a784412978dbbd93175 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [GIT PULL] fbdev changes for v5.1
The pull request you sent on Fri, 15 Mar 2019 12:12:27 +0100: > https://github.com/bzolnier/linux.git tags/fbdev-v5.1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/2b9c272cf5cd81708e51b4ce3e432ce9566cfa47 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/5 v2] dma-buf: Add dma-buf heaps framework
On Fri, Mar 15, 2019 at 1:18 PM Laura Abbott wrote: > > On 3/5/19 12:54 PM, John Stultz wrote: > > +DMA-BUF HEAPS FRAMEWORK > > +M: Laura Abbott > > +R: Liam Mark > > +R: Brian Starkey > > +R: "Andrew F. Davis" > > +R: John Stultz > > +S: Maintained > > +L: linux-me...@vger.kernel.org > > +L: dri-devel@lists.freedesktop.org > > +L: linaro-mm-...@lists.linaro.org (moderated for non-subscribers) > > +F: include/uapi/linux/dma-heap.h > > +F: include/linux/dma-heap.h > > +F: drivers/dma-buf/dma-heap.c > > +F: drivers/dma-buf/heaps/* > > +T: git git://anongit.freedesktop.org/drm/drm-misc > > So I talked about this with Sumit privately but I think > it might make sense to have me step down as maintainer when > this goes out of staging. I mostly worked on Ion at my > previous position and anything I do now is mostly a side > project. I still want to see it succeed which is why I > took on the maintainer role but I don't want to become blocking > for people who have a stronger vision about where this needs > to go (see also, I'm not working with this on a daily basis). > > If you just want someone to help review or take patches > to be pulled, I'm happy to do so but I'd hate to become > the bottleneck on getting things done for people who > are attempting to do real work. I worry this will make everyone to touch the side of their nose and yell "NOT IT!" :) First of all, thank you so much for your efforts maintaining ION along with your attempts to drag out requirements from interested parties and the numerous attempts to get collaborative discussion going at countless conferences! Your persistence and continual nudging in the face of apathetic private users of the code probably cannot be appreciated enough! Your past practical experience with ION and active work with the upstream community made you a stand out pick for this, but I understand not wanting to be eternally stuck with a maintainership if your not active in the area. I'm happy to volunteer as a neutral party, but I worry my limited experience with some of the more complicated usage would make my opinions less informed then they probably need to be. Further, as a neutral party, Sumit would probably be a better pick since he's already maintaining the dmabuf core. So I'd nominate Andrew, Liam or Benjamin (or all three?) as they all have more practical experience enabling past ION heaps on real devices and have demonstrated active interest in working in the community. So, in other words... NOT IT! :) -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/5 v2] dma-buf: Add dma-buf heaps framework
On 3/15/19 3:18 PM, Laura Abbott wrote: > On 3/5/19 12:54 PM, John Stultz wrote: >> +DMA-BUF HEAPS FRAMEWORK >> +M: Laura Abbott >> +R: Liam Mark >> +R: Brian Starkey >> +R: "Andrew F. Davis" >> +R: John Stultz >> +S: Maintained >> +L: linux-me...@vger.kernel.org >> +L: dri-devel@lists.freedesktop.org >> +L: linaro-mm-...@lists.linaro.org (moderated for non-subscribers) >> +F: include/uapi/linux/dma-heap.h >> +F: include/linux/dma-heap.h >> +F: drivers/dma-buf/dma-heap.c >> +F: drivers/dma-buf/heaps/* >> +T: git git://anongit.freedesktop.org/drm/drm-misc > > So I talked about this with Sumit privately but I think > it might make sense to have me step down as maintainer when > this goes out of staging. I mostly worked on Ion at my > previous position and anything I do now is mostly a side > project. I still want to see it succeed which is why I > took on the maintainer role but I don't want to become blocking > for people who have a stronger vision about where this needs > to go (see also, I'm not working with this on a daily basis). > > If you just want someone to help review or take patches > to be pulled, I'm happy to do so but I'd hate to become > the bottleneck on getting things done for people who > are attempting to do real work. > We could consider this as an "ION inspired" framework, and treat it like an extension of DMA-BUF. In which case Sumit could become the default Maintainer if he's up for it. Andrew > Thanks, > Laura ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 5/5 v2] kselftests: Add dma-heap test
On 3/15/19 1:13 PM, John Stultz wrote: On Fri, Mar 15, 2019 at 1:07 PM Laura Abbott wrote: On 3/6/19 9:01 AM, John Stultz wrote: On Wed, Mar 6, 2019 at 8:14 AM Benjamin Gaignard wrote: Le mar. 5 mars 2019 à 21:54, John Stultz a écrit : + + printf("Allocating 1 MEG\n"); + ret = dmabuf_heap_alloc(heap_fd, ONE_MEG, 0, _fd); + if (ret) + goto out; + + /* DO SOMETHING WITH THE DMABUF HERE? */ You can do a call to mmap and write a pattern in the buffer. Yea. I can also do some invalid allocations to make sure things fail properly. But I was talking a bit w/ Sumit about the lack of any general dmabuf tests, and am curious if we need to have a importer device driver that can validate its a real dmabuf and exercise more of the dmabuf ops. thanks -john There's the vgem driver in drm. I did some work to clean that up so it could take an import af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces") . I mostly used it for private tests and never ended up upstreaming any of the tests. Thanks for the poitner, I'll check that out as well! Also, if you still have them around, I'd be interested in checking out the tests to try to get something integrated into kselftest. Talking with Brian yesterday, there was some thought that we should try to put together some sort of example dmabuf pipeline that isn't hardware dependent and can be used to demonstrate the usage model as well as validate the frameworks and maybe even benchmark some of the ideas floating around right now. So suggestions here would be welcome! So the existing ion selftest (tools/testing/selftests/android/ion) does make use of the import to do some very simple tests. I can't seem to find the more complex tests I had though they may have been lost during my last machine move :( I do think building off of vgem would be a good first step for a testing pipeline, although I worry we wouldn't be able to measure caching effects without a real device since setting up coherency testing otherwise seems error prone. Thanks, Laura thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 0/5 v2] DMA-BUF Heaps (destaging ION)
On 3/5/19 12:54 PM, John Stultz wrote: Here is a initial RFC of the dma-buf heaps patchset Andrew and I have been working on which tries to destage a fair chunk of ION functionality. The patchset implements per-heap devices which can be opened directly and then an ioctl is used to allocate a dmabuf from the heap. The interface is similar, but much simpler then IONs, only providing an ALLOC ioctl. Also, I've provided simple system and cma heaps. The system heap in particular is missing the page-pool optimizations ION had, but works well enough to validate the interface. I've booted and tested these patches with AOSP on the HiKey960 using the kernel tree here: https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/dma-buf-heap And the userspace changes here: https://android-review.googlesource.com/c/device/linaro/hikey/+/909436 Compared to ION, this patchset is missing the system-contig, carveout and chunk heaps, as I don't have a device that uses those, so I'm unable to do much useful validation there. Additionally we have no upstream users of chunk or carveout, and the system-contig has been deprecated in the common/andoid-* kernels, so this should be ok. I've also removed the stats accounting for now, since it should be implemented by the heaps themselves. Eventual TODOS: * Reimplement page-pool for system heap (working on this) * Add stats accounting to system/cma heaps * Make the kselftest actually useful * Add other heaps folks see as useful (would love to get some help from actual carveout/chunk users)! That said, the main user-interface is shaping up and I wanted to get some input on the device model (particularly from GreKH) and any other API/ABI specific input. thanks -john Cc: Laura Abbott Cc: Benjamin Gaignard Cc: Greg KH Cc: Sumit Semwal Cc: Liam Mark Cc: Brian Starkey Cc: Andrew F. Davis Cc: Chenbo Feng Cc: Alistair Strachan Cc: dri-devel@lists.freedesktop.org Andrew F. Davis (1): dma-buf: Add dma-buf heaps framework John Stultz (4): dma-buf: heaps: Add heap helpers dma-buf: heaps: Add system heap to dmabuf heaps dma-buf: heaps: Add CMA heap to dmabuf heapss kselftests: Add dma-heap test MAINTAINERS| 16 + drivers/dma-buf/Kconfig| 10 + drivers/dma-buf/Makefile | 2 + drivers/dma-buf/dma-heap.c | 191 drivers/dma-buf/heaps/Kconfig | 14 + drivers/dma-buf/heaps/Makefile | 4 + drivers/dma-buf/heaps/cma_heap.c | 164 ++ drivers/dma-buf/heaps/heap-helpers.c | 335 + drivers/dma-buf/heaps/heap-helpers.h | 48 +++ drivers/dma-buf/heaps/system_heap.c| 132 include/linux/dma-heap.h | 65 include/uapi/linux/dma-heap.h | 52 tools/testing/selftests/dmabuf-heaps/Makefile | 11 + tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 96 ++ 14 files changed, 1140 insertions(+) create mode 100644 drivers/dma-buf/dma-heap.c create mode 100644 drivers/dma-buf/heaps/Kconfig create mode 100644 drivers/dma-buf/heaps/Makefile create mode 100644 drivers/dma-buf/heaps/cma_heap.c create mode 100644 drivers/dma-buf/heaps/heap-helpers.c create mode 100644 drivers/dma-buf/heaps/heap-helpers.h create mode 100644 drivers/dma-buf/heaps/system_heap.c create mode 100644 include/linux/dma-heap.h create mode 100644 include/uapi/linux/dma-heap.h create mode 100644 tools/testing/selftests/dmabuf-heaps/Makefile create mode 100644 tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c This is looking really great. Thanks for doing the work to push this forward. It seems like we're in general agreement about much of this. Which of the issues that have come up do you think are a "hard no" to keeping this from going in? Thanks, Laura ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/5 v2] dma-buf: Add dma-buf heaps framework
On 3/15/19 3:54 AM, Christoph Hellwig wrote: >> +static int dma_heap_release(struct inode *inode, struct file *filp) >> +{ >> +filp->private_data = NULL; >> + >> +return 0; >> +} > > No point in clearing ->private_data, the file is about to be freed. > This was leftover from when release had some memory to free, will remove. >> + >> +static long dma_heap_ioctl(struct file *filp, unsigned int cmd, >> + unsigned long arg) > > Pleae don't use the weird legacy filp naming, file is a perfectly > valid and readable default name for struct file pointers. > Thanks for info, I saw both used and this was used where I found the prototype so I used it too, will fix. >> +{ >> +switch (cmd) { >> +case DMA_HEAP_IOC_ALLOC: >> +{ >> +struct dma_heap_allocation_data heap_allocation; >> +struct dma_heap *heap = filp->private_data; >> +int fd; > > Please split each ioctl into a separate function from the very start, > otherwise this will grow into a spaghetty mess sooner than you can > see cheese. > Good idea, will fix. >> +dev_ret = device_create(dma_heap_class, >> +NULL, >> +heap->heap_devt, >> +NULL, >> +heap->name); > > No need this weird argument alignment. > I kinda like it this way, if everything cant fit on one line then everything gets its own line, seems more consistent. If there is strong objection I can fix. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/5 v2] dma-buf: Add dma-buf heaps framework
On 3/5/19 12:54 PM, John Stultz wrote: +DMA-BUF HEAPS FRAMEWORK +M: Laura Abbott +R: Liam Mark +R: Brian Starkey +R: "Andrew F. Davis" +R: John Stultz +S: Maintained +L: linux-me...@vger.kernel.org +L: dri-devel@lists.freedesktop.org +L: linaro-mm-...@lists.linaro.org (moderated for non-subscribers) +F: include/uapi/linux/dma-heap.h +F: include/linux/dma-heap.h +F: drivers/dma-buf/dma-heap.c +F: drivers/dma-buf/heaps/* +T: git git://anongit.freedesktop.org/drm/drm-misc So I talked about this with Sumit privately but I think it might make sense to have me step down as maintainer when this goes out of staging. I mostly worked on Ion at my previous position and anything I do now is mostly a side project. I still want to see it succeed which is why I took on the maintainer role but I don't want to become blocking for people who have a stronger vision about where this needs to go (see also, I'm not working with this on a daily basis). If you just want someone to help review or take patches to be pulled, I'm happy to do so but I'd hate to become the bottleneck on getting things done for people who are attempting to do real work. Thanks, Laura ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 5/5 v2] kselftests: Add dma-heap test
On Fri, Mar 15, 2019 at 1:07 PM Laura Abbott wrote: > > On 3/6/19 9:01 AM, John Stultz wrote: > > On Wed, Mar 6, 2019 at 8:14 AM Benjamin Gaignard > > wrote: > >> Le mar. 5 mars 2019 à 21:54, John Stultz a écrit : > >>> + > >>> + printf("Allocating 1 MEG\n"); > >>> + ret = dmabuf_heap_alloc(heap_fd, ONE_MEG, 0, _fd); > >>> + if (ret) > >>> + goto out; > >>> + > >>> + /* DO SOMETHING WITH THE DMABUF HERE? */ > >> > >> You can do a call to mmap and write a pattern in the buffer. > > > > Yea. I can also do some invalid allocations to make sure things fail > > properly. > > > > But I was talking a bit w/ Sumit about the lack of any general dmabuf > > tests, and am curious if we need to have a importer device driver that > > can validate its a real dmabuf and exercise more of the dmabuf ops. > > > > thanks > > -john > > > > There's the vgem driver in drm. I did some work to clean that > up so it could take an import af33a9190d02 ("drm/vgem: Enable dmabuf import > interfaces") . I mostly used it for private tests and never ended > up upstreaming any of the tests. Thanks for the poitner, I'll check that out as well! Also, if you still have them around, I'd be interested in checking out the tests to try to get something integrated into kselftest. Talking with Brian yesterday, there was some thought that we should try to put together some sort of example dmabuf pipeline that isn't hardware dependent and can be used to demonstrate the usage model as well as validate the frameworks and maybe even benchmark some of the ideas floating around right now. So suggestions here would be welcome! thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 4/5 v2] dma-buf: heaps: Add CMA heap to dmabuf heapss
On Fri, Mar 15, 2019 at 2:06 AM Christoph Hellwig wrote: > > On Tue, Mar 05, 2019 at 12:54:32PM -0800, John Stultz wrote: > > This adds a CMA heap, which allows userspace to allocate > > a dma-buf of contiguous memory out of a CMA region. > > With my previous suggestion of DMA API usage you'd get CMA support for > free in the system one instead of all this duplicate code.. Hey Christoph! Thanks for the review here! I'm still digesting your comments, so apologies if I misunderstand. On the point here, unless you're referring to some earlier suggestion on a previous discussion (and not the system heap feedback), part of the reason there are separate heaps is to allow Android to be able to optimize where the allocations are coming from to best match the use case. So they only want to allocate CMA backed dmabufs when the use case has devices that require it, or they may even want to have separate a CMA region reserved for a specific use case (like camera buffers). Similarly for any future heap for allocating secure dma-bufs. So while in the implementation we can consolidate the code more, but we'd still probably want to have separate heaps. Does that make sense? Am I misinterpreting your feedback? thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 5/5 v2] kselftests: Add dma-heap test
On 3/6/19 9:01 AM, John Stultz wrote: On Wed, Mar 6, 2019 at 8:14 AM Benjamin Gaignard wrote: Le mar. 5 mars 2019 à 21:54, John Stultz a écrit : + + printf("Allocating 1 MEG\n"); + ret = dmabuf_heap_alloc(heap_fd, ONE_MEG, 0, _fd); + if (ret) + goto out; + + /* DO SOMETHING WITH THE DMABUF HERE? */ You can do a call to mmap and write a pattern in the buffer. Yea. I can also do some invalid allocations to make sure things fail properly. But I was talking a bit w/ Sumit about the lack of any general dmabuf tests, and am curious if we need to have a importer device driver that can validate its a real dmabuf and exercise more of the dmabuf ops. thanks -john There's the vgem driver in drm. I did some work to clean that up so it could take an import af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces") . I mostly used it for private tests and never ended up upstreaming any of the tests. Thanks, Laura ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Missing Thunderbolt 3 PCI-E atomics support
As Mika said pci atomics have to be supported by the bridge for the device to be able to use them. Alex On Fri, Mar 15, 2019 at 3:46 PM Dieter Nützel wrote: > > Hello to both of you, Tim and Mika, > > sorry that I step in so late, but after I've read that Tim's laptop > didn't have PCIe atomics (is it realy PCIe 2.xx?) I have some hints, > too. Question: Is PCIe 3.xx+ needed together with Thunderbold 3 or is > PCIe 2.xx enaugth (like Tim's laptop)? > > If Tim's laptop is realy only PCIe 2.xx (_without_ atomics) like mine > 'old' Intel Xeon X3470 (3400 Series Chipset) _without_ atomics you/we > only have the option running Clover or AMD-pro's OpenCL. ROCm isn't an > option with Polaris (20/RX580, here), currently. But we had some > questions about AMD (Polaris) running as eGPU on one of our lists > (mesa-devel / dri-devel / amd-gfx), lately. > > I've CC'ed Alex. > > Have a look here (ROCm 2.2), too: > https://www.phoronix.com/forums/forum/linux-graphics-x-org-drivers/open-source-amd-linux/1086267-radeon-rocm-2-2-released-with-vega-20-optimization-caffe2-multi-gpu-training?p=1086387#post1086387 > > Greetings, > Dieter > > Am 14.03.2019 19:36, schrieb Timur Kristóf: > > On Thu, 2019-03-14 at 20:17 +0200, Mika Westerberg wrote: > >> > > > Here is the output of 'lspci -vv': > >> > > > https://pastebin.com/Qt5RUFVc > >> > > > >> > > The root port (1c.4) says this: > >> > > > >> > > DevCap2: Completion Timeout: Range ABC, TimeoutDis+, LTR+, > >> > > OBFF > >> > > Not Supported ARIFwd+ > >> > > AtomicOpsCap: Routing- 32bit- 64bit- 128bitCAS- > >> > > > >> > > Not knowing much about AtomicOps but to me this looks like the > >> > > root > >> > > port > >> > > does not support the feature. > >> > > >> > What kind of output should lspci show if the feature were > >> > supported? > >> > >> The AMD card has this: > >> > >> DevCap2: Completion Timeout: Not Supported, TimeoutDis-, > >> LTR+, OBFF Not Supported > >>AtomicOpsCap: 32bit+ 64bit+ 128bitCAS- > >> > >> so I would expect something similar on the root port side as > >> pci_enable_atomic_ops_to_root() fails otherwise with mask of > >> PCI_EXP_DEVCAP2_ATOMIC_COMP32 | PCI_EXP_DEVCAP2_ATOMIC_COMP64 that > >> the > >> AMD driver requests. > >> > >> > As far as I understand the root port is integrated in the CPU, or > >> > in > >> > the chipset maybe? It says it's a Sunrise Point-LP, and I googled > >> > it > >> > but was unable to find a spec sheet. > >> > >> You can find it here: > >> > >> > >> https://www.intel.com/content/www/us/en/products/docs/processors/core/6th-gen-core-pch-u-y-io-datasheet-vol-2.html > >> > >> Pages 845-826 show the DEVCAP2 register for the 1c.4 (D28/F4) and it > >> does not seem to have AtomicOps caps set. > > > > This would be the 8th gen (8550U) but I assume it has similar > > capabilities to the 6th gen. So, it seems that this is a hardware > > limitation of the chipset. > > > > Thanks Mika for clearing that up. > > > > Best regards, > > Tim ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Missing Thunderbolt 3 PCI-E atomics support
Hello to both of you, Tim and Mika, sorry that I step in so late, but after I've read that Tim's laptop didn't have PCIe atomics (is it realy PCIe 2.xx?) I have some hints, too. Question: Is PCIe 3.xx+ needed together with Thunderbold 3 or is PCIe 2.xx enaugth (like Tim's laptop)? If Tim's laptop is realy only PCIe 2.xx (_without_ atomics) like mine 'old' Intel Xeon X3470 (3400 Series Chipset) _without_ atomics you/we only have the option running Clover or AMD-pro's OpenCL. ROCm isn't an option with Polaris (20/RX580, here), currently. But we had some questions about AMD (Polaris) running as eGPU on one of our lists (mesa-devel / dri-devel / amd-gfx), lately. I've CC'ed Alex. Have a look here (ROCm 2.2), too: https://www.phoronix.com/forums/forum/linux-graphics-x-org-drivers/open-source-amd-linux/1086267-radeon-rocm-2-2-released-with-vega-20-optimization-caffe2-multi-gpu-training?p=1086387#post1086387 Greetings, Dieter Am 14.03.2019 19:36, schrieb Timur Kristóf: On Thu, 2019-03-14 at 20:17 +0200, Mika Westerberg wrote: > > > Here is the output of 'lspci -vv': > > > https://pastebin.com/Qt5RUFVc > > > > The root port (1c.4) says this: > > > > DevCap2: Completion Timeout: Range ABC, TimeoutDis+, LTR+, > > OBFF > > Not Supported ARIFwd+ > > AtomicOpsCap: Routing- 32bit- 64bit- 128bitCAS- > > > > Not knowing much about AtomicOps but to me this looks like the > > root > > port > > does not support the feature. > > What kind of output should lspci show if the feature were > supported? The AMD card has this: DevCap2: Completion Timeout: Not Supported, TimeoutDis-, LTR+, OBFF Not Supported AtomicOpsCap: 32bit+ 64bit+ 128bitCAS- so I would expect something similar on the root port side as pci_enable_atomic_ops_to_root() fails otherwise with mask of PCI_EXP_DEVCAP2_ATOMIC_COMP32 | PCI_EXP_DEVCAP2_ATOMIC_COMP64 that the AMD driver requests. > As far as I understand the root port is integrated in the CPU, or > in > the chipset maybe? It says it's a Sunrise Point-LP, and I googled > it > but was unable to find a spec sheet. You can find it here: https://www.intel.com/content/www/us/en/products/docs/processors/core/6th-gen-core-pch-u-y-io-datasheet-vol-2.html Pages 845-826 show the DEVCAP2 register for the 1c.4 (D28/F4) and it does not seem to have AtomicOps caps set. This would be the 8th gen (8550U) but I assume it has similar capabilities to the 6th gen. So, it seems that this is a hardware limitation of the chipset. Thanks Mika for clearing that up. Best regards, Tim ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 14/18] drm: writeback: Add job prepare and cleanup operations
On Wed, Mar 13, 2019 at 02:05:28AM +0200, Laurent Pinchart wrote: > As writeback jobs contain a framebuffer, drivers may need to prepare and > cleanup them the same way they can prepare and cleanup framebuffers for > planes. Add two new optional connector helper operations, > .prepare_writeback_job() and .cleanup_writeback_job() to support this. > > The job prepare operation is called from > drm_atomic_helper_prepare_planes() to avoid a new atomic commit helper > that would need to be called by all drivers not using > drm_atomic_helper_commit(). The job cleanup operation is called from the > existing drm_writeback_cleanup_job() function, invoked both when > destroying the job as part of a aborted commit, or when the job > completes. > > The drm_writeback_job structure is extended with a priv field to let > drivers store per-job data, such as mappings related to the writeback > framebuffer. > > For internal plumbing reasons the drm_writeback_job structure needs to > store a back-pointer to the drm_writeback_connector. To avoid pushing > too much writeback-specific knowledge to drm_atomic_uapi.c, create a > drm_writeback_set_fb() function, move the writeback job setup code > there, and set the connector backpointer. The prepare_signaling() > function doesn't need to allocate writeback jobs and can ignore > connectors without a job, as it is called after the writeback jobs are > allocated to store framebuffers, and a writeback fence with a > framebuffer is an invalid configuration that gets rejected by the commit > check. > > Signed-off-by: Laurent Pinchart Reviewed-by: Liviu Dudau Best regards, Liviu > --- > Changes since v5: > > - Export drm_writeback_prepare_job() > - Check for .prepare_writeback_job() in drm_writeback_prepare_job() > --- > drivers/gpu/drm/drm_atomic_helper.c | 11 ++ > drivers/gpu/drm/drm_atomic_uapi.c| 31 + > drivers/gpu/drm/drm_writeback.c | 44 > include/drm/drm_modeset_helper_vtables.h | 7 > include/drm/drm_writeback.h | 28 ++- > 5 files changed, 97 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 6fe2303fccd9..70a4886c6e65 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2245,10 +2245,21 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done); > int drm_atomic_helper_prepare_planes(struct drm_device *dev, >struct drm_atomic_state *state) > { > + struct drm_connector *connector; > + struct drm_connector_state *new_conn_state; > struct drm_plane *plane; > struct drm_plane_state *new_plane_state; > int ret, i, j; > > + for_each_new_connector_in_state(state, connector, new_conn_state, i) { > + if (!new_conn_state->writeback_job) > + continue; > + > + ret = drm_writeback_prepare_job(new_conn_state->writeback_job); > + if (ret < 0) > + return ret; > + } > + > for_each_new_plane_in_state(state, plane, new_plane_state, i) { > const struct drm_plane_helper_funcs *funcs; > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > index c40889888a16..e802152a01ad 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -647,28 +647,15 @@ drm_atomic_plane_get_property(struct drm_plane *plane, > return 0; > } > > -static struct drm_writeback_job * > -drm_atomic_get_writeback_job(struct drm_connector_state *conn_state) > -{ > - WARN_ON(conn_state->connector->connector_type != > DRM_MODE_CONNECTOR_WRITEBACK); > - > - if (!conn_state->writeback_job) > - conn_state->writeback_job = > - kzalloc(sizeof(*conn_state->writeback_job), GFP_KERNEL); > - > - return conn_state->writeback_job; > -} > - > static int drm_atomic_set_writeback_fb_for_connector( > struct drm_connector_state *conn_state, > struct drm_framebuffer *fb) > { > - struct drm_writeback_job *job = > - drm_atomic_get_writeback_job(conn_state); > - if (!job) > - return -ENOMEM; > + int ret; > > - drm_framebuffer_assign(>fb, fb); > + ret = drm_writeback_set_fb(conn_state, fb); > + if (ret < 0) > + return ret; > > if (fb) > DRM_DEBUG_ATOMIC("Set [FB:%d] for connector state %p\n", > @@ -1158,19 +1145,17 @@ static int prepare_signaling(struct drm_device *dev, > > for_each_new_connector_in_state(state, conn, conn_state, i) { > struct drm_writeback_connector *wb_conn; > - struct drm_writeback_job *job; > struct drm_out_fence_state *f; > struct dma_fence *fence; > s32 __user *fence_ptr; > > + if
[Bug 102646] Screen flickering under amdgpu-experimental [buggy auto power profile]
https://bugs.freedesktop.org/show_bug.cgi?id=102646 --- Comment #68 from bmil...@gmail.com --- At this point we need at least a workaround. Add a runtime kernel parameter that keeps memory clock maxed up always, even after sleep/wake and resolution changes. -- 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 v2 5/6] drm: rcar-du: Create a group state object
Create a new private state object for the DU groups, and move the initialisation of a group object to a new function rcar_du_group_init(). Signed-off-by: Kieran Bingham --- v2: - No change drivers/gpu/drm/rcar-du/rcar_du_group.c | 81 + drivers/gpu/drm/rcar-du/rcar_du_group.h | 22 +++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 27 ++--- 3 files changed, 107 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c index 9eee47969e77..9c82d666f170 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -26,6 +26,10 @@ #include #include +#include +#include +#include + #include "rcar_du_drv.h" #include "rcar_du_group.h" #include "rcar_du_regs.h" @@ -351,3 +355,80 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp) return rcar_du_set_dpad0_vsp1_routing(rgrp->dev); } + +static struct drm_private_state * +rcar_du_group_atomic_duplicate_state(struct drm_private_obj *obj) +{ + struct rcar_du_group_state *state; + + if (WARN_ON(!obj->state)) + return NULL; + + state = kzalloc(sizeof(*state), GFP_KERNEL); + if (state == NULL) + return NULL; + + __drm_atomic_helper_private_obj_duplicate_state(obj, >state); + + return >state; +} + +static void rcar_du_group_atomic_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + kfree(to_rcar_group_state(state)); +} + +static const struct drm_private_state_funcs rcar_du_group_state_funcs = { + .atomic_duplicate_state = rcar_du_group_atomic_duplicate_state, + .atomic_destroy_state = rcar_du_group_atomic_destroy_state, +}; + +/* + * rcar_du_group_init - Initialise and reset a group object + * + * Return 0 in case of success or a negative error code otherwise. + */ +int rcar_du_group_init(struct rcar_du_device *rcdu, struct rcar_du_group *rgrp, + unsigned int index) +{ + static const unsigned int mmio_offsets[] = { + DU0_REG_OFFSET, DU2_REG_OFFSET + }; + + struct rcar_du_group_state *state; + + state = kzalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return -ENOMEM; + + drm_atomic_private_obj_init(rcdu->ddev, >private, >state, + _du_group_state_funcs); + + mutex_init(>lock); + + rgrp->dev = rcdu; + rgrp->mmio_offset = mmio_offsets[index]; + rgrp->index = index; + /* Extract the channel mask for this group only. */ + rgrp->channels_mask = (rcdu->info->channels_mask >> (2 * index)) + & GENMASK(1, 0); + rgrp->num_crtcs = hweight8(rgrp->channels_mask); + + /* +* If we have more than one CRTC in this group pre-associate +* the low-order planes with CRTC 0 and the high-order planes +* with CRTC 1 to minimize flicker occurring when the +* association is changed. +*/ + rgrp->dptsr_planes = rgrp->num_crtcs > 1 + ? (rcdu->info->gen >= 3 ? 0x04 : 0xf0) + : 0; + + return 0; +} + +void rcar_du_group_cleanup(struct rcar_du_group *rgrp) +{ + drm_atomic_private_obj_fini(>private); +} diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h index 87950c1f6a52..4b812e167987 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h @@ -12,12 +12,15 @@ #include +#include + #include "rcar_du_plane.h" struct rcar_du_device; /* * struct rcar_du_group - CRTCs and planes group + * @private: The base drm private object * @dev: the DU device * @mmio_offset: registers offset in the device memory map * @index: group index @@ -32,6 +35,8 @@ struct rcar_du_device; * @need_restart: the group needs to be restarted due to a configuration change */ struct rcar_du_group { + struct drm_private_obj private; + struct rcar_du_device *dev; unsigned int mmio_offset; unsigned int index; @@ -49,6 +54,19 @@ struct rcar_du_group { bool need_restart; }; +#define to_rcar_group(s) container_of(s, struct rcar_du_group, private) + +/** + * struct rcar_du_group_state - Driver-specific group state + * @state: base DRM private state + */ +struct rcar_du_group_state { + struct drm_private_state state; +}; + +#define to_rcar_group_state(s) \ + container_of(s, struct rcar_du_group_state, state) + u32 rcar_du_group_read(struct rcar_du_group *rgrp, u32 reg); void rcar_du_group_write(struct rcar_du_group *rgrp, u32 reg, u32 data); @@ -60,4 +78,8 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp); int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu); +int rcar_du_group_init(struct rcar_du_device *rcdu, struct
[PATCH v2 6/6] drm: rcar-du: Add group hooks for atomic-commit
The group can now be handled independently from the CRTC tracking its own state. Introduce an rcar_du_group_atomic_check() call which will iterate the CRTCs to determine the per-state use-count of the group. This use count then allows us to determine if the group should be configured or disabled in our commit tail through the introduction of two new calls rcar_du_group_atomic_{pre,post}_commit(). The existing rcar_du_group_{get,put}() functions are now redundant and removed along with their interactions in the CRTC get/put calls. The groups share clocking with the CRTCs within the group, and so are accessible only when one of its CRTCs has been powered through rcar_du_crtc_atomic_exit_standby(). Signed-off-by: Kieran Bingham --- v2: - All register sequences now maintained. - Clock management is no longer handled by the group (_crtc_{exit,enter}_standby handles this for us) drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 8 -- drivers/gpu/drm/rcar-du/rcar_du_group.c | 101 drivers/gpu/drm/rcar-du/rcar_du_group.h | 14 +++- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 6 ++ 4 files changed, 85 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 2606de788688..fce8bd1d9d25 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -511,14 +511,8 @@ static int rcar_du_crtc_enable(struct rcar_du_crtc *rcrtc) if (ret < 0) goto error_clock; - ret = rcar_du_group_get(rcrtc->group); - if (ret < 0) - goto error_group; - return 0; -error_group: - clk_disable_unprepare(rcrtc->extclock); error_clock: clk_disable_unprepare(rcrtc->clock); return ret; @@ -526,8 +520,6 @@ static int rcar_du_crtc_enable(struct rcar_du_crtc *rcrtc) static void rcar_du_crtc_disable(struct rcar_du_crtc *rcrtc) { - rcar_du_group_put(rcrtc->group); - clk_disable_unprepare(rcrtc->extclock); clk_disable_unprepare(rcrtc->clock); } diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c index 9c82d666f170..1f9504bca0f3 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -24,6 +24,7 @@ */ #include +#include #include #include @@ -172,38 +173,6 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp) mutex_unlock(>lock); } -/* - * rcar_du_group_get - Acquire a reference to the DU channels group - * - * Acquiring the first reference setups core registers. A reference must be held - * before accessing any hardware registers. - * - * This function must be called with the DRM mode_config lock held. - * - * Return 0 in case of success or a negative error code otherwise. - */ -int rcar_du_group_get(struct rcar_du_group *rgrp) -{ - if (rgrp->use_count) - goto done; - - rcar_du_group_setup(rgrp); - -done: - rgrp->use_count++; - return 0; -} - -/* - * rcar_du_group_put - Release a reference to the DU - * - * This function must be called with the DRM mode_config lock held. - */ -void rcar_du_group_put(struct rcar_du_group *rgrp) -{ - --rgrp->use_count; -} - static void __rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start) { struct rcar_du_device *rcdu = rgrp->dev; @@ -384,6 +353,74 @@ static const struct drm_private_state_funcs rcar_du_group_state_funcs = { .atomic_destroy_state = rcar_du_group_atomic_destroy_state, }; +#define for_each_oldnew_group_in_state(__state, __obj, __old_state, __new_state, __i) \ + for_each_oldnew_private_obj_in_state((__state), (__obj), (__old_state), (__new_state), (__i)) \ + for_each_if((__obj)->funcs == _du_group_state_funcs) + +static struct rcar_du_group_state * +rcar_du_get_group_state(struct drm_atomic_state *state, + struct rcar_du_group *rgrp) +{ + struct drm_private_state *pstate; + + pstate = drm_atomic_get_private_obj_state(state, >private); + if (IS_ERR(pstate)) + return ERR_CAST(pstate); + + return to_rcar_group_state(pstate); +} + +int rcar_du_group_atomic_check(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + struct rcar_du_group_state *rstate; + + if (crtc_state->active_changed || crtc_state->mode_changed) { + rstate = rcar_du_get_group_state(state, rcrtc->group); + if (IS_ERR(rstate)) + return PTR_ERR(rstate); + + if (crtc_state->active) +
[PATCH v2 1/6] drm: rcar-du: Link CRTCs to the DU device
The rcar_du_crtc functions have a heavy reliance on the rcar_du_group structure, in many cases just to access the DU device context. To better separate the groups out of the CRTC handling code, give the rcar_du_crtc its own pointer to the device and remove the indirection through the group pointers. Signed-off-by: Kieran Bingham --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 48 +- drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 ++ drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 57fd5c00494b..471ce464654a 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -32,21 +32,21 @@ static u32 rcar_du_crtc_read(struct rcar_du_crtc *rcrtc, u32 reg) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; return rcar_du_read(rcdu, rcrtc->mmio_offset + reg); } static void rcar_du_crtc_write(struct rcar_du_crtc *rcrtc, u32 reg, u32 data) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; rcar_du_write(rcdu, rcrtc->mmio_offset + reg, data); } static void rcar_du_crtc_clr(struct rcar_du_crtc *rcrtc, u32 reg, u32 clr) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; rcar_du_write(rcdu, rcrtc->mmio_offset + reg, rcar_du_read(rcdu, rcrtc->mmio_offset + reg) & ~clr); @@ -54,7 +54,7 @@ static void rcar_du_crtc_clr(struct rcar_du_crtc *rcrtc, u32 reg, u32 clr) static void rcar_du_crtc_set(struct rcar_du_crtc *rcrtc, u32 reg, u32 set) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; rcar_du_write(rcdu, rcrtc->mmio_offset + reg, rcar_du_read(rcdu, rcrtc->mmio_offset + reg) | set); @@ -62,7 +62,7 @@ static void rcar_du_crtc_set(struct rcar_du_crtc *rcrtc, u32 reg, u32 set) void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 set) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; rcrtc->dsysr = (rcrtc->dsysr & ~clr) | set; rcar_du_write(rcdu, rcrtc->mmio_offset + DSYSR, rcrtc->dsysr); @@ -157,10 +157,9 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc *rcrtc, } done: - dev_dbg(rcrtc->group->dev->dev, + dev_dbg(rcrtc->dev->dev, "output:%u, fdpll:%u, n:%u, m:%u, diff:%lu\n", -dpll->output, dpll->fdpll, dpll->n, dpll->m, -best_diff); +dpll->output, dpll->fdpll, dpll->n, dpll->m, best_diff); } struct du_clk_params { @@ -212,7 +211,7 @@ static const struct soc_device_attribute rcar_du_r8a7795_es1[] = { static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) { const struct drm_display_mode *mode = >crtc.state->adjusted_mode; - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; unsigned long mode_clock = mode->clock * 1000; u32 dsmr; u32 escr; @@ -277,7 +276,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) rcar_du_escr_divider(rcrtc->extclock, mode_clock, ESCR_DCLKSEL_DCLKIN, ); - dev_dbg(rcrtc->group->dev->dev, "mode clock %lu %s rate %lu\n", + dev_dbg(rcrtc->dev->dev, "mode clock %lu %s rate %lu\n", mode_clock, params.clk == rcrtc->clock ? "cpg" : "ext", params.rate); @@ -285,7 +284,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) escr = params.escr; } - dev_dbg(rcrtc->group->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr); + dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr); rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr); rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0); @@ -333,7 +332,7 @@ plane_format(struct rcar_du_plane *plane) static void rcar_du_crtc_update_planes(struct rcar_du_crtc *rcrtc) { struct rcar_du_plane *planes[RCAR_DU_NUM_HW_PLANES]; - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; unsigned int num_planes = 0; unsigned int dptsr_planes; unsigned int hwplanes = 0; @@ -463,7 +462,7 @@ static bool rcar_du_crtc_page_flip_pending(struct rcar_du_crtc *rcrtc) static void rcar_du_crtc_wait_page_flip(struct rcar_du_crtc *rcrtc) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; if
[PATCH v2 4/6] drm: rcar-du: Provide for_each_group helper
Refactoring of the group control code will soon require more iteration over the available groups. Simplify this process by introducing a group iteration helper. Signed-off-by: Kieran Bingham --- v2: - no change drivers/gpu/drm/rcar-du/rcar_du_drv.h | 5 + drivers/gpu/drm/rcar-du/rcar_du_kms.c | 10 ++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 1327cd0df90a..1e9dd494e8ac 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h @@ -96,6 +96,11 @@ struct rcar_du_device { unsigned int vspd1_sink; }; +#define for_each_rcdu_group(__rcdu, __group, __i) \ + for ((__i) = 0; (__group = &(rcdu)->groups[__i]), \ +(__i) < DIV_ROUND_UP((rcdu)->num_crtcs, 2); \ +__i++) + static inline bool rcar_du_has(struct rcar_du_device *rcdu, unsigned int feature) { diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index e4befb1937f8..ece92cff2137 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -522,9 +522,9 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) struct drm_device *dev = rcdu->ddev; struct drm_encoder *encoder; + struct rcar_du_group *rgrp; unsigned int dpad0_sources; unsigned int num_encoders; - unsigned int num_groups; unsigned int swindex; unsigned int hwindex; unsigned int i; @@ -565,11 +565,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) return ret; /* Initialize the groups. */ - num_groups = DIV_ROUND_UP(rcdu->num_crtcs, 2); - - for (i = 0; i < num_groups; ++i) { - struct rcar_du_group *rgrp = >groups[i]; - + for_each_rcdu_group(rcdu, rgrp, i) { mutex_init(>lock); rgrp->dev = rcdu; @@ -606,8 +602,6 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) /* Create the CRTCs. */ for (swindex = 0, hwindex = 0; swindex < rcdu->num_crtcs; ++hwindex) { - struct rcar_du_group *rgrp; - /* Skip unpopulated DU channels. */ if (!(rcdu->info->channels_mask & BIT(hwindex))) continue; -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/6] drm: rcar-du: Add CRTC standby helpers
Provide helpers to manage the power state, and initial configuration of the CRTC. rcar_du_crtc_get() and rcar_du_crtc_get() are no longer used, and are removed, simplifying the implementation and removing the initialized flag which was needed to track the state of the CRTC. Signed-off-by: Kieran Bingham --- v2: - Registers sequence confirmed unchanged - re-ordered in the series to handle before groups. - Do not merge rcar_du_crtc_setup(). (now handled by _crtc_pre_commit) drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 69 +++--- drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 7 ++- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 4 ++ 3 files changed, 49 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 471ce464654a..6109a97b0bb9 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -499,17 +499,10 @@ static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) drm_crtc_vblank_on(>crtc); } -static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) +static int rcar_du_crtc_enable(struct rcar_du_crtc *rcrtc) { int ret; - /* -* Guard against double-get, as the function is called from both the -* .atomic_enable() and .atomic_begin() handlers. -*/ - if (rcrtc->initialized) - return 0; - ret = clk_prepare_enable(rcrtc->clock); if (ret < 0) return ret; @@ -523,7 +516,6 @@ static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) goto error_group; rcar_du_crtc_setup(rcrtc); - rcrtc->initialized = true; return 0; @@ -534,14 +526,12 @@ static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) return ret; } -static void rcar_du_crtc_put(struct rcar_du_crtc *rcrtc) +static void rcar_du_crtc_disable(struct rcar_du_crtc *rcrtc) { rcar_du_group_put(rcrtc->group); clk_disable_unprepare(rcrtc->extclock); clk_disable_unprepare(rcrtc->clock); - - rcrtc->initialized = false; } static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc) @@ -655,6 +645,44 @@ static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc, return 0; } +int rcar_du_crtc_atomic_exit_standby(struct drm_device *dev, +struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + + if (crtc_state->active_changed && crtc_state->active) { + int ret = rcar_du_crtc_enable(rcrtc); + + if (ret) + return ret; + } + } + + return 0; +} + +int rcar_du_crtc_atomic_enter_standby(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + + if (crtc_state->active_changed && !crtc_state->active) + rcar_du_crtc_disable(rcrtc); + } + + return 0; +} + static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { @@ -662,8 +690,6 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc, struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(crtc->state); struct rcar_du_device *rcdu = rcrtc->dev; - rcar_du_crtc_get(rcrtc); - /* * On D3/E3 the dot clock is provided by the LVDS encoder attached to * the DU channel. We need to enable its clock output explicitly if @@ -691,7 +717,6 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc, struct rcar_du_device *rcdu = rcrtc->dev; rcar_du_crtc_stop(rcrtc); - rcar_du_crtc_put(rcrtc); if (rcdu->info->lvds_clk_mask & BIT(rcrtc->index) && rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0)) { @@ -720,20 +745,6 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc, WARN_ON(!crtc->state->enable); - /* -* If a mode set is in progress we can be called with the CRTC disabled. -* We thus need to first get and setup the CRTC in order to configure -* planes. We must *not* put the CRTC in .atomic_flush(), as it must be -* kept awake until the .atomic_enable() call that will follow. The get -* operation in .atomic_enable() will in that case be a no-op, and the -* CRTC will be put later in .atomic_disable(). -* -* If a mode set is not in progress the CRTC is enabled, and the -
[PATCH v2 3/6] drm: rcar-du: Add pre/post commit CRTC helpers
Provide helpers to allow CRTC configuration to be separated from the power state handling. rcar_du_crtc_atomic_post_commit() is a no-op, but maintained for API symmetry. --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 25 +++-- drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 5 + drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 ++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 6109a97b0bb9..2606de788688 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -515,8 +515,6 @@ static int rcar_du_crtc_enable(struct rcar_du_crtc *rcrtc) if (ret < 0) goto error_group; - rcar_du_crtc_setup(rcrtc); - return 0; error_group: @@ -683,6 +681,29 @@ int rcar_du_crtc_atomic_enter_standby(struct drm_device *dev, return 0; } +int rcar_du_crtc_atomic_pre_commit(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + + if (crtc_state->active_changed && crtc_state->active) + rcar_du_crtc_setup(rcrtc); + } + + return 0; +} + +int rcar_du_crtc_atomic_post_commit(struct drm_device *dev, + struct drm_atomic_state *state) +{ + return 0; +} + static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index d12d4a788e9f..0b60a6e0b753 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h @@ -105,6 +105,11 @@ int rcar_du_crtc_atomic_exit_standby(struct drm_device *dev, int rcar_du_crtc_atomic_enter_standby(struct drm_device *dev, struct drm_atomic_state *state); +int rcar_du_crtc_atomic_pre_commit(struct drm_device *dev, + struct drm_atomic_state *state); +int rcar_du_crtc_atomic_post_commit(struct drm_device *dev, + struct drm_atomic_state *state); + void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 set); #endif /* __RCAR_DU_CRTC_H__ */ diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index b8da4dfc79d2..e4befb1937f8 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -304,12 +304,14 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state) /* Apply the atomic update. */ rcar_du_crtc_atomic_exit_standby(dev, old_state); + rcar_du_crtc_atomic_pre_commit(dev, old_state); drm_atomic_helper_commit_modeset_disables(dev, old_state); drm_atomic_helper_commit_planes(dev, old_state, DRM_PLANE_COMMIT_ACTIVE_ONLY); drm_atomic_helper_commit_modeset_enables(dev, old_state); + rcar_du_crtc_atomic_post_commit(dev, old_state); rcar_du_crtc_atomic_enter_standby(dev, old_state); drm_atomic_helper_commit_hw_done(old_state); -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/6] drm: rcar-du: Rework CRTC and groups for atomic commits
The DU processes the CRTC and group objects, with various operations storing state of those objects globally, along with performing configuration operations in the atomic_begin handlers. This series aims to refactor the code to establish a private object for the group state, and move all hardware operations to occur only in the commit_tail code paths. The sequence of register writes to the DU is confirmed to be unchanged by this series, however follow on patches after this may begin to modify the sequences. Because of this, these patches are kept as their own series. Note that the rcar_du_crtc_get() in rcar_du_crtc_atomic_begin() is safe to remove, as the call to rcar_du_vsp_atomic_begin() now maps to a no-op, but is kept for API integrity, though it could now be removed. Removal of rcar_du_vsp_atomic_begin() and indeed rcar_du_crtc_atomic_begin() could occur in a later patch, but needs to be synchronised with VSP update. These patches are based on top of my previous cleanup series, which is on top of drm-next. Kieran Bingham (6): drm: rcar-du: Link CRTCs to the DU device drm: rcar-du: Add CRTC standby helpers drm: rcar-du: Add pre/post commit CRTC helpers drm: rcar-du: Provide for_each_group helper drm: rcar-du: Create a group state object drm: rcar-du: Add group hooks for atomic-commit drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 150 +++ drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 14 +- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 5 + drivers/gpu/drm/rcar-du/rcar_du_group.c | 182 +++- drivers/gpu/drm/rcar-du/rcar_du_group.h | 36 - drivers/gpu/drm/rcar-du/rcar_du_kms.c | 49 +++ drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +- 7 files changed, 305 insertions(+), 133 deletions(-) -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/5] drm/rockchip: fix fb references in async update
On 3/15/19 8:29 AM, Michel Dänzer wrote: > On 2019-03-15 11:25 a.m., Boris Brezillon wrote: >> On Fri, 15 Mar 2019 11:11:36 +0100 >> Michel Dänzer wrote: >> >>> On 2019-03-14 6:51 p.m., Helen Koike wrote: On 3/14/19 6:15 AM, Michel Dänzer wrote: > On 2019-03-13 7:08 p.m., Helen Koike wrote: >> On 3/13/19 6:58 AM, Michel Dänzer wrote: >>> On 2019-03-13 4:42 a.m., Tomasz Figa wrote: On Wed, Mar 13, 2019 at 12:52 AM Boris Brezillon wrote: > On Tue, 12 Mar 2019 12:34:45 -0300 > Helen Koike wrote: >> On 3/12/19 3:34 AM, Boris Brezillon wrote: >>> On Mon, 11 Mar 2019 23:21:59 -0300 >>> Helen Koike wrote: >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -912,30 +912,31 @@ static void vop_plane_atomic_async_update(struct drm_plane *plane, struct drm_plane_state *new_state) { struct vop *vop = to_vop(plane->state->crtc); - struct drm_plane_state *plane_state; + struct drm_framebuffer *old_fb = plane->state->fb; - plane_state = plane->funcs->atomic_duplicate_state(plane); - plane_state->crtc_x = new_state->crtc_x; - plane_state->crtc_y = new_state->crtc_y; - plane_state->crtc_h = new_state->crtc_h; - plane_state->crtc_w = new_state->crtc_w; - plane_state->src_x = new_state->src_x; - plane_state->src_y = new_state->src_y; - plane_state->src_h = new_state->src_h; - plane_state->src_w = new_state->src_w; - - if (plane_state->fb != new_state->fb) - drm_atomic_set_fb_for_plane(plane_state, new_state->fb); - - swap(plane_state, plane->state); - - if (plane->state->fb && plane->state->fb != new_state->fb) { + /* + * A scanout can still be occurring, so we can't drop the reference to + * the old framebuffer. To solve this we get a reference to old_fb and + * set a worker to release it later. >>> >>> Hm, doesn't look like an async update to me if we have to wait for >>> the >>> next VBLANK to happen to get the new content on the screen. Maybe we >>> should reject async updates when old_fb != new_fb in the rk >>> ->async_check() hook. >> >> Unless I am misunderstanding this, we don't wait here, we just grab a >> reference to the fb in case it is being still used by the hw, so it >> doesn't get released prematurely. > > I was just reacting to the comment that says the new FB should stay > around until the next VBLANK event happens. If the FB must stay around > that probably means the HW is still using, which made me wonder if > this > HW actually supports async update (where async means "update now and > don't care about about tearing"). Or maybe it takes some time to > switch > to the new FB and waiting for the next VBLANK to release the old FB > was > an easy solution to not wait for the flip to actually happen in > ->async_update() (which is kind of a combination of > async+non-blocking). The hardware switches framebuffers on vblank, so whatever framebuffer is currently being scanned out from needs to stay there until the hardware switches to the new one in shadow registers. If that doesn't happen, you get IOMMU faults and the display controller stops working since we don't have any fault handling currently, just printing a message. >>> >>> Sounds like your hardware doesn't actually support async flips. It's >>> probably better for the driver not to pretend otherwise. >> >> I think wee need to clarify the meaning of the async_update callback >> (and we should clarify it in the docs). >> >> The way I understand what the async_update callback should do is: don't >> block (i.e. don't wait for the next vblank), > > Note that those are two separate things. "Async flips" are about "don't > wait for vblank", not about "don't block". > > >> and update the hw state at some point with the latest state from the >> last call to async_update. >> >> Which means that: any driver can implement the async_update callback, >> independently if it supports changing its state right away or not. >> If hw supports, async_update can change the hw state right away, if not, >> then changes will be applied in the next vblank (it can even amend the
[PATCH 1/3] drm/file: Rehabilitate the firstopen hook for non-legacy drivers
The firstopen DRM driver hook was initially used to perform hardware initialization, which is now considered legacy. Only a single user of firstopen remains at this point (savage). In some specific cases, non-legacy drivers may also need to implement these hooks. For instance on VC4, we need to allocate a 16 MiB buffer for the GPU. Because it's not required for fbcon, it's a waste to allocate it before userspace starts using the DRM device. Using firstopen and lastclose for this allocation seems like the best fit, so re-habilitate the hook to allow it to be called for non-legacy drivers. Signed-off-by: Paul Kocialkowski --- drivers/gpu/drm/drm_file.c | 3 +-- include/drm/drm_drv.h | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index b1838a41ad43..c011b5cbfb6b 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -266,8 +266,7 @@ static int drm_setup(struct drm_device * dev) { int ret; - if (dev->driver->firstopen && - drm_core_check_feature(dev, DRIVER_LEGACY)) { + if (dev->driver->firstopen) { ret = dev->driver->firstopen(dev); if (ret != 0) return ret; diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index ca46a45a9cce..aa14607e54d4 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -236,7 +236,7 @@ struct drm_driver { * to set/unset the VT into raw mode. * * Legacy drivers initialize the hardware in the @firstopen callback, -* which isn't even called for modern drivers. +* modern drivers can use it for other purposes only. */ void (*lastclose) (struct drm_device *); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/vc4: Don't liberate the binner BO at runtime suspend
The binner BO is a pre-requisite to GPU operations, so we must ensure that it is always allocated when the GPU is in use. Because the buffer is allocated from the same pool as other GPU buffers, we might run into a situation where we are out of memory at runtime resume. This causes the binner BO allocation to fail and results in all subsequent operations to fail, resulting in a major hang in userspace. Now that we allocate the buffer at firstopen and liberate it at lastclose, we can just keep it alive during runtime suspend. Signed-off-by: Paul Kocialkowski --- drivers/gpu/drm/vc4/vc4_v3d.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c index ab15e71df001..e04a51a75f01 100644 --- a/drivers/gpu/drm/vc4/vc4_v3d.c +++ b/drivers/gpu/drm/vc4/vc4_v3d.c @@ -303,9 +303,6 @@ static int vc4_v3d_runtime_suspend(struct device *dev) vc4_irq_uninstall(vc4->dev); - drm_gem_object_put_unlocked(>bin_bo->base.base); - vc4->bin_bo = NULL; - clk_disable_unprepare(v3d->clk); return 0; @@ -317,10 +314,6 @@ static int vc4_v3d_runtime_resume(struct device *dev) struct vc4_dev *vc4 = v3d->vc4; int ret; - ret = vc4_allocate_bin_bo(vc4->dev); - if (ret) - return ret; - ret = clk_prepare_enable(v3d->clk); if (ret != 0) return ret; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/vc4: Allocate/liberate the binner BO at firstopen/lastclose
Since the binner buffer is only required for GPU rendering, it's a waste to allocate it when the driver probes since internal users of the driver (such as fbcon) won't try to use the GPU. Move the allocation/liberation to the firstopen/lastclose to only allocate it when userspace has opened the device. Signed-off-by: Paul Kocialkowski --- drivers/gpu/drm/vc4/vc4_drv.c | 26 ++ drivers/gpu/drm/vc4/vc4_drv.h | 1 + drivers/gpu/drm/vc4/vc4_irq.c | 3 +++ drivers/gpu/drm/vc4/vc4_v3d.c | 8 +--- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index 3227706700f9..605dc50613e3 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -134,6 +134,30 @@ static void vc4_close(struct drm_device *dev, struct drm_file *file) kfree(vc4file); } +static int vc4_firstopen(struct drm_device *drm) +{ + struct vc4_dev *vc4 = to_vc4_dev(drm); + int ret; + + if (!vc4->bin_bo) { + ret = vc4_allocate_bin_bo(drm); + if (ret) + return ret; + } + + return 0; +} + +static void vc4_lastclose(struct drm_device *drm) +{ + struct vc4_dev *vc4 = to_vc4_dev(drm); + + if (vc4->bin_bo) { + drm_gem_object_put_unlocked(>bin_bo->base.base); + vc4->bin_bo = NULL; + } +} + static const struct vm_operations_struct vc4_vm_ops = { .fault = vc4_fault, .open = drm_gem_vm_open, @@ -180,6 +204,8 @@ static struct drm_driver vc4_drm_driver = { DRIVER_SYNCOBJ), .open = vc4_open, .postclose = vc4_close, + .firstopen = vc4_firstopen, + .lastclose = vc4_lastclose, .irq_handler = vc4_irq, .irq_preinstall = vc4_irq_preinstall, .irq_postinstall = vc4_irq_postinstall, diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 7a3c093e7443..f52bb21e9885 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -808,6 +808,7 @@ extern struct platform_driver vc4_v3d_driver; int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused); int vc4_v3d_debugfs_regs(struct seq_file *m, void *unused); int vc4_v3d_get_bin_slot(struct vc4_dev *vc4); +int vc4_allocate_bin_bo(struct drm_device *drm); /* vc4_validate.c */ int diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c index 4cd2ccfe15f4..efaba2b02f6c 100644 --- a/drivers/gpu/drm/vc4/vc4_irq.c +++ b/drivers/gpu/drm/vc4/vc4_irq.c @@ -64,6 +64,9 @@ vc4_overflow_mem_work(struct work_struct *work) struct vc4_exec_info *exec; unsigned long irqflags; + if (!bo) + return; + bin_bo_slot = vc4_v3d_get_bin_slot(vc4); if (bin_bo_slot < 0) { DRM_ERROR("Couldn't allocate binner overflow mem\n"); diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c index e47e29426078..ab15e71df001 100644 --- a/drivers/gpu/drm/vc4/vc4_v3d.c +++ b/drivers/gpu/drm/vc4/vc4_v3d.c @@ -218,7 +218,7 @@ int vc4_v3d_get_bin_slot(struct vc4_dev *vc4) * overall CMA pool before they make scenes complicated enough to run * out of bin space. */ -static int vc4_allocate_bin_bo(struct drm_device *drm) +int vc4_allocate_bin_bo(struct drm_device *drm) { struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_v3d *v3d = vc4->v3d; @@ -384,12 +384,6 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data) if (ret != 0) return ret; - ret = vc4_allocate_bin_bo(drm); - if (ret) { - clk_disable_unprepare(v3d->clk); - return ret; - } - /* Reset the binner overflow address/size at setup, to be sure * we don't reuse an old one. */ -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/lima: driver for ARM Mali4xx GPUs
Am 15.03.19 um 17:05 schrieb Eric Anholt: > [SNIP] > I think for lima not having this is fine, but for panfrost it really > should have it. > > If you can implement vulkan you probably want this, nouveau hasn't a > vulkan driver because of exactly this problem in their uapi, so maybe > adjust panfrost to do user-space managed vma. Wouldn't this just require an allocation flag to not map the BO up front and then new ioctl's like above to map and unmap at specified VAs? Seems like we could add that when we get there. >>> Sounds pretty reasonable to me. >> I can only advise to NOT do this. >> >> A address space manager in userspace is rather easily doable, but fixing >> up UAPI without breaking existing applications isn't. > Can you expand on what goes wrong with Rob's idea? The only thing I can > come up with is maybe dmabuf imports don't have a flag for > don't-automatically-map? Suppressing automatically mapping is not the problem. The problem is that you need exactly one instance to have the authority over the address space management. With Vulkan and features like PRT this must be done by userspace. So can't just go ahead in the kernel and allocate some address space and map your buffers. You can of course hope that all client respect this and you don't have combinations of multiple clients in the same process, but from experience I can say that this won't work at all. Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V2] drm/vkms: Remove useless call to drm_connector_register/unregister()
The function vkms_output_init() is invoked during the module initialization, and it handles the creation/configuration of the vkms essential elements (e.g., connectors, encoder, etc). Among the initializations, this function tries to initialize a connector and register it by calling drm_connector_register(). However, inside the drm_connector_register(), at the beginning of this function there is the following validation: if (!connector->dev->registered) return 0; In this sense, invoke drm_connector_register() after initializing the connector has no effect because the register field is false. The connector register happens when drm_dev_register() is invoked; the same issue exists with drm_connector_unregister(). Therefore, this commit removes the unnecessary call to drm_connector_register() and drm_connector_unregister(). Changes since v2: * Remove unnecessary call to drm_connector_unregister() * Remove unused label Signed-off-by: Rodrigo Siqueira Reviewed-by: Daniel Vetter --- drivers/gpu/drm/vkms/vkms_output.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 3b162b25312e..56fb5c2a2315 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -6,7 +6,6 @@ static void vkms_connector_destroy(struct drm_connector *connector) { - drm_connector_unregister(connector); drm_connector_cleanup(connector); } @@ -71,12 +70,6 @@ int vkms_output_init(struct vkms_device *vkmsdev) drm_connector_helper_add(connector, _conn_helper_funcs); - ret = drm_connector_register(connector); - if (ret) { - DRM_ERROR("Failed to register connector\n"); - goto err_connector_register; - } - ret = drm_encoder_init(dev, encoder, _encoder_funcs, DRM_MODE_ENCODER_VIRTUAL, NULL); if (ret) { @@ -99,9 +92,6 @@ int vkms_output_init(struct vkms_device *vkmsdev) drm_encoder_cleanup(encoder); err_encoder: - drm_connector_unregister(connector); - -err_connector_register: drm_connector_cleanup(connector); err_connector: -- 2.21.0 signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/lima: driver for ARM Mali4xx GPUs
"Koenig, Christian" writes: > Am 14.03.19 um 23:28 schrieb Eric Anholt: >> Rob Herring writes: >> >>> On Thu, Mar 14, 2019 at 3:45 PM Dave Airlie wrote: On Thu, 14 Feb 2019 at 19:12, Christian König via dri-devel wrote: > Am 14.02.19 um 03:52 schrieb Alex Deucher via dri-devel: >> [SNIP] >> +static int lima_ioctl_gem_va(struct drm_device *dev, void *data, >> struct drm_file *file) >> +{ >> + struct drm_lima_gem_va *args = data; >> + >> + switch (args->op) { >> + case LIMA_VA_OP_MAP: >> + return lima_gem_va_map(file, args->handle, >> args->flags, args->va); >> + case LIMA_VA_OP_UNMAP: >> + return lima_gem_va_unmap(file, args->handle, >> args->va); > These are mapping to GPU VA. Why not do that on GEM object creation or > import or when the objects are submitted with cmd queue as other > drivers do? > > To put it another way, These ioctls look different than what other > drivers do. Why do you need to do things differently? My understanding > is best practice is to map and return the GPU offset when the GEM > object is created. This is what v3d does. I think Intel is moving to > that. And panfrost will do that. I think it would be a good idea to look at the amdgpu driver. This driver is heavily modeled after it. Basically the GEM VA ioctl allows userspace to manage per process (per fd really) virtual addresses. >>> Why do you want userspace to manage assigning VAs versus the kernel to >>> do so? Exposing that detail to userspace means the driver must support >>> a per process address space. Letting the kernel assign addresses means >>> it can either be a single address space or be a per process address >>> space. It seems to me more flexible to allow the kernel driver to >>> evolve without that ABI. >> Having it in userspace provides a lot more flexibility and makes it >> easier to support things like unified address space between CPU and >> GPU. I guess it depends on the hw as to what is the right choice. > To summarize we actually have tried this approach with the radeon and it > turned out to be a really bad mistake. > > To implement features like partial residential textures and shared > virtual address space you absolutely need userspace to be in charge of > allocating virtual addresses. > I think for lima not having this is fine, but for panfrost it really should have it. If you can implement vulkan you probably want this, nouveau hasn't a vulkan driver because of exactly this problem in their uapi, so maybe adjust panfrost to do user-space managed vma. >>> Wouldn't this just require an allocation flag to not map the BO up >>> front and then new ioctl's like above to map and unmap at specified >>> VAs? Seems like we could add that when we get there. >> Sounds pretty reasonable to me. > > I can only advise to NOT do this. > > A address space manager in userspace is rather easily doable, but fixing > up UAPI without breaking existing applications isn't. Can you expand on what goes wrong with Rob's idea? The only thing I can come up with is maybe dmabuf imports don't have a flag for don't-automatically-map? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/selftest:- Adding a new format(whose cpp=0) to be tested by igt_check_drm_format_min_pitch
We have introduced some new formats DRM_FORMAT_VUY101010, DRM_FORMAT_YUV420_8BIT, and DRM_FORMAT_YUV420_10BIT whose cpp is 0 (as they are defined in bits per pixel). We need to ensure that the pitch returned by drm_format_info_min_pitch() for such formats is always 0. Signed-off-by: Ayan Kumar halder Depends on:- https://patchwork.freedesktop.org/patch/msgid/1552414556-5756-1-git-send-email-ayan.hal...@arm.com --- drivers/gpu/drm/selftests/test-drm_format.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/selftests/test-drm_format.c b/drivers/gpu/drm/selftests/test-drm_format.c index c5e212a..155e4ce 100644 --- a/drivers/gpu/drm/selftests/test-drm_format.c +++ b/drivers/gpu/drm/selftests/test-drm_format.c @@ -276,5 +276,14 @@ int igt_check_drm_format_min_pitch(void *ignored) FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX - 1) != (uint64_t)(UINT_MAX - 1) * 2); + /* Test format with cpp/char_per_block 0 */ + info = drm_format_info(DRM_FORMAT_VUY101010); + FAIL_ON(!info); + FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0); + FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0); + FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0); + FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 0); + FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) != 0); + return 0; } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: dw-hdmi: disable SCDC configuration for invalid setups
On Fri, Mar 15, 2019 at 4:54 AM Neil Armstrong wrote: > > This patch is an attempt to limit HDMI 2.0 SCDC setup when : > - the SoC embeds an HDMI 1.4 only controller > - the EDID supports SCDC but not scrambling > - the EDID supports SCDC scrambling but not for low TMDS bit rates, > while only supporting low TMDS bit rates > > This to avoid communicating with the SCDC DDC slave uncessary, and > setting the DW-HDMI TMDS Scrambler setup when not supported by the > underlying hardware. > > Reported-by: Rob Herring > Fixes: 264fce6cc2c1 ("drm/bridge: dw-hdmi: Add SCDC and TMDS Scrambling > support") > Signed-off-by: Neil Armstrong > --- > > Rob, > > this patch should also solve your issue with your 11' display, could you > test it ? That works for me. Tested-by: Rob Herring Too answer your question on the other thread, there's not really any model name for the panel I have. Probably what's in the EDID is the best thing to go on. Here's the panel on Amazon: https://www.amazon.com/gp/product/B07GDDG3WJ/ref=ppx_yo_dt_b_asin_title_o00_s01?ie=UTF8=1 > If this works, I will focus on the underlying issue where the RK3399 SoC > freezes in your setup. > > Thanks, > Neil > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 34 --- > 1 file changed, 30 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index a63e5f0dae56..db761329a1e3 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -1037,6 +1037,31 @@ void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, > unsigned short data, > } > EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_write); > > +/* Filter out invalid setups to avoid configuring SCDC and scrambling */ > +static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi) > +{ > + struct drm_display_info *display = >connector.display_info; > + > + /* Completely disable SCDC support for older controllers */ > + if (hdmi->version < 0x200a) > + return false; > + > + /* Disable if SCDC is not supported, or if an HF-VSDB block is absent > */ > + if (!display->hdmi.scdc.supported || > + !display->hdmi.scdc.scrambling.supported) > + return false; > + > + /* > +* Disable if display only support low TMDS rates and scrambling > +* for low rates is not supported either > +*/ > + if (!display->hdmi.scdc.scrambling.low_rates && > + display->max_tmds_clock <= 34) > + return false; > + > + return true; > +} > + > /* > * HDMI2.0 Specifies the following procedure for High TMDS Bit Rates: > * - The Source shall suspend transmission of the TMDS clock and data > @@ -1055,7 +1080,7 @@ void dw_hdmi_set_high_tmds_clock_ratio(struct dw_hdmi > *hdmi) > unsigned long mtmdsclock = hdmi->hdmi_data.video_mode.mtmdsclock; > > /* Control for TMDS Bit Period/TMDS Clock-Period Ratio */ > - if (hdmi->connector.display_info.hdmi.scdc.supported) { > + if (dw_hdmi_support_scdc(hdmi)) { > if (mtmdsclock > HDMI14_MAX_TMDSCLK) > drm_scdc_set_high_tmds_clock_ratio(hdmi->ddc, 1); > else > @@ -1579,8 +1604,9 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi, > > /* Set up HDMI_FC_INVIDCONF */ > inv_val = (hdmi->hdmi_data.hdcp_enable || > - vmode->mtmdsclock > HDMI14_MAX_TMDSCLK || > - hdmi_info->scdc.scrambling.low_rates ? > + (dw_hdmi_support_scdc(hdmi) && > + (vmode->mtmdsclock > HDMI14_MAX_TMDSCLK || > +hdmi_info->scdc.scrambling.low_rates)) ? > HDMI_FC_INVIDCONF_HDCP_KEEPOUT_ACTIVE : > HDMI_FC_INVIDCONF_HDCP_KEEPOUT_INACTIVE); > > @@ -1646,7 +1672,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi, > } > > /* Scrambling Control */ > - if (hdmi_info->scdc.supported) { > + if (dw_hdmi_support_scdc(hdmi)) { > if (vmode->mtmdsclock > HDMI14_MAX_TMDSCLK || > hdmi_info->scdc.scrambling.low_rates) { > /* > -- > 2.20.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/udl: add a release method and delay modeset teardown
Den 15.03.2019 06.13, skrev Dave Airlie: > From: Dave Airlie > > If we unplug a udl device, the usb callback with deinit the > mode_config struct, however userspace will still have an open > file descriptor and a framebuffer on that device. When userspace > closes the fd, we'll oops because it'll try and look stuff up > in the object idr which we've destroyed. > > This punts destroying the mode objects until release time instead. > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/udl/udl_drv.c | 1 + > drivers/gpu/drm/udl/udl_drv.h | 1 + > drivers/gpu/drm/udl/udl_main.c | 8 +++- > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c > index 22cd2d13e272..ff47f890e6ad 100644 > --- a/drivers/gpu/drm/udl/udl_drv.c > +++ b/drivers/gpu/drm/udl/udl_drv.c > @@ -52,6 +52,7 @@ static struct drm_driver driver = { > .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME, > .load = udl_driver_load, > .unload = udl_driver_unload, > + .release = udl_driver_release, > > /* gem hooks */ > .gem_free_object_unlocked = udl_gem_free_object, > diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h > index e9e9b1ff678e..4ae67d882eae 100644 > --- a/drivers/gpu/drm/udl/udl_drv.h > +++ b/drivers/gpu/drm/udl/udl_drv.h > @@ -104,6 +104,7 @@ void udl_urb_completion(struct urb *urb); > > int udl_driver_load(struct drm_device *dev, unsigned long flags); > void udl_driver_unload(struct drm_device *dev); > +void udl_driver_release(struct drm_device *dev); > > int udl_fbdev_init(struct drm_device *dev); > void udl_fbdev_cleanup(struct drm_device *dev); > diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c > index 9086d0d1b880..5626e1f11852 100644 > --- a/drivers/gpu/drm/udl/udl_main.c > +++ b/drivers/gpu/drm/udl/udl_main.c > @@ -379,6 +379,12 @@ void udl_driver_unload(struct drm_device *dev) > udl_free_urb_list(dev); > > udl_fbdev_cleanup(dev); > - udl_modeset_cleanup(dev); > kfree(udl); > } > + > +void udl_driver_release(struct drm_device *dev) > +{ > + drm_mode_config_cleanup(dev); > + drm_dev_fini(dev); > + kfree(dev); > +} > You can remove udl_modeset_cleanup() now. There's still a problem here and that is if udl_driver_load() errors out before drm_mode_config_init() is called, drm_mode_config_cleanup() will crash on uninitialized lists etc. I have solved this by calling drm_mode_config_init() right after drm_device has been initialized. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: do lower level device put on unplugged releases
Den 15.03.2019 06.13, skrev Dave Airlie: > From: Dave Airlie > > When we release the file handle on a device that has been unplugged > it has already called the unregister path, which doesn't like being > called again. We should just do the dev put version instead. > > This fixes some crashes unplugged in a udl device. > > Signed-off-by: Dave Airlie > --- I believe this is fixed already in drm-misc-next: drm: Fix drm_release() and device unplug https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1ee57d4d75fbc74bb2ae601c8f334219165ef276 Noralf. > drivers/gpu/drm/drm_file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index 83a5bbca6e7e..900fe8228745 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -492,7 +492,7 @@ int drm_release(struct inode *inode, struct file *filp) > if (!--dev->open_count) { > drm_lastclose(dev); > if (drm_dev_is_unplugged(dev)) > - drm_put_dev(dev); > + drm_dev_put(dev); > } > mutex_unlock(_global_mutex); > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: udl hotunplug broken
Den 15.03.2019 05.19, skrev Dave Airlie: > Hey, > > Not sure how long this has been broken, considering plugging it in was > broken, unplugging is much worse. Is there anything outside my tree > that might be fixing this? > > Currently it appears if I unplug udl while userspace has the device > open, it's bad, I get the userspace fb leaked thing which isn't > surprising, but I do get a lot of backtracing and warns which differ > depending on which way the race happens between X and unload. > > I'm going to see if I can find a fix, but I'm guessing it's a lot of > digging to figure this out. > I've made changes to the udl unload path: drm: Fix drm_release() and device unplug https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1ee57d4d75fbc74bb2ae601c8f334219165ef276 drm/drv: drm_dev_unplug(): Move out drm_dev_put() call https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ba3bf37e150a99b51b13f5cebf89715685d21212 I don't know if these could be the cause of your problem. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6] drm/panel: simple: Add TFC S9700RTWV43TR-01B 800x480 panel support
Add support for Three Five displays TFC S9700RTWV43TR-01B 800x480 panel with resistive touch found on TI's AM335X-EVM. Signed-off-by: Jyri Sarha Reviewed-by: Tomi Valkeinen --- Changes since v5: - Add all simple-panel properties that make sense with this panel to the binding document Previous round some time ago: https://patchwork.kernel.org/patch/10366741/ .../display/panel/tfc,s9700rtwv43tr-01b.txt | 17 +++ .../devicetree/bindings/vendor-prefixes.txt | 1 + drivers/gpu/drm/panel/panel-simple.c | 28 +++ 3 files changed, 46 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.txt diff --git a/Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.txt b/Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.txt new file mode 100644 index ..fbd45b01e5c5 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.txt @@ -0,0 +1,17 @@ +TFC S9700RTWV43TR-01B 7" Three Five Corp 800x480 LCD panel with +resistive touch + +The panel is found on TI AM335x-evm. + +Required properties: +- compatible: should be "tfc,S9700RTWV43tr-01b" + +Optional properties: +- power-supply: If there is one that can be controlled by the driver. + If the property is not present a dummy regulator is provided by the + framework. +- enable-gpios: GPIO pin to enable or disable the panel, if there is one +- backlight: phandle of the backlight device attached to the panel + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 389508584f48..0c0ffe2cc823 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -391,6 +391,7 @@ technexion TechNexion technologicTechnologic Systems tempo Tempo Semiconductor terasicTerasic Inc. +tfcThree Five Corp thine THine Electronics, Inc. ti Texas Instruments tianma Tianma Micro-electronics Co., Ltd. diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 9c69e739a524..cfa57ee78482 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -2281,6 +2281,31 @@ static const struct panel_desc starry_kr122ea0sra = { }, }; +static const struct drm_display_mode tfc_s9700rtwv43tr_01b_mode = { + .clock = 3, + .hdisplay = 800, + .hsync_start = 800 + 39, + .hsync_end = 800 + 39 + 47, + .htotal = 800 + 39 + 47 + 39, + .vdisplay = 480, + .vsync_start = 480 + 13, + .vsync_end = 480 + 13 + 2, + .vtotal = 480 + 13 + 2 + 29, + .vrefresh = 62, +}; + +static const struct panel_desc tfc_s9700rtwv43tr_01b = { + .modes = _s9700rtwv43tr_01b_mode, + .num_modes = 1, + .bpc = 8, + .size = { + .width = 155, + .height = 90, + }, + .bus_format = MEDIA_BUS_FMT_RGB888_1X24, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE, +}; + static const struct display_timing tianma_tm070jdhg30_timing = { .pixelclock = { 6260, 6820, 7810 }, .hactive = { 1280, 1280, 1280 }, @@ -2718,6 +2743,9 @@ static const struct of_device_id platform_of_match[] = { }, { .compatible = "starry,kr122ea0sra", .data = _kr122ea0sra, + }, { + .compatible = "tfc,s9700rtwv43tr-01b", + .data = _s9700rtwv43tr_01b, }, { .compatible = "tianma,tm070jdhg30", .data = _tm070jdhg30, -- 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 v3 41/50] drm/bridge: ti-tfp410: Report input bus config through bridge timings
On 15/03/2019 15.30, Tomi Valkeinen wrote: > On 15/03/2019 14:28, Peter Ujfalusi wrote: >> On 15/03/2019 14.07, Tomi Valkeinen wrote: If the pclk-sample is not defined in DT, it will default to 0 which selects SAMPLE_NEGEDGE (== DRIVE_POSEDGE), right? But all the boards where I can find schematics with tfp410 have their EDGE/HTPLG pin pulled up and according to the documentation when EDGE=1 then tfp410 will sample on the rising edge. imho the pclk_sample should be initialized to 1 to avoid regression for most of the boards using tfp410. >>> >>> Define "regression" =). If the omapdrm driver was always using >>> DRIVE_POSEDGE, this driver should also be using DRIVE_POSEDGE, no? If it >>> does the same as the old driver, it can't be a regression. This is, of >>> course, only considering omapdrm based boards. >>> >>> That said, it sounds odd that this would be wrong in the old driver, but >>> then again, it might well be, as code related to these sync signals has >>> changed sooo many times, and the related DSS registers are somewhat >>> confusing. >> >> On the boards data skew is enabled as well with maximum delay selected >> with DK1=DK2=DK3=1, so sampling of data is delayed by 3 * 350 ps, so >> about 1 ns, not much, but might be enough for the signal to transition >> on the bus so even if the HW drivers on POSEDGE and tfp410 samples on >> POSEDGE (+1ns delay from the edge) we don't see corruption? > > Ok. I don't think anyone has looked at it that closely. So, the syncs > could well be wrong. Still, not a regression if it's the same way as it was. True. > If the fixed default works fine (or better) than the wrong ones, I think > that can be the default. But I'd be careful about changing what the > default is, if we've had it the old way for a long time. The HW default in I2C mode is the EDGE=1 so SAMPLE_POSEDGE and it looks to me that the same default was implemented via hw control on the boards I can access the schematics. If it was the other way in the past, then it is safer to not change the default in the driver, but probably put a comment somewhere that the HW default is the opposite? In any case if the i2c control is implemented we will read and/or configure the tfp410 anyways. - Péter 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 v2 0/2] arm64: meson-gxm: Add support for the Mali T820 GPU
This patchset adds : - Optional reset properties in the midgard bindings - Mali T820 Node in Amlogic Meson GXM DTSI Changes since v1: - Updated midgard DT wording following the recently submitted bifrost bindings Christian Hewitt (1): arm64: dts: meson-gxm: Add Mali-T820 node Neil Armstrong (1): dt-bindings: gpu: mali-midgard: Add resets property .../bindings/gpu/arm,mali-midgard.txt | 14 ++ arch/arm64/boot/dts/amlogic/meson-gxm.dtsi| 27 +++ 2 files changed, 41 insertions(+) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] arm64: dts: meson-gxm: Add Mali-T820 node
From: Christian Hewitt The Amlogic Meson GXM SoC embeds an ARM Mali T820 GPU. This patch adds the node with all the needed properties to power on the GPU. This has been tested with the work-in-progress PanFrost project aiming support for ARM Mali Midgard and later GPUs. Signed-off-by: Christian Hewitt Signed-off-by: Neil Armstrong --- arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 27 ++ 1 file changed, 27 insertions(+) diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi index 247888d68a3a..35e59d390903 100644 --- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi @@ -91,6 +91,33 @@ reset-names = "phy"; status = "okay"; }; + + mali: gpu@c { + compatible = "amlogic,meson-gxm-mali", "arm,mali-t820"; + reg = <0x0 0xc 0x0 0x4>; + interrupt-parent = <>; + interrupts = , +, +; + interrupt-names = "gpu", "mmu", "job"; + clocks = < CLKID_MALI>; + resets = < RESET_MALI_CAPB3>, < RESET_MALI>; + + /* +* Mali clocking is provided by two identical clock paths +* MALI_0 and MALI_1 muxed to a single clock by a glitch +* free mux to safely change frequency while running. +*/ + assigned-clocks = < CLKID_MALI_0_SEL>, + < CLKID_MALI_0>, + < CLKID_MALI>; /* Glitch free mux */ + assigned-clock-parents = < CLKID_FCLK_DIV3>, +<0>, /* Do Nothing */ +< CLKID_MALI_0>; + assigned-clock-rates = <0>, /* Do Nothing */ + <6>, + <0>; /* Do Nothing */ + }; }; _AO { -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] dt-bindings: gpu: mali-midgard: Add resets property
The Amlogic ARM Mali Midgard requires reset controls to power on and software reset the GPU, adds these as optional in the bindings. Signed-off-by: Neil Armstrong --- .../devicetree/bindings/gpu/arm,mali-midgard.txt | 14 ++ 1 file changed, 14 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt index 18a2cde2e5f3..1b1a74129141 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt @@ -37,6 +37,20 @@ Optional properties: - operating-points-v2 : Refer to Documentation/devicetree/bindings/opp/opp.txt for details. +- resets : Phandle of the GPU reset line. + +Vendor-specific bindings + + +The Mali GPU is integrated very differently from one SoC to +another. In order to accomodate those differences, you have the option +to specify one more vendor-specific compatible, among: + +- "amlogic,meson-gxm-mali" + Required properties: + - resets : Should contain phandles of : ++ GPU reset line ++ GPU APB glue reset line Example for a Mali-T760: -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH 3/6] drm/sun4i: dsi: Add bridge support
Hi, On Fri, 2019-03-15 at 14:45 +0100, Maxime Ripard wrote: > On Fri, Mar 15, 2019 at 02:32:55PM +0100, Paul Kocialkowski wrote: > > Hi, > > > > On Fri, 2019-03-15 at 18:38 +0530, Jagan Teki wrote: > > > Some display panels would come up with a non-DSI output which > > > can have an option to connect DSI interface by means of bridge > > > convertor. > > > > > > This DSI to non-DSI bridge convertor would require a bridge > > > driver that would communicate the DSI controller for bridge > > > functionalities. > > > > > > So, add support for bridge functionalities in Allwinner DSI > > > controller. > > > > See a few comments below. > > > > > Signed-off-by: Jagan Teki > > > --- > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 65 +++--- > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 1 + > > > 2 files changed, 49 insertions(+), 17 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > index 0960b96b62cc..64d74313b842 100644 > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > @@ -781,6 +781,9 @@ static void sun6i_dsi_encoder_enable(struct > > > drm_encoder *encoder) > > > if (!IS_ERR(dsi->panel)) > > > drm_panel_prepare(dsi->panel); > > > > > > + if (!IS_ERR(dsi->bridge)) > > > + drm_bridge_pre_enable(dsi->bridge); > > > + > > > /* > > >* FIXME: This should be moved after the switch to HS mode. > > >* > > > @@ -796,6 +799,9 @@ static void sun6i_dsi_encoder_enable(struct > > > drm_encoder *encoder) > > > if (!IS_ERR(dsi->panel)) > > > drm_panel_enable(dsi->panel); > > > > > > + if (!IS_ERR(dsi->bridge)) > > > + drm_bridge_enable(dsi->bridge); > > > + > > > sun6i_dsi_start(dsi, DSI_START_HSC); > > > > > > udelay(1000); > > > @@ -812,6 +818,9 @@ static void sun6i_dsi_encoder_disable(struct > > > drm_encoder *encoder) > > > if (!IS_ERR(dsi->panel)) { > > > drm_panel_disable(dsi->panel); > > > drm_panel_unprepare(dsi->panel); > > > + } else if (!IS_ERR(dsi->bridge)) { > > > + drm_bridge_disable(dsi->bridge); > > > + drm_bridge_post_disable(dsi->bridge); > > > } > > > > > > phy_power_off(dsi->dphy); > > > @@ -973,11 +982,16 @@ static int sun6i_dsi_attach(struct mipi_dsi_host > > > *host, > > > struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); > > > > > > dsi->device = device; > > > - dsi->panel = of_drm_find_panel(device->dev.of_node); > > > - if (IS_ERR(dsi->panel)) > > > - return PTR_ERR(dsi->panel); > > > > > > - dev_info(host->dev, "Attached device %s\n", device->name); > > > + dsi->bridge = of_drm_find_bridge(device->dev.of_node); > > > + if (!dsi->bridge) { > > > > You are using IS_ERR to check that the bridge is alive in the changes > > above, but switch to checking that it's non-NULL at this point. > > > > Are both guaranteed to be interchangeable? > > They aren't. Any ERR_PTR will be !NULL > > > > + dsi->panel = of_drm_find_panel(device->dev.of_node); > > > + if (IS_ERR(dsi->panel)) > > > + return PTR_ERR(dsi->panel); > > > + } > > > > You should probably use drm_of_find_panel_or_bridge instead of > > duplicating the logic here. > > Or we can even use the drm_panel_bridge_add to simplify things. Indeed, I was just looking at that and it seems to be specifically tailored for this use case. Cheers, Paul -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vkms: Solve bug on kms_crc_cursor tests
On Fri, Mar 15, 2019 at 08:51:57AM -0300, Rodrigo Siqueira wrote: > On 03/11, Ville Syrjälä wrote: > > On Sun, Mar 10, 2019 at 05:35:05PM -0300, Rodrigo Siqueira wrote: > > > On 03/01, Ville Syrjälä wrote: > > > > On Fri, Mar 01, 2019 at 03:35:35PM -0300, Shayenne Moura wrote: > > > > > Em sex, 1 de mar de 2019 às 12:26, Ville Syrjälä > > > > > escreveu: > > > > > > > > > > > > On Fri, Mar 01, 2019 at 11:55:11AM -0300, Shayenne Moura wrote: > > > > > > > Em qui, 28 de fev de 2019 às 11:03, Ville Syrjälä > > > > > > > escreveu: > > > > > > > > > > > > > > > > On Thu, Feb 28, 2019 at 11:11:07AM +0100, Daniel Vetter wrote: > > > > > > > > > On Mon, Feb 25, 2019 at 11:26:06AM -0300, Shayenne Moura > > > > > > > > > wrote: > > > > > > > > > > vkms_crc_work_handle needs the value of the actual frame to > > > > > > > > > > schedule the workqueue that calls periodically the vblank > > > > > > > > > > handler and the destroy state functions. However, the frame > > > > > > > > > > value returned from vkms_vblank_simulate is updated and > > > > > > > > > > diminished in vblank_get_timestamp because it is not in a > > > > > > > > > > vblank interrupt, and return an inaccurate value. > > > > > > > > > > > > > > > > > > > > Solve this getting the actual vblank frame directly from the > > > > > > > > > > vblank->count inside the `struct drm_crtc`, instead of using > > > > > > > > > > the `drm_accurate_vblank_count` function. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Shayenne Moura > > > > > > > > > > > > > > > > > > Sorry for the delay, I'm a bit swamped right now :-/ > > > > > > > > > > > > > > > > > > Debug work you're doing here is really impressive! But I have > > > > > > > > > no idea > > > > > > > > > what's going on. It doesn't look like it's just papering over > > > > > > > > > a bug (like > > > > > > > > > the in_vblank_irq check we've discussed on irc), but I also > > > > > > > > > have no idea > > > > > > > > > why it works. > > > > > > > > > > > > > > > > > > I'll pull in Ville, he understands this better than me. > > > > > > > > > > > > > > > > It's not entirely clear what we're trying to fix. From what I > > > > > > > > can see > > > > > > > > the crc work seems to be in no way synchronized with page > > > > > > > > flips, so > > > > > > > > I'm not sure how all this is really supposed to work. > > > > > > > > > > > > > > > > > > > > > > Hi, Ville! > > > > > > > > > > > > > > Thank you for the review! :) > > > > > > > > > > > > > > I do not understand well what crc code is doing, but the issue > > > > > > > that I found > > > > > > > is related to the vblank timestamp and frame count. > > > > > > > > > > > > > > When vkms handles the crc_cursor it uses the start frame and end > > > > > > > frame > > > > > > > values to verify if it needs to call the function > > > > > > > 'drm_crtc_add_crc_entry()' > > > > > > > for each frame. > > > > > > > > > > > > > > However, when getting the frame count, the code is calling the > > > > > > > function > > > > > > > drm_update_vblank_count(dev, pipe, false) and, because of the > > > > > > > 'false', > > > > > > > subtracting the actual vblank timestamp (consequently, the frame > > > > > > > count > > > > > > > value), causing conflicts. > > > > > > > > > > > > The in_vblank_irq behavour looks sane to me. What are these > > > > > > conflicts? > > > > > > > > > > > > > > > > The entire history was: > > > > > - I sent the patch with bugfix for vblank extra frame. The patch > > > > > changed > > > > >our function vkms_get_vblank_timestamp() to look like this: > > > > > > > > > > bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int > > > > > pipe, > > > > >int *max_error, ktime_t *vblank_time, > > > > >bool in_vblank_irq) > > > > > { > > > > > struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev); > > > > > struct vkms_output *output = >output; > > > > > > > > > > *vblank_time = output->vblank_hrtimer.node.expires; > > > > > > > > > > + if (!in_vblank_irq) > > > > > +*vblank_time -= output->period_ns; > > > > > > > > > > return true; > > > > > } > > > > > > > > > > - This patch solve the issue that I was looking for (extra vblank > > > > > frames on kms_flip). > > > > > > > > > > - However, kms_cursor_crc tests, which passed before my patch, > > > > > started to fail. > > > > > > > > > > - Debugging them, I found that the problem was inside of function > > > > > `vkms_vblank_simulate()` > > > > > when it was handling the crc_enabled (inside if (state && > > > > > output->crc_enabled)) > > > > > and inside the function vkms_crc_work_handle() too. > > > > > > > > > > - Following the steps: > > > > > 1. Inside vkms_vblank_simulate() we call > > > > > drm_crtc_accurate_vblank_count() > > > > > 2. In its turn, drm_crtc_accurate_vblank_count() calls the function > > > > >drm_update_vblank_count(dev, pipe, false). /* This false is > > >
Re: [linux-sunxi] [PATCH 3/6] drm/sun4i: dsi: Add bridge support
On Fri, Mar 15, 2019 at 02:32:55PM +0100, Paul Kocialkowski wrote: > Hi, > > On Fri, 2019-03-15 at 18:38 +0530, Jagan Teki wrote: > > Some display panels would come up with a non-DSI output which > > can have an option to connect DSI interface by means of bridge > > convertor. > > > > This DSI to non-DSI bridge convertor would require a bridge > > driver that would communicate the DSI controller for bridge > > functionalities. > > > > So, add support for bridge functionalities in Allwinner DSI > > controller. > > See a few comments below. > > > Signed-off-by: Jagan Teki > > --- > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 65 +++--- > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 1 + > > 2 files changed, 49 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > index 0960b96b62cc..64d74313b842 100644 > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > @@ -781,6 +781,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder > > *encoder) > > if (!IS_ERR(dsi->panel)) > > drm_panel_prepare(dsi->panel); > > > > + if (!IS_ERR(dsi->bridge)) > > + drm_bridge_pre_enable(dsi->bridge); > > + > > /* > > * FIXME: This should be moved after the switch to HS mode. > > * > > @@ -796,6 +799,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder > > *encoder) > > if (!IS_ERR(dsi->panel)) > > drm_panel_enable(dsi->panel); > > > > + if (!IS_ERR(dsi->bridge)) > > + drm_bridge_enable(dsi->bridge); > > + > > sun6i_dsi_start(dsi, DSI_START_HSC); > > > > udelay(1000); > > @@ -812,6 +818,9 @@ static void sun6i_dsi_encoder_disable(struct > > drm_encoder *encoder) > > if (!IS_ERR(dsi->panel)) { > > drm_panel_disable(dsi->panel); > > drm_panel_unprepare(dsi->panel); > > + } else if (!IS_ERR(dsi->bridge)) { > > + drm_bridge_disable(dsi->bridge); > > + drm_bridge_post_disable(dsi->bridge); > > } > > > > phy_power_off(dsi->dphy); > > @@ -973,11 +982,16 @@ static int sun6i_dsi_attach(struct mipi_dsi_host > > *host, > > struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); > > > > dsi->device = device; > > - dsi->panel = of_drm_find_panel(device->dev.of_node); > > - if (IS_ERR(dsi->panel)) > > - return PTR_ERR(dsi->panel); > > > > - dev_info(host->dev, "Attached device %s\n", device->name); > > + dsi->bridge = of_drm_find_bridge(device->dev.of_node); > > + if (!dsi->bridge) { > > You are using IS_ERR to check that the bridge is alive in the changes > above, but switch to checking that it's non-NULL at this point. > > Are both guaranteed to be interchangeable? They aren't. Any ERR_PTR will be !NULL > > + dsi->panel = of_drm_find_panel(device->dev.of_node); > > + if (IS_ERR(dsi->panel)) > > + return PTR_ERR(dsi->panel); > > + } > > You should probably use drm_of_find_panel_or_bridge instead of > duplicating the logic here. Or we can even use the drm_panel_bridge_add to simplify things. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201795] [Regression] Wrong 4k resolution detected with DisplayPort to HDMI adapter on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=201795 --- Comment #21 from thomas.lassdiesonner...@gmx.de --- Ok thanks again. You can close this as "won't fix" then. My last and final question. Why does the old code (kernel 4.14) work? -- 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 1/1] drm/atomic: integrate private objects with suspend/resume helpers
Daniel Vetter wrote on Fri [2019-Mar-15 11:50:57 +0100]: > On Thu, Mar 14, 2019 at 08:44:45AM -0500, Benoit Parrot wrote: > > During a suspend cycle the atomic state is saved to be used during the > > restore cycle. > > > > However the current state duplication logic does not duplicate private > > objects. This leads to state inconsistencies at resume time. > > > > With private objects modeset lock now integrated, we can make sure that > > private object state are properly saved and restored. > > > > Signed-off-by: Benoit Parrot > > Why do you need this? We're doing a full atomic_check, and your > atomic_check should be pulling in any private state objects and > recomputing their states. This smells like papering over a driver bug. I have not seen any atomic_check called during the suspend duplicate helper. And you are correct normally the private_object state in my case would get pulled in during a plane atomic_check but that does not happen here in this step. Why wouldn't we save all the "state" artifacts so that the whole atomic state is consistent? Benoit > > The duplicate helpers should only need to duplicate the uapi-relevant > states, i.e. connector/crtc/planes. > -Daniel > > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 16 > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index 540a77a2ade9..b108021cc092 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -3189,6 +3189,7 @@ drm_atomic_helper_duplicate_state(struct drm_device > > *dev, > > struct drm_connector_list_iter conn_iter; > > struct drm_plane *plane; > > struct drm_crtc *crtc; > > + struct drm_private_obj *privobj; > > int err = 0; > > > > state = drm_atomic_state_alloc(dev); > > @@ -3218,6 +3219,16 @@ drm_atomic_helper_duplicate_state(struct drm_device > > *dev, > > } > > } > > > > + drm_for_each_privobj(privobj, dev) { > > + struct drm_private_state *priv_state; > > + > > + priv_state = drm_atomic_get_private_obj_state(state, privobj); > > + if (IS_ERR(priv_state)) { > > + err = PTR_ERR(priv_state); > > + goto free; > > + } > > + } > > + > > drm_connector_list_iter_begin(dev, _iter); > > drm_for_each_connector_iter(conn, _iter) { > > struct drm_connector_state *conn_state; > > @@ -3325,12 +3336,17 @@ int > > drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state, > > struct drm_connector_state *new_conn_state; > > struct drm_crtc *crtc; > > struct drm_crtc_state *new_crtc_state; > > + struct drm_private_obj *privobj; > > + struct drm_private_state *new_priv_state; > > > > state->acquire_ctx = ctx; > > > > for_each_new_plane_in_state(state, plane, new_plane_state, i) > > state->planes[i].old_state = plane->state; > > > > + for_each_new_private_obj_in_state(state, privobj, new_priv_state, i) > > + state->private_objs[i].old_state = privobj->state; > > + > > for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) > > state->crtcs[i].old_state = crtc->state; > > > > -- > > 2.17.1 > > > > -- > 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 4/6] dt-bindings: display: bridge: Add ICN6211 MIPI-DSI to RGB convertor bridge
On Fri, Mar 15, 2019 at 06:38:23PM +0530, Jagan Teki wrote: > ICN6211 is MIPI-DSI/RGB converter bridge from chipone. > It has a flexible configuration of MIPI DSI signal input > and produce RGB565, RGB666, RGB888 output format. > > Add dt-bingings for it. > > Signed-off-by: Jagan Teki > --- > .../display/bridge/chipone,icn6211.txt| 36 +++ > 1 file changed, 36 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt > > diff --git > a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt > b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt > new file mode 100644 > index ..7f13efd7ee7f > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt > @@ -0,0 +1,36 @@ > +Chipone ICN6211 MIPI-DSI to RGB Convertor Bridge > + > +ICN6211 is MIPI-DSI/RGB converter bridge from chipone. > +It has a flexible configuration of MIPI DSI signal input > +and produce RGB565, RGB666, RGB888 output format. > + > +Required properties for RGB: > +- compatible: must be "chipone,icn6211" and one of: > + * "bananapi,icn6211" Why is that compatible needed? > +- reg: the virtual channel number of a DSI peripheral > +- reset-gpios: a GPIO phandle for the reset pin > + > +The device node can contain following 'port' child nodes, > +according to the OF graph bindings defined in [1]: > + 0: DSI Input, not required, if the bridge is DSI controlled > + 1: RGB Output, mandatory Your example doesn't have that input port Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH 3/6] drm/sun4i: dsi: Add bridge support
Hi, On Fri, 2019-03-15 at 18:38 +0530, Jagan Teki wrote: > Some display panels would come up with a non-DSI output which > can have an option to connect DSI interface by means of bridge > convertor. > > This DSI to non-DSI bridge convertor would require a bridge > driver that would communicate the DSI controller for bridge > functionalities. > > So, add support for bridge functionalities in Allwinner DSI > controller. See a few comments below. > Signed-off-by: Jagan Teki > --- > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 65 +++--- > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 1 + > 2 files changed, 49 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > index 0960b96b62cc..64d74313b842 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > @@ -781,6 +781,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder > *encoder) > if (!IS_ERR(dsi->panel)) > drm_panel_prepare(dsi->panel); > > + if (!IS_ERR(dsi->bridge)) > + drm_bridge_pre_enable(dsi->bridge); > + > /* >* FIXME: This should be moved after the switch to HS mode. >* > @@ -796,6 +799,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder > *encoder) > if (!IS_ERR(dsi->panel)) > drm_panel_enable(dsi->panel); > > + if (!IS_ERR(dsi->bridge)) > + drm_bridge_enable(dsi->bridge); > + > sun6i_dsi_start(dsi, DSI_START_HSC); > > udelay(1000); > @@ -812,6 +818,9 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder > *encoder) > if (!IS_ERR(dsi->panel)) { > drm_panel_disable(dsi->panel); > drm_panel_unprepare(dsi->panel); > + } else if (!IS_ERR(dsi->bridge)) { > + drm_bridge_disable(dsi->bridge); > + drm_bridge_post_disable(dsi->bridge); > } > > phy_power_off(dsi->dphy); > @@ -973,11 +982,16 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host, > struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); > > dsi->device = device; > - dsi->panel = of_drm_find_panel(device->dev.of_node); > - if (IS_ERR(dsi->panel)) > - return PTR_ERR(dsi->panel); > > - dev_info(host->dev, "Attached device %s\n", device->name); > + dsi->bridge = of_drm_find_bridge(device->dev.of_node); > + if (!dsi->bridge) { You are using IS_ERR to check that the bridge is alive in the changes above, but switch to checking that it's non-NULL at this point. Are both guaranteed to be interchangeable? > + dsi->panel = of_drm_find_panel(device->dev.of_node); > + if (IS_ERR(dsi->panel)) > + return PTR_ERR(dsi->panel); > + } You should probably use drm_of_find_panel_or_bridge instead of duplicating the logic here. > + dev_info(host->dev, "Attached %s %s\n", > + dsi->bridge ? "bridge" : "panel", device->name); > > return 0; > } > @@ -1055,8 +1069,10 @@ static int sun6i_dsi_bind(struct device *dev, struct > device *master, > struct sun4i_tcon *tcon0 = sun4i_get_tcon0(drm); > int ret; > > - if (!dsi->panel) > + if (!(dsi->panel || dsi->bridge)) { > + dev_info(drm->dev, "No panel or bridge found... DSI output > disabled\n"); > return -EPROBE_DEFER; > + } > > dsi->drv = drv; > > @@ -1078,19 +1094,29 @@ static int sun6i_dsi_bind(struct device *dev, struct > device *master, > } > dsi->encoder.possible_crtcs = BIT(0); > > - drm_connector_helper_add(>connector, > - _dsi_connector_helper_funcs); > - ret = drm_connector_init(drm, >connector, > - _dsi_connector_funcs, > - DRM_MODE_CONNECTOR_DSI); > - if (ret) { > - dev_err(dsi->dev, > - "Couldn't initialise the DSI connector\n"); > - goto err_cleanup_connector; > + if (dsi->panel) { > + drm_connector_helper_add(>connector, > + _dsi_connector_helper_funcs); > + ret = drm_connector_init(drm, >connector, > + _dsi_connector_funcs, > + DRM_MODE_CONNECTOR_DSI); > + if (ret) { > + dev_err(dsi->dev, > + "Couldn't initialise the DSI connector\n"); > + goto err_cleanup_connector; > + } > + > + drm_connector_attach_encoder(>connector, >encoder); > + drm_panel_attach(dsi->panel, >connector); > } > > - drm_connector_attach_encoder(>connector, >encoder); > - drm_panel_attach(dsi->panel, >connector); > + if (dsi->bridge) { > + ret = drm_bridge_attach(>encoder, dsi->bridge, NULL); > + if (ret)
Re: [PATCH 5/6] drm/bridge: Add Chipone ICN6211 MIPI-DSI/RGB convertor bridge
On Fri, Mar 15, 2019 at 06:38:24PM +0530, Jagan Teki wrote: > ICN6211 is MIPI-DSI/RGB converter bridge from chipone. > It has a flexible configuration of MIPI DSI signal input > and produce RGB565, RGB666, RGB888 output format. > > Add bridge driver for it. > > Signed-off-by: Jagan Teki > --- > MAINTAINERS | 6 + > drivers/gpu/drm/bridge/Kconfig | 10 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/chipone-icn6211.c | 275 +++ > 4 files changed, 292 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/chipone-icn6211.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4de80222cffb..e529addd30f5 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4897,6 +4897,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > S: Maintained > F: drivers/gpu/drm/bochs/ > > +DRM DRIVER FOR CHIPONE ICN6211 MIPI-DSI to RGB CONVERTOR BRIDGE > +M: Jagan Teki > +S: Maintained > +F: drivers/gpu/drm/bridge/chipone-icn6211.c > +F: Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt > + > DRM DRIVER FOR FARADAY TVE200 TV ENCODER > M: Linus Walleij > T: git git://anongit.freedesktop.org/drm/drm-misc > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 8840f396a7b6..cd314572e4ed 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -36,6 +36,16 @@ config DRM_CDNS_DSI > Support Cadence DPI to DSI bridge. This is an internal > bridge and is meant to be directly embedded in a SoC. > > +config DRM_CHIPONE_ICN6211 > + tristate "Chipone ICN6211 MIPI-DSI/RGB converter bridge" > + depends on DRM && DRM_PANEL > + depends on OF > + select DRM_MIPI_DSI > + help > + ICN6211 is MIPI-DSI/RGB converter bridge from chipone. > + It has a flexible configuration of MIPI DSI signal input > + and produce RGB565, RGB666, RGB888 output format. > + > config DRM_DUMB_VGA_DAC > tristate "Dumb VGA DAC Bridge support" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 4934fcf5a6f8..541fdccad10b 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o > obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o > +obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o > obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o > obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o > obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += > megachips-stdp-ge-b850v3-fw.o > diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c > b/drivers/gpu/drm/bridge/chipone-icn6211.c > new file mode 100644 > index ..cd2f3505f845 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c > @@ -0,0 +1,275 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2018 Amarula Solutions > + * Author: Jagan Teki > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#define ICN6211_INIT_CMD_LEN 2 > + > +struct chipone { > + struct device *dev; > + struct drm_bridge bridge; > + struct drm_connector connector; > + struct drm_panel *panel; > + > + struct gpio_desc *reset; > +}; > + > +static inline struct chipone *bridge_to_chipone(struct drm_bridge *bridge) > +{ > + return container_of(bridge, struct chipone, bridge); > +} > + > +static inline > +struct chipone *connector_to_chipone(struct drm_connector *connector) > +{ > + return container_of(connector, struct chipone, connector); > +} > + > +struct icn6211_init_cmd { > + u8 data[ICN6211_INIT_CMD_LEN]; > +}; > + > +static const struct icn6211_init_cmd icn6211_init_cmds[] = { > + { .data = { 0x7A, 0xC1 } }, > + { .data = { 0x20, 0x20 } }, > + { .data = { 0x21, 0xE0 } }, > + { .data = { 0x22, 0x13 } }, > + { .data = { 0x23, 0x28 } }, > + { .data = { 0x24, 0x30 } }, > + { .data = { 0x25, 0x28 } }, > + { .data = { 0x26, 0x00 } }, > + { .data = { 0x27, 0x0D } }, > + { .data = { 0x28, 0x03 } }, > + { .data = { 0x29, 0x1D } }, > + { .data = { 0x34, 0x80 } }, > + { .data = { 0x36, 0x28 } }, > + { .data = { 0xB5, 0xA0 } }, > + { .data = { 0x5C, 0xFF } }, > + { .data = { 0x2A, 0x01 } }, > + { .data = { 0x56, 0x92 } }, > + { .data = { 0x6B, 0x71 } }, > + { .data = { 0x69, 0x2B } }, > + { .data = { 0x10, 0x40 } }, > + { .data = { 0x11, 0x98 } }, > + { .data = { 0xB6, 0x20 } }, > + { .data = { 0x51, 0x20 } }, > + { .data = { 0x09, 0x10 } }, > +}; What are those commands supposed to be? Some of them at least look pretty standard (and have proper functions): MIPI_DCS_EXIT_INVERT_MODE, _SET_DISPLAY_ON,
Re: [PATCH v3 41/50] drm/bridge: ti-tfp410: Report input bus config through bridge timings
On 15/03/2019 14:28, Peter Ujfalusi wrote: > On 15/03/2019 14.07, Tomi Valkeinen wrote: >>> If the pclk-sample is not defined in DT, it will default to 0 which >>> selects SAMPLE_NEGEDGE (== DRIVE_POSEDGE), right? >>> >>> But all the boards where I can find schematics with tfp410 have their >>> EDGE/HTPLG pin pulled up and according to the documentation when EDGE=1 >>> then tfp410 will sample on the rising edge. >>> >>> imho the pclk_sample should be initialized to 1 to avoid regression for >>> most of the boards using tfp410. >> >> Define "regression" =). If the omapdrm driver was always using >> DRIVE_POSEDGE, this driver should also be using DRIVE_POSEDGE, no? If it >> does the same as the old driver, it can't be a regression. This is, of >> course, only considering omapdrm based boards. >> >> That said, it sounds odd that this would be wrong in the old driver, but >> then again, it might well be, as code related to these sync signals has >> changed sooo many times, and the related DSS registers are somewhat >> confusing. > > On the boards data skew is enabled as well with maximum delay selected > with DK1=DK2=DK3=1, so sampling of data is delayed by 3 * 350 ps, so > about 1 ns, not much, but might be enough for the signal to transition > on the bus so even if the HW drivers on POSEDGE and tfp410 samples on > POSEDGE (+1ns delay from the edge) we don't see corruption? Ok. I don't think anyone has looked at it that closely. So, the syncs could well be wrong. Still, not a regression if it's the same way as it was. If the fixed default works fine (or better) than the wrong ones, I think that can be the default. But I'd be careful about changing what the default is, if we've had it the old way for a long time. 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: [linux-sunxi] [PATCH 1/6] drm/bridge: Export drm_bridge_detach
Hi Jakan, On Fri, 2019-03-15 at 18:38 +0530, Jagan Teki wrote: > Export drm_bridge_detach from drm bridge core so-that it > can use on respective interface or bridge driver while > detaching the bridge. I don't see why this change is required based on the commit log. The DRM bridge code clearly indicates that drm_bridge_attach should *not* be balanced with a drm_bridge_detach call in the driver, so this seems quite wrong. The DRM core itself should handle detaching the bridge, not the driver. Is there any reason why you need to do things differently for DSI? Cheers, Paul > Signed-off-by: Jagan Teki > --- > drivers/gpu/drm/drm_bridge.c | 1 + > include/drm/drm_bridge.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index 138b2711d389..569d4f345429 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -159,6 +159,7 @@ void drm_bridge_detach(struct drm_bridge *bridge) > > bridge->dev = NULL; > } > +EXPORT_SYMBOL(drm_bridge_detach); > > /** > * DOC: bridge callbacks > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 9da8c93f7976..4955e3e50fa4 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -301,6 +301,7 @@ void drm_bridge_remove(struct drm_bridge *bridge); > struct drm_bridge *of_drm_find_bridge(struct device_node *np); > int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, > struct drm_bridge *previous); > +void drm_bridge_detach(struct drm_bridge *bridge); > > bool drm_bridge_mode_fixup(struct drm_bridge *bridge, > const struct drm_display_mode *mode, > -- > 2.18.0.321.gffc6fa0e3 > -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/6] arm64: dts: allwinner: bananapi-m64: Enable S070WV20-CT16 DSI panel
On Fri, Mar 15, 2019 at 06:38:25PM +0530, Jagan Teki wrote: > This patch add support for Bananapi S070WV20-CT16 DSI panel to > BPI-M64 board. > > Bananapi S070WV20-CT16 is a pure RGB output panel with ICN6211 DSI/RGB > convertor bridge, so enable bridge along with associated panel. > > DSI panel connected via board DSI port with, > - DLDO1 as VCC-DSI supply > - PD6 gpio for reset pin > - PD5 gpio for backlight enable pin > - PD7 gpio for backlight vdd supply > > Signed-off-by: Jagan Teki > --- > .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 63 +++ > 1 file changed, 63 insertions(+) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts > b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts > index 7793ebb5d2b8..f31083aa4521 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts > @@ -45,6 +45,7 @@ > #include "sun50i-a64.dtsi" > > #include > +#include > > / { > model = "BananaPi-M64"; > @@ -56,6 +57,15 @@ > serial1 = > }; > > + backlight: backlight { > + compatible = "pwm-backlight"; > + pwms = <_pwm 0 5 PWM_POLARITY_INVERTED>; > + brightness-levels = <1 2 4 8 16 32 64 128 512>; > + default-brightness-level = <2>; > + enable-gpios = < 3 5 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PD5 */ > + power-supply = <_vdd_backlight>; > + }; > + > chosen { > stdout-path = "serial0:115200n8"; > }; > @@ -91,6 +101,26 @@ > }; > }; > > + panel-connector { > + compatible = "bananapi,s070wv20-ct16", "simple-panel"; > + > + port { > + backlight = <>; > + panel_ep: endpoint { > + remote-endpoint = <_out_ep>; > + }; > + }; > + }; > + > + reg_vdd_backlight: vdd-backlight { > + compatible = "regulator-fixed"; > + regulator-name = "vdd-backlight"; > + regulator-min-microvolt = <330>; > + regulator-max-microvolt = <330>; > + gpio = < 3 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PD7 */ > + enable-active-high; > + }; > + > wifi_pwrseq: wifi_pwrseq { > compatible = "mmc-pwrseq-simple"; > reset-gpios = <_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */ > @@ -116,6 +146,33 @@ > status = "okay"; > }; > > + { > + status = "okay"; > +}; > + > + { > + vcc-dsi-supply = <_dldo1>; /* VCC3V3-DSI */ > + status = "okay"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + bridge@0 { > + compatible = "bananapi,icn6211", "chipone, icn6211"; There's a typo in the compatible > + reg = <0>; > + reset-gpios = < 3 6 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD6 */ > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@1 { > + reg = <1>; > + > + bridge_out_ep: endpoint { > + remote-endpoint = <_ep>; > + }; > + }; The port here should be under a node called ports, with two ports, which is what you documented in your binding. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6] arm64: dts: allwinner: bananapi-m64: Enable S070WV20-CT16 DSI panel
This patch add support for Bananapi S070WV20-CT16 DSI panel to BPI-M64 board. Bananapi S070WV20-CT16 is a pure RGB output panel with ICN6211 DSI/RGB convertor bridge, so enable bridge along with associated panel. DSI panel connected via board DSI port with, - DLDO1 as VCC-DSI supply - PD6 gpio for reset pin - PD5 gpio for backlight enable pin - PD7 gpio for backlight vdd supply Signed-off-by: Jagan Teki --- .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 63 +++ 1 file changed, 63 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts index 7793ebb5d2b8..f31083aa4521 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts @@ -45,6 +45,7 @@ #include "sun50i-a64.dtsi" #include +#include / { model = "BananaPi-M64"; @@ -56,6 +57,15 @@ serial1 = }; + backlight: backlight { + compatible = "pwm-backlight"; + pwms = <_pwm 0 5 PWM_POLARITY_INVERTED>; + brightness-levels = <1 2 4 8 16 32 64 128 512>; + default-brightness-level = <2>; + enable-gpios = < 3 5 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PD5 */ + power-supply = <_vdd_backlight>; + }; + chosen { stdout-path = "serial0:115200n8"; }; @@ -91,6 +101,26 @@ }; }; + panel-connector { + compatible = "bananapi,s070wv20-ct16", "simple-panel"; + + port { + backlight = <>; + panel_ep: endpoint { + remote-endpoint = <_out_ep>; + }; + }; + }; + + reg_vdd_backlight: vdd-backlight { + compatible = "regulator-fixed"; + regulator-name = "vdd-backlight"; + regulator-min-microvolt = <330>; + regulator-max-microvolt = <330>; + gpio = < 3 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PD7 */ + enable-active-high; + }; + wifi_pwrseq: wifi_pwrseq { compatible = "mmc-pwrseq-simple"; reset-gpios = <_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */ @@ -116,6 +146,33 @@ status = "okay"; }; + { + status = "okay"; +}; + + { + vcc-dsi-supply = <_dldo1>; /* VCC3V3-DSI */ + status = "okay"; + #address-cells = <1>; + #size-cells = <0>; + + bridge@0 { + compatible = "bananapi,icn6211", "chipone, icn6211"; + reg = <0>; + reset-gpios = < 3 6 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD6 */ + #address-cells = <1>; + #size-cells = <0>; + + port@1 { + reg = <1>; + + bridge_out_ep: endpoint { + remote-endpoint = <_ep>; + }; + }; + }; +}; + { status = "okay"; }; @@ -208,6 +265,12 @@ status = "okay"; }; +_pwm { + pinctrl-names = "default"; + pinctrl-0 = <_pwm_pin>; + status = "okay"; +}; + _rsb { status = "okay"; -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] dt-bindings: display: bridge: Add ICN6211 MIPI-DSI to RGB convertor bridge
ICN6211 is MIPI-DSI/RGB converter bridge from chipone. It has a flexible configuration of MIPI DSI signal input and produce RGB565, RGB666, RGB888 output format. Add dt-bingings for it. Signed-off-by: Jagan Teki --- .../display/bridge/chipone,icn6211.txt| 36 +++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt diff --git a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt new file mode 100644 index ..7f13efd7ee7f --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt @@ -0,0 +1,36 @@ +Chipone ICN6211 MIPI-DSI to RGB Convertor Bridge + +ICN6211 is MIPI-DSI/RGB converter bridge from chipone. +It has a flexible configuration of MIPI DSI signal input +and produce RGB565, RGB666, RGB888 output format. + +Required properties for RGB: +- compatible: must be "chipone,icn6211" and one of: + * "bananapi,icn6211" +- reg: the virtual channel number of a DSI peripheral +- reset-gpios: a GPIO phandle for the reset pin + +The device node can contain following 'port' child nodes, +according to the OF graph bindings defined in [1]: + 0: DSI Input, not required, if the bridge is DSI controlled + 1: RGB Output, mandatory + +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt + +Example: + + bridge@0 { + compatible = "bananapi,icn6211", "chipone,icn6211"; + reg = <0>; + reset-gpios = < 3 6 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD6 */ + #address-cells = <1>; + #size-cells = <0>; + + port@1 { + reg = <1>; + + bridge_out_ep: endpoint { + remote-endpoint = <_ep>; + }; + }; + }; -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/6] drm/sun4i: dsi: Add bridge support
Some display panels would come up with a non-DSI output which can have an option to connect DSI interface by means of bridge convertor. This DSI to non-DSI bridge convertor would require a bridge driver that would communicate the DSI controller for bridge functionalities. So, add support for bridge functionalities in Allwinner DSI controller. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 65 +++--- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 1 + 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 0960b96b62cc..64d74313b842 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -781,6 +781,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder) if (!IS_ERR(dsi->panel)) drm_panel_prepare(dsi->panel); + if (!IS_ERR(dsi->bridge)) + drm_bridge_pre_enable(dsi->bridge); + /* * FIXME: This should be moved after the switch to HS mode. * @@ -796,6 +799,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder) if (!IS_ERR(dsi->panel)) drm_panel_enable(dsi->panel); + if (!IS_ERR(dsi->bridge)) + drm_bridge_enable(dsi->bridge); + sun6i_dsi_start(dsi, DSI_START_HSC); udelay(1000); @@ -812,6 +818,9 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder) if (!IS_ERR(dsi->panel)) { drm_panel_disable(dsi->panel); drm_panel_unprepare(dsi->panel); + } else if (!IS_ERR(dsi->bridge)) { + drm_bridge_disable(dsi->bridge); + drm_bridge_post_disable(dsi->bridge); } phy_power_off(dsi->dphy); @@ -973,11 +982,16 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host, struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); dsi->device = device; - dsi->panel = of_drm_find_panel(device->dev.of_node); - if (IS_ERR(dsi->panel)) - return PTR_ERR(dsi->panel); - dev_info(host->dev, "Attached device %s\n", device->name); + dsi->bridge = of_drm_find_bridge(device->dev.of_node); + if (!dsi->bridge) { + dsi->panel = of_drm_find_panel(device->dev.of_node); + if (IS_ERR(dsi->panel)) + return PTR_ERR(dsi->panel); + } + + dev_info(host->dev, "Attached %s %s\n", +dsi->bridge ? "bridge" : "panel", device->name); return 0; } @@ -1055,8 +1069,10 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master, struct sun4i_tcon *tcon0 = sun4i_get_tcon0(drm); int ret; - if (!dsi->panel) + if (!(dsi->panel || dsi->bridge)) { + dev_info(drm->dev, "No panel or bridge found... DSI output disabled\n"); return -EPROBE_DEFER; + } dsi->drv = drv; @@ -1078,19 +1094,29 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master, } dsi->encoder.possible_crtcs = BIT(0); - drm_connector_helper_add(>connector, -_dsi_connector_helper_funcs); - ret = drm_connector_init(drm, >connector, -_dsi_connector_funcs, -DRM_MODE_CONNECTOR_DSI); - if (ret) { - dev_err(dsi->dev, - "Couldn't initialise the DSI connector\n"); - goto err_cleanup_connector; + if (dsi->panel) { + drm_connector_helper_add(>connector, +_dsi_connector_helper_funcs); + ret = drm_connector_init(drm, >connector, +_dsi_connector_funcs, +DRM_MODE_CONNECTOR_DSI); + if (ret) { + dev_err(dsi->dev, + "Couldn't initialise the DSI connector\n"); + goto err_cleanup_connector; + } + + drm_connector_attach_encoder(>connector, >encoder); + drm_panel_attach(dsi->panel, >connector); } - drm_connector_attach_encoder(>connector, >encoder); - drm_panel_attach(dsi->panel, >connector); + if (dsi->bridge) { + ret = drm_bridge_attach(>encoder, dsi->bridge, NULL); + if (ret) { + dev_err(dsi->dev, "Couldn't attach the DSI bridge\n"); + goto err_cleanup_connector; + } + } return 0; @@ -1104,7 +1130,12 @@ static void sun6i_dsi_unbind(struct device *dev, struct device *master, { struct sun6i_dsi *dsi = dev_get_drvdata(dev); - drm_panel_detach(dsi->panel); + if (dsi->panel) + drm_panel_detach(dsi->panel); + +
[PATCH 2/6] drm/exynos: dsi: Use drm_bridge_detach
drm_bridge_detach now available to use while detaching bridge, use this core wrapper instead of explicitly pointing the bridge funcs. Cc: Dae Cc: Joonyoung Shim Cc: Seung-Woo Kim Cc: Kyungmin Park Signed-off-by: Jagan Teki --- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index a4253dd55f86..5daf43d02768 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1583,8 +1583,7 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, dsi->connector.status = connector_status_disconnected; mutex_unlock(>mode_config.mutex); } else { - if (dsi->out_bridge->funcs->detach) - dsi->out_bridge->funcs->detach(dsi->out_bridge); + drm_bridge_detach(dsi->out_bridge); dsi->out_bridge = NULL; } -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/6] drm/bridge: Add Chipone ICN6211 MIPI-DSI/RGB convertor bridge
ICN6211 is MIPI-DSI/RGB converter bridge from chipone. It has a flexible configuration of MIPI DSI signal input and produce RGB565, RGB666, RGB888 output format. Add bridge driver for it. Signed-off-by: Jagan Teki --- MAINTAINERS | 6 + drivers/gpu/drm/bridge/Kconfig | 10 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/chipone-icn6211.c | 275 +++ 4 files changed, 292 insertions(+) create mode 100644 drivers/gpu/drm/bridge/chipone-icn6211.c diff --git a/MAINTAINERS b/MAINTAINERS index 4de80222cffb..e529addd30f5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4897,6 +4897,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc S: Maintained F: drivers/gpu/drm/bochs/ +DRM DRIVER FOR CHIPONE ICN6211 MIPI-DSI to RGB CONVERTOR BRIDGE +M: Jagan Teki +S: Maintained +F: drivers/gpu/drm/bridge/chipone-icn6211.c +F: Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt + DRM DRIVER FOR FARADAY TVE200 TV ENCODER M: Linus Walleij T: git git://anongit.freedesktop.org/drm/drm-misc diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 8840f396a7b6..cd314572e4ed 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -36,6 +36,16 @@ config DRM_CDNS_DSI Support Cadence DPI to DSI bridge. This is an internal bridge and is meant to be directly embedded in a SoC. +config DRM_CHIPONE_ICN6211 + tristate "Chipone ICN6211 MIPI-DSI/RGB converter bridge" + depends on DRM && DRM_PANEL + depends on OF + select DRM_MIPI_DSI + help + ICN6211 is MIPI-DSI/RGB converter bridge from chipone. + It has a flexible configuration of MIPI DSI signal input + and produce RGB565, RGB666, RGB888 output format. + config DRM_DUMB_VGA_DAC tristate "Dumb VGA DAC Bridge support" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 4934fcf5a6f8..541fdccad10b 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o +obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += megachips-stdp-ge-b850v3-fw.o diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c new file mode 100644 index ..cd2f3505f845 --- /dev/null +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c @@ -0,0 +1,275 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2018 Amarula Solutions + * Author: Jagan Teki + */ + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#define ICN6211_INIT_CMD_LEN 2 + +struct chipone { + struct device *dev; + struct drm_bridge bridge; + struct drm_connector connector; + struct drm_panel *panel; + + struct gpio_desc *reset; +}; + +static inline struct chipone *bridge_to_chipone(struct drm_bridge *bridge) +{ + return container_of(bridge, struct chipone, bridge); +} + +static inline +struct chipone *connector_to_chipone(struct drm_connector *connector) +{ + return container_of(connector, struct chipone, connector); +} + +struct icn6211_init_cmd { + u8 data[ICN6211_INIT_CMD_LEN]; +}; + +static const struct icn6211_init_cmd icn6211_init_cmds[] = { + { .data = { 0x7A, 0xC1 } }, + { .data = { 0x20, 0x20 } }, + { .data = { 0x21, 0xE0 } }, + { .data = { 0x22, 0x13 } }, + { .data = { 0x23, 0x28 } }, + { .data = { 0x24, 0x30 } }, + { .data = { 0x25, 0x28 } }, + { .data = { 0x26, 0x00 } }, + { .data = { 0x27, 0x0D } }, + { .data = { 0x28, 0x03 } }, + { .data = { 0x29, 0x1D } }, + { .data = { 0x34, 0x80 } }, + { .data = { 0x36, 0x28 } }, + { .data = { 0xB5, 0xA0 } }, + { .data = { 0x5C, 0xFF } }, + { .data = { 0x2A, 0x01 } }, + { .data = { 0x56, 0x92 } }, + { .data = { 0x6B, 0x71 } }, + { .data = { 0x69, 0x2B } }, + { .data = { 0x10, 0x40 } }, + { .data = { 0x11, 0x98 } }, + { .data = { 0xB6, 0x20 } }, + { .data = { 0x51, 0x20 } }, + { .data = { 0x09, 0x10 } }, +}; + +static int chipone_get_modes(struct drm_connector *connector) +{ + struct chipone *icn = connector_to_chipone(connector); + + return drm_panel_get_modes(icn->panel); +} + +static const +struct drm_connector_helper_funcs chipone_connector_helper_funcs = { + .get_modes = chipone_get_modes, +}; + +static const struct drm_connector_funcs chipone_connector_funcs = { + .fill_modes =
[PATCH 1/6] drm/bridge: Export drm_bridge_detach
Export drm_bridge_detach from drm bridge core so-that it can use on respective interface or bridge driver while detaching the bridge. Signed-off-by: Jagan Teki --- drivers/gpu/drm/drm_bridge.c | 1 + include/drm/drm_bridge.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 138b2711d389..569d4f345429 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -159,6 +159,7 @@ void drm_bridge_detach(struct drm_bridge *bridge) bridge->dev = NULL; } +EXPORT_SYMBOL(drm_bridge_detach); /** * DOC: bridge callbacks diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 9da8c93f7976..4955e3e50fa4 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -301,6 +301,7 @@ void drm_bridge_remove(struct drm_bridge *bridge); struct drm_bridge *of_drm_find_bridge(struct device_node *np); int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, struct drm_bridge *previous); +void drm_bridge_detach(struct drm_bridge *bridge); bool drm_bridge_mode_fixup(struct drm_bridge *bridge, const struct drm_display_mode *mode, -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/6] drm/bridge: Add ICN6211 MIPI-DSI/RGB bridge
This series support Chipone ICN6211 DSI/RGB bridge support. This ICN6211 bridge is taking flexible configuration of MIPI DSI signal input and produce RGB565, RGB666, RGB888 output format and it is present in the Bananapi s070wv20-ct16 panel. Initially similar support is written as dsi panel driver, but based on the discussion from this thread [1] and input from "Andrzej Hajda" I would ended-up writing bridge driver. patch 1, 2: export drm_bridge_detach patch 3: bridge support on Allwinner DSI controller patch 4: ICN6211 dt-bindings patch 5: ICN6211 bridge driver patch 6: overlay to enable bridge and panel on BPI-M64, this would eventually depends on 'Allwinner A64 MIPI-DSI" support [2] [2] https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=90117 [1] https://patchwork.kernel.org/patch/10657579/ Any inputs? Jagan. Jagan Teki (6): drm/bridge: Export drm_bridge_detach drm/exynos: dsi: Use drm_bridge_detach drm/sun4i: dsi: Add bridge support dt-bindings: display: bridge: Add ICN6211 MIPI-DSI to RGB convertor bridge drm/bridge: Add Chipone ICN6211 MIPI-DSI/RGB convertor bridge [DO NOT MERGE] arm64: dts: allwinner: bananapi-m64: Enable S070WV20-CT16 DSI panel .../display/bridge/chipone,icn6211.txt| 36 +++ MAINTAINERS | 6 + .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 63 drivers/gpu/drm/bridge/Kconfig| 10 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/chipone-icn6211.c | 275 ++ drivers/gpu/drm/drm_bridge.c | 1 + drivers/gpu/drm/exynos/exynos_drm_dsi.c | 3 +- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c| 65 +++-- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h| 1 + include/drm/drm_bridge.h | 1 + 11 files changed, 443 insertions(+), 19 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/chipone,icn6211.txt create mode 100644 drivers/gpu/drm/bridge/chipone-icn6211.c -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201795] [Regression] Wrong 4k resolution detected with DisplayPort to HDMI adapter on amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=201795 --- Comment #20 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) --- Not every user will want the same scaling parameters for non-native signals. The display itself will often offer a configurable mode itself for its scaler for this reason. I don't have the exact displays that you have, but I have a test setup with a 4096x2160 and 3840x2160 display and I'm able to mirror the desktop using Plasma and have the image fullscreen on both displays (without black bars) because the 4096x2160 display defaults to fullscreen scaling. But I can also reproduce your problem by changing the scaling mode in the display's OSD to 1:1 scaling, which shows black bars. This is because the driver doesn't do any scaling itself by default. The driver can't query the monitor to understand what the display is going to do, and it also can't guess what the user's intended preference is here. So we expose the standard (ie. shared across other drivers as well) DRM property for "scaling mode" to give more control over the behavior when possible. As for why it's not configurable via GUI, that's more of a userspace feature request for the Plasma / GNOME / etc developers. -- 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
[PATCH v2 1/1] drm/tilcdc: Register cpufreq notifier after we have initialized crtc
Register cpufreq notifier after we have initialized the crtc and unregister it before we remove the ctrc. Receiving a cpufreq notify without crtc causes a crash. Reported-by: Peter Ujfalusi Signed-off-by: Jyri Sarha --- Couple of minor changes: - Add forgotten Reported-by: - While moving cpufreq_register_notifier-call, fix indentation to dim's liking drivers/gpu/drm/tilcdc/tilcdc_drv.c | 34 ++--- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 36fd05b145a4..a5d5d5ae3287 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -183,6 +183,12 @@ static void tilcdc_fini(struct drm_device *dev) { struct tilcdc_drm_private *priv = dev->dev_private; +#ifdef CONFIG_CPU_FREQ + if (priv->freq_transition.notifier_call) + cpufreq_unregister_notifier(>freq_transition, + CPUFREQ_TRANSITION_NOTIFIER); +#endif + if (priv->crtc) tilcdc_crtc_shutdown(priv->crtc); @@ -193,12 +199,6 @@ static void tilcdc_fini(struct drm_device *dev) drm_irq_uninstall(dev); drm_mode_config_cleanup(dev); -#ifdef CONFIG_CPU_FREQ - if (priv->freq_transition.notifier_call) - cpufreq_unregister_notifier(>freq_transition, - CPUFREQ_TRANSITION_NOTIFIER); -#endif - if (priv->clk) clk_put(priv->clk); @@ -269,17 +269,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) goto init_failed; } -#ifdef CONFIG_CPU_FREQ - priv->freq_transition.notifier_call = cpufreq_transition; - ret = cpufreq_register_notifier(>freq_transition, - CPUFREQ_TRANSITION_NOTIFIER); - if (ret) { - dev_err(dev, "failed to register cpufreq notifier\n"); - priv->freq_transition.notifier_call = NULL; - goto init_failed; - } -#endif - if (of_property_read_u32(node, "max-bandwidth", >max_bandwidth)) priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH; @@ -356,6 +345,17 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) } modeset_init(ddev); +#ifdef CONFIG_CPU_FREQ + priv->freq_transition.notifier_call = cpufreq_transition; + ret = cpufreq_register_notifier(>freq_transition, + CPUFREQ_TRANSITION_NOTIFIER); + if (ret) { + dev_err(dev, "failed to register cpufreq notifier\n"); + priv->freq_transition.notifier_call = NULL; + goto init_failed; + } +#endif + if (priv->is_componentized) { ret = component_bind_all(dev, ddev); if (ret < 0) -- 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
[Bug 108493] Unigine Heaven at 4K crashes amdgpu and causes a GPU hang
https://bugs.freedesktop.org/show_bug.cgi?id=108493 Timur Kristóf changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #19 from Timur Kristóf --- Since this is fixed by kernel 5.0, I'm marking it as resolved fixed. -- 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 110117] Waking from Suspend causes screen to appear with grey static (like a TV with no signal)
https://bugs.freedesktop.org/show_bug.cgi?id=110117 --- Comment #1 from Nicholas Kazlauskas --- I don't see anything immediately wrong in the log at the first glance at least so it'd be helpful if you could post your dmesg log from after the problem occurred with drm.debug=4 in your kernel boot parameters. Please post your xorg log if applicable as well. -- 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 41/50] drm/bridge: ti-tfp410: Report input bus config through bridge timings
On 15/03/2019 14.07, Tomi Valkeinen wrote: >> If the pclk-sample is not defined in DT, it will default to 0 which >> selects SAMPLE_NEGEDGE (== DRIVE_POSEDGE), right? >> >> But all the boards where I can find schematics with tfp410 have their >> EDGE/HTPLG pin pulled up and according to the documentation when EDGE=1 >> then tfp410 will sample on the rising edge. >> >> imho the pclk_sample should be initialized to 1 to avoid regression for >> most of the boards using tfp410. > > Define "regression" =). If the omapdrm driver was always using > DRIVE_POSEDGE, this driver should also be using DRIVE_POSEDGE, no? If it > does the same as the old driver, it can't be a regression. This is, of > course, only considering omapdrm based boards. > > That said, it sounds odd that this would be wrong in the old driver, but > then again, it might well be, as code related to these sync signals has > changed sooo many times, and the related DSS registers are somewhat > confusing. On the boards data skew is enabled as well with maximum delay selected with DK1=DK2=DK3=1, so sampling of data is delayed by 3 * 350 ps, so about 1 ns, not much, but might be enough for the signal to transition on the bus so even if the HW drivers on POSEDGE and tfp410 samples on POSEDGE (+1ns delay from the edge) we don't see corruption? > That gave me an idea, I need to write an rwmem script to print the DSS > sync flags in plain english... > > Tomi > - Péter 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 8/9] drm/syncobj: add timeline signal ioctl for syncobj v3
v2: individually allocate chain array, since chain node is free independently. v3: all existing points must be already signaled before cpu perform signal operation, so add check condition for that. Signed-off-by: Chunming Zhou --- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 103 + include/uapi/drm/drm.h | 1 + 4 files changed, 108 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index dd11ae5f1eef..d9a483a5fce0 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -190,6 +190,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 92b3b7b2fd81..d337f161909c 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -696,6 +696,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, drm_syncobj_timeline_signal_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 306c7b7e2770..eaeb038f97d7 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1183,6 +1183,109 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, return ret; } +int +drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_timeline_array *args = data; + struct drm_syncobj **syncobjs; + struct dma_fence_chain **chains; + uint64_t *points; + uint32_t i, j, timeline_count = 0; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -EOPNOTSUPP; + + if (args->pad != 0) + return -EINVAL; + + if (args->count_handles == 0) + return -EINVAL; + + ret = drm_syncobj_array_find(file_private, +u64_to_user_ptr(args->handles), +args->count_handles, +); + if (ret < 0) + return ret; + + for (i = 0; i < args->count_handles; i++) { + struct dma_fence_chain *chain; +struct dma_fence *fence; + +fence = drm_syncobj_fence_get(syncobjs[i]); +chain = to_dma_fence_chain(fence); +if (chain) { +struct dma_fence *iter; + +dma_fence_chain_for_each(iter, fence) { +if (!iter) +break; + if (!dma_fence_is_signaled(iter)) { + dma_fence_put(iter); + DRM_ERROR("Client must guarantee all existing timeline points signaled before performing host signal operation!"); + ret = -EPERM; + goto out; + } +} +} +} + + points = kmalloc_array(args->count_handles, sizeof(*points), + GFP_KERNEL); + if (!points) { + ret = -ENOMEM; + goto out; + } + if (!u64_to_user_ptr(args->points)) { + memset(points, 0, args->count_handles * sizeof(uint64_t)); + } else if (copy_from_user(points, u64_to_user_ptr(args->points), + sizeof(uint64_t) * args->count_handles)) { + ret = -EFAULT; + goto err_points; + } + + + for (i = 0; i < args->count_handles; i++) { + if (points[i]) + timeline_count++; + } + chains = kmalloc_array(timeline_count, sizeof(void *), GFP_KERNEL); + if (!chains) { + ret = -ENOMEM; + goto err_points; +
[PATCH 7/9] drm/syncobj: add transition iotcls between binary and timeline v2
we need to import/export timeline point. v2: unify to one transfer ioctl Signed-off-by: Chunming Zhou --- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 74 ++ include/uapi/drm/drm.h | 10 + 4 files changed, 88 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 695179bb88dc..dd11ae5f1eef 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -180,6 +180,8 @@ int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 7a534c184e52..92b3b7b2fd81 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -686,6 +686,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE, drm_syncobj_fd_to_handle_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TRANSFER, drm_syncobj_transfer_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, drm_syncobj_timeline_wait_ioctl, diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index dd19c47d0b44..306c7b7e2770 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -679,6 +679,80 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, >handle); } +static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private, + struct drm_syncobj_transfer *args) +{ + struct drm_syncobj *timeline_syncobj = NULL; + struct dma_fence *fence; + struct dma_fence_chain *chain; + int ret; + + timeline_syncobj = drm_syncobj_find(file_private, args->dst_handle); + if (!timeline_syncobj) { + return -ENOENT; + } + ret = drm_syncobj_find_fence(file_private, args->src_handle, +args->src_point, args->flags, +); + if (ret) + goto err; + chain = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL); + if (!chain) { + ret = -ENOMEM; + goto err1; + } + drm_syncobj_add_point(timeline_syncobj, chain, fence, args->dst_point); +err1: + dma_fence_put(fence); +err: + drm_syncobj_put(timeline_syncobj); + + return ret; +} + +static int +drm_syncobj_transfer_to_binary(struct drm_file *file_private, + struct drm_syncobj_transfer *args) +{ + struct drm_syncobj *binary_syncobj = NULL; + struct dma_fence *fence; + int ret; + + binary_syncobj = drm_syncobj_find(file_private, args->dst_handle); + if (!binary_syncobj) + return -ENOENT; + ret = drm_syncobj_find_fence(file_private, args->src_handle, +args->src_point, args->flags, ); + if (ret) + goto err; + drm_syncobj_replace_fence(binary_syncobj, fence); + dma_fence_put(fence); +err: + drm_syncobj_put(binary_syncobj); + + return ret; +} +int +drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_transfer *args = data; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -ENODEV; + + if (args->pad) + return -EINVAL; + + if (args->dst_point) + ret = drm_syncobj_transfer_to_timeline(file_private, args); + else + ret = drm_syncobj_transfer_to_binary(file_private, args); + + return ret; +} + static void syncobj_wait_fence_func(struct dma_fence *fence, struct dma_fence_cb *cb) { diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index b2c36f2b2599..4c1e2e6579fa 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -735,6 +735,15 @@ struct drm_syncobj_handle { __u32 pad; }; +struct drm_syncobj_transfer { + __u32
[PATCH 6/9] drm/amdgpu: add timeline support in amdgpu CS v3
syncobj wait/signal operation is appending in command submission. v2: separate to two kinds in/out_deps functions v3: fix checking for timeline syncobj Signed-off-by: Chunming Zhou Cc: Daniel Rakos Cc: Jason Ekstrand Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Christian König Cc: Chris Wilson --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 152 + include/uapi/drm/amdgpu_drm.h | 8 ++ 3 files changed, 144 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 8d0d7f3dd5fb..deec2c796253 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -433,6 +433,12 @@ struct amdgpu_cs_chunk { void*kdata; }; +struct amdgpu_cs_post_dep { + struct drm_syncobj *syncobj; + struct dma_fence_chain *chain; + u64 point; +}; + struct amdgpu_cs_parser { struct amdgpu_device*adev; struct drm_file *filp; @@ -462,8 +468,8 @@ struct amdgpu_cs_parser { /* user fence */ struct amdgpu_bo_list_entry uf_entry; - unsigned num_post_dep_syncobjs; - struct drm_syncobj **post_dep_syncobjs; + unsignednum_post_deps; + struct amdgpu_cs_post_dep *post_deps; }; static inline u32 amdgpu_get_ib_value(struct amdgpu_cs_parser *p, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 52a5e4fdc95b..2f6239b6be6f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -215,6 +215,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs case AMDGPU_CHUNK_ID_SYNCOBJ_IN: case AMDGPU_CHUNK_ID_SYNCOBJ_OUT: case AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES: + case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT: + case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL: break; default: @@ -804,9 +806,11 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, ttm_eu_backoff_reservation(>ticket, >validated); - for (i = 0; i < parser->num_post_dep_syncobjs; i++) - drm_syncobj_put(parser->post_dep_syncobjs[i]); - kfree(parser->post_dep_syncobjs); + for (i = 0; i < parser->num_post_deps; i++) { + drm_syncobj_put(parser->post_deps[i].syncobj); + kfree(parser->post_deps[i].chain); + } + kfree(parser->post_deps); dma_fence_put(parser->fence); @@ -1117,13 +1121,18 @@ static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p, } static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, -uint32_t handle) +uint32_t handle, u64 point, +u64 flags) { - int r; struct dma_fence *fence; - r = drm_syncobj_find_fence(p->filp, handle, 0, 0, ); - if (r) + int r; + + r = drm_syncobj_find_fence(p->filp, handle, point, flags, ); + if (r) { + DRM_ERROR("syncobj %u failed to find fence @ %llu (%d)!\n", + handle, point, r); return r; + } r = amdgpu_sync_fence(p->adev, >job->sync, fence, true); dma_fence_put(fence); @@ -1134,46 +1143,118 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, static int amdgpu_cs_process_syncobj_in_dep(struct amdgpu_cs_parser *p, struct amdgpu_cs_chunk *chunk) { + struct drm_amdgpu_cs_chunk_sem *deps; unsigned num_deps; int i, r; - struct drm_amdgpu_cs_chunk_sem *deps; deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; num_deps = chunk->length_dw * 4 / sizeof(struct drm_amdgpu_cs_chunk_sem); + for (i = 0; i < num_deps; ++i) { + r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle, + 0, 0); + if (r) + return r; + } + + return 0; +} + +static int amdgpu_cs_process_syncobj_timeline_in_dep(struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ + struct drm_amdgpu_cs_chunk_syncobj *syncobj_deps; + unsigned num_deps; + int i, r; + + syncobj_deps = (struct drm_amdgpu_cs_chunk_syncobj *)chunk->kdata; + num_deps = chunk->length_dw * 4 / + sizeof(struct drm_amdgpu_cs_chunk_syncobj); for (i = 0; i < num_deps; ++i) { - r =
[PATCH 5/9] drm/syncobj: use the timeline point in drm_syncobj_find_fence v3
From: Christian König Implement finding the right timeline point in drm_syncobj_find_fence. v2: return -EINVAL when the point is not submitted yet. v3: fix reference counting bug, add flags handling as well v4: add timeout for find fence Signed-off-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 50 --- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 0e62a793c8dd..dd19c47d0b44 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -213,6 +213,8 @@ static void drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) dma_fence_put(fence); } +/* 5s */ +#define DRM_SYNCOBJ_WAIT_FOR_SUBMIT_TIMEOUT 50 /** * drm_syncobj_find_fence - lookup and reference the fence in a sync object * @file_private: drm file private pointer @@ -233,16 +235,58 @@ int drm_syncobj_find_fence(struct drm_file *file_private, struct dma_fence **fence) { struct drm_syncobj *syncobj = drm_syncobj_find(file_private, handle); - int ret = 0; + struct syncobj_wait_entry wait; + u64 timeout = nsecs_to_jiffies64(DRM_SYNCOBJ_WAIT_FOR_SUBMIT_TIMEOUT); + int ret; if (!syncobj) return -ENOENT; *fence = drm_syncobj_fence_get(syncobj); - if (!*fence) { + drm_syncobj_put(syncobj); + + if (*fence) { + ret = dma_fence_chain_find_seqno(fence, point); + if (!ret) + return 0; + dma_fence_put(*fence); + } else { ret = -EINVAL; } - drm_syncobj_put(syncobj); + + if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) + return ret; + + memset(, 0, sizeof(wait)); + wait.task = current; + wait.point = point; + drm_syncobj_fence_add_wait(syncobj, ); + + do { + set_current_state(TASK_INTERRUPTIBLE); + if (wait.fence) { + ret = 0; + break; + } +if (timeout == 0) { +ret = -ETIME; +break; +} + + if (signal_pending(current)) { + ret = -ERESTARTSYS; + break; + } + +timeout = schedule_timeout(timeout); + } while (1); + + __set_current_state(TASK_RUNNING); + *fence = wait.fence; + + if (wait.node.next) + drm_syncobj_remove_wait(syncobj, ); + return ret; } EXPORT_SYMBOL(drm_syncobj_find_fence); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 9/9] drm/amdgpu: update version for timeline syncobj support in amdgpu
Signed-off-by: Chunming Zhou --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 8a0732088640..4d8db87048d3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -74,9 +74,10 @@ * - 3.28.0 - Add AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES * - 3.29.0 - Add AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID * - 3.30.0 - Add AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE. + * - 3.31.0 - Add syncobj timeline support to AMDGPU_CS. */ #define KMS_DRIVER_MAJOR 3 -#define KMS_DRIVER_MINOR 30 +#define KMS_DRIVER_MINOR 31 #define KMS_DRIVER_PATCHLEVEL 0 int amdgpu_vram_limit = 0; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/9] drm/syncobj: add timeline payload query ioctl v6
user mode can query timeline payload. v2: check return value of copy_to_user v3: handle querying entry by entry v4: rebase on new chain container, simplify interface v5: query last signaled timeline point, not last point. v6: add unorder point check Signed-off-by: Chunming Zhou Cc: Daniel Rakos Cc: Jason Ekstrand Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Christian König Cc: Chris Wilson --- drivers/gpu/drm/drm_internal.h | 2 ++ drivers/gpu/drm/drm_ioctl.c| 2 ++ drivers/gpu/drm/drm_syncobj.c | 62 ++ include/uapi/drm/drm.h | 10 ++ 4 files changed, 76 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 331ac6225b58..695179bb88dc 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -188,6 +188,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); /* drm_framebuffer.c */ void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index c984654646fa..7a534c184e52 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -694,6 +694,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_UNLOCKED), diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index eae51978cda4..0e62a793c8dd 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1064,3 +1064,65 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, return ret; } + +int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_timeline_array *args = data; + struct drm_syncobj **syncobjs; + uint64_t __user *points = u64_to_user_ptr(args->points); + uint32_t i; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -ENODEV; + + if (args->pad != 0) + return -EINVAL; + + if (args->count_handles == 0) + return -EINVAL; + + ret = drm_syncobj_array_find(file_private, +u64_to_user_ptr(args->handles), +args->count_handles, +); + if (ret < 0) + return ret; + + for (i = 0; i < args->count_handles; i++) { + struct dma_fence_chain *chain; + struct dma_fence *fence; + uint64_t point; + + fence = drm_syncobj_fence_get(syncobjs[i]); + chain = to_dma_fence_chain(fence); + if (chain) { + struct dma_fence *iter, *last_signaled = NULL; + + dma_fence_chain_for_each(iter, fence) { + if (!iter) + break; + dma_fence_put(last_signaled); + last_signaled = dma_fence_get(iter); + if (!to_dma_fence_chain(last_signaled)->prev_seqno) + /* It is most likely that timeline has +* unorder points. */ + break; + } + point = dma_fence_is_signaled(last_signaled) ? + last_signaled->seqno : + to_dma_fence_chain(last_signaled)->prev_seqno; + dma_fence_put(last_signaled); + } else { + point = 0; + } + ret = copy_to_user([i], , sizeof(uint64_t)); + ret = ret ? -EFAULT : 0; + if (ret) + break; + } + drm_syncobj_array_free(syncobjs, args->count_handles); + + return ret; +} diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 0092111d002c..b2c36f2b2599 100644 ---
[PATCH 2/9] drm/syncobj: add new drm_syncobj_add_point interface v4
From: Christian König Use the dma_fence_chain object to create a timeline of fence objects instead of just replacing the existing fence. v2: rebase and cleanup v3: fix garbage collection parameters v4: add unorder point check, print a warn calltrace Signed-off-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 39 +++ include/drm/drm_syncobj.h | 5 + 2 files changed, 44 insertions(+) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 5329e66598c6..19a9ce638119 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -122,6 +122,45 @@ static void drm_syncobj_remove_wait(struct drm_syncobj *syncobj, spin_unlock(>lock); } +/** + * drm_syncobj_add_point - add new timeline point to the syncobj + * @syncobj: sync object to add timeline point do + * @chain: chain node to use to add the point + * @fence: fence to encapsulate in the chain node + * @point: sequence number to use for the point + * + * Add the chain node as new timeline point to the syncobj. + */ +void drm_syncobj_add_point(struct drm_syncobj *syncobj, + struct dma_fence_chain *chain, + struct dma_fence *fence, + uint64_t point) +{ + struct syncobj_wait_entry *cur, *tmp; + struct dma_fence *prev; + + dma_fence_get(fence); + + spin_lock(>lock); + + prev = drm_syncobj_fence_get(syncobj); + /* You are adding an unorder point to timeline, which could cause payload returned from query_ioctl is 0! */ + WARN_ON_ONCE(prev && prev->seqno >= point); + dma_fence_chain_init(chain, prev, fence, point); + rcu_assign_pointer(syncobj->fence, >base); + + list_for_each_entry_safe(cur, tmp, >cb_list, node) { + list_del_init(>node); + syncobj_wait_syncobj_func(syncobj, cur); + } + spin_unlock(>lock); + + /* Walk the chain once to trigger garbage collection */ + dma_fence_chain_for_each(fence, prev); + dma_fence_put(prev); +} +EXPORT_SYMBOL(drm_syncobj_add_point); + /** * drm_syncobj_replace_fence - replace fence in a sync object. * @syncobj: Sync object to replace fence in diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 0311c9fdbd2f..6cf7243a1dc5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -27,6 +27,7 @@ #define __DRM_SYNCOBJ_H__ #include +#include struct drm_file; @@ -112,6 +113,10 @@ drm_syncobj_fence_get(struct drm_syncobj *syncobj) struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, u32 handle); +void drm_syncobj_add_point(struct drm_syncobj *syncobj, + struct dma_fence_chain *chain, + struct dma_fence *fence, + uint64_t point); void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *fence); int drm_syncobj_find_fence(struct drm_file *file_private, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/9] drm/syncobj: add support for timeline point wait v8
points array is one-to-one match with syncobjs array. v2: add seperate ioctl for timeline point wait, otherwise break uapi. v3: userspace can specify two kinds waits:: a. Wait for time point to be completed. b. and wait for time point to become available v4: rebase v5: add comment for xxx_WAIT_AVAILABLE v6: rebase and rework on new container v7: drop _WAIT_COMPLETED, it is the default anyway v8: correctly handle garbage collected fences Signed-off-by: Chunming Zhou Signed-off-by: Christian König Cc: Daniel Rakos Cc: Jason Ekstrand Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Chris Wilson --- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 153 ++--- include/uapi/drm/drm.h | 15 4 files changed, 143 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 251d67e04c2d..331ac6225b58 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -182,6 +182,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 687943df58e1..c984654646fa 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -688,6 +688,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, drm_syncobj_timeline_wait_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 19a9ce638119..eae51978cda4 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -61,6 +61,7 @@ struct syncobj_wait_entry { struct task_struct *task; struct dma_fence *fence; struct dma_fence_cb fence_cb; + u64point; }; static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj, @@ -95,6 +96,8 @@ EXPORT_SYMBOL(drm_syncobj_find); static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj, struct syncobj_wait_entry *wait) { + struct dma_fence *fence; + if (wait->fence) return; @@ -103,11 +106,15 @@ static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj, * have the lock, try one more time just to be sure we don't add a * callback when a fence has already been set. */ - if (syncobj->fence) - wait->fence = dma_fence_get( - rcu_dereference_protected(syncobj->fence, 1)); - else + fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, 1)); + if (!fence || dma_fence_chain_find_seqno(, wait->point)) { + dma_fence_put(fence); list_add_tail(>node, >cb_list); + } else if (!fence) { + wait->fence = dma_fence_get_stub(); + } else { + wait->fence = fence; + } spin_unlock(>lock); } @@ -149,10 +156,8 @@ void drm_syncobj_add_point(struct drm_syncobj *syncobj, dma_fence_chain_init(chain, prev, fence, point); rcu_assign_pointer(syncobj->fence, >base); - list_for_each_entry_safe(cur, tmp, >cb_list, node) { - list_del_init(>node); + list_for_each_entry_safe(cur, tmp, >cb_list, node) syncobj_wait_syncobj_func(syncobj, cur); - } spin_unlock(>lock); /* Walk the chain once to trigger garbage collection */ @@ -184,10 +189,8 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, rcu_assign_pointer(syncobj->fence, fence); if (fence != old_fence) { - list_for_each_entry_safe(cur, tmp, >cb_list, node) { - list_del_init(>node); + list_for_each_entry_safe(cur, tmp, >cb_list, node) syncobj_wait_syncobj_func(syncobj, cur); - } } spin_unlock(>lock); @@ -644,13 +647,27 @@ static void syncobj_wait_fence_func(struct dma_fence
[PATCH 1/9] dma-buf: add new dma_fence_chain container v5
From: Christian König Lockless container implementation similar to a dma_fence_array, but with only two elements per node and automatic garbage collection. v2: properly document dma_fence_chain_for_each, add dma_fence_chain_find_seqno, drop prev reference during garbage collection if it's not a chain fence. v3: use head and iterator for dma_fence_chain_for_each v4: fix reference count in dma_fence_chain_enable_signaling v5: fix iteration when walking each chain node Signed-off-by: Christian König --- drivers/dma-buf/Makefile | 3 +- drivers/dma-buf/dma-fence-chain.c | 241 ++ include/linux/dma-fence-chain.h | 81 ++ 3 files changed, 324 insertions(+), 1 deletion(-) create mode 100644 drivers/dma-buf/dma-fence-chain.c create mode 100644 include/linux/dma-fence-chain.h diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 0913a6ccab5a..1f006e083eb9 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,4 +1,5 @@ -obj-y := dma-buf.o dma-fence.o dma-fence-array.o reservation.o seqno-fence.o +obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ +reservation.o seqno-fence.o obj-$(CONFIG_SYNC_FILE)+= sync_file.o obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o obj-$(CONFIG_UDMABUF) += udmabuf.o diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c new file mode 100644 index ..0c5e3c902fa0 --- /dev/null +++ b/drivers/dma-buf/dma-fence-chain.c @@ -0,0 +1,241 @@ +/* + * fence-chain: chain fences together in a timeline + * + * Copyright (C) 2018 Advanced Micro Devices, Inc. + * Authors: + * Christian König + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include + +static bool dma_fence_chain_enable_signaling(struct dma_fence *fence); + +/** + * dma_fence_chain_get_prev - use RCU to get a reference to the previous fence + * @chain: chain node to get the previous node from + * + * Use dma_fence_get_rcu_safe to get a reference to the previous fence of the + * chain node. + */ +static struct dma_fence *dma_fence_chain_get_prev(struct dma_fence_chain *chain) +{ + struct dma_fence *prev; + + rcu_read_lock(); + prev = dma_fence_get_rcu_safe(>prev); + rcu_read_unlock(); + return prev; +} + +/** + * dma_fence_chain_walk - chain walking function + * @fence: current chain node + * + * Walk the chain to the next node. Returns the next fence or NULL if we are at + * the end of the chain. Garbage collects chain nodes which are already + * signaled. + */ +struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence) +{ + struct dma_fence_chain *chain, *prev_chain; + struct dma_fence *prev, *replacement, *tmp; + + chain = to_dma_fence_chain(fence); + if (!chain) { + dma_fence_put(fence); + return NULL; + } + + while ((prev = dma_fence_chain_get_prev(chain))) { + + prev_chain = to_dma_fence_chain(prev); + if (prev_chain) { + if (!dma_fence_is_signaled(prev_chain->fence)) + break; + + replacement = dma_fence_chain_get_prev(prev_chain); + } else { + if (!dma_fence_is_signaled(prev)) + break; + + replacement = NULL; + } + + tmp = cmpxchg(>prev, prev, replacement); + if (tmp == prev) + dma_fence_put(tmp); + else + dma_fence_put(replacement); + dma_fence_put(prev); + } + + dma_fence_put(fence); + return prev; +} +EXPORT_SYMBOL(dma_fence_chain_walk); + +/** + * dma_fence_chain_find_seqno - find fence chain node by seqno + * @pfence: pointer to the chain node where to start + * @seqno: the sequence number to search for + * + * Advance the fence pointer to the chain node which will signal this sequence + * number. If no sequence number is provided then this is a no-op. + * + * Returns EINVAL if the fence is not a chain node or the sequence number has + * not yet advanced far enough. + */ +int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) +{ + struct dma_fence_chain *chain; + + if (!seqno) + return 0; + + chain = to_dma_fence_chain(*pfence); + if (!chain || chain->base.seqno < seqno) + return -EINVAL; + + dma_fence_chain_for_each(*pfence, >base)
Re: [PATCH v3 41/50] drm/bridge: ti-tfp410: Report input bus config through bridge timings
On 15/03/2019 13:30, Peter Ujfalusi wrote: > > > On 28/02/2019 12.31, Tomi Valkeinen wrote: >> On 28/02/2019 12:27, Tomi Valkeinen wrote: >>> Hi Laurent, >>> >>> On 11/02/2019 11:46, Laurent Pinchart wrote: >>> + /* Get the sampling edge from the endpoint. */ + of_property_read_u32(ep, "pclk-sample", _sample); + of_node_put(ep); + + timings->input_bus_flags = DRM_BUS_FLAG_DE_HIGH; + + switch (pclk_sample) { + case 0: + timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE + | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE; + break; + case 1: + timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE + | DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE; + break; + default: + return -EINVAL; + } >>> >>> The default for pclk_sample is just the opposite of what omapdrm's >>> tfp410 used to do. The dts doc file also says that pclk-sample is >>> required, but the driver works fine without it, defaulting to 0. >>> >>> This means that none of the omap dts files with tfp410 work correctly, >>> instead they silently use the wrong settings which may work but easily >>> also won't... >>> >>> As the bus flags are added in this patch for the first time, maybe we >>> can assume that no one is using them, and the default could be made to >>> be the same as was on omapdrm's tfp410? >> >> Aaaand never mind. In omapdrm's driver we were using >> DRM_BUS_FLAG_SYNC_DRIVE_* variant, here we have SAMPLE variant. So it's >> fine =). > > If the pclk-sample is not defined in DT, it will default to 0 which > selects SAMPLE_NEGEDGE (== DRIVE_POSEDGE), right? > > But all the boards where I can find schematics with tfp410 have their > EDGE/HTPLG pin pulled up and according to the documentation when EDGE=1 > then tfp410 will sample on the rising edge. > > imho the pclk_sample should be initialized to 1 to avoid regression for > most of the boards using tfp410. Define "regression" =). If the omapdrm driver was always using DRIVE_POSEDGE, this driver should also be using DRIVE_POSEDGE, no? If it does the same as the old driver, it can't be a regression. This is, of course, only considering omapdrm based boards. That said, it sounds odd that this would be wrong in the old driver, but then again, it might well be, as code related to these sync signals has changed sooo many times, and the related DSS registers are somewhat confusing. That gave me an idea, I need to write an rwmem script to print the DSS sync flags in plain english... 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 v2 13/13] ARM: dts: sun9i: Add missing unit address
On Fri, Mar 15, 2019 at 06:22:44PM +0800, Chen-Yu Tsai wrote: > On Fri, Mar 15, 2019 at 5:16 PM Maxime Ripard > wrote: > > > > On Fri, Mar 15, 2019 at 05:09:22PM +0800, Chen-Yu Tsai wrote: > > > On Fri, Mar 15, 2019 at 5:02 PM Maxime Ripard > > > wrote: > > > > > > > > On Fri, Mar 15, 2019 at 10:39:24AM +0800, Chen-Yu Tsai wrote: > > > > > On Fri, Mar 15, 2019 at 4:16 AM Maxime Ripard > > > > > wrote: > > > > > > > > > > > > The soc node in the A80 DTSI has a ranges property, but no matching > > > > > > unit > > > > > > address, which results in a DTC warning. Add the unit address to > > > > > > remove > > > > > > that warning. > > > > > > > > > > > > Signed-off-by: Maxime Ripard > > > > > > --- > > > > > > arch/arm/boot/dts/sun9i-a80.dtsi | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi > > > > > > b/arch/arm/boot/dts/sun9i-a80.dtsi > > > > > > index 9b15f272e5f5..7a495c84ab65 100644 > > > > > > --- a/arch/arm/boot/dts/sun9i-a80.dtsi > > > > > > +++ b/arch/arm/boot/dts/sun9i-a80.dtsi > > > > > > @@ -289,7 +289,7 @@ > > > > > > status = "disabled"; > > > > > > }; > > > > > > > > > > > > - soc { > > > > > > + soc@2 { > > > > > > > > > > I thought we didn't like the soc node having an address? > > > > > > > > In general, yes, but in general we also don't have a ranges property. > > > > > > > > > Maybe we just bite the bullet and use 64-bit addresses and sizes for > > > > > the A80? > > > > > > > > I'd rather not, the current layout of the DT is pretty nice. > > > > > > > > But now I'm thinking, do you remember why we need to do that mapping > > > > in the first place? It's a 32bits SoCs, so why do we need to care > > > > about 64 bits addresses? > > > > > > It supports LPAE, addressing up to 8GB of RAM. Not that I've seen a > > > board sporting that much RAM though. Theobroma Systems might have > > > had such a board though, as their product page says "up to 8GB RAM". > > > > Ah, right. What should we do about this patch then? > > I'm OK with it I suppose. AFAICT only sysfs paths and overlays (which should > use labels instead) are affected. We can always revert it if there's any fallouts, but we did it already on a number of other SoCs (well, the opposite actually, dropping the unit address), and it wasn't an issue. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110130] ring gfx timeout when playing warcraft 3
https://bugs.freedesktop.org/show_bug.cgi?id=110130 Bug ID: 110130 Summary: ring gfx timeout when playing warcraft 3 Product: Mesa Version: git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: haa...@frickel.club QA Contact: dri-devel@lists.freedesktop.org Created attachment 143678 --> https://bugs.freedesktop.org/attachment.cgi?id=143678=edit kernel log Vega 64, Linux 5.0, mesa git 1ea42894c70, llvm git https://github.com/llvm-mirror/llvm/commit/d8706fcd747 Unfortunately hard to reproduce: Play custom maps in warcraft 3 (started with -opengl) in wine (4.1) for a few hours and maybe it'll happen. Interestingly amdgpu thinks kwin is the culprit: Mär 14 23:58:30 c-pc kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=5170371, emitted seq=5170373 Mär 14 23:58:30 c-pc kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process kwin_x11 pid 999 thread kwin_x11:cs0 pid 1396 I've also seen it say steam instead of kwin. Other games do not seem to trigger this. -- 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] drm/vkms: Solve bug on kms_crc_cursor tests
On 03/11, Ville Syrjälä wrote: > On Sun, Mar 10, 2019 at 05:35:05PM -0300, Rodrigo Siqueira wrote: > > On 03/01, Ville Syrjälä wrote: > > > On Fri, Mar 01, 2019 at 03:35:35PM -0300, Shayenne Moura wrote: > > > > Em sex, 1 de mar de 2019 às 12:26, Ville Syrjälä > > > > escreveu: > > > > > > > > > > On Fri, Mar 01, 2019 at 11:55:11AM -0300, Shayenne Moura wrote: > > > > > > Em qui, 28 de fev de 2019 às 11:03, Ville Syrjälä > > > > > > escreveu: > > > > > > > > > > > > > > On Thu, Feb 28, 2019 at 11:11:07AM +0100, Daniel Vetter wrote: > > > > > > > > On Mon, Feb 25, 2019 at 11:26:06AM -0300, Shayenne Moura wrote: > > > > > > > > > vkms_crc_work_handle needs the value of the actual frame to > > > > > > > > > schedule the workqueue that calls periodically the vblank > > > > > > > > > handler and the destroy state functions. However, the frame > > > > > > > > > value returned from vkms_vblank_simulate is updated and > > > > > > > > > diminished in vblank_get_timestamp because it is not in a > > > > > > > > > vblank interrupt, and return an inaccurate value. > > > > > > > > > > > > > > > > > > Solve this getting the actual vblank frame directly from the > > > > > > > > > vblank->count inside the `struct drm_crtc`, instead of using > > > > > > > > > the `drm_accurate_vblank_count` function. > > > > > > > > > > > > > > > > > > Signed-off-by: Shayenne Moura > > > > > > > > > > > > > > > > Sorry for the delay, I'm a bit swamped right now :-/ > > > > > > > > > > > > > > > > Debug work you're doing here is really impressive! But I have > > > > > > > > no idea > > > > > > > > what's going on. It doesn't look like it's just papering over a > > > > > > > > bug (like > > > > > > > > the in_vblank_irq check we've discussed on irc), but I also > > > > > > > > have no idea > > > > > > > > why it works. > > > > > > > > > > > > > > > > I'll pull in Ville, he understands this better than me. > > > > > > > > > > > > > > It's not entirely clear what we're trying to fix. From what I can > > > > > > > see > > > > > > > the crc work seems to be in no way synchronized with page flips, > > > > > > > so > > > > > > > I'm not sure how all this is really supposed to work. > > > > > > > > > > > > > > > > > > > Hi, Ville! > > > > > > > > > > > > Thank you for the review! :) > > > > > > > > > > > > I do not understand well what crc code is doing, but the issue that > > > > > > I found > > > > > > is related to the vblank timestamp and frame count. > > > > > > > > > > > > When vkms handles the crc_cursor it uses the start frame and end > > > > > > frame > > > > > > values to verify if it needs to call the function > > > > > > 'drm_crtc_add_crc_entry()' > > > > > > for each frame. > > > > > > > > > > > > However, when getting the frame count, the code is calling the > > > > > > function > > > > > > drm_update_vblank_count(dev, pipe, false) and, because of the > > > > > > 'false', > > > > > > subtracting the actual vblank timestamp (consequently, the frame > > > > > > count > > > > > > value), causing conflicts. > > > > > > > > > > The in_vblank_irq behavour looks sane to me. What are these conflicts? > > > > > > > > > > > > > The entire history was: > > > > - I sent the patch with bugfix for vblank extra frame. The patch > > > > changed > > > >our function vkms_get_vblank_timestamp() to look like this: > > > > > > > > bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int > > > > pipe, > > > >int *max_error, ktime_t *vblank_time, > > > >bool in_vblank_irq) > > > > { > > > > struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev); > > > > struct vkms_output *output = >output; > > > > > > > > *vblank_time = output->vblank_hrtimer.node.expires; > > > > > > > > + if (!in_vblank_irq) > > > > +*vblank_time -= output->period_ns; > > > > > > > > return true; > > > > } > > > > > > > > - This patch solve the issue that I was looking for (extra vblank > > > > frames on kms_flip). > > > > > > > > - However, kms_cursor_crc tests, which passed before my patch, started > > > > to fail. > > > > > > > > - Debugging them, I found that the problem was inside of function > > > > `vkms_vblank_simulate()` > > > > when it was handling the crc_enabled (inside if (state && > > > > output->crc_enabled)) > > > > and inside the function vkms_crc_work_handle() too. > > > > > > > > - Following the steps: > > > > 1. Inside vkms_vblank_simulate() we call > > > > drm_crtc_accurate_vblank_count() > > > > 2. In its turn, drm_crtc_accurate_vblank_count() calls the function > > > >drm_update_vblank_count(dev, pipe, false). /* This false is default > > > > */ > > > > 3. Finally, the “false” used in drm_update_vblank_count(), will be > > > > passed to vkms_get_vblank_timestamp() and the condition “if > > > > (!in_vblank_irq)” will be executed multiple times (we don’t want it). > > > > > > > > - Inside vkms_crc, the issue is that the returned frame
[RFC PATCH 1/5] drm: rcar-du: Link CRTCs to the DU device
The rcar_du_crtc functions have a heavy reliance on the rcar_du_group structure, in many cases just to access the DU device context. To better separate the groups out of the CRTC handling code, give the rcar_du_crtc its own pointer to the device and remove the indirection through the group pointers. Signed-off-by: Kieran Bingham --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 48 +- drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 ++ drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 57fd5c00494b..471ce464654a 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -32,21 +32,21 @@ static u32 rcar_du_crtc_read(struct rcar_du_crtc *rcrtc, u32 reg) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; return rcar_du_read(rcdu, rcrtc->mmio_offset + reg); } static void rcar_du_crtc_write(struct rcar_du_crtc *rcrtc, u32 reg, u32 data) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; rcar_du_write(rcdu, rcrtc->mmio_offset + reg, data); } static void rcar_du_crtc_clr(struct rcar_du_crtc *rcrtc, u32 reg, u32 clr) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; rcar_du_write(rcdu, rcrtc->mmio_offset + reg, rcar_du_read(rcdu, rcrtc->mmio_offset + reg) & ~clr); @@ -54,7 +54,7 @@ static void rcar_du_crtc_clr(struct rcar_du_crtc *rcrtc, u32 reg, u32 clr) static void rcar_du_crtc_set(struct rcar_du_crtc *rcrtc, u32 reg, u32 set) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; rcar_du_write(rcdu, rcrtc->mmio_offset + reg, rcar_du_read(rcdu, rcrtc->mmio_offset + reg) | set); @@ -62,7 +62,7 @@ static void rcar_du_crtc_set(struct rcar_du_crtc *rcrtc, u32 reg, u32 set) void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 set) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; rcrtc->dsysr = (rcrtc->dsysr & ~clr) | set; rcar_du_write(rcdu, rcrtc->mmio_offset + DSYSR, rcrtc->dsysr); @@ -157,10 +157,9 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc *rcrtc, } done: - dev_dbg(rcrtc->group->dev->dev, + dev_dbg(rcrtc->dev->dev, "output:%u, fdpll:%u, n:%u, m:%u, diff:%lu\n", -dpll->output, dpll->fdpll, dpll->n, dpll->m, -best_diff); +dpll->output, dpll->fdpll, dpll->n, dpll->m, best_diff); } struct du_clk_params { @@ -212,7 +211,7 @@ static const struct soc_device_attribute rcar_du_r8a7795_es1[] = { static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) { const struct drm_display_mode *mode = >crtc.state->adjusted_mode; - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; unsigned long mode_clock = mode->clock * 1000; u32 dsmr; u32 escr; @@ -277,7 +276,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) rcar_du_escr_divider(rcrtc->extclock, mode_clock, ESCR_DCLKSEL_DCLKIN, ); - dev_dbg(rcrtc->group->dev->dev, "mode clock %lu %s rate %lu\n", + dev_dbg(rcrtc->dev->dev, "mode clock %lu %s rate %lu\n", mode_clock, params.clk == rcrtc->clock ? "cpg" : "ext", params.rate); @@ -285,7 +284,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) escr = params.escr; } - dev_dbg(rcrtc->group->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr); + dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr); rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr); rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0); @@ -333,7 +332,7 @@ plane_format(struct rcar_du_plane *plane) static void rcar_du_crtc_update_planes(struct rcar_du_crtc *rcrtc) { struct rcar_du_plane *planes[RCAR_DU_NUM_HW_PLANES]; - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; unsigned int num_planes = 0; unsigned int dptsr_planes; unsigned int hwplanes = 0; @@ -463,7 +462,7 @@ static bool rcar_du_crtc_page_flip_pending(struct rcar_du_crtc *rcrtc) static void rcar_du_crtc_wait_page_flip(struct rcar_du_crtc *rcrtc) { - struct rcar_du_device *rcdu = rcrtc->group->dev; + struct rcar_du_device *rcdu = rcrtc->dev; if
[RFC PATCH 4/5] drm: rcar-du: add group hooks for atomic-commit
The group can now be handled independently from the CRTC tracking its own state. Introduce an rcar_du_group_atomic_check() call which will iterate the CRTCs to determine the per-state use-count of the group. This use count then allows us to determine if the group should be configured or disabled in our commit tail through the introduction of two new calls rcar_du_group_atomic_{enter,exit}_standby(). The existing rcar_du_group_{get,put}() functions are now redundant and removed along with their interactions in the CRTC get/put calls. The group takes the clock of the first CRTC in the group. Signed-off-by: Kieran Bingham --- I'm already looking to refactor the series and put the CRTC handlng first. This will mean that the CRTC will power up the group instead, and the 'stealing' of the CRTC clock (rgrp->clock = rcdu->crtcs[rgrp->index * 2].clock;) can be removed drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 8 -- drivers/gpu/drm/rcar-du/rcar_du_group.c | 122 +--- drivers/gpu/drm/rcar-du/rcar_du_group.h | 16 +++- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 13 +++ 4 files changed, 115 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 471ce464654a..8d35b9e987f1 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -518,17 +518,11 @@ static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) if (ret < 0) goto error_clock; - ret = rcar_du_group_get(rcrtc->group); - if (ret < 0) - goto error_group; - rcar_du_crtc_setup(rcrtc); rcrtc->initialized = true; return 0; -error_group: - clk_disable_unprepare(rcrtc->extclock); error_clock: clk_disable_unprepare(rcrtc->clock); return ret; @@ -536,8 +530,6 @@ static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) static void rcar_du_crtc_put(struct rcar_du_crtc *rcrtc) { - rcar_du_group_put(rcrtc->group); - clk_disable_unprepare(rcrtc->extclock); clk_disable_unprepare(rcrtc->clock); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c index 9c82d666f170..c31b968c1134 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -24,6 +24,7 @@ */ #include +#include #include #include @@ -172,38 +173,6 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp) mutex_unlock(>lock); } -/* - * rcar_du_group_get - Acquire a reference to the DU channels group - * - * Acquiring the first reference setups core registers. A reference must be held - * before accessing any hardware registers. - * - * This function must be called with the DRM mode_config lock held. - * - * Return 0 in case of success or a negative error code otherwise. - */ -int rcar_du_group_get(struct rcar_du_group *rgrp) -{ - if (rgrp->use_count) - goto done; - - rcar_du_group_setup(rgrp); - -done: - rgrp->use_count++; - return 0; -} - -/* - * rcar_du_group_put - Release a reference to the DU - * - * This function must be called with the DRM mode_config lock held. - */ -void rcar_du_group_put(struct rcar_du_group *rgrp) -{ - --rgrp->use_count; -} - static void __rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start) { struct rcar_du_device *rcdu = rgrp->dev; @@ -384,6 +353,95 @@ static const struct drm_private_state_funcs rcar_du_group_state_funcs = { .atomic_destroy_state = rcar_du_group_atomic_destroy_state, }; +#define for_each_oldnew_group_in_state(__state, __obj, __old_state, __new_state, __i) \ + for_each_oldnew_private_obj_in_state((__state), (__obj), (__old_state), (__new_state), (__i)) \ + for_each_if((__obj)->funcs == _du_group_state_funcs) + +static struct rcar_du_group_state * +rcar_du_get_group_state(struct drm_atomic_state *state, + struct rcar_du_group *rgrp) +{ + struct drm_private_state *pstate; + + pstate = drm_atomic_get_private_obj_state(state, >private); + if (IS_ERR(pstate)) + return ERR_CAST(pstate); + + return to_rcar_group_state(pstate); +} + +int rcar_du_group_atomic_check(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + struct rcar_du_group_state *rstate; + + if (crtc_state->active_changed || crtc_state->mode_changed) { + rstate = rcar_du_get_group_state(state, rcrtc->group); + if (IS_ERR(rstate)) + return PTR_ERR(rstate); + + if (crtc_state->active) +
[RFC PATCH 2/5] drm: rcar-du: Provide for_each_group helper
Refactoring of the group control code will soon require more iteration over the available groups. Simplify this process by introducing a group iteration helper. Signed-off-by: Kieran Bingham --- I'm not yet sure if the second patch which utilises this helper will make it to the final cut. Will this change still be warrented on it's own, if there is only a solitary user of for_each_rcdu_group? drivers/gpu/drm/rcar-du/rcar_du_drv.h | 5 + drivers/gpu/drm/rcar-du/rcar_du_kms.c | 10 ++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 1327cd0df90a..1e9dd494e8ac 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h @@ -96,6 +96,11 @@ struct rcar_du_device { unsigned int vspd1_sink; }; +#define for_each_rcdu_group(__rcdu, __group, __i) \ + for ((__i) = 0; (__group = &(rcdu)->groups[__i]), \ +(__i) < DIV_ROUND_UP((rcdu)->num_crtcs, 2); \ +__i++) + static inline bool rcar_du_has(struct rcar_du_device *rcdu, unsigned int feature) { diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 3b7d50a8fb9b..83cd07a404eb 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -516,9 +516,9 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) struct drm_device *dev = rcdu->ddev; struct drm_encoder *encoder; + struct rcar_du_group *rgrp; unsigned int dpad0_sources; unsigned int num_encoders; - unsigned int num_groups; unsigned int swindex; unsigned int hwindex; unsigned int i; @@ -559,11 +559,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) return ret; /* Initialize the groups. */ - num_groups = DIV_ROUND_UP(rcdu->num_crtcs, 2); - - for (i = 0; i < num_groups; ++i) { - struct rcar_du_group *rgrp = >groups[i]; - + for_each_rcdu_group(rcdu, rgrp, i) { mutex_init(>lock); rgrp->dev = rcdu; @@ -600,8 +596,6 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) /* Create the CRTCs. */ for (swindex = 0, hwindex = 0; swindex < rcdu->num_crtcs; ++hwindex) { - struct rcar_du_group *rgrp; - /* Skip unpopulated DU channels. */ if (!(rcdu->info->channels_mask & BIT(hwindex))) continue; -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 5/5] drm: rcar-du: Add crtc standby helpers
Provide helpers to manage the power state, and initial configuration of the CRTC to match the group implementation. rcar_du_crtc_get() and rcar_du_crtc_get() are no longer used, and are removed, simplifying the implementation and removing the initialized flag which was needed to track the state of the CRTC. Signed-off-by: Kieran Bingham --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 94 ++ drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 7 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 + 3 files changed, 57 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 8d35b9e987f1..f3145c24070d 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -478,8 +478,18 @@ static void rcar_du_crtc_wait_page_flip(struct rcar_du_crtc *rcrtc) * Start/Stop and Suspend/Resume */ -static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) +static int rcar_du_crtc_enable(struct rcar_du_crtc *rcrtc) { + int ret; + + ret = clk_prepare_enable(rcrtc->clock); + if (ret < 0) + return ret; + + ret = clk_prepare_enable(rcrtc->extclock); + if (ret < 0) + goto error_clock; + /* Set display off and background to black */ rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0)); rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0)); @@ -497,29 +507,6 @@ static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) /* Turn vertical blanking interrupt reporting on. */ drm_crtc_vblank_on(>crtc); -} - -static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) -{ - int ret; - - /* -* Guard against double-get, as the function is called from both the -* .atomic_enable() and .atomic_begin() handlers. -*/ - if (rcrtc->initialized) - return 0; - - ret = clk_prepare_enable(rcrtc->clock); - if (ret < 0) - return ret; - - ret = clk_prepare_enable(rcrtc->extclock); - if (ret < 0) - goto error_clock; - - rcar_du_crtc_setup(rcrtc); - rcrtc->initialized = true; return 0; @@ -528,12 +515,10 @@ static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) return ret; } -static void rcar_du_crtc_put(struct rcar_du_crtc *rcrtc) +static void rcar_du_crtc_disable(struct rcar_du_crtc *rcrtc) { clk_disable_unprepare(rcrtc->extclock); clk_disable_unprepare(rcrtc->clock); - - rcrtc->initialized = false; } static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc) @@ -629,6 +614,44 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc) * CRTC Functions */ +int rcar_du_crtc_atomic_exit_standby(struct drm_device *dev, +struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + + if (crtc_state->active_changed && crtc_state->active) { + int ret = rcar_du_crtc_enable(rcrtc); + + if (ret) + return ret; + } + } + + return 0; +} + +int rcar_du_crtc_atomic_enter_standby(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + + if (crtc_state->active_changed && !crtc_state->active) + rcar_du_crtc_disable(rcrtc); + } + + return 0; +} + static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *state) { @@ -654,8 +677,6 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc, struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(crtc->state); struct rcar_du_device *rcdu = rcrtc->dev; - rcar_du_crtc_get(rcrtc); - /* * On D3/E3 the dot clock is provided by the LVDS encoder attached to * the DU channel. We need to enable its clock output explicitly if @@ -683,7 +704,6 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc, struct rcar_du_device *rcdu = rcrtc->dev; rcar_du_crtc_stop(rcrtc); - rcar_du_crtc_put(rcrtc); if (rcdu->info->lvds_clk_mask & BIT(rcrtc->index) && rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0)) { @@ -712,20 +732,6 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc, WARN_ON(!crtc->state->enable); - /* -* If a mode set is in progress we can be called with the CRTC disabled. -
[RFC PATCH 3/5] drm: rcar-du: Create a group state object
Create a new private state object for the DU groups, and move the initialisation of a group object to a new function rcar_du_group_init(). Signed-off-by: Kieran Bingham --- I'll be extending the data that goes into this private structure. Have I got the creation and handling of the private object correct? drivers/gpu/drm/rcar-du/rcar_du_group.c | 81 + drivers/gpu/drm/rcar-du/rcar_du_group.h | 22 +++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 27 ++--- 3 files changed, 107 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c index 9eee47969e77..9c82d666f170 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -26,6 +26,10 @@ #include #include +#include +#include +#include + #include "rcar_du_drv.h" #include "rcar_du_group.h" #include "rcar_du_regs.h" @@ -351,3 +355,80 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp) return rcar_du_set_dpad0_vsp1_routing(rgrp->dev); } + +static struct drm_private_state * +rcar_du_group_atomic_duplicate_state(struct drm_private_obj *obj) +{ + struct rcar_du_group_state *state; + + if (WARN_ON(!obj->state)) + return NULL; + + state = kzalloc(sizeof(*state), GFP_KERNEL); + if (state == NULL) + return NULL; + + __drm_atomic_helper_private_obj_duplicate_state(obj, >state); + + return >state; +} + +static void rcar_du_group_atomic_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + kfree(to_rcar_group_state(state)); +} + +static const struct drm_private_state_funcs rcar_du_group_state_funcs = { + .atomic_duplicate_state = rcar_du_group_atomic_duplicate_state, + .atomic_destroy_state = rcar_du_group_atomic_destroy_state, +}; + +/* + * rcar_du_group_init - Initialise and reset a group object + * + * Return 0 in case of success or a negative error code otherwise. + */ +int rcar_du_group_init(struct rcar_du_device *rcdu, struct rcar_du_group *rgrp, + unsigned int index) +{ + static const unsigned int mmio_offsets[] = { + DU0_REG_OFFSET, DU2_REG_OFFSET + }; + + struct rcar_du_group_state *state; + + state = kzalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return -ENOMEM; + + drm_atomic_private_obj_init(rcdu->ddev, >private, >state, + _du_group_state_funcs); + + mutex_init(>lock); + + rgrp->dev = rcdu; + rgrp->mmio_offset = mmio_offsets[index]; + rgrp->index = index; + /* Extract the channel mask for this group only. */ + rgrp->channels_mask = (rcdu->info->channels_mask >> (2 * index)) + & GENMASK(1, 0); + rgrp->num_crtcs = hweight8(rgrp->channels_mask); + + /* +* If we have more than one CRTC in this group pre-associate +* the low-order planes with CRTC 0 and the high-order planes +* with CRTC 1 to minimize flicker occurring when the +* association is changed. +*/ + rgrp->dptsr_planes = rgrp->num_crtcs > 1 + ? (rcdu->info->gen >= 3 ? 0x04 : 0xf0) + : 0; + + return 0; +} + +void rcar_du_group_cleanup(struct rcar_du_group *rgrp) +{ + drm_atomic_private_obj_fini(>private); +} diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h index 87950c1f6a52..4b812e167987 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h @@ -12,12 +12,15 @@ #include +#include + #include "rcar_du_plane.h" struct rcar_du_device; /* * struct rcar_du_group - CRTCs and planes group + * @private: The base drm private object * @dev: the DU device * @mmio_offset: registers offset in the device memory map * @index: group index @@ -32,6 +35,8 @@ struct rcar_du_device; * @need_restart: the group needs to be restarted due to a configuration change */ struct rcar_du_group { + struct drm_private_obj private; + struct rcar_du_device *dev; unsigned int mmio_offset; unsigned int index; @@ -49,6 +54,19 @@ struct rcar_du_group { bool need_restart; }; +#define to_rcar_group(s) container_of(s, struct rcar_du_group, private) + +/** + * struct rcar_du_group_state - Driver-specific group state + * @state: base DRM private state + */ +struct rcar_du_group_state { + struct drm_private_state state; +}; + +#define to_rcar_group_state(s) \ + container_of(s, struct rcar_du_group_state, state) + u32 rcar_du_group_read(struct rcar_du_group *rgrp, u32 reg); void rcar_du_group_write(struct rcar_du_group *rgrp, u32 reg, u32 data); @@ -60,4 +78,8 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp); int
[RFC PATCH 0/5] drm: rcar-du: Rework CRTC and groups for atomic commits
An early iteration for refactoring the RCAR-DU so that the group and CRTC state is only changed during rcar_du_atomic_commit_tail. Currnently, various parts of the group and crtc are adjusted and state is stored globaly. Introduce a private object state for the groups so that we can track per-state group-state, and start to utilise that to refactor so that all hardware operations can occur during rcar_du_atomic_commit_tail(). This is only RFC state at the moment, and I am actively working on this code. Kieran Bingham (5): drm: rcar-du: Link CRTCs to the DU device drm: rcar-du: Provide for_each_group helper drm: rcar-du: Create a group state object drm: rcar-du: add group hooks for atomic-commit drm: rcar-du: Add crtc standby helpers drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 150 + drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 9 +- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 5 + drivers/gpu/drm/rcar-du/rcar_du_group.c | 203 drivers/gpu/drm/rcar-du/rcar_du_group.h | 38 - drivers/gpu/drm/rcar-du/rcar_du_kms.c | 52 +++--- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +- 7 files changed, 313 insertions(+), 146 deletions(-) -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 41/50] drm/bridge: ti-tfp410: Report input bus config through bridge timings
On 28/02/2019 12.31, Tomi Valkeinen wrote: > On 28/02/2019 12:27, Tomi Valkeinen wrote: >> Hi Laurent, >> >> On 11/02/2019 11:46, Laurent Pinchart wrote: >> >>> + /* Get the sampling edge from the endpoint. */ >>> + of_property_read_u32(ep, "pclk-sample", _sample); >>> + of_node_put(ep); >>> + >>> + timings->input_bus_flags = DRM_BUS_FLAG_DE_HIGH; >>> + >>> + switch (pclk_sample) { >>> + case 0: >>> + timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE >>> +| DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE; >>> + break; >>> + case 1: >>> + timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE >>> +| DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE; >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >> >> The default for pclk_sample is just the opposite of what omapdrm's >> tfp410 used to do. The dts doc file also says that pclk-sample is >> required, but the driver works fine without it, defaulting to 0. >> >> This means that none of the omap dts files with tfp410 work correctly, >> instead they silently use the wrong settings which may work but easily >> also won't... >> >> As the bus flags are added in this patch for the first time, maybe we >> can assume that no one is using them, and the default could be made to >> be the same as was on omapdrm's tfp410? > > Aaaand never mind. In omapdrm's driver we were using > DRM_BUS_FLAG_SYNC_DRIVE_* variant, here we have SAMPLE variant. So it's > fine =). If the pclk-sample is not defined in DT, it will default to 0 which selects SAMPLE_NEGEDGE (== DRIVE_POSEDGE), right? But all the boards where I can find schematics with tfp410 have their EDGE/HTPLG pin pulled up and according to the documentation when EDGE=1 then tfp410 will sample on the rising edge. imho the pclk_sample should be initialized to 1 to avoid regression for most of the boards using tfp410. > > Sorry for the noise. > > Tomi > - Péter 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 v2 1/5] drm/rockchip: fix fb references in async update
On 2019-03-15 11:25 a.m., Boris Brezillon wrote: > On Fri, 15 Mar 2019 11:11:36 +0100 > Michel Dänzer wrote: > >> On 2019-03-14 6:51 p.m., Helen Koike wrote: >>> On 3/14/19 6:15 AM, Michel Dänzer wrote: On 2019-03-13 7:08 p.m., Helen Koike wrote: > On 3/13/19 6:58 AM, Michel Dänzer wrote: >> On 2019-03-13 4:42 a.m., Tomasz Figa wrote: >>> On Wed, Mar 13, 2019 at 12:52 AM Boris Brezillon >>> wrote: On Tue, 12 Mar 2019 12:34:45 -0300 Helen Koike wrote: > On 3/12/19 3:34 AM, Boris Brezillon wrote: >> On Mon, 11 Mar 2019 23:21:59 -0300 >> Helen Koike wrote: >> >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> @@ -912,30 +912,31 @@ static void >>> vop_plane_atomic_async_update(struct drm_plane *plane, >>> struct drm_plane_state >>> *new_state) >>> { >>>struct vop *vop = to_vop(plane->state->crtc); >>> - struct drm_plane_state *plane_state; >>> + struct drm_framebuffer *old_fb = plane->state->fb; >>> >>> - plane_state = plane->funcs->atomic_duplicate_state(plane); >>> - plane_state->crtc_x = new_state->crtc_x; >>> - plane_state->crtc_y = new_state->crtc_y; >>> - plane_state->crtc_h = new_state->crtc_h; >>> - plane_state->crtc_w = new_state->crtc_w; >>> - plane_state->src_x = new_state->src_x; >>> - plane_state->src_y = new_state->src_y; >>> - plane_state->src_h = new_state->src_h; >>> - plane_state->src_w = new_state->src_w; >>> - >>> - if (plane_state->fb != new_state->fb) >>> - drm_atomic_set_fb_for_plane(plane_state, new_state->fb); >>> - >>> - swap(plane_state, plane->state); >>> - >>> - if (plane->state->fb && plane->state->fb != new_state->fb) { >>> + /* >>> + * A scanout can still be occurring, so we can't drop the >>> reference to >>> + * the old framebuffer. To solve this we get a reference to >>> old_fb and >>> + * set a worker to release it later. >> >> Hm, doesn't look like an async update to me if we have to wait for >> the >> next VBLANK to happen to get the new content on the screen. Maybe we >> should reject async updates when old_fb != new_fb in the rk >> ->async_check() hook. > > Unless I am misunderstanding this, we don't wait here, we just grab a > reference to the fb in case it is being still used by the hw, so it > doesn't get released prematurely. I was just reacting to the comment that says the new FB should stay around until the next VBLANK event happens. If the FB must stay around that probably means the HW is still using, which made me wonder if this HW actually supports async update (where async means "update now and don't care about about tearing"). Or maybe it takes some time to switch to the new FB and waiting for the next VBLANK to release the old FB was an easy solution to not wait for the flip to actually happen in ->async_update() (which is kind of a combination of async+non-blocking). >>> >>> The hardware switches framebuffers on vblank, so whatever framebuffer >>> is currently being scanned out from needs to stay there until the >>> hardware switches to the new one in shadow registers. If that doesn't >>> happen, you get IOMMU faults and the display controller stops working >>> since we don't have any fault handling currently, just printing a >>> message. >> >> Sounds like your hardware doesn't actually support async flips. It's >> probably better for the driver not to pretend otherwise. > > I think wee need to clarify the meaning of the async_update callback > (and we should clarify it in the docs). > > The way I understand what the async_update callback should do is: don't > block (i.e. don't wait for the next vblank), Note that those are two separate things. "Async flips" are about "don't wait for vblank", not about "don't block". > and update the hw state at some point with the latest state from the > last call to async_update. > > Which means that: any driver can implement the async_update callback, > independently if it supports changing its state right away or not. > If hw supports, async_update can change the hw state right away, if not, > then changes will be applied in the next vblank (it can even amend the > pending commit if there is one). > With this, we can remove all the legacy cursor code to use the > async_update callback, since async_update can be called 100
Re: [PATCH 1/2] drm/udl: use drm_gem_object_put_unlocked.
On Fri, Mar 15, 2019 at 11:46:20AM +1000, Dave Airlie wrote: > From: Dave Airlie > > When Daniel removed struct_mutex he didn't fix this call to the unlocked > variant which is required since we no longer use struct mutex. > > This fixes a bunch of: > WARNING: CPU: 4 PID: 1370 at drivers/gpu/drm/drm_gem.c:931 > drm_gem_object_put+0x2b/0x30 [drm] > Modules linked in: udl xt_CHECKSUM ipt_MASQUERADE tun bridge stp llc > nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t> > CPU: 4 PID: 1370 Comm: Xorg Not tainted 5.0.0+ #2 > > backtraces when you plug in a udl device. > > Fixes: ae358dacd217 (drm/udl: Get rid of dev->struct_mutex usage) > Cc: Daniel Vetter > Cc: Sean Paul > Signed-off-by: Dave Airlie Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/udl/udl_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c > index d5a23295dd80..bb7b58407039 100644 > --- a/drivers/gpu/drm/udl/udl_gem.c > +++ b/drivers/gpu/drm/udl/udl_gem.c > @@ -224,7 +224,7 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device > *dev, > *offset = drm_vma_node_offset_addr(>base.vma_node); > > out: > - drm_gem_object_put(>base); > + drm_gem_object_put_unlocked(>base); > unlock: > mutex_unlock(>gem_lock); > return ret; > -- > 2.20.1 > -- 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
[GIT PULL] fbdev changes for v5.1
Hi Linus, Please pull fbdev changes for v5.1. Just a couple of small fixes and cleanups (please see the signed tag description for details). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics The following changes since commit 8834f5600cf3c8db365e18a3d5cac2c2780c81e5: Linux 5.0-rc5 (2019-02-03 13:48:04 -0800) are available in the git repository at: https://github.com/bzolnier/linux.git tags/fbdev-v5.1 for you to fetch changes up to 9a9f1d1a81a972513636c333e26c542f8aebae55: fbdev: mbx: fix a misspelled variable name (2019-03-05 12:36:33 +0100) fbdev changes for v5.1: - fix memory access if logo is bigger than the screen (Manfred Schlaegl) - silence fbcon logo on 'quiet' boots (Prarit Bhargava) - use kvmalloc() for scrollback buffer in fbcon (Konstantin Khorenko) - misc fixes (Colin Ian King, YueHaibing, Matteo Croce, Mathieu Malaterre, Anders Roxell, Arnd Bergmann) - misc cleanups (Rob Herring, Lubomir Rintel, Greg Kroah-Hartman, Jani Nikula, Michal Vokáč) Anders Roxell (1): fbdev: omap2: fix warnings in dss core Arnd Bergmann (1): fbdev: mbx: fix a misspelled variable name Bartlomiej Zolnierkiewicz (1): Merge tag 'v5.0-rc5' of https://git.kernel.org/.../torvalds/linux into fbdev-for-next Colin Ian King (1): fbdev/via: fix spelling mistake "Expandsion" -> "Expansion" Greg Kroah-Hartman (2): fbdev: omap2: no need to check return value of debugfs_create functions fbdev: mbx: fix up debugfs file creation Jani Nikula (1): video/fbdev: refactor video= cmdline parsing Konstantin Khorenko (1): fbcon: use kvmalloc() for scrollback buffer Lubomir Rintel (1): video: fbdev: geode: remove ifdef OLPC noise Manfred Schlaegl (1): fbdev: fbmem: fix memory access if logo is bigger than the screen Mathieu Malaterre (1): video: offb: annotate implicit fall throughs Matteo Croce (1): omapfb: fix typo Michal Vokáč (3): dt-bindings: display: ssd1307fb: Remove reset-active-low from examples video: ssd1307fb: Do not hard code active-low reset sequence ARM: dts: imx28-cfa10036: Fix the reset gpio signal polarity Prarit Bhargava (2): printk: Export console_printk fbcon: Silence fbcon logo on 'quiet' boots Rob Herring (1): fbdev: Use of_node_name_eq for node name comparisons YueHaibing (2): fbdev: chipsfb: remove set but not used variable 'size' video: fbdev: Fix potential NULL pointer dereference .../devicetree/bindings/display/ssd1307fb.txt | 2 -- arch/arm/boot/dts/imx28-cfa10036.dts | 3 +- drivers/video/fbdev/aty/radeon_pm.c| 6 ++-- drivers/video/fbdev/cg14.c | 4 +-- drivers/video/fbdev/cg3.c | 2 +- drivers/video/fbdev/chipsfb.c | 3 +- drivers/video/fbdev/core/fb_cmdline.c | 23 ++--- drivers/video/fbdev/core/fbcon.c | 14 +--- drivers/video/fbdev/core/fbmem.c | 3 ++ drivers/video/fbdev/core/fbmon.c | 2 ++ drivers/video/fbdev/ffb.c | 2 +- drivers/video/fbdev/geode/gxfb_core.c | 13 ++- drivers/video/fbdev/geode/lxfb_core.c | 13 ++- drivers/video/fbdev/imsttfb.c | 4 +-- drivers/video/fbdev/mbx/mbxdebugfs.c | 40 +++--- drivers/video/fbdev/mbx/mbxfb.c| 2 +- drivers/video/fbdev/offb.c | 4 ++- drivers/video/fbdev/omap2/omapfb/dss/core.c| 34 -- drivers/video/fbdev/omap2/omapfb/dss/dss-of.c | 4 +-- drivers/video/fbdev/omap2/omapfb/dss/dss.h | 2 +- drivers/video/fbdev/omap2/omapfb/dss/hdmi4_core.c | 2 +- drivers/video/fbdev/ssd1307fb.c| 4 +-- drivers/video/fbdev/via/viafbdev.c | 2 +- kernel/printk/printk.c | 1 + 24 files changed, 72 insertions(+), 117 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v5 09/13] drm/i915: Add HLG EOTF
On 3/11/2019 9:28 AM, Uma Shankar wrote: From: Ville Syrjälä ADD HLG EOTF to the list of EOTF transfer functions supported. Hybrid Log-Gamma (HLG) is a high dynamic range (HDR) standard. HLG defines a nonlinear transfer function in which the lower half of the signal values use a gamma curve and the upper half of the signal values use a logarithmic curve. v2: Rebase v3: Fixed a warning message v4: Addressed Shashank's review comments Signed-off-by: Ville Syrjälä Signed-off-by: Uma Shankar --- drivers/gpu/drm/drm_edid.c | 4 ++-- include/linux/hdmi.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 49f8340..4cd22e8 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3864,8 +3864,8 @@ static uint8_t eotf_supported(const u8 *edid_ext) return edid_ext[2] & (BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) | BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) | -BIT(HDMI_EOTF_SMPTE_ST2084)); - +BIT(HDMI_EOTF_SMPTE_ST2084) | +BIT(HDMI_EOTF_BT_2100_HLG)); } static uint8_t hdr_metadata_type(const u8 *edid_ext) diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index 33243b2..ad652e6 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -161,6 +161,7 @@ enum hdmi_eotf { HDMI_EOTF_TRADITIONAL_GAMMA_SDR, HDMI_EOTF_TRADITIONAL_GAMMA_HDR, HDMI_EOTF_SMPTE_ST2084, + HDMI_EOTF_BT_2100_HLG, }; Looks good to me, Feel free to use: Reviewed-by: Shashank Sharma struct hdmi_avi_infoframe { ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v5 07/13] drm/i915: Write HDR infoframe and send to panel
On 3/11/2019 9:27 AM, Uma Shankar wrote: Enable writing of HDR metadata infoframe to panel. The data will be provid by usersapace compositors, based on blending policies and passsed to driver through a blob property. v2: Rebase v3: Fixed a warning message v4: Addressed Shashank's review comments v5: Rebase. Added infoframe calculation in compute config. Signed-off-by: Uma Shankar --- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_hdmi.c | 33 + 2 files changed, 34 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 58483f8..a5ee124 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1037,6 +1037,7 @@ struct intel_crtc_state { union hdmi_infoframe avi; union hdmi_infoframe spd; union hdmi_infoframe hdmi; + union hdmi_infoframe drm; } infoframes; /* HDMI scrambling status */ diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 0952475..ea7afa0 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -555,6 +555,7 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder, HDMI_INFOFRAME_TYPE_AVI, HDMI_INFOFRAME_TYPE_SPD, HDMI_INFOFRAME_TYPE_VENDOR, + HDMI_INFOFRAME_TYPE_DRM, }; u32 intel_hdmi_infoframe_enable(unsigned int type) @@ -777,6 +778,30 @@ void intel_read_infoframe(struct intel_encoder *encoder, return true; } +static bool +intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder, + struct intel_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct hdmi_drm_infoframe *frame = _state->infoframes.drm.drm; + struct hdr_static_metadata *hdr_metadata; + int ret; + Don't we need a GEN/monitor check before going ahead with HDR metadata ? - Shashank + hdr_metadata = (struct hdr_static_metadata *) + conn_state->hdr_output_metadata_blob_ptr->data; + + ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata); + if (ret < 0) { + DRM_ERROR("couldn't set HDR metadata in infoframe\n"); + return false; + } + + crtc_state->infoframes.enable |= + intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM); + + return true; +} + static void g4x_set_infoframes(struct intel_encoder *encoder, bool enable, const struct intel_crtc_state *crtc_state, @@ -1175,6 +1200,9 @@ static void hsw_set_infoframes(struct intel_encoder *encoder, intel_write_infoframe(encoder, crtc_state, HDMI_INFOFRAME_TYPE_VENDOR, _state->infoframes.hdmi); + intel_write_infoframe(encoder, crtc_state, + HDMI_INFOFRAME_TYPE_DRM, + _state->infoframes.drm); } void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable) @@ -2381,6 +2409,11 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, return -EINVAL; } + if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config, conn_state)) { + DRM_DEBUG_KMS("bad DRM infoframe\n"); + return -EINVAL; + } + return 0; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC dma-buf 0/3] Improve the dma-buf tracking
On Thu, Mar 14, 2019 at 6:49 PM Sumit Semwal wrote: > > Hello Chenbo,Thank you for your RFC series. > > On Wed, 27 Feb 2019 at 09:24, Chenbo Feng wrote: > > > > Currently, all dma-bufs share the same anonymous inode. While we can count > > how many dma-buf fds or mappings a process has, we can't get the size of > > the backing buffers or tell if two entries point to the same dma-buf. And > > in debugfs, we can get a per-buffer breakdown of size and reference count, > > but can't tell which processes are actually holding the references to each > > buffer. > > > > To resolve the issue above and provide better method for userspace to track > > the dma-buf usage across different processes, the following changes are > > proposed in dma-buf kernel side. First of all, replace the singleton inode > > inside the dma-buf subsystem with a mini-filesystem, and assign each > > dma-buf a unique inode out of this filesystem. With this change, calling > > stat(2) on each entry gives the caller a unique ID (st_ino), the buffer's > > size (st_size), and even the number of pages assigned to each dma-buffer. > > Secoundly, add the inode information to /sys/kernel/debug/dma_buf/bufinfo > > so in the case where a buffer is mmap()ed into a process’s address space > > but all remaining fds have been closed, we can still get the dma-buf > > information and try to accociate it with the process by searching the > > proc/pid/maps and looking for the corresponding inode number exposed in > > dma-buf debug fs. Thirdly, created an ioctl to assign names to dma-bufs > > which lets userspace assign short names (e.g., "CAMERA") to buffers. This > > information can be extremely helpful for tracking and accounting shared > > buffers based on their usage and original purpose. Last but not least, add > > dma-buf information to /proc/pid/fdinfo by adding a show_fdinfo() handler > > to dma_buf_file_operations. The handler will print the file_count and name > > of each buffer. > > In general, I think I like the idea as it contributes to a much more > relevant usage analysis of dma-buf backed buffers. > I will get to doing a more detailed review soon, but immediately, we > might want to think a bit about the get/set_name IOCTLS - do we need > to think of disallowing multiple renaming of buffers once they start > getting used? It could otherwise make the whole metrics a lot > confused? > > > > > Greg Hackmann (3): > > dma-buf: give each buffer a full-fledged inode > > dma-buf: add DMA_BUF_{GET,SET}_NAME ioctls > > dma-buf: add show_fdinfo handler I'm not seeing the patches, so just quick comment here: Some drivers (at least vc4) already have the concept of buffer names. Would be neat to integrate this between dma-buf and drm_gem in prime. Aside from that sounds like a good idea overall, but I can't see any details. -Daniel > > > > drivers/dma-buf/dma-buf.c| 121 --- > > include/linux/dma-buf.h | 5 +- > > include/uapi/linux/dma-buf.h | 4 ++ > > include/uapi/linux/magic.h | 1 + > > 4 files changed, 122 insertions(+), 9 deletions(-) > > > > -- > > 2.21.0.rc2.261.ga7da99ff1b-goog > > > > Best, > Sumit. > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/udl: add a release method and delay modeset teardown
On Fri, Mar 15, 2019 at 03:13:30PM +1000, Dave Airlie wrote: > From: Dave Airlie > > If we unplug a udl device, the usb callback with deinit the > mode_config struct, however userspace will still have an open > file descriptor and a framebuffer on that device. When userspace > closes the fd, we'll oops because it'll try and look stuff up > in the object idr which we've destroyed. > > This punts destroying the mode objects until release time instead. > > Signed-off-by: Dave Airlie Reviewed-by: Daniel Vetter For the first one pls ping Noralf to make sure it all aligns with the Grand Plan. -Daniel > --- > drivers/gpu/drm/udl/udl_drv.c | 1 + > drivers/gpu/drm/udl/udl_drv.h | 1 + > drivers/gpu/drm/udl/udl_main.c | 8 +++- > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c > index 22cd2d13e272..ff47f890e6ad 100644 > --- a/drivers/gpu/drm/udl/udl_drv.c > +++ b/drivers/gpu/drm/udl/udl_drv.c > @@ -52,6 +52,7 @@ static struct drm_driver driver = { > .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME, > .load = udl_driver_load, > .unload = udl_driver_unload, > + .release = udl_driver_release, > > /* gem hooks */ > .gem_free_object_unlocked = udl_gem_free_object, > diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h > index e9e9b1ff678e..4ae67d882eae 100644 > --- a/drivers/gpu/drm/udl/udl_drv.h > +++ b/drivers/gpu/drm/udl/udl_drv.h > @@ -104,6 +104,7 @@ void udl_urb_completion(struct urb *urb); > > int udl_driver_load(struct drm_device *dev, unsigned long flags); > void udl_driver_unload(struct drm_device *dev); > +void udl_driver_release(struct drm_device *dev); > > int udl_fbdev_init(struct drm_device *dev); > void udl_fbdev_cleanup(struct drm_device *dev); > diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c > index 9086d0d1b880..5626e1f11852 100644 > --- a/drivers/gpu/drm/udl/udl_main.c > +++ b/drivers/gpu/drm/udl/udl_main.c > @@ -379,6 +379,12 @@ void udl_driver_unload(struct drm_device *dev) > udl_free_urb_list(dev); > > udl_fbdev_cleanup(dev); > - udl_modeset_cleanup(dev); > kfree(udl); > } > + > +void udl_driver_release(struct drm_device *dev) > +{ > + drm_mode_config_cleanup(dev); > + drm_dev_fini(dev); > + kfree(dev); > +} > -- > 2.20.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- 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 V2] drm/atomic-helper: Make atomic_enable/disable crtc callbacks optional
On Thu, Mar 14, 2019 at 03:48:45PM -0300, Rodrigo Siqueira wrote: > Allow atomic_enable and atomic_disable operations from > drm_crtc_helper_funcs struct optional. With this, the target display > drivers don't need to define a dummy function if they don't need one. > > Changes since v2: > * Don't make funcs optional > * Update kerneldoc for atomic_enable/disable > * Replace "if (funcs->atomic_enable)" by "if (funcs->commit)" > * Improve commit message > > Signed-off-by: Rodrigo Siqueira Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic_helper.c | 5 ++--- > include/drm/drm_modeset_helper_vtables.h | 4 > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 540a77a2ade9..d506e13c2945 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1034,7 +1034,7 @@ disable_outputs(struct drm_device *dev, struct > drm_atomic_state *old_state) > funcs->atomic_disable(crtc, old_crtc_state); > else if (funcs->disable) > funcs->disable(crtc); > - else > + else if (funcs->dpms) > funcs->dpms(crtc, DRM_MODE_DPMS_OFF); > > if (!(dev->irq_enabled && dev->num_crtcs)) > @@ -1277,10 +1277,9 @@ void drm_atomic_helper_commit_modeset_enables(struct > drm_device *dev, > if (new_crtc_state->enable) { > DRM_DEBUG_ATOMIC("enabling [CRTC:%d:%s]\n", >crtc->base.id, crtc->name); > - > if (funcs->atomic_enable) > funcs->atomic_enable(crtc, old_crtc_state); > - else > + else if (funcs->commit) > funcs->commit(crtc); > } > } > diff --git a/include/drm/drm_modeset_helper_vtables.h > b/include/drm/drm_modeset_helper_vtables.h > index cfb7be40bed7..ce4de6b1e444 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -418,6 +418,8 @@ struct drm_crtc_helper_funcs { >* Drivers can use the @old_crtc_state input parameter if the operations >* needed to enable the CRTC don't depend solely on the new state but >* also on the transition between the old state and the new state. > + * > + * This function is optional. >*/ > void (*atomic_enable)(struct drm_crtc *crtc, > struct drm_crtc_state *old_crtc_state); > @@ -441,6 +443,8 @@ struct drm_crtc_helper_funcs { >* parameter @old_crtc_state which could be used to access the old >* state. Atomic drivers should consider to use this one instead >* of @disable. > + * > + * This function is optional. >*/ > void (*atomic_disable)(struct drm_crtc *crtc, > struct drm_crtc_state *old_crtc_state); > -- > 2.21.0 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel