[Bug 93998] Linux 4.5-rc2 ATI Radeon 3000 Graphics not rendering correctly
https://bugs.freedesktop.org/show_bug.cgi?id=93998 --- Comment #9 from SMF --- Result of bisect: 01c8f1c44b83a0825b573e7c723b033cece37b86 is the first bad commit commit 01c8f1c44b83a0825b573e7c723b033cece37b86 Author: Dan Williams Date: Fri Jan 15 16:56:40 2016 -0800 mm, dax, gpu: convert vm_insert_mixed to pfn_t Convert the raw unsigned long 'pfn' argument to pfn_t for the purpose of evaluating the PFN_MAP and PFN_DEV flags. When both are set it triggers _PAGE_DEVMAP to be set in the resulting pte. There are no functional changes to the gpu drivers as a result of this conversion. Signed-off-by: Dan Williams Cc: Dave Hansen Cc: David Airlie Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds :04 04 dc71b6dc188dea4b6ce95f5fd1e4e73cc3663cbe 8a745459a14cd2a6f4e1c71fda3e54333d023f43 March :04 04 9e8103b65698f677e17becdac53914ca029a6197 f656e15bffef4ea8c848176a79eb50a071ec323e Mdrivers :04 04 3ffc222d7e0f1067772798d08777aeb54136026e e2b423f85aaaf0cb25d48d1e15771fc5bddf7981 Mfs :04 04 392d2dea8185ba8f0d1c101782831ceed9aa9812 9e3001b04052daa1b3204b6f2a6e26138a4ed7dd Minclude :04 04 f81d8cfe3052b7259b75a58656a65efdaad40284 969d0ff262020f27a695172ce89ae5f6088e8d44 Mmm -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/6c71ca97/attachment.html>
[pull] radeon and amdgpu drm-fixes-4.5
On Thu, Feb 4, 2016 at 11:43 PM, Dave Airlie wrote: > On 4 February 2016 at 07:12, Alex Deucher wrote: >> Hi Dave, >> >> Radeon and amdgpu fixes for 4.5. Highlights: >> - fix and enable iceland/topaz support >> - handle WC on platforms that don't support it >> >> >> The following changes since commit d8b8eb829d4c30cd1e41a1ddc308a0e7c22169da: >> >> Merge branch 'drm-rockchip-next-fixes-2016-01-22' of >> https://github.com/markyzq/kernel-drm-rockchip into drm-fixes (2016-01-29 >> 10:04:29 +1000) >> >> are available in the git repository at: >> >> git://people.freedesktop.org/~agd5f/linux >> > > Hey Alex, > > something went wrong with this pull request generation? no branch name > in this, can you check this in future, I got confused for a while. > > sometimes git pull-request reports an error, or your remote branch > differs from your local one etc. Sorry. The branch is drm-fixes-4.5. The latest git upgrade on this box seems to leave the branch off when I generate pull requests now for some reason. I've checked the branch out and all is well, so I'm not exactly sure why it does that. Alex > > Dave.
[Bug 93988] Amdgpu, Tonga R9 380: ring 0 test failed (scratch(0xC040)=0xCAFEDEAD)
https://bugs.freedesktop.org/show_bug.cgi?id=93988 jk changed: What|Removed |Added Resolution|--- |NOTOURBUG Status|NEW |RESOLVED -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/2b6fd49f/attachment.html>
[Bug 93998] Linux 4.5-rc2 ATI Radeon 3000 Graphics not rendering correctly
https://bugs.freedesktop.org/show_bug.cgi?id=93998 --- Comment #8 from SMF --- (In reply to SMF from comment #7) > (In reply to SMF from comment #6) > > (In reply to Alex Deucher from comment #5) > > > Looks like you forced dpm on. Had you been using that previously? > > > > Yes I have been using this setting for some time, I cannot remember how long > > (some years ?) or why I set it in the first place. > > The other machine I have tested uses a Turks PRO [Radeon HD 6570/7570/8550] > with the same boot settings and fails in the same way. Looking back over my bug history I found this activity: https://bugzilla.kernel.org/show_bug.cgi?id=60857 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/95d9497c/attachment.html>
[PATCH 6/7] tests/amdgpu: make amdgpu_command_submission_sdma_copy_linear generic
On 4 February 2016 at 15:51, Alex Deucher wrote: > On Thu, Feb 4, 2016 at 10:47 AM, Emil Velikov > wrote: >> On 4 February 2016 at 14:59, Alex Deucher wrote: >>> So it can be shared for CP tests. >>> >>> Reviewed-by: Ken Wang >>> Signed-off-by: Alex Deucher >>> --- >>> tests/amdgpu/basic_tests.c | 27 +-- >>> 1 file changed, 17 insertions(+), 10 deletions(-) >>> >>> diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c >>> index 7806be7..701ccf1 100644 >>> --- a/tests/amdgpu/basic_tests.c >>> +++ b/tests/amdgpu/basic_tests.c >>> @@ -57,6 +57,7 @@ static void >>> amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, >>>struct amdgpu_cs_request >>> *ibs_request); >>> static void amdgpu_command_submission_write_linear_helper(unsigned >>> ip_type); >>> static void amdgpu_command_submission_const_fill_helper(unsigned ip_type); >>> +static void amdgpu_command_submission_copy_linear_helper(unsigned ip_type); >>> >> Unless I'm missing something we don't need these three ? > > They are used by patches 3, 5, and 7. > You're absolutely correct. I forgot that the amdgpu tests keep the brief functions at the top (one-online wrappers, test list). Sorry about the noise. Glad to see that there are tests sent out as the kernel bits get implemented. Keep it up :-) -Emil
[PATCH] adv7511: Added mode_fixup function.
On 02/04/2016 04:22 PM, Carlos Palminha wrote: > Hi guys, > > any feedback? patch will be accepted for adv7511 driver? Hi, Thanks for the patch, but please try to find and fix the call site that is trying to invoke the callback even though it does not exist. This is most likely drm_i2c_encoder_mode_fixup(). - Lars > > Regards, > C.Palminha > > On 01-02-2016 12:37, Carlos Palminha wrote: >> Hi Laurent >> >> On 29-01-2016 17:48, Laurent Pinchart wrote: >>> Hi Carlos, >>> >>> Thank you for the patch. >>> >>> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote: The mode_fixup is necessary when using it in a DRM FB driver pipeline. >>> >>> Instead of implementing stubs in encoder drivers, wouldn't it be better to >>> make mode_fixup optional ? >> Probably you are right but i don't have enough knowledge or time to do that >> for the DRM framework. :( >> I limited myself to do what the other drivers already implement. >> >> The patch is mandatory to have to the ADV working or else will get some NULL >> pointer crash. >> >> Regards, >> C.Palminha >> >>> Signed-off-by: Carlos Palminha --- drivers/gpu/drm/i2c/adv7511.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c index 533d1e3..90082d2 100644 --- a/drivers/gpu/drm/i2c/adv7511.c +++ b/drivers/gpu/drm/i2c/adv7511.c @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder, return status; } +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + return true; +} + static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, struct drm_display_mode *mode) { @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder *encoder, static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { .dpms = adv7511_encoder_dpms, + .mode_fixup = adv7511_encoder_mode_fixup, .mode_valid = adv7511_encoder_mode_valid, .mode_set = adv7511_encoder_mode_set, .detect = adv7511_encoder_detect, >>> >> >> >> On 29-01-2016 17:48, Laurent Pinchart wrote: >>> Hi Carlos, >>> >>> Thank you for the patch. >>> >>> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote: The mode_fixup is necessary when using it in a DRM FB driver pipeline. >>> >>> Instead of implementing stubs in encoder drivers, wouldn't it be better to >>> make mode_fixup optional ? >>> Signed-off-by: Carlos Palminha --- drivers/gpu/drm/i2c/adv7511.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c index 533d1e3..90082d2 100644 --- a/drivers/gpu/drm/i2c/adv7511.c +++ b/drivers/gpu/drm/i2c/adv7511.c @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder, return status; } +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + return true; +} + static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, struct drm_display_mode *mode) { @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder *encoder, static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { .dpms = adv7511_encoder_dpms, + .mode_fixup = adv7511_encoder_mode_fixup, .mode_valid = adv7511_encoder_mode_valid, .mode_set = adv7511_encoder_mode_set, .detect = adv7511_encoder_detect, >>> > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
[Bug 93615] Tonga does not recover from standby
https://bugs.freedesktop.org/show_bug.cgi?id=93615 EoD changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #4 from EoD --- This has been fixed in kernel 4.5-rc2. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/c15f3787/attachment.html>
[Bug 93998] Linux 4.5-rc2 ATI Radeon 3000 Graphics not rendering correctly
https://bugs.freedesktop.org/show_bug.cgi?id=93998 --- Comment #7 from SMF --- (In reply to SMF from comment #6) > (In reply to Alex Deucher from comment #5) > > Looks like you forced dpm on. Had you been using that previously? > > Yes I have been using this setting for some time, I cannot remember how long > (some years ?) or why I set it in the first place. The other machine I have tested uses a Turks PRO [Radeon HD 6570/7570/8550] with the same boot settings and fails in the same way. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/e4b43733/attachment.html>
[PATCH v9 03/14] drm/mediatek: Add DSI sub driver
On Thu, Feb 4, 2016 at 2:37 PM, CK Hu wrote: > Hi Philipp: > > On Wed, 2016-02-03 at 12:01 +0100, Philipp Zabel wrote: >> Hi Daniel, >> > >> > > +static void mtk_output_dsi_disable(struct mtk_dsi *dsi) >> > > +{ >> > > + if (!dsi->enabled) >> > > + return; >> > > + >> > > + if (dsi->panel) { >> > > + if (drm_panel_disable(dsi->panel)) { >> > > + DRM_ERROR("failed to disable the panel\n"); >> > > + return; >> > > + } >> > > + } >> > > + >> > > + mtk_dsi_poweroff(dsi); >> > >> > The order is a bit suspicious here; I would expect to poweroff dsi >> > before the panel to mirror the turn on order. >> >> CK, could you comment on this? >> > > According to the experience of other Mediatek SoC, > In mtk_output_dsi_enable(), we should do power on dsi first and then > prepare panel because dsi should be ready to receive panel prepare error > message. So we should disable panel and then power off dsi in > mtk_output_dsi_disable(). Then what about the other direction? Should we be powering up dsi first before enabling the panel so DSI can receive an panel enabling errors? -Dan > >> I can reorder this, but I'm not sure about the reasoning (what happens >> hardware wise if we just cut panel power vs. if the DSI panel first sees >> the ULP transition). Further, I don't have a panel to test, just the >> PS8640. >> >> thanks >> Philipp >> >> > > Regards, > CK >
[Bug 93998] Linux 4.5-rc2 ATI Radeon 3000 Graphics not rendering correctly
https://bugs.freedesktop.org/show_bug.cgi?id=93998 --- Comment #6 from SMF --- (In reply to Alex Deucher from comment #5) > Looks like you forced dpm on. Had you been using that previously? Yes I have been using this setting for some time, I cannot remember how long (some years ?) or why I set it in the first place. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/1519bf0e/attachment.html>
[drm:drm-fixes-mst 5/9] include/drm/drm_fixed.h:159: undefined reference to `__aeabi_uldivmod'
Hi Dave, I have a small fix to the following patch in the drm-fixes-mst branch to resolve a 32-bit compile error. I've attached the patch separately and also squashed it into the previous change. Thanks, Jordan commit 6d3d3d07381401ac6b89dfd429002c9d6ffda5e3 Author: Harry Wentland Date: Fri Jan 22 17:07:25 2016 -0500 drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil -Original Message- From: Wentland, Harry Sent: Thursday, February 04, 2016 11:40 AM To: Lazare, Jordan Subject: FW: [drm:drm-fixes-mst 5/9] include/drm/drm_fixed.h:159: undefined reference to `__aeabi_uldivmod' -Original Message- From: kbuild test robot [mailto:fengguang...@intel.com] Sent: February 4, 2016 8:34 AM To: Wentland, Harry Cc: kbuild-all at 01.org; Dave Airlie ; Deucher, Alexander Subject: [drm:drm-fixes-mst 5/9] include/drm/drm_fixed.h:159: undefined reference to `__aeabi_uldivmod' tree: git://people.freedesktop.org/~airlied/linux.git drm-fixes-mst head: 18ddcd63d22663841b8f61ee52733736bf3a4c7e commit: 275e06c063ea920c4b89ee293b1975080f887b00 [5/9] drm/dp/mst: Calculate MST PBN with 31.32 fixed point config: arm-exynos_defconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 275e06c063ea920c4b89ee293b1975080f887b00 # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): drivers/built-in.o: In function `drm_fixp_from_fraction': >> include/drm/drm_fixed.h:159: undefined reference to `__aeabi_uldivmod' include/drm/drm_fixed.h:160: undefined reference to `__aeabi_uldivmod' vim +159 include/drm/drm_fixed.h 210a0b9e Alex Deucher 2013-03-22 143 if (shift > DRM_FIXED_POINT) 210a0b9e Alex Deucher 2013-03-22 144 return result >> (shift - DRM_FIXED_POINT); 210a0b9e Alex Deucher 2013-03-22 145 210a0b9e Alex Deucher 2013-03-22 146 return result; 210a0b9e Alex Deucher 2013-03-22 147 } 210a0b9e Alex Deucher 2013-03-22 148 6d3d3d07 Harry Wentland 2016-01-22 149 static inline s64 drm_fixp_from_fraction(s64 a, s64 b) 6d3d3d07 Harry Wentland 2016-01-22 150 { 6d3d3d07 Harry Wentland 2016-01-22 151 s64 res; 6d3d3d07 Harry Wentland 2016-01-22 152 bool a_neg = a < 0; 6d3d3d07 Harry Wentland 2016-01-22 153 bool b_neg = b < 0; 6d3d3d07 Harry Wentland 2016-01-22 154 u64 a_abs = a_neg ? -a : a; 6d3d3d07 Harry Wentland 2016-01-22 155 u64 b_abs = b_neg ? -b : b; 6d3d3d07 Harry Wentland 2016-01-22 156 u64 rem; 6d3d3d07 Harry Wentland 2016-01-22 157 6d3d3d07 Harry Wentland 2016-01-22 158 /* determine integer part */ 6d3d3d07 Harry Wentland 2016-01-22 @159 u64 res_abs = a_abs / b_abs; 6d3d3d07 Harry Wentland 2016-01-22 160 rem = a_abs % b_abs; 6d3d3d07 Harry Wentland 2016-01-22 161 6d3d3d07 Harry Wentland 2016-01-22 162 /* determine fractional part */ 6d3d3d07 Harry Wentland 2016-01-22 163 { 6d3d3d07 Harry Wentland 2016-01-22 164 u32 i = DRM_FIXED_POINT; 6d3d3d07 Harry Wentland 2016-01-22 165 6d3d3d07 Harry Wentland 2016-01-22 166 do { 6d3d3d07 Harry Wentland 2016-01-22 167 rem <<= 1; :: The code at line 159 was first introduced by commit :: 6d3d3d07381401ac6b89dfd429002c9d6ffda5e3 drm: Add drm_fixp_from_fraction and drm_fixp2int_ceil :: TO: Harry Wentland :: CC: Dave Airlie --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- next part -- A non-text attachment was scrubbed... Name: 0001-drm-dp-mst-Fix-32-bit-fixed-point-division-calculati.patch Type: application/octet-stream Size: 984 bytes Desc: 0001-drm-dp-mst-Fix-32-bit-fixed-point-division-calculati.patch URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/d5ddea10/attachment.obj> -- next part -- A non-text attachment was scrubbed... Name: 0001-drm-Add-drm_fixp_from_fraction-and-drm_fixp2int_ceil.patch Type: application/octet-stream Size: 2613 bytes Desc: 0001-drm-Add-drm_fixp_from_fraction-and-drm_fixp2int_ceil.patch URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/d5ddea10/attachment-0001.obj>
[PATCH] drivers/gpu/vga: use __GFP_NOWARN for user-controlled kmalloc
On Thu, Feb 4, 2016 at 5:59 PM, Ville Syrjälä wrote: > On Thu, Feb 04, 2016 at 05:37:49PM +0100, Dmitry Vyukov wrote: >> On Thu, Feb 4, 2016 at 5:32 PM, Ville Syrjälä >> wrote: >> > On Thu, Feb 04, 2016 at 04:49:49PM +0100, Dmitry Vyukov wrote: >> >> Size of kmalloc() in vga_arb_write() is controlled by user. >> >> Too large kmalloc() size triggers WARNING message on console. >> >> >> >> Use GFP_USER | __GFP_NOWARN for this kmalloc() to not scare admins. >> >> >> >> Signed-off-by: Dmitry Vyukov >> >> --- >> >> Example WARNING: >> >> >> >> WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999 >> >> __alloc_pages_nodemask+0x7d2/0x1760() >> >> Modules linked in: >> >> CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB 4.5.0-rc1+ #283 >> >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs >> >> 01/01/2011 >> >> 880069eff670 8299a06d >> >> 8800658a4740 864985a0 880069eff6b0 8134fcf9 >> >> 8166de32 864985a0 0bb7 024040c0 >> >> Call Trace: >> >> [< inline >] __dump_stack lib/dump_stack.c:15 >> >> [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 >> >> [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 >> >> [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 >> >> [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999 >> >> [] __alloc_pages_nodemask+0x7d2/0x1760 >> >> mm/page_alloc.c:3253 >> >> [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090 >> >> [< inline >] alloc_pages include/linux/gfp.h:459 >> >> [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433 >> >> [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008 >> >> [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019 >> >> [< inline >] kmalloc_large include/linux/slab.h:395 >> >> [] __kmalloc+0x2f4/0x340 mm/slub.c:3557 >> >> [< inline >] kmalloc include/linux/slab.h:468 >> >> [] vga_arb_write+0xd4/0xe40 >> >> drivers/gpu/vga/vgaarb.c:926 >> >> [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719 >> >> [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849 >> >> [] vfs_writev+0x86/0xc0 fs/read_write.c:886 >> >> [< inline >] SYSC_writev fs/read_write.c:919 >> >> [] SyS_writev+0x111/0x2b0 fs/read_write.c:911 >> >> [] entry_SYSCALL_64_fastpath+0x16/0x7a >> >> arch/x86/entry/entry_64.S:185 >> >> --- >> >> drivers/gpu/vga/vgaarb.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c >> >> index f17cb04..d73b85b 100644 >> >> --- a/drivers/gpu/vga/vgaarb.c >> >> +++ b/drivers/gpu/vga/vgaarb.c >> >> @@ -923,7 +923,7 @@ static ssize_t vga_arb_write(struct file *file, const >> >> char __user *buf, >> >> int i; >> >> >> >> >> >> - kbuf = kmalloc(count + 1, GFP_KERNEL); >> >> + kbuf = kmalloc(count + 1, GFP_USER | __GFP_NOWARN); >> > >> > I don't really see why it does this user controlled malloc in the >> > first place. The max legth of the string it will actually handle looks >> > well bounded, so it could just use some fixed length buffer (on stack >> > even). >> >> >> What would be the right limit on data len? > > From the looks of things the longest command could be the > "target PCI:domain:bus:dev.fn" thing. Even assuming something silly like > having 10 characters for each domain,bus,dev,fn that would still be only > 55 bytes. So based on that even something like 64 bytes should be more > than enough AFAICS. David, what do you think? I can allocate char kbuf[64] on stack. > The other thing that strikes me as bit odd in this code is that it > just ignores whatever data is left over after it's done parsing the > string. But it returns the full count to userspace, indicating it > ate all of it. I guess that's fairly sane when userspace just uses a > fixed size buffer and checks that the kernel consumed it all. But > maybe there should be an actual check to see that there's a '\0' > or maybe +'\0' after the parsed string.
[PATCH] drm: modes: add missing [drm] to message printing
On Thu, 04 Feb 2016, Lukas Wunner wrote: > Hi, > > On Thu, Feb 04, 2016 at 03:03:52PM +0100, LABBE Corentin wrote: >> The warning message in drm_mode_parse_command_line_for_connector miss >> the [drm] at beginning. >> This patch add it and take the opportunity to convert >> printk(KERN_WARNING to pr_warn() >> >> Signed-off-by: LABBE Corentin >> --- >> drivers/gpu/drm/drm_modes.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c >> index 20775c0..f7448a5 100644 >> --- a/drivers/gpu/drm/drm_modes.c >> +++ b/drivers/gpu/drm/drm_modes.c >> @@ -1371,8 +1371,7 @@ bool drm_mode_parse_command_line_for_connector(const >> char *mode_option, >> } >> done: >> if (i >= 0) { >> -printk(KERN_WARNING >> -"parse error at position %i in video mode '%s'\n", >> +pr_warn("[drm] parse error at position %i in video mode '%s'\n", >> i, name); > > Hm, maybe it's warranted to add a DRM_WARNING macro to include/drm/drmP.h > and use that instead? I think it's good as-is for now. We have this all over the place, and I'd rather see a concerted effort moving away from all this custom stuff than having a one-off definition for a single patch. And that probably needs some more discussion, not worth blocking this patch for that. For pr_warn and friends, one approach is defining pr_fmt() macro at the top. BR, Jani. > > Best regards, > > Lukas > >> mode->specified = false; >> return false; >> -- >> 2.4.10 >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center
[PATCH] drivers/gpu/vga: use __GFP_NOWARN for user-controlled kmalloc
On Thu, Feb 04, 2016 at 05:37:49PM +0100, Dmitry Vyukov wrote: > On Thu, Feb 4, 2016 at 5:32 PM, Ville Syrjälä > wrote: > > On Thu, Feb 04, 2016 at 04:49:49PM +0100, Dmitry Vyukov wrote: > >> Size of kmalloc() in vga_arb_write() is controlled by user. > >> Too large kmalloc() size triggers WARNING message on console. > >> > >> Use GFP_USER | __GFP_NOWARN for this kmalloc() to not scare admins. > >> > >> Signed-off-by: Dmitry Vyukov > >> --- > >> Example WARNING: > >> > >> WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999 > >> __alloc_pages_nodemask+0x7d2/0x1760() > >> Modules linked in: > >> CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB 4.5.0-rc1+ #283 > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs > >> 01/01/2011 > >> 880069eff670 8299a06d > >> 8800658a4740 864985a0 880069eff6b0 8134fcf9 > >> 8166de32 864985a0 0bb7 024040c0 > >> Call Trace: > >> [< inline >] __dump_stack lib/dump_stack.c:15 > >> [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 > >> [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 > >> [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 > >> [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999 > >> [] __alloc_pages_nodemask+0x7d2/0x1760 > >> mm/page_alloc.c:3253 > >> [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090 > >> [< inline >] alloc_pages include/linux/gfp.h:459 > >> [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433 > >> [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008 > >> [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019 > >> [< inline >] kmalloc_large include/linux/slab.h:395 > >> [] __kmalloc+0x2f4/0x340 mm/slub.c:3557 > >> [< inline >] kmalloc include/linux/slab.h:468 > >> [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926 > >> [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719 > >> [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849 > >> [] vfs_writev+0x86/0xc0 fs/read_write.c:886 > >> [< inline >] SYSC_writev fs/read_write.c:919 > >> [] SyS_writev+0x111/0x2b0 fs/read_write.c:911 > >> [] entry_SYSCALL_64_fastpath+0x16/0x7a > >> arch/x86/entry/entry_64.S:185 > >> --- > >> drivers/gpu/vga/vgaarb.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > >> index f17cb04..d73b85b 100644 > >> --- a/drivers/gpu/vga/vgaarb.c > >> +++ b/drivers/gpu/vga/vgaarb.c > >> @@ -923,7 +923,7 @@ static ssize_t vga_arb_write(struct file *file, const > >> char __user *buf, > >> int i; > >> > >> > >> - kbuf = kmalloc(count + 1, GFP_KERNEL); > >> + kbuf = kmalloc(count + 1, GFP_USER | __GFP_NOWARN); > > > > I don't really see why it does this user controlled malloc in the > > first place. The max legth of the string it will actually handle looks > > well bounded, so it could just use some fixed length buffer (on stack > > even). > > > What would be the right limit on data len? >From the looks of things the longest command could be the "target PCI:domain:bus:dev.fn" thing. Even assuming something silly like having 10 characters for each domain,bus,dev,fn that would still be only 55 bytes. So based on that even something like 64 bytes should be more than enough AFAICS. The other thing that strikes me as bit odd in this code is that it just ignores whatever data is left over after it's done parsing the string. But it returns the full count to userspace, indicating it ate all of it. I guess that's fairly sane when userspace just uses a fixed size buffer and checks that the kernel consumed it all. But maybe there should be an actual check to see that there's a '\0' or maybe +'\0' after the parsed string. -- Ville Syrjälä Intel OTC
[PATCH] drivers/gpu/vga: use __GFP_NOWARN for user-controlled kmalloc
On Thu, Feb 04, 2016 at 04:49:49PM +0100, Dmitry Vyukov wrote: > Size of kmalloc() in vga_arb_write() is controlled by user. > Too large kmalloc() size triggers WARNING message on console. > > Use GFP_USER | __GFP_NOWARN for this kmalloc() to not scare admins. > > Signed-off-by: Dmitry Vyukov > --- > Example WARNING: > > WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999 > __alloc_pages_nodemask+0x7d2/0x1760() > Modules linked in: > CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB 4.5.0-rc1+ #283 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > 880069eff670 8299a06d > 8800658a4740 864985a0 880069eff6b0 8134fcf9 > 8166de32 864985a0 0bb7 024040c0 > Call Trace: > [< inline >] __dump_stack lib/dump_stack.c:15 > [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 > [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 > [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 > [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999 > [] __alloc_pages_nodemask+0x7d2/0x1760 mm/page_alloc.c:3253 > [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090 > [< inline >] alloc_pages include/linux/gfp.h:459 > [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433 > [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008 > [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019 > [< inline >] kmalloc_large include/linux/slab.h:395 > [] __kmalloc+0x2f4/0x340 mm/slub.c:3557 > [< inline >] kmalloc include/linux/slab.h:468 > [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926 > [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719 > [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849 > [] vfs_writev+0x86/0xc0 fs/read_write.c:886 > [< inline >] SYSC_writev fs/read_write.c:919 > [] SyS_writev+0x111/0x2b0 fs/read_write.c:911 > [] entry_SYSCALL_64_fastpath+0x16/0x7a > arch/x86/entry/entry_64.S:185 > --- > drivers/gpu/vga/vgaarb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > index f17cb04..d73b85b 100644 > --- a/drivers/gpu/vga/vgaarb.c > +++ b/drivers/gpu/vga/vgaarb.c > @@ -923,7 +923,7 @@ static ssize_t vga_arb_write(struct file *file, const > char __user *buf, > int i; > > > - kbuf = kmalloc(count + 1, GFP_KERNEL); > + kbuf = kmalloc(count + 1, GFP_USER | __GFP_NOWARN); I don't really see why it does this user controlled malloc in the first place. The max legth of the string it will actually handle looks well bounded, so it could just use some fixed length buffer (on stack even). > if (!kbuf) > return -ENOMEM; > > -- > 2.7.0.rc3.207.g0ac5344 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC
[PATCH] qxl: handling failed allocation
Since kmalloc can be failed in memory pressure, check and return error code otherwise NULL deference could be happened Signed-off-by: Insu Yun --- drivers/gpu/drm/qxl/qxl_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index b2977a1..02d26b3 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -221,6 +221,9 @@ static int qxl_device_init(struct qxl_device *qdev, kmalloc(qdev->n_mem_slots * sizeof(struct qxl_memslot), GFP_KERNEL); + if (!qdev->mem_slots) + return -ENOMEM; + idr_init(&qdev->release_idr); spin_lock_init(&qdev->release_idr_lock); spin_lock_init(&qdev->release_lock); -- 1.9.1
[Intel-gfx] [PATCH] drm/i915/skl: Fix typo in DPLL_CFGCR1 definition
On Thu, Feb 04, 2016 at 10:43:21AM -0500, Lyude wrote: > We accidentally point both cfgcr registers for the second shared DPLL to > the same location in i915_reg.h. This results in a lot of hw pipe state > mismatches whenever we try to do a modeset that requires allocating the > DPLL to a CRTC: > > [drm:intel_pipe_config_compare [i915]] *ERROR* mismatch in > dpll_hw_state.cfgcr1 (expected 0x8168, found 0x04a5) > [drm:intel_pipe_config_compare [i915]] *ERROR* mismatch in > base.adjusted_mode.crtc_clock (expected 108000, found 49500) > [drm:intel_pipe_config_compare [i915]] *ERROR* mismatch in port_clock > (expected 108000, found 49500) > > This usually ends up causing blank monitors, since the DPLL never can > get set to the right clock. > > Fixes: f0f59a00a1 ("drm/i915: Type safe register read/write") Actually Fixes: 086f8e84a085 ("drm/i915: Prefix raw register defines with underscore") That's the second regression from the type safety stuff :( I guess we still don't have enough testing coverage since this has gone undetected for so long. Reviewed-by: Ville Syrjälä > Signed-off-by: Lyude > --- > drivers/gpu/drm/i915/i915_reg.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 007ae83..b9a564b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7514,7 +7514,7 @@ enum skl_disp_power_wells { > #define DPLL_CFGCR2_PDIV_7 (4<<2) > #define DPLL_CFGCR2_CENTRAL_FREQ_MASK (3) > > -#define DPLL_CFGCR1(id) _MMIO_PIPE((id) - SKL_DPLL1, _DPLL1_CFGCR1, > _DPLL2_CFGCR2) > +#define DPLL_CFGCR1(id) _MMIO_PIPE((id) - SKL_DPLL1, _DPLL1_CFGCR1, > _DPLL2_CFGCR1) > #define DPLL_CFGCR2(id) _MMIO_PIPE((id) - SKL_DPLL1, _DPLL1_CFGCR2, > _DPLL2_CFGCR2) > > /* BXT display engine PLL */ > -- > 2.5.0 > > ___ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC
[PATCH] drivers/gpu/vga: use __GFP_NOWARN for user-controlled kmalloc
On Thu, Feb 4, 2016 at 5:32 PM, Ville Syrjälä wrote: > On Thu, Feb 04, 2016 at 04:49:49PM +0100, Dmitry Vyukov wrote: >> Size of kmalloc() in vga_arb_write() is controlled by user. >> Too large kmalloc() size triggers WARNING message on console. >> >> Use GFP_USER | __GFP_NOWARN for this kmalloc() to not scare admins. >> >> Signed-off-by: Dmitry Vyukov >> --- >> Example WARNING: >> >> WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999 >> __alloc_pages_nodemask+0x7d2/0x1760() >> Modules linked in: >> CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB 4.5.0-rc1+ #283 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> 880069eff670 8299a06d >> 8800658a4740 864985a0 880069eff6b0 8134fcf9 >> 8166de32 864985a0 0bb7 024040c0 >> Call Trace: >> [< inline >] __dump_stack lib/dump_stack.c:15 >> [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 >> [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 >> [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 >> [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999 >> [] __alloc_pages_nodemask+0x7d2/0x1760 >> mm/page_alloc.c:3253 >> [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090 >> [< inline >] alloc_pages include/linux/gfp.h:459 >> [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433 >> [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008 >> [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019 >> [< inline >] kmalloc_large include/linux/slab.h:395 >> [] __kmalloc+0x2f4/0x340 mm/slub.c:3557 >> [< inline >] kmalloc include/linux/slab.h:468 >> [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926 >> [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719 >> [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849 >> [] vfs_writev+0x86/0xc0 fs/read_write.c:886 >> [< inline >] SYSC_writev fs/read_write.c:919 >> [] SyS_writev+0x111/0x2b0 fs/read_write.c:911 >> [] entry_SYSCALL_64_fastpath+0x16/0x7a >> arch/x86/entry/entry_64.S:185 >> --- >> drivers/gpu/vga/vgaarb.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c >> index f17cb04..d73b85b 100644 >> --- a/drivers/gpu/vga/vgaarb.c >> +++ b/drivers/gpu/vga/vgaarb.c >> @@ -923,7 +923,7 @@ static ssize_t vga_arb_write(struct file *file, const >> char __user *buf, >> int i; >> >> >> - kbuf = kmalloc(count + 1, GFP_KERNEL); >> + kbuf = kmalloc(count + 1, GFP_USER | __GFP_NOWARN); > > I don't really see why it does this user controlled malloc in the > first place. The max legth of the string it will actually handle looks > well bounded, so it could just use some fixed length buffer (on stack > even). What would be the right limit on data len?
[Bug 93998] Linux 4.5-rc2 ATI Radeon 3000 Graphics not rendering correctly
https://bugs.freedesktop.org/show_bug.cgi?id=93998 --- Comment #5 from Alex Deucher --- Looks like you forced dpm on. Had you been using that previously? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/fe9e05b4/attachment.html>
[Bug 93998] Linux 4.5-rc2 ATI Radeon 3000 Graphics not rendering correctly
https://bugs.freedesktop.org/show_bug.cgi?id=93998 --- Comment #4 from SMF --- Created attachment 121526 --> https://bugs.freedesktop.org/attachment.cgi?id=121526&action=edit dmesg output dmesg output as requested, I will bisect as suggested. Thanks -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/7dcd9042/attachment.html>
[PATCH 6/7] tests/amdgpu: make amdgpu_command_submission_sdma_copy_linear generic
On Thu, Feb 4, 2016 at 4:38 PM, Emil Velikov wrote: > On 4 February 2016 at 15:51, Alex Deucher wrote: >> On Thu, Feb 4, 2016 at 10:47 AM, Emil Velikov >> wrote: >>> On 4 February 2016 at 14:59, Alex Deucher wrote: So it can be shared for CP tests. Reviewed-by: Ken Wang Signed-off-by: Alex Deucher --- tests/amdgpu/basic_tests.c | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c index 7806be7..701ccf1 100644 --- a/tests/amdgpu/basic_tests.c +++ b/tests/amdgpu/basic_tests.c @@ -57,6 +57,7 @@ static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, struct amdgpu_cs_request *ibs_request); static void amdgpu_command_submission_write_linear_helper(unsigned ip_type); static void amdgpu_command_submission_const_fill_helper(unsigned ip_type); +static void amdgpu_command_submission_copy_linear_helper(unsigned ip_type); >>> Unless I'm missing something we don't need these three ? >> >> They are used by patches 3, 5, and 7. >> > You're absolutely correct. I forgot that the amdgpu tests keep the > brief functions at the top (one-online wrappers, test list). Sorry > about the noise. No worries. I thought about moving the implementations up, but this was less code churn. Alex
[PATCH] drivers/gpu/vga: use __GFP_NOWARN for user-controlled kmalloc
Size of kmalloc() in vga_arb_write() is controlled by user. Too large kmalloc() size triggers WARNING message on console. Use GFP_USER | __GFP_NOWARN for this kmalloc() to not scare admins. Signed-off-by: Dmitry Vyukov --- Example WARNING: WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999 __alloc_pages_nodemask+0x7d2/0x1760() Modules linked in: CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB 4.5.0-rc1+ #283 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 880069eff670 8299a06d 8800658a4740 864985a0 880069eff6b0 8134fcf9 8166de32 864985a0 0bb7 024040c0 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999 [] __alloc_pages_nodemask+0x7d2/0x1760 mm/page_alloc.c:3253 [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090 [< inline >] alloc_pages include/linux/gfp.h:459 [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433 [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008 [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019 [< inline >] kmalloc_large include/linux/slab.h:395 [] __kmalloc+0x2f4/0x340 mm/slub.c:3557 [< inline >] kmalloc include/linux/slab.h:468 [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926 [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719 [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849 [] vfs_writev+0x86/0xc0 fs/read_write.c:886 [< inline >] SYSC_writev fs/read_write.c:919 [] SyS_writev+0x111/0x2b0 fs/read_write.c:911 [] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185 --- drivers/gpu/vga/vgaarb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c index f17cb04..d73b85b 100644 --- a/drivers/gpu/vga/vgaarb.c +++ b/drivers/gpu/vga/vgaarb.c @@ -923,7 +923,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, int i; - kbuf = kmalloc(count + 1, GFP_KERNEL); + kbuf = kmalloc(count + 1, GFP_USER | __GFP_NOWARN); if (!kbuf) return -ENOMEM; -- 2.7.0.rc3.207.g0ac5344
[PATCH] drm: modes: add missing [drm] to message printing
Hi, On Thu, Feb 04, 2016 at 03:03:52PM +0100, LABBE Corentin wrote: > The warning message in drm_mode_parse_command_line_for_connector miss > the [drm] at beginning. > This patch add it and take the opportunity to convert > printk(KERN_WARNING to pr_warn() > > Signed-off-by: LABBE Corentin > --- > drivers/gpu/drm/drm_modes.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 20775c0..f7448a5 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1371,8 +1371,7 @@ bool drm_mode_parse_command_line_for_connector(const > char *mode_option, > } > done: > if (i >= 0) { > - printk(KERN_WARNING > - "parse error at position %i in video mode '%s'\n", > + pr_warn("[drm] parse error at position %i in video mode '%s'\n", > i, name); Hm, maybe it's warranted to add a DRM_WARNING macro to include/drm/drmP.h and use that instead? Best regards, Lukas > mode->specified = false; > return false; > -- > 2.4.10 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/7] tests/amdgpu: make amdgpu_command_submission_sdma_copy_linear generic
On 4 February 2016 at 14:59, Alex Deucher wrote: > So it can be shared for CP tests. > > Reviewed-by: Ken Wang > Signed-off-by: Alex Deucher > --- > tests/amdgpu/basic_tests.c | 27 +-- > 1 file changed, 17 insertions(+), 10 deletions(-) > > diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c > index 7806be7..701ccf1 100644 > --- a/tests/amdgpu/basic_tests.c > +++ b/tests/amdgpu/basic_tests.c > @@ -57,6 +57,7 @@ static void > amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, >struct amdgpu_cs_request *ibs_request); > static void amdgpu_command_submission_write_linear_helper(unsigned ip_type); > static void amdgpu_command_submission_const_fill_helper(unsigned ip_type); > +static void amdgpu_command_submission_copy_linear_helper(unsigned ip_type); > Unless I'm missing something we don't need these three ? -Emil
[PATCH 1/7] tests/amdgpu: make amdgpu_sdma_test_exec_cs() generic
On 4 February 2016 at 14:59, Alex Deucher wrote: > Share with upcoming CP tests. > > Reviewed-by: Ken Wang > Signed-off-by: Alex Deucher > --- > tests/amdgpu/basic_tests.c | 58 > -- > 1 file changed, 35 insertions(+), 23 deletions(-) > > diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c > index fa0ed12..1bcc835 100644 > --- a/tests/amdgpu/basic_tests.c > +++ b/tests/amdgpu/basic_tests.c > @@ -49,6 +49,13 @@ static void amdgpu_command_submission_sdma(void); > static void amdgpu_userptr_test(void); > static void amdgpu_semaphore_test(void); > > +static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, > + unsigned ip_type, > + int instance, int pm4_dw, uint32_t > *pm4_src, > + int res_cnt, amdgpu_bo_handle > *resources, > + struct amdgpu_cs_ib_info *ib_info, > + struct amdgpu_cs_request *ibs_request); > + We don't need the forward declaration, do we ? -Emil
[PATCH] adv7511: Added mode_fixup function.
Hi guys, any feedback? patch will be accepted for adv7511 driver? Regards, C.Palminha On 01-02-2016 12:37, Carlos Palminha wrote: > Hi Laurent > > On 29-01-2016 17:48, Laurent Pinchart wrote: >> Hi Carlos, >> >> Thank you for the patch. >> >> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote: >>> The mode_fixup is necessary when using it in a DRM FB driver pipeline. >> >> Instead of implementing stubs in encoder drivers, wouldn't it be better to >> make mode_fixup optional ? > Probably you are right but i don't have enough knowledge or time to do that > for the DRM framework. :( > I limited myself to do what the other drivers already implement. > > The patch is mandatory to have to the ADV working or else will get some NULL > pointer crash. > > Regards, > C.Palminha > >> >>> Signed-off-by: Carlos Palminha >>> --- >>> drivers/gpu/drm/i2c/adv7511.c | 8 >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c >>> index 533d1e3..90082d2 100644 >>> --- a/drivers/gpu/drm/i2c/adv7511.c >>> +++ b/drivers/gpu/drm/i2c/adv7511.c >>> @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder, >>> return status; >>> } >>> >>> +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder, >>> + const struct drm_display_mode *mode, >>> + struct drm_display_mode *adjusted_mode) >>> +{ >>> + return true; >>> +} >>> + >>> static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, >>> struct drm_display_mode *mode) >>> { >>> @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder >>> *encoder, >>> >>> static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { >>> .dpms = adv7511_encoder_dpms, >>> + .mode_fixup = adv7511_encoder_mode_fixup, >>> .mode_valid = adv7511_encoder_mode_valid, >>> .mode_set = adv7511_encoder_mode_set, >>> .detect = adv7511_encoder_detect, >> > > > On 29-01-2016 17:48, Laurent Pinchart wrote: >> Hi Carlos, >> >> Thank you for the patch. >> >> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote: >>> The mode_fixup is necessary when using it in a DRM FB driver pipeline. >> >> Instead of implementing stubs in encoder drivers, wouldn't it be better to >> make mode_fixup optional ? >> >>> Signed-off-by: Carlos Palminha >>> --- >>> drivers/gpu/drm/i2c/adv7511.c | 8 >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c >>> index 533d1e3..90082d2 100644 >>> --- a/drivers/gpu/drm/i2c/adv7511.c >>> +++ b/drivers/gpu/drm/i2c/adv7511.c >>> @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder, >>> return status; >>> } >>> >>> +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder, >>> + const struct drm_display_mode *mode, >>> + struct drm_display_mode *adjusted_mode) >>> +{ >>> + return true; >>> +} >>> + >>> static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, >>> struct drm_display_mode *mode) >>> { >>> @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder >>> *encoder, >>> >>> static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { >>> .dpms = adv7511_encoder_dpms, >>> + .mode_fixup = adv7511_encoder_mode_fixup, >>> .mode_valid = adv7511_encoder_mode_valid, >>> .mode_set = adv7511_encoder_mode_set, >>> .detect = adv7511_encoder_detect, >>
[PATCH] drm: modes: add missing [drm] to message printing
The warning message in drm_mode_parse_command_line_for_connector miss the [drm] at beginning. This patch add it and take the opportunity to convert printk(KERN_WARNING to pr_warn() Signed-off-by: LABBE Corentin --- drivers/gpu/drm/drm_modes.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 20775c0..f7448a5 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1371,8 +1371,7 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, } done: if (i >= 0) { - printk(KERN_WARNING - "parse error at position %i in video mode '%s'\n", + pr_warn("[drm] parse error at position %i in video mode '%s'\n", i, name); mode->specified = false; return false; -- 2.4.10
[PATCH v9 03/14] drm/mediatek: Add DSI sub driver
Hi Philipp: On Wed, 2016-02-03 at 12:01 +0100, Philipp Zabel wrote: > Hi Daniel, > > > > +static void mtk_output_dsi_disable(struct mtk_dsi *dsi) > > > +{ > > > + if (!dsi->enabled) > > > + return; > > > + > > > + if (dsi->panel) { > > > + if (drm_panel_disable(dsi->panel)) { > > > + DRM_ERROR("failed to disable the panel\n"); > > > + return; > > > + } > > > + } > > > + > > > + mtk_dsi_poweroff(dsi); > > > > The order is a bit suspicious here; I would expect to poweroff dsi > > before the panel to mirror the turn on order. > > CK, could you comment on this? > According to the experience of other Mediatek SoC, In mtk_output_dsi_enable(), we should do power on dsi first and then prepare panel because dsi should be ready to receive panel prepare error message. So we should disable panel and then power off dsi in mtk_output_dsi_disable(). > I can reorder this, but I'm not sure about the reasoning (what happens > hardware wise if we just cut panel power vs. if the DSI panel first sees > the ULP transition). Further, I don't have a panel to test, just the > PS8640. > > thanks > Philipp > > Regards, CK
[PATCH 1/8] drm/i915: Add i915 perf infrastructure
On Thu, Feb 4, 2016 at 1:17 PM, Robert Bragg wrote: > > On Thu, Feb 4, 2016 at 1:42 AM, Emil Velikov > wrote: > >> On 3 February 2016 at 18:39, Robert Bragg wrote: >> >>> >>> > +}; >>> > + >>> > +struct drm_i915_perf_open_param { >>> > + /** CLOEXEC, NONBLOCK... */ >>> > + __u32 flags; >>> > + >>> And ... we broke 32 bit userspare on 64 bit kernels. Please check for >>> holes and other issues as described in Daniel Vetter's >>> article/documentation [1] >>> >>> [1] >>> https://www.kernel.org/doc/Documentation/ioctl/botching-up-ioctls.txt >> >> >> Ah yeah, don't think this would break 32bit userspace, but still would be >> good to remove that hole, this has been through a few iterations and there >> used to be a __u32 type here, well spotted thanks. >> >> I think I'll bump the flags to be 64bit. >> >> > Actually, just noticed that since the structure has a u32 hole at the end too I can move the trailing u32 num_properties up into here instead. Am also renaming properties to properties_ptr which seems the norm in i915_drm.h. - Robert -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/cedc2dbd/attachment.html>
[PATCH v10 03/13] drm/mediatek: Add DSI sub driver
Hi Emil, Am Donnerstag, den 04.02.2016, 13:09 + schrieb Emil Velikov: > Hi Philipp, > > On 3 February 2016 at 19:25, Philipp Zabel wrote: > > > diff --git a/drivers/gpu/drm/mediatek/Kconfig > > b/drivers/gpu/drm/mediatek/Kconfig > > index 8dad892..b7e0404 100644 > > --- a/drivers/gpu/drm/mediatek/Kconfig > > +++ b/drivers/gpu/drm/mediatek/Kconfig > > @@ -3,6 +3,9 @@ config DRM_MEDIATEK > > depends on DRM > > depends on ARCH_MEDIATEK || (ARM && COMPILE_TEST) > > select DRM_KMS_HELPER > > + select DRM_MIPI_DSI > > + select DRM_PANEL > > > + select DRM_PANEL_SIMPLE > Please don't force a particular panel like that. No other driver does > it, so might as well not start now ? I'll drop this select on DRM_PANEL_SIMPLE. thanks Philipp
[PATCH v3 06/11] staging/android: turn fence_info into a __u64 pointer
Op 04-02-16 om 14:05 schreef Gustavo Padovan: > 2016-02-04 Maarten Lankhorst : > >> Op 03-02-16 om 21:09 schreef Gustavo Padovan: >>> Hi Maarten, >>> >>> 2016-02-03 Maarten Lankhorst : >>> Op 03-02-16 om 14:25 schreef Gustavo Padovan: > From: Gustavo Padovan > > Turn sync_fence_info into __u64 type enable us to extend the struct in the > future without breaking the ABI. > > v2: use type __u64 for fence_info > > v3: fix commit message to reflect the v2 change > > Signed-off-by: Gustavo Padovan > --- > drivers/staging/android/sync.c | 2 +- > drivers/staging/android/uapi/sync.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/android/sync.c > b/drivers/staging/android/sync.c > index 2ab0c20..8425457 100644 > --- a/drivers/staging/android/sync.c > +++ b/drivers/staging/android/sync.c > @@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct > sync_file *sync_file, > if (info->status >= 0) > info->status = !info->status; > > - len = sizeof(struct sync_file_info); > + len = sizeof(struct sync_file_info) - sizeof(__u64); > > for (i = 0; i < sync_file->num_fences; ++i) { > struct fence *fence = sync_file->cbs[i].fence; > diff --git a/drivers/staging/android/uapi/sync.h > b/drivers/staging/android/uapi/sync.h > index a0cf357..e649953 100644 > --- a/drivers/staging/android/uapi/sync.h > +++ b/drivers/staging/android/uapi/sync.h > @@ -54,7 +54,7 @@ struct sync_file_info { > charname[32]; > __s32 status; > > - __u8sync_fence_info[0]; > + __u64 sync_fence_info; > }; > > #define SYNC_IOC_MAGIC '>' This still doesn't do what you expect it to. I think this is what you want is for userspace to do: struct sync_file_info info; info.flags = info.num_fences = 0; ioctl(fd, SYNC_IOC_FENCE_INFO, &info); if (info.num_fences) { info.sync_fence_info = (uintptr)kcalloc(info.num_fences, sizeof(struct sync_fence_info)); ioctl(fd, SYNC_IOC_FENCE_INFO, &info); } Maybe userspace could preallocate the max in advance and set num_fences higher, kernel would do something like: num_fences = min(info.num_fences, sync->num_fences); struct sync_fence_info array[num_fences]; info.num_fences = sync->num_fences; if (num_fences && copy_to_user((void * __user)(unsigned long)info.sync_fence_info, array, num_fences * sizeof(array))) return -EFAULT; >>> If we are going to call IOCTL twice I would actually have a new IOCTL only >>> to fetch sync_fence_info. >>> >>> First we would call >>> >>> ioctl(fd, SYNC_IOC_FILE_INFO, &info); >>> >>> where info is: >>> >>> struct sync_file_info { >>> charname[32]; >>> __s32 status; >>> __u32 flags; >>> __u32 num_fences; >>> }; >>> >>> then we would allocate a buffer with >>> >>> size = info.num_fences * sizeof(struct sync_fence_info) >>> >>> and call the new ioctl >>> >>> ioctl(fd, SYNC_IOC_SYNC_FENCE_INFO, sync_fence_info); >>> >>> This looks like a cleaner solution and doesn't break ABI. What do you >>> think? >> I think it's good taste that userspace specifies the size of the buffer it >> passes, so former feels more clean to me, >> since you need to pass num_fences anyway. > Just to clarify, userspace specifies the size of the buffer in the > solution I proposed. It would be > > size = info.num_fences * sizeof(struct sync_fence_info) > > sync_fence_info = malloc(size); > > ioctl(fd, SYNC_IOC_SYNC_FENCE_INFO, sync_fence_info); > For someone writing a wrapper like valgrind would mean having prior knowledge of previous ioctl results too. Hence you would need something like struct fence_collection { u32 num_fences; u32 pad; struct sync_fence_info fences[0]; } in which case you might as well return it optionally as a pointer in sync_file_info. ~Maarten
[Bug 93998] Linux 4.5-rc2 ATI Radeon 3000 Graphics not rendering correctly
https://bugs.freedesktop.org/show_bug.cgi?id=93998 --- Comment #3 from Alex Deucher --- If this is a regression, can you bisect? Please also attach your dmesg output. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/c927f3e9/attachment.html>
[PATCH v9 03/14] drm/mediatek: Add DSI sub driver
Am Donnerstag, den 04.02.2016, 14:37 +0800 schrieb CK Hu: > Hi Philipp: > > On Wed, 2016-02-03 at 12:01 +0100, Philipp Zabel wrote: > > Hi Daniel, > > > > > > > +static void mtk_output_dsi_disable(struct mtk_dsi *dsi) > > > > +{ > > > > + if (!dsi->enabled) > > > > + return; > > > > + > > > > + if (dsi->panel) { > > > > + if (drm_panel_disable(dsi->panel)) { > > > > + DRM_ERROR("failed to disable the panel\n"); > > > > + return; > > > > + } > > > > + } > > > > + > > > > + mtk_dsi_poweroff(dsi); > > > > > > The order is a bit suspicious here; I would expect to poweroff dsi > > > before the panel to mirror the turn on order. > > > > CK, could you comment on this? > > > > According to the experience of other Mediatek SoC, > In mtk_output_dsi_enable(), we should do power on dsi first and then > prepare panel because dsi should be ready to receive panel prepare error > message. So we should disable panel and then power off dsi in > mtk_output_dsi_disable(). > > > I can reorder this, but I'm not sure about the reasoning (what happens > > hardware wise if we just cut panel power vs. if the DSI panel first sees > > the ULP transition). Further, I don't have a panel to test, just the > > PS8640. > > > > thanks > > Philipp I just realized that this code isn't even using drm_panel_enable and drm_panel_unprepare. I suppose the order generally should be: prepare and enable dsi (but don't start stream yet) drm_panel_prepare() enable dsi output drm_panel_enable() and to disable: drm_panel_disable() disable dsi output drm_panel_unprepare() power off dsi ? regards Philipp
[PATCH libdrm] android: enable building static version of libdrm
On 29 January 2016 at 16:00, Rob Herring wrote: > From: Sumit Semwal > > Android needs libdrm built statically for recovery; > enable that as well. > > Signed-off-by: Sumit Semwal > Signed-off-by: Rob Herring > Cc: Chih-Wei Huang > Cc: Emil Velikov > --- > I posted this to mesa-dev, but I guess libdrm patches are supposed to go > to dri-devel. > Thanks Rob. I've pushed all outstanding libdrm patches that I could find. Do let me know if I missed anything. -Emil
[PATCH 1/8] drm/i915: Add i915 perf infrastructure
On Thu, Feb 4, 2016 at 1:42 AM, Emil Velikov wrote: > On 3 February 2016 at 18:39, Robert Bragg wrote: > > > index a5524cc..68ca26e 100644 > > --- a/include/uapi/drm/i915_drm.h > > +++ b/include/uapi/drm/i915_drm.h > > > @@ -1170,4 +1172,71 @@ struct drm_i915_gem_context_param { > > __u64 value; > > }; > > > > +#define I915_PERF_FLAG_FD_CLOEXEC (1<<0) > > +#define I915_PERF_FLAG_FD_NONBLOCK (1<<1) > > +#define I915_PERF_FLAG_DISABLED(1<<2) > > + > > +enum drm_i915_perf_property_id { > > + /** > > +* Open the stream for a specific context handle (as used with > > +* execbuffer2). A stream opened for a specific context this way > > +* won't typically require root privileges. > > +*/ > > + DRM_I915_PERF_CTX_HANDLE_PROP = 1, > > + > Wouldn't DRM_I915_PERF_PROP_CTX_HANDLE be a better name ? It's more > obvious at least :-P > Yeah would be more consistent to keep the common namespacing at the front. > > > + DRM_I915_PERF_PROP_MAX /* non-ABI */ > Isn't the use of enums (in drm UAPI) discouraged ? Afaics only the old > DRI1 i915 code has them. > Same question applies throughout the patch/series. > An enum here seems like a pretty good technical fit. I think the potential for different enums to have different sizes is a likely reason for being cautious about using them as part of a kernel ABI (so probably unwise to have enum based members in an ioctl structure) but in this case these IDs are always passed to the kernel as a u64. We benefit from the compiler reminding us to handle all properties in the driver if we switch on this enum which I like, as well as having the _MAX value to refer to in the driver which is perhaps a little more reliable at the end of an enum vs a #define which needs to be explicitly updated for each addition. For reference, the first iteration of this driver was based on the core pref infrastructure and in this case the enum + _MAX /* non-ABI */ approach is borrowed from there (ref: include/uapi/linux/perf_event.h) > > +}; > > + > > +struct drm_i915_perf_open_param { > > + /** CLOEXEC, NONBLOCK... */ > Some other places in i915 define the macros just after the __u32 > flags. This will allow you to drop the comment ;-) > > > + __u32 flags; > > + > And ... we broke 32 bit userspare on 64 bit kernels. Please check for > holes and other issues as described in Daniel Vetter's > article/documentation [1] > > [1] https://www.kernel.org/doc/Documentation/ioctl/botching-up-ioctls.txt Ah yeah, don't think this would break 32bit userspace, but still would be good to remove that hole, this has been through a few iterations and there used to be a __u32 type here, well spotted thanks. I think I'll bump the flags to be 64bit. > > > > + /** > > +* Pointer to array of u64 (id, value) pairs configuring the > stream > > +* to open. > > +*/ > > + __u64 __user properties; > > + > > + /** The number of u64 (id, value) pairs */ > > + __u32 n_properties; > The rest of the file uses num_foo or foo_count. Can we opt for one of > those ? > Ah yep, num_properties looks good to me. > > > +}; > > + > > +#define I915_PERF_IOCTL_ENABLE _IO('i', 0x0) > > +#define I915_PERF_IOCTL_DISABLE_IO('i', 0x1) > > + > > +/** > > + * Common to all i915 perf records > > + */ > > +struct drm_i915_perf_record_header { > > + __u32 type; > > + __u16 pad; > Move pad at the end ? This way we can (maybe?) reuse it in the future. > This header was originally based on struct perf_event_header which is layed out with the padding in the middle too. I can't currently think of a strong reason to maintain a record header that's ABI compatible with perf, but /maybe/ it could be convenient in some case to have a common layout for profiling tools that deal with both. I think we can consider it reserved for future use either way. > > > + __u16 size; > > +}; > > + > > Daniel, is there a consensus about zeroing the pad from userspace and > checking it for garbage in the ioctl ? The documentation says "do it", > yet I have a hunch that we're missing a few. Also what error should > the ioctl return in such a case - EINVAL or EFAULT ? Worth > documenting, just in case ? > In case you're wondering about the padding in the header above, note that this header never gets passed in from userspace, it's a header for data read by userspace and the driver zeros the padding. For the hole in the ioctl I'll probably fill that by extending the flags member and the driver currently returns -EINVAL if any unknown flag bits are set by userspace. Thanks for looking through this, - Robert > > -Emil > -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/84447ccd/attachment.html>
[PATCH v10 03/13] drm/mediatek: Add DSI sub driver
Hi Philipp, On 3 February 2016 at 19:25, Philipp Zabel wrote: > diff --git a/drivers/gpu/drm/mediatek/Kconfig > b/drivers/gpu/drm/mediatek/Kconfig > index 8dad892..b7e0404 100644 > --- a/drivers/gpu/drm/mediatek/Kconfig > +++ b/drivers/gpu/drm/mediatek/Kconfig > @@ -3,6 +3,9 @@ config DRM_MEDIATEK > depends on DRM > depends on ARCH_MEDIATEK || (ARM && COMPILE_TEST) > select DRM_KMS_HELPER > + select DRM_MIPI_DSI > + select DRM_PANEL > + select DRM_PANEL_SIMPLE Please don't force a particular panel like that. No other driver does it, so might as well not start now ? Cheers, Emil
Direct userspace dma-buf mmap (v7)
On Tue, Dec 22, 2015 at 1:36 PM, Tiago Vignatti wrote: > Hey back, > > Thank you Daniel, Chris, Alex and Thomas for reviewing the last series. I > think I addressed most of the comments now in version 7, including: > - being even more wording in the doc about sync usage. > - pass .write = false always in i915 end_cpu_access. > - add sync invalid flags test (igt). > - in kms_mmap_write_crc, use CPU hog and testing rounds to catch the sync > problems (igt). > > Here are the trees: > > https://github.com/tiagovignatti/drm-intel/commits/drm-intel-nightly_dma-buf-mmap-v7 > https://github.com/tiagovignatti/intel-gpu-tools/commits/dma-buf-mmap-v7 > > Also, Chrome OS side is in progress. This past week I've been mostly > struggling with fail attempts to build it (boots and goes to a black screen. > Sigh.) and also finding a way to make my funky BayTrail-T laptop with 32-bit > UEFI firmware boot up (success with Ubuntu but no success yet in CrOS). A WIP > of Chromium changes can be seen here anyways: > > https://codereview.chromium.org/1262043002/ > > Happy Holidays! For the series: Reviewed-by: Stéphane Marchesin > > Tiago > > -- > 2.1.4 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/7] drm/fsl-dcu: use mode flags for hsync/vsync pixelclk polarity
On 2016-02-03 15:18, Stefan Agner wrote: > On 2016-02-03 06:00, Thierry Reding wrote: >> On Wed, Jan 27, 2016 at 06:46:50PM -0800, Stefan Agner wrote: >> [...] >>> > diff --git a/drivers/gpu/drm/panel/panel-simple.c >>> > b/drivers/gpu/drm/panel/panel-simple.c >>> > index f97b73e..fa68b56 100644 >>> > --- a/drivers/gpu/drm/panel/panel-simple.c >>> > +++ b/drivers/gpu/drm/panel/panel-simple.c >>> > @@ -960,6 +960,8 @@ static const struct drm_display_mode >>> > nec_nl4827hc19_05b_mode = { >>> > .vsync_end = 272 + 2 + 4, >>> > .vtotal = 272 + 2 + 4 + 2, >>> > .vrefresh = 74, >>> > + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC | >>> > + DISPLAY_FLAGS_PIXDATA_POSEDGE, >> >> It doesn't seem like these two types of flags should be mixed because >> they overlap. DISPLAY_FLAGS_PIXDATA_POSEDGE has the same value as the >> DRM_MODE_FLAG_CSYNC define. > > There are other entries such as hannstar_hsd100pxn1_timing which make > use of DISPLAY_FLAGS too, hence I did not bother further. Having a > conflict is definitely not what we want. Oh, just realized, hannstar_hsd100pxn1_timing is actually a different type of struct, and for this struct the DISPALY_FLAGS make sense... > >> The definition of the DISPLAY_FLAGS_PIXDATA_POSEDGE is also not very >> clear to me. I don't think we have an equivalent DRM_MODE_FLAG_* but >> we could add one if there's really a need. > > E.g. assuming a parallel video signal, the pixel data signals could be > changing on positive or negative edge relative to the clock signal. Most > displays _sample_ the data on rising edge, hence controllers should > normally drive data on falling edge. > > However, as always, there are exceptions to the rule, and one of the > exception is Freescales default display for the Tower evaluation board. > It samples data on falling edge, hence the controller should drive data > on rising edge... > > Yeah I also did not found an equivalent in DRM_MODE_FLAG_*. I have here > also other displays where we would need this flag. The display-timings > binds and enum display_flags support it too, hence I guess we should > have it here too. > > I will split out this patch from the patchset (I already applied the > rest), add another patch to add the flag, make use of the flag in this > patch and resend it as v2. I realized that after this, there are only two flags which are not yet in DRM_MODE_FLAG: DE_LOW/DE_HIGH. Thierry, what do you think, should I add these remaining two flags too? -- Stefan
[PATCH] amdgpu: fix for submition with no ibs
From: Ken Wang Avoid a crash if no IBs are specified. Signed-off-by: Ken Wang Reviewed-by: Alex Deucher --- amdgpu/amdgpu_cs.c | 8 amdgpu/amdgpu_internal.h | 1 + 2 files changed, 9 insertions(+) diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index b4f41b0..fb5b3a8 100644 --- a/amdgpu/amdgpu_cs.c +++ b/amdgpu/amdgpu_cs.c @@ -190,6 +190,10 @@ static int amdgpu_cs_submit_one(amdgpu_context_handle context, return -EINVAL; if (ibs_request->number_of_ibs > AMDGPU_CS_MAX_IBS_PER_SUBMIT) return -EINVAL; + if (ibs_request->number_of_ibs == 0) { + ibs_request->seq_no = AMDGPU_NULL_SUBMIT_SEQ; + return 0; + } user_fence = (ibs_request->fence_info.handle != NULL); size = ibs_request->number_of_ibs + (user_fence ? 2 : 1) + 1; @@ -422,6 +426,10 @@ int amdgpu_cs_query_fence_status(struct amdgpu_cs_fence *fence, return -EINVAL; if (fence->ring >= AMDGPU_CS_MAX_RINGS) return -EINVAL; + if (fence->fence == AMDGPU_NULL_SUBMIT_SEQ) { + *expired = true; + return 0; + } *expired = false; diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h index 557ba1f..4f039b6 100644 --- a/amdgpu/amdgpu_internal.h +++ b/amdgpu/amdgpu_internal.h @@ -44,6 +44,7 @@ #define ROUND_DOWN(x, y) ((x) & ~__round_mask(x, y)) #define AMDGPU_INVALID_VA_ADDRESS 0x +#define AMDGPU_NULL_SUBMIT_SEQ 0 struct amdgpu_bo_va_hole { struct list_head list; -- 2.5.0
[PATCH 10/10] drm/vc4: Add support for YUV planes.
This supports 420 and 422 subsampling with 2 or 3 planes, tested with modetest. It doesn't set up chroma subsampling position (which it appears KMS doesn't deal with yet). The LBM memory is overallocated in many cases, but apparently the docs aren't quite correct and I'll probably need to look at the hardware source to really figure it out. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_plane.c | 252 +++- drivers/gpu/drm/vc4/vc4_regs.h | 56 - 2 files changed, 249 insertions(+), 59 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 9a390cf..f20878e 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -54,15 +54,19 @@ struct vc4_plane_state { /* Clipped coordinates of the plane on the display. */ int crtc_x, crtc_y, crtc_w, crtc_h; /* Clipped area being scanned from in the FB. */ - u32 src_x, src_y, src_w, src_h; + u32 src_x, src_y; - enum vc4_scaling_mode x_scaling, y_scaling; + u32 src_w[2], src_h[2]; + + /* Scaling selection for the RGB/Y plane and the Cb/Cr planes. */ + enum vc4_scaling_mode x_scaling[2], y_scaling[2]; bool is_unity; + bool is_yuv; /* Offset to start scanning out from the start of the plane's * BO. */ - u32 offset; + u32 offsets[3]; /* Our allocation in LBM for temporary storage during scaling. */ struct drm_mm_node lbm; @@ -79,6 +83,7 @@ static const struct hvs_format { u32 hvs; /* HVS_FORMAT_* */ u32 pixel_order; bool has_alpha; + bool flip_cbcr; } hvs_formats[] = { { .drm = DRM_FORMAT_XRGB, .hvs = HVS_PIXEL_FORMAT_RGBA, @@ -104,6 +109,32 @@ static const struct hvs_format { .drm = DRM_FORMAT_XRGB1555, .hvs = HVS_PIXEL_FORMAT_RGBA5551, .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = false, }, + { + .drm = DRM_FORMAT_YUV422, + .hvs = HVS_PIXEL_FORMAT_YCBCR_YUV422_3PLANE, + }, + { + .drm = DRM_FORMAT_YVU422, + .hvs = HVS_PIXEL_FORMAT_YCBCR_YUV422_3PLANE, + .flip_cbcr = true, + }, + { + .drm = DRM_FORMAT_YUV420, + .hvs = HVS_PIXEL_FORMAT_YCBCR_YUV420_3PLANE, + }, + { + .drm = DRM_FORMAT_YVU420, + .hvs = HVS_PIXEL_FORMAT_YCBCR_YUV420_3PLANE, + .flip_cbcr = true, + }, + { + .drm = DRM_FORMAT_NV12, + .hvs = HVS_PIXEL_FORMAT_YCBCR_YUV420_2PLANE, + }, + { + .drm = DRM_FORMAT_NV16, + .hvs = HVS_PIXEL_FORMAT_YCBCR_YUV422_2PLANE, + }, }; static const struct hvs_format *vc4_get_hvs_format(u32 drm_format) @@ -219,11 +250,11 @@ static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) * * This is a replication of a table from the spec. */ -static u32 vc4_get_scl_field(struct drm_plane_state *state) +static u32 vc4_get_scl_field(struct drm_plane_state *state, int plane) { struct vc4_plane_state *vc4_state = to_vc4_plane_state(state); - switch (vc4_state->x_scaling << 2 | vc4_state->y_scaling) { + switch (vc4_state->x_scaling[plane] << 2 | vc4_state->y_scaling[plane]) { case VC4_SCALING_PPF << 2 | VC4_SCALING_PPF: return SCALER_CTL0_SCL_H_PPF_V_PPF; case VC4_SCALING_TPZ << 2 | VC4_SCALING_PPF: @@ -254,9 +285,16 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) struct drm_plane *plane = state->plane; struct vc4_plane_state *vc4_state = to_vc4_plane_state(state); struct drm_framebuffer *fb = state->fb; + struct drm_gem_cma_object *bo = drm_fb_cma_get_gem_obj(fb, 0); u32 subpixel_src_mask = (1 << 16) - 1; + u32 format = fb->pixel_format; + int num_planes = drm_format_num_planes(format); + u32 h_subsample = 1; + u32 v_subsample = 1; + int i; - vc4_state->offset = fb->offsets[0]; + for (i = 0; i < num_planes; i++) + vc4_state->offsets[i] = bo->paddr + fb->offsets[i]; /* We don't support subpixel source positioning for scaling. */ if ((state->src_x & subpixel_src_mask) || @@ -268,20 +306,48 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) vc4_state->src_x = state->src_x >> 16; vc4_state->src_y = state->src_y >> 16; - vc4_state->src_w = state->src_w >> 16; - vc4_state->src_h = state->src_h >> 16; + vc4_state->src_w[0] = state->src_w >> 16; + vc4_state->src_h[0] = state->src_h >> 16; vc4_state->crtc_x = state->crtc_x; vc4_state->crtc_y = state->crtc_y; vc4_state->crtc_w = state->crtc_w; vc4_state->crtc_h = state->crtc_h; - vc4_state->x_scaling
[PATCH 09/10] drm/vc4: Add support a few more RGB display plane formats.
These were all touch-tested with modetest. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_plane.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 7cedc726..9a390cf 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -88,6 +88,22 @@ static const struct hvs_format { .drm = DRM_FORMAT_ARGB, .hvs = HVS_PIXEL_FORMAT_RGBA, .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = true, }, + { + .drm = DRM_FORMAT_RGB565, .hvs = HVS_PIXEL_FORMAT_RGB565, + .pixel_order = HVS_PIXEL_ORDER_XRGB, .has_alpha = false, + }, + { + .drm = DRM_FORMAT_BGR565, .hvs = HVS_PIXEL_FORMAT_RGB565, + .pixel_order = HVS_PIXEL_ORDER_XBGR, .has_alpha = false, + }, + { + .drm = DRM_FORMAT_ARGB1555, .hvs = HVS_PIXEL_FORMAT_RGBA5551, + .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = true, + }, + { + .drm = DRM_FORMAT_XRGB1555, .hvs = HVS_PIXEL_FORMAT_RGBA5551, + .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = false, + }, }; static const struct hvs_format *vc4_get_hvs_format(u32 drm_format) -- 2.7.0
[PATCH 08/10] drm/vc4: Add support for scaling of display planes.
This implements a simple policy for choosing scaling modes (trapezoidal for decimation, PPF for magnification), and a single PPF filter (Mitchell/Netravali's recommendation). Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_drv.h | 4 + drivers/gpu/drm/vc4/vc4_hvs.c | 85 ++ drivers/gpu/drm/vc4/vc4_plane.c | 253 +--- drivers/gpu/drm/vc4/vc4_regs.h | 46 4 files changed, 375 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 663e440..a7fea77 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -155,7 +155,11 @@ struct vc4_hvs { * list. Units are dwords. */ struct drm_mm dlist_mm; + /* Memory manager for the LBM memory used by HVS scaling. */ + struct drm_mm lbm_mm; spinlock_t mm_lock; + + struct drm_mm_node mitchell_netravali_filter; }; struct vc4_plane { diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index 9e43554..1c8b220 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -100,12 +100,77 @@ int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused) } #endif +/* The filter kernel is composed of dwords each containing 3 9-bit + * signed integers packed next to each other. + */ +#define VC4_INT_TO_COEFF(coeff) (coeff & 0x1ff) +#define VC4_PPF_FILTER_WORD(c0, c1, c2)\ + c0) & 0x1ff) << 0) |\ +(((c1) & 0x1ff) << 9) |\ +(((c2) & 0x1ff) << 18)) + +/* The whole filter kernel is arranged as the coefficients 0-16 going + * up, then a pad, then 17-31 going down and reversed within the + * dwords. This means that a linear phase kernel (where it's + * symmetrical at the boundary between 15 and 16) has the last 5 + * dwords matching the first 5, but reversed.. + */ +#define VC4_LINEAR_PHASE_KERNEL(c0, c1, c2, c3, c4, c5, c6, c7, c8,\ + c9, c10, c11, c12, c13, c14, c15) \ + {VC4_PPF_FILTER_WORD(c0, c1, c2), \ +VC4_PPF_FILTER_WORD(c3, c4, c5), \ +VC4_PPF_FILTER_WORD(c6, c7, c8), \ +VC4_PPF_FILTER_WORD(c9, c10, c11), \ +VC4_PPF_FILTER_WORD(c12, c13, c14),\ +VC4_PPF_FILTER_WORD(c15, c15, 0)} + +#define VC4_LINEAR_PHASE_KERNEL_DWORDS 6 +#define VC4_KERNEL_DWORDS (VC4_LINEAR_PHASE_KERNEL_DWORDS * 2 - 1) + +/* Recommended B=1/3, C=1/3 filter choice from Mitchell/Netravali. + * http://www.cs.utexas.edu/~fussell/courses/cs384g/lectures/mitchell/Mitchell.pdf + */ +static const u32 mitchell_netravali_1_3_1_3_kernel[] = + VC4_LINEAR_PHASE_KERNEL(0, -2, -6, -8, -10, -8, -3, 2, 18, + 50, 82, 119, 155, 187, 213, 227); + +static int vc4_hvs_upload_linear_kernel(struct vc4_hvs *hvs, + struct drm_mm_node *space, + const u32 *kernel) +{ + int ret, i; + u32 __iomem *dst_kernel; + + ret = drm_mm_insert_node(&hvs->dlist_mm, space, VC4_KERNEL_DWORDS, 1, +0); + if (ret) { + DRM_ERROR("Failed to allocate space for filter kernel: %d\n", + ret); + return ret; + } + + dst_kernel = hvs->dlist + space->start; + + /* Upload up to the symmetric word */ + for (i = 0; i < VC4_KERNEL_DWORDS; i++) { + if (i < VC4_LINEAR_PHASE_KERNEL_DWORDS) + writel(kernel[i], &dst_kernel[i]); + else { + writel(kernel[VC4_KERNEL_DWORDS - i - 1], + &dst_kernel[i]); + } + } + + return 0; +} + static int vc4_hvs_bind(struct device *dev, struct device *master, void *data) { struct platform_device *pdev = to_platform_device(dev); struct drm_device *drm = dev_get_drvdata(master); struct vc4_dev *vc4 = drm->dev_private; struct vc4_hvs *hvs = NULL; + int ret; hvs = devm_kzalloc(&pdev->dev, sizeof(*hvs), GFP_KERNEL); if (!hvs) @@ -130,6 +195,22 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data) HVS_BOOTLOADER_DLIST_END, (SCALER_DLIST_SIZE >> 2) - HVS_BOOTLOADER_DLIST_END); + /* Set up the HVS LBM memory manager. We could have some more +* complicated data structure that allowed reuse of LBM areas +* between planes when they don't overlap on the screen, but +* for now we just allocate globally. +*/ + drm_mm_init(&hvs->lbm_mm, 0, 96 * 1024); + + /* Upload filter kernels. We only have the one for now, so we +
[PATCH 07/10] drm/vc4: Fix which value is being used for source image size.
This doesn't matter yet since we only allow 1:1 scaling, but the comment clearly says we should be using the source size. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_plane.c | 23 ++- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 9ef3569..4e4c3ea 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -47,6 +47,8 @@ struct vc4_plane_state { /* Clipped coordinates of the plane on the display. */ int crtc_x, crtc_y, crtc_w, crtc_h; + /* Clipped size of the area scanned from in the FB. */ + u32 src_w, src_h; /* Offset to start scanning out from the start of the plane's * BO. @@ -170,11 +172,6 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) vc4_state->offset = fb->offsets[0]; - vc4_state->crtc_x = state->crtc_x; - vc4_state->crtc_y = state->crtc_y; - vc4_state->crtc_w = state->crtc_w; - vc4_state->crtc_h = state->crtc_h; - if (state->crtc_w << 16 != state->src_w || state->crtc_h << 16 != state->src_h) { /* We don't support scaling yet, which involves @@ -185,17 +182,25 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) return -EINVAL; } + vc4_state->src_w = state->src_w >> 16; + vc4_state->src_h = state->src_h >> 16; + + vc4_state->crtc_x = state->crtc_x; + vc4_state->crtc_y = state->crtc_y; + vc4_state->crtc_w = state->crtc_w; + vc4_state->crtc_h = state->crtc_h; + if (vc4_state->crtc_x < 0) { vc4_state->offset += (drm_format_plane_cpp(fb->pixel_format, 0) * -vc4_state->crtc_x); - vc4_state->crtc_w += vc4_state->crtc_x; + vc4_state->src_w += vc4_state->crtc_x; vc4_state->crtc_x = 0; } if (vc4_state->crtc_y < 0) { vc4_state->offset += fb->pitches[0] * -vc4_state->crtc_y; - vc4_state->crtc_h += vc4_state->crtc_y; + vc4_state->src_h += vc4_state->crtc_y; vc4_state->crtc_y = 0; } @@ -244,8 +249,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane, SCALER_POS2_ALPHA_MODE_PIPELINE : SCALER_POS2_ALPHA_MODE_FIXED, SCALER_POS2_ALPHA_MODE) | - VC4_SET_FIELD(vc4_state->crtc_w, SCALER_POS2_WIDTH) | - VC4_SET_FIELD(vc4_state->crtc_h, SCALER_POS2_HEIGHT)); + VC4_SET_FIELD(vc4_state->src_w, SCALER_POS2_WIDTH) | + VC4_SET_FIELD(vc4_state->src_h, SCALER_POS2_HEIGHT)); /* Position Word 3: Context. Written by the HVS. */ vc4_dlist_write(vc4_state, 0xc0c0c0c0); -- 2.7.0
[PATCH 06/10] drm/vc4: Add more display planes to each CRTC.
Previously we only did the primary and cursor plane, but overlay planes are useful and just require this setup to add, since all planes go into the HVS display list in the same way. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_crtc.c | 56 ++ 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 12a61d5..68227cc 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -677,9 +677,9 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data) struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_crtc *vc4_crtc; struct drm_crtc *crtc; - struct drm_plane *primary_plane, *cursor_plane; + struct drm_plane *primary_plane, *cursor_plane, *destroy_plane, *temp; const struct of_device_id *match; - int ret; + int ret, i; vc4_crtc = devm_kzalloc(dev, sizeof(*vc4_crtc), GFP_KERNEL); if (!vc4_crtc) @@ -708,27 +708,49 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data) goto err; } - cursor_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_CURSOR); - if (IS_ERR(cursor_plane)) { - dev_err(dev, "failed to construct cursor plane\n"); - ret = PTR_ERR(cursor_plane); - goto err_primary; - } - - drm_crtc_init_with_planes(drm, crtc, primary_plane, cursor_plane, + drm_crtc_init_with_planes(drm, crtc, primary_plane, NULL, &vc4_crtc_funcs, NULL); drm_crtc_helper_add(crtc, &vc4_crtc_helper_funcs); primary_plane->crtc = crtc; - cursor_plane->crtc = crtc; vc4->crtc[drm_crtc_index(crtc)] = vc4_crtc; vc4_crtc->channel = vc4_crtc->data->hvs_channel; + /* Set up some arbitrary number of planes. We're not limited +* by a set number of physical registers, just the space in +* the HVS (16k) and how small an plane can be (28 bytes). +* However, each plane we set up takes up some memory, and +* increases the cost of looping over planes, which atomic +* modesetting does quite a bit. As a result, we pick a +* modest number of planes to expose, that should hopefully +* still cover any sane usecase. +*/ + for (i = 0; i < 8; i++) { + struct drm_plane *plane = + vc4_plane_init(drm, DRM_PLANE_TYPE_OVERLAY); + + if (IS_ERR(plane)) + continue; + + plane->possible_crtcs = 1 << drm_crtc_index(crtc); + } + + /* Set up the legacy cursor after overlay initialization, +* since we overlay planes on the CRTC in the order they were +* initialized. +*/ + cursor_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_CURSOR); + if (!IS_ERR(cursor_plane)) { + cursor_plane->possible_crtcs = 1 << drm_crtc_index(crtc); + cursor_plane->crtc = crtc; + crtc->cursor = cursor_plane; + } + CRTC_WRITE(PV_INTEN, 0); CRTC_WRITE(PV_INTSTAT, PV_INT_VFP_START); ret = devm_request_irq(dev, platform_get_irq(pdev, 0), vc4_crtc_irq_handler, 0, "vc4 crtc", vc4_crtc); if (ret) - goto err_cursor; + goto err_destroy_planes; vc4_set_crtc_possible_masks(drm, crtc); @@ -736,10 +758,12 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data) return 0; -err_cursor: - cursor_plane->funcs->destroy(cursor_plane); -err_primary: - primary_plane->funcs->destroy(primary_plane); +err_destroy_planes: + list_for_each_entry_safe(destroy_plane, temp, +&drm->mode_config.plane_list, head) { + if (destroy_plane->possible_crtcs == 1 << drm_crtc_index(crtc)) + destroy_plane->funcs->destroy(destroy_plane); + } err: return ret; } -- 2.7.0
[PATCH 05/10] drm/vc4: Make the CRTCs cooperate on allocating display lists.
So far, we've only ever lit up one CRTC, so this has been fine. To extend to more displays or more planes, we need to make sure we don't run our display lists into each other. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_crtc.c | 115 +++-- drivers/gpu/drm/vc4/vc4_drv.h | 8 ++- drivers/gpu/drm/vc4/vc4_hvs.c | 13 + 3 files changed, 84 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 018145e..12a61d5 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -49,22 +49,27 @@ struct vc4_crtc { /* Which HVS channel we're using for our CRTC. */ int channel; - /* Pointer to the actual hardware display list memory for the -* crtc. -*/ - u32 __iomem *dlist; - - u32 dlist_size; /* in dwords */ - struct drm_pending_vblank_event *event; }; +struct vc4_crtc_state { + struct drm_crtc_state base; + /* Dlist area for this CRTC configuration. */ + struct drm_mm_node mm; +}; + static inline struct vc4_crtc * to_vc4_crtc(struct drm_crtc *crtc) { return (struct vc4_crtc *)crtc; } +static inline struct vc4_crtc_state * +to_vc4_crtc_state(struct drm_crtc_state *crtc_state) +{ + return (struct vc4_crtc_state *)crtc_state; +} + struct vc4_crtc_data { /* Which channel of the HVS this pixelvalve sources from. */ int hvs_channel; @@ -319,11 +324,13 @@ static void vc4_crtc_enable(struct drm_crtc *crtc) static int vc4_crtc_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *state) { + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state); struct drm_device *dev = crtc->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); struct drm_plane *plane; - struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); + unsigned long flags; u32 dlist_count = 0; + int ret; /* The pixelvalve can only feed one encoder (and encoders are * 1:1 with connectors.) @@ -346,18 +353,12 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc, dlist_count++; /* Account for SCALER_CTL0_END. */ - if (!vc4_crtc->dlist || dlist_count > vc4_crtc->dlist_size) { - vc4_crtc->dlist = ((u32 __iomem *)vc4->hvs->dlist + - HVS_BOOTLOADER_DLIST_END); - vc4_crtc->dlist_size = ((SCALER_DLIST_SIZE >> 2) - - HVS_BOOTLOADER_DLIST_END); - - if (dlist_count > vc4_crtc->dlist_size) { - DRM_DEBUG_KMS("dlist too large for CRTC (%d > %d).\n", - dlist_count, vc4_crtc->dlist_size); - return -EINVAL; - } - } + spin_lock_irqsave(&vc4->hvs->mm_lock, flags); + ret = drm_mm_insert_node(&vc4->hvs->dlist_mm, &vc4_state->mm, +dlist_count, 1, 0); + spin_unlock_irqrestore(&vc4->hvs->mm_lock, flags); + if (ret) + return ret; return 0; } @@ -368,47 +369,29 @@ static void vc4_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state); struct drm_plane *plane; bool debug_dump_regs = false; - u32 __iomem *dlist_next = vc4_crtc->dlist; + u32 __iomem *dlist_start = vc4->hvs->dlist + vc4_state->mm.start; + u32 __iomem *dlist_next = dlist_start; if (debug_dump_regs) { DRM_INFO("CRTC %d HVS before:\n", drm_crtc_index(crtc)); vc4_hvs_dump_state(dev); } - /* Copy all the active planes' dlist contents to the hardware dlist. -* -* XXX: If the new display list was large enough that it -* overlapped a currently-read display list, we need to do -* something like disable scanout before putting in the new -* list. For now, we're safe because we only have the two -* planes. -*/ + /* Copy all the active planes' dlist contents to the hardware dlist. */ drm_atomic_crtc_for_each_plane(plane, crtc) { dlist_next += vc4_plane_write_dlist(plane, dlist_next); } - if (dlist_next == vc4_crtc->dlist) { - /* If no planes were enabled, use the SCALER_CTL0_END -* at the start of the display list memory (in the -* bootloader section). We'll rewrite that -* SCALER_CTL0_END, just in case, though. -*/ - writel(SCALER_CTL0_END, vc4->hvs->dlist); - HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel), 0); - } else { - writel(SCALER_CTL0_END, dlist_next); -
[PATCH 04/10] drm/vc4: Add a proper short-circut path for legacy cursor updates.
Previously, on every modeset we would allocate new display list memory, recompute changed planes, write all of them to the new memory, and pointed scanout at the new list (which will latch approximately at the next line of scanout). We let drm_atomic_helper_wait_for_vblanks() decide whether we needed to wait for a vblank after a modeset before cleaning up the old state and letting the next modeset proceed, and on legacy cursor updates we wouldn't wait. If you moved the cursor fast enough, we could potentially wrap around the display list memory area and overwrite the existing display list while it was still being scanned out, resulting in the HVS scanning out garbage or just halting. Instead of making cursor updates wait for scanout to move to the new display list area (which introduces significant cursor lag in X), we just rewrite our current display list. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_kms.c | 9 drivers/gpu/drm/vc4/vc4_plane.c | 93 + 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index f95f2df..4718ae5 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -49,6 +49,15 @@ vc4_atomic_complete_commit(struct vc4_commit *c) drm_atomic_helper_commit_modeset_enables(dev, state); + /* Make sure that drm_atomic_helper_wait_for_vblanks() +* actually waits for vblank. If we're doing a full atomic +* modeset (as opposed to a vc4_update_plane() short circuit), +* then we need to wait for scanout to be done with our +* display lists before we free it and potentially reallocate +* and overwrite the dlist memory with a new modeset. +*/ + state->legacy_cursor_update = false; + drm_atomic_helper_wait_for_vblanks(dev, state); drm_atomic_helper_cleanup_planes(dev, state); diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 554ed54..9ef3569 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -33,8 +33,12 @@ struct vc4_plane_state { u32 dlist_size; /* Number of dwords allocated for the display list */ u32 dlist_count; /* Number of used dwords in the display list. */ - /* Offset in the dlist to pointer word 0. */ - u32 pw0_offset; + /* Offset in the dlist to various words, for pageflip or +* cursor updates. +*/ + u32 pos0_offset; + u32 pos2_offset; + u32 ptr0_offset; /* Offset where the plane's dlist was last stored in the * hardware at vc4_crtc_atomic_flush() time. @@ -223,6 +227,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane, SCALER_CTL0_UNITY); /* Position Word 0: Image Positions and Alpha Value */ + vc4_state->pos0_offset = vc4_state->dlist_count; vc4_dlist_write(vc4_state, VC4_SET_FIELD(0xff, SCALER_POS0_FIXED_ALPHA) | VC4_SET_FIELD(vc4_state->crtc_x, SCALER_POS0_START_X) | @@ -233,6 +238,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane, */ /* Position Word 2: Source Image Size, Alpha Mode */ + vc4_state->pos2_offset = vc4_state->dlist_count; vc4_dlist_write(vc4_state, VC4_SET_FIELD(format->has_alpha ? SCALER_POS2_ALPHA_MODE_PIPELINE : @@ -244,9 +250,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane, /* Position Word 3: Context. Written by the HVS. */ vc4_dlist_write(vc4_state, 0xc0c0c0c0); - vc4_state->pw0_offset = vc4_state->dlist_count; - /* Pointer Word 0: RGB / Y Pointer */ + vc4_state->ptr0_offset = vc4_state->dlist_count; vc4_dlist_write(vc4_state, bo->paddr + vc4_state->offset); /* Pointer Context Word 0: Written by the HVS */ @@ -332,13 +337,13 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb) * scanout will start from this address as soon as the FIFO * needs to refill with pixels. */ - writel(addr, &vc4_state->hw_dlist[vc4_state->pw0_offset]); + writel(addr, &vc4_state->hw_dlist[vc4_state->ptr0_offset]); /* Also update the CPU-side dlist copy, so that any later * atomic updates that don't do a new modeset on our plane * also use our updated address. */ - vc4_state->dlist[vc4_state->pw0_offset] = addr; + vc4_state->dlist[vc4_state->ptr0_offset] = addr; } static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = { @@ -354,8 +359,82 @@ static void vc4_plane_destroy(struct drm_plane *plane) drm_plane_cleanup(plane); } +/* Implements immediate (non-vblank-synced) updates of the cursor + * position, or falls back to the atomic helper otherwise. + */ +static int +vc4_update_plane(struct
[PATCH 03/10] drm/vc4: Move the plane clipping/scaling setup to a separate function.
As we add actual scaling, this is going to get way more complicated. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_plane.c | 78 +++-- 1 file changed, 52 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index ed07ee5..554ed54 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -40,6 +40,14 @@ struct vc4_plane_state { * hardware at vc4_crtc_atomic_flush() time. */ u32 __iomem *hw_dlist; + + /* Clipped coordinates of the plane on the display. */ + int crtc_x, crtc_y, crtc_w, crtc_h; + + /* Offset to start scanning out from the start of the plane's +* BO. +*/ + u32 offset; }; static inline struct vc4_plane_state * @@ -151,22 +159,17 @@ static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) vc4_state->dlist[vc4_state->dlist_count++] = val; } -/* Writes out a full display list for an active plane to the plane's - * private dlist state. - */ -static int vc4_plane_mode_set(struct drm_plane *plane, - struct drm_plane_state *state) +static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) { struct vc4_plane_state *vc4_state = to_vc4_plane_state(state); struct drm_framebuffer *fb = state->fb; - struct drm_gem_cma_object *bo = drm_fb_cma_get_gem_obj(fb, 0); - u32 ctl0_offset = vc4_state->dlist_count; - const struct hvs_format *format = vc4_get_hvs_format(fb->pixel_format); - uint32_t offset = fb->offsets[0]; - int crtc_x = state->crtc_x; - int crtc_y = state->crtc_y; - int crtc_w = state->crtc_w; - int crtc_h = state->crtc_h; + + vc4_state->offset = fb->offsets[0]; + + vc4_state->crtc_x = state->crtc_x; + vc4_state->crtc_y = state->crtc_y; + vc4_state->crtc_w = state->crtc_w; + vc4_state->crtc_h = state->crtc_h; if (state->crtc_w << 16 != state->src_w || state->crtc_h << 16 != state->src_h) { @@ -178,18 +181,41 @@ static int vc4_plane_mode_set(struct drm_plane *plane, return -EINVAL; } - if (crtc_x < 0) { - offset += drm_format_plane_cpp(fb->pixel_format, 0) * -crtc_x; - crtc_w += crtc_x; - crtc_x = 0; + if (vc4_state->crtc_x < 0) { + vc4_state->offset += (drm_format_plane_cpp(fb->pixel_format, + 0) * + -vc4_state->crtc_x); + vc4_state->crtc_w += vc4_state->crtc_x; + vc4_state->crtc_x = 0; } - if (crtc_y < 0) { - offset += fb->pitches[0] * -crtc_y; - crtc_h += crtc_y; - crtc_y = 0; + if (vc4_state->crtc_y < 0) { + vc4_state->offset += fb->pitches[0] * -vc4_state->crtc_y; + vc4_state->crtc_h += vc4_state->crtc_y; + vc4_state->crtc_y = 0; } + return 0; +} + + +/* Writes out a full display list for an active plane to the plane's + * private dlist state. + */ +static int vc4_plane_mode_set(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct vc4_plane_state *vc4_state = to_vc4_plane_state(state); + struct drm_framebuffer *fb = state->fb; + struct drm_gem_cma_object *bo = drm_fb_cma_get_gem_obj(fb, 0); + u32 ctl0_offset = vc4_state->dlist_count; + const struct hvs_format *format = vc4_get_hvs_format(fb->pixel_format); + int ret; + + ret = vc4_plane_setup_clipping_and_scaling(state); + if (ret) + return ret; + vc4_dlist_write(vc4_state, SCALER_CTL0_VALID | (format->pixel_order << SCALER_CTL0_ORDER_SHIFT) | @@ -199,8 +225,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane, /* Position Word 0: Image Positions and Alpha Value */ vc4_dlist_write(vc4_state, VC4_SET_FIELD(0xff, SCALER_POS0_FIXED_ALPHA) | - VC4_SET_FIELD(crtc_x, SCALER_POS0_START_X) | - VC4_SET_FIELD(crtc_y, SCALER_POS0_START_Y)); + VC4_SET_FIELD(vc4_state->crtc_x, SCALER_POS0_START_X) | + VC4_SET_FIELD(vc4_state->crtc_y, SCALER_POS0_START_Y)); /* Position Word 1: Scaled Image Dimensions. * Skipped due to SCALER_CTL0_UNITY scaling. @@ -212,8 +238,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane, SCALER_POS2_ALPHA_MODE_PIPELINE : SCALER_POS2_ALPHA_MODE_FIXED, SCALER_POS2_ALPHA_MODE) | - VC4_SET_FIELD(crtc_w, SCALER_POS2_WIDTH) | - VC4_SET_FIELD(crtc_h, SCALER_PO
[PATCH 02/10] drm/vc4: Add missing __iomem annotation to hw_dlist.
This is the pointer to the HVS device's memory where we stored the contents of *dlist. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 45e353d..ed07ee5 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -39,7 +39,7 @@ struct vc4_plane_state { /* Offset where the plane's dlist was last stored in the * hardware at vc4_crtc_atomic_flush() time. */ - u32 *hw_dlist; + u32 __iomem *hw_dlist; }; static inline struct vc4_plane_state * -- 2.7.0
[PATCH 01/10] drm/vc4: Improve comments on vc4_plane_state members.
Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_plane.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 0addbad..45e353d 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -26,16 +26,19 @@ struct vc4_plane_state { struct drm_plane_state base; + /* System memory copy of the display list for this element, computed +* at atomic_check time. +*/ u32 *dlist; - u32 dlist_size; /* Number of dwords in allocated for the display list */ + u32 dlist_size; /* Number of dwords allocated for the display list */ u32 dlist_count; /* Number of used dwords in the display list. */ /* Offset in the dlist to pointer word 0. */ u32 pw0_offset; /* Offset where the plane's dlist was last stored in the - hardware at vc4_crtc_atomic_flush() time. - */ +* hardware at vc4_crtc_atomic_flush() time. +*/ u32 *hw_dlist; }; -- 2.7.0
[PATCH 00/10] vc4: scaling and YUV overlays for 4.6
Here's the major series I have currently for 4.6: Overlay plane support. There's definitely more work to be done here, on better filter kernels at various sizes, choosing our scaling modes better, and being more aggressive on LBM allocation (right now I think we're barely allocating for YUV 1:1 at 1920x1080, but it could do more than that). However, I think this is a good base to build from and may unblock people trying to start building software stacks on the open driver. Eric Anholt (10): drm/vc4: Improve comments on vc4_plane_state members. drm/vc4: Add missing __iomem annotation to hw_dlist. drm/vc4: Move the plane clipping/scaling setup to a separate function. drm/vc4: Add a proper short-circut path for legacy cursor updates. drm/vc4: Make the CRTCs cooperate on allocating display lists. drm/vc4: Add more display planes to each CRTC. drm/vc4: Fix which value is being used for source image size. drm/vc4: Add support for scaling of display planes. drm/vc4: Add support a few more RGB display plane formats. drm/vc4: Add support for YUV planes. drivers/gpu/drm/vc4/vc4_crtc.c | 171 +++- drivers/gpu/drm/vc4/vc4_drv.h | 12 +- drivers/gpu/drm/vc4/vc4_hvs.c | 98 +++ drivers/gpu/drm/vc4/vc4_kms.c | 9 + drivers/gpu/drm/vc4/vc4_plane.c | 598 drivers/gpu/drm/vc4/vc4_regs.h | 102 ++- 6 files changed, 868 insertions(+), 122 deletions(-) -- 2.7.0
[PATCH 1/7] tests/amdgpu: make amdgpu_sdma_test_exec_cs() generic (v2)
Share with upcoming CP tests. v2: drop unnecessary forward declaration Reviewed-by: Ken Wang Signed-off-by: Alex Deucher --- tests/amdgpu/basic_tests.c | 51 +- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c index fa0ed12..df5f5bc 100644 --- a/tests/amdgpu/basic_tests.c +++ b/tests/amdgpu/basic_tests.c @@ -542,11 +542,12 @@ static void amdgpu_command_submission_compute(void) * pm4_src, resources, ib_info, and ibs_request * submit command stream described in ibs_request and wait for this IB accomplished */ -static void amdgpu_sdma_test_exec_cs(amdgpu_context_handle context_handle, -int instance, int pm4_dw, uint32_t *pm4_src, -int res_cnt, amdgpu_bo_handle *resources, -struct amdgpu_cs_ib_info *ib_info, -struct amdgpu_cs_request *ibs_request) +static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, + unsigned ip_type, + int instance, int pm4_dw, uint32_t *pm4_src, + int res_cnt, amdgpu_bo_handle *resources, + struct amdgpu_cs_ib_info *ib_info, + struct amdgpu_cs_request *ibs_request) { int r; uint32_t expired; @@ -579,7 +580,7 @@ static void amdgpu_sdma_test_exec_cs(amdgpu_context_handle context_handle, ib_info->ib_mc_address = ib_result_mc_address; ib_info->size = pm4_dw; - ibs_request->ip_type = AMDGPU_HW_IP_DMA; + ibs_request->ip_type = ip_type; ibs_request->ring = instance; ibs_request->number_of_ibs = 1; ibs_request->ibs = ib_info; @@ -601,7 +602,7 @@ static void amdgpu_sdma_test_exec_cs(amdgpu_context_handle context_handle, r = amdgpu_bo_list_destroy(ibs_request->resources); CU_ASSERT_EQUAL(r, 0); - fence_status.ip_type = AMDGPU_HW_IP_DMA; + fence_status.ip_type = ip_type; fence_status.ring = ibs_request->ring; fence_status.context = context_handle; fence_status.fence = ibs_request->seq_no; @@ -676,10 +677,11 @@ static void amdgpu_command_submission_sdma_write_linear(void) while(j++ < sdma_write_length) pm4[i++] = 0xdeadbeaf; - amdgpu_sdma_test_exec_cs(context_handle, 0, - i, pm4, - 1, resources, - ib_info, ibs_request); + amdgpu_test_exec_cs_helper(context_handle, + AMDGPU_HW_IP_DMA, 0, + i, pm4, + 1, resources, + ib_info, ibs_request); /* verify if SDMA test result meets with expected */ i = 0; @@ -759,10 +761,11 @@ static void amdgpu_command_submission_sdma_const_fill(void) pm4[i++] = 0xdeadbeaf; pm4[i++] = sdma_write_length; - amdgpu_sdma_test_exec_cs(context_handle, 0, - i, pm4, - 1, resources, - ib_info, ibs_request); + amdgpu_test_exec_cs_helper(context_handle, + AMDGPU_HW_IP_DMA, 0, + i, pm4, + 1, resources, + ib_info, ibs_request); /* verify if SDMA test result meets with expected */ i = 0; @@ -860,10 +863,11 @@ static void amdgpu_command_submission_sdma_copy_linear(void) pm4[i++] = (0x & bo2_mc) >> 32; - amdgpu_sdma_test_exec_cs(context_handle, 0, - i, pm4, - 2, resources, - ib_info, ibs_request); + amdgpu_test_exec_cs_helper(context_handle, + AMDGPU_HW_IP_DMA, 0, + i, pm4, + 2, resources, + ib_info, ibs_request); /* verify if SDMA test result meets with expected */ i = 0; @@ -954,10 +958,11 @@ static void amdgpu_userptr_test(void) while (j++ < sdma_write_length) pm4[i++] = 0xdeadbeaf; - amdgpu_sdma_test_exec_cs(context_handle, 0, -i, pm4, -
[PATCH v4 2/2] drm: sunxi: Add a basic DRM driver for Allwinner DE2
On Tue, 2 Feb 2016 15:50:36 -0600 Rob Herring wrote: > > diff --git a/Documentation/devicetree/bindings/display/sunxi.txt > > b/Documentation/devicetree/bindings/display/sunxi.txt > > new file mode 100644 > > index 000..35f9763 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/sunxi.txt > > @@ -0,0 +1,81 @@ > > +Allwinner sunxi display subsystem > > += > > + > > +The sunxi display subsystems contain a display controller (DE), > > +one or two LCD controllers (TCON) and their external interfaces. > > + > > +Display controller > > +== > > + > > +Required properties: > > + > > +- compatible: value should be one of the following > > + "allwinner,sun8i-h3-display-engine" > > + > > +- clocks: must include clock specifiers corresponding to entries in the > > + clock-names property. > > + > > +- clock-names: must contain > > + gate: for DE activation > > + clock: DE clock > > + > > +- resets: phandle to the reset of the device > > + > > +- ports: phandle's to the LCD ports > > + > > +LCD controller > > +== > > + > > +Required properties: > > + > > +- compatible: value should be one of the following > > + "allwinner,sun8i-h3-lcd" > > + > > +- clocks: must include clock specifiers corresponding to entries in the > > + clock-names property. > > + > > +- clock-names: must contain > > + gate: for LCD activation > > + clock: pixel clock > > + > > +- resets: phandle to the reset of the device > > + > > +- port: port node with endpoint definitions as defined in > > + Documentation/devicetree/bindings/media/video-interfaces.txt > > Define how many ports and endpoints. As told in some other mail, such a video binding should be generic. The number of ports depends on the hardware. Most SoCs have only one port per LCD, but some other have many ports as the A83T (3 ports from the LCD0). > > + > > +Example: > > + > > + de: de-controller at 0100 { > > + compatible = "allwinner,sun8i-h3-display-engine"; > > + ... > > + clocks = <&bus_gates 44>, <&de_clk>; > > + clock-names = "gate", "clock"; > > + resets = <&ahb_rst 44>; > > + ports = <&lcd0_p>; > > This is pointless if you only have one item in ports. Is this really a > separate h/w block? Can't you move all this into the node below? Well, this example is extracted from the H3 where the DE and LCDs have different clocks, I/O resources and functions (the DE builds the video frames while the LCDs - TCONs - convert them to video flows). The example is not complete because there are 2 LCDs in the H3: ports = <&lcd0_p>, /* HDMI */ <&lcd1_p>; /* TV (CVBS) */ But, for SoCs with only one LCD, the description would be the same. > > + }; > > + > > + lcd0: lcd-controller at 01c0c000 { > > + compatible = "allwinner,sun8i-h3-lcd"; > > + ... > > + clocks = <&bus_gates 35>, <&tcon0_clk>; > > + clock-names = "gate", "clock"; > > + resets = <&ahb_rst 35>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + lcd0_p: port { > > + lcd0_ep: endpoint { > > + remote-endpoint = <&hdmi_ep>; > > + }; > > + }; > > + }; -- Ken ar c'hentañ| ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/
[git pull] drm amd acp audio support - optional
On Wed, Feb 3, 2016 at 4:04 PM, Dave Airlie wrote: > > I'm happy enough it shouldn't cause any problems, just wanted to > check if you'd take it now or not. Honestly, this feels like "might as well wait for 4.6" to me. I'm generally ok with taking completely new drivers later in the rc if there's a good reason for it, but then I want to feel like there's no way that can regress (because it doesn't impact an existing driver), and I also tend to only do that if the driver is something critical for new machines (ie a new network or video driver can be the difference between "that machine is usable" and "that machine is a doorstop"). This fails both those guidelines - it *might* screw up machines that work now, and while audio certainly isn't unimportant, it generally isn't a "you can't use the machine for anything" kind of issue. Thus the "I'd rather get this in a few weeks during the next merge window" reaction. Linus
[PATCH libdrm] android: enable building static version of libdrm
On Thu, Feb 4, 2016 at 7:30 AM, Emil Velikov wrote: > On 29 January 2016 at 16:00, Rob Herring wrote: >> From: Sumit Semwal >> >> Android needs libdrm built statically for recovery; >> enable that as well. >> >> Signed-off-by: Sumit Semwal >> Signed-off-by: Rob Herring >> Cc: Chih-Wei Huang >> Cc: Emil Velikov >> --- >> I posted this to mesa-dev, but I guess libdrm patches are supposed to go >> to dri-devel. >> > Thanks Rob. > > I've pushed all outstanding libdrm patches that I could find. Do let > me know if I missed anything. Thanks. That's the only one I had for libdrm. Rob
[PATCH 02/10] drm/exynos: ipp: fix incorrect format specifiers in debug messages
On 04.02.2016 11:34, Inki Dae wrote: > > > 2016ë 02ì 04ì¼ 11:17ì Krzysztof Kozlowski ì´(ê°) ì´ ê¸: >> On 03.02.2016 21:42, Marek Szyprowski wrote: >>> Drivers should use %p for printing pointers instead of hardcoding them >>> as hexadecimal integers. This patch fixes compilation warnings on 64bit >>> architectures. >>> >>> Signed-off-by: Marek Szyprowski >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_fimc.c| 2 +- >>> drivers/gpu/drm/exynos/exynos_drm_gsc.c | 2 +- >>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 32 >>> ++--- >>> drivers/gpu/drm/exynos/exynos_drm_rotator.c | 2 +- >>> 4 files changed, 19 insertions(+), 19 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c >>> b/drivers/gpu/drm/exynos/exynos_drm_fimc.c >>> index c747824..8a4f4a0 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c >>> @@ -1723,7 +1723,7 @@ static int fimc_probe(struct platform_device *pdev) >>> goto err_put_clk; >>> } >>> >>> - DRM_DEBUG_KMS("id[%d]ippdrv[0x%x]\n", ctx->id, (int)ippdrv); >>> + DRM_DEBUG_KMS("id[%d]ippdrv[%p]\n", ctx->id, ippdrv); >> >> I don't oppose the patch itself but I have different concern. First - >> probably you meant %pK because this is a writeable structure with >> function pointers. >> Second - why the ippdrv has to be printed? Is it useful for debugging? > > Marek fixed just compilation warnings on ARM64 so yes it wouldn't need to > print out ippdrv address but as other cleanup patch. Another patch? There is no point in a flow like this: 1. Make a patch which changes %x to %p. 2. Make a second patch right after the first one which changes %p to %pK. 3. Optionally make a third patch removing %pK entirely. There are two issues here (and in other places like in patch 3/10) - 64bit compilation warnings and security related issues. Properly fixing security related issues would fix in the same time the compilation warnings... Best regards, Krzysztof
[PATCH 02/10] drm/exynos: ipp: fix incorrect format specifiers in debug messages
2016ë 02ì 04ì¼ 11:17ì Krzysztof Kozlowski ì´(ê°) ì´ ê¸: > On 03.02.2016 21:42, Marek Szyprowski wrote: >> Drivers should use %p for printing pointers instead of hardcoding them >> as hexadecimal integers. This patch fixes compilation warnings on 64bit >> architectures. >> >> Signed-off-by: Marek Szyprowski >> --- >> drivers/gpu/drm/exynos/exynos_drm_fimc.c| 2 +- >> drivers/gpu/drm/exynos/exynos_drm_gsc.c | 2 +- >> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 32 >> ++--- >> drivers/gpu/drm/exynos/exynos_drm_rotator.c | 2 +- >> 4 files changed, 19 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c >> b/drivers/gpu/drm/exynos/exynos_drm_fimc.c >> index c747824..8a4f4a0 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c >> @@ -1723,7 +1723,7 @@ static int fimc_probe(struct platform_device *pdev) >> goto err_put_clk; >> } >> >> -DRM_DEBUG_KMS("id[%d]ippdrv[0x%x]\n", ctx->id, (int)ippdrv); >> +DRM_DEBUG_KMS("id[%d]ippdrv[%p]\n", ctx->id, ippdrv); > > I don't oppose the patch itself but I have different concern. First - > probably you meant %pK because this is a writeable structure with > function pointers. > Second - why the ippdrv has to be printed? Is it useful for debugging? Marek fixed just compilation warnings on ARM64 so yes it wouldn't need to print out ippdrv address but as other cleanup patch. Thanks, Inki Dae > > Best regards, > Krzysztof > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > >
[PATCH 21/21] drm/tilcdc: Disable sync lost interrupt if it fires on every frame
Disable the sync lost interrupt if it fires on every frame for 50 consecutive frames in a row. This is relatively sure sign of the sync lost interrupt being stuck and firing on every frame even if the display otherwise appears to work OK. Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 87c4765..e886277 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -43,6 +43,9 @@ struct tilcdc_crtc { /* Only set if an external encoder is connected */ bool simulate_vesa_sync; + + int sync_lost_count; + bool frame_intact; }; #define to_tilcdc_crtc(x) container_of(x, struct tilcdc_crtc, base) @@ -662,6 +665,8 @@ out: pm_runtime_put_sync(dev->dev); } +#define SYNC_LOST_COUNT_LIMIT 50 + irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) { struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); @@ -707,6 +712,11 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) spin_unlock_irqrestore(&dev->event_lock, flags); } + + if (tilcdc_crtc->frame_intact) + tilcdc_crtc->sync_lost_count = 0; + else + tilcdc_crtc->frame_intact = true; } if (priv->rev == 2) { @@ -717,9 +727,18 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) tilcdc_write(dev, LCDC_END_OF_INT_IND_REG, 0); } - if (stat & LCDC_SYNC_LOST) + if (stat & LCDC_SYNC_LOST) { dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost", __func__, stat); + tilcdc_crtc->frame_intact = false; + if (tilcdc_crtc->sync_lost_count++ > SYNC_LOST_COUNT_LIMIT) { + dev_err(dev->dev, + "%s(0x%08x): Sync lost flood detected, disabling the interrupt", + __func__, stat); + tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, +LCDC_SYNC_LOST); + } + } if (stat & LCDC_FIFO_UNDERFLOW) dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underfow", -- 1.9.1
[PATCH 20/21] drm/tilcdc: Add prints on sync lost and FIFO underrun interrupts
Add ratelimited prints on sync lost and FIFO underrun interrupts. Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 8 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 7445356..87c4765 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -717,6 +717,14 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) tilcdc_write(dev, LCDC_END_OF_INT_IND_REG, 0); } + if (stat & LCDC_SYNC_LOST) + dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost", + __func__, stat); + + if (stat & LCDC_FIFO_UNDERFLOW) + dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underfow", + __func__, stat); + return IRQ_HANDLED; } diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index be5c4f1..209c748 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -396,7 +396,7 @@ static int tilcdc_irq_postinstall(struct drm_device *dev) tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG, LCDC_V2_UNDERFLOW_INT_ENA | LCDC_V2_END_OF_FRAME0_INT_ENA | - LCDC_FRAME_DONE); + LCDC_FRAME_DONE | LCDC_SYNC_LOST); } return 0; @@ -415,7 +415,7 @@ static void tilcdc_irq_uninstall(struct drm_device *dev) tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, LCDC_V2_UNDERFLOW_INT_ENA | LCDC_V2_PL_INT_ENA | LCDC_V2_END_OF_FRAME0_INT_ENA | - LCDC_FRAME_DONE); + LCDC_FRAME_DONE | LCDC_SYNC_LOST); } } -- 1.9.1
[PATCH 19/21] drm/tilcdc: Remove the duplicate LCDC_INT_ENABLE_SET_REG in registers[]
Removes the duplicate LCDC_INT_ENABLE_SET_REG-entry in registers array. Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 8ae19e3..be5c4f1 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -453,11 +453,10 @@ static const struct { /* new in revision 2: */ REG(2, false, LCDC_RAW_STAT_REG), REG(2, false, LCDC_MASKED_STAT_REG), - REG(2, false, LCDC_INT_ENABLE_SET_REG), + REG(2, true, LCDC_INT_ENABLE_SET_REG), REG(2, false, LCDC_INT_ENABLE_CLR_REG), REG(2, false, LCDC_END_OF_INT_IND_REG), REG(2, true, LCDC_CLK_ENABLE_REG), - REG(2, true, LCDC_INT_ENABLE_SET_REG), #undef REG }; -- 1.9.1
[PATCH 18/21] drm/tilcdc: Fix interrupt enable/disable code for version 2 tilcdc
Fix interrupt enable/disable code for version 2 tilcdc. In version 2 tilcdc there is a separate register for disabling interrupts. Writing 0 to enable registers bits does not have any effect. The interrupt clear register works the same way, writing 1 to specific bit disables the interrupt and writing 0 does not have any effect. The "bug" that is fixed here does not really do any harm since the interrupts are enabled only once in the power up and disabled before power down. Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 41c8c96..8ae19e3 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -390,13 +390,14 @@ static int tilcdc_irq_postinstall(struct drm_device *dev) struct tilcdc_drm_private *priv = dev->dev_private; /* enable FIFO underflow irq: */ - if (priv->rev == 1) + if (priv->rev == 1) { tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_UNDERFLOW_INT_ENA); - else - tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG, + } else { + tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG, LCDC_V2_UNDERFLOW_INT_ENA | LCDC_V2_END_OF_FRAME0_INT_ENA | LCDC_FRAME_DONE); + } return 0; } @@ -411,7 +412,7 @@ static void tilcdc_irq_uninstall(struct drm_device *dev) LCDC_V1_UNDERFLOW_INT_ENA | LCDC_V1_PL_INT_ENA); tilcdc_clear(dev, LCDC_DMA_CTRL_REG, LCDC_V1_END_OF_FRAME_INT_ENA); } else { - tilcdc_clear(dev, LCDC_INT_ENABLE_SET_REG, + tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, LCDC_V2_UNDERFLOW_INT_ENA | LCDC_V2_PL_INT_ENA | LCDC_V2_END_OF_FRAME0_INT_ENA | LCDC_FRAME_DONE); @@ -456,7 +457,7 @@ static const struct { REG(2, false, LCDC_INT_ENABLE_CLR_REG), REG(2, false, LCDC_END_OF_INT_IND_REG), REG(2, true, LCDC_CLK_ENABLE_REG), - REG(2, true, LCDC_INT_ENABLE_SET_REG), + REG(2, true, LCDC_INT_ENABLE_SET_REG), #undef REG }; -- 1.9.1
[PATCH 17/21] drm/tilcdc: Do not update the next frame buffer close to vertical blank
From: Tomi Valkeinen Do not update the next frame buffer close to vertical blank. This is to avoid situation when the frame changes between writing of LCDC_DMA_FB_BASE_ADDR_0_REG and LCDC_DMA_FB_CEILING_ADDR_0_REG. Signed-off-by: Tomi Valkeinen [Added description to the patch] Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 61 +++- 1 file changed, 53 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 7514d40..7445356 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -21,6 +21,8 @@ #include "tilcdc_drv.h" #include "tilcdc_regs.h" +#define TILCDC_VBLANK_SAFETY_THRESHOLD_US 1000 + struct tilcdc_crtc { struct drm_crtc base; @@ -29,8 +31,12 @@ struct tilcdc_crtc { int dpms; wait_queue_head_t frame_done_wq; bool frame_done; + spinlock_t irq_lock; + + ktime_t last_vblank; struct drm_framebuffer *curr_fb; + struct drm_framebuffer *next_fb; /* for deferred fb unref's: */ struct drm_flip_work unref_work; @@ -146,6 +152,8 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; int r; unsigned long flags; + s64 tdiff; + ktime_t next_vblank; r = tilcdc_verify_fb(crtc, fb); if (r) @@ -162,12 +170,21 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc, pm_runtime_get_sync(dev->dev); + spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags); - set_scanout(crtc, fb); + next_vblank = ktime_add_us(tilcdc_crtc->last_vblank, + 100 / crtc->hwmode.vrefresh); + + tdiff = ktime_to_us(ktime_sub(next_vblank, ktime_get())); + + if (tdiff >= TILCDC_VBLANK_SAFETY_THRESHOLD_US) + set_scanout(crtc, fb); + else + tilcdc_crtc->next_fb = fb; - spin_lock_irqsave(&dev->event_lock, flags); tilcdc_crtc->event = event; - spin_unlock_irqrestore(&dev->event_lock, flags); + + spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags); pm_runtime_put_sync(dev->dev); @@ -211,6 +228,12 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode) pm_runtime_put_sync(dev->dev); + if (tilcdc_crtc->next_fb) { + drm_flip_work_queue(&tilcdc_crtc->unref_work, + tilcdc_crtc->next_fb); + tilcdc_crtc->next_fb = NULL; + } + if (tilcdc_crtc->curr_fb) { drm_flip_work_queue(&tilcdc_crtc->unref_work, tilcdc_crtc->curr_fb); @@ -651,19 +674,39 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) if (stat & LCDC_END_OF_FRAME0) { unsigned long flags; + bool skip_event = false; + ktime_t now; + + now = ktime_get(); drm_flip_work_commit(&tilcdc_crtc->unref_work, priv->wq); + spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags); + + tilcdc_crtc->last_vblank = now; + + if (tilcdc_crtc->next_fb) { + set_scanout(crtc, tilcdc_crtc->next_fb); + tilcdc_crtc->next_fb = NULL; + skip_event = true; + } + + spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags); + drm_handle_vblank(dev, 0); - spin_lock_irqsave(&dev->event_lock, flags); + if (!skip_event) { + struct drm_pending_vblank_event *event; + + spin_lock_irqsave(&dev->event_lock, flags); - if (tilcdc_crtc->event) { - drm_send_vblank_event(dev, 0, tilcdc_crtc->event); + event = tilcdc_crtc->event; tilcdc_crtc->event = NULL; - } + if (event) + drm_send_vblank_event(dev, 0, event); - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irqrestore(&dev->event_lock, flags); + } } if (priv->rev == 2) { @@ -716,6 +759,8 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev) drm_flip_work_init(&tilcdc_crtc->unref_work, "unref", unref_worker); + spin_lock_init(&tilcdc_crtc->irq_lock); + ret = drm_crtc_init(dev, crtc, &tilcdc_crtc_funcs); if (ret < 0) goto fail; -- 1.9.1
[PATCH 16/21] drm/tilcdc: Get rid of complex ping-pong mechanism
From: Tomi Valkeinen Get rid of complex ping-pong mechanism and replace it with simpler single buffer flipping code. The LCDC HW appears to be designed mainly static framebuffers in mind. There are two modes of operation, either static single buffer, or ping pong double buffering with two static buffers switching back and forth. Luckily the framebuffer start address is fetched only in the beginning of the frame and changing the address after that only takes effect after the next vertical blank. The page flipping code can simply write the address of the new framebuffer and the page is flipped automatically after the next vertical blank. Using the ping pong double buffering makes the flipping code way more complex and it does not provide any benefit, so it is better to switch to single buffer operation. There is still one problem in updating the framebuffer dma address on the fly. There are two registers defining the framebuffer dma area and things may break if the dma address is fetched in while the registers are are being updated. Signed-off-by: Tomi Valkeinen [Added description to the patch] Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 122 ++- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 27 +--- 2 files changed, 53 insertions(+), 96 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 280c76a..7514d40 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -25,15 +25,12 @@ struct tilcdc_crtc { struct drm_crtc base; const struct tilcdc_panel_info *info; - uint32_t dirty; - dma_addr_t start, end; struct drm_pending_vblank_event *event; int dpms; wait_queue_head_t frame_done_wq; bool frame_done; - /* fb currently set to scanout 0/1: */ - struct drm_framebuffer *scanout[2]; + struct drm_framebuffer *curr_fb; /* for deferred fb unref's: */ struct drm_flip_work unref_work; @@ -54,62 +51,31 @@ static void unref_worker(struct drm_flip_work *work, void *val) mutex_unlock(&dev->mode_config.mutex); } -static void set_scanout(struct drm_crtc *crtc, int n) +static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb) { - static const uint32_t base_reg[] = { - LCDC_DMA_FB_BASE_ADDR_0_REG, - LCDC_DMA_FB_BASE_ADDR_1_REG, - }; - static const uint32_t ceil_reg[] = { - LCDC_DMA_FB_CEILING_ADDR_0_REG, - LCDC_DMA_FB_CEILING_ADDR_1_REG, - }; - static const uint32_t stat[] = { - LCDC_END_OF_FRAME0, LCDC_END_OF_FRAME1, - }; struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); struct drm_device *dev = crtc->dev; - struct tilcdc_drm_private *priv = dev->dev_private; - - tilcdc_write(dev, base_reg[n], tilcdc_crtc->start); - tilcdc_write(dev, ceil_reg[n], tilcdc_crtc->end); - if (tilcdc_crtc->scanout[n]) { - drm_flip_work_queue(&tilcdc_crtc->unref_work, tilcdc_crtc->scanout[n]); - drm_flip_work_commit(&tilcdc_crtc->unref_work, priv->wq); - } - tilcdc_crtc->scanout[n] = crtc->primary->fb; - drm_framebuffer_reference(tilcdc_crtc->scanout[n]); - tilcdc_crtc->dirty &= ~stat[n]; -} - -static void update_scanout(struct drm_crtc *crtc) -{ - struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); - struct drm_device *dev = crtc->dev; - struct drm_framebuffer *fb = crtc->primary->fb; struct drm_gem_cma_object *gem; unsigned int depth, bpp; + dma_addr_t start, end; drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp); gem = drm_fb_cma_get_gem_obj(fb, 0); - tilcdc_crtc->start = gem->paddr + fb->offsets[0] + - (crtc->y * fb->pitches[0]) + (crtc->x * bpp/8); + start = gem->paddr + fb->offsets[0] + + crtc->y * fb->pitches[0] + + crtc->x * bpp / 8; - tilcdc_crtc->end = tilcdc_crtc->start + - (crtc->mode.vdisplay * fb->pitches[0]); + end = start + (crtc->mode.vdisplay * fb->pitches[0]); - if (tilcdc_crtc->dpms == DRM_MODE_DPMS_ON) { - /* already enabled, so just mark the frames that need -* updating and they will be updated on vblank: -*/ - tilcdc_crtc->dirty |= LCDC_END_OF_FRAME0 | LCDC_END_OF_FRAME1; - drm_vblank_get(dev, 0); - } else { - /* not enabled yet, so update registers immediately: */ - set_scanout(crtc, 0); - set_scanout(crtc, 1); - } + tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, start); + tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG, end); + + if (tilcdc_crtc->curr_fb) + drm_flip_work_queue(&tilc
[PATCH 15/21] drm/tilcdc: cleanup irq handling
From: Tomi Valkeinen Cleanup irq handling. Clear the irq status unconditionally and restructure the status bit conditions. Signed-off-by: Tomi Valkeinen [Added description to the patch] Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index fe2aed7..280c76a 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -656,11 +656,12 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); struct drm_device *dev = crtc->dev; struct tilcdc_drm_private *priv = dev->dev_private; - uint32_t stat = tilcdc_read_irqstatus(dev); + uint32_t stat; - if (stat & LCDC_PL_LOAD_DONE) { - tilcdc_clear_irqstatus(dev, stat); - } else { + stat = tilcdc_read_irqstatus(dev); + tilcdc_clear_irqstatus(dev, stat); + + if ((stat & LCDC_END_OF_FRAME0) || (stat & LCDC_END_OF_FRAME1)) { struct drm_pending_vblank_event *event; unsigned long flags; uint32_t dirty = tilcdc_crtc->dirty & stat; -- 1.9.1
[PATCH 14/21] drm/tilcdc: remove broken error handling
From: Tomi Valkeinen Remove broken error handling. The condition for handling the LCDC_SYNC_LOST and LCDC_FIFO_UNDERFLOW could never be satisfied as the LCDC_SYNC_LOST interrupt is not enabled. Also the requirement to have both LCDC_SYNC_LOST and LCDC_FIFO_UNDERFLOW fired at once before handling the error looks weird. Signed-off-by: Tomi Valkeinen [Added description to the patch] Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index ec7be2c..fe2aed7 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -658,12 +658,7 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) struct tilcdc_drm_private *priv = dev->dev_private; uint32_t stat = tilcdc_read_irqstatus(dev); - if ((stat & LCDC_SYNC_LOST) && (stat & LCDC_FIFO_UNDERFLOW)) { - stop(crtc); - dev_err(dev->dev, "error: %08x\n", stat); - tilcdc_clear_irqstatus(dev, stat); - start(crtc); - } else if (stat & LCDC_PL_LOAD_DONE) { + if (stat & LCDC_PL_LOAD_DONE) { tilcdc_clear_irqstatus(dev, stat); } else { struct drm_pending_vblank_event *event; -- 1.9.1
[PATCH 13/21] drm/tilcdc: split reset to a separate function
From: Tomi Valkeinen Split reset to a separate function and use usleep_range(250, 1000) instead of msleep(1) to to keep the reset bit on long enough. Signed-off-by: Tomi Valkeinen [Added description to the patch, changed mdelay(500) to usleep_range(250, 1000)] Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 3a6f11e..ec7be2c 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -112,17 +112,24 @@ static void update_scanout(struct drm_crtc *crtc) } } -static void start(struct drm_crtc *crtc) +static void reset(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct tilcdc_drm_private *priv = dev->dev_private; - if (priv->rev == 2) { - tilcdc_set(dev, LCDC_CLK_RESET_REG, LCDC_CLK_MAIN_RESET); - msleep(1); - tilcdc_clear(dev, LCDC_CLK_RESET_REG, LCDC_CLK_MAIN_RESET); - msleep(1); - } + if (priv->rev != 2) + return; + + tilcdc_set(dev, LCDC_CLK_RESET_REG, LCDC_CLK_MAIN_RESET); + usleep_range(250, 1000); + tilcdc_clear(dev, LCDC_CLK_RESET_REG, LCDC_CLK_MAIN_RESET); +} + +static void start(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + + reset(crtc); tilcdc_set(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE); tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_PALETTE_LOAD_MODE(DATA_ONLY)); -- 1.9.1
[PATCH 12/21] drm/tilcdc: disable crtc on unload
From: Tomi Valkeinen Disable crtc on unload. Call tilcdc_crtc_dpms() with DRM_MODE_DPMS_OFF in the beginning of unload function. Signed-off-by: Tomi Valkeinen [Added description to the patch] Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 715e160..33aa40d 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -112,6 +112,8 @@ static int tilcdc_unload(struct drm_device *dev) { struct tilcdc_drm_private *priv = dev->dev_private; + tilcdc_crtc_dpms(priv->crtc, DRM_MODE_DPMS_OFF); + tilcdc_remove_external_encoders(dev); drm_fbdev_cma_fini(priv->fbdev); -- 1.9.1
[PATCH 11/21] drm/tilcdc: cleanup runtime PM handling
From: Tomi Valkeinen Cleanup runtime PM handling. Before the patch the usage of pm_runtime calls was inconsistent and hard to follow. After the update the pm_runtime calls are removed from set_scanout() and called around major operations that access the HW. After the patch the DPMS code does not have pm_runtime_forbid/allow calls any more and pm_runtime_irq_safe() is not set anymore. Signed-off-by: Tomi Valkeinen [Added description to the patch] Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 19 +++ drivers/gpu/drm/tilcdc/tilcdc_drv.c | 1 - 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index cc45818..3a6f11e 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -71,7 +71,6 @@ static void set_scanout(struct drm_crtc *crtc, int n) struct drm_device *dev = crtc->dev; struct tilcdc_drm_private *priv = dev->dev_private; - pm_runtime_get_sync(dev->dev); tilcdc_write(dev, base_reg[n], tilcdc_crtc->start); tilcdc_write(dev, ceil_reg[n], tilcdc_crtc->end); if (tilcdc_crtc->scanout[n]) { @@ -81,7 +80,6 @@ static void set_scanout(struct drm_crtc *crtc, int n) tilcdc_crtc->scanout[n] = crtc->primary->fb; drm_framebuffer_reference(tilcdc_crtc->scanout[n]); tilcdc_crtc->dirty &= ~stat[n]; - pm_runtime_put_sync(dev->dev); } static void update_scanout(struct drm_crtc *crtc) @@ -186,8 +184,13 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc, crtc->primary->fb = fb; tilcdc_crtc->event = event; + + pm_runtime_get_sync(dev->dev); + update_scanout(crtc); + pm_runtime_put_sync(dev->dev); + return 0; } @@ -206,10 +209,8 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode) tilcdc_crtc->dpms = mode; - pm_runtime_get_sync(dev->dev); - if (mode == DRM_MODE_DPMS_ON) { - pm_runtime_forbid(dev->dev); + pm_runtime_get_sync(dev->dev); start(crtc); } else { tilcdc_crtc->frame_done = false; @@ -227,10 +228,9 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode) if (ret == 0) dev_err(dev->dev, "timeout waiting for framedone\n"); } - pm_runtime_allow(dev->dev); - } - pm_runtime_put_sync(dev->dev); + pm_runtime_put_sync(dev->dev); + } } static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc, @@ -455,13 +455,16 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc, static int tilcdc_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb) { + struct drm_device *dev = crtc->dev; int r; r = tilcdc_verify_fb(crtc, crtc->primary->fb); if (r) return r; + pm_runtime_get_sync(dev->dev); update_scanout(crtc); + pm_runtime_put_sync(dev->dev); return 0; } diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 9bc15e7..715e160 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -229,7 +229,6 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock); pm_runtime_enable(dev->dev); - pm_runtime_irq_safe(dev->dev); /* * disable creation of new console during suspend. -- 1.9.1
[PATCH 10/21] drm/tilcdc: Allocate register storage based on the actual number registers
Allocate suspend/resume register storage based on the actual number registers the driver is aware of. The static allocation for register storage had falen behind badly. Reported-by: Michael Bode Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 20 +++- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 5cbb250..9bc15e7 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -141,11 +141,14 @@ static int tilcdc_unload(struct drm_device *dev) pm_runtime_disable(dev->dev); + kfree(priv->saved_register); kfree(priv); return 0; } +static size_t tilcdc_num_regs(void); + static int tilcdc_load(struct drm_device *dev, unsigned long flags) { struct platform_device *pdev = dev->platformdev; @@ -157,7 +160,11 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) int ret; priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (!priv) { + if (priv) + priv->saved_register = kcalloc(sizeof(*priv->saved_register), + tilcdc_num_regs(), GFP_KERNEL); + if (!priv || !priv->saved_register) { + kfree(priv); dev_err(dev->dev, "failed to allocate private data\n"); return -ENOMEM; } @@ -347,6 +354,7 @@ fail_free_wq: fail_free_priv: dev->dev_private = NULL; + kfree(priv->saved_register); kfree(priv); return ret; } @@ -471,6 +479,16 @@ static const struct { REG(2, true, LCDC_INT_ENABLE_SET_REG), #undef REG }; + +static size_t tilcdc_num_regs(void) +{ + return ARRAY_SIZE(registers); +} +#else +static size_t tilcdc_num_regs(void) +{ + return 0; +} #endif #ifdef CONFIG_DEBUG_FS diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h index 77c600d..80badad 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h @@ -66,7 +66,7 @@ struct tilcdc_drm_private { uint32_t max_width; /* register contents saved across suspend/resume: */ - u32 saved_register[12]; + u32 *saved_register; bool ctx_valid; #ifdef CONFIG_CPU_FREQ -- 1.9.1
[PATCH 09/21] drm/tilcdc: fix build error when !CONFIG_CPU_FREQ
From: Grygorii Strashko Fix build error when !CONFIG_CPU_FREQ drivers/gpu/drm/tilcdc/tilcdc_drv.c: In function 'tilcdc_load': drivers/gpu/drm/tilcdc/tilcdc_drv.c:327:1: error: label 'fail_put_clk' defined but not used [-Werror=unused-label] fail_put_clk: ^ Signed-off-by: Grygorii Strashko Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index f947007..5cbb250 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -333,9 +333,9 @@ fail_cpufreq_unregister: #ifdef CONFIG_CPU_FREQ cpufreq_unregister_notifier(&priv->freq_transition, CPUFREQ_TRANSITION_NOTIFIER); -#endif fail_put_clk: +#endif clk_put(priv->clk); fail_iounmap: -- 1.9.1
[PATCH 08/21] drm/tilcdc: Implement dma-buf support for tilcdc
There is nothing special about tilcdc HW when the video memory is concerned. Just using the standard drm helpers for implementation is enough. Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index ef52731..f947007 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -557,7 +557,8 @@ static const struct file_operations fops = { }; static struct drm_driver tilcdc_driver = { - .driver_features= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET, + .driver_features= (DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET | + DRIVER_PRIME), .load = tilcdc_load, .unload = tilcdc_unload, .preclose = tilcdc_preclose, @@ -575,6 +576,16 @@ static struct drm_driver tilcdc_driver = { .dumb_create= drm_gem_cma_dumb_create, .dumb_map_offset= drm_gem_cma_dumb_map_offset, .dumb_destroy = drm_gem_dumb_destroy, + + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .gem_prime_import = drm_gem_prime_import, + .gem_prime_export = drm_gem_prime_export, + .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, + .gem_prime_vmap = drm_gem_cma_prime_vmap, + .gem_prime_vunmap = drm_gem_cma_prime_vunmap, + .gem_prime_mmap = drm_gem_cma_prime_mmap, #ifdef CONFIG_DEBUG_FS .debugfs_init = tilcdc_debugfs_init, .debugfs_cleanup= tilcdc_debugfs_cleanup, -- 1.9.1
[PATCH 07/21] drm/tilcdc: disable the lcd controller/dma engine when suspend invoked
From: Darren Etheridge The LCD controller must be deactivated and all DMA transactions stopped when the suspend power state is entered otherwise the PRCM causes the L3 bus to get stuck in transition state. This commit forces the lcdc to be shut down and waits for all pending DMA transactions to complete as part of the suspend handler for this driver. Signed-off-by: Darren Etheridge Tested-by: Dave Gerlach Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 3 +-- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +++ drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 105f286..cc45818 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -138,7 +138,6 @@ static void stop(struct drm_crtc *crtc) tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE); } -static void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode); static void tilcdc_crtc_destroy(struct drm_crtc *crtc) { struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); @@ -192,7 +191,7 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc, return 0; } -static void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode) +void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode) { struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); struct drm_device *dev = crtc->dev; diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 149befa..ef52731 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -608,6 +608,9 @@ static int tilcdc_pm_suspend(struct device *dev) return 0; } + /* Disable the LCDC controller, to avoid locking up the PRCM */ + tilcdc_crtc_dpms(priv->crtc, DRM_MODE_DPMS_OFF); + /* Save register state: */ for (i = 0; i < ARRAY_SIZE(registers); i++) if (registers[i].save && (priv->rev >= registers[i].rev)) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h index 7d214cc..77c600d 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h @@ -172,5 +172,6 @@ void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc *crtc, bool simulate_vesa_sync); int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode); int tilcdc_crtc_max_width(struct drm_crtc *crtc); +void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode); #endif /* __TILCDC_DRV_H__ */ -- 1.9.1
[PATCH 06/21] drm/tilcdc: make frame_done interrupt active at all times
From: Darren Etheridge The frame_done interrupt was only being enabled when the vsync interrupts were being enabled by DRM. However the frame_done is used to determine if the LCD controller has successfully completed the raster_enable, raster_disable commands and the vsync interrupts are not always enabled during these operations. Signed-off-by: Darren Etheridge Tested-by: Dave Gerlach Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 188c3ce..149befa 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -384,7 +384,9 @@ static int tilcdc_irq_postinstall(struct drm_device *dev) if (priv->rev == 1) tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_UNDERFLOW_INT_ENA); else - tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG, LCDC_V2_UNDERFLOW_INT_ENA); + tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG, + LCDC_V2_UNDERFLOW_INT_ENA | + LCDC_FRAME_DONE); return 0; } @@ -418,7 +420,7 @@ static void enable_vblank(struct drm_device *dev, bool enable) } else { reg = LCDC_INT_ENABLE_SET_REG; mask = LCDC_V2_END_OF_FRAME0_INT_ENA | - LCDC_V2_END_OF_FRAME1_INT_ENA | LCDC_FRAME_DONE; + LCDC_V2_END_OF_FRAME1_INT_ENA; } if (enable) -- 1.9.1
[PATCH 05/21] drm/tilcdc: fix kernel panic on suspend when no hdmi monitor connected
From: Darren Etheridge On BeagleBone Black if no HDMI monitor is connected and suspend is requested a kernel panic will result: root at am335x-evm:~# echo mem > /sys/power/state [ 65.548710] PM: Syncing filesystems ... done. [ 65.631311] Freezing user space processes ... (elapsed 0.006 seconds) done. [ 65.648619] Freezing remaining freezable tasks ... (elapsed 0.005 seconds) done. [ 65.833500] Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa30e004 [ 65.841692] Internal error: : 1028 [#1] SMP ARM [ 66.105287] [] (platform_pm_suspend) from [] (dpm_run_callback+0x34/0x70) [ 66.114370] [] (dpm_run_callback) from [] (__device_suspend+0x10c/0x2f4) [ 66.123357] [] (__device_suspend) from [] (dpm_suspend+0x58/0x218) [ 66.131796] [] (dpm_suspend) from [] (suspend_devices_and_enter+0x9c/0x3c0) [ 66.141055] [] (suspend_devices_and_enter) from [] (pm_suspend+0x210/0x24c) [ 66.150312] [] (pm_suspend) from [] (state_store+0x68/0xb8) [ 66.158103] [] (state_store) from [] (kobj_attr_store+0x14/0x20) [ 66.166355] [] (kobj_attr_store) from [] (sysfs_kf_write+0x4c/0x50) [ 66.174883] [] (sysfs_kf_write) from [] (kernfs_fop_write+0xb4/0x150) [ 66.183598] [] (kernfs_fop_write) from [] (vfs_write+0xa8/0x180) [ 66.191846] [] (vfs_write) from [] (SyS_write+0x40/0x8c) [ 66.199365] [] (SyS_write) from [] (ret_fast_syscall+0x0/0x48) [ 66.207426] Code: e595c210 e5932000 e59cc000 e08c2002 (e592c000) This is because the lcdc module is not enabled when no monitor is detected to save power. However the suspend handler just blindly tries to save the lcdc state by copying out the pertinent registers. However module is off so no good things happen when you try and access it. This patch only saves off the registers if the module is enabled, and then only restores the registers on resume if they were saved off during suspend. Signed-off-by: Darren Etheridge Tested-by: Dave Gerlach Acked-by: Felipe Balbi Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 23 +-- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 + 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index e25db59..188c3ce 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -598,13 +598,20 @@ static int tilcdc_pm_suspend(struct device *dev) drm_kms_helper_poll_disable(ddev); + /* Select sleep pin state */ + pinctrl_pm_select_sleep_state(dev); + + if (pm_runtime_suspended(dev)) { + priv->ctx_valid = false; + return 0; + } + /* Save register state: */ for (i = 0; i < ARRAY_SIZE(registers); i++) if (registers[i].save && (priv->rev >= registers[i].rev)) priv->saved_register[n++] = tilcdc_read(ddev, registers[i].reg); - /* Select sleep pin state */ - pinctrl_pm_select_sleep_state(dev); + priv->ctx_valid = true; return 0; } @@ -618,10 +625,14 @@ static int tilcdc_pm_resume(struct device *dev) /* Select default pin state */ pinctrl_pm_select_default_state(dev); - /* Restore register state: */ - for (i = 0; i < ARRAY_SIZE(registers); i++) - if (registers[i].save && (priv->rev >= registers[i].rev)) - tilcdc_write(ddev, registers[i].reg, priv->saved_register[n++]); + if (priv->ctx_valid == true) { + /* Restore register state: */ + for (i = 0; i < ARRAY_SIZE(registers); i++) + if (registers[i].save && + (priv->rev >= registers[i].rev)) + tilcdc_write(ddev, registers[i].reg, +priv->saved_register[n++]); + } drm_kms_helper_poll_enable(ddev); diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h index c00f518..7d214cc 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h @@ -67,6 +67,7 @@ struct tilcdc_drm_private { /* register contents saved across suspend/resume: */ u32 saved_register[12]; + bool ctx_valid; #ifdef CONFIG_CPU_FREQ struct notifier_block freq_transition; -- 1.9.1
[PATCH 04/21] drm/tilcdc: adopt pinctrl support
From: Dave Gerlach Update tilcdc driver to set the state of the pins to: - "default on resume - "sleep" on suspend By optionally putting the pins into sleep state in the suspend callback we can accomplish two things. - minimize current leakage from pins and thus save power, - prevent the IP from driving pins output in an uncontrolled manner, which may happen if the power domain drops the domain regulator. Signed-off-by: Dave Gerlach Signed-off-by: Darren Etheridge Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index c6acce9..e25db59 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -18,6 +18,7 @@ /* LCDC DRM driver, based on da8xx-fb */ #include +#include #include #include "tilcdc_drv.h" @@ -602,6 +603,9 @@ static int tilcdc_pm_suspend(struct device *dev) if (registers[i].save && (priv->rev >= registers[i].rev)) priv->saved_register[n++] = tilcdc_read(ddev, registers[i].reg); + /* Select sleep pin state */ + pinctrl_pm_select_sleep_state(dev); + return 0; } @@ -611,6 +615,9 @@ static int tilcdc_pm_resume(struct device *dev) struct tilcdc_drm_private *priv = ddev->dev_private; unsigned i, n = 0; + /* Select default pin state */ + pinctrl_pm_select_default_state(dev); + /* Restore register state: */ for (i = 0; i < ARRAY_SIZE(registers); i++) if (registers[i].save && (priv->rev >= registers[i].rev)) -- 1.9.1
[PATCH 03/21] drm/tilcdc: verify fb pitch
From: Tomi Valkeinen LCDC hardware does not support fb pitch that is different (i.e. larger) than the screen size. The driver currently does no checks for this, and the results of too big pitch are are flickering and lower fps. This issue easily happens when using libdrm's modetest tool with non-32 bpp modes. As modetest always allocated 4 bytes per pixel, it implies a bigger pitch for 16 or 24 bpp modes. This patch adds a check to reject pitches the hardware cannot support. Signed-off-by: Tomi Valkeinen Signed-off-by: Darren Etheridge Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 7b687ae..105f286 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -151,6 +151,22 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc) kfree(tilcdc_crtc); } +static int tilcdc_verify_fb(struct drm_crtc *crtc, struct drm_framebuffer *fb) +{ + struct drm_device *dev = crtc->dev; + unsigned int depth, bpp; + + drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp); + + if (fb->pitches[0] != crtc->mode.hdisplay * bpp / 8) { + dev_err(dev->dev, + "Invalid pitch: fb and crtc widths must be the same"); + return -EINVAL; + } + + return 0; +} + static int tilcdc_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, @@ -158,6 +174,11 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc, { struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); struct drm_device *dev = crtc->dev; + int r; + + r = tilcdc_verify_fb(crtc, fb); + if (r) + return r; if (tilcdc_crtc->event) { dev_err(dev->dev, "already pending page flip!\n"); @@ -272,6 +293,10 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc, if (WARN_ON(!info)) return -EINVAL; + ret = tilcdc_verify_fb(crtc, crtc->primary->fb); + if (ret) + return ret; + pm_runtime_get_sync(dev->dev); /* Configure the Burst Size and fifo threshold of DMA: */ @@ -431,6 +456,12 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc, static int tilcdc_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb) { + int r; + + r = tilcdc_verify_fb(crtc, crtc->primary->fb); + if (r) + return r; + update_scanout(crtc); return 0; } -- 1.9.1
[PATCH 02/21] drm/tilcdc: rewrite pixel clock calculation
From: Darren Etheridge Updating the tilcdc DRM driver code to calculate the LCD controller pixel clock more accurately. Based on a suggested implementation by Tomi Valkeinen. The current code does not work correctly and produces wrong results with many requested clock rates. It also oddly uses two different clocks, a display pll clock and a divider clock (child of display pll), instead of just using the clock coming to the lcdc. This patch removes the use of the display pll clock, and rewrites the code to calculate the clock rates. The idea is simply to request a clock rate of pixelclock*2, as the LCD controller has an internal divider which we set to 2. Signed-off-by: Darren Etheridge [Rewrapped description] Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 11 +-- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 - 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 7d07733..7b687ae 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -573,7 +573,8 @@ void tilcdc_crtc_update_clk(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; struct tilcdc_drm_private *priv = dev->dev_private; int dpms = tilcdc_crtc->dpms; - unsigned int lcd_clk, div; + unsigned long lcd_clk; + const unsigned clkdiv = 2; /* using a fixed divider of 2 */ int ret; pm_runtime_get_sync(dev->dev); @@ -581,22 +582,21 @@ void tilcdc_crtc_update_clk(struct drm_crtc *crtc) if (dpms == DRM_MODE_DPMS_ON) tilcdc_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); - /* in raster mode, minimum divisor is 2: */ - ret = clk_set_rate(priv->disp_clk, crtc->mode.clock * 1000 * 2); - if (ret) { + /* mode.clock is in KHz, set_rate wants parameter in Hz */ + ret = clk_set_rate(priv->clk, crtc->mode.clock * 1000 * clkdiv); + if (ret < 0) { dev_err(dev->dev, "failed to set display clock rate to: %d\n", crtc->mode.clock); goto out; } lcd_clk = clk_get_rate(priv->clk); - div = lcd_clk / (crtc->mode.clock * 1000); - DBG("lcd_clk=%u, mode clock=%d, div=%u", lcd_clk, crtc->mode.clock, div); - DBG("fck=%lu, dpll_disp_ck=%lu", clk_get_rate(priv->clk), clk_get_rate(priv->disp_clk)); + DBG("lcd_clk=%lu, mode clock=%d, div=%u", + lcd_clk, crtc->mode.clock, clkdiv); /* Configure the LCD clock divisor. */ - tilcdc_write(dev, LCDC_CTRL_REG, LCDC_CLK_DIVISOR(div) | + tilcdc_write(dev, LCDC_CTRL_REG, LCDC_CLK_DIVISOR(clkdiv) | LCDC_RASTER_MODE); if (priv->rev == 2) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 90ed233..c6acce9 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -193,13 +193,6 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) goto fail_iounmap; } - priv->disp_clk = clk_get(dev->dev, "dpll_disp_ck"); - if (IS_ERR(priv->clk)) { - dev_err(dev->dev, "failed to get display clock\n"); - ret = -ENODEV; - goto fail_put_clk; - } - #ifdef CONFIG_CPU_FREQ priv->lcd_fck_rate = clk_get_rate(priv->clk); priv->freq_transition.notifier_call = cpufreq_transition; @@ -207,7 +200,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) CPUFREQ_TRANSITION_NOTIFIER); if (ret) { dev_err(dev->dev, "failed to register cpufreq notifier\n"); - goto fail_put_disp_clk; + goto fail_put_clk; } #endif @@ -339,8 +332,6 @@ fail_cpufreq_unregister: #ifdef CONFIG_CPU_FREQ cpufreq_unregister_notifier(&priv->freq_transition, CPUFREQ_TRANSITION_NOTIFIER); -fail_put_disp_clk: - clk_put(priv->disp_clk); #endif fail_put_clk: diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h index e863ad0..c00f518 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h @@ -49,7 +49,6 @@ struct tilcdc_drm_private { void __iomem *mmio; - struct clk *disp_clk;/* display dpll */ struct clk *clk; /* functional clock */ int rev; /* IP revision */ -- 1.9.1
[PATCH 01/21] drm/tilcdc: disable console switching during pm operations
From: Darren Etheridge The default behavior of consoles during power management operations is: On entry to suspend a new console is allocated and switched to. On resume the original console is restored. However this isn't the observed behavior and the original console is not restored. This commit avoids the problem by disabling the switching of consoles at suspend/resume. This works because the driver already restores all necessary hardware context during such pm operations. Signed-off-by: Darren Etheridge Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index d7f5b89..90ed233 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -18,6 +18,7 @@ /* LCDC DRM driver, based on da8xx-fb */ #include +#include #include "tilcdc_drv.h" #include "tilcdc_regs.h" @@ -229,6 +230,14 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) pm_runtime_enable(dev->dev); pm_runtime_irq_safe(dev->dev); + /* +* disable creation of new console during suspend. +* this works around a problem where a ctrl-c is needed +* to be entered on the VT to actually get the device +* to continue into the suspend state. +*/ + pm_set_vt_switch(0); + /* Determine LCD IP Version */ pm_runtime_get_sync(dev->dev); switch (tilcdc_read(dev, LCDC_PID_REG)) { -- 1.9.1
[PATCH 00/21] drm/ticdc: Accumulated fixes over the past couple of years
We have not been too active in pushing the tilcdc fixes to mainline. This series tries to bring the mainline tilcdc upto same level with TI ti-linux tree. Some patches that touch the same place over and over again have been squashed into one, leaving author of the last rewrite on top. Best regards, Jyri Darren Etheridge (5): drm/tilcdc: disable console switching during pm operations drm/tilcdc: rewrite pixel clock calculation drm/tilcdc: fix kernel panic on suspend when no hdmi monitor connected drm/tilcdc: make frame_done interrupt active at all times drm/tilcdc: disable the lcd controller/dma engine when suspend invoked Dave Gerlach (1): drm/tilcdc: adopt pinctrl support Grygorii Strashko (1): drm/tilcdc: fix build error when !CONFIG_CPU_FREQ Jyri Sarha (6): drm/tilcdc: Implement dma-buf support for tilcdc drm/tilcdc: Allocate register storage based on the actual number registers drm/tilcdc: Fix interrupt enable/disable code for version 2 tilcdc drm/tilcdc: Remove the duplicate LCDC_INT_ENABLE_SET_REG in registers[] drm/tilcdc: Add prints on sync lost and FIFO underrun interrupts drm/tilcdc: Disable sync lost interrupt if it fires on every frame Tomi Valkeinen (8): drm/tilcdc: verify fb pitch drm/tilcdc: cleanup runtime PM handling drm/tilcdc: disable crtc on unload drm/tilcdc: split reset to a separate function drm/tilcdc: remove broken error handling drm/tilcdc: cleanup irq handling drm/tilcdc: Get rid of complex ping-pong mechanism drm/tilcdc: Do not update the next frame buffer close to vertical blank drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 292 +++ drivers/gpu/drm/tilcdc/tilcdc_drv.c | 130 ++-- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 5 +- 3 files changed, 273 insertions(+), 154 deletions(-) -- 1.9.1
[PATCH 5/5] drm/vc4: Drop error message on seqno wait timeouts.
These ioctls end up getting exposed to fairly directly to GL users, and having normal user operations print DRM errors is obviously wrong. The message was originally to give us some idea of what happened when a hang occurred, but we have a DRM_INFO from reset for that. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_gem.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index 15619db..a9d020e 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -340,9 +340,6 @@ vc4_wait_for_seqno(struct drm_device *dev, uint64_t seqno, uint64_t timeout_ns, finish_wait(&vc4->job_wait_queue, &wait); trace_vc4_wait_for_seqno_end(dev, seqno); - if (ret && ret != -ERESTARTSYS) - DRM_ERROR("timeout waiting for render thread idle\n"); - return ret; } -- 2.7.0
[PATCH 4/5] drm/vc4: Fix -ERESTARTSYS error return from BO waits.
This caused the wait ioctls to claim that waiting had completed when we actually got interrupted by a signal before it was done. Fixes broken rendering throttling that produced serious lag in X window dragging. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_gem.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index 3bf679d..15619db 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -340,12 +340,10 @@ vc4_wait_for_seqno(struct drm_device *dev, uint64_t seqno, uint64_t timeout_ns, finish_wait(&vc4->job_wait_queue, &wait); trace_vc4_wait_for_seqno_end(dev, seqno); - if (ret && ret != -ERESTARTSYS) { + if (ret && ret != -ERESTARTSYS) DRM_ERROR("timeout waiting for render thread idle\n"); - return ret; - } - return 0; + return ret; } static void -- 2.7.0
[PATCH 3/5] drm/vc4: Return an ERR_PTR from BO creation instead of NULL.
Fixes igt vc4_create_bo/create-bo-0 by returning -EINVAL from the ioctl instead of -ENOMEM. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_bo.c| 16 drivers/gpu/drm/vc4/vc4_gem.c | 4 ++-- drivers/gpu/drm/vc4/vc4_irq.c | 2 +- drivers/gpu/drm/vc4/vc4_render_cl.c | 4 ++-- drivers/gpu/drm/vc4/vc4_validate.c | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c index 18dfe3e..22278bc 100644 --- a/drivers/gpu/drm/vc4/vc4_bo.c +++ b/drivers/gpu/drm/vc4/vc4_bo.c @@ -215,7 +215,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size, struct drm_gem_cma_object *cma_obj; if (size == 0) - return NULL; + return ERR_PTR(-EINVAL); /* First, try to get a vc4_bo from the kernel BO cache. */ if (from_cache) { @@ -237,7 +237,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size, if (IS_ERR(cma_obj)) { DRM_ERROR("Failed to allocate from CMA:\n"); vc4_bo_stats_dump(vc4); - return NULL; + return ERR_PTR(-ENOMEM); } } @@ -259,8 +259,8 @@ int vc4_dumb_create(struct drm_file *file_priv, args->size = args->pitch * args->height; bo = vc4_bo_create(dev, args->size, false); - if (!bo) - return -ENOMEM; + if (IS_ERR(bo)) + return PTR_ERR(bo); ret = drm_gem_handle_create(file_priv, &bo->base.base, &args->handle); drm_gem_object_unreference_unlocked(&bo->base.base); @@ -443,8 +443,8 @@ int vc4_create_bo_ioctl(struct drm_device *dev, void *data, * get zeroed, and that might leak data between users. */ bo = vc4_bo_create(dev, args->size, false); - if (!bo) - return -ENOMEM; + if (IS_ERR(bo)) + return PTR_ERR(bo); ret = drm_gem_handle_create(file_priv, &bo->base.base, &args->handle); drm_gem_object_unreference_unlocked(&bo->base.base); @@ -496,8 +496,8 @@ vc4_create_shader_bo_ioctl(struct drm_device *dev, void *data, } bo = vc4_bo_create(dev, args->size, true); - if (!bo) - return -ENOMEM; + if (IS_ERR(bo)) + return PTR_ERR(bo); ret = copy_from_user(bo->base.vaddr, (void __user *)(uintptr_t)args->data, diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index cc89ffc..3bf679d 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -578,9 +578,9 @@ vc4_get_bcl(struct drm_device *dev, struct vc4_exec_info *exec) } bo = vc4_bo_create(dev, exec_size, true); - if (!bo) { + if (IS_ERR(bo)) { DRM_ERROR("Couldn't allocate BO for binning\n"); - ret = -ENOMEM; + ret = PTR_ERR(bo); goto fail; } exec->exec_bo = &bo->base; diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c index b68060e..78a2135 100644 --- a/drivers/gpu/drm/vc4/vc4_irq.c +++ b/drivers/gpu/drm/vc4/vc4_irq.c @@ -57,7 +57,7 @@ vc4_overflow_mem_work(struct work_struct *work) struct vc4_bo *bo; bo = vc4_bo_create(dev, 256 * 1024, true); - if (!bo) { + if (IS_ERR(bo)) { DRM_ERROR("Couldn't allocate binner overflow mem\n"); return; } diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c b/drivers/gpu/drm/vc4/vc4_render_cl.c index dea97f4..0f12418 100644 --- a/drivers/gpu/drm/vc4/vc4_render_cl.c +++ b/drivers/gpu/drm/vc4/vc4_render_cl.c @@ -316,8 +316,8 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec, size += xtiles * ytiles * loop_body_size; setup->rcl = &vc4_bo_create(dev, size, true)->base; - if (!setup->rcl) - return -ENOMEM; + if (IS_ERR(setup->rcl)) + return PTR_ERR(setup->rcl); list_add_tail(&to_vc4_bo(&setup->rcl->base)->unref_head, &exec->unref_list); diff --git a/drivers/gpu/drm/vc4/vc4_validate.c b/drivers/gpu/drm/vc4/vc4_validate.c index e26d9f6..24c2c74 100644 --- a/drivers/gpu/drm/vc4/vc4_validate.c +++ b/drivers/gpu/drm/vc4/vc4_validate.c @@ -401,8 +401,8 @@ validate_tile_binning_config(VALIDATE_ARGS) tile_bo = vc4_bo_create(dev, exec->tile_alloc_offset + tile_alloc_size, true); exec->tile_bo = &tile_bo->base; - if (!exec->tile_bo) - return -ENOMEM; + if (IS_ERR(exec->tile_bo)) + return PTR_ERR(exec->tile_bo); list_add_tail(&tile_bo->unref_head, &exec->unref_list); /* tile alloc address. */ -- 2.7.0
[PATCH 2/5] drm/vc4: Fix the clear color for the first tile rendered.
Apparently in hardware (as opposed to simulation), the clear colors need to be uploaded before the render config, otherwise they won't take effect. Fixes igt's vc4_wait_bo/used-bo-* subtests. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_render_cl.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c b/drivers/gpu/drm/vc4/vc4_render_cl.c index 8a2a312..dea97f4 100644 --- a/drivers/gpu/drm/vc4/vc4_render_cl.c +++ b/drivers/gpu/drm/vc4/vc4_render_cl.c @@ -321,15 +321,6 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec, list_add_tail(&to_vc4_bo(&setup->rcl->base)->unref_head, &exec->unref_list); - rcl_u8(setup, VC4_PACKET_TILE_RENDERING_MODE_CONFIG); - rcl_u32(setup, - (setup->color_write ? (setup->color_write->paddr + - args->color_write.offset) : -0)); - rcl_u16(setup, args->width); - rcl_u16(setup, args->height); - rcl_u16(setup, args->color_write.bits); - /* The tile buffer gets cleared when the previous tile is stored. If * the clear values changed between frames, then the tile buffer has * stale clear values in it, so we have to do a store in None mode (no @@ -349,6 +340,15 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec, rcl_u32(setup, 0); /* no address, since we're in None mode */ } + rcl_u8(setup, VC4_PACKET_TILE_RENDERING_MODE_CONFIG); + rcl_u32(setup, + (setup->color_write ? (setup->color_write->paddr + + args->color_write.offset) : +0)); + rcl_u16(setup, args->width); + rcl_u16(setup, args->height); + rcl_u16(setup, args->color_write.bits); + for (y = min_y_tile; y <= max_y_tile; y++) { for (x = min_x_tile; x <= max_x_tile; x++) { bool first = (x == min_x_tile && y == min_y_tile); -- 2.7.0
[PATCH 1/5] drm/vc4: Validate that WAIT_BO padding is cleared.
This is ABI future-proofing if we ever want to extend the pad to mean something. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_gem.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index 48ce30a..cc89ffc 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -746,6 +746,9 @@ vc4_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_gem_object *gem_obj; struct vc4_bo *bo; + if (args->pad != 0) + return -EINVAL; + gem_obj = drm_gem_object_lookup(dev, file_priv, args->handle); if (!gem_obj) { DRM_ERROR("Failed to look up GEM BO %d\n", args->handle); -- 2.7.0
[PATCH 0/5] vc4 fixes for 4.5
Here's a series of fixes for vc4 from writing igt testcases while debugging X window dragging. Eric Anholt (5): drm/vc4: Validate that WAIT_BO padding is cleared. drm/vc4: Fix the clear color for the first tile rendered. drm/vc4: Return an ERR_PTR from BO creation instead of NULL. drm/vc4: Fix -ERESTARTSYS error return from BO waits. drm/vc4: Drop error message on seqno wait timeouts. drivers/gpu/drm/vc4/vc4_bo.c| 16 drivers/gpu/drm/vc4/vc4_gem.c | 14 ++ drivers/gpu/drm/vc4/vc4_irq.c | 2 +- drivers/gpu/drm/vc4/vc4_render_cl.c | 22 +++--- drivers/gpu/drm/vc4/vc4_validate.c | 4 ++-- 5 files changed, 28 insertions(+), 30 deletions(-) -- 2.7.0
[PATCH 02/10] drm/exynos: ipp: fix incorrect format specifiers in debug messages
On 03.02.2016 21:42, Marek Szyprowski wrote: > Drivers should use %p for printing pointers instead of hardcoding them > as hexadecimal integers. This patch fixes compilation warnings on 64bit > architectures. > > Signed-off-by: Marek Szyprowski > --- > drivers/gpu/drm/exynos/exynos_drm_fimc.c| 2 +- > drivers/gpu/drm/exynos/exynos_drm_gsc.c | 2 +- > drivers/gpu/drm/exynos/exynos_drm_ipp.c | 32 > ++--- > drivers/gpu/drm/exynos/exynos_drm_rotator.c | 2 +- > 4 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c > b/drivers/gpu/drm/exynos/exynos_drm_fimc.c > index c747824..8a4f4a0 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c > @@ -1723,7 +1723,7 @@ static int fimc_probe(struct platform_device *pdev) > goto err_put_clk; > } > > - DRM_DEBUG_KMS("id[%d]ippdrv[0x%x]\n", ctx->id, (int)ippdrv); > + DRM_DEBUG_KMS("id[%d]ippdrv[%p]\n", ctx->id, ippdrv); I don't oppose the patch itself but I have different concern. First - probably you meant %pK because this is a writeable structure with function pointers. Second - why the ippdrv has to be printed? Is it useful for debugging? Best regards, Krzysztof
[PATCH v3 06/11] staging/android: turn fence_info into a __u64 pointer
2016-02-04 Maarten Lankhorst : > Op 03-02-16 om 21:09 schreef Gustavo Padovan: > > Hi Maarten, > > > > 2016-02-03 Maarten Lankhorst : > > > >> Op 03-02-16 om 14:25 schreef Gustavo Padovan: > >>> From: Gustavo Padovan > >>> > >>> Turn sync_fence_info into __u64 type enable us to extend the struct in the > >>> future without breaking the ABI. > >>> > >>> v2: use type __u64 for fence_info > >>> > >>> v3: fix commit message to reflect the v2 change > >>> > >>> Signed-off-by: Gustavo Padovan > >>> --- > >>> drivers/staging/android/sync.c | 2 +- > >>> drivers/staging/android/uapi/sync.h | 2 +- > >>> 2 files changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/staging/android/sync.c > >>> b/drivers/staging/android/sync.c > >>> index 2ab0c20..8425457 100644 > >>> --- a/drivers/staging/android/sync.c > >>> +++ b/drivers/staging/android/sync.c > >>> @@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct > >>> sync_file *sync_file, > >>> if (info->status >= 0) > >>> info->status = !info->status; > >>> > >>> - len = sizeof(struct sync_file_info); > >>> + len = sizeof(struct sync_file_info) - sizeof(__u64); > >>> > >>> for (i = 0; i < sync_file->num_fences; ++i) { > >>> struct fence *fence = sync_file->cbs[i].fence; > >>> diff --git a/drivers/staging/android/uapi/sync.h > >>> b/drivers/staging/android/uapi/sync.h > >>> index a0cf357..e649953 100644 > >>> --- a/drivers/staging/android/uapi/sync.h > >>> +++ b/drivers/staging/android/uapi/sync.h > >>> @@ -54,7 +54,7 @@ struct sync_file_info { > >>> charname[32]; > >>> __s32 status; > >>> > >>> - __u8sync_fence_info[0]; > >>> + __u64 sync_fence_info; > >>> }; > >>> > >>> #define SYNC_IOC_MAGIC '>' > >> This still doesn't do what you expect it to. > >> > >> I think this is what you want is for userspace to do: > >> > >> struct sync_file_info info; > >> > >> info.flags = info.num_fences = 0; > >> ioctl(fd, SYNC_IOC_FENCE_INFO, &info); > >> if (info.num_fences) { > >> info.sync_fence_info = (uintptr)kcalloc(info.num_fences, sizeof(struct > >> sync_fence_info)); > >> ioctl(fd, SYNC_IOC_FENCE_INFO, &info); > >> } > >> > >> Maybe userspace could preallocate the max in advance and set num_fences > >> higher, > >> > >> kernel would do something like: > >> > >> num_fences = min(info.num_fences, sync->num_fences); > >> struct sync_fence_info array[num_fences]; > >> > >> info.num_fences = sync->num_fences; > >> if (num_fences && > >> copy_to_user((void * __user)(unsigned long)info.sync_fence_info, > >> array, num_fences * sizeof(array))) > >> return -EFAULT; > > If we are going to call IOCTL twice I would actually have a new IOCTL only > > to fetch sync_fence_info. > > > > First we would call > > > > ioctl(fd, SYNC_IOC_FILE_INFO, &info); > > > > where info is: > > > > struct sync_file_info { > > charname[32]; > > __s32 status; > > __u32 flags; > > __u32 num_fences; > > }; > > > > then we would allocate a buffer with > > > > size = info.num_fences * sizeof(struct sync_fence_info) > > > > and call the new ioctl > > > > ioctl(fd, SYNC_IOC_SYNC_FENCE_INFO, sync_fence_info); > > > > This looks like a cleaner solution and doesn't break ABI. What do you > > think? > I think it's good taste that userspace specifies the size of the buffer it > passes, so former feels more clean to me, > since you need to pass num_fences anyway. Just to clarify, userspace specifies the size of the buffer in the solution I proposed. It would be size = info.num_fences * sizeof(struct sync_fence_info) sync_fence_info = malloc(size); ioctl(fd, SYNC_IOC_SYNC_FENCE_INFO, sync_fence_info);
[PATCH v3 06/11] staging/android: turn fence_info into a __u64 pointer
Op 03-02-16 om 21:09 schreef Gustavo Padovan: > Hi Maarten, > > 2016-02-03 Maarten Lankhorst : > >> Op 03-02-16 om 14:25 schreef Gustavo Padovan: >>> From: Gustavo Padovan >>> >>> Turn sync_fence_info into __u64 type enable us to extend the struct in the >>> future without breaking the ABI. >>> >>> v2: use type __u64 for fence_info >>> >>> v3: fix commit message to reflect the v2 change >>> >>> Signed-off-by: Gustavo Padovan >>> --- >>> drivers/staging/android/sync.c | 2 +- >>> drivers/staging/android/uapi/sync.h | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c >>> index 2ab0c20..8425457 100644 >>> --- a/drivers/staging/android/sync.c >>> +++ b/drivers/staging/android/sync.c >>> @@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct sync_file >>> *sync_file, >>> if (info->status >= 0) >>> info->status = !info->status; >>> >>> - len = sizeof(struct sync_file_info); >>> + len = sizeof(struct sync_file_info) - sizeof(__u64); >>> >>> for (i = 0; i < sync_file->num_fences; ++i) { >>> struct fence *fence = sync_file->cbs[i].fence; >>> diff --git a/drivers/staging/android/uapi/sync.h >>> b/drivers/staging/android/uapi/sync.h >>> index a0cf357..e649953 100644 >>> --- a/drivers/staging/android/uapi/sync.h >>> +++ b/drivers/staging/android/uapi/sync.h >>> @@ -54,7 +54,7 @@ struct sync_file_info { >>> charname[32]; >>> __s32 status; >>> >>> - __u8sync_fence_info[0]; >>> + __u64 sync_fence_info; >>> }; >>> >>> #define SYNC_IOC_MAGIC '>' >> This still doesn't do what you expect it to. >> >> I think this is what you want is for userspace to do: >> >> struct sync_file_info info; >> >> info.flags = info.num_fences = 0; >> ioctl(fd, SYNC_IOC_FENCE_INFO, &info); >> if (info.num_fences) { >> info.sync_fence_info = (uintptr)kcalloc(info.num_fences, sizeof(struct >> sync_fence_info)); >> ioctl(fd, SYNC_IOC_FENCE_INFO, &info); >> } >> >> Maybe userspace could preallocate the max in advance and set num_fences >> higher, >> >> kernel would do something like: >> >> num_fences = min(info.num_fences, sync->num_fences); >> struct sync_fence_info array[num_fences]; >> >> info.num_fences = sync->num_fences; >> if (num_fences && >> copy_to_user((void * __user)(unsigned long)info.sync_fence_info, array, >> num_fences * sizeof(array))) >> return -EFAULT; > If we are going to call IOCTL twice I would actually have a new IOCTL only > to fetch sync_fence_info. > > First we would call > > ioctl(fd, SYNC_IOC_FILE_INFO, &info); > > where info is: > > struct sync_file_info { > charname[32]; > __s32 status; > __u32 flags; > __u32 num_fences; > }; > > then we would allocate a buffer with > > size = info.num_fences * sizeof(struct sync_fence_info) > > and call the new ioctl > > ioctl(fd, SYNC_IOC_SYNC_FENCE_INFO, sync_fence_info); > > This looks like a cleaner solution and doesn't break ABI. What do you > think? I think it's good taste that userspace specifies the size of the buffer it passes, so former feels more clean to me, since you need to pass num_fences anyway. But Daniel knows more about designing ioctl's than I do, so for exact behavior it's best to ask him. ~Maarten
[PATCH 6/7] tests/amdgpu: make amdgpu_command_submission_sdma_copy_linear generic
On Thu, Feb 4, 2016 at 10:47 AM, Emil Velikov wrote: > On 4 February 2016 at 14:59, Alex Deucher wrote: >> So it can be shared for CP tests. >> >> Reviewed-by: Ken Wang >> Signed-off-by: Alex Deucher >> --- >> tests/amdgpu/basic_tests.c | 27 +-- >> 1 file changed, 17 insertions(+), 10 deletions(-) >> >> diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c >> index 7806be7..701ccf1 100644 >> --- a/tests/amdgpu/basic_tests.c >> +++ b/tests/amdgpu/basic_tests.c >> @@ -57,6 +57,7 @@ static void >> amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, >>struct amdgpu_cs_request >> *ibs_request); >> static void amdgpu_command_submission_write_linear_helper(unsigned ip_type); >> static void amdgpu_command_submission_const_fill_helper(unsigned ip_type); >> +static void amdgpu_command_submission_copy_linear_helper(unsigned ip_type); >> > Unless I'm missing something we don't need these three ? They are used by patches 3, 5, and 7. Alex
[PATCH 1/7] tests/amdgpu: make amdgpu_sdma_test_exec_cs() generic
On Thu, Feb 4, 2016 at 10:43 AM, Emil Velikov wrote: > On 4 February 2016 at 14:59, Alex Deucher wrote: >> Share with upcoming CP tests. >> >> Reviewed-by: Ken Wang >> Signed-off-by: Alex Deucher >> --- >> tests/amdgpu/basic_tests.c | 58 >> -- >> 1 file changed, 35 insertions(+), 23 deletions(-) >> >> diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c >> index fa0ed12..1bcc835 100644 >> --- a/tests/amdgpu/basic_tests.c >> +++ b/tests/amdgpu/basic_tests.c >> @@ -49,6 +49,13 @@ static void amdgpu_command_submission_sdma(void); >> static void amdgpu_userptr_test(void); >> static void amdgpu_semaphore_test(void); >> >> +static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, >> + unsigned ip_type, >> + int instance, int pm4_dw, uint32_t >> *pm4_src, >> + int res_cnt, amdgpu_bo_handle >> *resources, >> + struct amdgpu_cs_ib_info *ib_info, >> + struct amdgpu_cs_request >> *ibs_request); >> + > We don't need the forward declaration, do we ? Nope. Leftover from an earlier version of this patch set. I'll drop it. Alex
[PATCH] drm/i915/skl: Fix typo in DPLL_CFGCR1 definition
We accidentally point both cfgcr registers for the second shared DPLL to the same location in i915_reg.h. This results in a lot of hw pipe state mismatches whenever we try to do a modeset that requires allocating the DPLL to a CRTC: [drm:intel_pipe_config_compare [i915]] *ERROR* mismatch in dpll_hw_state.cfgcr1 (expected 0x8168, found 0x04a5) [drm:intel_pipe_config_compare [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_clock (expected 108000, found 49500) [drm:intel_pipe_config_compare [i915]] *ERROR* mismatch in port_clock (expected 108000, found 49500) This usually ends up causing blank monitors, since the DPLL never can get set to the right clock. Fixes: f0f59a00a1 ("drm/i915: Type safe register read/write") Signed-off-by: Lyude --- drivers/gpu/drm/i915/i915_reg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 007ae83..b9a564b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7514,7 +7514,7 @@ enum skl_disp_power_wells { #define DPLL_CFGCR2_PDIV_7 (4<<2) #define DPLL_CFGCR2_CENTRAL_FREQ_MASK (3) -#define DPLL_CFGCR1(id)_MMIO_PIPE((id) - SKL_DPLL1, _DPLL1_CFGCR1, _DPLL2_CFGCR2) +#define DPLL_CFGCR1(id)_MMIO_PIPE((id) - SKL_DPLL1, _DPLL1_CFGCR1, _DPLL2_CFGCR1) #define DPLL_CFGCR2(id)_MMIO_PIPE((id) - SKL_DPLL1, _DPLL1_CFGCR2, _DPLL2_CFGCR2) /* BXT display engine PLL */ -- 2.5.0
[PATCH] drm/msm/dsi: fix definition of msm_dsi_pll_28nm_8960_init()
On 02/03/2016 07:55 PM, Luis Henriques wrote: > This fixes the following build failure: > > drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.o: In function > `msm_dsi_pll_28nm_8960_init': > dsi_pll_28nm.c:(.text+0x1198): multiple definition of > `msm_dsi_pll_28nm_8960_init' > drivers/gpu/drm/msm/dsi/pll/dsi_pll.o:dsi_pll.c:(.text+0x0): first defined > here Thanks for the fix. Acked-by: Archit Taneja Dave, Could you please queue this for the next -rc cycle since it causes a build break? Thanks, Archit > > Signed-off-by: Luis Henriques > --- > drivers/gpu/drm/msm/dsi/pll/dsi_pll.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll.h > b/drivers/gpu/drm/msm/dsi/pll/dsi_pll.h > index 80b6038334a6..2cf1664723e8 100644 > --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll.h > +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll.h > @@ -97,8 +97,8 @@ static inline struct msm_dsi_pll *msm_dsi_pll_28nm_init( > struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init(struct platform_device *pdev, > int id); > #else > -struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init(struct platform_device *pdev, > -int id) > +static inline struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init( > + struct platform_device *pdev, int id) > { > return ERR_PTR(-ENODEV); > } > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
[PATCH 0/5] Fixes for MST (daisy-chain and 4k tiles)
On 3 February 2016 at 07:20, Alex Deucher wrote: > On Mon, Jan 25, 2016 at 1:55 PM, Wentland, Harry > wrote: >> Hi Dave, >> >> I've been running with lockdep with these changes for over a week now. Just >> ran another test with our daisy-chain displays and the 4k tiled display with >> no deadlocks or lockdep prints (other than "RCU lockdep checking is >> enabled."). >> > > Dave, do you want to pick these up directly or should I pull them into > my -fixes tree? > I'm going to pull them in myself for this round I think, there are a few other MST bits around I want to test before pushing. Dave.
[PATCH 7/7] tests/amdgpu: add a test for cp dma copy
Use the CP to copy data between buffers Reviewed-by: Ken Wang Signed-off-by: Alex Deucher --- tests/amdgpu/basic_tests.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c index 701ccf1..aac5615 100644 --- a/tests/amdgpu/basic_tests.c +++ b/tests/amdgpu/basic_tests.c @@ -461,12 +461,19 @@ static void amdgpu_command_submission_cp_const_fill(void) amdgpu_command_submission_const_fill_helper(AMDGPU_HW_IP_GFX); } +static void amdgpu_command_submission_cp_copy_data(void) +{ + amdgpu_command_submission_copy_linear_helper(AMDGPU_HW_IP_GFX); +} + static void amdgpu_command_submission_gfx(void) { /* write data using the CP */ amdgpu_command_submission_cp_write_data(); /* const fill using the CP */ amdgpu_command_submission_cp_const_fill(); + /* copy data using the CP */ + amdgpu_command_submission_cp_copy_data(); /* separate IB buffers for multi-IB submission */ amdgpu_command_submission_gfx_separate_ibs(); /* shared IB buffer for multi-IB submission */ @@ -1029,6 +1036,17 @@ static void amdgpu_command_submission_copy_linear_helper(unsigned ip_type) pm4[i++] = (0x & bo1_mc) >> 32; pm4[i++] = 0x & bo2_mc; pm4[i++] = (0x & bo2_mc) >> 32; + } else if (ip_type == AMDGPU_HW_IP_GFX) { + pm4[i++] = PACKET3(PACKET3_DMA_DATA, 5); + pm4[i++] = PACKET3_DMA_DATA_ENGINE(0) | + PACKET3_DMA_DATA_DST_SEL(0) | + PACKET3_DMA_DATA_SRC_SEL(0) | + PACKET3_DMA_DATA_CP_SYNC; + pm4[i++] = 0xfffc & bo1_mc; + pm4[i++] = (0x & bo1_mc) >> 32; + pm4[i++] = 0xfffc & bo2_mc; + pm4[i++] = (0x & bo2_mc) >> 32; + pm4[i++] = sdma_write_length; } amdgpu_test_exec_cs_helper(context_handle, -- 2.5.0
[PATCH 6/7] tests/amdgpu: make amdgpu_command_submission_sdma_copy_linear generic
So it can be shared for CP tests. Reviewed-by: Ken Wang Signed-off-by: Alex Deucher --- tests/amdgpu/basic_tests.c | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c index 7806be7..701ccf1 100644 --- a/tests/amdgpu/basic_tests.c +++ b/tests/amdgpu/basic_tests.c @@ -57,6 +57,7 @@ static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, struct amdgpu_cs_request *ibs_request); static void amdgpu_command_submission_write_linear_helper(unsigned ip_type); static void amdgpu_command_submission_const_fill_helper(unsigned ip_type); +static void amdgpu_command_submission_copy_linear_helper(unsigned ip_type); CU_TestInfo basic_tests[] = { { "Query Info Test", amdgpu_query_info_test }, @@ -955,7 +956,7 @@ static void amdgpu_command_submission_sdma_const_fill(void) amdgpu_command_submission_const_fill_helper(AMDGPU_HW_IP_DMA); } -static void amdgpu_command_submission_sdma_copy_linear(void) +static void amdgpu_command_submission_copy_linear_helper(unsigned ip_type) { const int sdma_write_length = 1024; const int pm4_dw = 256; @@ -1020,17 +1021,18 @@ static void amdgpu_command_submission_sdma_copy_linear(void) /* fullfill PM4: test DMA copy linear */ i = j = 0; - pm4[i++] = SDMA_PACKET(SDMA_OPCODE_COPY, SDMA_COPY_SUB_OPCODE_LINEAR, 0); - pm4[i++] = sdma_write_length; - pm4[i++] = 0; - pm4[i++] = 0x & bo1_mc; - pm4[i++] = (0x & bo1_mc) >> 32; - pm4[i++] = 0x & bo2_mc; - pm4[i++] = (0x & bo2_mc) >> 32; - + if (ip_type == AMDGPU_HW_IP_DMA) { + pm4[i++] = SDMA_PACKET(SDMA_OPCODE_COPY, SDMA_COPY_SUB_OPCODE_LINEAR, 0); + pm4[i++] = sdma_write_length; + pm4[i++] = 0; + pm4[i++] = 0x & bo1_mc; + pm4[i++] = (0x & bo1_mc) >> 32; + pm4[i++] = 0x & bo2_mc; + pm4[i++] = (0x & bo2_mc) >> 32; + } amdgpu_test_exec_cs_helper(context_handle, - AMDGPU_HW_IP_DMA, 0, + ip_type, 0, i, pm4, 2, resources, ib_info, ibs_request); @@ -1061,6 +1063,11 @@ static void amdgpu_command_submission_sdma_copy_linear(void) CU_ASSERT_EQUAL(r, 0); } +static void amdgpu_command_submission_sdma_copy_linear(void) +{ + amdgpu_command_submission_copy_linear_helper(AMDGPU_HW_IP_DMA); +} + static void amdgpu_command_submission_sdma(void) { amdgpu_command_submission_sdma_write_linear(); -- 2.5.0
[PATCH 5/7] tests/amdgpu: add a test for cp dma fill
Use the CP to fill to memory. Reviewed-by: Ken Wang Signed-off-by: Alex Deucher --- tests/amdgpu/basic_tests.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c index 4d382e6..7806be7 100644 --- a/tests/amdgpu/basic_tests.c +++ b/tests/amdgpu/basic_tests.c @@ -455,10 +455,17 @@ static void amdgpu_command_submission_cp_write_data(void) amdgpu_command_submission_write_linear_helper(AMDGPU_HW_IP_GFX); } +static void amdgpu_command_submission_cp_const_fill(void) +{ + amdgpu_command_submission_const_fill_helper(AMDGPU_HW_IP_GFX); +} + static void amdgpu_command_submission_gfx(void) { /* write data using the CP */ amdgpu_command_submission_cp_write_data(); + /* const fill using the CP */ + amdgpu_command_submission_cp_const_fill(); /* separate IB buffers for multi-IB submission */ amdgpu_command_submission_gfx_separate_ibs(); /* shared IB buffer for multi-IB submission */ @@ -902,6 +909,17 @@ static void amdgpu_command_submission_const_fill_helper(unsigned ip_type) pm4[i++] = (0x & bo_mc) >> 32; pm4[i++] = 0xdeadbeaf; pm4[i++] = sdma_write_length; + } else if (ip_type == AMDGPU_HW_IP_GFX) { + pm4[i++] = PACKET3(PACKET3_DMA_DATA, 5); + pm4[i++] = PACKET3_DMA_DATA_ENGINE(0) | + PACKET3_DMA_DATA_DST_SEL(0) | + PACKET3_DMA_DATA_SRC_SEL(2) | + PACKET3_DMA_DATA_CP_SYNC; + pm4[i++] = 0xdeadbeaf; + pm4[i++] = 0; + pm4[i++] = 0xfffc & bo_mc; + pm4[i++] = (0x & bo_mc) >> 32; + pm4[i++] = sdma_write_length; } amdgpu_test_exec_cs_helper(context_handle, -- 2.5.0
[PATCH 4/7] tests/amdgpu: make amdgpu_command_submission_sdma_const_fill generic
So it can be shared for CP tests. Reviewed-by: Ken Wang Signed-off-by: Alex Deucher --- tests/amdgpu/basic_tests.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c index 5c9debe..4d382e6 100644 --- a/tests/amdgpu/basic_tests.c +++ b/tests/amdgpu/basic_tests.c @@ -56,6 +56,7 @@ static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, struct amdgpu_cs_ib_info *ib_info, struct amdgpu_cs_request *ibs_request); static void amdgpu_command_submission_write_linear_helper(unsigned ip_type); +static void amdgpu_command_submission_const_fill_helper(unsigned ip_type); CU_TestInfo basic_tests[] = { { "Query Info Test", amdgpu_query_info_test }, @@ -845,7 +846,7 @@ static void amdgpu_command_submission_sdma_write_linear(void) amdgpu_command_submission_write_linear_helper(AMDGPU_HW_IP_DMA); } -static void amdgpu_command_submission_sdma_const_fill(void) +static void amdgpu_command_submission_const_fill_helper(unsigned ip_type) { const int sdma_write_length = 1024 * 1024; const int pm4_dw = 256; @@ -894,15 +895,17 @@ static void amdgpu_command_submission_sdma_const_fill(void) /* fullfill PM4: test DMA const fill */ i = j = 0; - pm4[i++] = SDMA_PACKET(SDMA_OPCODE_CONSTANT_FILL, 0, - SDMA_CONSTANT_FILL_EXTRA_SIZE(2)); - pm4[i++] = 0x & bo_mc; - pm4[i++] = (0x & bo_mc) >> 32; - pm4[i++] = 0xdeadbeaf; - pm4[i++] = sdma_write_length; + if (ip_type == AMDGPU_HW_IP_DMA) { + pm4[i++] = SDMA_PACKET(SDMA_OPCODE_CONSTANT_FILL, 0, + SDMA_CONSTANT_FILL_EXTRA_SIZE(2)); + pm4[i++] = 0x & bo_mc; + pm4[i++] = (0x & bo_mc) >> 32; + pm4[i++] = 0xdeadbeaf; + pm4[i++] = sdma_write_length; + } amdgpu_test_exec_cs_helper(context_handle, - AMDGPU_HW_IP_DMA, 0, + ip_type, 0, i, pm4, 1, resources, ib_info, ibs_request); @@ -929,6 +932,11 @@ static void amdgpu_command_submission_sdma_const_fill(void) CU_ASSERT_EQUAL(r, 0); } +static void amdgpu_command_submission_sdma_const_fill(void) +{ + amdgpu_command_submission_const_fill_helper(AMDGPU_HW_IP_DMA); +} + static void amdgpu_command_submission_sdma_copy_linear(void) { const int sdma_write_length = 1024; -- 2.5.0
[PATCH 3/7] tests/amdgpu: add a test for cp write data
Use the CP to write data to memory. Reviewed-by: Ken Wang Signed-off-by: Alex Deucher --- tests/amdgpu/basic_tests.c | 125 + 1 file changed, 125 insertions(+) diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c index 5804503..5c9debe 100644 --- a/tests/amdgpu/basic_tests.c +++ b/tests/amdgpu/basic_tests.c @@ -90,6 +90,117 @@ CU_TestInfo basic_tests[] = { #define GFX_COMPUTE_NOP 0x1000 #define SDMA_NOP 0x0 +/* PM4 */ +#definePACKET_TYPE00 +#definePACKET_TYPE11 +#definePACKET_TYPE22 +#definePACKET_TYPE33 + +#define CP_PACKET_GET_TYPE(h) (((h) >> 30) & 3) +#define CP_PACKET_GET_COUNT(h) (((h) >> 16) & 0x3FFF) +#define CP_PACKET0_GET_REG(h) ((h) & 0x) +#define CP_PACKET3_GET_OPCODE(h) (((h) >> 8) & 0xFF) +#define PACKET0(reg, n)((PACKET_TYPE0 << 30) | \ +((reg) & 0x) | \ +((n) & 0x3FFF) << 16) +#define CP_PACKET2 0x8000 +#definePACKET2_PAD_SHIFT 0 +#definePACKET2_PAD_MASK(0x3fff << 0) + +#define PACKET2(v) (CP_PACKET2 | REG_SET(PACKET2_PAD, (v))) + +#define PACKET3(op, n) ((PACKET_TYPE3 << 30) | \ +(((op) & 0xFF) << 8) | \ +((n) & 0x3FFF) << 16) + +/* Packet 3 types */ +#definePACKET3_NOP 0x10 + +#definePACKET3_WRITE_DATA 0x37 +#defineWRITE_DATA_DST_SEL(x) ((x) << 8) + /* 0 - register +* 1 - memory (sync - via GRBM) +* 2 - gl2 +* 3 - gds +* 4 - reserved +* 5 - memory (async - direct) +*/ +#defineWR_ONE_ADDR (1 << 16) +#defineWR_CONFIRM (1 << 20) +#defineWRITE_DATA_CACHE_POLICY(x) ((x) << 25) + /* 0 - LRU +* 1 - Stream +*/ +#defineWRITE_DATA_ENGINE_SEL(x)((x) << 30) + /* 0 - me +* 1 - pfp +* 2 - ce +*/ + +#definePACKET3_DMA_DATA0x50 +/* 1. header + * 2. CONTROL + * 3. SRC_ADDR_LO or DATA [31:0] + * 4. SRC_ADDR_HI [31:0] + * 5. DST_ADDR_LO [31:0] + * 6. DST_ADDR_HI [7:0] + * 7. COMMAND [30:21] | BYTE_COUNT [20:0] + */ +/* CONTROL */ +# define PACKET3_DMA_DATA_ENGINE(x) ((x) << 0) + /* 0 - ME +* 1 - PFP +*/ +# define PACKET3_DMA_DATA_SRC_CACHE_POLICY(x) ((x) << 13) + /* 0 - LRU +* 1 - Stream +* 2 - Bypass +*/ +# define PACKET3_DMA_DATA_SRC_VOLATILE (1 << 15) +# define PACKET3_DMA_DATA_DST_SEL(x) ((x) << 20) + /* 0 - DST_ADDR using DAS +* 1 - GDS +* 3 - DST_ADDR using L2 +*/ +# define PACKET3_DMA_DATA_DST_CACHE_POLICY(x) ((x) << 25) + /* 0 - LRU +* 1 - Stream +* 2 - Bypass +*/ +# define PACKET3_DMA_DATA_DST_VOLATILE (1 << 27) +# define PACKET3_DMA_DATA_SRC_SEL(x) ((x) << 29) + /* 0 - SRC_ADDR using SAS +* 1 - GDS +* 2 - DATA +* 3 - SRC_ADDR using L2 +*/ +# define PACKET3_DMA_DATA_CP_SYNC (1 << 31) +/* COMMAND */ +# define PACKET3_DMA_DATA_DIS_WC (1 << 21) +# define PACKET3_DMA_DATA_CMD_SRC_SWAP(x) ((x) << 22) + /* 0 - none +* 1 - 8 in 16 +* 2 - 8 in 32 +* 3 - 8 in 64 +*/ +# define PACKET3_DMA_DATA_CMD_DST_SWAP(x) ((x) << 24) + /* 0 - none +* 1 - 8 in 16 +* 2 - 8 in 32 +* 3 - 8 in 64 +*/ +# define PACKET3_DMA_DATA_CMD_SAS (1 << 26) + /* 0 - memory +* 1 - register +*/ +# define PACKET3_DMA_DATA_CMD_DAS (1 << 27) + /* 0 - memory +* 1 - register +*/ +# define PACKET3_DMA_DATA_CMD_SAIC(1 << 28) +# define PACKET3_DMA_DATA_CMD_DAIC(1 << 29) +# define PACKET3_DMA_DATA_CMD_RAW_WAIT (1 << 30) + int suite_basic_tests_init(void) { int r; @@ -338,8 +449,15 @@ static void amdgpu_command_submission_gfx_shared_ib(void) CU_ASSERT_EQUAL(r, 0); } +static void amdgpu_command_submission_cp_write_data(void
[PATCH 2/7] tests/amdgpu: make amdgpu_command_submission_sdma_write_linear generic
So it can be shared for CP tests. Reviewed-by: Ken Wang Signed-off-by: Alex Deucher --- tests/amdgpu/basic_tests.c | 26 +- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c index 1bcc835..5804503 100644 --- a/tests/amdgpu/basic_tests.c +++ b/tests/amdgpu/basic_tests.c @@ -55,6 +55,7 @@ static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, int res_cnt, amdgpu_bo_handle *resources, struct amdgpu_cs_ib_info *ib_info, struct amdgpu_cs_request *ibs_request); +static void amdgpu_command_submission_write_linear_helper(unsigned ip_type); CU_TestInfo basic_tests[] = { { "Query Info Test", amdgpu_query_info_test }, @@ -626,7 +627,7 @@ static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, CU_ASSERT_EQUAL(r, 0); } -static void amdgpu_command_submission_sdma_write_linear(void) +static void amdgpu_command_submission_write_linear_helper(unsigned ip_type) { const int sdma_write_length = 128; const int pm4_dw = 256; @@ -676,16 +677,18 @@ static void amdgpu_command_submission_sdma_write_linear(void) /* fullfill PM4: test DMA write-linear */ i = j = 0; - pm4[i++] = SDMA_PACKET(SDMA_OPCODE_WRITE, - SDMA_WRITE_SUB_OPCODE_LINEAR, 0); - pm4[i++] = 0x & bo_mc; - pm4[i++] = (0x & bo_mc) >> 32; - pm4[i++] = sdma_write_length; - while(j++ < sdma_write_length) - pm4[i++] = 0xdeadbeaf; + if (ip_type == AMDGPU_HW_IP_DMA) { + pm4[i++] = SDMA_PACKET(SDMA_OPCODE_WRITE, + SDMA_WRITE_SUB_OPCODE_LINEAR, 0); + pm4[i++] = 0x & bo_mc; + pm4[i++] = (0x & bo_mc) >> 32; + pm4[i++] = sdma_write_length; + while(j++ < sdma_write_length) + pm4[i++] = 0xdeadbeaf; + } amdgpu_test_exec_cs_helper(context_handle, - AMDGPU_HW_IP_DMA, 0, + ip_type, 0, i, pm4, 1, resources, ib_info, ibs_request); @@ -712,6 +715,11 @@ static void amdgpu_command_submission_sdma_write_linear(void) CU_ASSERT_EQUAL(r, 0); } +static void amdgpu_command_submission_sdma_write_linear(void) +{ + amdgpu_command_submission_write_linear_helper(AMDGPU_HW_IP_DMA); +} + static void amdgpu_command_submission_sdma_const_fill(void) { const int sdma_write_length = 1024 * 1024; -- 2.5.0
[PATCH 1/7] tests/amdgpu: make amdgpu_sdma_test_exec_cs() generic
Share with upcoming CP tests. Reviewed-by: Ken Wang Signed-off-by: Alex Deucher --- tests/amdgpu/basic_tests.c | 58 -- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c index fa0ed12..1bcc835 100644 --- a/tests/amdgpu/basic_tests.c +++ b/tests/amdgpu/basic_tests.c @@ -49,6 +49,13 @@ static void amdgpu_command_submission_sdma(void); static void amdgpu_userptr_test(void); static void amdgpu_semaphore_test(void); +static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, + unsigned ip_type, + int instance, int pm4_dw, uint32_t *pm4_src, + int res_cnt, amdgpu_bo_handle *resources, + struct amdgpu_cs_ib_info *ib_info, + struct amdgpu_cs_request *ibs_request); + CU_TestInfo basic_tests[] = { { "Query Info Test", amdgpu_query_info_test }, { "Memory alloc Test", amdgpu_memory_alloc }, @@ -542,11 +549,12 @@ static void amdgpu_command_submission_compute(void) * pm4_src, resources, ib_info, and ibs_request * submit command stream described in ibs_request and wait for this IB accomplished */ -static void amdgpu_sdma_test_exec_cs(amdgpu_context_handle context_handle, -int instance, int pm4_dw, uint32_t *pm4_src, -int res_cnt, amdgpu_bo_handle *resources, -struct amdgpu_cs_ib_info *ib_info, -struct amdgpu_cs_request *ibs_request) +static void amdgpu_test_exec_cs_helper(amdgpu_context_handle context_handle, + unsigned ip_type, + int instance, int pm4_dw, uint32_t *pm4_src, + int res_cnt, amdgpu_bo_handle *resources, + struct amdgpu_cs_ib_info *ib_info, + struct amdgpu_cs_request *ibs_request) { int r; uint32_t expired; @@ -579,7 +587,7 @@ static void amdgpu_sdma_test_exec_cs(amdgpu_context_handle context_handle, ib_info->ib_mc_address = ib_result_mc_address; ib_info->size = pm4_dw; - ibs_request->ip_type = AMDGPU_HW_IP_DMA; + ibs_request->ip_type = ip_type; ibs_request->ring = instance; ibs_request->number_of_ibs = 1; ibs_request->ibs = ib_info; @@ -601,7 +609,7 @@ static void amdgpu_sdma_test_exec_cs(amdgpu_context_handle context_handle, r = amdgpu_bo_list_destroy(ibs_request->resources); CU_ASSERT_EQUAL(r, 0); - fence_status.ip_type = AMDGPU_HW_IP_DMA; + fence_status.ip_type = ip_type; fence_status.ring = ibs_request->ring; fence_status.context = context_handle; fence_status.fence = ibs_request->seq_no; @@ -676,10 +684,11 @@ static void amdgpu_command_submission_sdma_write_linear(void) while(j++ < sdma_write_length) pm4[i++] = 0xdeadbeaf; - amdgpu_sdma_test_exec_cs(context_handle, 0, - i, pm4, - 1, resources, - ib_info, ibs_request); + amdgpu_test_exec_cs_helper(context_handle, + AMDGPU_HW_IP_DMA, 0, + i, pm4, + 1, resources, + ib_info, ibs_request); /* verify if SDMA test result meets with expected */ i = 0; @@ -759,10 +768,11 @@ static void amdgpu_command_submission_sdma_const_fill(void) pm4[i++] = 0xdeadbeaf; pm4[i++] = sdma_write_length; - amdgpu_sdma_test_exec_cs(context_handle, 0, - i, pm4, - 1, resources, - ib_info, ibs_request); + amdgpu_test_exec_cs_helper(context_handle, + AMDGPU_HW_IP_DMA, 0, + i, pm4, + 1, resources, + ib_info, ibs_request); /* verify if SDMA test result meets with expected */ i = 0; @@ -860,10 +870,11 @@ static void amdgpu_command_submission_sdma_copy_linear(void) pm4[i++] = (0x & bo2_mc) >> 32; - amdgpu_sdma_test_exec_cs(context_handle, 0, - i, pm4, - 2, resources, - ib_inf
[Bug 93998] Linux 4.5-rc2 ATI Radeon 3000 Graphics not rendering correctly
https://bugs.freedesktop.org/show_bug.cgi?id=93998 --- Comment #2 from SMF --- Created attachment 121511 --> https://bugs.freedesktop.org/attachment.cgi?id=121511&action=edit xorg.conf My xorg.conf file from /etc/X11 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160204/4c8c74bf/attachment.html>