[Bug 207383] [Regression] 5.7 amdgpu/polaris11 gpf: amdgpu_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=207383 --- Comment #27 from rtmasura+ker...@hotmail.com --- and another crash, chrome's good at causing them (watching youtube). Used -s "" for the setting which I think should set it to 'auto', and what I assumed was default. I've changed that to -s "off" to see if that helps. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207383] [Regression] 5.7 amdgpu/polaris11 gpf: amdgpu_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=207383 --- Comment #26 from rtmasura+ker...@hotmail.com --- and just got another crash, only watching a video in chrome. Guess the chrome bit at the end might be more important than I thought I *think* I've turned off the glx for xfwm.. we'll see. My computer has been showing video in chrome every day without issues before today. I hadn't updated since last week either, no changes in the system. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbtft-bus.c: Removing that prohibited space before ')'
On Sat, Jun 27, 2020 at 12:51:50AM -0400, B K Karthik wrote: > fbtft-bus.c: > > fixing ERROR: space prohibited before that close parenthesis ')' by removing > that space and ',' in line 65 and 67. > > Signed-off-by: B K Karthik > --- > drivers/staging/fbtft/fbtft-bus.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/fbtft/fbtft-bus.c > b/drivers/staging/fbtft/fbtft-bus.c > index 63c65dd67b17..847cbfbbd766 100644 > --- a/drivers/staging/fbtft/fbtft-bus.c > +++ b/drivers/staging/fbtft/fbtft-bus.c > @@ -62,9 +62,9 @@ out: > \ > } > \ > EXPORT_SYMBOL(func); > > -define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, ) > +define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8) > define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, cpu_to_be16) > -define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, ) > +define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16) Also, did you test-build this patch? I think this just broke the build... ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbtft-bus.c: Removing that prohibited space before ')'
On Sat, 2020-06-27 at 00:51 -0400, B K Karthik wrote: > fbtft-bus.c: > > fixing ERROR: space prohibited before that close parenthesis ')' by removing > that space and ',' in line 65 and 67. [] > diff --git a/drivers/staging/fbtft/fbtft-bus.c > b/drivers/staging/fbtft/fbtft-bus.c > index 63c65dd67b17..847cbfbbd766 100644 > --- a/drivers/staging/fbtft/fbtft-bus.c > +++ b/drivers/staging/fbtft/fbtft-bus.c > @@ -62,9 +62,9 @@ out: > \ > } > \ > EXPORT_SYMBOL(func); > > -define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, ) > +define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8) > define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, cpu_to_be16) > -define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, ) > +define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16) Q: Did you compile the files modified by this patch before you submitted it? A: No ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbtft-bus.c:
On Sat, Jun 27, 2020 at 12:50:04AM -0400, B K Karthik wrote: > fbtft-bus.c: > > fixing ERROR: space prohibited before that close parenthesis ')' by removing > that space and ',' in line 65 and 67. > > Signed-off-by: B K Karthik > --- > drivers/staging/fbtft/fbtft-bus.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Your subject line is really odd :( Please read the following text from my patch-bot: - You did not specify a description of why the patch is needed, or possibly, any description at all, in the email body. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/SubmittingPatches for what is needed in order to properly describe the change. - You did not write a descriptive Subject: for the patch, allowing Greg, and everyone else, to know what this patch is all about. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/SubmittingPatches for what a proper Subject: line should look like. And fix up both the subject lines, and the changelog body text so that it looks correct. Look at the other patches for this code that have been accepted as examples of what to do. thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207383] [Regression] 5.7 amdgpu/polaris11 gpf: amdgpu_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=207383 --- Comment #25 from rtmasura+ker...@hotmail.com --- Same kernel (5.7.4) and I'll try to reproduce it, and if it happens I'll turn off the screen tear and try to reproduce again Let me know if that's anything I can provide you -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207383] [Regression] 5.7 amdgpu/polaris11 gpf: amdgpu_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=207383 --- Comment #24 from rtmasura+ker...@hotmail.com --- I've been up and stable on XFCE4 since that last message, but just crashed today with a bit of a different error. This happened after I turned on a screen tear fix: xfconf-query -c xfwm4 -p /general/vblank_mode -s glx I also didn't reboot to activate it, I just hot loaded it with: xfwm4 --replace --vblank=glx & Don't think that changes anything, but just in case. Not sure if it's related, I had a game idling on my monitor while I was cooking, and it's the first time I had played it. It was Battle of Wesnoth. Anyway, here's the log: Jun 26 21:08:03 abiggun kernel: general protection fault, probably for non-canonical address 0x3b963e011fb9f84: [#1] PREEMPT SMP NOPTI Jun 26 21:08:03 abiggun kernel: CPU: 4 PID: 362093 Comm: kworker/u12:1 Not tainted 5.7.4-arch1-1 #1 Jun 26 21:08:03 abiggun kernel: Hardware name: System manufacturer System Product Name/Crosshair IV Formula, BIOS 110208/24/2010 Jun 26 21:08:03 abiggun kernel: Workqueue: events_unbound commit_work [drm_kms_helper] Jun 26 21:08:03 abiggun kernel: RIP: 0010:amdgpu_dm_atomic_commit_tail+0x2aa/0x2310 [amdgpu] Jun 26 21:08:03 abiggun kernel: Code: 4f 08 8b 81 e0 02 00 00 41 83 c5 01 44 39 e8 0f 87 46 ff ff ff 48 83 bd f0 fc ff ff 00 0f 84 03 01 00 00 48 8b bd f0 fc ff ff <80> bf b0 01 00 00 01 0f 86 ac 00 00 00 48 b9 00 00 00 00 01 00 00 Jun 26 21:08:03 abiggun kernel: RSP: 0018:993cc4037af8 EFLAGS: 00010206 Jun 26 21:08:03 abiggun kernel: RAX: 0006 RBX: 931ae09c0800 RCX: 931bfe478000 Jun 26 21:08:03 abiggun kernel: RDX: 931bf2dd2600 RSI: c10a51a0 RDI: 03b963e011fb9f84 Jun 26 21:08:03 abiggun kernel: RBP: 993cc4037e60 R08: 0001 R09: 0001 Jun 26 21:08:03 abiggun kernel: R10: 0018 R11: 0018 R12: Jun 26 21:08:03 abiggun kernel: R13: 0006 R14: 931bd0450c00 R15: 931b3574dc80 Jun 26 21:08:03 abiggun kernel: FS: () GS:931c3fd0() knlGS: Jun 26 21:08:03 abiggun kernel: CS: 0010 DS: ES: CR0: 80050033 Jun 26 21:08:03 abiggun kernel: CR2: 7fe602dc0008 CR3: 00041808 CR4: 06e0 Jun 26 21:08:03 abiggun kernel: Call Trace: Jun 26 21:08:03 abiggun kernel: ? tomoyo_write_self+0x100/0x1d0 Jun 26 21:08:03 abiggun kernel: ? __switch_to_asm+0x34/0x70 Jun 26 21:08:03 abiggun kernel: ? __switch_to_asm+0x40/0x70 Jun 26 21:08:03 abiggun kernel: ? __switch_to_asm+0x34/0x70 Jun 26 21:08:03 abiggun kernel: ? __switch_to_asm+0x40/0x70 Jun 26 21:08:03 abiggun kernel: ? rescuer_thread+0x3f0/0x3f0 Jun 26 21:08:03 abiggun kernel: commit_tail+0x94/0x130 [drm_kms_helper] Jun 26 21:08:03 abiggun kernel: process_one_work+0x1da/0x3d0 Jun 26 21:08:03 abiggun kernel: ? rescuer_thread+0x3f0/0x3f0 Jun 26 21:08:03 abiggun kernel: worker_thread+0x4d/0x3e0 Jun 26 21:08:03 abiggun kernel: ? rescuer_thread+0x3f0/0x3f0 Jun 26 21:08:03 abiggun kernel: kthread+0x13e/0x160 Jun 26 21:08:03 abiggun kernel: ? __kthread_bind_mask+0x60/0x60 Jun 26 21:08:03 abiggun kernel: ret_from_fork+0x22/0x40 Jun 26 21:08:03 abiggun kernel: Modules linked in: snd_usb_audio snd_usbmidi_lib snd_rawmidi snd_seq_device mc hid_plantronics macvtap macvlan vhost_net vhost tap vhost_iotlb fuse xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter bridge stp llc rfkill tun lm92 hwmon_vid input_leds amdgpu squashfs nouveau loop edac_mce_amd kvm_amd ccp rng_core mxm_wmi snd_hda_codec_via gpu_sched snd_hda_codec_generic snd_hda_codec_hdmi ledtrig_audio kvm ttm snd_hda_intel snd_intel_dspcfg wmi_bmof snd_hda_codec drm_kms_helper snd_hda_core pcspkr sp5100_tco k10temp snd_hwdep snd_pcm cec i2c_piix4 joydev rc_core mousedev igb syscopyarea snd_timer sysfillrect snd sysimgblt i2c_algo_bit dca fb_sys_fops soundcore asus_atk0110 evdev mac_hid wmi drm crypto_user agpgart ip_tables x_tables ext4 crc16 mbcache jbd2 ecb crypto_simd cryptd Jun 26 21:08:03 abiggun kernel: glue_helper xts hid_generic usbhid hid dm_crypt raid456 libcrc32c crc32c_generic async_raid6_recov async_memcpy async_pq async_xor xor async_tx ohci_pci raid6_pq md_mod ehci_pci ehci_hcd ohci_hcd xhci_pci xhci_hcd ata_generic pata_acpi pata_jmicron vfio_pci irqbypass vfio_virqfd vfio_iommu_type1 vfio dm_mod Jun 26 21:08:03 abiggun kernel: ---[ end trace 4e7c8ad2195077a2 ]--- Jun 26 21:08:03 abiggun kernel: RIP: 0010:amdgpu_dm_atomic_commit_tail+0x2aa/0x2310 [amdgpu] Jun 26 21:08:03 abiggun kernel: Code: 4f 08 8b 81 e0 02 00 00 41 83 c5 01 44 39 e8 0f 87 46 ff ff ff 48 83 bd f0 fc ff ff 00 0f 84 03 01 00 00 48 8b bd f0 fc ff ff <80> bf b0 01 00 00 01 0f 86 ac 00 00 00 48 b9 00 00 00 00 01 00 00 Jun 26 21:08:03 abiggun kernel: RSP: 0018:993c
Re: [PATCH] drm: gma500: Drop surplus include
Hi Linus, I love your patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.8-rc2 next-20200626] [cannot apply to drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Linus-Walleij/drm-gma500-Drop-surplus-include/20200627-060308 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-allyesconfig (attached as .config) compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project ee3620643dfc88a178fa4ca116cf83014e4ee547) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): >> drivers/gpu/drm/gma500/mdfld_dsi_output.c:452:8: error: implicit declaration >> of function 'gpio_request' [-Werror,-Wimplicit-function-declaration] ret = gpio_request(gpio, "gfx"); ^ >> drivers/gpu/drm/gma500/mdfld_dsi_output.c:458:8: error: implicit declaration >> of function 'gpio_direction_output' [-Werror,-Wimplicit-function-declaration] ret = gpio_direction_output(gpio, 1); ^ >> drivers/gpu/drm/gma500/mdfld_dsi_output.c:464:2: error: implicit declaration >> of function 'gpio_get_value' [-Werror,-Wimplicit-function-declaration] gpio_get_value(128); ^ >> drivers/gpu/drm/gma500/mdfld_dsi_output.c:467:6: error: implicit declaration >> of function 'gpio_is_valid' [-Werror,-Wimplicit-function-declaration] if (gpio_is_valid(gpio)) ^ drivers/gpu/drm/gma500/mdfld_dsi_output.c:467:6: note: did you mean 'uuid_is_valid'? include/linux/uuid.h:92:19: note: 'uuid_is_valid' declared here bool __must_check uuid_is_valid(const char *uuid); ^ >> drivers/gpu/drm/gma500/mdfld_dsi_output.c:468:3: error: implicit declaration >> of function 'gpio_free' [-Werror,-Wimplicit-function-declaration] gpio_free(gpio); ^ 5 errors generated. vim +/gpio_request +452 drivers/gpu/drm/gma500/mdfld_dsi_output.c 026abc333205c1 Kirill A. Shutemov 2012-03-08 434 026abc333205c1 Kirill A. Shutemov 2012-03-08 435 int mdfld_dsi_panel_reset(int pipe) 026abc333205c1 Kirill A. Shutemov 2012-03-08 436 { 026abc333205c1 Kirill A. Shutemov 2012-03-08 437 unsigned gpio; 026abc333205c1 Kirill A. Shutemov 2012-03-08 438 int ret = 0; 026abc333205c1 Kirill A. Shutemov 2012-03-08 439 026abc333205c1 Kirill A. Shutemov 2012-03-08 440 switch (pipe) { 026abc333205c1 Kirill A. Shutemov 2012-03-08 441 case 0: 026abc333205c1 Kirill A. Shutemov 2012-03-08 442 gpio = 128; 026abc333205c1 Kirill A. Shutemov 2012-03-08 443 break; 026abc333205c1 Kirill A. Shutemov 2012-03-08 444 case 2: 026abc333205c1 Kirill A. Shutemov 2012-03-08 445 gpio = 34; 026abc333205c1 Kirill A. Shutemov 2012-03-08 446 break; 026abc333205c1 Kirill A. Shutemov 2012-03-08 447 default: 026abc333205c1 Kirill A. Shutemov 2012-03-08 448 DRM_ERROR("Invalid output\n"); 026abc333205c1 Kirill A. Shutemov 2012-03-08 449 return -EINVAL; 026abc333205c1 Kirill A. Shutemov 2012-03-08 450 } 026abc333205c1 Kirill A. Shutemov 2012-03-08 451 026abc333205c1 Kirill A. Shutemov 2012-03-08 @452 ret = gpio_request(gpio, "gfx"); 026abc333205c1 Kirill A. Shutemov 2012-03-08 453 if (ret) { 026abc333205c1 Kirill A. Shutemov 2012-03-08 454 DRM_ERROR("gpio_rqueset failed\n"); 026abc333205c1 Kirill A. Shutemov 2012-03-08 455 return ret; 026abc333205c1 Kirill A. Shutemov 2012-03-08 456 } 026abc333205c1 Kirill A. Shutemov 2012-03-08 457 026abc333205c1 Kirill A. Shutemov 2012-03-08 @458 ret = gpio_direction_output(gpio, 1); 026abc333205c1 Kirill A. Shutemov 2012-03-08 459 if (ret) { 026abc333205c1 Kirill A. Shutemov 2012-03-08 460 DRM_ERROR("gpio_direction_output failed\n"); 026abc333205c1 Kirill A. Shutemov 2012-03-08 461 goto gpio_error; 026abc333205c1 Kirill A. Shutemov 2012-03-08
Re: [PATCH v4] drm/mediatek: check plane visibility in atomic_update
Hi, Hsin-Yi: Hsin-Yi Wang 於 2020年6月22日 週一 下午11:57寫道: > > Disable the plane if it's not visible. Otherwise mtk_ovl_layer_config() > would proceed with invalid plane and we may see vblank timeout. > Applied to mediatek-drm-next [1], thanks. [1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next Regards, Chun-Kuang. > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.") > Signed-off-by: Hsin-Yi Wang > Reviewed-by: Chun-Kuang Hu > Reviewed-by: Tomasz Figa > --- > v4: fix commit message > v3: Address comment > v2: Add fixes tag > --- > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 25 ++-- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > index c2bd683a87c8..92141a19681b 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > @@ -164,6 +164,16 @@ static int mtk_plane_atomic_check(struct drm_plane > *plane, >true, true); > } > > +static void mtk_plane_atomic_disable(struct drm_plane *plane, > +struct drm_plane_state *old_state) > +{ > + struct mtk_plane_state *state = to_mtk_plane_state(plane->state); > + > + state->pending.enable = false; > + wmb(); /* Make sure the above parameter is set before update */ > + state->pending.dirty = true; > +} > + > static void mtk_plane_atomic_update(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > @@ -178,6 +188,11 @@ static void mtk_plane_atomic_update(struct drm_plane > *plane, > if (!crtc || WARN_ON(!fb)) > return; > > + if (!plane->state->visible) { > + mtk_plane_atomic_disable(plane, old_state); > + return; > + } > + > gem = fb->obj[0]; > mtk_gem = to_mtk_gem_obj(gem); > addr = mtk_gem->dma_addr; > @@ -200,16 +215,6 @@ static void mtk_plane_atomic_update(struct drm_plane > *plane, > state->pending.dirty = true; > } > > -static void mtk_plane_atomic_disable(struct drm_plane *plane, > -struct drm_plane_state *old_state) > -{ > - struct mtk_plane_state *state = to_mtk_plane_state(plane->state); > - > - state->pending.enable = false; > - wmb(); /* Make sure the above parameter is set before update */ > - state->pending.dirty = true; > -} > - > static const struct drm_plane_helper_funcs mtk_plane_helper_funcs = { > .prepare_fb = drm_gem_fb_prepare_fb, > .atomic_check = mtk_plane_atomic_check, > -- > 2.27.0.111.gc72c7da667-goog > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4] Revert "drm/amd/display: Expose connector VRR range via debugfs"
From: Bhanuprakash Modem v3: * Rebase (Manasi) v2: * Rebase (Manasi) As both VRR min and max are already part of drm_display_info, drm can expose this VRR range for each connector. Hence this logic should move to core DRM. This reverts commit 727962f030c23422a01e8b22d0f463815fb15ec4. Signed-off-by: Bhanuprakash Modem Cc: Nicholas Kazlauskas Cc: Harry Wentland Cc: Alex Deucher Cc: Manasi Navare Cc: AMD gfx Reviewed-by: Nicholas Kazlauskas --- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 20 --- 1 file changed, 20 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c index 1d692f4f42f3..b246354967bc 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c @@ -820,24 +820,6 @@ static int output_bpc_show(struct seq_file *m, void *data) return res; } -/* - * Returns the min and max vrr vfreq through the connector's debugfs file. - * Example usage: cat /sys/kernel/debug/dri/0/DP-1/vrr_range - */ -static int vrr_range_show(struct seq_file *m, void *data) -{ - struct drm_connector *connector = m->private; - struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); - - if (connector->status != connector_status_connected) - return -ENODEV; - - seq_printf(m, "Min: %u\n", (unsigned int)aconnector->min_vfreq); - seq_printf(m, "Max: %u\n", (unsigned int)aconnector->max_vfreq); - - return 0; -} - #ifdef CONFIG_DRM_AMD_DC_HDCP /* * Returns the HDCP capability of the Display (1.4 for now). @@ -1001,7 +983,6 @@ static ssize_t dp_dpcd_data_read(struct file *f, char __user *buf, DEFINE_SHOW_ATTRIBUTE(dmub_fw_state); DEFINE_SHOW_ATTRIBUTE(dmub_tracebuffer); DEFINE_SHOW_ATTRIBUTE(output_bpc); -DEFINE_SHOW_ATTRIBUTE(vrr_range); #ifdef CONFIG_DRM_AMD_DC_HDCP DEFINE_SHOW_ATTRIBUTE(hdcp_sink_capability); #endif @@ -1058,7 +1039,6 @@ static const struct { {"link_settings", &dp_link_settings_debugfs_fops}, {"phy_settings", &dp_phy_settings_debugfs_fop}, {"test_pattern", &dp_phy_test_pattern_fops}, - {"vrr_range", &vrr_range_fops}, #ifdef CONFIG_DRM_AMD_DC_HDCP {"hdcp_sink_capability", &hdcp_sink_capability_fops}, #endif -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: omapdrm: Delete surplus GPIO includes
The OMAP DRM driver includes into the two hdmi4.c and hdmi5.c files but does not use any symbols from these files. Drop the includes. Cc: Tomi Valkeinen Cc: Tony Lindgren Cc: Jyri Sarha Signed-off-by: Linus Walleij --- drivers/gpu/drm/omapdrm/dss/hdmi4.c | 1 - drivers/gpu/drm/omapdrm/dss/hdmi5.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c index 2578c95570f6..a14fbf06cb30 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c index 4d4c1fabd0a1..b738d9750686 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c @@ -24,7 +24,6 @@ #include #include #include -#include #include #include #include -- 2.25.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: gma500: Drop surplus include
This file includes but does not use any symbols from it, drop the include. Cc: Patrik Jakobsson Signed-off-by: Linus Walleij --- drivers/gpu/drm/gma500/psb_intel_drv.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/gma500/psb_intel_drv.h b/drivers/gpu/drm/gma500/psb_intel_drv.h index fb601983cef0..9221d1f545b0 100644 --- a/drivers/gpu/drm/gma500/psb_intel_drv.h +++ b/drivers/gpu/drm/gma500/psb_intel_drv.h @@ -13,7 +13,6 @@ #include #include #include -#include #include "gma_display.h" /* -- 2.25.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] amdgpu, amdkfd, radeon drm-next-5.9
Hi Dave, Daniel, First pull for 5.9. Big feature here is initial support for a new GPU, sienna cichlid. The following changes since commit 9ca1f474cea0edc14a1d7ec933e5472c0ff115d3: Merge tag 'amd-drm-next-5.8-2020-05-27' of git://people.freedesktop.org/~agd5f/linux into drm-next (2020-05-28 16:10:17 +1000) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux tags/amd-drm-next-5.9-2020-06-26 for you to fetch changes up to 0749c81d923d3d5dcd0de13e32061449d393f1eb: drm/amd/powerplay: Fix NULL dereference in lock_bus() on Vega20 w/o RAS (2020-06-25 13:33:16 -0400) amd-drm-next-5.9-2020-06-26: amdgpu: - DC DMUB updates - HDCP fixes - Thermal interrupt fixes - Add initial support for Sienna Cichlid GPU - Add support for unique id on Arcturus - Major swSMU code cleanup - Skip BAR resizing if the bios already did id - Fixes for DCN bandwidth calculations - Runtime PM reference count fixes - Add initial UVD support for SI - Add support for ASSR on eDP links - Lots of misc fixes and cleanups - Enable runtime PM on vega10 boards that support BACO - RAS fixes - SR-IOV fixes - Use IP discovery table on renoir - DC stream synchronization fixes amdkfd: - Track SDMA usage per process - Fix GCC10 compiler warnings - Locking fix radeon: - Default to on chip GART for AGP boards on all arches - Runtime PM reference count fixes UAPI: - Update comments to clarify MTYPE Aditya Pakki (2): drm/radeon: fix multiple reference count leak drm/radeon: Fix reference count leaks caused by pm_runtime_get_sync Alex Deucher (36): drm/amdgpu: simplify ATIF backlight handling drm/amdgpu/sdma4: add renoir to powergating setup drm/amdgpu/gfx10: add navi12 to gfxoff case drm/amdgpu: simplify raven and renoir checks drm/amdgpu: simplify CZ/ST and KV/KB/ML checks drm/amdgpu: simplify mec2 fw check drm/amdgpu/sdma4: simplify the logic around powering up sdma drm/amdgpu: put some case statments in family order drm/amdgpu/gmc10: program the smallK fragment size drm/amdgpu/pm: return an error during GPU reset or suspend (v2) drm/amdgpu: skip gpu_info firmware if discovery info is available drm/amdgpu: clean up discovery testing drm/amdgpu: use IP discovery table for renoir drm/amdgpu/nv: allow access to SDMA status registers drm/amdgpu/nv: remove some dead code drm/amdgpu/nv: enable init reset check drm/amdgpu/fru: fix header guard and include header drm/amdgpu/mes10.1: add no scheduler flag for mes drm/amdgpu/vcn3.0: schedule instance 0 for decode and 1 for encode drm/amdgpu/display: fix build without CONFIG_DRM_AMD_DC_DCN3_0 Revert "drm/[radeon|amdgpu]: Replace one-element array and use struct_size() helper" drm/amdgpu/fence: use the no_scheduler flag drm/amdgpu/display: use blanked rather than plane state for sync groups drm/amdgpu: skip BAR resizing if the bios already did it drm/amdgpu/pm: update comment to clarify Overdrive interfaces drm/amdgpu: fix documentation around busy_percentage drm/amdgpu/fence: fix ref count leak when pm_runtime_get_sync fails drm/amdkfd: fix ref count leak when pm_runtime_get_sync fails drm/amdgpu/debugfs: fix ref count leak when pm_runtime_get_sync fails drm/amdgpu/pm: fix ref count leak when pm_runtime_get_sync fails drm/amdgpu/display bail early in dm_pp_get_static_clocks drm/amdgpu/display: properly guard the calls to swSMU functions drm/amdgpu/uvd3.x: fix register definition warnings drm/amdgpu: make sure to reserve tmr region on all asics which support it drm/amdgpu: rework runtime pm enablement for BACO drm/amdgpu: enable runtime pm on vega10 when noretry=0 Alvin Lee (5): drm/amd/display: Disable PG on NV12 drm/amd/display: Don't compare same stream for synchronized vblank drm/amd/display: Get num_chans from VBIOS table drm/amd/display: Update DCN3 bounding box drm/amd/display: Update bounding box states (v2) Anthony Koo (11): drm/amd/display: FW release 1.0.10 drm/amd/display: FW Release 1.0.11 drm/amd/display: combine public interfaces into single header drm/amd/display: [FW Promotion] Release 1.0.12 drm/amd/display: [FW Promotion] Release 1.0.13 drm/amd/display: [FW Promotion] Release 1.0.14 drm/amd/display: [FW Promotion] Release 1.0.15 drm/amd/display: [FW Promotion] Release 1.0.16 drm/amd/display: [FW Promotion] Release 1.0.17 drm/amd/display: [FW Promotion] Release 1.0.18 drm/amd/display: [FW Promotion] Release 1.0.19 Aric Cyr (10): drm/amd/display: 3.2.85 drm/amd/display: 3.2.86 drm/amd/display: Handle link loss interrupt better drm/amd/display: Guard against invalid array acces
Re: [PATCH] drm/atomic_helper: duplicate state for drm_private_obj
On Fri, Jun 26, 2020 at 08:25:18AM -0700, Rob Clark wrote: > On Fri, Jun 26, 2020 at 6:46 AM Daniel Vetter wrote: > > > > On Thu, Jun 25, 2020 at 09:24:38AM -0700, Rob Clark wrote: > > > On Thu, Jun 25, 2020 at 8:55 AM Daniel Vetter > > > wrote: > > > > > > > > On Thu, Jun 25, 2020 at 4:17 PM Rob Clark wrote: > > > > > > > > > > On Thu, Jun 25, 2020 at 5:35 AM Daniel Vetter > > > > > wrote: > > > > > > > > > > > > On Thu, Jun 25, 2020 at 1:58 PM Shawn Guo > > > > > > wrote: > > > > > > > > > > > > > > From: Shawn Guo > > > > > > > > > > > > > > The msm/mdp5 driver uses drm_private_obj as its global atomic > > > > > > > state, > > > > > > > which keeps the assignment of hwpipe to plane. With > > > > > > > drm_private_obj > > > > > > > missing from duplicate state call, mdp5 suspend works with no > > > > > > > problem > > > > > > > only for the very first time. Any subsequent suspend will hit the > > > > > > > following warning, because hwpipe assignment doesn't get > > > > > > > duplicated for > > > > > > > suspend state. Adding drm_private_obj handling for duplicate > > > > > > > state call > > > > > > > fixes the problem. > > > > > > > > > > > > If the driver needs a private state, it's supposed to duplicate that > > > > > > in its atomic_check functionality. This isn't the helper's job. > > > > > > > > > > > > If this is a bug in msm code, then pretty sure if you try hard > > > > > > enough, > > > > > > you can hit the exact same bug from userspace too. Maybe good idea > > > > > > to > > > > > > try and reproduce this with igt or something. > > > > > > > > > > The problem is how duplicate_state is used by the atomic > > > > > suspend/resume helpers. They duplicate the running state on suspend, > > > > > forgetting to duplicate the global state. Then everything is > > > > > disabled, the driver correctly duplicates and updates it's global > > > > > atomic state releasing the hwpipe. > > > > > > > > > > But then on resume, we are re-applying plane state that thinks it > > > > > already has a hwpipe assigned (because that is part of the plane state > > > > > that was duplicated), without reapplying the matching global state. > > > > > > > > > > On a normal atomic commit, we would duplicate the plane state that has > > > > > the hwpipe disabled, which would be in sync with the drivers global > > > > > state. But since that is not what the atomic resume helper does, we > > > > > hit the situation where the plane state and the global state are out > > > > > of sync. > > > > > > > > > > So the driver is dtrt, the problem really is with the helpers. I > > > > > think this patch is the right thing to do. It is incorrect for the > > > > > suspend/resume helpers to assume that they can re-apply duplicated > > > > > state without including the global state. > > > > > > > > Hm, this is a bit awkward. Generally the assumption is that you should > > > > be recomputing the derived state (like hwpipe) no matter what. If your > > > > driver doesn't do that, then all kinds of things can leak from the > > > > pre-resume to the post-resume side of things, that's kinda why I'm not > > > > thrilled with this patch, I think it has good potential to open up a > > > > can of worms. Iirc this patch has come up in the past, and in those > > > > cases it was a driver bug. > > > > > > > > For this case, why is msm reusing a hw pipe assignment of a disabled > > > > plane? > > > > > > Because it is part of the plane state that is being restored. > > > > > > Since resume uses the old state saved before the > > > drm_atomic_helper_disable_all(), rather than duplicating the current > > > state, we end up with this mismatch between global and plane state. I > > > think stashing away the old state is probably ok, but we can't just do > > > it piecemeal without including the global state. > > > > > > I suppose part of the problem is the hwpipe (and other such > > > dynamically assigned resources) touch both private and plane (and > > > crtc) state. The global state object knows which resources are > > > assigned to which plane/crtc. But the plane/crtc state knows which of > > > the (potentially) two hwpipe/mixers is "left" (primary) and "right" > > > (secondary). > > > > Yeah I get all that, what I meant is: Why don't you just blindly recompute > > the hwpipe assignment every time a full modeset is done? Caching it for > > pure plane flips makes sense, but drm_crtc_needs_modset == true and just > > throw it all overboard and assign new ones I think would also solve this > > problem. Since the hwpipe global state would indicate that all pipes are > > unallocated that should work (I hope). > > > > Imo just recomputing state is a good atomic pattern, it avoids drivers > > getting stuck in a corner somewhere you can't reset them out of anymore. > > > > My question here was, why can't you do that? > > We do release the hwpipe on disable, and that is where things are > getting out of sync. > > I suppose we could do so
Re: [RFC v7 03/11] drm/vblank: Add vblank works
On Wed, Jun 24, 2020 at 07:03:10PM -0400, Lyude Paul wrote: > Add some kind of vblank workers. The interface is similar to regular > delayed works, and is mostly based off kthread_work. It allows for > scheduling delayed works that execute once a particular vblank sequence > has passed. It also allows for accurate flushing of scheduled vblank > works - in that flushing waits for both the vblank sequence and job > execution to complete, or for the work to get cancelled - whichever > comes first. > > Whatever hardware programming we do in the work must be fast (must at > least complete during the vblank or scanout period, sometimes during the > first few scanlines of the vblank). As such we use a high-priority > per-CRTC thread to accomplish this. > > Changes since v6: > * Get rid of ->pending and seqcounts, and implement flushing through > simpler means - danvet > * Get rid of work_lock, just use drm_device->event_lock > * Move drm_vblank_work item cleanup into drm_crtc_vblank_off() so that > we ensure that all vblank work has finished before disabling vblanks > * Add checks into drm_crtc_vblank_reset() so we yell if it gets called > while there's vblank workers active > * Grab event_lock in both drm_crtc_vblank_on()/drm_crtc_vblank_off(), > the main reason for this is so that other threads calling > drm_vblank_work_schedule() are blocked from attempting to schedule > while we're in the middle of enabling/disabling vblanks. > * Move drm_handle_vblank_works() call below drm_handle_vblank_events() > * Simplify drm_vblank_work_cancel_sync() > * Fix drm_vblank_work_cancel_sync() documentation > * Move wake_up_all() calls out of spinlock where we can. The only one I > left was the call to wake_up_all() in drm_vblank_handle_works() as > this seemed like it made more sense just living in that function > (which is all technically under lock) > * Move drm_vblank_work related functions into their own source files > * Add drm_vblank_internal.h so we can export some functions we don't > want drivers using, but that we do need to use in drm_vblank_work.c > * Add a bunch of documentation > Changes since v4: > * Get rid of kthread interfaces we tried adding and move all of the > locking into drm_vblank.c. For implementing drm_vblank_work_flush(), > we now use a wait_queue and sequence counters in order to > differentiate between multiple work item executions. > * Get rid of drm_vblank_work_cancel() - this would have been pretty > difficult to actually reimplement and it occurred to me that neither > nouveau or i915 are even planning to use this function. Since there's > also no async cancel function for most of the work interfaces in the > kernel, it seems a bit unnecessary anyway. > * Get rid of to_drm_vblank_work() since we now are also able to just > pass the struct drm_vblank_work to work item callbacks anyway > Changes since v3: > * Use our own spinlocks, don't integrate so tightly with kthread_works > Changes since v2: > * Use kthread_workers instead of reinventing the wheel. > > Cc: Daniel Vetter > Cc: Tejun Heo > Cc: dri-devel@lists.freedesktop.org > Cc: nouv...@lists.freedesktop.org > Co-developed-by: Ville Syrjälä > Signed-off-by: Lyude Paul I found a bunch of tiny details below, but overall looks great and thanks for polishing the kerneldoc. With the details addressed one way or another: Reviewed-by: Daniel Vetter But feel free to resend and poke me again if you want me to recheck the details that needed changing. Cheers, Daniel > --- > Documentation/gpu/drm-kms.rst | 15 ++ > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/drm_vblank.c | 55 +++-- > drivers/gpu/drm/drm_vblank_internal.h | 19 ++ > drivers/gpu/drm/drm_vblank_work.c | 259 + > drivers/gpu/drm/drm_vblank_work_internal.h | 24 ++ > include/drm/drm_vblank.h | 20 ++ > include/drm/drm_vblank_work.h | 71 ++ > 8 files changed, 447 insertions(+), 18 deletions(-) > create mode 100644 drivers/gpu/drm/drm_vblank_internal.h > create mode 100644 drivers/gpu/drm/drm_vblank_work.c > create mode 100644 drivers/gpu/drm/drm_vblank_work_internal.h > create mode 100644 include/drm/drm_vblank_work.h > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > index 975cfeb8a3532..3c5ae4f6dfd23 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -543,3 +543,18 @@ Vertical Blanking and Interrupt Handling Functions > Reference > > .. kernel-doc:: drivers/gpu/drm/drm_vblank.c > :export: > + > +Vertical Blank Work > +=== > + > +.. kernel-doc:: drivers/gpu/drm/drm_vblank_work.c > + :doc: vblank works > + > +Vertical Blank Work Functions Reference > +--- > + > +.. kernel-doc:: include/drm/drm_vblank_work.h > + :internal: > + > +.. kernel-doc:: drivers/gpu/drm/drm_vblank_work
Re: [RFC v7 02/11] drm/vblank: Use spin_(un)lock_irq() in drm_crtc_vblank_off()
On Wed, Jun 24, 2020 at 07:03:09PM -0400, Lyude Paul wrote: > This got me confused for a bit while looking over this code: I had been > planning on adding some blocking function calls into this function, but > seeing the irqsave/irqrestore variants of spin_(un)lock() didn't make it > very clear whether or not that would actually be safe. > > So I went ahead and reviewed every single driver in the kernel that uses > this function, and they all fall into three categories: > > * Driver probe code > * ->atomic_disable() callbacks > * Legacy modesetting callbacks > > All of these will be guaranteed to have IRQs enabled, which means it's > perfectly safe to block here. Just to make things a little less > confusing to others in the future, let's switch over to > spin_lock_irq()/spin_unlock_irq() to make that fact a little more > obvious. > > Signed-off-by: Lyude Paul > Cc: Daniel Vetter > Cc: Ville Syrjälä I think the patch is correct, but now we're having a bit a inconsistency, since all other functions where the same applies still use _irqsave. I looked through the file and I think drm_vblank_get, drm_crtc_vblank_reset, drm_crtc_vblank_on and drm_legacy_vblank_post_modeset, drm_queue_vblank_event and drm_crtc_queue_sequence_ioctl are all candiates for the same cleanup. Maybe follow up patches for less confusion? On this: Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_vblank.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index ce5c1e1d29963..e895f5331fdb4 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -1283,13 +1283,12 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc) > struct drm_pending_vblank_event *e, *t; > > ktime_t now; > - unsigned long irqflags; > u64 seq; > > if (drm_WARN_ON(dev, pipe >= dev->num_crtcs)) > return; > > - spin_lock_irqsave(&dev->event_lock, irqflags); > + spin_lock_irq(&dev->event_lock); > > spin_lock(&dev->vbl_lock); > drm_dbg_vbl(dev, "crtc %d, vblank enabled %d, inmodeset %d\n", > @@ -1325,7 +1324,7 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc) > drm_vblank_put(dev, pipe); > send_vblank_event(dev, e, seq, now); > } > - spin_unlock_irqrestore(&dev->event_lock, irqflags); > + spin_unlock_irq(&dev->event_lock); > > /* Will be reset by the modeset helpers when re-enabling the crtc by >* calling drm_calc_timestamping_constants(). */ > -- > 2.26.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] backlight: sky81452: Convert to GPIO descriptors
The SKY81452 backlight driver just obtains a GPIO (named "gpios" in the device tree) drives it high and leaves it high until the driver is removed. Switch to use GPIO descriptors for this, simple and straight-forward. Cc: Gyungoh Yoo Signed-off-by: Linus Walleij --- drivers/video/backlight/sky81452-backlight.c | 18 -- .../linux/platform_data/sky81452-backlight.h | 6 -- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/drivers/video/backlight/sky81452-backlight.c b/drivers/video/backlight/sky81452-backlight.c index 2355f00f5773..81d2c8f3ca50 100644 --- a/drivers/video/backlight/sky81452-backlight.c +++ b/drivers/video/backlight/sky81452-backlight.c @@ -8,12 +8,11 @@ #include #include -#include +#include #include #include #include #include -#include #include #include #include @@ -182,7 +181,7 @@ static struct sky81452_bl_platform_data *sky81452_bl_parse_dt( pdata->ignore_pwm = of_property_read_bool(np, "skyworks,ignore-pwm"); pdata->dpwm_mode = of_property_read_bool(np, "skyworks,dpwm-mode"); pdata->phase_shift = of_property_read_bool(np, "skyworks,phase-shift"); - pdata->gpio_enable = of_get_gpio(np, 0); + pdata->gpiod_enable = devm_gpiod_get_optional(dev, NULL, GPIOD_OUT_HIGH); ret = of_property_count_u32_elems(np, "led-sources"); if (ret < 0) { @@ -264,15 +263,6 @@ static int sky81452_bl_probe(struct platform_device *pdev) return PTR_ERR(pdata); } - if (gpio_is_valid(pdata->gpio_enable)) { - ret = devm_gpio_request_one(dev, pdata->gpio_enable, - GPIOF_OUT_INIT_HIGH, "sky81452-en"); - if (ret < 0) { - dev_err(dev, "failed to request GPIO. err=%d\n", ret); - return ret; - } - } - ret = sky81452_bl_init_device(regmap, pdata); if (ret < 0) { dev_err(dev, "failed to initialize. err=%d\n", ret); @@ -312,8 +302,8 @@ static int sky81452_bl_remove(struct platform_device *pdev) bd->props.brightness = 0; backlight_update_status(bd); - if (gpio_is_valid(pdata->gpio_enable)) - gpio_set_value_cansleep(pdata->gpio_enable, 0); + if (pdata->gpiod_enable) + gpiod_set_value_cansleep(pdata->gpiod_enable, 0); return 0; } diff --git a/include/linux/platform_data/sky81452-backlight.h b/include/linux/platform_data/sky81452-backlight.h index 02653d92d84f..d6f46670d923 100644 --- a/include/linux/platform_data/sky81452-backlight.h +++ b/include/linux/platform_data/sky81452-backlight.h @@ -9,11 +9,13 @@ #ifndef _SKY81452_BACKLIGHT_H #define _SKY81452_BACKLIGHT_H +#include + /** * struct sky81452_platform_data * @name: backlight driver name. If it is not defined, default name is lcd-backlight. - * @gpio_enable:GPIO number which control EN pin + * @gpios_enable:GPIO descriptor which control EN pin * @enable:Enable mask for current sink channel 1, 2, 3, 4, 5 and 6. * @ignore_pwm:true if DPWMI should be ignored. * @dpwm_mode: true is DPWM dimming mode, otherwise Analog dimming mode. @@ -23,7 +25,7 @@ */ struct sky81452_bl_platform_data { const char *name; - int gpio_enable; + struct gpio_desc *gpiod_enable; unsigned int enable; bool ignore_pwm; bool dpwm_mode; -- 2.25.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] backlight: sky81452: Privatize platform data
The only way the platform data for the SKY81452 ever gets populated is through the device tree. The MFD device is bothered with this for no reason at all. Just allocate the platform data in the driver and be happy. Cc: Gyungoh Yoo Signed-off-by: Linus Walleij --- drivers/mfd/sky81452.c| 2 - drivers/video/backlight/sky81452-backlight.c | 34 + include/linux/mfd/sky81452.h | 2 - .../linux/platform_data/sky81452-backlight.h | 37 --- 4 files changed, 27 insertions(+), 48 deletions(-) delete mode 100644 include/linux/platform_data/sky81452-backlight.h diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c index 76eedfae8553..3ad35bf0c015 100644 --- a/drivers/mfd/sky81452.c +++ b/drivers/mfd/sky81452.c @@ -47,8 +47,6 @@ static int sky81452_probe(struct i2c_client *client, memset(cells, 0, sizeof(cells)); cells[0].name = "sky81452-backlight"; cells[0].of_compatible = "skyworks,sky81452-backlight"; - cells[0].platform_data = pdata->bl_pdata; - cells[0].pdata_size = sizeof(*pdata->bl_pdata); cells[1].name = "sky81452-regulator"; cells[1].platform_data = pdata->regulator_init_data; cells[1].pdata_size = sizeof(*pdata->regulator_init_data); diff --git a/drivers/video/backlight/sky81452-backlight.c b/drivers/video/backlight/sky81452-backlight.c index 81d2c8f3ca50..83ccb3d940fa 100644 --- a/drivers/video/backlight/sky81452-backlight.c +++ b/drivers/video/backlight/sky81452-backlight.c @@ -15,7 +15,6 @@ #include #include #include -#include #include /* registers */ @@ -41,6 +40,29 @@ #define SKY81452_DEFAULT_NAME "lcd-backlight" #define SKY81452_MAX_BRIGHTNESS(SKY81452_CS + 1) +/** + * struct sky81452_platform_data + * @name: backlight driver name. + If it is not defined, default name is lcd-backlight. + * @gpios_enable:GPIO descriptor which control EN pin + * @enable:Enable mask for current sink channel 1, 2, 3, 4, 5 and 6. + * @ignore_pwm:true if DPWMI should be ignored. + * @dpwm_mode: true is DPWM dimming mode, otherwise Analog dimming mode. + * @phase_shift:true is phase shift mode. + * @short_detecion_threshold: It should be one of 4, 5, 6 and 7V. + * @boost_current_limit: It should be one of 2300, 2750mA. + */ +struct sky81452_bl_platform_data { + const char *name; + struct gpio_desc *gpiod_enable; + unsigned int enable; + bool ignore_pwm; + bool dpwm_mode; + bool phase_shift; + unsigned int short_detection_threshold; + unsigned int boost_current_limit; +}; + #define CTZ(b) __builtin_ctz(b) static int sky81452_bl_update_status(struct backlight_device *bd) @@ -251,17 +273,15 @@ static int sky81452_bl_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct regmap *regmap = dev_get_drvdata(dev->parent); - struct sky81452_bl_platform_data *pdata = dev_get_platdata(dev); + struct sky81452_bl_platform_data *pdata; struct backlight_device *bd; struct backlight_properties props; const char *name; int ret; - if (!pdata) { - pdata = sky81452_bl_parse_dt(dev); - if (IS_ERR(pdata)) - return PTR_ERR(pdata); - } + pdata = sky81452_bl_parse_dt(dev); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); ret = sky81452_bl_init_device(regmap, pdata); if (ret < 0) { diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky81452.h index d469aa481243..b08570ff34df 100644 --- a/include/linux/mfd/sky81452.h +++ b/include/linux/mfd/sky81452.h @@ -9,11 +9,9 @@ #ifndef _SKY81452_H #define _SKY81452_H -#include #include struct sky81452_platform_data { - struct sky81452_bl_platform_data *bl_pdata; struct regulator_init_data *regulator_init_data; }; diff --git a/include/linux/platform_data/sky81452-backlight.h b/include/linux/platform_data/sky81452-backlight.h deleted file mode 100644 index d6f46670d923.. --- a/include/linux/platform_data/sky81452-backlight.h +++ /dev/null @@ -1,37 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * sky81452.h SKY81452 backlight driver - * - * Copyright 2014 Skyworks Solutions Inc. - * Author : Gyungoh Yoo - */ - -#ifndef _SKY81452_BACKLIGHT_H -#define _SKY81452_BACKLIGHT_H - -#include - -/** - * struct sky81452_platform_data - * @name: backlight driver name. - If it is not defined, default name is lcd-backlight. - * @gpios_enable:GPIO descriptor which control EN pin - * @enable:Enable mask for current sink channel 1, 2, 3, 4, 5 and 6. - * @ignore_pwm:true if DPWMI should be ignored. - * @dpwm_mode: true is DPWM dimming mode, otherwise Analog dimming mode. - * @phase_shift:true is phase shift mode. - * @short_detecion_threshold: It should be one of 4, 5, 6 and 7V. - * @boost_c
[PATCH v2 6/6] drm/msm/a6xx: Add support for per-instance pagetables
Add support for using per-instance pagetables if all the dependencies are available. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 43 +++ drivers/gpu/drm/msm/msm_ringbuffer.h | 1 + 2 files changed, 44 insertions(+) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index aa53f47b7e8b..95ed2ceac121 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -79,6 +79,34 @@ static void get_stats_counter(struct msm_ringbuffer *ring, u32 counter, OUT_RING(ring, upper_32_bits(iova)); } +static void a6xx_set_pagetable(struct msm_gpu *gpu, struct msm_ringbuffer *ring, + struct msm_file_private *ctx) +{ + phys_addr_t ttbr; + u32 asid; + + if (msm_iommu_pagetable_params(ctx->aspace->mmu, &ttbr, &asid)) + return; + + /* Execute the table update */ + OUT_PKT7(ring, CP_SMMU_TABLE_UPDATE, 4); + OUT_RING(ring, lower_32_bits(ttbr)); + OUT_RING(ring, (((u64) asid) << 48) | upper_32_bits(ttbr)); + /* CONTEXTIDR is currently unused */ + OUT_RING(ring, 0); + /* CONTEXTBANK is currently unused */ + OUT_RING(ring, 0); + + /* +* Write the new TTBR0 to the memstore. This is good for debugging. +*/ + OUT_PKT7(ring, CP_MEM_WRITE, 4); + OUT_RING(ring, lower_32_bits(rbmemptr(ring, ttbr0))); + OUT_RING(ring, upper_32_bits(rbmemptr(ring, ttbr0))); + OUT_RING(ring, lower_32_bits(ttbr)); + OUT_RING(ring, (((u64) asid) << 48) | upper_32_bits(ttbr)); +} + static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit, struct msm_file_private *ctx) { @@ -89,6 +117,8 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit, struct msm_ringbuffer *ring = submit->ring; unsigned int i; + a6xx_set_pagetable(gpu, ring, ctx); + get_stats_counter(ring, REG_A6XX_RBBM_PERFCTR_CP_0_LO, rbmemptr_stats(ring, index, cpcycles_start)); @@ -872,6 +902,18 @@ static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu) return (unsigned long)busy_time; } +struct msm_gem_address_space *a6xx_address_space_instance(struct msm_gpu *gpu) +{ + struct msm_mmu *mmu; + + mmu = msm_iommu_pagetable_create(gpu->aspace->mmu); + if (IS_ERR(mmu)) + return msm_gem_address_space_get(gpu->aspace); + + return msm_gem_address_space_create(mmu, + "gpu", 0x1ULL, 0x1ULL); +} + static const struct adreno_gpu_funcs funcs = { .base = { .get_param = adreno_get_param, @@ -895,6 +937,7 @@ static const struct adreno_gpu_funcs funcs = { .gpu_state_put = a6xx_gpu_state_put, #endif .create_address_space = adreno_iommu_create_address_space, + .address_space_instance = a6xx_address_space_instance, }, .get_timestamp = a6xx_get_timestamp, }; diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h b/drivers/gpu/drm/msm/msm_ringbuffer.h index 7764373d0ed2..0987d6bf848c 100644 --- a/drivers/gpu/drm/msm/msm_ringbuffer.h +++ b/drivers/gpu/drm/msm/msm_ringbuffer.h @@ -31,6 +31,7 @@ struct msm_rbmemptrs { volatile uint32_t fence; volatile struct msm_gpu_submit_stats stats[MSM_GPU_SUBMIT_STATS_COUNT]; + volatile u64 ttbr0; }; struct msm_ringbuffer { -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 5/6] drm/msm: Add support for address space instances
Add support for allocating an address space instance. Targets that support per-instance pagetables should implement their own function to allocate a new instance. The default will return the existing generic address space. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_drv.c | 15 +-- drivers/gpu/drm/msm/msm_drv.h | 4 drivers/gpu/drm/msm/msm_gem_vma.c | 9 + drivers/gpu/drm/msm/msm_gpu.c | 17 + drivers/gpu/drm/msm/msm_gpu.h | 5 + 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 6c57cc72d627..092c49552ddd 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -588,7 +588,7 @@ static int context_init(struct drm_device *dev, struct drm_file *file) msm_submitqueue_init(dev, ctx); - ctx->aspace = priv->gpu ? priv->gpu->aspace : NULL; + ctx->aspace = msm_gpu_address_space_instance(priv->gpu); file->driver_priv = ctx; return 0; @@ -607,6 +607,8 @@ static int msm_open(struct drm_device *dev, struct drm_file *file) static void context_close(struct msm_file_private *ctx) { msm_submitqueue_close(ctx); + + msm_gem_address_space_put(ctx->aspace); kfree(ctx); } @@ -771,18 +773,19 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device *dev, void *data, } static int msm_ioctl_gem_info_iova(struct drm_device *dev, - struct drm_gem_object *obj, uint64_t *iova) + struct drm_file *file, struct drm_gem_object *obj, + uint64_t *iova) { - struct msm_drm_private *priv = dev->dev_private; + struct msm_file_private *ctx = file->driver_priv; - if (!priv->gpu) + if (!ctx->aspace) return -EINVAL; /* * Don't pin the memory here - just get an address so that userspace can * be productive */ - return msm_gem_get_iova(obj, priv->gpu->aspace, iova); + return msm_gem_get_iova(obj, ctx->aspace, iova); } static int msm_ioctl_gem_info(struct drm_device *dev, void *data, @@ -821,7 +824,7 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, args->value = msm_gem_mmap_offset(obj); break; case MSM_INFO_GET_IOVA: - ret = msm_ioctl_gem_info_iova(dev, obj, &args->value); + ret = msm_ioctl_gem_info_iova(dev, file, obj, &args->value); break; case MSM_INFO_SET_NAME: /* length check should leave room for terminating null: */ diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index e2d6a6056418..983a8b7e5a74 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -249,6 +249,10 @@ int msm_gem_map_vma(struct msm_gem_address_space *aspace, void msm_gem_close_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma); + +struct msm_gem_address_space * +msm_gem_address_space_get(struct msm_gem_address_space *aspace); + void msm_gem_address_space_put(struct msm_gem_address_space *aspace); struct msm_gem_address_space * diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index 5f6a11211b64..29cc1305cf37 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -27,6 +27,15 @@ void msm_gem_address_space_put(struct msm_gem_address_space *aspace) kref_put(&aspace->kref, msm_gem_address_space_destroy); } +struct msm_gem_address_space * +msm_gem_address_space_get(struct msm_gem_address_space *aspace) +{ + if (!IS_ERR_OR_NULL(aspace)) + kref_get(&aspace->kref); + + return aspace; +} + /* Actually unmap memory for the vma */ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 86a138641477..0fa614430799 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -821,6 +821,23 @@ static int get_clocks(struct platform_device *pdev, struct msm_gpu *gpu) return 0; } +/* Return a new address space instance */ +struct msm_gem_address_space * +msm_gpu_address_space_instance(struct msm_gpu *gpu) +{ + if (!gpu) + return NULL; + + /* +* If the GPU doesn't support instanced address spaces return the +* default address space +*/ + if (!gpu->funcs->address_space_instance) + return msm_gem_address_space_get(gpu->aspace); + + return gpu->funcs->address_space_instance(gpu); +} + int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs, const char *name, struct msm_gpu_config *config) diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gp
[PATCH v2 0/6] iommu-arm-smmu: Add auxiliary domains and per-instance pagetables
This is a new refresh of support for auxiliary domains for arm-smmu-v2 and per-instance pagetables for drm/msm. The big change here from past efforts is that outside of creating a single aux-domain to enable TTBR0 all of the per-instance pagetables are created and managed exclusively in drm/msm without involving the arm-smmu driver. This fits in with the suggested model of letting the GPU hardware do what it needs and leave the arm-smmu driver blissfully unaware. Almost. In order to set up the io-pgtable properly in drm/msm we need to query the pagetable configuration from the current active domain and we need to rely on the iommu API to flush TLBs after a unmap. In the future we can optimize this in the drm/msm driver to track the state of the TLBs but for now the big hammer lets us get off the ground. This series is built on the split pagetable support [1]. [1] https://patchwork.kernel.org/patch/11628543/ v2: Remove unneeded cruft in the a6xx page switch sequence Jordan Crouse (6): iommu/arm-smmu: Add auxiliary domain support for arm-smmuv2 iommu/io-pgtable: Allow a pgtable implementation to skip TLB operations iommu/arm-smmu: Add a domain attribute to pass the pagetable config drm/msm: Add support to create a local pagetable drm/msm: Add support for address space instances drm/msm/a6xx: Add support for per-instance pagetables drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 43 + drivers/gpu/drm/msm/msm_drv.c | 15 +- drivers/gpu/drm/msm/msm_drv.h | 4 + drivers/gpu/drm/msm/msm_gem_vma.c | 9 + drivers/gpu/drm/msm/msm_gpu.c | 17 ++ drivers/gpu/drm/msm/msm_gpu.h | 5 + drivers/gpu/drm/msm/msm_gpummu.c | 2 +- drivers/gpu/drm/msm/msm_iommu.c | 180 +++- drivers/gpu/drm/msm/msm_mmu.h | 16 +- drivers/gpu/drm/msm/msm_ringbuffer.h | 1 + drivers/iommu/arm-smmu.c | 231 -- drivers/iommu/arm-smmu.h | 1 + include/linux/io-pgtable.h| 11 +- include/linux/iommu.h | 1 + 14 files changed, 507 insertions(+), 29 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/6] drm/msm: Add support to create a local pagetable
Add support to create a io-pgtable for use by targets that support per-instance pagetables. In order to support per-instance pagetables the GPU SMMU device needs to have the qcom,adreno-smmu compatible string and split pagetables and auxiliary domains need to be supported and enabled. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpummu.c | 2 +- drivers/gpu/drm/msm/msm_iommu.c | 180 ++- drivers/gpu/drm/msm/msm_mmu.h| 16 ++- 3 files changed, 195 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gpummu.c b/drivers/gpu/drm/msm/msm_gpummu.c index 310a31b05faa..aab121f4beb7 100644 --- a/drivers/gpu/drm/msm/msm_gpummu.c +++ b/drivers/gpu/drm/msm/msm_gpummu.c @@ -102,7 +102,7 @@ struct msm_mmu *msm_gpummu_new(struct device *dev, struct msm_gpu *gpu) } gpummu->gpu = gpu; - msm_mmu_init(&gpummu->base, dev, &funcs); + msm_mmu_init(&gpummu->base, dev, &funcs, MSM_MMU_GPUMMU); return &gpummu->base; } diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index 1b6635504069..f455c597f76d 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -4,15 +4,192 @@ * Author: Rob Clark */ +#include #include "msm_drv.h" #include "msm_mmu.h" struct msm_iommu { struct msm_mmu base; struct iommu_domain *domain; + struct iommu_domain *aux_domain; }; + #define to_msm_iommu(x) container_of(x, struct msm_iommu, base) +struct msm_iommu_pagetable { + struct msm_mmu base; + struct msm_mmu *parent; + struct io_pgtable_ops *pgtbl_ops; + phys_addr_t ttbr; + u32 asid; +}; + +static struct msm_iommu_pagetable *to_pagetable(struct msm_mmu *mmu) +{ + return container_of(mmu, struct msm_iommu_pagetable, base); +} + +static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova, + size_t size) +{ + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); + struct io_pgtable_ops *ops = pagetable->pgtbl_ops; + size_t unmapped = 0; + + /* Unmap the block one page at a time */ + while (size) { + unmapped += ops->unmap(ops, iova, 4096, NULL); + iova += 4096; + size -= 4096; + } + + iommu_flush_tlb_all(to_msm_iommu(pagetable->parent)->domain); + + return (unmapped == size) ? 0 : -EINVAL; +} + +static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova, + struct sg_table *sgt, size_t len, int prot) +{ + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); + struct io_pgtable_ops *ops = pagetable->pgtbl_ops; + struct scatterlist *sg; + size_t mapped = 0; + u64 addr = iova; + unsigned int i; + + for_each_sg(sgt->sgl, sg, sgt->nents, i) { + size_t size = sg->length; + phys_addr_t phys = sg_phys(sg); + + /* Map the block one page at a time */ + while (size) { + if (ops->map(ops, addr, phys, 4096, prot)) { + msm_iommu_pagetable_unmap(mmu, iova, mapped); + return -EINVAL; + } + + phys += 4096; + addr += 4096; + size -= 4096; + mapped += 4096; + } + } + + return 0; +} + +static void msm_iommu_pagetable_destroy(struct msm_mmu *mmu) +{ + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); + + free_io_pgtable_ops(pagetable->pgtbl_ops); + kfree(pagetable); +} + +/* + * Given a parent device, create and return an aux domain. This will enable the + * TTBR0 region + */ +static struct iommu_domain *msm_iommu_get_aux_domain(struct msm_mmu *parent) +{ + struct msm_iommu *iommu = to_msm_iommu(parent); + struct iommu_domain *domain; + int ret; + + if (iommu->aux_domain) + return iommu->aux_domain; + + if (!iommu_dev_has_feature(parent->dev, IOMMU_DEV_FEAT_AUX)) + return ERR_PTR(-ENODEV); + + domain = iommu_domain_alloc(&platform_bus_type); + if (!domain) + return ERR_PTR(-ENODEV); + + ret = iommu_aux_attach_device(domain, parent->dev); + if (ret) { + iommu_domain_free(domain); + return ERR_PTR(ret); + } + + iommu->aux_domain = domain; + return domain; +} + +int msm_iommu_pagetable_params(struct msm_mmu *mmu, + phys_addr_t *ttbr, int *asid) +{ + struct msm_iommu_pagetable *pagetable; + + if (mmu->type != MSM_MMU_IOMMU_PAGETABLE) + return -EINVAL; + + pagetable = to_pagetable(mmu); + + if (ttbr) + *ttbr = pagetable->ttbr; + + if (asid) + *asid = pagetable->asid; + + return 0; +} + +static const struct msm_mmu_funcs pagetable_funcs = { +
[PATCH v9 6/7] drm/msm: Set the global virtual address range from the IOMMU domain
Use the aperture settings from the IOMMU domain to set up the virtual address range for the GPU. This allows us to transparently deal with IOMMU side features (like split pagetables). Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 13 +++-- drivers/gpu/drm/msm/msm_iommu.c | 7 +++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 5db06b590943..3e717c1ebb7f 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -192,9 +192,18 @@ adreno_iommu_create_address_space(struct msm_gpu *gpu, struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type); struct msm_mmu *mmu = msm_iommu_new(&pdev->dev, iommu); struct msm_gem_address_space *aspace; + u64 start, size; - aspace = msm_gem_address_space_create(mmu, "gpu", SZ_16M, - 0x - SZ_16M); + /* +* Use the aperture start or SZ_16M, whichever is greater. This will +* ensure that we align with the allocated pagetable range while still +* allowing room in the lower 32 bits for GMEM and whatnot +*/ + start = max_t(u64, SZ_16M, iommu->geometry.aperture_start); + size = iommu->geometry.aperture_end - start + 1; + + aspace = msm_gem_address_space_create(mmu, "gpu", + start & GENMASK(48, 0), size); if (IS_ERR(aspace) && !IS_ERR(mmu)) mmu->funcs->destroy(mmu); diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index 3a381a9674c9..1b6635504069 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -36,6 +36,10 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova, struct msm_iommu *iommu = to_msm_iommu(mmu); size_t ret; + /* The arm-smmu driver expects the addresses to be sign extended */ + if (iova & BIT_ULL(48)) + iova |= GENMASK_ULL(63, 49); + ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot); WARN_ON(!ret); @@ -46,6 +50,9 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t iova, size_t len) { struct msm_iommu *iommu = to_msm_iommu(mmu); + if (iova & BIT_ULL(48)) + iova |= GENMASK_ULL(63, 49); + iommu_unmap(iommu->domain, iova, len); return 0; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 0/7] iommu/arm-smmu: Enable split pagetable support
Another iteration of the split-pagetable support for arm-smmu and the Adreno GPU SMMU. After email discussions [1] we opted to make a arm-smmu implementation for specifically for the Adreno GPU and use that to enable split pagetable support and later other implementation specific bits that we need. On the hardware side this is very close to the same code from before [2] only the TTBR1 quirk is turned on by the implementation and not a domain attribute. In drm/msm we use the returned size of the aperture as a clue to let us know which virtual address space we should use for global memory objects. There are two open items that you should be aware of. First, in the implementation specific code we have to check the compatible string of the device so that we only enable TTBR1 for the GPU (SID 0) and not the GMU (SID 4). I went back and forth trying to decide if I wanted to use the compatible string or the SID as the filter and settled on the compatible string but I could be talked out of it. The other open item is that in drm/msm the hardware only uses 49 bits of the address space but arm-smmu expects the address to be sign extended all the way to 64 bits. This isn't a problem normally unless you look at the hardware registers that contain a IOVA and then the upper bits will be zero. I opted to restrict the internal drm/msm IOVA range to only 49 bits and then sign extend right before calling iommu_map / iommu_unmap. This is a bit wonky but I thought that matching the hardware would be less confusing when debugging a hang. v9: Fix bot-detected merge conflict v7: Add attached device to smmu_domain to pass to implementation specific functions [1] https://lists.linuxfoundation.org/pipermail/iommu/2020-May/044537.html [2] https://patchwork.kernel.org/patch/11482591/ Jordan Crouse (7): iommu/arm-smmu: Pass io-pgtable config to implementation specific function iommu/arm-smmu: Add support for split pagetables dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU iommu/arm-smmu: Add a pointer to the attached device to smmu_domain iommu/arm-smmu: Add implementation for the adreno GPU SMMU drm/msm: Set the global virtual address range from the IOMMU domain arm: dts: qcom: sm845: Set the compatible string for the GPU SMMU .../devicetree/bindings/iommu/arm,smmu.yaml | 4 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 13 +- drivers/gpu/drm/msm/msm_iommu.c | 7 +++ drivers/iommu/arm-smmu-impl.c | 6 ++- drivers/iommu/arm-smmu-qcom.c | 45 ++- drivers/iommu/arm-smmu.c | 38 +++- drivers/iommu/arm-smmu.h | 30 ++--- 8 files changed, 120 insertions(+), 25 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm fixes for v5.8-rc3
The pull request you sent on Fri, 26 Jun 2020 14:26:34 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2020-06-26 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/6a6c9b220a7fb7c8285ad1739ac3c909584feb43 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] gpu: ipu-v3: image-convert: Wait for all EOFs before completing a tile
Hi Philipp, On 6/26/20 2:38 AM, Philipp Zabel wrote: Hi Steve, On Thu, 2020-06-25 at 11:13 -0700, Steve Longerbeam wrote: Use a bit-mask of EOF irqs to determine when all required idmac channel EOFs have been received for a tile conversion, and only do tile completion processing after all EOFs have been received. Otherwise it was found that a conversion would stall after the completion of a tile and the start of the next tile, because the input/read idmac channel had not completed and entered idle state, thus locking up the channel when attempting to re-start it for the next tile. Do I understand correctly that there are cases where the output channel EOF IRQ has triggered and the next tile processing is kicked off before the input channel EOF IRQ triggers even without rotation? Yes. What is the cause of this? It would seem that the read channel EOF should occur before the write channel EOF, but there are cases seen where the opposite occurs. Maybe this has to do with idmac channel priorities, the IC PP read/write channels are set to the same priority (low), in which case the IPU should resort to round-robin when handling requests on those channels. Maybe the EOF irq is not signalled until after the IPU has updated CPMEM with status info after the transfers complete, and round-robin selects the write channel before the read channel for the CPMEM updates? Do you have any way to reproduce this? Yes, try a scaling only conversion, 1920x1080.422p -> 1024x768.422p. No rotation needed. Steve regards Philipp Fixes: 0537db801bb01 ("gpu: ipu-v3: image-convert: reconfigure IC per tile") Signed-off-by: Steve Longerbeam --- Changes in v2: - need to clear eof_mask at completion of every tile, not just in convert_start(). --- drivers/gpu/ipu-v3/ipu-image-convert.c | 109 +++-- 1 file changed, 82 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/ipu-v3/ipu-image-convert.c b/drivers/gpu/ipu-v3/ipu-image-convert.c index f8b031ded3cf..aa1d4b6d278f 100644 --- a/drivers/gpu/ipu-v3/ipu-image-convert.c +++ b/drivers/gpu/ipu-v3/ipu-image-convert.c @@ -137,6 +137,17 @@ struct ipu_image_convert_ctx; struct ipu_image_convert_chan; struct ipu_image_convert_priv; +enum eof_irq_mask { + EOF_IRQ_IN = BIT(0), + EOF_IRQ_ROT_IN = BIT(1), + EOF_IRQ_OUT = BIT(2), + EOF_IRQ_ROT_OUT = BIT(3), +}; + +#define EOF_IRQ_COMPLETE (EOF_IRQ_IN | EOF_IRQ_OUT) +#define EOF_IRQ_ROT_COMPLETE (EOF_IRQ_IN | EOF_IRQ_OUT | \ + EOF_IRQ_ROT_IN | EOF_IRQ_ROT_OUT) + struct ipu_image_convert_ctx { struct ipu_image_convert_chan *chan; @@ -173,6 +184,9 @@ struct ipu_image_convert_ctx { /* where to place converted tile in dest image */ unsigned int out_tile_map[MAX_TILES]; + /* mask of completed EOF irqs at every tile conversion */ + enum eof_irq_mask eof_mask; + struct list_head list; }; @@ -189,6 +203,8 @@ struct ipu_image_convert_chan { struct ipuv3_channel *rotation_out_chan; /* the IPU end-of-frame irqs */ + int in_eof_irq; + int rot_in_eof_irq; int out_eof_irq; int rot_out_eof_irq; @@ -1380,6 +1396,9 @@ static int convert_start(struct ipu_image_convert_run *run, unsigned int tile) dev_dbg(priv->ipu->dev, "%s: task %u: starting ctx %p run %p tile %u -> %u\n", __func__, chan->ic_task, ctx, run, tile, dst_tile); + /* clear EOF irq mask */ + ctx->eof_mask = 0; + if (ipu_rot_mode_is_irt(ctx->rot_mode)) { /* swap width/height for resizer */ dest_width = d_image->tile[dst_tile].height; @@ -1615,7 +1634,7 @@ static bool ic_settings_changed(struct ipu_image_convert_ctx *ctx) } /* hold irqlock when calling */ -static irqreturn_t do_irq(struct ipu_image_convert_run *run) +static irqreturn_t do_tile_complete(struct ipu_image_convert_run *run) { struct ipu_image_convert_ctx *ctx = run->ctx; struct ipu_image_convert_chan *chan = ctx->chan; @@ -1700,6 +1719,7 @@ static irqreturn_t do_irq(struct ipu_image_convert_run *run) ctx->cur_buf_num ^= 1; } + ctx->eof_mask = 0; /* clear EOF irq mask for next tile */ ctx->next_tile++; return IRQ_HANDLED; done: @@ -1715,8 +1735,9 @@ static irqreturn_t eof_irq(int irq, void *data) struct ipu_image_convert_priv *priv = chan->priv; struct ipu_image_convert_ctx *ctx; struct ipu_image_convert_run *run; + irqreturn_t ret = IRQ_HANDLED; + bool tile_complete = false; unsigned long flags; - irqreturn_t ret; spin_lock_irqsave(&chan->irqlock, flags); @@ -1729,27 +1750,33 @@ static irqreturn_t eof_irq(int irq, void *data) ctx = run->ctx; - if (irq == chan->out_eof_irq) { - if (ipu_rot_mode_is_irt(ctx->rot_mode)) { - /* this is a rotation op, just ignore */ -
Re: [PATCH 1/2] drm/ttm: cleanup ttm_mem_type_manager_func.get_node interface
Am 26.06.20 um 19:39 schrieb Ruhl, Michael J: -Original Message- From: dri-devel On Behalf Of Christian König Sent: Wednesday, June 24, 2020 9:36 AM To: dri-devel@lists.freedesktop.org Subject: [PATCH 1/2] drm/ttm: cleanup ttm_mem_type_manager_func.get_node interface Instead of signaling failure by setting the node pointer to NULL do so by returning -ENOSPC. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 +--- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 ++--- drivers/gpu/drm/nouveau/nouveau_ttm.c | 8 drivers/gpu/drm/ttm/ttm_bo.c | 11 +-- drivers/gpu/drm/ttm/ttm_bo_manager.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 4 +--- 6 files changed, 10 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index 627104401e84..2baa80224fa4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -229,7 +229,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man, if ((&tbo->mem == mem || tbo->mem.mem_type != TTM_PL_TT) && atomic64_read(&mgr->available) < mem->num_pages) { spin_unlock(&mgr->lock); - return 0; + return -ENOSPC; } atomic64_sub(mem->num_pages, &mgr->available); spin_unlock(&mgr->lock); @@ -249,8 +249,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man, r = amdgpu_gtt_mgr_alloc(man, tbo, place, mem); if (unlikely(r)) { kfree(node); - mem->mm_node = NULL; Hmm, amdgpu_gtt_mgr_del() grabs mem->mm_node and kfrees it. If this value is not NUL, it looks like bad things could happen. Will _mgr_del never get called in this case? Yes, everything else would be a bug. Using the return value seems pretty reasonable, leaving bad pointers lying around makes me slightly nervous. The caller should not touch the member when an error occurred since it is certainly not initialized. But it might be a good idea to zero initialize the structure by the caller just to be sure. Thanks for the comment, Christian. Mike - r = 0; goto err_out; } } else { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c index 128a667ed8fa..e8d1dd564006 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c @@ -336,8 +336,7 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager *man, mem_bytes = (u64)mem->num_pages << PAGE_SHIFT; if (atomic64_add_return(mem_bytes, &mgr->usage) > max_bytes) { atomic64_sub(mem_bytes, &mgr->usage); - mem->mm_node = NULL; - return 0; + return -ENOSPC; } if (place->flags & TTM_PL_FLAG_CONTIGUOUS) { @@ -417,7 +416,7 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager *man, atomic64_sub(mem->num_pages << PAGE_SHIFT, &mgr->usage); kvfree(nodes); - return r == -ENOSPC ? 0 : r; + return r; } /** diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c index 7ca0a2498532..e89ea052cf71 100644 --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c @@ -75,10 +75,6 @@ nouveau_vram_manager_new(struct ttm_mem_type_manager *man, ret = nouveau_mem_vram(reg, nvbo->contig, nvbo->page); if (ret) { nouveau_mem_del(reg); - if (ret == -ENOSPC) { - reg->mm_node = NULL; - return 0; - } return ret; } @@ -139,10 +135,6 @@ nv04_gart_manager_new(struct ttm_mem_type_manager *man, reg->num_pages << PAGE_SHIFT, &mem->vma[0]); if (ret) { nouveau_mem_del(reg); - if (ret == -ENOSPC) { - reg->mm_node = NULL; - return 0; - } return ret; } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index f73b81c2576e..15f9b19fa00d 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -916,10 +916,10 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, ticket = dma_resv_locking_ctx(bo->base.resv); do { ret = (*man->func->get_node)(man, bo, place, mem); - if (unlikely(ret != 0)) - return ret; - if (mem->mm_node) + if (likely(!ret)) break; + if (unlikely(ret != -ENOSPC)) + return ret; ret = ttm_mem_evict_first(bdev, mem->mem_typ
RE: [PATCH 2/2] drm/ttm: make TT creation purely optional
>-Original Message- >From: dri-devel On Behalf Of >Christian König >Sent: Wednesday, June 24, 2020 9:36 AM >To: dri-devel@lists.freedesktop.org >Subject: [PATCH 2/2] drm/ttm: make TT creation purely optional > >We only need the page array when the BO is about to be accessed. > >So not only populate, but also create it on demand. > >Signed-off-by: Christian König >--- > drivers/gpu/drm/ttm/ttm_bo.c | 26 -- > drivers/gpu/drm/ttm/ttm_bo_util.c | 9 +++-- > drivers/gpu/drm/ttm/ttm_bo_vm.c | 5 + > 3 files changed, 16 insertions(+), 24 deletions(-) > >diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >index 15f9b19fa00d..0e0a9dadf3ed 100644 >--- a/drivers/gpu/drm/ttm/ttm_bo.c >+++ b/drivers/gpu/drm/ttm/ttm_bo.c >@@ -667,13 +667,8 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, > placement.num_busy_placement = 0; > bdev->driver->evict_flags(bo, &placement); > >- if (!placement.num_placement && >!placement.num_busy_placement) { >- ret = ttm_bo_pipeline_gutting(bo); >- if (ret) >- return ret; >- >- return ttm_tt_create(bo, false); >- } >+ if (!placement.num_placement && >!placement.num_busy_placement) >+ return ttm_bo_pipeline_gutting(bo); > > evict_mem = bo->mem; > evict_mem.mm_node = NULL; >@@ -1200,13 +1195,8 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, > /* >* Remove the backing store if no placement is given. >*/ >- if (!placement->num_placement && !placement- >>num_busy_placement) { >- ret = ttm_bo_pipeline_gutting(bo); >- if (ret) >- return ret; >- >- return ttm_tt_create(bo, false); >- } >+ if (!placement->num_placement && !placement- >>num_busy_placement) >+ return ttm_bo_pipeline_gutting(bo); > > /* >* Check whether we need to move buffer. >@@ -1223,14 +1213,6 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, > ttm_flag_masked(&bo->mem.placement, new_flags, > ~TTM_PL_MASK_MEMTYPE); > } >- /* >- * We might need to add a TTM. >- */ >- if (bo->mem.mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { >- ret = ttm_tt_create(bo, true); >- if (ret) >- return ret; >- } > return 0; > } > EXPORT_SYMBOL(ttm_bo_validate); >diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c >b/drivers/gpu/drm/ttm/ttm_bo_util.c >index 52d2b71f1588..f8414f820350 100644 >--- a/drivers/gpu/drm/ttm/ttm_bo_util.c >+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c >@@ -580,12 +580,17 @@ static int ttm_bo_kmap_ttm(struct >ttm_buffer_object *bo, > .interruptible = false, > .no_wait_gpu = false > }; >- struct ttm_tt *ttm = bo->ttm; >+ struct ttm_tt *ttm; > pgprot_t prot; > int ret; > >- BUG_ON(!ttm); >+ if (!bo->ttm) { >+ ret = ttm_tt_create(bo, true); Would it be reasonable to move the NULL check into ttm_tt_create()? Kind of an opposite path NULL check, but it makes the path a little more clean. Mike >+ if (ret) >+ return ret; >+ } > >+ ttm = bo->ttm; > ret = ttm_tt_populate(ttm, &ctx); > if (ret) > return ret; >diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c >b/drivers/gpu/drm/ttm/ttm_bo_vm.c >index 0ad30b112982..bdfed6725d6f 100644 >--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c >+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c >@@ -349,6 +349,11 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct >vm_fault *vmf, > > }; > >+ if (!bo->ttm && ttm_tt_create(bo, true)) { >+ ret = VM_FAULT_OOM; >+ goto out_io_unlock; >+ } >+ > ttm = bo->ttm; > if (ttm_tt_populate(bo->ttm, &ctx)) { > ret = VM_FAULT_OOM; >-- >2.17.1 > >___ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 1/2] drm/ttm: cleanup ttm_mem_type_manager_func.get_node interface
>-Original Message- >From: dri-devel On Behalf Of >Christian König >Sent: Wednesday, June 24, 2020 9:36 AM >To: dri-devel@lists.freedesktop.org >Subject: [PATCH 1/2] drm/ttm: cleanup >ttm_mem_type_manager_func.get_node interface > >Instead of signaling failure by setting the node pointer to >NULL do so by returning -ENOSPC. > >Signed-off-by: Christian König >--- > drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 +--- > drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 ++--- > drivers/gpu/drm/nouveau/nouveau_ttm.c | 8 > drivers/gpu/drm/ttm/ttm_bo.c | 11 +-- > drivers/gpu/drm/ttm/ttm_bo_manager.c | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 4 +--- > 6 files changed, 10 insertions(+), 24 deletions(-) > >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >index 627104401e84..2baa80224fa4 100644 >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >@@ -229,7 +229,7 @@ static int amdgpu_gtt_mgr_new(struct >ttm_mem_type_manager *man, > if ((&tbo->mem == mem || tbo->mem.mem_type != TTM_PL_TT) && > atomic64_read(&mgr->available) < mem->num_pages) { > spin_unlock(&mgr->lock); >- return 0; >+ return -ENOSPC; > } > atomic64_sub(mem->num_pages, &mgr->available); > spin_unlock(&mgr->lock); >@@ -249,8 +249,6 @@ static int amdgpu_gtt_mgr_new(struct >ttm_mem_type_manager *man, > r = amdgpu_gtt_mgr_alloc(man, tbo, place, mem); > if (unlikely(r)) { > kfree(node); >- mem->mm_node = NULL; Hmm, amdgpu_gtt_mgr_del() grabs mem->mm_node and kfrees it. If this value is not NUL, it looks like bad things could happen. Will _mgr_del never get called in this case? Using the return value seems pretty reasonable, leaving bad pointers lying around makes me slightly nervous. Mike >- r = 0; > goto err_out; > } > } else { >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c >b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c >index 128a667ed8fa..e8d1dd564006 100644 >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c >@@ -336,8 +336,7 @@ static int amdgpu_vram_mgr_new(struct >ttm_mem_type_manager *man, > mem_bytes = (u64)mem->num_pages << PAGE_SHIFT; > if (atomic64_add_return(mem_bytes, &mgr->usage) > max_bytes) { > atomic64_sub(mem_bytes, &mgr->usage); >- mem->mm_node = NULL; >- return 0; >+ return -ENOSPC; > } > > if (place->flags & TTM_PL_FLAG_CONTIGUOUS) { >@@ -417,7 +416,7 @@ static int amdgpu_vram_mgr_new(struct >ttm_mem_type_manager *man, > atomic64_sub(mem->num_pages << PAGE_SHIFT, &mgr->usage); > > kvfree(nodes); >- return r == -ENOSPC ? 0 : r; >+ return r; > } > > /** >diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c >b/drivers/gpu/drm/nouveau/nouveau_ttm.c >index 7ca0a2498532..e89ea052cf71 100644 >--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c >+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c >@@ -75,10 +75,6 @@ nouveau_vram_manager_new(struct >ttm_mem_type_manager *man, > ret = nouveau_mem_vram(reg, nvbo->contig, nvbo->page); > if (ret) { > nouveau_mem_del(reg); >- if (ret == -ENOSPC) { >- reg->mm_node = NULL; >- return 0; >- } > return ret; > } > >@@ -139,10 +135,6 @@ nv04_gart_manager_new(struct >ttm_mem_type_manager *man, > reg->num_pages << PAGE_SHIFT, &mem->vma[0]); > if (ret) { > nouveau_mem_del(reg); >- if (ret == -ENOSPC) { >- reg->mm_node = NULL; >- return 0; >- } > return ret; > } > >diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >index f73b81c2576e..15f9b19fa00d 100644 >--- a/drivers/gpu/drm/ttm/ttm_bo.c >+++ b/drivers/gpu/drm/ttm/ttm_bo.c >@@ -916,10 +916,10 @@ static int ttm_bo_mem_force_space(struct >ttm_buffer_object *bo, > ticket = dma_resv_locking_ctx(bo->base.resv); > do { > ret = (*man->func->get_node)(man, bo, place, mem); >- if (unlikely(ret != 0)) >- return ret; >- if (mem->mm_node) >+ if (likely(!ret)) > break; >+ if (unlikely(ret != -ENOSPC)) >+ return ret; > ret = ttm_mem_evict_first(bdev, mem->mem_type, place, >ctx, > ticket); > if (unlikely(ret != 0)) >@@ -1063,12 +1063,11 @@ int ttm_bo_mem_space(struct ttm_buffer_object >*bo, > > man = &bdev->man[mem->mem_type]; > ret =
Re: [PATCH v3 0/7] Convert the remaining text files to ReST
On Tue, 23 Jun 2020 15:31:33 +0200 Mauro Carvalho Chehab wrote: > The main goal of this series is to finish the ReST conversion. After this > series, we have just those files still in plain old format: > > - Documentation/RCU/RTFP.txt > - Documentation/atomic_bitops.txt > - Documentation/memory-barriers.txt > - Documentation/atomic_t.txt > - Documentation/filesystems/dax.txt > - Documentation/filesystems/path-lookup.txt > - Documentation/virt/kvm/devices/README OK, I've applied this set - glad to see the last one! Still *not* glad to see the LaTeX markup in the staging stuff; hopefully we can do something about that soon. Thanks, jon ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: drm_fourcc: Add generic alias for 16_16_TILE modifier
In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier describes a generic pixel re-ordering which can be applicable to multiple vendors. Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be used to describe this layout in a vendor-neutral way, and add a comment about the expected usage of such "generic" modifiers. Changes in v2: - Move note about future cases to comment (Daniel) Signed-off-by: Brian Starkey --- include/uapi/drm/drm_fourcc.h | 25 + 1 file changed, 25 insertions(+) diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 299f9ac4840b..cb3db4a21012 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -345,8 +345,33 @@ extern "C" { * When adding a new token please document the layout with a code comment, * similar to the fourcc codes above. drm_fourcc.h is considered the * authoritative source for all of these. + * + * Generic modifier names: + * + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral names + * for layouts which are common across multiple vendors. To preserve + * compatibility, in cases where a vendor-specific definition already exists and + * a generic name for it is desired, the common name is a purely symbolic alias + * and must use the same numerical value as the original definition. + * + * Note that generic names should only be used for modifiers which describe + * generic layouts (such as pixel re-ordering), which may have + * independently-developed support across multiple vendors. + * + * In future cases where a generic layout is identified before merging with a + * vendor-specific modifier, a new 'GENERIC' vendor or modifier using vendor + * 'NONE' could be considered. This should only be for obvious, exceptional + * cases to avoid polluting the 'GENERIC' namespace with modifiers which only + * apply to a single vendor. + * + * Generic names should not be used for cases where multiple hardware vendors + * have implementations of the same standardised compression scheme (such as + * AFBC). In those cases, all implementations should use the same format + * modifier(s), reflecting the vendor of the standard. */ +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE + /* * Invalid Modifier * -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: drm_fourcc: Add generic alias for 16_16_TILE modifier
On Fri, Jun 26, 2020 at 06:15:36PM +0200, Daniel Vetter wrote: > On Fri, Jun 26, 2020 at 5:21 PM Brian Starkey wrote: > > > > On Fri, Jun 26, 2020 at 05:16:53PM +0200, Daniel Vetter wrote: > > > On Fri, Jun 26, 2020 at 4:16 PM Brian Starkey > > > wrote: > > > > > > > > Hi Daniel, > > > > > > > > On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > > > > > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey > > > > > wrote: > > > > > > > > > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > > > > > describes a generic pixel re-ordering which can be applicable to > > > > > > multiple vendors. > > > > > > > > > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > > > > > used to describe this layout in a vendor-neutral way, and add a > > > > > > comment about the expected usage of such "generic" modifiers. > > > > > > > > > > > > Signed-off-by: Brian Starkey > > > > > > --- > > > > > > > > > > > > This is based off a suggestion from Daniel here: > > > > > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > > > > > > > > > If there are future cases where a "generic" modifier is identified > > > > > > before merging with a specific vendor code, perhaps we could use > > > > > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > > > > > > > > > However, I also think we shouldn't try and "guess" what might be > > > > > > generic > > > > > > up-front and end up polluting the generic namespace. It seems fine > > > > > > to > > > > > > just alias vendor-specific definitions unless it's very clear-cut. > > > > > > > > > > I think the above two things would also be good additions to the > > > > > comment. > > > > > > > > > > A totally different thing: I just noticed that we don't pull in all > > > > > the comments for modifiers. I think we could convert them to > > > > > kerneldoc, and then pull them into a separate section in drm-kms.rst. > > > > > Maybe even worth to pull out into a new drm-fourcc.rst document, since > > > > > these are officially shared with gl and vk standard extensions. Then > > > > > this new modifier's doc could simply point at teh existing one (and > > > > > the generated kerneldoc would make that a hyperlink for added > > > > > awesomeness). > > > > > > > > > > Aside from that makes sense to me, maybe just add it to your patch > > > > > series that will start making use of these. > > > > > > > > This is only supported by the GPU, so we wouldn't be using this on the > > > > Display side. > > > > > > I mean the patch to add the NV15 fource, it mentions "suitable for 16 > > > by 16 tile layout", would be nice to just put the generic modifier in > > > that comment. > > > -Daniel > > > > Ah right. I saw that one went out in Maarten's pull request this > > morning. > > Oh I missed that, then maybe just amend this patch to update the other > comment? It was only mentioned in the commit message there, so I guess I can't do anything about it now. I'll respin to put the extra notes into the comment. Thanks, -Brian > -Daniel > > > > > -Brian > > > > > > > > > > > > > Thanks, > > > > -Brian > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > > > > > welcome. > > > > > > > > > > > > Cheers, > > > > > > -Brian > > > > > > > > > > > > include/uapi/drm/drm_fourcc.h | 19 +++ > > > > > > 1 file changed, 19 insertions(+) > > > > > > > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h > > > > > > b/include/uapi/drm/drm_fourcc.h > > > > > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > > > > > --- a/include/uapi/drm/drm_fourcc.h > > > > > > +++ b/include/uapi/drm/drm_fourcc.h > > > > > > @@ -345,8 +345,27 @@ extern "C" { > > > > > > * When adding a new token please document the layout with a code > > > > > > comment, > > > > > > * similar to the fourcc codes above. drm_fourcc.h is considered > > > > > > the > > > > > > * authoritative source for all of these. > > > > > > + * > > > > > > + * Generic modifier names: > > > > > > + * > > > > > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide > > > > > > vendor-neutral names > > > > > > + * for layouts which are common across multiple vendors. To > > > > > > preserve > > > > > > + * compatibility, in cases where a vendor-specific definition > > > > > > already exists and > > > > > > + * a generic name for it is desired, the common name is a purely > > > > > > symbolic alias > > > > > > + * and must use the same numerical value as the original > > > > > > definition. > > > > > > + * > > > > > > + * Note that generic names should only be used for modifiers which > > > > > > describe > > > > > > + * generic layouts (such as pixel re-ordering), which may have > > > > > > + * independently-developed support across multiple vendors. > > > > > > + * > > > > > > + * Generic names should not be used for cases where multiple >
Re: [PATCH v6 1/4] driver core: add device probe log helper
Hi Andrzej On Fri, Jun 26, 2020 at 12:01:00PM +0200, Andrzej Hajda wrote: > During probe every time driver gets resource it should usually check for > error printk some message if it is not -EPROBE_DEFER and return the error. > This pattern is simple but requires adding few lines after any resource > acquisition code, as a result it is often omitted or implemented only > partially. > dev_err_probe helps to replace such code sequences with simple call, > so code: > if (err != -EPROBE_DEFER) > dev_err(dev, ...); > return err; > becomes: > return probe_err(dev, err, ...); s/probe_err/dev_err_probe/ Sam > > Signed-off-by: Andrzej Hajda > --- > drivers/base/core.c| 42 ++ > include/linux/device.h | 3 +++ > 2 files changed, 45 insertions(+) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 67d39a90b45c..3a827c82933f 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -3953,6 +3953,48 @@ define_dev_printk_level(_dev_info, KERN_INFO); > > #endif > > +/** > + * dev_err_probe - probe error check and log helper > + * @dev: the pointer to the struct device > + * @err: error value to test > + * @fmt: printf-style format string > + * @...: arguments as specified in the format string > + * > + * This helper implements common pattern present in probe functions for error > + * checking: print debug or error message depending if the error value is > + * -EPROBE_DEFER and propagate error upwards. > + * It replaces code sequence: > + * if (err != -EPROBE_DEFER) > + * dev_err(dev, ...); > + * else > + * dev_dbg(dev, ...); > + * return err; > + * with > + * return dev_err_probe(dev, err, ...); > + * > + * Returns @err. > + * > + */ > +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...) > +{ > + struct va_format vaf; > + va_list args; > + > + va_start(args, fmt); > + vaf.fmt = fmt; > + vaf.va = &args; > + > + if (err != -EPROBE_DEFER) > + dev_err(dev, "error %d: %pV", err, &vaf); > + else > + dev_dbg(dev, "error %d: %pV", err, &vaf); > + > + va_end(args); > + > + return err; > +} > +EXPORT_SYMBOL_GPL(dev_err_probe); > + > static inline bool fwnode_is_primary(struct fwnode_handle *fwnode) > { > return fwnode && !IS_ERR(fwnode->secondary); > diff --git a/include/linux/device.h b/include/linux/device.h > index 15460a5ac024..6b2272ae9af8 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -964,6 +964,9 @@ void device_link_remove(void *consumer, struct device > *supplier); > void device_links_supplier_sync_state_pause(void); > void device_links_supplier_sync_state_resume(void); > > +extern __printf(3, 4) > +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...); > + > /* Create alias, so I can be autoloaded. */ > #define MODULE_ALIAS_CHARDEV(major,minor) \ > MODULE_ALIAS("char-major-" __stringify(major) "-" __stringify(minor)) > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: drm_fourcc: Add generic alias for 16_16_TILE modifier
On Fri, Jun 26, 2020 at 5:21 PM Brian Starkey wrote: > > On Fri, Jun 26, 2020 at 05:16:53PM +0200, Daniel Vetter wrote: > > On Fri, Jun 26, 2020 at 4:16 PM Brian Starkey wrote: > > > > > > Hi Daniel, > > > > > > On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > > > > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey > > > > wrote: > > > > > > > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > > > > describes a generic pixel re-ordering which can be applicable to > > > > > multiple vendors. > > > > > > > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > > > > used to describe this layout in a vendor-neutral way, and add a > > > > > comment about the expected usage of such "generic" modifiers. > > > > > > > > > > Signed-off-by: Brian Starkey > > > > > --- > > > > > > > > > > This is based off a suggestion from Daniel here: > > > > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > > > > > > > If there are future cases where a "generic" modifier is identified > > > > > before merging with a specific vendor code, perhaps we could use > > > > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > > > > > > > However, I also think we shouldn't try and "guess" what might be > > > > > generic > > > > > up-front and end up polluting the generic namespace. It seems fine to > > > > > just alias vendor-specific definitions unless it's very clear-cut. > > > > > > > > I think the above two things would also be good additions to the > > > > comment. > > > > > > > > A totally different thing: I just noticed that we don't pull in all > > > > the comments for modifiers. I think we could convert them to > > > > kerneldoc, and then pull them into a separate section in drm-kms.rst. > > > > Maybe even worth to pull out into a new drm-fourcc.rst document, since > > > > these are officially shared with gl and vk standard extensions. Then > > > > this new modifier's doc could simply point at teh existing one (and > > > > the generated kerneldoc would make that a hyperlink for added > > > > awesomeness). > > > > > > > > Aside from that makes sense to me, maybe just add it to your patch > > > > series that will start making use of these. > > > > > > This is only supported by the GPU, so we wouldn't be using this on the > > > Display side. > > > > I mean the patch to add the NV15 fource, it mentions "suitable for 16 > > by 16 tile layout", would be nice to just put the generic modifier in > > that comment. > > -Daniel > > Ah right. I saw that one went out in Maarten's pull request this > morning. Oh I missed that, then maybe just amend this patch to update the other comment? -Daniel > > -Brian > > > > > > > > > Thanks, > > > -Brian > > > > > > > -Daniel > > > > > > > > > > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > > > > welcome. > > > > > > > > > > Cheers, > > > > > -Brian > > > > > > > > > > include/uapi/drm/drm_fourcc.h | 19 +++ > > > > > 1 file changed, 19 insertions(+) > > > > > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h > > > > > b/include/uapi/drm/drm_fourcc.h > > > > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > > > > --- a/include/uapi/drm/drm_fourcc.h > > > > > +++ b/include/uapi/drm/drm_fourcc.h > > > > > @@ -345,8 +345,27 @@ extern "C" { > > > > > * When adding a new token please document the layout with a code > > > > > comment, > > > > > * similar to the fourcc codes above. drm_fourcc.h is considered the > > > > > * authoritative source for all of these. > > > > > + * > > > > > + * Generic modifier names: > > > > > + * > > > > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide > > > > > vendor-neutral names > > > > > + * for layouts which are common across multiple vendors. To preserve > > > > > + * compatibility, in cases where a vendor-specific definition > > > > > already exists and > > > > > + * a generic name for it is desired, the common name is a purely > > > > > symbolic alias > > > > > + * and must use the same numerical value as the original definition. > > > > > + * > > > > > + * Note that generic names should only be used for modifiers which > > > > > describe > > > > > + * generic layouts (such as pixel re-ordering), which may have > > > > > + * independently-developed support across multiple vendors. > > > > > + * > > > > > + * Generic names should not be used for cases where multiple > > > > > hardware vendors > > > > > + * have implementations of the same standardised compression scheme > > > > > (such as > > > > > + * AFBC). In those cases, all implementations should use the same > > > > > format > > > > > + * modifier(s), reflecting the vendor of the standard. > > > > > */ > > > > > > > > > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE > > > > > DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > > > > > + > > > > > /* > > > > > * Invalid Modifier > > > > > * > > > > > -- > > > > > 2.17.1 > > >
Re: [PATCH v6 2/4] driver core: add deferring probe reason to devices_deferred property
On Fri, Jun 26, 2020 at 12:01 PM Andrzej Hajda wrote: > > /sys/kernel/debug/devices_deferred property contains list of deferred devices. > This list does not contain reason why the driver deferred probe, the patch > improves it. > The natural place to set the reason is probe_err function introduced recently, > ie. if probe_err will be called with -EPROBE_DEFER instead of printk the > message > will be attached to deferred device and printed when user read > devices_deferred > property. > > Signed-off-by: Andrzej Hajda > Reviewed-by: Mark Brown > Reviewed-by: Javier Martinez Canillas > Reviewed-by: Andy Shevchenko Reviewed-by: Rafael J. Wysocki > --- > drivers/base/base.h | 3 +++ > drivers/base/core.c | 8 ++-- > drivers/base/dd.c | 23 ++- > 3 files changed, 31 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/base.h b/drivers/base/base.h > index 95c22c0f9036..6954fccab3d7 100644 > --- a/drivers/base/base.h > +++ b/drivers/base/base.h > @@ -93,6 +93,7 @@ struct device_private { > struct klist_node knode_class; > struct list_head deferred_probe; > struct device_driver *async_driver; > + char *deferred_probe_reason; > struct device *device; > u8 dead:1; > }; > @@ -134,6 +135,8 @@ extern void device_release_driver_internal(struct device > *dev, > extern void driver_detach(struct device_driver *drv); > extern int driver_probe_device(struct device_driver *drv, struct device > *dev); > extern void driver_deferred_probe_del(struct device *dev); > +extern void device_set_deferred_probe_reson(const struct device *dev, > + struct va_format *vaf); > static inline int driver_match_device(struct device_driver *drv, > struct device *dev) > { > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 3a827c82933f..fee047f03681 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -3963,6 +3963,8 @@ define_dev_printk_level(_dev_info, KERN_INFO); > * This helper implements common pattern present in probe functions for error > * checking: print debug or error message depending if the error value is > * -EPROBE_DEFER and propagate error upwards. > + * In case of -EPROBE_DEFER it sets also defer probe reason, which can be > + * checked later by reading devices_deferred debugfs attribute. > * It replaces code sequence: > * if (err != -EPROBE_DEFER) > * dev_err(dev, ...); > @@ -3984,10 +3986,12 @@ int dev_err_probe(const struct device *dev, int err, > const char *fmt, ...) > vaf.fmt = fmt; > vaf.va = &args; > > - if (err != -EPROBE_DEFER) > + if (err != -EPROBE_DEFER) { > dev_err(dev, "error %d: %pV", err, &vaf); > - else > + } else { > + device_set_deferred_probe_reson(dev, &vaf); > dev_dbg(dev, "error %d: %pV", err, &vaf); > + } > > va_end(args); > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 9a1d940342ac..dd5683b61f74 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > > #include "base.h" > #include "power/power.h" > @@ -136,6 +137,8 @@ void driver_deferred_probe_del(struct device *dev) > if (!list_empty(&dev->p->deferred_probe)) { > dev_dbg(dev, "Removed from deferred list\n"); > list_del_init(&dev->p->deferred_probe); > + kfree(dev->p->deferred_probe_reason); > + dev->p->deferred_probe_reason = NULL; > } > mutex_unlock(&deferred_probe_mutex); > } > @@ -211,6 +214,23 @@ void device_unblock_probing(void) > driver_deferred_probe_trigger(); > } > > +/** > + * device_set_deferred_probe_reson() - Set defer probe reason message for > device > + * @dev: the pointer to the struct device > + * @vaf: the pointer to va_format structure with message > + */ > +void device_set_deferred_probe_reson(const struct device *dev, struct > va_format *vaf) > +{ > + const char *drv = dev_driver_string(dev); > + > + mutex_lock(&deferred_probe_mutex); > + > + kfree(dev->p->deferred_probe_reason); > + dev->p->deferred_probe_reason = kasprintf(GFP_KERNEL, "%s: %pV", drv, > vaf); > + > + mutex_unlock(&deferred_probe_mutex); > +} > + > /* > * deferred_devs_show() - Show the devices in the deferred probe pending > list. > */ > @@ -221,7 +241,8 @@ static int deferred_devs_show(struct seq_file *s, void > *data) > mutex_lock(&deferred_probe_mutex); > > list_for_each_entry(curr, &deferred_probe_pending_list, > deferred_probe) > - seq_printf(s, "%s\n", dev_name(curr->device)); > + seq_printf(s, "%s\t%s", dev_name(curr->device), > + curr->device->p->deferred_probe_reason ?: "\n"); > > mutex_unlock(&deferred_probe_mutex); > > --
Re: [PATCH v6 1/4] driver core: add device probe log helper
On Fri, Jun 26, 2020 at 12:01 PM Andrzej Hajda wrote: > > During probe every time driver gets resource it should usually check for > error printk some message if it is not -EPROBE_DEFER and return the error. > This pattern is simple but requires adding few lines after any resource > acquisition code, as a result it is often omitted or implemented only > partially. > dev_err_probe helps to replace such code sequences with simple call, > so code: > if (err != -EPROBE_DEFER) > dev_err(dev, ...); > return err; > becomes: > return probe_err(dev, err, ...); > > Signed-off-by: Andrzej Hajda Reviewed-by: Rafael J. Wysocki > --- > drivers/base/core.c| 42 ++ > include/linux/device.h | 3 +++ > 2 files changed, 45 insertions(+) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 67d39a90b45c..3a827c82933f 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -3953,6 +3953,48 @@ define_dev_printk_level(_dev_info, KERN_INFO); > > #endif > > +/** > + * dev_err_probe - probe error check and log helper > + * @dev: the pointer to the struct device > + * @err: error value to test > + * @fmt: printf-style format string > + * @...: arguments as specified in the format string > + * > + * This helper implements common pattern present in probe functions for error > + * checking: print debug or error message depending if the error value is > + * -EPROBE_DEFER and propagate error upwards. > + * It replaces code sequence: > + * if (err != -EPROBE_DEFER) > + * dev_err(dev, ...); > + * else > + * dev_dbg(dev, ...); > + * return err; > + * with > + * return dev_err_probe(dev, err, ...); > + * > + * Returns @err. > + * > + */ > +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...) > +{ > + struct va_format vaf; > + va_list args; > + > + va_start(args, fmt); > + vaf.fmt = fmt; > + vaf.va = &args; > + > + if (err != -EPROBE_DEFER) > + dev_err(dev, "error %d: %pV", err, &vaf); > + else > + dev_dbg(dev, "error %d: %pV", err, &vaf); > + > + va_end(args); > + > + return err; > +} > +EXPORT_SYMBOL_GPL(dev_err_probe); > + > static inline bool fwnode_is_primary(struct fwnode_handle *fwnode) > { > return fwnode && !IS_ERR(fwnode->secondary); > diff --git a/include/linux/device.h b/include/linux/device.h > index 15460a5ac024..6b2272ae9af8 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -964,6 +964,9 @@ void device_link_remove(void *consumer, struct device > *supplier); > void device_links_supplier_sync_state_pause(void); > void device_links_supplier_sync_state_resume(void); > > +extern __printf(3, 4) > +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...); > + > /* Create alias, so I can be autoloaded. */ > #define MODULE_ALIAS_CHARDEV(major,minor) \ > MODULE_ALIAS("char-major-" __stringify(major) "-" __stringify(minor)) > -- > 2.17.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] drm/msm: msm-fixes for 5.8-rc3
(retry with $subject) Hi Dave, A few fixes, mostly fallout from the address space refactor and dpu color processing. The following changes since commit 1cb2c4a2c89b2004a36399860c85a1af9b3fcba7: Revert "drm/msm/dpu: add support for clk and bw scaling for display" (2020-06-01 20:56:18 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/drm/msm.git drm-msm-fixes-2020-06-25 for you to fetch changes up to 30480e6ed508e3ff7a3e03c975696aa5196ffe8a: drm/msm: Fix up the rest of the messed up address sizes (2020-06-22 12:12:29 -0700) Bernard Zhao (1): drm/msm: fix potential memleak in error branch Chen Tao (1): drm/msm/dpu: fix error return code in dpu_encoder_init Eric Anholt (2): drm/msm: Fix address space size after refactor. drm/msm: Fix setup of a6xx create_address_space. John Stultz (1): drm/msm: Fix 0xlub in "Refactor address space initialization" Jordan Crouse (1): drm/msm: Fix up the rest of the messed up address sizes Kalyan Thota (1): drm/msm/dpu: request for display color blocks based on hw catalog entry Krishna Manikandan (1): drm/msm/dpu: allow initialization of encoder locks during encoder init drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c| 2 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c| 2 +- drivers/gpu/drm/msm/msm_submitqueue.c | 4 +++- 9 files changed, 21 insertions(+), 15 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain"
Hi Christian, On Fri, 26 Jun 2020, 18:10 Daniel Vetter, wrote: > On Fri, Jun 26, 2020 at 9:03 AM Christian König > wrote: > > > > Am 26.06.20 um 06:43 schrieb Sumit Semwal: > > > On Fri, 26 Jun 2020 at 01:24, Daniel Vetter wrote: > > >> Ignoring everything else ... > > >> > > >> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula < > jani.nik...@linux.intel.com> wrote: > > >>> As a side note, there seem to be extra checks in place for acks when > > >>> applying non-i915 patches to drm-intel; there are no such checks for > > >>> drm-misc. > > >> One option to generalize that that I pondered is to consult > > >> get_maintainers.pl asking for git repo link, and if that returns > > >> something else, then insist that there's an ack from a relevant > > >> maintainer. It's a bit of typing, but I think the bigger problem is > > >> that there's a ton of false positives. > > > Right; for the particular patch, I wasn't even in the to: or cc: field > > > and that made it slip from my radar. I would definitely ask any one > > > sending patches for dma-buf directory to follow the get_maintainers.pl > > > religiously. > > >> But maybe that's a good thing, would give some motivation to keep > > >> MAINTAINERS updated. > > > > Should I maybe add myself as maintainer as well? I've written enough > > stuff in there to know the code quite a bit. > > I think that makes lots of sense, since defacto you already are :-) > > If you feel like bikeshed, get_maintainers.pl also supports R: for > reviewer, but given that you also push patches to drm-misc M: for > maintainer feels more accurate. > I think given you've been reviewing and changing most of the code around dma-fences, it should be ok to add you as the maintainer for those bits? -Daniel > Best, Sumit. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args
On Fri, 26 Jun 2020, Daniel Thompson wrote: > On Thu, Jun 25, 2020 at 11:33:34AM +0100, Lee Jones wrote: > > On Thu, 25 Jun 2020, Daniel Thompson wrote: > > > > > On Wed, Jun 24, 2020 at 03:57:16PM +0100, Lee Jones wrote: > > > > Kerneldoc syntax is used, but not complete. Descriptions required. > > > > > > > > Prevents warnings like: > > > > > > > > drivers/video/backlight/ili922x.c:116: warning: Function parameter or > > > > member 's' not described in 'CHECK_FREQ_REG' > > > > drivers/video/backlight/ili922x.c:116: warning: Function parameter or > > > > member 'x' not described in 'CHECK_FREQ_REG' > > > > > > > > Cc: > > > > Cc: Bartlomiej Zolnierkiewicz > > > > Cc: Software Engineering > > > > Signed-off-by: Lee Jones > > > > --- > > > > drivers/video/backlight/ili922x.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/drivers/video/backlight/ili922x.c > > > > b/drivers/video/backlight/ili922x.c > > > > index 9c5aa3fbb2842..8cb4b9d3c3bba 100644 > > > > --- a/drivers/video/backlight/ili922x.c > > > > +++ b/drivers/video/backlight/ili922x.c > > > > @@ -107,6 +107,8 @@ > > > > * lower frequency when the registers are read/written. > > > > * The macro sets the frequency in the spi_transfer structure if > > > > * the frequency exceeds the maximum value. > > > > + * @s: pointer to controller side proxy for an SPI slave device > > > > > > What's wrong with "a pointer to an SPI device"? > > > > > > I am aware, having looked it up to find out what the above actually > > > means, that this is how struct spi_device is described in its own kernel > > > doc but quoting at that level of detail of both overkill and confusing. > > > > I figured that using the official description would be better than > > making something up. However if you think it's better to KISS, then I > > can change it. > > Yes, I'd strongly prefer KISS here. > > I know it is an "I am the world" argument[1] but I found using such a > dogmatically accurate description out of context to be very confusing > and therefore I don't think such a comment improves readability. > > [1]: See #3 from http://www.leany.com/logic/Adams.html It's fine, you are the world, I get it. ;) Do you even like Country music? Will fix! -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[no subject]
Hi Dave, A few fixes, mostly fallout from the address space refactor and dpu color processing. The following changes since commit 1cb2c4a2c89b2004a36399860c85a1af9b3fcba7: Revert "drm/msm/dpu: add support for clk and bw scaling for display" (2020-06-01 20:56:18 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/drm/msm.git drm-msm-fixes-2020-06-25 for you to fetch changes up to 30480e6ed508e3ff7a3e03c975696aa5196ffe8a: drm/msm: Fix up the rest of the messed up address sizes (2020-06-22 12:12:29 -0700) Bernard Zhao (1): drm/msm: fix potential memleak in error branch Chen Tao (1): drm/msm/dpu: fix error return code in dpu_encoder_init Eric Anholt (2): drm/msm: Fix address space size after refactor. drm/msm: Fix setup of a6xx create_address_space. John Stultz (1): drm/msm: Fix 0xlub in "Refactor address space initialization" Jordan Crouse (1): drm/msm: Fix up the rest of the messed up address sizes Kalyan Thota (1): drm/msm/dpu: request for display color blocks based on hw catalog entry Krishna Manikandan (1): drm/msm/dpu: allow initialization of encoder locks during encoder init drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c| 2 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c| 2 +- drivers/gpu/drm/msm/msm_submitqueue.c | 4 +++- 9 files changed, 21 insertions(+), 15 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic_helper: duplicate state for drm_private_obj
On Fri, Jun 26, 2020 at 6:46 AM Daniel Vetter wrote: > > On Thu, Jun 25, 2020 at 09:24:38AM -0700, Rob Clark wrote: > > On Thu, Jun 25, 2020 at 8:55 AM Daniel Vetter > > wrote: > > > > > > On Thu, Jun 25, 2020 at 4:17 PM Rob Clark wrote: > > > > > > > > On Thu, Jun 25, 2020 at 5:35 AM Daniel Vetter > > > > wrote: > > > > > > > > > > On Thu, Jun 25, 2020 at 1:58 PM Shawn Guo wrote: > > > > > > > > > > > > From: Shawn Guo > > > > > > > > > > > > The msm/mdp5 driver uses drm_private_obj as its global atomic state, > > > > > > which keeps the assignment of hwpipe to plane. With drm_private_obj > > > > > > missing from duplicate state call, mdp5 suspend works with no > > > > > > problem > > > > > > only for the very first time. Any subsequent suspend will hit the > > > > > > following warning, because hwpipe assignment doesn't get duplicated > > > > > > for > > > > > > suspend state. Adding drm_private_obj handling for duplicate state > > > > > > call > > > > > > fixes the problem. > > > > > > > > > > If the driver needs a private state, it's supposed to duplicate that > > > > > in its atomic_check functionality. This isn't the helper's job. > > > > > > > > > > If this is a bug in msm code, then pretty sure if you try hard enough, > > > > > you can hit the exact same bug from userspace too. Maybe good idea to > > > > > try and reproduce this with igt or something. > > > > > > > > The problem is how duplicate_state is used by the atomic > > > > suspend/resume helpers. They duplicate the running state on suspend, > > > > forgetting to duplicate the global state. Then everything is > > > > disabled, the driver correctly duplicates and updates it's global > > > > atomic state releasing the hwpipe. > > > > > > > > But then on resume, we are re-applying plane state that thinks it > > > > already has a hwpipe assigned (because that is part of the plane state > > > > that was duplicated), without reapplying the matching global state. > > > > > > > > On a normal atomic commit, we would duplicate the plane state that has > > > > the hwpipe disabled, which would be in sync with the drivers global > > > > state. But since that is not what the atomic resume helper does, we > > > > hit the situation where the plane state and the global state are out > > > > of sync. > > > > > > > > So the driver is dtrt, the problem really is with the helpers. I > > > > think this patch is the right thing to do. It is incorrect for the > > > > suspend/resume helpers to assume that they can re-apply duplicated > > > > state without including the global state. > > > > > > Hm, this is a bit awkward. Generally the assumption is that you should > > > be recomputing the derived state (like hwpipe) no matter what. If your > > > driver doesn't do that, then all kinds of things can leak from the > > > pre-resume to the post-resume side of things, that's kinda why I'm not > > > thrilled with this patch, I think it has good potential to open up a > > > can of worms. Iirc this patch has come up in the past, and in those > > > cases it was a driver bug. > > > > > > For this case, why is msm reusing a hw pipe assignment of a disabled > > > plane? > > > > Because it is part of the plane state that is being restored. > > > > Since resume uses the old state saved before the > > drm_atomic_helper_disable_all(), rather than duplicating the current > > state, we end up with this mismatch between global and plane state. I > > think stashing away the old state is probably ok, but we can't just do > > it piecemeal without including the global state. > > > > I suppose part of the problem is the hwpipe (and other such > > dynamically assigned resources) touch both private and plane (and > > crtc) state. The global state object knows which resources are > > assigned to which plane/crtc. But the plane/crtc state knows which of > > the (potentially) two hwpipe/mixers is "left" (primary) and "right" > > (secondary). > > Yeah I get all that, what I meant is: Why don't you just blindly recompute > the hwpipe assignment every time a full modeset is done? Caching it for > pure plane flips makes sense, but drm_crtc_needs_modset == true and just > throw it all overboard and assign new ones I think would also solve this > problem. Since the hwpipe global state would indicate that all pipes are > unallocated that should work (I hope). > > Imo just recomputing state is a good atomic pattern, it avoids drivers > getting stuck in a corner somewhere you can't reset them out of anymore. > > My question here was, why can't you do that? We do release the hwpipe on disable, and that is where things are getting out of sync. I suppose we could do some hack if needs_modeset and walk thru the global state to detect that we've got ourselves into this condition and the hwpipe(s) the plane *thinks* it has assigned to itself are no more. That sounds like a worse solution. (note that hwpipe can change for a lot of reasons other than modeset) > > > > Unfortunately we
Re: [PATCH] drm: drm_fourcc: Add generic alias for 16_16_TILE modifier
On Fri, Jun 26, 2020 at 05:16:53PM +0200, Daniel Vetter wrote: > On Fri, Jun 26, 2020 at 4:16 PM Brian Starkey wrote: > > > > Hi Daniel, > > > > On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > > > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey > > > wrote: > > > > > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > > > describes a generic pixel re-ordering which can be applicable to > > > > multiple vendors. > > > > > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > > > used to describe this layout in a vendor-neutral way, and add a > > > > comment about the expected usage of such "generic" modifiers. > > > > > > > > Signed-off-by: Brian Starkey > > > > --- > > > > > > > > This is based off a suggestion from Daniel here: > > > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > > > > > If there are future cases where a "generic" modifier is identified > > > > before merging with a specific vendor code, perhaps we could use > > > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > > > > > However, I also think we shouldn't try and "guess" what might be generic > > > > up-front and end up polluting the generic namespace. It seems fine to > > > > just alias vendor-specific definitions unless it's very clear-cut. > > > > > > I think the above two things would also be good additions to the comment. > > > > > > A totally different thing: I just noticed that we don't pull in all > > > the comments for modifiers. I think we could convert them to > > > kerneldoc, and then pull them into a separate section in drm-kms.rst. > > > Maybe even worth to pull out into a new drm-fourcc.rst document, since > > > these are officially shared with gl and vk standard extensions. Then > > > this new modifier's doc could simply point at teh existing one (and > > > the generated kerneldoc would make that a hyperlink for added > > > awesomeness). > > > > > > Aside from that makes sense to me, maybe just add it to your patch > > > series that will start making use of these. > > > > This is only supported by the GPU, so we wouldn't be using this on the > > Display side. > > I mean the patch to add the NV15 fource, it mentions "suitable for 16 > by 16 tile layout", would be nice to just put the generic modifier in > that comment. > -Daniel Ah right. I saw that one went out in Maarten's pull request this morning. -Brian > > > > > Thanks, > > -Brian > > > > > -Daniel > > > > > > > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > > > welcome. > > > > > > > > Cheers, > > > > -Brian > > > > > > > > include/uapi/drm/drm_fourcc.h | 19 +++ > > > > 1 file changed, 19 insertions(+) > > > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h > > > > b/include/uapi/drm/drm_fourcc.h > > > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > > > --- a/include/uapi/drm/drm_fourcc.h > > > > +++ b/include/uapi/drm/drm_fourcc.h > > > > @@ -345,8 +345,27 @@ extern "C" { > > > > * When adding a new token please document the layout with a code > > > > comment, > > > > * similar to the fourcc codes above. drm_fourcc.h is considered the > > > > * authoritative source for all of these. > > > > + * > > > > + * Generic modifier names: > > > > + * > > > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide > > > > vendor-neutral names > > > > + * for layouts which are common across multiple vendors. To preserve > > > > + * compatibility, in cases where a vendor-specific definition already > > > > exists and > > > > + * a generic name for it is desired, the common name is a purely > > > > symbolic alias > > > > + * and must use the same numerical value as the original definition. > > > > + * > > > > + * Note that generic names should only be used for modifiers which > > > > describe > > > > + * generic layouts (such as pixel re-ordering), which may have > > > > + * independently-developed support across multiple vendors. > > > > + * > > > > + * Generic names should not be used for cases where multiple hardware > > > > vendors > > > > + * have implementations of the same standardised compression scheme > > > > (such as > > > > + * AFBC). In those cases, all implementations should use the same > > > > format > > > > + * modifier(s), reflecting the vendor of the standard. > > > > */ > > > > > > > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE > > > > DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > > > > + > > > > /* > > > > * Invalid Modifier > > > > * > > > > -- > > > > 2.17.1 > > > > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: drm_fourcc: Add generic alias for 16_16_TILE modifier
On Fri, Jun 26, 2020 at 4:16 PM Brian Starkey wrote: > > Hi Daniel, > > On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey wrote: > > > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > > describes a generic pixel re-ordering which can be applicable to > > > multiple vendors. > > > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > > used to describe this layout in a vendor-neutral way, and add a > > > comment about the expected usage of such "generic" modifiers. > > > > > > Signed-off-by: Brian Starkey > > > --- > > > > > > This is based off a suggestion from Daniel here: > > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > > > If there are future cases where a "generic" modifier is identified > > > before merging with a specific vendor code, perhaps we could use > > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > > > However, I also think we shouldn't try and "guess" what might be generic > > > up-front and end up polluting the generic namespace. It seems fine to > > > just alias vendor-specific definitions unless it's very clear-cut. > > > > I think the above two things would also be good additions to the comment. > > > > A totally different thing: I just noticed that we don't pull in all > > the comments for modifiers. I think we could convert them to > > kerneldoc, and then pull them into a separate section in drm-kms.rst. > > Maybe even worth to pull out into a new drm-fourcc.rst document, since > > these are officially shared with gl and vk standard extensions. Then > > this new modifier's doc could simply point at teh existing one (and > > the generated kerneldoc would make that a hyperlink for added > > awesomeness). > > > > Aside from that makes sense to me, maybe just add it to your patch > > series that will start making use of these. > > This is only supported by the GPU, so we wouldn't be using this on the > Display side. I mean the patch to add the NV15 fource, it mentions "suitable for 16 by 16 tile layout", would be nice to just put the generic modifier in that comment. -Daniel > > Thanks, > -Brian > > > -Daniel > > > > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > > welcome. > > > > > > Cheers, > > > -Brian > > > > > > include/uapi/drm/drm_fourcc.h | 19 +++ > > > 1 file changed, 19 insertions(+) > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > > --- a/include/uapi/drm/drm_fourcc.h > > > +++ b/include/uapi/drm/drm_fourcc.h > > > @@ -345,8 +345,27 @@ extern "C" { > > > * When adding a new token please document the layout with a code > > > comment, > > > * similar to the fourcc codes above. drm_fourcc.h is considered the > > > * authoritative source for all of these. > > > + * > > > + * Generic modifier names: > > > + * > > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide > > > vendor-neutral names > > > + * for layouts which are common across multiple vendors. To preserve > > > + * compatibility, in cases where a vendor-specific definition already > > > exists and > > > + * a generic name for it is desired, the common name is a purely > > > symbolic alias > > > + * and must use the same numerical value as the original definition. > > > + * > > > + * Note that generic names should only be used for modifiers which > > > describe > > > + * generic layouts (such as pixel re-ordering), which may have > > > + * independently-developed support across multiple vendors. > > > + * > > > + * Generic names should not be used for cases where multiple hardware > > > vendors > > > + * have implementations of the same standardised compression scheme > > > (such as > > > + * AFBC). In those cases, all implementations should use the same format > > > + * modifier(s), reflecting the vendor of the standard. > > > */ > > > > > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE > > > DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > > > + > > > /* > > > * Invalid Modifier > > > * > > > -- > > > 2.17.1 > > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: document dma-fence-chain purpose/behavior
On 26/06/2020 17:22, Chris Wilson wrote: Quoting Lionel Landwerlin (2020-06-26 13:21:00) Trying to explain a bit how this thing works. In my opinion diagrams are a bit easier to understand than words. Signed-off-by: Lionel Landwerlin --- drivers/dma-buf/dma-fence-chain.c | 37 +++ 1 file changed, 37 insertions(+) diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c index 3d123502ff12..ac90ddf37b55 100644 --- a/drivers/dma-buf/dma-fence-chain.c +++ b/drivers/dma-buf/dma-fence-chain.c @@ -9,6 +9,43 @@ #include +/** + * DOC: DMA fence chains overview + * + * DMA fence chains, represented by &struct dma_fence_chain, are a kernel + * internal synchronization primitive providing a wrapping mechanism of other + * DMA fences in the form a single link list. + * + * One of the use case of this primitive is to implement Vulkan timeline + * semaphores (see VK_KHR_timeline_semaphore extension or Vulkan specification + * 1.2). + * + * Each DMA fence chain item wraps 2 items : + * + * - A previous DMA fence. + * + * - A DMA fence associated to the current &struct dma_fence_chain. + * + * A DMA fence chain becomes signaled when its previous fence as well as its + * associated fence are signaled. If a chain of dma fence chains is created, + * this property recurses, meaning that any dma fence chain element in the + * list becomes signaled only if its associated fence and all the previous + * fences in the chain are also signaled. + * + * A DMA fence chain's seqno is specified through dma_fence_chain_init(). This + * value is lower bound to the seqno of the previous fence to ensure the chain + * is monotically increasing. + * + * By traversing the chain's linked list, one can compute a seqno number + * associated with the chain such that is the highest number for which all + * previous fences have signaled. Next fence - 1 == highest seqno for all previous fences. Ok, what about the end point then? If you ask for a seqno higher than the last fence. Since that is not yet defined, it is an error, right? Correct, find_seqno() will return -EINVAL in that case. -Lionel Otherwise, we could interpret the highest possible seqno for the last fence as meaning U64_MAX. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: document dma-fence-chain purpose/behavior
Quoting Lionel Landwerlin (2020-06-26 13:21:00) > Trying to explain a bit how this thing works. In my opinion diagrams > are a bit easier to understand than words. > > Signed-off-by: Lionel Landwerlin > --- > drivers/dma-buf/dma-fence-chain.c | 37 +++ > 1 file changed, 37 insertions(+) > > diff --git a/drivers/dma-buf/dma-fence-chain.c > b/drivers/dma-buf/dma-fence-chain.c > index 3d123502ff12..ac90ddf37b55 100644 > --- a/drivers/dma-buf/dma-fence-chain.c > +++ b/drivers/dma-buf/dma-fence-chain.c > @@ -9,6 +9,43 @@ > > #include > > +/** > + * DOC: DMA fence chains overview > + * > + * DMA fence chains, represented by &struct dma_fence_chain, are a kernel > + * internal synchronization primitive providing a wrapping mechanism of other > + * DMA fences in the form a single link list. > + * > + * One of the use case of this primitive is to implement Vulkan timeline > + * semaphores (see VK_KHR_timeline_semaphore extension or Vulkan > specification > + * 1.2). > + * > + * Each DMA fence chain item wraps 2 items : > + * > + * - A previous DMA fence. > + * > + * - A DMA fence associated to the current &struct dma_fence_chain. > + * > + * A DMA fence chain becomes signaled when its previous fence as well as its > + * associated fence are signaled. If a chain of dma fence chains is created, > + * this property recurses, meaning that any dma fence chain element in the > + * list becomes signaled only if its associated fence and all the previous > + * fences in the chain are also signaled. > + * > + * A DMA fence chain's seqno is specified through dma_fence_chain_init(). > This > + * value is lower bound to the seqno of the previous fence to ensure the > chain > + * is monotically increasing. > + * > + * By traversing the chain's linked list, one can compute a seqno number > + * associated with the chain such that is the highest number for which all > + * previous fences have signaled. Next fence - 1 == highest seqno for all previous fences. Ok, what about the end point then? If you ask for a seqno higher than the last fence. Since that is not yet defined, it is an error, right? Otherwise, we could interpret the highest possible seqno for the last fence as meaning U64_MAX. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: drm_fourcc: add NV20 YUV format【请注意,邮件由linux-rockchip-bounces+sandy.huang=rock-chips....@lists.infradead.org代发】
On 2020-06-17 14:07, Huang Jiachai wrote: > Hi Jonas Karlman, > > Is there an another yuv 10bit format with 4:4:4 sub-simpling but > has no padding? > > Maybe we can call it DRM_FORMAT_NV30: > > { .format = DRM_FORMAT_NV30, .depth = 0, >.num_planes = 2, .char_per_block = { 5, 5, 0 }, >.block_w = { 4, 2, 0 }, .block_h = { 1, 1, 0 }, .hsub = 1, >.vsub = 1, .is_yuv = true }, > > this format can supported by rockchip rk3288/rk3399... platform, can you > add this format at this series patches? I will send a v2 including this 4:4:4 format later this weekend. Is there any hw block on rk3288/rk3399 that can produce a buffer in such format? If I am not mistaken rkvdec only support 10-bit h264 in 4:2:0/4:2:2 and hevc 4:2:0 10-bit, those are the formats I have been able to test so far. Regards, Jonas > > 在 2020/6/8 4:25, Jonas Karlman 写道: >> DRM_FORMAT_NV20 is a 2 plane format suitable for linear memory layout. >> The format is similar to P210 with 4:2:2 sub-sampling but has no padding >> between components. Instead, luminance and chrominance samples are grouped >> into 4s so that each group is packed into an integer number of bytes: >> >> = UVUV = 4 * 10 bits = 40 bits = 5 bytes >> >> The '20' suffix refers to the optimum effective bits per pixel which is >> achieved when the total number of luminance samples is a multiple of 4. >> >> Signed-off-by: Jonas Karlman >> --- >> drivers/gpu/drm/drm_fourcc.c | 4 >> include/uapi/drm/drm_fourcc.h | 1 + >> 2 files changed, 5 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c >> index 722c7ebe4e88..2a9c8ae719ed 100644 >> --- a/drivers/gpu/drm/drm_fourcc.c >> +++ b/drivers/gpu/drm/drm_fourcc.c >> @@ -278,6 +278,10 @@ const struct drm_format_info *__drm_format_info(u32 >> format) >>.num_planes = 2, .char_per_block = { 5, 5, 0 }, >>.block_w = { 4, 2, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2, >>.vsub = 2, .is_yuv = true }, >> +{ .format = DRM_FORMAT_NV20,.depth = 0, >> + .num_planes = 2, .char_per_block = { 5, 5, 0 }, >> + .block_w = { 4, 2, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2, >> + .vsub = 1, .is_yuv = true }, >> { .format = DRM_FORMAT_Q410,.depth = 0, >>.num_planes = 3, .char_per_block = { 2, 2, 2 }, >>.block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 0, >> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h >> index b5bf1c0e630e..244d32433a67 100644 >> --- a/include/uapi/drm/drm_fourcc.h >> +++ b/include/uapi/drm/drm_fourcc.h >> @@ -242,6 +242,7 @@ extern "C" { >>* index 1 = Cr:Cb plane, [39:0] Cr1:Cb1:Cr0:Cb0 little endian >>*/ >> #define DRM_FORMAT_NV15fourcc_code('N', 'V', '1', '5') /* 2x2 >> subsampled Cr:Cb plane */ >> +#define DRM_FORMAT_NV20 fourcc_code('N', 'V', '2', '0') /* 2x1 >> subsampled Cr:Cb plane */ >> >> /* >>* 2 plane YCbCr MSB aligned > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: drm_fourcc: Add generic alias for 16_16_TILE modifier
Hi Daniel, On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey wrote: > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > describes a generic pixel re-ordering which can be applicable to > > multiple vendors. > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > used to describe this layout in a vendor-neutral way, and add a > > comment about the expected usage of such "generic" modifiers. > > > > Signed-off-by: Brian Starkey > > --- > > > > This is based off a suggestion from Daniel here: > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > If there are future cases where a "generic" modifier is identified > > before merging with a specific vendor code, perhaps we could use > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > However, I also think we shouldn't try and "guess" what might be generic > > up-front and end up polluting the generic namespace. It seems fine to > > just alias vendor-specific definitions unless it's very clear-cut. > > I think the above two things would also be good additions to the comment. > > A totally different thing: I just noticed that we don't pull in all > the comments for modifiers. I think we could convert them to > kerneldoc, and then pull them into a separate section in drm-kms.rst. > Maybe even worth to pull out into a new drm-fourcc.rst document, since > these are officially shared with gl and vk standard extensions. Then > this new modifier's doc could simply point at teh existing one (and > the generated kerneldoc would make that a hyperlink for added > awesomeness). > > Aside from that makes sense to me, maybe just add it to your patch > series that will start making use of these. This is only supported by the GPU, so we wouldn't be using this on the Display side. Thanks, -Brian > -Daniel > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > welcome. > > > > Cheers, > > -Brian > > > > include/uapi/drm/drm_fourcc.h | 19 +++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > --- a/include/uapi/drm/drm_fourcc.h > > +++ b/include/uapi/drm/drm_fourcc.h > > @@ -345,8 +345,27 @@ extern "C" { > > * When adding a new token please document the layout with a code comment, > > * similar to the fourcc codes above. drm_fourcc.h is considered the > > * authoritative source for all of these. > > + * > > + * Generic modifier names: > > + * > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral > > names > > + * for layouts which are common across multiple vendors. To preserve > > + * compatibility, in cases where a vendor-specific definition already > > exists and > > + * a generic name for it is desired, the common name is a purely symbolic > > alias > > + * and must use the same numerical value as the original definition. > > + * > > + * Note that generic names should only be used for modifiers which describe > > + * generic layouts (such as pixel re-ordering), which may have > > + * independently-developed support across multiple vendors. > > + * > > + * Generic names should not be used for cases where multiple hardware > > vendors > > + * have implementations of the same standardised compression scheme (such > > as > > + * AFBC). In those cases, all implementations should use the same format > > + * modifier(s), reflecting the vendor of the standard. > > */ > > > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > > + > > /* > > * Invalid Modifier > > * > > -- > > 2.17.1 > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: drm_fourcc: Add generic alias for 16_16_TILE modifier
On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey wrote: > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > describes a generic pixel re-ordering which can be applicable to > multiple vendors. > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > used to describe this layout in a vendor-neutral way, and add a > comment about the expected usage of such "generic" modifiers. > > Signed-off-by: Brian Starkey > --- > > This is based off a suggestion from Daniel here: > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > If there are future cases where a "generic" modifier is identified > before merging with a specific vendor code, perhaps we could use > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > However, I also think we shouldn't try and "guess" what might be generic > up-front and end up polluting the generic namespace. It seems fine to > just alias vendor-specific definitions unless it's very clear-cut. I think the above two things would also be good additions to the comment. A totally different thing: I just noticed that we don't pull in all the comments for modifiers. I think we could convert them to kerneldoc, and then pull them into a separate section in drm-kms.rst. Maybe even worth to pull out into a new drm-fourcc.rst document, since these are officially shared with gl and vk standard extensions. Then this new modifier's doc could simply point at teh existing one (and the generated kerneldoc would make that a hyperlink for added awesomeness). Aside from that makes sense to me, maybe just add it to your patch series that will start making use of these. -Daniel > > I typed a version which was s/GENERIC/COMMON/, other suggestions > welcome. > > Cheers, > -Brian > > include/uapi/drm/drm_fourcc.h | 19 +++ > 1 file changed, 19 insertions(+) > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > index 299f9ac4840b..ef78dc9c3fb0 100644 > --- a/include/uapi/drm/drm_fourcc.h > +++ b/include/uapi/drm/drm_fourcc.h > @@ -345,8 +345,27 @@ extern "C" { > * When adding a new token please document the layout with a code comment, > * similar to the fourcc codes above. drm_fourcc.h is considered the > * authoritative source for all of these. > + * > + * Generic modifier names: > + * > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral > names > + * for layouts which are common across multiple vendors. To preserve > + * compatibility, in cases where a vendor-specific definition already exists > and > + * a generic name for it is desired, the common name is a purely symbolic > alias > + * and must use the same numerical value as the original definition. > + * > + * Note that generic names should only be used for modifiers which describe > + * generic layouts (such as pixel re-ordering), which may have > + * independently-developed support across multiple vendors. > + * > + * Generic names should not be used for cases where multiple hardware vendors > + * have implementations of the same standardised compression scheme (such as > + * AFBC). In those cases, all implementations should use the same format > + * modifier(s), reflecting the vendor of the standard. > */ > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > + > /* > * Invalid Modifier > * > -- > 2.17.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Warning triggered in drm_dp_delayed_destroy_work workqueue
On Fri, Jun 26, 2020 at 03:40:20PM +0200, Daniel Vetter wrote: > Adding Lyude, she's been revamping all the lifetime refcouting in the > dp code last few kernel releases. At a glance I don't even have an > idea what's going wrong here ... Already fixed by Imre I believe. 7d11507605a7 ("drm/dp_mst: Fix the DDC I2C device unregistration of an MST port") > -Daniel > > On Thu, Jun 25, 2020 at 12:22 PM Luis Henriques wrote: > > > > Hi! > > > > I've been seeing this warning occasionally, not sure if it has been > > reported yet. It's not a regression as I remember seeing it in, at least, > > 5.7. > > > > Anyway, here it is: > > > > [ cut here ] > > sysfs group 'power' not found for kobject 'i2c-7' > > WARNING: CPU: 1 PID: 17996 at fs/sysfs/group.c:279 > > sysfs_remove_group+0x74/0x80 > > Modules linked in: ccm(E) dell_rbtn(E) iwlmvm(E) mei_wdt(E) mac80211(E) > > libarc4(E) uvcvideo(E) dell_laptop(E) videobuf2_vmalloc(E) intel_rapl_> > > soundcore(E) intel_soc_dts_iosf(E) rng_core(E) battery(E) acpi_pad(E) > > sparse_keymap(E) acpi_thermal_rel(E) intel_pch_thermal(E) int3402_therm> > > sysfillrect(E) intel_lpss(E) sysimgblt(E) fb_sys_fops(E) idma64(E) > > scsi_mod(E) virt_dma(E) mfd_core(E) drm(E) fan(E) thermal(E) i2c_hid(E) hi> > > CPU: 1 PID: 17996 Comm: kworker/1:1 Tainted: GE 5.8.0-rc2+ > > #36 > > Hardware name: Dell Inc. Precision 5510/0N8J4R, BIOS 1.14.2 05/25/2020 > > Workqueue: events drm_dp_delayed_destroy_work [drm_kms_helper] > > RIP: 0010:sysfs_remove_group+0x74/0x80 > > Code: ff 5b 48 89 ef 5d 41 5c e9 79 bc ff ff 48 89 ef e8 01 b8 ff ff eb cc > > 49 8b 14 24 48 8b 33 48 c7 c7 90 ac 8b 93 e8 de b1 d4 ff <0f> 0b 5b> > > RSP: :b12d40c13c38 EFLAGS: 00010282 > > RAX: RBX: 936e6a60 RCX: 0027 > > RDX: 0027 RSI: 0086 RDI: 8e37de097b68 > > RBP: R08: 8e37de097b60 R09: 93fb4624 > > R10: 0904 R11: 0001002c R12: 8e37d3081c18 > > R13: 8e375f1450a8 R14: R15: 8e375f145410 > > FS: () GS:8e37de08() knlGS: > > CS: 0010 DS: ES: CR0: 80050033 > > CR2: CR3: 0004ab20a001 CR4: 003606e0 > > DR0: DR1: DR2: > > DR3: DR6: fffe0ff0 DR7: 0400 > > Call Trace: > > device_del+0x97/0x3f0 > > cdev_device_del+0x15/0x30 > > put_i2c_dev+0x7b/0x90 [i2c_dev] > > i2cdev_detach_adapter+0x33/0x60 [i2c_dev] > > notifier_call_chain+0x47/0x70 > > blocking_notifier_call_chain+0x3d/0x60 > > device_del+0x8f/0x3f0 > > device_unregister+0x16/0x60 > > i2c_del_adapter+0x247/0x300 > > drm_dp_port_set_pdt+0x90/0x2c0 [drm_kms_helper] > > drm_dp_delayed_destroy_work+0x2be/0x340 [drm_kms_helper] > > process_one_work+0x1ae/0x370 > > worker_thread+0x50/0x3a0 > > ? process_one_work+0x370/0x370 > > kthread+0x11b/0x140 > > ? kthread_associate_blkcg+0x90/0x90 > > ret_from_fork+0x22/0x30 > > ---[ end trace 16486ad3c2627482 ]--- > > [ cut here ] > > > > Cheers, > > -- > > Luis > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: drm_fourcc: Add generic alias for 16_16_TILE modifier
In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier describes a generic pixel re-ordering which can be applicable to multiple vendors. Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be used to describe this layout in a vendor-neutral way, and add a comment about the expected usage of such "generic" modifiers. Signed-off-by: Brian Starkey --- This is based off a suggestion from Daniel here: https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ If there are future cases where a "generic" modifier is identified before merging with a specific vendor code, perhaps we could use `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. However, I also think we shouldn't try and "guess" what might be generic up-front and end up polluting the generic namespace. It seems fine to just alias vendor-specific definitions unless it's very clear-cut. I typed a version which was s/GENERIC/COMMON/, other suggestions welcome. Cheers, -Brian include/uapi/drm/drm_fourcc.h | 19 +++ 1 file changed, 19 insertions(+) diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 299f9ac4840b..ef78dc9c3fb0 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -345,8 +345,27 @@ extern "C" { * When adding a new token please document the layout with a code comment, * similar to the fourcc codes above. drm_fourcc.h is considered the * authoritative source for all of these. + * + * Generic modifier names: + * + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral names + * for layouts which are common across multiple vendors. To preserve + * compatibility, in cases where a vendor-specific definition already exists and + * a generic name for it is desired, the common name is a purely symbolic alias + * and must use the same numerical value as the original definition. + * + * Note that generic names should only be used for modifiers which describe + * generic layouts (such as pixel re-ordering), which may have + * independently-developed support across multiple vendors. + * + * Generic names should not be used for cases where multiple hardware vendors + * have implementations of the same standardised compression scheme (such as + * AFBC). In those cases, all implementations should use the same format + * modifier(s), reflecting the vendor of the standard. */ +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE + /* * Invalid Modifier * -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: document dma-fence-chain purpose/behavior
On 26/06/2020 15:43, Daniel Vetter wrote: On Fri, Jun 26, 2020 at 2:21 PM Lionel Landwerlin wrote: Trying to explain a bit how this thing works. In my opinion diagrams are a bit easier to understand than words. kerneldoc supports in-line DOT graphs, see e.g. https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#overview If that doesn't work, then you can include a full-blown svg too. And yes for this a quick DOT graph that explains how things connect sound like the perfect use of a diagramm. Cheers, Daniel Thanks! Though I'm thinking I need a few to show the signaling behavior. Not sure how tractable that is with DOT/SVG. My last attempt was a series of slides... -Lionel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic_helper: duplicate state for drm_private_obj
On Thu, Jun 25, 2020 at 09:24:38AM -0700, Rob Clark wrote: > On Thu, Jun 25, 2020 at 8:55 AM Daniel Vetter wrote: > > > > On Thu, Jun 25, 2020 at 4:17 PM Rob Clark wrote: > > > > > > On Thu, Jun 25, 2020 at 5:35 AM Daniel Vetter > > > wrote: > > > > > > > > On Thu, Jun 25, 2020 at 1:58 PM Shawn Guo wrote: > > > > > > > > > > From: Shawn Guo > > > > > > > > > > The msm/mdp5 driver uses drm_private_obj as its global atomic state, > > > > > which keeps the assignment of hwpipe to plane. With drm_private_obj > > > > > missing from duplicate state call, mdp5 suspend works with no problem > > > > > only for the very first time. Any subsequent suspend will hit the > > > > > following warning, because hwpipe assignment doesn't get duplicated > > > > > for > > > > > suspend state. Adding drm_private_obj handling for duplicate state > > > > > call > > > > > fixes the problem. > > > > > > > > If the driver needs a private state, it's supposed to duplicate that > > > > in its atomic_check functionality. This isn't the helper's job. > > > > > > > > If this is a bug in msm code, then pretty sure if you try hard enough, > > > > you can hit the exact same bug from userspace too. Maybe good idea to > > > > try and reproduce this with igt or something. > > > > > > The problem is how duplicate_state is used by the atomic > > > suspend/resume helpers. They duplicate the running state on suspend, > > > forgetting to duplicate the global state. Then everything is > > > disabled, the driver correctly duplicates and updates it's global > > > atomic state releasing the hwpipe. > > > > > > But then on resume, we are re-applying plane state that thinks it > > > already has a hwpipe assigned (because that is part of the plane state > > > that was duplicated), without reapplying the matching global state. > > > > > > On a normal atomic commit, we would duplicate the plane state that has > > > the hwpipe disabled, which would be in sync with the drivers global > > > state. But since that is not what the atomic resume helper does, we > > > hit the situation where the plane state and the global state are out > > > of sync. > > > > > > So the driver is dtrt, the problem really is with the helpers. I > > > think this patch is the right thing to do. It is incorrect for the > > > suspend/resume helpers to assume that they can re-apply duplicated > > > state without including the global state. > > > > Hm, this is a bit awkward. Generally the assumption is that you should > > be recomputing the derived state (like hwpipe) no matter what. If your > > driver doesn't do that, then all kinds of things can leak from the > > pre-resume to the post-resume side of things, that's kinda why I'm not > > thrilled with this patch, I think it has good potential to open up a > > can of worms. Iirc this patch has come up in the past, and in those > > cases it was a driver bug. > > > > For this case, why is msm reusing a hw pipe assignment of a disabled plane? > > Because it is part of the plane state that is being restored. > > Since resume uses the old state saved before the > drm_atomic_helper_disable_all(), rather than duplicating the current > state, we end up with this mismatch between global and plane state. I > think stashing away the old state is probably ok, but we can't just do > it piecemeal without including the global state. > > I suppose part of the problem is the hwpipe (and other such > dynamically assigned resources) touch both private and plane (and > crtc) state. The global state object knows which resources are > assigned to which plane/crtc. But the plane/crtc state knows which of > the (potentially) two hwpipe/mixers is "left" (primary) and "right" > (secondary). Yeah I get all that, what I meant is: Why don't you just blindly recompute the hwpipe assignment every time a full modeset is done? Caching it for pure plane flips makes sense, but drm_crtc_needs_modset == true and just throw it all overboard and assign new ones I think would also solve this problem. Since the hwpipe global state would indicate that all pipes are unallocated that should work (I hope). Imo just recomputing state is a good atomic pattern, it avoids drivers getting stuck in a corner somewhere you can't reset them out of anymore. My question here was, why can't you do that? > > Unfortunately we can't copy the drm state into the overall structure > > to solve this, since that would miss driver-private properties. So > > unfortunately we can't make this match a real atomic commit from > > userspace perfectly. > > I'm not sure I understand this? The driver private properties > would/should be part of one of the state objs (plane/crtc/global)? If > the atomic state (including global) represents the entirety of the hw > state, you should be able to stash off N different versions of them > and re-apply them in any order you want because they are all > self-consistent. So for normal atomic commit we have: 1. duplicate current state 2. set pro
Re: Warning triggered in drm_dp_delayed_destroy_work workqueue
Adding Lyude, she's been revamping all the lifetime refcouting in the dp code last few kernel releases. At a glance I don't even have an idea what's going wrong here ... -Daniel On Thu, Jun 25, 2020 at 12:22 PM Luis Henriques wrote: > > Hi! > > I've been seeing this warning occasionally, not sure if it has been > reported yet. It's not a regression as I remember seeing it in, at least, > 5.7. > > Anyway, here it is: > > [ cut here ] > sysfs group 'power' not found for kobject 'i2c-7' > WARNING: CPU: 1 PID: 17996 at fs/sysfs/group.c:279 > sysfs_remove_group+0x74/0x80 > Modules linked in: ccm(E) dell_rbtn(E) iwlmvm(E) mei_wdt(E) mac80211(E) > libarc4(E) uvcvideo(E) dell_laptop(E) videobuf2_vmalloc(E) intel_rapl_> > soundcore(E) intel_soc_dts_iosf(E) rng_core(E) battery(E) acpi_pad(E) > sparse_keymap(E) acpi_thermal_rel(E) intel_pch_thermal(E) int3402_therm> > sysfillrect(E) intel_lpss(E) sysimgblt(E) fb_sys_fops(E) idma64(E) > scsi_mod(E) virt_dma(E) mfd_core(E) drm(E) fan(E) thermal(E) i2c_hid(E) hi> > CPU: 1 PID: 17996 Comm: kworker/1:1 Tainted: GE 5.8.0-rc2+ #36 > Hardware name: Dell Inc. Precision 5510/0N8J4R, BIOS 1.14.2 05/25/2020 > Workqueue: events drm_dp_delayed_destroy_work [drm_kms_helper] > RIP: 0010:sysfs_remove_group+0x74/0x80 > Code: ff 5b 48 89 ef 5d 41 5c e9 79 bc ff ff 48 89 ef e8 01 b8 ff ff eb cc 49 > 8b 14 24 48 8b 33 48 c7 c7 90 ac 8b 93 e8 de b1 d4 ff <0f> 0b 5b> > RSP: :b12d40c13c38 EFLAGS: 00010282 > RAX: RBX: 936e6a60 RCX: 0027 > RDX: 0027 RSI: 0086 RDI: 8e37de097b68 > RBP: R08: 8e37de097b60 R09: 93fb4624 > R10: 0904 R11: 0001002c R12: 8e37d3081c18 > R13: 8e375f1450a8 R14: R15: 8e375f145410 > FS: () GS:8e37de08() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: CR3: 0004ab20a001 CR4: 003606e0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > Call Trace: > device_del+0x97/0x3f0 > cdev_device_del+0x15/0x30 > put_i2c_dev+0x7b/0x90 [i2c_dev] > i2cdev_detach_adapter+0x33/0x60 [i2c_dev] > notifier_call_chain+0x47/0x70 > blocking_notifier_call_chain+0x3d/0x60 > device_del+0x8f/0x3f0 > device_unregister+0x16/0x60 > i2c_del_adapter+0x247/0x300 > drm_dp_port_set_pdt+0x90/0x2c0 [drm_kms_helper] > drm_dp_delayed_destroy_work+0x2be/0x340 [drm_kms_helper] > process_one_work+0x1ae/0x370 > worker_thread+0x50/0x3a0 > ? process_one_work+0x370/0x370 > kthread+0x11b/0x140 > ? kthread_associate_blkcg+0x90/0x90 > ret_from_fork+0x22/0x30 > ---[ end trace 16486ad3c2627482 ]--- > [ cut here ] > > Cheers, > -- > Luis -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Host1x/TegraDRM UAPI
On Fri, Jun 26, 2020 at 01:40:40PM +0200, Thierry Reding wrote: > On Fri, Jun 26, 2020 at 01:06:58PM +0200, Karol Herbst wrote: > > On Tue, Jun 23, 2020 at 3:03 PM Mikko Perttunen wrote: > > > > > > # Host1x/TegraDRM UAPI proposal > > > > > > This is a proposal for a stable UAPI for Host1x and TegraDRM, to replace > > > the current TegraDRM UAPI that is behind `STAGING` and quite obsolete in > > > many ways. > > > > > > I haven't written any implementation yet -- I'll do that once there is > > > some agreement on the high-level design. > > > > > > Current open items: > > > > > > * The syncpoint UAPI allows userspace to create sync_file FDs with > > > arbitrary syncpoint fences. dma_fence code currently seems to assume all > > > fences will be signaled, which would not necessarily be the case with > > > this interface. > > > * Previously present GEM IOCTLs (GEM_CREATE, GEM_MMAP) are not present. > > > Not sure if they are still needed. > > > > > > > Hi, as this wasn't addressed here (and sorry if I missed it): is there > > an open source userspace making use of this UAPI? Because this is > > something which needs to be seen before it can be included at all. > > There's a set of commits that implement an earlier draft here: > > https://github.com/thierryreding/linux/tree/for-5.6/drm-tegra-destage-abi > > and corresponding changes to libdrm to make use of that and implement > tests using the various generations of VIC here: > > https://cgit.freedesktop.org/~tagr/drm/log/ > > Before actually jumping to an implementation we wanted to go over the > design a bit more to avoid wasting a lot of work, like we've done in > the past. Turns out it isn't easy to get everyone to agree on a good > ABI. Who would've thought? =) > > I expect that once the discussion around the ABI settles Mikko will > start on implementing this ABI in the kernel and we'll update the > libdrm patches to make use of the new ABI as well. Do we have more than libdrm and tests for this, like something somewhat functional? Since tegradrm landed years ago we've refined the criteria a bit in this regard, latest version is explicit in that it needs to be something that's functional (for end-users in some form), not just infrastructure and tests. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 27/27] drm: Add default modes for connectors in unknown state
On Fri, Jun 26, 2020 at 10:25:45AM +0100, Daniel Stone wrote: > Hi, > > On Fri, 26 Jun 2020 at 10:00, Pekka Paalanen wrote: > > On Thu, 25 Jun 2020 12:44:36 +0200 Daniel Vetter wrote: > > > Maybe an aside, but the guideline is for autoconfiguration: > > > - Light up everything that has connector status connected. > > > - If nothing has that status, try to light up the connectors with > > > status "unknown". > > > > > > This is only really relevant on older platforms, mostly for VGA and > > > somewhat for dvi outputs. > > > > > > Maybe another thing we should put down somewhere in the uapi docs ... > > > > As I had no idea what "unknown" means or when it can happen, I assumed > > that it must mean "the hardware cannot know". If the hardware cannot > > know, then I certainly will not be trying to enable that, unless > > explicitly configured to do so. Having a phantom output is worse than > > having a real output that does not light up, because it's not obvious at > > first with phantom output that anything is wrong. You may just be > > wondering where your windows disappear, or where did you mouse cursor > > go, or why you see a wallpaper but no login dialog, etc. > > How about a refinement of Dan's suggestion, proceeding down this > logical order and breaking if true: > - ignore all disconnected outputs > - if any outputs are connected, ignore all unknown outputs > - if only one output is unknown, use only that output (with default > mode if need be) > - if any outputs are unknown but have EDID present, use only those outputs > - at this point, we have multiple unknown outputs with no EDID - break > and demand explicit user configuration EDID present generally means the status will be "connected". So not much of a refined. I'd say if you have multiple unknown, use a cloned config to avoid the "windows are disappearing" problem. Which is also what fbcon does, and iirc also -modesetting by default. But the most important part is to not light up "unknown" outputs if there's another output with a solid "connected". That avoids the problems Pekka points out, phantom outputs are bad. Really no need to refine beyond that, since imo that's a kernel bug. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 208333] New: No desktop on 5.8 rc1 rc2 with Gtx 760 driver nouveau
https://bugzilla.kernel.org/show_bug.cgi?id=208333 Bug ID: 208333 Summary: No desktop on 5.8 rc1 rc2 with Gtx 760 driver nouveau Product: Drivers Version: 2.5 Kernel Version: 5.8 rc2 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: robyguerr...@yahoo.it Regression: No With Kernel 5.8 rc1/rc2/ the desktop don't is visualized this on Ubuntu 20.04 and vedeo card Gtx 760 Nvidia with Nouveau Driver. The machine is on... the keyboards led working but the screen is black. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND] [PATCH v6 0/8] do not store GPU address in TTM
On Fri, Jun 26, 2020 at 3:04 PM Christian König wrote: > > Am 25.06.20 um 18:02 schrieb Christian König: > > Am 25.06.20 um 17:52 schrieb Christian König: > >> Am 25.06.20 um 17:44 schrieb Daniel Vetter: > >>> On Thu, Jun 25, 2020 at 11:50 AM Christian König > >>> wrote: > I've pushed patches #1, #2 and #5-#8 of this series to drm-misc-next. > >>> I think you left an unresolved conflict behind in drm-tip, please > >>> resolve per > >>> > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdrm.pages.freedesktop.org%2Fmaintainer-tools%2Fdrm-tip.html%23resolving-conflicts-when-rebuilding-drm-tip&data=02%7C01%7Cchristian.koenig%40amd.com%7Cc1441a81e9dc4fa4507208d8191ea0f6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637286966631595300&sdata=pfWS4VgDvU8IsR2638MDr7fYWE0nefa3b6XxyPCCsOU%3D&reserved=0 > >>> > >>> > >>> The script should have told you that already, so maybe reinvent that > >>> in whatever thing you're using :-) Jani has reported this on > >>> #dri-devel, this also holds up CI testing since we're running on top > >>> of drm-tip. > >> > >> Well I used dim push-branch. I even had to rebase once more because I > >> forgot a signed-of on one of the patches, so I'm pretty sure of that. > >> > >> Haven't seen anything problematic except for that while doing so. > > > > Ok, anyway that one was trivial to fix. > > Ran into the next merge conflict. But no surprise that I missed the > warning, that needs to be in red or so. Yeah the trouble is that the merging is done after you pushed, we don't do a test drm-tip rebuild before you push. Reason is that I've not gotten around to implementing this since years, it's on the wishlist. If we'd do a test-rebuild before pushing (something like use all the normal branches, excpet for $branch use the local one, or a specific sha1, with git worktree that's all shared so doable), then it would be a lot more obvious that people have some work to do before they're done with their push. Trouble is only rebuilding in dim push isn't enough, since then you can't test. So we need the full suite of tools be able to work with a local branch. And once you have a conflict which needs a fixup patch that's kinda not possible anymore. But maybe just doing the test-rebuild and if that fails, pointing it out and requiring a special --promises-I-fix-it-up-right-afterwards flag might be good enough. If you know how to print stuff in red if it's a terminal, then patching warn_or_fail should catch them all. -Daniel > > Regards, > Christian. > > > > > Thanks, > > Christian. > > > >> > >> Regards, > >> Christian. > >> > >>> -Daniel > >>> > >>> > >>> > Only VMGFX and Nouveau are missing and I'm pretty close to just push > them with my Acked-by since they should not contain any functional > change. > > Any objections? > > Thanks, > Christian. > > Am 24.06.20 um 20:26 schrieb Nirmoy Das: > > With this patch series I am trying to remove GPU address > > dependency in > > TTM and moving GPU address calculation to individual drm drivers. > > This > > cleanup will simplify introduction of drm_mem_region/domain work > > started > > by Brian Welty[1]. > > > > > > It would be nice if someone test this for nouveau. Rest of the > > drivers > > are already tested. > > > > v2: > > * set bo->offset = 0 for drm/nouveau if bo->mem.mm_node == NULL > > > > v3: > > * catch return value of drm_gem_vram_offset() in drm/bochs > > * introduce drm_gem_vram_pg_offset() in vram helper > > * improve nbo->offset calculation for nouveau > > > > v4: > > * minor coding style fixes in amdgpu and radeon > > * remove unnecessary kerneldoc for internal function > > > > v5: > > * rebase on top of drm-misc-next > > * fix return value of drm_gem_vram_pg_offset() > > * add a comment in drm_gem_vram_pg_offset() to clearify why we > > return 0. > > > > v6: > > * rebase to drm-misc-next > > * removed acked for vmwgfx as there was a small conflict > > > > Nirmoy Das (8): > > drm/amdgpu: move ttm bo->offset to amdgpu_bo > > drm/radeon: don't use ttm bo->offset > > drm/vmwgfx: don't use ttm bo->offset > > drm/nouveau: don't use ttm bo->offset v3 > > drm/qxl: don't use ttm bo->offset > > drm/vram-helper: don't use ttm bo->offset v4 > > drm/bochs: use drm_gem_vram_offset to get bo offset v2 > > drm/ttm: do not keep GPU dependent addresses > > > >drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 23 ++-- > >drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + > >drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 30 > > - > >drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + > >drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 +-- > >drivers/gpu/drm/bochs/bochs_kms.c | 7 +++
Re: [RESEND] [PATCH v6 0/8] do not store GPU address in TTM
On Fri, Jun 26, 2020 at 3:12 PM Daniel Vetter wrote: > > On Fri, Jun 26, 2020 at 3:04 PM Christian König > wrote: > > > > Am 25.06.20 um 18:02 schrieb Christian König: > > > Am 25.06.20 um 17:52 schrieb Christian König: > > >> Am 25.06.20 um 17:44 schrieb Daniel Vetter: > > >>> On Thu, Jun 25, 2020 at 11:50 AM Christian König > > >>> wrote: > > I've pushed patches #1, #2 and #5-#8 of this series to drm-misc-next. > > >>> I think you left an unresolved conflict behind in drm-tip, please > > >>> resolve per > > >>> > > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdrm.pages.freedesktop.org%2Fmaintainer-tools%2Fdrm-tip.html%23resolving-conflicts-when-rebuilding-drm-tip&data=02%7C01%7Cchristian.koenig%40amd.com%7Cc1441a81e9dc4fa4507208d8191ea0f6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637286966631595300&sdata=pfWS4VgDvU8IsR2638MDr7fYWE0nefa3b6XxyPCCsOU%3D&reserved=0 > > >>> > > >>> > > >>> The script should have told you that already, so maybe reinvent that > > >>> in whatever thing you're using :-) Jani has reported this on > > >>> #dri-devel, this also holds up CI testing since we're running on top > > >>> of drm-tip. > > >> > > >> Well I used dim push-branch. I even had to rebase once more because I > > >> forgot a signed-of on one of the patches, so I'm pretty sure of that. > > >> > > >> Haven't seen anything problematic except for that while doing so. > > > > > > Ok, anyway that one was trivial to fix. > > > > Ran into the next merge conflict. But no surprise that I missed the > > warning, that needs to be in red or so. > > Yeah the trouble is that the merging is done after you pushed, we > don't do a test drm-tip rebuild before you push. Reason is that I've > not gotten around to implementing this since years, it's on the > wishlist. > > If we'd do a test-rebuild before pushing (something like use all the > normal branches, excpet for $branch use the local one, or a specific > sha1, with git worktree that's all shared so doable), then it would be > a lot more obvious that people have some work to do before they're > done with their push. > > Trouble is only rebuilding in dim push isn't enough, since then you > can't test. So we need the full suite of tools be able to work with a > local branch. And once you have a conflict which needs a fixup patch > that's kinda not possible anymore. > > But maybe just doing the test-rebuild and if that fails, pointing it > out and requiring a special --promises-I-fix-it-up-right-afterwards > flag might be good enough. > > If you know how to print stuff in red if it's a terminal, then > patching warn_or_fail should catch them all. Uh I mean patching echoerr. -Daniel > -Daniel > > > > > > Regards, > > Christian. > > > > > > > > Thanks, > > > Christian. > > > > > >> > > >> Regards, > > >> Christian. > > >> > > >>> -Daniel > > >>> > > >>> > > >>> > > Only VMGFX and Nouveau are missing and I'm pretty close to just push > > them with my Acked-by since they should not contain any functional > > change. > > > > Any objections? > > > > Thanks, > > Christian. > > > > Am 24.06.20 um 20:26 schrieb Nirmoy Das: > > > With this patch series I am trying to remove GPU address > > > dependency in > > > TTM and moving GPU address calculation to individual drm drivers. > > > This > > > cleanup will simplify introduction of drm_mem_region/domain work > > > started > > > by Brian Welty[1]. > > > > > > > > > It would be nice if someone test this for nouveau. Rest of the > > > drivers > > > are already tested. > > > > > > v2: > > > * set bo->offset = 0 for drm/nouveau if bo->mem.mm_node == NULL > > > > > > v3: > > > * catch return value of drm_gem_vram_offset() in drm/bochs > > > * introduce drm_gem_vram_pg_offset() in vram helper > > > * improve nbo->offset calculation for nouveau > > > > > > v4: > > > * minor coding style fixes in amdgpu and radeon > > > * remove unnecessary kerneldoc for internal function > > > > > > v5: > > > * rebase on top of drm-misc-next > > > * fix return value of drm_gem_vram_pg_offset() > > > * add a comment in drm_gem_vram_pg_offset() to clearify why we > > > return 0. > > > > > > v6: > > > * rebase to drm-misc-next > > > * removed acked for vmwgfx as there was a small conflict > > > > > > Nirmoy Das (8): > > > drm/amdgpu: move ttm bo->offset to amdgpu_bo > > > drm/radeon: don't use ttm bo->offset > > > drm/vmwgfx: don't use ttm bo->offset > > > drm/nouveau: don't use ttm bo->offset v3 > > > drm/qxl: don't use ttm bo->offset > > > drm/vram-helper: don't use ttm bo->offset v4 > > > drm/bochs: use drm_gem_vram_offset to get bo offset v2 > > > drm/ttm: do not keep GPU dependent addresses > > > > > >drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 23 ++--
Re: [RESEND] [PATCH v6 0/8] do not store GPU address in TTM
Am 25.06.20 um 18:02 schrieb Christian König: Am 25.06.20 um 17:52 schrieb Christian König: Am 25.06.20 um 17:44 schrieb Daniel Vetter: On Thu, Jun 25, 2020 at 11:50 AM Christian König wrote: I've pushed patches #1, #2 and #5-#8 of this series to drm-misc-next. I think you left an unresolved conflict behind in drm-tip, please resolve per https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdrm.pages.freedesktop.org%2Fmaintainer-tools%2Fdrm-tip.html%23resolving-conflicts-when-rebuilding-drm-tip&data=02%7C01%7Cchristian.koenig%40amd.com%7Cc1441a81e9dc4fa4507208d8191ea0f6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637286966631595300&sdata=pfWS4VgDvU8IsR2638MDr7fYWE0nefa3b6XxyPCCsOU%3D&reserved=0 The script should have told you that already, so maybe reinvent that in whatever thing you're using :-) Jani has reported this on #dri-devel, this also holds up CI testing since we're running on top of drm-tip. Well I used dim push-branch. I even had to rebase once more because I forgot a signed-of on one of the patches, so I'm pretty sure of that. Haven't seen anything problematic except for that while doing so. Ok, anyway that one was trivial to fix. Ran into the next merge conflict. But no surprise that I missed the warning, that needs to be in red or so. Regards, Christian. Thanks, Christian. Regards, Christian. -Daniel Only VMGFX and Nouveau are missing and I'm pretty close to just push them with my Acked-by since they should not contain any functional change. Any objections? Thanks, Christian. Am 24.06.20 um 20:26 schrieb Nirmoy Das: With this patch series I am trying to remove GPU address dependency in TTM and moving GPU address calculation to individual drm drivers. This cleanup will simplify introduction of drm_mem_region/domain work started by Brian Welty[1]. It would be nice if someone test this for nouveau. Rest of the drivers are already tested. v2: * set bo->offset = 0 for drm/nouveau if bo->mem.mm_node == NULL v3: * catch return value of drm_gem_vram_offset() in drm/bochs * introduce drm_gem_vram_pg_offset() in vram helper * improve nbo->offset calculation for nouveau v4: * minor coding style fixes in amdgpu and radeon * remove unnecessary kerneldoc for internal function v5: * rebase on top of drm-misc-next * fix return value of drm_gem_vram_pg_offset() * add a comment in drm_gem_vram_pg_offset() to clearify why we return 0. v6: * rebase to drm-misc-next * removed acked for vmwgfx as there was a small conflict Nirmoy Das (8): drm/amdgpu: move ttm bo->offset to amdgpu_bo drm/radeon: don't use ttm bo->offset drm/vmwgfx: don't use ttm bo->offset drm/nouveau: don't use ttm bo->offset v3 drm/qxl: don't use ttm bo->offset drm/vram-helper: don't use ttm bo->offset v4 drm/bochs: use drm_gem_vram_offset to get bo offset v2 drm/ttm: do not keep GPU dependent addresses drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 23 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 30 - drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 +-- drivers/gpu/drm/bochs/bochs_kms.c | 7 - drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++- drivers/gpu/drm/nouveau/dispnv04/crtc.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv04/disp.c | 3 ++- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv50/base507c.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +- drivers/gpu/drm/nouveau/nouveau_abi16.c | 8 +++--- drivers/gpu/drm/nouveau/nouveau_bo.c | 8 ++ drivers/gpu/drm/nouveau/nouveau_bo.h | 3 +++ drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +- drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +++ drivers/gpu/drm/qxl/qxl_drv.h | 6 ++--- drivers/gpu/drm/qxl/qxl_kms.c | 5 ++-- drivers/gpu/drm/qxl/qxl_object.h | 5 drivers/gpu/drm/qxl/qxl_ttm.c | 9 --- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_object.h | 16 ++- drivers/gpu/drm/radeon/radeon_ttm.c | 4 +-- drivers/gpu/drm/ttm/ttm_bo.c | 7 - drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 -- include/drm/ttm/ttm_bo_api.h | 2 -- include/drm/ttm/ttm_bo_driver.h | 1 - 36 files c
Re: [RFC] drm: expose connector status values in uapi
On Fri, 26 Jun 2020 12:05:16 + Simon Ser wrote: > On Friday, June 26, 2020 11:15 AM, Pekka Paalanen wrote: > > > I have no opinion really if adding yet another set of the same > > definitions is good or not. We do need the UAPI doc, but that does not > > necessarily mean we also need definition code in UAPI headers. > > > > I give this one a shrug. > > But then uapi docs don't document uapi, instead document internal > kernel enums? And also user-space not using libdrm needs to have these > hardcoded somewhere. DRM properties are already like this. You don't find property names or enum value names in UAPI headers, you only find them in UAPI docs. > The libdrm re-definitions are annoying. Maybe a better way forward > would be to have a "status" prop, which could then also be used for > the planned fine-grained uapi events. That might be nice. Thanks, pq pgpe8SjJCLFmy.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: document dma-fence-chain purpose/behavior
On Fri, Jun 26, 2020 at 2:21 PM Lionel Landwerlin wrote: > > Trying to explain a bit how this thing works. In my opinion diagrams > are a bit easier to understand than words. kerneldoc supports in-line DOT graphs, see e.g. https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#overview If that doesn't work, then you can include a full-blown svg too. And yes for this a quick DOT graph that explains how things connect sound like the perfect use of a diagramm. Cheers, Daniel > > Signed-off-by: Lionel Landwerlin > --- > drivers/dma-buf/dma-fence-chain.c | 37 +++ > 1 file changed, 37 insertions(+) > > diff --git a/drivers/dma-buf/dma-fence-chain.c > b/drivers/dma-buf/dma-fence-chain.c > index 3d123502ff12..ac90ddf37b55 100644 > --- a/drivers/dma-buf/dma-fence-chain.c > +++ b/drivers/dma-buf/dma-fence-chain.c > @@ -9,6 +9,43 @@ > > #include > > +/** > + * DOC: DMA fence chains overview > + * > + * DMA fence chains, represented by &struct dma_fence_chain, are a kernel > + * internal synchronization primitive providing a wrapping mechanism of other > + * DMA fences in the form a single link list. > + * > + * One of the use case of this primitive is to implement Vulkan timeline > + * semaphores (see VK_KHR_timeline_semaphore extension or Vulkan > specification > + * 1.2). > + * > + * Each DMA fence chain item wraps 2 items : > + * > + * - A previous DMA fence. > + * > + * - A DMA fence associated to the current &struct dma_fence_chain. > + * > + * A DMA fence chain becomes signaled when its previous fence as well as its > + * associated fence are signaled. If a chain of dma fence chains is created, > + * this property recurses, meaning that any dma fence chain element in the > + * list becomes signaled only if its associated fence and all the previous > + * fences in the chain are also signaled. > + * > + * A DMA fence chain's seqno is specified through dma_fence_chain_init(). > This > + * value is lower bound to the seqno of the previous fence to ensure the > chain > + * is monotically increasing. > + * > + * By traversing the chain's linked list, one can compute a seqno number > + * associated with the chain such that is the highest number for which all > + * previous fences have signaled. > + * > + * One can also traverse the chain's linked list to find a &struct > + * dma_fence_chain that when signaled guarantees that all previous fences in > + * the chain are signaled. dma_fence_chain_find_seqno() provides this > + * functionality. > + */ > + > static bool dma_fence_chain_enable_signaling(struct dma_fence *fence); > > /** > -- > 2.27.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain"
On Fri, Jun 26, 2020 at 9:03 AM Christian König wrote: > > Am 26.06.20 um 06:43 schrieb Sumit Semwal: > > On Fri, 26 Jun 2020 at 01:24, Daniel Vetter wrote: > >> Ignoring everything else ... > >> > >> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula > >> wrote: > >>> As a side note, there seem to be extra checks in place for acks when > >>> applying non-i915 patches to drm-intel; there are no such checks for > >>> drm-misc. > >> One option to generalize that that I pondered is to consult > >> get_maintainers.pl asking for git repo link, and if that returns > >> something else, then insist that there's an ack from a relevant > >> maintainer. It's a bit of typing, but I think the bigger problem is > >> that there's a ton of false positives. > > Right; for the particular patch, I wasn't even in the to: or cc: field > > and that made it slip from my radar. I would definitely ask any one > > sending patches for dma-buf directory to follow the get_maintainers.pl > > religiously. > >> But maybe that's a good thing, would give some motivation to keep > >> MAINTAINERS updated. > > Should I maybe add myself as maintainer as well? I've written enough > stuff in there to know the code quite a bit. I think that makes lots of sense, since defacto you already are :-) If you feel like bikeshed, get_maintainers.pl also supports R: for reviewer, but given that you also push patches to drm-misc M: for maintainer feels more accurate. -Daniel > > Christian. > > >> > >> The other issue is though that drm-misc is plenty used to merge > >> patches even when the respective maintainers are absent for weeks, or > >> unresponsive. If we just blindly implement that rule, then the only > >> possible Ack for these would be Dave&me as subsystem maintainers, and > >> I don't want to be in the business of stamping approvals for all this > >> stuff. Much better if people just collaborate. > >> > >> So I think an ack check would be nice, but probably not practical. > >> > >> Plus in this situation here drm-misc.git actually is the main repo, > >> and we wont ever be able to teach a script to make a judgement call of > >> whether that patch has the right amount of review on it. > >> -Daniel > > Best, > > Sumit. > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dma-buf: document dma-fence-chain purpose/behavior
Trying to explain a bit how this thing works. In my opinion diagrams are a bit easier to understand than words. Signed-off-by: Lionel Landwerlin --- drivers/dma-buf/dma-fence-chain.c | 37 +++ 1 file changed, 37 insertions(+) diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c index 3d123502ff12..ac90ddf37b55 100644 --- a/drivers/dma-buf/dma-fence-chain.c +++ b/drivers/dma-buf/dma-fence-chain.c @@ -9,6 +9,43 @@ #include +/** + * DOC: DMA fence chains overview + * + * DMA fence chains, represented by &struct dma_fence_chain, are a kernel + * internal synchronization primitive providing a wrapping mechanism of other + * DMA fences in the form a single link list. + * + * One of the use case of this primitive is to implement Vulkan timeline + * semaphores (see VK_KHR_timeline_semaphore extension or Vulkan specification + * 1.2). + * + * Each DMA fence chain item wraps 2 items : + * + * - A previous DMA fence. + * + * - A DMA fence associated to the current &struct dma_fence_chain. + * + * A DMA fence chain becomes signaled when its previous fence as well as its + * associated fence are signaled. If a chain of dma fence chains is created, + * this property recurses, meaning that any dma fence chain element in the + * list becomes signaled only if its associated fence and all the previous + * fences in the chain are also signaled. + * + * A DMA fence chain's seqno is specified through dma_fence_chain_init(). This + * value is lower bound to the seqno of the previous fence to ensure the chain + * is monotically increasing. + * + * By traversing the chain's linked list, one can compute a seqno number + * associated with the chain such that is the highest number for which all + * previous fences have signaled. + * + * One can also traverse the chain's linked list to find a &struct + * dma_fence_chain that when signaled guarantees that all previous fences in + * the chain are signaled. dma_fence_chain_find_seqno() provides this + * functionality. + */ + static bool dma_fence_chain_enable_signaling(struct dma_fence *fence); /** -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] drm: expose connector status values in uapi
On Friday, June 26, 2020 11:15 AM, Pekka Paalanen wrote: > I have no opinion really if adding yet another set of the same > definitions is good or not. We do need the UAPI doc, but that does not > necessarily mean we also need definition code in UAPI headers. > > I give this one a shrug. But then uapi docs don't document uapi, instead document internal kernel enums? And also user-space not using libdrm needs to have these hardcoded somewhere. The libdrm re-definitions are annoying. Maybe a better way forward would be to have a "status" prop, which could then also be used for the planned fine-grained uapi events. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v3] drm/bridge: ti-sn65dsi86: ensure bridge suspend happens during PM sleep
Hi, On 19/06/2020 00:09, Doug Anderson wrote: > Hi, > > On Tue, Jun 9, 2020 at 5:05 AM Harigovindan P wrote: >> >> ti-sn65dsi86 bridge is enumerated as a runtime device. When >> suspend is triggered, PM core adds a refcount on all the >> devices and calls device suspend, since usage count is >> already incremented, runtime suspend will not be called >> and it kept the bridge regulators and gpios ON which resulted >> in platform not entering into XO shutdown. >> >> Add changes to force suspend on the runtime device during pm sleep. >> >> Signed-off-by: Harigovindan P >> --- >> >> Changes in v2: >> - Include bridge name in the commit message and >> remove dependent patchwork link from the commit >> text as bridge is independent of OEM(Stephen Boyd) >> >> Changes in v3: >> - Updating changelog to explain the need for patch >> >> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 ++ >> 1 file changed, 2 insertions(+) > > I think this patch is good to go now (has both Stephen's and my > reviews). I noticed that Neil landed my other patches to this driver > recently (thanks!) and wondered why he didn't land this one. Then, I > realized that you didn't send it to him or the other bridge > maintainer. :( Have you tried running get_maintainer? > > $ ./scripts/get_maintainer.pl -f drivers/gpu/drm/bridge/ti-sn65dsi86.c > Andrzej Hajda (maintainer:DRM DRIVERS FOR BRIDGE CHIPS) > Neil Armstrong (maintainer:DRM DRIVERS FOR > BRIDGE CHIPS) > Laurent Pinchart (reviewer:DRM > DRIVERS FOR BRIDGE CHIPS) > Jonas Karlman (reviewer:DRM DRIVERS FOR BRIDGE CHIPS) > Jernej Skrabec (reviewer:DRM DRIVERS FOR BRIDGE > CHIPS) > David Airlie (maintainer:DRM DRIVERS) > Daniel Vetter (maintainer:DRM DRIVERS) > dri-devel@lists.freedesktop.org (open list:DRM DRIVERS) > linux-ker...@vger.kernel.org (open list) > > In any case, unless someone has extra feedback on this patch I think > it's ready to land. > > Neil: If you're willing to land this patch too, can you let > Harigovindan know if it needs to be re-sent with you in the "To:" list > or if you can find it on the dri-devel list? Sorry missed this one, Applying to drm-misc-next Neil > > Thanks! > > -Doug > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Host1x/TegraDRM UAPI
On Fri, Jun 26, 2020 at 01:06:58PM +0200, Karol Herbst wrote: > On Tue, Jun 23, 2020 at 3:03 PM Mikko Perttunen wrote: > > > > # Host1x/TegraDRM UAPI proposal > > > > This is a proposal for a stable UAPI for Host1x and TegraDRM, to replace > > the current TegraDRM UAPI that is behind `STAGING` and quite obsolete in > > many ways. > > > > I haven't written any implementation yet -- I'll do that once there is > > some agreement on the high-level design. > > > > Current open items: > > > > * The syncpoint UAPI allows userspace to create sync_file FDs with > > arbitrary syncpoint fences. dma_fence code currently seems to assume all > > fences will be signaled, which would not necessarily be the case with > > this interface. > > * Previously present GEM IOCTLs (GEM_CREATE, GEM_MMAP) are not present. > > Not sure if they are still needed. > > > > Hi, as this wasn't addressed here (and sorry if I missed it): is there > an open source userspace making use of this UAPI? Because this is > something which needs to be seen before it can be included at all. There's a set of commits that implement an earlier draft here: https://github.com/thierryreding/linux/tree/for-5.6/drm-tegra-destage-abi and corresponding changes to libdrm to make use of that and implement tests using the various generations of VIC here: https://cgit.freedesktop.org/~tagr/drm/log/ Before actually jumping to an implementation we wanted to go over the design a bit more to avoid wasting a lot of work, like we've done in the past. Turns out it isn't easy to get everyone to agree on a good ABI. Who would've thought? =) I expect that once the discussion around the ABI settles Mikko will start on implementing this ABI in the kernel and we'll update the libdrm patches to make use of the new ABI as well. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/1] drm/bridge: analogix_dp: Add PSR toggle
This patch adds a commandline toggle for the Panel Self-Refresh feature to the analogix_dp bridge driver, much like the one in i915. This is required to work around a hardware fault in some Pinebook Pro units from the May 2020 batch whose display panels seem to behave sporadically when PSR is enabled. Shawn Anastasio (1): drm/bridge: analogix_dp: Add enable_psr param drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/1] drm/bridge: analogix_dp: Add enable_psr param
Add a toggle to enable/disable PSR from the kernel commandline. This is useful in situations where PSR is supported by the hardware but is not desired by the user. One such use case is working around hardware errata. Signed-off-by: Shawn Anastasio --- drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 76736fb8ed94..9735ab71fca7 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -35,6 +35,10 @@ static const bool verify_fast_training; +static bool enable_psr = true; +module_param(enable_psr, bool, 0644); +MODULE_PARM_DESC(enable_psr, "PSR support (1 = enabled (default), 0 = disabled)"); + struct bridge_init { struct i2c_client *client; struct device_node *node; @@ -979,7 +983,7 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) if (ret) return ret; - if (analogix_dp_detect_sink_psr(dp)) { + if (enable_psr && analogix_dp_detect_sink_psr(dp)) { ret = analogix_dp_enable_sink_psr(dp); if (ret) return ret; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Host1x/TegraDRM UAPI
On Fri, Jun 26, 2020 at 1:13 PM Mikko Perttunen wrote: > > On 6/26/20 2:06 PM, Karol Herbst wrote: > > On Tue, Jun 23, 2020 at 3:03 PM Mikko Perttunen wrote: > >> > >> # Host1x/TegraDRM UAPI proposal > >> > >> This is a proposal for a stable UAPI for Host1x and TegraDRM, to replace > >> the current TegraDRM UAPI that is behind `STAGING` and quite obsolete in > >> many ways. > >> > >> I haven't written any implementation yet -- I'll do that once there is > >> some agreement on the high-level design. > >> > >> Current open items: > >> > >> * The syncpoint UAPI allows userspace to create sync_file FDs with > >> arbitrary syncpoint fences. dma_fence code currently seems to assume all > >> fences will be signaled, which would not necessarily be the case with > >> this interface. > >> * Previously present GEM IOCTLs (GEM_CREATE, GEM_MMAP) are not present. > >> Not sure if they are still needed. > >> > > > > Hi, as this wasn't addressed here (and sorry if I missed it): is there > > an open source userspace making use of this UAPI? Because this is > > something which needs to be seen before it can be included at all. > > > > Hi Karol, > > not currently, but once we have hashed out the design a little bit (and > I'm back from vacation), I'll work on open source userspace and > converting existing code using the staging UAPI to this one. > > Mikko > okay, cool, sounds good! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Host1x/TegraDRM UAPI
On 6/26/20 2:06 PM, Karol Herbst wrote: On Tue, Jun 23, 2020 at 3:03 PM Mikko Perttunen wrote: # Host1x/TegraDRM UAPI proposal This is a proposal for a stable UAPI for Host1x and TegraDRM, to replace the current TegraDRM UAPI that is behind `STAGING` and quite obsolete in many ways. I haven't written any implementation yet -- I'll do that once there is some agreement on the high-level design. Current open items: * The syncpoint UAPI allows userspace to create sync_file FDs with arbitrary syncpoint fences. dma_fence code currently seems to assume all fences will be signaled, which would not necessarily be the case with this interface. * Previously present GEM IOCTLs (GEM_CREATE, GEM_MMAP) are not present. Not sure if they are still needed. Hi, as this wasn't addressed here (and sorry if I missed it): is there an open source userspace making use of this UAPI? Because this is something which needs to be seen before it can be included at all. Hi Karol, not currently, but once we have hashed out the design a little bit (and I'm back from vacation), I'll work on open source userspace and converting existing code using the staging UAPI to this one. Mikko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: Delete the OT200 backlight driver
On Fri, Jun 26, 2020 at 12:25:00PM +0200, Linus Walleij wrote: > This driver has no in-kernel users. The device can only be populated > by board files since it does not support device tree nor ACPI, > and nothing in the kernel creates a device named "ot200-backlight". > > This driver has been in the kernel since 2012. If it is used by > out-of-tree code that code should have been upstreamed by now, > it's been 8 years. > > It uses the idiomatic forked GPIO of the CS5535 which combines > pin control and GPIO into its private custom interface, which > causes me a headache because that is not how we do things these > days: we creates separate pin control and GPIO drivers. > > Delete this unused driver. > > Cc: Christian Gmeiner > Signed-off-by: Linus Walleij I was curious about this if only because this a driver for an x86 system and I wondered how the platform device could ever have been registered. However I drew a total blank... I can find some tweaks to coreboot that make this code work better[1] but even after a careful search of the coreboot code (just in case some driver I don't know about managed to fetch the "ot200-backlight" string from the firmware) but I can't find any code within reach of a search engine (out of tree of not) that could cause this driver to activate :-( So, unless we get objections, I've reached the same view as Linus (and in this post have documented the effort involved): Reviewed-by: Daniel Thompson Daniel. [1] https://review.coreboot.org/cgit/coreboot.git/commit/?id=4412bc4 > --- > drivers/video/backlight/Kconfig| 7 -- > drivers/video/backlight/Makefile | 1 - > drivers/video/backlight/ot200_bl.c | 162 - > 3 files changed, 170 deletions(-) > delete mode 100644 drivers/video/backlight/ot200_bl.c > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > index 7d22d7377606..95c546cc8774 100644 > --- a/drivers/video/backlight/Kconfig > +++ b/drivers/video/backlight/Kconfig > @@ -386,13 +386,6 @@ config BACKLIGHT_LP8788 > help > This supports TI LP8788 backlight driver. > > -config BACKLIGHT_OT200 > - tristate "Backlight driver for ot200 visualisation device" > - depends on CS5535_MFGPT && GPIO_CS5535 > - help > - To compile this driver as a module, choose M here: the module will be > - called ot200_bl. > - > config BACKLIGHT_PANDORA > tristate "Backlight driver for Pandora console" > depends on TWL4030_CORE > diff --git a/drivers/video/backlight/Makefile > b/drivers/video/backlight/Makefile > index 0c1a1524627a..2072d21b60f7 100644 > --- a/drivers/video/backlight/Makefile > +++ b/drivers/video/backlight/Makefile > @@ -45,7 +45,6 @@ obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o > obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o > obj-$(CONFIG_BACKLIGHT_MAX8925) += max8925_bl.o > obj-$(CONFIG_BACKLIGHT_OMAP1)+= omap1_bl.o > -obj-$(CONFIG_BACKLIGHT_OT200)+= ot200_bl.o > obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o > obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o > obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o > diff --git a/drivers/video/backlight/ot200_bl.c > b/drivers/video/backlight/ot200_bl.c > deleted file mode 100644 > index 23ee7106c72a.. > --- a/drivers/video/backlight/ot200_bl.c > +++ /dev/null > @@ -1,162 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0-only > -/* > - * Copyright (C) 2012 Bachmann electronic GmbH > - * Christian Gmeiner > - * > - * Backlight driver for ot200 visualisation device from > - * Bachmann electronic GmbH. > - */ > - > -#include > -#include > -#include > -#include > -#include > -#include > - > -static struct cs5535_mfgpt_timer *pwm_timer; > - > -/* this array defines the mapping of brightness in % to pwm frequency */ > -static const u8 dim_table[101] = {0, 0, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, > 2, > - 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 4, 4, 4, 4, > - 4, 5, 5, 5, 5, 6, 6, 6, 7, 7, 7, 8, 8, 9, 9, > - 10, 10, 11, 11, 12, 12, 13, 14, 15, 15, 16, > - 17, 18, 19, 20, 21, 22, 23, 24, 26, 27, 28, > - 30, 31, 33, 35, 37, 39, 41, 43, 45, 47, 50, > - 53, 55, 58, 61, 65, 68, 72, 75, 79, 84, 88, > - 93, 97, 103, 108, 114, 120, 126, 133, 140, > - 147, 155, 163}; > - > -struct ot200_backlight_data { > - int current_brightness; > -}; > - > -#define GPIO_DIMM27 > -#define SCALE1 > -#define CMP1MODE 0x2 /* compare on GE; output high on compare > - * greater than or equal */ > -#define PWM_SETUP(SCALE | CMP1MODE << 6 | MFGPT_SETUP_CNTEN) > -#define MAX_COMP2163 > - > -static int ot200_backlight_update_status(st
Re: [RFC] Host1x/TegraDRM UAPI
On Tue, Jun 23, 2020 at 3:03 PM Mikko Perttunen wrote: > > # Host1x/TegraDRM UAPI proposal > > This is a proposal for a stable UAPI for Host1x and TegraDRM, to replace > the current TegraDRM UAPI that is behind `STAGING` and quite obsolete in > many ways. > > I haven't written any implementation yet -- I'll do that once there is > some agreement on the high-level design. > > Current open items: > > * The syncpoint UAPI allows userspace to create sync_file FDs with > arbitrary syncpoint fences. dma_fence code currently seems to assume all > fences will be signaled, which would not necessarily be the case with > this interface. > * Previously present GEM IOCTLs (GEM_CREATE, GEM_MMAP) are not present. > Not sure if they are still needed. > Hi, as this wasn't addressed here (and sorry if I missed it): is there an open source userspace making use of this UAPI? Because this is something which needs to be seen before it can be included at all. > ## Introduction to the hardware > > Tegra Host1x is a hardware block providing the following capabilities: > > * Syncpoints, a unified whole-system synchronization primitive, allowing > synchronization of work between graphics, compute and multimedia > engines, CPUs including cross-VM synchronization, and devices on the > PCIe bus, without incurring CPU overhead. > * Channels, a command DMA mechanism allowing asynchronous programming of > various engines, integrating with syncpoints. > * Hardware virtualization support for syncpoints and channels. (On > Tegra186 and newer) > > This proposal defines APIs for userspace access to syncpoints and > channels. Kernel drivers can additionally use syncpoints and channels > internally, providing other userspace interfaces (e.g. V4L2). > > Syncpoint and channel interfaces are split into separate parts, as > syncpoints are useful as a system synchronization primitive even without > using the engine drivers provided through TegraDRM. For example, a > computer vision pipeline consisting of video capture, CPU processing and > GPU processing would not necessarily use the engines provided by > TegraDRM. See the Example workflows section for more details. > > ## Syncpoint interface > > Syncpoints are a set of 32-bit values providing the following operations: > > * Atomically increment value by one > * Read current value > * Wait until value reaches specified threshold. For waiting, the 32-bit > value space is treated modulo 2^32; e.g. if the current value is > 0x, then value 0x0 is considered to be one increment in the future. > > Each syncpoint is identified by a system-global ID ranging between [0, > number of syncpoints supported by hardware). The entire system has > read-only access to all syncpoints based on their ID. > > Syncpoints are managed through the device node /dev/host1x provided by > the gpu/host1x driver. > > ### IOCTL HOST1X_ALLOCATE_SYNCPOINT (on /dev/host1x) > > Allocates a free syncpoint, returning a file descriptor representing it. > Only the owner of the file descriptor is allowed to mutate the value of > the syncpoint. > > ``` > struct host1x_ctrl_allocate_syncpoint { > /** > * @fd: > * > * [out] New file descriptor representing the allocated syncpoint. > */ > __s32 fd; > > __u32 reserved[3]; > }; > ``` > > ### IOCTL HOST1X_SYNCPOINT_INFO (on syncpoint file descriptor) > > Allows retrieval of system-global syncpoint ID corresponding to syncpoint. > > Use cases: > > * Passing ID to other system components that identify syncpoints by ID > * Debugging and testing > > ``` > struct host1x_syncpoint_info { > /** > * @id: > * > * [out] System-global ID of the syncpoint. > */ > __u32 id; > > __u32 reserved[3]; > }; > ``` > > ### IOCTL HOST1X_SYNCPOINT_INCREMENT (on syncpoint file descriptor) > > Allows incrementing of the syncpoint value. > > Use cases: > > * Signalling work completion when executing a pipeline step on the CPU > * Debugging and testing > > ``` > struct host1x_syncpoint_increment { > /** > * @count: > * > * [in] Number of times to increment syncpoint. Value can be > * observed in in-between values, but increments are atomic. > */ > __u32 count; > }; > ``` > > ### IOCTL HOST1X_READ_SYNCPOINT (on /dev/host1x) > > Read the value of a syncpoint based on its ID. > > Use cases: > > * Allows more fine-grained tracking of task progression for debugging > purposes > > ``` > struct host1x_ctrl_read_syncpoint { > /** > * @id: > * > * [in] ID of syncpoint to read. > */ > __u32 id; > > /** > * @value: > * > * [out] Value of the syncpoint. > */ > __u32 value; > }; > ``` > > ### IOCTL HOST1X_CREATE_FENCE (on /dev/host1x) > > Creates a new SYNC_FILE fence file descriptor for the specified > syncpoint ID and threshold. > > Use cases:
[PULL] drm-misc-next
drm-misc-next-2020-06-26: drm-misc-next for v5.9: Cross-subsystem Changes: - Improve dma-buf docs. Core Changes: - Add NV15, Q410, Q401 yuv formats. - Add uncompressed AFBC modifier. - Add DP helepr for reading Ignore MSA from DPCD. - Add missing panel type for some panels - Optimize drm/mm hole handling. - Constify connector to infoframe functions. - Add debugfs for VRR monitor range. Driver Changes: - Assorted small bugfixes in panfrost, malidp, panel/otm8009a. - Convert tfp410 dt bindings to yaml, and rework time calculations. - Add support for a few more simple panels. - Cleanups and optimizations for ast. - Allow adv7511 and simple-bridge to be used without connector creation. - Cleanups to dw-hdmi function prototypes. - Remove enabled bool from tiny/repaper and mipi-dbi, atomic handles it. - Remove unused header file from dw-mipi-dsi - Begin removing ttm_bo->offset. The following changes since commit 114427b8927a4def2942b2b886f7e4aeae289ccb: drm/panfrost: Use kvfree() to free bo->sgts (2020-06-19 11:00:02 +0100) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2020-06-26 for you to fetch changes up to 41752663b410c6265e24ff0570350b0b05ecdafe: drm/debug: Expose connector VRR monitor range via debugfs (2020-06-25 15:47:14 -0700) drm-misc-next for v5.9: Cross-subsystem Changes: - Improve dma-buf docs. Core Changes: - Add NV15, Q410, Q401 yuv formats. - Add uncompressed AFBC modifier. - Add DP helepr for reading Ignore MSA from DPCD. - Add missing panel type for some panels - Optimize drm/mm hole handling. - Constify connector to infoframe functions. - Add debugfs for VRR monitor range. Driver Changes: - Assorted small bugfixes in panfrost, malidp, panel/otm8009a. - Convert tfp410 dt bindings to yaml, and rework time calculations. - Add support for a few more simple panels. - Cleanups and optimizations for ast. - Allow adv7511 and simple-bridge to be used without connector creation. - Cleanups to dw-hdmi function prototypes. - Remove enabled bool from tiny/repaper and mipi-dbi, atomic handles it. - Remove unused header file from dw-mipi-dsi - Begin removing ttm_bo->offset. Angelo Ribeiro (1): drm/bridge: dw-mipi-dsi.c: remove unused header file Ben Davis (2): drm: drm_fourcc: add NV15, Q410, Q401 YUV formats drm: drm_fourcc: Add uncompressed AFBC modifier Bhanuprakash Modem (1): drm/debug: Expose connector VRR monitor range via debugfs Christian König (3): drm/mm: remove unused rb_hole_size() drm/mm: optimize find_hole() as well drm/mm: cleanup and improve next_hole_*_addr() Colin Ian King (1): drm/arm: fix unintentional integer overflow on left shift Daniel Vetter (3): drm/tiny/repaper: Drop edp->enabled drm/mipi-dbi: Remove ->enabled dma-buf: minor doc touch-ups Dmitry Osipenko (1): drm/panel-simple: Add missing connector type for some panels Laurent Pinchart (21): drm: bridge: adv7511: Split EDID read to a separate function drm: bridge: adv7511: Split connector creation to a separate function drm: bridge: adv7511: Implement bridge connector operations drm: bridge: adv7511: Make connector creation optional drm: bridge: Return NULL on error from drm_bridge_get_edid() drm: bridge: simple-bridge: Delegate operations to next bridge drm: bridge: simple-bridge: Make connector creation optional drm: edid: Constify connector argument to infoframe functions drm: bridge: Pass drm_display_info to drm_bridge_funcs .mode_valid() drm: bridge: dw-hdmi: Pass private data pointer to .mode_valid() drm: bridge: dw-hdmi: Pass private data pointer to .configure_phy() drm: bridge: dw-hdmi: Remove unused field from dw_hdmi_plat_data drm: meson: dw-hdmi: Use dw_hdmi context to replace hack drm: bridge: dw-hdmi: Pass drm_display_info to .mode_valid() drm: bridge: dw-hdmi: Constify mode argument to dw_hdmi_phy_ops .init() drm: bridge: dw-hdmi: Constify mode argument to internal functions drm: bridge: dw-hdmi: Pass drm_display_info to dw_hdmi_support_scdc() drm: bridge: dw-hdmi: Split connector creation to a separate function drm: bridge: dw-hdmi: Store current connector in struct dw_hdmi drm: bridge: dw-hdmi: Pass drm_connector to internal functions as needed drm: bridge: dw-hdmi: Make connector creation optional Manasi Navare (1): drm/dp: DRM DP helper for reading Ignore MSA from DPCD Matthias Schiffer (2): dt-bindings: display: simple: add CDTech S070PWS19HP-FC21 and S070SWV29HG-DC44 dt-bindings: display: simple: add Tianma TM070JVHG33 Max Merchel (1): drm/panel: simple: add Tianma TM070JVHG33 Michael Krummsdorf (1): drm/panel: simple: add CDTech S070PWS19HP-FC21 and S070SWV29HG-DC44 Nirmoy
[PATCH] drm/vmwgfx: Fix two list_for_each loop exit tests
These if statements are supposed to be true if we ended the list_for_each_entry() loops without hitting a break statement but they don't work. In the first loop, we increment "i" after the "if (i == unit)" condition so we don't necessarily know that "i" is not equal to unit at the end of the loop. In the second loop we exit when mode is not pointing to a valid drm_display_mode struct so it doesn't make sense to check "mode->type". Fixes: a278724aa23c ("drm/vmwgfx: Implement fbdev on kms v2") Signed-off-by: Dan Carpenter --- I reversed the second condition as well, just because I was copy and pasting the exit condition. Plus I always feel like error handling is better than success handling. If anyone feel strongly, then I can send a v2. drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 3c97654b5a43..44168a7d7b44 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -2576,7 +2576,7 @@ int vmw_kms_fbdev_init_data(struct vmw_private *dev_priv, ++i; } - if (i != unit) { + if (&con->head == &dev_priv->dev->mode_config.connector_list) { DRM_ERROR("Could not find initial display unit.\n"); ret = -EINVAL; goto out_unlock; @@ -2600,13 +2600,13 @@ int vmw_kms_fbdev_init_data(struct vmw_private *dev_priv, break; } - if (mode->type & DRM_MODE_TYPE_PREFERRED) - *p_mode = mode; - else { + if (&mode->head == &con->modes) { WARN_ONCE(true, "Could not find initial preferred mode.\n"); *p_mode = list_first_entry(&con->modes, struct drm_display_mode, head); + } else { + *p_mode = mode; } out_unlock: -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vmwgfx: Use correct vmw_legacy_display_unit pointer
The "entry" pointer is an offset from the list head and it doesn't point to a valid vmw_legacy_display_unit struct. Presumably the intent was to point to the last entry. Also the "i++" wasn't used so I have removed that as well. Fixes: d7e1958dbe4a ("drm/vmwgfx: Support older hardware.") Signed-off-by: Dan Carpenter --- >From static analysis. Not tested. This bug celebrated its tenth birthday last month. :) drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c index 16dafff5cab1..009f1742bed5 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c @@ -81,7 +81,7 @@ static int vmw_ldu_commit_list(struct vmw_private *dev_priv) struct vmw_legacy_display_unit *entry; struct drm_framebuffer *fb = NULL; struct drm_crtc *crtc = NULL; - int i = 0; + int i; /* If there is no display topology the host just assumes * that the guest will set the same layout as the host. @@ -92,12 +92,11 @@ static int vmw_ldu_commit_list(struct vmw_private *dev_priv) crtc = &entry->base.crtc; w = max(w, crtc->x + crtc->mode.hdisplay); h = max(h, crtc->y + crtc->mode.vdisplay); - i++; } if (crtc == NULL) return 0; - fb = entry->base.crtc.primary->state->fb; + fb = crtc->primary->state->fb; return vmw_kms_write_svga(dev_priv, w, h, fb->pitches[0], fb->format->cpp[0] * 8, -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: call ->post_disable() functions in vc4_dsi_encoder_disable()
On Fri, 26 Jun 2020 13:34:01 +0300 Dan Carpenter wrote: > The problem is that the iterator is already at the list head so the > list_for_each_entry_from() loop is a no-op and we don't call the > the iter->funcs->post_disable() functions. This should be > list_for_each_entry() instead. > > Fixes: 033bfe7538a1 ("drm/vc4: dsi: Fix bridge chain handling") > Signed-off-by: Dan Carpenter Reviewed-by: Boris Brezillon > --- > drivers/gpu/drm/vc4/vc4_dsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c > index eaf276978ee7..f92d0e92fa72 100644 > --- a/drivers/gpu/drm/vc4/vc4_dsi.c > +++ b/drivers/gpu/drm/vc4/vc4_dsi.c > @@ -754,7 +754,7 @@ static void vc4_dsi_encoder_disable(struct drm_encoder > *encoder) > > vc4_dsi_ulps(dsi, true); > > - list_for_each_entry_from(iter, &dsi->bridge_chain, chain_node) { > + list_for_each_entry(iter, &dsi->bridge_chain, chain_node) { > if (iter->funcs->post_disable) > iter->funcs->post_disable(iter); > } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vc4: call ->post_disable() functions in vc4_dsi_encoder_disable()
The problem is that the iterator is already at the list head so the list_for_each_entry_from() loop is a no-op and we don't call the the iter->funcs->post_disable() functions. This should be list_for_each_entry() instead. Fixes: 033bfe7538a1 ("drm/vc4: dsi: Fix bridge chain handling") Signed-off-by: Dan Carpenter --- drivers/gpu/drm/vc4/vc4_dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index eaf276978ee7..f92d0e92fa72 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -754,7 +754,7 @@ static void vc4_dsi_encoder_disable(struct drm_encoder *encoder) vc4_dsi_ulps(dsi, true); - list_for_each_entry_from(iter, &dsi->bridge_chain, chain_node) { + list_for_each_entry(iter, &dsi->bridge_chain, chain_node) { if (iter->funcs->post_disable) iter->funcs->post_disable(iter); } -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] backlight: Delete the OT200 backlight driver
This driver has no in-kernel users. The device can only be populated by board files since it does not support device tree nor ACPI, and nothing in the kernel creates a device named "ot200-backlight". This driver has been in the kernel since 2012. If it is used by out-of-tree code that code should have been upstreamed by now, it's been 8 years. It uses the idiomatic forked GPIO of the CS5535 which combines pin control and GPIO into its private custom interface, which causes me a headache because that is not how we do things these days: we creates separate pin control and GPIO drivers. Delete this unused driver. Cc: Christian Gmeiner Signed-off-by: Linus Walleij --- drivers/video/backlight/Kconfig| 7 -- drivers/video/backlight/Makefile | 1 - drivers/video/backlight/ot200_bl.c | 162 - 3 files changed, 170 deletions(-) delete mode 100644 drivers/video/backlight/ot200_bl.c diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig index 7d22d7377606..95c546cc8774 100644 --- a/drivers/video/backlight/Kconfig +++ b/drivers/video/backlight/Kconfig @@ -386,13 +386,6 @@ config BACKLIGHT_LP8788 help This supports TI LP8788 backlight driver. -config BACKLIGHT_OT200 - tristate "Backlight driver for ot200 visualisation device" - depends on CS5535_MFGPT && GPIO_CS5535 - help - To compile this driver as a module, choose M here: the module will be - called ot200_bl. - config BACKLIGHT_PANDORA tristate "Backlight driver for Pandora console" depends on TWL4030_CORE diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile index 0c1a1524627a..2072d21b60f7 100644 --- a/drivers/video/backlight/Makefile +++ b/drivers/video/backlight/Makefile @@ -45,7 +45,6 @@ obj-$(CONFIG_BACKLIGHT_LP8788)+= lp8788_bl.o obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o obj-$(CONFIG_BACKLIGHT_MAX8925)+= max8925_bl.o obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o -obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o obj-$(CONFIG_BACKLIGHT_PANDORA)+= pandora_bl.o obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o obj-$(CONFIG_BACKLIGHT_PWM)+= pwm_bl.o diff --git a/drivers/video/backlight/ot200_bl.c b/drivers/video/backlight/ot200_bl.c deleted file mode 100644 index 23ee7106c72a.. --- a/drivers/video/backlight/ot200_bl.c +++ /dev/null @@ -1,162 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * Copyright (C) 2012 Bachmann electronic GmbH - * Christian Gmeiner - * - * Backlight driver for ot200 visualisation device from - * Bachmann electronic GmbH. - */ - -#include -#include -#include -#include -#include -#include - -static struct cs5535_mfgpt_timer *pwm_timer; - -/* this array defines the mapping of brightness in % to pwm frequency */ -static const u8 dim_table[101] = {0, 0, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, - 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 4, 4, 4, 4, - 4, 5, 5, 5, 5, 6, 6, 6, 7, 7, 7, 8, 8, 9, 9, - 10, 10, 11, 11, 12, 12, 13, 14, 15, 15, 16, - 17, 18, 19, 20, 21, 22, 23, 24, 26, 27, 28, - 30, 31, 33, 35, 37, 39, 41, 43, 45, 47, 50, - 53, 55, 58, 61, 65, 68, 72, 75, 79, 84, 88, - 93, 97, 103, 108, 114, 120, 126, 133, 140, - 147, 155, 163}; - -struct ot200_backlight_data { - int current_brightness; -}; - -#define GPIO_DIMM 27 -#define SCALE 1 -#define CMP1MODE 0x2 /* compare on GE; output high on compare -* greater than or equal */ -#define PWM_SETUP (SCALE | CMP1MODE << 6 | MFGPT_SETUP_CNTEN) -#define MAX_COMP2 163 - -static int ot200_backlight_update_status(struct backlight_device *bl) -{ - struct ot200_backlight_data *data = bl_get_data(bl); - int brightness = bl->props.brightness; - - if (bl->props.state & BL_CORE_FBBLANK) - brightness = 0; - - /* enable or disable PWM timer */ - if (brightness == 0) - cs5535_mfgpt_write(pwm_timer, MFGPT_REG_SETUP, 0); - else if (data->current_brightness == 0) { - cs5535_mfgpt_write(pwm_timer, MFGPT_REG_COUNTER, 0); - cs5535_mfgpt_write(pwm_timer, MFGPT_REG_SETUP, - MFGPT_SETUP_CNTEN); - } - - /* apply new brightness value */ - cs5535_mfgpt_write(pwm_timer, MFGPT_REG_CMP1, - MAX_COMP2 - dim_table[brightness]); - data->current_brightness = brightness; - - return 0; -} - -static int ot200_backlight_get_brightness(struct backlight_device *bl) -{ - struct ot200_backlight_data *data = bl_get_data(bl); - return data->current_brightness; -} - -st
[PATCH v6 0/4] driver core: add probe error check helper
Hi All, Thanks for multiple comments. Changes since v5: - removed patch adding macro, dev_err_probe(dev, PTR_ERR(ptr), ...) should be used instead, - added dev_dbg logging in case of -EPROBE_DEFER, - renamed functions and vars according to comments, - extended docs, - cosmetics. Original message (with small adjustments): Recently I took some time to re-check error handling in drivers probe code, and I have noticed that number of incorrect resource acquisition error handling increased and there are no other propositions which can cure the situation. So I have decided to resend my old proposition of probe_err helper which should simplify resource acquisition error handling, it also extend it with adding defer probe reason to devices_deferred debugfs property, which should improve debugging experience for developers/testers. I have also added two patches showing usage and benefits of the helper. My dirty/ad-hoc cocci scripts shows that this helper can be used in at least 2700 places saving about 3500 lines of code. Regards Andrzej Andrzej Hajda (4): driver core: add device probe log helper driver core: add deferring probe reason to devices_deferred property drm/bridge/sii8620: fix resource acquisition error handling drm/bridge: lvds-codec: simplify error handling drivers/base/base.h | 3 ++ drivers/base/core.c | 46 drivers/base/dd.c| 23 +- drivers/gpu/drm/bridge/lvds-codec.c | 10 ++ drivers/gpu/drm/bridge/sil-sii8620.c | 21 ++--- include/linux/device.h | 3 ++ 6 files changed, 86 insertions(+), 20 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 3/4] drm/bridge/sii8620: fix resource acquisition error handling
In case of error during resource acquisition driver should print error message only in case it is not deferred probe, using dev_err_probe helper solves the issue. Moreover it records defer probe reason for debugging. Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/bridge/sil-sii8620.c | 21 + 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index 92acd336aa89..389c1f029774 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -2299,10 +2299,9 @@ static int sii8620_probe(struct i2c_client *client, INIT_LIST_HEAD(&ctx->mt_queue); ctx->clk_xtal = devm_clk_get(dev, "xtal"); - if (IS_ERR(ctx->clk_xtal)) { - dev_err(dev, "failed to get xtal clock from DT\n"); - return PTR_ERR(ctx->clk_xtal); - } + if (IS_ERR(ctx->clk_xtal)) + return dev_err_probe(dev, PTR_ERR(ctx->clk_xtal), +"failed to get xtal clock from DT\n"); if (!client->irq) { dev_err(dev, "no irq provided\n"); @@ -2313,16 +2312,14 @@ static int sii8620_probe(struct i2c_client *client, sii8620_irq_thread, IRQF_TRIGGER_HIGH | IRQF_ONESHOT, "sii8620", ctx); - if (ret < 0) { - dev_err(dev, "failed to install IRQ handler\n"); - return ret; - } + if (ret < 0) + return dev_err_probe(dev, ret, +"failed to install IRQ handler\n"); ctx->gpio_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); - if (IS_ERR(ctx->gpio_reset)) { - dev_err(dev, "failed to get reset gpio from DT\n"); - return PTR_ERR(ctx->gpio_reset); - } + if (IS_ERR(ctx->gpio_reset)) + return dev_err_probe(dev, PTR_ERR(ctx->gpio_reset), +"failed to get reset gpio from DT\n"); ctx->supplies[0].supply = "cvcc10"; ctx->supplies[1].supply = "iovcc18"; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 1/4] driver core: add device probe log helper
During probe every time driver gets resource it should usually check for error printk some message if it is not -EPROBE_DEFER and return the error. This pattern is simple but requires adding few lines after any resource acquisition code, as a result it is often omitted or implemented only partially. dev_err_probe helps to replace such code sequences with simple call, so code: if (err != -EPROBE_DEFER) dev_err(dev, ...); return err; becomes: return probe_err(dev, err, ...); Signed-off-by: Andrzej Hajda --- drivers/base/core.c| 42 ++ include/linux/device.h | 3 +++ 2 files changed, 45 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index 67d39a90b45c..3a827c82933f 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -3953,6 +3953,48 @@ define_dev_printk_level(_dev_info, KERN_INFO); #endif +/** + * dev_err_probe - probe error check and log helper + * @dev: the pointer to the struct device + * @err: error value to test + * @fmt: printf-style format string + * @...: arguments as specified in the format string + * + * This helper implements common pattern present in probe functions for error + * checking: print debug or error message depending if the error value is + * -EPROBE_DEFER and propagate error upwards. + * It replaces code sequence: + * if (err != -EPROBE_DEFER) + * dev_err(dev, ...); + * else + * dev_dbg(dev, ...); + * return err; + * with + * return dev_err_probe(dev, err, ...); + * + * Returns @err. + * + */ +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...) +{ + struct va_format vaf; + va_list args; + + va_start(args, fmt); + vaf.fmt = fmt; + vaf.va = &args; + + if (err != -EPROBE_DEFER) + dev_err(dev, "error %d: %pV", err, &vaf); + else + dev_dbg(dev, "error %d: %pV", err, &vaf); + + va_end(args); + + return err; +} +EXPORT_SYMBOL_GPL(dev_err_probe); + static inline bool fwnode_is_primary(struct fwnode_handle *fwnode) { return fwnode && !IS_ERR(fwnode->secondary); diff --git a/include/linux/device.h b/include/linux/device.h index 15460a5ac024..6b2272ae9af8 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -964,6 +964,9 @@ void device_link_remove(void *consumer, struct device *supplier); void device_links_supplier_sync_state_pause(void); void device_links_supplier_sync_state_resume(void); +extern __printf(3, 4) +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...); + /* Create alias, so I can be autoloaded. */ #define MODULE_ALIAS_CHARDEV(major,minor) \ MODULE_ALIAS("char-major-" __stringify(major) "-" __stringify(minor)) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 4/4] drm/bridge: lvds-codec: simplify error handling
Using dev_err_probe code has following advantages: - shorter code, - recorded defer probe reason for debugging, - uniform error code logging. Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/bridge/lvds-codec.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/bridge/lvds-codec.c b/drivers/gpu/drm/bridge/lvds-codec.c index 24fb1befdfa2..f19d9f7a5db2 100644 --- a/drivers/gpu/drm/bridge/lvds-codec.c +++ b/drivers/gpu/drm/bridge/lvds-codec.c @@ -71,13 +71,9 @@ static int lvds_codec_probe(struct platform_device *pdev) lvds_codec->connector_type = (uintptr_t)of_device_get_match_data(dev); lvds_codec->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown", GPIOD_OUT_HIGH); - if (IS_ERR(lvds_codec->powerdown_gpio)) { - int err = PTR_ERR(lvds_codec->powerdown_gpio); - - if (err != -EPROBE_DEFER) - dev_err(dev, "powerdown GPIO failure: %d\n", err); - return err; - } + if (IS_ERR(lvds_codec->powerdown_gpio)) + return dev_err_probe(dev, PTR_ERR(lvds_codec->powerdown_gpio), +"powerdown GPIO failure\n"); /* Locate the panel DT node. */ panel_node = of_graph_get_remote_node(dev->of_node, 1, 0); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 2/4] driver core: add deferring probe reason to devices_deferred property
/sys/kernel/debug/devices_deferred property contains list of deferred devices. This list does not contain reason why the driver deferred probe, the patch improves it. The natural place to set the reason is probe_err function introduced recently, ie. if probe_err will be called with -EPROBE_DEFER instead of printk the message will be attached to deferred device and printed when user read devices_deferred property. Signed-off-by: Andrzej Hajda Reviewed-by: Mark Brown Reviewed-by: Javier Martinez Canillas Reviewed-by: Andy Shevchenko --- drivers/base/base.h | 3 +++ drivers/base/core.c | 8 ++-- drivers/base/dd.c | 23 ++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index 95c22c0f9036..6954fccab3d7 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -93,6 +93,7 @@ struct device_private { struct klist_node knode_class; struct list_head deferred_probe; struct device_driver *async_driver; + char *deferred_probe_reason; struct device *device; u8 dead:1; }; @@ -134,6 +135,8 @@ extern void device_release_driver_internal(struct device *dev, extern void driver_detach(struct device_driver *drv); extern int driver_probe_device(struct device_driver *drv, struct device *dev); extern void driver_deferred_probe_del(struct device *dev); +extern void device_set_deferred_probe_reson(const struct device *dev, + struct va_format *vaf); static inline int driver_match_device(struct device_driver *drv, struct device *dev) { diff --git a/drivers/base/core.c b/drivers/base/core.c index 3a827c82933f..fee047f03681 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -3963,6 +3963,8 @@ define_dev_printk_level(_dev_info, KERN_INFO); * This helper implements common pattern present in probe functions for error * checking: print debug or error message depending if the error value is * -EPROBE_DEFER and propagate error upwards. + * In case of -EPROBE_DEFER it sets also defer probe reason, which can be + * checked later by reading devices_deferred debugfs attribute. * It replaces code sequence: * if (err != -EPROBE_DEFER) * dev_err(dev, ...); @@ -3984,10 +3986,12 @@ int dev_err_probe(const struct device *dev, int err, const char *fmt, ...) vaf.fmt = fmt; vaf.va = &args; - if (err != -EPROBE_DEFER) + if (err != -EPROBE_DEFER) { dev_err(dev, "error %d: %pV", err, &vaf); - else + } else { + device_set_deferred_probe_reson(dev, &vaf); dev_dbg(dev, "error %d: %pV", err, &vaf); + } va_end(args); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 9a1d940342ac..dd5683b61f74 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -27,6 +27,7 @@ #include #include #include +#include #include "base.h" #include "power/power.h" @@ -136,6 +137,8 @@ void driver_deferred_probe_del(struct device *dev) if (!list_empty(&dev->p->deferred_probe)) { dev_dbg(dev, "Removed from deferred list\n"); list_del_init(&dev->p->deferred_probe); + kfree(dev->p->deferred_probe_reason); + dev->p->deferred_probe_reason = NULL; } mutex_unlock(&deferred_probe_mutex); } @@ -211,6 +214,23 @@ void device_unblock_probing(void) driver_deferred_probe_trigger(); } +/** + * device_set_deferred_probe_reson() - Set defer probe reason message for device + * @dev: the pointer to the struct device + * @vaf: the pointer to va_format structure with message + */ +void device_set_deferred_probe_reson(const struct device *dev, struct va_format *vaf) +{ + const char *drv = dev_driver_string(dev); + + mutex_lock(&deferred_probe_mutex); + + kfree(dev->p->deferred_probe_reason); + dev->p->deferred_probe_reason = kasprintf(GFP_KERNEL, "%s: %pV", drv, vaf); + + mutex_unlock(&deferred_probe_mutex); +} + /* * deferred_devs_show() - Show the devices in the deferred probe pending list. */ @@ -221,7 +241,8 @@ static int deferred_devs_show(struct seq_file *s, void *data) mutex_lock(&deferred_probe_mutex); list_for_each_entry(curr, &deferred_probe_pending_list, deferred_probe) - seq_printf(s, "%s\n", dev_name(curr->device)); + seq_printf(s, "%s\t%s", dev_name(curr->device), + curr->device->p->deferred_probe_reason ?: "\n"); mutex_unlock(&deferred_probe_mutex); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args
On Thu, Jun 25, 2020 at 11:33:34AM +0100, Lee Jones wrote: > On Thu, 25 Jun 2020, Daniel Thompson wrote: > > > On Wed, Jun 24, 2020 at 03:57:16PM +0100, Lee Jones wrote: > > > Kerneldoc syntax is used, but not complete. Descriptions required. > > > > > > Prevents warnings like: > > > > > > drivers/video/backlight/ili922x.c:116: warning: Function parameter or > > > member 's' not described in 'CHECK_FREQ_REG' > > > drivers/video/backlight/ili922x.c:116: warning: Function parameter or > > > member 'x' not described in 'CHECK_FREQ_REG' > > > > > > Cc: > > > Cc: Bartlomiej Zolnierkiewicz > > > Cc: Software Engineering > > > Signed-off-by: Lee Jones > > > --- > > > drivers/video/backlight/ili922x.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/video/backlight/ili922x.c > > > b/drivers/video/backlight/ili922x.c > > > index 9c5aa3fbb2842..8cb4b9d3c3bba 100644 > > > --- a/drivers/video/backlight/ili922x.c > > > +++ b/drivers/video/backlight/ili922x.c > > > @@ -107,6 +107,8 @@ > > > * lower frequency when the registers are read/written. > > > * The macro sets the frequency in the spi_transfer structure if > > > * the frequency exceeds the maximum value. > > > + * @s: pointer to controller side proxy for an SPI slave device > > > > What's wrong with "a pointer to an SPI device"? > > > > I am aware, having looked it up to find out what the above actually > > means, that this is how struct spi_device is described in its own kernel > > doc but quoting at that level of detail of both overkill and confusing. > > I figured that using the official description would be better than > making something up. However if you think it's better to KISS, then I > can change it. Yes, I'd strongly prefer KISS here. I know it is an "I am the world" argument[1] but I found using such a dogmatically accurate description out of context to be very confusing and therefore I don't think such a comment improves readability. Daniel. [1]: See #3 from http://www.leany.com/logic/Adams.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: lms501kf03: Drop unused include
On Fri, Jun 26, 2020 at 01:25:12AM +0200, Linus Walleij wrote: > This driver includes but does not use any > symbols from that file, drop the include. > > Signed-off-by: Linus Walleij Reviewed-by: Daniel Thompson > --- > drivers/video/backlight/lms501kf03.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/video/backlight/lms501kf03.c > b/drivers/video/backlight/lms501kf03.c > index 8ae32e3573c1..52d3ee6c3f7f 100644 > --- a/drivers/video/backlight/lms501kf03.c > +++ b/drivers/video/backlight/lms501kf03.c > @@ -9,7 +9,6 @@ > #include > #include > #include > -#include > #include > #include > #include > -- > 2.25.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] gpu: ipu-v3: image-convert: Wait for all EOFs before completing a tile
Hi Steve, On Thu, 2020-06-25 at 11:13 -0700, Steve Longerbeam wrote: > Use a bit-mask of EOF irqs to determine when all required idmac > channel EOFs have been received for a tile conversion, and only do > tile completion processing after all EOFs have been received. Otherwise > it was found that a conversion would stall after the completion of a > tile and the start of the next tile, because the input/read idmac > channel had not completed and entered idle state, thus locking up the > channel when attempting to re-start it for the next tile. Do I understand correctly that there are cases where the output channel EOF IRQ has triggered and the next tile processing is kicked off before the input channel EOF IRQ triggers even without rotation? Do you have any way to reproduce this? regards Philipp > Fixes: 0537db801bb01 ("gpu: ipu-v3: image-convert: reconfigure IC per tile") > Signed-off-by: Steve Longerbeam > --- > Changes in v2: > - need to clear eof_mask at completion of every tile, not just in > convert_start(). > --- > drivers/gpu/ipu-v3/ipu-image-convert.c | 109 +++-- > 1 file changed, 82 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/ipu-v3/ipu-image-convert.c > b/drivers/gpu/ipu-v3/ipu-image-convert.c > index f8b031ded3cf..aa1d4b6d278f 100644 > --- a/drivers/gpu/ipu-v3/ipu-image-convert.c > +++ b/drivers/gpu/ipu-v3/ipu-image-convert.c > @@ -137,6 +137,17 @@ struct ipu_image_convert_ctx; > struct ipu_image_convert_chan; > struct ipu_image_convert_priv; > > +enum eof_irq_mask { > + EOF_IRQ_IN = BIT(0), > + EOF_IRQ_ROT_IN = BIT(1), > + EOF_IRQ_OUT = BIT(2), > + EOF_IRQ_ROT_OUT = BIT(3), > +}; > + > +#define EOF_IRQ_COMPLETE (EOF_IRQ_IN | EOF_IRQ_OUT) > +#define EOF_IRQ_ROT_COMPLETE (EOF_IRQ_IN | EOF_IRQ_OUT | \ > + EOF_IRQ_ROT_IN | EOF_IRQ_ROT_OUT) > + > struct ipu_image_convert_ctx { > struct ipu_image_convert_chan *chan; > > @@ -173,6 +184,9 @@ struct ipu_image_convert_ctx { > /* where to place converted tile in dest image */ > unsigned int out_tile_map[MAX_TILES]; > > + /* mask of completed EOF irqs at every tile conversion */ > + enum eof_irq_mask eof_mask; > + > struct list_head list; > }; > > @@ -189,6 +203,8 @@ struct ipu_image_convert_chan { > struct ipuv3_channel *rotation_out_chan; > > /* the IPU end-of-frame irqs */ > + int in_eof_irq; > + int rot_in_eof_irq; > int out_eof_irq; > int rot_out_eof_irq; > > @@ -1380,6 +1396,9 @@ static int convert_start(struct ipu_image_convert_run > *run, unsigned int tile) > dev_dbg(priv->ipu->dev, "%s: task %u: starting ctx %p run %p tile %u -> > %u\n", > __func__, chan->ic_task, ctx, run, tile, dst_tile); > > + /* clear EOF irq mask */ > + ctx->eof_mask = 0; > + > if (ipu_rot_mode_is_irt(ctx->rot_mode)) { > /* swap width/height for resizer */ > dest_width = d_image->tile[dst_tile].height; > @@ -1615,7 +1634,7 @@ static bool ic_settings_changed(struct > ipu_image_convert_ctx *ctx) > } > > /* hold irqlock when calling */ > -static irqreturn_t do_irq(struct ipu_image_convert_run *run) > +static irqreturn_t do_tile_complete(struct ipu_image_convert_run *run) > { > struct ipu_image_convert_ctx *ctx = run->ctx; > struct ipu_image_convert_chan *chan = ctx->chan; > @@ -1700,6 +1719,7 @@ static irqreturn_t do_irq(struct ipu_image_convert_run > *run) > ctx->cur_buf_num ^= 1; > } > > + ctx->eof_mask = 0; /* clear EOF irq mask for next tile */ > ctx->next_tile++; > return IRQ_HANDLED; > done: > @@ -1715,8 +1735,9 @@ static irqreturn_t eof_irq(int irq, void *data) > struct ipu_image_convert_priv *priv = chan->priv; > struct ipu_image_convert_ctx *ctx; > struct ipu_image_convert_run *run; > + irqreturn_t ret = IRQ_HANDLED; > + bool tile_complete = false; > unsigned long flags; > - irqreturn_t ret; > > spin_lock_irqsave(&chan->irqlock, flags); > > @@ -1729,27 +1750,33 @@ static irqreturn_t eof_irq(int irq, void *data) > > ctx = run->ctx; > > - if (irq == chan->out_eof_irq) { > - if (ipu_rot_mode_is_irt(ctx->rot_mode)) { > - /* this is a rotation op, just ignore */ > - ret = IRQ_HANDLED; > - goto out; > - } > - } else if (irq == chan->rot_out_eof_irq) { > + if (irq == chan->in_eof_irq) { > + ctx->eof_mask |= EOF_IRQ_IN; > + } else if (irq == chan->out_eof_irq) { > + ctx->eof_mask |= EOF_IRQ_OUT; > + } else if (irq == chan->rot_in_eof_irq || > +irq == chan->rot_out_eof_irq) { > if (!ipu_rot_mode_is_irt(ctx->rot_mode)) { > /* this was NOT a rotation op, shouldn't happen */ > dev_err(priv->ipu->dev, >
Re: [PATCH 2/3] gpu: ipu-v3: image-convert: Combine rotate/no-rotate irq handlers
On Wed, 2020-06-17 at 15:40 -0700, Steve Longerbeam wrote: > Combine the rotate_irq() and norotate_irq() handlers into a single > eof_irq() handler. > > Signed-off-by: Steve Longerbeam Reviewed-by: Philipp Zabel regards Philipp > --- > drivers/gpu/ipu-v3/ipu-image-convert.c | 58 +- > 1 file changed, 20 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/ipu-v3/ipu-image-convert.c > b/drivers/gpu/ipu-v3/ipu-image-convert.c > index eeca50d9a1ee..f8b031ded3cf 100644 > --- a/drivers/gpu/ipu-v3/ipu-image-convert.c > +++ b/drivers/gpu/ipu-v3/ipu-image-convert.c > @@ -1709,9 +1709,10 @@ static irqreturn_t do_irq(struct ipu_image_convert_run > *run) > return IRQ_WAKE_THREAD; > } > > -static irqreturn_t norotate_irq(int irq, void *data) > +static irqreturn_t eof_irq(int irq, void *data) > { > struct ipu_image_convert_chan *chan = data; > + struct ipu_image_convert_priv *priv = chan->priv; > struct ipu_image_convert_ctx *ctx; > struct ipu_image_convert_run *run; > unsigned long flags; > @@ -1728,45 +1729,26 @@ static irqreturn_t norotate_irq(int irq, void *data) > > ctx = run->ctx; > > - if (ipu_rot_mode_is_irt(ctx->rot_mode)) { > - /* this is a rotation operation, just ignore */ > - spin_unlock_irqrestore(&chan->irqlock, flags); > - return IRQ_HANDLED; > - } > - > - ret = do_irq(run); > -out: > - spin_unlock_irqrestore(&chan->irqlock, flags); > - return ret; > -} > - > -static irqreturn_t rotate_irq(int irq, void *data) > -{ > - struct ipu_image_convert_chan *chan = data; > - struct ipu_image_convert_priv *priv = chan->priv; > - struct ipu_image_convert_ctx *ctx; > - struct ipu_image_convert_run *run; > - unsigned long flags; > - irqreturn_t ret; > - > - spin_lock_irqsave(&chan->irqlock, flags); > - > - /* get current run and its context */ > - run = chan->current_run; > - if (!run) { > + if (irq == chan->out_eof_irq) { > + if (ipu_rot_mode_is_irt(ctx->rot_mode)) { > + /* this is a rotation op, just ignore */ > + ret = IRQ_HANDLED; > + goto out; > + } > + } else if (irq == chan->rot_out_eof_irq) { > + if (!ipu_rot_mode_is_irt(ctx->rot_mode)) { > + /* this was NOT a rotation op, shouldn't happen */ > + dev_err(priv->ipu->dev, > + "Unexpected rotation interrupt\n"); > + ret = IRQ_HANDLED; > + goto out; > + } > + } else { > + dev_err(priv->ipu->dev, "Received unknown irq %d\n", irq); > ret = IRQ_NONE; > goto out; > } > > - ctx = run->ctx; > - > - if (!ipu_rot_mode_is_irt(ctx->rot_mode)) { > - /* this was NOT a rotation operation, shouldn't happen */ > - dev_err(priv->ipu->dev, "Unexpected rotation interrupt\n"); > - spin_unlock_irqrestore(&chan->irqlock, flags); > - return IRQ_HANDLED; > - } > - > ret = do_irq(run); > out: > spin_unlock_irqrestore(&chan->irqlock, flags); > @@ -1859,7 +1841,7 @@ static int get_ipu_resources(struct > ipu_image_convert_chan *chan) > chan->out_chan, > IPU_IRQ_EOF); > > - ret = request_threaded_irq(chan->out_eof_irq, norotate_irq, do_bh, > + ret = request_threaded_irq(chan->out_eof_irq, eof_irq, do_bh, > 0, "ipu-ic", chan); > if (ret < 0) { > dev_err(priv->ipu->dev, "could not acquire irq %d\n", > @@ -1872,7 +1854,7 @@ static int get_ipu_resources(struct > ipu_image_convert_chan *chan) >chan->rotation_out_chan, >IPU_IRQ_EOF); > > - ret = request_threaded_irq(chan->rot_out_eof_irq, rotate_irq, do_bh, > + ret = request_threaded_irq(chan->rot_out_eof_irq, eof_irq, do_bh, > 0, "ipu-ic", chan); > if (ret < 0) { > dev_err(priv->ipu->dev, "could not acquire irq %d\n", ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 27/27] drm: Add default modes for connectors in unknown state
Hi, On Fri, 26 Jun 2020 at 10:00, Pekka Paalanen wrote: > On Thu, 25 Jun 2020 12:44:36 +0200 Daniel Vetter wrote: > > Maybe an aside, but the guideline is for autoconfiguration: > > - Light up everything that has connector status connected. > > - If nothing has that status, try to light up the connectors with > > status "unknown". > > > > This is only really relevant on older platforms, mostly for VGA and > > somewhat for dvi outputs. > > > > Maybe another thing we should put down somewhere in the uapi docs ... > > As I had no idea what "unknown" means or when it can happen, I assumed > that it must mean "the hardware cannot know". If the hardware cannot > know, then I certainly will not be trying to enable that, unless > explicitly configured to do so. Having a phantom output is worse than > having a real output that does not light up, because it's not obvious at > first with phantom output that anything is wrong. You may just be > wondering where your windows disappear, or where did you mouse cursor > go, or why you see a wallpaper but no login dialog, etc. How about a refinement of Dan's suggestion, proceeding down this logical order and breaking if true: - ignore all disconnected outputs - if any outputs are connected, ignore all unknown outputs - if only one output is unknown, use only that output (with default mode if need be) - if any outputs are unknown but have EDID present, use only those outputs - at this point, we have multiple unknown outputs with no EDID - break and demand explicit user configuration Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] gpu: ipu-v3: Restore RGB32, BGR32
On Wed, 2020-06-17 at 15:40 -0700, Steve Longerbeam wrote: > RGB32 and BGR32 formats were inadvertently removed from the switch > statement in ipu_pixelformat_to_colorspace(). Restore them. > > Fixes: a59957172b0c ("gpu: ipu-v3: enable remaining 32-bit RGB V4L2 pixel > formats") > Signed-off-by: Steve Longerbeam Reviewed-by: Philipp Zabel regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] drm: expose connector status values in uapi
On Fri, 26 Jun 2020 07:55:12 + Simon Ser wrote: > Right now user-space doesn't have access to connector status definitions. > This means user-space needs to maintain a separate enum for these, and > makes it difficult to document the uapi. > > Introduce defines in drm_mode.h, and copy over the docs. Keep using the > drm_connector_status enum in drivers, because it allows for nice things > when using the enum as a type. > > Signed-off-by: Simon Ser > Cc: Daniel Vetter > Cc: Pekka Paalanen > --- > > - Would something like this be desirable? > - Docs are just copied over for now, not improved > - Same applies to e.g. the subpixel field Hi, xf86drmMode.h in libdrm already has: typedef enum { DRM_MODE_CONNECTED = 1, DRM_MODE_DISCONNECTED = 2, DRM_MODE_UNKNOWNCONNECTION = 3 } drmModeConnection; I have no opinion really if adding yet another set of the same definitions is good or not. We do need the UAPI doc, but that does not necessarily mean we also need definition code in UAPI headers. I give this one a *shrug*. Thanks, pq > include/drm/drm_connector.h | 30 -- > include/uapi/drm/drm_mode.h | 27 ++- > 2 files changed, 30 insertions(+), 27 deletions(-) > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index fd543d1db9b2..f48f8072aa89 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -53,34 +53,12 @@ enum drm_connector_force { > /** > * enum drm_connector_status - status for a &drm_connector > * > - * This enum is used to track the connector status. There are no separate > - * #defines for the uapi! > + * This enum is used to track the connector status. See the uapi docs. > */ > enum drm_connector_status { > - /** > - * @connector_status_connected: The connector is definitely connected to > - * a sink device, and can be enabled. > - */ > - connector_status_connected = 1, > - /** > - * @connector_status_disconnected: The connector isn't connected to a > - * sink device which can be autodetect. For digital outputs like DP or > - * HDMI (which can be realiable probed) this means there's really > - * nothing there. It is driver-dependent whether a connector with this > - * status can be lit up or not. > - */ > - connector_status_disconnected = 2, > - /** > - * @connector_status_unknown: The connector's status could not be > - * reliably detected. This happens when probing would either cause > - * flicker (like load-detection when the connector is in use), or when a > - * hardware resource isn't available (like when load-detection needs a > - * free CRTC). It should be possible to light up the connector with one > - * of the listed fallback modes. For default configuration userspace > - * should only try to light up connectors with unknown status when > - * there's not connector with @connector_status_connected. > - */ > - connector_status_unknown = 3, > + connector_status_connected = DRM_MODE_STATUS_CONNECTED, > + connector_status_disconnected = DRM_MODE_STATUS_DISCONNECTED, > + connector_status_unknown = DRM_MODE_STATUS_UNKNOWN, > }; > > /** > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 735c8cfdaaa1..0deffda35487 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -363,6 +363,31 @@ enum drm_mode_subconnector { > #define DRM_MODE_CONNECTOR_WRITEBACK 18 > #define DRM_MODE_CONNECTOR_SPI 19 > > +/** > + * @DRM_MODE_STATUS_CONNECTED: The connector is definitely connected to > + * a sink device, and can be enabled. > + */ > +#define DRM_MODE_STATUS_CONNECTED 1 > +/** > + * @DRM_MODE_STATUS_DISCONNECTED: The connector isn't connected to a > + * sink device which can be autodetect. For digital outputs like DP or > + * HDMI (which can be realiable probed) this means there's really > + * nothing there. It is driver-dependent whether a connector with this > + * status can be lit up or not. > + */ > +#define DRM_MODE_STATUS_DISCONNECTED 2 > +/** > + * @DRM_MODE_STATUS_UNKNOWN: The connector's status could not be > + * reliably detected. This happens when probing would either cause > + * flicker (like load-detection when the connector is in use), or when a > + * hardware resource isn't available (like when load-detection needs a > + * free CRTC). It should be possible to light up the connector with one > + * of the listed fallback modes. For default configuration userspace > + * should only try to light up connectors with unknown status when > + * there's not connector with @connector_status_connected. > + */ > +#define DRM_MODE_STATUS_UNKNOWN 3 > + > struct drm_mode_get_connector { > > __u64 encoders_ptr; > @@ -379,7 +404,7 @@ struct drm_mode_get_connector { > __u32 connector_type; > __u32 connector_type_id; > > -
[radeon-alex:drm-next 462/499] drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn20.c:326:8: warning: missing braces around initializer
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next head: 7aebbbd59a2ee62e3b75d7e8e3617171c3c6a208 commit: 956a0ff27b40732740abac49cd773574a4a81793 [462/499] drm/amd/display: add mechanism to skip DCN init config: i386-randconfig-a014-20200624 (attached as .config) compiler: gcc-4.9 (Ubuntu 4.9.3-13ubuntu2) 4.9.3 reproduce (this is a W=1 build): git checkout 956a0ff27b40732740abac49cd773574a4a81793 # save the attached .config to linux build tree make W=1 ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/amd/amdgpu/../display/dmub/src/../dmub_srv.h:67:0, from drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn20.c:26: drivers/gpu/drm/amd/amdgpu/../display/dmub/src/../inc/dmub_cmd.h: In function 'dmub_rb_flush_pending': drivers/gpu/drm/amd/amdgpu/../display/dmub/src/../inc/dmub_cmd.h:785:12: warning: variable 'temp' set but not used [-Wunused-but-set-variable] uint64_t temp; ^ drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn20.c: In function 'dmub_dcn20_enable_dmub_boot_options': >> drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn20.c:326:8: warning: >> missing braces around initializer [-Wmissing-braces] union dmub_fw_boot_options boot_options = {0}; ^ drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn20.c:326:8: warning: (near initialization for 'boot_options.bits') [-Wmissing-braces] vim +326 drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn20.c 323 324 void dmub_dcn20_enable_dmub_boot_options(struct dmub_srv *dmub) 325 { > 326 union dmub_fw_boot_options boot_options = {0}; --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Host1x/TegraDRM UAPI
On 6/26/20 2:24 AM, Dmitry Osipenko wrote: 25.06.2020 12:16, Mikko Perttunen пишет: On 6/25/20 2:11 AM, Dmitry Osipenko wrote: 23.06.2020 15:09, Mikko Perttunen пишет: /* Command is an opcode gather from a GEM handle */ #define DRM_TEGRA_SUBMIT_COMMAND_GATHER 0 /* Command is an opcode gather from a user pointer */ #define DRM_TEGRA_SUBMIT_COMMAND_GATHER_UPTR 1 I'm a bit dubious about whether we really need to retain the non-UPTR variant. The memory-copying overhead is negligible because cmdstream's data usually is hot in CPU's cache IIRC, the most (if not all) of the modern DRM drivers drivers use the usrptr-only for the cmdstream. At least there is no any real-world userspace example today that could benefit from a non-UPTR variant. I'm suggesting to leave out the non-UPTR gather variant for now, keeping it in mind as a potential future extension of the submission UAPI. Any objections? Sure, we should be able to drop it. Downstream userspace is using it, but we should be able to fix that. I was thinking that we can directly map the user pages and point the gather to them without copying - that way we wouldn't need to make DMA allocations inside the driver for every submit. We will need to create a Host1x DMA pool and then the dynamic allocations will be cheap. This is an implementation detail that we can discuss separately. We will need the UPTR anyways for the older Tergas because we need to validate the cmdstreams and it's much more efficient to copy from UPTR than from the uncacheable memory. The non-UPTR variant will be fine to add if you'll have a realworld example that demonstrates a noticeable performance difference. Previously, I thought that there will be some perf difference if GR3D shaders are moved into the "insert-opcode" gather, but it was negligible once I implemented it and it should be even more negligible on a modern hardware. (On early Tegras we could just copy into the pushbuffer but that won't work for newer ones). Yes, we should copy data into a gather and then push it into channel's pushbuffer. Just like it works with the current upstream driver. Once all the UAPI will be settled, we'll also need to discuss the pushbuffer's implementation because the current driver has some problems with it. True, for earlier Tegras we'll need to copy anyway. So let's just implement copying for now, while making sure that extending to directly mapping pages will be possible later (don't know why it wouldn't be), and implement direct mapping or GEM gathers later if needed. Mikko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Host1x/TegraDRM UAPI (drm_tegra_submit_relocation)
On 6/26/20 1:50 AM, Dmitry Osipenko wrote: 25.06.2020 12:27, Mikko Perttunen пишет: On 6/25/20 1:33 AM, Dmitry Osipenko wrote: 23.06.2020 15:09, Mikko Perttunen пишет: struct drm_tegra_submit_relocation { /* [in] Index of GATHER or GATHER_UPTR command in commands. */ __u32 gather_command_index; /* * [in] Mapping handle (obtained through CHANNEL_MAP) of the memory * the address of which will be patched in. */ __u32 mapping_id; /* * [in] Offset in the gather that will be patched. */ __u64 gather_offset; /* * [in] Offset in target buffer whose paddr/IOVA will be written * to the gather. */ __u64 target_offset; /* * [in] Number of bits the resulting address will be logically * shifted right before writing to gather. */ __u32 shift; __u32 reserved[1]; }; We will also need read/write direction flag here for the DMA-reservations set up, it will be used for the implicit BO fencing by the job's scheduler. Ideally on newer chips with context isolation, we no longer know what DMA-BUFs are being used by the job at submit time - they would just be pointers after being MAPped. The DMA-BUFs themselves shouldn't be needed, but GEMs should. Yes, I meant to say GEM instead of DMA-BUF. Do you know how other GPUs deal with DMA reservations - I expect separate MAP and SUBMIT steps would be pretty common? I can't instantly recall what other drivers do, could be worthwhile to take a closer look. I'll see if I can find some similar situations in other drivers. Mikko Do you have to pass the DMA-BUF to both steps (i.e. do IOMMU mapping during MAP, and manage reservations at SUBMIT)? Yes, this sounds good to me if DMA-BUF part is replaced with a GEM. Please see my other reply regarding the MAP IOCTL where I'm suggesting to back mapping IDs with a GEM. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 27/27] drm: Add default modes for connectors in unknown state
On Thu, 25 Jun 2020 12:44:36 +0200 Daniel Vetter wrote: > On Thu, Jun 25, 2020 at 12:32 PM Pekka Paalanen wrote: > > > > On Thu, 25 Jun 2020 09:57:44 +0200 > > Daniel Vetter wrote: > > > > > On Thu, Jun 25, 2020 at 9:56 AM Daniel Vetter wrote: > > > > > > > > On Wed, Jun 24, 2020 at 03:40:42PM -0400, Alex Deucher wrote: ... > > > > > No, you are right; you will have the EDID so this shouldn't be an > > > > > issue. I was mis-remembering the original issue. We originally > > > > > always reported connected for LVDS in radeon if the panel was present, > > > > > but then we got flack because some userspace expected unknown in > > > > > certain cases (e.g., lid or muxed displays). Either way the EDID info > > > > > is still there. > > > > > > > > Yeah I think i915 started that habit, but I guess people realized it's > > > > unreliable enough that they should have their own lid handler in the > > > > desktop enviromnent doing whatever they want to do on lid close. > > > > > > > > Should we perhaps document that somewhere, that panels are always marked > > > > as connected? Not even sure where to put that in the docs ... > > > > > > > > Maybe adding a few of the usual suspects from the compositor side, > > > > Simon, > > > > Pekka? > > > > > > Actually adding Simon and Pekka this time around ... > > > > I don't know what anyone else does, but Weston (is not a DE) does not > > look at any lid switch, and assumes that if connector status is not > > DRM_MODE_CONNECTED, then it is disconnected. So, if a driver switched > > to "Unknown" status, it would be taken as disconnected. > > Maybe an aside, but the guideline is for autoconfiguration: > - Light up everything that has connector status connected. > - If nothing has that status, try to light up the connectors with > status "unknown". > > This is only really relevant on older platforms, mostly for VGA and > somewhat for dvi outputs. > > Maybe another thing we should put down somewhere in the uapi docs ... As I had no idea what "unknown" means or when it can happen, I assumed that it must mean "the hardware cannot know". If the hardware cannot know, then I certainly will not be trying to enable that, unless explicitly configured to do so. Having a phantom output is worse than having a real output that does not light up, because it's not obvious at first with phantom output that anything is wrong. You may just be wondering where your windows disappear, or where did you mouse cursor go, or why you see a wallpaper but no login dialog, etc. I certainly do hope no-one uses this quirk of Weston to get their lid do what they want... it's possible, OTOH the desktop user base of Weston according to what I've heard is around one person. It's not me. Thanks, pq pgpq3Yc3HC5QX.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC] drm: expose connector status values in uapi
Right now user-space doesn't have access to connector status definitions. This means user-space needs to maintain a separate enum for these, and makes it difficult to document the uapi. Introduce defines in drm_mode.h, and copy over the docs. Keep using the drm_connector_status enum in drivers, because it allows for nice things when using the enum as a type. Signed-off-by: Simon Ser Cc: Daniel Vetter Cc: Pekka Paalanen --- - Would something like this be desirable? - Docs are just copied over for now, not improved - Same applies to e.g. the subpixel field include/drm/drm_connector.h | 30 -- include/uapi/drm/drm_mode.h | 27 ++- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index fd543d1db9b2..f48f8072aa89 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -53,34 +53,12 @@ enum drm_connector_force { /** * enum drm_connector_status - status for a &drm_connector * - * This enum is used to track the connector status. There are no separate - * #defines for the uapi! + * This enum is used to track the connector status. See the uapi docs. */ enum drm_connector_status { - /** -* @connector_status_connected: The connector is definitely connected to -* a sink device, and can be enabled. -*/ - connector_status_connected = 1, - /** -* @connector_status_disconnected: The connector isn't connected to a -* sink device which can be autodetect. For digital outputs like DP or -* HDMI (which can be realiable probed) this means there's really -* nothing there. It is driver-dependent whether a connector with this -* status can be lit up or not. -*/ - connector_status_disconnected = 2, - /** -* @connector_status_unknown: The connector's status could not be -* reliably detected. This happens when probing would either cause -* flicker (like load-detection when the connector is in use), or when a -* hardware resource isn't available (like when load-detection needs a -* free CRTC). It should be possible to light up the connector with one -* of the listed fallback modes. For default configuration userspace -* should only try to light up connectors with unknown status when -* there's not connector with @connector_status_connected. -*/ - connector_status_unknown = 3, + connector_status_connected = DRM_MODE_STATUS_CONNECTED, + connector_status_disconnected = DRM_MODE_STATUS_DISCONNECTED, + connector_status_unknown = DRM_MODE_STATUS_UNKNOWN, }; /** diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 735c8cfdaaa1..0deffda35487 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -363,6 +363,31 @@ enum drm_mode_subconnector { #define DRM_MODE_CONNECTOR_WRITEBACK 18 #define DRM_MODE_CONNECTOR_SPI 19 +/** + * @DRM_MODE_STATUS_CONNECTED: The connector is definitely connected to + * a sink device, and can be enabled. + */ +#define DRM_MODE_STATUS_CONNECTED 1 +/** + * @DRM_MODE_STATUS_DISCONNECTED: The connector isn't connected to a + * sink device which can be autodetect. For digital outputs like DP or + * HDMI (which can be realiable probed) this means there's really + * nothing there. It is driver-dependent whether a connector with this + * status can be lit up or not. + */ +#define DRM_MODE_STATUS_DISCONNECTED 2 +/** + * @DRM_MODE_STATUS_UNKNOWN: The connector's status could not be + * reliably detected. This happens when probing would either cause + * flicker (like load-detection when the connector is in use), or when a + * hardware resource isn't available (like when load-detection needs a + * free CRTC). It should be possible to light up the connector with one + * of the listed fallback modes. For default configuration userspace + * should only try to light up connectors with unknown status when + * there's not connector with @connector_status_connected. + */ +#define DRM_MODE_STATUS_UNKNOWN 3 + struct drm_mode_get_connector { __u64 encoders_ptr; @@ -379,7 +404,7 @@ struct drm_mode_get_connector { __u32 connector_type; __u32 connector_type_id; - __u32 connection; + __u32 connection; /**< see DRM_MODE_STATUS_* */ __u32 mm_width; /**< width in millimeters */ __u32 mm_height; /**< height in millimeters */ __u32 subpixel; -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 08/13] drm/panel: st7703: Move generic part of init sequence to enable callback
Calling sleep out and display on is a controller specific part of the initialization process. Move it out of the panel specific initialization function to the enable callback. Signed-off-by: Ondrej Jirman --- drivers/gpu/drm/panel/panel-sitronix-st7703.c | 33 ++- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c b/drivers/gpu/drm/panel/panel-sitronix-st7703.c index d03aab10cfef..cdbf7dfb4dd4 100644 --- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c +++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c @@ -84,8 +84,6 @@ static inline struct st7703 *panel_to_st7703(struct drm_panel *panel) static int jh057n_init_sequence(struct st7703 *ctx) { struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); - struct device *dev = ctx->dev; - int ret; /* * Init sequence was supplied by the panel vendor. Most of the commands @@ -136,20 +134,7 @@ static int jh057n_init_sequence(struct st7703 *ctx) 0x18, 0x00, 0x09, 0x0E, 0x29, 0x2D, 0x3C, 0x41, 0x37, 0x07, 0x0B, 0x0D, 0x10, 0x11, 0x0F, 0x10, 0x11, 0x18); - msleep(20); - - ret = mipi_dsi_dcs_exit_sleep_mode(dsi); - if (ret < 0) { - DRM_DEV_ERROR(dev, "Failed to exit sleep mode: %d\n", ret); - return ret; - } - /* Panel is operational 120 msec after reset */ - msleep(60); - ret = mipi_dsi_dcs_set_display_on(dsi); - if (ret) - return ret; - DRM_DEV_DEBUG_DRIVER(dev, "Panel init sequence done\n"); return 0; } @@ -181,6 +166,7 @@ struct st7703_panel_desc jh057n00900_panel_desc = { static int st7703_enable(struct drm_panel *panel) { struct st7703 *ctx = panel_to_st7703(panel); + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); int ret; ret = ctx->desc->init_sequence(ctx); @@ -190,6 +176,23 @@ static int st7703_enable(struct drm_panel *panel) return ret; } + msleep(20); + + ret = mipi_dsi_dcs_exit_sleep_mode(dsi); + if (ret < 0) { + DRM_DEV_ERROR(ctx->dev, "Failed to exit sleep mode: %d\n", ret); + return ret; + } + + /* Panel is operational 120 msec after reset */ + msleep(60); + + ret = mipi_dsi_dcs_set_display_on(dsi); + if (ret) + return ret; + + DRM_DEV_DEBUG_DRIVER(ctx->dev, "Panel init sequence done\n"); + return 0; } -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 07/13] drm/panel: st7703: Move code specific to jh057n closer together
It's better than having it spread around the driver. Signed-off-by: Ondrej Jirman --- drivers/gpu/drm/panel/panel-sitronix-st7703.c | 50 +-- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c b/drivers/gpu/drm/panel/panel-sitronix-st7703.c index 08cbc316266c..d03aab10cfef 100644 --- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c +++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c @@ -153,6 +153,31 @@ static int jh057n_init_sequence(struct st7703 *ctx) return 0; } +static const struct drm_display_mode jh057n00900_mode = { + .hdisplay= 720, + .hsync_start = 720 + 90, + .hsync_end = 720 + 90 + 20, + .htotal = 720 + 90 + 20 + 20, + .vdisplay= 1440, + .vsync_start = 1440 + 20, + .vsync_end = 1440 + 20 + 4, + .vtotal = 1440 + 20 + 4 + 12, + .vrefresh= 60, + .clock = 75276, + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, + .width_mm= 65, + .height_mm = 130, +}; + +struct st7703_panel_desc jh057n00900_panel_desc = { + .mode = &jh057n00900_mode, + .lanes = 4, + .mode_flags = MIPI_DSI_MODE_VIDEO | + MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE, + .format = MIPI_DSI_FMT_RGB888, + .init_sequence = jh057n_init_sequence, +}; + static int st7703_enable(struct drm_panel *panel) { struct st7703 *ctx = panel_to_st7703(panel); @@ -226,31 +251,6 @@ static int st7703_prepare(struct drm_panel *panel) return ret; } -static const struct drm_display_mode jh057n00900_mode = { - .hdisplay= 720, - .hsync_start = 720 + 90, - .hsync_end = 720 + 90 + 20, - .htotal = 720 + 90 + 20 + 20, - .vdisplay= 1440, - .vsync_start = 1440 + 20, - .vsync_end = 1440 + 20 + 4, - .vtotal = 1440 + 20 + 4 + 12, - .vrefresh= 60, - .clock = 75276, - .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, - .width_mm= 65, - .height_mm = 130, -}; - -struct st7703_panel_desc jh057n00900_panel_desc = { - .mode = &jh057n00900_mode, - .lanes = 4, - .mode_flags = MIPI_DSI_MODE_VIDEO | - MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_VIDEO_SYNC_PULSE, - .format = MIPI_DSI_FMT_RGB888, - .init_sequence = jh057n_init_sequence, -}; - static int st7703_get_modes(struct drm_panel *panel, struct drm_connector *connector) { -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Host1x/TegraDRM UAPI (drm_tegra_channel_map)
23.06.2020 15:09, Mikko Perttunen пишет: > ### DRM_TEGRA_CHANNEL_MAP > > Make memory accessible by the engine while executing work on the channel. > > ``` > #define DRM_TEGRA_CHANNEL_MAP_READWRITE (1<<0) > > struct drm_tegra_channel_map { > /* > * [in] ID of the channel for which to map memory to. > */ > __u32 channel_id; > /* > * [in] GEM handle of the memory to map. > */ > __u32 handle; > > /* > * [in] Offset in GEM handle of the memory area to map. > * > * Must be aligned by 4K. > */ > __u64 offset; Could you please give a use-case example for this partial mapping? I vaguely recalling that maybe it was me who suggested this in the past.. I kinda see that this could be useful for a case where userspace allocates a large chunk of memory and then performs sub-allocations in the userspace driver. But do we have a real-world example for this right now? Please see more below. > /* > * [in] Length of memory area to map in bytes. > * > * Must be aligned by 4K. > */ > __u64 length; > > /* > * [out] IOVA of mapped memory. Userspace can use this IOVA > * directly to refer to the memory to skip using relocations. > * Only available if hardware memory isolation is enabled. > * > * Will be set to 0x___ if unavailable. > */ > __u64 iova; > > /* > * [out] ID corresponding to the mapped memory to be used for > * relocations or unmapping. > */ > __u32 mapping_id; > /* > * [in] Flags. > */ > __u32 flags; > > __u32 reserved[6]; > }; It looks to me that we actually need a bit different thing here. This MAP IOCTL maps a portion of a GEM and then returns the mapping_id. And I think we need something more flexible that will allow us to use GEM handles for the relocation IDs, which should fit nicely with the DMA-reservations. What about an IOCTL that wraps GEM into another GEM? We could wrap a portion of GEM_A into a GEM_B, and then map the GEM_B using the MAP IOCTL. It could be something like this: ### DRM_TEGRA_BO_WRAP struct drm_tegra_wrap_bo { __u32 bo_handle_wrapped; // out __u32 bo_handle; // in __u64 offset; __u64 length; }; ### DRM_TEGRA_CHANNEL_MAP struct drm_tegra_channel_map { __u32 channels_mask; __u32 mapping_id; __u32 bo_handle; __u32 flags; __u64 iova; }; === This allows multiple mapping_ids to have the same backing GEM, so the mapping_id could be resolved into a BO during of job's submission for the DMA-reservations handling. Also: 1. Maybe the WRAP IOCTL could be a generic GEM IOCTL? 2. I guess we could start easy without the WRAP IOCTL and add it later on once there will be a real-world need. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 13/13] arm64: dts: sun50i-a64-pinephone: Add touchscreen support
Pinephone has a Goodix GT917S capacitive touchscreen controller on I2C0 bus. Add support for it. Signed-off-by: Ondrej Jirman --- .../dts/allwinner/sun50i-a64-pinephone.dtsi | 19 +++ 1 file changed, 19 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi index 85a7aa5efd32..2d5694446d17 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi @@ -123,6 +123,25 @@ &ehci1 { status = "okay"; }; +&i2c0 { + pinctrl-names = "default"; + pinctrl-0 = <&i2c0_pins>; + status = "okay"; + + touchscreen@5d { + compatible = "goodix,gt917s", "goodix,gt911"; + reg = <0x5d>; + interrupt-parent = <&pio>; + interrupts = <7 4 IRQ_TYPE_LEVEL_HIGH>; /* PH4 */ + irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */ + reset-gpios = <&pio 7 11 GPIO_ACTIVE_HIGH>; /* PH11 */ + AVDD28-supply = <®_ldo_io0>; + VDDIO-supply = <®_ldo_io0>; + touchscreen-size-x = <720>; + touchscreen-size-y = <1440>; + }; +}; + &i2c1 { status = "okay"; -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1] drm/amd/powerplay: Fix NULL dereference in lock_bus() on Vega20 w/o RAS
Issue still reproduces on latest 5.8.0-rc2+ (8be3a53e18e0e1a98f288f6c7f5e9da3adbe9c49). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel