RE: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
Hi Daniel, > > > > >>> The solution: > > > > >>> - To ensure full framerate, the Guest compositor has to start it's > > > > >>> repaint cycle > > > (including > > > > >>> the 9 ms wait) when the Host compositor sends the frame callback > > > > >>> event to its > > > clients. > > > > >>> In order for this to happen, the dma-fence that the Guest KMS waits > > > > >>> on -- before > > > sending > > > > >>> pageflip completion -- cannot be tied to a wl_buffer.release event. > > > > >>> This means > that, > > > the > > > > >>> Guest compositor has to be forced to use a new buffer for its next > > > > >>> repaint cycle > > > when it > > > > >>> gets a pageflip completion. > > > > >> > > > > >> Is that really the only solution? > > > > > [Kasireddy, Vivek] There are a few others I mentioned here: > > > > > https://gitlab.freedesktop.org/wayland/weston/-/issues/514#note_986572 > > > > > But I think none of them are as compelling as this one. > > > > > > > > > >> > > > > >> If we fix the event timestamps so that both guest and host use the > > > > >> same > > > > >> timestamp, but then the guest starts 5ms (or something like that) > > > > >> earlier, > > > > >> then things should work too? I.e. > > > > >> - host compositor starts at (previous_frametime + 9ms) > > > > >> - guest compositor starts at (previous_frametime + 4ms) > > > > >> > > > > >> Ofc this only works if the frametimes we hand out to both match > > > > >> _exactly_ > > > > >> and are as high-precision as the ones on the host side. Which for > > > > >> many gpu > > > > >> drivers at least is the case, and all the ones you care about for > > > > >> sure :-) > > > > >> > > > > >> But if the frametimes the guest receives are the no_vblank fake > > > > >> ones, then > > > > >> they'll be all over the place and this carefully tuned low-latency > > > > >> redraw > > > > >> loop falls apart. Aside fromm the fact that without tuning the > > > > >> guests to > > > > >> be earlier than the hosts, you're guaranteed to miss every frame > > > > >> (except > > > > >> when the timing wobbliness in the guest is big enough by chance to > > > > >> make > > > > >> the deadline on the oddball frame). > > > > > [Kasireddy, Vivek] The Guest and Host use different event timestamps > > > > > as we don't > > > > > share these between the Guest and the Host. It does not seem to be > > > > > causing any > other > > > > > problems so far but we did try the experiment you mentioned (i.e., > > > > > adjusting the > > > delays) > > > > > and it works. However, this patch series is meant to fix the issue > > > > > without having to > > > tweak > > > > > anything (delays) because we can't do this for every compositor out > > > > > there. > > > > > > > > Maybe there could be a mechanism which allows the compositor in the > > > > guest to > > > automatically adjust its repaint cycle as needed. > > > > > > > > This might even be possible without requiring changes in each > > > > compositor, by > adjusting > > > the vertical blank periods in the guest to be aligned with the host > > > compositor repaint > > > cycles. Not sure about that though. > > > > > > > > Even if not, both this series or making it possible to queue multiple > > > > flips require > > > corresponding changes in each compositor as well to have any effect. > > > > > > Yeah from all the discussions and tests done it sounds even with a > > > deeper queue we have big coordination issues between the guest and > > > host compositor (like the example that the guest is now rendering at > > > 90fps instead of 60fps like the host). > > [Kasireddy, Vivek] Oh, I think you are referring to my reply to Gerd. That > > 90 FPS vs > > 60 FPS problem is a completely different issue that is associated with Qemu > > GTK UI > > backend. With the GTK backend -- and also with SDL backend -- we Blit the > > Guest > > scanout FB onto one of the backbuffers managed by EGL. > > > > I am trying to add a new Qemu Wayland UI backend so that we can eliminate > > that Blit > > and thereby have a truly zero-copy solution. And, this is there I am > > running into the > > halved frame-rate issue -- the current problem. > > Yes, that's what I referenced. But I disagree that it's a different > problem. The underlying problem in both cases is that the guest and host > compositor free-wheel instead of rendering in sync. It's just that > depending upon how exactly the flip completion event on the gues side > plays out you either get guest rendering that's faster than the host-side > 60fps, or guest rendering that's much slower than the host-side 60fps. [Kasireddy, Vivek] That used to be the case before we added a synchronization mechanism to the GTK UI backend that uses a sync file. After adding this and making the Guest wait until this sync file fd on the Host is signaled, we consistently get 60 FPS because the flip completion event for the Guest is directly tied to the signaling of the sync file in this particular case (GTK
[git pull] drm fixes for 5.14-rc5
Hi Linus, Regular weekly fixes pull, live from a Brisbane lockdown with kids at home. A big bunch of scattered amdgpu fixes, but they are all pretty small, minor i915 fixes, kmb, and one vmwgfx regression fixes, all pretty quiet for this time. Dave. drm-fixes-2021-08-06: drm fixes for 5.14-rc5 amdgpu: - Fix potential out-of-bounds read when updating GPUVM mapping - Renoir powergating fix - Yellow Carp updates - 8K fix for navi1x - Beige Goby updates and new DIDs - Fix DMUB firmware version output - EDP fix - pmops config fix i915: - Call i915_globals_exit if pci_register_device fails - (follow on fix for section mismatch) - Correct SFC_DONE register offset kmb: - DMA fix - driver date/version macros vmwgfx: - Fix I/O memory access on 64-bit systems The following changes since commit c500bee1c5b2f1d59b1081ac879d73268ab0ff17: Linux 5.14-rc4 (2021-08-01 17:04:17 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-08-06 for you to fetch changes up to d186f9c28008810d8f984d6bdd1c07757048ed63: Merge tag 'amd-drm-fixes-5.14-2021-08-05' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes (2021-08-06 11:22:09 +1000) drm fixes for 5.14-rc5 amdgpu: - Fix potential out-of-bounds read when updating GPUVM mapping - Renoir powergating fix - Yellow Carp updates - 8K fix for navi1x - Beige Goby updates and new DIDs - Fix DMUB firmware version output - EDP fix - pmops config fix i915: - Call i915_globals_exit if pci_register_device fails - (follow on fix for section mismatch) - Correct SFC_DONE register offset kmb: - DMA fix - driver date/version macros vmwgfx: - Fix I/O memory access on 64-bit systems Bing Guo (2): drm/amd/display: Fix Dynamic bpp issue with 8K30 with Navi 1X drm/amd/display: Increase stutter watermark for dcn303 Chengming Gui (1): drm/amdgpu: add DID for beige goby Dave Airlie (3): Merge tag 'drm-misc-fixes-2021-08-04' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'drm-intel-fixes-2021-08-04' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge tag 'amd-drm-fixes-5.14-2021-08-05' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Edmund Dea (2): drm/kmb: Enable LCD DMA for low TVDDCV drm/kmb: Define driver date and major/minor version Jason Ekstrand (1): drm/i915: Call i915_globals_exit() if pci_register_device() fails Jude Shih (1): drm/amd/display: Fix resetting DCN3.1 HW when resuming from S4 Matt Roper (1): drm/i915: Correct SFC_DONE register offset Qingqing Zhuo (1): drm/amd/display: workaround for hard hang on HPD on native DP Randy Dunlap (2): drm/i915: fix i915_globals_exit() section mismatch error drm/amdgpu: fix checking pmops when PM_SLEEP is not enabled Shirish S (1): drm/amdgpu/display: fix DMUB firmware version info Wesley Chalmers (1): drm/amd/display: Assume LTTPR interop for DCN31+ Xiaomeng Hou (1): drm/amd/pm: update yellow carp pmfw interface version Yifan Zhang (1): drm/amdgpu: fix the doorbell missing when in CGPG issue for renoir. Zack Rusin (1): drm/vmwgfx: Fix a 64bit regression on svga3 xinhui pan (1): drm/amdgpu: Fix out-of-bounds read when update mapping drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 7 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 3 ++- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 21 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- .../drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c | 4 +++- drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 21 ++--- drivers/gpu/drm/amd/display/dc/dc.h| 2 ++ drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c | 2 +- .../gpu/drm/amd/display/dc/dcn30/dcn30_resource.c | 20 .../drm/amd/display/dc/dcn303/dcn303_resource.c| 4 ++-- .../gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 16 drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.c | 8 +--- drivers/gpu/drm/amd/pm/inc/smu_v13_0.h | 2 +- drivers/gpu/drm/i915/i915_globals.c| 4 ++-- drivers/gpu/drm/i915/i915_pci.c| 1 + drivers/gpu/drm/i915/i915_reg.h| 2 +- drivers/gpu/drm/kmb/kmb_drv.c | 22 ++ drivers/gpu/drm/kmb/kmb_drv.h | 5 + drivers/gpu/drm/kmb/kmb_plane.c| 15 +-- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h| 2 +- 21 files changed, 124 insertions(+), 41 deletions(-)
Re: linux-next: Signed-off-by missing for commit in the drm-intel tree
Hi Matt,
Always use the dim tooling when applying patches, it will do the right
thing with regards to adding the S-o-b.
Regards, Joonas
Quoting Stephen Rothwell (2021-07-15 07:18:54)
> Hi all,
>
> Commit
>
> db47fe727e1f ("drm/i915/step: s/_revid_tbl/_revids")
>
> is missing a Signed-off-by from its committer.
>
> --
> Cheers,
> Stephen Rothwell
[PATCH] drm/msm/dsi: Fix some reference counted resource leaks
'of_find_device_by_node()' takes a reference that must be released when
not needed anymore.
This is expected to be done in 'dsi_destroy()'.
However, there are 2 issues in 'dsi_get_phy()'.
First, if 'of_find_device_by_node()' succeeds but 'platform_get_drvdata()'
returns NULL, 'msm_dsi->phy_dev' will still be NULL, and the reference
won't be released in 'dsi_destroy()'.
Secondly, as 'of_find_device_by_node()' already takes a reference, there is
no need for an additional 'get_device()'.
Move the assignment to 'msm_dsi->phy_dev' a few lines above and remove the
unneeded 'get_device()' to solve both issues.
Fixes: ec31abf6684e ("drm/msm/dsi: Separate PHY to another platform device")
Signed-off-by: Christophe JAILLET
---
Review carefully, management of reference counted resources is sometimes
tricky.
---
drivers/gpu/drm/msm/dsi/dsi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 75afc12a7b25..29d11f1cb79b 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -26,8 +26,10 @@ static int dsi_get_phy(struct msm_dsi *msm_dsi)
}
phy_pdev = of_find_device_by_node(phy_node);
- if (phy_pdev)
+ if (phy_pdev) {
msm_dsi->phy = platform_get_drvdata(phy_pdev);
+ msm_dsi->phy_dev = _pdev->dev;
+ }
of_node_put(phy_node);
@@ -36,8 +38,6 @@ static int dsi_get_phy(struct msm_dsi *msm_dsi)
return -EPROBE_DEFER;
}
- msm_dsi->phy_dev = get_device(_pdev->dev);
-
return 0;
}
--
2.30.2
[PATCH v2] drm/bridge: anx7625: Tune K value for IVO panel
IVO panel require less input video clock variation than video clock
variation in DP CTS spec.
This patch decreases the K value of ANX7625 which will shrink eDP Tx
video clock variation to meet IVO panel's requirement.
Acked-by: Sam Ravnborg
Signed-off-by: Xin Ji
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 24 ---
drivers/gpu/drm/bridge/analogix/anx7625.h | 4 +++-
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index a3d82377066b..9b9e3984dd38 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -384,6 +384,25 @@ static int anx7625_odfc_config(struct anx7625_data *ctx,
return ret;
}
+/*
+ * The MIPI source video data exist large variation (e.g. 59Hz ~ 61Hz),
+ * anx7625 defined K ratio for matching MIPI input video clock and
+ * DP output video clock. Increase K value can match bigger video data
+ * variation. IVO panel has small variation than DP CTS spec, need
+ * decrease the K value.
+ */
+static int anx7625_set_k_value(struct anx7625_data *ctx)
+{
+ struct edid *edid = (struct edid *)ctx->slimport_edid_p.edid_raw_data;
+
+ if (edid->mfg_id[0] == IVO_MID0 && edid->mfg_id[1] == IVO_MID1)
+ return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
+MIPI_DIGITAL_ADJ_1, 0x3B);
+
+ return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
+MIPI_DIGITAL_ADJ_1, 0x3D);
+}
+
static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx)
{
struct device *dev = >client->dev;
@@ -470,9 +489,8 @@ static int anx7625_dsi_video_timing_config(struct
anx7625_data *ctx)
MIPI_PLL_N_NUM_15_8, (n >> 8) & 0xff);
ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, MIPI_PLL_N_NUM_7_0,
(n & 0xff));
- /* Diff */
- ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
- MIPI_DIGITAL_ADJ_1, 0x3D);
+
+ anx7625_set_k_value(ctx);
ret |= anx7625_odfc_config(ctx, post_divider - 1);
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 034c3840028f..6dcf64c703f9 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -210,7 +210,9 @@
#define MIPI_VIDEO_STABLE_CNT 0x0A
#define MIPI_LANE_CTRL_10 0x0F
-#define MIPI_DIGITAL_ADJ_1 0x1B
+#define MIPI_DIGITAL_ADJ_1 0x1B
+#define IVO_MID0 0x26
+#define IVO_MID1 0xCF
#define MIPI_PLL_M_NUM_23_16 0x1E
#define MIPI_PLL_M_NUM_15_80x1F
--
2.25.1
Re: [V2][PATCH] drm: xlnx: zynqmp: release reset to DP controller before accessing DP registers
Hi Michale, On Fri, Aug 06, 2021 at 12:37:07PM +0200, Michal Simek wrote: > st 24. 3. 2021 v 4:15 odesílatel Laurent Pinchart napsal: > > On Tue, Mar 23, 2021 at 10:55:01AM +0800, quanyang.w...@windriver.com wrote: > > > From: Quanyang Wang > > > > > > When insmod zynqmp-dpsub.ko after rmmod it, system will hang with the > > > error log as below: > > > > > > root@xilinx-zynqmp:~# insmod zynqmp-dpsub.ko > > > [ 88.391289] [drm] Initialized zynqmp-dpsub 1.0.0 20130509 for > > > fd4a.display on minor 0 > > > [ 88.529906] Console: switching to colour frame buffer device 128x48 > > > [ 88.549402] zynqmp-dpsub fd4a.display: [drm] fb0: zynqmp-dpsubdrm > > > frame buffer device > > > [ 88.571624] zynqmp-dpsub fd4a.display: ZynqMP DisplayPort > > > Subsystem driver probed > > > root@xilinx-zynqmp:~# rmmod zynqmp_dpsub > > > [ 94.023404] Console: switching to colour dummy device 80x25 > > > root@xilinx-zynqmp:~# insmod zynqmp-dpsub.ko > > > > > > > > > This is because that in zynqmp_dp_probe it tries to access some DP > > > registers while the DP controller is still in the reset state. When > > > running "rmmod zynqmp_dpsub", zynqmp_dp_reset(dp, true) in > > > zynqmp_dp_phy_exit is called to force the DP controller into the reset > > > state. Then insmod will call zynqmp_dp_probe to program the DP registers, > > > but at this moment the DP controller hasn't been brought out of the reset > > > state yet since the function zynqmp_dp_reset(dp, false) is called later > > > and > > > this will result the system hang. > > > > > > Releasing the reset to DP controller before any read/write operation to it > > > will fix this issue. And for symmetry, move zynqmp_dp_reset() call from > > > zynqmp_dp_phy_exit() to zynqmp_dp_remove(). > > > > > > Signed-off-by: Quanyang Wang > > > > Reviewed-by: Laurent Pinchart > > Can someone pick this patch? I have it in my tree with a set of other changes, I intend to send a pull request today. -- Regards, Laurent Pinchart
Re: [Intel-gfx] [PATCH] drm/i915: Be more gentle when exiting non-persistent contexts
On 05/08/2021 17:32, Matthew Brost wrote:
On Thu, Aug 05, 2021 at 01:05:09PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
When a non-persistent context exits we currently mark it as banned in
order to trigger fast termination of any outstanding GPU jobs it may have
left running.
In doing so we apply a very strict 1ms limit in which the left over job
has to preempt before we issues an engine resets.
Some workloads are not able to cleanly preempt in that time window and it
can be argued that it would instead be better to give them a bit more
grace since avoiding engine resets is generally preferrable.
To achieve this the patch splits handling of banned contexts from simply
closed non-persistent ones and then applies different timeouts for both
and also extends the criteria which determines if a request should be
scheduled back in after preemption or not.
15ms preempt timeout grace is given to exited non-persistent contexts
which have been empirically tested to satisfy customers requirements
and still provides reasonably quick cleanup post exit.
I think you need to rework your thinking here a bit as this a very
execlists specific solution and the GuC needs to be considered.
Slipped my mind GuC patches were merged in the meantime. (This patch
predates that.) But I think wording in the commit message is fine. It is
just the implementation that now has to handle the GuC as well.
v2:
* Streamline fast path checks.
v3:
* Simplify by using only schedulable status.
* Increase timeout to 20ms.
v4:
* Fix live_execlists selftest.
v5:
* Fix logic in kill_engines.
v6:
* Rebase.
Signed-off-by: Tvrtko Ursulin
Cc: Chris Wilson
Cc: Zhen Han
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 22 +--
drivers/gpu/drm/i915/gt/intel_context.c | 2 ++
drivers/gpu/drm/i915/gt/intel_context.h | 17 +-
drivers/gpu/drm/i915/gt/intel_context_types.h | 1 +
.../drm/i915/gt/intel_execlists_submission.c | 11 --
drivers/gpu/drm/i915/gt/selftest_execlists.c | 20 +++--
drivers/gpu/drm/i915/i915_request.c | 2 +-
7 files changed, 57 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cff72679ad7c..21fe5d4057ab 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1065,7 +1065,8 @@ static struct intel_engine_cs *active_engine(struct
intel_context *ce)
return engine;
}
-static void kill_engines(struct i915_gem_engines *engines, bool ban)
+static void
+kill_engines(struct i915_gem_engines *engines, bool ban, bool persistent)
{
struct i915_gem_engines_iter it;
struct intel_context *ce;
@@ -1079,8 +1080,15 @@ static void kill_engines(struct i915_gem_engines
*engines, bool ban)
*/
for_each_gem_engine(ce, engines, it) {
struct intel_engine_cs *engine;
+ bool skip = false;
+
+ if (ban)
+ skip = intel_context_ban(ce, NULL);
+ else if (!persistent)
+ skip = !intel_context_clear_schedulable(ce);
schedulable doesn't hook into the backend at all, while
intel_context_ban does. In the case of GuC submission intel_context_ban
changes to preemption timeout to 1 us and disables scheduling resulting
in the context getting kicked off the hardware immediately. You likely
need to update intel_context_clear_schedulable to use the same vfunc as
intel_context_ban() but accept an argument for the value of the
preemption timeout. For a ban user a lower value, for clearing
schedulable use a higher value.
Okay I'll have a look. Might go back to closed flag as opposed to
schedulable as well since I don't quite like schedulable being the odd
one out.
- if (ban && intel_context_ban(ce, NULL))
+ /* Already previously banned or made non-schedulable? */
+ if (skip)
continue;
/*
@@ -1093,7 +1101,7 @@ static void kill_engines(struct i915_gem_engines
*engines, bool ban)
engine = active_engine(ce);
/* First attempt to gracefully cancel the context */
- if (engine && !__cancel_engine(engine) && ban)
+ if (engine && !__cancel_engine(engine) && (ban || !persistent))
/*
* If we are unable to send a preemptive pulse to bump
* the context from the GPU, we have to resort to a full
@@ -1105,8 +1113,6 @@ static void kill_engines(struct i915_gem_engines
*engines, bool ban)
static void kill_context(struct i915_gem_context *ctx)
{
- bool ban = (!i915_gem_context_is_persistent(ctx) ||
- !ctx->i915->params.enable_hangcheck);
struct i915_gem_engines *pos, *next;
spin_lock_irq(>stale.lock);
@@ -1119,7 +1125,8 @@ static void
[PATCH v2] drm/bridge: anx7625: Tune K value for IVO panel
IVO panel require less input video clock variation than video clock
variation in DP CTS spec.
This patch decreases the K value of ANX7625 which will shrink eDP Tx
video clock variation to meet IVO panel's requirement.
Acked-by: Sam Ravnborg
Signed-off-by: Xin Ji
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 24 ---
drivers/gpu/drm/bridge/analogix/anx7625.h | 4 +++-
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index a3d82377066b..9b9e3984dd38 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -384,6 +384,25 @@ static int anx7625_odfc_config(struct anx7625_data *ctx,
return ret;
}
+/*
+ * The MIPI source video data exist large variation (e.g. 59Hz ~ 61Hz),
+ * anx7625 defined K ratio for matching MIPI input video clock and
+ * DP output video clock. Increase K value can match bigger video data
+ * variation. IVO panel has small variation than DP CTS spec, need
+ * decrease the K value.
+ */
+static int anx7625_set_k_value(struct anx7625_data *ctx)
+{
+ struct edid *edid = (struct edid *)ctx->slimport_edid_p.edid_raw_data;
+
+ if (edid->mfg_id[0] == IVO_MID0 && edid->mfg_id[1] == IVO_MID1)
+ return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
+MIPI_DIGITAL_ADJ_1, 0x3B);
+
+ return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
+MIPI_DIGITAL_ADJ_1, 0x3D);
+}
+
static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx)
{
struct device *dev = >client->dev;
@@ -470,9 +489,8 @@ static int anx7625_dsi_video_timing_config(struct
anx7625_data *ctx)
MIPI_PLL_N_NUM_15_8, (n >> 8) & 0xff);
ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, MIPI_PLL_N_NUM_7_0,
(n & 0xff));
- /* Diff */
- ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
- MIPI_DIGITAL_ADJ_1, 0x3D);
+
+ anx7625_set_k_value(ctx);
ret |= anx7625_odfc_config(ctx, post_divider - 1);
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 034c3840028f..6dcf64c703f9 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -210,7 +210,9 @@
#define MIPI_VIDEO_STABLE_CNT 0x0A
#define MIPI_LANE_CTRL_10 0x0F
-#define MIPI_DIGITAL_ADJ_1 0x1B
+#define MIPI_DIGITAL_ADJ_1 0x1B
+#define IVO_MID0 0x26
+#define IVO_MID1 0xCF
#define MIPI_PLL_M_NUM_23_16 0x1E
#define MIPI_PLL_M_NUM_15_80x1F
--
2.25.1
Re: [PATCH v1] drm: bridge: it66121: Check drm_bridge_attach retval
On Thu, 5 Aug 2021 at 22:37, Jernej Škrabec wrote:
>
> Hi!
>
> Dne četrtek, 05. avgust 2021 ob 20:50:39 CEST je Robert Foss napisal(a):
> > The return value of drm_bridge_attach() is ignored during
> > the it66121_bridge_attach() call, which is incorrect.
> >
> > Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")
> > Signed-off-by: Robert Foss
> > ---
> > drivers/gpu/drm/bridge/ite-it66121.c | 2 ++
> > 1 file changed, 2 insertions(+)
>
> Acked-by: Jernej Skrabec
>
Applied to drm-misc-next
Re: [PATCH v1 1/1] drm/bridge: anx7625: Tune K value for IVO panel
On Thu, Aug 05, 2021 at 09:05:29PM +0200, Robert Foss wrote:
> Hey Xin,
>
> Thanks for submitting this.
>
> On Thu, 5 Aug 2021 at 09:31, Xin Ji wrote:
> >
> > IVO panel require less input video clock variation than video clock
> > variation in DP CTS spec.
> >
> > This patch decreases the K value of ANX7625 which will shrink eDP Tx
> > video clock variation to meet IVO panel's requirement.
> >
> > Signed-off-by: Xin Ji
> > ---
> > drivers/gpu/drm/bridge/analogix/anx7625.c | 17 ++---
> > drivers/gpu/drm/bridge/analogix/anx7625.h | 4 +++-
> > 2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index a3d82377066b..ceed1c7f3f28 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > @@ -384,6 +384,18 @@ static int anx7625_odfc_config(struct anx7625_data
> > *ctx,
> > return ret;
> > }
> >
> > +static int anx7625_set_k_value(struct anx7625_data *ctx)
>
> Pardon my ignorance, but I don't know what a K-value is. Could you add
> a comment detailing
> what the K-value does?
Hi Robert Foss, OK, I'll add more comment.
Thanks,
Xin
>
> > +{
> > + struct edid *edid = (struct edid
> > *)ctx->slimport_edid_p.edid_raw_data;
> > +
> > + if (edid->mfg_id[0] == IVO_MID0 && edid->mfg_id[1] == IVO_MID1)
> > + return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> > +MIPI_DIGITAL_ADJ_1, 0x3B);
> > +
> > + return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> > +MIPI_DIGITAL_ADJ_1, 0x3D);
> > +}
> > +
> > static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx)
> > {
> > struct device *dev = >client->dev;
> > @@ -470,9 +482,8 @@ static int anx7625_dsi_video_timing_config(struct
> > anx7625_data *ctx)
> > MIPI_PLL_N_NUM_15_8, (n >> 8) & 0xff);
> > ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> > MIPI_PLL_N_NUM_7_0,
> > (n & 0xff));
> > - /* Diff */
> > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> > - MIPI_DIGITAL_ADJ_1, 0x3D);
> > + /* Diff and K value */
>
> With a proper comment above, this comment is no longer needed.
OK
>
> > + anx7625_set_k_value(ctx);
> >
> > ret |= anx7625_odfc_config(ctx, post_divider - 1);
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
> > b/drivers/gpu/drm/bridge/analogix/anx7625.h
> > index 034c3840028f..6dcf64c703f9 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.h
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
> > @@ -210,7 +210,9 @@
> > #define MIPI_VIDEO_STABLE_CNT 0x0A
> >
> > #define MIPI_LANE_CTRL_10 0x0F
> > -#define MIPI_DIGITAL_ADJ_1 0x1B
> > +#define MIPI_DIGITAL_ADJ_1 0x1B
> > +#define IVO_MID0 0x26
> > +#define IVO_MID1 0xCF
> >
> > #define MIPI_PLL_M_NUM_23_16 0x1E
> > #define MIPI_PLL_M_NUM_15_80x1F
> > --
> > 2.25.1
> >
>
> LGTM with the above fix.
>
> Reviewed-by: Robert Foss
Re: [PATCH v2 1/1] drm/bridge: anx7625: Tune K value for IVO panel
Hey Xin,
Thanks for implementing the suggestion so quickly.
Can you send this version of the patch out as v2? Versioning is
important and both tools and processes break if different versions
aren't submitted in different emails.
On Fri, 6 Aug 2021 at 11:35, Xin Ji wrote:
>
> IVO panel require less input video clock variation than video clock
> variation in DP CTS spec.
>
> This patch decreases the K value of ANX7625 which will shrink eDP Tx
> video clock variation to meet IVO panel's requirement.
>
> Acked-by: Sam Ravnborg
> Signed-off-by: Xin Ji
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 24 ---
> drivers/gpu/drm/bridge/analogix/anx7625.h | 4 +++-
> 2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index a3d82377066b..9b9e3984dd38 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -384,6 +384,25 @@ static int anx7625_odfc_config(struct anx7625_data *ctx,
> return ret;
> }
>
> +/*
> + * The MIPI source video data exist large variation (e.g. 59Hz ~ 61Hz),
> + * anx7625 defined K ratio for matching MIPI input video clock and
> + * DP output video clock. Increase K value can match bigger video data
> + * variation. IVO panel has small variation than DP CTS spec, need
> + * decrease the K value.
> + */
> +static int anx7625_set_k_value(struct anx7625_data *ctx)
> +{
> + struct edid *edid = (struct edid *)ctx->slimport_edid_p.edid_raw_data;
> +
> + if (edid->mfg_id[0] == IVO_MID0 && edid->mfg_id[1] == IVO_MID1)
> + return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> +MIPI_DIGITAL_ADJ_1, 0x3B);
> +
> + return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> +MIPI_DIGITAL_ADJ_1, 0x3D);
> +}
> +
> static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx)
> {
> struct device *dev = >client->dev;
> @@ -470,9 +489,8 @@ static int anx7625_dsi_video_timing_config(struct
> anx7625_data *ctx)
> MIPI_PLL_N_NUM_15_8, (n >> 8) & 0xff);
> ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> MIPI_PLL_N_NUM_7_0,
> (n & 0xff));
> - /* Diff */
> - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> - MIPI_DIGITAL_ADJ_1, 0x3D);
> +
> + anx7625_set_k_value(ctx);
>
> ret |= anx7625_odfc_config(ctx, post_divider - 1);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
> b/drivers/gpu/drm/bridge/analogix/anx7625.h
> index 034c3840028f..6dcf64c703f9 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.h
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
> @@ -210,7 +210,9 @@
> #define MIPI_VIDEO_STABLE_CNT 0x0A
>
> #define MIPI_LANE_CTRL_10 0x0F
> -#define MIPI_DIGITAL_ADJ_1 0x1B
> +#define MIPI_DIGITAL_ADJ_1 0x1B
> +#define IVO_MID0 0x26
> +#define IVO_MID1 0xCF
>
> #define MIPI_PLL_M_NUM_23_16 0x1E
> #define MIPI_PLL_M_NUM_15_80x1F
> --
> 2.25.1
>
Re: [PATCH 2/2] gpu/drm: ingenic: Add workaround for disabled drivers
On Thu, Aug 05, 2021 at 10:05:27PM +0200, Paul Cercueil wrote:
> Hi Greg,
>
> Le jeu., août 5 2021 at 21:35:34 +0200, Greg Kroah-Hartman
> a écrit :
> > On Thu, Aug 05, 2021 at 09:21:09PM +0200, Paul Cercueil wrote:
> > > When the drivers of remote devices (e.g. HDMI chip) are disabled in
> > > the
> > > config, we want the ingenic-drm driver to be able to probe
> > > nonetheless
> > > with the other devices (e.g. internal LCD panel) that are enabled.
> > >
> > > Signed-off-by: Paul Cercueil
> > > ---
> > > drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 12
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > > b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > > index d261f7a03b18..5e1fdbb0ba6b 100644
> > > --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > > +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > > @@ -1058,6 +1058,18 @@ static int ingenic_drm_bind(struct device
> > > *dev, bool has_components)
> > > for (i = 0; ; i++) {
> > > ret = drm_of_find_panel_or_bridge(dev->of_node, 0, i,
> > > ,
> > > );
> > > if (ret) {
> > > +/*
> > > + * Workaround for the case where the drivers
> > > for the
> > > + * remote devices are not enabled. When that
> > > happens,
> > > + * drm_of_find_panel_or_bridge() returns
> > > -EPROBE_DEFER
> > > + * endlessly, which prevents the ingenic-drm
> > > driver from
> > > + * working at all.
> > > + */
> > > +if (ret == -EPROBE_DEFER) {
> > > +ret =
> > > driver_deferred_probe_check_state(dev);
> > > +if (ret == -ENODEV || ret == -ETIMEDOUT)
> > > +continue;
> > > +}
> >
> > So you are mucking around with devices on other busses within this
> > driver? What could go wrong? :(
>
> I'm doing the same thing as everybody else. This is the DRM driver, and
> there is a driver for the external HDMI chip which gives us a DRM bridge
> that we can obtain from the device tree.
But then why do you need to call this function that is there for a bus,
not for a driver.
> > Please use the existing driver core functionality for this type of
> > thing, it is not unique, no need for this function to be called.
>
> I'm not sure you understand what I'm doing here. This driver calls
> drm_of_find_panel_or_bridge(), without guarantee that the driver for the
> remote device (connected via DT graph) has been enabled in the kernel
> config. In that case it will always return -EPROBE_DEFER and the ingenic-drm
> driver will never probe.
>
> This patch makes sure that the driver can probe if the HDMI driver has been
> disabled in the kernel config, nothing more.
That should not be an issue as you do not care if the config is enabled,
you just want to do something in the future if the driver shows up,
right?
Much like the device link code, have you looked at that?
thanks,
greg k-h
Re: [PATCH v2] drm/bridge: anx7625: Tune K value for IVO panel
Applied to drm-misc-next
[Bug 213983] New: WARNING: CPU: 3 PID: 520 at drivers/gpu/drm/ttm/ttm_bo.c:409 ttm_bo_release+0x7a/0x803 [ttm]
https://bugzilla.kernel.org/show_bug.cgi?id=213983 Bug ID: 213983 Summary: WARNING: CPU: 3 PID: 520 at drivers/gpu/drm/ttm/ttm_bo.c:409 ttm_bo_release+0x7a/0x803 [ttm] Product: Drivers Version: 2.5 Kernel Version: 5.14-rc4 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: erhar...@mailbox.org Regression: No Created attachment 298221 --> https://bugzilla.kernel.org/attachment.cgi?id=298221=edit kernel dmesg (5.14-rc4, AMD A10-7860K) System was running fine for hours but got this at reboot: [...] [35939.202358] [ cut here ] [35939.202603] WARNING: CPU: 3 PID: 520 at drivers/gpu/drm/ttm/ttm_bo.c:409 ttm_bo_release+0x7a/0x803 [ttm] [35939.202870] Modules linked in: rfkill dm_crypt nhpoly1305_sse2 nhpoly1305 chacha_generic chacha_x86_64 libchacha adiantum libpoly1305 algif_skcipher input_leds joydev led_class dm_mod hid_generic usbhid hid raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx amdgpu md_mod evdev f2fs crc32_generic lz4hc_compress lz4_compress lz4_decompress edac_mce_amd crc32_pclmul ext4 crc16 mbcache jbd2 ohci_pci snd_hda_codec_hdmi drm_ttm_helper ttm aesni_intel snd_hda_intel mfd_core libaes crypto_simd cryptd gpu_sched snd_intel_dspcfg i2c_algo_bit k10temp snd_hda_codec fam15h_power i2c_piix4 snd_hwdep drm_kms_helper snd_hda_core ohci_hcd cfbfillrect ehci_pci xhci_pci xhci_hcd ehci_hcd syscopyarea cfbimgblt snd_pcm sysfillrect sysimgblt usbcore fb_sys_fops snd_timer cfbcopyarea usb_common snd soundcore video acpi_cpufreq processor button zram nct6775 zsmalloc hwmon_vid hwmon drm nfsd fb fuse font fbdev auth_rpcgss drm_panel_orientation_quirks backlight lockd grace configfs sunrpc [35939.203200] efivarfs [35939.205121] CPU: 3 PID: 520 Comm: X Not tainted 5.14.0-rc4-bdver3 #2 [35939.212327] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./A88M-G/3.1, BIOS P1.40C 11/21/2016 [35939.219364] RIP: 0010:ttm_bo_release+0x7a/0x803 [ttm] [35939.219387] Code: e0 2a 48 c1 ea 03 8a 14 02 48 8b 04 24 83 e0 07 83 c0 03 38 d0 7c 0d 84 d2 74 09 48 8b 3c 24 e8 99 eb 7c dd 83 7b 4c 00 74 02 <0f> 0b 48 8d 43 18 48 89 c2 48 89 44 24 10 b8 ff ff 37 00 48 c1 ea [35939.230367] RSP: 0018:c900050dfb08 EFLAGS: 00010202 [35939.230376] RAX: 0007 RBX: 8881716441f0 RCX: 8881716441f0 [35939.230381] RDX: 11102e2c8800 RSI: 0004 RDI: 8881716441f0 [35939.230387] RBP: 8881716441d8 R08: 0001 R09: ed102e2c883f [35939.230391] R10: 8881716441f3 R11: c0b6a201 R12: 88811510 [35939.264555] R13: 888115005608 R14: 888171644058 R15: 9f5cf160 [35939.264564] FS: 7fe7325f9980() GS:8883e058() knlGS: [35939.264572] CS: 0010 DS: ES: CR0: 80050033 [35939.264579] CR2: 7fe73190d0e8 CR3: 000141978000 CR4: 000506e0 [35939.264586] Call Trace: [35939.264591] ? fsnotify_grab_connector+0x8c/0x93 [35939.264608] amdgpu_bo_unref+0x30/0x57 [amdgpu] [35939.318763] amdgpu_gem_object_free+0x69/0x95 [amdgpu] [35939.319132] ? list_add+0xd1/0xd1 [amdgpu] [35939.319478] ? test_bit+0x1d/0x27 [35939.319489] drm_gem_dmabuf_release+0x5b/0x67 [drm] [35939.319551] dma_buf_release+0x113/0x1b6 [35939.319563] __dentry_kill+0x29e/0x302 [35939.319573] dput+0x184/0x1c3 [35939.319582] __fput+0x4dc/0x598 [35939.319590] task_work_run+0xfa/0x118 [35939.319599] do_exit+0x984/0x17ba [35939.319609] ? mm_update_next_owner+0x3fd/0x3fd [35939.319619] do_group_exit+0x229/0x229 [35939.319627] __x64_sys_exit_group+0x39/0x39 [35939.319635] do_syscall_64+0x75/0x88 [35939.319649] ? do_syscall_64+0xe/0x88 [35939.319658] entry_SYSCALL_64_after_hwframe+0x44/0xae [35939.319668] RIP: 0033:0x7fe731edd459 [35939.319676] Code: Unable to access opcode bytes at RIP 0x7fe731edd42f. [35939.319680] RSP: 002b:7ffc8d3e1298 EFLAGS: 0246 ORIG_RAX: 00e7 [35939.319691] RAX: ffda RBX: 7fe731fc6920 RCX: 7fe731edd459 [35939.319698] RDX: 003c RSI: 00e7 RDI: [35939.319704] RBP: 7fe731fc6920 R08: fd40 R09: 5630715eb7c0 [35939.319711] R10: 7fe710db0a14 R11: 0246 R12: [35939.319716] R13: R14: 0668 R15: [35939.319724] ---[ end trace 0f92591c8b7a0f11 ]--- # inxi -bZ System:Kernel: 5.14.0-rc4-bdver3 x86_64 bits: 64 Desktop: MATE 1.24.1 Distro: Gentoo Base System release 2.7 Machine: Type: Desktop Mobo: ASRock model: A88M-G/3.1 serial: UEFI: American Megatrends v: P1.40C date: 11/21/2016 CPU: Info: Quad Core AMD A10-7860K Radeon R7 12 Compute Cores 4C+8G [MCP]
[Bug 213983] WARNING: CPU: 3 PID: 520 at drivers/gpu/drm/ttm/ttm_bo.c:409 ttm_bo_release+0x7a/0x803 [ttm]
https://bugzilla.kernel.org/show_bug.cgi?id=213983 --- Comment #1 from Erhard F. (erhar...@mailbox.org) --- Created attachment 298223 --> https://bugzilla.kernel.org/attachment.cgi?id=298223=edit kernel .config (5.14-rc4, AMD A10-7860K) -- 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: [RFC PATCH 13/15] mm: convert MAX_ORDER sized static arrays to dynamic ones.
Am 05.08.21 um 21:58 schrieb Zi Yan: On 5 Aug 2021, at 15:16, Christian König wrote: Am 05.08.21 um 21:02 schrieb Zi Yan: From: Zi Yan This prepares for the upcoming changes to make MAX_ORDER a boot time parameter instead of compilation time constant. All static arrays with MAX_ORDER size are converted to pointers and their memory is allocated at runtime. Well in general I strongly suggest to not use the patter kmalloc(sizeof(some struct) * MAX_ORDER,...) instead use kmalloc_array, kcalloc etc.. Then when a array is embedded at the end of a structure you can use a trailing array and the struct_size() macro to determine the allocation size. Sure. Will fix it. Additional to that separating the patch into changes for TTM to make the maximum allocation order independent from MAX_ORDER would be rather good to have I think. Can you elaborate a little bit more on “make the maximum allocation order independent from MAX_ORDER”? My idea was that you have a single patch to give the maximum order as parameter to the TTM pool. From what I understand of ttm_pool_alloc(), it tries to get num_pages pages by allocating as large pages as possible, starting from MAX_ORDER. What is the rationale behind this algorithm? Why not just call alloc_page(order=0) num_pages times? What we do here is essentially transparent huge pages for GPUs. In opposite to CPU which can usually only use fixed sizes like 4KiB, 2MiB, 1GiB at least AMD GPUs can use any power of two. So it makes sense to allocate big pages as much as possible and only fallback to 4KiB pages when necessary. Down side is that we potentially exhaust larger orders for other use cases. Is it mean to reduce the number of calls to alloc_page? That is a nice to have side effect, but the performance improvement for the TLB is the main reason here. The allocated pages do not need to get as high as MAX_ORDER, is that the case? Actually we would really like to have pages as large as 1GiB for best TLB utilization :) If yes, I probably can keep ttm pool as static arrays with length of MIN_MAX_ORDER, which I introduce in Patch 14 as the lower bound of boot time parameter MAX_ORDER. Let me know your thoughts. Well you could do something like MAX_MAX_ORDER with a value of at least 19 (1GiB with 4KiB pages IIRC). And then limit to the real available max_order when you make that a run time option. Since the array elements consists only of a linked list and a few extra parameters / pointers we probably won't need more than a page or two for those. Regards, Christian. Thanks. — Best Regards, Yan, Zi
Re: [PULL] drm-intel-gt-next
Quoting Joonas Lahtinen (2021-08-06 13:06:17) > Hi Dave & Daniel, > > Sorry for the big PR in advance. Had the summer vacations and did not > notice until tool late how many patches were in already before leaving. > > As requested, there is a lot of refactoring to increase the use of TTM > allocator and prep for DRM scheduler. Note that at times the patches trade > simplicity for performance and revert optimizations not seen as critical. > So some performance regressions are expected. > > As an example is dropping of faster GPU relocation path for older platforms, > which should be mitigated by updating to the latest UMD versions to regain > the perf. > > This PR drops various bits of uAPI where userspace has dropped the ball after > reviews > have been completed on both sides (Thanks Jason for doing the > due-diligence!). [1] > Due to the complexity of tracing back who used what, I think we could do > better in the > future to avoid such situations to begin with. See below for my suggestion > [2]. > > In addition to the refactoring and uAPI cleanup there is preliminay code for > ADL-P/XeHP and DG2 platforms. Fixes for ADL-S and removal of CNL code. > A couple of fixes that have been Cc: stable already. Removing unnecessary > workarounds per stepping and adding missing for Gen12 iGFX. > > I915_MMAP_OFFSET_FIXED is also added to to align with the static/fixed caching > mode per BO instead of per-mapping mode (for dGFX only). There is GuC firmware > interface update and backend code major rework that unblock enabling GuC on > Gen11 > (not on by default). Then there is the addition of the GuCRC power management > feature which can be enabled for Gen12+ when submission is enabled. There is also addition of I915_USERPTR_PROBE with userspace changes in: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12044 Regards, Joonas > > Then there is finally the drm-next backmerge and 3 topic branch merges. > > I think that is mostly all. I tried to summarize much in the tag description > so > it should hopefully not be horribly long... > > Regards, Joonas > > [1] Given that Jason is only human and there is no way to automate this > analysis, we > may have to bring something back as fixes if we find breakage (like with the > MMAP IOCTL). > > [2] As we first require to merge the kernel code, should we introduce some > further rules > that the userspace has to land their code before the final kernel version > release? If > that is not followed through, we would neuter the new uAPI with as small > patch as possible > in the final release candidate and then remove it in next release. Thoughts? > > *** > > drm-intel-gt-next-2021-08-06-1: > > UAPI Changes: > > - Add I915_MMAP_OFFSET_FIXED > > On devices with local memory `I915_MMAP_OFFSET_FIXED` is the only valid > type. On devices without local memory, this caching mode is invalid. > > As caching mode when specifying `I915_MMAP_OFFSET_FIXED`, WC or WB will > be used, depending on the object placement on creation. WB will be used > when the object can only exist in system memory, WC otherwise. > > Userspace: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11888 > > - Reinstate the mmap ioctl for (already released) integrated Gen12 platforms > > Rationale: Otherwise media driver breaks eg. for ADL-P. Long term goal is > still to sunset the IOCTL even for integrated and require using mmap_offset. > > - Reject caching/set_domain IOCTLs on discrete > > Expected to become immutable property of the BO > > - Disallow changing context parameters after first use on Gen12 and earlier > - Require setting context parameters at creation on platforms after Gen12 > > Rationale (for both): Allow less dynamic changes to the context to simplify > the implementation and avoid user shooting theirselves in the foot. > > - Drop I915_CONTEXT_PARAM_RINGSIZE > > Userspace PR for compute-driver has not been merged > > - Drop I915_CONTEXT_PARAM_NO_ZEROMAP > > Userspace PR for libdrm / Beignet was never landed > > - Drop CONTEXT_CLONE API > > Userspace PR for Mesa was never landed > > - Drop getparam support for I915_CONTEXT_PARAM_ENGINES > > Only existed for symmetry wrt. setparam, never used. > > - Disallow bonding of virtual engines > > Drop the prep work, no hardware has been released needing it. > > - (Implicit) Disable gpu relocations > > Media userspace was the last userspace to still use them. They > have converted so performance can be regained with an update. > > Core Changes: > > - Merge topic branch 'topic/i915-ttm-2021-06-11' (from Maarten) > - Merge topic branch 'topic/revid_steppings' (from Matt R) > - Merge topic branch 'topic/xehp-dg2-definitions-2021-07-21' (from Matt R) > - Backmerges drm-next (Rodrigo) > > Driver Changes: > > - Initial workarounds for ADL-P (Clint) > - Preliminary code for XeHP/DG2 (Stuart, Umesh, Matt R, Prathap, Ram, > Venkata, Akeem, Tvrtko, John, Lucas)
Re: [PATCH 2/2] gpu/drm: ingenic: Add workaround for disabled drivers
Hi Greg,
Le ven., août 6 2021 at 12:17:55 +0200, Greg Kroah-Hartman
a écrit :
On Thu, Aug 05, 2021 at 10:05:27PM +0200, Paul Cercueil wrote:
Hi Greg,
Le jeu., août 5 2021 at 21:35:34 +0200, Greg Kroah-Hartman
a écrit :
> On Thu, Aug 05, 2021 at 09:21:09PM +0200, Paul Cercueil wrote:
> > When the drivers of remote devices (e.g. HDMI chip) are
disabled in
> > the
> > config, we want the ingenic-drm driver to be able to probe
> > nonetheless
> > with the other devices (e.g. internal LCD panel) that are
enabled.
> >
> > Signed-off-by: Paul Cercueil
> > ---
> > drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > index d261f7a03b18..5e1fdbb0ba6b 100644
> > --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > @@ -1058,6 +1058,18 @@ static int ingenic_drm_bind(struct
device
> > *dev, bool has_components)
> > for (i = 0; ; i++) {
> > ret = drm_of_find_panel_or_bridge(dev->of_node, 0, i,
,
> > );
> > if (ret) {
> > + /*
> > +* Workaround for the case where the drivers for the
> > +* remote devices are not enabled. When that happens,
> > +* drm_of_find_panel_or_bridge() returns -EPROBE_DEFER
> > +* endlessly, which prevents the ingenic-drm driver
from
> > +* working at all.
> > +*/
> > + if (ret == -EPROBE_DEFER) {
> > + ret = driver_deferred_probe_check_state(dev);
> > + if (ret == -ENODEV || ret == -ETIMEDOUT)
> > + continue;
> > + }
>
> So you are mucking around with devices on other busses within this
> driver? What could go wrong? :(
I'm doing the same thing as everybody else. This is the DRM driver,
and
there is a driver for the external HDMI chip which gives us a DRM
bridge
that we can obtain from the device tree.
But then why do you need to call this function that is there for a
bus,
not for a driver.
The documentation disagrees with you :)
And, if that has any weight, this solution was proposed by Rob.
> Please use the existing driver core functionality for this type of
> thing, it is not unique, no need for this function to be called.
I'm not sure you understand what I'm doing here. This driver calls
drm_of_find_panel_or_bridge(), without guarantee that the driver
for the
remote device (connected via DT graph) has been enabled in the
kernel
config. In that case it will always return -EPROBE_DEFER and the
ingenic-drm
driver will never probe.
This patch makes sure that the driver can probe if the HDMI driver
has been
disabled in the kernel config, nothing more.
That should not be an issue as you do not care if the config is
enabled,
you just want to do something in the future if the driver shows up,
right?
Well, the DRM subsystem doesn't really seem to handle hotplug of
hardware. Right now all the drivers for the connected hardware need to
probe before the main DRM driver. So I need to know that a remote
device (connected via DT graph) will never probe.
Give me a of_graph_remote_device_driver_will_never_probe() and I'll use
that.
Much like the device link code, have you looked at that?
I don't see how that would help in any way. The device link code would
allow me to set a dependency between the remote hardware (HDMI chip,
provider) and the LCD controller (consumer), but I already have that
dependency though the DT graph. What I need is a way for the consumer
to continue probing if the provider is not going to probe.
Cheers,
-Paul
[PATCH v2 1/1] drm/bridge: anx7625: Tune K value for IVO panel
IVO panel require less input video clock variation than video clock
variation in DP CTS spec.
This patch decreases the K value of ANX7625 which will shrink eDP Tx
video clock variation to meet IVO panel's requirement.
Acked-by: Sam Ravnborg
Signed-off-by: Xin Ji
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 24 ---
drivers/gpu/drm/bridge/analogix/anx7625.h | 4 +++-
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index a3d82377066b..9b9e3984dd38 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -384,6 +384,25 @@ static int anx7625_odfc_config(struct anx7625_data *ctx,
return ret;
}
+/*
+ * The MIPI source video data exist large variation (e.g. 59Hz ~ 61Hz),
+ * anx7625 defined K ratio for matching MIPI input video clock and
+ * DP output video clock. Increase K value can match bigger video data
+ * variation. IVO panel has small variation than DP CTS spec, need
+ * decrease the K value.
+ */
+static int anx7625_set_k_value(struct anx7625_data *ctx)
+{
+ struct edid *edid = (struct edid *)ctx->slimport_edid_p.edid_raw_data;
+
+ if (edid->mfg_id[0] == IVO_MID0 && edid->mfg_id[1] == IVO_MID1)
+ return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
+MIPI_DIGITAL_ADJ_1, 0x3B);
+
+ return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
+MIPI_DIGITAL_ADJ_1, 0x3D);
+}
+
static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx)
{
struct device *dev = >client->dev;
@@ -470,9 +489,8 @@ static int anx7625_dsi_video_timing_config(struct
anx7625_data *ctx)
MIPI_PLL_N_NUM_15_8, (n >> 8) & 0xff);
ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client, MIPI_PLL_N_NUM_7_0,
(n & 0xff));
- /* Diff */
- ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
- MIPI_DIGITAL_ADJ_1, 0x3D);
+
+ anx7625_set_k_value(ctx);
ret |= anx7625_odfc_config(ctx, post_divider - 1);
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 034c3840028f..6dcf64c703f9 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -210,7 +210,9 @@
#define MIPI_VIDEO_STABLE_CNT 0x0A
#define MIPI_LANE_CTRL_10 0x0F
-#define MIPI_DIGITAL_ADJ_1 0x1B
+#define MIPI_DIGITAL_ADJ_1 0x1B
+#define IVO_MID0 0x26
+#define IVO_MID1 0xCF
#define MIPI_PLL_M_NUM_23_16 0x1E
#define MIPI_PLL_M_NUM_15_80x1F
--
2.25.1
[PULL] drm-intel-gt-next
Hi Dave & Daniel, Sorry for the big PR in advance. Had the summer vacations and did not notice until tool late how many patches were in already before leaving. As requested, there is a lot of refactoring to increase the use of TTM allocator and prep for DRM scheduler. Note that at times the patches trade simplicity for performance and revert optimizations not seen as critical. So some performance regressions are expected. As an example is dropping of faster GPU relocation path for older platforms, which should be mitigated by updating to the latest UMD versions to regain the perf. This PR drops various bits of uAPI where userspace has dropped the ball after reviews have been completed on both sides (Thanks Jason for doing the due-diligence!). [1] Due to the complexity of tracing back who used what, I think we could do better in the future to avoid such situations to begin with. See below for my suggestion [2]. In addition to the refactoring and uAPI cleanup there is preliminay code for ADL-P/XeHP and DG2 platforms. Fixes for ADL-S and removal of CNL code. A couple of fixes that have been Cc: stable already. Removing unnecessary workarounds per stepping and adding missing for Gen12 iGFX. I915_MMAP_OFFSET_FIXED is also added to to align with the static/fixed caching mode per BO instead of per-mapping mode (for dGFX only). There is GuC firmware interface update and backend code major rework that unblock enabling GuC on Gen11 (not on by default). Then there is the addition of the GuCRC power management feature which can be enabled for Gen12+ when submission is enabled. Then there is finally the drm-next backmerge and 3 topic branch merges. I think that is mostly all. I tried to summarize much in the tag description so it should hopefully not be horribly long... Regards, Joonas [1] Given that Jason is only human and there is no way to automate this analysis, we may have to bring something back as fixes if we find breakage (like with the MMAP IOCTL). [2] As we first require to merge the kernel code, should we introduce some further rules that the userspace has to land their code before the final kernel version release? If that is not followed through, we would neuter the new uAPI with as small patch as possible in the final release candidate and then remove it in next release. Thoughts? *** drm-intel-gt-next-2021-08-06-1: UAPI Changes: - Add I915_MMAP_OFFSET_FIXED On devices with local memory `I915_MMAP_OFFSET_FIXED` is the only valid type. On devices without local memory, this caching mode is invalid. As caching mode when specifying `I915_MMAP_OFFSET_FIXED`, WC or WB will be used, depending on the object placement on creation. WB will be used when the object can only exist in system memory, WC otherwise. Userspace: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11888 - Reinstate the mmap ioctl for (already released) integrated Gen12 platforms Rationale: Otherwise media driver breaks eg. for ADL-P. Long term goal is still to sunset the IOCTL even for integrated and require using mmap_offset. - Reject caching/set_domain IOCTLs on discrete Expected to become immutable property of the BO - Disallow changing context parameters after first use on Gen12 and earlier - Require setting context parameters at creation on platforms after Gen12 Rationale (for both): Allow less dynamic changes to the context to simplify the implementation and avoid user shooting theirselves in the foot. - Drop I915_CONTEXT_PARAM_RINGSIZE Userspace PR for compute-driver has not been merged - Drop I915_CONTEXT_PARAM_NO_ZEROMAP Userspace PR for libdrm / Beignet was never landed - Drop CONTEXT_CLONE API Userspace PR for Mesa was never landed - Drop getparam support for I915_CONTEXT_PARAM_ENGINES Only existed for symmetry wrt. setparam, never used. - Disallow bonding of virtual engines Drop the prep work, no hardware has been released needing it. - (Implicit) Disable gpu relocations Media userspace was the last userspace to still use them. They have converted so performance can be regained with an update. Core Changes: - Merge topic branch 'topic/i915-ttm-2021-06-11' (from Maarten) - Merge topic branch 'topic/revid_steppings' (from Matt R) - Merge topic branch 'topic/xehp-dg2-definitions-2021-07-21' (from Matt R) - Backmerges drm-next (Rodrigo) Driver Changes: - Initial workarounds for ADL-P (Clint) - Preliminary code for XeHP/DG2 (Stuart, Umesh, Matt R, Prathap, Ram, Venkata, Akeem, Tvrtko, John, Lucas) - Fix ADL-S DMA mask size to 39 bits (Tejas) - Remove code for CNL (Lucas) - Add ADL-P GuC/HuC firmwares (John) - Update HuC to 7.9.3 for TGL/ADL-S/RKL (John) - Fix -EDEADLK handling regression (Ville) - Implement Wa_1508744258 for DG1 and Gen12 iGFX (Jose) - Extend Wa_1406941453 to ADL-S (Jose) - Drop unnecessary workarounds per stepping for SKL/BXT/ICL (Matt R) - Use fuse info to enable SFC on Gen12 (Venkata) - Unconditionally flush the pages on
Re: [PATCH v1 1/1] drm/bridge: anx7625: Tune K value for IVO panel
On Thu, Aug 05, 2021 at 09:33:20PM +0200, Sam Ravnborg wrote: > On Thu, Aug 05, 2021 at 03:30:55PM +0800, Xin Ji wrote: > > IVO panel require less input video clock variation than video clock > > variation in DP CTS spec. > > > > This patch decreases the K value of ANX7625 which will shrink eDP Tx > > video clock variation to meet IVO panel's requirement. > > > > Signed-off-by: Xin Ji > > Looks good, I assume someone else (Robert) picks this. > > Acked-by: Sam Ravnborg > > Sam Hi Sam Ravnborg, OK, thanks, Xin
Re: [PATCH v2 1/1] drm/bridge: anx7625: Tune K value for IVO panel
On Fri, Aug 06, 2021 at 11:42:26AM +0200, Robert Foss wrote:
> Hey Xin,
>
> Thanks for implementing the suggestion so quickly.
>
> Can you send this version of the patch out as v2? Versioning is
> important and both tools and processes break if different versions
> aren't submitted in different emails.
Hi Robert Foss, OK, thanks.
Xin
>
> On Fri, 6 Aug 2021 at 11:35, Xin Ji wrote:
> >
> > IVO panel require less input video clock variation than video clock
> > variation in DP CTS spec.
> >
> > This patch decreases the K value of ANX7625 which will shrink eDP Tx
> > video clock variation to meet IVO panel's requirement.
> >
> > Acked-by: Sam Ravnborg
> > Signed-off-by: Xin Ji
> > ---
> > drivers/gpu/drm/bridge/analogix/anx7625.c | 24 ---
> > drivers/gpu/drm/bridge/analogix/anx7625.h | 4 +++-
> > 2 files changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index a3d82377066b..9b9e3984dd38 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > @@ -384,6 +384,25 @@ static int anx7625_odfc_config(struct anx7625_data
> > *ctx,
> > return ret;
> > }
> >
> > +/*
> > + * The MIPI source video data exist large variation (e.g. 59Hz ~ 61Hz),
> > + * anx7625 defined K ratio for matching MIPI input video clock and
> > + * DP output video clock. Increase K value can match bigger video data
> > + * variation. IVO panel has small variation than DP CTS spec, need
> > + * decrease the K value.
> > + */
> > +static int anx7625_set_k_value(struct anx7625_data *ctx)
> > +{
> > + struct edid *edid = (struct edid
> > *)ctx->slimport_edid_p.edid_raw_data;
> > +
> > + if (edid->mfg_id[0] == IVO_MID0 && edid->mfg_id[1] == IVO_MID1)
> > + return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> > +MIPI_DIGITAL_ADJ_1, 0x3B);
> > +
> > + return anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> > +MIPI_DIGITAL_ADJ_1, 0x3D);
> > +}
> > +
> > static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx)
> > {
> > struct device *dev = >client->dev;
> > @@ -470,9 +489,8 @@ static int anx7625_dsi_video_timing_config(struct
> > anx7625_data *ctx)
> > MIPI_PLL_N_NUM_15_8, (n >> 8) & 0xff);
> > ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> > MIPI_PLL_N_NUM_7_0,
> > (n & 0xff));
> > - /* Diff */
> > - ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p1_client,
> > - MIPI_DIGITAL_ADJ_1, 0x3D);
> > +
> > + anx7625_set_k_value(ctx);
> >
> > ret |= anx7625_odfc_config(ctx, post_divider - 1);
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h
> > b/drivers/gpu/drm/bridge/analogix/anx7625.h
> > index 034c3840028f..6dcf64c703f9 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.h
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
> > @@ -210,7 +210,9 @@
> > #define MIPI_VIDEO_STABLE_CNT 0x0A
> >
> > #define MIPI_LANE_CTRL_10 0x0F
> > -#define MIPI_DIGITAL_ADJ_1 0x1B
> > +#define MIPI_DIGITAL_ADJ_1 0x1B
> > +#define IVO_MID0 0x26
> > +#define IVO_MID1 0xCF
> >
> > #define MIPI_PLL_M_NUM_23_16 0x1E
> > #define MIPI_PLL_M_NUM_15_80x1F
> > --
> > 2.25.1
> >
Re: [V2][PATCH] drm: xlnx: zynqmp: release reset to DP controller before accessing DP registers
Hi, st 24. 3. 2021 v 4:15 odesílatel Laurent Pinchart napsal: > > Hi Quanyang, > > Thank you for the patch. > > On Tue, Mar 23, 2021 at 10:55:01AM +0800, quanyang.w...@windriver.com wrote: > > From: Quanyang Wang > > > > When insmod zynqmp-dpsub.ko after rmmod it, system will hang with the > > error log as below: > > > > root@xilinx-zynqmp:~# insmod zynqmp-dpsub.ko > > [ 88.391289] [drm] Initialized zynqmp-dpsub 1.0.0 20130509 for > > fd4a.display on minor 0 > > [ 88.529906] Console: switching to colour frame buffer device 128x48 > > [ 88.549402] zynqmp-dpsub fd4a.display: [drm] fb0: zynqmp-dpsubdrm > > frame buffer device > > [ 88.571624] zynqmp-dpsub fd4a.display: ZynqMP DisplayPort Subsystem > > driver probed > > root@xilinx-zynqmp:~# rmmod zynqmp_dpsub > > [ 94.023404] Console: switching to colour dummy device 80x25 > > root@xilinx-zynqmp:~# insmod zynqmp-dpsub.ko > > > > > > This is because that in zynqmp_dp_probe it tries to access some DP > > registers while the DP controller is still in the reset state. When > > running "rmmod zynqmp_dpsub", zynqmp_dp_reset(dp, true) in > > zynqmp_dp_phy_exit is called to force the DP controller into the reset > > state. Then insmod will call zynqmp_dp_probe to program the DP registers, > > but at this moment the DP controller hasn't been brought out of the reset > > state yet since the function zynqmp_dp_reset(dp, false) is called later and > > this will result the system hang. > > > > Releasing the reset to DP controller before any read/write operation to it > > will fix this issue. And for symmetry, move zynqmp_dp_reset() call from > > zynqmp_dp_phy_exit() to zynqmp_dp_remove(). > > > > Signed-off-by: Quanyang Wang > > Reviewed-by: Laurent Pinchart Can someone pick this patch? Thanks, Michal
Re: [PATCH] drm/amdgpu: check for allocation failure in amdgpu_vkms_sw_init()
Am 06.08.21 um 17:05 schrieb Dan Carpenter:
Check whether the kcalloc() fails and return -ENOMEM if it does.
Fixes: eeba0b9046fc ("drm/amdgpu: create amdgpu_vkms (v4)")
Signed-off-by: Dan Carpenter
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
index 50bdc39733aa..ce982afeff91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
@@ -482,6 +482,8 @@ static int amdgpu_vkms_sw_init(void *handle)
return r;
adev->amdgpu_vkms_output = kcalloc(adev->mode_info.num_crtc, sizeof(struct amdgpu_vkms_output), GFP_KERNEL);
Is the line above not a bit long?
+ if (!adev->amdgpu_vkms_output)
+ return -ENOMEM;
/* allocate crtcs, encoders, connectors */
for (i = 0; i < adev->mode_info.num_crtc; i++) {
[PATCH] drm/amdgpu: check for allocation failure in amdgpu_vkms_sw_init()
Check whether the kcalloc() fails and return -ENOMEM if it does.
Fixes: eeba0b9046fc ("drm/amdgpu: create amdgpu_vkms (v4)")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
index 50bdc39733aa..ce982afeff91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
@@ -482,6 +482,8 @@ static int amdgpu_vkms_sw_init(void *handle)
return r;
adev->amdgpu_vkms_output = kcalloc(adev->mode_info.num_crtc,
sizeof(struct amdgpu_vkms_output), GFP_KERNEL);
+ if (!adev->amdgpu_vkms_output)
+ return -ENOMEM;
/* allocate crtcs, encoders, connectors */
for (i = 0; i < adev->mode_info.num_crtc; i++) {
--
2.20.1
Re: [PATCH v2 08/14] soc: mediatek: add mtk-mmsys config API for mt8195 vdosys1
On 22/07/2021 11:45, Nancy.Lin wrote:
> Add mmsys config API.
This patch is doing a lot of things, it adds a "config" and it adds cmdq
support. Please explain better in the commit message what the config is for.
Please add comments to the different values of struct mtk_mmsys_config.
I understand that cmdq is optional, so please make addition to cmdq a separate
patch.
I'm a bit puzzled about that fact, can you please explain who you get the HW to
behave the same way when you write the same value and offset to mmsys-regs and
via cmdq.
Thanks,
Matthias
>
> Signed-off-by: Nancy.Lin
> ---
> drivers/soc/mediatek/mt8195-mmsys.h| 38
> drivers/soc/mediatek/mtk-mmsys.c | 50 ++
> drivers/soc/mediatek/mtk-mmsys.h | 10 ++
> include/linux/soc/mediatek/mtk-mmsys.h | 18 ++
> 4 files changed, 116 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mt8195-mmsys.h
> b/drivers/soc/mediatek/mt8195-mmsys.h
> index 104ba575f765..4bdb2087250c 100644
> --- a/drivers/soc/mediatek/mt8195-mmsys.h
> +++ b/drivers/soc/mediatek/mt8195-mmsys.h
> @@ -154,6 +154,18 @@
> #define DISP_DP_INTF0_SEL_IN_FROM_VDO0_MERGE_DL_ASYNC_MOUT (1 << 0)
> #define DISP_DP_INTF0_SEL_IN_FROM_VDO0_DSC_DL_ASYNC_MOUT (2 << 0)
>
> +#define MT8195_VDO1_MERGE0_ASYNC_CFG_WD 0xe30
> +#define MT8195_VDO1_MERGE1_ASYNC_CFG_WD 0xe40
> +#define MT8195_VDO1_MERGE2_ASYNC_CFG_WD 0xe50
> +#define MT8195_VDO1_MERGE3_ASYNC_CFG_WD 0xe60
> +#define MT8195_VDO1_HDRBE_ASYNC_CFG_WD 0xe70
> +#define MT8195_VDO1_HDR_TOP_CFG 0xd00
> +#define MT8195_VDO1_MIXER_IN1_ALPHA 0xd30
> +#define MT8195_VDO1_MIXER_IN2_ALPHA 0xd34
> +#define MT8195_VDO1_MIXER_IN3_ALPHA 0xd38
> +#define MT8195_VDO1_MIXER_IN4_ALPHA 0xd3c
> +#define MT8195_VDO1_MIXER_IN4_PAD0xd4c
> +
> static const struct mtk_mmsys_routes mmsys_mt8195_routing_table[] = {
> {
> DDP_COMPONENT_OVL0, DDP_COMPONENT_RDMA0,
> @@ -261,4 +273,30 @@ static const struct mtk_mmsys_routes
> mmsys_mt8195_routing_table[] = {
> }
> };
>
> +static const struct mtk_mmsys_config mmsys_mt8195_config_table[] = {
> + { MMSYS_CONFIG_MERGE_ASYNC_WIDTH, 0, MT8195_VDO1_MERGE0_ASYNC_CFG_WD,
> GENMASK(13, 0), 0},
> + { MMSYS_CONFIG_MERGE_ASYNC_HEIGHT, 0, MT8195_VDO1_MERGE0_ASYNC_CFG_WD,
> GENMASK(29, 16), 16},
> + { MMSYS_CONFIG_MERGE_ASYNC_WIDTH, 1, MT8195_VDO1_MERGE1_ASYNC_CFG_WD,
> GENMASK(13, 0), 0},
> + { MMSYS_CONFIG_MERGE_ASYNC_HEIGHT, 1, MT8195_VDO1_MERGE1_ASYNC_CFG_WD,
> GENMASK(29, 16), 16},
> + { MMSYS_CONFIG_MERGE_ASYNC_WIDTH, 2, MT8195_VDO1_MERGE2_ASYNC_CFG_WD,
> GENMASK(13, 0), 0},
> + { MMSYS_CONFIG_MERGE_ASYNC_HEIGHT, 2, MT8195_VDO1_MERGE2_ASYNC_CFG_WD,
> GENMASK(29, 16), 16},
> + { MMSYS_CONFIG_MERGE_ASYNC_WIDTH, 3, MT8195_VDO1_MERGE3_ASYNC_CFG_WD,
> GENMASK(13, 0), 0},
> + { MMSYS_CONFIG_MERGE_ASYNC_HEIGHT, 3, MT8195_VDO1_MERGE3_ASYNC_CFG_WD,
> GENMASK(29, 16), 16},
> + { MMSYS_CONFIG_HDR_BE_ASYNC_WIDTH, 0, MT8195_VDO1_HDRBE_ASYNC_CFG_WD,
> GENMASK(13, 0), 0},
> + { MMSYS_CONFIG_HDR_BE_ASYNC_HEIGHT, 0, MT8195_VDO1_HDRBE_ASYNC_CFG_WD,
> GENMASK(29, 16), 16},
> + { MMSYS_CONFIG_MIXER_IN_ALPHA_ODD, 1, MT8195_VDO1_MIXER_IN1_ALPHA,
> GENMASK(8, 0), 0},
> + { MMSYS_CONFIG_MIXER_IN_ALPHA_EVEN, 1, MT8195_VDO1_MIXER_IN1_ALPHA,
> GENMASK(24, 16), 16},
> + { MMSYS_CONFIG_MIXER_IN_ALPHA_ODD, 2, MT8195_VDO1_MIXER_IN2_ALPHA,
> GENMASK(8, 0), 0},
> + { MMSYS_CONFIG_MIXER_IN_ALPHA_EVEN, 2, MT8195_VDO1_MIXER_IN2_ALPHA,
> GENMASK(24, 16), 16},
> + { MMSYS_CONFIG_MIXER_IN_ALPHA_ODD, 3, MT8195_VDO1_MIXER_IN3_ALPHA,
> GENMASK(8, 0), 0},
> + { MMSYS_CONFIG_MIXER_IN_ALPHA_EVEN, 3, MT8195_VDO1_MIXER_IN3_ALPHA,
> GENMASK(24, 16), 16},
> + { MMSYS_CONFIG_MIXER_IN_ALPHA_ODD, 4, MT8195_VDO1_MIXER_IN4_ALPHA,
> GENMASK(8, 0), 0},
> + { MMSYS_CONFIG_MIXER_IN_ALPHA_EVEN, 4, MT8195_VDO1_MIXER_IN4_ALPHA,
> GENMASK(24, 16), 16},
> + { MMSYS_CONFIG_MIXER_IN_CH_SWAP, 4, MT8195_VDO1_MIXER_IN4_PAD,
> GENMASK(4, 4), 4},
> + { MMSYS_CONFIG_HDR_ALPHA_SEL, 1, MT8195_VDO1_HDR_TOP_CFG, GENMASK(20,
> 20), 20},
> + { MMSYS_CONFIG_HDR_ALPHA_SEL, 2, MT8195_VDO1_HDR_TOP_CFG, GENMASK(21,
> 21), 21},
> + { MMSYS_CONFIG_HDR_ALPHA_SEL, 3, MT8195_VDO1_HDR_TOP_CFG, GENMASK(22,
> 22), 22},
> + { MMSYS_CONFIG_HDR_ALPHA_SEL, 4, MT8195_VDO1_HDR_TOP_CFG, GENMASK(23,
> 23), 23},
> +};
> +
> #endif /* __SOC_MEDIATEK_MT8195_MMSYS_H */
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c
> b/drivers/soc/mediatek/mtk-mmsys.c
> index 9e31aad6c5c8..d0f4a407f8f8 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -63,10 +63,13 @@ static const struct mtk_mmsys_driver_data
> mt8195_vdosys1_driver_data = {
> .clk_driver = "clk-mt8195-vdo1",
> .routes = mmsys_mt8195_routing_table,
> .num_routes = ARRAY_SIZE(mmsys_mt8195_routing_table),
> + .config =
Re: [Intel-gfx] [PATCH 0/4] Enable GuC submission by default on DG1
On 8/6/21 1:34 PM, Thomas Hellström (Intel) wrote: Hi, On 8/3/21 7:26 PM, Matthew Brost wrote: On Tue, Aug 03, 2021 at 02:15:13PM +0200, Daniel Vetter wrote: On Tue, Aug 3, 2021 at 6:53 AM Matthew Brost wrote: Minimum set of patches to enable GuC submission on DG1 and enable it by default. A little difficult to test as IGTs do not work with DG1 due to a bunch of uAPI features being disabled (e.g. relocations, caching memory options, etc...). Matt Auld has an igt series which fixes a lot of this stuff, would be good to do at least a Test-With run with that. It looks like Maarten now merged Matt's series to IGT. There is a series on IGT trybot with pending work to have some igt tests support relocations, https://patchwork.freedesktop.org/series/92043/ One of the tests that have WIP fixes is gem_exec_whisper, and that particular test has historically shown occasional hangs with GuC submission on DG1 so it would be very desirable if we could make that test in particular work (I haven't verified that that's the case) reliably. Also the following series: https://patchwork.freedesktop.org/series/93455/ tries a bit harder to get some more tests running, squashing the above series on top of latest IGT. Thanks, /Thomas Just verified that gem-exec-whisper is running now on DG1 on latest igt and the above series. Haven't tried with GuC submission, though. /Thomas
[PATCH] drm/amd/display: Remove redundant initialization of variable eng_id
From: Colin Ian King
The variable eng_id is being initialized with a value that is never
read, it is being re-assigned on the next statment. The assignment
is redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
index 1a89d565c92e..de80a9ea4cfa 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
@@ -305,7 +305,7 @@ struct link_encoder *link_enc_cfg_get_next_avail_link_enc(
const struct dc_state *state)
{
struct link_encoder *link_enc = NULL;
- enum engine_id eng_id = ENGINE_ID_UNKNOWN;
+ enum engine_id eng_id;
eng_id = find_first_avail_link_enc(dc->ctx, state);
if (eng_id != ENGINE_ID_UNKNOWN)
--
2.31.1
Re: [RFC PATCH 13/15] mm: convert MAX_ORDER sized static arrays to dynamic ones.
On 6 Aug 2021, at 5:37, Christian König wrote: > Am 05.08.21 um 21:58 schrieb Zi Yan: >> On 5 Aug 2021, at 15:16, Christian König wrote: >> >>> Am 05.08.21 um 21:02 schrieb Zi Yan: From: Zi Yan This prepares for the upcoming changes to make MAX_ORDER a boot time parameter instead of compilation time constant. All static arrays with MAX_ORDER size are converted to pointers and their memory is allocated at runtime. >>> Well in general I strongly suggest to not use the patter >>> kmalloc(sizeof(some struct) * MAX_ORDER,...) instead use kmalloc_array, >>> kcalloc etc.. >>> >>> Then when a array is embedded at the end of a structure you can use a >>> trailing array and the struct_size() macro to determine the allocation size. >> Sure. Will fix it. >> >>> Additional to that separating the patch into changes for TTM to make the >>> maximum allocation order independent from MAX_ORDER would be rather good to >>> have I think. >> Can you elaborate a little bit more on “make the maximum allocation order >> independent from MAX_ORDER”? > > My idea was that you have a single patch to give the maximum order as > parameter to the TTM pool. Got it. No problem. > >> From what I understand of ttm_pool_alloc(), it tries to get num_pages pages >> by allocating as large pages as possible, starting from MAX_ORDER. What is >> the rationale behind this algorithm? Why not just call alloc_page(order=0) >> num_pages times? > > What we do here is essentially transparent huge pages for GPUs. > > In opposite to CPU which can usually only use fixed sizes like 4KiB, 2MiB, > 1GiB at least AMD GPUs can use any power of two. FYI, Matthew Wilcox’s memory folio patchset adds support for arbitrary THP sizes[1]. You might want to check it out. :) > > So it makes sense to allocate big pages as much as possible and only fallback > to 4KiB pages when necessary. > > Down side is that we potentially exhaust larger orders for other use cases. > >> Is it mean to reduce the number of calls to alloc_page? > > That is a nice to have side effect, but the performance improvement for the > TLB is the main reason here. > >> The allocated pages do not need to get as high as MAX_ORDER, is that the >> case? > > Actually we would really like to have pages as large as 1GiB for best TLB > utilization :) > >> If yes, I probably can keep ttm pool as static arrays with length of >> MIN_MAX_ORDER, which I introduce in Patch 14 as the lower bound of boot time >> parameter MAX_ORDER. Let me know your thoughts. > > Well you could do something like MAX_MAX_ORDER with a value of at least 19 > (1GiB with 4KiB pages IIRC). And then limit to the real available max_order > when you make that a run time option. > > Since the array elements consists only of a linked list and a few extra > parameters / pointers we probably won't need more than a page or two for > those. Thanks for you explanation. Now I understand that ttm_pool does want pages as large as possible for performance reasons. I will keep the existing code, so that ttm_pool can get the largest pages from buddy allocator. I will separate the changes to TTM to a single patch like you suggested. [1] https://lore.kernel.org/linux-mm/20210715033704.692967-1-wi...@infradead.org — Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature
linux-next: manual merge of the drm tree with the drm-fixes tree
Hi all,
Today's linux-next merge of the drm tree got a conflict in:
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
between commit:
e00f543d3596 ("drm/amdgpu: add DID for beige goby")
from the drm-fixes tree and commit:
a8f706966b92 ("drm/amdgpu: add pci device id for cyan_skillfish")
from the drm tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
diff --cc drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 5ed8381ae0f5,d637b0536f84..
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@@ -1213,13 -1212,9 +1212,16 @@@ static const struct pci_device_id pciid
{0x1002, 0x740F, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_ALDEBARAN|AMD_EXP_HW_SUPPORT},
{0x1002, 0x7410, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_ALDEBARAN|AMD_EXP_HW_SUPPORT},
+ /* BEIGE_GOBY */
+ {0x1002, 0x7420, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+ {0x1002, 0x7421, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+ {0x1002, 0x7422, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+ {0x1002, 0x7423, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+ {0x1002, 0x743F, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+
+ /* CYAN_SKILLFISH */
+ {0x1002, 0x13FE, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_CYAN_SKILLFISH|AMD_IS_APU},
+
{0, 0, 0}
};
Re: [PATCH v6 2/7] soc: mediatek: add mtk-mmsys support for mt8195 vdosys0
Hi Jason, On 05/08/2021 22:52, jason-jh.lin wrote: > Add mt8195 vdosys0 clock driver name and routing table to > the driver data of mtk-mmsys. > I'd like to see the implementation of vdosys1 as well, to better understand why we need two compatibles. > Signed-off-by: jason-jh.lin > --- > This patch is base on [1] > > [1] dt-bindings: arm: mediatek: mmsys: add mt8195 SoC binding > https://patchwork.kernel.org/project/linux-mediatek/patch/20210805171346.24249-2-jason-jh@mediatek.com/ Please add the binding description to this series. > --- > drivers/soc/mediatek/mt8195-mmsys.h| 96 ++ > drivers/soc/mediatek/mtk-mmsys.c | 11 +++ > include/linux/soc/mediatek/mtk-mmsys.h | 9 +++ > 3 files changed, 116 insertions(+) > create mode 100644 drivers/soc/mediatek/mt8195-mmsys.h > > diff --git a/drivers/soc/mediatek/mt8195-mmsys.h > b/drivers/soc/mediatek/mt8195-mmsys.h > new file mode 100644 > index ..9339a786ec5d > --- /dev/null > +++ b/drivers/soc/mediatek/mt8195-mmsys.h > @@ -0,0 +1,96 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > + > +#ifndef __SOC_MEDIATEK_MT8195_MMSYS_H > +#define __SOC_MEDIATEK_MT8195_MMSYS_H > + > +#define MT8195_VDO0_OVL_MOUT_EN 0xf14 > +#define MT8195_MOUT_DISP_OVL0_TO_DISP_RDMA0 BIT(0) > +#define MT8195_MOUT_DISP_OVL0_TO_DISP_WDMA0 BIT(1) > +#define MT8195_MOUT_DISP_OVL0_TO_DISP_OVL1 BIT(2) > +#define MT8195_MOUT_DISP_OVL1_TO_DISP_RDMA1 BIT(4) > +#define MT8195_MOUT_DISP_OVL1_TO_DISP_WDMA1 BIT(5) > +#define MT8195_MOUT_DISP_OVL1_TO_DISP_OVL0 BIT(6) > + > +#define MT8195_VDO0_SEL_IN 0xf34 > +#define MT8195_SEL_IN_VPP_MERGE_FROM_DSC_WRAP0_OUT (0 << 0) > +#define MT8195_SEL_IN_VPP_MERGE_FROM_DISP_DITHER1(1 << 0) > +#define MT8195_SEL_IN_VPP_MERGE_FROM_VDO1_VIRTUAL0 (2 << 0) > +#define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_DISP_DITHER0 (0 << 4) > +#define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_VPP_MERGE(1 << 4) > +#define MT8195_SEL_IN_DSC_WRAP1_IN_FROM_DISP_DITHER1 (0 << 5) > +#define MT8195_SEL_IN_DSC_WRAP1_IN_FROM_VPP_MERGE(1 << 5) > +#define MT8195_SEL_IN_SINA_VIRTUAL0_FROM_VPP_MERGE (0 << 8) > +#define MT8195_SEL_IN_SINA_VIRTUAL0_FROM_DSC_WRAP1_OUT (1 << 8) > +#define MT8195_SEL_IN_SINB_VIRTUAL0_FROM_DSC_WRAP0_OUT (0 << 9) > +#define MT8195_SEL_IN_DP_INTF0_FROM_DSC_WRAP1_OUT(0 << 12) > +#define MT8195_SEL_IN_DP_INTF0_FROM_VPP_MERGE(1 << > 12) > +#define MT8195_SEL_IN_DP_INTF0_FROM_VDO1_VIRTUAL0(2 << 12) > +#define MT8195_SEL_IN_DSI0_FROM_DSC_WRAP0_OUT(0 << > 16) > +#define MT8195_SEL_IN_DSI0_FROM_DISP_DITHER0 (1 << 16) > +#define MT8195_SEL_IN_DSI1_FROM_DSC_WRAP1_OUT(0 << > 17) > +#define MT8195_SEL_IN_DSI1_FROM_VPP_MERGE(1 << 17) > +#define MT8195_SEL_IN_DISP_WDMA1_FROM_DISP_OVL1 (0 << > 20) > +#define MT8195_SEL_IN_DISP_WDMA1_FROM_VPP_MERGE (1 << > 20) > +#define MT8195_SEL_IN_DSC_WRAP1_OUT_FROM_DSC_WRAP1_IN(0 << > 21) > +#define MT8195_SEL_IN_DSC_WRAP1_OUT_FROM_DISP_DITHER1(1 << > 21) > +#define MT8195_SEL_IN_DISP_WDMA0_FROM_DISP_OVL0 (0 << > 22) > +#define MT8195_SEL_IN_DISP_WDMA0_FROM_VPP_MERGE (1 << > 22) > + > +#define MT8195_VDO0_SEL_OUT 0xf38 > +#define MT8195_SOUT_DISP_DITHER0_TO_DSC_WRAP0_IN (0 << 0) > +#define MT8195_SOUT_DISP_DITHER0_TO_DSI0 (1 << 0) > +#define MT8195_SOUT_DISP_DITHER1_TO_DSC_WRAP1_IN (0 << 1) > +#define MT8195_SOUT_DISP_DITHER1_TO_VPP_MERGE(1 << 1) > +#define MT8195_SOUT_DISP_DITHER1_TO_DSC_WRAP1_OUT(2 << 1) > +#define MT8195_SOUT_VDO1_VIRTUAL0_TO_VPP_MERGE (0 << 4) > +#define MT8195_SOUT_VDO1_VIRTUAL0_TO_DP_INTF0(1 << 4) > +#define MT8195_SOUT_VPP_MERGE_TO_DSI1(0 << 8) > +#define MT8195_SOUT_VPP_MERGE_TO_DP_INTF0(1 << 8) > +#define MT8195_SOUT_VPP_MERGE_TO_SINA_VIRTUAL0 (2 << 8) > +#define MT8195_SOUT_VPP_MERGE_TO_DISP_WDMA1 (3 << 8) > +#define MT8195_SOUT_VPP_MERGE_TO_DSC_WRAP0_IN(4 << 8) > +#define MT8195_SOUT_VPP_MERGE_TO_DSC_WRAP1_IN(0 << > 11) > +#define MT8195_SOUT_VPP_MERGE_TO_DISP_WDMA0 (1 << 11) > +#define MT8195_SOUT_DSC_WRAP0_OUT_TO_DSI0(0 << 12) > +#define MT8195_SOUT_DSC_WRAP0_OUT_TO_SINB_VIRTUAL0 (1 << 12) > +#define MT8195_SOUT_DSC_WRAP0_OUT_TO_VPP_MERGE (2 << > 12) > +#define
Re: [Intel-gfx] [PATCH 0/4] Enable GuC submission by default on DG1
Hi, On 8/3/21 7:26 PM, Matthew Brost wrote: On Tue, Aug 03, 2021 at 02:15:13PM +0200, Daniel Vetter wrote: On Tue, Aug 3, 2021 at 6:53 AM Matthew Brost wrote: Minimum set of patches to enable GuC submission on DG1 and enable it by default. A little difficult to test as IGTs do not work with DG1 due to a bunch of uAPI features being disabled (e.g. relocations, caching memory options, etc...). Matt Auld has an igt series which fixes a lot of this stuff, would be good to do at least a Test-With run with that. It looks like Maarten now merged Matt's series to IGT. There is a series on IGT trybot with pending work to have some igt tests support relocations, https://patchwork.freedesktop.org/series/92043/ One of the tests that have WIP fixes is gem_exec_whisper, and that particular test has historically shown occasional hangs with GuC submission on DG1 so it would be very desirable if we could make that test in particular work (I haven't verified that that's the case) reliably. Also the following series: https://patchwork.freedesktop.org/series/93455/ tries a bit harder to get some more tests running, squashing the above series on top of latest IGT. Thanks, /Thomas
Re: [Intel-gfx] [PATCH 0/4] Enable GuC submission by default on DG1
On 8/6/21 1:34 PM, Thomas Hellström (Intel) wrote: Hi, On 8/3/21 7:26 PM, Matthew Brost wrote: On Tue, Aug 03, 2021 at 02:15:13PM +0200, Daniel Vetter wrote: On Tue, Aug 3, 2021 at 6:53 AM Matthew Brost wrote: Minimum set of patches to enable GuC submission on DG1 and enable it by default. A little difficult to test as IGTs do not work with DG1 due to a bunch of uAPI features being disabled (e.g. relocations, caching memory options, etc...). Matt Auld has an igt series which fixes a lot of this stuff, would be good to do at least a Test-With run with that. It looks like Maarten now merged Matt's series to IGT. There is a series on IGT trybot with pending work to have some igt tests support relocations, https://patchwork.freedesktop.org/series/92043/ One of the tests that have WIP fixes is gem_exec_whisper, and that particular test has historically shown occasional hangs with GuC submission on DG1 so it would be very desirable if we could make that test in particular work (I haven't verified that that's the case) reliably. Also the following series: https://patchwork.freedesktop.org/series/93455/ tries a bit harder to get some more tests running, squashing the above series on top of latest IGT. Thanks, /Thomas And also while we're working on getting igt adapted to uapi changes and to get more LMEM coverage in there, an IMO relevant test case to run manually is "piglit quick"on top of DG1-enabled OpenGL checking for regressions and significant changes in execution time. /Thomas
Re: [PATCH v2 07/14] soc: mediatek: add mtk-mmsys support for mt8195 vdosys1
On 28/07/2021 07:34, Nancy.Lin wrote:
> Hi Enric,
>
> Thanks for your review.
>
> On Fri, 2021-07-23 at 13:05 +0200, Enric Balletbo Serra wrote:
>> Hi Nancy,
>>
>> Thank you for your patch.
>>
>> Missatge de Nancy.Lin del dia dj., 22 de
>> jul.
>> 2021 a les 11:45:
>>>
>>> Add mt8195 vdosys1 clock driver name and routing table to
>>> the driver data of mtk-mmsys.
>>>
>>> Signed-off-by: Nancy.Lin
>>> ---
>>> drivers/soc/mediatek/mt8195-mmsys.h| 83
>>> --
>>> drivers/soc/mediatek/mtk-mmsys.c | 10
>>> include/linux/soc/mediatek/mtk-mmsys.h | 2 +
>>> 3 files changed, 90 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/soc/mediatek/mt8195-mmsys.h
>>> b/drivers/soc/mediatek/mt8195-mmsys.h
>>> index 73e9e8286d50..104ba575f765 100644
>>> --- a/drivers/soc/mediatek/mt8195-mmsys.h
>>> +++ b/drivers/soc/mediatek/mt8195-mmsys.h
>>> @@ -64,16 +64,16 @@
>>> #define SOUT_TO_VPP_MERGE0_P1_SEL (1
>>> << 0)
>>>
>>> #define
>>> MT8195_VDO1_MERGE0_ASYNC_SOUT_SEL 0xf40
>>> -#define SOUT_TO_HDR_VDO_FE0(0
>>> << 0)
>>
>> This definition was introduced in this patch [1] that didn't land
>> yet.
>> And you're removing it now. Could you sync with Jason and only
>> introduce the bits that are needed for your patches. Also all the
>> comments I made to the Jason's patch apply here.
>>
>> [1]
>> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-3-jason-jh@mediatek.com/__;!!CTRNKA9wMg0ARbw!0rDdPxfBPcZC9icK37sCxT55RMqwRngO0BF4-uDwgYZP7UwQkx7iidkINqLBb7yi$
>>
>>
> OK, I will sync with Jason and modify it.
>
>>> +#define SOUT_TO_MIXER_IN1_SEL (1
>>> << 0)
>>>
>>> #define
>>> MT8195_VDO1_MERGE1_ASYNC_SOUT_SEL 0xf44
>>> -#define SOUT_TO_HDR_VDO_FE1(0
>>> << 0)
>>> +#define SOUT_TO_MIXER_IN2_SEL (1
>>> << 0)
>>>
>>> #define
>>> MT8195_VDO1_MERGE2_ASYNC_SOUT_SEL 0xf48
>>> -#define SOUT_TO_HDR_GFX_FE0(0
>>> << 0)
>>> +#define SOUT_TO_MIXER_IN3_SEL (1
>>> << 0)
>>>
>>> #define
>>> MT8195_VDO1_MERGE3_ASYNC_SOUT_SEL 0xf4c
>>> -#define SOUT_TO_HDR_GFX_FE1(0
>>> << 0)
>>> +#define SOUT_TO_MIXER_IN4_SEL (1
>>> << 0)
>>>
>>> #define
>>> MT8195_VDO1_MIXER_IN1_SOUT_SEL 0xf58
>>> #define MIXER_IN1_SOUT_TO_DISP_MIXER (0
>>> << 0)
>>> @@ -88,7 +88,7 @@
>>> #define MIXER_IN4_SOUT_TO_DISP_MIXER (0
>>> << 0)
>>>
>>> #define
>>> MT8195_VDO1_MIXER_OUT_SOUT_SEL 0xf34
>>> -#define MIXER_SOUT_TO_HDR_VDO_BE0 (0
>>> << 0)
>>> +#define MIXER_SOUT_TO_MERGE4_ASYNC_SEL (1
>>> << 0)
>>>
>>> #define
>>> MT8195_VDO1_MERGE4_SOUT_SEL0xf18
>>> #define MERGE4_SOUT_TO_VDOSYS0 (0
>>> << 0)
>>> @@ -185,6 +185,79 @@ static const struct mtk_mmsys_routes
>>> mmsys_mt8195_routing_table[] = {
>>> }, {
>>> DDP_COMPONENT_DSC0, DDP_COMPONENT_MERGE0,
>>> MT8195_VDO0_SEL_OUT,
>>> SOUT_DSC_WRAP0_OUT_TO_VPP_MERGE
>>> + },
>>> + {
>>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5,
>>> + MT8195_VDO1_VPP_MERGE0_P0_SEL_IN,
>>> VPP_MERGE0_P0_SEL_IN_FROM_MDP_RDMA0
>>> + },
>>> + {
>>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5,
>>> + MT8195_VDO1_VPP_MERGE0_P1_SEL_IN,
>>> VPP_MERGE0_P1_SEL_IN_FROM_MDP_RDMA1
>>> + },
>>> + {
>>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5,
>>> + MT8195_VDO1_VPP_MERGE1_P0_SEL_IN,
>>> VPP_MERGE1_P0_SEL_IN_FROM_MDP_RDMA2
>>> + },
>>> + {
>>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5,
>>> + MT8195_VDO1_MERGE0_ASYNC_SOUT_SEL,
>>> SOUT_TO_MIXER_IN1_SEL
>>> + },
>>> + {
>>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5,
>>> + MT8195_VDO1_MERGE1_ASYNC_SOUT_SEL,
>>> SOUT_TO_MIXER_IN2_SEL
>>> + },
>>> + {
>>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5,
>>> + MT8195_VDO1_MERGE2_ASYNC_SOUT_SEL,
>>> SOUT_TO_MIXER_IN3_SEL
>>> + },
>>> + {
>>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5,
>>> + MT8195_VDO1_MERGE3_ASYNC_SOUT_SEL,
>>> SOUT_TO_MIXER_IN4_SEL
>>> + },
>>> + {
>>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5,
>>> + MT8195_VDO1_MIXER_OUT_SOUT_SEL,
>>> MIXER_SOUT_TO_MERGE4_ASYNC_SEL
>>> + },
>>> + {
>>> +
Re: [PATCH v5 1/2] dt-bindings: drm/bridge: anx7625: add force_dsi_end_without_null
On Mon, Aug 02, 2021 at 09:07:10AM +0800, Jitao Shi wrote: > The force_dsi_end_without_null requires the dsi host ent at > the same time in line. > > Signed-off-by: Jitao Shi > --- > .../bindings/display/bridge/analogix,anx7625.yaml | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > index ab48ab2f4240..8b868a6a3d60 100644 > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > @@ -43,6 +43,11 @@ properties: >vdd33-supply: > description: Regulator that provides the supply 3.3V power. > > + force_dsi_end_without_null: > +description: | > + Requires the dsi host send the dsi packets on all lanes aligned > + at the end. Can't this be implied from the compatible string if it is a property of this chip?
Re: [PATCH 1/4] dt-bindings: vendor-prefixes: add Zhishengxin
On Mon, 02 Aug 2021 14:35:35 +0800, Icenowy Zheng wrote: > Shenzhen Zhishengxin Technology Co., Ltd. is a LCD module supplier. > > Add vendor prefix for it. > > Signed-off-by: Icenowy Zheng > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring
Re: [PATCH 2/4] dt-bindings: display: add binding for simple-dbi panels
On Mon, Aug 02, 2021 at 02:35:36PM +0800, Icenowy Zheng wrote: > As we're going to introduce a driver for MIPI DBI panels that need only > standard MIPI-DCS commands to initialize (usually because the controller > has some configuration pre-programmed), add a DT binding file for it, > which now includes only one DBI Type C Option 3 panel, ZSX154-B1206. > > Signed-off-by: Icenowy Zheng > --- > .../bindings/display/simple-dbi.yaml | 72 +++ > 1 file changed, 72 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/simple-dbi.yaml > > diff --git a/Documentation/devicetree/bindings/display/simple-dbi.yaml > b/Documentation/devicetree/bindings/display/simple-dbi.yaml > new file mode 100644 > index ..f49b0fda0935 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/simple-dbi.yaml > @@ -0,0 +1,72 @@ > +# SPDX-License-Identifier: GPL-2.0-only checkpatch.pl will tell you this should be dual licensed.
Re: [PATCH v2 7/7] drm/mediatek: mtk_dsi: Reset the dsi0 hardware
On 14/07/2021 12:11, Enric Balletbo i Serra wrote: > Reset dsi0 HW to default when power on. This prevents to have different > settingbetween the bootloader and the kernel. settings between the... > > As not all Mediatek boards have the reset consumer configured in their > board description, also is not needed on all of them, the reset is optional, > so the change is compatible with all boards. > > Cc: Jitao Shi > Suggested-by: Chun-Kuang Hu > Signed-off-by: Enric Balletbo i Serra Reviewed-by: Matthias Brugger > --- > > (no changes since v1) > > drivers/gpu/drm/mediatek/mtk_dsi.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index ae403c67cbd9..d8b81e2ab841 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -980,8 +981,10 @@ static int mtk_dsi_bind(struct device *dev, struct > device *master, void *data) > struct mtk_dsi *dsi = dev_get_drvdata(dev); > > ret = mtk_dsi_encoder_init(drm, dsi); > + if (ret) > + return ret; > > - return ret; > + return device_reset_optional(dev); > } > > static void mtk_dsi_unbind(struct device *dev, struct device *master, >
Re: [Intel-gfx] [PATCH] drm/i915: Fix syncmap memory leak
On 7/30/2021 12:53, Matthew Brost wrote: A small race exists between intel_gt_retire_requests_timeout and intel_timeline_exit which could result in the syncmap not getting free'd. Rather than work to hard to seal this race, simply cleanup the free'd -> freed syncmap on fini. unreferenced object 0x88813bc53b18 (size 96): comm "gem_close_race", pid 5410, jiffies 4294917818 (age 1105.600s) hex dump (first 32 bytes): 01 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 00 00 00 00 6b 6b 6b 6b 06 00 00 00 backtrace: [<120b863a>] __sync_alloc_leaf+0x1e/0x40 [i915] [<042f6959>] __sync_set+0x1bb/0x240 [i915] [<90f0e90f>] i915_request_await_dma_fence+0x1c7/0x400 [i915] [<56a48219>] i915_request_await_object+0x222/0x360 [i915] [] i915_gem_do_execbuffer+0x1bd0/0x2250 [i915] [<3c9d830f>] i915_gem_execbuffer2_ioctl+0x405/0xce0 [i915] [ ] drm_ioctl_kernel+0xb0/0xf0 [drm] [ ] drm_ioctl+0x305/0x3c0 [drm] [<8b0d8986>] __x64_sys_ioctl+0x71/0xb0 [<76c362a4>] do_syscall_64+0x33/0x80 [ ] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Signed-off-by: Matthew Brost Fixes: 531958f6f357 ("drm/i915/gt: Track timeline activeness in enter/exit") Cc: --- drivers/gpu/drm/i915/gt/intel_timeline.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c index c4a126c8caef..1257f4f11e66 100644 --- a/drivers/gpu/drm/i915/gt/intel_timeline.c +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c @@ -127,6 +127,15 @@ static void intel_timeline_fini(struct rcu_head *rcu) i915_vma_put(timeline->hwsp_ggtt); i915_active_fini(>active); + + /* +* A small race exists between intel_gt_retire_requests_timeout and +* intel_timeline_exit which could result in the syncmap not getting +* free'd. Rather than work to hard to seal this race, simply cleanup +* the syncmap on fini. What is the race? I'm going round in circles just trying to work out how intel_gt_retire_requests_timeout is supposed to get to intel_timeline_exit in the first place. Also, free'd -> freed. John. +*/ + i915_syncmap_free(>sync); + kfree(timeline); }
Re: [git pull] drm fixes for 5.14-rc5
The pull request you sent on Fri, 6 Aug 2021 16:03:00 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-08-06 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/1254f05ce097c9bf2872a8407725346faba59844 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
[PATCH v2 0/3] NVIDIA Tegra NVDEC support
Hi all, here is a v2 of the NVDEC support series. Changes have been done to accommodate review comments on v1, and the NVDEC1 instance on Tegra194 is now supported. The series has been tested on Tegra186 on top of my/Thierry's TegraDRM v8 series (though should work on top of v7 as well). NVDEC hardware documentation can be found at https://github.com/NVIDIA/open-gpu-doc/tree/master/classes/video and example userspace can be found at https://github.com/cyndis/vaapi-tegra-driver Thanks, Mikko Mikko Perttunen (3): dt-bindings: Add YAML bindings for Host1x and NVDEC arm64: tegra: Add NVDEC to Tegra186/194 device trees drm/tegra: Add NVDEC driver .../gpu/host1x/nvidia,tegra20-host1x.yaml | 131 + .../gpu/host1x/nvidia,tegra210-nvdec.yaml | 109 MAINTAINERS | 1 + arch/arm64/boot/dts/nvidia/tegra186.dtsi | 16 + arch/arm64/boot/dts/nvidia/tegra194.dtsi | 36 ++ drivers/gpu/drm/tegra/Makefile| 3 +- drivers/gpu/drm/tegra/drm.c | 4 + drivers/gpu/drm/tegra/drm.h | 1 + drivers/gpu/drm/tegra/nvdec.c | 473 ++ drivers/gpu/host1x/dev.c | 18 + include/linux/host1x.h| 2 + 11 files changed, 793 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml create mode 100644 drivers/gpu/drm/tegra/nvdec.c -- 2.32.0
[PATCH v2 3/3] drm/tegra: Add NVDEC driver
Add support for booting and using NVDEC on Tegra210, Tegra186
and Tegra194 to the Host1x and TegraDRM drivers. Booting in
secure mode is not currently supported.
Signed-off-by: Mikko Perttunen
---
v2:
* Use devm_platform_get_and_ioremap_resource
* Remove reset handling, done by power domain code
* Assume runtime PM is enabled
---
drivers/gpu/drm/tegra/Makefile | 3 +-
drivers/gpu/drm/tegra/drm.c| 4 +
drivers/gpu/drm/tegra/drm.h| 1 +
drivers/gpu/drm/tegra/nvdec.c | 473 +
drivers/gpu/host1x/dev.c | 18 ++
include/linux/host1x.h | 2 +
6 files changed, 500 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/tegra/nvdec.c
diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
index 5d2039f0c734..b248c631f790 100644
--- a/drivers/gpu/drm/tegra/Makefile
+++ b/drivers/gpu/drm/tegra/Makefile
@@ -24,7 +24,8 @@ tegra-drm-y := \
gr2d.o \
gr3d.o \
falcon.o \
- vic.o
+ vic.o \
+ nvdec.o
tegra-drm-y += trace.o
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index b20fd0833661..5f5afd7ba37e 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1337,15 +1337,18 @@ static const struct of_device_id host1x_drm_subdevs[] =
{
{ .compatible = "nvidia,tegra210-sor", },
{ .compatible = "nvidia,tegra210-sor1", },
{ .compatible = "nvidia,tegra210-vic", },
+ { .compatible = "nvidia,tegra210-nvdec", },
{ .compatible = "nvidia,tegra186-display", },
{ .compatible = "nvidia,tegra186-dc", },
{ .compatible = "nvidia,tegra186-sor", },
{ .compatible = "nvidia,tegra186-sor1", },
{ .compatible = "nvidia,tegra186-vic", },
+ { .compatible = "nvidia,tegra186-nvdec", },
{ .compatible = "nvidia,tegra194-display", },
{ .compatible = "nvidia,tegra194-dc", },
{ .compatible = "nvidia,tegra194-sor", },
{ .compatible = "nvidia,tegra194-vic", },
+ { .compatible = "nvidia,tegra194-nvdec", },
{ /* sentinel */ }
};
@@ -1369,6 +1372,7 @@ static struct platform_driver * const drivers[] = {
_gr2d_driver,
_gr3d_driver,
_vic_driver,
+ _nvdec_driver,
};
static int __init host1x_drm_init(void)
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 8b28327c931c..fc0a19554eac 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -202,5 +202,6 @@ extern struct platform_driver tegra_sor_driver;
extern struct platform_driver tegra_gr2d_driver;
extern struct platform_driver tegra_gr3d_driver;
extern struct platform_driver tegra_vic_driver;
+extern struct platform_driver tegra_nvdec_driver;
#endif /* HOST1X_DRM_H */
diff --git a/drivers/gpu/drm/tegra/nvdec.c b/drivers/gpu/drm/tegra/nvdec.c
new file mode 100644
index ..4a58b5357473
--- /dev/null
+++ b/drivers/gpu/drm/tegra/nvdec.c
@@ -0,0 +1,473 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2015-2021, NVIDIA Corporation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "drm.h"
+#include "falcon.h"
+#include "vic.h"
+
+struct nvdec_config {
+ const char *firmware;
+ unsigned int version;
+ bool supports_sid;
+ int num_instances;
+};
+
+struct nvdec {
+ struct falcon falcon;
+
+ void __iomem *regs;
+ struct tegra_drm_client client;
+ struct host1x_channel *channel;
+ struct device *dev;
+ struct clk *clk;
+
+ /* Platform configuration */
+ const struct nvdec_config *config;
+};
+
+static inline struct nvdec *to_nvdec(struct tegra_drm_client *client)
+{
+ return container_of(client, struct nvdec, client);
+}
+
+static void nvdec_writel(struct nvdec *nvdec, u32 value, unsigned int offset)
+{
+ writel(value, nvdec->regs + offset);
+}
+
+static int nvdec_boot(struct nvdec *nvdec)
+{
+#ifdef CONFIG_IOMMU_API
+ struct iommu_fwspec *spec = dev_iommu_fwspec_get(nvdec->dev);
+#endif
+ int err = 0;
+
+#ifdef CONFIG_IOMMU_API
+ if (nvdec->config->supports_sid && spec) {
+ u32 value;
+
+ value = TRANSCFG_ATT(1, TRANSCFG_SID_FALCON) |
+ TRANSCFG_ATT(0, TRANSCFG_SID_HW);
+ nvdec_writel(nvdec, value, VIC_TFBIF_TRANSCFG);
+
+ if (spec->num_ids > 0) {
+ value = spec->ids[0] & 0x;
+
+ nvdec_writel(nvdec, value, VIC_THI_STREAMID0);
+ nvdec_writel(nvdec, value, VIC_THI_STREAMID1);
+ }
+ }
+#endif
+
+ err = falcon_boot(>falcon);
+ if (err < 0)
+ return err;
+
+ err = falcon_wait_idle(>falcon);
+ if (err < 0) {
+ dev_err(nvdec->dev,
+ "failed to set application ID and
[PATCH v2 1/3] dt-bindings: Add YAML bindings for Host1x and NVDEC
Convert the original Host1x bindings to YAML and add new bindings for
NVDEC, now in a more appropriate location. The old text bindings
for Host1x and engines are still kept at display/tegra/ since they
encompass a lot more engines that haven't been converted over yet.
Signed-off-by: Mikko Perttunen
---
v2:
* Fix issues pointed out in v1
* Add T194 nvidia,instance property
---
.../gpu/host1x/nvidia,tegra20-host1x.yaml | 131 ++
.../gpu/host1x/nvidia,tegra210-nvdec.yaml | 109 +++
MAINTAINERS | 1 +
3 files changed, 241 insertions(+)
create mode 100644
Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
create mode 100644
Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
diff --git
a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
new file mode 100644
index ..5344524c26d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/gpu/host1x/nvidia,tegra20-host1x.yaml#;
+$schema: "http://devicetree.org/meta-schemas/core.yaml#;
+
+title: Device tree binding for NVIDIA Host1x
+
+maintainers:
+ - Thierry Reding
+ - Mikko Perttunen
+
+properties:
+ $nodename:
+pattern: "^host1x@[0-9a-f]*$"
+
+ compatible:
+oneOf:
+ - enum:
+ - nvidia,tegra20-host1x
+ - nvidia,tegra30-host1x
+ - nvidia,tegra114-host1x
+ - nvidia,tegra124-host1x
+ - nvidia,tegra210-host1x
+ - items:
+ - const: nvidia,tegra132-host1x
+ - const: nvidia,tegra124-host1x
+
+ interrupts:
+items:
+ - description: Syncpoint threshold interrupt
+ - description: General interrupt
+
+ interrupt-names:
+items:
+ - const: syncpt
+ - const: host1x
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+items:
+ - const: host1x
+
+ resets:
+maxItems: 1
+
+ reset-names:
+items:
+ - const: host1x
+
+ iommus:
+maxItems: 1
+
+ interconnects:
+maxItems: 1
+
+ interconnect-names:
+items:
+ - const: dma-mem
+
+ '#address-cells':
+const: 1
+
+ '#size-cells':
+const: 1
+
+ ranges: true
+
+required:
+ - compatible
+ - interrupts
+ - interrupt-names
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - '#address-cells'
+ - '#size-cells'
+ - ranges
+
+if:
+ properties:
+compatible:
+ enum:
+- nvidia,tegra186-host1x
+- nvidia,tegra194-host1x
+then:
+ properties:
+reg:
+ items:
+- description: Hypervisor-accessible register area
+- description: VM-accessible register area
+reg-names:
+ items:
+- const: hypervisor
+- const: vm
+ required:
+- reg
+- reg-names
+else:
+ properties:
+reg:
+ maxItems: 1
+ required:
+- reg
+
+additionalProperties: true
+
+examples:
+ - |
+#include
+#include
+
+host1x@5000 {
+compatible = "nvidia,tegra20-host1x";
+reg = <0x5000 0x00024000>;
+interrupts = , /* syncpt */
+ ; /* general */
+interrupt-names = "syncpt", "host1x";
+clocks = <_car TEGRA20_CLK_HOST1X>;
+clock-names = "host1x";
+resets = <_car 28>;
+reset-names = "host1x";
+
+#address-cells = <1>;
+#size-cells = <1>;
+
+ranges = <0x5400 0x5400 0x0400>;
+};
diff --git
a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
new file mode 100644
index ..fc535bb7aee0
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/gpu/host1x/nvidia,tegra210-nvdec.yaml#;
+$schema: "http://devicetree.org/meta-schemas/core.yaml#;
+
+title: Device tree binding for NVIDIA Tegra NVDEC
+
+description: |
+ NVDEC is the hardware video decoder present on NVIDIA Tegra210
+ and newer chips. It is located on the Host1x bus and typically
+ programmed through Host1x channels.
+
+maintainers:
+ - Thierry Reding
+ - Mikko Perttunen
+
+properties:
+ $nodename:
+pattern: "^nvdec@[0-9a-f]*$"
+
+ compatible:
+enum:
+ - nvidia,tegra210-nvdec
+ - nvidia,tegra186-nvdec
+ - nvidia,tegra194-nvdec
+
+ reg:
+maxItems: 1
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+items:
+ - const: nvdec
+
+ resets:
+maxItems: 1
+
+ reset-names:
+items:
+ - const: nvdec
+
+ power-domains:
+maxItems: 1
+
+ iommus:
+maxItems: 1
+
+ interconnects:
+items:
+ - description: DMA read memory client
+ -
[PATCH v2 2/3] arm64: tegra: Add NVDEC to Tegra186/194 device trees
Add a device tree node for NVDEC on Tegra186, and
device tree nodes for NVDEC and NVDEC1 on Tegra194.
Signed-off-by: Mikko Perttunen
---
v2:
* Add NVDECSRD1 memory client
* Add also to T194 (both NVDEC0/1)
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 16 +++
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 36
2 files changed, 52 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index d02f6bf3e2ca..b65eda4238de 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1342,6 +1342,22 @@ dsib: dsi@1540 {
power-domains = < TEGRA186_POWER_DOMAIN_DISP>;
};
+ nvdec@1548 {
+ compatible = "nvidia,tegra186-nvdec";
+ reg = <0x1548 0x4>;
+ clocks = < TEGRA186_CLK_NVDEC>;
+ clock-names = "nvdec";
+ resets = < TEGRA186_RESET_NVDEC>;
+ reset-names = "nvdec";
+
+ power-domains = < TEGRA186_POWER_DOMAIN_NVDEC>;
+ interconnects = < TEGRA186_MEMORY_CLIENT_NVDECSRD
>,
+ < TEGRA186_MEMORY_CLIENT_NVDECSRD1
>,
+ < TEGRA186_MEMORY_CLIENT_NVDECSWR
>;
+ interconnect-names = "dma-mem", "read2", "write";
+ iommus = < TEGRA186_SID_NVDEC>;
+ };
+
sor0: sor@1554 {
compatible = "nvidia,tegra186-sor";
reg = <0x1554 0x1>;
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 5ba7a4519b95..3d4e81a6de70 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -1412,6 +1412,24 @@ host1x@13e0 {
interconnect-names = "dma-mem";
iommus = < TEGRA194_SID_HOST1X>;
+ nvdec@1514 {
+ compatible = "nvidia,tegra194-nvdec";
+ reg = <0x1514 0x0004>;
+ clocks = < TEGRA194_CLK_NVDEC1>;
+ clock-names = "nvdec";
+ resets = < TEGRA194_RESET_NVDEC1>;
+ reset-names = "nvdec";
+
+ power-domains = <
TEGRA194_POWER_DOMAIN_NVDECB>;
+ interconnects = <
TEGRA194_MEMORY_CLIENT_NVDEC1SRD >,
+ <
TEGRA194_MEMORY_CLIENT_NVDEC1SRD1 >,
+ <
TEGRA194_MEMORY_CLIENT_NVDEC1SWR >;
+ interconnect-names = "dma-mem", "read2",
"write";
+ iommus = < TEGRA194_SID_NVDEC1>;
+
+ nvidia,instance = <1>;
+ };
+
display-hub@1520 {
compatible = "nvidia,tegra194-display";
reg = <0x1520 0x0004>;
@@ -1525,6 +1543,24 @@ vic@1534 {
iommus = < TEGRA194_SID_VIC>;
};
+ nvdec@1548 {
+ compatible = "nvidia,tegra194-nvdec";
+ reg = <0x1548 0x0004>;
+ clocks = < TEGRA194_CLK_NVDEC>;
+ clock-names = "nvdec";
+ resets = < TEGRA194_RESET_NVDEC>;
+ reset-names = "nvdec";
+
+ power-domains = <
TEGRA194_POWER_DOMAIN_NVDECA>;
+ interconnects = <
TEGRA194_MEMORY_CLIENT_NVDECSRD >,
+ <
TEGRA194_MEMORY_CLIENT_NVDECSRD1 >,
+ <
TEGRA194_MEMORY_CLIENT_NVDECSWR >;
+ interconnect-names = "dma-mem", "read2",
"write";
+ iommus = < TEGRA194_SID_NVDEC>;
+
+ nvidia,instance = <0>;
+ };
+
dpaux0: dpaux@155c {
compatible = "nvidia,tegra194-dpaux";
reg = <0x155c 0x1>;
--
2.32.0
Re: [PATCH v6 5/7] drm/mediatek: add DSC support for mediatek-drm
Hi, Jason:
jason-jh.lin 於 2021年8月6日 週五 上午4:52寫道：
>
> DSC is designed for real-time systems with real-time compression,
> transmission, decompression and display.
> The DSC standard is a specification of the algorithms used for
> compressing and decompressing image display streams, including
> the specification of the syntax and semantics of the compressed
> video bit stream.
>
> Signed-off-by: jason-jh.lin
> ---
> This patch is base on [1]
>
> [1] dt-bindings: mediatek: add mediatek, dsc.yaml for mt8195 SoC binding
> https://patchwork.kernel.org/project/linux-mediatek/patch/20210805171346.24249-4-jason-jh@mediatek.com/
> ---
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 62 +
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
> 2 files changed, 63 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 328ee19f931e..24c7b004fe4d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -43,6 +43,12 @@
> #define DITHER_LSB_ERR_SHIFT_G(x) (((x) & 0x7) << 12)
> #define DITHER_ADD_LSHIFT_G(x) (((x) & 0x7) << 4)
>
> +#define DISP_REG_DSC_CON 0x
> +#define DSC_EN BIT(0)
> +#define DSC_DUAL_INOUT BIT(2)
> +#define DSC_BYPASS BIT(4)
> +#define DSC_UFOE_SEL BIT(16)
> +
> #define DISP_REG_OD_EN 0x
> #define DISP_REG_OD_CFG0x0020
> #define OD_RELAYMODE BIT(0)
> @@ -209,6 +215,35 @@ static void mtk_dither_set(struct device *dev, unsigned
> int bpc,
> DISP_DITHERING, cmdq_pkt);
> }
>
> +static void mtk_dsc_config(struct device *dev, unsigned int w,
> + unsigned int h, unsigned int vrefresh,
> + unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> +{
> + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> +
> + /* dsc bypass mode */
> + mtk_ddp_write_mask(cmdq_pkt, DSC_BYPASS, >cmdq_reg, priv->regs,
> + DISP_REG_DSC_CON, DSC_BYPASS);
> + mtk_ddp_write_mask(cmdq_pkt, DSC_UFOE_SEL, >cmdq_reg,
> priv->regs,
> + DISP_REG_DSC_CON, DSC_UFOE_SEL);
> + mtk_ddp_write_mask(cmdq_pkt, DSC_DUAL_INOUT, >cmdq_reg,
> priv->regs,
> + DISP_REG_DSC_CON, DSC_DUAL_INOUT);
> +}
> +
> +static void mtk_dsc_start(struct device *dev)
> +{
> + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> +
> + writel_relaxed(DSC_EN, >regs + DISP_REG_DSC_CON);
> +}
> +
> +static void mtk_dsc_stop(struct device *dev)
> +{
> + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> +
> + writel_relaxed(0x0, priv->regs + DISP_REG_DSC_CON);
> +}
> +
> static void mtk_od_config(struct device *dev, unsigned int w,
> unsigned int h, unsigned int vrefresh,
> unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> @@ -272,6 +307,14 @@ static const struct mtk_ddp_comp_funcs ddp_dpi = {
> .stop = mtk_dpi_stop,
> };
>
> +static const struct mtk_ddp_comp_funcs ddp_dsc = {
> + .clk_enable = mtk_ddp_clk_enable,
> + .clk_disable = mtk_ddp_clk_disable,
> + .config = mtk_dsc_config,
> + .start = mtk_dsc_start,
> + .stop = mtk_dsc_stop,
> +};
> +
> static const struct mtk_ddp_comp_funcs ddp_dsi = {
> .start = mtk_dsi_ddp_start,
> .stop = mtk_dsi_ddp_stop,
> @@ -286,6 +329,14 @@ static const struct mtk_ddp_comp_funcs ddp_gamma = {
> .stop = mtk_gamma_stop,
> };
>
> +static const struct mtk_ddp_comp_funcs ddp_merge = {
> + .clk_enable = mtk_merge_clk_enable,
> + .clk_disable = mtk_merge_clk_disable,
> + .start = mtk_merge_start,
> + .stop = mtk_merge_stop,
> + .config = mtk_merge_config,
> +};
Move the merge modification to the patch of merge.
> +
> static const struct mtk_ddp_comp_funcs ddp_od = {
> .clk_enable = mtk_ddp_clk_enable,
> .clk_disable = mtk_ddp_clk_disable,
> @@ -333,7 +384,9 @@ static const char * const
> mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] = {
> [MTK_DISP_CCORR] = "ccorr",
> [MTK_DISP_COLOR] = "color",
> [MTK_DISP_DITHER] = "dither",
> + [MTK_DISP_DSC] = "dsc",
> [MTK_DISP_GAMMA] = "gamma",
> + [MTK_DISP_MERGE] = "merge",
Ditto.
> [MTK_DISP_MUTEX] = "mutex",
> [MTK_DISP_OD] = "od",
> [MTK_DISP_OVL] = "ovl",
> @@ -362,11 +415,19 @@ static const struct mtk_ddp_comp_match
> mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = {
> [DDP_COMPONENT_DITHER] = { MTK_DISP_DITHER,0, _dither },
> [DDP_COMPONENT_DPI0]= { MTK_DPI,0, _dpi },
> [DDP_COMPONENT_DPI1]= { MTK_DPI,
Re: [PATCH v6 6/7] drm/mediatek: add MERGE support for mediatek-drm
Hi, Jason: jason-jh.lin 於 2021年8月6日 週五 上午4:52寫道： > > Add MERGE engine file: > MERGE module is used to merge two slice-per-line inputs > into one side-by-side output. > > Signed-off-by: jason-jh.lin > --- > This patch is base on [1] > > [1] dt-bindings: mediatek: display: add mt8195 SoC binding > https://patchwork.kernel.org/project/linux-mediatek/patch/20210805171346.24249-5-jason-jh@mediatek.com/ > --- > drivers/gpu/drm/mediatek/Makefile | 1 + > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 8 + > drivers/gpu/drm/mediatek/mtk_disp_merge.c | 263 > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +- > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 + > 6 files changed, 277 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_merge.c > > diff --git a/drivers/gpu/drm/mediatek/Makefile > b/drivers/gpu/drm/mediatek/Makefile > index dc54a7a69005..538e0087a44c 100644 > --- a/drivers/gpu/drm/mediatek/Makefile > +++ b/drivers/gpu/drm/mediatek/Makefile > @@ -3,6 +3,7 @@ > mediatek-drm-y := mtk_disp_ccorr.o \ > mtk_disp_color.o \ > mtk_disp_gamma.o \ > + mtk_disp_merge.o \ > mtk_disp_ovl.o \ > mtk_disp_rdma.o \ > mtk_drm_crtc.o \ > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > index cafd9df2d63b..f407cd9d873e 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > @@ -46,6 +46,14 @@ void mtk_gamma_set_common(void __iomem *regs, struct > drm_crtc_state *state); > void mtk_gamma_start(struct device *dev); > void mtk_gamma_stop(struct device *dev); > > +int mtk_merge_clk_enable(struct device *dev); > +void mtk_merge_clk_disable(struct device *dev); > +void mtk_merge_config(struct device *dev, unsigned int width, > + unsigned int height, unsigned int vrefresh, > + unsigned int bpc, struct cmdq_pkt *cmdq_pkt); > +void mtk_merge_start(struct device *dev); > +void mtk_merge_stop(struct device *dev); > + > void mtk_ovl_bgclr_in_on(struct device *dev); > void mtk_ovl_bgclr_in_off(struct device *dev); > void mtk_ovl_bypass_shadow(struct device *dev); > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_merge.c > b/drivers/gpu/drm/mediatek/mtk_disp_merge.c > new file mode 100644 > index ..f3d262792054 > --- /dev/null > +++ b/drivers/gpu/drm/mediatek/mtk_disp_merge.c > @@ -0,0 +1,263 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2021 MediaTek Inc. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "mtk_drm_ddp_comp.h" > +#include "mtk_drm_drv.h" > +#include "mtk_disp_drv.h" > + > +#define DISP_REG_MERGE_CTRL0x000 > +#define MERGE_EN 1 One more indent for the bitwise value. > +#define DISP_REG_MERGE_CFG_0 0x010 > +#define DISP_REG_MERGE_CFG_4 0x020 > +#define DISP_REG_MERGE_CFG_10 0x038 > +/* no swap */ > +#define SWAP_MODE 0 > +#define FLD_SWAP_MODE GENMASK(4, 0) > +#define DISP_REG_MERGE_CFG_12 0x040 > +#define CFG_10_10_1PI_2PO_BUF_MODE 6 > +#define CFG_10_10_2PI_2PO_BUF_MODE 8 > +#define FLD_CFG_MERGE_MODE GENMASK(4, 0) > +#define DISP_REG_MERGE_CFG_24 0x070 > +#define DISP_REG_MERGE_CFG_25 0x074 > +#define DISP_REG_MERGE_CFG_36 0x0a0 > +#define ULTRA_EN 1 > +#define PREULTRA_EN1 > +#define HALT_FOR_DVFS_EN 0 > +#define FLD_ULTRA_EN GENMASK(0, 0) > +#define FLD_PREULTRA_ENGENMASK(4, 4) > +#define FLD_HALT_FOR_DVFS_EN GENMASK(8, 8) > +#define DISP_REG_MERGE_CFG_37 0x0a4 > +/* 0: Off, 1: SRAM0, 2: SRAM1, 3: SRAM0 + SRAM1 */ > +#define BUFFER_MODE3 > +#define FLD_BUFFER_MODEGENMASK(1, 0) > +#define DISP_REG_MERGE_CFG_38 0x0a8 > +#define FLD_VDE_BLOCK_ULTRAGENMASK(0, 0) > +#define FLD_VALID_TH_BLOCK_ULTRA GENMASK(4, 4) > +#define FLD_ULTRA_FIFO_VALID_THGENMASK(31, 16) > +#define DISP_REG_MERGE_CFG_39 0x0ac > +#define FLD_NVDE_FORCE_PREULTRAGENMASK(8, 8) > +#define FLD_NVALID_TH_FORCE_PREULTRA GENMASK(12, 12) > +#define FLD_PREULTRA_FIFO_VALID_TH GENMASK(31, 16) > +#define DISP_REG_MERGE_CFG_40 0x0b0 > +/* 6 us, 600M pixel/sec */ > +#define ULTRA_TH_LOW (6 * 600) > +/* 8 us, 600M pixel/sec */ > +#define ULTRA_TH_HIGH (8 * 600) > +#define FLD_ULTRA_TH_LOW GENMASK(15, 0) > +#define FLD_ULTRA_TH_HIGH GENMASK(31, 16) > +#define DISP_REG_MERGE_CFG_41 0x0b4 > +/* 8 us, 600M pixel/sec */ > +#define
Re: [PATCH v5 02/20] drm/msm: Fix drm/sched point of no return rules
On Fri, Aug 6, 2021 at 9:42 AM Daniel Vetter wrote:
>
> On Fri, Aug 6, 2021 at 12:58 AM Rob Clark wrote:
> >
> > On Thu, Aug 5, 2021 at 3:47 AM Daniel Vetter wrote:
> > >
> > > Originally drm_sched_job_init was the point of no return, after which
> > > drivers must submit a job. I've split that up, which allows us to fix
> > > this issue pretty easily.
> > >
> > > Only thing we have to take care of is to not skip to error paths after
> > > that. Other drivers do this the same for out-fence and similar things.
> > >
> > > Fixes: 1d8a5ca436ee ("drm/msm: Conversion to drm scheduler")
> > > Cc: Rob Clark
> > > Cc: Rob Clark
> > > Cc: Sean Paul
> > > Cc: Sumit Semwal
> > > Cc: "Christian König"
> > > Cc: linux-arm-...@vger.kernel.org
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: freedr...@lists.freedesktop.org
> > > Cc: linux-me...@vger.kernel.org
> > > Cc: linaro-mm-...@lists.linaro.org
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > drivers/gpu/drm/msm/msm_gem_submit.c | 15 +++
> > > 1 file changed, 7 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > index 6d6c44f0e1f3..d0ed4ddc509e 100644
> > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > @@ -52,9 +52,6 @@ static struct msm_gem_submit *submit_create(struct
> > > drm_device *dev,
> > > return ERR_PTR(ret);
> > > }
> > >
> > > - /* FIXME: this is way too early */
> > > - drm_sched_job_arm(>base);
> > > -
> > > xa_init_flags(>deps, XA_FLAGS_ALLOC);
> > >
> > > kref_init(>ref);
> > > @@ -883,6 +880,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> > > *data,
> > >
> > > submit->user_fence =
> > > dma_fence_get(>base.s_fence->finished);
> > >
> > > + /* point of no return, we _have_ to submit no matter what */
> > > + drm_sched_job_arm(>base);
> > > +
> > > /*
> > > * Allocate an id which can be used by WAIT_FENCE ioctl to map
> > > back
> > > * to the underlying fence.
> > > @@ -892,17 +892,16 @@ int msm_ioctl_gem_submit(struct drm_device *dev,
> > > void *data,
> > > if (submit->fence_id < 0) {
> > > ret = submit->fence_id = 0;
> > > submit->fence_id = 0;
> > > - goto out;
> > > }
> > >
> > > - if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > + if (ret == 0 && args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > struct sync_file *sync_file =
> > > sync_file_create(submit->user_fence);
> > > if (!sync_file) {
> > > ret = -ENOMEM;
> > > - goto out;
> > > + } else {
> > > + fd_install(out_fence_fd, sync_file->file);
> > > + args->fence_fd = out_fence_fd;
> > > }
> > > - fd_install(out_fence_fd, sync_file->file);
> > > - args->fence_fd = out_fence_fd;
> >
> > I wonder if instead we should (approximately) undo "drm/msm/submit:
> > Simplify out-fence-fd handling" so that the point that it could fail
> > is moved up ahead of the drm_sched_job_arm()?
>
> Hm yeah. Up to you how you want to paint this shed, I think either is fine.
>
> > Also, does the dma_fence_get() work before drm_sched_job_arm()? From
> > a quick look, it looks like it won't, but I'm still playing catchup
> > and haven't had a chance to look at your entire series. If it doesn't
> > work before drm_sched_job_arm(), then there is really no way to
> > prevent a error path between the fence-init and job-submit.
>
> Yes. I thought I've checked that I put the _arm() in the right spot,
> but I guess I screwed up and you need the fence before the point where
> I've put the job_arm()? And yes the error path cannot be avoided for
> out-fences, that's what I tried to explain in the commit message.
>
> > But, prior to your series, wouldn't a failure after
> > drm_sched_job_init() but before the job is submitted just burn a
> > fence-id, and otherwise carry on it's merry way?
>
> Maybe? I'm not sure whether the scheduler gets confused about the gap
> and freak out abou that. I'm fairly new to that code and learning
> (which is part why I'm working on it). Since you look up in
> fences/syncobj after job_init() it should be pretty easy to whip up a
> testcase and see what happens. Also as long as nothing fails you won't
> see an issue, that's for sure.
fair.. I'll try to come up with a test case.. pre-scheduler-conversion
it wasn't a problem to fail after the fence seqno was allocated (well,
I guess you might have problems if you had 2^31 failures in a row)
BR,
-R
> -Daniel
>
> > BR,
> > -R
> >
> > > }
> > >
> > > submit_attach_object_fences(submit);
> > > --
> > > 2.32.0
> > >
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
>
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Check if engine has heartbeat when closing a context
On 8/2/2021 02:40, Tvrtko Ursulin wrote: On 30/07/2021 19:13, John Harrison wrote: On 7/30/2021 02:49, Tvrtko Ursulin wrote: On 30/07/2021 01:13, John Harrison wrote: On 7/28/2021 17:34, Matthew Brost wrote: If an engine associated with a context does not have a heartbeat, ban it immediately. This is needed for GuC submission as a idle pulse doesn't kick the context off the hardware where it then can check for a heartbeat and ban the context. Pulse, that is a request with I915_PRIORITY_BARRIER, does not preempt a running normal priority context? Why does it matter then whether or not heartbeats are enabled - when heartbeat just ends up sending the same engine pulse (eventually, with raising priority)? The point is that the pulse is pointless. See the rest of my comments below, specifically "the context will get resubmitted to the hardware after the pulse completes". To re-iterate... Yes, it preempts the context. Yes, it does so whether heartbeats are enabled or not. But so what? Who cares? You have preempted a context. It is no longer running on the hardware. BUT IT IS STILL A VALID CONTEXT. It is valid yes, and it even may be the current ABI so another question is whether it is okay to change that. The backend scheduler will just resubmit it to the hardware as soon as the pulse completes. The only reason this works at all is because of the horrid hack in the execlist scheduler's back end implementation (in __execlists_schedule_in): if (unlikely(intel_context_is_closed(ce) && !intel_engine_has_heartbeat(engine))) intel_context_set_banned(ce); Right, is the above code then needed with this patch - when ban is immediately applied on the higher level? The actual back end scheduler is saying "Is this a zombie context? Is the heartbeat disabled? Then ban it". No other scheduler backend is going to have knowledge of zombie context status or of the heartbeat status. Nor are they going to call back into the higher levels of the i915 driver to trigger a ban operation. Certainly a hardware implemented scheduler is not going to be looking at private i915 driver information to decide whether to submit a context or whether to tell the OS to kill it off instead. For persistence to work with a hardware scheduler (or a non-Intel specific scheduler such as the DRM one), the handling of zombie contexts, banning, etc. *must* be done entirely in the front end. It cannot rely on any backend hacks. That means you can't rely on any fancy behaviour of pulses. If you want to ban a context then you must explicitly ban that context. If you want to ban it at some later point then you need to track it at the top level as a zombie and then explicitly ban that zombie at whatever later point. I am still trying to understand it all. If I go by the commit message: """ This is needed for GuC submission as a idle pulse doesn't kick the context off the hardware where it then can check for a heartbeat and ban the context. """ That did not explain things for me. Sentence does not appear to make sense. Now, it seems "kick off the hardware" is meant as revoke and not just preempt. Which is fine, perhaps just needs to be written more explicitly. But the part of checking for heartbeat after idle pulse does not compute for me. It is the heartbeat which emits idle pulses, not idle pulse emitting heartbeats. I am in agreement that the commit message is confusing and does not explain either the problem or the solution. But anyway, I can buy the handling at the front end story completely. It makes sense. We just need to agree that a) it is okay to change the ABI and b) remove the backend check from execlists if it is not needed any longer. And if ABI change is okay then commit message needs to talk about it loudly and clearly. I don't think we have a choice. The current ABI is not and cannot ever be compatible with any scheduler external to i915. It cannot be implemented with a hardware scheduler such as the GuC and it cannot be implemented with an external software scheduler such as the DRM one. My view is that any implementation involving knowledge of the heartbeat is fundamentally broken. According to Daniel Vetter, the DRM ABI on this subject is that an actively executing context should persist until the DRM file handle is closed. That seems like a much more plausible and simple ABI than one that says 'if the heartbeat is running then a context will persist forever, if the heartbeat is not running then it will be killed immediately, if the heart was running but then stops running then the context will be killed on the next context switch, ...'. And if I understand it correctly, the current ABI allows a badly written user app to cause a denial of service by leaving contexts permanently running an infinite loop on the hardware even after the app has been killed! How can that ever be considered a good idea?
Re: [Intel-gfx] [PATCH 2/4] drm/i915/guc: put all guc objects in lmem when available
On 8/2/2021 22:11, Matthew Brost wrote:
From: Daniele Ceraolo Spurio
The firmware binary has to be loaded from lmem and the recommendation is
to put all other objects in there as well. Note that we don't fall back
to system memory if the allocation in lmem fails because all objects are
allocated during driver load and if we have issues with lmem at that point
something is seriously wrong with the system, so no point in trying to
handle it.
Cc: Matthew Auld
Cc: Abdiel Janulgue
Cc: Michal Wajdeczko
Cc: Vinay Belgaumkar
Cc: Radoslaw Szwichtenberg
Signed-off-by: Daniele Ceraolo Spurio
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 26
drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 4 ++
drivers/gpu/drm/i915/gt/uc/intel_guc.c| 9 ++-
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 11 +++-
drivers/gpu/drm/i915/gt/uc/intel_huc.c| 14 -
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 75 +--
6 files changed, 127 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
index eb345305dc52..034226c5d4d0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
@@ -103,6 +103,32 @@ __i915_gem_object_create_lmem_with_ps(struct
drm_i915_private *i915,
size, page_size, flags);
}
+struct drm_i915_gem_object *
+i915_gem_object_create_lmem_from_data(struct drm_i915_private *i915,
+ const void *data, size_t size)
+{
+ struct drm_i915_gem_object *obj;
+ void *map;
+
+ obj = i915_gem_object_create_lmem(i915,
+ round_up(size, PAGE_SIZE),
+ I915_BO_ALLOC_CONTIGUOUS);
+ if (IS_ERR(obj))
+ return obj;
+
+ map = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WC);
+ if (IS_ERR(map)) {
+ i915_gem_object_put(obj);
+ return map;
+ }
+
+ memcpy(map, data, size);
+
+ i915_gem_object_unpin_map(obj);
+
+ return obj;
+}
+
struct drm_i915_gem_object *
i915_gem_object_create_lmem(struct drm_i915_private *i915,
resource_size_t size,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
index 4ee81fc66302..1b88ea13435c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
@@ -23,6 +23,10 @@ bool i915_gem_object_is_lmem(struct drm_i915_gem_object
*obj);
bool __i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
+struct drm_i915_gem_object *
+i915_gem_object_create_lmem_from_data(struct drm_i915_private *i915,
+ const void *data, size_t size);
+
struct drm_i915_gem_object *
__i915_gem_object_create_lmem_with_ps(struct drm_i915_private *i915,
resource_size_t size,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 979128e28372..55160d3e401a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -3,6 +3,7 @@
* Copyright © 2014-2019 Intel Corporation
*/
+#include "gem/i915_gem_lmem.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_irq.h"
#include "gt/intel_gt_pm_irq.h"
@@ -630,7 +631,13 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc
*guc, u32 size)
u64 flags;
int ret;
- obj = i915_gem_object_create_shmem(gt->i915, size);
+ if (HAS_LMEM(gt->i915))
+ obj = i915_gem_object_create_lmem(gt->i915, size,
+ I915_BO_ALLOC_CPU_CLEAR |
+ I915_BO_ALLOC_CONTIGUOUS);
+ else
+ obj = i915_gem_object_create_shmem(gt->i915, size);
+
if (IS_ERR(obj))
return ERR_CAST(obj);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 76fe766ad1bc..962be0c12208 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -41,7 +41,7 @@ static void guc_prepare_xfer(struct intel_uncore *uncore)
}
/* Copy RSA signature from the fw image to HW for verification */
-static void guc_xfer_rsa(struct intel_uc_fw *guc_fw,
+static int guc_xfer_rsa(struct intel_uc_fw *guc_fw,
struct intel_uncore *uncore)
{
u32 rsa[UOS_RSA_SCRATCH_COUNT];
@@ -49,10 +49,13 @@ static void guc_xfer_rsa(struct intel_uc_fw *guc_fw,
int i;
copied = intel_uc_fw_copy_rsa(guc_fw, rsa, sizeof(rsa));
- GEM_BUG_ON(copied < sizeof(rsa));
+ if (copied < sizeof(rsa))
+ return -ENOMEM;
for (i = 0; i < UOS_RSA_SCRATCH_COUNT; i++)
Re: [Intel-gfx] [PATCH 4/4] drm/i915/guc: Enable GuC submission by default on DG1
On 8/2/2021 22:11, Matthew Brost wrote:
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index da57d18d9f6b..fc2fc8d111d8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -35,7 +35,7 @@ static void uc_expand_default_options(struct intel_uc *uc)
}
/* Intermediate platforms are HuC authentication only */
- if (IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
+ if (IS_ALDERLAKE_S(i915)) {
i915->params.enable_guc = ENABLE_GUC_LOAD_HUC;
return;
}
Reviewed-by: John Harrison
Re: [Intel-gfx] [PATCH] drm/i915: Disable bonding on gen12+ platforms
On Fri, Aug 6, 2021 at 8:25 PM John Harrison wrote:
> On 7/28/2021 12:21, Matthew Brost wrote:
> > Disable bonding on gen12+ platforms aside from ones already supported by
> > the i915 - TGL, RKL, and ADL-S.
> >
> > Signed-off-by: Matthew Brost
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_context.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index 05c3ee191710..9c3672bac0e2 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -446,6 +446,13 @@ set_proto_ctx_engines_bond(struct i915_user_extension
> > __user *base, void *data)
> > u16 idx, num_bonds;
> > int err, n;
> >
> > + if (GRAPHICS_VER(i915) >= 12 && !IS_TIGERLAKE(i915) &&
> > + !IS_ROCKETLAKE(i915) && !IS_ALDERLAKE_S(i915)) {
> > + drm_dbg(>drm,
> > + "Bonding on gen12+ aside from TGL, RKL, and ADL_S not
> > allowed\n");
> I would have said not supported rather than not allowed. Either way:
> Reviewed-by: John Harrison
Either is fine with me.
Acked-by: Daniel Vetter
>
> > + return -ENODEV;
> > + }
> > +
> > if (get_user(idx, >virtual_index))
> > return -EFAULT;
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v5 02/20] drm/msm: Fix drm/sched point of no return rules
On Fri, Aug 6, 2021 at 12:58 AM Rob Clark wrote:
>
> On Thu, Aug 5, 2021 at 3:47 AM Daniel Vetter wrote:
> >
> > Originally drm_sched_job_init was the point of no return, after which
> > drivers must submit a job. I've split that up, which allows us to fix
> > this issue pretty easily.
> >
> > Only thing we have to take care of is to not skip to error paths after
> > that. Other drivers do this the same for out-fence and similar things.
> >
> > Fixes: 1d8a5ca436ee ("drm/msm: Conversion to drm scheduler")
> > Cc: Rob Clark
> > Cc: Rob Clark
> > Cc: Sean Paul
> > Cc: Sumit Semwal
> > Cc: "Christian König"
> > Cc: linux-arm-...@vger.kernel.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: freedr...@lists.freedesktop.org
> > Cc: linux-me...@vger.kernel.org
> > Cc: linaro-mm-...@lists.linaro.org
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/msm/msm_gem_submit.c | 15 +++
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > index 6d6c44f0e1f3..d0ed4ddc509e 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > @@ -52,9 +52,6 @@ static struct msm_gem_submit *submit_create(struct
> > drm_device *dev,
> > return ERR_PTR(ret);
> > }
> >
> > - /* FIXME: this is way too early */
> > - drm_sched_job_arm(>base);
> > -
> > xa_init_flags(>deps, XA_FLAGS_ALLOC);
> >
> > kref_init(>ref);
> > @@ -883,6 +880,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> > *data,
> >
> > submit->user_fence = dma_fence_get(>base.s_fence->finished);
> >
> > + /* point of no return, we _have_ to submit no matter what */
> > + drm_sched_job_arm(>base);
> > +
> > /*
> > * Allocate an id which can be used by WAIT_FENCE ioctl to map back
> > * to the underlying fence.
> > @@ -892,17 +892,16 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> > *data,
> > if (submit->fence_id < 0) {
> > ret = submit->fence_id = 0;
> > submit->fence_id = 0;
> > - goto out;
> > }
> >
> > - if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > + if (ret == 0 && args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > struct sync_file *sync_file =
> > sync_file_create(submit->user_fence);
> > if (!sync_file) {
> > ret = -ENOMEM;
> > - goto out;
> > + } else {
> > + fd_install(out_fence_fd, sync_file->file);
> > + args->fence_fd = out_fence_fd;
> > }
> > - fd_install(out_fence_fd, sync_file->file);
> > - args->fence_fd = out_fence_fd;
>
> I wonder if instead we should (approximately) undo "drm/msm/submit:
> Simplify out-fence-fd handling" so that the point that it could fail
> is moved up ahead of the drm_sched_job_arm()?
Hm yeah. Up to you how you want to paint this shed, I think either is fine.
> Also, does the dma_fence_get() work before drm_sched_job_arm()? From
> a quick look, it looks like it won't, but I'm still playing catchup
> and haven't had a chance to look at your entire series. If it doesn't
> work before drm_sched_job_arm(), then there is really no way to
> prevent a error path between the fence-init and job-submit.
Yes. I thought I've checked that I put the _arm() in the right spot,
but I guess I screwed up and you need the fence before the point where
I've put the job_arm()? And yes the error path cannot be avoided for
out-fences, that's what I tried to explain in the commit message.
> But, prior to your series, wouldn't a failure after
> drm_sched_job_init() but before the job is submitted just burn a
> fence-id, and otherwise carry on it's merry way?
Maybe? I'm not sure whether the scheduler gets confused about the gap
and freak out abou that. I'm fairly new to that code and learning
(which is part why I'm working on it). Since you look up in
fences/syncobj after job_init() it should be pretty easy to whip up a
testcase and see what happens. Also as long as nothing fails you won't
see an issue, that's for sure.
-Daniel
> BR,
> -R
>
> > }
> >
> > submit_attach_object_fences(submit);
> > --
> > 2.32.0
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Proposal for allowing more Nouveau contributors to merge patches
Hey everybody, so, here is a proposal of what we could change in order to allow patches to land faster, more reliably and to increase the overall bus factor in terms of nouveau kernel maintenance. But let's start with the current situation: At the moment contributors have to send patches to the nouveau mailing list in order to submit them for inclusion into Bens nouveau tree. Then Ben has to look at them and either respond with a comment on what needs to change or merge it. At some point Ben submits all new changes to nouveau for inclusion into the drm-next tree. Practically there are a few problems here: 1. Not all patch submissions get a response 2. Patch submitters usually don't get to know if their patches are accepted unless they appear in drm-next or linus' tree. 3. A lot of trivial patches never make it into the tree. 4. Depending on how overloaded Ben is, even bigger patch series adding new features never get any responses at all. 5. Patch series might get stale for any other reason and there is no good tool to track open patch submissions (no, patchwork isn't good and it sucks and not even in the mood to explain this to people thinking otherwise as this should just be obvious) This usually ends up discouraging new contributors from making more contributions to nouveau as they see that some or all of their patches never get any reaction and it's even annoying to current contributors if they see their patches being stuck as well. And here I want to stress that none of this is Ben's fault and has his own things to work on and none of this should just depend on one person finding enough time. So the solution shouldn't be a simple "let's find a different tool and everything should be perfect" but the solution should be "how can we change the process so it's less time consuming for Ben to accept patches". And while working on this, I'd also want to tackle the problem that contributing to nouveau shouldn't be frustrating for new contributors and the process should be more lively overall. So for this I want to propose a new workflow, which would also spread some responsibilities around active members of the nouveau community: 1. We should have a public nouveau tree (which could be https://gitlab.freedesktop.org/drm/nouveau or something else) with a nouveau-next branch collecting all patches for the next kernel major kernel release. At the moment the "official" nouveau tree is kind of https://github.com/skeggsb/nouveau but there is https://github.com/skeggsb/linux as well. The main problem is, it's a repository tight to a person. We already have https://gitlab.freedesktop.org/drm/nouveau as a bug tracker, but it also contains a full linux git tree which is updated automatically through a CI job. 2. A chosen group of people should have push rights to this repository in order to accept patches sent in via emails to the nouveau Mailing List or fancy UIs (like gitlabs MRs) or other ways. Trusted contributors should be allowed to review and accept submitted patches in order to reduce the workload on Ben. Those patches will be collected on the nouveau-next branch. Patches from a mailing list could e.g. be submitted through a MR on gitlab or just pushed to the branch directly. 3. We should ensure through CI that submitted patches are at least passing basic quality control (checkpatch, compile testing, sparse, etc...) I already worked on this and one example can be seen here: https://gitlab.freedesktop.org/drm/nouveau/-/merge_requests/3 There are more things we could add to it, like checking if sparse is happy or add additional checks. Adding tests against hw is something we want to target for the future as well. 4. patches from nouveau-next should be included into a "higher" git tree (drm-next or drm-misc-next) by authorized people after getting a final review by Ben or somebody else. The idea is to post all collected patches for final review to the mailing list as Ben wanted to have a final possibility to nack it in case something stands out. We could then depend on Ben including those patches, but we could also use commit rights to drm-next or drm-misc-next from other people as well. I have commit access to the drm-misc repository, and I would assume that Lyude would be able to get it as well or already has it. 5. Urgent fixes should land via drm-fixes or drm-misc-fixes. We kind of already do this already though. We could spin up a nouveau-fixes branch in the future, but the amount of such fixes is so low it's not really worth the effort at the moment. And we have access to drm-misc. Once we decide to start and agree to some process, we should try it out with trivial patches, we already get enough of. Like Typo fixes, patches adding docs or removing dead code and can expand this to more "serious" work once we agree that it actually works and does have benefits to nouveau overall. Any thoughts on this?
Re: [PATCH v6 7/7] drm/mediatek: add mediatek-drm of vdosys0 support for mt8195
Hi, Jason:
jason-jh.lin 於 2021年8月6日 週五 上午4:52寫道：
>
> Add driver data of mt8195 vdosys0 to mediatek-drm and the sub driver.
>
Reviewed-by: Chun-Kuang Hu
> Signed-off-by: jason-jh.lin
> ---
> This patch is base on [1]
>
> [1] dt-bindings: mediatek: display: add mt8195 SoC binding
> https://patchwork.kernel.org/project/linux-mediatek/patch/20210805171346.24249-5-jason-jh@mediatek.com/
> ---
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 28
> 2 files changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> index 728aaadfea8c..00e9827acefe 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> @@ -355,6 +355,10 @@ static const struct mtk_disp_rdma_data
> mt8183_rdma_driver_data = {
> .fifo_size = 5 * SZ_1K,
> };
>
> +static const struct mtk_disp_rdma_data mt8195_rdma_driver_data = {
> + .fifo_size = 1920,
> +};
> +
> static const struct of_device_id mtk_disp_rdma_driver_dt_match[] = {
> { .compatible = "mediatek,mt2701-disp-rdma",
> .data = _rdma_driver_data},
> @@ -362,6 +366,8 @@ static const struct of_device_id
> mtk_disp_rdma_driver_dt_match[] = {
> .data = _rdma_driver_data},
> { .compatible = "mediatek,mt8183-disp-rdma",
> .data = _rdma_driver_data},
> + { .compatible = "mediatek,mt8195-disp-rdma",
> + .data = _rdma_driver_data},
> {},
> };
> MODULE_DEVICE_TABLE(of, mtk_disp_rdma_driver_dt_match);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 5eb9c0a04447..9aebf73144c6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -147,6 +147,19 @@ static const enum mtk_ddp_comp_id mt8183_mtk_ddp_ext[] =
> {
> DDP_COMPONENT_DPI0,
> };
>
> +static const enum mtk_ddp_comp_id mt8195_mtk_ddp_main[] = {
> + DDP_COMPONENT_OVL0,
> + DDP_COMPONENT_RDMA0,
> + DDP_COMPONENT_COLOR0,
> + DDP_COMPONENT_CCORR,
> + DDP_COMPONENT_AAL0,
> + DDP_COMPONENT_GAMMA,
> + DDP_COMPONENT_DITHER,
> + DDP_COMPONENT_DSC0,
> + DDP_COMPONENT_MERGE0,
> + DDP_COMPONENT_DP_INTF0,
> +};
> +
> static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
> .main_path = mt2701_mtk_ddp_main,
> .main_len = ARRAY_SIZE(mt2701_mtk_ddp_main),
> @@ -186,6 +199,11 @@ static const struct mtk_mmsys_driver_data
> mt8183_mmsys_driver_data = {
> .ext_len = ARRAY_SIZE(mt8183_mtk_ddp_ext),
> };
>
> +static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data = {
> + .main_path = mt8195_mtk_ddp_main,
> + .main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
> +};
> +
> static int mtk_drm_kms_init(struct drm_device *drm)
> {
> struct mtk_drm_private *private = drm->dev_private;
> @@ -406,10 +424,14 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[]
> = {
> .data = (void *)MTK_DISP_COLOR },
> { .compatible = "mediatek,mt8183-disp-dither",
> .data = (void *)MTK_DISP_DITHER },
> + { .compatible = "mediatek,mt8195-disp-dsc",
> + .data = (void *)MTK_DISP_DSC },
> { .compatible = "mediatek,mt8173-disp-gamma",
> .data = (void *)MTK_DISP_GAMMA, },
> { .compatible = "mediatek,mt8183-disp-gamma",
> .data = (void *)MTK_DISP_GAMMA, },
> + { .compatible = "mediatek,mt8195-disp-merge",
> + .data = (void *)MTK_DISP_MERGE },
> { .compatible = "mediatek,mt2701-disp-mutex",
> .data = (void *)MTK_DISP_MUTEX },
> { .compatible = "mediatek,mt2712-disp-mutex",
> @@ -418,6 +440,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
> .data = (void *)MTK_DISP_MUTEX },
> { .compatible = "mediatek,mt8183-disp-mutex",
> .data = (void *)MTK_DISP_MUTEX },
> + { .compatible = "mediatek,mt8195-disp-mutex",
> + .data = (void *)MTK_DISP_MUTEX },
> { .compatible = "mediatek,mt8173-disp-od",
> .data = (void *)MTK_DISP_OD },
> { .compatible = "mediatek,mt2701-disp-ovl",
> @@ -438,6 +462,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
> .data = (void *)MTK_DISP_RDMA },
> { .compatible = "mediatek,mt8183-disp-rdma",
> .data = (void *)MTK_DISP_RDMA },
> + { .compatible = "mediatek,mt8195-disp-rdma",
> + .data = (void *)MTK_DISP_RDMA },
> { .compatible = "mediatek,mt8173-disp-ufoe",
> .data = (void *)MTK_DISP_UFOE },
> { .compatible = "mediatek,mt8173-disp-wdma",
> @@ -468,6 +494,8 @@ static const struct of_device_id mtk_drm_of_ids[] = {
> .data = _mmsys_driver_data},
> { .compatible = "mediatek,mt8183-mmsys",
> .data = _mmsys_driver_data},
> +
Re: [Intel-gfx] [PATCH] drm/i915: Disable bonding on gen12+ platforms
On 7/28/2021 12:21, Matthew Brost wrote:
Disable bonding on gen12+ platforms aside from ones already supported by
the i915 - TGL, RKL, and ADL-S.
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 05c3ee191710..9c3672bac0e2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -446,6 +446,13 @@ set_proto_ctx_engines_bond(struct i915_user_extension
__user *base, void *data)
u16 idx, num_bonds;
int err, n;
+ if (GRAPHICS_VER(i915) >= 12 && !IS_TIGERLAKE(i915) &&
+ !IS_ROCKETLAKE(i915) && !IS_ALDERLAKE_S(i915)) {
+ drm_dbg(>drm,
+ "Bonding on gen12+ aside from TGL, RKL, and ADL_S not
allowed\n");
I would have said not supported rather than not allowed. Either way:
Reviewed-by: John Harrison
+ return -ENODEV;
+ }
+
if (get_user(idx, >virtual_index))
return -EFAULT;
Re: [Intel-gfx] [PATCH] drm/i915: Fix syncmap memory leak
On Fri, Aug 06, 2021 at 11:23:06AM -0700, John Harrison wrote: > On 7/30/2021 12:53, Matthew Brost wrote: > > A small race exists between intel_gt_retire_requests_timeout and > > intel_timeline_exit which could result in the syncmap not getting > > free'd. Rather than work to hard to seal this race, simply cleanup the > free'd -> freed > Sure. > > syncmap on fini. > > > > unreferenced object 0x88813bc53b18 (size 96): > >comm "gem_close_race", pid 5410, jiffies 4294917818 (age 1105.600s) > >hex dump (first 32 bytes): > > 01 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 > > 00 00 00 00 00 00 00 00 6b 6b 6b 6b 06 00 00 00 > >backtrace: > > [<120b863a>] __sync_alloc_leaf+0x1e/0x40 [i915] > > [<042f6959>] __sync_set+0x1bb/0x240 [i915] > > [<90f0e90f>] i915_request_await_dma_fence+0x1c7/0x400 [i915] > > [<56a48219>] i915_request_await_object+0x222/0x360 [i915] > > [] i915_gem_do_execbuffer+0x1bd0/0x2250 [i915] > > [<3c9d830f>] i915_gem_execbuffer2_ioctl+0x405/0xce0 [i915] > > [ ] drm_ioctl_kernel+0xb0/0xf0 [drm] > > [ ] drm_ioctl+0x305/0x3c0 [drm] > > [<8b0d8986>] __x64_sys_ioctl+0x71/0xb0 > > [<76c362a4>] do_syscall_64+0x33/0x80 > > [ ] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > Signed-off-by: Matthew Brost > > Fixes: 531958f6f357 ("drm/i915/gt: Track timeline activeness in enter/exit") > > Cc: > > --- > > drivers/gpu/drm/i915/gt/intel_timeline.c | 9 + > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c > > b/drivers/gpu/drm/i915/gt/intel_timeline.c > > index c4a126c8caef..1257f4f11e66 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_timeline.c > > +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c > > @@ -127,6 +127,15 @@ static void intel_timeline_fini(struct rcu_head *rcu) > > i915_vma_put(timeline->hwsp_ggtt); > > i915_active_fini(>active); > > + > > + /* > > +* A small race exists between intel_gt_retire_requests_timeout and > > +* intel_timeline_exit which could result in the syncmap not getting > > +* free'd. Rather than work to hard to seal this race, simply cleanup > > +* the syncmap on fini. > What is the race? I'm going round in circles just trying to work out how > intel_gt_retire_requests_timeout is supposed to get to intel_timeline_exit > in the first place. > intel_gt_retire_requests_timeout increments tl->active_count, active_count == 2 intel_timeline_exit is called, returns on atomic_add_unless, active_count == 1 intel_gt_retire_requests_timeout decrements tl->active_count, active_count == 0 i915_syncmap_free is never called, memory leak Matt > Also, free'd -> freed. > > John. > > > > +*/ > > + i915_syncmap_free(>sync); > > + > > kfree(timeline); > > } >
Re: [PATCH v5 02/20] drm/msm: Fix drm/sched point of no return rules
On Fri, Aug 6, 2021 at 7:15 PM Rob Clark wrote:
>
> On Fri, Aug 6, 2021 at 9:42 AM Daniel Vetter wrote:
> >
> > On Fri, Aug 6, 2021 at 12:58 AM Rob Clark wrote:
> > >
> > > On Thu, Aug 5, 2021 at 3:47 AM Daniel Vetter
> > > wrote:
> > > >
> > > > Originally drm_sched_job_init was the point of no return, after which
> > > > drivers must submit a job. I've split that up, which allows us to fix
> > > > this issue pretty easily.
> > > >
> > > > Only thing we have to take care of is to not skip to error paths after
> > > > that. Other drivers do this the same for out-fence and similar things.
> > > >
> > > > Fixes: 1d8a5ca436ee ("drm/msm: Conversion to drm scheduler")
> > > > Cc: Rob Clark
> > > > Cc: Rob Clark
> > > > Cc: Sean Paul
> > > > Cc: Sumit Semwal
> > > > Cc: "Christian König"
> > > > Cc: linux-arm-...@vger.kernel.org
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Cc: freedr...@lists.freedesktop.org
> > > > Cc: linux-me...@vger.kernel.org
> > > > Cc: linaro-mm-...@lists.linaro.org
> > > > Signed-off-by: Daniel Vetter
> > > > ---
> > > > drivers/gpu/drm/msm/msm_gem_submit.c | 15 +++
> > > > 1 file changed, 7 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > index 6d6c44f0e1f3..d0ed4ddc509e 100644
> > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > @@ -52,9 +52,6 @@ static struct msm_gem_submit *submit_create(struct
> > > > drm_device *dev,
> > > > return ERR_PTR(ret);
> > > > }
> > > >
> > > > - /* FIXME: this is way too early */
> > > > - drm_sched_job_arm(>base);
> > > > -
> > > > xa_init_flags(>deps, XA_FLAGS_ALLOC);
> > > >
> > > > kref_init(>ref);
> > > > @@ -883,6 +880,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev,
> > > > void *data,
> > > >
> > > > submit->user_fence =
> > > > dma_fence_get(>base.s_fence->finished);
> > > >
> > > > + /* point of no return, we _have_ to submit no matter what */
> > > > + drm_sched_job_arm(>base);
> > > > +
> > > > /*
> > > > * Allocate an id which can be used by WAIT_FENCE ioctl to map
> > > > back
> > > > * to the underlying fence.
> > > > @@ -892,17 +892,16 @@ int msm_ioctl_gem_submit(struct drm_device *dev,
> > > > void *data,
> > > > if (submit->fence_id < 0) {
> > > > ret = submit->fence_id = 0;
> > > > submit->fence_id = 0;
> > > > - goto out;
> > > > }
> > > >
> > > > - if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > > + if (ret == 0 && args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > > struct sync_file *sync_file =
> > > > sync_file_create(submit->user_fence);
> > > > if (!sync_file) {
> > > > ret = -ENOMEM;
> > > > - goto out;
> > > > + } else {
> > > > + fd_install(out_fence_fd, sync_file->file);
> > > > + args->fence_fd = out_fence_fd;
> > > > }
> > > > - fd_install(out_fence_fd, sync_file->file);
> > > > - args->fence_fd = out_fence_fd;
> > >
> > > I wonder if instead we should (approximately) undo "drm/msm/submit:
> > > Simplify out-fence-fd handling" so that the point that it could fail
> > > is moved up ahead of the drm_sched_job_arm()?
> >
> > Hm yeah. Up to you how you want to paint this shed, I think either is fine.
> >
> > > Also, does the dma_fence_get() work before drm_sched_job_arm()? From
> > > a quick look, it looks like it won't, but I'm still playing catchup
> > > and haven't had a chance to look at your entire series. If it doesn't
> > > work before drm_sched_job_arm(), then there is really no way to
> > > prevent a error path between the fence-init and job-submit.
> >
> > Yes. I thought I've checked that I put the _arm() in the right spot,
> > but I guess I screwed up and you need the fence before the point where
> > I've put the job_arm()? And yes the error path cannot be avoided for
> > out-fences, that's what I tried to explain in the commit message.
> >
> > > But, prior to your series, wouldn't a failure after
> > > drm_sched_job_init() but before the job is submitted just burn a
> > > fence-id, and otherwise carry on it's merry way?
> >
> > Maybe? I'm not sure whether the scheduler gets confused about the gap
> > and freak out abou that. I'm fairly new to that code and learning
> > (which is part why I'm working on it). Since you look up in
> > fences/syncobj after job_init() it should be pretty easy to whip up a
> > testcase and see what happens. Also as long as nothing fails you won't
> > see an issue, that's for sure.
>
> fair.. I'll try to come up with a test case.. pre-scheduler-conversion
> it wasn't a problem to fail after the fence seqno was allocated (well,
> I guess you
Re: [Intel-gfx] [PATCH 3/4] drm/i915/guc: Add DG1 GuC / HuC firmware defs
On 8/2/2021 22:11, Matthew Brost wrote: Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c index f8cb00ffb506..a685d563df72 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c @@ -51,6 +51,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw, #define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \ fw_def(ALDERLAKE_P, 0, guc_def(adlp, 62, 0, 3), huc_def(tgl, 7, 9, 3)) \ fw_def(ALDERLAKE_S, 0, guc_def(tgl, 62, 0, 0), huc_def(tgl, 7, 9, 3)) \ + fw_def(DG1, 0, guc_def(dg1, 62, 0, 0), huc_def(dg1, 7, 9, 3)) \ fw_def(ROCKETLAKE, 0, guc_def(tgl, 62, 0, 0), huc_def(tgl, 7, 9, 3)) \ fw_def(TIGERLAKE, 0, guc_def(tgl, 62, 0, 0), huc_def(tgl, 7, 9, 3)) \ fw_def(JASPERLAKE, 0, guc_def(ehl, 62, 0, 0), huc_def(ehl, 9, 0, 0)) \ Reviewed-by: John Harrison
Re: [PATCH v5 02/20] drm/msm: Fix drm/sched point of no return rules
On Fri, Aug 6, 2021 at 11:41 AM Daniel Vetter wrote:
>
> On Fri, Aug 6, 2021 at 7:15 PM Rob Clark wrote:
> >
> > On Fri, Aug 6, 2021 at 9:42 AM Daniel Vetter wrote:
> > >
> > > On Fri, Aug 6, 2021 at 12:58 AM Rob Clark wrote:
> > > >
> > > > On Thu, Aug 5, 2021 at 3:47 AM Daniel Vetter
> > > > wrote:
> > > > >
> > > > > Originally drm_sched_job_init was the point of no return, after which
> > > > > drivers must submit a job. I've split that up, which allows us to fix
> > > > > this issue pretty easily.
> > > > >
> > > > > Only thing we have to take care of is to not skip to error paths after
> > > > > that. Other drivers do this the same for out-fence and similar things.
> > > > >
> > > > > Fixes: 1d8a5ca436ee ("drm/msm: Conversion to drm scheduler")
> > > > > Cc: Rob Clark
> > > > > Cc: Rob Clark
> > > > > Cc: Sean Paul
> > > > > Cc: Sumit Semwal
> > > > > Cc: "Christian König"
> > > > > Cc: linux-arm-...@vger.kernel.org
> > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > Cc: freedr...@lists.freedesktop.org
> > > > > Cc: linux-me...@vger.kernel.org
> > > > > Cc: linaro-mm-...@lists.linaro.org
> > > > > Signed-off-by: Daniel Vetter
> > > > > ---
> > > > > drivers/gpu/drm/msm/msm_gem_submit.c | 15 +++
> > > > > 1 file changed, 7 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > index 6d6c44f0e1f3..d0ed4ddc509e 100644
> > > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > @@ -52,9 +52,6 @@ static struct msm_gem_submit *submit_create(struct
> > > > > drm_device *dev,
> > > > > return ERR_PTR(ret);
> > > > > }
> > > > >
> > > > > - /* FIXME: this is way too early */
> > > > > - drm_sched_job_arm(>base);
> > > > > -
> > > > > xa_init_flags(>deps, XA_FLAGS_ALLOC);
> > > > >
> > > > > kref_init(>ref);
> > > > > @@ -883,6 +880,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev,
> > > > > void *data,
> > > > >
> > > > > submit->user_fence =
> > > > > dma_fence_get(>base.s_fence->finished);
> > > > >
> > > > > + /* point of no return, we _have_ to submit no matter what */
> > > > > + drm_sched_job_arm(>base);
> > > > > +
> > > > > /*
> > > > > * Allocate an id which can be used by WAIT_FENCE ioctl to
> > > > > map back
> > > > > * to the underlying fence.
> > > > > @@ -892,17 +892,16 @@ int msm_ioctl_gem_submit(struct drm_device
> > > > > *dev, void *data,
> > > > > if (submit->fence_id < 0) {
> > > > > ret = submit->fence_id = 0;
> > > > > submit->fence_id = 0;
> > > > > - goto out;
> > > > > }
> > > > >
> > > > > - if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > > > + if (ret == 0 && args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > > > struct sync_file *sync_file =
> > > > > sync_file_create(submit->user_fence);
> > > > > if (!sync_file) {
> > > > > ret = -ENOMEM;
> > > > > - goto out;
> > > > > + } else {
> > > > > + fd_install(out_fence_fd, sync_file->file);
> > > > > + args->fence_fd = out_fence_fd;
> > > > > }
> > > > > - fd_install(out_fence_fd, sync_file->file);
> > > > > - args->fence_fd = out_fence_fd;
> > > >
> > > > I wonder if instead we should (approximately) undo "drm/msm/submit:
> > > > Simplify out-fence-fd handling" so that the point that it could fail
> > > > is moved up ahead of the drm_sched_job_arm()?
> > >
> > > Hm yeah. Up to you how you want to paint this shed, I think either is
> > > fine.
> > >
> > > > Also, does the dma_fence_get() work before drm_sched_job_arm()? From
> > > > a quick look, it looks like it won't, but I'm still playing catchup
> > > > and haven't had a chance to look at your entire series. If it doesn't
> > > > work before drm_sched_job_arm(), then there is really no way to
> > > > prevent a error path between the fence-init and job-submit.
> > >
> > > Yes. I thought I've checked that I put the _arm() in the right spot,
> > > but I guess I screwed up and you need the fence before the point where
> > > I've put the job_arm()? And yes the error path cannot be avoided for
> > > out-fences, that's what I tried to explain in the commit message.
> > >
> > > > But, prior to your series, wouldn't a failure after
> > > > drm_sched_job_init() but before the job is submitted just burn a
> > > > fence-id, and otherwise carry on it's merry way?
> > >
> > > Maybe? I'm not sure whether the scheduler gets confused about the gap
> > > and freak out abou that. I'm fairly new to that code and learning
> > > (which is part why I'm working on it). Since you look up in
> > > fences/syncobj after job_init() it should be pretty easy to whip
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Check if engine has heartbeat when closing a context
Seen this fly by and figured I dropped a few thoughts in here. At the likely cost of looking a bit out of whack :-) On Fri, Aug 6, 2021 at 8:01 PM John Harrison wrote: > On 8/2/2021 02:40, Tvrtko Ursulin wrote: > > On 30/07/2021 19:13, John Harrison wrote: > >> On 7/30/2021 02:49, Tvrtko Ursulin wrote: > >>> On 30/07/2021 01:13, John Harrison wrote: > On 7/28/2021 17:34, Matthew Brost wrote: > > If an engine associated with a context does not have a heartbeat, > > ban it > > immediately. This is needed for GuC submission as a idle pulse > > doesn't > > kick the context off the hardware where it then can check for a > > heartbeat and ban the context. > >>> > >>> Pulse, that is a request with I915_PRIORITY_BARRIER, does not > >>> preempt a running normal priority context? > >>> > >>> Why does it matter then whether or not heartbeats are enabled - when > >>> heartbeat just ends up sending the same engine pulse (eventually, > >>> with raising priority)? > >> The point is that the pulse is pointless. See the rest of my comments > >> below, specifically "the context will get resubmitted to the hardware > >> after the pulse completes". To re-iterate... > >> > >> Yes, it preempts the context. Yes, it does so whether heartbeats are > >> enabled or not. But so what? Who cares? You have preempted a context. > >> It is no longer running on the hardware. BUT IT IS STILL A VALID > >> CONTEXT. > > > > It is valid yes, and it even may be the current ABI so another > > question is whether it is okay to change that. > > > >> The backend scheduler will just resubmit it to the hardware as soon > >> as the pulse completes. The only reason this works at all is because > >> of the horrid hack in the execlist scheduler's back end > >> implementation (in __execlists_schedule_in): > >> if (unlikely(intel_context_is_closed(ce) && > >> !intel_engine_has_heartbeat(engine))) > >> intel_context_set_banned(ce); > > > > Right, is the above code then needed with this patch - when ban is > > immediately applied on the higher level? > > > >> The actual back end scheduler is saying "Is this a zombie context? Is > >> the heartbeat disabled? Then ban it". No other scheduler backend is > >> going to have knowledge of zombie context status or of the heartbeat > >> status. Nor are they going to call back into the higher levels of the > >> i915 driver to trigger a ban operation. Certainly a hardware > >> implemented scheduler is not going to be looking at private i915 > >> driver information to decide whether to submit a context or whether > >> to tell the OS to kill it off instead. > >> > >> For persistence to work with a hardware scheduler (or a non-Intel > >> specific scheduler such as the DRM one), the handling of zombie > >> contexts, banning, etc. *must* be done entirely in the front end. It > >> cannot rely on any backend hacks. That means you can't rely on any > >> fancy behaviour of pulses. > >> > >> If you want to ban a context then you must explicitly ban that > >> context. If you want to ban it at some later point then you need to > >> track it at the top level as a zombie and then explicitly ban that > >> zombie at whatever later point. > > > > I am still trying to understand it all. If I go by the commit message: > > > > """ > > This is needed for GuC submission as a idle pulse doesn't > > kick the context off the hardware where it then can check for a > > heartbeat and ban the context. > > """ > > > > That did not explain things for me. Sentence does not appear to make > > sense. Now, it seems "kick off the hardware" is meant as revoke and > > not just preempt. Which is fine, perhaps just needs to be written more > > explicitly. But the part of checking for heartbeat after idle pulse > > does not compute for me. It is the heartbeat which emits idle pulses, > > not idle pulse emitting heartbeats. > I am in agreement that the commit message is confusing and does not > explain either the problem or the solution. > > > > > > > > But anyway, I can buy the handling at the front end story completely. > > It makes sense. We just need to agree that a) it is okay to change the > > ABI and b) remove the backend check from execlists if it is not needed > > any longer. > > > > And if ABI change is okay then commit message needs to talk about it > > loudly and clearly. > I don't think we have a choice. The current ABI is not and cannot ever > be compatible with any scheduler external to i915. It cannot be > implemented with a hardware scheduler such as the GuC and it cannot be > implemented with an external software scheduler such as the DRM one. So generally on linux we implement helper libraries, which means massive flexibility everywhere. https://blog.ffwll.ch/2016/12/midlayers-once-more-with-feeling.html So it shouldn't be an insurmountable problem to make this happen even with drm/scheduler, we can patch it up. Whether that's justified is another question. >
Re: [PATCH] drm/amdgpu: check for allocation failure in amdgpu_vkms_sw_init()
Applied. Thanks!
Alex
On Fri, Aug 6, 2021 at 11:09 AM Christian König
wrote:
>
> Am 06.08.21 um 17:05 schrieb Dan Carpenter:
> > Check whether the kcalloc() fails and return -ENOMEM if it does.
> >
> > Fixes: eeba0b9046fc ("drm/amdgpu: create amdgpu_vkms (v4)")
> > Signed-off-by: Dan Carpenter
>
> Reviewed-by: Christian König
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> > index 50bdc39733aa..ce982afeff91 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> > @@ -482,6 +482,8 @@ static int amdgpu_vkms_sw_init(void *handle)
> > return r;
> >
> > adev->amdgpu_vkms_output = kcalloc(adev->mode_info.num_crtc,
> > sizeof(struct amdgpu_vkms_output), GFP_KERNEL);
>
> Is the line above not a bit long?
>
> > + if (!adev->amdgpu_vkms_output)
> > + return -ENOMEM;
> >
> > /* allocate crtcs, encoders, connectors */
> > for (i = 0; i < adev->mode_info.num_crtc; i++) {
>
Re: [PATCH v5 02/20] drm/msm: Fix drm/sched point of no return rules
On Fri, Aug 6, 2021 at 12:11 PM Daniel Vetter wrote:
>
> On Fri, Aug 6, 2021 at 8:57 PM Rob Clark wrote:
> >
> > On Fri, Aug 6, 2021 at 11:41 AM Daniel Vetter
> > wrote:
> > >
> > > On Fri, Aug 6, 2021 at 7:15 PM Rob Clark wrote:
> > > >
> > > > On Fri, Aug 6, 2021 at 9:42 AM Daniel Vetter
> > > > wrote:
> > > > >
> > > > > On Fri, Aug 6, 2021 at 12:58 AM Rob Clark wrote:
> > > > > >
> > > > > > On Thu, Aug 5, 2021 at 3:47 AM Daniel Vetter
> > > > > > wrote:
> > > > > > >
> > > > > > > Originally drm_sched_job_init was the point of no return, after
> > > > > > > which
> > > > > > > drivers must submit a job. I've split that up, which allows us to
> > > > > > > fix
> > > > > > > this issue pretty easily.
> > > > > > >
> > > > > > > Only thing we have to take care of is to not skip to error paths
> > > > > > > after
> > > > > > > that. Other drivers do this the same for out-fence and similar
> > > > > > > things.
> > > > > > >
> > > > > > > Fixes: 1d8a5ca436ee ("drm/msm: Conversion to drm scheduler")
> > > > > > > Cc: Rob Clark
> > > > > > > Cc: Rob Clark
> > > > > > > Cc: Sean Paul
> > > > > > > Cc: Sumit Semwal
> > > > > > > Cc: "Christian König"
> > > > > > > Cc: linux-arm-...@vger.kernel.org
> > > > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > > > Cc: freedr...@lists.freedesktop.org
> > > > > > > Cc: linux-me...@vger.kernel.org
> > > > > > > Cc: linaro-mm-...@lists.linaro.org
> > > > > > > Signed-off-by: Daniel Vetter
> > > > > > > ---
> > > > > > > drivers/gpu/drm/msm/msm_gem_submit.c | 15 +++
> > > > > > > 1 file changed, 7 insertions(+), 8 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > > index 6d6c44f0e1f3..d0ed4ddc509e 100644
> > > > > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > > @@ -52,9 +52,6 @@ static struct msm_gem_submit
> > > > > > > *submit_create(struct drm_device *dev,
> > > > > > > return ERR_PTR(ret);
> > > > > > > }
> > > > > > >
> > > > > > > - /* FIXME: this is way too early */
> > > > > > > - drm_sched_job_arm(>base);
> > > > > > > -
> > > > > > > xa_init_flags(>deps, XA_FLAGS_ALLOC);
> > > > > > >
> > > > > > > kref_init(>ref);
> > > > > > > @@ -883,6 +880,9 @@ int msm_ioctl_gem_submit(struct drm_device
> > > > > > > *dev, void *data,
> > > > > > >
> > > > > > > submit->user_fence =
> > > > > > > dma_fence_get(>base.s_fence->finished);
> > > > > > >
> > > > > > > + /* point of no return, we _have_ to submit no matter what
> > > > > > > */
> > > > > > > + drm_sched_job_arm(>base);
> > > > > > > +
> > > > > > > /*
> > > > > > > * Allocate an id which can be used by WAIT_FENCE ioctl
> > > > > > > to map back
> > > > > > > * to the underlying fence.
> > > > > > > @@ -892,17 +892,16 @@ int msm_ioctl_gem_submit(struct drm_device
> > > > > > > *dev, void *data,
> > > > > > > if (submit->fence_id < 0) {
> > > > > > > ret = submit->fence_id = 0;
> > > > > > > submit->fence_id = 0;
> > > > > > > - goto out;
> > > > > > > }
> > > > > > >
> > > > > > > - if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > > > > > + if (ret == 0 && args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > > > > > struct sync_file *sync_file =
> > > > > > > sync_file_create(submit->user_fence);
> > > > > > > if (!sync_file) {
> > > > > > > ret = -ENOMEM;
> > > > > > > - goto out;
> > > > > > > + } else {
> > > > > > > + fd_install(out_fence_fd, sync_file->file);
> > > > > > > + args->fence_fd = out_fence_fd;
> > > > > > > }
> > > > > > > - fd_install(out_fence_fd, sync_file->file);
> > > > > > > - args->fence_fd = out_fence_fd;
> > > > > >
> > > > > > I wonder if instead we should (approximately) undo "drm/msm/submit:
> > > > > > Simplify out-fence-fd handling" so that the point that it could fail
> > > > > > is moved up ahead of the drm_sched_job_arm()?
> > > > >
> > > > > Hm yeah. Up to you how you want to paint this shed, I think either is
> > > > > fine.
> > > > >
> > > > > > Also, does the dma_fence_get() work before drm_sched_job_arm()?
> > > > > > From
> > > > > > a quick look, it looks like it won't, but I'm still playing catchup
> > > > > > and haven't had a chance to look at your entire series. If it
> > > > > > doesn't
> > > > > > work before drm_sched_job_arm(), then there is really no way to
> > > > > > prevent a error path between the fence-init and job-submit.
> > > > >
> > > > > Yes. I thought I've checked that I put the _arm() in the right spot,
> > > > > but I guess I screwed up and you need the fence before the point where
> > > >
Re: [git pull] drm fixes for 5.14-rc4
On Thu, Aug 5, 2021 at 8:14 PM Linus Torvalds wrote: > > This might possibly have been fixed already by the previous drm pull, > but I wanted to report it anyway, just in case. > > It happened after an uptime of over a week, so it might not be trivial > to reproduce. > > It's a NULL pointer dereference in dc_stream_retain() with the code being > > lock xadd %eax,0x390(%rdi) <-- trapping instruction > > and that's just the > > kref_get(>refcount); > > with a NULL 'stream' argument. > > Call Trace: >dc_resource_state_copy_construct+0x13f/0x190 [amdgpu] >amdgpu_dm_atomic_commit_tail+0xd5/0x1540 [amdgpu] >commit_tail+0x97/0x180 [drm_kms_helper] >process_one_work+0x1df/0x3a0 > > the oops is followed by a stream of > > [drm:amdgpu_dm_atomic_check [amdgpu]] *ERROR* [CRTC:55:crtc-1] > hw_done or flip_done timed out > > and the machine was not usable afterwards. Hm that part is a bit disappointing because the atomic modeset commit helpers are designed to recover from this (assuming we didn't fry the hw). But amdgpu does these waits in amdgpu_dm_atomic_check() which is decidedly not great (you're not supposed to block on hw or a previous in that atomic_check ever, because it can be called by userspace in a TEST_ONLY mode to figure out whether a desired config would work), and then returns that error to userspace, which is worse. I guess that's another area where the integration between what atomic modeset expects and the DC backend provides is suboptimal. I think the data structures we managed to fuse together fairly ok, but the check/commit flow and semantics are a bit a struggle. Anyway this was just an aside, I guess given the bug the driver wouldn't have recovered anyway. -Daniel > lspci says this is a > > 49:00.0 VGA compatible controller [0300]: >Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere >[Radeon RX 470/480/570/570X/580/580X/590] >[1002:67df] (rev e7) (prog-if 00 [VGA controller]) > > Full oops in the attachment, but I think the above is all the really > salient details. > >Linus -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v5 02/20] drm/msm: Fix drm/sched point of no return rules
On Fri, Aug 6, 2021 at 8:57 PM Rob Clark wrote:
>
> On Fri, Aug 6, 2021 at 11:41 AM Daniel Vetter wrote:
> >
> > On Fri, Aug 6, 2021 at 7:15 PM Rob Clark wrote:
> > >
> > > On Fri, Aug 6, 2021 at 9:42 AM Daniel Vetter
> > > wrote:
> > > >
> > > > On Fri, Aug 6, 2021 at 12:58 AM Rob Clark wrote:
> > > > >
> > > > > On Thu, Aug 5, 2021 at 3:47 AM Daniel Vetter
> > > > > wrote:
> > > > > >
> > > > > > Originally drm_sched_job_init was the point of no return, after
> > > > > > which
> > > > > > drivers must submit a job. I've split that up, which allows us to
> > > > > > fix
> > > > > > this issue pretty easily.
> > > > > >
> > > > > > Only thing we have to take care of is to not skip to error paths
> > > > > > after
> > > > > > that. Other drivers do this the same for out-fence and similar
> > > > > > things.
> > > > > >
> > > > > > Fixes: 1d8a5ca436ee ("drm/msm: Conversion to drm scheduler")
> > > > > > Cc: Rob Clark
> > > > > > Cc: Rob Clark
> > > > > > Cc: Sean Paul
> > > > > > Cc: Sumit Semwal
> > > > > > Cc: "Christian König"
> > > > > > Cc: linux-arm-...@vger.kernel.org
> > > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > > Cc: freedr...@lists.freedesktop.org
> > > > > > Cc: linux-me...@vger.kernel.org
> > > > > > Cc: linaro-mm-...@lists.linaro.org
> > > > > > Signed-off-by: Daniel Vetter
> > > > > > ---
> > > > > > drivers/gpu/drm/msm/msm_gem_submit.c | 15 +++
> > > > > > 1 file changed, 7 insertions(+), 8 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > index 6d6c44f0e1f3..d0ed4ddc509e 100644
> > > > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > @@ -52,9 +52,6 @@ static struct msm_gem_submit
> > > > > > *submit_create(struct drm_device *dev,
> > > > > > return ERR_PTR(ret);
> > > > > > }
> > > > > >
> > > > > > - /* FIXME: this is way too early */
> > > > > > - drm_sched_job_arm(>base);
> > > > > > -
> > > > > > xa_init_flags(>deps, XA_FLAGS_ALLOC);
> > > > > >
> > > > > > kref_init(>ref);
> > > > > > @@ -883,6 +880,9 @@ int msm_ioctl_gem_submit(struct drm_device
> > > > > > *dev, void *data,
> > > > > >
> > > > > > submit->user_fence =
> > > > > > dma_fence_get(>base.s_fence->finished);
> > > > > >
> > > > > > + /* point of no return, we _have_ to submit no matter what */
> > > > > > + drm_sched_job_arm(>base);
> > > > > > +
> > > > > > /*
> > > > > > * Allocate an id which can be used by WAIT_FENCE ioctl to
> > > > > > map back
> > > > > > * to the underlying fence.
> > > > > > @@ -892,17 +892,16 @@ int msm_ioctl_gem_submit(struct drm_device
> > > > > > *dev, void *data,
> > > > > > if (submit->fence_id < 0) {
> > > > > > ret = submit->fence_id = 0;
> > > > > > submit->fence_id = 0;
> > > > > > - goto out;
> > > > > > }
> > > > > >
> > > > > > - if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > > > > + if (ret == 0 && args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
> > > > > > struct sync_file *sync_file =
> > > > > > sync_file_create(submit->user_fence);
> > > > > > if (!sync_file) {
> > > > > > ret = -ENOMEM;
> > > > > > - goto out;
> > > > > > + } else {
> > > > > > + fd_install(out_fence_fd, sync_file->file);
> > > > > > + args->fence_fd = out_fence_fd;
> > > > > > }
> > > > > > - fd_install(out_fence_fd, sync_file->file);
> > > > > > - args->fence_fd = out_fence_fd;
> > > > >
> > > > > I wonder if instead we should (approximately) undo "drm/msm/submit:
> > > > > Simplify out-fence-fd handling" so that the point that it could fail
> > > > > is moved up ahead of the drm_sched_job_arm()?
> > > >
> > > > Hm yeah. Up to you how you want to paint this shed, I think either is
> > > > fine.
> > > >
> > > > > Also, does the dma_fence_get() work before drm_sched_job_arm()? From
> > > > > a quick look, it looks like it won't, but I'm still playing catchup
> > > > > and haven't had a chance to look at your entire series. If it doesn't
> > > > > work before drm_sched_job_arm(), then there is really no way to
> > > > > prevent a error path between the fence-init and job-submit.
> > > >
> > > > Yes. I thought I've checked that I put the _arm() in the right spot,
> > > > but I guess I screwed up and you need the fence before the point where
> > > > I've put the job_arm()? And yes the error path cannot be avoided for
> > > > out-fences, that's what I tried to explain in the commit message.
> > > >
> > > > > But, prior to your series, wouldn't a failure after
> > > > > drm_sched_job_init() but before the job is submitted just burn a
> > > > > fence-id, and
Re: [PATCH] drm/amd/display: Remove redundant initialization of variable eng_id
Applied. Thanks!
Alex
On Fri, Aug 6, 2021 at 7:16 AM Colin King wrote:
>
> From: Colin Ian King
>
> The variable eng_id is being initialized with a value that is never
> read, it is being re-assigned on the next statment. The assignment
> is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
> index 1a89d565c92e..de80a9ea4cfa 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
> @@ -305,7 +305,7 @@ struct link_encoder *link_enc_cfg_get_next_avail_link_enc(
> const struct dc_state *state)
> {
> struct link_encoder *link_enc = NULL;
> - enum engine_id eng_id = ENGINE_ID_UNKNOWN;
> + enum engine_id eng_id;
>
> eng_id = find_first_avail_link_enc(dc->ctx, state);
> if (eng_id != ENGINE_ID_UNKNOWN)
> --
> 2.31.1
>
Re: [Intel-gfx] [PATCH] drm/i915: Fix syncmap memory leak
On 8/6/2021 11:29, Matthew Brost wrote: On Fri, Aug 06, 2021 at 11:23:06AM -0700, John Harrison wrote: On 7/30/2021 12:53, Matthew Brost wrote: A small race exists between intel_gt_retire_requests_timeout and intel_timeline_exit which could result in the syncmap not getting free'd. Rather than work to hard to seal this race, simply cleanup the free'd -> freed Sure. syncmap on fini. unreferenced object 0x88813bc53b18 (size 96): comm "gem_close_race", pid 5410, jiffies 4294917818 (age 1105.600s) hex dump (first 32 bytes): 01 00 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 00 00 00 00 6b 6b 6b 6b 06 00 00 00 backtrace: [<120b863a>] __sync_alloc_leaf+0x1e/0x40 [i915] [<042f6959>] __sync_set+0x1bb/0x240 [i915] [<90f0e90f>] i915_request_await_dma_fence+0x1c7/0x400 [i915] [<56a48219>] i915_request_await_object+0x222/0x360 [i915] [] i915_gem_do_execbuffer+0x1bd0/0x2250 [i915] [<3c9d830f>] i915_gem_execbuffer2_ioctl+0x405/0xce0 [i915] [ ] drm_ioctl_kernel+0xb0/0xf0 [drm] [ ] drm_ioctl+0x305/0x3c0 [drm] [<8b0d8986>] __x64_sys_ioctl+0x71/0xb0 [<76c362a4>] do_syscall_64+0x33/0x80 [ ] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Signed-off-by: Matthew Brost Fixes: 531958f6f357 ("drm/i915/gt: Track timeline activeness in enter/exit") Cc: --- drivers/gpu/drm/i915/gt/intel_timeline.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c index c4a126c8caef..1257f4f11e66 100644 --- a/drivers/gpu/drm/i915/gt/intel_timeline.c +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c @@ -127,6 +127,15 @@ static void intel_timeline_fini(struct rcu_head *rcu) i915_vma_put(timeline->hwsp_ggtt); i915_active_fini(>active); + + /* +* A small race exists between intel_gt_retire_requests_timeout and +* intel_timeline_exit which could result in the syncmap not getting +* free'd. Rather than work to hard to seal this race, simply cleanup +* the syncmap on fini. What is the race? I'm going round in circles just trying to work out how intel_gt_retire_requests_timeout is supposed to get to intel_timeline_exit in the first place. intel_gt_retire_requests_timeout increments tl->active_count, active_count == 2 intel_timeline_exit is called, returns on atomic_add_unless, active_count == 1 intel_gt_retire_requests_timeout decrements tl->active_count, active_count == 0 i915_syncmap_free is never called, memory leak Matt Okay. Think I follow it now. Seems like the syncmap free should have been in timeline_fini instead of timeline_exit in the first place? Reviewed-by: John Harrison Also, free'd -> freed. John. +*/ + i915_syncmap_free(>sync); + kfree(timeline); }
[PATCH] drm/i915: Release ctx->syncobj on final put, not on ctx close
gem context refcounting is another exercise in least locking design it
seems, where most things get destroyed upon context closure (which can
race with anything really). Only the actual memory allocation and the
locks survive while holding a reference.
This tripped up Jason when reimplementing the single timeline feature
in
commit 00dae4d3d35d4f526929633b76e00b0ab4d3970d
Author: Jason Ekstrand
Date: Thu Jul 8 10:48:12 2021 -0500
drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)
We could fix the bug by holding ctx->mutex, but it's cleaner to just
make the context object actually invariant over its _entire_ lifetime.
Signed-off-by: Daniel Vetter
Fixes: 00dae4d3d35d ("drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)")
Cc: Jason Ekstrand
Cc: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
Cc: Matthew Brost
Cc: Matthew Auld
Cc: Maarten Lankhorst
Cc: "Thomas Hellström"
Cc: Lionel Landwerlin
Cc: Dave Airlie
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 754b9b8d4981..93ba0197d70a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -940,6 +940,9 @@ void i915_gem_context_release(struct kref *ref)
trace_i915_context_free(ctx);
GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
+ if (ctx->syncobj)
+ drm_syncobj_put(ctx->syncobj);
+
mutex_destroy(>engines_mutex);
mutex_destroy(>lut_mutex);
@@ -1159,9 +1162,6 @@ static void context_close(struct i915_gem_context *ctx)
if (vm)
i915_vm_close(vm);
- if (ctx->syncobj)
- drm_syncobj_put(ctx->syncobj);
-
ctx->file_priv = ERR_PTR(-EBADF);
/*
--
2.32.0
[pull] amdgpu, amdkfd, radeon drm-next-5.15
Hi Dave, Daniel, More updates for 5.15. The following changes since commit 04d505de7f82c8f2daa6139b460b05dc01e354e0: Merge tag 'amd-drm-next-5.15-2021-07-29' of https://gitlab.freedesktop.org/agd5f/linux into drm-next (2021-07-30 16:48:35 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-next-5.15-2021-08-06 for you to fetch changes up to a43e2a0e11491b73e2acaa27ee74d6c3b86deac0: drm/amdkfd: Allow querying SVM attributes that are clear (2021-08-06 16:12:32 -0400) amd-drm-next-5.15-2021-08-06: amdgpu: - Aldebaran fixes - Powergating fix for Renoir - Switch virtual DCE over to vkms based atomic modesetting - Misc typo fixes - PSP handling cleanups - DC FP cleanups - RAS fixes - Wave debug improvements - Freesync fix - BACO/BOCO fixes - Misc fixes amdkfd: - Expose gfx version in sysfs - Aldebaran fixes radeon: - Coding style fix - Typo fixes - Pageflip fix UAPI: - amdkfd: SVM address range query Proposed userspace: https://github.com/RadeonOpenCompute/ROCR-Runtime/tree/memory_model_queries Alex Deucher (1): drm/amdgpu: don't enable baco on boco platforms in runpm Anthony Koo (1): drm/amd/display: [FW Promotion] Release 0.0.77 Aric Cyr (1): drm/amd/display: 3.2.147 Bing Guo (2): drm/amd/display: Fix Dynamic bpp issue with 8K30 with Navi 1X drm/amd/display: Increase stutter watermark for dcn303 Cai Huoqing (2): drm/amd/display: Fix typo in comments gpu/drm/radeon: Fix typo in comments Candice Li (1): drm/amd/amdgpu: remove redundant host to psp cmd buf allocations Chengming Gui (1): drm/amdgpu: add DID for beige goby Christophe JAILLET (1): drm/amd/pm: Fix a memory leak in an error handling path in 'vangogh_tables_init()' Colin Ian King (1): drm/amd/display: Remove redundant initialization of variable eng_id Dan Carpenter (1): drm/amdgpu: check for allocation failure in amdgpu_vkms_sw_init() Eric Huang (7): Revert "Revert "drm/amdkfd: Add heavy-weight TLB flush after unmapping"" Revert "Revert "drm/amdgpu: Add table_freed parameter to amdgpu_vm_bo_update"" Revert "Revert "drm/amdkfd: Make TLB flush conditional on mapping"" Revert "Revert "drm/amdgpu: Fix warning of Function parameter or member not described"" Revert "Revert "drm/amdkfd: Add memory sync before TLB flush on unmap"" Revert "Revert "drm/amdkfd: Only apply TLB flush optimization on ALdebaran"" drm/amdkfd: Only apply heavy-weight TLB flush on Aldebaran Felix Kuehling (1): drm/amdkfd: Allow querying SVM attributes that are clear Graham Sider (1): drm/amdkfd: Expose GFXIP engine version to sysfs Guchun Chen (1): drm/amdgpu: avoid over-handle of fence driver fini in s3 test (v2) Jaehyun Chung (1): drm/amd/display: Add check for validating unsupported ODM plus MPO case John Clements (3): drm/amdgpu: update PSP BL cmd IDs drm/amdgpu: added synchronization for psp cmd buf access drm/amdgpu: set RAS EEPROM address from VBIOS Joseph Greathouse (1): drm/amdgpu: Put MODE register in wave debug info Jude Shih (1): drm/amd/display: Fix resetting DCN3.1 HW when resuming from S4 Kenneth Feng (1): drm/amd/pm: bug fix for the runtime pm BACO Kevin Wang (1): drm/amd/pm: correct aldebaran smu feature mapping FEATURE_DATA_CALCULATIONS Masanari Iida (1): drm/amdgpu/powerplay/smu10: Fix a typo in error message Mukul Joshi (1): drm/amdgpu: Fix channel_index table layout for Aldebaran Peng Ju Zhou (1): drm/amd/amdgpu: Recovery vcn instance iterate. Qingqing Zhuo (1): drm/amd/display: workaround for hard hang on HPD on native DP Randy Dunlap (1): drm/amdgpu: fix checking pmops when PM_SLEEP is not enabled Rodrigo Siqueira (4): drm/amd/display: Move specific DCN2x code that uses FPU to DML drm/amd/display: Add control mechanism for FPU drm/amd/display: Add control mechanism for FPU utilization drm/amd/display: Add DC_FP helper to check FPU state Roman Li (1): drm/amd/display: Remove redundant vblank workqueues in DM Ryan Taylor (3): drm/amdgpu: create amdgpu_vkms (v4) drm/amdgpu: cleanup dce_virtual drm/amdgpu: replace dce_virtual with amdgpu_vkms (v3) Sergio Miguéns Iglesias (1): DRM: gpu: radeon: Fixed coding style issues Shirish S (1): drm/amdgpu/display: fix DMUB firmware version info Solomon Chiu (1): drm/amdgpu: Add preferred mode in modeset when freesync video mode's enabled. Tom St Denis (1): drm/amd/amdgpu: add regCP_MEx_INT_STAT_DEBUG for Aldebaran debugging Tuo Li (1): drm/amdgpu: drop redundant null-pointer checks in amdgpu_ttm_tt_populate() and amdgpu_ttm_tt_unpopulate() Wesley Chalmers (1): drm/amd/display: Assume LTTPR
Re: [Intel-gfx] [PATCH] drm/i915: Disable bonding on gen12+ platforms
On Fri, Aug 06, 2021 at 08:54:59PM +0200, Daniel Vetter wrote:
> On Fri, Aug 6, 2021 at 8:25 PM John Harrison
> wrote:
> > On 7/28/2021 12:21, Matthew Brost wrote:
> > > Disable bonding on gen12+ platforms aside from ones already supported by
> > > the i915 - TGL, RKL, and ADL-S.
> > >
> > > Signed-off-by: Matthew Brost
> > > ---
> > > drivers/gpu/drm/i915/gem/i915_gem_context.c | 7 +++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > index 05c3ee191710..9c3672bac0e2 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > @@ -446,6 +446,13 @@ set_proto_ctx_engines_bond(struct
> > > i915_user_extension __user *base, void *data)
> > > u16 idx, num_bonds;
> > > int err, n;
> > >
> > > + if (GRAPHICS_VER(i915) >= 12 && !IS_TIGERLAKE(i915) &&
> > > + !IS_ROCKETLAKE(i915) && !IS_ALDERLAKE_S(i915)) {
> > > + drm_dbg(>drm,
> > > + "Bonding on gen12+ aside from TGL, RKL, and ADL_S
> > > not allowed\n");
> > I would have said not supported rather than not allowed. Either way:
> > Reviewed-by: John Harrison
>
> Either is fine with me.
>
> Acked-by: Daniel Vetter
>
Applied to drm-intel-gt-next (with the suggested debug message wording
tweak). Thanks for the patch and reviews.
Matt
> >
> > > + return -ENODEV;
> > > + }
> > > +
> > > if (get_user(idx, >virtual_index))
> > > return -EFAULT;
> > >
> >
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
