Re: [PATCH 09/10] drm/ast: Detect ast device type and config mode without ast device
Hi
Am 13.11.23 um 16:25 schrieb Jocelyn Falempe:
On 13/11/2023 09:50, Thomas Zimmermann wrote:
Return the ast chip and config in the detection function's parameters
instead of storing them directly in the ast device instance.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_main.c | 104 ++---
1 file changed, 57 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_main.c
b/drivers/gpu/drm/ast/ast_main.c
index f100df8d74f71..331a9a861153b 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -76,25 +76,27 @@ static void ast_open_key(void __iomem *ioregs)
__ast_write8_i(ioregs, AST_IO_VGACRI, 0x80,
AST_IO_VGACR80_PASSWORD);
}
-static int ast_device_config_init(struct ast_device *ast)
+static int ast_detect_chip(struct pci_dev *pdev,
+ void __iomem *regs, void __iomem *ioregs,
+ enum ast_chip *chip_out,
+ enum ast_config_mode *config_mode_out)
{
- struct drm_device *dev = >base;
- struct pci_dev *pdev = to_pci_dev(dev->dev);
- struct device_node *np = dev->dev->of_node;
+ struct device *dev = >dev;
+ struct device_node *np = dev->of_node;
+ enum ast_config_mode config_mode = ast_use_defaults;
uint32_t scu_rev = 0x;
+ enum ast_chip chip;
u32 data;
- u8 jregd0, jregd1;
+ u8 vgacrd0, vgacrd1;
/*
* Find configuration mode and read SCU revision
*/
- ast->config_mode = ast_use_defaults;
-
/* Check if we have device-tree properties */
if (np && !of_property_read_u32(np, "aspeed,scu-revision-id",
)) {
/* We do, disable P2A access */
- ast->config_mode = ast_use_dt;
+ config_mode = ast_use_dt;
scu_rev = data;
} else if (pdev->device == PCI_CHIP_AST2000) { // Not all
families have a P2A bridge
/*
@@ -102,9 +104,9 @@ static int ast_device_config_init(struct
ast_device *ast)
* is disabled. We force using P2A if VGA only mode bit
* is set D[7]
*/
- jregd0 = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
- jregd1 = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, 0xff);
- if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+ vgacrd0 = __ast_read8_i(ioregs, AST_IO_VGACRI, 0xd0);
+ vgacrd1 = __ast_read8_i(ioregs, AST_IO_VGACRI, 0xd1);
+ if (!(vgacrd0 & 0x80) || !(vgacrd1 & 0x10)) {
/*
* We have a P2A bridge and it is enabled.
@@ -112,32 +114,32 @@ static int ast_device_config_init(struct
ast_device *ast)
/* Patch AST2500/AST2510 */
if ((pdev->revision & 0xf0) == 0x40) {
- if (!(jregd0 & AST_VRAM_INIT_STATUS_MASK))
- ast_patch_ahb_2500(ast->regs);
+ if (!(vgacrd0 & AST_VRAM_INIT_STATUS_MASK))
+ ast_patch_ahb_2500(regs);
}
/* Double check that it's actually working */
- data = ast_read32(ast, 0xf004);
+ data = __ast_read32(regs, 0xf004);
if ((data != 0x) && (data != 0x00)) {
- ast->config_mode = ast_use_p2a;
+ config_mode = ast_use_p2a;
/* Read SCU7c (silicon revision register) */
- ast_write32(ast, 0xf004, 0x1e6e);
- ast_write32(ast, 0xf000, 0x1);
- scu_rev = ast_read32(ast, 0x1207c);
+ __ast_write32(regs, 0xf004, 0x1e6e);
+ __ast_write32(regs, 0xf000, 0x1);
+ scu_rev = __ast_read32(regs, 0x1207c);
}
}
}
- switch (ast->config_mode) {
+ switch (config_mode) {
case ast_use_defaults:
- drm_info(dev, "Using default configuration\n");
+ dev_info(dev, "Using default configuration\n");
break;
case ast_use_dt:
- drm_info(dev, "Using device-tree for configuration\n");
+ dev_info(dev, "Using device-tree for configuration\n");
break;
case ast_use_p2a:
- drm_info(dev, "Using P2A bridge for configuration\n");
+ dev_info(dev, "Using P2A bridge for configuration\n");
break;
}
@@ -146,63 +148,66 @@ static int ast_device_config_init(struct
ast_device *ast)
*/
if (pdev->revision >= 0x50) {
- ast->chip = AST2600;
- drm_info(dev, "AST 2600 detected\n");
+ chip = AST2600;
+ dev_info(dev, "AST 2600 detected\n");
Adding a macro to set chip and printk could be handy here:
something like
#define set_chip(version) \
chip = version; \
dev_info(dev, "%s detected\n", #version); \
and then set_chip(AST2510)
I was thinking about reworking these messages at some point. Maybe have
a single print somewhere in the code.
} else if (pdev->revision >= 0x40) {
switch (scu_rev & 0x300) {
case 0x0100:
- ast->chip = AST2510;
-
[PATCH] mm/gup: Introduce pin_user_pages_fd() for pinning shmem/hugetlbfs file pages (v3)
For drivers that would like to longterm-pin the pages associated
with a file, the pin_user_pages_fd() API provides an option to
not only pin the pages via FOLL_PIN but also to check and migrate
them if they reside in movable zone or CMA block. This API
currently works with files that belong to either shmem or hugetlbfs.
Files belonging to other filesystems are rejected for now.
The pages need to be located first before pinning them via FOLL_PIN.
If they are found in the page cache, they can be immediately pinned.
Otherwise, they need to be allocated using the filesystem specific
APIs and then pinned.
v2:
- Drop gup_flags and improve comments and commit message (David)
- Allocate a page if we cannot find in page cache for the hugetlbfs
case as well (David)
- Don't unpin pages if there is a migration related failure (David)
- Drop the unnecessary nr_pages <= 0 check (Jason)
- Have the caller of the API pass in file * instead of fd (Jason)
v3: (David)
- Enclose the huge page allocation code with #ifdef CONFIG_HUGETLB_PAGE
(Build error reported by kernel test robot )
- Don't forget memalloc_pin_restore() on non-migration related errors
- Improve the readability of the cleanup code associated with
non-migration related errors
- Augment the comments by describing FOLL_LONGTERM like behavior
- Include the R-b tag from Jason
Cc: David Hildenbrand
Cc: Daniel Vetter
Cc: Mike Kravetz
Cc: Hugh Dickins
Cc: Peter Xu
Cc: Gerd Hoffmann
Cc: Dongwon Kim
Cc: Junxiao Chang
Suggested-by: Jason Gunthorpe
Reviewed-by: Jason Gunthorpe (v2)
Signed-off-by: Vivek Kasireddy
---
include/linux/mm.h | 2 +
mm/gup.c | 109 +
2 files changed, 111 insertions(+)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 418d26608ece..1b675fa35059 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2472,6 +2472,8 @@ long get_user_pages_unlocked(unsigned long start,
unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
+long pin_user_pages_fd(struct file *file, pgoff_t start,
+ unsigned long nr_pages, struct page **pages);
int get_user_pages_fast(unsigned long start, int nr_pages,
unsigned int gup_flags, struct page **pages);
diff --git a/mm/gup.c b/mm/gup.c
index 231711efa390..b3af967cdff1 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -3410,3 +3410,112 @@ long pin_user_pages_unlocked(unsigned long start,
unsigned long nr_pages,
, gup_flags);
}
EXPORT_SYMBOL(pin_user_pages_unlocked);
+
+static struct page *alloc_file_page(struct file *file, pgoff_t idx)
+{
+#ifdef CONFIG_HUGETLB_PAGE
+ struct page *page = ERR_PTR(-ENOMEM);
+ struct folio *folio;
+ int err;
+
+ if (is_file_hugepages(file)) {
+ folio = alloc_hugetlb_folio_nodemask(hstate_file(file),
+NUMA_NO_NODE,
+NULL,
+GFP_USER);
+ if (folio && folio_try_get(folio)) {
+ page = >page;
+ err = hugetlb_add_to_page_cache(folio,
+ file->f_mapping,
+ idx);
+ if (err) {
+ folio_put(folio);
+ free_huge_folio(folio);
+ page = ERR_PTR(err);
+ }
+ }
+ return page;
+ }
+#endif
+ return shmem_read_mapping_page(file->f_mapping, idx);
+}
+
+/**
+ * pin_user_pages_fd() - pin user pages associated with a file
+ * @file: the file whose pages are to be pinned
+ * @start: starting file offset
+ * @nr_pages: number of pages from start to pin
+ * @pages: array that receives pointers to the pages pinned.
+ * Should be at-least nr_pages long.
+ *
+ * Attempt to pin pages associated with a file that belongs to either shmem
+ * or hugetlb. The pages are either found in the page cache or allocated if
+ * necessary. Once the pages are located, they are all pinned via FOLL_PIN.
+ * And, these pinned pages need to be released either using unpin_user_pages()
+ * or unpin_user_page().
+ *
+ * It must be noted that the pages may be pinned for an indefinite amount
+ * of time. And, in most cases, the duration of time they may stay pinned
+ * would be controlled by the userspace. This behavior is effectively the
+ * same as using FOLL_LONGTERM with other GUP APIs.
+ *
+ * Returns number of pages pinned. This would be equal to the number of
+ * pages requested. If no pages were pinned, it returns -errno.
+ */
+long
Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Hi Michael,
Am Montag, 13. November 2023, 17:43:44 CET schrieb Michael Walle:
> The FORCE_STOP_STATE bit is unsuitable to force the DSI link into LP-11
> mode. It seems the bridge internally queues DSI packets and when the
> FORCE_STOP_STATE bit is cleared, they are sent in close succession
> without any useful timing (this also means that the DSI lanes won't go
> into LP-11 mode). The length of this gibberish varies between 1ms and
> 5ms. This sometimes breaks an attached bridge (TI SN65DSI84 in this
> case). In our case, the bridge will fail in about 1 per 500 reboots.
>
> The FORCE_STOP_STATE handling was introduced to have the DSI lanes in
> LP-11 state during the .pre_enable phase. But as it turns out, none of
> this is needed at all. Between samsung_dsim_init() and
> samsung_dsim_set_display_enable() the lanes are already in LP-11 mode.
Apparently LP-11 is actually entered with the call to
samsung_dsim_enable_lane(), but I don't know about other requisites on that
matter. Unfortunately documentation lacks a lot in that regard.
> The code as it was before commit 20c827683de0 ("drm: bridge:
> samsung-dsim: Fix init during host transfer") and 0c14d3130654 ("drm:
> bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec") was correct
> in this regard.
>
> This patch basically reverts both commits. It was tested on an i.MX8M
> SoC with an SN65DSI84 bridge. The signals were probed and the DSI
> packets were decoded during initialization and link start-up. After this
> patch the first DSI packet on the link is a VSYNC packet and the timing
> is correct.
At which point does SN65DSI84 require LP-11?
You have access to a DSI/D-PHY analyzer?
> Command mode between .pre_enable and .enable was also briefly tested by
> a quick hack. There was no DSI link partner which would have responded,
> but it was made sure the DSI packet was send on the link. As a side
> note, the command mode seems to just work in HS mode. I couldn't find
> that the bridge will handle commands in LP mode.
AFAIK ti-sn65dsi83.c only uses I2C for communication. Did you send DSI read/
writes instead?
best regards,
Alexander
> Fixes: 20c827683de0 ("drm: bridge: samsung-dsim: Fix init during host
> transfer") Fixes: 0c14d3130654 ("drm: bridge: samsung-dsim: Fix i.MX8M
> enable flow to meet spec") Signed-off-by: Michael Walle
> ---
> Let me know wether this should be two commits each reverting one, but both
> commits appeared first in kernel 6.5.
>
> drivers/gpu/drm/bridge/samsung-dsim.c | 32 ++-
> 1 file changed, 2 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c index cf777bdb25d2..4233a50baac7
> 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -939,10 +939,6 @@ static int samsung_dsim_init_link(struct samsung_dsim
> *dsi) reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> reg &= ~DSIM_STOP_STATE_CNT_MASK;
> reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
> -
> - if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> - reg |= DSIM_FORCE_STOP_STATE;
> -
> samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>
> reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
> @@ -1387,18 +1383,6 @@ static void samsung_dsim_disable_irq(struct
> samsung_dsim *dsi) disable_irq(dsi->irq);
> }
>
> -static void samsung_dsim_set_stop_state(struct samsung_dsim *dsi, bool
> enable) -{
> - u32 reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> -
> - if (enable)
> - reg |= DSIM_FORCE_STOP_STATE;
> - else
> - reg &= ~DSIM_FORCE_STOP_STATE;
> -
> - samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> -}
> -
> static int samsung_dsim_init(struct samsung_dsim *dsi)
> {
> const struct samsung_dsim_driver_data *driver_data = dsi-
>driver_data;
> @@ -1448,9 +1432,6 @@ static void samsung_dsim_atomic_pre_enable(struct
> drm_bridge *bridge, ret = samsung_dsim_init(dsi);
> if (ret)
> return;
> -
> - samsung_dsim_set_display_mode(dsi);
> - samsung_dsim_set_display_enable(dsi, true);
> }
> }
>
> @@ -1459,12 +1440,8 @@ static void samsung_dsim_atomic_enable(struct
> drm_bridge *bridge, {
> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>
> - if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> - samsung_dsim_set_display_mode(dsi);
> - samsung_dsim_set_display_enable(dsi, true);
> - } else {
> - samsung_dsim_set_stop_state(dsi, false);
> - }
> + samsung_dsim_set_display_mode(dsi);
> + samsung_dsim_set_display_enable(dsi, true);
>
> dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> }
> @@ -1477,9 +1454,6 @@ static void samsung_dsim_atomic_disable(struct
> drm_bridge *bridge, if (!(dsi->state & DSIM_STATE_ENABLED))
> return;
>
> - if
[PATCH] drm/amd/display: fix NULL dereference
The following patch will fix a minor issue where a debug message is
referencing an struct that has just being checked whether is null or
not. This has been noticed by using coccinelle, in the following output:
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c:540:25-29: ERROR:
aconnector is NULL but dereferenced.
Fixes: 5d72e247e58c9 ("drm/amd/display: switch DC over to the new DRM logging
macros")
Signed-off-by: José Pekkarinen
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index ed784cf27d39..7048dab5e356 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -537,8 +537,7 @@ bool dm_helpers_dp_read_dpcd(
struct amdgpu_dm_connector *aconnector = link->priv;
if (!aconnector) {
- drm_dbg_dp(aconnector->base.dev,
- "Failed to find connector for link!\n");
+ DRM_ERROR("Failed to find connector for link!");
return false;
}
--
2.39.2
[V3] drm/panel: auo,b101uan08.3: Fine tune the panel power sequence
For "auo,b101uan08.3" this panel, it is stipulated in the panel spec that
MIPI needs to keep the LP11 state before the lcm_reset pin is pulled high.
Fixes: 56ad624b4cb5 ("drm/panel: support for auo, b101uan08.3 wuxga dsi video
mode panel")
Signed-off-by: Xuxin Xiong
---
Changes in V3:
- Updated the Fixes tag's style.
link to V2:
https://patchwork.kernel.org/project/dri-devel/patch/20231114034505.288569-1-xuxinxi...@huaqin.corp-partner.google.com/
---
Changes in V2:
- Updated the commit message and added the Fixes tag.
link to V1:
https://patchwork.kernel.org/project/dri-devel/patch/20231109092634.1694066-1-xuxinxi...@huaqin.corp-partner.google.com/
---
drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index 9323e7b9e384..a287be1aaf70 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -1709,6 +1709,7 @@ static const struct panel_desc auo_b101uan08_3_desc = {
.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
MIPI_DSI_MODE_LPM,
.init_cmds = auo_b101uan08_3_init_cmd,
+ .lp11_before_reset = true,
};
static const struct drm_display_mode boe_tv105wum_nw0_default_mode = {
--
2.39.2
[V2] drm/panel: auo,b101uan08.3: Fine tune the panel power sequence
For "auo,b101uan08.3" this panel, it is stipulated in the panel spec that
MIPI needs to keep the LP11 state before the lcm_reset pin is pulled high.
Fixes: 56ad624b4cb5("drm/panel: support for auo, b101uan08.3 wuxga dsi video
mode panel")
Signed-off-by: Xuxin Xiong
---
drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index 9323e7b9e384..a287be1aaf70 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -1709,6 +1709,7 @@ static const struct panel_desc auo_b101uan08_3_desc = {
.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
MIPI_DSI_MODE_LPM,
.init_cmds = auo_b101uan08_3_init_cmd,
+ .lp11_before_reset = true,
};
static const struct drm_display_mode boe_tv105wum_nw0_default_mode = {
--
2.39.2
linux-next: build warning after merge of the drm-intel tree
Hi all,
After merging the drm-intel tree, today's linux-next build (htmldocs)
produced this warning:
Documentation/gpu/drm-kms-helpers:296:
drivers/gpu/drm/display/drm_dp_mst_topology.c:5484: ERROR: Unexpected
indentation.
Documentation/gpu/drm-kms-helpers:296:
drivers/gpu/drm/display/drm_dp_mst_topology.c:5488: WARNING: Block quote ends
without a blank line; unexpected unindent.
Introduced by commit
1cd0a5ea4279 ("drm/dp_mst: Factor out a helper to check the atomic state of a
topology manager")
--
Cheers,
Stephen Rothwell
pgplrxRmA_2gh.pgp
Description: OpenPGP digital signature
Re: linux-next: Signed-off-by missing for commit in the drm-misc tree
Hi Luben, BTW, cherry picking commits does not avoid conflicts - in fact it can cause conflicts if there are further changes to the files affected by the cherry picked commit in either the tree/branch the commit was cheery picked from or the destination tree/branch (I have to deal with these all the time when merging the drm trees in linux-next). Much better is to cross merge the branches so that the patch only appears once or have a shared branches that are merged by any other branch that needs the changes. I understand that things are not done like this in the drm trees :-( -- Cheers, Stephen Rothwell pgpOiFluMmljx.pgp Description: OpenPGP digital signature
Re: linux-next: Signed-off-by missing for commit in the drm-misc tree
On 2023-11-13 21:45, Stephen Rothwell wrote:
> Hi Luben,
>
> On Mon, 13 Nov 2023 20:32:40 -0500 Luben Tuikov wrote:
>>
>> On 2023-11-13 20:08, Luben Tuikov wrote:
>>> On 2023-11-13 15:55, Stephen Rothwell wrote:
Hi all,
Commit
0da611a87021 ("dma-buf: add dma_fence_timestamp helper")
is missing a Signed-off-by from its committer.
>>>
>>> In order to merge the scheduler changes necessary for the Xe driver, those
>>> changes
>>> were based on drm-tip, which included this change from drm-misc-fixes, but
>>> which
>>> wasn't present in drm-misc-next.
>>>
>>> I didn't want to create a merge conflict between drm-misc-next and
>>> drm-misc-fixes,
>>> when pulling that change from drm-misc-next to drm-misc-fixes, so that I
>>> can apply
>>
>> ... when pulling that change from from drm-misc-fixes into drm-misc-next, so
>> that I can apply...
>>
>>> the Xe scheduler changes on top of drm-misc-next.
>>
>> The change in drm-misc-fixes is b83ce9cb4a465b. The latter is contained
>> in linus-master, and in drm-misc-fixes, while the former is in drm-misc-next.
>> When we merge linus-master/drm-misc-fixes into drm-misc-next, or whichever
>> way
>> it happens, I'd like to avoid a merge conflict, but wanted to expedite the
>> changes
>> for Xe.
>
> None of that is relevant ... if you commit a patch to a tree that will
> be in the linux kernel tree, you must add your Signed-off-by to the commit.
Hi Stephen,
Noted!
So I always do this when I do git-am and such, but wasn't sure for this one
single cherry-pick whose
original author was the committer in drm-misc-fixes, but will add my
Signed-off-by in those
rare circumstances.
Thanks for the clarification!
--
Regards,
Luben
OpenPGP_0x4C15479431A334AF.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: linux-next: Signed-off-by missing for commit in the drm-misc tree
Hi Luben,
On Mon, 13 Nov 2023 20:32:40 -0500 Luben Tuikov wrote:
>
> On 2023-11-13 20:08, Luben Tuikov wrote:
> > On 2023-11-13 15:55, Stephen Rothwell wrote:
> >> Hi all,
> >>
> >> Commit
> >>
> >> 0da611a87021 ("dma-buf: add dma_fence_timestamp helper")
> >>
> >> is missing a Signed-off-by from its committer.
> >>
> >
> > In order to merge the scheduler changes necessary for the Xe driver, those
> > changes
> > were based on drm-tip, which included this change from drm-misc-fixes, but
> > which
> > wasn't present in drm-misc-next.
> >
> > I didn't want to create a merge conflict between drm-misc-next and
> > drm-misc-fixes,
> > when pulling that change from drm-misc-next to drm-misc-fixes, so that I
> > can apply
>
> ... when pulling that change from from drm-misc-fixes into drm-misc-next, so
> that I can apply...
>
> > the Xe scheduler changes on top of drm-misc-next.
>
> The change in drm-misc-fixes is b83ce9cb4a465b. The latter is contained
> in linus-master, and in drm-misc-fixes, while the former is in drm-misc-next.
> When we merge linus-master/drm-misc-fixes into drm-misc-next, or whichever way
> it happens, I'd like to avoid a merge conflict, but wanted to expedite the
> changes
> for Xe.
None of that is relevant ... if you commit a patch to a tree that will
be in the linux kernel tree, you must add your Signed-off-by to the commit.
--
Cheers,
Stephen Rothwell
pgpeJfWzDw5CV.pgp
Description: OpenPGP digital signature
[PATCH v2] drm/i915: correct the input parameter on _intel_dsb_commit()
Current, the dewake_scanline variable is defined as unsigned int,
an unsigned int variable that is always greater than or equal to 0.
when _intel_dsb_commit function is called by intel_dsb_commit function,
the dewake_scanline variable may have an int value.
So the dewake_scanline variable is necessary to defined as an int.
Fixes: f83b94d23770 ("drm/i915/dsb: Use DEwake to combat PkgC latency")
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202310052201.anvbpgpr-...@intel.com/
Cc: Ville Syrjälä
Cc: Uma Shankar
Signed-off-by: heminhong
---
drivers/gpu/drm/i915/display/intel_dsb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
b/drivers/gpu/drm/i915/display/intel_dsb.c
index 78b6fe24dcd8..7fd6280c54a7 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -340,7 +340,7 @@ static int intel_dsb_dewake_scanline(const struct
intel_crtc_state *crtc_state)
}
static void _intel_dsb_commit(struct intel_dsb *dsb, u32 ctrl,
- unsigned int dewake_scanline)
+ int dewake_scanline)
{
struct intel_crtc *crtc = dsb->crtc;
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
--
2.25.1
Re: linux-next: Signed-off-by missing for commit in the drm-misc tree
On 2023-11-13 20:08, Luben Tuikov wrote:
> On 2023-11-13 15:55, Stephen Rothwell wrote:
>> Hi all,
>>
>> Commit
>>
>> 0da611a87021 ("dma-buf: add dma_fence_timestamp helper")
>>
>> is missing a Signed-off-by from its committer.
>>
>
> In order to merge the scheduler changes necessary for the Xe driver, those
> changes
> were based on drm-tip, which included this change from drm-misc-fixes, but
> which
> wasn't present in drm-misc-next.
>
> I didn't want to create a merge conflict between drm-misc-next and
> drm-misc-fixes,
> when pulling that change from drm-misc-next to drm-misc-fixes, so that I can
> apply
... when pulling that change from from drm-misc-fixes into drm-misc-next, so
that I can apply...
> the Xe scheduler changes on top of drm-misc-next.
The change in drm-misc-fixes is b83ce9cb4a465b. The latter is contained
in linus-master, and in drm-misc-fixes, while the former is in drm-misc-next.
When we merge linus-master/drm-misc-fixes into drm-misc-next, or whichever way
it happens, I'd like to avoid a merge conflict, but wanted to expedite the
changes
for Xe.
--
Regards,
Luben
OpenPGP_0x4C15479431A334AF.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: linux-next: Signed-off-by missing for commit in the drm-misc tree
On 2023-11-13 15:55, Stephen Rothwell wrote:
> Hi all,
>
> Commit
>
> 0da611a87021 ("dma-buf: add dma_fence_timestamp helper")
>
> is missing a Signed-off-by from its committer.
>
In order to merge the scheduler changes necessary for the Xe driver, those
changes
were based on drm-tip, which included this change from drm-misc-fixes, but which
wasn't present in drm-misc-next.
I didn't want to create a merge conflict between drm-misc-next and
drm-misc-fixes,
when pulling that change from drm-misc-next to drm-misc-fixes, so that I can
apply
the Xe scheduler changes on top of drm-misc-next.
--
Regards,
Luben
OpenPGP_0x4C15479431A334AF.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH] drm/sched: Define pr_fmt() for DRM using pr_*()
Hi Jani,
On 2023-11-10 07:40, Jani Nikula wrote:
> On Thu, 09 Nov 2023, Luben Tuikov wrote:
>> Define pr_fmt() as "[drm] " for DRM code using pr_*() facilities, especially
>> when no devices are available. This makes it easier to browse kernel logs.
>
> Please do not merge patches before people have actually had a chance to
> look at them. This was merged *way* too quickly.
>
> This does not do what you think it does, and it's not robust enough.
>
> The drm_print.[ch] facilities use very few pr_*() calls directly. The
> users of pr_*() calls do not necessarily include at
> all, and really don't have to.
>
> Even the ones that do include it, usually have includes
> first, and includes next. Notably, includes
> .
>
> And, of course, defines pr_fmt() itself if not already
> defined.
>
>> Signed-off-by: Luben Tuikov
>> ---
>> include/drm/drm_print.h | 14 ++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
>> index a93a387f8a1a15..e8fe60d0eb8783 100644
>> --- a/include/drm/drm_print.h
>> +++ b/include/drm/drm_print.h
>> @@ -26,6 +26,20 @@
>> #ifndef DRM_PRINT_H_
>> #define DRM_PRINT_H_
>>
>> +/* Define this before including linux/printk.h, so that the format
>> + * string in pr_*() macros is correctly set for DRM. If a file wants
>> + * to define this to something else, it should do so before including
>> + * this header file.
>
> The only way this would work is by including as the
> very first header, and that's fragile at best.
>
>> + *
>> + * It is encouraged code using pr_err() to prefix their format with
>> + * the string "*ERROR* ", to make it easier to scan kernel logs. For
>> + * instance,
>> + * pr_err("*ERROR* ", args).
>
> No, it's encouraged not to use pr_*() at all, and prefer drm device
> based logging, or device based logging.
>
> I'd rather this whole thing was just reverted.
The revert has been pushed--thanks for R-B-ing it.
FWIW, I wanted a device-less DRM print, with a prefix "[drm] *ERROR* ",
because this is what we scan for, especially when we get a blank screen at
boot/modprobe.
There's a few cases in DRM where when we return -E... it's most likely a blank
screen result,
as was the case with a recent debug I had with amdgpu when pushing the variable
sched->rq.
So then I went by this, in linux/printk.h:
/**
* pr_fmt - used by the pr_*() macros to generate the printk format string
* @fmt: format string passed from a pr_*() macro
*
* This macro can be used to generate a unified format string for pr_*()
* macros. A common use is to prefix all pr_*() messages in a file with a common
* string. For example, defining this at the top of a source file:
*
*#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
*
* would prefix all pr_info, pr_emerg... messages in the file with the module
* name.
*/
#ifndef pr_fmt
#define pr_fmt(fmt) fmt
#endif
Any suggestions as to a device-less DRM print with prefix "[drm] *ERROR* "?
--
Regards,
Luben
OpenPGP_0x4C15479431A334AF.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
[PATCH v2 2/2] drm/i915/guc: Add a selftest for FAST_REQUEST errors
From: John Harrison
There is a mechanism for reporting errors from fire and forget H2G
messages. This is the only way to find out about almost any error in
the GuC backend submission path. So it would be useful to know that it
is working.
v2: Fix some dumb over-complications and a couple of typos - review
feedback from Daniele.
Signed-off-by: John Harrison
---
drivers/gpu/drm/i915/gt/uc/intel_guc.h| 4 +
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 9 ++
drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 115 ++
3 files changed, 128 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 2b6dfe62c8f2a..e22c12ce245ad 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -297,6 +297,10 @@ struct intel_guc {
* @number_guc_id_stolen: The number of guc_ids that have been stolen
*/
int number_guc_id_stolen;
+ /**
+* @fast_response_selftest: Backdoor to CT handler for fast response
selftest
+*/
+ u32 fast_response_selftest;
#endif
};
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index 89e314b3756bb..ed6ce73ef3b07 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -1076,6 +1076,15 @@ static int ct_handle_response(struct intel_guc_ct *ct,
struct ct_incoming_msg *r
found = true;
break;
}
+
+#ifdef CONFIG_DRM_I915_SELFTEST
+ if (!found && ct_to_guc(ct)->fast_response_selftest) {
+ CT_DEBUG(ct, "Assuming unsolicited response due to FAST_REQUEST
selftest\n");
+ ct_to_guc(ct)->fast_response_selftest++;
+ found = true;
+ }
+#endif
+
if (!found) {
CT_ERROR(ct, "Unsolicited response message: len %u, data %#x
(fence %u, last %u)\n",
len, hxg[0], fence, ct->requests.last_fence);
diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
index bfb72143566f6..c900aac85adba 100644
--- a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
@@ -286,11 +286,126 @@ static int intel_guc_steal_guc_ids(void *arg)
return ret;
}
+/*
+ * Send a context schedule H2G message with an invalid context id.
+ * This should generate a GUC_RESULT_INVALID_CONTEXT response.
+ */
+static int bad_h2g(struct intel_guc *guc)
+{
+ u32 action[] = {
+ INTEL_GUC_ACTION_SCHED_CONTEXT,
+ 0x12345678,
+ };
+
+ return intel_guc_send_nb(guc, action, ARRAY_SIZE(action), 0);
+}
+
+/*
+ * Set a spinner running to make sure the system is alive and active,
+ * then send a bad but asynchronous H2G command and wait to see if an
+ * error response is returned. If no response is received or if the
+ * spinner dies then the test will fail.
+ */
+#define FAST_RESPONSE_TIMEOUT_MS 1000
+static int intel_guc_fast_request(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_context *ce;
+ struct igt_spinner spin;
+ struct i915_request *rq;
+ intel_wakeref_t wakeref;
+ struct intel_engine_cs *engine = intel_selftest_find_any_engine(gt);
+ bool spinning = false;
+ int ret = 0;
+
+ if (!engine)
+ return 0;
+
+ wakeref = intel_runtime_pm_get(gt->uncore->rpm);
+
+ ce = intel_context_create(engine);
+ if (IS_ERR(ce)) {
+ ret = PTR_ERR(ce);
+ gt_err(gt, "Failed to create spinner request: %pe\n", ce);
+ goto err_pm;
+ }
+
+ ret = igt_spinner_init(, engine->gt);
+ if (ret) {
+ gt_err(gt, "Failed to create spinner: %pe\n", ERR_PTR(ret));
+ goto err_pm;
+ }
+ spinning = true;
+
+ rq = igt_spinner_create_request(, ce, MI_ARB_CHECK);
+ intel_context_put(ce);
+ if (IS_ERR(rq)) {
+ ret = PTR_ERR(rq);
+ gt_err(gt, "Failed to create spinner request: %pe\n", rq);
+ goto err_spin;
+ }
+
+ ret = request_add_spin(rq, );
+ if (ret) {
+ gt_err(gt, "Failed to add Spinner request: %pe\n",
ERR_PTR(ret));
+ goto err_rq;
+ }
+
+ gt->uc.guc.fast_response_selftest = 1;
+
+ ret = bad_h2g(>uc.guc);
+ if (ret) {
+ gt_err(gt, "Failed to send H2G: %pe\n", ERR_PTR(ret));
+ goto err_rq;
+ }
+
+ ret = wait_for(gt->uc.guc.fast_response_selftest != 1 ||
i915_request_completed(rq),
+ FAST_RESPONSE_TIMEOUT_MS);
+ if (ret) {
+ gt_err(gt, "Request wait failed: %pe\n", ERR_PTR(ret));
+ goto err_rq;
+ }
+
+ if (i915_request_completed(rq)) {
+ gt_err(gt, "Spinner died waiting for fast request error!\n");
+
[PATCH v2 0/2] Selftest for FAST_REQUEST feature
From: John Harrison Add a selftest to verify that the FAST_REQUEST mechanism (getting errors back from fire-and-forget H2G commands) is functional. Also fix up a potential false positive in the GuC hang selftest. v2: Fix some dumb over-complications and typos - review feedback from Daniele. Signed-off-by: John Harrison John Harrison (2): drm/i915/guc: Fix for potential false positives in GuC hang selftest drm/i915/guc: Add a selftest for FAST_REQUEST errors drivers/gpu/drm/i915/gt/uc/intel_guc.h| 4 + drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 9 ++ drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 115 ++ .../drm/i915/gt/uc/selftest_guc_hangcheck.c | 2 +- 4 files changed, 129 insertions(+), 1 deletion(-) -- 2.41.0
[PATCH v2 1/2] drm/i915/guc: Fix for potential false positives in GuC hang selftest
From: John Harrison
Noticed that the hangcheck selftest is submitting a non-preemptoble
spinner. That means that even if the GuC does not die, the heartbeat
will still kick in and trigger a reset. Which is rather defeating the
purpose of the test - to verify that the heartbeat will kick in if the
GuC itself has died. The test is deliberately killing the GuC, so it
should never hit the case of a non-dead GuC. But it is not impossible
that the kill might fail at some future point due to other driver
re-work.
So, make the spinner pre-emptible. That way the heartbeat can get
through if the GuC is alive and context switching. Thus a reset only
happens if the GuC dies. Thus, if the kill should stop working the
test will now fail rather than claim to pass.
Signed-off-by: John Harrison
Reviewed-by: Daniele Ceraolo Spurio
---
drivers/gpu/drm/i915/gt/uc/selftest_guc_hangcheck.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc_hangcheck.c
b/drivers/gpu/drm/i915/gt/uc/selftest_guc_hangcheck.c
index 34b5d952e2bcb..26fdc392fce6c 100644
--- a/drivers/gpu/drm/i915/gt/uc/selftest_guc_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc_hangcheck.c
@@ -74,7 +74,7 @@ static int intel_hang_guc(void *arg)
goto err;
}
- rq = igt_spinner_create_request(, ce, MI_NOOP);
+ rq = igt_spinner_create_request(, ce, MI_ARB_CHECK);
intel_context_put(ce);
if (IS_ERR(rq)) {
ret = PTR_ERR(rq);
--
2.41.0
Re: [PATCH] Revert "drm/sched: Define pr_fmt() for DRM using pr_*()"
On 2023-11-11 06:33, Jani Nikula wrote:
> On Sat, 11 Nov 2023, Luben Tuikov wrote:
>> From Jani:
>> The drm_print.[ch] facilities use very few pr_*() calls directly. The
>> users of pr_*() calls do not necessarily include at
>> all, and really don't have to.
>>
>> Even the ones that do include it, usually have includes
>> first, and includes next. Notably, includes
>> .
>>
>> And, of course, defines pr_fmt() itself if not already
>> defined.
>>
>> No, it's encouraged not to use pr_*() at all, and prefer drm device
>> based logging, or device based logging.
>>
>> This reverts commit 36245bd02e88e68ac5955c2958c968879d7b75a9.
>>
>> Signed-off-by: Luben Tuikov
>> Link: https://patchwork.freedesktop.org/patch/msgid/878r75wzm9@intel.com
>
> Reviewed-by: Jani Nikula
>
>
>> ---
>> include/drm/drm_print.h | 14 --
>> 1 file changed, 14 deletions(-)
>>
>> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
>> index e8fe60d0eb8783..a93a387f8a1a15 100644
>> --- a/include/drm/drm_print.h
>> +++ b/include/drm/drm_print.h
>> @@ -26,20 +26,6 @@
>> #ifndef DRM_PRINT_H_
>> #define DRM_PRINT_H_
>>
>> -/* Define this before including linux/printk.h, so that the format
>> - * string in pr_*() macros is correctly set for DRM. If a file wants
>> - * to define this to something else, it should do so before including
>> - * this header file.
>> - *
>> - * It is encouraged code using pr_err() to prefix their format with
>> - * the string "*ERROR* ", to make it easier to scan kernel logs. For
>> - * instance,
>> - * pr_err("*ERROR* ", args).
>> - */
>> -#ifndef pr_fmt
>> -#define pr_fmt(fmt) "[drm] " fmt
>> -#endif
>> -
>> #include
>> #include
>> #include
>>
>> base-commit: 540527b1385fb203cc4513ca838b4de60bbbc49a
>
Pushed.
--
Regards,
Luben
OpenPGP_0x4C15479431A334AF.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
linux-next: manual merge of the drm-misc tree with Linus' tree
Hi all,
Today's linux-next merge of the drm-misc tree got a conflict in:
drivers/gpu/drm/tests/drm_mm_test.c
between commit:
2ba157983974 ("drm/tests: Fix incorrect argument in drm_test_mm_insert_range")
from Linus' tree and commit:
078a5b498d6a ("drm/tests: Remove slow tests")
from the drm-misc tree.
I fixed it up (the latter removed the code updated by the former, so I
did that) and can carry the fix as necessary. This is now fixed as far as
linux-next is concerned, but any non trivial conflicts should be mentioned
to your upstream maintainer when your tree is submitted for merging.
You may also want to consider cooperating with the maintainer of the
conflicting tree to minimise any particularly complex conflicts.
--
Cheers,
Stephen Rothwell
pgpz6sezu_tn1.pgp
Description: OpenPGP digital signature
linux-next: manual merge of the drm-misc tree with Linus' tree
Hi all,
Today's linux-next merge of the drm-misc tree got a conflict in:
drivers/accel/ivpu/ivpu_job.c
between commit:
6309727ef271 ("kthread: add kthread_stop_put")
from Linus' tree and commit:
57c7e3e4800a ("accel/ivpu: Stop job_done_thread on suspend")
from the drm-misc tree.
I fixed it up (I just used the latter version) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.
--
Cheers,
Stephen Rothwell
pgphhWjy0o2hz.pgp
Description: OpenPGP digital signature
linux-next: manual merge of the drm-misc tree with Linus' tree
Hi all,
Today's linux-next merge of the drm-misc tree got a conflict in:
drivers/accel/ivpu/ivpu_ipc.c
between commit:
b0873eead1d1 ("accel/ivpu: Do not use wait event interruptible")
from Linus' tree and commit:
57c7e3e4800a ("accel/ivpu: Stop job_done_thread on suspend")
from the drm-misc tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/accel/ivpu/ivpu_ipc.c
index a4ca40b184d4,618dbc17df80..
--- a/drivers/accel/ivpu/ivpu_ipc.c
+++ b/drivers/accel/ivpu/ivpu_ipc.c
@@@ -210,10 -227,9 +227,9 @@@ int ivpu_ipc_receive(struct ivpu_devic
struct ivpu_ipc_rx_msg *rx_msg;
int wait_ret, ret = 0;
- wait_ret = wait_event_interruptible_timeout(cons->rx_msg_wq,
-
ivpu_ipc_rx_need_wakeup(cons),
-
msecs_to_jiffies(timeout_ms));
+ wait_ret = wait_event_timeout(cons->rx_msg_wq,
- (IS_KTHREAD() && kthread_should_stop()) ||
- !list_empty(>rx_msg_list),
++ivpu_ipc_rx_need_wakeup(cons),
+msecs_to_jiffies(timeout_ms));
if (IS_KTHREAD() && kthread_should_stop())
return -EINTR;
pgpKxqoHuQBCp.pgp
Description: OpenPGP digital signature
[PATCH drm-misc-next 2/2] drm/nouveau: enable dynamic job-flow control
Make use of the scheduler's credit limit and scheduler job's credit
count to account for the actual size of a job, such that we fill up the
ring efficiently.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_abi16.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_exec.c | 4 +++-
drivers/gpu/drm/nouveau/nouveau_sched.c | 9 -
drivers/gpu/drm/nouveau/nouveau_sched.h | 3 ++-
drivers/gpu/drm/nouveau/nouveau_uvmm.c | 4 +++-
6 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c
b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index f8e59cfb1d34..207945700c94 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -316,7 +316,8 @@ nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS)
if (ret)
goto done;
- ret = nouveau_sched_init(>sched, drm, drm->sched_wq);
+ ret = nouveau_sched_init(>sched, drm, drm->sched_wq,
+chan->chan->dma.ib_max);
if (ret)
goto done;
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 7e5f19153829..6f6c31a9937b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -320,7 +320,7 @@ nouveau_cli_init(struct nouveau_drm *drm, const char *sname,
* locks which indirectly or directly are held for allocations
* elsewhere.
*/
- ret = nouveau_sched_init(>sched, drm, NULL);
+ ret = nouveau_sched_init(>sched, drm, NULL, 1);
if (ret)
goto done;
diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c
b/drivers/gpu/drm/nouveau/nouveau_exec.c
index 388831c53551..63c344f4f78e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_exec.c
+++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
@@ -258,10 +258,12 @@ nouveau_exec_job_init(struct nouveau_exec_job **pjob,
}
}
+ args.file_priv = __args->file_priv;
job->chan = __args->chan;
args.sched = __args->sched;
- args.file_priv = __args->file_priv;
+ /* Plus one to account for the HW fence. */
+ args.credits = job->push.count + 1;
args.in_sync.count = __args->in_sync.count;
args.in_sync.s = __args->in_sync.s;
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c
b/drivers/gpu/drm/nouveau/nouveau_sched.c
index faabd662b165..6406d6659361 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.c
@@ -12,7 +12,6 @@
#include "nouveau_abi16.h"
#include "nouveau_sched.h"
-#define NOUVEAU_SCHED_HW_SUBMISSIONS 1
#define NOUVEAU_SCHED_JOB_TIMEOUT_MS 1
/* Starts at 0, since the DRM scheduler interprets those parameters as
(initial)
@@ -85,10 +84,10 @@ nouveau_job_init(struct nouveau_job *job,
ret = -ENOMEM;
goto err_free_objs;
}
-
}
- ret = drm_sched_job_init(>base, >entity, 1, NULL);
+ ret = drm_sched_job_init(>base, >entity,
+args->credits, NULL);
if (ret)
goto err_free_chains;
@@ -396,7 +395,7 @@ static const struct drm_sched_backend_ops nouveau_sched_ops
= {
int
nouveau_sched_init(struct nouveau_sched *sched, struct nouveau_drm *drm,
- struct workqueue_struct *wq)
+ struct workqueue_struct *wq, u32 credit_limit)
{
struct drm_gpu_scheduler *drm_sched = >base;
struct drm_sched_entity *entity = >entity;
@@ -414,7 +413,7 @@ nouveau_sched_init(struct nouveau_sched *sched, struct
nouveau_drm *drm,
ret = drm_sched_init(drm_sched, _sched_ops, wq,
NOUVEAU_SCHED_PRIORITY_COUNT,
-NOUVEAU_SCHED_HW_SUBMISSIONS, 0, job_hang_limit,
+credit_limit, 0, job_hang_limit,
NULL, NULL, "nouveau_sched", drm->dev->dev);
if (ret)
goto fail_wq;
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.h
b/drivers/gpu/drm/nouveau/nouveau_sched.h
index 026f33d9b70c..7ba8ffec135a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.h
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.h
@@ -27,6 +27,7 @@ enum nouveau_job_state {
struct nouveau_job_args {
struct drm_file *file_priv;
struct nouveau_sched *sched;
+ u32 credits;
enum dma_resv_usage resv_usage;
bool sync;
@@ -112,7 +113,7 @@ struct nouveau_sched {
};
int nouveau_sched_init(struct nouveau_sched *sched, struct nouveau_drm *drm,
- struct workqueue_struct *wq);
+ struct workqueue_struct *wq, u32 credit_limit);
void nouveau_sched_fini(struct nouveau_sched *sched);
#endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index
[PATCH drm-misc-next 1/2] drm/nouveau: implement 1:1 scheduler - entity relationship
Recent patches to the DRM scheduler [1][2] allow for a variable number
of run-queues and add support for (shared) workqueues rather than
dedicated kthreads per scheduler. This allows us to create a 1:1
relationship between a GPU scheduler and a scheduler entity, in order to
properly support firmware schedulers being able to handle an arbitrary
amount of dynamically allocated command ring buffers. This perfectly
matches Nouveau's needs, hence make use of it.
Topology wise we create one scheduler instance per client (handling
VM_BIND jobs) and one scheduler instance per channel (handling EXEC
jobs).
All channel scheduler instances share a workqueue, but every client
scheduler instance has a dedicated workqueue. The latter is required to
ensure that for VM_BIND job's free_job() work and run_job() work can
always run concurrently and hence, free_job() work can never stall
run_job() work. For EXEC jobs we don't have this requirement, since EXEC
job's free_job() does not require to take any locks which indirectly or
directly are held for allocations elsewhere.
[1]
https://lore.kernel.org/all/8f53f7ef-7621-4f0b-bdef-d8d20bc49...@redhat.com/T/
[2]
https://lore.kernel.org/all/20231031032439.1558703-1-matthew.br...@intel.com/T/
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_abi16.c | 18 +-
drivers/gpu/drm/nouveau/nouveau_abi16.h | 2 +-
drivers/gpu/drm/nouveau/nouveau_drm.c | 31 ++--
drivers/gpu/drm/nouveau/nouveau_drv.h | 9 +-
drivers/gpu/drm/nouveau/nouveau_exec.c | 7 +-
drivers/gpu/drm/nouveau/nouveau_exec.h | 2 +-
drivers/gpu/drm/nouveau/nouveau_sched.c | 209 +---
drivers/gpu/drm/nouveau/nouveau_sched.h | 35 ++--
drivers/gpu/drm/nouveau/nouveau_uvmm.c | 69 +++-
drivers/gpu/drm/nouveau/nouveau_uvmm.h | 4 +-
10 files changed, 188 insertions(+), 198 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c
b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index 30afbec9e3b1..f8e59cfb1d34 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -126,21 +126,14 @@ nouveau_abi16_chan_fini(struct nouveau_abi16 *abi16,
{
struct nouveau_abi16_ntfy *ntfy, *temp;
- /* When a client exits without waiting for it's queued up jobs to
-* finish it might happen that we fault the channel. This is due to
-* drm_file_free() calling drm_gem_release() before the postclose()
-* callback. Hence, we can't tear down this scheduler entity before
-* uvmm mappings are unmapped. Currently, we can't detect this case.
-*
-* However, this should be rare and harmless, since the channel isn't
-* needed anymore.
-*/
- nouveau_sched_entity_fini(>sched_entity);
+ /* Cancel all jobs from the entity's queue. */
+ drm_sched_entity_fini(>sched.entity);
- /* wait for all activity to stop before cleaning up */
if (chan->chan)
nouveau_channel_idle(chan->chan);
+ nouveau_sched_fini(>sched);
+
/* cleanup notifier state */
list_for_each_entry_safe(ntfy, temp, >notifiers, head) {
nouveau_abi16_ntfy_fini(chan, ntfy);
@@ -323,8 +316,7 @@ nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS)
if (ret)
goto done;
- ret = nouveau_sched_entity_init(>sched_entity, >sched,
- drm->sched_wq);
+ ret = nouveau_sched_init(>sched, drm, drm->sched_wq);
if (ret)
goto done;
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.h
b/drivers/gpu/drm/nouveau/nouveau_abi16.h
index 9f538486c10e..1f5e243c0c75 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.h
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.h
@@ -26,7 +26,7 @@ struct nouveau_abi16_chan {
struct nouveau_bo *ntfy;
struct nouveau_vma *ntfy_vma;
struct nvkm_mm heap;
- struct nouveau_sched_entity sched_entity;
+ struct nouveau_sched sched;
};
struct nouveau_abi16 {
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index f603eaef1560..7e5f19153829 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -201,9 +201,9 @@ nouveau_cli_fini(struct nouveau_cli *cli)
WARN_ON(!list_empty(>worker));
usif_client_fini(cli);
+ nouveau_sched_fini(>sched);
if (uvmm)
nouveau_uvmm_fini(uvmm);
- nouveau_sched_entity_fini(>sched_entity);
nouveau_vmm_fini(>svm);
nouveau_vmm_fini(>vmm);
nvif_mmu_dtor(>mmu);
@@ -310,8 +310,17 @@ nouveau_cli_init(struct nouveau_drm *drm, const char
*sname,
cli->mem = [ret];
- ret = nouveau_sched_entity_init(>sched_entity, >sched,
- drm->sched_wq);
+ /* Don't pass in the (shared) sched_wq in order to let
+* nouveau_sched_init() create a dedicated one for
linux-next: manual merge of the drm-misc tree with Linus' tree
Hi all,
Today's linux-next merge of the drm-misc tree got a conflict in:
drivers/accel/ivpu/ivpu_drv.c
between commit:
828d63042aec ("accel/ivpu: Don't enter d0i3 during FLR")
from Linus' tree and commit:
57c7e3e4800a ("accel/ivpu: Stop job_done_thread on suspend")
from the drm-misc tree.
I fixed it up (I think - see below) and can carry the fix as necessary.
This is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/accel/ivpu/ivpu_drv.c
index 790603017653,51fa60b6254c..
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@@ -389,13 -390,7 +388,14 @@@ void ivpu_prepare_for_reset(struct ivpu
disable_irq(vdev->irq);
ivpu_ipc_disable(vdev);
ivpu_mmu_disable(vdev);
+ ivpu_job_done_thread_disable(vdev);
+}
+
+int ivpu_shutdown(struct ivpu_device *vdev)
+{
+ int ret;
+
+ ivpu_prepare_for_reset(vdev);
ret = ivpu_hw_power_down(vdev);
if (ret)
pgpuUoEepRhXU.pgp
Description: OpenPGP digital signature
Re: [Intel-gfx] [PATCH v3] drm/i915: Skip pxp init if gt is wedged
On Mon, 2023-11-13 at 14:49 -0800, Zhanjun Dong wrote: > The gt wedged could be triggered by missing guc firmware file, HW not > working, etc. Once triggered, it means all gt usage is dead, therefore we > can't enable pxp under this fatal error condition. > > alan:skip alan: this looks good (as per our offline review/discussion), we dont mess with the current driver startup sequence (i.e. pxp failures can never pull down the driver probing and will not generate warnings or errors). Also, if something does break for PXP, we only do a drm_debug if the failure returned is not -ENODEV (since -ENODEV can happen on the majority of cases with legacy products or with non-PXP kernel configs): Reviewed-by: Alan Previn
Re: [PATCH] drm/i915: Initialize residency registers earlier
On Mon, 2023-10-30 at 16:45 -0700, Belgaumkar, Vinay wrote:
alan:skip
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -608,11 +608,13 @@ void intel_rc6_init(struct intel_rc6 *rc6)
> /* Disable runtime-pm until we can save the GPU state with rc6 pctx */
> rpm_get(rc6);
>
> - if (!rc6_supported(rc6))
> - return;
> -
> rc6_res_reg_init(rc6);
>
> + if (!rc6_supported(rc6)) {
> + rpm_put(rc6);
> + return;
> + }
> +
> if (IS_CHERRYVIEW(i915))
> err = chv_rc6_init(rc6);
> else if (IS_VALLEYVIEW(i915))
alan: as far as the bug this patch is addressing (i.e. ensuring that
intel_rc6_print_residency has valid rc6.res_reg values for correct dprc
debugfs output when rc6 is disabled) and release the rpm, this looks good
to me.
However, when looking at the other code flows around the intel_rc6_init/fini
and intel_rc6_enable/disable, i must point out that the calls to rpm_get
and rpm_put from these functions don't seem to be designed with proper
mirror-ing. For example during driver startup, intel_rc6_init (which is called
by intel_gt_pm_init) calls rpm_get at the start but doesn't call rpm_put
before it returns. But back up the callstack in intel_gt_init,
after intel_gt_pm_init, a couple of subsystems get intialized before
intel_gt_resume
is called - which in turn calls intel_rc6_enable which does the rpm_put at its
end.
However before that get and put, i see several error paths that trigger cleanups
(leading eventually to driver load failure), but i think some cases are
completely
missing the put_rpm that intel_rc6_init took. Additionally, the function names
of
rc6_init and __get_rc6 inside i915_pmu.c seems to be confusing although static.
I wish those were named pmu_rc6_init and __pmu_rc6_init and etc.
Anyways, as per offline conversation, we are not trying to solve every
bug and design gap in this patch but just one specific bug fix. So as
per the agreed condition that we create a separate internal issue
to address this "lack of a clean mirrored-function design of rpm_get/put
across the rc6 startup sequences", here is my rb:
Reviewed-by: Alan Previn
Re: [PATCH 4/4] drm/dp_mst: Fix PBN divider calculation for UHBR rates
Hi Imre,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-tip/drm-tip]
url:
https://github.com/intel-lab-lkp/linux/commits/Imre-Deak/drm-i915-dp-Fix-UHBR-link-M-N-values/20231114-043135
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link:
https://lore.kernel.org/r/20231113201110.510724-4-imre.deak%40intel.com
patch subject: [PATCH 4/4] drm/dp_mst: Fix PBN divider calculation for UHBR
rates
config: i386-randconfig-002-20231114
(https://download.01.org/0day-ci/archive/20231114/202311140621.sw31vg8m-...@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20231114/202311140621.sw31vg8m-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202311140621.sw31vg8m-...@intel.com/
All errors (new ones prefixed by >>):
ld: drivers/gpu/drm/display/drm_dp_mst_topology.o: in function
`drm_dp_get_vc_payload_bw':
>> drivers/gpu/drm/display/drm_dp_mst_topology.c:3598: undefined reference to
>> `__udivdi3'
vim +3598 drivers/gpu/drm/display/drm_dp_mst_topology.c
3570
3571 /**
3572 * drm_dp_get_vc_payload_bw - get the VC payload BW for an MST link
3573 * @mgr: The _dp_mst_topology_mgr to use
3574 * @link_rate: link rate in 10kbits/s units
3575 * @link_lane_count: lane count
3576 *
3577 * Calculate the total bandwidth of a MultiStream Transport link. The
returned
3578 * value is in units of PBNs/(timeslots/1 MTP). This value can be used
to
3579 * convert the number of PBNs required for a given stream to the number
of
3580 * timeslots this stream requires in each MTP.
3581 */
3582 int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr,
3583 int link_rate, int link_lane_count)
3584 {
3585 int ret;
3586
3587 if (link_rate == 0 || link_lane_count == 0)
3588 drm_dbg_kms(mgr->dev, "invalid link rate/lane count:
(%d / %d)\n",
3589 link_rate, link_lane_count);
3590
3591 /* See DP v2.0 2.6.4.2, 2.7.6.3
VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */
3592 /*
3593 * TODO: Return the value with a higher precision, allowing a
better
3594 * slots per MTP allocation granularity. With the current
returned
3595 * value +1 slot/MTP can get allocated on UHBR links.
3596 */
3597 ret = mul_u32_u32(link_rate * link_lane_count,
> 3598
> drm_dp_bw_channel_coding_efficiency(drm_dp_is_uhbr_rate(link_rate))) /
3599(100ULL * 8 * 5400);
3600
3601 return ret;
3602 }
3603 EXPORT_SYMBOL(drm_dp_get_vc_payload_bw);
3604
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[PATCH v3] drm: omapdrm: Improve check for contiguous buffers
While a scatter-gather table having only 1 entry does imply it is
contiguous, it is a logic error to assume the inverse. Tables can have
more than 1 entry and still be contiguous. Use a proper check here.
Signed-off-by: Andrew Davis
---
Changes from v2:
- Double check that these multi-segment SGTs are handled correctly elsewhere
in the driver
- Rebase on v6.7-rc1
Changes from v1:
- Sent correct version of patch :)
drivers/gpu/drm/omapdrm/omap_gem.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
b/drivers/gpu/drm/omapdrm/omap_gem.c
index c48fa531ca321..3421e8389222a 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -48,7 +48,7 @@ struct omap_gem_object {
* OMAP_BO_MEM_DMA_API flag set)
*
* - buffers imported from dmabuf (with the OMAP_BO_MEM_DMABUF flag set)
-* if they are physically contiguous (when sgt->orig_nents == 1)
+* if they are physically contiguous
*
* - buffers mapped through the TILER when pin_cnt is not zero, in which
* case the DMA address points to the TILER aperture
@@ -148,12 +148,18 @@ u64 omap_gem_mmap_offset(struct drm_gem_object *obj)
return drm_vma_node_offset_addr(>vma_node);
}
+static bool omap_gem_sgt_is_contiguous(struct sg_table *sgt, size_t size)
+{
+ return !(drm_prime_get_contiguous_size(sgt) < size);
+}
+
static bool omap_gem_is_contiguous(struct omap_gem_object *omap_obj)
{
if (omap_obj->flags & OMAP_BO_MEM_DMA_API)
return true;
- if ((omap_obj->flags & OMAP_BO_MEM_DMABUF) && omap_obj->sgt->nents == 1)
+ if ((omap_obj->flags & OMAP_BO_MEM_DMABUF) &&
+ omap_gem_sgt_is_contiguous(omap_obj->sgt, omap_obj->base.size))
return true;
return false;
@@ -1385,7 +1391,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct
drm_device *dev, size_t size,
union omap_gem_size gsize;
/* Without a DMM only physically contiguous buffers can be supported. */
- if (sgt->orig_nents != 1 && !priv->has_dmm)
+ if (!omap_gem_sgt_is_contiguous(sgt, size) && !priv->has_dmm)
return ERR_PTR(-EINVAL);
gsize.bytes = PAGE_ALIGN(size);
@@ -1399,7 +1405,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct
drm_device *dev, size_t size,
omap_obj->sgt = sgt;
- if (sgt->orig_nents == 1) {
+ if (omap_gem_sgt_is_contiguous(sgt, size)) {
omap_obj->dma_addr = sg_dma_address(sgt->sgl);
} else {
/* Create pages list from sgt */
--
2.39.2
Re: [PATCH 4/4] drm/dp_mst: Fix PBN divider calculation for UHBR rates
Hi Imre, kernel test robot noticed the following build errors: [auto build test ERROR on drm-tip/drm-tip] url: https://github.com/intel-lab-lkp/linux/commits/Imre-Deak/drm-i915-dp-Fix-UHBR-link-M-N-values/20231114-043135 base: git://anongit.freedesktop.org/drm/drm-tip drm-tip patch link: https://lore.kernel.org/r/20231113201110.510724-4-imre.deak%40intel.com patch subject: [PATCH 4/4] drm/dp_mst: Fix PBN divider calculation for UHBR rates config: i386-buildonly-randconfig-002-20231114 (https://download.01.org/0day-ci/archive/20231114/202311140620.1ghqrb4g-...@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231114/202311140620.1ghqrb4g-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202311140620.1ghqrb4g-...@intel.com/ All errors (new ones prefixed by >>): ld: drivers/gpu/drm/display/drm_dp_mst_topology.o: in function `drm_dp_get_vc_payload_bw': >> drm_dp_mst_topology.c:(.text+0x931): undefined reference to `__udivdi3' -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
[PATCH v3] drm/i915: Skip pxp init if gt is wedged
The gt wedged could be triggered by missing guc firmware file, HW not working, etc. Once triggered, it means all gt usage is dead, therefore we can't enable pxp under this fatal error condition. v2: Updated commit message. v3: Updated return code check. Signed-off-by: Zhanjun Dong --- drivers/gpu/drm/i915/i915_driver.c | 4 +++- drivers/gpu/drm/i915/pxp/intel_pxp.c | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index 80e85cadb9a2..b74977ceb455 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -804,7 +804,9 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_cleanup_modeset2; - intel_pxp_init(i915); + ret = intel_pxp_init(i915); + if (ret != -ENODEV) + drm_dbg(>drm, "pxp init failed with %d\n", ret); ret = intel_display_driver_probe(i915); if (ret) diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index dc327cf40b5a..3e33b7de1dfd 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -199,6 +199,9 @@ int intel_pxp_init(struct drm_i915_private *i915) struct intel_gt *gt; bool is_full_feature = false; + if (intel_gt_is_wedged(to_gt(i915))) + return -ENOTCONN; + /* * NOTE: Get the ctrl_gt before checking intel_pxp_is_supported since * we still need it if PXP's backend tee transport is needed. -- 2.34.1
Re: [RFC PATCH v3 12/12] selftests: add ncdevmem, netcat for devmem TCP
On Sun, 12 Nov 2023 20:08:10 -0800 Mina Almasry wrote: > 1. For (b), would it be OK to implement a very minimal version of > queue_[stop|start]/queue_mem_[alloc|free], which I use for the sole > purpose of reposting buffers to an individual queue, and then later > whoever picks up your queue API effort (maybe me) extends the > implementation to do the rest of the things you described in your > email? If not, what is the minimal queue API I can implement and use > for devmem TCP? Any form of queue API is better than a temporary ndo. IIUC it will not bubble up into uAPI in any way so we can extend/change it later as needed. > 2. Since this is adding ndo, do I need to implement the ndo for 2 > drivers or is GVE sufficient? One driver is fine, especially if we're doing this instead of the reset hack.
Re: [RFC PATCH v3 08/12] net: support non paged skb frags
On Sun, 12 Nov 2023 22:05:30 -0800 Mina Almasry wrote: > My issue here is that all these skb helpers call each other so I end > up having to move a lot of the unrelated skb helpers to this new > header (maybe that is acceptable but it feels weird). Splitting pp headers again is not an option, we already split it into types and helpers. The series doesn't apply and it's a bit hard for me to, in between LPC talks, figure out how to extract your patches from the GH UI to take a proper look :( We can defer this for now, and hopefully v4 will apply to net-next. But it will need to get solved.
[PATCH drm-misc-next] drm/nouveau: use GPUVM common infrastructure
GPUVM provides common infrastructure to track external and evicted GEM
objects as well as locking and validation helpers.
Especially external and evicted object tracking is a huge improvement
compared to the current brute force approach of iterating all mappings
in order to lock and validate the GPUVM's GEM objects. Hence, make us of
it.
Signed-off-by: Danilo Krummrich
---
Originally, this patch was part of [1]. However, while applying the series,
I noticed that this patch needs another iteration, hence I held it back to
rework it and send it separately.
Changes since detached from [1]:
- Don't use drm_gpuvm_exec_lock() since it would, unnecessarily, lock all the
backing buffer's dma-resv locks. Instead, lock only the GEMs affected by the
requested mapping operations, directly or indirectly through map, remap or
unmap.
- Validate backing buffers in drm_exec loop directly.
[1]
https://lore.kernel.org/dri-devel/20231108001259.15123-1-d...@redhat.com/T/#u
---
drivers/gpu/drm/nouveau/nouveau_bo.c| 4 +-
drivers/gpu/drm/nouveau/nouveau_exec.c | 57 +++---
drivers/gpu/drm/nouveau/nouveau_exec.h | 4 -
drivers/gpu/drm/nouveau/nouveau_sched.c | 9 +-
drivers/gpu/drm/nouveau/nouveau_sched.h | 7 +-
drivers/gpu/drm/nouveau/nouveau_uvmm.c | 134 +---
6 files changed, 100 insertions(+), 115 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 7afad86da64b..b7dda486a7ea 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1061,17 +1061,18 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool
evict,
{
struct nouveau_drm *drm = nouveau_bdev(bo->bdev);
struct nouveau_bo *nvbo = nouveau_bo(bo);
+ struct drm_gem_object *obj = >base;
struct ttm_resource *old_reg = bo->resource;
struct nouveau_drm_tile *new_tile = NULL;
int ret = 0;
-
if (new_reg->mem_type == TTM_PL_TT) {
ret = nouveau_ttm_tt_bind(bo->bdev, bo->ttm, new_reg);
if (ret)
return ret;
}
+ drm_gpuvm_bo_gem_evict(obj, evict);
nouveau_bo_move_ntfy(bo, new_reg);
ret = ttm_bo_wait_ctx(bo, ctx);
if (ret)
@@ -1136,6 +1137,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
out_ntfy:
if (ret) {
nouveau_bo_move_ntfy(bo, bo->resource);
+ drm_gpuvm_bo_gem_evict(obj, !evict);
}
return ret;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c
b/drivers/gpu/drm/nouveau/nouveau_exec.c
index bf6c12f4342a..9d9835fb5970 100644
--- a/drivers/gpu/drm/nouveau/nouveau_exec.c
+++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: MIT
-#include
-
#include "nouveau_drv.h"
#include "nouveau_gem.h"
#include "nouveau_mem.h"
@@ -86,14 +84,12 @@
*/
static int
-nouveau_exec_job_submit(struct nouveau_job *job)
+nouveau_exec_job_submit(struct nouveau_job *job,
+ struct drm_gpuvm_exec *vme)
{
struct nouveau_exec_job *exec_job = to_nouveau_exec_job(job);
struct nouveau_cli *cli = job->cli;
struct nouveau_uvmm *uvmm = nouveau_cli_uvmm(cli);
- struct drm_exec *exec = >exec;
- struct drm_gem_object *obj;
- unsigned long index;
int ret;
/* Create a new fence, but do not emit yet. */
@@ -102,52 +98,29 @@ nouveau_exec_job_submit(struct nouveau_job *job)
return ret;
nouveau_uvmm_lock(uvmm);
- drm_exec_init(exec, DRM_EXEC_INTERRUPTIBLE_WAIT |
- DRM_EXEC_IGNORE_DUPLICATES);
- drm_exec_until_all_locked(exec) {
- struct drm_gpuva *va;
-
- drm_gpuvm_for_each_va(va, >base) {
- if (unlikely(va == >base.kernel_alloc_node))
- continue;
-
- ret = drm_exec_prepare_obj(exec, va->gem.obj, 1);
- drm_exec_retry_on_contention(exec);
- if (ret)
- goto err_uvmm_unlock;
- }
+ ret = drm_gpuvm_exec_lock(vme);
+ if (ret) {
+ nouveau_uvmm_unlock(uvmm);
+ return ret;
}
nouveau_uvmm_unlock(uvmm);
- drm_exec_for_each_locked_object(exec, index, obj) {
- struct nouveau_bo *nvbo = nouveau_gem_object(obj);
-
- ret = nouveau_bo_validate(nvbo, true, false);
- if (ret)
- goto err_exec_fini;
+ ret = drm_gpuvm_exec_validate(vme);
+ if (ret) {
+ drm_gpuvm_exec_unlock(vme);
+ return ret;
}
return 0;
-
-err_uvmm_unlock:
- nouveau_uvmm_unlock(uvmm);
-err_exec_fini:
- drm_exec_fini(exec);
- return ret;
-
}
static void
-nouveau_exec_job_armed_submit(struct
Re: [RFC PATCH v3 02/12] net: page_pool: create hooks for custom page providers
On Sun, 12 Nov 2023 19:28:52 -0800 Mina Almasry wrote: > My issue with this is that if the driver doesn't support dmabuf then > the driver will accidentally use the pp backed by the dmabuf, allocate > a page from it, then call page_address() on it or something, and > crash. > > Currently I avoid that by having the driver be responsible for picking > up the dmabuf from the netdev_rx_queue and giving it to the page pool. > What would be the appropriate way to check for driver support in the > netlink API? Perhaps adding something to ndo_features_check? We need some form of capabilities. I was expecting to add that as part of the queue API. Either a new field in struct net_device or in ndos. I tend to put static driver caps of this nature into ops. See for instance .supported_ring_params in ethtool ops.
Re: [PATCH 0/3] pwm: Drop useless member "pwm" from struct pwm_device
Hello, On Fri, Jul 28, 2023 at 04:58:21PM +0200, Uwe Kleine-König wrote: > there are only two users of struct pwm_device::pwm in the tree; both use > it for some dev_dbg output. While this number allows to identify the > PWM, it's not trivial, for example the data currently available in > /sys/kernel/debug/pwm isn't enough. (You have to look in /sys/class/pwm, > pick the pwmchip with the highest number that isn't bigger than the > PWM's number.) > > To be honest the label isn't always usefull either, but it's easy to use > and should be enough to identify the used PWM. The parent device + hwid > might be more useful?! On the other hand using that for a dev_dbg that > is probably only looked at by someone debugging the driver and thus > knowing the used PWM anyhow is of little value either. > > Assuming this change is still considered worthwile I suggest that patches #1 > and #2 go in via their respective maintainer trees and I resend patch #3 to go > via the pwm tree once these two are "in". > > Best regards > Uwe > > Uwe Kleine-König (3): > drm/ssd130x: Print the PWM's label instead of its number > video: fbdev: ssd1307fb: Print the PWM's label instead of its number > pwm: Drop unused member "pwm" from struct pwm_device The two patches to stop making use of struct pwm_device::pwm are now in Linus's tree (as of v6.7-rc1). The third patch is still "new" in patchwork, so I don't resend. It's great if patch #3 goes in during the next merge window. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
linux-next: Signed-off-by missing for commit in the drm-misc tree
Hi all,
Commit
0da611a87021 ("dma-buf: add dma_fence_timestamp helper")
is missing a Signed-off-by from its committer.
--
Cheers,
Stephen Rothwell
pgpeMaIL1m2Gt.pgp
Description: OpenPGP digital signature
Re: [PATCH 00/20] remove I2C_CLASS_DDC support
> We're not in a hurry. It's just my experience with patch series' affecting > multiple subsystems that typically the decision was to apply the full > series via one tree. Also to avoid inquires from maintainers like: > Shall I take it or are you going to take it? > Of course there may be different opinions. Please advise. Ok, then this turns out to be a negotation thing between the drm/fbdev maintainers and me. I *can* take all the patches, of course. But since the number of patches touching the non-i2c subsystems is high, I'd like to hear their preference, too. signature.asc Description: PGP signature
Re: [PATCH 01/20] drivers/gpu/drm/rockchip: remove I2C_CLASS_DDC support
Am Montag, 13. November 2023, 12:23:25 CET schrieb Heiner Kallweit: > After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in > olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. > Class-based device auto-detection is a legacy mechanism and shouldn't > be used in new code. So we can remove this class completely now. > > Preferably this series should be applied via the i2c tree. > > Signed-off-by: Heiner Kallweit Acked-by: Heiko Stuebner > > --- > drivers/gpu/drm/rockchip/inno_hdmi.c |1 - > drivers/gpu/drm/rockchip/rk3066_hdmi.c |1 - > 2 files changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c > b/drivers/gpu/drm/rockchip/inno_hdmi.c > index 6e5b922a1..a7739b27c 100644 > --- a/drivers/gpu/drm/rockchip/inno_hdmi.c > +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c > @@ -793,7 +793,6 @@ static struct i2c_adapter *inno_hdmi_i2c_adapter(struct > inno_hdmi *hdmi) > init_completion(>cmp); > > adap = >adap; > - adap->class = I2C_CLASS_DDC; > adap->owner = THIS_MODULE; > adap->dev.parent = hdmi->dev; > adap->dev.of_node = hdmi->dev->of_node; > diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c > b/drivers/gpu/drm/rockchip/rk3066_hdmi.c > index fa6e592e0..7a3f71aa2 100644 > --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c > +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c > @@ -725,7 +725,6 @@ static struct i2c_adapter *rk3066_hdmi_i2c_adapter(struct > rk3066_hdmi *hdmi) > init_completion(>cmpltn); > > adap = >adap; > - adap->class = I2C_CLASS_DDC; > adap->owner = THIS_MODULE; > adap->dev.parent = hdmi->dev; > adap->dev.of_node = hdmi->dev->of_node; > >
Re: [PATCH v2 6/8] dt-bindings: reserved-memory: Add secure CMA reserved memory range
On Sat, 11 Nov 2023 19:15:57 +0800, Yong Wu wrote: > Add a binding for describing the secure CMA reserved memory range. The > memory range also will be defined in the TEE firmware. It means the TEE > will be configured with the same address/size that is being set in this > DT node. > > Signed-off-by: Yong Wu > --- > .../reserved-memory/secure_cma_region.yaml| 44 +++ > 1 file changed, 44 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml:12:1: [error] syntax error: could not find expected ':' (syntax) dtschema/dtc warnings/errors: make[2]: *** Deleting file 'Documentation/devicetree/bindings/reserved-memory/secure_cma_region.example.dts' Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml:12:1: could not find expected ':' make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/reserved-memory/secure_cma_region.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs ./Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml:12:1: could not find expected ':' /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml: ignoring, error parsing file make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1427: dt_binding_check] Error 2 make: *** [Makefile:234: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/2023111559.8218-7-yong...@mediatek.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH] drm/i915: eliminate warnings
On Mon, Nov 13, 2023 at 11:36:13AM +0800, heminhong wrote:
> Current, the dewake_scanline variable is defined as unsigned int,
> an unsigned int variable that is always greater than or equal to 0.
> when _intel_dsb_commit function is called by intel_dsb_commit function,
> the dewake_scanline variable may have an int value.
> So the dewake_scanline variable is necessary to defined as an int.
A good catch. But this patch deserves a better commit subject since
it is not just fixing 'warnings' but the behavior of this function
accounts on the -1 that is explicitly given as input in some cases.
>
> Signed-off-by: heminhong
also perhaps:
Fixes: f83b94d23770 ("drm/i915/dsb: Use DEwake to combat PkgC latency")
Cc: Ville Syrjälä
Cc: Uma Shankar
?
> ---
> drivers/gpu/drm/i915/display/intel_dsb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 78b6fe24dcd8..7fd6280c54a7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -340,7 +340,7 @@ static int intel_dsb_dewake_scanline(const struct
> intel_crtc_state *crtc_state)
> }
>
> static void _intel_dsb_commit(struct intel_dsb *dsb, u32 ctrl,
> - unsigned int dewake_scanline)
> + int dewake_scanline)
> {
> struct intel_crtc *crtc = dsb->crtc;
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> --
> 2.25.1
>
Re: [PATCH 5/5] accel/ivpu: Use threaded IRQ to handle JOB done messages
On 11/13/2023 10:02 AM, Jacek Lawrynowicz wrote: Remove job_done thread and replace it with generic callback based mechanism. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 4/5] accel/ivpu: Use dedicated work for job timeout detection
On 11/13/2023 10:02 AM, Jacek Lawrynowicz wrote: From: Stanislaw Gruszka Change to use work for timeout detection. Needed for thread_irq conversion. Signed-off-by: Stanislaw Gruszka Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 3/5] accel/ivpu: Do not use cons->aborted for job_done_thread
On 11/13/2023 10:02 AM, Jacek Lawrynowicz wrote: From: Stanislaw Gruszka This allow to simplify ivpu_ipc_receive() as now we do not have to process all messages in aborted state - they will be freed in ivpu_ipc_consumer_del(). Signed-off-by: Stanislaw Gruszka Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
[PATCH 4/4] drm/dp_mst: Fix PBN divider calculation for UHBR rates
The current way of calculating the pbn_div value, the link BW per each
MTP slot, worked only for DP 1.4 link rates. Fix things up for UHBR
rates calculating with the correct channel coding efficiency based on
the link rate.
On UHBR the resulting pbn_div value is not an integer (vs. DP 1.4 where
the value is always an integer), so ideally a scaled value containing
the fractional part should be returned, so that the PBN -> MTP slot
count (aka TU size) conversion can be done with less error. For now
return a rounded-down value - which can result in +1 excess MTP slot
getting allocated on UHBR links.
Cc: Lyude Paul
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Imre Deak
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 15 +--
include/drm/display/drm_dp_helper.h | 13 +
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 4d72c9a32026e..940a9fc0d0244 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3582,12 +3582,23 @@ static int drm_dp_send_up_ack_reply(struct
drm_dp_mst_topology_mgr *mgr,
int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr,
int link_rate, int link_lane_count)
{
+ int ret;
+
if (link_rate == 0 || link_lane_count == 0)
drm_dbg_kms(mgr->dev, "invalid link rate/lane count: (%d /
%d)\n",
link_rate, link_lane_count);
- /* See DP v2.0 2.6.4.2,
VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */
- return link_rate * link_lane_count / 54000;
+ /* See DP v2.0 2.6.4.2, 2.7.6.3
VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */
+ /*
+* TODO: Return the value with a higher precision, allowing a better
+* slots per MTP allocation granularity. With the current returned
+* value +1 slot/MTP can get allocated on UHBR links.
+*/
+ ret = mul_u32_u32(link_rate * link_lane_count,
+
drm_dp_bw_channel_coding_efficiency(drm_dp_is_uhbr_rate(link_rate))) /
+ (100ULL * 8 * 5400);
+
+ return ret;
}
EXPORT_SYMBOL(drm_dp_get_vc_payload_bw);
diff --git a/include/drm/display/drm_dp_helper.h
b/include/drm/display/drm_dp_helper.h
index caee29d28463c..18ff6af0b5a31 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -251,6 +251,19 @@ drm_edp_backlight_supported(const u8
edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE])
return !!(edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP);
}
+/**
+ * drm_dp_is_uhbr_rate - Determine if a link rate is UHBR
+ * @link_rate: link rate in 10kbits/s units
+ *
+ * Determine if the provided link rate is an UHBR rate.
+ *
+ * Returns: %True if @link_rate is an UHBR rate.
+ */
+static inline bool drm_dp_is_uhbr_rate(int link_rate)
+{
+ return link_rate >= 100;
+}
+
/*
* DisplayPort AUX channel
*/
--
2.39.2
Re: [PATCH 2/5] accel/ivpu: Do not use irqsave in ivpu_ipc_dispatch
On 11/13/2023 10:02 AM, Jacek Lawrynowicz wrote: From: Stanislaw Gruszka ivpu_ipc_dispatch is always called with irqs disabled. Add lockdep assertion and remove unneeded _irqsave/_irqrestore. Signed-off-by: Stanislaw Gruszka Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 1/5] accel/ivpu: Rename cons->rx_msg_lock
On 11/13/2023 10:02 AM, Jacek Lawrynowicz wrote: From: Stanislaw Gruszka Now the cons->rx_msg_lock protects also 'abort' field so rename the lock. "protects also" -> "also protects" Signed-off-by: Stanislaw Gruszka Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
On Tue, Aug 1, 2023 at 11:50 AM Dave Stevenson
wrote:
>
> Hi Jagan
>
> My apologies for dropping the ball on this one, and thanks to Frieder
> for the nudge.
>
> On Wed, 12 Apr 2023 at 07:25, Jagan Teki wrote:
> >
> > Hi Dave,
> >
> > Added Maxime, Laurent [which I thought I added before]
> >
> > On Tue, Mar 28, 2023 at 10:38 PM Jagan Teki
> > wrote:
> > >
> > > For a given bridge pipeline if any bridge sets pre_enable_prev_first
> > > flag then the pre_enable for the previous bridge will be called before
> > > pre_enable of this bridge and opposite is done for post_disable.
> > >
> > > These are the potential bridge flags to alter bridge init order in order
> > > to satisfy the MIPI DSI host and downstream panel or bridge to function.
> > > However the existing pre_enable_prev_first logic with associated bridge
> > > ordering has broken for both pre_enable and post_disable calls.
> > >
> > > [pre_enable]
> > >
> > > The altered bridge ordering has failed if two consecutive bridges on a
> > > given pipeline enables the pre_enable_prev_first flag.
> > >
> > > Example:
> > > - Panel
> > > - Bridge 1
> > > - Bridge 2 pre_enable_prev_first
> > > - Bridge 3
> > > - Bridge 4 pre_enable_prev_first
> > > - Bridge 5 pre_enable_prev_first
> > > - Bridge 6
> > > - Encoder
> > >
> > > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first.
> > >
> > > The logic looks for a bridge which enabled pre_enable_prev_first flag
> > > on each iteration and assigned the previou bridge to limit pointer
> > > if the bridge doesn't enable pre_enable_prev_first flags.
> > >
> > > If control found Bridge 2 is pre_enable_prev_first then the iteration
> > > looks for Bridge 3 and found it is not pre_enable_prev_first and assigns
> > > it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3
> > > and Bridge 2 and assign iter pointer with limit which is Bridge 4.
> > >
> > > Here is the actual problem, for the next iteration control look for
> > > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration
> > > moved to Bridge 4 so this iteration skips the Bridge 4. The iteration
> > > found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned
> > > to Encoder. From next iteration Encoder skips as it is the last bridge
> > > for reverse order pipeline.
> > >
> > > So, the resulting pre_enable bridge order would be,
> > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5.
> > >
> > > This patch fixes this by assigning limit to next pointer instead of
> > > previous bridge since the iteration always looks for bridge that does
> > > NOT request prev so assigning next makes sure the last bridge on a
> > > given iteration what exactly the limit bridge is.
> > >
> > > So, the resulting pre_enable bridge order with fix would be,
> > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
> > > Encoder.
> > >
> > > [post_disable]
> > >
> > > The altered bridge ordering has failed if two consecutive bridges on a
> > > given pipeline enables the pre_enable_prev_first flag.
> > >
> > > Example:
> > > - Panel
> > > - Bridge 1
> > > - Bridge 2 pre_enable_prev_first
> > > - Bridge 3
> > > - Bridge 4 pre_enable_prev_first
> > > - Bridge 5 pre_enable_prev_first
> > > - Bridge 6
> > > - Encoder
> > >
> > > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
> > >
> > > The logic looks for a bridge which enabled pre_enable_prev_first flags
> > > on each iteration and assigned the previou bridge to next and next to
> > > limit pointer if the bridge does enable pre_enable_prev_first flag.
> > >
> > > If control starts from Bridge 6 then it found next Bridge 5 is
> > > pre_enable_prev_first and immediately the next assigned to previous
> > > Bridge 6 and limit assignments to next Bridge 6 and call post_enable
> > > of Bridge 6 even though the next consecutive Bridge 5 is enabled with
> > > pre_enable_prev_first. This clearly misses the logic to find the state
> > > of next conducive bridge as everytime the next and limit assigns
> > > previous bridge if given bridge enabled pre_enable_prev_first.
> > >
> > > So, the resulting post_disable bridge order would be,
> > > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1,
> > > Panel.
> > >
> > > This patch fixes this by assigning next with previou bridge only if the
> > > bridge doesn't enable pre_enable_prev_first flag and the next further
> > > assign it to limit. This way we can find the bridge that NOT requested
> > > prev to disable last.
> > >
> > > So, the resulting pre_enable bridge order with fix would be,
> > > - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1,
> > > Panel.
> > >
> > > Validated the bridge init ordering by incorporating dummy bridges in
> > > the sun6i-mipi-dsi pipeline
> > >
> > > Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
> > > alter bridge init order")
> > > Signed-off-by: Jagan Teki
>
> Thanks for investigating and
Incomplete stable drm/ast backport - screen freeze on boot
Greetings,
When connected to a remote machine via the BMC KVM functionality,
I am experiencing screen freezes on boot when using 6.5 stable,
but not master.
The BMC on the machine in question is an ASpeed AST2600.
A quick bisect shows the problematic commit to be 2fb9667
("drm/ast: report connection status on Display Port.").
This is commit f81bb0ac upstream.
I believe the problem is that the previous commit in the series
e329cb5 ("drm/ast: Add BMC virtual connector")
was not backported to the stable branch.
As a consequence, it appears that the more accurate DP state detection
is causing the kernel to believe that no display is connected,
even when the BMC's virtual display is in fact in use.
A cherry-pick of e329cb5 onto the stable branch resolves the issue.
Cheers,
Keno
Re: [PATCH v3] driver: gpu: Fixing warning directly dereferencing a rcu pointer
On 11/14/23 00:43, Abhinav Singh wrote:
This patch fixes a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
We should not be directly dereferencing a rcu pointer. To get a normal
(non __rcu tagged pointer) from a __rcu tagged pointer we are using the
function unrcu_pointer(...). The non __rcu tagged pointer then can be
dereferenced just like a normal pointer.
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
Signed-off-by: Abhinav Singh
---
v1 -> v2 : Replaced the rcu_dereference(...) with unrcu_pointer(...) and
also removed the rcu locking and unlocking function call.
v2 -> v3 : Changed the description of the patch to match it with the actual
implementation.
drivers/gpu/drm/nouveau/nv04_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_fence.c
b/drivers/gpu/drm/nouveau/nv04_fence.c
index 5b71a5a5cd85..cdbc75e3d1f6 100644
--- a/drivers/gpu/drm/nouveau/nv04_fence.c
+++ b/drivers/gpu/drm/nouveau/nv04_fence.c
@@ -39,7 +39,7 @@ struct nv04_fence_priv {
static int
nv04_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ struct nvif_push *push = unrcu_pointer(fence->channel)->chan.push;
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_NVSQ(push, NV_SW, 0x0150, fence->base.seqno);
Hi, just for the sake of my own confirmation, the patch is merge ready
right? once the CI runs successfully it will be merged right?
Thank You,
Abhinav Singh
[PATCH v3] driver: gpu: Fixing warning directly dereferencing a rcu pointer
This patch fixes a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
We should not be directly dereferencing a rcu pointer. To get a normal
(non __rcu tagged pointer) from a __rcu tagged pointer we are using the
function unrcu_pointer(...). The non __rcu tagged pointer then can be
dereferenced just like a normal pointer.
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
Signed-off-by: Abhinav Singh
---
v1 -> v2 : Replaced the rcu_dereference(...) with unrcu_pointer(...) and
also removed the rcu locking and unlocking function call.
v2 -> v3 : Changed the description of the patch to match it with the actual
implementation.
drivers/gpu/drm/nouveau/nv04_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_fence.c
b/drivers/gpu/drm/nouveau/nv04_fence.c
index 5b71a5a5cd85..cdbc75e3d1f6 100644
--- a/drivers/gpu/drm/nouveau/nv04_fence.c
+++ b/drivers/gpu/drm/nouveau/nv04_fence.c
@@ -39,7 +39,7 @@ struct nv04_fence_priv {
static int
nv04_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ struct nvif_push *push = unrcu_pointer(fence->channel)->chan.push;
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_NVSQ(push, NV_SW, 0x0150, fence->base.seqno);
--
2.39.2
Re: [PATCH v2] driver: gpu: Fixing warning directly dereferencing a rcu pointer
On 11/13/23 19:55, Abhinav Singh wrote:
On 11/14/23 00:19, Danilo Krummrich wrote:
Hi,
thanks for sending a v2.
On 11/13/23 19:42, Abhinav Singh wrote:
This patch fixes a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
Better use imperative here, e.g. "Fix a sparse warning ...".
Wouldn't ask you to send a v3 for that alone...
We should not be directly dereferencing a rcu pointer, rather we should
be using rcu helper function rcu_dereferece() inside rcu read critical
section to get a normal pointer which can be dereferenced.
...but this doesn't seem accurate anymore as well.
- Danilo
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
Signed-off-by: Abhinav Singh
---
v1 -> v2 : Replaced the rcu_dereference(...) with unrcu_pointer(...) and
also removed the rcu locking and unlocking function call.
drivers/gpu/drm/nouveau/nv04_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_fence.c
b/drivers/gpu/drm/nouveau/nv04_fence.c
index 5b71a5a5cd85..cdbc75e3d1f6 100644
--- a/drivers/gpu/drm/nouveau/nv04_fence.c
+++ b/drivers/gpu/drm/nouveau/nv04_fence.c
@@ -39,7 +39,7 @@ struct nv04_fence_priv {
static int
nv04_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ struct nvif_push *push = unrcu_pointer(fence->channel)->chan.push;
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_NVSQ(push, NV_SW, 0x0150, fence->base.seqno);
Hi maintainers thanks a lot for reviewing this patch.
I think I should fix my mistake by sending in another patch so that the code
changes and description matches. So should I send another patch ?
Yes, please send a v3.
Thank You,
Abhinav Singh
Re: [PATCH v2] driver: gpu: Fixing warning directly dereferencing a rcu pointer
On 11/14/23 00:19, Danilo Krummrich wrote:
Hi,
thanks for sending a v2.
On 11/13/23 19:42, Abhinav Singh wrote:
This patch fixes a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
Better use imperative here, e.g. "Fix a sparse warning ...".
Wouldn't ask you to send a v3 for that alone...
We should not be directly dereferencing a rcu pointer, rather we should
be using rcu helper function rcu_dereferece() inside rcu read critical
section to get a normal pointer which can be dereferenced.
...but this doesn't seem accurate anymore as well.
- Danilo
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial
net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
Signed-off-by: Abhinav Singh
---
v1 -> v2 : Replaced the rcu_dereference(...) with unrcu_pointer(...) and
also removed the rcu locking and unlocking function call.
drivers/gpu/drm/nouveau/nv04_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_fence.c
b/drivers/gpu/drm/nouveau/nv04_fence.c
index 5b71a5a5cd85..cdbc75e3d1f6 100644
--- a/drivers/gpu/drm/nouveau/nv04_fence.c
+++ b/drivers/gpu/drm/nouveau/nv04_fence.c
@@ -39,7 +39,7 @@ struct nv04_fence_priv {
static int
nv04_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ struct nvif_push *push = unrcu_pointer(fence->channel)->chan.push;
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_NVSQ(push, NV_SW, 0x0150, fence->base.seqno);
Hi maintainers thanks a lot for reviewing this patch.
I think I should fix my mistake by sending in another patch so that the
code changes and description matches. So should I send another patch ?
Thank You,
Abhinav Singh
Re: [PATCH v2] driver: gpu: Fixing warning directly dereferencing a rcu pointer
Hi,
thanks for sending a v2.
On 11/13/23 19:42, Abhinav Singh wrote:
This patch fixes a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
Better use imperative here, e.g. "Fix a sparse warning ...".
Wouldn't ask you to send a v3 for that alone...
We should not be directly dereferencing a rcu pointer, rather we should
be using rcu helper function rcu_dereferece() inside rcu read critical
section to get a normal pointer which can be dereferenced.
...but this doesn't seem accurate anymore as well.
- Danilo
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
Signed-off-by: Abhinav Singh
---
v1 -> v2 : Replaced the rcu_dereference(...) with unrcu_pointer(...) and
also removed the rcu locking and unlocking function call.
drivers/gpu/drm/nouveau/nv04_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_fence.c
b/drivers/gpu/drm/nouveau/nv04_fence.c
index 5b71a5a5cd85..cdbc75e3d1f6 100644
--- a/drivers/gpu/drm/nouveau/nv04_fence.c
+++ b/drivers/gpu/drm/nouveau/nv04_fence.c
@@ -39,7 +39,7 @@ struct nv04_fence_priv {
static int
nv04_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ struct nvif_push *push = unrcu_pointer(fence->channel)->chan.push;
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_NVSQ(push, NV_SW, 0x0150, fence->base.seqno);
[PATCH v2] driver: gpu: Fixing warning directly dereferencing a rcu pointer
This patch fixes a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
We should not be directly dereferencing a rcu pointer, rather we should
be using rcu helper function rcu_dereferece() inside rcu read critical
section to get a normal pointer which can be dereferenced.
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
Signed-off-by: Abhinav Singh
---
v1 -> v2 : Replaced the rcu_dereference(...) with unrcu_pointer(...) and
also removed the rcu locking and unlocking function call.
drivers/gpu/drm/nouveau/nv04_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_fence.c
b/drivers/gpu/drm/nouveau/nv04_fence.c
index 5b71a5a5cd85..cdbc75e3d1f6 100644
--- a/drivers/gpu/drm/nouveau/nv04_fence.c
+++ b/drivers/gpu/drm/nouveau/nv04_fence.c
@@ -39,7 +39,7 @@ struct nv04_fence_priv {
static int
nv04_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ struct nvif_push *push = unrcu_pointer(fence->channel)->chan.push;
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_NVSQ(push, NV_SW, 0x0150, fence->base.seqno);
--
2.39.2
[PATCH v2] drivers: gpu: Fixing warning directly dereferencing a rcu pointer v2
This patch fixes a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
We should not be directly dereferencing a rcu pointer, rather we should
be using rcu helper function rcu_dereferece() inside rcu read critical
section to get a normal pointer which can be dereferenced.
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
Signed-off-by: Abhinav Singh
---
v1 -> v2 : Replaced the rcu_dereference(...) with unrcu_pointer(...) and
also removed the rcu locking and unlocking function call.
drivers/gpu/drm/nouveau/nv04_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_fence.c
b/drivers/gpu/drm/nouveau/nv04_fence.c
index 5b71a5a5cd85..cdbc75e3d1f6 100644
--- a/drivers/gpu/drm/nouveau/nv04_fence.c
+++ b/drivers/gpu/drm/nouveau/nv04_fence.c
@@ -39,7 +39,7 @@ struct nv04_fence_priv {
static int
nv04_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ struct nvif_push *push = unrcu_pointer(fence->channel)->chan.push;
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_NVSQ(push, NV_SW, 0x0150, fence->base.seqno);
--
2.39.2
Re: [PATCH 03/20] drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c: remove I2C_CLASS_DDC support
On 13.11.2023 18:50, Harry Wentland wrote: > Please just use "drm/amd/display:" as tag in the commit subject. > Thanks, noted. Tags have been automatically created by Coccinelle's splitpatch. > With that fixed, this is > Acked-by: Harry Wentland > > Harry > > On 2023-11-13 06:23, Heiner Kallweit wrote: >> After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in >> olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. >> Class-based device auto-detection is a legacy mechanism and shouldn't >> be used in new code. So we can remove this class completely now. >> >> Preferably this series should be applied via the i2c tree. >> >> Signed-off-by: Heiner Kallweit >> >> --- >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> index 6f99f6754..ae1edc6ab 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -7529,7 +7529,6 @@ create_i2c(struct ddc_service *ddc_service, >> if (!i2c) >> return NULL; >> i2c->base.owner = THIS_MODULE; >> -i2c->base.class = I2C_CLASS_DDC; >> i2c->base.dev.parent = >pdev->dev; >> i2c->base.algo = _dm_i2c_algo; >> snprintf(i2c->base.name, sizeof(i2c->base.name), "AMDGPU DM i2c hw bus >> %d", link_index); >> >
RE: [RFC PATCH v2 1/1] drm/virtio: new fence for every plane update
Hi Dmitry, > -Original Message- > From: Dmitry Osipenko > Sent: Monday, November 13, 2023 8:16 AM > To: Kim, Dongwon ; dri- > de...@lists.freedesktop.org > Cc: kra...@redhat.com; Kasireddy, Vivek > Subject: Re: [RFC PATCH v2 1/1] drm/virtio: new fence for every plane update > > On 10/23/23 20:31, Kim, Dongwon wrote: > ... > >> Please write a guide how to test it. Are you using spice for the > >> multi-display viewer? > > > > [DW] Yeah, let me come up with a simple test case. So we use virtio-gpu as > KMS device. It is used to share the guest frame with QEMU. > > SPICE is one of client solutions we use but we primarily use QEMU GTK-UI > w/ multi displays (QEMU with this params '-device virtio- > vga,max_outputs=2,blob=true'). > > Thanks! > > > I'm having trouble wit h reproducing the issue. For GTK-UI you should be > using virtio-vga-gl, shouldn't you? [Kim, Dongwon] I was trying to replicate the issue with more general case as our solution - Mesa kmsro (iris w/ render target imported from virtio-gpu), using i915 for render-only device and virtio-gpu as a display only device -https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9592 hasn't been upstreamed yet. But I got no luck. What I tried was Mesa - sw-rasterizer + virtio-gpu and it worked just fine without any issue. I think the reason would be some timing difference. The problem happens when a cycle of prepare->update-plane->cleanup is overlapped with another, but that didn't seem to happen in the test cases I tried. > > I assume you meant that your simple case is reproducible using dGPU, > correct? I need a test case that is reproducing using virgl and no dGPU. [Kim, Dongwon] We use iGPU only. And we don’t use virgl. We enabled SR-IOV on iGPU so virtual machine can see it as a dedicated HW and it can run HW driver (i915 + Iris) on it. > > Using virtio-vga-gl+virgl+max_outputs=2 I don't see any issues. In the same [Kim, Dongwon] Yeah, this makes me think again the problem can be replicated only in our current setup - Iris/i915 + virtio-gpu and zero copy virtio-gpu display sharing (blob=true) > time switching to the second virtual display doesn't work with Qemu's GTK UI, > I'm getting black screen for the second display. In the KDE settings I have > two > displays and it says both are active. For the first display that works, I > don't > see wrong frames ordering. [Kim, Dongwon] You mean you switched to the second virtual display by changing between tabs? There were some issue with GTK-UI regarding tab handling. I fixed some of them. What version of QEMU are you using? Have you tried the latest one like > 8.0? Wrong ordering of frame is actually what the other patch - drm/virtio: drm_gem_plane_helper_prepare_fb for obj is solving. The problem this patch is trying to solve is complete lock up of virtual display update and fence timeout errors (i915) on the dmesg. Anyway, I just can't say the problem I have been trying to solve can happen on most of general cases but wouldn't it makes sense to you that the fence should be assigned per plane update instead of FB in general? Having fence per plane update is working in the same way as before but it prevents any potential issue like our case. > > Please give me a test case that is reproducing using latest version of > vanilla/upstream Qemu. > > -- > Best regards, > Dmitry
Re: [PATCH 00/20] remove I2C_CLASS_DDC support
On 13.11.2023 18:49, Wolfram Sang wrote: > >> Preferably this series should be applied via the i2c tree. > > Are we in a hurry here, i.e. does it block further development of the > i801 smbus driver? My gut feeling says the patches should rather go via > drm and fbdev trees, but I may be convinced otherwise. > We're not in a hurry. It's just my experience with patch series' affecting multiple subsystems that typically the decision was to apply the full series via one tree. Also to avoid inquires from maintainers like: Shall I take it or are you going to take it? Of course there may be different opinions. Please advise.
Re: [PATCH 15/20] drivers/gpu/drm/i915/display: remove I2C_CLASS_DDC support
On 13.11.2023 18:50, Jani Nikula wrote:
> On Mon, 13 Nov 2023, Heiner Kallweit wrote:
>> On 13.11.2023 13:17, Jani Nikula wrote:
>>> On Mon, 13 Nov 2023, Heiner Kallweit wrote:
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in
olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC.
Class-based device auto-detection is a legacy mechanism and shouldn't
be used in new code. So we can remove this class completely now.
>>>
>>> So this is copy-pasted to all commits and the cover letter, but please
>>> do explain why there are no functional changes here (or are there?),
>>> without me having to go through the i2c stack and try to find the
>>> commits alluded to in "After removal of the legacy ...".
>>>
>> Legacy eeprom driver was marked deprecated 4 yrs ago with:
>> 3079b54aa9a0 ("eeprom: Warn that the driver is deprecated")
>> Now it has been removed with:
>> 0113a99b8a75 ("eeprom: Remove deprecated legacy eeprom driver")
>>
>> Declaration of I2C_CLASS_DDC support is a no-op now, so there's
>> no functional change in this patch.
>>
>> If loaded manually, the legacy eeprom driver exposed the DDC EEPROM
>> to userspace. If this functionality is needed, then now the DDC
>> EEPROM has to be explicitly instantiated using at24.
>>
>> See also:
>> https://docs.kernel.org/i2c/instantiating-devices.html
>
> I'll take your word for it. Though none of the documentation I can find
> say that setting the class is legacy or deprecated or should be
> avoided. *shrug*.
>
I have to agree that it's not obvious that class-based instantiation
is considered a legacy mechanism. The commit message of this 9 yrs old
commit provides an explanation.
0c176170089c ("i2c: add deprecation warning for class based instantiation")
> Acked-by: Jani Nikula
>
Thanks
>
>>
>>
>>> What does this mean?
>>>
>>>
>>> BR,
>>> Jani.
>>>
>> Heiner
>>
>>>
Preferably this series should be applied via the i2c tree.
Signed-off-by: Heiner Kallweit
---
drivers/gpu/drm/i915/display/intel_gmbus.c |1 -
drivers/gpu/drm/i915/display/intel_sdvo.c |1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c
b/drivers/gpu/drm/i915/display/intel_gmbus.c
index 40d7b6f3f..e9e4dcf34 100644
--- a/drivers/gpu/drm/i915/display/intel_gmbus.c
+++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
@@ -899,7 +899,6 @@ int intel_gmbus_setup(struct drm_i915_private *i915)
}
bus->adapter.owner = THIS_MODULE;
- bus->adapter.class = I2C_CLASS_DDC;
snprintf(bus->adapter.name,
sizeof(bus->adapter.name),
"i915 gmbus %s", gmbus_pin->name);
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index a636f42ce..5e64d1baf 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -3311,7 +3311,6 @@ intel_sdvo_init_ddc_proxy(struct intel_sdvo_ddc *ddc,
ddc->ddc_bus = ddc_bus;
ddc->ddc.owner = THIS_MODULE;
- ddc->ddc.class = I2C_CLASS_DDC;
snprintf(ddc->ddc.name, I2C_NAME_SIZE, "SDVO %c DDC%d",
port_name(sdvo->base.port), ddc_bus);
ddc->ddc.dev.parent = >dev;
>>>
>>
>
Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/gt: Timeout when waiting for idle in suspending
On Wed, 2023-10-25 at 13:58 +0100, Tvrtko Ursulin wrote: > On 04/10/2023 18:59, Teres Alexis, Alan Previn wrote: > > On Thu, 2023-09-28 at 13:46 +0100, Tvrtko Ursulin wrote: > > > On 27/09/2023 17:36, Teres Alexis, Alan Previn wrote: alan:snip > > > It is not possible to wait for lost G2H in something like > > > intel_uc_suspend() and simply declare "bad things happened" if it times > > > out there, and forcibly clean it all up? (Which would include releasing > > > all the abandoned pm refs, so this patch wouldn't be needed.) > > > > > alan: I'm not sure if intel_uc_suspend should be held up by gt-level wakeref > > check unless huc/guc/gsc-uc are the only ones ever taking a gt wakeref. > > > > As we already know, what we do know from a uc-perspective: > > - ensure the outstanding guc related workers is flushed which we didnt > > before > > (addressed by patch #1). > > - any further late H2G-SchedDisable is not leaking wakerefs when calling H2G > > and not realizing it failed (addressed by patch #2). > > - (we already), "forcibly clean it all up" at the end of the > > intel_uc_suspend > > when we do the guc reset and cleanup all guc-ids. (pre-existing upstream > > code) > > - we previously didnt have a coherrent guarantee that "this is the end" > > i.e. no > > more new request after intel_uc_suspend. I mean by code logic, we thought > > we did > > (thats why intel_uc_suspend ends wth a guc reset), but we now know > > otherwise. > > So we that fix by adding the additional rcu_barrier (also part of patch #2). > > It is not clear to me from the above if that includes cleaning up the > outstanding CT replies or no. But anyway.. alan: Apologies, i should have made it clearer by citing the actual function name calling out the steps of interest: So if you look at the function "intel_gt_suspend_late", it calls "intel_uc_suspend" which in turn calls "intel_guc_suspend" which does a soft reset onto guc firmware - so after that we can assume all outstanding G2Hs are done. Going back up the stack, intel_gt_suspend_late finally calls gt_sanitize which calls "intel_uc_reset_prepare" which calls "intel_guc_submission_reset_prepare" which calls "scrub_guc_desc_for_outstanding_g2h" which does what we are looking for for all types of outstanding G2H. As per prior review comments, besides closing the race condition, we were missing an rcu_barrier (which caused new requests to come in way late). So we have resolved both in Patch #2. > > That said, patch-3 is NOT fixing a bug in guc -its about "if we ever have > > a future racy gt-wakeref late-leak somewhere - no matter which subsystem > > took it (guc is not the only subsystem taking gt wakerefs), we at > > least don't hang forever in this code. Ofc, based on that, even without > > patch-3 i am confident the issue is resolved anyway. > > So we could just drop patch-3 is you prefer? > > .. given this it does sound to me that if you are confident patch 3 > isn't fixing anything today that it should be dropped. alan: I won't say its NOT fixing anything - i am saying it's not fixing this specific bug where we have the outstanding G2H from a context destruction operation that got dropped. I am okay with dropping this patch in the next rev but shall post a separate stand alone version of Patch3 - because I believe all other i915 subsystems that take runtime-pm's DO NOT wait forever when entering suspend - but GT is doing this. This means if there ever was a bug introduced, it would require serial port or ramoops collection to debug. So i think such a patch, despite not fixing this specific bug will be very helpful for debuggability of future issues. After all, its better to fail our suspend when we have a bug rather than to hang the kernel forever.
Re: [PATCH v2] Remove custom dumb_map_offset implementations in i915 driver
On Sat, 11 Nov 2023, Dipam Turkar wrote:
> Making i915 use drm_gem_create_mmap_offset() instead of its custom
> implementations for associating GEM object with a fake offset.
It would probably help a lot if your commit messages explained what you
are trying to achieve and, especially, why. This only describes the
patch in English.
BR,
Jani.
>
> Signed-off-by: Dipam Turkar
> ---
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 21 -
> drivers/gpu/drm/i915/gem/i915_gem_mman.h | 4
> drivers/gpu/drm/i915/i915_driver.c | 3 ++-
> 3 files changed, 2 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index aa4d842d4c5a..71d621a1f249 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -771,27 +771,6 @@ __assign_mmap_offset_handle(struct drm_file *file,
> return err;
> }
>
> -int
> -i915_gem_dumb_mmap_offset(struct drm_file *file,
> - struct drm_device *dev,
> - u32 handle,
> - u64 *offset)
> -{
> - struct drm_i915_private *i915 = to_i915(dev);
> - enum i915_mmap_type mmap_type;
> -
> - if (HAS_LMEM(to_i915(dev)))
> - mmap_type = I915_MMAP_TYPE_FIXED;
> - else if (pat_enabled())
> - mmap_type = I915_MMAP_TYPE_WC;
> - else if (!i915_ggtt_has_aperture(to_gt(i915)->ggtt))
> - return -ENODEV;
> - else
> - mmap_type = I915_MMAP_TYPE_GTT;
> -
> - return __assign_mmap_offset_handle(file, handle, mmap_type, offset);
> -}
> -
> /**
> * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
> * @dev: DRM device
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> index 196417fd0f5c..253435795caf 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> @@ -20,10 +20,6 @@ struct mutex;
> int i915_gem_mmap_gtt_version(void);
> int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>
> -int i915_gem_dumb_mmap_offset(struct drm_file *file_priv,
> - struct drm_device *dev,
> - u32 handle, u64 *offset);
> -
> void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
> void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c
> b/drivers/gpu/drm/i915/i915_driver.c
> index d50347e5773a..48d7e53c49d6 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -42,6 +42,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -1826,7 +1827,7 @@ static const struct drm_driver i915_drm_driver = {
> .gem_prime_import = i915_gem_prime_import,
>
> .dumb_create = i915_gem_dumb_create,
> - .dumb_map_offset = i915_gem_dumb_mmap_offset,
> + .dumb_map_offset = drm_gem_dumb_map_offset,
>
> .ioctls = i915_ioctls,
> .num_ioctls = ARRAY_SIZE(i915_ioctls),
--
Jani Nikula, Intel
Re: [PATCH 15/20] drivers/gpu/drm/i915/display: remove I2C_CLASS_DDC support
On Mon, 13 Nov 2023, Heiner Kallweit wrote:
> On 13.11.2023 13:17, Jani Nikula wrote:
>> On Mon, 13 Nov 2023, Heiner Kallweit wrote:
>>> After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in
>>> olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC.
>>> Class-based device auto-detection is a legacy mechanism and shouldn't
>>> be used in new code. So we can remove this class completely now.
>>
>> So this is copy-pasted to all commits and the cover letter, but please
>> do explain why there are no functional changes here (or are there?),
>> without me having to go through the i2c stack and try to find the
>> commits alluded to in "After removal of the legacy ...".
>>
> Legacy eeprom driver was marked deprecated 4 yrs ago with:
> 3079b54aa9a0 ("eeprom: Warn that the driver is deprecated")
> Now it has been removed with:
> 0113a99b8a75 ("eeprom: Remove deprecated legacy eeprom driver")
>
> Declaration of I2C_CLASS_DDC support is a no-op now, so there's
> no functional change in this patch.
>
> If loaded manually, the legacy eeprom driver exposed the DDC EEPROM
> to userspace. If this functionality is needed, then now the DDC
> EEPROM has to be explicitly instantiated using at24.
>
> See also:
> https://docs.kernel.org/i2c/instantiating-devices.html
I'll take your word for it. Though none of the documentation I can find
say that setting the class is legacy or deprecated or should be
avoided. *shrug*.
Acked-by: Jani Nikula
>
>
>> What does this mean?
>>
>>
>> BR,
>> Jani.
>>
> Heiner
>
>>
>>>
>>> Preferably this series should be applied via the i2c tree.
>>>
>>> Signed-off-by: Heiner Kallweit
>>>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_gmbus.c |1 -
>>> drivers/gpu/drm/i915/display/intel_sdvo.c |1 -
>>> 2 files changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c
>>> b/drivers/gpu/drm/i915/display/intel_gmbus.c
>>> index 40d7b6f3f..e9e4dcf34 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
>>> @@ -899,7 +899,6 @@ int intel_gmbus_setup(struct drm_i915_private *i915)
>>> }
>>>
>>> bus->adapter.owner = THIS_MODULE;
>>> - bus->adapter.class = I2C_CLASS_DDC;
>>> snprintf(bus->adapter.name,
>>> sizeof(bus->adapter.name),
>>> "i915 gmbus %s", gmbus_pin->name);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
>>> b/drivers/gpu/drm/i915/display/intel_sdvo.c
>>> index a636f42ce..5e64d1baf 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
>>> @@ -3311,7 +3311,6 @@ intel_sdvo_init_ddc_proxy(struct intel_sdvo_ddc *ddc,
>>> ddc->ddc_bus = ddc_bus;
>>>
>>> ddc->ddc.owner = THIS_MODULE;
>>> - ddc->ddc.class = I2C_CLASS_DDC;
>>> snprintf(ddc->ddc.name, I2C_NAME_SIZE, "SDVO %c DDC%d",
>>> port_name(sdvo->base.port), ddc_bus);
>>> ddc->ddc.dev.parent = >dev;
>>>
>>
>
--
Jani Nikula, Intel
Re: [PATCH 03/20] drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c: remove I2C_CLASS_DDC support
Please just use "drm/amd/display:" as tag in the commit subject. With that fixed, this is Acked-by: Harry Wentland Harry On 2023-11-13 06:23, Heiner Kallweit wrote: > After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in > olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. > Class-based device auto-detection is a legacy mechanism and shouldn't > be used in new code. So we can remove this class completely now. > > Preferably this series should be applied via the i2c tree. > > Signed-off-by: Heiner Kallweit > > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 6f99f6754..ae1edc6ab 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -7529,7 +7529,6 @@ create_i2c(struct ddc_service *ddc_service, > if (!i2c) > return NULL; > i2c->base.owner = THIS_MODULE; > - i2c->base.class = I2C_CLASS_DDC; > i2c->base.dev.parent = >pdev->dev; > i2c->base.algo = _dm_i2c_algo; > snprintf(i2c->base.name, sizeof(i2c->base.name), "AMDGPU DM i2c hw bus > %d", link_index); >
Re: [PATCH 00/20] remove I2C_CLASS_DDC support
> Preferably this series should be applied via the i2c tree. Are we in a hurry here, i.e. does it block further development of the i801 smbus driver? My gut feeling says the patches should rather go via drm and fbdev trees, but I may be convinced otherwise. signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 0/4] drm/i915: Fix LUT rounding
On Fri, 13 Oct 2023, Ville Syrjala wrote: > From: Ville Syrjälä > > The current LUT rounding is generating weird results. Adjust > it to follow the OpenGL int<->float conversion rules. Reviewed-by: Jani Nikula > > Ville Syrjälä (4): > drm: Fix color LUT rounding > drm/i915: Adjust LUT rounding rules > drm/i915: s/clamp()/min()/ in i965_lut_11p6_max_pack() > drm/i915: Fix glk+ degamma LUT conversions > > drivers/gpu/drm/i915/display/intel_color.c | 70 +++--- > include/drm/drm_color_mgmt.h | 18 +++--- > 2 files changed, 42 insertions(+), 46 deletions(-) -- Jani Nikula, Intel
[Bug 218141] fb: trapped write at 0000006000 on channel -1 [3fed0000 unknown] engine 06 [BAR] client 04 [PFIFO_WRITE] subclient 00 [FB] reason 00000002 [PAGE_NOT_PRESENT]
https://bugzilla.kernel.org/show_bug.cgi?id=218141 --- Comment #1 from sander44 (ionut_n2...@yahoo.com) --- Created attachment 305402 --> https://bugzilla.kernel.org/attachment.cgi?id=305402=edit dmesg nouveau -- 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 218141] fb: trapped write at 0000006000 on channel -1 [3fed0000 unknown] engine 06 [BAR] client 04 [PFIFO_WRITE] subclient 00 [FB] reason 00000002 [PAGE_NOT_PRESENT]
https://bugzilla.kernel.org/show_bug.cgi?id=218141 sander44 (ionut_n2...@yahoo.com) changed: What|Removed |Added Kernel Version||6.6.1 -- 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 218141] New: fb: trapped write at 0000006000 on channel -1 [3fed0000 unknown] engine 06 [BAR] client 04 [PFIFO_WRITE] subclient 00 [FB] reason 00000002 [PAGE_NOT_PRESENT]
https://bugzilla.kernel.org/show_bug.cgi?id=218141 Bug ID: 218141 Summary: fb: trapped write at 006000 on channel -1 [3fed unknown] engine 06 [BAR] client 04 [PFIFO_WRITE] subclient 00 [FB] reason 0002 [PAGE_NOT_PRESENT] Product: Drivers Version: 2.5 Hardware: All OS: Linux Status: NEW Severity: normal Priority: P3 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: ionut_n2...@yahoo.com Regression: No Hi Kernel Team, I notice today this: nouveau :65:00.0: fb: trapped write at 006000 on channel -1 [3fed unknown] engine 06 [BAR] client 04 [PFIFO_WRITE] subclient 00 [FB] reason 0002 [PAGE_NOT_PRESENT] Kernel: 6.6.1-vanilla Sporadic nouveau driver have issue with performance. -- 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] driver: gpu: Fixing warning directly dereferencing a rcu pointer
On 11/13/23 09:24, Maarten Lankhorst wrote:
Hey,
Den 2023-11-13 kl. 09:10, skrev Abhinav Singh:
This patch fixes a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
We should not be directly dereferencing a rcu pointer, rather we should
be using rcu helper function rcu_dereferece() inside rcu read critical
section to get a normal pointer which can be dereferenced.
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
Signed-off-by: Abhinav Singh
---
drivers/gpu/drm/nouveau/nv04_fence.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_fence.c
b/drivers/gpu/drm/nouveau/nv04_fence.c
index 5b71a5a5cd85..e62bad1ac720 100644
--- a/drivers/gpu/drm/nouveau/nv04_fence.c
+++ b/drivers/gpu/drm/nouveau/nv04_fence.c
@@ -39,7 +39,9 @@ struct nv04_fence_priv {
static int
nv04_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ rcu_read_lock();
+ struct nvif_push *push = rcu_dereference(fence->channel)->chan.push;
+ rcu_read_unlock();
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_NVSQ(push, NV_SW, 0x0150, fence->base.seqno);
I'm not an expert in nouveau fence channel lifetime, but I'm pretty sure this
should probably be a rcu_dereference_protected, since a fence likely can't lose
its channel before its command to signal is submitted.
Yes, before nouveau_fence_emit() did not add this fence to the fence context's
pending list ->channel doesn't need any protection. We can probably just use
unrcu_pointer(), as in [1].
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/nouveau/nouveau_fence.c#L210
But in case it's not, I would at least advise to check for fence->channel
lifetime before submitting a patch like this. At least the original code warned
about it not being 100% correct.
Cheers,
~Maarten
Re: [PATCH v3 2/2] drm/uapi: add explicit virtgpu context debug name
On Sat, Nov 11, 2023 at 2:37 PM Dmitry Osipenko <
dmitry.osipe...@collabora.com> wrote:
> On 10/18/23 21:17, Gurchetan Singh wrote:
> > + case VIRTGPU_CONTEXT_PARAM_DEBUG_NAME:
> > + if (vfpriv->explicit_debug_name) {
> > + ret = -EINVAL;
> > + goto out_unlock;
> > + }
> > +
> > + ret = strncpy_from_user(vfpriv->debug_name,
> > + u64_to_user_ptr(value),
> > + DEBUG_NAME_MAX_LEN - 1);
> > +
> > + if (ret < 0) {
> > + ret = -EFAULT;
> > + goto out_unlock;
> > + }
> > +
> > + vfpriv->explicit_debug_name = true;
> > + break;
>
> Spotted a problem here. The ret needs to be set to zero on success. I'll
> send the fix shortly. Gurchetan you should've been getting the
> DRM_IOCTL_VIRTGPU_CONTEXT_INIT failure from gfxstream when you tested
> this patch, haven't you?
>
To accommodate older kernels/QEMU, gfxstream doesn't fail if CONTEXT_INIT
fails. So the guest thought it failed and didn't react, but the value was
propagated to the host.
>
> Also noticed that the patch title says "drm/uapi" instead of
> "drm/virtio". My bad for not noticing it earlier. Please be more careful
> next time too :)
>
> --
> Best regards,
> Dmitry
>
>
[PATCH 4/5] accel/ivpu: Use dedicated work for job timeout detection
From: Stanislaw Gruszka
Change to use work for timeout detection. Needed for thread_irq
conversion.
Signed-off-by: Stanislaw Gruszka
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_job.c | 24 +---
drivers/accel/ivpu/ivpu_pm.c | 31 +++
drivers/accel/ivpu/ivpu_pm.h | 3 +++
3 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 77b1b8abadd6..796366d67984 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -24,10 +24,6 @@
#define JOB_ID_CONTEXT_MASK GENMASK(31, 8)
#define JOB_MAX_BUFFER_COUNT 65535
-static unsigned int ivpu_tdr_timeout_ms;
-module_param_named(tdr_timeout_ms, ivpu_tdr_timeout_ms, uint, 0644);
-MODULE_PARM_DESC(tdr_timeout_ms, "Timeout for device hang detection, in
milliseconds, 0 - default");
-
static void ivpu_cmdq_ring_db(struct ivpu_device *vdev, struct ivpu_cmdq *cmdq)
{
ivpu_hw_reg_db_set(vdev, cmdq->db_id);
@@ -342,6 +338,8 @@ static int ivpu_job_done(struct ivpu_device *vdev, u32
job_id, u32 job_status)
ivpu_dbg(vdev, JOB, "Job complete: id %3u ctx %2d engine %d status
0x%x\n",
job->job_id, job->file_priv->ctx.id, job->engine_idx,
job_status);
+ ivpu_stop_job_timeout_detection(vdev);
+
job_put(job);
return 0;
}
@@ -357,6 +355,9 @@ static void ivpu_job_done_message(struct ivpu_device *vdev,
void *msg)
ret = ivpu_job_done(vdev, payload->job_id, payload->job_status);
if (ret)
ivpu_err(vdev, "Failed to finish job %d: %d\n",
payload->job_id, ret);
+
+ if (!ret && !xa_empty(>submitted_jobs_xa))
+ ivpu_start_job_timeout_detection(vdev);
}
void ivpu_jobs_abort_all(struct ivpu_device *vdev)
@@ -400,6 +401,8 @@ static int ivpu_direct_job_submission(struct ivpu_job *job)
if (ret)
goto err_xa_erase;
+ ivpu_start_job_timeout_detection(vdev);
+
ivpu_dbg(vdev, JOB, "Job submitted: id %3u addr 0x%llx ctx %2d engine
%d next %d\n",
job->job_id, job->cmd_buf_vpu_addr, file_priv->ctx.id,
job->engine_idx, cmdq->jobq->header.tail);
@@ -569,7 +572,6 @@ static int ivpu_job_done_thread(void *arg)
struct ivpu_device *vdev = (struct ivpu_device *)arg;
struct ivpu_ipc_consumer cons;
struct vpu_jsm_msg jsm_msg;
- bool jobs_submitted;
unsigned int timeout;
int ret;
@@ -578,18 +580,10 @@ static int ivpu_job_done_thread(void *arg)
ivpu_ipc_consumer_add(vdev, , VPU_IPC_CHAN_JOB_RET);
while (!kthread_should_stop()) {
- timeout = ivpu_tdr_timeout_ms ? ivpu_tdr_timeout_ms :
vdev->timeout.tdr;
- jobs_submitted = !xa_empty(>submitted_jobs_xa);
ret = ivpu_ipc_receive(vdev, , NULL, _msg, timeout);
- if (!ret) {
+ if (!ret)
ivpu_job_done_message(vdev, _msg);
- } else if (ret == -ETIMEDOUT) {
- if (jobs_submitted &&
!xa_empty(>submitted_jobs_xa)) {
- ivpu_err(vdev, "TDR detected, timeout %d ms",
timeout);
- ivpu_hw_diagnose_failure(vdev);
- ivpu_pm_schedule_recovery(vdev);
- }
- }
+
if (kthread_should_park()) {
ivpu_dbg(vdev, JOB, "Parked %s\n", __func__);
kthread_parkme();
diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
index d915f3c2991f..0af8864cb3b5 100644
--- a/drivers/accel/ivpu/ivpu_pm.c
+++ b/drivers/accel/ivpu/ivpu_pm.c
@@ -23,6 +23,10 @@ static bool ivpu_disable_recovery;
module_param_named_unsafe(disable_recovery, ivpu_disable_recovery, bool, 0644);
MODULE_PARM_DESC(disable_recovery, "Disables recovery when VPU hang is
detected");
+static unsigned long ivpu_tdr_timeout_ms;
+module_param_named(tdr_timeout_ms, ivpu_tdr_timeout_ms, ulong, 0644);
+MODULE_PARM_DESC(tdr_timeout_ms, "Timeout for device hang detection, in
milliseconds, 0 - default");
+
#define PM_RESCHEDULE_LIMIT 5
static void ivpu_pm_prepare_cold_boot(struct ivpu_device *vdev)
@@ -141,6 +145,31 @@ void ivpu_pm_schedule_recovery(struct ivpu_device *vdev)
}
}
+static void ivpu_job_timeout_work(struct work_struct *work)
+{
+ struct ivpu_pm_info *pm = container_of(work, struct ivpu_pm_info,
job_timeout_work.work);
+ struct ivpu_device *vdev = pm->vdev;
+ unsigned long timeout_ms = ivpu_tdr_timeout_ms ? ivpu_tdr_timeout_ms :
vdev->timeout.tdr;
+
+ ivpu_err(vdev, "TDR detected, timeout %lu ms", timeout_ms);
+ ivpu_hw_diagnose_failure(vdev);
+
+ ivpu_pm_schedule_recovery(vdev);
+}
+
+void ivpu_start_job_timeout_detection(struct ivpu_device *vdev)
+{
+ unsigned long timeout_ms = ivpu_tdr_timeout_ms ?
[PATCH 5/5] accel/ivpu: Use threaded IRQ to handle JOB done messages
Remove job_done thread and replace it with generic callback based
mechanism.
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_drv.c | 30 +++--
drivers/accel/ivpu/ivpu_drv.h | 3 +-
drivers/accel/ivpu/ivpu_hw_37xx.c | 29 +++--
drivers/accel/ivpu/ivpu_hw_40xx.c | 30 ++---
drivers/accel/ivpu/ivpu_ipc.c | 196 +-
drivers/accel/ivpu/ivpu_ipc.h | 22 +++-
drivers/accel/ivpu/ivpu_job.c | 84 +++--
drivers/accel/ivpu/ivpu_job.h | 6 +-
8 files changed, 199 insertions(+), 201 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index bc15b06e1744..64927682161b 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -318,13 +318,11 @@ static int ivpu_wait_for_ready(struct ivpu_device *vdev)
if (ivpu_test_mode & IVPU_TEST_MODE_FW_TEST)
return 0;
- ivpu_ipc_consumer_add(vdev, , IVPU_IPC_CHAN_BOOT_MSG);
+ ivpu_ipc_consumer_add(vdev, , IVPU_IPC_CHAN_BOOT_MSG, NULL);
timeout = jiffies + msecs_to_jiffies(vdev->timeout.boot);
while (1) {
- ret = ivpu_ipc_irq_handler(vdev);
- if (ret)
- break;
+ ivpu_ipc_irq_handler(vdev, NULL);
ret = ivpu_ipc_receive(vdev, , _hdr, NULL, 0);
if (ret != -ETIMEDOUT || time_after_eq(jiffies, timeout))
break;
@@ -378,7 +376,6 @@ int ivpu_boot(struct ivpu_device *vdev)
enable_irq(vdev->irq);
ivpu_hw_irq_enable(vdev);
ivpu_ipc_enable(vdev);
- ivpu_job_done_thread_enable(vdev);
return 0;
}
@@ -388,7 +385,6 @@ void ivpu_prepare_for_reset(struct ivpu_device *vdev)
disable_irq(vdev->irq);
ivpu_ipc_disable(vdev);
ivpu_mmu_disable(vdev);
- ivpu_job_done_thread_disable(vdev);
}
int ivpu_shutdown(struct ivpu_device *vdev)
@@ -429,6 +425,13 @@ static const struct drm_driver driver = {
.minor = DRM_IVPU_DRIVER_MINOR,
};
+static irqreturn_t ivpu_irq_thread_handler(int irq, void *arg)
+{
+ struct ivpu_device *vdev = arg;
+
+ return ivpu_ipc_irq_thread_handler(vdev);
+}
+
static int ivpu_irq_init(struct ivpu_device *vdev)
{
struct pci_dev *pdev = to_pci_dev(vdev->drm.dev);
@@ -442,8 +445,8 @@ static int ivpu_irq_init(struct ivpu_device *vdev)
vdev->irq = pci_irq_vector(pdev, 0);
- ret = devm_request_irq(vdev->drm.dev, vdev->irq,
vdev->hw->ops->irq_handler,
- IRQF_NO_AUTOEN, DRIVER_NAME, vdev);
+ ret = devm_request_threaded_irq(vdev->drm.dev, vdev->irq,
vdev->hw->ops->irq_handler,
+ ivpu_irq_thread_handler,
IRQF_NO_AUTOEN, DRIVER_NAME, vdev);
if (ret)
ivpu_err(vdev, "Failed to request an IRQ %d\n", ret);
@@ -581,20 +584,15 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
ivpu_pm_init(vdev);
- ret = ivpu_job_done_thread_init(vdev);
- if (ret)
- goto err_ipc_fini;
-
ret = ivpu_boot(vdev);
if (ret)
- goto err_job_done_thread_fini;
+ goto err_ipc_fini;
+ ivpu_job_done_consumer_init(vdev);
ivpu_pm_enable(vdev);
return 0;
-err_job_done_thread_fini:
- ivpu_job_done_thread_fini(vdev);
err_ipc_fini:
ivpu_ipc_fini(vdev);
err_fw_fini:
@@ -619,7 +617,7 @@ static void ivpu_dev_fini(struct ivpu_device *vdev)
ivpu_shutdown(vdev);
if (IVPU_WA(d3hot_after_power_off))
pci_set_power_state(to_pci_dev(vdev->drm.dev), PCI_D3hot);
- ivpu_job_done_thread_fini(vdev);
+ ivpu_job_done_consumer_fini(vdev);
ivpu_pm_cancel_recovery(vdev);
ivpu_ipc_fini(vdev);
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index f1e7072449f1..ebc4b84f27b2 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -17,6 +17,7 @@
#include
#include "ivpu_mmu_context.h"
+#include "ivpu_ipc.h"
#define DRIVER_NAME "intel_vpu"
#define DRIVER_DESC "Driver for Intel NPU (Neural Processing Unit)"
@@ -120,7 +121,7 @@ struct ivpu_device {
struct list_head bo_list;
struct xarray submitted_jobs_xa;
- struct task_struct *job_done_thread;
+ struct ivpu_ipc_consumer job_done_consumer;
atomic64_t unique_id_counter;
diff --git a/drivers/accel/ivpu/ivpu_hw_37xx.c
b/drivers/accel/ivpu/ivpu_hw_37xx.c
index a172cfb1c31f..4ab1f14cf360 100644
--- a/drivers/accel/ivpu/ivpu_hw_37xx.c
+++ b/drivers/accel/ivpu/ivpu_hw_37xx.c
@@ -891,17 +891,20 @@ static void ivpu_hw_37xx_irq_noc_firewall_handler(struct
ivpu_device *vdev)
}
/* Handler for IRQs from VPU core (irqV) */
-static u32 ivpu_hw_37xx_irqv_handler(struct ivpu_device *vdev, int irq)
+static bool ivpu_hw_37xx_irqv_handler(struct ivpu_device *vdev, int irq, bool
*wake_thread)
{
u32
[PATCH 3/5] accel/ivpu: Do not use cons->aborted for job_done_thread
From: Stanislaw Gruszka
This allow to simplify ivpu_ipc_receive() as now we do not have
to process all messages in aborted state - they will be freed in
ivpu_ipc_consumer_del().
Signed-off-by: Stanislaw Gruszka
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_ipc.c | 18 +-
drivers/accel/ivpu/ivpu_job.c | 1 -
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
index 781c7e40505a..1dd4413dc88f 100644
--- a/drivers/accel/ivpu/ivpu_ipc.c
+++ b/drivers/accel/ivpu/ivpu_ipc.c
@@ -238,17 +238,16 @@ int ivpu_ipc_receive(struct ivpu_device *vdev, struct
ivpu_ipc_consumer *cons,
return -ETIMEDOUT;
spin_lock_irq(>rx_lock);
+ if (cons->aborted) {
+ spin_unlock_irq(>rx_lock);
+ return -ECANCELED;
+ }
rx_msg = list_first_entry_or_null(>rx_msg_list, struct
ivpu_ipc_rx_msg, link);
if (!rx_msg) {
spin_unlock_irq(>rx_lock);
return -EAGAIN;
}
list_del(_msg->link);
- if (cons->aborted) {
- spin_unlock_irq(>rx_lock);
- ret = -ECANCELED;
- goto out;
- }
spin_unlock_irq(>rx_lock);
if (ipc_buf)
@@ -266,7 +265,6 @@ int ivpu_ipc_receive(struct ivpu_device *vdev, struct
ivpu_ipc_consumer *cons,
}
ivpu_ipc_rx_mark_free(vdev, rx_msg->ipc_hdr, rx_msg->jsm_msg);
-out:
atomic_dec(>rx_msg_count);
kfree(rx_msg);
@@ -528,9 +526,11 @@ void ivpu_ipc_disable(struct ivpu_device *vdev)
spin_lock_irqsave(>cons_list_lock, flags);
list_for_each_entry_safe(cons, c, >cons_list, link) {
- spin_lock(>rx_lock);
- cons->aborted = true;
- spin_unlock(>rx_lock);
+ if (cons->channel != VPU_IPC_CHAN_JOB_RET) {
+ spin_lock(>rx_lock);
+ cons->aborted = true;
+ spin_unlock(>rx_lock);
+ }
wake_up(>rx_msg_wq);
}
spin_unlock_irqrestore(>cons_list_lock, flags);
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 02acd8dba02a..77b1b8abadd6 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -578,7 +578,6 @@ static int ivpu_job_done_thread(void *arg)
ivpu_ipc_consumer_add(vdev, , VPU_IPC_CHAN_JOB_RET);
while (!kthread_should_stop()) {
- cons.aborted = false;
timeout = ivpu_tdr_timeout_ms ? ivpu_tdr_timeout_ms :
vdev->timeout.tdr;
jobs_submitted = !xa_empty(>submitted_jobs_xa);
ret = ivpu_ipc_receive(vdev, , NULL, _msg, timeout);
--
2.42.0
[PATCH 2/5] accel/ivpu: Do not use irqsave in ivpu_ipc_dispatch
From: Stanislaw Gruszka
ivpu_ipc_dispatch is always called with irqs disabled. Add lockdep
assertion and remove unneeded _irqsave/_irqrestore.
Signed-off-by: Stanislaw Gruszka
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_ipc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
index 31ae0e71a8a3..781c7e40505a 100644
--- a/drivers/accel/ivpu/ivpu_ipc.c
+++ b/drivers/accel/ivpu/ivpu_ipc.c
@@ -367,9 +367,9 @@ ivpu_ipc_dispatch(struct ivpu_device *vdev, struct
ivpu_ipc_consumer *cons,
{
struct ivpu_ipc_info *ipc = vdev->ipc;
struct ivpu_ipc_rx_msg *rx_msg;
- unsigned long flags;
lockdep_assert_held(>cons_list_lock);
+ lockdep_assert_irqs_disabled();
rx_msg = kzalloc(sizeof(*rx_msg), GFP_ATOMIC);
if (!rx_msg) {
@@ -382,9 +382,9 @@ ivpu_ipc_dispatch(struct ivpu_device *vdev, struct
ivpu_ipc_consumer *cons,
rx_msg->ipc_hdr = ipc_hdr;
rx_msg->jsm_msg = jsm_msg;
- spin_lock_irqsave(>rx_lock, flags);
+ spin_lock(>rx_lock);
list_add_tail(_msg->link, >rx_msg_list);
- spin_unlock_irqrestore(>rx_lock, flags);
+ spin_unlock(>rx_lock);
wake_up(>rx_msg_wq);
}
--
2.42.0
[PATCH 1/5] accel/ivpu: Rename cons->rx_msg_lock
From: Stanislaw Gruszka
Now the cons->rx_msg_lock protects also 'abort' field so rename the lock.
Signed-off-by: Stanislaw Gruszka
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_ipc.c | 27 +--
drivers/accel/ivpu/ivpu_ipc.h | 2 +-
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
index 88453762c9d5..31ae0e71a8a3 100644
--- a/drivers/accel/ivpu/ivpu_ipc.c
+++ b/drivers/accel/ivpu/ivpu_ipc.c
@@ -150,7 +150,7 @@ ivpu_ipc_consumer_add(struct ivpu_device *vdev, struct
ivpu_ipc_consumer *cons,
cons->tx_vpu_addr = 0;
cons->request_id = 0;
cons->aborted = false;
- spin_lock_init(>rx_msg_lock);
+ spin_lock_init(>rx_lock);
INIT_LIST_HEAD(>rx_msg_list);
init_waitqueue_head(>rx_msg_wq);
@@ -168,7 +168,7 @@ void ivpu_ipc_consumer_del(struct ivpu_device *vdev, struct
ivpu_ipc_consumer *c
list_del(>link);
spin_unlock_irq(>cons_list_lock);
- spin_lock_irq(>rx_msg_lock);
+ spin_lock_irq(>rx_lock);
list_for_each_entry_safe(rx_msg, r, >rx_msg_list, link) {
list_del(_msg->link);
if (!cons->aborted)
@@ -176,7 +176,7 @@ void ivpu_ipc_consumer_del(struct ivpu_device *vdev, struct
ivpu_ipc_consumer *c
atomic_dec(>rx_msg_count);
kfree(rx_msg);
}
- spin_unlock_irq(>rx_msg_lock);
+ spin_unlock_irq(>rx_lock);
ivpu_ipc_tx_release(vdev, cons->tx_vpu_addr);
}
@@ -212,9 +212,9 @@ static int ivpu_ipc_rx_need_wakeup(struct ivpu_ipc_consumer
*cons)
if (IS_KTHREAD())
ret |= (kthread_should_stop() || kthread_should_park());
- spin_lock_irq(>rx_msg_lock);
+ spin_lock_irq(>rx_lock);
ret |= !list_empty(>rx_msg_list) || cons->aborted;
- spin_unlock_irq(>rx_msg_lock);
+ spin_unlock_irq(>rx_lock);
return ret;
}
@@ -237,20 +237,19 @@ int ivpu_ipc_receive(struct ivpu_device *vdev, struct
ivpu_ipc_consumer *cons,
if (wait_ret == 0)
return -ETIMEDOUT;
- spin_lock_irq(>rx_msg_lock);
+ spin_lock_irq(>rx_lock);
rx_msg = list_first_entry_or_null(>rx_msg_list, struct
ivpu_ipc_rx_msg, link);
if (!rx_msg) {
- spin_unlock_irq(>rx_msg_lock);
+ spin_unlock_irq(>rx_lock);
return -EAGAIN;
}
list_del(_msg->link);
if (cons->aborted) {
- spin_unlock_irq(>rx_msg_lock);
+ spin_unlock_irq(>rx_lock);
ret = -ECANCELED;
goto out;
}
-
- spin_unlock_irq(>rx_msg_lock);
+ spin_unlock_irq(>rx_lock);
if (ipc_buf)
memcpy(ipc_buf, rx_msg->ipc_hdr, sizeof(*ipc_buf));
@@ -383,9 +382,9 @@ ivpu_ipc_dispatch(struct ivpu_device *vdev, struct
ivpu_ipc_consumer *cons,
rx_msg->ipc_hdr = ipc_hdr;
rx_msg->jsm_msg = jsm_msg;
- spin_lock_irqsave(>rx_msg_lock, flags);
+ spin_lock_irqsave(>rx_lock, flags);
list_add_tail(_msg->link, >rx_msg_list);
- spin_unlock_irqrestore(>rx_msg_lock, flags);
+ spin_unlock_irqrestore(>rx_lock, flags);
wake_up(>rx_msg_wq);
}
@@ -529,9 +528,9 @@ void ivpu_ipc_disable(struct ivpu_device *vdev)
spin_lock_irqsave(>cons_list_lock, flags);
list_for_each_entry_safe(cons, c, >cons_list, link) {
- spin_lock(>rx_msg_lock);
+ spin_lock(>rx_lock);
cons->aborted = true;
- spin_unlock(>rx_msg_lock);
+ spin_unlock(>rx_lock);
wake_up(>rx_msg_wq);
}
spin_unlock_irqrestore(>cons_list_lock, flags);
diff --git a/drivers/accel/ivpu/ivpu_ipc.h b/drivers/accel/ivpu/ivpu_ipc.h
index a380787f7222..71b2e648dffd 100644
--- a/drivers/accel/ivpu/ivpu_ipc.h
+++ b/drivers/accel/ivpu/ivpu_ipc.h
@@ -49,7 +49,7 @@ struct ivpu_ipc_consumer {
u32 request_id;
bool aborted;
- spinlock_t rx_msg_lock; /* Protects rx_msg_list and aborted */
+ spinlock_t rx_lock; /* Protects rx_msg_list and aborted */
struct list_head rx_msg_list;
wait_queue_head_t rx_msg_wq;
};
--
2.42.0
[PATCH v2 0/5] accel/ivpu: Replace IPC kthread with threaded IRQ
Use threaded IRQ to handle incoming IPC messages. IPC consumers can now provide optional callback that will be executed once message is received. This allows to handle multiple message types in a generic manner. Removing kthread also simplifies synchronization as disable_irq() will block until all pending messages are handled. v2: - Don't do bit operations on enum irqreturn v1: https://lore.kernel.org/all/20231107123514.2218850-1-jacek.lawrynow...@linux.intel.com Jacek Lawrynowicz (1): accel/ivpu: Use threaded IRQ to handle JOB done messages Stanislaw Gruszka (4): accel/ivpu: Rename cons->rx_msg_lock accel/ivpu: Do not use irqsave in ivpu_ipc_dispatch accel/ivpu: Do not use cons->aborted for job_done_thread accel/ivpu: Use dedicated work for job timeout detection drivers/accel/ivpu/ivpu_drv.c | 30 ++-- drivers/accel/ivpu/ivpu_drv.h | 3 +- drivers/accel/ivpu/ivpu_hw_37xx.c | 29 ++-- drivers/accel/ivpu/ivpu_hw_40xx.c | 30 ++-- drivers/accel/ivpu/ivpu_ipc.c | 223 +- drivers/accel/ivpu/ivpu_ipc.h | 24 +++- drivers/accel/ivpu/ivpu_job.c | 99 +++-- drivers/accel/ivpu/ivpu_job.h | 6 +- drivers/accel/ivpu/ivpu_pm.c | 31 + drivers/accel/ivpu/ivpu_pm.h | 3 + 10 files changed, 251 insertions(+), 227 deletions(-) -- 2.42.0
Re: [PATCH v1] drm/virtio: Fix return value for VIRTGPU_CONTEXT_PARAM_DEBUG_NAME
On Sat, Nov 11, 2023 at 2:43 PM Dmitry Osipenko <
dmitry.osipe...@collabora.com> wrote:
> The strncpy_from_user() returns number of copied bytes and not zero on
> success. The non-zero return value of ioctl is treated as error. Return
> zero on success instead of the number of copied bytes.
>
> Fixes: 7add80126bce ("drm/uapi: add explicit virtgpu context debug name")
> Signed-off-by: Dmitry Osipenko
>
Reviewed-by: Gurchetan Singh
> ---
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 1e2042419f95..e4f76f315550 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -665,6 +665,7 @@ static int virtio_gpu_context_init_ioctl(struct
> drm_device *dev,
> goto out_unlock;
>
> vfpriv->explicit_debug_name = true;
> + ret = 0;
> break;
> default:
> ret = -EINVAL;
> --
> 2.41.0
>
>
[PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
The FORCE_STOP_STATE bit is unsuitable to force the DSI link into LP-11
mode. It seems the bridge internally queues DSI packets and when the
FORCE_STOP_STATE bit is cleared, they are sent in close succession
without any useful timing (this also means that the DSI lanes won't go
into LP-11 mode). The length of this gibberish varies between 1ms and
5ms. This sometimes breaks an attached bridge (TI SN65DSI84 in this
case). In our case, the bridge will fail in about 1 per 500 reboots.
The FORCE_STOP_STATE handling was introduced to have the DSI lanes in
LP-11 state during the .pre_enable phase. But as it turns out, none of
this is needed at all. Between samsung_dsim_init() and
samsung_dsim_set_display_enable() the lanes are already in LP-11 mode.
The code as it was before commit 20c827683de0 ("drm: bridge:
samsung-dsim: Fix init during host transfer") and 0c14d3130654 ("drm:
bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec") was correct
in this regard.
This patch basically reverts both commits. It was tested on an i.MX8M
SoC with an SN65DSI84 bridge. The signals were probed and the DSI
packets were decoded during initialization and link start-up. After this
patch the first DSI packet on the link is a VSYNC packet and the timing
is correct.
Command mode between .pre_enable and .enable was also briefly tested by
a quick hack. There was no DSI link partner which would have responded,
but it was made sure the DSI packet was send on the link. As a side
note, the command mode seems to just work in HS mode. I couldn't find
that the bridge will handle commands in LP mode.
Fixes: 20c827683de0 ("drm: bridge: samsung-dsim: Fix init during host transfer")
Fixes: 0c14d3130654 ("drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet
spec")
Signed-off-by: Michael Walle
---
Let me know wether this should be two commits each reverting one, but both
commits appeared first in kernel 6.5.
drivers/gpu/drm/bridge/samsung-dsim.c | 32 ++-
1 file changed, 2 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index cf777bdb25d2..4233a50baac7 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -939,10 +939,6 @@ static int samsung_dsim_init_link(struct samsung_dsim *dsi)
reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
reg &= ~DSIM_STOP_STATE_CNT_MASK;
reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
-
- if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
- reg |= DSIM_FORCE_STOP_STATE;
-
samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
@@ -1387,18 +1383,6 @@ static void samsung_dsim_disable_irq(struct samsung_dsim
*dsi)
disable_irq(dsi->irq);
}
-static void samsung_dsim_set_stop_state(struct samsung_dsim *dsi, bool enable)
-{
- u32 reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
-
- if (enable)
- reg |= DSIM_FORCE_STOP_STATE;
- else
- reg &= ~DSIM_FORCE_STOP_STATE;
-
- samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
-}
-
static int samsung_dsim_init(struct samsung_dsim *dsi)
{
const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
@@ -1448,9 +1432,6 @@ static void samsung_dsim_atomic_pre_enable(struct
drm_bridge *bridge,
ret = samsung_dsim_init(dsi);
if (ret)
return;
-
- samsung_dsim_set_display_mode(dsi);
- samsung_dsim_set_display_enable(dsi, true);
}
}
@@ -1459,12 +1440,8 @@ static void samsung_dsim_atomic_enable(struct drm_bridge
*bridge,
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
- if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
- samsung_dsim_set_display_mode(dsi);
- samsung_dsim_set_display_enable(dsi, true);
- } else {
- samsung_dsim_set_stop_state(dsi, false);
- }
+ samsung_dsim_set_display_mode(dsi);
+ samsung_dsim_set_display_enable(dsi, true);
dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
}
@@ -1477,9 +1454,6 @@ static void samsung_dsim_atomic_disable(struct drm_bridge
*bridge,
if (!(dsi->state & DSIM_STATE_ENABLED))
return;
- if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
- samsung_dsim_set_stop_state(dsi, true);
-
dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
}
@@ -1781,8 +1755,6 @@ static ssize_t samsung_dsim_host_transfer(struct
mipi_dsi_host *host,
if (ret)
return ret;
- samsung_dsim_set_stop_state(dsi, false);
-
ret = mipi_dsi_create_packet(, msg);
if (ret < 0)
return ret;
--
2.39.2
Re: Incomplete stable drm/ast backport - screen freeze on boot
On Mon, Nov 13, 2023 at 10:49:01AM +0100, Thomas Zimmermann wrote:
(cc: gregkh)
Hi Jocelyn
Am 13.11.23 um 10:36 schrieb Jocelyn Falempe:
On 13/11/2023 09:34, Keno Fischer wrote:
Greetings,
When connected to a remote machine via the BMC KVM functionality,
I am experiencing screen freezes on boot when using 6.5 stable,
but not master.
The BMC on the machine in question is an ASpeed AST2600.
A quick bisect shows the problematic commit to be 2fb9667
("drm/ast: report connection status on Display Port.").
This is commit f81bb0ac upstream.
I believe the problem is that the previous commit in the series
e329cb5 ("drm/ast: Add BMC virtual connector")
was not backported to the stable branch.
As a consequence, it appears that the more accurate DP state detection
is causing the kernel to believe that no display is connected,
even when the BMC's virtual display is in fact in use.
A cherry-pick of e329cb5 onto the stable branch resolves the issue.
Yes, you're right this two patches must be backported together.
I'm sorry I didn't pay enough attention, that only one of the two
was picked up for the stable branch.
Is it possible to backport e329cb5 to the stable branch, or should I
push it to drm-misc-fixes ?
I think stable, which is in cc, will pick up commit e329cb5
semi-automatically now. Otherwise, maybe ping gregkh in a few days
about it.
I thikn it would be more appropriate to revert 2fb9667, as e329cb5
doesn't look like -stable material. I'll go ahead and do that.
--
Thanks,
Sasha
Re: [Patch v3] drm/ttm: Schedule delayed_delete worker closer
Am 11.11.23 um 14:08 schrieb Rajneesh Bhardwaj:
Try to allocate system memory on the NUMA node the device is closest to
and try to run delayed_delete workers on a CPU of this node as well.
To optimize the memory clearing operation when a TTM BO gets freed by
the delayed_delete worker, scheduling it closer to a NUMA node where the
memory was initially allocated helps avoid the cases where the worker
gets randomly scheduled on the CPU cores that are across interconnect
boundaries such as xGMI, PCIe etc.
This change helps USWC GTT allocations on NUMA systems (dGPU) and AMD
APU platforms such as GFXIP9.4.3.
Acked-by: Felix Kuehling
Signed-off-by: Rajneesh Bhardwaj
Reviewed-by: Christian König
---
Changes in v3:
* Use WQ_UNBOUND to address the warning reported by CI pipeline.
drivers/gpu/drm/ttm/ttm_bo.c | 8 +++-
drivers/gpu/drm/ttm/ttm_device.c | 6 --
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 5757b9415e37..6f28a77a565b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -370,7 +370,13 @@ static void ttm_bo_release(struct kref *kref)
spin_unlock(>bdev->lru_lock);
INIT_WORK(>delayed_delete, ttm_bo_delayed_delete);
- queue_work(bdev->wq, >delayed_delete);
+
+ /* Schedule the worker on the closest NUMA node. This
+* improves performance since system memory might be
+* cleared on free and that is best done on a CPU core
+* close to it.
+*/
+ queue_work_node(bdev->pool.nid, bdev->wq,
>delayed_delete);
return;
}
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 43e27ab77f95..bc97e3dd40f0 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -204,7 +204,8 @@ int ttm_device_init(struct ttm_device *bdev, struct
ttm_device_funcs *funcs,
if (ret)
return ret;
- bdev->wq = alloc_workqueue("ttm", WQ_MEM_RECLAIM | WQ_HIGHPRI, 16);
+ bdev->wq = alloc_workqueue("ttm",
+ WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_UNBOUND,
16);
if (!bdev->wq) {
ttm_global_release();
return -ENOMEM;
@@ -213,7 +214,8 @@ int ttm_device_init(struct ttm_device *bdev, struct
ttm_device_funcs *funcs,
bdev->funcs = funcs;
ttm_sys_man_init(bdev);
- ttm_pool_init(>pool, dev, NUMA_NO_NODE, use_dma_alloc, use_dma32);
+
+ ttm_pool_init(>pool, dev, dev_to_node(dev), use_dma_alloc,
use_dma32);
bdev->vma_manager = vma_manager;
spin_lock_init(>lru_lock);
Re: [PATCH v2] drm/amd/display: add a debugfs interface for the DMUB trace mask
On 11/13/2023 9:56 AM, Hamza Mahfooz wrote:
For features that are implemented primarily in DMUB (e.g. PSR), it is
useful to be able to trace them at a DMUB level from the kernel,
especially when debugging issues. So, introduce a debugfs interface that
is able to read and set the DMUB trace mask dynamically at runtime and
document how to use it.
Cc: Alex Deucher
Cc: Mario Limonciello
Signed-off-by: Hamza Mahfooz
---
v2: only return -ETIMEDOUT for DMUB_STATUS_TIMEOUT
---
Documentation/gpu/amdgpu/display/dc-debug.rst | 41
.../gpu/amdgpu/display/trace-groups-table.csv | 29 ++
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 97 +++
.../gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 40 +++-
4 files changed, 205 insertions(+), 2 deletions(-)
create mode 100644 Documentation/gpu/amdgpu/display/trace-groups-table.csv
diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst
b/Documentation/gpu/amdgpu/display/dc-debug.rst
index 40c55a618918..817631b1dbf3 100644
--- a/Documentation/gpu/amdgpu/display/dc-debug.rst
+++ b/Documentation/gpu/amdgpu/display/dc-debug.rst
@@ -75,3 +75,44 @@ change in real-time by using something like::
When reporting a bug related to DC, consider attaching this log before and
after you reproduce the bug.
+
+DMUB Firmware Debug
+===
+
+Sometimes, dmesg logs aren't enough. This is especially true if a feature is
+implemented primarily in DMUB firmware. In such cases, all we see in dmesg when
+an issue arises is some generic timeout error. So, to get more relevant
+information, we can trace DMUB commands by enabling the relevant bits in
+`amdgpu_dm_dmub_trace_mask`.
+
+Currently, we support the tracing of the following groups:
+
+Trace Groups
+
+
+.. csv-table::
+ :header-rows: 1
+ :widths: 1, 1
+ :file: ./trace-groups-table.csv
+
+**Note: Not all ASICs support all of the listed trace groups**
+
+So, to enable just PSR tracing you can use the following command::
+
+ # echo 0x8020 > /sys/kernel/debug/dri/0/amdgpu_dm_dmub_trace_mask
+
+Then, you need to enable logging trace events to the buffer, which you can do
+using the following::
+
+ # echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+
+Lastly, after you are able to reproduce the issue you are trying to debug,
+you can disable tracing and read the trace log by using the following::
+
+ # echo 0 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+ # cat /sys/kernel/debug/dri/0/amdgpu_dm_dmub_tracebuffer
+
+So, when reporting bugs related to features such as PSR and ABM, consider
+enabling the relevant bits in the mask before reproducing the issue and
+attach the log that you obtain from the trace buffer in any bug reports that
you
+create.
diff --git a/Documentation/gpu/amdgpu/display/trace-groups-table.csv
b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
new file mode 100644
index ..3f6a50d1d883
--- /dev/null
+++ b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
@@ -0,0 +1,29 @@
+Name, Mask Value
+INFO, 0x1
+IRQ SVC, 0x2
+VBIOS, 0x4
+REGISTER, 0x8
+PHY DBG, 0x10
+PSR, 0x20
+AUX, 0x40
+SMU, 0x80
+MALL, 0x100
+ABM, 0x200
+ALPM, 0x400
+TIMER, 0x800
+HW LOCK MGR, 0x1000
+INBOX1, 0x2000
+PHY SEQ, 0x4000
+PSR STATE, 0x8000
+ZSTATE, 0x1
+TRANSMITTER CTL, 0x2
+PANEL CNTL, 0x4
+FAMS, 0x8
+DPIA, 0x10
+SUBVP, 0x20
+INBOX0, 0x40
+SDP, 0x400
+REPLAY, 0x800
+REPLAY RESIDENCY, 0x2000
+CURSOR INFO, 0x8000
+IPS, 0x1
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 45c972f2630d..67dea56cf583 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -2971,6 +2971,100 @@ static int allow_edp_hotplug_detection_set(void *data,
u64 val)
return 0;
}
+static int dmub_trace_mask_set(void *data, u64 val)
+{
+ struct amdgpu_device *adev = data;
+ struct dmub_srv *srv = adev->dm.dc->ctx->dmub_srv->dmub;
+ enum dmub_gpint_command cmd;
+ enum dmub_status status;
+ u64 mask = 0x;
+ u8 shift = 0;
+ u32 res;
+ int i;
+
+ if (!srv->fw_version)
+ return -EINVAL;
+
+ for (i = 0; i < 4; i++) {
+ res = (val & mask) >> shift;
+
+ switch (i) {
+ case 0:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD0;
+ break;
+ case 1:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD1;
+ break;
+ case 2:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD2;
+ break;
+ case 3:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD3;
+ break;
+ }
+
+ status =
Re: [RFC PATCH v2 1/1] drm/virtio: new fence for every plane update
On 10/23/23 20:31, Kim, Dongwon wrote: ... >> Please write a guide how to test it. Are you using spice for the >> multi-display >> viewer? > > [DW] Yeah, let me come up with a simple test case. So we use virtio-gpu as > KMS device. It is used to share the guest frame with QEMU. > SPICE is one of client solutions we use but we primarily use QEMU GTK-UI w/ > multi displays (QEMU with this params '-device > virtio-vga,max_outputs=2,blob=true'). > Thanks! I'm having trouble wit h reproducing the issue. For GTK-UI you should be using virtio-vga-gl, shouldn't you? I assume you meant that your simple case is reproducible using dGPU, correct? I need a test case that is reproducing using virgl and no dGPU. Using virtio-vga-gl+virgl+max_outputs=2 I don't see any issues. In the same time switching to the second virtual display doesn't work with Qemu's GTK UI, I'm getting black screen for the second display. In the KDE settings I have two displays and it says both are active. For the first display that works, I don't see wrong frames ordering. Please give me a test case that is reproducing using latest version of vanilla/upstream Qemu. -- Best regards, Dmitry
Re: [PATCH v2] drm/amd/display: add a debugfs interface for the DMUB trace mask
Am 13.11.23 um 15:56 schrieb Hamza Mahfooz:
For features that are implemented primarily in DMUB (e.g. PSR), it is
useful to be able to trace them at a DMUB level from the kernel,
especially when debugging issues. So, introduce a debugfs interface that
is able to read and set the DMUB trace mask dynamically at runtime and
document how to use it.
Cc: Alex Deucher
Cc: Mario Limonciello
Signed-off-by: Hamza Mahfooz
Acked-by: Christian König
---
v2: only return -ETIMEDOUT for DMUB_STATUS_TIMEOUT
---
Documentation/gpu/amdgpu/display/dc-debug.rst | 41
.../gpu/amdgpu/display/trace-groups-table.csv | 29 ++
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 97 +++
.../gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 40 +++-
4 files changed, 205 insertions(+), 2 deletions(-)
create mode 100644 Documentation/gpu/amdgpu/display/trace-groups-table.csv
diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst
b/Documentation/gpu/amdgpu/display/dc-debug.rst
index 40c55a618918..817631b1dbf3 100644
--- a/Documentation/gpu/amdgpu/display/dc-debug.rst
+++ b/Documentation/gpu/amdgpu/display/dc-debug.rst
@@ -75,3 +75,44 @@ change in real-time by using something like::
When reporting a bug related to DC, consider attaching this log before and
after you reproduce the bug.
+
+DMUB Firmware Debug
+===
+
+Sometimes, dmesg logs aren't enough. This is especially true if a feature is
+implemented primarily in DMUB firmware. In such cases, all we see in dmesg when
+an issue arises is some generic timeout error. So, to get more relevant
+information, we can trace DMUB commands by enabling the relevant bits in
+`amdgpu_dm_dmub_trace_mask`.
+
+Currently, we support the tracing of the following groups:
+
+Trace Groups
+
+
+.. csv-table::
+ :header-rows: 1
+ :widths: 1, 1
+ :file: ./trace-groups-table.csv
+
+**Note: Not all ASICs support all of the listed trace groups**
+
+So, to enable just PSR tracing you can use the following command::
+
+ # echo 0x8020 > /sys/kernel/debug/dri/0/amdgpu_dm_dmub_trace_mask
+
+Then, you need to enable logging trace events to the buffer, which you can do
+using the following::
+
+ # echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+
+Lastly, after you are able to reproduce the issue you are trying to debug,
+you can disable tracing and read the trace log by using the following::
+
+ # echo 0 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+ # cat /sys/kernel/debug/dri/0/amdgpu_dm_dmub_tracebuffer
+
+So, when reporting bugs related to features such as PSR and ABM, consider
+enabling the relevant bits in the mask before reproducing the issue and
+attach the log that you obtain from the trace buffer in any bug reports that
you
+create.
diff --git a/Documentation/gpu/amdgpu/display/trace-groups-table.csv
b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
new file mode 100644
index ..3f6a50d1d883
--- /dev/null
+++ b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
@@ -0,0 +1,29 @@
+Name, Mask Value
+INFO, 0x1
+IRQ SVC, 0x2
+VBIOS, 0x4
+REGISTER, 0x8
+PHY DBG, 0x10
+PSR, 0x20
+AUX, 0x40
+SMU, 0x80
+MALL, 0x100
+ABM, 0x200
+ALPM, 0x400
+TIMER, 0x800
+HW LOCK MGR, 0x1000
+INBOX1, 0x2000
+PHY SEQ, 0x4000
+PSR STATE, 0x8000
+ZSTATE, 0x1
+TRANSMITTER CTL, 0x2
+PANEL CNTL, 0x4
+FAMS, 0x8
+DPIA, 0x10
+SUBVP, 0x20
+INBOX0, 0x40
+SDP, 0x400
+REPLAY, 0x800
+REPLAY RESIDENCY, 0x2000
+CURSOR INFO, 0x8000
+IPS, 0x1
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 45c972f2630d..67dea56cf583 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -2971,6 +2971,100 @@ static int allow_edp_hotplug_detection_set(void *data,
u64 val)
return 0;
}
+static int dmub_trace_mask_set(void *data, u64 val)
+{
+ struct amdgpu_device *adev = data;
+ struct dmub_srv *srv = adev->dm.dc->ctx->dmub_srv->dmub;
+ enum dmub_gpint_command cmd;
+ enum dmub_status status;
+ u64 mask = 0x;
+ u8 shift = 0;
+ u32 res;
+ int i;
+
+ if (!srv->fw_version)
+ return -EINVAL;
+
+ for (i = 0; i < 4; i++) {
+ res = (val & mask) >> shift;
+
+ switch (i) {
+ case 0:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD0;
+ break;
+ case 1:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD1;
+ break;
+ case 2:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD2;
+ break;
+ case 3:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD3;
+ break;
+ }
+
+
Re: [PATCH v2] drm/loongson: Add platform dependency
Hi, thanks for the patch. On 2023/11/13 19:55, Jean Delvare wrote: Only offer the Loongson DRM driver as an option on platforms where it makes sense. Signed-off-by: Jean Delvare Reviewed-by: Sui Jingfeng Cc: Sui Jingfeng Cc: David Airlie Cc: Daniel Vetter --- Changes since v1: * Use the architecture dependencies suggested by Sui Jingfeng. drivers/gpu/drm/loongson/Kconfig |1 + 1 file changed, 1 insertion(+) --- linux-6.6.orig/drivers/gpu/drm/loongson/Kconfig +++ linux-6.6/drivers/gpu/drm/loongson/Kconfig @@ -3,6 +3,7 @@ config DRM_LOONGSON tristate "DRM support for Loongson Graphics" depends on DRM && PCI && MMU + depends on LOONGARCH || MIPS || COMPILE_TEST select DRM_KMS_HELPER select DRM_TTM select I2C
Re: [PATCH 00/10] drm/ast: Detect device type before init
On 13/11/2023 09:50, Thomas Zimmermann wrote: Detecting the ast device's chipset type and configuration mode involves several registers, DT properties and possibly POSTing parts of the chip. It is preferable to do this before initializing the DRM driver, so that that each chip type can have an individual setup code. The patchset addresses the problem by moving all early detection code before the allocation of the ast device. Patch one gets a lock out of the way. The lock is only relevant for mode setting. Move it there. Patches 2 and 3 rework the detection of the correct I/O memory ranges. It is now self-contained, more readable and works without an instance of struct ast_device. Patches 4 to 7 rework the setup of various registers that are required for detection. Access helpers for I/O can now operate without an instance of struct ast_device. The setup functions operate on the I/O ranges that have been made available with patch 3, but again without struct ast_device. With the detection's internals done, patches 8 and 9 rework the chip's and config-mode's detection code to operate without struct ast_device as well. Finally, patch 10 moves the detection code into the PCI probe function. it runs before any of the DRM device code. The fucntion for creating an ast device, ast_device_create(), receives the detected I/O memory ranges, chip type and configuration mode. This cleans up the detection code. There is more chip-specific code in other parts of the driver. In a later patch, the ast device setup can be split up so that each chip type gets its own code path that does not interfere with other chips. Tested on AST1100 and AST2100. Thomas Zimmermann (10): drm/ast: Turn ioregs_lock to modeset_lock drm/ast: Rework I/O register setup drm/ast: Retrieve I/O-memory ranges without ast device drm/ast: Add I/O helpers without ast device drm/ast: Enable VGA without ast device instance drm/ast: Enable MMIO without ast device instance drm/ast: Partially implement POST without ast device instance drm/ast: Add enum ast_config_mode drm/ast: Detect ast device type and config mode without ast device drm/ast: Move detection code into PCI probe helper drivers/gpu/drm/ast/ast_drv.c | 261 - drivers/gpu/drm/ast/ast_drv.h | 101 + drivers/gpu/drm/ast/ast_main.c | 244 ++ drivers/gpu/drm/ast/ast_mode.c | 26 ++-- drivers/gpu/drm/ast/ast_post.c | 73 + drivers/gpu/drm/ast/ast_reg.h | 12 +- 6 files changed, 411 insertions(+), 306 deletions(-) base-commit: b7816c393496dc4497c1327310821407f7171d8b prerequisite-patch-id: 0aa359f6144c4015c140c8a6750be19099c676fb prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 prerequisite-patch-id: cbc453ee02fae02af22fbfdce56ab732c7a88c36 I've reviewed the whole series, and I have only a minor comment on patch 9. That's a good thing to move the chip detection to its own functions, and will allow further refactoring later. For the whole series: Reviewed-by: Jocelyn Falempe -- Jocelyn
RE: [PATCH v2 1/1] dt-bindings: backlight: mp3309c: remove two required properties
Hi Daniel, > On Wed, Oct 25, 2023 at 05:50:57PM +0200, Flavio Suligoi wrote: > > NOTE: there are no compatibility problems with the previous version, > > since the device driver has not yet been included in any kernel. > > Only this dt-binding yaml file is already included in the > > "for-backlight-next" branch of the "backlight" kernel repository. > > No developer may have used it. > > I'm afraid I got confused by the fragmented MP3309C patches from all the > different patchsets. > > Please can you rebase whatever is left on v6.7-rc1 and send a single patchset > with all pending changes as a single patch set. > No problem, I'll do it! > > Thanks > > Daniel. Regards, Flavio
Re: [PATCH 09/10] drm/ast: Detect ast device type and config mode without ast device
On 13/11/2023 09:50, Thomas Zimmermann wrote:
Return the ast chip and config in the detection function's parameters
instead of storing them directly in the ast device instance.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_main.c | 104 ++---
1 file changed, 57 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f100df8d74f71..331a9a861153b 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -76,25 +76,27 @@ static void ast_open_key(void __iomem *ioregs)
__ast_write8_i(ioregs, AST_IO_VGACRI, 0x80, AST_IO_VGACR80_PASSWORD);
}
-static int ast_device_config_init(struct ast_device *ast)
+static int ast_detect_chip(struct pci_dev *pdev,
+ void __iomem *regs, void __iomem *ioregs,
+ enum ast_chip *chip_out,
+ enum ast_config_mode *config_mode_out)
{
- struct drm_device *dev = >base;
- struct pci_dev *pdev = to_pci_dev(dev->dev);
- struct device_node *np = dev->dev->of_node;
+ struct device *dev = >dev;
+ struct device_node *np = dev->of_node;
+ enum ast_config_mode config_mode = ast_use_defaults;
uint32_t scu_rev = 0x;
+ enum ast_chip chip;
u32 data;
- u8 jregd0, jregd1;
+ u8 vgacrd0, vgacrd1;
/*
* Find configuration mode and read SCU revision
*/
- ast->config_mode = ast_use_defaults;
-
/* Check if we have device-tree properties */
if (np && !of_property_read_u32(np, "aspeed,scu-revision-id", )) {
/* We do, disable P2A access */
- ast->config_mode = ast_use_dt;
+ config_mode = ast_use_dt;
scu_rev = data;
} else if (pdev->device == PCI_CHIP_AST2000) { // Not all families have
a P2A bridge
/*
@@ -102,9 +104,9 @@ static int ast_device_config_init(struct ast_device *ast)
* is disabled. We force using P2A if VGA only mode bit
* is set D[7]
*/
- jregd0 = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
- jregd1 = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, 0xff);
- if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+ vgacrd0 = __ast_read8_i(ioregs, AST_IO_VGACRI, 0xd0);
+ vgacrd1 = __ast_read8_i(ioregs, AST_IO_VGACRI, 0xd1);
+ if (!(vgacrd0 & 0x80) || !(vgacrd1 & 0x10)) {
/*
* We have a P2A bridge and it is enabled.
@@ -112,32 +114,32 @@ static int ast_device_config_init(struct ast_device *ast)
/* Patch AST2500/AST2510 */
if ((pdev->revision & 0xf0) == 0x40) {
- if (!(jregd0 & AST_VRAM_INIT_STATUS_MASK))
- ast_patch_ahb_2500(ast->regs);
+ if (!(vgacrd0 & AST_VRAM_INIT_STATUS_MASK))
+ ast_patch_ahb_2500(regs);
}
/* Double check that it's actually working */
- data = ast_read32(ast, 0xf004);
+ data = __ast_read32(regs, 0xf004);
if ((data != 0x) && (data != 0x00)) {
- ast->config_mode = ast_use_p2a;
+ config_mode = ast_use_p2a;
/* Read SCU7c (silicon revision register) */
- ast_write32(ast, 0xf004, 0x1e6e);
- ast_write32(ast, 0xf000, 0x1);
- scu_rev = ast_read32(ast, 0x1207c);
+ __ast_write32(regs, 0xf004, 0x1e6e);
+ __ast_write32(regs, 0xf000, 0x1);
+ scu_rev = __ast_read32(regs, 0x1207c);
}
}
}
- switch (ast->config_mode) {
+ switch (config_mode) {
case ast_use_defaults:
- drm_info(dev, "Using default configuration\n");
+ dev_info(dev, "Using default configuration\n");
break;
case ast_use_dt:
- drm_info(dev, "Using device-tree for configuration\n");
+ dev_info(dev, "Using device-tree for configuration\n");
break;
case ast_use_p2a:
- drm_info(dev, "Using P2A bridge for configuration\n");
+ dev_info(dev, "Using P2A bridge for configuration\n");
break;
}
@@ -146,63 +148,66 @@ static int ast_device_config_init(struct ast_device *ast)
*/
if (pdev->revision >= 0x50) {
- ast->chip = AST2600;
- drm_info(dev, "AST 2600 detected\n");
+ chip = AST2600;
+ dev_info(dev, "AST 2600 detected\n");
Adding a
[PATCH v2] drm/amd/display: add a debugfs interface for the DMUB trace mask
For features that are implemented primarily in DMUB (e.g. PSR), it is
useful to be able to trace them at a DMUB level from the kernel,
especially when debugging issues. So, introduce a debugfs interface that
is able to read and set the DMUB trace mask dynamically at runtime and
document how to use it.
Cc: Alex Deucher
Cc: Mario Limonciello
Signed-off-by: Hamza Mahfooz
---
v2: only return -ETIMEDOUT for DMUB_STATUS_TIMEOUT
---
Documentation/gpu/amdgpu/display/dc-debug.rst | 41
.../gpu/amdgpu/display/trace-groups-table.csv | 29 ++
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 97 +++
.../gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 40 +++-
4 files changed, 205 insertions(+), 2 deletions(-)
create mode 100644 Documentation/gpu/amdgpu/display/trace-groups-table.csv
diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst
b/Documentation/gpu/amdgpu/display/dc-debug.rst
index 40c55a618918..817631b1dbf3 100644
--- a/Documentation/gpu/amdgpu/display/dc-debug.rst
+++ b/Documentation/gpu/amdgpu/display/dc-debug.rst
@@ -75,3 +75,44 @@ change in real-time by using something like::
When reporting a bug related to DC, consider attaching this log before and
after you reproduce the bug.
+
+DMUB Firmware Debug
+===
+
+Sometimes, dmesg logs aren't enough. This is especially true if a feature is
+implemented primarily in DMUB firmware. In such cases, all we see in dmesg when
+an issue arises is some generic timeout error. So, to get more relevant
+information, we can trace DMUB commands by enabling the relevant bits in
+`amdgpu_dm_dmub_trace_mask`.
+
+Currently, we support the tracing of the following groups:
+
+Trace Groups
+
+
+.. csv-table::
+ :header-rows: 1
+ :widths: 1, 1
+ :file: ./trace-groups-table.csv
+
+**Note: Not all ASICs support all of the listed trace groups**
+
+So, to enable just PSR tracing you can use the following command::
+
+ # echo 0x8020 > /sys/kernel/debug/dri/0/amdgpu_dm_dmub_trace_mask
+
+Then, you need to enable logging trace events to the buffer, which you can do
+using the following::
+
+ # echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+
+Lastly, after you are able to reproduce the issue you are trying to debug,
+you can disable tracing and read the trace log by using the following::
+
+ # echo 0 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+ # cat /sys/kernel/debug/dri/0/amdgpu_dm_dmub_tracebuffer
+
+So, when reporting bugs related to features such as PSR and ABM, consider
+enabling the relevant bits in the mask before reproducing the issue and
+attach the log that you obtain from the trace buffer in any bug reports that
you
+create.
diff --git a/Documentation/gpu/amdgpu/display/trace-groups-table.csv
b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
new file mode 100644
index ..3f6a50d1d883
--- /dev/null
+++ b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
@@ -0,0 +1,29 @@
+Name, Mask Value
+INFO, 0x1
+IRQ SVC, 0x2
+VBIOS, 0x4
+REGISTER, 0x8
+PHY DBG, 0x10
+PSR, 0x20
+AUX, 0x40
+SMU, 0x80
+MALL, 0x100
+ABM, 0x200
+ALPM, 0x400
+TIMER, 0x800
+HW LOCK MGR, 0x1000
+INBOX1, 0x2000
+PHY SEQ, 0x4000
+PSR STATE, 0x8000
+ZSTATE, 0x1
+TRANSMITTER CTL, 0x2
+PANEL CNTL, 0x4
+FAMS, 0x8
+DPIA, 0x10
+SUBVP, 0x20
+INBOX0, 0x40
+SDP, 0x400
+REPLAY, 0x800
+REPLAY RESIDENCY, 0x2000
+CURSOR INFO, 0x8000
+IPS, 0x1
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 45c972f2630d..67dea56cf583 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -2971,6 +2971,100 @@ static int allow_edp_hotplug_detection_set(void *data,
u64 val)
return 0;
}
+static int dmub_trace_mask_set(void *data, u64 val)
+{
+ struct amdgpu_device *adev = data;
+ struct dmub_srv *srv = adev->dm.dc->ctx->dmub_srv->dmub;
+ enum dmub_gpint_command cmd;
+ enum dmub_status status;
+ u64 mask = 0x;
+ u8 shift = 0;
+ u32 res;
+ int i;
+
+ if (!srv->fw_version)
+ return -EINVAL;
+
+ for (i = 0; i < 4; i++) {
+ res = (val & mask) >> shift;
+
+ switch (i) {
+ case 0:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD0;
+ break;
+ case 1:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD1;
+ break;
+ case 2:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD2;
+ break;
+ case 3:
+ cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD3;
+ break;
+ }
+
+ status = dmub_srv_send_gpint_command(srv, cmd, res, 30);
+
+ if (status
Re: [PATCH v2 1/1] dt-bindings: backlight: mp3309c: remove two required properties
On Wed, Oct 25, 2023 at 05:50:57PM +0200, Flavio Suligoi wrote: > NOTE: there are no compatibility problems with the previous version, > since the device driver has not yet been included in any kernel. > Only this dt-binding yaml file is already included in the > "for-backlight-next" branch of the "backlight" kernel repository. > No developer may have used it. I'm afraid I got confused by the fragmented MP3309C patches from all the different patchsets. Please can you rebase whatever is left on v6.7-rc1 and send a single patchset with all pending changes as a single patch set. Thanks Daniel.
Re: [RFC][PATCH v5 0/6] drm/panic: Add a drm panic handler
On Friday, November 3, 2023 10:53:24 AM EST Jocelyn Falempe wrote: > This introduces a new drm panic handler, which displays a message when a > panic occurs. > So when fbcon is disabled, you can still see a kernel panic. > > This is one of the missing feature, when disabling VT/fbcon in the kernel: > https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/ > Fbcon can be replaced by a userspace kms console, but the panic screen must > be done in the kernel. > > This is a proof of concept, and works with simpledrm and mgag200, using a new > get_scanout_buffer() api > > To test it, make sure you're using the simpledrm driver, and trigger a panic: > echo c > /proc/sysrq-trigger > > v2: > * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) > * Add the panic reason to the panic message (Nerdopolis) > * Add an exclamation mark (Nerdopolis) > > v3: > * Rework the drawing functions, to write the pixels line by line and > to use the drm conversion helper to support other formats. > (Thomas Zimmermann) > > v4: > * Fully support all simpledrm formats using drm conversion helpers > * Rename dpanic_* to drm_panic_*, and have more coherent function name. >(Thomas Zimmermann) > * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) > * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) > * Add foreground/background color config option > * Fix the bottom lines not painted if the framebuffer height >is not a multiple of the font height. > * Automatically register the driver to drm_panic, if the function >get_scanout_buffer() exists. (Thomas Zimmermann) > * Add mgag200 support. > > v5: > * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. >(Thomas Zimmermann) > * Also add drm_fb_fill() to fill area with background color. > * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. > * Add a flush() callback for drivers that needs to synchronize the buffer. > * Add a void *private field, so drivers can pass private data to >draw_pixel_xy() and flush(). > * Add ast support. > * Add experimental imx/ipuv3 support, to test on an ARM hw. (Maxime Ripard) > > > With mgag200 support, I was able to test that the xrgb to rgb565 > conversion is working. > I am unable to test with that hardware, but I am able to test with simpledrm, and it works pretty good > IMX/IPUV3 support is not complete, I wasn't able to have etnaviv working on > my board. > But it shows that it can still work on ARM with DMA buffer in this case. > > Best regards, > > > Jocelyn Falempe (6): > drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill > drm/panic: Add a drm panic handler > drm/simpledrm: Add drm_panic support > drm/mgag200: Add drm_panic support > drm/ast: Add drm_panic support > drm/imx: Add drm_panic support > > drivers/gpu/drm/Kconfig | 22 ++ > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/ast/ast_drv.c| 4 +- > drivers/gpu/drm/ast/ast_drv.h| 3 + > drivers/gpu/drm/ast/ast_mode.c | 26 ++ > drivers/gpu/drm/drm_drv.c| 8 + > drivers/gpu/drm/drm_format_helper.c | 421 ++- > drivers/gpu/drm/drm_panic.c | 368 > drivers/gpu/drm/imx/ipuv3/imx-drm-core.c | 30 ++ > drivers/gpu/drm/mgag200/mgag200_drv.c| 25 ++ > drivers/gpu/drm/tiny/simpledrm.c | 15 + > include/drm/drm_drv.h| 21 ++ > include/drm/drm_format_helper.h | 9 + > include/drm/drm_panic.h | 96 ++ > 14 files changed, 966 insertions(+), 83 deletions(-) > create mode 100644 drivers/gpu/drm/drm_panic.c > create mode 100644 include/drm/drm_panic.h > > > base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa >
Re: [PATCH 3/5] dt-bindings: gpu: samsung: constrain clocks in top-level properties
On Sun, Nov 12, 2023 at 07:44:01PM +0100, Krzysztof Kozlowski wrote: > When number of clock varies between variants, the Devicetree bindings > coding convention expects to have widest constraints in top-level > definition of the properties and narrow them in allOf:if:then block. > > This is more readable and sometimes allows to spot some errors in the > bindings. > > Signed-off-by: Krzysztof Kozlowski Åcked-by: Conor Dooley signature.asc Description: PGP signature
Re: [PATCH 2/5] dt-bindings: gpu: samsung: re-order entries to match coding convention
On Sun, Nov 12, 2023 at 07:44:00PM +0100, Krzysztof Kozlowski wrote:
> The Devicetree bindings coding convention, as used in most of the files
> and expressed in Documentation/devicetree/bindings/example-schema.yaml,
> expects "allOf:" block with if-statements after "required:" block.
>
> Re-order few schemas to match the convention to avoid repeating review
> comments for new patches using existing code as template. No functional
> changes.
>
> Signed-off-by: Krzysztof Kozlowski
Acked-by: Conor Dooley
thanks,
Conor,
> ---
> .../devicetree/bindings/gpu/samsung-g2d.yaml | 53 +
> .../bindings/gpu/samsung-scaler.yaml | 59 +--
> 2 files changed, 56 insertions(+), 56 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpu/samsung-g2d.yaml
> b/Documentation/devicetree/bindings/gpu/samsung-g2d.yaml
> index e7daae862578..b6951acc7643 100644
> --- a/Documentation/devicetree/bindings/gpu/samsung-g2d.yaml
> +++ b/Documentation/devicetree/bindings/gpu/samsung-g2d.yaml
> @@ -27,32 +27,6 @@ properties:
>iommus: {}
>power-domains: {}
>
> -if:
> - properties:
> -compatible:
> - contains:
> -const: samsung,exynos5250-g2d
> -
> -then:
> - properties:
> -clocks:
> - items:
> -- description: fimg2d clock
> -clock-names:
> - items:
> -- const: fimg2d
> -
> -else:
> - properties:
> -clocks:
> - items:
> -- description: sclk_fimg2d clock
> -- description: fimg2d clock
> -clock-names:
> - items:
> -- const: sclk_fimg2d
> -- const: fimg2d
> -
> required:
>- compatible
>- reg
> @@ -60,6 +34,33 @@ required:
>- clocks
>- clock-names
>
> +allOf:
> + - if:
> + properties:
> +compatible:
> + contains:
> +const: samsung,exynos5250-g2d
> +
> +then:
> + properties:
> +clocks:
> + items:
> +- description: fimg2d clock
> +clock-names:
> + items:
> +- const: fimg2d
> +
> +else:
> + properties:
> +clocks:
> + items:
> +- description: sclk_fimg2d clock
> +- description: fimg2d clock
> +clock-names:
> + items:
> +- const: sclk_fimg2d
> +- const: fimg2d
> +
> additionalProperties: false
>
> examples:
> diff --git a/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml
> b/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml
> index 5317ac64426a..97d86a002a90 100644
> --- a/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml
> +++ b/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml
> @@ -26,36 +26,6 @@ properties:
>iommus: {}
>power-domains: {}
>
> -if:
> - properties:
> -compatible:
> - contains:
> -const: samsung,exynos5420-scaler
> -
> -then:
> - properties:
> -clocks:
> - items:
> -- description: mscl clock
> -
> -clock-names:
> - items:
> -- const: mscl
> -
> -else:
> - properties:
> -clocks:
> - items:
> -- description: pclk clock
> -- description: aclk clock
> -- description: aclk_xiu clock
> -
> -clock-names:
> - items:
> -- const: pclk
> -- const: aclk
> -- const: aclk_xiu
> -
> required:
>- compatible
>- reg
> @@ -63,6 +33,35 @@ required:
>- clocks
>- clock-names
>
> +allOf:
> + - if:
> + properties:
> +compatible:
> + contains:
> +const: samsung,exynos5420-scaler
> +
> +then:
> + properties:
> +clocks:
> + items:
> +- description: mscl clock
> +clock-names:
> + items:
> +- const: mscl
> +
> +else:
> + properties:
> +clocks:
> + items:
> +- description: pclk clock
> +- description: aclk clock
> +- description: aclk_xiu clock
> +clock-names:
> + items:
> +- const: pclk
> +- const: aclk
> +- const: aclk_xiu
> +
> additionalProperties: false
>
> examples:
> --
> 2.34.1
>
signature.asc
Description: PGP signature
Re: [PATCH 4/5] dt-bindings: gpu: samsung-g2d: constrain iommus and power-domains
On Sun, Nov 12, 2023 at 07:44:02PM +0100, Krzysztof Kozlowski wrote: > Provide specific constraints for iommus and power-domains, based on > current DTS. > > Signed-off-by: Krzysztof Kozlowski Acked-by: Conor Dooley Thanks, Conor. signature.asc Description: PGP signature
Re: [PATCH 5/5] dt-bindings: gpu: samsung-scaler: constrain iommus and power-domains
On Sun, Nov 12, 2023 at 07:44:03PM +0100, Krzysztof Kozlowski wrote: > Provide specific constraints for iommus and power-domains, based on > current DTS. > > Signed-off-by: Krzysztof Kozlowski Acked-by: Conor Dooley signature.asc Description: PGP signature
Re: [PATCH 1/5] dt-bindings: gpu: samsung-rotator: drop redundant quotes
On Sun, Nov 12, 2023 at 07:43:59PM +0100, Krzysztof Kozlowski wrote: > Compatibles should not use quotes in the bindings. > > Signed-off-by: Krzysztof Kozlowski Acked-by: Conor Dooley Cheers… Conor. > --- > .../devicetree/bindings/gpu/samsung-rotator.yaml | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml > b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml > index d60626ffb28e..18bf44e06e8f 100644 > --- a/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml > +++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml > @@ -12,10 +12,11 @@ maintainers: > properties: >compatible: > enum: > - - "samsung,s5pv210-rotator" > - - "samsung,exynos4210-rotator" > - - "samsung,exynos4212-rotator" > - - "samsung,exynos5250-rotator" > + - samsung,s5pv210-rotator > + - samsung,exynos4210-rotator > + - samsung,exynos4212-rotator > + - samsung,exynos5250-rotator > + >reg: > maxItems: 1 > > -- > 2.34.1 > signature.asc Description: PGP signature
Re: [PATCH v2 6/6] drm/vs: Add hdmi driver
On Mon, 13 Nov 2023 at 14:11, Keith Zhao wrote:
>
>
>
> On 2023/10/26 6:23, Dmitry Baryshkov wrote:
> > On 25/10/2023 13:39, Keith Zhao wrote:
> >> add hdmi driver as encoder and connect
> >>
> >> Signed-off-by: Keith Zhao
> >> ---
> >> drivers/gpu/drm/verisilicon/Kconfig | 8 +-
> >> drivers/gpu/drm/verisilicon/Makefile| 1 +
> >> drivers/gpu/drm/verisilicon/starfive_hdmi.c | 949
> >> drivers/gpu/drm/verisilicon/starfive_hdmi.h | 295 ++
> >> drivers/gpu/drm/verisilicon/vs_drv.c| 5 +
> >> drivers/gpu/drm/verisilicon/vs_drv.h| 4 +
> >> 6 files changed, 1261 insertions(+), 1 deletion(-)
> >> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.c
> >> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.h
> >>
> >> diff --git a/drivers/gpu/drm/verisilicon/Kconfig
> >> b/drivers/gpu/drm/verisilicon/Kconfig
> >> index 3a361f8c8..122c786e3 100644
> >> --- a/drivers/gpu/drm/verisilicon/Kconfig
> >> +++ b/drivers/gpu/drm/verisilicon/Kconfig
> >> @@ -12,4 +12,10 @@ config DRM_VERISILICON
> >> setting and buffer management. It does not
> >> provide 2D or 3D acceleration.
> >> -
> >> +config DRM_VERISILICON_STARFIVE_HDMI
> >> +bool "Starfive HDMI extensions"
> >> +depends on DRM_VERISILICON
> >> +help
> >> + This selects support for StarFive soc specific extensions
> >> + for the Innosilicon HDMI driver. If you want to enable
> >> + HDMI on JH7110 based soc, you should select this option.
> >> diff --git a/drivers/gpu/drm/verisilicon/Makefile
> >> b/drivers/gpu/drm/verisilicon/Makefile
> >> index 1d48016ca..08350f25b 100644
> >> --- a/drivers/gpu/drm/verisilicon/Makefile
> >> +++ b/drivers/gpu/drm/verisilicon/Makefile
> >> @@ -7,5 +7,6 @@ vs_drm-objs := vs_dc_hw.o \
> >> vs_modeset.o \
> >> vs_plane.o
> >> +vs_drm-$(CONFIG_DRM_VERISILICON_STARFIVE_HDMI) += starfive_hdmi.o
> >> obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
> >> diff --git a/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> >> b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> >> new file mode 100644
> >> index 0..d296c4b71
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> >> @@ -0,0 +1,949 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#include "starfive_hdmi.h"
> >> +#include "vs_drv.h"
> >> +#include "vs_crtc.h"
> >> +
> >> +static struct starfive_hdmi *encoder_to_hdmi(struct drm_encoder *encoder)
> >> +{
> >> +return container_of(encoder, struct starfive_hdmi, encoder);
> >> +}
> >> +
> >> +static struct starfive_hdmi *connector_to_hdmi(struct drm_connector
> >> *connector)
> >> +{
> >> +return container_of(connector, struct starfive_hdmi, connector);
> >> +}
> >> +
> >> +struct starfive_hdmi_i2c {
> >> +struct i2c_adapter adap;
> >> +
> >> +u8 ddc_addr;
> >> +u8 segment_addr;
> >> +/* protects the edid data when use i2c cmd to read edid */
> >> +struct mutex lock;
> >> +struct completion cmp;
> >> +};
> >> +
> >> +static const struct pre_pll_config pre_pll_cfg_table[] = {
> >> +{ 25175000, 25175000, 1, 100, 2, 3, 3, 12, 3, 3, 4, 0, 0xf5},
> >> +{ 2520, 2520, 1, 100, 2, 3, 3, 12, 3, 3, 4, 0, 0},
> >> +{ 2700, 2700, 1, 90, 3, 2, 2, 10, 3, 3, 4, 0, 0},
> >
> > Such data structures are usually pretyt limited and hard to handle. Could
> > you please replace it with the runtime calculations of
> >
> >> +{ 27027000, 27027000, 1, 90, 3, 2, 2, 10, 3, 3, 4, 0, 0x170a3d},
> >> +{ 2832, 2832, 1, 28, 2, 1, 1, 3, 0, 3, 4, 0, 0x51eb85},
> >> +{ 3024, 3024, 1, 30, 2, 1, 1, 3, 0, 3, 4, 0, 0x3d70a3},
> >> +{ 3150, 3150, 1, 31, 2, 1, 1, 3, 0, 3, 4, 0, 0x7f},
> >> +{ 3375, 3375, 1, 33, 2, 1, 1, 3, 0, 3, 4, 0, 0xcf},
> >> +{ 3600, 3600, 1, 36, 2, 1, 1, 3, 0, 3, 4, 0, 0},
> >> +{ 4000, 4000, 1, 80, 2, 2, 2, 12, 2, 2, 2, 0, 0},
> >> +{ 4697, 4697, 1, 46, 2, 1, 1, 3, 0, 3, 4, 0, 0xf851eb},
> >> +{ 4950, 4950, 1, 49, 2, 1, 1, 3, 0, 3, 4, 0, 0x7f},
> >> +{ 4900, 4900, 1, 49, 2, 1, 1, 3, 0, 3, 4, 0, 0},
> >> +{ 5000, 5000, 1, 50, 2, 1, 1, 3, 0, 3, 4, 0, 0},
> >> +{ 5400, 5400, 1, 54, 2, 1, 1, 3, 0, 3, 4, 0, 0},
> >> +{ 54054000, 54054000, 1, 54, 2, 1, 1, 3, 0, 3, 4, 0, 0x0dd2f1},
> >> +{ 57284000, 57284000, 1, 57, 2, 1, 1, 3, 0, 3, 4, 0, 0x48b439},
> >> +{ 5823, 5823,
Re: [PATCH] drm/scheduler: improve GPU scheduler documentation
Am 13.11.23 um 14:14 schrieb Danilo Krummrich: Hi Christian, On 11/13/23 13:38, Christian König wrote: Start to improve the scheduler document. Especially document the lifetime of each of the objects as well as the restrictions around DMA-fence handling and userspace compatibility. Thanks a lot for submitting this - it's very much appreciated! Before reviewing in detail, do you mind to re-structure this a little bit? Not the slightest. I'm not a native speaker of English and generally not very good at writing documentation. Instead of packing everything in an enumeration I'd suggest to have separate DOC paragraphs. For instance: - keep "Overview" to introduce the overall idea and basic structures of the component - a paragraph for each of those basic structures (drm_gpu_scheduler, drm_sched_entity, drm_sched_fence) explaining their purpose and lifetime - a paragraph about the pitfalls dealing with DMA fences - a paragraph about the pitfalls of the driver callbacks (although this might highly intersect with the previous suggested one) I feel like this would be much easier to read. Going to give that a try. Besides that, which covers the conceptual side of things, I think we also need to improve the documentation on what the scheduler implementation expects from drivers, e.g. zero initialize structures, valid initialization parameters for typical use cases, etc. However, that's for a separate patch. Yeah, each individual function should have kerneldoc attached to it. I think we should also try to deprecate more of the hacks AMD came up. Especially the error and GPU reset handling is more than messed up. Regards, Christian. - Danilo Signed-off-by: Christian König --- drivers/gpu/drm/scheduler/sched_main.c | 126 - 1 file changed, 104 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 506371c42745..36a7c5dc852d 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -24,28 +24,110 @@ /** * DOC: Overview * - * The GPU scheduler provides entities which allow userspace to push jobs - * into software queues which are then scheduled on a hardware run queue. - * The software queues have a priority among them. The scheduler selects the entities - * from the run queue using a FIFO. The scheduler provides dependency handling - * features among jobs. The driver is supposed to provide callback functions for - * backend operations to the scheduler like submitting a job to hardware run queue, - * returning the dependencies of a job etc. - * - * The organisation of the scheduler is the following: - * - * 1. Each hw run queue has one scheduler - * 2. Each scheduler has multiple run queues with different priorities - * (e.g., HIGH_HW,HIGH_SW, KERNEL, NORMAL) - * 3. Each scheduler run queue has a queue of entities to schedule - * 4. Entities themselves maintain a queue of jobs that will be scheduled on - * the hardware. - * - * The jobs in a entity are always scheduled in the order that they were pushed. - * - * Note that once a job was taken from the entities queue and pushed to the - * hardware, i.e. the pending queue, the entity must not be referenced anymore - * through the jobs entity pointer. + * The GPU scheduler implements some logic to decide which command submission + * to push next to the hardware. Another major use case for the GPU scheduler + * is to enforce correct driver behavior around those command submission. + * Because of this it's also used by drivers which don't need the actual + * scheduling functionality. + * + * To fulfill this task the GPU scheduler uses of the following objects: + * + * 1. The job object which contains a bunch of dependencies in the form of + * DMA-fence objects. Drivers can also implement an optional prepare_job + * callback which returns additional dependencies as DMA-fence objects. + * It's important to note that this callback must follow the DMA-fence rules, + * so it can't easily allocate memory or grab locks under which memory is + * allocated. Drivers should use this as base class for an object which + * contains the necessary state to push the command submission to the + * hardware. + * + * The lifetime of the job object should at least be from pushing it into the + * scheduler until the scheduler notes through the free callback that a job + * isn't needed any more. Drivers can of course keep their job object alive + * longer than that, but that's outside of the scope of the scheduler + * component. Job initialization is split into two parts, + * drm_sched_job_init() and drm_sched_job_arm(). It's important to note that + * after arming a job drivers must follow the DMA-fence rules and can't + * easily allocate memory or takes locks under which memory is allocated. + * + * 2. The
Re: [RFC PATCH] of/platform: Disable sysfb if a simple-framebuffer node is found
On Mon, 13 Nov 2023 at 23:57, Javier Martinez Canillas wrote: > > Andrew Worsley writes: > > Hello Andrew, > > > On Mon, 13 Nov 2023 at 20:18, Thomas Zimmermann wrote: > >> Am 13.11.23 um 09:51 schrieb Javier Martinez Canillas: > >> > Some DT platforms use EFI to boot and in this case the EFI Boot Services > >> > may register a EFI_GRAPHICS_OUTPUT_PROTOCOL handle, that will later be > >> > queried by the Linux EFI stub to fill the global struct screen_info data. > >> > > > [...] > > > > > I applied the patch and just the simpledrm driver is probed (the efifb is > > not): > > >[...] > > Great, thanks for testing. The patch works then as expected. Can I get > your Tested-by then ? Sure absolutely. > > > [...] > We were talking with Thomas that the sysfb design seems to be reaching its > limits and need some rework but currently you either need some driver that > matches the "simple-framebuffer" device that is registered by OF or won't > get an early framebuffer in the system. > > That could be either simpledrm or simplefb. But if a DT has a device node > for "simple-framebuffer", how can the OF core know if there is a driver to > match that device? And same for any other device defined in the DTB. > > It's similar on platforms that use sysfb to register the device (e.g: x86) > since either "simple-framebuffer" is registered (if CONFIG_SYSFB_SIMPLEFB > is enabled) or "efi-framebuffer" (if CONFIG_SYSFB_SIMPLEFB is disabled). > > That means CONFIG_SYSFB_SIMPLEFB=y and CONFIG_DRM_SIMPLEDRM disabled won't > work either, even if CONFIG_FB_EFI=y which is the case you are mentioning. > > What I think that doesn't make sense is to remove conflicting framebuffers > from drivers that can only handle firmware provided framebuffers. As said > in the other thread, drm_aperture_remove_framebuffers() is only meant for > native DRM drivers. Ok - I'm taking it that conflicts between EFI and DT didn't happen in the past but when they do DT wins. I guess there may be more such conflicts in the future so would be resolved in a similar way as more drivers are updated to support DT settings. Perhaps one day all drivers would have DT settings and this could be standardised in some way. > -- > Best regards, > > Javier Martinez Canillas > Core Platforms > Red Hat > Thanks Andrew
Re: [PATCH 2/2] drm/virtio: Modify RESOURCE_GET_LAYOUT ioctl
On 11/10/23 10:16, Julia Zhang wrote: > Modify RESOURCE_GET_LAYOUT ioctl to handle the use case that query > correct stride for guest linear resource before it is created. > > Signed-off-by: Julia Zhang > --- > drivers/gpu/drm/virtio/virtgpu_drv.h | 26 -- > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 47 -- > drivers/gpu/drm/virtio/virtgpu_vq.c| 35 +++ > include/uapi/drm/virtgpu_drm.h | 6 ++-- > include/uapi/linux/virtio_gpu.h| 8 ++--- > 5 files changed, 66 insertions(+), 56 deletions(-) 1. Please squash this all into a single patch. For upstream kernel it's not acceptable to have subsequent commits modifying previous commits. To commit message add your s-o-b, your co-developed-by tags and a brief comment explaining changes you've done to the original patch. Signed-off-by: Daniel Stone Co-developed-by: Julia Zhang # query correct stride for guest linear resource before it's created Signed-off-by: Julia Zhang 2. Make sure that patch passes `scripts/checkpatch.pl` 3. Add link to the commit message for the relevant Mesa MR that makes use of the new ioctl. The MR should be already merged or ready to be merged. Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/123456 -- Best regards, Dmitry
Re: [PATCH] drm/scheduler: improve GPU scheduler documentation
Hi Christian, On 11/13/23 13:38, Christian König wrote: Start to improve the scheduler document. Especially document the lifetime of each of the objects as well as the restrictions around DMA-fence handling and userspace compatibility. Thanks a lot for submitting this - it's very much appreciated! Before reviewing in detail, do you mind to re-structure this a little bit? Instead of packing everything in an enumeration I'd suggest to have separate DOC paragraphs. For instance: - keep "Overview" to introduce the overall idea and basic structures of the component - a paragraph for each of those basic structures (drm_gpu_scheduler, drm_sched_entity, drm_sched_fence) explaining their purpose and lifetime - a paragraph about the pitfalls dealing with DMA fences - a paragraph about the pitfalls of the driver callbacks (although this might highly intersect with the previous suggested one) I feel like this would be much easier to read. Besides that, which covers the conceptual side of things, I think we also need to improve the documentation on what the scheduler implementation expects from drivers, e.g. zero initialize structures, valid initialization parameters for typical use cases, etc. However, that's for a separate patch. - Danilo Signed-off-by: Christian König --- drivers/gpu/drm/scheduler/sched_main.c | 126 - 1 file changed, 104 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 506371c42745..36a7c5dc852d 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -24,28 +24,110 @@ /** * DOC: Overview * - * The GPU scheduler provides entities which allow userspace to push jobs - * into software queues which are then scheduled on a hardware run queue. - * The software queues have a priority among them. The scheduler selects the entities - * from the run queue using a FIFO. The scheduler provides dependency handling - * features among jobs. The driver is supposed to provide callback functions for - * backend operations to the scheduler like submitting a job to hardware run queue, - * returning the dependencies of a job etc. - * - * The organisation of the scheduler is the following: - * - * 1. Each hw run queue has one scheduler - * 2. Each scheduler has multiple run queues with different priorities - *(e.g., HIGH_HW,HIGH_SW, KERNEL, NORMAL) - * 3. Each scheduler run queue has a queue of entities to schedule - * 4. Entities themselves maintain a queue of jobs that will be scheduled on - *the hardware. - * - * The jobs in a entity are always scheduled in the order that they were pushed. - * - * Note that once a job was taken from the entities queue and pushed to the - * hardware, i.e. the pending queue, the entity must not be referenced anymore - * through the jobs entity pointer. + * The GPU scheduler implements some logic to decide which command submission + * to push next to the hardware. Another major use case for the GPU scheduler + * is to enforce correct driver behavior around those command submission. + * Because of this it's also used by drivers which don't need the actual + * scheduling functionality. + * + * To fulfill this task the GPU scheduler uses of the following objects: + * + * 1. The job object which contains a bunch of dependencies in the form of + *DMA-fence objects. Drivers can also implement an optional prepare_job + *callback which returns additional dependencies as DMA-fence objects. + *It's important to note that this callback must follow the DMA-fence rules, + *so it can't easily allocate memory or grab locks under which memory is + *allocated. Drivers should use this as base class for an object which + *contains the necessary state to push the command submission to the + *hardware. + * + *The lifetime of the job object should at least be from pushing it into the + *scheduler until the scheduler notes through the free callback that a job + *isn't needed any more. Drivers can of course keep their job object alive + *longer than that, but that's outside of the scope of the scheduler + *component. Job initialization is split into two parts, + *drm_sched_job_init() and drm_sched_job_arm(). It's important to note that + *after arming a job drivers must follow the DMA-fence rules and can't + *easily allocate memory or takes locks under which memory is allocated. + * + * 2. The entity object which is a container for jobs which should execute + *sequentially. Drivers should create an entity for each individual context + *they maintain for command submissions which can run in parallel. + * + *The lifetime of the entity should *not* exceed the lifetime of the + *userspace process it was created for and drivers should call the + *drm_sched_entity_flush() function from their file_operations.flush + *callback.
