[PATCH] drm/ttm: fix regression in ttm moves
From: Dave Airlie
This fixes a bug introduced in be1213a341a289afc51f89181c310e368fba0b66
drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2
On vmwgfx this causes a Command buffer error WARN to trigger.
This is because the old code used to check if bo->ttm was true,
and the new code doesn't, fix it code to add back the check resolves
the issue.
Fixes: be1213a341a2 ("drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2")
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/ttm/ttm_bo.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 70b3bee27850..e8aa2fe8e9d1 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
/* Zero init the new TTM structure if the old location should
* have used one as well.
*/
- ret = ttm_tt_create(bo, old_man->use_tt);
- if (ret)
- goto out_err;
+ if (!bo->ttm) {
+ ret = ttm_tt_create(bo, old_man->use_tt);
+ if (ret)
+ goto out_err;
+ }
ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
if (ret)
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 18/45] drm/nouveau: handle move notify inside move callback.
On Thu, 24 Sep 2020 at 15:20, Dave Airlie wrote:
>
> From: Dave Airlie
>
> Don't use explicit move notify for moves just do it in the driver side.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/nouveau/nouveau_bo.c | 62
> 1 file changed, 44 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 1e6c2561d692..144b82db16ac 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -970,38 +970,42 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo,
> bool evict,
> }
>
> static void
> -nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
> -struct ttm_resource *new_reg)
> +nouveau_bo_vma_map_update(struct nouveau_bo *nvbo,
> + uint32_t mem_type,
> + struct nouveau_mem *mem)
> {
> - struct nouveau_mem *mem = new_reg ? nouveau_mem(new_reg) : NULL;
> - struct nouveau_bo *nvbo = nouveau_bo(bo);
> struct nouveau_vma *vma;
>
> - /* ttm can now (stupidly) pass the driver bos it didn't create... */
> - if (bo->destroy != nouveau_bo_del_ttm)
> - return;
> -
> - nouveau_bo_del_io_reserve_lru(bo);
> -
> - if (mem && new_reg->mem_type != TTM_PL_SYSTEM &&
> + if (mem && mem_type != TTM_PL_SYSTEM &&
> mem->mem.page == nvbo->page) {
> list_for_each_entry(vma, >vma_list, head) {
> nouveau_vma_map(vma, mem);
> }
> } else {
> list_for_each_entry(vma, >vma_list, head) {
> - WARN_ON(ttm_bo_wait(bo, false, false));
> + WARN_ON(ttm_bo_wait(>bo, false, false));
I can look at this more closely myself a bit down the track, as I need
to do a lot in this area in the near future anyway, but it'd be nice
to pass the failure back here where possible to do so. The new
invalidate_notify() hook still can't fail, but the other uses can and
it'd be nice to do the right thing where it's possible.
Ben.
> nouveau_vma_unmap(vma);
> }
> }
> +}
>
> - if (new_reg) {
> - if (new_reg->mm_node)
> - nvbo->offset = (new_reg->start << PAGE_SHIFT);
> - else
> - nvbo->offset = 0;
> - }
> +static void
> +nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
> +struct ttm_resource *new_reg)
> +{
> + struct nouveau_bo *nvbo = nouveau_bo(bo);
> +
> + /* ttm can now (stupidly) pass the driver bos it didn't create... */
> + if (bo->destroy != nouveau_bo_del_ttm)
> + return;
> +
> + /* handle new_reg path in move */
> + if (new_reg)
> + return;
> +
> + nouveau_bo_del_io_reserve_lru(bo);
>
> + nouveau_bo_vma_map_update(nvbo, 0, NULL);
> }
>
> static int
> @@ -1038,6 +1042,20 @@ nouveau_bo_vm_cleanup(struct ttm_buffer_object *bo,
> *old_tile = new_tile;
> }
>
> +
> +static void
> +nouveau_bo_update_mem(struct nouveau_bo *nvbo,
> + struct ttm_resource *new_reg)
> +{
> + nouveau_bo_vma_map_update(nvbo, new_reg->mem_type,
> nouveau_mem(new_reg));
> + if (new_reg) {
> + if (new_reg->mm_node)
> + nvbo->offset = (new_reg->start << PAGE_SHIFT);
> + else
> + nvbo->offset = 0;
> + }
> +}
> +
> static int
> nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
> struct ttm_operation_ctx *ctx,
> @@ -1053,6 +1071,9 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool
> evict,
> if (ret)
> return ret;
>
> + nouveau_bo_del_io_reserve_lru(bo);
> + nouveau_bo_update_mem(nvbo, new_reg);
> +
> if (nvbo->bo.pin_count)
> NV_WARN(drm, "Moving pinned object %p!\n", nvbo);
>
> @@ -1108,6 +1129,11 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool
> evict,
> nouveau_bo_vm_cleanup(bo, new_tile, >tile);
> }
>
> + if (ret) {
> + nouveau_bo_del_io_reserve_lru(bo);
> + nouveau_bo_update_mem(nvbo, >mem);
> + }
> +
> 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
Re: [PATCH 00/45] TTM move refactoring
On Thu, 24 Sep 2020 at 15:19, Dave Airlie wrote: > > This refactors how TTM moves get called between core and drivers. > > 1) puts the driver in charge of all moves, and enforces > drivers have a move callback. > 2) moves move_notify actions around moves to the driver side > 3) moves binding/unbinding completely into move and driver side > 4) add a new invalidate callback to replace the last use of move_notify > 5) add some helpers to cleanup the resulting move code > > There's a bit of internal churn to try and make each patch logical, so > don't get too caught up in each patches changes unless the end result > is a problem. I've looked over the nouveau-specific patches, and the ttm ones (mostly ignoring the changes to other drivers beyond a quick glance over). For all but 37/45 and the patches that depend on it: Reviewed-by: Ben Skeggs I'll add some more specific comments to one of the patches, but it's also fine as-is. Ben. > > Dave. > > > ___ > 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 0/3] Prevent out-of-bounds access for built-in font data buffers
On 29. 09. 20, 14:34, Peilin Ye wrote: > the work in general? I couldn't think of how do we clean up subsystems > one by one, while keeping a `console_font` in `struct vc_data`. Hi, feel free to change struct vc_data's content as you need, of course. Only the UAPI _definitions_ have to be preserved (like struct console_font). thanks, -- js suse labs ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vmwgfx: Fix error handling in get_node
On Wed, Sep 30, 2020 at 06:03:03AM +0200, Roland Scheidegger (VMware) wrote: > Sigend-off-by: Roland Scheidegger ^^^ typo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] vmwgfx-fixes-5.9
Dave, Daniel One important fix for recent breakage. The following changes since commit a1b8638ba1320e6684aa98233c15255eb803fac7: Linux 5.9-rc7 (2020-09-27 14:38:10 -0700) are available in the Git repository at: git://people.freedesktop.org/~sroland/linux vmwgfx-fixes-5.9 for you to fetch changes up to f54c4442893b8dfbd3aff8e903c54dfff1aef990: drm/vmwgfx: Fix error handling in get_node (2020-09-30 05:44:28 +0200) Zack Rusin (1): drm/vmwgfx: Fix error handling in get_node drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vmwgfx: Fix error handling in get_node
From: Zack Rusin ttm_mem_type_manager_func.get_node was changed to return -ENOSPC instead of setting the node pointer to NULL. Unfortunately vmwgfx still had two places where it was explicitly converting -ENOSPC to 0 causing regressions. This fixes those spots by allowing -ENOSPC to be returned. That seems to fix recent regressions with vmwgfx. Signed-off-by: Zack Rusin Reviewed-by: Roland Scheidegger Reviewed-by: Martin Krastev Sigend-off-by: Roland Scheidegger --- drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c index 4a76fc7114ad..f8bdd4ea294a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c @@ -55,7 +55,7 @@ static int vmw_gmrid_man_get_node(struct ttm_mem_type_manager *man, id = ida_alloc_max(>gmr_ida, gman->max_gmr_ids - 1, GFP_KERNEL); if (id < 0) - return (id != -ENOMEM ? 0 : id); + return id; spin_lock(>lock); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c index b7c816ba7166..c8b9335bccd8 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c @@ -95,7 +95,7 @@ static int vmw_thp_get_node(struct ttm_mem_type_manager *man, mem->start = node->start; } - return 0; + return ret; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vmwgfx: return value from inserting in thp mode.
On Wed, 30 Sep 2020 at 12:17, Zack Rusin wrote: > > > > On Sep 29, 2020, at 22:07, Dave Airlie wrote: > > > > From: Dave Airlie > > > > This seems to fix the failure I'm seeing with 5.9-rc7 under > > vmplayer. > > > > Signed-off-by: Dave Airlie > > --- > > drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > > b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > > index b7c816ba7166..c8b9335bccd8 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > > @@ -95,7 +95,7 @@ static int vmw_thp_get_node(struct ttm_mem_type_manager > > *man, > > mem->start = node->start; > > } > > > > - return 0; > > + return ret; > > } > > That’s part of it. Roland, has a patch from me in his tree that fixes it. > Roland, can you please submit it to Dave for merge? > Please ASAP. I'm trying to test TTM changes, so I'd like to know I've had a stable base here. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vmwgfx: return value from inserting in thp mode.
> On Sep 29, 2020, at 22:07, Dave Airlie wrote: > > From: Dave Airlie > > This seems to fix the failure I'm seeing with 5.9-rc7 under > vmplayer. > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > index b7c816ba7166..c8b9335bccd8 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c > @@ -95,7 +95,7 @@ static int vmw_thp_get_node(struct ttm_mem_type_manager > *man, > mem->start = node->start; > } > > - return 0; > + return ret; > } That’s part of it. Roland, has a patch from me in his tree that fixes it. Roland, can you please submit it to Dave for merge? z ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: vmwgfx on 5.9-rc7 lots of spam
On Wed, 30 Sep 2020 at 11:09, Dave Airlie wrote: > > Hey Roland (et al), > > I took the opportunity to boot vmwgfx from Linus 5.9-rc7 on VMware > Player 15.5.6.build.16341506. > > Pretty much stock fedora 32 userspace, all updates as of today. > > It isn't pretty, can someone look into this urgently? I'll see if I > can work out what is happening myself, but it would be good to have > some engaged on vmware end. Okay I tracked it down, patch on the list. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vmwgfx: return value from inserting in thp mode.
From: Dave Airlie This seems to fix the failure I'm seeing with 5.9-rc7 under vmplayer. Signed-off-by: Dave Airlie --- drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c index b7c816ba7166..c8b9335bccd8 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c @@ -95,7 +95,7 @@ static int vmw_thp_get_node(struct ttm_mem_type_manager *man, mem->start = node->start; } - return 0; + return ret; } -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [External] Re: [PATCH 2/2] sched: mark PRINTK_DEFERRED_CONTEXT_MASK in __schedule()
On (20/09/29 17:09), Peter Zijlstra wrote: > > 2. The registration and unregistration of consoles should not longer > >be handled by console_lock (semaphore). It should be possible to > >call most consoles without a sleeping lock. It should remove all > >these deadlocks between printk() and scheduler(). > > I'm confused, who cares about registation? That only happens once at > boot, right? You can modprobe or rmmod (register/unregister) netconsole anytime, for example. -ss ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] drm/nouveau: Drop mutex_lock_nested for atomic
On Wed, 30 Sep 2020 at 00:52, Daniel Vetter wrote:
>
> On Thu, Sep 17, 2020 at 3:15 PM Daniel Vetter wrote:
> >
> > Ben, did you have a chance to look at this?
>
> Ping
> -Daniel
>
> > On Mon, Aug 3, 2020 at 1:22 PM Maarten Lankhorst
> > wrote:
> > >
> > > Op 02-08-2020 om 20:18 schreef Daniel Vetter:
> > > > Purely conjecture, but I think the original locking inversion with the
> > > > legacy page flip code between flipping and ttm's bo move function
> > > > shoudn't exist anymore with atomic: With atomic the bo pinning and
> > > > actual modeset commit is completely separated in the code patsh.
> > > >
> > > > This annotation was originally added in
> > > >
> > > > commit 060810d7abaabcab282e062c595871d661561400
> > > > Author: Ben Skeggs
> > > > Date: Mon Jul 8 14:15:51 2013 +1000
> > > >
> > > > drm/nouveau: fix locking issues in page flipping paths
> > > >
> > > > due to
> > > >
> > > > commit b580c9e2b7ba5030a795aa2fb73b796523d65a78
> > > > Author: Maarten Lankhorst
> > > > Date: Thu Jun 27 13:48:18 2013 +0200
> > > >
> > > > drm/nouveau: make flipping lockdep safe
> > > >
> > > > Signed-off-by: Daniel Vetter
> > > > Cc: Maarten Lankhorst
> > > > Cc: Ben Skeggs
> > > > Cc: Dave Airlie
> > > > Cc: nouv...@lists.freedesktop.org
> > > > ---
> > > > I might be totally wrong, so this definitely needs testing :-)
> > > >
> > > > Cheers, Daniel
> > > > ---
> > > > drivers/gpu/drm/nouveau/nouveau_bo.c | 6 +-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > > b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > > index 7806278dce57..a7b2a9bb0ffe 100644
> > > > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > > @@ -776,7 +776,11 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo,
> > > > int evict, bool intr,
> > > > return ret;
> > > > }
> > > >
> > > > - mutex_lock_nested(>mutex, SINGLE_DEPTH_NESTING);
> > > > + if (drm_drv_uses_atomic_modeset(drm->dev))
> > > > + mutex_lock(>mutex);
> > > > + else
> > > > + mutex_lock_nested(>mutex, SINGLE_DEPTH_NESTING);
> > > > +
> > > > ret = nouveau_fence_sync(nouveau_bo(bo), chan, true, intr);
> > > > if (ret == 0) {
> > > > ret = drm->ttm.move(chan, bo, >mem, new_reg);
> > >
> > > Well if you're certain it works now. :)
> > >
> > > Reviewed-by: Maarten Lankhorst
Acked-by: Ben Skeggs
> > >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Wed, Sep 30, 2020 at 12:59:58AM +0300, Ville Syrjälä wrote: > On Wed, Sep 30, 2020 at 12:11:48AM +0300, Ville Syrjälä wrote: > > On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote: > > > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote: > > > > On Tue, Sep 29, 2020 at 08:20:22PM +, Souza, Jose wrote: > > > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote: > > > > > > On Tue, Sep 29, 2020 at 07:33:45PM +, Souza, Jose wrote: > > > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote: > > > > > > > > JSL has update in vswing table for eDP > > > > > > > > > > > > > > Would be nice to mention in the commit description why PCH is > > > > > > > being used, that would avoid Ville's question. > > > > > > > > > > > > If the thing has nothing to do PCH then it should not use the PCH > > > > > > type > > > > > > for the the check. Instead we should just do the EHL/JSL split. > > > > > > > > > > In the first version Matt Roper suggested to use PCH to differentiate > > > > > between EHL and JSL, Jani also agreed with this solution.This 2 PCHs > > > > > can only be > > > > > associate with EHL and JSL respectively, so no downsides here. > > > > > > > > The downside is that the code makes no sense on the first glance. > > > > It's going to generate a "wtf?" exception in the brain and require > > > > me to take a second look to figure what is going on. Exception > > > > handling is expensive and shouldn't be needed in cases where it's > > > > trivial to make the code 100% obvious. > > > > > > The bspec documents EHL and JSL as being the same platform and identical > > > in all programming since they are literally the same display IP; this > > > vswing table is the one and only place where the two are treated in a > > > distinct manner for reasons that lie outside the display controller. If > > > you had to stop and take a closer look at the code here, that's a > > > probably a good thing since in general there should generally never be a > > > difference in the behavior between the two. Adding an additional > > > clarifying comment is probably in order too since this is a very > > > exceptional special case. > > > > > > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE > > > and IS_JASPERLAKE across the whole driver, that's going to be a lot more > > > pain to maintain down the road since we'll almost certainly have cases > > > where someone silently leaves one or the other off a condition and gets > > > unexepcted behavior. I could see arguments for using a SUBPLATFORM here > > > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we > > > already have a clear way to distinguish the two cases (PCH pairing) and > > > can just leave a clarifying comment. > > > > That fixed PCH pairing is totally undocumented AFAICS. And vswing has > > nothing to do with the south display, so the wtf will still happen. > > Comment or no comment. > > Oh and JSP does not show up in bspec even once. MCC appears exactly once > where it talks about the differences between MCC and ICL-N PCH (which I > guess is the same as JSP?). No, ICL-N PCH is something different. :-( There were some early test chips created that paired the EHL/JSL graphics/media/display IP with an ICL PCH just for early debug/test purposes, but that pairing isn't something that actually exists as a real platform. I think the confusion here arises because most driver developers only look at (or have access to) the bspec, which does not aim to document end-user platforms, but rather IP families that the graphics/media/display hardware IP teams design. The bspec is not an authoritative document for anything that lies outside the GMD IP itself. The GMD architects do sometimes try to pull in additional information from external teams/sources (such as PCH pairing or the electrical details like the vswing programming here) to make life easier for the software teams like us that only really deal with the bspec, but that information comes from external sources, so it's a bit inconsistent in terms of how much detail there is (or even whether it's described at all). We could probably file bspec defects to get them to include the PCH pairing details for EHL/JSL in the bspec, but ultimately EHL="EHL G/M/D + MCC PCH" and JSL="EHL G/M/D + JSP PCH;" this has already been confirmed in an offline email thread with the hardware teams. Elkhart Lake and Jasper Lake are two separate end-user platforms, that both incorporated the same G/M/D IP design. The name "Jasper Lake" existed as a codename first, so that's the name that shows up in the bspec; this wound up being a bit confusing when Elkhart Lake was actually the first of the two to be released and thus wound up being the name we have in our code. But the situation seems similar to CHT vs BSW which are both referred to as "CHV" in the bspec and in our code (although obviously there was no PCH pairing for those SoCs). Steppings,
Re: [PATCH v2] MAINTAINERS: add Dan Murphy as TP LP8xxx drivers maintainer
Hi, On Thu, Sep 24, 2020 at 01:23:31PM +0100, Lee Jones wrote: > On Thu, 24 Sep 2020, Krzysztof Kozlowski wrote: > > > On Wed, 23 Sep 2020 at 22:01, Jonathan Cameron wrote: > > > > > > On Wed, 23 Sep 2020 11:53:33 -0500 > > > Dan Murphy wrote: > > > > > > > Hello > > > > > > > > On 9/22/20 10:28 AM, Krzysztof Kozlowski wrote: > > > > > Milo Kim's email in TI bounces with permanent error (550: Invalid > > > > > recipient). Last email from him on LKML was in 2017. Move Milo Kim > > > > > to > > > > > credits and add Dan Murphy from TI to look after: > > > > > - TI LP855x backlight driver, > > > > > - TI LP8727 charger driver, > > > > > - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers. > > > > > > > > > > Cc: Dan Murphy > > > > > Signed-off-by: Krzysztof Kozlowski > > > > > > > > Acked-by: Dan Murphy > > > > > > > Not sure who will pick this one up, but > > > Acked-by: Jonathan Cameron > > > > I guess whoever is first. :) > > This spans across systems but the common part is MFD, so maybe Lee - > > could you pick it up? > > Yes, I'll handle it. Acked-by: Sebastian Reichel -- Sebastian signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid()
While I thought I had this correct (since it actually did reject modes
like I expected during testing), Ville Syrjala from Intel pointed out
that the logic here isn't correct. max_clock refers to the max data rate
supported by the DP encoder. So, limiting it to the output of ds_clock (which
refers to the maximum dotclock of the downstream DP device) doesn't make any
sense. Additionally, since we're using the connector's bpc as the canonical BPC
we should use this in mode_valid until we support dynamically setting the bpp
based on bandwidth constraints.
https://lists.freedesktop.org/archives/dri-devel/2020-September/280276.html
For more info.
So, let's rewrite this using Ville's advice.
v2:
* Ville pointed out I mixed up the dotclock and the link rate. So fix that...
* ...and also rename all the variables in this function to be more appropriately
labeled so I stop mixing them up.
* Reuse the bpp from the connector for now until we have dynamic bpp selection.
* Use use DIV_ROUND_UP for calculating the mode rate like i915 does, which we
should also have been doing from the start
Signed-off-by: Lyude Paul
Fixes: 409d38139b42 ("drm/nouveau/kms/nv50-: Use downstream DP clock limits for
mode validation")
Cc: Ville Syrjälä
Cc: Lyude Paul
Cc: Ben Skeggs
---
drivers/gpu/drm/nouveau/nouveau_dp.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index 93e3751ad7f1..040ed88d362d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -231,8 +231,9 @@ nv50_dp_mode_valid(struct drm_connector *connector,
const struct drm_display_mode *mode,
unsigned *out_clock)
{
- const unsigned min_clock = 25000;
- unsigned max_clock, ds_clock, clock = mode->clock;
+ const unsigned int min_clock = 25000;
+ unsigned int max_rate, mode_rate, ds_max_dotclock, clock = mode->clock;
+ const u8 bpp = connector->display_info.bpc * 3;
if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace)
return MODE_NO_INTERLACE;
@@ -240,17 +241,17 @@ nv50_dp_mode_valid(struct drm_connector *connector,
if ((mode->flags & DRM_MODE_FLAG_3D_MASK) ==
DRM_MODE_FLAG_3D_FRAME_PACKING)
clock *= 2;
- max_clock = outp->dp.link_nr * outp->dp.link_bw;
- ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd,
- outp->dp.downstream_ports);
- if (ds_clock)
- max_clock = min(max_clock, ds_clock);
+ max_rate = outp->dp.link_nr * outp->dp.link_bw;
+ mode_rate = DIV_ROUND_UP(clock * bpp, 8);
+ if (mode_rate > max_rate)
+ return MODE_CLOCK_HIGH;
+
+ ds_max_dotclock = drm_dp_downstream_max_dotclock(outp->dp.dpcd,
outp->dp.downstream_ports);
+ if (ds_max_dotclock && clock > ds_max_dotclock)
+ return MODE_CLOCK_HIGH;
- clock = mode->clock * (connector->display_info.bpc * 3) / 10;
if (clock < min_clock)
return MODE_CLOCK_LOW;
- if (clock > max_clock)
- return MODE_CLOCK_HIGH;
if (out_clock)
*out_clock = clock;
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/nouveau/kms/nv50-: Get rid of bogus nouveau_conn_mode_valid()
Ville also pointed out that I got a lot of the logic here wrong as well, whoops.
While I don't think anyone's likely using 3D output with nouveau, the next patch
will make nouveau_conn_mode_valid() make a lot less sense. So, let's just get
rid of it and open-code it like before, while taking care to move the 3D frame
packing calculations on the dot clock into the right place.
Signed-off-by: Lyude Paul
Fixes: d6a9efece724 ("drm/nouveau/kms/nv50-: Share DP SST mode_valid() handling
with MST")
Cc: Ville Syrjälä
Cc: # v5.8+
---
drivers/gpu/drm/nouveau/nouveau_connector.c | 36 ++---
drivers/gpu/drm/nouveau/nouveau_dp.c| 16 ++---
2 files changed, 21 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 49dd0cbc332f..6f21f36719fc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1023,29 +1023,6 @@ get_tmds_link_bandwidth(struct drm_connector *connector)
return 112000 * duallink_scale;
}
-enum drm_mode_status
-nouveau_conn_mode_clock_valid(const struct drm_display_mode *mode,
- const unsigned min_clock,
- const unsigned max_clock,
- unsigned int *clock_out)
-{
- unsigned int clock = mode->clock;
-
- if ((mode->flags & DRM_MODE_FLAG_3D_MASK) ==
- DRM_MODE_FLAG_3D_FRAME_PACKING)
- clock *= 2;
-
- if (clock < min_clock)
- return MODE_CLOCK_LOW;
- if (clock > max_clock)
- return MODE_CLOCK_HIGH;
-
- if (clock_out)
- *clock_out = clock;
-
- return MODE_OK;
-}
-
static enum drm_mode_status
nouveau_connector_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
@@ -1053,7 +1030,7 @@ nouveau_connector_mode_valid(struct drm_connector
*connector,
struct nouveau_connector *nv_connector = nouveau_connector(connector);
struct nouveau_encoder *nv_encoder = nv_connector->detected_encoder;
struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
- unsigned min_clock = 25000, max_clock = min_clock;
+ unsigned int min_clock = 25000, max_clock = min_clock, clock =
mode->clock;
switch (nv_encoder->dcb->type) {
case DCB_OUTPUT_LVDS:
@@ -1082,8 +1059,15 @@ nouveau_connector_mode_valid(struct drm_connector
*connector,
return MODE_BAD;
}
- return nouveau_conn_mode_clock_valid(mode, min_clock, max_clock,
-NULL);
+ if ((mode->flags & DRM_MODE_FLAG_3D_MASK) ==
DRM_MODE_FLAG_3D_FRAME_PACKING)
+ clock *= 2;
+
+ if (clock < min_clock)
+ return MODE_CLOCK_LOW;
+ if (clock > max_clock)
+ return MODE_CLOCK_HIGH;
+
+ return MODE_OK;
}
static struct drm_encoder *
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index 7b640e05bd4c..93e3751ad7f1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -232,12 +232,14 @@ nv50_dp_mode_valid(struct drm_connector *connector,
unsigned *out_clock)
{
const unsigned min_clock = 25000;
- unsigned max_clock, ds_clock, clock;
- enum drm_mode_status ret;
+ unsigned max_clock, ds_clock, clock = mode->clock;
if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace)
return MODE_NO_INTERLACE;
+ if ((mode->flags & DRM_MODE_FLAG_3D_MASK) ==
DRM_MODE_FLAG_3D_FRAME_PACKING)
+ clock *= 2;
+
max_clock = outp->dp.link_nr * outp->dp.link_bw;
ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd,
outp->dp.downstream_ports);
@@ -245,9 +247,13 @@ nv50_dp_mode_valid(struct drm_connector *connector,
max_clock = min(max_clock, ds_clock);
clock = mode->clock * (connector->display_info.bpc * 3) / 10;
- ret = nouveau_conn_mode_clock_valid(mode, min_clock, max_clock,
- );
+ if (clock < min_clock)
+ return MODE_CLOCK_LOW;
+ if (clock > max_clock)
+ return MODE_CLOCK_HIGH;
+
if (out_clock)
*out_clock = clock;
- return ret;
+
+ return MODE_OK;
}
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Wed, Sep 30, 2020 at 12:11:48AM +0300, Ville Syrjälä wrote: > On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote: > > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote: > > > On Tue, Sep 29, 2020 at 08:20:22PM +, Souza, Jose wrote: > > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote: > > > > > On Tue, Sep 29, 2020 at 07:33:45PM +, Souza, Jose wrote: > > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote: > > > > > > > JSL has update in vswing table for eDP > > > > > > > > > > > > Would be nice to mention in the commit description why PCH is being > > > > > > used, that would avoid Ville's question. > > > > > > > > > > If the thing has nothing to do PCH then it should not use the PCH type > > > > > for the the check. Instead we should just do the EHL/JSL split. > > > > > > > > In the first version Matt Roper suggested to use PCH to differentiate > > > > between EHL and JSL, Jani also agreed with this solution.This 2 PCHs > > > > can only be > > > > associate with EHL and JSL respectively, so no downsides here. > > > > > > The downside is that the code makes no sense on the first glance. > > > It's going to generate a "wtf?" exception in the brain and require > > > me to take a second look to figure what is going on. Exception > > > handling is expensive and shouldn't be needed in cases where it's > > > trivial to make the code 100% obvious. > > > > The bspec documents EHL and JSL as being the same platform and identical > > in all programming since they are literally the same display IP; this > > vswing table is the one and only place where the two are treated in a > > distinct manner for reasons that lie outside the display controller. If > > you had to stop and take a closer look at the code here, that's a > > probably a good thing since in general there should generally never be a > > difference in the behavior between the two. Adding an additional > > clarifying comment is probably in order too since this is a very > > exceptional special case. > > > > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE > > and IS_JASPERLAKE across the whole driver, that's going to be a lot more > > pain to maintain down the road since we'll almost certainly have cases > > where someone silently leaves one or the other off a condition and gets > > unexepcted behavior. I could see arguments for using a SUBPLATFORM here > > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we > > already have a clear way to distinguish the two cases (PCH pairing) and > > can just leave a clarifying comment. > > That fixed PCH pairing is totally undocumented AFAICS. And vswing has > nothing to do with the south display, so the wtf will still happen. > Comment or no comment. Oh and JSP does not show up in bspec even once. MCC appears exactly once where it talks about the differences between MCC and ICL-N PCH (which I guess is the same as JSP?). Furthermore the spec never really talks about EHL except in very select places. So the IS_ELKHARTLAKE is already totally confusing and IMO more likely to cause maintenance problems than the split. Making it IS_JSL||IS_EHL at least gives the reader some hint as to where they should look in the spec. Another argument why the split is fine is CFL/CML. Those are more or less exactly in the same boat as EHL. Ie. only really mentioned in the "configurations" section. Beyond that only KBL is ever really mentioned. And yet we have separate IS_FOOs for all of them, and apparently no one had any objections to that situation. tldr;we have an established way to handle these things, so IMO lets just follow it and stop adding special cases. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 01/19] drm/virtio: blob prep: refactor getting pages and attaching backing
On Tue, Sep 29, 2020 at 2:33 AM Gerd Hoffmann wrote: > On Wed, Sep 23, 2020 at 05:31:56PM -0700, Gurchetan Singh wrote: > > Useful for upcoming blob resources. > > Pushed to drm-misc-next (whole series). > Thanks -- sent over a 32-bit/64-bit bug fix and requested a virtio-spec vote. > > thanks, > Gerd > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] virtio-gpu api: fix 64/32 compat issue with blob implementation
From: Alistair Delva
We encountered this issue when booting blob with a 32-bit kernel.
The implementation doesn't match v6 of the virtio-spec change, so fix
this.
Fixes: ff886cbdcc44 ("virtio-gpu api: blob resources")
Signed-off-by: Alistair Delva
---
include/uapi/linux/virtio_gpu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
index fa2ae4a1da5f9..0ec6b610402cb 100644
--- a/include/uapi/linux/virtio_gpu.h
+++ b/include/uapi/linux/virtio_gpu.h
@@ -388,9 +388,9 @@ struct virtio_gpu_resource_create_blob {
/* zero is invalid blob mem */
__le32 blob_mem;
__le32 blob_flags;
+ __le32 nr_entries;
__le64 blob_id;
__le64 size;
- __le32 nr_entries;
/*
* sizeof(nr_entries * virtio_gpu_mem_entry) bytes follow
*/
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote: > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote: > > On Tue, Sep 29, 2020 at 08:20:22PM +, Souza, Jose wrote: > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote: > > > > On Tue, Sep 29, 2020 at 07:33:45PM +, Souza, Jose wrote: > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote: > > > > > > JSL has update in vswing table for eDP > > > > > > > > > > Would be nice to mention in the commit description why PCH is being > > > > > used, that would avoid Ville's question. > > > > > > > > If the thing has nothing to do PCH then it should not use the PCH type > > > > for the the check. Instead we should just do the EHL/JSL split. > > > > > > In the first version Matt Roper suggested to use PCH to differentiate > > > between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can > > > only be > > > associate with EHL and JSL respectively, so no downsides here. > > > > The downside is that the code makes no sense on the first glance. > > It's going to generate a "wtf?" exception in the brain and require > > me to take a second look to figure what is going on. Exception > > handling is expensive and shouldn't be needed in cases where it's > > trivial to make the code 100% obvious. > > The bspec documents EHL and JSL as being the same platform and identical > in all programming since they are literally the same display IP; this > vswing table is the one and only place where the two are treated in a > distinct manner for reasons that lie outside the display controller. If > you had to stop and take a closer look at the code here, that's a > probably a good thing since in general there should generally never be a > difference in the behavior between the two. Adding an additional > clarifying comment is probably in order too since this is a very > exceptional special case. > > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE > and IS_JASPERLAKE across the whole driver, that's going to be a lot more > pain to maintain down the road since we'll almost certainly have cases > where someone silently leaves one or the other off a condition and gets > unexepcted behavior. I could see arguments for using a SUBPLATFORM here > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we > already have a clear way to distinguish the two cases (PCH pairing) and > can just leave a clarifying comment. That fixed PCH pairing is totally undocumented AFAICS. And vswing has nothing to do with the south display, so the wtf will still happen. Comment or no comment. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote: > On Tue, Sep 29, 2020 at 08:20:22PM +, Souza, Jose wrote: > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote: > > > On Tue, Sep 29, 2020 at 07:33:45PM +, Souza, Jose wrote: > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote: > > > > > JSL has update in vswing table for eDP > > > > > > > > Would be nice to mention in the commit description why PCH is being > > > > used, that would avoid Ville's question. > > > > > > If the thing has nothing to do PCH then it should not use the PCH type > > > for the the check. Instead we should just do the EHL/JSL split. > > > > In the first version Matt Roper suggested to use PCH to differentiate > > between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can > > only be > > associate with EHL and JSL respectively, so no downsides here. > > The downside is that the code makes no sense on the first glance. > It's going to generate a "wtf?" exception in the brain and require > me to take a second look to figure what is going on. Exception > handling is expensive and shouldn't be needed in cases where it's > trivial to make the code 100% obvious. The bspec documents EHL and JSL as being the same platform and identical in all programming since they are literally the same display IP; this vswing table is the one and only place where the two are treated in a distinct manner for reasons that lie outside the display controller. If you had to stop and take a closer look at the code here, that's a probably a good thing since in general there should generally never be a difference in the behavior between the two. Adding an additional clarifying comment is probably in order too since this is a very exceptional special case. If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE and IS_JASPERLAKE across the whole driver, that's going to be a lot more pain to maintain down the road since we'll almost certainly have cases where someone silently leaves one or the other off a condition and gets unexepcted behavior. I could see arguments for using a SUBPLATFORM here like we do for TGL_U vs TGL_Y, but even that seems like overkill if we already have a clear way to distinguish the two cases (PCH pairing) and can just leave a clarifying comment. Matt > > -- > Ville Syrjälä > Intel -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, 2020-09-29 at 23:30 +0300, Ville Syrjälä wrote: > On Tue, Sep 29, 2020 at 08:20:22PM +, Souza, Jose wrote: > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote: > > > On Tue, Sep 29, 2020 at 07:33:45PM +, Souza, Jose wrote: > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote: > > > > > JSL has update in vswing table for eDP > > > > > > > > Would be nice to mention in the commit description why PCH is being > > > > used, that would avoid Ville's question. > > > > > > If the thing has nothing to do PCH then it should not use the PCH type > > > for the the check. Instead we should just do the EHL/JSL split. > > > > In the first version Matt Roper suggested to use PCH to differentiate > > between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can > > only be > > associate with EHL and JSL respectively, so no downsides here. > > The downside is that the code makes no sense on the first glance. > It's going to generate a "wtf?" exception in the brain and require > me to take a second look to figure what is going on. Exception > handling is expensive and shouldn't be needed in cases where it's > trivial to make the code 100% obvious. > Adding a comment on the top of jsl_get_combo_buf_trans() would help? Otherwise Tejas you will need to rework this then. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] Partially revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 9:53 PM Peter Collingbourne wrote: > The fbdev driver is used by Android's FVP configuration. Using the > DRM driver together with DRM's fbdev emulation results in a failure > to boot Android. The root cause is that Android's generic fbdev > userspace driver relies on the ability to set the pixel format via > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > There have been other less critical behavioral differences identified > between the fbdev driver and the DRM driver with fbdev emulation. The > DRM driver exposes different values for the panel's width, height and > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > with yres_virtual greater than the maximum supported value instead > of letting the syscall succeed and setting yres_virtual based on yres. Is there a way to reproduce this? A simple binary image where we can start Android on FVP and just replace the kernel image would be great. That way we can look at the incompatibilities in the FBDEV emulation and try to fix them. Is it working with a stock kernel or do you need any out-of-tree Android patches to run this? Can you also share the kernel config used for this build so it is easy to rebuild a similar kernel? Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 08:20:22PM +, Souza, Jose wrote: > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote: > > On Tue, Sep 29, 2020 at 07:33:45PM +, Souza, Jose wrote: > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote: > > > > JSL has update in vswing table for eDP > > > > > > Would be nice to mention in the commit description why PCH is being used, > > > that would avoid Ville's question. > > > > If the thing has nothing to do PCH then it should not use the PCH type > > for the the check. Instead we should just do the EHL/JSL split. > > In the first version Matt Roper suggested to use PCH to differentiate between > EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be > associate with EHL and JSL respectively, so no downsides here. The downside is that the code makes no sense on the first glance. It's going to generate a "wtf?" exception in the brain and require me to take a second look to figure what is going on. Exception handling is expensive and shouldn't be needed in cases where it's trivial to make the code 100% obvious. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 8:44 PM Daniel Vetter wrote: > On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > But aside from all this, why is this blocking the migration from fbdev > to drm? With fbdev you don't have buffer allocations, nor dma-buf > support, and somehow android can boot. I do not know how Android does things these days but back in the days it would request a virtual framebuffer twice the height of the physical framebuffer and then pan that up/down between composing frames, thus achieving a type of double-buffering from userspace. Given the type of bugs Peter is seeing this seems to still be the case, Peter can you confirm? Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 07:33:45PM +, Souza, Jose wrote:
> > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > JSL has update in vswing table for eDP
> >
> > Would be nice to mention in the commit description why PCH is being used,
> > that would avoid Ville's question.
>
> If the thing has nothing to do PCH then it should not use the PCH type
> for the the check. Instead we should just do the EHL/JSL split.
In the first version Matt Roper suggested to use PCH to differentiate between
EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
associate with EHL and JSL respectively, so no downsides here.
>
> > > BSpec: 21257
> > >
> > > Changes since V1 :
> > > - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> > > HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
> > > - Reverted EHL/JSL PCI ids split change
> > >
> > > Signed-off-by: Tejas Upadhyay <
> > > tejaskumarx.surendrakumar.upadh...@intel.com
> > >
> > >
> > > ---
> > > drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++--
> > > 1 file changed, 64 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 4d06178cd76c..e6e93d01d0ce 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans
> > > ehl_combo_phy_ddi_translations_dp[] = {
> > > { 0x6, 0x7F, 0x3F, 0x00, 0x00 },/* 900 900 0.0 */
> > > };
> > >
> > > +static const struct cnl_ddi_buf_trans
> > > jsl_combo_phy_ddi_translations_edp_hbr[] = {
> > > + /* NT mV Trans mV db*/
> > > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> > > + { 0x8, 0x7F, 0x38, 0x00, 0x07 },/* 200 250 1.9 */
> > > + { 0x1, 0x7F, 0x33, 0x00, 0x0C },/* 200 300 3.5 */
> > > + { 0xA, 0x35, 0x36, 0x00, 0x09 },/* 200 350 4.9 */
> > > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> > > + { 0x1, 0x7F, 0x38, 0x00, 0x07 },/* 250 300 1.6 */
> > > + { 0xA, 0x35, 0x35, 0x00, 0x0A },/* 250 350 2.9 */
> > > + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> > > + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> > > + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> > > +};
> > > +
> > > +static const struct cnl_ddi_buf_trans
> > > jsl_combo_phy_ddi_translations_edp_hbr2[] = {
> > > + /* NT mV Trans mV db*/
> > > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> > > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 250 1.9 */
> > > + { 0x1, 0x7F, 0x3D, 0x00, 0x02 },/* 200 300 3.5 */
> > > + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 200 350 4.9 */
> > > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> > > + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 250 300 1.6 */
> > > + { 0xA, 0x35, 0x3A, 0x00, 0x05 },/* 250 350 2.9 */
> > > + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> > > + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> > > + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> > > +};
> >
> > Tables matches specification.
> >
> > > +
> > > struct icl_mg_phy_ddi_buf_trans {
> > > u32 cri_txdeemph_override_11_6;
> > > u32 cri_txdeemph_override_5_0;
> > > @@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder,
> > > int type, int rate,
> > > *n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
> > > return icl_mg_phy_ddi_translations_rbr_hbr;
> > > }
> > > -
> >
> > Probably not intentional.
> >
> > Reviewed-by: José Roberto de Souza <
> > jose.so...@intel.com
> > >
> >
> > Will push with this line fixed as soon as CI finish testing.
> >
> >
> > > static const struct cnl_ddi_buf_trans *
> > > ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int
> > > rate,
> > > int *n_entries)
> > > @@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder
> > > *encoder, int type, int rate,
> > > }
> > > }
> > >
> > > +static const struct cnl_ddi_buf_trans *
> > > +jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int
> > > rate,
> > > + int *n_entries)
> > > +{
> > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > +
> > > + switch (type) {
> > > + case INTEL_OUTPUT_HDMI:
> > > + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > > + return icl_combo_phy_ddi_translations_hdmi;
> > > + case INTEL_OUTPUT_EDP:
> > > + if
Re: [PATCH v2 2/2] dt-bindings: display: sii902x: Add supply bindings
On Mon, 28 Sep 2020 12:30:54 -0500, Alexandru Gagniuc wrote: > The sii902x chip family requires IO and core voltages to reach the > correct voltage before chip initialization. Add binding for describing > the two supplies. > > Signed-off-by: Alexandru Gagniuc > --- > Changes since v1: > * Nothing. version incremented to stay in sync with sii902x regulator patch > > Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 > 1 file changed, 4 insertions(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
On Tue, Sep 29, 2020 at 8:31 AM Jan Kiszka wrote:
>
> On 10.09.20 20:18, Deucher, Alexander wrote:
> > [AMD Public Use]
> >
> >
> >
> >> -Original Message-
> >> From: amd-gfx On Behalf Of
> >> Daniel Vetter
> >> Sent: Monday, September 7, 2020 3:15 AM
> >> To: Jan Kiszka ; amd-gfx list >> g...@lists.freedesktop.org>; Wentland, Harry ;
> >> Kazlauskas, Nicholas
> >> Cc: dri-devel ; intel-gfx >> g...@lists.freedesktop.org>; Thomas Zimmermann
> >> ; Ville Syrjala
> >> Subject: Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
> >>
> >> On Sun, Sep 6, 2020 at 1:19 PM Jan Kiszka wrote:
> >>>
> >>> On 11.02.20 18:04, Daniel Vetter wrote:
> On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > WARN if the encoder possible_crtcs is effectively empty or contains
> > bits for non-existing crtcs.
> >
> > v2: Move to drm_mode_config_validate() (Daniel)
> > Make the docs say we WARN when this is wrong (Daniel)
> > Extract full_crtc_mask()
> >
> > Cc: Thomas Zimmermann
> > Cc: Daniel Vetter
> > Signed-off-by: Ville Syrjälä
>
> When pushing the fixup needs to be applied before the validation
> patch here, because we don't want to anger the bisect gods.
>
> Reviewed-by: Daniel Vetter
>
> I think with the fixup we should be good enough with the existing
> nonsense in drivers. Fingers crossed.
> -Daniel
>
>
> > ---
> > drivers/gpu/drm/drm_mode_config.c | 27
> >> ++-
> > include/drm/drm_encoder.h | 2 +-
> > 2 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_mode_config.c
> > b/drivers/gpu/drm/drm_mode_config.c
> > index afc91447293a..4c1b350ddb95 100644
> > --- a/drivers/gpu/drm/drm_mode_config.c
> > +++ b/drivers/gpu/drm/drm_mode_config.c
> > @@ -581,6 +581,29 @@ static void
> >> validate_encoder_possible_clones(struct drm_encoder *encoder)
> > encoder->possible_clones, encoder_mask); }
> >
> > +static u32 full_crtc_mask(struct drm_device *dev) {
> > +struct drm_crtc *crtc;
> > +u32 crtc_mask = 0;
> > +
> > +drm_for_each_crtc(crtc, dev)
> > +crtc_mask |= drm_crtc_mask(crtc);
> > +
> > +return crtc_mask;
> > +}
> > +
> > +static void validate_encoder_possible_crtcs(struct drm_encoder
> > +*encoder) {
> > +u32 crtc_mask = full_crtc_mask(encoder->dev);
> > +
> > +WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
> > + (encoder->possible_crtcs & ~crtc_mask) != 0,
> > + "Bogus possible_crtcs: "
> > + "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
> > + encoder->base.id, encoder->name,
> > + encoder->possible_crtcs, crtc_mask); }
> > +
> > void drm_mode_config_validate(struct drm_device *dev) {
> > struct drm_encoder *encoder;
> > @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct
> >> drm_device *dev)
> > drm_for_each_encoder(encoder, dev)
> > fixup_encoder_possible_clones(encoder);
> >
> > -drm_for_each_encoder(encoder, dev)
> > +drm_for_each_encoder(encoder, dev) {
> > validate_encoder_possible_clones(encoder);
> > +validate_encoder_possible_crtcs(encoder);
> > +}
> > }
> > diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> > index 3741963b9587..b236269f41ac 100644
> > --- a/include/drm/drm_encoder.h
> > +++ b/include/drm/drm_encoder.h
> > @@ -142,7 +142,7 @@ struct drm_encoder {
> > * the bits for all _crtc objects this encoder can be
> > connected to
> > * before calling drm_dev_register().
> > *
> > - * In reality almost every driver gets this wrong.
> > + * You will get a WARN if you get this wrong in the driver.
> > *
> > * Note that since CRTC objects can't be hotplugged the assigned
> >> indices
> > * are stable and hence known before registering all objects.
> > --
> > 2.24.1
> >
>
> >>>
> >>> Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):
> >>
> >> Adding amdgpu display folks.
> >
> > I took a quick look at this and it looks like we limit the number of crtcs
> > later in the mode init process if the number of physical displays can't
> > actually use more crtcs. E.g., the physical board configuration would only
> > allow for 3 active displays even if the hardware technically supports 4
> > crtcs. I presume that way we can just leave the additional hardware power
> > gated all the time.
> >
>
> So, will this be fixed any time soon? I don't feel qualified writing
> such a patch but I would obviously be happy to test one.
It's harmless, but I'll send out a patch
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 07:33:45PM +, Souza, Jose wrote:
> On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > JSL has update in vswing table for eDP
>
> Would be nice to mention in the commit description why PCH is being used,
> that would avoid Ville's question.
If the thing has nothing to do PCH then it should not use the PCH type
for the the check. Instead we should just do the EHL/JSL split.
>
> >
> > BSpec: 21257
> >
> > Changes since V1 :
> > - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> > HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
> > - Reverted EHL/JSL PCI ids split change
> >
> > Signed-off-by: Tejas Upadhyay <
> > tejaskumarx.surendrakumar.upadh...@intel.com
> > >
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++--
> > 1 file changed, 64 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 4d06178cd76c..e6e93d01d0ce 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans
> > ehl_combo_phy_ddi_translations_dp[] = {
> > { 0x6, 0x7F, 0x3F, 0x00, 0x00 },/* 900 900 0.0 */
> > };
> >
> > +static const struct cnl_ddi_buf_trans
> > jsl_combo_phy_ddi_translations_edp_hbr[] = {
> > + /* NT mV Trans mV db*/
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> > + { 0x8, 0x7F, 0x38, 0x00, 0x07 },/* 200 250 1.9 */
> > + { 0x1, 0x7F, 0x33, 0x00, 0x0C },/* 200 300 3.5 */
> > + { 0xA, 0x35, 0x36, 0x00, 0x09 },/* 200 350 4.9 */
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> > + { 0x1, 0x7F, 0x38, 0x00, 0x07 },/* 250 300 1.6 */
> > + { 0xA, 0x35, 0x35, 0x00, 0x0A },/* 250 350 2.9 */
> > + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> > + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> > + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> > +};
> > +
> > +static const struct cnl_ddi_buf_trans
> > jsl_combo_phy_ddi_translations_edp_hbr2[] = {
> > + /* NT mV Trans mV db*/
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 250 1.9 */
> > + { 0x1, 0x7F, 0x3D, 0x00, 0x02 },/* 200 300 3.5 */
> > + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 200 350 4.9 */
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> > + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 250 300 1.6 */
> > + { 0xA, 0x35, 0x3A, 0x00, 0x05 },/* 250 350 2.9 */
> > + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> > + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> > + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> > +};
>
> Tables matches specification.
>
> > +
> > struct icl_mg_phy_ddi_buf_trans {
> > u32 cri_txdeemph_override_11_6;
> > u32 cri_txdeemph_override_5_0;
> > @@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder,
> > int type, int rate,
> > *n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
> > return icl_mg_phy_ddi_translations_rbr_hbr;
> > }
> > -
>
> Probably not intentional.
>
> Reviewed-by: José Roberto de Souza
>
> Will push with this line fixed as soon as CI finish testing.
>
>
> > static const struct cnl_ddi_buf_trans *
> > ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> > int *n_entries)
> > @@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder
> > *encoder, int type, int rate,
> > }
> > }
> >
> > +static const struct cnl_ddi_buf_trans *
> > +jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> > + int *n_entries)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +
> > + switch (type) {
> > + case INTEL_OUTPUT_HDMI:
> > + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > + return icl_combo_phy_ddi_translations_hdmi;
> > + case INTEL_OUTPUT_EDP:
> > + if (dev_priv->vbt.edp.low_vswing) {
> > + if (rate > 27) {
> > + *n_entries =
> > ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr2);
> > + return jsl_combo_phy_ddi_translations_edp_hbr2;
> > + } else {
> > + *n_entries =
> > ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr);
> > + return jsl_combo_phy_ddi_translations_edp_hbr;
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
Would be nice to mention in the commit description why PCH is being used, that
would avoid Ville's question.
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay <
> tejaskumarx.surendrakumar.upadh...@intel.com
> >
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++--
> 1 file changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4d06178cd76c..e6e93d01d0ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans
> ehl_combo_phy_ddi_translations_dp[] = {
> { 0x6, 0x7F, 0x3F, 0x00, 0x00 },/* 900 900 0.0 */
> };
>
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x38, 0x00, 0x07 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x33, 0x00, 0x0C },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x36, 0x00, 0x09 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x38, 0x00, 0x07 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x35, 0x00, 0x0A },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
> +
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr2[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x3D, 0x00, 0x02 },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x3A, 0x00, 0x05 },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
Tables matches specification.
> +
> struct icl_mg_phy_ddi_buf_trans {
> u32 cri_txdeemph_override_11_6;
> u32 cri_txdeemph_override_5_0;
> @@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder, int
> type, int rate,
> *n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
> return icl_mg_phy_ddi_translations_rbr_hbr;
> }
> -
Probably not intentional.
Reviewed-by: José Roberto de Souza
Will push with this line fixed as soon as CI finish testing.
> static const struct cnl_ddi_buf_trans *
> ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> int *n_entries)
> @@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder,
> int type, int rate,
> }
> }
>
> +static const struct cnl_ddi_buf_trans *
> +jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> + int *n_entries)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> + switch (type) {
> + case INTEL_OUTPUT_HDMI:
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> + return icl_combo_phy_ddi_translations_hdmi;
> + case INTEL_OUTPUT_EDP:
> + if (dev_priv->vbt.edp.low_vswing) {
> + if (rate > 27) {
> + *n_entries =
> ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr2);
> + return jsl_combo_phy_ddi_translations_edp_hbr2;
> + } else {
> + *n_entries =
> ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr);
> + return jsl_combo_phy_ddi_translations_edp_hbr;
> + }
> + }
> + /* fall through */
> + default:
> + /* All combo DP and eDP ports that do not support low_vswing */
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hbr2);
> + return icl_combo_phy_ddi_translations_dp_hbr2;
> +
Re: [PATCH 1/2] drm/vc4: hdmi: Disable Wifi Frequencies
On Fri, Sep 25, 2020 at 03:07:43PM +0200, Maxime Ripard wrote: > There's cross-talk on the RPi4 between the 2.4GHz channels used by the WiFi > chip and some resolutions, most notably 1440p at 60Hz. > > In such a case, we can either reject entirely the mode, or lower slightly > the pixel frequency to remove the overlap. Let's go for the latter. > > Signed-off-by: Maxime Ripard > --- > .../bindings/display/brcm,bcm2711-hdmi.yaml| 6 ++ > drivers/gpu/drm/vc4/vc4_hdmi.c | 14 +- > drivers/gpu/drm/vc4/vc4_hdmi.h | 8 > 3 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml > b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml > index 03a76729d26c..63e7fe999c0a 100644 > --- a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml > @@ -76,6 +76,12 @@ properties: >resets: > maxItems: 1 > > + raspberrypi,disable-wifi-frequencies: > +type: boolean > +description: > > + Should the pixel frequencies in the WiFi frequencies range be > + avoided? Based on googling the issue, perhaps should be a common property? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] dt-bindings: gpu: samsung-rotator: Add missing properties
On Wed, 23 Sep 2020 17:03:39 +0200, Krzysztof Kozlowski wrote: > Add common properties appearing in DTSes (iommus, power-domains) to fix > dtbs_check warnings like: > > arch/arm/boot/dts/exynos4210-i9100.dt.yaml: rotator@1281: > 'iommus', 'power-domains' do not match any of the regexes: > 'pinctrl-[0-9]+' > > Signed-off-by: Krzysztof Kozlowski > > --- > > Changes since v2: > 1. Add maxItems to power domains and iommus > > Changes since v1: > 1. Add properties instead of using unevaluated > --- > Documentation/devicetree/bindings/gpu/samsung-rotator.yaml | 6 ++ > 1 file changed, 6 insertions(+) > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > > Hi, > > > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > > > > register header". > > > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > > > > DRM driver together with DRM's fbdev emulation results in a > > > > > > > failure > > > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > > > userspace driver relies on the ability to set the pixel format via > > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > > > Not without kernel changes. See e.g. > > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > > > That discussion seems to have died down with no further action. > > > > > > I also was kinda under the assumption that with Android these buffers > > > would be allocated directly from dma-buf heaps/ion, so this all seems not > > > that well baked out. > > The disagreement about whether these allocations should be made via > the render node or via dma-buf/ion is one reason why it was hard to > make progress on this, unfortunately. Yeah, using dumb buffer create to allocate random buffers for shared software rendering isn't super popular move. But aside from all this, why is this blocking the migration from fbdev to drm? With fbdev you don't have buffer allocations, nor dma-buf support, and somehow android can boot. But on drm you can't boot if these things are not available. That sounds like the bar for drm is maybe a tad too high for simple dumb kms drivers like pl which are just there to get a picture on the screen/panel. -Daniel > > > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > > > emulation, then let's do that. Not keep fbdev drivers on life support > > > > > for > > > > > longer than necessary. > > > > > > > > That should have been done *before* removing the old driver, which was > > > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > > > userspace broken for the period of time that it would take to develop > > > > that change. Even if such a change were developed before 5.9 is > > > > released, it probably wouldn't be considered a bug fix that would be > > > > eligible for 5.9. > > > > > > > > > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > > > > There have been other less critical behavioral differences > > > > > > > identified > > > > > > > between the fbdev driver and the DRM driver with fbdev emulation. > > > > > > > The > > > > > > > DRM driver exposes different values for the panel's width, height > > > > > > > and > > > > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO > > > > > > > syscall > > > > > > > with yres_virtual greater than the maximum supported value instead > > > > > > > of letting the syscall succeed and setting yres_virtual based on > > > > > > > yres. > > > > > > > > > > Also something that should be fixed I think in upstream, in the drm > > > > > fbdev > > > > > emulation. At least doesn't sound like it's something unfixable. > > > > > > > > Again, it should have been fixed before removing the old driver. > > > > > > Yeah, but we also don't have a whole lot people who seem to push for this. > > > So the only way to find the people who still care is to remove the fbdev > > > drivers. > > > > > > fbdev is dead. I'm totally fine reverting the driver to shut up the > > > regression report, but it's not fixing anything long term. There's not > > > going to be new drivers, or new hw support in existing drivers in fbdev. > > > > > > Also note that there's not spec nor test suite for fbdev, so "you guys > > > should have known before removing drivers" doesn't work. > > > > btw revert itself is stuck somewhere, so I can't find it. And for the > > You should be able to download the commit like this (the commit > message will need amending to remove most of the trailers at the > bottom, though): > > git fetch https://linux.googlesource.com/linux/kernel/git/torvalds/linux > refs/changes/52/2952/1 > > > revert can you pls just resurrect the files in drivers/video, without > > touching anything in drivers/gpu? Tying down drm drivers with stuff in > > fbdev doesn't make much sense to me. > > The touching of code in drm was
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > > Hi, > > > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > > > > register header". > > > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > > > > DRM driver together with DRM's fbdev emulation results in a > > > > > > > failure > > > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > > > userspace driver relies on the ability to set the pixel format via > > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > > > Not without kernel changes. See e.g. > > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > > > That discussion seems to have died down with no further action. > > > > > > I also was kinda under the assumption that with Android these buffers > > > would be allocated directly from dma-buf heaps/ion, so this all seems not > > > that well baked out. > > The disagreement about whether these allocations should be made via > the render node or via dma-buf/ion is one reason why it was hard to > make progress on this, unfortunately. > > > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > > > emulation, then let's do that. Not keep fbdev drivers on life support > > > > > for > > > > > longer than necessary. > > > > > > > > That should have been done *before* removing the old driver, which was > > > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > > > userspace broken for the period of time that it would take to develop > > > > that change. Even if such a change were developed before 5.9 is > > > > released, it probably wouldn't be considered a bug fix that would be > > > > eligible for 5.9. > > > > > > > > > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > > > > There have been other less critical behavioral differences > > > > > > > identified > > > > > > > between the fbdev driver and the DRM driver with fbdev emulation. > > > > > > > The > > > > > > > DRM driver exposes different values for the panel's width, height > > > > > > > and > > > > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO > > > > > > > syscall > > > > > > > with yres_virtual greater than the maximum supported value instead > > > > > > > of letting the syscall succeed and setting yres_virtual based on > > > > > > > yres. > > > > > > > > > > Also something that should be fixed I think in upstream, in the drm > > > > > fbdev > > > > > emulation. At least doesn't sound like it's something unfixable. > > > > > > > > Again, it should have been fixed before removing the old driver. > > > > > > Yeah, but we also don't have a whole lot people who seem to push for this. > > > So the only way to find the people who still care is to remove the fbdev > > > drivers. > > > > > > fbdev is dead. I'm totally fine reverting the driver to shut up the > > > regression report, but it's not fixing anything long term. There's not > > > going to be new drivers, or new hw support in existing drivers in fbdev. > > > > > > Also note that there's not spec nor test suite for fbdev, so "you guys > > > should have known before removing drivers" doesn't work. > > > > btw revert itself is stuck somewhere, so I can't find it. And for the > > You should be able to download the commit like this (the commit > message will need amending to remove most of the trailers at the > bottom, though): > > git fetch https://linux.googlesource.com/linux/kernel/git/torvalds/linux > refs/changes/52/2952/1 > > > revert can you pls just resurrect the files in drivers/video, without > > touching anything in drivers/gpu? Tying down drm drivers with stuff in > > fbdev doesn't make much sense to me. > > The touching of code in drm was a consequence of needing to revert a > follow-up commit which moved code previously shared between fbdev and > drm into the drm driver. > > Or do you think we should "manually" revert the first commit and as a > consequence end up with the two copies of the code? Yup. -Daniel > > Peter > > > > > Thanks, Daniel > > > > > -Daniel > > > > > > > > > > > Peter > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > > > Signed-off-by: Peter Collingbourne > > > > > > > --- > > > > > > > View this change in
Re: [PATCH] drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid()
On Tue, 2020-09-29 at 21:09 +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 01:54:13PM -0400, Lyude Paul wrote:
> > On Mon, 2020-09-28 at 16:01 +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 22, 2020 at 05:05:10PM -0400, Lyude Paul wrote:
> > > > While I thought I had this correct (since it actually did reject modes
> > > > like I expected during testing), Ville Syrjala from Intel pointed out
> > > > that the logic here isn't correct. max_clock refers to the max symbol
> > > > rate supported by the encoder, so limiting clock to ds_clock using
> > > > max()
> > > > doesn't make sense. Additionally, we want to check against 6bpc for
> > > > the
> > > > time being since that's the minimum possible bpc here, not the
> > > > reported
> > > > bpc from the connector. See:
> > > >
> > > > https://lists.freedesktop.org/archives/dri-devel/2020-September/280276.html
> > > >
> > > > For more info.
> > > >
> > > > So, let's rewrite this using Ville's advice.
> > > >
> > > > Signed-off-by: Lyude Paul
> > > > Fixes: 409d38139b42 ("drm/nouveau/kms/nv50-: Use downstream DP clock
> > > > limits for mode validation")
> > > > Cc: Ville Syrjälä
> > > > Cc: Lyude Paul
> > > > Cc: Ben Skeggs
> > > > ---
> > > > drivers/gpu/drm/nouveau/nouveau_dp.c | 23 +--
> > > > 1 file changed, 13 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
> > > > b/drivers/gpu/drm/nouveau/nouveau_dp.c
> > > > index 7b640e05bd4cd..24c81e423d349 100644
> > > > --- a/drivers/gpu/drm/nouveau/nouveau_dp.c
> > > > +++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
> > > > @@ -231,23 +231,26 @@ nv50_dp_mode_valid(struct drm_connector
> > > > *connector,
> > > >const struct drm_display_mode *mode,
> > > >unsigned *out_clock)
> > > > {
> > > > - const unsigned min_clock = 25000;
> > > > - unsigned max_clock, ds_clock, clock;
> > > > + const unsigned int min_clock = 25000;
> > > > + unsigned int max_clock, ds_clock, clock;
> > > > + const u8 bpp = 18; /* 6 bpc */
> > >
> > > AFAICS nv50_outp_atomic_check() and nv50_msto_atomic_check()
> > > just blindly use connector->display_info.bpc without any fallback
> > > logic to lower the bpc. So Ilia's concerns seem well founded.
> > > Without that logic I guess you should just use
> > > connector->display_info.bpc here as well.
> > >
> > > > enum drm_mode_status ret;
> > > >
> > > > if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp-
> > > > >caps.dp_interlace)
> > > > return MODE_NO_INTERLACE;
> > > >
> > > > max_clock = outp->dp.link_nr * outp->dp.link_bw;
> > > > - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd,
> > > > - outp-
> > > > >dp.downstream_ports);
> > > > - if (ds_clock)
> > > > - max_clock = min(max_clock, ds_clock);
> > > > -
> > > > - clock = mode->clock * (connector->display_info.bpc * 3) / 10;
> > > > - ret = nouveau_conn_mode_clock_valid(mode, min_clock,
> > > > max_clock,
> > > > - );
> > > > + clock = mode->clock * bpp / 8;
> > > > + if (clock > max_clock)
> > > > + return MODE_CLOCK_HIGH;
> > >
> > > This stuff vs. nouveau_conn_mode_clock_valid() still seems a bit messy.
> > > The max_clock you pass to nouveau_conn_mode_clock_valid() is the max
> > > symbol clock, but nouveau_conn_mode_clock_valid() checks it against the
> > > dotclock. Also only nouveau_conn_mode_clock_valid() has any kind of
> > > stereo 3D handling, but AFAICS stereo_allowed is also set for DP?
> >
> > ...not sure I'm following you here, it's set to true for DP so don't we
> > want
> > to check it and adjust the pixel clock we output accordingly?
>
> Yes, but then you need to also double your your pixel clock
> derived values in this function. Ie. all the mode->clock
> needs to become mode->clock*2 when dealing with a 3D frame
> packing mode.
oh this is a good point, thanks for noticing. I guess I'll get to changing
this around once I start working on the rest of the bpp stuff, since I'd
rather get this fixed first (I doubt many folks are using nouveau for 3D right
now, but if other nouveau folks disagree i'm happy to change my mind)
>
> > > > +
> > > > + ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, outp-
> > > > > dp.downstream_ports);
> > > > + if (ds_clock && mode->clock > ds_clock)
> > > > + return MODE_CLOCK_HIGH;
> > > > +
> > > > + ret = nouveau_conn_mode_clock_valid(mode, min_clock,
> > > > max_clock,
> > > > );
> > > > if (out_clock)
> > > > *out_clock = clock;
> > > > +
> > > > return ret;
> > > > }
> > > > --
> > > > 2.26.2
> > --
> > Cheers,
> > Lyude Paul (she/her)
> > Software Engineer at Red Hat
--
Cheers,
Lyude Paul (she/her)
Software Engineer at
Re: [PATCH] drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid()
On Tue, Sep 29, 2020 at 01:54:13PM -0400, Lyude Paul wrote:
> On Mon, 2020-09-28 at 16:01 +0300, Ville Syrjälä wrote:
> > On Tue, Sep 22, 2020 at 05:05:10PM -0400, Lyude Paul wrote:
> > > While I thought I had this correct (since it actually did reject modes
> > > like I expected during testing), Ville Syrjala from Intel pointed out
> > > that the logic here isn't correct. max_clock refers to the max symbol
> > > rate supported by the encoder, so limiting clock to ds_clock using max()
> > > doesn't make sense. Additionally, we want to check against 6bpc for the
> > > time being since that's the minimum possible bpc here, not the reported
> > > bpc from the connector. See:
> > >
> > > https://lists.freedesktop.org/archives/dri-devel/2020-September/280276.html
> > >
> > > For more info.
> > >
> > > So, let's rewrite this using Ville's advice.
> > >
> > > Signed-off-by: Lyude Paul
> > > Fixes: 409d38139b42 ("drm/nouveau/kms/nv50-: Use downstream DP clock
> > > limits for mode validation")
> > > Cc: Ville Syrjälä
> > > Cc: Lyude Paul
> > > Cc: Ben Skeggs
> > > ---
> > > drivers/gpu/drm/nouveau/nouveau_dp.c | 23 +--
> > > 1 file changed, 13 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
> > > b/drivers/gpu/drm/nouveau/nouveau_dp.c
> > > index 7b640e05bd4cd..24c81e423d349 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_dp.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
> > > @@ -231,23 +231,26 @@ nv50_dp_mode_valid(struct drm_connector *connector,
> > > const struct drm_display_mode *mode,
> > > unsigned *out_clock)
> > > {
> > > - const unsigned min_clock = 25000;
> > > - unsigned max_clock, ds_clock, clock;
> > > + const unsigned int min_clock = 25000;
> > > + unsigned int max_clock, ds_clock, clock;
> > > + const u8 bpp = 18; /* 6 bpc */
> >
> > AFAICS nv50_outp_atomic_check() and nv50_msto_atomic_check()
> > just blindly use connector->display_info.bpc without any fallback
> > logic to lower the bpc. So Ilia's concerns seem well founded.
> > Without that logic I guess you should just use
> > connector->display_info.bpc here as well.
> >
> > > enum drm_mode_status ret;
> > >
> > > if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace)
> > > return MODE_NO_INTERLACE;
> > >
> > > max_clock = outp->dp.link_nr * outp->dp.link_bw;
> > > - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd,
> > > - outp->dp.downstream_ports);
> > > - if (ds_clock)
> > > - max_clock = min(max_clock, ds_clock);
> > > -
> > > - clock = mode->clock * (connector->display_info.bpc * 3) / 10;
> > > - ret = nouveau_conn_mode_clock_valid(mode, min_clock, max_clock,
> > > - );
> > > + clock = mode->clock * bpp / 8;
> > > + if (clock > max_clock)
> > > + return MODE_CLOCK_HIGH;
> >
> > This stuff vs. nouveau_conn_mode_clock_valid() still seems a bit messy.
> > The max_clock you pass to nouveau_conn_mode_clock_valid() is the max
> > symbol clock, but nouveau_conn_mode_clock_valid() checks it against the
> > dotclock. Also only nouveau_conn_mode_clock_valid() has any kind of
> > stereo 3D handling, but AFAICS stereo_allowed is also set for DP?
>
> ...not sure I'm following you here, it's set to true for DP so don't we want
> to check it and adjust the pixel clock we output accordingly?
Yes, but then you need to also double your your pixel clock
derived values in this function. Ie. all the mode->clock
needs to become mode->clock*2 when dealing with a 3D frame
packing mode.
>
> >
> > > +
> > > + ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, outp-
> > > >dp.downstream_ports);
> > > + if (ds_clock && mode->clock > ds_clock)
> > > + return MODE_CLOCK_HIGH;
> > > +
> > > + ret = nouveau_conn_mode_clock_valid(mode, min_clock, max_clock,
> > > );
> > > if (out_clock)
> > > *out_clock = clock;
> > > +
> > > return ret;
> > > }
> > > --
> > > 2.26.2
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] dt-bindings: phy: convert phy-mtk-xsphy.txt to YAML schema
On Tue, Sep 22, 2020 at 03:55:05PM +0800, Chunfeng Yun wrote: > Convert phy-mtk-xsphy.txt to YAML schema mediatek,xsphy.yaml > > Signed-off-by: Chunfeng Yun > --- > .../bindings/phy/mediatek,xsphy.yaml | 203 ++ > .../devicetree/bindings/phy/phy-mtk-xsphy.txt | 109 -- > 2 files changed, 203 insertions(+), 109 deletions(-) > create mode 100644 Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml > delete mode 100644 Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt > > diff --git a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml > b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml > new file mode 100644 > index ..0aaa10640b5a > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml > @@ -0,0 +1,203 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (c) 2020 MediaTek > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/phy/mediatek,xsphy.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek XS-PHY Controller Device Tree Bindings > + > +maintainers: > + - Chunfeng Yun > + > +description: | > + The XS-PHY controller supports physical layer functionality for USB3.1 > + GEN2 controller on MediaTek SoCs. > + > +properties: > + $nodename: > +pattern: "^xs-phy@[0-9a-f]+$" > + > + compatible: > +oneOf: > + - items: > + - enum: > + - mediatek,mt3611-xsphy > + - enum: > + - mediatek,xsphy > + - items: > + - const: mediatek,xsphy mediatek,xsphy alone should not be valid. > + > + reg: > +description: | > + Register shared by multiple U3 ports, exclude port's private register, > + if only U2 ports provided, shouldn't use the property. > +maxItems: 1 > + > + "#address-cells": > + enum: [1, 2] > + > + "#size-cells": > + enum: [1, 2] > + > + ranges: true > + > + mediatek,src-ref-clk-mhz: > +description: > + Frequency of reference clock for slew rate calibrate > +$ref: /schemas/types.yaml#/definitions/uint32 > +default: 26 > + > + mediatek,src-coef: > +description: > + Coefficient for slew rate calibrate, depends on SoC process > +$ref: /schemas/types.yaml#/definitions/uint32 > +default: 17 > + > +# Required child node: > +patternProperties: > + "^usb-phy@[0-9a-f]+$": > +type: object > +description: | > + A sub-node is required for each port the controller provides. > + Address range information including the usual 'reg' property > + is used inside these nodes to describe the controller's topology. > + > +properties: > + reg: > +maxItems: 1 > + > + clocks: > +items: > + - description: Reference clock, (HS is 48Mhz, SS/P is 24~27Mhz) > + > + clock-names: > +items: > + - const: ref > + > + "#phy-cells": > +const: 1 > +description: | > + The cells contain the following arguments. > + > + - description: The PHY type > + enum: > +- PHY_TYPE_USB2 > +- PHY_TYPE_USB3 > + > + #The following optional vendor properties are only for debug or HQA > test > + mediatek,eye-src: > +description: > + The value of slew rate calibrate (U2 phy) > +$ref: /schemas/types.yaml#/definitions/uint32 > +minimum: 1 > +maximum: 7 > + > + mediatek,eye-vrt: > +description: > + The selection of VRT reference voltage (U2 phy) > +$ref: /schemas/types.yaml#/definitions/uint32 > +minimum: 1 > +maximum: 7 > + > + mediatek,eye-term: > +description: > + The selection of HS_TX TERM reference voltage (U2 phy) > +$ref: /schemas/types.yaml#/definitions/uint32 > +minimum: 1 > +maximum: 7 > + > + mediatek,efuse-intr: > +description: > + The selection of Internal Resistor (U2/U3 phy) > +$ref: /schemas/types.yaml#/definitions/uint32 > +minimum: 1 > +maximum: 63 > + > + mediatek,efuse-tx-imp: > +description: > + The selection of TX Impedance (U3 phy) > +$ref: /schemas/types.yaml#/definitions/uint32 > +minimum: 1 > +maximum: 31 > + > + mediatek,efuse-rx-imp: > +description: > + The selection of RX Impedance (U3 phy) > +$ref: /schemas/types.yaml#/definitions/uint32 > +minimum: 1 > +maximum: 31 > + > +required: > + - reg > + - clocks > + - clock-names > + - "#phy-cells" > + > +additionalProperties: false > + > +required: > + - compatible > + - "#address-cells" > + - "#size-cells" > + - ranges > + > +additionalProperties: false > + > +#Banks layout of xsphy > +#- Move this to top-level 'description'. > +#portoffset
Re: [PATCH] drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid()
On Mon, 2020-09-28 at 16:01 +0300, Ville Syrjälä wrote:
> On Tue, Sep 22, 2020 at 05:05:10PM -0400, Lyude Paul wrote:
> > While I thought I had this correct (since it actually did reject modes
> > like I expected during testing), Ville Syrjala from Intel pointed out
> > that the logic here isn't correct. max_clock refers to the max symbol
> > rate supported by the encoder, so limiting clock to ds_clock using max()
> > doesn't make sense. Additionally, we want to check against 6bpc for the
> > time being since that's the minimum possible bpc here, not the reported
> > bpc from the connector. See:
> >
> > https://lists.freedesktop.org/archives/dri-devel/2020-September/280276.html
> >
> > For more info.
> >
> > So, let's rewrite this using Ville's advice.
> >
> > Signed-off-by: Lyude Paul
> > Fixes: 409d38139b42 ("drm/nouveau/kms/nv50-: Use downstream DP clock
> > limits for mode validation")
> > Cc: Ville Syrjälä
> > Cc: Lyude Paul
> > Cc: Ben Skeggs
> > ---
> > drivers/gpu/drm/nouveau/nouveau_dp.c | 23 +--
> > 1 file changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
> > b/drivers/gpu/drm/nouveau/nouveau_dp.c
> > index 7b640e05bd4cd..24c81e423d349 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_dp.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
> > @@ -231,23 +231,26 @@ nv50_dp_mode_valid(struct drm_connector *connector,
> >const struct drm_display_mode *mode,
> >unsigned *out_clock)
> > {
> > - const unsigned min_clock = 25000;
> > - unsigned max_clock, ds_clock, clock;
> > + const unsigned int min_clock = 25000;
> > + unsigned int max_clock, ds_clock, clock;
> > + const u8 bpp = 18; /* 6 bpc */
>
> AFAICS nv50_outp_atomic_check() and nv50_msto_atomic_check()
> just blindly use connector->display_info.bpc without any fallback
> logic to lower the bpc. So Ilia's concerns seem well founded.
> Without that logic I guess you should just use
> connector->display_info.bpc here as well.
>
> > enum drm_mode_status ret;
> >
> > if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace)
> > return MODE_NO_INTERLACE;
> >
> > max_clock = outp->dp.link_nr * outp->dp.link_bw;
> > - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd,
> > - outp->dp.downstream_ports);
> > - if (ds_clock)
> > - max_clock = min(max_clock, ds_clock);
> > -
> > - clock = mode->clock * (connector->display_info.bpc * 3) / 10;
> > - ret = nouveau_conn_mode_clock_valid(mode, min_clock, max_clock,
> > - );
> > + clock = mode->clock * bpp / 8;
> > + if (clock > max_clock)
> > + return MODE_CLOCK_HIGH;
>
> This stuff vs. nouveau_conn_mode_clock_valid() still seems a bit messy.
> The max_clock you pass to nouveau_conn_mode_clock_valid() is the max
> symbol clock, but nouveau_conn_mode_clock_valid() checks it against the
> dotclock. Also only nouveau_conn_mode_clock_valid() has any kind of
> stereo 3D handling, but AFAICS stereo_allowed is also set for DP?
...not sure I'm following you here, it's set to true for DP so don't we want
to check it and adjust the pixel clock we output accordingly?
>
> > +
> > + ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, outp-
> > >dp.downstream_ports);
> > + if (ds_clock && mode->clock > ds_clock)
> > + return MODE_CLOCK_HIGH;
> > +
> > + ret = nouveau_conn_mode_clock_valid(mode, min_clock, max_clock,
> > );
> > if (out_clock)
> > *out_clock = clock;
> > +
> > return ret;
> > }
> > --
> > 2.26.2
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209417] kernel-5.8*, amdgpu - Blank screen shortly after boot, "fixed" with suspend and wake up
https://bugzilla.kernel.org/show_bug.cgi?id=209417 --- Comment #4 from Alex Deucher (alexdeuc...@gmail.com) --- yeah, if it was working with 5.7.0 and not working with 5.8.0, you can just do a bit bisect on https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ between the v5.7 and v5.8 tags. Google for "kernel git bisect howto". -- 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 v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
Hi Christian
Am 29.09.20 um 17:35 schrieb Christian König:
> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann:
>> The new helper ttm_kmap_obj_to_dma_buf() extracts address and location
>> from and instance of TTM's kmap_obj and initializes struct dma_buf_map
>> with these values. Helpful for TTM-based drivers.
>
> We could completely drop that if we use the same structure inside TTM as
> well.
>
> Additional to that which driver is going to use this?
As Daniel mentioned, it's in patch 3. The TTM-based drivers will
retrieve the pointer via this function.
I do want to see all that being more tightly integrated into TTM, but
not in this series. This one is about fixing the bochs-on-sparc64
problem for good. Patch 7 adds an update to TTM to the DRM TODO list.
Best regards
Thomas
>
> Regards,
> Christian.
>
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> include/drm/ttm/ttm_bo_api.h | 24
>> include/linux/dma-buf-map.h | 20
>> 2 files changed, 44 insertions(+)
>>
>> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
>> index c96a25d571c8..62d89f05a801 100644
>> --- a/include/drm/ttm/ttm_bo_api.h
>> +++ b/include/drm/ttm/ttm_bo_api.h
>> @@ -34,6 +34,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct
>> ttm_bo_kmap_obj *map,
>> return map->virtual;
>> }
>> +/**
>> + * ttm_kmap_obj_to_dma_buf_map
>> + *
>> + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap.
>> + * @map: Returns the mapping as struct dma_buf_map
>> + *
>> + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the memory
>> + * is not mapped, the returned mapping is initialized to NULL.
>> + */
>> +static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj
>> *kmap,
>> + struct dma_buf_map *map)
>> +{
>> + bool is_iomem;
>> + void *vaddr = ttm_kmap_obj_virtual(kmap, _iomem);
>> +
>> + if (!vaddr)
>> + dma_buf_map_clear(map);
>> + else if (is_iomem)
>> + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem *)vaddr);
>> + else
>> + dma_buf_map_set_vaddr(map, vaddr);
>> +}
>> +
>> /**
>> * ttm_bo_kmap
>> *
>> diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
>> index fd1aba545fdf..2e8bbecb5091 100644
>> --- a/include/linux/dma-buf-map.h
>> +++ b/include/linux/dma-buf-map.h
>> @@ -45,6 +45,12 @@
>> *
>> * dma_buf_map_set_vaddr( 0xdeadbeaf);
>> *
>> + * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem().
>> + *
>> + * .. code-block:: c
>> + *
>> + * dma_buf_map_set_vaddr_iomem( 0xdeadbeaf);
>> + *
>> * Test if a mapping is valid with either dma_buf_map_is_set() or
>> * dma_buf_map_is_null().
>> *
>> @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct
>> dma_buf_map *map, void *vaddr)
>> map->is_iomem = false;
>> }
>> +/**
>> + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to
>> an address in I/O memory
>> + * @map: The dma-buf mapping structure
>> + * @vaddr_iomem: An I/O-memory address
>> + *
>> + * Sets the address and the I/O-memory flag.
>> + */
>> +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map,
>> + void __iomem *vaddr_iomem)
>> +{
>> + map->vaddr_iomem = vaddr_iomem;
>> + map->is_iomem = true;
>> +}
>> +
>> /**
>> * dma_buf_map_is_equal - Compares two dma-buf mapping structures
>> for equality
>> * @lhs: The dma-buf mapping structure
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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
Re: [PATCH] drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid()
On Fri, 2020-09-25 at 19:53 -0400, Ilia Mirkin wrote: > On Fri, Sep 25, 2020 at 6:08 PM Lyude Paul wrote: > > On Tue, 2020-09-22 at 17:22 -0400, Ilia Mirkin wrote: > > > On Tue, Sep 22, 2020 at 5:14 PM Lyude Paul wrote: > > > > On Tue, 2020-09-22 at 17:10 -0400, Ilia Mirkin wrote: > > > > > Can we use 6bpc on arbitrary DP monitors, or is there a capability > > > > > for > > > > > it? Maybe only use 6bpc if display_info.bpc == 6 and otherwise use > > > > > 8? > > > > > > > > I don't think that display_info.bpc actually implies a minimum bpc, > > > > only a > > > > maximum bpc iirc (Ville would know the answer to this one). The other > > > > thing > > > > to > > > > note here is that we want to assume the lowest possible bpc here since > > > > we're > > > > only concerned if the mode passed to ->mode_valid can be set under > > > > -any- > > > > conditions (including those that require lowering the bpc beyond it's > > > > maximum > > > > value), so we definitely do want to always use 6bpc here even once we > > > > get > > > > support for optimizing the bpc based on the available display > > > > bandwidth. > > > > > > Yeah, display_info is the max bpc. But would an average monitor > > > support 6bpc? And if it does, does the current link training code even > > > try that when display_info.bpc != 6? > > > > So I did confirm that 6bpc support is mandatory for DP, so yes-6 bpc will > > always > > work. > > > > But also, your second comment doesn't really apply here. So: to be clear, > > we're > > not really concerned here about whether nouveau will actually use 6bpc or > > not. > > In truth I'm not actually sure either if we have any code that uses 6bpc > > (iirc > > we don't), since we don't current optimize bpc. I think it's very possible > > for > > us to use 6bpc for eDP displays if I recall though, but I'm not sure on > > that. > > > > But that's also not the point of this code. ->mode_valid() is only used in > > two > > situations in DRM modesetting: when probing connector modes, and when > > checking > > if a mode is valid or not during the atomic check for atomic modesetting. > > Its > > purpose is only to reject display modes that are physically impossible to > > set in > > hardware due to static hardware constraints. Put another way, we only > > check the > > given mode against constraints which will always remain constant > > regardless of > > the rest of the display state. An example of a static constraint would be > > the > > max pixel clock supported by the hardware, since on sensible hardware this > > never > > changes. A dynamic constraint would be something like how much bandwidth > > is > > currently unused on an MST topology, since that value is entirely > > dependent on > > the rest of the display state. > > > > So - with that said, bpc is technically a dynamic constraint because while > > a > > sink and source both likely have their own bpc limits, any bpc which is > > equal or > > below that limit can be used depending on what the driver decides - which > > will > > be based on the max_bpc property, and additionally for MST displays it > > will also > > depend on the available bandwidth on the topology. The only non-dynamic > > thing > > about bpc is that at a minimum, it will be 6 - so any mode that doesn't > > fit on > > the link with a bpc of 6 is guaranteed to be a mode that we'll never be > > able to > > set and therefore want to prune. > > > > So, even if we're not using 6 in the majority of situations, I'm fairly > > confident it's the right value here. It's also what i915 does as well (and > > they > > previously had to fix a bug that was the result of assuming a minimum of > > 8bpc > > instead of 6). > > Here's the situation I'm trying to avoid: > > 1. Mode is considered "OK" from a bandwidth perspective @6bpc > 2. Modesetting logic never tries 6bpc and the modeset fails > > As long as the two bits of logic agree on what is settable, I'm happy. I gotcha-I guess I was just confused because this is already possible with the current code we have (and it was also possible before we started adding these checks into ->mode_valid, which is why I need to get to the max_bpc thing soon). I guess I'll just use the connector's reported bpc for now until we add support for that > > Cheers, > > -ilia > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
On Thu, Sep 24, 2020 at 09:38:07PM +0200, Sam Ravnborg wrote: > Hi Guido. > > On Mon, Sep 21, 2020 at 06:55:52PM +0200, Guido Günther wrote: > > We need to reset both for the panel to show an image. > > > > Signed-off-by: Guido Günther > > --- > > .../bindings/display/panel/mantix,mlaf057we51-x.yaml | 7 +-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml > > b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml > > index 937323cc9aaa..ba5a18fac9f9 100644 > > --- > > a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml > > +++ > > b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml > > @@ -35,7 +35,9 @@ properties: > >vddi-supply: > > description: 1.8V I/O voltage supply > > > > - reset-gpios: true > > + reset-gpios: > > +minItems: 2 > > +maxItems: 2 > > reset-gpios is, as you already wrote, defined in panel-common.yaml. > Do not try to change it here. > It would be much better, I think, to introduce a mantix,reset-gpios > property. Yes. You also need to define what each reset entry corresponds to as the assertion/deassertion order could be important. You might just do 2 properties with -gpios. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209417] kernel-5.8*, amdgpu - Blank screen shortly after boot, "fixed" with suspend and wake up
https://bugzilla.kernel.org/show_bug.cgi?id=209417 --- Comment #3 from Juan (juantxor...@gmail.com) --- (In reply to Alex Deucher from comment #2) > Can you bisect? I never did it but I'm happy to help. It will take a while for me to do it, though. Any guidance would be appreciated. I assume that I will start with the first vanilla version that presented this problem, right? -- 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] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
On Tue, Sep 29, 2020 at 06:56:57PM +0200, Daniel Vetter wrote: > On Tue, Sep 29, 2020 at 12:52:03PM +0200, Martin Hostettler wrote: > > On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote: > > > On 2020/09/29 2:59, Martin Hostettler wrote: > > > > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote: > > > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what > > > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX. > > > >> > > > > > > > > It seems this is/was used by "svgatextmode" which seems to be at > > > > http://www.ibiblio.org/pub/Linux/utils/console/ > > > > > > > > Not sure if that kind of software still has a chance to work nowadays. > > > > > > > > > > Thanks for the information. > > > > > > It seems that v.v_vlin = curr_textmode->VDisplay / > > > (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1) > > > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's > > > height and seems to be non-zero. > > > But according to https://bugs.gentoo.org/19485 , people are using kernel > > > framebuffer instead. > > > > > > > Yes, this seems to be from pre framebuffer times. > > > > Back in the days "svga" was the wording you got for "pokes svga card > > hardware registers from userspace drivers". And textmode means font > > rendering is done via (fixed function in those times) hardware scanout > > engine. Of course having only to update 2 bytes per character was a huge > > saving early on. Likely this is also before vesa VBE was reliable. > > > > So i guess the point where this all starts going wrong allowing the X parts > > of the api to be combined with FB based rendering at all? Sounds the only > > user didn't use that combination and so it was never tested? > > > > Then again, this all relates to hardware from 20 years ago... > > Imo userspace modesetting should be burned down anywhere we can. We've > gotten away with this in drivers/gpu by just seamlessly transitioning to > kernel drivers. > > Since th only userspace we've found seems to be able to cope if this ioctl > doesn't do anything, my vote goes towards ripping it out completely and > doing nothing in there. Only question is whether we should error or fail > with a silent success: Former is safer, latter can avoid a few regression > reports since the userspace tools keep "working", and usually people don't > notice for stuff this old. It worked in drivers/gpu :-) This patch just ignores the ioctl and keeps on going, so userspace "shouldn't" notice it :) And it's in linux-next now, so all should be good. thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
On Tue, Sep 29, 2020 at 12:52:03PM +0200, Martin Hostettler wrote: > On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote: > > On 2020/09/29 2:59, Martin Hostettler wrote: > > > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote: > > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what > > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX. > > >> > > > > > > It seems this is/was used by "svgatextmode" which seems to be at > > > http://www.ibiblio.org/pub/Linux/utils/console/ > > > > > > Not sure if that kind of software still has a chance to work nowadays. > > > > > > > Thanks for the information. > > > > It seems that v.v_vlin = curr_textmode->VDisplay / > > (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1) > > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height > > and seems to be non-zero. > > But according to https://bugs.gentoo.org/19485 , people are using kernel > > framebuffer instead. > > > > Yes, this seems to be from pre framebuffer times. > > Back in the days "svga" was the wording you got for "pokes svga card > hardware registers from userspace drivers". And textmode means font > rendering is done via (fixed function in those times) hardware scanout > engine. Of course having only to update 2 bytes per character was a huge > saving early on. Likely this is also before vesa VBE was reliable. > > So i guess the point where this all starts going wrong allowing the X parts > of the api to be combined with FB based rendering at all? Sounds the only > user didn't use that combination and so it was never tested? > > Then again, this all relates to hardware from 20 years ago... Imo userspace modesetting should be burned down anywhere we can. We've gotten away with this in drivers/gpu by just seamlessly transitioning to kernel drivers. Since th only userspace we've found seems to be able to cope if this ioctl doesn't do anything, my vote goes towards ripping it out completely and doing nothing in there. Only question is whether we should error or fail with a silent success: Former is safer, latter can avoid a few regression reports since the userspace tools keep "working", and usually people don't notice for stuff this old. It worked in drivers/gpu :-) Cheers, 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] drm: update todo.rst
On Tue, Sep 29, 2020 at 12:51:24PM -0300, Melissa Wen wrote: > On 09/29, Daniel Vetter wrote: > > - debugfs cleanup has moved forward thanks to the cleanup Wambui has > > done > > > > Cc: Wambui Karuga > > Cc: Greg Kroah-Hartman > > Cc: Melissa Wen > > Signed-off-by: Daniel Vetter > > --- > > Documentation/gpu/todo.rst | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 3751ac976c3e..700637e25ecd 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -515,9 +515,6 @@ There's a bunch of issues with it: > >this (together with the drm_minor->drm_device move) would allow us to > > remove > >debugfs_init. > > > > -- Drop the return code and error checking from all debugfs functions. Greg > > KH is > > - working on this already. > > - > > Contact: Daniel Vetter > > > > Level: Intermediate > > -- > > 2.28.0 > > > Acked-by: Melissa Wen Queued up in drm-misc-next for 5.11 or so, thanks for taking a look. -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] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > Hi, > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > > register header". > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > > DRM driver together with DRM's fbdev emulation results in a failure > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > userspace driver relies on the ability to set the pixel format via > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > Not without kernel changes. See e.g. > > https://www.spinics.net/lists/dri-devel/msg255883.html > > That discussion seems to have died down with no further action. > > I also was kinda under the assumption that with Android these buffers > would be allocated directly from dma-buf heaps/ion, so this all seems not > that well baked out. > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > emulation, then let's do that. Not keep fbdev drivers on life support for > > > longer than necessary. > > > > That should have been done *before* removing the old driver, which was > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > userspace broken for the period of time that it would take to develop > > that change. Even if such a change were developed before 5.9 is > > released, it probably wouldn't be considered a bug fix that would be > > eligible for 5.9. > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > There have been other less critical behavioral differences identified > > > > > between the fbdev driver and the DRM driver with fbdev emulation. The > > > > > DRM driver exposes different values for the panel's width, height and > > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > > > > with yres_virtual greater than the maximum supported value instead > > > > > of letting the syscall succeed and setting yres_virtual based on yres. > > > > > > Also something that should be fixed I think in upstream, in the drm fbdev > > > emulation. At least doesn't sound like it's something unfixable. > > > > Again, it should have been fixed before removing the old driver. > > Yeah, but we also don't have a whole lot people who seem to push for this. > So the only way to find the people who still care is to remove the fbdev > drivers. > > fbdev is dead. I'm totally fine reverting the driver to shut up the > regression report, but it's not fixing anything long term. There's not > going to be new drivers, or new hw support in existing drivers in fbdev. > > Also note that there's not spec nor test suite for fbdev, so "you guys > should have known before removing drivers" doesn't work. btw revert itself is stuck somewhere, so I can't find it. And for the revert can you pls just resurrect the files in drivers/video, without touching anything in drivers/gpu? Tying down drm drivers with stuff in fbdev doesn't make much sense to me. Thanks, Daniel > -Daniel > > > > > Peter > > > > > -Daniel > > > > > > > > > > > > > Signed-off-by: Peter Collingbourne > > > > > --- > > > > > View this change in Gerrit: > > > > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > > > > > > > MAINTAINERS | 5 + > > > > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > > > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > > > > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > > > > > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > > > > > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > > > > > drivers/video/fbdev/Kconfig | 20 + > > > > > drivers/video/fbdev/Makefile| 1 + > > > > > drivers/video/fbdev/amba-clcd.c | 986 > > > > > > > > > > include/linux/amba/clcd-regs.h | 87 +++ > > > > > include/linux/amba/clcd.h | 290 +++ > > > > > 11 files changed, 1393 insertions(+), 73 deletions(-) > > > > > create mode 100644 drivers/video/fbdev/amba-clcd.c > > > > > create mode 100644 include/linux/amba/clcd-regs.h > > > > > create mode 100644 include/linux/amba/clcd.h > > > > > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > > index 190c7fa2ea01..671c1fa79e64 100644 > > > > > --- a/MAINTAINERS > > > > > +++ b/MAINTAINERS > > > > > @@ -1460,6 +1460,11 @@ S: Odd Fixes > > > > > F: drivers/amba/ > > > > > F: include/linux/amba/bus.h > > > > > >
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > Hi, > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > register header". > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > DRM driver together with DRM's fbdev emulation results in a failure > > > > to boot Android. The root cause is that Android's generic fbdev > > > > userspace driver relies on the ability to set the pixel format via > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > Not without kernel changes. See e.g. > https://www.spinics.net/lists/dri-devel/msg255883.html That discussion seems to have died down with no further action. I also was kinda under the assumption that with Android these buffers would be allocated directly from dma-buf heaps/ion, so this all seems not that well baked out. > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > emulation, then let's do that. Not keep fbdev drivers on life support for > > longer than necessary. > > That should have been done *before* removing the old driver, which was > a userspace break that was introduced in 5.9rc1. We shouldn't leave > userspace broken for the period of time that it would take to develop > that change. Even if such a change were developed before 5.9 is > released, it probably wouldn't be considered a bug fix that would be > eligible for 5.9. > > > > > > > > > Neil > > > > > > > > > > > There have been other less critical behavioral differences identified > > > > between the fbdev driver and the DRM driver with fbdev emulation. The > > > > DRM driver exposes different values for the panel's width, height and > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > > > with yres_virtual greater than the maximum supported value instead > > > > of letting the syscall succeed and setting yres_virtual based on yres. > > > > Also something that should be fixed I think in upstream, in the drm fbdev > > emulation. At least doesn't sound like it's something unfixable. > > Again, it should have been fixed before removing the old driver. Yeah, but we also don't have a whole lot people who seem to push for this. So the only way to find the people who still care is to remove the fbdev drivers. fbdev is dead. I'm totally fine reverting the driver to shut up the regression report, but it's not fixing anything long term. There's not going to be new drivers, or new hw support in existing drivers in fbdev. Also note that there's not spec nor test suite for fbdev, so "you guys should have known before removing drivers" doesn't work. -Daniel > > Peter > > > -Daniel > > > > > > > > > > Signed-off-by: Peter Collingbourne > > > > --- > > > > View this change in Gerrit: > > > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > > > > > MAINTAINERS | 5 + > > > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > > > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > > > > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > > > > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > > > > drivers/video/fbdev/Kconfig | 20 + > > > > drivers/video/fbdev/Makefile| 1 + > > > > drivers/video/fbdev/amba-clcd.c | 986 > > > > include/linux/amba/clcd-regs.h | 87 +++ > > > > include/linux/amba/clcd.h | 290 +++ > > > > 11 files changed, 1393 insertions(+), 73 deletions(-) > > > > create mode 100644 drivers/video/fbdev/amba-clcd.c > > > > create mode 100644 include/linux/amba/clcd-regs.h > > > > create mode 100644 include/linux/amba/clcd.h > > > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > index 190c7fa2ea01..671c1fa79e64 100644 > > > > --- a/MAINTAINERS > > > > +++ b/MAINTAINERS > > > > @@ -1460,6 +1460,11 @@ S: Odd Fixes > > > > F: drivers/amba/ > > > > F: include/linux/amba/bus.h > > > > > > > > +ARM PRIMECELL CLCD PL110 DRIVER > > > > +M: Russell King > > > > +S: Odd Fixes > > > > +F: drivers/video/fbdev/amba-clcd.* > > > > + > > > > ARM PRIMECELL KMI PL050 DRIVER > > > > M: Russell King > > > > S: Odd Fixes > > > > diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > b/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > index 317f68abf18b..26ca8cdf3e60 100644 > > > > --- a/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > @@ -3,6 +3,7 @@ > > > > * Copyright © 2017
Re: [PATCH] drm: update todo.rst
On Tue, Sep 29, 2020 at 05:03:33PM +0200, Daniel Vetter wrote: > - debugfs cleanup has moved forward thanks to the cleanup Wambui has > done > > Cc: Wambui Karuga > Cc: Greg Kroah-Hartman > Cc: Melissa Wen > Signed-off-by: Daniel Vetter > --- > Documentation/gpu/todo.rst | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 3751ac976c3e..700637e25ecd 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -515,9 +515,6 @@ There's a bunch of issues with it: >this (together with the drm_minor->drm_device move) would allow us to > remove >debugfs_init. > > -- Drop the return code and error checking from all debugfs functions. Greg > KH is > - working on this already. > - > Contact: Daniel Vetter > > Level: Intermediate Nice! Reviewed-by: Greg Kroah-Hartman ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: update todo.rst
On 09/29, Daniel Vetter wrote: > - debugfs cleanup has moved forward thanks to the cleanup Wambui has > done > > Cc: Wambui Karuga > Cc: Greg Kroah-Hartman > Cc: Melissa Wen > Signed-off-by: Daniel Vetter > --- > Documentation/gpu/todo.rst | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 3751ac976c3e..700637e25ecd 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -515,9 +515,6 @@ There's a bunch of issues with it: >this (together with the drm_minor->drm_device move) would allow us to > remove >debugfs_init. > > -- Drop the return code and error checking from all debugfs functions. Greg > KH is > - working on this already. > - > Contact: Daniel Vetter > > Level: Intermediate > -- > 2.28.0 > Acked-by: Melissa Wen ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/atomic: debug output for EBUSY
Hi, On Fri, 25 Sep 2020 at 09:46, Daniel Vetter wrote: > Hopefully we'll have the drm crash recorder RSN, but meanwhile > compositors would like to know a bit better why they get an EBUSY. Thanks a lot, this is super helpful! Both patches are: Reviewed-by: Daniel Stone Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Tue, Sep 29, 2020 at 5:35 PM Christian König
wrote:
>
> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann:
> > The new helper ttm_kmap_obj_to_dma_buf() extracts address and location
> > from and instance of TTM's kmap_obj and initializes struct dma_buf_map
> > with these values. Helpful for TTM-based drivers.
>
> We could completely drop that if we use the same structure inside TTM as
> well.
> Additional to that which driver is going to use this?
Patch 3 in this series.
I also think this makes sense for gradual conversion:
1. add this helper
2. convert over all users of vmap, this should get rid of is_iomem
flags (and will probably result in a pile of small additions to
dma-buf-map.h)
3. push the struct dma_buf_map down into ttm drivers
Cheers, Daniel
> Regards,
> Christian.
>
> >
> > Signed-off-by: Thomas Zimmermann
> > ---
> > include/drm/ttm/ttm_bo_api.h | 24
> > include/linux/dma-buf-map.h | 20
> > 2 files changed, 44 insertions(+)
> >
> > diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> > index c96a25d571c8..62d89f05a801 100644
> > --- a/include/drm/ttm/ttm_bo_api.h
> > +++ b/include/drm/ttm/ttm_bo_api.h
> > @@ -34,6 +34,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct
> > ttm_bo_kmap_obj *map,
> > return map->virtual;
> > }
> >
> > +/**
> > + * ttm_kmap_obj_to_dma_buf_map
> > + *
> > + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap.
> > + * @map: Returns the mapping as struct dma_buf_map
> > + *
> > + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the memory
> > + * is not mapped, the returned mapping is initialized to NULL.
> > + */
> > +static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj
> > *kmap,
> > +struct dma_buf_map *map)
> > +{
> > + bool is_iomem;
> > + void *vaddr = ttm_kmap_obj_virtual(kmap, _iomem);
> > +
> > + if (!vaddr)
> > + dma_buf_map_clear(map);
> > + else if (is_iomem)
> > + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem
> > *)vaddr);
> > + else
> > + dma_buf_map_set_vaddr(map, vaddr);
> > +}
> > +
> > /**
> >* ttm_bo_kmap
> >*
> > diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
> > index fd1aba545fdf..2e8bbecb5091 100644
> > --- a/include/linux/dma-buf-map.h
> > +++ b/include/linux/dma-buf-map.h
> > @@ -45,6 +45,12 @@
> >*
> >* dma_buf_map_set_vaddr( 0xdeadbeaf);
> >*
> > + * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem().
> > + *
> > + * .. code-block:: c
> > + *
> > + * dma_buf_map_set_vaddr_iomem( 0xdeadbeaf);
> > + *
> >* Test if a mapping is valid with either dma_buf_map_is_set() or
> >* dma_buf_map_is_null().
> >*
> > @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct
> > dma_buf_map *map, void *vaddr)
> > map->is_iomem = false;
> > }
> >
> > +/**
> > + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to an
> > address in I/O memory
> > + * @map: The dma-buf mapping structure
> > + * @vaddr_iomem: An I/O-memory address
> > + *
> > + * Sets the address and the I/O-memory flag.
> > + */
> > +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map,
> > +void __iomem *vaddr_iomem)
> > +{
> > + map->vaddr_iomem = vaddr_iomem;
> > + map->is_iomem = true;
> > +}
> > +
> > /**
> >* dma_buf_map_is_equal - Compares two dma-buf mapping structures for
> > equality
> >* @lhs:The dma-buf mapping structure
>
--
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 v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
Am 29.09.20 um 17:14 schrieb Thomas Zimmermann:
The new helper ttm_kmap_obj_to_dma_buf() extracts address and location
from and instance of TTM's kmap_obj and initializes struct dma_buf_map
with these values. Helpful for TTM-based drivers.
We could completely drop that if we use the same structure inside TTM as
well.
Additional to that which driver is going to use this?
Regards,
Christian.
Signed-off-by: Thomas Zimmermann
---
include/drm/ttm/ttm_bo_api.h | 24
include/linux/dma-buf-map.h | 20
2 files changed, 44 insertions(+)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index c96a25d571c8..62d89f05a801 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct
ttm_bo_kmap_obj *map,
return map->virtual;
}
+/**
+ * ttm_kmap_obj_to_dma_buf_map
+ *
+ * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap.
+ * @map: Returns the mapping as struct dma_buf_map
+ *
+ * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the memory
+ * is not mapped, the returned mapping is initialized to NULL.
+ */
+static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj *kmap,
+ struct dma_buf_map *map)
+{
+ bool is_iomem;
+ void *vaddr = ttm_kmap_obj_virtual(kmap, _iomem);
+
+ if (!vaddr)
+ dma_buf_map_clear(map);
+ else if (is_iomem)
+ dma_buf_map_set_vaddr_iomem(map, (void __force __iomem *)vaddr);
+ else
+ dma_buf_map_set_vaddr(map, vaddr);
+}
+
/**
* ttm_bo_kmap
*
diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
index fd1aba545fdf..2e8bbecb5091 100644
--- a/include/linux/dma-buf-map.h
+++ b/include/linux/dma-buf-map.h
@@ -45,6 +45,12 @@
*
*dma_buf_map_set_vaddr( 0xdeadbeaf);
*
+ * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem().
+ *
+ * .. code-block:: c
+ *
+ * dma_buf_map_set_vaddr_iomem( 0xdeadbeaf);
+ *
* Test if a mapping is valid with either dma_buf_map_is_set() or
* dma_buf_map_is_null().
*
@@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct
dma_buf_map *map, void *vaddr)
map->is_iomem = false;
}
+/**
+ * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to an
address in I/O memory
+ * @map: The dma-buf mapping structure
+ * @vaddr_iomem: An I/O-memory address
+ *
+ * Sets the address and the I/O-memory flag.
+ */
+static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map,
+ void __iomem *vaddr_iomem)
+{
+ map->vaddr_iomem = vaddr_iomem;
+ map->is_iomem = true;
+}
+
/**
* dma_buf_map_is_equal - Compares two dma-buf mapping structures for equality
* @lhs: The dma-buf mapping structure
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 6/7] drm/fb_helper: Support framebuffers in I/O memory
At least sparc64 requires I/O-specific access to framebuffers. This
patch updates the fbdev console accordingly.
For drivers with direct access to the framebuffer memory, the callback
functions in struct fb_ops test for the type of memory and call the rsp
fb_sys_ of fb_cfb_ functions.
For drivers that employ a shadow buffer, fbdev's blit function retrieves
the framebuffer address as struct dma_buf_map, and uses dma_buf_map
interfaces to access the buffer.
The bochs driver on sparc64 uses a workaround to flag the framebuffer as
I/O memory and avoid a HW exception. With the introduction of struct
dma_buf_map, this is not required any longer. The patch removes the rsp
code from both, bochs and fbdev.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/bochs/bochs_kms.c | 1 -
drivers/gpu/drm/drm_fb_helper.c | 217 --
include/drm/drm_mode_config.h | 12 --
include/linux/dma-buf-map.h | 72 --
4 files changed, 265 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
b/drivers/gpu/drm/bochs/bochs_kms.c
index 13d0d04c4457..853081d186d5 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
bochs->dev->mode_config.preferred_depth = 24;
bochs->dev->mode_config.prefer_shadow = 0;
bochs->dev->mode_config.prefer_shadow_fbdev = 1;
- bochs->dev->mode_config.fbdev_use_iomem = true;
bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
bochs->dev->mode_config.funcs = _mode_funcs;
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 343a292f2c7c..f345a314a437 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -388,24 +388,22 @@ static void drm_fb_helper_resume_worker(struct
work_struct *work)
}
static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
- struct drm_clip_rect *clip)
+ struct drm_clip_rect *clip,
+ struct dma_buf_map *dst)
{
struct drm_framebuffer *fb = fb_helper->fb;
unsigned int cpp = fb->format->cpp[0];
size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
void *src = fb_helper->fbdev->screen_buffer + offset;
- void *dst = fb_helper->buffer->map.vaddr + offset;
size_t len = (clip->x2 - clip->x1) * cpp;
unsigned int y;
- for (y = clip->y1; y < clip->y2; y++) {
- if (!fb_helper->dev->mode_config.fbdev_use_iomem)
- memcpy(dst, src, len);
- else
- memcpy_toio((void __iomem *)dst, src, len);
+ dma_buf_map_incr(dst, offset); /* go to first pixel within clip rect */
+ for (y = clip->y1; y < clip->y2; y++) {
+ dma_buf_map_memcpy_to(dst, src, len);
+ dma_buf_map_incr(dst, fb->pitches[0]);
src += fb->pitches[0];
- dst += fb->pitches[0];
}
}
@@ -433,8 +431,9 @@ static void drm_fb_helper_dirty_work(struct work_struct
*work)
ret = drm_client_buffer_vmap(helper->buffer, );
if (ret)
return;
- drm_fb_helper_dirty_blit_real(helper, _copy);
+ drm_fb_helper_dirty_blit_real(helper, _copy, );
}
+
if (helper->fb->funcs->dirty)
helper->fb->funcs->dirty(helper->fb, NULL, 0, 0,
_copy, 1);
@@ -771,6 +770,136 @@ void drm_fb_helper_sys_imageblit(struct fb_info *info,
}
EXPORT_SYMBOL(drm_fb_helper_sys_imageblit);
+static ssize_t drm_fb_helper_cfb_read(struct fb_info *info, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ unsigned long p = *ppos;
+ u8 *dst;
+ u8 __iomem *src;
+ int c, err = 0;
+ unsigned long total_size;
+ unsigned long alloc_size;
+ ssize_t ret = 0;
+
+ if (info->state != FBINFO_STATE_RUNNING)
+ return -EPERM;
+
+ total_size = info->screen_size;
+
+ if (total_size == 0)
+ total_size = info->fix.smem_len;
+
+ if (p >= total_size)
+ return 0;
+
+ if (count >= total_size)
+ count = total_size;
+
+ if (count + p > total_size)
+ count = total_size - p;
+
+ src = (u8 __iomem *)(info->screen_base + p);
+
+ alloc_size = min(count, PAGE_SIZE);
+
+ dst = kmalloc(alloc_size, GFP_KERNEL);
+ if (!dst)
+ return -ENOMEM;
+
+ while (count) {
+ c = min(count, alloc_size);
+
+ memcpy_fromio(dst, src, c);
+ if (copy_to_user(buf, dst, c)) {
+ err =
[PATCH v3 4/7] drm/gem: Update internal GEM vmap/vunmap interfaces to use struct dma_buf_map
GEM's vmap and vunmap interfaces now wrap memory pointers in struct
dma_buf_map.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_client.c | 18 +++---
drivers/gpu/drm/drm_gem.c | 28 ++--
drivers/gpu/drm/drm_internal.h | 5 +++--
drivers/gpu/drm/drm_prime.c| 14 --
4 files changed, 32 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 495f47d23d87..ac0082bed966 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -3,6 +3,7 @@
* Copyright 2018 Noralf Trønnes
*/
+#include
#include
#include
#include
@@ -304,7 +305,8 @@ drm_client_buffer_create(struct drm_client_dev *client, u32
width, u32 height, u
*/
void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
{
- void *vaddr;
+ struct dma_buf_map map;
+ int ret;
if (buffer->vaddr)
return buffer->vaddr;
@@ -317,13 +319,13 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
*buffer)
* fd_install step out of the driver backend hooks, to make that
* final step optional for internal users.
*/
- vaddr = drm_gem_vmap(buffer->gem);
- if (IS_ERR(vaddr))
- return vaddr;
+ ret = drm_gem_vmap(buffer->gem, );
+ if (ret)
+ return ERR_PTR(ret);
- buffer->vaddr = vaddr;
+ buffer->vaddr = map.vaddr;
- return vaddr;
+ return map.vaddr;
}
EXPORT_SYMBOL(drm_client_buffer_vmap);
@@ -337,7 +339,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
*/
void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
{
- drm_gem_vunmap(buffer->gem, buffer->vaddr);
+ struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
+
+ drm_gem_vunmap(buffer->gem, );
buffer->vaddr = NULL;
}
EXPORT_SYMBOL(drm_client_buffer_vunmap);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 0c4a66dea5c2..f2b2f37d41c4 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1205,32 +1205,32 @@ void drm_gem_unpin(struct drm_gem_object *obj)
obj->funcs->unpin(obj);
}
-void *drm_gem_vmap(struct drm_gem_object *obj)
+int drm_gem_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
{
- struct dma_buf_map map;
int ret;
- if (!obj->funcs->vmap) {
- return ERR_PTR(-EOPNOTSUPP);
+ if (!obj->funcs->vmap)
+ return -EOPNOTSUPP;
- ret = obj->funcs->vmap(obj, );
+ ret = obj->funcs->vmap(obj, map);
if (ret)
- return ERR_PTR(ret);
- else if (dma_buf_map_is_null())
- return ERR_PTR(-ENOMEM);
+ return ret;
+ else if (dma_buf_map_is_null(map))
+ return -ENOMEM;
- return map.vaddr;
+ return 0;
}
-void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr)
+void drm_gem_vunmap(struct drm_gem_object *obj, struct dma_buf_map *map)
{
- struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(vaddr);
-
- if (!vaddr)
+ if (dma_buf_map_is_null(map))
return;
if (obj->funcs->vunmap)
- obj->funcs->vunmap(obj, );
+ obj->funcs->vunmap(obj, map);
+
+ /* Always set the mapping to NULL. Callers may rely on this. */
+ dma_buf_map_clear(map);
}
/**
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index b65865c630b0..58832d75a9bd 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -33,6 +33,7 @@
struct dentry;
struct dma_buf;
+struct dma_buf_map;
struct drm_connector;
struct drm_crtc;
struct drm_framebuffer;
@@ -187,8 +188,8 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int
indent,
int drm_gem_pin(struct drm_gem_object *obj);
void drm_gem_unpin(struct drm_gem_object *obj);
-void *drm_gem_vmap(struct drm_gem_object *obj);
-void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr);
+int drm_gem_vmap(struct drm_gem_object *obj, struct dma_buf_map *map);
+void drm_gem_vunmap(struct drm_gem_object *obj, struct dma_buf_map *map);
/* drm_debugfs.c drm_debugfs_crc.c */
#if defined(CONFIG_DEBUG_FS)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 89e2a2496734..cb8fbeeb731b 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -667,21 +667,15 @@ EXPORT_SYMBOL(drm_gem_unmap_dma_buf);
*
* Sets up a kernel virtual mapping. This can be used as the _buf_ops.vmap
* callback. Calls into _gem_object_funcs.vmap for device specific
handling.
+ * The kernel virtual address is returned in map.
*
- * Returns the kernel virtual address or NULL on failure.
+ * Returns 0 on success or a negative errno code otherwise.
*/
int drm_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct dma_buf_map *map)
{
struct drm_gem_object *obj = dma_buf->priv;
-
[PATCH v3 3/7] drm/gem: Use struct dma_buf_map in GEM vmap ops and convert GEM backends
This patch replaces the vmap/vunmap's use of raw pointers in GEM object
functions with instances of struct dma_buf_map. GEM backends are
converted as well.
For most GEM backends, this simply change the returned type. GEM VRAM
helpers are also updated to indicate whether the returned framebuffer
address is in system or I/O memory.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 14 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 4 +-
drivers/gpu/drm/ast/ast_cursor.c| 29 +++
drivers/gpu/drm/ast/ast_drv.h | 7 +-
drivers/gpu/drm/drm_gem.c | 22 ++---
drivers/gpu/drm/drm_gem_cma_helper.c| 14 ++--
drivers/gpu/drm/drm_gem_shmem_helper.c | 48 ++-
drivers/gpu/drm/drm_gem_vram_helper.c | 90 +++--
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 4 +-
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 11 ++-
drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +-
drivers/gpu/drm/exynos/exynos_drm_gem.h | 4 +-
drivers/gpu/drm/lima/lima_gem.c | 6 +-
drivers/gpu/drm/lima/lima_sched.c | 11 ++-
drivers/gpu/drm/mgag200/mgag200_mode.c | 12 +--
drivers/gpu/drm/nouveau/nouveau_gem.h | 4 +-
drivers/gpu/drm/nouveau/nouveau_prime.c | 9 ++-
drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 14 ++--
drivers/gpu/drm/qxl/qxl_display.c | 13 +--
drivers/gpu/drm/qxl/qxl_draw.c | 16 ++--
drivers/gpu/drm/qxl/qxl_drv.h | 8 +-
drivers/gpu/drm/qxl/qxl_object.c| 23 +++---
drivers/gpu/drm/qxl/qxl_object.h| 2 +-
drivers/gpu/drm/qxl/qxl_prime.c | 12 +--
drivers/gpu/drm/radeon/radeon_gem.c | 4 +-
drivers/gpu/drm/radeon/radeon_prime.c | 9 ++-
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 22 +++--
drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 4 +-
drivers/gpu/drm/tiny/cirrus.c | 10 ++-
drivers/gpu/drm/tiny/gm12u320.c | 10 ++-
drivers/gpu/drm/udl/udl_modeset.c | 8 +-
drivers/gpu/drm/vboxvideo/vbox_mode.c | 11 ++-
drivers/gpu/drm/vc4/vc4_bo.c| 6 +-
drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
drivers/gpu/drm/vgem/vgem_drv.c | 16 ++--
drivers/gpu/drm/xen/xen_drm_front_gem.c | 18 +++--
drivers/gpu/drm/xen/xen_drm_front_gem.h | 6 +-
include/drm/drm_gem.h | 5 +-
include/drm/drm_gem_cma_helper.h| 4 +-
include/drm/drm_gem_shmem_helper.h | 4 +-
include/drm/drm_gem_vram_helper.h | 4 +-
41 files changed, 304 insertions(+), 222 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 5b465ab774d1..de7d0cfe1b93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -44,13 +44,14 @@
/**
* amdgpu_gem_prime_vmap - _buf_ops.vmap implementation
* @obj: GEM BO
+ * @map: The virtual address of the mapping.
*
* Sets up an in-kernel virtual mapping of the BO's memory.
*
* Returns:
- * The virtual address of the mapping or an error pointer.
+ * 0 on success, or a negative errno code otherwise.
*/
-void *amdgpu_gem_prime_vmap(struct drm_gem_object *obj)
+int amdgpu_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
{
struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
int ret;
@@ -58,19 +59,20 @@ void *amdgpu_gem_prime_vmap(struct drm_gem_object *obj)
ret = ttm_bo_kmap(>tbo, 0, bo->tbo.num_pages,
>dma_buf_vmap);
if (ret)
- return ERR_PTR(ret);
+ return ret;
+ ttm_kmap_obj_to_dma_buf_map(>dma_buf_vmap, map);
- return bo->dma_buf_vmap.virtual;
+ return 0;
}
/**
* amdgpu_gem_prime_vunmap - _buf_ops.vunmap implementation
* @obj: GEM BO
- * @vaddr: Virtual address (unused)
+ * @map: Virtual address (unused)
*
* Tears down the in-kernel virtual mapping of the BO's memory.
*/
-void amdgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
+void amdgpu_gem_prime_vunmap(struct drm_gem_object *obj, struct dma_buf_map
*map)
{
struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 2c5c84a06bb9..622642793064 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,8 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct
drm_device *dev,
struct dma_buf *dma_buf);
bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
struct amdgpu_bo *bo);
-void *amdgpu_gem_prime_vmap(struct drm_gem_object *obj);
-void amdgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
+int amdgpu_gem_prime_vmap(struct drm_gem_object
[PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
The new helper ttm_kmap_obj_to_dma_buf() extracts address and location
from and instance of TTM's kmap_obj and initializes struct dma_buf_map
with these values. Helpful for TTM-based drivers.
Signed-off-by: Thomas Zimmermann
---
include/drm/ttm/ttm_bo_api.h | 24
include/linux/dma-buf-map.h | 20
2 files changed, 44 insertions(+)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index c96a25d571c8..62d89f05a801 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct
ttm_bo_kmap_obj *map,
return map->virtual;
}
+/**
+ * ttm_kmap_obj_to_dma_buf_map
+ *
+ * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap.
+ * @map: Returns the mapping as struct dma_buf_map
+ *
+ * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the memory
+ * is not mapped, the returned mapping is initialized to NULL.
+ */
+static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj *kmap,
+ struct dma_buf_map *map)
+{
+ bool is_iomem;
+ void *vaddr = ttm_kmap_obj_virtual(kmap, _iomem);
+
+ if (!vaddr)
+ dma_buf_map_clear(map);
+ else if (is_iomem)
+ dma_buf_map_set_vaddr_iomem(map, (void __force __iomem *)vaddr);
+ else
+ dma_buf_map_set_vaddr(map, vaddr);
+}
+
/**
* ttm_bo_kmap
*
diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
index fd1aba545fdf..2e8bbecb5091 100644
--- a/include/linux/dma-buf-map.h
+++ b/include/linux/dma-buf-map.h
@@ -45,6 +45,12 @@
*
* dma_buf_map_set_vaddr( 0xdeadbeaf);
*
+ * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem().
+ *
+ * .. code-block:: c
+ *
+ * dma_buf_map_set_vaddr_iomem( 0xdeadbeaf);
+ *
* Test if a mapping is valid with either dma_buf_map_is_set() or
* dma_buf_map_is_null().
*
@@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct
dma_buf_map *map, void *vaddr)
map->is_iomem = false;
}
+/**
+ * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to an
address in I/O memory
+ * @map: The dma-buf mapping structure
+ * @vaddr_iomem: An I/O-memory address
+ *
+ * Sets the address and the I/O-memory flag.
+ */
+static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map,
+ void __iomem *vaddr_iomem)
+{
+ map->vaddr_iomem = vaddr_iomem;
+ map->is_iomem = true;
+}
+
/**
* dma_buf_map_is_equal - Compares two dma-buf mapping structures for equality
* @lhs: The dma-buf mapping structure
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/7] Support GEM object mappings from I/O memory
DRM's fbdev console uses regular load and store operations to update framebuffer memory. The bochs driver on sparc64 requires the use of I/O-specific load and store operations. We have a workaround, but need a long-term solution to the problem. This patchset changes GEM's vmap/vunmap interfaces to forward pointers of type struct dma_buf_map and updates the generic fbdev emulation to use them correctly. This enables I/O-memory operations on all framebuffers that require and support them. Patches #1 and #2 prepare VRAM helpers and TTM for the changes in patch #3. Patch #3 updates GEM's vmap/vunmap callback to forward instances of type struct dma_buf_map. While the patch touches many files throughout the DRM modules, the applied changes are mostly trivial interface fixes. Patch #4 updates GEM's internal vmap/vunmap functions to forward struct dma_buf_map. Patches #5 and #6 convert DRM clients and generic fbdev emulation to use struct dma_buf_map. Updating the fbdev framebuffer will select the correct functions, either for system or I/O memory. The patch set is just enough to fix the bochs issue on sparc64 and a correct way. Patch #7 updates the TODO list with ideas for further improvements v3: * recreate the whole patchset on top of struct dma_buf_map v2: * RFC patchset Thomas Zimmermann (7): drm/vram-helper: Remove invariant parameters from internal kmap function drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion drm/gem: Use struct dma_buf_map in GEM vmap ops and convert GEM backends drm/gem: Update internal GEM vmap/vunmap interfaces to use struct dma_buf_map drm/gem: Store client buffer mappings as struct dma_buf_map drm/fb_helper: Support framebuffers in I/O memory drm/todo: Update entries around struct dma_buf_map Documentation/gpu/todo.rst | 24 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 14 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 4 +- drivers/gpu/drm/ast/ast_cursor.c| 29 ++- drivers/gpu/drm/ast/ast_drv.h | 7 +- drivers/gpu/drm/bochs/bochs_kms.c | 1 - drivers/gpu/drm/drm_client.c| 38 ++-- drivers/gpu/drm/drm_fb_helper.c | 238 ++-- drivers/gpu/drm/drm_gem.c | 28 ++- drivers/gpu/drm/drm_gem_cma_helper.c| 14 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 48 ++-- drivers/gpu/drm/drm_gem_vram_helper.c | 93 drivers/gpu/drm/drm_internal.h | 5 +- drivers/gpu/drm/drm_prime.c | 14 +- drivers/gpu/drm/etnaviv/etnaviv_drv.h | 4 +- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 11 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +- drivers/gpu/drm/exynos/exynos_drm_gem.h | 4 +- drivers/gpu/drm/lima/lima_gem.c | 6 +- drivers/gpu/drm/lima/lima_sched.c | 11 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 12 +- drivers/gpu/drm/nouveau/nouveau_gem.h | 4 +- drivers/gpu/drm/nouveau/nouveau_prime.c | 9 +- drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 14 +- drivers/gpu/drm/qxl/qxl_display.c | 13 +- drivers/gpu/drm/qxl/qxl_draw.c | 16 +- drivers/gpu/drm/qxl/qxl_drv.h | 8 +- drivers/gpu/drm/qxl/qxl_object.c| 23 +- drivers/gpu/drm/qxl/qxl_object.h| 2 +- drivers/gpu/drm/qxl/qxl_prime.c | 12 +- drivers/gpu/drm/radeon/radeon_gem.c | 4 +- drivers/gpu/drm/radeon/radeon_prime.c | 9 +- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 22 +- drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 4 +- drivers/gpu/drm/tiny/cirrus.c | 10 +- drivers/gpu/drm/tiny/gm12u320.c | 10 +- drivers/gpu/drm/udl/udl_modeset.c | 8 +- drivers/gpu/drm/vboxvideo/vbox_mode.c | 11 +- drivers/gpu/drm/vc4/vc4_bo.c| 6 +- drivers/gpu/drm/vc4/vc4_drv.h | 2 +- drivers/gpu/drm/vgem/vgem_drv.c | 16 +- drivers/gpu/drm/xen/xen_drm_front_gem.c | 18 +- drivers/gpu/drm/xen/xen_drm_front_gem.h | 6 +- include/drm/drm_client.h| 7 +- include/drm/drm_gem.h | 5 +- include/drm/drm_gem_cma_helper.h| 4 +- include/drm/drm_gem_shmem_helper.h | 4 +- include/drm/drm_gem_vram_helper.h | 4 +- include/drm/drm_mode_config.h | 12 - include/drm/ttm/ttm_bo_api.h| 24 ++ include/linux/dma-buf-map.h | 92 +++- 51 files changed, 685 insertions(+), 305 deletions(-) -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/7] drm/vram-helper: Remove invariant parameters from internal kmap function
The parameters map and is_iomem are always of the same value. Removed them
to prepares the function for conversion to struct dma_buf_map.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_vram_helper.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 3fe4b326e18e..256b346664f2 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -382,16 +382,16 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
}
EXPORT_SYMBOL(drm_gem_vram_unpin);
-static void *drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo,
- bool map, bool *is_iomem)
+static void *drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo)
{
int ret;
struct ttm_bo_kmap_obj *kmap = >kmap;
+ bool is_iomem;
if (gbo->kmap_use_count > 0)
goto out;
- if (kmap->virtual || !map)
+ if (kmap->virtual)
goto out;
ret = ttm_bo_kmap(>bo, 0, gbo->bo.num_pages, kmap);
@@ -399,15 +399,10 @@ static void *drm_gem_vram_kmap_locked(struct
drm_gem_vram_object *gbo,
return ERR_PTR(ret);
out:
- if (!kmap->virtual) {
- if (is_iomem)
- *is_iomem = false;
+ if (!kmap->virtual)
return NULL; /* not mapped; don't increment ref */
- }
++gbo->kmap_use_count;
- if (is_iomem)
- return ttm_kmap_obj_virtual(kmap, is_iomem);
- return kmap->virtual;
+ return ttm_kmap_obj_virtual(kmap, _iomem);
}
static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
@@ -452,7 +447,7 @@ void *drm_gem_vram_vmap(struct drm_gem_vram_object *gbo)
ret = drm_gem_vram_pin_locked(gbo, 0);
if (ret)
goto err_ttm_bo_unreserve;
- base = drm_gem_vram_kmap_locked(gbo, true, NULL);
+ base = drm_gem_vram_kmap_locked(gbo);
if (IS_ERR(base)) {
ret = PTR_ERR(base);
goto err_drm_gem_vram_unpin_locked;
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 7/7] drm/todo: Update entries around struct dma_buf_map
Instances of struct dma_buf_map should be useful throughout DRM's memory management code. Furthermore, several drivers can now use generic fbdev emulation. Signed-off-by: Thomas Zimmermann --- Documentation/gpu/todo.rst | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 3751ac976c3e..023626c1837b 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -197,8 +197,10 @@ Convert drivers to use drm_fbdev_generic_setup() Most drivers can use drm_fbdev_generic_setup(). Driver have to implement -atomic modesetting and GEM vmap support. Current generic fbdev emulation -expects the framebuffer in system memory (or system-like memory). +atomic modesetting and GEM vmap support. Historically, generic fbdev emulation +expected the framebuffer in system memory or system-like memory. By employing +struct dma_buf_map, drivers with frambuffers in I/O memory can be supported +as well. Contact: Maintainer of the driver you plan to convert @@ -446,6 +448,24 @@ Contact: Ville Syrjälä, Daniel Vetter Level: Intermediate +Use struct dma_buf_map throughout codebase +-- + +Pointers to shared device memory are stored in struct dma_buf_map. Each +instance knows whether it refers to system or I/O memory. Most of the DRM-wide +interface have been converted to use struct dma_buf_map, but implementations +often still use raw pointers. + +The task is to use struct dma_buf_map where it makes sense. + +* Memory managers should use struct dma_buf_map for dma-buf-imported buffers. +* TTM might benefit from using struct dma_buf_map internally. +* Framebuffer copying and blitting helpers should operate on struct dma_buf_map. + +Contact: Thomas Zimmermann , Christian König, Daniel Vetter + +Level: Intermediate + Core refactorings = -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 5/7] drm/gem: Store client buffer mappings as struct dma_buf_map
Kernel DRM clients now store their framebuffer address in an instance
of struct dma_buf_map. Depending on the buffer's location, the address
refers to system or I/O memory.
Callers of drm_client_buffer_vmap() receive a copy of the value in
the call's supplied arguments. It can be accessed and modified with
dma_buf_map interfaces.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_client.c| 34 +++--
drivers/gpu/drm/drm_fb_helper.c | 23 +-
include/drm/drm_client.h| 7 ---
3 files changed, 38 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index ac0082bed966..fe573acf1067 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
drm_client_buffer *buffer)
{
struct drm_device *dev = buffer->client->dev;
- drm_gem_vunmap(buffer->gem, buffer->vaddr);
+ drm_gem_vunmap(buffer->gem, >map);
if (buffer->gem)
drm_gem_object_put(buffer->gem);
@@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev *client,
u32 width, u32 height, u
/**
* drm_client_buffer_vmap - Map DRM client buffer into address space
* @buffer: DRM client buffer
+ * @map_copy: Returns the mapped memory's address
*
* This function maps a client buffer into kernel address space. If the
- * buffer is already mapped, it returns the mapping's address.
+ * buffer is already mapped, it returns the existing mapping's address.
*
* Client buffer mappings are not ref'counted. Each call to
* drm_client_buffer_vmap() should be followed by a call to
* drm_client_buffer_vunmap(); or the client buffer should be mapped
* throughout its lifetime.
*
+ * The returned address is a copy of the internal value. In contrast to
+ * other vmap interfaces, you don't need it for the client's vunmap
+ * function. So you can modify it at will during blit and draw operations.
+ *
* Returns:
- * The mapped memory's address
+ * 0 on success, or a negative errno code otherwise.
*/
-void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
+int
+drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map
*map_copy)
{
- struct dma_buf_map map;
+ struct dma_buf_map *map = >map;
int ret;
- if (buffer->vaddr)
- return buffer->vaddr;
+ if (dma_buf_map_is_set(map))
+ goto out;
/*
* FIXME: The dependency on GEM here isn't required, we could
@@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
*buffer)
* fd_install step out of the driver backend hooks, to make that
* final step optional for internal users.
*/
- ret = drm_gem_vmap(buffer->gem, );
+ ret = drm_gem_vmap(buffer->gem, map);
if (ret)
- return ERR_PTR(ret);
+ return ret;
- buffer->vaddr = map.vaddr;
+out:
+ *map_copy = *map;
- return map.vaddr;
+ return 0;
}
EXPORT_SYMBOL(drm_client_buffer_vmap);
@@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
*/
void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
{
- struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
+ struct dma_buf_map *map = >map;
- drm_gem_vunmap(buffer->gem, );
- buffer->vaddr = NULL;
+ drm_gem_vunmap(buffer->gem, map);
}
EXPORT_SYMBOL(drm_client_buffer_vunmap);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8697554ccd41..343a292f2c7c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -394,7 +394,7 @@ static void drm_fb_helper_dirty_blit_real(struct
drm_fb_helper *fb_helper,
unsigned int cpp = fb->format->cpp[0];
size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
void *src = fb_helper->fbdev->screen_buffer + offset;
- void *dst = fb_helper->buffer->vaddr + offset;
+ void *dst = fb_helper->buffer->map.vaddr + offset;
size_t len = (clip->x2 - clip->x1) * cpp;
unsigned int y;
@@ -416,7 +416,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
*work)
struct drm_clip_rect *clip = >dirty_clip;
struct drm_clip_rect clip_copy;
unsigned long flags;
- void *vaddr;
+ struct dma_buf_map map;
+ int ret;
spin_lock_irqsave(>dirty_lock, flags);
clip_copy = *clip;
@@ -429,8 +430,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
*work)
/* Generic fbdev uses a shadow buffer */
if (helper->buffer) {
- vaddr = drm_client_buffer_vmap(helper->buffer);
- if (IS_ERR(vaddr))
+ ret = drm_client_buffer_vmap(helper->buffer, );
+ if (ret)
Re: [External] Re: [PATCH 2/2] sched: mark PRINTK_DEFERRED_CONTEXT_MASK in __schedule()
On Tue, Sep 29, 2020 at 04:27:51PM +0200, Petr Mladek wrote: > Upstreaming the console handling will be the next big step. I am sure > that there will be long discussion about it. But there might be > few things that would help removing printk_deferred(). > > 1. Messages will be printed on consoles by dedicated kthreads. It will >be safe context. No deadlocks. This, that's what I remember. With sane consoles having a ->write_atomic() callback which is called in-line. Current -RT has a single kthread that's flushing the consoles. > 2. The registration and unregistration of consoles should not longer >be handled by console_lock (semaphore). It should be possible to >call most consoles without a sleeping lock. It should remove all >these deadlocks between printk() and scheduler(). I'm confused, who cares about registation? That only happens once at boot, right? >There might be problems with some consoles. For example, tty would >most likely still need a sleeping lock because it is using the console >semaphore also internally. But per 1) above, that's done from a kthread, so nobody cares. > 3. We will try harder to get the messages out immediately during >panic(). As long as you guarantee to first write everything to consoles with ->write_atomic() before attempting to flush others that should be fine. > It would take some time until the above reaches upstream. But it seems > to be the right way to go. Yep. > Finally, the deadlock happens "only" when someone is waiting on > console_lock() in parallel. Otherwise, the waitqueue for the semaphore > is empty and scheduler is not called. There's more deadlocks. In fact the one described earlier in this thread isn't the one you mention. > It means that there is quite a big change to see the WARN(). It might > be even bigger than with printk_deferred() because WARN() in scheduler > means that the scheduler is big troubles. Nobody guarantees that > the deferred messages will get handled later. I only care about ->write_atomic() :-) Anything else is a best-effort-loss anyway. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: update todo.rst
- debugfs cleanup has moved forward thanks to the cleanup Wambui has done Cc: Wambui Karuga Cc: Greg Kroah-Hartman Cc: Melissa Wen Signed-off-by: Daniel Vetter --- Documentation/gpu/todo.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 3751ac976c3e..700637e25ecd 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -515,9 +515,6 @@ There's a bunch of issues with it: this (together with the drm_minor->drm_device move) would allow us to remove debugfs_init. -- Drop the return code and error checking from all debugfs functions. Greg KH is - working on this already. - Contact: Daniel Vetter Level: Intermediate -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: Fix 'reg' size issues in zynqmp examples
On Tue, Sep 29, 2020 at 1:55 AM Michal Simek wrote:
>
> Hi Rob,
>
> On 28. 09. 20 17:59, Rob Herring wrote:
> > The default sizes in examples for 'reg' are 1 cell each. Fix the
> > incorrect sizes in zynqmp examples:
> >
> > Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.example.dt.yaml:
> > example-0: dma-controller@fd4c:reg:0: [0, 4249616384, 0, 4096] is too
> > long
> > From schema:
> > /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> > Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml:
> > example-0: display@fd4a:reg:0: [0, 4249485312, 0, 4096] is too long
> > From schema:
> > /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> > Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml:
> > example-0: display@fd4a:reg:1: [0, 4249526272, 0, 4096] is too long
> > From schema:
> > /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> > Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml:
> > example-0: display@fd4a:reg:2: [0, 4249530368, 0, 4096] is too long
> > From schema:
> > /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> > Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml:
> > example-0: display@fd4a:reg:3: [0, 4249534464, 0, 4096] is too long
> > From schema:
> > /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> >
> > Cc: Hyun Kwon
> > Cc: Laurent Pinchart
> > Cc: Michal Simek
> > Cc: Vinod Koul
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: dmaeng...@vger.kernel.org
> > Signed-off-by: Rob Herring
> > ---
> > .../bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml | 8
> > .../devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml | 2 +-
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml
> > b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml
> > index 52a939cade3b..7b9d468c3e52 100644
> > --- a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml
> > +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml
> > @@ -145,10 +145,10 @@ examples:
> >
> > display@fd4a {
> > compatible = "xlnx,zynqmp-dpsub-1.7";
> > -reg = <0x0 0xfd4a 0x0 0x1000>,
> > - <0x0 0xfd4aa000 0x0 0x1000>,
> > - <0x0 0xfd4ab000 0x0 0x1000>,
> > - <0x0 0xfd4ac000 0x0 0x1000>;
> > +reg = <0xfd4a 0x1000>,
> > + <0xfd4aa000 0x1000>,
> > + <0xfd4ab000 0x1000>,
> > + <0xfd4ac000 0x1000>;
> > reg-names = "dp", "blend", "av_buf", "aud";
> > interrupts = <0 119 4>;
> > interrupt-parent = <>;
> > diff --git
> > a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> > b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> > index 5de510f8c88c..2a595b18ff6c 100644
> > --- a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> > +++ b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> > @@ -57,7 +57,7 @@ examples:
> >
> > dma: dma-controller@fd4c {
> >compatible = "xlnx,zynqmp-dpdma";
> > - reg = <0x0 0xfd4c 0x0 0x1000>;
> > + reg = <0xfd4c 0x1000>;
> >interrupts = ;
> >interrupt-parent = <>;
> >clocks = <_clk>;
> >
>
> I would prefer to keep 64bit version.
> I use this style.
I prefer to keep the examples simple. The address size is outside the
scope of the binding.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/nouveau: Drop mutex_lock_nested for atomic
On Thu, Sep 17, 2020 at 3:15 PM Daniel Vetter wrote:
>
> Ben, did you have a chance to look at this?
Ping
-Daniel
> On Mon, Aug 3, 2020 at 1:22 PM Maarten Lankhorst
> wrote:
> >
> > Op 02-08-2020 om 20:18 schreef Daniel Vetter:
> > > Purely conjecture, but I think the original locking inversion with the
> > > legacy page flip code between flipping and ttm's bo move function
> > > shoudn't exist anymore with atomic: With atomic the bo pinning and
> > > actual modeset commit is completely separated in the code patsh.
> > >
> > > This annotation was originally added in
> > >
> > > commit 060810d7abaabcab282e062c595871d661561400
> > > Author: Ben Skeggs
> > > Date: Mon Jul 8 14:15:51 2013 +1000
> > >
> > > drm/nouveau: fix locking issues in page flipping paths
> > >
> > > due to
> > >
> > > commit b580c9e2b7ba5030a795aa2fb73b796523d65a78
> > > Author: Maarten Lankhorst
> > > Date: Thu Jun 27 13:48:18 2013 +0200
> > >
> > > drm/nouveau: make flipping lockdep safe
> > >
> > > Signed-off-by: Daniel Vetter
> > > Cc: Maarten Lankhorst
> > > Cc: Ben Skeggs
> > > Cc: Dave Airlie
> > > Cc: nouv...@lists.freedesktop.org
> > > ---
> > > I might be totally wrong, so this definitely needs testing :-)
> > >
> > > Cheers, Daniel
> > > ---
> > > drivers/gpu/drm/nouveau/nouveau_bo.c | 6 +-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > index 7806278dce57..a7b2a9bb0ffe 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > @@ -776,7 +776,11 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo,
> > > int evict, bool intr,
> > > return ret;
> > > }
> > >
> > > - mutex_lock_nested(>mutex, SINGLE_DEPTH_NESTING);
> > > + if (drm_drv_uses_atomic_modeset(drm->dev))
> > > + mutex_lock(>mutex);
> > > + else
> > > + mutex_lock_nested(>mutex, SINGLE_DEPTH_NESTING);
> > > +
> > > ret = nouveau_fence_sync(nouveau_bo(bo), chan, true, intr);
> > > if (ret == 0) {
> > > ret = drm->ttm.move(chan, bo, >mem, new_reg);
> >
> > Well if you're certain it works now. :)
> >
> > Reviewed-by: Maarten Lankhorst
> >
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
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 0/3] Prevent out-of-bounds access for built-in font data buffers
On Tue, Sep 29, 2020 at 2:34 PM Peilin Ye wrote:
>
> On Fri, Sep 25, 2020 at 03:25:51PM +0200, Daniel Vetter wrote:
> > I think the only way to make this work is that we have one place which
> > takes in the userspace uapi struct, and then converts it once into a
> > kernel_console_font. With all the error checking.
>
> Hi Daniel,
>
> It seems that users don't use `console_font` directly, they use
> `console_font_op`. Then, in TTY:
Wow, this is a maze :-/
> (drivers/tty/vt/vt.c)
> int con_font_op(struct vc_data *vc, struct console_font_op *op)
> {
> switch (op->op) {
> case KD_FONT_OP_SET:
> return con_font_set(vc, op);
> case KD_FONT_OP_GET:
> return con_font_get(vc, op);
> case KD_FONT_OP_SET_DEFAULT:
> return con_font_default(vc, op);
> case KD_FONT_OP_COPY:
> return con_font_copy(vc, op);
> }
> return -ENOSYS;
> }
So my gut feeling is that this is just a bit of overenthusiastic
common code sharing, and all it results is confuse everyone. I think
if we change the conf_font_get/set/default/copy functions to not take
the *op struct (which is take pretty arbitrarily from one of the
ioctl), but the parameters each needs directly, that would clean up
the code a _lot_. Since most callers would then directly call the
right operation, instead of this detour through console_font_op.
struct console_font_op is an uapi struct, so really shouldn't be used
for internal abstractions - we can't change uapi, hence this makes it
impossible to refactor anything from the get-go.
I also think that trying to get rid of con_font_op callers as much as
possible (everywhere where the op struct is constructed in the kernel
and doesn't come from userspace essentially) should be doable as a
stand-alone patch series.
> These 4 functions allocate `console_font`. We can replace them with our
> `kernel_console_font`. So, ...
>
> $ vgrep "\.con_font_set"
An aside: git grep is awesome, and really fast.
> Index FileLine Content
> 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1294 .con_font_set =
> sisusbcon_font_set,
> 1 drivers/usb/misc/sisusbvga/sisusb_con.c 1378 .con_font_set =
> sisusbdummycon_font_set,
> 2 drivers/video/console/dummycon.c 162 .con_font_set =
> dummycon_font_set,
> 3 drivers/video/console/newport_con.c 693 .con_font_set =
> newport_font_set,
> 4 drivers/video/console/vgacon.c 1226 .con_font_set =
> vgacon_font_set,
> 5 drivers/video/fbdev/core/fbcon.c3120 .con_font_set
> = fbcon_set_font,
> $
> $ vgrep "\.con_font_get"
> Index FileLine Content
> 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1295 .con_font_get =
> sisusbcon_font_get,
> 1 drivers/video/console/vgacon.c 1227 .con_font_get =
> vgacon_font_get,
> 2 drivers/video/fbdev/core/fbcon.c3121 .con_font_get
> = fbcon_get_font,
> $
> $ vgrep "\.con_font_default"
> Index FileLine Content
> 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1379 .con_font_default =
> sisusbdummycon_font_default,
> 1 drivers/video/console/dummycon.c 163 .con_font_default =
> dummycon_font_default,
The above two return 0 but do nothing, which means width/height are
now bogus (or well the same as what userspace set). I don't think that
works correctly ...
> 2 drivers/video/console/newport_con.c 694 .con_font_default =
> newport_font_default,
This just seems to release the userspace font. This is already done in
other places where it makes a lot more sense to clean up.
> 3 drivers/video/fbdev/core/fbcon.c3122 .con_font_default=
> fbcon_set_def_font,
This actually does something. tbh I would not be surprises if the
fb_set utility is the only thing that uses this - with a bit of code
search we could perhaps confirm this, and delete all the other
implementations.
> $
> $ vgrep "\.con_font_copy"
> Index FileLine Content
> 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1380 .con_font_copy =
> sisusbdummycon_font_copy,
> 1 drivers/video/console/dummycon.c 164 .con_font_copy =
> dummycon_font_copy,
Above two do nothing, but return 0. Again this wont work I think.
> 2 drivers/video/fbdev/core/fbcon.c3123 .con_font_copy
> = fbcon_copy_font,
Smells again like something that's only used by fb_set, and we could
probably delete the other dummy implementations. Also I'm not even
really clear on what this does ...
Removing these dummy functions means that for a dummy console these
ioctls would start failing, but then I don't think anyone boots up
into a dummy console and expects font changes to work. So again I
think we could split this cleanup as prep work.
> $ _
>
> ... are
[Bug 209417] kernel-5.8*, amdgpu - Blank screen shortly after boot, "fixed" with suspend and wake up
https://bugzilla.kernel.org/show_bug.cgi?id=209417 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #2 from Alex Deucher (alexdeuc...@gmail.com) --- Can you bisect? -- 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 v2 2/2] drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data
On 20/09/2020 19:59, Jyri Sarha wrote: > We already have a private data member for maximum display width so > let's use it and get rid of the redundant tilcdc_crtc_max_width(). > > The LCDC version probing is moved to before reading the device tree > properties so that the version information is available when private > data maximum width is initialized, if "max-width" property is not > found. > > Signed-off-by: Jyri Sarha > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 +--- > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 38 +++- > drivers/gpu/drm/tilcdc/tilcdc_drv.h | 7 ++--- > 3 files changed, 26 insertions(+), 35 deletions(-) Reviewed-by: Tomi Valkeinen 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 v2] drm: bridge: cdns-mhdp8546: fix compile warning
On x64 we get:
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:751:10: warning: conversion
from 'long unsigned int' to 'unsigned int' changes value from
'18446744073709551613' to '4294967293' [-Woverflow]
The registers are 32 bit, so fix by casting to u32.
Fixes: fb43aa0acdfd ("drm: bridge: Add support for Cadence MHDP8546 DPI/DP
bridge")
Signed-off-by: Tomi Valkeinen
Reported-by: Stephen Rothwell
Reviewed-by: Swapnil Jakhade
Acked-by: Laurent Pinchart
---
v2:
No changes to code, added tags.
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 621ebdbff8a3..d0c65610ebb5 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -748,7 +748,7 @@ static int cdns_mhdp_fw_activate(const struct firmware *fw,
* bridge should already be detached.
*/
if (mhdp->bridge_attached)
- writel(~CDNS_APB_INT_MASK_SW_EVENT_INT,
+ writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT,
mhdp->regs + CDNS_APB_INT_MASK);
spin_unlock(>start_lock);
@@ -1689,7 +1689,7 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
/* Enable SW event interrupts */
if (hw_ready)
- writel(~CDNS_APB_INT_MASK_SW_EVENT_INT,
+ writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT,
mhdp->regs + CDNS_APB_INT_MASK);
return 0;
@@ -2122,7 +2122,7 @@ static void cdns_mhdp_bridge_hpd_enable(struct drm_bridge
*bridge)
/* Enable SW event interrupts */
if (mhdp->bridge_attached)
- writel(~CDNS_APB_INT_MASK_SW_EVENT_INT,
+ writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT,
mhdp->regs + CDNS_APB_INT_MASK);
}
--
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
RE: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
-Original Message-
From: Ville Syrjälä
Sent: 29 September 2020 18:23
To: Surendrakumar Upadhyay, TejaskumarX
Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Ausmus,
James ; Roper, Matthew D ;
Souza, Jose ; De Marchi, Lucas
; Pandey, Hariom
Subject: Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 05:41:27PM +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
What do vswing values have to do with the PCH type?
Tejas : There is difference in voltage swing values for EHL and JSL. Till now
we were not differentiating between EHL and JSL as both were
based on ICL. Now as per compliance test we have found change in JSL eDP vswing
values which does not apply to EHL which leads to differentiate
between EHL and JSL. Thus differentiator between JSL and EHL is PCH i.e
HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL). There is no direct relation of PCH with
vswing.
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay
>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 67
> ++--
> 1 file changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4d06178cd76c..e6e93d01d0ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans
> ehl_combo_phy_ddi_translations_dp[] = {
> { 0x6, 0x7F, 0x3F, 0x00, 0x00 },/* 900 900 0.0 */
> };
>
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x38, 0x00, 0x07 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x33, 0x00, 0x0C },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x36, 0x00, 0x09 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x38, 0x00, 0x07 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x35, 0x00, 0x0A },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
> +
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr2[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x3D, 0x00, 0x02 },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x3A, 0x00, 0x05 },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
> +
> struct icl_mg_phy_ddi_buf_trans {
> u32 cri_txdeemph_override_11_6;
> u32 cri_txdeemph_override_5_0;
> @@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder, int
> type, int rate,
> *n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
> return icl_mg_phy_ddi_translations_rbr_hbr;
> }
> -
> static const struct cnl_ddi_buf_trans *
> ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> int *n_entries)
> @@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder,
> int type, int rate,
> }
> }
>
> +static const struct cnl_ddi_buf_trans *
> +jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> + int *n_entries)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> + switch (type) {
> + case INTEL_OUTPUT_HDMI:
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> + return icl_combo_phy_ddi_translations_hdmi;
> + case INTEL_OUTPUT_EDP:
> + if (dev_priv->vbt.edp.low_vswing) {
> + if (rate > 27) {
> + *n_entries =
> ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr2);
> + return
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 05:41:27PM +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
What do vswing values have to do with the PCH type?
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++--
> 1 file changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4d06178cd76c..e6e93d01d0ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans
> ehl_combo_phy_ddi_translations_dp[] = {
> { 0x6, 0x7F, 0x3F, 0x00, 0x00 },/* 900 900 0.0 */
> };
>
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x38, 0x00, 0x07 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x33, 0x00, 0x0C },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x36, 0x00, 0x09 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x38, 0x00, 0x07 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x35, 0x00, 0x0A },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
> +
> +static const struct cnl_ddi_buf_trans
> jsl_combo_phy_ddi_translations_edp_hbr2[] = {
> + /* NT mV Trans mV db*/
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 250 1.9 */
> + { 0x1, 0x7F, 0x3D, 0x00, 0x02 },/* 200 300 3.5 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 250 300 1.6 */
> + { 0xA, 0x35, 0x3A, 0x00, 0x05 },/* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> +};
> +
> struct icl_mg_phy_ddi_buf_trans {
> u32 cri_txdeemph_override_11_6;
> u32 cri_txdeemph_override_5_0;
> @@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder, int
> type, int rate,
> *n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
> return icl_mg_phy_ddi_translations_rbr_hbr;
> }
> -
> static const struct cnl_ddi_buf_trans *
> ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> int *n_entries)
> @@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder,
> int type, int rate,
> }
> }
>
> +static const struct cnl_ddi_buf_trans *
> +jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> + int *n_entries)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> + switch (type) {
> + case INTEL_OUTPUT_HDMI:
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> + return icl_combo_phy_ddi_translations_hdmi;
> + case INTEL_OUTPUT_EDP:
> + if (dev_priv->vbt.edp.low_vswing) {
> + if (rate > 27) {
> + *n_entries =
> ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr2);
> + return jsl_combo_phy_ddi_translations_edp_hbr2;
> + } else {
> + *n_entries =
> ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr);
> + return jsl_combo_phy_ddi_translations_edp_hbr;
> + }
> + }
> + /* fall through */
> + default:
> + /* All combo DP and eDP ports that do not support low_vswing */
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hbr2);
> + return icl_combo_phy_ddi_translations_dp_hbr2;
> + }
> +}
> +
> static const struct cnl_ddi_buf_trans *
> tgl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> int *n_entries)
> @@ -2265,7 +2320,10 @@ static u8 intel_ddi_dp_voltage_max(struct intel_dp
>
Re: [Intel-gfx] [PATCH 2/2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Mon, Sep 28, 2020 at 08:20:59PM +0300, Jani Nikula wrote: > On Mon, 28 Sep 2020, Ville Syrjälä wrote: > > On Mon, Sep 28, 2020 at 07:15:43AM -0700, James Ausmus wrote: > >> On Mon, Sep 28, 2020 at 04:43:11PM +0300, Jani Nikula wrote: > >> > On Mon, 28 Sep 2020, Tejas Upadhyay > >> > wrote: > >> > > JSL has update in vswing table for eDP > >> > > >> > I've thought the TLA for Jasper Lake is JSP, not JSL. At least we have > >> > PCH_JSP for Jasper Lake PCH. > >> > >> JSP == Point (the PCH), JSL == Lake > > > > .PT was " Point", ..P stands just for " PCH" IIRC. > > Yeah, nowadays it doesn't have "Point", however bspec agrees on the JSL > acronym for Jasper Lake. Bspec uses ..P for " PCH", when it acknowledges the existence of said PCH (see eg. CNP,ICP,TGP). JSP is not among that select crowd however, neither really is MCC (although it is mentioned by name in the JSL section). I kinda want to nuke the JSP and MCC types entirely. I believe we should be able to treat them as just ICP and TGP variants respectively. But theres's still a bit of work left to do before we can get there. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: remove alloc_vm_area v2
Quoting Christoph Hellwig (2020-09-28 15:37:41) > On Mon, Sep 28, 2020 at 01:13:38PM +0300, Joonas Lahtinen wrote: > > I think we have a gap that after splitting the drm-intel-next pull requests > > into > > two the drm-intel/for-linux-next branch is now missing material from > > drm-intel/drm-intel-gt-next. > > > > I think a simple course of action might be to start including > > drm-intel-gt-next > > in linux-next, which would mean that we should update DIM tooling to add > > extra branch "drm-intel/gt-for-linux-next" or so. > > > > Which specific patches are missing in this case? > > The two dependencies required by my series not in mainline are: > > drm/i915/gem: Avoid implicit vmap for highmem on x86-32 > drm/i915/gem: Prevent using pgprot_writecombine() if PAT is not supported > > so it has to be one or both of those. Hmm, those are both committed after our last -next pull request, so they would normally only target next merge window. drm-next closes the merge window around -rc5 already. But, in this specific case those are both Fixes: patches with Cc: stable, so they should be pulled into drm-intel-next-fixes PR. Rodrigo, can you cherry-pick those patches to -next-fixes that you send to Dave? Regards, Joonas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
JSL has update in vswing table for eDP
BSpec: 21257
Changes since V1 :
- IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
- Reverted EHL/JSL PCI ids split change
Signed-off-by: Tejas Upadhyay
---
drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++--
1 file changed, 64 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 4d06178cd76c..e6e93d01d0ce 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans
ehl_combo_phy_ddi_translations_dp[] = {
{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },/* 900 900 0.0 */
};
+static const struct cnl_ddi_buf_trans jsl_combo_phy_ddi_translations_edp_hbr[]
= {
+ /* NT mV Trans mV db*/
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
+ { 0x8, 0x7F, 0x38, 0x00, 0x07 },/* 200 250 1.9 */
+ { 0x1, 0x7F, 0x33, 0x00, 0x0C },/* 200 300 3.5 */
+ { 0xA, 0x35, 0x36, 0x00, 0x09 },/* 200 350 4.9 */
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
+ { 0x1, 0x7F, 0x38, 0x00, 0x07 },/* 250 300 1.6 */
+ { 0xA, 0x35, 0x35, 0x00, 0x0A },/* 250 350 2.9 */
+ { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
+ { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
+ { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
+};
+
+static const struct cnl_ddi_buf_trans
jsl_combo_phy_ddi_translations_edp_hbr2[] = {
+ /* NT mV Trans mV db*/
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 200 0.0 */
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 200 250 1.9 */
+ { 0x1, 0x7F, 0x3D, 0x00, 0x02 },/* 200 300 3.5 */
+ { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 200 350 4.9 */
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 },/* 250 250 0.0 */
+ { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 250 300 1.6 */
+ { 0xA, 0x35, 0x3A, 0x00, 0x05 },/* 250 350 2.9 */
+ { 0x1, 0x7F, 0x3F, 0x00, 0x00 },/* 300 300 0.0 */
+ { 0xA, 0x35, 0x38, 0x00, 0x07 },/* 300 350 1.3 */
+ { 0xA, 0x35, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
+};
+
struct icl_mg_phy_ddi_buf_trans {
u32 cri_txdeemph_override_11_6;
u32 cri_txdeemph_override_5_0;
@@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder, int
type, int rate,
*n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
return icl_mg_phy_ddi_translations_rbr_hbr;
}
-
static const struct cnl_ddi_buf_trans *
ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
int *n_entries)
@@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder,
int type, int rate,
}
}
+static const struct cnl_ddi_buf_trans *
+jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
+ int *n_entries)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+ switch (type) {
+ case INTEL_OUTPUT_HDMI:
+ *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
+ return icl_combo_phy_ddi_translations_hdmi;
+ case INTEL_OUTPUT_EDP:
+ if (dev_priv->vbt.edp.low_vswing) {
+ if (rate > 27) {
+ *n_entries =
ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr2);
+ return jsl_combo_phy_ddi_translations_edp_hbr2;
+ } else {
+ *n_entries =
ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr);
+ return jsl_combo_phy_ddi_translations_edp_hbr;
+ }
+ }
+ /* fall through */
+ default:
+ /* All combo DP and eDP ports that do not support low_vswing */
+ *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hbr2);
+ return icl_combo_phy_ddi_translations_dp_hbr2;
+ }
+}
+
static const struct cnl_ddi_buf_trans *
tgl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
int *n_entries)
@@ -2265,7 +2320,10 @@ static u8 intel_ddi_dp_voltage_max(struct intel_dp
*intel_dp)
tgl_get_dkl_buf_trans(encoder, encoder->type,
intel_dp->link_rate, _entries);
} else if (INTEL_GEN(dev_priv) == 11) {
- if
Re: [PATCH] drm/qxl: fix usage of ttm_bo_init
On Tue, Sep 29, 2020 at 01:23:06PM +0200, Christian König wrote: > We need to use ttm_bo_init_reserved here to make sure > that the BO is pinned before it becomes visible on the LRU. > > Signed-off-by: Christian König Reviewed-by: Gerd Hoffmann # Tested-by: Gerd Hoffmann # ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5.4 119/388] drm/omap: fix possible object reference leak
From: Wen Yang [ Upstream commit 47340e46f34a3b1d80e40b43ae3d7a8da34a3541 ] The call to of_find_matching_node returns a node pointer with refcount incremented thus it must be explicitly decremented after the last usage. Detected by coccinelle with the following warnings: drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:212:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:237:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. Signed-off-by: Wen Yang Reviewed-by: Laurent Pinchart Reviewed-by: Mukesh Ojha Cc: Tomi Valkeinen Cc: David Airlie Cc: Daniel Vetter Cc: Sebastian Reichel Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: Markus Elfring Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/1554692313-28882-2-git-send-email-wen.yan...@zte.com.cn Signed-off-by: Sasha Levin --- drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c index 31502857f013d..ce67891eedd46 100644 --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c @@ -192,7 +192,7 @@ static int __init omapdss_boot_init(void) dss = of_find_matching_node(NULL, omapdss_of_match); if (dss == NULL || !of_device_is_available(dss)) - return 0; + goto put_node; omapdss_walk_device(dss, true); @@ -217,6 +217,8 @@ static int __init omapdss_boot_init(void) kfree(n); } +put_node: + of_node_put(dss); return 0; } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/qxl: fix usage of ttm_bo_init
On Tue, Sep 29, 2020 at 01:23:06PM +0200, Christian König wrote:
> We need to use ttm_bo_init_reserved here to make sure
> that the BO is pinned before it becomes visible on the LRU.
>
> Signed-off-by: Christian König
Reviewed-by: Daniel Vetter
But maybe let Gerd test this first before pushing :-)
-Daniel
> ---
> drivers/gpu/drm/qxl/qxl_object.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_object.c
> b/drivers/gpu/drm/qxl/qxl_object.c
> index d3635e3e3267..c8b67e7a3f02 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.c
> +++ b/drivers/gpu/drm/qxl/qxl_object.c
> @@ -106,6 +106,7 @@ int qxl_bo_create(struct qxl_device *qdev,
> struct qxl_surface *surf,
> struct qxl_bo **bo_ptr)
> {
> + struct ttm_operation_ctx ctx = { !kernel, false };
> struct qxl_bo *bo;
> enum ttm_bo_type type;
> int r;
> @@ -134,9 +135,9 @@ int qxl_bo_create(struct qxl_device *qdev,
>
> qxl_ttm_placement_from_domain(bo, domain);
>
> - r = ttm_bo_init(>mman.bdev, >tbo, size, type,
> - >placement, 0, !kernel, size,
> - NULL, NULL, _ttm_bo_destroy);
> + r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
> + >placement, 0, , size,
> + NULL, NULL, _ttm_bo_destroy);
> if (unlikely(r != 0)) {
> if (r != -ERESTARTSYS)
> dev_err(qdev->ddev.dev,
> @@ -146,6 +147,7 @@ int qxl_bo_create(struct qxl_device *qdev,
> }
> if (pinned)
> ttm_bo_pin(>tbo);
> + ttm_bo_unreserve(>tbo);
> *bo_ptr = bo;
> return 0;
> }
> --
> 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: [PATCH 1/9] drm/format-helper: Pass destination pitch to drm_fb_memcpy_dstclip()
On Tue, Sep 29, 2020 at 11:39:21AM +0200, Thomas Zimmermann wrote: > Hi > > Am 29.09.20 um 11:19 schrieb Daniel Vetter: > > On Mon, Sep 28, 2020 at 11:13:06AM +0200, Thomas Zimmermann wrote: > >> Hi > >> > >> Am 28.09.20 um 10:53 schrieb Daniel Vetter: > >>> On Mon, Sep 28, 2020 at 9:22 AM Thomas Zimmermann > >>> wrote: > > Hi > > Am 26.09.20 um 18:42 schrieb Daniel Vetter: > > On Fri, Sep 25, 2020 at 4:55 PM Thomas Zimmermann > > wrote: > >> > >> Hi > >> > >> Am 29.06.20 um 10:40 schrieb Daniel Vetter: > >>> On Thu, Jun 25, 2020 at 02:00:03PM +0200, Thomas Zimmermann wrote: > The memcpy's destination buffer might have a different pitch than the > source. Support different pitches as function argument. > > Signed-off-by: Thomas Zimmermann > >>> > >>> Reviewed-by: Daniel Vetter > >>> > >>> But I do have questions ... why did we allocate a source > >>> drm_framebuffer > >>> with mismatching pitch? That sounds backwards, especially for > >>> simplekms. > >> > >> There's userspace that allocates framebuffers in tiles of 64x64 pixels. > >> I think I've seen this with Gnome. So if you have a 800x600 display > >> mode, the allocated framebuffer has a scanline pitch of 832 pixels and > >> the final 32 pixels are ignored. > > > > At least with dumb buffer allocation ioctls userspace should not do > > that. If it wants 800x600, it needs to allocate 800x600, not something > > That ship has sailed. > >>> > >>> Not really, right now that ship is simply leaking and sinking. If we > >>> decide to patch this up from the kernel side, then indeed it has > >>> sailed. And I'm not sure that's a good idea. > >> > >> We have code in at least cirrus, ast and mgag200 to support this. And > >> userspace has been behaving like this since at least when I got involved > >> (2017). > > > > Hm where do these drivers copy stuff around and rematch the stride? > > They don't. These drivers adopt their HW stride to match whatever > userspace framebuffers tell them. [1] And that's because userspace gives > them framebuffer sizes like 832*640. And then they do a memcpy with the > given width. > > My sole point here was that userspace already relies on this behavior. Yeah but the problem is, if we support this through copying, then we rob userspace of the change to do something better. I guess the reason that userspace aligns to 64b is because of i915 gbm (iirc 64b is the requirement for being able to render, or maybe it's the requirement for scanout). So you get zero-copy buffer sharing. But if the kernel now does copies on its own, without telling userspace, then we might end up with a fairly crappy path. It might not matter much for old devices like this one, but for cases where we really care about zero-copy it does. And unlike the kernel userspace can perhaps do the copying using a different gpu for these cases. So that's kinda why I'm vary of rolling this out at large scale. Since once we've done it, we cannot ever undo it since that breaks existing userspace on existing hardware. Which means even if we fix userspace to have a better/different fallback that uses a correctly sized buffer and maybe even a gpu copy, that code wont run because the kernel papers over the problem with a dog-slow re-stride cpu copy. -Daniel > > Best regards > Thomas > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/tiny/cirrus.c#n285 > > > > > else. The driver is supposed to apply any rounding necessary for the > > size. Or is this a buffer allocated somewhere else and then shared? > > I don't quite remember where exactly this was implemented. It was not a > shared buffer, though. IIRC the buffer allocation code in one of the > libs rounded the size towards multiples of 64. I remember thinking that > it was probably done for tiled rendering. > >>> > >>> Yeah, but you don't do rendering on dumb buffers. Like ever. So this > >>> smells like a userspace bug. > >> > >> It's also part of the software rendering. It is not a bug, but > >> implemented deliberately in one of the userspace components that > >> allocates framebuffers (but I cannot remember which one.) > > > > I think it would be good to document this. > > > > We already fake xrgb everywhere because userspace is not flexible > > enough, I guess having to fake 64b stride support everywhere isn't that > > much worse. > > > > But it's definitely not great either :-/ > > -Daniel > > > >> > >> Best regards > >> Thomas > >> > >>> > >>> If it's for shared buffers then I think that sounds more reasonable. > >>> -Daniel > >>> > > Best regards > Thomas > > > -Daniel > > > >> In regular drivers, we can handle this with the VGA offset register [1] > >> or some equivalent. That's obviously not an option with simplekms, so
[PATCH 4.19 077/245] drm/omap: fix possible object reference leak
From: Wen Yang [ Upstream commit 47340e46f34a3b1d80e40b43ae3d7a8da34a3541 ] The call to of_find_matching_node returns a node pointer with refcount incremented thus it must be explicitly decremented after the last usage. Detected by coccinelle with the following warnings: drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:212:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:237:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. Signed-off-by: Wen Yang Reviewed-by: Laurent Pinchart Reviewed-by: Mukesh Ojha Cc: Tomi Valkeinen Cc: David Airlie Cc: Daniel Vetter Cc: Sebastian Reichel Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: Markus Elfring Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/1554692313-28882-2-git-send-email-wen.yan...@zte.com.cn Signed-off-by: Sasha Levin --- drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c index 3bfb95d230e0e..d8fb686c1fda9 100644 --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c @@ -193,7 +193,7 @@ static int __init omapdss_boot_init(void) dss = of_find_matching_node(NULL, omapdss_of_match); if (dss == NULL || !of_device_is_available(dss)) - return 0; + goto put_node; omapdss_walk_device(dss, true); @@ -218,6 +218,8 @@ static int __init omapdss_boot_init(void) kfree(n); } +put_node: + of_node_put(dss); return 0; } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/4] drm/qxl: use qxl pin function
Am 29.09.20 um 12:53 schrieb Daniel Vetter:
On Tue, Sep 29, 2020 at 11:51:15AM +0200, Gerd Hoffmann wrote:
Otherwise ttm throws a WARN because we try to pin without a reservation.
Fixes: 9d36d4320462 ("drm/qxl: switch over to the new pin interface")
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index d3635e3e3267..eb45267d51db 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -145,7 +145,7 @@ int qxl_bo_create(struct qxl_device *qdev,
return r;
}
if (pinned)
- ttm_bo_pin(>tbo);
+ qxl_bo_pin(bo);
I think this is now after ttm_bo_init, and at that point the object is
visible to lru users and everything. So I do think you need to grab locks
here instead of just incrementing the pin count alone.
It's also I think a bit racy, since ttm_bo_init drops the lock, so someone
might have snuck in and evicted the object already.
I think what you need is to call ttm_bo_init_reserved, then ttm_bo_pin,
then ttm_bo_unreserve, all explicitly.
Ah, yes Daniel is right. I thought I've fixed that up, but looks like I
only did that for VMWGFX.
Sorry for the noise, fix to correctly address this is underway.
Regards,
Christian.
-Daniel
*bo_ptr = bo;
return 0;
}
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/qxl: fix usage of ttm_bo_init
We need to use ttm_bo_init_reserved here to make sure
that the BO is pinned before it becomes visible on the LRU.
Signed-off-by: Christian König
---
drivers/gpu/drm/qxl/qxl_object.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index d3635e3e3267..c8b67e7a3f02 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -106,6 +106,7 @@ int qxl_bo_create(struct qxl_device *qdev,
struct qxl_surface *surf,
struct qxl_bo **bo_ptr)
{
+ struct ttm_operation_ctx ctx = { !kernel, false };
struct qxl_bo *bo;
enum ttm_bo_type type;
int r;
@@ -134,9 +135,9 @@ int qxl_bo_create(struct qxl_device *qdev,
qxl_ttm_placement_from_domain(bo, domain);
- r = ttm_bo_init(>mman.bdev, >tbo, size, type,
- >placement, 0, !kernel, size,
- NULL, NULL, _ttm_bo_destroy);
+ r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
+>placement, 0, , size,
+NULL, NULL, _ttm_bo_destroy);
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
dev_err(qdev->ddev.dev,
@@ -146,6 +147,7 @@ int qxl_bo_create(struct qxl_device *qdev,
}
if (pinned)
ttm_bo_pin(>tbo);
+ ttm_bo_unreserve(>tbo);
*bo_ptr = bo;
return 0;
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4.9 049/121] drm/omap: fix possible object reference leak
From: Wen Yang [ Upstream commit 47340e46f34a3b1d80e40b43ae3d7a8da34a3541 ] The call to of_find_matching_node returns a node pointer with refcount incremented thus it must be explicitly decremented after the last usage. Detected by coccinelle with the following warnings: drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:212:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:237:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. Signed-off-by: Wen Yang Reviewed-by: Laurent Pinchart Reviewed-by: Mukesh Ojha Cc: Tomi Valkeinen Cc: David Airlie Cc: Daniel Vetter Cc: Sebastian Reichel Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: Markus Elfring Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/1554692313-28882-2-git-send-email-wen.yan...@zte.com.cn Signed-off-by: Sasha Levin --- drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c index 136d30484d023..46111e9ee9a25 100644 --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c @@ -194,7 +194,7 @@ static int __init omapdss_boot_init(void) dss = of_find_matching_node(NULL, omapdss_of_match); if (dss == NULL || !of_device_is_available(dss)) - return 0; + goto put_node; omapdss_walk_device(dss, true); @@ -219,6 +219,8 @@ static int __init omapdss_boot_init(void) kfree(n); } +put_node: + of_node_put(dss); return 0; } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
Also revert the follow-up change "drm: pl111: Absorb the external register header". This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. The fbdev driver is used by Android's FVP configuration. Using the DRM driver together with DRM's fbdev emulation results in a failure to boot Android. The root cause is that Android's generic fbdev userspace driver relies on the ability to set the pixel format via FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. There have been other less critical behavioral differences identified between the fbdev driver and the DRM driver with fbdev emulation. The DRM driver exposes different values for the panel's width, height and refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall with yres_virtual greater than the maximum supported value instead of letting the syscall succeed and setting yres_virtual based on yres. Signed-off-by: Peter Collingbourne --- View this change in Gerrit: https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 MAINTAINERS | 5 + drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + drivers/gpu/drm/pl111/pl111_display.c | 1 + drivers/gpu/drm/pl111/pl111_drm.h | 73 -- drivers/gpu/drm/pl111/pl111_drv.c | 1 + drivers/gpu/drm/pl111/pl111_versatile.c | 1 + drivers/video/fbdev/Kconfig | 20 + drivers/video/fbdev/Makefile| 1 + drivers/video/fbdev/amba-clcd.c | 986 include/linux/amba/clcd-regs.h | 87 +++ include/linux/amba/clcd.h | 290 +++ 11 files changed, 1393 insertions(+), 73 deletions(-) create mode 100644 drivers/video/fbdev/amba-clcd.c create mode 100644 include/linux/amba/clcd-regs.h create mode 100644 include/linux/amba/clcd.h diff --git a/MAINTAINERS b/MAINTAINERS index 190c7fa2ea01..671c1fa79e64 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1460,6 +1460,11 @@ S: Odd Fixes F: drivers/amba/ F: include/linux/amba/bus.h +ARM PRIMECELL CLCD PL110 DRIVER +M: Russell King +S: Odd Fixes +F: drivers/video/fbdev/amba-clcd.* + ARM PRIMECELL KMI PL050 DRIVER M: Russell King S: Odd Fixes diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c b/drivers/gpu/drm/pl111/pl111_debugfs.c index 317f68abf18b..26ca8cdf3e60 100644 --- a/drivers/gpu/drm/pl111/pl111_debugfs.c +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c @@ -3,6 +3,7 @@ * Copyright © 2017 Broadcom */ +#include #include #include diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c index b3e8697cafcf..703ddc803c55 100644 --- a/drivers/gpu/drm/pl111/pl111_display.c +++ b/drivers/gpu/drm/pl111/pl111_display.c @@ -9,6 +9,7 @@ * Copyright (C) 2011 Texas Instruments */ +#include #include #include #include diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h index 2a46b5bd8576..ba399bcb792f 100644 --- a/drivers/gpu/drm/pl111/pl111_drm.h +++ b/drivers/gpu/drm/pl111/pl111_drm.h @@ -23,79 +23,6 @@ #include #include -/* - * CLCD Controller Internal Register addresses - */ -#define CLCD_TIM0 0x -#define CLCD_TIM1 0x0004 -#define CLCD_TIM2 0x0008 -#define CLCD_TIM3 0x000c -#define CLCD_UBAS 0x0010 -#define CLCD_LBAS 0x0014 - -#define CLCD_PL110_IENB0x0018 -#define CLCD_PL110_CNTL0x001c -#define CLCD_PL110_STAT0x0020 -#define CLCD_PL110_INTR0x0024 -#define CLCD_PL110_UCUR0x0028 -#define CLCD_PL110_LCUR0x002C - -#define CLCD_PL111_CNTL0x0018 -#define CLCD_PL111_IENB0x001c -#define CLCD_PL111_RIS 0x0020 -#define CLCD_PL111_MIS 0x0024 -#define CLCD_PL111_ICR 0x0028 -#define CLCD_PL111_UCUR0x002c -#define CLCD_PL111_LCUR0x0030 - -#define CLCD_PALL 0x0200 -#define CLCD_PALETTE 0x0200 - -#define TIM2_PCD_LO_MASK GENMASK(4, 0) -#define TIM2_PCD_LO_BITS 5 -#define TIM2_CLKSEL(1 << 5) -#define TIM2_ACB_MASK GENMASK(10, 6) -#define TIM2_IVS (1 << 11) -#define TIM2_IHS (1 << 12) -#define TIM2_IPC (1 << 13) -#define TIM2_IOE (1 << 14) -#define TIM2_BCD (1 << 26) -#define TIM2_PCD_HI_MASK GENMASK(31, 27) -#define TIM2_PCD_HI_BITS 5 -#define TIM2_PCD_HI_SHIFT 27 - -#define CNTL_LCDEN (1 << 0) -#define CNTL_LCDBPP1 (0 << 1) -#define CNTL_LCDBPP2 (1 << 1) -#define CNTL_LCDBPP4 (2 << 1) -#define CNTL_LCDBPP8 (3 << 1) -#define CNTL_LCDBPP16 (4 << 1) -#define CNTL_LCDBPP16_565 (6 << 1) -#define
[PATCH 4.14 064/166] drm/omap: fix possible object reference leak
From: Wen Yang [ Upstream commit 47340e46f34a3b1d80e40b43ae3d7a8da34a3541 ] The call to of_find_matching_node returns a node pointer with refcount incremented thus it must be explicitly decremented after the last usage. Detected by coccinelle with the following warnings: drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:212:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:237:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. Signed-off-by: Wen Yang Reviewed-by: Laurent Pinchart Reviewed-by: Mukesh Ojha Cc: Tomi Valkeinen Cc: David Airlie Cc: Daniel Vetter Cc: Sebastian Reichel Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: Markus Elfring Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/1554692313-28882-2-git-send-email-wen.yan...@zte.com.cn Signed-off-by: Sasha Levin --- drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c index bf626acae2712..cd8e9b799b9a5 100644 --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c @@ -193,7 +193,7 @@ static int __init omapdss_boot_init(void) dss = of_find_matching_node(NULL, omapdss_of_match); if (dss == NULL || !of_device_is_available(dss)) - return 0; + goto put_node; omapdss_walk_device(dss, true); @@ -218,6 +218,8 @@ static int __init omapdss_boot_init(void) kfree(n); } +put_node: + of_node_put(dss); return 0; } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/4] dma-buf: Flag vmap'ed memory as system or I/O memory
Am 29.09.20 um 13:09 schrieb Christian König:
> Am 29.09.20 um 11:14 schrieb Daniel Vetter:
>> On Mon, Sep 28, 2020 at 01:22:13PM +0200, Christian König wrote:
>>> Am 28.09.20 um 09:37 schrieb Thomas Zimmermann:
Hi
Am 28.09.20 um 08:50 schrieb Christian König:
> Am 27.09.20 um 21:16 schrieb Sam Ravnborg:
>> Hi Thomas.
>>
struct simap {
union {
void __iomem *vaddr_iomem;
void *vaddr;
};
bool is_iomem;
};
Where simap is a shorthand for system_iomem_map
And it could al be stuffed into a include/linux/simap.h file.
Not totally sold on the simap name - but wanted to come up with
something.
>>> Yes. Others, myself included, have suggested to use a name that
>>> does not
>>> imply a connection to the dma-buf framework, but no one has come
>>> up with
>>> a good name.
>>>
>>> I strongly dislike simap, as it's entirely non-obvious what it does.
>>> dma-buf-map is not actually wrong. The structures represents the
>>> mapping
>>> of a dma-able buffer in most cases.
>>>
With this approach users do not have to pull in dma-buf to use
it and
users will not confuse that this is only for dma-buf usage.
>>> There's no need to enable dma-buf. It's all in the header file
>>> without
>>> dependencies on dma-buf. It's really just the name.
>>>
>>> But there's something else to take into account. The whole issue
>>> here is
>>> that the buffer is disconnected from its originating driver, so
>>> we don't
>>> know which kind of memory ops we have to use. Thinking about it, I
>>> realized that no one else seemed to have this problem until now.
>>> Otherwise there would be a solution already. So maybe the dma-buf
>>> framework *is* the native use case for this data structure.
>> We have at least:
>> linux/fb.h:
>> union {
>> char __iomem *screen_base; /* Virtual address */
>> char *screen_buffer;
>> };
>>
>> Which solve more or less the same problem.
I thought this was for convenience. The important is_iomem bit is
missing.
> I also already noted that in TTM we have exactly the same problem
> and a
> whole bunch of helpers to allow operations on those pointers.
How do you call this within TTM?
>>> ttm_bus_placement, but I really don't like that name.
>>>
The data structure represents a pointer to either system or I/O memory,
but not necessatrily device memory. It contains raw data. That would
give something like
struct databuf_map
struct databuf_ptr
struct dbuf_map
struct dbuf_ptr
My favorite would be dbuf_ptr. It's short and the API names would make
sense: dbuf_ptr_clear() for clearing, dbuf_ptr_set_vaddr() to set an
address, dbuf_ptr_incr() to increment, etc. Also, the _ptr indicates
that it's a single address; not an offset with length.
>>> Puh, no idea. All of that doesn't sound like it 100% hits the underlying
>>> meaning of the structure.
>> Imo first let's merge this and then second with more users we might come
>> up with a better name. And cocci is fairly good at large-scale rename, to
>> the point where we manged to rename struct fence to struct dma_fence.
>
> Agreed, renaming things later on is easy if somebody comes up with
> something better.
>
> But blocking a necessary technical change just because of the naming is
> usually not a good idea.
OK, merged now.
Best regards
Thomas
>
> Christian.
>
>>
>> Also this entire thread here imo shows that we haven't yet figured out
>> the
>> perfect name anyway, and I don't think it's worth it to hold this up
>> because of this bikeshed :-)
>>
>> Cheers, Daniel
>>
>>> Christian.
>>>
Best regards
Thomas
> Christian.
>
>>> Anyway, if a better name than dma-buf-map comes in, I'm willing to
>>> rename the thing. Otherwise I intend to merge the patchset by the
>>> end of
>>> the week.
>> Well, the main thing is that I think this shoud be moved away from
>> dma-buf. But if indeed dma-buf is the only relevant user in drm then
>> I am totally fine with the current naming.
>>
>> One alternative named that popped up in my head: struct sys_io_map {}
>> But again, if this is kept in dma-buf then I am fine with the current
>> naming.
>>
>> In other words, if you continue to think this is mostly a dma-buf
>> thing all three patches are:
>> Acked-by: Sam Ravnborg
>>
>> Sam
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
>
Re: [PATCH v3 0/4] dma-buf: Flag vmap'ed memory as system or I/O memory
Am 29.09.20 um 11:14 schrieb Daniel Vetter:
On Mon, Sep 28, 2020 at 01:22:13PM +0200, Christian König wrote:
Am 28.09.20 um 09:37 schrieb Thomas Zimmermann:
Hi
Am 28.09.20 um 08:50 schrieb Christian König:
Am 27.09.20 um 21:16 schrieb Sam Ravnborg:
Hi Thomas.
struct simap {
union {
void __iomem *vaddr_iomem;
void *vaddr;
};
bool is_iomem;
};
Where simap is a shorthand for system_iomem_map
And it could al be stuffed into a include/linux/simap.h file.
Not totally sold on the simap name - but wanted to come up with
something.
Yes. Others, myself included, have suggested to use a name that does not
imply a connection to the dma-buf framework, but no one has come up with
a good name.
I strongly dislike simap, as it's entirely non-obvious what it does.
dma-buf-map is not actually wrong. The structures represents the mapping
of a dma-able buffer in most cases.
With this approach users do not have to pull in dma-buf to use it and
users will not confuse that this is only for dma-buf usage.
There's no need to enable dma-buf. It's all in the header file without
dependencies on dma-buf. It's really just the name.
But there's something else to take into account. The whole issue here is
that the buffer is disconnected from its originating driver, so we don't
know which kind of memory ops we have to use. Thinking about it, I
realized that no one else seemed to have this problem until now.
Otherwise there would be a solution already. So maybe the dma-buf
framework *is* the native use case for this data structure.
We have at least:
linux/fb.h:
union {
char __iomem *screen_base; /* Virtual address */
char *screen_buffer;
};
Which solve more or less the same problem.
I thought this was for convenience. The important is_iomem bit is missing.
I also already noted that in TTM we have exactly the same problem and a
whole bunch of helpers to allow operations on those pointers.
How do you call this within TTM?
ttm_bus_placement, but I really don't like that name.
The data structure represents a pointer to either system or I/O memory,
but not necessatrily device memory. It contains raw data. That would
give something like
struct databuf_map
struct databuf_ptr
struct dbuf_map
struct dbuf_ptr
My favorite would be dbuf_ptr. It's short and the API names would make
sense: dbuf_ptr_clear() for clearing, dbuf_ptr_set_vaddr() to set an
address, dbuf_ptr_incr() to increment, etc. Also, the _ptr indicates
that it's a single address; not an offset with length.
Puh, no idea. All of that doesn't sound like it 100% hits the underlying
meaning of the structure.
Imo first let's merge this and then second with more users we might come
up with a better name. And cocci is fairly good at large-scale rename, to
the point where we manged to rename struct fence to struct dma_fence.
Agreed, renaming things later on is easy if somebody comes up with
something better.
But blocking a necessary technical change just because of the naming is
usually not a good idea.
Christian.
Also this entire thread here imo shows that we haven't yet figured out the
perfect name anyway, and I don't think it's worth it to hold this up
because of this bikeshed :-)
Cheers, Daniel
Christian.
Best regards
Thomas
Christian.
Anyway, if a better name than dma-buf-map comes in, I'm willing to
rename the thing. Otherwise I intend to merge the patchset by the end of
the week.
Well, the main thing is that I think this shoud be moved away from
dma-buf. But if indeed dma-buf is the only relevant user in drm then
I am totally fine with the current naming.
One alternative named that popped up in my head: struct sys_io_map {}
But again, if this is kept in dma-buf then I am fine with the current
naming.
In other words, if you continue to think this is mostly a dma-buf
thing all three patches are:
Acked-by: Sam Ravnborg
Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-develdata=02%7C01%7Cchristian.koenig%40amd.com%7C71c630a7ca1d48476eed08d864581e4f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637369676925032848sdata=CsekzASvj2lY%2B74FIiLc9B5QG7AHma8B2M5y8Cassj4%3Dreserved=0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4.4 36/85] drm/omap: fix possible object reference leak
From: Wen Yang [ Upstream commit 47340e46f34a3b1d80e40b43ae3d7a8da34a3541 ] The call to of_find_matching_node returns a node pointer with refcount incremented thus it must be explicitly decremented after the last usage. Detected by coccinelle with the following warnings: drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:212:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:237:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function. Signed-off-by: Wen Yang Reviewed-by: Laurent Pinchart Reviewed-by: Mukesh Ojha Cc: Tomi Valkeinen Cc: David Airlie Cc: Daniel Vetter Cc: Sebastian Reichel Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: Markus Elfring Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/1554692313-28882-2-git-send-email-wen.yan...@zte.com.cn Signed-off-by: Sasha Levin --- drivers/video/fbdev/omap2/dss/omapdss-boot-init.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c index 8b6f6d5fdd68b..43186fa8a13c9 100644 --- a/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c +++ b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c @@ -194,7 +194,7 @@ static int __init omapdss_boot_init(void) dss = of_find_matching_node(NULL, omapdss_of_match); if (dss == NULL || !of_device_is_available(dss)) - return 0; + goto put_node; omapdss_walk_device(dss, true); @@ -221,6 +221,8 @@ static int __init omapdss_boot_init(void) kfree(n); } +put_node: + of_node_put(dss); return 0; } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/4] drm/qxl: use qxl pin function
On Tue, Sep 29, 2020 at 11:51:15AM +0200, Gerd Hoffmann wrote:
> Otherwise ttm throws a WARN because we try to pin without a reservation.
>
> Fixes: 9d36d4320462 ("drm/qxl: switch over to the new pin interface")
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/qxl/qxl_object.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_object.c
> b/drivers/gpu/drm/qxl/qxl_object.c
> index d3635e3e3267..eb45267d51db 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.c
> +++ b/drivers/gpu/drm/qxl/qxl_object.c
> @@ -145,7 +145,7 @@ int qxl_bo_create(struct qxl_device *qdev,
> return r;
> }
> if (pinned)
> - ttm_bo_pin(>tbo);
> + qxl_bo_pin(bo);
I think this is now after ttm_bo_init, and at that point the object is
visible to lru users and everything. So I do think you need to grab locks
here instead of just incrementing the pin count alone.
It's also I think a bit racy, since ttm_bo_init drops the lock, so someone
might have snuck in and evicted the object already.
I think what you need is to call ttm_bo_init_reserved, then ttm_bo_pin,
then ttm_bo_unreserve, all explicitly.
-Daniel
> *bo_ptr = bo;
> return 0;
> }
> --
> 2.27.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
[PATCH v2 4/4] drm/qxl: use qxl pin function
Otherwise ttm throws a WARN because we try to pin without a reservation.
Fixes: 9d36d4320462 ("drm/qxl: switch over to the new pin interface")
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index d3635e3e3267..eb45267d51db 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -145,7 +145,7 @@ int qxl_bo_create(struct qxl_device *qdev,
return r;
}
if (pinned)
- ttm_bo_pin(>tbo);
+ qxl_bo_pin(bo);
*bo_ptr = bo;
return 0;
}
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/4] drm/qxl: release shadow on shutdown
In case we have a shadow surface on shutdown release
it so it doesn't leak.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 5bef8f121e54..1d9c51022be4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1220,5 +1220,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
+ if (qdev->dumb_shadow_bo) {
+ drm_gem_object_put(>dumb_shadow_bo->tbo.base);
+ qdev->dumb_shadow_bo = NULL;
+ }
qxl_destroy_monitors_object(qdev);
}
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/4] drm/qxl: handle shadow in primary destroy
qxl_primary_atomic_disable must check whenever the framebuffer bo has a
shadow surface and in case it has check the shadow primary status.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 1d9c51022be4..d133e6c2aaf4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -561,6 +561,8 @@ static void qxl_primary_atomic_disable(struct drm_plane
*plane,
if (old_state->fb) {
struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
+ if (bo->shadow)
+ bo = bo->shadow;
if (bo->is_primary) {
qxl_io_destroy_primary(qdev);
bo->is_primary = false;
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/4] drm/qxl: use drmm_mode_config_init
Signed-off-by: Gerd Hoffmann
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/qxl/qxl_display.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 65de1f69af58..5bef8f121e54 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1186,7 +1186,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
int i;
int ret;
- drm_mode_config_init(>ddev);
+ ret = drmm_mode_config_init(>ddev);
+ if (ret)
+ return ret;
ret = qxl_create_monitors_object(qdev);
if (ret)
@@ -1219,5 +1221,4 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
qxl_destroy_monitors_object(qdev);
- drm_mode_config_cleanup(>ddev);
}
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/4] drm/qxl: fix stale mm entries on driver shutdown
v2: repost, add a pin fix. Gerd Hoffmann (4): drm/qxl: use drmm_mode_config_init drm/qxl: release shadow on shutdown drm/qxl: handle shadow in primary destroy drm/qxl: use qxl pin function drivers/gpu/drm/qxl/qxl_display.c | 11 +-- drivers/gpu/drm/qxl/qxl_object.c | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 0/8] Asynchronous flip implementation for i915
On 9/28/2020 5:48 PM, Ville Syrjälä wrote: On Mon, Sep 21, 2020 at 04:32:02PM +0530, Karthik B S wrote: Without async flip support in the kernel, fullscreen apps where game resolution is equal to the screen resolution, must perform an extra blit per frame prior to flipping. Asynchronous page flips will also boost the FPS of Mesa benchmarks. v2: -Few patches have been squashed and patches have been shuffled as per the reviews on the previous version. v3: -Few patches have been squashed and patches have been shuffled as per the reviews on the previous version. v4: -Made changes to fix the sequence and time stamp issue as per the comments received on the previous version. -Timestamps are calculated using the flip done time stamp and current timestamp. Here I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP flag is used for timestamp calculations. -Event is sent from the interrupt handler immediately using this updated timestamps and sequence. -Added more state checks as async flip should only allow change in plane surface address and nothing else should be allowed to change. -Added a separate plane hook for async flip. -Need to find a way to reject fbc enabling if it comes as part of this flip as bspec states that changes to FBC are not allowed. v5: -Fixed the Checkpatch and sparse warnings. v6: -Reverted back to the old timestamping code as per the feedback received. -Added documentation. v7: -Changes in intel_atomic_check_async() -Add vfunc for skl_program_async_surface_address() v8: -Add WA for older platforms with double buffered async address update enable bit. v9: -Changes as per feedback received on previous version. v10: -Changes as per feedback received on previous version. Everything seems good, so pushed the series to dinq. Thanks. Gave this a little test run on my cfl as well. At first it didn't kick in, but then I remebered that I'm running X with modifiers enabled so I was getting compression instead. After disabling modifiers I got plain old X-tile again and did see async flips happening. Thanks for the merge. Thanks, Karthik.B.S Karthik B S (8): drm/i915: Add enable/disable flip done and flip done handler drm/i915: Add support for async flips in I915 drm/i915: Add checks specific to async flips drm/i915: Do not call drm_crtc_arm_vblank_event in async flips drm/i915: Add dedicated plane hook for async flip case drm/i915: WA for platforms with double buffered address update enable bit Documentation/gpu: Add asynchronous flip documentation for i915 drm/i915: Enable async flips in i915 Documentation/gpu/i915.rst| 6 + .../gpu/drm/i915/display/intel_atomic_plane.c | 6 +- drivers/gpu/drm/i915/display/intel_display.c | 199 ++ .../drm/i915/display/intel_display_types.h| 3 + drivers/gpu/drm/i915/display/intel_sprite.c | 30 +++ drivers/gpu/drm/i915/i915_irq.c | 52 + drivers/gpu/drm/i915/i915_irq.h | 3 + drivers/gpu/drm/i915/i915_reg.h | 1 + 8 files changed, 299 insertions(+), 1 deletion(-) -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/9] drm/format-helper: Pass destination pitch to drm_fb_memcpy_dstclip()
Hi Am 29.09.20 um 11:19 schrieb Daniel Vetter: > On Mon, Sep 28, 2020 at 11:13:06AM +0200, Thomas Zimmermann wrote: >> Hi >> >> Am 28.09.20 um 10:53 schrieb Daniel Vetter: >>> On Mon, Sep 28, 2020 at 9:22 AM Thomas Zimmermann >>> wrote: Hi Am 26.09.20 um 18:42 schrieb Daniel Vetter: > On Fri, Sep 25, 2020 at 4:55 PM Thomas Zimmermann > wrote: >> >> Hi >> >> Am 29.06.20 um 10:40 schrieb Daniel Vetter: >>> On Thu, Jun 25, 2020 at 02:00:03PM +0200, Thomas Zimmermann wrote: The memcpy's destination buffer might have a different pitch than the source. Support different pitches as function argument. Signed-off-by: Thomas Zimmermann >>> >>> Reviewed-by: Daniel Vetter >>> >>> But I do have questions ... why did we allocate a source drm_framebuffer >>> with mismatching pitch? That sounds backwards, especially for simplekms. >> >> There's userspace that allocates framebuffers in tiles of 64x64 pixels. >> I think I've seen this with Gnome. So if you have a 800x600 display >> mode, the allocated framebuffer has a scanline pitch of 832 pixels and >> the final 32 pixels are ignored. > > At least with dumb buffer allocation ioctls userspace should not do > that. If it wants 800x600, it needs to allocate 800x600, not something That ship has sailed. >>> >>> Not really, right now that ship is simply leaking and sinking. If we >>> decide to patch this up from the kernel side, then indeed it has >>> sailed. And I'm not sure that's a good idea. >> >> We have code in at least cirrus, ast and mgag200 to support this. And >> userspace has been behaving like this since at least when I got involved >> (2017). > > Hm where do these drivers copy stuff around and rematch the stride? They don't. These drivers adopt their HW stride to match whatever userspace framebuffers tell them. [1] And that's because userspace gives them framebuffer sizes like 832*640. And then they do a memcpy with the given width. My sole point here was that userspace already relies on this behavior. Best regards Thomas [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/tiny/cirrus.c#n285 > > else. The driver is supposed to apply any rounding necessary for the > size. Or is this a buffer allocated somewhere else and then shared? I don't quite remember where exactly this was implemented. It was not a shared buffer, though. IIRC the buffer allocation code in one of the libs rounded the size towards multiples of 64. I remember thinking that it was probably done for tiled rendering. >>> >>> Yeah, but you don't do rendering on dumb buffers. Like ever. So this >>> smells like a userspace bug. >> >> It's also part of the software rendering. It is not a bug, but >> implemented deliberately in one of the userspace components that >> allocates framebuffers (but I cannot remember which one.) > > I think it would be good to document this. > > We already fake xrgb everywhere because userspace is not flexible > enough, I guess having to fake 64b stride support everywhere isn't that > much worse. > > But it's definitely not great either :-/ > -Daniel > >> >> Best regards >> Thomas >> >>> >>> If it's for shared buffers then I think that sounds more reasonable. >>> -Daniel >>> Best regards Thomas > -Daniel > >> In regular drivers, we can handle this with the VGA offset register [1] >> or some equivalent. That's obviously not an option with simplekms, so >> the different pitch is required. >> >> Best regards >> Thomas >> >> [1] >> https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/crtcreg.htm#13 >> >>> >>> Would be good to add the reasons why we need this to the commit message, >>> I'm sure I'll discover it later on eventually. >>> -Daniel >>> --- drivers/gpu/drm/drm_format_helper.c| 9 + drivers/gpu/drm/mgag200/mgag200_mode.c | 2 +- drivers/gpu/drm/tiny/cirrus.c | 2 +- include/drm/drm_format_helper.h| 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index c043ca364c86..8d5a683afea7 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -52,6 +52,7 @@ EXPORT_SYMBOL(drm_fb_memcpy); /** * drm_fb_memcpy_dstclip - Copy clip buffer * @dst: Destination buffer (iomem) + * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @vaddr: Source buffer * @fb: DRM framebuffer * @clip: Clip rectangle area to copy @@ -59,12 +60,12 @@ EXPORT_SYMBOL(drm_fb_memcpy);
Re: [PATCH v4 01/19] drm/virtio: blob prep: refactor getting pages and attaching backing
On Wed, Sep 23, 2020 at 05:31:56PM -0700, Gurchetan Singh wrote: > Useful for upcoming blob resources. Pushed to drm-misc-next (whole series). thanks, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/qxl: simplify the return expression of qxl_plane_prepare_fb()
On Mon, Sep 21, 2020 at 09:10:22PM +0800, Qinglang Miao wrote: > Simplify the return expression. Pushed to drm-misc-next. thanks, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > Hi, > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > Also revert the follow-up change "drm: pl111: Absorb the external > > register header". > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > The fbdev driver is used by Android's FVP configuration. Using the > > DRM driver together with DRM's fbdev emulation results in a failure > > to boot Android. The root cause is that Android's generic fbdev > > userspace driver relies on the ability to set the pixel format via > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > Can't Android FVP use drm-hwcomposer instead ? Also, if we need to add more random fbdev ioctls to the drm fbdev emulation, then let's do that. Not keep fbdev drivers on life support for longer than necessary. > > Neil > > > > > There have been other less critical behavioral differences identified > > between the fbdev driver and the DRM driver with fbdev emulation. The > > DRM driver exposes different values for the panel's width, height and > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > with yres_virtual greater than the maximum supported value instead > > of letting the syscall succeed and setting yres_virtual based on yres. Also something that should be fixed I think in upstream, in the drm fbdev emulation. At least doesn't sound like it's something unfixable. -Daniel > > > > Signed-off-by: Peter Collingbourne > > --- > > View this change in Gerrit: > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > MAINTAINERS | 5 + > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > > drivers/video/fbdev/Kconfig | 20 + > > drivers/video/fbdev/Makefile| 1 + > > drivers/video/fbdev/amba-clcd.c | 986 > > include/linux/amba/clcd-regs.h | 87 +++ > > include/linux/amba/clcd.h | 290 +++ > > 11 files changed, 1393 insertions(+), 73 deletions(-) > > create mode 100644 drivers/video/fbdev/amba-clcd.c > > create mode 100644 include/linux/amba/clcd-regs.h > > create mode 100644 include/linux/amba/clcd.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 190c7fa2ea01..671c1fa79e64 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -1460,6 +1460,11 @@ S: Odd Fixes > > F: drivers/amba/ > > F: include/linux/amba/bus.h > > > > +ARM PRIMECELL CLCD PL110 DRIVER > > +M: Russell King > > +S: Odd Fixes > > +F: drivers/video/fbdev/amba-clcd.* > > + > > ARM PRIMECELL KMI PL050 DRIVER > > M: Russell King > > S: Odd Fixes > > diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c > > b/drivers/gpu/drm/pl111/pl111_debugfs.c > > index 317f68abf18b..26ca8cdf3e60 100644 > > --- a/drivers/gpu/drm/pl111/pl111_debugfs.c > > +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c > > @@ -3,6 +3,7 @@ > > * Copyright © 2017 Broadcom > > */ > > > > +#include > > #include > > > > #include > > diff --git a/drivers/gpu/drm/pl111/pl111_display.c > > b/drivers/gpu/drm/pl111/pl111_display.c > > index b3e8697cafcf..703ddc803c55 100644 > > --- a/drivers/gpu/drm/pl111/pl111_display.c > > +++ b/drivers/gpu/drm/pl111/pl111_display.c > > @@ -9,6 +9,7 @@ > > * Copyright (C) 2011 Texas Instruments > > */ > > > > +#include > > #include > > #include > > #include > > diff --git a/drivers/gpu/drm/pl111/pl111_drm.h > > b/drivers/gpu/drm/pl111/pl111_drm.h > > index 2a46b5bd8576..ba399bcb792f 100644 > > --- a/drivers/gpu/drm/pl111/pl111_drm.h > > +++ b/drivers/gpu/drm/pl111/pl111_drm.h > > @@ -23,79 +23,6 @@ > > #include > > #include > > > > -/* > > - * CLCD Controller Internal Register addresses > > - */ > > -#define CLCD_TIM0 0x > > -#define CLCD_TIM1 0x0004 > > -#define CLCD_TIM2 0x0008 > > -#define CLCD_TIM3 0x000c > > -#define CLCD_UBAS 0x0010 > > -#define CLCD_LBAS 0x0014 > > - > > -#define CLCD_PL110_IENB0x0018 > > -#define CLCD_PL110_CNTL0x001c > > -#define CLCD_PL110_STAT0x0020 > > -#define CLCD_PL110_INTR0x0024 > > -#define CLCD_PL110_UCUR0x0028 > > -#define CLCD_PL110_LCUR0x002C > > - > > -#define CLCD_PL111_CNTL0x0018 > > -#define CLCD_PL111_IENB0x001c > > -#define CLCD_PL111_RIS 0x0020 > > -#define CLCD_PL111_MIS 0x0024 > > -#define CLCD_PL111_ICR 0x0028 > > -#define CLCD_PL111_UCUR0x002c > > -#define CLCD_PL111_LCUR0x0030 > > -
Re: [PATCH v4 0/4] drm: panel: add support for TDO tl070wsh30 panel
Hi Sam, Is there anything more to change ? Thanks, Neil On 15/09/2020 14:19, Neil Armstrong wrote: > This adds support bindings and support for the TDO TL070WSH30 TFT-LCD panel > module shipped with the Amlogic S400 Development Kit. > The panel has a 1024×600 resolution and uses 24 bit RGB per pixel. > It provides a MIPI DSI interface to the host, a built-in LED backlight > and touch controller. > > Changes since v3 at [3]: > - reordered tdo vendor prefix > > Changes since v2 at [2]: > - added reset gpio to panel-simple-dsi bindings > - moved bindings to panel-simple > - re-ordered prepare/unprepare > - added back refresh print > - moved regulator/reset handling in prepare/unprepare > - aligned remove/shutdown with panel-simple > - added sentinel comment into dt match table > > Changes since v1 at [1]: > - added missing vendor-prefixes patch > - removed vrefresh > - fixed warning on add_panel return > - removed DRM logging > > [1] https://patchwork.freedesktop.org/series/81376/#rev1 > [2] https://patchwork.freedesktop.org/series/81376/#rev2 > [3] https://patchwork.freedesktop.org/series/81376/#rev3 > > Neil Armstrong (4): > dt-bindings: vendor-prefixes: Add Shanghai Top Display > Optolelectronics vendor prefix > dt-bindings: display: panel-simple-dsi: add optional reset gpio > dt-bindings: display: panel-simple-dsi: add TDO TL070WSH30 DSI panel > bindings > drm: panel: add TDO tl070wsh30 panel driver > > .../display/panel/panel-simple-dsi.yaml | 3 + > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > drivers/gpu/drm/panel/Kconfig | 11 + > drivers/gpu/drm/panel/Makefile| 1 + > drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c | 250 ++ > 5 files changed, 267 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
