Re: [PATCH v3] drm/test: add a test suite for GEM objects backed by shmem
Hi Marco, kernel test robot noticed the following build warnings: [auto build test WARNING on c79b972eb88b077d2765e7790d0902b3dc94d55c] url: https://github.com/intel-lab-lkp/linux/commits/Marco-Pagani/drm-test-add-a-test-suite-for-GEM-objects-backed-by-shmem/20231120-230829 base: c79b972eb88b077d2765e7790d0902b3dc94d55c patch link: https://lore.kernel.org/r/20231120150339.104257-1-marpagan%40redhat.com patch subject: [PATCH v3] drm/test: add a test suite for GEM objects backed by shmem config: arm-kismet-CONFIG_DRM_GEM_SHMEM_HELPER-CONFIG_DRM_KUNIT_TEST-0-0 (https://download.01.org/0day-ci/archive/20231121/202311211542.3zrzo6j2-...@intel.com/config) reproduce: (https://download.01.org/0day-ci/archive/20231121/202311211542.3zrzo6j2-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202311211542.3zrzo6j2-...@intel.com/ kismet warnings: (new ones prefixed by >>) >> kismet: WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER >> when selected by DRM_KUNIT_TEST WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && MMU [=n] Selected by [y]: - DRM_KUNIT_TEST [=y] && HAS_IOMEM [=y] && DRM [=y] && KUNIT [=y] -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
RE: [PATCH v4 3/5] mm/gup: Introduce pin_user_pages_fd() for pinning shmem/hugetlbfs file pages (v4)
Hi David,
>
> On 18.11.23 07:32, Vivek Kasireddy wrote:
> > For drivers that would like to longterm-pin the pages associated
> > with a file, the pin_user_pages_fd() API provides an option to
> > not only pin the pages via FOLL_PIN but also to check and migrate
> > them if they reside in movable zone or CMA block. This API
> > currently works with files that belong to either shmem or hugetlbfs.
> > Files belonging to other filesystems are rejected for now.
> >
> > The pages need to be located first before pinning them via FOLL_PIN.
> > If they are found in the page cache, they can be immediately pinned.
> > Otherwise, they need to be allocated using the filesystem specific
> > APIs and then pinned.
> >
> > v2:
> > - Drop gup_flags and improve comments and commit message (David)
> > - Allocate a page if we cannot find in page cache for the hugetlbfs
> >case as well (David)
> > - Don't unpin pages if there is a migration related failure (David)
> > - Drop the unnecessary nr_pages <= 0 check (Jason)
> > - Have the caller of the API pass in file * instead of fd (Jason)
> >
> > v3: (David)
> > - Enclose the huge page allocation code with #ifdef
> CONFIG_HUGETLB_PAGE
> >(Build error reported by kernel test robot )
> > - Don't forget memalloc_pin_restore() on non-migration related errors
> > - Improve the readability of the cleanup code associated with
> >non-migration related errors
> > - Augment the comments by describing FOLL_LONGTERM like behavior
> > - Include the R-b tag from Jason
> >
> > v4:
> > - Remove the local variable "page" and instead use 3 return statements
> >in alloc_file_page() (David)
> > - Add the R-b tag from David
> >
> > Cc: David Hildenbrand
> > Cc: Daniel Vetter
> > Cc: Mike Kravetz
> > Cc: Hugh Dickins
> > Cc: Peter Xu
> > Cc: Gerd Hoffmann
> > Cc: Dongwon Kim
> > Cc: Junxiao Chang
> > Suggested-by: Jason Gunthorpe
> > Reviewed-by: Jason Gunthorpe (v2)
> > Reviewed-by: David Hildenbrand (v3)
> > Signed-off-by: Vivek Kasireddy
> > ---
>
>
> [...]
>
>
> > +static struct page *alloc_file_page(struct file *file, pgoff_t idx)
> > +{
> > +#ifdef CONFIG_HUGETLB_PAGE
> > + struct folio *folio;
> > + int err;
> > +
> > + if (is_file_hugepages(file)) {
> > + folio = alloc_hugetlb_folio_nodemask(hstate_file(file),
> > +NUMA_NO_NODE,
> > +NULL,
> > +GFP_USER);
> > + if (folio && folio_try_get(folio)) {
> > + err = hugetlb_add_to_page_cache(folio,
> > + file->f_mapping,
> > + idx);
> > + if (err) {
> > + folio_put(folio);
> > + free_huge_folio(folio);
> > + return ERR_PTR(err);
> > + }
> > + return &folio->page;
>
> While looking at the user of pin_user_pages_fd(), I realized something:
>
> Assume idx is not aligned to the hugetlb page size.
> find_get_page_flags() would always return a tail page in that case, but
> you'd be returning the head page here.
>
> See pagecache_get_page()->folio_file_page(folio, index);
Thank you for catching this. Looking at how udambuf uses this API for hugetlb
case:
hpstate = hstate_file(memfd);
mapidx = list[i].offset >> huge_page_shift(hpstate);
do {
nr_pages = shmem_file(memfd) ? pgcnt : 1;
ret = pin_user_pages_fd(memfd, mapidx, nr_pages,
ubuf->pages +
pgbuf);
As the raw file offset is translated into huge page size units, represented by
mapidx, I was expecting find_get_page_flags() to return a head page but I
did not realize that find_get_page_flags() now returns tail pages given that
it had returned head pages in the previous kernel versions I had tested IIRC.
As my goal is to only grab the head pages, __filemap_get_folio() seems like
the right API to use instead of find_get_page_flags(). With this change, the
hugetlb subtest (that I have not tested with kernels >= 6.7) that fails with
kernel 6.7 RC1 now seems to work as expected.
>
> > + }
> > + return ERR_PTR(-ENOMEM);
> > + }
> > +#endif
> > + return shmem_read_mapping_page(file->f_mapping, idx);
> > +}
> > +
> > +/**
> > + * pin_user_pages_fd() - pin user pages associated with a file
> > + * @file: the file whose pages are to be pinned
> > + * @start: starting file offset
> > + * @nr_pages: number of pages from start to pin
> > + * @pages: array that receives pointers to the pages pinned.
> > + * Should be at-least nr_pages long.
> > + *
> > + * Attempt to pin pages associated with a file that belongs to either
> shmem
> > + * or hugetlb. The pages are either found in the page cache or allocated if
> > + * necessary. Once
[PATCH v6] drm/mediatek: add dma buffer control for drm plane disable
the DMA buffer is released when still accessed by Mediatek display hardware,
this flow can lead to M4U reporting a translation fault warning
add dma buffer control flow in mediatek driver:
get dma buffer when drm plane disable
put dma buffer when overlay really disable
Fixes: 41016fe1028e ("drm: Rename plane->state variables in atomic update and
disable")
Signed-off-by: Yongqiang Niu
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 25
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 12
drivers/gpu/drm/mediatek/mtk_drm_plane.h | 1 +
4 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index c277b9fae950..188aaa97e5e3 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -4,6 +4,7 @@
*/
#include
+#include
#include
#include
#include
@@ -283,6 +284,23 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct
drm_crtc *crtc,
return NULL;
}
+static void mtk_drm_dma_buf_put(struct mtk_drm_crtc *mtk_crtc)
+{
+ unsigned int i;
+
+ for (i = 0; i < mtk_crtc->layer_nr; i++) {
+ struct drm_plane *plane = &mtk_crtc->planes[i];
+ struct mtk_plane_state *plane_state;
+
+ plane_state = to_mtk_plane_state(plane->state);
+
+ if (plane_state && plane_state->pending.dma_buf) {
+ dma_buf_put(plane_state->pending.dma_buf);
+ plane_state->pending.dma_buf = NULL;
+ }
+ }
+}
+
#if IS_REACHABLE(CONFIG_MTK_CMDQ)
static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg)
{
@@ -323,6 +341,8 @@ static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg)
mtk_crtc->pending_async_planes = false;
}
+ mtk_drm_dma_buf_put(mtk_crtc);
+
mtk_crtc->cmdq_vblank_cnt = 0;
wake_up(&mtk_crtc->cb_blocking_queue);
}
@@ -627,9 +647,14 @@ static void mtk_crtc_ddp_irq(void *data)
else if (mtk_crtc->cmdq_vblank_cnt > 0 && --mtk_crtc->cmdq_vblank_cnt
== 0)
DRM_ERROR("mtk_crtc %d CMDQ execute command timeout!\n",
drm_crtc_index(&mtk_crtc->base));
+
+ if (!mtk_crtc->cmdq_client.chan)
+ mtk_drm_dma_buf_put(mtk_crtc);
#else
if (!priv->data->shadow_register)
mtk_crtc_ddp_config(crtc, NULL);
+
+ mtk_drm_dma_buf_put(mtk_crtc);
#endif
mtk_drm_finish_page_flip(mtk_crtc);
}
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 2dfaa613276a..4fd232644383 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -1019,4 +1019,5 @@ module_exit(mtk_drm_exit);
MODULE_AUTHOR("YT SHEN ");
MODULE_DESCRIPTION("Mediatek SoC DRM driver");
+MODULE_IMPORT_NS(DMA_BUF);
MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index ddc9355b06d5..fbfcd0d1a56c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include "mtk_drm_crtc.h"
#include "mtk_drm_ddp_comp.h"
@@ -283,6 +284,17 @@ static void mtk_plane_atomic_disable(struct drm_plane
*plane,
struct drm_plane_state *new_state =
drm_atomic_get_new_plane_state(state,
plane);
struct mtk_plane_state *mtk_plane_state = to_mtk_plane_state(new_state);
+ struct drm_plane_state *old_state =
drm_atomic_get_old_plane_state(state,
+
plane);
+
+ if (old_state && old_state->fb) {
+ struct drm_gem_object *gem = old_state->fb->obj[0];
+
+ if (gem && gem->dma_buf) {
+ get_dma_buf(gem->dma_buf);
+ mtk_plane_state->pending.dma_buf = gem->dma_buf;
+ }
+ }
mtk_plane_state->pending.enable = false;
wmb(); /* Make sure the above parameter is set before update */
mtk_plane_state->pending.dirty = true;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
index 99aff7da0831..3aba0b58ef3c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
@@ -33,6 +33,7 @@ struct mtk_plane_pending_state {
boolasync_dirty;
boolasync_config;
enum drm_color_encoding color_encoding;
+ struct dma_buf *dma_buf;
};
struct mtk_plane_state {
--
2.25.1
RE: [PATCH 2/4] drm/i915: Adjust LUT rounding rules
> -Original Message-
> From: Ville Syrjälä
> Sent: Monday, November 20, 2023 7:57 PM
> To: Borah, Chaitanya Kumar
> Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH 2/4] drm/i915: Adjust LUT rounding rules
>
> On Mon, Nov 20, 2023 at 06:08:57AM +, Borah, Chaitanya Kumar wrote:
> > Hello Ville,
> >
> > > -Original Message-
> > > From: dri-devel On Behalf
> > > Of Ville Syrjala
> > > Sent: Friday, October 13, 2023 6:44 PM
> > > To: intel-...@lists.freedesktop.org
> > > Cc: dri-devel@lists.freedesktop.org
> > > Subject: [PATCH 2/4] drm/i915: Adjust LUT rounding rules
> > >
> > > From: Ville Syrjälä
> > >
> > > drm_color_lut_extract() rounding was changed to follow the OpenGL
> > > int<-
> > > >float conversion rules. Adjust intel_color_lut_pack() to match.
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/display/intel_color.c | 14 ++
> > > 1 file changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > > b/drivers/gpu/drm/i915/display/intel_color.c
> > > index 2a2a163ea652..b01f463af861 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > > @@ -785,14 +785,12 @@ static void chv_assign_csc(struct
> > > intel_crtc_state
> > > *crtc_state)
> > > /* convert hw value with given bit_precision to lut property val */
> > > static u32
> > > intel_color_lut_pack(u32 val, int bit_precision) {
> >
> > Is this operation unique to Intel. Should there be a drm helper for this?
>
> If some other driver gains gamma readout support they could probably use
> something like this. The other option would be to rework the current helper
> to allow conversions both ways.
>
The function name could be a minor inconvenience but anyway until that time
arrives.
LGTM.
Reviewed-by: Chaitanya Kumar Borah
> >
> > Regards
> >
> > Chaitanya
> >
> > > - u32 max = 0x >> (16 - bit_precision);
> > > -
> > > - val = clamp_val(val, 0, max);
> > > -
> > > - if (bit_precision < 16)
> > > - val <<= 16 - bit_precision;
> > > -
> > > - return val;
> > > + if (bit_precision > 16)
> > > + return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(val, (1 << 16)
> > > - 1),
> > > + (1 << bit_precision) - 1);
> > > + else
> > > + return DIV_ROUND_CLOSEST(val * ((1 << 16) - 1),
> > > + (1 << bit_precision) - 1);
> > > }
> > >
> > > static u32 i9xx_lut_8(const struct drm_color_lut *color)
> > > --
> > > 2.41.0
> >
>
> --
> Ville Syrjälä
> Intel
Re: [PATCH v3 1/1] drm/mediatek: Fix errors when reporting rotation capability
Re: [PATCH v2] Remove custom dumb_map_offset implementation in msm driver
On Wed, Nov 15, 2023 at 11:33 AM Dmitry Baryshkov
wrote:
>
> On Wed, 15 Nov 2023 at 20:46, Dipam Turkar wrote:
> >
> > They are not outdated, my bad. I went through the locks' code and saw that
> > they have been updated. But they are probably not necessary here as most of
> > the drivers do not use any form of locking in their implementations. The
> > generic implementations drm_gem_dumb_map_offset() and
> > drm_gem_ttm_dumb_map_offset() do not have any locking mechanisms either.
>
> Excuse me, but this doesn't sound right to me. There are different
> drivers with different implementations. So either we'd need a good
> explanation of why it is not necessary, or this patch is NAKed.
Digging a bit thru history, it looks like commit 0de23977cfeb
("drm/gem: convert to new unified vma manager") made external locking
unnecessary, since the vma mgr already had it's own internal locking.
BR,
-R
> >
> > Thanks and regards
> > Dipam Turkar
> >
> > On Wed, Nov 15, 2023 at 8:37 PM Dmitry Baryshkov
> > wrote:
> >>
> >> On Wed, 15 Nov 2023 at 16:30, Dipam Turkar wrote:
> >> >
> >> > Make msm use drm_gem_create_map_offset() instead of its custom
> >> > implementation for associating GEM object with a fake offset. Since,
> >> > we already have this generic implementation, we don't need the custom
> >> > implementation and it is better to standardize the code for GEM based
> >> > drivers. This also removes the outdated locking leftovers.
> >>
> >> Why are they outdated?
> >>
> >> >
> >> > Signed-off-by: Dipam Turkar
> >> > ---
> >> > drivers/gpu/drm/msm/msm_drv.c | 2 +-
> >> > drivers/gpu/drm/msm/msm_gem.c | 21 -
> >> > drivers/gpu/drm/msm/msm_gem.h | 2 --
> >> > 3 files changed, 1 insertion(+), 24 deletions(-)
> >> >
> >> > Changes in v2:
> >> > Modify commit message to include the absence of internal locking
> >> > leftovers
> >> > around allocating a fake offset in msm_gem_mmap_offset() in the generic
> >> > implementation drm_gem_create_map_offset().
> >> >
> >> > diff --git a/drivers/gpu/drm/msm/msm_drv.c
> >> > b/drivers/gpu/drm/msm/msm_drv.c
> >> > index a428951ee539..86a15992c717 100644
> >> > --- a/drivers/gpu/drm/msm/msm_drv.c
> >> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> >> > @@ -1085,7 +1085,7 @@ static const struct drm_driver msm_driver = {
> >> > .open = msm_open,
> >> > .postclose = msm_postclose,
> >> > .dumb_create= msm_gem_dumb_create,
> >> > - .dumb_map_offset= msm_gem_dumb_map_offset,
> >> > + .dumb_map_offset= drm_gem_dumb_map_offset,
> >> > .gem_prime_import_sg_table = msm_gem_prime_import_sg_table,
> >> > #ifdef CONFIG_DEBUG_FS
> >> > .debugfs_init = msm_debugfs_init,
> >> > diff --git a/drivers/gpu/drm/msm/msm_gem.c
> >> > b/drivers/gpu/drm/msm/msm_gem.c
> >> > index db1e748daa75..489694ef79cb 100644
> >> > --- a/drivers/gpu/drm/msm/msm_gem.c
> >> > +++ b/drivers/gpu/drm/msm/msm_gem.c
> >> > @@ -671,27 +671,6 @@ int msm_gem_dumb_create(struct drm_file *file,
> >> > struct drm_device *dev,
> >> > MSM_BO_SCANOUT | MSM_BO_WC, &args->handle,
> >> > "dumb");
> >> > }
> >> >
> >> > -int msm_gem_dumb_map_offset(struct drm_file *file, struct drm_device
> >> > *dev,
> >> > - uint32_t handle, uint64_t *offset)
> >> > -{
> >> > - struct drm_gem_object *obj;
> >> > - int ret = 0;
> >> > -
> >> > - /* GEM does all our handle to object mapping */
> >> > - obj = drm_gem_object_lookup(file, handle);
> >> > - if (obj == NULL) {
> >> > - ret = -ENOENT;
> >> > - goto fail;
> >> > - }
> >> > -
> >> > - *offset = msm_gem_mmap_offset(obj);
> >> > -
> >> > - drm_gem_object_put(obj);
> >> > -
> >> > -fail:
> >> > - return ret;
> >> > -}
> >> > -
> >> > static void *get_vaddr(struct drm_gem_object *obj, unsigned madv)
> >> > {
> >> > struct msm_gem_object *msm_obj = to_msm_bo(obj);
> >> > diff --git a/drivers/gpu/drm/msm/msm_gem.h
> >> > b/drivers/gpu/drm/msm/msm_gem.h
> >> > index 8ddef5443140..dc74a0ef865d 100644
> >> > --- a/drivers/gpu/drm/msm/msm_gem.h
> >> > +++ b/drivers/gpu/drm/msm/msm_gem.h
> >> > @@ -139,8 +139,6 @@ struct page **msm_gem_pin_pages(struct
> >> > drm_gem_object *obj);
> >> > void msm_gem_unpin_pages(struct drm_gem_object *obj);
> >> > int msm_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
> >> > struct drm_mode_create_dumb *args);
> >> > -int msm_gem_dumb_map_offset(struct drm_file *file, struct drm_device
> >> > *dev,
> >> > - uint32_t handle, uint64_t *offset);
> >> > void *msm_gem_get_vaddr_locked(struct drm_gem_object *obj);
> >> > void *msm_gem_get_vaddr(struct drm_gem_object *obj);
> >> > void *msm_gem_get_vaddr_active(struct drm_gem_object *obj);
> >> > --
> >> > 2.34.1
> >> >
> >>
> >>
> >> --
> >> With best wishes
> >> Dmitry
>
>
>
> --
> With best wishes
> Dmitry
Re: [PATCH v3] driver: gpu: Fixing warning directly dereferencing a rcu pointer
On 11/13/23 20:13, Abhinav Singh wrote:
This patch fixes a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
We should not be directly dereferencing a rcu pointer. To get a normal
(non __rcu tagged pointer) from a __rcu tagged pointer we are using the
function unrcu_pointer(...). The non __rcu tagged pointer then can be
dereferenced just like a normal pointer.
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
Signed-off-by: Abhinav Singh
Applied, thanks!
There are a few more such occurrences. [1][2] Plan to fix them as well?
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/nouveau/nv10_fence.c#L35
[2]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/nouveau/nv84_fence.c#L88
---
v1 -> v2 : Replaced the rcu_dereference(...) with unrcu_pointer(...) and
also removed the rcu locking and unlocking function call.
v2 -> v3 : Changed the description of the patch to match it with the actual
implementation.
drivers/gpu/drm/nouveau/nv04_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nv04_fence.c
b/drivers/gpu/drm/nouveau/nv04_fence.c
index 5b71a5a5cd85..cdbc75e3d1f6 100644
--- a/drivers/gpu/drm/nouveau/nv04_fence.c
+++ b/drivers/gpu/drm/nouveau/nv04_fence.c
@@ -39,7 +39,7 @@ struct nv04_fence_priv {
static int
nv04_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ struct nvif_push *push = unrcu_pointer(fence->channel)->chan.push;
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_NVSQ(push, NV_SW, 0x0150, fence->base.seqno);
Re: [PATCH v2 2/2] fbdev/simplefb: Add support for generic power-domains
Hello,
On Wed, Nov 01, 2023 at 06:20:17PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> The simple-framebuffer device tree bindings document the power-domains
> property, so make sure that simplefb supports it. This ensures that the
> power domains remain enabled as long as simplefb is active.
>
> v2: - remove unnecessary call to simplefb_detach_genpds() since that's
> already done automatically by devres
> - fix crash if power-domains property is missing in DT
>
> Signed-off-by: Thierry Reding
> ---
> drivers/video/fbdev/simplefb.c | 93 ++
> 1 file changed, 93 insertions(+)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index 18025f34fde7..fe682af63827 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> static const struct fb_fix_screeninfo simplefb_fix = {
> @@ -78,6 +79,11 @@ struct simplefb_par {
> unsigned int clk_count;
> struct clk **clks;
> #endif
> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> + unsigned int num_genpds;
This is the cause of the crash that occurred on the older patch series.
The field is unsigned, a deviation from v6.6:drivers/remoteproc/imx_rproc.c.
Instead of making it signed, this version emits an error whenever the
count is negative.
> + struct device **genpds;
> + struct device_link **genpd_links;
> +#endif
> #if defined CONFIG_OF && defined CONFIG_REGULATOR
> bool regulators_enabled;
> u32 regulator_count;
> @@ -432,6 +438,89 @@ static void simplefb_regulators_enable(struct
> simplefb_par *par,
> static void simplefb_regulators_destroy(struct simplefb_par *par) { }
> #endif
>
> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> +static void simplefb_detach_genpds(void *res)
> +{
> + struct simplefb_par *par = res;
> + unsigned int i = par->num_genpds;
> +
> + if (par->num_genpds <= 1)
> + return;
> +
> + while (i--) {
> + if (par->genpd_links[i])
> + device_link_del(par->genpd_links[i]);
> +
> + if (!IS_ERR_OR_NULL(par->genpds[i]))
> + dev_pm_domain_detach(par->genpds[i], true);
> + }
> +}
> +
> +static int simplefb_attach_genpds(struct simplefb_par *par,
> + struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + unsigned int i;
> + int err;
> +
> + err = of_count_phandle_with_args(dev->of_node, "power-domains",
> + "#power-domain-cells");
> + if (err < 0) {
> + dev_info(dev, "failed to parse power-domains: %d\n", err);
> + return err;
This error path is taken when there is no power-domains property in the
device tree with err = -ENOENT.
Strangely, this does not suppress the error like the next if statement,
even though it is possible that nothing is wrong.
> + }
> +
> + par->num_genpds = err;
> +
> + /*
> + * Single power-domain devices are handled by the driver core, so
> + * nothing to do here.
> + */
> + if (par->num_genpds <= 1)
> + return 0;
> +
> + par->genpds = devm_kcalloc(dev, par->num_genpds, sizeof(*par->genpds),
> +GFP_KERNEL);
> @@ -518,6 +607,10 @@ static int simplefb_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_clocks;
>
> + ret = simplefb_attach_genpds(par, pdev);
> + if (ret < 0)
> + goto error_regulators;
With the error case specified above, not specifying power-domains (which
is valid according to dtschema) causes the entire driver to fail
whenever there are no power domains in the device tree.
On google-sargo, this causes a bug where the framebuffer fails to probe:
[0.409290] simple-framebuffer 9c00.framebuffer: failed to parse
power-domains: -2
[0.409340] simple-framebuffer: probe of 9c00.framebuffer failed
with error -2
[PATCH -next] drm/amd/display: Remove duplicated include in dcn201_resource.c
./drivers/gpu/drm/amd/display/dc/resource/dcn201/dcn201_resource.c: dcn201/dcn201_hubbub.h is included more than once. Reported-by: Abaci Robot Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7583 Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/resource/dcn201/dcn201_resource.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn201/dcn201_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn201/dcn201_resource.c index 914b234d7f6b..3cfb7e913c4c 100644 --- a/drivers/gpu/drm/amd/display/dc/resource/dcn201/dcn201_resource.c +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn201/dcn201_resource.c @@ -55,7 +55,6 @@ #include "dce110/dce110_resource.h" #include "dce/dce_aux.h" #include "dce/dce_i2c.h" -#include "dcn201/dcn201_hubbub.h" #include "dcn10/dcn10_resource.h" #include "cyan_skillfish_ip_offset.h" -- 2.20.1.7.g153144c
[PATCH v2 7/7] drm/msm/gem: Convert to drm_exec
From: Rob Clark
Replace the ww_mutex locking dance with the drm_exec helper.
v2: Error path fixes, move drm_exec_fini so we only call it once (and
only if we have drm_exec_init()
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/Kconfig | 1 +
drivers/gpu/drm/msm/msm_gem.h| 5 +-
drivers/gpu/drm/msm/msm_gem_submit.c | 119 +--
3 files changed, 24 insertions(+), 101 deletions(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 6309a857ca31..f91d87afc0d3 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -16,6 +16,7 @@ config DRM_MSM
select DRM_DP_AUX_BUS
select DRM_DISPLAY_DP_HELPER
select DRM_DISPLAY_HELPER
+ select DRM_EXEC
select DRM_KMS_HELPER
select DRM_PANEL
select DRM_BRIDGE
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index af884ced7a0d..7f34263048a3 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -9,6 +9,7 @@
#include
#include
+#include "drm/drm_exec.h"
#include "drm/gpu_scheduler.h"
#include "msm_drv.h"
@@ -254,7 +255,7 @@ struct msm_gem_submit {
struct msm_gpu *gpu;
struct msm_gem_address_space *aspace;
struct list_head node; /* node in ring submit list */
- struct ww_acquire_ctx ticket;
+ struct drm_exec exec;
uint32_t seqno; /* Sequence number of the submit on the ring */
/* Hw fence, which is created when the scheduler executes the job, and
@@ -287,8 +288,6 @@ struct msm_gem_submit {
struct drm_msm_gem_submit_reloc *relocs;
} *cmd; /* array of size nr_cmds */
struct {
-/* make sure these don't conflict w/ MSM_SUBMIT_BO_x */
-#define BO_LOCKED 0x4000 /* obj lock is held */
uint32_t flags;
union {
struct drm_gem_object *obj;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 603f04d851d9..40878c26a749 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -248,85 +248,30 @@ static int submit_lookup_cmds(struct msm_gem_submit
*submit,
return ret;
}
-static void submit_unlock_bo(struct msm_gem_submit *submit, int i)
-{
- struct drm_gem_object *obj = submit->bos[i].obj;
- unsigned cleanup_flags = BO_LOCKED;
- unsigned flags = submit->bos[i].flags & cleanup_flags;
-
- /*
-* Clear flags bit before dropping lock, so that the msm_job_run()
-* path isn't racing with submit_cleanup() (ie. the read/modify/
-* write is protected by the obj lock in all paths)
-*/
- submit->bos[i].flags &= ~cleanup_flags;
-
- if (flags & BO_LOCKED)
- dma_resv_unlock(obj->resv);
-}
-
/* This is where we make sure all the bo's are reserved and pin'd: */
static int submit_lock_objects(struct msm_gem_submit *submit)
{
- int contended, slow_locked = -1, i, ret = 0;
-
-retry:
- for (i = 0; i < submit->nr_bos; i++) {
- struct drm_gem_object *obj = submit->bos[i].obj;
-
- if (slow_locked == i)
- slow_locked = -1;
+ int ret;
- contended = i;
+ drm_exec_init(&submit->exec, DRM_EXEC_INTERRUPTIBLE_WAIT,
submit->nr_bos);
- if (!(submit->bos[i].flags & BO_LOCKED)) {
- ret = dma_resv_lock_interruptible(obj->resv,
- &submit->ticket);
+ drm_exec_until_all_locked (&submit->exec) {
+ for (unsigned i = 0; i < submit->nr_bos; i++) {
+ struct drm_gem_object *obj = submit->bos[i].obj;
+ ret = drm_exec_prepare_obj(&submit->exec, obj, 1);
+ drm_exec_retry_on_contention(&submit->exec);
if (ret)
- goto fail;
- submit->bos[i].flags |= BO_LOCKED;
+ goto error;
}
}
- ww_acquire_done(&submit->ticket);
-
return 0;
-fail:
- if (ret == -EALREADY) {
- SUBMIT_ERROR(submit, "handle %u at index %u already on submit
list\n",
-submit->bos[i].handle, i);
- ret = -EINVAL;
- }
-
- for (; i >= 0; i--)
- submit_unlock_bo(submit, i);
-
- if (slow_locked > 0)
- submit_unlock_bo(submit, slow_locked);
-
- if (ret == -EDEADLK) {
- struct drm_gem_object *obj = submit->bos[contended].obj;
- /* we lost out in a seqno race, lock and retry.. */
- ret = dma_resv_lock_slow_interruptible(obj->resv,
- &submit->ticket);
- if (!ret) {
- submit->bos[contended].flag
[PATCH v2 6/7] drm/exec: Pass in initial # of objects
From: Rob Clark
In cases where the # is known ahead of time, it is silly to do the table
resize dance.
Signed-off-by: Rob Clark
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c | 4 ++--
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +-
drivers/gpu/drm/drm_exec.c | 13 ++---
drivers/gpu/drm/nouveau/nouveau_exec.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_uvmm.c | 2 +-
drivers/gpu/drm/tests/drm_exec_test.c| 16
include/drm/drm_exec.h | 2 +-
12 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 41fbc4fd0fac..0bd3c4a6267a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1137,7 +1137,7 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
ctx->n_vms = 1;
ctx->sync = &mem->sync;
- drm_exec_init(&ctx->exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_init(&ctx->exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
drm_exec_until_all_locked(&ctx->exec) {
ret = amdgpu_vm_lock_pd(vm, &ctx->exec, 2);
drm_exec_retry_on_contention(&ctx->exec);
@@ -1176,7 +1176,7 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
int ret;
ctx->sync = &mem->sync;
- drm_exec_init(&ctx->exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_init(&ctx->exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
drm_exec_until_all_locked(&ctx->exec) {
ctx->n_vms = 0;
list_for_each_entry(entry, &mem->attachments, list) {
@@ -2552,7 +2552,7 @@ static int validate_invalid_user_pages(struct
amdkfd_process_info *process_info)
amdgpu_sync_create(&sync);
- drm_exec_init(&exec, 0);
+ drm_exec_init(&exec, 0, 0);
/* Reserve all BOs and page tables for validation */
drm_exec_until_all_locked(&exec) {
/* Reserve all the page directories */
@@ -2793,7 +2793,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info,
struct dma_fence **ef)
mutex_lock(&process_info->lock);
- drm_exec_init(&exec, 0);
+ drm_exec_init(&exec, 0, 0);
drm_exec_until_all_locked(&exec) {
list_for_each_entry(peer_vm, &process_info->vm_list_head,
vm_list_node) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index df3ecfa9e13f..2464606494d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -66,7 +66,7 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p,
amdgpu_sync_create(&p->sync);
drm_exec_init(&p->exec, DRM_EXEC_INTERRUPTIBLE_WAIT |
- DRM_EXEC_IGNORE_DUPLICATES);
+ DRM_EXEC_IGNORE_DUPLICATES, 0);
return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index 720011019741..796fa6f1420b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -70,7 +70,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
struct drm_exec exec;
int r;
- drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
drm_exec_until_all_locked(&exec) {
r = amdgpu_vm_lock_pd(vm, &exec, 0);
if (likely(!r))
@@ -110,7 +110,7 @@ int amdgpu_unmap_static_csa(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
struct drm_exec exec;
int r;
- drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
drm_exec_until_all_locked(&exec) {
r = amdgpu_vm_lock_pd(vm, &exec, 0);
if (likely(!r))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 84beeaa4d21c..49a5f1c73b3e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -203,7 +203,7 @@ static void amdgpu_gem_object_close(struct drm_gem_object
*obj,
struct drm_exec exec;
long r;
- drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES);
+ drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 0);
drm_exec_until_all_locked(&exec) {
r = drm_exec_prepare_obj(&exec, &bo->tbo.base, 1);
drm_exec_retry_on_contention(&exec
[PATCH v2 5/7] drm/msm/gem: Cleanup submit_cleanup_bo()
From: Rob Clark
Now that it only handles unlock duty, drop the superfluous arg and
rename it.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem_submit.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index d001bf286606..603f04d851d9 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -248,14 +248,10 @@ static int submit_lookup_cmds(struct msm_gem_submit
*submit,
return ret;
}
-/* Unwind bo state, according to cleanup_flags. In the success case, only
- * the lock is dropped at the end of the submit (and active/pin ref is dropped
- * later when the submit is retired).
- */
-static void submit_cleanup_bo(struct msm_gem_submit *submit, int i,
- unsigned cleanup_flags)
+static void submit_unlock_bo(struct msm_gem_submit *submit, int i)
{
struct drm_gem_object *obj = submit->bos[i].obj;
+ unsigned cleanup_flags = BO_LOCKED;
unsigned flags = submit->bos[i].flags & cleanup_flags;
/*
@@ -304,10 +300,10 @@ static int submit_lock_objects(struct msm_gem_submit
*submit)
}
for (; i >= 0; i--)
- submit_cleanup_bo(submit, i, BO_LOCKED);
+ submit_unlock_bo(submit, i);
if (slow_locked > 0)
- submit_cleanup_bo(submit, slow_locked, BO_LOCKED);
+ submit_unlock_bo(submit, slow_locked);
if (ret == -EDEADLK) {
struct drm_gem_object *obj = submit->bos[contended].obj;
@@ -533,7 +529,6 @@ static int submit_reloc(struct msm_gem_submit *submit,
struct drm_gem_object *ob
*/
static void submit_cleanup(struct msm_gem_submit *submit, bool error)
{
- unsigned cleanup_flags = BO_LOCKED;
unsigned i;
if (error)
@@ -541,7 +536,7 @@ static void submit_cleanup(struct msm_gem_submit *submit,
bool error)
for (i = 0; i < submit->nr_bos; i++) {
struct drm_gem_object *obj = submit->bos[i].obj;
- submit_cleanup_bo(submit, i, cleanup_flags);
+ submit_unlock_bo(submit, i);
if (error)
drm_gem_object_put(obj);
}
--
2.42.0
[PATCH v2 1/7] drm/msm/gem: Remove "valid" tracking
From: Rob Clark
This was a small optimization for pre-soft-pin userspace. But mesa
switched to soft-pin nearly 5yrs ago. So lets drop the optimization
and simplify the code.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem.h| 2 --
drivers/gpu/drm/msm/msm_gem_submit.c | 44 +---
2 files changed, 8 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 8ddef5443140..c36c1c1fa222 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -271,7 +271,6 @@ struct msm_gem_submit {
struct msm_gpu_submitqueue *queue;
struct pid *pid;/* submitting process */
bool fault_dumped; /* Limit devcoredump dumping to one per submit */
- bool valid; /* true if no cmdstream patching needed */
bool in_rb; /* "sudo" mode, copy cmds into RB */
struct msm_ringbuffer *ring;
unsigned int nr_cmds;
@@ -288,7 +287,6 @@ struct msm_gem_submit {
} *cmd; /* array of size nr_cmds */
struct {
/* make sure these don't conflict w/ MSM_SUBMIT_BO_x */
-#define BO_VALID 0x8000 /* is current addr in cmdstream correct/valid?
*/
#define BO_LOCKED 0x4000 /* obj lock is held */
#define BO_PINNED 0x2000 /* obj (pages) is pinned and on active list */
uint32_t flags;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 6d8ec1337e8b..996274ef32a6 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -150,8 +150,6 @@ static int submit_lookup_objects(struct msm_gem_submit
*submit,
submit->bos[i].handle = submit_bo.handle;
submit->bos[i].flags = submit_bo.flags;
- /* in validate_objects() we figure out if this is true: */
- submit->bos[i].iova = submit_bo.presumed;
}
spin_lock(&file->table_lock);
@@ -278,9 +276,6 @@ static void submit_unlock_unpin_bo(struct msm_gem_submit
*submit, int i)
{
unsigned cleanup_flags = BO_PINNED | BO_LOCKED;
submit_cleanup_bo(submit, i, cleanup_flags);
-
- if (!(submit->bos[i].flags & BO_VALID))
- submit->bos[i].iova = 0;
}
/* This is where we make sure all the bo's are reserved and pin'd: */
@@ -390,8 +385,6 @@ static int submit_pin_objects(struct msm_gem_submit *submit)
struct msm_drm_private *priv = submit->dev->dev_private;
int i, ret = 0;
- submit->valid = true;
-
for (i = 0; i < submit->nr_bos; i++) {
struct drm_gem_object *obj = submit->bos[i].obj;
struct msm_gem_vma *vma;
@@ -407,14 +400,7 @@ static int submit_pin_objects(struct msm_gem_submit
*submit)
if (ret)
break;
- if (vma->iova == submit->bos[i].iova) {
- submit->bos[i].flags |= BO_VALID;
- } else {
- submit->bos[i].iova = vma->iova;
- /* iova changed, so address in cmdstream is not valid:
*/
- submit->bos[i].flags &= ~BO_VALID;
- submit->valid = false;
- }
+ submit->bos[i].iova = vma->iova;
}
/*
@@ -451,7 +437,7 @@ static void submit_attach_object_fences(struct
msm_gem_submit *submit)
}
static int submit_bo(struct msm_gem_submit *submit, uint32_t idx,
- struct drm_gem_object **obj, uint64_t *iova, bool *valid)
+ struct drm_gem_object **obj, uint64_t *iova)
{
if (idx >= submit->nr_bos) {
SUBMIT_ERROR(submit, "invalid buffer index: %u (out of %u)\n",
@@ -463,8 +449,6 @@ static int submit_bo(struct msm_gem_submit *submit,
uint32_t idx,
*obj = submit->bos[idx].obj;
if (iova)
*iova = submit->bos[idx].iova;
- if (valid)
- *valid = !!(submit->bos[idx].flags & BO_VALID);
return 0;
}
@@ -477,9 +461,6 @@ static int submit_reloc(struct msm_gem_submit *submit,
struct drm_gem_object *ob
uint32_t *ptr;
int ret = 0;
- if (!nr_relocs)
- return 0;
-
if (offset % 4) {
SUBMIT_ERROR(submit, "non-aligned cmdstream buffer: %u\n",
offset);
return -EINVAL;
@@ -500,7 +481,6 @@ static int submit_reloc(struct msm_gem_submit *submit,
struct drm_gem_object *ob
struct drm_msm_gem_submit_reloc submit_reloc = relocs[i];
uint32_t off;
uint64_t iova;
- bool valid;
if (submit_reloc.submit_offset % 4) {
SUBMIT_ERROR(submit, "non-aligned reloc offset: %u\n",
@@ -519,13 +499,10 @@ static int submit_reloc(struct msm_gem_submit *submit,
struct drm_gem_object *ob
goto out;
}
- ret = submit_bo
[PATCH v2 4/7] drm/msm/gem: Split out submit_unpin_objects() helper
From: Rob Clark
Untangle unpinning from unlock/unref loop. The unpin only happens in
error paths so it is easier to decouple from the normal unlock path.
Since we never have an intermediate state where a subset of buffers
are pinned (ie. we never bail out of the pin or unpin loops) we can
replace the bo state flag bit with a global flag in the submit.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem.h| 6 +++---
drivers/gpu/drm/msm/msm_gem_submit.c | 22 +-
drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++-
3 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index c36c1c1fa222..af884ced7a0d 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -270,8 +270,9 @@ struct msm_gem_submit {
int fence_id; /* key into queue->fence_idr */
struct msm_gpu_submitqueue *queue;
struct pid *pid;/* submitting process */
- bool fault_dumped; /* Limit devcoredump dumping to one per submit */
- bool in_rb; /* "sudo" mode, copy cmds into RB */
+ bool bos_pinned : 1;
+ bool fault_dumped:1;/* Limit devcoredump dumping to one per submit */
+ bool in_rb : 1; /* "sudo" mode, copy cmds into RB */
struct msm_ringbuffer *ring;
unsigned int nr_cmds;
unsigned int nr_bos;
@@ -288,7 +289,6 @@ struct msm_gem_submit {
struct {
/* make sure these don't conflict w/ MSM_SUBMIT_BO_x */
#define BO_LOCKED 0x4000 /* obj lock is held */
-#define BO_PINNED 0x2000 /* obj (pages) is pinned and on active list */
uint32_t flags;
union {
struct drm_gem_object *obj;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 786b48a55309..d001bf286606 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -265,9 +265,6 @@ static void submit_cleanup_bo(struct msm_gem_submit
*submit, int i,
*/
submit->bos[i].flags &= ~cleanup_flags;
- if (flags & BO_PINNED)
- msm_gem_unpin_locked(obj);
-
if (flags & BO_LOCKED)
dma_resv_unlock(obj->resv);
}
@@ -407,13 +404,28 @@ static int submit_pin_objects(struct msm_gem_submit
*submit)
mutex_lock(&priv->lru.lock);
for (i = 0; i < submit->nr_bos; i++) {
msm_gem_pin_obj_locked(submit->bos[i].obj);
- submit->bos[i].flags |= BO_PINNED;
}
mutex_unlock(&priv->lru.lock);
+ submit->bos_pinned = true;
+
return ret;
}
+static void submit_unpin_objects(struct msm_gem_submit *submit)
+{
+ if (!submit->bos_pinned)
+ return;
+
+ for (int i = 0; i < submit->nr_bos; i++) {
+ struct drm_gem_object *obj = submit->bos[i].obj;
+
+ msm_gem_unpin_locked(obj);
+ }
+
+ submit->bos_pinned = false;
+}
+
static void submit_attach_object_fences(struct msm_gem_submit *submit)
{
int i;
@@ -525,7 +537,7 @@ static void submit_cleanup(struct msm_gem_submit *submit,
bool error)
unsigned i;
if (error)
- cleanup_flags |= BO_PINNED;
+ submit_unpin_objects(submit);
for (i = 0; i < submit->nr_bos; i++) {
struct drm_gem_object *obj = submit->bos[i].obj;
diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c
b/drivers/gpu/drm/msm/msm_ringbuffer.c
index 929df7243792..bd125ca4d230 100644
--- a/drivers/gpu/drm/msm/msm_ringbuffer.c
+++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
@@ -29,9 +29,10 @@ static struct dma_fence *msm_job_run(struct drm_sched_job
*job)
struct drm_gem_object *obj = submit->bos[i].obj;
msm_gem_unpin_active(obj);
- submit->bos[i].flags &= ~BO_PINNED;
}
+ submit->bos_pinned = false;
+
mutex_unlock(&priv->lru.lock);
msm_gpu_submit(gpu, submit);
--
2.42.0
[PATCH v2 3/7] drm/msm/gem: Don't queue job to sched in error cases
From: Rob Clark We shouldn't be running the job in error cases. This also avoids having to think too hard about where the objs get unpinned (and if necessary, the resv takes over tracking that the obj is busy).. ie. error cases it always happens synchronously, and normal cases it happens from scheduler job_run() callback. Signed-off-by: Rob Clark Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/msm_gem_submit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 2d5527dc3e1a..786b48a55309 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -946,6 +946,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, } } + if (ret) + goto out; + submit_attach_object_fences(submit); /* The scheduler owns a ref now: */ -- 2.42.0
[PATCH v2 2/7] drm/msm/gem: Remove submit_unlock_unpin_bo()
From: Rob Clark
The only point it is called is before pinning objects, so the "unpin"
part of the name is fiction. Just remove it and call submit_cleanup_bo()
directly.
Signed-off-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/msm_gem_submit.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 996274ef32a6..2d5527dc3e1a 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -272,12 +272,6 @@ static void submit_cleanup_bo(struct msm_gem_submit
*submit, int i,
dma_resv_unlock(obj->resv);
}
-static void submit_unlock_unpin_bo(struct msm_gem_submit *submit, int i)
-{
- unsigned cleanup_flags = BO_PINNED | BO_LOCKED;
- submit_cleanup_bo(submit, i, cleanup_flags);
-}
-
/* This is where we make sure all the bo's are reserved and pin'd: */
static int submit_lock_objects(struct msm_gem_submit *submit)
{
@@ -313,10 +307,10 @@ static int submit_lock_objects(struct msm_gem_submit
*submit)
}
for (; i >= 0; i--)
- submit_unlock_unpin_bo(submit, i);
+ submit_cleanup_bo(submit, i, BO_LOCKED);
if (slow_locked > 0)
- submit_unlock_unpin_bo(submit, slow_locked);
+ submit_cleanup_bo(submit, slow_locked, BO_LOCKED);
if (ret == -EDEADLK) {
struct drm_gem_object *obj = submit->bos[contended].obj;
--
2.42.0
[PATCH v2 0/7] drm/msm/gem: drm_exec conversion
From: Rob Clark Simplify the exec path (removing a legacy optimization) and convert to drm_exec. One drm_exec patch to allow passing in the expected # of GEM objects to avoid re-allocation. I'd be a bit happier if I could avoid the extra objects table allocation in drm_exec in the first place, but wasn't really happy with any of the things I tried to get rid of that. v2: updates in 6/7 and other nit-addressing Rob Clark (7): drm/msm/gem: Remove "valid" tracking drm/msm/gem: Remove submit_unlock_unpin_bo() drm/msm/gem: Don't queue job to sched in error cases drm/msm/gem: Split out submit_unpin_objects() helper drm/msm/gem: Cleanup submit_cleanup_bo() drm/exec: Pass in initial # of objects drm/msm/gem: Convert to drm_exec .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c | 4 +- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +- drivers/gpu/drm/drm_exec.c| 13 +- drivers/gpu/drm/msm/Kconfig | 1 + drivers/gpu/drm/msm/msm_gem.h | 13 +- drivers/gpu/drm/msm/msm_gem_submit.c | 199 +- drivers/gpu/drm/msm/msm_ringbuffer.c | 3 +- drivers/gpu/drm/nouveau/nouveau_exec.c| 2 +- drivers/gpu/drm/nouveau/nouveau_uvmm.c| 2 +- drivers/gpu/drm/tests/drm_exec_test.c | 16 +- include/drm/drm_exec.h| 2 +- 16 files changed, 92 insertions(+), 187 deletions(-) -- 2.42.0
[GIT PULL] exynos-drm-fixes
Hi Dave and Daniel, Two fixups - fixing a potential error pointer dereference and wrong error checking. Ps. regarding the first patch, I had sent a GIT-PULL[1] but it seems you missed. [1] https://lore.kernel.org/dri-devel/20231006040950.4397-1-inki@samsung.com/T/#u Please kindly let me know if there is any problem. Thanks, Inki Dae The following changes since commit 98b1cc82c4affc16f5598d4fa14b1858671b2263: Linux 6.7-rc2 (2023-11-19 15:02:14 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos tags/exynos-drm-fixes-for-v6.7-rc3 for you to fetch changes up to a30ba4bd7cdb5726d86a557c5df8df71c7bc7fad: drm/exynos: fix a wrong error checking (2023-11-21 07:41:11 +0900) Two fixups - Fix a potential error pointer dereference by checking the return value of exynos_drm_crtc_get_by_type() function before accessing to crtc object. - Fix a wrong error checking in exynos_drm_dma.c modules, which was reported by Dan[1] [1] https://lore.kernel.org/all/33e52277-1349-472b-a55b-ab5c3462bfcf@moroto.mountain/ Inki Dae (1): drm/exynos: fix a wrong error checking Xiang Yang (1): drm/exynos: fix a potential error pointer dereference drivers/gpu/drm/exynos/exynos_drm_dma.c | 8 +++- drivers/gpu/drm/exynos/exynos_hdmi.c| 2 ++ 2 files changed, 5 insertions(+), 5 deletions(-)
Re: Radeon regression in 6.6 kernel
Alex Deucher writes: > Yes. Those changes went into 6.7 though, not 6.6 AFAIK. Maybe I'm > misunderstanding what the original report was actually testing. If it > was 6.7, then try reverting: > 56e449603f0ac580700621a356d35d5716a62ce5 > b70438004a14f4d0f9890b3297cd66248728546c I had been running v6.6-rc5 before pulling. It looks like that got me somewhere between v6.6 and v6.7-rc1. Reverting those two commits fixes it.
Re: Radeon regression in 6.6 kernel
Christian König writes: > Well none of the commits mentioned can affect radeon in any way. Radeon > simply doesn't use the scheduler. > > My suspicion is that the user is actually using amdgpu instead of > radeon. The switch potentially occurred accidentally, for example by > compiling amdgpu support for SI/CIK. Indeed, the lspci I originally posted does indicate amdgpu. What is the difference and should I switch it? If so, how? > Those amdgpu problems for older ASIC have already been worked on and > should be fixed by now. I just pulled v6.7-rc2 and it's still broken. I'll see if I can revert those 3 patches.
[PATCH v4 04/20] drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Jernej Skrabec Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c index d1a65a921..f5f62eb0e 100644 --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c @@ -302,7 +302,6 @@ int sun4i_hdmi_i2c_create(struct device *dev, struct sun4i_hdmi *hdmi) return -ENOMEM; adap->owner = THIS_MODULE; - adap->class = I2C_CLASS_DDC; adap->algo = &sun4i_hdmi_i2c_algorithm; strscpy(adap->name, "sun4i_hdmi_i2c adapter", sizeof(adap->name)); i2c_set_adapdata(adap, hdmi);
[PATCH v4 05/20] drivers/video/fbdev: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in
olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC.
Class-based device auto-detection is a legacy mechanism and shouldn't
be used in new code. So we can remove this class completely now.
Preferably this series should be applied via the i2c tree.
Acked-by: Helge Deller
Signed-off-by: Heiner Kallweit
---
v3:
- fix compile error
---
drivers/video/fbdev/i740fb.c | 1 -
drivers/video/fbdev/matrox/i2c-matroxfb.c | 15 +--
drivers/video/fbdev/s3fb.c| 1 -
drivers/video/fbdev/tdfxfb.c | 1 -
drivers/video/fbdev/tridentfb.c | 1 -
5 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/video/fbdev/i740fb.c b/drivers/video/fbdev/i740fb.c
index 1897e65ab..9b74dae71 100644
--- a/drivers/video/fbdev/i740fb.c
+++ b/drivers/video/fbdev/i740fb.c
@@ -163,7 +163,6 @@ static int i740fb_setup_ddc_bus(struct fb_info *info)
strscpy(par->ddc_adapter.name, info->fix.id,
sizeof(par->ddc_adapter.name));
par->ddc_adapter.owner = THIS_MODULE;
- par->ddc_adapter.class = I2C_CLASS_DDC;
par->ddc_adapter.algo_data = &par->ddc_algo;
par->ddc_adapter.dev.parent = info->device;
par->ddc_algo.setsda= i740fb_ddc_setsda;
diff --git a/drivers/video/fbdev/matrox/i2c-matroxfb.c
b/drivers/video/fbdev/matrox/i2c-matroxfb.c
index e2e4705e3..bb048e14b 100644
--- a/drivers/video/fbdev/matrox/i2c-matroxfb.c
+++ b/drivers/video/fbdev/matrox/i2c-matroxfb.c
@@ -100,8 +100,7 @@ static const struct i2c_algo_bit_data
matrox_i2c_algo_template =
};
static int i2c_bus_reg(struct i2c_bit_adapter* b, struct matrox_fb_info*
minfo,
- unsigned int data, unsigned int clock, const char *name,
- int class)
+ unsigned int data, unsigned int clock, const char *name)
{
int err;
@@ -112,7 +111,6 @@ static int i2c_bus_reg(struct i2c_bit_adapter* b, struct
matrox_fb_info* minfo,
snprintf(b->adapter.name, sizeof(b->adapter.name), name,
minfo->fbcon.node);
i2c_set_adapdata(&b->adapter, b);
- b->adapter.class = class;
b->adapter.algo_data = &b->bac;
b->adapter.dev.parent = &minfo->pcidev->dev;
b->bac = matrox_i2c_algo_template;
@@ -160,27 +158,24 @@ static void* i2c_matroxfb_probe(struct matrox_fb_info*
minfo) {
case MGA_2164:
err = i2c_bus_reg(&m2info->ddc1, minfo,
DDC1B_DATA, DDC1B_CLK,
- "DDC:fb%u #0", I2C_CLASS_DDC);
+ "DDC:fb%u #0");
break;
default:
err = i2c_bus_reg(&m2info->ddc1, minfo,
DDC1_DATA, DDC1_CLK,
- "DDC:fb%u #0", I2C_CLASS_DDC);
+ "DDC:fb%u #0");
break;
}
if (err)
goto fail_ddc1;
if (minfo->devflags.dualhead) {
- err = i2c_bus_reg(&m2info->ddc2, minfo,
- DDC2_DATA, DDC2_CLK,
- "DDC:fb%u #1", I2C_CLASS_DDC);
+ err = i2c_bus_reg(&m2info->ddc2, minfo, DDC2_DATA, DDC2_CLK,
"DDC:fb%u #1");
if (err == -ENODEV) {
printk(KERN_INFO "i2c-matroxfb: VGA->TV plug detected,
DDC unavailable.\n");
} else if (err)
printk(KERN_INFO "i2c-matroxfb: Could not register
secondary output i2c bus. Continuing anyway.\n");
/* Register maven bus even on G450/G550 */
- err = i2c_bus_reg(&m2info->maven, minfo,
- MAT_DATA, MAT_CLK, "MAVEN:fb%u", 0);
+ err = i2c_bus_reg(&m2info->maven, minfo, MAT_DATA, MAT_CLK,
"MAVEN:fb%u");
if (err)
printk(KERN_INFO "i2c-matroxfb: Could not register
Maven i2c bus. Continuing anyway.\n");
else {
diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index 589b349cb..07722a5ea 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -252,7 +252,6 @@ static int s3fb_setup_ddc_bus(struct fb_info *info)
strscpy(par->ddc_adapter.name, info->fix.id,
sizeof(par->ddc_adapter.name));
par->ddc_adapter.owner = THIS_MODULE;
- par->ddc_adapter.class = I2C_CLASS_DDC;
par->ddc_adapter.algo_data = &par->ddc_algo;
par->ddc_adapter.dev.parent = info->device;
par->ddc_algo.setsda= s3fb_ddc_setsda;
diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
index 22aa95313..51ebe7835 100644
--- a/drivers/video/fbdev/tdfxfb.c
+++
[PATCH v4 13/20] drivers/video/fbdev/intelfb/intelfb_i2c.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in
olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC.
Class-based device auto-detection is a legacy mechanism and shouldn't
be used in new code. So we can remove this class completely now.
Preferably this series should be applied via the i2c tree.
Acked-by: Helge Deller
Signed-off-by: Heiner Kallweit
---
drivers/video/fbdev/intelfb/intelfb_i2c.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/video/fbdev/intelfb/intelfb_i2c.c
b/drivers/video/fbdev/intelfb/intelfb_i2c.c
index 3300bd31d..f24c7cb4c 100644
--- a/drivers/video/fbdev/intelfb/intelfb_i2c.c
+++ b/drivers/video/fbdev/intelfb/intelfb_i2c.c
@@ -99,8 +99,7 @@ static int intelfb_gpio_getsda(void *data)
static int intelfb_setup_i2c_bus(struct intelfb_info *dinfo,
struct intelfb_i2c_chan *chan,
-const u32 reg, const char *name,
-int class)
+const u32 reg, const char *name)
{
int rc;
@@ -108,7 +107,6 @@ static int intelfb_setup_i2c_bus(struct intelfb_info *dinfo,
chan->reg = reg;
snprintf(chan->adapter.name, sizeof(chan->adapter.name),
"intelfb %s", name);
- chan->adapter.class = class;
chan->adapter.owner = THIS_MODULE;
chan->adapter.algo_data = &chan->algo;
chan->adapter.dev.parent= &chan->dinfo->pdev->dev;
@@ -144,8 +142,7 @@ void intelfb_create_i2c_busses(struct intelfb_info *dinfo)
dinfo->output[i].type = INTELFB_OUTPUT_ANALOG;
/* setup the DDC bus for analog output */
- intelfb_setup_i2c_bus(dinfo, &dinfo->output[i].ddc_bus, GPIOA,
- "CRTDDC_A", I2C_CLASS_DDC);
+ intelfb_setup_i2c_bus(dinfo, &dinfo->output[i].ddc_bus, GPIOA,
"CRTDDC_A");
i++;
/* need to add the output busses for each device
@@ -159,10 +156,8 @@ void intelfb_create_i2c_busses(struct intelfb_info *dinfo)
case INTEL_855GM:
case INTEL_865G:
dinfo->output[i].type = INTELFB_OUTPUT_DVO;
- intelfb_setup_i2c_bus(dinfo, &dinfo->output[i].ddc_bus,
- GPIOD, "DVODDC_D", I2C_CLASS_DDC);
- intelfb_setup_i2c_bus(dinfo, &dinfo->output[i].i2c_bus,
- GPIOE, "DVOI2C_E", 0);
+ intelfb_setup_i2c_bus(dinfo, &dinfo->output[i].ddc_bus, GPIOD,
"DVODDC_D");
+ intelfb_setup_i2c_bus(dinfo, &dinfo->output[i].i2c_bus, GPIOE,
"DVOI2C_E");
i++;
break;
case INTEL_915G:
@@ -176,7 +171,7 @@ void intelfb_create_i2c_busses(struct intelfb_info *dinfo)
/* SDVO ports have a single control bus - 2 devices */
dinfo->output[i].type = INTELFB_OUTPUT_SDVO;
intelfb_setup_i2c_bus(dinfo, &dinfo->output[i].i2c_bus,
- GPIOE, "SDVOCTRL_E", 0);
+ GPIOE, "SDVOCTRL_E");
/* TODO: initialize the SDVO */
/* I830SDVOInit(pScrn, i, DVOB); */
i++;
[PATCH v4 11/20] drivers/gpu/drm/bridge/synopsys/dw-hdmi.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Reviewed-by: Neil Armstrong Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 52d91a0df..aca5bb086 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -515,7 +515,6 @@ static struct i2c_adapter *dw_hdmi_i2c_adapter(struct dw_hdmi *hdmi) init_completion(&i2c->cmp); adap = &i2c->adap; - adap->class = I2C_CLASS_DDC; adap->owner = THIS_MODULE; adap->dev.parent = hdmi->dev; adap->algo = &dw_hdmi_algorithm;
[PATCH v4 19/20] drivers/gpu/drm/display: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Alex Deucher Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/display/drm_dp_helper.c |1 - drivers/gpu/drm/display/drm_dp_mst_topology.c |1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index f3680f4e6..ac901f4b4 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -2102,7 +2102,6 @@ int drm_dp_aux_register(struct drm_dp_aux *aux) if (!aux->ddc.algo) drm_dp_aux_init(aux); - aux->ddc.class = I2C_CLASS_DDC; aux->ddc.owner = THIS_MODULE; aux->ddc.dev.parent = aux->dev; diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 0e0d0e76d..4376e2c1f 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -5803,7 +5803,6 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port) aux->ddc.algo_data = aux; aux->ddc.retries = 3; - aux->ddc.class = I2C_CLASS_DDC; aux->ddc.owner = THIS_MODULE; /* FIXME: set the kdev of the port's connector as parent */ aux->ddc.dev.parent = parent_dev;
[PATCH v4 06/20] drivers/video/fbdev/core/fb_ddc.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Helge Deller Signed-off-by: Heiner Kallweit --- drivers/video/fbdev/core/fb_ddc.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/fbdev/core/fb_ddc.c b/drivers/video/fbdev/core/fb_ddc.c index 8bf5f2f54..e25143219 100644 --- a/drivers/video/fbdev/core/fb_ddc.c +++ b/drivers/video/fbdev/core/fb_ddc.c @@ -116,7 +116,6 @@ unsigned char *fb_ddc_read(struct i2c_adapter *adapter) algo_data->setsda(algo_data->data, 1); algo_data->setscl(algo_data->data, 1); - adapter->class |= I2C_CLASS_DDC; return edid; }
[PATCH v4 15/20] drivers/gpu/drm/i915/display: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Jani Nikula Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/i915/display/intel_gmbus.c |1 - drivers/gpu/drm/i915/display/intel_sdvo.c |1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 40d7b6f3f..e9e4dcf34 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -899,7 +899,6 @@ int intel_gmbus_setup(struct drm_i915_private *i915) } bus->adapter.owner = THIS_MODULE; - bus->adapter.class = I2C_CLASS_DDC; snprintf(bus->adapter.name, sizeof(bus->adapter.name), "i915 gmbus %s", gmbus_pin->name); diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index a636f42ce..5e64d1baf 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -3311,7 +3311,6 @@ intel_sdvo_init_ddc_proxy(struct intel_sdvo_ddc *ddc, ddc->ddc_bus = ddc_bus; ddc->ddc.owner = THIS_MODULE; - ddc->ddc.class = I2C_CLASS_DDC; snprintf(ddc->ddc.name, I2C_NAME_SIZE, "SDVO %c DDC%d", port_name(sdvo->base.port), ddc_bus); ddc->ddc.dev.parent = &pdev->dev;
[PATCH v4 14/20] drivers/gpu/drm/msm/hdmi/hdmi_i2c.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Dmitry Baryshkov Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/msm/hdmi/hdmi_i2c.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c b/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c index de182c004..7aa500d24 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c @@ -249,7 +249,6 @@ struct i2c_adapter *msm_hdmi_i2c_init(struct hdmi *hdmi) i2c->owner = THIS_MODULE; - i2c->class = I2C_CLASS_DDC; snprintf(i2c->name, sizeof(i2c->name), "msm hdmi i2c"); i2c->dev.parent = &hdmi->pdev->dev; i2c->algo = &msm_hdmi_i2c_algorithm;
[PATCH v4 12/20] drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c b/drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c index d675c954b..54e46e440 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c @@ -297,7 +297,6 @@ static int mtk_hdmi_ddc_probe(struct platform_device *pdev) strscpy(ddc->adap.name, "mediatek-hdmi-ddc", sizeof(ddc->adap.name)); ddc->adap.owner = THIS_MODULE; - ddc->adap.class = I2C_CLASS_DDC; ddc->adap.algo = &mtk_hdmi_ddc_algorithm; ddc->adap.retries = 3; ddc->adap.dev.of_node = dev->of_node;
[PATCH v4 16/20] drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in
olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC.
Class-based device auto-detection is a legacy mechanism and shouldn't
be used in new code. So we can remove this class completely now.
Preferably this series should be applied via the i2c tree.
Signed-off-by: Heiner Kallweit
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c |1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
index 410bd019b..e6e48651c 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
@@ -81,7 +81,6 @@ int hibmc_ddc_create(struct drm_device *drm_dev,
struct hibmc_connector *connector)
{
connector->adapter.owner = THIS_MODULE;
- connector->adapter.class = I2C_CLASS_DDC;
snprintf(connector->adapter.name, I2C_NAME_SIZE, "HIS i2c bit bus");
connector->adapter.dev.parent = drm_dev->dev;
i2c_set_adapdata(&connector->adapter, connector);
[PATCH v4 18/20] drivers/gpu/drm/gma500: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in
olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC.
Class-based device auto-detection is a legacy mechanism and shouldn't
be used in new code. So we can remove this class completely now.
Preferably this series should be applied via the i2c tree.
Signed-off-by: Heiner Kallweit
---
drivers/gpu/drm/gma500/cdv_intel_dp.c |1 -
drivers/gpu/drm/gma500/intel_gmbus.c |1 -
drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c |1 -
drivers/gpu/drm/gma500/psb_intel_sdvo.c|1 -
4 files changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c
b/drivers/gpu/drm/gma500/cdv_intel_dp.c
index 8992a9507..dd1eb7e98 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
@@ -855,7 +855,6 @@ cdv_intel_dp_i2c_init(struct gma_connector *connector,
memset(&intel_dp->adapter, '\0', sizeof (intel_dp->adapter));
intel_dp->adapter.owner = THIS_MODULE;
- intel_dp->adapter.class = I2C_CLASS_DDC;
strncpy (intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) -
1);
intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0';
intel_dp->adapter.algo_data = &intel_dp->algo;
diff --git a/drivers/gpu/drm/gma500/intel_gmbus.c
b/drivers/gpu/drm/gma500/intel_gmbus.c
index 09cedabf4..aa4550985 100644
--- a/drivers/gpu/drm/gma500/intel_gmbus.c
+++ b/drivers/gpu/drm/gma500/intel_gmbus.c
@@ -411,7 +411,6 @@ int gma_intel_setup_gmbus(struct drm_device *dev)
struct intel_gmbus *bus = &dev_priv->gmbus[i];
bus->adapter.owner = THIS_MODULE;
- bus->adapter.class = I2C_CLASS_DDC;
snprintf(bus->adapter.name,
sizeof(bus->adapter.name),
"gma500 gmbus %s",
diff --git a/drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c
b/drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c
index fc9a34ed5..6daa6669e 100644
--- a/drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c
+++ b/drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c
@@ -168,7 +168,6 @@ static struct i2c_adapter oaktrail_hdmi_i2c_adapter = {
.name = "oaktrail_hdmi_i2c",
.nr = 3,
.owner = THIS_MODULE,
- .class = I2C_CLASS_DDC,
.algo = &oaktrail_hdmi_i2c_algorithm,
};
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index d6fd5d726..e4f914dec 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -2426,7 +2426,6 @@ psb_intel_sdvo_init_ddc_proxy(struct psb_intel_sdvo *sdvo,
struct drm_device *dev)
{
sdvo->ddc.owner = THIS_MODULE;
- sdvo->ddc.class = I2C_CLASS_DDC;
snprintf(sdvo->ddc.name, I2C_NAME_SIZE, "SDVO DDC proxy");
sdvo->ddc.dev.parent = dev->dev;
sdvo->ddc.algo_data = sdvo;
[PATCH v4 03/20] drm/amd/display: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Harry Wentland Acked-by: Alex Deucher Signed-off-by: Heiner Kallweit --- v2: - adjust tag in commit subject --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 6f99f6754..ae1edc6ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7529,7 +7529,6 @@ create_i2c(struct ddc_service *ddc_service, if (!i2c) return NULL; i2c->base.owner = THIS_MODULE; - i2c->base.class = I2C_CLASS_DDC; i2c->base.dev.parent = &adev->pdev->dev; i2c->base.algo = &amdgpu_dm_i2c_algo; snprintf(i2c->base.name, sizeof(i2c->base.name), "AMDGPU DM i2c hw bus %d", link_index);
[PATCH v4 07/20] drivers/gpu/drm: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Alex Deucher Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c |1 - drivers/gpu/drm/radeon/radeon_i2c.c |1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c index 82608df43..d79cb13e1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c @@ -175,7 +175,6 @@ struct amdgpu_i2c_chan *amdgpu_i2c_create(struct drm_device *dev, i2c->rec = *rec; i2c->adapter.owner = THIS_MODULE; - i2c->adapter.class = I2C_CLASS_DDC; i2c->adapter.dev.parent = dev->dev; i2c->dev = dev; i2c_set_adapdata(&i2c->adapter, i2c); diff --git a/drivers/gpu/drm/radeon/radeon_i2c.c b/drivers/gpu/drm/radeon/radeon_i2c.c index 314d066e6..3d174390a 100644 --- a/drivers/gpu/drm/radeon/radeon_i2c.c +++ b/drivers/gpu/drm/radeon/radeon_i2c.c @@ -918,7 +918,6 @@ struct radeon_i2c_chan *radeon_i2c_create(struct drm_device *dev, i2c->rec = *rec; i2c->adapter.owner = THIS_MODULE; - i2c->adapter.class = I2C_CLASS_DDC; i2c->adapter.dev.parent = dev->dev; i2c->dev = dev; i2c_set_adapdata(&i2c->adapter, i2c);
[PATCH v4 02/20] drivers/gpu/drm/mgag200/mgag200_i2c.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Reviewed-by: Thomas Zimmermann Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/mgag200/mgag200_i2c.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_i2c.c b/drivers/gpu/drm/mgag200/mgag200_i2c.c index 0c48bdf3e..423eb302b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_i2c.c +++ b/drivers/gpu/drm/mgag200/mgag200_i2c.c @@ -106,7 +106,6 @@ int mgag200_i2c_init(struct mga_device *mdev, struct mga_i2c_chan *i2c) i2c->data = BIT(info->i2c.data_bit); i2c->clock = BIT(info->i2c.clock_bit); i2c->adapter.owner = THIS_MODULE; - i2c->adapter.class = I2C_CLASS_DDC; i2c->adapter.dev.parent = dev->dev; i2c->dev = dev; i2c_set_adapdata(&i2c->adapter, i2c);
[PATCH v4 17/20] drivers/gpu/drm/ast/ast_i2c.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Reviewed-by: Thomas Zimmermann Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/ast/ast_i2c.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/ast/ast_i2c.c b/drivers/gpu/drm/ast/ast_i2c.c index 0e845e7ac..e5d3f7121 100644 --- a/drivers/gpu/drm/ast/ast_i2c.c +++ b/drivers/gpu/drm/ast/ast_i2c.c @@ -120,7 +120,6 @@ struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev) return NULL; i2c->adapter.owner = THIS_MODULE; - i2c->adapter.class = I2C_CLASS_DDC; i2c->adapter.dev.parent = dev->dev; i2c->dev = dev; i2c_set_adapdata(&i2c->adapter, i2c);
[PATCH v4 10/20] drivers/video/fbdev/cyber2000fb.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Helge Deller Signed-off-by: Heiner Kallweit --- drivers/video/fbdev/cyber2000fb.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/fbdev/cyber2000fb.c b/drivers/video/fbdev/cyber2000fb.c index 52105dc1a..79775deda 100644 --- a/drivers/video/fbdev/cyber2000fb.c +++ b/drivers/video/fbdev/cyber2000fb.c @@ -1234,7 +1234,6 @@ static int cyber2000fb_setup_ddc_bus(struct cfb_info *cfb) strscpy(cfb->ddc_adapter.name, cfb->fb.fix.id, sizeof(cfb->ddc_adapter.name)); cfb->ddc_adapter.owner = THIS_MODULE; - cfb->ddc_adapter.class = I2C_CLASS_DDC; cfb->ddc_adapter.algo_data = &cfb->ddc_algo; cfb->ddc_adapter.dev.parent = cfb->fb.device; cfb->ddc_algo.setsda= cyber2000fb_ddc_setsda;
[PATCH v4 08/20] drivers/gpu/drm/loongson/lsdc_i2c.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/loongson/lsdc_i2c.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/loongson/lsdc_i2c.c b/drivers/gpu/drm/loongson/lsdc_i2c.c index 9625d0b1d..ce90c2553 100644 --- a/drivers/gpu/drm/loongson/lsdc_i2c.c +++ b/drivers/gpu/drm/loongson/lsdc_i2c.c @@ -154,7 +154,6 @@ int lsdc_create_i2c_chan(struct drm_device *ddev, adapter = &li2c->adapter; adapter->algo_data = &li2c->bit; adapter->owner = THIS_MODULE; - adapter->class = I2C_CLASS_DDC; adapter->dev.parent = ddev->dev; adapter->nr = -1;
[PATCH v4 09/20] drivers/video/fbdev/via/via_i2c.c: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Helge Deller Signed-off-by: Heiner Kallweit --- drivers/video/fbdev/via/via_i2c.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/fbdev/via/via_i2c.c b/drivers/video/fbdev/via/via_i2c.c index c35e530e0..582502810 100644 --- a/drivers/video/fbdev/via/via_i2c.c +++ b/drivers/video/fbdev/via/via_i2c.c @@ -201,7 +201,6 @@ static int create_i2c_bus(struct i2c_adapter *adapter, sprintf(adapter->name, "viafb i2c io_port idx 0x%02x", adap_cfg->ioport_index); adapter->owner = THIS_MODULE; - adapter->class = I2C_CLASS_DDC; adapter->algo_data = algo; if (pdev) adapter->dev.parent = &pdev->dev;
[PATCH v4 00/20] remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. v2: - change tag in commit subject of patch 03 - add ack tags v3: - fix a compile error in patch 5 v4: - more ack and review tags Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c |1 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |1 - drivers/gpu/drm/ast/ast_i2c.c |1 - drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |1 - drivers/gpu/drm/display/drm_dp_helper.c |1 - drivers/gpu/drm/display/drm_dp_mst_topology.c |1 - drivers/gpu/drm/gma500/cdv_intel_dp.c |1 - drivers/gpu/drm/gma500/intel_gmbus.c |1 - drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c|1 - drivers/gpu/drm/gma500/psb_intel_sdvo.c |1 - drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c |1 - drivers/gpu/drm/i915/display/intel_gmbus.c|1 - drivers/gpu/drm/i915/display/intel_sdvo.c |1 - drivers/gpu/drm/loongson/lsdc_i2c.c |1 - drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c |1 - drivers/gpu/drm/mgag200/mgag200_i2c.c |1 - drivers/gpu/drm/msm/hdmi/hdmi_i2c.c |1 - drivers/gpu/drm/radeon/radeon_i2c.c |1 - drivers/gpu/drm/rockchip/inno_hdmi.c |1 - drivers/gpu/drm/rockchip/rk3066_hdmi.c|1 - drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c|1 - drivers/video/fbdev/core/fb_ddc.c |1 - drivers/video/fbdev/cyber2000fb.c |1 - drivers/video/fbdev/i740fb.c |1 - drivers/video/fbdev/intelfb/intelfb_i2c.c | 15 +-- drivers/video/fbdev/matrox/i2c-matroxfb.c | 12 drivers/video/fbdev/s3fb.c|1 - drivers/video/fbdev/tdfxfb.c |1 - drivers/video/fbdev/tridentfb.c |1 - drivers/video/fbdev/via/via_i2c.c |1 - include/linux/i2c.h |1 - 31 files changed, 9 insertions(+), 47 deletions(-)
[PATCH v4 01/20] drivers/gpu/drm/rockchip: remove I2C_CLASS_DDC support
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Acked-by: Heiko Stuebner Signed-off-by: Heiner Kallweit --- drivers/gpu/drm/rockchip/inno_hdmi.c |1 - drivers/gpu/drm/rockchip/rk3066_hdmi.c |1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c index 6e5b922a1..a7739b27c 100644 --- a/drivers/gpu/drm/rockchip/inno_hdmi.c +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c @@ -793,7 +793,6 @@ static struct i2c_adapter *inno_hdmi_i2c_adapter(struct inno_hdmi *hdmi) init_completion(&i2c->cmp); adap = &i2c->adap; - adap->class = I2C_CLASS_DDC; adap->owner = THIS_MODULE; adap->dev.parent = hdmi->dev; adap->dev.of_node = hdmi->dev->of_node; diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c index fa6e592e0..7a3f71aa2 100644 --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c @@ -725,7 +725,6 @@ static struct i2c_adapter *rk3066_hdmi_i2c_adapter(struct rk3066_hdmi *hdmi) init_completion(&i2c->cmpltn); adap = &i2c->adap; - adap->class = I2C_CLASS_DDC; adap->owner = THIS_MODULE; adap->dev.parent = hdmi->dev; adap->dev.of_node = hdmi->dev->of_node;
Re: Implement per-key keyboard backlight as auxdisplay?
On Mon 2023-10-23 13:44:46, Miguel Ojeda wrote: > On Mon, Oct 23, 2023 at 1:40 PM Jani Nikula > wrote: > > > > One could also reasonably make the argument that controlling the > > individual keyboard key backlights should be part of the input > > subsystem. It's not a display per se. (Unless you actually have small > > displays on the keycaps, and I think that's a thing too.) > > > > There's force feedback, there could be light feedback? There's also > > drivers/input/input-leds.c for the keycaps that have leds, like caps > > lock, num lock, etc. > > > > Anyway, just throwing ideas around, no strong opinions, really. > > Yeah, sounds quite reasonable too, in fact it may make more sense > there given the LEDs are associated per-key rather than being an > uniform matrix in a rectangle if I understand correctly. If the input > subsystem wants to take it, that would be great. Unfortunately we are getting no input from input subsystem. Question seems to be more of "is auxdisplay willing to take it if it is done properly"? Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Implement per-key keyboard backlight as auxdisplay?
Hi!
> >> So... a bit of rationale. The keyboard does not really fit into the
> >> LED subsystem; LEDs are expected to be independent ("hdd led") and not
> >> a matrix of them.
> >
> > Makes sense.
> >
> >> We do see various strange displays these days -- they commonly have
> >> rounded corners and holes in them. I'm not sure how that's currently
> >> supported, but I believe it is reasonable to view keyboard as a
> >> display with slightly weird placing of pixels.
> >>
> >> Plus, I'd really like to play tetris on one of those :-).
> >>
> >> So, would presenting them as auxdisplay be acceptable? Or are there
> >> better options?
> >
> > It sounds like a fair use case -- auxdisplay are typically simple
> > character-based or small graphical displays, e.g. 128x64, that may not
> > be a "main" / usual screen as typically understood, but the concept is
> > a bit fuzzy and we are a bit of a catch-all.
> >
> > And "keyboard backlight display with a pixel/color per-key" does not
> > sound like a "main" screen, and having some cute effects displayed
> > there are the kind of thing that one could do in the usual small
> > graphical ones too. :)
> >
> > But if somebody prefers to create new categories (or subcategories
> > within auxdisplay) to hold these, that could be nice too (in the
> > latter case, I would perhaps suggest reorganizing all of the existing
> > ones while at it).
>
> One could also reasonably make the argument that controlling the
> individual keyboard key backlights should be part of the input
> subsystem. It's not a display per se. (Unless you actually have small
> displays on the keycaps, and I think that's a thing too.)
While it would not be completely crazy to do that... I believe the
backlight is more of a display and less of a keyboard. Plus input
subystem is very far away from supporting this, and we had no input
from input people here.
I don't think LED subsystem is right place for this, and I believe
auxdisplay makes slightly more sense than input.
Unless someone steps up, I'd suggest Werner tries to implement this as
an auxdisplay. [And yes, this will not be simple task. RGB on LED is
different from RGB on display. But there are other LED displays, so
auxdisplay should handle this. Plus pixels are really funnily
shaped. But displays with missing pixels -- aka holes for camera --
are common in phones, and I believe we'll get variable pixel densities
-- less dense over camera -- too. So displays will have to deal with
these in the end.]
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
[PATCH] drm/mediatek: dp: Add phy_mtk_dp module as pre-dependency
The mtk_dp driver registers a phy device which is handled by the
phy_mtk_dp driver and assumes that the phy probe will complete
synchronously, proceeding to make use of functionality exposed by that
driver right away. This assumption however is false when the phy driver
is built as a module, causing the mtk_dp driver to fail probe in this
case.
Add the phy_mtk_dp module as a pre-dependency to the mtk_dp module to
ensure the phy module has been loaded before the dp, so that the phy
probe happens synchrounously and the mtk_dp driver can probe
successfully even with the phy driver built as a module.
Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
Signed-off-by: Nícolas F. R. A. Prado
---
drivers/gpu/drm/mediatek/mtk_dp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
b/drivers/gpu/drm/mediatek/mtk_dp.c
index e4c16ba9902d..2136a596efa1 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2818,3 +2818,4 @@ MODULE_AUTHOR("Markus Schneider-Pargmann
");
MODULE_AUTHOR("Bo-Chen Chen ");
MODULE_DESCRIPTION("MediaTek DisplayPort Driver");
MODULE_LICENSE("GPL");
+MODULE_SOFTDEP("pre: phy_mtk_dp");
--
2.42.1
Re: [PATCH 1/3] dt-bindings: display: ssd1307fb: Change "solomon,page-offset" default value
Conor Dooley writes: Hello Connor, > On Thu, Nov 16, 2023 at 07:07:37PM +0100, Javier Martinez Canillas wrote: >> This is used to specify the page start address offset of the display RAM. >> >> The value is used as offset when setting the page start address with the >> SSD130X_SET_PAGE_RANGE command, and the driver currently sets its value to >> 1 if the property is not present in the Device Tree. >> >> But the datasheet mentions that the value on reset for the page start is a >> 0, so it makes more sense to also have 0 as the default value for the page >> offset if the property is not present. >> >> In fact, using a default value of 1 leads to the display not working when >> the fbdev is attached to the framebuffer console. >> >> Reported-by: Sahaj Sarup >> Signed-off-by: Javier Martinez Canillas >> --- >> >> .../devicetree/bindings/display/solomon,ssd1307fb.yaml | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git >> a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml >> b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml >> index 3afbb52d1b7f..badd81459aaa 100644 >> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml >> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml >> @@ -35,7 +35,7 @@ properties: >> >>solomon,page-offset: >> $ref: /schemas/types.yaml#/definitions/uint32 >> -default: 1 >> +default: 0 > > I think I saw it pointed out by Maxime elsewhere that this breaks the > ABI. It would be nice if DT defaults matched the hardware's, but I don't > really think it is worth breaking the ABI here. Expanding the property Yes, Maxime also agrees with you as you mentioned. It's fair to say that this may affect potential users even when I honestly think that's unlikely. I'll just discard these patches and point out to people reporting the same problem than Sahaj, that they need to add a "solomon,page-offset" property. > description to explain the impact of the particular values might help > with incorrect usage. > I'm unsure how much that would help to be honest but I might post a patch. > Thanks, > Conor. > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH V3] drm/panel: boe-tv101wum-nl6: Fine tune Himax83102-j02 panel HFP and HBP
Hi,
On Sun, Nov 19, 2023 at 6:01 PM Cong Yang
wrote:
>
> The refresh reported by modetest is 60.46Hz, and the actual measurement
> is 60.01Hz, which is outside the expected tolerance. Adjust hporch and
> pixel clock to fix it. After repair, modetest and actual measurement were
> all 60.01Hz.
>
> Modetest refresh = Pixel CLK/ htotal* vtotal, but measurement frame rate
> is HS->LP cycle time(Vblanking). Measured frame rate is not only affecte
> by Htotal/Vtotal/pixel clock, also affected by Lane-num/PixelBit/LineTime
> /DSI CLK. Assume that the DSI controller could not make the mode that we
> requested(presumably it's PLL couldn't generate the exact pixel clock?).
> If you use a different DSI controller, you may need to readjust these
> parameters. Now this panel looks like it's only used by me on the MTK
> platform, so let's change this set of parameters.
>
> Fixes: 1bc2ef065f13 ("drm/panel: Support for Starry-himax83102-j02 TDDI
> MIPI-DSI panel")
> Signed-off-by: Cong Yang
> Reviewed-by: Douglas Anderson
> ---
> Chage since V2:
>
> - Update commit message.
>
> V2:
> https://lore.kernel.org/all/20231117032500.2923624-1-yangco...@huaqin.corp-partner.google.com
>
> Chage since V1:
>
> - Update commit message.
>
> V1:
> https://lore.kernel.org/all/20231110094553.2361842-1-yangco...@huaqin.corp-partner.google.com
> ---
> drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
As per previous discussions, this seems OK to me. I'll give it one
more day for anyone to speak up and then plan to land it.
-Doug
Re: [PATCH V2] drm/panel: boe-tv101wum-nl6: Fine tune Himax83102-j02 panel HFP and HBP
Hi, On Sun, Nov 19, 2023 at 5:33 PM cong yang wrote: > > Hi, > > On Sat, Nov 18, 2023 at 1:11 AM Doug Anderson wrote: > > > > Hi, > > > > On Thu, Nov 16, 2023 at 7:25 PM Cong Yang > > wrote: > > > > > > The refresh reported by modetest is 60.46Hz, and the actual measurement > > > is 60.01Hz, which is outside the expected tolerance. > > > > Presumably you've swapped the numbers above? The value reported by > > modetest is 60.01Hz and the actual measurement is 60.46Hz? > > No, the value reported by modetest is 60.46Hz. Indeed. I somehow assumed that the old value of "clock / (htotal * vtotal)" would have been the one that was closer to 60 Hz, but doing the math I agree with you. Specifically: >>> 16160 / ((1200 + 40 + 20 + 40) * (1920 + 116 + 8 + 12)) 60.46093983837174 >>> 16285 / ((1200 + 50 + 20 + 50) * (1920 + 116 + 8 + 12)) 60.005453366348306 Thanks for correcting me! -Doug
[Bug 218168] amdgpu: kfd_topology.c warning: the frame size of 1408 bytes is larger than 1024 bytes
https://bugzilla.kernel.org/show_bug.cgi?id=218168 Artem S. Tashkinov (a...@gmx.com) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |ANSWERED --- Comment #2 from Artem S. Tashkinov (a...@gmx.com) --- Please report here: https://gitlab.freedesktop.org/drm/amd/-/issues -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: Radeon regression in 6.6 kernel
On Mon, Nov 20, 2023 at 11:24 AM Christian König wrote: > > Am 20.11.23 um 17:08 schrieb Alex Deucher: > > On Mon, Nov 20, 2023 at 10:57 AM Christian König > > wrote: > >> Am 19.11.23 um 07:47 schrieb Dave Airlie: > On 12.11.23 01:46, Phillip Susi wrote: > > I had been testing some things on a post 6.6-rc5 kernel for a week or > > two and then when I pulled to a post 6.6 release kernel, I found that > > system suspend was broken. It seems that the radeon driver failed to > > suspend, leaving the display dead, the wayland display server hung, and > > the system still running. I have been trying to bisect it for the last > > few days and have only been able to narrow it down to the following 3 > > commits: > > > > There are only 'skip'ped commits left to test. > > The first bad commit could be any of: > > 56e449603f0ac580700621a356d35d5716a62ce5 > > c07bf1636f0005f9eb7956404490672286ea59d3 > > b70438004a14f4d0f9890b3297cd66248728546c > > We cannot bisect more! > Hmm, not a single reply from the amdgpu folks. Wondering how we can > encourage them to look into this. > > Phillip, reporting issues by mail should still work, but you might have > more luck here, as that's where the amdgpu afaics prefer to track bugs: > https://gitlab.freedesktop.org/drm/amd/-/issues > > When you file an issue there, please mention it here. > > Furthermore it might help if you could verify if 6.7-rc1 (or rc2, which > comes out later today) or 6.6.2-rc1 improve things. > >>> It would also be good to test if reverting any of these is possible or > >>> not. > >> Well none of the commits mentioned can affect radeon in any way. Radeon > >> simply doesn't use the scheduler. > >> > >> My suspicion is that the user is actually using amdgpu instead of > >> radeon. The switch potentially occurred accidentally, for example by > >> compiling amdgpu support for SI/CIK. > >> > >> Those amdgpu problems for older ASIC have already been worked on and > >> should be fixed by now. > > In this case it's a navi23 (so radeon in the marketing sense). > > Thanks, couldn't find that in the mail thread. > > In that case those are the already known problems with the scheduler > changes, aren't they? Yes. Those changes went into 6.7 though, not 6.6 AFAIK. Maybe I'm misunderstanding what the original report was actually testing. If it was 6.7, then try reverting: 56e449603f0ac580700621a356d35d5716a62ce5 b70438004a14f4d0f9890b3297cd66248728546c Alex > > Christian. > > > > > Alex > > > >> Regards, > >> Christian. > >> > >>> File the gitlab issue and we should poke amd a but more to take a look. > >>> > >>> Dave. >
Re: [PATCH v1 3/4] drm/rockchip: rk3066_hdmi: Remove useless output format
Hi Johan,
Am Donnerstag, 2. November 2023, 14:42:19 CET schrieb Johan Jonker:
> The Rk3066 hdmi output format is hard coded to RGB. Remove
> all useless code related to colorimetry and enc_out_format.
>
> Signed-off-by: Johan Jonker
I guess my first question is, is the hardcoding happening just because
of missing functionality in the driver, or does the hardware only
support RGB?
> ---
> drivers/gpu/drm/rockchip/rk3066_hdmi.c | 20 +---
> 1 file changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> index 0e7aae341960..f2b1b2faa096 100644
> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> @@ -23,8 +23,6 @@
>
> struct hdmi_data_info {
> int vic; /* The CEA Video ID (VIC) of the current drm display mode. */
> - unsigned int enc_out_format;
> - unsigned int colorimetry;
> };
>
> struct rk3066_hdmi_i2c {
> @@ -200,14 +198,7 @@ static int rk3066_hdmi_config_avi(struct rk3066_hdmi
> *hdmi,
> rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> &hdmi->connector, mode);
>
> - if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
> - frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> - else if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV422)
> - frame.avi.colorspace = HDMI_COLORSPACE_YUV422;
> - else
> - frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> -
> - frame.avi.colorimetry = hdmi->hdmi_data.colorimetry;
> + frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> frame.avi.scan_mode = HDMI_SCAN_MODE_NONE;
>
> return rk3066_hdmi_upload_frame(hdmi, rc, &frame,
> @@ -329,15 +320,6 @@ static int rk3066_hdmi_setup(struct rk3066_hdmi *hdmi,
> struct drm_display_info *display = &hdmi->connector.display_info;
>
> hdmi->hdmi_data.vic = drm_match_cea_mode(mode);
> - hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
> -
> - if (hdmi->hdmi_data.vic == 6 || hdmi->hdmi_data.vic == 7 ||
> - hdmi->hdmi_data.vic == 21 || hdmi->hdmi_data.vic == 22 ||
> - hdmi->hdmi_data.vic == 2 || hdmi->hdmi_data.vic == 3 ||
> - hdmi->hdmi_data.vic == 17 || hdmi->hdmi_data.vic == 18)
> - hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_601;
> - else
> - hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;
while I can understand the RGB output format, why does the colorimetry
also get removed? This looks like it is dependent on the mode itself
and not the output format?
Thanks
Heiko
Re: [PATCH] drm/rockchip: vop2: Add NV20 and NV30 support
On Wed, 25 Oct 2023 21:32:46 +, Jonas Karlman wrote: > Add support for the 10-bit 4:2:2 and 4:4:4 formats NV20 and NV30. > > These formats can be tested using modetest [1]: > > modetest -P @:1920x1080@ > > e.g. on a ROCK 3 Model A (rk3568): > > [...] Applied, thanks! [1/1] drm/rockchip: vop2: Add NV20 and NV30 support commit: 5fc6aa7db080fd90ef00846aac04e8a211088132 Best regards, -- Heiko Stuebner
Re: (subset) [PATCH v1 0/4] Rockchip rk3066_hdmi update
On Thu, 2 Nov 2023 14:40:13 +0100, Johan Jonker wrote: > Update the Rockchip rk3066_hdmi driver in a somewhat similar way > to what is proposed for the inno_hdmi driver. > > Johan Jonker (4): > drm/rockchip: rk3066_hdmi: Remove useless mode_fixup > drm/rockchip: rk3066_hdmi: Switch encoder hooks to atomic > drm/rockchip: rk3066_hdmi: Remove useless output format > drm/rockchip: rk3066_hdmi: Remove unused drm device pointer > > [...] Applied, thanks! [1/4] drm/rockchip: rk3066_hdmi: Remove useless mode_fixup commit: 1044f4a31734eef000f42cdaaf35bb2f76286be5 [2/4] drm/rockchip: rk3066_hdmi: Switch encoder hooks to atomic commit: ae3436a5e7c2ef4f92938133bd99f92fc47ea34e Best regards, -- Heiko Stuebner
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
Am 20.11.23 um 17:28 schrieb Thomas Zimmermann: Hi Am 20.11.23 um 17:22 schrieb Christian König: Am 20.11.23 um 17:15 schrieb Felix Kuehling: On 2023-11-20 11:02, Thomas Zimmermann wrote: Hi Christian Am 20.11.23 um 16:22 schrieb Christian König: Am 20.11.23 um 16:18 schrieb Thomas Zimmermann: Hi Am 20.11.23 um 16:06 schrieb Felix Kuehling: On 2023-11-20 6:54, Thomas Zimmermann wrote: Hi Am 17.11.23 um 22:44 schrieb Felix Kuehling: This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3. These helper functions are needed for KFD to export and import DMABufs the right way without duplicating the tracking of DMABufs associated with GEM objects while ensuring that move notifier callbacks are working as intended. I'm unhappy to see these functions making a comeback. They are the boiler-plate logic that all drivers should use. Historically, drivers did a lot one things in their GEM code that was only semi-correct. Unifying most of that made the memory management more readable. Not giving back drivers to option of tinkering with this might be preferable. The rsp hooks in struct drm_driver, prime_fd_to_handle and prime_handle_to_fd, are only there for vmwgfx. If you want to hook into prime import and export, there are drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't it possible to move the additional code behind these pointers? I'm not trying to hook into these functions, I'm just trying to call them. I'm not bringing back the .prime_handle_to_fd and .prime_fd_to_handle hooks in struct drm_driver. I need a clean way to export and import DMAbuffers from a kernel mode context. I had incorrect or semi-correct ways of doing that by calling some driver-internal functions, but then my DMABufs aren't correctly linked with GEM handles in DRM and move notifiers in amdgpu aren't working correctly. I understand that. But why don't you use drm_driver.gem_prime_import and drm_gem_object_funcs.export to add the amdkfd-specific code? These callbacks are being invoked from within drm_gem_prime_fd_to_handle() and drm_gem_prime_handle_to_fd() as part of the importing and exporting logic. With the intention of doing driver-specific things. Hence you should not have to re-export the internal drm_gem_prime_*_to_*() helpers. My question is if the existing hooks are not suitable for your needs. If so, how could we improve them? No no. You don't seem to understand the use case :) Felix doesn't try to implement any driver-specific things. I meant that I understand that this patchset is not about setting drm_driver.prime_handle_to_fd, et al. What Felix tries to do is to export a DMA-buf handle from kernel space. For example, looking at patch 2, it converts a GEM handle to a file descriptor and then assigns the rsp dmabuf to mem, which is of type struct kgd_mem. From my impression, this could be done within the existing ->export hook. That would skip the call to export_and_register_object. I think that's what I'm currently missing to set up gem_obj->dmabuf. I think we are talking past each other. kgd_mem is not part of the amdgpu driver, so it can't go into an ->export callback. What happens here is that a drm_client code uses the amdgpu driver to export some DMA-buf to file descriptors. In other words this is a high level user of drm_client and not something driver internal. If you don't want to export the drm_gem_prime_handle_to_fd() function directly we could add some drm_client_buffer_export() or similar. I think it's the fd that's bothering me. As I've mentioned in another email, fb appears to be not required. So the overall interface looks suboptimal. Something like drm_gem_prime_handle_to_dmabuf() would be better. Such a helper would then also invoke the existing hooks. Really good point. I think that should work for Felix as well. Thanks, Christian. But it's certainly not a blocker. Best regards Thomas Regards, Christian. Regards, Felix Then there's close_fd(), which cannot go into ->export. It looks like the fd isn't really required. Could the drm_prime_handle_to_fd() be reworked into a helper that converts the handle to the dmabuf without the fd? Something like drm_gem_prime_handle_to_dmabuf(), which would then be exported? And I have the question wrt the 3rd patch; just that it's about importing. (Looking further through the code, it appears that the fd could be removed from the helpers, the callbacks and vmwgfx. It would then be used entirely in the ioctl entry points, such as drm_prime_fd_to_handle_ioctl().) Best regards Thomas Regards, Christian. Best regards Thomas Regards, Felix Best regards Thomas CC: Christian König CC: Thomas Zimmermann Signed-off-by: Felix Kuehling --- drivers/gpu/drm/drm_prime.c | 33 ++--- include/drm/drm_prime.h | 7 +++ 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
Hi Am 20.11.23 um 17:22 schrieb Christian König: Am 20.11.23 um 17:15 schrieb Felix Kuehling: On 2023-11-20 11:02, Thomas Zimmermann wrote: Hi Christian Am 20.11.23 um 16:22 schrieb Christian König: Am 20.11.23 um 16:18 schrieb Thomas Zimmermann: Hi Am 20.11.23 um 16:06 schrieb Felix Kuehling: On 2023-11-20 6:54, Thomas Zimmermann wrote: Hi Am 17.11.23 um 22:44 schrieb Felix Kuehling: This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3. These helper functions are needed for KFD to export and import DMABufs the right way without duplicating the tracking of DMABufs associated with GEM objects while ensuring that move notifier callbacks are working as intended. I'm unhappy to see these functions making a comeback. They are the boiler-plate logic that all drivers should use. Historically, drivers did a lot one things in their GEM code that was only semi-correct. Unifying most of that made the memory management more readable. Not giving back drivers to option of tinkering with this might be preferable. The rsp hooks in struct drm_driver, prime_fd_to_handle and prime_handle_to_fd, are only there for vmwgfx. If you want to hook into prime import and export, there are drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't it possible to move the additional code behind these pointers? I'm not trying to hook into these functions, I'm just trying to call them. I'm not bringing back the .prime_handle_to_fd and .prime_fd_to_handle hooks in struct drm_driver. I need a clean way to export and import DMAbuffers from a kernel mode context. I had incorrect or semi-correct ways of doing that by calling some driver-internal functions, but then my DMABufs aren't correctly linked with GEM handles in DRM and move notifiers in amdgpu aren't working correctly. I understand that. But why don't you use drm_driver.gem_prime_import and drm_gem_object_funcs.export to add the amdkfd-specific code? These callbacks are being invoked from within drm_gem_prime_fd_to_handle() and drm_gem_prime_handle_to_fd() as part of the importing and exporting logic. With the intention of doing driver-specific things. Hence you should not have to re-export the internal drm_gem_prime_*_to_*() helpers. My question is if the existing hooks are not suitable for your needs. If so, how could we improve them? No no. You don't seem to understand the use case :) Felix doesn't try to implement any driver-specific things. I meant that I understand that this patchset is not about setting drm_driver.prime_handle_to_fd, et al. What Felix tries to do is to export a DMA-buf handle from kernel space. For example, looking at patch 2, it converts a GEM handle to a file descriptor and then assigns the rsp dmabuf to mem, which is of type struct kgd_mem. From my impression, this could be done within the existing ->export hook. That would skip the call to export_and_register_object. I think that's what I'm currently missing to set up gem_obj->dmabuf. I think we are talking past each other. kgd_mem is not part of the amdgpu driver, so it can't go into an ->export callback. What happens here is that a drm_client code uses the amdgpu driver to export some DMA-buf to file descriptors. In other words this is a high level user of drm_client and not something driver internal. If you don't want to export the drm_gem_prime_handle_to_fd() function directly we could add some drm_client_buffer_export() or similar. I think it's the fd that's bothering me. As I've mentioned in another email, fb appears to be not required. So the overall interface looks suboptimal. Something like drm_gem_prime_handle_to_dmabuf() would be better. Such a helper would then also invoke the existing hooks. But it's certainly not a blocker. Best regards Thomas Regards, Christian. Regards, Felix Then there's close_fd(), which cannot go into ->export. It looks like the fd isn't really required. Could the drm_prime_handle_to_fd() be reworked into a helper that converts the handle to the dmabuf without the fd? Something like drm_gem_prime_handle_to_dmabuf(), which would then be exported? And I have the question wrt the 3rd patch; just that it's about importing. (Looking further through the code, it appears that the fd could be removed from the helpers, the callbacks and vmwgfx. It would then be used entirely in the ioctl entry points, such as drm_prime_fd_to_handle_ioctl().) Best regards Thomas Regards, Christian. Best regards Thomas Regards, Felix Best regards Thomas CC: Christian König CC: Thomas Zimmermann Signed-off-by: Felix Kuehling --- drivers/gpu/drm/drm_prime.c | 33 ++--- include/drm/drm_prime.h | 7 +++ 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 63b709a67471..834a5e28abbe 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -278,7 +278
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
Hi
Am 20.11.23 um 17:15 schrieb Felix Kuehling:
On 2023-11-20 11:02, Thomas Zimmermann wrote:
Hi Christian
Am 20.11.23 um 16:22 schrieb Christian König:
Am 20.11.23 um 16:18 schrieb Thomas Zimmermann:
Hi
Am 20.11.23 um 16:06 schrieb Felix Kuehling:
On 2023-11-20 6:54, Thomas Zimmermann wrote:
Hi
Am 17.11.23 um 22:44 schrieb Felix Kuehling:
This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3.
These helper functions are needed for KFD to export and import
DMABufs
the right way without duplicating the tracking of DMABufs
associated with
GEM objects while ensuring that move notifier callbacks are
working as
intended.
I'm unhappy to see these functions making a comeback. They are the
boiler-plate logic that all drivers should use. Historically,
drivers did a lot one things in their GEM code that was only
semi-correct. Unifying most of that made the memory management more
readable. Not giving back drivers to option of tinkering with this
might be preferable. The rsp hooks in struct drm_driver,
prime_fd_to_handle and prime_handle_to_fd, are only there for vmwgfx.
If you want to hook into prime import and export, there are
drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't
it possible to move the additional code behind these pointers?
I'm not trying to hook into these functions, I'm just trying to call
them. I'm not bringing back the .prime_handle_to_fd and
.prime_fd_to_handle hooks in struct drm_driver. I need a clean way to
export and import DMAbuffers from a kernel mode context. I had
incorrect or semi-correct ways of doing that by calling some
driver-internal functions, but then my DMABufs aren't correctly
linked with GEM handles in DRM and move notifiers in amdgpu aren't
working correctly.
I understand that. But why don't you use drm_driver.gem_prime_import
and drm_gem_object_funcs.export to add the amdkfd-specific code? These
callbacks are being invoked from within drm_gem_prime_fd_to_handle()
and
drm_gem_prime_handle_to_fd() as part of the importing and exporting
logic. With the intention of doing driver-specific things. Hence you
should not have to re-export the internal drm_gem_prime_*_to_*()
helpers.
My question is if the existing hooks are not suitable for your needs.
If so, how could we improve them?
No no. You don't seem to understand the use case :) Felix doesn't try to
implement any driver-specific things.
I meant that I understand that this patchset is not about setting
drm_driver.prime_handle_to_fd, et al.
What Felix tries to do is to export a DMA-buf handle from kernel space.
For example, looking at patch 2, it converts a GEM handle to a file
descriptor and then assigns the rsp dmabuf to mem, which is of type
struct kgd_mem. From my impression, this could be done within the
existing ->export hook.
That would skip the call to export_and_register_object. I think that's
what I'm currently missing to set up gem_obj->dmabuf.
Well, OK. Nevermind.
Best regards
Thomas
Regards,
Felix
Then there's close_fd(), which cannot go into ->export. It looks like
the fd isn't really required. Could the drm_prime_handle_to_fd() be
reworked into a helper that converts the handle to the dmabuf without
the fd? Something like drm_gem_prime_handle_to_dmabuf(), which would
then be exported?
And I have the question wrt the 3rd patch; just that it's about
importing.
(Looking further through the code, it appears that the fd could be
removed from the helpers, the callbacks and vmwgfx. It would then be
used entirely in the ioctl entry points, such as
drm_prime_fd_to_handle_ioctl().)
Best regards
Thomas
Regards,
Christian.
Best regards
Thomas
Regards,
Felix
Best regards
Thomas
CC: Christian König
CC: Thomas Zimmermann
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/drm_prime.c | 33
++---
include/drm/drm_prime.h | 7 +++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c
b/drivers/gpu/drm/drm_prime.c
index 63b709a67471..834a5e28abbe 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf
*dma_buf)
}
EXPORT_SYMBOL(drm_gem_dmabuf_release);
-/*
+/**
* drm_gem_prime_fd_to_handle - PRIME import function for GEM
drivers
* @dev: drm_device to import into
* @file_priv: drm file-private structure
@@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
*
* Returns 0 on success or a negative error code on failure.
*/
-static int drm_gem_prime_fd_to_handle(struct drm_device *dev,
- struct drm_file *file_priv, int prime_fd,
- uint32_t *handle)
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+ struct drm_file *file_priv, int prime_fd,
+ uint32_t *handle)
{
struct dma_buf *dma_buf;
struct drm_gem_object *obj;
@@ -360,6 +360,7 @@ static in
Re: Radeon regression in 6.6 kernel
Am 20.11.23 um 17:08 schrieb Alex Deucher: On Mon, Nov 20, 2023 at 10:57 AM Christian König wrote: Am 19.11.23 um 07:47 schrieb Dave Airlie: On 12.11.23 01:46, Phillip Susi wrote: I had been testing some things on a post 6.6-rc5 kernel for a week or two and then when I pulled to a post 6.6 release kernel, I found that system suspend was broken. It seems that the radeon driver failed to suspend, leaving the display dead, the wayland display server hung, and the system still running. I have been trying to bisect it for the last few days and have only been able to narrow it down to the following 3 commits: There are only 'skip'ped commits left to test. The first bad commit could be any of: 56e449603f0ac580700621a356d35d5716a62ce5 c07bf1636f0005f9eb7956404490672286ea59d3 b70438004a14f4d0f9890b3297cd66248728546c We cannot bisect more! Hmm, not a single reply from the amdgpu folks. Wondering how we can encourage them to look into this. Phillip, reporting issues by mail should still work, but you might have more luck here, as that's where the amdgpu afaics prefer to track bugs: https://gitlab.freedesktop.org/drm/amd/-/issues When you file an issue there, please mention it here. Furthermore it might help if you could verify if 6.7-rc1 (or rc2, which comes out later today) or 6.6.2-rc1 improve things. It would also be good to test if reverting any of these is possible or not. Well none of the commits mentioned can affect radeon in any way. Radeon simply doesn't use the scheduler. My suspicion is that the user is actually using amdgpu instead of radeon. The switch potentially occurred accidentally, for example by compiling amdgpu support for SI/CIK. Those amdgpu problems for older ASIC have already been worked on and should be fixed by now. In this case it's a navi23 (so radeon in the marketing sense). Thanks, couldn't find that in the mail thread. In that case those are the already known problems with the scheduler changes, aren't they? Christian. Alex Regards, Christian. File the gitlab issue and we should poke amd a but more to take a look. Dave.
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
Am 20.11.23 um 17:15 schrieb Felix Kuehling: On 2023-11-20 11:02, Thomas Zimmermann wrote: Hi Christian Am 20.11.23 um 16:22 schrieb Christian König: Am 20.11.23 um 16:18 schrieb Thomas Zimmermann: Hi Am 20.11.23 um 16:06 schrieb Felix Kuehling: On 2023-11-20 6:54, Thomas Zimmermann wrote: Hi Am 17.11.23 um 22:44 schrieb Felix Kuehling: This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3. These helper functions are needed for KFD to export and import DMABufs the right way without duplicating the tracking of DMABufs associated with GEM objects while ensuring that move notifier callbacks are working as intended. I'm unhappy to see these functions making a comeback. They are the boiler-plate logic that all drivers should use. Historically, drivers did a lot one things in their GEM code that was only semi-correct. Unifying most of that made the memory management more readable. Not giving back drivers to option of tinkering with this might be preferable. The rsp hooks in struct drm_driver, prime_fd_to_handle and prime_handle_to_fd, are only there for vmwgfx. If you want to hook into prime import and export, there are drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't it possible to move the additional code behind these pointers? I'm not trying to hook into these functions, I'm just trying to call them. I'm not bringing back the .prime_handle_to_fd and .prime_fd_to_handle hooks in struct drm_driver. I need a clean way to export and import DMAbuffers from a kernel mode context. I had incorrect or semi-correct ways of doing that by calling some driver-internal functions, but then my DMABufs aren't correctly linked with GEM handles in DRM and move notifiers in amdgpu aren't working correctly. I understand that. But why don't you use drm_driver.gem_prime_import and drm_gem_object_funcs.export to add the amdkfd-specific code? These callbacks are being invoked from within drm_gem_prime_fd_to_handle() and drm_gem_prime_handle_to_fd() as part of the importing and exporting logic. With the intention of doing driver-specific things. Hence you should not have to re-export the internal drm_gem_prime_*_to_*() helpers. My question is if the existing hooks are not suitable for your needs. If so, how could we improve them? No no. You don't seem to understand the use case :) Felix doesn't try to implement any driver-specific things. I meant that I understand that this patchset is not about setting drm_driver.prime_handle_to_fd, et al. What Felix tries to do is to export a DMA-buf handle from kernel space. For example, looking at patch 2, it converts a GEM handle to a file descriptor and then assigns the rsp dmabuf to mem, which is of type struct kgd_mem. From my impression, this could be done within the existing ->export hook. That would skip the call to export_and_register_object. I think that's what I'm currently missing to set up gem_obj->dmabuf. I think we are talking past each other. kgd_mem is not part of the amdgpu driver, so it can't go into an ->export callback. What happens here is that a drm_client code uses the amdgpu driver to export some DMA-buf to file descriptors. In other words this is a high level user of drm_client and not something driver internal. If you don't want to export the drm_gem_prime_handle_to_fd() function directly we could add some drm_client_buffer_export() or similar. Regards, Christian. Regards, Felix Then there's close_fd(), which cannot go into ->export. It looks like the fd isn't really required. Could the drm_prime_handle_to_fd() be reworked into a helper that converts the handle to the dmabuf without the fd? Something like drm_gem_prime_handle_to_dmabuf(), which would then be exported? And I have the question wrt the 3rd patch; just that it's about importing. (Looking further through the code, it appears that the fd could be removed from the helpers, the callbacks and vmwgfx. It would then be used entirely in the ioctl entry points, such as drm_prime_fd_to_handle_ioctl().) Best regards Thomas Regards, Christian. Best regards Thomas Regards, Felix Best regards Thomas CC: Christian König CC: Thomas Zimmermann Signed-off-by: Felix Kuehling --- drivers/gpu/drm/drm_prime.c | 33 ++--- include/drm/drm_prime.h | 7 +++ 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 63b709a67471..834a5e28abbe 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf) } EXPORT_SYMBOL(drm_gem_dmabuf_release); -/* +/** * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers * @dev: drm_device to import into * @file_priv: drm file-private structure @@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release); * * Returns 0 on success or a negative error c
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
On 2023-11-20 11:02, Thomas Zimmermann wrote:
Hi Christian
Am 20.11.23 um 16:22 schrieb Christian König:
Am 20.11.23 um 16:18 schrieb Thomas Zimmermann:
Hi
Am 20.11.23 um 16:06 schrieb Felix Kuehling:
On 2023-11-20 6:54, Thomas Zimmermann wrote:
Hi
Am 17.11.23 um 22:44 schrieb Felix Kuehling:
This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3.
These helper functions are needed for KFD to export and import DMABufs
the right way without duplicating the tracking of DMABufs
associated with
GEM objects while ensuring that move notifier callbacks are working as
intended.
I'm unhappy to see these functions making a comeback. They are the
boiler-plate logic that all drivers should use. Historically,
drivers did a lot one things in their GEM code that was only
semi-correct. Unifying most of that made the memory management more
readable. Not giving back drivers to option of tinkering with this
might be preferable. The rsp hooks in struct drm_driver,
prime_fd_to_handle and prime_handle_to_fd, are only there for vmwgfx.
If you want to hook into prime import and export, there are
drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't
it possible to move the additional code behind these pointers?
I'm not trying to hook into these functions, I'm just trying to call
them. I'm not bringing back the .prime_handle_to_fd and
.prime_fd_to_handle hooks in struct drm_driver. I need a clean way to
export and import DMAbuffers from a kernel mode context. I had
incorrect or semi-correct ways of doing that by calling some
driver-internal functions, but then my DMABufs aren't correctly
linked with GEM handles in DRM and move notifiers in amdgpu aren't
working correctly.
I understand that. But why don't you use drm_driver.gem_prime_import
and drm_gem_object_funcs.export to add the amdkfd-specific code? These
callbacks are being invoked from within drm_gem_prime_fd_to_handle() and
drm_gem_prime_handle_to_fd() as part of the importing and exporting
logic. With the intention of doing driver-specific things. Hence you
should not have to re-export the internal drm_gem_prime_*_to_*() helpers.
My question is if the existing hooks are not suitable for your needs.
If so, how could we improve them?
No no. You don't seem to understand the use case :) Felix doesn't try to
implement any driver-specific things.
I meant that I understand that this patchset is not about setting
drm_driver.prime_handle_to_fd, et al.
What Felix tries to do is to export a DMA-buf handle from kernel space.
For example, looking at patch 2, it converts a GEM handle to a file
descriptor and then assigns the rsp dmabuf to mem, which is of type
struct kgd_mem. From my impression, this could be done within the
existing ->export hook.
That would skip the call to export_and_register_object. I think that's
what I'm currently missing to set up gem_obj->dmabuf.
Regards,
Felix
Then there's close_fd(), which cannot go into ->export. It looks like
the fd isn't really required. Could the drm_prime_handle_to_fd() be
reworked into a helper that converts the handle to the dmabuf without
the fd? Something like drm_gem_prime_handle_to_dmabuf(), which would
then be exported?
And I have the question wrt the 3rd patch; just that it's about importing.
(Looking further through the code, it appears that the fd could be
removed from the helpers, the callbacks and vmwgfx. It would then be
used entirely in the ioctl entry points, such as
drm_prime_fd_to_handle_ioctl().)
Best regards
Thomas
Regards,
Christian.
Best regards
Thomas
Regards,
Felix
Best regards
Thomas
CC: Christian König
CC: Thomas Zimmermann
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/drm_prime.c | 33 ++---
include/drm/drm_prime.h | 7 +++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 63b709a67471..834a5e28abbe 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf
*dma_buf)
}
EXPORT_SYMBOL(drm_gem_dmabuf_release);
-/*
+/**
* drm_gem_prime_fd_to_handle - PRIME import function for GEM
drivers
* @dev: drm_device to import into
* @file_priv: drm file-private structure
@@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
*
* Returns 0 on success or a negative error code on failure.
*/
-static int drm_gem_prime_fd_to_handle(struct drm_device *dev,
- struct drm_file *file_priv, int prime_fd,
- uint32_t *handle)
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+ struct drm_file *file_priv, int prime_fd,
+ uint32_t *handle)
{
struct dma_buf *dma_buf;
struct drm_gem_object *obj;
@@ -360,6 +360,7 @@ static int drm_gem_prime_fd_to_handle(struct
drm_device *dev,
dma_buf_put(dma_buf);
return ret;
}
+EX
Re: [PATCH v2] drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full
On Thu, 26 Oct 2023 19:14:58 +, Jonas Karlman wrote: > Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328 > and RK3399 result in wrong colors being displayed. > > The issue can be observed using modetest: > > modetest -s @:1920x1080-60@RG24 > modetest -s @:1920x1080-60@BG24 > > [...] Applied, thanks! [1/1] drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full commit: bb0a05acd6121ff0e810b44fdc24dbdfaa46b642 Best regards, -- Heiko Stuebner
Re: Radeon regression in 6.6 kernel
On Mon, Nov 20, 2023 at 10:57 AM Christian König wrote: > > Am 19.11.23 um 07:47 schrieb Dave Airlie: > >> On 12.11.23 01:46, Phillip Susi wrote: > >>> I had been testing some things on a post 6.6-rc5 kernel for a week or > >>> two and then when I pulled to a post 6.6 release kernel, I found that > >>> system suspend was broken. It seems that the radeon driver failed to > >>> suspend, leaving the display dead, the wayland display server hung, and > >>> the system still running. I have been trying to bisect it for the last > >>> few days and have only been able to narrow it down to the following 3 > >>> commits: > >>> > >>> There are only 'skip'ped commits left to test. > >>> The first bad commit could be any of: > >>> 56e449603f0ac580700621a356d35d5716a62ce5 > >>> c07bf1636f0005f9eb7956404490672286ea59d3 > >>> b70438004a14f4d0f9890b3297cd66248728546c > >>> We cannot bisect more! > >> Hmm, not a single reply from the amdgpu folks. Wondering how we can > >> encourage them to look into this. > >> > >> Phillip, reporting issues by mail should still work, but you might have > >> more luck here, as that's where the amdgpu afaics prefer to track bugs: > >> https://gitlab.freedesktop.org/drm/amd/-/issues > >> > >> When you file an issue there, please mention it here. > >> > >> Furthermore it might help if you could verify if 6.7-rc1 (or rc2, which > >> comes out later today) or 6.6.2-rc1 improve things. > > It would also be good to test if reverting any of these is possible or not. > > Well none of the commits mentioned can affect radeon in any way. Radeon > simply doesn't use the scheduler. > > My suspicion is that the user is actually using amdgpu instead of > radeon. The switch potentially occurred accidentally, for example by > compiling amdgpu support for SI/CIK. > > Those amdgpu problems for older ASIC have already been worked on and > should be fixed by now. In this case it's a navi23 (so radeon in the marketing sense). Alex > > Regards, > Christian. > > > > > File the gitlab issue and we should poke amd a but more to take a look. > > > > Dave. >
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
Hi Christian
Am 20.11.23 um 16:22 schrieb Christian König:
Am 20.11.23 um 16:18 schrieb Thomas Zimmermann:
Hi
Am 20.11.23 um 16:06 schrieb Felix Kuehling:
On 2023-11-20 6:54, Thomas Zimmermann wrote:
Hi
Am 17.11.23 um 22:44 schrieb Felix Kuehling:
This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3.
These helper functions are needed for KFD to export and import DMABufs
the right way without duplicating the tracking of DMABufs
associated with
GEM objects while ensuring that move notifier callbacks are working as
intended.
I'm unhappy to see these functions making a comeback. They are the
boiler-plate logic that all drivers should use. Historically,
drivers did a lot one things in their GEM code that was only
semi-correct. Unifying most of that made the memory management more
readable. Not giving back drivers to option of tinkering with this
might be preferable. The rsp hooks in struct drm_driver,
prime_fd_to_handle and prime_handle_to_fd, are only there for vmwgfx.
If you want to hook into prime import and export, there are
drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't
it possible to move the additional code behind these pointers?
I'm not trying to hook into these functions, I'm just trying to call
them. I'm not bringing back the .prime_handle_to_fd and
.prime_fd_to_handle hooks in struct drm_driver. I need a clean way to
export and import DMAbuffers from a kernel mode context. I had
incorrect or semi-correct ways of doing that by calling some
driver-internal functions, but then my DMABufs aren't correctly
linked with GEM handles in DRM and move notifiers in amdgpu aren't
working correctly.
I understand that. But why don't you use drm_driver.gem_prime_import
and drm_gem_object_funcs.export to add the amdkfd-specific code? These
callbacks are being invoked from within drm_gem_prime_fd_to_handle() and
drm_gem_prime_handle_to_fd() as part of the importing and exporting
logic. With the intention of doing driver-specific things. Hence you
should not have to re-export the internal drm_gem_prime_*_to_*() helpers.
My question is if the existing hooks are not suitable for your needs.
If so, how could we improve them?
No no. You don't seem to understand the use case :) Felix doesn't try to
implement any driver-specific things.
I meant that I understand that this patchset is not about setting
drm_driver.prime_handle_to_fd, et al.
What Felix tries to do is to export a DMA-buf handle from kernel space.
For example, looking at patch 2, it converts a GEM handle to a file
descriptor and then assigns the rsp dmabuf to mem, which is of type
struct kgd_mem. From my impression, this could be done within the
existing ->export hook.
Then there's close_fd(), which cannot go into ->export. It looks like
the fd isn't really required. Could the drm_prime_handle_to_fd() be
reworked into a helper that converts the handle to the dmabuf without
the fd? Something like drm_gem_prime_handle_to_dmabuf(), which would
then be exported?
And I have the question wrt the 3rd patch; just that it's about importing.
(Looking further through the code, it appears that the fd could be
removed from the helpers, the callbacks and vmwgfx. It would then be
used entirely in the ioctl entry points, such as
drm_prime_fd_to_handle_ioctl().)
Best regards
Thomas
Regards,
Christian.
Best regards
Thomas
Regards,
Felix
Best regards
Thomas
CC: Christian König
CC: Thomas Zimmermann
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/drm_prime.c | 33 ++---
include/drm/drm_prime.h | 7 +++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 63b709a67471..834a5e28abbe 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf
*dma_buf)
}
EXPORT_SYMBOL(drm_gem_dmabuf_release);
-/*
+/**
* drm_gem_prime_fd_to_handle - PRIME import function for GEM
drivers
* @dev: drm_device to import into
* @file_priv: drm file-private structure
@@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
*
* Returns 0 on success or a negative error code on failure.
*/
-static int drm_gem_prime_fd_to_handle(struct drm_device *dev,
- struct drm_file *file_priv, int prime_fd,
- uint32_t *handle)
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+ struct drm_file *file_priv, int prime_fd,
+ uint32_t *handle)
{
struct dma_buf *dma_buf;
struct drm_gem_object *obj;
@@ -360,6 +360,7 @@ static int drm_gem_prime_fd_to_handle(struct
drm_device *dev,
dma_buf_put(dma_buf);
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_pri
Re: Radeon regression in 6.6 kernel
Am 19.11.23 um 07:47 schrieb Dave Airlie: On 12.11.23 01:46, Phillip Susi wrote: I had been testing some things on a post 6.6-rc5 kernel for a week or two and then when I pulled to a post 6.6 release kernel, I found that system suspend was broken. It seems that the radeon driver failed to suspend, leaving the display dead, the wayland display server hung, and the system still running. I have been trying to bisect it for the last few days and have only been able to narrow it down to the following 3 commits: There are only 'skip'ped commits left to test. The first bad commit could be any of: 56e449603f0ac580700621a356d35d5716a62ce5 c07bf1636f0005f9eb7956404490672286ea59d3 b70438004a14f4d0f9890b3297cd66248728546c We cannot bisect more! Hmm, not a single reply from the amdgpu folks. Wondering how we can encourage them to look into this. Phillip, reporting issues by mail should still work, but you might have more luck here, as that's where the amdgpu afaics prefer to track bugs: https://gitlab.freedesktop.org/drm/amd/-/issues When you file an issue there, please mention it here. Furthermore it might help if you could verify if 6.7-rc1 (or rc2, which comes out later today) or 6.6.2-rc1 improve things. It would also be good to test if reverting any of these is possible or not. Well none of the commits mentioned can affect radeon in any way. Radeon simply doesn't use the scheduler. My suspicion is that the user is actually using amdgpu instead of radeon. The switch potentially occurred accidentally, for example by compiling amdgpu support for SI/CIK. Those amdgpu problems for older ASIC have already been worked on and should be fixed by now. Regards, Christian. File the gitlab issue and we should poke amd a but more to take a look. Dave.
Re: [PATCH 1/3] dt-bindings: display: ssd1307fb: Change "solomon,page-offset" default value
On Thu, Nov 16, 2023 at 07:07:37PM +0100, Javier Martinez Canillas wrote: > This is used to specify the page start address offset of the display RAM. > > The value is used as offset when setting the page start address with the > SSD130X_SET_PAGE_RANGE command, and the driver currently sets its value to > 1 if the property is not present in the Device Tree. > > But the datasheet mentions that the value on reset for the page start is a > 0, so it makes more sense to also have 0 as the default value for the page > offset if the property is not present. > > In fact, using a default value of 1 leads to the display not working when > the fbdev is attached to the framebuffer console. > > Reported-by: Sahaj Sarup > Signed-off-by: Javier Martinez Canillas > --- > > .../devicetree/bindings/display/solomon,ssd1307fb.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > index 3afbb52d1b7f..badd81459aaa 100644 > --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > @@ -35,7 +35,7 @@ properties: > >solomon,page-offset: > $ref: /schemas/types.yaml#/definitions/uint32 > -default: 1 > +default: 0 I think I saw it pointed out by Maxime elsewhere that this breaks the ABI. It would be nice if DT defaults matched the hardware's, but I don't really think it is worth breaking the ABI here. Expanding the property description to explain the impact of the particular values might help with incorrect usage. Thanks, Conor. > description: >Offset of pages (band of 8 pixels) that the screen is mapped to > > -- > 2.41.0 > signature.asc Description: PGP signature
[Bug 218168] amdgpu: kfd_topology.c warning: the frame size of 1408 bytes is larger than 1024 bytes
https://bugzilla.kernel.org/show_bug.cgi?id=218168 --- Comment #1 from hamza.mahf...@amd.com --- + amd-gfx + Felix On 11/20/23 10:16, bugzilla-dae...@kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=218168 > > Bug ID: 218168 > Summary: amdgpu: kfd_topology.c warning: the frame size of 1408 > bytes is larger than 1024 bytes > Product: Drivers > Version: 2.5 >Hardware: All > OS: Linux > Status: NEW >Severity: normal >Priority: P3 > Component: Video(DRI - non Intel) >Assignee: drivers_video-...@kernel-bugs.osdl.org >Reporter: bluesun...@gmail.com > Regression: No > > Trying to compile Linux 6.6.2 with GCC 13.2.1 and CONFIG_WERROR=y: > > [...] > drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In function > 'kfd_topology_add_device': > drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:2082:1: error: the frame > size of 1408 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > 2082 | } >| ^ > cc1: all warnings being treated as errors > [...] > -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [Bug 218168] New: amdgpu: kfd_topology.c warning: the frame size of 1408 bytes is larger than 1024 bytes
+ amd-gfx + Felix On 11/20/23 10:16, bugzilla-dae...@kernel.org wrote: https://bugzilla.kernel.org/show_bug.cgi?id=218168 Bug ID: 218168 Summary: amdgpu: kfd_topology.c warning: the frame size of 1408 bytes is larger than 1024 bytes Product: Drivers Version: 2.5 Hardware: All OS: Linux Status: NEW Severity: normal Priority: P3 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: bluesun...@gmail.com Regression: No Trying to compile Linux 6.6.2 with GCC 13.2.1 and CONFIG_WERROR=y: [...] drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In function 'kfd_topology_add_device': drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:2082:1: error: the frame size of 1408 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] 2082 | } | ^ cc1: all warnings being treated as errors [...] -- Hamza
Re: (subset) [PATCH V4 0/6] rockchip: Add Powkiddy RK2023
On Fri, 17 Nov 2023 14:25:30 -0600, Chris Morgan wrote: > From: Chris Morgan > > Add support for the Powkiddy RK2023, which is extremely similar to > existing Powkiddy RGB30 device. > > Changes since V3: > - Corrected commit subject lines. > > [...] Applied, thanks! [4/6] dt-bindings: arm: rockchip: Add Powkiddy RK2023 commit: 213615d742f0c039ab73f8946ae18000cf2c7b65 [5/6] arm64: dts: rockchip: Update powkiddy,rgb30 include to rk2023 DTSI commit: 46d84ceb7eec85b60e8a5eb0dfb2fae6a1bf4166 [6/6] arm64: dts: rockchip: Add Powkiddy RK2023 commit: e926380ea2a2b10d01069917e6d678ca818f6ad8 Best regards, -- Heiko Stuebner
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
Am 20.11.23 um 16:18 schrieb Thomas Zimmermann:
Hi
Am 20.11.23 um 16:06 schrieb Felix Kuehling:
On 2023-11-20 6:54, Thomas Zimmermann wrote:
Hi
Am 17.11.23 um 22:44 schrieb Felix Kuehling:
This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3.
These helper functions are needed for KFD to export and import DMABufs
the right way without duplicating the tracking of DMABufs
associated with
GEM objects while ensuring that move notifier callbacks are working as
intended.
I'm unhappy to see these functions making a comeback. They are the
boiler-plate logic that all drivers should use. Historically,
drivers did a lot one things in their GEM code that was only
semi-correct. Unifying most of that made the memory management more
readable. Not giving back drivers to option of tinkering with this
might be preferable. The rsp hooks in struct drm_driver,
prime_fd_to_handle and prime_handle_to_fd, are only there for vmwgfx.
If you want to hook into prime import and export, there are
drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't
it possible to move the additional code behind these pointers?
I'm not trying to hook into these functions, I'm just trying to call
them. I'm not bringing back the .prime_handle_to_fd and
.prime_fd_to_handle hooks in struct drm_driver. I need a clean way to
export and import DMAbuffers from a kernel mode context. I had
incorrect or semi-correct ways of doing that by calling some
driver-internal functions, but then my DMABufs aren't correctly
linked with GEM handles in DRM and move notifiers in amdgpu aren't
working correctly.
I understand that. But why don't you use drm_driver.gem_prime_import
and drm_gem_object_funcs.export to add the amdkfd-specific code? These
callbacks are being invoked from within drm_gem_prime_fd_to_handle() and
drm_gem_prime_handle_to_fd() as part of the importing and exporting
logic. With the intention of doing driver-specific things. Hence you
should not have to re-export the internal drm_gem_prime_*_to_*() helpers.
My question is if the existing hooks are not suitable for your needs.
If so, how could we improve them?
No no. You don't seem to understand the use case :) Felix doesn't try to
implement any driver-specific things.
What Felix tries to do is to export a DMA-buf handle from kernel space.
Regards,
Christian.
Best regards
Thomas
Regards,
Felix
Best regards
Thomas
CC: Christian König
CC: Thomas Zimmermann
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/drm_prime.c | 33 ++---
include/drm/drm_prime.h | 7 +++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 63b709a67471..834a5e28abbe 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf
*dma_buf)
}
EXPORT_SYMBOL(drm_gem_dmabuf_release);
-/*
+/**
* drm_gem_prime_fd_to_handle - PRIME import function for GEM
drivers
* @dev: drm_device to import into
* @file_priv: drm file-private structure
@@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
*
* Returns 0 on success or a negative error code on failure.
*/
-static int drm_gem_prime_fd_to_handle(struct drm_device *dev,
- struct drm_file *file_priv, int prime_fd,
- uint32_t *handle)
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+ struct drm_file *file_priv, int prime_fd,
+ uint32_t *handle)
{
struct dma_buf *dma_buf;
struct drm_gem_object *obj;
@@ -360,6 +360,7 @@ static int drm_gem_prime_fd_to_handle(struct
drm_device *dev,
dma_buf_put(dma_buf);
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_priv)
@@ -408,7 +409,7 @@ static struct dma_buf
*export_and_register_object(struct drm_device *dev,
return dmabuf;
}
-/*
+/**
* drm_gem_prime_handle_to_fd - PRIME export function for GEM
drivers
* @dev: dev to export the buffer from
* @file_priv: drm file-private structure
@@ -421,10 +422,10 @@ static struct dma_buf
*export_and_register_object(struct drm_device *dev,
* The actual exporting from GEM object to a dma-buf is done
through the
* &drm_gem_object_funcs.export callback.
*/
-static int drm_gem_prime_handle_to_fd(struct drm_device *dev,
- struct drm_file *file_priv, uint32_t handle,
- uint32_t flags,
- int *prime_fd)
+int drm_gem_prime_handle_to_fd(struct drm_device *dev,
+ struct drm_file *file_priv, uint32_t handle,
+ uint32_t flags,
+ int *prime_fd)
{
struct drm_gem_object *obj;
int ret = 0;
@@ -506,6 +507,7 @@ static int drm_gem_prime_handle_t
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
Hi
Am 20.11.23 um 16:06 schrieb Felix Kuehling:
On 2023-11-20 6:54, Thomas Zimmermann wrote:
Hi
Am 17.11.23 um 22:44 schrieb Felix Kuehling:
This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3.
These helper functions are needed for KFD to export and import DMABufs
the right way without duplicating the tracking of DMABufs associated
with
GEM objects while ensuring that move notifier callbacks are working as
intended.
I'm unhappy to see these functions making a comeback. They are the
boiler-plate logic that all drivers should use. Historically, drivers
did a lot one things in their GEM code that was only semi-correct.
Unifying most of that made the memory management more readable. Not
giving back drivers to option of tinkering with this might be
preferable. The rsp hooks in struct drm_driver, prime_fd_to_handle and
prime_handle_to_fd, are only there for vmwgfx.
If you want to hook into prime import and export, there are
drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't it
possible to move the additional code behind these pointers?
I'm not trying to hook into these functions, I'm just trying to call
them. I'm not bringing back the .prime_handle_to_fd and
.prime_fd_to_handle hooks in struct drm_driver. I need a clean way to
export and import DMAbuffers from a kernel mode context. I had incorrect
or semi-correct ways of doing that by calling some driver-internal
functions, but then my DMABufs aren't correctly linked with GEM handles
in DRM and move notifiers in amdgpu aren't working correctly.
I understand that. But why don't you use drm_driver.gem_prime_import and
drm_gem_object_funcs.export to add the amdkfd-specific code? These
callbacks are being invoked from within drm_gem_prime_fd_to_handle() and
drm_gem_prime_handle_to_fd() as part of the importing and exporting
logic. With the intention of doing driver-specific things. Hence you
should not have to re-export the internal drm_gem_prime_*_to_*() helpers.
My question is if the existing hooks are not suitable for your needs. If
so, how could we improve them?
Best regards
Thomas
Regards,
Felix
Best regards
Thomas
CC: Christian König
CC: Thomas Zimmermann
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/drm_prime.c | 33 ++---
include/drm/drm_prime.h | 7 +++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 63b709a67471..834a5e28abbe 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
}
EXPORT_SYMBOL(drm_gem_dmabuf_release);
-/*
+/**
* drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
* @dev: drm_device to import into
* @file_priv: drm file-private structure
@@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
*
* Returns 0 on success or a negative error code on failure.
*/
-static int drm_gem_prime_fd_to_handle(struct drm_device *dev,
- struct drm_file *file_priv, int prime_fd,
- uint32_t *handle)
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+ struct drm_file *file_priv, int prime_fd,
+ uint32_t *handle)
{
struct dma_buf *dma_buf;
struct drm_gem_object *obj;
@@ -360,6 +360,7 @@ static int drm_gem_prime_fd_to_handle(struct
drm_device *dev,
dma_buf_put(dma_buf);
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -408,7 +409,7 @@ static struct dma_buf
*export_and_register_object(struct drm_device *dev,
return dmabuf;
}
-/*
+/**
* drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
* @dev: dev to export the buffer from
* @file_priv: drm file-private structure
@@ -421,10 +422,10 @@ static struct dma_buf
*export_and_register_object(struct drm_device *dev,
* The actual exporting from GEM object to a dma-buf is done
through the
* &drm_gem_object_funcs.export callback.
*/
-static int drm_gem_prime_handle_to_fd(struct drm_device *dev,
- struct drm_file *file_priv, uint32_t handle,
- uint32_t flags,
- int *prime_fd)
+int drm_gem_prime_handle_to_fd(struct drm_device *dev,
+ struct drm_file *file_priv, uint32_t handle,
+ uint32_t flags,
+ int *prime_fd)
{
struct drm_gem_object *obj;
int ret = 0;
@@ -506,6 +507,7 @@ static int drm_gem_prime_handle_to_fd(struct
drm_device *dev,
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -864,9 +866,9 @@ EXPORT_SYMBOL(drm_prime_get_c
[Bug 218168] New: amdgpu: kfd_topology.c warning: the frame size of 1408 bytes is larger than 1024 bytes
https://bugzilla.kernel.org/show_bug.cgi?id=218168 Bug ID: 218168 Summary: amdgpu: kfd_topology.c warning: the frame size of 1408 bytes is larger than 1024 bytes Product: Drivers Version: 2.5 Hardware: All OS: Linux Status: NEW Severity: normal Priority: P3 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: bluesun...@gmail.com Regression: No Trying to compile Linux 6.6.2 with GCC 13.2.1 and CONFIG_WERROR=y: [...] drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In function 'kfd_topology_add_device': drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:2082:1: error: the frame size of 1408 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] 2082 | } | ^ cc1: all warnings being treated as errors [...] -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH V2] drm/modes: Fix divide error in drm_mode_debug_printmodeline
On Mon, Nov 20, 2023 at 10:41:18PM +0800, Edward Adam Davis wrote:
> [Syz Log]
> divide error: [#1] PREEMPT SMP KASAN
> CPU: 0 PID: 5068 Comm: syz-executor357 Not tainted
> 6.6.0-syzkaller-16039-gac347a0655db #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 10/09/2023
> RIP: 0010:drm_mode_vrefresh drivers/gpu/drm/drm_modes.c:1303 [inline]
> RIP: 0010:drm_mode_debug_printmodeline+0x118/0x4e0
> drivers/gpu/drm/drm_modes.c:60
> Code: 00 41 0f b7 07 66 83 f8 02 b9 01 00 00 00 0f 43 c8 0f b7 c1 0f af e8 44
> 89 f0 48 69 c8 e8 03 00 00 89 e8 d1 e8 48 01 c8 31 d2 <48> f7 f5 49 89 c6 eb
> 0c e8 fb 07 66 fc eb 05 e8 f4 07 66 fc 48 89
> RSP: 0018:c9000391f8d0 EFLAGS: 00010246
> RAX: 0001f400 RBX: 888025045000 RCX: 0001f400
> RDX: RSI: 8000 RDI: 888025045018
> RBP: R08: 8528b9af R09:
> R10: c9000391f8a0 R11: f52000723f17 R12: 0080
> R13: dc00 R14: 0080 R15: 888025045016
> FS: 56932380() GS:8880b980() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: 005fdeb8 CR3: 7fcff000 CR4: 003506f0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> Call Trace:
>
> drm_mode_setcrtc+0x83b/0x1880 drivers/gpu/drm/drm_crtc.c:794
> drm_ioctl_kernel+0x362/0x500 drivers/gpu/drm/drm_ioctl.c:792
> drm_ioctl+0x636/0xb00 drivers/gpu/drm/drm_ioctl.c:895
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:871 [inline]
> __se_sys_ioctl+0xf8/0x170 fs/ioctl.c:857
> do_syscall_x64 arch/x86/entry/common.c:51 [inline]
> do_syscall_64+0x44/0x110 arch/x86/entry/common.c:82
> entry_SYSCALL_64_after_hwframe+0x63/0x6b
>
> [Analysis]
> When calculating den in drm_mode_vrefresh(), if the vscan value is too large,
> there is a probability of unsigned integer overflow.
>
> [Fix]
> Before multiplying by vscan, first check if their product will overflow.
> If overflow occurs, return 0 and exit the subsequent process.
>
> Reported-and-tested-by: syzbot+2e93e6fb36e6fdc56...@syzkaller.appspotmail.com
> Fixes: ea40d7857d52 ("drm/vkms: fbdev emulation support")
> Signed-off-by: Edward Adam Davis
> ---
> drivers/gpu/drm/drm_modes.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index ac9a406250c5..60739d861da2 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -36,6 +36,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -1297,8 +1298,10 @@ int drm_mode_vrefresh(const struct drm_display_mode
> *mode)
> num *= 2;
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> den *= 2;
> - if (mode->vscan > 1)
> - den *= mode->vscan;
> + if (mode->vscan > 1) {
> + if (unlikely(check_mul_overflow(den, mode->vscan, &den)))
> + return 0;
> + }
I can't see any driver that actually supports vscan>1. Only
nouveau has some code for it, but doesn't look like it does
anything sensible. All other drivers for sure should be
rejecting vscan>1 outright. Which driver is this?
Is there an actual usecase where nouveau needs this (and does
it even work?) or could we just rip out the whole thing and
reject vscan>1 globally?
>
> return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(num, 1000), den);
> }
> --
> 2.25.1
--
Ville Syrjälä
Intel
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
Am 20.11.23 um 12:54 schrieb Thomas Zimmermann:
Hi
Am 17.11.23 um 22:44 schrieb Felix Kuehling:
This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3.
These helper functions are needed for KFD to export and import DMABufs
the right way without duplicating the tracking of DMABufs associated
with
GEM objects while ensuring that move notifier callbacks are working as
intended.
I'm unhappy to see these functions making a comeback. They are the
boiler-plate logic that all drivers should use. Historically, drivers
did a lot one things in their GEM code that was only semi-correct.
Unifying most of that made the memory management more readable. Not
giving back drivers to option of tinkering with this might be
preferable. The rsp hooks in struct drm_driver, prime_fd_to_handle and
prime_handle_to_fd, are only there for vmwgfx.
If you want to hook into prime import and export, there are
drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't it
possible to move the additional code behind these pointers?
No, the problem here is that the KFD code wants to create DMA-buf
exports for GEM buffers but from a different file descriptor than the
DRM render or primary node.
Essentially the KFD node is a separate file descriptor AMD GPUs came up
with for supporting compute.
Regards,
Christian.
Best regards
Thomas
CC: Christian König
CC: Thomas Zimmermann
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/drm_prime.c | 33 ++---
include/drm/drm_prime.h | 7 +++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 63b709a67471..834a5e28abbe 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
}
EXPORT_SYMBOL(drm_gem_dmabuf_release);
-/*
+/**
* drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
* @dev: drm_device to import into
* @file_priv: drm file-private structure
@@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
*
* Returns 0 on success or a negative error code on failure.
*/
-static int drm_gem_prime_fd_to_handle(struct drm_device *dev,
- struct drm_file *file_priv, int prime_fd,
- uint32_t *handle)
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+ struct drm_file *file_priv, int prime_fd,
+ uint32_t *handle)
{
struct dma_buf *dma_buf;
struct drm_gem_object *obj;
@@ -360,6 +360,7 @@ static int drm_gem_prime_fd_to_handle(struct
drm_device *dev,
dma_buf_put(dma_buf);
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -408,7 +409,7 @@ static struct dma_buf
*export_and_register_object(struct drm_device *dev,
return dmabuf;
}
-/*
+/**
* drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
* @dev: dev to export the buffer from
* @file_priv: drm file-private structure
@@ -421,10 +422,10 @@ static struct dma_buf
*export_and_register_object(struct drm_device *dev,
* The actual exporting from GEM object to a dma-buf is done
through the
* &drm_gem_object_funcs.export callback.
*/
-static int drm_gem_prime_handle_to_fd(struct drm_device *dev,
- struct drm_file *file_priv, uint32_t handle,
- uint32_t flags,
- int *prime_fd)
+int drm_gem_prime_handle_to_fd(struct drm_device *dev,
+ struct drm_file *file_priv, uint32_t handle,
+ uint32_t flags,
+ int *prime_fd)
{
struct drm_gem_object *obj;
int ret = 0;
@@ -506,6 +507,7 @@ static int drm_gem_prime_handle_to_fd(struct
drm_device *dev,
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -864,9 +866,9 @@ EXPORT_SYMBOL(drm_prime_get_contiguous_size);
* @obj: GEM object to export
* @flags: flags like DRM_CLOEXEC and DRM_RDWR
*
- * This is the implementation of the &drm_gem_object_funcs.export
functions
- * for GEM drivers using the PRIME helpers. It is used as the
default for
- * drivers that do not set their own.
+ * This is the implementation of the &drm_gem_object_funcs.export
functions for GEM drivers
+ * using the PRIME helpers. It is used as the default in
+ * drm_gem_prime_handle_to_fd().
*/
struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
int flags)
@@ -962,9 +964,10 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev);
* @dev: drm_device to import into
* @dma_buf: dma-buf object to import
*
- * This is the implementation of the gem_prime_import functions for GEM
- *
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
On 2023-11-20 6:54, Thomas Zimmermann wrote:
Hi
Am 17.11.23 um 22:44 schrieb Felix Kuehling:
This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3.
These helper functions are needed for KFD to export and import DMABufs
the right way without duplicating the tracking of DMABufs associated
with
GEM objects while ensuring that move notifier callbacks are working as
intended.
I'm unhappy to see these functions making a comeback. They are the
boiler-plate logic that all drivers should use. Historically, drivers
did a lot one things in their GEM code that was only semi-correct.
Unifying most of that made the memory management more readable. Not
giving back drivers to option of tinkering with this might be
preferable. The rsp hooks in struct drm_driver, prime_fd_to_handle and
prime_handle_to_fd, are only there for vmwgfx.
If you want to hook into prime import and export, there are
drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't it
possible to move the additional code behind these pointers?
I'm not trying to hook into these functions, I'm just trying to call
them. I'm not bringing back the .prime_handle_to_fd and
.prime_fd_to_handle hooks in struct drm_driver. I need a clean way to
export and import DMAbuffers from a kernel mode context. I had incorrect
or semi-correct ways of doing that by calling some driver-internal
functions, but then my DMABufs aren't correctly linked with GEM handles
in DRM and move notifiers in amdgpu aren't working correctly.
Regards,
Felix
Best regards
Thomas
CC: Christian König
CC: Thomas Zimmermann
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/drm_prime.c | 33 ++---
include/drm/drm_prime.h | 7 +++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 63b709a67471..834a5e28abbe 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
}
EXPORT_SYMBOL(drm_gem_dmabuf_release);
-/*
+/**
* drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
* @dev: drm_device to import into
* @file_priv: drm file-private structure
@@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
*
* Returns 0 on success or a negative error code on failure.
*/
-static int drm_gem_prime_fd_to_handle(struct drm_device *dev,
- struct drm_file *file_priv, int prime_fd,
- uint32_t *handle)
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+ struct drm_file *file_priv, int prime_fd,
+ uint32_t *handle)
{
struct dma_buf *dma_buf;
struct drm_gem_object *obj;
@@ -360,6 +360,7 @@ static int drm_gem_prime_fd_to_handle(struct
drm_device *dev,
dma_buf_put(dma_buf);
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -408,7 +409,7 @@ static struct dma_buf
*export_and_register_object(struct drm_device *dev,
return dmabuf;
}
-/*
+/**
* drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
* @dev: dev to export the buffer from
* @file_priv: drm file-private structure
@@ -421,10 +422,10 @@ static struct dma_buf
*export_and_register_object(struct drm_device *dev,
* The actual exporting from GEM object to a dma-buf is done
through the
* &drm_gem_object_funcs.export callback.
*/
-static int drm_gem_prime_handle_to_fd(struct drm_device *dev,
- struct drm_file *file_priv, uint32_t handle,
- uint32_t flags,
- int *prime_fd)
+int drm_gem_prime_handle_to_fd(struct drm_device *dev,
+ struct drm_file *file_priv, uint32_t handle,
+ uint32_t flags,
+ int *prime_fd)
{
struct drm_gem_object *obj;
int ret = 0;
@@ -506,6 +507,7 @@ static int drm_gem_prime_handle_to_fd(struct
drm_device *dev,
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -864,9 +866,9 @@ EXPORT_SYMBOL(drm_prime_get_contiguous_size);
* @obj: GEM object to export
* @flags: flags like DRM_CLOEXEC and DRM_RDWR
*
- * This is the implementation of the &drm_gem_object_funcs.export
functions
- * for GEM drivers using the PRIME helpers. It is used as the
default for
- * drivers that do not set their own.
+ * This is the implementation of the &drm_gem_object_funcs.export
functions for GEM drivers
+ * using the PRIME helpers. It is used as the default in
+ * drm_gem_prime_handle_to_fd().
*/
struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
int flags)
@@ -962,9 +964,10 @@
[PATCH v3] drm/test: add a test suite for GEM objects backed by shmem
This patch introduces an initial KUnit test suite for GEM objects
backed by shmem buffers.
Suggested-by: Javier Martinez Canillas
Signed-off-by: Marco Pagani
v3:
- Explicitly cast pointers in the helpers
- Removed unused pointer to parent dev in struct fake_dev
- Test entries reordering in Kconfig and Makefile sent as a separate patch
v2:
- Improved description of test cases
- Cleaner error handling using KUnit actions
- Alphabetical order in Kconfig and Makefile
---
drivers/gpu/drm/Kconfig| 1 +
drivers/gpu/drm/tests/Makefile | 1 +
drivers/gpu/drm/tests/drm_gem_shmem_test.c | 385 +
3 files changed, 387 insertions(+)
create mode 100644 drivers/gpu/drm/tests/drm_gem_shmem_test.c
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index cdbc56e07649..8f0061a0c6c1 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -80,6 +80,7 @@ config DRM_KUNIT_TEST
select DRM_DISPLAY_HELPER
select DRM_EXEC
select DRM_EXPORT_FOR_TESTS if m
+ select DRM_GEM_SHMEM_HELPER
select DRM_KMS_HELPER
select DRM_KUNIT_TEST_HELPERS
select DRM_LIB_RANDOM
diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
index 2645af241ff0..d6183b3d7688 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
drm_format_helper_test.o \
drm_format_test.o \
drm_framebuffer_test.o \
+ drm_gem_shmem_test.o \
drm_managed_test.o \
drm_mm_test.o \
drm_modes_test.o \
diff --git a/drivers/gpu/drm/tests/drm_gem_shmem_test.c
b/drivers/gpu/drm/tests/drm_gem_shmem_test.c
new file mode 100644
index ..a60c2a28a92a
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_gem_shmem_test.c
@@ -0,0 +1,385 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test suite for GEM objects backed by shmem buffers
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani
+ */
+
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#define TEST_SIZE SZ_1M
+#define TEST_BYTE 0xae
+
+/*
+ * Wrappers to avoid an explicit type casting when passing action
+ * functions to kunit_add_action().
+ */
+static void kfree_wrapper(void *ptr)
+{
+ const void *obj = ptr;
+
+ kfree(obj);
+}
+
+static void sg_free_table_wrapper(void *ptr)
+{
+ struct sg_table *sgt = ptr;
+
+ sg_free_table(sgt);
+}
+
+static void drm_gem_shmem_free_wrapper(void *ptr)
+{
+ struct drm_gem_shmem_object *shmem = ptr;
+
+ drm_gem_shmem_free(shmem);
+}
+
+/*
+ * Test creating a shmem GEM object backed by shmem buffer. The test
+ * case succeeds if the GEM object is successfully allocated with the
+ * shmem file node and object functions attributes set, and the size
+ * attribute is equal to the correct size.
+ */
+static void drm_gem_shmem_test_obj_create(struct kunit *test)
+{
+ struct drm_device *drm_dev = test->priv;
+ struct drm_gem_shmem_object *shmem;
+
+ shmem = drm_gem_shmem_create(drm_dev, TEST_SIZE);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, shmem);
+ KUNIT_EXPECT_EQ(test, shmem->base.size, TEST_SIZE);
+ KUNIT_EXPECT_NOT_NULL(test, shmem->base.filp);
+ KUNIT_EXPECT_NOT_NULL(test, shmem->base.funcs);
+
+ drm_gem_shmem_free(shmem);
+}
+
+/*
+ * Test creating a shmem GEM object from a scatter/gather table exported
+ * via a DMA-BUF. The test case succeed if the GEM object is successfully
+ * created with the shmem file node attribute equal to NULL and the sgt
+ * attribute pointing to the scatter/gather table that has been imported.
+ */
+static void drm_gem_shmem_test_obj_create_private(struct kunit *test)
+{
+ struct drm_device *drm_dev = test->priv;
+ struct drm_gem_shmem_object *shmem;
+ struct drm_gem_object *gem_obj;
+ struct dma_buf buf_mock;
+ struct dma_buf_attachment attach_mock;
+ struct sg_table *sgt;
+ char *buf;
+ int ret;
+
+ /* Create a mock scatter/gather table */
+ buf = kunit_kzalloc(test, TEST_SIZE, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, buf);
+
+ sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, sgt);
+
+ ret = kunit_add_action_or_reset(test, kfree_wrapper, sgt);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = kunit_add_action_or_reset(test, sg_free_table_wrapper, sgt);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ sg_init_one(sgt->sgl, buf, TEST_SIZE);
+
+ /* Init a mock DMA-BUF */
+ buf_mock.size = TEST_SIZE;
+ attach_mock.dmabuf = &buf_mock;
+
+ gem_obj = drm_gem_shmem_prime_import_sg_table(drm_dev, &attach_mock,
sgt);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, gem_obj);
+ KUNIT_EXPECT
[PATCH V2] drm/modes: Fix divide error in drm_mode_debug_printmodeline
[Syz Log]
divide error: [#1] PREEMPT SMP KASAN
CPU: 0 PID: 5068 Comm: syz-executor357 Not tainted
6.6.0-syzkaller-16039-gac347a0655db #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
10/09/2023
RIP: 0010:drm_mode_vrefresh drivers/gpu/drm/drm_modes.c:1303 [inline]
RIP: 0010:drm_mode_debug_printmodeline+0x118/0x4e0
drivers/gpu/drm/drm_modes.c:60
Code: 00 41 0f b7 07 66 83 f8 02 b9 01 00 00 00 0f 43 c8 0f b7 c1 0f af e8 44
89 f0 48 69 c8 e8 03 00 00 89 e8 d1 e8 48 01 c8 31 d2 <48> f7 f5 49 89 c6 eb 0c
e8 fb 07 66 fc eb 05 e8 f4 07 66 fc 48 89
RSP: 0018:c9000391f8d0 EFLAGS: 00010246
RAX: 0001f400 RBX: 888025045000 RCX: 0001f400
RDX: RSI: 8000 RDI: 888025045018
RBP: R08: 8528b9af R09:
R10: c9000391f8a0 R11: f52000723f17 R12: 0080
R13: dc00 R14: 0080 R15: 888025045016
FS: 56932380() GS:8880b980() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 005fdeb8 CR3: 7fcff000 CR4: 003506f0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
Call Trace:
drm_mode_setcrtc+0x83b/0x1880 drivers/gpu/drm/drm_crtc.c:794
drm_ioctl_kernel+0x362/0x500 drivers/gpu/drm/drm_ioctl.c:792
drm_ioctl+0x636/0xb00 drivers/gpu/drm/drm_ioctl.c:895
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:871 [inline]
__se_sys_ioctl+0xf8/0x170 fs/ioctl.c:857
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x44/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
[Analysis]
When calculating den in drm_mode_vrefresh(), if the vscan value is too large,
there is a probability of unsigned integer overflow.
[Fix]
Before multiplying by vscan, first check if their product will overflow.
If overflow occurs, return 0 and exit the subsequent process.
Reported-and-tested-by: syzbot+2e93e6fb36e6fdc56...@syzkaller.appspotmail.com
Fixes: ea40d7857d52 ("drm/vkms: fbdev emulation support")
Signed-off-by: Edward Adam Davis
---
drivers/gpu/drm/drm_modes.c | 7 +--
1 file changed, 5 insertions(+), 2 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ac9a406250c5..60739d861da2 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1297,8 +1298,10 @@ int drm_mode_vrefresh(const struct drm_display_mode
*mode)
num *= 2;
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
den *= 2;
- if (mode->vscan > 1)
- den *= mode->vscan;
+ if (mode->vscan > 1) {
+ if (unlikely(check_mul_overflow(den, mode->vscan, &den)))
+ return 0;
+ }
return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(num, 1000), den);
}
--
2.25.1
Re: [PATCH v2] drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full
On Thursday, 26 October 2023 21:14:58 CET Jonas Karlman wrote: > Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328 > and RK3399 result in wrong colors being displayed. > > The issue can be observed using modetest: > > modetest -s @:1920x1080-60@RG24 > modetest -s @:1920x1080-60@BG24 On my Rock64 (rk3328) I was able to see the problem without this patch and see it displaying correctly with it, so Tested-by: Diederik de Haas signature.asc Description: This is a digitally signed message part.
Re: [PATCH 0/4] drm/i915: Fix LUT rounding
On Fri, Oct 13, 2023 at 04:13:58PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > The current LUT rounding is generating weird results. Adjust > it to follow the OpenGL int<->float conversion rules. > > Ville Syrjälä (4): > drm: Fix color LUT rounding ^ I'd like to merge this via drm-intel-next as needs to match the rounding done in the readout path in i915. Maarten,Maxime,Thomas can I get an ack for that? > drm/i915: Adjust LUT rounding rules > drm/i915: s/clamp()/min()/ in i965_lut_11p6_max_pack() > drm/i915: Fix glk+ degamma LUT conversions > > drivers/gpu/drm/i915/display/intel_color.c | 70 +++--- > include/drm/drm_color_mgmt.h | 18 +++--- > 2 files changed, 42 insertions(+), 46 deletions(-) > > -- > 2.41.0 -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 1/4] drm: Fix color LUT rounding
On Mon, Nov 20, 2023 at 01:17:05PM +, Borah, Chaitanya Kumar wrote: > > > > -Original Message- > > From: Borah, Chaitanya Kumar > > Sent: Monday, November 20, 2023 6:33 PM > > To: Ville Syrjälä > > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Jani > > Nikula > > Subject: RE: [Intel-gfx] [PATCH 1/4] drm: Fix color LUT rounding > > > > Hello Ville, > > > > > -Original Message- > > > From: dri-devel On Behalf Of > > > Ville Syrjälä > > > Sent: Tuesday, October 31, 2023 9:37 PM > > > To: Jani Nikula > > > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH 1/4] drm: Fix color LUT rounding > > > > > > On Tue, Oct 31, 2023 at 11:15:35AM +0200, Jani Nikula wrote: > > > > On Fri, 13 Oct 2023, Ville Syrjala > > > > wrote: > > > > > entrirely. But perhaps a better idea would be to follow the OpenGL > > > > > int<->float conversion rules, in which case we get the following > > > > > results: > > > > > > > > Do you have a pointer to the rules handy, I couldn't find it. :( > > > > > > Eg. '2.3.5 Fixed-Point Data Conversions' in GL 4.6 spec. The section > > > number probably changes depending on which version of the spec you look > > at. > > > > > > > This section particularly talks about conversion of normalized fixed point > > to > > floating point numbers and vice versa. > > Pardon my limited knowledge on the topic but aren't we just doing a scaling > > factor conversion(Q0.16 -> Q0.8) in these patches? > > > > I could not draw a direct relation between the formulas in the section[1] > > and > > what we are doing here.(but it could be just me!) > > Scratch that! As I understand, in effect we are doing a Q0.16 Fixed Point -> > Floating point -> Q0.8 Fixed Point conversion. Yep, that's it. > Correct me if I am wrong! Otherwise > > LGTM. > > Reviewed-by: Chaitanya Kumar Borah > > > > > > Regards > > > > Chaitanya > > > > [1] https://registry.khronos.org/OpenGL/specs/gl/glspec46.core.pdf '2.3.5 > > Fixed-Point Data Conversions' > > > > > > > > > > Might also add the reference to the commit message and/or comment. > > > > > > > > BR, > > > > Jani. > > > > > > > > -- > > > > Jani Nikula, Intel > > > > > > -- > > > Ville Syrjälä > > > Intel -- Ville Syrjälä Intel
Re: [PATCH 2/4] drm/i915: Adjust LUT rounding rules
On Mon, Nov 20, 2023 at 06:08:57AM +, Borah, Chaitanya Kumar wrote:
> Hello Ville,
>
> > -Original Message-
> > From: dri-devel On Behalf Of Ville
> > Syrjala
> > Sent: Friday, October 13, 2023 6:44 PM
> > To: intel-...@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org
> > Subject: [PATCH 2/4] drm/i915: Adjust LUT rounding rules
> >
> > From: Ville Syrjälä
> >
> > drm_color_lut_extract() rounding was changed to follow the OpenGL int<-
> > >float conversion rules. Adjust intel_color_lut_pack() to match.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/display/intel_color.c | 14 ++
> > 1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > b/drivers/gpu/drm/i915/display/intel_color.c
> > index 2a2a163ea652..b01f463af861 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -785,14 +785,12 @@ static void chv_assign_csc(struct intel_crtc_state
> > *crtc_state)
> > /* convert hw value with given bit_precision to lut property val */
> > static u32
> > intel_color_lut_pack(u32 val, int bit_precision) {
>
> Is this operation unique to Intel. Should there be a drm helper for this?
If some other driver gains gamma readout support they
could probably use something like this. The other option
would be to rework the current helper to allow conversions
both ways.
>
> Regards
>
> Chaitanya
>
> > - u32 max = 0x >> (16 - bit_precision);
> > -
> > - val = clamp_val(val, 0, max);
> > -
> > - if (bit_precision < 16)
> > - val <<= 16 - bit_precision;
> > -
> > - return val;
> > + if (bit_precision > 16)
> > + return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(val, (1 << 16)
> > - 1),
> > +(1 << bit_precision) - 1);
> > + else
> > + return DIV_ROUND_CLOSEST(val * ((1 << 16) - 1),
> > +(1 << bit_precision) - 1);
> > }
> >
> > static u32 i9xx_lut_8(const struct drm_color_lut *color)
> > --
> > 2.41.0
>
--
Ville Syrjälä
Intel
Re: [PATCH v3 12/20] drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c: remove I2C_CLASS_DDC support
Il 19/11/23 12:28, Heiner Kallweit ha scritto: After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Signed-off-by: Heiner Kallweit Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH] drm/rockchip: lvds: do not print error message when deferring probe
Hi Fabio,
On 11/17/23 20:27, Fabio Estevam wrote:
Hi Quentin,
On Fri, Nov 17, 2023 at 3:31 PM Quentin Schulz wrote:
From: Quentin Schulz
This scary message may happen if the panel or bridge is not probed
before the LVDS controller is, resulting in some head scratching because
the LVDS panel is actually working, since a later try will eventually
find the panel or bridge.
Therefore let's demote this error message into a debug message to not
scare users unnecessarily.
...
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c
b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index f0f47e9abf5a..52e2ce2a61a8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -577,7 +577,7 @@ static int rockchip_lvds_bind(struct device *dev, struct
device *master,
ret = -EINVAL;
goto err_put_port;
} else if (ret) {
- DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n");
+ DRM_DEV_DEBUG(dev, "failed to find panel and bridge node\n");
ret = -EPROBE_DEFER;
What about using dev_err_probe() instead?
Either is fine by me, will send a v2 and DRM maintainers can decide for
themselves before merging :)
Cheers,
Quentin
Re: [PATCH v2 00/11] drm/i915: Fix UHBR data,link M/N/TU and PBN values
On Mon, 20 Nov 2023, Imre Deak wrote:
> On Mon, Nov 20, 2023 at 02:31:34PM +0200, Jani Nikula wrote:
>> On Thu, 16 Nov 2023, Imre Deak wrote:
>> > This is v2 of [1], with the following changes:
>> > - Store the pbn_div value in fixed point format.
>> > - Fix PBN calculation in patch 8.
>> > - Reuse intel_dp_max_data_rate(), intel_dp_effective_data_rate() in
>> > intel_link_compute_m_n() (Jani).
>> >
>> > [1] https://lore.kernel.org/all/20231113201110.510724-1-imre.d...@intel.com
>> >
>> > Cc: Arun R Murthy
>> > Cc: Jani Nikula
>> > Cc: Lyude Paul
>> >
>> > Imre Deak (11):
>> > drm/dp_mst: Store the MST PBN divider value in fixed point format
>> > drm/dp_mst: Fix PBN divider calculation for UHBR rates
>> > drm/dp_mst: Add kunit tests for drm_dp_get_vc_payload_bw()
>>
>> Maarten, Maxime, Thomas, ack for merging these three via drm-intel-next?
>>
>> Imre, I note that said patches were Cc: dri-devel, but for future
>> reference please Cc: the entire series to dri-devel when you include
>> dependencies that you plan to merge via drm-intel.
>
> Ok. I assumed the alternative to merge the 3 patches via drm-misc-next,
> wait for that to get merged back to i915 and then merge the rest to i915
> was still a preferred way; wondering now if in general this is better to
> avoid merge conflicts similar to the one reported now wrt.
> "drm/dp_mst: Fix fractional DSC bpp handling".
>
> In any case yes, I can CC dri-devel the whole patchset whenever there
> are any drm changes in it. While still wondering about the ideal
> approach above, I'd still prefer if the 3 drm patches in this one could
> also get merged via the i915 tree.
There are no rigid rules for this.
But it's clearly more problematic when the patches touch not just drm +
one driver, but drm + several drivers. Perhaps that's an indication they
should be merged via drm-misc-next instead of a driver tree.
Also, we probably need to clear up the existing conflicts before adding
more patches in the area to drm-intel-next.
BR,
Jani.
>
> --Imre
>
>> BR,
>> Jani.
>>
>>
>> > drm/i915/dp: Replace intel_dp_is_uhbr_rate() with
>> > drm_dp_is_uhbr_rate()
>> > drm/i915/dp: Account for channel coding efficiency on UHBR links
>> > drm/i915/dp: Fix UHBR link M/N values
>> > drm/i915/dp_mst: Calculate the BW overhead in
>> > intel_dp_mst_find_vcpi_slots_for_bpp()
>> > drm/i915/dp_mst: Fix PBN / MTP_TU size calculation for UHBR rates
>> > drm/i915/dp: Report a rounded-down value as the maximum data rate
>> > drm/i915/dp: Simplify intel_dp_max_data_rate()
>> > drm/i915/dp: Reuse intel_dp_{max,effective}_data_rate in
>> > intel_link_compute_m_n()
>> >
>> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +-
>> > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 3 +-
>> > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 +-
>> > drivers/gpu/drm/display/drm_dp_mst_topology.c | 31 +++-
>> > drivers/gpu/drm/i915/display/intel_display.c | 51 ++
>> > drivers/gpu/drm/i915/display/intel_dp.c | 78 +++---
>> > drivers/gpu/drm/i915/display/intel_dp.h | 5 +-
>> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 55 +--
>> > drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +-
>> > .../gpu/drm/tests/drm_dp_mst_helper_test.c| 145 ++
>> > include/drm/display/drm_dp_helper.h | 13 ++
>> > include/drm/display/drm_dp_mst_helper.h | 7 +-
>> > 12 files changed, 311 insertions(+), 93 deletions(-)
>>
>> --
>> Jani Nikula, Intel
--
Jani Nikula, Intel
RE: [Intel-gfx] [PATCH 1/4] drm: Fix color LUT rounding
> -Original Message- > From: Borah, Chaitanya Kumar > Sent: Monday, November 20, 2023 6:33 PM > To: Ville Syrjälä > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Jani > Nikula > Subject: RE: [Intel-gfx] [PATCH 1/4] drm: Fix color LUT rounding > > Hello Ville, > > > -Original Message- > > From: dri-devel On Behalf Of > > Ville Syrjälä > > Sent: Tuesday, October 31, 2023 9:37 PM > > To: Jani Nikula > > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH 1/4] drm: Fix color LUT rounding > > > > On Tue, Oct 31, 2023 at 11:15:35AM +0200, Jani Nikula wrote: > > > On Fri, 13 Oct 2023, Ville Syrjala wrote: > > > > entrirely. But perhaps a better idea would be to follow the OpenGL > > > > int<->float conversion rules, in which case we get the following > > > > results: > > > > > > Do you have a pointer to the rules handy, I couldn't find it. :( > > > > Eg. '2.3.5 Fixed-Point Data Conversions' in GL 4.6 spec. The section > > number probably changes depending on which version of the spec you look > at. > > > > This section particularly talks about conversion of normalized fixed point to > floating point numbers and vice versa. > Pardon my limited knowledge on the topic but aren't we just doing a scaling > factor conversion(Q0.16 -> Q0.8) in these patches? > > I could not draw a direct relation between the formulas in the section[1] and > what we are doing here.(but it could be just me!) Scratch that! As I understand, in effect we are doing a Q0.16 Fixed Point -> Floating point -> Q0.8 Fixed Point conversion. Correct me if I am wrong! Otherwise LGTM. Reviewed-by: Chaitanya Kumar Borah > > Regards > > Chaitanya > > [1] https://registry.khronos.org/OpenGL/specs/gl/glspec46.core.pdf '2.3.5 > Fixed-Point Data Conversions' > > > > > > > Might also add the reference to the commit message and/or comment. > > > > > > BR, > > > Jani. > > > > > > -- > > > Jani Nikula, Intel > > > > -- > > Ville Syrjälä > > Intel
Re: [PATCH v2 00/11] drm/i915: Fix UHBR data,link M/N/TU and PBN values
On Mon, Nov 20, 2023 at 02:31:34PM +0200, Jani Nikula wrote:
> On Thu, 16 Nov 2023, Imre Deak wrote:
> > This is v2 of [1], with the following changes:
> > - Store the pbn_div value in fixed point format.
> > - Fix PBN calculation in patch 8.
> > - Reuse intel_dp_max_data_rate(), intel_dp_effective_data_rate() in
> > intel_link_compute_m_n() (Jani).
> >
> > [1] https://lore.kernel.org/all/20231113201110.510724-1-imre.d...@intel.com
> >
> > Cc: Arun R Murthy
> > Cc: Jani Nikula
> > Cc: Lyude Paul
> >
> > Imre Deak (11):
> > drm/dp_mst: Store the MST PBN divider value in fixed point format
> > drm/dp_mst: Fix PBN divider calculation for UHBR rates
> > drm/dp_mst: Add kunit tests for drm_dp_get_vc_payload_bw()
>
> Maarten, Maxime, Thomas, ack for merging these three via drm-intel-next?
>
> Imre, I note that said patches were Cc: dri-devel, but for future
> reference please Cc: the entire series to dri-devel when you include
> dependencies that you plan to merge via drm-intel.
Ok. I assumed the alternative to merge the 3 patches via drm-misc-next,
wait for that to get merged back to i915 and then merge the rest to i915
was still a preferred way; wondering now if in general this is better to
avoid merge conflicts similar to the one reported now wrt.
"drm/dp_mst: Fix fractional DSC bpp handling".
In any case yes, I can CC dri-devel the whole patchset whenever there
are any drm changes in it. While still wondering about the ideal
approach above, I'd still prefer if the 3 drm patches in this one could
also get merged via the i915 tree.
--Imre
> BR,
> Jani.
>
>
> > drm/i915/dp: Replace intel_dp_is_uhbr_rate() with
> > drm_dp_is_uhbr_rate()
> > drm/i915/dp: Account for channel coding efficiency on UHBR links
> > drm/i915/dp: Fix UHBR link M/N values
> > drm/i915/dp_mst: Calculate the BW overhead in
> > intel_dp_mst_find_vcpi_slots_for_bpp()
> > drm/i915/dp_mst: Fix PBN / MTP_TU size calculation for UHBR rates
> > drm/i915/dp: Report a rounded-down value as the maximum data rate
> > drm/i915/dp: Simplify intel_dp_max_data_rate()
> > drm/i915/dp: Reuse intel_dp_{max,effective}_data_rate in
> > intel_link_compute_m_n()
> >
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +-
> > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 3 +-
> > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 +-
> > drivers/gpu/drm/display/drm_dp_mst_topology.c | 31 +++-
> > drivers/gpu/drm/i915/display/intel_display.c | 51 ++
> > drivers/gpu/drm/i915/display/intel_dp.c | 78 +++---
> > drivers/gpu/drm/i915/display/intel_dp.h | 5 +-
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 55 +--
> > drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +-
> > .../gpu/drm/tests/drm_dp_mst_helper_test.c| 145 ++
> > include/drm/display/drm_dp_helper.h | 13 ++
> > include/drm/display/drm_dp_mst_helper.h | 7 +-
> > 12 files changed, 311 insertions(+), 93 deletions(-)
>
> --
> Jani Nikula, Intel
[ANNOUNCE] libdrm 2.4.118
David Jagu (1):
meson: fix typo in libdrm_intel
Geert Uytterhoeven (18):
util: improve SMPTE color LUT accuracy
util: factor out and optimize C8 SMPTE color LUT
util: add support for DRM_FORMAT_C[124]
util: store number of colors for indexed formats
util: add SMPTE pattern support for C4 format
util: add SMPTE pattern support for C1 format
util: add SMPTE pattern support for C2 format
modetest: add support for DRM_FORMAT_C[124]
modetest: add SMPTE pattern support for C[124] formats
intel: determine target endianness using meson
util: fix 32 bpp patterns on big-endian
util: fix 16 bpp patterns on big-endian
util: add missing big-endian RGB16 frame buffer formats
modetest: add support for parsing big-endian formats
util: add test pattern support for big-endian XRGB1555/RGB565
util: fix pwetty on big-endian
util: add pwetty support for big-endian RGB565
modetest: add support for big-endian XRGB1555/RGB565
Jonas Karlman (1):
modetest: add support for DRM_FORMAT_NV{15,20,30}
Neil Armstrong (1):
modetest: switch usage to proper options grammar
Simon Ser (4):
xf86drm: add drmGetNodeTypeFromDevId
Sync headers with drm-next
xf86drmMode: add drmModeCloseFB()
build: bump version to 2.4.118
git tag: libdrm-2.4.118
https://dri.freedesktop.org/libdrm/libdrm-2.4.118.tar.xz
SHA256: a777bd85f2b5fc9c57f886c82058300578317cafdbc77d0a769d7e9a9567ab88
libdrm-2.4.118.tar.xz
SHA512:
2740ec10dfe96b520345c3f6f0d99a30aac95b1f96656bd9cd11269c2a83a9dac423da29d74a3deb55360e3ae2ca4a1de283e1e443667bedd22673f6629c9920
libdrm-2.4.118.tar.xz
PGP: https://dri.freedesktop.org/libdrm/libdrm-2.4.118.tar.xz.sig
RE: [Intel-gfx] [PATCH 1/4] drm: Fix color LUT rounding
Hello Ville, > -Original Message- > From: dri-devel On Behalf Of Ville > Syrjälä > Sent: Tuesday, October 31, 2023 9:37 PM > To: Jani Nikula > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 1/4] drm: Fix color LUT rounding > > On Tue, Oct 31, 2023 at 11:15:35AM +0200, Jani Nikula wrote: > > On Fri, 13 Oct 2023, Ville Syrjala wrote: > > > entrirely. But perhaps a better idea would be to follow the OpenGL > > > int<->float conversion rules, in which case we get the following > > > results: > > > > Do you have a pointer to the rules handy, I couldn't find it. :( > > Eg. '2.3.5 Fixed-Point Data Conversions' in GL 4.6 spec. The section number > probably changes depending on which version of the spec you look at. > This section particularly talks about conversion of normalized fixed point to floating point numbers and vice versa. Pardon my limited knowledge on the topic but aren't we just doing a scaling factor conversion(Q0.16 -> Q0.8) in these patches? I could not draw a direct relation between the formulas in the section[1] and what we are doing here.(but it could be just me!) Regards Chaitanya [1] https://registry.khronos.org/OpenGL/specs/gl/glspec46.core.pdf '2.3.5 Fixed-Point Data Conversions' > > > > Might also add the reference to the commit message and/or comment. > > > > BR, > > Jani. > > > > -- > > Jani Nikula, Intel > > -- > Ville Syrjälä > Intel
[PATCH v5 03/11] drm/dp_mst: Add kunit tests for drm_dp_get_vc_payload_bw()
Add kunit test cases for drm_dp_get_vc_payload_bw() with all the DP1.4
and UHBR link configurations.
v2:
- List test cases in decreasing rate,lane count order matching the
corresponding DP Standard tables. (Ville)
- Add references to the DP Standard tables.
v3:
- Sort the testcases properly.
v4:
- Avoid 'stack frame size x exceeds limit y in
drm_test_dp_mst_calc_pbn_div()' compiler warn. (LKP)
Cc: Ville Syrjälä
Cc: Lyude Paul
Cc: dri-devel@lists.freedesktop.org
Cc: kernel test robot
Reviewed-by: Ville Syrjälä (v3)
Signed-off-by: Imre Deak
---
.../gpu/drm/tests/drm_dp_mst_helper_test.c| 160 ++
1 file changed, 160 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
index e3c818dfc0e6d..d916e548fcb12 100644
--- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
@@ -68,6 +68,152 @@ static void dp_mst_calc_pbn_mode_desc(const struct
drm_dp_mst_calc_pbn_mode_test
KUNIT_ARRAY_PARAM(drm_dp_mst_calc_pbn_mode, drm_dp_mst_calc_pbn_mode_cases,
dp_mst_calc_pbn_mode_desc);
+struct drm_dp_mst_calc_pbn_div_test {
+ int link_rate;
+ int lane_count;
+ fixed20_12 expected;
+};
+
+#define fp_init(__int, __frac) { \
+ .full = (__int) * (1 << 12) + \
+ (__frac) * (1 << 12) / 10 \
+}
+
+static const struct drm_dp_mst_calc_pbn_div_test
drm_dp_mst_calc_pbn_div_dp1_4_cases[] = {
+ /*
+* UHBR rates (DP Standard v2.1 2.7.6.3, specifying the rounded to
+* closest value to 2 decimal places):
+* .expected = .link_rate * .lane_count * 0.9671 / 8 / 54 / 100
+* DP1.4 rates (DP Standard v2.1 2.6.4.2):
+* .expected = .link_rate * .lane_count * 0.8000 / 8 / 54 / 100
+*
+* truncated to 5 decimal places.
+*/
+ {
+ .link_rate = 200,
+ .lane_count = 4,
+ .expected = fp_init(179, 9259), /* 179.09259 */
+ },
+ {
+ .link_rate = 200,
+ .lane_count = 2,
+ .expected = fp_init(89, 54629),
+ },
+ {
+ .link_rate = 200,
+ .lane_count = 1,
+ .expected = fp_init(44, 77314),
+ },
+ {
+ .link_rate = 135,
+ .lane_count = 4,
+ .expected = fp_init(120, 88750),
+ },
+ {
+ .link_rate = 135,
+ .lane_count = 2,
+ .expected = fp_init(60, 44375),
+ },
+ {
+ .link_rate = 135,
+ .lane_count = 1,
+ .expected = fp_init(30, 22187),
+ },
+ {
+ .link_rate = 100,
+ .lane_count = 4,
+ .expected = fp_init(89, 54629),
+ },
+ {
+ .link_rate = 100,
+ .lane_count = 2,
+ .expected = fp_init(44, 77314),
+ },
+ {
+ .link_rate = 100,
+ .lane_count = 1,
+ .expected = fp_init(22, 38657),
+ },
+ {
+ .link_rate = 81,
+ .lane_count = 4,
+ .expected = fp_init(60, 0),
+ },
+ {
+ .link_rate = 81,
+ .lane_count = 2,
+ .expected = fp_init(30, 0),
+ },
+ {
+ .link_rate = 81,
+ .lane_count = 1,
+ .expected = fp_init(15, 0),
+ },
+ {
+ .link_rate = 54,
+ .lane_count = 4,
+ .expected = fp_init(40, 0),
+ },
+ {
+ .link_rate = 54,
+ .lane_count = 2,
+ .expected = fp_init(20, 0),
+ },
+ {
+ .link_rate = 54,
+ .lane_count = 1,
+ .expected = fp_init(10, 0),
+ },
+ {
+ .link_rate = 27,
+ .lane_count = 4,
+ .expected = fp_init(20, 0),
+ },
+ {
+ .link_rate = 27,
+ .lane_count = 2,
+ .expected = fp_init(10, 0),
+ },
+ {
+ .link_rate = 27,
+ .lane_count = 1,
+ .expected = fp_init(5, 0),
+ },
+ {
+ .link_rate = 162000,
+ .lane_count = 4,
+ .expected = fp_init(12, 0),
+ },
+ {
+ .link_rate = 162000,
+ .lane_count = 2,
+ .expected = fp_init(6, 0),
+ },
+ {
+ .link_rate = 162000,
+ .lane_count = 1,
+ .expected = fp_init(3, 0),
+ },
+};
+
+static void drm_test_dp_mst_calc_pbn_div(struct kunit *test)
+{
+ const struct drm_dp_mst_calc_pbn_div_test *params = test->param_value;
+ /* mgr->dev is only needed by drm_dbg_kms(),
Re: [PATCH v2 00/11] drm/i915: Fix UHBR data,link M/N/TU and PBN values
On Thu, 16 Nov 2023, Imre Deak wrote:
> This is v2 of [1], with the following changes:
> - Store the pbn_div value in fixed point format.
> - Fix PBN calculation in patch 8.
> - Reuse intel_dp_max_data_rate(), intel_dp_effective_data_rate() in
> intel_link_compute_m_n() (Jani).
>
> [1] https://lore.kernel.org/all/20231113201110.510724-1-imre.d...@intel.com
>
> Cc: Arun R Murthy
> Cc: Jani Nikula
> Cc: Lyude Paul
>
> Imre Deak (11):
> drm/dp_mst: Store the MST PBN divider value in fixed point format
> drm/dp_mst: Fix PBN divider calculation for UHBR rates
> drm/dp_mst: Add kunit tests for drm_dp_get_vc_payload_bw()
Maarten, Maxime, Thomas, ack for merging these three via drm-intel-next?
Imre, I note that said patches were Cc: dri-devel, but for future
reference please Cc: the entire series to dri-devel when you include
dependencies that you plan to merge via drm-intel.
BR,
Jani.
> drm/i915/dp: Replace intel_dp_is_uhbr_rate() with
> drm_dp_is_uhbr_rate()
> drm/i915/dp: Account for channel coding efficiency on UHBR links
> drm/i915/dp: Fix UHBR link M/N values
> drm/i915/dp_mst: Calculate the BW overhead in
> intel_dp_mst_find_vcpi_slots_for_bpp()
> drm/i915/dp_mst: Fix PBN / MTP_TU size calculation for UHBR rates
> drm/i915/dp: Report a rounded-down value as the maximum data rate
> drm/i915/dp: Simplify intel_dp_max_data_rate()
> drm/i915/dp: Reuse intel_dp_{max,effective}_data_rate in
> intel_link_compute_m_n()
>
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +-
> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 3 +-
> .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 +-
> drivers/gpu/drm/display/drm_dp_mst_topology.c | 31 +++-
> drivers/gpu/drm/i915/display/intel_display.c | 51 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 78 +++---
> drivers/gpu/drm/i915/display/intel_dp.h | 5 +-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 55 +--
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +-
> .../gpu/drm/tests/drm_dp_mst_helper_test.c| 145 ++
> include/drm/display/drm_dp_helper.h | 13 ++
> include/drm/display/drm_dp_mst_helper.h | 7 +-
> 12 files changed, 311 insertions(+), 93 deletions(-)
--
Jani Nikula, Intel
[PATCH v2 2/2] drm/rockchip: lvds: do not print scary message when probing defer
From: Quentin Schulz
This scary message can misled the user into thinking something bad has
happened and needs to be fixed, however it could simply be part of a
normal boot process where EPROBE_DEFER is taken into account. Therefore,
let's use dev_err_probe so that this message doesn't get shown (by
default) when the return code is EPROBE_DEFER.
Fixes: 34cc0aa25456 ("drm/rockchip: Add support for Rockchip Soc LVDS")
Cc: Quentin Schulz
Signed-off-by: Quentin Schulz
---
drivers/gpu/drm/rockchip/rockchip_lvds.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c
b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 17d8fc797151..f2831d304e7b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -577,7 +577,7 @@ static int rockchip_lvds_bind(struct device *dev, struct
device *master,
ret = -EINVAL;
goto err_put_port;
} else if (ret) {
- DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n");
+ dev_err_probe(dev, ret, "failed to find panel and bridge
node\n");
goto err_put_port;
}
if (lvds->panel)
--
2.42.0
[PATCH v2 1/2] drm/rockchip: lvds: do not overwrite error code
From: Quentin Schulz
ret variable stores the return value of drm_of_find_panel_or_bridge
which can return error codes different from EPROBE_DEFER. Therefore,
let's just return that error code instead of forcing it to EPROBE_DEFER.
Fixes: 34cc0aa25456 ("drm/rockchip: Add support for Rockchip Soc LVDS")
Cc: Quentin Schulz
Signed-off-by: Quentin Schulz
---
drivers/gpu/drm/rockchip/rockchip_lvds.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c
b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index f0f47e9abf5a..17d8fc797151 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -578,7 +578,6 @@ static int rockchip_lvds_bind(struct device *dev, struct
device *master,
goto err_put_port;
} else if (ret) {
DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n");
- ret = -EPROBE_DEFER;
goto err_put_port;
}
if (lvds->panel)
--
2.42.0
[PATCH v2 0/2] drm/rockchip: lvds: improve erroring out when drm_of_find_panel_or_bridge fails
drm_of_find_panel_or_bridge may return a different error code than EPROBE_DEFER so let's not overwrite it. At the same time, let's demote the DRM_DEV_ERROR message to dev_err_probe so that the scary message isn't shown (by default) whenever EPROBE_DEFER is returned to not mislead users. Signed-off-by: Quentin Schulz --- Changes in v2: - add a patch for not overwriting return code with EPROBE_DEFER - use dev_err_probe instead of DRM_DEV_DEBUG - Link to v1: https://lore.kernel.org/r/20231117-rk-lvds-defer-msg-v1-1-1e6894cf9...@theobroma-systems.com --- Quentin Schulz (2): drm/rockchip: lvds: do not overwrite error code drm/rockchip: lvds: do not print scary message when probing defer drivers/gpu/drm/rockchip/rockchip_lvds.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- base-commit: 98b1cc82c4affc16f5598d4fa14b1858671b2263 change-id: 20231117-rk-lvds-defer-msg-b2944b73d791 Best regards, -- Quentin Schulz
[PATCH v2 4/4] arm64: dts: qcom: sc7280: Add 0xac Adreno speed bin
A643 (A635 speedbin 0xac) tops out at 812 MHz. Fill in the
opp-supported-hw appropriately.
Note that fuseval 0xac is referred to as speedbin 1 downstream, but
that was already in use upstream, so 2 was chosen instead.
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 6964c14ffce5..b4e6951d9359 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2630,14 +2630,14 @@ opp-31500 {
opp-hz = /bits/ 64 <31500>;
opp-level =
;
opp-peak-kBps = <1804000>;
- opp-supported-hw = <0x03>;
+ opp-supported-hw = <0x07>;
};
opp-45000 {
opp-hz = /bits/ 64 <45000>;
opp-level = ;
opp-peak-kBps = <4068000>;
- opp-supported-hw = <0x03>;
+ opp-supported-hw = <0x07>;
};
/* Only applicable for SKUs which has 550Mhz as
Fmax */
@@ -2652,28 +2652,28 @@ opp-55000-1 {
opp-hz = /bits/ 64 <55000>;
opp-level =
;
opp-peak-kBps = <6832000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-60800 {
opp-hz = /bits/ 64 <60800>;
opp-level =
;
opp-peak-kBps = <8368000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-7 {
opp-hz = /bits/ 64 <7>;
opp-level = ;
opp-peak-kBps = <8532000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-81200 {
opp-hz = /bits/ 64 <81200>;
opp-level =
;
opp-peak-kBps = <8532000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-84000 {
--
2.42.1
[PATCH v2 3/4] arm64: dts: qcom: sc7280: Mark Adreno SMMU as DMA coherent
The SMMUs on sc7280 are cache-coherent. APPS_SMMU is marked as such,
mark the GPU one as well.
Fixes: 96c471970b7b ("arm64: dts: qcom: sc7280: Add gpu support")
Reviewed-by: Akhil P Oommen
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index db47af668232..6964c14ffce5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2787,6 +2787,7 @@ adreno_smmu: iommu@3da {
"gpu_cc_hub_aon_clk";
power-domains = <&gpucc GPU_CC_CX_GDSC>;
+ dma-coherent;
};
remoteproc_mpss: remoteproc@408 {
--
2.42.1
[PATCH v2 2/4] arm64: dts: qcom: sc7280: Fix up GPU SIDs
GPU_SMMU SID 1 is meant for Adreno LPAC (Low Priority Async Compute).
On platforms that support it (in firmware), it is necessary to
describe that link, or Adreno register access will hang the board.
The current settings are functionally identical, *but* due to what is
likely hardcoded security policies, the secure firmware rejects them,
resulting in the board hanging. To avoid that, alter the settings such
that SID 0 and 1 are described separately.
Fixes: 96c471970b7b ("arm64: dts: qcom: sc7280: Add gpu support")
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 58563f8fdc16..db47af668232 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2608,7 +2608,8 @@ gpu: gpu@3d0 {
"cx_mem",
"cx_dbgc";
interrupts = ;
- iommus = <&adreno_smmu 0 0x401>;
+ iommus = <&adreno_smmu 0 0x400>,
+<&adreno_smmu 1 0x400>;
operating-points-v2 = <&gpu_opp_table>;
qcom,gmu = <&gmu>;
interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt
SLAVE_EBI1 0>;
--
2.42.1
[PATCH v2 1/4] arm64: dts: qcom: sc7280: Add ZAP shader support
Non-Chrome SC7280-family platforms ship a ZAP shader with the Adreno GPU.
Describe that and make sure it doesn't interfere with Chrome devices.
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 2 ++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 9 +
2 files changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index 5d462ae14ba1..88fc67c3646e 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -17,6 +17,8 @@
* required by the setup for Chrome boards.
*/
+/delete-node/ &gpu_zap_mem;
+/delete-node/ &gpu_zap_shader;
/delete-node/ &hyp_mem;
/delete-node/ &xbl_mem;
/delete-node/ &reserved_xbl_uefi_log;
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 04bf85b0399a..58563f8fdc16 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -152,6 +152,11 @@ ipa_fw_mem: memory@8b70 {
no-map;
};
+ gpu_zap_mem: zap@8b71a000 {
+ reg = <0 0x8b71a000 0 0x2000>;
+ no-map;
+ };
+
rmtfs_mem: memory@9c90 {
compatible = "qcom,rmtfs-mem";
reg = <0x0 0x9c90 0x0 0x28>;
@@ -2613,6 +2618,10 @@ gpu: gpu@3d0 {
nvmem-cells = <&gpu_speed_bin>;
nvmem-cell-names = "speed_bin";
+ gpu_zap_shader: zap-shader {
+ memory-region = <&gpu_zap_mem>;
+ };
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.42.1
[PATCH v2 0/4] Adreno 643 + fixes
as it says on the can drm/msm patches for Rob arm64 patches for linux-arm-msm for use with https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25408 tested on QCM6490 (SC7280-IOT) Fairphone FP5 Signed-off-by: Konrad Dybcio --- Changes in v2: - Drop drm/msm patches (all applied) - Make the commit message of "Fix up GPU SIDs" clearer (Abhinav) - Drop unwanted firmware-name in "Add ZAP shader support" (self) - Pick up tags - Link to v1: https://lore.kernel.org/r/20230926-topic-a643-v1-0-7af6937ac...@linaro.org --- Konrad Dybcio (4): arm64: dts: qcom: sc7280: Add ZAP shader support arm64: dts: qcom: sc7280: Fix up GPU SIDs arm64: dts: qcom: sc7280: Mark Adreno SMMU as DMA coherent arm64: dts: qcom: sc7280: Add 0xac Adreno speed bin arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 2 ++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 -- 2 files changed, 20 insertions(+), 7 deletions(-) --- base-commit: 5a82d69d48c82e89aef44483d2a129f869f3506a change-id: 20230926-topic-a643-a7ec9a08a3a1 Best regards, -- Konrad Dybcio
Re: [PATCH v3 00/16] drm: Convert to platform remove callback returning void
[Dropped a few people from To that resulted in bounces before.] On Thu, Nov 02, 2023 at 05:56:41PM +0100, Uwe Kleine-König wrote: > Hello, > > this series converts all platform drivers below drivers/gpu/drm to use > .remove_new(). It starts with a fix for a problem that potentially might > crash the kernel that I stumbled over while implementing the conversion. > > Some of the conversion patches following this fix were already send in > earlier series: > > > https://lore.kernel.org/dri-devel/20230801110239.831099-1-u.kleine-koe...@pengutronix.de > > https://lore.kernel.org/dri-devel/20230318190804.234610-1-u.kleine-koe...@pengutronix.de > > and three patches (bridge/tpd12s015, exynos + tilcdc) are new. Parts of > the above series were picked up, the patches resend here are not. Apart from a Reviewed-by: by Toni Valkeinen for patch #16 and Inki Dae who wrote to have taken patch #8 (but that didn't appear in neither next nor drm-misc-next yet). Also in v2 they didn't result in euphoric replies. Can someone who cares about drm as a whole please care for this series apply it? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH 1/3] Revert "drm/prime: Unexport helpers for fd/handle conversion"
Hi
Am 17.11.23 um 22:44 schrieb Felix Kuehling:
This reverts commit 71a7974ac7019afeec105a54447ae1dc7216cbb3.
These helper functions are needed for KFD to export and import DMABufs
the right way without duplicating the tracking of DMABufs associated with
GEM objects while ensuring that move notifier callbacks are working as
intended.
I'm unhappy to see these functions making a comeback. They are the
boiler-plate logic that all drivers should use. Historically, drivers
did a lot one things in their GEM code that was only semi-correct.
Unifying most of that made the memory management more readable. Not
giving back drivers to option of tinkering with this might be
preferable. The rsp hooks in struct drm_driver, prime_fd_to_handle and
prime_handle_to_fd, are only there for vmwgfx.
If you want to hook into prime import and export, there are
drm_driver.gem_prime_import and drm_gem_object_funcs.export. Isn't it
possible to move the additional code behind these pointers?
Best regards
Thomas
CC: Christian König
CC: Thomas Zimmermann
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/drm_prime.c | 33 ++---
include/drm/drm_prime.h | 7 +++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 63b709a67471..834a5e28abbe 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
}
EXPORT_SYMBOL(drm_gem_dmabuf_release);
-/*
+/**
* drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
* @dev: drm_device to import into
* @file_priv: drm file-private structure
@@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
*
* Returns 0 on success or a negative error code on failure.
*/
-static int drm_gem_prime_fd_to_handle(struct drm_device *dev,
- struct drm_file *file_priv, int prime_fd,
- uint32_t *handle)
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+ struct drm_file *file_priv, int prime_fd,
+ uint32_t *handle)
{
struct dma_buf *dma_buf;
struct drm_gem_object *obj;
@@ -360,6 +360,7 @@ static int drm_gem_prime_fd_to_handle(struct drm_device
*dev,
dma_buf_put(dma_buf);
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -408,7 +409,7 @@ static struct dma_buf *export_and_register_object(struct
drm_device *dev,
return dmabuf;
}
-/*
+/**
* drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
* @dev: dev to export the buffer from
* @file_priv: drm file-private structure
@@ -421,10 +422,10 @@ static struct dma_buf *export_and_register_object(struct
drm_device *dev,
* The actual exporting from GEM object to a dma-buf is done through the
* &drm_gem_object_funcs.export callback.
*/
-static int drm_gem_prime_handle_to_fd(struct drm_device *dev,
- struct drm_file *file_priv, uint32_t
handle,
- uint32_t flags,
- int *prime_fd)
+int drm_gem_prime_handle_to_fd(struct drm_device *dev,
+ struct drm_file *file_priv, uint32_t handle,
+ uint32_t flags,
+ int *prime_fd)
{
struct drm_gem_object *obj;
int ret = 0;
@@ -506,6 +507,7 @@ static int drm_gem_prime_handle_to_fd(struct drm_device
*dev,
return ret;
}
+EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -864,9 +866,9 @@ EXPORT_SYMBOL(drm_prime_get_contiguous_size);
* @obj: GEM object to export
* @flags: flags like DRM_CLOEXEC and DRM_RDWR
*
- * This is the implementation of the &drm_gem_object_funcs.export functions
- * for GEM drivers using the PRIME helpers. It is used as the default for
- * drivers that do not set their own.
+ * This is the implementation of the &drm_gem_object_funcs.export functions
for GEM drivers
+ * using the PRIME helpers. It is used as the default in
+ * drm_gem_prime_handle_to_fd().
*/
struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
int flags)
@@ -962,9 +964,10 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev);
* @dev: drm_device to import into
* @dma_buf: dma-buf object to import
*
- * This is the implementation of the gem_prime_import functions for GEM
- * drivers using the PRIME helpers. It is the default for drivers that do
- * not set their own &drm_driver.gem_prime_import.
+ * This is the implementation of the
Re: [PATCH v18 11/26] drm/shmem-helper: Prepare drm_gem_shmem_free() to shrinker addition
On 11/20/23 14:19, Boris Brezillon wrote:
...
- dma_resv_lock(shmem->base.resv, NULL);
-
drm_WARN_ON(obj->dev, refcount_read(&shmem->vmap_use_count));
if (shmem->sgt) {
@@ -157,8 +171,6 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
>>> If you drop the dma_resv_lock/unlock(), you should also replace the
>>> drm_gem_shmem_put_pages_locked() by a drm_gem_shmem_free_pages() in this
>>> commit.
>>
>> drm_gem_shmem_put_pages_locked() is exported by a later patch of this
>> series, it's not worthwhile to remove this function
>
> I'm not talking about removing drm_gem_shmem_put_pages_locked(), but
> replacing the drm_gem_shmem_put_pages_locked() call you have in
> drm_gem_shmem_free() by a drm_gem_shmem_free_pages(), so you don't end
> up with a lockdep warning when you stop exactly here in the patch
> series, which is important if we want to keep things bisectable.
Indeed, there is assert_locked() there. Thanks for the clarification :)
--
Best regards,
Dmitry
Re: [PATCH v3 17/20] drivers/gpu/drm/ast/ast_i2c.c: remove I2C_CLASS_DDC support
Am 19.11.23 um 11:14 schrieb Heiner Kallweit: After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Signed-off-by: Heiner Kallweit Reviewed-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_i2c.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/ast/ast_i2c.c b/drivers/gpu/drm/ast/ast_i2c.c index 0e845e7ac..e5d3f7121 100644 --- a/drivers/gpu/drm/ast/ast_i2c.c +++ b/drivers/gpu/drm/ast/ast_i2c.c @@ -120,7 +120,6 @@ struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev) return NULL; i2c->adapter.owner = THIS_MODULE; - i2c->adapter.class = I2C_CLASS_DDC; i2c->adapter.dev.parent = dev->dev; i2c->dev = dev; i2c_set_adapdata(&i2c->adapter, i2c); -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 02/20] drivers/gpu/drm/mgag200/mgag200_i2c.c: remove I2C_CLASS_DDC support
Am 19.11.23 um 11:14 schrieb Heiner Kallweit: After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Signed-off-by: Heiner Kallweit Reviewed-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_i2c.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_i2c.c b/drivers/gpu/drm/mgag200/mgag200_i2c.c index 0c48bdf3e..423eb302b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_i2c.c +++ b/drivers/gpu/drm/mgag200/mgag200_i2c.c @@ -106,7 +106,6 @@ int mgag200_i2c_init(struct mga_device *mdev, struct mga_i2c_chan *i2c) i2c->data = BIT(info->i2c.data_bit); i2c->clock = BIT(info->i2c.clock_bit); i2c->adapter.owner = THIS_MODULE; - i2c->adapter.class = I2C_CLASS_DDC; i2c->adapter.dev.parent = dev->dev; i2c->dev = dev; i2c_set_adapdata(&i2c->adapter, i2c); -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH] drm/modes: Fix divide error in drm_mode_debug_printmodeline
On Sun, 19 Nov 2023, Edward Adam Davis wrote:
> [Syz Log]
> divide error: [#1] PREEMPT SMP KASAN
> CPU: 0 PID: 5068 Comm: syz-executor357 Not tainted
> 6.6.0-syzkaller-16039-gac347a0655db #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 10/09/2023
> RIP: 0010:drm_mode_vrefresh drivers/gpu/drm/drm_modes.c:1303 [inline]
> RIP: 0010:drm_mode_debug_printmodeline+0x118/0x4e0
> drivers/gpu/drm/drm_modes.c:60
> Code: 00 41 0f b7 07 66 83 f8 02 b9 01 00 00 00 0f 43 c8 0f b7 c1 0f af e8 44
> 89 f0 48 69 c8 e8 03 00 00 89 e8 d1 e8 48 01 c8 31 d2 <48> f7 f5 49 89 c6 eb
> 0c e8 fb 07 66 fc eb 05 e8 f4 07 66 fc 48 89
> RSP: 0018:c9000391f8d0 EFLAGS: 00010246
> RAX: 0001f400 RBX: 888025045000 RCX: 0001f400
> RDX: RSI: 8000 RDI: 888025045018
> RBP: R08: 8528b9af R09:
> R10: c9000391f8a0 R11: f52000723f17 R12: 0080
> R13: dc00 R14: 0080 R15: 888025045016
> FS: 56932380() GS:8880b980() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: 005fdeb8 CR3: 7fcff000 CR4: 003506f0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> Call Trace:
>
> drm_mode_setcrtc+0x83b/0x1880 drivers/gpu/drm/drm_crtc.c:794
> drm_ioctl_kernel+0x362/0x500 drivers/gpu/drm/drm_ioctl.c:792
> drm_ioctl+0x636/0xb00 drivers/gpu/drm/drm_ioctl.c:895
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:871 [inline]
> __se_sys_ioctl+0xf8/0x170 fs/ioctl.c:857
> do_syscall_x64 arch/x86/entry/common.c:51 [inline]
> do_syscall_64+0x44/0x110 arch/x86/entry/common.c:82
> entry_SYSCALL_64_after_hwframe+0x63/0x6b
>
> [Analysis]
> When calculating den in drm_mode_vrefresh(), if the vscan value is too large,
> there is a probability of unsigned integer overflow.
>
> [Fix]
> Before multiplying by vscan, first determine their ilog2. When their total
> exceeds 32, return -EINVAL and exit the subsequent calculation.
>
> Reported-and-tested-by: syzbot+2e93e6fb36e6fdc56...@syzkaller.appspotmail.com
> Fixes: ea40d7857d52 ("drm/vkms: fbdev emulation support")
> Signed-off-by: Edward Adam Davis
> ---
> drivers/gpu/drm/drm_modes.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index ac9a406250c5..c7ec1ab041f8 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -36,6 +36,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -1297,8 +1298,11 @@ int drm_mode_vrefresh(const struct drm_display_mode
> *mode)
> num *= 2;
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> den *= 2;
> - if (mode->vscan > 1)
> + if (mode->vscan > 1) {
> + if (ilog2(den) + ilog2(mode->vscan) >= 32)
For future reference, check_mul_overflow() is the way to handle this.
> + return -EINVAL;
Just so there's no confusion: NAK.
I'd be surprised if there were even a single place in the kernel where
someone checks drm_mode_vrefresh() for a negative error return.
This function must succeed.
Please change the types as needed instead.
BR,
Jani.
> den *= mode->vscan;
> + }
>
> return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(num, 1000), den);
> }
--
Jani Nikula, Intel
