Re: [PATCH] drm/amd/display: rewrite the check for mods
This breaks the "size" out-parameter.
[PATCH] drm/atomic: add quirks for blind save/restore
Two quirks to make blind atomic save/restore [1] work correctly: - Mark the DPMS property as immutable for atomic clients, since atomic clients cannot change it. - Allow user-space to set content protection to "enabled", interpret it as "desired". [1]: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/3794 Signed-off-by: Simon Ser --- I don't have the motivation to write IGT tests for this. drivers/gpu/drm/drm_atomic_uapi.c | 5 +++-- drivers/gpu/drm/drm_property.c| 7 +++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index c06d0639d552..95363aac7f69 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -741,8 +741,9 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, state->scaling_mode = val; } else if (property == config->content_protection_property) { if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) { - drm_dbg_kms(dev, "only drivers can set CP Enabled\n"); - return -EINVAL; + /* Degrade ENABLED to DESIRED so that blind atomic +* save/restore works as intended. */ + val = DRM_MODE_CONTENT_PROTECTION_DESIRED; } state->content_protection = val; } else if (property == config->hdcp_content_type_property) { diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index dfec479830e4..dde42986f8cb 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -474,7 +474,14 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev, return -ENOENT; strscpy_pad(out_resp->name, property->name, DRM_PROP_NAME_LEN); + out_resp->flags = property->flags; + if (file_priv->atomic && property == dev->mode_config.dpms_property) { + /* Quirk: indicate that the legacy DPMS property is not +* writable from atomic user-space, so that blind atomic +* save/restore works as intended. */ + out_resp->flags |= DRM_MODE_PROP_IMMUTABLE; + } value_count = property->num_values; values_ptr = u64_to_user_ptr(out_resp->values_ptr); -- 2.38.1
Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export
Sometime back Dan also reported the same issue[1] where I do mentioned that fput()-->dma_buf_file_release() will remove it from the list. But it seems that I failed to notice fput() here calls the dma_buf_file_release() asynchronously i.e. dmabuf that is accessed in the close path is already freed. Am I wrong here? Should we have the __fput_sync(file) here instead of just fput(file) which can solve this problem? [1]https://lore.kernel.org/all/20220516084704.GG29930@kadam/ Thanks, Charan On 11/17/2022 11:51 AM, Gaosheng Cui wrote: > Smatch report warning as follows: > > drivers/dma-buf/dma-buf.c:681 dma_buf_export() warn: > '&dmabuf->list_node' not removed from list > > If dma_buf_stats_setup() fails in dma_buf_export(), goto err_sysfs > and dmabuf will be freed, but dmabuf->list_node will not be removed > from db_list.head, then list traversal may cause UAF. > > Fix by removeing it from db_list.head before free(). > > Fixes: ef3a6b70507a ("dma-buf: call dma_buf_stats_setup after dmabuf is in > valid list") > Signed-off-by: Gaosheng Cui > --- > drivers/dma-buf/dma-buf.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index b809513b03fe..6848f50226d5 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -675,6 +675,9 @@ struct dma_buf *dma_buf_export(const struct > dma_buf_export_info *exp_info) > return dmabuf; > > err_sysfs: > + mutex_lock(&db_list.lock); > + list_del(&dmabuf->list_node); > + mutex_unlock(&db_list.lock); > /* >* Set file->f_path.dentry->d_fsdata to NULL so that when >* dma_buf_release() gets invoked by dentry_ops, it exits
[PATCH] drm/amd/display: rewrite the check for mods
When the *mods is NULL, it should be better to return error immediately, rather than continue with redundant operations. Signed-off-by: Jiasheng Jiang --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c index e6854f7270a6..05efc76b2226 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c @@ -638,11 +638,14 @@ static int get_plane_modifiers(struct amdgpu_device *adev, unsigned int plane_ty return 0; *mods = kmalloc(capacity * sizeof(uint64_t), GFP_KERNEL); + if (!*mods) + return -ENOMEM; + if (plane_type == DRM_PLANE_TYPE_CURSOR) { add_modifier(mods, &size, &capacity, DRM_FORMAT_MOD_LINEAR); add_modifier(mods, &size, &capacity, DRM_FORMAT_MOD_INVALID); - return *mods ? 0 : -ENOMEM; + return 0; } switch (adev->family) { @@ -671,9 +674,6 @@ static int get_plane_modifiers(struct amdgpu_device *adev, unsigned int plane_ty /* INVALID marks the end of the list. */ add_modifier(mods, &size, &capacity, DRM_FORMAT_MOD_INVALID); - if (!*mods) - return -ENOMEM; - return 0; } -- 2.25.1
linux-next: build failure after merge of the drm-misc tree
Hi all, After merging the drm-misc tree, today's linux-next build (powerpc ppc44x_defconfig) failed like this: ld: drivers/video/fbdev/core/fbmon.o: in function `fb_modesetting_disabled': fbmon.c:(.text+0x1e4): multiple definition of `fb_modesetting_disabled'; drivers/video/fbdev/core/fbmem.o:fbmem.c:(.text+0x1bac): first defined here ld: drivers/video/fbdev/core/fbcmap.o: in function `fb_modesetting_disabled': fbcmap.c:(.text+0x478): multiple definition of `fb_modesetting_disabled'; drivers/video/fbdev/core/fbmem.o:fbmem.c:(.text+0x1bac): first defined here ld: drivers/video/fbdev/core/fbsysfs.o: in function `fb_modesetting_disabled': fbsysfs.c:(.text+0xb64): multiple definition of `fb_modesetting_disabled'; drivers/video/fbdev/core/fbmem.o:fbmem.c:(.text+0x1bac): first defined here ld: drivers/video/fbdev/core/modedb.o: in function `fb_modesetting_disabled': modedb.c:(.text+0x129c): multiple definition of `fb_modesetting_disabled'; drivers/video/fbdev/core/fbmem.o:fbmem.c:(.text+0x1bac): first defined here ld: drivers/video/fbdev/core/fbcvt.o: in function `fb_modesetting_disabled': fbcvt.c:(.text+0x0): multiple definition of `fb_modesetting_disabled'; drivers/video/fbdev/core/fbmem.o:fbmem.c:(.text+0x1bac): first defined here Caused by commit 0ba2fa8cbd29 ("fbdev: Add support for the nomodeset kernel parameter") This build does not have CONFIG_VIDEO_NOMODESET set. I applied the following patch for today. From 63f957a050c62478ed1348c5b204bc65c68df4d7 Mon Sep 17 00:00:00 2001 From: Stephen Rothwell Date: Thu, 17 Nov 2022 18:19:22 +1100 Subject: [PATCH] fix up for "fbdev: Add support for the nomodeset kernel parameter" Signed-off-by: Stephen Rothwell --- include/linux/fb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/fb.h b/include/linux/fb.h index 3a822e4357b1..ea421724f733 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -807,7 +807,7 @@ extern int fb_find_mode(struct fb_var_screeninfo *var, #if defined(CONFIG_VIDEO_NOMODESET) bool fb_modesetting_disabled(const char *drvname); #else -bool fb_modesetting_disabled(const char *drvname) +static inline bool fb_modesetting_disabled(const char *drvname) { return false; } -- 2.35.1 -- Cheers, Stephen Rothwell pgp_OcetDWn84.pgp Description: OpenPGP digital signature
[PATCH] drm: bridge: dw_hdmi: fix preference of RGB modes over YUV420
Cheap monitors sometimes advertise YUV modes they don't really have (HDMI specification mandates YUV support so even monitors without actual support will often wrongfully advertise it) which results in YUV matches and user forum complaints of a red tint to light colour display areas in common desktop environments. Moving the default RGB fall-back before YUV selection results in RGB mode matching in most cases, reducing complaints. Fixes: 6c3c719936da ("drm/bridge: synopsys: dw-hdmi: add bus format negociation") Signed-off-by: Guillaume BRUN Tested-by: Christian Hewitt --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 40d8ca37f5bc..aa51c61a78c7 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2720,6 +2720,9 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, * if supported. In any case the default RGB888 format is added */ + /* Default 8bit RGB fallback */ + output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24; + if (max_bpc >= 16 && info->bpc == 16) { if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444) output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48; @@ -2753,9 +2756,6 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444) output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24; - /* Default 8bit RGB fallback */ - output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24; - *num_output_fmts = i; return output_fmts; -- 2.37.3
[PATCH linux-next] drm/panel: Use device_match_of_node()
From: ye xingchen Replace the open-code with device_match_of_node(). Signed-off-by: ye xingchen --- drivers/gpu/drm/drm_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index f634371c717a..ba66ac1ad88a 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -250,7 +250,7 @@ struct drm_panel *of_drm_find_panel(const struct device_node *np) mutex_lock(&panel_lock); list_for_each_entry(panel, &panel_list, list) { - if (panel->dev->of_node == np) { + if (device_match_of_node(panel->dev, np)) { mutex_unlock(&panel_lock); return panel; } -- 2.25.1
[PATCH linux-next] drm/imx: Use device_match_of_node()
From: ye xingchen Replace the open-code with device_match_of_node(). Signed-off-by: ye xingchen --- drivers/gpu/drm/imx/imx-drm-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index e060fa6cbcb9..2e4f5af894b0 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -182,7 +182,7 @@ static int compare_of(struct device *dev, void *data) if (strcmp(dev->driver->name, "imx-ipuv3-crtc") == 0) { struct ipu_client_platformdata *pdata = dev->platform_data; - return pdata->of_node == np; + return device_match_of_node(pdata, np); } /* Special case for LDB, one device for two channels */ @@ -191,7 +191,7 @@ static int compare_of(struct device *dev, void *data) of_node_put(np); } - return dev->of_node == np; + return device_match_of_node(dev, np); } static int imx_drm_bind(struct device *dev) -- 2.25.1
[PATCH linux-next] drm/bridge: Use device_match_of_node()
From: ye xingchen Replace the open-code with device_match_of_node(). Signed-off-by: ye xingchen --- drivers/gpu/drm/drm_bridge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 1545c50fd1c8..4a8cafe2b130 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1307,7 +1307,7 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np) mutex_lock(&bridge_lock); list_for_each_entry(bridge, &bridge_list, list) { - if (bridge->of_node == np) { + if (device_match_of_node(bridge, np)) { mutex_unlock(&bridge_lock); return bridge; } -- 2.25.1
[PATCH] drm: Fix possible UAF in drm_addmap_core
smatch report warning as follows: drivers/gpu/drm/drm_bufs.c:365 drm_addmap_core() warn: '&list->head' not removed from list If drm_map_handle() fails in drm_addmap_core(), the list will be freed, but the list->head will not be removed from dev->maplist, then list traversal may cause UAF. Fix by removeing it from list->head before free(). Fixes: 9a298b2acd77 ("drm: Remove memory debugging infrastructure.") Signed-off-by: Yi Yang --- drivers/gpu/drm/drm_bufs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index fcca21e8efac..e739e577902c 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -362,6 +362,7 @@ static int drm_addmap_core(struct drm_device *dev, resource_size_t offset, if (map->type == _DRM_REGISTERS) iounmap(map->handle); kfree(map); + list_del(&list->head); kfree(list); mutex_unlock(&dev->struct_mutex); return ret; -- 2.17.1
[PATCH] drm: bridge: dw_hdmi: fix preference of RGB modes over YUV420
Cheap monitors sometimes advertise YUV modes they don't really have (HDMI specification mandates YUV support so even monitors without actual support will often wrongfully advertise it) which results in YUV matches and user forum complaints of a red tint to light colour display areas in common desktop environments. Moving the default RGB fall-back before YUV selection results in RGB mode matching in most cases, reducing complaints. Fixes: 6c3c719936da ("drm/bridge: synopsys: dw-hdmi: add bus format negociation") Signed-off-by: Guillaume BRUN Tested-by: Christian Hewitt --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 40d8ca37f5bc..aa51c61a78c7 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2720,6 +2720,9 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, * if supported. In any case the default RGB888 format is added */ + /* Default 8bit RGB fallback */ + output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24; + if (max_bpc >= 16 && info->bpc == 16) { if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444) output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48; @@ -2753,9 +2756,6 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444) output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24; - /* Default 8bit RGB fallback */ - output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24; - *num_output_fmts = i; return output_fmts; -- 2.37.3
[PATCH] backlight: pwm_bl: Drop support for legacy PWM probing
There is no in-tree user left which relies on legacy probing. So drop support for it which removes another user of the deprecated pwm_request() function. Signed-off-by: Uwe Kleine-König --- drivers/video/backlight/pwm_bl.c | 12 include/linux/pwm_backlight.h| 1 - 2 files changed, 13 deletions(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index c0523a0269ee..d0b22158cd70 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -28,7 +28,6 @@ struct pwm_bl_data { struct regulator*power_supply; struct gpio_desc*enable_gpio; unsigned intscale; - boollegacy; unsigned intpost_pwm_on_delay; unsigned intpwm_off_delay; int (*notify)(struct device *, @@ -455,7 +454,6 @@ static int pwm_backlight_probe(struct platform_device *pdev) struct platform_pwm_backlight_data defdata; struct backlight_properties props; struct backlight_device *bl; - struct device_node *node = pdev->dev.of_node; struct pwm_bl_data *pb; struct pwm_state state; unsigned int i; @@ -506,12 +504,6 @@ static int pwm_backlight_probe(struct platform_device *pdev) } pb->pwm = devm_pwm_get(&pdev->dev, NULL); - if (IS_ERR(pb->pwm) && PTR_ERR(pb->pwm) != -EPROBE_DEFER && !node) { - dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n"); - pb->legacy = true; - pb->pwm = pwm_request(data->pwm_id, "pwm-backlight"); - } - if (IS_ERR(pb->pwm)) { ret = PTR_ERR(pb->pwm); if (ret != -EPROBE_DEFER) @@ -604,8 +596,6 @@ static int pwm_backlight_probe(struct platform_device *pdev) if (IS_ERR(bl)) { dev_err(&pdev->dev, "failed to register backlight\n"); ret = PTR_ERR(bl); - if (pb->legacy) - pwm_free(pb->pwm); goto err_alloc; } @@ -639,8 +629,6 @@ static int pwm_backlight_remove(struct platform_device *pdev) if (pb->exit) pb->exit(&pdev->dev); - if (pb->legacy) - pwm_free(pb->pwm); return 0; } diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h index 06086cb93b6f..cdd2ac366bc7 100644 --- a/include/linux/pwm_backlight.h +++ b/include/linux/pwm_backlight.h @@ -8,7 +8,6 @@ #include struct platform_pwm_backlight_data { - int pwm_id; unsigned int max_brightness; unsigned int dft_brightness; unsigned int lth_brightness; base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780 -- 2.38.1
[PATCH 2/2] drm/amd/display: Remove set but unused variable cursor_bpp
Variable cursor_bpp is not effectively used in the function, so delete it. drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hwseq.c:217:10: warning: variable ‘cursor_bpp’ set but not used. Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=3120 Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c index 763311ffb967..311be35de315 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c @@ -214,7 +214,6 @@ static uint32_t dcn32_calculate_cab_allocation(struct dc *dc, struct dc_state *c uint32_t lines_per_way = 0; uint8_t num_ways = 0; uint8_t bytes_per_pixel = 0; - uint8_t cursor_bpp = 0; uint16_t mblk_width = 0; uint16_t mblk_height = 0; uint16_t mall_alloc_width_blk_aligned = 0; @@ -288,19 +287,16 @@ static uint32_t dcn32_calculate_cab_allocation(struct dc *dc, struct dc_state *c switch (pipe->stream->cursor_attributes.color_format) { case CURSOR_MODE_MONO: cursor_size /= 2; - cursor_bpp = 4; break; case CURSOR_MODE_COLOR_1BIT_AND: case CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA: case CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA: cursor_size *= 4; - cursor_bpp = 4; break; case CURSOR_MODE_COLOR_64BIT_FP_PRE_MULTIPLIED: case CURSOR_MODE_COLOR_64BIT_FP_UN_PRE_MULTIPLIED: cursor_size *= 8; - cursor_bpp = 8; break; } -- 2.20.1.7.g153144c
[PATCH 1/2] drm/amd/display: Remove set but unused variable 'TPreMargin'
Variable TPreMargin is not effectively used in the function, so delete it. drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_util_32.c:3478:10: warning: variable ‘TPreMargin’ set but not used. Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=3144 Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- .../gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c index d967264e25cf..7f840c20d213 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c @@ -3475,7 +3475,6 @@ bool dml32_CalculatePrefetchSchedule( double min_Lsw; double Tsw_est1 = 0; double Tsw_est3 = 0; - double TPreMargin = 0; if (v->GPUVMEnable == true && v->HostVMEnable == true) HostVMDynamicLevelsTrips = v->HostVMMaxNonCachedPageTableLevels; @@ -3703,7 +3702,6 @@ bool dml32_CalculatePrefetchSchedule( dst_y_prefetch_equ = dml_floor(4.0 * (dst_y_prefetch_equ + 0.125), 1) / 4.0; Tpre_rounded = dst_y_prefetch_equ * LineTime; - TPreMargin = Tpre_rounded - TPreReq; #ifdef __DML_VBA_DEBUG__ dml_print("DML::%s: dst_y_prefetch_equ: %f (after round)\n", __func__, dst_y_prefetch_equ); dml_print("DML::%s: LineTime: %f\n", __func__, LineTime); -- 2.20.1.7.g153144c
Re: [Intel-gfx] [PATCH 1/1] drm/i915/mtl: Enable Idle Messaging for GSC CS
Hi Rodrigo, On 15-11-2022 20:57, Rodrigo Vivi wrote: On Tue, Nov 15, 2022 at 07:14:40PM +0530, Badal Nilawar wrote: From: Vinay Belgaumkar By defaut idle mesaging is disabled for GSC CS so to unblock RC6 entry on media tile idle messaging need to be enabled. v2: - Fix review comments (Vinay) - Set GSC idle hysterisis to 5 us (Badal) btw, no need for the cover letter in single/standalone patches. This history here is enough. Sure, next revision I will take care of this. Bspec: 71496 Cc: Daniele Ceraolo Spurio Signed-off-by: Vinay Belgaumkar Signed-off-by: Badal Nilawar --- drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++ drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index b0a4a2dbe3ee..5522885b2db0 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -15,6 +15,22 @@ #include "intel_rc6.h" #include "intel_ring.h" #include "shmem_utils.h" +#include "intel_gt_regs.h" + +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine) +{ + struct drm_i915_private *i915 = engine->i915; + + if (IS_METEORLAKE(i915) && engine->id == GSC0) { + intel_uncore_write(engine->gt->uncore, + RC_PSMI_CTRL_GSCCS, + _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE)); + /* 5 us hysterisis */ typo + intel_uncore_write(engine->gt->uncore, + PWRCTX_MAXCNT_GSCCS, + 0xA); you said 5 above, but used 10 here, why? Bspec:71496 specifies 0xA = 5us. + } +} static void dbg_poison_ce(struct intel_context *ce) { @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine) intel_wakeref_init(&engine->wakeref, rpm, &wf_ops); intel_engine_init_heartbeat(engine); + + intel_gsc_idle_msg_enable(engine); } /** diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index 07031e03f80c..20472eb15364 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -913,6 +913,10 @@ #define MSG_IDLE_FW_MASK REG_GENMASK(13, 9) #define MSG_IDLE_FW_SHIFT9 +#define RC_PSMI_CTRL_GSCCS _MMIO(0x11a050) +#define IDLE_MSG_DISABLE BIT(0) REG_BIT should be preferred. I will take care of this and above comments in next revision. Regards, Badal +#define PWRCTX_MAXCNT_GSCCS_MMIO(0x11a054) + #define FORCEWAKE_MEDIA_GEN9 _MMIO(0xa270) #define FORCEWAKE_RENDER_GEN9 _MMIO(0xa278) -- 2.25.1
Re: [PATCH 0/7] DYNAMIC_DEBUG fixups for rc
On Fri, Nov 11, 2022 at 03:17:08PM -0700, Jim Cromie wrote: > hi Jason, Greg, DRM-folk, > > drm.debug-on-dyndbg has a regression due to a chicken-&-egg problem; > drm.debug is applied to enable dyndbg callsites before the dependent > modules' callsites are available to be enabled. > > My "fixes" are unready, so lets just mark it BROKEN for now. > > Meanwhile, heres some other fixes, a comment tweak, a proof of > non-bug, an internal simplification, and a cleanup/improvement to the > main macro (API): > > Split DECLARE_DYNDBG_CLASSMAP in 1/2; REFERENCE_DYNDBG_CLASSMAP now > refers to a classmap DECLARE'd just once. I think this gives a path > away from the coordination-by-identical-classmaps "feature" that Jani > and others thought was "weird" (my term). > Acked-by: Greg Kroah-Hartman
Re: [PATCH 1/7] drm: mark drm.debug-on-dyndbg as BROKEN for now
On Fri, Nov 11, 2022 at 03:17:09PM -0700, Jim Cromie wrote: > drm.debug-on-dyndbg has a regression, due to a chicken-egg > initialization problem: > > 1- modprobe i915 >i915 needs drm.ko, which is loaded 1st > > 2- "modprobe drm drm.debug=0x1ff" (virtual/implied) >drm.debug is set post-initialization, from boot-args etc > > 3- `modprobe i915` finishes > > W/O drm.debug-on-dyndbg that just works, because all drm_dbg* > callsites use drm_debug_enabled() to check __drm_debug & DEM_UT_ > before printing. > > But the whole point of drm.debug-on-dyndbg is to avoid that runtime > test, by enabling (at post-modinit) a static-key at each callsite in > the just-loaded module. > > And since drm.ko is loaded before all dependent modules, none are > "just-loaded", and no drm.debug callsites are present yet, except > those in drm.ko itself. > > Signed-off-by: Jim Cromie > --- > drivers/gpu/drm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 34f5a092c99e..0d1e59e6bb7e 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -54,6 +54,7 @@ config DRM_DEBUG_MM > config DRM_USE_DYNAMIC_DEBUG > bool "use dynamic debug to implement drm.debug" > default y > + depends on BROKEN # chicken-egg initial enable problem > depends on DRM > depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE > depends on JUMP_LABEL > -- > 2.38.1 This should go through the drm tree now. The rest probably should also go that way and not through my tree as well. thanks, greg k-h
[PATCH] dma-buf: Fix possible UAF in dma_buf_export
Smatch report warning as follows: drivers/dma-buf/dma-buf.c:681 dma_buf_export() warn: '&dmabuf->list_node' not removed from list If dma_buf_stats_setup() fails in dma_buf_export(), goto err_sysfs and dmabuf will be freed, but dmabuf->list_node will not be removed from db_list.head, then list traversal may cause UAF. Fix by removeing it from db_list.head before free(). Fixes: ef3a6b70507a ("dma-buf: call dma_buf_stats_setup after dmabuf is in valid list") Signed-off-by: Gaosheng Cui --- drivers/dma-buf/dma-buf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index b809513b03fe..6848f50226d5 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -675,6 +675,9 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) return dmabuf; err_sysfs: + mutex_lock(&db_list.lock); + list_del(&dmabuf->list_node); + mutex_unlock(&db_list.lock); /* * Set file->f_path.dentry->d_fsdata to NULL so that when * dma_buf_release() gets invoked by dentry_ops, it exits -- 2.25.1
[RFC PATCH v6] media: mediatek: vcodec: support stateless AV1 decoder
Add mediatek av1 decoder linux driver which use the stateless API in MT8195. Signed-off-by: Xiaoyong Lu --- Changes from v5: - change av1 PROFILE and LEVEL cfg - test by av1 fluster, result is 173/239 Changes from v4: - convert vb2_find_timestamp to vb2_find_buffer - test by av1 fluster, result is 173/239 Changes from v3: - modify comment for struct vdec_av1_slice_slot - add define SEG_LVL_ALT_Q - change use_lr/use_chroma_lr parse from av1 spec - use ARRAY_SIZE to replace size for loop_filter_level and loop_filter_mode_deltas - change array size of loop_filter_mode_deltas from 4 to 2 - add define SECONDARY_FILTER_STRENGTH_NUM_BITS - change some hex values from upper case to lower case - change *dpb_sz equal to V4L2_AV1_TOTAL_REFS_PER_FRAME + 1 - test by av1 fluster, result is 173/239 Changes from v2: - Match with av1 uapi v3 modify - test by av1 fluster, result is 173/239 --- Reference series: [1]: v3 of this series is presend by Daniel Almeida. message-id: 20220825225312.564619-1-daniel.alme...@collabora.com .../media/platform/mediatek/vcodec/Makefile |1 + .../vcodec/mtk_vcodec_dec_stateless.c | 47 +- .../platform/mediatek/vcodec/mtk_vcodec_drv.h |1 + .../vcodec/vdec/vdec_av1_req_lat_if.c | 2234 + .../platform/mediatek/vcodec/vdec_drv_if.c|4 + .../platform/mediatek/vcodec/vdec_drv_if.h|1 + .../platform/mediatek/vcodec/vdec_msg_queue.c | 27 + .../platform/mediatek/vcodec/vdec_msg_queue.h |4 + 8 files changed, 2318 insertions(+), 1 deletion(-) create mode 100644 drivers/media/platform/mediatek/vcodec/vdec/vdec_av1_req_lat_if.c diff --git a/drivers/media/platform/mediatek/vcodec/Makefile b/drivers/media/platform/mediatek/vcodec/Makefile index 93e7a343b5b0e..7537259130072 100644 --- a/drivers/media/platform/mediatek/vcodec/Makefile +++ b/drivers/media/platform/mediatek/vcodec/Makefile @@ -10,6 +10,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \ vdec/vdec_vp8_req_if.o \ vdec/vdec_vp9_if.o \ vdec/vdec_vp9_req_lat_if.o \ + vdec/vdec_av1_req_lat_if.o \ vdec/vdec_h264_req_if.o \ vdec/vdec_h264_req_common.o \ vdec/vdec_h264_req_multi_if.o \ diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c index c45bd2599bb2d..ceb6fabc67749 100644 --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c @@ -107,11 +107,51 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = { }, .codec_type = V4L2_PIX_FMT_VP9_FRAME, }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_SEQUENCE, + + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_FRAME, + + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_TILE_GROUP_ENTRY, + .dims = { V4L2_AV1_MAX_TILE_COUNT }, + + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_PROFILE, + .min = V4L2_STATELESS_AV1_PROFILE_MAIN, + .def = V4L2_STATELESS_AV1_PROFILE_MAIN, + .max = V4L2_STATELESS_AV1_PROFILE_MAIN, + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_LEVEL, + .min = V4L2_STATELESS_AV1_LEVEL_2_0, + .def = V4L2_STATELESS_AV1_LEVEL_4_0, + .max = V4L2_STATELESS_AV1_LEVEL_5_1, + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, }; #define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls) -static struct mtk_video_fmt mtk_video_formats[5]; +static struct mtk_video_fmt mtk_video_formats[6]; static struct mtk_video_fmt default_out_format; static struct mtk_video_fmt default_cap_format; @@ -351,6 +391,7 @@ static void mtk_vcodec_add_formats(unsigned int fourcc, case V4L2_PIX_FMT_H264_SLICE: case V4L2_PIX_FMT_VP8_FRAME: case V4L2_PIX_FMT_VP9_FRAME: + case V4L2_PIX_FMT_AV1_FRAME: mtk_video_formats[count_formats].fourcc = fourcc; mtk_video_formats[count_formats].type = MTK_FMT_DEC; mtk_video_formats[count_formats].num_planes = 1; @@ -407,6 +448,10 @@ static void mtk_vcodec_get_supported_formats(struct mtk_vcodec_ctx *ctx) mtk_vcodec_add_formats(V4L2_PIX_FMT_VP9_FRAME, ctx);
Re: [PATCH v8 03/14] drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge
On 11/10/22 19:38, Jagan Teki wrote: [...] +static int samsung_dsim_wait_for_hdr_fifo(struct samsung_dsim *dsi) +{ + int timeout = 2000; + + do { + u32 reg = samsung_dsim_read(dsi, DSIM_FIFOCTRL_REG); + + if (!(reg & DSIM_SFR_HEADER_FULL)) Seems that unless I wait for DSIM_SFR_HEADER_EMPTY here, there may be some command transfer corruption if very short commands are transferred in rapid succession. This can be triggered with icn6211 driver for example. + return 0; + + if (!cond_resched()) + usleep_range(950, 1050); + } while (--timeout); + + return -ETIMEDOUT; +} [...]
[PATCH v4] udmabuf: add vmap and vunmap methods to udmabuf_ops
The reason behind that patch is associated with videobuf2 subsystem (or more genrally with v4l2 framework) and user created dma buffers (udmabuf). In some circumstances when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem wants to use dma_buf_vmap() method on the attached dma buffer. As udmabuf does not have .vmap operation implemented, such dma_buf_vmap() natually fails. videobuf2_common: __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each videobuf2_common: __prepare_dmabuf: buffer for plane 0 changed videobuf2_common: __prepare_dmabuf: failed to map dmabuf for plane 0 videobuf2_common: __buf_prepare: buffer preparation failed: -14 The patch itself seems to be strighforward. It adds implementation of .vmap and .vunmap methods to 'struct dma_buf_ops udmabuf_ops'. .vmap method itself uses vm_map_ram() to map pages linearly into the kernel virtual address space. .vunmap removes mapping created earlier by .vmap. All locking and 'vmapping counting' is done in dma_buf.c so it seems to be redundant/unnecessary in .vmap/.vunmap. Signed-off-by: Lukasz Wiecaszek --- v1: https://lore.kernel.org/linux-media/202211120352.g7wpasop-...@intel.com/T/#t v2: https://lore.kernel.org/linux-media/20221114052944.GA7264@thinkpad-p72/T/#t v3: https://lore.kernel.org/linux-media/4f92e95f-a0dc-4eac-4c08-0df85de78...@collabora.com/T/#t v3 -> v4: Removed line/info 'reported by kernel test robot' v2 -> v3: Added .vunmap to 'struct dma_buf_ops udmabuf_ops' v1 -> v2: Patch prepared and tested against 6.1.0-rc2+ drivers/dma-buf/udmabuf.c | 28 1 file changed, 28 insertions(+) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 283816fbd72f..740d6e426ee9 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -13,6 +13,8 @@ #include #include #include +#include +#include static int list_limit = 1024; module_param(list_limit, int, 0644); @@ -60,6 +62,30 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) return 0; } +static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) +{ + struct udmabuf *ubuf = buf->priv; + void *vaddr; + + dma_resv_assert_held(buf->resv); + + vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1); + if (!vaddr) + return -EINVAL; + + iosys_map_set_vaddr(map, vaddr); + return 0; +} + +static void vunmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) +{ + struct udmabuf *ubuf = buf->priv; + + dma_resv_assert_held(buf->resv); + + vm_unmap_ram(map->vaddr, ubuf->pagecount); +} + static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, enum dma_data_direction direction) { @@ -162,6 +188,8 @@ static const struct dma_buf_ops udmabuf_ops = { .unmap_dma_buf = unmap_udmabuf, .release = release_udmabuf, .mmap = mmap_udmabuf, + .vmap = vmap_udmabuf, + .vunmap= vunmap_udmabuf, .begin_cpu_access = begin_cpu_udmabuf, .end_cpu_access= end_cpu_udmabuf, }; -- 2.25.1
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 11/10/22 19:38, Jagan Teki wrote: DSI host initialization handling in previous exynos dsi driver has some pitfalls. It initializes the host during host transfer() hook that is indeed not the desired call flow for I2C and any other DSI configured downstream bridges. Host transfer() is usually triggered for downstream DSI panels or bridges and I2C-configured-DSI bridges miss these host initialization as these downstream bridges use bridge operations hooks like pre_enable, and enable in order to initialize or set up the host. This patch is trying to handle the host init handler to satisfy all downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state flag to ensure that host init is also done on first cmd transfer, this helps existing DSI panels work on exynos platform (form Marek Szyprowski). v8, v7, v6, v5: * none v4: * update init handling to ensure host init done on first cmd transfer v3: * none v2: * check initialized state in samsung_dsim_init v1: * keep DSI init in host transfer Signed-off-by: Marek Szyprowski Signed-off-by: Jagan Teki --- drivers/gpu/drm/bridge/samsung-dsim.c | 25 + include/drm/bridge/samsung-dsim.h | 5 +++-- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c index bb1f45fd5a88..ec7e01ae02ea 100644 --- a/drivers/gpu/drm/bridge/samsung-dsim.c +++ b/drivers/gpu/drm/bridge/samsung-dsim.c @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct samsung_dsim *dsi) disable_irq(dsi->irq); } -static int samsung_dsim_init(struct samsung_dsim *dsi) +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int flag) { const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; + if (dsi->state & flag) + return 0; + samsung_dsim_reset(dsi); - samsung_dsim_enable_irq(dsi); + + if (!(dsi->state & DSIM_STATE_INITIALIZED)) + samsung_dsim_enable_irq(dsi); if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST) samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1); @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi) samsung_dsim_set_phy_ctrl(dsi); samsung_dsim_init_link(dsi); + dsi->state |= flag; + return 0; } @@ -1269,6 +1276,10 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge, } dsi->state |= DSIM_STATE_ENABLED; + + ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED); + if (ret) + return; } static void samsung_dsim_atomic_enable(struct drm_bridge *bridge, @@ -1458,12 +1469,9 @@ static ssize_t samsung_dsim_host_transfer(struct mipi_dsi_host *host, if (!(dsi->state & DSIM_STATE_ENABLED)) return -EINVAL; - if (!(dsi->state & DSIM_STATE_INITIALIZED)) { - ret = samsung_dsim_init(dsi); - if (ret) - return ret; - dsi->state |= DSIM_STATE_INITIALIZED; - } + ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED); This triggers full controller reset and reprogramming upon first command transfer, is such heavy handed reload really necessary ?
[pull] amdgpu, amdkfd drm-fixes-6.1
Hi Dave, Daniel, Fixes for 6.1. The following changes since commit 094226ad94f471a9f19e8f8e7140a09c2625abaa: Linux 6.1-rc5 (2022-11-13 13:12:55 -0800) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-fixes-6.1-2022-11-16 for you to fetch changes up to 4b14841c9a820e484bc8c4c3f5a6fed1bc528cbc: drm/amd/pm: fix SMU13 runpm hang due to unintentional workaround (2022-11-15 13:29:06 -0500) amd-drm-fixes-6.1-2022-11-16: amdgpu: - Fix a possible memory leak in ganng submit error path - DP tunneling fixes - DCN 3.1 page flip fix - DCN 3.2.x fixes - DCN 3.1.4 fixes - Don't expose degamma on hardware that doesn't support it - BACO fixes for SMU 11.x - BACO fixes for SMU 13.x - Virtual display fix for devices with no display hardware amdkfd: - Memory limit regression fix Alex Deucher (1): drm/amdgpu: there is no vbios fb on devices with no display hw (v2) Candice Li (1): drm/amdgpu: Add psp_13_0_10_ta firmware to modinfo Dillon Varone (3): drm/amd/display: Fix prefetch calculations for dcn32 drm/amd/display: use uclk pstate latency for fw assisted mclk validation dcn32 drm/amd/display: Set max for prefetch lines on dcn32 Dong Chenchen (1): drm/amdgpu: Fix memory leak in amdgpu_cs_pass1 Eric Huang (1): drm/amdkfd: Fix a memory limit issue Evan Quan (3): drm/amd/pm: enable runpm support over BACO for SMU13.0.0 drm/amd/pm: enable runpm support over BACO for SMU13.0.7 drm/amd/pm: fix SMU13 runpm hang due to unintentional workaround George Shen (2): drm/amd/display: Support parsing VRAM info v3.0 from VBIOS drm/amd/display: Fix calculation for cursor CAB allocation Guchun Chen (1): drm/amdgpu: disable BACO support on more cards Melissa Wen (1): drm/amd/display: don't enable DRM CRTC degamma property for DCE Rodrigo Siqueira (1): drm/amd/display: Add HUBP surface flip interrupt handler Roman Li (1): drm/amd/display: Fix optc2_configure warning on dcn314 Stylon Wang (2): drm/amd/display: Fix access timeout to DPIA AUX at boot time drm/amd/display: Fix invalid DPIA AUX reply causing system hang drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 41 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c| 2 +- drivers/gpu/drm/amd/amdgpu/psp_v13_0.c | 1 + drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 + drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 6 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 10 -- drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 30 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hubp.c | 1 + .../gpu/drm/amd/display/dc/dcn314/dcn314_optc.c| 2 +- drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c | 14 +++- .../gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c | 16 - .../amd/display/dc/dml/dcn32/display_mode_vba_32.c | 2 ++ .../amd/display/dc/dml/dcn32/display_mode_vba_32.h | 2 ++ .../dc/dml/dcn32/display_mode_vba_util_32.c| 7 ++-- drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 23 ++-- drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 8 + drivers/gpu/drm/amd/pm/swsmu/inc/smu_v11_0.h | 10 +- drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h | 11 ++ .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c| 4 +++ drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 9 + .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 30 ++-- .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 30 ++-- 26 files changed, 235 insertions(+), 69 deletions(-)
Re: [PATCH] drm/msm/hdmi: remove unnecessary NULL check
On 11/15/2022 5:03 AM, Dan Carpenter wrote: This code was refactored in commit 69a88d8633ec ("drm/msm/hdmi: move resource allocation to probe function") and now the "hdmi" pointer can't be NULL. Checking causes a Smatch warning: drivers/gpu/drm/msm/hdmi/hdmi.c:141 msm_hdmi_init() warn: variable dereferenced before check 'hdmi' (see line 119) Signed-off-by: Dan Carpenter Can you please add the fixes tag to point to the commit you have referenced in the commit message? LTGM, Reviewed-by: Abhinav Kumar --- drivers/gpu/drm/msm/hdmi/hdmi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 7001fabd0977..4d3fdc806bef 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -138,8 +138,7 @@ static int msm_hdmi_init(struct hdmi *hdmi) return 0; fail: - if (hdmi) - msm_hdmi_destroy(hdmi); + msm_hdmi_destroy(hdmi); return ret; }
Re: [PATCH 12/13] drm/scheduler: rework entity flush, kill and fini
Hi, On 10/14/22 11:46, Christian König wrote: > +/* Remove the entity from the scheduler and kill all pending jobs */ > +static void drm_sched_entity_kill(struct drm_sched_entity *entity) > +{ > + struct drm_sched_job *job; > + struct dma_fence *prev; > + > + if (!entity->rq) > + return; > + > + spin_lock(&entity->rq_lock); > + entity->stopped = true; > + drm_sched_rq_remove_entity(entity->rq, entity); > + spin_unlock(&entity->rq_lock); > + > + /* Make sure this entity is not used by the scheduler at the moment */ > + wait_for_completion(&entity->entity_idle); I'm always hitting lockup here using Panfrost driver on terminating Xorg. Revering this patch helps. Any ideas how to fix it? -- Best regards, Dmitry
linux-next: manual merge of the drm tree with Linus' tree
Hi all, Today's linux-next merge of the drm tree got a conflict in: drivers/gpu/drm/rcar-du/Kconfig between commit: a830a1567859 ("drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI") from Linus' tree and commit: 7a043f978ed1 ("drm: rcar-du: Add RZ/G2L DSI driver") from the drm tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc drivers/gpu/drm/rcar-du/Kconfig index fd2c2eaee26b,f14686549cbe.. --- a/drivers/gpu/drm/rcar-du/Kconfig +++ b/drivers/gpu/drm/rcar-du/Kconfig @@@ -41,21 -41,22 +41,27 @@@ config DRM_RCAR_LVD depends on DRM_RCAR_USE_LVDS select DRM_KMS_HELPER select DRM_PANEL - select OF_FLATTREE - select OF_OVERLAY -config DRM_RCAR_MIPI_DSI - tristate "R-Car DU MIPI DSI Encoder Support" - depends on DRM && DRM_BRIDGE && OF - select DRM_MIPI_DSI +config DRM_RCAR_USE_MIPI_DSI + bool "R-Car DU MIPI DSI Encoder Support" + depends on DRM_BRIDGE && OF + default DRM_RCAR_DU help Enable support for the R-Car Display Unit embedded MIPI DSI encoders. +config DRM_RCAR_MIPI_DSI + def_tristate DRM_RCAR_DU + depends on DRM_RCAR_USE_MIPI_DSI + select DRM_MIPI_DSI + + config DRM_RZG2L_MIPI_DSI + tristate "RZ/G2L MIPI DSI Encoder Support" + depends on DRM_BRIDGE && OF + depends on ARCH_RENESAS || COMPILE_TEST + select DRM_MIPI_DSI + help + Enable support for the RZ/G2L Display Unit embedded MIPI DSI encoders. + config DRM_RCAR_VSP bool "R-Car DU VSP Compositor Support" if ARM default y if ARM64 pgp9dIgy44_1r.pgp Description: OpenPGP digital signature
Re: [PATCH] drm/i915/huc: fix leak of debug object in huc load fence on driver unload
Hi Daniele, On Thu, Nov 10, 2022 at 04:56:51PM -0800, Daniele Ceraolo Spurio wrote: > The fence is always initialized in huc_init_early, but the cleanup in > huc_fini is only being run if HuC is enabled. This causes a leaking of > the debug object when HuC is disabled/not supported, which can in turn > trigger a warning if we try to register a new debug offset at the same > address on driver reload. > > To fix the issue, make sure to always run the cleanup code. > > Reported-by: Tvrtko Ursulin > Reported-by: Brian Norris > Fixes: 27536e03271d ("drm/i915/huc: track delayed HuC load with a fence") > Signed-off-by: Daniele Ceraolo Spurio > Cc: Tvrtko Ursulin > Cc: Brian Norris > Cc: Alan Previn > Cc: John Harrison > --- > > Note: I didn't manage to repro the reported warning, but I did confirm > that we weren't correctly calling i915_sw_fence_fini and that this patch > fixes that. I *did* reproduce, and with this patch, I no longer reproduce. So: Tested-by: Brian Norris I see this differs very slightly from the draft version (which didn't work for me): https://lore.kernel.org/all/ac5fde11-c17d-8574-c938-c2278d53c...@intel.com/ so presumably that diff is the fix. Thanks a bunch! Brian > drivers/gpu/drm/i915/gt/uc/intel_huc.c | 12 +++- > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 1 + > 2 files changed, 8 insertions(+), 5 deletions(-)
Re: [PATCH mm-unstable v1 12/20] RDMA/siw: remove FOLL_FORCE usage
On Wed, Nov 16, 2022 at 11:26:51AM +0100, David Hildenbrand wrote: > GUP now supports reliable R/O long-term pinning in COW mappings, such > that we break COW early. MAP_SHARED VMAs only use the shared zeropage so > far in one corner case (DAXFS file with holes), which can be ignored > because GUP does not support long-term pinning in fsdax (see > check_vma_flags()). > > Consequently, FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM is no longer required > for reliable R/O long-term pinning: FOLL_LONGTERM is sufficient. So stop > using FOLL_FORCE, which is really only for ptrace access. > > Cc: Bernard Metzler > Cc: Jason Gunthorpe > Cc: Leon Romanovsky > Signed-off-by: David Hildenbrand > --- > drivers/infiniband/sw/siw/siw_mem.c | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH mm-unstable v1 11/20] RDMA/usnic: remove FOLL_FORCE usage
On Wed, Nov 16, 2022 at 11:26:50AM +0100, David Hildenbrand wrote: > GUP now supports reliable R/O long-term pinning in COW mappings, such > that we break COW early. MAP_SHARED VMAs only use the shared zeropage so > far in one corner case (DAXFS file with holes), which can be ignored > because GUP does not support long-term pinning in fsdax (see > check_vma_flags()). > > Consequently, FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM is no longer required > for reliable R/O long-term pinning: FOLL_LONGTERM is sufficient. So stop > using FOLL_FORCE, which is really only for ptrace access. > > Cc: Christian Benvenuti > Cc: Nelson Escobar > Cc: Jason Gunthorpe > Cc: Leon Romanovsky > Signed-off-by: David Hildenbrand > --- > drivers/infiniband/hw/usnic/usnic_uiom.c | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH mm-unstable v1 10/20] RDMA/umem: remove FOLL_FORCE usage
On Wed, Nov 16, 2022 at 11:26:49AM +0100, David Hildenbrand wrote: > GUP now supports reliable R/O long-term pinning in COW mappings, such > that we break COW early. MAP_SHARED VMAs only use the shared zeropage so > far in one corner case (DAXFS file with holes), which can be ignored > because GUP does not support long-term pinning in fsdax (see > check_vma_flags()). > > Consequently, FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM is no longer required > for reliable R/O long-term pinning: FOLL_LONGTERM is sufficient. So stop > using FOLL_FORCE, which is really only for ptrace access. > > Tested-by: Leon Romanovsky # Over mlx4 and mlx5. > Cc: Jason Gunthorpe > Cc: Leon Romanovsky > Signed-off-by: David Hildenbrand > --- > drivers/infiniband/core/umem.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH v2 1/4] drm/amdgpu/mst: Stop ignoring error codes and deadlocking
On Wed, 2022-11-16 at 04:39 +, Lin, Wayne wrote: > [Public] > > All the patch set looks good to me. Feel free to add: > Reviewed-by: Wayne Lin > > Again, thank you Lyude for helping on this!!! No problem! I was the one who introduced the bug anyway :P, I'm just glad we were able to fix this on time. Harry, Alex - feel free to merge this on whatever branch you want (I'm fine with the mst-helper bits going through amd's branch, especially since AMD is the only driver using the dsc stuff right now) > > Regards, > Wayne > > -Original Message- > > From: Lyude Paul > > Sent: Tuesday, November 15, 2022 6:18 AM > > To: amd-...@lists.freedesktop.org > > Cc: Wentland, Harry ; sta...@vger.kernel.org; > > Li, Sun peng (Leo) ; Siqueira, Rodrigo > > ; Deucher, Alexander > > ; Koenig, Christian > > ; Pan, Xinhui ; David > > Airlie ; Daniel Vetter ; Kazlauskas, > > Nicholas ; Pillai, Aurabindo > > ; Li, Roman ; Zuo, Jerry > > ; Wu, Hersen ; Lin, Wayne > > ; Thomas Zimmermann ; > > Mahfooz, Hamza ; Hung, Alex > > ; Mikita Lipski ; Liu, > > Wenjing ; Francis, David > > ; open list:DRM DRIVERS > de...@lists.freedesktop.org>; open list > > Subject: [PATCH v2 1/4] drm/amdgpu/mst: Stop ignoring error codes and > > deadlocking > > > > It appears that amdgpu makes the mistake of completely ignoring the return > > values from the DP MST helpers, and instead just returns a simple > > true/false. > > In this case, it seems to have come back to bite us because as a result of > > simply returning false from compute_mst_dsc_configs_for_state(), amdgpu > > had no way of telling when a deadlock happened from these helpers. This > > could definitely result in some kernel splats. > > > > V2: > > * Address Wayne's comments (fix another bunch of spots where we weren't > > passing down return codes) > > > > Signed-off-by: Lyude Paul > > Fixes: 8c20a1ed9b4f ("drm/amd/display: MST DSC compute fair share") > > Cc: Harry Wentland > > Cc: # v5.6+ > > --- > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 18 +- > > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 235 ++-- > > -- > > .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 12 +- > > 3 files changed, 147 insertions(+), 118 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > index 0db2a88cd4d7b..852a2100c6b38 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > @@ -6462,7 +6462,7 @@ static int > > dm_update_mst_vcpi_slots_for_dsc(struct drm_atomic_state *state, > > struct drm_connector_state *new_con_state; > > struct amdgpu_dm_connector *aconnector; > > struct dm_connector_state *dm_conn_state; > > - int i, j; > > + int i, j, ret; > > int vcpi, pbn_div, pbn, slot_num = 0; > > > > for_each_new_connector_in_state(state, connector, > > new_con_state, i) { @@ -6509,8 +6509,11 @@ static int > > dm_update_mst_vcpi_slots_for_dsc(struct drm_atomic_state *state, > > dm_conn_state->pbn = pbn; > > dm_conn_state->vcpi_slots = slot_num; > > > > - drm_dp_mst_atomic_enable_dsc(state, aconnector- > > > port, dm_conn_state->pbn, > > -false); > > + ret = drm_dp_mst_atomic_enable_dsc(state, > > aconnector->port, > > + dm_conn_state- > > > pbn, false); > > + if (ret < 0) > > + return ret; > > + > > continue; > > } > > > > @@ -9523,10 +9526,9 @@ static int amdgpu_dm_atomic_check(struct > > drm_device *dev, > > > > #if defined(CONFIG_DRM_AMD_DC_DCN) > > if (dc_resource_is_dsc_encoding_supported(dc)) { > > - if (!pre_validate_dsc(state, &dm_state, vars)) { > > - ret = -EINVAL; > > + ret = pre_validate_dsc(state, &dm_state, vars); > > + if (ret != 0) > > goto fail; > > - } > > } > > #endif > > > > @@ -9621,9 +9623,9 @@ static int amdgpu_dm_atomic_check(struct > > drm_device *dev, > > } > > > > #if defined(CONFIG_DRM_AMD_DC_DCN) > > - if (!compute_mst_dsc_configs_for_state(state, dm_state- > > > context, vars)) { > > + ret = compute_mst_dsc_configs_for_state(state, dm_state- > > > context, vars); > > + if (ret) { > > > > DRM_DEBUG_DRIVER("compute_mst_dsc_configs_for_state() > > failed\n"); > > - ret = -EINVAL; > > goto fail; > > } > > > > diff --git > > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > index 6ff96b4bdda5c..bba2e8aaa2c20 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > +++ > > b/drivers/gpu/drm/amd/display/amdgpu
[drm-misc:drm-misc-next 8/8] hid-picolcd_leds.c:(.text+0x140): multiple definition of `fb_modesetting_disabled'; drivers/hid/hid-picolcd_core.o:hid-picolcd_core.c:(.text+0x1060): first defined here
tree: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next head: 330ff5a555869aa0ba3b4c206bf046232e356842 commit: 0ba2fa8cbd29278a180ac90bd66b2c0bbdeacc89 [8/8] fbdev: Add support for the nomodeset kernel parameter config: s390-allmodconfig compiler: s390-linux-gcc (GCC) 12.1.0 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 git remote add drm-misc git://anongit.freedesktop.org/drm/drm-misc git fetch --no-tags drm-misc drm-misc-next git checkout 0ba2fa8cbd29278a180ac90bd66b2c0bbdeacc89 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All errors (new ones prefixed by >>): s390-linux-ld: drivers/hid/hid-picolcd_leds.o: in function `fb_modesetting_disabled': >> hid-picolcd_leds.c:(.text+0x140): multiple definition of >> `fb_modesetting_disabled'; >> drivers/hid/hid-picolcd_core.o:hid-picolcd_core.c:(.text+0x1060): first >> defined here s390-linux-ld: drivers/hid/hid-picolcd_cir.o: in function `fb_modesetting_disabled': hid-picolcd_cir.c:(.text+0xe8): multiple definition of `fb_modesetting_disabled'; drivers/hid/hid-picolcd_core.o:hid-picolcd_core.c:(.text+0x1060): first defined here s390-linux-ld: drivers/hid/hid-picolcd_debugfs.o: in function `fb_modesetting_disabled': hid-picolcd_debugfs.c:(.text+0x1cd0): multiple definition of `fb_modesetting_disabled'; drivers/hid/hid-picolcd_core.o:hid-picolcd_core.c:(.text+0x1060): first defined here -- 0-DAY CI Kernel Test Service https://01.org/lkp # # Automatically generated file; DO NOT EDIT. # Linux/s390 6.1.0-rc2 Kernel Configuration # CONFIG_CC_VERSION_TEXT="s390-linux-gcc (GCC) 12.1.0" CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=120100 CONFIG_CLANG_VERSION=0 CONFIG_AS_IS_GNU=y CONFIG_AS_VERSION=23800 CONFIG_LD_IS_BFD=y CONFIG_LD_VERSION=23800 CONFIG_LLD_VERSION=0 CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y CONFIG_CC_HAS_ASM_INLINE=y CONFIG_CC_HAS_NO_PROFILE_FN_ATTR=y CONFIG_PAHOLE_VERSION=123 CONFIG_CONSTRUCTORS=y CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_TABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 # CONFIG_WERROR is not set CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_BUILD_SALT="" CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y CONFIG_HAVE_KERNEL_ZSTD=y CONFIG_HAVE_KERNEL_UNCOMPRESSED=y CONFIG_KERNEL_GZIP=y # CONFIG_KERNEL_BZIP2 is not set # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set # CONFIG_KERNEL_ZSTD is not set # CONFIG_KERNEL_UNCOMPRESSED is not set CONFIG_DEFAULT_INIT="" CONFIG_DEFAULT_HOSTNAME="(none)" CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_SYSVIPC_COMPAT=y CONFIG_POSIX_MQUEUE=y CONFIG_POSIX_MQUEUE_SYSCTL=y CONFIG_WATCH_QUEUE=y CONFIG_CROSS_MEMORY_ATTACH=y CONFIG_USELIB=y CONFIG_AUDIT=y CONFIG_HAVE_ARCH_AUDITSYSCALL=y CONFIG_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_INJECTION=y CONFIG_GENERIC_IRQ_CHIP=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_SIM=y CONFIG_SPARSE_IRQ=y CONFIG_GENERIC_IRQ_DEBUGFS=y # end of IRQ subsystem CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_TIME_KUNIT_TEST=m CONFIG_CONTEXT_TRACKING=y CONFIG_CONTEXT_TRACKING_IDLE=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y CONFIG_NO_HZ_COMMON=y # CONFIG_HZ_PERIODIC is not set CONFIG_NO_HZ_IDLE=y CONFIG_NO_HZ=y CONFIG_HIGH_RES_TIMERS=y # end of Timers subsystem CONFIG_BPF=y CONFIG_HAVE_EBPF_JIT=y CONFIG_ARCH_WANT_DEFAULT_BPF_JIT=y # # BPF subsystem # CONFIG_BPF_SYSCALL=y CONFIG_BPF_JIT=y CONFIG_BPF_JIT_ALWAYS_ON=y CONFIG_BPF_JIT_DEFAULT_ON=y CONFIG_BPF_UNPRIV_DEFAULT_OFF=y CONFIG_USERMODE_DRIVER=y # CONFIG_BPF_PRELOAD is not set CONFIG_BPF_LSM=y # end of BPF subsystem CONFIG_PREEMPT_NONE_BUILD=y CONFIG_PREEMPT_NONE=y # CONFIG_PREEMPT_VOLUNTARY is not set # CONFIG_PREEMPT is not set CONFIG_PREEMPT_COUNT=y CONFIG_SCHED_CORE=y # # CPU/Task time and stats accounting # CONFIG_VIRT_CPU_ACCOUNTING=y CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y CONFIG_BSD_PROCESS_ACCT=y CONFIG_BSD_PROCESS_ACCT_V3=y CONFIG_TASKSTATS=y CONFIG_TASK_DELAY_ACCT=y CONFIG_TASK_XACCT=y CONFIG_TASK_IO_ACCOUNTING=y CONFIG_PSI=y CONFIG_PSI_DEFAULT_DISABLED=y # end of CPU/Task time and stats accounting CONFIG_CPU_ISOLATION=y # # RCU Subsystem # CONFIG_TREE_RCU=y CONFIG_RCU_EXPERT=y CONFIG_SRCU=y CONFIG_TREE_SRCU=y CONFIG_TASKS_RCU_GENERIC=y CONFIG_FORCE_TASKS_RCU=y CONFIG_TASKS_RCU=y CONFIG_FORCE_TASKS_RUDE_RCU=y CONFIG_TASKS_RUDE_RCU=y CONFIG_FORCE_TASKS_TRACE_RCU=y CONFIG_TASKS_TRACE_RCU=y CONFIG_RCU_STALL_COMMON=y CONFIG_RCU_NEED_SEGCBLIST=y CONFIG
Re: [pull] amdgpu, amdkfd, radeon drm-next-6.2
On Wed, Nov 16, 2022 at 12:03 AM Dave Airlie wrote: > > arm32 build fails > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c: > In function ‘disable_dangling_plane’: > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1134:46: > error: ‘const struct timing_generator_funcs’ has no member named > ‘disable_phantom_crtc’ > 1134 | if (tg->funcs->disable_phantom_crtc) > | ^~ > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1135:50: > error: ‘const struct timing_generator_funcs’ has no member named > ‘disable_phantom_crtc’ > 1135 | > tg->funcs->disable_phantom_crtc(tg); > | ^~ > make[6]: *** [/home/airlied/devel/kernel/dim/src/scripts/Makefile.build:250: > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.o] Error 1 > Fixed in: https://patchwork.freedesktop.org/patch/511897/ Will send a new PR when I get an RB. Alex > Dave. > > On Sat, 12 Nov 2022 at 06:19, Alex Deucher wrote: > > > > Hi Dave, Daniel, > > > > More new stuff for 6.2. > > > > The following changes since commit a143bc517bf31c4575191efbaac216a11ec016e0: > > > > Merge branch '00.06-gr-ampere' of > > https://gitlab.freedesktop.org/skeggsb/nouveau into drm-next (2022-11-09 > > 11:18:56 +1000) > > > > are available in the Git repository at: > > > > https://gitlab.freedesktop.org/agd5f/linux.git > > tags/amd-drm-next-6.2-2022-11-11 > > > > for you to fetch changes up to 2ebf61f2cfb9a11bc17db30df3e675a4cd7418d3: > > > > drm/amdgpu: Fix memory leak in amdgpu_cs_pass1 (2022-11-10 15:30:34 -0500) > > > > > > amd-drm-next-6.2-2022-11-11: > > > > amdgpu: > > - SMU 13.x updates > > - GPUVM TLB race fix > > - DCN 3.1.4 updates > > - DCN 3.2.x updates > > - PSR fixes > > - Kerneldoc fix > > - Vega10 fan fix > > - GPUVM locking fixes in error pathes > > - BACO fix for Beige Goby > > - EEPROM I2C address cleanup > > - GFXOFF fix > > - Fix DC memory leak in error pathes > > - Flexible array updates > > - Mtype fix for GPUVM PTEs > > - Move Kconfig into amdgpu directory > > - SR-IOV updates > > - Fix possible memory leak in CS IOCTL error path > > > > amdkfd: > > - Fix possible memory overrun > > - CRIU fixes > > > > radeon: > > - ACPI ref count fix > > - HDA audio notifier support > > - Move Kconfig into radeon directory > > > > UAPI: > > - Add new GEM_CREATE flags to help to transition more KFD functionality to > > the DRM UAPI. > > These are used internally in the driver to align location based memory > > coherency > > requirements from memory allocated in the KFD with how we manage GPUVM > > PTEs. They > > are currently blocked in the GEM_CREATE IOCTL as we don't have a user > > right now. > > They are just used internally in the kernel driver for now for existing > > KFD memory > > allocations. So a change to the UAPI header, but no functional change in > > the UAPI. > > > > > > Alvin Lee (4): > > drm/amd/display: Wait for VBLANK during pipe programming > > drm/amd/display: Use min transition for SubVP into MPO > > drm/amd/display: Disable phantom OTG after enable for plane disable > > drm/amd/display: Add margin for max vblank time for SubVP + DRR > > > > Andrew Davis (1): > > drm: Move radeon and amdgpu Kconfig options into their directories > > > > Aric Cyr (1): > > drm/amd/display: 3.2.211 > > > > Asher Song (1): > > Revert "drm/amdgpu: Revert "drm/amdgpu: getting fan speed pwm for > > vega10 properly"" > > > > Aurabindo Pillai (1): > > drm/amd/display: Zeromem mypipe heap struct before using it > > > > Chaitanya Dhere (1): > > drm/amd/display: Fix FCLK deviation and tool compile issues > > > > Christian König (1): > > drm/amdgpu: workaround for TLB seq race > > > > Dillon Varone (1): > > drm/amd/display: Enforce minimum prefetch time for low memclk on DCN32 > > > > Dong Chenchen (1): > > drm/amdgpu: Fix memory leak in amdgpu_cs_pass1 > > > > Felix Kuehling (3): > > drm/amdkfd: Fix error handling in kfd_criu_restore_events > > drm/amdkfd: Fix error handling in criu_checkpoint > > drm/amdgpu: Set MTYPE in PTE based on BO flags > > > > Gavin Wan (1): > > drm/amdgpu: Ignore stop rlc on SRIOV environment. > > > > George Shen (1): > > drm/amd/display: Populate DP2.0 output type for DML pipe > > > > Guchun Chen (1): > > drm/amdgpu: disable BACO on special BEIGE_GOBY card > > > > Hamza Mahfooz (1): > > drm/amd/display: only fill dirty rectangles when PSR is enabled > > > > Hanjun Guo (1): > > drm/radeon: Add the missed acpi_put_table() to fix memory leak > > > > Harsh Jain (1): > > drm/amdgpu: complete gfxoff allow signal during suspen
Re: [PATCH v3] udmabuf: add vmap and vunmap methods to udmabuf_ops
On Wed, Nov 16, 2022 at 01:01:46PM +0100, Christian König wrote: > Am 15.11.22 um 21:04 schrieb Lukasz Wiecaszek: > > The reason behind that patch is associated with videobuf2 subsystem > > (or more genrally with v4l2 framework) and user created > > dma buffers (udmabuf). In some circumstances > > when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem > > wants to use dma_buf_vmap() method on the attached dma buffer. > > As udmabuf does not have .vmap operation implemented, > > such dma_buf_vmap() natually fails. > > > > videobuf2_common: __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each > > videobuf2_common: __prepare_dmabuf: buffer for plane 0 changed > > videobuf2_common: __prepare_dmabuf: failed to map dmabuf for plane 0 > > videobuf2_common: __buf_prepare: buffer preparation failed: -14 > > > > The patch itself seems to be strighforward. > > It adds implementation of .vmap and .vunmap methods > > to 'struct dma_buf_ops udmabuf_ops'. > > .vmap method itself uses vm_map_ram() to map pages linearly > > into the kernel virtual address space. > > .vunmap removes mapping created earlier by .vmap. > > All locking and 'vmapping counting' is done in dma_buf.c > > so it seems to be redundant/unnecessary in .vmap/.vunmap. > > > > Signed-off-by: Lukasz Wiecaszek > > > Reported-by: kernel test robot > > Please drop this line, the kernel test robot should only be mentioned if the > original report came from it. > > And keep in mind that it might be necessary to implement begin/end cpu > access callbacks as well. > > Apart from that the patch is Acked-by: Christian König > . > > Regards, > Christian. Thanks for that lesson with the 'kernel test robot' line. The second issue with begin/end cpu access callbacks is more complicated to me. My understaning is that memory allocated for udambuf will be the memory obtained most likely (if not always) by memfd_create(). So this will be the anonymous system memory which is 'by definition' coherent for cpu access. So no need for begin/end callbacks. But if I miss something, plese let me/us know. > > > --- > > v1: > > https://lore.kernel.org/linux-media/202211120352.g7wpasop-...@intel.com/T/#t > > v2: > > https://lore.kernel.org/linux-media/20221114052944.GA7264@thinkpad-p72/T/#t > > > > v2 -> v3: Added .vunmap to 'struct dma_buf_ops udmabuf_ops' > > v1 -> v2: Patch prepared and tested against 6.1.0-rc2+ > > > > drivers/dma-buf/udmabuf.c | 28 > > 1 file changed, 28 insertions(+) > > > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > > index 283816fbd72f..740d6e426ee9 100644 > > --- a/drivers/dma-buf/udmabuf.c > > +++ b/drivers/dma-buf/udmabuf.c > > @@ -13,6 +13,8 @@ > > #include > > #include > > #include > > +#include > > +#include > > static int list_limit = 1024; > > module_param(list_limit, int, 0644); > > @@ -60,6 +62,30 @@ static int mmap_udmabuf(struct dma_buf *buf, struct > > vm_area_struct *vma) > > return 0; > > } > > +static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) > > +{ > > + struct udmabuf *ubuf = buf->priv; > > + void *vaddr; > > + > > + dma_resv_assert_held(buf->resv); > > + > > + vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1); > > + if (!vaddr) > > + return -EINVAL; > > + > > + iosys_map_set_vaddr(map, vaddr); > > + return 0; > > +} > > + > > +static void vunmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) > > +{ > > + struct udmabuf *ubuf = buf->priv; > > + > > + dma_resv_assert_held(buf->resv); > > + > > + vm_unmap_ram(map->vaddr, ubuf->pagecount); > > +} > > + > > static struct sg_table *get_sg_table(struct device *dev, struct dma_buf > > *buf, > > enum dma_data_direction direction) > > { > > @@ -162,6 +188,8 @@ static const struct dma_buf_ops udmabuf_ops = { > > .unmap_dma_buf = unmap_udmabuf, > > .release = release_udmabuf, > > .mmap = mmap_udmabuf, > > + .vmap = vmap_udmabuf, > > + .vunmap= vunmap_udmabuf, > > .begin_cpu_access = begin_cpu_udmabuf, > > .end_cpu_access= end_cpu_udmabuf, > > }; >
[linux-next:master] BUILD REGRESSION 15f3bff12cf6a888ec2ad39652828c60e6836b3d
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 15f3bff12cf6a888ec2ad39652828c60e6836b3d Add linux-next specific files for 20221116 Error/Warning reports: https://lore.kernel.org/linux-mm/202210261404.b6ulzg7h-...@intel.com https://lore.kernel.org/oe-kbuild-all/202210270637.q5y7fikj-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211041320.coq8eelj-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211130053.np70vidn-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211161807.01czsomy-...@intel.com Error/Warning: (recently discovered and may have been fixed) aarch64-linux-ld: Unexpected GOT/PLT entries detected! aarch64-linux-ld: Unexpected run-time procedure linkages detected! arch/arm/mach-s3c/devs.c:32:10: fatal error: linux/platform_data/dma-s3c24xx.h: No such file or directory drivers/clk/clk.c:1022:5: error: redefinition of 'clk_prepare' drivers/clk/clk.c:1268:6: error: redefinition of 'clk_is_enabled_when_prepared' drivers/clk/clk.c:941:6: error: redefinition of 'clk_unprepare' drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:4939: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:5075:24: warning: implicit conversion from 'enum ' to 'enum dc_status' [-Wenum-conversion] drivers/gpu/drm/amd/amdgpu/../display/dc/irq/dcn201/irq_service_dcn201.c:40:20: warning: no previous prototype for 'to_dal_irq_source_dcn201' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gf100.c:451:1: warning: no previous prototype for 'gf100_fifo_nonstall_block' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gf100.c:451:1: warning: no previous prototype for function 'gf100_fifo_nonstall_block' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c:34:1: warning: no previous prototype for 'nvkm_engn_cgrp_get' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c:34:1: warning: no previous prototype for function 'nvkm_engn_cgrp_get' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c:210:1: warning: no previous prototype for 'tu102_gr_load' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c:210:1: warning: no previous prototype for function 'tu102_gr_load' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:49:1: warning: no previous prototype for 'wpr_generic_header_dump' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:49:1: warning: no previous prototype for function 'wpr_generic_header_dump' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c:221:21: warning: variable 'loc' set but not used [-Wunused-but-set-variable] ld.lld: error: .btf.vmlinux.bin.o: unknown file type ld.lld: error: undefined symbol: ipv6_icmp_error Unverified Error/Warning (likely false positive, please contact us if interested): ERROR: modpost: "usb_disabled" [drivers/usb/fotg210/fotg210.ko] undefined! lib/zstd/compress/huf_compress.c:460 HUF_getIndex() warn: the 'RANK_POSITION_LOG_BUCKETS_BEGIN' macro might need parens lib/zstd/decompress/zstd_decompress_block.c:1009 ZSTD_execSequence() warn: inconsistent indenting lib/zstd/decompress/zstd_decompress_block.c:894 ZSTD_execSequenceEnd() warn: inconsistent indenting lib/zstd/decompress/zstd_decompress_block.c:942 ZSTD_execSequenceEndSplitLitBuffer() warn: inconsistent indenting lib/zstd/decompress/zstd_decompress_internal.h:206 ZSTD_DCtx_get_bmi2() warn: inconsistent indenting mm/khugepaged.c:2038 collapse_file() warn: iterator used outside loop: 'page' Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status | |-- drivers-gpu-drm-nouveau-nvkm-engine-fifo-gf100.c:warning:no-previous-prototype-for-gf100_fifo_nonstall_block | |-- drivers-gpu-drm-nouveau-nvkm-engine-fifo-runl.c:warning:no-previous-prototype-for-nvkm_engn_cgrp_get | |-- drivers-gpu-drm-nouveau-nvkm-engine-gr-tu102.c:warning:no-previous-prototype-for-tu102_gr_load | |-- drivers-gpu-drm-nouveau-nvkm-nvfw-acr.c:warning:no-previous-prototype-for-wpr_generic_header_dump | `-- drivers-gpu-drm-nouveau-nvkm-subdev-acr-lsfw.c:warning:variable-loc-set-but-not-used |-- alpha-randconfig-r013-20221116 | |-- drivers-gpu-drm-nouveau-nvkm-engine-fifo-gf100.c:warning:no-previous-prototype-for-gf100_fifo_nonstall_block | |-- drivers-gpu-drm-nouveau-nvkm-engine-fifo-runl.c:warning:no-previous-prototype-for-nvkm_engn_cgrp_get | |-- drivers-gpu-drm-nouveau-nvkm-engine-gr-tu102.c:warning:no-previous-prototype-for-tu102_gr_load
[drm-misc:drm-misc-next 8/8] include/linux/fb.h:810:6: warning: no previous prototype for function 'fb_modesetting_disabled'
tree: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next head: 330ff5a555869aa0ba3b4c206bf046232e356842 commit: 0ba2fa8cbd29278a180ac90bd66b2c0bbdeacc89 [8/8] fbdev: Add support for the nomodeset kernel parameter config: s390-randconfig-r012-20221116 compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 463da45892e2d2a262277b91b96f5f8c05dc25d0) 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 s390 cross compiling tool for clang build # apt-get install binutils-s390x-linux-gnu git remote add drm-misc git://anongit.freedesktop.org/drm/drm-misc git fetch --no-tags drm-misc drm-misc-next git checkout 0ba2fa8cbd29278a180ac90bd66b2c0bbdeacc89 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from drivers/hid/hid-picolcd_core.c:11: In file included from include/linux/hid-debug.h:14: In file included from include/linux/kfifo.h:42: In file included from include/linux/scatterlist.h:9: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __raw_readb(PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x)) ^ include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16' #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x)) ^ In file included from drivers/hid/hid-picolcd_core.c:11: In file included from include/linux/hid-debug.h:14: In file included from include/linux/kfifo.h:42: In file included from include/linux/scatterlist.h:9: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) ^ include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32' #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) ^ In file included from drivers/hid/hid-picolcd_core.c:11: In file included from include/linux/hid-debug.h:14: In file included from include/linux/kfifo.h:42: In file included from include/linux/scatterlist.h:9: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writeb(value, PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] readsb(PCI_IOBASE + addr, buffer, count); ~~ ^ include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] readsw(PCI_IOBASE + addr, buffer, count);
[drm-misc:drm-misc-next 8/8] include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined here
tree: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next head: 330ff5a555869aa0ba3b4c206bf046232e356842 commit: 0ba2fa8cbd29278a180ac90bd66b2c0bbdeacc89 [8/8] fbdev: Add support for the nomodeset kernel parameter config: s390-defconfig compiler: s390-linux-gcc (GCC) 12.1.0 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 git remote add drm-misc git://anongit.freedesktop.org/drm/drm-misc git fetch --no-tags drm-misc drm-misc-next git checkout 0ba2fa8cbd29278a180ac90bd66b2c0bbdeacc89 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All error/warnings (new ones prefixed by >>): s390-linux-ld: drivers/video/fbdev/core/fb_notify.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here s390-linux-ld: drivers/video/fbdev/core/fbmem.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here s390-linux-ld: drivers/video/fbdev/core/fbmon.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here s390-linux-ld: drivers/video/fbdev/core/fbcmap.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here s390-linux-ld: drivers/video/fbdev/core/fbsysfs.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here s390-linux-ld: drivers/video/fbdev/core/modedb.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here s390-linux-ld: drivers/video/fbdev/core/fbcvt.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here s390-linux-ld: drivers/video/fbdev/core/fbcon.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here s390-linux-ld: drivers/video/fbdev/core/bitblit.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here s390-linux-ld: drivers/video/fbdev/core/softcursor.o: in function `fb_modesetting_disabled': >> include/linux/fb.h:811: multiple definition of `fb_modesetting_disabled'; >> drivers/video/fbdev/core/fb_cmdline.o:include/linux/fb.h:811: first defined >> here -- In file included from drivers/video/fbdev/core/fb_cmdline.c:18: >> include/linux/fb.h:810:6: warning: no previous prototype for >> 'fb_modesetting_disabled' [-Wmissing-prototypes] 810 | bool fb_modesetting_disabled(const char *drvname) | ^~~ vim +811 include/linux/fb.h 799 800 extern int fb_find_mode(struct fb_var_screeninfo *var, 801 struct fb_info *info, const char *mode_option, 802 const struct fb_videomode *db, 803 unsigned int dbsize, 804 const struct fb_videomode *default_mode, 805 unsigned int default_bpp); 806 807 #if defined(CONFIG_VIDEO_NOMODESET) 808 bool fb_modesetting_disabled(const char *drvname); 809 #else > 810 bool fb_modesetting_disabled(const char *drvname) > 811 { 812 return false; 813 } 814 #endif 815 -- 0-DAY CI Kernel Test Service https://01.org/lkp # # Automatically generated file; DO NOT EDIT. # Linux/s390 6.1.0-rc2 Kernel Configuration # CONFIG_CC_VERSION_TEXT="s390-linux-gcc (GCC) 12.1.0" CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=120100 CONFIG_CLANG_VERSION=0 CONFIG_AS_IS_GNU=y CONFIG_AS_VERSION=23800 CONFIG_LD_IS_BFD=y CONFIG_LD_VERSION=23800 CONFIG_LLD_VERSION=0 CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y CONFIG_CC_HAS_ASM_INLINE=y
[PATCH v5] drm/mediatek: Add AFBC support to Mediatek DRM driver
From: Justin Green Tested on MT8195 and confirmed both correct video output and improved DRAM bandwidth performance. v5: * Removed some dead defines. * Refactored mtk_ovl_set_afbc(). v4: * Move modifier validation to format_mod_supported function. * Add modifiers to drm_universal_plane_init() call. * Make comparisons to DRM_FORMAT_MOD_LINEAR explicit rather than relying on DRM_FORMAT_LINEAR being equal to 0. * Gate AFBC control bit writes on device compatibility. v3: * Replaced pitch bitshift math with union based approach. * Refactored overlay register writes to shared code between non-AFBC and AFBC. * Minor code cleanups. v2: * Marked mtk_ovl_set_afbc as static. * Reflowed some lines to fit column limit. Signed-off-by: Justin Green --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 57 +- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 74 +++- drivers/gpu/drm/mediatek/mtk_drm_plane.h | 8 +++ 3 files changed, 134 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 002b0f6cae1a..5a59e7b99c5d 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -29,17 +29,22 @@ #define DISP_REG_OVL_DATAPATH_CON 0x0024 #define OVL_LAYER_SMI_ID_ENBIT(0) #define OVL_BGCLR_SEL_IN BIT(2) +#define OVL_LAYER_AFBC_EN(n) BIT(4+n) #define DISP_REG_OVL_ROI_BGCLR 0x0028 #define DISP_REG_OVL_SRC_CON 0x002c #define DISP_REG_OVL_CON(n)(0x0030 + 0x20 * (n)) #define DISP_REG_OVL_SRC_SIZE(n) (0x0038 + 0x20 * (n)) #define DISP_REG_OVL_OFFSET(n) (0x003c + 0x20 * (n)) +#define DISP_REG_OVL_PITCH_MSB(n) (0x0040 + 0x20 * (n)) +#define OVL_PITCH_MSB_2ND_SUBBUF BIT(16) #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n)) #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n)) #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n)) #define DISP_REG_OVL_ADDR_MT2701 0x0040 #define DISP_REG_OVL_ADDR_MT8173 0x0f40 #define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n)) +#define DISP_REG_OVL_HDR_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x04) +#define DISP_REG_OVL_HDR_PITCH(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x08) #define GMC_THRESHOLD_BITS 16 #define GMC_THRESHOLD_HIGH ((1 << GMC_THRESHOLD_BITS) / 4) @@ -67,6 +72,7 @@ struct mtk_disp_ovl_data { unsigned int layer_nr; bool fmt_rgb565_is_0; bool smi_id_en; + bool supports_afbc; }; /* @@ -172,7 +178,14 @@ void mtk_ovl_stop(struct device *dev) reg = reg & ~OVL_LAYER_SMI_ID_EN; writel_relaxed(reg, ovl->regs + DISP_REG_OVL_DATAPATH_CON); } +} +static void mtk_ovl_set_afbc(struct mtk_disp_ovl *ovl, struct cmdq_pkt *cmdq_pkt, +int idx, bool enabled) +{ + mtk_ddp_write_mask(cmdq_pkt, enabled ? OVL_LAYER_AFBC_EN(idx) : 0, + &ovl->cmdq_reg, ovl->regs, + DISP_REG_OVL_DATAPATH_CON, OVL_LAYER_AFBC_EN(idx)); } void mtk_ovl_config(struct device *dev, unsigned int w, @@ -310,11 +323,23 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx, struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); struct mtk_plane_pending_state *pending = &state->pending; unsigned int addr = pending->addr; - unsigned int pitch = pending->pitch & 0x; + unsigned int hdr_addr = pending->hdr_addr; + unsigned int pitch = pending->pitch; + unsigned int hdr_pitch = pending->hdr_pitch; unsigned int fmt = pending->format; unsigned int offset = (pending->y << 16) | pending->x; unsigned int src_size = (pending->height << 16) | pending->width; unsigned int con; + bool is_afbc = pending->modifier != DRM_FORMAT_MOD_LINEAR; + union overlay_pitch { + struct split_pitch { + u16 lsb; + u16 msb; + } split_pitch; + u32 pitch; + } overlay_pitch; + + overlay_pitch.pitch = pitch; if (!pending->enable) { mtk_ovl_layer_off(dev, idx, cmdq_pkt); @@ -335,9 +360,12 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx, addr += pending->pitch - 1; } + if (ovl->data->supports_afbc) + mtk_ovl_set_afbc(ovl, cmdq_pkt, idx, is_afbc); + mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_CON(idx)); - mtk_ddp_write_relaxed(cmdq_pkt, pitch, &ovl->cmdq_reg, ovl->regs, + mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb,
Re: [PATCH mm-unstable v1 20/20] mm: rename FOLL_FORCE to FOLL_PTRACE
On 16.11.22 19:16, Linus Torvalds wrote: On Wed, Nov 16, 2022 at 2:30 AM David Hildenbrand wrote: Let's make it clearer that functionality provided by FOLL_FORCE is really only for ptrace access. I'm not super-happy about this one. I do understand the "let's rename the bit so that no new user shows up". And it's true that the main traditional use is ptrace. But from the patch itself it becomes obvious that no, it's not *just* ptrace. At least not yet. It's used for get_arg_page(), which uses it to basically look up (and install) pages in the newly created VM. Now, I'm not entirely sure why it even uses FOLL_FORCE, - I think it might be historical, because the target should always be the new stack vma. Following the history of it is a big of a mess, because there's a number of renamings and re-organizations, but it seems to go back to 2007 and commit b6a2fea39318 ("mm: variable length argument support"). Right. Before that commit, we kept our own array of "this is the set of pages that I will install in the new VM". That commit basically just inserts the pages directly into the VM instead, getting rid of the array size limitation. So at a minimum, I think that FOLL_FORCE would need to be removed before any renaming to FOLL_PTRACE, because that's not some kind of small random case. It *might* be as simple as just removing it, but maybe there's some reason for having it that I don't immediately see. Right, I have the same feeling. It might just be a copy-and-paste legacy leftover. There _are_ also small random cases too, like get_cmdline(). Maybe that counts as ptrace, but the execve() case most definitely does not. I agree. I'd suggest moving forward without this (last) patch for now and figuring out how to further cleanup FOLL_FORCE usage on top. @Andrew, if you intend to put this into mm-unstable, please drop the last patch for now. -- Thanks, David / dhildenb
Re: [RESEND] drm/edid/firmware: stop using a throwaway platform device
Thank you everyone for your work! Matthieu. On Wed, Nov 16 2022 at 03:32:01 PM +0200, Jani Nikula wrote: On Wed, 16 Nov 2022, Thomas Zimmermann wrote: Hi Am 14.11.22 um 12:17 schrieb Jani Nikula: We've used a temporary platform device for firmware EDID loading since it was introduced in commit da0df92b5731 ("drm: allow loading an EDID as firmware to override broken monitor"), but there's no explanation why. Using a temporary device does not play well with CONFIG_FW_CACHE=y, which caches firmware images (e.g. on suspend) so that drivers can request firmware when the system is not ready for it, and return the images from the cache (e.g. during resume). This works automatically for regular devices, but obviously not for a temporarily created device. Stop using the throwaway platform device, and use the drm device instead. Note that this may still be problematic for cases where the display was plugged in during suspend, and the firmware wasn't loaded and therefore not cached before suspend. References: https://lore.kernel.org/r/20220727074152.43059-1-matthieu.chare...@gmail.com Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2061 Reported-by: Matthieu CHARETTE Tested-by: Matthieu CHARETTE Cc: Ville Syrjälä Signed-off-by: Jani Nikula Acked-by: Thomas Zimmermann I looked through request_firmware() but did not see any signs that it somehow depends on a platform device. I assume that this might only affect the device name in the error message. Thanks, pushed to drm-misc-next. Matthieu, thanks for you patience and the report as well! BR, Jani. Best regards Thomas --- Resend with a proper commit message; patch itself is unchanged. --- drivers/gpu/drm/drm_edid_load.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index ef4ab59d6935..5d9ef267ebb3 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -172,20 +172,9 @@ static const struct drm_edid *edid_load(struct drm_connector *connector, const c fwdata = generic_edid[builtin]; fwsize = sizeof(generic_edid[builtin]); } else { - struct platform_device *pdev; int err; - pdev = platform_device_register_simple(connector->name, -1, NULL, 0); - if (IS_ERR(pdev)) { - drm_err(connector->dev, -"[CONNECTOR:%d:%s] Failed to register EDID firmware platform device for connector \"%s\"\n", - connector->base.id, connector->name, - connector->name); - return ERR_CAST(pdev); - } - - err = request_firmware(&fw, name, &pdev->dev); - platform_device_unregister(pdev); + err = request_firmware(&fw, name, connector->dev->dev); if (err) { drm_err(connector->dev, "[CONNECTOR:%d:%s] Requesting EDID firmware \"%s\" failed (err=%d)\n", -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH mm-unstable v1 20/20] mm: rename FOLL_FORCE to FOLL_PTRACE
On Wed, Nov 16, 2022 at 2:30 AM David Hildenbrand wrote: > > Let's make it clearer that functionality provided by FOLL_FORCE is > really only for ptrace access. I'm not super-happy about this one. I do understand the "let's rename the bit so that no new user shows up". And it's true that the main traditional use is ptrace. But from the patch itself it becomes obvious that no, it's not *just* ptrace. At least not yet. It's used for get_arg_page(), which uses it to basically look up (and install) pages in the newly created VM. Now, I'm not entirely sure why it even uses FOLL_FORCE, - I think it might be historical, because the target should always be the new stack vma. Following the history of it is a big of a mess, because there's a number of renamings and re-organizations, but it seems to go back to 2007 and commit b6a2fea39318 ("mm: variable length argument support"). Before that commit, we kept our own array of "this is the set of pages that I will install in the new VM". That commit basically just inserts the pages directly into the VM instead, getting rid of the array size limitation. So at a minimum, I think that FOLL_FORCE would need to be removed before any renaming to FOLL_PTRACE, because that's not some kind of small random case. It *might* be as simple as just removing it, but maybe there's some reason for having it that I don't immediately see. There _are_ also small random cases too, like get_cmdline(). Maybe that counts as ptrace, but the execve() case most definitely does not. Linus
Re: [PATCH 1/1] drm/i915/mtl: Enable Idle Messaging for GSC CS
On 11/15/2022 5:44 AM, Badal Nilawar wrote: From: Vinay Belgaumkar By defaut idle mesaging is disabled for GSC CS so to unblock RC6 entry on media tile idle messaging need to be enabled. v2: - Fix review comments (Vinay) - Set GSC idle hysterisis to 5 us (Badal) Bspec: 71496 Cc: Daniele Ceraolo Spurio Signed-off-by: Vinay Belgaumkar Signed-off-by: Badal Nilawar --- drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++ drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index b0a4a2dbe3ee..5522885b2db0 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -15,6 +15,22 @@ #include "intel_rc6.h" #include "intel_ring.h" #include "shmem_utils.h" +#include "intel_gt_regs.h" + +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine) +{ + struct drm_i915_private *i915 = engine->i915; + + if (IS_METEORLAKE(i915) && engine->id == GSC0) { + intel_uncore_write(engine->gt->uncore, + RC_PSMI_CTRL_GSCCS, + _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE)); + /* 5 us hysterisis */ + intel_uncore_write(engine->gt->uncore, + PWRCTX_MAXCNT_GSCCS, + 0xA); + } +} static void dbg_poison_ce(struct intel_context *ce) { @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine) intel_wakeref_init(&engine->wakeref, rpm, &wf_ops); intel_engine_init_heartbeat(engine); + + intel_gsc_idle_msg_enable(engine); } /** diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index 07031e03f80c..20472eb15364 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -913,6 +913,10 @@ #define MSG_IDLE_FW_MASK REG_GENMASK(13, 9) #define MSG_IDLE_FW_SHIFT9 +#define RC_PSMI_CTRL_GSCCS _MMIO(0x11a050) Alignment still seems off? Other than that, Reviewed-by: Vinay Belgaumkar +#define IDLE_MSG_DISABLE BIT(0) +#define PWRCTX_MAXCNT_GSCCS_MMIO(0x11a054) + #define FORCEWAKE_MEDIA_GEN9 _MMIO(0xa270) #define FORCEWAKE_RENDER_GEN9 _MMIO(0xa278)
Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote: [...] > new file mode 100644 > index ..c3cf6f92a766 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml > @@ -0,0 +1,110 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and > i.MX27 > + > +maintainers: > + - Sascha Hauer > + - Pengutronix Kernel Team > + > +properties: > + compatible: > +oneOf: > + - items: > + - enum: > + - fsl,imx1-fb > + - fsl,imx21-fb Are the items/enum keywords superfluous here? Couldn't this just be two - const: fsl,imx1-fb - const: fsl,imx21-fb entries? > + - items: > + - enum: > + - fsl,imx25-fb > + - fsl,imx27-fb > + - const: fsl,imx21-fb > + > + clocks: > +maxItems: 3 > + > + clock-names: > +items: > + - const: ipg > + - const: ahb > + - const: per clocks and clock-names are new, so this is a little bit more than a straight forward conversion. I'd mention this in the commit description. regards Philipp
Re: [PATCH] drm/i915/guc: add the GSC CS to the GuC capture list
I'm assuming that you have verified that the GuC ADS code is calling for the registration of the GSC capture register list accordingly and for the correct tile. That said: Reviewed-by: Alan Previn On Thu, 2022-11-10 at 16:18 -0800, Ceraolo Spurio, Daniele wrote: > For the GSC engine we only want to capture the instance regs. > > Signed-off-by: Daniele Ceraolo Spurio > Cc: Alan Previn > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c > b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c > index 4e6dca707d94..1d49a7ec0bd8 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c > @@ -132,6 +132,11 @@ static const struct __guc_mmio_reg_descr > xe_lpd_blt_inst_regs[] = { > COMMON_BASE_ENGINE_INSTANCE, > }; > > +/* XE_LPD - GSC Per-Engine-Instance */ > +static const struct __guc_mmio_reg_descr xe_lpd_gsc_inst_regs[] = { > + COMMON_BASE_ENGINE_INSTANCE, > +}; > + > /* GEN9 - Global */ > static const struct __guc_mmio_reg_descr default_global_regs[] = { > COMMON_BASE_GLOBAL, > @@ -177,6 +182,8 @@ static struct __guc_mmio_reg_descr_group default_lists[] > = { > MAKE_REGLIST(xe_lpd_vec_inst_regs, PF, ENGINE_INSTANCE, > GUC_VIDEOENHANCE_CLASS), > MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_BLITTER_CLASS), > MAKE_REGLIST(xe_lpd_blt_inst_regs, PF, ENGINE_INSTANCE, > GUC_BLITTER_CLASS), > + MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_GSC_OTHER_CLASS), > + MAKE_REGLIST(xe_lpd_gsc_inst_regs, PF, ENGINE_INSTANCE, > GUC_GSC_OTHER_CLASS), > {} > }; > > @@ -192,6 +199,8 @@ static const struct __guc_mmio_reg_descr_group > xe_lpd_lists[] = { > MAKE_REGLIST(xe_lpd_vec_inst_regs, PF, ENGINE_INSTANCE, > GUC_VIDEOENHANCE_CLASS), > MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_BLITTER_CLASS), > MAKE_REGLIST(xe_lpd_blt_inst_regs, PF, ENGINE_INSTANCE, > GUC_BLITTER_CLASS), > + MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_GSC_OTHER_CLASS), > + MAKE_REGLIST(xe_lpd_gsc_inst_regs, PF, ENGINE_INSTANCE, > GUC_GSC_OTHER_CLASS), > {} > }; > > @@ -454,6 +463,8 @@ __stringify_engclass(u32 class) > return "Blitter"; > case GUC_COMPUTE_CLASS: > return "Compute"; > + case GUC_GSC_OTHER_CLASS: > + return "GSC-Other"; > default: > break; > } > -- > 2.37.3 >
Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote: > This is a straight forward conversion. Note that fsl,imx-lcdc was picked > as the new name as this is the compatible that should supersede the > legacy fb binding. > > Signed-off-by: Uwe Kleine-König > --- > Hello, > > the eventual goal is to add drm support for this hardware. That one will > use a different (and more sensible) binding. However fsl,imx*-fb won't > go away directly, and Rob requested to describe both bindings in the > same file given that it describes a single hardware type. > > As a first step I convert the old binding to yaml. I tried to put the > new binding on top of that but I'm not sure about a few things in this > patch and so post only this first patch and once it's accepted add the > new binding which I guess is less overall work. > > What I'm unsure about is the description of the display node (Is there a > better description? I didn't find a schema for that.) That's going to be a challenge to describe because every panel binding will need a reference to those custom properties. It's a similar problem that spi-peripheral-props.yaml solved. But here, there may not be enough instances to do such a general solution. Do the panels used even have schemas yet? It's kind of a separate problem. You could start with just creating a schema just listing the custom properties. > Further I didn't find documentation about additionalProperties and > unevaluatedProperties. Did I pick the right one here? example-schema.yaml talks about it some. In general, if there's a $ref to other properties for a node not defined locally, then you need unevaluatedProperties. Otherwise, additionalProperties is fine. > Best regards > Uwe > > .../bindings/display/imx/fsl,imx-fb.txt | 57 - > .../bindings/display/imx/fsl,imx-lcdc.yaml| 110 ++ > 2 files changed, 110 insertions(+), 57 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt > create mode 100644 > Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml [...] > diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml > b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml > new file mode 100644 > index ..c3cf6f92a766 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml > @@ -0,0 +1,110 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and > i.MX27 > + > +maintainers: > + - Sascha Hauer > + - Pengutronix Kernel Team > + > +properties: > + compatible: > +oneOf: > + - items: > + - enum: > + - fsl,imx1-fb > + - fsl,imx21-fb > + - items: > + - enum: > + - fsl,imx25-fb > + - fsl,imx27-fb > + - const: fsl,imx21-fb > + > + clocks: > +maxItems: 3 > + > + clock-names: > +items: > + - const: ipg > + - const: ahb > + - const: per > + > + display: > +$ref: /schemas/types.yaml#/definitions/phandle > +description: | > + Display hardware node > + It needs to define the following properties: > +- bits-per-pixel > +- fsl,pcr: LCDC PCR value > + And optionally: > +- fsl,aus-mode: boolean to enable AUS mode (only for imx21) > + > + interrupts: > +maxItems: 1 > + > + reg: > +maxItems: 1 > + > + lcd-supply: > +description: > + Regulator for LCD supply voltage. > + > + fsl,dmacr: > +$ref: '/schemas/types.yaml#/definitions/uint32' Drop quotes. > +description: > + Override value for DMA Control Register > + > + fsl,lpccr: > +$ref: '/schemas/types.yaml#/definitions/uint32' Drop quotes. > +description: > + Contrast Control Register value. > + > + fsl,lscr1: > +$ref: '/schemas/types.yaml#/definitions/uint32' Drop quotes. > +description: > + LCDC Sharp Configuration Register value. > + > +required: > + - compatible > + - clocks > + - clock-names > + - display > + - interrupts > + - reg > + > +additionalProperties: false > + > +examples: > + - | > +imxfb: fb@10021000 { lcd-controller@... > +compatible = "fsl,imx21-fb"; > +interrupts = <61>; > +reg = <0x10021000 0x1000>; > +display = <&display0>; > +clocks = <&clks 103>, <&clks 49>, <&clks 66>; > +clock-names = "ipg", "ahb", "per"; > +}; > + > +display0: display0 { > +model = "Primeview-PD050VL1"; > +bits-per-pixel = <16>; > +fsl,pcr = <0xf0c88080>; /* non-standard but required */ > + > +display-timings { > +native-mode = <&timing_disp0>; > +timing_disp0: timing0 { > +hact
Re: [PATCH 2/2] drm/tests: helpers: Add SPDX header
Hi Maxime, On 11/16/22 12:18, Maxime Ripard wrote: > The SPDX header is missing, let's add it and fix the corresponding > checkpatch warning. > > Suggested-by: Maíra Canal > Fixes: 44a3928324e9 ("drm/tests: Add Kunit Helpers") > Signed-off-by: Maxime Ripard Could you add the SPDX-License-Identifier tag on the header file as well? With the SPDX header on both files, this is: Reviewed-by: Maíra Canal Best Regards, - Maíra Canal > --- > drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c > b/drivers/gpu/drm/tests/drm_kunit_helpers.c > index eea450de7de8..f1662091f250 100644 > --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c > +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c > @@ -1,3 +1,5 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > #include > #include >
Re: [PATCH 1/2] drm/tests: client: Remove extra blank lines
On 11/16/22 12:18, Maxime Ripard wrote: > Some extra blank lines slipped through, remove them. > > Fixes: 8fc0380f6ba7 ("drm/client: Add some tests for > drm_connector_pick_cmdline_mode()") > Signed-off-by: Maxime Ripard Reviewed-by: Maíra Canal Best Regards, - Maíra Canal > --- > drivers/gpu/drm/tests/drm_client_modeset_test.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c > b/drivers/gpu/drm/tests/drm_client_modeset_test.c > index 558c098b0384..362a5fbd82f5 100644 > --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c > +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c > @@ -53,7 +53,6 @@ static int drm_client_modeset_test_init(struct kunit *test) > drm_connector_helper_add(&priv->connector, > &drm_client_modeset_connector_helper_funcs); > > return 0; > - > } > > static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test) > @@ -85,7 +84,6 @@ static void drm_test_pick_cmdline_res_1920_1080_60(struct > kunit *test) > KUNIT_EXPECT_TRUE(test, drm_mode_equal(expected_mode, mode)); > } > > - > static struct kunit_case drm_test_pick_cmdline_tests[] = { > KUNIT_CASE(drm_test_pick_cmdline_res_1920_1080_60), > {}
[PATCH] dt-bindings: msm/dsi: Don't require vdds-supply on 10nm PHY
On some SoCs (hello SM6350) vdds-supply is not wired to any smd-rpm or rpmh regulator, but instead powered by the VDD_MX/mx.lvl line, which is voted for in the DSI ctrl node. Signed-off-by: Konrad Dybcio --- Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml index d9ad8b659f58..3ec466c3ab38 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml @@ -69,7 +69,6 @@ required: - compatible - reg - reg-names - - vdds-supply unevaluatedProperties: false -- 2.38.1
Re: [PATCH v3 05/10] dt-bindings: display: bridge: Add MHDP DP for i.MX8MQ
On Tue, Nov 08, 2022 at 09:00:08PM +0800, Sandor Yu wrote: > Add bindings for i.MX8MQ MHDP DisplayPort. > > Signed-off-by: Sandor Yu > --- > .../display/bridge/cdns,mhdp-imx8mq-dp.yaml | 59 +++ > 1 file changed, 59 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/cdns,mhdp-imx8mq-dp.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp-imx8mq-dp.yaml > b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp-imx8mq-dp.yaml > new file mode 100644 > index ..c4d5362db2b5 > --- /dev/null > +++ > b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp-imx8mq-dp.yaml > @@ -0,0 +1,59 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/cdns,mhdp-imx8mq-dp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Cadence MHDP Displayport bridge > + > +maintainers: > + - Sandor Yu > + > +description: > + The Cadence MHDP Displayport TX interface. > + > +properties: > + compatible: > +enum: > + - cdns,mhdp-imx8mq-dp > + > + reg: > +maxItems: 1 > + > + phys: > +maxItems: 1 > + > + interrupts: > +items: > + - description: Hotplug detect interrupter for cable plugin event. > + - description: Hotplug detect interrupter for cable plugout event. > + > + interrupt-names: > +items: > + - const: plug_in > + - const: plug_out > + > + port: > +$ref: /schemas/graph.yaml#/properties/port > +description: > + A port node pointing to the output port of a display controller. Similarly, you need an output port to DP (or USB-C) connector. > + > +additionalProperties: false > + > +examples: > + - | > +#include > +mhdp_dp: dp-bridge@32c0 { > +compatible = "cdns,mhdp-imx8mq-dp"; > +reg = <0x32c0 0x10>; > +interrupts = , > +; > +interrupt-names = "plug_in", "plug_out"; > +phys = <&dp_phy>; > + > +port { > +mhdp_in: endpoint { > +remote-endpoint = <&dcss_out>; > +}; > +}; > +}; > -- > 2.34.1 > >
Re: [PATCH v3 02/10] dt-bindings: display: bridge: Add MHDP HDMI for i.MX8MQ
On Tue, Nov 08, 2022 at 09:00:05PM +0800, Sandor Yu wrote: > Add bindings for i.MX8MQ MHDP HDMI. > > Signed-off-by: Sandor Yu > --- > .../display/bridge/cdns,mhdp-imx8mq-hdmi.yaml | 59 +++ > 1 file changed, 59 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/cdns,mhdp-imx8mq-hdmi.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp-imx8mq-hdmi.yaml > b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp-imx8mq-hdmi.yaml > new file mode 100644 > index ..8c0afef157aa > --- /dev/null > +++ > b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp-imx8mq-hdmi.yaml > @@ -0,0 +1,59 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/cdns,mhdp-imx8mq-hdmi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Cadence MHDP HDMI bridge > + > +maintainers: > + - Sandor Yu > + > +description: > + The Cadence MHDP TX HDMI interface. > + > +properties: > + compatible: > +enum: > + - cdns,mhdp-imx8mq-hdmi > + > + reg: > +maxItems: 1 > + > + phys: > +maxItems: 1 > + > + interrupts: > +items: > + - description: Hotplug detect interrupter for cable plugin event. > + - description: Hotplug detect interrupter for cable plugout event. > + > + interrupt-names: > +items: > + - const: plug_in > + - const: plug_out > + > + port: > +$ref: /schemas/graph.yaml#/properties/port > +description: > + A port node pointing to the output port of a display controller. You also need an output port to an HDMI connector node. > + > +additionalProperties: false > + > +examples: > + - | > +#include > +mhdp_hdmi: hdmi-bridge@32c0 { > +compatible = "cdns,mhdp-imx8mq-hdmi"; > +reg = <0x32c0 0x10>; > +interrupts = , > +; > +interrupt-names = "plug_in", "plug_out"; > +phys = <&hdmi_phy>; > + > +port { > +mhdp_in: endpoint { > +remote-endpoint = <&dcss_out>; > +}; > +}; > +}; > -- > 2.34.1 > >
RE: [v1] drm/msm/disp/dpu1: add color management support for the crtc
>-Original Message- >From: Dmitry Baryshkov >Sent: Wednesday, November 16, 2022 8:37 PM >To: Kalyan Thota ; Kalyan Thota (QUIC) >; dri-devel@lists.freedesktop.org; linux-arm- >m...@vger.kernel.org; freedr...@lists.freedesktop.org; >devicet...@vger.kernel.org >Cc: linux-ker...@vger.kernel.org; robdcl...@chromium.org; >diand...@chromium.org; swb...@chromium.org; Vinod Polimera (QUIC) >; Abhinav Kumar (QUIC) > >Subject: Re: [v1] drm/msm/disp/dpu1: add color management support for the >crtc > >WARNING: This email originated from outside of Qualcomm. Please be wary of >any links or attachments, and do not enable macros. > >On 16/11/2022 17:29, Kalyan Thota wrote: >> >> >>> -Original Message- >>> From: Dmitry Baryshkov >>> Sent: Saturday, November 12, 2022 4:13 AM >>> To: Kalyan Thota (QUIC) ; dri- >>> de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org; >>> freedr...@lists.freedesktop.org; devicet...@vger.kernel.org >>> Cc: linux-ker...@vger.kernel.org; robdcl...@chromium.org; >>> diand...@chromium.org; swb...@chromium.org; Vinod Polimera (QUIC) >>> ; Abhinav Kumar (QUIC) >>> >>> Subject: Re: [v1] drm/msm/disp/dpu1: add color management support for >>> the crtc >>> >>> WARNING: This email originated from outside of Qualcomm. Please be >>> wary of any links or attachments, and do not enable macros. >>> >>> On 11/11/2022 16:57, Kalyan Thota wrote: Add color management support for the crtc provided there are enough dspps that can be allocated from the catalogue. Changes in v1: - cache color enabled state in the dpu crtc obj (Dmitry) - simplify dspp allocation while creating crtc (Dmitry) - register for color when crtc is created (Dmitry) Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 5 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 53 >>> - 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 4170fbe..ca4c3b3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1571,7 +1571,7 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = { /* initialize crtc */ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane >>> *plane, - struct drm_plane *cursor) + struct drm_plane *cursor, unsigned + long + features) { struct drm_crtc *crtc = NULL; struct dpu_crtc *dpu_crtc = NULL; @@ -1583,6 +1583,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, crtc = &dpu_crtc->base; crtc->dev = dev; + dpu_crtc->color_enabled = features & BIT(DPU_DSPP_PCC); spin_lock_init(&dpu_crtc->spin_lock); atomic_set(&dpu_crtc->frame_pending, 0); @@ -1604,7 +1605,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs); - drm_crtc_enable_color_mgmt(crtc, 0, true, 0); + drm_crtc_enable_color_mgmt(crtc, 0, dpu_crtc->color_enabled, + 0); /* save user friendly CRTC name for later */ snprintf(dpu_crtc->name, DPU_CRTC_NAME_SIZE, "crtc%u", crtc->base.id); diff --git crtc->a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index 539b68b..342f9ae 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -136,6 +136,7 @@ struct dpu_crtc_frame_event { * @enabled : whether the DPU CRTC is currently enabled. updated in >the * commit-thread, not state-swap time which is earlier, so * safe to make decisions on during VBLANK on/off work + * @color_enabled : whether crtc supports color management * @feature_list : list of color processing features supported on a crtc * @active_list : list of color processing features are active * @dirty_list: list of color processing features are dirty @@ -164,7 +165,7 @@ struct dpu_crtc { u64 play_count; ktime_t vblank_cb_time; bool enabled; - + bool color_enabled; struct list_head feature_list; struct list_head active_list; struct list_head dirty_list; @@ -269,10 +270,11 @@ void dpu_crtc_complete_commit(struct drm_crtc >>> *crtc); * @dev: dpu device * @plane: base plane * @cur
[PATCH printk v5 32/40] printk, xen: fbfront: create/use safe function for forcing preferred
With commit 9e124fe16ff2("xen: Enable console tty by default in domU if it's not a dummy") a hack was implemented to make sure that the tty console remains the console behind the /dev/console device. The main problem with the hack is that, after getting the console pointer to the tty console, it is assumed the pointer is still valid after releasing the console_sem. This assumption is incorrect and unsafe. Make the hack safe by introducing a new function console_force_preferred_locked() and perform the full operation under the console_list_lock. Signed-off-by: John Ogness Reviewed-by: Petr Mladek --- drivers/video/fbdev/xen-fbfront.c | 12 +++- include/linux/console.h | 1 + kernel/printk/printk.c| 49 +-- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c index 4d2694d904aa..8752d389e382 100644 --- a/drivers/video/fbdev/xen-fbfront.c +++ b/drivers/video/fbdev/xen-fbfront.c @@ -504,18 +504,14 @@ static void xenfb_make_preferred_console(void) if (console_set_on_cmdline) return; - console_lock(); + console_list_lock(); for_each_console(c) { if (!strcmp(c->name, "tty") && c->index == 0) break; } - console_unlock(); - if (c) { - unregister_console(c); - c->flags |= CON_CONSDEV; - c->flags &= ~CON_PRINTBUFFER; /* don't print again */ - register_console(c); - } + if (c) + console_force_preferred_locked(c); + console_list_unlock(); } static int xenfb_resume(struct xenbus_device *dev) diff --git a/include/linux/console.h b/include/linux/console.h index f716e1dd9eaf..9cea254b34b8 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -291,6 +291,7 @@ enum con_flush_mode { }; extern int add_preferred_console(char *name, int idx, char *options); +extern void console_force_preferred_locked(struct console *con); extern void register_console(struct console *); extern int unregister_console(struct console *); extern void console_lock(void); diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 410d3f2cdeb3..ece34abbc9cc 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -247,9 +247,10 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write, void console_list_lock(void) { /* -* In unregister_console(), synchronize_srcu() is called with the -* console_list_lock held. Therefore it is not allowed that the -* console_list_lock is taken with the srcu_lock held. +* In unregister_console() and console_force_preferred_locked(), +* synchronize_srcu() is called with the console_list_lock held. +* Therefore it is not allowed that the console_list_lock is taken +* with the srcu_lock held. * * Detecting if this context is really in the read-side critical * section is only possible if the appropriate debug options are @@ -3489,6 +3490,48 @@ int unregister_console(struct console *console) } EXPORT_SYMBOL(unregister_console); +/** + * console_force_preferred_locked - force a registered console preferred + * @con: The registered console to force preferred. + * + * Must be called under console_list_lock(). + */ +void console_force_preferred_locked(struct console *con) +{ + struct console *cur_pref_con; + + if (!console_is_registered_locked(con)) + return; + + cur_pref_con = console_first(); + + /* Already preferred? */ + if (cur_pref_con == con) + return; + + /* +* Delete, but do not re-initialize the entry. This allows the console +* to continue to appear registered (via any hlist_unhashed_lockless() +* checks), even though it was briefly removed from the console list. +*/ + hlist_del_rcu(&con->node); + + /* +* Ensure that all SRCU list walks have completed so that the console +* can be added to the beginning of the console list and its forward +* list pointer can be re-initialized. +*/ + synchronize_srcu(&console_srcu); + + con->flags |= CON_CONSDEV; + WARN_ON(!con->device); + + /* Only the new head can have CON_CONSDEV set. */ + console_srcu_write_flags(cur_pref_con, cur_pref_con->flags & ~CON_CONSDEV); + hlist_add_head_rcu(&con->node, &console_list); +} +EXPORT_SYMBOL(console_force_preferred_locked); + /* * Initialize the console device. This is called *early*, so * we can't necessarily depend on lots of kernel help here. -- 2.30.2
[PATCH printk v5 00/40] reduce console_lock scope
This is v5 of a series to prepare for threaded/atomic printing. v4 is here [0]. This series focuses on reducing the scope of the BKL console_lock. It achieves this by switching to SRCU and a dedicated mutex for console list iteration and modification, respectively. The console_lock will no longer offer this protection. Also, during the review of v2 it came to our attention that many console drivers are checking CON_ENABLED to see if they are registered. Because this flag can change without unregistering and because this flag does not represent an atomic point when an (un)registration process is complete, a new console_is_registered() function is introduced. This function uses the console_list_lock to synchronize with the (un)registration process to provide a reliable status. All users of the console_lock for list iteration have been modified. For the call sites where the console_lock is still needed (for other reasons), comments are added to explain exactly why the console_lock is needed. All users of CON_ENABLED for registration status have been modified to use console_is_registered(). Note that there are still users of CON_ENABLED, but this is for legitimate purposes about a registered console being able to print. The base commit for this series is from Paul McKenney's RCU tree and provides an NMI-safe SRCU implementation [1]. Without the NMI-safe SRCU implementation, this series is not less safe than mainline. But we will need the NMI-safe SRCU implementation for atomic consoles anyway, so we might as well get it in now. Especially since it _does_ increase the reliability for mainline in the panic path. Changes since v4: printk: - Introduce console_init_seq() to handle the now rather complex procedure to find an appropriate start sequence number for a new console upon registration. - When registering a non-boot console and boot consoles are registered, try to flush all the consoles to get the next @seq value before falling back to use the @seq of the enabled boot console that is furthest behind. - For console_force_preferred_locked(), make the console the head of the console list. John Ogness [0] https://lore.kernel.org/lkml/20221114162932.141883-1-john.ogn...@linutronix.de [1] https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/log/?h=srcunmisafe.2022.11.09a John Ogness (38): printk: Prepare for SRCU console list protection printk: register_console: use "registered" for variable names printk: move @seq initialization to helper printk: fix setting first seq for consoles um: kmsg_dump: only dump when no output console available tty: serial: kgdboc: document console_lock usage tty: tty_io: document console_lock usage proc: consoles: document console_lock usage printk: introduce console_list_lock console: introduce wrappers to read/write console flags um: kmsg_dumper: use srcu console list iterator kdb: use srcu console list iterator printk: console_flush_all: use srcu console list iterator printk: __pr_flush: use srcu console list iterator printk: console_is_usable: use console_srcu_read_flags printk: console_unblank: use srcu console list iterator printk: console_flush_on_panic: use srcu console list iterator printk: console_device: use srcu console list iterator console: introduce console_is_registered() serial_core: replace uart_console_enabled() with uart_console_registered() tty: nfcon: use console_is_registered() efi: earlycon: use console_is_registered() tty: hvc: use console_is_registered() tty: serial: earlycon: use console_is_registered() tty: serial: pic32_uart: use console_is_registered() tty: serial: samsung_tty: use console_is_registered() tty: serial: xilinx_uartps: use console_is_registered() usb: early: xhci-dbc: use console_is_registered() netconsole: avoid CON_ENABLED misuse to track registration printk, xen: fbfront: create/use safe function for forcing preferred tty: tty_io: use console_list_lock for list synchronization proc: consoles: use console_list_lock for list iteration tty: serial: kgdboc: use srcu console list iterator tty: serial: kgdboc: use console_list_lock for list traversal tty: serial: kgdboc: synchronize tty_find_polling_driver() and register_console() tty: serial: kgdboc: use console_list_lock to trap exit printk: relieve console_lock of list synchronization duties tty: serial: sh-sci: use setup() callback for early console Thomas Gleixner (2): serial: kgdboc: Lock console list in probe function printk: Convert console_drivers list to hlist .clang-format | 1 + arch/m68k/emu/nfcon.c | 9 +- arch/um/kernel/kmsg_dump.c | 24 +- drivers/firmware/efi/earlycon.c | 8 +- drivers/net/netconsole.c| 21 +- drivers/tty/hvc/hvc_console.c | 4 +- drivers/tty/serial/8250/8250_core.c | 2 +- drivers/tty/serial/earlycon.c | 4 +- drivers/tty/serial/kgdboc.c
[PATCH 0/7] drm: Fix the color-depth/bpp confusion
A number of drivers mix up the meaning of bits-per-pixel and color depth. For each of them, set the correct values. As a rule of thumb, the color depth is the number of color and alpha bits that affect image composition. The bpp value is the color depth in the pixel plus the filler bits. The color depth is exported to userspace, while the bpp value only affects fbdev emulation. Currently, fbdev fails if it selects a color format that is unsupported by the driver. The fix would be to fall back to a driver default value for the bpp. Getting the default fixed in drivers will then allow us to fix the fbdev format selection. Thomas Zimmermann (7): drm/hisilicon/hibmc: Fix preferred depth and bpp drm/logicvc: Fix preferred fbdev cpp drm/cirrus: Decouple fbdev bpp value from color depth drm/ofdrm: Set preferred depth from format of scanout buffer drm/simpledrm: Set preferred depth from format of scanout buffer drm/solomon: Set preferred color depth and bpp to the correct values drm/fb-helper: Don't use the preferred depth for the BPP default drivers/gpu/drm/drm_fbdev_generic.c | 15 +-- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 4 ++-- drivers/gpu/drm/logicvc/logicvc_drm.c | 14 +- drivers/gpu/drm/solomon/ssd130x.c | 4 ++-- drivers/gpu/drm/tiny/cirrus.c | 2 +- drivers/gpu/drm/tiny/ofdrm.c| 13 + drivers/gpu/drm/tiny/simpledrm.c| 4 ++-- 7 files changed, 30 insertions(+), 26 deletions(-) -- 2.38.1
[PATCH 2/7] drm/logicvc: Fix preferred fbdev cpp
Logicvc can have different values for the preferred color depth. Set the fbdev bpp value depending on the runtime value. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/logicvc/logicvc_drm.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c b/drivers/gpu/drm/logicvc/logicvc_drm.c index 9de24d9f0c963..d9cd5d967e31f 100644 --- a/drivers/gpu/drm/logicvc/logicvc_drm.c +++ b/drivers/gpu/drm/logicvc/logicvc_drm.c @@ -301,6 +301,7 @@ static int logicvc_drm_probe(struct platform_device *pdev) struct regmap *regmap = NULL; struct resource res; void __iomem *base; + unsigned int preferred_bpp; int irq; int ret; @@ -438,7 +439,18 @@ static int logicvc_drm_probe(struct platform_device *pdev) goto error_mode; } - drm_fbdev_generic_setup(drm_dev, drm_dev->mode_config.preferred_depth); + switch (drm_dev->mode_config.preferred_depth) { + case 15: + case 16: + preferred_bpp = 16; + break; + case 24: + case 32: + default: + preferred_bpp = 32; + break; + } + drm_fbdev_generic_setup(drm_dev, preferred_bpp); return 0; -- 2.38.1
[PATCH 5/7] drm/simpledrm: Set preferred depth from format of scanout buffer
Set the preferred depth from the format of the scanout buffer. The value cannot be hardcoded, as the scanout buffer's is only known at runtime. Also derive the fbdev emulations bpp value from the scanout format. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tiny/simpledrm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 162eb44dcba89..30e928d627e8f 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -739,7 +739,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv, dev->mode_config.max_width = max_width; dev->mode_config.min_height = height; dev->mode_config.max_height = max_height; - dev->mode_config.preferred_depth = format->cpp[0] * 8; + dev->mode_config.preferred_depth = format->depth; dev->mode_config.funcs = &simpledrm_mode_config_funcs; /* Primary plane */ @@ -834,7 +834,7 @@ static int simpledrm_probe(struct platform_device *pdev) if (ret) return ret; - drm_fbdev_generic_setup(dev, 0); + drm_fbdev_generic_setup(dev, drm_format_info_bpp(sdev->format, 0)); return 0; } -- 2.38.1
[PATCH 6/7] drm/solomon: Set preferred color depth and bpp to the correct values
Set the preferred color depth to 24 bits and the fbdev bpp to 32 bits. This will signal XRGB as default format to clients. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/solomon/ssd130x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c index 53464afc2b9ac..c3bf3a18302ea 100644 --- a/drivers/gpu/drm/solomon/ssd130x.c +++ b/drivers/gpu/drm/solomon/ssd130x.c @@ -876,7 +876,7 @@ static int ssd130x_init_modeset(struct ssd130x_device *ssd130x) drm->mode_config.max_width = max_width; drm->mode_config.min_height = mode->vdisplay; drm->mode_config.max_height = max_height; - drm->mode_config.preferred_depth = 32; + drm->mode_config.preferred_depth = 24; drm->mode_config.funcs = &ssd130x_mode_config_funcs; /* Primary plane */ @@ -1006,7 +1006,7 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap) if (ret) return ERR_PTR(dev_err_probe(dev, ret, "DRM device register failed\n")); - drm_fbdev_generic_setup(drm, 0); + drm_fbdev_generic_setup(drm, 32); return ssd130x; } -- 2.38.1
[PATCH 7/7] drm/fb-helper: Don't use the preferred depth for the BPP default
If no preferred value for bits-per-pixel has been given, fall back to 32. Never use the preferred depth. The color depth is the number of color/alpha bits per pixel, while bpp is the overall number of bits in most cases. Most noteworthy, XRGB has a depth of 24 and a bpp value of 32. Using depth for bpp would make the value 24 as well and format selection in fbdev helpers fails. Unfortunately XRGB is the most common format and the old heuristic therefore fails for most of the drivers (unless they implement the 24-bit RGB888 format). Picking a bpp of 32 will lateron result in a default depth of 24 and the format XRGB. As XRGB is the default format for most of the current and legacy graphics stack, all drivers must support it. So it is the safe choice. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/drm_fbdev_generic.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index ab86956692795..0a4c160e0e58a 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -431,7 +431,6 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = { * drm_fbdev_generic_setup() - Setup generic fbdev emulation * @dev: DRM device * @preferred_bpp: Preferred bits per pixel for the device. - * @dev->mode_config.preferred_depth is used if this is zero. * * This function sets up generic fbdev emulation for drivers that supports * dumb buffers with a virtual address and that can be mmap'ed. @@ -475,12 +474,16 @@ void drm_fbdev_generic_setup(struct drm_device *dev, } /* -* FIXME: This mixes up depth with bpp, which results in a glorious -* mess, resulting in some drivers picking wrong fbdev defaults and -* others wrong preferred_depth defaults. +* Pick a preferred bpp of 32 if no value has been given. This +* will select XRGB for the framebuffer formats. All drivers +* have to support XRGB for backwards compatibility with legacy +* userspace, so it's the safe choice here. +* +* TODO: Replace struct drm_mode_config.preferred_depth and this +* bpp value with a preferred format that is given as struct +* drm_format_info. Then derive all other values from the +* format. */ - if (!preferred_bpp) - preferred_bpp = dev->mode_config.preferred_depth; if (!preferred_bpp) preferred_bpp = 32; fb_helper->preferred_bpp = preferred_bpp; -- 2.38.1
[PATCH 3/7] drm/cirrus: Decouple fbdev bpp value from color depth
Cirrus has a preferred color depth of 16 bit; also use it as fbdev bpp value. Don't use the color depth directly. It has a different meaning than bpp and both cannot be used interchangeably. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tiny/cirrus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c index 678c2ef1cae70..cf35b60905032 100644 --- a/drivers/gpu/drm/tiny/cirrus.c +++ b/drivers/gpu/drm/tiny/cirrus.c @@ -604,7 +604,7 @@ static int cirrus_pci_probe(struct pci_dev *pdev, if (ret) return ret; - drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth); + drm_fbdev_generic_setup(dev, 16); return 0; } -- 2.38.1
[PATCH 4/7] drm/ofdrm: Set preferred depth from format of scanout buffer
Set the preferred depth from the format of the scanout buffer. The value cannot be hardcoded, as the scanout buffer's is only known at runtime. Keeping the existing switch statement just duplicates the driver's existing logic for format detection. Also remove the FIXME comment from the call to drm_fbdev_generic_setup() as the driver now handles color depth and bpp values correctly. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tiny/ofdrm.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c index dc9e4d71b12ae..33eefeba437c5 100644 --- a/drivers/gpu/drm/tiny/ofdrm.c +++ b/drivers/gpu/drm/tiny/ofdrm.c @@ -1284,14 +1284,7 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv, dev->mode_config.min_height = height; dev->mode_config.max_height = max_height; dev->mode_config.funcs = &ofdrm_mode_config_funcs; - switch (depth) { - case 32: - dev->mode_config.preferred_depth = 24; - break; - default: - dev->mode_config.preferred_depth = depth; - break; - } + dev->mode_config.preferred_depth = format->depth; dev->mode_config.quirk_addfb_prefer_host_byte_order = true; /* Primary plane */ @@ -1390,10 +1383,6 @@ static int ofdrm_probe(struct platform_device *pdev) if (ret) return ret; - /* -* FIXME: 24-bit color depth does not work reliably with a 32-bpp -* value. Force the bpp value of the scanout buffer's format. -*/ drm_fbdev_generic_setup(dev, drm_format_info_bpp(odev->format, 0)); return 0; -- 2.38.1
[PATCH 1/7] drm/hisilicon/hibmc: Fix preferred depth and bpp
Set the preferred color depth to 24 bits and the fbdev bpp to 32 bits. This will signal XRGB as default format to clients. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 22053c613644a..0c4aa4d9b0a77 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -106,7 +106,7 @@ static int hibmc_kms_init(struct hibmc_drm_private *priv) dev->mode_config.max_width = 1920; dev->mode_config.max_height = 1200; - dev->mode_config.preferred_depth = 32; + dev->mode_config.preferred_depth = 24; dev->mode_config.prefer_shadow = 1; dev->mode_config.funcs = (void *)&hibmc_mode_funcs; @@ -340,7 +340,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, goto err_unload; } - drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth); + drm_fbdev_generic_setup(dev, 32); return 0; -- 2.38.1
Re: [PATCH v2 2/3] drm/msm/disp/dpu1: add helper to know if display is pluggable
On 16/11/2022 18:35, Abhinav Kumar wrote: On 11/16/2022 7:18 AM, Dmitry Baryshkov wrote: On 16/11/2022 18:11, Abhinav Kumar wrote: On 11/16/2022 7:08 AM, Dmitry Baryshkov wrote: On 16/11/2022 17:30, Kalyan Thota wrote: Since DRM encoder type for few encoders can be similar (like eDP and DP) find out if the interface supports HPD from encoder bridge to differentiate between builtin and pluggable displays. Changes in v1: - add connector type in the disp_info (Dmitry) - add helper functions to know encoder type - update commit text reflecting the change Changes in v2: - avoid hardcode of connector type for DSI as it may not be true (Dmitry) - get the HPD information from encoder bridge Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++ 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 9c6817b..be93269 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "msm_drv.h" #include "dpu_kms.h" @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) +{ + struct drm_bridge *bridge; + int ops = 0; + + if (!encoder) + return false; + + /* Get last bridge in the chain to determine pluggable state */ + drm_for_each_bridge_in_chain(encoder, bridge) + if (!drm_bridge_get_next_bridge(bridge)) + ops = bridge->ops; + + return ops & DRM_BRIDGE_OP_HPD; No. This is not what you should be checking (hint: polled connectors also can be pluggable). Please check the type of the actual connector connected to this encoder. Even if we check the connector type as DSI or eDP that does not necessarily mean its built-in. We can even use DSI or eDP as a pluggable display. Well, I don't think so. eDP and DSI connectors are not pluggable per design. One can use them so, but they are not thought to be used this way. Unlike e.g. HDMI, DP, VGA, etc. We have had many products where we used HDMI as the primary display where the HPD line was disconnected in the design, so now if we generalize all HDMI connectors to be pluggable we can never enable color management on those even though DSI is not even used in that product. Did they use HDMI connector internally? Or was it just a panel wired somehow to the HDMI pins? If the former is true, then they still are pluggable. Even if you don't have a way to detect that via the HPD pin. If the later is the case, then it shouldn't be DRM_MODE_CONNECTOR_HDMI. Well, even if this is an internal HDMI, I'd still use some other connector (e.g. DRM_MODE_CONNECTOR_Unknown) just to point out that this is not an externally visible HDMI connector one assumes to be able to find on the body of the device. Last, but not least, how would you remove DRM_BRIDGE_OPS_HPD from the corresponding bridge driver? Thats why I felt we should rely on the HPD_OPS as that way we know that it will be set only if HPD will be used. Wouldnt it be just better to also check polling displays to complete this check? Is there a way to do it? Yes, check DRM_BRIDGE_OP_DETECT. But as I noted, there is a list of connectors that will not ever have HPD or polling, but still always are external. Well, even for VGA there is no good way to detect whether it is plugged in or not (see the comment in display-connector.c). I would say LVDS, eDP, DSI, DPI and SPI can be assumed to be constantly plugged. Compare this with Composite, SVIDEO, 9PinDIN, TV. They can be assumed to be external even if they do not have the HPD (or even polling). And these connectors usually don't have it. Thats why we thought of this check. -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Never return 0 if request wait succeeds
On Wednesday, 16 November 2022 15:42:46 CET Andrzej Hajda wrote: > On 16.11.2022 12:25, Janusz Krzysztofik wrote: > > According to the docs of i915_request_wait_timeout(), its return value > > "may be zero if the request is unfinished after the timeout expires." > > However, 0 is also returned when the request is found finished right > > after the timeout has expired. > > > > Since the docs also state: "If the timeout is 0, it will return 1 if the > > fence is signaled.", return 1 also when the fence is found signaled after > > non-zero timeout has expired. > > As I understand the patch "drm/i915: Fix i915_request fence wait > semantics", and the docs "timeout is 0" means the initial value of > timeout argument and this is handled already on the beginning of the > function. > In case initial timeout is greater than zero and then it expires, > function should return 0 regardless of fence state. This is what I have > understood from reading docs and implementation of > dma_fence_default_wait [1], which should be the best source of info > about "dma_fence wait semantic". > > As I said already, mixing remaining time and bool in return value of > dma_fence_wait* functions is very confusing, but changing it would > require major rework of the framework. OK, let's drop this controversial patch. The corner case it touches should already be handled correctly by intel_gt_retire_requests_timeout(), which this series is about, after 1/3 and 2/3 are applied. Thanks, Janusz > > [1]: > https://elixir.bootlin.com/linux/latest/source/drivers/dma-buf/dma-fence.c#L753 > > Regards > Andrzej > > > > > Fixes: 7e2e69ed4678 ("drm/i915: Fix i915_request fence wait semantics") > > Signed-off-by: Janusz Krzysztofik > > Cc: sta...@vger.kernel.org # v5.17 > > --- > > drivers/gpu/drm/i915/i915_request.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c > > b/drivers/gpu/drm/i915/i915_request.c > > index f949a9495758a..406ddfafbed4d 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -2079,6 +2079,8 @@ long i915_request_wait_timeout(struct i915_request > > *rq, > > > > timeout = io_schedule_timeout(timeout); > > } > > + if (!timeout) /* expired but signaled, we shouldn't return 0 */ > > + timeout = 1; > > __set_current_state(TASK_RUNNING); > > > > if (READ_ONCE(wait.tsk)) > >
Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
Hi Chun-Kuang, > > + mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc); > > mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs, > > DISP_REG_OVL_CON(idx)); > > - mtk_ddp_write_relaxed(cmdq_pkt, pitch, &ovl->cmdq_reg, ovl->regs, > > + mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb, > > &ovl->cmdq_reg, ovl->regs, > > DISP_REG_OVL_PITCH(idx)); > > Is this general for all MediaTek SoC? If so, separate this to an > independent patch. Otherwise, use a device variable to separate this > setting. Yes and no. Technically all MediaTek SoCs have two separate registers for the pitch, each are 16 bits, so this code is technically always needed. However, because the lsb register is 16 bit, this issue has never come up, because nobody has tried to display a plane that was 16384 ARGB pixels across. In fact, I think most MediaTek SoCs have a resolution limit of 4K. The reason this issue comes up now is because "pitch" is calculated differently for AFBC frames, and actually refers to the size in bytes of one row of AFBC blocks. Should I still separate this into an independent patch? > > } > > @@ -492,6 +567,15 @@ static const struct mtk_disp_ovl_data > > mt8192_ovl_2l_driver_data = { > > .smi_id_en = true, > > }; > > > > +static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = { > > In this binding document, mt8195 ovl is compatible to mt8133 ovl. > Please confirm that mt8195 is not identical with mt8133. Do you mean MT8183? If so, we do not have any documentation indicating that the MT8183 supports AFBC. Do you have some reason to believe otherwise? > Usually the pitch needs alignment. So I guess the formula is > > hdr_pitch = ALIGN(width_in_blocks * AFBC_HEADER_BLOCK_SIZE, > AFBC_HEADER_ALIGNMENT); > hdr_size = hdr_pitch * height_in_blocks; The documentation does not indicate that the pitch needs alignment beyond the AFBC header block size. > Could you explain the meaning of hdr_pitch? hdr_pitch refers to the size in bytes of one row of AFBC header blocks. AFBC is a proprietary compressed frame buffer format, but from what public information we have, it appears to be block compressed data stored in 2 contiguous planes. The first is called the "header" plane, and the second is called the "body" plane. The header plane contains metadata for each block of pixel data, and the body plane contains sparse compressed block data. I'll send another patch with the other changes you mentioned. Thanks! Justin
Re: [PATCH v2 2/3] drm/msm/disp/dpu1: add helper to know if display is pluggable
On 11/16/2022 7:18 AM, Dmitry Baryshkov wrote: On 16/11/2022 18:11, Abhinav Kumar wrote: On 11/16/2022 7:08 AM, Dmitry Baryshkov wrote: On 16/11/2022 17:30, Kalyan Thota wrote: Since DRM encoder type for few encoders can be similar (like eDP and DP) find out if the interface supports HPD from encoder bridge to differentiate between builtin and pluggable displays. Changes in v1: - add connector type in the disp_info (Dmitry) - add helper functions to know encoder type - update commit text reflecting the change Changes in v2: - avoid hardcode of connector type for DSI as it may not be true (Dmitry) - get the HPD information from encoder bridge Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++ 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 9c6817b..be93269 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "msm_drv.h" #include "dpu_kms.h" @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) +{ + struct drm_bridge *bridge; + int ops = 0; + + if (!encoder) + return false; + + /* Get last bridge in the chain to determine pluggable state */ + drm_for_each_bridge_in_chain(encoder, bridge) + if (!drm_bridge_get_next_bridge(bridge)) + ops = bridge->ops; + + return ops & DRM_BRIDGE_OP_HPD; No. This is not what you should be checking (hint: polled connectors also can be pluggable). Please check the type of the actual connector connected to this encoder. Even if we check the connector type as DSI or eDP that does not necessarily mean its built-in. We can even use DSI or eDP as a pluggable display. Well, I don't think so. eDP and DSI connectors are not pluggable per design. One can use them so, but they are not thought to be used this way. Unlike e.g. HDMI, DP, VGA, etc. We have had many products where we used HDMI as the primary display where the HPD line was disconnected in the design, so now if we generalize all HDMI connectors to be pluggable we can never enable color management on those even though DSI is not even used in that product. Thats why I felt we should rely on the HPD_OPS as that way we know that it will be set only if HPD will be used. Wouldnt it be just better to also check polling displays to complete this check? Is there a way to do it? I would say LVDS, eDP, DSI, DPI and SPI can be assumed to be constantly plugged. Compare this with Composite, SVIDEO, 9PinDIN, TV. They can be assumed to be external even if they do not have the HPD (or even polling). And these connectors usually don't have it. Thats why we thought of this check.
Re: [PATCH 2/3] drm/tests: helpers: Add module infos
On Wed, 16 Nov 2022, Maxime Ripard wrote: > On Wed, Nov 16, 2022 at 01:32:51PM +0200, Jani Nikula wrote: >> On Wed, 16 Nov 2022, Maíra Canal wrote: >> > Hi Maxime, >> > >> > On 11/16/22 06:17, Maxime Ripard wrote: >> >> The MODULE_LICENSE macro is missing from the kunit helpers file, thus >> >> leading to a build error. >> >> >> >> Let's introduce it along with MODULE_AUTHOR. >> >> >> >> Fixes: 44a3928324e9 ("drm/tests: Add Kunit Helpers") >> >> Reported-by: Stephen Rothwell >> >> Signed-off-by: Maxime Ripard >> > >> > It would be nice to add the SPDX-License-Identifier tag in the source >> > file as well. Besides that, >> >> It's not just nice, it's basically mandatory to add license boilerplate. >> >> Checkpatch would've warned about this. And actually about a lot of stuff >> in the series. > > Right, sorry about that. I'll send additional patches to address the > issues already in. > >> (And our CI checkpatch did too, although we don't send the replies to >> the world, just intel-gfx [1].) > > I'm not sure how helpful it is though if the author is not a recipient > of the report It actually should be, but I have absolutely no idea why in this case it decided to do To: "Mateusz Kwiatkowski" instead of you. Baffled. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
[PATCH 2/2] drm/tests: helpers: Add SPDX header
The SPDX header is missing, let's add it and fix the corresponding checkpatch warning. Suggested-by: Maíra Canal Fixes: 44a3928324e9 ("drm/tests: Add Kunit Helpers") Signed-off-by: Maxime Ripard --- drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c index eea450de7de8..f1662091f250 100644 --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: GPL-2.0 + #include #include -- 2.38.1
Re: [PATCH v2 2/3] drm/msm/disp/dpu1: add helper to know if display is pluggable
On 16/11/2022 18:11, Abhinav Kumar wrote: On 11/16/2022 7:08 AM, Dmitry Baryshkov wrote: On 16/11/2022 17:30, Kalyan Thota wrote: Since DRM encoder type for few encoders can be similar (like eDP and DP) find out if the interface supports HPD from encoder bridge to differentiate between builtin and pluggable displays. Changes in v1: - add connector type in the disp_info (Dmitry) - add helper functions to know encoder type - update commit text reflecting the change Changes in v2: - avoid hardcode of connector type for DSI as it may not be true (Dmitry) - get the HPD information from encoder bridge Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++ 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 9c6817b..be93269 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "msm_drv.h" #include "dpu_kms.h" @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) +{ + struct drm_bridge *bridge; + int ops = 0; + + if (!encoder) + return false; + + /* Get last bridge in the chain to determine pluggable state */ + drm_for_each_bridge_in_chain(encoder, bridge) + if (!drm_bridge_get_next_bridge(bridge)) + ops = bridge->ops; + + return ops & DRM_BRIDGE_OP_HPD; No. This is not what you should be checking (hint: polled connectors also can be pluggable). Please check the type of the actual connector connected to this encoder. Even if we check the connector type as DSI or eDP that does not necessarily mean its built-in. We can even use DSI or eDP as a pluggable display. Well, I don't think so. eDP and DSI connectors are not pluggable per design. One can use them so, but they are not thought to be used this way. Unlike e.g. HDMI, DP, VGA, etc. I would say LVDS, eDP, DSI, DPI and SPI can be assumed to be constantly plugged. Compare this with Composite, SVIDEO, 9PinDIN, TV. They can be assumed to be external even if they do not have the HPD (or even polling). And these connectors usually don't have it. Thats why we thought of this check. -- With best wishes Dmitry
[PATCH 1/2] drm/tests: client: Remove extra blank lines
Some extra blank lines slipped through, remove them. Fixes: 8fc0380f6ba7 ("drm/client: Add some tests for drm_connector_pick_cmdline_mode()") Signed-off-by: Maxime Ripard --- drivers/gpu/drm/tests/drm_client_modeset_test.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c index 558c098b0384..362a5fbd82f5 100644 --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c @@ -53,7 +53,6 @@ static int drm_client_modeset_test_init(struct kunit *test) drm_connector_helper_add(&priv->connector, &drm_client_modeset_connector_helper_funcs); return 0; - } static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test) @@ -85,7 +84,6 @@ static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test) KUNIT_EXPECT_TRUE(test, drm_mode_equal(expected_mode, mode)); } - static struct kunit_case drm_test_pick_cmdline_tests[] = { KUNIT_CASE(drm_test_pick_cmdline_res_1920_1080_60), {} -- 2.38.1
Re: (subset) [PATCH 2/3] drm/tests: helpers: Add module infos
On Wed, 16 Nov 2022 10:17:11 +0100, Maxime Ripard wrote: > The MODULE_LICENSE macro is missing from the kunit helpers file, thus > leading to a build error. > > Let's introduce it along with MODULE_AUTHOR. > > Applied to local tree (tmp). Thanks! Maxime
Re: (subset) [PATCH 1/3] drm/tests: Include helpers header
On Wed, 16 Nov 2022 10:17:10 +0100, Maxime Ripard wrote: > The kunit helpers code weren't including its header, leading to a > warning that no previous prototype had been defined for public > functions. > > Include the matching header to fix the warning. > > > [...] Applied to local tree (tmp). Thanks! Maxime
Re: [PATCH 2/3] drm/tests: helpers: Add module infos
On Wed, Nov 16, 2022 at 01:32:51PM +0200, Jani Nikula wrote: > On Wed, 16 Nov 2022, Maíra Canal wrote: > > Hi Maxime, > > > > On 11/16/22 06:17, Maxime Ripard wrote: > >> The MODULE_LICENSE macro is missing from the kunit helpers file, thus > >> leading to a build error. > >> > >> Let's introduce it along with MODULE_AUTHOR. > >> > >> Fixes: 44a3928324e9 ("drm/tests: Add Kunit Helpers") > >> Reported-by: Stephen Rothwell > >> Signed-off-by: Maxime Ripard > > > > It would be nice to add the SPDX-License-Identifier tag in the source > > file as well. Besides that, > > It's not just nice, it's basically mandatory to add license boilerplate. > > Checkpatch would've warned about this. And actually about a lot of stuff > in the series. Right, sorry about that. I'll send additional patches to address the issues already in. > (And our CI checkpatch did too, although we don't send the replies to > the world, just intel-gfx [1].) I'm not sure how helpful it is though if the author is not a recipient of the report Maxime signature.asc Description: PGP signature
Re: [PATCH v2 2/3] drm/msm/disp/dpu1: add helper to know if display is pluggable
On 11/16/2022 7:08 AM, Dmitry Baryshkov wrote: On 16/11/2022 17:30, Kalyan Thota wrote: Since DRM encoder type for few encoders can be similar (like eDP and DP) find out if the interface supports HPD from encoder bridge to differentiate between builtin and pluggable displays. Changes in v1: - add connector type in the disp_info (Dmitry) - add helper functions to know encoder type - update commit text reflecting the change Changes in v2: - avoid hardcode of connector type for DSI as it may not be true (Dmitry) - get the HPD information from encoder bridge Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++ 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 9c6817b..be93269 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "msm_drv.h" #include "dpu_kms.h" @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) +{ + struct drm_bridge *bridge; + int ops = 0; + + if (!encoder) + return false; + + /* Get last bridge in the chain to determine pluggable state */ + drm_for_each_bridge_in_chain(encoder, bridge) + if (!drm_bridge_get_next_bridge(bridge)) + ops = bridge->ops; + + return ops & DRM_BRIDGE_OP_HPD; No. This is not what you should be checking (hint: polled connectors also can be pluggable). Please check the type of the actual connector connected to this encoder. Even if we check the connector type as DSI or eDP that does not necessarily mean its built-in. We can even use DSI or eDP as a pluggable display. Thats why we thought of this check. +} bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc) { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index 9e7236e..691ab57 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -224,4 +224,10 @@ void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc, */ bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc); +/** + * dpu_encoder_is_pluggable - find if the encoder is of type pluggable + * @drm_enc: Pointer to previously created drm encoder structure + */ +bool dpu_encoder_is_pluggable(struct drm_encoder *drm_enc); + #endif /* __DPU_ENCODER_H__ */
Re: [PATCH v2 2/3] drm/msm/disp/dpu1: add helper to know if display is pluggable
On 16/11/2022 17:30, Kalyan Thota wrote: Since DRM encoder type for few encoders can be similar (like eDP and DP) find out if the interface supports HPD from encoder bridge to differentiate between builtin and pluggable displays. Changes in v1: - add connector type in the disp_info (Dmitry) - add helper functions to know encoder type - update commit text reflecting the change Changes in v2: - avoid hardcode of connector type for DSI as it may not be true (Dmitry) - get the HPD information from encoder bridge Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++ 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 9c6817b..be93269 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "msm_drv.h" #include "dpu_kms.h" @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) +{ + struct drm_bridge *bridge; + int ops = 0; + + if (!encoder) + return false; + + /* Get last bridge in the chain to determine pluggable state */ + drm_for_each_bridge_in_chain(encoder, bridge) + if (!drm_bridge_get_next_bridge(bridge)) + ops = bridge->ops; + + return ops & DRM_BRIDGE_OP_HPD; No. This is not what you should be checking (hint: polled connectors also can be pluggable). Please check the type of the actual connector connected to this encoder. +} bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc) { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index 9e7236e..691ab57 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -224,4 +224,10 @@ void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc, */ bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc); +/** + * dpu_encoder_is_pluggable - find if the encoder is of type pluggable + * @drm_enc:Pointer to previously created drm encoder structure + */ +bool dpu_encoder_is_pluggable(struct drm_encoder *drm_enc); + #endif /* __DPU_ENCODER_H__ */ -- With best wishes Dmitry
Re: [v1] drm/msm/disp/dpu1: add color management support for the crtc
On 16/11/2022 17:29, Kalyan Thota wrote: -Original Message- From: Dmitry Baryshkov Sent: Saturday, November 12, 2022 4:13 AM To: Kalyan Thota (QUIC) ; dri- de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org; freedr...@lists.freedesktop.org; devicet...@vger.kernel.org Cc: linux-ker...@vger.kernel.org; robdcl...@chromium.org; diand...@chromium.org; swb...@chromium.org; Vinod Polimera (QUIC) ; Abhinav Kumar (QUIC) Subject: Re: [v1] drm/msm/disp/dpu1: add color management support for the crtc WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. On 11/11/2022 16:57, Kalyan Thota wrote: Add color management support for the crtc provided there are enough dspps that can be allocated from the catalogue. Changes in v1: - cache color enabled state in the dpu crtc obj (Dmitry) - simplify dspp allocation while creating crtc (Dmitry) - register for color when crtc is created (Dmitry) Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 5 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 53 - 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 4170fbe..ca4c3b3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1571,7 +1571,7 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = { /* initialize crtc */ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, - struct drm_plane *cursor) + struct drm_plane *cursor, unsigned long + features) { struct drm_crtc *crtc = NULL; struct dpu_crtc *dpu_crtc = NULL; @@ -1583,6 +1583,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, crtc = &dpu_crtc->base; crtc->dev = dev; + dpu_crtc->color_enabled = features & BIT(DPU_DSPP_PCC); spin_lock_init(&dpu_crtc->spin_lock); atomic_set(&dpu_crtc->frame_pending, 0); @@ -1604,7 +1605,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs); - drm_crtc_enable_color_mgmt(crtc, 0, true, 0); + drm_crtc_enable_color_mgmt(crtc, 0, dpu_crtc->color_enabled, 0); /* save user friendly CRTC name for later */ snprintf(dpu_crtc->name, DPU_CRTC_NAME_SIZE, "crtc%u", crtc->base.id); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index 539b68b..342f9ae 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -136,6 +136,7 @@ struct dpu_crtc_frame_event { * @enabled : whether the DPU CRTC is currently enabled. updated in the * commit-thread, not state-swap time which is earlier, so * safe to make decisions on during VBLANK on/off work + * @color_enabled : whether crtc supports color management * @feature_list : list of color processing features supported on a crtc * @active_list : list of color processing features are active * @dirty_list: list of color processing features are dirty @@ -164,7 +165,7 @@ struct dpu_crtc { u64 play_count; ktime_t vblank_cb_time; bool enabled; - + bool color_enabled; struct list_head feature_list; struct list_head active_list; struct list_head dirty_list; @@ -269,10 +270,11 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc); * @dev: dpu device * @plane: base plane * @cursor: cursor plane + * @features: color features * @Return: new crtc object or error */ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, -struct drm_plane *cursor); +struct drm_plane *cursor, unsigned long + features); /** * dpu_crtc_register_custom_event - api for enabling/disabling crtc event diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index c9058aa..d72edb8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -579,6 +579,7 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc) static struct msm_display_topology dpu_encoder_get_topology( struct dpu_encoder_virt *dpu_enc, struct dpu_kms *dpu_kms, + struct dpu_crtc *dpu_crtc, struct drm_display_mode *mode) { struct msm_display_topology topology = {0}; @@ -607,7 +608,7 @@ static struct msm_display_topology dpu_encoder_get_topology(
Re: [PATCH v2 01/12] dt-bindings: display: msm: Add qcom,sm8350-dpu binding
On Tue, 15 Nov 2022 14:30:54 +0100, Robert Foss wrote: > Mobile Display Subsystem (MDSS) encapsulates sub-blocks > like DPU display controller, DSI etc. Add YAML schema for DPU device > tree bindings > > Signed-off-by: Robert Foss > --- > .../bindings/display/msm/qcom,sm8350-dpu.yaml | 120 ++ > 1 file changed, 120 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sm8350-dpu.yaml > Reviewed-by: Rob Herring
Re: [PATCH v2 2/3] drm/etnaviv: allocate unique ID per drm_file
On 16/11/2022 13:18, Philipp Zabel wrote: On Fri, Sep 16, 2022 at 05:12:04PM +0200, Lucas Stach wrote: Allows to easily track if several fd are pointing to the same execution context due to being dup'ed. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++ drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 + 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index 1d2b4fb4bcf8..b69edb40ae2a 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -49,6 +49,7 @@ static void load_gpu(struct drm_device *dev) static int etnaviv_open(struct drm_device *dev, struct drm_file *file) { struct etnaviv_drm_private *priv = dev->dev_private; + static atomic_t ident = ATOMIC_INIT(0); struct etnaviv_file_private *ctx; int ret, i; @@ -56,6 +57,8 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file) if (!ctx) return -ENOMEM; + ctx->id = atomic_inc_return(&ident); I suppose we can ignore that this could theoretically wrap around. Depends on your usecases - if you can envisage a long running client, say the compositor, while other clients come and go then eventually these will not be unique and will break the fdinfo spec. Hence I used a cyclic xarray in i915 (aka idr). I would recommend you just do that and remove future debug sessions around the area of "why does gputop show nonsense all of a sudden". Regards, Tvrtko Reviewed-by: Philipp Zabel regards Philipp
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Never return 0 if request wait succeeds
On 16.11.2022 12:25, Janusz Krzysztofik wrote: According to the docs of i915_request_wait_timeout(), its return value "may be zero if the request is unfinished after the timeout expires." However, 0 is also returned when the request is found finished right after the timeout has expired. Since the docs also state: "If the timeout is 0, it will return 1 if the fence is signaled.", return 1 also when the fence is found signaled after non-zero timeout has expired. As I understand the patch "drm/i915: Fix i915_request fence wait semantics", and the docs "timeout is 0" means the initial value of timeout argument and this is handled already on the beginning of the function. In case initial timeout is greater than zero and then it expires, function should return 0 regardless of fence state. This is what I have understood from reading docs and implementation of dma_fence_default_wait [1], which should be the best source of info about "dma_fence wait semantic". As I said already, mixing remaining time and bool in return value of dma_fence_wait* functions is very confusing, but changing it would require major rework of the framework. [1]: https://elixir.bootlin.com/linux/latest/source/drivers/dma-buf/dma-fence.c#L753 Regards Andrzej Fixes: 7e2e69ed4678 ("drm/i915: Fix i915_request fence wait semantics") Signed-off-by: Janusz Krzysztofik Cc: sta...@vger.kernel.org # v5.17 --- drivers/gpu/drm/i915/i915_request.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index f949a9495758a..406ddfafbed4d 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -2079,6 +2079,8 @@ long i915_request_wait_timeout(struct i915_request *rq, timeout = io_schedule_timeout(timeout); } + if (!timeout) /* expired but signaled, we shouldn't return 0 */ + timeout = 1; __set_current_state(TASK_RUNNING); if (READ_ONCE(wait.tsk))
[Bug 216624] The system freezes when it reaches the screen to ask password for LUKS
https://bugzilla.kernel.org/show_bug.cgi?id=216624 --- Comment #13 from The Linux kernel's regression tracker (Thorsten Leemhuis) (regressi...@leemhuis.info) --- FWIW, the nouveau developers might not see this report here, they want bugs filed here: https://gitlab.freedesktop.org/drm/nouveau/-/issues I'd suggest you file it there and make it obvious that's a regression between 5.19.15 and 5.19.16 (it isn isn't it?) that continues to exist with 6.0 A bisection would be ideal to find the cause, but with a bit of luck the nouveau developers have a idea what might be wrong. -- 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 v2 1/3] drm/msm/disp/dpu1: pin 1 crtc to 1 encoder
Pin each crtc with one encoder. This arrangement will disallow crtc switching between encoders and also will facilitate to advertise certain features on crtc based on encoder type. Changes in v1: - use drm_for_each_encoder macro while iterating through encoder list (Dmitry) Changes in v2: - make sure no encoder miss to have a crtc (Dmitry) - revisit various factors in deciding the crtc count such as num_mixers, num_sspp (Dmitry) Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 7a5fabc..4784db8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -763,7 +763,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms) drm_for_each_encoder(encoder, dev) num_encoders++; - max_crtc_count = min(catalog->mixer_count, num_encoders); + max_crtc_count = num_encoders; /* Create the planes, keeping track of one primary/cursor per crtc */ for (i = 0; i < catalog->sspp_count; i++) { @@ -795,22 +795,25 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms) primary_planes[primary_planes_idx++] = plane; } - max_crtc_count = min(max_crtc_count, primary_planes_idx); + /* +* All the platforms should have at least 1 primary plane for an +* encoder. The below warn should help in setting up the catalog +*/ + WARN_ON(num_encoders > primary_planes_idx); /* Create one CRTC per encoder */ - for (i = 0; i < max_crtc_count; i++) { + i = 0; + drm_for_each_encoder(encoder, dev) { crtc = dpu_crtc_init(dev, primary_planes[i], cursor_planes[i]); if (IS_ERR(crtc)) { ret = PTR_ERR(crtc); return ret; } priv->crtcs[priv->num_crtcs++] = crtc; + encoder->possible_crtcs = 1 << drm_crtc_index(crtc); + i++; } - /* All CRTCs are compatible with all encoders */ - drm_for_each_encoder(encoder, dev) - encoder->possible_crtcs = (1 << priv->num_crtcs) - 1; - return 0; } -- 2.7.4
[PATCH v2 3/3] drm/msm/disp/dpu1: add color management support for the crtc
Add color management support for the crtc provided there are enough dspps that can be allocated from the catalog. Changes in v1: - cache color enabled state in the dpu crtc obj (Dmitry) - simplify dspp allocation while creating crtc (Dmitry) - register for color when crtc is created (Dmitry) Changes in v2: - avoid primary encoders in the documentation (Dmitry) Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 5 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 58 - 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 4170fbe..ca4c3b3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1571,7 +1571,7 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = { /* initialize crtc */ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, - struct drm_plane *cursor) + struct drm_plane *cursor, unsigned long features) { struct drm_crtc *crtc = NULL; struct dpu_crtc *dpu_crtc = NULL; @@ -1583,6 +1583,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, crtc = &dpu_crtc->base; crtc->dev = dev; + dpu_crtc->color_enabled = features & BIT(DPU_DSPP_PCC); spin_lock_init(&dpu_crtc->spin_lock); atomic_set(&dpu_crtc->frame_pending, 0); @@ -1604,7 +1605,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs); - drm_crtc_enable_color_mgmt(crtc, 0, true, 0); + drm_crtc_enable_color_mgmt(crtc, 0, dpu_crtc->color_enabled, 0); /* save user friendly CRTC name for later */ snprintf(dpu_crtc->name, DPU_CRTC_NAME_SIZE, "crtc%u", crtc->base.id); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index 539b68b..342f9ae 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -136,6 +136,7 @@ struct dpu_crtc_frame_event { * @enabled : whether the DPU CRTC is currently enabled. updated in the * commit-thread, not state-swap time which is earlier, so * safe to make decisions on during VBLANK on/off work + * @color_enabled : whether crtc supports color management * @feature_list : list of color processing features supported on a crtc * @active_list : list of color processing features are active * @dirty_list: list of color processing features are dirty @@ -164,7 +165,7 @@ struct dpu_crtc { u64 play_count; ktime_t vblank_cb_time; bool enabled; - + bool color_enabled; struct list_head feature_list; struct list_head active_list; struct list_head dirty_list; @@ -269,10 +270,11 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc); * @dev: dpu device * @plane: base plane * @cursor: cursor plane + * @features: color features * @Return: new crtc object or error */ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, - struct drm_plane *cursor); + struct drm_plane *cursor, unsigned long features); /** * dpu_crtc_register_custom_event - api for enabling/disabling crtc event diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index be93269..7f1cfc5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -561,6 +561,7 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc) static struct msm_display_topology dpu_encoder_get_topology( struct dpu_encoder_virt *dpu_enc, struct dpu_kms *dpu_kms, + struct dpu_crtc *dpu_crtc, struct drm_display_mode *mode) { struct msm_display_topology topology = {0}; @@ -589,7 +590,7 @@ static struct msm_display_topology dpu_encoder_get_topology( else topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1; - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) { + if (dpu_crtc->color_enabled) { if (dpu_kms->catalog->dspp && (dpu_kms->catalog->dspp_count >= topology.num_lm)) topology.num_dspp = topology.num_lm; @@ -624,6 +625,7 @@ static int dpu_encoder_virt_atomic_check( struct drm_display_mode *adj_mode; struct msm_display_topology topology; struct dpu_global_state *global_state; + struct dpu_crtc *dpu_crtc; int i = 0;
[PATCH v2 2/3] drm/msm/disp/dpu1: add helper to know if display is pluggable
Since DRM encoder type for few encoders can be similar (like eDP and DP) find out if the interface supports HPD from encoder bridge to differentiate between builtin and pluggable displays. Changes in v1: - add connector type in the disp_info (Dmitry) - add helper functions to know encoder type - update commit text reflecting the change Changes in v2: - avoid hardcode of connector type for DSI as it may not be true (Dmitry) - get the HPD information from encoder bridge Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++ 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 9c6817b..be93269 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "msm_drv.h" #include "dpu_kms.h" @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) +{ + struct drm_bridge *bridge; + int ops = 0; + + if (!encoder) + return false; + + /* Get last bridge in the chain to determine pluggable state */ + drm_for_each_bridge_in_chain(encoder, bridge) + if (!drm_bridge_get_next_bridge(bridge)) + ops = bridge->ops; + + return ops & DRM_BRIDGE_OP_HPD; +} bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc) { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index 9e7236e..691ab57 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -224,4 +224,10 @@ void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc, */ bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc); +/** + * dpu_encoder_is_pluggable - find if the encoder is of type pluggable + * @drm_enc:Pointer to previously created drm encoder structure + */ +bool dpu_encoder_is_pluggable(struct drm_encoder *drm_enc); + #endif /* __DPU_ENCODER_H__ */ -- 2.7.4
[PATCH v2 0/3] add color management support for the crtc
Add color management support for the crtc provided there are enough dspps that can be allocated from the catalog Kalyan Thota (3): drm/msm/disp/dpu1: pin 1 crtc to 1 encoder drm/msm/disp/dpu1: add helper to know if display is pluggable drm/msm/disp/dpu1: add color management support for the crtc drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 5 +- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 23 - drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 75 ++--- 5 files changed, 101 insertions(+), 14 deletions(-) -- 2.7.4
RE: [v1] drm/msm/disp/dpu1: add color management support for the crtc
>-Original Message- >From: Dmitry Baryshkov >Sent: Saturday, November 12, 2022 4:13 AM >To: Kalyan Thota (QUIC) ; dri- >de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org; >freedr...@lists.freedesktop.org; devicet...@vger.kernel.org >Cc: linux-ker...@vger.kernel.org; robdcl...@chromium.org; >diand...@chromium.org; swb...@chromium.org; Vinod Polimera (QUIC) >; Abhinav Kumar (QUIC) > >Subject: Re: [v1] drm/msm/disp/dpu1: add color management support for the >crtc > >WARNING: This email originated from outside of Qualcomm. Please be wary of >any links or attachments, and do not enable macros. > >On 11/11/2022 16:57, Kalyan Thota wrote: >> Add color management support for the crtc provided there are enough >> dspps that can be allocated from the catalogue. >> >> Changes in v1: >> - cache color enabled state in the dpu crtc obj (Dmitry) >> - simplify dspp allocation while creating crtc (Dmitry) >> - register for color when crtc is created (Dmitry) >> >> Signed-off-by: Kalyan Thota >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 5 +-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 ++-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 53 >- >> 4 files changed, 64 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> index 4170fbe..ca4c3b3 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> @@ -1571,7 +1571,7 @@ static const struct drm_crtc_helper_funcs >> dpu_crtc_helper_funcs = { >> >> /* initialize crtc */ >> struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane >*plane, >> - struct drm_plane *cursor) >> + struct drm_plane *cursor, unsigned long >> + features) >> { >> struct drm_crtc *crtc = NULL; >> struct dpu_crtc *dpu_crtc = NULL; @@ -1583,6 +1583,7 @@ struct >> drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane >> *plane, >> >> crtc = &dpu_crtc->base; >> crtc->dev = dev; >> + dpu_crtc->color_enabled = features & BIT(DPU_DSPP_PCC); >> >> spin_lock_init(&dpu_crtc->spin_lock); >> atomic_set(&dpu_crtc->frame_pending, 0); @@ -1604,7 +1605,7 @@ >> struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct >> drm_plane *plane, >> >> drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs); >> >> - drm_crtc_enable_color_mgmt(crtc, 0, true, 0); >> + drm_crtc_enable_color_mgmt(crtc, 0, dpu_crtc->color_enabled, 0); >> >> /* save user friendly CRTC name for later */ >> snprintf(dpu_crtc->name, DPU_CRTC_NAME_SIZE, "crtc%u", >> crtc->base.id); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h >> index 539b68b..342f9ae 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h >> @@ -136,6 +136,7 @@ struct dpu_crtc_frame_event { >>* @enabled : whether the DPU CRTC is currently enabled. updated in >> the >>* commit-thread, not state-swap time which is earlier, so >>* safe to make decisions on during VBLANK on/off work >> + * @color_enabled : whether crtc supports color management >>* @feature_list : list of color processing features supported on a crtc >>* @active_list : list of color processing features are active >>* @dirty_list: list of color processing features are dirty >> @@ -164,7 +165,7 @@ struct dpu_crtc { >> u64 play_count; >> ktime_t vblank_cb_time; >> bool enabled; >> - >> + bool color_enabled; >> struct list_head feature_list; >> struct list_head active_list; >> struct list_head dirty_list; >> @@ -269,10 +270,11 @@ void dpu_crtc_complete_commit(struct drm_crtc >*crtc); >>* @dev: dpu device >>* @plane: base plane >>* @cursor: cursor plane >> + * @features: color features >>* @Return: new crtc object or error >>*/ >> struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane >*plane, >> -struct drm_plane *cursor); >> +struct drm_plane *cursor, unsigned long >> + features); >> >> /** >>* dpu_crtc_register_custom_event - api for enabling/disabling crtc >> event diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> index c9058aa..d72edb8 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -579,6 +579,7 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder >*drm_enc) >> static struct msm_display_topology dpu_encoder_get_topology( >> struct dpu_encoder_virt *dpu_enc, >> struct dpu_kms *dpu_kms, >> + struct dpu_cr
RE: [v1] drm/msm/disp/dpu1: pin 1 crtc to 1 encoder
>-Original Message- >From: Dmitry Baryshkov >Sent: Saturday, November 12, 2022 3:51 AM >To: Kalyan Thota (QUIC) ; dri- >de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org; >freedr...@lists.freedesktop.org; devicet...@vger.kernel.org >Cc: linux-ker...@vger.kernel.org; robdcl...@chromium.org; >diand...@chromium.org; swb...@chromium.org; Vinod Polimera (QUIC) >; Abhinav Kumar (QUIC) > >Subject: Re: [v1] drm/msm/disp/dpu1: pin 1 crtc to 1 encoder > >WARNING: This email originated from outside of Qualcomm. Please be wary of >any links or attachments, and do not enable macros. > >On 11/11/2022 16:56, Kalyan Thota wrote: >> Pin each crtc with one encoder. This arrangement will disallow crtc >> switching between encoders and also will facilitate to advertise >> certain features on crtc based on encoder type. >> >> Changes in v1: >> - use drm_for_each_encoder macro while iterating through >>encoder list (Dmitry) > >BTW: if these patches form a series, please send them so. > >> >> Signed-off-by: Kalyan Thota >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 +++-- >> 1 file changed, 11 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> index 7a5fabc..0d94eec0d 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> @@ -798,19 +798,20 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms >*dpu_kms) >> max_crtc_count = min(max_crtc_count, primary_planes_idx); >> >> /* Create one CRTC per encoder */ >> - for (i = 0; i < max_crtc_count; i++) { >> - crtc = dpu_crtc_init(dev, primary_planes[i], cursor_planes[i]); >> - if (IS_ERR(crtc)) { >> - ret = PTR_ERR(crtc); >> - return ret; >> + i = 0; >> + drm_for_each_encoder(encoder, dev) { >> + if (i < max_crtc_count) { > >What if max_crtc_counter < num_encoders? I think we should disallow such >configuration. Can it happen on any of relevant platforms? > Yes, we don't need the below checks accounting for crtc as all the platforms in the catalog has sufficient resources. max_crtc_count = min(catalog->mixer_count, num_encoders); This check is not needed, as mixer resource allocation will happen at later time, even though you have less mixers than encoders, one can turn off the crtc and get the mixers back to free pool. max_crtc_count = min(max_crtc_count, primary_planes_idx); A safety check, but mostly, all the platforms are ensured that at least 1 primary plane is available per interface. will add WARN ON additionally >> + crtc = dpu_crtc_init(dev, primary_planes[i], >> cursor_planes[i]); >> + if (IS_ERR(crtc)) { >> + ret = PTR_ERR(crtc); >> + return ret; >> + } >> + priv->crtcs[priv->num_crtcs++] = crtc; >> + encoder->possible_crtcs = 1 << >> + drm_crtc_index(crtc); >> } >> - priv->crtcs[priv->num_crtcs++] = crtc; >> + i++; >> } >> >> - /* All CRTCs are compatible with all encoders */ >> - drm_for_each_encoder(encoder, dev) >> - encoder->possible_crtcs = (1 << priv->num_crtcs) - 1; >> - >> return 0; >> } >> > >-- >With best wishes >Dmitry
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Never return 0 on timeout when retiring requests
On 16.11.2022 12:25, Janusz Krzysztofik wrote: Users of intel_gt_retire_requests_timeout() expect 0 return value on success. However, we have no protection from passing back 0 potentially returned by dma_fence_wait_timeout() on timeout. Replace 0 with -ETIME before using timeout as return value. Fixes: f33a8a51602c ("drm/i915: Merge wait_for_timelines with retire_request") Signed-off-by: Janusz Krzysztofik Cc: sta...@vger.kernel.org # v5.5+ --- drivers/gpu/drm/i915/gt/intel_gt_requests.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c b/drivers/gpu/drm/i915/gt/intel_gt_requests.c index ccaf2fd80625b..ac6b2b1861397 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c @@ -213,6 +213,9 @@ out_active: spin_lock(&timelines->lock); list_for_each_entry_safe(tl, tn, &free, link) __intel_timeline_free(&tl->kref); + if (!timeout) + timeout = -ETIME; + Reviewed-by: Andrzej Hajda Regards Andrzej if (flush_submission(gt, timeout)) /* Wait, there's more! */ active_count++;
Re: [PATCH i-g-t 8/8] gputop: Basic vendor agnostic GPU top tool
On Fr, 2022-11-11 at 15:58 +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Rudimentary vendor agnostic example of how lib_igt_drm_clients can be used > to display a sorted by card and usage list of processes using GPUs. > > Borrows a bit of code from intel_gpu_top but for now omits the fancy > features like interactive functionality, card selection, client > aggregation, sort modes, JSON output and pretty engine names. Also no > support for global GPU or system metrics. > > On the other hand it shows clients from all DRM cards which > intel_gpu_top does not do. > > Signed-off-by: Tvrtko Ursulin > Cc: Rob Clark > Cc: Christian König > Acked-by: Christian König Tested-by: Philipp Zabel on etnaviv with [1]. [1] https://lore.kernel.org/dri-devel/20220916151205.165687-3-l.st...@pengutronix.de/ regards Philipp
Re: [PATCH 19/26] drm: shmobile: Remove #ifdef guards for PM related functions
Quoting Paul Cercueil (2022-11-07 17:52:49) > Use the DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr() macros to handle > the .suspend/.resume callbacks. > > These macros allow the suspend and resume functions to be automatically > dropped by the compiler when CONFIG_SUSPEND is disabled, without having > to use #ifdef guards. > > This has the advantage of always compiling these functions in, > independently of any Kconfig option. Thanks to that, bugs and other > regressions are subsequently easier to catch. > > Signed-off-by: Paul Cercueil Reviewed-by: Kieran Bingham > --- > Cc: Laurent Pinchart > Cc: Kieran Bingham > Cc: linux-renesas-...@vger.kernel.org > --- > drivers/gpu/drm/shmobile/shmob_drm_drv.c | 9 +++-- > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c > b/drivers/gpu/drm/shmobile/shmob_drm_drv.c > index 3d511fa38913..337040fa6438 100644 > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c > @@ -143,7 +143,6 @@ static const struct drm_driver shmob_drm_driver = { > * Power management > */ > > -#ifdef CONFIG_PM_SLEEP > static int shmob_drm_pm_suspend(struct device *dev) > { > struct shmob_drm_device *sdev = dev_get_drvdata(dev); > @@ -165,11 +164,9 @@ static int shmob_drm_pm_resume(struct device *dev) > drm_kms_helper_poll_enable(sdev->ddev); > return 0; > } > -#endif > > -static const struct dev_pm_ops shmob_drm_pm_ops = { > - SET_SYSTEM_SLEEP_PM_OPS(shmob_drm_pm_suspend, shmob_drm_pm_resume) > -}; > +static DEFINE_SIMPLE_DEV_PM_OPS(shmob_drm_pm_ops, > + shmob_drm_pm_suspend, shmob_drm_pm_resume); > > /* > - > * Platform driver > @@ -292,7 +289,7 @@ static struct platform_driver shmob_drm_platform_driver = > { > .remove = shmob_drm_remove, > .driver = { > .name = "shmob-drm", > - .pm = &shmob_drm_pm_ops, > + .pm = pm_sleep_ptr(&shmob_drm_pm_ops), > }, > }; > > -- > 2.35.1 >
Re: [PATCH 17/26] drm: rcar-du: Remove #ifdef guards for PM related functions
Quoting Paul Cercueil (2022-11-07 17:52:47) > Use the DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr() macros to handle > the .suspend/.resume callbacks. > > These macros allow the suspend and resume functions to be automatically > dropped by the compiler when CONFIG_SUSPEND is disabled, without having > to use #ifdef guards. > > This has the advantage of always compiling these functions in, > independently of any Kconfig option. Thanks to that, bugs and other > regressions are subsequently easier to catch. > > Signed-off-by: Paul Cercueil Seems reasonable to me. Reviewed-by: Kieran Bingham > --- > Cc: Laurent Pinchart > Cc: Kieran Bingham > Cc: linux-renesas-...@vger.kernel.org > --- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 9 +++-- > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index a2776f1d6f2c..0a89094461cc 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -599,7 +599,6 @@ static const struct drm_driver rcar_du_driver = { > * Power management > */ > > -#ifdef CONFIG_PM_SLEEP > static int rcar_du_pm_suspend(struct device *dev) > { > struct rcar_du_device *rcdu = dev_get_drvdata(dev); > @@ -613,11 +612,9 @@ static int rcar_du_pm_resume(struct device *dev) > > return drm_mode_config_helper_resume(&rcdu->ddev); > } > -#endif > > -static const struct dev_pm_ops rcar_du_pm_ops = { > - SET_SYSTEM_SLEEP_PM_OPS(rcar_du_pm_suspend, rcar_du_pm_resume) > -}; > +static DEFINE_SIMPLE_DEV_PM_OPS(rcar_du_pm_ops, > + rcar_du_pm_suspend, rcar_du_pm_resume); > > /* > - > * Platform driver > @@ -712,7 +709,7 @@ static struct platform_driver rcar_du_platform_driver = { > .shutdown = rcar_du_shutdown, > .driver = { > .name = "rcar-du", > - .pm = &rcar_du_pm_ops, > + .pm = pm_sleep_ptr(&rcar_du_pm_ops), > .of_match_table = rcar_du_of_table, > }, > }; > -- > 2.35.1 >
Re: [PATCH v2 3/3] drm/etnaviv: export client GPU usage statistics via fdinfo
On Fri, Sep 16, 2022 at 05:12:05PM +0200, Lucas Stach wrote: > This exposes a accumulated GPU active time per client via the > fdinfo infrastructure. > > Signed-off-by: Lucas Stach > --- > v2: > - fix code style > - switch to raw seq_printf > - leave some breadcrumbs about the output format > --- > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 40 ++- > 1 file changed, 39 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > index b69edb40ae2a..c08748472f74 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > @@ -22,6 +22,7 @@ > #include "etnaviv_gem.h" > #include "etnaviv_mmu.h" > #include "etnaviv_perfmon.h" > +#include "common.xml.h" > > /* > * DRM operations: > @@ -471,7 +472,44 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = { > ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW), > }; > > -DEFINE_DRM_GEM_FOPS(fops); > +static void etnaviv_fop_show_fdinfo(struct seq_file *m, struct file *f) > +{ > + struct drm_file *file = f->private_data; > + struct drm_device *dev = file->minor->dev; > + struct etnaviv_drm_private *priv = dev->dev_private; > + struct etnaviv_file_private *ctx = file->driver_priv; > + > + /* > + * For a description of the text output format used here, see > + * Documentation/gpu/drm-usage-stats.rst. > + */ > + seq_printf(m, "drm-driver:\t%s\n", dev->driver->name); > + seq_printf(m, "drm-client-id:\t%u\n", ctx->id); > + > + for (int i = 0; i < ETNA_MAX_PIPES; i++) { > + struct etnaviv_gpu *gpu = priv->gpu[i]; > + char engine[8]; Maybe initialize this as well? See below. > + int cur = 0; > + > + if (!gpu) > + continue; > + > + if (gpu->identity.features & chipFeatures_PIPE_2D) > + cur = snprintf(engine, sizeof(engine), "2D"); > + if (gpu->identity.features & chipFeatures_PIPE_3D) > + cur = snprintf(engine + cur, sizeof(engine) - cur, > +"%s3D", cur ? "/" : ""); Does the NPU have either bit set? If not, this must not be forgotten to be changed when NPU support is added, to avoid uninitalized use of the engine variable. Reviewed-by: Philipp Zabel Tested-by: Philipp Zabel with gputop [1]. [1] https://lore.kernel.org/dri-devel/2022155844.3290531-1-tvrtko.ursu...@linux.intel.com/ regards Philipp
Re: [PATCH] drm/msm/dp: remove limitation of link rate at 5.4G to support HBR3
On 15/11/2022 21:43, Kuogee Hsieh wrote: On 11/9/2022 11:43 PM, Dmitry Baryshkov wrote: On 10/11/2022 02:47, Kuogee Hsieh wrote: On 11/2/2022 11:04 AM, Dmitry Baryshkov wrote: On 02/11/2022 20:28, Doug Anderson wrote: Hi, On Wed, Nov 2, 2022 at 10:23 AM Dmitry Baryshkov wrote: 1. Someone figures out how to model this with the bridge chain and then we only allow HBR3 if we detect we've got a TCPC that supports it. This seems like the cleanest / best but feels like a long pole. Not only have we been trying to get the TCPC-modeled-as-a-bridge stuff landed for a long time but even when we do it we still don't have a solution for how to communicate the number of lanes and other stuff between the TCPC and the DP controller so we have to enrich the bridge interface. I think we'd need some OOB interface. For example for DSI interfaces we have mipi_dsi_device struct to communicate such OOB data. Also take a note regarding data-lanes from my previous email. Right, we can somehow communicate the max link rate through the bridge chain to the DP controller in an OOB manner that would work. I'd note that our dp_panel has some notion of such OOB data. So do AUX drivers including the panel-edp. My suggestion would be to consider both of them while modelling the OOB data. 2. We add in a DT property to the display controller node that says the max link rate for use on this board. This feels like a hack, but maybe it's not too bad. Certainly it would be incredibly simple to implement. Actually... ...one could argue that even if we later model the TCPC as a bridge that this property would still be valid / useful! You could certainly imagine that the SoC supports HBR3 and the TCPC supports HBR3 but that the board routing between the SoC and the TCPC is bad and only supports HBR2. In this case the only way out is essentially a "board constraint" AKA a DT property in the DP controller. We have been discussing similar topics with Abhinav. Krzysztof suggested using link-frequencies property to provide max and min values. questions, 1)is Krzysztof suggested had been implemented? I can not parse this question, please excuse me. Yes, Krzysztof suggested this being implemented as a link property, see media/video-interfaces.txt. Moreover your implementation goes against both the existing definition (array with the list of frequencies) and Krzysztof's suggested extension (min and max). Listing just a single frequency goes against both these suggestions. In case of DP we have a fixed set of frequencies. Thus I'd suggest listing all supported frequencies instead. I think this proposal is kind of strange. According to DP spec, if a link support 5,4G, then it must support 1.6, 2.7 and 5.4. If it support 8.1G, then it must support 1.6 , 2.7 and 5.4. There is no link can only support 2.7 and 5.4G without supporting 1.6G. Let me quote the docs. link-frequencies: $ref: /schemas/types.yaml#/definitions/uint64-array description: Allowed data bus frequencies. For MIPI CSI-2, for instance, this is the actual frequency of the bus, not bits per clock per lane value. An array of 64-bit unsigned integers. Note. 'allowed data bus frequencies'. So by listing only the max frequency you'd break this description. -- With best wishes Dmitry
Re: [RESEND] drm/edid/firmware: stop using a throwaway platform device
On Wed, 16 Nov 2022, Thomas Zimmermann wrote: > Hi > > Am 14.11.22 um 12:17 schrieb Jani Nikula: >> We've used a temporary platform device for firmware EDID loading since >> it was introduced in commit da0df92b5731 ("drm: allow loading an EDID as >> firmware to override broken monitor"), but there's no explanation why. >> >> Using a temporary device does not play well with CONFIG_FW_CACHE=y, >> which caches firmware images (e.g. on suspend) so that drivers can >> request firmware when the system is not ready for it, and return the >> images from the cache (e.g. during resume). This works automatically for >> regular devices, but obviously not for a temporarily created device. >> >> Stop using the throwaway platform device, and use the drm device >> instead. >> >> Note that this may still be problematic for cases where the display was >> plugged in during suspend, and the firmware wasn't loaded and therefore >> not cached before suspend. >> >> References: >> https://lore.kernel.org/r/20220727074152.43059-1-matthieu.chare...@gmail.com >> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2061 >> Reported-by: Matthieu CHARETTE >> Tested-by: Matthieu CHARETTE >> Cc: Ville Syrjälä >> Signed-off-by: Jani Nikula > > Acked-by: Thomas Zimmermann > > I looked through request_firmware() but did not see any signs that it > somehow depends on a platform device. I assume that this might only > affect the device name in the error message. Thanks, pushed to drm-misc-next. Matthieu, thanks for you patience and the report as well! BR, Jani. > > Best regards > Thomas > >> >> --- >> >> Resend with a proper commit message; patch itself is unchanged. >> --- >> drivers/gpu/drm/drm_edid_load.c | 13 + >> 1 file changed, 1 insertion(+), 12 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid_load.c >> b/drivers/gpu/drm/drm_edid_load.c >> index ef4ab59d6935..5d9ef267ebb3 100644 >> --- a/drivers/gpu/drm/drm_edid_load.c >> +++ b/drivers/gpu/drm/drm_edid_load.c >> @@ -172,20 +172,9 @@ static const struct drm_edid *edid_load(struct >> drm_connector *connector, const c >> fwdata = generic_edid[builtin]; >> fwsize = sizeof(generic_edid[builtin]); >> } else { >> -struct platform_device *pdev; >> int err; >> >> -pdev = platform_device_register_simple(connector->name, -1, >> NULL, 0); >> -if (IS_ERR(pdev)) { >> -drm_err(connector->dev, >> -"[CONNECTOR:%d:%s] Failed to register EDID >> firmware platform device for connector \"%s\"\n", >> -connector->base.id, connector->name, >> -connector->name); >> -return ERR_CAST(pdev); >> -} >> - >> -err = request_firmware(&fw, name, &pdev->dev); >> -platform_device_unregister(pdev); >> +err = request_firmware(&fw, name, connector->dev->dev); >> if (err) { >> drm_err(connector->dev, >> "[CONNECTOR:%d:%s] Requesting EDID firmware >> \"%s\" failed (err=%d)\n", -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v2 2/3] drm/etnaviv: allocate unique ID per drm_file
On Fri, Sep 16, 2022 at 05:12:04PM +0200, Lucas Stach wrote: > Allows to easily track if several fd are pointing to the same > execution context due to being dup'ed. > > Signed-off-by: Lucas Stach > --- > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++ > drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > index 1d2b4fb4bcf8..b69edb40ae2a 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > @@ -49,6 +49,7 @@ static void load_gpu(struct drm_device *dev) > static int etnaviv_open(struct drm_device *dev, struct drm_file *file) > { > struct etnaviv_drm_private *priv = dev->dev_private; > + static atomic_t ident = ATOMIC_INIT(0); > struct etnaviv_file_private *ctx; > int ret, i; > > @@ -56,6 +57,8 @@ static int etnaviv_open(struct drm_device *dev, struct > drm_file *file) > if (!ctx) > return -ENOMEM; > > + ctx->id = atomic_inc_return(&ident); I suppose we can ignore that this could theoretically wrap around. Reviewed-by: Philipp Zabel regards Philipp
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix negative remaining time after retire requests
On 16.11.2022 12:25, Janusz Krzysztofik wrote: Commit b97060a99b01 ("drm/i915/guc: Update intel_gt_wait_for_idle to work with GuC") extended the API of intel_gt_retire_requests_timeout() with an extra argument 'remaining_timeout', intended for passing back unconsumed portion of requested timeout when 0 (success) is returned. However, when request retirement happens to succeed despite an error returned by dma_fence_wait_timeout(), the error code (a negative value) is passed back instead of remaining time. If a user then passes that negative value forward as requested timeout to another wait, an explicit WARN or BUG can be triggered. Instead of copying the value of timeout variable to *remaining_timeout before return, update the *remaining_timeout after each DMA fence wait. Set it to 0 on -ETIME, -EINTR or -ERESTARTSYS, and assume no time has been consumed on other errors returned from the wait. Fixes: b97060a99b01 ("drm/i915/guc: Update intel_gt_wait_for_idle to work with GuC") Signed-off-by: Janusz Krzysztofik Cc: sta...@vger.kernel.org # v5.15+ --- drivers/gpu/drm/i915/gt/intel_gt_requests.c | 23 ++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c b/drivers/gpu/drm/i915/gt/intel_gt_requests.c index edb881d756309..ccaf2fd80625b 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c @@ -138,6 +138,9 @@ long intel_gt_retire_requests_timeout(struct intel_gt *gt, long timeout, unsigned long active_count = 0; LIST_HEAD(free); + if (remaining_timeout) + *remaining_timeout = timeout; + flush_submission(gt, timeout); /* kick the ksoftirqd tasklets */ spin_lock(&timelines->lock); list_for_each_entry_safe(tl, tn, &timelines->active_list, link) { @@ -163,6 +166,23 @@ long intel_gt_retire_requests_timeout(struct intel_gt *gt, long timeout, timeout); dma_fence_put(fence); +if (remaining_timeout) { + /* +* If we get an error here but request +* retirement succeeds anyway +* (!active_count) and we return 0, the +* caller may want to spend remaining +* time on waiting for other events. +*/ + if (timeout == -ETIME || + timeout == -EINTR || + timeout == -ERESTARTSYS) + *remaining_timeout = 0; + else if (timeout >= 0) + *remaining_timeout = timeout; + /* else assume no time consumed */ Looks correct, but the crazy semantic of dma_fence_wait_timeout does not make it easy to understand. Reviewed-by: Andrzej Hajda Regards Andrzej + } + /* Retirement is best effort */ if (!mutex_trylock(&tl->mutex)) { active_count++; @@ -196,9 +216,6 @@ out_active: spin_lock(&timelines->lock); if (flush_submission(gt, timeout)) /* Wait, there's more! */ active_count++; - if (remaining_timeout) - *remaining_timeout = timeout; - return active_count ? timeout : 0; }
Re: [Intel-gfx] How is the progress for removing flush_scheduled_work() callers?
On Wed, Nov 16, 2022 at 12:08:27PM +0200, Jani Nikula wrote: > On Sun, 06 Nov 2022, Tetsuo Handa wrote: > > Like commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() using a > > macro") says, flush_scheduled_work() is dangerous and will be forbidden. > > We are on the way for removing all flush_scheduled_work() callers from > > the kernel, and there are only 4 callers remaining as of linux-20221104. > > > > drivers/gpu/drm/i915/display/intel_display.c:8997: > > flush_scheduled_work(); > > Thanks for the reminder, I've pinged folks to get someone working on > this. We do schedule quite a bunch of work, so it's not immediately > obvious (at least to me) what exactly needs flushing. Here's my earlier cursory analysis of the subject: https://lore.kernel.org/intel-gfx/yy3byxfrfafql...@intel.com/ > > https://gitlab.freedesktop.org/drm/intel/-/issues/7546 > > > drivers/gpu/drm/i915/gt/selftest_execlists.c:88: > > flush_scheduled_work(); > > Removed by commit 7d33fd02dd94 ("drm/i915/selftests: Remove > flush_scheduled_work() from live_execlists") in drm-next. > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Graphics Center -- Ville Syrjälä Intel
[PATCH RESEND v4 2/2] gpu: drm: meson: Use devm_regulator_*get_enable*()
Simplify using the devm_regulator_get_enable_optional(). Also drop the seemingly unused struct member 'hdmi_supply'. Signed-off-by: Matti Vaittinen --- I am doing a clean-up for my local git and encountered this one. Respinning as it seems this one fell through the cracks. --- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++ 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c index 5cd2b2ebbbd3..7642f740272b 100644 --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c @@ -140,7 +140,6 @@ struct meson_dw_hdmi { struct reset_control *hdmitx_apb; struct reset_control *hdmitx_ctrl; struct reset_control *hdmitx_phy; - struct regulator *hdmi_supply; u32 irq_stat; struct dw_hdmi *hdmi; struct drm_bridge *bridge; @@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi) } -static void meson_disable_regulator(void *data) -{ - regulator_disable(data); -} - static void meson_disable_clk(void *data) { clk_disable_unprepare(data); @@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, meson_dw_hdmi->data = match; dw_plat_data = &meson_dw_hdmi->dw_plat_data; - meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi"); - if (IS_ERR(meson_dw_hdmi->hdmi_supply)) { - if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER) - return -EPROBE_DEFER; - meson_dw_hdmi->hdmi_supply = NULL; - } else { - ret = regulator_enable(meson_dw_hdmi->hdmi_supply); - if (ret) - return ret; - ret = devm_add_action_or_reset(dev, meson_disable_regulator, - meson_dw_hdmi->hdmi_supply); - if (ret) - return ret; - } + ret = devm_regulator_get_enable_optional(dev, "hdmi"); + if (ret != -ENODEV) + return ret; meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev, "hdmitx_apb"); -- 2.38.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =] signature.asc Description: PGP signature
[PATCH RESEND v4 1/2] gpu: drm: sii902x: Use devm_regulator_bulk_get_enable()
Simplify using devm_regulator_bulk_get_enable() Signed-off-by: Matti Vaittinen Acked-by: Robert Foss --- I am doing a clean-up for my local git and encountered this one. Respinning as it seems this one fell through the cracks. --- drivers/gpu/drm/bridge/sii902x.c | 26 -- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 878fb7d3732b..f6e8b401069b 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -171,7 +171,6 @@ struct sii902x { struct drm_connector connector; struct gpio_desc *reset_gpio; struct i2c_mux_core *i2cmux; - struct regulator_bulk_data supplies[2]; bool sink_is_hdmi; /* * Mutex protects audio and video functions from interfering @@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client, struct device *dev = &client->dev; struct device_node *endpoint; struct sii902x *sii902x; + static const char * const supplies[] = {"iovcc", "cvcc12"}; int ret; ret = i2c_check_functionality(client->adapter, @@ -1122,27 +1122,11 @@ static int sii902x_probe(struct i2c_client *client, mutex_init(&sii902x->mutex); - sii902x->supplies[0].supply = "iovcc"; - sii902x->supplies[1].supply = "cvcc12"; - ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies), - sii902x->supplies); + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies), supplies); if (ret < 0) - return ret; - - ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies), - sii902x->supplies); - if (ret < 0) { - dev_err_probe(dev, ret, "Failed to enable supplies"); - return ret; - } + return dev_err_probe(dev, ret, "Failed to enable supplies"); - ret = sii902x_init(sii902x); - if (ret < 0) { - regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies), - sii902x->supplies); - } - - return ret; + return sii902x_init(sii902x); } static void sii902x_remove(struct i2c_client *client) @@ -1152,8 +1136,6 @@ static void sii902x_remove(struct i2c_client *client) i2c_mux_del_adapters(sii902x->i2cmux); drm_bridge_remove(&sii902x->bridge); - regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies), - sii902x->supplies); } static const struct of_device_id sii902x_dt_ids[] = { -- 2.38.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =] signature.asc Description: PGP signature
[PATCH RESEND v4 0/2] Use devm helpers for regulator get and enable
Simplify couple of drivers by using the new devm_regulator_*get_enable*() Found these patches when doing some clean-up for my local git. Seems like these two fell through the cracks while other were merged. So, this is a respin of subset of original series v4. These patches were previously part of the series: https://lore.kernel.org/lkml/cover.1660934107.git.mazziesacco...@gmail.com/ "Devm helpers for regulator get and enable". I did keep the patch series versioning even though I changed the series name (subject of this mail) to "Use devm helpers for regulator get and enable". Name was changed because the devm helpers are already in 6.1-rc1. Also, most of the patches in the series are already merged to subsystem trees so this series now contains only the patches that have not yet been merged. I hope they can be now directly taken sirectly into respective subsystem trees as the dependencies should be in v6.1-rc1. Please note that these changes are only compile-tested as I don't have the HW to do proper testing. Thus, reviewing / testing is highly appreciated. Revision history: v3 => v4: - Drop applied patches - rewrite cover-letter - rebase on v6.1-rc1 - split meson and sii902x into own patches as requested. - slightly modify dev_err_probe() return in sii902x. Matti Vaittinen (2): gpu: drm: sii902x: Use devm_regulator_bulk_get_enable() gpu: drm: meson: Use devm_regulator_*get_enable*() drivers/gpu/drm/bridge/sii902x.c | 26 -- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++ 2 files changed, 7 insertions(+), 42 deletions(-) base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa -- 2.38.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =] signature.asc Description: PGP signature
Re: The state of Quantization Range handling
On Tue, 15 Nov 2022 00:11:56 +0100 Sebastian Wick wrote: > There are still regular bug reports about monitors (sinks) and sources > disagreeing about the quantization range of the pixel data. In > particular sources sending full range data when the sink expects > limited range. From a user space perspective, this is all hidden in > the kernel. We send full range data to the kernel and then hope it > does the right thing but as the bug reports show: some combinations of > displays and drivers result in problems. > > In general the whole handling of the quantization range on linux is > not defined or documented at all. User space sends full range data > because that's what seems to work most of the time but technically > this is all undefined and user space can not fix those issues. Some > compositors have resorted to giving users the option to choose the > quantization range but this really should only be necessary for > straight up broken hardware. > > Quantization Range can be explicitly controlled by AVI InfoFrame or > HDMI General Control Packets. This is the ideal case and when the > source uses them there is not a lot that can go wrong. Not all > displays support those explicit controls in which case the chosen > video format (IT, CE, SD; details in CTA-861-H 5.1) influences which > quantization range the sink expects. > > This means that we have to expect that sometimes we have to send > limited and sometimes full range content. The big question however > that is not answered in the docs: who is responsible for making sure > the data is in the correct range? Is it the kernel or user space? > > If it's the kernel: does user space supply full range or limited range > content? Each of those has a disadvantage. If we send full range > content and the driver scales it down to limited range, we can't use > the out-of-range bits to transfer information. If we send limited > range content and the driver scales it up we lose information. > > Either way, this must be documented. My suggestion is to say that the > kernel always expects full range data as input and is responsible for > scaling it to limited range data if the sink expects limited range > data. Hi Sebastian, you are proposing the that driver/hardware will do either no range conversion, or full-to-limited range conversion. Limited-to-full range conversion would never be supported. I still wonder if limited-to-full range conversion could be useful with video content. > Another problem is that some displays do not behave correctly. It must > be possible to override the kernel when the user detects such a > situation. This override then controls if the driver converts the full > range data coming from the client or not (Default, Force Limited, > Force Full). It does not try to control what range the sink expects. > Let's call this the Quantization Range Override property which should > be implemented by all drivers. In other words, a CRTC "quantization range conversion" property with values: - auto, with the assumption that color pipeline always produces full-range - identity - full-to-limited (- limited-to-full) If this property was truly independent of the metadata being sent to the sink, and of the framebuffer format, it would allow us to do four ways: both full/limited framebuffer on both full/limited sink. It would allow us to send sub-blacks and super-whites as well. More precisely, framebuffers would always have *undefined* quantization range. The configuration of the color pipeline then determines how that data is manipulated into a video signal. So I am advocating the same design as with color spaces: do not tell KMS what your colorspaces are. Instead tell KMS what operations it needs to do with the pixel data, and what metadata to send to the sink. > All drivers should make sure their behavior is correct: > > * check that drivers choose the correct default quantization range for > the selected mode Mode implying a quantization range is awkward, but maybe the kernel established modes should just have a flag for it. Then userspace would know. Unless the video mode system is extended to communicate IT/CE/SD/VIC and whatnot to userspace, making the modes better defined. Then userspace would know too. > * whenever explicit control is available, use it and set the > quantization range to full > * make sure that the hardware converts from full range to limited > range whenever the sink expects limited range > * implement the Quantization Range Override property > > I'm volunteering for the documentation, UAPI and maybe even the drm > core parts if there is willingness to tackle the issue. Is it a good idea to put even more automation/magic into configuring the color pipeline and metadata for a sink, making them even more intertwined? I would prefer the opposite direction, making thing more explicit and orthogonal. Thanks, pq > Appendix A: Broadcast RGB property > > A few drivers already implement the Broadcast RGB property
Re: [PATCH 1/2] drm/ttm: Clean up page shift operation
Am 16.11.22 um 09:50 schrieb Somalapuram Amaranath: remove page shift operations as ttm_resource moved from num_pages to size_t size in bytes. Signed-off-by: Somalapuram Amaranath --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +--- drivers/gpu/drm/ttm/ttm_range_manager.c| 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 974e85d8b6cc..19ad365dc159 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -541,12 +541,10 @@ int amdgpu_bo_create(struct amdgpu_device *adev, if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) { /* GWS and OA don't need any alignment. */ page_align = bp->byte_align; - size <<= PAGE_SHIFT; - } else if (bp->domain & AMDGPU_GEM_DOMAIN_GDS) { /* Both size and alignment must be a multiple of 4. */ page_align = ALIGN(bp->byte_align, 4); - size = ALIGN(size, 4) << PAGE_SHIFT; + size = ALIGN(size, 4); } else { /* Memory should be aligned at least to a page size. */ page_align = ALIGN(bp->byte_align, PAGE_SIZE) >> PAGE_SHIFT; diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c b/drivers/gpu/drm/ttm/ttm_range_manager.c index 0a8bc0b7f380..4c7cba4ffdbf 100644 --- a/drivers/gpu/drm/ttm/ttm_range_manager.c +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c @@ -83,7 +83,7 @@ static int ttm_range_man_alloc(struct ttm_resource_manager *man, spin_lock(&rman->lock); ret = drm_mm_insert_node_in_range(mm, &node->mm_nodes[0], - PFN_UP(node->base.size), + node->base.size, bo->page_alignment, 0, place->fpfn, lpfn, mode); You need to make sure that fpfn and lpfn are now page shifted instead. Same for the overlap and compatible functions. Regards, Christian. spin_unlock(&rman->lock);