Re: [PATCH v5] drm/panel: Add support for E Ink VB3300-KCA
On Sun, Aug 1, 2021 at 4:26 AM Sam Ravnborg wrote: > > Hi Alistair, > > On Fri, Jul 30, 2021 at 10:13:10PM +1000, Alistair Francis wrote: > > Add support for the 10.3" E Ink panel described at: > > https://www.eink.com/product.html?type=productdetail&id=7 > > > > Signed-off-by: Alistair Francis > > Acked-by: Rob Herring > > Reviewed-by: Sam Ravnborg > > --- > > v5: > > - Add .connector_type > > I missed this revision before sending my last mail. > I tried to apply this patch but every patch confliced due to other > changes since the kernel this is based on. > > I need you to generate a new patch on top of drm-misc-next, > or as an alternative on top of linux-next. > You are in a much better position to do this right than I am. > > Sorry for the troubles! No worries! Just sent a v6 rebased on linux-next. I am never sure if my patches should be based on master or linux-next. Sorry for the conflicts. Alistair
[PATCH v3 0/3] Remove all strcpy() uses
strcpy() performs no bounds checking on the destination buffer. This could result in linear overflows beyond the end of the buffer, leading to all kinds of misbehaviors. So, this serie removes all strcpy uses from the "staging/fbtft" subsystem. Also, refactor the code a bit to follow the kernel coding-style and avoid unnecessary variable initialization. Changelog v1 -> v2 - Add two new commits to clean the code. - Use the "%*ph" format specifier instead of strscpy() function (Geert Uytterhoeven) Changelog v2 -> v3 - Change the initialization of the "j" variable in the "for" loop and update the code accordingly (Andy Shevchenko). - Improve the commit message to inform that the "%*ph" replacement won't cut output earlier than requested (Andy Shevchenko). - Don't remove the braces in the "if" statement due to the presence of the comment (Geert Uytterhoeven). Len Baker (3): staging/fbtft: Remove all strcpy() uses staging/fbtft: Remove unnecessary variable initialization staging/fbtft: Fix braces coding style drivers/staging/fbtft/fbtft-core.c | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) -- 2.25.1
[PATCH v3 1/3] staging/fbtft: Remove all strcpy() uses
strcpy() performs no bounds checking on the destination buffer. This
could result in linear overflows beyond the end of the buffer, leading
to all kinds of misbehaviors. The safe replacement is strscpy() but in
this case it is simpler to use the "%*ph" format specifier.
Moreover, with the "0x%02X " in the sprintf followed by the strcat, the
msg buffer (now removed) can print 128/5 values (25 hex values). So, the
"%*ph" replacement won't cut output earlier than requested since this
format specifier can print up to 64 bytes.
Signed-off-by: Len Baker
---
drivers/staging/fbtft/fbtft-core.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index 3723269890d5..e6286043bff7 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -992,8 +992,6 @@ static int fbtft_init_display_from_property(struct
fbtft_par *par)
int fbtft_init_display(struct fbtft_par *par)
{
int buf[64];
- char msg[128];
- char str[16];
int i = 0;
int j;
@@ -1036,17 +1034,14 @@ int fbtft_init_display(struct fbtft_par *par)
switch (par->init_sequence[i]) {
case -1:
i++;
+
/* make debug message */
- strcpy(msg, "");
- j = i + 1;
- while (par->init_sequence[j] >= 0) {
- sprintf(str, "0x%02X ", par->init_sequence[j]);
- strcat(msg, str);
- j++;
- }
+ for (j = 0; par->init_sequence[i + 1 + j] >= 0; j++);
+
fbtft_par_dbg(DEBUG_INIT_DISPLAY, par,
- "init: write(0x%02X) %s\n",
- par->init_sequence[i], msg);
+ "init: write(0x%02X) %*ph\n",
+ par->init_sequence[i], j,
+ &par->init_sequence[i + 1]);
/* Write */
j = 0;
--
2.25.1
[Bug 211425] [drm:atom_op_jump] *ERROR* atombios stuck in loop for more than 20secs aborting
https://bugzilla.kernel.org/show_bug.cgi?id=211425 Andreas (icedragon...@web.de) changed: What|Removed |Added Kernel Version|5.13.4 |5.13.6 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 213935] New: AMDGPU Renoir crash/freeze while using vaapi with some video types in some apps - drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out!
https://bugzilla.kernel.org/show_bug.cgi?id=213935 Bug ID: 213935 Summary: AMDGPU Renoir crash/freeze while using vaapi with some video types in some apps - drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out! Product: Drivers Version: 2.5 Kernel Version: 5.13.6 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: plusf...@gmail.com Regression: No Created attachment 298139 --> https://bugzilla.kernel.org/attachment.cgi?id=298139&action=edit dmesg Jul 31 09:50:49 helium kernel: [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out! Jul 31 09:50:52 helium kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=18739, emitted seq=18742 Jul 31 09:50:52 helium kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process com.github.rafo pid 4266 thread gjs:cs0 pid 4320 Jul 31 09:50:52 helium kernel: amdgpu :04:00.0: [drm:amdgpu_ring_test_helper [amdgpu]] *ERROR* ring kiq_2.1.0 test failed (-110) Jul 31 09:50:53 helium kernel: amdgpu :04:00.0: [drm:amdgpu_ring_test_helper [amdgpu]] *ERROR* ring kiq_2.1.0 test failed (-110) Jul 31 09:50:53 helium kernel: [drm:amdgpu_gfx_enable_kcq.cold [amdgpu]] *ERROR* KCQ enable failed Jul 31 09:50:53 helium kernel: [drm:amdgpu_device_ip_resume_phase2 [amdgpu]] *ERROR* resume of IP block failed -110 In certain situations I'm able to crash/freeze my system by playing mp4 videos with (vaapi-acceleration) enabled. If the crash/freeze happens, the screen goes black and the system isn't responding to any input. Sadly in this case it's at random if something was able to write output to the log. In most cases there is nothing written about the crash in the log. My environment is GNOME in Wayland mode With native Wayland apps (in this Case Firefox and clapper (https://rafostar.github.io/clapper/) APU is an Ryzen 5 4500U For Firefox I'm not able to reliable recreate that bug. It happens at random while scrolling in twitter and reddit. Never happens in Netflix or Youtube for example. Luckily I was able to recreate it with an app called clapper and a video provided by someone on reddit: https://cdn.discordapp.com/attachments/399812928854949890/870910339548590180/VID_20210731_124021.mp4 Steps: 1. Have GNOME running in Wayland mode and vaapi installed (check with 'vainfo`) 2. Install clapper 3. Download the video 4. Run the video in Clapper 5. While running, launch the video again in clapper It should *not* create another instance of clapper, but try to re-launch the video in the same instance of clapper that was already running. You'll hear maybe a few sec of the audio, but your whole session is frozen and will enter an all black screen without possible recovery a few sec later. I'm able to recreate this with every Kernel I tested. So down to 5.8 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v3 2/3] staging/fbtft: Remove unnecessary variable initialization
Remove the initialization of the variable "i" since it is written a few
lines later.
Signed-off-by: Len Baker
---
drivers/staging/fbtft/fbtft-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index e6286043bff7..ed896049118c 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -992,7 +992,7 @@ static int fbtft_init_display_from_property(struct
fbtft_par *par)
int fbtft_init_display(struct fbtft_par *par)
{
int buf[64];
- int i = 0;
+ int i;
int j;
/* sanity check */
--
2.25.1
Re: [PATCH v3 0/3] Remove all strcpy() uses
On Sun, Aug 1, 2021 at 11:53 AM Len Baker wrote: > > strcpy() performs no bounds checking on the destination buffer. This > could result in linear overflows beyond the end of the buffer, leading > to all kinds of misbehaviors. So, this serie removes all strcpy uses > from the "staging/fbtft" subsystem. > > Also, refactor the code a bit to follow the kernel coding-style and > avoid unnecessary variable initialization. I don't see patch 3 (even on lore.kernel.org). Greg, Geert, does it make sense to move this driver outside of staging? I would volunteer to maintain it there. > Changelog v1 -> v2 > - Add two new commits to clean the code. > - Use the "%*ph" format specifier instead of strscpy() function (Geert > Uytterhoeven) > > Changelog v2 -> v3 > - Change the initialization of the "j" variable in the "for" loop and > update the code accordingly (Andy Shevchenko). > - Improve the commit message to inform that the "%*ph" replacement > won't cut output earlier than requested (Andy Shevchenko). > - Don't remove the braces in the "if" statement due to the presence of > the comment (Geert Uytterhoeven). > > Len Baker (3): > staging/fbtft: Remove all strcpy() uses > staging/fbtft: Remove unnecessary variable initialization > staging/fbtft: Fix braces coding style > > drivers/staging/fbtft/fbtft-core.c | 23 ++- > 1 file changed, 10 insertions(+), 13 deletions(-) > > -- > 2.25.1 > -- With Best Regards, Andy Shevchenko
[PATCH v3 3/3] staging/fbtft: Fix braces coding style
Add braces to the "for" loop. This way, the kernel coding style is
followed.
Signed-off-by: Len Baker
---
drivers/staging/fbtft/fbtft-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index ed896049118c..ed992ca605eb 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1003,9 +1003,11 @@ int fbtft_init_display(struct fbtft_par *par)
}
/* make sure stop marker exists */
- for (i = 0; i < FBTFT_MAX_INIT_SEQUENCE; i++)
+ for (i = 0; i < FBTFT_MAX_INIT_SEQUENCE; i++) {
if (par->init_sequence[i] == -3)
break;
+ }
+
if (i == FBTFT_MAX_INIT_SEQUENCE) {
dev_err(par->info->device,
"missing stop marker at end of init sequence\n");
--
2.25.1
Re: [PATCH] DRM: ast: Fixed coding style issues of ast_mode.c
Hi
Am 31.07.21 um 02:53 schrieb Gregory Williams:
Removed space before comma, fixed if statements by putting trailing
statements on new line, fixed unsigned int declaration, and removed
not needed else statement after return.
Signed-off-by: Gregory Williams
Added to drm-misc-next. Thanks for the patch.
Best regards
Thomas
---
drivers/gpu/drm/ast/ast_mode.c | 31 ---
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 36d9575aa27b..1310ed092073 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -275,7 +275,7 @@ static void ast_set_std_reg(struct ast_private *ast,
ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x01, 0xdf,
stdtable->seq[0]);
for (i = 1; i < 4; i++) {
jreg = stdtable->seq[i];
- ast_set_index_reg(ast, AST_IO_SEQ_PORT, (i + 1) , jreg);
+ ast_set_index_reg(ast, AST_IO_SEQ_PORT, (i + 1), jreg);
}
/* Set CRTC; except base address and offset */
@@ -498,13 +498,15 @@ static void ast_set_sync_reg(struct ast_private *ast,
jreg = ast_io_read8(ast, AST_IO_MISC_PORT_READ);
jreg &= ~0xC0;
- if (vbios_mode->enh_table->flags & NVSync) jreg |= 0x80;
- if (vbios_mode->enh_table->flags & NHSync) jreg |= 0x40;
+ if (vbios_mode->enh_table->flags & NVSync)
+ jreg |= 0x80;
+ if (vbios_mode->enh_table->flags & NHSync)
+ jreg |= 0x40;
ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, jreg);
}
static void ast_set_start_address_crt1(struct ast_private *ast,
- unsigned offset)
+ unsigned int offset)
{
u32 addr;
@@ -1212,6 +1214,7 @@ static int ast_get_modes(struct drm_connector *connector)
struct edid *edid;
int ret;
bool flags = false;
+
if (ast->tx_chip_type == AST_TX_DP501) {
ast->dp501_maxclk = 0xff;
edid = kmalloc(128, GFP_KERNEL);
@@ -1231,8 +1234,8 @@ static int ast_get_modes(struct drm_connector *connector)
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
return ret;
- } else
- drm_connector_update_edid_property(&ast_connector->base, NULL);
+ }
+ drm_connector_update_edid_property(&ast_connector->base, NULL);
return 0;
}
@@ -1272,19 +1275,24 @@ static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
}
switch (mode->hdisplay) {
case 640:
- if (mode->vdisplay == 480) flags = MODE_OK;
+ if (mode->vdisplay == 480)
+ flags = MODE_OK;
break;
case 800:
- if (mode->vdisplay == 600) flags = MODE_OK;
+ if (mode->vdisplay == 600)
+ flags = MODE_OK;
break;
case 1024:
- if (mode->vdisplay == 768) flags = MODE_OK;
+ if (mode->vdisplay == 768)
+ flags = MODE_OK;
break;
case 1280:
- if (mode->vdisplay == 1024) flags = MODE_OK;
+ if (mode->vdisplay == 1024)
+ flags = MODE_OK;
break;
case 1600:
- if (mode->vdisplay == 1200) flags = MODE_OK;
+ if (mode->vdisplay == 1200)
+ flags = MODE_OK;
break;
default:
return flags;
@@ -1296,6 +1304,7 @@ static enum drm_mode_status ast_mode_valid(struct
drm_connector *connector,
static void ast_connector_destroy(struct drm_connector *connector)
{
struct ast_connector *ast_connector = to_ast_connector(connector);
+
ast_i2c_destroy(ast_connector->i2c);
drm_connector_cleanup(connector);
}
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v3 0/3] Remove all strcpy() uses
Hi Andy, On Sun, Aug 01, 2021 at 02:40:40PM +0300, Andy Shevchenko wrote: > On Sun, Aug 1, 2021 at 11:53 AM Len Baker wrote: > > > > strcpy() performs no bounds checking on the destination buffer. This > > could result in linear overflows beyond the end of the buffer, leading > > to all kinds of misbehaviors. So, this serie removes all strcpy uses > > from the "staging/fbtft" subsystem. > > > > Also, refactor the code a bit to follow the kernel coding-style and > > avoid unnecessary variable initialization. > > I don't see patch 3 (even on lore.kernel.org). Due to my email provider restrictions (number of emails per hour), I need to send an email every x time. Regards, Len
Re: [Intel-gfx] [PATCH] drm/i915: Use Transparent Hugepages when IOMMU is enabled
Hi Tvrtko,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Tvrtko-Ursulin/drm-i915-Use-Transparent-Hugepages-when-IOMMU-is-enabled/20210728-221515
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-a015-20210728 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce (this is a W=1 build):
#
https://github.com/0day-ci/linux/commit/26ad4b5023a428526c23ce544b593eced1916b49
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Tvrtko-Ursulin/drm-i915-Use-Transparent-Hugepages-when-IOMMU-is-enabled/20210728-221515
git checkout 26ad4b5023a428526c23ce544b593eced1916b49
# save the attached .config to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
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/i915/gem/i915_gemfs.c: In function 'i915_gemfs_init':
>> drivers/gpu/drm/i915/gem/i915_gemfs.c:16:30: error: expected ',' or ';'
>> before 'CONFIG_DRM_I915_THP_NATIVE'
16 | char thp_native[] = "huge=" CONFIG_DRM_I915_THP_NATIVE;
| ^~
>> drivers/gpu/drm/i915/gem/i915_gemfs.c:17:29: error: expected ',' or ';'
>> before 'CONFIG_DRM_I915_THP_IOMMU'
17 | char thp_iommu[] = "huge=" CONFIG_DRM_I915_THP_IOMMU;
| ^
vim +16 drivers/gpu/drm/i915/gem/i915_gemfs.c
13
14 int i915_gemfs_init(struct drm_i915_private *i915)
15 {
> 16 char thp_native[] = "huge=" CONFIG_DRM_I915_THP_NATIVE;
> 17 char thp_iommu[] = "huge=" CONFIG_DRM_I915_THP_IOMMU;
18 struct file_system_type *type;
19 struct vfsmount *gemfs;
20 char *opts;
21
22 type = get_fs_type("tmpfs");
23 if (!type)
24 return -ENODEV;
25
26 /*
27 * By creating our own shmemfs mountpoint, we can pass in
28 * mount flags that better match our usecase.
29 *
30 * One example, although it is probably better with a per-file
31 * control, is selecting huge page allocations
("huge=within_size").
32 * However, we only do so to offset the overhead of iommu
lookups
33 * due to bandwidth issues (slow reads) on Broadwell+.
34 */
35 opts = intel_vtd_active() ? thp_iommu : thp_native;
36
37 gemfs = vfs_kern_mount(type, SB_KERNMOUNT, type->name, opts);
38 if (IS_ERR(gemfs))
39 return PTR_ERR(gemfs);
40
41 i915->mm.gemfs = gemfs;
42
43 drm_info(&i915->drm, "Transparent Hugepage mode '%s'", opts);
44
45 return 0;
46 }
47
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[PATCH V5 0/2] drm/vkms: Add virtual hardware module
This patchset adds support for emulating virtual hardware with VKMS. The virtual hardware mode can be enabled by using the following command while loading the module: sudo modprobe vkms enable_virtual_hw=1 The first patch is prep work for adding virtual_hw mode and refactors the plane composition in vkms by adding a helper function vkms_composer_common() which can be used for both vblank mode and virtual mode. The second patch adds virtual hardware support as a module option. It adds new atomic helper functions for the virtual mode and modifies the existing atomic helpers for usage by the vblank mode This gives us two sets of drm_crtc_helper_funcs struct for both modes, making the code flow cleaner and easier to debug. This patchset has been tested with the igt tests- kms_writeback, kms_atomic, kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for subtests related to crc reads and skips tests that rely on vertical blanking. Sumera Priyadarsini (2): drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode drm/vkms: Add support for virtual hardware mode drivers/gpu/drm/vkms/vkms_composer.c | 93 ++-- drivers/gpu/drm/vkms/vkms_crtc.c | 51 ++- drivers/gpu/drm/vkms/vkms_drv.c | 16 +++-- drivers/gpu/drm/vkms/vkms_drv.h | 4 ++ 4 files changed, 112 insertions(+), 52 deletions(-) -- 2.31.1
[PATCH V5 1/2] drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode
Add a new function vkms_composer_common(). The actual plane
composition work has been moved to the helper function,
vkms_composer_common() which is called by vkms_composer_worker()
and will be called in the implementation of virtual_hw mode
as well.
Signed-off-by: Sumera Priyadarsini
---
Changes in V5:
- Move vkms_crtc_composer() to the patch that introduces
virtual_hw mode (Melissa)
- Fix checkpatch errors(Melissa)
Changes in V4:
- Fix warning
Changes in V3:
- Refactor patchset (Melissa)
Change in V2:
- Add vkms_composer_common() (Daniel)
---
drivers/gpu/drm/vkms/vkms_composer.c | 76
drivers/gpu/drm/vkms/vkms_drv.h | 2 +
2 files changed, 45 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index ead8fff81f30..bf3d576db225 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -206,6 +206,47 @@ static int compose_active_planes(void **vaddr_out,
return 0;
}
+int vkms_composer_common(struct vkms_crtc_state *crtc_state,
+struct vkms_output *out, bool wb_pending, uint32_t
*crc32)
+{
+ struct vkms_composer *primary_composer = NULL;
+ struct vkms_plane_state *act_plane = NULL;
+ void *vaddr_out = NULL;
+ int ret;
+
+ if (crtc_state->num_active_planes >= 1) {
+ act_plane = crtc_state->active_planes[0];
+ if (act_plane->base.base.plane->type == DRM_PLANE_TYPE_PRIMARY)
+ primary_composer = act_plane->composer;
+ }
+
+ if (!primary_composer)
+ return -EINVAL;
+ if (wb_pending)
+ vaddr_out = crtc_state->active_writeback;
+
+ ret = compose_active_planes(&vaddr_out, primary_composer, crtc_state);
+
+ if (ret) {
+ if ((ret == -EINVAL || ret == -ENOMEM) && !wb_pending)
+ kfree(vaddr_out);
+ return ret;
+ }
+
+ *crc32 = compute_crc(vaddr_out, primary_composer);
+
+ if (wb_pending) {
+ drm_writeback_signal_completion(&out->wb_connector, 0);
+ spin_lock_irq(&out->composer_lock);
+ crtc_state->wb_pending = false;
+ spin_unlock_irq(&out->composer_lock);
+ } else {
+ kfree(vaddr_out);
+ }
+
+ return 0;
+}
+
/**
* vkms_composer_worker - ordered work_struct to compute CRC
*
@@ -222,10 +263,7 @@ void vkms_composer_worker(struct work_struct *work)
composer_work);
struct drm_crtc *crtc = crtc_state->base.crtc;
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
- struct vkms_composer *primary_composer = NULL;
- struct vkms_plane_state *act_plane = NULL;
bool crc_pending, wb_pending;
- void *vaddr_out = NULL;
u32 crc32 = 0;
u64 frame_start, frame_end;
int ret;
@@ -247,37 +285,9 @@ void vkms_composer_worker(struct work_struct *work)
if (!crc_pending)
return;
- if (crtc_state->num_active_planes >= 1) {
- act_plane = crtc_state->active_planes[0];
- if (act_plane->base.base.plane->type == DRM_PLANE_TYPE_PRIMARY)
- primary_composer = act_plane->composer;
- }
-
- if (!primary_composer)
- return;
-
- if (wb_pending)
- vaddr_out = crtc_state->active_writeback;
-
- ret = compose_active_planes(&vaddr_out, primary_composer,
- crtc_state);
- if (ret) {
- if (ret == -EINVAL && !wb_pending)
- kfree(vaddr_out);
+ ret = vkms_composer_common(crtc_state, out, wb_pending, &crc32);
+ if (ret == -EINVAL)
return;
- }
-
- crc32 = compute_crc(vaddr_out, primary_composer);
-
- if (wb_pending) {
- drm_writeback_signal_completion(&out->wb_connector, 0);
- spin_lock_irq(&out->composer_lock);
- crtc_state->wb_pending = false;
- spin_unlock_irq(&out->composer_lock);
- } else {
- kfree(vaddr_out);
- }
-
/*
* The worker can fall behind the vblank hrtimer, make sure we catch up.
*/
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 8c731b6dcba7..01beba424f18 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -132,6 +132,8 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const
char *source_name,
size_t *values_cnt);
/* Composer Support */
+int vkms_composer_common(struct vkms_crtc_state *crtc_state, struct
vkms_output *out,
+bool wb_pending, uint32_t *crcs);
void vkms_composer_worker(struct work_struct *work);
void vkms_set_composer(struct vkms_output *out, bool enabled);
--
2.31.1
[PATCH V5 2/2] drm/vkms: Add support for virtual hardware mode
Add a virtual hardware or vblank-less mode as a module
to enable VKMS to emulate virtual hardware drivers. This means
no vertical blanking events occur and pageflips are completed
arbitrarily and when required for updating the frame.
Add a new drm_crtc_helper_funcs struct,
vkms_virtual_crtc_helper_funcs() which holds the atomic helpers
for virtual hardware mode. Rename the existing
vkms_crtc_helper_funcs struct to vkms_vblank_crtc_helper_funcs
which holds atomic helpers for the vblank mode.
This makes the code flow clearer and testing
virtual hardware mode.
Add a function vkms_crtc_composer() which calls the helper function,
vkms_composer_common() for plane composition in vblank-less mode.
vkms_crtc_composer() is directly called in the atomic hook in
vkms_crtc_atomic_begin().
However, some crc captures still use vblanks which causes the crc-based
igt tests to crash. Currently, I am unsure about how to approach the
one-shot implementation of crc reads so I am still working on that.
This patchset has been tested with the igt tests- kms_writeback, kms_atomic
, kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
subtests related to crc reads and skips tests that rely on vertical
blanking.
The patch is based on Rodrigo Siqueira's
patch(https://patchwork.freedesktop.org/patch/316851/?series=48469&rev=3)
and the ensuing review.
Signed-off-by: Sumera Priyadarsini
---
Changes in V5:
- Move vkms_crtc_composer() to this patch(Melissa)
- Add more clarification for "vblank-less" mode(Pekka)
- Replace kzalloc() with kvmalloc() in compose_active_planes()
to fix memory allocation error for output frame
- Fix checkpatch warnings (Melissa)
Changes in V3:
- Refactor patchset(Melissa)
Changes in V2:
- Add atomic helper functions in a separate struct for virtual hardware
mode (Daniel)
- Remove spinlock across 'vkms_output->lock' in vkms_crtc.c(Daniel)
- Add vkms_composer_common() (Daniel)
---
drivers/gpu/drm/vkms/vkms_composer.c | 21 ++--
drivers/gpu/drm/vkms/vkms_crtc.c | 51
drivers/gpu/drm/vkms/vkms_drv.c | 16 ++---
drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
4 files changed, 69 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index bf3d576db225..2988d5b49eb6 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -176,11 +176,12 @@ static int compose_active_planes(void **vaddr_out,
{
struct drm_framebuffer *fb = &primary_composer->fb;
struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
+
const void *vaddr;
int i;
if (!*vaddr_out) {
- *vaddr_out = kzalloc(gem_obj->size, GFP_KERNEL);
+ *vaddr_out = kvmalloc(gem_obj->size, GFP_KERNEL);
if (!*vaddr_out) {
DRM_ERROR("Cannot allocate memory for output frame.");
return -ENOMEM;
@@ -229,7 +230,7 @@ int vkms_composer_common(struct vkms_crtc_state *crtc_state,
if (ret) {
if ((ret == -EINVAL || ret == -ENOMEM) && !wb_pending)
- kfree(vaddr_out);
+ kvfree(vaddr_out);
return ret;
}
@@ -241,7 +242,7 @@ int vkms_composer_common(struct vkms_crtc_state *crtc_state,
crtc_state->wb_pending = false;
spin_unlock_irq(&out->composer_lock);
} else {
- kfree(vaddr_out);
+ kvfree(vaddr_out);
}
return 0;
@@ -295,6 +296,20 @@ void vkms_composer_worker(struct work_struct *work)
drm_crtc_add_crc_entry(crtc, true, frame_start++, &crc32);
}
+void vkms_crtc_composer(struct vkms_crtc_state *crtc_state)
+{
+ struct drm_crtc *crtc = crtc_state->base.crtc;
+ struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+ u32 crc32 = 0;
+ int ret;
+
+ ret = vkms_composer_common(crtc_state, out, crtc_state->wb_pending,
&crc32);
+ if (ret == -EINVAL)
+ return;
+
+ drm_crtc_add_crc_entry(crtc, true, 0, &crc32);
+}
+
static const char * const pipe_crc_sources[] = {"auto"};
const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 57bbd32e9beb..8477b33c4d09 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -222,20 +222,20 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
return 0;
}
-static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
- struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
drm_crtc_vblank_on(crtc);
}
-static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
-
Re: [PATCH] drm/amdgpu: fix possible null-pointer dereference in amdgpu_ttm_tt_populate()
Am 31.07.21 um 10:04 schrieb Tuo Li:
The variable ttm is assigned to the variable gtt, and the variable gtt
is checked in:
if (gtt && gtt->userptr)
This indicates that both ttm and gtt can be NULL.
If so, a null-pointer dereference will occur:
if (ttm->page_flags & TTM_PAGE_FLAG_SG)
Also, some null-pointer dereferences will occur in the function
ttm_pool_alloc() which is called in:
return ttm_pool_alloc(&adev->mman.bdev.pool, ttm, ctx);
To fix these possible null-pointer dereferences, the function returns
-EINVAL when ttm is NULL.
NAK, the NULL test is just a leftover from when the objects where distinct.
Please remove the NULL test instead.
Regards,
Christian.
Reported-by: TOTE Robot
Signed-off-by: Tuo Li
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3a55f08e00e1..80440f799c09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1120,8 +1120,11 @@ static int amdgpu_ttm_tt_populate(struct ttm_device
*bdev,
struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ if (ttm == NULL)
+ return -EINVAL;
+
/* user pages are bound by amdgpu_ttm_tt_pin_userptr() */
- if (gtt && gtt->userptr) {
+ if (gtt->userptr) {
ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!ttm->sg)
return -ENOMEM;
Re: [PATCH] drm/amdgpu: fix possible null-pointer dereference in amdgpu_ttm_tt_unpopulate()
Am 31.07.21 um 10:13 schrieb Tuo Li:
The variable ttm is assigned to the variable gtt, and the variable gtt
is checked in:
if (gtt && gtt->userptr)
This indicates that both ttm and gtt can be NULL.
If so, a null-pointer dereference will occur:
if (ttm->page_flags & TTM_PAGE_FLAG_SG)
Also, some null-pointer dereferences will occur in the function
ttm_pool_free() which is called in:
return ttm_pool_free(&adev->mman.bdev.pool, ttm);
To fix these possible null-pointer dereferences, the function returns
when ttm is NULL.
NAK, same as with the other patch.
The ttm object is mandatory, asking the driver to destroy a ttm object
which doesn't exists makes no sense at all and is a bug in the upper layer.
The NULL check is just a leftover from when the gtt and ttm objects
where distinct. Please remove that one instead.
BTW: Bonus points for changing the (void *) cast into a much cleaner
container_of().
Thanks,
Christian.
Reported-by: TOTE Robot
Signed-off-by: Tuo Li
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3a55f08e00e1..0216ca085f11 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1146,7 +1146,10 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device
*bdev,
struct amdgpu_ttm_tt *gtt = (void *)ttm;
struct amdgpu_device *adev;
- if (gtt && gtt->userptr) {
+ if (ttm == NULL)
+ return;
+
+ if (gtt->userptr) {
amdgpu_ttm_tt_set_user_pages(ttm, NULL);
kfree(ttm->sg);
ttm->sg = NULL;
Re: [BUG] video: fbdev: sis: possible uninitialized-variable access in SiS_SetCRT2FIFO_300()
Hi Tuo Li, On Sat, Jul 31, 2021 at 02:28:39PM +0800, Li Tuo wrote: > Hello, > > Our static analysis tool finds a possible uninitialized-variable access in > the sis driver in Linux 5.14.0-rc3: > > At the beginning of the function SiS_SetCRT2FIFO_300(), the variable > modeidindex is not initialized. > If the following conditions are false, it remains uninitialized. > 5346: if(!SiS_Pr->CRT1UsesCustomMode) > 5438: if(!SiS_Pr->UseCustomMode) > > But it is accessed at: > 5466: colorth = SiS_GetColorDepth(SiS_Pr,CRT2ModeNo,modeidindex) >> 1; > > I am not quite sure whether this possible uninitialized-variable access is > real and how to fix it if it is real. > Any feedback would be appreciated, thanks! First, the report looks correct. There is a path where modeindex may not be initilized. But I find it very hard to care for such an ancient driver. If this was somethign we hit is real life we had heard about it - and the risk of introducing bugs is higher than the the cance that this fixes a real life bug. So my advice, find something more relevant to look at. Sam
[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125
https://bugzilla.kernel.org/show_bug.cgi?id=205089 mcmar...@gmx.net changed: What|Removed |Added CC||mcmar...@gmx.net --- Comment #16 from mcmar...@gmx.net --- i have the same problem with the kernel 5.11.22-2-MANJARO -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 00/14] drm: Make DRM's IRQ helpers legacy
Hi Sam Am 31.07.21 um 20:50 schrieb Sam Ravnborg: Hi Thomas, On Tue, Jul 27, 2021 at 08:27:07PM +0200, Thomas Zimmermann wrote: DRM's IRQ helpers are only helpful for old, non-KMS drivers. Move the code behind CONFIG_DRM_LEGACY. Convert KMS drivers to Linux IRQ interfaces. DRM provides IRQ helpers for setting up, receiving and removing IRQ handlers. It's an abstraction over plain Linux functions. The code is mid-layerish with several callbacks to hook into the rsp drivers. Old UMS driver have their interrupts enabled via ioctl, so these abstractions makes some sense. Modern KMS manage all their interrupts internally. Using the DRM helpers adds indirection without benefits. Most KMs drivers already use Linux IRQ functions instead of DRM's abstraction layer. Patches 1 to 12 convert the remaining ones. The patches also resolve a bug for devices without assigned interrupt number. DRM helpers don't test for IRQ_NOTCONNECTED, so drivers do not detect if the device has no interrupt assigned. Patch 13 removes an unused function. Patch 14 moves the DRM IRQ helpers behind CONFIG_DRM_LEGACY. Only the old non-KMS drivers still use the functionality. Thomas Zimmermann (14): drm/amdgpu: Convert to Linux IRQ interfaces drm/arm/hdlcd: Convert to Linux IRQ interfaces drm/atmel-hlcdc: Convert to Linux IRQ interfaces drm/fsl-dcu: Convert to Linux IRQ interfaces drm/gma500: Convert to Linux IRQ interfaces drm/kmb: Convert to Linux IRQ interfaces drm/msm: Convert to Linux IRQ interfaces drm/mxsfb: Convert to Linux IRQ interfaces drm/radeon: Convert to Linux IRQ interfaces drm/tidss: Convert to Linux IRQ interfaces drm/tilcdc: Convert to Linux IRQ interfaces drm/vc4: Convert to Linux IRQ interfaces drm: Remove unused devm_drm_irq_install() drm: IRQ midlayer is now legacy With the irq_enabled confusion out of the way I want to re-address two issues here that I know you have answered but I am just not convinced. 1) IRQ_NOTCONNECTED We do not have this check in drm_irq today and we should avoid spreading it all over. We are either carrying it forever or we wil lsee patches floating in to drop the check again. The current use in the kernel is minimal: https://elixir.bootlin.com/linux/latest/A/ident/IRQ_NOTCONNECTED So as a minimum drop it from atmel_hlcdc and preferably from the rest as it is really not used. (Speaking as atmel_hlcdc maintainer) I'll drop it from atmel_hlcdc then. But saying that it's not used is not correct. At least radeon an gma500 handle PCI-based devices and BIOSes often had the option of disabling the rsp graphics interrupts. 2) devm_request_irq() We are moving towards managed allocation so we do not fail to free resources. And an irq has a lifetime equal the device itself - so an obvious cnadidate for devm_request_irq. If we do not introduce it now we will see a revisit of this later. I can be convinced to wait with this as we will have to do much more in each driver, but I cannot see any good arguments to avoid the more modern way to use devm_request_irq. I'll change this in atmel_hdlcd and maybe I can find trivial cases where devm_request_irq() can be used. But drivers that had an uninstall callback before should not have the cleanup logic altered by a patch as this one. I suspect that most of the IRQ cleanup is actually a vblank cleanup and should be done in response to drm_vblank_init(). But that's again not something for this patchset here. We cannot change multiple things at once and still expect any of it to work. I welcome the use of devm_ et al. But these changes are better done in a per-driver patchset that changes all of the driver to managed release. Best regards Thomas Sam -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 00/14] drm: Make DRM's IRQ helpers legacy
Hi Thomas, > > > > 1) IRQ_NOTCONNECTED > > > > We do not have this check in drm_irq today and we should avoid spreading > > it all over. We are either carrying it forever or we wil lsee patches > > floating in to drop the check again. > > The current use in the kernel is minimal: > > https://elixir.bootlin.com/linux/latest/A/ident/IRQ_NOTCONNECTED > > > > So as a minimum drop it from atmel_hlcdc and preferably from the rest as > > it is really not used. (Speaking as atmel_hlcdc maintainer) > > I'll drop it from atmel_hlcdc then. > > But saying that it's not used is not correct. My point is the drm_irq do not check this - so adding this check add something there was not needed/done before. > > 2) devm_request_irq() > > > > We are moving towards managed allocation so we do not fail to free > > resources. And an irq has a lifetime equal the device itself - so an > > obvious cnadidate for devm_request_irq. > > If we do not introduce it now we will see a revisit of this later. > > I can be convinced to wait with this as we will have to do much more in > > each driver, but I cannot see any good arguments to avoid the more > > modern way to use devm_request_irq. > > I'll change this in atmel_hdlcd and maybe I can find trivial cases where > devm_request_irq() can be used. But drivers that had an uninstall callback > before should not have the cleanup logic altered by a patch as this one. I > suspect that most of the IRQ cleanup > is actually a vblank cleanup and should be done in response to > drm_vblank_init(). But that's again not something for this patchset here. We > cannot change multiple things at once and still expect any of it to work. > > I welcome the use of devm_ et al. But these changes are better done in a > per-driver patchset that changes all of the driver to managed release. Fair enough, and fine with me. I have yet to read through all patches - will do so in the coming week. Sam
Re: [PATCH v4 2/3] drm/loongson: Add GPIO and I2C driver for loongson drm.
Hi lichenyang, On Fri, Jul 30, 2021 at 05:41:47PM +0800, lichenyang wrote: > Implement use GPIO and I2C driver to detect connector > and fetch EDID via DDC. > > v3: > - Change some driver log to the drm_ version. > > v2: > - Optimize the error handling process. > - Delete loongson_i2c_bus_match and loongson_i2c_add function. > - Optimize part of the code flow. > > Signed-off-by: lichenyang I will return later with more detailed feedback. One high-level comment is that all the i2c support would be much better modelled as a bridge. And then you could use the bridge_connector. Sam
Re: [PATCH v4 2/2] dt-bindings: mediatek: add force_dsi_end_without_null
Hi, Jitao:
Jitao Shi 於 2021年8月1日 週日 下午12:06寫道：
Move this patch before the patch "drm/mediatek: force hsa hbp hfp
packets multiple of lanenum to avoid screen shift",
and this patch's title should be "dt-bindings: drm/bridge: anx7625:
add force_dsi_end_without_null"
Regards,
Chun-Kuang.
>
> The force_dsi_end_without_null requires the dsi host ent at
> the same time in line.
>
> Signed-off-by: Jitao Shi
> ---
> .../bindings/display/bridge/analogix,anx7625.yaml | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index ab48ab2f4240..8b868a6a3d60 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -43,6 +43,11 @@ properties:
>vdd33-supply:
> description: Regulator that provides the supply 3.3V power.
>
> + force_dsi_end_without_null:
> +description: |
> + Requires the dsi host send the dsi packets on all lanes aligned
> + at the end.
> +
>ports:
> $ref: /schemas/graph.yaml#/properties/ports
>
> @@ -87,6 +92,7 @@ examples:
> vdd10-supply = <&pp1000_mipibrdg>;
> vdd18-supply = <&pp1800_mipibrdg>;
> vdd33-supply = <&pp3300_mipibrdg>;
> +force_dsi_end_without_null;
>
> ports {
> #address-cells = <1>;
> --
> 2.25.1
[PATCH v5 1/2] dt-bindings: drm/bridge: anx7625: add force_dsi_end_without_null
The force_dsi_end_without_null requires the dsi host ent at
the same time in line.
Signed-off-by: Jitao Shi
---
.../bindings/display/bridge/analogix,anx7625.yaml | 6 ++
1 file changed, 6 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index ab48ab2f4240..8b868a6a3d60 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -43,6 +43,11 @@ properties:
vdd33-supply:
description: Regulator that provides the supply 3.3V power.
+ force_dsi_end_without_null:
+description: |
+ Requires the dsi host send the dsi packets on all lanes aligned
+ at the end.
+
ports:
$ref: /schemas/graph.yaml#/properties/ports
@@ -87,6 +92,7 @@ examples:
vdd10-supply = <&pp1000_mipibrdg>;
vdd18-supply = <&pp1800_mipibrdg>;
vdd33-supply = <&pp3300_mipibrdg>;
+force_dsi_end_without_null;
ports {
#address-cells = <1>;
--
2.25.1
[PATCH v5 0/2] force hsa hbp hfp packets multiple of lanenum to avoid screen shift
Changes since v4: - Move "dt-bindings: drm/bridge: anx7625: add force_dsi_end_without_null" before "drm/mediatek: force hsa hbp hfp packets multiple of lanenum to avoid". - Retitle "dt-bindings: drm/bridge: anx7625: add force_dsi_end_without_null". Jitao Shi (2): dt-bindings: drm/bridge: anx7625: add force_dsi_end_without_null drm/mediatek: force hsa hbp hfp packets multiple of lanenum to avoid screen shift .../bindings/display/bridge/analogix,anx7625.yaml | 6 ++ drivers/gpu/drm/mediatek/mtk_dsi.c | 13 + 2 files changed, 19 insertions(+) -- 2.25.1
[PATCH v5 2/2] drm/mediatek: force hsa hbp hfp packets multiple of lanenum to avoid screen shift
The bridge chip ANX7625 requires the packets on lanes aligned at the end,
or ANX7625 will shift the screen.
Signed-off-by: Jitao Shi
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index ae403c67cbd9..4735e0092ffe 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -194,6 +194,8 @@ struct mtk_dsi {
struct clk *hs_clk;
u32 data_rate;
+ /* force dsi line end without dsi_null data */
+ bool force_dsi_end_without_null;
unsigned long mode_flags;
enum mipi_dsi_pixel_format format;
@@ -499,6 +501,13 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
DRM_WARN("HFP + HBP less than d-phy, FPS will under 60Hz\n");
}
+ if (dsi->force_dsi_end_without_null) {
+ horizontal_sync_active_byte =
roundup(horizontal_sync_active_byte, dsi->lanes) - 2;
+ horizontal_frontporch_byte =
roundup(horizontal_frontporch_byte, dsi->lanes) - 2;
+ horizontal_backporch_byte = roundup(horizontal_backporch_byte,
dsi->lanes) - 2;
+ horizontal_backporch_byte -= (vm->hactive * dsi_tmp_buf_bpp +
2) % dsi->lanes;
+ }
+
writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC);
writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
@@ -1095,6 +1104,10 @@ static int mtk_dsi_probe(struct platform_device *pdev)
dsi->bridge.of_node = dev->of_node;
dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
+ if (dsi->next_bridge)
+ dsi->force_dsi_end_without_null =
of_property_read_bool(dsi->next_bridge->of_node,
+
"force_dsi_end_without_null");
+
drm_bridge_add(&dsi->bridge);
ret = component_add(&pdev->dev, &mtk_dsi_component_ops);
--
2.25.1
[PATCH v2] drm:Fixes the following checkpatch error:
ERROR: do not initialise statics to false
FILE: :drivers/gpu/drm/msm/msm_drv.c:21:
static bool reglog = false;
FILE: :drivers/gpu/drm/msm/msm_drv.c:31:
-bool dumpstate = false;
Signed-off-by: zhaoxiao
---
v2: add the more detailed patch description
drivers/gpu/drm/msm/msm_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9b8fa2ad0d84..d9ca4bc9620b 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -59,7 +59,7 @@ static const struct drm_mode_config_helper_funcs
mode_config_helper_funcs = {
};
#ifdef CONFIG_DRM_MSM_REGISTER_LOGGING
-static bool reglog = false;
+static bool reglog;
MODULE_PARM_DESC(reglog, "Enable register read/write logging");
module_param(reglog, bool, 0600);
#else
@@ -76,7 +76,7 @@ static char *vram = "16m";
MODULE_PARM_DESC(vram, "Configure VRAM size (for devices without
IOMMU/GPUMMU)");
module_param(vram, charp, 0);
-bool dumpstate = false;
+bool dumpstate;
MODULE_PARM_DESC(dumpstate, "Dump KMS state on errors");
module_param(dumpstate, bool, 0600);
--
2.20.1
[PATCH] drm:This patch fixes the checkpatch.pl error to msm_drv.c
ERROR: do not initialise statics to false
Signed-off-by: zhaoxiao
---
drivers/gpu/drm/msm/msm_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9b8fa2ad0d84..d9ca4bc9620b 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -59,7 +59,7 @@ static const struct drm_mode_config_helper_funcs
mode_config_helper_funcs = {
};
#ifdef CONFIG_DRM_MSM_REGISTER_LOGGING
-static bool reglog = false;
+static bool reglog;
MODULE_PARM_DESC(reglog, "Enable register read/write logging");
module_param(reglog, bool, 0600);
#else
@@ -76,7 +76,7 @@ static char *vram = "16m";
MODULE_PARM_DESC(vram, "Configure VRAM size (for devices without
IOMMU/GPUMMU)");
module_param(vram, charp, 0);
-bool dumpstate = false;
+bool dumpstate;
MODULE_PARM_DESC(dumpstate, "Dump KMS state on errors");
module_param(dumpstate, bool, 0600);
--
2.20.1
Re: [BUG] video: fbdev: sis: possible uninitialized-variable access in SiS_SetCRT2FIFO_300()
Thanks for your feedback! Should I commit a patch to fix this problem? Best wishes, Tuo Li On 2021/8/2 1:59, Sam Ravnborg wrote: Hi Tuo Li, On Sat, Jul 31, 2021 at 02:28:39PM +0800, Li Tuo wrote: Hello, Our static analysis tool finds a possible uninitialized-variable access in the sis driver in Linux 5.14.0-rc3: At the beginning of the function SiS_SetCRT2FIFO_300(), the variable modeidindex is not initialized. If the following conditions are false, it remains uninitialized. 5346: if(!SiS_Pr->CRT1UsesCustomMode) 5438: if(!SiS_Pr->UseCustomMode) But it is accessed at: 5466: colorth = SiS_GetColorDepth(SiS_Pr,CRT2ModeNo,modeidindex) >> 1; I am not quite sure whether this possible uninitialized-variable access is real and how to fix it if it is real. Any feedback would be appreciated, thanks! First, the report looks correct. There is a path where modeindex may not be initilized. But I find it very hard to care for such an ancient driver. If this was somethign we hit is real life we had heard about it - and the risk of introducing bugs is higher than the the cance that this fixes a real life bug. So my advice, find something more relevant to look at. Sam
Re: [PATCH] drm/amdgpu: fix possible null-pointer dereference in amdgpu_ttm_tt_populate()
Thanks for your feedback! We will remove the null tests according to
your advice and prepare a V2 patch.
Best wishes,
Tuo Li
On 2021/8/2 1:19, Christian König wrote:
Am 31.07.21 um 10:04 schrieb Tuo Li:
The variable ttm is assigned to the variable gtt, and the variable gtt
is checked in:
if (gtt && gtt->userptr)
This indicates that both ttm and gtt can be NULL.
If so, a null-pointer dereference will occur:
if (ttm->page_flags & TTM_PAGE_FLAG_SG)
Also, some null-pointer dereferences will occur in the function
ttm_pool_alloc() which is called in:
return ttm_pool_alloc(&adev->mman.bdev.pool, ttm, ctx);
To fix these possible null-pointer dereferences, the function returns
-EINVAL when ttm is NULL.
NAK, the NULL test is just a leftover from when the objects where
distinct.
Please remove the NULL test instead.
Regards,
Christian.
Reported-by: TOTE Robot
Signed-off-by: Tuo Li
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3a55f08e00e1..80440f799c09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1120,8 +1120,11 @@ static int amdgpu_ttm_tt_populate(struct
ttm_device *bdev,
struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ if (ttm == NULL)
+ return -EINVAL;
+
/* user pages are bound by amdgpu_ttm_tt_pin_userptr() */
- if (gtt && gtt->userptr) {
+ if (gtt->userptr) {
ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!ttm->sg)
return -ENOMEM;
[PATCH] drm/vmwgfx: Make use of PFN_ALIGN/PFN_UP helper macro
it's a refactor to make use of PFN_ALIGN/PFN_UP helper macro
Signed-off-by: Cai Huoqing
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 5 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 3 +--
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_mob.c | 5 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 3 +--
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 3 +--
7 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 362f56d5b12b..9e3e1429db94 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -405,7 +405,7 @@ static size_t vmw_bo_acc_size(struct vmw_private *dev_priv,
size_t size,
bool user)
{
static size_t struct_size, user_struct_size;
- size_t num_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+ size_t num_pages = PFN_UP(size);
size_t page_array_size = ttm_round_pot(num_pages * sizeof(void *));
if (unlikely(struct_size == 0)) {
@@ -474,7 +474,6 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv,
unsigned long size,
struct ttm_placement *placement,
struct ttm_buffer_object **p_bo)
{
- unsigned npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
struct ttm_operation_ctx ctx = { false, false };
struct ttm_buffer_object *bo;
size_t acc_size;
@@ -485,7 +484,7 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv,
unsigned long size,
return -ENOMEM;
acc_size = ttm_round_pot(sizeof(*bo));
- acc_size += ttm_round_pot(npages * sizeof(void *));
+ acc_size += ttm_round_pot(PFN_UP(size) * sizeof(void *));
acc_size += ttm_round_pot(sizeof(struct ttm_tt));
ret = ttm_mem_global_alloc(&ttm_mem_glob, acc_size, &ctx);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
index 9656d4a2abff..891e58b951a6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
@@ -802,7 +802,7 @@ static int vmw_cmdbuf_alloc_space(struct vmw_cmdbuf_man
*man,
{
struct vmw_cmdbuf_alloc_info info;
- info.page_size = PAGE_ALIGN(size) >> PAGE_SHIFT;
+ info.page_size = PFN_UP(size);
info.node = node;
info.done = false;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
index c84a16c1def0..17a98db00017 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
@@ -607,8 +607,7 @@ struct vmw_resource *vmw_cotable_alloc(struct vmw_private
*dev_priv,
if (num_entries < co_info[type].min_initial_entries) {
vcotbl->res.backup_size = co_info[type].min_initial_entries *
co_info[type].size;
- vcotbl->res.backup_size =
- (vcotbl->res.backup_size + PAGE_SIZE - 1) & PAGE_MASK;
+ vcotbl->res.backup_size = PFN_ALIGN(vcotbl->res.backup_size);
}
vcotbl->scrubbed = true;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 2ddf4932d62c..9679d2fa40a5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -100,7 +100,7 @@ static int vmw_cursor_update_bo(struct vmw_private
*dev_priv,
int ret;
kmap_offset = 0;
- kmap_num = (width*height*4 + PAGE_SIZE - 1) >> PAGE_SHIFT;
+ kmap_num = PFN_UP(width*height*4);
ret = ttm_bo_reserve(&bo->base, true, false, NULL);
if (unlikely(ret != 0)) {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
index c8e578f63c9c..4d02ae210805 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
@@ -256,8 +256,7 @@ static int vmw_otable_batch_setup(struct vmw_private
*dev_priv,
if (!otables[i].enabled)
continue;
- otables[i].size =
- (otables[i].size + PAGE_SIZE - 1) & PAGE_MASK;
+ otables[i].size = PFN_ALIGN(otables[i].size);
bo_size += otables[i].size;
}
@@ -385,7 +384,7 @@ static unsigned long vmw_mob_calculate_pt_pages(unsigned
long data_pages)
while (likely(data_size > PAGE_SIZE)) {
data_size = DIV_ROUND_UP(data_size, PAGE_SIZE);
data_size *= VMW_PPN_SIZE;
- tot_size += (data_size + PAGE_SIZE - 1) & PAGE_MASK;
+ tot_size += PFN_ALIGN(data_size);
}
return tot_size >> PAGE_SHIFT;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 3b6f6044c325..8d1e869cc196 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -353,8 +3
Re: [PATCH v2 14/14] drm/mediatek: add mediatek-drm of vdosys1 support for MT8195
Hi, Nancy:
On Thu, 2021-07-22 at 17:45 +0800, Nancy.Lin wrote:
> Add driver data of mt8195 vdosys1 to mediatek-drm and modify drm for
> multi-mmsys support. The two mmsys (vdosys0 and vdosys1) will bring
> up two drm drivers, only one drm driver register as the drm device.
> Each drm driver binds its own component. The first bind drm driver
> will allocate the drm device, and the last bind drm driver registers
> the drm device to drm core. Each crtc path is created with the
> corresponding drm driver data.
>
> Signed-off-by: Nancy.Lin
> ---
> drivers/gpu/drm/mediatek/mtk_disp_merge.c | 4 +
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 18 +-
> drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 3 +-
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 15 +
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 378
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 8 +-
> 7 files changed, 356 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> index 768c282d2d63..829570308761 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> @@ -4,6 +4,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -106,6 +107,9 @@ void mtk_merge_stop(struct device *dev)
> struct mtk_disp_merge *priv = dev_get_drvdata(dev);
>
> mtk_ddp_write(NULL, 0x0, &priv->cmdq_reg, priv->regs,
> DISP_REG_MERGE_CTRL);
> +
> + if (priv->async_clk)
> + device_reset_optional(dev);
Move this to the merge patch [1].
[1]
https://patchwork.kernel.org/project/linux-mediatek/patch/20210729170737.21424-7-jason-jh@mediatek.com/
> }
>
> static int mtk_merge_check_params(struct mtk_merge_config_struct
> *merge_config)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 40df2c823187..3324fa1a9e8c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -737,21 +737,28 @@ static int mtk_drm_crtc_init_comp_planes(struct
> drm_device *drm_dev,
> }
>
> int mtk_drm_crtc_create(struct drm_device *drm_dev,
> - const enum mtk_ddp_comp_id *path, unsigned int path_len)
> + const enum mtk_ddp_comp_id *path, unsigned int path_len,
> + int priv_data_index)
> {
> struct mtk_drm_private *priv = drm_dev->dev_private;
> struct device *dev = drm_dev->dev;
> struct mtk_drm_crtc *mtk_crtc;
> unsigned int num_comp_planes = 0;
> - int pipe = priv->num_pipes;
> int ret;
> int i;
> bool has_ctm = false;
> uint gamma_lut_size = 0;
> + struct drm_crtc *tmp;
> + int crtc_i = 0;
>
> if (!path)
> return 0;
>
> + priv = priv->all_drm_private[priv_data_index];
> +
> + drm_for_each_crtc(tmp, drm_dev)
> + crtc_i++;
> +
> for (i = 0; i < path_len; i++) {
> enum mtk_ddp_comp_id comp_id = path[i];
> struct device_node *node;
> @@ -760,7 +767,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
> if (!node) {
> dev_info(dev,
>"Not creating crtc %d because component %d is
> disabled or missing\n",
> - pipe, comp_id);
> + crtc_i, comp_id);
> return 0;
> }
> }
> @@ -816,19 +823,18 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
>
> for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> ret = mtk_drm_crtc_init_comp_planes(drm_dev, mtk_crtc, i,
> - pipe);
> + crtc_i);
> if (ret)
> return ret;
> }
>
> - ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, pipe);
> + ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, crtc_i);
> if (ret < 0)
> return ret;
>
> if (gamma_lut_size)
> drm_mode_crtc_set_gamma_size(&mtk_crtc->base, gamma_lut_size);
> drm_crtc_enable_color_mgmt(&mtk_crtc->base, 0, has_ctm, gamma_lut_size);
> - priv->num_pipes++;
> mutex_init(&mtk_crtc->hw_lock);
>
> #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> index 66d1cf03dfe8..0646fafffd8b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> @@ -31,7 +31,8 @@
> void mtk_drm_crtc_commit(struct drm_crtc *crtc);
> int mtk_drm_crtc_create(struct drm_device *drm_dev,
> const enum mtk_ddp_comp_id *path,
> - unsigned int path_len);
> + unsigned int path_len,
> +
[PATCH] drm/vmwgfx: Replace "vmw_num_pages" with "PFN_UP"
we counld use PFN_UP instead of vmw_num_pages()
Signed-off-by: Cai Huoqing
---
.../gpu/drm/vmwgfx/device_include/vm_basic_types.h | 13 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++---
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/device_include/vm_basic_types.h
b/drivers/gpu/drm/vmwgfx/device_include/vm_basic_types.h
index 35bd2852189f..1f6e3bbc6605 100644
--- a/drivers/gpu/drm/vmwgfx/device_include/vm_basic_types.h
+++ b/drivers/gpu/drm/vmwgfx/device_include/vm_basic_types.h
@@ -97,19 +97,18 @@ typedef __attribute__((aligned(32))) struct
MKSGuestStatInfoEntry {
} MKSGuestStatInfoEntry;
#define INVALID_PPN64 ((PPN64)0x000fULL)
-#define vmw_num_pages(size) (PAGE_ALIGN(size) >> PAGE_SHIFT)
#define MKS_GUEST_STAT_INSTANCE_DESC_LENGTH 1024
#define MKS_GUEST_STAT_INSTANCE_MAX_STATS 4096
-#define MKS_GUEST_STAT_INSTANCE_MAX_STAT_PPNS\
- (vmw_num_pages(MKS_GUEST_STAT_INSTANCE_MAX_STATS * \
+#define MKS_GUEST_STAT_INSTANCE_MAX_STAT_PPNS\
+ (PFN_UP(MKS_GUEST_STAT_INSTANCE_MAX_STATS * \
sizeof(MKSGuestStatCounterTime)))
-#define MKS_GUEST_STAT_INSTANCE_MAX_INFO_PPNS\
- (vmw_num_pages(MKS_GUEST_STAT_INSTANCE_MAX_STATS * \
+#define MKS_GUEST_STAT_INSTANCE_MAX_INFO_PPNS\
+ (PFN_UP(MKS_GUEST_STAT_INSTANCE_MAX_STATS * \
sizeof(MKSGuestStatInfoEntry)))
#define MKS_GUEST_STAT_AVERAGE_NAME_LENGTH 40
-#define MKS_GUEST_STAT_INSTANCE_MAX_STRS_PPNS\
- (vmw_num_pages(MKS_GUEST_STAT_INSTANCE_MAX_STATS * \
+#define MKS_GUEST_STAT_INSTANCE_MAX_STRS_PPNS\
+ (PFN_UP(MKS_GUEST_STAT_INSTANCE_MAX_STATS * \
MKS_GUEST_STAT_AVERAGE_NAME_LENGTH))
/*
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index ed9c7b3a1e08..e50fb82a3030 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1016,9 +1016,9 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
struct page *page;
MKSGuestStatInstanceDescriptor *pdesc;
- const size_t num_pages_stat = vmw_num_pages(arg->stat_len);
- const size_t num_pages_info = vmw_num_pages(arg->info_len);
- const size_t num_pages_strs = vmw_num_pages(arg->strs_len);
+ const size_t num_pages_stat = PFN_UP(arg->stat_len);
+ const size_t num_pages_info = PFN_UP(arg->info_len);
+ const size_t num_pages_strs = PFN_UP(arg->strs_len);
long desc_len;
long nr_pinned_stat;
long nr_pinned_info;
--
2.25.1
RE: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
> -Original Message- > From: Daniel Vetter > Sent: Friday, July 30, 2021 6:26 PM > To: Kasireddy, Vivek > Cc: dri-devel@lists.freedesktop.org; Daniel Vetter ; Gerd > Hoffmann ; Pekka Paalanen ; > Simon Ser ; Michel Dänzer ; > Zhang, Tina ; Kim, Dongwon > > Subject: Re: [RFC v1 0/4] drm: Add support for > DRM_CAP_DEFERRED_OUT_FENCE capability > > On Thu, Jul 29, 2021 at 01:16:55AM -0700, Vivek Kasireddy wrote: > > By separating the OUT_FENCE signalling from pageflip completion allows > > a Guest compositor to start a new repaint cycle with a new buffer > > instead of waiting for the old buffer to be free. > > > > This work is based on the idea/suggestion from Simon and Pekka. > > > > This capability can be a solution for this issue: > > https://gitlab.freedesktop.org/wayland/weston/-/issues/514 > > > > Corresponding Weston MR: > > https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/668 > > Uh I kinda wanted to discuss this a bit more before we jump into typing code, > but well I guess not that much work yet. > > So maybe I'm not understanding the problem, but I think the fundamental > underlying issue is that with KMS you can have at most 2 buffers in-flight, > due to our queue depth limit of 1 pending flip. > > Unfortunately that means for virtual hw where it takes a few more > steps/vblanks until the framebuffer actually shows up on screen and is > scanned out, we suffer deeply. The usual fix for that is to drop the latency > and increase throughput, and have more buffers in-flight. Which this patch > tries to do. > > Now I think where we go wrong here is that we're trying to hack this up by > defining different semantics for the out-fence and for the drm-event. Imo > that's wrong, they're both meant to show eactly the same thing: > - when is the new frame actually visible to the user (as in, eyeballs in a > human head, preferrably, not the time when we've handed the buffer off > to the virtual hw) > - when is the previous buffer no longer being used by the scanout hw > > We do cheat a bit right now in so far that we assume they're both the same, > as in, panel-side latency is currently the compositor's problem to figure out. > > So for virtual hw I think the timestamp and even completion really need to > happen only when the buffer has been pushed through the entire > virtualization chain, i.e. ideally we get the timestamp from the kms driver > from the host side. Currently that's not done, so this is most likely quite > broken already (virtio relies on the no-vblank auto event sending, which > definitely doesn't wait for anything, or I'm completely missing something). Agree. One lesson we got from previous direct-display related work is that using host hardware event is kind of "must". Otherwise, problems like flickering or tearing or frame drop will come out. Besides, as the wayland-ui is working as a weston client, it needs to have more than 2 buffers to support the full-frame redraw. I tried the Weston-simple-dmabuf-egl with 2 buffers and it was bad. BR, Tina > > I think instead of hacking up some ill-defined 1.5 queue depth support, what > we should do is support queue depth > 1 properly. So: > > - Change atomic to support queue depth > 1, this needs to be a per-driver > thing due to a bunch of issues in driver code. Essentially drivers must > never look at obj->state pointers, and only ever look up state through > the passed-in drm_atomic_state * update container. > > - Aside: virtio should loose all it's empty hooks, there's no point in > that. > > - We fix virtio to send out the completion event at the end of this entire > pipeline, i.e. virtio code needs to take care of sending out the > crtc_state->event correctly. > > - We probably also want some kind of (maybe per-crtc) recommended queue > depth property so compositors know how many buffers to keep in flight. > Not sure about that. > > It's a bit more work, but also a lot less hacking around infrastructure in > dubious ways. > > Thoughts? > > Cheers, Daniel > > > > > Cc: Daniel Vetter > > Cc: Gerd Hoffmann > > Cc: Pekka Paalanen > > Cc: Simon Ser > > Cc: Michel Dänzer > > Cc: Tina Zhang > > Cc: Dongwon Kim > > > > Vivek Kasireddy (4): > > drm: Add a capability flag to support deferred out_fence signalling > > virtio-gpu uapi: Add VIRTIO_GPU_F_OUT_FENCE feature > > drm/virtio: Add VIRTIO_GPU_CMD_RESOURCE_OUT_FENCE cmd > > drm/virtio: Probe and implement VIRTIO_GPU_F_OUT_FENCE feature > > > > drivers/gpu/drm/drm_file.c | 11 +++--- > > drivers/gpu/drm/drm_ioctl.c | 3 ++ > > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 1 + > > drivers/gpu/drm/virtio/virtgpu_drv.c | 1 + > > drivers/gpu/drm/virtio/virtgpu_drv.h | 6 > > drivers/gpu/drm/virtio/virtgpu_fence.c | 9 + > > drivers/gpu/drm/virtio/virtgpu_kms.c | 10 -- > > drivers/gpu/drm/virtio/virtgpu_plane.c | 44 +++- > > drivers/gpu/
RE: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
Hi Daniel, > > On Thu, Jul 29, 2021 at 01:16:55AM -0700, Vivek Kasireddy wrote: > > By separating the OUT_FENCE signalling from pageflip completion allows > > a Guest compositor to start a new repaint cycle with a new buffer > > instead of waiting for the old buffer to be free. > > > > This work is based on the idea/suggestion from Simon and Pekka. > > > > This capability can be a solution for this issue: > > https://gitlab.freedesktop.org/wayland/weston/-/issues/514 > > > > Corresponding Weston MR: > > https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/668 > > Uh I kinda wanted to discuss this a bit more before we jump into typing > code, but well I guess not that much work yet. [Kasireddy, Vivek] Right, it wasn't a lot of work :) > > So maybe I'm not understanding the problem, but I think the fundamental > underlying issue is that with KMS you can have at most 2 buffers > in-flight, due to our queue depth limit of 1 pending flip. [Kasireddy, Vivek] Let me summarize the problem again from the perspective of both the Host (Weston) and Guest (Weston) compositors assuming a refresh-rate of 60 -- which implies the Vblank/Vsync is generated every ~16.66 ms. Host compositor: - After a pageflip completion event, it starts its next repaint cycle by waiting for 9 ms and then submits the atomic commit and at the tail end of its cycle sends a frame callback event to all its clients (who registered and submitted frames) indicating to them to start their next redraw -- giving them at-least ~16 ms to submit a new frame to be included in its next repaint. Why a configurable 9 ms delay is needed is explained in Pekka's blog post here: https://ppaalanen.blogspot.com/2015/02/weston-repaint-scheduling.html - It'll send a wl_buffer.release event for a client submitted previous buffer only when the client has submitted a new buffer and: a) When it hasn't started its repaint cycle yet OR b) When it clears its old state after it gets a pageflip completion event -- if it had flipped the client's buffer onto a hardware plane. Guest compositor: - After a pageflip completion is sent by Guest KMS, it takes about 10-12 ms for the Guest compositor to submit a new atomic commit. This time of 10-12 ms includes the 9 ms wait -- just like the Host compositor -- for its clients to submit new buffers. - When it gets a pageflip completion, it assumes that the previously submitted buffer is free for re-use and uses it again -- resulting in the usage of only 2 out of a maximum of 4 backbuffers included as part of the Mesa GBM surface implementation. Guest KMS/Virtio-gpu/Qemu Wayland UI: - Because no_vblank=true for Guest KMS and since the vblank event (which also serves as the pageflip completion event for user-space) is sent right away after atomic commit, as Gerd said, we use an internal dma-fence to block/wait the Guest KMS until we know for sure that the Host is completely done using the buffer. To ensure this, we signal the dma-fence only after the Host compositor sends a wl_buffer.release event or an equivalent signal. The goal: - Maintain full framerate even when the Guest scanout FB is flipped onto a hardware plane on the Host -- regardless of either compositor's scheduling policy -- without making any copies and ensuring that both Host and Guest are not accessing the buffer at the same time. The problem: - If the Host compositor flips the client's buffer (in this case Guest compositor's buffer) onto a hardware plane, then it can send a wl_buffer.release event for the previous buffer only after it gets a pageflip completion. And, if the Guest compositor takes 10-12 ms to submit a new buffer and given the fact that the Host compositor waits only for 9 ms, the Guest compositor will miss the Host's repaint cycle resulting in halved frame-rate. The solution: - To ensure full framerate, the Guest compositor has to start it's repaint cycle (including the 9 ms wait) when the Host compositor sends the frame callback event to its clients. In order for this to happen, the dma-fence that the Guest KMS waits on -- before sending pageflip completion -- cannot be tied to a wl_buffer.release event. This means that, the Guest compositor has to be forced to use a new buffer for its next repaint cycle when it gets a pageflip completion. - The Weston MR I linked above does this by getting an out_fence fd and taking a reference on all the FBs included in the atomic commit forcing the compositor to use new FBs for its next repaint cycle. It releases the references when the out_fence is signalled later when the Host compositor sends a wl_buffer.release event. > > Unfortunately that means for virtual hw where it takes a few more > steps/vblanks until the framebuffer actually shows up on screen and is > scanned out, we suffer deeply. The usual fix for that is to drop the > latency and increase throughput, and have more buffers in-flight. Which > this patch tries to do. > > Now I think where we go
