Re: [PATCH v2 2/4] dma-buf: enable signaling for the stub fence on debug
Am 05.09.22 um 18:35 schrieb Arvind Yadav: Here's on debug enabling software signaling for the stub fence which is always signaled. This fence should enable software signaling otherwise the AMD GPU scheduler will cause a GPU reset due to a GPU scheduler cleanup activity timeout. Signed-off-by: Arvind Yadav --- Changes in v1 : 1- Addressing Christian's comment to remove unnecessary callback. 2- Replacing CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS. 3- The version of this patch is also changed and previously it was [PATCH 3/4] --- drivers/dma-buf/dma-fence.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 066400ed8841..2378b12538c4 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -27,6 +27,10 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled); static DEFINE_SPINLOCK(dma_fence_stub_lock); static struct dma_fence dma_fence_stub; +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH +static bool __dma_fence_enable_signaling(struct dma_fence *fence); +#endif + I would rename the function to something like dma_fence_enable_signaling_locked(). And please don't add any #ifdef if it isn't absolutely necessary. This makes the code pretty fragile. /* * fence context counter: each execution context should have its own * fence context, this allows checking if fences belong to the same @@ -136,6 +140,9 @@ struct dma_fence *dma_fence_get_stub(void) &dma_fence_stub_ops, &dma_fence_stub_lock, 0, 0); +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH + __dma_fence_enable_signaling(&dma_fence_stub); +#endif Alternatively in this particular case you could just set the bit manually here since this is part of the dma_fence code anyway. Christian. dma_fence_signal_locked(&dma_fence_stub); } spin_unlock(&dma_fence_stub_lock);
Re: [PATCH v2 3/4] dma-buf: enable signaling for selftest fence on debug
Am 05.09.22 um 18:35 schrieb Arvind Yadav:
Here's on debug enabling software signaling for selftest.
Please drop all the #ifdefs, apart from that looks pretty good to me.
Christian.
Signed-off-by: Arvind Yadav
---
Changes in v1 :
1- Addressing Christian's comment to remove unnecessary callback.
2- Replacing CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS.
3- The version of this patch is also changed and previously
it was [PATCH 4/4]
---
drivers/dma-buf/st-dma-fence-chain.c | 8 +
drivers/dma-buf/st-dma-fence-unwrap.c | 44 +++
drivers/dma-buf/st-dma-fence.c| 25 ++-
drivers/dma-buf/st-dma-resv.c | 20
4 files changed, 96 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/st-dma-fence-chain.c
b/drivers/dma-buf/st-dma-fence-chain.c
index 8ce1ea59d31b..d3070f8a393c 100644
--- a/drivers/dma-buf/st-dma-fence-chain.c
+++ b/drivers/dma-buf/st-dma-fence-chain.c
@@ -87,6 +87,10 @@ static int sanitycheck(void *arg)
if (!chain)
err = -ENOMEM;
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(chain);
+#endif
+
dma_fence_signal(f);
dma_fence_put(f);
@@ -143,6 +147,10 @@ static int fence_chains_init(struct fence_chains *fc, unsigned int count,
}
fc->tail = fc->chains[i];
+
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(fc->chains[i]);
+#endif
}
fc->chain_length = i;
diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c
b/drivers/dma-buf/st-dma-fence-unwrap.c
index 4105d5ea8dde..b76cdd9ee0c7 100644
--- a/drivers/dma-buf/st-dma-fence-unwrap.c
+++ b/drivers/dma-buf/st-dma-fence-unwrap.c
@@ -102,6 +102,10 @@ static int sanitycheck(void *arg)
if (!f)
return -ENOMEM;
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f);
+#endif
+
array = mock_array(1, f);
if (!array)
return -ENOMEM;
@@ -124,12 +128,20 @@ static int unwrap_array(void *arg)
if (!f1)
return -ENOMEM;
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f1);
+#endif
+
f2 = mock_fence();
if (!f2) {
dma_fence_put(f1);
return -ENOMEM;
}
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f2);
+#endif
+
array = mock_array(2, f1, f2);
if (!array)
return -ENOMEM;
@@ -164,12 +176,20 @@ static int unwrap_chain(void *arg)
if (!f1)
return -ENOMEM;
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f1);
+#endif
+
f2 = mock_fence();
if (!f2) {
dma_fence_put(f1);
return -ENOMEM;
}
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f2);
+#endif
+
chain = mock_chain(f1, f2);
if (!chain)
return -ENOMEM;
@@ -204,12 +224,20 @@ static int unwrap_chain_array(void *arg)
if (!f1)
return -ENOMEM;
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f1);
+#endif
+
f2 = mock_fence();
if (!f2) {
dma_fence_put(f1);
return -ENOMEM;
}
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f2);
+#endif
+
array = mock_array(2, f1, f2);
if (!array)
return -ENOMEM;
@@ -248,12 +276,20 @@ static int unwrap_merge(void *arg)
if (!f1)
return -ENOMEM;
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f1);
+#endif
+
f2 = mock_fence();
if (!f2) {
err = -ENOMEM;
goto error_put_f1;
}
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f2);
+#endif
+
f3 = dma_fence_unwrap_merge(f1, f2);
if (!f3) {
err = -ENOMEM;
@@ -296,10 +332,18 @@ static int unwrap_merge_complex(void *arg)
if (!f1)
return -ENOMEM;
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f1);
+#endif
+
f2 = mock_fence();
if (!f2)
goto error_put_f1;
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f2);
+#endif
+
f3 = dma_fence_unwrap_merge(f1, f2);
if (!f3)
goto error_put_f2;
diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c
index c8a12d7ad71a..b7880d8374db 100644
--- a/drivers/dma-buf/st-dma-fence.c
+++ b/drivers/dma-buf/st-dma-fence.c
@@ -101,7 +101,9 @@ static int sanitycheck(void *arg)
f = mock_fence();
if (!f)
return -ENOMEM;
-
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ dma_fence_enable_sw_signaling(f);
+#endif
dma_fence
[PATCH linux-next] drm/vmwgfx: Remove the unneeded result variable
From: ye xingchen Return the value vmw_cotable_notify() directly instead of storing it in another redundant variable. Reported-by: Zeal Robot Signed-off-by: ye xingchen --- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c index f085dbd4736d..080c9c11277b 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c @@ -1263,7 +1263,6 @@ static int vmw_cmd_dx_define_query(struct vmw_private *dev_priv, VMW_DECLARE_CMD_VAR(*cmd, SVGA3dCmdDXDefineQuery); struct vmw_ctx_validation_info *ctx_node = VMW_GET_CTX_NODE(sw_context); struct vmw_resource *cotable_res; - int ret; if (!ctx_node) return -EINVAL; @@ -1275,9 +1274,8 @@ static int vmw_cmd_dx_define_query(struct vmw_private *dev_priv, return -EINVAL; cotable_res = vmw_context_cotable(ctx_node->ctx, SVGA_COTABLE_DXQUERY); - ret = vmw_cotable_notify(cotable_res, cmd->body.queryId); - return ret; + return vmw_cotable_notify(cotable_res, cmd->body.queryId); } /** @@ -2576,7 +2574,6 @@ static int vmw_cmd_dx_so_define(struct vmw_private *dev_priv, uint32 defined_id; } *cmd; enum vmw_so_type so_type; - int ret; if (!ctx_node) return -EINVAL; @@ -2586,9 +2583,8 @@ static int vmw_cmd_dx_so_define(struct vmw_private *dev_priv, if (IS_ERR(res)) return PTR_ERR(res); cmd = container_of(header, typeof(*cmd), header); - ret = vmw_cotable_notify(res, cmd->defined_id); - return ret; + return vmw_cotable_notify(res, cmd->defined_id); } /** -- 2.25.1
[PATCH v2 01/11] drm/udl: Restore display mode on resume
From: Thomas Zimmermann Restore the display mode whne resuming from suspend. Currently, the display remains dark. On resume, the CRTC's mode does not change, but the 'active' flag changes to 'true'. Taking this into account when considering a mode switch restores the display mode. The bug is reproducable by using Gnome with udl and observing the adapter's suspend/resume behavior. Signed-off-by: Thomas Zimmermann Signed-off-by: Takashi Iwai --- drivers/gpu/drm/udl/udl_modeset.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c index 169110d8fc2e..df987644fb5d 100644 --- a/drivers/gpu/drm/udl/udl_modeset.c +++ b/drivers/gpu/drm/udl/udl_modeset.c @@ -8,6 +8,7 @@ * Copyright (C) 2009 Bernie Thompson */ +#include #include #include #include @@ -382,7 +383,7 @@ udl_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, udl_handle_damage(fb, &shadow_plane_state->data[0], 0, 0, fb->width, fb->height); - if (!crtc_state->mode_changed) + if (!drm_atomic_crtc_needs_modeset(crtc_state)) return; /* enable display */ -- 2.35.3
[PATCH v2 02/11] drm/udl: Add reset_resume
From: Thomas Zimmermann
Implement the reset_resume callback of struct usb_driver. Set the
standard channel when called.
Signed-off-by: Thomas Zimmermann
Signed-off-by: Takashi Iwai
---
drivers/gpu/drm/udl/udl_drv.c | 11 +++
drivers/gpu/drm/udl/udl_drv.h | 1 +
drivers/gpu/drm/udl/udl_main.c | 2 +-
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 5703277c6f52..0ba88e5472a9 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -32,6 +32,16 @@ static int udl_usb_resume(struct usb_interface *interface)
return drm_mode_config_helper_resume(dev);
}
+static int udl_usb_reset_resume(struct usb_interface *interface)
+{
+ struct drm_device *dev = usb_get_intfdata(interface);
+ struct udl_device *udl = to_udl(dev);
+
+ udl_select_std_channel(udl);
+
+ return drm_mode_config_helper_resume(dev);
+}
+
/*
* FIXME: Dma-buf sharing requires DMA support by the importing device.
*This function is a workaround to make USB devices work as well.
@@ -140,6 +150,7 @@ static struct usb_driver udl_driver = {
.disconnect = udl_usb_disconnect,
.suspend = udl_usb_suspend,
.resume = udl_usb_resume,
+ .reset_resume = udl_usb_reset_resume,
.id_table = id_table,
};
module_usb_driver(udl_driver);
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 28aaf75d71cf..37c14b0ff1fc 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -95,6 +95,7 @@ int udl_render_hline(struct drm_device *dev, int log_bpp,
struct urb **urb_ptr,
u32 byte_offset, u32 device_byte_offset, u32 byte_width);
int udl_drop_usb(struct drm_device *dev);
+int udl_select_std_channel(struct udl_device *udl);
#define CMD_WRITE_RAW8 "\xAF\x60" /**< 8 bit raw write command. */
#define CMD_WRITE_RL8"\xAF\x61" /**< 8 bit run length command. */
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index fdafbf8f3c3c..7d1e6bbc165c 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -92,7 +92,7 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl)
/*
* Need to ensure a channel is selected before submitting URBs
*/
-static int udl_select_std_channel(struct udl_device *udl)
+int udl_select_std_channel(struct udl_device *udl)
{
static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
0x1C, 0x88, 0x5E, 0x15,
--
2.35.3
[PATCH v2 00/11] drm/udl: More fixes
Hi, this is a revised patch set for cleaning up and fixes for UDL driver. It covers the PM problems, regressions in the previous patch set, fixes for the stalls on some systems, as well as more hardening. Takashi === v1->v2: cleanups as suggested by Thomas - Drop numurbs parameter patch - Clean up / simplify clipping patch - Code cleanup and changes for urb management patch - Put Acks on some given patches === Takashi Iwai (8): Revert "drm/udl: Kill pending URBs at suspend and disconnect" drm/udl: Suppress error print for -EPROTO at URB completion drm/udl: Increase the default URB list size to 20 drm/udl: Drop unneeded alignment drm/udl: Fix potential URB leaks drm/udl: Fix inconsistent urbs.count value during udl_free_urb_list() drm/udl: Don't re-initialize stuff at retrying the URB list allocation drm/udl: Sync pending URBs at the end of suspend Thomas Zimmermann (3): drm/udl: Restore display mode on resume drm/udl: Add reset_resume drm/udl: Enable damage clipping drivers/gpu/drm/udl/udl_drv.c | 19 +- drivers/gpu/drm/udl/udl_drv.h | 13 +--- drivers/gpu/drm/udl/udl_main.c | 95 +++--- drivers/gpu/drm/udl/udl_modeset.c | 36 ++- drivers/gpu/drm/udl/udl_transfer.c | 45 ++ 5 files changed, 75 insertions(+), 133 deletions(-) -- 2.35.3
[PATCH v2 04/11] Revert "drm/udl: Kill pending URBs at suspend and disconnect"
This reverts the recent fix commit
e25d5954264d ("drm/udl: Kill pending URBs at suspend and disconnect")
as it turned out to lead to potential hangup at a disconnection, and
it doesn't help much for suspend/resume problem, either.
Acked-by: Thomas Zimmermann
Signed-off-by: Takashi Iwai
---
drivers/gpu/drm/udl/udl_drv.h | 2 --
drivers/gpu/drm/udl/udl_main.c| 25 +++--
drivers/gpu/drm/udl/udl_modeset.c | 2 --
3 files changed, 3 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 37c14b0ff1fc..5923d2e02bc8 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -39,7 +39,6 @@ struct urb_node {
struct urb_list {
struct list_head list;
- struct list_head in_flight;
spinlock_t lock;
wait_queue_head_t sleep;
int available;
@@ -85,7 +84,6 @@ static inline struct urb *udl_get_urb(struct drm_device *dev)
int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len);
int udl_sync_pending_urbs(struct drm_device *dev);
-void udl_kill_pending_urbs(struct drm_device *dev);
void udl_urb_completion(struct urb *urb);
int udl_init(struct udl_device *udl);
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 7d1e6bbc165c..a9f6b710b254 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -135,7 +135,7 @@ void udl_urb_completion(struct urb *urb)
urb->transfer_buffer_length = udl->urbs.size; /* reset to actual */
spin_lock_irqsave(&udl->urbs.lock, flags);
- list_move(&unode->entry, &udl->urbs.list);
+ list_add_tail(&unode->entry, &udl->urbs.list);
udl->urbs.available++;
spin_unlock_irqrestore(&udl->urbs.lock, flags);
@@ -180,7 +180,6 @@ static int udl_alloc_urb_list(struct drm_device *dev, int
count, size_t size)
retry:
udl->urbs.size = size;
INIT_LIST_HEAD(&udl->urbs.list);
- INIT_LIST_HEAD(&udl->urbs.in_flight);
init_waitqueue_head(&udl->urbs.sleep);
udl->urbs.count = 0;
@@ -247,7 +246,7 @@ struct urb *udl_get_urb_timeout(struct drm_device *dev,
long timeout)
}
unode = list_first_entry(&udl->urbs.list, struct urb_node, entry);
- list_move(&unode->entry, &udl->urbs.in_flight);
+ list_del_init(&unode->entry);
udl->urbs.available--;
unlock:
@@ -281,7 +280,7 @@ int udl_sync_pending_urbs(struct drm_device *dev)
spin_lock_irq(&udl->urbs.lock);
/* 2 seconds as a sane timeout */
if (!wait_event_lock_irq_timeout(udl->urbs.sleep,
-list_empty(&udl->urbs.in_flight),
+udl->urbs.available == udl->urbs.count,
udl->urbs.lock,
msecs_to_jiffies(2000)))
ret = -ETIMEDOUT;
@@ -289,23 +288,6 @@ int udl_sync_pending_urbs(struct drm_device *dev)
return ret;
}
-/* kill pending URBs */
-void udl_kill_pending_urbs(struct drm_device *dev)
-{
- struct udl_device *udl = to_udl(dev);
- struct urb_node *unode;
-
- spin_lock_irq(&udl->urbs.lock);
- while (!list_empty(&udl->urbs.in_flight)) {
- unode = list_first_entry(&udl->urbs.in_flight,
-struct urb_node, entry);
- spin_unlock_irq(&udl->urbs.lock);
- usb_kill_urb(unode->urb);
- spin_lock_irq(&udl->urbs.lock);
- }
- spin_unlock_irq(&udl->urbs.lock);
-}
-
int udl_init(struct udl_device *udl)
{
struct drm_device *dev = &udl->drm;
@@ -354,7 +336,6 @@ int udl_drop_usb(struct drm_device *dev)
{
struct udl_device *udl = to_udl(dev);
- udl_kill_pending_urbs(dev);
udl_free_urb_list(dev);
put_device(udl->dmadev);
udl->dmadev = NULL;
diff --git a/drivers/gpu/drm/udl/udl_modeset.c
b/drivers/gpu/drm/udl/udl_modeset.c
index 187aba2d7825..c34d564773a3 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -398,8 +398,6 @@ udl_simple_display_pipe_disable(struct
drm_simple_display_pipe *pipe)
struct urb *urb;
char *buf;
- udl_kill_pending_urbs(dev);
-
urb = udl_get_urb(dev);
if (!urb)
return;
--
2.35.3
[PATCH v2 03/11] drm/udl: Enable damage clipping
From: Thomas Zimmermann Call drm_plane_enable_fb_damage_clips() and give userspace a chance of minimizing the updated display area. Signed-off-by: Thomas Zimmermann Signed-off-by: Takashi Iwai --- drivers/gpu/drm/udl/udl_modeset.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c index df987644fb5d..187aba2d7825 100644 --- a/drivers/gpu/drm/udl/udl_modeset.c +++ b/drivers/gpu/drm/udl/udl_modeset.c @@ -484,6 +484,7 @@ int udl_modeset_init(struct drm_device *dev) format_count, NULL, connector); if (ret) return ret; + drm_plane_enable_fb_damage_clips(&udl->display_pipe.plane); drm_mode_config_reset(dev); -- 2.35.3
[PATCH v2 11/11] drm/udl: Sync pending URBs at the end of suspend
It's better to perform the sync at the very last of the suspend
instead of the pipe-disable function, so that we can catch all pending
URBs (if any).
While we're at it, drop the error code from udl_sync_pending_urb()
since we basically ignore it; instead, give a clear error message
indicating a problem.
Signed-off-by: Takashi Iwai
---
drivers/gpu/drm/udl/udl_drv.c | 8 +++-
drivers/gpu/drm/udl/udl_drv.h | 2 +-
drivers/gpu/drm/udl/udl_main.c| 6 ++
drivers/gpu/drm/udl/udl_modeset.c | 2 --
4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 0ba88e5472a9..91effdcefb6d 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -21,8 +21,14 @@ static int udl_usb_suspend(struct usb_interface *interface,
pm_message_t message)
{
struct drm_device *dev = usb_get_intfdata(interface);
+ int ret;
- return drm_mode_config_helper_suspend(dev);
+ ret = drm_mode_config_helper_suspend(dev);
+ if (ret)
+ return ret;
+
+ udl_sync_pending_urbs(dev);
+ return 0;
}
static int udl_usb_resume(struct usb_interface *interface)
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index d943684b5bbb..b4cc7cc568c7 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -77,7 +77,7 @@ struct drm_connector *udl_connector_init(struct drm_device
*dev);
struct urb *udl_get_urb(struct drm_device *dev);
int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len);
-int udl_sync_pending_urbs(struct drm_device *dev);
+void udl_sync_pending_urbs(struct drm_device *dev);
void udl_urb_completion(struct urb *urb);
int udl_init(struct udl_device *udl);
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 808a5ab5e14e..442080fa1061 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -290,10 +290,9 @@ int udl_submit_urb(struct drm_device *dev, struct urb
*urb, size_t len)
}
/* wait until all pending URBs have been processed */
-int udl_sync_pending_urbs(struct drm_device *dev)
+void udl_sync_pending_urbs(struct drm_device *dev)
{
struct udl_device *udl = to_udl(dev);
- int ret = 0;
spin_lock_irq(&udl->urbs.lock);
/* 2 seconds as a sane timeout */
@@ -301,9 +300,8 @@ int udl_sync_pending_urbs(struct drm_device *dev)
udl->urbs.available == udl->urbs.count,
udl->urbs.lock,
msecs_to_jiffies(2000)))
- ret = -ETIMEDOUT;
+ drm_err(dev, "Timeout for syncing pending URBs\n");
spin_unlock_irq(&udl->urbs.lock);
- return ret;
}
int udl_init(struct udl_device *udl)
diff --git a/drivers/gpu/drm/udl/udl_modeset.c
b/drivers/gpu/drm/udl/udl_modeset.c
index 9896c16c74f5..c506fff8f0c4 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -383,8 +383,6 @@ udl_simple_display_pipe_disable(struct
drm_simple_display_pipe *pipe)
buf = udl_dummy_render(buf);
udl_submit_urb(dev, urb, buf - (char *)urb->transfer_buffer);
-
- udl_sync_pending_urbs(dev);
}
static void
--
2.35.3
[PATCH v2 06/11] drm/udl: Increase the default URB list size to 20
It seems that the current size (4) for the URB list is too small on some devices, and it resulted in the occasional stalls. Increase the default URB list size to 20 for working around it. Acked-by: Thomas Zimmermann Signed-off-by: Takashi Iwai --- drivers/gpu/drm/udl/udl_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 6aed6e0f669c..2b7eafd48ec2 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -20,7 +20,7 @@ #define NR_USB_REQUEST_CHANNEL 0x12 #define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE) -#define WRITES_IN_FLIGHT (4) +#define WRITES_IN_FLIGHT (20) #define MAX_VENDOR_DESCRIPTOR_SIZE 256 static int udl_parse_vendor_descriptor(struct udl_device *udl) -- 2.35.3
[PATCH v2 10/11] drm/udl: Don't re-initialize stuff at retrying the URB list allocation
udl_alloc_urb_list() retires the allocation if there is no enough room
left, and it reinitializes the stuff unnecessarily such as the linked
list head and the waitqueue, which could be harmful. Those should be
outside the retry loop.
Acked-by: Thomas Zimmermann
Signed-off-by: Takashi Iwai
---
drivers/gpu/drm/udl/udl_main.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index df7ebe1fdc90..808a5ab5e14e 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -182,15 +182,14 @@ static int udl_alloc_urb_list(struct drm_device *dev, int
count, size_t size)
struct usb_device *udev = udl_to_usb_device(udl);
spin_lock_init(&udl->urbs.lock);
-
-retry:
- udl->urbs.size = size;
INIT_LIST_HEAD(&udl->urbs.list);
-
init_waitqueue_head(&udl->urbs.sleep);
udl->urbs.count = 0;
udl->urbs.available = 0;
+retry:
+ udl->urbs.size = size;
+
while (udl->urbs.count * size < wanted_size) {
unode = kzalloc(sizeof(struct urb_node), GFP_KERNEL);
if (!unode)
--
2.35.3
[PATCH v2 07/11] drm/udl: Drop unneeded alignment
The alignment of damaged area was needed for the original udlfb driver
that tried to trim the superfluous copies between front and backend
buffers and handle data in long int. It's not the case for udl DRM
driver, hence we can omit the whole unneeded alignment, as well as the
dead code.
Signed-off-by: Takashi Iwai
---
drivers/gpu/drm/udl/udl_modeset.c | 28 +
drivers/gpu/drm/udl/udl_transfer.c | 40 --
2 files changed, 1 insertion(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_modeset.c
b/drivers/gpu/drm/udl/udl_modeset.c
index c34d564773a3..9896c16c74f5 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -243,28 +243,6 @@ static long udl_log_cpp(unsigned int cpp)
return __ffs(cpp);
}
-static int udl_aligned_damage_clip(struct drm_rect *clip, int x, int y,
- int width, int height)
-{
- int x1, x2;
-
- if (WARN_ON_ONCE(x < 0) ||
- WARN_ON_ONCE(y < 0) ||
- WARN_ON_ONCE(width < 0) ||
- WARN_ON_ONCE(height < 0))
- return -EINVAL;
-
- x1 = ALIGN_DOWN(x, sizeof(unsigned long));
- x2 = ALIGN(width + (x - x1), sizeof(unsigned long)) + x1;
-
- clip->x1 = x1;
- clip->y1 = y;
- clip->x2 = x2;
- clip->y2 = y + height;
-
- return 0;
-}
-
static int udl_handle_damage(struct drm_framebuffer *fb,
const struct iosys_map *map,
int x, int y, int width, int height)
@@ -282,11 +260,7 @@ static int udl_handle_damage(struct drm_framebuffer *fb,
return ret;
log_bpp = ret;
- ret = udl_aligned_damage_clip(&clip, x, y, width, height);
- if (ret)
- return ret;
- else if ((clip.x2 > fb->width) || (clip.y2 > fb->height))
- return -EINVAL;
+ drm_rect_init(&clip, x, y, width, height);
ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
if (ret)
diff --git a/drivers/gpu/drm/udl/udl_transfer.c
b/drivers/gpu/drm/udl/udl_transfer.c
index 176ef2a6a731..a431208dda85 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -25,46 +25,6 @@
#define MIN_RAW_PIX_BYTES 2
#define MIN_RAW_CMD_BYTES (RAW_HEADER_BYTES + MIN_RAW_PIX_BYTES)
-/*
- * Trims identical data from front and back of line
- * Sets new front buffer address and width
- * And returns byte count of identical pixels
- * Assumes CPU natural alignment (unsigned long)
- * for back and front buffer ptrs and width
- */
-#if 0
-static int udl_trim_hline(const u8 *bback, const u8 **bfront, int *width_bytes)
-{
- int j, k;
- const unsigned long *back = (const unsigned long *) bback;
- const unsigned long *front = (const unsigned long *) *bfront;
- const int width = *width_bytes / sizeof(unsigned long);
- int identical = width;
- int start = width;
- int end = width;
-
- for (j = 0; j < width; j++) {
- if (back[j] != front[j]) {
- start = j;
- break;
- }
- }
-
- for (k = width - 1; k > j; k--) {
- if (back[k] != front[k]) {
- end = k+1;
- break;
- }
- }
-
- identical = start + (width - end);
- *bfront = (u8 *) &front[start];
- *width_bytes = (end - start) * sizeof(unsigned long);
-
- return identical * sizeof(unsigned long);
-}
-#endif
-
static inline u16 pixel32_to_be16(const uint32_t pixel)
{
return (((pixel >> 3) & 0x001f) |
--
2.35.3
[PATCH v2 09/11] drm/udl: Fix inconsistent urbs.count value during udl_free_urb_list()
In the current design, udl_get_urb() may be called asynchronously
during the driver freeing its URL list via udl_free_urb_list().
The problem is that the sync is determined by comparing the urbs.count
and urbs.available fields, while we clear urbs.count field only once
after udl_free_urb_list() finishes, i.e. during udl_free_urb_list(),
the state becomes inconsistent.
For fixing this inconsistency and also for hardening the locking
scheme, this patch does a slight refactoring of the code around
udl_get_urb() and udl_free_urb_list(). Now urbs.count is updated in
the same spinlock at extracting a URB from the list in
udl_free_url_list().
Signed-off-by: Takashi Iwai
---
drivers/gpu/drm/udl/udl_drv.h | 8 +--
drivers/gpu/drm/udl/udl_main.c | 44 +++---
2 files changed, 31 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 5923d2e02bc8..d943684b5bbb 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -74,13 +74,7 @@ static inline struct usb_device *udl_to_usb_device(struct
udl_device *udl)
int udl_modeset_init(struct drm_device *dev);
struct drm_connector *udl_connector_init(struct drm_device *dev);
-struct urb *udl_get_urb_timeout(struct drm_device *dev, long timeout);
-
-#define GET_URB_TIMEOUTHZ
-static inline struct urb *udl_get_urb(struct drm_device *dev)
-{
- return udl_get_urb_timeout(dev, GET_URB_TIMEOUT);
-}
+struct urb *udl_get_urb(struct drm_device *dev);
int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len);
int udl_sync_pending_urbs(struct drm_device *dev);
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index de28eeff3155..df7ebe1fdc90 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -23,6 +23,8 @@
#define WRITES_IN_FLIGHT (20)
#define MAX_VENDOR_DESCRIPTOR_SIZE 256
+static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout);
+
static int udl_parse_vendor_descriptor(struct udl_device *udl)
{
struct usb_device *udev = udl_to_usb_device(udl);
@@ -140,21 +142,23 @@ void udl_urb_completion(struct urb *urb)
udl->urbs.available++;
spin_unlock_irqrestore(&udl->urbs.lock, flags);
- wake_up(&udl->urbs.sleep);
+ wake_up_all(&udl->urbs.sleep);
}
static void udl_free_urb_list(struct drm_device *dev)
{
struct udl_device *udl = to_udl(dev);
- int count = udl->urbs.count;
struct urb_node *unode;
struct urb *urb;
DRM_DEBUG("Waiting for completes and freeing all render urbs\n");
/* keep waiting and freeing, until we've got 'em all */
- while (count--) {
- urb = udl_get_urb_timeout(dev, MAX_SCHEDULE_TIMEOUT);
+ while (udl->urbs.count) {
+ spin_lock_irq(&udl->urbs.lock);
+ urb = udl_get_urb_locked(udl, MAX_SCHEDULE_TIMEOUT);
+ udl->urbs.count--;
+ spin_unlock_irq(&udl->urbs.lock);
if (WARN_ON(!urb))
break;
unode = urb->context;
@@ -164,7 +168,8 @@ static void udl_free_urb_list(struct drm_device *dev)
usb_free_urb(urb);
kfree(unode);
}
- udl->urbs.count = 0;
+
+ wake_up(&udl->urbs.sleep);
}
static int udl_alloc_urb_list(struct drm_device *dev, int count, size_t size)
@@ -228,33 +233,44 @@ static int udl_alloc_urb_list(struct drm_device *dev, int
count, size_t size)
return udl->urbs.count;
}
-struct urb *udl_get_urb_timeout(struct drm_device *dev, long timeout)
+static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout)
{
- struct udl_device *udl = to_udl(dev);
- struct urb_node *unode = NULL;
+ struct urb_node *unode;
- if (!udl->urbs.count)
- return NULL;
+ assert_spin_locked(&udl->urbs.lock);
/* Wait for an in-flight buffer to complete and get re-queued */
- spin_lock_irq(&udl->urbs.lock);
if (!wait_event_lock_irq_timeout(udl->urbs.sleep,
+!udl->urbs.count ||
!list_empty(&udl->urbs.list),
udl->urbs.lock, timeout)) {
DRM_INFO("wait for urb interrupted: available: %d\n",
udl->urbs.available);
- goto unlock;
+ return NULL;
}
+ if (!udl->urbs.count)
+ return NULL;
+
unode = list_first_entry(&udl->urbs.list, struct urb_node, entry);
list_del_init(&unode->entry);
udl->urbs.available--;
-unlock:
- spin_unlock_irq(&udl->urbs.lock);
return unode ? unode->urb : NULL;
}
+#define GET_URB_TIMEOUTHZ
+struct urb *udl_get_urb(struct drm_device *dev)
+{
+ struct udl_device *udl = to_udl(dev);
+ struct urb *urb;
+
+
[PATCH v2 05/11] drm/udl: Suppress error print for -EPROTO at URB completion
The driver may receive -EPROTO at the URB completion when the device
gets disconnected, and it's a normal situation. Suppress the error
print for that, too.
Signed-off-by: Takashi Iwai
---
drivers/gpu/drm/udl/udl_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index a9f6b710b254..6aed6e0f669c 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -126,6 +126,7 @@ void udl_urb_completion(struct urb *urb)
if (urb->status) {
if (!(urb->status == -ENOENT ||
urb->status == -ECONNRESET ||
+ urb->status == -EPROTO ||
urb->status == -ESHUTDOWN)) {
DRM_ERROR("%s - nonzero write bulk status received:
%d\n",
__func__, urb->status);
--
2.35.3
[PATCH v2 08/11] drm/udl: Fix potential URB leaks
A couple of error handlings forgot to process the URB completion.
Those are both with WARN_ON() so should be visible, but we must fix
them in anyway.
Fixes: 7350b2a3fbc6 ("drm/udl: Replace BUG_ON() with WARN_ON()")
Acked-by: Thomas Zimmermann
Signed-off-by: Takashi Iwai
---
drivers/gpu/drm/udl/udl_main.c | 8 +---
drivers/gpu/drm/udl/udl_transfer.c | 5 -
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 2b7eafd48ec2..de28eeff3155 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -260,11 +260,13 @@ int udl_submit_urb(struct drm_device *dev, struct urb
*urb, size_t len)
struct udl_device *udl = to_udl(dev);
int ret;
- if (WARN_ON(len > udl->urbs.size))
- return -EINVAL;
-
+ if (WARN_ON(len > udl->urbs.size)) {
+ ret = -EINVAL;
+ goto error;
+ }
urb->transfer_buffer_length = len; /* set to actual payload len */
ret = usb_submit_urb(urb, GFP_ATOMIC);
+ error:
if (ret) {
udl_urb_completion(urb); /* because no one else will */
DRM_ERROR("usb_submit_urb error %x\n", ret);
diff --git a/drivers/gpu/drm/udl/udl_transfer.c
b/drivers/gpu/drm/udl/udl_transfer.c
index a431208dda85..b57844632dbd 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -180,8 +180,11 @@ int udl_render_hline(struct drm_device *dev, int log_bpp,
struct urb **urb_ptr,
u8 *cmd = *urb_buf_ptr;
u8 *cmd_end = (u8 *) urb->transfer_buffer + urb->transfer_buffer_length;
- if (WARN_ON(!(log_bpp == 1 || log_bpp == 2)))
+ if (WARN_ON(!(log_bpp == 1 || log_bpp == 2))) {
+ /* need to finish URB at error from this function */
+ udl_urb_completion(urb);
return -EINVAL;
+ }
line_start = (u8 *) (front + byte_offset);
next_pixel = line_start;
--
2.35.3
Re: [PATCH v3 4/5] drm/panfrost: devfreq: set opp to the recommended one to configure and enable regulator
Hi Biju,
On Tue, 6 Sept 2022 at 08:42, Biju Das wrote:
>
> Hi Clement,
>
> >
> > Hi,
> >
> > On Mon, 5 Sept 2022 at 20:17, Biju Das
> > wrote:
> > >
> > > Hi,
> > >
> > > Thanks for the patch.
> > >
> > > > Subject: [PATCH v3 4/5] drm/panfrost: devfreq: set opp to the
> > > > recommended one to configure and enable regulator
> > > >
> > > > devm_pm_opp_set_regulators() doesn't enable regulator, which make
> > > > regulator framework switching it off during regulator_late_cleanup().
> > >
> > > In that case, why not regulator_get()for Dynamic regulator(non fixed
> > > regulator)??
> >
> > Sorry I don't understand, what do you mean?
>
> Normally we need to turn on regulator and clock only when needed.
> I am not sure with your new code, will make it always on and
> drains the power unnecessarily and does it set lower opp or higher
> opp at the start??
The code doesn't make it always on, it makes it how it should be at
the recommended OPP which is the "start point".
If the recommended OPP says to switch off the regulator then it will.
>
> Compared to the fixed regulator, you have voltage regulator to
> control that is the difference between my environment and
> Your environment.
>
> I am not sure any other SoC is using voltage regulator??
> If yes, thenthere should be some bug or some difference in HW
> which is giving different behaviour??
>
> If you are the first one using voltage regulator with mali gpu,
> Then Your implementation may be correct, as you have proper
> HW to check.
The issue is that my regulator is not marked as "always-on", if no OPP
is called before regulator_late_cleanup() then nobody sets the
regulator_enable() and the regulator is switched off, which makes my
board hang.
Like Viresh recommends I will send an update with more details in the
commit log.
Regards,
Clement
>
> >
> > >
> > > >
> > > > Call dev_pm_opp_set_opp() with the recommend OPP in
> > > > panfrost_devfreq_init() to enable the regulator and avoid any switch
> > > > off by regulator_late_cleanup().
> > > >
> > > > Suggested-by: Viresh Kumar
> > > > Signed-off-by: Clément Péron
> > > > ---
> > > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 8
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > index 5110cd9b2425..67b242407156 100644
> > > > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > @@ -131,6 +131,14 @@ int panfrost_devfreq_init(struct
> > > > panfrost_device
> > > > *pfdev)
> > > > return PTR_ERR(opp);
> > > >
> > > > panfrost_devfreq_profile.initial_freq = cur_freq;
> > > > +
> > > > + /* Setup and enable regulator */
> > > > + ret = dev_pm_opp_set_opp(dev, opp);
> > > > + if (ret) {
> > > > + DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n");
> > > > + return ret;
> > > > + }
> > >
> > >
> > > FYI,
> > > On RZ/G2L mali gpu, we have fixed regulator and I was able to do GPU
> > > OPP transition without any issues previously.
> >
> > rzg2l-smarc-som.dtsi uses regulator reg_1p1v; which is marked as
> > regulator-always-on; that's why
> > regulator_late_cleanup() doesn't switch it off.
>
> Yes that is correct. It is fixed regulator and always on.
> We control only frequency.
>
> Cheers,
> Biju
>
> >
> > >
> > > root@smarc-rzg2l:~# cat /sys/class/devfreq/1184.gpu/trans_stat
> > > From : To
> > >: 5000 6250 1 12500 2
> > 25000 4 5 time(ms)
> > > * 5000: 0 0 0 0 0
> > 0 0 1 144
> > >6250: 0 0 0 0 0
> > 0 0 0 0
> > > 1: 0 0 0 0 0
> > 0 0 9 524
> > > 12500: 0 0 9 0 0
> > 0 0 3 2544
> > > 2: 0 0 011 0
> > 0 046 3304
> > > 25000: 1 0 0 033
> > 0 0 0 7496
> > > 4: 0 0 0 016
> > 19 0 0 2024
> > > 5: 1 0 0 1 8
> > 1535 0 4032
> > > Total transition : 208
> > >
> > > Cheers,
> > > Biju
> > >
RE: [PATCH v3 4/5] drm/panfrost: devfreq: set opp to the recommended one to configure and enable regulator
Hi Clement,
> Subject: Re: [PATCH v3 4/5] drm/panfrost: devfreq: set opp to the
> recommended one to configure and enable regulator
>
> Hi Biju,
>
> On Tue, 6 Sept 2022 at 08:42, Biju Das
> wrote:
> >
> > Hi Clement,
> >
> > >
> > > Hi,
> > >
> > > On Mon, 5 Sept 2022 at 20:17, Biju Das
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > Thanks for the patch.
> > > >
> > > > > Subject: [PATCH v3 4/5] drm/panfrost: devfreq: set opp to the
> > > > > recommended one to configure and enable regulator
> > > > >
> > > > > devm_pm_opp_set_regulators() doesn't enable regulator, which
> > > > > make regulator framework switching it off during
> regulator_late_cleanup().
> > > >
> > > > In that case, why not regulator_get()for Dynamic regulator(non
> > > > fixed regulator)??
> > >
> > > Sorry I don't understand, what do you mean?
> >
> > Normally we need to turn on regulator and clock only when needed.
> > I am not sure with your new code, will make it always on and drains
> > the power unnecessarily and does it set lower opp or higher opp at the
> > start??
>
> The code doesn't make it always on, it makes it how it should be at the
> recommended OPP which is the "start point".
>
> If the recommended OPP says to switch off the regulator then it will.
>
> >
> > Compared to the fixed regulator, you have voltage regulator to control
> > that is the difference between my environment and Your environment.
> >
> > I am not sure any other SoC is using voltage regulator??
> > If yes, thenthere should be some bug or some difference in HW which is
> > giving different behaviour??
> >
> > If you are the first one using voltage regulator with mali gpu, Then
> > Your implementation may be correct, as you have proper HW to check.
>
> The issue is that my regulator is not marked as "always-on", if no OPP is
> called before regulator_late_cleanup() then nobody sets the
> regulator_enable() and the regulator is switched off, which makes my
> board hang.
Cool, From your testing looks like no one tested this feature with
mali GPU on mainline??
Cheers,
Biju
>
> Like Viresh recommends I will send an update with more details in the
> commit log.
>
> Regards,
> Clement
>
>
> >
> > >
> > > >
> > > > >
> > > > > Call dev_pm_opp_set_opp() with the recommend OPP in
> > > > > panfrost_devfreq_init() to enable the regulator and avoid any
> > > > > switch off by regulator_late_cleanup().
> > > > >
> > > > > Suggested-by: Viresh Kumar
> > > > > Signed-off-by: Clément Péron
> > > > > ---
> > > > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 8
> > > > > 1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > > index 5110cd9b2425..67b242407156 100644
> > > > > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > > @@ -131,6 +131,14 @@ int panfrost_devfreq_init(struct
> > > > > panfrost_device
> > > > > *pfdev)
> > > > > return PTR_ERR(opp);
> > > > >
> > > > > panfrost_devfreq_profile.initial_freq = cur_freq;
> > > > > +
> > > > > + /* Setup and enable regulator */
> > > > > + ret = dev_pm_opp_set_opp(dev, opp);
> > > > > + if (ret) {
> > > > > + DRM_DEV_ERROR(dev, "Couldn't set recommended
> OPP\n");
> > > > > + return ret;
> > > > > + }
> > > >
> > > >
> > > > FYI,
> > > > On RZ/G2L mali gpu, we have fixed regulator and I was able to do
> > > > GPU OPP transition without any issues previously.
> > >
> > > rzg2l-smarc-som.dtsi uses regulator reg_1p1v; which is marked as
> > > regulator-always-on; that's why
> > > regulator_late_cleanup() doesn't switch it off.
> >
> > Yes that is correct. It is fixed regulator and always on.
> > We control only frequency.
> >
> > Cheers,
> > Biju
> >
> > >
> > > >
> > > > root@smarc-rzg2l:~# cat /sys/class/devfreq/1184.gpu/trans_stat
> > > > From : To
> > > >: 5000 6250 1 12500 2
> > > 25000 4 5 time(ms)
> > > > * 5000: 0 0 0 0 0
> > > 0 0 1 144
> > > >6250: 0 0 0 0 0
> > > 0 0 0 0
> > > > 1: 0 0 0 0 0
> > > 0 0 9 524
> > > > 12500: 0 0 9 0 0
> > > 0 0 3 2544
> > > > 2: 0 0 011 0
> > > 0 046 3304
> > > > 25000: 1 0 0 033
> > > 0 0 0 7496
> > > > 4: 0 0 0 016
> > > 19 0 0 2024
> > > > 5: 1 0 0 1 8
> > > 1535 0 4032
> > > > Total
Re: [PATCH v3 4/5] drm/panfrost: devfreq: set opp to the recommended one to configure and enable regulator
Hi,
On Tue, 6 Sept 2022 at 10:06, Biju Das wrote:
>
> Hi Clement,
>
> > Subject: Re: [PATCH v3 4/5] drm/panfrost: devfreq: set opp to the
> > recommended one to configure and enable regulator
> >
> > Hi Biju,
> >
> > On Tue, 6 Sept 2022 at 08:42, Biju Das
> > wrote:
> > >
> > > Hi Clement,
> > >
> > > >
> > > > Hi,
> > > >
> > > > On Mon, 5 Sept 2022 at 20:17, Biju Das
> > > > wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > Thanks for the patch.
> > > > >
> > > > > > Subject: [PATCH v3 4/5] drm/panfrost: devfreq: set opp to the
> > > > > > recommended one to configure and enable regulator
> > > > > >
> > > > > > devm_pm_opp_set_regulators() doesn't enable regulator, which
> > > > > > make regulator framework switching it off during
> > regulator_late_cleanup().
> > > > >
> > > > > In that case, why not regulator_get()for Dynamic regulator(non
> > > > > fixed regulator)??
> > > >
> > > > Sorry I don't understand, what do you mean?
> > >
> > > Normally we need to turn on regulator and clock only when needed.
> > > I am not sure with your new code, will make it always on and drains
> > > the power unnecessarily and does it set lower opp or higher opp at the
> > > start??
> >
> > The code doesn't make it always on, it makes it how it should be at the
> > recommended OPP which is the "start point".
> >
> > If the recommended OPP says to switch off the regulator then it will.
> >
> > >
> > > Compared to the fixed regulator, you have voltage regulator to control
> > > that is the difference between my environment and Your environment.
> > >
> > > I am not sure any other SoC is using voltage regulator??
> > > If yes, thenthere should be some bug or some difference in HW which is
> > > giving different behaviour??
> > >
> > > If you are the first one using voltage regulator with mali gpu, Then
> > > Your implementation may be correct, as you have proper HW to check.
> >
> > The issue is that my regulator is not marked as "always-on", if no OPP is
> > called before regulator_late_cleanup() then nobody sets the
> > regulator_enable() and the regulator is switched off, which makes my
> > board hang.
>
> Cool, From your testing looks like no one tested this feature with
> mali GPU on mainline??
Or no one without always-on.
Clement
>
> Cheers,
> Biju
>
>
> >
> > Like Viresh recommends I will send an update with more details in the
> > commit log.
> >
> > Regards,
> > Clement
> >
> >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > Call dev_pm_opp_set_opp() with the recommend OPP in
> > > > > > panfrost_devfreq_init() to enable the regulator and avoid any
> > > > > > switch off by regulator_late_cleanup().
> > > > > >
> > > > > > Suggested-by: Viresh Kumar
> > > > > > Signed-off-by: Clément Péron
> > > > > > ---
> > > > > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 8
> > > > > > 1 file changed, 8 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > > > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > > > index 5110cd9b2425..67b242407156 100644
> > > > > > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > > > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> > > > > > @@ -131,6 +131,14 @@ int panfrost_devfreq_init(struct
> > > > > > panfrost_device
> > > > > > *pfdev)
> > > > > > return PTR_ERR(opp);
> > > > > >
> > > > > > panfrost_devfreq_profile.initial_freq = cur_freq;
> > > > > > +
> > > > > > + /* Setup and enable regulator */
> > > > > > + ret = dev_pm_opp_set_opp(dev, opp);
> > > > > > + if (ret) {
> > > > > > + DRM_DEV_ERROR(dev, "Couldn't set recommended
> > OPP\n");
> > > > > > + return ret;
> > > > > > + }
> > > > >
> > > > >
> > > > > FYI,
> > > > > On RZ/G2L mali gpu, we have fixed regulator and I was able to do
> > > > > GPU OPP transition without any issues previously.
> > > >
> > > > rzg2l-smarc-som.dtsi uses regulator reg_1p1v; which is marked as
> > > > regulator-always-on; that's why
> > > > regulator_late_cleanup() doesn't switch it off.
> > >
> > > Yes that is correct. It is fixed regulator and always on.
> > > We control only frequency.
> > >
> > > Cheers,
> > > Biju
> > >
> > > >
> > > > >
> > > > > root@smarc-rzg2l:~# cat /sys/class/devfreq/1184.gpu/trans_stat
> > > > > From : To
> > > > >: 5000 6250 1 12500 2
> > > > 25000 4 5 time(ms)
> > > > > * 5000: 0 0 0 0 0
> > > > 0 0 1 144
> > > > >6250: 0 0 0 0 0
> > > > 0 0 0 0
> > > > > 1: 0 0 0 0 0
> > > > 0 0 9 524
> > > > > 12500: 0 0 9 0 0
> > > > 0 0 3 2544
> > > > > 2: 0 0 011 0
> > > > 0 0
Re: [PATCH] drm/doc: Custom Kconfig for KUnit is no longer needed
On Tue, Sep 06, 2022 at 08:37:00AM +0700, Bagas Sanjaya wrote:
> On 9/6/22 01:47, Michał Winiarski wrote:
> > References: commit 6fc3a8636a7b ("kunit: tool: Enable virtio/PCI by default
> > on UML")
>
> Use Fixes: tag for bugfix patches instead.
Can documentation update (when the referenced patch didn't touch the docs)
really be treated as a bugfix?
Or is it just a reference, validating the reasoning behind this patch?
-Michał
>
> --
> An old man doll... just what I always wanted! - Clara
Re: [PATCH 2/2] drm/etnaviv: fix power register offset on GC300
Hi Doug, Am Montag, dem 05.09.2022 um 14:05 -0700 schrieb Doug Brown: > Hi Christian, > > On 9/3/2022 4:49 AM, Christian Gmeiner wrote: > > > I had a quick look at what vivantes kernel driver did. It uses a per > > gpu instance variable powerBaseAddress > > that gets set accordingly. I am not sure if I really like the > > gpu_fix_reg_address(..) idea, as it gets called on every > > register read and write. For me I see two other possible solutions: > > > > 1) Add two seperate helpers ala gpu_read_power() and gpu_write_power() > > where we do the if beast. > > 2) Add a power register offset variable to etnaviv_gpu and explicitly > > use it on for reads and writes - like the Vivante driver does. > > > > But that's just my personal opinion. Can't wait to hear what Lucas thinks. > > > > Thanks for reviewing so fast! I honestly agree. It felt kind of dirty > modifying gpu_write and gpu_read. The reason I went for it is I was > thinking that in most cases the compiler is going to optimize the ugly > "if" out. > > The two solutions listed above both sound good. They would need a > special case in etnaviv_core_dump_registers, but that's probably much > less nasty than modifying gpu_read and gpu_write. Any preferences from > everyone on which of the other two options I should do? The > gpu_read_power and gpu_write_power approach sounds pretty clean to me. Yes, please add a specialized wrapper for the power register accesses. Regards, Lucas
Re: [PATCH v2 4/4] dma-buf: Check status of enable-signaling bit on debug
Am 05.09.22 um 18:35 schrieb Arvind Yadav:
The core DMA-buf framework needs to enable signaling
before the fence is signaled. The core DMA-buf framework
can forget to enable signaling before the fence is signaled.
This sentence is a bit confusing. I'm not a native speaker of English
either, but I suggest something like:
"Fence signaling must be enable to make sure that the
dma_fence_is_signaled() function ever returns true."
To avoid this scenario on the debug kernel, check the
DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT status bit before checking
the signaling bit status to confirm that enable_signaling
is enabled.
This describes the implementation, but we should rather describe the
background of the change. The implementation should be obvious.
Something like this maybe:
"
Since drivers and implementations sometimes mess this up enforce correct
behavior when DEBUG_WW_MUTEX_SLOWPATH is used during debugging.
This should make any implementations bugs resulting in not signaled
fences much more obvious.
"
Signed-off-by: Arvind Yadav
With the improved commit message this patch is Reviewed-by: Christian
König
Regards,
Christian.
---
Changes in v1 :
1- Addressing Christian's comment to replace
CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS.
2- As per Christian's comment moving this patch at last so
The version of this patch is also changed and previously
it was [PATCH 1/4]
---
include/linux/dma-fence.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..ba1ddc14c5d4 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,11 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
[PATCH v2] drm/ttm: update bulk move object of ghost BO
[Why]
Ghost BO is released with non-empty bulk move object. There is a
warning trace:
WARNING: CPU: 19 PID: 1582 at ttm/ttm_bo.c:366 ttm_bo_release+0x2e1/0x2f0
[amdttm]
Call Trace:
amddma_resv_reserve_fences+0x10d/0x1f0 [amdkcl]
amdttm_bo_put+0x28/0x30 [amdttm]
amdttm_bo_move_accel_cleanup+0x126/0x200 [amdttm]
amdgpu_bo_move+0x1a8/0x770 [amdgpu]
ttm_bo_handle_move_mem+0xb0/0x140 [amdttm]
amdttm_bo_validate+0xbf/0x100 [amdttm]
[How]
The resource of ghost BO should be moved to LRU directly, instead of
using bulk move. The bulk move object of ghost BO should set to NULL
before function ttm_bo_move_to_lru_tail_unlocked.
v2: set bulk move to NULL manually if no resource associated with ghost BO
Fixed: 5b951e487fd6bf5f ("drm/ttm: fix bulk move handling v2")
Signed-off-by: ZhenGuo Yin
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 1cbfb00c1d65..57a27847206f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -239,6 +239,9 @@ static int ttm_buffer_object_transfer(struct
ttm_buffer_object *bo,
if (fbo->base.resource) {
ttm_resource_set_bo(fbo->base.resource, &fbo->base);
bo->resource = NULL;
+ ttm_bo_set_bulk_move(&fbo->base, NULL);
+ } else {
+ fbo->base.bulk_move = NULL;
}
dma_resv_init(&fbo->base.base._resv);
--
2.35.1
Re: [PATCH v2] drm/ttm: update bulk move object of ghost BO
Am 06.09.22 um 10:46 schrieb ZhenGuo Yin:
[Why]
Ghost BO is released with non-empty bulk move object. There is a
warning trace:
WARNING: CPU: 19 PID: 1582 at ttm/ttm_bo.c:366 ttm_bo_release+0x2e1/0x2f0
[amdttm]
Call Trace:
amddma_resv_reserve_fences+0x10d/0x1f0 [amdkcl]
amdttm_bo_put+0x28/0x30 [amdttm]
amdttm_bo_move_accel_cleanup+0x126/0x200 [amdttm]
amdgpu_bo_move+0x1a8/0x770 [amdgpu]
ttm_bo_handle_move_mem+0xb0/0x140 [amdttm]
amdttm_bo_validate+0xbf/0x100 [amdttm]
[How]
The resource of ghost BO should be moved to LRU directly, instead of
using bulk move. The bulk move object of ghost BO should set to NULL
before function ttm_bo_move_to_lru_tail_unlocked.
v2: set bulk move to NULL manually if no resource associated with ghost BO
Fixed: 5b951e487fd6bf5f ("drm/ttm: fix bulk move handling v2")
Signed-off-by: ZhenGuo Yin
Reviewed-by: Christian König
Going to push that to drm-misc-fixes in a minute.
Thanks,
Christian.
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 1cbfb00c1d65..57a27847206f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -239,6 +239,9 @@ static int ttm_buffer_object_transfer(struct
ttm_buffer_object *bo,
if (fbo->base.resource) {
ttm_resource_set_bo(fbo->base.resource, &fbo->base);
bo->resource = NULL;
+ ttm_bo_set_bulk_move(&fbo->base, NULL);
+ } else {
+ fbo->base.bulk_move = NULL;
}
dma_resv_init(&fbo->base.base._resv);
Re: [PATCH] drm/doc: Custom Kconfig for KUnit is no longer needed
Michał Winiarski writes:
> On Tue, Sep 06, 2022 at 08:37:00AM +0700, Bagas Sanjaya wrote:
>> On 9/6/22 01:47, Michał Winiarski wrote:
>> > References: commit 6fc3a8636a7b ("kunit: tool: Enable virtio/PCI by
>> > default on UML")
>>
>> Use Fixes: tag for bugfix patches instead.
>
> Can documentation update (when the referenced patch didn't touch the docs)
> really be treated as a bugfix?
> Or is it just a reference, validating the reasoning behind this patch?
I kind of agree; I'm not sure that a Fixes tag is really warranted here.
Thanks,
jon
Re: [PATCH v2 10/12] drm/i915/xelpmp: Expose media as another GT
On 03-09-2022 05:02, Matt Roper wrote:
> Xe_LPM+ platforms have "standalone media." I.e., the media unit is
> designed as an additional GT with its own engine list, GuC, forcewake,
> etc. Let's allow platforms to include media GTs in their device info.
>
> v2:
> - Simplify GSI register handling and split it out to a separate patch
>for ease of review. (Daniele)
>
> Cc: Aravind Iddamsetty
> Cc: Daniele Ceraolo Spurio
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/i915/Makefile| 1 +
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 +
> drivers/gpu/drm/i915/gt/intel_sa_media.c | 39
> drivers/gpu/drm/i915/gt/intel_sa_media.h | 15 +
> drivers/gpu/drm/i915/i915_pci.c | 15 +
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> drivers/gpu/drm/i915/intel_uncore.c | 4 +++
> 7 files changed, 83 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/gt/intel_sa_media.c
> create mode 100644 drivers/gpu/drm/i915/gt/intel_sa_media.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 522ef9b4aff3..e83e4cd46968 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -123,6 +123,7 @@ gt-y += \
> gt/intel_ring.o \
> gt/intel_ring_submission.o \
> gt/intel_rps.o \
> + gt/intel_sa_media.o \
> gt/intel_sseu.o \
> gt/intel_sseu_debugfs.o \
> gt/intel_timeline.o \
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index d414785003cc..fb2c56777480 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1578,4 +1578,12 @@
>
> #define GEN12_SFC_DONE(n)_MMIO(0x1cc000 + (n) * 0x1000)
>
> +/*
> + * Standalone Media's non-engine GT registers are located at their regular GT
> + * offsets plus 0x38. This extra offset is stored inside the
> intel_uncore
> + * structure so that the existing code can be used for both GTs without
> + * modification.
> + */
> +#define MTL_MEDIA_GSI_BASE 0x38
> +
> #endif /* __INTEL_GT_REGS__ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_sa_media.c
> b/drivers/gpu/drm/i915/gt/intel_sa_media.c
> new file mode 100644
> index ..8c5c519457cc
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/intel_sa_media.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +
> +#include
> +
> +#include "i915_drv.h"
> +#include "gt/intel_gt.h"
> +#include "gt/intel_sa_media.h"
> +
> +int intel_sa_mediagt_setup(struct intel_gt *gt, phys_addr_t phys_addr,
> +u32 gsi_offset)
> +{
> + struct drm_i915_private *i915 = gt->i915;
> + struct intel_uncore *uncore;
> +
> + uncore = drmm_kzalloc(&i915->drm, sizeof(*uncore), GFP_KERNEL);
> + if (!uncore)
> + return -ENOMEM;
> +
> + uncore->gsi_offset = gsi_offset;
> +
> + intel_gt_common_init_early(gt);
> + intel_uncore_init_early(uncore, gt);
> +
> + /*
> + * Standalone media shares the general MMIO space with the primary
> + * GT. We'll re-use the primary GT's mapping.
> + */
> + uncore->regs = i915->uncore.regs;
> + if (drm_WARN_ON(&i915->drm, uncore->regs == NULL))
> + return -EIO;
> +
> + gt->uncore = uncore;
> + gt->phys_addr = phys_addr;
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/intel_sa_media.h
> b/drivers/gpu/drm/i915/gt/intel_sa_media.h
> new file mode 100644
> index ..3afb310de932
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/intel_sa_media.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +#ifndef __INTEL_SA_MEDIA__
> +#define __INTEL_SA_MEDIA__
> +
> +#include
> +
> +struct intel_gt;
> +
> +int intel_sa_mediagt_setup(struct intel_gt *gt, phys_addr_t phys_addr,
> +u32 gsi_offset);
> +
> +#endif /* __INTEL_SA_MEDIA_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 26b25d9434d6..18d3722331e4 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -26,6 +26,9 @@
> #include
> #include
>
> +#include "gt/intel_gt_regs.h"
> +#include "gt/intel_sa_media.h"
> +
> #include "i915_driver.h"
> #include "i915_drv.h"
> #include "i915_pci.h"
> @@ -1115,6 +1118,17 @@ static const struct intel_device_info pvc_info = {
> .display.has_cdclk_crawl = 1, \
> .__runtime.fbc_mask = BIT(INTEL_FBC_A) | BIT(INTEL_FBC_B)
>
> +static const struct intel_gt_definition xelpmp_extra_gt[] = {
> + {
> + .type = GT_MEDIA,
> + .name = "Standalone Media GT",
> + .setup = intel_sa_mediagt_setup,
> + .gsi_offset = MTL_MEDIA_GSI_BASE,
> + .engine_mask = BIT(VECS0) | BIT(VCS0) | BIT(VCS2)
Re: [PATCH] drm/i915: prevent integer overflow in query_engine_info()
Hi Dan,
On 01/09/2022 16:38, Dan Carpenter wrote:
This code uses struct_size() but it stores the result in an int so the
integer overflow checks are not effective. Record the types as size_t
to prevent the size from being truncated.
Fixes: bf3c50837506 ("drm/i915/query: Use struct_size() helper")
Signed-off-by: Dan Carpenter
---
I do not know if the integer overflow can happen. This is a hardenning
patch just like the conversion to struct_size().
It can't since num_uabi_engines in "len = struct_size(query_ptr,
engines, num_uabi_engines);" is max double digits and sizeof(struct
drm_i915_engine_info) is 56 bytes on a glance.
Nevertheless hardening is almost always beneficial so no fundamental
complaints. Just that this patch I think leaves some parts unresolved.
Mostly that copy_query_item now can implicitly truncate in "return
total_length" and likewise query_engine_info in "return ret;".
Maybe we could add, on top of your patch, something like:
static int copy_query_item(void *query_hdr, size_t query_sz,
- u32 total_length,
+ size_t total_length,
struct drm_i915_query_item *query_item)
{
+ if (overflows_type(query_sz, query_item->length) ||
+ overflows_type(total_length, query_item->length))
+ return -ERANGE; /* ??? */
+
(query->item_length is s32 so matches the int return type.)
And change all variables holding result of copy_query_item to size_t.
Don't know, it could be it's an overkill. More opinions?
Regards,
Tvrtko
drivers/gpu/drm/i915/i915_query.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 6ec9c9fb7b0d..43a499fbdc8d 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -13,7 +13,7 @@
#include
static int copy_query_item(void *query_hdr, size_t query_sz,
- u32 total_length,
+ size_t total_length,
struct drm_i915_query_item *query_item)
{
if (query_item->length == 0)
@@ -135,7 +135,8 @@ query_engine_info(struct drm_i915_private *i915,
struct drm_i915_engine_info info = { };
unsigned int num_uabi_engines = 0;
struct intel_engine_cs *engine;
- int len, ret;
+ size_t len;
+ int ret;
if (query_item->flags)
return -EINVAL;
Re: [PATCH v2 2/6] dt-bindings: mediatek: modify VDOSYS0 mmsys device tree Documentations for MT8188
Il 06/09/22 10:44, nathan.lu ha scritto: From: Nathan Lu modify VDOSYS0 mmsys device tree Documentations for MT8188. Signed-off-by: Nathan Lu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v2 3/6] dt-bindings: mediatek: modify VDOSYS0 mutex device tree Documentations for MT8188
Il 06/09/22 10:44, nathan.lu ha scritto: From: Nathan Lu modify VDOSYS0 mutex device tree Documentations for MT8188. Signed-off-by: Nathan Lu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v2 5/6] soc: mediatek: add mtk-mutex support for mt8188 vdosys0
Il 06/09/22 10:44, nathan.lu ha scritto: From: Nathan Lu add mtk-mutex support for mt8188 vdosys0. Signed-off-by: amy zhang Signed-off-by: Nathan Lu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v2 4/4] dma-buf: Check status of enable-signaling bit on debug
On 06/09/2022 09:39, Christian König wrote:
Am 05.09.22 um 18:35 schrieb Arvind Yadav:
The core DMA-buf framework needs to enable signaling
before the fence is signaled. The core DMA-buf framework
can forget to enable signaling before the fence is signaled.
This sentence is a bit confusing. I'm not a native speaker of English
either, but I suggest something like:
"Fence signaling must be enable to make sure that the
dma_fence_is_signaled() function ever returns true."
To avoid this scenario on the debug kernel, check the
DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT status bit before checking
the signaling bit status to confirm that enable_signaling
is enabled.
This describes the implementation, but we should rather describe the
background of the change. The implementation should be obvious.
Something like this maybe:
"
Since drivers and implementations sometimes mess this up enforce correct
behavior when DEBUG_WW_MUTEX_SLOWPATH is used during debugging.
This should make any implementations bugs resulting in not signaled
fences much more obvious.
"
I think I follow the idea but am not sure coupling (well "coupling".. not
really, but cross-contaminating in a way) dma-fence.c with a foreign and effectively
unrelated concept of a ww mutex is the best way.
Instead, how about a dma-buf specific debug kconfig option?
Condition would then be, according to my understanding of the rules and
expectations, along the lines of:
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..147a9df2c9d0 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,17 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_DMAFENCE
+ /*
+* Implementations not providing the enable_signaling callback are
+* required to always have signaling enabled or fences are not
+* guaranteed to ever signal.
+*/
+ if (!fence->ops->enable_signaling &&
+ !test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Thoughts?
Regards,
Tvrtko
Signed-off-by: Arvind Yadav
With the improved commit message this patch is Reviewed-by: Christian
König
Regards,
Christian.
---
Changes in v1 :
1- Addressing Christian's comment to replace
CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS.
2- As per Christian's comment moving this patch at last so
The version of this patch is also changed and previously
it was [PATCH 1/4]
---
include/linux/dma-fence.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..ba1ddc14c5d4 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,11 @@ dma_fence_is_signaled_locked(struct dma_fence
*fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Re: [PATCH 2/3] drm/gma500: Fix crtc_vblank reference leak when userspace queues multiple events
On 2022-09-05 15:37, Hans de Goede wrote: > The gma500 page-flip code kinda assume that userspace never queues more > then 1 vblank event. So basically it assume that userspace does: > > - page-flip > - wait for vblank event > - render > - page-flip > - etc. > > In the case where userspace would submit 2 page-flips without waiting > for the first to finish, the current code will just overwrite > gma_crtc->page_flip_event with the event from the 2nd page-flip. This cannot happen. Common code returns -EBUSY for an attempt to submit a page flip while another one is still pending. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH v2 4/4] vfio/pci: Allow MMIO regions to be exported through dma-buf
Am 06.09.22 um 11:51 schrieb Christoph Hellwig:
+{
+ struct vfio_pci_dma_buf *priv = dmabuf->priv;
+ int rc;
+
+ rc = pci_p2pdma_distance_many(priv->vdev->pdev, &attachment->dev, 1,
+ true);
This should just use pci_p2pdma_distance.
+ /*
+* Since the memory being mapped is a device memory it could never be in
+* CPU caches.
+*/
DMA_ATTR_SKIP_CPU_SYNC doesn't even apply to dma_map_resource, not sure
where this wisdom comes from.
+ dma_addr = dma_map_resource(
+ attachment->dev,
+ pci_resource_start(priv->vdev->pdev, priv->index) +
+ priv->offset,
+ priv->dmabuf->size, dir, DMA_ATTR_SKIP_CPU_SYNC);
This is not how P2P addresses are mapped. You need to use
dma_map_sgtable and have the proper pgmap for it.
The problem is once more that this is MMIO space, in other words
register BARs which needs to be exported/imported.
Adding struct pages for it generally sounds like the wrong approach
here. You can't even access this with the CPU or would trigger
potentially unwanted hardware actions.
Would you mind if I start to tackle this problem?
Regards,
Christian.
The above is just a badly implemented version of the dma-direct
PCI_P2PDMA_MAP_BUS_ADDR case, ignoring mappings through the host
bridge or dma-map-ops interactions.
Re: [PATCH v2 4/4] dma-buf: Check status of enable-signaling bit on debug
Am 06.09.22 um 12:20 schrieb Tvrtko Ursulin:
On 06/09/2022 09:39, Christian König wrote:
Am 05.09.22 um 18:35 schrieb Arvind Yadav:
The core DMA-buf framework needs to enable signaling
before the fence is signaled. The core DMA-buf framework
can forget to enable signaling before the fence is signaled.
This sentence is a bit confusing. I'm not a native speaker of English
either, but I suggest something like:
"Fence signaling must be enable to make sure that the
dma_fence_is_signaled() function ever returns true."
To avoid this scenario on the debug kernel, check the
DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT status bit before checking
the signaling bit status to confirm that enable_signaling
is enabled.
This describes the implementation, but we should rather describe the
background of the change. The implementation should be obvious.
Something like this maybe:
"
Since drivers and implementations sometimes mess this up enforce
correct behavior when DEBUG_WW_MUTEX_SLOWPATH is used during debugging.
This should make any implementations bugs resulting in not signaled
fences much more obvious.
"
I think I follow the idea but am not sure coupling (well "coupling"..
not really, but cross-contaminating in a way) dma-fence.c with a
foreign and effectively unrelated concept of a ww mutex is the best way.
Instead, how about a dma-buf specific debug kconfig option?
Yeah, I was thinking about that as well.
The slowpath config option was just at hand because when you want to
test the slowpath you want to test this here as well.
Condition would then be, according to my understanding of the rules
and expectations, along the lines of:
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..147a9df2c9d0 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,17 @@ dma_fence_is_signaled_locked(struct dma_fence
*fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_DMAFENCE
+ /*
+ * Implementations not providing the enable_signaling callback
are
+ * required to always have signaling enabled or fences are not
+ * guaranteed to ever signal.
+ */
Well that comment is a bit misleading. The intention of the extra check
is to find bugs in the frontend and not the backend.
In other words somewhere in the drm_syncobj code we have a
dma_fence_is_signaled() call without matching
dma_fence_enable_sw_signaling().
Regards,
Christian.
+ if (!fence->ops->enable_signaling &&
+ !test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Thoughts?
Regards,
Tvrtko
Signed-off-by: Arvind Yadav
With the improved commit message this patch is Reviewed-by: Christian
König
Regards,
Christian.
---
Changes in v1 :
1- Addressing Christian's comment to replace
CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS.
2- As per Christian's comment moving this patch at last so
The version of this patch is also changed and previously
it was [PATCH 1/4]
---
include/linux/dma-fence.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..ba1ddc14c5d4 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,11 @@ dma_fence_is_signaled_locked(struct dma_fence
*fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Re: [Intel-gfx] [PATCH v2 09/12] drm/i915/uncore: Add GSI offset to uncore
On 03-09-2022 05:02, Matt Roper wrote:
> GT non-engine registers (referred to as "GSI" registers by the spec)
> have the same relative offsets on standalone media as they do on the
> primary GT, just with an additional "GSI offset" added to their MMIO
> address. If we store this GSI offset in the standalone media's
> intel_uncore structure, it can be automatically applied to all GSI reg
> reads/writes that happen on that GT, allowing us to re-use our existing
> GT code with minimal changes.
>
> Forcewake and shadowed register tables for the media GT (which will be
> added in a future patch) are listed as final addresses that already
> include the GSI offset, so we also need to add the GSI offset before
> doing lookups of registers in one of those tables.
>
> Cc: Daniele Ceraolo Spurio
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 17 ++---
> drivers/gpu/drm/i915/intel_device_info.h | 4 +++-
> drivers/gpu/drm/i915/intel_uncore.c | 10 --
> drivers/gpu/drm/i915/intel_uncore.h | 22 --
> 4 files changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index fbb5e32979a4..a6ed11b933eb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -776,10 +776,20 @@ void intel_gt_driver_late_release_all(struct
> drm_i915_private *i915)
> }
> }
>
> -static int intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr)
> +/*
> + * Note: the gsi_offset parameter here isn't used, but we want to keep the
> + * function signature equivalent to gtdef->setup() so that it can be plugged
> + * in when we enabled remote tiles in the future.
> + */
> +static int intel_gt_tile_setup(struct intel_gt *gt,
> +phys_addr_t phys_addr,
> +u32 gsi_offset)
> {
> int ret;
>
> + /* GSI offset is only applicable for media GTs */
> + drm_WARN_ON(>->i915->drm, gsi_offset);
> +
> if (!gt_is_root(gt)) {
> struct intel_uncore *uncore;
>
> @@ -832,7 +842,7 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> gt->info.engine_mask = RUNTIME_INFO(i915)->platform_engine_mask;
>
> drm_dbg(&i915->drm, "Setting up %s\n", gt->name);
> - ret = intel_gt_tile_setup(gt, phys_addr);
> + ret = intel_gt_tile_setup(gt, phys_addr, 0);
> if (ret)
> return ret;
>
> @@ -865,7 +875,8 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> goto err;
> }
>
> - ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base);
> + ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base,
> +gtdef->gsi_offset);
> if (ret)
> goto err;
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> b/drivers/gpu/drm/i915/intel_device_info.h
> index b408ce384cd7..85e0ef0e91b1 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -254,8 +254,10 @@ struct intel_gt_definition {
> enum intel_gt_type type;
> char *name;
> int (*setup)(struct intel_gt *gt,
> - phys_addr_t phys_addr);
> + phys_addr_t phys_addr,
> + u32 gsi_offset);
> u32 mapping_base;
> + u32 gsi_offset;
> intel_engine_mask_t engine_mask;
> };
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 33bdcbc77ab2..ecb02421502d 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -927,6 +927,9 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
> {
> const struct intel_forcewake_range *entry;
>
> + if (IS_GSI_REG(offset))
> + offset += uncore->gsi_offset;
> +
> entry = BSEARCH(offset,
> uncore->fw_domains_table,
> uncore->fw_domains_table_entries,
> @@ -1142,6 +1145,9 @@ static bool is_shadowed(struct intel_uncore *uncore,
> u32 offset)
> if (drm_WARN_ON(&uncore->i915->drm, !uncore->shadowed_reg_table))
> return false;
>
> + if (IS_GSI_REG(offset))
> + offset += uncore->gsi_offset;
> +
> return BSEARCH(offset,
> uncore->shadowed_reg_table,
> uncore->shadowed_reg_table_entries,
> @@ -1994,8 +2000,8 @@ static int __fw_domain_init(struct intel_uncore *uncore,
>
> d->uncore = uncore;
> d->wake_count = 0;
> - d->reg_set = uncore->regs + i915_mmio_reg_offset(reg_set);
> - d->reg_ack = uncore->regs + i915_mmio_reg_offset(reg_ack);
> + d->reg_set = uncore->regs + i915_mmio_reg_offset(reg_set) +
> uncore->gsi_offset;
> + d->reg_ack = uncore->regs + i915_mmio_reg_offset(reg_ack) +
> uncore->gsi_offset;
>
>
Re: [PATCH v2 1/3] media: vsp1: add premultiplied alpha support
Em Fri, 19 Aug 2022 05:38:28 +0300 Laurent Pinchart escreveu: > Mauro, would you be fine with this patch going through the DRM tree for > v6.1 ? I don't foresee any risk of conflict with other changes to the > VSP driver scheduled for the next kernel version. If that's fine with > you, could you give an Acked-by ? Otherwise I can send you a pull > request to create an immutable branch and base the rest on it in my pull > request for DRM, but given how small this change is, it seems a bit > overkill. Please, don't top-post. > > On Fri, Aug 19, 2022 at 05:01:10AM +0300, Laurent Pinchart wrote: > > Hi Hayama-san, > > > > Thank you for the patch. > > > > On Wed, Aug 10, 2022 at 05:37:09PM +0900, Takanari Hayama wrote: > > > To support DRM blend mode in R-Car DU driver, we must be able to pass > > > a plane with the premultiplied alpha. Adding a new property to > > > vsp1_du_atomic_config allows the R-Car DU driver to pass the > > > premultiplied alpha plane. > > > > > > Signed-off-by: Takanari Hayama Sure, this can be merged via DRM tree. > > > --- > > > drivers/media/platform/renesas/vsp1/vsp1_drm.c | 2 ++ > > > include/media/vsp1.h | 2 ++ > > > 2 files changed, 4 insertions(+) > > > > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > index 0c2507dc03d6..019e18976bd8 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > @@ -856,6 +856,8 @@ int vsp1_du_atomic_update(struct device *dev, > > > unsigned int pipe_index, > > > rpf->mem.addr[1] = cfg->mem[1]; > > > rpf->mem.addr[2] = cfg->mem[2]; > > > > > > + rpf->format.flags = (cfg->premult) ? V4L2_PIX_FMT_FLAG_PREMUL_ALPHA : > > > 0; > > > > I'll drop the parentheses when applying. > > > > Reviewed-by: Laurent Pinchart With this change: Reviewed-by: Mauro Carvalho Chehab Regards, Mauro
Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_kms.c
Hi Jingyu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on e47eb90a0a9ae20b82635b9b99a8d0979b757ad8]
url:
https://github.com/intel-lab-lkp/linux/commits/Jingyu-Wang/drm-amdgpu-cleanup-coding-style-in-amdgpu_kms-c/20220906-104802
base: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
config: mips-allyesconfig
(https://download.01.org/0day-ci/archive/20220906/202209061955.bbpvthp7-...@intel.com/config)
compiler: mips-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/778e4a57afd0db6a6a752487e1a1cda253cd9682
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Jingyu-Wang/drm-amdgpu-cleanup-coding-style-in-amdgpu_kms-c/20220906-104802
git checkout 778e4a57afd0db6a6a752487e1a1cda253cd9682
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=mips SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c: In function 'amdgpu_info_ioctl':
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:76: error: macro "min_t"
>> requires 3 arguments, but only 2 given
554 | ret = copy_to_user(out, &ip, min_t((size_t)size,
sizeof(ip)));
|
^
In file included from include/linux/kernel.h:26,
from include/linux/cpumask.h:10,
from include/linux/smp.h:13,
from arch/mips/include/asm/cpu-type.h:12,
from arch/mips/include/asm/timex.h:19,
from include/linux/timex.h:67,
from include/linux/time32.h:13,
from include/linux/time.h:60,
from include/linux/ktime.h:24,
from drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h:26,
from drivers/gpu/drm/amd/amdgpu/amdgpu.h:43,
from drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:29:
include/linux/minmax.h:104: note: macro "min_t" defined here
104 | #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y),
<)
|
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:46: error: 'min_t' undeclared
>> (first use in this function)
554 | ret = copy_to_user(out, &ip, min_t((size_t)size,
sizeof(ip)));
| ^
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:46: note: each undeclared
identifier is reported only once for each function it appears in
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c: In function
'amdgpu_debugfs_firmware_info_show':
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: error: expected expression
>> before '[' token
1384 | #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type)
| ^
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1385:17: note: in expansion of macro
'TA_FW_NAME'
1385 | TA_FW_NAME(XGMI),
| ^~
vim +/min_t +554 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
494
495 /*
496 * Userspace get information ioctl
497 */
498 /**
499 * amdgpu_info_ioctl - answer a device specific request.
500 *
501 * @dev: drm device pointer
502 * @data: request object
503 * @filp: drm filp
504 *
505 * This function is used to pass device specific parameters to the
userspace
506 * drivers. Examples include: pci device id, pipeline parms, tiling
params,
507 * etc. (all asics).
508 * Returns 0 on success, -EINVAL on failure.
509 */
510 int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct
drm_file *filp)
511 {
512 struct amdgpu_device *adev = drm_to_adev(dev);
513 struct drm_amdgpu_info *info = data;
514 struct amdgpu_mode_info *minfo = &adev->mode_info;
515 void __user *out = (void __user
*)(uintptr_t)info->return_pointer;
516 uint32_t size = info->return_size;
517 struct drm_crtc *crtc;
518 uint32_t ui32 = 0;
519 uint64_t ui64 = 0;
520 int i, found;
521 int ui32_size = sizeof(ui32);
522
523 if (!info->return_size || !info->return_pointer)
524 return -EINVAL;
525
526 switch (info->query) {
527 case AMDGPU_INFO_ACCEL_W
Re: [PATCH v2 4/4] dma-buf: Check status of enable-signaling bit on debug
On 06/09/2022 11:43, Christian König wrote:
Am 06.09.22 um 12:20 schrieb Tvrtko Ursulin:
On 06/09/2022 09:39, Christian König wrote:
Am 05.09.22 um 18:35 schrieb Arvind Yadav:
The core DMA-buf framework needs to enable signaling
before the fence is signaled. The core DMA-buf framework
can forget to enable signaling before the fence is signaled.
This sentence is a bit confusing. I'm not a native speaker of English
either, but I suggest something like:
"Fence signaling must be enable to make sure that the
dma_fence_is_signaled() function ever returns true."
To avoid this scenario on the debug kernel, check the
DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT status bit before checking
the signaling bit status to confirm that enable_signaling
is enabled.
This describes the implementation, but we should rather describe the
background of the change. The implementation should be obvious.
Something like this maybe:
"
Since drivers and implementations sometimes mess this up enforce
correct behavior when DEBUG_WW_MUTEX_SLOWPATH is used during debugging.
This should make any implementations bugs resulting in not signaled
fences much more obvious.
"
I think I follow the idea but am not sure coupling (well "coupling"..
not really, but cross-contaminating in a way) dma-fence.c with a
foreign and effectively unrelated concept of a ww mutex is the best way.
Instead, how about a dma-buf specific debug kconfig option?
Yeah, I was thinking about that as well.
Cool, lets see about the specifics below and then decide.
The slowpath config option was just at hand because when you want to
test the slowpath you want to test this here as well.
Condition would then be, according to my understanding of the rules
and expectations, along the lines of:
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..147a9df2c9d0 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,17 @@ dma_fence_is_signaled_locked(struct dma_fence
*fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_DMAFENCE
+ /*
+ * Implementations not providing the enable_signaling callback
are
+ * required to always have signaling enabled or fences are not
+ * guaranteed to ever signal.
+ */
Well that comment is a bit misleading. The intention of the extra check
is to find bugs in the frontend and not the backend.
By backend you mean drivers/dma-buf/dma-fence.c and by front end driver
specific implementations? Or simply users for dma-fence?
Could be that I got confused.. I was reading these two:
* This callback is optional. If this callback is not present, then the
* driver must always have signaling enabled.
*/
bool (*enable_signaling)(struct dma_fence *fence);
And dma_fence_is_signaled:
* Returns true if the fence was already signaled, false if not. Since this
* function doesn't enable signaling, it is not guaranteed to ever return
* true if dma_fence_add_callback(), dma_fence_wait() or
* dma_fence_enable_sw_signaling() haven't been called before.
Right, I think I did get confused, apologies. What I was thinking was probably
two separate conditions:
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_DMAFENCE
+ if (WARN_ON_ONCE(!fence->ops->enable_signaling &&
+!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
&fence->flags)))
+ return false;
+
+ if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Not sure "is signaled" is the best place for the first one or that it should
definitely be added.
Regards,
Tvrtko
In other words somewhere in the drm_syncobj code we have a
dma_fence_is_signaled() call without matching
dma_fence_enable_sw_signaling().
Regards,
Christian.
+ if (!fence->ops->enable_signaling &&
+ !test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Thoughts?
Regards,
Tvrtko
Signed-off-by: Arvind Yadav
With the improved commit message this patch is Reviewed-by: Christian
König
Regards,
Christian.
---
Changes in v1 :
1- Addressing Christian's comment to replace
CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS.
2- As per Christian's comment moving this patch at last so
The version of this patch is also changed and previously
it was [PATCH 1/4]
---
include/linux/dma-fence.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..ba1ddc14c5d4 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,11 @@ dma_fence_is_s
Re: [PATCH v2 4/4] dma-buf: Check status of enable-signaling bit on debug
Am 06.09.22 um 13:21 schrieb Tvrtko Ursulin:
On 06/09/2022 11:43, Christian König wrote:
Am 06.09.22 um 12:20 schrieb Tvrtko Ursulin:
On 06/09/2022 09:39, Christian König wrote:
Am 05.09.22 um 18:35 schrieb Arvind Yadav:
The core DMA-buf framework needs to enable signaling
before the fence is signaled. The core DMA-buf framework
can forget to enable signaling before the fence is signaled.
This sentence is a bit confusing. I'm not a native speaker of
English either, but I suggest something like:
"Fence signaling must be enable to make sure that the
dma_fence_is_signaled() function ever returns true."
To avoid this scenario on the debug kernel, check the
DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT status bit before checking
the signaling bit status to confirm that enable_signaling
is enabled.
This describes the implementation, but we should rather describe
the background of the change. The implementation should be obvious.
Something like this maybe:
"
Since drivers and implementations sometimes mess this up enforce
correct behavior when DEBUG_WW_MUTEX_SLOWPATH is used during
debugging.
This should make any implementations bugs resulting in not signaled
fences much more obvious.
"
I think I follow the idea but am not sure coupling (well
"coupling".. not really, but cross-contaminating in a way)
dma-fence.c with a foreign and effectively unrelated concept of a ww
mutex is the best way.
Instead, how about a dma-buf specific debug kconfig option?
Yeah, I was thinking about that as well.
Cool, lets see about the specifics below and then decide.
The slowpath config option was just at hand because when you want to
test the slowpath you want to test this here as well.
Condition would then be, according to my understanding of the rules
and expectations, along the lines of:
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..147a9df2c9d0 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,17 @@ dma_fence_is_signaled_locked(struct dma_fence
*fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_DMAFENCE
+ /*
+ * Implementations not providing the enable_signaling
callback are
+ * required to always have signaling enabled or fences are not
+ * guaranteed to ever signal.
+ */
Well that comment is a bit misleading. The intention of the extra
check is to find bugs in the frontend and not the backend.
By backend you mean drivers/dma-buf/dma-fence.c and by front end
driver specific implementations? Or simply users for dma-fence?
Backend are the driver specific implementation of the dma_fence_ops.
Middleware is the framework in drivers/dma-buf.
Frontend is the users of the dma_fence interface, e.g. either drivers or
components (drm_syncobj, drm_scheduler etc...).
Could be that I got confused.. I was reading these two:
* This callback is optional. If this callback is not present,
then the
* driver must always have signaling enabled.
*/
bool (*enable_signaling)(struct dma_fence *fence);
And dma_fence_is_signaled:
* Returns true if the fence was already signaled, false if not. Since
this
* function doesn't enable signaling, it is not guaranteed to ever return
* true if dma_fence_add_callback(), dma_fence_wait() or
* dma_fence_enable_sw_signaling() haven't been called before.
Right, I think I did get confused, apologies. What I was thinking was
probably two separate conditions:
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_DMAFENCE
+ if (WARN_ON_ONCE(!fence->ops->enable_signaling &&
+ !test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)))
+ return false;
+
+ if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Not sure "is signaled" is the best place for the first one or that it
should definitely be added.
I was thinking about adding this WARN_ON() as well, but then decided
against it.
There are a couple of cases where calling dma_fence_is_signaled()
without previously calling dma_fence_enable_sw_signaling() is valid
because it is done just for optimization purposes and we guarantee that
dma_fence_enable_sw_signaling() is called just a little bit later.
But yes, in general it's the same idea I already had as well.
Regards,
Christian.
Regards,
Tvrtko
In other words somewhere in the drm_syncobj code we have a
dma_fence_is_signaled() call without matching
dma_fence_enable_sw_signaling().
Regards,
Christian.
+ if (!fence->ops->enable_signaling &&
+ !test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Thoug
Re: [PATCH] drm/i915/gvt: fix double-free bug in split_2MB_gtt_entry.
Hi Greg,
Alex has explained how we figured out the patch. We did analyze the
code and found it possible to reach the vulnerability code. But we
have no physical device in hand to test the driver. So we'd like to
discuss with developers to see if the issue exists or not.
Best regards,
Zheng Wang.
Greg KH 于2022年9月5日周一 16:04写道：
>
> On Mon, Sep 05, 2022 at 03:46:09PM +0800, Zheng Hacker wrote:
> > I rewrote the letter. Hope it works.
> >
> > There is a double-free security bug in split_2MB_gtt_entry.
> >
> > Here is a calling chain :
> > ppgtt_populate_spt->ppgtt_populate_shadow_entry->split_2MB_gtt_entry.
> > If intel_gvt_dma_map_guest_page failed, it will call
> > ppgtt_invalidate_spt, which will finally call ppgtt_free_spt and
> > kfree(spt). But the caller does not notice that, and it will call
> > ppgtt_free_spt again in error path.
> >
> > Fix this by returning the result of ppgtt_invalidate_spt to
> > split_2MB_gtt_entry.
> >
> > Signed-off-by: Zheng Wang
> >
> > ---
> > drivers/gpu/drm/i915/gvt/gtt.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> > index ce0eb03709c3..9f14fded8c0c 100644
> > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > @@ -1215,7 +1215,7 @@ static int split_2MB_gtt_entry(struct intel_vgpu
> > *vgpu,
> > ret = intel_gvt_dma_map_guest_page(vgpu, start_gfn +
> > sub_index,
> >PAGE_SIZE, &dma_addr);
> > if (ret) {
> > - ppgtt_invalidate_spt(spt);
> > + ret = ppgtt_invalidate_spt(spt);
> > return ret;
>
> But now you just lost the original error, shouldn't this succeed even if
> intel_gvt_dma_map_guest_page() failed?
>
> And how are you causing intel_gvt_dma_map_guest_page() to fail in a real
> system?
>
> thanks,
>
> greg k-h
Re: [PATCH v2 4/4] dma-buf: Check status of enable-signaling bit on debug
On 06/09/2022 12:21, Tvrtko Ursulin wrote:
On 06/09/2022 11:43, Christian König wrote:
Am 06.09.22 um 12:20 schrieb Tvrtko Ursulin:
On 06/09/2022 09:39, Christian König wrote:
Am 05.09.22 um 18:35 schrieb Arvind Yadav:
The core DMA-buf framework needs to enable signaling
before the fence is signaled. The core DMA-buf framework
can forget to enable signaling before the fence is signaled.
This sentence is a bit confusing. I'm not a native speaker of
English either, but I suggest something like:
"Fence signaling must be enable to make sure that the
dma_fence_is_signaled() function ever returns true."
To avoid this scenario on the debug kernel, check the
DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT status bit before checking
the signaling bit status to confirm that enable_signaling
is enabled.
This describes the implementation, but we should rather describe the
background of the change. The implementation should be obvious.
Something like this maybe:
"
Since drivers and implementations sometimes mess this up enforce
correct behavior when DEBUG_WW_MUTEX_SLOWPATH is used during debugging.
This should make any implementations bugs resulting in not signaled
fences much more obvious.
"
I think I follow the idea but am not sure coupling (well "coupling"..
not really, but cross-contaminating in a way) dma-fence.c with a
foreign and effectively unrelated concept of a ww mutex is the best way.
Instead, how about a dma-buf specific debug kconfig option?
Yeah, I was thinking about that as well.
Cool, lets see about the specifics below and then decide.
The slowpath config option was just at hand because when you want to
test the slowpath you want to test this here as well.
Condition would then be, according to my understanding of the rules
and expectations, along the lines of:
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..147a9df2c9d0 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,17 @@ dma_fence_is_signaled_locked(struct dma_fence
*fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_DMAFENCE
+ /*
+ * Implementations not providing the enable_signaling
callback are
+ * required to always have signaling enabled or fences are not
+ * guaranteed to ever signal.
+ */
Well that comment is a bit misleading. The intention of the extra
check is to find bugs in the frontend and not the backend.
By backend you mean drivers/dma-buf/dma-fence.c and by front end driver
specific implementations? Or simply users for dma-fence?
Could be that I got confused.. I was reading these two:
* This callback is optional. If this callback is not present, then
the
* driver must always have signaling enabled.
*/
bool (*enable_signaling)(struct dma_fence *fence);
And dma_fence_is_signaled:
* Returns true if the fence was already signaled, false if not. Since
this
* function doesn't enable signaling, it is not guaranteed to ever return
* true if dma_fence_add_callback(), dma_fence_wait() or
* dma_fence_enable_sw_signaling() haven't been called before.
Right, I think I did get confused, apologies. What I was thinking was
probably two separate conditions:
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_DMAFENCE
+ if (WARN_ON_ONCE(!fence->ops->enable_signaling &&
+ !test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
&fence->flags)))
+ return false;
+
+ if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
Or simpler OFC:
if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) {
WARN_ON_ONCE(!fence->ops->enable_signaling);
return false;
}
You could also run this core change past i915 CI to see if it catches anything.
Just send to our trybot and see what happens? With the debug option enabled of
course. Hope it won't be painful.
Regards,
Tvrtko
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Not sure "is signaled" is the best place for the first one or that it
should definitely be added.
Regards,
Tvrtko
In other words somewhere in the drm_syncobj code we have a
dma_fence_is_signaled() call without matching
dma_fence_enable_sw_signaling().
Regards,
Christian.
+ if (!fence->ops->enable_signaling &&
+ !test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return true;
Thoughts?
Regards,
Tvrtko
Signed-off-by: Arvind Yadav
With the improved commit message this patch is Reviewed-by:
Christian König
Regards,
Christian.
---
Changes in v1 :
1- Addressing Christian's comment to replace
CONFIG_DEBUG_WW_MUTE
Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_drv.c
On 2022-09-04 20:15, Jingyu Wang wrote: [...] @@ -565,8 +566,8 @@ module_param_named(timeout_period, amdgpu_watchdog_timer.period, uint, 0644); */ #ifdef CONFIG_DRM_AMDGPU_SI -#if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE) -int amdgpu_si_support = 0; +#if IS_ENABLED(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE) Hint: read the checkpatch warning again more closely, and consider what IS_ENABLED() does and therefore why this is still not quite right. Robin. +int amdgpu_si_support; MODULE_PARM_DESC(si_support, "SI support (1 = enabled, 0 = disabled (default))"); #else int amdgpu_si_support = 1;
Re: [PATCH v2 4/4] vfio/pci: Allow MMIO regions to be exported through dma-buf
On Tue, Sep 06, 2022 at 12:38:44PM +0200, Christian König wrote:
> Am 06.09.22 um 11:51 schrieb Christoph Hellwig:
> > > +{
> > > + struct vfio_pci_dma_buf *priv = dmabuf->priv;
> > > + int rc;
> > > +
> > > + rc = pci_p2pdma_distance_many(priv->vdev->pdev, &attachment->dev, 1,
> > > + true);
> > This should just use pci_p2pdma_distance.
OK
> > > + /*
> > > + * Since the memory being mapped is a device memory it could never be in
> > > + * CPU caches.
> > > + */
> > DMA_ATTR_SKIP_CPU_SYNC doesn't even apply to dma_map_resource, not sure
> > where this wisdom comes from.
Habana driver
> > > + dma_addr = dma_map_resource(
> > > + attachment->dev,
> > > + pci_resource_start(priv->vdev->pdev, priv->index) +
> > > + priv->offset,
> > > + priv->dmabuf->size, dir, DMA_ATTR_SKIP_CPU_SYNC);
> > This is not how P2P addresses are mapped. You need to use
> > dma_map_sgtable and have the proper pgmap for it.
>
> The problem is once more that this is MMIO space, in other words register
> BARs which needs to be exported/imported.
>
> Adding struct pages for it generally sounds like the wrong approach here.
> You can't even access this with the CPU or would trigger potentially
> unwanted hardware actions.
Right, this whole thing is the "standard" that dmabuf has adopted
instead of the struct pages. Once the AMD GPU driver started doing
this some time ago other drivers followed.
Now we have struct pages, almost, but I'm not sure if their limits are
compatible with VFIO? This has to work for small bars as well.
Jason
Re: mesa-22.3.0-devel + linux-5.19.6 + mediapipe: panfrost js fault
On 2022-09-04 05:13, Chris Ruehl wrote: Hi, Something you might have a head up for it, have a mediapipe application for POSE which use the T860 GPU for the calculation but the kernel driver report error (js fault) - I see one or 2 calculation frames on the mat-picture output only before the pipe stop working. Linux bullseye64 5.19.6 #1 SMP PREEMPT Fri Sep 2 02:25:59 UTC 2022 aarch64 GNU/Linux [ 5.164415] panfrost ff9a.gpu: clock rate = 5 [ 5.169845] panfrost ff9a.gpu: [drm:panfrost_devfreq_init [panfrost]] Failed to register cooling device [ 5.169989] panfrost ff9a.gpu: mali-t860 id 0x860 major 0x2 minor 0x0 status 0x0 [ 5.16] panfrost ff9a.gpu: features: ,0407, issues: ,24040400 [ 5.170008] panfrost ff9a.gpu: Features: L2:0x07120206 Shader:0x Tiler:0x0809 Mem:0x1 MMU:0x2830 AS:0xff JS:0x7 [ 5.170017] panfrost ff9a.gpu: shader_present=0xf l2_present=0x1 [ 5.206827] [drm] Initialized panfrost 1.2.0 20180908 for ff9a.gpu on minor 1 ... [ 162.862064] panfrost ff9a.gpu: js fault, js=1, status=DATA_INVALID_FAULT, head=0xaba7100, tail=0xaba7100 [ 162.862269] panfrost ff9a.gpu: js fault, js=1, status=DATA_INVALID_FAULT, head=0xa1e0100, tail=0xa1e0100 Have a RK3399 customized board and compiled the mesa drivers for it meson $1 . build/ \ -D dri-drivers= \ -D vulkan-drivers= \ -D gallium-drivers=panfrost,kmsro \ -D llvm=disabled \ -D libunwind=false \ -D platforms=x11,wayland glmark2 runs flawless scores 588. Same code run on a x86_64 with an AMD GPU working fine. Anything help fix the bug is welcome. This is almost certainly a userspace problem, so you're best off raising a Mesa issue with some more details - an apitrace of the failing application and/or PAN_MESA_DEBUG=trace output capturing the offending invalid descriptors would probably be a good starting point. Robin.
Re: [PATCH] drm/bridge: anx7625: Set HPD irq detect window to 2ms
On Tue, 6 Sept 2022 at 04:58, Xin Ji wrote:
>
> On Mon, Sep 05, 2022 at 06:48:06PM +0200, Robert Foss wrote:
> > Hi Xin,
> >
> > On Sat, 3 Sept 2022 at 15:09, Xin Ji wrote:
> > >
> > > Some panels trigger HPD irq due to noise, the HPD debounce
> > > may be 1.8ms, exceeding the default irq detect window, ~1.4ms.
> > > This patch set HPD irq detection window to 2ms to
> > > tolerate the HPD noise.
> > >
> > > Signed-off-by: Xin Ji
> > > ---
> > > drivers/gpu/drm/bridge/analogix/anx7625.c | 14 ++
> > > drivers/gpu/drm/bridge/analogix/anx7625.h | 6 ++
> > > 2 files changed, 20 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > index c74b5df4cade..0c323b5a1c99 100644
> > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > @@ -1440,6 +1440,20 @@ static void anx7625_start_dp_work(struct
> > > anx7625_data *ctx)
> > >
> > > static int anx7625_read_hpd_status_p0(struct anx7625_data *ctx)
> > > {
> > > + int ret;
> > > +
> > > + /* Set irq detect window to 2ms */
> > > + ret = anx7625_reg_write(ctx, ctx->i2c.tx_p2_client,
> > > + HPD_DET_TIMER_BIT0_7, HPD_TIME & 0xFF);
> > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client,
> > > +HPD_DET_TIMER_BIT8_15,
> > > +(HPD_TIME >> 8) & 0xFF);
> > > + ret |= anx7625_reg_write(ctx, ctx->i2c.tx_p2_client,
> > > +HPD_DET_TIMER_BIT16_23,
> > > +(HPD_TIME >> 16) & 0xFF);
> >
> > Does the HPD debounce timer register need to be written for every HPD
> > status read?
> Hi Robert Foss, yes, it is better to set it in every HPD status check,
> because the
> HPD may be affected by noise, once the chip detect HPD is low, the timer
> register will be automatically set to 1.4ms, so the driver better set it
> in each check loop.
>
> Thanks,
> Xin
> >
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > return anx7625_reg_read(ctx, ctx->i2c.rx_p0_client,
> > > SYSTEM_STSTUS);
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
> > > b/drivers/gpu/drm/bridge/analogix/anx7625.h
> > > index e257a84db962..14f33d6be289 100644
> > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.h
> > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
> > > @@ -132,6 +132,12 @@
> > > #define I2S_SLAVE_MODE 0x08
> > > #define AUDIO_LAYOUT 0x01
> > >
> > > +#define HPD_DET_TIMER_BIT0_7 0xea
> > > +#define HPD_DET_TIMER_BIT8_15 0xeb
> > > +#define HPD_DET_TIMER_BIT16_23 0xec
> > > +/* HPD debounce time 2ms for 27M clock */
> > > +#define HPD_TIME 54000
> > > +
> > > #define AUDIO_CONTROL_REGISTER 0xe6
> > > #define TDM_TIMING_MODE 0x08
> > >
> > > --
> > > 2.25.1
> > >
Reviewed-by: Robert Foss
Applied to drm-misc-next.
Re: [PATCH v4 04/11] drm/i915/mtl: Define engine context layouts
On 01.09.2022 23:03, Radhakrishna Sripada wrote:
> From: Matt Roper
>
> The part of the media and blitter engine contexts that we care about for
> setting up an initial state are the same on MTL as they were on DG2
> (and PVC), so we need to update the driver conditions to re-use the DG2
> context table.
>
> For render/compute engines, the part of the context images are nearly
> the same, although the layout had a very slight change --- one POSH
> register was removed and the placement of some LRI/noops adjusted
> slightly to compensate.
>
> v2:
> - Dg2, mtl xcs offsets slightly vary. Use a separate offsets array(Bala)
> - Drop unused registers in mtl rcs offsets.(Bala)
>
> Bspec: 46261, 46260, 45585
> Cc: Balasubramani Vivekanandan
> Signed-off-by: Matt Roper
> Signed-off-by: Radhakrishna Sripada
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 81 -
> 1 file changed, 79 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 070cec4ff8a4..ecb030ee39cd 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -264,6 +264,38 @@ static const u8 dg2_xcs_offsets[] = {
> END
> };
>
> +static const u8 mtl_xcs_offsets[] = {
> + NOP(1),
> + LRI(13, POSTED),
> + REG16(0x244),
> + REG(0x034),
> + REG(0x030),
> + REG(0x038),
> + REG(0x03c),
> + REG(0x168),
> + REG(0x140),
> + REG(0x110),
> + REG(0x1c0),
> + REG(0x1c4),
> + REG(0x1c8),
> + REG(0x180),
> + REG16(0x2b4),
Comparing Bspec 45585, there are few NOP missing here
> +
> + NOP(1),
> + LRI(9, POSTED),
> + REG16(0x3a8),
> + REG16(0x28c),
> + REG16(0x288),
> + REG16(0x284),
> + REG16(0x280),
> + REG16(0x27c),
> + REG16(0x278),
> + REG16(0x274),
> + REG16(0x270),
> +
> + END
> +};
> +
> static const u8 gen8_rcs_offsets[] = {
> NOP(1),
> LRI(14, POSTED),
> @@ -606,6 +638,47 @@ static const u8 dg2_rcs_offsets[] = {
> END
> };
>
> +static const u8 mtl_rcs_offsets[] = {
> + NOP(1),
> + LRI(13, POSTED),
> + REG16(0x244),
> + REG(0x034),
> + REG(0x030),
> + REG(0x038),
> + REG(0x03c),
> + REG(0x168),
> + REG(0x140),
> + REG(0x110),
> + REG(0x1c0),
> + REG(0x1c4),
> + REG(0x1c8),
> + REG(0x180),
> + REG16(0x2b4),
> +
> + NOP(1),
> + LRI(9, POSTED),
> + REG16(0x3a8),
> + REG16(0x28c),
> + REG16(0x288),
> + REG16(0x284),
> + REG16(0x280),
> + REG16(0x27c),
> + REG16(0x278),
> + REG16(0x274),
> + REG16(0x270),
> +
> + NOP(2),
> + LRI(2, POSTED),
> + REG16(0x5a8),
> + REG16(0x5ac),
> +
> + NOP(6),
> + LRI(1, 0),
> + REG(0x0c8),
> +
> + END
> +};
> +
> #undef END
> #undef REG16
> #undef REG
> @@ -624,7 +697,9 @@ static const u8 *reg_offsets(const struct intel_engine_cs
> *engine)
> !intel_engine_has_relative_mmio(engine));
>
> if (engine->flags & I915_ENGINE_HAS_RCS_REG_STATE) {
> - if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 55))
> + if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 70))
> + return mtl_rcs_offsets;
> + else if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 55))
> return dg2_rcs_offsets;
> else if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50))
> return xehp_rcs_offsets;
> @@ -637,7 +712,9 @@ static const u8 *reg_offsets(const struct intel_engine_cs
> *engine)
> else
> return gen8_rcs_offsets;
> } else {
> - if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 55))
> + if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 70))
> + return mtl_xcs_offsets;
> + else if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 55))
> return dg2_xcs_offsets;
> else if (GRAPHICS_VER(engine->i915) >= 12)
> return gen12_xcs_offsets;
> --
> 2.34.1
>
Re: [PATCH v1 04/11] usb: phy: tegra: switch to using devm_gpiod_get()
On Mon, Sep 05, 2022 at 03:07:48PM -0700, Guenter Roeck wrote: > On 9/5/22 12:55, Andy Shevchenko wrote: > > On Mon, Sep 5, 2022 at 10:51 PM Dmitry Torokhov > > wrote: > > > On Mon, Sep 05, 2022 at 10:41:40PM +0300, Andy Shevchenko wrote: > > > > On Mon, Sep 5, 2022 at 10:40 PM Dmitry Torokhov > > > > wrote: > > > > > On Mon, Sep 05, 2022 at 01:59:44PM +0300, Andy Shevchenko wrote: > > > > > > On Mon, Sep 5, 2022 at 9:32 AM Dmitry Torokhov > > > > > > wrote: ... > > > > > > > + gpiod = devm_gpiod_get(&pdev->dev, > > > > > > > "nvidia,phy-reset", > > > > > > > + GPIOD_OUT_HIGH); > > > > > > > err = PTR_ERR_OR_ZERO(gpiod); > > > > > > > > > > > > What does _OR_ZERO mean now? > > > > > > > > > > This converts a pointer to an error code if a pointer represents > > > > > ERR_PTR() encoded error, or 0 to indicate success. > > > > > > > > Yes, I know that. My point is, how is it useful now (or even before)? > > > > I mean that devm_gpio_get() never returns NULL, right? > > > > > > What does returning NULL have to do with anything. > > > > It has to do with a dead code. If defm_gpiod_get() does not return > > NULL, then why do we even bother to check? > > PTR_ERR_OR_ZERO() converts into an error code (if the pointer is an > ERR_PTR) or 0 if it is a real pointer. Its purpose is not to convert > NULL into 0, its purpose is to convert a pointer either into an error > code or 0. That is what is done here, and it is done all over the place > in the kernel. I don't see your problem with it. Care to explain ? > > > > It converts a pointer > > > to a "classic" return code, with negative errors and 0 on success. > > > > > > It allows to not use multiple IS_ERR/PTR_ERR in the code (I'd need 1 > > > IS_ERR and 2 PTR_ERR, one in dev_err() and another to return). > > > > I don't see how this is relevant. > > You lost me. Really, please explain your problem with PTR_ERR_OR_ZERO(). I don't know what I was thinking about... You, guys, are right, sorry for my noise. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 4/4] vfio/pci: Allow MMIO regions to be exported through dma-buf
On Tue, Sep 6, 2022 at 2:48 PM Jason Gunthorpe wrote:
>
> On Tue, Sep 06, 2022 at 12:38:44PM +0200, Christian König wrote:
> > Am 06.09.22 um 11:51 schrieb Christoph Hellwig:
> > > > +{
> > > > + struct vfio_pci_dma_buf *priv = dmabuf->priv;
> > > > + int rc;
> > > > +
> > > > + rc = pci_p2pdma_distance_many(priv->vdev->pdev, &attachment->dev, 1,
> > > > + true);
> > > This should just use pci_p2pdma_distance.
>
> OK
>
> > > > + /*
> > > > + * Since the memory being mapped is a device memory it could never be
> > > > in
> > > > + * CPU caches.
> > > > + */
> > > DMA_ATTR_SKIP_CPU_SYNC doesn't even apply to dma_map_resource, not sure
> > > where this wisdom comes from.
>
> Habana driver
I hate to throw the ball at someone else, but I actually copied the
code from the amdgpu driver, from amdgpu_vram_mgr_alloc_sgt() iirc.
And if you remember Jason, you asked why we use this specific define
in the original review you did and I replied the following (to which
you agreed and that's why we added the comment):
"The memory behind this specific dma-buf has *always* resided on the
device itself, i.e. it lives only in the 'device' domain (after all,
it maps a PCI bar address which points to the device memory).
Therefore, it was never in the 'CPU' domain and hence, there is no
need to perform a sync of the memory to the CPU's cache, as it was
never inside that cache to begin with.
This is not the same case as with regular memory which is dma-mapped
and then copied into the device using a dma engine. In that case,
the memory started in the 'CPU' domain and moved to the 'device'
domain. When it is unmapped it will indeed be recycled to be used
for another purpose and therefore we need to sync the CPU cache."
Oded
>
> > > > + dma_addr = dma_map_resource(
> > > > + attachment->dev,
> > > > + pci_resource_start(priv->vdev->pdev, priv->index) +
> > > > + priv->offset,
> > > > + priv->dmabuf->size, dir, DMA_ATTR_SKIP_CPU_SYNC);
> > > This is not how P2P addresses are mapped. You need to use
> > > dma_map_sgtable and have the proper pgmap for it.
> >
> > The problem is once more that this is MMIO space, in other words register
> > BARs which needs to be exported/imported.
> >
> > Adding struct pages for it generally sounds like the wrong approach here.
> > You can't even access this with the CPU or would trigger potentially
> > unwanted hardware actions.
>
> Right, this whole thing is the "standard" that dmabuf has adopted
> instead of the struct pages. Once the AMD GPU driver started doing
> this some time ago other drivers followed.
>
> Now we have struct pages, almost, but I'm not sure if their limits are
> compatible with VFIO? This has to work for small bars as well.
>
> Jason
Re: [PATCH 1/3] drm/gma500: Fix BUG: sleeping function called from invalid context errors
On Mon, Sep 5, 2022 at 3:37 PM Hans de Goede wrote:
>
> gma_crtc_page_flip() was holding the event_lock spinlock while calling
> crtc_funcs->mode_set_base() which takes ww_mutex.
>
> The only reason to hold event_lock is to clear gma_crtc->page_flip_event
> on mode_set_base() errors.
>
> Instead unlock it after setting gma_crtc->page_flip_event and on
> errors re-take the lock and clear gma_crtc->page_flip_event it
> it is still set.
Hi Hans, thanks for having a look at gma500.
See comments below.
>
> This fixes the following WARN/stacktrace:
>
> [ 512.122953] BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:870
> [ 512.123004] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 1253,
> name: gnome-shell
> [ 512.123031] preempt_count: 1, expected: 0
> [ 512.123048] RCU nest depth: 0, expected: 0
> [ 512.123066] INFO: lockdep is turned off.
> [ 512.123080] irq event stamp: 0
> [ 512.123094] hardirqs last enabled at (0): [<>] 0x0
> [ 512.123134] hardirqs last disabled at (0): []
> copy_process+0x9fc/0x1de0
> [ 512.123176] softirqs last enabled at (0): []
> copy_process+0x9fc/0x1de0
> [ 512.123207] softirqs last disabled at (0): [<>] 0x0
> [ 512.123233] Preemption disabled at:
> [ 512.123241] [<>] 0x0
> [ 512.123275] CPU: 3 PID: 1253 Comm: gnome-shell Tainted: GW
> 5.19.0+ #1
> [ 512.123304] Hardware name: Packard Bell dot s/SJE01_CT, BIOS V1.10
> 07/23/2013
> [ 512.123323] Call Trace:
> [ 512.123346]
> [ 512.123370] dump_stack_lvl+0x5b/0x77
> [ 512.123412] __might_resched.cold+0xff/0x13a
> [ 512.123458] ww_mutex_lock+0x1e/0xa0
> [ 512.123495] psb_gem_pin+0x2c/0x150 [gma500_gfx]
> [ 512.123601] gma_pipe_set_base+0x76/0x240 [gma500_gfx]
> [ 512.123708] gma_crtc_page_flip+0x95/0x130 [gma500_gfx]
> [ 512.123808] drm_mode_page_flip_ioctl+0x57d/0x5d0
> [ 512.123897] ? drm_mode_cursor2_ioctl+0x10/0x10
> [ 512.123936] drm_ioctl_kernel+0xa1/0x150
> [ 512.123984] drm_ioctl+0x21f/0x420
> [ 512.124025] ? drm_mode_cursor2_ioctl+0x10/0x10
> [ 512.124070] ? rcu_read_lock_bh_held+0xb/0x60
> [ 512.124104] ? lock_release+0x1ef/0x2d0
> [ 512.124161] __x64_sys_ioctl+0x8d/0xd0
> [ 512.124203] do_syscall_64+0x58/0x80
> [ 512.124239] ? do_syscall_64+0x67/0x80
> [ 512.124267] ? trace_hardirqs_on_prepare+0x55/0xe0
> [ 512.124300] ? do_syscall_64+0x67/0x80
> [ 512.124340] ? rcu_read_lock_sched_held+0x10/0x80
> [ 512.124377] entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [ 512.124411] RIP: 0033:0x7fcc4a70740f
> [ 512.124442] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00
> 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2
> 3d 00 f0 ff ff 77 18 48 8b 44 24 18 64 48 2b 04 25 28 00 00
> [ 512.124470] RSP: 002b:7ffda73f5390 EFLAGS: 0246 ORIG_RAX:
> 0010
> [ 512.124503] RAX: ffda RBX: 55cc9e474500 RCX:
> 7fcc4a70740f
> [ 512.124524] RDX: 7ffda73f5420 RSI: c01864b0 RDI:
> 0009
> [ 512.124544] RBP: 7ffda73f5420 R08: 55cc9c0b0cb0 R09:
> 0034
> [ 512.124564] R10: R11: 0246 R12:
> c01864b0
> [ 512.124584] R13: 0009 R14: 55cc9df484d0 R15:
> 55cc9af5d0c0
> [ 512.124647]
>
> Signed-off-by: Hans de Goede
> ---
> drivers/gpu/drm/gma500/gma_display.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/gma_display.c
> b/drivers/gpu/drm/gma500/gma_display.c
> index bd40c040a2c9..cf038e322164 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -532,15 +532,19 @@ int gma_crtc_page_flip(struct drm_crtc *crtc,
> WARN_ON(drm_crtc_vblank_get(crtc) != 0);
>
> gma_crtc->page_flip_event = event;
> + spin_unlock_irqrestore(&dev->event_lock, flags);
>
> /* Call this locked if we want an event at vblank interrupt.
> */
If we don't hold the event_lock around mode_set_base() we could
potentially get a vblank before we do the modeset. That would send the
event prematurely. I think this is what the comment tries to tell us.
-Patrik
> ret = crtc_funcs->mode_set_base(crtc, crtc->x, crtc->y,
> old_fb);
> if (ret) {
> - gma_crtc->page_flip_event = NULL;
> - drm_crtc_vblank_put(crtc);
> + spin_lock_irqsave(&dev->event_lock, flags);
> + if (gma_crtc->page_flip_event) {
> + gma_crtc->page_flip_event = NULL;
> + drm_crtc_vblank_put(crtc);
> + }
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> }
>
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> } else {
> ret
RE: [PATCH v2 01/12] drm/i915: Move locking and unclaimed check into mmio_debug_{suspend, resume}
>-Original Message-
>From: dri-devel On Behalf Of
>Matt Roper
>Sent: Friday, September 2, 2022 7:33 PM
>To: intel-...@lists.freedesktop.org
>Cc: dri-devel@lists.freedesktop.org; Sripada, Radhakrishna
>
>Subject: [PATCH v2 01/12] drm/i915: Move locking and unclaimed check into
>mmio_debug_{suspend, resume}
>
>Moving the locking for MMIO debug (and the final check for unclaimed
>accesses when resuming debug after a userspace-initiated forcewake) will
>make it simpler to completely skip MMIO debug handling on uncores that
>don't support it in future patches.
>
>Signed-off-by: Matt Roper
>Reviewed-by: Radhakrishna Sripada
>---
> drivers/gpu/drm/i915/intel_uncore.c | 41 +++--
> 1 file changed, 21 insertions(+), 20 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>b/drivers/gpu/drm/i915/intel_uncore.c
>index 9b81b2543ce2..e717ea55484a 100644
>--- a/drivers/gpu/drm/i915/intel_uncore.c
>+++ b/drivers/gpu/drm/i915/intel_uncore.c
>@@ -50,23 +50,33 @@ intel_uncore_mmio_debug_init_early(struct
>intel_uncore_mmio_debug *mmio_debug)
> mmio_debug->unclaimed_mmio_check = 1;
> }
>
>-static void mmio_debug_suspend(struct intel_uncore_mmio_debug
>*mmio_debug)
>+static void mmio_debug_suspend(struct intel_uncore *uncore)
/bike-shedding...
It seems like there has been a tend to name functions with the
_unlocked
postfix when the lock is being taken within the function.
Would this be a reasonable name update for these changes?
M
> {
>- lockdep_assert_held(&mmio_debug->lock);
>+ spin_lock(&uncore->debug->lock);
>
> /* Save and disable mmio debugging for the user bypass */
>- if (!mmio_debug->suspend_count++) {
>- mmio_debug->saved_mmio_check = mmio_debug-
>>unclaimed_mmio_check;
>- mmio_debug->unclaimed_mmio_check = 0;
>+ if (!uncore->debug->suspend_count++) {
>+ uncore->debug->saved_mmio_check = uncore->debug-
>>unclaimed_mmio_check;
>+ uncore->debug->unclaimed_mmio_check = 0;
> }
>+
>+ spin_unlock(&uncore->debug->lock);
> }
>
>-static void mmio_debug_resume(struct intel_uncore_mmio_debug
>*mmio_debug)
>+static bool check_for_unclaimed_mmio(struct intel_uncore *uncore);
>+
>+static void mmio_debug_resume(struct intel_uncore *uncore)
> {
>- lockdep_assert_held(&mmio_debug->lock);
>+ spin_lock(&uncore->debug->lock);
>+
>+ if (!--uncore->debug->suspend_count)
>+ uncore->debug->unclaimed_mmio_check = uncore->debug-
>>saved_mmio_check;
>
>- if (!--mmio_debug->suspend_count)
>- mmio_debug->unclaimed_mmio_check = mmio_debug-
>>saved_mmio_check;
>+ if (check_for_unclaimed_mmio(uncore))
>+ drm_info(&uncore->i915->drm,
>+ "Invalid mmio detected during user access\n");
>+
>+ spin_unlock(&uncore->debug->lock);
> }
>
> static const char * const forcewake_domain_names[] = {
>@@ -677,9 +687,7 @@ void intel_uncore_forcewake_user_get(struct
>intel_uncore *uncore)
> spin_lock_irq(&uncore->lock);
> if (!uncore->user_forcewake_count++) {
> intel_uncore_forcewake_get__locked(uncore,
>FORCEWAKE_ALL);
>- spin_lock(&uncore->debug->lock);
>- mmio_debug_suspend(uncore->debug);
>- spin_unlock(&uncore->debug->lock);
>+ mmio_debug_suspend(uncore);
> }
> spin_unlock_irq(&uncore->lock);
> }
>@@ -695,14 +703,7 @@ void intel_uncore_forcewake_user_put(struct
>intel_uncore *uncore)
> {
> spin_lock_irq(&uncore->lock);
> if (!--uncore->user_forcewake_count) {
>- spin_lock(&uncore->debug->lock);
>- mmio_debug_resume(uncore->debug);
>-
>- if (check_for_unclaimed_mmio(uncore))
>- drm_info(&uncore->i915->drm,
>- "Invalid mmio detected during user
>access\n");
>- spin_unlock(&uncore->debug->lock);
>-
>+ mmio_debug_resume(uncore);
> intel_uncore_forcewake_put__locked(uncore,
>FORCEWAKE_ALL);
> }
> spin_unlock_irq(&uncore->lock);
>--
>2.37.2
Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_amdkfd_gpuvm.c
Am 2022-09-05 um 04:38 schrieb Jingyu Wang:
Fix everything checkpatch.pl complained about in amdgpu_amdkfd_gpuvm.c
Signed-off-by: Jingyu Wang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index cbd593f7d553..eff596c60c89 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: MIT
I'm not sure if this is correct. We've used "GPL-2.0 OR MIT" in KFD. In
amdgpu there is currently a mix of licenses. Alex, do you want to make a
call on a consistent one to use in amdgpu?
Other than that, this patch is
Reviewed-by: Felix Kuehling
/*
* Copyright 2014-2018 Advanced Micro Devices, Inc.
*
@@ -1612,6 +1613,7 @@ size_t amdgpu_amdkfd_get_available_memory(struct
amdgpu_device *adev)
uint64_t reserved_for_pt =
ESTIMATE_PT_SIZE(amdgpu_amdkfd_total_mem_size);
size_t available;
+
spin_lock(&kfd_mem_limit.mem_limit_lock);
available = adev->gmc.real_vram_size
- adev->kfd.vram_used_aligned
@@ -2216,7 +2218,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct
amdgpu_device *adev,
{
if (atomic_read(&adev->gmc.vm_fault_info_updated) == 1) {
*mem = *adev->gmc.vm_fault_info;
- mb();
+ mb(); /* make sure read happened */
atomic_set(&adev->gmc.vm_fault_info_updated, 0);
}
return 0;
base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
[PATCH 8/8] drm/tegra: Add Tegra234 support to NVDEC driver
From: Mikko Perttunen
Add support for the Tegra234 version of NVDEC to the NVDEC driver.
This version sports a RISC-V controller and requires a few additional
clocks. After firmware has been loaded, the behavior is, however,
backwards compatible.
Signed-off-by: Mikko Perttunen
---
drivers/gpu/drm/tegra/drm.c | 1 +
drivers/gpu/drm/tegra/nvdec.c | 140 ++
2 files changed, 126 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 6748ec1e0005..a014f11e9edb 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1382,6 +1382,7 @@ static const struct of_device_id host1x_drm_subdevs[] = {
{ .compatible = "nvidia,tegra194-vic", },
{ .compatible = "nvidia,tegra194-nvdec", },
{ .compatible = "nvidia,tegra234-vic", },
+ { .compatible = "nvidia,tegra234-nvdec", },
{ /* sentinel */ }
};
diff --git a/drivers/gpu/drm/tegra/nvdec.c b/drivers/gpu/drm/tegra/nvdec.c
index 05af4d107421..10fd21517281 100644
--- a/drivers/gpu/drm/tegra/nvdec.c
+++ b/drivers/gpu/drm/tegra/nvdec.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -16,18 +17,21 @@
#include
#include
-#include
+#include
#include "drm.h"
#include "falcon.h"
+#include "riscv.h"
#include "vic.h"
+#define NVDEC_FALCON_DEBUGINFO 0x1094
#define NVDEC_TFBIF_TRANSCFG 0x2c44
struct nvdec_config {
const char *firmware;
unsigned int version;
bool supports_sid;
+ bool has_riscv;
bool has_extra_clocks;
};
@@ -40,9 +44,14 @@ struct nvdec {
struct device *dev;
struct clk_bulk_data clks[3];
unsigned int num_clks;
+ struct reset_control *reset;
/* Platform configuration */
const struct nvdec_config *config;
+
+ /* RISC-V specific data */
+ struct tegra_drm_riscv riscv;
+ phys_addr_t carveout_base;
};
static inline struct nvdec *to_nvdec(struct tegra_drm_client *client)
@@ -56,7 +65,7 @@ static inline void nvdec_writel(struct nvdec *nvdec, u32
value,
writel(value, nvdec->regs + offset);
}
-static int nvdec_boot(struct nvdec *nvdec)
+static int nvdec_boot_falcon(struct nvdec *nvdec)
{
#ifdef CONFIG_IOMMU_API
struct iommu_fwspec *spec = dev_iommu_fwspec_get(nvdec->dev);
@@ -92,6 +101,64 @@ static int nvdec_boot(struct nvdec *nvdec)
return 0;
}
+static int nvdec_wait_debuginfo(struct nvdec *nvdec, const char *phase)
+{
+ int err;
+ u32 val;
+
+ err = readl_poll_timeout(nvdec->regs + NVDEC_FALCON_DEBUGINFO, val, val
== 0x0, 10, 10);
+ if (err) {
+ dev_err(nvdec->dev, "failed to boot %s, debuginfo=0x%x\n",
phase, val);
+ return err;
+ }
+
+ return 0;
+}
+
+static int nvdec_boot_riscv(struct nvdec *nvdec)
+{
+ int err;
+
+ err = reset_control_acquire(nvdec->reset);
+ if (err)
+ return err;
+
+ nvdec_writel(nvdec, 0xabcd1234, NVDEC_FALCON_DEBUGINFO);
+
+ err = tegra_drm_riscv_boot_bootrom(&nvdec->riscv, nvdec->carveout_base,
1,
+ &nvdec->riscv.bl_desc);
+ if (err) {
+ dev_err(nvdec->dev, "failed to execute bootloader\n");
+ goto release_reset;
+ }
+
+ err = nvdec_wait_debuginfo(nvdec, "bootloader");
+ if (err)
+ goto release_reset;
+
+ err = reset_control_reset(nvdec->reset);
+ if (err)
+ goto release_reset;
+
+ nvdec_writel(nvdec, 0xabcd1234, NVDEC_FALCON_DEBUGINFO);
+
+ err = tegra_drm_riscv_boot_bootrom(&nvdec->riscv, nvdec->carveout_base,
1,
+ &nvdec->riscv.os_desc);
+ if (err) {
+ dev_err(nvdec->dev, "failed to execute firmware\n");
+ goto release_reset;
+ }
+
+ err = nvdec_wait_debuginfo(nvdec, "firmware");
+ if (err)
+ goto release_reset;
+
+release_reset:
+ reset_control_release(nvdec->reset);
+
+ return err;
+}
+
static int nvdec_init(struct host1x_client *client)
{
struct tegra_drm_client *drm = host1x_to_drm_client(client);
@@ -191,7 +258,7 @@ static const struct host1x_client_ops nvdec_client_ops = {
.exit = nvdec_exit,
};
-static int nvdec_load_firmware(struct nvdec *nvdec)
+static int nvdec_load_falcon_firmware(struct nvdec *nvdec)
{
struct host1x_client *client = &nvdec->client.base;
struct tegra_drm *tegra = nvdec->client.drm;
@@ -254,7 +321,6 @@ static int nvdec_load_firmware(struct nvdec *nvdec)
return err;
}
-
static __maybe_unused int nvdec_runtime_resume(struct device *dev)
{
struct nvdec *nvdec = dev_get_drvdata(dev);
@@ -266,13 +332,19 @@ static __maybe_unused int nvdec_runtime_resume(struct
device *dev)
usleep_range(10, 20);
-
[PATCH 7/8] drm/tegra: Add code for booting RISC-V based engines
From: Mikko Perttunen
Add helper code for booting RISC-V based engines where firmware is
located in a carveout.
Signed-off-by: Mikko Perttunen
---
drivers/gpu/drm/tegra/Makefile | 3 +-
drivers/gpu/drm/tegra/riscv.c | 106 +
drivers/gpu/drm/tegra/riscv.h | 30 ++
3 files changed, 138 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/tegra/riscv.c
create mode 100644 drivers/gpu/drm/tegra/riscv.h
diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
index df6cc986aeba..bb0d2c144b55 100644
--- a/drivers/gpu/drm/tegra/Makefile
+++ b/drivers/gpu/drm/tegra/Makefile
@@ -24,7 +24,8 @@ tegra-drm-y := \
gr3d.o \
falcon.o \
vic.o \
- nvdec.o
+ nvdec.o \
+ riscv.o
tegra-drm-y += trace.o
diff --git a/drivers/gpu/drm/tegra/riscv.c b/drivers/gpu/drm/tegra/riscv.c
new file mode 100644
index ..6580416408f8
--- /dev/null
+++ b/drivers/gpu/drm/tegra/riscv.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022, NVIDIA Corporation.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "riscv.h"
+
+#define RISCV_CPUCTL 0x4388
+#define RISCV_CPUCTL_STARTCPU_TRUE (1 << 0)
+#define RISCV_BR_RETCODE 0x465c
+#define RISCV_BR_RETCODE_RESULT_V(x) ((x) & 0x3)
+#define RISCV_BR_RETCODE_RESULT_PASS_V 3
+#define RISCV_BCR_CTRL 0x4668
+#define RISCV_BCR_CTRL_CORE_SELECT_RISCV (1 << 4)
+#define RISCV_BCR_DMACFG 0x466c
+#define RISCV_BCR_DMACFG_TARGET_LOCAL_FB (0 << 0)
+#define RISCV_BCR_DMACFG_LOCK_LOCKED (1 << 31)
+#define RISCV_BCR_DMAADDR_PKCPARAM_LO 0x4670
+#define RISCV_BCR_DMAADDR_PKCPARAM_HI 0x4674
+#define RISCV_BCR_DMAADDR_FMCCODE_LO 0x4678
+#define RISCV_BCR_DMAADDR_FMCCODE_HI 0x467c
+#define RISCV_BCR_DMAADDR_FMCDATA_LO 0x4680
+#define RISCV_BCR_DMAADDR_FMCDATA_HI 0x4684
+#define RISCV_BCR_DMACFG_SEC 0x4694
+#define RISCV_BCR_DMACFG_SEC_GSCID(v) ((v) << 16)
+
+static void riscv_writel(struct tegra_drm_riscv *riscv, u32 value, u32 offset)
+{
+ writel(value, riscv->regs + offset);
+}
+
+int tegra_drm_riscv_read_descriptors(struct tegra_drm_riscv *riscv)
+{
+ struct tegra_drm_riscv_descriptor *bl = &riscv->bl_desc;
+ struct tegra_drm_riscv_descriptor *os = &riscv->os_desc;
+ const struct device_node *np = riscv->dev->of_node;
+ int err;
+
+#define READ_PROP(name, location) \
+ err = of_property_read_u32(np, name, location); \
+ if (err) { \
+ dev_err(riscv->dev, "failed to read " name ": %d\n", err); \
+ return err; \
+ }
+
+ READ_PROP("nvidia,bl-manifest-offset", &bl->manifest_offset);
+ READ_PROP("nvidia,bl-code-offset", &bl->code_offset);
+ READ_PROP("nvidia,bl-data-offset", &bl->data_offset);
+ READ_PROP("nvidia,os-manifest-offset", &os->manifest_offset);
+ READ_PROP("nvidia,os-code-offset", &os->code_offset);
+ READ_PROP("nvidia,os-data-offset", &os->data_offset);
+#undef READ_PROP
+
+ if (bl->manifest_offset == 0 && bl->code_offset == 0 &&
+ bl->data_offset == 0 && os->manifest_offset == 0 &&
+ os->code_offset == 0 && os->data_offset == 0) {
+ dev_err(riscv->dev, "descriptors not available\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+int tegra_drm_riscv_boot_bootrom(struct tegra_drm_riscv *riscv, phys_addr_t
image_address,
+u32 gscid, const struct
tegra_drm_riscv_descriptor *desc)
+{
+ phys_addr_t addr;
+ int err;
+ u32 val;
+
+ riscv_writel(riscv, RISCV_BCR_CTRL_CORE_SELECT_RISCV, RISCV_BCR_CTRL);
+
+ addr = image_address + desc->manifest_offset;
+ riscv_writel(riscv, lower_32_bits(addr >> 8),
RISCV_BCR_DMAADDR_PKCPARAM_LO);
+ riscv_writel(riscv, upper_32_bits(addr >> 8),
RISCV_BCR_DMAADDR_PKCPARAM_HI);
+
+ addr = image_address + desc->code_offset;
+ riscv_writel(riscv, lower_32_bits(addr >> 8),
RISCV_BCR_DMAADDR_FMCCODE_LO);
+ riscv_writel(riscv, upper_32_bits(addr >> 8),
RISCV_BCR_DMAADDR_FMCCODE_HI);
+
+ addr = image_address + desc->data_offset;
+ riscv_writel(riscv, lower_32_bits(addr >> 8),
RISCV_BCR_DMAADDR_FMCDATA_LO);
+ riscv_writel(riscv, upper_32_bits(addr >> 8),
RISCV_BCR_DMAADDR_FMCDATA_HI);
+
+ riscv_writel(riscv, RISCV_BCR_DMACFG_SEC_GSCID(gscid),
RISCV_BCR_DMACFG_SEC);
+ riscv_writel(riscv,
+ RISCV_BCR_DMACFG_TARGET_LOCAL_FB |
RISCV_BCR_DMACFG_LOCK_LOCKED, RISCV_BCR_DMACFG);
+
+ riscv_writel(riscv, RISCV_CPUCTL_STARTCPU_TRUE, RIS
[PATCH 6/8] drm/tegra: nvdec: Support multiple clocks
From: Mikko Perttunen
NVDEC on Tegra234 requires multiple clocks. Add support for that.
Signed-off-by: Mikko Perttunen
---
drivers/gpu/drm/tegra/nvdec.c | 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/tegra/nvdec.c b/drivers/gpu/drm/tegra/nvdec.c
index 276fe0472730..05af4d107421 100644
--- a/drivers/gpu/drm/tegra/nvdec.c
+++ b/drivers/gpu/drm/tegra/nvdec.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (c) 2015-2021, NVIDIA Corporation.
+ * Copyright (c) 2015-2022, NVIDIA Corporation.
*/
#include
@@ -28,6 +28,7 @@ struct nvdec_config {
const char *firmware;
unsigned int version;
bool supports_sid;
+ bool has_extra_clocks;
};
struct nvdec {
@@ -37,7 +38,8 @@ struct nvdec {
struct tegra_drm_client client;
struct host1x_channel *channel;
struct device *dev;
- struct clk *clk;
+ struct clk_bulk_data clks[3];
+ unsigned int num_clks;
/* Platform configuration */
const struct nvdec_config *config;
@@ -258,7 +260,7 @@ static __maybe_unused int nvdec_runtime_resume(struct
device *dev)
struct nvdec *nvdec = dev_get_drvdata(dev);
int err;
- err = clk_prepare_enable(nvdec->clk);
+ err = clk_bulk_prepare_enable(nvdec->num_clks, nvdec->clks);
if (err < 0)
return err;
@@ -275,7 +277,7 @@ static __maybe_unused int nvdec_runtime_resume(struct
device *dev)
return 0;
disable:
- clk_disable_unprepare(nvdec->clk);
+ clk_bulk_disable_unprepare(nvdec->num_clks, nvdec->clks);
return err;
}
@@ -285,7 +287,7 @@ static __maybe_unused int nvdec_runtime_suspend(struct
device *dev)
host1x_channel_stop(nvdec->channel);
- clk_disable_unprepare(nvdec->clk);
+ clk_bulk_disable_unprepare(nvdec->num_clks, nvdec->clks);
return 0;
}
@@ -383,13 +385,22 @@ static int nvdec_probe(struct platform_device *pdev)
if (IS_ERR(nvdec->regs))
return PTR_ERR(nvdec->regs);
- nvdec->clk = devm_clk_get(dev, NULL);
- if (IS_ERR(nvdec->clk)) {
- dev_err(&pdev->dev, "failed to get clock\n");
- return PTR_ERR(nvdec->clk);
+ nvdec->clks[0].id = "nvdec";
+ nvdec->num_clks = 1;
+
+ if (nvdec->config->has_extra_clocks) {
+ nvdec->num_clks = 3;
+ nvdec->clks[1].id = "fuse";
+ nvdec->clks[2].id = "tsec_pka";
+ }
+
+ err = devm_clk_bulk_get(dev, nvdec->num_clks, nvdec->clks);
+ if (err) {
+ dev_err(&pdev->dev, "failed to get clock(s)\n");
+ return err;
}
- err = clk_set_rate(nvdec->clk, ULONG_MAX);
+ err = clk_set_rate(nvdec->clks[0].clk, ULONG_MAX);
if (err < 0) {
dev_err(&pdev->dev, "failed to set clock rate\n");
return err;
--
2.37.0
[PATCH 5/8] gpu: host1x: Add stream ID register data for NVDEC on Tegra234
From: Mikko Perttunen
Add entries for NVDEC to the Tegra234 SID table.
Signed-off-by: Mikko Perttunen
---
drivers/gpu/host1x/dev.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index 0cd3f97e7e49..d6b4614f968f 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -225,6 +225,18 @@ static const struct host1x_sid_entry tegra234_sid_table[]
= {
.offset = 0x34,
.limit = 0x34
},
+ {
+ /* NVDEC channel */
+ .base = 0x17c8,
+ .offset = 0x30,
+ .limit = 0x30,
+ },
+ {
+ /* NVDEC MMIO */
+ .base = 0x1698,
+ .offset = 0x34,
+ .limit = 0x34,
+ },
};
static const struct host1x_info host1x08_info = {
--
2.37.0
[PATCH 4/8] arm64: tegra: Add NVDEC on Tegra234
From: Mikko Perttunen
Add a device tree node for NVDEC on Tegra234.
Booting the firmware requires some information regarding offsets
within the firmware binary. These are passed through the device
tree, but since the values vary depending on the firmware version,
and the firmware itself is not available to the OS, the flasher is
expected to provide a device tree overlay with values corresponding
to the firmware it is flashing. The overlay then replaces the
placeholder values here.
Signed-off-by: Mikko Perttunen
---
arch/arm64/boot/dts/nvidia/tegra234.dtsi | 27
1 file changed, 27 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index 81a0f599685f..65d49b27bc5f 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -586,6 +586,33 @@ vic@1534 {
iommus = <&smmu_niso1 TEGRA234_SID_VIC>;
dma-coherent;
};
+
+ nvdec@1548 {
+ compatible = "nvidia,tegra234-nvdec";
+ reg = <0x1548 0x0004>;
+ clocks = <&bpmp TEGRA234_CLK_NVDEC>,
+<&bpmp TEGRA234_CLK_FUSE>,
+<&bpmp TEGRA234_CLK_TSEC_PKA>;
+ clock-names = "nvdec", "fuse", "tsec_pka";
+ resets = <&bpmp TEGRA234_RESET_NVDEC>;
+ reset-names = "nvdec";
+ power-domains = <&bpmp
TEGRA234_POWER_DOMAIN_NVDEC>;
+ interconnects = <&mc
TEGRA234_MEMORY_CLIENT_NVDECSRD &emc>,
+ <&mc
TEGRA234_MEMORY_CLIENT_NVDECSWR &emc>;
+ interconnect-names = "dma-mem", "write";
+ iommus = <&smmu_niso1 TEGRA234_SID_NVDEC>;
+ dma-coherent;
+
+ nvidia,memory-controller = <&mc>;
+
+ /* Placeholder values, to be replaced with
values from overlay */
+ nvidia,bl-manifest-offset = <0>;
+ nvidia,bl-data-offset = <0>;
+ nvidia,bl-code-offset = <0>;
+ nvidia,os-manifest-offset = <0>;
+ nvidia,os-data-offset = <0>;
+ nvidia,os-code-offset = <0>;
+ };
};
gpio: gpio@220 {
--
2.37.0
[PATCH 0/8] Support for NVDEC on Tegra234
From: Mikko Perttunen Hi all, this series adds support for the HW video decoder, NVDEC, on Tegra234 (Orin). The main change is a switch from Falcon to RISC-V for the internal microcontroller, which brings along a change in how the engine is booted. Otherwise it is backwards compatible with earlier versions. In previous iterations, firmware was simply loaded from disk and written into engine internal memory. Now, the engine has a bootrom that loads the firmware from a carveout where it has been loaded by the system bootloader; however, we still need to tell it where that carveout is loaded and some offsets into it. For that, the first patch adds a new memory controller API to query the carveout address. The offsets are read from device tree -- the expectation is that at flashing time (when the firmware is also flashed), the flasher also delivers a device tree overlay with values corresponding to the flashed firmware. The currently available Linux for Tegra release doesn't yet include this device tree overlay flashing, and the firmware version it contains is incompatible with this series. The plan is to fix that for the next Linux for Tegra release, but if necessary, we can postpone merging of this series to once those changes are available. Thanks! Mikko Mikko Perttunen (8): memory: tegra: Add API for retrieving carveout bounds dt-bindings: Add headers for NVDEC on Tegra234 dt-bindings: Add bindings for Tegra234 NVDEC arm64: tegra: Add NVDEC on Tegra234 gpu: host1x: Add stream ID register data for NVDEC on Tegra234 drm/tegra: nvdec: Support multiple clocks drm/tegra: Add code for booting RISC-V based engines drm/tegra: Add Tegra234 support to NVDEC driver .../gpu/host1x/nvidia,tegra210-nvdec.yaml | 118 ++-- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 27 +++ drivers/gpu/drm/tegra/Makefile| 3 +- drivers/gpu/drm/tegra/drm.c | 1 + drivers/gpu/drm/tegra/nvdec.c | 171 +++--- drivers/gpu/drm/tegra/riscv.c | 106 +++ drivers/gpu/drm/tegra/riscv.h | 30 +++ drivers/gpu/host1x/dev.c | 12 ++ drivers/memory/tegra/mc.c | 23 +++ drivers/memory/tegra/tegra234.c | 5 + include/dt-bindings/clock/tegra234-clock.h| 4 + include/dt-bindings/memory/tegra234-mc.h | 3 + .../dt-bindings/power/tegra234-powergate.h| 1 + include/dt-bindings/reset/tegra234-reset.h| 1 + include/soc/tegra/mc.h| 11 ++ 15 files changed, 470 insertions(+), 46 deletions(-) create mode 100644 drivers/gpu/drm/tegra/riscv.c create mode 100644 drivers/gpu/drm/tegra/riscv.h -- 2.37.0
[PATCH 1/8] memory: tegra: Add API for retrieving carveout bounds
From: Mikko Perttunen
On Tegra234 NVDEC firmware is loaded from a secure carveout, where it
has been loaded by a bootloader. When booting NVDEC, we need to tell it
the address of this firmware, which we can determine by checking the
starting address of the carveout. As such, add an MC API to query the
bounds of carveouts, and add related information on Tegra234.
Signed-off-by: Mikko Perttunen
---
drivers/memory/tegra/mc.c | 23 +++
drivers/memory/tegra/tegra234.c | 5 +
include/soc/tegra/mc.h | 11 +++
3 files changed, 39 insertions(+)
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index 2f7a58a9df1a..4650300d3ec3 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -107,6 +107,29 @@ int tegra_mc_probe_device(struct tegra_mc *mc, struct
device *dev)
}
EXPORT_SYMBOL_GPL(tegra_mc_probe_device);
+int tegra_mc_get_carveout_info(struct tegra_mc *mc, unsigned int id,
+ phys_addr_t *base, u64 *size)
+{
+ u32 offset;
+
+ if (id < 1 || id >= mc->soc->num_carveouts)
+ return -EINVAL;
+
+ if (id < 6)
+ offset = 0xc0c + 0x50 * (id - 1);
+ else
+ offset = 0x2004 + 0x50 * (id - 6);
+
+ *base = mc_ch_readl(mc, MC_BROADCAST_CHANNEL, offset + 0x0);
+ *base |= (phys_addr_t)mc_ch_readl(mc, MC_BROADCAST_CHANNEL, offset +
0x4) << 32;
+
+ if (size)
+ *size = mc_ch_readl(mc, MC_BROADCAST_CHANNEL, offset + 0x8) <<
17;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tegra_mc_get_carveout_info);
+
static int tegra_mc_block_dma_common(struct tegra_mc *mc,
const struct tegra_mc_reset *rst)
{
diff --git a/drivers/memory/tegra/tegra234.c b/drivers/memory/tegra/tegra234.c
index a9e8fd99730f..74d291d66366 100644
--- a/drivers/memory/tegra/tegra234.c
+++ b/drivers/memory/tegra/tegra234.c
@@ -187,4 +187,9 @@ const struct tegra_mc_soc tegra234_mc_soc = {
.ops = &tegra186_mc_ops,
.ch_intmask = 0xff00,
.global_intstatus_channel_shift = 8,
+ /*
+* Additionally, there are lite carveouts but those are not currently
+* supported.
+*/
+ .num_carveouts = 32,
};
diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h
index 47ce6d434427..51a2263e1bc5 100644
--- a/include/soc/tegra/mc.h
+++ b/include/soc/tegra/mc.h
@@ -193,6 +193,8 @@ struct tegra_mc_soc {
unsigned int num_address_bits;
unsigned int atom_size;
+ unsigned int num_carveouts;
+
u16 client_id_mask;
u8 num_channels;
@@ -244,6 +246,8 @@ unsigned int tegra_mc_get_emem_device_count(struct tegra_mc
*mc);
#ifdef CONFIG_TEGRA_MC
struct tegra_mc *devm_tegra_memory_controller_get(struct device *dev);
int tegra_mc_probe_device(struct tegra_mc *mc, struct device *dev);
+int tegra_mc_get_carveout_info(struct tegra_mc *mc, unsigned int id,
+ phys_addr_t *base, u64 *size);
#else
static inline struct tegra_mc *
devm_tegra_memory_controller_get(struct device *dev)
@@ -256,6 +260,13 @@ tegra_mc_probe_device(struct tegra_mc *mc, struct device
*dev)
{
return -ENODEV;
}
+
+static inline int
+tegra_mc_get_carveout_info(struct tegra_mc *mc, unsigned int id,
+ phys_addr_t *base, u64 *size)
+{
+ return -ENODEV;
+}
#endif
#endif /* __SOC_TEGRA_MC_H__ */
--
2.37.0
[PATCH 2/8] dt-bindings: Add headers for NVDEC on Tegra234
From: Mikko Perttunen Add clock, memory controller, powergate and reset dt-binding headers necessary for NVDEC. Signed-off-by: Mikko Perttunen --- include/dt-bindings/clock/tegra234-clock.h | 4 include/dt-bindings/memory/tegra234-mc.h | 3 +++ include/dt-bindings/power/tegra234-powergate.h | 1 + include/dt-bindings/reset/tegra234-reset.h | 1 + 4 files changed, 9 insertions(+) diff --git a/include/dt-bindings/clock/tegra234-clock.h b/include/dt-bindings/clock/tegra234-clock.h index 173364a93381..25b4a3fb4588 100644 --- a/include/dt-bindings/clock/tegra234-clock.h +++ b/include/dt-bindings/clock/tegra234-clock.h @@ -82,6 +82,8 @@ #define TEGRA234_CLK_I2S6 66U /** @brief clock recovered from I2S6 input */ #define TEGRA234_CLK_I2S6_SYNC_INPUT 67U +/** @brief output of mux controlled by CLK_RST_CONTROLLER_CLK_SOURCE_NVDEC */ +#define TEGRA234_CLK_NVDEC 83U /** PLL controlled by CLK_RST_CONTROLLER_PLLA_BASE for use by audio clocks */ #define TEGRA234_CLK_PLLA 93U /** @brief PLLP clk output */ @@ -130,6 +132,8 @@ #define TEGRA234_CLK_SYNC_I2S5 149U /** @brief output of mux controlled by CLK_RST_CONTROLLER_AUDIO_SYNC_CLK_I2S6 */ #define TEGRA234_CLK_SYNC_I2S6 150U +/** output of mux controlled by CLK_RST_CONTROLLER_CLK_SOURCE_PKA */ +#define TEGRA234_CLK_TSEC_PKA 154U /** @brief output of mux controlled by CLK_RST_CONTROLLER_CLK_SOURCE_UARTA */ #define TEGRA234_CLK_UARTA 155U /** @brief output of gate CLK_ENB_PEX1_CORE_6 */ diff --git a/include/dt-bindings/memory/tegra234-mc.h b/include/dt-bindings/memory/tegra234-mc.h index 62987b47ce81..75f0bd30d365 100644 --- a/include/dt-bindings/memory/tegra234-mc.h +++ b/include/dt-bindings/memory/tegra234-mc.h @@ -32,6 +32,7 @@ #define TEGRA234_SID_PCIE100x0b #define TEGRA234_SID_BPMP 0x10 #define TEGRA234_SID_HOST1X0x27 +#define TEGRA234_SID_NVDEC 0x29 #define TEGRA234_SID_VIC 0x34 /* @@ -91,6 +92,8 @@ #define TEGRA234_MEMORY_CLIENT_SDMMCWAB 0x67 #define TEGRA234_MEMORY_CLIENT_VICSRD 0x6c #define TEGRA234_MEMORY_CLIENT_VICSWR 0x6d +#define TEGRA234_MEMORY_CLIENT_NVDECSRD 0x78 +#define TEGRA234_MEMORY_CLIENT_NVDECSWR 0x79 /* BPMP read client */ #define TEGRA234_MEMORY_CLIENT_BPMPR 0x93 /* BPMP write client */ diff --git a/include/dt-bindings/power/tegra234-powergate.h b/include/dt-bindings/power/tegra234-powergate.h index ae9286cef85c..e5dc1e00be95 100644 --- a/include/dt-bindings/power/tegra234-powergate.h +++ b/include/dt-bindings/power/tegra234-powergate.h @@ -19,6 +19,7 @@ #define TEGRA234_POWER_DOMAIN_MGBEB18U #define TEGRA234_POWER_DOMAIN_MGBEC19U #define TEGRA234_POWER_DOMAIN_MGBED20U +#define TEGRA234_POWER_DOMAIN_NVDEC 23U #define TEGRA234_POWER_DOMAIN_VIC 29U #endif diff --git a/include/dt-bindings/reset/tegra234-reset.h b/include/dt-bindings/reset/tegra234-reset.h index d48d22b2bc7f..17163019316c 100644 --- a/include/dt-bindings/reset/tegra234-reset.h +++ b/include/dt-bindings/reset/tegra234-reset.h @@ -30,6 +30,7 @@ #define TEGRA234_RESET_I2C733U #define TEGRA234_RESET_I2C834U #define TEGRA234_RESET_I2C935U +#define TEGRA234_RESET_NVDEC44U #define TEGRA234_RESET_MGBE0_PCS 45U #define TEGRA234_RESET_MGBE0_MAC 46U #define TEGRA234_RESET_MGBE1_PCS 49U -- 2.37.0
[PATCH 3/8] dt-bindings: Add bindings for Tegra234 NVDEC
From: Mikko Perttunen Update NVDEC bindings for Tegra234. This new engine version only has two memory clients, but now requires three clocks, and as a bigger change the engine loads firmware from a secure carveout configured by the bootloader. For the latter, we need to add a phandle to the memory controller to query the location of this carveout, and several other properties containing offsets into the firmware inside the carveout. These properties are intended to be populated through a device tree overlay configured at flashing time, so that the values correspond to the flashed NVDEC firmware. Signed-off-by: Mikko Perttunen --- .../gpu/host1x/nvidia,tegra210-nvdec.yaml | 118 +++--- 1 file changed, 98 insertions(+), 20 deletions(-) diff --git a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml index 3cf862976448..27128a195b66 100644 --- a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml +++ b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml @@ -24,17 +24,11 @@ properties: - nvidia,tegra210-nvdec - nvidia,tegra186-nvdec - nvidia,tegra194-nvdec + - nvidia,tegra234-nvdec reg: maxItems: 1 - clocks: -maxItems: 1 - - clock-names: -items: - - const: nvdec - resets: maxItems: 1 @@ -50,18 +44,6 @@ properties: dma-coherent: true - interconnects: -items: - - description: DMA read memory client - - description: DMA read 2 memory client - - description: DMA write memory client - - interconnect-names: -items: - - const: dma-mem - - const: read-1 - - const: write - nvidia,host1x-class: description: | Host1x class of the engine, used to specify the targeted engine @@ -79,7 +61,103 @@ required: - reset-names - power-domains -additionalProperties: false +unevaluatedProperties: false + +allOf: + - if: + properties: +compatible: + contains: +enum: + - nvidia,tegra234-nvdec +then: + properties: +clocks: + items: +- description: NVDEC clock +- description: FUSE clock +- description: TSEC_PKA clock +clock-names: + items: +- const: nvdec +- const: fuse +- const: tsec_pka +interconnects: + items: +- description: DMA read memory client +- description: DMA write memory client +interconnect-names: + items: +- const: dma-mem +- const: write +nvidia,memory-controller: + $ref: /schemas/types.yaml#/definitions/phandle + description: +phandle to the memory controller for determining carveout information. +nvidia,bl-manifest-offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: +Offset to bootloader manifest from beginning of firmware. Typically set as +part of a device tree overlay corresponding to flashed firmware. +nvidia,bl-code-offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: +Offset to bootloader code section from beginning of firmware. Typically set as +part of a device tree overlay corresponding to flashed firmware. +nvidia,bl-data-offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: +Offset to bootloader data section from beginning of firmware. Typically set as +part of a device tree overlay corresponding to flashed firmware. +nvidia,os-manifest-offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: +Offset to operating system manifest from beginning of firmware. Typically set as +part of a device tree overlay corresponding to flashed firmware. +nvidia,os-code-offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: +Offset to operating system code section from beginning of firmware. Typically set as +part of a device tree overlay corresponding to flashed firmware. +nvidia,os-data-offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: +Offset to operating system data section from beginning of firmware. Typically set as +part of a device tree overlay corresponding to flashed firmware. + required: +- nvidia,memory-controller +- nvidia,bl-manifest-offset +- nvidia,bl-code-offset +- nvidia,bl-data-offset +- nvidia,os-manifest-offset +- nvidia,os-code-offset +- nvidia,os-data-offset + - if: + properties: +compatible: + contains: +enum: + - nvidia,tegra2
[PATCH v3 0/4] drm: Use full allocated minor range for DRM
64 DRM device nodes is not enough for everyone. Upgrade it to ~512K (which definitely is more than enough). To allow testing userspace support for >64 devices, add additional DRM modparam (skip_legacy_minors) which causes DRM to skip allocating minors in 0-192 range. Additionally - one minor tweak around minor DRM IDR locking and IDR lockdep annotations. v1 -> v2: Don't touch DRM_MINOR_CONTROL and its range (Simon Ser) v2 -> v3: Don't use legacy scheme for >=192 minor range (Dave Airlie) Add modparam for testing (Dave Airlie) Add lockdep annotation for IDR (Daniel Vetter) Michał Winiarski (4): drm: Expand max DRM device number to full MINORBITS drm: Introduce skip_legacy_minors modparam drm: Use mutex for minors idr: Add might_alloc() annotation drivers/gpu/drm/drm_drv.c | 55 ++- lib/radix-tree.c | 3 +++ 2 files changed, 34 insertions(+), 24 deletions(-) -- 2.37.3
[PATCH v3 1/4] drm: Expand max DRM device number to full MINORBITS
Having a limit of 64 DRM devices is not good enough for modern world where we have multi-GPU servers, SR-IOV virtual functions and virtual devices used for testing. Let's utilize full minor range for DRM devices. To avoid regressing the existing userspace, we're still maintaining the numbering scheme where 0-63 is used for primary, 64-127 is reserved (formerly for control) and 128-191 is used for render. For minors >= 192, we're allocating minors dynamically on a first-come, first-served basis. Signed-off-by: Michał Winiarski --- drivers/gpu/drm/drm_drv.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 8214a0b1ab7f..9432b1619602 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -122,6 +122,12 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type) minor->type = type; minor->dev = dev; + /* +* DRM used to support 64 devices, for backwards compatibility we need to maintain the +* minor allocation scheme where minors 0-63 are primary nodes, 64-127 are control nodes, +* and 128-191 are render nodes. +* After reaching the limit, we're allocating minors dynamically - first-come, first-serve. +*/ idr_preload(GFP_KERNEL); spin_lock_irqsave(&drm_minor_lock, flags); r = idr_alloc(&drm_minors_idr, @@ -129,6 +135,8 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type) 64 * type, 64 * (type + 1), GFP_NOWAIT); + if (r == -ENOSPC) + r = idr_alloc(&drm_minors_idr, NULL, 192, 1 << MINORBITS, GFP_NOWAIT); spin_unlock_irqrestore(&drm_minor_lock, flags); idr_preload_end(); -- 2.37.3
[PATCH v3 2/4] drm: Introduce skip_legacy_minors modparam
While there is support for >64 DRM devices on kernel side, existing
userspace may still have some hardcoded assumptions and it's possible
that it will require changes to be able to use more than 64 devices.
Add a modparam to simplify testing and development of >64 devices
support on userspace side by allocating minors from the >=192 range
(without the need of having >64 physical devices connected).
Signed-off-by: Michał Winiarski
---
drivers/gpu/drm/drm_drv.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9432b1619602..0bdcca0db611 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -56,6 +56,11 @@ MODULE_LICENSE("GPL and additional rights");
static DEFINE_SPINLOCK(drm_minor_lock);
static struct idr drm_minors_idr;
+static bool skip_legacy_minors;
+module_param_unsafe(skip_legacy_minors, bool, 0400);
+MODULE_PARM_DESC(skip_legacy_minors,
+"Don't allocate minors in 0-192 range. This can be used for
testing userspace support for >64 drm devices (default: false)");
+
/*
* If the drm core fails to init for whatever reason,
* we should prevent any drivers from registering with it.
@@ -113,7 +118,7 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned
int type)
{
struct drm_minor *minor;
unsigned long flags;
- int r;
+ int r = -ENOSPC;
minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL);
if (!minor)
@@ -130,11 +135,12 @@ static int drm_minor_alloc(struct drm_device *dev,
unsigned int type)
*/
idr_preload(GFP_KERNEL);
spin_lock_irqsave(&drm_minor_lock, flags);
- r = idr_alloc(&drm_minors_idr,
- NULL,
- 64 * type,
- 64 * (type + 1),
- GFP_NOWAIT);
+ if (!skip_legacy_minors)
+ r = idr_alloc(&drm_minors_idr,
+ NULL,
+ 64 * type,
+ 64 * (type + 1),
+ GFP_NOWAIT);
if (r == -ENOSPC)
r = idr_alloc(&drm_minors_idr, NULL, 192, 1 << MINORBITS,
GFP_NOWAIT);
spin_unlock_irqrestore(&drm_minor_lock, flags);
--
2.37.3
[PATCH v3 3/4] drm: Use mutex for minors
Operating on drm minor is not done in IRQ context, which means that we
could safely downgrade to regular non-irq spinlock.
But we can also go further and drop the idr_preload tricks by just using
a mutex.
Signed-off-by: Michał Winiarski
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/drm_drv.c | 33 +
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 0bdcca0db611..f66904527256 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -53,7 +53,7 @@ MODULE_AUTHOR("Gareth Hughes, Leif Delgass, José Fonseca, Jon
Smirl");
MODULE_DESCRIPTION("DRM shared core routines");
MODULE_LICENSE("GPL and additional rights");
-static DEFINE_SPINLOCK(drm_minor_lock);
+static DEFINE_MUTEX(drm_minor_lock);
static struct idr drm_minors_idr;
static bool skip_legacy_minors;
@@ -103,21 +103,19 @@ static struct drm_minor **drm_minor_get_slot(struct
drm_device *dev,
static void drm_minor_alloc_release(struct drm_device *dev, void *data)
{
struct drm_minor *minor = data;
- unsigned long flags;
WARN_ON(dev != minor->dev);
put_device(minor->kdev);
- spin_lock_irqsave(&drm_minor_lock, flags);
+ mutex_lock(&drm_minor_lock);
idr_remove(&drm_minors_idr, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ mutex_unlock(&drm_minor_lock);
}
static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
- unsigned long flags;
int r = -ENOSPC;
minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL);
@@ -133,18 +131,16 @@ static int drm_minor_alloc(struct drm_device *dev,
unsigned int type)
* and 128-191 are render nodes.
* After reaching the limit, we're allocating minors dynamically -
first-come, first-serve.
*/
- idr_preload(GFP_KERNEL);
- spin_lock_irqsave(&drm_minor_lock, flags);
+ mutex_lock(&drm_minor_lock);
if (!skip_legacy_minors)
r = idr_alloc(&drm_minors_idr,
NULL,
64 * type,
64 * (type + 1),
- GFP_NOWAIT);
+ GFP_KERNEL);
if (r == -ENOSPC)
- r = idr_alloc(&drm_minors_idr, NULL, 192, 1 << MINORBITS,
GFP_NOWAIT);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
- idr_preload_end();
+ r = idr_alloc(&drm_minors_idr, NULL, 192, 1 << MINORBITS,
GFP_KERNEL);
+ mutex_unlock(&drm_minor_lock);
if (r < 0)
return r;
@@ -166,7 +162,6 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned
int type)
static int drm_minor_register(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
- unsigned long flags;
int ret;
DRM_DEBUG("\n");
@@ -186,9 +181,9 @@ static int drm_minor_register(struct drm_device *dev,
unsigned int type)
goto err_debugfs;
/* replace NULL with @minor so lookups will succeed from now on */
- spin_lock_irqsave(&drm_minor_lock, flags);
+ mutex_lock(&drm_minor_lock);
idr_replace(&drm_minors_idr, minor, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ mutex_unlock(&drm_minor_lock);
DRM_DEBUG("new minor registered %d\n", minor->index);
return 0;
@@ -201,16 +196,15 @@ static int drm_minor_register(struct drm_device *dev,
unsigned int type)
static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
- unsigned long flags;
minor = *drm_minor_get_slot(dev, type);
if (!minor || !device_is_registered(minor->kdev))
return;
/* replace @minor with NULL so lookups will fail from now on */
- spin_lock_irqsave(&drm_minor_lock, flags);
+ mutex_lock(&drm_minor_lock);
idr_replace(&drm_minors_idr, NULL, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ mutex_unlock(&drm_minor_lock);
device_del(minor->kdev);
dev_set_drvdata(minor->kdev, NULL); /* safety belt */
@@ -229,13 +223,12 @@ static void drm_minor_unregister(struct drm_device *dev,
unsigned int type)
struct drm_minor *drm_minor_acquire(unsigned int minor_id)
{
struct drm_minor *minor;
- unsigned long flags;
- spin_lock_irqsave(&drm_minor_lock, flags);
+ mutex_lock(&drm_minor_lock);
minor = idr_find(&drm_minors_idr, minor_id);
if (minor)
drm_dev_get(minor->dev);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ mutex_unlock(&drm_minor_lock);
if (!minor) {
return ERR_PTR(-ENODEV);
--
2.37.3
[PATCH v3 4/4] idr: Add might_alloc() annotation
Using might_alloc() lets us catch problems in a deterministic manner, even if we end up not allocating anything. Signed-off-by: Michał Winiarski --- lib/radix-tree.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/radix-tree.c b/lib/radix-tree.c index 3c78e1e8b2ad..787ab01001de 100644 --- a/lib/radix-tree.c +++ b/lib/radix-tree.c @@ -23,6 +23,7 @@ #include /* in_interrupt() */ #include #include +#include #include #include #include @@ -1481,6 +1482,8 @@ void __rcu **idr_get_free(struct radix_tree_root *root, unsigned long maxindex, start = iter->next_index; unsigned int shift, offset = 0; + might_alloc(gfp); + grow: shift = radix_tree_load_root(root, &child, &maxindex); if (!radix_tree_tagged(root, IDR_FREE)) -- 2.37.3
Re: [PATCH v3 0/4] drm: Use full allocated minor range for DRM
On Tue, Sep 06, 2022 at 04:01:13PM +0200, Michał Winiarski wrote: > 64 DRM device nodes is not enough for everyone. > Upgrade it to ~512K (which definitely is more than enough). > > To allow testing userspace support for >64 devices, add additional DRM > modparam (skip_legacy_minors) which causes DRM to skip allocating minors > in 0-192 range. > Additionally - one minor tweak around minor DRM IDR locking and IDR lockdep > annotations. The IDR is deprecated; rather than making all these changes around the IDR, could you convert it to use the XArray instead? I did it once before, but those patches bounced off the submissions process.
Re: [PATCH 2/6] dt-bindings: reserved-memory: Support framebuffer reserved memory
On Mon, Sep 05, 2022 at 06:32:56PM +0200, Thierry Reding wrote:
> From: Thierry Reding
>
> Document the "framebuffer" compatible string for reserved memory nodes
> to annotate reserved memory regions used for framebuffer carveouts.
>
> Signed-off-by: Thierry Reding
> ---
> .../bindings/reserved-memory/framebuffer.yaml | 46 +++
> 1 file changed, 46 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
> b/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
> new file mode 100644
> index ..80574854025d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/framebuffer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: /reserved-memory framebuffer node bindings
> +
> +maintainers:
> + - devicetree-s...@vger.kernel.org
> +
> +allOf:
> + - $ref: "reserved-memory.yaml"
Don't need quotes.
> +
> +properties:
> + compatible:
> +const: framebuffer
> +description: >
> + This indicates a region of memory meant to be used as a framebuffer for
> + a set of display devices. It can be used by an operating system to keep
> + the framebuffer from being overwritten and use it as the backing memory
> + for a display device (such as simple-framebuffer).
I'm on the fence whether we need this. It doesn't really add anything
because 'simple-framebuffer' will reference this node and you can find
it that way. I guess a bootloader may not setup 'simple-framebuffer',
but then it should probably not have this node either.
On the flip side, better to have compatibles than not to identify nodes.
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
Use '/ {' to skip the boilerplate causing the error.
> + chosen {
> +framebuffer {
> + compatible = "simple-framebuffer";
> + memory-region = <&fb>;
> +};
> + };
> +
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + fb: framebuffer@8000 {
> + compatible = "framebuffer";
> + reg = <0x8000 0x007e9000>;
> + };
> + };
> +
> +...
> --
> 2.37.2
>
>
[PATCH] drm/bochs: fix blanking
VGA_IS1_RC is the color mode register (VGA_IS1_RM the one for monochrome
mode, note C vs. M at the end). So when using VGA_IS1_RC make sure the
vga device is actually in color mode and set the corresponding bit in the
misc register.
Reproducible when booting VMs in UEFI mode with some edk2 versions (edk2
fix is on the way too). Doesn't happen in BIOS mode because in that
case the vgabios already flips the bit.
Fixes: 250e743915d4 ("drm/bochs: Add screen blanking support")
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/tiny/bochs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 08de13774862..a51262289aef 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -309,6 +309,8 @@ static void bochs_hw_fini(struct drm_device *dev)
static void bochs_hw_blank(struct bochs_device *bochs, bool blank)
{
DRM_DEBUG_DRIVER("hw_blank %d\n", blank);
+ /* enable color bit (so VGA_IS1_RC access works) */
+ bochs_vga_writeb(bochs, VGA_MIS_W, VGA_MIS_COLOR);
/* discard ar_flip_flop */
(void)bochs_vga_readb(bochs, VGA_IS1_RC);
/* blank or unblank; we need only update index and set 0x20 */
--
2.37.3
Re: [PATCH 1/4] drm: Add missing DP DSC extended capability definitions.
On Mon, 05 Sep 2022, Stanislav Lisovskiy wrote: > Adding DP DSC register definitions, we might need for further > DSC implementation, supporting MST and DP branch pass-through mode. > > v2: - Fixed checkpatch comment warning > v3: - Removed function which is not yet used(Jani Nikula) > > Reviewed-by: Vinod Govindapillai > > Signed-off-by: Stanislav Lisovskiy Maarten, Maxime, Thomas - So this got pushed to drm-intel-next without your acks. Apologies. Can we live with it, or want a revert? BR, Jani. > --- > include/drm/display/drm_dp.h | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 6c0871164771..02c4b6f20478 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -239,6 +239,9 @@ > > #define DP_DSC_SUPPORT 0x060 /* DP 1.4 */ > # define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0) > +# define DP_DSC_PASS_THROUGH_IS_SUPPORTED (1 << 1) > +# define DP_DSC_DYNAMIC_PPS_UPDATE_SUPPORT_COMP_TO_COMP(1 << 2) > +# define DP_DSC_DYNAMIC_PPS_UPDATE_SUPPORT_UNCOMP_TO_COMP (1 << 3) > > #define DP_DSC_REV 0x061 > # define DP_DSC_MAJOR_MASK (0xf << 0) > @@ -277,12 +280,15 @@ > > #define DP_DSC_BLK_PREDICTION_SUPPORT 0x066 > # define DP_DSC_BLK_PREDICTION_IS_SUPPORTED (1 << 0) > +# define DP_DSC_RGB_COLOR_CONV_BYPASS_SUPPORT (1 << 1) > > #define DP_DSC_MAX_BITS_PER_PIXEL_LOW 0x067 /* eDP 1.4 */ > > #define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068 /* eDP 1.4 */ > # define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK (0x3 << 0) > # define DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8 > +# define DP_DSC_MAX_BPP_DELTA_VERSION_MASK 0x06 > +# define DP_DSC_MAX_BPP_DELTA_AVAILABILITY 0x08 > > #define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069 > # define DP_DSC_RGB (1 << 0) > @@ -344,11 +350,13 @@ > # define DP_DSC_24_PER_DP_DSC_SINK (1 << 2) > > #define DP_DSC_BITS_PER_PIXEL_INC 0x06F > +# define DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK 0x1f > +# define DP_DSC_RGB_YCbCr420_MAX_BPP_DELTA_MASK 0xe0 > # define DP_DSC_BITS_PER_PIXEL_1_16 0x0 > # define DP_DSC_BITS_PER_PIXEL_1_8 0x1 > # define DP_DSC_BITS_PER_PIXEL_1_4 0x2 > # define DP_DSC_BITS_PER_PIXEL_1_2 0x3 > -# define DP_DSC_BITS_PER_PIXEL_10x4 > +# define DP_DSC_BITS_PER_PIXEL_1_1 0x4 > > #define DP_PSR_SUPPORT 0x070 /* XXX 1.2? */ > # define DP_PSR_IS_SUPPORTED1 -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 1/4] drm: Add missing DP DSC extended capability definitions.
On Tue, 06 Sep 2022, Jani Nikula wrote: > On Mon, 05 Sep 2022, Stanislav Lisovskiy > wrote: >> Adding DP DSC register definitions, we might need for further >> DSC implementation, supporting MST and DP branch pass-through mode. >> >> v2: - Fixed checkpatch comment warning >> v3: - Removed function which is not yet used(Jani Nikula) >> >> Reviewed-by: Vinod Govindapillai >> >> Signed-off-by: Stanislav Lisovskiy > > Maarten, Maxime, Thomas - > > So this got pushed to drm-intel-next without your acks. Apologies. Can > we live with it, or want a revert? I think dim should've warned about missing acks, did it not? :( BR, Jani. > > > BR, > Jani. > > >> --- >> include/drm/display/drm_dp.h | 10 +- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h >> index 6c0871164771..02c4b6f20478 100644 >> --- a/include/drm/display/drm_dp.h >> +++ b/include/drm/display/drm_dp.h >> @@ -239,6 +239,9 @@ >> >> #define DP_DSC_SUPPORT 0x060 /* DP 1.4 */ >> # define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0) >> +# define DP_DSC_PASS_THROUGH_IS_SUPPORTED (1 << 1) >> +# define DP_DSC_DYNAMIC_PPS_UPDATE_SUPPORT_COMP_TO_COMP(1 << 2) >> +# define DP_DSC_DYNAMIC_PPS_UPDATE_SUPPORT_UNCOMP_TO_COMP (1 << 3) >> >> #define DP_DSC_REV 0x061 >> # define DP_DSC_MAJOR_MASK (0xf << 0) >> @@ -277,12 +280,15 @@ >> >> #define DP_DSC_BLK_PREDICTION_SUPPORT 0x066 >> # define DP_DSC_BLK_PREDICTION_IS_SUPPORTED (1 << 0) >> +# define DP_DSC_RGB_COLOR_CONV_BYPASS_SUPPORT (1 << 1) >> >> #define DP_DSC_MAX_BITS_PER_PIXEL_LOW 0x067 /* eDP 1.4 */ >> >> #define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068 /* eDP 1.4 */ >> # define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK (0x3 << 0) >> # define DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8 >> +# define DP_DSC_MAX_BPP_DELTA_VERSION_MASK 0x06 >> +# define DP_DSC_MAX_BPP_DELTA_AVAILABILITY 0x08 >> >> #define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069 >> # define DP_DSC_RGB (1 << 0) >> @@ -344,11 +350,13 @@ >> # define DP_DSC_24_PER_DP_DSC_SINK (1 << 2) >> >> #define DP_DSC_BITS_PER_PIXEL_INC 0x06F >> +# define DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK 0x1f >> +# define DP_DSC_RGB_YCbCr420_MAX_BPP_DELTA_MASK 0xe0 >> # define DP_DSC_BITS_PER_PIXEL_1_16 0x0 >> # define DP_DSC_BITS_PER_PIXEL_1_8 0x1 >> # define DP_DSC_BITS_PER_PIXEL_1_4 0x2 >> # define DP_DSC_BITS_PER_PIXEL_1_2 0x3 >> -# define DP_DSC_BITS_PER_PIXEL_10x4 >> +# define DP_DSC_BITS_PER_PIXEL_1_1 0x4 >> >> #define DP_PSR_SUPPORT 0x070 /* XXX 1.2? */ >> # define DP_PSR_IS_SUPPORTED1 -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()
On 2022-09-02 09:10, Greg Kroah-Hartman wrote: On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote: When calling debugfs_lookup() the result must have dput() called on it, otherwise the memory will leak over time. Fix this up by properly calling dput(). Cc: Harry Wentland Cc: Leo Li Cc: Rodrigo Siqueira Cc: Alex Deucher Cc: "Christian König" Cc: "Pan, Xinhui" Cc: David Airlie Cc: Daniel Vetter Cc: Wayne Lin Cc: hersen wu Cc: Wenjing Liu Cc: Patrik Jakobsson Cc: Thelford Williams Cc: Fangzhi Zuo Cc: Yongzhi Liu Cc: Mikita Lipski Cc: Jiapeng Chong Cc: Bhanuprakash Modem Cc: Sean Paul Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Greg Kroah-Hartman --- Despite a zillion cc: items, I forgot to cc: stable on this. Can the maintainer add that here, or do you all want me to resend it with that item added? thanks, greg k-h Hi Greg, Reviewed-by: Rodrigo Siqueira Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change before I merge it into amd-staging-drm-next. Thanks Siqueira
Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()
On Tue, Sep 06, 2022 at 10:52:28AM -0400, Rodrigo Siqueira Jordao wrote: > > > On 2022-09-02 09:10, Greg Kroah-Hartman wrote: > > On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote: > > > When calling debugfs_lookup() the result must have dput() called on it, > > > otherwise the memory will leak over time. Fix this up by properly > > > calling dput(). > > > > > > Cc: Harry Wentland > > > Cc: Leo Li > > > Cc: Rodrigo Siqueira > > > Cc: Alex Deucher > > > Cc: "Christian König" > > > Cc: "Pan, Xinhui" > > > Cc: David Airlie > > > Cc: Daniel Vetter > > > Cc: Wayne Lin > > > Cc: hersen wu > > > Cc: Wenjing Liu > > > Cc: Patrik Jakobsson > > > Cc: Thelford Williams > > > Cc: Fangzhi Zuo > > > Cc: Yongzhi Liu > > > Cc: Mikita Lipski > > > Cc: Jiapeng Chong > > > Cc: Bhanuprakash Modem > > > Cc: Sean Paul > > > Cc: amd-...@lists.freedesktop.org > > > Cc: dri-devel@lists.freedesktop.org > > > Signed-off-by: Greg Kroah-Hartman > > > --- > > > > Despite a zillion cc: items, I forgot to cc: stable on this. Can the > > maintainer add that here, or do you all want me to resend it with that > > item added? > > > > thanks, > > > > greg k-h > > Hi Greg, > > Reviewed-by: Rodrigo Siqueira > > Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change before I > merge it into amd-staging-drm-next. Yes, please add the cc: stable@ line to the body of the patch, sorry I forgot that. thanks, greg k-h
Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_atpx_handler.c
On Mon, Sep 5, 2022 at 2:29 AM Jingyu Wang wrote:
>
> Fix everything checkpatch.pl complained about in amdgpu_atpx_handler.c
>
> Signed-off-by: Jingyu Wang
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 27 +++
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> index d6d986be906a..911d6a130ec5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> @@ -74,24 +74,29 @@ struct atpx_mux {
> u16 mux;
> } __packed;
>
> -bool amdgpu_has_atpx(void) {
> +bool amdgpu_has_atpx(void)
> +{
> return amdgpu_atpx_priv.atpx_detected;
> }
>
> -bool amdgpu_has_atpx_dgpu_power_cntl(void) {
> +bool amdgpu_has_atpx_dgpu_power_cntl(void)
> +{
> return amdgpu_atpx_priv.atpx.functions.power_cntl;
> }
>
> -bool amdgpu_is_atpx_hybrid(void) {
> +bool amdgpu_is_atpx_hybrid(void)
> +{
> return amdgpu_atpx_priv.atpx.is_hybrid;
> }
>
> -bool amdgpu_atpx_dgpu_req_power_for_displays(void) {
> +bool amdgpu_atpx_dgpu_req_power_for_displays(void)
> +{
> return amdgpu_atpx_priv.atpx.dgpu_req_power_for_displays;
> }
>
> #if defined(CONFIG_ACPI)
> -void *amdgpu_atpx_get_dhandle(void) {
> +void *amdgpu_atpx_get_dhandle(void)
> +{
> return amdgpu_atpx_priv.dhandle;
> }
> #endif
> @@ -134,7 +139,7 @@ static union acpi_object *amdgpu_atpx_call(acpi_handle
> handle, int function,
>
> /* Fail only if calling the method fails and ATPX is supported */
> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> - printk("failed to evaluate ATPX got %s\n",
> + DRM_WARN("failed to evaluate ATPX got %s\n",
I'm not sure these need to be warnings. Also, please use the
dev_info() functions instead so we can tell which device on the system
is reporting the issue.
>acpi_format_exception(status));
> kfree(buffer.pointer);
> return NULL;
> @@ -190,7 +195,7 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx *atpx)
>
> size = *(u16 *) info->buffer.pointer;
> if (size < 10) {
> - printk("ATPX buffer is too small: %zu\n", size);
> + DRM_WARN("ATPX buffer is too small: %zu\n", size);
> kfree(info);
> return -EINVAL;
> }
> @@ -223,11 +228,11 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx
> *atpx)
> atpx->is_hybrid = false;
> if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> if (amdgpu_atpx_priv.quirks & AMDGPU_PX_QUIRK_FORCE_ATPX) {
> - printk("ATPX Hybrid Graphics, forcing to ATPX\n");
> + DRM_WARN("ATPX Hybrid Graphics, forcing to ATPX\n");
> atpx->functions.power_cntl = true;
> atpx->is_hybrid = false;
> } else {
> - printk("ATPX Hybrid Graphics\n");
> + DRM_WARN("ATPX Hybrid Graphics\n");
These are definitely not warnings. Please use dev_info() here.
> /*
> * Disable legacy PM methods only when pcie port PM
> is usable,
> * otherwise the device might fail to power off or
> power on.
> @@ -269,7 +274,7 @@ static int amdgpu_atpx_verify_interface(struct
> amdgpu_atpx *atpx)
>
> size = *(u16 *) info->buffer.pointer;
> if (size < 8) {
> - printk("ATPX buffer is too small: %zu\n", size);
> + DRM_WARN("ATPX buffer is too small: %zu\n", size);
> err = -EINVAL;
> goto out;
> }
> @@ -278,7 +283,7 @@ static int amdgpu_atpx_verify_interface(struct
> amdgpu_atpx *atpx)
> memcpy(&output, info->buffer.pointer, size);
>
> /* TODO: check version? */
> - printk("ATPX version %u, functions 0x%08x\n",
> + DRM_WARN("ATPX version %u, functions 0x%08x\n",
Same here.
>output.version, output.function_bits);
>
> amdgpu_atpx_parse_functions(&atpx->functions, output.function_bits);
>
> base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
> --
> 2.34.1
>
Re: [PULL] drm-misc-next
On Tue, Sep 06, 2022 at 08:53:25AM +0200, Maarten Lankhorst wrote:
> Hi Dave, Daniel,
>
> A pull request prepared in Germany and Denmark, but sent from Sweden after
> fighting with gpg on an infamous bridge.
>
> My computer's somewhere in my trunk so I just copied someone else's pull
> request and pretend my laptop is a dev machine that sends pull requests every
> day works..
>
> Tag is still correctly signed, hope I didn't mess up anything!
[Dave should be back to handling pulls for everything this Fri]
Applied to drm-next, thanks.
-Daniel
>
> drm-misc-next-2022-08-20-1:
> drm-misc-next for v6.1:
>
> UAPI Changes:
>
> Cross-subsystem Changes:
> - DMA-buf: documentation updates.
> - Assorted small fixes to vga16fb
> - Fix fbdev drivers to use the aperture helpers.
> - Make removal of conflicting drivers work correctly without fbdev enabled.
>
> Core Changes:
> - bridge, scheduler, dp-mst: Assorted small fixes.
> - Add more format helpers to fourcc, and use it to replace the cpp usage.
> - Add DRM_FORMAT_Cxx, DRM_FORMAT_Rxx (single channel), and DRM_FORMAT_Dxx
> ("darkness", inverted single channel)
> - Add packed AYUV and XYUV formats.
> - Assorted documentation updates.
> - Rename ttm_bo_init to ttm_bo_init_validate.
> - Allow TTM bo's to exist without backing store.
> - Convert drm selftests to kunit.
> - Add managed init functions for (panel) bridge, crtc, encoder and connector.
> - Fix endianness handling in various format conversion helpers.
> - Make tests pass on big-endian platforms, and add test for rgb888 -> rgb565
> - Move DRM_PLANE_HELPER_NO_SCALING to atomic helpers and rename, so
> drm_plane_helper is no longer needed in most drivers.
> - Use idr_init_base instead of idr_init.
> - Rename FB and GEM CMA helpers to DMA helpers.
> - Rework XRGB related conversion helpers, and add drm_fb_blit() that
> takes a iosys_map. Make drm_fb_memcpy take an iosys_map too.
> - Move edid luminance calculation to core, and use it in i915.
>
> Driver Changes:
> - bridge/{adv7511,ti-sn65dsi86,parade-ps8640},
> panel/{simple,nt35510,tc358767},
> nouveau, sun4i, mipi-dsi, mgag200, bochs, arm, komeda, vmwgfx, pl111:
> Assorted small fixes and doc updates.
> - vc4: Rework hdmi power up, and depend on PM.
> - panel/simple: Add Samsung LTL101AL01.
> - ingenic: Add JZ4760(B) support, avoid a modeset when sharpness property
> is unchanged, and use the new PM ops.
> - Revert some amdgpu commits that cause garbaged graphics when starting
> X, and reapply them with the real problem fixed.
> - Completely rework vc4 init to use managed helpers.
> - Rename via_drv to via_dri1, and move all stuff there only used by the
> dri1 implementation in preperation for atomic modeset.
> - Use regmap bulk write in ssd130x.
> - Power sequence and clock updates to it6505.
> - Split panel-sitrox-st7701 init sequence and rework mode programming code.
> - virtio: Improve error and edge conditions handling, and convert to use
> managed
> helpers.
> - Add Samsung LTL101AL01, B120XAN01.0, R140NWF5 RH, DMT028VGHMCMI-1A T,
> panels.
> - Add generic fbdev support to komeda.
> - Split mgag200 modeset handling to make it more model-specific.
> - Convert simpledrm to use atomic helpers.
> - Improve udl suspend/disconnect handling.
> The following changes since commit 2bc7ea71a73747a77e7f83bc085b0d2393235410:
>
> Merge tag 'topic/nouveau-misc-2022-07-27' of
> git://anongit.freedesktop.org/drm/drm into drm-next (2022-07-27 11:34:07
> +1000)
>
> are available in the Git repository at:
>
> git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2022-08-20-1
>
> for you to fetch changes up to 8869fa666a9e6782c3c896c1fa57d65adca23249:
>
> drm/virtio: remove drm_plane_cleanup() destroy hook (2022-08-19 16:00:15
> +0200)
>
>
> drm-misc-next for v6.1:
>
> UAPI Changes:
>
> Cross-subsystem Changes:
> - DMA-buf: documentation updates.
> - Assorted small fixes to vga16fb
> - Fix fbdev drivers to use the aperture helpers.
> - Make removal of conflicting drivers work correctly without fbdev enabled.
>
> Core Changes:
> - bridge, scheduler, dp-mst: Assorted small fixes.
> - Add more format helpers to fourcc, and use it to replace the cpp usage.
> - Add DRM_FORMAT_Cxx, DRM_FORMAT_Rxx (single channel), and DRM_FORMAT_Dxx
> ("darkness", inverted single channel)
> - Add packed AYUV and XYUV formats.
> - Assorted documentation updates.
> - Rename ttm_bo_init to ttm_bo_init_validate.
> - Allow TTM bo's to exist without backing store.
> - Convert drm selftests to kunit.
> - Add managed init functions for (panel) bridge, crtc, encoder and connector.
> - Fix endianness handling in various format conversion helpers.
> - Make tests pass on big-endian platforms, and add test for rgb888 -> rgb565
> - Move DRM_PLANE_HELPER_NO_SCALING to atomic helpers and rename, so
> drm_plane_helper is no longer needed in most drivers.
> - Use idr_i
Re: [PATCH 1/8] memory: tegra: Add API for retrieving carveout bounds
Hi Mikko,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on clk/clk-next krzk-mem-ctrl/for-next pza/reset/next
linus/master v6.0-rc4 next-20220906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Mikko-Perttunen/Support-for-NVDEC-on-Tegra234/20220906-215151
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: powerpc-allyesconfig
(https://download.01.org/0day-ci/archive/20220906/202209062313.buowjwo0-...@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/7e2bd1173420c8e09ec90e3322e059a7350482e3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Mikko-Perttunen/Support-for-NVDEC-on-Tegra234/20220906-215151
git checkout 7e2bd1173420c8e09ec90e3322e059a7350482e3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/memory/tegra/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/memory/tegra/mc.c: In function 'tegra_mc_get_carveout_info':
>> drivers/memory/tegra/mc.c:124:83: warning: left shift count >= width of type
>> [-Wshift-count-overflow]
124 | *base |= (phys_addr_t)mc_ch_readl(mc, MC_BROADCAST_CHANNEL,
offset + 0x4) << 32;
|
^~
vim +124 drivers/memory/tegra/mc.c
109
110 int tegra_mc_get_carveout_info(struct tegra_mc *mc, unsigned int id,
111 phys_addr_t *base, u64 *size)
112 {
113 u32 offset;
114
115 if (id < 1 || id >= mc->soc->num_carveouts)
116 return -EINVAL;
117
118 if (id < 6)
119 offset = 0xc0c + 0x50 * (id - 1);
120 else
121 offset = 0x2004 + 0x50 * (id - 6);
122
123 *base = mc_ch_readl(mc, MC_BROADCAST_CHANNEL, offset + 0x0);
> 124 *base |= (phys_addr_t)mc_ch_readl(mc, MC_BROADCAST_CHANNEL,
> offset + 0x4) << 32;
125
126 if (size)
127 *size = mc_ch_readl(mc, MC_BROADCAST_CHANNEL, offset +
0x8) << 17;
128
129 return 0;
130 }
131 EXPORT_SYMBOL_GPL(tegra_mc_get_carveout_info);
132
--
0-DAY CI Kernel Test Service
https://01.org/lkp
[PATCH v4 0/5] Allwinner H6 GPU devfreq
Hi, This is a refresh of previous patches sent to enable GPU Devfreq on H6 Beelink GS1 but that wasn't stable at that time[0]. With the recent fix on GPU PLL from Roman Stratiienko I have retested and everything seems stable and works as expected[1]. Regards, Clement 0: https://lore.kernel.org/lkml/cajiucce58gaxf_qg2cnmwvoguqyu__ekb3mdx1fe_+47htg...@mail.gmail.com/ 1: https://lore.kernel.org/linux-arm-kernel/2562485.k3LOHGUjKi@kista/T/ Changes since v3: - Try to be more explicit for panfrost OPP patch - Fix typo Changes since v2: - Fixes device-tree warnings - Add panfrost fix to enable regulator - Remove always-on regulator from device-tree - Update cooling map from vendor kernel Clément Péron (5): arm64: defconfig: Enable devfreq cooling device arm64: dts: allwinner: h6: Add cooling map for GPU arm64: dts: allwinner: h6: Add GPU OPP table drm/panfrost: devfreq: set opp to the recommended one to configure regulator arm64: dts: allwinner: beelink-gs1: Enable GPU OPP .../dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 + .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 87 +++ arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 51 ++- arch/arm64/configs/defconfig | 1 + drivers/gpu/drm/panfrost/panfrost_devfreq.c | 11 +++ 5 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi -- 2.34.1
[PATCH v4 4/5] drm/panfrost: devfreq: set opp to the recommended one to configure regulator
Enabling panfrost GPU OPP with dynamic regulator will make OPP
responsible to enable and configure it.
Unfortunatly OPP configure and enable the regulator when an OPP
is asked to be set, which is not the case during
panfrost_devfreq_init().
This leave the regulator unconfigured and if no GPU load is
triggered, no OPP is asked to be set which make the regulator framework
switching it off during regulator_late_cleanup() without
noticing and therefore make the board hang as any access to GPU
memory space make bus locks up.
Call dev_pm_opp_set_opp() with the recommend OPP in
panfrost_devfreq_init() to enable the regulator, this will properly
configure and enable the regulator and will avoid any switch off
by regulator_late_cleanup().
Suggested-by: Viresh Kumar
Signed-off-by: Clément Péron
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 5110cd9b2425..fe5f12f16a63 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -131,6 +131,17 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
return PTR_ERR(opp);
panfrost_devfreq_profile.initial_freq = cur_freq;
+
+ /*
+* Set the recommend OPP this will enable and configure the regulator
+* if any and will avoid a switch off by regulator_late_cleanup()
+*/
+ ret = dev_pm_opp_set_opp(dev, opp);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n");
+ return ret;
+ }
+
dev_pm_opp_put(opp);
/*
--
2.34.1
[PATCH v4 3/5] arm64: dts: allwinner: h6: Add GPU OPP table
Add an Operating Performance Points table for the GPU to
enable Dynamic Voltage & Frequency Scaling on the H6.
The voltage range is set with minimal voltage set to the target
and the maximal voltage set to 1.2V. This allow DVFS framework to
work properly on board with fixed regulator.
Signed-off-by: Clément Péron
---
.../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 87 +++
1 file changed, 87 insertions(+)
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
new file mode 100644
index ..b48049c4fc85
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+// Copyright (C) 2022 Clément Péron
+
+/ {
+ gpu_opp_table: opp-table-gpu {
+ compatible = "operating-points-v2";
+
+ opp-21600 {
+ opp-hz = /bits/ 64 <21600>;
+ opp-microvolt = <81 81 120>;
+ };
+
+ opp-26400 {
+ opp-hz = /bits/ 64 <26400>;
+ opp-microvolt = <81 81 120>;
+ };
+
+ opp-31200 {
+ opp-hz = /bits/ 64 <31200>;
+ opp-microvolt = <81 81 120>;
+ };
+
+ opp-33600 {
+ opp-hz = /bits/ 64 <33600>;
+ opp-microvolt = <81 81 120>;
+ };
+
+ opp-36000 {
+ opp-hz = /bits/ 64 <36000>;
+ opp-microvolt = <82 82 120>;
+ };
+
+ opp-38400 {
+ opp-hz = /bits/ 64 <38400>;
+ opp-microvolt = <83 83 120>;
+ };
+
+ opp-40800 {
+ opp-hz = /bits/ 64 <40800>;
+ opp-microvolt = <84 84 120>;
+ };
+
+ opp-42000 {
+ opp-hz = /bits/ 64 <42000>;
+ opp-microvolt = <85 85 120>;
+ };
+
+ opp-43200 {
+ opp-hz = /bits/ 64 <43200>;
+ opp-microvolt = <86 86 120>;
+ };
+
+ opp-45600 {
+ opp-hz = /bits/ 64 <45600>;
+ opp-microvolt = <87 87 120>;
+ };
+
+ opp-50400 {
+ opp-hz = /bits/ 64 <50400>;
+ opp-microvolt = <89 89 120>;
+ };
+
+ opp-54000 {
+ opp-hz = /bits/ 64 <54000>;
+ opp-microvolt = <91 91 120>;
+ };
+
+ opp-57600 {
+ opp-hz = /bits/ 64 <57600>;
+ opp-microvolt = <93 93 120>;
+ };
+
+ opp-62400 {
+ opp-hz = /bits/ 64 <62400>;
+ opp-microvolt = <95 95 120>;
+ };
+
+ opp-75600 {
+ opp-hz = /bits/ 64 <75600>;
+ opp-microvolt = <104 104 120>;
+ };
+ };
+};
+
+&gpu {
+ operating-points-v2 = <&gpu_opp_table>;
+};
--
2.34.1
[PATCH v4 2/5] arm64: dts: allwinner: h6: Add cooling map for GPU
Add a simple cooling map for the GPU.
This cooling map come from the vendor kernel 4.9 with a
2°C hysteresis added.
Signed-off-by: Clément Péron
---
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 51 +++-
1 file changed, 49 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 5a28303d3d4c..53f6660656ac 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -186,6 +186,7 @@ gpu: gpu@180 {
clocks = <&ccu CLK_GPU>, <&ccu CLK_BUS_GPU>;
clock-names = "core", "bus";
resets = <&ccu RST_BUS_GPU>;
+ #cooling-cells = <2>;
status = "disabled";
};
@@ -1072,9 +1073,55 @@ map0 {
};
gpu-thermal {
- polling-delay-passive = <0>;
- polling-delay = <0>;
+ polling-delay-passive = <1000>;
+ polling-delay = <2000>;
thermal-sensors = <&ths 1>;
+
+ trips {
+ gpu_alert0: gpu-alert-0 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ gpu_alert1: gpu-alert-1 {
+ temperature = <10>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ gpu_alert2: gpu-alert-2 {
+ temperature = <105000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ gpu-crit {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ // Forbid the GPU to go over 756MHz
+ map0 {
+ trip = <&gpu_alert0>;
+ cooling-device = <&gpu 1
THERMAL_NO_LIMIT>;
+ };
+
+ // Forbid the GPU to go over 624MHz
+ map1 {
+ trip = <&gpu_alert1>;
+ cooling-device = <&gpu 2
THERMAL_NO_LIMIT>;
+ };
+
+ // Forbid the GPU to go over 576MHz
+ map2 {
+ trip = <&gpu_alert2>;
+ cooling-device = <&gpu 3
THERMAL_NO_LIMIT>;
+ };
+ };
};
};
};
--
2.34.1
[PATCH v4 1/5] arm64: defconfig: Enable devfreq cooling device
Devfreq cooling device framework is used in Panfrost to throttle GPU in order to regulate its temperature. Enable this driver for ARM64 SoC. Signed-off-by: Clément Péron Acked-by: Jernej Skrabec --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 91e58cf59c99..e557ccac8d9c 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -582,6 +582,7 @@ CONFIG_SENSORS_INA2XX=m CONFIG_SENSORS_INA3221=m CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y CONFIG_CPU_THERMAL=y +CONFIG_DEVFREQ_THERMAL=y CONFIG_THERMAL_EMULATION=y CONFIG_IMX_SC_THERMAL=m CONFIG_IMX8MM_THERMAL=m -- 2.34.1
[PATCH v4 5/5] arm64: dts: allwinner: beelink-gs1: Enable GPU OPP
Enable GPU OPP table for Beelink GS1. Signed-off-by: Clément Péron --- arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts index 6249e9e02928..9ec49ac2f6fd 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts @@ -5,6 +5,7 @@ #include "sun50i-h6.dtsi" #include "sun50i-h6-cpu-opp.dtsi" +#include "sun50i-h6-gpu-opp.dtsi" #include -- 2.34.1
Re: [PATCH] drm/amdgpu: recleanup coding style in amdgpu_fence.c
On Mon, Sep 5, 2022 at 3:40 AM Jingyu Wang wrote:
>
> Fix everything checkpatch.pl complained about in amdgpu_fence.c,
> modified use "} else {", sent it again, thx.
>
> Signed-off-by: Jingyu Wang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 8adeb7469f1e..0759d86d92da 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: MIT
> /*
> * Copyright 2009 Jerome Glisse.
> * All Rights Reserved.
> @@ -42,7 +43,6 @@
> #include "amdgpu_reset.h"
>
> /*
> - * Fences
What is the point of this change?
Alex
> * Fences mark an event in the GPUs pipeline and are used
> * for GPU/CPU synchronization. When the fence is written,
> * it is expected that all buffers associated with that fence
> @@ -139,7 +139,7 @@ static u32 amdgpu_fence_read(struct amdgpu_ring *ring)
> * Returns 0 on success, -ENOMEM on failure.
> */
> int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f, struct
> amdgpu_job *job,
> - unsigned flags)
> + unsigned int flags)
> {
> struct amdgpu_device *adev = ring->adev;
> struct dma_fence *fence;
> @@ -173,11 +173,11 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct
> dma_fence **f, struct amd
>adev->fence_context + ring->idx, seq);
> /* Against remove in amdgpu_job_{free, free_cb} */
> dma_fence_get(fence);
> - }
> - else
> + } else {
> dma_fence_init(fence, &amdgpu_fence_ops,
>&ring->fence_drv.lock,
>adev->fence_context + ring->idx, seq);
> + }
> }
>
> amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
> @@ -393,7 +393,7 @@ signed long amdgpu_fence_wait_polling(struct amdgpu_ring
> *ring,
> * Returns the number of emitted fences on the ring. Used by the
> * dynpm code to ring track activity.
> */
> -unsigned amdgpu_fence_count_emitted(struct amdgpu_ring *ring)
> +unsigned int amdgpu_fence_count_emitted(struct amdgpu_ring *ring)
> {
> uint64_t emitted;
>
> @@ -422,7 +422,7 @@ unsigned amdgpu_fence_count_emitted(struct amdgpu_ring
> *ring)
> */
> int amdgpu_fence_driver_start_ring(struct amdgpu_ring *ring,
>struct amdgpu_irq_src *irq_src,
> - unsigned irq_type)
> + unsigned int irq_type)
> {
> struct amdgpu_device *adev = ring->adev;
> uint64_t index;
> @@ -594,6 +594,7 @@ void amdgpu_fence_driver_hw_init(struct amdgpu_device
> *adev)
>
> for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
> struct amdgpu_ring *ring = adev->rings[i];
> +
> if (!ring || !ring->fence_drv.initialized)
> continue;
>
> @@ -772,6 +773,7 @@ static int amdgpu_debugfs_fence_info_show(struct seq_file
> *m, void *unused)
>
> for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> struct amdgpu_ring *ring = adev->rings[i];
> +
> if (!ring || !ring->fence_drv.initialized)
> continue;
>
> @@ -845,6 +847,7 @@ static void amdgpu_debugfs_reset_work(struct work_struct
> *work)
> reset_work);
>
> struct amdgpu_reset_context reset_context;
> +
> memset(&reset_context, 0, sizeof(reset_context));
>
> reset_context.method = AMD_RESET_METHOD_NONE;
>
> base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
> --
> 2.34.1
>
Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()
On 2022-09-06 11:06, Greg Kroah-Hartman wrote: On Tue, Sep 06, 2022 at 10:52:28AM -0400, Rodrigo Siqueira Jordao wrote: On 2022-09-02 09:10, Greg Kroah-Hartman wrote: On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote: When calling debugfs_lookup() the result must have dput() called on it, otherwise the memory will leak over time. Fix this up by properly calling dput(). Cc: Harry Wentland Cc: Leo Li Cc: Rodrigo Siqueira Cc: Alex Deucher Cc: "Christian König" Cc: "Pan, Xinhui" Cc: David Airlie Cc: Daniel Vetter Cc: Wayne Lin Cc: hersen wu Cc: Wenjing Liu Cc: Patrik Jakobsson Cc: Thelford Williams Cc: Fangzhi Zuo Cc: Yongzhi Liu Cc: Mikita Lipski Cc: Jiapeng Chong Cc: Bhanuprakash Modem Cc: Sean Paul Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Greg Kroah-Hartman --- Despite a zillion cc: items, I forgot to cc: stable on this. Can the maintainer add that here, or do you all want me to resend it with that item added? thanks, greg k-h Hi Greg, Reviewed-by: Rodrigo Siqueira Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change before I merge it into amd-staging-drm-next. Yes, please add the cc: stable@ line to the body of the patch, sorry I forgot that. Change applied to amd-staging-drm-next, with the Cc to the stable branch. Thanks Siqueira thanks, greg k-h
Re: [PATCH 0/4] Add DP MST DSC support to i915
On Mon, 05 Sep 2022, Stanislav Lisovskiy wrote: > Currently we have only DSC support for DP SST. As discussed elsewhere, the patches were modified while applying to resolve conflicts, leading to further conflicts in drm-tip rebuild, and eventually drm-tip build breakage. They would've created further conflicts in linux-next as well as drm-intel-next feature pull (or drm-next backmerge to drm-intel-next). I've gone ahead and reverted the commits from drm-intel-next directly, with Rodrigo's ack. The conflict resolution would have been messy and generated a bunch of extra work, and we needed to get drm-tip build back on track ASAP. We'll need to have a clean baseline to apply the patches on, i.e. drm-misc-next pull request to drm-next, and drm-next backmerge to drm-intel-next. BR, Jani. > > Stanislav Lisovskiy (4): > drm: Add missing DP DSC extended capability definitions. > drm/i915: Fix intel_dp_mst_compute_link_config > drm/i915: Extract drm_dp_atomic_find_vcpi_slots cycle to separate > function > drm/i915: Add DSC support to MST path > > drivers/gpu/drm/i915/display/intel_dp.c | 73 > drivers/gpu/drm/i915/display/intel_dp.h | 17 ++ > drivers/gpu/drm/i915/display/intel_dp_mst.c | 195 ++-- > include/drm/display/drm_dp.h| 10 +- > 4 files changed, 237 insertions(+), 58 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 1/4] drm: Add missing DP DSC extended capability definitions.
On Tue, 06 Sep 2022, Jani Nikula wrote: > On Mon, 05 Sep 2022, Stanislav Lisovskiy > wrote: >> Adding DP DSC register definitions, we might need for further >> DSC implementation, supporting MST and DP branch pass-through mode. >> >> v2: - Fixed checkpatch comment warning >> v3: - Removed function which is not yet used(Jani Nikula) >> >> Reviewed-by: Vinod Govindapillai >> >> Signed-off-by: Stanislav Lisovskiy > > Maarten, Maxime, Thomas - > > So this got pushed to drm-intel-next without your acks. Apologies. Can > we live with it, or want a revert? I've reverted anyway for other reasons. But can we have an ack for the future? :) BR, Jani. > > > BR, > Jani. > > >> --- >> include/drm/display/drm_dp.h | 10 +- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h >> index 6c0871164771..02c4b6f20478 100644 >> --- a/include/drm/display/drm_dp.h >> +++ b/include/drm/display/drm_dp.h >> @@ -239,6 +239,9 @@ >> >> #define DP_DSC_SUPPORT 0x060 /* DP 1.4 */ >> # define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0) >> +# define DP_DSC_PASS_THROUGH_IS_SUPPORTED (1 << 1) >> +# define DP_DSC_DYNAMIC_PPS_UPDATE_SUPPORT_COMP_TO_COMP(1 << 2) >> +# define DP_DSC_DYNAMIC_PPS_UPDATE_SUPPORT_UNCOMP_TO_COMP (1 << 3) >> >> #define DP_DSC_REV 0x061 >> # define DP_DSC_MAJOR_MASK (0xf << 0) >> @@ -277,12 +280,15 @@ >> >> #define DP_DSC_BLK_PREDICTION_SUPPORT 0x066 >> # define DP_DSC_BLK_PREDICTION_IS_SUPPORTED (1 << 0) >> +# define DP_DSC_RGB_COLOR_CONV_BYPASS_SUPPORT (1 << 1) >> >> #define DP_DSC_MAX_BITS_PER_PIXEL_LOW 0x067 /* eDP 1.4 */ >> >> #define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068 /* eDP 1.4 */ >> # define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK (0x3 << 0) >> # define DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8 >> +# define DP_DSC_MAX_BPP_DELTA_VERSION_MASK 0x06 >> +# define DP_DSC_MAX_BPP_DELTA_AVAILABILITY 0x08 >> >> #define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069 >> # define DP_DSC_RGB (1 << 0) >> @@ -344,11 +350,13 @@ >> # define DP_DSC_24_PER_DP_DSC_SINK (1 << 2) >> >> #define DP_DSC_BITS_PER_PIXEL_INC 0x06F >> +# define DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK 0x1f >> +# define DP_DSC_RGB_YCbCr420_MAX_BPP_DELTA_MASK 0xe0 >> # define DP_DSC_BITS_PER_PIXEL_1_16 0x0 >> # define DP_DSC_BITS_PER_PIXEL_1_8 0x1 >> # define DP_DSC_BITS_PER_PIXEL_1_4 0x2 >> # define DP_DSC_BITS_PER_PIXEL_1_2 0x3 >> -# define DP_DSC_BITS_PER_PIXEL_10x4 >> +# define DP_DSC_BITS_PER_PIXEL_1_1 0x4 >> >> #define DP_PSR_SUPPORT 0x070 /* XXX 1.2? */ >> # define DP_PSR_IS_SUPPORTED1 -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH linux-next] drm/amd/display: Remove the unneeded result variable
On 2022-09-02 03:54, cgel@gmail.com wrote:
From: zhang songyi
Return the enable_link_dp() directly instead of storing it in another
redundant variable.
Reported-by: Zeal Robot
Signed-off-by: zhang songyi
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index f9b798b7933c..4ab27e231337 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -2077,11 +2077,7 @@ static enum dc_status enable_link_edp(
struct dc_state *state,
struct pipe_ctx *pipe_ctx)
{
- enum dc_status status;
-
- status = enable_link_dp(state, pipe_ctx);
-
- return status;
+ return enable_link_dp(state, pipe_ctx);
}
static enum dc_status enable_link_dp_mst(
LGTM,
Reviewed-by: Rodrigo Siqueira
and applied to amd-staging-drm-next.
Thanks
Siqueira
Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()
On Tue, Sep 06, 2022 at 11:39:44AM -0400, Rodrigo Siqueira Jordao wrote: > > > On 2022-09-06 11:06, Greg Kroah-Hartman wrote: > > On Tue, Sep 06, 2022 at 10:52:28AM -0400, Rodrigo Siqueira Jordao wrote: > > > > > > > > > On 2022-09-02 09:10, Greg Kroah-Hartman wrote: > > > > On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote: > > > > > When calling debugfs_lookup() the result must have dput() called on > > > > > it, > > > > > otherwise the memory will leak over time. Fix this up by properly > > > > > calling dput(). > > > > > > > > > > Cc: Harry Wentland > > > > > Cc: Leo Li > > > > > Cc: Rodrigo Siqueira > > > > > Cc: Alex Deucher > > > > > Cc: "Christian König" > > > > > Cc: "Pan, Xinhui" > > > > > Cc: David Airlie > > > > > Cc: Daniel Vetter > > > > > Cc: Wayne Lin > > > > > Cc: hersen wu > > > > > Cc: Wenjing Liu > > > > > Cc: Patrik Jakobsson > > > > > Cc: Thelford Williams > > > > > Cc: Fangzhi Zuo > > > > > Cc: Yongzhi Liu > > > > > Cc: Mikita Lipski > > > > > Cc: Jiapeng Chong > > > > > Cc: Bhanuprakash Modem > > > > > Cc: Sean Paul > > > > > Cc: amd-...@lists.freedesktop.org > > > > > Cc: dri-devel@lists.freedesktop.org > > > > > Signed-off-by: Greg Kroah-Hartman > > > > > --- > > > > > > > > Despite a zillion cc: items, I forgot to cc: stable on this. Can the > > > > maintainer add that here, or do you all want me to resend it with that > > > > item added? > > > > > > > > thanks, > > > > > > > > greg k-h > > > > > > Hi Greg, > > > > > > Reviewed-by: Rodrigo Siqueira > > > > > > Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change > > > before I > > > merge it into amd-staging-drm-next. > > > > Yes, please add the cc: stable@ line to the body of the patch, sorry I > > forgot that. > > > > Change applied to amd-staging-drm-next, with the Cc to the stable branch. Wonderful, thanks!
build failure of next-20220906 for vkms
Hi All, The builds of next-20220906 fails for mips, xtensa and arm allmodconfig. The errors in mips and xtensa are: ERROR: modpost: "__divdi3" [drivers/gpu/drm/vkms/vkms.ko] undefined! ERROR: modpost: "__udivdi3" [drivers/gpu/drm/vkms/vkms.ko] undefined! The error in arm is: ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/vkms/vkms.ko] undefined! ERROR: modpost: "__aeabi_ldivmod" [drivers/gpu/drm/vkms/vkms.ko] undefined! Trying to do a git bisect to find out the offending commit. I will be happy to test any patch or provide any extra log if needed. -- Regards Sudip
Re: [PATCH v3 0/4] drm: Use full allocated minor range for DRM
On Tue, Sep 06, 2022 at 03:21:25PM +0100, Matthew Wilcox wrote: > On Tue, Sep 06, 2022 at 04:01:13PM +0200, Michał Winiarski wrote: > > 64 DRM device nodes is not enough for everyone. > > Upgrade it to ~512K (which definitely is more than enough). > > > > To allow testing userspace support for >64 devices, add additional DRM > > modparam (skip_legacy_minors) which causes DRM to skip allocating minors > > in 0-192 range. > > Additionally - one minor tweak around minor DRM IDR locking and IDR lockdep > > annotations. > > The IDR is deprecated; rather than making all these changes around > the IDR, could you convert it to use the XArray instead? I did it > once before, but those patches bounced off the submissions process. Sure. The IDR annotation can still be useful for existing users though, are you saying I should drop it as well? -Michał
Re: [Intel-gfx] [PATCH] drm/i915: Rename ggtt_view as gtt_view
On 05/09/2022 10:34, Tvrtko Ursulin wrote:
On 01/09/2022 19:38, Niranjana Vishwanathapura wrote:
So far, different views (normal, partial, rotated and remapped)
into the same object are only supported for GGTT mappings.
But with the upcoming VM_BIND feature, PPGTT will also use the
partial view mapping. Hence rename ggtt_view to more generic
gtt_view.
Signed-off-by: Niranjana Vishwanathapura
Acked-by: Tvrtko Ursulin
Easily even r-b since I did scroll through it and it all looks
straightforward.
For the record:
Reviewed-by: Tvrtko Ursulin
Regards,
Tvrtko
Regards,
Tvrtko
---
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
drivers/gpu/drm/i915/display/intel_display.h | 2 +-
.../drm/i915/display/intel_display_types.h | 2 +-
drivers/gpu/drm/i915/display/intel_fb.c | 18 ++---
drivers/gpu/drm/i915/display/intel_fb_pin.c | 4 +-
drivers/gpu/drm/i915/display/intel_fb_pin.h | 4 +-
drivers/gpu/drm/i915/display/intel_fbdev.c | 4 +-
drivers/gpu/drm/i915/gem/i915_gem_domain.c | 4 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 16 ++---
drivers/gpu/drm/i915/gem/i915_gem_object.h | 2 +-
.../drm/i915/gem/selftests/i915_gem_mman.c | 4 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 2 +-
drivers/gpu/drm/i915/i915_debugfs.c | 56 +++
drivers/gpu/drm/i915/i915_drv.h | 4 +-
drivers/gpu/drm/i915/i915_gem.c | 6 +-
drivers/gpu/drm/i915/i915_vma.c | 40 +--
drivers/gpu/drm/i915/i915_vma.h | 18 ++---
drivers/gpu/drm/i915/i915_vma_types.h | 42 ++--
drivers/gpu/drm/i915/selftests/i915_vma.c | 68 +--
19 files changed, 149 insertions(+), 149 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index be7cff722196..8251f87064f6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -670,7 +670,7 @@ bool intel_plane_uses_fence(const struct
intel_plane_state *plane_state)
return DISPLAY_VER(dev_priv) < 4 ||
(plane->fbc &&
- plane_state->view.gtt.type == I915_GGTT_VIEW_NORMAL);
+ plane_state->view.gtt.type == I915_GTT_VIEW_NORMAL);
}
/*
diff --git a/drivers/gpu/drm/i915/display/intel_display.h
b/drivers/gpu/drm/i915/display/intel_display.h
index e895277c4cd9..e322011877bb 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -45,7 +45,7 @@ struct drm_modeset_acquire_ctx;
struct drm_plane;
struct drm_plane_state;
struct i915_address_space;
-struct i915_ggtt_view;
+struct i915_gtt_view;
struct intel_atomic_state;
struct intel_crtc;
struct intel_crtc_state;
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 0da9b208d56e..01977cd237eb 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -105,7 +105,7 @@ struct intel_fb_view {
* In the normal view the FB object's backing store sg list is used
* directly and hence the remap information here is not used.
*/
- struct i915_ggtt_view gtt;
+ struct i915_gtt_view gtt;
/*
* The GTT view (gtt.type) specific information for each FB color
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
b/drivers/gpu/drm/i915/display/intel_fb.c
index b191915ab351..eefa33c555ac 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -1395,7 +1395,7 @@ static u32 calc_plane_remap_info(const struct
intel_framebuffer *fb, int color_p
plane_view_height_tiles(fb, color_plane, dims, y));
}
- if (view->gtt.type == I915_GGTT_VIEW_ROTATED) {
+ if (view->gtt.type == I915_GTT_VIEW_ROTATED) {
drm_WARN_ON(&i915->drm, remap_info->linear);
check_array_bounds(i915, view->gtt.rotated.plane, color_plane);
@@ -1420,7 +1420,7 @@ static u32 calc_plane_remap_info(const struct
intel_framebuffer *fb, int color_p
/* rotate the tile dimensions to match the GTT view */
swap(tile_width, tile_height);
} else {
- drm_WARN_ON(&i915->drm, view->gtt.type !=
I915_GGTT_VIEW_REMAPPED);
+ drm_WARN_ON(&i915->drm, view->gtt.type !=
I915_GTT_VIEW_REMAPPED);
check_array_bounds(i915, view->gtt.remapped.plane,
color_plane);
@@ -1503,12 +1503,12 @@ calc_plane_normal_size(const struct
intel_framebuffer *fb, int color_plane,
}
static void intel_fb_view_init(struct drm_i915_private *i915, struct
intel_fb_view *view,
- enum i915_ggtt_view_type view_type)
+ enum i915_gtt_view_type view_type)
{
memset(view, 0, sizeof(*view));
view->gtt.type = view_type;
- if (view_type == I915_GGTT_VIEW_REMAPPED && IS_ALDERLAKE_P(
[Bug 216455] New: PCI AER error caused by LTR enablement on amdgpu with LTR disabled on video card PCIe bridge
https://bugzilla.kernel.org/show_bug.cgi?id=216455 Bug ID: 216455 Summary: PCI AER error caused by LTR enablement on amdgpu with LTR disabled on video card PCIe bridge Product: Drivers Version: 2.5 Kernel Version: 5.19.6 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: wielkie...@gmail.com Regression: No Created attachment 301753 --> https://bugzilla.kernel.org/attachment.cgi?id=301753&action=edit dmesg with pci=earlydump (v5.19.5 + ltr debug patch) Split off bug 216373 as this is a different issue than what it was initially about. To quote Bjorn: > The issue you're seeing is an Unsupported Request error logged by a Switch > Downstream Port when it received an LTR message sent by 44:00.0 when the > Switch has LTR disabled: > > pcieport :43:00.0: device [1022:1471] error > status/mask=0010/ > pcieport :43:00.0:[20] UnsupReq (First) > pcieport :43:00.0: AER: TLP Header: 3400 4410 > 84288428 The errors themselves can be masked by providing pci=noaer to the kernel. The amdgpu.aspm=0 kernel option makes this issue disappear. -- 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 216455] PCI AER error caused by LTR enablement on amdgpu with LTR disabled on video card PCIe bridge
https://bugzilla.kernel.org/show_bug.cgi?id=216455 --- Comment #1 from Gustaw Smolarczyk (wielkie...@gmail.com) --- Created attachment 301754 --> https://bugzilla.kernel.org/attachment.cgi?id=301754&action=edit lspci -vvnn on vega10 system -- 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 216455] PCI AER error caused by LTR enablement on amdgpu with LTR disabled on video card PCIe bridge
https://bugzilla.kernel.org/show_bug.cgi?id=216455 --- Comment #2 from Gustaw Smolarczyk (wielkie...@gmail.com) --- Hardware: CPU: Ryzen Threadripper 1950X MB: Asrock X399 Taichi GPU: Radeon Vega 64 [1002:687f] -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v2 1/4] dma-buf: Add dma_buf_try_get()
On Thu, Sep 01, 2022 at 09:55:08AM +0200, Christian König wrote: > Am 01.09.22 um 01:12 schrieb Jason Gunthorpe: > > Used to increment the refcount of the dma buf's struct file, only if the > > refcount is not zero. Useful to allow the struct file's lifetime to > > control the lifetime of the dmabuf while still letting the driver to keep > > track of created dmabufs. > > > > Signed-off-by: Jason Gunthorpe > > --- > > include/linux/dma-buf.h | 13 + > > 1 file changed, 13 insertions(+) > > > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > > index 71731796c8c3a8..a35f1554f2fb36 100644 > > --- a/include/linux/dma-buf.h > > +++ b/include/linux/dma-buf.h > > @@ -618,6 +618,19 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags); > > struct dma_buf *dma_buf_get(int fd); > > void dma_buf_put(struct dma_buf *dmabuf); > > +/** > > + * dma_buf_try_get - try to get a reference on a dmabuf > > + * @dmabuf - the dmabuf to get > > + * > > + * Returns true if a reference was successfully obtained. The caller must > > + * interlock with the dmabuf's release function in some way, such as RCU, > > to > > + * ensure that this is not called on freed memory. > > I still have a bad feeling about this, but I also see that we can only > choose between evils here. > > Could you just call get_file_rcu() from the exporter with a comment > explaining why this works instead? I guess, are you sure? It seems very hacky. Jason
Re: [PATCH 1/4] drm: Add missing DP DSC extended capability definitions.
On Tue, Sep 06, 2022 at 06:43:22PM +0300, Jani Nikula wrote: > On Tue, 06 Sep 2022, Jani Nikula wrote: > > On Mon, 05 Sep 2022, Stanislav Lisovskiy > > wrote: > >> Adding DP DSC register definitions, we might need for further > >> DSC implementation, supporting MST and DP branch pass-through mode. > >> > >> v2: - Fixed checkpatch comment warning > >> v3: - Removed function which is not yet used(Jani Nikula) > >> > >> Reviewed-by: Vinod Govindapillai > >> > >> Signed-off-by: Stanislav Lisovskiy > > > > Maarten, Maxime, Thomas - > > > > So this got pushed to drm-intel-next without your acks. Apologies. Can > > we live with it, or want a revert? > > I've reverted anyway for other reasons. But can we have an ack for the > future? :) > > BR, > Jani. I've resolved the conflict properly now(not the best way of learning about drm-rerere) but I guess its too late now. Sorry for the hassle. But what am I supposed to do now? Should proceed with merge again or wait for some acks? Patches basically would be the same anyway. Stan > > > > > > > BR, > > Jani. > > > > > >> --- > >> include/drm/display/drm_dp.h | 10 +- > >> 1 file changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > >> index 6c0871164771..02c4b6f20478 100644 > >> --- a/include/drm/display/drm_dp.h > >> +++ b/include/drm/display/drm_dp.h > >> @@ -239,6 +239,9 @@ > >> > >> #define DP_DSC_SUPPORT 0x060 /* DP 1.4 */ > >> # define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0) > >> +# define DP_DSC_PASS_THROUGH_IS_SUPPORTED (1 << 1) > >> +# define DP_DSC_DYNAMIC_PPS_UPDATE_SUPPORT_COMP_TO_COMP(1 << 2) > >> +# define DP_DSC_DYNAMIC_PPS_UPDATE_SUPPORT_UNCOMP_TO_COMP (1 << 3) > >> > >> #define DP_DSC_REV 0x061 > >> # define DP_DSC_MAJOR_MASK (0xf << 0) > >> @@ -277,12 +280,15 @@ > >> > >> #define DP_DSC_BLK_PREDICTION_SUPPORT 0x066 > >> # define DP_DSC_BLK_PREDICTION_IS_SUPPORTED (1 << 0) > >> +# define DP_DSC_RGB_COLOR_CONV_BYPASS_SUPPORT (1 << 1) > >> > >> #define DP_DSC_MAX_BITS_PER_PIXEL_LOW 0x067 /* eDP 1.4 */ > >> > >> #define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068 /* eDP 1.4 */ > >> # define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK (0x3 << 0) > >> # define DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8 > >> +# define DP_DSC_MAX_BPP_DELTA_VERSION_MASK 0x06 > >> +# define DP_DSC_MAX_BPP_DELTA_AVAILABILITY 0x08 > >> > >> #define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069 > >> # define DP_DSC_RGB (1 << 0) > >> @@ -344,11 +350,13 @@ > >> # define DP_DSC_24_PER_DP_DSC_SINK (1 << 2) > >> > >> #define DP_DSC_BITS_PER_PIXEL_INC 0x06F > >> +# define DP_DSC_RGB_YCbCr444_MAX_BPP_DELTA_MASK 0x1f > >> +# define DP_DSC_RGB_YCbCr420_MAX_BPP_DELTA_MASK 0xe0 > >> # define DP_DSC_BITS_PER_PIXEL_1_16 0x0 > >> # define DP_DSC_BITS_PER_PIXEL_1_8 0x1 > >> # define DP_DSC_BITS_PER_PIXEL_1_4 0x2 > >> # define DP_DSC_BITS_PER_PIXEL_1_2 0x3 > >> -# define DP_DSC_BITS_PER_PIXEL_10x4 > >> +# define DP_DSC_BITS_PER_PIXEL_1_1 0x4 > >> > >> #define DP_PSR_SUPPORT 0x070 /* XXX 1.2? */ > >> # define DP_PSR_IS_SUPPORTED1 > > -- > Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 1/3] drm/gma500: Fix BUG: sleeping function called from invalid context errors
Hi, On 9/6/22 14:50, Patrik Jakobsson wrote: > On Mon, Sep 5, 2022 at 3:37 PM Hans de Goede wrote: >> >> gma_crtc_page_flip() was holding the event_lock spinlock while calling >> crtc_funcs->mode_set_base() which takes ww_mutex. >> >> The only reason to hold event_lock is to clear gma_crtc->page_flip_event >> on mode_set_base() errors. >> >> Instead unlock it after setting gma_crtc->page_flip_event and on >> errors re-take the lock and clear gma_crtc->page_flip_event it >> it is still set. > > Hi Hans, thanks for having a look at gma500. > > See comments below. > >> >> This fixes the following WARN/stacktrace: >> >> [ 512.122953] BUG: sleeping function called from invalid context at >> kernel/locking/mutex.c:870 >> [ 512.123004] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 1253, >> name: gnome-shell >> [ 512.123031] preempt_count: 1, expected: 0 >> [ 512.123048] RCU nest depth: 0, expected: 0 >> [ 512.123066] INFO: lockdep is turned off. >> [ 512.123080] irq event stamp: 0 >> [ 512.123094] hardirqs last enabled at (0): [<>] 0x0 >> [ 512.123134] hardirqs last disabled at (0): [] >> copy_process+0x9fc/0x1de0 >> [ 512.123176] softirqs last enabled at (0): [] >> copy_process+0x9fc/0x1de0 >> [ 512.123207] softirqs last disabled at (0): [<>] 0x0 >> [ 512.123233] Preemption disabled at: >> [ 512.123241] [<>] 0x0 >> [ 512.123275] CPU: 3 PID: 1253 Comm: gnome-shell Tainted: GW >> 5.19.0+ #1 >> [ 512.123304] Hardware name: Packard Bell dot s/SJE01_CT, BIOS V1.10 >> 07/23/2013 >> [ 512.123323] Call Trace: >> [ 512.123346] >> [ 512.123370] dump_stack_lvl+0x5b/0x77 >> [ 512.123412] __might_resched.cold+0xff/0x13a >> [ 512.123458] ww_mutex_lock+0x1e/0xa0 >> [ 512.123495] psb_gem_pin+0x2c/0x150 [gma500_gfx] >> [ 512.123601] gma_pipe_set_base+0x76/0x240 [gma500_gfx] >> [ 512.123708] gma_crtc_page_flip+0x95/0x130 [gma500_gfx] >> [ 512.123808] drm_mode_page_flip_ioctl+0x57d/0x5d0 >> [ 512.123897] ? drm_mode_cursor2_ioctl+0x10/0x10 >> [ 512.123936] drm_ioctl_kernel+0xa1/0x150 >> [ 512.123984] drm_ioctl+0x21f/0x420 >> [ 512.124025] ? drm_mode_cursor2_ioctl+0x10/0x10 >> [ 512.124070] ? rcu_read_lock_bh_held+0xb/0x60 >> [ 512.124104] ? lock_release+0x1ef/0x2d0 >> [ 512.124161] __x64_sys_ioctl+0x8d/0xd0 >> [ 512.124203] do_syscall_64+0x58/0x80 >> [ 512.124239] ? do_syscall_64+0x67/0x80 >> [ 512.124267] ? trace_hardirqs_on_prepare+0x55/0xe0 >> [ 512.124300] ? do_syscall_64+0x67/0x80 >> [ 512.124340] ? rcu_read_lock_sched_held+0x10/0x80 >> [ 512.124377] entry_SYSCALL_64_after_hwframe+0x63/0xcd >> [ 512.124411] RIP: 0033:0x7fcc4a70740f >> [ 512.124442] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 >> 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> >> c2 3d 00 f0 ff ff 77 18 48 8b 44 24 18 64 48 2b 04 25 28 00 00 >> [ 512.124470] RSP: 002b:7ffda73f5390 EFLAGS: 0246 ORIG_RAX: >> 0010 >> [ 512.124503] RAX: ffda RBX: 55cc9e474500 RCX: >> 7fcc4a70740f >> [ 512.124524] RDX: 7ffda73f5420 RSI: c01864b0 RDI: >> 0009 >> [ 512.124544] RBP: 7ffda73f5420 R08: 55cc9c0b0cb0 R09: >> 0034 >> [ 512.124564] R10: R11: 0246 R12: >> c01864b0 >> [ 512.124584] R13: 0009 R14: 55cc9df484d0 R15: >> 55cc9af5d0c0 >> [ 512.124647] >> >> Signed-off-by: Hans de Goede >> --- >> drivers/gpu/drm/gma500/gma_display.c | 10 +++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/gma500/gma_display.c >> b/drivers/gpu/drm/gma500/gma_display.c >> index bd40c040a2c9..cf038e322164 100644 >> --- a/drivers/gpu/drm/gma500/gma_display.c >> +++ b/drivers/gpu/drm/gma500/gma_display.c >> @@ -532,15 +532,19 @@ int gma_crtc_page_flip(struct drm_crtc *crtc, >> WARN_ON(drm_crtc_vblank_get(crtc) != 0); >> >> gma_crtc->page_flip_event = event; >> + spin_unlock_irqrestore(&dev->event_lock, flags); >> >> /* Call this locked if we want an event at vblank interrupt. >> */ > > If we don't hold the event_lock around mode_set_base() we could > potentially get a vblank before we do the modeset. That would send the > event prematurely. I think this is what the comment tries to tell us. There is no way to avoid the vblank-irq and the mode_set_base() call racing with each other and then delivering a blank event from what is actually the page-flip which just completed. Even with the old code, of we hold the lock over the mode_set_base() and then the vblank-irq triggers while we are holding the lock this will block the irq-handler until the mode_set_base() has completed (which is probably only a couple 100s of usecs / max 1 msec) and then as soon as gma_crtc_page_flip() releases the lock the irq-handler will continue running and still prematurely de
Re: [PATCH 2/3] drm/gma500: Fix crtc_vblank reference leak when userspace queues multiple events
Hi Michel, On 9/6/22 12:25, Michel Dänzer wrote: > On 2022-09-05 15:37, Hans de Goede wrote: >> The gma500 page-flip code kinda assume that userspace never queues more >> then 1 vblank event. So basically it assume that userspace does: >> >> - page-flip >> - wait for vblank event >> - render >> - page-flip >> - etc. >> >> In the case where userspace would submit 2 page-flips without waiting >> for the first to finish, the current code will just overwrite >> gma_crtc->page_flip_event with the event from the 2nd page-flip. > > This cannot happen. Common code returns -EBUSY for an attempt to submit a > page flip while another one is still pending. Ah I did not know that, that is very useful to know, thank you. I will drop this patch for the next version of this patch-set (which will include some further fixes). Regards, Hans
Re: [PATCH v2 0/3] Add generic framebuffer support to EFI earlycon driver
On Sat, 6 Aug 2022 at 18:34, Markuss Broks wrote: > > Make the EFI earlycon driver be suitable for any linear framebuffers. > This should be helpful for early porting of boards with no other means of > output, like smartphones/tablets. There seems to be an issue with > early_ioremap > function on ARM32, but I am unable to find the exact cause. It appears the > mappings > returned by it are somehow incorrect, thus the driver is disabled on ARM. EFI > early > console was disabled on IA64 previously because of missing > early_memremap_prot, > and this is inherited to this driver. > > This patch also changes behavior on EFI systems, by selecting the mapping type > based on if the framebuffer region intersects with system RAM. If it does, > it's > common sense that it should be in RAM as a whole, and so the system RAM > mapping is > used. It was tested to be working on my PC (Intel Z490 platform), as well as > several > ARM64 boards (Samsung Galaxy S9 (Exynos), iPad Air 2, Xiaomi Mi Pad 4, ...). > > Markuss Broks (2): > drivers: serial: earlycon: Pass device-tree node > efi: earlycon: Add support for generic framebuffers and move to fbdev > subsystem > > > v1 -> v2: > > - a new patch correcting serial/earlycon.c argument name to "offset" instead > of "node" > - move IA64 exclusion from EFI earlycon Kconfig to earlycon driver Kconfig > (IA64 has no early_memremap_prot) > - move driver from fbdev to console subsystem > - select EFI earlycon by default Wasn't EFI earlycon already enabled by default? > - fetch stride manually from device-tree, as on some devices it seems stride > doesn't match the horizontal resolution * bpp. > - use saner format (e.g. 1920x1080x32 instead of 1920,1080,32). > > .../admin-guide/kernel-parameters.txt | 12 +- > MAINTAINERS | 5 + > drivers/firmware/efi/Kconfig | 6 +- > drivers/firmware/efi/Makefile | 1 - > drivers/firmware/efi/earlycon.c | 246 -- > drivers/tty/serial/earlycon.c | 3 + > drivers/video/fbdev/Kconfig | 11 + > drivers/video/fbdev/Makefile | 1 + > drivers/video/fbdev/earlycon.c| 301 ++ > include/linux/serial_core.h | 1 + > 10 files changed, 331 insertions(+), 256 deletions(-) > delete mode 100644 drivers/firmware/efi/earlycon.c > create mode 100644 drivers/video/fbdev/earlycon.c > > -- > 2.37.0 >
Re: [PATCH v2 01/12] drm/i915: Move locking and unclaimed check into mmio_debug_{suspend, resume}
On Tue, Sep 06, 2022 at 06:39:14AM -0700, Ruhl, Michael J wrote:
> >-Original Message-
> >From: dri-devel On Behalf Of
> >Matt Roper
> >Sent: Friday, September 2, 2022 7:33 PM
> >To: intel-...@lists.freedesktop.org
> >Cc: dri-devel@lists.freedesktop.org; Sripada, Radhakrishna
> >
> >Subject: [PATCH v2 01/12] drm/i915: Move locking and unclaimed check into
> >mmio_debug_{suspend, resume}
> >
> >Moving the locking for MMIO debug (and the final check for unclaimed
> >accesses when resuming debug after a userspace-initiated forcewake) will
> >make it simpler to completely skip MMIO debug handling on uncores that
> >don't support it in future patches.
> >
> >Signed-off-by: Matt Roper
> >Reviewed-by: Radhakrishna Sripada
> >---
> > drivers/gpu/drm/i915/intel_uncore.c | 41 +++--
> > 1 file changed, 21 insertions(+), 20 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> >b/drivers/gpu/drm/i915/intel_uncore.c
> >index 9b81b2543ce2..e717ea55484a 100644
> >--- a/drivers/gpu/drm/i915/intel_uncore.c
> >+++ b/drivers/gpu/drm/i915/intel_uncore.c
> >@@ -50,23 +50,33 @@ intel_uncore_mmio_debug_init_early(struct
> >intel_uncore_mmio_debug *mmio_debug)
> > mmio_debug->unclaimed_mmio_check = 1;
> > }
> >
> >-static void mmio_debug_suspend(struct intel_uncore_mmio_debug
> >*mmio_debug)
> >+static void mmio_debug_suspend(struct intel_uncore *uncore)
>
> /bike-shedding...
>
> It seems like there has been a tend to name functions with the
>
> _unlocked
>
> postfix when the lock is being taken within the function.
>
> Would this be a reasonable name update for these changes?
I think foo_unlocked() naming is usually used when there's also a
separate foo() that can be called if/when locks are already held (or
sometimes it's foo() and foo_locked() if the situation is the other way
around). In this case we still only have one version of the function,
and it's only called from a single place in the code
(intel_uncore_forcewake_user_get) so I don't think the special naming is
necessary. It might actually add confusion here since there's a
different lock (the general uncore lock) that is still held by the
caller. It's just the mmio_debug-specific lock that's been moved into
the mmio-debug specific function here.
Matt
>
> M
>
> > {
> >-lockdep_assert_held(&mmio_debug->lock);
> >+spin_lock(&uncore->debug->lock);
> >
> > /* Save and disable mmio debugging for the user bypass */
> >-if (!mmio_debug->suspend_count++) {
> >-mmio_debug->saved_mmio_check = mmio_debug-
> >>unclaimed_mmio_check;
> >-mmio_debug->unclaimed_mmio_check = 0;
> >+if (!uncore->debug->suspend_count++) {
> >+uncore->debug->saved_mmio_check = uncore->debug-
> >>unclaimed_mmio_check;
> >+uncore->debug->unclaimed_mmio_check = 0;
> > }
> >+
> >+spin_unlock(&uncore->debug->lock);
> > }
> >
> >-static void mmio_debug_resume(struct intel_uncore_mmio_debug
> >*mmio_debug)
> >+static bool check_for_unclaimed_mmio(struct intel_uncore *uncore);
> >+
> >+static void mmio_debug_resume(struct intel_uncore *uncore)
> > {
> >-lockdep_assert_held(&mmio_debug->lock);
> >+spin_lock(&uncore->debug->lock);
> >+
> >+if (!--uncore->debug->suspend_count)
> >+uncore->debug->unclaimed_mmio_check = uncore->debug-
> >>saved_mmio_check;
> >
> >-if (!--mmio_debug->suspend_count)
> >-mmio_debug->unclaimed_mmio_check = mmio_debug-
> >>saved_mmio_check;
> >+if (check_for_unclaimed_mmio(uncore))
> >+drm_info(&uncore->i915->drm,
> >+ "Invalid mmio detected during user access\n");
> >+
> >+spin_unlock(&uncore->debug->lock);
> > }
> >
> > static const char * const forcewake_domain_names[] = {
> >@@ -677,9 +687,7 @@ void intel_uncore_forcewake_user_get(struct
> >intel_uncore *uncore)
> > spin_lock_irq(&uncore->lock);
> > if (!uncore->user_forcewake_count++) {
> > intel_uncore_forcewake_get__locked(uncore,
> >FORCEWAKE_ALL);
> >-spin_lock(&uncore->debug->lock);
> >-mmio_debug_suspend(uncore->debug);
> >-spin_unlock(&uncore->debug->lock);
> >+mmio_debug_suspend(uncore);
> > }
> > spin_unlock_irq(&uncore->lock);
> > }
> >@@ -695,14 +703,7 @@ void intel_uncore_forcewake_user_put(struct
> >intel_uncore *uncore)
> > {
> > spin_lock_irq(&uncore->lock);
> > if (!--uncore->user_forcewake_count) {
> >-spin_lock(&uncore->debug->lock);
> >-mmio_debug_resume(uncore->debug);
> >-
> >-if (check_for_unclaimed_mmio(uncore))
> >-drm_info(&uncore->i915->drm,
> >- "Invalid mmio detected during user
> >access\n");
> >-spin_unlock(&uncore->debug->lock);
> >-
> >+mmio_debug_resume(uncore);
> > intel_uncore_forcewake_put__locked(uncore,
> >FORCEWAKE_ALL);
> > }
> > spin_unlock_irq(&uncore->lock);
> >-
RE: [PATCH v2 01/12] drm/i915: Move locking and unclaimed check into mmio_debug_{suspend, resume}
>-Original Message-
>From: Roper, Matthew D
>Sent: Tuesday, September 6, 2022 1:09 PM
>To: Ruhl, Michael J
>Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Sripada,
>Radhakrishna
>Subject: Re: [PATCH v2 01/12] drm/i915: Move locking and unclaimed check
>into mmio_debug_{suspend, resume}
>
>On Tue, Sep 06, 2022 at 06:39:14AM -0700, Ruhl, Michael J wrote:
>> >-Original Message-
>> >From: dri-devel On Behalf Of
>> >Matt Roper
>> >Sent: Friday, September 2, 2022 7:33 PM
>> >To: intel-...@lists.freedesktop.org
>> >Cc: dri-devel@lists.freedesktop.org; Sripada, Radhakrishna
>> >
>> >Subject: [PATCH v2 01/12] drm/i915: Move locking and unclaimed check
>into
>> >mmio_debug_{suspend, resume}
>> >
>> >Moving the locking for MMIO debug (and the final check for unclaimed
>> >accesses when resuming debug after a userspace-initiated forcewake) will
>> >make it simpler to completely skip MMIO debug handling on uncores that
>> >don't support it in future patches.
>> >
>> >Signed-off-by: Matt Roper
>> >Reviewed-by: Radhakrishna Sripada
>> >---
>> > drivers/gpu/drm/i915/intel_uncore.c | 41 +++--
>> > 1 file changed, 21 insertions(+), 20 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> >b/drivers/gpu/drm/i915/intel_uncore.c
>> >index 9b81b2543ce2..e717ea55484a 100644
>> >--- a/drivers/gpu/drm/i915/intel_uncore.c
>> >+++ b/drivers/gpu/drm/i915/intel_uncore.c
>> >@@ -50,23 +50,33 @@ intel_uncore_mmio_debug_init_early(struct
>> >intel_uncore_mmio_debug *mmio_debug)
>> >mmio_debug->unclaimed_mmio_check = 1;
>> > }
>> >
>> >-static void mmio_debug_suspend(struct intel_uncore_mmio_debug
>> >*mmio_debug)
>> >+static void mmio_debug_suspend(struct intel_uncore *uncore)
>>
>> /bike-shedding...
>>
>> It seems like there has been a tend to name functions with the
>>
>> _unlocked
>>
>> postfix when the lock is being taken within the function.
>>
>> Would this be a reasonable name update for these changes?
>
>I think foo_unlocked() naming is usually used when there's also a
>separate foo() that can be called if/when locks are already held (or
>sometimes it's foo() and foo_locked() if the situation is the other way
>around). In this case we still only have one version of the function,
>and it's only called from a single place in the code
>(intel_uncore_forcewake_user_get) so I don't think the special naming is
>necessary. It might actually add confusion here since there's a
>different lock (the general uncore lock) that is still held by the
>caller. It's just the mmio_debug-specific lock that's been moved into
>the mmio-debug specific function here.
Got it. That makes sense.
Thanks,
Mike
>
>Matt
>
>>
>> M
>>
>> > {
>> >- lockdep_assert_held(&mmio_debug->lock);
>> >+ spin_lock(&uncore->debug->lock);
>> >
>> >/* Save and disable mmio debugging for the user bypass */
>> >- if (!mmio_debug->suspend_count++) {
>> >- mmio_debug->saved_mmio_check = mmio_debug-
>> >>unclaimed_mmio_check;
>> >- mmio_debug->unclaimed_mmio_check = 0;
>> >+ if (!uncore->debug->suspend_count++) {
>> >+ uncore->debug->saved_mmio_check = uncore->debug-
>> >>unclaimed_mmio_check;
>> >+ uncore->debug->unclaimed_mmio_check = 0;
>> >}
>> >+
>> >+ spin_unlock(&uncore->debug->lock);
>> > }
>> >
>> >-static void mmio_debug_resume(struct intel_uncore_mmio_debug
>> >*mmio_debug)
>> >+static bool check_for_unclaimed_mmio(struct intel_uncore *uncore);
>> >+
>> >+static void mmio_debug_resume(struct intel_uncore *uncore)
>> > {
>> >- lockdep_assert_held(&mmio_debug->lock);
>> >+ spin_lock(&uncore->debug->lock);
>> >+
>> >+ if (!--uncore->debug->suspend_count)
>> >+ uncore->debug->unclaimed_mmio_check = uncore->debug-
>> >>saved_mmio_check;
>> >
>> >- if (!--mmio_debug->suspend_count)
>> >- mmio_debug->unclaimed_mmio_check = mmio_debug-
>> >>saved_mmio_check;
>> >+ if (check_for_unclaimed_mmio(uncore))
>> >+ drm_info(&uncore->i915->drm,
>> >+"Invalid mmio detected during user access\n");
>> >+
>> >+ spin_unlock(&uncore->debug->lock);
>> > }
>> >
>> > static const char * const forcewake_domain_names[] = {
>> >@@ -677,9 +687,7 @@ void intel_uncore_forcewake_user_get(struct
>> >intel_uncore *uncore)
>> >spin_lock_irq(&uncore->lock);
>> >if (!uncore->user_forcewake_count++) {
>> >intel_uncore_forcewake_get__locked(uncore,
>> >FORCEWAKE_ALL);
>> >- spin_lock(&uncore->debug->lock);
>> >- mmio_debug_suspend(uncore->debug);
>> >- spin_unlock(&uncore->debug->lock);
>> >+ mmio_debug_suspend(uncore);
>> >}
>> >spin_unlock_irq(&uncore->lock);
>> > }
>> >@@ -695,14 +703,7 @@ void intel_uncore_forcewake_user_put(struct
>> >intel_uncore *uncore)
>> > {
>> >spin_lock_irq(&uncore->lock);
>> >if (!--uncore->user_forcewake_count) {
>> >- spin_lock(&uncore->debug->lock);
>> >- mmio_debug_
Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_kms.c
Hi Jingyu, Thank you for the patch! Yet something to improve: [auto build test ERROR on e47eb90a0a9ae20b82635b9b99a8d0979b757ad8] url: https://github.com/intel-lab-lkp/linux/commits/Jingyu-Wang/drm-amdgpu-cleanup-coding-style-in-amdgpu_kms-c/20220906-104802 base: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8 config: arm64-randconfig-r031-20220906 (https://download.01.org/0day-ci/archive/20220907/202209070101.lerxi9zr-...@intel.com/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project c55b41d5199d2394dd6cdb8f52180d8b81d809d4) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm64 cross compiling tool for clang build # apt-get install binutils-aarch64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/778e4a57afd0db6a6a752487e1a1cda253cd9682 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Jingyu-Wang/drm-amdgpu-cleanup-coding-style-in-amdgpu_kms-c/20220906-104802 git checkout 778e4a57afd0db6a6a752487e1a1cda253cd9682 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot All errors (new ones prefixed by >>): >> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:62: error: too few arguments >> provided to function-like macro invocation ret = copy_to_user(out, &ip, min_t((size_t)size, sizeof(ip))); ^ include/linux/minmax.h:104:9: note: macro 'min_t' defined here #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) ^ >> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:32: error: use of undeclared >> identifier 'min_t'; did you mean 'minfo'? ret = copy_to_user(out, &ip, min_t((size_t)size, sizeof(ip))); ^ minfo drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:514:27: note: 'minfo' declared here struct amdgpu_mode_info *minfo = &adev->mode_info; ^ >> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1385:3: error: expected expression TA_FW_NAME(XGMI), ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 'TA_FW_NAME' #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type) ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1386:3: error: expected expression TA_FW_NAME(RAS), ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 'TA_FW_NAME' #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type) ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1387:3: error: expected expression TA_FW_NAME(HDCP), ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 'TA_FW_NAME' #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type) ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1388:3: error: expected expression TA_FW_NAME(DTM), ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 'TA_FW_NAME' #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type) ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1389:3: error: expected expression TA_FW_NAME(RAP), ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 'TA_FW_NAME' #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type) ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1390:3: error: expected expression TA_FW_NAME(SECUREDISPLAY), ^ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 'TA_FW_NAME' #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type) ^ 8 errors generated. vim +554 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 494 495 /* 496 * Userspace get information ioctl 497 */ 498 /** 499 * amdgpu_info_ioctl - answer a device specific request. 500 * 501 * @dev: drm device pointer 502 * @data: request object 503 * @filp: drm filp 504 * 505 * This function is used to pass device specific parameters to the userspace 5
