Re: [PATCH -next] fbdev: via: Fix section mismatch warning in via_core_init()
On 2022/11/15 15:05, Helge Deller wrote: On 11/15/22 07:52, shangxiaojing wrote: On 2022/11/15 13:14, Helge Deller wrote: On 11/15/22 03:53, Shang XiaoJing wrote: Due to viafb_exit() with "__exit" tag, it should not be called by the __init function via_core_init(). I wonder if you can't instead of this and your previous patch (ab885d8c7e15) turn the i2c and gpio drivers to proper platform drivers, e.g. adding to bottom of via/via_i2c.c: module_platform_driver(_i2c_driver) instead of viafb_i2c_init() and viafb_i2c_exit(). Shouldn't they then automatically be loaded/unloaded? I'm sorry that I have no idea how to change an i2c driver to a platform driver. As for module_platform_driver(), I have checked and it looks like just a helper macro to definite XXX_init() and XXX_exit() instead of automatically load/unload the driver. module_platform_driver() uses module_driver() which adds module_init() which then adds code to call the generated xxx_init() functions at startup and exit. Besides, the XXX_init() and XXX_exit() definited by the module_platform_driver() can only call platform_driver_register()/platform_driver_unregister(), perhaps not suitable for via_driver? (or just need to call platform_driver_register() after change to platform driver) platform_driver_register() will be called by the newly generated XXX_init(). Do you have such a viafb card and can try? I'm sorry that I run the test on qemu, and have no viafb card. Thanks, -- Shang XiaoJing
Re: [PATCH -next] fbdev: via: Fix section mismatch warning in via_core_init()
On 11/15/22 07:52, shangxiaojing wrote: On 2022/11/15 13:14, Helge Deller wrote: On 11/15/22 03:53, Shang XiaoJing wrote: Due to viafb_exit() with "__exit" tag, it should not be called by the __init function via_core_init(). I wonder if you can't instead of this and your previous patch (ab885d8c7e15) turn the i2c and gpio drivers to proper platform drivers, e.g. adding to bottom of via/via_i2c.c: module_platform_driver(_i2c_driver) instead of viafb_i2c_init() and viafb_i2c_exit(). Shouldn't they then automatically be loaded/unloaded? I'm sorry that I have no idea how to change an i2c driver to a platform driver. As for module_platform_driver(), I have checked and it looks like just a helper macro to definite XXX_init() and XXX_exit() instead of automatically load/unload the driver. module_platform_driver() uses module_driver() which adds module_init() which then adds code to call the generated xxx_init() functions at startup and exit. Besides, the XXX_init() and XXX_exit() definited by the module_platform_driver() can only call platform_driver_register()/platform_driver_unregister(), perhaps not suitable for via_driver? (or just need to call platform_driver_register() after change to platform driver) platform_driver_register() will be called by the newly generated XXX_init(). Do you have such a viafb card and can try? Helge
[PATCH v2] drm/i915: remove circ_buf.h includes
The last user of macros from that include was removed in 2018 by the
commit below.
Fixes: 6cc42152b02b ("drm/i915: Remove support for legacy debugfs crc
interface")
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: David Airlie
Cc: Daniel Vetter
Cc: intel-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jiri Slaby (SUSE)
---
Notes:
[v2] fixed e-mail setup
drivers/gpu/drm/i915/display/intel_pipe_crc.c | 1 -
drivers/gpu/drm/i915/i915_irq.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
index 673454fbf784..e9774670e3f6 100644
--- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
@@ -24,7 +24,6 @@
*
*/
-#include
#include
#include
#include
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b0180ea38de0..a815a45a6e6b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -28,7 +28,6 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include
#include
#include
--
2.38.1
Re: [PATCH 1/3] Documentation/gpu: Fix section in the wrong scope
On Tue, Nov 08, 2022 at 09:49:45AM +, Tvrtko Ursulin wrote: On 07/11/2022 17:32, Lucas De Marchi wrote: That section should still be inside "DRM client usage stats" rather than as a sibling. Signed-off-by: Lucas De Marchi --- Documentation/gpu/drm-usage-stats.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst index 92c5117368d7..b46327356e80 100644 --- a/Documentation/gpu/drm-usage-stats.rst +++ b/Documentation/gpu/drm-usage-stats.rst @@ -126,7 +126,6 @@ percentage utilization of the engine, whereas drm-engine- only reflects time active without considering what frequency the engine is operating as a percentage of it's maximum frequency. -=== Driver specific implementations === Oops - yep. Reviewed-by: Tvrtko Ursulin thanks, applied this. Anyone could take a look in the other 2? thanks Lucas De Marchi
Re: [PATCH -next] fbdev: via: Fix section mismatch warning in via_core_init()
On 2022/11/15 13:14, Helge Deller wrote: On 11/15/22 03:53, Shang XiaoJing wrote: Due to viafb_exit() with "__exit" tag, it should not be called by the __init function via_core_init(). I wonder if you can't instead of this and your previous patch (ab885d8c7e15) turn the i2c and gpio drivers to proper platform drivers, e.g. adding to bottom of via/via_i2c.c: module_platform_driver(_i2c_driver) instead of viafb_i2c_init() and viafb_i2c_exit(). Shouldn't they then automatically be loaded/unloaded? I'm sorry that I have no idea how to change an i2c driver to a platform driver. As for module_platform_driver(), I have checked and it looks like just a helper macro to definite XXX_init() and XXX_exit() instead of automatically load/unload the driver. Besides, the XXX_init() and XXX_exit() definited by the module_platform_driver() can only call platform_driver_register()/platform_driver_unregister(), perhaps not suitable for via_driver? (or just need to call platform_driver_register() after change to platform driver) Thanks, -- Shang XiaoJing
[PATCH v5 2/3] drm/bridge: anx7625: register content protect property
Set support_hdcp so the connector can register content protect proterty when it's initializing. Signed-off-by: Hsin-Yi Wang Reviewed-by: Sean Paul --- v4->v5: no change --- drivers/gpu/drm/bridge/analogix/anx7625.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index b0ff1ecb80a5..0636ac59c739 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -2680,6 +2680,7 @@ static int anx7625_i2c_probe(struct i2c_client *client, platform->bridge.type = platform->pdata.panel_bridge ? DRM_MODE_CONNECTOR_eDP : DRM_MODE_CONNECTOR_DisplayPort; + platform->bridge.support_hdcp = true; drm_bridge_add(>bridge); -- 2.38.1.431.g37b22c650d-goog
[PATCH v5 3/3] drm/bridge: it6505: handle HDCP request
it6505 supports HDCP 1.3, but current implementation lacks the update of
HDCP status through drm_hdcp_update_content_protection(). it6505 default
enables the HDCP. If user set it to undesired then the driver will stop
HDCP.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: allen chen
---
v4->v5: no change
---
drivers/gpu/drm/bridge/ite-it6505.c | 55 +
1 file changed, 55 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
b/drivers/gpu/drm/bridge/ite-it6505.c
index 21a9b8422bda..be08b42de4ea 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -423,6 +423,7 @@ struct it6505 {
struct extcon_dev *extcon;
struct work_struct extcon_wq;
int extcon_state;
+ struct drm_connector *connector;
enum drm_connector_status connector_status;
enum link_train_status link_state;
struct work_struct link_works;
@@ -2399,6 +2400,14 @@ static void it6505_irq_hdcp_done(struct it6505 *it6505)
DRM_DEV_DEBUG_DRIVER(dev, "hdcp done interrupt");
it6505->hdcp_status = HDCP_AUTH_DONE;
+ if (it6505->connector) {
+ struct drm_device *drm_dev = it6505->connector->dev;
+
+ drm_modeset_lock(_dev->mode_config.connection_mutex, NULL);
+ drm_hdcp_update_content_protection(it6505->connector,
+
DRM_MODE_CONTENT_PROTECTION_ENABLED);
+ drm_modeset_unlock(_dev->mode_config.connection_mutex);
+ }
it6505_show_hdcp_info(it6505);
}
@@ -2931,6 +2940,7 @@ static void it6505_bridge_atomic_enable(struct drm_bridge
*bridge,
if (WARN_ON(!connector))
return;
+ it6505->connector = connector;
conn_state = drm_atomic_get_new_connector_state(state, connector);
if (WARN_ON(!conn_state))
@@ -2974,6 +2984,7 @@ static void it6505_bridge_atomic_disable(struct
drm_bridge *bridge,
DRM_DEV_DEBUG_DRIVER(dev, "start");
+ it6505->connector = NULL;
if (it6505->powered) {
it6505_drm_dp_link_set_power(>aux, >link,
DP_SET_POWER_D3);
@@ -3028,6 +3039,48 @@ static struct edid *it6505_bridge_get_edid(struct
drm_bridge *bridge,
return edid;
}
+static int it6505_connector_atomic_check(struct it6505 *it6505,
+struct drm_connector_state *state)
+{
+ struct device *dev = >client->dev;
+ int cp = state->content_protection;
+
+ DRM_DEV_DEBUG_DRIVER(dev, "hdcp connector state:%d, curr hdcp state:%d",
+cp, it6505->hdcp_status);
+
+ if (!it6505->hdcp_desired) {
+ DRM_DEV_DEBUG_DRIVER(dev, "sink not support hdcp");
+ return 0;
+ }
+
+ if (it6505->hdcp_status == HDCP_AUTH_GOING)
+ return -EINVAL;
+
+ if (cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
+ if (it6505->hdcp_status == HDCP_AUTH_DONE)
+ it6505_stop_hdcp(it6505);
+ } else if (cp == DRM_MODE_CONTENT_PROTECTION_DESIRED) {
+ if (it6505->hdcp_status == HDCP_AUTH_IDLE &&
+ it6505->link_state == LINK_OK)
+ it6505_start_hdcp(it6505);
+ } else {
+ DRM_DEV_DEBUG_DRIVER(dev, "invalid to set hdcp enabled");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int it6505_bridge_atomic_check(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ struct it6505 *it6505 = bridge_to_it6505(bridge);
+
+ return it6505_connector_atomic_check(it6505, conn_state);
+}
+
static const struct drm_bridge_funcs it6505_bridge_funcs = {
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
@@ -3035,6 +3088,7 @@ static const struct drm_bridge_funcs it6505_bridge_funcs
= {
.attach = it6505_bridge_attach,
.detach = it6505_bridge_detach,
.mode_valid = it6505_bridge_mode_valid,
+ .atomic_check = it6505_bridge_atomic_check,
.atomic_enable = it6505_bridge_atomic_enable,
.atomic_disable = it6505_bridge_atomic_disable,
.atomic_pre_enable = it6505_bridge_atomic_pre_enable,
@@ -3354,6 +3408,7 @@ static int it6505_i2c_probe(struct i2c_client *client,
it6505->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
it6505->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID |
DRM_BRIDGE_OP_HPD;
+ it6505->bridge.support_hdcp = true;
drm_bridge_add(>bridge);
return 0;
--
2.38.1.431.g37b22c650d-goog
[PATCH v5 1/3] drm_bridge: register content protect property
Some bridges are able to update HDCP status from userspace request if
they support HDCP.
HDCP property is the same as other connector properties that needs to be
created after the connecter is initialized and before the connector is
registered.
If there exists a bridge that supports HDCP, add the property to the
bridge connector.
Signed-off-by: Hsin-Yi Wang
Reviewed-by: Sean Paul
Reported-by: kernel test robot
---
v4->v5: fix compile warning when CONFIG_DRM_DISPLAY_HELPER=m
---
drivers/gpu/drm/drm_bridge_connector.c | 8
include/drm/drm_bridge.h | 4
2 files changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index 1c7d936523df..16d038c2982f 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -7,6 +7,7 @@
#include
#include
+#include
#include
#include
#include
@@ -333,6 +334,7 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
struct i2c_adapter *ddc = NULL;
struct drm_bridge *bridge, *panel_bridge = NULL;
int connector_type;
+ bool support_hdcp = false;
bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL);
if (!bridge_connector)
@@ -376,6 +378,9 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
if (drm_bridge_is_panel(bridge))
panel_bridge = bridge;
+
+ if (bridge->support_hdcp)
+ support_hdcp = true;
}
if (connector_type == DRM_MODE_CONNECTOR_Unknown) {
@@ -398,6 +403,9 @@ struct drm_connector *drm_bridge_connector_init(struct
drm_device *drm,
if (panel_bridge)
drm_panel_bridge_set_orientation(connector, panel_bridge);
+ if (support_hdcp && IS_REACHABLE(CONFIG_DRM_DISPLAY_HDCP_HELPER))
+ drm_connector_attach_content_protection_property(connector,
true);
+
return connector;
}
EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 6b65b0dfb4fb..1d2ab70f3436 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -768,6 +768,10 @@ struct drm_bridge {
* modes.
*/
bool interlace_allowed;
+ /**
+* @support_hdcp: Indicate that the bridge supports HDCP.
+*/
+ bool support_hdcp;
/**
* @ddc: Associated I2C adapter for DDC access, if any.
*/
--
2.38.1.431.g37b22c650d-goog
RE: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On 2022/11/14 22:37, Yang, Lixiao wrote: > On 2022/11/14 20:51, Yi Liu wrote: >> On 2022/11/10 00:57, Jason Gunthorpe wrote: >>> On Tue, Nov 08, 2022 at 11:18:03PM +0800, Yi Liu wrote: On 2022/11/8 17:19, Nicolin Chen wrote: > On Mon, Nov 07, 2022 at 08:52:44PM -0400, Jason Gunthorpe wrote: > >> This is on github: >> https://github.com/jgunthorpe/linux/commits/vfio_iommufd > [...] >> v2: >> - Rebase to v6.1-rc3, v4 iommufd series >> - Fixup comments and commit messages from list remarks >> - Fix leaking of the iommufd for mdevs >> - New patch to fix vfio modaliases when vfio container is disabled >> - Add a dmesg once when the iommufd provided /dev/vfio/vfio is opened >> to signal that iommufd is providing this > > I've redone my previous sanity tests. Except those reported bugs, > things look fine. Once we fix those issues, GVT and other modules > can run some more stressful tests, I think. our side is also starting test (gvt, nic passthrough) this version. need to wait a while for the result. >>> >>> I've updated the branches with the two functional fixes discussed on >>> the list plus all the doc updates. >>> >> >> I see, due to timzone, the kernel we grabbed is 37c9e6e44d77a, it has >> slight diff in the scripts/kernel-doc compared with the latest commit >> (6bb16a9c67769). I don't think it impacts the test. >> >> https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next >>(37c9e6e44d77a) >> >> Our side, Yu He, Lixiao Yang has done below tests on Intel platform >> with the above kernel, results are: >> >> 1) GVT-g test suit passed, Intel iGFx passthrough passed. >> >> 2) NIC passthrough test with different guest memory (1G/4G), passed. >> >> 3) Booting two different QEMUs in the same time but one QEMU opens >> legacy /dev/vfio/vfio and another opens /dev/iommu. Tests passed. >> >> 4) Tried below Kconfig combinations, results are expected. >> >> VFIO_CONTAINER=y, IOMMUFD=y -- test pass >> VFIO_CONTAINER=y, IOMMUFD=n -- test pass >> VFIO_CONTAINER=n, IOMMUFD=y , IOMMUFD_VFIO_CONTAINER=y -- test pass >> VFIO_CONTAINER=n, IOMMUFD=y , IOMMUFD_VFIO_CONTAINER=n -- no >> /dev/vfio/vfio, so test fail, expected >> >> 5) Tested devices from multi-device group. Assign such devices to the >> same VM, pass; assign them to different VMs, fail; assign them to a VM >> with Intel virtual VT-d, fail; Results are expected. >> >> Meanwhile, I also tested the branch in development branch for nesting, >> the basic functionality looks good. >> >> Tested-by: Yi Liu >> > Tested-by: Lixiao Yang > Tested-by: Yu He -- Best regards, He,Yu
Re: [PATCH -next] fbdev: via: Fix section mismatch warning in via_core_init()
On 11/15/22 03:53, Shang XiaoJing wrote:
Due to viafb_exit() with "__exit" tag, it should not be called by the
__init function via_core_init().
I wonder if you can't instead of this and your previous patch (ab885d8c7e15)
turn the i2c and gpio drivers to proper platform drivers, e.g.
adding to bottom of via/via_i2c.c:
module_platform_driver(_i2c_driver)
instead of viafb_i2c_init() and viafb_i2c_exit().
Shouldn't they then automatically be loaded/unloaded?
Helge
WARNING: modpost: drivers/video/fbdev/via/viafb.o: section mismatch in
reference: init_module (section: .init.text) -> viafb_exit (section:
.exit.text)
Fixes: ab885d8c7e15 ("fbdev: via: Fix error in via_core_init()")
Signed-off-by: Shang XiaoJing
---
drivers/video/fbdev/via/via-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/fbdev/via/via-core.c
b/drivers/video/fbdev/via/via-core.c
index b2e3b5df38cd..b8cd04defc5e 100644
--- a/drivers/video/fbdev/via/via-core.c
+++ b/drivers/video/fbdev/via/via-core.c
@@ -734,7 +734,6 @@ static int __init via_core_init(void)
if (ret) {
viafb_gpio_exit();
viafb_i2c_exit();
- viafb_exit();
return ret;
}
Re: [PATCH 1/1] drm/shmem: Dual licence the files as GPL-2 and MIT
Hi Robert, On Sat, 12 Nov 2022 19:42:10 + Robert Swindells wrote: > > Contributors to these files are: > > Stephen Rothwell I don't think my (1 line) contribution is even copyrightable, but anyway Acked-by: Stephen Rothwell -- Cheers, Stephen Rothwell pgpieGqz3_45S.pgp Description: OpenPGP digital signature
[PATCH -next] fbdev: via: Fix section mismatch warning in via_core_init()
Due to viafb_exit() with "__exit" tag, it should not be called by the
__init function via_core_init().
WARNING: modpost: drivers/video/fbdev/via/viafb.o: section mismatch in
reference: init_module (section: .init.text) -> viafb_exit (section:
.exit.text)
Fixes: ab885d8c7e15 ("fbdev: via: Fix error in via_core_init()")
Signed-off-by: Shang XiaoJing
---
drivers/video/fbdev/via/via-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/fbdev/via/via-core.c
b/drivers/video/fbdev/via/via-core.c
index b2e3b5df38cd..b8cd04defc5e 100644
--- a/drivers/video/fbdev/via/via-core.c
+++ b/drivers/video/fbdev/via/via-core.c
@@ -734,7 +734,6 @@ static int __init via_core_init(void)
if (ret) {
viafb_gpio_exit();
viafb_i2c_exit();
- viafb_exit();
return ret;
}
--
2.17.1
[PATCH -next] drm/msm/dpu: Fix some kernel-doc comments
Make the description of @init to @p in dpu_encoder_phys_wb_init() and remove @wb_roi in dpu_encoder_phys_wb_setup_fb() to clear the below warnings: drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c:139: warning: Excess function parameter 'wb_roi' description in 'dpu_encoder_phys_wb_setup_fb' drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c:699: warning: Function parameter or member 'p' not described in 'dpu_encoder_phys_wb_init' drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c:699: warning: Excess function parameter 'init' description in 'dpu_encoder_phys_wb_init' Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=3067 Reported-by: Abaci Robot Signed-off-by: Yang Li --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index 7cbcef6efe17..62f6ff6abf41 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -132,7 +132,6 @@ static void dpu_encoder_phys_wb_set_qos(struct dpu_encoder_phys *phys_enc) * dpu_encoder_phys_wb_setup_fb - setup output framebuffer * @phys_enc: Pointer to physical encoder * @fb:Pointer to output framebuffer - * @wb_roi:Pointer to output region of interest */ static void dpu_encoder_phys_wb_setup_fb(struct dpu_encoder_phys *phys_enc, struct drm_framebuffer *fb) @@ -692,7 +691,7 @@ static void dpu_encoder_phys_wb_init_ops(struct dpu_encoder_phys_ops *ops) /** * dpu_encoder_phys_wb_init - initialize writeback encoder - * @init: Pointer to init info structure with initialization params + * @p: Pointer to init info structure with initialization params */ struct dpu_encoder_phys *dpu_encoder_phys_wb_init( struct dpu_enc_phys_init_params *p) -- 2.20.1.7.g153144c
Re: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On 11/9/22 11:57 AM, Jason Gunthorpe wrote: > On Tue, Nov 08, 2022 at 11:18:03PM +0800, Yi Liu wrote: >> On 2022/11/8 17:19, Nicolin Chen wrote: >>> On Mon, Nov 07, 2022 at 08:52:44PM -0400, Jason Gunthorpe wrote: >>> This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd >>> [...] v2: - Rebase to v6.1-rc3, v4 iommufd series - Fixup comments and commit messages from list remarks - Fix leaking of the iommufd for mdevs - New patch to fix vfio modaliases when vfio container is disabled - Add a dmesg once when the iommufd provided /dev/vfio/vfio is opened to signal that iommufd is providing this >>> >>> I've redone my previous sanity tests. Except those reported bugs, >>> things look fine. Once we fix those issues, GVT and other modules >>> can run some more stressful tests, I think. >> >> our side is also starting test (gvt, nic passthrough) this version. need to >> wait a while for the result. > > I've updated the branches with the two functional fixes discussed on > the list plus all the doc updates. > For s390, tested vfio-pci against some data mover workloads using QEMU on s390x with CONFIG_VFIO_CONTAINER=y and =n using zPCI interpretation assists (over ism/SMC-D, mlx5 and NVMe) and without zPCI interpretation assists (over mlx5 and NVMe) - will continue testing with more aggressive workloads. (I did not run with CONFIG_IOMMUFD_TEST other than when building the selftest, but I see you mentioned this to Yi -- I'll incorporate that setting into future runs.) Ran the self-tests on s390 in LPAR and within a QEMU guest -- all tests pass (used 1M hugepages) Did light regression testing of vfio-ap and vfio-ccw on s390x with CONFIG_VFIO_CONTAINER=y and =n. Didn't see it in your branch yet, but also verified the proposed change to iommufd_fill_cap_dma_avail (.avail = U32_MAX) would work as expected. Tested-by: Matthew Rosato
Re: [PATCH] mm/migrate_device: Return number of migrating pages in args->cpages
On 11/10/22 16:51, Alistair Popple wrote:
migrate_vma->cpages originally contained a count of the number of
pages migrating including non-present pages which can be poluated
"populated"
directly on the target.
Commit 241f68859656 ("mm/migrate_device.c: refactor migrate_vma and
migrate_deivce_coherent_page()") inadvertantly changed this to contain
just the number of pages that were unmapped. Usage of
migrate_vma->cpages isn't documented, but most drivers use it to see
if all the requested addresses can be migrated so restore the original
behaviour.
Fixes: 241f68859656 ("mm/migrate_device.c: refactor migrate_vma and
migrate_deivce_coherent_page()")
Signed-off-by: Alistair Popple
Reported-by: Ralph Campbell
You can add
Reviewed-by: Ralph Campbell
Thanks!
The state of Quantization Range handling
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. 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. All drivers should make sure their behavior is correct: * check that drivers choose the correct default quantization range for the selected mode * 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. Appendix A: Broadcast RGB property A few drivers already implement the Broadcast RGB property to control the quantization range. However, it is pointless: It can be set to Auto, Full and Limited when the sink supports explicitly setting the quantization range. The driver expects full range content and converts it to limited range content when necessary. Selecting limited range never makes any sense: the out-of-range bits can't be used because the input is full range. Selecting Default never makes sense: relying on the default quantization range is risky because sinks often get it wrong and as we established there is no reason to select limited range if not necessary. The limited and full options also are not suitable as an override because the property is not available if the sink does not support explicitly setting the quantization range.
[Bug 216143] [bisected] garbled screen when starting X + dmesg cluttered with "[drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed in the dependencies handling -1431655766!"
https://bugzilla.kernel.org/show_bug.cgi?id=216143 --- Comment #17 from Erhard F. (erhar...@mailbox.org) --- Created attachment 303181 --> https://bugzilla.kernel.org/attachment.cgi?id=303181=edit kernel .config (kernel 6.1-rc5, AMD Ryzen 9 5950X) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 216143] [bisected] garbled screen when starting X + dmesg cluttered with "[drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed in the dependencies handling -1431655766!"
https://bugzilla.kernel.org/show_bug.cgi?id=216143 --- Comment #16 from Erhard F. (erhar...@mailbox.org) --- Created attachment 303180 --> https://bugzilla.kernel.org/attachment.cgi?id=303180=edit kernel dmesg (kernel 6.1-rc5, AMD Ryzen 9 5950X) Reinvestigating on kernel 6.1-rc5 built with clang 15.0.3 + lld 15.0.3. So far I was not able to reproduce the bug! X runs just fine for now. I'll close here once 6.1 is stable and I can assure the bug does no longer show up on my other affected machines (AMD PRO A12-8830B, AMD PRO A10-8750B) too. -- 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] drm/amd/dc/dce120: Fix audio register mapping, stop triggering KASAN
There's been a very long running bug that seems to have been neglected for
a while, where amdgpu consistently triggers a KASAN error at start:
BUG: KASAN: global-out-of-bounds in read_indirect_azalia_reg+0x1d4/0x2a0
[amdgpu]
Read of size 4 at addr c2274b28 by task modprobe/1889
After digging through amd's rather creative method for accessing registers,
I eventually discovered the problem likely has to do with the fact that on
my dce120 GPU there are supposedly 7 sets of audio registers. But we only
define a register mapping for 6 sets.
So, fix this and fix the KASAN warning finally.
Signed-off-by: Lyude Paul
Cc: sta...@vger.kernel.org
---
Sending this one separately from the rest of my fixes since:
* It's definitely completely unrelated to the Gitlab 2171 issue
* I'm not sure if this is the correct fix since it's in DC
drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
index 1b70b78e2fa15..af631085e88c5 100644
--- a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
@@ -359,7 +359,8 @@ static const struct dce_audio_registers audio_regs[] = {
audio_regs(2),
audio_regs(3),
audio_regs(4),
- audio_regs(5)
+ audio_regs(5),
+ audio_regs(6),
};
#define DCE120_AUD_COMMON_MASK_SH_LIST(mask_sh)\
--
2.37.3
[PATCH v2 3/4] drm/amdgpu/dm/mst: Use the correct topology mgr pointer in amdgpu_dm_connector
This bug hurt me. Basically, it appears that we've been grabbing the
entirely wrong mutex in the MST DSC computation code for amdgpu! While
we've been grabbing:
amdgpu_dm_connector->mst_mgr
That's zero-initialized memory, because the only connectors we'll ever
actually be doing DSC computations for are MST ports. Which have mst_mgr
zero-initialized, and instead have the correct topology mgr pointer located
at:
amdgpu_dm_connector->mst_port->mgr;
I'm a bit impressed that until now, this code has managed not to crash
anyone's systems! It does seem to cause a warning in LOCKDEP though:
[ 66.637670] DEBUG_LOCKS_WARN_ON(lock->magic != lock)
This was causing the problems that appeared to have been introduced by:
commit 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the
atomic state")
This wasn't actually where they came from though. Presumably, before the
only thing we were doing with the topology mgr pointer was attempting to
grab mst_mgr->lock. Since the above commit however, we grab much more
information from mst_mgr including the atomic MST state and respective
modesetting locks.
This patch also implies that up until now, it's quite likely we could be
susceptible to race conditions when going through the MST topology state
for DSC computations since we technically will not have grabbed any lock
when going through it.
So, let's fix this by adjusting all the respective code paths to look at
the right pointer and skip things that aren't actual MST connectors from a
topology.
Signed-off-by: Lyude Paul
Gitlab issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
Fixes: 8c20a1ed9b4f ("drm/amd/display: MST DSC compute fair share")
Cc: # v5.6+
---
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 37 +--
1 file changed, 18 insertions(+), 19 deletions(-)
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 bba2e8aaa2c20..5196c9a0e432d 100644
--- 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
@@ -1117,6 +1117,7 @@ int compute_mst_dsc_configs_for_state(struct
drm_atomic_state *state,
struct dc_stream_state *stream;
bool computed_streams[MAX_PIPES];
struct amdgpu_dm_connector *aconnector;
+ struct drm_dp_mst_topology_mgr *mst_mgr;
int link_vars_start_index = 0;
int ret = 0;
@@ -1131,7 +1132,7 @@ int compute_mst_dsc_configs_for_state(struct
drm_atomic_state *state,
aconnector = (struct amdgpu_dm_connector
*)stream->dm_stream_context;
- if (!aconnector || !aconnector->dc_sink)
+ if (!aconnector || !aconnector->dc_sink || !aconnector->port)
continue;
if
(!aconnector->dc_sink->dsc_caps.dsc_dec_caps.is_dsc_supported)
@@ -1146,16 +1147,13 @@ int compute_mst_dsc_configs_for_state(struct
drm_atomic_state *state,
if (!is_dsc_need_re_compute(state, dc_state, stream->link))
continue;
- mutex_lock(>mst_mgr.lock);
-
- ret = compute_mst_dsc_configs_for_link(state, dc_state,
stream->link, vars,
- >mst_mgr,
+ mst_mgr = aconnector->port->mgr;
+ mutex_lock(_mgr->lock);
+ ret = compute_mst_dsc_configs_for_link(state, dc_state,
stream->link, vars, mst_mgr,
_vars_start_index);
- if (ret != 0) {
- mutex_unlock(>mst_mgr.lock);
+ mutex_unlock(_mgr->lock);
+ if (ret != 0)
return ret;
- }
- mutex_unlock(>mst_mgr.lock);
for (j = 0; j < dc_state->stream_count; j++) {
if (dc_state->streams[j]->link == stream->link)
@@ -1182,6 +1180,7 @@ static int pre_compute_mst_dsc_configs_for_state(struct
drm_atomic_state *state,
struct dc_stream_state *stream;
bool computed_streams[MAX_PIPES];
struct amdgpu_dm_connector *aconnector;
+ struct drm_dp_mst_topology_mgr *mst_mgr;
int link_vars_start_index = 0;
int ret;
@@ -1196,7 +1195,7 @@ static int pre_compute_mst_dsc_configs_for_state(struct
drm_atomic_state *state,
aconnector = (struct amdgpu_dm_connector
*)stream->dm_stream_context;
- if (!aconnector || !aconnector->dc_sink)
+ if (!aconnector || !aconnector->dc_sink || !aconnector->port)
continue;
if
(!aconnector->dc_sink->dsc_caps.dsc_dec_caps.is_dsc_supported)
@@ -1208,15 +1207,13 @@ static int pre_compute_mst_dsc_configs_for_state(struct
drm_atomic_state *state,
if (!is_dsc_need_re_compute(state, dc_state, stream->link))
continue;
-
[PATCH v2 4/4] drm/amdgpu/dm/dp_mst: Don't grab mst_mgr->lock when computing DSC state
Now that we've fixed the issue with using the incorrect topology manager,
we're actually grabbing the topology manager's lock - and consequently
deadlocking. Luckily for us though, there's actually nothing in AMD's DSC
state computation code that really should need this lock. The one exception
is the mutex_lock() in dm_dp_mst_is_port_support_mode(), however we grab no
locks beneath >lock there so that should be fine to leave be.
Signed-off-by: Lyude Paul
Gitlab issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
Fixes: 8c20a1ed9b4f ("drm/amd/display: MST DSC compute fair share")
Cc: # v5.6+
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 4
1 file changed, 4 deletions(-)
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 5196c9a0e432d..59648f5ffb59d 100644
--- 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
@@ -1148,10 +1148,8 @@ int compute_mst_dsc_configs_for_state(struct
drm_atomic_state *state,
continue;
mst_mgr = aconnector->port->mgr;
- mutex_lock(_mgr->lock);
ret = compute_mst_dsc_configs_for_link(state, dc_state,
stream->link, vars, mst_mgr,
_vars_start_index);
- mutex_unlock(_mgr->lock);
if (ret != 0)
return ret;
@@ -1208,10 +1206,8 @@ static int pre_compute_mst_dsc_configs_for_state(struct
drm_atomic_state *state,
continue;
mst_mgr = aconnector->port->mgr;
- mutex_lock(_mgr->lock);
ret = compute_mst_dsc_configs_for_link(state, dc_state,
stream->link, vars, mst_mgr,
_vars_start_index);
- mutex_unlock(_mgr->lock);
if (ret != 0)
return ret;
--
2.37.3
[PATCH v2 2/4] drm/display/dp_mst: Fix drm_dp_mst_add_affected_dsc_crtcs() return code
Looks like that we're accidentally dropping a pretty important return code
here. For some reason, we just return -EINVAL if we fail to get the MST
topology state. This is wrong: error codes are important and should never
be squashed without being handled, which here seems to have the potential
to cause a deadlock.
Signed-off-by: Lyude Paul
Reviewed-by: Wayne Lin
Fixes: 8ec046716ca8 ("drm/dp_mst: Add helper to trigger modeset on affected DSC
MST CRTCs")
Cc: # v5.6+
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index ecd22c038c8c0..51a46689cda70 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -5186,7 +5186,7 @@ int drm_dp_mst_add_affected_dsc_crtcs(struct
drm_atomic_state *state, struct drm
mst_state = drm_atomic_get_mst_topology_state(state, mgr);
if (IS_ERR(mst_state))
- return -EINVAL;
+ return PTR_ERR(mst_state);
list_for_each_entry(pos, _state->payloads, next) {
--
2.37.3
[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, _state, vars)) {
- ret = -EINVAL;
+ ret = pre_validate_dsc(state, _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_dm/amdgpu_dm_mst_types.c
@@ -703,13 +703,13 @@ static int bpp_x16_from_pbn(struct
dsc_mst_fairness_params param, int pbn)
return dsc_config.bits_per_pixel;
}
-static bool increase_dsc_bpp(struct drm_atomic_state *state,
-struct drm_dp_mst_topology_state *mst_state,
-struct dc_link *dc_link,
-struct dsc_mst_fairness_params *params,
-struct dsc_mst_fairness_vars *vars,
-int count,
-int k)
+static int increase_dsc_bpp(struct drm_atomic_state *state,
+ struct drm_dp_mst_topology_state *mst_state,
+ struct dc_link *dc_link,
+ struct dsc_mst_fairness_params *params,
+ struct dsc_mst_fairness_vars *vars,
+ int count,
+ int k)
{
int i;
bool bpp_increased[MAX_PIPES];
@@ -719,6 +719,7 @@ static bool increase_dsc_bpp(struct drm_atomic_state *state,
int remaining_to_increase = 0;
int link_timeslots_used;
int fair_pbn_alloc;
+ int ret = 0;
for (i = 0; i < count; i++) {
if (vars[i + k].dsc_enabled) {
@@ -757,52 +758,60 @@ static bool increase_dsc_bpp(struct drm_atomic_state
*state,
if (initial_slack[next_index] > fair_pbn_alloc) {
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gt: Add GT oriented dmesg output
On 11/10/2022 01:43, Tvrtko Ursulin wrote:
On 09/11/2022 17:46, John Harrison wrote:
On 11/9/2022 03:05, Tvrtko Ursulin wrote:
On 08/11/2022 20:15, John Harrison wrote:
On 11/8/2022 01:01, Tvrtko Ursulin wrote:
On 07/11/2022 19:14, John Harrison wrote:
On 11/7/2022 08:17, Tvrtko Ursulin wrote:
On 07/11/2022 09:33, Tvrtko Ursulin wrote:
On 05/11/2022 01:03, Ceraolo Spurio, Daniele wrote:
On 11/4/2022 10:25 AM, john.c.harri...@intel.com wrote:
From: John Harrison
When trying to analyse bug reports from CI, customers, etc.
it can be
difficult to work out exactly what is happening on which GT in a
multi-GT system. So add GT oriented debug/error message
wrappers. If
used instead of the drm_ equivalents, you get the same output
but with
a GT# prefix on it.
Signed-off-by: John Harrison
The only downside to this is that we'll print "GT0: " even on
single-GT devices. We could introduce a gt->info.name and
print that, so we could have it different per-platform, but
IMO it's not worth the effort.
Reviewed-by: Daniele Ceraolo Spurio
I think it might be worth getting an ack from one of the
maintainers to make sure we're all aligned on transitioning to
these new logging macro for gt code.
Idea is I think a very good one. First I would suggest
standardising to lowercase GT in logs because:
$ grep "GT%" i915/ -r
$ grep "gt%" i915/ -r
i915/gt/intel_gt_sysfs.c: gt->i915->sysfs_gt, "gt%d",
gt->info.id))
i915/gt/intel_gt_sysfs.c: "failed to initialize
gt%d sysfs root\n", gt->info.id);
i915/gt/intel_gt_sysfs_pm.c: "failed to create gt%u RC6 sysfs
files (%pe)\n",
i915/gt/intel_gt_sysfs_pm.c: "failed to create gt%u RC6p sysfs
files (%pe)\n",
i915/gt/intel_gt_sysfs_pm.c: "failed to create gt%u RPS sysfs
files (%pe)",
i915/gt/intel_gt_sysfs_pm.c: "failed to create gt%u
punit_req_freq_mhz sysfs (%pe)",
i915/gt/intel_gt_sysfs_pm.c: "failed to create gt%u throttle
sysfs files (%pe)",
i915/gt/intel_gt_sysfs_pm.c: "failed to create gt%u
media_perf_power_attrs sysfs (%pe)\n",
i915/gt/intel_gt_sysfs_pm.c: "failed to add gt%u rps defaults
(%pe)\n",
i915/i915_driver.c: drm_err(>i915->drm, "gt%d:
intel_pcode_init failed %d\n", id, ret);
i915/i915_hwmon.c: snprintf(ddat_gt->name,
sizeof(ddat_gt->name), "i915_gt%u", i);
Just because there are 11 existing instances of one form doesn't
mean that the 275 instances that are waiting to be converted
should be done incorrectly. GT is an acronym and should be
capitalised.
Okay just make it consistent then.
Besides:
grep -r "GT " i915 | grep '"'
i915/vlv_suspend.c: drm_err(>drm, "timeout disabling GT
waking\n");
i915/vlv_suspend.c: "timeout waiting for GT
wells to go %s\n",
i915/vlv_suspend.c: drm_dbg(>drm, "GT register access
while GT waking disabled\n");
i915/i915_gpu_error.c: err_printf(m, "GT awake: %s\n",
str_yes_no(gt->awake));
i915/i915_debugfs.c: seq_printf(m, "GT awake? %s [%d], %llums\n",
i915/selftests/i915_gem_evict.c: pr_err("Failed to idle GT (on
%s)", engine->name);
i915/intel_uncore.c: "GT thread status wait
timed out\n");
i915/gt/uc/selftest_guc_multi_lrc.c: drm_err(>i915->drm, "GT
failed to idle: %d\n", ret);
i915/gt/uc/selftest_guc.c: drm_err(>i915->drm, "GT failed to
idle: %d\n", ret);
i915/gt/uc/selftest_guc.c: drm_err(>i915->drm, "GT failed to
idle: %d\n", ret);
i915/gt/intel_gt_mcr.c: * Some GT registers are designed as
"multicast" or "replicated" registers:
i915/gt/selftest_rps.c: pr_info("%s: rps counted
%d C0 cycles [%lldns] in %lldns [%d cycles], using GT clock
frequency of %uKHz\n",
i915/gt/selftest_hangcheck.c: pr_err("[%s] GT is wedged!\n",
engine->name);
i915/gt/selftest_hangcheck.c: pr_err("GT is wedged!\n");
i915/gt/intel_gt_clock_utils.c: "GT clock
frequency changed, was %uHz, now %uHz!\n",
i915/gt/selftest_engine_pm.c: pr_err("Unable to flush
GT pm before test\n");
i915/gt/selftest_engine_pm.c: pr_err("GT failed to idle\n");
i915/i915_sysfs.c: "failed to register GT
sysfs directory\n");
i915/intel_uncore.h: * of the basic non-engine GT registers
(referred to as "GSI" on
i915/intel_uncore.h: * newer platforms, or "GT block" on
older platforms)? If so, we'll
Then there is a question of naming. Are we okay with GT_XXX or,
do we want intel_gt_, or something completely different. I
don't have a strong opinion at the moment so I'll add some more
folks to Cc.
You mean GT_ERR("msg") vs intel_gt_err("msg")? Personally, I
would prefer just gt_err("msg") to keep it as close to the
official drm_* versions as possible. Print lines tend to be
excessively long already. Taking a 'gt' parameter instead of a
'>i915->drm' parameter does help with that but it seems like
calling the wrapper intel_gt_* is shooting ourselves in the foot
on that one. And GT_ERR vs gt_err just comes down to the fact
that it is a macro wrapper and therefore is required to
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gt: Add GT oriented dmesg output
On 11/10/2022 02:33, Jani Nikula wrote:
On Wed, 09 Nov 2022, Michal Wajdeczko wrote:
Instead of merging this patch now, oriented on GT only, I would rather
wait until we discuss and plan solution for the all sub-components.
Once that's done (with agreement on naming and output) we can start
converting exiting messages.
My proposal would be:
- use wrappers per component
- use lower case names
- don't add colon
#define i915_xxx(_i915, _fmt, ...) \
drm_xxx(&(_i915)->drm, _fmt, ##__VA_ARGS__)
#define gt_xxx(_gt, _fmt, ...) \
i915_xxx((_gt)->i915, "GT%u " _fmt, (_gt)->info.id, ..
#define guc_xxx(_guc, _fmt, ...) \
gt_xxx(guc_to_gt(_guc), "GuC " _fmt, ..
#define ct_xxx(_ct, _fmt, ...) \
guc_xxx(ct_to_guc(_ct), "CTB " _fmt, ..
where
xxx = { err, warn, notice, info, dbg }
and then for calls like:
i915_err(i915, "Foo failed (%pe)\n", ERR_PTR(err));
gt_err(gt, "Foo failed (%pe)\n", ERR_PTR(err));
guc_err(guc, "Foo failed (%pe)\n", ERR_PTR(err));
ct_err(ct, "Foo failed (%pe)\n", ERR_PTR(err));
the output would look like:
[ ] i915 :00:02.0: [drm] *ERROR* Foo failed (-EIO)
[ ] i915 :00:02.0: [drm] *ERROR* GT0 Foo failed (-EIO)
[ ] i915 :00:02.0: [drm] *ERROR* GT0 GuC Foo failed (-EIO)
[ ] i915 :00:02.0: [drm] *ERROR* GT0 GuC CTB Foo failed (-EIO)
Would that work for all of you?
I think we borderline have too many wrappers already. I've basically
blocked the i915_* variants early on in favour of just using the drm_*
calls directly - which, themselves, are also wrappers on dev_*.
In general, there's aversion in the kernel to adding wrappers that hide
what's actually going on. WYSIWYG.
That is blatantly not the case. There is a very strong push in kernel
code reviews to abstract common code into helper functions. How is this
any different? If you removed all helper functions with the desire to
have truly WYSIWYG code then each line of code would be a paragraph long.
So I wasn't thrilled to see the GT_* wrappers either.
And indeed part of it is that anything you do sets an example for the
next developer coming along. You add wrappers for this, and sure enough
someone's going to cargo cult and add wrappers for some other
component. We didn't even have to merge this to see that idea pop up.
The thing with logging is that it's not just one wrapper. It's info,
notice, warn, err, multiplied by the "once" variants, with some
duplicated by the "ratelimited" variants. (And yeah, C sucks at this.)
Arguably the display code could use logging wrappers for connectors,
encoders and crtcs, too. Not just i915, but across drm. See:
git grep "\[\(CONNECTOR\|ENCODER\|CRTC\)"
But we've opted not to.
This is just my educated guess, but what I think is more likely to
happen is adding a "debug name" member to the relevant structs that you
could use like:
drm_dbg_kms(connector->dev, "%s foo\n", connector->dbg_id);
Huh? You want the wrapper to be called drm_dbg_gt instead of gt_dbg but
otherwise it is identical to what is currently in the patch? Other than
being a longer name, how is that any different? It is still very much a
wrapper of exactly the type that you don't seem to like? And as noted
already, in that case shouldn't we be pushing a patch to rename drm_dbg
to dev_dbg_drm first? In which case the above should be dev_dbg_drm_kms?
---
I'm not going to block this patchset, but I also really *really* don't
want to see a cargo culted copy of this for other components. Which kind
of makes me anxious.
I really don't understand the problem with adding such wrappers. Sure,
there are a bunch of varients - dbg, err, warn, etc. But the variant
list is contained and unlikely to grow at any point. It is trivially
easy to look up gt_dbg to find out what it is doing. It makes the code
much easier to read. I mean, who cares that gt_dbg is implemented via
drm_dbg which is implemented via dev_dbg? Only someone that is actively
wanting to further change the implementation. Anyone who is using it
just wants to know that they are calling the correct debug printer for
their module and are giving it the correct parameters. Having a nice
simple name makes that totally clear.
So what if you looked again what you could do to make using the drm_*
logging variants easier?
Not a lot. All other proposals so far have been adding complicated and
unwieldy constructs to every single print.
Fundamentally, the drm_dbg wrappers as they stand do not allow for
further customisation. So your choice is to replace, rewrite or wrap.
Replacing it with an entirely parallel implementation is fundamentally
the wrong direction. Re-writing the DRM layer to take a pointer to a
custom printer object instead of a 'struct drm_device' (where each
sub-module defines it's own private printer that adds it's preferred
string and chains on to the next printer) seems like a huge change to
the entire DRM code base for
Re: [PATCH] drm/edid: Dump the EDID when drm_edid_get_panel_id() has an error
Hi, On Mon, Nov 14, 2022 at 2:02 AM Jani Nikula wrote: > > On Fri, 11 Nov 2022, Doug Anderson wrote: > > Hi, > > > > On Tue, Oct 25, 2022 at 1:39 PM Abhinav Kumar > > wrote: > >> > >> Hi Doug > >> > >> On 10/24/2022 1:28 PM, Doug Anderson wrote: > >> > Hi, > >> > > >> > On Fri, Oct 21, 2022 at 2:18 PM Abhinav Kumar > >> > wrote: > >> >> > >> >> Hi Doug > >> >> > >> >> On 10/21/2022 1:07 PM, Douglas Anderson wrote: > >> >>> If we fail to get a valid panel ID in drm_edid_get_panel_id() we'd > >> >>> like to see the EDID that was read so we have a chance of > >> >>> understanding what's wrong. There's already a function for that, so > >> >>> let's call it in the error case. > >> >>> > >> >>> NOTE: edid_block_read() has a retry loop in it, so actually we'll only > >> >>> print the block read back from the final attempt. This still seems > >> >>> better than nothing. > >> >>> > >> >>> Signed-off-by: Douglas Anderson > >> >> > >> >> Instead of checkinf for edid_block_status_valid() on the base_block, do > >> >> you want to use drm_edid_block_valid() instead? > >> >> > >> >> That way you get the edid_block_dump() for free if it was invalid. > >> > > >> > I can... ...but it feels a bit awkward and maybe not quite how the > >> > functions were intended to work together? > >> > > >> > One thing I notice is that if I call drm_edid_block_valid() I'm doing > >> > a bunch of duplicate work that already happened in edid_block_read(), > >> > which already calls edid_block_check() and handles fixing headers. I > >> > guess also if I call drm_edid_block_valid() then I should ignore the > >> > "status" return value of edid_block_read() because we don't need to > >> > pass it anywhere (because the work is re-done in > >> > drm_edid_block_valid()). > >> > > >> > So I guess I'm happy to do a v2 like that if everyone likes it better, > >> > but to me it feels a little weird. > >> > > >> > -Doug > >> > >> Alright, agreed. There is some duplication of code happening if we use > >> drm_edid_block_valid(). I had suggested that because it has inherent > >> support for dumping the bad EDID. > >> > >> In that case, this change LGTM, because in principle you are doing the > >> same thing as _drm_do_get_edid() (with the only difference being here we > >> read only the base block as opposed to the full EDID there). > >> > >> Hence, > >> > >> Reviewed-by: Abhinav Kumar > > > > I've given this patch a bunch of time because it wasn't urgent, but > > seems like it could be about time to land. I'll plan to land it next > > Monday or Tuesday unless anyone has any other comments. > > Ack, it's benign enough. Thanks! Since you didn't write the above as an Acked-by tag I didn't add it to the commit, but I went ahead and landed with Abhinav's tag. 69c7717c20cc drm/edid: Dump the EDID when drm_edid_get_panel_id() has an error -Doug
Re: [PATCH] drm/sti: Fix return type of sti_{dvo,hda,hdmi}_connector_mode_valid()
Hi all,
On Wed, Nov 02, 2022 at 08:56:23AM -0700, Nathan Chancellor wrote:
> With clang's kernel control flow integrity (kCFI, CONFIG_CFI_CLANG),
> indirect call targets are validated against the expected function
> pointer prototype to make sure the call target is valid to help mitigate
> ROP attacks. If they are not identical, there is a failure at run time,
> which manifests as either a kernel panic or thread getting killed. A
> proposed warning in clang aims to catch these at compile time, which
> reveals:
>
> drivers/gpu/drm/sti/sti_hda.c:637:16: error: incompatible function pointer
> types initializing 'enum drm_mode_status (*)(struct drm_connector *, struct
> drm_display_mode *)' with an expression of type 'int (struct drm_connector *,
> struct drm_display_mode *)'
> [-Werror,-Wincompatible-function-pointer-types-strict]
> .mode_valid = sti_hda_connector_mode_valid,
> ^~~~
> drivers/gpu/drm/sti/sti_dvo.c:376:16: error: incompatible function pointer
> types initializing 'enum drm_mode_status (*)(struct drm_connector *, struct
> drm_display_mode *)' with an expression of type 'int (struct drm_connector *,
> struct drm_display_mode *)'
> [-Werror,-Wincompatible-function-pointer-types-strict]
> .mode_valid = sti_dvo_connector_mode_valid,
> ^~~~
> drivers/gpu/drm/sti/sti_hdmi.c:1035:16: error: incompatible function
> pointer types initializing 'enum drm_mode_status (*)(struct drm_connector *,
> struct drm_display_mode *)' with an expression of type 'int (struct
> drm_connector *, struct drm_display_mode *)'
> [-Werror,-Wincompatible-function-pointer-types-strict]
> .mode_valid = sti_hdmi_connector_mode_valid,
> ^
>
> ->mode_valid() in 'struct drm_connector_helper_funcs' expects a return
> type of 'enum drm_mode_status', not 'int'. Adjust the return type of
> sti_{dvo,hda,hdmi}_connector_mode_valid() to match the prototype's to
> resolve the warning and CFI failure.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1750
> Signed-off-by: Nathan Chancellor
> ---
> drivers/gpu/drm/sti/sti_dvo.c | 5 +++--
> drivers/gpu/drm/sti/sti_hda.c | 5 +++--
> drivers/gpu/drm/sti/sti_hdmi.c | 5 +++--
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> index b6ee8a82e656..076d5f30a09c 100644
> --- a/drivers/gpu/drm/sti/sti_dvo.c
> +++ b/drivers/gpu/drm/sti/sti_dvo.c
> @@ -346,8 +346,9 @@ static int sti_dvo_connector_get_modes(struct
> drm_connector *connector)
>
> #define CLK_TOLERANCE_HZ 50
>
> -static int sti_dvo_connector_mode_valid(struct drm_connector *connector,
> - struct drm_display_mode *mode)
> +static enum drm_mode_status
> +sti_dvo_connector_mode_valid(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> {
> int target = mode->clock * 1000;
> int target_min = target - CLK_TOLERANCE_HZ;
> diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> index 03cc401ed593..a53b5a15c2a9 100644
> --- a/drivers/gpu/drm/sti/sti_hda.c
> +++ b/drivers/gpu/drm/sti/sti_hda.c
> @@ -601,8 +601,9 @@ static int sti_hda_connector_get_modes(struct
> drm_connector *connector)
>
> #define CLK_TOLERANCE_HZ 50
>
> -static int sti_hda_connector_mode_valid(struct drm_connector *connector,
> - struct drm_display_mode *mode)
> +static enum drm_mode_status
> +sti_hda_connector_mode_valid(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> {
> int target = mode->clock * 1000;
> int target_min = target - CLK_TOLERANCE_HZ;
> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index cb82622877d2..09e0cadb6368 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -1004,8 +1004,9 @@ static int sti_hdmi_connector_get_modes(struct
> drm_connector *connector)
>
> #define CLK_TOLERANCE_HZ 50
>
> -static int sti_hdmi_connector_mode_valid(struct drm_connector *connector,
> - struct drm_display_mode *mode)
> +static enum drm_mode_status
> +sti_hdmi_connector_mode_valid(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> {
> int target = mode->clock * 1000;
> int target_min = target - CLK_TOLERANCE_HZ;
>
> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
> --
> 2.38.1
>
>
Could someone please pick this up so that it makes 6.2? We would like
to try and get this warning turned on so that it can catch more
potential run time issues at compile time but that can only happen when
all the warnings are fixed.
Cheers,
Nathan
Re: [PATCH] drm/fsl-dcu: Fix return type of fsl_dcu_drm_connector_mode_valid()
Hi all,
On Wed, Nov 02, 2022 at 08:42:15AM -0700, Nathan Chancellor wrote:
> With clang's kernel control flow integrity (kCFI, CONFIG_CFI_CLANG),
> indirect call targets are validated against the expected function
> pointer prototype to make sure the call target is valid to help mitigate
> ROP attacks. If they are not identical, there is a failure at run time,
> which manifests as either a kernel panic or thread getting killed. A
> proposed warning in clang aims to catch these at compile time, which
> reveals:
>
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c:74:16: error: incompatible
> function pointer types initializing 'enum drm_mode_status (*)(struct
> drm_connector *, struct drm_display_mode *)' with an expression of type 'int
> (struct drm_connector *, struct drm_display_mode *)'
> [-Werror,-Wincompatible-function-pointer-types-strict]
> .mode_valid = fsl_dcu_drm_connector_mode_valid,
> ^~~~
> 1 error generated.
>
> ->mode_valid() in 'struct drm_connector_helper_funcs' expects a return
> type of 'enum drm_mode_status', not 'int'. Adjust the return type of
> fsl_dcu_drm_connector_mode_valid() to match the prototype's to resolve
> the warning and CFI failure.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1750
> Reported-by: Sami Tolvanen
> Signed-off-by: Nathan Chancellor
> ---
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> index 4d4a715b429d..2c2b92324a2e 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> @@ -60,8 +60,9 @@ static int fsl_dcu_drm_connector_get_modes(struct
> drm_connector *connector)
> return drm_panel_get_modes(fsl_connector->panel, connector);
> }
>
> -static int fsl_dcu_drm_connector_mode_valid(struct drm_connector *connector,
> - struct drm_display_mode *mode)
> +static enum drm_mode_status
> +fsl_dcu_drm_connector_mode_valid(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> {
> if (mode->hdisplay & 0xf)
> return MODE_ERROR;
>
> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
> --
> 2.38.1
>
>
Could someone please pick this up so that it makes 6.2? We would like
to try and get this warning turned on so that it can catch more
potential run time issues at compile time but that can only happen when
all the warnings are fixed.
Cheers,
Nathan
Re: [PATCH 1/2] drm/amdgpu/mst: Stop ignoring error codes and deadlocking
On Wed, 2022-11-09 at 09:48 +, Lin, Wayne wrote:
> > }
> > - if (!drm_dp_mst_atomic_check(state) && !debugfs_overwrite) {
> > + ret = drm_dp_mst_atomic_check(state);
> > + if (ret == 0 && !debugfs_overwrite) {
> > set_dsc_configs_from_fairness_vars(params, vars, count,
> > k);
> > - return true;
> > + return 0;
> > + } else if (ret == -EDEADLK) {
> > + return ret;
>
> I think we should return here whenever there is an error. Not just for
> EDEADLK case.
Are we sure about this one? I think we may actually want to make this so it
returns on ret != -ENOSPC, since we want the function to continue if there's
no space in the atomic state available so it can try recomputing things with
compression enabled. On ret == 0 it should return early without doing
compression, and on ret == -ENOSPC it should just continue the function from
there
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
[linux-next:master] BUILD REGRESSION 5c92ddca1053df02387e8006d06094e18cc8538a
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 5c92ddca1053df02387e8006d06094e18cc8538a Add linux-next specific files for 20221114 Error/Warning reports: 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/202211130943.2q8u5ndp-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211142244.sgkxbwo2-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211150003.lkfys4he-...@intel.com Error/Warning: (recently discovered and may have been fixed) ERROR: modpost: "ipv6_icmp_error" [net/rxrpc/rxrpc.ko] undefined! arch/arm/mach-s3c/devs.c:32:10: fatal error: 'linux/platform_data/dma-s3c24xx.h' file not found arch/arm/mach-s3c/devs.c:32:10: fatal error: linux/platform_data/dma-s3c24xx.h: No such file or directory arch/arm/mach-s3c/s3c24xx.c:21:10: fatal error: 'linux/platform_data/dma-s3c24xx.h' file not found 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:4887: 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:5073:24: warning: implicit conversion from 'enum ' to 'enum dc_status' [-Wenum-conversion] 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/runl.c:34:1: warning: no previous prototype for '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/nvfw/acr.c:49:1: warning: no previous prototype for '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 net/rxrpc/local_object.c:36: undefined reference to `ipv6_icmp_error' vmlinux.o: warning: objtool: apply_dir_move+0xb1: unreachable instruction Unverified Error/Warning (likely false positive, please contact us if interested): drivers/cpufreq/acpi-cpufreq.c:970 acpi_cpufreq_boost_init() error: uninitialized symbol 'ret'. drivers/gpu/drm/nouveau/nvkm/engine/fifo/gf100.c:451:1: sparse: sparse: symbol 'gf100_fifo_nonstall_block' was not declared. Should it be static? drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c:33:18: sparse: sparse: symbol 'nvkm_engn_cgrp_get' was not declared. Should it be static? drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c:210:1: sparse: sparse: symbol 'tu102_gr_load' was not declared. Should it be static? drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:49:1: sparse: sparse: symbol 'wpr_generic_header_dump' was not declared. Should it be static? drivers/gpu/drm/nouveau/nvkm/subdev/gsp/ga102.c:49:1: sparse: sparse: symbol 'ga102_gsps' was not declared. Should it be static? drivers/gpu/drm/nouveau/nvkm/subdev/mc/ga100.c:51:1: sparse: sparse: symbol 'ga100_mc_device' was not declared. Should it be static? fotg210-udc.c:(.text+0x11b8): undefined reference to `usb_del_gadget_udc' fotg210-udc.c:(.text+0x3ba): undefined reference to `usb_gadget_giveback_request' fotg210-udc.c:(.text+0xb9e): undefined reference to `usb_gadget_udc_reset' lib/test_objpool.c:1007:16: sparse: sparse: symbol 'g_ot_async' was not declared. Should it be static? lib/test_objpool.c:516:3: sparse: sparse: symbol 'g_ot_sync_ops' was not declared. Should it be static? lib/test_objpool.c:76:3: sparse: sparse: symbol 'g_ot_data' was not declared. Should it be static? lib/test_objpool.c:824:3: sparse: sparse: symbol 'g_ot_async_ops' was not declared. Should it be static? lib/test_objpool.c:989:16: sparse: sparse: symbol 'g_ot_sync' was not declared. Should it be static? lib/test_objpool.c:998:16: sparse: sparse: symbol 'g_ot_miss' was not declared. Should it be static? 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' riscv32-linux-ld: fotg210-udc.c:(.text+0x13b6): undefined reference to `usb_ep_set_maxpacket_limit' riscv3
Re: [PATCH 1/1] drm/shmem: Dual licence the files as GPL-2 and MIT
On Sat, Nov 12, 2022 at 07:42:10PM +, Robert Swindells wrote: > Contributors to these files are: > > Noralf Trønnes > Liu Zixian > Dave Airlie > Thomas Zimmermann > Lucas De Marchi > Gerd Hoffmann > Rob Herring > Jakub Kicinski > Marcel Ziswiler > Stephen Rothwell > Daniel Vetter > Cai Huoqing > Neil Roberts > Marek Szyprowski > Emil Velikov > Sam Ravnborg Acked-by: Sam Ravnborg
Re: [PATCH 1/2] drm/amdgpu/mst: Stop ignoring error codes and deadlocking
On Wed, 2022-11-09 at 09:48 +, Lin, Wayne wrote:
> [Public]
>
> Thanks, Lyude!
> Comments inline.
>
> > -Original Message-
> > From: Lyude Paul
> > Sent: Saturday, November 5, 2022 7:59 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
> > ; Francis, David ; Mikita
> > Lipski ; Liu, Wenjing ;
> > open list:DRM DRIVERS ; open list > ker...@vger.kernel.org>
> > Subject: [PATCH 1/2] 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.
> >
> > 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 | 107 ++--
> > --
> > .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 12 +-
> > 3 files changed, 73 insertions(+), 64 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..6f76b2c84cdb5 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, _state, vars)) {
> > - ret = -EINVAL;
> > + ret = pre_validate_dsc(state, _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..30bc2e5058b70 100644
> > --- 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
> > @@ -864,25 +864,25 @@ static bool try_disable_dsc(struct
> > drm_atomic_state *state,
> > return true;
> > }
> >
> > -static bool compute_mst_dsc_configs_for_link(struct drm_atomic_state
> > *state,
> > -struct dc_state *dc_state,
> > -struct dc_link *dc_link,
> > -struct dsc_mst_fairness_vars *vars,
> > -struct drm_dp_mst_topology_mgr
> > *mgr,
> > -
Re: [PATCH v2] drm/msm/a6xx: Fix speed-bin detection vs probe-defer
Hi,
On Mon, Nov 14, 2022 at 12:50 PM Rob Clark wrote:
>
> From: Rob Clark
>
> If we get an error (other than -ENOENT) we need to propagate that up the
> stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up
> end up claiming that we support all the OPPs which is not likely to be
> true (and on some generations impossible to be true, ie. if there are
> conflicting OPPs).
>
> v2: Update commit msg, gc unused label, etc
>
> Fixed: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu")
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 7fe60c65a1eb..6ae77e88060f 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1941,7 +1941,7 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
> adreno_rev rev, u32 fuse)
>
> static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev)
> {
> - u32 supp_hw = UINT_MAX;
> + u32 supp_hw;
> u32 speedbin;
> int ret;
>
> @@ -1953,15 +1953,13 @@ static int a6xx_set_supported_hw(struct device *dev,
> struct adreno_rev rev)
> if (ret == -ENOENT) {
> return 0;
> } else if (ret) {
> - DRM_DEV_ERROR(dev,
> - "failed to read speed-bin (%d). Some OPPs may
> not be supported by hardware",
> - ret);
> - goto done;
> + dev_err_probe(dev, ret,
> + "failed to read speed-bin. Some OPPs may not be
> supported by hardware");
> + return ret;
Both before and after this change, I think you're missing a "\n" at
the end of your error string?
If you want to get even fancier, dev_err_probe is designed to run
"braceless" and returns "ret" as its return value. This you could do:
if (ret == -ENOENT)
return ret;
else if (ret)
return dev_err_probe(dev, ret, ...)
After adding the "\n" then either with the extra fanciness or as you have it:
Reviewed-by: Douglas Anderson
-Doug
Re: [PATCH] drm/i915: Update workaround documentation
On Mon, Nov 07, 2022 at 04:30:28PM -0800, Lucas De Marchi wrote: > There were several updates in the driver on how the workarounds are > handled since its documentation was written. Update the documentation to > reflect the current reality. > > Signed-off-by: Lucas De Marchi > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 87 + > 1 file changed, 56 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 3cdf5c24dbc5..0db3713c1beb 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -17,43 +17,68 @@ > /** > * DOC: Hardware workarounds > * > - * This file is intended as a central place to implement most [1]_ of the > - * required workarounds for hardware to work as originally intended. They > fall > - * in five basic categories depending on how/when they are applied: > + * This is intended as a central place to implement most [1]_ of the Your footnotes don't hook up properly anymore. The original code had [1] and [2], but the new code hooks [1] to what used to be [2]. Since we moved this file under gt/ a while back, I wonder if we should note somehow that sgunit/soc workarounds and display workarounds aren't expected to be part of this file? > + * required workarounds for hardware to work as originally intended. Hardware > + * workarounds are register programming documented to be executed in the > driver > + * that fall outside of the normal programming sequences for a platform. > There > + * are some basic categories of workarounds, depending on how/when they are > + * applied: > * > - * - Workarounds that touch registers that are saved/restored to/from the HW > - * context image. The list is emitted (via Load Register Immediate > commands) > - * everytime a new context is created. > - * - GT workarounds. The list of these WAs is applied whenever these > registers > - * revert to default values (on GPU reset, suspend/resume [2]_, etc..). > - * - Display workarounds. The list is applied during display clock-gating > - * initialization. > - * - Workarounds that whitelist a privileged register, so that UMDs can > manage > - * them directly. This is just a special case of a MMMIO workaround (as we > - * write the list of these to/be-whitelisted registers to some special HW > - * registers). > - * - Workaround batchbuffers, that get executed automatically by the hardware > - * on every HW context restore. > + * - Context workarounds: workarounds that touch registers that are > + * saved/restored to/from the HW context image. The list is emitted (via > Load > + * Register Immediate commands) once when initializing the device and > saved in > + * the default context. That default context is then used on every context > + * creation to have a "primed golden context", i.e. a context image that > + * already contains the changes needed to all the registers. > * > - * .. [1] Please notice that there are other WAs that, due to their nature, > - *cannot be applied from a central place. Those are peppered around the > rest > - *of the code, as needed. > + * - Engine workarounds: the list of these WAs is applied whenever the > specific > + * engine is reset. It's also possible that a set of engine classes share a > + * common power domain and they are reset together. This happens on some > + * platforms with render and compute engines. In this case (at least) one > of > + * them need to keeep the workaround programming: the approach taken in the > + * driver is to tie those workarounds to the first compute/render engine > that > + * is registered. When executing with GuC submission, engine resets are > + * outside of kernel driver control, hence the list of registers involved > in > + * written once, on engine initialization, and then passed to GuC, that > + * saves/restores their values before/after the reset takes place. See > + * ``drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c`` for reference. > * > - * .. [2] Technically, some registers are powercontext saved & restored, so > they > - *survive a suspend/resume. In practice, writing them again is not too > - *costly and simplifies things. We can revisit this in the future. > + * - GT workarounds: the list of these WAs is applied whenever these > registers > + * revert to their default values: on GPU reset, suspend/resume, etc. This is where the new [1] used to be referenced from. > * > - * Layout > - * ~~ > + * - Register whitelist: some workarounds need to be implemented in > userspace, > + * but need to touch privileged registers. The whitelist in the kernel > + * instructs the hardware to allow the access to happen. From the kernel > side, > + * this is just a special case of a MMIO workaround (as we write the list > of > + * these to/be-whitelisted registers to some
[PATCH v2] drm/msm/a6xx: Fix speed-bin detection vs probe-defer
From: Rob Clark
If we get an error (other than -ENOENT) we need to propagate that up the
stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up
end up claiming that we support all the OPPs which is not likely to be
true (and on some generations impossible to be true, ie. if there are
conflicting OPPs).
v2: Update commit msg, gc unused label, etc
Fixed: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 7fe60c65a1eb..6ae77e88060f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1941,7 +1941,7 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
adreno_rev rev, u32 fuse)
static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev)
{
- u32 supp_hw = UINT_MAX;
+ u32 supp_hw;
u32 speedbin;
int ret;
@@ -1953,15 +1953,13 @@ static int a6xx_set_supported_hw(struct device *dev,
struct adreno_rev rev)
if (ret == -ENOENT) {
return 0;
} else if (ret) {
- DRM_DEV_ERROR(dev,
- "failed to read speed-bin (%d). Some OPPs may not
be supported by hardware",
- ret);
- goto done;
+ dev_err_probe(dev, ret,
+ "failed to read speed-bin. Some OPPs may not be
supported by hardware");
+ return ret;
}
supp_hw = fuse_to_supp_hw(dev, rev, speedbin);
-done:
ret = devm_pm_opp_set_supported_hw(dev, _hw, 1);
if (ret)
return ret;
--
2.38.1
Re: [PATCH] drm/msm/a6xx: Fix speed-bin detection vs probe-defer
On Mon, Nov 14, 2022 at 11:59 AM Akhil P Oommen
wrote:
>
> On 11/15/2022 1:11 AM, Rob Clark wrote:
> > From: Rob Clark
> >
> > If we get an error (other than -ENOENT) we need to propagate that up the
> > stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with
> > whatever OPP(s) are represented by bit zero.
> >
> > Fixed: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu")
> > Signed-off-by: Rob Clark
> > ---
> > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > index 7fe60c65a1eb..96de2202c86c 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > @@ -1956,7 +1956,7 @@ static int a6xx_set_supported_hw(struct device *dev,
> > struct adreno_rev rev)
> > DRM_DEV_ERROR(dev,
> > "failed to read speed-bin (%d). Some OPPs may
> > not be supported by hardware",
> I just noticed and was going to send a similar fix. We should remove ".
> Some OPPs may not be supported by hardware" here.
>
> Reviewed-by: Akhil P Oommen
>
> Btw, on msm-next-external-fixes + this fix, I still see boot up issue
> in herobrine due to drm_dev_alloc() failure with -ENOSPC error.
Could you track it down one level deeper? I wonder if there is some
missing cleanup in the probe-defer path and we end up failing in
drm_minor_alloc() or something along those lines
BR,
-R
> -Akhil.
> > ret);
> > - goto done;
> > + return ret;
> > }
> >
> > supp_hw = fuse_to_supp_hw(dev, rev, speedbin);
>
Re: [PATCH] drm/msm/a6xx: Fix speed-bin detection vs probe-defer
On Mon, Nov 14, 2022 at 12:27 PM Doug Anderson wrote: > > Hi, > > On Mon, Nov 14, 2022 at 11:41 AM Rob Clark wrote: > > > > From: Rob Clark > > > > If we get an error (other than -ENOENT) we need to propagate that up the > > stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with > > whatever OPP(s) are represented by bit zero. > > Can you explain the "whatever OPP(s) are represented by bit zero" > part? This doesn't seem to be true because `supp_hw` is initiated to > UINT_MAX. If I'm remembering how this all works, doesn't that mean > that if we get an error we'll assume all OPPs are OK? Oh, that's right.. and even worse! Ok, stand by for v2 > I'm not saying that I'm against your change, but I think maybe you're > misdescribing the old behavior. > > Speaking of the initialization of supp_hw, if we want to change the > behavior like your patch does then we should be able to remove that > initialization, right? > > I would also suspect that your patch will result in a compiler > warning, at least on some compilers. The goto label `done` is no > longer needed, right? > > -Doug
Re: [PATCH] drm/msm/a6xx: Fix speed-bin detection vs probe-defer
On 11/15/2022 1:57 AM, Doug Anderson wrote: Hi, On Mon, Nov 14, 2022 at 11:41 AM Rob Clark wrote: From: Rob Clark If we get an error (other than -ENOENT) we need to propagate that up the stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with whatever OPP(s) are represented by bit zero. Can you explain the "whatever OPP(s) are represented by bit zero" part? This doesn't seem to be true because `supp_hw` is initiated to UINT_MAX. If I'm remembering how this all works, doesn't that mean that if we get an error we'll assume all OPPs are OK? I'm not saying that I'm against your change, but I think maybe you're misdescribing the old behavior. Speaking of the initialization of supp_hw, if we want to change the behavior like your patch does then we should be able to remove that initialization, right? I would also suspect that your patch will result in a compiler warning, at least on some compilers. The goto label `done` is no longer needed, right? -Doug You are right about the commit message. The problem is we can't enable all bits in supp_hw anymore due to changes like this: https://patchwork.kernel.org/project/linux-arm-msm/patch/20220829011035.1.Ie3564662150e038571b7e2779cac7229191cf3bf@changeid/ This creates 2 opps with same freq when supp_hw = UINT_MAX. -Akhil.
Re: [PATCH] drm/msm/a6xx: Fix speed-bin detection vs probe-defer
Hi, On Mon, Nov 14, 2022 at 11:41 AM Rob Clark wrote: > > From: Rob Clark > > If we get an error (other than -ENOENT) we need to propagate that up the > stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with > whatever OPP(s) are represented by bit zero. Can you explain the "whatever OPP(s) are represented by bit zero" part? This doesn't seem to be true because `supp_hw` is initiated to UINT_MAX. If I'm remembering how this all works, doesn't that mean that if we get an error we'll assume all OPPs are OK? I'm not saying that I'm against your change, but I think maybe you're misdescribing the old behavior. Speaking of the initialization of supp_hw, if we want to change the behavior like your patch does then we should be able to remove that initialization, right? I would also suspect that your patch will result in a compiler warning, at least on some compilers. The goto label `done` is no longer needed, right? -Doug
Re: [PATCH] drm/msm/a6xx: Fix speed-bin detection vs probe-defer
On 11/15/2022 1:11 AM, Rob Clark wrote:
From: Rob Clark
If we get an error (other than -ENOENT) we need to propagate that up the
stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with
whatever OPP(s) are represented by bit zero.
Fixed: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 7fe60c65a1eb..96de2202c86c 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1956,7 +1956,7 @@ static int a6xx_set_supported_hw(struct device *dev,
struct adreno_rev rev)
DRM_DEV_ERROR(dev,
"failed to read speed-bin (%d). Some OPPs may not be
supported by hardware",
I just noticed and was going to send a similar fix. We should remove ".
Some OPPs may not be supported by hardware" here.
Reviewed-by: Akhil P Oommen
Btw, on msm-next-external-fixes + this fix, I still see boot up issue
in herobrine due to drm_dev_alloc() failure with -ENOSPC error.
-Akhil.
ret);
- goto done;
+ return ret;
}
supp_hw = fuse_to_supp_hw(dev, rev, speedbin);
Re: [PATCH printk v4 31/39] printk, xen: fbfront: create/use safe function for forcing preferred
Hi,
After more detailed runtime testing I discovered that I didn't re-insert
the console to the correct place in the list. More below...
On 2022-11-14, John Ogness wrote:
> 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 e770b1ede6c9..dff76c1cef80 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3461,6 +3462,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(>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(_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_behind_rcu(>node, console_list.first);
This is adding the console as the 2nd item. It should be the new
head. The patch below fixes it.
I have done careful runtime testing with this fixup. After the
force_preferred, the console is the new head and sending data to
/dev/console redirects to that console.
It would be nice if we could fold this in. Sorry.
John Ogness
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8d635467882f..4b77586cf4cb 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3494,7 +3494,7 @@ void console_force_preferred_locked(struct console *con)
/* Only the new head can have CON_CONSDEV set. */
console_srcu_write_flags(cur_pref_con, cur_pref_con->flags &
~CON_CONSDEV);
- hlist_add_behind_rcu(>node, console_list.first);
+ hlist_add_head_rcu(>node, _list);
}
EXPORT_SYMBOL(console_force_preferred_locked);
[PATCH] drm/msm/a6xx: Fix speed-bin detection vs probe-defer
From: Rob Clark
If we get an error (other than -ENOENT) we need to propagate that up the
stack. Otherwise if the nvmem driver hasn't probed yet, we'll end up with
whatever OPP(s) are represented by bit zero.
Fixed: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 7fe60c65a1eb..96de2202c86c 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1956,7 +1956,7 @@ static int a6xx_set_supported_hw(struct device *dev,
struct adreno_rev rev)
DRM_DEV_ERROR(dev,
"failed to read speed-bin (%d). Some OPPs may not
be supported by hardware",
ret);
- goto done;
+ return ret;
}
supp_hw = fuse_to_supp_hw(dev, rev, speedbin);
--
2.38.1
[PATCH v4 1/2] drm/msm/adreno: Simplify read64/write64 helpers
From: Rob Clark
The _HI reg is always following the _LO reg, so no need to pass these
offsets seprately.
Signed-off-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Akhil P Oommen
---
drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 3 +--
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 27 -
drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 4 +--
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 24 ++
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 3 +--
drivers/gpu/drm/msm/msm_gpu.h | 12 -
6 files changed, 27 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index 7cb8d9849c07..a10feb8a4194 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -606,8 +606,7 @@ static int a4xx_pm_suspend(struct msm_gpu *gpu) {
static int a4xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
{
- *value = gpu_read64(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO,
- REG_A4XX_RBBM_PERFCTR_CP_0_HI);
+ *value = gpu_read64(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO);
return 0;
}
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 3dcec7acb384..ba22d3c918bc 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -605,11 +605,9 @@ static int a5xx_ucode_init(struct msm_gpu *gpu)
a5xx_ucode_check_version(a5xx_gpu, a5xx_gpu->pfp_bo);
}
- gpu_write64(gpu, REG_A5XX_CP_ME_INSTR_BASE_LO,
- REG_A5XX_CP_ME_INSTR_BASE_HI, a5xx_gpu->pm4_iova);
+ gpu_write64(gpu, REG_A5XX_CP_ME_INSTR_BASE_LO, a5xx_gpu->pm4_iova);
- gpu_write64(gpu, REG_A5XX_CP_PFP_INSTR_BASE_LO,
- REG_A5XX_CP_PFP_INSTR_BASE_HI, a5xx_gpu->pfp_iova);
+ gpu_write64(gpu, REG_A5XX_CP_PFP_INSTR_BASE_LO, a5xx_gpu->pfp_iova);
return 0;
}
@@ -868,8 +866,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
* memory rendering at this point in time and we don't want to block off
* part of the virtual memory space.
*/
- gpu_write64(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_LO,
- REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_HI, 0x);
+ gpu_write64(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_LO, 0x);
gpu_write(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_SIZE, 0x);
/* Put the GPU into 64 bit by default */
@@ -908,8 +905,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
return ret;
/* Set the ringbuffer address */
- gpu_write64(gpu, REG_A5XX_CP_RB_BASE, REG_A5XX_CP_RB_BASE_HI,
- gpu->rb[0]->iova);
+ gpu_write64(gpu, REG_A5XX_CP_RB_BASE, gpu->rb[0]->iova);
/*
* If the microcode supports the WHERE_AM_I opcode then we can use that
@@ -936,7 +932,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
}
gpu_write64(gpu, REG_A5XX_CP_RB_RPTR_ADDR,
- REG_A5XX_CP_RB_RPTR_ADDR_HI, shadowptr(a5xx_gpu,
gpu->rb[0]));
+ shadowptr(a5xx_gpu, gpu->rb[0]));
} else if (gpu->nr_rings > 1) {
/* Disable preemption if WHERE_AM_I isn't available */
a5xx_preempt_fini(gpu);
@@ -1239,9 +1235,9 @@ static void a5xx_fault_detect_irq(struct msm_gpu *gpu)
gpu_read(gpu, REG_A5XX_RBBM_STATUS),
gpu_read(gpu, REG_A5XX_CP_RB_RPTR),
gpu_read(gpu, REG_A5XX_CP_RB_WPTR),
- gpu_read64(gpu, REG_A5XX_CP_IB1_BASE, REG_A5XX_CP_IB1_BASE_HI),
+ gpu_read64(gpu, REG_A5XX_CP_IB1_BASE),
gpu_read(gpu, REG_A5XX_CP_IB1_BUFSZ),
- gpu_read64(gpu, REG_A5XX_CP_IB2_BASE, REG_A5XX_CP_IB2_BASE_HI),
+ gpu_read64(gpu, REG_A5XX_CP_IB2_BASE),
gpu_read(gpu, REG_A5XX_CP_IB2_BUFSZ));
/* Turn off the hangcheck timer to keep it from bothering us */
@@ -1427,8 +1423,7 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu)
static int a5xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
{
- *value = gpu_read64(gpu, REG_A5XX_RBBM_ALWAYSON_COUNTER_LO,
- REG_A5XX_RBBM_ALWAYSON_COUNTER_HI);
+ *value = gpu_read64(gpu, REG_A5XX_RBBM_ALWAYSON_COUNTER_LO);
return 0;
}
@@ -1465,8 +1460,7 @@ static int a5xx_crashdumper_run(struct msm_gpu *gpu,
if (IS_ERR_OR_NULL(dumper->ptr))
return -EINVAL;
- gpu_write64(gpu, REG_A5XX_CP_CRASH_SCRIPT_BASE_LO,
- REG_A5XX_CP_CRASH_SCRIPT_BASE_HI, dumper->iova);
+ gpu_write64(gpu, REG_A5XX_CP_CRASH_SCRIPT_BASE_LO, dumper->iova);
gpu_write(gpu, REG_A5XX_CP_CRASH_DUMP_CNTL, 1);
@@ -1666,8 +1660,7 @@ static u64 a5xx_gpu_busy(struct msm_gpu *gpu, unsigned
long *out_sample_rate)
{
u64 busy_cycles;
- busy_cycles = gpu_read64(gpu,
[PATCH v4 2/2] drm/msm: Hangcheck progress detection
From: Rob Clark
If the hangcheck timer expires, check if the fw's position in the
cmdstream has advanced (changed) since last timer expiration, and
allow it up to three additional "extensions" to it's alotted time.
The intention is to continue to catch "shader stuck in a loop" type
hangs quickly, but allow more time for things that are actually
making forward progress.
Because we need to sample the CP state twice to detect if there has
not been progress, this also cuts the the timer's duration in half.
v2: Fix typo (REG_A6XX_CP_CSQ_IB2_STAT), add comment
v3: Only halve hangcheck timer duration for generations which
support progress detection (hdanton); removed unused a5xx
progress (without knowing how to adjust for data buffered
in ROQ it is too likely to report a false negative)
v4: Comment updates to better describe the total hangcheck
duration when progress detection is applied
Reviewed-by: Chia-I Wu
Tested-by: Chia-I Wu #
dEQP-GLES2.functional.flush_finish.wait
Signed-off-by: Rob Clark
Reviewed-by: Akhil P Oommen
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 34 +++
drivers/gpu/drm/msm/msm_drv.c | 1 -
drivers/gpu/drm/msm/msm_drv.h | 8 ++-
drivers/gpu/drm/msm/msm_gpu.c | 31 +++-
drivers/gpu/drm/msm/msm_gpu.h | 10
drivers/gpu/drm/msm/msm_ringbuffer.h | 28 ++
6 files changed, 109 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 1ff605c18ee6..7fe60c65a1eb 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1843,6 +1843,39 @@ static uint32_t a6xx_get_rptr(struct msm_gpu *gpu,
struct msm_ringbuffer *ring)
return ring->memptrs->rptr = gpu_read(gpu, REG_A6XX_CP_RB_RPTR);
}
+static bool a6xx_progress(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
+{
+ struct msm_cp_state cp_state = {
+ .ib1_base = gpu_read64(gpu, REG_A6XX_CP_IB1_BASE),
+ .ib2_base = gpu_read64(gpu, REG_A6XX_CP_IB2_BASE),
+ .ib1_rem = gpu_read(gpu, REG_A6XX_CP_IB1_REM_SIZE),
+ .ib2_rem = gpu_read(gpu, REG_A6XX_CP_IB2_REM_SIZE),
+ };
+ bool progress;
+
+ /*
+* Adjust the remaining data to account for what has already been
+* fetched from memory, but not yet consumed by the SQE.
+*
+* This is not *technically* correct, the amount buffered could
+* exceed the IB size due to hw prefetching ahead, but:
+*
+* (1) We aren't trying to find the exact position, just whether
+* progress has been made
+* (2) The CP_REG_TO_MEM at the end of a submit should be enough
+* to prevent prefetching into an unrelated submit. (And
+* either way, at some point the ROQ will be full.)
+*/
+ cp_state.ib1_rem += gpu_read(gpu, REG_A6XX_CP_CSQ_IB1_STAT) >> 16;
+ cp_state.ib2_rem += gpu_read(gpu, REG_A6XX_CP_CSQ_IB2_STAT) >> 16;
+
+ progress = !!memcmp(_state, >last_cp_state, sizeof(cp_state));
+
+ ring->last_cp_state = cp_state;
+
+ return progress;
+}
+
static u32 a618_get_speed_bin(u32 fuse)
{
if (fuse == 0)
@@ -1961,6 +1994,7 @@ static const struct adreno_gpu_funcs funcs = {
.create_address_space = a6xx_create_address_space,
.create_private_address_space =
a6xx_create_private_address_space,
.get_rptr = a6xx_get_rptr,
+ .progress = a6xx_progress,
},
.get_timestamp = a6xx_get_timestamp,
};
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 670651cdfa79..c3b77b44b2aa 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -419,7 +419,6 @@ static int msm_drm_init(struct device *dev, const struct
drm_driver *drv)
priv->dev = ddev;
priv->wq = alloc_ordered_workqueue("msm", 0);
- priv->hangcheck_period = DRM_MSM_HANGCHECK_DEFAULT_PERIOD;
INIT_LIST_HEAD(>objects);
mutex_init(>obj_lock);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 0609daf4fa4c..876d8d5eec2f 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -225,7 +225,13 @@ struct msm_drm_private {
struct drm_atomic_state *pm_state;
- /* For hang detection, in ms */
+ /**
+* hangcheck_period: For hang detection, in ms
+*
+* Note that in practice, a submit/job will get at least two hangcheck
+* periods, due to checking for progress being implemented as simply
+* "have the CP position registers changed since last time?"
+*/
unsigned int hangcheck_period;
/**
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 3dffee54a951..bfef659d3a5c 100644
---
[PATCH v4 0/2] drm/msm: Improved hang detection
From: Rob Clark Try to detect when submit jobs are making forward progress and give them a bit more time. Rob Clark (2): drm/msm/adreno: Simplify read64/write64 helpers drm/msm: Hangcheck progress detection drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 3 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 27 -- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 4 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 58 +++-- drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 3 +- drivers/gpu/drm/msm/msm_drv.c | 1 - drivers/gpu/drm/msm/msm_drv.h | 8 ++- drivers/gpu/drm/msm/msm_gpu.c | 31 ++- drivers/gpu/drm/msm/msm_gpu.h | 22 +--- drivers/gpu/drm/msm/msm_ringbuffer.h| 28 ++ 10 files changed, 136 insertions(+), 49 deletions(-) -- 2.38.1
Re: [Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree
On Mon, Nov 14, 2022 at 01:02:46PM +0200, Jani Nikula wrote:
> On Mon, 14 Nov 2022, Hans de Goede wrote:
> > Hi,
> >
> > On 11/14/22 11:10, Jani Nikula wrote:
> >> On Mon, 14 Nov 2022, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 11/14/22 00:23, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the drm-intel tree got a conflict in:
>
> drivers/gpu/drm/i915/display/intel_backlight.c
>
> between commit:
>
> b1d36e73cc1c ("drm/i915: Don't register backlight when another
> backlight should be used (v2)")
>
> from Linus' tree and commit:
>
> 801543b2593b ("drm/i915: stop including i915_irq.h from i915_trace.h")
>
> from the drm-intel tree.
> >>>
> >>> This is weird, because the:
> >>>
> >>>b1d36e73cc1c ("drm/i915: Don't register backlight when another
> >>> backlight should be used (v2)")
> >>>
> >>> commit is in 6.1-rc1, so there can only be a conflict it 6.1-rc1 has not
> >>> been back-merged into drm-intel yet ?
> >>
> >> That's the reason it *is* a conflict, right?
> >
> > Right what I was trying to say is that I am surprised that 6.1-rc1 has not
> > been back-merged into drm-intel yet even though it has been released
> > 4 weeks ago.
>
> Right, -ENOCOFFEE at my end.
>
> > I thought it was more or less standard process to backmerge rc1 soon after
> > it is released ?
>
> The delay may be because v6.1-rc1 brought in more regressions for us
> than any other -rc1 in recent memory. Our CI's been suffering, and our
> folks have been spending a lot of time debugging, bisecting and
> reporting. (And before you ask, yes, we're going to be more proactive in
> reporting issues we find in linux-next.)
>
> That said, Rodrigo's been in charge of drm-intel-next this cycle, maybe
> it's time to backmerge drm-next?
yeap, I'm on it...
>
>
> BR,
> Jani.
>
> --
> Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On Mon, Nov 14, 2022 at 10:21:50AM -0500, Matthew Rosato wrote: > On 11/14/22 9:59 AM, Jason Gunthorpe wrote: > > On Mon, Nov 14, 2022 at 09:55:21AM -0500, Matthew Rosato wrote: > AFAICT there is no equivalent means to specify > vfio_iommu_type1.dma_entry_limit when using iommufd; looks like > we'll just always get the default 65535. > >>> > >>> No, there is no arbitary limit on iommufd > >> > >> Yeah, that's what I suspected. But FWIW, userspace checks the > >> advertised limit via VFIO_IOMMU_GET_INFO / > >> VFIO_IOMMU_TYPE1_INFO_DMA_AVAIL, and this is still being advertised > >> as 65535 when using iommufd. I don't think there is a defined way > >> to return 'ignore this value'. > > > > Is something using this? Should we make it much bigger? > > Yes, s390 when doing lazy unmapping likes to use larger amounts of > concurrent DMA, so there can be cases where we want to raise this > limit. > > The initial value of 65535 is already pretty arbitrary (U16_MAX) -- It was choosen to match VFIO's default > If iommufd is doing its own management and this value becomes > deprecated in this scenario, and we can't set it to a magic value > that says 'ignore me' then maybe it just makes sense for now to set > it arbitrarily larger when using iommufd e.g. U32_MAX? Sure /* * iommufd's limit is based on the cgroup's memory limit. * Normally vfio would return U16_MAX here, and provide a module * parameter to adjust it. Since S390 qemu userspace actually * pays attention and needs a value bigger than U16_MAX return * U32_MAX. */ .avail = U32_MAX, Thanks, Jason
Re: [PATCH v2 2/4] usb: gadget: hid: Convert to use list_count()
On Mon, Nov 14, 2022 at 1:22 PM Andy Shevchenko wrote: > > The list API now provides the list_count() to help with counting > existing nodes in the list. Uilise it. s/Uilise/Utilise
Re: [PATCH 1/1] drm/shmem: Dual licence the files as GPL-2 and MIT
On Sat, Nov 12, 2022 at 1:44 PM Robert Swindells wrote: > > Contributors to these files are: > > Noralf Trønnes > Liu Zixian > Dave Airlie > Thomas Zimmermann > Lucas De Marchi > Gerd Hoffmann > Rob Herring My contributions are related to the madvise functions. That's largely lifted or inspired from the MSM code which is GPL only. That in turn looks inspired from i915 which is MIT (though not much more than the comment): $ git grep 'Our goal here is to return as much of the memory' drivers/gpu/drm/drm_gem_shmem_helper.c: /* Our goal here is to return as much of the memory as drivers/gpu/drm/i915/gem/i915_gem_shmem.c: * Our goal here is to return as much of the memory as drivers/gpu/drm/msm/msm_gem.c: /* Our goal here is to return as much of the memory as I imagine this is not the only example in this file. In fact, looking at the introduction of this file, it looks like it originated from V3D code as that was the first driver to convert over. V3D is licensed GPL2+. Of course, its code was not written in a vacuum either and came from ??? This to me is a problem with the dual licensing in DRM drivers. Code moves around with little attention paid at the time to licensing. I wouldn't trust anything claiming MIT license is not GPL 'contaminated'. OTOH, there's really only one way for the madvise code to work, so maybe not a copyrightable work on its own. Rob
Re: [PATCH] video: fbdev: vermilion: decrease reference count in error path
On 11/14/22 09:56, Xiongfeng Wang wrote:
pci_get_device() will increase the reference count for the returned
pci_dev. For the error path, we need to use pci_dev_put() to decrease
the reference count.
Fixes: dbe7e429fedb ("vmlfb: framebuffer driver for Intel Vermilion Range")
Signed-off-by: Xiongfeng Wang
applied.
Thanks!
Helge
---
drivers/video/fbdev/vermilion/vermilion.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/vermilion/vermilion.c
b/drivers/video/fbdev/vermilion/vermilion.c
index 82b36dbb5b1a..33051e3a2561 100644
--- a/drivers/video/fbdev/vermilion/vermilion.c
+++ b/drivers/video/fbdev/vermilion/vermilion.c
@@ -278,8 +278,10 @@ static int vmlfb_get_gpu(struct vml_par *par)
mutex_unlock(_mutex);
- if (pci_enable_device(par->gpu) < 0)
+ if (pci_enable_device(par->gpu) < 0) {
+ pci_dev_put(par->gpu);
return -ENODEV;
+ }
return 0;
}
Re: [PATCH v2 1/4] i915: Move list_count() to list.h for broader use
On Mon, Nov 14, 2022 at 06:11:51PM +, Ruhl, Michael J wrote: ... > So all of the non-list_for_each code appears to be an inline. This is not true. > This which, resembles the non-list_for_each pattern is a macro? > > Just curious as to why the macro rather than inline? See above. However, I'm fine with the inline. -- With Best Regards, Andy Shevchenko
RE: [PATCH v2 1/4] i915: Move list_count() to list.h for broader use
>-Original Message-
>From: dri-devel On Behalf Of
>Andy Shevchenko
>Sent: Monday, November 14, 2022 11:22 AM
>To: Jakob Koschel ; Andy Shevchenko
>; Greg Kroah-Hartman
>; Mathias Nyman
>; intel-...@lists.freedesktop.org; dri-
>de...@lists.freedesktop.org; linux-ker...@vger.kernel.org; linux-
>u...@vger.kernel.org
>Cc: Tvrtko Ursulin ; Kevin Cernekee
>; Nyman, Mathias ; Vivi,
>Rodrigo ; Andrew Morton foundation.org>
>Subject: [PATCH v2 1/4] i915: Move list_count() to list.h for broader use
>
>Some of the existing users, and definitely will be new ones, want to
>count existing nodes in the list. Provide a generic API for that by
>moving code from i915 to list.h.
>
>Signed-off-by: Andy Shevchenko
>---
>v2: dropped the duplicate code in i915 (LKP)
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 13 +
> include/linux/list.h | 13 +
> 2 files changed, 14 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>index 6ae8b07cfaa1..b5d474be564d 100644
>--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>@@ -2085,17 +2085,6 @@ static void print_request_ring(struct drm_printer
>*m, struct i915_request *rq)
> }
> }
>
>-static unsigned long list_count(struct list_head *list)
>-{
>- struct list_head *pos;
>- unsigned long count = 0;
>-
>- list_for_each(pos, list)
>- count++;
>-
>- return count;
>-}
>-
> static unsigned long read_ul(void *p, size_t x)
> {
> return *(unsigned long *)(p + x);
>@@ -2270,7 +2259,7 @@ void intel_engine_dump(struct intel_engine_cs
>*engine,
> spin_lock_irqsave(>sched_engine->lock, flags);
> engine_dump_active_requests(engine, m);
>
>- drm_printf(m, "\tOn hold?: %lu\n",
>+ drm_printf(m, "\tOn hold?: %zu\n",
> list_count(>sched_engine->hold));
> spin_unlock_irqrestore(>sched_engine->lock, flags);
>
>diff --git a/include/linux/list.h b/include/linux/list.h
>index 61762054b4be..098eccf8c1b6 100644
>--- a/include/linux/list.h
>+++ b/include/linux/list.h
>@@ -655,6 +655,19 @@ static inline void list_splice_tail_init(struct list_head
>*list,
>!list_is_head(pos, (head)); \
>pos = n, n = pos->prev)
>
>+/**
>+ * list_count - count nodes in the list
>+ * @head: the head for your list.
>+ */
>+#define list_count(head) \
>+({\
>+ struct list_head *__tmp;\
>+ size_t __i = 0; \
>+ list_for_each(__tmp, head) \
>+ __i++; \
>+ __i;\
>+})
So all of the non-list_for_each code appears to be an inline.
This which, resembles the non-list_for_each pattern is a macro?
Just curious as to why the macro rather than inline?
Mike
+
> /**
> * list_entry_is_head - test if the entry points to the head of the list
> * @pos: the type * to cursor
>--
>2.35.1
Re: [PATCH 1/1] drm/shmem: Dual licence the files as GPL-2 and MIT
On Sat, 12 Nov 2022 19:42:10 + Robert Swindells wrote: > Contributors to these files are: > > Noralf Trønnes > Liu Zixian > Dave Airlie > Thomas Zimmermann > Lucas De Marchi > Gerd Hoffmann > Rob Herring > Jakub Kicinski > Marcel Ziswiler > Stephen Rothwell > Daniel Vetter > Cai Huoqing > Neil Roberts > Marek Szyprowski > Emil Velikov > Sam Ravnborg > Boris Brezillon Acked-by: Boris Brezillon > Dan Carpenter > > Signed-off-by: Robert Swindells > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- > include/drm/drm_gem_shmem_helper.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 35138f8a375c..f1a68a71f876 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -1,4 +1,4 @@ > -// SPDX-License-Identifier: GPL-2.0 > +// SPDX-License-Identifier: GPL-2.0 or MIT > /* > * Copyright 2018 Noralf Trønnes > */ > diff --git a/include/drm/drm_gem_shmem_helper.h > b/include/drm/drm_gem_shmem_helper.h > index a2201b2488c5..56ac32947d1c 100644 > --- a/include/drm/drm_gem_shmem_helper.h > +++ b/include/drm/drm_gem_shmem_helper.h > @@ -1,4 +1,4 @@ > -/* SPDX-License-Identifier: GPL-2.0 */ > +/* SPDX-License-Identifier: GPL-2.0 or MIT */ > > #ifndef __DRM_GEM_SHMEM_HELPER_H__ > #define __DRM_GEM_SHMEM_HELPER_H__
Re: [PATCH 1/1] drm/shmem: Dual licence the files as GPL-2 and MIT
Den 12.11.2022 20.42, skrev Robert Swindells: > Contributors to these files are: > > Noralf Trønnes Acked-by: Noralf Trønnes > Liu Zixian > Dave Airlie > Thomas Zimmermann > Lucas De Marchi > Gerd Hoffmann > Rob Herring > Jakub Kicinski > Marcel Ziswiler > Stephen Rothwell > Daniel Vetter > Cai Huoqing > Neil Roberts > Marek Szyprowski > Emil Velikov > Sam Ravnborg > Boris Brezillon > Dan Carpenter > > Signed-off-by: Robert Swindells > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- > include/drm/drm_gem_shmem_helper.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 35138f8a375c..f1a68a71f876 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -1,4 +1,4 @@ > -// SPDX-License-Identifier: GPL-2.0 > +// SPDX-License-Identifier: GPL-2.0 or MIT > /* > * Copyright 2018 Noralf Trønnes > */ > diff --git a/include/drm/drm_gem_shmem_helper.h > b/include/drm/drm_gem_shmem_helper.h > index a2201b2488c5..56ac32947d1c 100644 > --- a/include/drm/drm_gem_shmem_helper.h > +++ b/include/drm/drm_gem_shmem_helper.h > @@ -1,4 +1,4 @@ > -/* SPDX-License-Identifier: GPL-2.0 */ > +/* SPDX-License-Identifier: GPL-2.0 or MIT */ > > #ifndef __DRM_GEM_SHMEM_HELPER_H__ > #define __DRM_GEM_SHMEM_HELPER_H__
Re: [PATCH v9 01/25] docs/fb: Document current named modes
Den 14.11.2022 14.00, skrev Maxime Ripard: > KMS supports a number of named modes already, but it's never been > documented anywhere, let's fix that. > > Signed-off-by: Maxime Ripard > > --- Reviewed-by: Noralf Trønnes
Re: [PATCH] mm/memory: Return vm_fault_t result from migrate_to_ram() callback
Am 2022-11-14 um 11:41 schrieb David Hildenbrand:
On 14.11.22 12:55, Alistair Popple wrote:
The migrate_to_ram() callback should always succeed, but in rare cases
can fail usually returning VM_FAULT_SIGBUS. Commit 16ce101db85d
("mm/memory.c: fix race when faulting a device private page")
incorrectly stopped passing the return code up the stack. Fix this by
setting the ret variable, restoring the previous behaviour on
migrate_to_ram() failure.
Fixes: 16ce101db85d ("mm/memory.c: fix race when faulting a device
private page")
Signed-off-by: Alistair Popple
Cc: Ralph Campbell
Cc: John Hubbard
Cc: Alex Sierra
Cc: Ben Skeggs
Cc: Felix Kuehling
Cc: Lyude Paul
Cc: Jason Gunthorpe
Cc: Michael Ellerman
Reviewed-by: Felix Kuehling
---
Hi Andrew, I noticed my series needs another small fix which I'm
hoping you can apply for v6.1-rcX. Thanks (and sorry for not catching
this sooner).
---
mm/memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/memory.c b/mm/memory.c
index f88c351aecd4..8a6d5c823f91 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3763,7 +3763,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
*/
get_page(vmf->page);
pte_unmap_unlock(vmf->pte, vmf->ptl);
- vmf->page->pgmap->ops->migrate_to_ram(vmf);
+ ret = vmf->page->pgmap->ops->migrate_to_ram(vmf);
put_page(vmf->page);
} else if (is_hwpoison_entry(entry)) {
ret = VM_FAULT_HWPOISON;
Acked-by: David Hildenbrand
Re: [PATCH v8 7/7] drm/panfrost: Switch to generic memory shrinker
Hello Steve, On 11/14/22 19:54, Steven Price wrote: > On 05/11/2022 23:27, Dmitry Osipenko wrote: >> Replace Panfrost's custom memory shrinker with a common drm-shmem >> memory shrinker. >> >> Signed-off-by: Dmitry Osipenko > > Sadly this triggers GPU faults under memory pressure - it looks > suspiciously like mappings are being freed while the jobs are still running. > > I'm not sure I understand how the generic shrinker replicates the > "gpu_usecount" atomic that Panfrost currently has, and I'm wondering if > that's the cause? > > Also just reverting this commit (so just patches 1-6) I can't actually > get Panfrost to purge any memory. So I don't think the changes (most > likely in patch 4) are quite right either. > > At the moment I don't have the time to investigate in detail. But if > you've any ideas for something specific I should look at I can run more > testing. Thank you for the testing! It just occurred to me that the shrinker callback lost the dma_resv_test_signaled() in comparison to the previous versions of this patchset. It appeared to me that the drm_gem_lru now checks whether reservation is busy, but it doesn't. I saw a similar page faults once in a while when was testing the Panfrost driver, but then couldn't reproduce the faults after applying the IOMMU unmap range fix that Robin made recently. I'll re-add the dma_resv_test_signaled() in v9, it was a luck that I didn't hit it much during my testing. -- Best regards, Dmitry
Re: [PATCH v8 09/14] drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts
On Mon, Nov 14, 2022 at 8:10 PM Marek Szyprowski
wrote:
>
> On 14.11.2022 11:57, Marek Szyprowski wrote:
> > On 10.11.2022 19:38, Jagan Teki wrote:
> >> Finding the right input bus format throughout the pipeline is hard
> >> so add atomic_get_input_bus_fmts callback and initialize with the
> >> proper input format from list of supported output formats.
> >>
> >> This format can be used in pipeline for negotiating bus format between
> >> the DSI-end of this bridge and the other component closer to pipeline
> >> components.
> >>
> >> List of Pixel formats are taken from,
> >> AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
> >> 3.7.4 Pixel formats
> >> Table 14. DSI pixel packing formats
> >>
> >> v8:
> >> * added pixel formats supported by NXP AN13573 i.MX 8/RT MIPI DSI/CSI-2
> >>
> >> v7, v6, v5, v4:
> >> * none
> >>
> >> v3:
> >> * include media-bus-format.h
> >>
> >> v2:
> >> * none
> >>
> >> v1:
> >> * new patch
> >>
> >> Signed-off-by: Jagan Teki
> >> ---
> >> drivers/gpu/drm/bridge/samsung-dsim.c | 53 +++
> >> 1 file changed, 53 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> >> b/drivers/gpu/drm/bridge/samsung-dsim.c
> >> index 0fe153b29e4f..33e5ae9c865f 100644
> >> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> >> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> >> @@ -15,6 +15,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >> #include
> >> #include
> >> @@ -1321,6 +1322,57 @@ static void
> >> samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
> >> pm_runtime_put_sync(dsi->dev);
> >> }
> >> +/*
> >> + * This pixel output formats list referenced from,
> >> + * AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
> >> + * 3.7.4 Pixel formats
> >> + * Table 14. DSI pixel packing formats
> >> + */
> >> +static const u32 samsung_dsim_pixel_output_fmts[] = {
> >> +MEDIA_BUS_FMT_UYVY8_1X16,
> >> +MEDIA_BUS_FMT_RGB101010_1X30,
> >> +MEDIA_BUS_FMT_RGB121212_1X36,
> >> +MEDIA_BUS_FMT_RGB565_1X16,
> >> +MEDIA_BUS_FMT_RGB666_1X18,
> >> +MEDIA_BUS_FMT_RGB888_1X24,
> >> +};
> >> +
> >> +static bool samsung_dsim_pixel_output_fmt_supported(u32 fmt)
> >> +{
> >> +int i;
> >> +
> >> +for (i = 0; i < ARRAY_SIZE(samsung_dsim_pixel_output_fmts); i++) {
> >> +if (samsung_dsim_pixel_output_fmts[i] == fmt)
> >> +return true;
> >> +}
> >> +
> >> +return false;
> >> +}
> >> +
> >> +static u32 *
> >> +samsung_dsim_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> >> + struct drm_bridge_state *bridge_state,
> >> + struct drm_crtc_state *crtc_state,
> >> + struct drm_connector_state *conn_state,
> >> + u32 output_fmt,
> >> + unsigned int *num_input_fmts)
> >> +{
> >> +u32 *input_fmts;
> >> +
> >> +if (!samsung_dsim_pixel_output_fmt_supported(output_fmt))
> >> +return NULL;
> >
> >
> > Please add support for MEDIA_BUS_FMT_FIXED and maybe default to
> > MEDIA_BUS_FMT_RGB888_1X24 if requested format is not matched.
> >
> > Otherwise the above check breaks all current clients of the Samsung
> > DSIM/Exynos DSI. I didn't dig into the bus matching code yet, but all
> > DSI panels requests such format on my test systems...
>
> I've checked a bit more the bus format related code and it looks that
> there are more issues. The DSI panels don't use the MEDIA_BUS_FMT_*
> formats thus the bridge negotiation code selects MEDIA_BUS_FMT_FIXED for
> them. On Arndale board with Toshiba tc358764 bridge the
> MEDIA_BUS_FMT_RGB888_1X7X4_SPWG output_fmt is requested in
> samsung_dsim_atomic_get_input_bus_fmts() (forwarded from the LVDS panel,
dsim => tc358764 => panel-simple
If I understand the bridge format negotiation correctly - If
atomic_get_input_bus_fmts is not implemented in tc358764 then
MEDIA_BUS_FMT_FIXED will be the output_fmt for dsim so we can assign
MEDIA_BUS_FMT_RGB888_1X24 for FIXED formats.
from include/drm/drm_bridge.h:
* This method is called on all elements of the bridge chain as part of
* the bus format negotiation process that happens in
* drm_atomic_bridge_chain_select_bus_fmts().
* This method is optional. When not implemented, the core will bypass
* bus format negotiation on this element of the bridge without
* failing, and the previous element in the chain will be passed
* MEDIA_BUS_FMT_FIXED as its output bus format.
As I can see tc358764 is not implemented either
atomic_get_input_bus_fmts or atomic API, so I think dsim gets the
MEDIA_BUS_FMT_FIXED bridge pipeline. I have tested sn65dsi without
atomic_get_input_bus_fmts I can see the dsim is getting
MEDIA_BUS_FMT_FIXED.
Can you check the same from your side?
On the other side, MEDIA_BUS_FMT_RGB888_1X7X4_SPWG is 24-bit samples
transferred over an LVDS bus with four differential data pairs,
Re: [PATCH v8 7/7] drm/panfrost: Switch to generic memory shrinker
On 05/11/2022 23:27, Dmitry Osipenko wrote:
> Replace Panfrost's custom memory shrinker with a common drm-shmem
> memory shrinker.
>
> Signed-off-by: Dmitry Osipenko
Sadly this triggers GPU faults under memory pressure - it looks
suspiciously like mappings are being freed while the jobs are still running.
I'm not sure I understand how the generic shrinker replicates the
"gpu_usecount" atomic that Panfrost currently has, and I'm wondering if
that's the cause?
Also just reverting this commit (so just patches 1-6) I can't actually
get Panfrost to purge any memory. So I don't think the changes (most
likely in patch 4) are quite right either.
At the moment I don't have the time to investigate in detail. But if
you've any ideas for something specific I should look at I can run more
testing.
Steve
> ---
> drivers/gpu/drm/panfrost/Makefile | 1 -
> drivers/gpu/drm/panfrost/panfrost_device.h| 4 -
> drivers/gpu/drm/panfrost/panfrost_drv.c | 19 +--
> drivers/gpu/drm/panfrost/panfrost_gem.c | 33 +++--
> drivers/gpu/drm/panfrost/panfrost_gem.h | 9 --
> .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 129 --
> drivers/gpu/drm/panfrost/panfrost_job.c | 18 ++-
> 7 files changed, 42 insertions(+), 171 deletions(-)
> delete mode 100644 drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
>
> diff --git a/drivers/gpu/drm/panfrost/Makefile
> b/drivers/gpu/drm/panfrost/Makefile
> index 7da2b3f02ed9..11622e22cf15 100644
> --- a/drivers/gpu/drm/panfrost/Makefile
> +++ b/drivers/gpu/drm/panfrost/Makefile
> @@ -5,7 +5,6 @@ panfrost-y := \
> panfrost_device.o \
> panfrost_devfreq.o \
> panfrost_gem.o \
> - panfrost_gem_shrinker.o \
> panfrost_gpu.o \
> panfrost_job.o \
> panfrost_mmu.o \
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
> b/drivers/gpu/drm/panfrost/panfrost_device.h
> index 8b25278f34c8..fe04b21fc044 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -115,10 +115,6 @@ struct panfrost_device {
> atomic_t pending;
> } reset;
>
> - struct mutex shrinker_lock;
> - struct list_head shrinker_list;
> - struct shrinker shrinker;
> -
> struct panfrost_devfreq pfdevfreq;
> };
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 94b8e6de34b8..fe78d5c75abf 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -160,7 +160,6 @@ panfrost_lookup_bos(struct drm_device *dev,
> break;
> }
>
> - atomic_inc(>gpu_usecount);
> job->mappings[i] = mapping;
> }
>
> @@ -392,7 +391,6 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
> void *data,
> {
> struct panfrost_file_priv *priv = file_priv->driver_priv;
> struct drm_panfrost_madvise *args = data;
> - struct panfrost_device *pfdev = dev->dev_private;
> struct drm_gem_object *gem_obj;
> struct panfrost_gem_object *bo;
> int ret = 0;
> @@ -409,7 +407,6 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
> void *data,
> if (ret)
> goto out_put_object;
>
> - mutex_lock(>shrinker_lock);
> mutex_lock(>mappings.lock);
> if (args->madv == PANFROST_MADV_DONTNEED) {
> struct panfrost_gem_mapping *first;
> @@ -435,17 +432,8 @@ static int panfrost_ioctl_madvise(struct drm_device
> *dev, void *data,
>
> args->retained = drm_gem_shmem_madvise(>base, args->madv);
>
> - if (args->retained) {
> - if (args->madv == PANFROST_MADV_DONTNEED)
> - list_move_tail(>base.madv_list,
> ->shrinker_list);
> - else if (args->madv == PANFROST_MADV_WILLNEED)
> - list_del_init(>base.madv_list);
> - }
> -
> out_unlock_mappings:
> mutex_unlock(>mappings.lock);
> - mutex_unlock(>shrinker_lock);
> dma_resv_unlock(bo->base.base.resv);
> out_put_object:
> drm_gem_object_put(gem_obj);
> @@ -577,9 +565,6 @@ static int panfrost_probe(struct platform_device *pdev)
> ddev->dev_private = pfdev;
> pfdev->ddev = ddev;
>
> - mutex_init(>shrinker_lock);
> - INIT_LIST_HEAD(>shrinker_list);
> -
> err = panfrost_device_init(pfdev);
> if (err) {
> if (err != -EPROBE_DEFER)
> @@ -601,7 +586,7 @@ static int panfrost_probe(struct platform_device *pdev)
> if (err < 0)
> goto err_out1;
>
> - panfrost_gem_shrinker_init(ddev);
> + drm_gem_shmem_shrinker_register(ddev, "panfrost-shrinker");
>
> return 0;
>
> @@ -619,8 +604,8 @@ static int panfrost_remove(struct platform_device *pdev)
> struct panfrost_device *pfdev = platform_get_drvdata(pdev);
> struct drm_device *ddev = pfdev->ddev;
>
> +
Re: [PATCH] mm/memory: Return vm_fault_t result from migrate_to_ram() callback
On 14.11.22 12:55, Alistair Popple wrote:
The migrate_to_ram() callback should always succeed, but in rare cases
can fail usually returning VM_FAULT_SIGBUS. Commit 16ce101db85d
("mm/memory.c: fix race when faulting a device private page")
incorrectly stopped passing the return code up the stack. Fix this by
setting the ret variable, restoring the previous behaviour on
migrate_to_ram() failure.
Fixes: 16ce101db85d ("mm/memory.c: fix race when faulting a device private
page")
Signed-off-by: Alistair Popple
Cc: Ralph Campbell
Cc: John Hubbard
Cc: Alex Sierra
Cc: Ben Skeggs
Cc: Felix Kuehling
Cc: Lyude Paul
Cc: Jason Gunthorpe
Cc: Michael Ellerman
---
Hi Andrew, I noticed my series needs another small fix which I'm
hoping you can apply for v6.1-rcX. Thanks (and sorry for not catching
this sooner).
---
mm/memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/memory.c b/mm/memory.c
index f88c351aecd4..8a6d5c823f91 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3763,7 +3763,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
*/
get_page(vmf->page);
pte_unmap_unlock(vmf->pte, vmf->ptl);
- vmf->page->pgmap->ops->migrate_to_ram(vmf);
+ ret = vmf->page->pgmap->ops->migrate_to_ram(vmf);
put_page(vmf->page);
} else if (is_hwpoison_entry(entry)) {
ret = VM_FAULT_HWPOISON;
Acked-by: David Hildenbrand
--
Thanks,
David / dhildenb
Re: [PATCH 1/1] drm/shmem: Dual licence the files as GPL-2 and MIT
On Sat, 12 Nov 2022 19:42:10 + Robert Swindells wrote: > Contributors to these files are: > > Noralf Trønnes > Liu Zixian > Dave Airlie > Thomas Zimmermann > Lucas De Marchi > Gerd Hoffmann > Rob Herring > Jakub Kicinski > Marcel Ziswiler > Stephen Rothwell > Daniel Vetter > Cai Huoqing > Neil Roberts > Marek Szyprowski > Emil Velikov > Sam Ravnborg > Boris Brezillon > Dan Carpenter > > Signed-off-by: Robert Swindells For the one-line #include addition I've contributed: Acked-by: Jakub Kicinski :)
[PATCH printk v4 31/39] 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
---
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 e770b1ede6c9..dff76c1cef80 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
@@ -3461,6 +3462,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(>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(_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_behind_rcu(>node, console_list.first);
+}
+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 v4 00/39] reduce console_lock scope
This is v4 of a series to prepare for threaded/atomic printing. v3 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 was 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 v3: general: - Implement console_srcu_read_flags() and console_srcu_write_flags() to be used for console->flags access under the srcu_read_lock or console_list_lock, respectively. The functions document their relationship to one another and use data_race(), READ_ONCE(), and WRITE_ONCE() macros to annotate their relationship. They also make use of lockdep to warn if used in improper contexts. - Replace all console_is_enabled() usage with console_srcu_read_flags() (all were under the srcu_read_lock). serial_core: - For uart_console_registered(), check uart_console() before taking the console_list_lock to avoid unnecessary lock contention for non-console ports. m68k/emu/nfcon: - Only explicitly enable the console if registering via debug=nfcon. tty/serial/sh-sci: - Add comments about why @options will always be NULL for the earlyprintk console. kdb: - Add comments explaining the expectations for console drivers to work correctly. printk: - Some code sites under SRCU were checking flags directly. Use console_srcu_read_flags() instead. - In register_console() rename bootcon_enabled/realcon_enabled to bootcon_registered/realcon_registered to avoid confusion. - In register_console() only check for boot console sequences if a boot console is registered and !keep_bootcon. In this case, also take the console_lock to guarantee safe access to console->seq. - In console_force_preferred_locked() use hlist_del_rcu() instead of hlist_del_init_rcu() so that there is never a window where the console can be viewed as unregistered. John Ogness [0] https://lore.kernel.org/lkml/20221107141638.3790965-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 (37): printk: Prepare for SRCU console list protection printk: register_console: use "registered" for variable names 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
[PATCH v2 3/4] usb: gadget: udc: bcm63xx: Convert to use list_count()
The list API now provides the list_count() to help with counting
existing nodes in the list. Uilise it.
Signed-off-by: Andy Shevchenko
---
v2: no change
drivers/usb/gadget/udc/bcm63xx_udc.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c
b/drivers/usb/gadget/udc/bcm63xx_udc.c
index 2cdb07905bde..0762e49e85f8 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -2172,7 +2172,6 @@ static int bcm63xx_iudma_dbg_show(struct seq_file *s,
void *p)
for (ch_idx = 0; ch_idx < BCM63XX_NUM_IUDMA; ch_idx++) {
struct iudma_ch *iudma = >iudma[ch_idx];
- struct list_head *pos;
seq_printf(s, "IUDMA channel %d -- ", ch_idx);
switch (iudma_defaults[ch_idx].ep_type) {
@@ -2205,14 +2204,10 @@ static int bcm63xx_iudma_dbg_show(struct seq_file *s,
void *p)
seq_printf(s, " desc: %d/%d used", iudma->n_bds_used,
iudma->n_bds);
- if (iudma->bep) {
- i = 0;
- list_for_each(pos, >bep->queue)
- i++;
- seq_printf(s, "; %d queued\n", i);
- } else {
+ if (iudma->bep)
+ seq_printf(s, "; %zu queued\n",
list_count(>bep->queue));
+ else
seq_printf(s, "\n");
- }
for (i = 0; i < iudma->n_bds; i++) {
struct bcm_enet_desc *d = >bd_ring[i];
--
2.35.1
[PATCH v2 4/4] xhci: Convert to use list_count()
The list API now provides the list_count() to help with counting
existing nodes in the list. Uilise it.
Signed-off-by: Andy Shevchenko
---
v2: no change
drivers/usb/host/xhci-ring.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ad81e9a508b1..817c31e3b0c8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2532,7 +2532,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
union xhci_trb *ep_trb;
int status = -EINPROGRESS;
struct xhci_ep_ctx *ep_ctx;
- struct list_head *tmp;
u32 trb_comp_code;
int td_num = 0;
bool handling_skipped_tds = false;
@@ -2580,10 +2579,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
}
/* Count current td numbers if ep->skip is set */
- if (ep->skip) {
- list_for_each(tmp, _ring->td_list)
- td_num++;
- }
+ if (ep->skip)
+ td_num += list_count(_ring->td_list);
/* Look for common error cases */
switch (trb_comp_code) {
--
2.35.1
[PATCH v2 1/4] i915: Move list_count() to list.h for broader use
Some of the existing users, and definitely will be new ones, want to
count existing nodes in the list. Provide a generic API for that by
moving code from i915 to list.h.
Signed-off-by: Andy Shevchenko
---
v2: dropped the duplicate code in i915 (LKP)
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 13 +
include/linux/list.h | 13 +
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 6ae8b07cfaa1..b5d474be564d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -2085,17 +2085,6 @@ static void print_request_ring(struct drm_printer *m,
struct i915_request *rq)
}
}
-static unsigned long list_count(struct list_head *list)
-{
- struct list_head *pos;
- unsigned long count = 0;
-
- list_for_each(pos, list)
- count++;
-
- return count;
-}
-
static unsigned long read_ul(void *p, size_t x)
{
return *(unsigned long *)(p + x);
@@ -2270,7 +2259,7 @@ void intel_engine_dump(struct intel_engine_cs *engine,
spin_lock_irqsave(>sched_engine->lock, flags);
engine_dump_active_requests(engine, m);
- drm_printf(m, "\tOn hold?: %lu\n",
+ drm_printf(m, "\tOn hold?: %zu\n",
list_count(>sched_engine->hold));
spin_unlock_irqrestore(>sched_engine->lock, flags);
diff --git a/include/linux/list.h b/include/linux/list.h
index 61762054b4be..098eccf8c1b6 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -655,6 +655,19 @@ static inline void list_splice_tail_init(struct list_head
*list,
!list_is_head(pos, (head)); \
pos = n, n = pos->prev)
+/**
+ * list_count - count nodes in the list
+ * @head: the head for your list.
+ */
+#define list_count(head) \
+({ \
+ struct list_head *__tmp;\
+ size_t __i = 0; \
+ list_for_each(__tmp, head) \
+ __i++; \
+ __i;\
+})
+
/**
* list_entry_is_head - test if the entry points to the head of the list
* @pos: the type * to cursor
--
2.35.1
[PATCH v2 2/4] usb: gadget: hid: Convert to use list_count()
The list API now provides the list_count() to help with counting
existing nodes in the list. Uilise it.
Signed-off-by: Andy Shevchenko
---
v2: no change
drivers/usb/gadget/legacy/hid.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/legacy/hid.c b/drivers/usb/gadget/legacy/hid.c
index 1187ee4f316a..6196c3456e0b 100644
--- a/drivers/usb/gadget/legacy/hid.c
+++ b/drivers/usb/gadget/legacy/hid.c
@@ -133,14 +133,11 @@ static struct usb_configuration config_driver = {
static int hid_bind(struct usb_composite_dev *cdev)
{
struct usb_gadget *gadget = cdev->gadget;
- struct list_head *tmp;
struct hidg_func_node *n = NULL, *m, *iter_n;
struct f_hid_opts *hid_opts;
- int status, funcs = 0;
-
- list_for_each(tmp, _func_list)
- funcs++;
+ int status, funcs;
+ funcs = list_count(_func_list);
if (!funcs)
return -ENODEV;
--
2.35.1
Re: [PATCH v3 4/8] drm/msm/dsi: add support for DSI-PHY on SM8350 and SM8450
On 10/11/2022 23:16, Dmitry Baryshkov wrote:
On 04/11/2022 16:54, Konrad Dybcio wrote:
On 04/11/2022 14:03, Dmitry Baryshkov wrote:
SM8350 and SM8450 use 5nm DSI PHYs, which share register definitions
with 7nm DSI PHYs. Rather than duplicating the driver, handle 5nm
variants inside the common 5+7nm driver.
Co-developed-by: Robert Foss
Tested-by: Vinod Koul
Reviewed-by: Vinod Koul
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/Kconfig | 6 +-
drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 4 +
drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 2 +
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 128 --
4 files changed, 127 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 3c9dfdb0b328..e7b100d97f88 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -140,12 +140,12 @@ config DRM_MSM_DSI_10NM_PHY
Choose this option if DSI PHY on SDM845 is used on the platform.
config DRM_MSM_DSI_7NM_PHY
- bool "Enable DSI 7nm PHY driver in MSM DRM"
+ bool "Enable DSI 7nm/5nm PHY driver in MSM DRM"
depends on DRM_MSM_DSI
default y
help
- Choose this option if DSI PHY on SM8150/SM8250/SC7280 is used on
- the platform.
+ Choose this option if DSI PHY on
SM8150/SM8250/SM8350/SM8450/SC7280
+ is used on the platform.
config DRM_MSM_HDMI
bool "Enable HDMI support in MSM DRM driver"
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index ee6051367679..0c956fdab23e 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -569,6 +569,10 @@ static const struct of_device_id
dsi_phy_dt_match[] = {
.data = _phy_7nm_8150_cfgs },
{ .compatible = "qcom,sc7280-dsi-phy-7nm",
.data = _phy_7nm_7280_cfgs },
+ { .compatible = "qcom,dsi-phy-5nm-8350",
+ .data = _phy_5nm_8350_cfgs },
+ { .compatible = "qcom,dsi-phy-5nm-8450",
+ .data = _phy_5nm_8450_cfgs },
#endif
{}
};
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index 1096afedd616..f7a907ed2b4b 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -57,6 +57,8 @@ extern const struct msm_dsi_phy_cfg
dsi_phy_10nm_8998_cfgs;
extern const struct msm_dsi_phy_cfg dsi_phy_7nm_cfgs;
extern const struct msm_dsi_phy_cfg dsi_phy_7nm_8150_cfgs;
extern const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8350_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs;
struct msm_dsi_dphy_timing {
u32 clk_zero;
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 9e7fa7d88ead..00d92fe97bc3 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -39,8 +39,14 @@
#define VCO_REF_CLK_RATE 1920
#define FRAC_BITS 18
+/* Hardware is pre V4.1 */
+#define DSI_PHY_7NM_QUIRK_PRE_V4_1 BIT(0)
/* Hardware is V4.1 */
-#define DSI_PHY_7NM_QUIRK_V4_1 BIT(0)
+#define DSI_PHY_7NM_QUIRK_V4_1 BIT(1)
+/* Hardware is V4.2 */
+#define DSI_PHY_7NM_QUIRK_V4_2 BIT(2)
+/* Hardware is V4.3 */
+#define DSI_PHY_7NM_QUIRK_V4_3 BIT(3)
Quirk is quite an unfortunate name considering what we use it for.. but I
suppose it can stay, as otherwise even more renaming would have to be
done.
struct dsi_pll_config {
bool enable_ssc;
@@ -116,7 +122,7 @@ static void dsi_pll_calc_dec_frac(struct
dsi_pll_7nm *pll, struct dsi_pll_config
dec_multiple = div_u64(pll_freq * multiplier, divider);
dec = div_u64_rem(dec_multiple, multiplier, );
- if (!(pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1))
+ if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1)
config->pll_clock_inverters = 0x28;
else if (pll_freq <= 10ULL)
config->pll_clock_inverters = 0xa0;
@@ -197,16 +203,25 @@ static void dsi_pll_config_hzindep_reg(struct
dsi_pll_7nm *pll)
void __iomem *base = pll->phy->pll_base;
u8 analog_controls_five_1 = 0x01, vco_config_1 = 0x00;
- if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
+ if (!(pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1))
if (pll->vco_current_rate >= 31ULL)
analog_controls_five_1 = 0x03;
+ if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
if (pll->vco_current_rate < 152000ULL)
vco_config_1 = 0x08;
else if (pll->vco_current_rate < 299000ULL)
vco_config_1 = 0x01;
}
+ if ((pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_2) ||
+ (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3)) {
+ if (pll->vco_current_rate < 152000ULL)
+ vco_config_1 = 0x08;
+ else if (pll->vco_current_rate >=
Re: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On 11/14/22 9:59 AM, Jason Gunthorpe wrote: > On Mon, Nov 14, 2022 at 09:55:21AM -0500, Matthew Rosato wrote: AFAICT there is no equivalent means to specify vfio_iommu_type1.dma_entry_limit when using iommufd; looks like we'll just always get the default 65535. >>> >>> No, there is no arbitary limit on iommufd >> >> Yeah, that's what I suspected. But FWIW, userspace checks the >> advertised limit via VFIO_IOMMU_GET_INFO / >> VFIO_IOMMU_TYPE1_INFO_DMA_AVAIL, and this is still being advertised >> as 65535 when using iommufd. I don't think there is a defined way >> to return 'ignore this value'. > > Is something using this? Should we make it much bigger? Yes, s390 when doing lazy unmapping likes to use larger amounts of concurrent DMA, so there can be cases where we want to raise this limit. The initial value of 65535 is already pretty arbitrary (U16_MAX) -- If iommufd is doing its own management and this value becomes deprecated in this scenario, and we can't set it to a magic value that says 'ignore me' then maybe it just makes sense for now to set it arbitrarily larger when using iommufd e.g. U32_MAX?
Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
* , ,
Hi, Justin:
Justin Green 於 2022年10月13日 週四 凌晨3:12寫道：
>
> Tested on MT8195 and confirmed both correct video output and improved DRAM
> bandwidth performance.
>
> 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 | 90 +++-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 37 +-
> drivers/gpu/drm/mediatek/mtk_drm_plane.h | 8 +++
> 3 files changed, 131 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 002b0f6cae1a..3f89b5f5064f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -29,17 +29,24 @@
> #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 OVL_PITCH_MSB_YUV_TRANSBIT(20)
Useless, so drop it.
> #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n))
> +#define DISP_REG_OVL_CLIP(n) (0x004c + 0x20 * (n))
Useless, so drop it.
> #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 +74,7 @@ struct mtk_disp_ovl_data {
> unsigned int layer_nr;
> bool fmt_rgb565_is_0;
> bool smi_id_en;
> + bool supports_afbc;
> };
>
> /*
> @@ -172,7 +180,22 @@ 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 device *dev, struct cmdq_pkt *cmdq_pkt,
> +int idx, bool enabled)
> +{
> + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
This is a ovl internal function, so you could pass ovl directly into
this function.
> + unsigned int reg;
>
> + reg = readl(ovl->regs + DISP_REG_OVL_DATAPATH_CON);
> + if (enabled)
> + reg = reg | OVL_LAYER_AFBC_EN(idx);
> + else
> + reg = reg & ~OVL_LAYER_AFBC_EN(idx);
> +
> + mtk_ddp_write_relaxed(cmdq_pkt, reg, >cmdq_reg,
> + ovl->regs, DISP_REG_OVL_DATAPATH_CON);
In normal case, read/write register by cmdq, so
mtk_ddp_write_mask(cmdq_pkt, enable ? OVL_LAYER_AFBC_EN(idx) : 0,
>cmdq_reg, ovl->regs,
DISP_REG_OVL_DATAPATH_CON,
OVL_LAYER_AFBC_EN(idx));
> }
>
> void mtk_ovl_config(struct device *dev, unsigned int w,
> @@ -226,6 +249,32 @@ int mtk_ovl_layer_check(struct device *dev, unsigned int
> idx,
> if (state->fb->format->is_yuv && rotation != 0)
> return -EINVAL;
>
> + if (state->fb->modifier) {
> + unsigned long long modifier;
> + unsigned int fourcc;
> + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> +
> + if (!ovl->data->supports_afbc)
> + return -EINVAL;
> +
> + modifier = state->fb->modifier;
> +
> + if (modifier !=
> DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
> + AFBC_FORMAT_MOD_SPLIT
> |
> +
> AFBC_FORMAT_MOD_SPARSE))
> + return -EINVAL;
> +
> + fourcc = state->fb->format->format;
> + if (fourcc != DRM_FORMAT_BGRA &&
> + fourcc != DRM_FORMAT_ABGR &&
> + fourcc !=
Re: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On Mon, Nov 14, 2022 at 09:55:21AM -0500, Matthew Rosato wrote: > >> AFAICT there is no equivalent means to specify > >> vfio_iommu_type1.dma_entry_limit when using iommufd; looks like > >> we'll just always get the default 65535. > > > > No, there is no arbitary limit on iommufd > > Yeah, that's what I suspected. But FWIW, userspace checks the > advertised limit via VFIO_IOMMU_GET_INFO / > VFIO_IOMMU_TYPE1_INFO_DMA_AVAIL, and this is still being advertised > as 65535 when using iommufd. I don't think there is a defined way > to return 'ignore this value'. Is something using this? Should we make it much bigger? Jason
Re: [PATCH v3 2/2] gpu/drm/panel: Add Sony TD4353 JDI panel driver
On 14/10/2022 18:36, Konrad Dybcio wrote:
On 30.09.2022 20:08, Konrad Dybcio wrote:
Add support for the Sony TD4353 JDI 2160x1080 display panel used in
some Sony Xperia XZ2 and XZ2 Compact smartphones. Due to the specifics
of smartphone manufacturing, it is impossible to retrieve a better name
for this panel.
This revision adds support for the default 60 Hz configuration, however
there could possibly be some room for expansion, as the display panels
used on Sony devices have historically been capable of >2x refresh rate
overclocking.
Signed-off-by: Konrad Dybcio
---
Gentle bump
Konrad
Yet another one.
Konrad
Changes since v2:
- "GPL v2" -> "GPL"
- add missing S-o-b (how embarassing)
- move { after sony_td4353_assert_reset_gpios() to a new line
drivers/gpu/drm/panel/Kconfig | 10 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-sony-td4353-jdi.c | 329 ++
3 files changed, 340 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index a582ddd583c2..6ef1b48169b5 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -637,6 +637,16 @@ config DRM_PANEL_SONY_ACX565AKM
Say Y here if you want to enable support for the Sony ACX565AKM
800x600 3.5" panel (found on the Nokia N900).
+config DRM_PANEL_SONY_TD4353_JDI
+ tristate "Sony TD4353 JDI panel"
+ depends on GPIOLIB && OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y here if you want to enable support for the Sony Tama
+ TD4353 JDI command mode panel as found on some Sony Xperia
+ XZ2 and XZ2 Compact smartphones.
+
config DRM_PANEL_SONY_TULIP_TRULY_NT35521
tristate "Sony Tulip Truly NT35521 panel"
depends on GPIOLIB && OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 8e71aa7581b8..8ef27bc86f94 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) +=
panel-sitronix-st7701.o
obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7703) += panel-sitronix-st7703.o
obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
+obj-$(CONFIG_DRM_PANEL_SONY_TD4353_JDI) += panel-sony-td4353-jdi.o
obj-$(CONFIG_DRM_PANEL_SONY_TULIP_TRULY_NT35521) +=
panel-sony-tulip-truly-nt35521.o
obj-$(CONFIG_DRM_PANEL_TDO_TL070WSH30) += panel-tdo-tl070wsh30.o
obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
diff --git a/drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
b/drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
new file mode 100644
index ..11db62992b8b
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
@@ -0,0 +1,329 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Konrad Dybcio
+ *
+ * Generated with linux-mdss-dsi-panel-driver-generator with a
+ * substantial amount of manual adjustments.
+ *
+ * SONY Downstream kernel calls this one:
+ * - "JDI ID3" for Akari (XZ2)
+ * - "JDI ID4" for Apollo (XZ2 Compact)
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+
+enum {
+ TYPE_TAMA_60HZ,
+ /*
+* Leaving room for expansion - SONY very often uses
+* *truly reliably* overclockable panels on their flagships!
+*/
+};
+
+struct sony_td4353_jdi {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi;
+ struct regulator_bulk_data supplies[3];
+ struct gpio_desc *panel_reset_gpio;
+ struct gpio_desc *touch_reset_gpio;
+ bool prepared;
+ int type;
+};
+
+static inline struct sony_td4353_jdi *to_sony_td4353_jdi(struct drm_panel
*panel)
+{
+ return container_of(panel, struct sony_td4353_jdi, panel);
+}
+
+static int sony_td4353_jdi_on(struct sony_td4353_jdi *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_set_column_address(dsi, 0x, 0x0437);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set column address: %d\n", ret);
+ return ret;
+ }
+
+ ret = mipi_dsi_dcs_set_page_address(dsi, 0x, 0x086f);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set page address: %d\n", ret);
+ return ret;
+ }
+
+ ret = mipi_dsi_dcs_set_tear_scanline(dsi, 0x);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set tear scanline: %d\n", ret);
+ return ret;
+ }
+
+ ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set tear on:
Re: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On 11/14/22 9:23 AM, Jason Gunthorpe wrote: > On Thu, Nov 10, 2022 at 10:01:13PM -0500, Matthew Rosato wrote: >> On 11/7/22 7:52 PM, Jason Gunthorpe wrote: >>> This series provides an alternative container layer for VFIO implemented >>> using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will >>> not be compiled in. >>> >>> At this point iommufd can be injected by passing in a iommfd FD to >>> VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd >>> to obtain the compat IOAS and then connect up all the VFIO drivers as >>> appropriate. >>> >>> This is temporary stopping point, a following series will provide a way to >>> directly open a VFIO device FD and directly connect it to IOMMUFD using >>> native ioctls that can expose the IOMMUFD features like hwpt, future >>> vPASID and dynamic attachment. >>> >>> This series, in compat mode, has passed all the qemu tests we have >>> available, including the test suites for the Intel GVT mdev. Aside from >>> the temporary limitation with P2P memory this is belived to be fully >>> compatible with VFIO. >> >> AFAICT there is no equivalent means to specify >> vfio_iommu_type1.dma_entry_limit when using iommufd; looks like >> we'll just always get the default 65535. > > No, there is no arbitary limit on iommufd Yeah, that's what I suspected. But FWIW, userspace checks the advertised limit via VFIO_IOMMU_GET_INFO / VFIO_IOMMU_TYPE1_INFO_DMA_AVAIL, and this is still being advertised as 65535 when using iommufd. I don't think there is a defined way to return 'ignore this value'. This should go away later when we bind to iommufd directly since QEMU would not be sharing the type1 codepath in userspace. > >> Was this because you envision the limit being not applicable for >> iommufd (limits will be enforced via either means and eventually we >> won't want to ) or was it an oversight? > > The limit here is primarily about limiting userspace abuse of the > interface. > > iommufd is using GFP_KERNEL_ACCOUNT which shifts the responsiblity to > cgroups, which is similar to how KVM works. > > So, for a VM sandbox you'd set a cgroup limit and if a hostile > userspace in the sanbox decides to try to OOM the system it will hit > that limit, regardless of which kernel APIs it tries to abuse. > > This work is not entirely complete as we also need the iommu driver to > use GFP_KERNEL_ACCOUNT for allocations connected to the iommu_domain, > particularly for allocations of the IO page tables themselves - which > can be quite big. > > Jason
Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
Hi Laurent,
On Mon, Nov 14, 2022 at 3:22 PM Laurent Pinchart
wrote:
> On Mon, Nov 14, 2022 at 10:05:58AM +0100, Geert Uytterhoeven wrote:
> > On Sun, Oct 2, 2022 at 12:06 AM Laurent Pinchart wrote:
> > > When the R-Car MIPI DSI driver was added, it was a standalone encoder
> > > driver without any dependency to or from the R-Car DU driver. Commit
> > > 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence") then
> > > added a direct call from the DU driver to the MIPI DSI driver, without
> > > updating Kconfig to take the new dependency into account. Fix it the
> > > same way that the LVDS encoder is handled.
> > >
> > > Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
> > > Reported-by: kernel test robot
> > > Signed-off-by: Laurent Pinchart
> > >
> >
> > Thanks for your patch, which is now commit a830a15678593948
> > ("drm: rcar-du: Fix Kconfig dependency between RCAR_DU
> > and RCAR_MIPI_DSI") in v6.1-rc5.
> >
> > > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > > @@ -44,12 +44,17 @@ config DRM_RCAR_LVDS
> > > select OF_FLATTREE
> > > select OF_OVERLAY
> > >
> > > +config DRM_RCAR_USE_MIPI_DSI
> > > + bool "R-Car DU MIPI DSI Encoder Support"
> > > + depends on DRM_BRIDGE && OF
> > > + default DRM_RCAR_DU
> >
> > This means this driver is now enabled by default on systems that do not
> > have the MIPI DSI Encoder (e.g. R-Car Gen2), and that we should probably
> > disable it explicitly in shmobile_defconfig. Is that intentional?
>
> I don't think so, no. Would you like to send a patch ? If so, it should
> enable the option in relevant defconfig files.
You mean just drop the "default DRM_RCAR_DU" here?
I'm wondering if we can solve this in a consistent way.
Currently we have:
config DRM_RCAR_USE_CMM default DRM_RCAR_DU
config DRM_RCAR_DW_HDMI default n
config DRM_RCAR_USE_LVDS default DRM_RCAR_DU
config DRM_RCAR_MIPI_DSI default n
config DRM_RCAR_VSP default y if ARM64
HDMI is only used on R-Car Gen3 parts
MIPI_DSI is only used on R-Car Gen4 parts
LVDS is used on R-Car Gen2 and Gen3 parts
Thoughts?
> > > + help
> > > + Enable support for the R-Car Display Unit embedded MIPI DSI
> > > encoders.
> > > +
> > > config DRM_RCAR_MIPI_DSI
> > > - tristate "R-Car DU MIPI DSI Encoder Support"
> > > - depends on DRM && DRM_BRIDGE && OF
> > > + def_tristate DRM_RCAR_DU
> > > + depends on DRM_RCAR_USE_MIPI_DSI
> > > select DRM_MIPI_DSI
> > > - help
> > > - Enable support for the R-Car Display Unit embedded MIPI DSI
> > > encoders.
> > >
> > > config DRM_RCAR_VSP
> > > bool "R-Car DU VSP Compositor Support" if ARM
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 11/11] iommufd: Allow iommufd to supply /dev/vfio/vfio
On Fri, Nov 11, 2022 at 12:16:02PM +0800, Yi Liu wrote:
> > +#if IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)
> > +MODULE_ALIAS_MISCDEV(VFIO_MINOR);
> > +MODULE_ALIAS("devname:vfio/vfio");
>
> will this line also result in systemd to create this devnodes at boot
> based on the module info even if the IOMMUFD_VFIO_CONTAINER is not
> configured?
No, it is contained by an ifdef.
The MODULE_ALIAS mechanism works by inspecting the compiled object
files for special ELF sections, if the code is discarded by the
preprocessor then the sections will not be created by the compiler.
Jason
Re: [PATCH v2 07/11] vfio-iommufd: Support iommufd for physical VFIO devices
On Fri, Nov 11, 2022 at 12:12:36PM +0800, Yi Liu wrote:
> > +int vfio_iommufd_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx)
> > +{
> > + u32 ioas_id;
> > + u32 device_id;
> > + int ret;
> > +
> > + lockdep_assert_held(>dev_set->lock);
> > +
> > + /*
> > +* If the driver doesn't provide this op then it means the device does
> > +* not do DMA at all. So nothing to do.
> > +*/
> > + if (!vdev->ops->bind_iommufd)
> > + return 0;
> > +
> > + ret = vdev->ops->bind_iommufd(vdev, ictx, _id);
> > + if (ret)
> > + return ret;
> > +
> > + ret = iommufd_vfio_compat_ioas_id(ictx, _id);
> > + if (ret)
> > + goto err_unbind;
> > + ret = vdev->ops->attach_ioas(vdev, _id);
> > + if (ret)
> > + goto err_unbind;
> > + vdev->iommufd_attached = true;
>
> it's better to set this bool in vfio_iommufd_physical_attach_ioas() as
> the emulated devices uses iommufd_access instead. is it? or you mean this
> flag to cover both cases?
Yes, that is probably clearer:
@@ -50,7 +50,6 @@ int vfio_iommufd_bind(struct vfio_device *vdev, struct
iommufd_ctx *ictx)
ret = vdev->ops->attach_ioas(vdev, _id);
if (ret)
goto err_unbind;
- vdev->iommufd_attached = true;
/*
* The legacy path has no way to return the device id or the selected
@@ -110,10 +109,15 @@ EXPORT_SYMBOL_GPL(vfio_iommufd_physical_unbind);
int vfio_iommufd_physical_attach_ioas(struct vfio_device *vdev, u32 *pt_id)
{
unsigned int flags = 0;
+ int rc;
if (vfio_allow_unsafe_interrupts)
flags |= IOMMUFD_ATTACH_FLAGS_ALLOW_UNSAFE_INTERRUPT;
- return iommufd_device_attach(vdev->iommufd_device, pt_id, flags);
+ rc = iommufd_device_attach(vdev->iommufd_device, pt_id, flags);
+ if (rc)
+ return rc;
+ vdev->iommufd_attached = true;
+ return 0;
}
EXPORT_SYMBOL_GPL(vfio_iommufd_physical_attach_ioas);
Thanks,
Jason
Re: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On 2022/11/14 22:38, Jason Gunthorpe wrote: On Mon, Nov 14, 2022 at 08:51:58PM +0800, Yi Liu wrote: Our side, Yu He, Lixiao Yang has done below tests on Intel platform with the above kernel, results are: 1) GVT-g test suit passed, Intel iGFx passthrough passed. 2) NIC passthrough test with different guest memory (1G/4G), passed. 3) Booting two different QEMUs in the same time but one QEMU opens legacy /dev/vfio/vfio and another opens /dev/iommu. Tests passed. 4) Tried below Kconfig combinations, results are expected. VFIO_CONTAINER=y, IOMMUFD=y -- test pass VFIO_CONTAINER=y, IOMMUFD=n -- test pass VFIO_CONTAINER=n, IOMMUFD=y , IOMMUFD_VFIO_CONTAINER=y -- test pass VFIO_CONTAINER=n, IOMMUFD=y , IOMMUFD_VFIO_CONTAINER=n -- no /dev/vfio/vfio, so test fail, expected 5) Tested devices from multi-device group. Assign such devices to the same VM, pass; assign them to different VMs, fail; assign them to a VM with Intel virtual VT-d, fail; Results are expected. Meanwhile, I also tested the branch in development branch for nesting, the basic functionality looks good. Tested-by: Yi Liu Great thanks! you are welcome. this is a team work. :) In future I also recommend running tests with the CONFIG_IOMMUFD_TEST turned on, it enables a bunch more fast path assertions that might catch something interesting oh, sure. will try. -- Regards, Yi Liu
Re: [PATCH v8 09/14] drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts
On 14.11.2022 11:57, Marek Szyprowski wrote:
> On 10.11.2022 19:38, Jagan Teki wrote:
>> Finding the right input bus format throughout the pipeline is hard
>> so add atomic_get_input_bus_fmts callback and initialize with the
>> proper input format from list of supported output formats.
>>
>> This format can be used in pipeline for negotiating bus format between
>> the DSI-end of this bridge and the other component closer to pipeline
>> components.
>>
>> List of Pixel formats are taken from,
>> AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
>> 3.7.4 Pixel formats
>> Table 14. DSI pixel packing formats
>>
>> v8:
>> * added pixel formats supported by NXP AN13573 i.MX 8/RT MIPI DSI/CSI-2
>>
>> v7, v6, v5, v4:
>> * none
>>
>> v3:
>> * include media-bus-format.h
>>
>> v2:
>> * none
>>
>> v1:
>> * new patch
>>
>> Signed-off-by: Jagan Teki
>> ---
>> drivers/gpu/drm/bridge/samsung-dsim.c | 53 +++
>> 1 file changed, 53 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index 0fe153b29e4f..33e5ae9c865f 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -15,6 +15,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> @@ -1321,6 +1322,57 @@ static void
>> samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
>> pm_runtime_put_sync(dsi->dev);
>> }
>> +/*
>> + * This pixel output formats list referenced from,
>> + * AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
>> + * 3.7.4 Pixel formats
>> + * Table 14. DSI pixel packing formats
>> + */
>> +static const u32 samsung_dsim_pixel_output_fmts[] = {
>> + MEDIA_BUS_FMT_UYVY8_1X16,
>> + MEDIA_BUS_FMT_RGB101010_1X30,
>> + MEDIA_BUS_FMT_RGB121212_1X36,
>> + MEDIA_BUS_FMT_RGB565_1X16,
>> + MEDIA_BUS_FMT_RGB666_1X18,
>> + MEDIA_BUS_FMT_RGB888_1X24,
>> +};
>> +
>> +static bool samsung_dsim_pixel_output_fmt_supported(u32 fmt)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(samsung_dsim_pixel_output_fmts); i++) {
>> + if (samsung_dsim_pixel_output_fmts[i] == fmt)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static u32 *
>> +samsung_dsim_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>> + struct drm_bridge_state *bridge_state,
>> + struct drm_crtc_state *crtc_state,
>> + struct drm_connector_state *conn_state,
>> + u32 output_fmt,
>> + unsigned int *num_input_fmts)
>> +{
>> + u32 *input_fmts;
>> +
>> + if (!samsung_dsim_pixel_output_fmt_supported(output_fmt))
>> + return NULL;
>
>
> Please add support for MEDIA_BUS_FMT_FIXED and maybe default to
> MEDIA_BUS_FMT_RGB888_1X24 if requested format is not matched.
>
> Otherwise the above check breaks all current clients of the Samsung
> DSIM/Exynos DSI. I didn't dig into the bus matching code yet, but all
> DSI panels requests such format on my test systems...
I've checked a bit more the bus format related code and it looks that
there are more issues. The DSI panels don't use the MEDIA_BUS_FMT_*
formats thus the bridge negotiation code selects MEDIA_BUS_FMT_FIXED for
them. On Arndale board with Toshiba tc358764 bridge the
MEDIA_BUS_FMT_RGB888_1X7X4_SPWG output_fmt is requested in
samsung_dsim_atomic_get_input_bus_fmts() (forwarded from the LVDS panel,
but this doesn't look like a format really needed on that bridge). A bit
more logic seems to be needed there to make it working properly.
I suggest to move all this bus format related changes into a separate
patchset and adjust other bridges/panels/etc as needed in it.
>
>> +
>> + *num_input_fmts = 1;
>> +
>> + input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
>> + if (!input_fmts)
>> + return NULL;
>> +
>> + input_fmts[0] = output_fmt;
>> +
>> + return input_fmts;
>> +}
>> +
>> static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
>> struct drm_bridge_state *bridge_state,
>> struct drm_crtc_state *crtc_state,
>> @@ -1385,6 +1437,7 @@ static const struct drm_bridge_funcs
>> samsung_dsim_bridge_funcs = {
>> .atomic_duplicate_state =
>> drm_atomic_helper_bridge_duplicate_state,
>> .atomic_destroy_state =
>> drm_atomic_helper_bridge_destroy_state,
>> .atomic_reset = drm_atomic_helper_bridge_reset,
>> + .atomic_get_input_bus_fmts =
>> samsung_dsim_atomic_get_input_bus_fmts,
>> .atomic_check = samsung_dsim_atomic_check,
>> .atomic_pre_enable = samsung_dsim_atomic_pre_enable,
>> .atomic_enable = samsung_dsim_atomic_enable,
>
> Best regards
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
Re: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On Mon, Nov 14, 2022 at 08:51:58PM +0800, Yi Liu wrote: > Our side, Yu He, Lixiao Yang has done below tests on Intel platform with > the above kernel, results are: > > 1) GVT-g test suit passed, Intel iGFx passthrough passed. > > 2) NIC passthrough test with different guest memory (1G/4G), passed. > > 3) Booting two different QEMUs in the same time but one QEMU opens > legacy /dev/vfio/vfio and another opens /dev/iommu. Tests passed. > > 4) Tried below Kconfig combinations, results are expected. > > VFIO_CONTAINER=y, IOMMUFD=y -- test pass > VFIO_CONTAINER=y, IOMMUFD=n -- test pass > VFIO_CONTAINER=n, IOMMUFD=y , IOMMUFD_VFIO_CONTAINER=y -- test pass > VFIO_CONTAINER=n, IOMMUFD=y , IOMMUFD_VFIO_CONTAINER=n -- no > /dev/vfio/vfio, so test fail, expected > > 5) Tested devices from multi-device group. Assign such devices to the same > VM, pass; assign them to different VMs, fail; assign them to a VM with Intel > virtual VT-d, fail; Results are expected. > > Meanwhile, I also tested the branch in development branch for nesting, > the basic functionality looks good. > > Tested-by: Yi Liu Great thanks! In future I also recommend running tests with the CONFIG_IOMMUFD_TEST turned on, it enables a bunch more fast path assertions that might catch something interesting Jason
RE: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On 2022/11/14 20:51, Yi Liu wrote: > On 2022/11/10 00:57, Jason Gunthorpe wrote: >> On Tue, Nov 08, 2022 at 11:18:03PM +0800, Yi Liu wrote: >>> On 2022/11/8 17:19, Nicolin Chen wrote: On Mon, Nov 07, 2022 at 08:52:44PM -0400, Jason Gunthorpe wrote: > This is on github: > https://github.com/jgunthorpe/linux/commits/vfio_iommufd [...] > v2: >- Rebase to v6.1-rc3, v4 iommufd series >- Fixup comments and commit messages from list remarks >- Fix leaking of the iommufd for mdevs >- New patch to fix vfio modaliases when vfio container is disabled >- Add a dmesg once when the iommufd provided /dev/vfio/vfio is opened > to signal that iommufd is providing this I've redone my previous sanity tests. Except those reported bugs, things look fine. Once we fix those issues, GVT and other modules can run some more stressful tests, I think. >>> >>> our side is also starting test (gvt, nic passthrough) this version. >>> need to wait a while for the result. >> >> I've updated the branches with the two functional fixes discussed on >> the list plus all the doc updates. >> > > I see, due to timzone, the kernel we grabbed is 37c9e6e44d77a, it has > slight diff in the scripts/kernel-doc compared with the latest commit > (6bb16a9c67769). I don't think it impacts the test. > > https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next > (37c9e6e44d77a) > > Our side, Yu He, Lixiao Yang has done below tests on Intel platform > with the above kernel, results are: > > 1) GVT-g test suit passed, Intel iGFx passthrough passed. > > 2) NIC passthrough test with different guest memory (1G/4G), passed. > > 3) Booting two different QEMUs in the same time but one QEMU opens > legacy /dev/vfio/vfio and another opens /dev/iommu. Tests passed. > > 4) Tried below Kconfig combinations, results are expected. > > VFIO_CONTAINER=y, IOMMUFD=y -- test pass > VFIO_CONTAINER=y, IOMMUFD=n -- test pass > VFIO_CONTAINER=n, IOMMUFD=y , IOMMUFD_VFIO_CONTAINER=y -- test pass > VFIO_CONTAINER=n, IOMMUFD=y , IOMMUFD_VFIO_CONTAINER=n -- no > /dev/vfio/vfio, so test fail, expected > > 5) Tested devices from multi-device group. Assign such devices to the > same VM, pass; assign them to different VMs, fail; assign them to a VM > with Intel virtual VT-d, fail; Results are expected. > > Meanwhile, I also tested the branch in development branch for nesting, > the basic functionality looks good. > > Tested-by: Yi Liu > Tested-by: Lixiao Yang -- Regards, Lixiao Yang
Re: [PATCH v2 00/11] Connect VFIO to IOMMUFD
On Thu, Nov 10, 2022 at 10:01:13PM -0500, Matthew Rosato wrote: > On 11/7/22 7:52 PM, Jason Gunthorpe wrote: > > This series provides an alternative container layer for VFIO implemented > > using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will > > not be compiled in. > > > > At this point iommufd can be injected by passing in a iommfd FD to > > VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd > > to obtain the compat IOAS and then connect up all the VFIO drivers as > > appropriate. > > > > This is temporary stopping point, a following series will provide a way to > > directly open a VFIO device FD and directly connect it to IOMMUFD using > > native ioctls that can expose the IOMMUFD features like hwpt, future > > vPASID and dynamic attachment. > > > > This series, in compat mode, has passed all the qemu tests we have > > available, including the test suites for the Intel GVT mdev. Aside from > > the temporary limitation with P2P memory this is belived to be fully > > compatible with VFIO. > > AFAICT there is no equivalent means to specify > vfio_iommu_type1.dma_entry_limit when using iommufd; looks like > we'll just always get the default 65535. No, there is no arbitary limit on iommufd > Was this because you envision the limit being not applicable for > iommufd (limits will be enforced via either means and eventually we > won't want to ) or was it an oversight? The limit here is primarily about limiting userspace abuse of the interface. iommufd is using GFP_KERNEL_ACCOUNT which shifts the responsiblity to cgroups, which is similar to how KVM works. So, for a VM sandbox you'd set a cgroup limit and if a hostile userspace in the sanbox decides to try to OOM the system it will hit that limit, regardless of which kernel APIs it tries to abuse. This work is not entirely complete as we also need the iommu driver to use GFP_KERNEL_ACCOUNT for allocations connected to the iommu_domain, particularly for allocations of the IO page tables themselves - which can be quite big. Jason
Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
Hi Geert,
On Mon, Nov 14, 2022 at 10:05:58AM +0100, Geert Uytterhoeven wrote:
> On Sun, Oct 2, 2022 at 12:06 AM Laurent Pinchart wrote:
> > When the R-Car MIPI DSI driver was added, it was a standalone encoder
> > driver without any dependency to or from the R-Car DU driver. Commit
> > 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence") then
> > added a direct call from the DU driver to the MIPI DSI driver, without
> > updating Kconfig to take the new dependency into account. Fix it the
> > same way that the LVDS encoder is handled.
> >
> > Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
> > Reported-by: kernel test robot
> > Signed-off-by: Laurent Pinchart
>
> Thanks for your patch, which is now commit a830a15678593948
> ("drm: rcar-du: Fix Kconfig dependency between RCAR_DU
> and RCAR_MIPI_DSI") in v6.1-rc5.
>
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -44,12 +44,17 @@ config DRM_RCAR_LVDS
> > select OF_FLATTREE
> > select OF_OVERLAY
> >
> > +config DRM_RCAR_USE_MIPI_DSI
> > + bool "R-Car DU MIPI DSI Encoder Support"
> > + depends on DRM_BRIDGE && OF
> > + default DRM_RCAR_DU
>
> This means this driver is now enabled by default on systems that do not
> have the MIPI DSI Encoder (e.g. R-Car Gen2), and that we should probably
> disable it explicitly in shmobile_defconfig. Is that intentional?
I don't think so, no. Would you like to send a patch ? If so, it should
enable the option in relevant defconfig files.
> > + help
> > + Enable support for the R-Car Display Unit embedded MIPI DSI
> > encoders.
> > +
> > config DRM_RCAR_MIPI_DSI
> > - tristate "R-Car DU MIPI DSI Encoder Support"
> > - depends on DRM && DRM_BRIDGE && OF
> > + def_tristate DRM_RCAR_DU
> > + depends on DRM_RCAR_USE_MIPI_DSI
> > select DRM_MIPI_DSI
> > - help
> > - Enable support for the R-Car Display Unit embedded MIPI DSI
> > encoders.
> >
> > config DRM_RCAR_VSP
> > bool "R-Car DU VSP Compositor Support" if ARM
--
Regards,
Laurent Pinchart
Re: [6.1][regression] after commit dd80d9c8eecac8c516da5b240d01a35660ba6cb6 some games (Cyberpunk 2077, Forza Horizon 4/5) hang at start
Hi Mikhail, Am 02.11.22 um 14:43 schrieb Christian König: Am 02.11.22 um 14:36 schrieb Mikhail Gavrilov: On Tue, Nov 1, 2022 at 10:52 PM Christian König wrote: [SNIP] But the most interesting thing is that all previous kernels 6.0, 5.19 are affected by the problem. It is not enough to revert the dd80d9c8eecac8c516da5b240d01a35660ba6cb6 commit. Yeah, that totally confirms what I expected. The context lock just hides the problem because userspace tended to use the same context. What the application now seems to do is to use multiple contexts for its submission and in this case re-adding the lock doesn't even help. Thanks for that information, gets me a lot closer to a solution. I've found and fixed a few problems around the userptr handling which might explain what you see here. A series of four patches starting with "drm/amdgpu: always register an MMU notifier for userptr" is under review now. Going to give that a bit cleanup later today and will CC you when I send that out. Would be nice if you could give that some testing. Thanks, Christian. Regards, Christian.
Re: [PATCH 1/2] drm/vc4: hdmi: Enforce the minimum rate at runtime_resume
Hi Stefan,
On Mon, Nov 14, 2022 at 01:48:14AM +0100, Stefan Wahren wrote:
> Am 11.11.22 um 22:08 schrieb Stefan Wahren:
> > Hi Maxime,
> >
> > Am 29.09.22 um 11:21 schrieb Maxime Ripard:
> > > This is a revert of commit fd5894fa2413 ("drm/vc4: hdmi: Remove clock
> > > rate initialization"), with the code slightly moved around.
> > >
> > > It turns out that we can't downright remove that code from the driver,
> > > since the Pi0-3 and Pi4 are in different cases, and it only works for
> > > the Pi4.
> > >
> > > Indeed, the commit mentioned above was relying on the RaspberryPi
> > > firmware clocks driver to initialize the rate if it wasn't done by the
> > > firmware. However, the Pi0-3 are using the clk-bcm2835 clock driver that
> > > wasn't doing this initialization. We therefore end up with the clock not
> > > being assigned a rate, and the CPU stalling when trying to access a
> > > register.
> > >
> > > We can't move that initialization in the clk-bcm2835 driver, since the
> > > HSM clock we depend on is actually part of the HDMI power domain, so any
> > > rate setup is only valid when the power domain is enabled. Thus, we
> > > reinstated the minimum rate setup at runtime_suspend, which should
> > > address both issues.
> > >
> > > Link:
> > > https://lore.kernel.org/dri-devel/20220922145448.w3xfywkn5ecak...@pengutronix.de/
> > > Fixes: fd5894fa2413 ("drm/vc4: hdmi: Remove clock rate initialization")
> > > Reported-by: Marc Kleine-Budde
> > > Signed-off-by: Maxime Ripard
> > > ---
> > > drivers/gpu/drm/vc4/vc4_hdmi.c | 9 +
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > index 199bc398817f..2e28fe16ed5e 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > @@ -2891,6 +2891,15 @@ static int vc4_hdmi_runtime_resume(struct
> > > device *dev)
> > > u32 __maybe_unused value;
> > > int ret;
> > > + /*
> > > + * The HSM clock is in the HDMI power domain, so we need to set
> > > + * its frequency while the power domain is active so that it
> > > + * keeps its rate.
> > > + */
> > > + ret = clk_set_min_rate(vc4_hdmi->hsm_clock, HSM_MIN_CLOCK_FREQ);
> > > + if (ret)
> > > + return ret;
> > > +
> >
> > unfortunately this breaks X on Raspberry Pi 4 in Linux 6.0.5
> > (multi_v7_defconfig + LPAE). Today i saw this report [1] and bisected
> > the issue down to this patch. Shame on me that i only tested this patch
> > with Rpi 3B+ :-(
>
> Looks like "drm/vc4: hdmi: Fix HSM clock too low on Pi4" addresses this
> issue ...
Yes, indeed. The fix should be on its way to -stable already
Maxime
signature.asc
Description: PGP signature
[PATCH v9 22/25] drm/vc4: vec: Check for VEC output constraints
From: Mateusz Kwiatkowski
The VEC can accept pretty much any relatively reasonable mode, but still
has a bunch of constraints to meet.
Let's create an atomic_check() implementation that will make sure we
don't end up accepting a non-functional mode.
Acked-by: Noralf Trønnes
Signed-off-by: Mateusz Kwiatkowski
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v6:
- Used htotal instead of vtotal to discriminate PAL against NTSC
---
drivers/gpu/drm/vc4/vc4_vec.c | 50 +++
1 file changed, 50 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index 90e375a8a8f9..bfa8a58dba30 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -453,6 +453,7 @@ static int vc4_vec_encoder_atomic_check(struct drm_encoder
*encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
{
+ const struct drm_display_mode *mode = _state->adjusted_mode;
const struct vc4_vec_tv_mode *vec_mode;
vec_mode = _vec_tv_modes[conn_state->tv.legacy_mode];
@@ -461,6 +462,55 @@ static int vc4_vec_encoder_atomic_check(struct drm_encoder
*encoder,
!drm_mode_equal(vec_mode->mode, _state->adjusted_mode))
return -EINVAL;
+ if (mode->crtc_hdisplay % 4)
+ return -EINVAL;
+
+ if (!(mode->crtc_hsync_end - mode->crtc_hsync_start))
+ return -EINVAL;
+
+ switch (mode->htotal) {
+ /* NTSC */
+ case 858:
+ if (mode->crtc_vtotal > 262)
+ return -EINVAL;
+
+ if (mode->crtc_vdisplay < 1 || mode->crtc_vdisplay > 253)
+ return -EINVAL;
+
+ if (!(mode->crtc_vsync_start - mode->crtc_vdisplay))
+ return -EINVAL;
+
+ if ((mode->crtc_vsync_end - mode->crtc_vsync_start) != 3)
+ return -EINVAL;
+
+ if ((mode->crtc_vtotal - mode->crtc_vsync_end) < 4)
+ return -EINVAL;
+
+ break;
+
+ /* PAL/SECAM */
+ case 864:
+ if (mode->crtc_vtotal > 312)
+ return -EINVAL;
+
+ if (mode->crtc_vdisplay < 1 || mode->crtc_vdisplay > 305)
+ return -EINVAL;
+
+ if (!(mode->crtc_vsync_start - mode->crtc_vdisplay))
+ return -EINVAL;
+
+ if ((mode->crtc_vsync_end - mode->crtc_vsync_start) != 3)
+ return -EINVAL;
+
+ if ((mode->crtc_vtotal - mode->crtc_vsync_end) < 2)
+ return -EINVAL;
+
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
return 0;
}
--
b4 0.11.0-dev-99e3a
[PATCH v9 20/25] drm/atomic-helper: Add an analog TV atomic_check implementation
The analog TV connector drivers share some atomic_check logic, and the new
TV standard property have created some boilerplate that can be be shared
across drivers too.
Let's create an atomic_check helper for those use cases.
Reviewed-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_atomic_state_helper.c | 49 +++
include/drm/drm_atomic_state_helper.h | 3 ++
2 files changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index e1fc3f26340a..3a467013c656 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -556,6 +556,55 @@ void drm_atomic_helper_connector_tv_reset(struct
drm_connector *connector)
}
EXPORT_SYMBOL(drm_atomic_helper_connector_tv_reset);
+/**
+ * @drm_atomic_helper_connector_tv_check: Validate an analog TV connector state
+ * @connector: DRM Connector
+ * @state: the DRM State object
+ *
+ * Checks the state object to see if the requested state is valid for an
+ * analog TV connector.
+ *
+ * Returns:
+ * Zero for success, a negative error code on error.
+ */
+int drm_atomic_helper_connector_tv_check(struct drm_connector *connector,
+struct drm_atomic_state *state)
+{
+ struct drm_connector_state *old_conn_state =
+ drm_atomic_get_old_connector_state(state, connector);
+ struct drm_connector_state *new_conn_state =
+ drm_atomic_get_new_connector_state(state, connector);
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+
+ crtc = new_conn_state->crtc;
+ if (!crtc)
+ return 0;
+
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ if (!crtc_state)
+ return -EINVAL;
+
+ if (old_conn_state->tv.mode != new_conn_state->tv.mode)
+ crtc_state->mode_changed = true;
+
+ if ((old_conn_state->tv.margins.left !=
new_conn_state->tv.margins.left) ||
+ (old_conn_state->tv.margins.right !=
new_conn_state->tv.margins.right) ||
+ (old_conn_state->tv.margins.top != new_conn_state->tv.margins.top)
||
+ (old_conn_state->tv.margins.bottom !=
new_conn_state->tv.margins.bottom) ||
+ (old_conn_state->tv.mode != new_conn_state->tv.mode) ||
+ (old_conn_state->tv.brightness != new_conn_state->tv.brightness) ||
+ (old_conn_state->tv.contrast != new_conn_state->tv.contrast) ||
+ (old_conn_state->tv.flicker_reduction !=
new_conn_state->tv.flicker_reduction) ||
+ (old_conn_state->tv.overscan != new_conn_state->tv.overscan) ||
+ (old_conn_state->tv.saturation != new_conn_state->tv.saturation) ||
+ (old_conn_state->tv.hue != new_conn_state->tv.hue))
+ crtc_state->connectors_changed = true;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_connector_tv_check);
+
/**
* __drm_atomic_helper_connector_duplicate_state - copy atomic connector state
* @connector: connector object
diff --git a/include/drm/drm_atomic_state_helper.h
b/include/drm/drm_atomic_state_helper.h
index c8fbce795ee7..b9740edb2658 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -26,6 +26,7 @@
#include
+struct drm_atomic_state;
struct drm_bridge;
struct drm_bridge_state;
struct drm_crtc;
@@ -71,6 +72,8 @@ void __drm_atomic_helper_connector_reset(struct drm_connector
*connector,
struct drm_connector_state
*conn_state);
void drm_atomic_helper_connector_reset(struct drm_connector *connector);
void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector);
+int drm_atomic_helper_connector_tv_check(struct drm_connector *connector,
+struct drm_atomic_state *state);
void drm_atomic_helper_connector_tv_margins_reset(struct drm_connector
*connector);
void
__drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
--
b4 0.11.0-dev-99e3a
[PATCH v9 08/25] drm/client: Add some tests for drm_connector_pick_cmdline_mode()
drm_connector_pick_cmdline_mode() is in charge of finding a proper
drm_display_mode from the definition we got in the video= command line
argument.
Let's add some unit tests to make sure we're not getting any regressions
there.
Acked-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v6:
- Rename tests to be consistent with DRM tests naming convention
Changes in v5:
- Removed useless (for now) count and modes intermediate variables in
get_modes
- Switched to kunit assertions in test init, and to KUNIT_ASSERT_NOT_NULL
instead of KUNIT_ASSERT_PTR_NE(..., NULL)
Changes in v4:
- Removed MODULE macros
---
drivers/gpu/drm/drm_client_modeset.c| 4 +
drivers/gpu/drm/tests/drm_client_modeset_test.c | 100
2 files changed, 104 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index bbc535cc50dd..d553e793e673 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -1237,3 +1237,7 @@ int drm_client_modeset_dpms(struct drm_client_dev
*client, int mode)
return ret;
}
EXPORT_SYMBOL(drm_client_modeset_dpms);
+
+#ifdef CONFIG_DRM_KUNIT_TEST
+#include "tests/drm_client_modeset_test.c"
+#endif
diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c
b/drivers/gpu/drm/tests/drm_client_modeset_test.c
new file mode 100644
index ..558c098b0384
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 Maxime Ripard
+ */
+
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "drm_kunit_helpers.h"
+
+struct drm_client_modeset_test_priv {
+ struct drm_device *drm;
+ struct drm_connector connector;
+};
+
+static int drm_client_modeset_connector_get_modes(struct drm_connector
*connector)
+{
+ return drm_add_modes_noedid(connector, 1920, 1200);
+}
+
+static const struct drm_connector_helper_funcs
drm_client_modeset_connector_helper_funcs = {
+ .get_modes = drm_client_modeset_connector_get_modes,
+};
+
+static const struct drm_connector_funcs drm_client_modeset_connector_funcs = {
+};
+
+static int drm_client_modeset_test_init(struct kunit *test)
+{
+ struct drm_client_modeset_test_priv *priv;
+ int ret;
+
+ priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ test->priv = priv;
+
+ priv->drm = drm_kunit_device_init(test, DRIVER_MODESET,
"drm-client-modeset-test");
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm);
+
+ ret = drmm_connector_init(priv->drm, >connector,
+ _client_modeset_connector_funcs,
+ DRM_MODE_CONNECTOR_Unknown,
+ NULL);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ drm_connector_helper_add(>connector,
_client_modeset_connector_helper_funcs);
+
+ return 0;
+
+}
+
+static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test)
+{
+ struct drm_client_modeset_test_priv *priv = test->priv;
+ struct drm_device *drm = priv->drm;
+ struct drm_connector *connector = >connector;
+ struct drm_cmdline_mode *cmdline_mode = >cmdline_mode;
+ struct drm_display_mode *expected_mode, *mode;
+ const char *cmdline = "1920x1080@60";
+ int ret;
+
+ expected_mode = drm_mode_find_dmt(priv->drm, 1920, 1080, 60, false);
+ KUNIT_ASSERT_NOT_NULL(test, expected_mode);
+
+ KUNIT_ASSERT_TRUE(test,
+ drm_mode_parse_command_line_for_connector(cmdline,
+ connector,
+
cmdline_mode));
+
+ mutex_lock(>mode_config.mutex);
+ ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080);
+ mutex_unlock(>mode_config.mutex);
+ KUNIT_ASSERT_GT(test, ret, 0);
+
+ mode = drm_connector_pick_cmdline_mode(connector);
+ KUNIT_ASSERT_NOT_NULL(test, mode);
+
+ 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),
+ {}
+};
+
+static struct kunit_suite drm_test_pick_cmdline_test_suite = {
+ .name = "drm_test_pick_cmdline",
+ .init = drm_client_modeset_test_init,
+ .test_cases = drm_test_pick_cmdline_tests
+};
+
+kunit_test_suite(drm_test_pick_cmdline_test_suite);
--
b4 0.11.0-dev-99e3a
[PATCH v9 23/25] drm/vc4: vec: Convert to the new TV mode property
Now that the core can deal fine with analog TV modes, let's convert the vc4
VEC driver to leverage those new features.
We've added some backward compatibility to support the old TV mode property
and translate it into the new TV norm property. We're also making use of
the new analog TV atomic_check helper to make sure we trigger a modeset
whenever the TV mode is updated.
Acked-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v7:
- Lookup the tv mode in atomic_check to make sure it's supported
Changes in v6:
- Use new get_modes helper
Changes in v5:
- Renamed tv_mode_names into legacy_tv_mode_names
Changes in v4:
- Removed the count variable in .get_modes
---
drivers/gpu/drm/vc4/vc4_vec.c | 186 ++
1 file changed, 132 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index bfa8a58dba30..a828fc6fb776 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -172,6 +172,8 @@ struct vc4_vec {
struct clk *clock;
+ struct drm_property *legacy_tv_mode_property;
+
struct debugfs_regset32 regset;
};
@@ -184,6 +186,12 @@ encoder_to_vc4_vec(struct drm_encoder *encoder)
return container_of(encoder, struct vc4_vec, encoder.base);
}
+static inline struct vc4_vec *
+connector_to_vc4_vec(struct drm_connector *connector)
+{
+ return container_of(connector, struct vc4_vec, connector);
+}
+
enum vc4_vec_tv_mode_id {
VC4_VEC_TV_MODE_NTSC,
VC4_VEC_TV_MODE_NTSC_J,
@@ -192,7 +200,7 @@ enum vc4_vec_tv_mode_id {
};
struct vc4_vec_tv_mode {
- const struct drm_display_mode *mode;
+ unsigned int mode;
u32 config0;
u32 config1;
u32 custom_freq;
@@ -225,43 +233,51 @@ static const struct debugfs_reg32 vec_regs[] = {
VC4_REG32(VEC_DAC_MISC),
};
-static const struct drm_display_mode ntsc_mode = {
- DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 13500,
-720, 720 + 14, 720 + 14 + 64, 720 + 14 + 64 + 60, 0,
-480, 480 + 7, 480 + 7 + 6, 525, 0,
-DRM_MODE_FLAG_INTERLACE)
-};
-
-static const struct drm_display_mode pal_mode = {
- DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 13500,
-720, 720 + 20, 720 + 20 + 64, 720 + 20 + 64 + 60, 0,
-576, 576 + 4, 576 + 4 + 6, 625, 0,
-DRM_MODE_FLAG_INTERLACE)
-};
-
static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = {
- [VC4_VEC_TV_MODE_NTSC] = {
- .mode = _mode,
+ {
+ .mode = DRM_MODE_TV_MODE_NTSC,
.config0 = VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
- [VC4_VEC_TV_MODE_NTSC_J] = {
- .mode = _mode,
+ {
+ .mode = DRM_MODE_TV_MODE_NTSC_J,
.config0 = VEC_CONFIG0_NTSC_STD,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
- [VC4_VEC_TV_MODE_PAL] = {
- .mode = _mode,
+ {
+ .mode = DRM_MODE_TV_MODE_PAL,
.config0 = VEC_CONFIG0_PAL_BDGHI_STD,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
- [VC4_VEC_TV_MODE_PAL_M] = {
- .mode = _mode,
+ {
+ .mode = DRM_MODE_TV_MODE_PAL_M,
.config0 = VEC_CONFIG0_PAL_M_STD,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
};
+static inline const struct vc4_vec_tv_mode *
+vc4_vec_tv_mode_lookup(unsigned int mode)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(vc4_vec_tv_modes); i++) {
+ const struct vc4_vec_tv_mode *tv_mode = _vec_tv_modes[i];
+
+ if (tv_mode->mode == mode)
+ return tv_mode;
+ }
+
+ return NULL;
+}
+
+static const struct drm_prop_enum_list legacy_tv_mode_names[] = {
+ { VC4_VEC_TV_MODE_NTSC, "NTSC", },
+ { VC4_VEC_TV_MODE_NTSC_J, "NTSC-J", },
+ { VC4_VEC_TV_MODE_PAL, "PAL", },
+ { VC4_VEC_TV_MODE_PAL_M, "PAL-M", },
+};
+
static enum drm_connector_status
vc4_vec_connector_detect(struct drm_connector *connector, bool force)
{
@@ -274,21 +290,74 @@ static void vc4_vec_connector_reset(struct drm_connector
*connector)
drm_atomic_helper_connector_tv_reset(connector);
}
-static int vc4_vec_connector_get_modes(struct drm_connector *connector)
+static int
+vc4_vec_connector_set_property(struct drm_connector *connector,
+ struct drm_connector_state *state,
+ struct drm_property *property,
+ uint64_t val)
{
- struct drm_connector_state *state = connector->state;
- struct drm_display_mode *mode;
-
- mode = drm_mode_duplicate(connector->dev,
- vc4_vec_tv_modes[state->tv.legacy_mode].mode);
- if (!mode) {
-
[PATCH v9 24/25] drm/vc4: vec: Add support for more analog TV standards
From: Mateusz Kwiatkowski
Add support for the following composite output modes (all of them are
somewhat more obscure than the previously defined ones):
- NTSC_443 - NTSC-style signal with the chroma subcarrier shifted to
4.43361875 MHz (the PAL subcarrier frequency). Never used for
broadcasting, but sometimes used as a hack to play NTSC content in PAL
regions (e.g. on VCRs).
- PAL_N - PAL with alternative chroma subcarrier frequency,
3.58205625 MHz. Used as a broadcast standard in Argentina, Paraguay
and Uruguay to fit 576i50 with colour in 6 MHz channel raster.
- PAL60 - 480i60 signal with PAL-style color at normal European PAL
frequency. Another non-standard, non-broadcast mode, used in similar
contexts as NTSC_443. Some displays support one but not the other.
- SECAM - French frequency-modulated analog color standard; also have
been broadcast in Eastern Europe and various parts of Africa and Asia.
Uses the same 576i50 timings as PAL.
Also added some comments explaining color subcarrier frequency
registers.
Acked-by: Noralf Trønnes
Signed-off-by: Mateusz Kwiatkowski
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v6:
- Support PAL60 again
---
drivers/gpu/drm/vc4/vc4_vec.c | 111 --
1 file changed, 107 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index a828fc6fb776..d23dbad3cbf6 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -46,6 +46,7 @@
#define VEC_CONFIG0_YDEL(x)((x) << 26)
#define VEC_CONFIG0_CDEL_MASK GENMASK(25, 24)
#define VEC_CONFIG0_CDEL(x)((x) << 24)
+#define VEC_CONFIG0_SECAM_STD BIT(21)
#define VEC_CONFIG0_PBPR_FIL BIT(18)
#define VEC_CONFIG0_CHROMA_GAIN_MASK GENMASK(17, 16)
#define VEC_CONFIG0_CHROMA_GAIN_UNITY (0 << 16)
@@ -76,6 +77,27 @@
#define VEC_SOFT_RESET 0x10c
#define VEC_CLMP0_START0x144
#define VEC_CLMP0_END 0x148
+
+/*
+ * These set the color subcarrier frequency
+ * if VEC_CONFIG1_CUSTOM_FREQ is enabled.
+ *
+ * VEC_FREQ1_0 contains the most significant 16-bit half-word,
+ * VEC_FREQ3_2 contains the least significant 16-bit half-word.
+ * 0x8000 seems to be equivalent to the pixel clock
+ * (which itself is the VEC clock divided by 8).
+ *
+ * Reference values (with the default pixel clock of 13.5 MHz):
+ *
+ * NTSC (3579545.[45] Hz) - 0x21F07C1F
+ * PAL (4433618.75 Hz) - 0x2A098ACB
+ * PAL-M (3575611.[888111] Hz) - 0x21E6EFE3
+ * PAL-N (3582056.25 Hz) - 0x21F69446
+ *
+ * NOTE: For SECAM, it is used as the Dr center frequency,
+ * regardless of whether VEC_CONFIG1_CUSTOM_FREQ is enabled or not;
+ * that is specified as 4406250 Hz, which corresponds to 0x29C71C72.
+ */
#define VEC_FREQ3_20x180
#define VEC_FREQ1_00x184
@@ -118,6 +140,14 @@
#define VEC_INTERRUPT_CONTROL 0x190
#define VEC_INTERRUPT_STATUS 0x194
+
+/*
+ * Db center frequency for SECAM; the clock for this is the same as for
+ * VEC_FREQ3_2/VEC_FREQ1_0, which is used for Dr center frequency.
+ *
+ * This is specified as 425 Hz, which corresponds to 0x284BDA13.
+ * That is also the default value, so no need to set it explicitly.
+ */
#define VEC_FCW_SECAM_B0x198
#define VEC_SECAM_GAIN_VAL 0x19c
@@ -197,10 +227,15 @@ enum vc4_vec_tv_mode_id {
VC4_VEC_TV_MODE_NTSC_J,
VC4_VEC_TV_MODE_PAL,
VC4_VEC_TV_MODE_PAL_M,
+ VC4_VEC_TV_MODE_NTSC_443,
+ VC4_VEC_TV_MODE_PAL_60,
+ VC4_VEC_TV_MODE_PAL_N,
+ VC4_VEC_TV_MODE_SECAM,
};
struct vc4_vec_tv_mode {
unsigned int mode;
+ u16 expected_htotal;
u32 config0;
u32 config1;
u32 custom_freq;
@@ -236,35 +271,68 @@ static const struct debugfs_reg32 vec_regs[] = {
static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = {
{
.mode = DRM_MODE_TV_MODE_NTSC,
+ .expected_htotal = 858,
.config0 = VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
+ {
+ .mode = DRM_MODE_TV_MODE_NTSC_443,
+ .expected_htotal = 858,
+ .config0 = VEC_CONFIG0_NTSC_STD,
+ .config1 = VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ,
+ .custom_freq = 0x2a098acb,
+ },
{
.mode = DRM_MODE_TV_MODE_NTSC_J,
+ .expected_htotal = 858,
.config0 = VEC_CONFIG0_NTSC_STD,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
{
.mode = DRM_MODE_TV_MODE_PAL,
+ .expected_htotal = 864,
.config0 = VEC_CONFIG0_PAL_BDGHI_STD,
.config1 = VEC_CONFIG1_C_CVBS_CVBS,
},
+ {
+
[PATCH v9 19/25] drm/atomic-helper: Add a TV properties reset helper
The drm_tv_create_properties() function will create a bunch of properties,
but it's up to each and every driver using that function to properly reset
the state of these properties leading to inconsistent behaviours.
Let's create a helper that will take care of it.
Reviewed-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v6:
- Use tv_mode_specified instead of a !0 tv_mode to set the default
---
drivers/gpu/drm/drm_atomic_state_helper.c | 75 +++
include/drm/drm_atomic_state_helper.h | 1 +
2 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index dfb57217253b..e1fc3f26340a 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -481,6 +481,81 @@ void drm_atomic_helper_connector_tv_margins_reset(struct
drm_connector *connecto
}
EXPORT_SYMBOL(drm_atomic_helper_connector_tv_margins_reset);
+/**
+ * drm_atomic_helper_connector_tv_reset - Resets Analog TV connector properties
+ * @connector: DRM connector
+ *
+ * Resets the analog TV properties attached to a connector
+ */
+void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_cmdline_mode *cmdline = >cmdline_mode;
+ struct drm_connector_state *state = connector->state;
+ struct drm_property *prop;
+ uint64_t val;
+
+ prop = dev->mode_config.tv_mode_property;
+ if (prop)
+ if (!drm_object_property_get_default_value(>base,
+ prop, ))
+ state->tv.mode = val;
+
+ if (cmdline->tv_mode_specified)
+ state->tv.mode = cmdline->tv_mode;
+
+ prop = dev->mode_config.tv_select_subconnector_property;
+ if (prop)
+ if (!drm_object_property_get_default_value(>base,
+ prop, ))
+ state->tv.select_subconnector = val;
+
+ prop = dev->mode_config.tv_subconnector_property;
+ if (prop)
+ if (!drm_object_property_get_default_value(>base,
+ prop, ))
+ state->tv.subconnector = val;
+
+ prop = dev->mode_config.tv_brightness_property;
+ if (prop)
+ if (!drm_object_property_get_default_value(>base,
+ prop, ))
+ state->tv.brightness = val;
+
+ prop = dev->mode_config.tv_contrast_property;
+ if (prop)
+ if (!drm_object_property_get_default_value(>base,
+ prop, ))
+ state->tv.contrast = val;
+
+ prop = dev->mode_config.tv_flicker_reduction_property;
+ if (prop)
+ if (!drm_object_property_get_default_value(>base,
+ prop, ))
+ state->tv.flicker_reduction = val;
+
+ prop = dev->mode_config.tv_overscan_property;
+ if (prop)
+ if (!drm_object_property_get_default_value(>base,
+ prop, ))
+ state->tv.overscan = val;
+
+ prop = dev->mode_config.tv_saturation_property;
+ if (prop)
+ if (!drm_object_property_get_default_value(>base,
+ prop, ))
+ state->tv.saturation = val;
+
+ prop = dev->mode_config.tv_hue_property;
+ if (prop)
+ if (!drm_object_property_get_default_value(>base,
+ prop, ))
+ state->tv.hue = val;
+
+ drm_atomic_helper_connector_tv_margins_reset(connector);
+}
+EXPORT_SYMBOL(drm_atomic_helper_connector_tv_reset);
+
/**
* __drm_atomic_helper_connector_duplicate_state - copy atomic connector state
* @connector: connector object
diff --git a/include/drm/drm_atomic_state_helper.h
b/include/drm/drm_atomic_state_helper.h
index 192766656b88..c8fbce795ee7 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -70,6 +70,7 @@ void __drm_atomic_helper_connector_state_reset(struct
drm_connector_state *conn_
void __drm_atomic_helper_connector_reset(struct drm_connector *connector,
struct drm_connector_state
*conn_state);
void drm_atomic_helper_connector_reset(struct drm_connector *connector);
+void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector);
void drm_atomic_helper_connector_tv_margins_reset(struct drm_connector
*connector);
void
__drm_atomic_helper_connector_duplicate_state(struct drm_connector
[PATCH v9 25/25] drm/sun4i: tv: Convert to the new TV mode property
Now that the core can deal fine with analog TV modes, let's convert the
sun4i TV driver to leverage those new features.
Acked-by: Noralf Trønnes
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
Changes in v6:
- Convert to new get_modes helper
Changes in v5:
- Removed the count variable in get_modes
- Removed spurious vc4 change
---
drivers/gpu/drm/sun4i/sun4i_tv.c | 141 ++-
1 file changed, 34 insertions(+), 107 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
index c65f0a89b6b0..9625a00a48ba 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
@@ -141,23 +141,14 @@ struct resync_parameters {
struct tv_mode {
char*name;
+ unsigned inttv_mode;
+
u32 mode;
u32 chroma_freq;
u16 back_porch;
u16 front_porch;
- u16 line_number;
u16 vblank_level;
- u32 hdisplay;
- u16 hfront_porch;
- u16 hsync_len;
- u16 hback_porch;
-
- u32 vdisplay;
- u16 vfront_porch;
- u16 vsync_len;
- u16 vback_porch;
-
boolyc_en;
booldac3_en;
booldac_bit25_en;
@@ -213,7 +204,7 @@ static const struct resync_parameters pal_resync_parameters
= {
static const struct tv_mode tv_modes[] = {
{
- .name = "NTSC",
+ .tv_mode= DRM_MODE_TV_MODE_NTSC,
.mode = SUN4I_TVE_CFG0_RES_480i,
.chroma_freq= 0x21f07c1f,
.yc_en = true,
@@ -222,17 +213,6 @@ static const struct tv_mode tv_modes[] = {
.back_porch = 118,
.front_porch= 32,
- .line_number= 525,
-
- .hdisplay = 720,
- .hfront_porch = 18,
- .hsync_len = 2,
- .hback_porch= 118,
-
- .vdisplay = 480,
- .vfront_porch = 26,
- .vsync_len = 2,
- .vback_porch= 17,
.vblank_level = 240,
@@ -242,23 +222,12 @@ static const struct tv_mode tv_modes[] = {
.resync_params = _resync_parameters,
},
{
- .name = "PAL",
+ .tv_mode= DRM_MODE_TV_MODE_PAL,
.mode = SUN4I_TVE_CFG0_RES_576i,
.chroma_freq= 0x2a098acb,
.back_porch = 138,
.front_porch= 24,
- .line_number= 625,
-
- .hdisplay = 720,
- .hfront_porch = 3,
- .hsync_len = 2,
- .hback_porch= 139,
-
- .vdisplay = 576,
- .vfront_porch = 28,
- .vsync_len = 2,
- .vback_porch= 19,
.vblank_level = 252,
@@ -276,63 +245,21 @@ drm_encoder_to_sun4i_tv(struct drm_encoder *encoder)
encoder);
}
-/*
- * FIXME: If only the drm_display_mode private field was usable, this
- * could go away...
- *
- * So far, it doesn't seem to be preserved when the mode is passed by
- * to mode_set for some reason.
- */
-static const struct tv_mode *sun4i_tv_find_tv_by_mode(const struct
drm_display_mode *mode)
+static const struct tv_mode *
+sun4i_tv_find_tv_by_mode(unsigned int mode)
{
int i;
- /* First try to identify the mode by name */
for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
const struct tv_mode *tv_mode = _modes[i];
- DRM_DEBUG_DRIVER("Comparing mode %s vs %s",
-mode->name, tv_mode->name);
-
- if (!strcmp(mode->name, tv_mode->name))
- return tv_mode;
- }
-
- /* Then by number of lines */
- for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
- const struct tv_mode *tv_mode = _modes[i];
-
- DRM_DEBUG_DRIVER("Comparing mode %s vs %s (X: %d vs %d)",
-mode->name, tv_mode->name,
-mode->vdisplay, tv_mode->vdisplay);
-
- if (mode->vdisplay == tv_mode->vdisplay)
+ if (tv_mode->tv_mode == mode)
return tv_mode;
}
return NULL;
}
-static void sun4i_tv_mode_to_drm_mode(const struct tv_mode *tv_mode,
- struct drm_display_mode *mode)
-{
- DRM_DEBUG_DRIVER("Creating mode %s\n", mode->name);
-
- mode->type = DRM_MODE_TYPE_DRIVER;
- mode->clock = 13500;
- mode->flags = DRM_MODE_FLAG_INTERLACE;
-
- mode->hdisplay = tv_mode->hdisplay;
-
[PATCH v9 15/25] drm/modes: Properly generate a drm_display_mode from a named mode
The framework will get the drm_display_mode from the drm_cmdline_mode it
got by parsing the video command line argument by calling
drm_connector_pick_cmdline_mode().
The heavy lifting will then be done by the drm_mode_create_from_cmdline_mode()
function.
In the case of the named modes though, there's no real code to make that
translation and we rely on the drivers to guess which actual display mode
we meant.
Let's modify drm_mode_create_from_cmdline_mode() to properly generate the
drm_display_mode we mean when passing a named mode.
Reviewed-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v9:
- Switch to parameterized tests for named modes
Changes in v8:
- Return the result of drm_analog_tv_mode directly
Changes in v7:
- Use tv_mode_specified in drm_mode_parse_command_line_for_connector
Changes in v6:
- Fix get_modes to return 0 instead of an error code
- Rename the tests to follow the DRM test naming convention
Changes in v5:
- Switched to KUNIT_ASSERT_NOT_NULL
---
drivers/gpu/drm/drm_modes.c | 29 -
drivers/gpu/drm/tests/drm_client_modeset_test.c | 83 -
2 files changed, 110 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index dc037f7ceb37..d3f0a3559812 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -2497,6 +2497,31 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
}
EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector);
+static struct drm_display_mode *drm_named_mode(struct drm_device *dev,
+ struct drm_cmdline_mode *cmd)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(drm_named_modes); i++) {
+ const struct drm_named_mode *named_mode = _named_modes[i];
+
+ if (strcmp(cmd->name, named_mode->name))
+ continue;
+
+ if (!cmd->tv_mode_specified)
+ continue;
+
+ return drm_analog_tv_mode(dev,
+ named_mode->tv_mode,
+ named_mode->pixel_clock_khz * 1000,
+ named_mode->xres,
+ named_mode->yres,
+ named_mode->flags &
DRM_MODE_FLAG_INTERLACE);
+ }
+
+ return NULL;
+}
+
/**
* drm_mode_create_from_cmdline_mode - convert a command line modeline into a
DRM display mode
* @dev: DRM device to create the new mode for
@@ -2514,7 +2539,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
if (cmd->xres == 0 || cmd->yres == 0)
return NULL;
- if (cmd->cvt)
+ if (strlen(cmd->name))
+ mode = drm_named_mode(dev, cmd);
+ else if (cmd->cvt)
mode = drm_cvt_mode(dev,
cmd->xres, cmd->yres,
cmd->refresh_specified ? cmd->refresh : 60,
diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c
b/drivers/gpu/drm/tests/drm_client_modeset_test.c
index 558c098b0384..497434cc56cd 100644
--- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
+++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
@@ -21,7 +21,26 @@ struct drm_client_modeset_test_priv {
static int drm_client_modeset_connector_get_modes(struct drm_connector
*connector)
{
- return drm_add_modes_noedid(connector, 1920, 1200);
+ struct drm_display_mode *mode;
+ int count;
+
+ count = drm_add_modes_noedid(connector, 1920, 1200);
+
+ mode = drm_mode_analog_ntsc_480i(connector->dev);
+ if (!mode)
+ return count;
+
+ drm_mode_probed_add(connector, mode);
+ count += 1;
+
+ mode = drm_mode_analog_pal_576i(connector->dev);
+ if (!mode)
+ return count;
+
+ drm_mode_probed_add(connector, mode);
+ count += 1;
+
+ return count;
}
static const struct drm_connector_helper_funcs
drm_client_modeset_connector_helper_funcs = {
@@ -52,6 +71,9 @@ static int drm_client_modeset_test_init(struct kunit *test)
drm_connector_helper_add(>connector,
_client_modeset_connector_helper_funcs);
+ priv->connector.interlace_allowed = true;
+ priv->connector.doublescan_allowed = true;
+
return 0;
}
@@ -85,9 +107,68 @@ static void drm_test_pick_cmdline_res_1920_1080_60(struct
kunit *test)
KUNIT_EXPECT_TRUE(test, drm_mode_equal(expected_mode, mode));
}
+struct drm_connector_pick_cmdline_mode_test {
+ const char *cmdline;
+ struct drm_display_mode *(*func)(struct drm_device *);
+};
+
+#define TEST_CMDLINE(_cmdline, _fn)\
+ { \
+ .cmdline = _cmdline,\
+ .func = _fn,\
+ }
+
[PATCH v9 21/25] drm/vc4: vec: Use TV Reset implementation
The analog TV properties created by the drm_mode_create_tv_properties() are
not properly initialised at reset. Let's switch our implementation to call
drm_atomic_helper_connector_tv_reset().
Reviewed-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_vec.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index adc9bf99e3fd..90e375a8a8f9 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -268,6 +268,12 @@ vc4_vec_connector_detect(struct drm_connector *connector,
bool force)
return connector_status_unknown;
}
+static void vc4_vec_connector_reset(struct drm_connector *connector)
+{
+ drm_atomic_helper_connector_reset(connector);
+ drm_atomic_helper_connector_tv_reset(connector);
+}
+
static int vc4_vec_connector_get_modes(struct drm_connector *connector)
{
struct drm_connector_state *state = connector->state;
@@ -288,7 +294,7 @@ static int vc4_vec_connector_get_modes(struct drm_connector
*connector)
static const struct drm_connector_funcs vc4_vec_connector_funcs = {
.detect = vc4_vec_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
- .reset = drm_atomic_helper_connector_reset,
+ .reset = vc4_vec_connector_reset,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
--
b4 0.11.0-dev-99e3a
[PATCH v9 17/25] drm/modes: Introduce more named modes
Now that we can easily extend the named modes list, let's add a few more
analog TV modes that were used in the wild, and some unit tests to make
sure it works as intended.
Reviewed-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v9:
- Document the new supported names
Changes in v6:
- Renamed the tests to follow DRM test naming convention
Changes in v5:
- Switched to KUNIT_ASSERT_NOT_NULL
---
Documentation/fb/modedb.rst | 3 +++
drivers/gpu/drm/drm_modes.c | 2 ++
drivers/gpu/drm/tests/drm_client_modeset_test.c | 2 ++
3 files changed, 7 insertions(+)
diff --git a/Documentation/fb/modedb.rst b/Documentation/fb/modedb.rst
index bebfe61caa77..bb2889c6ea27 100644
--- a/Documentation/fb/modedb.rst
+++ b/Documentation/fb/modedb.rst
@@ -29,7 +29,10 @@ Things between square brackets are optional.
Valid names are::
- NSTC: 480i output, with the CCIR System-M TV mode and NTSC color encoding
+ - NTSC-J: 480i output, with the CCIR System-M TV mode, the NTSC color
+encoding, and a black level equal to the blanking level.
- PAL: 576i output, with the CCIR System-B TV mode and PAL color encoding
+ - PAL-M: 480i output, with the CCIR System-M TV mode and PAL color encoding
If 'M' is specified in the mode_option argument (after and before
and , if specified) the timings will be calculated using
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index d3f0a3559812..855569a269b8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -2272,7 +2272,9 @@ struct drm_named_mode {
static const struct drm_named_mode drm_named_modes[] = {
NAMED_MODE("NTSC", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE,
DRM_MODE_TV_MODE_NTSC),
+ NAMED_MODE("NTSC-J", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE,
DRM_MODE_TV_MODE_NTSC_J),
NAMED_MODE("PAL", 13500, 720, 576, DRM_MODE_FLAG_INTERLACE,
DRM_MODE_TV_MODE_PAL),
+ NAMED_MODE("PAL-M", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE,
DRM_MODE_TV_MODE_PAL_M),
};
static int drm_mode_parse_cmdline_named_mode(const char *name,
diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c
b/drivers/gpu/drm/tests/drm_client_modeset_test.c
index 497434cc56cd..f2e18392a953 100644
--- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
+++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
@@ -151,7 +151,9 @@ static void drm_test_pick_cmdline_named(struct kunit *test)
static const
struct drm_connector_pick_cmdline_mode_test
drm_connector_pick_cmdline_mode_tests[] = {
TEST_CMDLINE("NTSC", drm_mode_analog_ntsc_480i),
+ TEST_CMDLINE("NTSC-J", drm_mode_analog_ntsc_480i),
TEST_CMDLINE("PAL", drm_mode_analog_pal_576i),
+ TEST_CMDLINE("PAL-M", drm_mode_analog_ntsc_480i),
};
static void
--
b4 0.11.0-dev-99e3a
[PATCH v9 10/25] drm/modes: Switch to named mode descriptors
The current named mode parsing relies only on the mode name, and doesn't
allow to specify any other parameter.
Let's convert that string list to an array of a custom structure that will
hold the name and some additional parameters in the future.
Reviewed-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v7:
- Fix typo in the commit log
- Add Noralf Reviewed-by
---
drivers/gpu/drm/drm_modes.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 37542612912b..7594b657f86a 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -2224,9 +2224,13 @@ static int drm_mode_parse_cmdline_options(const char
*str,
return 0;
}
-static const char * const drm_named_modes_whitelist[] = {
- "NTSC",
- "PAL",
+struct drm_named_mode {
+ const char *name;
+};
+
+static const struct drm_named_mode drm_named_modes[] = {
+ { "NTSC", },
+ { "PAL", },
};
static int drm_mode_parse_cmdline_named_mode(const char *name,
@@ -2258,14 +2262,15 @@ static int drm_mode_parse_cmdline_named_mode(const char
*name,
* We're sure we're a named mode at this point, iterate over the
* list of modes we're aware of.
*/
- for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
+ for (i = 0; i < ARRAY_SIZE(drm_named_modes); i++) {
+ const struct drm_named_mode *mode = _named_modes[i];
int ret;
- ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
+ ret = str_has_prefix(name, mode->name);
if (ret != name_end)
continue;
- strcpy(cmdline_mode->name, drm_named_modes_whitelist[i]);
+ strcpy(cmdline_mode->name, mode->name);
cmdline_mode->specified = true;
return 1;
--
b4 0.11.0-dev-99e3a
[PATCH v9 12/25] drm/connector: Add pixel clock to cmdline mode
We'll need to get the pixel clock to generate proper display modes for
all the current named modes. Let's add it to struct drm_cmdline_mode and
fill it when parsing the named mode.
Reviewed-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v7:
- Add Noralf Reviewed-by:w
---
drivers/gpu/drm/drm_modes.c | 9 ++---
include/drm/drm_connector.h | 7 +++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index acee23e1a8b7..c826f9583a1d 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -2226,22 +2226,24 @@ static int drm_mode_parse_cmdline_options(const char
*str,
struct drm_named_mode {
const char *name;
+ unsigned int pixel_clock_khz;
unsigned int xres;
unsigned int yres;
unsigned int flags;
};
-#define NAMED_MODE(_name, _x, _y, _flags) \
+#define NAMED_MODE(_name, _pclk, _x, _y, _flags) \
{ \
.name = _name, \
+ .pixel_clock_khz = _pclk, \
.xres = _x, \
.yres = _y, \
.flags = _flags,\
}
static const struct drm_named_mode drm_named_modes[] = {
- NAMED_MODE("NTSC", 720, 480, DRM_MODE_FLAG_INTERLACE),
- NAMED_MODE("PAL", 720, 576, DRM_MODE_FLAG_INTERLACE),
+ NAMED_MODE("NTSC", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE),
+ NAMED_MODE("PAL", 13500, 720, 576, DRM_MODE_FLAG_INTERLACE),
};
static int drm_mode_parse_cmdline_named_mode(const char *name,
@@ -2282,6 +2284,7 @@ static int drm_mode_parse_cmdline_named_mode(const char
*name,
continue;
strcpy(cmdline_mode->name, mode->name);
+ cmdline_mode->pixel_clock = mode->pixel_clock_khz;
cmdline_mode->xres = mode->xres;
cmdline_mode->yres = mode->yres;
cmdline_mode->interlace = !!(mode->flags &
DRM_MODE_FLAG_INTERLACE);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 9afc7956fdc6..4927dcb2573f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1273,6 +1273,13 @@ struct drm_cmdline_mode {
*/
bool bpp_specified;
+ /**
+* @pixel_clock:
+*
+* Pixel Clock in kHz. Optional.
+*/
+ unsigned int pixel_clock;
+
/**
* @xres:
*
--
b4 0.11.0-dev-99e3a
[PATCH v9 18/25] drm/probe-helper: Provide a TV get_modes helper
From: Noralf Trønnes
Most of the TV connectors will need a similar get_modes implementation
that will, depending on the drivers' capabilities, register the 480i and
576i modes.
That implementation will also need to set the preferred flag and order
the modes based on the driver and users preferrence.
This is especially important to guarantee that a userspace stack such as
Xorg can start and pick up the preferred mode while maintaining a
working output.
Signed-off-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v9:
- Store a function pointer instead of duplicating the expected mode
- Switch to kunit_test_suite
Changes in v8:
- Remove unused tv_mode_support function
- Add unit tests
Changes in v7:
- Used Noralf's implementation
Changes in v6:
- New patch
---
drivers/gpu/drm/drm_probe_helper.c| 82 +++
drivers/gpu/drm/tests/Makefile| 1 +
drivers/gpu/drm/tests/drm_probe_helper_test.c | 202 ++
include/drm/drm_probe_helper.h| 1 +
4 files changed, 286 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index bcd9611dabfd..1ea053cef557 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -1146,3 +1146,85 @@ int drm_connector_helper_get_modes(struct drm_connector
*connector)
return count;
}
EXPORT_SYMBOL(drm_connector_helper_get_modes);
+
+/**
+ * drm_connector_helper_tv_get_modes - Fills the modes availables to a TV
connector
+ * @connector: The connector
+ *
+ * Fills the available modes for a TV connector based on the supported
+ * TV modes, and the default mode expressed by the kernel command line.
+ *
+ * This can be used as the default TV connector helper .get_modes() hook
+ * if the driver does not need any special processing.
+ *
+ * Returns:
+ * The number of modes added to the connector.
+ */
+int drm_connector_helper_tv_get_modes(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_property *tv_mode_property =
+ dev->mode_config.tv_mode_property;
+ struct drm_cmdline_mode *cmdline = >cmdline_mode;
+ unsigned int ntsc_modes = BIT(DRM_MODE_TV_MODE_NTSC) |
+ BIT(DRM_MODE_TV_MODE_NTSC_443) |
+ BIT(DRM_MODE_TV_MODE_NTSC_J) |
+ BIT(DRM_MODE_TV_MODE_PAL_M);
+ unsigned int pal_modes = BIT(DRM_MODE_TV_MODE_PAL) |
+ BIT(DRM_MODE_TV_MODE_PAL_N) |
+ BIT(DRM_MODE_TV_MODE_SECAM);
+ unsigned int tv_modes[2] = { UINT_MAX, UINT_MAX };
+ unsigned int i, supported_tv_modes = 0;
+
+ if (!tv_mode_property)
+ return 0;
+
+ for (i = 0; i < tv_mode_property->num_values; i++)
+ supported_tv_modes |= BIT(tv_mode_property->values[i]);
+
+ if ((supported_tv_modes & ntsc_modes) &&
+ (supported_tv_modes & pal_modes)) {
+ uint64_t default_mode;
+
+ if (drm_object_property_get_default_value(>base,
+ tv_mode_property,
+ _mode))
+ return 0;
+
+ if (cmdline->tv_mode_specified)
+ default_mode = cmdline->tv_mode;
+
+ if (BIT(default_mode) & ntsc_modes) {
+ tv_modes[0] = DRM_MODE_TV_MODE_NTSC;
+ tv_modes[1] = DRM_MODE_TV_MODE_PAL;
+ } else {
+ tv_modes[0] = DRM_MODE_TV_MODE_PAL;
+ tv_modes[1] = DRM_MODE_TV_MODE_NTSC;
+ }
+ } else if (supported_tv_modes & ntsc_modes) {
+ tv_modes[0] = DRM_MODE_TV_MODE_NTSC;
+ } else if (supported_tv_modes & pal_modes) {
+ tv_modes[0] = DRM_MODE_TV_MODE_PAL;
+ } else {
+ return 0;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
+ struct drm_display_mode *mode;
+
+ if (tv_modes[i] == DRM_MODE_TV_MODE_NTSC)
+ mode = drm_mode_analog_ntsc_480i(dev);
+ else if (tv_modes[i] == DRM_MODE_TV_MODE_PAL)
+ mode = drm_mode_analog_pal_576i(dev);
+ else
+ break;
+ if (!mode)
+ return i;
+ if (!i)
+ mode->type |= DRM_MODE_TYPE_PREFERRED;
+ drm_mode_probed_add(connector, mode);
+ }
+
+ return i;
+}
+EXPORT_SYMBOL(drm_connector_helper_tv_get_modes);
diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
index c7903c112c65..94fe546d937d 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -13,4 +13,5 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
drm_mm_test.o \
drm_modes_test.o \
[PATCH v9 05/25] drm/connector: Rename drm_mode_create_tv_properties
drm_mode_create_tv_properties(), among other things, will create the
"mode" property that stores the analog TV mode that connector is
supposed to output.
However, that property is getting deprecated, so let's rename that
function to mention it's deprecated. We'll introduce a new variant of
that function creating the property superseeding it in a later patch.
Reviewed-by: Lyude Paul # nouveau
Reviewed-by: Noralf Trønnes
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_connector.c | 12 ++--
drivers/gpu/drm/gud/gud_connector.c | 4 ++--
drivers/gpu/drm/i2c/ch7006_drv.c | 2 +-
drivers/gpu/drm/i915/display/intel_tv.c | 2 +-
drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 2 +-
drivers/gpu/drm/vc4/vc4_vec.c | 5 +++--
include/drm/drm_connector.h | 6 +++---
7 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 78fcffae100b..06e737ed15f5 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1604,7 +1604,7 @@ EXPORT_SYMBOL(drm_connector_attach_tv_margin_properties);
* Called by a driver's HDMI connector initialization routine, this function
* creates the TV margin properties for a given device. No need to call this
* function for an SDTV connector, it's already called from
- * drm_mode_create_tv_properties().
+ * drm_mode_create_tv_properties_legacy().
*
* Returns:
* 0 on success or a negative error code on failure.
@@ -1639,7 +1639,7 @@ int drm_mode_create_tv_margin_properties(struct
drm_device *dev)
EXPORT_SYMBOL(drm_mode_create_tv_margin_properties);
/**
- * drm_mode_create_tv_properties - create TV specific connector properties
+ * drm_mode_create_tv_properties_legacy - create TV specific connector
properties
* @dev: DRM device
* @num_modes: number of different TV formats (modes) supported
* @modes: array of pointers to strings containing name of each format
@@ -1652,9 +1652,9 @@ EXPORT_SYMBOL(drm_mode_create_tv_margin_properties);
* Returns:
* 0 on success or a negative error code on failure.
*/
-int drm_mode_create_tv_properties(struct drm_device *dev,
- unsigned int num_modes,
- const char * const modes[])
+int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
+unsigned int num_modes,
+const char * const modes[])
{
struct drm_property *tv_selector;
struct drm_property *tv_subconnector;
@@ -1737,7 +1737,7 @@ int drm_mode_create_tv_properties(struct drm_device *dev,
nomem:
return -ENOMEM;
}
-EXPORT_SYMBOL(drm_mode_create_tv_properties);
+EXPORT_SYMBOL(drm_mode_create_tv_properties_legacy);
/**
* drm_mode_create_scaling_mode_property - create scaling mode property
diff --git a/drivers/gpu/drm/gud/gud_connector.c
b/drivers/gpu/drm/gud/gud_connector.c
index 86e992b2108b..034e78360d4f 100644
--- a/drivers/gpu/drm/gud/gud_connector.c
+++ b/drivers/gpu/drm/gud/gud_connector.c
@@ -400,7 +400,7 @@ static int gud_connector_add_tv_mode(struct gud_device
*gdrm, struct drm_connect
for (i = 0; i < num_modes; i++)
modes[i] = [i * GUD_CONNECTOR_TV_MODE_NAME_LEN];
- ret = drm_mode_create_tv_properties(connector->dev, num_modes, modes);
+ ret = drm_mode_create_tv_properties_legacy(connector->dev, num_modes,
modes);
free:
kfree(buf);
if (ret < 0)
@@ -539,7 +539,7 @@ static int gud_connector_add_properties(struct gud_device
*gdrm, struct gud_conn
fallthrough;
case GUD_PROPERTY_TV_HUE:
/* This is a no-op if already added. */
- ret = drm_mode_create_tv_properties(drm, 0, NULL);
+ ret = drm_mode_create_tv_properties_legacy(drm, 0,
NULL);
if (ret)
goto out;
break;
diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
index ef69f9bdeace..b63bad04b09d 100644
--- a/drivers/gpu/drm/i2c/ch7006_drv.c
+++ b/drivers/gpu/drm/i2c/ch7006_drv.c
@@ -250,7 +250,7 @@ static int ch7006_encoder_create_resources(struct
drm_encoder *encoder,
struct drm_device *dev = encoder->dev;
struct drm_mode_config *conf = >mode_config;
- drm_mode_create_tv_properties(dev, NUM_TV_NORMS, ch7006_tv_norm_names);
+ drm_mode_create_tv_properties_legacy(dev, NUM_TV_NORMS,
ch7006_tv_norm_names);
priv->scale_property = drm_property_create_range(dev, 0, "scale", 0, 2);
if (!priv->scale_property)
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c
b/drivers/gpu/drm/i915/display/intel_tv.c
index 95b021da5a11..0affbc80ba89 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
[PATCH v9 16/25] drm/client: Remove match on mode name
Commit 3aeeb13d8996 ("drm/modes: Support modes names on the command
line") initially introduced the named modes support by essentially
matching the name passed on the command-line to the mode names defined
by the drivers.
This proved to be difficult to work with, since all drivers had to
provide properly named modes. This was also needed because we weren't
passing a full blown-mode to the drivers, but were only filling its
name.
Thanks to the previous patches, we now generate a proper mode, and we
thus can use the usual matching algo on timings, and can simply drop the
name match.
Reviewed-by: Noralf Trønnes
Suggested-by: Noralf Trønnes
Signed-off-by: Maxime Ripard
---
Changes in v8:
- New patch
---
drivers/gpu/drm/drm_client_modeset.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index d553e793e673..1b12a3c201a3 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -188,10 +188,6 @@ static struct drm_display_mode
*drm_connector_pick_cmdline_mode(struct drm_conne
prefer_non_interlace = !cmdline_mode->interlace;
again:
list_for_each_entry(mode, >modes, head) {
- /* Check (optional) mode name first */
- if (!strcmp(mode->name, cmdline_mode->name))
- return mode;
-
/* check width/height */
if (mode->hdisplay != cmdline_mode->xres ||
mode->vdisplay != cmdline_mode->yres)
--
b4 0.11.0-dev-99e3a
[PATCH v9 07/25] drm/modes: Add a function to generate analog display modes
Multiple drivers (meson, vc4, sun4i) define analog TV 525-lines and
625-lines modes in their drivers.
Since those modes are fairly standard, and that we'll need to use them
in more places in the future, it makes sense to move their definition
into the core framework.
However, analog display usually have fairly loose timings requirements,
the only discrete parameters being the total number of lines and pixel
clock frequency. Thus, we created a function that will create a display
mode from the standard, the pixel frequency and the active area.
Tested-by: Mateusz Kwiatkowski
Signed-off-by: Maxime Ripard
---
Changes in v9:
- Rename the tests
- Switch to kunit_test_suite
Changes in v6:
- Fix typo
Changes in v4:
- Reworded the line length check comment
- Switch to HZ_PER_KHZ in tests
- Use previous timing to fill our mode
- Move the number of lines check earlier
---
drivers/gpu/drm/drm_modes.c| 474 +
drivers/gpu/drm/tests/Makefile | 1 +
drivers/gpu/drm/tests/drm_modes_test.c | 142 ++
include/drm/drm_modes.h| 17 ++
4 files changed, 634 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 5d4ac79381c4..71c050c3ee6b 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -116,6 +116,480 @@ void drm_mode_probed_add(struct drm_connector *connector,
}
EXPORT_SYMBOL(drm_mode_probed_add);
+enum drm_mode_analog {
+ DRM_MODE_ANALOG_NTSC, /* 525 lines, 60Hz */
+ DRM_MODE_ANALOG_PAL, /* 625 lines, 50Hz */
+};
+
+/*
+ * The timings come from:
+ * -
https://web.archive.org/web/20220406232708/http://www.kolumbus.fi/pami1/video/pal_ntsc.html
+ * -
https://web.archive.org/web/20220406124914/http://martin.hinner.info/vga/pal.html
+ * -
https://web.archive.org/web/20220609202433/http://www.batsocks.co.uk/readme/video_timing.htm
+ */
+#define NTSC_LINE_DURATION_NS 63556U
+#define NTSC_LINES_NUMBER 525
+
+#define NTSC_HBLK_DURATION_TYP_NS 10900U
+#define NTSC_HBLK_DURATION_MIN_NS (NTSC_HBLK_DURATION_TYP_NS - 200)
+#define NTSC_HBLK_DURATION_MAX_NS (NTSC_HBLK_DURATION_TYP_NS + 200)
+
+#define NTSC_HACT_DURATION_TYP_NS (NTSC_LINE_DURATION_NS -
NTSC_HBLK_DURATION_TYP_NS)
+#define NTSC_HACT_DURATION_MIN_NS (NTSC_LINE_DURATION_NS -
NTSC_HBLK_DURATION_MAX_NS)
+#define NTSC_HACT_DURATION_MAX_NS (NTSC_LINE_DURATION_NS -
NTSC_HBLK_DURATION_MIN_NS)
+
+#define NTSC_HFP_DURATION_TYP_NS 1500
+#define NTSC_HFP_DURATION_MIN_NS 1270
+#define NTSC_HFP_DURATION_MAX_NS 2220
+
+#define NTSC_HSLEN_DURATION_TYP_NS 4700
+#define NTSC_HSLEN_DURATION_MIN_NS (NTSC_HSLEN_DURATION_TYP_NS - 100)
+#define NTSC_HSLEN_DURATION_MAX_NS (NTSC_HSLEN_DURATION_TYP_NS + 100)
+
+#define NTSC_HBP_DURATION_TYP_NS 4700
+
+/*
+ * I couldn't find the actual tolerance for the back porch, so let's
+ * just reuse the sync length ones.
+ */
+#define NTSC_HBP_DURATION_MIN_NS (NTSC_HBP_DURATION_TYP_NS - 100)
+#define NTSC_HBP_DURATION_MAX_NS (NTSC_HBP_DURATION_TYP_NS + 100)
+
+#define PAL_LINE_DURATION_NS 64000U
+#define PAL_LINES_NUMBER 625
+
+#define PAL_HACT_DURATION_TYP_NS 51950U
+#define PAL_HACT_DURATION_MIN_NS (PAL_HACT_DURATION_TYP_NS - 100)
+#define PAL_HACT_DURATION_MAX_NS (PAL_HACT_DURATION_TYP_NS + 400)
+
+#define PAL_HBLK_DURATION_TYP_NS (PAL_LINE_DURATION_NS -
PAL_HACT_DURATION_TYP_NS)
+#define PAL_HBLK_DURATION_MIN_NS (PAL_LINE_DURATION_NS -
PAL_HACT_DURATION_MAX_NS)
+#define PAL_HBLK_DURATION_MAX_NS (PAL_LINE_DURATION_NS -
PAL_HACT_DURATION_MIN_NS)
+
+#define PAL_HFP_DURATION_TYP_NS1650
+#define PAL_HFP_DURATION_MIN_NS(PAL_HFP_DURATION_TYP_NS - 100)
+#define PAL_HFP_DURATION_MAX_NS(PAL_HFP_DURATION_TYP_NS + 400)
+
+#define PAL_HSLEN_DURATION_TYP_NS 4700
+#define PAL_HSLEN_DURATION_MIN_NS (PAL_HSLEN_DURATION_TYP_NS - 200)
+#define PAL_HSLEN_DURATION_MAX_NS (PAL_HSLEN_DURATION_TYP_NS + 200)
+
+#define PAL_HBP_DURATION_TYP_NS5700
+#define PAL_HBP_DURATION_MIN_NS(PAL_HBP_DURATION_TYP_NS - 200)
+#define PAL_HBP_DURATION_MAX_NS(PAL_HBP_DURATION_TYP_NS + 200)
+
+struct analog_param_field {
+ unsigned int even, odd;
+};
+
+#define PARAM_FIELD(_odd, _even) \
+ { .even = _even, .odd = _odd }
+
+struct analog_param_range {
+ unsigned intmin, typ, max;
+};
+
+#define PARAM_RANGE(_min, _typ, _max) \
+ { .min = _min, .typ = _typ, .max = _max }
+
+struct analog_parameters {
+ unsigned intnum_lines;
+ unsigned intline_duration_ns;
+
+ struct analog_param_range hact_ns;
+ struct analog_param_range hfp_ns;
+ struct analog_param_range hslen_ns;
+ struct analog_param_range hbp_ns;
+ struct
