[PATCH v1 2/2] drm/panel: simple: add LOGIC Technologies LTTD800480070-L6WH-RT
From: Søren Andersen
Add support for the LOGIC Technologies, Inc LTTD800480070-L6WH-RT
Co-Developed-by: Søren Andersen
Co-Developed-by: Sam Ravnborg
Signed-off-by: Søren Andersen
Signed-off-by: Sam Ravnborg
Signed-off-by: Oleksij Rempel
---
drivers/gpu/drm/panel/panel-simple.c | 34
1 file changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index fda79a986d12..e5213a610ae9 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2967,6 +2967,37 @@ static const struct panel_desc logictechno_lt170410_2whc
= {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
};
+static const struct drm_display_mode logictechno_lttd800480070_l6wh_rt_mode = {
+ .clock = 33000,
+ .hdisplay = 800,
+ .hsync_start = 800 + 154,
+ .hsync_end = 800 + 154 + 3,
+ .htotal = 800 + 154 + 3 + 43,
+ .vdisplay = 480,
+ .vsync_start = 480 + 47,
+ .vsync_end = 480 + 47 + 3,
+ .vtotal = 480 + 47 + 3 + 20,
+ .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static const struct panel_desc logictechno_lttd800480070_l6wh_rt = {
+ .modes = _lttd800480070_l6wh_rt_mode,
+ .num_modes = 1,
+ .bpc = 8,
+ .size = {
+ .width = 154,
+ .height = 86,
+ },
+ .delay = {
+ .prepare = 45,
+ .enable = 100,
+ .disable = 100,
+ .unprepare = 45
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+ .bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
+};
+
static const struct drm_display_mode mitsubishi_aa070mc01_mode = {
.clock = 30400,
.hdisplay = 800,
@@ -4492,6 +4523,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "logictechno,lt170410-2whc",
.data = _lt170410_2whc,
+ }, {
+ .compatible = "logictechno,lttd800480070-l6wh-rt",
+ .data = _lttd800480070_l6wh_rt,
}, {
.compatible = "mitsubishi,aa070mc01-ca1",
.data = _aa070mc01,
--
2.30.2
[PATCH v1 1/2] drm/panel: simple: add Multi-Innotechnology MI1010AIT-1CP1
From: Sam Ravnborg
The Multi Innotechnology is a 10.1" 1280x800 panel.
The datasheet did not specify specific values for sync, back, front porch.
The values are a best guess based on values for similar panels.
Co-Developed-by: Sam Ravnborg
Co-Developed-by: Ulrich Ölmann
Signed-off-by: Sam Ravnborg
Signed-off-by: Ulrich Ölmann
Signed-off-by: Oleksij Rempel
---
drivers/gpu/drm/panel/panel-simple.c | 34
1 file changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 21939d4352cf..fda79a986d12 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3033,6 +3033,37 @@ static const struct panel_desc mitsubishi_aa070mc01 = {
.bus_flags = DRM_BUS_FLAG_DE_HIGH,
};
+static const struct display_timing multi_inno_mi1010ait_1cp_timing = {
+ .pixelclock = { 6890, 7000, 7340 },
+ .hactive = { 1280, 1280, 1280 },
+ .hfront_porch = { 30, 60, 71 },
+ .hback_porch = { 30, 60, 71 },
+ .hsync_len = { 10, 10, 48 },
+ .vactive = { 800, 800, 800 },
+ .vfront_porch = { 5, 10, 10 },
+ .vback_porch = { 5, 10, 10 },
+ .vsync_len = { 5, 6, 13 },
+ .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+DISPLAY_FLAGS_DE_HIGH,
+};
+
+static const struct panel_desc multi_inno_mi1010ait_1cp = {
+ .timings = _inno_mi1010ait_1cp_timing,
+ .num_timings = 1,
+ .bpc = 8,
+ .size = {
+ .width = 217,
+ .height = 136,
+ },
+ .delay = {
+ .enable = 50,
+ .disable = 50,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
+ .bus_flags = DRM_BUS_FLAG_DE_HIGH,
+ .connector_type = DRM_MODE_CONNECTOR_LVDS,
+};
+
static const struct display_timing nec_nl12880bc20_05_timing = {
.pixelclock = { 6700, 7100, 7500 },
.hactive = { 1280, 1280, 1280 },
@@ -4464,6 +4495,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "mitsubishi,aa070mc01-ca1",
.data = _aa070mc01,
+ }, {
+ .compatible = "multi-inno,mi1010ait-1cp",
+ .data = _inno_mi1010ait_1cp,
}, {
.compatible = "nec,nl12880bc20-05",
.data = _nl12880bc20_05,
--
2.30.2
Re: [PATCH] drm/amdgpu: drop redundant null-pointer checks in amdgpu_ttm_tt_populate() and amdgpu_ttm_tt_unpopulate()
Applied. Thanks!
Alex
On Wed, Aug 4, 2021 at 2:49 AM Christian König wrote:
>
> Am 04.08.21 um 03:51 schrieb Tuo Li:
> > The varialbe gtt in the function amdgpu_ttm_tt_populate() and
> > amdgpu_ttm_tt_unpopulate() is guaranteed to be not NULL in the context.
> > Thus the null-pointer checks are redundant and can be dropped.
> >
> > Reported-by: TOTE Robot
> > Signed-off-by: Tuo Li
>
> Reviewed-by: Christian König
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 3a55f08e00e1..719539bd6c44 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -1121,7 +1121,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_device
> > *bdev,
> > struct amdgpu_ttm_tt *gtt = (void *)ttm;
> >
> > /* user pages are bound by amdgpu_ttm_tt_pin_userptr() */
> > - if (gtt && gtt->userptr) {
> > + if (gtt->userptr) {
> > ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
> > if (!ttm->sg)
> > return -ENOMEM;
> > @@ -1146,7 +1146,7 @@ static void amdgpu_ttm_tt_unpopulate(struct
> > ttm_device *bdev,
> > struct amdgpu_ttm_tt *gtt = (void *)ttm;
> > struct amdgpu_device *adev;
> >
> > - if (gtt && gtt->userptr) {
> > + if (gtt->userptr) {
> > amdgpu_ttm_tt_set_user_pages(ttm, NULL);
> > kfree(ttm->sg);
> > ttm->sg = NULL;
>
Re: [PATCH v2] drm/radeon: Update pitch for page flip
On Tue, Aug 3, 2021 at 10:39 PM Zhenneng Li wrote:
>
>
> When primary bo is updated, crtc's pitch may
> have not been updated, this will lead to show
> disorder content when user changes display mode,
> we update crtc's pitch in page flip to avoid
> this bug.
> This refers to amdgpu's pageflip.
>
> v1->v2:
> Update all of the pitch in all of the page_flip functions
> in radeon rather than just the evergreen one.
>
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: "Pan, Xinhui"
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: amd-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Zhenneng Li
> ---
> drivers/gpu/drm/radeon/evergreen.c | 8 +++-
> drivers/gpu/drm/radeon/r100.c | 5 +
> drivers/gpu/drm/radeon/rs600.c | 8 +++-
> drivers/gpu/drm/radeon/rv770.c | 8 +++-
> 4 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen.c
> b/drivers/gpu/drm/radeon/evergreen.c
> index 36a888e1b179..eeb590d2dec2 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -28,6 +28,7 @@
>
> #include
> #include
> +#include
>
> #include "atom.h"
> #include "avivod.h"
> @@ -1414,10 +1415,15 @@ void evergreen_page_flip(struct radeon_device *rdev,
> int crtc_id, u64 crtc_base,
> bool async)
> {
> struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
> + struct drm_framebuffer *fb = radeon_crtc->base.primary->fb;
>
> - /* update the scanout addresses */
> + /* flip at hsync for async, default is vsync */
> WREG32(EVERGREEN_GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset,
>async ? EVERGREEN_GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0);
> + /* update pitch */
> + WREG32(EVERGREEN_GRPH_PITCH + radeon_crtc->crtc_offset,
> + fb->pitches[0] / fb->format->cpp[0]);
> + /* update the scanout addresses */
> WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH +
> radeon_crtc->crtc_offset,
>upper_32_bits(crtc_base));
> WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS +
> radeon_crtc->crtc_offset,
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index ba724198b72e..1268854552ff 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -162,6 +162,7 @@ void r100_wait_for_vblank(struct radeon_device *rdev, int
> crtc)
> void r100_page_flip(struct radeon_device *rdev, int crtc_id, u64 crtc_base,
> bool async)
> {
> struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
> + struct drm_framebuffer *fb = radeon_crtc->base.primary->fb;
> u32 tmp = ((u32)crtc_base) | RADEON_CRTC_OFFSET__OFFSET_LOCK;
> int i;
>
> @@ -169,6 +170,10 @@ void r100_page_flip(struct radeon_device *rdev, int
> crtc_id, u64 crtc_base, bool
> /* update the scanout addresses */
> WREG32(RADEON_CRTC_OFFSET + radeon_crtc->crtc_offset, tmp);
>
> + /* update pitch */
> + WREG32(RADEON_CRTC_PITCH + radeon_crtc->crtc_offset,
> + fb->pitches[0] / fb->format->cpp[0]);
> +
This needs the follow formatting (from radeon_legacy_crtc.c):
pitch_pixels = fb->pitches[0] / fb->format->cpp[0];
crtc_pitch = DIV_ROUND_UP(pitch_pixels * fb->format->cpp[0] * 8,
fb->format->cpp[0] * 8 * 8);
crtc_pitch |= crtc_pitch << 16;
WREG32(RADEON_CRTC_PITCH + radeon_crtc->crtc_offset, crtc_pitch);
With that fixed,
Reviewed-by: Alex Deucher
> /* Wait for update_pending to go high. */
> for (i = 0; i < rdev->usec_timeout; i++) {
> if (RREG32(RADEON_CRTC_OFFSET + radeon_crtc->crtc_offset) &
> RADEON_CRTC_OFFSET__GUI_TRIG_OFFSET)
> diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
> index b2d22e25eee1..b87dd551e939 100644
> --- a/drivers/gpu/drm/radeon/rs600.c
> +++ b/drivers/gpu/drm/radeon/rs600.c
> @@ -41,6 +41,7 @@
>
> #include
> #include
> +#include
>
> #include "atom.h"
> #include "radeon.h"
> @@ -118,6 +119,7 @@ void avivo_wait_for_vblank(struct radeon_device *rdev,
> int crtc)
> void rs600_page_flip(struct radeon_device *rdev, int crtc_id, u64 crtc_base,
> bool async)
> {
> struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
> + struct drm_framebuffer *fb = radeon_crtc->base.primary->fb;
> u32 tmp = RREG32(AVIVO_D1GRPH_UPDATE + radeon_crtc->crtc_offset);
> int i;
>
> @@ -125,9 +127,13 @@ void rs600_page_flip(struct radeon_device *rdev, int
> crtc_id, u64 crtc_base, boo
> tmp |= AVIVO_D1GRPH_UPDATE_LOCK;
> WREG32(AVIVO_D1GRPH_UPDATE + radeon_crtc->crtc_offset, tmp);
>
> - /* update the scanout addresses */
> + /* flip at hsync for async, default is vsync */
> WREG32(AVIVO_D1GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset,
>
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. > > Hence my gut feeling reaction that first we need to get these two > compositors aligned in their timings, which propobably needs > consistent vblank periods/timestamps across them (plus/minux > guest/host clocksource fun ofc). Without this any of the next steps > will simply not work because there's too much jitter by the time the > guest compositor gets the flip completion events. [Kasireddy, Vivek] Timings are not a problem and do not significantly affect the repaint cycles from what I have seen so far. > > Once we have solid events I think we should look into statically > tuning guest/host compositor deadlines (like you've suggested in a > bunch of places) to consisently make that deadline and hit 60 fps. > With that we can then look into tuning this automatically and what to > do when e.g. switching between copying and zero-copy on the host side > (which might be needed in some cases) and how to handle all that. [Kasireddy, Vivek] As I confirm here: https://gitlab.freedesktop.org/wayland/weston/-/issues/514#note_984065 tweaking the deadlines works (i.e., we get 60 FPS) as we expect. However, I feel that this zero-copy solution I am trying to create should be independent of compositors' deadlines, delays or other scheduling
[pull] amdgpu drm-fixes-5.14
Hi Dave, Daniel, Fixes for 5.14. The following changes since commit d28e2568ac26fff351c846bf74ba6ca5dded733e: Merge tag 'amd-drm-fixes-5.14-2021-07-28' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes (2021-07-29 17:20:29 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-fixes-5.14-2021-08-04 for you to fetch changes up to 574fdb20f3e2b001eeddcaf4f16a5c8258243323: drm/amdgpu: add DID for beige goby (2021-08-03 16:59:16 -0400) amd-drm-fixes-5.14-2021-08-04: 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 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 Jude Shih (1): drm/amd/display: Fix resetting DCN3.1 HW when resuming from S4 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 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. 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 .../gpu/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 +- 14 files changed, 83 insertions(+), 31 deletions(-)
Re: [PATCH 2/2] drm/i915: delete gpu reloc code
On Tue, Aug 03, 2021 at 10:47:10AM -0500, Jason Ekstrand wrote:
> Both are
>
> Reviewed-by: Jason Ekstrand
CI is happy, I guess you got all the igt changes indeed. Both pushed
thanks for reviewing.
-Daniel
>
> On Tue, Aug 3, 2021 at 7:49 AM Daniel Vetter wrote:
> >
> > It's already removed, this just garbage collects it all.
> >
> > v2: Rebase over s/GEN/GRAPHICS_VER/
> >
> > v3: Also ditch eb.reloc_pool and eb.reloc_context (Maarten)
> >
> > Signed-off-by: Daniel Vetter
> > Cc: Jon Bloomfield
> > Cc: Chris Wilson
> > Cc: Maarten Lankhorst
> > Cc: Daniel Vetter
> > Cc: Joonas Lahtinen
> > Cc: "Thomas Hellström"
> > Cc: Matthew Auld
> > Cc: Lionel Landwerlin
> > Cc: Dave Airlie
> > Cc: Jason Ekstrand
> > ---
> > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 360 +-
> > .../drm/i915/selftests/i915_live_selftests.h | 1 -
> > 2 files changed, 1 insertion(+), 360 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index e4dc4c3b4df3..98e25efffb59 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -277,16 +277,8 @@ struct i915_execbuffer {
> > bool has_llc : 1;
> > bool has_fence : 1;
> > bool needs_unfenced : 1;
> > -
> > - struct i915_request *rq;
> > - u32 *rq_cmd;
> > - unsigned int rq_size;
> > - struct intel_gt_buffer_pool_node *pool;
> > } reloc_cache;
> >
> > - struct intel_gt_buffer_pool_node *reloc_pool; /** relocation pool
> > for -EDEADLK handling */
> > - struct intel_context *reloc_context;
> > -
> > u64 invalid_flags; /** Set of execobj.flags that are invalid */
> >
> > u64 batch_len; /** Length of batch within object */
> > @@ -1035,8 +1027,6 @@ static void eb_release_vmas(struct i915_execbuffer
> > *eb, bool final)
> >
> > static void eb_destroy(const struct i915_execbuffer *eb)
> > {
> > - GEM_BUG_ON(eb->reloc_cache.rq);
> > -
> > if (eb->lut_size > 0)
> > kfree(eb->buckets);
> > }
> > @@ -1048,14 +1038,6 @@ relocation_target(const struct
> > drm_i915_gem_relocation_entry *reloc,
> > return gen8_canonical_addr((int)reloc->delta + target->node.start);
> > }
> >
> > -static void reloc_cache_clear(struct reloc_cache *cache)
> > -{
> > - cache->rq = NULL;
> > - cache->rq_cmd = NULL;
> > - cache->pool = NULL;
> > - cache->rq_size = 0;
> > -}
> > -
> > static void reloc_cache_init(struct reloc_cache *cache,
> > struct drm_i915_private *i915)
> > {
> > @@ -1068,7 +1050,6 @@ static void reloc_cache_init(struct reloc_cache
> > *cache,
> > cache->has_fence = cache->graphics_ver < 4;
> > cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
> > cache->node.flags = 0;
> > - reloc_cache_clear(cache);
> > }
> >
> > static inline void *unmask_page(unsigned long p)
> > @@ -1090,48 +1071,10 @@ static inline struct i915_ggtt
> > *cache_to_ggtt(struct reloc_cache *cache)
> > return >ggtt;
> > }
> >
> > -static void reloc_cache_put_pool(struct i915_execbuffer *eb, struct
> > reloc_cache *cache)
> > -{
> > - if (!cache->pool)
> > - return;
> > -
> > - /*
> > -* This is a bit nasty, normally we keep objects locked until the
> > end
> > -* of execbuffer, but we already submit this, and have to unlock
> > before
> > -* dropping the reference. Fortunately we can only hold 1 pool node
> > at
> > -* a time, so this should be harmless.
> > -*/
> > - i915_gem_ww_unlock_single(cache->pool->obj);
> > - intel_gt_buffer_pool_put(cache->pool);
> > - cache->pool = NULL;
> > -}
> > -
> > -static void reloc_gpu_flush(struct i915_execbuffer *eb, struct reloc_cache
> > *cache)
> > -{
> > - struct drm_i915_gem_object *obj = cache->rq->batch->obj;
> > -
> > - GEM_BUG_ON(cache->rq_size >= obj->base.size / sizeof(u32));
> > - cache->rq_cmd[cache->rq_size] = MI_BATCH_BUFFER_END;
> > -
> > - i915_gem_object_flush_map(obj);
> > - i915_gem_object_unpin_map(obj);
> > -
> > - intel_gt_chipset_flush(cache->rq->engine->gt);
> > -
> > - i915_request_add(cache->rq);
> > - reloc_cache_put_pool(eb, cache);
> > - reloc_cache_clear(cache);
> > -
> > - eb->reloc_pool = NULL;
> > -}
> > -
> > static void reloc_cache_reset(struct reloc_cache *cache, struct
> > i915_execbuffer *eb)
> > {
> > void *vaddr;
> >
> > - if (cache->rq)
> > - reloc_gpu_flush(eb, cache);
> > -
> > if (!cache->vaddr)
> > return;
> >
> > @@ -1313,295 +1256,6 @@ static void clflush_write32(u32 *addr, u32 value,
> > unsigned int flushes)
> > *addr = value;
> > }
> >
> >
Re: [Intel-gfx] [PATCH] fbdev/efifb: Release PCI device's runtime PM ref during FB destroy
On Mon, Aug 02, 2021 at 04:35:51PM +0300, Imre Deak wrote:
> Atm the EFI FB driver gets a runtime PM reference for the associated GFX
> PCI device during driver probing and releases it only when removing the
> driver.
>
> When fbcon switches to the FB provided by the PCI device's driver (for
> instance i915/drmfb), the EFI FB will get only unregistered without the
> EFI FB driver getting unloaded, keeping the runtime PM reference
> acquired during driver probing. This reference will prevent the PCI
> driver from runtime suspending the device.
>
> Fix this by releasing the RPM reference from the EFI FB's destroy hook,
> called when the FB gets unregistered.
>
> Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at PCI
> D0")
> Cc: Kai-Heng Feng
> Signed-off-by: Imre Deak
Patch looks good:
Reviewed-by: Daniel Vetter
But I've found a bunch of ordering issues here:
- we should probably get the runtime pm reference _before_ we register the
framebuffer. There's a race right now about there.
- the sysfs_remove_groups and framebuffer_release should also be moved
into the destroy callback. This is more a leak type of situation.
Cheers, Daniel
> ---
> drivers/video/fbdev/efifb.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 8ea8f079cde26..25cdea32b9633 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -47,6 +47,8 @@ static bool use_bgrt = true;
> static bool request_mem_succeeded = false;
> static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC;
>
> +static struct pci_dev *efifb_pci_dev;/* dev with BAR covering the
> efifb */
> +
> static struct fb_var_screeninfo efifb_defined = {
> .activate = FB_ACTIVATE_NOW,
> .height = -1,
> @@ -243,6 +245,9 @@ static inline void efifb_show_boot_graphics(struct
> fb_info *info) {}
>
> static void efifb_destroy(struct fb_info *info)
> {
> + if (efifb_pci_dev)
> + pm_runtime_put(_pci_dev->dev);
> +
> if (info->screen_base) {
> if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC))
> iounmap(info->screen_base);
> @@ -333,7 +338,6 @@ ATTRIBUTE_GROUPS(efifb);
>
> static bool pci_dev_disabled;/* FB base matches BAR of a disabled
> device */
>
> -static struct pci_dev *efifb_pci_dev;/* dev with BAR covering the
> efifb */
> static struct resource *bar_resource;
> static u64 bar_offset;
>
> @@ -603,8 +607,6 @@ static int efifb_remove(struct platform_device *pdev)
> unregister_framebuffer(info);
> sysfs_remove_groups(>dev.kobj, efifb_groups);
> framebuffer_release(info);
> - if (efifb_pci_dev)
> - pm_runtime_put(_pci_dev->dev);
>
> return 0;
> }
> --
> 2.27.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH] depend on BACKLIGHT_CLASS_DEVICE for more devices
On Wed, Aug 4, 2021 at 11:10 PM Arnd Bergmann wrote: > > On Wed, Aug 4, 2021 at 8:59 PM Karol Herbst wrote: > > On Wed, Aug 4, 2021 at 4:43 PM Karol Herbst wrote: > > > On Wed, Aug 4, 2021 at 4:19 PM Arnd Bergmann wrote: > > > > On Wed, Aug 4, 2021 at 4:10 PM Karol Herbst wrote: > > > > > > > > > > playing around a little bit with this, I think the original "select > > > > > BACKLIGHT_CLASS_DEVICE" is fine. Atm we kind of have this weird mix of > > > > > drivers selecting and others depending on it. We could of course > > > > > convert > > > > > everything over to depend, and break those cycling dependency issues > > > > > with > > > > > this. > > > > > > > > > > Anyway this change on top of my initial patch is enough to make > > > > > Kconfig > > > > > happy and has the advantage of not having to mess with the deps of > > > > > nouveau > > > > > too much. > > > > > > > > Looks good to me. We'd probably want to make the BACKLIGHT_CLASS_DEVICE > > > > option itself 'default FB || DRM' though, to ensure that defconfigs > > > > keep working. > > > > > > > > > > okay cool. Will send out a proper updated patch series soonish. > > > > > > > mhh, actually that breaks drivers selecting FB_BACKLIGHT as now > > BACKLIGHT_CLASS_DEVICE might be disabled :( > > Are you sure? It should already be the case that any driver that selects > FB_BACKLIGHT either 'depends on BACKLIGHT_CLASS_DEVICE' > or 'select BACKLIGHT_CLASS_DEVICE'. > none of the fb drivers seem to do that. > If you change all the 'select BACKLIGHT_CLASS_DEVICE' to 'depends > on', I don't see a problem with doing 'select FB_BACKLIGHT' from > those. > > I have applied your patch to my randconfig tree and built a few dozen > kernels, don't see any regressions so far, but will let it run over night. > > Arnd >
Re: [PATCH] docs/drm: Add a new bullet to the uAPI requirements (v2)
On Wed, 4 Aug 2021 at 19:57, Jason Ekstrand wrote: > While tracking down various bits of i915 uAPI, it's been difficult to > find the userspace much of the time because no one bothers to mention it > in commit messages. Require the kernel patch to be a one-stop shop for > finding the various bits which were used to justify the new uAPI. Acked-by: Daniel Stone
Re: [PATCH v4 1/3] drm/loongson: Add DRM Driver for Loongson 7A1000 bridge chip
Hi Chenyang, some feedback in the following. I will try to find more time for review during the week. Hi Thomas, please see my question near drm_gem_vram_of_gem(). Sam On Fri, Jul 30, 2021 at 05:41:46PM +0800, lichenyang wrote: > From: Chenyang Li > > This patch adds an initial DRM driver for the Loongson LS7A1000 > bridge chip(LS7A). The LS7A bridge chip contains two display > controllers, support dual display output. The maximum support for > each channel display is to 1920x1080@60Hz. > At present, DC device detection and DRM driver registration are > completed, the crtc/plane/encoder/connector objects has been > implemented. > On Loongson 3A4000 CPU and 7A1000 system, we have achieved the use > of dual screen, and support dual screen clone mode and expansion > mode. > > v10: > - Replace the drmm_ version functions. > - Replace the simple_encoder version function. > - Alphabetize file names. > > v9: > - Optimize the error handling process. > - Remove the useless flags parameter. > - Fix some incorrect use of variables and constructs. > > v8: > - Update the atomic_update function interface. > > v7: > - The pixel clock is limited to less than 173000. > > v6: > - Remove spin_lock in mmio reg read and write. > - TO_UNCAC is replac with ioremap. > - Fix error arguments in crtc_atomic_enable/disable/mode_valid. > > v5: > - Change the name of the chip to LS7A. > - Change magic value in crtc to macros. > - Correct mistakes words. > - Change the register operation function prefix to ls7a. > > v4: > - Move the mode_valid function to the crtc. > > v3: > - Move the mode_valid function to the connector and optimize it. > - Fix num_crtc calculation method. > > v2: > - Complete the case of 32-bit color in CRTC. > > Signed-off-by: Chenyang Li > --- > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/loongson/Kconfig | 14 + > drivers/gpu/drm/loongson/Makefile | 14 + > drivers/gpu/drm/loongson/loongson_connector.c | 47 +++ > drivers/gpu/drm/loongson/loongson_crtc.c | 238 +++ > drivers/gpu/drm/loongson/loongson_device.c| 35 +++ > drivers/gpu/drm/loongson/loongson_drv.c | 271 ++ > drivers/gpu/drm/loongson/loongson_drv.h | 149 ++ > drivers/gpu/drm/loongson/loongson_encoder.c | 21 ++ > drivers/gpu/drm/loongson/loongson_plane.c | 92 ++ > 11 files changed, 884 insertions(+) > create mode 100644 drivers/gpu/drm/loongson/Kconfig > create mode 100644 drivers/gpu/drm/loongson/Makefile > create mode 100644 drivers/gpu/drm/loongson/loongson_connector.c > create mode 100644 drivers/gpu/drm/loongson/loongson_crtc.c > create mode 100644 drivers/gpu/drm/loongson/loongson_device.c > create mode 100644 drivers/gpu/drm/loongson/loongson_drv.c > create mode 100644 drivers/gpu/drm/loongson/loongson_drv.h > create mode 100644 drivers/gpu/drm/loongson/loongson_encoder.c > create mode 100644 drivers/gpu/drm/loongson/loongson_plane.c > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 7ff89690a976..08562d9be6e3 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -365,6 +365,8 @@ source "drivers/gpu/drm/xen/Kconfig" > > source "drivers/gpu/drm/vboxvideo/Kconfig" > > +source "drivers/gpu/drm/loongson/Kconfig" > + > source "drivers/gpu/drm/lima/Kconfig" Preferably in alphabetical order, so after lima. > > source "drivers/gpu/drm/panfrost/Kconfig" > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index a118692a6df7..29c05b8cf2ad 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -119,6 +119,7 @@ obj-$(CONFIG_DRM_PL111) += pl111/ > obj-$(CONFIG_DRM_TVE200) += tve200/ > obj-$(CONFIG_DRM_XEN) += xen/ > obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/ > +obj-$(CONFIG_DRM_LOONGSON) += loongson/ > obj-$(CONFIG_DRM_LIMA) += lima/ Likewise, after lima > obj-$(CONFIG_DRM_PANFROST) += panfrost/ > obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/ > diff --git a/drivers/gpu/drm/loongson/Kconfig > b/drivers/gpu/drm/loongson/Kconfig > new file mode 100644 > index ..3cf42a4cca08 > --- /dev/null > +++ b/drivers/gpu/drm/loongson/Kconfig > @@ -0,0 +1,14 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +config DRM_LOONGSON > + tristate "DRM support for LS7A bridge chipset" > + depends on DRM && PCI > + depends on CPU_LOONGSON64 Maybe add || COMPILE_TEST - so we get better build coverage. You risk we miss this driver when we do refactoring, if we cannot build it using an allmodconfig for example. > + select DRM_KMS_HELPER > + select DRM_VRAM_HELPER > + select DRM_TTM > + select DRM_TTM_HELPER Please verify that they are all needed. There are no hits on "ttm" in the code, so the the two TTM symbols is likely wrong. > + default n Drop this. n is default. > + help > + Support the display controllers
Re: [PATCH] depend on BACKLIGHT_CLASS_DEVICE for more devices
On Wed, Aug 4, 2021 at 8:59 PM Karol Herbst wrote: > On Wed, Aug 4, 2021 at 4:43 PM Karol Herbst wrote: > > On Wed, Aug 4, 2021 at 4:19 PM Arnd Bergmann wrote: > > > On Wed, Aug 4, 2021 at 4:10 PM Karol Herbst wrote: > > > > > > > > playing around a little bit with this, I think the original "select > > > > BACKLIGHT_CLASS_DEVICE" is fine. Atm we kind of have this weird mix of > > > > drivers selecting and others depending on it. We could of course convert > > > > everything over to depend, and break those cycling dependency issues > > > > with > > > > this. > > > > > > > > Anyway this change on top of my initial patch is enough to make Kconfig > > > > happy and has the advantage of not having to mess with the deps of > > > > nouveau > > > > too much. > > > > > > Looks good to me. We'd probably want to make the BACKLIGHT_CLASS_DEVICE > > > option itself 'default FB || DRM' though, to ensure that defconfigs > > > keep working. > > > > > > > okay cool. Will send out a proper updated patch series soonish. > > > > mhh, actually that breaks drivers selecting FB_BACKLIGHT as now > BACKLIGHT_CLASS_DEVICE might be disabled :( Are you sure? It should already be the case that any driver that selects FB_BACKLIGHT either 'depends on BACKLIGHT_CLASS_DEVICE' or 'select BACKLIGHT_CLASS_DEVICE'. If you change all the 'select BACKLIGHT_CLASS_DEVICE' to 'depends on', I don't see a problem with doing 'select FB_BACKLIGHT' from those. I have applied your patch to my randconfig tree and built a few dozen kernels, don't see any regressions so far, but will let it run over night. Arnd
Re: [PATCH -next] drm/i915: fix i915_globals_exit() section mismatch error
On Wed, Aug 4, 2021 at 3:41 PM Randy Dunlap wrote:
>
> Fix modpost Section mismatch error in i915_globals_exit().
> Since both an __init function and an __exit function can call
> i915_globals_exit(), any function that i915_globals_exit() calls
> should not be marked as __init or __exit. I.e., it needs to be
> available for either of them.
>
> WARNING: modpost: vmlinux.o(.text+0x8b796a): Section mismatch in reference
> from the function i915_globals_exit() to the function
> .exit.text:__i915_globals_flush()
> The function i915_globals_exit() references a function in an exit section.
> Often the function __i915_globals_flush() has valid usage outside the exit
> section
> and the fix is to remove the __exit annotation of __i915_globals_flush.
>
> ERROR: modpost: Section mismatches detected.
> Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.
My gut says we actually want to back-port
https://lore.kernel.org/dri-devel/YPk3OCMrhg7UlU6T@phenom.ffwll.local/
instead. Daniel, thoughts?
--Jason
>
> Fixes: 1354d830cb8f ("drm/i915: Call i915_globals_exit() if
> pci_register_device() fails")
> Signed-off-by: Randy Dunlap
> Cc: Jason Ekstrand
> Cc: Daniel Vetter
> Cc: Rodrigo Vivi
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: intel-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/i915/i915_globals.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linext-2021-0804.orig/drivers/gpu/drm/i915/i915_globals.c
> +++ linext-2021-0804/drivers/gpu/drm/i915/i915_globals.c
> @@ -138,7 +138,7 @@ void i915_globals_unpark(void)
> atomic_inc();
> }
>
> -static void __exit __i915_globals_flush(void)
> +static void __i915_globals_flush(void)
> {
> atomic_inc(); /* skip shrinking */
>
[PATCH -next] drm/i915: fix i915_globals_exit() section mismatch error
Fix modpost Section mismatch error in i915_globals_exit().
Since both an __init function and an __exit function can call
i915_globals_exit(), any function that i915_globals_exit() calls
should not be marked as __init or __exit. I.e., it needs to be
available for either of them.
WARNING: modpost: vmlinux.o(.text+0x8b796a): Section mismatch in reference from
the function i915_globals_exit() to the function
.exit.text:__i915_globals_flush()
The function i915_globals_exit() references a function in an exit section.
Often the function __i915_globals_flush() has valid usage outside the exit
section
and the fix is to remove the __exit annotation of __i915_globals_flush.
ERROR: modpost: Section mismatches detected.
Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.
Fixes: 1354d830cb8f ("drm/i915: Call i915_globals_exit() if
pci_register_device() fails")
Signed-off-by: Randy Dunlap
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Rodrigo Vivi
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: intel-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/i915/i915_globals.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linext-2021-0804.orig/drivers/gpu/drm/i915/i915_globals.c
+++ linext-2021-0804/drivers/gpu/drm/i915/i915_globals.c
@@ -138,7 +138,7 @@ void i915_globals_unpark(void)
atomic_inc();
}
-static void __exit __i915_globals_flush(void)
+static void __i915_globals_flush(void)
{
atomic_inc(); /* skip shrinking */
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Do not define vma on stack
On Mon, Aug 02, 2021 at 10:11:18PM -0700, Matthew Brost wrote:
> From: Venkata Sandeep Dhanalakota
>
> Defining vma on stack can cause stack overflow, if
> vma gets populated with new fields.
>
> Cc: Daniele Ceraolo Spurio
> Cc: Tvrtko Ursulin
> Signed-off-by: Venkata Sandeep Dhanalakota
> Signef-off-by: Matthew Brost
Reviewed-by: Matthew Brost
> ---
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 18 +-
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 ++
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
> 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 3a16d08608a5..f632dbd32b42 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -413,20 +413,20 @@ static void uc_fw_bind_ggtt(struct intel_uc_fw *uc_fw)
> {
> struct drm_i915_gem_object *obj = uc_fw->obj;
> struct i915_ggtt *ggtt = __uc_fw_to_gt(uc_fw)->ggtt;
> - struct i915_vma dummy = {
> - .node.start = uc_fw_ggtt_offset(uc_fw),
> - .node.size = obj->base.size,
> - .pages = obj->mm.pages,
> - .vm = >vm,
> - };
> + struct i915_vma *dummy = _fw->dummy;
> +
> + dummy->node.start = uc_fw_ggtt_offset(uc_fw);
> + dummy->node.size = obj->base.size;
> + dummy->pages = obj->mm.pages;
> + dummy->vm = >vm;
>
> GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
> - GEM_BUG_ON(dummy.node.size > ggtt->uc_fw.size);
> + GEM_BUG_ON(dummy->node.size > ggtt->uc_fw.size);
>
> /* uc_fw->obj cache domains were not controlled across suspend */
> - drm_clflush_sg(dummy.pages);
> + drm_clflush_sg(dummy->pages);
>
> - ggtt->vm.insert_entries(>vm, , I915_CACHE_NONE, 0);
> + ggtt->vm.insert_entries(>vm, dummy, I915_CACHE_NONE, 0);
> }
>
> static void uc_fw_unbind_ggtt(struct intel_uc_fw *uc_fw)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> index 99bb1fe1af66..693cc0ebcd63 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -10,6 +10,7 @@
> #include "intel_uc_fw_abi.h"
> #include "intel_device_info.h"
> #include "i915_gem.h"
> +#include "i915_vma.h"
>
> struct drm_printer;
> struct drm_i915_private;
> @@ -75,6 +76,7 @@ struct intel_uc_fw {
> bool user_overridden;
> size_t size;
> struct drm_i915_gem_object *obj;
> + struct i915_vma dummy;
>
> /*
>* The firmware build process will generate a version header file with
> major and
> --
> 2.28.0
>
Re: [PATCH] depend on BACKLIGHT_CLASS_DEVICE for more devices
On Wed, Aug 4, 2021 at 4:43 PM Karol Herbst wrote: > > On Wed, Aug 4, 2021 at 4:19 PM Arnd Bergmann wrote: > > > > On Wed, Aug 4, 2021 at 4:10 PM Karol Herbst wrote: > > > > > > playing around a little bit with this, I think the original "select > > > BACKLIGHT_CLASS_DEVICE" is fine. Atm we kind of have this weird mix of > > > drivers selecting and others depending on it. We could of course convert > > > everything over to depend, and break those cycling dependency issues with > > > this. > > > > > > Anyway this change on top of my initial patch is enough to make Kconfig > > > happy and has the advantage of not having to mess with the deps of nouveau > > > too much. > > > > Looks good to me. We'd probably want to make the BACKLIGHT_CLASS_DEVICE > > option itself 'default FB || DRM' though, to ensure that defconfigs > > keep working. > > > > okay cool. Will send out a proper updated patch series soonish. > mhh, actually that breaks drivers selecting FB_BACKLIGHT as now BACKLIGHT_CLASS_DEVICE might be disabled :( somehow it doesn't feel like worth the effort converting it all over to depend.. dunno. Atm I would just use "select" in nouveau and deal with the conversion later once somebody gets annoyed enough or so... > > Arnd > >
[PATCH] docs/drm: Add a new bullet to the uAPI requirements (v2)
While tracking down various bits of i915 uAPI, it's been difficult to find the userspace much of the time because no one bothers to mention it in commit messages. Require the kernel patch to be a one-stop shop for finding the various bits which were used to justify the new uAPI. v2 (Daniel Vetter): - Minor wording tweaks Signed-off-by: Jason Ekstrand Acked-by: Daniel Vetter Cc: Dave Airlie --- Documentation/gpu/drm-uapi.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 199afb503ab1..7b398c6fadc6 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -109,6 +109,11 @@ leads to a few additional requirements: userspace patches land. uAPI always flows from the kernel, doing things the other way round risks divergence of the uAPI definitions and header files. +- The kernel patch which adds the new uAPI **must** reference the patch series + or merge requests in the userspaces projects which demonstrate the use of the + new uAPI and against which the review was done so that future developers can + find all of the pieces which tie together. + These are fairly steep requirements, but have grown out from years of shared pain and experience with uAPI added hastily, and almost always regretted about just as fast. GFX devices change really fast, requiring a paradigm shift and -- 2.31.1
Re: [PATCH] docs/drm: Add a new bullet to the uAPI requirements
On Wed, Aug 4, 2021 at 8:50 PM Jason Ekstrand wrote: > > On Wed, Aug 4, 2021 at 1:48 PM Jason Ekstrand wrote: > > > > While tracking down various bits of i915 uAPI, it's been difficult to > > find the userspace much of the time because no one bothers to mention it > > in commit messages. Require the kernel patch to be a one-stop shop for > > finding the various bits which were used to justify the new uAPI. > > > > Signed-off-by: Jason Ekstrand > > Cc: Daniel Vetter > > Cc: Dave Airlie > > --- > > Documentation/gpu/drm-uapi.rst | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > > index 199afb503ab1..82f780bc3fce 100644 > > --- a/Documentation/gpu/drm-uapi.rst > > +++ b/Documentation/gpu/drm-uapi.rst > > @@ -109,6 +109,11 @@ leads to a few additional requirements: > >userspace patches land. uAPI always flows from the kernel, doing things > > the > >other way round risks divergence of the uAPI definitions and header > > files. > > > > +- The kernel patch which adds the new uAPI **must** reference the patch > > series > > + or merge requests in the userspaces project which use the new uAPI and > > Locally, I've done s/project which use/projects which demonstrate/ While we paint this shed ... projects which demonstrate the use of the new uAPI? Either way this seems like a fine addition to our docs and process, since grepping through all the things just to figure out what something does because documentation was deemed optional too gets boring. Acked-by: Daniel Vetter -Daniel > > --Jason > > > + against which the review was done so that future developers can find all > > of > > + the pieces which tie together. > > + > > These are fairly steep requirements, but have grown out from years of > > shared > > pain and experience with uAPI added hastily, and almost always regretted > > about > > just as fast. GFX devices change really fast, requiring a paradigm shift > > and > > -- > > 2.31.1 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] docs/drm: Add a new bullet to the uAPI requirements
On Wed, Aug 4, 2021 at 1:48 PM Jason Ekstrand wrote: > > While tracking down various bits of i915 uAPI, it's been difficult to > find the userspace much of the time because no one bothers to mention it > in commit messages. Require the kernel patch to be a one-stop shop for > finding the various bits which were used to justify the new uAPI. > > Signed-off-by: Jason Ekstrand > Cc: Daniel Vetter > Cc: Dave Airlie > --- > Documentation/gpu/drm-uapi.rst | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 199afb503ab1..82f780bc3fce 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -109,6 +109,11 @@ leads to a few additional requirements: >userspace patches land. uAPI always flows from the kernel, doing things the >other way round risks divergence of the uAPI definitions and header files. > > +- The kernel patch which adds the new uAPI **must** reference the patch > series > + or merge requests in the userspaces project which use the new uAPI and Locally, I've done s/project which use/projects which demonstrate/ --Jason > + against which the review was done so that future developers can find all of > + the pieces which tie together. > + > These are fairly steep requirements, but have grown out from years of shared > pain and experience with uAPI added hastily, and almost always regretted > about > just as fast. GFX devices change really fast, requiring a paradigm shift and > -- > 2.31.1 >
[PATCH] docs/drm: Add a new bullet to the uAPI requirements
While tracking down various bits of i915 uAPI, it's been difficult to find the userspace much of the time because no one bothers to mention it in commit messages. Require the kernel patch to be a one-stop shop for finding the various bits which were used to justify the new uAPI. Signed-off-by: Jason Ekstrand Cc: Daniel Vetter Cc: Dave Airlie --- Documentation/gpu/drm-uapi.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 199afb503ab1..82f780bc3fce 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -109,6 +109,11 @@ leads to a few additional requirements: userspace patches land. uAPI always flows from the kernel, doing things the other way round risks divergence of the uAPI definitions and header files. +- The kernel patch which adds the new uAPI **must** reference the patch series + or merge requests in the userspaces project which use the new uAPI and + against which the review was done so that future developers can find all of + the pieces which tie together. + These are fairly steep requirements, but have grown out from years of shared pain and experience with uAPI added hastily, and almost always regretted about just as fast. GFX devices change really fast, requiring a paradigm shift and -- 2.31.1
Re: [PATCH v2 11/14] drm/tilcdc: Convert to Linux IRQ interfaces
Hi Thomas, On Wed, Aug 04, 2021 at 08:30:41PM +0200, Thomas Zimmermann wrote: > Hi > > Am 03.08.21 um 17:00 schrieb Sam Ravnborg: > > Hi Thomas, > > > > On Tue, Aug 03, 2021 at 11:07:01AM +0200, Thomas Zimmermann wrote: > > > Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's > > > IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers > > > don't benefit from using it. > > > > > > DRM IRQ callbacks are now being called directly or inlined. > > > > > > Calls to platform_get_irq() can fail with a negative errno code. > > > Abort initialization in this case. The DRM IRQ midlayer does not > > > handle this case correctly. > > > > I cannot see why the irq_enabled flag is needed here, and the changelog > > do not help me. > > What do I miss? > > That's a good point. Usually, only the DRM IRQ helpers use irq_enabled in > struct drm_device. It'll become legacy and we can ignore it for the driver > conversion. > > The exception is tilcdc, which also uses irq_enabled to make its error > rollback work correctly. So I duplicated the state in the local private > structure. > > I'll add this explanation to the commit message. With this added: Acked-by: Sam Ravnborg
[PULL] drm-intel-fixes
Hi Dave and Daniel, Here goes drm-intel-fixes-2021-08-04: - Call i915_globals_exit if pci_register_device fails (Jason) - Correct SFC_DONE register offset (Matt) Thanks, Rodrigo. 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-intel tags/drm-intel-fixes-2021-08-04 for you to fetch changes up to 1354d830cb8f9be966cc07fc61368af27ffb7c4a: drm/i915: Call i915_globals_exit() if pci_register_device() fails (2021-08-03 07:13:53 -0400) - Call i915_globals_exit if pci_register_device fails (Jason) - Correct SFC_DONE register offset (Matt) Jason Ekstrand (1): drm/i915: Call i915_globals_exit() if pci_register_device() fails Matt Roper (1): drm/i915: Correct SFC_DONE register offset drivers/gpu/drm/i915/i915_globals.c | 2 +- drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/i915_reg.h | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-)
Re: [PATCH v2 11/14] drm/tilcdc: Convert to Linux IRQ interfaces
Hi
Am 03.08.21 um 17:00 schrieb Sam Ravnborg:
Hi Thomas,
On Tue, Aug 03, 2021 at 11:07:01AM +0200, Thomas Zimmermann wrote:
Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.
DRM IRQ callbacks are now being called directly or inlined.
Calls to platform_get_irq() can fail with a negative errno code.
Abort initialization in this case. The DRM IRQ midlayer does not
handle this case correctly.
I cannot see why the irq_enabled flag is needed here, and the changelog
do not help me.
What do I miss?
That's a good point. Usually, only the DRM IRQ helpers use irq_enabled
in struct drm_device. It'll become legacy and we can ignore it for the
driver conversion.
The exception is tilcdc, which also uses irq_enabled to make its error
rollback work correctly. So I duplicated the state in the local private
structure.
I'll add this explanation to the commit message.
Best regards
Thomas
Sam
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 51 ++---
drivers/gpu/drm/tilcdc/tilcdc_drv.h | 3 ++
2 files changed, 43 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index f1d3a9f919fd..6b03f89a98d4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -20,7 +20,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -124,6 +123,39 @@ static int cpufreq_transition(struct notifier_block *nb,
}
#endif
+static irqreturn_t tilcdc_irq(int irq, void *arg)
+{
+ struct drm_device *dev = arg;
+ struct tilcdc_drm_private *priv = dev->dev_private;
+
+ return tilcdc_crtc_irq(priv->crtc);
+}
+
+static int tilcdc_irq_install(struct drm_device *dev, unsigned int irq)
+{
+ struct tilcdc_drm_private *priv = dev->dev_private;
+ int ret;
+
+ ret = request_irq(irq, tilcdc_irq, 0, dev->driver->name, dev);
+ if (ret)
+ return ret;
+
+ priv->irq_enabled = false;
+
+ return 0;
+}
+
+static void tilcdc_irq_uninstall(struct drm_device *dev)
+{
+ struct tilcdc_drm_private *priv = dev->dev_private;
+
+ if (!priv->irq_enabled)
+ return;
+
+ free_irq(priv->irq, dev);
+ priv->irq_enabled = false;
+}
+
/*
* DRM operations:
*/
@@ -145,7 +177,7 @@ static void tilcdc_fini(struct drm_device *dev)
drm_dev_unregister(dev);
drm_kms_helper_poll_fini(dev);
- drm_irq_uninstall(dev);
+ tilcdc_irq_uninstall(dev);
drm_mode_config_cleanup(dev);
if (priv->clk)
@@ -336,7 +368,12 @@ static int tilcdc_init(const struct drm_driver *ddrv,
struct device *dev)
goto init_failed;
}
- ret = drm_irq_install(ddev, platform_get_irq(pdev, 0));
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0)
+ goto init_failed;
+ priv->irq = ret;
+
+ ret = tilcdc_irq_install(ddev, priv->irq);
if (ret < 0) {
dev_err(dev, "failed to install IRQ handler\n");
goto init_failed;
@@ -360,13 +397,6 @@ static int tilcdc_init(const struct drm_driver *ddrv,
struct device *dev)
return ret;
}
-static irqreturn_t tilcdc_irq(int irq, void *arg)
-{
- struct drm_device *dev = arg;
- struct tilcdc_drm_private *priv = dev->dev_private;
- return tilcdc_crtc_irq(priv->crtc);
-}
-
#if defined(CONFIG_DEBUG_FS)
static const struct {
const char *name;
@@ -454,7 +484,6 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
static const struct drm_driver tilcdc_driver = {
.driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
- .irq_handler= tilcdc_irq,
DRM_GEM_CMA_DRIVER_OPS,
#ifdef CONFIG_DEBUG_FS
.debugfs_init = tilcdc_debugfs_init,
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index d29806ca8817..b818448c83f6 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -46,6 +46,8 @@ struct tilcdc_drm_private {
struct clk *clk; /* functional clock */
int rev; /* IP revision */
+ unsigned int irq;
+
/* don't attempt resolutions w/ higher W * H * Hz: */
uint32_t max_bandwidth;
/*
@@ -82,6 +84,7 @@ struct tilcdc_drm_private {
bool is_registered;
bool is_componentized;
+ bool irq_enabled;
};
/* Sub-module for display. Since we don't know at compile time what panels
--
2.32.0
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v4 3/3] drm/panel-simple: add Gopher 2b LCD panel
Hi Artjom, On Wed, Aug 04, 2021 at 03:23:53AM +0300, Artjom Vejsel wrote: > The Gopher 2b LCD panel is used in Gopher 2b handhelds. > It's simple panel with NewVision NV3047 driver, but SPI lines are not > connected. > It has no specific name, since it's unique to that handhelds. > lot name at AliExpress: 4.3 inch 40PIN TFT LCD Screen COG NV3047 Drive IC > 480(RGB)*272 No Touch 24Bit RGB Interface > > Signed-off-by: Artjom Vejsel Reviewed-by: Sam Ravnborg
Re: [PATCH v4 2/3] dt-bindings: Add DT bindings for QiShenglong Gopher 2b panel
On Wed, Aug 04, 2021 at 03:23:52AM +0300, Artjom Vejsel wrote: > Add DT bindings for QiShenglong Gopher 2b 4.3" 480(RGB)x272 TFT LCD panel. > > Signed-off-by: Artjom Vejsel Reviewed-by: Sam Ravnborg Paul, I assume you will apply when you are happy with the driver. Sam
Re: [PATCH v4 1/3] dt-bindings: Add QiShenglong vendor prefix
On Wed, Aug 04, 2021 at 03:23:51AM +0300, Artjom Vejsel wrote:
> Add vendor prefix for Shenzhen QiShenglong Industrialist Co., Ltd.
> QiShenglong is a Chinese manufacturer of handheld gaming consoles, most of
> which run (very old) versions of Linux.
> QiShenglong is known as Hamy.
>
> Signed-off-by: Artjom Vejsel
This is already applied - see
f98f273f3a98 ("dt-bindings: Add QiShenglong vendor prefix")
in drm-misc-next.
Sam
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index b868cefc7c55..1d45a2d7a7bb 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -926,6 +926,8 @@ patternProperties:
> description: Chengdu Kaixuan Information Technology Co., Ltd.
>"^qiaodian,.*":
> description: QiaoDian XianShi Corporation
> + "^qishenglong,.*":
> +description: Shenzhen QiShenglong Industrialist Co., Ltd.
>"^qnap,.*":
> description: QNAP Systems, Inc.
>"^radxa,.*":
> --
> 2.32.0
Re: [PATCH 00/11] Provide offset-adjusted framebuffer mappings
Hi Sam Am 03.08.21 um 18:21 schrieb Sam Ravnborg: Hi Thomas, On Tue, Aug 03, 2021 at 02:59:17PM +0200, Thomas Zimmermann wrote: A framebuffer's offsets field might be non-zero to make the BO data start at the specified offset within the BO's memory. Handle this case in drm_gem_fb_vmap() and update all callers. So far, many drivers ignore the offsets, which can lead to visual artifacts. Patch 1 adds an optional argument to drm_gem_fb_vmap() to return the offset-adjusted data address for use with shadow-buffered planes. Patches 3 and 11 convert gud and vkms, which are the other callers of drm_gem_fb_vmap(). For gud, it's just a cleanup. Vkms will handle the framebuffer offsets correctly for its input and output framebuffers. The other patches convert users of shadow-buffered planes to use the data address. After conversion, each driver will use the correct data for non-zero offsets. drm/ast: Use offset-adjusted shadow-plane mappings drm/gud: Get offset-adjusted mapping from drm_gem_fb_vmap() drm/hyperv: Use offset-adjusted shadow-plane mappings drm/mgag200: Use offset-adjusted shadow-plane mappings drm/cirrus: Use offset-adjusted shadow-plane mappings drm/gm12u320: Use offset-adjusted shadow-plane mappings drm/simpledrm: Use offset-adjusted shadow-plane mapping drm/udl: Use offset-adjusted shadow-plane mapping drm/vbox: Use offset-adjusted shadow-plane mappings drm/vkms: Use offset-adjusted shadow-plane mappings and output Everything looked good while reading through the patches. I cannot say if everything was properly converted but the patches looked good. So they are all: Acked-by: Sam Ravnborg Thanks! There was a few TODO comments visible aboput using the mapping api properly. I assume this is coming in a later patch set.. There are indeed quite a few such comments. When we introduced the dma_buf_map type to solve the fbdev issue on sparc64, in many places I simply put the existing vaddr pointers into the map structure, and vice versa. The code works as expected, but in the future we may want to use dma_buf_map for all framebuffer pointers. This would, for example, require format conversion helpers that operate on these structures. Adding that will require a number of changes throughout these helpers. Best regards Thomas Sam -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 02/14] drm/kmb: Define driver date and major/minor version
Hi,
just a friendly reminder that branches that end with -fixes are for
fixes that are required in upstream ASAP. I found this patch in
drm-misc-fixes. It's not important, so it should have gone into
drm-misc-next instead.
Best regards
Thomas
Am 28.07.21 um 02:31 schrieb Anitha Chrisanthus:
From: Edmund Dea
Added macros for date and version
Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Signed-off-by: Edmund Dea
---
drivers/gpu/drm/kmb/kmb_drv.c | 8
drivers/gpu/drm/kmb/kmb_drv.h | 5 +
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index c0b1c6f99249..f54392ec4fab 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -425,10 +425,10 @@ static const struct drm_driver kmb_driver = {
.fops = ,
DRM_GEM_CMA_DRIVER_OPS_VMAP,
.name = "kmb-drm",
- .desc = "KEEMBAY DISPLAY DRIVER ",
- .date = "20201008",
- .major = 1,
- .minor = 0,
+ .desc = "KEEMBAY DISPLAY DRIVER",
+ .date = DRIVER_DATE,
+ .major = DRIVER_MAJOR,
+ .minor = DRIVER_MINOR,
};
static int kmb_remove(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h
index 02e806712a64..ebbaa5f422d5 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.h
+++ b/drivers/gpu/drm/kmb/kmb_drv.h
@@ -15,6 +15,11 @@
#define KMB_MAX_HEIGHT1080 /*Max height in pixels */
#define KMB_MIN_WIDTH 1920 /*Max width in pixels */
#define KMB_MIN_HEIGHT 1080 /*Max height in pixels */
+
+#define DRIVER_DATE"20210223"
+#define DRIVER_MAJOR 1
+#define DRIVER_MINOR 1
+
#define KMB_LCD_DEFAULT_CLK 2
#define KMB_SYS_CLK_MHZ 500
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
[PULL] drm-misc-fixes
Hi Dave and Daniel, here's the weekly PR for drm-misc-fixes. I cherry-picked the vmwgfx fix from drm-misc-next-fixes where it accidentally landed first. Best regards Thomas drm-misc-fixes-2021-08-04: Short summary of fixes pull: * kmb: DMA fix; Add macros for driver date/version * vmwgfx: Fix I/O memory access on 64-bit systems The following changes since commit 8ee18e769dd621104fecad584c84ec3c4c9ef3fa: Merge drm/drm-fixes into drm-misc-fixes (2021-07-27 14:08:29 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2021-08-04 for you to fetch changes up to e89afb51f97ae03ee246c1fd0b47e3e491266aef: drm/vmwgfx: Fix a 64bit regression on svga3 (2021-08-02 21:00:37 +0200) Short summary of fixes pull: * kmb: DMA fix; Add macros for driver date/version * vmwgfx: Fix I/O memory access on 64-bit systems Edmund Dea (2): drm/kmb: Enable LCD DMA for low TVDDCV drm/kmb: Define driver date and major/minor version Zack Rusin (1): drm/vmwgfx: Fix a 64bit regression on svga3 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 +- 4 files changed, 37 insertions(+), 7 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Re: [PATCH v4] drm/msm/dp: update is_connected status base on sink count at dp_pm_resume()
Quoting Kuogee Hsieh (2021-08-04 08:51:01)
> Currently at dp_pm_resume() is_connected state is decided base on hpd
> connection
> status only. This will put is_connected in wrongly "true" state at the
> scenario
> that dongle attached to DUT but without hmdi cable connecting to it. Fix this
> problem by adding read sink count from dongle and decided is_connected state
> base
> on both sink count and hpd connection status.
>
> Changes in v2:
> -- remove dp_get_sink_count() cand call drm_dp_read_sink_count()
>
> Changes in v3:
> -- delete status local variable from dp_pm_resume()
>
> Changes in v4:
> -- delete un necessary comment at dp_pm_resume()
>
> Fixes: d9aa6571b28ba ("drm/msm/dp: check sink_count before update
> is_connected status")
> Signed-off-by: Kuogee Hsieh
> ---
Tested-by: Stephen Boyd
Reviewed-by: Stephen Boyd
Re: [PATCH v3] drm/msm/dp: update is_connected status base on sink count at dp_pm_resume()
Quoting khs...@codeaurora.org (2021-08-04 08:48:04) > On 2021-08-03 12:05, Stephen Boyd wrote: > > Quoting Kuogee Hsieh (2021-08-03 09:25:13) > >> @@ -1327,14 +1327,26 @@ static int dp_pm_resume(struct device *dev) > >> > >> dp_catalog_ctrl_hpd_config(dp->catalog); > >> > >> - status = dp_catalog_link_is_connected(dp->catalog); > >> + /* > >> +* set sink to normal operation mode -- D0 > >> +* before dpcd read > >> +*/ > >> + dp_link_psm_config(dp->link, >panel->link_info, false); > >> + > >> + /* if sink conencted, do dpcd read sink count */ > > > > s/conencted/connected/ > > > > This also just says what the code is doing. Why do we only read the > > sink > > count if the link is connected? Can we read the sink count even if the > > link isn't connected and then consider sink count as 0 if trying to > > read > > fails? > > > yes, we can do that. > But it will suffer aux time out and retry. > i think it is better to avoid this overhead by check connection first. > Hmm ok. Maybe something is wrong with the aux channel where it doesn't avoid the timeout if the connection is obviously not established yet.
Re: [PATCH 2/2] drm/panel: Add Truly NT35521 panel driver
Hi Shawn,
see a few comments in the following.
Sam
On Wed, Aug 04, 2021 at 04:13:52PM +0800, Shawn Guo wrote:
> It adds a drm driver for Truly NT35521 5.24" 1280x720 DSI panel, which
> can be found on Sony Xperia M4 Aqua phone. The panel backlight is
> managed through DSI link.
>
> Signed-off-by: Shawn Guo
> ---
> drivers/gpu/drm/panel/Kconfig | 9 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-truly-nt35521.c | 491
> 3 files changed, 501 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35521.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index ef87d92cdf49..cdc4abd5c40c 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -537,6 +537,15 @@ config DRM_PANEL_TPO_TPG110
> 400CH LTPS TFT LCD Single Chip Digital Driver for up to
> 800x400 LCD panels.
>
> +config DRM_PANEL_TRULY_NT35521
> + tristate "Truly NT35521 panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to enable support for Truly NT35521
> + 1280x720 DSI panel.
Here you could mention that the first use is the Sone eXperia M4 Aqua to
help the user.
> +
> config DRM_PANEL_TRULY_NT35597_WQXGA
> tristate "Truly WQXGA"
> depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index cae4d976c069..3d3c98cb7a7b 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_DRM_PANEL_TDO_TL070WSH30) +=
> panel-tdo-tl070wsh30.o
> obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
> obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
> obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
> +obj-$(CONFIG_DRM_PANEL_TRULY_NT35521) += panel-truly-nt35521.o
> obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
> obj-$(CONFIG_DRM_PANEL_VISIONOX_RM69299) += panel-visionox-rm69299.o
> obj-$(CONFIG_DRM_PANEL_XINPENG_XPP055C272) += panel-xinpeng-xpp055c272.o
> diff --git a/drivers/gpu/drm/panel/panel-truly-nt35521.c
> b/drivers/gpu/drm/panel/panel-truly-nt35521.c
> new file mode 100644
> index ..ea3cfb46be7e
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-truly-nt35521.c
> @@ -0,0 +1,491 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021, Linaro Limited
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
drm_print is not used by panel drivers - this also imply that you need
to replace all uses of DRM_ERROR and friends.
> +
> +struct nt35521_panel {
> + struct drm_panel panel;
> + struct device *dev;
> + struct gpio_desc *rst_gpio;
> + struct gpio_desc *pwrp5_gpio;
> + struct gpio_desc *pwrn5_gpio;
> + struct gpio_desc *en_gpio;
> + bool prepared;
> + bool enabled;
> +};
> +
> +static inline struct nt35521_panel *panel_to_nt35521(struct drm_panel *panel)
> +{
> + return container_of(panel, struct nt35521_panel, panel);
> +}
> +
> +#define nt_dcs_write(seq...) \
> +({ \
> + const u8 d[] = { seq }; \
> + if (mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)) < 0) \
> + DRM_DEV_ERROR(dev, "dcs write buffer failed\n");\
> +})
So in case of an error the code just continues with the next write, that
likely also errors out.
Please see what is for example implemented in panel-elida-kd35t133.c
That pattern looks much saner.
> +
> +#define nt_gen_write(seq...) \
> +({ \
> + const u8 d[] = { seq }; \
> + if (mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)) < 0) \
> + DRM_DEV_ERROR(dev, "generic write buffer failed\n");\
> +})
For the two uses, please consider open coding this.
> +
> +static void nt35521_panel_on(struct nt35521_panel *nt)
> +{
> + struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
> + struct device *dev = nt->dev;
> +
> + /* Transmit data in low power mode */
> + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> + nt_dcs_write(0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
> + nt_dcs_write(0xff, 0xaa, 0x55, 0xa5, 0x80);
> + nt_dcs_write(0x6f, 0x11, 0x00);
> + nt_dcs_write(0xf7, 0x20, 0x00);
> + nt_dcs_write(0x6f, 0x01);
> + nt_dcs_write(0xb1, 0x21);
> + nt_dcs_write(0xbd, 0x01, 0xa0, 0x10, 0x08, 0x01);
> + nt_dcs_write(0xb8, 0x01, 0x02, 0x0c, 0x02);
> + nt_dcs_write(0xbb, 0x11, 0x11);
> + nt_dcs_write(0xbc, 0x00, 0x00);
> +
Re: [PATCH 1/2] dt-bindings: display: panel: Add Truly NT35521 panel support
Hi Shawn, On Wed, Aug 04, 2021 at 04:13:51PM +0800, Shawn Guo wrote: > The Truly NT35521 is a 5.24" 1280x720 DSI panel, and the backlight is > managed through DSI link. > > Signed-off-by: Shawn Guo Please consider adding an optional port node, so we can use this panels in a setup using a graph. A simple port: true would do the trick. I am aware that it may not be used today, this is a preparation for potential future use. With this fixed, Reviewed-by: Sam Ravnborg Sam
[PATCH v4] drm/msm/dp: update is_connected status base on sink count at dp_pm_resume()
Currently at dp_pm_resume() is_connected state is decided base on hpd connection
status only. This will put is_connected in wrongly "true" state at the scenario
that dongle attached to DUT but without hmdi cable connecting to it. Fix this
problem by adding read sink count from dongle and decided is_connected state
base
on both sink count and hpd connection status.
Changes in v2:
-- remove dp_get_sink_count() cand call drm_dp_read_sink_count()
Changes in v3:
-- delete status local variable from dp_pm_resume()
Changes in v4:
-- delete un necessary comment at dp_pm_resume()
Fixes: d9aa6571b28ba ("drm/msm/dp: check sink_count before update is_connected
status")
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 78c5301..25fb4f8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1313,7 +1313,7 @@ static int dp_pm_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct msm_dp *dp_display = platform_get_drvdata(pdev);
struct dp_display_private *dp;
- u32 status;
+ int sink_count = 0;
dp = container_of(dp_display, struct dp_display_private, dp_display);
@@ -1327,14 +1327,25 @@ static int dp_pm_resume(struct device *dev)
dp_catalog_ctrl_hpd_config(dp->catalog);
- status = dp_catalog_link_is_connected(dp->catalog);
+ /*
+* set sink to normal operation mode -- D0
+* before dpcd read
+*/
+ dp_link_psm_config(dp->link, >panel->link_info, false);
+
+ if (dp_catalog_link_is_connected(dp->catalog)) {
+ sink_count = drm_dp_read_sink_count(dp->aux);
+ if (sink_count < 0)
+ sink_count = 0;
+ }
+ dp->link->sink_count = sink_count;
/*
* can not declared display is connected unless
* HDMI cable is plugged in and sink_count of
* dongle become 1
*/
- if (status && dp->link->sink_count)
+ if (dp->link->sink_count)
dp->dp_display.is_connected = true;
else
dp->dp_display.is_connected = false;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v3] drm/msm/dp: update is_connected status base on sink count at dp_pm_resume()
On 2021-08-03 12:05, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2021-08-03 09:25:13)
Currently at dp_pm_resume() is_connected state is decided base on hpd
connection
status only. This will put is_connected in wrongly "true" state at the
scenario
that dongle attached to DUT but without hmdi cable connecting to it.
Fix this
problem by adding read sink count from dongle and decided is_connected
state base
on both sink count and hpd connection status.
Changes in v2:
-- remove dp_get_sink_count() cand call drm_dp_read_sink_count()
Changes in v3:
-- delete status local variable from dp_pm_resume()
Fixes: d9aa6571b28ba ("drm/msm/dp: check sink_count before update
is_connected status")
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 78c5301..0f39256 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1313,7 +1313,7 @@ static int dp_pm_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct msm_dp *dp_display = platform_get_drvdata(pdev);
struct dp_display_private *dp;
- u32 status;
+ int sink_count = 0;
dp = container_of(dp_display, struct dp_display_private,
dp_display);
@@ -1327,14 +1327,26 @@ static int dp_pm_resume(struct device *dev)
dp_catalog_ctrl_hpd_config(dp->catalog);
- status = dp_catalog_link_is_connected(dp->catalog);
+ /*
+* set sink to normal operation mode -- D0
+* before dpcd read
+*/
+ dp_link_psm_config(dp->link, >panel->link_info, false);
+
+ /* if sink conencted, do dpcd read sink count */
s/conencted/connected/
This also just says what the code is doing. Why do we only read the
sink
count if the link is connected? Can we read the sink count even if the
link isn't connected and then consider sink count as 0 if trying to
read
fails?
yes, we can do that.
But it will suffer aux time out and retry.
i think it is better to avoid this overhead by check connection first.
+ if (dp_catalog_link_is_connected(dp->catalog)) {
+ sink_count = drm_dp_read_sink_count(dp->aux);
+ if (sink_count < 0)
+ sink_count = 0;
+ }
+ dp->link->sink_count = sink_count;
/*
* can not declared display is connected unless
* HDMI cable is plugged in and sink_count of
* dongle become 1
*/
- if (status && dp->link->sink_count)
+ if (dp->link->sink_count)
dp->dp_display.is_connected = true;
else
dp->dp_display.is_connected = false;
Re: [PATCH] drm/radeon: Update pitch for page flip
On Tue, Aug 03, 2021 at 10:49:39AM -0400, Alex Deucher wrote: > On Tue, Aug 3, 2021 at 4:34 AM Michel Dänzer wrote: > > > > On 2021-08-02 4:51 p.m., Alex Deucher wrote: > > > On Mon, Aug 2, 2021 at 4:31 AM Daniel Vetter wrote: > > >> > > >> On Mon, Aug 02, 2021 at 10:12:47AM +0200, Christian König wrote: > > >>> Am 02.08.21 um 09:43 schrieb Zhenneng Li: > > When primary bo is updated, crtc's pitch may > > have not been updated, this will lead to show > > disorder content when user changes display mode, > > we update crtc's pitch in page flip to avoid > > this bug. > > This refers to amdgpu's pageflip. > > >>> > > >>> Alex is the expert to ask about that code, but I'm not sure if that is > > >>> really correct for the old hardware. > > >>> > > >>> As far as I know the crtc's pitch should not change during a page flip, > > >>> but > > >>> only during a full mode set. > > >>> > > >>> So could you elaborate a bit more how you trigger this? > > >> > > >> legacy page_flip ioctl only verifies that the fb->format stays the same. > > >> It doesn't check anything else (afair never has), this is all up to > > >> drivers to verify. > > >> > > >> Personally I'd say add a check to reject this, since testing this and > > >> making sure it really works everywhere is probably a bit much on this old > > >> hw. > > > > > > If just the pitch changed, that probably wouldn't be much of a > > > problem, but if the pitch is changing, that probably implies other > > > stuff has changed as well and we'll just be chasing changes. I agree > > > it would be best to just reject anything other than updating the > > > scanout address. > > > > FWIW, that means page flipping cannot be used in some cases which work fine > > by changing the pitch, which can result in tearing: > > https://gitlab.freedesktop.org/xorg/xserver/-/issues/839 (which says the > > i915 driver handles this as well). > > > > Ok. In that case, @Zhenneng can you update all of the pitch in all of > the page_flip functions in radeon rather than just the evergreen one? atomic drivers handle this fairly ok-ish, since we have a proper atomic_check there. For legacy kms I just wouldn't bother, too many corners to validate. But also if you're bored, just do it :-) -Daniel > > Thanks, > > Alex > > > > > > -- > > Earthling Michel Dänzer | https://redhat.com > > Libre software enthusiast | Mesa and X developer -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/i915/dp: Use max params for older panels
Users reported that after commit 2bbd6dba84d4 ("drm/i915: Try to use
fast+narrow link on eDP again and fall back to the old max strategy on
failure"), the screen starts to have wobbly effect.
Commit a5c936add6a2 ("drm/i915/dp: Use slow and wide link training for
everything") doesn't help either, that means the affected panels only
work with max params.
The panels are all DP 1.1 ones, so apply max params to them to resolve
the issue.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3714
Fixes: 2bbd6dba84d4 ("drm/i915: Try to use fast+narrow link on eDP again and
fall back to the old max strategy on failure")
Fixes: a5c936add6a2 ("drm/i915/dp: Use slow and wide link training for
everything")
Signed-off-by: Kai-Heng Feng
---
drivers/gpu/drm/i915/display/intel_dp.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 75d4ebc669411..e64bab4b016e1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1330,14 +1330,16 @@ intel_dp_compute_link_config(struct intel_encoder
*encoder,
limits.min_bpp = intel_dp_min_bpp(pipe_config->output_format);
limits.max_bpp = intel_dp_max_bpp(intel_dp, pipe_config);
- if (intel_dp->use_max_params) {
+ if (intel_dp->use_max_params ||
+ intel_dp->dpcd[DP_DPCD_REV] <= DP_DPCD_REV_11) {
/*
* Use the maximum clock and number of lanes the eDP panel
* advertizes being capable of in case the initial fast
-* optimal params failed us. The panels are generally
-* designed to support only a single clock and lane
-* configuration, and typically on older panels these
-* values correspond to the native resolution of the panel.
+* optimal params failed us or the panel is DP 1.1 or earlier.
+* The panels are generally designed to support only a single
+* clock and lane configuration, and typically on older panels
+* these values correspond to the native resolution of the
+* panel.
*/
limits.min_lane_count = limits.max_lane_count;
limits.min_clock = limits.max_clock;
--
2.31.1
[RFC PATCH 3/3] drm/v3d: add multiple syncobjs support
Using the generic extension support set in the previous patch, this
patch enables more than one in/out binary syncobj per job submission.
Arrays of syncobjs are set in a specific extension type (multisync)
that also cares of determining the stage for sync (bin/render)
through a flag - when this is the case.
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/v3d/v3d_drv.c | 3 +
drivers/gpu/drm/v3d/v3d_drv.h | 14 +++
drivers/gpu/drm/v3d/v3d_gem.c | 210 --
include/uapi/drm/v3d_drm.h| 38 ++
4 files changed, 231 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 6a0516160bb2..939ca8c833f5 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -96,6 +96,9 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void
*data,
case DRM_V3D_PARAM_SUPPORTS_PERFMON:
args->value = (v3d->ver >= 40);
return 0;
+ case DRM_V3D_PARAM_SUPPORTS_MULTISYNC_EXT:
+ args->value = 1;
+ return 0;
default:
DRM_DEBUG("Unknown parameter %d\n", args->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 270134779073..a4fdd7539691 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -299,6 +299,20 @@ struct v3d_csd_job {
struct drm_v3d_submit_csd args;
};
+struct v3d_submit_outsync {
+ struct drm_syncobj *syncobj;
+};
+
+struct v3d_submit_ext {
+ u32 flags;
+
+ u32 in_sync_count;
+ u64 in_syncs;
+
+ u32 out_sync_count;
+ struct v3d_submit_outsync *out_syncs;
+};
+
/**
* __wait_for - magic wait macro
*
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 22f3ef9f52d2..52ec52f6340e 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -452,9 +452,12 @@ v3d_job_add_deps(struct drm_file *file_priv, struct
v3d_job *job,
static int
v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
struct v3d_job *job, void (*free)(struct kref *ref),
-u32 in_sync)
+u32 in_sync, struct v3d_submit_ext *se, u32 rcl_sync_flag)
{
- int ret;
+ int ret, i;
+ struct drm_v3d_sem __user *handle = NULL;
+ struct dma_fence *in_fence = NULL;
+ unsigned long index;
job->v3d = v3d;
job->free = free;
@@ -465,14 +468,34 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file
*file_priv,
xa_init_flags(>deps, XA_FLAGS_ALLOC);
- ret = v3d_job_add_deps(file_priv, job, in_sync, 0);
- if (ret)
- goto fail;
+ if (!(se && se->in_sync_count)) {
+ ret = v3d_job_add_deps(file_priv, job, in_sync, 0);
+ if (ret)
+ goto fail;
+ } else if (se && se->in_sync_count && (se->flags & DRM_V3D_IN_SYNC_RCL)
== rcl_sync_flag) {
+ handle = u64_to_user_ptr(se->in_syncs);
+
+ for (i = 0; i < se->in_sync_count; i++) {
+ struct drm_v3d_sem in;
+
+ ret = copy_from_user(, handle++, sizeof(in));
+ if (ret) {
+ DRM_DEBUG("Failed to copy wait dep handle.\n");
+ goto fail;
+ }
+
+ ret = v3d_job_add_deps(file_priv, job, in.handle, 0);
+ if (ret)
+ goto fail;
+ }
+ }
kref_init(>refcount);
return 0;
fail:
+ xa_for_each(>deps, index, in_fence)
+ dma_fence_put(in_fence);
xa_destroy(>deps);
pm_runtime_put_autosuspend(v3d->drm.dev);
return ret;
@@ -504,6 +527,7 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file
*file_priv,
struct v3d_job *job,
struct ww_acquire_ctx *acquire_ctx,
u32 out_sync,
+struct v3d_submit_ext *se,
struct dma_fence *done_fence)
{
struct drm_syncobj *sync_out;
@@ -518,6 +542,18 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file
*file_priv,
drm_gem_unlock_reservations(job->bo, job->bo_count, acquire_ctx);
/* Update the return sync object for the job */
+ /* If multiples semaphores is supported */
+ if (se && se->out_sync_count) {
+ for (i = 0; i < se->out_sync_count; i++) {
+ drm_syncobj_replace_fence(se->out_syncs[i].syncobj,
+ done_fence);
+ drm_syncobj_put(se->out_syncs[i].syncobj);
+ }
+ kvfree(se->out_syncs);
+ return;
+ }
+
+
[RFC PATCH 2/3] drm/v3d: add generic ioctl extension
Add support to attach generic extensions on job submission.
This patch is a second prep work to enable multiple syncobjs on job
submission. With this work, when the job submission interface needs
to be extended to accomodate a new feature, we will use a generic
extension struct where an id determines the data type to be pointed.
The first application is to enable multiples in/out syncobj (next
patch), but the base is already done for future features.
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/v3d/v3d_drv.c | 4 +-
drivers/gpu/drm/v3d/v3d_gem.c | 84 ---
include/uapi/drm/v3d_drm.h| 38 +++-
3 files changed, 115 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 9403c3b36aca..6a0516160bb2 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -83,7 +83,6 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void
*data,
return 0;
}
-
switch (args->param) {
case DRM_V3D_PARAM_SUPPORTS_TFU:
args->value = 1;
@@ -147,7 +146,7 @@ v3d_postclose(struct drm_device *dev, struct drm_file *file)
DEFINE_DRM_GEM_FOPS(v3d_drm_fops);
/* DRM_AUTH is required on SUBMIT_CL for now, while we don't have GMP
- * protection between clients. Note that render nodes would be be
+ * protection between clients. Note that render nodes would be
* able to submit CLs that could access BOs from clients authenticated
* with the master node. The TFU doesn't use the GMP, so it would
* need to stay DRM_AUTH until we do buffer size/offset validation.
@@ -222,7 +221,6 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
u32 mmu_debug;
u32 ident1;
-
v3d = devm_drm_dev_alloc(dev, _drm_driver, struct v3d_dev, drm);
if (IS_ERR(v3d))
return PTR_ERR(v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 944bc4728055..22f3ef9f52d2 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -525,6 +525,38 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file
*file_priv,
}
}
+static int
+v3d_get_extensions(struct drm_file *file_priv,
+ u32 ext_count, u64 ext_handles)
+{
+ int i;
+ struct drm_v3d_extension __user *handles;
+
+ if (!ext_count)
+ return 0;
+
+ handles = u64_to_user_ptr(ext_handles);
+ for (i = 0; i < ext_count; i++) {
+ struct drm_v3d_extension ext;
+
+ if (copy_from_user(, handles, sizeof(ext))) {
+ DRM_DEBUG("Failed to copy submit extension\n");
+ return -EFAULT;
+ }
+
+ switch (ext.id) {
+ case 0:
+ default:
+ DRM_DEBUG_DRIVER("Unknown extension id: %d\n", ext.id);
+ return -EINVAL;
+ }
+
+ handles = u64_to_user_ptr(ext.next);
+ }
+
+ return 0;
+}
+
/**
* v3d_submit_cl_ioctl() - Submits a job (frame) to the V3D.
* @dev: DRM device
@@ -553,15 +585,23 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
trace_v3d_submit_cl_ioctl(>drm, args->rcl_start, args->rcl_end);
- if (args->pad != 0)
- return -EINVAL;
-
- if (args->flags != 0 &&
- args->flags != DRM_V3D_SUBMIT_CL_FLUSH_CACHE) {
+ if (args->flags &&
+ args->flags & ~(DRM_V3D_SUBMIT_CL_FLUSH_CACHE |
+ DRM_V3D_SUBMIT_EXTENSION)) {
DRM_INFO("invalid flags: %d\n", args->flags);
return -EINVAL;
}
+ if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {
+ ret = v3d_get_extensions(file_priv,
+args->extension_count,
+args->extensions);
+ if (ret) {
+ DRM_DEBUG("Failed to get extensions.\n");
+ return ret;
+ }
+ }
+
render = kcalloc(1, sizeof(*render), GFP_KERNEL);
if (!render)
return -ENOMEM;
@@ -721,6 +761,21 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
trace_v3d_submit_tfu_ioctl(>drm, args->iia);
+ if (args->flags && !(args->flags & DRM_V3D_SUBMIT_EXTENSION)) {
+ DRM_DEBUG("invalid flags: %d\n", args->flags);
+ return -EINVAL;
+ }
+
+ if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {
+ ret = v3d_get_extensions(file_priv,
+args->extension_count,
+args->extensions);
+ if (ret) {
+ DRM_DEBUG("Failed to get extensions.\n");
+ return ret;
+ }
+ }
+
job = kcalloc(1, sizeof(*job), GFP_KERNEL);
[RFC PATCH 1/3] drm/v3d: decouple adding job dependencies from job init
Prep work to enable multiple syncobj as job dependency.
Also get rid of old checkpatch warnings in the v3d_gem file.
No functional changes.
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/v3d/v3d_gem.c | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 5689da118197..944bc4728055 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -417,7 +417,7 @@ v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
return -EINVAL;
ret = drm_gem_dma_resv_wait(file_priv, args->handle,
- true, timeout_jiffies);
+ true, timeout_jiffies);
/* Decrement the user's timeout, in case we got interrupted
* such that the ioctl will be restarted.
@@ -435,12 +435,25 @@ v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
return ret;
}
+static int
+v3d_job_add_deps(struct drm_file *file_priv, struct v3d_job *job,
+u32 in_sync, u32 point)
+{
+ struct dma_fence *in_fence = NULL;
+ int ret;
+
+ ret = drm_syncobj_find_fence(file_priv, in_sync, point, 0, _fence);
+ if (ret == -EINVAL)
+ return ret;
+
+ return drm_gem_fence_array_add(>deps, in_fence);
+}
+
static int
v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
struct v3d_job *job, void (*free)(struct kref *ref),
u32 in_sync)
{
- struct dma_fence *in_fence = NULL;
int ret;
job->v3d = v3d;
@@ -452,11 +465,7 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file
*file_priv,
xa_init_flags(>deps, XA_FLAGS_ALLOC);
- ret = drm_syncobj_find_fence(file_priv, in_sync, 0, 0, _fence);
- if (ret == -EINVAL)
- goto fail;
-
- ret = drm_gem_fence_array_add(>deps, in_fence);
+ ret = v3d_job_add_deps(file_priv, job, in_sync, 0);
if (ret)
goto fail;
@@ -503,7 +512,7 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file
*file_priv,
for (i = 0; i < job->bo_count; i++) {
/* XXX: Use shared fences for read-only objects. */
dma_resv_add_excl_fence(job->bo[i]->resv,
- job->done_fence);
+ job->done_fence);
}
drm_gem_unlock_reservations(job->bo, job->bo_count, acquire_ctx);
@@ -924,8 +933,7 @@ v3d_gem_init(struct drm_device *dev)
if (!v3d->pt) {
drm_mm_takedown(>mm);
dev_err(v3d->drm.dev,
- "Failed to allocate page tables. "
- "Please ensure you have CMA enabled.\n");
+ "Failed to allocate page tables. Please ensure you have
CMA enabled.\n");
return -ENOMEM;
}
--
2.30.2
signature.asc
Description: PGP signature
[RFC PATCH 0/3] drm/v3d: add multiple in/out syncobjs support
Hi, Currently, v3d only supports single in/out syncobj per submission (in v3d_submit_cl we have two in_sync, one for bin and another for render job); however, Vulkan queue submit operations expect multiples wait and signal semaphores. This series extending v3d interface and job dependency operations to handle more than one in/out syncobj. The first patch just decouples the steps to add job dependency from the job init code since the operation repeats for every syncobj that a job should wait before starting. So, we can just reuse it when handling multiples wait for semaphores in the third patch of this series. The second patch extends our interface by using a generic extension. We chose this approach inspired by i915_user_extension[1] and amd_cs_chunks[2] to give a little more flexibility in adding other submission features in the future. Therefore, the list of extensions will work as a hub of features that use an id to determine the corresponding feature data type. With this base, the third patch adds multiple wait/signal semaphores support. For this, we add to the list of the generic extensions a new data type (drm_v3d_multi_sync) that points to two arrays of syncobjs (in/out) and also determines (flags) if the dependencies must be added to the bin job or render job (in the case of v3d_submit_cl). An auxiliary struct (v3d_submit_ext) is used when parsing submission extensions. Finally, we reserve some space in the semaphore struct (drm_v3d_sem) to accommodate timeline semaphores that we aim to add support soon (same reason for already defining v3d_submit_outsync). [1] https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/i915/i915_user_extensions.c?id=9d1305ef80b95dde0337106ed8b826604e2155ad [2] https://cgit.freedesktop.org/drm/drm-misc/tree/include/uapi/drm/amdgpu_drm.h#n556 PS: I'm cc'ing more people to get any comments for the generic extension approach and the multiple semaphores support too. Melissa Wen (3): drm/v3d: decouple adding job dependencies from job init drm/v3d: add generic ioctl extension drm/v3d: add multiple syncobjs support drivers/gpu/drm/v3d/v3d_drv.c | 7 +- drivers/gpu/drm/v3d/v3d_drv.h | 14 ++ drivers/gpu/drm/v3d/v3d_gem.c | 304 +- include/uapi/drm/v3d_drm.h| 76 - 4 files changed, 355 insertions(+), 46 deletions(-) -- 2.30.2 signature.asc Description: PGP signature
Re: [PATCH] depend on BACKLIGHT_CLASS_DEVICE for more devices
On Wed, Aug 4, 2021 at 4:19 PM Arnd Bergmann wrote: > > On Wed, Aug 4, 2021 at 4:10 PM Karol Herbst wrote: > > > > playing around a little bit with this, I think the original "select > > BACKLIGHT_CLASS_DEVICE" is fine. Atm we kind of have this weird mix of > > drivers selecting and others depending on it. We could of course convert > > everything over to depend, and break those cycling dependency issues with > > this. > > > > Anyway this change on top of my initial patch is enough to make Kconfig > > happy and has the advantage of not having to mess with the deps of nouveau > > too much. > > Looks good to me. We'd probably want to make the BACKLIGHT_CLASS_DEVICE > option itself 'default FB || DRM' though, to ensure that defconfigs > keep working. > okay cool. Will send out a proper updated patch series soonish. > Arnd >
Re: [PATCH 5/7] drm/i915/gem/ttm: Respect the objection region in placement_from_obj
On Wed, Aug 4, 2021 at 10:00 AM Thomas Hellström wrote: > > Hi, > > On 7/22/21 11:59 AM, Matthew Auld wrote: > > On Thu, 22 Jul 2021 at 10:49, Matthew Auld > > wrote: > >> On Wed, 21 Jul 2021 at 21:11, Jason Ekstrand wrote: > >>> On Mon, Jul 19, 2021 at 8:35 AM Matthew Auld > >>> wrote: > On Fri, 16 Jul 2021 at 20:49, Jason Ekstrand > wrote: > > On Fri, Jul 16, 2021 at 1:45 PM Matthew Auld > > wrote: > >> On Fri, 16 Jul 2021 at 18:39, Jason Ekstrand > >> wrote: > >>> On Fri, Jul 16, 2021 at 11:00 AM Matthew Auld > >>> wrote: > On Fri, 16 Jul 2021 at 16:52, Matthew Auld > wrote: > > On Fri, 16 Jul 2021 at 15:10, Jason Ekstrand > > wrote: > >> On Fri, Jul 16, 2021 at 8:54 AM Matthew Auld > >> wrote: > >>> On Thu, 15 Jul 2021 at 23:39, Jason Ekstrand > >>> wrote: > Whenever we had a user object (n_placements > 0), we were > ignoring > obj->mm.region and always putting obj->placements[0] as the > requested > region. For LMEM+SMEM objects, this was causing them to get > shoved into > LMEM on every i915_ttm_get_pages() even when SMEM was requested > by, say, > i915_gem_object_migrate(). > >>> i915_ttm_migrate calls i915_ttm_place_from_region() directly with > >>> the > >>> requested region, so there shouldn't be an issue with migration > >>> right? > >>> Do you have some more details? > >> With i915_ttm_migrate directly, no. But, in the last patch in the > >> series, we're trying to migrate LMEM+SMEM buffers into SMEM on > >> attach() and pin it there. This blows up in a very unexpected > >> (IMO) > >> way. The flow goes something like this: > >> > >> - Client attempts a dma-buf import from another device > >> - In attach() we call i915_gem_object_migrate() which calls > >> i915_ttm_migrate() which migrates as requested. > >> - Once the migration is complete, we call > >> i915_gem_object_pin_pages() > >> which calls i915_ttm_get_pages() which depends on > >> i915_ttm_placement_from_obj() and so migrates it right back to > >> LMEM. > > The mm.pages must be NULL here, otherwise it would just increment > > the > > pages_pin_count? > >>> Given that the test is using the four_underscores version, it > >>> doesn't have that check. However, this executes after we've done the > >>> dma-buf import which pinned pages. So we should definitely have > >>> pages. > >> We shouldn't call four_underscores() if we might already have > >> pages though. Under non-TTM that would leak the pages, and in TTM we > >> might hit the WARN_ON(mm->pages) in __i915_ttm_get_pages(), if for > >> example nothing was moved. I take it we can't just call pin_pages()? > >> Four scary underscores usually means "don't call this in normal code". > > I've switched the four_underscores call to a __two_underscores in > > the selftests and it had no effect, good or bad. But, still, probably > > better to call that one. > > > >> Maybe the problem here is actually that our TTM code isn't > >> respecting > >> obj->mm.pages_pin_count? > > I think if the resource is moved, we always nuke the mm.pages after > > being notified of the move. Also TTM is also not allowed to move > > pinned buffers. > > > > I guess if we are evicted/swapped, so assuming we are not holding > > the > > object lock, and it's not pinned, the future call to get_pages() > > will > > see mm.pages = NULL, even though the ttm_resource is still there, > > and > > because we prioritise the placements[0], instead of mm.region we end > > up moving it for no good reason. But in your case you are holding > > the > > lock, or it's pinned? Also is this just with the selftest, or > > something real? > Or at least in the selftest I see i915_gem_object_get_pages() > which doesn't even consider the mm.pages AFAIK. > >>> The bogus migration is happening as part of the > >>> __i915_gem_object_get_pages() (2 __underscores) call in > >>> i915_gem_dmabuf_attach (see last patch). That code is attempting to > >>> migrate the BO to SMEM and then pin it there using the obvious calls > >>> to do so. However, in the pin_pages call, it gets implicitly migrated > >>> back to LMEM thanks to i915_ttm_get_pages(). Why is _get_pages() > >>> migrating things at all? > >> Not sure yet, but __two_underscores() checks if > >> i915_gem_object_has_pages() before actually calling into > >> i915_ttm_get_pages(), so the mm.pages
[PATCH v2 9/9] drm/i915: Split out intel_context_create_user
There's quite a fundamental difference between userspace contexts, and
kernel contexts. Latter all share intel_gt->vm, former get their vm
from gem_ctx->vm (on full ppgtt at least).
By splitting context creation for userspace from kernel-internal ones
we can make this all a bit more strict and WARN_ON if there's a vm
already set in intel_context_set_gem().
All this is only possible because gem_ctx cannot chance their VM
anymore since
commit ccbc1b97948ab671335e950271e39766729736c3
Author: Jason Ekstrand
Date: Thu Jul 8 10:48:30 2021 -0500
drm/i915/gem: Don't allow changing the VM on running contexts (v4)
v2: I didn't take care of virtual engine creation and on top of that
collided pretty good with Matt's abstraction from
commit 556120256ecd25aacea2c7e3ad11ec6584de7252
Author: Matthew Brost
Date: Mon Jul 26 17:23:16 2021 -0700
drm/i915/guc: GuC virtual engines
address this all by also splitting the new intel_engine_create_virtual
into a _user variant that doesn't set ce->vm.
Cc: Matthew Brost
Cc: Daniele Ceraolo Spurio
Cc: John Harrison
Signed-off-by: Daniel Vetter
Cc: Jon Bloomfield
Cc: Chris Wilson
Cc: Maarten Lankhorst
Cc: Joonas Lahtinen
Cc: Daniel Vetter
Cc: "Thomas Hellström"
Cc: Matthew Auld
Cc: Lionel Landwerlin
Cc: Dave Airlie
Cc: Jason Ekstrand
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 10 -
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 4 +++-
.../gpu/drm/i915/gem/selftests/mock_context.c | 2 +-
drivers/gpu/drm/i915/gt/intel_context.c | 22 +--
drivers/gpu/drm/i915/gt/intel_context.h | 2 ++
drivers/gpu/drm/i915/gt/intel_engine.h| 4
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 21 --
7 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 2f3cc73d4710..754b9b8d4981 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -746,7 +746,7 @@ static int intel_context_set_gem(struct intel_context *ce,
ce->ring_size = SZ_16K;
- i915_vm_put(ce->vm);
+ WARN_ON(ce->vm);
ce->vm = i915_gem_context_get_eb_vm(ctx);
if (ctx->sched.priority >= I915_PRIORITY_NORMAL &&
@@ -856,7 +856,7 @@ static struct i915_gem_engines *default_engines(struct
i915_gem_context *ctx,
GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
GEM_BUG_ON(e->engines[engine->legacy_idx]);
- ce = intel_context_create(engine);
+ ce = intel_context_create_user(engine);
if (IS_ERR(ce)) {
err = ERR_CAST(ce);
goto free_engines;
@@ -897,12 +897,12 @@ static struct i915_gem_engines *user_engines(struct
i915_gem_context *ctx,
switch (pe[n].type) {
case I915_GEM_ENGINE_TYPE_PHYSICAL:
- ce = intel_context_create(pe[n].engine);
+ ce = intel_context_create_user(pe[n].engine);
break;
case I915_GEM_ENGINE_TYPE_BALANCED:
- ce = intel_engine_create_virtual(pe[n].siblings,
-pe[n].num_siblings);
+ ce = intel_engine_create_virtual_user(pe[n].siblings,
+
pe[n].num_siblings);
break;
case I915_GEM_ENGINE_TYPE_INVALID:
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index bdf2b5785a81..54de94433365 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -30,15 +30,17 @@
struct eb_vma {
struct i915_vma *vma;
+ struct drm_i915_gem_object *obj;
unsigned int flags;
+ u32 handle;
+
/** This vma's place in the execbuf reservation list */
struct drm_i915_gem_exec_object2 *exec;
struct list_head bind_link;
struct list_head reloc_link;
struct hlist_node node;
- u32 handle;
};
enum {
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index fee070df1c97..e5efda1058a3 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -124,7 +124,7 @@ live_context_for_engine(struct intel_engine_cs *engine,
struct file *file)
return ctx;
}
- ce = intel_context_create(engine);
+ ce = intel_context_create_user(engine);
if (IS_ERR(ce)) {
__free_engines(engines, 0);
return ERR_CAST(ce);
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
[PATCH v2 5/9] drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem
Since
commit ccbc1b97948ab671335e950271e39766729736c3
Author: Jason Ekstrand
Date: Thu Jul 8 10:48:30 2021 -0500
drm/i915/gem: Don't allow changing the VM on running contexts (v4)
the gem_ctx->vm can't change anymore. Plus we always set the
intel_context->vm, so might as well use the helper we have for that.
This makes it very clear that we always overwrite intel_context->vm
for userspace contexts, since the default is gt->vm, which is
explicitly reserved for kernel context use. It would be good to split
things up a bit further and avoid any possibility for an accident
where we run kernel stuff in userspace vm or the other way round.
Signed-off-by: Daniel Vetter
Cc: Jon Bloomfield
Cc: Chris Wilson
Cc: Maarten Lankhorst
Cc: Joonas Lahtinen
Cc: Daniel Vetter
Cc: "Thomas Hellström"
Cc: Matthew Auld
Cc: Lionel Landwerlin
Cc: Dave Airlie
Cc: Jason Ekstrand
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index a80b06c98dba..fd24a1236682 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -784,16 +784,8 @@ static int intel_context_set_gem(struct intel_context *ce,
ce->ring_size = SZ_16K;
- if (rcu_access_pointer(ctx->vm)) {
- struct i915_address_space *vm;
-
- rcu_read_lock();
- vm = context_get_vm_rcu(ctx); /* hmm */
- rcu_read_unlock();
-
- i915_vm_put(ce->vm);
- ce->vm = vm;
- }
+ i915_vm_put(ce->vm);
+ ce->vm = i915_gem_context_get_eb_vm(ctx);
if (ctx->sched.priority >= I915_PRIORITY_NORMAL &&
intel_engine_has_timeslices(ce->engine) &&
--
2.32.0
[PATCH v2 7/9] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
We don't need the absolute speed of rcu for this. And
i915_address_space in general dont need rcu protection anywhere else,
after we've made gem contexts and engines a lot more immutable.
Note that this semantically reverts
commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499
Author: Chris Wilson
Date: Fri Aug 30 19:03:25 2019 +0100
drm/i915: Use RCU for unlocked vm_idr lookup
except we have the conversion from idr to xarray in between.
Signed-off-by: Daniel Vetter
Cc: Jon Bloomfield
Cc: Chris Wilson
Cc: Maarten Lankhorst
Cc: Joonas Lahtinen
Cc: Daniel Vetter
Cc: "Thomas Hellström"
Cc: Matthew Auld
Cc: Lionel Landwerlin
Cc: Dave Airlie
Cc: Jason Ekstrand
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1488d166d91c..df2d723c894a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1880,11 +1880,11 @@ i915_gem_vm_lookup(struct drm_i915_file_private
*file_priv, u32 id)
{
struct i915_address_space *vm;
- rcu_read_lock();
+ xa_lock(_priv->vm_xa);
vm = xa_load(_priv->vm_xa, id);
if (vm && !kref_get_unless_zero(>ref))
vm = NULL;
- rcu_read_unlock();
+ xa_unlock(_priv->vm_xa);
return vm;
}
--
2.32.0
[PATCH v2 3/9] drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam
Consolidates the "which is the vm my execbuf runs in" code a bit. We
do some get/put which isn't really required, but all the other users
want the refcounting, and I figured doing a function just for this
getparam to avoid 2 atomis is a bit much.
Signed-off-by: Daniel Vetter
Cc: Jon Bloomfield
Cc: Chris Wilson
Cc: Maarten Lankhorst
Cc: Joonas Lahtinen
Cc: Daniel Vetter
Cc: "Thomas Hellström"
Cc: Matthew Auld
Cc: Lionel Landwerlin
Cc: Dave Airlie
Cc: Jason Ekstrand
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 11 +--
1 file changed, 5 insertions(+), 6 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..6263563e15d6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -2124,6 +2124,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device
*dev, void *data,
struct drm_i915_file_private *file_priv = file->driver_priv;
struct drm_i915_gem_context_param *args = data;
struct i915_gem_context *ctx;
+ struct i915_address_space *vm;
int ret = 0;
ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
@@ -2133,12 +2134,10 @@ int i915_gem_context_getparam_ioctl(struct drm_device
*dev, void *data,
switch (args->param) {
case I915_CONTEXT_PARAM_GTT_SIZE:
args->size = 0;
- rcu_read_lock();
- if (rcu_access_pointer(ctx->vm))
- args->value = rcu_dereference(ctx->vm)->total;
- else
- args->value = to_i915(dev)->ggtt.vm.total;
- rcu_read_unlock();
+ vm = i915_gem_context_get_eb_vm(ctx);
+ args->value = vm->total;
+ i915_vm_put(vm);
+
break;
case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
--
2.32.0
[PATCH v2 8/9] drm/i915: Stop rcu support for i915_address_space
The full audit is quite a bit of work: - i915_dpt has very simple lifetime (somehow we create a display pagetable vm per object, so its _very_ simple, there's only ever a single vma in there), and uses i915_vm_close(), which internally does a i915_vm_put(). No rcu. Aside: wtf is i915_dpt doing in the intel_display.c garbage collector as a new feature, instead of added as a separate file with some clean-ish interface. Also, i915_dpt unfortunately re-introduces some coding patterns from pre-dma_resv_lock conversion times. - i915_gem_proto_ctx is fully refcounted and no rcu, all protected by fpriv->proto_context_lock. - i915_gem_context is itself rcu protected, and that might leak to anything it points at. Before commit cf977e18610e66e48c31619e7e0cfa871be9eada Author: Chris Wilson Date: Wed Dec 2 11:21:40 2020 + drm/i915/gem: Spring clean debugfs and commit db80a1294c231b6ac725085f046bb2931e00c9db Author: Chris Wilson Date: Mon Jan 18 11:08:54 2021 + drm/i915/gem: Remove per-client stats from debugfs/i915_gem_objects we had a bunch of debugfs files that relied on rcu protecting everything, but those are gone now. The main one was removed even earlier with There doesn't seem to be anything left that's actually protecting stuff now that the ctx->vm itself is invariant. See commit ccbc1b97948ab671335e950271e39766729736c3 Author: Jason Ekstrand Date: Thu Jul 8 10:48:30 2021 -0500 drm/i915/gem: Don't allow changing the VM on running contexts (v4) Note that we drop the vm refcount before the final release of the gem context refcount, so this is all very dangerous even without rcu. Note that aside from later on creating new engines (a defunct feature) and debug output we're never looked at gem_ctx->vm for anything functional, hence why this is ok. Fingers crossed. Preceeding patches removed all vestiges of rcu use from gem_ctx->vm derferencing to make it clear it's really not used. The gem_ctx->rcu protection was introduced in commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1 Author: Chris Wilson Date: Fri Oct 4 14:40:09 2019 +0100 drm/i915: Move context management under GEM The commit message is somewhat entertaining because it fails to mention this fact completely, and compensates that by an in-commit changelog entry that claims that ctx->vm is protected by ctx->mutex. Which was the case _before_ this commit, but no longer after it. - intel_context holds a full reference. Unfortunately intel_context is also rcu protected and the reference to the ->vm is dropped before the rcu barrier - only the kfree is delayed. So again we need to check whether that leaks anywhere on the intel_context->vm. RCU is only used to protect intel_context sitting on the breadcrumb lists, which don't look at the vm anywhere, so we are fine. Nothing else relies on rcu protection of intel_context and hence is fully protected by the kref refcount alone, which protects intel_context->vm in turn. The breadcrumbs rcu usage was added in commit c744d50363b714783bbc88d986cc16def13710f7 Author: Chris Wilson Date: Thu Nov 26 14:04:06 2020 + drm/i915/gt: Split the breadcrumb spinlock between global and contexts its parent commit added the intel_context rcu protection: commit 14d1eaf08845c534963c83f754afe0cb14cb2512 Author: Chris Wilson Date: Thu Nov 26 14:04:05 2020 + drm/i915/gt: Protect context lifetime with RCU given some credence to my claim that I've actually caught them all. - drm_i915_gem_object's shares_resv_from pointer has a full refcount to the dma_resv, which is a sub-refcount that's released after the final i915_vm_put() has been called. Safe. Aside: Maybe we should have a struct dma_resv_shared which is just dma_resv + kref as a stand-alone thing. It's a pretty useful pattern which other drivers might want to copy. For a bit more context see commit 4d8151ae5329cf50781a02fd2298a909589a5bab Author: Thomas Hellström Date: Tue Jun 1 09:46:41 2021 +0200 drm/i915: Don't free shared locks while shared - the fpriv->vm_xa was relying on rcu_read_lock for lookup, but that was updated in a prep patch too to just be a spinlock-protected lookup. - intel_gt->vm is set at driver load in intel_gt_init() and released in intel_gt_driver_release(). There seems to be some issue that in some error paths this is called twice, but otherwise no rcu to be found anywhere. This was added in the below commit, which unfortunately doesn't explain why this complication exists. commit e6ba76480299a0d77c51d846f7467b1673aad25b Author: Chris Wilson Date: Sat Dec 21 16:03:24 2019 + drm/i915: Remove
[PATCH v2 6/9] drm/i915: Drop __rcu from gem_context->vm
It's been invariant since
commit ccbc1b97948ab671335e950271e39766729736c3
Author: Jason Ekstrand
Date: Thu Jul 8 10:48:30 2021 -0500
drm/i915/gem: Don't allow changing the VM on running contexts (v4)
this just completes the deed. I've tried to split out prep work for
more careful review as much as possible, this is what's left:
- get_ppgtt gets simplified since we don't need to grab a temporary
reference - we can rely on the temporary reference for the gem_ctx
while we inspect the vm. The new vm_id still needs a full
i915_vm_open ofc. This also removes the final caller of context_get_vm_rcu
- A pile of selftests can now just look at ctx->vm instead of
rcu_dereference_protected( , true) or similar things.
- All callers of i915_gem_context_vm also disappear.
- I've changed the hugepage selftest to set scrub_64K without any
locking, because when we inspect that setting we're also not taking
any locks either. It works because it's a selftests that's careful
(single threaded gives you nice ordering) and not a live driver
where races can happen from anywhere.
These can only be split up further if we have some intermediate state
with a bunch more rcu_dereference_protected(ctx->vm, true), just to
shut up lockdep and sparse.
The conversion to __rcu happened in
commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1
Author: Chris Wilson
Date: Fri Oct 4 14:40:09 2019 +0100
drm/i915: Move context management under GEM
Note that we're not breaking the actual bugfix in there: The real
bugfix is pushing the i915_vm_relase onto a separate worker, to avoid
locking inversion issues. The rcu conversion was just thrown in for
entertainment value on top (no vm lookup isn't even close to anything
that's a hotpath where removing the single spinlock can be measured).
Signed-off-by: Daniel Vetter
Cc: Jon Bloomfield
Cc: Chris Wilson
Cc: Maarten Lankhorst
Cc: Joonas Lahtinen
Cc: Daniel Vetter
Cc: "Thomas Hellström"
Cc: Matthew Auld
Cc: Lionel Landwerlin
Cc: Dave Airlie
Cc: Jason Ekstrand
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 53 ++-
drivers/gpu/drm/i915/gem/i915_gem_context.h | 14 ++---
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +-
.../gpu/drm/i915/gem/selftests/huge_pages.c | 4 +-
.../drm/i915/gem/selftests/i915_gem_context.c | 24 -
drivers/gpu/drm/i915/i915_trace.h | 2 +-
drivers/gpu/drm/i915/selftests/i915_vma.c | 2 +-
7 files changed, 21 insertions(+), 80 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index fd24a1236682..2f3cc73d4710 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -735,44 +735,6 @@ static int set_proto_ctx_param(struct
drm_i915_file_private *fpriv,
return ret;
}
-static struct i915_address_space *
-context_get_vm_rcu(struct i915_gem_context *ctx)
-{
- GEM_BUG_ON(!rcu_access_pointer(ctx->vm));
-
- do {
- struct i915_address_space *vm;
-
- /*
-* We do not allow downgrading from full-ppgtt [to a shared
-* global gtt], so ctx->vm cannot become NULL.
-*/
- vm = rcu_dereference(ctx->vm);
- if (!kref_get_unless_zero(>ref))
- continue;
-
- /*
-* This ppgtt may have be reallocated between
-* the read and the kref, and reassigned to a third
-* context. In order to avoid inadvertent sharing
-* of this ppgtt with that third context (and not
-* src), we have to confirm that we have the same
-* ppgtt after passing through the strong memory
-* barrier implied by a successful
-* kref_get_unless_zero().
-*
-* Once we have acquired the current ppgtt of ctx,
-* we no longer care if it is released from ctx, as
-* it cannot be reallocated elsewhere.
-*/
-
- if (vm == rcu_access_pointer(ctx->vm))
- return rcu_pointer_handoff(vm);
-
- i915_vm_put(vm);
- } while (1);
-}
-
static int intel_context_set_gem(struct intel_context *ce,
struct i915_gem_context *ctx,
struct intel_sseu sseu)
@@ -1193,7 +1155,7 @@ static void context_close(struct i915_gem_context *ctx)
set_closed_name(ctx);
- vm = i915_gem_context_vm(ctx);
+ vm = ctx->vm;
if (vm)
i915_vm_close(vm);
@@ -1350,7 +1312,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
vm = >vm;
}
if (vm) {
- RCU_INIT_POINTER(ctx->vm, i915_vm_open(vm));
+ ctx->vm = i915_vm_open(vm);
/* i915_vm_open() takes a
[PATCH v2 4/9] drm/i915: Add i915_gem_context_is_full_ppgtt
And use it anywhere we have open-coded checks for ctx->vm that really
only check for full ppgtt.
Plus for paranoia add a GEM_BUG_ON that checks it's really only set
when we have full ppgtt, just in case. gem_context->vm is different
since it's NULL in ggtt mode, unlike intel_context->vm or gt->vm,
which is always set.
v2: 0day found a testcase that I missed.
Signed-off-by: Daniel Vetter
Cc: Jon Bloomfield
Cc: Chris Wilson
Cc: Maarten Lankhorst
Cc: Joonas Lahtinen
Cc: Daniel Vetter
Cc: "Thomas Hellström"
Cc: Matthew Auld
Cc: Lionel Landwerlin
Cc: Dave Airlie
Cc: Jason Ekstrand
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_context.h | 7 +++
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c| 2 +-
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 6 +++---
4 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 6263563e15d6..a80b06c98dba 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1581,7 +1581,7 @@ static int get_ppgtt(struct drm_i915_file_private
*file_priv,
int err;
u32 id;
- if (!rcu_access_pointer(ctx->vm))
+ if (!i915_gem_context_is_full_ppgtt(ctx))
return -ENODEV;
rcu_read_lock();
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h
b/drivers/gpu/drm/i915/gem/i915_gem_context.h
index da6e8b506d96..37536a260e6e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
@@ -154,6 +154,13 @@ i915_gem_context_vm(struct i915_gem_context *ctx)
return rcu_dereference_protected(ctx->vm, lockdep_is_held(>mutex));
}
+static inline bool i915_gem_context_is_full_ppgtt(struct i915_gem_context *ctx)
+{
+ GEM_BUG_ON(!!rcu_access_pointer(ctx->vm) != HAS_FULL_PPGTT(ctx->i915));
+
+ return !!rcu_access_pointer(ctx->vm);
+}
+
static inline struct i915_address_space *
i915_gem_context_get_eb_vm(struct i915_gem_context *ctx)
{
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 69e47b97d786..bdf2b5785a81 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -749,7 +749,7 @@ static int eb_select_context(struct i915_execbuffer *eb)
return PTR_ERR(ctx);
eb->gem_context = ctx;
- if (rcu_access_pointer(ctx->vm))
+ if (i915_gem_context_is_full_ppgtt(ctx))
eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
return 0;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index d436ce7fa25c..0708b9cdeb9f 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -704,7 +704,7 @@ static int igt_ctx_exec(void *arg)
pr_err("Failed to fill dword %lu [%lu/%lu] with
gpu (%s) [full-ppgtt? %s], err=%d\n",
ndwords, dw, max_dwords(obj),
engine->name,
- yesno(!!rcu_access_pointer(ctx->vm)),
+
yesno(i915_gem_context_is_full_ppgtt(ctx)),
err);
intel_context_put(ce);
kernel_context_close(ctx);
@@ -838,7 +838,7 @@ static int igt_shared_ctx_exec(void *arg)
pr_err("Failed to fill dword %lu [%lu/%lu] with
gpu (%s) [full-ppgtt? %s], err=%d\n",
ndwords, dw, max_dwords(obj),
engine->name,
- yesno(!!rcu_access_pointer(ctx->vm)),
+
yesno(i915_gem_context_is_full_ppgtt(ctx)),
err);
intel_context_put(ce);
kernel_context_close(ctx);
@@ -1417,7 +1417,7 @@ static int igt_ctx_readonly(void *arg)
pr_err("Failed to fill dword %lu [%lu/%lu] with
gpu (%s) [full-ppgtt? %s], err=%d\n",
ndwords, dw, max_dwords(obj),
ce->engine->name,
- yesno(!!ctx_vm(ctx)),
+
yesno(i915_gem_context_is_full_ppgtt(ctx)),
err);
i915_gem_context_unlock_engines(ctx);
goto out_file;
--
2.32.0
[PATCH 0/9] remove rcu support from i915_address_space
Hi all, Next round with some fixes: - missed a conversion, 0day spotted it running sparse - missed virtual engines in the last patch, intel-gfx-ci spotted that too (except it was mostly filtered out by a bogus cibuglog entry, so took a while to realize what's going on). Old version: https://lore.kernel.org/dri-devel/20210802154806.3710472-1-daniel.vet...@ffwll.ch/ Cheers, Daniel Daniel Vetter (9): drm/i915: Drop code to handle set-vm races from execbuf drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam drm/i915: Add i915_gem_context_is_full_ppgtt drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem drm/i915: Drop __rcu from gem_context->vm drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups drm/i915: Stop rcu support for i915_address_space drm/i915: Split out intel_context_create_user drivers/gpu/drm/i915/gem/i915_gem_context.c | 86 --- drivers/gpu/drm/i915/gem/i915_gem_context.h | 13 ++- .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 12 ++- .../gpu/drm/i915/gem/selftests/huge_pages.c | 8 +- .../drm/i915/gem/selftests/i915_gem_context.c | 34 +++- .../gpu/drm/i915/gem/selftests/mock_context.c | 2 +- drivers/gpu/drm/i915/gt/intel_context.c | 22 - drivers/gpu/drm/i915/gt/intel_context.h | 2 + drivers/gpu/drm/i915/gt/intel_engine.h| 4 + drivers/gpu/drm/i915/gt/intel_engine_cs.c | 21 - drivers/gpu/drm/i915/gt/intel_ggtt.c | 1 - drivers/gpu/drm/i915/gt/intel_gtt.c | 6 +- drivers/gpu/drm/i915/gt/intel_gtt.h | 2 +- drivers/gpu/drm/i915/gt/selftest_execlists.c | 2 +- drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 4 +- drivers/gpu/drm/i915/i915_trace.h | 2 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 +- drivers/gpu/drm/i915/selftests/i915_vma.c | 4 +- 20 files changed, 105 insertions(+), 128 deletions(-) -- 2.32.0
[PATCH v2 2/9] drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm
The important part isn't so much that this does an rcu lookup - that's
more an implementation detail, which will also be removed.
The thing that makes this different from other functions is that it's
gettting you the vm that batchbuffers will run in for that gem
context, which is either a full ppgtt stored in gem->ctx, or the ggtt.
We'll make more use of this function later on.
Signed-off-by: Daniel Vetter
Cc: Jon Bloomfield
Cc: Chris Wilson
Cc: Maarten Lankhorst
Cc: Joonas Lahtinen
Cc: Daniel Vetter
Cc: "Thomas Hellström"
Cc: Matthew Auld
Cc: Lionel Landwerlin
Cc: Dave Airlie
Cc: Jason Ekstrand
---
drivers/gpu/drm/i915/gem/i915_gem_context.h | 2 +-
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 4 ++--
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 4 ++--
drivers/gpu/drm/i915/gt/selftest_execlists.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 ++--
drivers/gpu/drm/i915/selftests/i915_vma.c | 2 +-
7 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h
b/drivers/gpu/drm/i915/gem/i915_gem_context.h
index 18060536b0c2..da6e8b506d96 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
@@ -155,7 +155,7 @@ i915_gem_context_vm(struct i915_gem_context *ctx)
}
static inline struct i915_address_space *
-i915_gem_context_get_vm_rcu(struct i915_gem_context *ctx)
+i915_gem_context_get_eb_vm(struct i915_gem_context *ctx)
{
struct i915_address_space *vm;
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index a094f3ce1a90..6c68fe26bb32 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1456,7 +1456,7 @@ static int igt_tmpfs_fallback(void *arg)
struct i915_gem_context *ctx = arg;
struct drm_i915_private *i915 = ctx->i915;
struct vfsmount *gemfs = i915->mm.gemfs;
- struct i915_address_space *vm = i915_gem_context_get_vm_rcu(ctx);
+ struct i915_address_space *vm = i915_gem_context_get_eb_vm(ctx);
struct drm_i915_gem_object *obj;
struct i915_vma *vma;
u32 *vaddr;
@@ -1512,7 +1512,7 @@ static int igt_shrink_thp(void *arg)
{
struct i915_gem_context *ctx = arg;
struct drm_i915_private *i915 = ctx->i915;
- struct i915_address_space *vm = i915_gem_context_get_vm_rcu(ctx);
+ struct i915_address_space *vm = i915_gem_context_get_eb_vm(ctx);
struct drm_i915_gem_object *obj;
struct i915_gem_engines_iter it;
struct intel_context *ce;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 8eb5050f8cb3..d436ce7fa25c 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -1528,7 +1528,7 @@ static int write_to_scratch(struct i915_gem_context *ctx,
intel_gt_chipset_flush(engine->gt);
- vm = i915_gem_context_get_vm_rcu(ctx);
+ vm = i915_gem_context_get_eb_vm(ctx);
vma = i915_vma_instance(obj, vm, NULL);
if (IS_ERR(vma)) {
err = PTR_ERR(vma);
@@ -1607,7 +1607,7 @@ static int read_from_scratch(struct i915_gem_context *ctx,
if (GRAPHICS_VER(i915) >= 8) {
const u32 GPR0 = engine->mmio_base + 0x600;
- vm = i915_gem_context_get_vm_rcu(ctx);
+ vm = i915_gem_context_get_eb_vm(ctx);
vma = i915_vma_instance(obj, vm, NULL);
if (IS_ERR(vma)) {
err = PTR_ERR(vma);
diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c
b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index f12ffe797639..b3863abc51f5 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -3493,7 +3493,7 @@ static int smoke_submit(struct preempt_smoke *smoke,
if (batch) {
struct i915_address_space *vm;
- vm = i915_gem_context_get_vm_rcu(ctx);
+ vm = i915_gem_context_get_eb_vm(ctx);
vma = i915_vma_instance(batch, vm, NULL);
i915_vm_put(vm);
if (IS_ERR(vma))
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 08f011f893b2..6023c418ee8a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -117,7 +117,7 @@ static struct i915_request *
hang_create_request(struct hang *h, struct intel_engine_cs *engine)
{
struct intel_gt *gt = h->gt;
- struct i915_address_space *vm = i915_gem_context_get_vm_rcu(h->ctx);
+ struct i915_address_space *vm = i915_gem_context_get_eb_vm(h->ctx);
[PATCH v2 1/9] drm/i915: Drop code to handle set-vm races from execbuf
Changing the vm from a finalized gem ctx is no longer possible, which
means we don't have to check for that anymore.
I was pondering whether to keep the check as a WARN_ON, but things go
boom real bad real fast if the vm of a vma is wrong. Plus we'd need to
also get the ggtt vm for !full-ppgtt platforms. Ditching it all seemed
like a better idea.
References: ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running
contexts (v4)")
Signed-off-by: Daniel Vetter
Cc: Jon Bloomfield
Cc: Chris Wilson
Cc: Maarten Lankhorst
Cc: Joonas Lahtinen
Cc: Daniel Vetter
Cc: "Thomas Hellström"
Cc: Matthew Auld
Cc: Lionel Landwerlin
Cc: Dave Airlie
Cc: Jason Ekstrand
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 538d9d2e52b7..69e47b97d786 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -775,11 +775,7 @@ static int __eb_add_lut(struct i915_execbuffer *eb,
/* Check that the context hasn't been closed in the meantime */
err = -EINTR;
if (!mutex_lock_interruptible(>lut_mutex)) {
- struct i915_address_space *vm = rcu_access_pointer(ctx->vm);
-
- if (unlikely(vm && vma->vm != vm))
- err = -EAGAIN; /* user racing with ctx set-vm */
- else if (likely(!i915_gem_context_is_closed(ctx)))
+ if (likely(!i915_gem_context_is_closed(ctx)))
err = radix_tree_insert(>handles_vma, handle, vma);
else
err = -ENOENT;
--
2.32.0
Re: [PATCH] depend on BACKLIGHT_CLASS_DEVICE for more devices
On Wed, Aug 4, 2021 at 4:10 PM Karol Herbst wrote: > > playing around a little bit with this, I think the original "select > BACKLIGHT_CLASS_DEVICE" is fine. Atm we kind of have this weird mix of > drivers selecting and others depending on it. We could of course convert > everything over to depend, and break those cycling dependency issues with > this. > > Anyway this change on top of my initial patch is enough to make Kconfig > happy and has the advantage of not having to mess with the deps of nouveau > too much. Looks good to me. We'd probably want to make the BACKLIGHT_CLASS_DEVICE option itself 'default FB || DRM' though, to ensure that defconfigs keep working. Arnd
Re: [PATCH v2 0/8] drm/bridge: Make panel and bridge probe order consistent
Hi Maxime,
I have been busy with other tasks, and I did not follow the list last
time, so sorry for my late response.
On 28.07.2021 15:32, Maxime Ripard wrote:
> Hi,
>
> We've encountered an issue with the RaspberryPi DSI panel that prevented the
> whole display driver from probing.
>
> The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> Only register our component once a DSI device is attached"), but the basic
> idea
> is that since the panel is probed through i2c, there's no synchronization
> between its probe and the registration of the MIPI-DSI host it's attached to.
>
> We initially moved the component framework registration to the MIPI-DSI Host
> attach hook to make sure we register our component only when we have a DSI
> device attached to our MIPI-DSI host, and then use lookup our DSI device in
> our
> bind hook.
>
> However, all the DSI bridges controlled through i2c are only registering their
> associated DSI device in their bridge attach hook, meaning with our change
I guess this is incorrect. I have promoted several times the pattern
that device driver shouldn't expose its interfaces (for example
component_add, drm_panel_add, drm_bridge_add) until it gathers all
required dependencies. In this particular case bridges should defer
probe until DSI bus becomes available. I guess this way the patch you
reverts would work.
I advised few times this pattern in case of DSI hosts, apparently I
didn't notice the similar issue can appear in case of bridges. Or there
is something I have missed???
Anyway there are already eleven(?) bridge drivers using this pattern. I
wonder if fixing it would be difficult, or if it expose other issues???
The patches should be quite straightforward - move
of_find_mipi_dsi_host_by_node and mipi_dsi_device_register_full to probe
time.
Finally I think that if we will not fix these bridge drivers we will
encounter another set of issues with new platforms connecting "DSI host
drivers assuming this pattern" and "i2c/dsi device drivers assuming
pattern already present in the bridges".
Regards
Andrzej
[PATCH] depend on BACKLIGHT_CLASS_DEVICE for more devices
playing around a little bit with this, I think the original "select BACKLIGHT_CLASS_DEVICE" is fine. Atm we kind of have this weird mix of drivers selecting and others depending on it. We could of course convert everything over to depend, and break those cycling dependency issues with this. Anyway this change on top of my initial patch is enough to make Kconfig happy and has the advantage of not having to mess with the deps of nouveau too much. Cc: Arnd Bergmann Cc: Lyude Paul Cc: Ben Skeggs Cc: Randy Dunlap Cc: Daniel Vetter Cc: nouv...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/bridge/Kconfig| 2 +- drivers/gpu/drm/fsl-dcu/Kconfig | 2 +- drivers/gpu/drm/gud/Kconfig | 2 +- drivers/gpu/drm/nouveau/Kconfig | 2 +- drivers/platform/x86/Kconfig | 4 ++-- drivers/staging/olpc_dcon/Kconfig | 2 +- drivers/usb/misc/Kconfig | 2 +- drivers/video/fbdev/Kconfig | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 431b6e12a81f..dc68532ede38 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -173,9 +173,9 @@ config DRM_NXP_PTN3460 config DRM_PARADE_PS8622 tristate "Parade eDP/LVDS bridge" depends on OF + depends on BACKLIGHT_CLASS_DEVICE select DRM_PANEL select DRM_KMS_HELPER - select BACKLIGHT_CLASS_DEVICE help Parade eDP-LVDS bridge chip driver. diff --git a/drivers/gpu/drm/fsl-dcu/Kconfig b/drivers/gpu/drm/fsl-dcu/Kconfig index d7dd8ba90e3a..79bfd7e6f6dc 100644 --- a/drivers/gpu/drm/fsl-dcu/Kconfig +++ b/drivers/gpu/drm/fsl-dcu/Kconfig @@ -2,7 +2,7 @@ config DRM_FSL_DCU tristate "DRM Support for Freescale DCU" depends on DRM && OF && ARM && COMMON_CLK - select BACKLIGHT_CLASS_DEVICE + depends on BACKLIGHT_CLASS_DEVICE select DRM_KMS_HELPER select DRM_KMS_CMA_HELPER select DRM_PANEL diff --git a/drivers/gpu/drm/gud/Kconfig b/drivers/gpu/drm/gud/Kconfig index 1c8601bf4d91..91a118928af7 100644 --- a/drivers/gpu/drm/gud/Kconfig +++ b/drivers/gpu/drm/gud/Kconfig @@ -3,10 +3,10 @@ config DRM_GUD tristate "GUD USB Display" depends on DRM && USB + depends on BACKLIGHT_CLASS_DEVICE select LZ4_COMPRESS select DRM_KMS_HELPER select DRM_GEM_SHMEM_HELPER - select BACKLIGHT_CLASS_DEVICE help This is a DRM display driver for GUD USB Displays or display adapters. diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index 2e159b0ea7fb..afb3eede8e2b 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -2,12 +2,12 @@ config DRM_NOUVEAU tristate "Nouveau (NVIDIA) cards" depends on DRM && PCI && MMU + depends on BACKLIGHT_CLASS_DEVICE select IOMMU_API select FW_LOADER select DRM_KMS_HELPER select DRM_TTM select DRM_TTM_HELPER - select BACKLIGHT_CLASS_DEVICE select ACPI_VIDEO if ACPI && X86 && INPUT select X86_PLATFORM_DEVICES if ACPI && X86 select ACPI_WMI if ACPI && X86 diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 7d385c3b2239..278368985fb2 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -838,7 +838,7 @@ config SAMSUNG_LAPTOP config SAMSUNG_Q10 tristate "Samsung Q10 Extras" depends on ACPI - select BACKLIGHT_CLASS_DEVICE + depends on BACKLIGHT_CLASS_DEVICE help This driver provides support for backlight control on Samsung Q10 and related laptops, including Dell Latitude X200. @@ -935,7 +935,7 @@ config ACPI_CMPC tristate "CMPC Laptop Extras" depends on ACPI && INPUT depends on RFKILL || RFKILL=n - select BACKLIGHT_CLASS_DEVICE + depends on BACKLIGHT_CLASS_DEVICE help Support for Intel Classmate PC ACPI devices, including some keys as input device, backlight device, tablet and accelerometer diff --git a/drivers/staging/olpc_dcon/Kconfig b/drivers/staging/olpc_dcon/Kconfig index d1a0dea09ef0..a9f36538d7ab 100644 --- a/drivers/staging/olpc_dcon/Kconfig +++ b/drivers/staging/olpc_dcon/Kconfig @@ -4,7 +4,7 @@ config FB_OLPC_DCON depends on OLPC && FB depends on I2C depends on GPIO_CS5535 && ACPI - select BACKLIGHT_CLASS_DEVICE + depends on BACKLIGHT_CLASS_DEVICE help In order to support very low power operation, the XO laptop uses a secondary Display CONtroller, or DCON. This secondary controller diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig index 8f1144359012..6f769a1616f0 100644 --- a/drivers/usb/misc/Kconfig +++ b/drivers/usb/misc/Kconfig @@ -132,7 +132,7 @@ config USB_FTDI_ELAN config USB_APPLEDISPLAY tristate
Re: [PATCH v34 0/3] Mainline imx6 based SKOV boards
On Wed, Aug 4, 2021 at 6:36 AM Oleksij Rempel wrote: > > changes v4: > - add vref-supply to adc@0 > - split gpio assignment for the mdio node Hi Oleksij, I've dropped the series from the soc patchwork, since this looks like something that should go through the i.MX tree. Please make it clear from the cover letter and from the 'to' list what you want to happen with the patches, and who should take care of them. Any patches that are meant for platform maintainers should not get sent to s...@kernel.org. If you think you need me to review the patches, you can send them cc my personal address. Arnd
[Bug 211277] sometimes crash at s2ram-wake (Ryzen 3500U): amdgpu, drm, commit_tail, amdgpu_dm_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=211277 --- Comment #36 from Jerome C (m...@jeromec.com) --- I've been watching linux-next and noticed that this commit https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/amd?id=65660ad349fd947feb16b45ff9231f2ceaf44318 was posted on linux-next back between 5.10-5.11, I don't remember but it keeps getting pushed back and not mainlined... I think this is why the issues are still here and none of AMD are responding to this now since comment 29 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 208981] trace with B550I AORUS PRO AX and AMD Ryzen 5 PRO 4650G
https://bugzilla.kernel.org/show_bug.cgi?id=208981 --- Comment #10 from Florian La Roche (florian.laro...@gmail.com) --- This seems to be fixed after updating to BIOS F12 from 2021-01-18, BIOS Revision: 5.17. There are even newer BIOS revisions available, but they only work with RAM at 2133 MT/s instead of the usual 3200 MT/s and seem to be unstable. best regards, Florian La Roche -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 211277] sometimes crash at s2ram-wake (Ryzen 3500U): amdgpu, drm, commit_tail, amdgpu_dm_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=211277 --- Comment #35 from kolAflash (kolafl...@kolahilft.de) --- Created attachment 298193 --> https://bugzilla.kernel.org/attachment.cgi?id=298193=edit /var/log/kern.log running amd-drm-next-5.14-2021-05-12 (ae30d41eb) with Xorg Sorry for the long delay. I've tested: 1. Current Debian-11 testing Linux-5.10.0-8 with amdgpu.ip_block_mask=0x0ff while running Xorg. Result: everything ok 2. amd-drm-next-5.14-2021-05-12* (ae30d41eb) without any special kernel options while running Xorg. Result: - crashes - also the screen starts flickering about every 10 seconds after second resume - flickering also happens with using a8f768874^ (before the first fix-commit by Alex D.) - log attached: 5.12.0-rc7-original-ae30d41eb_crash.txt 3. Upstream Linux-5.14.0-rc4. Result: Still broken. * amd-drm-next-5.14-2021-05-12 https://gitlab.freedesktop.org/agd5f/linux/-/tree/amd-drm-next-5.14-2021-05-12 ae30d41eb -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 2/2] drm/panel: Add Truly NT35521 panel driver
Hi Shawn,
Thanks for the patch!
On Wed, Aug 04, 2021 at 04:13:52PM +0800, Shawn Guo wrote:
> It adds a drm driver for Truly NT35521 5.24" 1280x720 DSI panel, which
> can be found on Sony Xperia M4 Aqua phone. The panel backlight is
> managed through DSI link.
>
> Signed-off-by: Shawn Guo
> ---
> drivers/gpu/drm/panel/Kconfig | 9 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-truly-nt35521.c | 491
> 3 files changed, 501 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35521.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index ef87d92cdf49..cdc4abd5c40c 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -537,6 +537,15 @@ config DRM_PANEL_TPO_TPG110
> 400CH LTPS TFT LCD Single Chip Digital Driver for up to
> 800x400 LCD panels.
>
> +config DRM_PANEL_TRULY_NT35521
> + tristate "Truly NT35521 panel"
I think the name "Truly NT35521" is a bit too generic. AFAIK "Truly" is
a panel vendor and the NovaTek NT35521 is the panel controller. But
there are almost certainly other Truly panels that were also combined
with a NT35521 but need a slightly different configuration.
If you don't know more than "Truly NT35521" based on the Sony sources,
maybe do it similar to "asus,z00t-tm5p5-n35596" and use a compatible
like "sony,-truly-nt35521". Would be good to clarify the Kconfig
option here too.
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to enable support for Truly NT35521
> + 1280x720 DSI panel.
> +
> config DRM_PANEL_TRULY_NT35597_WQXGA
> tristate "Truly WQXGA"
> depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index cae4d976c069..3d3c98cb7a7b 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_DRM_PANEL_TDO_TL070WSH30) +=
> panel-tdo-tl070wsh30.o
> obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
> obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
> obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
> +obj-$(CONFIG_DRM_PANEL_TRULY_NT35521) += panel-truly-nt35521.o
> obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
> obj-$(CONFIG_DRM_PANEL_VISIONOX_RM69299) += panel-visionox-rm69299.o
> obj-$(CONFIG_DRM_PANEL_XINPENG_XPP055C272) += panel-xinpeng-xpp055c272.o
> diff --git a/drivers/gpu/drm/panel/panel-truly-nt35521.c
> b/drivers/gpu/drm/panel/panel-truly-nt35521.c
> new file mode 100644
> index ..ea3cfb46be7e
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-truly-nt35521.c
> @@ -0,0 +1,491 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021, Linaro Limited
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +
> +struct nt35521_panel {
> + struct drm_panel panel;
> + struct device *dev;
> + struct gpio_desc *rst_gpio;
> + struct gpio_desc *pwrp5_gpio;
> + struct gpio_desc *pwrn5_gpio;
> + struct gpio_desc *en_gpio;
> + bool prepared;
> + bool enabled;
> +};
> +
> +static inline struct nt35521_panel *panel_to_nt35521(struct drm_panel *panel)
> +{
> + return container_of(panel, struct nt35521_panel, panel);
> +}
> +
> +#define nt_dcs_write(seq...) \
> +({ \
> + const u8 d[] = { seq }; \
> + if (mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)) < 0) \
> + DRM_DEV_ERROR(dev, "dcs write buffer failed\n");\
> +})
> +
> +#define nt_gen_write(seq...) \
> +({ \
> + const u8 d[] = { seq }; \
> + if (mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)) < 0) \
> + DRM_DEV_ERROR(dev, "generic write buffer failed\n");\
> +})
> +
> +static void nt35521_panel_on(struct nt35521_panel *nt)
> +{
> + struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
> + struct device *dev = nt->dev;
> +
> + /* Transmit data in low power mode */
> + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> + nt_dcs_write(0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
> + nt_dcs_write(0xff, 0xaa, 0x55, 0xa5, 0x80);
> + nt_dcs_write(0x6f, 0x11, 0x00);
> + nt_dcs_write(0xf7, 0x20, 0x00);
> + nt_dcs_write(0x6f, 0x01);
> + nt_dcs_write(0xb1, 0x21);
> + nt_dcs_write(0xbd, 0x01, 0xa0, 0x10, 0x08, 0x01);
> + nt_dcs_write(0xb8, 0x01, 0x02, 0x0c, 0x02);
> + nt_dcs_write(0xbb, 0x11, 0x11);
> + nt_dcs_write(0xbc, 0x00, 0x00);
> +
Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
On Tue, Aug 3, 2021 at 9:34 AM Michel Dänzer wrote: > On 2021-08-03 8:11 a.m., Kasireddy, Vivek wrote: > >>> The goal: > >>> - Maintain full framerate even when the Guest scanout FB is flipped onto > >>> a hardware > >> plane > >>> on the Host -- regardless of either compositor's scheduling policy -- > >>> without making any > >>> copies and ensuring that both Host and Guest are not accessing the buffer > >>> at the same > >> time. > >>> > >>> The problem: > >>> - If the Host compositor flips the client's buffer (in this case Guest > >>> compositor's buffer) > >>> onto a hardware plane, then it can send a wl_buffer.release event for the > >>> previous buffer > >>> only after it gets a pageflip completion. And, if the Guest compositor > >>> takes 10-12 ms to > >>> submit a new buffer and given the fact that the Host compositor waits > >>> only for 9 ms, the > >>> Guest compositor will miss the Host's repaint cycle resulting in halved > >>> frame-rate. > >>> > >>> 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). Hence my gut feeling reaction that first we need to get these two compositors aligned in their timings, which propobably needs consistent vblank periods/timestamps across them (plus/minux guest/host clocksource fun ofc). Without this any of the next steps will simply not work because there's too much jitter by the time the guest compositor gets the flip completion events. Once we have solid events I think we should look into statically tuning guest/host compositor deadlines (like you've suggested in a bunch of places) to consisently make that deadline and hit 60 fps. With that we can then look into tuning this automatically and what to do when e.g. switching between copying and zero-copy on the host side (which might be needed in some cases) and how to handle all that. Only when that all shows that we just can't hit 60fps consistently and really need 3 buffers in flight should we look at deeper kms queues. And then we really need to implement them properly and not
[Resend v3] drm/msm/disp/dpu1: add safe lut config in dpu driver
Add safe lut configuration for all the targets in dpu
driver as per QOS recommendation.
Issue reported on SC7280:
With wait-for-safe feature in smmu enabled, RT client
buffer levels are checked to be safe before smmu invalidation.
Since display was always set to unsafe it was delaying the
invalidaiton process thus impacting the performance on NRT clients
such as eMMC and NVMe.
Validated this change on SC7280, With this change eMMC performance
has improved significantly.
Changes in v2:
- Add fixes tag (Sai)
- CC stable kernel (Dimtry)
Changes in v3:
- Correct fixes tag with appropriate hash (stephen)
- Resend patch adding reviewed by tag
- Resend patch adding correct format for pushing into stable tree (Greg)
Fixes: 591e34a091d1 ("drm/msm/disp/dpu1: add support for display for SC7280
target")
Cc: sta...@vger.kernel.org
Signed-off-by: Kalyan Thota
Reviewed-by: Dmitry Baryshkov
Tested-by: Sai Prakash Ranjan (sc7280,
sc7180)
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index d01c4c9..2e482cd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -974,6 +974,7 @@ static const struct dpu_perf_cfg sdm845_perf_data = {
.amortizable_threshold = 25,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xf000, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sdm845_qos_linear),
.entries = sdm845_qos_linear
@@ -1001,6 +1002,7 @@ static const struct dpu_perf_cfg sc7180_perf_data = {
.min_dram_ib = 160,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xff, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear
@@ -1028,6 +1030,7 @@ static const struct dpu_perf_cfg sm8150_perf_data = {
.min_dram_ib = 80,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff8, 0xf000, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sm8150_qos_linear),
.entries = sm8150_qos_linear
@@ -1056,6 +1059,7 @@ static const struct dpu_perf_cfg sm8250_perf_data = {
.min_dram_ib = 80,
.min_prefill_lines = 35,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear
@@ -1084,6 +1088,7 @@ static const struct dpu_perf_cfg sc7280_perf_data = {
.min_dram_ib = 160,
.min_prefill_lines = 24,
.danger_lut_tbl = {0x, 0x, 0x0},
+ .safe_lut_tbl = {0xff00, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_macrotile),
.entries = sc7180_qos_macrotile
--
2.7.4
Re: [Resend v3] drm/msm/disp/dpu1: add safe lut config in dpu driver
On Wed, Aug 04, 2021 at 01:38:33AM -0700, Kalyan Thota wrote:
> Add safe lut configuration for all the targets in dpu
> driver as per QOS recommendation.
>
> Issue reported on SC7280:
>
> With wait-for-safe feature in smmu enabled, RT client
> buffer levels are checked to be safe before smmu invalidation.
> Since display was always set to unsafe it was delaying the
> invalidaiton process thus impacting the performance on NRT clients
> such as eMMC and NVMe.
>
> Validated this change on SC7280, With this change eMMC performance
> has improved significantly.
>
> Changes in v2:
> - Add fixes tag (Sai)
> - CC stable kernel (Dimtry)
>
> Changes in v3:
> - Correct fixes tag with appropriate hash (stephen)
> - Resend patch adding reviewed by tag
>
> Fixes: 591e34a091d1 ("drm/msm/disp/dpu1: add support for display for SC7280
> target")
> Signed-off-by: Kalyan Thota
> Reviewed-by: Dmitry Baryshkov
> Tested-by: Sai Prakash Ranjan (sc7280,
> sc7180)
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +
> 1 file changed, 5 insertions(+)
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
Re: [v3] drm/msm/disp/dpu1: add safe lut config in dpu driver
On Wed, Aug 04, 2021 at 01:16:30AM -0700, Kalyan Thota wrote: > Add safe lut configuration for all the targets in dpu > driver as per QOS recommendation. > > Issue reported on SC7280: > > With wait-for-safe feature in smmu enabled, RT client > buffer levels are checked to be safe before smmu invalidation. > Since display was always set to unsafe it was delaying the > invalidaiton process thus impacting the performance on NRT clients > such as eMMC and NVMe. > > Validated this change on SC7280, With this change eMMC performance > has improved significantly. > > Changes in v1: > - Add fixes tag (Sai) > - CC stable kernel (Dimtry) > > Changes in v2: > - Correct fixes tag with appropriate hash (stephen) > > Fixes: 591e34a091d1 (drm/msm/disp/dpu1: add support for display > for SC7280 target) > Signed-off-by: Kalyan Thota > Tested-by: Sai Prakash Ranjan (sc7280, > sc7180) > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 + > 1 file changed, 5 insertions(+) This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.
[Resend v3] drm/msm/disp/dpu1: add safe lut config in dpu driver
Add safe lut configuration for all the targets in dpu
driver as per QOS recommendation.
Issue reported on SC7280:
With wait-for-safe feature in smmu enabled, RT client
buffer levels are checked to be safe before smmu invalidation.
Since display was always set to unsafe it was delaying the
invalidaiton process thus impacting the performance on NRT clients
such as eMMC and NVMe.
Validated this change on SC7280, With this change eMMC performance
has improved significantly.
Changes in v2:
- Add fixes tag (Sai)
- CC stable kernel (Dimtry)
Changes in v3:
- Correct fixes tag with appropriate hash (stephen)
- Resend patch adding reviewed by tag
Fixes: 591e34a091d1 ("drm/msm/disp/dpu1: add support for display for SC7280
target")
Signed-off-by: Kalyan Thota
Reviewed-by: Dmitry Baryshkov
Tested-by: Sai Prakash Ranjan (sc7280,
sc7180)
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index d01c4c9..2e482cd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -974,6 +974,7 @@ static const struct dpu_perf_cfg sdm845_perf_data = {
.amortizable_threshold = 25,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xf000, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sdm845_qos_linear),
.entries = sdm845_qos_linear
@@ -1001,6 +1002,7 @@ static const struct dpu_perf_cfg sc7180_perf_data = {
.min_dram_ib = 160,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xff, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear
@@ -1028,6 +1030,7 @@ static const struct dpu_perf_cfg sm8150_perf_data = {
.min_dram_ib = 80,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff8, 0xf000, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sm8150_qos_linear),
.entries = sm8150_qos_linear
@@ -1056,6 +1059,7 @@ static const struct dpu_perf_cfg sm8250_perf_data = {
.min_dram_ib = 80,
.min_prefill_lines = 35,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear
@@ -1084,6 +1088,7 @@ static const struct dpu_perf_cfg sc7280_perf_data = {
.min_dram_ib = 160,
.min_prefill_lines = 24,
.danger_lut_tbl = {0x, 0x, 0x0},
+ .safe_lut_tbl = {0xff00, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_macrotile),
.entries = sc7180_qos_macrotile
--
2.7.4
[v3] drm/msm/disp/dpu1: add safe lut config in dpu driver
Add safe lut configuration for all the targets in dpu
driver as per QOS recommendation.
Issue reported on SC7280:
With wait-for-safe feature in smmu enabled, RT client
buffer levels are checked to be safe before smmu invalidation.
Since display was always set to unsafe it was delaying the
invalidaiton process thus impacting the performance on NRT clients
such as eMMC and NVMe.
Validated this change on SC7280, With this change eMMC performance
has improved significantly.
Changes in v1:
- Add fixes tag (Sai)
- CC stable kernel (Dimtry)
Changes in v2:
- Correct fixes tag with appropriate hash (stephen)
Fixes: 591e34a091d1 (drm/msm/disp/dpu1: add support for display
for SC7280 target)
Signed-off-by: Kalyan Thota
Tested-by: Sai Prakash Ranjan (sc7280,
sc7180)
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index d01c4c9..2e482cd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -974,6 +974,7 @@ static const struct dpu_perf_cfg sdm845_perf_data = {
.amortizable_threshold = 25,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xf000, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sdm845_qos_linear),
.entries = sdm845_qos_linear
@@ -1001,6 +1002,7 @@ static const struct dpu_perf_cfg sc7180_perf_data = {
.min_dram_ib = 160,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xff, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear
@@ -1028,6 +1030,7 @@ static const struct dpu_perf_cfg sm8150_perf_data = {
.min_dram_ib = 80,
.min_prefill_lines = 24,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff8, 0xf000, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sm8150_qos_linear),
.entries = sm8150_qos_linear
@@ -1056,6 +1059,7 @@ static const struct dpu_perf_cfg sm8250_perf_data = {
.min_dram_ib = 80,
.min_prefill_lines = 35,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear
@@ -1084,6 +1088,7 @@ static const struct dpu_perf_cfg sc7280_perf_data = {
.min_dram_ib = 160,
.min_prefill_lines = 24,
.danger_lut_tbl = {0x, 0x, 0x0},
+ .safe_lut_tbl = {0xff00, 0xff00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_macrotile),
.entries = sc7180_qos_macrotile
--
2.7.4
[PATCH 2/2] drm/panel: Add Truly NT35521 panel driver
It adds a drm driver for Truly NT35521 5.24" 1280x720 DSI panel, which
can be found on Sony Xperia M4 Aqua phone. The panel backlight is
managed through DSI link.
Signed-off-by: Shawn Guo
---
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-truly-nt35521.c | 491
3 files changed, 501 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35521.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index ef87d92cdf49..cdc4abd5c40c 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -537,6 +537,15 @@ config DRM_PANEL_TPO_TPG110
400CH LTPS TFT LCD Single Chip Digital Driver for up to
800x400 LCD panels.
+config DRM_PANEL_TRULY_NT35521
+ tristate "Truly NT35521 panel"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y here if you want to enable support for Truly NT35521
+ 1280x720 DSI panel.
+
config DRM_PANEL_TRULY_NT35597_WQXGA
tristate "Truly WQXGA"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index cae4d976c069..3d3c98cb7a7b 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_DRM_PANEL_TDO_TL070WSH30) +=
panel-tdo-tl070wsh30.o
obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
+obj-$(CONFIG_DRM_PANEL_TRULY_NT35521) += panel-truly-nt35521.o
obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
obj-$(CONFIG_DRM_PANEL_VISIONOX_RM69299) += panel-visionox-rm69299.o
obj-$(CONFIG_DRM_PANEL_XINPENG_XPP055C272) += panel-xinpeng-xpp055c272.o
diff --git a/drivers/gpu/drm/panel/panel-truly-nt35521.c
b/drivers/gpu/drm/panel/panel-truly-nt35521.c
new file mode 100644
index ..ea3cfb46be7e
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-truly-nt35521.c
@@ -0,0 +1,491 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+#include
+
+struct nt35521_panel {
+ struct drm_panel panel;
+ struct device *dev;
+ struct gpio_desc *rst_gpio;
+ struct gpio_desc *pwrp5_gpio;
+ struct gpio_desc *pwrn5_gpio;
+ struct gpio_desc *en_gpio;
+ bool prepared;
+ bool enabled;
+};
+
+static inline struct nt35521_panel *panel_to_nt35521(struct drm_panel *panel)
+{
+ return container_of(panel, struct nt35521_panel, panel);
+}
+
+#define nt_dcs_write(seq...) \
+({ \
+ const u8 d[] = { seq }; \
+ if (mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)) < 0) \
+ DRM_DEV_ERROR(dev, "dcs write buffer failed\n");\
+})
+
+#define nt_gen_write(seq...) \
+({ \
+ const u8 d[] = { seq }; \
+ if (mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)) < 0) \
+ DRM_DEV_ERROR(dev, "generic write buffer failed\n");\
+})
+
+static void nt35521_panel_on(struct nt35521_panel *nt)
+{
+ struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
+ struct device *dev = nt->dev;
+
+ /* Transmit data in low power mode */
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ nt_dcs_write(0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
+ nt_dcs_write(0xff, 0xaa, 0x55, 0xa5, 0x80);
+ nt_dcs_write(0x6f, 0x11, 0x00);
+ nt_dcs_write(0xf7, 0x20, 0x00);
+ nt_dcs_write(0x6f, 0x01);
+ nt_dcs_write(0xb1, 0x21);
+ nt_dcs_write(0xbd, 0x01, 0xa0, 0x10, 0x08, 0x01);
+ nt_dcs_write(0xb8, 0x01, 0x02, 0x0c, 0x02);
+ nt_dcs_write(0xbb, 0x11, 0x11);
+ nt_dcs_write(0xbc, 0x00, 0x00);
+ nt_dcs_write(0xb6, 0x02);
+ nt_dcs_write(0xf0, 0x55, 0xaa, 0x52, 0x08, 0x01);
+ nt_dcs_write(0xb0, 0x09, 0x09);
+ nt_dcs_write(0xb1, 0x09, 0x09);
+ nt_dcs_write(0xbc, 0x8c, 0x00);
+ nt_dcs_write(0xbd, 0x8c, 0x00);
+ nt_dcs_write(0xca, 0x00);
+ nt_dcs_write(0xc0, 0x04);
+ nt_dcs_write(0xbe, 0xb5);
+ nt_dcs_write(0xb3, 0x35, 0x35);
+ nt_dcs_write(0xb4, 0x25, 0x25);
+ nt_dcs_write(0xb9, 0x43, 0x43);
+ nt_dcs_write(0xba, 0x24, 0x24);
+ nt_dcs_write(0xf0, 0x55, 0xaa, 0x52, 0x08, 0x02);
+ nt_dcs_write(0xee, 0x03);
+ nt_dcs_write(0xb0, 0x00, 0xb2, 0x00, 0xb3, 0x00, 0xb6, 0x00,
+0xc3, 0x00, 0xce, 0x00, 0xe1, 0x00, 0xf3, 0x01,
+
[PATCH 1/2] dt-bindings: display: panel: Add Truly NT35521 panel support
The Truly NT35521 is a 5.24" 1280x720 DSI panel, and the backlight is
managed through DSI link.
Signed-off-by: Shawn Guo
---
.../bindings/display/panel/truly,nt35521.yaml | 62 +++
1 file changed, 62 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/truly,nt35521.yaml
diff --git a/Documentation/devicetree/bindings/display/panel/truly,nt35521.yaml
b/Documentation/devicetree/bindings/display/panel/truly,nt35521.yaml
new file mode 100644
index ..4727c3df6eb8
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/truly,nt35521.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/truly,nt35521.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Truly NT35521 5.24" 1280x720 MIPI-DSI Panel
+
+maintainers:
+ - Shawn Guo
+
+description: |
+ The Truly NT35521 is a 5.24" 1280x720 MIPI-DSI panel. The panel backlight
+ is managed through DSI link.
+
+allOf:
+ - $ref: panel-common.yaml#
+
+properties:
+ compatible:
+const: truly,nt35521
+
+ reg: true
+
+ reset-gpios: true
+
+ enable-gpios: true
+
+ pwr-positive5-gpios:
+maxItems: 1
+
+ pwr-negative5-gpios:
+maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - reset-gpios
+ - enable-gpios
+ - pwr-positive5-gpios
+ - pwr-negative5-gpios
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "truly,nt35521";
+reg = <0>;
+reset-gpios = < 25 GPIO_ACTIVE_LOW>;
+pwr-positive5-gpios = < 114 GPIO_ACTIVE_HIGH>;
+pwr-negative5-gpios = < 17 GPIO_ACTIVE_HIGH>;
+enable-gpios = < 10 GPIO_ACTIVE_HIGH>;
+};
+};
+...
--
2.17.1
[PATCH 0/2] Add Truly NT35521 panel driver support
It adds a drm driver for Truly NT35521 5.24" 1280x720 DSI panel, which can be found on Sony Xperia M4 Aqua phone. Shawn Guo (2): dt-bindings: display: panel: Add Truly NT35521 panel support drm/panel: Add Truly NT35521 panel driver .../bindings/display/panel/truly,nt35521.yaml | 62 +++ drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-truly-nt35521.c | 491 ++ 4 files changed, 563 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/truly,nt35521.yaml create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35521.c -- 2.17.1
Re: [PATCH 5/7] drm/i915/gem/ttm: Respect the objection region in placement_from_obj
Hi,
On 7/22/21 11:59 AM, Matthew Auld wrote:
On Thu, 22 Jul 2021 at 10:49, Matthew Auld
wrote:
On Wed, 21 Jul 2021 at 21:11, Jason Ekstrand wrote:
On Mon, Jul 19, 2021 at 8:35 AM Matthew Auld
wrote:
On Fri, 16 Jul 2021 at 20:49, Jason Ekstrand wrote:
On Fri, Jul 16, 2021 at 1:45 PM Matthew Auld
wrote:
On Fri, 16 Jul 2021 at 18:39, Jason Ekstrand wrote:
On Fri, Jul 16, 2021 at 11:00 AM Matthew Auld
wrote:
On Fri, 16 Jul 2021 at 16:52, Matthew Auld
wrote:
On Fri, 16 Jul 2021 at 15:10, Jason Ekstrand wrote:
On Fri, Jul 16, 2021 at 8:54 AM Matthew Auld
wrote:
On Thu, 15 Jul 2021 at 23:39, Jason Ekstrand wrote:
Whenever we had a user object (n_placements > 0), we were ignoring
obj->mm.region and always putting obj->placements[0] as the requested
region. For LMEM+SMEM objects, this was causing them to get shoved into
LMEM on every i915_ttm_get_pages() even when SMEM was requested by, say,
i915_gem_object_migrate().
i915_ttm_migrate calls i915_ttm_place_from_region() directly with the
requested region, so there shouldn't be an issue with migration right?
Do you have some more details?
With i915_ttm_migrate directly, no. But, in the last patch in the
series, we're trying to migrate LMEM+SMEM buffers into SMEM on
attach() and pin it there. This blows up in a very unexpected (IMO)
way. The flow goes something like this:
- Client attempts a dma-buf import from another device
- In attach() we call i915_gem_object_migrate() which calls
i915_ttm_migrate() which migrates as requested.
- Once the migration is complete, we call i915_gem_object_pin_pages()
which calls i915_ttm_get_pages() which depends on
i915_ttm_placement_from_obj() and so migrates it right back to LMEM.
The mm.pages must be NULL here, otherwise it would just increment the
pages_pin_count?
Given that the test is using the four_underscores version, it
doesn't have that check. However, this executes after we've done the
dma-buf import which pinned pages. So we should definitely have
pages.
We shouldn't call four_underscores() if we might already have
pages though. Under non-TTM that would leak the pages, and in TTM we
might hit the WARN_ON(mm->pages) in __i915_ttm_get_pages(), if for
example nothing was moved. I take it we can't just call pin_pages()?
Four scary underscores usually means "don't call this in normal code".
I've switched the four_underscores call to a __two_underscores in
the selftests and it had no effect, good or bad. But, still, probably
better to call that one.
Maybe the problem here is actually that our TTM code isn't respecting
obj->mm.pages_pin_count?
I think if the resource is moved, we always nuke the mm.pages after
being notified of the move. Also TTM is also not allowed to move
pinned buffers.
I guess if we are evicted/swapped, so assuming we are not holding the
object lock, and it's not pinned, the future call to get_pages() will
see mm.pages = NULL, even though the ttm_resource is still there, and
because we prioritise the placements[0], instead of mm.region we end
up moving it for no good reason. But in your case you are holding the
lock, or it's pinned? Also is this just with the selftest, or
something real?
Or at least in the selftest I see i915_gem_object_get_pages()
which doesn't even consider the mm.pages AFAIK.
The bogus migration is happening as part of the
__i915_gem_object_get_pages() (2 __underscores) call in
i915_gem_dmabuf_attach (see last patch). That code is attempting to
migrate the BO to SMEM and then pin it there using the obvious calls
to do so. However, in the pin_pages call, it gets implicitly migrated
back to LMEM thanks to i915_ttm_get_pages(). Why is _get_pages()
migrating things at all?
Not sure yet, but __two_underscores() checks if
i915_gem_object_has_pages() before actually calling into
i915_ttm_get_pages(), so the mm.pages would have to be NULL here for
some reason, so best guess is something to do with move_notify().
Did a bit of experimenting along those lines and added the following
to the self-test BEFORE the export/import:
i915_gem_object_lock(obj, NULL);
err = __i915_gem_object_get_pages(obj);
__i915_gem_object_unpin_pages(obj);
i915_gem_object_unlock(obj);
if (err) {
pr_err("__i915_gem_object_get_pages failed with err=%d\n", err);
goto out_ret;
}
This seems to make the migration happen as expected without this
patch. So it seems the problem only exists on buffers that haven't
gotten any backing storage yet (if I'm understanding get_pages
correctly).
One potential work-around (not sure if this is a good idea or not!)
would be to do this inside dmabuf_attach(). Is this reliable? Once
it has pages will it always have pages? Or are there crazy races I
need to be worried about here?
It turns out that the i915_ttm_adjust_gem_after_move() call in
ttm_object_init will always update the mm.region to system memory(so
that it matches the ttm resource), which seems reasonable
Re: [PATCH] drm/msm/gpu: fix link failure with QCOM_SCM=m
On Mon, Aug 2, 2021 at 4:53 PM Arnd Bergmann wrote:
>
> From: Arnd Bergmann
>
> Another missed dependency when SCM is a loadable module
> and adreno is built-in:
>
> drivers/gpu/drm/msm/adreno/adreno_gpu.o: In function `adreno_zap_shader_load':
> adreno_gpu.c:(.text+0x1e8): undefined reference to `qcom_scm_is_available'
> drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_hw_init':
> a5xx_gpu.c:(.text+0x28a6): undefined reference to `qcom_scm_set_remote_state'
>
> Change it so the dependency on QCOM_SCM and QCOM_MDT_LOADER can be
> ignored if we are not building for ARCH_QCOM, but prevent the
> link error during compile testing when SCM is a loadable module
> and ARCH_QCOM is disabled.
>
> Fixes: a9e2559c931d ("drm/msm/gpu: Move zap shader loading to adreno")
> Fixes: 5ea4dba68305 ("drm/msm/a6xx: add CONFIG_QCOM_LLCC dependency")
> Signed-off-by: Arnd Bergmann
Oh, this is still wrong, for two reasons:
> ---
> drivers/gpu/drm/msm/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index 52536e7adb95..69fbfe4568b2 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -9,14 +9,14 @@ config DRM_MSM
> depends on QCOM_OCMEM || QCOM_OCMEM=n
> depends on QCOM_LLCC || QCOM_LLCC=n
> depends on QCOM_COMMAND_DB || QCOM_COMMAND_DB=n
> + depends on QCOM_SCM || (QCOM_SCM=n && ARCH_QCOM=n)
> + depends on QCOM_MDT_LOADER || ARCH_QCOM=n
* Only QCOM_SCM has become user-selectable, but QCOM_MDT_LOADER
is still meant to only be selected by its users, so we cannot depend on it
here
* There are two other drivers that have the broken 'select QCOM_SCM if
ARCH_QCOM', we have to fix them all at once.
Arnd
Aw: [PATCH v8, 2/2] soc: mediatek: mmsys: Add mt8192 mmsys routing table
Hi can you please test if your device still work after applying this https://patchwork.kernel.org/project/linux-mediatek/patch/20210729070549.5514-1-li...@fw-web.de/ and duplicate value constants in your routes? e.g. changing + DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0, + MT8192_DISP_OVL0_2L_MOUT_EN, MT8192_OVL0_MOUT_EN_DISP_RDMA0, to + DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0, + MT8192_DISP_OVL0_2L_MOUT_EN, MT8192_OVL0_MOUT_EN_DISP_RDMA0, + MT8192_OVL0_MOUT_EN_DISP_RDMA0 regards Frank
RE: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
Hi Gerd, > > > > virtio_gpu_primary_plane_update() will send RESOURCE_FLUSH only for > > > DIRTYFB and both SET_SCANOUT + RESOURCE_FLUSH for page-flip, and I > > > think for the page-flip case the host (aka qemu) doesn't get the > > > "wait until old framebuffer is not in use any more" right yet. > > [Kasireddy, Vivek] As you know, with the GTK UI backend and this patch > > series: > > https://lists.nongnu.org/archive/html/qemu-devel/2021-06/msg06745.html > > we do create a sync file fd -- after the Blit -- and wait (adding it to > > Qemu's main > > event loop) for it to ensure that the Guest scanout FB is longer in use on > > the Host. > > This mechanism works in a similarly way for both frontbuffer DIRTYFB case > > and > > also the double-buffer case. > > Well, we don't explicitly wait on the old framebuffer. Not fully sure > this is actually needed, maybe the command ordering (SET_SCANOUT goes > first) is enough. [Kasireddy, Vivek] When the sync file fd is signaled, the new FB can be considered done/free on the Host; and, when this new FB becomes the old FB -- after another FB is submitted by the Guest -- we don't need to explicitly wait as we already did that in the previous cycle. Strictly speaking, in the double-buffered Guest case, we should be waiting for the sync file fd of the old FB and not the new one. However, if we do this, we saw that the Guest will render faster (~90 FPS) than what the Host can consume (~60 FPS) resulting in unnecessary GPU cycles. And, in addition, we can't be certain about whether a Guest is using double-buffering or single as we noticed that Windows Guests tend to switch between single and double-buffering at runtime based on the damage, etc. > > > > So we'll need a host-side fix for that and a guest-side fix to switch > > > from a blocking wait on the fence to vblank events. > > [Kasireddy, Vivek] Do you see any concerns with the blocking wait? > > Well, it's sync vs. async for userspace. > > With the blocking wait the userspace ioctl (PAGE_FLIP or the atomic > version of it) will return when the host is done. > > Without the blocking wait the userspace ioctl will return right away and > userspace can do something else until the host is done (and the vbland > event is sent to notify userspace). [Kasireddy, Vivek] Right, but upstream Weston -- and I am guessing Mutter as well -- almost always choose DRM_MODE_ATOMIC_NONBLOCK. In this case, the atomic ioctl call would not block and the blocking wait will instead happen in the commit_work/commit_tail workqueue thread. > > > And, are you > > suggesting that we use a vblank timer? > > I think we should send the vblank event when the RESOURCE_FLUSH fence > signals the host is done. [Kasireddy, Vivek] That is how it works now: drm_atomic_helper_commit_planes(dev, old_state, 0); drm_atomic_helper_commit_modeset_enables(dev, old_state); drm_atomic_helper_fake_vblank(old_state); The blocking wait is in the plane_update hook called by drm_atomic_helper_commit_planes() and immediately after that the fake vblank is sent. Thanks, Vivek > > take care, > Gerd
RE: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
Hi Michel, > > > >>> The goal: > >>> - Maintain full framerate even when the Guest scanout FB is flipped onto > >>> a hardware > >> plane > >>> on the Host -- regardless of either compositor's scheduling policy -- > >>> without making > any > >>> copies and ensuring that both Host and Guest are not accessing the buffer > >>> at the same > >> time. > >>> > >>> The problem: > >>> - If the Host compositor flips the client's buffer (in this case Guest > >>> compositor's > buffer) > >>> onto a hardware plane, then it can send a wl_buffer.release event for the > >>> previous > buffer > >>> only after it gets a pageflip completion. And, if the Guest compositor > >>> takes 10-12 ms > to > >>> submit a new buffer and given the fact that the Host compositor waits > >>> only for 9 ms, > the > >>> Guest compositor will miss the Host's repaint cycle resulting in halved > >>> frame-rate. > >>> > >>> 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. [Kasireddy, Vivek] The problem really is that the Guest compositor -- or any other compositor for that matter -- assumes that after a pageflip completion, the old buffer submitted in the previous flip is free and can be reused again. I think this is a guarantee given by KMS. If we have to enforce this, we (Guest KMS) have to wait until the Host compositor sends a wl_buffer.release -- which can only happen after Host gets a pageflip completion assuming it uses hardware planes . From this point onwards, the Guest compositor only has 9 ms (in the case of Weston) -- or less based on the Host compositor's scheduling policy -- to submit a new frame. Although, we can adjust the repaint-window of the Guest compositor to ensure a submission within 9 ms or increase the delay on the Host, these tweaks are just heuristics. I think in order to have a generic solution that'll work in all cases means that the Guest compositor has to start its repaint cycle with a new buffer when the Host sends out the frame callback event. > > 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. [Kasireddy, Vivek] Yes, unfortunately; but the hope is that the Guest KMS can do most of the heavy lifting and keep the changes for the compositors generic enough and minimal. Thanks, Vivek > > > -- > Earthling Michel Dänzer |
Re: [PATCH v2 00/14] drm: Make DRM's IRQ helpers legacy
Hi
Am 03.08.21 um 20:36 schrieb Chrisanthus, Anitha:
Hi Thomas,
Can you please hold off on applying the kmb patch, I am seeing some issues
while testing. Modetest works, but video playback only plays once, and it fails
the second time with this patch.
Sounds a bit like the testing issue at [1]. For testing, you need the
latest drm-misc-next or drm-tip. Specifically, you need commit
1e4cd78ed493 ("drm: Don't test for IRQ support in VBLANK ioctls").
Let me know whether this works for you.
Best regards
Thomas
[1] https://patchwork.freedesktop.org/patch/447057/?series=93078=1
Thanks,
Anitha
-Original Message-
From: Sam Ravnborg
Sent: Tuesday, August 3, 2021 8:05 AM
To: Thomas Zimmermann
Cc: dan...@ffwll.ch; airl...@linux.ie; alexander.deuc...@amd.com;
christian.koe...@amd.com; liviu.du...@arm.com; brian.star...@arm.com;
bbrezil...@kernel.org; nicolas.fe...@microchip.com;
maarten.lankho...@linux.intel.com; mrip...@kernel.org; ste...@agner.ch;
alison.w...@nxp.com; patrik.r.jakobs...@gmail.com; Chrisanthus, Anitha
; robdcl...@gmail.com; Dea, Edmund J
; s...@poorly.run; shawn...@kernel.org;
s.ha...@pengutronix.de; ker...@pengutronix.de; jyri.sa...@iki.fi;
to...@kernel.org; dan.sned...@microchip.com;
tomi.valkei...@ideasonboard.com; amd-...@lists.freedesktop.org; dri-
de...@lists.freedesktop.org; linux-arm-ker...@lists.infradead.org; linux-arm-
m...@vger.kernel.org; freedr...@lists.freedesktop.org
Subject: Re: [PATCH v2 00/14] drm: Make DRM's IRQ helpers legacy
Hi Thomas,
On Tue, Aug 03, 2021 at 11:06:50AM +0200, Thomas Zimmermann wrote:
DRM's IRQ helpers are only helpful for old, non-KMS drivers. Move
the code behind CONFIG_DRM_LEGACY. Convert KMS drivers to Linux
IRQ interfaces.
DRM provides IRQ helpers for setting up, receiving and removing IRQ
handlers. It's an abstraction over plain Linux functions. The code
is mid-layerish with several callbacks to hook into the rsp drivers.
Old UMS driver have their interrupts enabled via ioctl, so these
abstractions makes some sense. Modern KMS manage all their interrupts
internally. Using the DRM helpers adds indirection without benefits.
Most KMS drivers already use Linux IRQ functions instead of DRM's
abstraction layer. Patches 1 to 12 convert the remaining ones.
The patches also resolve a bug for devices without assigned interrupt
number. DRM helpers don't test for IRQ_NOTCONNECTED, so drivers do
not detect if the device has no interrupt assigned.
Patch 13 removes an unused function.
Patch 14 moves the DRM IRQ helpers behind CONFIG_DRM_LEGACY. Only
the old non-KMS drivers still use the functionality.
v2:
* drop IRQ_NOTCONNECTED test from atmel-hlcdc (Sam)
* use devm_request_irq() in atmel-hlcdc (Sam)
* unify variable names in arm/hlcdc (Sam)
Thomas Zimmermann (14):
The following patches are all:
Acked-by: Sam Ravnborg
drm/fsl-dcu: Convert to Linux IRQ interfaces
drm/gma500: Convert to Linux IRQ interfaces
drm/kmb: Convert to Linux IRQ interfaces
drm/msm: Convert to Linux IRQ interfaces
drm/mxsfb: Convert to Linux IRQ interfaces
drm/tidss: Convert to Linux IRQ interfaces
drm/vc4: Convert to Linux IRQ interfaces
drm: Remove unused devm_drm_irq_install()
The remaining patches I either skipped or already had a feedback from
me or I asked a question.
Sam
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
RE: [PATCH 2/4] drm/dp_mst: Only create connector for connected end device
[Public] > -Original Message- > From: Lyude Paul > Sent: Wednesday, August 4, 2021 8:09 AM > To: Lin, Wayne ; dri-devel@lists.freedesktop.org > Cc: Kazlauskas, Nicholas ; Wentland, Harry > ; Zuo, Jerry > ; Wu, Hersen ; Juston Li > ; Imre Deak ; > Ville Syrjälä ; Wentland, Harry > ; Daniel Vetter ; > Sean Paul ; Maarten Lankhorst > ; Maxime Ripard ; > Thomas Zimmermann ; David Airlie ; > Daniel Vetter ; Deucher, > Alexander ; Siqueira, Rodrigo > ; Pillai, Aurabindo > ; Eryk Brol ; Bas Nieuwenhuizen > ; Cornij, Nikola > ; Jani Nikula ; Manasi Navare > ; Ankit Nautiyal > ; José Roberto de Souza ; > Sean Paul ; Ben Skeggs > ; sta...@vger.kernel.org > Subject: Re: [PATCH 2/4] drm/dp_mst: Only create connector for connected end > device > > On Tue, 2021-08-03 at 19:58 -0400, Lyude Paul wrote: > > On Wed, 2021-07-21 at 00:03 +0800, Wayne Lin wrote: > > > [Why] > > > Currently, we will create connectors for all output ports no matter > > > it's connected or not. However, in MST, we can only determine > > > whether an output port really stands for a "connector" till it is > > > connected and check its peer device type as an end device. > > > > What is this commit trying to solve exactly? e.g. is AMD currently > > running into issues with there being too many DRM connectors or something > > like that? > > Ideally this is behavior I'd very much like us to keep as-is unless > > there's good reason to change it. Hi Lyude, Really appreciate for your time to elaborate in such detail. Thanks! I come up with this commit because I observed something confusing when I was analyzing MST connectors' life cycle. Take the topology instance you mentioned below Root MSTB -> Output_Port 1 -> MSTB 1.1 ->Output_Port 1(Connected w/ display) | ->Output_Port 2 (Disconnected) -> Output_Port 2 -> MSTB 2.1 ->Output_Port 1 (Disconnected) -> Output_Port 2 (Disconnected) Which is exactly the topology of Startech DP 1-to-4 hub. There are 3 1-to-2 branch chips within this hub. With our MST implementation today, we'll create drm connectors for all output ports. Hence, we totally create 6 drm connectors here. However, Output ports of Root MSTB are not connected to a stream sink. They are connected with branch devices. Thus, creating drm connector for such port looks a bit strange to me and increases complexity to tracking drm connectors. My thought is we only need to create drm connector for those connected end device. Once output port is connected then we can determine whether to add on a drm connector for this port based on the peer device type. Hence, this commit doesn't try to break the locking logic but add more constraints when We try to add drm connector. Please correct me if I misunderstand anything here. Thanks! > > > > Some context here btw - there's a lot of subtleties with MST locking > > that isn't immediately obvious. It's been a while since I wrote this > > code, but if I recall correctly one of those subtleties is that trying > > to create/destroy connectors on the fly when ports change types > > introduces a lot of potential issues with locking and some very > > complicated state transitions. Note that because we maintain the > > topology as much as possible across suspend/resumes this means there's > > a lot of potential state transitions with drm_dp_mst_port and > > drm_dp_mst_branch we need to handle that would typically be impossible > > to run into otherwise. > > > > An example of this, if we were to try to prune connectors based on PDT > > on the fly: assume we have a simple topology like this > > > > Root MSTB -> Port 1 -> MSTB 1.1 (Connected w/ display) > > -> Port 2 -> MSTB 2.1 > > > > We suspend the system, unplug MSTB 1.1, and then resume. Once the > > system starts reprobing, it will notice that MSTB 1.1 has been > > disconnected. Since we no longer have a PDT, we decide to unregister > > our connector. But there's a catch! We had a display connected to MSTB > > 1.1, so even after unregistering the connector it's going to stay > > around until userspace has committed a new mode with the connector disabled. > > > > Now - assuming we're still in the same spot in the resume processs, > > let's assume somehow MSTB 1.1 is suddenly plugged back in. Once we've > > finished responding to the hotplug event, we will have created a > > connector for it. Now we've hit a bug - userspace hasn't removed the > > previous zombie connector which means we have references to the > > drm_dp_mst_port in our atomic state and potentially also our payload > > tables (?? unsure about this one). > > Whoops. One thing I totally forgot to mention here: the reason this is a > problem is because we'd now have two drm_connectors > which both have the same drm_dp_mst_port pointer. > > > > > So then how do we manage to add/remove connectors
Re: [PATCH 4/7] drm/i915/gem/ttm: Place new BOs in the requested region
On 8/4/21 8:49 AM, Thomas Hellström wrote: Hi, Jason, On 7/16/21 12:38 AM, Jason Ekstrand wrote: __i915_gem_ttm_object_init() was ignoring the placement requests coming from the client and always placing all BOs in SMEM upon creation. Instead, compute the requested placement set from the object and pass that into ttm_bo_init_reserved(). This is done on purpose. When objects are initially created in SMEM, they are created in "Limbo", meaning they have no pages and costly allocation and clearing is deferred to first get_pages(). So we shouldn't be doing this. Ah, I see Matthew already responded on this. Sorry for the noise. /Thomas
Re: [PATCH 4/7] drm/i915/gem/ttm: Place new BOs in the requested region
Hi, Jason, On 7/16/21 12:38 AM, Jason Ekstrand wrote: __i915_gem_ttm_object_init() was ignoring the placement requests coming from the client and always placing all BOs in SMEM upon creation. Instead, compute the requested placement set from the object and pass that into ttm_bo_init_reserved(). This is done on purpose. When objects are initially created in SMEM, they are created in "Limbo", meaning they have no pages and costly allocation and clearing is deferred to first get_pages(). So we shouldn't be doing this. /Thomas
Re: [PATCH] drm/amdgpu: drop redundant null-pointer checks in amdgpu_ttm_tt_populate() and amdgpu_ttm_tt_unpopulate()
Am 04.08.21 um 03:51 schrieb Tuo Li:
The varialbe gtt in the function amdgpu_ttm_tt_populate() and
amdgpu_ttm_tt_unpopulate() is guaranteed to be not NULL in the context.
Thus the null-pointer checks are redundant and can be dropped.
Reported-by: TOTE Robot
Signed-off-by: Tuo Li
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3a55f08e00e1..719539bd6c44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1121,7 +1121,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_device *bdev,
struct amdgpu_ttm_tt *gtt = (void *)ttm;
/* user pages are bound by amdgpu_ttm_tt_pin_userptr() */
- if (gtt && gtt->userptr) {
+ if (gtt->userptr) {
ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!ttm->sg)
return -ENOMEM;
@@ -1146,7 +1146,7 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device
*bdev,
struct amdgpu_ttm_tt *gtt = (void *)ttm;
struct amdgpu_device *adev;
- if (gtt && gtt->userptr) {
+ if (gtt->userptr) {
amdgpu_ttm_tt_set_user_pages(ttm, NULL);
kfree(ttm->sg);
ttm->sg = NULL;
