Re: [PATCH 1/2] drm/ttm: drop size from resource manager base class.
On Thu, 6 Aug 2020 at 14:39, Dave Airlie wrote: > > From: Dave Airlie Just realised this is on an unpublished base, I'd had amended one amdgpu ttm patch to avoid kzalloc, but not sent it out, but hadn't rebased it either. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/ttm/nouveau: move io_lru storage into driver.
From: Dave Airlie This moves the io lru tracking into the driver allocated structure. Probably need to consider if we can move more stuff in there around the nouveau only io_lru functionality. --- drivers/gpu/drm/nouveau/nouveau_ttm.c | 24 +--- drivers/gpu/drm/ttm/ttm_bo.c | 11 --- drivers/gpu/drm/ttm/ttm_bo_util.c | 24 +++- include/drm/ttm/ttm_bo_api.h | 3 +++ include/drm/ttm/ttm_bo_driver.h | 19 +++ 5 files changed, 58 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c index 2aba2e245548..5e0008d98b07 100644 --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c @@ -31,6 +31,16 @@ #include +struct nouveau_resource_manager { + struct ttm_resource_manager manager; + struct ttm_resource_manager_io_lru io_lru; +}; + +static inline struct nouveau_resource_manager *to_mgr(struct ttm_resource_manager *man) +{ + return container_of(man, struct nouveau_resource_manager, manager); +} + static void nouveau_manager_del(struct ttm_resource_manager *man, struct ttm_resource *reg) { @@ -63,9 +73,15 @@ nouveau_vram_manager_new(struct ttm_resource_manager *man, return 0; } +static struct ttm_resource_manager_io_lru *nouveau_resource_io_lru(struct ttm_resource_manager *man) +{ + return _mgr(man)->io_lru; +} + const struct ttm_resource_manager_func nouveau_vram_manager = { .get_node = nouveau_vram_manager_new, .put_node = nouveau_manager_del, + .io_lru = nouveau_resource_io_lru, }; static int @@ -160,7 +176,8 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm) if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_TESLA) { /* Some BARs do not support being ioremapped WC */ const u8 type = mmu->type[drm->ttm.type_vram].type; - struct ttm_resource_manager *man = kzalloc(sizeof(struct ttm_resource_manager), GFP_KERNEL); + struct nouveau_resource_manager *nman = kzalloc(sizeof(*nman), GFP_KERNEL); + struct ttm_resource_manager *man = >manager; if (!man) return -ENOMEM; @@ -173,9 +190,9 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm) } man->func = _vram_manager; - man->use_io_reserve_lru = true; ttm_resource_manager_init(man); + ttm_resource_manager_io_lru_init(>io_lru); ttm_set_driver_manager(>ttm.bdev, TTM_PL_VRAM, man); ttm_resource_manager_set_used(man, true); return 0; @@ -193,11 +210,12 @@ nouveau_ttm_fini_vram(struct nouveau_drm *drm) struct ttm_resource_manager *man = ttm_manager_type(>ttm.bdev, TTM_PL_VRAM); if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_TESLA) { + struct nouveau_resource_manager *nman = to_mgr(man); ttm_resource_manager_set_used(man, false); ttm_resource_manager_force_list_clean(>ttm.bdev, man); ttm_resource_manager_cleanup(man); ttm_set_driver_manager(>ttm.bdev, TTM_PL_VRAM, NULL); - kfree(man); + kfree(nman); } else ttm_range_man_fini(>ttm.bdev, TTM_PL_VRAM); } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 74b7def25f91..e4aa6eb60a50 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1469,14 +1469,19 @@ int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned mem_type) } EXPORT_SYMBOL(ttm_bo_evict_mm); +void ttm_resource_manager_io_lru_init(struct ttm_resource_manager_io_lru *io_lru) +{ + mutex_init(_lru->io_reserve_mutex); + INIT_LIST_HEAD(_lru->io_reserve_lru); +} +EXPORT_SYMBOL(ttm_resource_manager_io_lru_init); + void ttm_resource_manager_init(struct ttm_resource_manager *man) { unsigned i; - man->use_io_reserve_lru = false; - mutex_init(>io_reserve_mutex); + spin_lock_init(>move_lock); - INIT_LIST_HEAD(>io_reserve_lru); for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) INIT_LIST_HEAD(>lru[i]); diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 496158acd5b9..137aab6c53ae 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -93,29 +93,33 @@ EXPORT_SYMBOL(ttm_bo_move_ttm); int ttm_mem_io_lock(struct ttm_resource_manager *man, bool interruptible) { - if (likely(!man->use_io_reserve_lru)) + struct ttm_resource_manager_io_lru *io_lru; + if (!man->func && !man->func->io_lru) return 0; + io_lru = man->func->io_lru(man); if (interruptible) - return mutex_lock_interruptible(>io_reserve_mutex); + return
[rfc] further ttm cleanups
these are just for discussion mostly, one drop size from the resource manager the other moves the storage for the io_lru into nouveau. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/ttm: drop size from resource manager base class.
From: Dave Airlie This is a bit more involved that it looked, the range manager needs accessors adding and amdgpu needs a bit of a refactor. --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 17 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 13 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 12 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 14 + drivers/gpu/drm/nouveau/nouveau_ttm.c | 5 ++--- drivers/gpu/drm/radeon/radeon_gem.c | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c | 3 +-- drivers/gpu/drm/ttm/ttm_bo.c | 7 ++- drivers/gpu/drm/ttm/ttm_range_manager.c | 21 +-- drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 9 include/drm/ttm/ttm_bo_api.h | 4 +--- include/drm/ttm/ttm_bo_driver.h | 20 -- 15 files changed, 84 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index ccc8b8196d26..db5c0d34daa1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -24,11 +24,6 @@ #include "amdgpu.h" -static inline struct amdgpu_gtt_mgr *to_gtt_mgr(struct ttm_resource_manager *man) -{ - return container_of(man, struct amdgpu_gtt_mgr, manager); -} - struct amdgpu_gtt_node { struct drm_mm_node node; struct ttm_buffer_object *tbo; @@ -48,8 +43,9 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev, struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = ddev->dev_private; struct ttm_resource_manager *man = ttm_manager_type(>mman.bdev, TTM_PL_TT); + struct amdgpu_gtt_mgr *gtt_mgr = to_gtt_mgr(man); return snprintf(buf, PAGE_SIZE, "%llu\n", - man->size * PAGE_SIZE); + gtt_mgr->size * PAGE_SIZE); } /** @@ -96,13 +92,14 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size) man->available_caching = TTM_PL_MASK_CACHING; man->default_caching = TTM_PL_FLAG_CACHED; - ttm_resource_manager_init(man, gtt_size >> PAGE_SHIFT); + mgr->size = gtt_size >> PAGE_SHIFT; + ttm_resource_manager_init(man); start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS; size = (adev->gmc.gart_size >> PAGE_SHIFT) - start; drm_mm_init(>mm, start, size); spin_lock_init(>lock); - atomic64_set(>available, gtt_size >> PAGE_SHIFT); + atomic64_set(>available, mgr->size); ret = device_create_file(adev->dev, _attr_mem_info_gtt_total); if (ret) { @@ -262,7 +259,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager *man) { struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man); - s64 result = man->size - atomic64_read(>available); + s64 result = mgr->size - atomic64_read(>available); return (result > 0 ? result : 0) * PAGE_SIZE; } @@ -304,7 +301,7 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man, spin_unlock(>lock); drm_printf(printer, "man size:%llu pages, gtt available:%lld pages, usage:%lluMB\n", - man->size, (u64)atomic64_read(>available), + mgr->size, (u64)atomic64_read(>available), amdgpu_gtt_mgr_usage(man) >> 20); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 2763bca163e3..8dcee654d7f5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -623,7 +623,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file min(adev->gmc.visible_vram_size - atomic64_read(>visible_pin_size), vram_gtt.vram_size); - vram_gtt.gtt_size = ttm_manager_type(>mman.bdev, TTM_PL_TT)->size; + vram_gtt.gtt_size = to_gtt_mgr(ttm_manager_type(>mman.bdev, TTM_PL_TT))->size; vram_gtt.gtt_size *= PAGE_SIZE; vram_gtt.gtt_size -= atomic64_read(>gart_pin_size); return copy_to_user(out, _gtt, @@ -635,6 +635,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file ttm_manager_type(>mman.bdev, TTM_PL_VRAM); struct ttm_resource_manager *gtt_man = ttm_manager_type(>mman.bdev, TTM_PL_TT); + struct amdgpu_gtt_mgr *gtt_mgr = to_gtt_mgr(gtt_man); memset(, 0, sizeof(mem)); mem.vram.total_heap_size =
Re: [PATCH 39/49] drm/ttm: make ttm_bo_man_init/takedown take type + args
> That was not what I was talking about. Take a look at what those fields > are used for :) > > > As far as I see the only usage of the size is in > ttm_resource_manager_debug(). But this size is actually totally opaque > to TTM, it could be pages, bytes, fried chicken wings or whatever. In > other words it would be much better to print it in the debug callback of > each resource manager. Size is a bit trickier as the drivers use in a couple of funky places, radeon pokes inside the range manager and adjust its size post init, and amdgpu uses size to validate a bunch of bo sizing. These shouldn't be too messy to workaround. > The available_caching is completely superfluous as well. The original > idea what that a driver could specify multiple placements as flags in > one entry, but no driver ever used that as far as I know and it became > completely deprecated in 2014 when I moved the lpfn and fpfn into each > place. > > > The default_caching is used in ttm_bo_select_caching(), but this is > complete utterly nonsense. It just results in multiple possible cache > behaviors being selected, which are then fortunately ignored by > ttm_tt_set_placement_caching :) > > > To be honest I think just removing those parameters and the associated > caching flags all together is the next logical step. Definitely something to burn down alright. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm next for 5.9-rc1
The pull request you sent on Thu, 6 Aug 2020 11:07:02 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-next-2020-08-06 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/8186749621ed6b8fc42644c399e8c755a2b6f630 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm next for 5.9-rc1
On Thu, 6 Aug 2020 at 11:07, Dave Airlie wrote: > > Hi Linus, > > This the main drm pull request for 5.9-rc1. > > New xilinx displayport driver, AMD support for two new GPUs (more > header files), i915 initial support for RocketLake and some work on > their DG1 (discrete chip). > > The core also grew some lockdep annotations to try and constrain what > drivers do with dma-fences, and added some documentation on why the > idea of indefinite fences doesn't work. > > The long list is below. > > I did a test merge into your tree and only had two minor conflicts, so > I think you should be able to take care of it fine. I should say I did a test merge yesterday, but you likely pulled more trees, https://lore.kernel.org/dri-devel/20200806115140.6aa46...@canb.auug.org.au/T/#t So there was an unfortunate miscommunication and one patch went two ways, in future Jason and Ben will coordinate better. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: rcar-du: Fix pitch handling for fully planar YUV formats
When creating a frame buffer, the driver verifies that the pitches for the chroma planes match the luma plane. This is done incorrectly for fully planar YUV formats, without taking horizontal subsampling into account. Fix it. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 52 ++- drivers/gpu/drm/rcar-du/rcar_du_kms.h | 1 + 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 482329102f19..2fda3734a57e 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -40,6 +40,7 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_RGB565, .bpp = 16, .planes = 1, + .hsub = 1, .pnmr = PnMR_SPIM_TP | PnMR_DDDF_16BPP, .edf = PnDDCR4_EDF_NONE, }, { @@ -47,6 +48,7 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_ARGB555, .bpp = 16, .planes = 1, + .hsub = 1, .pnmr = PnMR_SPIM_ALP | PnMR_DDDF_ARGB, .edf = PnDDCR4_EDF_NONE, }, { @@ -61,6 +63,7 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_XBGR32, .bpp = 32, .planes = 1, + .hsub = 1, .pnmr = PnMR_SPIM_TP | PnMR_DDDF_16BPP, .edf = PnDDCR4_EDF_RGB888, }, { @@ -68,6 +71,7 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_ABGR32, .bpp = 32, .planes = 1, + .hsub = 1, .pnmr = PnMR_SPIM_ALP | PnMR_DDDF_16BPP, .edf = PnDDCR4_EDF_ARGB, }, { @@ -75,6 +79,7 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_UYVY, .bpp = 16, .planes = 1, + .hsub = 2, .pnmr = PnMR_SPIM_TP_OFF | PnMR_DDDF_YC, .edf = PnDDCR4_EDF_NONE, }, { @@ -82,6 +87,7 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_YUYV, .bpp = 16, .planes = 1, + .hsub = 2, .pnmr = PnMR_SPIM_TP_OFF | PnMR_DDDF_YC, .edf = PnDDCR4_EDF_NONE, }, { @@ -89,6 +95,7 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_NV12M, .bpp = 12, .planes = 2, + .hsub = 2, .pnmr = PnMR_SPIM_TP_OFF | PnMR_DDDF_YC, .edf = PnDDCR4_EDF_NONE, }, { @@ -96,6 +103,7 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_NV21M, .bpp = 12, .planes = 2, + .hsub = 2, .pnmr = PnMR_SPIM_TP_OFF | PnMR_DDDF_YC, .edf = PnDDCR4_EDF_NONE, }, { @@ -103,6 +111,7 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_NV16M, .bpp = 16, .planes = 2, + .hsub = 2, .pnmr = PnMR_SPIM_TP_OFF | PnMR_DDDF_YC, .edf = PnDDCR4_EDF_NONE, }, @@ -115,156 +124,187 @@ static const struct rcar_du_format_info rcar_du_format_infos[] = { .v4l2 = V4L2_PIX_FMT_RGB332, .bpp = 8, .planes = 1, + .hsub = 1, }, { .fourcc = DRM_FORMAT_ARGB, .v4l2 = V4L2_PIX_FMT_ARGB444, .bpp = 16, .planes = 1, + .hsub = 1, }, { .fourcc = DRM_FORMAT_XRGB, .v4l2 = V4L2_PIX_FMT_XRGB444, .bpp = 16, .planes = 1, + .hsub = 1, }, { .fourcc = DRM_FORMAT_RGBA, .v4l2 = V4L2_PIX_FMT_RGBA444, .bpp = 16, .planes = 1, + .hsub = 1, }, { .fourcc = DRM_FORMAT_RGBX, .v4l2 = V4L2_PIX_FMT_RGBX444, .bpp = 16, .planes = 1, + .hsub = 1, }, { .fourcc = DRM_FORMAT_ABGR, .v4l2 = V4L2_PIX_FMT_ABGR444, .bpp = 16, .planes = 1, + .hsub = 1, }, { .fourcc = DRM_FORMAT_XBGR, .v4l2 = V4L2_PIX_FMT_XBGR444, .bpp = 16, .planes = 1, + .hsub = 1, }, { .fourcc = DRM_FORMAT_BGRA,
Re: linux-next: manual merge of the hmm tree with the drm tree
Hi all, On Thu, 30 Jul 2020 19:21:10 +1000 Stephen Rothwell wrote: > > Today's linux-next merge of the hmm tree got a conflict in: > > drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c > > between commit: > > 7763d24f3ba0 ("drm/nouveau/vmm/gp100-: fix mapping 2MB sysmem pages") > > from the drm tree and commits: > > 4725c6b82a48 ("nouveau: fix mapping 2MB sysmem pages") > 1a77decd0cae ("nouveau: fix storing invalid ptes") > > from the hmm tree. > > 7763d24f3ba0 and 4725c6b82a48 are exactly the same patch. > > I fixed it up (I just used the latter version) and can carry the fix as > necessary. This is now fixed as far as linux-next is concerned, but any > non trivial conflicts should be mentioned to your upstream maintainer > when your tree is submitted for merging. You may also want to consider > cooperating with the maintainer of the conflicting tree to minimise any > particularly complex conflicts. This is now a conflict between the drm tree and Linus' tree. -- Cheers, Stephen Rothwell pgpGHQlFemUhM.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: manual merge of the devicetree tree with the drm tree
Hi all, On Wed, 22 Jul 2020 15:52:39 +1000 Stephen Rothwell wrote: > > Today's linux-next merge of the devicetree tree got a conflict in: > > Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt > > between commit: > > 5a2e9b658cdc ("dt-bindings: drm/bridge: ti-sn65dsi86: Convert to yaml") > > from the drm tree and commit: > > 382646090f7f ("dt-bindings: drm/bridge: Replace HTTP links with HTTPS ones") > > from the devicetree tree. > > I fixed it up (I delete the file and adde the following merge fix > patch) and can carry the fix as necessary. This is now fixed as far as > linux-next is concerned, but any non trivial conflicts should be > mentioned to your upstream maintainer when your tree is submitted for > merging. You may also want to consider cooperating with the maintainer > of the conflicting tree to minimise any particularly complex conflicts. > > From: Stephen Rothwell > Date: Wed, 22 Jul 2020 15:47:22 +1000 > Subject: [PATCH] fix for "dt-bindings: drm/bridge: Replace HTTP links with > HTTPS ones" > > Signed-off-by: Stephen Rothwell > --- > .../devicetree/bindings/display/bridge/ti,sn65dsi86.yaml| 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml > index be10e8cf31e1..f8622bd0f61e 100644 > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml > @@ -11,7 +11,7 @@ maintainers: > > description: | >The Texas Instruments SN65DSI86 bridge takes MIPI DSI in and outputs eDP. > - > http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf > + > https://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf > > properties: >compatible: > -- > 2.27.0 This is now a conflict between the drm tree and Linus' tree. -- Cheers, Stephen Rothwell pgpn9JndYL3zH.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:amd-staging-drm-next 628/1313] drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10_3.c:268:17: sparse: sparse: cast removes address space '' of expression
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next head: e1d114658d448b5da11c7925beda241e8b8dd321 commit: 10be8791067fc672e44fcaa7afb886390909a0c0 [628/1313] drm/amdkfd: Support Sienna_Cichlid KFD v4 config: x86_64-randconfig-s021-20200805 (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-117-g8c7aee71-dirty git checkout 10be8791067fc672e44fcaa7afb886390909a0c0 # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10_3.c:268:17: sparse: sparse: >> cast removes address space '' of expression drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10_3.c:270:17: sparse: sparse: cast removes address space '' of expression vim +268 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10_3.c 192 193 static int hqd_load_v10_3(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id, 194 uint32_t queue_id, uint32_t __user *wptr, 195 uint32_t wptr_shift, uint32_t wptr_mask, 196 struct mm_struct *mm) 197 { 198 struct amdgpu_device *adev = get_amdgpu_device(kgd); 199 struct v10_compute_mqd *m; 200 uint32_t *mqd_hqd; 201 uint32_t reg, hqd_base, data; 202 203 m = get_mqd(mqd); 204 205 pr_debug("Load hqd of pipe %d queue %d\n", pipe_id, queue_id); 206 acquire_queue(kgd, pipe_id, queue_id); 207 208 /* HIQ is set during driver init period with vmid set to 0*/ 209 if (m->cp_hqd_vmid == 0) { 210 uint32_t value, mec, pipe; 211 212 mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1; 213 pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec); 214 215 pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n", 216 mec, pipe, queue_id); 217 value = RREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS)); 218 value = REG_SET_FIELD(value, RLC_CP_SCHEDULERS, scheduler1, 219 ((mec << 5) | (pipe << 3) | queue_id | 0x80)); 220 WREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS), value); 221 } 222 223 /* HQD registers extend from CP_MQD_BASE_ADDR to CP_HQD_EOP_WPTR_MEM. */ 224 mqd_hqd = >cp_mqd_base_addr_lo; 225 hqd_base = SOC15_REG_OFFSET(GC, 0, mmCP_MQD_BASE_ADDR); 226 227 for (reg = hqd_base; 228 reg <= SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_HI); reg++) 229 WREG32(reg, mqd_hqd[reg - hqd_base]); 230 231 232 /* Activate doorbell logic before triggering WPTR poll. */ 233 data = REG_SET_FIELD(m->cp_hqd_pq_doorbell_control, 234 CP_HQD_PQ_DOORBELL_CONTROL, DOORBELL_EN, 1); 235 WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_DOORBELL_CONTROL), data); 236 237 if (wptr) { 238 /* Don't read wptr with get_user because the user 239 * context may not be accessible (if this function 240 * runs in a work queue). Instead trigger a one-shot 241 * polling read from memory in the CP. This assumes 242 * that wptr is GPU-accessible in the queue's VMID via 243 * ATC or SVM. WPTR==RPTR before starting the poll so 244 * the CP starts fetching new commands from the right 245 * place. 246 * 247 * Guessing a 64-bit WPTR from a 32-bit RPTR is a bit 248 * tricky. Assume that the queue didn't overflow. The 249 * number of valid bits in the 32-bit RPTR depends on 250 * the queue size. The remaining bits are taken from 251 * the saved 64-bit WPTR. If the WPTR wrapped, add the 252 * queue size. 253 */ 254 uint32_t queue_size = 255 2 << REG_GET_FIELD(m->cp_hqd_pq_control, 256 CP_HQD_PQ_CONTROL, QUEUE_SIZE); 257 uint64_t guessed_wptr = m->cp_hqd_pq_rptr & (queue_size - 1); 258 259 if ((m->cp_hqd_pq_wptr_lo & (queue_size - 1)) < guessed_wptr) 260 guessed_wptr += queue_size; 261 g
[Bug 208825] New: lspci triggers NULL pointer dereference on AMD Renoir 4800H/5600M laptop
https://bugzilla.kernel.org/show_bug.cgi?id=208825 Bug ID: 208825 Summary: lspci triggers NULL pointer dereference on AMD Renoir 4800H/5600M laptop Product: Drivers Version: 2.5 Kernel Version: 5.8.0 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: jontourvi...@me.com Regression: No Created attachment 290791 --> https://bugzilla.kernel.org/attachment.cgi?id=290791=edit lspci triggers NULL pointer dereference on AMD Renoir laptop Running Arch Linux with 5.8.0 kernel built from linux-mainline on a Dell G5 15 SE 5505 laptop with a AMD 4800H Renoir APU and 5600M discrete GPU. On a fresh install of Arch, running lspci triggers an oops and NULL pointer dereference. The oops is not triggered if the kernel is booted with amdgpu.runpm=0, so it appears to be power management-related. The oops kicks off with the following errors (full dmesg and lspci -vvv output attached): [ 93.485414] [drm] PCIE GART of 512M enabled (table at 0x0080). [ 93.485452] [drm] PSP is resuming... [ 93.514696] [drm] reserve 0x90 from 0x800f40 for PSP TMR [ 93.684656] amdgpu :03:00.0: amdgpu: RAS: optional ras ta ucode is not available [ 93.704673] amdgpu: SMU is resuming... [ 95.835970] amdgpu: failed send message: RunBtc (58) param: 0x response 0xffc2 [ 95.835971] amdgpu: RunBtc failed! [ 95.836016] [drm:amdgpu_device_ip_resume_phase2 [amdgpu]] *ERROR* resume of IP block failed -62 [ 95.836053] [drm:amdgpu_device_resume [amdgpu]] *ERROR* amdgpu_device_ip_resume failed (-62). [ 95.851331] snd_hda_intel :03:00.1: refused to change power state from D3hot to D0 [ 95.956286] snd_hda_intel :03:00.1: CORB reset timeout#2, CORBRP = 65535 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:amd-staging-drm-next 468/1313] drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:172:15: sparse: sparse: incorrect type in argument 1 (different address spaces)
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next head: e1d114658d448b5da11c7925beda241e8b8dd321 commit: 3a2b9affb4c366dac8a088156c644cf329701816 [468/1313] drm/amdkfd: Track SDMA utilization per process config: x86_64-randconfig-s021-20200805 (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-117-g8c7aee71-dirty git checkout 3a2b9affb4c366dac8a088156c644cf329701816 # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot sparse warnings: (new ones prefixed by >>) drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:140:6: sparse: sparse: symbol 'increment_queue_count' was not declared. Should it be static? drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:148:6: sparse: sparse: symbol 'decrement_queue_count' was not declared. Should it be static? >> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:172:15: >> sparse: sparse: incorrect type in argument 1 (different address spaces) @@ >> expected void const volatile [noderef] * @@ got unsigned long >> long [usertype] * @@ >> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:172:15: >> sparse: expected void const volatile [noderef] * drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:172:15: sparse: got unsigned long long [usertype] * vim +172 drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c 155 156 int read_sdma_queue_counter(struct queue *q, uint64_t *val) 157 { 158 int ret; 159 uint64_t tmp = 0; 160 161 if (!q || !val) 162 return -EINVAL; 163 /* 164 * SDMA activity counter is stored at queue's RPTR + 0x8 location. 165 */ 166 if (!access_ok((const void __user *)((uint64_t)q->properties.read_ptr + 167 sizeof(uint64_t)), sizeof(uint64_t))) { 168 pr_err("Can't access sdma queue activity counter\n"); 169 return -EFAULT; 170 } 171 > 172 ret = get_user(tmp, (uint64_t > *)((uint64_t)(q->properties.read_ptr) + 173 sizeof(uint64_t))); 174 if (!ret) { 175 *val = tmp; 176 } 177 178 return ret; 179 } 180 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
Hi all, On Wed, 05 Aug 2020 15:19:38 -0700 Joe Perches wrote: > > On Wed, 2020-08-05 at 17:27 -0400, Alex Deucher wrote: > > On Wed, Aug 5, 2020 at 4:53 PM Joe Perches wrote: > > > On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote: > > > > On Wed, Aug 5, 2020 at 7:35 AM Colin King > > > > wrote: > > > > > From: Colin Ian King > > > > > > > > > > There is a spelling mistake in a DRM_ERROR message. Fix it. > > > > > > > > > > Signed-off-by: Colin Ian King > > > > > > > > This is already fixed. > > > > > > This fix is not in today's -next. > > > > > > Perhaps whatever tree it's fixed in should be in -next. > > > > > > > Weird. It's in the drm-next tree as: > > > > commit 4afaa61db9cf5250b5734c2531b226e7b3a3d691 > > Author: Colin Ian King > > Date: Fri Jul 10 09:37:58 2020 +0100 > > > > drm/amdgpu: fix spelling mistake "Falied" -> "Failed" > > > > There is a spelling mistake in a DRM_ERROR error message. Fix it. > > > > Signed-off-by: Colin Ian King > > Signed-off-by: Alex Deucher > > > > Alex > > > > > $ git show --oneline -s > > > d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add > > > linux-next specific files for 20200805 > > > > > > $ git grep -i falied drivers > > > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied > > > to terminate tmr\n"); > > > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > > > [] > > > > > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle) > > > > > > > > > > ret = psp_tmr_terminate(psp); > > > > > if (ret) { > > > > > - DRM_ERROR("Falied to terminate tmr\n"); > > > > > + DRM_ERROR("Failed to terminate tmr\n"); > > > > > return ret; > > > > > } > > Dunno. > > Maybe it's due to some ordering of trees in > how -next accumulates patches? The spelling error is introduced in two commits: c564b8601ae9 ("drm/amdgpu: add TMR destory function for psp") in Linus' tree between v5.8-rc4 and rc5 90937420c44f ("drm/amdgpu: add TMR destory function for psp") in the amdgpu tree between two merges by the drm tree. In this same interval, the error is corrected by commit 4afaa61db9cf ("drm/amdgpu: fix spelling mistake "Falied" -> "Failed"") so when David comes to merge the amdgpu tree in commit 206739119508 ("Merge tag 'amd-drm-next-5.9-2020-07-17' of git://people.freedesktop.org/~agd5f/linux into drm-next") the spelling error has been introduced on one side of the merge and introduced and corrected on the other. This would have produced a conflict which David presumably resolved in haste by picking the HEAD side of the merge instead of the MERGE_HEAD side (it happens). This could have been avoided by not cherry-picking fix commits around in the amdgpu process - instead having a fixes branch that is merged into the next branch after the fixes branch has been accepted upstream (that way there is only one commit for each fix and less conflicts). I have to deal with these sort of conflicts (sometimes daily) due to the drm processes. Its a pain as I have to track down each conflict to see if the same patches appear on both sides of merges and then try to figure out what other changes occur. (This is only slightly helped by have the "cherry-picked from" tags in the fix commits.) -- Cheers, Stephen Rothwell pgpMrldEaNsv1.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On Wed, 2020-08-05 at 17:27 -0400, Alex Deucher wrote: > On Wed, Aug 5, 2020 at 4:53 PM Joe Perches wrote: > > On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote: > > > On Wed, Aug 5, 2020 at 7:35 AM Colin King > > > wrote: > > > > From: Colin Ian King > > > > > > > > There is a spelling mistake in a DRM_ERROR message. Fix it. > > > > > > > > Signed-off-by: Colin Ian King > > > > > > This is already fixed. > > > > This fix is not in today's -next. > > > > Perhaps whatever tree it's fixed in should be in -next. > > > > Weird. It's in the drm-next tree as: > > commit 4afaa61db9cf5250b5734c2531b226e7b3a3d691 > Author: Colin Ian King > Date: Fri Jul 10 09:37:58 2020 +0100 > > drm/amdgpu: fix spelling mistake "Falied" -> "Failed" > > There is a spelling mistake in a DRM_ERROR error message. Fix it. > > Signed-off-by: Colin Ian King > Signed-off-by: Alex Deucher > > Alex > > > $ git show --oneline -s > > d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add > > linux-next specific files for 20200805 > > > > $ git grep -i falied drivers > > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied > > to terminate tmr\n"); > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > > [] > > > > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle) > > > > > > > > ret = psp_tmr_terminate(psp); > > > > if (ret) { > > > > - DRM_ERROR("Falied to terminate tmr\n"); > > > > + DRM_ERROR("Failed to terminate tmr\n"); > > > > return ret; > > > > } Dunno. Maybe it's due to some ordering of trees in how -next accumulates patches? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On 05/08/2020 22:27, Alex Deucher wrote: > On Wed, Aug 5, 2020 at 4:53 PM Joe Perches wrote: >> >> On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote: >>> On Wed, Aug 5, 2020 at 7:35 AM Colin King wrote: >>>> From: Colin Ian King >>>> >>>> There is a spelling mistake in a DRM_ERROR message. Fix it. >>>> >>>> Signed-off-by: Colin Ian King >>> >>> This is already fixed. >> >> This fix is not in today's -next. >> >> Perhaps whatever tree it's fixed in should be in -next. >> > > Weird. It's in the drm-next tree as: > > commit 4afaa61db9cf5250b5734c2531b226e7b3a3d691 > Author: Colin Ian King > Date: Fri Jul 10 09:37:58 2020 +0100 > > drm/amdgpu: fix spelling mistake "Falied" -> "Failed" > > There is a spelling mistake in a DRM_ERROR error message. Fix it. > > Signed-off-by: Colin Ian King > Signed-off-by: Alex Deucher > > Alex > >> >> $ git show --oneline -s >> d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add >> linux-next specific files for 20200805 >> >> $ git grep -i falied drivers >> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied to >> terminate tmr\n"); >> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c >> [] >>>> @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle) >>>> >>>> ret = psp_tmr_terminate(psp); >>>> if (ret) { >>>> - DRM_ERROR("Falied to terminate tmr\n"); >>>> + DRM_ERROR("Failed to terminate tmr\n"); >>>> return ret; >>>> } >> >> Somehow I omitted adding this to my tracking list and forgot that I sent this already. Apologies for patch deja-vu. Colin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On Wed, Aug 5, 2020 at 4:53 PM Joe Perches wrote: > > On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote: > > On Wed, Aug 5, 2020 at 7:35 AM Colin King wrote: > > > From: Colin Ian King > > > > > > There is a spelling mistake in a DRM_ERROR message. Fix it. > > > > > > Signed-off-by: Colin Ian King > > > > This is already fixed. > > This fix is not in today's -next. > > Perhaps whatever tree it's fixed in should be in -next. > Weird. It's in the drm-next tree as: commit 4afaa61db9cf5250b5734c2531b226e7b3a3d691 Author: Colin Ian King Date: Fri Jul 10 09:37:58 2020 +0100 drm/amdgpu: fix spelling mistake "Falied" -> "Failed" There is a spelling mistake in a DRM_ERROR error message. Fix it. Signed-off-by: Colin Ian King Signed-off-by: Alex Deucher Alex > > $ git show --oneline -s > d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add > linux-next specific files for 20200805 > > $ git grep -i falied drivers > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied to > terminate tmr\n"); > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > [] > > > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle) > > > > > > ret = psp_tmr_terminate(psp); > > > if (ret) { > > > - DRM_ERROR("Falied to terminate tmr\n"); > > > + DRM_ERROR("Failed to terminate tmr\n"); > > > return ret; > > > } > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/7] drm/amd/display: Use validated tiling_flags and tmz_surface in commit_tail
Reviewed-by: Rodrigo Siqueira On 07/30, Nicholas Kazlauskas wrote: > [Why] > So we're not racing with userspace or deadlocking DM. > > [How] > These flags are now stored on dm_plane_state itself and acquried and > validated during commit_check, so just use those instead. > > Cc: Daniel Vetter > Cc: Bhawanpreet Lakha > Cc: Rodrigo Siqueira > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 18 -- > 1 file changed, 4 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index f78c09c9585e..0d5f45742bb5 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -7094,8 +7094,6 @@ static void amdgpu_dm_commit_planes(struct > drm_atomic_state *state, > long r; > unsigned long flags; > struct amdgpu_bo *abo; > - uint64_t tiling_flags; > - bool tmz_surface = false; > uint32_t target_vblank, last_flip_vblank; > bool vrr_active = amdgpu_dm_vrr_active(acrtc_state); > bool pflip_present = false; > @@ -7179,20 +7177,12 @@ static void amdgpu_dm_commit_planes(struct > drm_atomic_state *state, > if (unlikely(r <= 0)) > DRM_ERROR("Waiting for fences timed out!"); > > - /* > - * We cannot reserve buffers here, which means the normal flag > - * access functions don't work. Paper over this with READ_ONCE, > - * but maybe the flags are invariant enough that not even that > - * would be needed. > - */ > - tiling_flags = READ_ONCE(abo->tiling_flags); > - tmz_surface = READ_ONCE(abo->flags) & > AMDGPU_GEM_CREATE_ENCRYPTED; > - > fill_dc_plane_info_and_addr( > - dm->adev, new_plane_state, tiling_flags, > + dm->adev, new_plane_state, > + dm_new_plane_state->tiling_flags, > >plane_infos[planes_count], > - >flip_addrs[planes_count].address, tmz_surface, > - false); > + >flip_addrs[planes_count].address, > + dm_new_plane_state->tmz_surface, false); > > DRM_DEBUG_DRIVER("plane: id=%d dcc_en=%d\n", >new_plane_state->plane->index, > -- > 2.25.1 > -- Rodrigo Siqueira https://siqueira.tech signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/7] drm/amd/display: Avoid using unvalidated tiling_flags and tmz_surface in prepare_planes
Reviewed-by: Rodrigo Siqueira On 07/30, Nicholas Kazlauskas wrote: > [Why] > We're racing with userspace as the flags could potentially change > from when we acquired and validated them in commit_check. > > [How] > We unfortunately can't drop this function in its entirety from > prepare_planes since we don't know the afb->address at commit_check > time yet. > > So instead of querying new tiling_flags and tmz_surface use the ones > from the plane_state directly. > > While we're at it, also update the force_disable_dcc option based > on the state from atomic check. > > Cc: Bhawanpreet Lakha > Cc: Rodrigo Siqueira > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 ++- > 1 file changed, 19 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index bf1881bd492c..f78c09c9585e 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -5794,14 +5794,8 @@ static int dm_plane_helper_prepare_fb(struct drm_plane > *plane, > struct list_head list; > struct ttm_validate_buffer tv; > struct ww_acquire_ctx ticket; > - uint64_t tiling_flags; > uint32_t domain; > int r; > - bool tmz_surface = false; > - bool force_disable_dcc = false; > - > - dm_plane_state_old = to_dm_plane_state(plane->state); > - dm_plane_state_new = to_dm_plane_state(new_state); > > if (!new_state->fb) { > DRM_DEBUG_DRIVER("No FB bound\n"); > @@ -5845,27 +5839,35 @@ static int dm_plane_helper_prepare_fb(struct > drm_plane *plane, > return r; > } > > - amdgpu_bo_get_tiling_flags(rbo, _flags); > - > - tmz_surface = amdgpu_bo_encrypted(rbo); > - > ttm_eu_backoff_reservation(, ); > > afb->address = amdgpu_bo_gpu_offset(rbo); > > amdgpu_bo_ref(rbo); > > + /** > + * We don't do surface updates on planes that have been newly created, > + * but we also don't have the afb->address during atomic check. > + * > + * Fill in buffer attributes depending on the address here, but only on > + * newly created planes since they're not being used by DC yet and this > + * won't modify global state. > + */ > + dm_plane_state_old = to_dm_plane_state(plane->state); > + dm_plane_state_new = to_dm_plane_state(new_state); > + > if (dm_plane_state_new->dc_state && > - dm_plane_state_old->dc_state != > dm_plane_state_new->dc_state) { > - struct dc_plane_state *plane_state = > dm_plane_state_new->dc_state; > + dm_plane_state_old->dc_state != dm_plane_state_new->dc_state) { > + struct dc_plane_state *plane_state = > + dm_plane_state_new->dc_state; > + bool force_disable_dcc = !plane_state->dcc.enable; > > - force_disable_dcc = adev->asic_type == CHIP_RAVEN && > adev->in_suspend; > fill_plane_buffer_attributes( > adev, afb, plane_state->format, plane_state->rotation, > - tiling_flags, _state->tiling_info, > - _state->plane_size, _state->dcc, > - _state->address, tmz_surface, > - force_disable_dcc); > + dm_plane_state_new->tiling_flags, > + _state->tiling_info, _state->plane_size, > + _state->dcc, _state->address, > + dm_plane_state_new->tmz_surface, force_disable_dcc); > } > > return 0; > -- > 2.25.1 > -- Rodrigo Siqueira https://siqueira.tech signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/7] drm/amd/display: Reset plane when tiling flags change
On 07/30, Nicholas Kazlauskas wrote: > [Why] > Enabling or disable DCC or switching between tiled and linear formats > can require bandwidth updates. > > They're currently skipping all DC validation by being treated as purely > surface updates. > > [How] > Treat tiling_flag changes (which encode DCC state) as a condition for > resetting the plane. > > Cc: Bhawanpreet Lakha > Cc: Rodrigo Siqueira > Cc: Hersen Wu > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 --- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 7cc5ab90ce13..bf1881bd492c 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -8332,6 +8332,8 @@ static bool should_reset_plane(struct drm_atomic_state > *state, >* TODO: Come up with a more elegant solution for this. >*/ > for_each_oldnew_plane_in_state(state, other, old_other_state, > new_other_state, i) { > + struct dm_plane_state *old_dm_plane_state, *new_dm_plane_state; > + > if (other->type == DRM_PLANE_TYPE_CURSOR) > continue; > > @@ -8342,9 +8344,20 @@ static bool should_reset_plane(struct drm_atomic_state > *state, > if (old_other_state->crtc != new_other_state->crtc) > return true; > > - /* TODO: Remove this once we can handle fast format changes. */ > - if (old_other_state->fb && new_other_state->fb && > - old_other_state->fb->format != new_other_state->fb->format) > + /* Framebuffer checks fall at the end. */ > + if (!old_other_state->fb || !new_other_state->fb) > + continue; > + > + /* Pixel format changes can require bandwidth updates. */ > + if (old_other_state->fb->format != new_other_state->fb->format) > + return true; > + > + old_dm_plane_state = to_dm_plane_state(old_other_state); > + new_dm_plane_state = to_dm_plane_state(new_other_state); > + > + /* Tiling and DCC changes also require bandwidth updates. */ > + if (old_dm_plane_state->tiling_flags != > + new_dm_plane_state->tiling_flags) Why not add a case when we move to a TMZ area? Reviewed-by: Rodrigo Siqueira > return true; > } > > -- > 2.25.1 > -- Rodrigo Siqueira https://siqueira.tech signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/amd/display: Store tiling_flags and tmz_surface on dm_plane_state
Reviewed-by: Rodrigo Siqueira On 07/30, Nicholas Kazlauskas wrote: > [Why] > Store these in advance so we can reuse them later in commit_tail without > having to reserve the fbo again. > > These will also be used for checking for tiling changes when deciding > to reset the plane or not. > > [How] > This change should mostly be a refactor. Only commit check is affected > for now and I'll drop the get_fb_info calls in prepare_planes and > commit_tail after. > > This runs a prepass loop once we think that all planes have been added > to the context and replaces the get_fb_info calls with accessing the > dm_plane_state instead. > > Cc: Bhawanpreet Lakha > Cc: Rodrigo Siqueira > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 60 +++ > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 + > 2 files changed, 37 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 8d64f5fde7e2..7cc5ab90ce13 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3700,8 +3700,17 @@ static int fill_dc_scaling_info(const struct > drm_plane_state *state, > static int get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb, > uint64_t *tiling_flags, bool *tmz_surface) > { > - struct amdgpu_bo *rbo = gem_to_amdgpu_bo(amdgpu_fb->base.obj[0]); > - int r = amdgpu_bo_reserve(rbo, false); > + struct amdgpu_bo *rbo; > + int r; > + > + if (!amdgpu_fb) { > + *tiling_flags = 0; > + *tmz_surface = false; > + return 0; > + } > + > + rbo = gem_to_amdgpu_bo(amdgpu_fb->base.obj[0]); > + r = amdgpu_bo_reserve(rbo, false); > > if (unlikely(r)) { > /* Don't show error message when returning -ERESTARTSYS */ > @@ -4124,13 +4133,10 @@ static int fill_dc_plane_attributes(struct > amdgpu_device *adev, > struct drm_crtc_state *crtc_state) > { > struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state); > - const struct amdgpu_framebuffer *amdgpu_fb = > - to_amdgpu_framebuffer(plane_state->fb); > + struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state); > struct dc_scaling_info scaling_info; > struct dc_plane_info plane_info; > - uint64_t tiling_flags; > int ret; > - bool tmz_surface = false; > bool force_disable_dcc = false; > > ret = fill_dc_scaling_info(plane_state, _info); > @@ -4142,15 +4148,12 @@ static int fill_dc_plane_attributes(struct > amdgpu_device *adev, > dc_plane_state->clip_rect = scaling_info.clip_rect; > dc_plane_state->scaling_quality = scaling_info.scaling_quality; > > - ret = get_fb_info(amdgpu_fb, _flags, _surface); > - if (ret) > - return ret; > - > force_disable_dcc = adev->asic_type == CHIP_RAVEN && adev->in_suspend; > - ret = fill_dc_plane_info_and_addr(adev, plane_state, tiling_flags, > + ret = fill_dc_plane_info_and_addr(adev, plane_state, > + dm_plane_state->tiling_flags, > _info, > _plane_state->address, > - tmz_surface, > + dm_plane_state->tmz_surface, > force_disable_dcc); > if (ret) > return ret; > @@ -5753,6 +5756,10 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane) > dc_plane_state_retain(dm_plane_state->dc_state); > } > > + /* Framebuffer hasn't been updated yet, so retain old flags. */ > + dm_plane_state->tiling_flags = old_dm_plane_state->tiling_flags; > + dm_plane_state->tmz_surface = old_dm_plane_state->tmz_surface; > + > return _plane_state->base; > } > > @@ -8557,13 +8564,9 @@ dm_determine_update_type_for_commit(struct > amdgpu_display_manager *dm, > continue; > > for_each_oldnew_plane_in_state(state, plane, old_plane_state, > new_plane_state, j) { > - const struct amdgpu_framebuffer *amdgpu_fb = > - to_amdgpu_framebuffer(new_plane_state->fb); > struct dc_plane_info *plane_info = > >plane_infos[num_plane]; > struct dc_flip_addrs *flip_addr = > >flip_addrs[num_plane]; > struct dc_scaling_info *scaling_info = > >scaling_infos[num_plane]; > - uint64_t tiling_flags; > - bool tmz_surface = false; > > new_plane_crtc = new_plane_state->crtc; > new_dm_plane_state = to_dm_plane_state(new_plane_state); > @@ -8610,16 +8613,12 @@
[PATCH] drm/gma500: fix error check
From: Tom Rix Reviewing this block of code in cdv_intel_dp_init() ret = cdv_intel_dp_aux_native_read(gma_encoder, DP_DPCD_REV, ... cdv_intel_edp_panel_vdd_off(gma_encoder); if (ret == 0) { /* if this fails, presume the device is a ghost */ DRM_INFO("failed to retrieve link info, disabling eDP\n"); drm_encoder_cleanup(encoder); cdv_intel_dp_destroy(connector); goto err_priv; } else { The (ret == 0) is not strict enough. cdv_intel_dp_aux_native_read() returns > 0 on success otherwise it is failure. So change to <= Fixes: d112a8163f83 ("gma500/cdv: Add eDP support") Signed-off-by: Tom Rix --- drivers/gpu/drm/gma500/cdv_intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c b/drivers/gpu/drm/gma500/cdv_intel_dp.c index f41cbb753bb4..720a767118c9 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c @@ -2078,7 +2078,7 @@ cdv_intel_dp_init(struct drm_device *dev, struct psb_intel_mode_device *mode_dev intel_dp->dpcd, sizeof(intel_dp->dpcd)); cdv_intel_edp_panel_vdd_off(gma_encoder); - if (ret == 0) { + if (ret <= 0) { /* if this fails, presume the device is a ghost */ DRM_INFO("failed to retrieve link info, disabling eDP\n"); drm_encoder_cleanup(encoder); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote: > On Wed, Aug 5, 2020 at 7:35 AM Colin King wrote: > > From: Colin Ian King > > > > There is a spelling mistake in a DRM_ERROR message. Fix it. > > > > Signed-off-by: Colin Ian King > > This is already fixed. This fix is not in today's -next. Perhaps whatever tree it's fixed in should be in -next. $ git show --oneline -s d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add linux-next specific files for 20200805 $ git grep -i falied drivers drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied to terminate tmr\n"); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c [] > > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle) > > > > ret = psp_tmr_terminate(psp); > > if (ret) { > > - DRM_ERROR("Falied to terminate tmr\n"); > > + DRM_ERROR("Failed to terminate tmr\n"); > > return ret; > > } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/7] drm/amd/display: Drop dm_determine_update_type_for_commit
Reviewed-by: Rodrigo Siqueira On 07/30, Nicholas Kazlauskas wrote: > [Why] > This was added in the past to solve the issue of not knowing when > to stall for medium and full updates in DM. > > Since DC is ultimately decides what requires bandwidth changes we > wanted to make use of it directly to determine this. > > The problem is that we can't actually pass any of the stream or surface > updates into DC global validation, so we don't actually check if the new > configuration is valid - we just validate the old existing config > instead and stall for outstanding commits to finish. > > There's also the problem of grabbing the DRM private object for > pageflips which can lead to page faults in the case where commits > execute out of order and free a DRM private object state that was > still required for commit tail. > > [How] > Now that we reset the plane in DM with the same conditions DC checks > we can have planes go through DC validation and we know when we need > to check and stall based on whether the stream or planes changed. > > We mark lock_and_validation_needed whenever we've done this, so just > go back to using that instead of dm_determine_update_type_for_commit. > > Since we'll skip resetting the plane for a pageflip we will no longer > grab the DRM private object for pageflips as well, avoiding the > page fault issued caused by pageflipping under load with commits > executing out of order. > > Cc: Harry Wentland > Cc: Bhawanpreet Lakha > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 199 ++ > 1 file changed, 17 insertions(+), 182 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 2cbb29199e61..59829ec81694 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -8542,161 +8542,6 @@ static int dm_update_plane_state(struct dc *dc, > return ret; > } > > -static int > -dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm, > - struct drm_atomic_state *state, > - enum surface_update_type *out_type) > -{ > - struct dc *dc = dm->dc; > - struct dm_atomic_state *dm_state = NULL, *old_dm_state = NULL; > - int i, j, num_plane, ret = 0; > - struct drm_plane_state *old_plane_state, *new_plane_state; > - struct dm_plane_state *new_dm_plane_state, *old_dm_plane_state; > - struct drm_crtc *new_plane_crtc; > - struct drm_plane *plane; > - > - struct drm_crtc *crtc; > - struct drm_crtc_state *new_crtc_state, *old_crtc_state; > - struct dm_crtc_state *new_dm_crtc_state, *old_dm_crtc_state; > - struct dc_stream_status *status = NULL; > - enum surface_update_type update_type = UPDATE_TYPE_FAST; > - struct surface_info_bundle { > - struct dc_surface_update surface_updates[MAX_SURFACES]; > - struct dc_plane_info plane_infos[MAX_SURFACES]; > - struct dc_scaling_info scaling_infos[MAX_SURFACES]; > - struct dc_flip_addrs flip_addrs[MAX_SURFACES]; > - struct dc_stream_update stream_update; > - } *bundle; > - > - bundle = kzalloc(sizeof(*bundle), GFP_KERNEL); > - > - if (!bundle) { > - DRM_ERROR("Failed to allocate update bundle\n"); > - /* Set type to FULL to avoid crashing in DC*/ > - update_type = UPDATE_TYPE_FULL; > - goto cleanup; > - } > - > - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > - > - memset(bundle, 0, sizeof(struct surface_info_bundle)); > - > - new_dm_crtc_state = to_dm_crtc_state(new_crtc_state); > - old_dm_crtc_state = to_dm_crtc_state(old_crtc_state); > - num_plane = 0; > - > - if (new_dm_crtc_state->stream != old_dm_crtc_state->stream) { > - update_type = UPDATE_TYPE_FULL; > - goto cleanup; > - } > - > - if (!new_dm_crtc_state->stream) > - continue; > - > - for_each_oldnew_plane_in_state(state, plane, old_plane_state, > new_plane_state, j) { > - struct dc_plane_info *plane_info = > >plane_infos[num_plane]; > - struct dc_flip_addrs *flip_addr = > >flip_addrs[num_plane]; > - struct dc_scaling_info *scaling_info = > >scaling_infos[num_plane]; > - > - new_plane_crtc = new_plane_state->crtc; > - new_dm_plane_state = to_dm_plane_state(new_plane_state); > - old_dm_plane_state = to_dm_plane_state(old_plane_state); > - > - if (plane->type == DRM_PLANE_TYPE_CURSOR) > - continue; > - > - if (new_dm_plane_state->dc_state != >
Re: [PATCH 5/7] drm/amd/display: Reset plane for anything that's not a FAST update
On 07/30, Nicholas Kazlauskas wrote: > [Why] > MEDIUM or FULL updates can require global validation or affect > bandwidth. By treating these all simply as surface updates we aren't > actually passing this through DC global validation. > > [How] > There's currently no way to pass surface updates through DC global > validation, nor do I think it's a good idea to change the interface > to accept these. > > DC global validation itself is currently stateless, and we can move > our update type checking to be stateless as well by duplicating DC > surface checks in DM based on DRM properties. > > We wanted to rely on DC automatically determining this since DC knows > best, but DM is ultimately what fills in everything into DC plane > state so it does need to know as well. > > There are basically only three paths that we exercise in DM today: > > 1) Cursor (async update) > 2) Pageflip (fast update) > 3) Full pipe programming (medium/full updates) > > Which means that anything that's more than a pageflip really needs to > go down path #3. > > So this change duplicates all the surface update checks based on DRM > state instead inside of should_reset_plane(). > > Next step is dropping dm_determine_update_type_for_commit and we no > longer require the old DC state at all for global validation. > > Optimization can come later so we don't reset DC planes at all for > MEDIUM udpates and avoid validation, but we might require some extra > checks in DM to achieve this. How about adding this optimization description in our TODO list under-display folder? Reviewed-by: Rodrigo Siqueira > Cc: Bhawanpreet Lakha > Cc: Hersen Wu > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 25 +++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 0d5f45742bb5..2cbb29199e61 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -8336,6 +8336,31 @@ static bool should_reset_plane(struct drm_atomic_state > *state, > if (old_other_state->crtc != new_other_state->crtc) > return true; > > + /* Src/dst size and scaling updates. */ > + if (old_other_state->src_w != new_other_state->src_w || > + old_other_state->src_h != new_other_state->src_h || > + old_other_state->crtc_w != new_other_state->crtc_w || > + old_other_state->crtc_h != new_other_state->crtc_h) > + return true; > + > + /* Rotation / mirroring updates. */ > + if (old_other_state->rotation != new_other_state->rotation) > + return true; > + > + /* Blending updates. */ > + if (old_other_state->pixel_blend_mode != > + new_other_state->pixel_blend_mode) > + return true; > + > + /* Alpha updates. */ > + if (old_other_state->alpha != new_other_state->alpha) > + return true; > + > + /* Colorspace changes. */ > + if (old_other_state->color_range != > new_other_state->color_range || > + old_other_state->color_encoding != > new_other_state->color_encoding) > + return true; > + > /* Framebuffer checks fall at the end. */ > if (!old_other_state->fb || !new_other_state->fb) > continue; > -- > 2.25.1 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CRodrigo.Siqueira%40amd.com%7Ccc095e7ce6164f529e2708d834c86d1b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637317382766607890sdata=omLC%2BizXVEjjGe6IylBpniZzyUGlzTATrgRoWEo6dHc%3Dreserved=0 -- Rodrigo Siqueira https://siqueira.tech signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/7] drm/amd/display: Replace DRM private objects with subclassed DRM atomic state
Hi, I have some minor inline comments, but everything looks fine when I tested it on Raven; feel free to add Tested-by: Rodrigo Siqueira in the whole series. On 07/30, Nicholas Kazlauskas wrote: > [Why] > DM atomic check was structured in a way that we required old DC state > in order to dynamically add and remove planes and streams from the > context to build the DC state context for validation. > > DRM private objects were used to carry over the last DC state and > were added to the context on nearly every commit - regardless of fast > or full so we could check whether or not the new state could affect > bandwidth. > > The problem with this model is that DRM private objects do not > implicitly stall out other commits. > > So if you have two commits touching separate DRM objects they could > run concurrently and potentially execute out of order - leading to a > use-after-free. > > If we want this to be safe we have two options: > 1. Stall out concurrent commits since they touch the same private object > 2. Refactor DM to not require old DC state and drop private object usage > > [How] > This implements approach #2 since it still allows for judder free > updates in multi-display scenarios. > > I'll list the big changes in order at a high level: > > 1. Subclass DRM atomic state instead of using DRM private objects. > > DC relied on the old state to determine which changes cause bandwidth > updates but now we have DM perform similar checks based on DRM state > instead - dropping the requirement for old state to exist at all. > > This means that we can now build a new DC context from scratch whenever > we have something that DM thinks could affect bandwidth. > > Whenever we need to rebuild bandwidth we now add all CRTCs and planes > to the DRM state in order to get the absolute set of DC streams and > DC planes. > > This introduces a stall on other commits, but this stall already > exists because of the lock_and_validation logic and it's necessary > since updates may move around pipes and require full reprogramming. > > 2. Drop workarounds to add planes to maintain z-order early in atomic > check. This is no longer needed because of the changes for (1). > > This also involves fixing up should_plane_reset checks since we can just > avoid resetting streams and planes when they haven't actually changed. > > 3. Rework dm_update_crtc_state and dm_update_plane_state to be single > pass instead of two pass. > > This is necessary since we no longer have the dc_state to add and > remove planes to the context in and we want to defer creation to the > end of commit_check. > > It also makes the logic a lot simpler to follow since as an added bonus. > > Cc: Bhawanpreet Lakha > Cc: Harry Wentland > Cc: Leo Li > Cc: Daniel Vetter > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 720 +++--- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 11 +- > 2 files changed, 280 insertions(+), 451 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 59829ec81694..97a7dfc620e8 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -1839,7 +1839,6 @@ static int dm_resume(void *handle) > struct drm_plane *plane; > struct drm_plane_state *new_plane_state; > struct dm_plane_state *dm_new_plane_state; > - struct dm_atomic_state *dm_state = > to_dm_atomic_state(dm->atomic_obj.state); > enum dc_connection_type new_connection_type = dc_connection_none; > struct dc_state *dc_state; > int i, r, j; > @@ -1879,11 +1878,6 @@ static int dm_resume(void *handle) > > return 0; > } > - /* Recreate dc_state - DC invalidates it when setting power state to > S3. */ > - dc_release_state(dm_state->context); > - dm_state->context = dc_create_state(dm->dc); > - /* TODO: Remove dc_state->dccg, use dc->dccg directly. */ > - dc_resource_state_construct(dm->dc, dm_state->context); > > /* Before powering on DC we need to re-initialize DMUB. */ > r = dm_dmub_hw_init(adev); > @@ -2019,11 +2013,51 @@ const struct amdgpu_ip_block_version dm_ip_block = > * *WIP* > */ > > +struct drm_atomic_state *dm_atomic_state_alloc(struct drm_device *dev) > +{ > + struct dm_atomic_state *dm_state; > + > + dm_state = kzalloc(sizeof(*dm_state), GFP_KERNEL); How about use GFP_ATOMIC here? > + > + if (!dm_state) > + return NULL; > + > + if (drm_atomic_state_init(dev, _state->base) < 0) { > + kfree(dm_state); > + return NULL; > + } > + > + return _state->base; > +} > + > +void dm_atomic_state_free(struct drm_atomic_state *state) > +{ > + struct dm_atomic_state *dm_state = to_dm_atomic_state(state); > + > + if (dm_state->context) { > +
Re: [PATCH v1 01/12] fbdev: gxfb: use generic power management
On Wed, Aug 05, 2020 at 11:37:11PM +0530, Vaibhav Gupta wrote: > Drivers using legacy power management .suspen()/.resume() callbacks > have to manage PCI states and device's PM states themselves. They also > need to take care of standard configuration registers. s/using legacy/using legacy PCI/ s/.suspen/.suspend/ (in all these patches) I wouldn't necessarily repost the whole series just for that (unless the maintainer wants it), but maybe update your branch so if you have occasion to repost for other reasons, this will be fixed. This particular driver actually doesn't *do* any of the PCI state or device PM state management you mention. And I don't see the "single 'struct dev_pm_ops'" you mention below -- I thought that meant you would have a single struct shared between drivers (I think you did that for IDE?), but that's not what you're doing. This driver has gxfb_pm_ops, the next has lxfb_pm_ops, etc. AFAICT the patches are fine, but the commit logs don't seem exactly accurate. > Switch to generic power management framework using a single > "struct dev_pm_ops" variable to take the unnecessary load from the driver. > This also avoids the need for the driver to directly call most of the PCI > helper functions and device power state control functions, as through > the generic framework PCI Core takes care of the necessary operations, > and drivers are required to do only device-specific jobs. > > Signed-off-by: Vaibhav Gupta > --- > drivers/video/fbdev/geode/gxfb.h | 5 > drivers/video/fbdev/geode/gxfb_core.c | 36 ++ > drivers/video/fbdev/geode/suspend_gx.c | 4 --- > 3 files changed, 20 insertions(+), 25 deletions(-) > > diff --git a/drivers/video/fbdev/geode/gxfb.h > b/drivers/video/fbdev/geode/gxfb.h > index d2e9c5c8e294..792c111c21e4 100644 > --- a/drivers/video/fbdev/geode/gxfb.h > +++ b/drivers/video/fbdev/geode/gxfb.h > @@ -21,7 +21,6 @@ struct gxfb_par { > void __iomem *dc_regs; > void __iomem *vid_regs; > void __iomem *gp_regs; > -#ifdef CONFIG_PM > int powered_down; > > /* register state, for power management functionality */ > @@ -36,7 +35,6 @@ struct gxfb_par { > uint64_t fp[FP_REG_COUNT]; > > uint32_t pal[DC_PAL_COUNT]; > -#endif > }; > > unsigned int gx_frame_buffer_size(void); > @@ -49,11 +47,8 @@ void gx_set_dclk_frequency(struct fb_info *info); > void gx_configure_display(struct fb_info *info); > int gx_blank_display(struct fb_info *info, int blank_mode); > > -#ifdef CONFIG_PM > int gx_powerdown(struct fb_info *info); > int gx_powerup(struct fb_info *info); > -#endif > - > > /* Graphics Processor registers (table 6-23 from the data book) */ > enum gp_registers { > diff --git a/drivers/video/fbdev/geode/gxfb_core.c > b/drivers/video/fbdev/geode/gxfb_core.c > index d38a148d4746..44089b331f91 100644 > --- a/drivers/video/fbdev/geode/gxfb_core.c > +++ b/drivers/video/fbdev/geode/gxfb_core.c > @@ -322,17 +322,14 @@ static struct fb_info *gxfb_init_fbinfo(struct device > *dev) > return info; > } > > -#ifdef CONFIG_PM > -static int gxfb_suspend(struct pci_dev *pdev, pm_message_t state) > +static int __maybe_unused gxfb_suspend(struct device *dev) > { > - struct fb_info *info = pci_get_drvdata(pdev); > + struct fb_info *info = dev_get_drvdata(dev); > > - if (state.event == PM_EVENT_SUSPEND) { > - console_lock(); > - gx_powerdown(info); > - fb_set_suspend(info, 1); > - console_unlock(); > - } > + console_lock(); > + gx_powerdown(info); > + fb_set_suspend(info, 1); > + console_unlock(); > > /* there's no point in setting PCI states; we emulate PCI, so >* we don't end up getting power savings anyways */ > @@ -340,9 +337,9 @@ static int gxfb_suspend(struct pci_dev *pdev, > pm_message_t state) > return 0; > } > > -static int gxfb_resume(struct pci_dev *pdev) > +static int __maybe_unused gxfb_resume(struct device *dev) > { > - struct fb_info *info = pci_get_drvdata(pdev); > + struct fb_info *info = dev_get_drvdata(dev); > int ret; > > console_lock(); > @@ -356,7 +353,6 @@ static int gxfb_resume(struct pci_dev *pdev) > console_unlock(); > return 0; > } > -#endif > > static int gxfb_probe(struct pci_dev *pdev, const struct pci_device_id *id) > { > @@ -467,15 +463,23 @@ static const struct pci_device_id gxfb_id_table[] = { > > MODULE_DEVICE_TABLE(pci, gxfb_id_table); > > +static const struct dev_pm_ops gxfb_pm_ops = { > +#ifdef CONFIG_PM_SLEEP > + .suspend= gxfb_suspend, > + .resume = gxfb_resume, > + .freeze = NULL, > + .thaw = gxfb_resume, > + .poweroff = NULL, > + .restore= gxfb_resume, > +#endif > +}; > + > static struct pci_driver gxfb_driver = { > .name = "gxfb", > .id_table = gxfb_id_table, > .probe = gxfb_probe, >
Re: [PATCH][next] drm/amdgpu: fix spelling mistake "paramter" -> "parameter"
On Wed, Aug 5, 2020 at 8:15 AM Colin King wrote: > > From: Colin Ian King > > There is a spelling mistake in a dev_warn message. Fix it. > > Signed-off-by: Colin Ian King Applied. Thanks! Alex > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index b72aeeb0a226..16e23f053361 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -1201,7 +1201,7 @@ static int amdgpu_device_check_arguments(struct > amdgpu_device *adev) > > if (amdgpu_num_kcq > 8 || amdgpu_num_kcq < 0) { > amdgpu_num_kcq = 8; > - dev_warn(adev->dev, "set kernel compute queue number to 8 due > to invalid paramter provided by user\n"); > + dev_warn(adev->dev, "set kernel compute queue number to 8 due > to invalid parameter provided by user\n"); > } > > return 0; > -- > 2.27.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On Wed, Aug 5, 2020 at 7:35 AM Colin King wrote: > > From: Colin Ian King > > There is a spelling mistake in a DRM_ERROR message. Fix it. > > Signed-off-by: Colin Ian King This is already fixed. Alex > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index 49d4514aa6ed..c68369731b20 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle) > > ret = psp_tmr_terminate(psp); > if (ret) { > - DRM_ERROR("Falied to terminate tmr\n"); > + DRM_ERROR("Failed to terminate tmr\n"); > return ret; > } > > -- > 2.27.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 208811] AMDGPU on-load null pointer dereference
https://bugzilla.kernel.org/show_bug.cgi?id=208811 --- Comment #10 from R0b0t1 (s...@aeam.us) --- Yep, seems obvious in retrospect. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/kmb: Add support for KeemBay Display
Hi Anitha. On Mon, Aug 03, 2020 at 09:02:24PM +, Chrisanthus, Anitha wrote: > Hi Sam, > I installed codespell, but the dictionary.txt in > usr/share/codespell/dictionary.txt > seems to be different from yours. Mine is version 1.8. Where can I get the > dictionary.txt > that you are using? I dunno. $ apt info codespell Package: codespell Version: 1.16.0-2 Priority: optional Section: universe/devel Origin: Ubuntu Maintainer: Ubuntu Developers Original-Maintainer: Debian Python Modules Team Bugs: https://bugs.launchpad.net/ubuntu/+filebug Installed-Size: 572 kB Depends: python3, python3-chardet, python3:any Homepage: https://github.com/codespell-project/codespell/ Download-Size: 118 kB APT-Manual-Installed: yes APT-Sources: http://dk.archive.ubuntu.com/ubuntu focal/universe amd64 Packages Description: Find and fix common misspellings in text files codespell is designed to find and fix common misspellings in text files. It is designed primarily for checking misspelled words in source code, but it can be used with other files as well. > I have corrected the relevant spelling warnings from your email and have sent > v5. The spelling mistakes was the least relevant warnings. Please see examples in the following. > > -:146: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open > > parenthesis > > #146: FILE: drivers/gpu/drm/kmb/kmb_crtc.c:58: > > + kmb_clr_bitmask_lcd(kmb, LCD_INT_ENABLE, > > + LCD_INT_VERT_COMP); Here we want LCD_INT_VERT_COMP to be aligned right after the opening '('. It must be indented with a number of tabs followed by the necessary spaces to achive this indent. Always uses tabs for indent if possible. So in other words 8 spaces are not OK, then use a tab. Same goes for similar warnings. > > -:427: CHECK:LINE_SPACING: Please don't use multiple blank lines > > #427: FILE: drivers/gpu/drm/kmb/kmb_drv.c:74: > > + > > + > > Do not use two consecutive blank lines. > > -:463: CHECK:SPACING: spaces preferred around that '/' (ctx:VxV) > > #463: FILE: drivers/gpu/drm/kmb/kmb_drv.c:110: > > + kmb->sys_clk_mhz = clk_get_rate(kmb_clk.clk_pll0)/100; > > ^ Spaces around all operatoers - so space before and after '/' here. Same goes for following warnings of the same type. > > -:688: CHECK:BRACES: Blank lines aren't necessary after an open brace '{' > > #688: FILE: drivers/gpu/drm/kmb/kmb_drv.c:335: > > + if (status & LCD_INT_EOF) { > > + As the warning says - no empty line after opening '{' > > > > -:701: CHECK:CAMELCASE: Avoid CamelCase: > > #701: FILE: drivers/gpu/drm/kmb/kmb_drv.c:348: > > + LCD_LAYERn_DMA_CFG > > If you have a reason to use CamelCase then this can be ignored. A good reason could be that this is how it is done in the datasheet. In this case maybe use LCD_LAYER_N_DMA_CFG or similar. > > -:957: CHECK:BRACES: braces {} should be used on all arms of this statement > > #957: FILE: drivers/gpu/drm/kmb/kmb_drv.c:604: > > + if (adv_bridge == ERR_PTR(-EPROBE_DEFER)) > > [...] > > + else if (IS_ERR(adv_bridge)) { > > [...] If we use {} in one arm of the statement use it in all arms. This, as the other tidbits, improve readability. Same for all similar warnings. > > -:1026: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string > > "intel,kmb_display" appears un-documented -- check > > ./Documentation/devicetree/bindings/ > > #1026: FILE: drivers/gpu/drm/kmb/kmb_drv.c:673: > > + {.compatible = "intel,kmb_display"}, Binding is missing - we cannot apply a driver for an unknown binding. The binding must be in DT-schema (yaml) format. > > > > -:1122: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without > > comment > > #1122: FILE: drivers/gpu/drm/kmb/kmb_drv.h:35: > > + spinlock_t irq_lock; Add comment. And consider a more specific name like kmb_irq_lock - allows for easier grepping. > > > > -:1360: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u16' over 'uint16_t' > > #1360: FILE: drivers/gpu/drm/kmb/kmb_dsi.c:95: > > + uint16_t default_bit_rate_mbps; As the warning says. This goes again later. > > -:1947: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written > > "fg_cfg->sections[i]" > > #1947: FILE: drivers/gpu/drm/kmb/kmb_dsi.c:682: > > + if (fg_cfg->sections[i] != NULL) Hmm, I like the current code. But better please checkpatch here. I did not go through them all. The point is that all the warnings from checkpatch should be considered, and for the most of them they are legit and should be fixed. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 208811] AMDGPU on-load null pointer dereference
https://bugzilla.kernel.org/show_bug.cgi?id=208811 --- Comment #9 from Alex Deucher (alexdeuc...@gmail.com) --- Does disabling memory encryption fix the issue? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 208811] AMDGPU on-load null pointer dereference
https://bugzilla.kernel.org/show_bug.cgi?id=208811 --- Comment #8 from Alex Deucher (alexdeuc...@gmail.com) --- (In reply to R0b0t1 from comment #7) > Related Bug 204181, related > https://bugzilla.redhat.com/show_bug.cgi?id=1851855. Those are unrelated. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 208811] AMDGPU on-load null pointer dereference
https://bugzilla.kernel.org/show_bug.cgi?id=208811 --- Comment #7 from R0b0t1 (s...@aeam.us) --- Related Bug 204181, related https://bugzilla.redhat.com/show_bug.cgi?id=1851855. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [2/2] drm/ttm: make TT creation purely optional v3
On 2020-06-29 5:19 p.m., Christian König wrote: > We only need the page array when the BO is about to be accessed. > > So not only populate, but also create it on demand. > > v2: move NULL check into ttm_tt_create() > v3: fix the occurrence in ttm_bo_kmap_ttm as well This broke amdgpu userptr functionality for me, in particular all GL_AMD_pinned_memory piglit tests, e.g. .../piglit/bin/amd_pinned_memory decrement-offset -auto -fbo Offset is decremented, no wait. Mesa: User error: GL_INVALID_OPERATION in glBufferData [...] Unexpected GL error: GL_INVALID_OPERATION 0x502 (Error at tests/spec/amd_pinned_memory/test.c:214) PIGLIT: {"result": "fail" } -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207383] [Regression] 5.7 amdgpu/polaris11 gpf: amdgpu_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=207383 Duncan (1i5t5.dun...@cox.net) changed: What|Removed |Added Status|NEW |RESOLVED Kernel Version|5.7-rc1 - 5.7 - 5.8-rc5+|5.7-rc1 - 5.7 - 5.8-rc5+, ||fixed in 5.8.0, 5.7.13, ||5.4.56 Resolution|--- |CODE_FIX --- Comment #116 from Duncan (1i5t5.dun...@cox.net) --- For those not on 5.8 yet, Mazin's patch is in the 5.7.13 stable and 5.4.56 LTS releases. As far as I'm concerned (and lacking any NAKs to my previous question about closing) there's no further reason to leave the bug open so I'm closing. The bugzilla.kernel.org installation has some confusing custom resolution choices that haven't been documented in the status help link (bug #13851, filed years ago as implied by the bug number compared to this one) and I don't know whether CODE_FIX or PATCH_ALREADY_AVAILABLE is more appropriate as both seem to apply equally, so I guess I'll leave it at the default CODE_FIX. Thanks again to everyone who confirmed the bug and/or worked on fixes and testing. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v1] drm/msm/dpu: Fix reservation failures in modeset
On Wed, Aug 5, 2020 at 6:34 AM Kalyan Thota wrote: > > In TEST_ONLY commit, rm global_state will duplicate the > object and request for new reservations, once they pass > then the new state will be swapped with the old and will > be available for the Atomic Commit. > > This patch fixes some of missing links in the resource > reservation sequence mentioned above. > > 1) Creation of a duplicate state in test_only commit (Rob) > 2) Allow resource release only during crtc_active false. > > For #2 > In a modeset operation, swap state happens well before disable. > Hence clearing reservations in disable will cause failures > in modeset enable. > > Sequence: > Swap state --> old, new > modeset disables --> virt disable > modeset enable --> virt modeset > > Allow reservations to be cleared only when crtc active is false > as in that case there wont be any modeset enable after disable. > > Signed-off-by: Kalyan Thota > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 63976dc..b85a576 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -582,7 +582,7 @@ static int dpu_encoder_virt_atomic_check( > dpu_kms = to_dpu_kms(priv->kms); > mode = _state->mode; > adj_mode = _state->adjusted_mode; > - global_state = dpu_kms_get_existing_global_state(dpu_kms); > + global_state = dpu_kms_get_global_state(crtc_state->state); > trace_dpu_enc_atomic_check(DRMID(drm_enc)); > > /* > @@ -1172,6 +1172,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder > *drm_enc) > struct msm_drm_private *priv; > struct dpu_kms *dpu_kms; > struct dpu_global_state *global_state; > + struct drm_crtc_state *crtc_state; > int i = 0; > > if (!drm_enc) { > @@ -1191,6 +1192,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder > *drm_enc) > priv = drm_enc->dev->dev_private; > dpu_kms = to_dpu_kms(priv->kms); > global_state = dpu_kms_get_existing_global_state(dpu_kms); > + crtc_state = drm_enc->crtc->state; > > trace_dpu_enc_disable(DRMID(drm_enc)); > > @@ -1220,7 +1222,8 @@ static void dpu_encoder_virt_disable(struct drm_encoder > *drm_enc) > > DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n"); > > - dpu_rm_release(global_state, drm_enc); > + if (crtc_state->active_changed && !crtc_state->active) > + dpu_rm_release(global_state, drm_enc); I still think releasing the state in the atomic_commit() path is the wrong thing to do. In the commit path, the various state objects should be immutable.. ie. in the atomic_test() path you derive the new hw state (including assignment/release of resources), and atomic_commit() is simply pushing the state down to the hw. Otherwise, this looks better than v1. BR, -R > > mutex_unlock(_enc->enc_lock); > } > -- > 1.9.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/omap: fix spelling mistake "propert" -> "property"
On 05/08/2020 13:20, Colin King wrote: > From: Colin Ian King > > There is a spelling mistake in a pr_err message. Fix it. > > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/omapdrm/dss/venc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c > b/drivers/gpu/drm/omapdrm/dss/venc.c > index e0817934ee16..0fa8f677b647 100644 > --- a/drivers/gpu/drm/omapdrm/dss/venc.c > +++ b/drivers/gpu/drm/omapdrm/dss/venc.c > @@ -781,7 +781,7 @@ static int venc_probe_of(struct venc_device *venc) > venc->type = OMAP_DSS_VENC_TYPE_SVIDEO; > break; > default: > - dev_err(>pdev->dev, "bad channel propert '%d'\n", > + dev_err(>pdev->dev, "bad channel property '%d'\n", > channels); > r = -EINVAL; > goto err; > Thanks, I'll pick this up. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] video: fbdev: sis: fix null ptr dereference
From: Tom Rix Clang static analysis reports this representative error init.c:2501:18: warning: Array access (from variable 'queuedata') results in a null pointer dereference templ |= ((queuedata[i] & 0xc0) << 3); This is the problem block of code if(ModeNo > 0x13) { ... if(SiS_Pr->ChipType == SIS_730) { queuedata = [0]; } else { queuedata = [0]; } } else { } queuedata is not set in the else block Reviewing the old code, the arrays FQBQData730 and FQBQData were used directly. So hoist the setting of queuedata out of the if-else block. Fixes: 544393fe584d ("[PATCH] sisfb update") Signed-off-by: Tom Rix --- drivers/video/fbdev/sis/init.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/video/fbdev/sis/init.c b/drivers/video/fbdev/sis/init.c index dfe3eb769638..fde27feae5d0 100644 --- a/drivers/video/fbdev/sis/init.c +++ b/drivers/video/fbdev/sis/init.c @@ -2428,6 +2428,11 @@ SiS_SetCRT1FIFO_630(struct SiS_Private *SiS_Pr, unsigned short ModeNo, i = 0; + if (SiS_Pr->ChipType == SIS_730) + queuedata = [0]; + else + queuedata = [0]; + if(ModeNo > 0x13) { /* Get VCLK */ @@ -2445,12 +2450,6 @@ SiS_SetCRT1FIFO_630(struct SiS_Private *SiS_Pr, unsigned short ModeNo, /* Get half colordepth */ colorth = colortharray[(SiS_Pr->SiS_ModeType - ModeEGA)]; - if(SiS_Pr->ChipType == SIS_730) { -queuedata = [0]; - } else { -queuedata = [0]; - } - do { templ = SiS_CalcDelay2(SiS_Pr, queuedata[i]) * VCLK * colorth; -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/mcde: Fix display pipeline restart
To make sure that the MCDE is in a reasonable state during set-up, perform a reset by power cycling the block by dropping the on-chip regulator reference after probe. The display subsystem (DSS) has no dedicated reset line so dropping the EPOD regulator is the only real way of resetting it. We introduce code to enable and disable the regulator in the display enable/disable callbacks. When we drop the power to the whole display subsystem, not only MCDE but also the DSI links lose their state. Therefore we move the DSI block reset and hardware initialization code to the mcde_dsi_bridge_pre_enable() callback so this happens every time we start up the bridge, as we may have lost the power. We move the final disablement of the interrupts and clocks to the mcde_dsi_bridge_post_disable() callback rather than have it in the mcde_dsi_bridge_disable() callback, as some control messages may still be sent over the DSI host after the bridge has been shut down. This (together with a patch for the corresponding panel) makes the Samsung GT-S7710 successfully disable and re-enable its display, cutting all power while disabled and re-initializing the hardware when coming back up. Cc: Stephan Gerhold Signed-off-by: Linus Walleij --- drivers/gpu/drm/mcde/mcde_display.c | 16 ++ drivers/gpu/drm/mcde/mcde_drv.c | 14 + drivers/gpu/drm/mcde/mcde_dsi.c | 48 + 3 files changed, 59 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c index 897d29e318d3..99d33fde3293 100644 --- a/drivers/gpu/drm/mcde/mcde_display.c +++ b/drivers/gpu/drm/mcde/mcde_display.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -873,6 +874,14 @@ static void mcde_display_enable(struct drm_simple_display_pipe *pipe, u32 formatter_frame; u32 pkt_div; u32 val; + int ret; + + /* This powers up the entire MCDE block and the DSI hardware */ + ret = regulator_enable(mcde->epod); + if (ret) { + dev_err(drm->dev, "can't re-enable EPOD regulator\n"); + return; + } dev_info(drm->dev, "enable MCDE, %d x %d format %s\n", mode->hdisplay, mode->vdisplay, @@ -1008,6 +1017,7 @@ static void mcde_display_disable(struct drm_simple_display_pipe *pipe) struct drm_device *drm = crtc->dev; struct mcde *mcde = to_mcde(drm); struct drm_pending_vblank_event *event; + int ret; drm_crtc_vblank_off(crtc); @@ -1023,6 +1033,12 @@ static void mcde_display_disable(struct drm_simple_display_pipe *pipe) spin_unlock_irq(>dev->event_lock); } + ret = regulator_disable(mcde->epod); + if (ret) + dev_err(drm->dev, "can't disable EPOD regulator\n"); + /* Make sure we are powered down (before we may power up again) */ + usleep_range(1000, 1500); + dev_info(drm->dev, "MCDE display is disabled\n"); } diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c index 1671af101cf2..ad90a2c2467f 100644 --- a/drivers/gpu/drm/mcde/mcde_drv.c +++ b/drivers/gpu/drm/mcde/mcde_drv.c @@ -63,6 +63,7 @@ #include #include #include +#include #include #include @@ -468,6 +469,19 @@ static int mcde_probe(struct platform_device *pdev) dev_err(dev, "failed to add component master\n"); goto clk_disable; } + + /* +* Perform an invasive reset of the MCDE and all blocks by +* cutting the power to the subsystem, then bring it back up +* later when we enable the display. +*/ + ret = regulator_disable(mcde->epod); + if (ret) { + dev_err(dev, "can't disable EPOD regulator\n"); + return ret; + } + usleep_range(1000, 1500); + return 0; clk_disable: diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c index 9063a5f5e8bf..92b1d037ed1c 100644 --- a/drivers/gpu/drm/mcde/mcde_dsi.c +++ b/drivers/gpu/drm/mcde/mcde_dsi.c @@ -902,6 +902,21 @@ static void mcde_dsi_bridge_pre_enable(struct drm_bridge *bridge) dev_info(d->dev, "DSI HS clock rate %lu Hz\n", d->hs_freq); + /* Assert RESET through the PRCMU, active low */ + /* FIXME: which DSI block? */ + regmap_update_bits(d->prcmu, PRCM_DSI_SW_RESET, + PRCM_DSI_SW_RESET_DSI0_SW_RESETN, 0); + + usleep_range(100, 200); + + /* De-assert RESET again */ + regmap_update_bits(d->prcmu, PRCM_DSI_SW_RESET, + PRCM_DSI_SW_RESET_DSI0_SW_RESETN, + PRCM_DSI_SW_RESET_DSI0_SW_RESETN); + + /* Start up the hardware */ + mcde_dsi_start(d); + if (d->mdsi->mode_flags & MIPI_DSI_MODE_VIDEO) { /* Put IF1 into video mode */ val =
Re: [Intel-gfx] [PATCH 6/6] drm/xen-front: Add support for EDID based configuration
Hi Oleksandr, I love your patch! Perhaps something to improve: [auto build test WARNING on drm-exynos/exynos-drm-next] [also build test WARNING on drm-intel/for-linux-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.8 next-20200804] [cannot apply to xen-tip/linux-next drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Oleksandr-Andrushchenko/Fixes-and-improvements-for-Xen-pvdrm/20200731-205350 base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next compiler: aarch64-linux-gcc (GCC) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot cppcheck warnings: (new ones prefixed by >>) >> drivers/irqchip/irq-gic.c:161:24: warning: Local variable gic_data shadows >> outer variable [shadowVar] struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d); ^ drivers/irqchip/irq-gic.c:123:29: note: Shadowed declaration static struct gic_chip_data gic_data[CONFIG_ARM_GIC_MAX_NR] __read_mostly; ^ drivers/irqchip/irq-gic.c:161:24: note: Shadow variable struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d); ^ drivers/irqchip/irq-gic.c:167:24: warning: Local variable gic_data shadows outer variable [shadowVar] struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d); ^ drivers/irqchip/irq-gic.c:123:29: note: Shadowed declaration static struct gic_chip_data gic_data[CONFIG_ARM_GIC_MAX_NR] __read_mostly; ^ drivers/irqchip/irq-gic.c:167:24: note: Shadow variable struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d); ^ >> drivers/irqchip/irq-gic.c:400:28: warning: Local variable gic_irq shadows >> outer function [shadowFunction] unsigned int cascade_irq, gic_irq; ^ drivers/irqchip/irq-gic.c:171:28: note: Shadowed declaration static inline unsigned int gic_irq(struct irq_data *d) ^ drivers/irqchip/irq-gic.c:400:28: note: Shadow variable unsigned int cascade_irq, gic_irq; ^ >> drivers/irqchip/irq-gic.c:1507:14: warning: Local variable gic_cpu_base >> shadows outer function [shadowFunction] phys_addr_t gic_cpu_base; ^ drivers/irqchip/irq-gic.c:165:29: note: Shadowed declaration static inline void __iomem *gic_cpu_base(struct irq_data *d) ^ drivers/irqchip/irq-gic.c:1507:14: note: Shadow variable phys_addr_t gic_cpu_base; ^ >> drivers/irqchip/irq-gic-v3.c:874:71: warning: Boolean result is used in >> bitwise operation. Clarify expression with parentheses. [clarifyCondition] gic_data.rdists.has_direct_lpi &= (!!(typer & GICR_TYPER_DirectLPIS) | ^ >> drivers/irqchip/irq-gic-v3.c:1808:6: warning: Local variable >> nr_redist_regions shadows outer variable [shadowVar] u32 nr_redist_regions; ^ drivers/irqchip/irq-gic-v3.c:1880:6: note: Shadowed declaration u32 nr_redist_regions; ^ drivers/irqchip/irq-gic-v3.c:1808:6: note: Shadow variable u32 nr_redist_regions; ^ >> drivers/irqchip/irq-gic-v3.c:2042:6: warning: Local variable maint_irq_mode >> shadows outer variable [shadowVar] int maint_irq_mode; ^ drivers/irqchip/irq-gic-v3.c:1884:6: note: Shadowed declaration int maint_irq_mode; ^ drivers/irqchip/irq-gic-v3.c:2042:6: note: Shadow variable int maint_irq_mode; ^ >> drivers/gpu/drm/xen/xen_drm_front_cfg.c:76:6: warning: Variable 'ret' is >> reassigned a value before the old one has been used. [redundantAssignment] ret = xen_drm_front_get_edid(front_info, index, pages, ^ drivers/gpu/drm/xen/xen_drm_front_cfg.c:61:0: note: Variable 'ret' is reassigned a value before the old one has been used. int i, npages, ret = -ENOMEM; ^ drivers/gpu/drm/xen/xen_drm_front_cfg.c:76:6: note: Variable 'ret' is reassigned a value before the old one has been used. ret = xen_drm_front_get_edid(front_info, index, pages, ^ vim +/ret +76 drivers/gpu/drm/xen/xen_drm_front_cfg.c 54 55 static void cfg_connector_edid(struct xen_drm_front_info *front_info, 56 struct xen_drm_front_cfg_connector *connector, 57 int index) 58 { 59 struct page **pages; 60 u32 edid_sz; 61 int i, npages, ret = -ENOMEM; 62 63 connector->edid = vmalloc(XENDISPL_EDID_MAX_SIZE); 64 if (!connector->edid) 65
[PATCH] drm/panel-notatek-nt35510: Fix enable/disable sequence
The driver was relying on only prepare()/unprepare() to enable/disable the display. This does not work because prepare() will be called before the DSI host/bridge is ready to send any DSI commands and disable() will be called after the DSI host/bridge is disabled and thus unable to send any DSI commands. Move all DCS command sending to the enable() and disable() callbacks, as is proper. The prepare() and unprepare() is now only used to enable and disable the regulators() and asserting/de-asserting the reset line so we inline the regulator and reset code here. While developing this it was discovered that during powercycling the device looses its ability to read the MTP ID:s. We were lucky before as the display came up with MTP reading enabled, but as this makes powercycling work, we also need to send two small sequences that enable the reading of the MTP ID after powering on the regulators. This makes it possible to use the user space UI stack Phosh on the Samsung GT-S7710 as Phosh enables/disables the display when starting. Rename the nt35510_power_on() function to nt35510_power_on_sequence() to reflect the fact that this is not really about regulators but a DCS command sequence. Cc: Stephan Gerhold Signed-off-by: Linus Walleij --- drivers/gpu/drm/panel/panel-novatek-nt35510.c | 107 +++--- 1 file changed, 68 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c index 4a8fa908a2cf..39113601e944 100644 --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c @@ -376,6 +376,10 @@ struct nt35510 { }; /* Manufacturer command has strictly this byte sequence */ +static const u8 nt35510_mauc_mtp_read_param[] = { 0xAA, 0x55, 0x25, 0x01 }; +static const u8 nt35510_mauc_mtp_read_setting[] = { 0x01, 0x02, 0x00, 0x20, + 0x33, 0x13, 0x00, 0x40, + 0x00, 0x00, 0x23, 0x02 }; static const u8 nt35510_mauc_select_page_0[] = { 0x55, 0xAA, 0x52, 0x08, 0x00 }; static const u8 nt35510_mauc_select_page_1[] = { 0x55, 0xAA, 0x52, 0x08, 0x01 }; static const u8 nt35510_vgh_on[] = { 0x01 }; @@ -674,29 +678,22 @@ static const struct backlight_ops nt35510_bl_ops = { /* * This power-on sequence */ -static int nt35510_power_on(struct nt35510 *nt) +static int nt35510_power_on_sequence(struct nt35510 *nt) { struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev); int ret; - ret = regulator_bulk_enable(ARRAY_SIZE(nt->supplies), nt->supplies); - if (ret < 0) { - dev_err(nt->dev, "unable to enable regulators\n"); + ret = nt35510_send_long(nt, dsi, MCS_CMD_MTP_READ_PARAM, + ARRAY_SIZE(nt35510_mauc_mtp_read_param), + nt35510_mauc_mtp_read_param); + if (ret) return ret; - } - /* Toggle RESET in accordance with datasheet page 370 */ - if (nt->reset_gpio) { - gpiod_set_value(nt->reset_gpio, 1); - /* Active min 10 us according to datasheet, let's say 20 */ - usleep_range(20, 1000); - gpiod_set_value(nt->reset_gpio, 0); - /* -* 5 ms during sleep mode, 120 ms during sleep out mode -* according to datasheet, let's use 120-140 ms. -*/ - usleep_range(12, 14); - } + ret = nt35510_send_long(nt, dsi, MCS_CMD_MTP_READ_SETTING, + ARRAY_SIZE(nt35510_mauc_mtp_read_setting), + nt35510_mauc_mtp_read_setting); + if (ret) + return ret; ret = nt35510_read_id(nt); if (ret) @@ -758,26 +755,16 @@ static int nt35510_power_on(struct nt35510 *nt) return 0; } -static int nt35510_power_off(struct nt35510 *nt) -{ - int ret; - - ret = regulator_bulk_disable(ARRAY_SIZE(nt->supplies), nt->supplies); - if (ret) - return ret; - - if (nt->reset_gpio) - gpiod_set_value(nt->reset_gpio, 1); - - return 0; -} - -static int nt35510_unprepare(struct drm_panel *panel) +static int nt35510_disable(struct drm_panel *panel) { struct nt35510 *nt = panel_to_nt35510(panel); struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev); int ret; + /* +* The final DCS traffic must happen here, at unprepare() the +* DSI host will not accept traffic any more. +*/ ret = mipi_dsi_dcs_set_display_off(dsi); if (ret) { DRM_DEV_ERROR(nt->dev, "failed to turn display off (%d)\n", @@ -797,20 +784,36 @@ static int nt35510_unprepare(struct drm_panel *panel) /* Wait 4 frames, how much is that 5ms in the vendor driver */ usleep_range(5000, 1); - ret =
Re: [PATCH v5 1/5] drm/i915: Add enable/disable flip done and flip done handler
On 7/28/2020 3:04 AM, Daniel Vetter wrote: On Mon, Jul 27, 2020 at 2:27 PM Michel Dänzer wrote: On 2020-07-25 1:26 a.m., Paulo Zanoni wrote: Em seg, 2020-07-20 às 17:01 +0530, Karthik B S escreveu: diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1fa67700d8f4..95953b393941 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -697,14 +697,24 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc) return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xff; } +static u32 g4x_get_flip_counter(struct drm_crtc *crtc) +{ +struct drm_i915_private *dev_priv = to_i915(crtc->dev); +enum pipe pipe = to_intel_crtc(crtc)->pipe; + +return I915_READ(PIPE_FLIPCOUNT_G4X(pipe)); +} + u32 g4x_get_vblank_counter(struct drm_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->dev); enum pipe pipe = to_intel_crtc(crtc)->pipe; +if (crtc->state->async_flip) +return g4x_get_flip_counter(crtc); + return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); I don't understand the intention behind this, can you please clarify? This goes back to my reply of the cover letter. It seems that here we're going to alternate between two different counters in our vblank count. So if user space alternates between sometimes using async flips and sometimes using normal flip it's going to get some very weird deltas, isn't it? At least this is what I remember from when I played with these registers: FLIPCOUNT drifts away from FRMCOUNT when we start using async flips. This definitely looks wrong. The counter value returned by the get_vblank_counter hook is supposed to increment when a vertical blank period occurs; page flips are not supposed to affect this in any way. Also you just flat out can't access crtc->state from interrupt context. Anything you need in there needs to be protected by the right irq-type spin_lock, updates correctly synchronized against both the interrupt handler and atomic updates, and data copied over, not pointers. Otherwise just crash Thanks for the review. I will be removing this change in the next revision based on the feedback received, but I will keep this in mind whenever I'll have to access something from the interrupt context. Thanks, Karthik.B.S -Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/5] drm/i915: Add enable/disable flip done and flip done handler
On 7/27/2020 5:57 PM, Michel Dänzer wrote: On 2020-07-25 1:26 a.m., Paulo Zanoni wrote: Em seg, 2020-07-20 às 17:01 +0530, Karthik B S escreveu: diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1fa67700d8f4..95953b393941 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -697,14 +697,24 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc) return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xff; } +static u32 g4x_get_flip_counter(struct drm_crtc *crtc) +{ + struct drm_i915_private *dev_priv = to_i915(crtc->dev); + enum pipe pipe = to_intel_crtc(crtc)->pipe; + + return I915_READ(PIPE_FLIPCOUNT_G4X(pipe)); +} + u32 g4x_get_vblank_counter(struct drm_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->dev); enum pipe pipe = to_intel_crtc(crtc)->pipe; + if (crtc->state->async_flip) + return g4x_get_flip_counter(crtc); + return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); I don't understand the intention behind this, can you please clarify? This goes back to my reply of the cover letter. It seems that here we're going to alternate between two different counters in our vblank count. So if user space alternates between sometimes using async flips and sometimes using normal flip it's going to get some very weird deltas, isn't it? At least this is what I remember from when I played with these registers: FLIPCOUNT drifts away from FRMCOUNT when we start using async flips. This definitely looks wrong. The counter value returned by the get_vblank_counter hook is supposed to increment when a vertical blank period occurs; page flips are not supposed to affect this in any way. Thanks for the review. As per the feedback received, I will be removing this and will revert back to the original implementation in the next revision. Thanks, Karthik.B.S ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 34/38] drm/virtio: convert to LE accessors
Virtgpu is modern-only. Use LE accessors for config space. Signed-off-by: Michael S. Tsirkin --- drivers/gpu/drm/virtio/virtgpu_kms.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 0a5c8cf409fb..4d944a0dff3e 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -39,8 +39,8 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work) u32 events_read, events_clear = 0; /* read the config space */ - virtio_cread(vgdev->vdev, struct virtio_gpu_config, -events_read, _read); + virtio_cread_le(vgdev->vdev, struct virtio_gpu_config, + events_read, _read); if (events_read & VIRTIO_GPU_EVENT_DISPLAY) { if (vgdev->has_edid) virtio_gpu_cmd_get_edids(vgdev); @@ -49,8 +49,8 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work) drm_helper_hpd_irq_event(vgdev->ddev); events_clear |= VIRTIO_GPU_EVENT_DISPLAY; } - virtio_cwrite(vgdev->vdev, struct virtio_gpu_config, - events_clear, _clear); + virtio_cwrite_le(vgdev->vdev, struct virtio_gpu_config, +events_clear, _clear); } static void virtio_gpu_init_vq(struct virtio_gpu_queue *vgvq, @@ -165,8 +165,8 @@ int virtio_gpu_init(struct drm_device *dev) } /* get display info */ - virtio_cread(vgdev->vdev, struct virtio_gpu_config, -num_scanouts, _scanouts); + virtio_cread_le(vgdev->vdev, struct virtio_gpu_config, + num_scanouts, _scanouts); vgdev->num_scanouts = min_t(uint32_t, num_scanouts, VIRTIO_GPU_MAX_SCANOUTS); if (!vgdev->num_scanouts) { @@ -176,8 +176,8 @@ int virtio_gpu_init(struct drm_device *dev) } DRM_INFO("number of scanouts: %d\n", num_scanouts); - virtio_cread(vgdev->vdev, struct virtio_gpu_config, -num_capsets, _capsets); + virtio_cread_le(vgdev->vdev, struct virtio_gpu_config, + num_capsets, _capsets); DRM_INFO("number of cap sets: %d\n", num_capsets); virtio_gpu_modeset_init(vgdev); -- MST ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/5] drm/i915: Add enable/disable flip done and flip done handler
On 7/25/2020 4:56 AM, Paulo Zanoni wrote: Em seg, 2020-07-20 às 17:01 +0530, Karthik B S escreveu: Add enable/disable flip done functions and the flip done handler function which handles the flip done interrupt. Enable the flip done interrupt in IER. Enable flip done function is called before writing the surface address register as the write to this register triggers the flip done interrupt Flip done handler is used to send the page flip event as soon as the surface address is written as per the requirement of async flips. The interrupt is disabled after the event is sent. v2: -Change function name from icl_* to skl_* (Paulo) -Move flip handler to this patch (Paulo) -Remove vblank_put() (Paulo) -Enable flip done interrupt for gen9+ only (Paulo) -Enable flip done interrupt in power_well_post_enable hook (Paulo) -Removed the event check in flip done handler to handle async flips without pageflip events. v3: -Move skl_disable_flip_done out of interrupt handler (Paulo) -Make the pending vblank event NULL in the beginning of flip_done_handler to remove sporadic WARN_ON that is seen. v4: -Calculate timestamps using flip done time stamp and current timestamp for async flips (Ville) v5: -Fix the sparse warning by making the function 'g4x_get_flip_counter' static.(Reported-by: kernel test robot ) -Fix the typo in commit message. Signed-off-by: Karthik B S Signed-off-by: Vandita Kulkarni --- drivers/gpu/drm/i915/display/intel_display.c | 10 +++ drivers/gpu/drm/i915/i915_irq.c | 83 ++-- drivers/gpu/drm/i915/i915_irq.h | 2 + drivers/gpu/drm/i915/i915_reg.h | 4 +- 4 files changed, 91 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index db2a5a1a9b35..b8ff032195d9 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -15562,6 +15562,13 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) intel_dbuf_pre_plane_update(state); + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + if (new_crtc_state->uapi.async_flip) { + skl_enable_flip_done(>base); + break; Do we really want the break here? What if more than one CRTC wants an async flip? Thanks for the review. This will fail for multiple CRTC case, I will remove this break. Perhaps you could extend IGT to try this. Currently we cannot add this scenario of having 2 crtc's in the same commit, as we're using the page flip ioctl. But I did try by hacking via the atomic path and 2 display with async is working fine. + } + } + /* Now enable the clocks, plane, pipe, and connectors that we set up. */ dev_priv->display.commit_modeset_enables(state); @@ -15583,6 +15590,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) drm_atomic_helper_wait_for_flip_done(dev, >base); for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + if (new_crtc_state->uapi.async_flip) + skl_disable_flip_done(>base); Here we don't break in the first found, so at least there's an inconsistency. I will remove the break in the earlier loop. + if (new_crtc_state->hw.active && !needs_modeset(new_crtc_state) && !new_crtc_state->preload_luts && diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1fa67700d8f4..95953b393941 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -697,14 +697,24 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc) return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xff; } +static u32 g4x_get_flip_counter(struct drm_crtc *crtc) +{ + struct drm_i915_private *dev_priv = to_i915(crtc->dev); + enum pipe pipe = to_intel_crtc(crtc)->pipe; + + return I915_READ(PIPE_FLIPCOUNT_G4X(pipe)); +} + u32 g4x_get_vblank_counter(struct drm_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->dev); enum pipe pipe = to_intel_crtc(crtc)->pipe; + if (crtc->state->async_flip) + return g4x_get_flip_counter(crtc); + return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); I don't understand the intention behind this, can you please clarify? This goes back to my reply of the cover letter. It seems that here we're going to alternate between two different counters in our vblank count. So if user space alternates between sometimes using async flips and sometimes using normal flip it's going to get some very weird deltas, isn't it? At least this is what I remember from when I played with these registers: FLIPCOUNT drifts away from FRMCOUNT when we start using async flips. IMHO we
[PATCH v3 10/38] virtio_gpu: correct tags for config space fields
Since gpu is a modern-only device, tag config space fields as having little endian-ness. Signed-off-by: Michael S. Tsirkin Reviewed-by: Cornelia Huck --- include/uapi/linux/virtio_gpu.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h index 0c85914d9369..ccbd174ef321 100644 --- a/include/uapi/linux/virtio_gpu.h +++ b/include/uapi/linux/virtio_gpu.h @@ -320,10 +320,10 @@ struct virtio_gpu_resp_edid { #define VIRTIO_GPU_EVENT_DISPLAY (1 << 0) struct virtio_gpu_config { - __u32 events_read; - __u32 events_clear; - __u32 num_scanouts; - __u32 num_capsets; + __le32 events_read; + __le32 events_clear; + __le32 num_scanouts; + __le32 num_capsets; }; /* simple formats for fbcon/X use */ -- MST ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/gma500: fix spelling mistake "pannel" -> "panel"
From: Colin Ian King There a handful of spelling mistakes. fix them. Signed-off-by: Colin Ian King --- drivers/gpu/drm/gma500/mdfld_dsi_output.c | 4 ++-- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c b/drivers/gpu/drm/gma500/mdfld_dsi_output.c index f350ac1ead18..2f3486f32fed 100644 --- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c +++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c @@ -366,7 +366,7 @@ static enum drm_mode_status mdfld_dsi_connector_mode_valid(struct drm_connector /** * FIXME: current DC has no fitting unit, reject any mode setting * request -* Will figure out a way to do up-scaling(pannel fitting) later. +* Will figure out a way to do up-scaling(panel fitting) later. **/ if (fixed_mode) { if (mode->hdisplay != fixed_mode->hdisplay) @@ -531,7 +531,7 @@ void mdfld_dsi_output_init(struct drm_device *dev, dsi_config->connector = dsi_connector; if (!dsi_config->fixed_mode) { - DRM_ERROR("No pannel fixed mode was found\n"); + DRM_ERROR("No panel fixed mode was found\n"); goto dsi_init_err0; } diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c index 06e44f47e73e..907f966d6f22 100644 --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c @@ -125,7 +125,7 @@ struct psb_intel_sdvo { bool is_lvds; /** -* This is sdvo fixed pannel mode pointer +* This is sdvo fixed panel mode pointer */ struct drm_display_mode *sdvo_lvds_fixed_mode; -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/amdgpu: fix spelling mistake "paramter" -> "parameter"
From: Colin Ian King There is a spelling mistake in a dev_warn message. Fix it. Signed-off-by: Colin Ian King --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index b72aeeb0a226..16e23f053361 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1201,7 +1201,7 @@ static int amdgpu_device_check_arguments(struct amdgpu_device *adev) if (amdgpu_num_kcq > 8 || amdgpu_num_kcq < 0) { amdgpu_num_kcq = 8; - dev_warn(adev->dev, "set kernel compute queue number to 8 due to invalid paramter provided by user\n"); + dev_warn(adev->dev, "set kernel compute queue number to 8 due to invalid parameter provided by user\n"); } return 0; -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [00/59] ttm misc cleanups, mem refactoring, rename objects. (v2)
Forget what I wrote the -ENOMEM was a type in my local change :) Sorry for the noise, Christian. Am 05.08.20 um 13:55 schrieb Christian König: Dave, do you have a branch with the latest version somewhere? I've tested your ttm-refactor-mem-manager branch from fdo today a bit and that pretty quickly results in an -ENOMEM. Looks like a leak somewhere to me. Christian. Am 04.08.20 um 04:55 schrieb Dave Airlie: I've decided to repost the whole queue this time, it has a few driver patches up front that are just cleanups. I've reorodered things a little since the first posting, but not much. misc core/driver cleanups mem type manager debug callback cleanup (reordered) new mem type manager init path new mem type manager cleanup path new wrapper to access managers driver managed mem type allocations cleanup after all of that rename ttm_bo_manager to ttm_range_manager (new) rename ttm_mem_type_manager to ttm_resource_manager (new) rename ttm_mem_reg to ttm_resource (new) I've marked most things with v2 that have changed, again I'd really like to land this in drm-misc sooner rather than later, so it would be good to get driver acks from qxl, vmwgfx and nouveau. (all cc'ed). I've also commented on the list on any outstanding comments from previous review that I disagree with or need more discussion. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [00/59] ttm misc cleanups, mem refactoring, rename objects. (v2)
Dave, do you have a branch with the latest version somewhere? I've tested your ttm-refactor-mem-manager branch from fdo today a bit and that pretty quickly results in an -ENOMEM. Looks like a leak somewhere to me. Christian. Am 04.08.20 um 04:55 schrieb Dave Airlie: I've decided to repost the whole queue this time, it has a few driver patches up front that are just cleanups. I've reorodered things a little since the first posting, but not much. misc core/driver cleanups mem type manager debug callback cleanup (reordered) new mem type manager init path new mem type manager cleanup path new wrapper to access managers driver managed mem type allocations cleanup after all of that rename ttm_bo_manager to ttm_range_manager (new) rename ttm_mem_type_manager to ttm_resource_manager (new) rename ttm_mem_reg to ttm_resource (new) I've marked most things with v2 that have changed, again I'd really like to land this in drm-misc sooner rather than later, so it would be good to get driver acks from qxl, vmwgfx and nouveau. (all cc'ed). I've also commented on the list on any outstanding comments from previous review that I disagree with or need more discussion. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Question about per process GPU context global priority enforcement
I don't think we have a uniform mechanism, currently each driver decides on their own. For the amdgpu driver we check that the process either has CAP_SYS_NICE or is the DRM master. On Wed, Aug 5, 2020 at 9:14 AM Yiwei Zhang wrote: > > Hi friends, > > For Vulkan/EGL, upon creating gpu contexts, applications can ask for a > system-wide higher priority levels via VK_EXT_global_priority or > EGL_IMG_context_priority extensions. > > I'm curious if we have certain rules(some form of process privilege > check) in the kernel to limit non-privileged ones to never go beyond > default system-wide gpu scheduling priority. (e.g. not allow random > app processes to contend the GPU queues repeatedly/infinitely with > high/realtime priorities) > > Many thanks, > Yiwei - from Android Platform Graphics Team > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
From: Colin Ian King There is a spelling mistake in a DRM_ERROR message. Fix it. Signed-off-by: Colin Ian King --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 49d4514aa6ed..c68369731b20 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle) ret = psp_tmr_terminate(psp); if (ret) { - DRM_ERROR("Falied to terminate tmr\n"); + DRM_ERROR("Failed to terminate tmr\n"); return ret; } -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vmwgfx: fix spelling mistake "Cound" -> "Could"
From: Colin Ian King There is a spelling mistake in a DRM_ERROR message. Fix it. Signed-off-by: Colin Ian King --- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c index 4284c4bd444d..e67e2e8f6e6f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c @@ -3037,7 +3037,7 @@ static int vmw_cmd_dx_bind_streamoutput(struct vmw_private *dev_priv, res = vmw_dx_streamoutput_lookup(vmw_context_res_man(ctx_node->ctx), cmd->body.soid); if (IS_ERR(res)) { - DRM_ERROR("Cound not find streamoutput to bind.\n"); + DRM_ERROR("Could not find streamoutput to bind.\n"); return PTR_ERR(res); } -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] arm: dts: remove stray /dts-v1/ from mt7623a.dtsi
From: David Woodhouse This isn't needed in dtsi files. Signed-off-by: David Woodhouse --- arch/arm/boot/dts/mt7623a.dtsi | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm/boot/dts/mt7623a.dtsi b/arch/arm/boot/dts/mt7623a.dtsi index 0735a1fb8ad9..a96075206cce 100644 --- a/arch/arm/boot/dts/mt7623a.dtsi +++ b/arch/arm/boot/dts/mt7623a.dtsi @@ -5,7 +5,6 @@ * */ -/dts-v1/; #include #include "mt7623.dtsi" -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] arm: dts: mt7623: add support for UniElec U7623 eMMC
From: David Woodhouse Based on a patch in OpenWrt from Kristian Evensen Signed-off-by: David Woodhouse --- arch/arm/boot/dts/Makefile| 1 + .../boot/dts/mt7623a-unielec-u7623-emmc.dts | 347 ++ 2 files changed, 348 insertions(+) create mode 100644 arch/arm/boot/dts/mt7623a-unielec-u7623-emmc.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 4572db3fa5ae..52f3954b600c 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -1347,6 +1347,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += \ mt6580-evbp1.dtb \ mt6589-aquaris5.dtb \ mt6592-evb.dtb \ + mt7623a-unielec-u7623-emmc.dtb \ mt7623a-rfb-emmc.dtb \ mt7623a-rfb-nand.dtb \ mt7623n-rfb-emmc.dtb \ diff --git a/arch/arm/boot/dts/mt7623a-unielec-u7623-emmc.dts b/arch/arm/boot/dts/mt7623a-unielec-u7623-emmc.dts new file mode 100644 index ..6205a32159ce --- /dev/null +++ b/arch/arm/boot/dts/mt7623a-unielec-u7623-emmc.dts @@ -0,0 +1,347 @@ +// SPDX-License-Identifier: (GPL-2.0+ or MIT) +/* + * Copyright © 2018 Kristian Evensen + */ + +/dts-v1/; +#include +#include "mt7623a.dtsi" +#include "mt6323.dtsi" + +/ { + model = "UniElec U7623 eMMC"; + compatible = "unielec,u7623-emmc", "mediatek,mt7623"; + + memory@8000 { + device_type = "memory"; + reg = <0 0x8000 0 0x2000>; + }; + + aliases { + serial2 = + }; + + chosen { + stdout-path = "serial2:115200n8"; + }; + + cpus { + cpu@0 { + proc-supply = <_vproc_reg>; + }; + + cpu@1 { + proc-supply = <_vproc_reg>; + }; + + cpu@2 { + proc-supply = <_vproc_reg>; + }; + + cpu@3 { + proc-supply = <_vproc_reg>; + }; + }; + + reg_1p8v: regulator-1p8v { + compatible = "regulator-fixed"; + regulator-name = "fixed-1.8V"; + regulator-min-microvolt = <180>; + regulator-max-microvolt = <180>; + regulator-boot-on; + regulator-always-on; + }; + + reg_3p3v: regulator-3p3v { + compatible = "regulator-fixed"; + regulator-name = "fixed-3.3V"; + regulator-min-microvolt = <330>; + regulator-max-microvolt = <330>; + regulator-boot-on; + regulator-always-on; + }; + + reg_5v: regulator-5v { + compatible = "regulator-fixed"; + regulator-name = "fixed-5V"; + regulator-min-microvolt = <500>; + regulator-max-microvolt = <500>; + regulator-boot-on; + regulator-always-on; + }; + + gpio-keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <_pins_a>; + + factory { + label = "factory"; + linux,code = ; + gpios = < 256 GPIO_ACTIVE_LOW>; + }; + }; + + leds { + compatible = "gpio-leds"; + pinctrl-names = "default"; + pinctrl-0 = <_pins_unielec>; + + led3 { + label = "u7623-01:green:led3"; + gpios = < 14 GPIO_ACTIVE_LOW>; + default-state = "off"; + }; + + led4 { + label = "u7623-01:green:led4"; + gpios = < 15 GPIO_ACTIVE_LOW>; + default-state = "off"; + }; + }; +}; + + { + status = "okay"; +}; + + { + status = "okay"; + + gmac0: mac@0 { + compatible = "mediatek,eth-mac"; + reg = <0>; + phy-mode = "trgmii"; + + fixed-link { + speed = <1000>; + full-duplex; + pause; + }; + }; + + mdio: mdio-bus { + #address-cells = <1>; + #size-cells = <0>; + + mt7530: switch@0 { + compatible = "mediatek,mt7530"; + }; + }; +}; + + { + compatible = "mediatek,mt7530"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + pinctrl-names = "default"; + mediatek,mcm; + resets = < 2>; + reset-names = "mcm"; + core-supply = <_vpa_reg>; + io-supply = <_vemc3v3_reg>; + + dsa,mii-bus = <>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + + port@0 { + reg = <0>; +
[PATCH 2/3] arm: dts: mt7623: move MT7623N GPU to separate mt7623n.dtsi file
From: David Woodhouse The MT7623A doesn't have a GPU; add it only for MT7623N boards. Fixes: 1f6ed224594 ("arm: dts: mt7623: add Mali-450 device node") Signed-off-by: David Woodhouse --- arch/arm/boot/dts/mt7623.dtsi | 24 - arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 2 +- arch/arm/boot/dts/mt7623n-rfb-emmc.dts| 2 +- arch/arm/boot/dts/mt7623n.dtsi| 35 +++ 4 files changed, 37 insertions(+), 26 deletions(-) create mode 100644 arch/arm/boot/dts/mt7623n.dtsi diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi index 3a6b856e5b74..dcd2f5ba4e20 100644 --- a/arch/arm/boot/dts/mt7623.dtsi +++ b/arch/arm/boot/dts/mt7623.dtsi @@ -734,30 +734,6 @@ g3dsys: syscon@1300 { #reset-cells = <1>; }; - mali: gpu@1304 { - compatible = "mediatek,mt7623-mali", "arm,mali-450"; - reg = <0 0x1304 0 0x3>; - interrupts = , -, -, -, -, -, -, -, -, -, -; - interrupt-names = "gp", "gpmmu", "pp0", "ppmmu0", "pp1", - "ppmmu1", "pp2", "ppmmu2", "pp3", "ppmmu3", - "pp"; - clocks = < CLK_TOP_MMPLL>, -< CLK_G3DSYS_CORE>; - clock-names = "bus", "core"; - power-domains = < MT2701_POWER_DOMAIN_MFG>; - resets = < MT2701_G3DSYS_CORE_RST>; - }; - mmsys: syscon@1400 { compatible = "mediatek,mt7623-mmsys", "mediatek,mt2701-mmsys", diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts index 2b760f90f38c..344f8c65c4aa 100644 --- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts +++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts @@ -6,7 +6,7 @@ /dts-v1/; #include -#include "mt7623.dtsi" +#include "mt7623n.dtsi" #include "mt6323.dtsi" / { diff --git a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts index 0447748f9fa0..f8efcc364bc3 100644 --- a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts +++ b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts @@ -7,7 +7,7 @@ /dts-v1/; #include -#include "mt7623.dtsi" +#include "mt7623n.dtsi" #include "mt6323.dtsi" / { diff --git a/arch/arm/boot/dts/mt7623n.dtsi b/arch/arm/boot/dts/mt7623n.dtsi new file mode 100644 index ..7724a4d05b89 --- /dev/null +++ b/arch/arm/boot/dts/mt7623n.dtsi @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright © 2017-2020 MediaTek Inc. + * Author: Sean Wang + *Ryder Lee + * + */ + +#include "mt7623.dtsi" + +/ { + mali: gpu@1304 { + compatible = "mediatek,mt7623-mali", "arm,mali-450"; + reg = <0 0x1304 0 0x3>; + interrupts = , +, +, +, +, +, +, +, +, +, +; + interrupt-names = "gp", "gpmmu", "pp0", "ppmmu0", "pp1", + "ppmmu1", "pp2", "ppmmu2", "pp3", "ppmmu3", + "pp"; + clocks = < CLK_TOP_MMPLL>, +< CLK_G3DSYS_CORE>; + clock-names = "bus", "core"; + power-domains = < MT2701_POWER_DOMAIN_MFG>; + resets = < MT2701_G3DSYS_CORE_RST>; + }; +}; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Aw: Re: Re: Re: [PATCH v4 6/6] arm: dts: mt7623: add display subsystem related device nodes
On Wed, 2020-08-05 at 10:49 +0200, Frank Wunderlich wrote: > > Gesendet: Mittwoch, 05. August 2020 um 10:36 Uhr > > Von: "David Woodhouse" > > > mt7623.dtsi => mt7623n.dtsi => mt7623n-bananapi-bpi-r2.dts > > > mt7623.dtsi => mt7623a.dtsi => mt7623a-unielec-u7623.dts (not > > > existing yet, > > > openwrt seems to use a board-specific dtsi) > > > > Yes, I think we should. > > i want to see what MTK/DT owner says to this... > my current way will be still adding the nodes to existing mt7623.dtsi > (like ryder lee did it in original patch) > but disabling them to not break mt7623a and splitting it afterwards. > > > I'll create mt7623a.dtsi and upstream the U7623 support; I think that > > can happen without conflicting with anything you do. > > > > I note that the GPU node has been added to mt7623.dtsi in 5.8 too; > > that'll want to move to the new mt7623n.dtsi that you create, along > > with your other new additions. > > i guess mali-node also needs to be moved to mt7623n.dtsi, so my > current way seems right... > but it's decision of MTK/DT owner. if they make a note i squash the > disabling-commit into this and post v5 Yes, the mali node needs moving too. I've pushed an untested series to http://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/mt7623 which does that and adds the UniElec board. smime.p7s Description: S/MIME cryptographic signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: sysfs: Add to get current mode
On Wed, Aug 05, 2020 at 06:03:15PM +0800, Huang Jiachai wrote: > Hi Daniel > > 在 2020/8/5 17:36, Daniel Vetter 写道: > > On Wed, Aug 5, 2020 at 10:37 AM Sandy Huang wrote: > > > add this node to get the current true mode. > > > > > > Signed-off-by: Sandy Huang > > Uh what's this for? Since it's sysfs, I guess there's something > > parsing this, which means we'd kinda need to have that piece too. > > > > If it's just for debugging purposes, then we already have this > > information in debugfs, together with everything else that's in the > > atomic modeset state. > > -Daniel > > yes, this is just for debug; > > and i get the information what i need from cat > /sys/kernel/debug/dri/0/state, thanks. Cool, sounds like this is resolved. And if you need any additional debug information about display state, then best to extend that file. It comes with driver hooks, so that you can include any additional driver stuff, or outright read out registers and everything. Cheers, Daniel > > > > --- > > > drivers/gpu/drm/drm_sysfs.c | 30 ++ > > > 1 file changed, 30 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > > > index 939f0032aab1..f39bcd34853b 100644 > > > --- a/drivers/gpu/drm/drm_sysfs.c > > > +++ b/drivers/gpu/drm/drm_sysfs.c > > > @@ -19,6 +19,7 @@ > > > #include > > > > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -236,16 +237,45 @@ static ssize_t modes_show(struct device *device, > > > return written; > > > } > > > > > > +static ssize_t current_mode_show(struct device *device, > > > + struct device_attribute *attr, > > > + char *buf) > > > +{ > > > + struct drm_connector *connector = to_drm_connector(device); > > > + struct drm_display_mode *mode; > > > + struct drm_crtc_state *crtc_state; > > > + bool interlaced; > > > + int written = 0; > > > + > > > + if (!connector->state || !connector->state->crtc) > > > + return written; > > > + > > > + crtc_state = connector->state->crtc->state; > > > + if (!crtc_state) > > > + return written; > > > + > > > + mode = _state->mode; > > > + > > > + interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE); > > > + written += snprintf(buf + written, PAGE_SIZE - written, > > > "%dx%d%s%d\n", > > > + mode->hdisplay, mode->vdisplay, > > > + interlaced ? "i" : "p", > > > drm_mode_vrefresh(mode)); > > > + > > > + return written; > > > +} > > > + > > > static DEVICE_ATTR_RW(status); > > > static DEVICE_ATTR_RO(enabled); > > > static DEVICE_ATTR_RO(dpms); > > > static DEVICE_ATTR_RO(modes); > > > +static DEVICE_ATTR_RO(current_mode); > > > > > > static struct attribute *connector_dev_attrs[] = { > > > _attr_status.attr, > > > _attr_enabled.attr, > > > _attr_dpms.attr, > > > _attr_modes.attr, > > > + _attr_current_mode.attr, > > > NULL > > > }; > > > > > > -- > > > 2.17.1 > > > > > > > > > > > > -- > Best Regard > > 黄家钗 > Sandy Huang > Addr: 福州市鼓楼区铜盘路软件大道89号福州软件园A区21号楼(350003) > No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC > Tel:+86 0591-87884919 8690 > E-mail:h...@rock-chips.com > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] staging: ion: fix spelling mistake in function name "detatch" -> "detach"
From: Colin Ian King There is a spelling mistake in the function name ion_dma_buf_detatch. Fix it by removing the extraneous t. Signed-off-by: Colin Ian King --- drivers/staging/android/ion/ion.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 3c9f09506ffa..e1fe03ceb7f1 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -205,8 +205,8 @@ static int ion_dma_buf_attach(struct dma_buf *dmabuf, return 0; } -static void ion_dma_buf_detatch(struct dma_buf *dmabuf, - struct dma_buf_attachment *attachment) +static void ion_dma_buf_detach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) { struct ion_dma_buf_attachment *a = attachment->priv; struct ion_buffer *buffer = dmabuf->priv; @@ -331,7 +331,7 @@ static const struct dma_buf_ops dma_buf_ops = { .mmap = ion_mmap, .release = ion_dma_buf_release, .attach = ion_dma_buf_attach, - .detach = ion_dma_buf_detatch, + .detach = ion_dma_buf_detach, .begin_cpu_access = ion_dma_buf_begin_cpu_access, .end_cpu_access = ion_dma_buf_end_cpu_access, }; -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] xen/gntdev: Fix dmabuf import with non-zero sgt offset
On 8/4/20 9:11 AM, Jürgen Groß wrote: > On 31.07.20 14:51, Oleksandr Andrushchenko wrote: >> From: Oleksandr Andrushchenko >> >> It is possible that the scatter-gather table during dmabuf import has >> non-zero offset of the data, but user-space doesn't expect that. >> Fix this by failing the import, so user-space doesn't access wrong data. >> >> Fixes: 37ccb44d0b00 ("xen/gntdev: Implement dma-buf import functionality") > > I can't find this commit in the tree. Did you mean bf8dc55b1358? I'll double-check, thank you > > And don't you want to Cc stable for this patch, too? Hm, yes, sounds reasonable > >> >> Signed-off-by: Oleksandr Andrushchenko > > Acked-by: Juergen Gross > > > Juergen ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] dmabuf/tracing: Add dma-buf trace events
On Tue, Aug 04, 2020 at 02:09:13AM +0100, Al Viro wrote: > On Mon, Aug 03, 2020 at 11:28:31PM +0100, Al Viro wrote: > > > IOW, what the hell is that horror for? You do realize, for example, that > > there's > > such thing as dup(), right? And dup2() as well. And while we are at it, > > how > > do you keep track of removals, considering the fact that you can stick a > > file > > reference into SCM_RIGHTS datagram sent to yourself, close descriptors and > > an hour > > later pick that datagram, suddenly getting descriptor back? > > > > Besides, "I have no descriptors left" != "I can't be currently sitting in > > the middle > > of syscall on that sucker"; close() does *NOT* terminate ongoing operations. > > > > You are looking at the drastically wrong abstraction level. Please, > > describe what > > it is that you are trying to achieve. Hi Al. Thank you for the comments. Ultimately what we need is to identify processes that hold a file reference to the dma-buf. Unfortunately we can't use only explicit dma_buf_get/dma_buf_put to track them because when an FD is being shared between processes the file references are taken implicitly. For example, on the sender side: unix_dgram_sendmsg -> send_scm -> __send_scm -> scm_fp_copy -> fget_raw and on the receiver side: unix_dgram_recvmsg -> scm_recv -> scm_detach_fds -> __scm_install_fd -> get_file I understand now that fd_install is not an appropriate abstraction level to track these. Is there a more appropriate alternative where we could use to track these implicit file references? > _IF_ it's "who keeps a particularly long-lived sucker pinned", I would suggest > fuser(1) run when you detect that kind of long-lived dmabuf. With events > generated > by their constructors and destructors, and detection of longevity done based > on > that. > > But that's only a semi-blind guess at the things you are trying to achieve; > please, > describe what it really is. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM/KMS experimental drivers for HiKey 970
On Wed, Aug 5, 2020 at 12:13 PM Mauro Carvalho Chehab wrote: > > Em Wed, 5 Aug 2020 11:34:54 +0200 > Daniel Vetter escreveu: > > > On Wed, Aug 5, 2020 at 10:51 AM Mauro Carvalho Chehab > > wrote: > > > > > > Hi, > > > > > > I've been working to get upstream support for the DRM driver on HiKey 970. > > > > > > While the patches are not ready yet for upstream merge, I'm placing > > > what I've sone so far on this repository: > > > > > > > > > https://github.com/mchehab/linux/tree/hikey970/to_upstream-2.0-v1.1 > > > > > > The drivers there are a port from the Linaro's HiKey official Kernel: > > > > > > https://github.com/96boards-hikey/linux > > > > > > The patches there preserve the old history. The porting patches > > > are applied after the original patches imported from that tree. > > > > > > Besides the DRM driver, this repository contains: > > > > > > - a PMIC/regulator driver; > > > - an iommu driver (still requiring some changes at DT properties); > > > - A DMA driver; > > > - a small ugly hack reverting some PM changes at the WiFi driver, > > > causing a regression on this board for HiKey 970. > > > > > > My current plans are to start upstreaming those needed drivers. > > > > > > The KMS/DRM driver there still need some changes. I guess it is > > > not ready for being upstreamed yet. Also, it depends on the > > > other drivers to work. > > > > > > In particular, its support for DPMS is broken: if the monitor is > > > suspended, it won't return back to the right frequency settings. > > > > Hm this is somewhat surprising, at least with atomic, since DPMS as a > > separate thing doesn't exist anymore - it's the same path as any other > > modeset. With the suspend/resume helpers even the same as after > > resume. But of course in reality there's always potential for some > > differences between boot-up state and what your atomic_disable leaves > > behind to creep in. > > Yeah, I didn't notice anything that would explain a problem there, > but I didn't have much time so far to try to identify what is > the real issue there. > > Btw, this problem is also present on the official Hikey Linaro's tree. > There, it had an ugly hack at the modeset logic on adv7535 downstream > driver. > > Ah, I forgot to mention, but this driver has a problem when talking > with EDID. So, I'm passing this parameter via grub: > > drm_kms_helper.edid_firmware=edid/1920x1080.bin > > With the EDID info from the monitor I'm using at the tests. Hm probing and atomic check/commit should be largely decoupled, at least for dumb outputs. For DP and hdmi 2, where we have sideband traffic to do link training and all that it's different. So fake edid (or just force-setting a mode you like, even on a disconnected output) should all work. > Perhaps the DPMS is somehow related to it. > > In any case, the modeset part of this driver needs to be revisited, > before merging it drivers/drm. > > Btw, both issues are also present at the official Hikey driver, > which makes a little harder to fix, as I was unable to get any > documentation about this chipset so far - except for the public > ones at 96boards.org. Uh yeah that makes it tough :-/ > > > > Just figured I drop this in quickly, always great to have more hw > > drivers showing up! > > Yeah, it has been great for me to work on this DRM driver. > > Btw, although I didn't test, this driver is meant to support > also Hikey 960. > > I intend to test it there when I have some spare time. Adding John Stulz, I think he's been working on upstreaming part of that too, but not sure. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 0/4] drm/ast: Disable HW cursor when switching modes
Should say [PATCH v2 0/4] instead Am 05.08.20 um 12:54 schrieb Thomas Zimmermann: > Since converting the ast driver to atomic modesettting, modesetting > occationally locks up the graphics hardware and turns the display > permanently dark. This happens once or twice per 10 mode switches. > Investigation shows that the ast hardware presumably requires the HW > cursor to be disabled while the modeswitch takes place. > > This patchset fixes the problem by disabling planes before programming > the CRTC mode or format. After the switch, the planes gets re-enabled if > they were enabled before. For mere pageflip operations, nothing changes. > > Patches #1 makes format changes work as intended: format registers are > only updated if necessary. They used to be changed on each pageflip. > Patch #2 puts the modesetting code before the plane-update code. This > way, mode setting runs while planes are disabled. Patches #3 and #4 > add a commit-tail function that disables planes if the display's mode > or format is going to change. The active planes will later get re-enabled > by the plane-update handler. > > Tested on AST2100 HW. The issue is not 100% reproducible, but does not > show up after applying the patchset. I think the problem has been fixed. > > v2: > * rewrote the whole patchset > * dropped the cursor patches > * moved modesetting into atomic_begin() > * disable planes in commit-tail function > * don't require full modeset for format changes > > Thomas Zimmermann (4): > drm/ast: Only set format registers if primary plane's format changes > drm/ast: Set display mode in atomic_begin() > drm/ast: Add commit-tail function > drm/ast: Disable planes while switching display modes > > drivers/gpu/drm/ast/ast_drv.h | 2 + > drivers/gpu/drm/ast/ast_mode.c | 148 + > 2 files changed, 114 insertions(+), 36 deletions(-) > > -- > 2.28.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 signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 4/4] drm/ast: Disable planes while switching display modes
The ast HW cursor requires the primary plane and CRTC to display at a valid mode and format. This is not the case while switching display modes, which can lead to the screen turing permanently dark. As a workaround, the ast driver now disables active planes while the mode or format switch takes place. It also synchronizes with the vertical refresh to give CRTC and planes some time to catch up on each other. The active planes planes (primary or cursor) will be re-enabled by each plane's atomic_update() function. v2: * move the logic into the commit-tail function Signed-off-by: Thomas Zimmermann Fixes: 4961eb60f145 ("drm/ast: Enable atomic modesetting") Cc: Thomas Zimmermann Cc: Gerd Hoffmann Cc: Dave Airlie Cc: Daniel Vetter Cc: Sam Ravnborg Cc: Emil Velikov Cc: "Y.C. Chen" Cc: # v5.6+ --- drivers/gpu/drm/ast/ast_drv.h | 2 + drivers/gpu/drm/ast/ast_mode.c | 68 -- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index c1af6b725933..467049ca8430 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -177,6 +177,8 @@ struct ast_private *ast_device_create(struct drm_driver *drv, #define AST_IO_MM_OFFSET (0x380) +#define AST_IO_VGAIR1_VREFRESH BIT(3) + #define __ast_read(x) \ static inline u##x ast_read##x(struct ast_private *ast, u32 reg) { \ u##x val = 0;\ diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index ae5cb0a333f7..a379d51f3543 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -514,6 +514,17 @@ static void ast_set_start_address_crt1(struct ast_private *ast, } +static void ast_wait_for_vretrace(struct ast_private *ast) +{ + unsigned long timeout = jiffies + HZ; + u8 vgair1; + + do { + vgair1 = ast_io_read8(ast, AST_IO_INPUT_STATUS1_READ); + } while (!(vgair1 & AST_IO_VGAIR1_VREFRESH) && +time_before(jiffies, timeout)); +} + /* * Primary plane */ @@ -1043,23 +1054,72 @@ static int ast_connector_init(struct drm_device *dev) * Mode config */ +static bool +ast_crtc_needs_planes_disabled(struct drm_crtc_state *old_crtc_state, + struct drm_crtc_state *new_crtc_state) +{ + struct ast_crtc_state *old_ast_crtc_state, *new_ast_crtc_state; + + if (drm_atomic_crtc_needs_modeset(new_crtc_state)) + return true; + + old_ast_crtc_state = to_ast_crtc_state(old_crtc_state); + new_ast_crtc_state = to_ast_crtc_state(new_crtc_state); + + if (old_ast_crtc_state->format != new_ast_crtc_state->format) + return true; + + return false; +} + static void ast_mode_config_helper_commit_tail(struct drm_atomic_state *old_state) { struct drm_device *dev = old_state->dev; + struct ast_private *ast = to_ast_private(dev); + struct drm_crtc_state *old_crtc_state, *new_crtc_state; + struct drm_crtc *crtc; + int i; + bool wait_for_vretrace = false; drm_atomic_helper_commit_modeset_disables(dev, old_state); - drm_atomic_helper_commit_planes(dev, old_state, 0); + /* +* HW cursors require the underlying primary plane and CRTC to +* display a valid mode and image. This is not the case during +* full modeset operations. So we temporarily disable any active +* plane, including the HW cursor. Each plane's atomic_update() +* helper will re-enable it if necessary. +* +* We only do this during *full* modesets. It does not affect +* simple pageflips on the planes. +*/ + for_each_oldnew_crtc_in_state(old_state, crtc, + old_crtc_state, + new_crtc_state, i) { + if (!ast_crtc_needs_planes_disabled(old_crtc_state, + new_crtc_state)) + continue; + drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, +false); + wait_for_vretrace = true; + } + + /* +* Ensure that no scanout takes place before reprogramming mode +* and format registers. +*/ + if (wait_for_vretrace) + ast_wait_for_vretrace(ast); + + drm_atomic_helper_commit_planes(dev, old_state, + DRM_PLANE_COMMIT_ACTIVE_ONLY); drm_atomic_helper_commit_modeset_enables(dev, old_state); drm_atomic_helper_fake_vblank(old_state); - drm_atomic_helper_commit_hw_done(old_state); - drm_atomic_helper_wait_for_vblanks(dev, old_state); - drm_atomic_helper_cleanup_planes(dev, old_state); } -- 2.28.0 ___ dri-devel mailing list
[PATCH v1 3/4] drm/ast: Add commit-tail function
Duplicates drm_atomic_helper_commit_tail(), so that planes can be disabled on full mode-setting changes. Signed-off-by: Thomas Zimmermann Fixes: 4961eb60f145 ("drm/ast: Enable atomic modesetting") Cc: Thomas Zimmermann Cc: Gerd Hoffmann Cc: Dave Airlie Cc: Daniel Vetter Cc: Sam Ravnborg Cc: Emil Velikov Cc: "Y.C. Chen" Cc: # v5.6+ --- drivers/gpu/drm/ast/ast_mode.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 0ea8a68ac2d9..ae5cb0a333f7 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1043,6 +1043,31 @@ static int ast_connector_init(struct drm_device *dev) * Mode config */ +static void +ast_mode_config_helper_commit_tail(struct drm_atomic_state *old_state) +{ + struct drm_device *dev = old_state->dev; + + drm_atomic_helper_commit_modeset_disables(dev, old_state); + + 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); + + drm_atomic_helper_commit_hw_done(old_state); + + drm_atomic_helper_wait_for_vblanks(dev, old_state); + + drm_atomic_helper_cleanup_planes(dev, old_state); +} + +static const struct drm_mode_config_helper_funcs +ast_mode_config_helper_funcs = { + .atomic_commit_tail = ast_mode_config_helper_commit_tail, +}; + static const struct drm_mode_config_funcs ast_mode_config_funcs = { .fb_create = drm_gem_fb_create, .mode_valid = drm_vram_helper_mode_valid, @@ -1082,6 +1107,8 @@ int ast_mode_config_init(struct ast_private *ast) dev->mode_config.max_height = 1200; } + dev->mode_config.helper_private = _mode_config_helper_funcs; + memset(>primary_plane, 0, sizeof(ast->primary_plane)); ret = drm_universal_plane_init(dev, >primary_plane, 0x01, _primary_plane_funcs, -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 2/4] drm/ast: Set display mode in atomic_begin()
Move the mode-setting code from atomic_flush() to atomic_begin(), and thus before the plane update. With the CRTC update before the plane updates, the cursor can be disabled while the mode is being changed. The patch removes the call ast_open_key() from atomic_begin(), The function unlocks ast's extended display registers; something that has been done at this point already. The now-empty implementation of atomic_flush() is also being removed. Signed-off-by: Thomas Zimmermann Fixes: 4961eb60f145 ("drm/ast: Enable atomic modesetting") Cc: Thomas Zimmermann Cc: Gerd Hoffmann Cc: Dave Airlie Cc: Daniel Vetter Cc: Sam Ravnborg Cc: Emil Velikov Cc: "Y.C. Chen" Cc: # v5.6+ --- drivers/gpu/drm/ast/ast_mode.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index b129833c0821..0ea8a68ac2d9 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -757,14 +757,6 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc, static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) -{ - struct ast_private *ast = to_ast_private(crtc->dev); - - ast_open_key(ast); -} - -static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, -struct drm_crtc_state *old_crtc_state) { struct drm_device *dev = crtc->dev; struct ast_private *ast = to_ast_private(dev); @@ -813,7 +805,6 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = { .atomic_check = ast_crtc_helper_atomic_check, .atomic_begin = ast_crtc_helper_atomic_begin, - .atomic_flush = ast_crtc_helper_atomic_flush, .atomic_enable = ast_crtc_helper_atomic_enable, .atomic_disable = ast_crtc_helper_atomic_disable, }; -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 1/4] drm/ast: Only set format registers if primary plane's format changes
The atomic modesetting code tried to distinguish format changes from full modesetting operations. But the implementation was buggy and the format registers were often updated even for simple pageflips. Fix this problem by distinguishing between format and mode changes; and handle each in it's own branch. Signed-off-by: Thomas Zimmermann Fixes: 4961eb60f145 ("drm/ast: Enable atomic modesetting") Cc: Thomas Zimmermann Cc: Gerd Hoffmann Cc: Dave Airlie Cc: Daniel Vetter Cc: Sam Ravnborg Cc: Emil Velikov Cc: "Y.C. Chen" Cc: # v5.6+ --- drivers/gpu/drm/ast/ast_mode.c | 52 -- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 62fe682a7de6..b129833c0821 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -768,34 +768,32 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, { struct drm_device *dev = crtc->dev; struct ast_private *ast = to_ast_private(dev); - struct ast_crtc_state *ast_state; - const struct drm_format_info *format; - struct ast_vbios_mode_info *vbios_mode_info; - struct drm_display_mode *adjusted_mode; - - ast_state = to_ast_crtc_state(crtc->state); - - format = ast_state->format; - if (!format) - return; - - vbios_mode_info = _state->vbios_mode_info; - - ast_set_color_reg(ast, format); - ast_set_vbios_color_reg(ast, format, vbios_mode_info); - - if (!crtc->state->mode_changed) - return; - - adjusted_mode = >state->adjusted_mode; + struct drm_crtc_state *crtc_state = crtc->state; + struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state); + struct ast_crtc_state *old_ast_crtc_state = + to_ast_crtc_state(old_crtc_state); + const struct drm_format_info *format = ast_crtc_state->format; + struct ast_vbios_mode_info *vbios_mode_info = + _crtc_state->vbios_mode_info; + struct drm_display_mode *adjusted_mode = _state->adjusted_mode; + + if (drm_WARN_ON_ONCE(dev, !format)) + return; /* BUG: We didn't set format in primary check(). */ + + if (format != old_ast_crtc_state->format) { + ast_set_color_reg(ast, format); + ast_set_vbios_color_reg(ast, format, vbios_mode_info); + } - ast_set_vbios_mode_reg(ast, adjusted_mode, vbios_mode_info); - ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06); - ast_set_std_reg(ast, adjusted_mode, vbios_mode_info); - ast_set_crtc_reg(ast, adjusted_mode, vbios_mode_info); - ast_set_dclk_reg(ast, adjusted_mode, vbios_mode_info); - ast_set_crtthd_reg(ast); - ast_set_sync_reg(ast, adjusted_mode, vbios_mode_info); + if (drm_atomic_crtc_needs_modeset(crtc_state)) { + ast_set_vbios_mode_reg(ast, adjusted_mode, vbios_mode_info); + ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06); + ast_set_std_reg(ast, adjusted_mode, vbios_mode_info); + ast_set_crtc_reg(ast, adjusted_mode, vbios_mode_info); + ast_set_dclk_reg(ast, adjusted_mode, vbios_mode_info); + ast_set_crtthd_reg(ast); + ast_set_sync_reg(ast, adjusted_mode, vbios_mode_info); + } } static void -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 0/4] drm/ast: Disable HW cursor when switching modes
Since converting the ast driver to atomic modesettting, modesetting occationally locks up the graphics hardware and turns the display permanently dark. This happens once or twice per 10 mode switches. Investigation shows that the ast hardware presumably requires the HW cursor to be disabled while the modeswitch takes place. This patchset fixes the problem by disabling planes before programming the CRTC mode or format. After the switch, the planes gets re-enabled if they were enabled before. For mere pageflip operations, nothing changes. Patches #1 makes format changes work as intended: format registers are only updated if necessary. They used to be changed on each pageflip. Patch #2 puts the modesetting code before the plane-update code. This way, mode setting runs while planes are disabled. Patches #3 and #4 add a commit-tail function that disables planes if the display's mode or format is going to change. The active planes will later get re-enabled by the plane-update handler. Tested on AST2100 HW. The issue is not 100% reproducible, but does not show up after applying the patchset. I think the problem has been fixed. v2: * rewrote the whole patchset * dropped the cursor patches * moved modesetting into atomic_begin() * disable planes in commit-tail function * don't require full modeset for format changes Thomas Zimmermann (4): drm/ast: Only set format registers if primary plane's format changes drm/ast: Set display mode in atomic_begin() drm/ast: Add commit-tail function drm/ast: Disable planes while switching display modes drivers/gpu/drm/ast/ast_drv.h | 2 + drivers/gpu/drm/ast/ast_mode.c | 148 + 2 files changed, 114 insertions(+), 36 deletions(-) -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] omapfb: fix spelling mistake "propert" -> "property"
From: Colin Ian King There is a spelling mistake in a pr_err message. Fix it. Signed-off-by: Colin Ian King --- drivers/video/fbdev/omap2/omapfb/dss/venc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/omap2/omapfb/dss/venc.c b/drivers/video/fbdev/omap2/omapfb/dss/venc.c index 0b0ad20afd63..f560fa4d7786 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/venc.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/venc.c @@ -787,7 +787,7 @@ static int venc_probe_of(struct platform_device *pdev) venc.type = OMAP_DSS_VENC_TYPE_SVIDEO; break; default: - dev_err(>dev, "bad channel propert '%d'\n", channels); + dev_err(>dev, "bad channel property '%d'\n", channels); r = -EINVAL; goto err; } -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/omap: fix spelling mistake "propert" -> "property"
From: Colin Ian King There is a spelling mistake in a pr_err message. Fix it. Signed-off-by: Colin Ian King --- drivers/gpu/drm/omapdrm/dss/venc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c b/drivers/gpu/drm/omapdrm/dss/venc.c index e0817934ee16..0fa8f677b647 100644 --- a/drivers/gpu/drm/omapdrm/dss/venc.c +++ b/drivers/gpu/drm/omapdrm/dss/venc.c @@ -781,7 +781,7 @@ static int venc_probe_of(struct venc_device *venc) venc->type = OMAP_DSS_VENC_TYPE_SVIDEO; break; default: - dev_err(>pdev->dev, "bad channel propert '%d'\n", + dev_err(>pdev->dev, "bad channel property '%d'\n", channels); r = -EINVAL; goto err; -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM/KMS experimental drivers for HiKey 970
Em Wed, 5 Aug 2020 11:34:54 +0200 Daniel Vetter escreveu: > On Wed, Aug 5, 2020 at 10:51 AM Mauro Carvalho Chehab > wrote: > > > > Hi, > > > > I've been working to get upstream support for the DRM driver on HiKey 970. > > > > While the patches are not ready yet for upstream merge, I'm placing > > what I've sone so far on this repository: > > > > https://github.com/mchehab/linux/tree/hikey970/to_upstream-2.0-v1.1 > > > > The drivers there are a port from the Linaro's HiKey official Kernel: > > > > https://github.com/96boards-hikey/linux > > > > The patches there preserve the old history. The porting patches > > are applied after the original patches imported from that tree. > > > > Besides the DRM driver, this repository contains: > > > > - a PMIC/regulator driver; > > - an iommu driver (still requiring some changes at DT properties); > > - A DMA driver; > > - a small ugly hack reverting some PM changes at the WiFi driver, > > causing a regression on this board for HiKey 970. > > > > My current plans are to start upstreaming those needed drivers. > > > > The KMS/DRM driver there still need some changes. I guess it is > > not ready for being upstreamed yet. Also, it depends on the > > other drivers to work. > > > > In particular, its support for DPMS is broken: if the monitor is > > suspended, it won't return back to the right frequency settings. > > Hm this is somewhat surprising, at least with atomic, since DPMS as a > separate thing doesn't exist anymore - it's the same path as any other > modeset. With the suspend/resume helpers even the same as after > resume. But of course in reality there's always potential for some > differences between boot-up state and what your atomic_disable leaves > behind to creep in. Yeah, I didn't notice anything that would explain a problem there, but I didn't have much time so far to try to identify what is the real issue there. Btw, this problem is also present on the official Hikey Linaro's tree. There, it had an ugly hack at the modeset logic on adv7535 downstream driver. Ah, I forgot to mention, but this driver has a problem when talking with EDID. So, I'm passing this parameter via grub: drm_kms_helper.edid_firmware=edid/1920x1080.bin With the EDID info from the monitor I'm using at the tests. Perhaps the DPMS is somehow related to it. In any case, the modeset part of this driver needs to be revisited, before merging it drivers/drm. Btw, both issues are also present at the official Hikey driver, which makes a little harder to fix, as I was unable to get any documentation about this chipset so far - except for the public ones at 96boards.org. > > Just figured I drop this in quickly, always great to have more hw > drivers showing up! Yeah, it has been great for me to work on this DRM driver. Btw, although I didn't test, this driver is meant to support also Hikey 960. I intend to test it there when I have some spare time. Thanks, Mauro ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next-fixes
drm-misc-next-fixes-2020-08-05: drm-misc-next-fixes for v5.9-rc1: - Fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi - Fix a fbcon OOB read in fbdev, found by syzbot. - Mark vga_tryget static as it's not used elsewhere. - Small fixes to xlnx. - Remove null check for kfree in drm_dev_release. - Fix DRM_FORMAT_MOD_AMLOGIC_FBC definition. - Fix mode initialization in omap_connector_mode_valid(). The following changes since commit 206739119508d5ab4b42ab480ff61a7e6cd72d7c: Merge tag 'amd-drm-next-5.9-2020-07-17' of git://people.freedesktop.org/~agd5f/linux into drm-next (2020-07-23 15:38:11 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2020-08-05 for you to fetch changes up to a34a0a632dd991a371fec56431d73279f9c54029: drm: fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi (2020-08-04 12:21:11 -0400) drm-misc-next-fixes for v5.9-rc1: - Fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi - Fix a fbcon OOB read in fbdev, found by syzbot. - Mark vga_tryget static as it's not used elsewhere. - Small fixes to xlnx. - Remove null check for kfree in drm_dev_release. - Fix DRM_FORMAT_MOD_AMLOGIC_FBC definition. - Fix mode initialization in omap_connector_mode_valid(). Christoph Hellwig (1): vgaarb: mark vga_tryget static Colin Ian King (1): drm: xlnx: fix spelling mistake "failes" -> "failed" Hyun Kwon (1): drm: xlnx: zynqmp: Use switch - case for link rate downshift Li Heng (1): drm: Remove redundant NULL check Neil Armstrong (1): drm/fourcc: fix Amlogic Video Framebuffer Compression macro Tetsuo Handa (1): fbmem: pull fbcon_update_vcs() out of fb_set_var() Ville Syrjälä (1): drm/omap: Use {} to zero initialize the mode Wei Yongjun (1): drm: xlnx: Fix typo in parameter description Xin Xiong (1): drm: fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi drivers/gpu/drm/drm_dp_mst_topology.c| 7 --- drivers/gpu/drm/drm_drv.c| 3 +-- drivers/gpu/drm/omapdrm/omap_connector.c | 2 +- drivers/gpu/drm/xlnx/zynqmp_dp.c | 33 +--- drivers/gpu/vga/vgaarb.c | 3 +-- drivers/video/fbdev/core/fbmem.c | 8 ++-- drivers/video/fbdev/core/fbsysfs.c | 4 ++-- drivers/video/fbdev/ps3fb.c | 5 +++-- include/linux/fb.h | 2 -- include/linux/vgaarb.h | 6 -- include/uapi/drm/drm_fourcc.h| 2 +- 11 files changed, 33 insertions(+), 42 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: sysfs: Add to get current mode
On Wed, Aug 5, 2020 at 10:37 AM Sandy Huang wrote: > > add this node to get the current true mode. > > Signed-off-by: Sandy Huang Uh what's this for? Since it's sysfs, I guess there's something parsing this, which means we'd kinda need to have that piece too. If it's just for debugging purposes, then we already have this information in debugfs, together with everything else that's in the atomic modeset state. -Daniel > --- > drivers/gpu/drm/drm_sysfs.c | 30 ++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index 939f0032aab1..f39bcd34853b 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -19,6 +19,7 @@ > #include > > #include > +#include > #include > #include > #include > @@ -236,16 +237,45 @@ static ssize_t modes_show(struct device *device, > return written; > } > > +static ssize_t current_mode_show(struct device *device, > + struct device_attribute *attr, > + char *buf) > +{ > + struct drm_connector *connector = to_drm_connector(device); > + struct drm_display_mode *mode; > + struct drm_crtc_state *crtc_state; > + bool interlaced; > + int written = 0; > + > + if (!connector->state || !connector->state->crtc) > + return written; > + > + crtc_state = connector->state->crtc->state; > + if (!crtc_state) > + return written; > + > + mode = _state->mode; > + > + interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE); > + written += snprintf(buf + written, PAGE_SIZE - written, "%dx%d%s%d\n", > + mode->hdisplay, mode->vdisplay, > + interlaced ? "i" : "p", drm_mode_vrefresh(mode)); > + > + return written; > +} > + > static DEVICE_ATTR_RW(status); > static DEVICE_ATTR_RO(enabled); > static DEVICE_ATTR_RO(dpms); > static DEVICE_ATTR_RO(modes); > +static DEVICE_ATTR_RO(current_mode); > > static struct attribute *connector_dev_attrs[] = { > _attr_status.attr, > _attr_enabled.attr, > _attr_dpms.attr, > _attr_modes.attr, > + _attr_current_mode.attr, > NULL > }; > > -- > 2.17.1 > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM/KMS experimental drivers for HiKey 970
On Wed, Aug 5, 2020 at 10:51 AM Mauro Carvalho Chehab wrote: > > Hi, > > I've been working to get upstream support for the DRM driver on HiKey 970. > > While the patches are not ready yet for upstream merge, I'm placing > what I've sone so far on this repository: > > https://github.com/mchehab/linux/tree/hikey970/to_upstream-2.0-v1.1 > > The drivers there are a port from the Linaro's HiKey official Kernel: > > https://github.com/96boards-hikey/linux > > The patches there preserve the old history. The porting patches > are applied after the original patches imported from that tree. > > Besides the DRM driver, this repository contains: > > - a PMIC/regulator driver; > - an iommu driver (still requiring some changes at DT properties); > - A DMA driver; > - a small ugly hack reverting some PM changes at the WiFi driver, > causing a regression on this board for HiKey 970. > > My current plans are to start upstreaming those needed drivers. > > The KMS/DRM driver there still need some changes. I guess it is > not ready for being upstreamed yet. Also, it depends on the > other drivers to work. > > In particular, its support for DPMS is broken: if the monitor is > suspended, it won't return back to the right frequency settings. Hm this is somewhat surprising, at least with atomic, since DPMS as a separate thing doesn't exist anymore - it's the same path as any other modeset. With the suspend/resume helpers even the same as after resume. But of course in reality there's always potential for some differences between boot-up state and what your atomic_disable leaves behind to creep in. Just figured I drop this in quickly, always great to have more hw drivers showing up! Cheers, Daniel > Feel free to test it and send me patches fixing things there :-) > > Thanks, > Mauro > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 49/59] drm/vmwgfx/gmrid: convert to driver controlled allocation.
On Tue, Aug 04, 2020 at 12:56:22PM +1000, Dave Airlie wrote: > From: Dave Airlie > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 32 +++ > 1 file changed, 18 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > index 54c85a59dd8b..bc51b7773084 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > @@ -37,6 +37,7 @@ > #include > > struct vmwgfx_gmrid_man { > + struct ttm_mem_type_manager manager; > spinlock_t lock; > struct ida gmr_ida; > uint32_t max_gmr_ids; > @@ -44,13 +45,17 @@ struct vmwgfx_gmrid_man { > uint32_t used_gmr_pages; > }; > > +static struct vmwgfx_gmrid_man *to_gmrid_manager(struct ttm_mem_type_manager > *man) > +{ > + return container_of(man, struct vmwgfx_gmrid_man, manager); > +} > + > static int vmw_gmrid_man_get_node(struct ttm_mem_type_manager *man, > struct ttm_buffer_object *bo, > const struct ttm_place *place, > struct ttm_mem_reg *mem) > { > - struct vmwgfx_gmrid_man *gman = > - (struct vmwgfx_gmrid_man *)man->priv; > + struct vmwgfx_gmrid_man *gman = to_gmrid_manager(man); > int id; > > id = ida_alloc_max(>gmr_ida, gman->max_gmr_ids - 1, GFP_KERNEL); > @@ -82,8 +87,7 @@ static int vmw_gmrid_man_get_node(struct > ttm_mem_type_manager *man, > static void vmw_gmrid_man_put_node(struct ttm_mem_type_manager *man, > struct ttm_mem_reg *mem) > { > - struct vmwgfx_gmrid_man *gman = > - (struct vmwgfx_gmrid_man *)man->priv; > + struct vmwgfx_gmrid_man *gman = to_gmrid_manager(man); > > if (mem->mm_node) { > ida_free(>gmr_ida, mem->start); > @@ -98,13 +102,15 @@ static const struct ttm_mem_type_manager_func > vmw_gmrid_manager_func; > > int vmw_gmrid_man_init(struct vmw_private *dev_priv, int type) > { > - struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > type); > + struct ttm_mem_type_manager *man; > struct vmwgfx_gmrid_man *gman = > kzalloc(sizeof(*gman), GFP_KERNEL); > > if (unlikely(!gman)) > return -ENOMEM; > > + man = >manager; > + > man->func = _gmrid_manager_func; > man->available_caching = TTM_PL_FLAG_CACHED; > man->default_caching = TTM_PL_FLAG_CACHED; > @@ -127,8 +133,7 @@ int vmw_gmrid_man_init(struct vmw_private *dev_priv, int > type) > default: > BUG(); > } > - man->priv = (void *) gman; > - > + ttm_set_driver_manager(_priv->bdev, type, >manager); > ttm_mem_type_manager_set_used(man, true); > return 0; > } > @@ -136,19 +141,18 @@ int vmw_gmrid_man_init(struct vmw_private *dev_priv, > int type) > void vmw_gmrid_man_fini(struct vmw_private *dev_priv, int type) > { > struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > type); > - struct vmwgfx_gmrid_man *gman = > - (struct vmwgfx_gmrid_man *)man->priv; > + struct vmwgfx_gmrid_man *gman = to_gmrid_manager(man); > > ttm_mem_type_manager_disable(man); > > ttm_mem_type_manager_force_list_clean(_priv->bdev, man); > > - if (gman) { Ah, here it goes, please disregard my suggestion earlier for adding a WARN_ON, that's just churn. Reviewed-by: Daniel Vetter > - ida_destroy(>gmr_ida); > - kfree(gman); > - } > - > ttm_mem_type_manager_cleanup(man); > + > + ttm_set_driver_manager(_priv->bdev, type, NULL); > + ida_destroy(>gmr_ida); > + kfree(gman); > + > } > > static const struct ttm_mem_type_manager_func vmw_gmrid_manager_func = { > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 48/59] drm/vmwgfx/ttm: move thp to driver managed
On Tue, Aug 04, 2020 at 12:56:21PM +1000, Dave Airlie wrote: > From: Dave Airlie > > Signed-off-by: Dave Airlie Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 32 +++-- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > index 720a24214c74..1cefd9c1e8ea 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > @@ -16,10 +16,16 @@ > * @lock: Manager lock. > */ > struct vmw_thp_manager { > + struct ttm_mem_type_manager manager; > struct drm_mm mm; > spinlock_t lock; > }; > > +static struct vmw_thp_manager *to_thp_manager(struct ttm_mem_type_manager > *man) > +{ > + return container_of(man, struct vmw_thp_manager, manager); > +} > + > static int vmw_thp_insert_aligned(struct drm_mm *mm, struct drm_mm_node > *node, > unsigned long align_pages, > const struct ttm_place *place, > @@ -43,7 +49,7 @@ static int vmw_thp_get_node(struct ttm_mem_type_manager > *man, > const struct ttm_place *place, > struct ttm_mem_reg *mem) > { > - struct vmw_thp_manager *rman = (struct vmw_thp_manager *) man->priv; > + struct vmw_thp_manager *rman = to_thp_manager(man); > struct drm_mm *mm = >mm; > struct drm_mm_node *node; > unsigned long align_pages; > @@ -103,7 +109,7 @@ static int vmw_thp_get_node(struct ttm_mem_type_manager > *man, > static void vmw_thp_put_node(struct ttm_mem_type_manager *man, >struct ttm_mem_reg *mem) > { > - struct vmw_thp_manager *rman = (struct vmw_thp_manager *) man->priv; > + struct vmw_thp_manager *rman = to_thp_manager(man); > > if (mem->mm_node) { > spin_lock(>lock); > @@ -117,20 +123,24 @@ static void vmw_thp_put_node(struct > ttm_mem_type_manager *man, > > int vmw_thp_init(struct vmw_private *dev_priv) > { > - struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > TTM_PL_VRAM); > + struct ttm_mem_type_manager *man; > struct vmw_thp_manager *rman; > + > + rman = kzalloc(sizeof(*rman), GFP_KERNEL); > + if (!rman) > + return -ENOMEM; > + > + man = >manager; > man->available_caching = TTM_PL_FLAG_CACHED; > man->default_caching = TTM_PL_FLAG_CACHED; > > ttm_mem_type_manager_init(_priv->bdev, man, > dev_priv->vram_size >> PAGE_SHIFT); > - rman = kzalloc(sizeof(*rman), GFP_KERNEL); > - if (!rman) > - return -ENOMEM; > > drm_mm_init(>mm, 0, man->size); > spin_lock_init(>lock); > - man->priv = rman; > + > + ttm_set_driver_manager(_priv->bdev, TTM_PL_VRAM, >manager); > ttm_mem_type_manager_set_used(man, true); > return 0; > } > @@ -138,7 +148,7 @@ int vmw_thp_init(struct vmw_private *dev_priv) > void vmw_thp_fini(struct vmw_private *dev_priv) > { > struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > TTM_PL_VRAM); > - struct vmw_thp_manager *rman = (struct vmw_thp_manager *) man->priv; > + struct vmw_thp_manager *rman = to_thp_manager(man); > struct drm_mm *mm = >mm; > int ret; > > @@ -151,15 +161,15 @@ void vmw_thp_fini(struct vmw_private *dev_priv) > drm_mm_clean(mm); > drm_mm_takedown(mm); > spin_unlock(>lock); > - kfree(rman); > - man->priv = NULL; > ttm_mem_type_manager_cleanup(man); > + ttm_set_driver_manager(_priv->bdev, TTM_PL_VRAM, NULL); > + kfree(rman); > } > > static void vmw_thp_debug(struct ttm_mem_type_manager *man, > struct drm_printer *printer) > { > - struct vmw_thp_manager *rman = (struct vmw_thp_manager *) man->priv; > + struct vmw_thp_manager *rman = to_thp_manager(man); > > spin_lock(>lock); > drm_mm_print(>mm, printer); > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 42/59] drm/vmwgfx/ttm: use wrapper to access memory manager
On Tue, Aug 04, 2020 at 12:56:15PM +1000, Dave Airlie wrote: > From: Dave Airlie > > Signed-off-by: Dave Airlie Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 23 +++ > drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 4 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 4 ++-- > 3 files changed, 18 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index f368a9cc0c2a..6ed92f38b54b 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -634,7 +634,7 @@ static int vmw_vram_manager_init(struct vmw_private > *dev_priv) > ret = ttm_range_man_init(_priv->bdev, man, >dev_priv->vram_size >> PAGE_SHIFT); > #endif > - dev_priv->bdev.man[TTM_PL_VRAM].use_type = false; > + ttm_manager_type(_priv->bdev, TTM_PL_VRAM)->use_type = false; > return ret; > } > > @@ -644,7 +644,7 @@ static void vmw_vram_manager_fini(struct vmw_private > *dev_priv) > vmw_thp_fini(dev_priv); > #else > ttm_bo_man_fini(_priv->bdev, > - _priv->bdev.man[TTM_PL_VRAM]); > + ttm_manager_type(_priv->bdev, TTM_PL_VRAM)); > #endif > } > > @@ -887,7 +887,7 @@ static int vmw_driver_load(struct drm_device *dev, > unsigned long chipset) > DRM_ERROR("Failed initializing TTM buffer object driver.\n"); > goto out_no_bdev; > } > - dev_priv->bdev.man[TTM_PL_SYSTEM].available_caching = > + ttm_manager_type(_priv->bdev, TTM_PL_SYSTEM)->available_caching = > TTM_PL_FLAG_CACHED; > > /* > @@ -1194,10 +1194,12 @@ static void vmw_master_drop(struct drm_device *dev, > */ > static void __vmw_svga_enable(struct vmw_private *dev_priv) > { > + struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > TTM_PL_VRAM); > + > spin_lock(_priv->svga_lock); > - if (!dev_priv->bdev.man[TTM_PL_VRAM].use_type) { > + if (!man->use_type) { > vmw_write(dev_priv, SVGA_REG_ENABLE, SVGA_REG_ENABLE); > - dev_priv->bdev.man[TTM_PL_VRAM].use_type = true; > + man->use_type = true; > } > spin_unlock(_priv->svga_lock); > } > @@ -1223,9 +1225,11 @@ void vmw_svga_enable(struct vmw_private *dev_priv) > */ > static void __vmw_svga_disable(struct vmw_private *dev_priv) > { > + struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > TTM_PL_VRAM); > + > spin_lock(_priv->svga_lock); > - if (dev_priv->bdev.man[TTM_PL_VRAM].use_type) { > - dev_priv->bdev.man[TTM_PL_VRAM].use_type = false; > + if (man->use_type) { > + man->use_type = false; > vmw_write(dev_priv, SVGA_REG_ENABLE, > SVGA_REG_ENABLE_HIDE | > SVGA_REG_ENABLE_ENABLE); > @@ -1242,6 +1246,7 @@ static void __vmw_svga_disable(struct vmw_private > *dev_priv) > */ > void vmw_svga_disable(struct vmw_private *dev_priv) > { > + struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > TTM_PL_VRAM); > /* >* Disabling SVGA will turn off device modesetting capabilities, so >* notify KMS about that so that it doesn't cache atomic state that > @@ -1257,8 +1262,8 @@ void vmw_svga_disable(struct vmw_private *dev_priv) > vmw_kms_lost_device(dev_priv->dev); > ttm_write_lock(_priv->reservation_sem, false); > spin_lock(_priv->svga_lock); > - if (dev_priv->bdev.man[TTM_PL_VRAM].use_type) { > - dev_priv->bdev.man[TTM_PL_VRAM].use_type = false; > + if (man->use_type) { > + man->use_type = false; > spin_unlock(_priv->svga_lock); > if (ttm_bo_evict_mm(_priv->bdev, TTM_PL_VRAM)) > DRM_ERROR("Failed evicting VRAM buffers.\n"); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > index ec1b5bb01a93..54c85a59dd8b 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > @@ -98,7 +98,7 @@ static const struct ttm_mem_type_manager_func > vmw_gmrid_manager_func; > > int vmw_gmrid_man_init(struct vmw_private *dev_priv, int type) > { > - struct ttm_mem_type_manager *man = _priv->bdev.man[type]; > + struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > type); > struct vmwgfx_gmrid_man *gman = > kzalloc(sizeof(*gman), GFP_KERNEL); > > @@ -135,7 +135,7 @@ int vmw_gmrid_man_init(struct vmw_private *dev_priv, int > type) > > void vmw_gmrid_man_fini(struct vmw_private *dev_priv, int type) > { > - struct ttm_mem_type_manager *man = _priv->bdev.man[type]; > + struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > type); > struct vmwgfx_gmrid_man *gman = > (struct
Re: [PATCH 33/59] drm/vmwgfx: fix gmrid takedown paths to new interface
On Tue, Aug 04, 2020 at 12:56:06PM +1000, Dave Airlie wrote: > From: Dave Airlie > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 9 - > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 + > drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 11 --- > 3 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index 4f4d22bac477..f368a9cc0c2a 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -996,9 +996,9 @@ static int vmw_driver_load(struct drm_device *dev, > unsigned long chipset) > vmw_kms_close(dev_priv); > out_no_kms: > if (dev_priv->has_mob) > - (void) ttm_bo_clean_mm(_priv->bdev, VMW_PL_MOB); > + vmw_gmrid_man_fini(dev_priv, VMW_PL_MOB); > if (dev_priv->has_gmr) > - (void) ttm_bo_clean_mm(_priv->bdev, VMW_PL_GMR); > + vmw_gmrid_man_fini(dev_priv, VMW_PL_GMR); > vmw_vram_manager_fini(dev_priv); > out_no_vram: > (void)ttm_bo_device_release(_priv->bdev); > @@ -1047,12 +1047,11 @@ static void vmw_driver_unload(struct drm_device *dev) > vmw_overlay_close(dev_priv); > > if (dev_priv->has_gmr) > - (void)ttm_bo_clean_mm(_priv->bdev, VMW_PL_GMR); > - (void)ttm_bo_clean_mm(_priv->bdev, TTM_PL_VRAM); > + vmw_gmrid_man_fini(dev_priv, VMW_PL_GMR); > > vmw_release_device_early(dev_priv); > if (dev_priv->has_mob) > - (void) ttm_bo_clean_mm(_priv->bdev, VMW_PL_MOB); > + vmw_gmrid_man_fini(dev_priv, VMW_PL_MOB); > vmw_vram_manager_fini(dev_priv); > (void) ttm_bo_device_release(_priv->bdev); > drm_vma_offset_manager_destroy(_priv->vma_manager); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index c6530d7b6d51..aa763c6b1146 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > @@ -1222,6 +1222,7 @@ int vmw_overlay_num_free_overlays(struct vmw_private > *dev_priv); > */ > > int vmw_gmrid_man_init(struct vmw_private *dev_priv, int type); > +void vmw_gmrid_man_fini(struct vmw_private *dev_priv, int type); > > /** > * Prime - vmwgfx_prime.c > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > index 141fb14e3583..ec1b5bb01a93 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > @@ -133,20 +133,25 @@ int vmw_gmrid_man_init(struct vmw_private *dev_priv, > int type) > return 0; > } > > -static int vmw_gmrid_man_takedown(struct ttm_mem_type_manager *man) > +void vmw_gmrid_man_fini(struct vmw_private *dev_priv, int type) > { > + struct ttm_mem_type_manager *man = _priv->bdev.man[type]; > struct vmwgfx_gmrid_man *gman = > (struct vmwgfx_gmrid_man *)man->priv; > > + ttm_mem_type_manager_disable(man); > + > + ttm_mem_type_manager_force_list_clean(_priv->bdev, man); > + > if (gman) { I don't think this can ever be non-NULL if init worked, not after the demidlayering. Maybe put a WARN_ON(!gman) in here. With that: Reviewed-by: Daniel Vetter > ida_destroy(>gmr_ida); > kfree(gman); > } > - return 0; > + > + ttm_mem_type_manager_cleanup(man); > } > > static const struct ttm_mem_type_manager_func vmw_gmrid_manager_func = { > - .takedown = vmw_gmrid_man_takedown, > .get_node = vmw_gmrid_man_get_node, > .put_node = vmw_gmrid_man_put_node, > }; > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 28/59] drm/vmwgfx: takedown vram manager
On Tue, Aug 04, 2020 at 12:56:01PM +1000, Dave Airlie wrote: > From: Dave Airlie > > Don't bother returning EBUSY, nobody cares enough, > if the driver has a problem, it should deal with it. > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 14 +- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 + > drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 23 +-- > 3 files changed, 27 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index 364d5f3ff9a8..4f4d22bac477 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -637,6 +637,17 @@ static int vmw_vram_manager_init(struct vmw_private > *dev_priv) > dev_priv->bdev.man[TTM_PL_VRAM].use_type = false; > return ret; > } > + > +static void vmw_vram_manager_fini(struct vmw_private *dev_priv) > +{ > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > + vmw_thp_fini(dev_priv); > +#else > + ttm_bo_man_fini(_priv->bdev, > + _priv->bdev.man[TTM_PL_VRAM]); > +#endif > +} > + > static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) > { > struct vmw_private *dev_priv; > @@ -988,7 +999,7 @@ static int vmw_driver_load(struct drm_device *dev, > unsigned long chipset) > (void) ttm_bo_clean_mm(_priv->bdev, VMW_PL_MOB); > if (dev_priv->has_gmr) > (void) ttm_bo_clean_mm(_priv->bdev, VMW_PL_GMR); > - (void)ttm_bo_clean_mm(_priv->bdev, TTM_PL_VRAM); > + vmw_vram_manager_fini(dev_priv); > out_no_vram: > (void)ttm_bo_device_release(_priv->bdev); > out_no_bdev: > @@ -1042,6 +1053,7 @@ static void vmw_driver_unload(struct drm_device *dev) > vmw_release_device_early(dev_priv); > if (dev_priv->has_mob) > (void) ttm_bo_clean_mm(_priv->bdev, VMW_PL_MOB); > + vmw_vram_manager_fini(dev_priv); I think drmm_ would be neat for all this stuff eventually, but we're still in the very early phases of rolling that out. > (void) ttm_bo_device_release(_priv->bdev); > drm_vma_offset_manager_destroy(_priv->vma_manager); > vmw_release_device_late(dev_priv); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index 8f319dd6cdb4..c6530d7b6d51 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > @@ -1521,6 +1521,7 @@ vm_fault_t vmw_bo_vm_huge_fault(struct vm_fault *vmf, > /* Transparent hugepage support - vmwgfx_thp.c */ > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > extern int vmw_thp_init(struct vmw_private *dev_priv); > +void vmw_thp_fini(struct vmw_private *dev_priv); > #endif > > /** > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > index 0292c931c265..548f152b9963 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > @@ -135,21 +135,25 @@ int vmw_thp_init(struct vmw_private *dev_priv) > return 0; > } > > -static int vmw_thp_takedown(struct ttm_mem_type_manager *man) > +void vmw_thp_fini(struct vmw_private *dev_priv) > { > + struct ttm_mem_type_manager *man = _priv->bdev.man[TTM_PL_VRAM]; > struct vmw_thp_manager *rman = (struct vmw_thp_manager *) man->priv; > struct drm_mm *mm = >mm; > + int ret; > + > + ttm_mem_type_manager_disable(man); > > + ret = ttm_mem_type_manager_force_list_clean(_priv->bdev, man); > + if (ret) > + return; > spin_lock(>lock); > - if (drm_mm_clean(mm)) { > - drm_mm_takedown(mm); > - spin_unlock(>lock); > - kfree(rman); > - man->priv = NULL; > - return 0; > - } > + drm_mm_clean(mm); > + drm_mm_takedown(mm); > spin_unlock(>lock); > - return -EBUSY; > + kfree(rman); > + man->priv = NULL; > + ttm_mem_type_manager_cleanup(man); > } > > static void vmw_thp_debug(struct ttm_mem_type_manager *man, > @@ -163,7 +167,6 @@ static void vmw_thp_debug(struct ttm_mem_type_manager > *man, > } > > const struct ttm_mem_type_manager_func vmw_thp_func = { > - .takedown = vmw_thp_takedown, > .get_node = vmw_thp_get_node, > .put_node = vmw_thp_put_node, > .debug = vmw_thp_debug Still the mildly icky #ifdef, but looks good to me. Reviewed-by: Daniel Vetter > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 56/59] drm/ttm: add a wrapper for checking if manager is in use
On Tue, Aug 04, 2020 at 12:56:29PM +1000, Dave Airlie wrote: > From: Dave Airlie > > This converts vmwgfx over to using an interface to set the > in use and check the in use flag. > > Signed-off-by: Dave Airlie Hm, I think this would be a good candidate to eventually rip out once we have full sub-classing, since it's for vmwgfx internally only. Maybe add a todo to the kernel-doc. -Daniel > --- > drivers/gpu/drm/nouveau/nouveau_ttm.c | 1 - > drivers/gpu/drm/ttm/ttm_bo.c | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 14 +++--- > include/drm/ttm/ttm_bo_driver.h | 14 ++ > 4 files changed, 22 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c > b/drivers/gpu/drm/nouveau/nouveau_ttm.c > index 22185a8dcfa1..38a0e4bd16f7 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c > @@ -240,7 +240,6 @@ nouveau_ttm_init_gtt(struct nouveau_drm *drm) > ttm_mem_type_manager_init(man, size_pages); > ttm_set_driver_manager(>ttm.bdev, TTM_PL_TT, man); > ttm_mem_type_manager_set_used(man, true); > - > return 0; > } > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index cda33b4af681..7d10abae9a60 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -1002,7 +1002,7 @@ static int ttm_bo_mem_placement(struct > ttm_buffer_object *bo, > return ret; > > man = ttm_manager_type(bdev, mem_type); > - if (!man || !man->use_type) > + if (!man || !ttm_mem_type_manager_used(man)) > return -EBUSY; > > if (!ttm_bo_mt_compatible(man, mem_type, place, _flags)) > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index 7168403fb4f8..b2f1e7a3b048 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -630,7 +630,7 @@ static int vmw_vram_manager_init(struct vmw_private > *dev_priv) >TTM_PL_FLAG_CACHED, TTM_PL_FLAG_CACHED, >false, dev_priv->vram_size >> PAGE_SHIFT); > #endif > - ttm_manager_type(_priv->bdev, TTM_PL_VRAM)->use_type = false; > + ttm_mem_type_manager_set_used(ttm_manager_type(_priv->bdev, > TTM_PL_VRAM), false); > return ret; > } > > @@ -1192,9 +1192,9 @@ static void __vmw_svga_enable(struct vmw_private > *dev_priv) > struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > TTM_PL_VRAM); > > spin_lock(_priv->svga_lock); > - if (!man->use_type) { > + if (!ttm_mem_type_manager_used(man)) { > vmw_write(dev_priv, SVGA_REG_ENABLE, SVGA_REG_ENABLE); > - man->use_type = true; > + ttm_mem_type_manager_set_used(man, true); > } > spin_unlock(_priv->svga_lock); > } > @@ -1223,8 +1223,8 @@ static void __vmw_svga_disable(struct vmw_private > *dev_priv) > struct ttm_mem_type_manager *man = ttm_manager_type(_priv->bdev, > TTM_PL_VRAM); > > spin_lock(_priv->svga_lock); > - if (man->use_type) { > - man->use_type = false; > + if (ttm_mem_type_manager_used(man)) { > + ttm_mem_type_manager_set_used(man, false); > vmw_write(dev_priv, SVGA_REG_ENABLE, > SVGA_REG_ENABLE_HIDE | > SVGA_REG_ENABLE_ENABLE); > @@ -1257,8 +1257,8 @@ void vmw_svga_disable(struct vmw_private *dev_priv) > vmw_kms_lost_device(dev_priv->dev); > ttm_write_lock(_priv->reservation_sem, false); > spin_lock(_priv->svga_lock); > - if (man->use_type) { > - man->use_type = false; > + if (ttm_mem_type_manager_used(man)) { > + ttm_mem_type_manager_set_used(man, false); > spin_unlock(_priv->svga_lock); > if (ttm_bo_evict_mm(_priv->bdev, TTM_PL_VRAM)) > DRM_ERROR("Failed evicting VRAM buffers.\n"); > diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h > index 300934289e64..f231fe34e744 100644 > --- a/include/drm/ttm/ttm_bo_driver.h > +++ b/include/drm/ttm/ttm_bo_driver.h > @@ -678,6 +678,20 @@ static inline void ttm_mem_type_manager_set_used(struct > ttm_mem_type_manager *ma > man->use_type = used; > } > > +/** > + * ttm_mem_type_manager_used > + * > + * @man: Manager to get used state for > + * > + * Get the in use flag for a manager. > + * Returns: > + * true is used, false if not. > + */ > +static inline bool ttm_mem_type_manager_used(struct ttm_mem_type_manager > *man) > +{ > + return man->use_type; > +} > + > /** > * ttm_mem_type_manager_cleanup > * > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 21/59] drm/vmwgfx/ttm: switch gmrid allocator to new init paths.
On Tue, Aug 04, 2020 at 12:55:54PM +1000, Dave Airlie wrote: > From: Dave Airlie > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 17 --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 21 --- > 3 files changed, 19 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index 5a889df2de03..364d5f3ff9a8 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -896,14 +896,10 @@ static int vmw_driver_load(struct drm_device *dev, > unsigned long chipset) >* slots as well as the bo size. >*/ > dev_priv->has_gmr = true; > - dev_priv->bdev.man[VMW_PL_GMR].func = _gmrid_manager_func; > - dev_priv->bdev.man[VMW_PL_GMR].available_caching = TTM_PL_FLAG_CACHED; > - dev_priv->bdev.man[VMW_PL_GMR].default_caching = TTM_PL_FLAG_CACHED; > /* TODO: This is most likely not correct */ > - dev_priv->bdev.man[VMW_PL_GMR].use_tt = true; > if (((dev_priv->capabilities & (SVGA_CAP_GMR | SVGA_CAP_GMR2)) == 0) || > - refuse_dma || ttm_bo_init_mm(_priv->bdev, VMW_PL_GMR, > - VMW_PL_GMR) != 0) { > + refuse_dma || > + vmw_gmrid_man_init(dev_priv, VMW_PL_GMR) != 0) { > DRM_INFO("No GMR memory available. " >"Graphics memory resources are very limited.\n"); > dev_priv->has_gmr = false; > @@ -911,13 +907,8 @@ static int vmw_driver_load(struct drm_device *dev, > unsigned long chipset) > > if (dev_priv->capabilities & SVGA_CAP_GBOBJECTS && !refuse_dma) { > dev_priv->has_mob = true; > - dev_priv->bdev.man[VMW_PL_MOB].func = _gmrid_manager_func; > - dev_priv->bdev.man[VMW_PL_MOB].available_caching = > TTM_PL_FLAG_CACHED; > - dev_priv->bdev.man[VMW_PL_MOB].default_caching = > TTM_PL_FLAG_CACHED; > - /* TODO: This is most likely not correct */ > - dev_priv->bdev.man[VMW_PL_MOB].use_tt = true; > - if (ttm_bo_init_mm(_priv->bdev, VMW_PL_MOB, > -VMW_PL_MOB) != 0) { > + > + if (vmw_gmrid_man_init(dev_priv, VMW_PL_MOB) != 0) { > DRM_INFO("No MOB memory available. " >"3D will be disabled.\n"); > dev_priv->has_mob = false; > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index 10b681725a53..8f319dd6cdb4 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > @@ -1221,7 +1221,7 @@ int vmw_overlay_num_free_overlays(struct vmw_private > *dev_priv); > * GMR Id manager > */ > > -extern const struct ttm_mem_type_manager_func vmw_gmrid_manager_func; > +int vmw_gmrid_man_init(struct vmw_private *dev_priv, int type); > > /** > * Prime - vmwgfx_prime.c > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > index fb1bf4dd91d1..141fb14e3583 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > @@ -94,22 +94,28 @@ static void vmw_gmrid_man_put_node(struct > ttm_mem_type_manager *man, > } > } > > -static int vmw_gmrid_man_init(struct ttm_mem_type_manager *man, > - unsigned long p_size) > +static const struct ttm_mem_type_manager_func vmw_gmrid_manager_func; > + > +int vmw_gmrid_man_init(struct vmw_private *dev_priv, int type) > { > - struct vmw_private *dev_priv = > - container_of(man->bdev, struct vmw_private, bdev); > + struct ttm_mem_type_manager *man = _priv->bdev.man[type]; > struct vmwgfx_gmrid_man *gman = > kzalloc(sizeof(*gman), GFP_KERNEL); > > if (unlikely(!gman)) > return -ENOMEM; > > + man->func = _gmrid_manager_func; > + man->available_caching = TTM_PL_FLAG_CACHED; > + man->default_caching = TTM_PL_FLAG_CACHED; > + /* TODO: This is most likely not correct */ > + man->use_tt = true; > + ttm_mem_type_manager_init(_priv->bdev, man, 0); > spin_lock_init(>lock); > gman->used_gmr_pages = 0; > ida_init(>gmr_ida); > > - switch (p_size) { > + switch (type) { > case VMW_PL_GMR: > gman->max_gmr_ids = dev_priv->max_gmr_ids; > gman->max_gmr_pages = dev_priv->max_gmr_pages; > @@ -122,6 +128,8 @@ static int vmw_gmrid_man_init(struct ttm_mem_type_manager > *man, > BUG(); > } > man->priv = (void *) gman; > + > + ttm_mem_type_manager_set_used(man, true); > return 0; > } > > @@ -137,8 +145,7 @@ static int vmw_gmrid_man_takedown(struct > ttm_mem_type_manager *man) > return 0; > } > > -const struct
Re: [PATCH 20/59] drm/vmwgfx/ttm: convert vram mm init to new code paths
On Tue, Aug 04, 2020 at 12:55:53PM +1000, Dave Airlie wrote: > From: Dave Airlie > > Split out the vram thp init path vs the range manager init. > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 25 +++-- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 4 +--- > drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 12 > 3 files changed, 28 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index 1e4c2c6119c3..5a889df2de03 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -620,6 +620,23 @@ static int vmw_dma_masks(struct vmw_private *dev_priv) > return ret; > } > > +static int vmw_vram_manager_init(struct vmw_private *dev_priv) > +{ > + int ret; > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > + ret = vmw_thp_init(dev_priv); > +#else > + struct ttm_mem_type_manager *man = _priv->bdev.man[TTM_PL_VRAM]; > + > + man->available_caching = TTM_PL_FLAG_CACHED; > + man->default_caching = TTM_PL_FLAG_CACHED; > + > + ret = ttm_range_man_init(_priv->bdev, man, > + dev_priv->vram_size >> PAGE_SHIFT); > +#endif The ifdeffery here looks a bit funny, but not really less convoluted than the old one I think. Reviewed-by: Daniel Vetter > + dev_priv->bdev.man[TTM_PL_VRAM].use_type = false; > + return ret; > +} > static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) > { > struct vmw_private *dev_priv; > @@ -866,16 +883,12 @@ static int vmw_driver_load(struct drm_device *dev, > unsigned long chipset) >* Enable VRAM, but initially don't use it until SVGA is enabled and >* unhidden. >*/ > - dev_priv->bdev.man[TTM_PL_VRAM].func = _thp_func; > - dev_priv->bdev.man[TTM_PL_VRAM].available_caching = TTM_PL_FLAG_CACHED; > - dev_priv->bdev.man[TTM_PL_VRAM].default_caching = TTM_PL_FLAG_CACHED; > - ret = ttm_bo_init_mm(_priv->bdev, TTM_PL_VRAM, > - (dev_priv->vram_size >> PAGE_SHIFT)); > + > + ret = vmw_vram_manager_init(dev_priv); > if (unlikely(ret != 0)) { > DRM_ERROR("Failed initializing memory manager for VRAM.\n"); > goto out_no_vram; > } > - dev_priv->bdev.man[TTM_PL_VRAM].use_type = false; > > /* >* "Guest Memory Regions" is an aperture like feature with > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index 65c414f119c0..10b681725a53 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > @@ -1520,9 +1520,7 @@ vm_fault_t vmw_bo_vm_huge_fault(struct vm_fault *vmf, > > /* Transparent hugepage support - vmwgfx_thp.c */ > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > -extern const struct ttm_mem_type_manager_func vmw_thp_func; > -#else > -#define vmw_thp_func ttm_bo_manager_func > +extern int vmw_thp_init(struct vmw_private *dev_priv); > #endif > > /** > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > index b7c816ba7166..0292c931c265 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > @@ -115,18 +115,23 @@ static void vmw_thp_put_node(struct > ttm_mem_type_manager *man, > } > } > > -static int vmw_thp_init(struct ttm_mem_type_manager *man, > - unsigned long p_size) > +int vmw_thp_init(struct vmw_private *dev_priv) > { > + struct ttm_mem_type_manager *man = _priv->bdev.man[TTM_PL_VRAM]; > struct vmw_thp_manager *rman; > + man->available_caching = TTM_PL_FLAG_CACHED; > + man->default_caching = TTM_PL_FLAG_CACHED; > > + ttm_mem_type_manager_init(_priv->bdev, man, > + dev_priv->vram_size >> PAGE_SHIFT); > rman = kzalloc(sizeof(*rman), GFP_KERNEL); > if (!rman) > return -ENOMEM; > > - drm_mm_init(>mm, 0, p_size); > + drm_mm_init(>mm, 0, man->size); > spin_lock_init(>lock); > man->priv = rman; > + ttm_mem_type_manager_set_used(man, true); > return 0; > } > > @@ -158,7 +163,6 @@ static void vmw_thp_debug(struct ttm_mem_type_manager > *man, > } > > const struct ttm_mem_type_manager_func vmw_thp_func = { > - .init = vmw_thp_init, > .takedown = vmw_thp_takedown, > .get_node = vmw_thp_get_node, > .put_node = vmw_thp_put_node, > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
DRM/KMS experimental drivers for HiKey 970
Hi, I've been working to get upstream support for the DRM driver on HiKey 970. While the patches are not ready yet for upstream merge, I'm placing what I've sone so far on this repository: https://github.com/mchehab/linux/tree/hikey970/to_upstream-2.0-v1.1 The drivers there are a port from the Linaro's HiKey official Kernel: https://github.com/96boards-hikey/linux The patches there preserve the old history. The porting patches are applied after the original patches imported from that tree. Besides the DRM driver, this repository contains: - a PMIC/regulator driver; - an iommu driver (still requiring some changes at DT properties); - A DMA driver; - a small ugly hack reverting some PM changes at the WiFi driver, causing a regression on this board for HiKey 970. My current plans are to start upstreaming those needed drivers. The KMS/DRM driver there still need some changes. I guess it is not ready for being upstreamed yet. Also, it depends on the other drivers to work. In particular, its support for DPMS is broken: if the monitor is suspended, it won't return back to the right frequency settings. Feel free to test it and send me patches fixing things there :-) Thanks, Mauro ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Aw: Re: Re: [PATCH v4 6/6] arm: dts: mt7623: add display subsystem related device nodes
On Wed, 2020-08-05 at 09:27 +0200, Frank Wunderlich wrote: > or should we split dtsi to have a common part (mt7623.dtsi), and one for > soc (mt7623n.dtsi/mt7623a.dtsi)? > > mt7623.dtsi => mt7623n.dtsi => mt7623n-bananapi-bpi-r2.dts > mt7623.dtsi => mt7623a.dtsi => mt7623a-unielec-u7623.dts (not existing yet, > openwrt seems to use a board-specific dtsi) Yes, I think we should. I'll create mt7623a.dtsi and upstream the U7623 support; I think that can happen without conflicting with anything you do. I note that the GPU node has been added to mt7623.dtsi in 5.8 too; that'll want to move to the new mt7623n.dtsi that you create, along with your other new additions. Does that seem reasonable? smime.p7s Description: S/MIME cryptographic signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/59] drm/vmwgfx: drop bo map/unmap dma functions.
On Tue, Aug 04, 2020 at 12:55:35PM +1000, Dave Airlie wrote: > From: Dave Airlie > > The map one was used once, just inline it, and drop them both. > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h| 2 - > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 46 +++--- > 2 files changed, 6 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index b7c763713b4c..65c414f119c0 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > @@ -1019,8 +1019,6 @@ extern struct ttm_placement vmw_mob_placement; > extern struct ttm_placement vmw_mob_ne_placement; > extern struct ttm_placement vmw_nonfixed_placement; > extern struct ttm_bo_driver vmw_bo_driver; > -extern int vmw_bo_map_dma(struct ttm_buffer_object *bo); > -extern void vmw_bo_unmap_dma(struct ttm_buffer_object *bo); > extern const struct vmw_sg_table * > vmw_bo_sg_table(struct ttm_buffer_object *bo); > extern int vmw_bo_create_and_populate(struct vmw_private *dev_priv, > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c > index 8e2a82ded900..3a141a08d4bd 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c > @@ -519,43 +519,6 @@ static void vmw_ttm_unmap_dma(struct vmw_ttm_tt *vmw_tt) > vmw_tt->mapped = false; > } > > - > -/** > - * vmw_bo_map_dma - Make sure buffer object pages are visible to the device > - * > - * @bo: Pointer to a struct ttm_buffer_object > - * > - * Wrapper around vmw_ttm_map_dma, that takes a TTM buffer object pointer > - * instead of a pointer to a struct vmw_ttm_backend as argument. > - * Note that the buffer object must be either pinned or reserved before > - * calling this function. > - */ > -int vmw_bo_map_dma(struct ttm_buffer_object *bo) > -{ > - struct vmw_ttm_tt *vmw_tt = > - container_of(bo->ttm, struct vmw_ttm_tt, dma_ttm.ttm); > - > - return vmw_ttm_map_dma(vmw_tt); > -} > - > - > -/** > - * vmw_bo_unmap_dma - Make sure buffer object pages are visible to the device > - * > - * @bo: Pointer to a struct ttm_buffer_object > - * > - * Wrapper around vmw_ttm_unmap_dma, that takes a TTM buffer object pointer > - * instead of a pointer to a struct vmw_ttm_backend as argument. > - */ > -void vmw_bo_unmap_dma(struct ttm_buffer_object *bo) > -{ > - struct vmw_ttm_tt *vmw_tt = > - container_of(bo->ttm, struct vmw_ttm_tt, dma_ttm.ttm); > - > - vmw_ttm_unmap_dma(vmw_tt); > -} > - > - > /** > * vmw_bo_sg_table - Return a struct vmw_sg_table object for a > * TTM buffer object > @@ -839,9 +802,12 @@ int vmw_bo_create_and_populate(struct vmw_private > *dev_priv, > > ret = ttm_bo_reserve(bo, false, true, NULL); > BUG_ON(ret != 0); > - ret = vmw_bo_driver.ttm_tt_populate(bo->ttm, ); > - if (likely(ret == 0)) > - ret = vmw_bo_map_dma(bo); > + ret = vmw_ttm_populate(bo->ttm, ); Snuck a replacement for a vfunc call with a direct all in here, maybe mention that in the commit message. Reviewed-by: Daniel Vetter > + if (likely(ret == 0)) { > + struct vmw_ttm_tt *vmw_tt = > + container_of(bo->ttm, struct vmw_ttm_tt, dma_ttm.ttm); > + ret = vmw_ttm_map_dma(vmw_tt); > + } > > ttm_bo_unreserve(bo); > > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/59] drm/vmwgfx: consolidate ttm object creation and populate
On Tue, Aug 04, 2020 at 12:55:34PM +1000, Dave Airlie wrote: > From: Dave Airlie > > These two functions has the same code in them, create a common > helper function instead. > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h| 4 ++ > drivers/gpu/drm/vmwgfx/vmwgfx_mob.c| 60 ++ > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 32 > 3 files changed, 39 insertions(+), 57 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index 3596f3923ea3..b7c763713b4c 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > @@ -1023,6 +1023,10 @@ extern int vmw_bo_map_dma(struct ttm_buffer_object > *bo); > extern void vmw_bo_unmap_dma(struct ttm_buffer_object *bo); > extern const struct vmw_sg_table * > vmw_bo_sg_table(struct ttm_buffer_object *bo); > +extern int vmw_bo_create_and_populate(struct vmw_private *dev_priv, > + unsigned long bo_size, > + struct ttm_buffer_object **bo_p); > + > extern void vmw_piter_start(struct vmw_piter *viter, > const struct vmw_sg_table *vsgt, > unsigned long p_offs); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c > index e8eb42933ca2..7f95ed6aa224 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c > @@ -238,10 +238,6 @@ static int vmw_otable_batch_setup(struct vmw_private > *dev_priv, > unsigned long offset; > unsigned long bo_size; > struct vmw_otable *otables = batch->otables; > - struct ttm_operation_ctx ctx = { > - .interruptible = false, > - .no_wait_gpu = false > - }; > SVGAOTableType i; > int ret; > > @@ -255,24 +251,9 @@ static int vmw_otable_batch_setup(struct vmw_private > *dev_priv, > bo_size += otables[i].size; > } > > - ret = ttm_bo_create(_priv->bdev, bo_size, > - ttm_bo_type_device, > - _sys_ne_placement, > - 0, false, >otable_bo); > - > - if (unlikely(ret != 0)) > - goto out_no_bo; > - > - ret = ttm_bo_reserve(batch->otable_bo, false, true, NULL); > - BUG_ON(ret != 0); > - ret = vmw_bo_driver.ttm_tt_populate(batch->otable_bo->ttm, ); > - if (unlikely(ret != 0)) > - goto out_unreserve; > - ret = vmw_bo_map_dma(batch->otable_bo); > + ret = vmw_bo_create_and_populate(dev_priv, bo_size, >otable_bo); > if (unlikely(ret != 0)) > - goto out_unreserve; > - > - ttm_bo_unreserve(batch->otable_bo); > + return ret; > > offset = 0; > for (i = 0; i < batch->num_otables; ++i) { > @@ -289,8 +270,6 @@ static int vmw_otable_batch_setup(struct vmw_private > *dev_priv, > > return 0; > > -out_unreserve: > - ttm_bo_unreserve(batch->otable_bo); > out_no_setup: > for (i = 0; i < batch->num_otables; ++i) { > if (batch->otables[i].enabled) > @@ -300,7 +279,6 @@ static int vmw_otable_batch_setup(struct vmw_private > *dev_priv, > > ttm_bo_put(batch->otable_bo); > batch->otable_bo = NULL; > -out_no_bo: > return ret; > } > > @@ -432,41 +410,9 @@ struct vmw_mob *vmw_mob_create(unsigned long data_pages) > static int vmw_mob_pt_populate(struct vmw_private *dev_priv, > struct vmw_mob *mob) > { > - int ret; > - struct ttm_operation_ctx ctx = { > - .interruptible = false, > - .no_wait_gpu = false > - }; > - > BUG_ON(mob->pt_bo != NULL); > > - ret = ttm_bo_create(_priv->bdev, mob->num_pages * PAGE_SIZE, > - ttm_bo_type_device, > - _sys_ne_placement, > - 0, false, >pt_bo); > - if (unlikely(ret != 0)) > - return ret; > - > - ret = ttm_bo_reserve(mob->pt_bo, false, true, NULL); > - > - BUG_ON(ret != 0); > - ret = vmw_bo_driver.ttm_tt_populate(mob->pt_bo->ttm, ); > - if (unlikely(ret != 0)) > - goto out_unreserve; > - ret = vmw_bo_map_dma(mob->pt_bo); > - if (unlikely(ret != 0)) > - goto out_unreserve; > - > - ttm_bo_unreserve(mob->pt_bo); > - > - return 0; > - > -out_unreserve: > - ttm_bo_unreserve(mob->pt_bo); > - ttm_bo_put(mob->pt_bo); > - mob->pt_bo = NULL; > - > - return ret; > + return vmw_bo_create_and_populate(dev_priv, mob->num_pages * PAGE_SIZE, > >pt_bo); > } > > /** > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c > index 2deaaed334e6..8e2a82ded900 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c > @@ -817,3 +817,35 @@ struct ttm_bo_driver
Re: [PATCH 6/8] drm/amd/display: Set DC options from modifiers.
On Tue, Aug 04, 2020 at 11:31:17PM +0200, Bas Nieuwenhuizen wrote: > This sets the DC tiling options from the modifier, if modifiers > are used for the FB. This patch by itself does not expose the > support yet though. > > There is not much validation yet to limit the scope of this > patch, but the current validation is at the same level as > the BO metadata path. > > Signed-off-by: Bas Nieuwenhuizen > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 109 +- > 1 file changed, 103 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 6ef7f2f8acab..ac913b8f10ef 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3754,6 +3754,93 @@ fill_gfx9_plane_attributes_from_flags(struct > amdgpu_device *adev, > return 0; > } > > +static bool > +modifier_has_dcc(uint64_t modifier) > +{ > + return IS_AMD_FMT_MOD(modifier) && AMD_FMT_MOD_GET(DCC, modifier); > +} > + > +static unsigned > +modifier_gfx9_swizzle_mode(uint64_t modifier) > +{ > + if (modifier == DRM_FORMAT_MOD_LINEAR) > + return 0; > + > + return AMD_FMT_MOD_GET(TILE, modifier); > +} > + > +static void > +fill_gfx9_tiling_info_from_modifier(const struct amdgpu_device *adev, > + union dc_tiling_info *tiling_info, > + uint64_t modifier) > +{ > + unsigned int mod_bank_xor_bits = AMD_FMT_MOD_GET(BANK_XOR_BITS, > modifier); > + unsigned int mod_pipe_xor_bits = AMD_FMT_MOD_GET(PIPE_XOR_BITS, > modifier); > + unsigned int pkrs_log2 = AMD_FMT_MOD_GET(PACKERS, modifier); > + unsigned int pipes_log2 = min(4u, mod_pipe_xor_bits); > + > + fill_gfx9_tiling_info_from_device(adev, tiling_info); > + > + if (!IS_AMD_FMT_MOD(modifier)) > + return; > + > + tiling_info->gfx9.num_pipes = 1u << pipes_log2; > + tiling_info->gfx9.num_shader_engines = 1u << (mod_pipe_xor_bits - > pipes_log2); > + > + if (adev->family >= AMDGPU_FAMILY_NV) { > + tiling_info->gfx9.num_pkrs = 1u << pkrs_log2; > + } else { > + tiling_info->gfx9.num_banks = 1u << mod_bank_xor_bits; > + > + /* for DCC we know it isn't rb aligned, so rb_per_se doesn't > matter. */ > + } > +} > + > +static void > +block_alignment(unsigned int blocksize_log2, unsigned int *width, unsigned > int *height) > +{ > + unsigned int height_log2 = blocksize_log2 / 2; > + unsigned int width_log2 = blocksize_log2 - height_log2; > + > + *width = 1u << width_log2; > + *height = 1u << height_log2; > +} > + > +static int > +fill_gfx9_plane_attributes_from_modifiers(struct amdgpu_device *adev, > + const struct amdgpu_framebuffer *afb, > + const enum surface_pixel_format format, > + const enum dc_rotation_angle rotation, > + const struct plane_size *plane_size, > + union dc_tiling_info *tiling_info, > + struct dc_plane_dcc_param *dcc, > + struct dc_plane_address *address, > + const bool force_disable_dcc) > +{ > + const uint64_t modifier = afb->base.modifier; > + int ret; > + > + fill_gfx9_tiling_info_from_modifier(adev, tiling_info, modifier); > + tiling_info->gfx9.swizzle = modifier_gfx9_swizzle_mode(modifier); > + > + if (modifier_has_dcc(modifier) && !force_disable_dcc) { > + uint64_t dcc_address = afb->address + afb->base.offsets[1]; > + > + dcc->enable = 1; > + dcc->meta_pitch = afb->base.pitches[1]; > + dcc->independent_64b_blks = > AMD_FMT_MOD_GET(DCC_INDEPENDENT_64B, modifier); > + > + address->grph.meta_addr.low_part = lower_32_bits(dcc_address); > + address->grph.meta_addr.high_part = upper_32_bits(dcc_address); > + } > + > + ret = validate_dcc(adev, format, rotation, tiling_info, dcc, address, > plane_size); > + if (ret) > + return ret; > + > + return 0; > +} > + > static int > fill_plane_buffer_attributes(struct amdgpu_device *adev, >const struct amdgpu_framebuffer *afb, > @@ -3823,12 +3910,22 @@ fill_plane_buffer_attributes(struct amdgpu_device > *adev, > > > if (adev->family >= AMDGPU_FAMILY_AI) { > - ret = fill_gfx9_plane_attributes_from_flags(adev, afb, format, > rotation, > - plane_size, > tiling_info, dcc, > - address, > tiling_flags, > - force_disable_dcc); > - if (ret) > -
Re: [PATCH 3/8] drm/amd/display: Honor the offset for plane 0.
On Tue, Aug 04, 2020 at 11:31:14PM +0200, Bas Nieuwenhuizen wrote: > With modifiers I'd like to support non-dedicated buffers for > images. > > Signed-off-by: Bas Nieuwenhuizen Uh, I think it'd be really good to preceed this with a bugfix (cc: stable) which checks that the offset is 0). And then this patch here removing that again. Or cc: stable this patch here, since we seem to have a gap in validating addfb. -Daniel > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 005331c772b7..abc70fbe176d 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3623,6 +3623,7 @@ fill_plane_dcc_attributes(struct amdgpu_device *adev, > struct dc *dc = adev->dm.dc; > struct dc_dcc_surface_param input; > struct dc_surface_dcc_cap output; > + uint64_t plane_address = afb->address + afb->base.offsets[0]; > uint32_t offset = AMDGPU_TILING_GET(info, DCC_OFFSET_256B); > uint32_t i64b = AMDGPU_TILING_GET(info, DCC_INDEPENDENT_64B) != 0; > uint64_t dcc_address; > @@ -3666,7 +3667,7 @@ fill_plane_dcc_attributes(struct amdgpu_device *adev, > AMDGPU_TILING_GET(info, DCC_PITCH_MAX) + 1; > dcc->independent_64b_blks = i64b; > > - dcc_address = get_dcc_address(afb->address, info); > + dcc_address = get_dcc_address(plane_address, info); > address->grph.meta_addr.low_part = lower_32_bits(dcc_address); > address->grph.meta_addr.high_part = upper_32_bits(dcc_address); > > @@ -3697,6 +3698,8 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev, > address->tmz_surface = tmz_surface; > > if (format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) { > + uint64_t addr = afb->address + fb->offsets[0]; > + > plane_size->surface_size.x = 0; > plane_size->surface_size.y = 0; > plane_size->surface_size.width = fb->width; > @@ -3705,9 +3708,10 @@ fill_plane_buffer_attributes(struct amdgpu_device > *adev, > fb->pitches[0] / fb->format->cpp[0]; > > address->type = PLN_ADDR_TYPE_GRAPHICS; > - address->grph.addr.low_part = lower_32_bits(afb->address); > - address->grph.addr.high_part = upper_32_bits(afb->address); > + address->grph.addr.low_part = lower_32_bits(addr); > + address->grph.addr.high_part = upper_32_bits(addr); > } else if (format < SURFACE_PIXEL_FORMAT_INVALID) { > + uint64_t luma_addr = afb->address + fb->offsets[0]; > uint64_t chroma_addr = afb->address + fb->offsets[1]; > > plane_size->surface_size.x = 0; > @@ -3728,9 +3732,9 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev, > > address->type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE; > address->video_progressive.luma_addr.low_part = > - lower_32_bits(afb->address); > + lower_32_bits(luma_addr); > address->video_progressive.luma_addr.high_part = > - upper_32_bits(afb->address); > + upper_32_bits(luma_addr); > address->video_progressive.chroma_addr.low_part = > lower_32_bits(chroma_addr); > address->video_progressive.chroma_addr.high_part = > -- > 2.28.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 03/22] backlight: Add get/set operations for brightness/power properties
On Tue, Aug 04, 2020 at 09:56:00PM +0200, Sam Ravnborg wrote: > Hi Daniel et al. > On Tue, Aug 04, 2020 at 06:43:30PM +0200, dan...@ffwll.ch wrote: > > On Sun, Aug 02, 2020 at 01:06:17PM +0200, Sam Ravnborg wrote: > > > Add get and set operations to incapsualte access to backlight properties. > > > > > > One easy win is that the get/set operatiosn can be used when backlight > > > is not included in the configuration, resulting in simpler code with > > > less ifdef's and thus more readable code. > > > > > > The set/get methods hides some of the confusing power states, limiting > > > the power state to either ON or OFF for the drivers. > > > > > > Signed-off-by: Sam Ravnborg > > > Cc: Lee Jones > > > Cc: Daniel Thompson > > > Cc: Jingoo Han > > > --- > > > include/linux/backlight.h | 72 +++ > > > 1 file changed, 72 insertions(+) > > > > > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h > > > index d683c053ec09..882ceea45ace 100644 > > > --- a/include/linux/backlight.h > > > +++ b/include/linux/backlight.h > > > @@ -474,6 +474,78 @@ static inline int backlight_get_brightness(const > > > struct backlight_device *bd) > > > return bd->props.brightness; > > > } > > > > > > +/** > > > + * backlight_get_actual_brightness - Returns the actual brightness > > > + * > > > + * On failure a negative error code is returned. > > > + */ > > > +static inline int backlight_get_actual_brightness(struct > > > backlight_device *bd) > > > +{ > > > + if (bd && bd->ops && bd->ops->get_brightness) > > > + return bd->ops->get_brightness(bd); > > > + else > > > + return -EINVAL; > > > +} > > > + > > > +/** > > > + * backlight_get_max_brightness - Returns maximum brightness > > > + * > > > + * This helper operation is preferred over direct access to > > > + * _properties.max_brightness > > > + * > > > + * Returns 0 if backlight_device is NULL > > > + */ > > > + > > > +static inline int backlight_get_max_brightness(const struct > > > backlight_device *bd) > > > +{ > > > + if (bd) > > > + return bd->props.max_brightness; > > > + else > > > + return 0; > > > +} > > > + > > > +/** > > > + * backlight_set_brightness - Set the brightness to the specified value > > > + * > > > + * This helper operation is preferred over direct assignment to > > > + * _properties.brightness. > > > + * > > > + * If backlight_device is NULL then silently exit. > > > + */ > > > +static inline void backlight_set_brightness(struct backlight_device *bd, > > > int brightness) > > > +{ > > > + if (bd) > > > + bd->props.brightness = brightness; > > > > Looking at the drivers I think including a call to backlight_update_status > > would simplify a lot of code. > > > > > +} > > > + > > > +/** > > > + * backlight_set_power_on - Set power state to ON. > > > + * > > > + * This helper operation is preferred over direct assignment to > > > + * backlight_properties.power. > > > + * > > > + * If backlight_device is NULL then silently exit. > > > + */ > > > +static inline void backlight_set_power_on(struct backlight_device *bd) > > > +{ > > > + if (bd) > > > + bd->props.power = FB_BLANK_UNBLANK; > > > +} > > > + > > > +/** > > > + * backlight_set_power_off - Set power state to OFF. > > > + * > > > + * This helper operation is preferred over direct assignment to > > > + * backlight_properties.power. > > > + * > > > + * If backlight_device is NULL then silently exit. > > > + */ > > > +static inline void backlight_set_power_off(struct backlight_device *bd) > > > > I'm not clear why we need these two above? I thought the long-term plan is > > only use backlight_enable/disable and then remove the tri-state power > > handling once everyone is converted over? > > > > Or maybe I'm just confused about the goal here ... > > My TODO for v2: > - Check all get_brightness implmentations. > Using backlight_get_brightness is wrong - find a better way > Check that they do return the actual brightness and not just the copy > from the backlight core Well it's only for the get_brigthness callback where I think this is problemantic. In update_state callback I think it's a good helper. > - Get rid of all uses of power_on/off - this is just another way to > disable backlight - so be explicit about it > - Consolidate the backlight_set_brightness(); backlight_update() pattern > to a helper > - Look into a mipi helper for backlight Imo perfectly fine to leave that out for some todo, otherwise this will get huge. > - Consider if we can change the backlight core to use an u32 for > brightness > - Take a look at Daniels rambling about drm_connector and backlight Also something we can postpone I think. We can still used devm_ together with a refcounted backlight (like we do with devm_drm_dev_alloc), and my gut feel says refcounted backlight is probably the way to go eventually. But also, we've been talking about drm_connector->panel for years, there's no
Re: [PATCH v3 00/23] device-dax: Support sub-dividing soft-reserved ranges
On Fri, Jul 31, 2020 at 08:24:58PM -0700, Dan Williams wrote: > - Fix test_hmm and other compilation fixups (Ralph) The hmm parts look OK Acked-by: Jason Gunthorpe Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Aw: Re: [PATCH v3 1/5] drm: mediatek: add ddp routing for mt7623
Hi, > Gesendet: Dienstag, 04. August 2020 um 16:34 Uhr > Von: "Chun-Kuang Hu" > > -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = { > > +static enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = { > > Why do you remove 'const'? was removed by previous patch and not re-added (revert failed), sorry. will fix in next version, if all other is ok. regards Frank ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Aw: [PATCH v4] net: ethernet: mtk_eth_soc: fix MTU warnings
sorry, send this accidentally while posting my hdmi series v4 (have not deleted patch-file) just ignore this...it's already merged regards Frank ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/5] drm/mediatek: disable tmds on mt2701
From: chunhui dai disable tmds on phy on mt2701 to support other resolutions like 1280x1024 Signed-off-by: chunhui dai Signed-off-by: Frank Wunderlich Tested-by: Frank Wunderlich --- drivers/gpu/drm/mediatek/mtk_hdmi_phy.c| 3 +++ drivers/gpu/drm/mediatek/mtk_hdmi_phy.h| 1 + drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c | 1 + 3 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c index 5223498502c4..edadb7a700f1 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c @@ -184,6 +184,9 @@ static int mtk_hdmi_phy_probe(struct platform_device *pdev) return PTR_ERR(phy_provider); } + if (hdmi_phy->conf->pll_default_off) + hdmi_phy->conf->hdmi_phy_disable_tmds(hdmi_phy); + return of_clk_add_provider(dev->of_node, of_clk_src_simple_get, hdmi_phy->pll); } diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h index 2d8b3182470d..f472fdeb63dc 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h @@ -22,6 +22,7 @@ struct mtk_hdmi_phy; struct mtk_hdmi_phy_conf { bool tz_disabled; unsigned long flags; + bool pll_default_off; const struct clk_ops *hdmi_phy_clk_ops; void (*hdmi_phy_enable_tmds)(struct mtk_hdmi_phy *hdmi_phy); void (*hdmi_phy_disable_tmds)(struct mtk_hdmi_phy *hdmi_phy); diff --git a/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c b/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c index d3cc4022e988..6fbedacfc1e8 100644 --- a/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c +++ b/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c @@ -239,6 +239,7 @@ static void mtk_hdmi_phy_disable_tmds(struct mtk_hdmi_phy *hdmi_phy) struct mtk_hdmi_phy_conf mtk_hdmi_phy_2701_conf = { .tz_disabled = true, .flags = CLK_SET_RATE_GATE, + .pll_default_off = true, .hdmi_phy_clk_ops = _hdmi_phy_pll_ops, .hdmi_phy_enable_tmds = mtk_hdmi_phy_enable_tmds, .hdmi_phy_disable_tmds = mtk_hdmi_phy_disable_tmds, -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/amd/display: Constify dcn20_res_pool_funcs
The only usage of dcn20_res_pool_funcs is to assign its address to a const pointer. Make it const to allow the compiler to put it in read-only memory. Signed-off-by: Rikard Falkeborn --- drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c index 2a5e7175926a..d7ce984a2ce3 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c @@ -3320,7 +3320,7 @@ enum dc_status dcn20_patch_unknown_plane_state(struct dc_plane_state *plane_stat return DC_OK; } -static struct resource_funcs dcn20_res_pool_funcs = { +static const struct resource_funcs dcn20_res_pool_funcs = { .destroy = dcn20_destroy_resource_pool, .link_enc_create = dcn20_link_encoder_create, .panel_cntl_create = dcn20_panel_cntl_create, -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Aw: Re: [PATCH v4 6/6] arm: dts: mt7623: add display subsystem related device nodes
> Gesendet: Dienstag, 04. August 2020 um 19:24 Uhr > Von: "David Woodhouse" > > + mipi_tx0: mipi-dphy@1001 { > > + compatible = "mediatek,mt7623-mipi-tx", > > +"mediatek,mt2701-mipi-tx"; > > + reg = <0 0x1001 0 0x90>; > > + clocks = <>; > > + clock-output-names = "mipi_tx0_pll"; > > + #clock-cells = <0>; > > + #phy-cells = <0>; > > + }; > > Doesn't this (and some others) also need status="disabled" since > they're not present on MT7623A? Or maybe it's time to split mt7623.dtsi > into a mt7623n.dtsi which includes mt7623a.dtsi and adds the extra > components? do you have a MT7623A board for testing? is there any list which components are existing on mt7623a? or should i disable all of them and re-enable them in bpi-r2 dts? regards Frank ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/5] make hdmi work on bananapi-r2
From: Frank Wunderlich This Patch-Series adds missing Patches/Bugfixes to get hdmi working on BPI-R2 v2->v3: - use own mmsys-routing for mt7623 instead of code getting different routing from dts - remove ddp routing bls -> dpi from bpir2/rfb dts - updated some commit-Messages as suggested by CK Hu v1->v2: - using get_possible_crtc API instead of hardcoded - drop unused dts-nodes - refine commit-messages as far as i can :) "config component output by device node port" is needed to fix a WARN_ON() "fix boot up for 720 and 480 but 1080" fixes flickering, which may cause also some resolutions not working on some TFT (had some problems on my smaller TFT) Frank Wunderlich (1): drm: mediatek: add ddp routing for mt7623 Jitao Shi (1): drm/mediatek: dpi/dsi: change the getting possible_crtc way Ryder Lee (1): arm: dts: mt7623: add display subsystem related device nodes Stu Hsieh (1): drm: Add get_possible_crtc API for dpi, dsi chunhui dai (1): drm/mediatek: disable tmds on mt2701 arch/arm/boot/dts/mt7623.dtsi | 177 ++ arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 72 +++ arch/arm/boot/dts/mt7623n-rfb-emmc.dts| 72 +++ drivers/gpu/drm/mediatek/mtk_dpi.c| 3 +- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 42 + drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 2 + drivers/gpu/drm/mediatek/mtk_drm_drv.c| 25 ++- drivers/gpu/drm/mediatek/mtk_dsi.c| 3 +- drivers/gpu/drm/mediatek/mtk_hdmi_phy.c | 3 + drivers/gpu/drm/mediatek/mtk_hdmi_phy.h | 1 + .../gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c| 1 + 11 files changed, 398 insertions(+), 3 deletions(-) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sun4i: Constify static structs
A number of static variables are not modified and can be made const to allow the compiler to put them in read-only memory. Signed-off-by: Rikard Falkeborn --- Perhaps it should be split up? If so, some guidance on how would be appreciated. drivers/gpu/drm/sun4i/sun4i_backend.c | 2 +- drivers/gpu/drm/sun4i/sun4i_framebuffer.c | 2 +- drivers/gpu/drm/sun4i/sun4i_frontend.c| 2 +- drivers/gpu/drm/sun4i/sun4i_lvds.c| 2 +- drivers/gpu/drm/sun4i/sun4i_rgb.c | 4 ++-- drivers/gpu/drm/sun4i/sun4i_tcon.c| 2 +- drivers/gpu/drm/sun4i/sun4i_tv.c | 6 +++--- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c| 2 +- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c| 2 +- drivers/gpu/drm/sun4i/sun8i_mixer.c | 2 +- drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 2 +- drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 2 +- 12 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c index 072ea113e6be..f025534eb30c 100644 --- a/drivers/gpu/drm/sun4i/sun4i_backend.c +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c @@ -769,7 +769,7 @@ static const struct sunxi_engine_ops sun4i_backend_engine_ops = { .vblank_quirk = sun4i_backend_vblank_quirk, }; -static struct regmap_config sun4i_backend_regmap_config = { +static const struct regmap_config sun4i_backend_regmap_config = { .reg_bits = 32, .val_bits = 32, .reg_stride = 4, diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c index 1568f68f9a9e..6825ef46f43f 100644 --- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c +++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c @@ -35,7 +35,7 @@ static const struct drm_mode_config_funcs sun4i_de_mode_config_funcs = { .fb_create = drm_gem_fb_create, }; -static struct drm_mode_config_helper_funcs sun4i_de_mode_config_helpers = { +static const struct drm_mode_config_helper_funcs sun4i_de_mode_config_helpers = { .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm, }; diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c index ec2a032e07b9..b51cc685c13a 100644 --- a/drivers/gpu/drm/sun4i/sun4i_frontend.c +++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c @@ -545,7 +545,7 @@ int sun4i_frontend_enable(struct sun4i_frontend *frontend) } EXPORT_SYMBOL(sun4i_frontend_enable); -static struct regmap_config sun4i_frontend_regmap_config = { +static const struct regmap_config sun4i_frontend_regmap_config = { .reg_bits = 32, .val_bits = 32, .reg_stride = 4, diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c b/drivers/gpu/drm/sun4i/sun4i_lvds.c index ffda3184aa12..ea430b0ab0f3 100644 --- a/drivers/gpu/drm/sun4i/sun4i_lvds.c +++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c @@ -47,7 +47,7 @@ static int sun4i_lvds_get_modes(struct drm_connector *connector) return drm_panel_get_modes(lvds->panel, connector); } -static struct drm_connector_helper_funcs sun4i_lvds_con_helper_funcs = { +static const struct drm_connector_helper_funcs sun4i_lvds_con_helper_funcs = { .get_modes = sun4i_lvds_get_modes, }; diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c index 5a7d43939ae6..20a49c1963cc 100644 --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c @@ -138,7 +138,7 @@ static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc, return MODE_OK; } -static struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = { +static const struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = { .get_modes = sun4i_rgb_get_modes, }; @@ -183,7 +183,7 @@ static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder) } } -static struct drm_encoder_helper_funcs sun4i_rgb_enc_helper_funcs = { +static const struct drm_encoder_helper_funcs sun4i_rgb_enc_helper_funcs = { .disable= sun4i_rgb_encoder_disable, .enable = sun4i_rgb_encoder_enable, .mode_valid = sun4i_rgb_mode_valid, diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 359b56e43b83..ae177b2bcf4b 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -825,7 +825,7 @@ static int sun4i_tcon_init_irq(struct device *dev, return 0; } -static struct regmap_config sun4i_tcon_regmap_config = { +static const struct regmap_config sun4i_tcon_regmap_config = { .reg_bits = 32, .val_bits = 32, .reg_stride = 4, diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c index 63f4428ac3bf..cb91bc11a0c7 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tv.c +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c @@ -468,7 +468,7 @@ static void sun4i_tv_mode_set(struct drm_encoder *encoder,
[PATCH 2/3] drm/amd/display: Constify dcn21_res_pool_funcs
The only usage of dcn21_res_pool_funcs is to assign its address to a const pointer. Make it const to allow the compiler to put it in read-only memory. Signed-off-by: Rikard Falkeborn --- drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c index 88d41a385add..a828696668bf 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c @@ -1754,7 +1754,7 @@ enum dc_status dcn21_patch_unknown_plane_state(struct dc_plane_state *plane_stat return result; } -static struct resource_funcs dcn21_res_pool_funcs = { +static const struct resource_funcs dcn21_res_pool_funcs = { .destroy = dcn21_destroy_resource_pool, .link_enc_create = dcn21_link_encoder_create, .panel_cntl_create = dcn21_panel_cntl_create, -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vt: defer kfree() of vc_screenbuf in vc_do_resize()
Do you think this approach is acceptable? Or, do we need to modify set_origin() ? On 2020/07/29 23:57, Tetsuo Handa wrote: > syzbot is reporting UAF bug in set_origin() from vc_do_resize() [1], for > vc_do_resize() calls kfree(vc->vc_screenbuf) before calling set_origin(). > > Unfortunately, in set_origin(), vc->vc_sw->con_set_origin() might access > vc->vc_pos when scroll is involved in order to manipulate cursor, but > vc->vc_pos refers already released vc->vc_screenbuf until vc->vc_pos gets > updated based on the result of vc->vc_sw->con_set_origin(). > > Preserving old buffer and tolerating outdated vc members until set_origin() > completes would be easier than preventing vc->vc_sw->con_set_origin() from > accessing outdated vc members. > > [1] > https://syzkaller.appspot.com/bug?id=6649da2081e2ebdc65c0642c214b27fe91099db3 > > Reported-by: syzbot > Signed-off-by: Tetsuo Handa > --- > drivers/tty/vt/vt.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index 42d8c67..c9ee8e9 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -1196,7 +1196,7 @@ static int vc_do_resize(struct tty_struct *tty, struct > vc_data *vc, > unsigned int old_rows, old_row_size, first_copied_row; > unsigned int new_cols, new_rows, new_row_size, new_screen_size; > unsigned int user; > - unsigned short *newscreen; > + unsigned short *oldscreen, *newscreen; > struct uni_screen *new_uniscr = NULL; > > WARN_CONSOLE_UNLOCKED(); > @@ -1294,10 +1294,11 @@ static int vc_do_resize(struct tty_struct *tty, > struct vc_data *vc, > if (new_scr_end > new_origin) > scr_memsetw((void *)new_origin, vc->vc_video_erase_char, > new_scr_end - new_origin); > - kfree(vc->vc_screenbuf); > + oldscreen = vc->vc_screenbuf; > vc->vc_screenbuf = newscreen; > vc->vc_screenbuf_size = new_screen_size; > set_origin(vc); > + kfree(oldscreen); > > /* do part of a reset_terminal() */ > vc->vc_top = 0; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel