kdg2kfd_probe failed
For some unknown reason, I started to get the "kgd2kfd_probe failed" message after the update of Fedora-27 (limited to security advisories). Here is dmesg output: Console: switching to colour frame buffer device 240x67 radeon :01:05.0: fb0: radeondrmfb frame buffer device kfd kfd: DID 9710 is missing in supported_devices kfd kfd: kgd2kfd_probe failed [drm] Initialized radeon 2.50.0 20080528 for :01:05.0 on minor 0 kernel version: 4.14.16-300.fc27.x86_64 Compare with the dmesg output: Console: switching to colour frame buffer device 240x67 radeon :01:05.0: fb0: radeondrmfb frame buffer device [drm] Initialized radeon 2.50.0 20080528 for :01:05.0 on minor 0 from the kernel version 4.13.9-300.fc27.x86_64 Q1. Is this failure expected? Note that this system doesn't support IOMMU. Q2. What to do if such failure is not expected? Thank you, - Sen Dion P.S. This is my first submission to the list. Kindly forgive me if I missed to adhere to the list posting guildelines. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 198123] Console is the wrong color at boot with radeon 6670
https://bugzilla.kernel.org/show_bug.cgi?id=198123 --- Comment #30 from Daniel Vetter (dan...@ffwll.ch) --- There was not a single backtrace in that attachement. Was this only up to fbcon, or did you start X too? Where the colors correct afterwards? It's also strange that it fails to compile on HEAD, that shouldn't happen (and would indicate some other leftovers or something). I'll attach another test patch, that time around please make sure you boot until X (or whatever ends up fixing your colors). -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 198123] Console is the wrong color at boot with radeon 6670
https://bugzilla.kernel.org/show_bug.cgi?id=198123 --- Comment #31 from Daniel Vetter (dan...@ffwll.ch) --- Created attachment 274229 --> https://bugzilla.kernel.org/attachment.cgi?id=274229&action=edit another test patch for Deposite Pirate -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/mm: Fix caching of leftmost node in the interval tree
When we descend the tree to find our slot, if we step to the right, we
are no longer the leftmost node.
Fixes: f808c13fd373 ("lib/interval_tree: fast overlap detection")
Signed-off-by: Chris Wilson
Cc: Davidlohr Bueso
Cc: Jérôme Glisse
Cc: Christian König
---
drivers/gpu/drm/drm_mm.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 186c4e90cc1c..a351bd888a61 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -180,7 +180,7 @@ static void drm_mm_interval_tree_add_node(struct
drm_mm_node *hole_node,
struct drm_mm *mm = hole_node->mm;
struct rb_node **link, *rb;
struct drm_mm_node *parent;
- bool leftmost = true;
+ bool leftmost;
node->__subtree_last = LAST(node);
@@ -201,6 +201,7 @@ static void drm_mm_interval_tree_add_node(struct
drm_mm_node *hole_node,
} else {
rb = NULL;
link = &mm->interval_tree.rb_root.rb_node;
+ leftmost = true;
}
while (*link) {
@@ -208,11 +209,11 @@ static void drm_mm_interval_tree_add_node(struct
drm_mm_node *hole_node,
parent = rb_entry(rb, struct drm_mm_node, rb);
if (parent->__subtree_last < node->__subtree_last)
parent->__subtree_last = node->__subtree_last;
- if (node->start < parent->start)
+ if (node->start < parent->start) {
link = &parent->rb.rb_left;
- else {
+ } else {
link = &parent->rb.rb_right;
- leftmost = true;
+ leftmost = false;
}
}
--
2.16.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/mm: Micro-optimise updating the upper layers of the interval tree
When we insert a node into a hole inside the interval tree, we need to
climb the layers above us to update the cached interval. When doing so,
we know that the initial node exists as it is our starting hole.
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-10 (-10)
Function old new delta
drm_mm_interval_tree_add_node221 211 -10
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/drm_mm.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index a351bd888a61..2d844c9a288f 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -179,21 +179,23 @@ static void drm_mm_interval_tree_add_node(struct
drm_mm_node *hole_node,
{
struct drm_mm *mm = hole_node->mm;
struct rb_node **link, *rb;
- struct drm_mm_node *parent;
bool leftmost;
node->__subtree_last = LAST(node);
- if (hole_node->allocated) {
- rb = &hole_node->rb;
- while (rb) {
- parent = rb_entry(rb, struct drm_mm_node, rb);
+ if (likely(hole_node->allocated)) {
+ struct drm_mm_node *parent;
+
+ /* Update the interval range above us */
+ parent = hole_node;
+ do {
if (parent->__subtree_last >= node->__subtree_last)
break;
parent->__subtree_last = node->__subtree_last;
rb = rb_parent(rb);
- }
+ } while ((parent = rb_entry_safe(&parent->rb,
+struct drm_mm_node, rb)));
rb = &hole_node->rb;
link = &hole_node->rb.rb_right;
@@ -205,6 +207,8 @@ static void drm_mm_interval_tree_add_node(struct
drm_mm_node *hole_node,
}
while (*link) {
+ struct drm_mm_node *parent;
+
rb = *link;
parent = rb_entry(rb, struct drm_mm_node, rb);
if (parent->__subtree_last < node->__subtree_last)
--
2.16.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 02/10] drm: drm_fourcc: Introduce macro-pixel info to drm_format_info
On Thu, Feb 08, 2018 at 06:19:32PM -0800, Hyun Kwon wrote:
> Hi Daniel,
>
> On Tue, 2018-01-30 at 02:22:40 -0800, Daniel Vetter wrote:
> > On Thu, Jan 25, 2018 at 06:03:59PM -0800, Hyun Kwon wrote:
> > > Multiple pixels can be grouped as a single unit and form a 'macro-pixel'.
> > > This is to model formats where multiple pixels are stored together
> > > in a specific way, likely byte-algined. For example, if 3 - 10 bit
> > > pixels are stored in 32 bit, the 32 bit stroage can be treated as
> > > a single macro-pixel with 3 pixels. This aligns non-byte addressable
> > > formats with drm core where bpp is expected to be multiple of 8 bit.
> > >
> > > Add 'ppm', pixels per macro-pixel, to note how many pixels are grouped
> > > in a macro-pixel. 'bpm', bits per macro-pixel, specifies how many bits
> > > are in a macro-pixel as there can be some extra padding bits.
> >
> > Should we mandate that macro-pixels are always byte-aligned? This would
> > mean cpm for characters per macro-pixel would be more meaningful.
> >
>
> Agreed. That would simplify stuff and be more clean.
>
> > >
> > > Signed-off-by: Hyun Kwon
> > > ---
> > > v2
> > > - Introduce macro-pixel over scaling factors
> > > ---
> > > ---
> > > drivers/gpu/drm/drm_fourcc.c | 136
> > > +--
> > > include/drm/drm_fourcc.h | 9 +++
> > > 2 files changed, 77 insertions(+), 68 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > > index b891fe0..8fc1e35 100644
> > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > @@ -105,74 +105,74 @@ EXPORT_SYMBOL(drm_get_format_name);
> > > const struct drm_format_info *__drm_format_info(u32 format)
> > > {
> > > static const struct drm_format_info formats[] = {
> > > - { .format = DRM_FORMAT_C8, .depth = 8,
> > > .num_planes = 1, .cpp = { 1, 0, 0 }, .bpp = { 8, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_RGB332, .depth = 8,
> > > .num_planes = 1, .cpp = { 1, 0, 0 }, .bpp = { 8, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_BGR233, .depth = 8,
> > > .num_planes = 1, .cpp = { 1, 0, 0 }, .bpp = { 8, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_XRGB,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_XBGR,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_RGBX,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_BGRX,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_ARGB,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_ABGR,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_RGBA,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_BGRA,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
>
> [snip]
>
> > + { .format = DRM_FORMAT_NV16,.depth = 0,
> > .num_planes = 2, .cpp = { 1, 2, 0 }, .bpp = { 8, 16, 0 }, .ppm = { 1, 1, 0
> > }, .bpm = { 8, 16, 0 }, .hsub = 2, .vsub = 1 },
> > > + { .format = DRM_FORMAT_NV61,.depth = 0,
> > > .num_planes = 2, .cpp = { 1, 2, 0 }, .bpp = { 8, 16, 0 }, .ppm = { 1, 1,
> > > 0 }, .bpm = { 8, 16, 0 }, .hsub = 2, .vsub = 1 },
> > > + { .format = DRM_FORMAT_NV24,.depth = 0,
> > > .num_planes = 2, .cpp = { 1, 2, 0 }, .bpp = { 8, 16, 0 }, .ppm = { 1, 1,
> > > 0 }, .bpm = { 8, 16, 0 }, .hsub = 1, .vsub = 1 },
> > > + { .format = DRM_FORMAT_NV42,.depth = 0,
> > > .num_planes = 2, .cpp = { 1, 2, 0 }, .bpp = { 8, 16, 0 }, .ppm = { 1, 1,
> > > 0 }, .bpm = { 8, 16, 0 }, .hsub = 1, .vsub = 1 },
> > > + { .format = DRM_FORMAT_YUYV,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .ppm = { 1, 0,
> > > 0 }, .bpm = { 16, 0, 0 }, .hsub = 2, .vsub = 1 },
> > > + { .format = DRM_FORMAT_YVYU,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .ppm = { 1, 0,
> > > 0 }, .bpm = { 16, 0, 0 }, .hsub = 2, .vsub = 1 },
> > > + { .format = DRM_FORMAT_UYVY,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0
Re: [RFC v2 02/10] drm: drm_fourcc: Introduce macro-pixel info to drm_format_info
On Thu, Feb 08, 2018 at 06:19:42PM -0800, Hyun Kwon wrote:
> Hi Daniel,
>
> On Tue, 2018-01-30 at 02:27:07 -0800, Daniel Vetter wrote:
> > On Thu, Jan 25, 2018 at 06:03:59PM -0800, Hyun Kwon wrote:
> > > Multiple pixels can be grouped as a single unit and form a 'macro-pixel'.
> > > This is to model formats where multiple pixels are stored together
> > > in a specific way, likely byte-algined. For example, if 3 - 10 bit
> > > pixels are stored in 32 bit, the 32 bit stroage can be treated as
> > > a single macro-pixel with 3 pixels. This aligns non-byte addressable
> > > formats with drm core where bpp is expected to be multiple of 8 bit.
> > >
> > > Add 'ppm', pixels per macro-pixel, to note how many pixels are grouped
> > > in a macro-pixel. 'bpm', bits per macro-pixel, specifies how many bits
> > > are in a macro-pixel as there can be some extra padding bits.
> > >
> > > Signed-off-by: Hyun Kwon
> >
> > Another thought: If we require that a format description has either cpp or
> > the macro-pixel stuff set (but not both), then we could avoid changing the
> > entire table. Calculating the width in bytes would first use cpp, and if
> > that's 0, try to compute the width using the macro-pixel stuff. And if
> > that's also 0, then WARN_ON (since it's a kernel bug).
>
> That certainly will minimize the change. Thanks a lot for suggestion! I will
> address your comments in next version.
Forgot to add: Also WARN_ON if both are set (since that risks creating an
incosistency).
-Daniel
>
> Thanks,
> -hyun
>
> > -Daniel
> >
> > > ---
> > > v2
> > > - Introduce macro-pixel over scaling factors
> > > ---
> > > ---
> > > drivers/gpu/drm/drm_fourcc.c | 136
> > > +--
> > > include/drm/drm_fourcc.h | 9 +++
> > > 2 files changed, 77 insertions(+), 68 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > > index b891fe0..8fc1e35 100644
> > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > @@ -105,74 +105,74 @@ EXPORT_SYMBOL(drm_get_format_name);
> > > const struct drm_format_info *__drm_format_info(u32 format)
> > > {
> > > static const struct drm_format_info formats[] = {
> > > - { .format = DRM_FORMAT_C8, .depth = 8,
> > > .num_planes = 1, .cpp = { 1, 0, 0 }, .bpp = { 8, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_RGB332, .depth = 8,
> > > .num_planes = 1, .cpp = { 1, 0, 0 }, .bpp = { 8, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_BGR233, .depth = 8,
> > > .num_planes = 1, .cpp = { 1, 0, 0 }, .bpp = { 8, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_XRGB,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_XBGR,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_RGBX,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_BGRX,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_ARGB,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_ABGR,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_RGBA,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_BGRA,.depth = 0,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_XRGB1555,.depth = 15,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_XBGR1555,.depth = 15,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_RGBX5551,.depth = 15,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_BGRX5551,.depth = 15,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_ARGB1555,.depth = 15,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp = { 16, 0, 0 }, .hsub = 1,
> > > .vsub = 1 },
> > > - { .format = DRM_FORMAT_ABGR1555,.depth = 15,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .bpp
Re: [PATCH 1/4] drm/simple_kms_helper: Add {enable|disable}_vblank callback support
ping
On 02/12/2018 10:52 AM, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko
If simple_kms_helper based driver needs to work with vblanks,
then it has to provide drm_driver.{enable|disable}_vblank callbacks,
because drm_simple_kms_helper.drm_crtc_funcs does not provide any.
At the same time drm_driver.{enable|disable}_vblank callbacks
are marked as deprecated and shouldn't be used by new drivers.
Fix this by extending drm_simple_kms_helper.drm_crtc_funcs
to provide the missing callbacks.
Signed-off-by: Oleksandr Andrushchenko
---
drivers/gpu/drm/drm_simple_kms_helper.c | 24
include/drm/drm_simple_kms_helper.h | 18 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c
b/drivers/gpu/drm/drm_simple_kms_helper.c
index 9d3f6b70812c..9ca8a4a59b74 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -77,6 +77,28 @@ static const struct drm_crtc_helper_funcs
drm_simple_kms_crtc_helper_funcs = {
.atomic_disable = drm_simple_kms_crtc_disable,
};
+static int drm_simple_kms_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+ struct drm_simple_display_pipe *pipe;
+
+ pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
+ if (!pipe->funcs || !pipe->funcs->enable_vblank)
+ return 0;
+
+ return pipe->funcs->enable_vblank(pipe);
+}
+
+static void drm_simple_kms_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+ struct drm_simple_display_pipe *pipe;
+
+ pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
+ if (!pipe->funcs || !pipe->funcs->disable_vblank)
+ return;
+
+ pipe->funcs->disable_vblank(pipe);
+}
+
static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs = {
.reset = drm_atomic_helper_crtc_reset,
.destroy = drm_crtc_cleanup,
@@ -84,6 +106,8 @@ static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs
= {
.page_flip = drm_atomic_helper_page_flip,
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+ .enable_vblank = drm_simple_kms_crtc_enable_vblank,
+ .disable_vblank = drm_simple_kms_crtc_disable_vblank,
};
static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
diff --git a/include/drm/drm_simple_kms_helper.h
b/include/drm/drm_simple_kms_helper.h
index 6d9adbb46293..79567826b099 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -93,6 +93,24 @@ struct drm_simple_display_pipe_funcs {
*/
void (*cleanup_fb)(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state);
+
+ /**
+* @enable_vblank:
+*
+* Optional, called by &drm_crtc_funcs.enable_vblank. Please read
+* the documentation for the &drm_crtc_funcs.enable_vblank hook for
+* more details.
+*/
+ int (*enable_vblank)(struct drm_simple_display_pipe *pipe);
+
+ /**
+* @disable_vblank:
+*
+* Optional, called by &drm_crtc_funcs.disable_vblank. Please read
+* the documentation for the &drm_crtc_funcs.disable_vblank hook for
+* more details.
+*/
+ void (*disable_vblank)(struct drm_simple_display_pipe *pipe);
};
/**
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fb-helper: Scale back depth to supported maximum
On Fri, Feb 02, 2018 at 04:03:43PM +0200, Ville Syrjälä wrote:
> On Fri, Feb 02, 2018 at 02:56:30PM +0100, Linus Walleij wrote:
> > On Thu, Feb 1, 2018 at 2:19 PM, Ville Syrjälä
> > wrote:
> > > [Me]
> > >> + /*
> > >> + * If we run into a situation where, for example, the primary plane
> > >> + * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp,
> > >> depth
> > >> + * 16) we need to scale down the depth of the sizes we request.
> > >> + */
> > >> + drm_for_each_plane(plane, fb_helper->dev) {
> > >> + /* Only check the primary plane */
> > >> + if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> > >> + continue;
> > >
> > > I think this should look at crtc->primary for each of the crtcs managed
> > > by the fb_helper.
> > >
> > > Also this probably shouldn't look at YUV formats at all?
> >
> > I guess I can look into doing it this way, sorry for not knowing how to
> > properly inspect DRM objects, I'm lost sometimes...
> >
> > > I do wonder if instead we should just have the driver specify the
> > > pixel format explicitly instead of trying to guess based on bpp?
> >
> > That makes a lot more sense to me actually. It would
> > give a better sense of control so the driver feel it knows
> > what is actually going on.
> >
> > So I would just update
> > drm_fb_cma_fbdev_init() and drm_fb_helper_initial_config()
> > to pass a reasonable pixel format instead and refactor all the
> > way down?
>
> Yeah, something along those lines would seem like the better approach
> to me. But it's been a while since I've looked at this code so I might
> be totally wrong :)
Yeah, that would definitely fit better with the new world - fbdev
emulation hasnt ever been updated to the pixel format approach.
But fbdev emulation also predates the list of acceptable formats for each
plane, so I'd go one step further and try to out-guess something if the
driver passes a pixel format of 0. Your above loop almost does that
already. That way only drivers which want something non-standard would
need to set that parameter. For everyone else we could go through all
rgb/bgr formats, from most pixels to least (well, all those that fbdev
supports), with C8 as the ultimate fallback.
But just going to pixel formats would be a good step already for sure.
-Daniel
>
> >
> > It does hit a lot of code on the way, but if everyone thinks this
> > is a good idea I can very well take a stab at it.
> >
> > Yours,
> > Linus Walleij
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ast: Add option to initialize palette on driver load
On Fri, Feb 02, 2018 at 01:59:51AM -0600, Timothy Pearson wrote:
>
> Non-x86, such as OpenPOWER and ARM machines, do not execute the
> ASPEED-provided
> option ROM on system start. As a result, the VGA palette registers remain
> uninitialized, leading to odd colors and generally hard to read output on the
> VGA port.
>
> Add a new module option, ast_resetpalette, to enable loading a linear
> greyscale
> palette into the VGA RAMDAC. This option is intended for use by the first
> Linux
> kernel to load after initial power on, such as the skiroot kernel on OpenPOWER
> systems.
Latest kernels already do that with a combination of
commit 24b8ef699e8221d2b7f813adaab13eec053e1507 (tag: drm-for-v4.16)
Author: Daniel Vetter
Date: Thu Jan 18 16:40:16 2018 +0100
drm/ast: Load lut in crtc_commit
and
commit cf48e2921ee95011a164dc31e8725022bd008666
Author: Daniel Vetter
Date: Wed Mar 30 11:51:16 2016 +0200
drm: Initialize a linear gamma table by default
either way this definitely shouldn't be a driver option, it's just a
driver bug :-)
-Daniel
>
> Signed-off-by: Timothy Pearson
> ---
> drivers/gpu/drm/ast/ast_drv.c | 4
> drivers/gpu/drm/ast/ast_drv.h | 14 ++
> drivers/gpu/drm/ast/ast_main.c | 8
> drivers/gpu/drm/ast/ast_mode.c | 14 --
> 4 files changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 69dab82a3771..8124eaa92ed3 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -1,5 +1,6 @@
> /*
> * Copyright 2012 Red Hat Inc.
> + * Copyright 2018 Raptor Engineering, LLC.
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> * copy of this software and associated documentation files (the
> @@ -34,9 +35,12 @@
> #include "ast_drv.h"
>
> int ast_modeset = -1;
> +int ast_resetpalette = -1;
>
> MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
> +MODULE_PARM_DESC(resetpalette, "Disable/Enable palette reset on load");
> module_param_named(modeset, ast_modeset, int, 0400);
> +module_param_named(resetpalette, ast_resetpalette, int, 0400);
>
> #define PCI_VENDOR_ASPEED 0x1a03
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index e6c4cd3dc50e..5e834e466b65 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -383,6 +383,20 @@ static inline int ast_bo_reserve(struct ast_bo *bo, bool
> no_wait)
> return 0;
> }
>
> +static inline void ast_load_palette_index(struct ast_private *ast,
> + u8 index, u8 red, u8 green,
> + u8 blue)
> +{
> + ast_io_write8(ast, AST_IO_DAC_INDEX_WRITE, index);
> + ast_io_read8(ast, AST_IO_SEQ_PORT);
> + ast_io_write8(ast, AST_IO_DAC_DATA, red);
> + ast_io_read8(ast, AST_IO_SEQ_PORT);
> + ast_io_write8(ast, AST_IO_DAC_DATA, green);
> + ast_io_read8(ast, AST_IO_SEQ_PORT);
> + ast_io_write8(ast, AST_IO_DAC_DATA, blue);
> + ast_io_read8(ast, AST_IO_SEQ_PORT);
> +}
> +
> static inline void ast_bo_unreserve(struct ast_bo *bo)
> {
> ttm_bo_unreserve(&bo->bo);
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index dac355812adc..c15c55f69e38 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -1,5 +1,6 @@
> /*
> * Copyright 2012 Red Hat Inc.
> + * Copyright 2018 Raptor Engineering, LLC.
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> * copy of this software and associated documentation files (the
> @@ -32,6 +33,8 @@
> #include
> #include
>
> +extern int ast_resetpalette;
> +
> void ast_set_index_reg_mask(struct ast_private *ast,
> uint32_t base, uint8_t index,
> uint8_t mask, uint8_t val)
> @@ -483,6 +486,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long
> flags)
> struct ast_private *ast;
> bool need_post;
> int ret = 0;
> + int index = 0;
>
> ast = kzalloc(sizeof(struct ast_private), GFP_KERNEL);
> if (!ast)
> @@ -516,6 +520,10 @@ int ast_driver_load(struct drm_device *dev, unsigned
> long flags)
> }
> }
>
> + if (ast_resetpalette == 1)
> + for (index = 0x00; index < 0x100; index++)
> + ast_load_palette_index(ast, index, index, index, index);
> +
> ast_detect_chip(dev, &need_post);
>
> if (need_post)
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 9555a3542022..9afc4d53bd60 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -46,20 +46,6 @@ static int ast_cursor_set(struct drm_crtc *crtc,
> static int ast_cursor_move(struct drm_crtc *crtc,
> int x, int y);
>
> -static inline void ast_load_palette_index
Re: [PATCH] drm/nouveau: prefer XBGR2101010 for addfb ioctl
On Mon, Feb 05, 2018 at 09:10:12AM -0500, Ilia Mirkin wrote: > On Mon, Feb 5, 2018 at 8:58 AM, Ville Syrjälä > wrote: > > On Sat, Feb 03, 2018 at 02:11:23PM -0500, Ilia Mirkin wrote: > >> Nouveau only exposes support for XBGR2101010. Prior to the atomic > >> conversion, drm would pass in the wrong format in the framebuffer, but > >> it was always ignored -- both userspace (xf86-video-nouveau) and the > >> kernel driver agreed on the layout, so the fact that the format was > >> wrong didn't matter. > >> > >> With the atomic conversion, nouveau all of a sudden started caring about > >> the exact format, and so the previously-working code in > >> xf86-video-nouveau no longer functioned since the (internally-assigned) > >> format from the addfb ioctl was wrong. > >> > >> This change adds infrastructure to allow a drm driver to specify that it > >> prefers the XBGR format variant for the addfb ioctl, and makes nouveau's > >> nv50 display driver set it. (Prior gens had no support for 30bpp at all.) > >> > >> Signed-off-by: Ilia Mirkin > >> Cc: sta...@vger.kernel.org # v4.10+ > >> --- > >> > >> Wasn't sure if the nouveau line needs to be split out into a separate > >> change or not. > >> > >> drivers/gpu/drm/drm_framebuffer.c | 4 > >> drivers/gpu/drm/nouveau/nv50_display.c | 1 + > >> include/drm/drm_drv.h | 1 + > >> 3 files changed, 6 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_framebuffer.c > >> b/drivers/gpu/drm/drm_framebuffer.c > >> index 5a13ff29f4f0..c0530a1af5e3 100644 > >> --- a/drivers/gpu/drm/drm_framebuffer.c > >> +++ b/drivers/gpu/drm/drm_framebuffer.c > >> @@ -121,6 +121,10 @@ int drm_mode_addfb(struct drm_device *dev, > >> r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth); > >> r.handles[0] = or->handle; > >> > >> + if (r.pixel_format == DRM_FORMAT_XRGB2101010 && > >> + dev->driver->driver_features & DRIVER_PREFER_XBGR_30BPP) > >> + r.pixel_format = DRM_FORMAT_XBGR2101010; > > > > I think I'd much prefer if the driver just passed some kind of a > > bpp/depth->format mapping table to the core, or maybe an optional > > vfunc to allow the driver to override drm_mode_legacy_fb_format() > > behaviour. > > > > drm_mode_legacy_fb_format() is called from the fbdev setup as well > > so handling this only in addfb doesn't look sufficient. > > Well, in practice fbdev won't pick a 30bpp mode. But I get the point. > It would also enable a driver to have a funky RGB ordering for depth > 24/32 and others. Although I don't know if there are any customers for > that in practice. > > A vfunc works for me. > > Anyone else want to opine before I go for it? I'm happy to do the > work, but want to make sure I'm not just doing things that'll get > rejected, as I have a limited amount of time to do these things. Imo the very obvious hack is totally fine, makes it stick out more that we have a fairly nasty uapi inconsistency here. Also vfunc or explicit table open up the door for even more driver abuse in the future, which I don't like. vfunc for 1 case ever is also overkill. Wrt fbdev: Linus seems to have volunteered to switch fbdev from depth/bpp to explicit pixel formats, so no worries. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/nouveau: prefer XBGR2101010 for addfb ioctl
On Mon, Feb 19, 2018 at 10:21:54AM +0100, Daniel Vetter wrote: > On Mon, Feb 05, 2018 at 09:10:12AM -0500, Ilia Mirkin wrote: > > On Mon, Feb 5, 2018 at 8:58 AM, Ville Syrjälä > > wrote: > > > On Sat, Feb 03, 2018 at 02:11:23PM -0500, Ilia Mirkin wrote: > > >> Nouveau only exposes support for XBGR2101010. Prior to the atomic > > >> conversion, drm would pass in the wrong format in the framebuffer, but > > >> it was always ignored -- both userspace (xf86-video-nouveau) and the > > >> kernel driver agreed on the layout, so the fact that the format was > > >> wrong didn't matter. > > >> > > >> With the atomic conversion, nouveau all of a sudden started caring about > > >> the exact format, and so the previously-working code in > > >> xf86-video-nouveau no longer functioned since the (internally-assigned) > > >> format from the addfb ioctl was wrong. > > >> > > >> This change adds infrastructure to allow a drm driver to specify that it > > >> prefers the XBGR format variant for the addfb ioctl, and makes nouveau's > > >> nv50 display driver set it. (Prior gens had no support for 30bpp at all.) > > >> > > >> Signed-off-by: Ilia Mirkin > > >> Cc: sta...@vger.kernel.org # v4.10+ > > >> --- > > >> > > >> Wasn't sure if the nouveau line needs to be split out into a separate > > >> change or not. > > >> > > >> drivers/gpu/drm/drm_framebuffer.c | 4 > > >> drivers/gpu/drm/nouveau/nv50_display.c | 1 + > > >> include/drm/drm_drv.h | 1 + > > >> 3 files changed, 6 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/drm_framebuffer.c > > >> b/drivers/gpu/drm/drm_framebuffer.c > > >> index 5a13ff29f4f0..c0530a1af5e3 100644 > > >> --- a/drivers/gpu/drm/drm_framebuffer.c > > >> +++ b/drivers/gpu/drm/drm_framebuffer.c > > >> @@ -121,6 +121,10 @@ int drm_mode_addfb(struct drm_device *dev, > > >> r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth); > > >> r.handles[0] = or->handle; > > >> > > >> + if (r.pixel_format == DRM_FORMAT_XRGB2101010 && > > >> + dev->driver->driver_features & DRIVER_PREFER_XBGR_30BPP) > > >> + r.pixel_format = DRM_FORMAT_XBGR2101010; > > > > > > I think I'd much prefer if the driver just passed some kind of a > > > bpp/depth->format mapping table to the core, or maybe an optional > > > vfunc to allow the driver to override drm_mode_legacy_fb_format() > > > behaviour. > > > > > > drm_mode_legacy_fb_format() is called from the fbdev setup as well > > > so handling this only in addfb doesn't look sufficient. > > > > Well, in practice fbdev won't pick a 30bpp mode. But I get the point. > > It would also enable a driver to have a funky RGB ordering for depth > > 24/32 and others. Although I don't know if there are any customers for > > that in practice. > > > > A vfunc works for me. > > > > Anyone else want to opine before I go for it? I'm happy to do the > > work, but want to make sure I'm not just doing things that'll get > > rejected, as I have a limited amount of time to do these things. > > Imo the very obvious hack is totally fine, makes it stick out more that we > have a fairly nasty uapi inconsistency here. > > Also vfunc or explicit table open up the door for even more driver abuse > in the future, which I don't like. vfunc for 1 case ever is also overkill. > > Wrt fbdev: Linus seems to have volunteered to switch fbdev from depth/bpp > to explicit pixel formats, so no worries. Forgot to add the most important bit. Reviewed-by: Daniel Vetter Also ack for merging through -nouveau. Or ping me on irc if you want me to apply it to drm-misc-next. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V3 09/29] drm/i915: deprecate pci_get_bus_and_slot()
On Fri, 16 Feb 2018, Bjorn Helgaas wrote:
> On Mon, Nov 27, 2017 at 11:57:46AM -0500, Sinan Kaya wrote:
>> pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
>> where a PCI device is present. This restricts the device drivers to be
>> reused for other domain numbers.
>>
>> Getting ready to remove pci_get_bus_and_slot() function in favor of
>> pci_get_domain_bus_and_slot().
>>
>> Extract the domain number from drm_device and pass it into
>> pci_get_domain_bus_and_slot() function.
>>
>> Signed-off-by: Sinan Kaya
>
> I don't know what happened to this, and it didn't make it into
> v4.16-rc1. I applied it to pci/deprecate-get-bus-and-slot for v4.17
> along with the patch that actually removes pci_get_bus_and_slot().
It fell between the cracks as we couldn't apply it before getting a
backmerge on the dependency. Sorry about that.
Ack for merging through your tree.
Thanks,
Jani.
>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index 9f45cfe..5a8cb79 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -419,7 +419,10 @@ static int i915_getparam(struct drm_device *dev, void
>> *data,
>>
>> static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
>> {
>> -dev_priv->bridge_dev = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0));
>> +int domain = pci_domain_nr(dev_priv->drm.pdev->bus);
>> +
>> +dev_priv->bridge_dev =
>> +pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0));
>> if (!dev_priv->bridge_dev) {
>> DRM_ERROR("bridge device not found\n");
>> return -1;
>> --
>> 1.9.1
>>
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-ker...@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Thinkpad X1 Carbon 3rd - Reducing the compressed framebuffer size
On Tuesday 13 February 2018 19:45:56 Ville Syrjälä wrote: > On Tue, Feb 13, 2018 at 06:43:41PM +0100, Pali Rohár wrote: > > On Tuesday 13 February 2018 18:12:21 Ville Syrjälä wrote: > > > On Tue, Feb 13, 2018 at 05:04:37PM +0100, Pali Rohár wrote: > > > > So it can be done only once after reboot? Or only prior to booting > > > > kernel? > > > > > > Never. > > > > Never? Now I'm lost. Why then dmesg message instruct user to try set up > > it in BIOS if you say it is never possible? > > You can change it in the BIOS. No way to change it from the operating system. Hi! Can you explain it a bit more? What does it mean "in BIOS"? Prior switching from 16bit real mode to protected or long? Or before exiting EFI boot services? Or before booting kernel (when initialize memory mapping)? I still do not see reason nor understand why this cannot be possible either in bootloader (e.g. grub2) or prior to loading bootloader which runs in protected or long mode. It is because BIOS uses some undocumented call/procedure which sets that amount of memory and it is unknown how to do it? -- Pali Rohár pali.ro...@gmail.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Questions on page flips and atomic modeset
On Tue, Feb 06, 2018 at 11:59:37AM +0200, Oleksandr Andrushchenko wrote: > Hello, Ville! > > Thank you very much for such a comprehensive answer. Just noticed a few of your questions threads. We'd very much appreciate (for the next person creating an atomic driver) if you check out our kernel docs (under Documentation/gpu, run make htmldocs to build them) and make sure they're describing this stuff accurately. If not, patches to improve them very much welcome. Thanks, Daniel > > Please see inline > > > On 02/05/2018 06:47 PM, Ville Syrjälä wrote: > > On Mon, Feb 05, 2018 at 06:03:25PM +0200, Oleksandr Andrushchenko wrote: > > > Hello, > > > > > > > > > I have a DRM driver which implements display protocol for Xen [1] > > > and this protocol has a dedicated XENDISPL_OP_PG_FLIP request which > > > tells the other end that some display buffer needs to be displayed, > > > e.g. it is issued by the frontend DRM driver to the corresponding > > > backend when the former wants to display a buffer. > > > Two notes: > > > 1. Communication between two remote parties can obviously fail. > > > 2. At the moment only primary plane is supported, so we can think of > > > the display buffer as of CRTC's primary fb. > > > > > > There are two APIs available for user-space to initiate a page-flip > > > (please correct me if I am wrong here): legacy via > > > DRM_IOCTL_MODE_PAGE_FLIP > > > and more recent via DRM_IOCTL_MODE_ATOMIC. My current implementation > > > (which is 4.9 based) uses drm_crtc_funcs.page_flip callback to send > > > XENDISPL_OP_PG_FLIP request to the backend, so if communication fails > > > I can return an error code, so the DRM core knows that page flip > > > cannot be done. > > > > > > But now I am about to enable page flipping via DRM_IOCTL_MODE_ATOMIC for > > > which this happens without DRM_IOCTL_MODE_PAGE_FLIP, but via > > > .atomic_check + > > > .atomic_flush callbacks. > > > > > > The solution I have in mind is that I move the XENDISPL_OP_PG_FLIP request > > > to the .atomic_flush callback which seems to be the right place to serve > > > both DRM_IOCTL_MODE_PAGE_FLIP and DRM_IOCTL_MODE_ATOMIC (is this?). > > > > > > The questions I have with this are: > > > > > > 1. What is the framebuffer I can send to the backend? > > > I assume I can use crtc->primary->fb from > > > drm_crtc_helper_funcs.atomic_flush, > > > is this assumption correct? > > Planes are explicit in the atomic world, so you should just deal with > > the plane if and when it's part of the drm_atomic_state. Look at eg. > > drm_atomic_helper_commit_planes() how to figure out what's in the > > state. > Yes, this is clear. The question was about the frame buffer > I get in drm_crtc_helper_funcs.atomic_flush which only has > old_crtc_state, so I assumed I can use crtc->primary->fb there. > Or you mean I have to dig into old_crtc_state->state to find > out if there is a plane and if it has a frambuffer and if so > use it instead of crtc->primary->fb? > > And I guess you're already planning on using that helper since > > you mention .atomic_flush(). > Not directly, but via drm_mode_config_funcs.atomic_commit > > > > One should really think of the drm_atomic_state as more of a transaction > > rather than as a state (since not all objects need be part of it). > Thank you > > > 2. Is the check (either to send or not) for crtc->primary->fb != NULL > > > enough? > > > I assume there are other cases when .atomic_flush gets called, > > > so is there a way I can filter out unneeded cases? E.g. those which > > > are not for page flipping? > > Each object taking part in the transaction will have an associated > > new state and old state. > As I replied above I only have old state in .atomic_flush > > You can compare the two to figure out what > > changed, and in addition there may already some flags in the base > > class for the state that indicate what may have changed (eg. > > drm_crtc_state::mode_changed etc.). Such flags may be set by the > > core to help figure out what needs doing. > Yes, thank you > > > 3. If communication with the backend fails there is no way for me > > > to tell the DRM core about this as .atomic_flush is called after > > > "the point of no return". Do you think I can remember the error > > > code and report it next time .atomic_check is called? So, other page > > > flips will not run on broken connection, for example. > > Do you know when it might fail? > Not really, this is a software protocol to talk from > the frontend para-virtual DRM driver to its backend > in other Xen domain > > If so you should implement the appropriate > > checks in .atomic_check() etc. to try and make sure it never happens > > (barring a total hardware failure for example). > > > > Generally what we (i915) do is try to check everything up front, but if > > an unexpected error does happen later we just muddle through and log an > > error. > > > > For us I think the most common late failure is DP link training failure. > > That
Re: [PATCH 1/3] drm: simple_kms_helper: Add mode_valid() callback support
On Sat, Feb 10, 2018 at 05:08:34PM +, Eric Anholt wrote:
> Linus Walleij writes:
>
> > The PL111 needs to filter valid modes based on memory bandwidth.
> > I guess it is a pretty simple operation, so we can still claim
> > the DRM KMS helper pipeline is simple after adding this (optional)
> > vtable callback.
> >
> > Signed-off-by: Linus Walleij
> > ---
> > drivers/gpu/drm/drm_simple_kms_helper.c | 15 +++
> > include/drm/drm_simple_kms_helper.h | 15 +++
> > 2 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c
> > b/drivers/gpu/drm/drm_simple_kms_helper.c
> > index dc9fd109de14..b7840cf34808 100644
> > --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> > +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> > @@ -34,6 +34,20 @@ static const struct drm_encoder_funcs
> > drm_simple_kms_encoder_funcs = {
> > .destroy = drm_encoder_cleanup,
> > };
> >
> > +static enum drm_mode_status
> > +drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
> > + const struct drm_display_mode *mode)
> > +{
> > + struct drm_simple_display_pipe *pipe;
> > +
> > + pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
> > + if (!pipe->funcs || !pipe->funcs->mode_valid)
> > + /* Anything goes */
> > + return MODE_OK;
> > +
> > + return pipe->funcs->mode_valid(crtc, mode);
> > +}
> > +
> > static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
> > struct drm_crtc_state *state)
> > {
> > @@ -72,6 +86,7 @@ static void drm_simple_kms_crtc_disable(struct drm_crtc
> > *crtc,
> > }
> >
> > static const struct drm_crtc_helper_funcs drm_simple_kms_crtc_helper_funcs
> > = {
> > + .mode_valid = drm_simple_kms_crtc_mode_valid,
> > .atomic_check = drm_simple_kms_crtc_check,
> > .atomic_enable = drm_simple_kms_crtc_enable,
> > .atomic_disable = drm_simple_kms_crtc_disable,
> > diff --git a/include/drm/drm_simple_kms_helper.h
> > b/include/drm/drm_simple_kms_helper.h
> > index 6d9adbb46293..ad74cb33c539 100644
> > --- a/include/drm/drm_simple_kms_helper.h
> > +++ b/include/drm/drm_simple_kms_helper.h
> > @@ -21,6 +21,21 @@ struct drm_simple_display_pipe;
> > *display pipeline
> > */
> > struct drm_simple_display_pipe_funcs {
> > + /**
> > +* @mode_valid:
> > +*
> > +* This function is called to filter out valid modes from the
> > +* suggestions suggested by the bridge or display. This optional
> > +* hook is passed in when initializing the pipeline.
> > +*
> > +* RETURNS:
> > +*
> > +* MODE_OK if the mode is acceptable.
> > +* MODE_BAD if we need to try something else.
> > +*/
>
> I don't see why MODE_BAD would be the only valid error return from this
> hook. Can we just use the same RETURNS docs as other mode_valid methods?
>
> Other than that,
>
> Reviewed-by: Eric Anholt
Same comment (although note that except for dmesg noise we currently throw
them away), and I agree that mode_valid makes sense for simple drivers,
especially with all the refactoring we've done to call mode_valid both in
the connector probe and atomic_check paths. On the respun patch (I'm
behind on mails ...):
Reviewed-by: Daniel Vetter
>
> > + enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
> > + const struct drm_display_mode *mode);
> > +
> > /**
> > * @enable:
> > *
> > --
> > 2.14.3
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 33/43] drm/panel: simple: Change mode for Sharp lq123p1jx31
Hi, 2018-02-16 21:54 GMT+01:00 Doug Anderson : > Hi, > > On Fri, Feb 16, 2018 at 4:34 AM, Enric Balletbo Serra > wrote: >> Hi, >> >> 2018-01-31 17:52 GMT+01:00 Doug Anderson : >>> Hi, >>> >>> >>> On Wed, Jan 31, 2018 at 7:16 AM, Sean Paul wrote: On Wed, Jan 31, 2018 at 7:54 AM, Lucas Stach wrote: > Am Dienstag, den 30.01.2018, 21:29 +0100 schrieb Thierry Escande: >> From: Sean Paul >> >> Change the mode for Sharp lq123p1jx31 panel to something more >> rockchip-friendly such that we can use the fixed PLLs to >> generate the pixel clock > > This should really switch to a display timing instead of exposing a > single mode. The display timing has min, typical, max tuples for all > the timings values, which would allow the attached driver to vary the > timings inside the allowed bounds if it makes sense. > > Trying to hit a specific pixel clock to free up a PLL is exactly one of > the use cases envisioned for the display timings stuff. > Agreed, I think we had this discussion the first time around. We should drop this patch. Thanks for catching this! >>> >>> Are you sure we should drop this? In order for things to work >>> properly (not generate noise on the digitizer or other EMI), this >>> needs to run at a very specific pixel clock with very specific >>> blanking times. I know that earlier we had slightly different >>> blanking times and Samsung came back and said that there was noise on >>> the digitizer. I could be wrong, but I don't think there's any way >>> currently to be able to specify exactly what timings should be used on >>> a particular board. >>> >>> Don't get be wrong--I think a patch such as this one that claims a >>> single board's timings as the "right" ones for a generic panel is a >>> bit of a hack. ...but at the same time there are no other users of >>> this panel (that I know of) in mainline and the timings presented here >>> are certainly sane timings for this panel. >>> >>> In any case, previous discussion at: >>> https://patchwork.kernel.org/patch/9614603/ >>> >>> >>> ...oh, and looking at the previous discussion reminds me that the >>> timings presented in this here patch are actually not the right ones >>> (they have the right PLL, but the wrong blankings to avoid the noise >>> issues). See >>> >> >> As Thierry no longer has the hardware to test these patch series, I'll >> take care of these and follow the upstreaming process. I think that >> doesn't make sense send a v4 version of all 43 patches for this >> change. Right now, only this patch received comments so I'll wait a >> bit more for if we can get the other patches reviewed. If the others >> are fine just and I don't need to send a new version just don't apply >> this one and I will send a second version of that specific patch. Or >> even better, is really trivial what needs to be changed, so maybe the >> maintainer can do it? ;) > > Just as a heads up, Sean Paul has a series of patches to replace this > patch. The following are IDs from patchwork.kernel.org: > > 10207583 New [v3,1/6] dt-bindings: Clarify timing subnode use > as panel-timing > 10207585 New [v3,2/6] dt-bindings: Add headings to > simple-panel bindings > 10207591 New [v3,3/6] dt-bindings: Add panel-timing subnode > to simple-panel > 10207593 New [v3,4/6] drm/panel: simple: Add ability to > override typical timing > 10207595 New [v3,5/6] drm/panel: simple: Use display_timing > for lq123p1jx31 > 10207603 New [v3,6/6] arm64: dts: rockchip: Specify override > mode for kevin panel > > -Doug Nice, I was not aware of these, I'll test. That means that this patch can be removed from these series as the Sean solution is a lot better. Just a note that this patch can be removed without any collateral impact on the other patches, so just ignore it. Regards, Enric ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/5] drm: xlnx: Xilinx DRM KMS module
On Tue, Feb 06, 2018 at 05:36:36PM -0800, Hyun Kwon wrote: > Xilinx has various platforms for display, where users can create > using multiple IPs in the programmable FPGA fabric, or where > some hardened piepline is available on the chip. Furthermore, > hardened pipeline can also interact with soft logics in FPGA. > > The Xilinx DRM KMS module is to integrate multiple subdevices and > to represent the entire pipeline as a single DRM device. The module > includes helper (ex, framebuffer and gem helpers) and > glue logic (ex, crtc interface) functions. > > Signed-off-by: Hyun Kwon > Acked-by: Daniel Vetter Looks all ready for merging. Did you apply for commit rights to drm-misc already so you could push this right away? -Daniel > --- > v5 > - Redefine xlnx_pipeline_init() > v4 > - Fix a bug in of graph binding handling > - Remove vblank callbacks from xlnx_crtc > - Remove the dt binding. This module becomes more like a library. > - Rephrase the commit message > v3 > - Add Laurent as a maintainer > - Fix multiple-reference on gem objects > v2 > - Change the SPDX identifier format > - Merge patches(crtc, gem, fb) into single one > v2 of xlnx_drv > - Rename kms to display in xlnx_drv > - Replace some xlnx specific fb helper with common helpers in xlnx_drv > - Don't set the commit tail callback in xlnx_drv > - Support 'ports' graph binding in xlnx_drv > v2 of xlnx_fb > - Remove wrappers in xlnx_fb > - Replace some functions with drm core helpers in xlnx_fb > --- > --- > MAINTAINERS | 9 + > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/xlnx/Kconfig | 12 + > drivers/gpu/drm/xlnx/Makefile| 2 + > drivers/gpu/drm/xlnx/xlnx_crtc.c | 177 ++ > drivers/gpu/drm/xlnx/xlnx_crtc.h | 70 ++ > drivers/gpu/drm/xlnx/xlnx_drv.c | 501 > +++ > drivers/gpu/drm/xlnx/xlnx_drv.h | 33 +++ > drivers/gpu/drm/xlnx/xlnx_fb.c | 298 +++ > drivers/gpu/drm/xlnx/xlnx_fb.h | 33 +++ > drivers/gpu/drm/xlnx/xlnx_gem.c | 47 > drivers/gpu/drm/xlnx/xlnx_gem.h | 26 ++ > 13 files changed, 1211 insertions(+) > create mode 100644 drivers/gpu/drm/xlnx/Kconfig > create mode 100644 drivers/gpu/drm/xlnx/Makefile > create mode 100644 drivers/gpu/drm/xlnx/xlnx_crtc.c > create mode 100644 drivers/gpu/drm/xlnx/xlnx_crtc.h > create mode 100644 drivers/gpu/drm/xlnx/xlnx_drv.c > create mode 100644 drivers/gpu/drm/xlnx/xlnx_drv.h > create mode 100644 drivers/gpu/drm/xlnx/xlnx_fb.c > create mode 100644 drivers/gpu/drm/xlnx/xlnx_fb.h > create mode 100644 drivers/gpu/drm/xlnx/xlnx_gem.c > create mode 100644 drivers/gpu/drm/xlnx/xlnx_gem.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 5bc088f..07c0e73 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4789,6 +4789,15 @@ F: drivers/gpu/drm/etnaviv/ > F: include/uapi/drm/etnaviv_drm.h > F: Documentation/devicetree/bindings/display/etnaviv/ > > +DRM DRIVERS FOR XILINX > +M: Hyun Kwon > +M: Laurent Pinchart > +L: dri-devel@lists.freedesktop.org > +S: Maintained > +F: drivers/gpu/drm/xlnx/ > +F: Documentation/devicetree/bindings/display/xlnx/ > +T: git git://anongit.freedesktop.org/drm/drm-misc > + > DRM DRIVERS FOR ZTE ZX > M: Shawn Guo > L: dri-devel@lists.freedesktop.org > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index deeefa7..5a3ec66 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -289,6 +289,8 @@ source "drivers/gpu/drm/pl111/Kconfig" > > source "drivers/gpu/drm/tve200/Kconfig" > > +source "drivers/gpu/drm/xlnx/Kconfig" > + > # Keep legacy drivers last > > menuconfig DRM_LEGACY > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 50093ff..f93557e 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -103,3 +103,4 @@ obj-$(CONFIG_DRM_MXSFB) += mxsfb/ > obj-$(CONFIG_DRM_TINYDRM) += tinydrm/ > obj-$(CONFIG_DRM_PL111) += pl111/ > obj-$(CONFIG_DRM_TVE200) += tve200/ > +obj-$(CONFIG_DRM_XLNX) += xlnx/ > diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig > new file mode 100644 > index 000..19fd7cd > --- /dev/null > +++ b/drivers/gpu/drm/xlnx/Kconfig > @@ -0,0 +1,12 @@ > +config DRM_XLNX > + tristate "Xilinx DRM KMS Driver" > + depends on DRM && OF > + select DRM_KMS_HELPER > + select DRM_KMS_CMA_HELPER > + select DRM_GEM_CMA_HELPER > + help > + Xilinx DRM KMS driver. Choose this option if you have > + a Xilinx SoCs with hardened display pipeline or soft > + display pipeline using Xilinx IPs in FPGA. This module > + provides the kernel mode setting functionalities > + for Xilinx display drivers. > diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile > new file mode 100644 > index 000..c60a281 > --- /dev/null > +++ b/drivers/gpu/drm/xlnx/Makefile > @@ -0,0 +1,
Re: Questions on page flips and atomic modeset
On 02/19/2018 11:36 AM, Daniel Vetter wrote: On Tue, Feb 06, 2018 at 11:59:37AM +0200, Oleksandr Andrushchenko wrote: Hello, Ville! Thank you very much for such a comprehensive answer. Just noticed a few of your questions threads. We'd very much appreciate (for the next person creating an atomic driver) if you check out our kernel docs (under Documentation/gpu, run make htmldocs to build them) and make sure they're describing this stuff accurately. If not, patches to improve them very much welcome. sure, I already did some before when I read unclear description for GEM/dma-buf [1] ;) Thanks, Daniel Please see inline On 02/05/2018 06:47 PM, Ville Syrjälä wrote: On Mon, Feb 05, 2018 at 06:03:25PM +0200, Oleksandr Andrushchenko wrote: Hello, I have a DRM driver which implements display protocol for Xen [1] and this protocol has a dedicated XENDISPL_OP_PG_FLIP request which tells the other end that some display buffer needs to be displayed, e.g. it is issued by the frontend DRM driver to the corresponding backend when the former wants to display a buffer. Two notes: 1. Communication between two remote parties can obviously fail. 2. At the moment only primary plane is supported, so we can think of the display buffer as of CRTC's primary fb. There are two APIs available for user-space to initiate a page-flip (please correct me if I am wrong here): legacy via DRM_IOCTL_MODE_PAGE_FLIP and more recent via DRM_IOCTL_MODE_ATOMIC. My current implementation (which is 4.9 based) uses drm_crtc_funcs.page_flip callback to send XENDISPL_OP_PG_FLIP request to the backend, so if communication fails I can return an error code, so the DRM core knows that page flip cannot be done. But now I am about to enable page flipping via DRM_IOCTL_MODE_ATOMIC for which this happens without DRM_IOCTL_MODE_PAGE_FLIP, but via .atomic_check + .atomic_flush callbacks. The solution I have in mind is that I move the XENDISPL_OP_PG_FLIP request to the .atomic_flush callback which seems to be the right place to serve both DRM_IOCTL_MODE_PAGE_FLIP and DRM_IOCTL_MODE_ATOMIC (is this?). The questions I have with this are: 1. What is the framebuffer I can send to the backend? I assume I can use crtc->primary->fb from drm_crtc_helper_funcs.atomic_flush, is this assumption correct? Planes are explicit in the atomic world, so you should just deal with the plane if and when it's part of the drm_atomic_state. Look at eg. drm_atomic_helper_commit_planes() how to figure out what's in the state. Yes, this is clear. The question was about the frame buffer I get in drm_crtc_helper_funcs.atomic_flush which only has old_crtc_state, so I assumed I can use crtc->primary->fb there. Or you mean I have to dig into old_crtc_state->state to find out if there is a plane and if it has a frambuffer and if so use it instead of crtc->primary->fb? And I guess you're already planning on using that helper since you mention .atomic_flush(). Not directly, but via drm_mode_config_funcs.atomic_commit One should really think of the drm_atomic_state as more of a transaction rather than as a state (since not all objects need be part of it). Thank you 2. Is the check (either to send or not) for crtc->primary->fb != NULL enough? I assume there are other cases when .atomic_flush gets called, so is there a way I can filter out unneeded cases? E.g. those which are not for page flipping? Each object taking part in the transaction will have an associated new state and old state. As I replied above I only have old state in .atomic_flush You can compare the two to figure out what changed, and in addition there may already some flags in the base class for the state that indicate what may have changed (eg. drm_crtc_state::mode_changed etc.). Such flags may be set by the core to help figure out what needs doing. Yes, thank you 3. If communication with the backend fails there is no way for me to tell the DRM core about this as .atomic_flush is called after "the point of no return". Do you think I can remember the error code and report it next time .atomic_check is called? So, other page flips will not run on broken connection, for example. Do you know when it might fail? Not really, this is a software protocol to talk from the frontend para-virtual DRM driver to its backend in other Xen domain If so you should implement the appropriate checks in .atomic_check() etc. to try and make sure it never happens (barring a total hardware failure for example). Generally what we (i915) do is try to check everything up front, but if an unexpected error does happen later we just muddle through and log an error. For us I think the most common late failure is DP link training failure. That we can't fully check up front since it depends on external factors: sink, dongles, cabling, etc. For those we added a new connector property to signal to userspace that the link is having issues, allowing userspace to reconfigure things such that we lower the link bandwidth require
Re: [PATCH] drm/bochs: make structure bochs_bo_driver static
On Wed, Feb 07, 2018 at 11:13:53AM +, Colin King wrote:
> From: Colin Ian King
>
> The structure bochs_bo_driver is local to the source and does not need to
> be in global scope, so make it static.
>
> Cleans up sparse warning:
> drivers/gpu/drm/bochs/bochs_mm.c:197:22: warning: symbol 'bochs_bo_driver'
> was not declared. Should it be static?
>
> Signed-off-by: Colin Ian King
Applied to drm-misc-next, thanks.
-Daniel
> ---
> drivers/gpu/drm/bochs/bochs_mm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bochs/bochs_mm.c
> b/drivers/gpu/drm/bochs/bochs_mm.c
> index 704e879711e4..b1d5aee46316 100644
> --- a/drivers/gpu/drm/bochs/bochs_mm.c
> +++ b/drivers/gpu/drm/bochs/bochs_mm.c
> @@ -194,7 +194,7 @@ static struct ttm_tt *bochs_ttm_tt_create(struct
> ttm_bo_device *bdev,
> return tt;
> }
>
> -struct ttm_bo_driver bochs_bo_driver = {
> +static struct ttm_bo_driver bochs_bo_driver = {
> .ttm_tt_create = bochs_ttm_tt_create,
> .ttm_tt_populate = ttm_pool_populate,
> .ttm_tt_unpopulate = ttm_pool_unpopulate,
> --
> 2.15.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V3 09/29] drm/i915: deprecate pci_get_bus_and_slot()
Quoting Jani Nikula (2018-02-19 11:34:34) > On Fri, 16 Feb 2018, Bjorn Helgaas wrote: > > On Mon, Nov 27, 2017 at 11:57:46AM -0500, Sinan Kaya wrote: > >> pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as > >> where a PCI device is present. This restricts the device drivers to be > >> reused for other domain numbers. > >> > >> Getting ready to remove pci_get_bus_and_slot() function in favor of > >> pci_get_domain_bus_and_slot(). > >> > >> Extract the domain number from drm_device and pass it into > >> pci_get_domain_bus_and_slot() function. > >> > >> Signed-off-by: Sinan Kaya > > > > I don't know what happened to this, and it didn't make it into > > v4.16-rc1. I applied it to pci/deprecate-get-bus-and-slot for v4.17 > > along with the patch that actually removes pci_get_bus_and_slot(). > > It fell between the cracks as we couldn't apply it before getting a > backmerge on the dependency. Sorry about that. > > Ack for merging through your tree. I just retested the patch and it still passes CI. We also now have the dependency in our tree through the backmerge, so I can send this for the next drm-next pull request. Either way suits me. Regards, Joonas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: add func to get max iomem address
On Thu, Feb 08, 2018 at 02:00:15PM +0800, Chunming Zhou wrote:
> it will be used to check if the driver needs swiotlb
>
> Change-Id: Idbe47af8f12032d4803bb3d47273e807f19169c3
> Signed-off-by: Chunming Zhou
> ---
> include/drm/drm_cache.h | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/drm/drm_cache.h b/include/drm/drm_cache.h
> index beab0f0d0cfb..442c9ba63d03 100644
> --- a/include/drm/drm_cache.h
> +++ b/include/drm/drm_cache.h
> @@ -39,6 +39,19 @@ void drm_clflush_pages(struct page *pages[], unsigned long
> num_pages);
> void drm_clflush_sg(struct sg_table *st);
> void drm_clflush_virt_range(void *addr, unsigned long length);
>
> +static inline u64 drm_get_max_iomem(void)
> +{
> + struct resource *tmp;
> + u64 max_iomem = 0;
> +
> + for (tmp = iomem_resource.child; tmp; tmp = tmp->sibling) {
> + max_iomem = max(max_iomem, tmp->end);
> + }
> +
> + return max_iomem;
> +}
linux-next noticed a compile warning on this (on some platforms, the
max_iomem has the wrong integer size I think). Also, drm_memory.c is a bit
a grab-bag of legacy stuff, not sure this is the best place. We're also
lacking kerneldoc (plus include stanza) for this new function.
Can you pls fix all that in a follow-up patch?
Thanks, Daniel
> +
> +
> static inline bool drm_arch_can_wc_memory(void)
> {
> #if defined(CONFIG_PPC) && !defined(CONFIG_NOT_COHERENT_CACHE)
> --
> 2.14.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] PCI: Restore BARs on runtime resume despite being unbound
On Sun, Feb 18, 2018 at 9:38 AM, Lukas Wunner wrote:
> PCI devices not bound to a driver are supposed to stay in D0 during
> runtime suspend. But they may have a parent which is bound and can be
> transitioned to D3cold at runtime. Once the parent goes to D3cold, the
> unbound child may go to D3cold as well. When the child comes out of
> D3cold, its BARs are uninitialized and thus inaccessible when a driver
> tries to probe.
>
> One example are recent hybrid graphics laptops which cut power to the
> discrete GPU when the root port above it goes to ACPI power state D3.
> Users may provoke this by unbinding the GPU driver and allowing runtime
> PM on the GPU via sysfs: The PM core will then treat the GPU as
> "suspended", which in turn allows the root port to runtime suspend,
> causing the power resources listed in its _PR3 object to be powered off.
> The GPU's BARs will be uninitialized when a driver later probes it.
>
> Another example are hybrid graphics laptops where the GPU itself (rather
> than the root port) is capable of runtime suspending to D3cold. If the
> GPU's integrated HDA controller is not bound and the GPU's driver
> decides to runtime suspend to D3cold, the HDA controller's BARs will be
> uninitialized when a driver later probes it.
>
> Fix by restoring the BARs on runtime resume if the device is not bound.
> This is sufficient to fix the above-mentioned use cases. Other use
> cases might require a full-blown pci_save_state() / pci_restore_state()
> or execution of fixups. We can add that once use cases materialize,
> let's not inflate the code unnecessarily.
>
> Cc: Bjorn Helgaas
> Cc: Rafael J. Wysocki
> Signed-off-by: Lukas Wunner
Reviewed-by: Rafael J. Wysocki
> ---
> drivers/pci/pci-driver.c | 8 ++--
> drivers/pci/pci.c| 2 +-
> drivers/pci/pci.h| 1 +
> 3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 3bed6beda051..51b11cbd48f6 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1277,10 +1277,14 @@ static int pci_pm_runtime_resume(struct device *dev)
>
> /*
> * If pci_dev->driver is not set (unbound), the device should
> -* always remain in D0 regardless of the runtime PM status
> +* always remain in D0 regardless of the runtime PM status.
> +* But if its parent can go to D3cold, this device may have
> +* been in D3cold as well and require restoration of its BARs.
> */
> - if (!pci_dev->driver)
> + if (!pci_dev->driver) {
> + pci_restore_bars(pci_dev);
> return 0;
> + }
>
> if (!pm || !pm->runtime_resume)
> return -ENOSYS;
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index f6a4dd10d9b0..f694650235f2 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -563,7 +563,7 @@ int pci_wait_for_pending(struct pci_dev *dev, int pos,
> u16 mask)
> * Restore the BAR values for a given device, so as to make it
> * accessible by its driver.
> */
> -static void pci_restore_bars(struct pci_dev *dev)
> +void pci_restore_bars(struct pci_dev *dev)
> {
> int i;
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index fcd81911b127..29dc15bbe3bf 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -83,6 +83,7 @@ void pci_allocate_cap_save_buffers(struct pci_dev *dev);
> void pci_free_cap_save_buffers(struct pci_dev *dev);
> bool pci_bridge_d3_possible(struct pci_dev *dev);
> void pci_bridge_d3_update(struct pci_dev *dev);
> +void pci_restore_bars(struct pci_dev *dev);
>
> static inline void pci_wakeup_event(struct pci_dev *dev)
> {
> --
> 2.15.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Fix kerneldoc warnings for drm_lease
On Thu, Feb 08, 2018 at 11:08:17AM +, Chris Wilson wrote:
> drivers/gpu/drm/drm_lease.c:56: warning: No description found for parameter
> 'lessee_id'
> drivers/gpu/drm/drm_lease.c:56: warning: Excess function parameter 'id'
> description in '_drm_find_lessee'
> drivers/gpu/drm/drm_lease.c:114: warning: No description found for parameter
> 'file_priv'
> drivers/gpu/drm/drm_lease.c:114: warning: Excess function parameter 'master'
> description in '_drm_lease_held'
> drivers/gpu/drm/drm_lease.c:134: warning: No description found for parameter
> 'file_priv'
> drivers/gpu/drm/drm_lease.c:134: warning: Excess function parameter 'master'
> description in 'drm_lease_held'
> drivers/gpu/drm/drm_lease.c:158: warning: No description found for parameter
> 'crtcs_in'
> drivers/gpu/drm/drm_lease.c:158: warning: Excess function parameter 'crtcs'
> description in 'drm_lease_filter_crtcs'
> drivers/gpu/drm/drm_lease.c:311: warning: No description found for parameter
> 'top'
> drivers/gpu/drm/drm_lease.c:311: warning: Excess function parameter 'master'
> description in '_drm_lease_revoke'
> drivers/gpu/drm/drm_lease.c:494: warning: No description found for parameter
> 'lessor_priv'
> drivers/gpu/drm/drm_lease.c:494: warning: Excess function parameter
> 'file_priv' description in 'drm_mode_create_lease_ioctl'
> drivers/gpu/drm/drm_lease.c:672: warning: No description found for parameter
> 'lessee_priv'
> drivers/gpu/drm/drm_lease.c:672: warning: Excess function parameter
> 'file_priv' description in 'drm_mode_get_lease_ioctl'
> drivers/gpu/drm/drm_lease.c:733: warning: No description found for parameter
> 'lessor_priv'
> drivers/gpu/drm/drm_lease.c:733: warning: Excess function parameter
> 'file_priv' description in 'drm_mode_revoke_lease_ioctl'
>
> Signed-off-by: Chris Wilson
Applied, thanks.
-Daniel
> ---
> drivers/gpu/drm/drm_lease.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> index 1402c0e71b03..d345563fdff3 100644
> --- a/drivers/gpu/drm/drm_lease.c
> +++ b/drivers/gpu/drm/drm_lease.c
> @@ -44,7 +44,7 @@ EXPORT_SYMBOL(drm_lease_owner);
> /**
> * _drm_find_lessee - find lessee by id (idr_mutex held)
> * @master: drm_master of lessor
> - * @id: lessee_id
> + * @lessee_id: id
> *
> * RETURN:
> *
> @@ -101,7 +101,7 @@ static bool _drm_has_leased(struct drm_master *master,
> int id)
>
> /**
> * _drm_lease_held - check drm_mode_object lease status (idr_mutex held)
> - * @master: the drm_master
> + * @file_priv: the master drm_file
> * @id: the object id
> *
> * Checks if the specified master holds a lease on the object. Return
> @@ -121,7 +121,7 @@ EXPORT_SYMBOL(_drm_lease_held);
>
> /**
> * drm_lease_held - check drm_mode_object lease status (idr_mutex not held)
> - * @master: the drm_master
> + * @file_priv: the master drm_file
> * @id: the object id
> *
> * Checks if the specified master holds a lease on the object. Return
> @@ -149,7 +149,7 @@ EXPORT_SYMBOL(drm_lease_held);
> /**
> * drm_lease_filter_crtcs - restricted crtc set to leased values (idr_mutex
> not held)
> * @file_priv: requestor file
> - * @crtcs: bitmask of crtcs to check
> + * @crtcs_in: bitmask of crtcs to check
> *
> * Reconstructs a crtc mask based on the crtcs which are visible
> * through the specified file.
> @@ -305,7 +305,7 @@ void drm_lease_destroy(struct drm_master *master)
>
> /**
> * _drm_lease_revoke - revoke access to all leased objects (idr_mutex held)
> - * @master: the master losing its lease
> + * @top: the master losing its lease
> */
> static void _drm_lease_revoke(struct drm_master *top)
> {
> @@ -482,7 +482,7 @@ static int fill_object_idr(struct drm_device *dev,
> * drm_mode_create_lease_ioctl - create a new lease
> * @dev: the drm device
> * @data: pointer to struct drm_mode_create_lease
> - * @file_priv: the file being manipulated
> + * @lessor_priv: the file being manipulated
> *
> * The master associated with the specified file will have a lease
> * created containing the objects specified in the ioctl structure.
> @@ -662,7 +662,7 @@ int drm_mode_list_lessees_ioctl(struct drm_device *dev,
> * drm_mode_get_lease_ioctl - list leased objects
> * @dev: the drm device
> * @data: pointer to struct drm_mode_get_lease
> - * @file_priv: the file being manipulated
> + * @lessee_priv: the file being manipulated
> *
> * Return the list of leased objects for the specified lessee
> */
> @@ -722,7 +722,7 @@ int drm_mode_get_lease_ioctl(struct drm_device *dev,
> * drm_mode_revoke_lease_ioctl - revoke lease
> * @dev: the drm device
> * @data: pointer to struct drm_mode_revoke_lease
> - * @file_priv: the file being manipulated
> + * @lessor_priv: the file being manipulated
> *
> * This removes all of the objects from the lease without
> * actually getting rid of the lease itself; that way all
> --
> 2.16.1
>
>
Re: [PATCH] dma-buf/sw_sync: Fix kerneldoc warnings
On Thu, Feb 08, 2018 at 11:38:16AM +, Chris Wilson wrote: > drivers/dma-buf/sw_sync.c:248: warning: No description found for parameter > 'obj' > drivers/dma-buf/sw_sync.c:248: warning: No description found for parameter > 'value' > drivers/dma-buf/sw_sync.c:248: warning: Excess function parameter 'parent' > description in 'sync_pt_create' > drivers/dma-buf/sw_sync.c:248: warning: Excess function parameter 'inc' > description in 'sync_pt_create' > > Signed-off-by: Chris Wilson Applied, thanks. -Daniel > --- > drivers/dma-buf/sw_sync.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c > index 7779bdbd18d1..3d78ca89a605 100644 > --- a/drivers/dma-buf/sw_sync.c > +++ b/drivers/dma-buf/sw_sync.c > @@ -235,10 +235,10 @@ static void sync_timeline_signal(struct sync_timeline > *obj, unsigned int inc) > > /** > * sync_pt_create() - creates a sync pt > - * @parent: fence's parent sync_timeline > - * @inc: value of the fence > + * @obj: parent sync_timeline > + * @value: value of the fence > * > - * Creates a new sync_pt as a child of @parent. @size bytes will be > + * Creates a new sync_pt (fence) as a child of @parent. @size bytes will be > * allocated allowing for implementation specific data to be kept after > * the generic sync_timeline struct. Returns the sync_pt object or > * NULL in case of error. > -- > 2.16.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] locking/ww_mutex: add ww_mutex_is_owned_by function
amdgpu needs to verify if userspace sends us valid addresses and the simplest
way of doing this is to check if the buffer object is locked with the ticket
of the current submission.
Clean up the access to the ww_mutex internals by providing a function
for this and extend the check to the thread owning the underlying mutex.
Signed-off-by: Christian König
---
include/linux/ww_mutex.h | 17 +
1 file changed, 17 insertions(+)
diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
index 39fda195bf78..dd580db289e8 100644
--- a/include/linux/ww_mutex.h
+++ b/include/linux/ww_mutex.h
@@ -358,4 +358,21 @@ static inline bool ww_mutex_is_locked(struct ww_mutex
*lock)
return mutex_is_locked(&lock->base);
}
+/**
+ * ww_mutex_is_owned_by - is the w/w mutex locked by this task in that context
+ * @lock: the mutex to be queried
+ * @task: the task structure to check
+ * @ctx: the w/w acquire context to test
+ *
+ * Returns true if the mutex is locked in the context by the given task, false
+ * otherwise.
+ */
+static inline bool ww_mutex_is_owned_by(struct ww_mutex *lock,
+ struct task_struct *task,
+ struct ww_acquire_ctx *ctx)
+{
+ return likely(__mutex_owner(&lock->base) == task) &&
+ READ_ONCE(lock->ctx) == ctx;
+}
+
#endif
--
2.14.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/ttm: handle already locked BOs during eviction and swapout.
This solves the problem that when we swapout a BO from a domain we
sometimes couldn't make room for it because holding the lock blocks all
other BOs with this reservation object.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d90b1cf10b27..fba40e22d088 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -713,31 +713,30 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object
*bo,
EXPORT_SYMBOL(ttm_bo_eviction_valuable);
/**
- * Check the target bo is allowable to be evicted or swapout, including cases:
- *
- * a. if share same reservation object with ctx->resv, have assumption
- * reservation objects should already be locked, so not lock again and
- * return true directly when either the opreation allow_reserved_eviction
- * or the target bo already is in delayed free list;
- *
- * b. Otherwise, trylock it.
+ * Check if the target bo is allowed to be evicted or swapedout.
*/
static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo,
- struct ttm_operation_ctx *ctx, bool *locked)
+ struct ttm_operation_ctx *ctx,
+ bool *locked)
{
- bool ret = false;
+ /* First check if we can lock it */
+ *locked = reservation_object_trylock(bo->resv);
+ if (*locked)
+ return true;
- *locked = false;
+ /* Check if it's locked because it is part of the current operation */
if (bo->resv == ctx->resv) {
reservation_object_assert_held(bo->resv);
- if (ctx->allow_reserved_eviction || !list_empty(&bo->ddestroy))
- ret = true;
- } else {
- *locked = reservation_object_trylock(bo->resv);
- ret = *locked;
+ return ctx->allow_reserved_eviction ||
+ !list_empty(&bo->ddestroy);
}
- return ret;
+ /* Check if it's locked because it was already evicted */
+ if (ww_mutex_is_owned_by(&bo->resv->lock, current, NULL))
+ return true;
+
+ /* Some other thread is using it, don't touch it */
+ return false;
}
static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
--
2.14.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: add func to get max iomem address v2
On Fri, Feb 16, 2018 at 02:40:11PM +, Chris Wilson wrote:
> Quoting Chunming Zhou (2018-02-09 02:44:08)
> > it will be used to check if the driver needs swiotlb
> > v2: Don't use inline, instead, move function to drm_memory.c (Mechel
> > Daenzer )
> >
> > Change-Id: Idbe47af8f12032d4803bb3d47273e807f19169c3
> > Signed-off-by: Chunming Zhou
> > Reviewed-by: Monk Liu
> > Reviewed-by: Christian König
> > ---
> > drivers/gpu/drm/drm_memory.c | 13 +
> > include/drm/drm_cache.h | 2 ++
> > 2 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
> > index fc0ebd273ef8..7ca500b8c399 100644
> > --- a/drivers/gpu/drm/drm_memory.c
> > +++ b/drivers/gpu/drm/drm_memory.c
> > @@ -149,3 +149,16 @@ void drm_legacy_ioremapfree(struct drm_local_map *map,
> > struct drm_device *dev)
> > iounmap(map->handle);
> > }
> > EXPORT_SYMBOL(drm_legacy_ioremapfree);
> > +
> > +u64 drm_get_max_iomem(void)
> > +{
> > + struct resource *tmp;
> > + u64 max_iomem = 0;
> > +
> > + for (tmp = iomem_resource.child; tmp; tmp = tmp->sibling) {
> > + max_iomem = max(max_iomem, tmp->end);
>
> Note that resource.end is of type resource_size_t which is not u64 on
> 32b.
>
> Either use max_t(u64) or resource_size_t max_iomem with the implicit
> cast on the return.
Yeah linux-next spotted that one too, pls fix in a follow-up with the
other things I've raised (lack of kerneldoc mostly).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/amdgpu: use new ww_mutex_is_owned_by function
Instead of accessing ww_mutex internals directly use the provided
function to check if the ww_mutex was indeed locked by the current
command submission.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index eaa3cb0c3ad1..4c04b560e358 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1594,7 +1594,8 @@ int amdgpu_cs_find_mapping(struct amdgpu_cs_parser
*parser,
*map = mapping;
/* Double check that the BO is reserved by this CS */
- if (READ_ONCE((*bo)->tbo.resv->lock.ctx) != &parser->ticket)
+ if (!ww_mutex_is_owned_by(&(*bo)->tbo.resv->lock, current,
+ &parser->ticket))
return -EINVAL;
if (!((*bo)->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)) {
--
2.14.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/ttm: keep BOs reserved until end of eviction
This avoids problems when BOs are evicted but directly moved back into
the domain from other threads.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 37 +
1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index fba40e22d088..568cf216b374 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -742,7 +742,8 @@ static bool ttm_bo_evict_swapout_allowable(struct
ttm_buffer_object *bo,
static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
uint32_t mem_type,
const struct ttm_place *place,
- struct ttm_operation_ctx *ctx)
+ struct ttm_operation_ctx *ctx,
+ struct list_head *evicted)
{
struct ttm_bo_global *glob = bdev->glob;
struct ttm_mem_type_manager *man = &bdev->man[mem_type];
@@ -792,17 +793,28 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
ret = ttm_bo_evict(bo, ctx);
if (locked) {
- ttm_bo_unreserve(bo);
+ list_add_tail(&bo->lru, evicted);
} else {
spin_lock(&glob->lru_lock);
ttm_bo_add_to_lru(bo);
spin_unlock(&glob->lru_lock);
+ kref_put(&bo->list_kref, ttm_bo_release_list);
}
- kref_put(&bo->list_kref, ttm_bo_release_list);
return ret;
}
+static void ttm_mem_evict_cleanup(struct list_head *evicted)
+{
+ struct ttm_buffer_object *bo, *tmp;
+
+ list_for_each_entry_safe(bo, tmp, evicted, lru) {
+ list_del_init(&bo->lru);
+ ttm_bo_unreserve(bo);
+ kref_put(&bo->list_kref, ttm_bo_release_list);
+ }
+}
+
void ttm_bo_mem_put(struct ttm_buffer_object *bo, struct ttm_mem_reg *mem)
{
struct ttm_mem_type_manager *man = &bo->bdev->man[mem->mem_type];
@@ -852,20 +864,26 @@ static int ttm_bo_mem_force_space(struct
ttm_buffer_object *bo,
{
struct ttm_bo_device *bdev = bo->bdev;
struct ttm_mem_type_manager *man = &bdev->man[mem_type];
+ struct list_head evicted;
int ret;
+ INIT_LIST_HEAD(&evicted);
do {
ret = (*man->func->get_node)(man, bo, place, mem);
if (unlikely(ret != 0))
return ret;
if (mem->mm_node)
break;
- ret = ttm_mem_evict_first(bdev, mem_type, place, ctx);
+ ret = ttm_mem_evict_first(bdev, mem_type, place, ctx, &evicted);
if (unlikely(ret != 0))
- return ret;
+ goto error;
} while (1);
mem->mem_type = mem_type;
- return ttm_bo_add_move_fence(bo, man, mem);
+ ret = ttm_bo_add_move_fence(bo, man, mem);
+
+error:
+ ttm_mem_evict_cleanup(&evicted);
+ return ret;
}
static uint32_t ttm_bo_select_caching(struct ttm_mem_type_manager *man,
@@ -1345,6 +1363,7 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device
*bdev,
struct ttm_operation_ctx ctx = { false, false };
struct ttm_mem_type_manager *man = &bdev->man[mem_type];
struct ttm_bo_global *glob = bdev->glob;
+ struct list_head evicted;
struct dma_fence *fence;
int ret;
unsigned i;
@@ -1352,18 +1371,20 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device
*bdev,
/*
* Can't use standard list traversal since we're unlocking.
*/
-
+ INIT_LIST_HEAD(&evicted);
spin_lock(&glob->lru_lock);
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
while (!list_empty(&man->lru[i])) {
spin_unlock(&glob->lru_lock);
- ret = ttm_mem_evict_first(bdev, mem_type, NULL, &ctx);
+ ret = ttm_mem_evict_first(bdev, mem_type, NULL, &ctx,
+ &evicted);
if (ret)
return ret;
spin_lock(&glob->lru_lock);
}
}
spin_unlock(&glob->lru_lock);
+ ttm_mem_evict_cleanup(&evicted);
spin_lock(&man->move_lock);
fence = dma_fence_get(man->move);
--
2.14.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] gpu/drm/udl: Replace struct_mutex with driver private lock
On Sat, Feb 10, 2018 at 06:47:31PM +0530, Shreeya Patel wrote:
> On Fri, 2018-02-09 at 12:18 +, Chris Wilson wrote:
> > Quoting Shreeya Patel (2018-02-09 12:10:56)
> > >
> > > dev->struct_mutex is the Big DRM Lock and the only bit where
> > > it’s mandatory is serializing GEM buffer object destruction.
> > > Which unfortunately means drivers have to keep track of that
> > > lock and either call unreference or unreference_locked
> > > depending upon context. Core GEM doesn’t have a need for
> > > struct_mutex any more since kernel 4.8.
> > >
> > > For drivers that need struct_mutex it should be replaced by a
> > > driver
> > > private lock.
> > In that regard, dev->struct_mutex is already a driver private lock.
> > The
> > impetus is to reduce a big lock into lots of little locks where
> > contention deems prudent.
>
> Ok. I'll make the changes in the commit message.
udl_driver_load would be a good place. Also, running with
CONFIG_PROVE_LOCKING enabled will catch this stuff. On that note, do you
have the hardware to test your changes? If not we need to find someone who
can do that.
Also please switch udl from the gem_free_object to the
gem_free_object_unlocked hook.
-Daniel
> >
> > >
> > > Signed-off-by: Shreeya Patel
> > > ---
> > > drivers/gpu/drm/udl/udl_dmabuf.c | 5 +++--
> > > drivers/gpu/drm/udl/udl_drv.h| 1 +
> > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c
> > > b/drivers/gpu/drm/udl/udl_dmabuf.c
> > > index 2867ed1..120d2d9 100644
> > > --- a/drivers/gpu/drm/udl/udl_dmabuf.c
> > > +++ b/drivers/gpu/drm/udl/udl_dmabuf.c
> > > @@ -76,6 +76,7 @@ static struct sg_table *udl_map_dma_buf(struct
> > > dma_buf_attachment *attach,
> > > struct udl_drm_dmabuf_attachment *udl_attach = attach-
> > > >priv;
> > > struct udl_gem_object *obj = to_udl_bo(attach->dmabuf-
> > > >priv);
> > > struct drm_device *dev = obj->base.dev;
> > > + struct udl_device *udl = dev->dev_private;
> > > struct scatterlist *rd, *wr;
> > > struct sg_table *sgt = NULL;
> > > unsigned int i;
> > > @@ -112,7 +113,7 @@ static struct sg_table *udl_map_dma_buf(struct
> > > dma_buf_attachment *attach,
> > > return ERR_PTR(-ENOMEM);
> > > }
> > >
> > > - mutex_lock(&dev->struct_mutex);
> > > + mutex_lock(&udl->urbs.plock);
> > >
> > > rd = obj->sg->sgl;
> > > wr = sgt->sgl;
> > > @@ -137,7 +138,7 @@ static struct sg_table *udl_map_dma_buf(struct
> > > dma_buf_attachment *attach,
> > > attach->priv = udl_attach;
> > >
> > > err_unlock:
> > > - mutex_unlock(&dev->struct_mutex);
> > > + mutex_unlock(&udl->urbs.plock);
> > > return sgt;
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/udl/udl_drv.h
> > > b/drivers/gpu/drm/udl/udl_drv.h
> > > index 2a75ab8..24cca17 100644
> > > --- a/drivers/gpu/drm/udl/udl_drv.h
> > > +++ b/drivers/gpu/drm/udl/udl_drv.h
> > > @@ -39,6 +39,7 @@ struct urb_node {
> > >
> > > struct urb_list {
> > > struct list_head list;
> > > + struct mutex plock;
> > > spinlock_t lock;
> > > struct semaphore limit_sem;
> > > int available;
> > This hasn't seen much testing as it lacks a mutex_init, and one would
> > wish for a description of what it is guarding.
>
> Yes, I'll add mutex_init but I am not sure that in which function I
> should add it as there is no probe or init function.
>
> Also I will add the description for the lock.
>
> Thanks
> > -Chris
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Add support for in-line nested struct comments
On Sun, 18 Feb 2018, Jonathan Corbet wrote: > On Fri, 16 Feb 2018 11:48:14 -0200 > Mauro Carvalho Chehab wrote: > >> his series fix two bugs at kernel-doc.rst examples and add support >> for in-line nested struct comments. >> >> It also converts one documentation at intel_dpio_phy to use it, >> in order to give a practical example about how to use it. > > OK, I've applied everything but the last patch, which I assume will go > through the DRM tree. I was going to reference the kernel-doc commit while applying patch 6, but I can't find the others. I guess applied literally meant just applied, not pushed... ;) BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: kdg2kfd_probe failed
HI Sen, Those messages are expected and you can ignore them. In kernel 4.14 a couple of warning/error messages were added to amdkfd. If you use the amdgpu driver you will not see them but because you use radeon you are seeing them. In kernel 4.15 we removed support of amdkfd from radeon, so once fedora will upgrade to 4.15 (or later) you will again stop seeing those messages. Oded On Mon, Feb 19, 2018 at 1:48 AM, Sen Dion wrote: > For some unknown reason, I started to get the "kgd2kfd_probe failed" message > after the update of Fedora-27 (limited to security advisories). Here is dmesg > output: > Console: switching to colour frame buffer device 240x67 > > radeon :01:05.0: fb0: radeondrmfb frame buffer device > kfd kfd: DID 9710 is missing in supported_devices > kfd kfd: kgd2kfd_probe failed > [drm] Initialized radeon 2.50.0 20080528 for :01:05.0 on minor 0 > kernel version: 4.14.16-300.fc27.x86_64 > > Compare with the dmesg output: > Console: switching to colour frame buffer device 240x67 > radeon :01:05.0: fb0: radeondrmfb frame buffer device > [drm] Initialized radeon 2.50.0 20080528 for :01:05.0 on minor 0 > from the kernel version 4.13.9-300.fc27.x86_64 > > > > Q1. Is this failure expected? Note that this system doesn't support IOMMU. > Q2. What to do if such failure is not expected? > >Thank you, >- Sen Dion > > > P.S. > > This is my first submission to the list. Kindly forgive me if I missed to > adhere to the list posting guildelines. > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105156] kdg2kfd_probe failed
https://bugs.freedesktop.org/show_bug.cgi?id=105156 Oded Gabbay changed: What|Removed |Added Status|NEW |ASSIGNED -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105156] kdg2kfd_probe failed
https://bugs.freedesktop.org/show_bug.cgi?id=105156 Oded Gabbay changed: What|Removed |Added Assignee|dri-devel@lists.freedesktop |oded.gab...@gmail.com |.org| -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[4.9-stable 2/9] drm/i915: hide unused intel_panel_set_backlight function
commit fd94d53e55bd487368dfee9f1af24da78b2bb582 upstream.
Building i915 without backlight support results in a harmless warning
for intel_panel_set_backlight:
drivers/gpu/drm/i915/intel_panel.c:653:13: error: 'intel_panel_set_backlight'
defined but not used [-Werror=unused-function]
This moves it into the CONFIG_BACKLIGHT_CLASS_DEVICE section that
its caller is in.
Signed-off-by: Arnd Bergmann
Signed-off-by: Daniel Vetter
Link:
https://patchwork.freedesktop.org/patch/msgid/20171127151239.1813673-2-a...@arndb.de
[arnd: manually rebased to 4.9]
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/i915/intel_panel.c | 86 +++---
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index be4b4d546fd9..1cb1b01e4471 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -544,25 +544,6 @@ static u32 pwm_get_backlight(struct intel_connector
*connector)
return DIV_ROUND_UP(duty_ns * 100, CRC_PMIC_PWM_PERIOD_NS);
}
-static u32 intel_panel_get_backlight(struct intel_connector *connector)
-{
- struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
- struct intel_panel *panel = &connector->panel;
- u32 val = 0;
-
- mutex_lock(&dev_priv->backlight_lock);
-
- if (panel->backlight.enabled) {
- val = panel->backlight.get(connector);
- val = intel_panel_compute_brightness(connector, val);
- }
-
- mutex_unlock(&dev_priv->backlight_lock);
-
- DRM_DEBUG_DRIVER("get backlight PWM = %d\n", val);
- return val;
-}
-
static void lpt_set_backlight(struct intel_connector *connector, u32 level)
{
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
@@ -646,30 +627,6 @@ intel_panel_actually_set_backlight(struct intel_connector
*connector, u32 level)
panel->backlight.set(connector, level);
}
-/* set backlight brightness to level in range [0..max], scaling wrt hw min */
-static void intel_panel_set_backlight(struct intel_connector *connector,
- u32 user_level, u32 user_max)
-{
- struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
- struct intel_panel *panel = &connector->panel;
- u32 hw_level;
-
- if (!panel->backlight.present)
- return;
-
- mutex_lock(&dev_priv->backlight_lock);
-
- WARN_ON(panel->backlight.max == 0);
-
- hw_level = scale_user_to_hw(connector, user_level, user_max);
- panel->backlight.level = hw_level;
-
- if (panel->backlight.enabled)
- intel_panel_actually_set_backlight(connector, hw_level);
-
- mutex_unlock(&dev_priv->backlight_lock);
-}
-
/* set backlight brightness to level in range [0..max], assuming hw min is
* respected.
*/
@@ -1122,6 +1079,49 @@ void intel_panel_enable_backlight(struct intel_connector
*connector)
}
#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+static u32 intel_panel_get_backlight(struct intel_connector *connector)
+{
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ struct intel_panel *panel = &connector->panel;
+ u32 val = 0;
+
+ mutex_lock(&dev_priv->backlight_lock);
+
+ if (panel->backlight.enabled) {
+ val = panel->backlight.get(connector);
+ val = intel_panel_compute_brightness(connector, val);
+ }
+
+ mutex_unlock(&dev_priv->backlight_lock);
+
+ DRM_DEBUG_DRIVER("get backlight PWM = %d\n", val);
+ return val;
+}
+
+/* set backlight brightness to level in range [0..max], scaling wrt hw min */
+static void intel_panel_set_backlight(struct intel_connector *connector,
+ u32 user_level, u32 user_max)
+{
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ struct intel_panel *panel = &connector->panel;
+ u32 hw_level;
+
+ if (!panel->backlight.present)
+ return;
+
+ mutex_lock(&dev_priv->backlight_lock);
+
+ WARN_ON(panel->backlight.max == 0);
+
+ hw_level = scale_user_to_hw(connector, user_level, user_max);
+ panel->backlight.level = hw_level;
+
+ if (panel->backlight.enabled)
+ intel_panel_actually_set_backlight(connector, hw_level);
+
+ mutex_unlock(&dev_priv->backlight_lock);
+}
+
static int intel_backlight_device_update_status(struct backlight_device *bd)
{
struct intel_connector *connector = bl_get_data(bd);
--
2.9.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: add documentation for tv connector state margins
A bit boring documentation fix, but gets rid of the warnings: ./include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.left' not described in 'drm_tv_connector_state' ./include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.right' not described in 'drm_tv_connector_state' ./include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.top' not described in 'drm_tv_connector_state' ./include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.bottom' not described in 'drm_tv_connector_state' Signed-off-by: Jani Nikula --- include/drm/drm_connector.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 758a176e7b57..675cc3f8cf85 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -342,7 +342,11 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info, /** * struct drm_tv_connector_state - TV connector related states * @subconnector: selected subconnector - * @margins: left/right/top/bottom margins + * @margins: margins + * @margins.left: left margin + * @margins.right: right margin + * @margins.top: top margin + * @margins.bottom: bottom margin * @mode: TV mode * @brightness: brightness in percent * @contrast: contrast in percent -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/edid: Add 6 bpc quirk for CPT panel in Asus UX303LA
On Sun, 18 Feb 2018, Kai-Heng Feng wrote:
> Similar to commit e10aec652f31 ("drm/edid: Add 6 bpc quirk for display
> AEO model 0."), the EDID reports "DFP 1.x compliant TMDS" but it support
> 6bpc instead of 8 bpc.
>
> Hence, use 6 bpc quirk for this panel.
>
> Fixes: 196f954e2509 ("drm/i915/dp: Revert "drm/i915/dp: fall back to 18 bpp
> when sink capability is unknown"")
Implies
Cc: # v4.8+
> BugLink: https://bugs.launchpad.net/bugs/1749420
> Signed-off-by: Kai-Heng Feng
> ---
> drivers/gpu/drm/drm_edid.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index ddd537914575..d9c8d718e261 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -113,6 +113,9 @@ static const struct edid_quirk {
> /* AEO model 0 reports 8 bpc, but is a 6 bpc panel */
> { "AEO", 0, EDID_QUIRK_FORCE_6BPC },
>
> + /* CPT panel of Asus UX303LA reports 8 bpc, but is a 6 bpc panel */
> + { "CPT", 0x17df, EDID_QUIRK_FORCE_6BPC },
> +
> /* Belinea 10 15 55 */
> { "MAX", 1516, EDID_QUIRK_PREFER_LARGE_60 },
> { "MAX", 0x77e, EDID_QUIRK_PREFER_LARGE_60 },
--
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v3 1/6] drm: fourcc.h: Use inline kern-doc style for struct drm_format_info
On Fri, Feb 09, 2018 at 05:35:51PM -0800, Hyun Kwon wrote:
> Use the inline kern-doc style for struct drm_format_info for better
> readability. This is just a preliminary change for further table update.
>
> Signed-off-by: Hyun Kwon
> ---
> v3
> - This is added
> ---
> ---
> include/drm/drm_fourcc.h | 45 +
> 1 file changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 6942e84..b00bae4 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -30,21 +30,50 @@ struct drm_mode_fb_cmd2;
>
> /**
> * struct drm_format_info - information about a DRM format
> - * @format: 4CC format identifier (DRM_FORMAT_*)
> - * @depth: Color depth (number of bits per pixel excluding padding bits),
> - * valid for a subset of RGB formats only. This is a legacy field, do not
> - * use in new code and set to 0 for new formats.
> - * @num_planes: Number of color planes (1 to 3)
> - * @cpp: Number of bytes per pixel (per plane)
> - * @hsub: Horizontal chroma subsampling factor
> - * @vsub: Vertical chroma subsampling factor
> */
> struct drm_format_info {
> + /**
> + * @format:
> + *
> + * 4CC format identifier (DRM_FORMAT_*)
> + */
> u32 format;
> +
> + /**
> + * @depth:
> + *
> + * Color depth (number of bits per pixel excluding padding bits),
> + * valid for a subset of RGB formats only. This is a legacy field,
> + * do not use in new code and set to 0 for new formats.
> + */
> u8 depth;
> +
> + /**
> + * @num_planes:
> + *
> + * Number of color planes (1 to 3)
> + */
> u8 num_planes;
> +
> + /**
> + * @cpp:
> + *
> + * Number of bytes per pixel (per plane)
> + */
> u8 cpp[3];
> +
> + /**
> + * @hsub:
> + *
> + * Horizontal chroma subsampling factor
Since we now have more space, I think it'd be good to clarify here that
this is only valid for YUV formats by adding:
"This is only valid for YUV format."
With that:
Reviewed-by: Daniel Vetter
> + */
> u8 hsub;
> +
> + /**
> + * @vsub:
> + *
> + * Vertical chroma subsampling factor
> + */
> u8 vsub;
> };
>
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v3 2/6] drm: drm_fourcc: Introduce macro-pixel info to drm_format_info
On Fri, Feb 09, 2018 at 05:35:52PM -0800, Hyun Kwon wrote:
> Multiple pixels can be grouped as a single unit and form a 'macro-pixel'.
> This is to model formats where multiple non-byte aligned pixels are stored
> together in a byte-aligned way. For example, if 3 - 10 bit
> pixels are stored in 32 bit, the 32 bit stroage can be treated as
> a single macro-pixel with 3 pixels. This aligns non-byte addressable
> formats with drm core where each pixel / component is expected to be
> byte aligned.
>
> Add 'pixels_per_macro' to note how many pixels are in a macro-pixel.
> 'bytes_per_macro' specifies the size of a macro-pixel in bytes.
>
> Signed-off-by: Hyun Kwon
> ---
> v3
> - Rename members and rephrase descriptions
> - Rephrase the commit message
> - Use in-line style comments
> v2
> - Introduce macro-pixel over scaling factors
> ---
> ---
> include/drm/drm_fourcc.h | 24 +++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index b00bae4..ce59329 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -58,11 +58,33 @@ struct drm_format_info {
> /**
>* @cpp:
>*
> - * Number of bytes per pixel (per plane)
> + * Number of bytes per pixel (per plane). @cpp shouldn't be used when
> + * @pixels_per_macropixel and @bytes_per_macropixel are used.
>*/
> u8 cpp[3];
>
> /**
> + * @pixels_per_macropixel:
> + *
> + * Number of pixels per macro-pixel (per plane). A macro-pixel is
> + * composed of multiple pixels, and there can be extra bits between
> + * pixels. This must be used along with @bytes_per_macropixel, only
> + * when single pixel size is not byte-aligned. In this case, @cpp
> + * is not valid and should be 0.
> + */
> + u8 pixels_per_macropixel[3];
> +
> + /*
> + * @bytes_per_macropixel:
> + *
> + * Number of bytes per macro-pixel (per plane). A macro-pixel is
> + * composed of multiple pixels. The size of single macro-pixel should
> + * be byte-aligned. This should be used with @pixels_per_macropixel,
> + * and @cpp should be 0.
> + */
> + u8 bytes_per_macropixel[3];
Reviewed-by: Daniel Vetter
> +
> + /**
>* @hsub:
>*
>* Horizontal chroma subsampling factor
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v3 3/6] drm: fourcc: Add drm_format_plane_width_bytes()
On Fri, Feb 09, 2018 at 05:35:53PM -0800, Hyun Kwon wrote:
> drm_format_plane_width_bytes() calculates and returns the number of bytes
> for given width of specified format. The calculation uses @cpp
> in drm format info for byte-aligned formats. If the format isn't
> byte-aligned, @cpp should 0, and the macro pixel information is used.
> This avoids bit level rounding.
>
> Use this drm_fb_cma_get_gem_addr() for offset calculation.
>
> Signed-off-by: Hyun Kwon
We also need to adjust the validation in framebuffer_check, that's using
raw cpp. I think that's all there is for the core code, but please also
grep for cpp anywhere in core code and make sure we have them all. You
probably want to use the newly introduced helper function in all these
core places (like you do for cma helpers already).
> ---
> v3
> - Update according to member changes
> - Use @cpp for byte-aligned formats, and macro-pixel for non byte-aligned ones
> - Squash a change in drm_fb_cma_helper.c into this
> v2
> - This function is added
> ---
> ---
> drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++-
> drivers/gpu/drm/drm_fourcc.c| 35 +++
> include/drm/drm_fourcc.h| 2 ++
> 3 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 186d00a..271175e 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -124,7 +124,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> *fb,
> return 0;
>
> paddr = obj->paddr + fb->offsets[plane];
> - paddr += fb->format->cpp[plane] * (state->src_x >> 16);
> + paddr += drm_format_plane_width_bytes(fb->format, plane,
> + state->src_x >> 16);
> paddr += fb->pitches[plane] * (state->src_y >> 16);
>
> return paddr;
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 9c0152d..ed95de2 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -348,3 +348,38 @@ int drm_format_plane_height(int height, uint32_t format,
> int plane)
> return height / info->vsub;
> }
> EXPORT_SYMBOL(drm_format_plane_height);
> +
> +/**
> + * drm_format_plane_width_bytes - bytes of the given width of the plane
> + * @info: DRM format information
> + * @plane: plane index
> + * @width: width to get the number of bytes
> + *
> + * This returns the number of bytes for given @width and @plane.
> + * The @cpp or macro pixel information should be valid.
> + *
> + * Returns:
> + * The bytes of @width of @plane. 0 for invalid format info.
> + */
> +int drm_format_plane_width_bytes(const struct drm_format_info *info,
> + int plane, int width)
We need to extend the kernel code for drm_format_plane_cpp to explain that
it can't handle formats with macropixels (for obvious reasons) and that
generic code must use drm_format_plane_width_bytes instead of cpp. Drivers
can keep using drm_format_plane_cpp as long as they don't support a 4CC
code with macro pixels.
> +{
> + if (!info || plane >= info->num_planes)
> + return 0;
> +
> + if (info->cpp[plane])
I think a WARN_ON(bytes_per_macropixel || pixels_per_macropixel); here
would be good, to make sure only one or the other is set (to force
consistency and avoid silly bugs).
With the above details addressed I think this patch looks good. I'll
triple-check the new version for any missing cpp conversions in the drm
core once more though.
-Daniel
> + return info->cpp[plane] * width;
> +
> + if (WARN_ON(!info->bytes_per_macropixel[plane] ||
> + !info->pixels_per_macropixel[plane])) {
> + struct drm_format_name_buf buf;
> +
> + DRM_WARN("Either cpp or macro-pixel info should be valid: %s\n",
> + drm_get_format_name(info->format, &buf));
> + return 0;
> + }
> +
> + return DIV_ROUND_UP(width * info->bytes_per_macropixel[plane],
> + info->pixels_per_macropixel[plane]);
> +}
> +EXPORT_SYMBOL(drm_format_plane_width_bytes);
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index ce59329..8158290 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -119,6 +119,8 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
> int drm_format_vert_chroma_subsampling(uint32_t format);
> int drm_format_plane_width(int width, uint32_t format, int plane);
> int drm_format_plane_height(int height, uint32_t format, int plane);
> +int drm_format_plane_width_bytes(const struct drm_format_info *info,
> + int plane, int width);
> const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf
> *buf);
>
> #endif /* __DRM_FOURCC_H__ */
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.
Re: [Nouveau] [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers
On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > Fix a deadlock on hybrid graphics laptops that's been present since 2013: > > DRM drivers poll connectors in 10 sec intervals. The poll worker is > stopped on ->runtime_suspend with cancel_delayed_work_sync(). However > the poll worker invokes the DRM drivers' ->detect callbacks, which call > pm_runtime_get_sync(). If the poll worker starts after runtime suspend > has begun, pm_runtime_get_sync() will wait for runtime suspend to finish > with the intention of runtime resuming the device afterwards. The result > is a circular wait between poll worker and autosuspend worker. Don't shut down the poll worker when runtime suspending, that' doesn't work. If you need the poll work, then that means waking up the gpu every few seconds. If you don't need it, then make sure the DRM_CONNECTOR_POLL flags are set correctly (you can update them at runtime, the poll worker will pick that up). That should fix the deadlock, and it's how we do it in i915 (where igt in CI totally hammers the runtime pm support, and it seems to hold up). And I guess we need to improve the poll worker docs about this. > I'm seeing this deadlock so often it's not funny anymore. I've also found > 3 nouveau bugzillas about it and 1 radeon bugzilla. (See patch [3/5] and > [4/5].) > > One theoretical solution would be to add a flag to the ->detect callback > to tell it that it's running in the poll worker's context. In that case > it doesn't need to call pm_runtime_get_sync() because the poll worker is > only enabled while runtime active and we know that ->runtime_suspend > waits for it to finish. But this approach would require touching every > single DRM driver's ->detect hook. Moreover the ->detect hook is called > from numerous other places, both in the DRM library as well as in each > driver, making it necessary to trace every possible call chain and check > if it's coming from the poll worker or not. I've tried to do that for > nouveau (see the call sites listed in the commit message of patch [3/5]) > and concluded that this approach is a nightmare to implement. > > Another reason for the infeasibility of this approach is that ->detect > is called from within the DRM library without driver involvement, e.g. > via DRM's sysfs interface. In those cases, pm_runtime_get_sync() would > have to be called by the DRM library, but the DRM library is supposed to > stay generic, wheras runtime PM is driver-specific. > > So instead, I've come up with this much simpler solution which gleans > from the current task's flags if it's a workqueue worker, retrieves the > work_struct and checks if it's the poll worker. All that's needed is > one small helper in the workqueue code and another small helper in the > DRM library. This solution lends itself nicely to stable-backporting. > > The patches for radeon and amdgpu are compile-tested only, I only have a > MacBook Pro with an Nvidia GK107 to test. To test the patches, add an > "msleep(12*1000);" at the top of the driver's ->runtime_suspend hook. > This ensures that the poll worker runs after ->runtime_suspend has begun. > Wait 12 sec after the GPU has begun runtime suspend, then check > /sys/bus/pci/devices/:01:00.0/power/runtime_status. Without this > series, the status will be stuck at "suspending" and you'll get hung task > errors in dmesg after a few minutes. > > i915, malidp and msm "solved" this issue by not stopping the poll worker > on runtime suspend. But this results in the GPU bouncing back and forth > between D0 and D3 continuously. That's a total no-go for GPUs which > runtime suspend to D3cold since every suspend/resume cycle costs a > significant amount of time and energy. (i915 and malidp do not seem > to acquire a runtime PM ref in the ->detect callbacks, which seems > questionable. msm however does and would also deadlock if it disabled > the poll worker on runtime suspend. cc += Archit, Liviu, intel-gfx) Well, userspace expects hotplug events, even when we runtime suspend stuff. Hence waking shit up with polling. Imo ignoring hotplug events is a pretty serious policy decision which is ok in the context of vga_switcheroo, but not really as an automatic thing. E.g. usb also wakes up if you plug something in, even with all the runtime pm stuff enabled. Same for sata and everything else. -Daniel > Please review. Thanks, > > Lukas > > Lukas Wunner (5): > workqueue: Allow retrieval of current task's work struct > drm: Allow determining if current task is output poll worker > drm/nouveau: Fix deadlock on runtime suspend > drm/radeon: Fix deadlock on runtime suspend > drm/amdgpu: Fix deadlock on runtime suspend > > drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 58 +--- > drivers/gpu/drm/drm_probe_helper.c | 14 + > drivers/gpu/drm/nouveau/nouveau_connector.c| 18 +-- > drivers/gpu/drm/radeon/radeon_connectors.c | 74 > +--
[PATCH] drm: Handle unexpected holes in color-eviction
During eviction, the driver may free more than one hole in the drm_mm
due to the side-effects in evicting the scanned nodes. However,
drm_mm_scan_color_evict() expects that the scan result is the first
available hole (in the mru freed hole_stack list):
kernel BUG at drivers/gpu/drm/drm_mm.c:844!
invalid opcode: [#1] PREEMPT SMP KASAN PTI
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in: i915 snd_hda_codec_analog snd_hda_codec_generic coretemp
snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core lpc_ich snd_pcm e1000e
mei_me prime_numbers mei
CPU: 1 PID: 1490 Comm: gem_userptr_bli Tainted: G U
4.16.0-rc1-g740f57c54ecf-kasan_6+ #1
Hardware name: Dell Inc. OptiPlex 755 /0PU052, BIOS A08
02/19/2008
RIP: 0010:drm_mm_scan_color_evict+0x2b8/0x3d0
RSP: 0018:880057a573f8 EFLAGS: 00010287
RAX: 8800611f5980 RBX: 880057a575d0 RCX: dc00
RDX: 029d5000 RSI: 11000af4aec1 RDI: 8800611f5a10
RBP: 88005ab884d0 R08: 880057a57600 R09: 0afff000
R10: 11000b5710b5 R11: 1000 R12: 11000af4ae82
R13: 8800611f59b0 R14: 8800611f5980 R15: 880057a57608
FS: 7f2de0c2e8c0() GS:88006ac4() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 7f2ddde1e000 CR3: 609b2000 CR4: 06e0
Call Trace:
? drm_mm_scan_remove_block+0x330/0x330
? drm_mm_scan_remove_block+0x151/0x330
i915_gem_evict_something+0x711/0xbd0 [i915]
? igt_evict_contexts+0x50/0x50 [i915]
? nop_clear_range+0x10/0x10 [i915]
? igt_evict_something+0x90/0x90 [i915]
? i915_gem_gtt_reserve+0x1a1/0x320 [i915]
i915_gem_gtt_insert+0x237/0x400 [i915]
__i915_vma_do_pin+0xc25/0x1a20 [i915]
eb_lookup_vmas+0x1c63/0x3790 [i915]
? i915_gem_check_execbuffer+0x250/0x250 [i915]
? trace_hardirqs_on_caller+0x33f/0x590
? _raw_spin_unlock_irqrestore+0x39/0x60
? __pm_runtime_resume+0x7d/0xf0
i915_gem_do_execbuffer+0x86a/0x2ff0 [i915]
? __kmalloc+0x132/0x340
? i915_gem_execbuffer2_ioctl+0x10f/0x760 [i915]
? drm_ioctl_kernel+0x12e/0x1c0
? drm_ioctl+0x662/0x980
? eb_relocate_slow+0xa90/0xa90 [i915]
? i915_gem_execbuffer2_ioctl+0x10f/0x760 [i915]
? __might_fault+0xea/0x1a0
i915_gem_execbuffer2_ioctl+0x3cc/0x760 [i915]
? i915_gem_execbuffer_ioctl+0xba0/0xba0 [i915]
? lock_acquire+0x3c0/0x3c0
? i915_gem_execbuffer_ioctl+0xba0/0xba0 [i915]
drm_ioctl_kernel+0x12e/0x1c0
drm_ioctl+0x662/0x980
? i915_gem_execbuffer_ioctl+0xba0/0xba0 [i915]
? drm_getstats+0x20/0x20
? debug_check_no_obj_freed+0x2a6/0x8c0
do_vfs_ioctl+0x170/0xe70
? ioctl_preallocate+0x170/0x170
? task_work_run+0xbe/0x160
? lock_acquire+0x3c0/0x3c0
? trace_hardirqs_on_caller+0x33f/0x590
? _raw_spin_unlock_irq+0x2f/0x50
SyS_ioctl+0x36/0x70
? do_vfs_ioctl+0xe70/0xe70
do_syscall_64+0x18c/0x5d0
entry_SYSCALL_64_after_hwframe+0x26/0x9b
RIP: 0033:0x7f2ddf13b587
RSP: 002b:7fff15c4f9d8 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 0003 RCX: 7f2ddf13b587
RDX: 7fff15c4fa20 RSI: 40406469 RDI: 0003
RBP: 7fff15c4fa20 R08: R09: 7f2ddf3fe120
R10: 0073 R11: 0246 R12: 40406469
R13: 0003 R14: 7fff15c4fa20 R15: 00c7
Code: 00 00 00 4a c7 44 22 08 00 00 00 00 42 c7 44 22 10 00 00 00 00 48 81 c4
b8 00 00 00 5b 5d 41 5c 41 5d 41 5e 41 5f c3 0f 0b 0f 0b <0f> 0b 31 c0 eb c0 4c
89 ef e8 9a 09 41 ff e9 1e fe ff ff 4c 89
RIP: drm_mm_scan_color_evict+0x2b8/0x3d0 RSP: 880057a573f8
We can trivially relax this assumption, by searching the hole_stack for
the scan result and warn instead if the driver called us without any
result.
Fixes: 3fa489dabea9 ("drm: Apply tight eviction scanning to color_adjust")
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
Cc: # v4.11+
---
drivers/gpu/drm/drm_mm.c | 21 ++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 186c4e90cc1c..89eef1bb4ddc 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -836,9 +836,24 @@ struct drm_mm_node *drm_mm_scan_color_evict(struct
drm_mm_scan *scan)
if (!mm->color_adjust)
return NULL;
- hole = list_first_entry(&mm->hole_stack, typeof(*hole), hole_stack);
- hole_start = __drm_mm_hole_node_start(hole);
- hole_end = hole_start + hole->hole_size;
+ /*
+* The hole found during scanning should ideally be the first element
+* in the hole_stack list, but due to side-effects in the driver it
+* may not be.
+*/
+ list_for_each_entry(hole, &mm->hole_stack, hole_stack) {
+ hole_start = __drm_mm_hole_node_start(hole);
+ hole_end = hole_start + hole->hole_size;
+
+
Re: [PATCH v2 4/8] drm/rockchip: dw_hdmi: Allow outputs that don't need output switching
Hi Heiko,
On 16/02/18 20:41, Heiko Stuebner wrote:
So far we always encountered socs with 2 output crtcs needing the driver
to tell the hdmi block which output to connect to. But there also exist
socs with only one crtc like the rk3228, rk3328 and rk3368.
So adapt the register field to simply carry a negative value to signal
that now output-switching is necessary.
Nit: s/now/no/ ? (Hooray for grammatically-correct typos which invert
the entire meaning of the sentence!)
What are the necessary dependencies for these patches? I tried applying
the series on 4.16-rc1 to test, but it failed at patch #6 with some
missing context (I tried checking drm-next for where rockchip_hdmi::hdmi
might have come from, but didn't see anything obvious)
Thanks,
Robin.
Signed-off-by: Heiko Stuebner
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 3574b0ae2ad1..6dc03f1a5709 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -36,7 +36,7 @@
* @lcdsel_lit: reg value of selecting vop little for HDMI
*/
struct rockchip_hdmi_chip_data {
- u32 lcdsel_grf_reg;
+ int lcdsel_grf_reg;
u32 lcdsel_big;
u32 lcdsel_lit;
};
@@ -253,6 +253,9 @@ static void dw_hdmi_rockchip_encoder_enable(struct
drm_encoder *encoder)
u32 val;
int ret;
+ if (hdmi->chip_data->lcdsel_grf_reg < 0)
+ return;
+
ret = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
if (ret)
val = hdmi->chip_data->lcdsel_lit;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/8] drm/rockchip: dw_hdmi: store rockchip_hdmi reference in phy_data object
On 16/02/18 20:41, Heiko Stuebner wrote:
When using special phy handling operations we'll often need access to
the rockchip_hdmi struct. As the chip-data that occupies the phy_data
pointer initially gets assigned to the rockchip_hdmi struct we can not
s/not/now/ (or s/not//) ? Again, this seems to imply the opposite of
what the patch actually does ;)
Robin.
re-use this phy_data pointer to hold the reference to the rockchip_hdmi
struct, similar to how meson-hdmi does it for example.
Signed-off-by: Heiko Stuebner
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 305b68d223d7..8595638d4990 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -343,7 +343,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct
device *master,
void *data)
{
struct platform_device *pdev = to_platform_device(dev);
- const struct dw_hdmi_plat_data *plat_data;
+ struct dw_hdmi_plat_data *plat_data;
const struct of_device_id *match;
struct drm_device *drm = data;
struct drm_encoder *encoder;
@@ -358,9 +358,14 @@ static int dw_hdmi_rockchip_bind(struct device *dev,
struct device *master,
return -ENOMEM;
match = of_match_node(dw_hdmi_rockchip_dt_ids, pdev->dev.of_node);
- plat_data = match->data;
+ plat_data = devm_kmemdup(&pdev->dev, match->data,
+sizeof(*plat_data), GFP_KERNEL);
+ if (!plat_data)
+ return -ENOMEM;
+
hdmi->dev = &pdev->dev;
hdmi->chip_data = plat_data->phy_data;
+ plat_data->phy_data = hdmi;
encoder = &hdmi->encoder;
encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/tinydrm: Remove chunk splitting in tinydrm_spi_transfer
-Remove chunk splitting in tinydrm_spi_transfer in tinydrm-helpers as The spi
core will split a buffer into max_dma_len chunks for the
spi controller driver to handle.
-Remove automatic byte swapping in tinydrm_spi_transfer as it doesn't have
users.
-Remove the upper bound check on dma transfer length in bcm2835_spi_can_dma().
Signed-off-by: Meghana Madhyastha
---
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 48 --
drivers/gpu/drm/tinydrm/mipi-dbi.c | 10 ++
drivers/spi/spi-bcm2835.c | 15 +---
3 files changed, 9 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bf96072d1b97..6064099e8e63 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -452,62 +452,26 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32
speed_hz,
struct spi_transfer tr = {
.bits_per_word = bpw,
.speed_hz = speed_hz,
+ .tx_buf = buf,
+ .len = len
};
struct spi_message m;
- u16 *swap_buf = NULL;
size_t max_chunk;
- size_t chunk;
- int ret = 0;
- if (WARN_ON_ONCE(bpw != 8 && bpw != 16))
- return -EINVAL;
-
- max_chunk = tinydrm_spi_max_transfer_size(spi, 0);
+ max_chunk = SIZE_MAX;
if (drm_debug & DRM_UT_DRIVER)
pr_debug("[drm:%s] bpw=%u, max_chunk=%zu, transfers:\n",
-__func__, bpw, max_chunk);
-
- if (bpw == 16 && !tinydrm_spi_bpw_supported(spi, 16)) {
- tr.bits_per_word = 8;
- if (tinydrm_machine_little_endian()) {
- swap_buf = kmalloc(min(len, max_chunk), GFP_KERNEL);
- if (!swap_buf)
- return -ENOMEM;
- }
- }
+ __func__, bpw, max_chunk);
spi_message_init(&m);
if (header)
spi_message_add_tail(header, &m);
spi_message_add_tail(&tr, &m);
- while (len) {
- chunk = min(len, max_chunk);
-
- tr.tx_buf = buf;
- tr.len = chunk;
-
- if (swap_buf) {
- const u16 *buf16 = buf;
- unsigned int i;
-
- for (i = 0; i < chunk / 2; i++)
- swap_buf[i] = swab16(buf16[i]);
-
- tr.tx_buf = swap_buf;
- }
-
- buf += chunk;
- len -= chunk;
-
- tinydrm_dbg_spi_message(spi, &m);
- ret = spi_sync(spi, &m);
- if (ret)
- return ret;
- }
+ tinydrm_dbg_spi_message(spi, &m);
- return 0;
+ return spi_sync(spi, &m);
}
EXPORT_SYMBOL(tinydrm_spi_transfer);
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 75dd65c57e74..c8af2d65c2ad 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -886,15 +886,9 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi,
u8 cmd,
int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
struct gpio_desc *dc)
{
- size_t tx_size = tinydrm_spi_max_transfer_size(spi, 0);
struct device *dev = &spi->dev;
int ret;
- if (tx_size < 16) {
- DRM_ERROR("SPI transmit buffer too small: %zu\n", tx_size);
- return -EINVAL;
- }
-
/*
* Even though it's not the SPI device that does DMA (the master does),
* the dma mask is necessary for the dma_alloc_wc() in
@@ -924,8 +918,8 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct
mipi_dbi *mipi,
mipi->swap_bytes = true;
} else {
mipi->command = mipi_dbi_typec1_command;
- mipi->tx_buf9_len = tx_size;
- mipi->tx_buf9 = devm_kmalloc(dev, tx_size, GFP_KERNEL);
+ mipi->tx_buf9_len = SZ_16K;
+ mipi->tx_buf9 = devm_kmalloc(dev, mipi->tx_buf9_len,
GFP_KERNEL);
if (!mipi->tx_buf9)
return -ENOMEM;
}
diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index f35cc10772f6..2fd650891c07 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -365,19 +365,6 @@ static bool bcm2835_spi_can_dma(struct spi_master *master,
if (tfr->len < BCM2835_SPI_DMA_MIN_LENGTH)
return false;
- /* BCM2835_SPI_DLEN has defined a max transfer size as
-* 16 bit, so max is 65535
-* we can revisit this by using an alternative transfer
-* method - ideally this would get done without any more
-* interaction...
-*/
- if (tfr->len > 65535) {
-
Re: [Intel-gfx] [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers
On Thu, Feb 15, 2018 at 06:38:44AM +0100, Lukas Wunner wrote: > On Wed, Feb 14, 2018 at 09:58:43AM -0500, Sean Paul wrote: > > On Wed, Feb 14, 2018 at 03:43:56PM +0100, Michel Dänzer wrote: > > > On 2018-02-14 03:08 PM, Sean Paul wrote: > > > > On Wed, Feb 14, 2018 at 10:26:35AM +0100, Maarten Lankhorst wrote: > > > >> Op 14-02-18 om 09:46 schreef Lukas Wunner: > > > >>> On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > > > Fix a deadlock on hybrid graphics laptops that's been present since > > > 2013: > > > >>> This series has been reviewed, consent has been expressed by the most > > > >>> interested parties, patch [1/5] which touches files outside > > > >>> drivers/gpu > > > >>> has been acked and I've just out a v2 addressing the only objection > > > >>> raised. My plan is thus to wait another two days for comments and, > > > >>> barring further objections, push to drm-misc this weekend. > > > >>> > > > >>> However I'm struggling with the decision whether to push to next or > > > >>> fixes. The series is marked for stable, however the number of > > > >>> affected machines is limited and for an issue that's been present > > > >>> for 5 years it probably doesn't matter if it soaks another two months > > > >>> in linux-next befor it gets backported. Hence I tend to err on the > > > >>> side of caution and push to next, however a case could be made that > > > >>> fixes is more appropriate. > > > >>> > > > >>> I'm lacking experience making such decisions and would be interested > > > >>> to learn how you'd handle this. > > > >> > > > >> I would say fixes, it doesn't look particularly scary. :) > > > > > > > > Agreed. If it's good enough for stable, it's good enough for -fixes! > > > > > > It's not that simple, is it? Fast-tracking patches (some of which appear > > > to be untested) to stable without an immediate cause for urgency seems > > > risky to me. > > > > /me should be more careful what he says > > > > Given where we are in the release cycle, it's barely a fast track. > > If these go in -fixes, they'll get in -rc2 and will have plenty of > > time to bake. If we were at rc5, it might be a different story. > > The patches are marked for stable though, so if they go in through > drm-misc-fixes, they may appear in stable kernels before 4.16-final > is out. Greg picks up patches once they're in Linus' tree, though > often with a delay of a few days or weeks. If they go in through > drm-misc-next, they're guaranteed not to appear in *any* release > before 4.16-final is out. > > This allows for differentiation between no-brainer stable fixes that > can be sent immediately and scarier, but similarly important stable > fixes that should soak for a while. I'm not sure which category > this series belongs to, though it's true what Maarten says, it's > not *that* grave a change. If you're this concerned about them, then pls do _not_ put cc: stable on the patches. Instead get them merged through -fixes (or maybe even -next), and once they're sufficiently tested, send a mail to stable@ asking for ane explicit backport. Stuff that's marked for stable must be obviuos and tested enough for backporting right away (which doesn't seem to be the case here). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/8] drm/rockchip: dw_hdmi: Allow outputs that don't need output switching
Hi Robin, Am Montag, 19. Februar 2018, 12:43:11 CET schrieb Robin Murphy: > Hi Heiko, > > On 16/02/18 20:41, Heiko Stuebner wrote: > > So far we always encountered socs with 2 output crtcs needing the driver > > to tell the hdmi block which output to connect to. But there also exist > > socs with only one crtc like the rk3228, rk3328 and rk3368. > > > > So adapt the register field to simply carry a negative value to signal > > that now output-switching is necessary. > > Nit: s/now/no/ ? (Hooray for grammatically-correct typos which invert > the entire meaning of the sentence!) :-D > What are the necessary dependencies for these patches? I tried applying > the series on 4.16-rc1 to test, but it failed at patch #6 with some > missing context (I tried checking drm-next for where rockchip_hdmi::hdmi > might have come from, but didn't see anything obvious) You'll need the most recent drm-misc changes [0], especially the dw-hdmi changes from Jernej Skrabec that touch similar areas and even included a similar patch to export some general phy functions, which I could drop from my series. Heiko [0] https://cgit.freedesktop.org/drm/drm-misc/log/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] drm: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On Mon, Feb 12, 2018 at 02:51:41PM -0500, Joe Moriarty wrote:
> The Parfait (version 2.1.0) static code analysis tool found the
> following NULL pointer dereference problem.
>
> - drivers/gpu/drm/drm_drv.c
> Any calls to drm_minor_get_slot() could result in the return of a NULL
> pointer when an invalid DRM device type is encountered. 2 helper
> functions where added for pointer manipulation (drm_minor_get_slot()
> and drm_minor_set_minor()) along with checks for valid pointers for
> struct drm_device variables throughout this module.
>
> Signed-off-by: Joe Moriarty
> Reviewed-by: Steven Sistare
We do not ask for an invalid minor (userspace can't do that, it would be a
kernel bug). BUG_ON for the invalid case instead of all these changes
acceptable to shut up your checker?
-Daniel
> ---
> drivers/gpu/drm/drm_drv.c | 38 ++
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 9acc1e157813..dee6a4470e2c 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -99,10 +99,36 @@ static struct drm_minor **drm_minor_get_slot(struct
> drm_device *dev,
> case DRM_MINOR_CONTROL:
> return &dev->control;
> default:
> + DRM_ERROR("Error in %s: Invalid dev, type = %d\n",
> + __func__, type);
> return NULL;
> }
> }
>
> +static inline int drm_minor_set_minor(struct drm_device *dev,
> + unsigned int type,
> + struct drm_minor *minor)
> +{
> + struct drm_minor **slot = drm_minor_get_slot(dev, type);
> + int retval = -ENODEV;
> +
> + if (slot) {
> + retval = 0;
> + *slot = minor;
> + }
> + return retval;
> +}
> +
> +static inline struct drm_minor *drm_minor_get_minor(struct drm_device *dev,
> + unsigned int type)
> +{
> + struct drm_minor **slot = drm_minor_get_slot(dev, type);
> +
> + if (slot)
> + return *slot;
> + return NULL;
> +}
> +
> static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> {
> struct drm_minor *minor;
> @@ -137,8 +163,9 @@ static int drm_minor_alloc(struct drm_device *dev,
> unsigned int type)
> goto err_index;
> }
>
> - *drm_minor_get_slot(dev, type) = minor;
> - return 0;
> + r = drm_minor_set_minor(dev, type, minor);
> + if (r == 0)
> + return r;
>
> err_index:
> spin_lock_irqsave(&drm_minor_lock, flags);
> @@ -155,6 +182,9 @@ static void drm_minor_free(struct drm_device *dev,
> unsigned int type)
> unsigned long flags;
>
> slot = drm_minor_get_slot(dev, type);
> + if (!slot)
> + return;
> +
> minor = *slot;
> if (!minor)
> return;
> @@ -177,7 +207,7 @@ static int drm_minor_register(struct drm_device *dev,
> unsigned int type)
>
> DRM_DEBUG("\n");
>
> - minor = *drm_minor_get_slot(dev, type);
> + minor = drm_minor_get_minor(dev, type);
> if (!minor)
> return 0;
>
> @@ -209,7 +239,7 @@ static void drm_minor_unregister(struct drm_device *dev,
> unsigned int type)
> struct drm_minor *minor;
> unsigned long flags;
>
> - minor = *drm_minor_get_slot(dev, type);
> + minor = drm_minor_get_minor(dev, type);
> if (!minor || !device_is_registered(minor->kdev))
> return;
>
> --
> 2.15.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers
On Mon, Feb 19, 2018 at 12:34:43PM +0100, Daniel Vetter wrote: > On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > > Fix a deadlock on hybrid graphics laptops that's been present since 2013: > > > > DRM drivers poll connectors in 10 sec intervals. The poll worker is > > stopped on ->runtime_suspend with cancel_delayed_work_sync(). However > > the poll worker invokes the DRM drivers' ->detect callbacks, which call > > pm_runtime_get_sync(). If the poll worker starts after runtime suspend > > has begun, pm_runtime_get_sync() will wait for runtime suspend to finish > > with the intention of runtime resuming the device afterwards. The result > > is a circular wait between poll worker and autosuspend worker. > > Don't shut down the poll worker when runtime suspending, that' doesn't > work. If you need the poll work, then that means waking up the gpu every > few seconds. If you don't need it, then make sure the DRM_CONNECTOR_POLL > flags are set correctly (you can update them at runtime, the poll worker > will pick that up). > > That should fix the deadlock, and it's how we do it in i915 (where igt in > CI totally hammers the runtime pm support, and it seems to hold up). It would fix the deadlock but it's not an option on dual GPU laptops. Power consumption of the discrete GPU is massive (9 W on my machine). > > i915, malidp and msm "solved" this issue by not stopping the poll worker > > on runtime suspend. But this results in the GPU bouncing back and forth > > between D0 and D3 continuously. That's a total no-go for GPUs which > > runtime suspend to D3cold since every suspend/resume cycle costs a > > significant amount of time and energy. (i915 and malidp do not seem > > to acquire a runtime PM ref in the ->detect callbacks, which seems > > questionable. msm however does and would also deadlock if it disabled > > the poll worker on runtime suspend. cc += Archit, Liviu, intel-gfx) > > Well, userspace expects hotplug events, even when we runtime suspend > stuff. Hence waking shit up with polling. Imo ignoring hotplug events is a > pretty serious policy decision which is ok in the context of > vga_switcheroo, but not really as an automatic thing. E.g. usb also wakes > up if you plug something in, even with all the runtime pm stuff enabled. > Same for sata and everything else. On the MacBook Pro, the HPD pin of external DP and HDMI ports goes into the gmux controller, which sends an interrupt on hotplug even if the GPU is powered down. Apparently Optimus has a similar functionality, cf. 3a6536c51d5d. For the rare cases where an external VGA or DVI-A port is present, I guess it's reasonable to have the user wake up the machine manually. I'm not sure why nouveau polls ports on my laptop, the GK107 only has an LVDS and three DP ports, need to investigate. Thanks, Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/4] drm: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On Mon, Feb 12, 2018 at 02:51:42PM -0500, Joe Moriarty wrote:
> The Parfait (version 2.1.0) static code analysis tool found the
> following NULL pointer derefernce problem.
>
> - drivers/gpu/drm/drm_dp_mst_topology.c
> The call to drm_dp_calculate_rad() in function drm_dp_port_setup_pdt()
> could result in a NULL pointer being returned to port->mstb due to a
> failure to allocate memory for port->mstb.
>
> Signed-off-by: Joe Moriarty
> Reviewed-by: Steven Sistare
Small allocations don't fail, so just pushed to drm-misc-next, not -fixes.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 70dcfa58d3c2..ec503d416062 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1082,10 +1082,12 @@ static bool drm_dp_port_setup_pdt(struct
> drm_dp_mst_port *port)
> lct = drm_dp_calculate_rad(port, rad);
>
> port->mstb = drm_dp_add_mst_branch_device(lct, rad);
> - port->mstb->mgr = port->mgr;
> - port->mstb->port_parent = port;
> + if (port->mstb) {
> + port->mstb->mgr = port->mgr;
> + port->mstb->port_parent = port;
>
> - send_link = true;
> + send_link = true;
> + }
> break;
> }
> return send_link;
> --
> 2.15.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 1/9] drm/omap: Update omapdss API to allow alternative DSS implementations
Hi Jyri, Laurent,
On 16/02/18 13:25, Jyri Sarha wrote:
> After this patch OMAP_DSS_BASE module is not including any OMAP2_DSS
> headers, only the API omapdss.h. "sturct dss_data", the piece of the
> data structure needed for base.c is defined in omapdss.h and added as
> a member to struct dss_device, and later to struct dss6_device.
>
> The patch is still a bit hackish. The struct dispc_device declaration
> is currently shared between alternative dss implementations, with
> different internal definitions. It should be relatively simple to use
> a similar struct dispc_data as struct dss_data is for dss_device, move
> some common parts - maybe the dispc_ops itself - there and find the
> private data with container_of macro. Also the contents of struct
> dss_data in side dss_device is currently redundant. These should be
> easy enough to fix, if we decide to take this route.
>
> Signed-off-by: Jyri Sarha
> ---
> drivers/gpu/drm/omapdrm/dss/base.c| 11 +--
> drivers/gpu/drm/omapdrm/dss/dispc.c | 4
> drivers/gpu/drm/omapdrm/dss/dss.c | 2 +-
> drivers/gpu/drm/omapdrm/dss/dss.h | 2 ++
> drivers/gpu/drm/omapdrm/dss/omapdss.h | 13 +
> drivers/gpu/drm/omapdrm/omap_drv.h| 2 +-
> 6 files changed, 22 insertions(+), 12 deletions(-)
I think something in this direction would be good. I'd really like to keep it
possible to run dss6 on top of omapdrm.
But I think this can be done slightly simpler like this:
diff --git a/drivers/gpu/drm/omapdrm/dss/base.c
b/drivers/gpu/drm/omapdrm/dss/base.c
index 99e8cb8dc65b..08913e006e93 100644
--- a/drivers/gpu/drm/omapdrm/dss/base.c
+++ b/drivers/gpu/drm/omapdrm/dss/base.c
@@ -19,10 +19,11 @@
#include
#include
-#include "dss.h"
#include "omapdss.h"
static struct dss_device *dss_device;
+static struct dispc_device *s_dispc_device;
+static const struct dispc_ops *s_dispc_ops;
static struct list_head omapdss_comp_list;
@@ -46,16 +47,23 @@ EXPORT_SYMBOL(omapdss_set_dss);
struct dispc_device *dispc_get_dispc(struct dss_device *dss)
{
- return dss->dispc;
+ return s_dispc_device;
}
EXPORT_SYMBOL(dispc_get_dispc);
const struct dispc_ops *dispc_get_ops(struct dss_device *dss)
{
- return dss->dispc_ops;
+ return s_dispc_ops;
}
EXPORT_SYMBOL(dispc_get_ops);
+void omapdss_set_dispc(struct dispc_device *dispc, const struct dispc_ops*
dispc_ops)
+{
+ s_dispc_device = dispc;
+ s_dispc_ops = dispc_ops;
+}
+EXPORT_SYMBOL(omapdss_set_dispc);
+
static bool omapdss_list_contains(const struct device_node *node)
{
struct omapdss_comp_node *comp;
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index ce470b51e326..b72f981d660e 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -4786,7 +4786,7 @@ static int dispc_bind(struct device *dev, struct device
*master, void *data)
dispc_runtime_put(dispc);
dss->dispc = dispc;
- dss->dispc_ops = &dispc_ops;
+ omapdss_set_dispc(dispc, &dispc_ops);
dispc->debugfs = dss_debugfs_create_file(dss, "dispc", dispc_dump_regs,
dispc);
@@ -4807,8 +4807,8 @@ static void dispc_unbind(struct device *dev, struct
device *master, void *data)
dss_debugfs_remove_file(dispc->debugfs);
+ omapdss_set_dispc(NULL, NULL);
dss->dispc = NULL;
- dss->dispc_ops = NULL;
pm_runtime_disable(dev);
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h
b/drivers/gpu/drm/omapdrm/dss/dss.h
index 6f6fd3d1b159..3d23232ec1f7 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.h
+++ b/drivers/gpu/drm/omapdrm/dss/dss.h
@@ -274,7 +274,6 @@ struct dss_device {
struct dss_pll *video2_pll;
struct dispc_device *dispc;
- const struct dispc_ops *dispc_ops;
};
/* core */
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h
b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index a4f71e082c1c..b724dae22d7a 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -751,6 +751,7 @@ struct dispc_ops {
struct dispc_device *dispc_get_dispc(struct dss_device *dss);
const struct dispc_ops *dispc_get_ops(struct dss_device *dss);
+void omapdss_set_dispc(struct dispc_device *dispc, const struct dispc_ops*
dispc_ops);
bool omapdss_component_is_display(struct device_node *node);
bool omapdss_component_is_output(struct device_node *node);
Yes, it adds two new globals. But I don't think those are a big issue. Note
that I left the dss->dispc there, for dss internal use.
Laurent, what do you think? If this is fine, can you squash to your series? Or
I can even have this on top as well. I think otherwise it's good for merging.
Can you also have a quick look at patches 2, 3, 4, 5, 6 and 7. While their aim
is to get dss6 working, I think they're ok cleanups and shouldn't cause issues
with the main dss rework.
Tomi
--
Texas In
Re: [PATCH v2 3/4] drm: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On Mon, Feb 12, 2018 at 02:51:43PM -0500, Joe Moriarty wrote:
> The Parfait (version 2.1.0) static code analysis tool found the
> following NULL pointer derefernce problem.
>
> - drivers/gpu/drm/drm_edid.c
> The call to drm_cvt_mode() in function drm_mode_std() for the
> HDTV hack resulted in the possibility of accessing a NULL pointer
> if drm_mode_std() returned NULL. A check for this added right after
> the call to drm_cvt_mode() in this particular area of code.
>
> Signed-off-by: Joe Moriarty
> Reviewed-by: Steven Sistare
Applied to drm-misc-next, thanks.
-Daniel
> ---
> drivers/gpu/drm/drm_edid.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index ddd537914575..23c9977d8999 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2083,6 +2083,8 @@ drm_mode_std(struct drm_connector *connector, struct
> edid *edid,
> if (hsize == 1366 && vsize == 768 && vrefresh_rate == 60) {
> mode = drm_cvt_mode(dev, 1366, 768, vrefresh_rate, 0, 0,
> false);
> + if (!mode)
> + return NULL;
> mode->hdisplay = 1366;
> mode->hsync_start = mode->hsync_start - 1;
> mode->hsync_end = mode->hsync_end - 1;
> --
> 2.15.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105005] No image after downtime (RX460)
https://bugs.freedesktop.org/show_bug.cgi?id=105005 Dmitry changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #10 from Dmitry --- Now on 4.15.3 - 2 problems already there. The problem is not relevant. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 5/9] drm/omap: move common stuff from dss.h to omapdss.h
Hi Jyri,
On 16/02/18 13:25, Jyri Sarha wrote:
> The new DSS6 driver needs some structs and defines which are currently
> in dss.h, which is for the old DSS driver.
>
> Move the required structs and defines from dss.h to omapdss.h.
>
> Signed-off-by: Tomi Valkeinen
> Signed-off-by: Jyri Sarha
> ---
> drivers/gpu/drm/omapdrm/dss/dss.h | 41
> ++-
> drivers/gpu/drm/omapdrm/dss/omapdss.h | 37 +++
> 2 files changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h
> b/drivers/gpu/drm/omapdrm/dss/dss.h
> index 434262a..fa206ca 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.h
> @@ -70,14 +70,6 @@ struct seq_file;
> pr_warn("omapdss: " format, ##__VA_ARGS__)
> #endif
>
> -/* OMAP TRM gives bitfields as start:end, where start is the higher bit
> - number. For example 7:0 */
> -#define FLD_MASK(start, end) (((1 << ((start) - (end) + 1)) - 1) << (end))
> -#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> -#define FLD_GET(val, start, end) (((val) & FLD_MASK(start, end)) >> (end))
> -#define FLD_MOD(orig, val, start, end) \
> - (((orig) & ~FLD_MASK(start, end)) | FLD_VAL(val, start, end))
> -
> enum dss_model {
> DSS_MODEL_OMAP2,
> DSS_MODEL_OMAP3,
> @@ -86,12 +78,6 @@ enum dss_model {
> DSS_MODEL_DRA7,
> };
>
> -enum dss_io_pad_mode {
> - DSS_IO_PAD_MODE_RESET,
> - DSS_IO_PAD_MODE_RFBI,
> - DSS_IO_PAD_MODE_BYPASS,
> -};
> -
> enum dss_hdmi_venc_clk_source_select {
> DSS_VENC_TV_CLK = 0,
> DSS_HDMI_M_PCLK = 1,
> @@ -215,34 +201,11 @@ struct dss_reg_field {
> u8 start, end;
> };
>
> -struct dispc_clock_info {
> - /* rates that we get with dividers below */
> - unsigned long lck;
> - unsigned long pck;
> -
> - /* dividers */
> - u16 lck_div;
> - u16 pck_div;
> -};
> -
> -struct dss_lcd_mgr_config {
> - enum dss_io_pad_mode io_pad_mode;
> -
> - bool stallmode;
> - bool fifohandcheck;
> -
> - struct dispc_clock_info clock_info;
> -
> - int video_port_width;
> -
> - int lcden_sig_polarity;
> -};
> -
> -#define DSS_SZ_REGS SZ_512
> +#define DSS_SZ_REGSSZ_512
>
> struct dss_device {
> struct platform_device *pdev;
> - void __iomem*base;
> + void __iomem*base;
Extra changes here, and in the above SZ_REGS.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] meson: do not use cairo/valgrind if disabled
On Sunday, 2018-02-18 14:00:50 +0100, Igor Gnatenko wrote:
> -Dcairo-tests=false currently results into enabling cairo support if it
> was found. Same for valgrind.
Indeed, this was wrong; thanks for the fix!
Reviewed-by: Eric Engestrom
Do you have commit access, or do you want me to push this for you?
>
> Signed-off-by: Igor Gnatenko
> ---
> meson.build | 20
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 166559e8..695f89b3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -226,8 +226,20 @@ endforeach
>
> dep_pciaccess = dependency('pciaccess', version : '>= 0.10', required :
> with_intel)
> dep_cunit = dependency('cunit', version : '>= 2.1', required : false)
> -dep_cairo = dependency('cairo', required : with_cairo_tests == 'true')
> -dep_valgrind = dependency('valgrind', required : with_valgrind == 'true')
> +if with_cairo_tests != 'false'
> + dep_cairo = dependency('cairo', required : with_cairo_tests == 'true')
> + with_cairo_tests = dep_cairo.found()
> +else
> + dep_cairo = declare_dependency()
Nit: `dep_cairo = []` is enough; I'll change that if I'm the one to push it.
> + with_cairo_tests = false
We try to avoid changing the type of a var; could you send a follow-up
patch to rename the get_option() var to `_cairo_tests`?
(same obviously applies for the valgrind bits)
> +endif
> +if with_valgrind != 'false'
> + dep_valgrind = dependency('valgrind', required : with_valgrind == 'true')
> + with_valgrind = dep_valgrind.found()
> +else
> + dep_valgrind = declare_dependency()
> + with_valgrind = false
> +endif
>
> with_man_pages = get_option('man-pages')
> prog_xslt = find_program('xsltproc', required : with_man_pages == 'true')
> @@ -259,8 +271,8 @@ foreach t : [
> [with_radeon, 'RADEON'],
> [with_vc4, 'VC4'],
> [with_vmwgfx, 'VMWGFX'],
> - [dep_cairo.found(), 'CAIRO'],
> - [dep_valgrind.found(), 'VALGRIND'],
> + [with_cairo_tests, 'CAIRO'],
> + [with_valgrind, 'VALGRIND'],
> ]
>config.set10('HAVE_@0@'.format(t[1]), t[0])
> endforeach
> --
> 2.16.2
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers
On Mon, Feb 19, 2018 at 12:48:04PM +0100, Daniel Vetter wrote: > On Thu, Feb 15, 2018 at 06:38:44AM +0100, Lukas Wunner wrote: > > On Wed, Feb 14, 2018 at 09:58:43AM -0500, Sean Paul wrote: > > > On Wed, Feb 14, 2018 at 03:43:56PM +0100, Michel Dänzer wrote: > > > > On 2018-02-14 03:08 PM, Sean Paul wrote: > > > > > On Wed, Feb 14, 2018 at 10:26:35AM +0100, Maarten Lankhorst wrote: > > > > >> Op 14-02-18 om 09:46 schreef Lukas Wunner: > > > > >>> On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > > > > Fix a deadlock on hybrid graphics laptops that's been present > > > > since 2013: > > > > >>> This series has been reviewed, consent has been expressed by the > > > > >>> most > > > > >>> interested parties, patch [1/5] which touches files outside > > > > >>> drivers/gpu > > > > >>> has been acked and I've just out a v2 addressing the only objection > > > > >>> raised. My plan is thus to wait another two days for comments and, > > > > >>> barring further objections, push to drm-misc this weekend. > > > > >>> > > > > >>> However I'm struggling with the decision whether to push to next or > > > > >>> fixes. The series is marked for stable, however the number of > > > > >>> affected machines is limited and for an issue that's been present > > > > >>> for 5 years it probably doesn't matter if it soaks another two > > > > >>> months > > > > >>> in linux-next befor it gets backported. Hence I tend to err on the > > > > >>> side of caution and push to next, however a case could be made that > > > > >>> fixes is more appropriate. > > > > >>> > > > > >>> I'm lacking experience making such decisions and would be interested > > > > >>> to learn how you'd handle this. > > > > >> > > > > >> I would say fixes, it doesn't look particularly scary. :) > > > > > > > > > > Agreed. If it's good enough for stable, it's good enough for -fixes! > > > > > > > > It's not that simple, is it? Fast-tracking patches (some of which appear > > > > to be untested) to stable without an immediate cause for urgency seems > > > > risky to me. > > > > > > /me should be more careful what he says > > > > > > Given where we are in the release cycle, it's barely a fast track. > > > If these go in -fixes, they'll get in -rc2 and will have plenty of > > > time to bake. If we were at rc5, it might be a different story. > > > > The patches are marked for stable though, so if they go in through > > drm-misc-fixes, they may appear in stable kernels before 4.16-final > > is out. Greg picks up patches once they're in Linus' tree, though > > often with a delay of a few days or weeks. If they go in through > > drm-misc-next, they're guaranteed not to appear in *any* release > > before 4.16-final is out. > > > > This allows for differentiation between no-brainer stable fixes that > > can be sent immediately and scarier, but similarly important stable > > fixes that should soak for a while. I'm not sure which category > > this series belongs to, though it's true what Maarten says, it's > > not *that* grave a change. > > If you're this concerned about them, then pls do _not_ put cc: stable on > the patches. Instead get them merged through -fixes (or maybe even -next), > and once they're sufficiently tested, send a mail to stable@ asking for > ane explicit backport. I'm not concerned about them, but would have erred on the side of caution. However consensus seems to have been that they're sufficiently unscary to push to -fixes. Do you disagree with that decision, if so, why? Can we amend the dim docs to codify guidelines whether to push to -fixes or -next? I allowed 1 week for comments, now you're returning from vacation and seem to be unhappy, was 1 week too short? Thanks, Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 4/9] drm/omap: Make omapdss API more generic
Hi Jyri, On 16/02/18 13:25, Jyri Sarha wrote: > The new omapdss API is HW independent and cleans up some of the DSS5 > specific hacks from the omapdrm side and gets rid off the DSS5 IRQ > register bits and replace them with HW independent generic u64 based > macros. The new macros make it more straight forward to implement the > IRQ code for the future DSS versions that do not share the same > register structure as DSS2 to DSS5 has. This removes DISPC_IRQ_FRAMEDONEWB. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104345] Playing video hangs X-Server with showing scrambled picture, sound still playing.
https://bugs.freedesktop.org/show_bug.cgi?id=104345 --- Comment #10 from bernhardu --- Same happens with vanilla kernel 4.15.2. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] meson: do not use cairo/valgrind if disabled
-Dcairo-tests=false currently results into enabling cairo support if it
was found. Same for valgrind.
v2:
* Use underscore-prefixed variables to not change type of variable
* Use empty array for "fake" dependency instead of real empty object
Signed-off-by: Igor Gnatenko
---
meson.build | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/meson.build b/meson.build
index 166559e8..e4e21bae 100644
--- a/meson.build
+++ b/meson.build
@@ -32,8 +32,6 @@ pkg = import('pkgconfig')
with_udev = get_option('udev')
with_freedreno_kgsl = get_option('freedreno-kgsl')
with_install_tests = get_option('install-test-programs')
-with_cairo_tests = get_option('cairo-tests')
-with_valgrind = get_option('valgrind')
config = configuration_data()
@@ -226,8 +224,22 @@ endforeach
dep_pciaccess = dependency('pciaccess', version : '>= 0.10', required :
with_intel)
dep_cunit = dependency('cunit', version : '>= 2.1', required : false)
-dep_cairo = dependency('cairo', required : with_cairo_tests == 'true')
-dep_valgrind = dependency('valgrind', required : with_valgrind == 'true')
+_cairo_tests = get_option('cairo-tests')
+if _cairo_tests != 'false'
+ dep_cairo = dependency('cairo', required : _cairo_tests == 'true')
+ with_cairo_tests = dep_cairo.found()
+else
+ dep_cairo = []
+ with_cairo_tests = false
+endif
+_valgrind = get_option('valgrind')
+if _valgrind != 'false'
+ dep_valgrind = dependency('valgrind', required : with_valgrind == 'true')
+ with_valgrind = dep_valgrind.found()
+else
+ dep_valgrind = []
+ with_valgrind = false
+endif
with_man_pages = get_option('man-pages')
prog_xslt = find_program('xsltproc', required : with_man_pages == 'true')
@@ -259,8 +271,8 @@ foreach t : [
[with_radeon, 'RADEON'],
[with_vc4, 'VC4'],
[with_vmwgfx, 'VMWGFX'],
- [dep_cairo.found(), 'CAIRO'],
- [dep_valgrind.found(), 'VALGRIND'],
+ [with_cairo_tests, 'CAIRO'],
+ [with_valgrind, 'VALGRIND'],
]
config.set10('HAVE_@0@'.format(t[1]), t[0])
endforeach
--
2.16.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] meson: do not use cairo/valgrind if disabled
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
On Mon, 2018-02-19 at 12:15 +, Eric Engestrom wrote:
> On Sunday, 2018-02-18 14:00:50 +0100, Igor Gnatenko wrote:
> > -Dcairo-tests=false currently results into enabling cairo support if it
> > was found. Same for valgrind.
>
> Indeed, this was wrong; thanks for the fix!
> Reviewed-by: Eric Engestrom
>
> Do you have commit access, or do you want me to push this for you?
I don't have commit access ☹
v2 sent with all your comments.
> >
> > Signed-off-by: Igor Gnatenko
> > ---
> > meson.build | 20
> > 1 file changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 166559e8..695f89b3 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -226,8 +226,20 @@ endforeach
> >
> > dep_pciaccess = dependency('pciaccess', version : '>= 0.10', required :
> > with_intel)
> > dep_cunit = dependency('cunit', version : '>= 2.1', required : false)
> > -dep_cairo = dependency('cairo', required : with_cairo_tests == 'true')
> > -dep_valgrind = dependency('valgrind', required : with_valgrind == 'true')
> > +if with_cairo_tests != 'false'
> > + dep_cairo = dependency('cairo', required : with_cairo_tests == 'true')
> > + with_cairo_tests = dep_cairo.found()
> > +else
> > + dep_cairo = declare_dependency()
>
> Nit: `dep_cairo = []` is enough; I'll change that if I'm the one to push it.
>
> > + with_cairo_tests = false
>
> We try to avoid changing the type of a var; could you send a follow-up
> patch to rename the get_option() var to `_cairo_tests`?
>
> (same obviously applies for the valgrind bits)
>
> > +endif
> > +if with_valgrind != 'false'
> > + dep_valgrind = dependency('valgrind', required : with_valgrind ==
> > 'true')
> > + with_valgrind = dep_valgrind.found()
> > +else
> > + dep_valgrind = declare_dependency()
> > + with_valgrind = false
> > +endif
> >
> > with_man_pages = get_option('man-pages')
> > prog_xslt = find_program('xsltproc', required : with_man_pages == 'true')
> > @@ -259,8 +271,8 @@ foreach t : [
> > [with_radeon, 'RADEON'],
> > [with_vc4, 'VC4'],
> > [with_vmwgfx, 'VMWGFX'],
> > - [dep_cairo.found(), 'CAIRO'],
> > - [dep_valgrind.found(), 'VALGRIND'],
> > + [with_cairo_tests, 'CAIRO'],
> > + [with_valgrind, 'VALGRIND'],
> > ]
> >config.set10('HAVE_@0@'.format(t[1]), t[0])
> > endforeach
> > --
> > 2.16.2
> >
- --
- -Igor Gnatenko
-BEGIN PGP SIGNATURE-
iQIzBAEBCAAdFiEEhLFO09aHZVqO+CM6aVcUvRu8X0wFAlqKyFAACgkQaVcUvRu8
X0zP3xAAsa1mi9r8Oi+pdJrXy02Bm2mGSzp8IcrDGF3i0v9d9uP7o484fmYxxz0t
RDu/4YpertGmR+aYiZiEeREnZ7n+GtaBb35O1leU/zz/TniEi+Qne6kQbUcUNE3z
3URS2VKmHPAivR5ctL5/DvwOaAP7sIojlDjsu3ydZafoPVw+FF/cnJaK6yItVH7T
7ZE48i434qBUJT6IsxUCy2jb7gQSCzs/G/gNxYjVxLO/h/rb52eIiPQ5XFd6Aqmt
R5ptwgb4wpQERTEkvSmFTe+tsIwmSyD8o6jJRAK4U0mGV8g5+AB/7fmZmFA7nxQ4
J9Ycqd2JZ96XruBE9qSvu9gK50oVKcQyJaq2heTKkRSwq+HP82qfhtaHsC5hsrTG
lg2+bzpyGStxXzt1bndYQ2u9hcPcbvDxb9mDP5wimacdmD/qmAg2LAv5OiZESgvd
Zm8TWygb/bjJoLZOTdbGqdjFGmOCq3g9ZTqbjWfhv1mmc2ZENoo/fcyyCjUg4MD3
P4IP3ogAUk9H1MORhN6I5rw2ERDGaXy60z4dJwhAfHWmwsoKRbdpRmu9Y2vFQ2n9
/kysL8yG+DPsUHB6f5ZZh/r7dsJDWeIea8ZI2gEMrGOIFxSZUcypwmjxxgC5Yo2x
EwRsGvw/GAKhQQX8ukgMMy878Qj7yRvf6PoMGlVqtjlmFRVXlgM=
=q2az
-END PGP SIGNATURE-
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] meson: do not use cairo/valgrind if disabled
-Dcairo-tests=false currently results into enabling cairo support if it
was found. Same for valgrind.
v2:
* Use underscore-prefixed variables to not change type of variable
* Use empty array for "fake" dependency instead of real empty object
v3:
* Fix typo
Reviewed-by: Eric Engestrom
Signed-off-by: Igor Gnatenko
---
meson.build | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/meson.build b/meson.build
index 166559e8..7f786a8c 100644
--- a/meson.build
+++ b/meson.build
@@ -32,8 +32,6 @@ pkg = import('pkgconfig')
with_udev = get_option('udev')
with_freedreno_kgsl = get_option('freedreno-kgsl')
with_install_tests = get_option('install-test-programs')
-with_cairo_tests = get_option('cairo-tests')
-with_valgrind = get_option('valgrind')
config = configuration_data()
@@ -226,8 +224,22 @@ endforeach
dep_pciaccess = dependency('pciaccess', version : '>= 0.10', required :
with_intel)
dep_cunit = dependency('cunit', version : '>= 2.1', required : false)
-dep_cairo = dependency('cairo', required : with_cairo_tests == 'true')
-dep_valgrind = dependency('valgrind', required : with_valgrind == 'true')
+_cairo_tests = get_option('cairo-tests')
+if _cairo_tests != 'false'
+ dep_cairo = dependency('cairo', required : _cairo_tests == 'true')
+ with_cairo_tests = dep_cairo.found()
+else
+ dep_cairo = []
+ with_cairo_tests = false
+endif
+_valgrind = get_option('valgrind')
+if _valgrind != 'false'
+ dep_valgrind = dependency('valgrind', required : _valgrind == 'true')
+ with_valgrind = dep_valgrind.found()
+else
+ dep_valgrind = []
+ with_valgrind = false
+endif
with_man_pages = get_option('man-pages')
prog_xslt = find_program('xsltproc', required : with_man_pages == 'true')
@@ -259,8 +271,8 @@ foreach t : [
[with_radeon, 'RADEON'],
[with_vc4, 'VC4'],
[with_vmwgfx, 'VMWGFX'],
- [dep_cairo.found(), 'CAIRO'],
- [dep_valgrind.found(), 'VALGRIND'],
+ [with_cairo_tests, 'CAIRO'],
+ [with_valgrind, 'VALGRIND'],
]
config.set10('HAVE_@0@'.format(t[1]), t[0])
endforeach
--
2.16.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/4] drm: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
On Mon, Feb 12, 2018 at 02:51:44PM -0500, Joe Moriarty wrote:
> The Parfait (version 2.1.0) static code analysis tool found the
> following NULL pointer derefernce problem.
>
> - drivers/gpu/drm/drm_vblank.c
> Null pointer checks were added to return values from calls to
> drm_crtc_from_index(). There is a possibility, however minute, that
> crtc->index may not be found when trying to find the struct crtc
> from it's assigned index given in drm_crtc_init_with_planes().
> 3 return checks for NULL where added.
>
> Signed-off-by: Joe Moriarty
> Reviewed-by: Steven Sistare
These are all drivers bugs, we'd need at least a WARN_ON when the crtc
doesn't exist. Otherwise this would just silently paper over a fairly
serious kernel bug (which doesn't improve things really).
Something like
if (WARN_ON(!crtc))
return NULL;
is what I'd go with.
-Daniel
> ---
> drivers/gpu/drm/drm_vblank.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 32d9bcf5be7f..a3a1bce87468 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -120,7 +120,7 @@ static u32 __get_vblank_counter(struct drm_device *dev,
> unsigned int pipe)
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>
> - if (crtc->funcs->get_vblank_counter)
> + if (crtc && crtc->funcs->get_vblank_counter)
> return crtc->funcs->get_vblank_counter(crtc);
> }
>
> @@ -318,7 +318,7 @@ static void __disable_vblank(struct drm_device *dev,
> unsigned int pipe)
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>
> - if (crtc->funcs->disable_vblank) {
> + if (crtc && crtc->funcs->disable_vblank) {
> crtc->funcs->disable_vblank(crtc);
> return;
> }
> @@ -918,7 +918,7 @@ static int __enable_vblank(struct drm_device *dev,
> unsigned int pipe)
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>
> - if (crtc->funcs->enable_vblank)
> + if (crtc && crtc->funcs->enable_vblank)
> return crtc->funcs->enable_vblank(crtc);
> }
>
> --
> 2.15.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] s390/console: enable dummy console for vt
On 02/19/2018 02:35 PM, Farhan Ali wrote: > > > On 02/15/2018 07:02 AM, Christian Borntraeger wrote: >> >> >> On 02/15/2018 12:57 PM, Thomas Huth wrote: >>> On 15.02.2018 12:26, Geert Uytterhoeven wrote: Hi Christian, On Thu, Feb 15, 2018 at 12:14 PM, Christian Borntraeger wrote: > To enable the virtual terminal layer with virtio-gpu, we need to > provide the dummy console. This console is hidden behind CONFIG_IOMEM > via the graphics support. Instead of fully enabling the graphic > drivers lets just provide a Kconfig option for the dummy console. > > Signed-off-by: Christian Borntraeger > --- > New version: instead of moving around the graphic and console stuff, > let's just keep an s390 specific variant of CONFIG_DUMMY_CONSOLE > arch/s390/Kconfig | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > index cbe1d978693a..a69690f616f3 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -952,6 +952,11 @@ config S390_HYPFS_FS > > source "arch/s390/kvm/Kconfig" > > +config DUMMY_CONSOLE > + bool > + depends on VT > + default y > + > config S390_GUEST > def_bool y > prompt "s390 support for virtio devices" Really? You already have your own copy of HAS_IOMEM, which makes it hard for people to track which one applies where. >>> >>> I think I agree with Geert - let's better fix this in a proper way >>> instead of doing hacks like this. I guess there will be other >>> architectures in the future that might want to use the dummy console >>> without CONFIG_IOMEM, so fixing this in drivers/video/ instead sounds >>> better to me. >> >> The question is, what is the proper fix? >> > > How about we only fence off sub menu items such as DRM or GPU or Fbdev, which > actually uses io memory, in drivers/video/Kconfig? Similar to what Thomas > suggested for moving the CONFIG_IOMEM dependency for fbdevs? Can you spin a patch? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 05/12] drm/bridge/synopsys: dw-hdmi: don't clobber drvdata
On Wed, 2018-02-14 at 21:08 +0100, Jernej Skrabec wrote:
> dw_hdmi shouldn't set drvdata since some drivers might need to store
> it's own data there. Rework dw_hdmi in a way to return struct dw_hdmi
> instead to store it in drvdata. This way drivers are responsible to
> store and pass structure when needed.
>
> Idea was taken from the following commit:
> 8242ecbd597d ("drm/bridge/synopsys: stop clobbering drvdata")
>
> Cc: p.za...@pengutronix.de
> Cc: narmstr...@baylibre.com
> Cc: laurent.pinch...@ideasonboard.com
> Cc: h...@rock-chips.com
> Cc: he...@sntech.de
> Acked-by: Neil Armstrong
> Signed-off-by: Jernej Skrabec
For i.MX:
Tested-by: Philipp Zabel
Acked-by: Philipp Zabel
regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [Nouveau] [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers
On Mon, Feb 19, 2018 at 12:58:17PM +0100, Lukas Wunner wrote: > On Mon, Feb 19, 2018 at 12:34:43PM +0100, Daniel Vetter wrote: > > On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > > > Fix a deadlock on hybrid graphics laptops that's been present since 2013: > > > > > > DRM drivers poll connectors in 10 sec intervals. The poll worker is > > > stopped on ->runtime_suspend with cancel_delayed_work_sync(). However > > > the poll worker invokes the DRM drivers' ->detect callbacks, which call > > > pm_runtime_get_sync(). If the poll worker starts after runtime suspend > > > has begun, pm_runtime_get_sync() will wait for runtime suspend to finish > > > with the intention of runtime resuming the device afterwards. The result > > > is a circular wait between poll worker and autosuspend worker. > > > > Don't shut down the poll worker when runtime suspending, that' doesn't > > work. If you need the poll work, then that means waking up the gpu every > > few seconds. If you don't need it, then make sure the DRM_CONNECTOR_POLL > > flags are set correctly (you can update them at runtime, the poll worker > > will pick that up). > > > > That should fix the deadlock, and it's how we do it in i915 (where igt in > > CI totally hammers the runtime pm support, and it seems to hold up). > > It would fix the deadlock but it's not an option on dual GPU laptops. > Power consumption of the discrete GPU is massive (9 W on my machine). > > > > > i915, malidp and msm "solved" this issue by not stopping the poll worker > > > on runtime suspend. But this results in the GPU bouncing back and forth > > > between D0 and D3 continuously. That's a total no-go for GPUs which > > > runtime suspend to D3cold since every suspend/resume cycle costs a > > > significant amount of time and energy. (i915 and malidp do not seem > > > to acquire a runtime PM ref in the ->detect callbacks, which seems > > > questionable. msm however does and would also deadlock if it disabled > > > the poll worker on runtime suspend. cc += Archit, Liviu, intel-gfx) > > > > Well, userspace expects hotplug events, even when we runtime suspend > > stuff. Hence waking shit up with polling. Imo ignoring hotplug events is a > > pretty serious policy decision which is ok in the context of > > vga_switcheroo, but not really as an automatic thing. E.g. usb also wakes > > up if you plug something in, even with all the runtime pm stuff enabled. > > Same for sata and everything else. > > On the MacBook Pro, the HPD pin of external DP and HDMI ports goes into > the gmux controller, which sends an interrupt on hotplug even if the GPU > is powered down. > > Apparently Optimus has a similar functionality, cf. 3a6536c51d5d. Yeah, for vga_switcheroo and gmux setups shutting down polling explicitly makes sense. I think ideally we'd stop polling in the gmux handler somehow (maybe by dropping the relevant DRM_CONNECTOR_POLL flags, or outright stopping it all). But not when runtime suspending the entire gpu (e.g. idle system that shuts down the screen and everything, before it decides a few minutes later to do a full system suspend). I also think that this approach would lead to cleaner code, having explicit checks for the (locking) execution context all over the place tends to result in regrets eventually ime. > For the rare cases where an external VGA or DVI-A port is present, I guess > it's reasonable to have the user wake up the machine manually. > > I'm not sure why nouveau polls ports on my laptop, the GK107 only has an > LVDS and three DP ports, need to investigate. Yeah, that'd be good to figure out. The probe helpers should shut down the worker if there's no connector that needs probing. We use that to enable/disable the poll worker when there's a hotplug storm on the irq line. Once that's fixed we can perhaps also untangle the poll-vs-gmux story. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] gpu: ipu-v3: prg: fix device node leak in ipu_prg_lookup_by_phandle
Hi Tobias,
On Thu, 2018-02-15 at 15:35 +0100, Tobias Jordan wrote:
> Before returning, call of_node_put() for the device node returned by
> of_parse_phandle().
>
> Fixes: ea9c260514c1 ("gpu: ipu-v3: add driver for Prefetch Resolve Gasket")
> Signed-off-by: Tobias Jordan
> ---
> drivers/gpu/ipu-v3/ipu-prg.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-prg.c b/drivers/gpu/ipu-v3/ipu-prg.c
> index 067365c733c6..97b99500153d 100644
> --- a/drivers/gpu/ipu-v3/ipu-prg.c
> +++ b/drivers/gpu/ipu-v3/ipu-prg.c
> @@ -102,11 +102,14 @@ ipu_prg_lookup_by_phandle(struct device *dev, const
> char *name, int ipu_id)
> mutex_unlock(&ipu_prg_list_mutex);
> device_link_add(dev, prg->dev, DL_FLAG_AUTOREMOVE);
> prg->id = ipu_id;
> + of_node_put(prg_node);
> return prg;
> }
> }
> mutex_unlock(&ipu_prg_list_mutex);
>
> + of_node_put(prg_node);
> +
> return NULL;
> }
>
Applied both to imx-drm/next, thank you.
regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] gpu: ipu-v3: make const arrays int_reg static, shrinks object size
Hi Colin,
On Wed, 2018-02-14 at 18:45 +, Colin King wrote:
> From: Colin Ian King
>
> Don't populate the const read-only arrays int_reg on the stack but instead
> make them static. Makes the object code smaller by over 80 bytes:
>
> Before:
>text data bss dec hex filename
> 28024 8936 192 371529120 drivers/gpu/ipu-v3/ipu-common.o
>
> After:
>text data bss dec hex filename
> 27794 9080 192 3706690ca drivers/gpu/ipu-v3/ipu-common.o
>
> (gcc version 7.2.0 x86_64)
>
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/ipu-v3/ipu-common.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
> index 658fa2d3e40c..48685cddbad1 100644
> --- a/drivers/gpu/ipu-v3/ipu-common.c
> +++ b/drivers/gpu/ipu-v3/ipu-common.c
> @@ -1089,7 +1089,7 @@ static void ipu_irq_handler(struct irq_desc *desc)
> {
> struct ipu_soc *ipu = irq_desc_get_handler_data(desc);
> struct irq_chip *chip = irq_desc_get_chip(desc);
> - const int int_reg[] = { 0, 1, 2, 3, 10, 11, 12, 13, 14};
> + static const int int_reg[] = { 0, 1, 2, 3, 10, 11, 12, 13, 14};
>
> chained_irq_enter(chip, desc);
>
> @@ -1102,7 +1102,7 @@ static void ipu_err_irq_handler(struct irq_desc *desc)
> {
> struct ipu_soc *ipu = irq_desc_get_handler_data(desc);
> struct irq_chip *chip = irq_desc_get_chip(desc);
> - const int int_reg[] = { 4, 5, 8, 9};
> + static const int int_reg[] = { 4, 5, 8, 9};
>
> chained_irq_enter(chip, desc);
Applied to imx-drm/next, thank you.
regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v3 4/6] drm: drm_fourcc: Add new formats for Xilinx IPs
On Fri, Feb 09, 2018 at 05:35:54PM -0800, Hyun Kwon wrote:
> This patch adds new formats needed by Xilinx IP. Pixels are not
> byte-aligned in these formats, and the drm_format_info for these
> formats has macro-pixel information.
>
> Signed-off-by: Jeffrey Mouroux
> Signed-off-by: Hyun Kwon
> ---
> v3
> - Update entries for changes
> - Squash fourcc patch into this
> v2
> - Add detailed descriptions
> - Remove formats with no user
> ---
> ---
> drivers/gpu/drm/drm_fourcc.c | 2 ++
> include/uapi/drm/drm_fourcc.h | 8
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index ed95de2..36bff7a 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -168,6 +168,8 @@ const struct drm_format_info *__drm_format_info(u32
> format)
> { .format = DRM_FORMAT_NV61,.depth = 0,
> .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_NV24,.depth = 0,
> .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> { .format = DRM_FORMAT_NV42,.depth = 0,
> .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_XV15,.depth = 0,
> .num_planes = 2, .pixels_per_macropixel = { 3, 3, 0 }, .bytes_per_macropixel
> = { 4, 8, 0 }, .hsub = 2, .vsub = 2, },
> + { .format = DRM_FORMAT_XV20,.depth = 0,
> .num_planes = 2, .pixels_per_macropixel = { 3, 3, 0 }, .bytes_per_macropixel
> = { 4, 8, 0 }, .hsub = 2, .vsub = 1, },
There's no need to set fields explicitly to 0. I think we could even do a
separate patch to nuke all the .depth = 0, assignments.
One thing that I've realized now that your new pixel formats stick out:
How is macropixel supposed to interact with hsub/vsub? From you example it
looks like macropixels are applied after subsampling (i.e. a macropixel
block of 3 pixels, but hsub = 2 means the macroblock will actually span 6
pixels). I think the kerneldoc in the earlier patch should explain this is
allowed, and how it's supposed to work exactly.
Also, do we have open-source userspace somewhere for this new pixel format?
Thanks, Daniel
> { .format = DRM_FORMAT_YUYV,.depth = 0,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_YVYU,.depth = 0,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_UYVY,.depth = 0,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index e04613d..6ac5282 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -142,6 +142,14 @@ extern "C" {
> #define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /*
> non-subsampled Cb:Cr plane */
>
> /*
> + * 2 plane 10 bit per component YCbCr
> + * index 0 = Y plane, [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian
> + * index 1 = Cb:Cr plane, [63:0] x:Cb2:Cr2:Cb1:x:Cr1:Cb0:Cr0
> 2:10:10:10:2:10:10:10 little endian
> + */
> +#define DRM_FORMAT_XV15 fourcc_code('X', 'V', '1', '5') /* 2x2
> subsampled Cb:Cr plane 2:10:10:10 */
> +#define DRM_FORMAT_XV20 fourcc_code('X', 'V', '2', '0') /* 2x1
> subsampled Cb:Cr plane 2:10:10:10 */
> +
> +/*
> * 3 plane YCbCr
> * index 0: Y plane, [7:0] Y
> * index 1: Cb plane, [7:0] Cb
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 1/6] dt-bindings: add bindings for USB physical connector
On Thu, Feb 15, 2018 at 11:39:15AM +0100, Andrzej Hajda wrote:
> These bindings allow to describe most known standard USB connectors
> and it should be possible to extend it if necessary.
> USB connectors, beside USB can be used to route other protocols,
> for example UART, Audio, MHL. In such case every device passing data
> through the connector should have appropriate graph bindings.
>
> Signed-off-by: Andrzej Hajda
> ---
> v3:
> - removed MHL port (samsung connector will have separate bindings),
> - added 2nd example for USB-C,
> - improved formatting
> v2:
> - moved connector type(A,B,C) to compatible string (Rob),
> - renamed size property to type (Rob),
> - changed type description to be less confusing (Laurent),
> - removed vendor specific compatibles (implied by graph port number),
> - added requirement of connector being a child of IC (Rob),
> - removed max-mode (subtly suggested by Rob, it should be detected anyway
> by USB Controller in runtime, downside is that device is not able to
> report its real capabilities, maybe better would be to make it optional(?)),
> - assigned port numbers to data buses (Rob).
>
> Regards
> Andrzej
>
> Signed-off-by: Andrzej Hajda
>
> dt-bindings: add bindings for USB physical connector v3
> ---
> .../bindings/connector/usb-connector.txt | 74
> ++
> 1 file changed, 74 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/connector/usb-connector.txt
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt
> b/Documentation/devicetree/bindings/connector/usb-connector.txt
> new file mode 100644
> index ..1efda92639da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -0,0 +1,74 @@
> +USB Connector
> +=
> +
> +USB connector node represents physical USB connector. It should be
> +a child of USB interface controller.
> +
> +Required properties:
> +- compatible: describes type of the connector, must be one of:
> +"usb-a-connector",
> +"usb-b-connector",
> +"usb-c-connector".
> +
> +Optional properties:
> +- label: symbolic name for the connector,
> +- type: size of the connector, should be specified in case of USB-A, USB-B
> + non-standard (large) connector sizes: "mini", "micro".
The smaller connectors are standard too. Perhaps "non-fullsize connector
sizes".
We're missing a micro-AB connector, but I think those are actually
pretty rare. Most phones are micro-B connectors, but do both host and
device.
> +
> +Required nodes:
> +- any data bus to the connector should be modeled using the OF graph bindings
> + specified in bindings/graph.txt, unless the bus is between parent node and
> + the connector. Since single connector can have multpile data buses every
> bus
> + has assigned OF graph port number as follows:
> +0: High Speed (HS), present in all connectors,
> +1: Super Speed (SS), present in SS capable connectors,
This should also say endpoint 0 is USB-SS, endpoint 1 (and higher?) is
Alternate Mode. And show in the example.
> +2: Sideband use (SBU), present in USB-C.
> +
> +Examples
> +
> +
> +1. Micro-USB connector with HS lines routed via controller (MUIC):
> +
> +muic-max77843@66 {
> + ...
> + usb_con: connector {
> + compatible = "usb-b-connector";
> + label = "micro-USB";
> + type = "micro";
> + };
> +};
> +
> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort:
Having SBU to DP but no DP video path connection is wrong.
> +
> +ccic: s2mm005@33 {
> + ...
> + usb_con: connector {
> + compatible = "usb-c-connector";
> + label = "USB-C";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + usb_con_hs: endpoint {
> + remote-endpoint = <&max77865_usbc_hs>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + usb_con_ss: endpoint {
> + remote-endpoint = <&usbdrd_phy_ss>;
> + };
> + };
> + port@2 {
> + reg = <2>;
> + usb_con_sbu: endpoint {
> + remote-endpoint = <&dp_aux>;
> + };
> + };
> + };
> + };
> +};
> --
> 2.16.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/simple_kms_helper: Fix NULL pointer dereference with no active CRTC
On Tue, Feb 13, 2018 at 10:44:16AM +0200, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > It is possible that drm_simple_kms_plane_atomic_check called > with no CRTC set, e.g. when user-space application sets CRTC_ID/FB_ID > to 0 before doing any actual drawing. This leads to NULL pointer > dereference because in this case new CRTC state is NULL and must be > checked before accessing. > > Signed-off-by: Oleksandr Andrushchenko > --- > drivers/gpu/drm/drm_simple_kms_helper.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c > b/drivers/gpu/drm/drm_simple_kms_helper.c > index 9ca8a4a59b74..a05eca9cec8b 100644 > --- a/drivers/gpu/drm/drm_simple_kms_helper.c > +++ b/drivers/gpu/drm/drm_simple_kms_helper.c > @@ -121,8 +121,10 @@ static int drm_simple_kms_plane_atomic_check(struct > drm_plane *plane, > pipe = container_of(plane, struct drm_simple_display_pipe, plane); > crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, > &pipe->crtc); > - if (!crtc_state->enable) > - return 0; /* nothing to check when disabling or disabled */ > + > + if (!crtc_state || !crtc_state->enable) > + /* nothing to check when disabling or disabled or no CRTC set */ > + return 0; > > if (crtc_state->enable) > drm_mode_get_hv_timing(&crtc_state->mode, Hm, this is a bit annoying, since the can_position = false parameter to drm_atomic_helper_check_plane_state is supposed to catch all this. Would moving all the checks after the call to that helper, and gating them on plane_state->visible also work? We'd need to add a guarantee to drm_atomic_helper_check_plane_state that it can cope with crtc_state == NULL, but I think that's a good idea anyway. Atm it shouldn't end up looking at the crtc_state pointer in that case. Otherwise we'll just go with your fix, but it feels all a bit too fragile, hence why I want to explore more robust options a bit. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 2/6] dt-bindings: add bindings for Samsung micro-USB 11-pin connector
On Thu, Feb 15, 2018 at 11:39:16AM +0100, Andrzej Hajda wrote:
> Samsung micro-USB 11-pin connector beside standard micro-USB pins,
> has pins dedicated to route MHL traffic.
>
> Signed-off-by: Andrzej Hajda
> ---
> .../connector/samsung,usb-connector-11pin.txt | 51
> ++
> 1 file changed, 51 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
>
> diff --git
> a/Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
> b/Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
> new file mode 100644
> index ..c8ef1ad6732f
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
> @@ -0,0 +1,51 @@
> +Samsung micro-USB 11-pin connector
> +==
> +
> +Samsung micro-USB 11-pin connector is an extension of micro-USB connector.
> +It is present in multiple Samsung mobile devices.
> +It has additional pins to route MHL traffic simultanously with USB.
> +
> +The bindings are superset of usb-connector bindings for micro-USB
> connector[1].
> +
> +Required properties:
> +- compatible: must be: "samsung,usb-connector-11pin", "usb-b-connector",
> +- type: must be "micro".
> +
> +Optional properties:
> +- label: symbolic name for the connector.
This is already defined in [1] so you don't need it here.
Otherwise,
Reviewed-by: Rob Herring
> +
> +Required nodes:
> +- any data bus to the connector should be modeled using the OF graph bindings
> + specified in bindings/graph.txt, unless the bus is between parent node and
> + the connector. Since single connector can have multpile data buses every
> bus
> + has assigned OF graph port number as follows:
> +0: High Speed (HS),
> +3: Mobile High-Definition Link (MHL), specific to 11-pin Samsung
> micro-USB.
> +
> +[1]: bindings/connector/usb-connector.txt
> +
> +Example
> +---
> +
> +Micro-USB connector with HS lines routed via controller (MUIC) and :
> +
> +muic-max77843@66 {
> + ...
> + usb_con: connector {
> + compatible = "samsung,usb-connector-11pin", "usb-b-connector";
> + label = "micro-USB";
> + type = "micro";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@3 {
> + reg = <3>;
> + usb_con_mhl: endpoint {
> + remote-endpoint = <&sii8620_mhl>;
> + };
> + };
> + };
> + };
> +};
> --
> 2.16.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 4/6] drm/panel: simple: Add ability to override typical timing
Hi,
2018-02-08 18:48 GMT+01:00 Sean Paul :
> This patch adds the ability to override the typical display timing for a
> given panel. This is useful for devices which have timing constraints
> that do not apply across the entire display driver (eg: to avoid
> crosstalk between panel and digitizer on certain laptops). The rules are
> as follows:
>
> - panel must not specify fixed mode (since the override mode will
> either be the same as the fixed mode, or we'll be unable to
> check the bounds of the overried)
> - panel must specify at least one display_timing range which will be
> used to ensure the override mode fits within its bounds
>
> Changes in v2:
> - Parse the full display-timings node (using the native-mode) (Rob)
> Changes in v3:
> - No longer parse display-timings subnode, use panel-timing (Rob)
>
> Cc: Doug Anderson
> Cc: Eric Anholt
> Cc: Heiko Stuebner
> Cc: Jeffy Chen
> Cc: Rob Herring
> Cc: Stéphane Marchesin
> Cc: Thierry Reding
> Cc: devicet...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Sean Paul
> ---
> drivers/gpu/drm/panel/panel-simple.c | 67
> +++-
> 1 file changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 5591984a392b..87488392bca1 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -34,6 +34,7 @@
> #include
>
> #include
> +#include
> #include
>
> struct panel_desc {
> @@ -87,6 +88,8 @@ struct panel_simple {
> struct i2c_adapter *ddc;
>
> struct gpio_desc *enable_gpio;
> +
> + struct drm_display_mode override_mode;
> };
>
> static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
> @@ -99,11 +102,22 @@ static int panel_simple_get_fixed_modes(struct
> panel_simple *panel)
> struct drm_connector *connector = panel->base.connector;
> struct drm_device *drm = panel->base.drm;
> struct drm_display_mode *mode;
> + bool has_override = panel->override_mode.type;
> unsigned int i, num = 0;
>
> if (!panel->desc)
> return 0;
>
> + if (has_override) {
> + mode = drm_mode_duplicate(drm, &panel->override_mode);
> + if (mode) {
> + drm_mode_probed_add(connector, mode);
> + num++;
> + } else {
> + dev_err(drm->dev, "failed to add override mode\n");
> + }
> + }
> +
> for (i = 0; i < panel->desc->num_timings; i++) {
> const struct display_timing *dt = &panel->desc->timings[i];
> struct videomode vm;
> @@ -120,7 +134,7 @@ static int panel_simple_get_fixed_modes(struct
> panel_simple *panel)
>
> mode->type |= DRM_MODE_TYPE_DRIVER;
>
> - if (panel->desc->num_timings == 1)
> + if (panel->desc->num_timings == 1 && !has_override)
> mode->type |= DRM_MODE_TYPE_PREFERRED;
>
> drm_mode_probed_add(connector, mode);
> @@ -291,10 +305,58 @@ static const struct drm_panel_funcs panel_simple_funcs
> = {
> .get_timings = panel_simple_get_timings,
> };
>
> +#define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \
> + (to_check->field.typ >= bounds->field.min && \
> +to_check->field.typ <= bounds->field.max)
> +static void panel_simple_add_override_mode(struct device *dev,
> + struct panel_simple *panel,
> + const struct display_timing *ot)
> +{
> + const struct panel_desc *desc = panel->desc;
> + struct videomode vm;
> + int i;
> +
> + if (desc->num_modes) {
> + dev_err(dev, "Reject override mode: panel has a fixed
> mode\n");
> + return;
> + }
> + if (!desc->num_timings) {
> + dev_err(dev, "Reject override mode: no timings specified\n");
> + return;
> + }
> +
> + for (i = 0; i < panel->desc->num_timings; i++) {
> + const struct display_timing *dt = &panel->desc->timings[i];
> +
> + if (!PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hactive) ||
> + !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hfront_porch) ||
> + !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hback_porch) ||
> + !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hsync_len) ||
> + !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vactive) ||
> + !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vfront_porch) ||
> + !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vback_porch) ||
> + !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vsync_len))
> + continue;
> +
> + if (ot->flags != dt->flags)
> + continue;
> +
> + videomode_from_timing(ot, &
Re: [PATCH v3 5/6] drm/panel: simple: Use display_timing for lq123p1jx31
Hi,
2018-02-08 18:48 GMT+01:00 Sean Paul :
> Convert the sharp lq123p1jx31 from using a fixed mode to specifying a
> display timing with min/typ/max values. This allows us to capture the
> timings set forth in the datasheet as well as the additional values that
> we've cleared with the display vendor to avoid interference with the
> digitizer on the Samsung Chromebook Plus (kevin).
>
> A follow-on patch will specify the override mode for kevin devices.
>
> Changes in v2:
> - None
> Changes in v3:
> - None
>
> Cc: Doug Anderson
> Cc: Eric Anholt
> Cc: Heiko Stuebner
> Cc: Jeffy Chen
> Cc: Rob Herring
> Cc: Stéphane Marchesin
> Cc: Thierry Reding
> Cc: devicet...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-rockc...@lists.infradead.org
> Signed-off-by: Sean Paul
> ---
> drivers/gpu/drm/panel/panel-simple.c | 27 +--
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 87488392bca1..6de9c39bc182 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1806,23 +1806,22 @@ static const struct panel_desc sharp_lq101k1ly04 = {
> .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA,
> };
>
> -static const struct drm_display_mode sharp_lq123p1jx31_mode = {
> - .clock = 252750,
> - .hdisplay = 2400,
> - .hsync_start = 2400 + 48,
> - .hsync_end = 2400 + 48 + 32,
> - .htotal = 2400 + 48 + 32 + 80,
> - .vdisplay = 1600,
> - .vsync_start = 1600 + 3,
> - .vsync_end = 1600 + 3 + 10,
> - .vtotal = 1600 + 3 + 10 + 33,
> - .vrefresh = 60,
> - .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> +static const struct display_timing sharp_lq123p1jx31_timing = {
> + .pixelclock = { 25275, 25275, 266604720 },
> + .hactive = { 2400, 2400, 2400 },
> + .hfront_porch = { 48, 48, 48 },
> + .hback_porch = { 80, 80, 84 },
> + .hsync_len = { 32, 32, 32 },
> + .vactive = { 1600, 1600, 1600 },
> + .vfront_porch = { 3, 3, 3 },
> + .vback_porch = { 33, 33, 120 },
> + .vsync_len = { 10, 10, 10 },
> + .flags = DISPLAY_FLAGS_VSYNC_LOW | DISPLAY_FLAGS_HSYNC_LOW,
> };
>
> static const struct panel_desc sharp_lq123p1jx31 = {
> - .modes = &sharp_lq123p1jx31_mode,
> - .num_modes = 1,
> + .timings = &sharp_lq123p1jx31_timing,
> + .num_timings = 1,
> .bpc = 8,
> .size = {
> .width = 259,
> --
Tested on top of linux-next on a Samsung Chromebook Plus.
Tested-by: Enric Balletbo i Serra
> 2.16.0.rc1.238.g530d649a79-goog
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105163] *-symbol-checks hard-code the nm executable
https://bugs.freedesktop.org/show_bug.cgi?id=105163 Bug ID: 105163 Summary: *-symbol-checks hard-code the nm executable Product: DRI Version: XOrg git Hardware: All OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: libdrm Assignee: dri-devel@lists.freedesktop.org Reporter: timo.g...@gmail.com Created attachment 137439 --> https://bugs.freedesktop.org/attachment.cgi?id=137439&action=edit libdrm-symbol-check-Don-t-hard-code-nm-executable.patch Currently the *-symbol-checks hard-code the nm executable which may result in build failures when using a prefixed toolchain, e.g. /usr/x86_64-pc-linux-gnu/bin/x86_64-pc-linux-gnu-nm. There was a patch suggested in 2016 and sent to the mailing list but it hasn't been fixed yet: https://lists.freedesktop.org/archives/dri-devel/2016-October/11.html Attached is the patch in question for the sake of completeness. It would be nice to get this fixed upstream. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel
2018-02-08 18:48 GMT+01:00 Sean Paul :
> This patch adds an override mode for kevin devices. The mode increases
> both back porches to allow a pixel clock of 2kHz as opposed to the
> 'typical' value of 252750kHz. This is needed to avoid interference with
> the touch digitizer on these laptops.
>
> Changes in v2:
> - Wrap the timing in display-timings node to match binding (Rob/Thierry)
> Changes in v3:
> - Unwrap the timing from display-timings and rename panel-timing (Rob)
>
> Cc: Doug Anderson
> Cc: Eric Anholt
> Cc: Heiko Stuebner
> Cc: Jeffy Chen
> Cc: Rob Herring
> Cc: Stéphane Marchesin
> Cc: Thierry Reding
> Cc: devicet...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-rockc...@lists.infradead.org
> Signed-off-by: Sean Paul
> ---
> arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> index 191a6bcb1704..658411ce37ea 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> @@ -98,6 +98,20 @@
> backlight = <&backlight>;
> power-supply = <&pp3300_disp>;
>
> + panel-timing {
> + clock-frequency = <266604720>;
> + hactive = <2400>;
> + hfront-porch = <48>;
> + hback-porch = <84>;
> + hsync-len = <32>;
> + hsync-active = <0>;
> + vactive = <1600>;
> + vfront-porch = <3>;
> + vback-porch = <120>;
> + vsync-len = <10>;
> + vsync-active = <0>;
> + };
> +
> ports {
> panel_in_edp: endpoint {
> remote-endpoint = <&edp_out_panel>;
> --
Tested on top of linux-next on a Samsung Chromebook Plus.
Tested-by: Enric Balletbo i Serra
> 2.16.0.rc1.238.g530d649a79-goog
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] drm/atomic: Abuse legacy cursor update flag for legacy gamma update too
On Tue, Feb 13, 2018 at 10:12:01AM +0100, Maarten Lankhorst wrote: > Programs like redshift set the legacy gamma for X.org every 5 seconds. > Because atomic commits wait for vblank completion, we get a frame drop > every 5 seconds because of the legacy gamma update. > > Work around this by setting the legacy_cursor_update flag, to force > legacy gamma updates not to be synced against vblank. > > Reported-by: Tholin #intel-gfx > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/drm_atomic_helper.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index ae3cbfe9e01c..f37ab26ef4d2 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -3806,6 +3806,9 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc > *crtc, > if (!state) > return -ENOMEM; > > + /* Don't wait for vblank after updating gamma. */ > + state->legacy_cursor_update = true; The legacy_cursor_update hack is a fairly serious hack, and I'd kinda want to avoid inflicting it on everyone. The cursor hack is a lot less minimal (because few drivers have cursors, and we'd have the async plane update stuff from Gustavo at least). Do we really need this? Making sure that no one can trample over async gamma table updates, especially when we eventually go to a vblank worker for them, sounds like serious amounts of pain. I'd prefer we teach -modesetting to just push the gamma update with the next frame as an atomic commit, or in the blockhandler (to avoid delaying it). -Daniel > + > blob = drm_property_create_blob(dev, > sizeof(struct drm_color_lut) * size, > NULL); > -- > 2.16.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Use idr_init_base(1) when using id==0 for invalid
Quoting Christian König (2018-02-13 13:48:24)
> Am 12.02.2018 um 18:14 schrieb Ville Syrjälä:
> > On Mon, Feb 12, 2018 at 02:55:33PM +, Chris Wilson wrote:
> >> Use the new idr_init_base() function to create an IDR that knows id==0
> >> is never allocated as it maps to an invalid identifier. By knowing that
> >> id==0 is invalid, the IDR can start from id=1 instead avoiding the issue
> >> of having to start each lookup from the zeroth leaf as id==0 is always
> >> unused (and thus the tree-of-bitmaps indicate that is the first
> >> available).
> >>
> >> References: 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
> >> Signed-off-by: Chris Wilson
> >> Cc: Daniel Vetter
> >> Cc: Christian Konig
> >> Cc: Dave Airlie
> > Yep, looks like all of these pass start==1 to idr_alloc().
> > Reviewed-by: Ville Syrjälä
>
> Acked-by: Christian König as well.
>
> Probably going to do this for the command submission context handles in
> amdgpu as well.
Thanks for the review, pushed to drm-misc-next.
As noted, there are probably quite a few more idr's that we can move
over to idr_init_base(1), e.g. the KMS objects. If someone feels like a
small task?
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Handle unexpected holes in color-eviction
Quoting Chris Wilson (2018-02-19 13:35:43)
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -836,9 +836,24 @@ struct drm_mm_node *drm_mm_scan_color_evict(struct
> drm_mm_scan *scan)
> if (!mm->color_adjust)
> return NULL;
>
> - hole = list_first_entry(&mm->hole_stack, typeof(*hole), hole_stack);
> - hole_start = __drm_mm_hole_node_start(hole);
> - hole_end = hole_start + hole->hole_size;
> + /*
> +* The hole found during scanning should ideally be the first element
> +* in the hole_stack list, but due to side-effects in the driver it
> +* may not be.
> +*/
> + list_for_each_entry(hole, &mm->hole_stack, hole_stack) {
> + hole_start = __drm_mm_hole_node_start(hole);
> + hole_end = hole_start + hole->hole_size;
> +
> + if (hole_start <= scan->hit_start &&
> + hole_end >= scan->hit_end)
How about some likely() here?
> + break;
> + }
> +
> + /* We should only be called after we found the hole previously */
> + DRM_MM_BUG_ON(&hole->hole_stack == &mm->hole_stack);
> + if (unlikely(&hole->hole_stack == &mm->hole_stack))
Would be more readable as:
if (...) {
DRM_MM_BUG()
}
Reviewed-by: Joonas Lahtinen
Regards, Joonas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Handle unexpected holes in color-eviction
Quoting Joonas Lahtinen (2018-02-19 14:40:32)
> Quoting Chris Wilson (2018-02-19 13:35:43)
> > +++ b/drivers/gpu/drm/drm_mm.c
> > @@ -836,9 +836,24 @@ struct drm_mm_node *drm_mm_scan_color_evict(struct
> > drm_mm_scan *scan)
> > if (!mm->color_adjust)
> > return NULL;
> >
> > - hole = list_first_entry(&mm->hole_stack, typeof(*hole), hole_stack);
> > - hole_start = __drm_mm_hole_node_start(hole);
> > - hole_end = hole_start + hole->hole_size;
> > + /*
> > +* The hole found during scanning should ideally be the first
> > element
> > +* in the hole_stack list, but due to side-effects in the driver it
> > +* may not be.
> > +*/
> > + list_for_each_entry(hole, &mm->hole_stack, hole_stack) {
> > + hole_start = __drm_mm_hole_node_start(hole);
> > + hole_end = hole_start + hole->hole_size;
> > +
> > + if (hole_start <= scan->hit_start &&
> > + hole_end >= scan->hit_end)
>
> How about some likely() here?
I felt that at the point where we admit we need a search, likely() went
out of the window. Given that I think we'll need 2 or 3 iterations at
most, that probably means in practice this is around the 50% mark.
Claiming that it's likely() may be misleading to the reader.
> > + break;
> > + }
> > +
> > + /* We should only be called after we found the hole previously */
> > + DRM_MM_BUG_ON(&hole->hole_stack == &mm->hole_stack);
> > + if (unlikely(&hole->hole_stack == &mm->hole_stack))
>
> Would be more readable as:
>
> if (...) {
> DRM_MM_BUG()
> }
Maybe DRM_MM_WARN_ON(), both hypothetical functions :)
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PULL] Add Display Support for Qualcomm SDM845
On Tue, Feb 13, 2018 at 03:00:09PM -0500, Rob Clark wrote: > On Tue, Feb 13, 2018 at 2:18 PM, Sean Paul wrote: > > Hi dri-devel, > > Qualcomm has been working for the past few weeks on forward porting their > > downstream drm driver from 4.14 to mainline. Please consider this PR as a > > request for review, rather than an attempt at mainlining the code as it > > currently stands. The goal is get this driver in shape over the next coming > > months. > > > > In the meantime, I'll be hosting a tree here [1] to stage the fixes. Patches > > will be posted and reviewed on linux-arm-...@vger.kernel.org. Once things > > look > > good, I'll send another pull 4realz. > > > > To get the ball rolling, I've done some review on the new connector code, my > > comments are below. > > > > Thanks in advance for your constructive feedback :) > > > > Sean > > > > [1]- git://people.freedesktop.org/~seanpaul/dpu-staging > > > > Review feedback: > > > > just so others aren't confused, s/sde/dpu/g in the list below (we did > our DAL->DC rename before sending first RFC :-P) > > > - Solve the splash screen handling (or remove it) > > - Simplify devicetree binding (remove register offsets) > > feedback from reviewing sde_connector.c: > > - Rationalize backlight implementation in sde_connector (display_count > > static) > > - Sort out the dsi event passing between dsi/encoder/connector (move to > > encoder) > > - include/uapi/drm/msm_drm_pp.h needs opensource userspace (or removal) > > - connector->state access violations reading/writing mode_info > > - s/sde_rect/drm_rect/ > > - sde_kms_info usage needs to be replaced with formal data structures (not > > stringified keypairs) > > - sde_connector_ops needs to be trimmed, duplicates connector helpers, info > > hooks circumvent state, and other hooks should be stored in state or > > prepopulated (get_dst_format) > > - sde_connector_get_dpms unused > > - esd status check should migrate to encoder from connector > > - backlight should be handled in panel drivers, not in the generic > > connector/dsi > > encoder > > - sde_connector_helper_bridge_disable is called from encoder and calls back > > into > > set_power encoder function. if backlight, and esd status are removed, > > pre_kickoff can probably go away > > - sde_connector_clk_ctrl is another example of encoder->connector->encoder > > call > > - RETIRE_FENCE connector property should be removed, opting for the native > > atomic fences > > - ROI (regions of interest) should be expressed per-plane instead of > > connector. > > there is work ongoing to support dirty_rects per-plane by Deepak Singh > > Rawat > >and Lukasz Spintzyk > > - Uma Shankar has proposed HDR source metadata > > properties on the list, we should pivot to those instead of hand-rolling > > them > > in the sde driver > > - Convert HDCP implementation to upstream Content Protection property > > - Merge dsi and dsi_staging into one driver > > - Writeback connector has been proposed by ARM (Liviu Dudau and Brian > > Starkey), > > we should work with their proposal instead of rolling OUT_FB ourselves > > - sde_connector_set_property should be replaced with atomic helper > > - dsi hotplug can probably be punted to the panel driver > > - dpms should switch to enable/disable (or at least use the atomic helpers) > > - dsi mode handling should also defer to the panel driver > > - SDE_WB_CONFIG ioctl should be removed in favor of the existing ioctl to > > add > > user-defined modes > > - dp implementation should use the existing dp helpers wherever possible > > - lots of duplicated structures in dsi_defs.h that can be replaced with > > existing > > drm structs > > - mode_valid should be split up and implemented directly in > > connector/encoder as > > appropriate > > - sde_connector->aspace seems like it's unused? > > > > To add to that, a few things I've noticed (but I've mostly only gotten > as far as the front-end of the pipeline, ie. dpu_plane): > > - It looks like, as was the case with mdp4/mdp5 (and really most other hw) >there are still unequal capabilities for planes (ie. some can do YUV, >scaling, etc). > >But drm-hwc (or weston) isn't going to know about that, so I think we'll >need to add support for dynamically assigning dpu_plane::pipe, similar >to what mdp5 does w/ plane<->hwpipe. (Which I actually added specifically >for drm-hwc ;-)) > > - I *think* we also need the same trick for combining mixers under one CRTC >for 4k modes too? Sounds like this should reuse the mdp5 framework in msm, at least in parts? Reinventing the same virtualization will get boring :-) > - in dpu_plane, and perhaps elsewhere, userspace pointers for things like CSC >and scaling coefficients need to be annotated w/ __user. (Except the > should >probably be blob properties instead.. and we probably need to strip out the >custom properties and re-introduce them separately wit
Re: [Intel-gfx] [Nouveau] [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers
On Mon, Feb 19, 2018 at 03:05:53PM +0100, Daniel Vetter wrote: > On Mon, Feb 19, 2018 at 12:58:17PM +0100, Lukas Wunner wrote: > > On Mon, Feb 19, 2018 at 12:34:43PM +0100, Daniel Vetter wrote: > > > Well, userspace expects hotplug events, even when we runtime suspend > > > stuff. Hence waking shit up with polling. Imo ignoring hotplug events is a > > > pretty serious policy decision which is ok in the context of > > > vga_switcheroo, but not really as an automatic thing. E.g. usb also wakes > > > up if you plug something in, even with all the runtime pm stuff enabled. > > > Same for sata and everything else. > > > > On the MacBook Pro, the HPD pin of external DP and HDMI ports goes into > > the gmux controller, which sends an interrupt on hotplug even if the GPU > > is powered down. > > > > Apparently Optimus has a similar functionality, cf. 3a6536c51d5d. > > Yeah, for vga_switcheroo and gmux setups shutting down polling explicitly > makes sense. I think ideally we'd stop polling in the gmux handler somehow > (maybe by dropping the relevant DRM_CONNECTOR_POLL flags, or outright > stopping it all). But not when runtime suspending the entire gpu (e.g. > idle system that shuts down the screen and everything, before it decides > a few minutes later to do a full system suspend). nouveau, radeon and amdgpu currently use runtime PM *only* on hybrid graphics laptops. Should the drivers later be extended to also use runtime PM in other scenarios (desktop machines, eGPUs), they can easily detect whether to disable polling on runtime suspend by calling apple_gmux_present() on Macs or the equivalent for Optimus/ATPX. Thanks, Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Use idr_init_base(1) when using id==0 for invalid
On Mon, Feb 19, 2018 at 02:35:38PM +, Chris Wilson wrote:
> Quoting Christian König (2018-02-13 13:48:24)
> > Am 12.02.2018 um 18:14 schrieb Ville Syrjälä:
> > > On Mon, Feb 12, 2018 at 02:55:33PM +, Chris Wilson wrote:
> > >> Use the new idr_init_base() function to create an IDR that knows id==0
> > >> is never allocated as it maps to an invalid identifier. By knowing that
> > >> id==0 is invalid, the IDR can start from id=1 instead avoiding the issue
> > >> of having to start each lookup from the zeroth leaf as id==0 is always
> > >> unused (and thus the tree-of-bitmaps indicate that is the first
> > >> available).
> > >>
> > >> References: 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
> > >> Signed-off-by: Chris Wilson
> > >> Cc: Daniel Vetter
> > >> Cc: Christian Konig
> > >> Cc: Dave Airlie
> > > Yep, looks like all of these pass start==1 to idr_alloc().
> > > Reviewed-by: Ville Syrjälä
> >
> > Acked-by: Christian König as well.
> >
> > Probably going to do this for the command submission context handles in
> > amdgpu as well.
>
> Thanks for the review, pushed to drm-misc-next.
>
> As noted, there are probably quite a few more idr's that we can move
> over to idr_init_base(1), e.g. the KMS objects. If someone feels like a
> small task?
I'll type a todo.rst patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [Nouveau] [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers
On Mon, Feb 19, 2018 at 03:47:42PM +0100, Lukas Wunner wrote: > On Mon, Feb 19, 2018 at 03:05:53PM +0100, Daniel Vetter wrote: > > On Mon, Feb 19, 2018 at 12:58:17PM +0100, Lukas Wunner wrote: > > > On Mon, Feb 19, 2018 at 12:34:43PM +0100, Daniel Vetter wrote: > > > > Well, userspace expects hotplug events, even when we runtime suspend > > > > stuff. Hence waking shit up with polling. Imo ignoring hotplug events > > > > is a > > > > pretty serious policy decision which is ok in the context of > > > > vga_switcheroo, but not really as an automatic thing. E.g. usb also > > > > wakes > > > > up if you plug something in, even with all the runtime pm stuff enabled. > > > > Same for sata and everything else. > > > > > > On the MacBook Pro, the HPD pin of external DP and HDMI ports goes into > > > the gmux controller, which sends an interrupt on hotplug even if the GPU > > > is powered down. > > > > > > Apparently Optimus has a similar functionality, cf. 3a6536c51d5d. > > > > Yeah, for vga_switcheroo and gmux setups shutting down polling explicitly > > makes sense. I think ideally we'd stop polling in the gmux handler somehow > > (maybe by dropping the relevant DRM_CONNECTOR_POLL flags, or outright > > stopping it all). But not when runtime suspending the entire gpu (e.g. > > idle system that shuts down the screen and everything, before it decides > > a few minutes later to do a full system suspend). > > nouveau, radeon and amdgpu currently use runtime PM *only* on hybrid > graphics laptops. > > Should the drivers later be extended to also use runtime PM in other > scenarios (desktop machines, eGPUs), they can easily detect whether > to disable polling on runtime suspend by calling apple_gmux_present() > on Macs or the equivalent for Optimus/ATPX. Ah, then I think the current solution is ok (if not entirely clean imo, but that can be fixed up whenever it hurts). Implementing runtime pm for other cases is up to the driver authors really (probably more pressing when the gpu is on the same SoC). -Daniel > > Thanks, > > Lukas > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] Forward Error Correction is supported on DP 1.4. This patch adds corresponding DPCD register definitions.
On Tue, Feb 13, 2018 at 11:55:35PM -0800, Anusha Srivatsa wrote: > v2: Add dri-devel mailing list to the CC list(Jani) > > v3: Change names, add missing masks (Manasi) > > v4: Add missing shifts to mask (Manasi) > > v5: Arrange the definitions in ascending order > of the address (Jani) > > Cc: dri-devel@lists.freedesktop.org > Cc: Ville Syrjala > Cc: Jani Nikula > Cc: Manasi Navare > Signed-off-by: Anusha Srivatsa Subject needs a drm/dp: prefix. Just a drive-by comment. -Daniel > --- > include/drm/drm_dp_helper.h | 30 ++ > 1 file changed, 30 insertions(+) > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index c239e6e..a19d6fb 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -329,6 +329,13 @@ > # define DP_DS_12BPC 2 > # define DP_DS_16BPC 3 > > +/* DP Forward error Correction Registers */ > +#define DP_FEC_CAPABILITY(0x090) > +# define DP_FEC_CAPABLE (1 << 0) > +# define DP_FEC_UNCORR_BLK_ERROR_COUNT_CAP (1 << 1) > +# define DP_FEC_CORR_BLK_ERROR_COUNT_CAP(1 << 2) > +# define DP_FEC_BIT_ERROR_COUNT_CAP (1 << 3) > + > /* link configuration */ > #define DP_LINK_BW_SET 0x100 > # define DP_LINK_RATE_TABLE 0x00/* eDP 1.4 */ > @@ -445,6 +452,18 @@ > #define DP_UPSTREAM_DEVICE_DP_PWR_NEED 0x118 /* 1.2 */ > # define DP_PWR_NOT_NEEDED (1 << 0) > > +#define DP_FEC_CONFIGURATION 0x120 > +# define DP_FEC_READY(1 << 0) > +# define DP_FEC_ERR_COUNT_DIS(0 << 1) > +# define DP_FEC_UNCORR_BLK_ERROR_COUNT (1 << 1) > +# define DP_FEC_CORR_BLK_ERROR_COUNT (2 << 1) > +# define DP_FEC_BIT_ERROR_COUNT (3 << 1) > +# define DP_FEC_ERR_COUNT_SEL_MASK (0xff << 4) > +# define DP_FEC_LANE_0_SELECT(0 << 4) > +# define DP_FEC_LANE_1_SELECT(1 << 4) > +# define DP_FEC_LANE_2_SELECT(2 << 4) > +# define DP_FEC_LANE_3_SELECT(3 << 4) > + > #define DP_AUX_FRAME_SYNC_VALUE 0x15c /* eDP 1.4 */ > # define DP_AUX_FRAME_SYNC_VALID (1 << 0) > > @@ -620,6 +639,17 @@ > #define DP_TEST_SINK 0x270 > # define DP_TEST_SINK_START (1 << 0) > > +#define DP_FEC_STATUS0x280 > +# define DP_FEC_DECODE_EN_DETECTED (1 << 0) > +# define DP_FEC_DECODE_DIS_DETECTED (1 << 1) > + > +#define DP_FEC_ERROR_COUNT_LSB 0x0281 > + > +#define DP_FEC_ERROR_COUNT_MSB 0x0282 > +# define DP_FEC_ERROR_COUNT_MASK 0x7F > +# define DP_FEC_ERR_COUNT_SHIFT 8 > +# define DP_FEC_ERR_COUNT_VALID (1 << 7) > + > #define DP_PAYLOAD_TABLE_UPDATE_STATUS 0x2c0 /* 1.2 MST */ > # define DP_PAYLOAD_TABLE_UPDATED (1 << 0) > # define DP_PAYLOAD_ACT_HANDLED (1 << 1) > -- > 2.7.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/todo: Add idr_init_base todo
Suggested-by: Chris Wilson Cc: Chris Wilson Acked-by: Chris Wilson Signed-off-by: Daniel Vetter --- Documentation/gpu/todo.rst | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 1e593370f64f..1a0a413eeced 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -212,6 +212,16 @@ probably use drm_fb_helper_fbdev_teardown(). Contact: Maintainer of the driver you plan to convert +idr_init_base() +--- + +DRM core&drivers uses a lot of idr (integer lookup directories) for mapping +userspace IDs to internal objects, and in most places ID=0 means NULL and hence +is never used. Switching to idr_init_base() for these would make the idr more +efficient. + +Contact: Daniel Vetter + Core refactorings = -- 2.15.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/6] dt-bindings: Clarify timing subnode use as panel-timing
On Thu, Feb 08, 2018 at 12:48:48PM -0500, Sean Paul wrote: > Add a note in the documentation explaining when it's appropriate to use > the display-timings subnode on its own, as well as the preferred name to > use (panel-timing). panel-timing seems rather redundant, but I guess that's the de facto standard, so: Acked-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/8] drm: Add COLOR_ENCODING and COLOR_RANGE plane properties
On Wed, Feb 14, 2018 at 09:23:19PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > Here's a refresh of Jyri's COLOR_ENCODING and COLOR_RANGE properties, > and the i915 implementation I did on top. I tossed in a few core > updates as well: plane state dump, and the BT.2020 constant luminance > variant. > > Apparently nouveau is already exposing a "iturbt_709" bool property > which allows one to choose between BT.601 and BT.709 encodings, but > given that we want at least BT.2020 in addition I don't think that > property is good enough. Trying to implement it, and somehow extend > it beyond BT.601 vs. BT.709 seems like wasted effort. Hence I think > we should just ignore it and move on. Yeah, with those legacy props in various kernels I think it's best if we ignore them, and leave the burden of figuring out compatibility interactions to the driver that went their own way. I'll ping Illia as fyi on irc at least. -Daniel > > My userspace implementation in the form of intel ddx > XV_COLORSPACE attribute: > https://patchwork.freedesktop.org/patch/204696/ > > Cc: Harry Wentland > Cc: Daniel Vetter > Cc: Daniel Stone > Cc: Russell King - ARM Linux > Cc: Ilia Mirkin > Cc: Hans Verkuil > Cc: Uma Shankar > Cc: Shashank Sharma > > Jyri Sarha (1): > drm: Add optional COLOR_ENCODING and COLOR_RANGE properties to > drm_plane > > Ville Syrjälä (7): > drm: Add BT.2020 constant luminance enum value for the COLOR_ENCODING > property > drm/atomic: Include color encoding/range in plane state dump > drm/i915: Correctly handle limited range YCbCr data on VLV/CHV > drm/i915: Fix plane YCbCr->RGB conversion for GLK > drm/i915: Add support for the YCbCr COLOR_ENCODING property > drm/i915: Change the COLOR_ENCODING prop default value to BT.709 > drm/i915: Add support for the YCbCr COLOR_RANGE property > > drivers/gpu/drm/drm_atomic.c | 12 > drivers/gpu/drm/drm_color_mgmt.c | 108 > drivers/gpu/drm/drm_crtc_internal.h | 2 + > drivers/gpu/drm/i915/i915_reg.h | 24 ++- > drivers/gpu/drm/i915/intel_display.c | 25 +++ > drivers/gpu/drm/i915/intel_drv.h | 2 + > drivers/gpu/drm/i915/intel_sprite.c | 134 > --- > include/drm/drm_color_mgmt.h | 20 ++ > include/drm/drm_plane.h | 8 +++ > 9 files changed, 309 insertions(+), 26 deletions(-) > > -- > 2.13.6 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/8] drm: Add optional COLOR_ENCODING and COLOR_RANGE properties to drm_plane
On Wed, Feb 14, 2018 at 09:23:20PM +0200, Ville Syrjala wrote:
> From: Jyri Sarha
>
> Add a standard optional properties to support different non RGB color
> encodings in DRM planes. COLOR_ENCODING select the supported non RGB
> color encoding, for instance ITU-R BT.709 YCbCr. COLOR_RANGE selects
> the value ranges within the selected color encoding. The properties
> are stored to drm_plane object to allow different set of supported
> encoding for different planes on the device.
>
> Cc: Harry Wentland
> Cc: Daniel Vetter
> Cc: Daniel Stone
> Cc: Russell King - ARM Linux
> Cc: Ilia Mirkin
> Cc: Hans Verkuil
> Cc: Uma Shankar
> Cc: Shashank Sharma
> Reviewed-by: Ville Syrjälä
> Signed-off-by: Jyri Sarha
> ---
> drivers/gpu/drm/drm_atomic.c | 8
> drivers/gpu/drm/drm_color_mgmt.c | 91
>
> include/drm/drm_color_mgmt.h | 19 +
> include/drm/drm_plane.h | 8
> 4 files changed, 126 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 46733d534587..452a0b0bafbc 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -759,6 +759,10 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> state->rotation = val;
> } else if (property == plane->zpos_property) {
> state->zpos = val;
> + } else if (property == plane->color_encoding_property) {
> + state->color_encoding = val;
> + } else if (property == plane->color_range_property) {
> + state->color_range = val;
> } else if (plane->funcs->atomic_set_property) {
> return plane->funcs->atomic_set_property(plane, state,
> property, val);
> @@ -818,6 +822,10 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->rotation;
> } else if (property == plane->zpos_property) {
> *val = state->zpos;
> + } else if (property == plane->color_encoding_property) {
> + *val = state->color_encoding;
> + } else if (property == plane->color_range_property) {
> + *val = state->color_range;
> } else if (plane->funcs->atomic_get_property) {
> return plane->funcs->atomic_get_property(plane, state,
> property, val);
> } else {
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
> b/drivers/gpu/drm/drm_color_mgmt.c
> index 0d002b045bd2..a84fc861e406 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -88,6 +88,19 @@
> * drm_mode_crtc_set_gamma_size(). Drivers which support both should use
> * drm_atomic_helper_legacy_gamma_set() to alias the legacy gamma ramp with
> the
> * "GAMMA_LUT" property above.
> + *
> + * Support for different non RGB color encodings is controlled through
> + * &drm_plane specific COLOR_ENCODING and COLOR_RANGE properties:
> + *
> + * "COLOR_ENCODING"
> + * Optional plane enum property to support different non RGB
> + * color encodings. The driver can provide a subset of standard
> + * enum values supported by the DRM plane.
Please also mention the function to setup/register them, like we try to do
with the other optional properties.
> + *
> + * "COLOR_RANGE"
> + * Optional plane enum property to support different non RGB
> + * color parameter ranges. The driver can provide a subset of
> + * standard enum values supported by the DRM plane.
> */
>
> /**
> @@ -339,3 +352,81 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev,
> drm_modeset_unlock(&crtc->mutex);
> return ret;
> }
> +
> +static const char * const color_encoding_name[] = {
> + [DRM_COLOR_YCBCR_BT601] = "ITU-R BT.601 YCbCr",
> + [DRM_COLOR_YCBCR_BT709] = "ITU-R BT.709 YCbCr",
> + [DRM_COLOR_YCBCR_BT2020] = "ITU-R BT.2020 YCbCr",
> +};
> +
> +static const char * const color_range_name[] = {
> + [DRM_COLOR_YCBCR_FULL_RANGE] = "YCbCr full range",
> + [DRM_COLOR_YCBCR_LIMITED_RANGE] = "YCbCr limited range",
> +};
> +
> +/**
> + * drm_plane_create_color_properties - color encoding related plane
> properties
> + * @supported_encodings: bitfield indicating supported color encodings
> + * @supported_ranges: bitfileld indicating supported color ranges
> + * @default_encoding: default color encoding
> + * @default_range: default color range
> + *
> + * Create and attach plane specific COLOR_ENCODING and COLOR_RANGE
> + * properties to to the drm_plane object. The supported encodings and
> + * ranges should be provided in supported_encodings and
> + * supported_ranges bitmasks. Each bit set in the bitmask indicates
> + * the its number as enum value being supported.
> + */
> +int drm_plane_create_color_properties(struct drm_plane *plane,
> + u32 supported_encodings,
> + u32 supported_ranges,
Is 0 in the above two supported_ masks a valid value? I
Re: [PATCH 3/8] drm/atomic: Include color encoding/range in plane state dump
On Wed, Feb 14, 2018 at 09:23:22PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Include color_enconding and color_range in the plane state dump.
>
> Cc: Harry Wentland
> Cc: Daniel Vetter
> Cc: Daniel Stone
> Cc: Russell King - ARM Linux
> Cc: Ilia Mirkin
> Cc: Hans Verkuil
> Cc: Uma Shankar
> Cc: Shashank Sharma
> Cc: Jyri Sarha
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/drm_atomic.c| 4
> drivers/gpu/drm/drm_color_mgmt.c| 16
> drivers/gpu/drm/drm_crtc_internal.h | 2 ++
> 3 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 452a0b0bafbc..9552052ed31a 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -952,6 +952,10 @@ static void drm_atomic_plane_print_state(struct
> drm_printer *p,
> drm_printf(p, "\tcrtc-pos=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&dest));
> drm_printf(p, "\tsrc-pos=" DRM_RECT_FP_FMT "\n", DRM_RECT_FP_ARG(&src));
> drm_printf(p, "\trotation=%x\n", state->rotation);
> + drm_printf(p, "\tcolor-encoding=%s\n",
> +drm_get_color_encoding_name(state->color_encoding));
> + drm_printf(p, "\tcolor-range=%s\n",
> +drm_get_color_range_name(state->color_range));
>
> if (plane->funcs->atomic_print_state)
> plane->funcs->atomic_print_state(p, state);
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
> b/drivers/gpu/drm/drm_color_mgmt.c
> index 061d342f9d96..06851d575f14 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -365,6 +365,22 @@ static const char * const color_range_name[] = {
> [DRM_COLOR_YCBCR_LIMITED_RANGE] = "YCbCr limited range",
> };
>
kerneldoc would be neat here ...
-Daniel
> +const char *drm_get_color_encoding_name(enum drm_color_encoding encoding)
> +{
> + if (WARN_ON(encoding >= ARRAY_SIZE(color_encoding_name)))
> + return "unknown";
> +
> + return color_encoding_name[encoding];
> +}
> +
> +const char *drm_get_color_range_name(enum drm_color_range range)
> +{
> + if (WARN_ON(range >= ARRAY_SIZE(color_range_name)))
> + return "unknown";
> +
> + return color_range_name[range];
> +}
> +
> /**
> * drm_plane_create_color_properties - color encoding related plane
> properties
> * @supported_encodings: bitfield indicating supported color encodings
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h
> b/drivers/gpu/drm/drm_crtc_internal.h
> index af00f42ba269..8ca2ffef6231 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -71,6 +71,8 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv);
>
> /* drm_color_mgmt.c */
> +const char *drm_get_color_encoding_name(enum drm_color_encoding encoding);
> +const char *drm_get_color_range_name(enum drm_color_range range);
>
> /* IOCTLs */
> int drm_mode_gamma_get_ioctl(struct drm_device *dev,
> --
> 2.13.6
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/8] drm: Add COLOR_ENCODING and COLOR_RANGE plane properties
On Wed, Feb 14, 2018 at 09:23:19PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > Here's a refresh of Jyri's COLOR_ENCODING and COLOR_RANGE properties, > and the i915 implementation I did on top. I tossed in a few core > updates as well: plane state dump, and the BT.2020 constant luminance > variant. > > Apparently nouveau is already exposing a "iturbt_709" bool property > which allows one to choose between BT.601 and BT.709 encodings, but > given that we want at least BT.2020 in addition I don't think that > property is good enough. Trying to implement it, and somehow extend > it beyond BT.601 vs. BT.709 seems like wasted effort. Hence I think > we should just ignore it and move on. > > My userspace implementation in the form of intel ddx > XV_COLORSPACE attribute: > https://patchwork.freedesktop.org/patch/204696/ Any plans to get this going for -modesetting too? Distro's kinda stopped shipping -intel. -Daniel > > Cc: Harry Wentland > Cc: Daniel Vetter > Cc: Daniel Stone > Cc: Russell King - ARM Linux > Cc: Ilia Mirkin > Cc: Hans Verkuil > Cc: Uma Shankar > Cc: Shashank Sharma > > Jyri Sarha (1): > drm: Add optional COLOR_ENCODING and COLOR_RANGE properties to > drm_plane > > Ville Syrjälä (7): > drm: Add BT.2020 constant luminance enum value for the COLOR_ENCODING > property > drm/atomic: Include color encoding/range in plane state dump > drm/i915: Correctly handle limited range YCbCr data on VLV/CHV > drm/i915: Fix plane YCbCr->RGB conversion for GLK > drm/i915: Add support for the YCbCr COLOR_ENCODING property > drm/i915: Change the COLOR_ENCODING prop default value to BT.709 > drm/i915: Add support for the YCbCr COLOR_RANGE property > > drivers/gpu/drm/drm_atomic.c | 12 > drivers/gpu/drm/drm_color_mgmt.c | 108 > drivers/gpu/drm/drm_crtc_internal.h | 2 + > drivers/gpu/drm/i915/i915_reg.h | 24 ++- > drivers/gpu/drm/i915/intel_display.c | 25 +++ > drivers/gpu/drm/i915/intel_drv.h | 2 + > drivers/gpu/drm/i915/intel_sprite.c | 134 > --- > include/drm/drm_color_mgmt.h | 20 ++ > include/drm/drm_plane.h | 8 +++ > 9 files changed, 309 insertions(+), 26 deletions(-) > > -- > 2.13.6 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/6] dt-bindings: Add panel-timing subnode to simple-panel
On Thu, Feb 08, 2018 at 12:48:50PM -0500, Sean Paul wrote: > This patch adds a new subnode to simple-panel allowing us to override > the typical timing expressed in the panel's display_timing. > > Changes in v2: > - Split out the binding into a new patch (Rob) > - display-timings is a new section (Rob) > - Use the full display-timings subnode instead of picking the timing >out (Rob/Thierry) > Changes in v3: > - Go back to using the timing subnode directly, but rename to >panel-timing (Rob) > > Cc: Doug Anderson > Cc: Eric Anholt > Cc: Heiko Stuebner > Cc: Jeffy Chen > Cc: Rob Herring > Cc: Stéphane Marchesin > Cc: Thierry Reding > Cc: devicet...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: linux-rockc...@lists.infradead.org > Signed-off-by: Sean Paul > --- > .../bindings/display/panel/simple-panel.txt| 30 > ++ > 1 file changed, 30 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt > b/Documentation/devicetree/bindings/display/panel/simple-panel.txt > index 45a457ad38f0..7788b9ce160b 100644 > --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt > +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt > @@ -12,6 +12,24 @@ Optional properties: > - enable-gpios: GPIO pin to enable or disable the panel > - backlight: phandle of the backlight device attached to the panel > > +panel-timing subnode > + > + > +This optional subnode is for devices which require a mode differing from the > +panel's "typical" display timing as programmed in the simple-panel driver. > +Overriding the driver mode must only be done in the following scenario: > + - The restrictions motivating the override cannot be applied to the > platform's > + display driver (ie: it must be specific to the device not the platform) > + - The panel must not have a fixed mode attributed to it in the driver I think this is somewhat ambiguous. One could argue that panels only have a fixed mode. What you means if obviously the fixed DRM display mode, but that's not something that should go into the binding. I'd simply drop this requirement. > + - The panel must provide at list one display_timing range by which the > override > + mode can be validated against This is the important bit and kind of obsoletes the above anyway. Oh, and you probably meant "... provide at _least_ one..." > + - The override mode will use the 'typ' values from the panel-timings subnode The properties for display timings in DT don't list anything other than the 'typ' values, right? I'm not sure I understand this requirement. > + - You must provide all required properties for the panel-timing subnode Isn't this redundant? required properties are always required. Other than the unclarities, the above does reflect what we had discussed on IRC. I wonder, though, how much of this really belongs in the device tree bindings. The above are almost all restrictions that apply to the driver, and are very specific to Linux. Perhaps this is something that belongs in the GPU documentation rather than DT bindings. They are more like guidelines for what panel drivers should look like rather than what the DT should look like. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm: Add Plane Gamma properties
On Thu, Feb 15, 2018 at 12:32:54AM -0500, Daniele Castagna wrote:
> From: "uma.shankar at intel.com (Uma Shankar)"
>
> Add plane gamma as blob property and size as a
> range property.
>
> (am from https://patchwork.kernel.org/patch/9971325/)
>
> Change-Id: I606cd40c9748b136fc2bf4750bea1da285add62d
> Signed-off-by: Uma Shankar
Please also add kerneldoc for the properties itself, see
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties
That means we need a bunch of rewording, now that we're adding planes.
I also think a small diagram that shows the full pipeline (which probably
no hw ever implements) would be really good. See the various inlined dot
files we have in the KMS documentation already.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_atomic.c| 8
> drivers/gpu/drm/drm_atomic_helper.c | 4
> drivers/gpu/drm/drm_mode_config.c | 14 ++
> include/drm/drm_mode_config.h | 11 +++
> include/drm/drm_plane.h | 9 +
> 5 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index d4b8c6cc84128..f634f6450f415 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -778,6 +778,12 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> &replaced);
> state->color_mgmt_changed |= replaced;
> return ret;
> + } else if (property == config->plane_gamma_lut_property) {
> + ret = drm_atomic_replace_property_blob_from_id(dev,
> + &state->gamma_lut,
> + val, -1, &replaced);
> + state->color_mgmt_changed |= replaced;
> + return ret;
> } else {
> return -EINVAL;
> }
> @@ -841,6 +847,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> state->degamma_lut->base.id : 0;
> } else if (property == config->plane_ctm_property) {
> *val = (state->ctm) ? state->ctm->base.id : 0;
> + } else if (property == config->plane_gamma_lut_property) {
> + *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> } else {
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 17e137a529a0e..97dbb36153d14 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3495,6 +3495,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct
> drm_plane *plane,
> drm_property_reference_blob(state->degamma_lut);
> if (state->ctm)
> drm_property_reference_blob(state->ctm);
> + if (state->gamma_lut)
> + drm_property_reference_blob(state->gamma_lut);
> +
> state->color_mgmt_changed = false;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
> @@ -3543,6 +3546,7 @@ void __drm_atomic_helper_plane_destroy_state(struct
> drm_plane_state *state)
>
> drm_property_unreference_blob(state->degamma_lut);
> drm_property_unreference_blob(state->ctm);
> + drm_property_unreference_blob(state->gamma_lut);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index c8763977413e7..bc6f8e51c7464 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -368,6 +368,20 @@ static int drm_mode_create_standard_properties(struct
> drm_device *dev)
> return -ENOMEM;
> dev->mode_config.plane_ctm_property = prop;
>
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB,
> + "PLANE_GAMMA_LUT", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.plane_gamma_lut_property = prop;
> +
> + prop = drm_property_create_range(dev,
> + DRM_MODE_PROP_IMMUTABLE,
> + "PLANE_GAMMA_LUT_SIZE", 0, UINT_MAX);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.plane_gamma_lut_size_property = prop;
> +
> return 0;
> }
>
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index ad7235ced531b..3ca3eb3c98950 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -740,6 +740,17 @@ struct drm_mode_config {
>* degamma LUT.
>*/
> struct drm_property *plane_ctm_property;
> + /**
> + * @plane_gamma_lut_property: Optional Plane property to set the LUT
> + * used to convert the colors, after the CTM matrix, to the common
> + * gamma space chosen for blending.
> + */
> + struct drm_property *plane_gamma_lut_property;
> + /**
> + * @plane_gamma_lut_size_property: Optional Plane property
Re: [PATCH 02/10] drm: Add Plane Degamma properties
On Thu, Feb 15, 2018 at 12:32:52AM -0500, Daniele Castagna wrote:
> From: "uma.shankar at intel.com (Uma Shankar)"
>
> Add Plane Degamma as a blob property and plane
> degamma size as a range property.
>
> (am from https://patchwork.kernel.org/patch/10046515/)
>
> Change-Id: Iaead6f944a8b677227d1be11169f46178de533b1
> Signed-off-by: Uma Shankar
> ---
> drivers/gpu/drm/drm_atomic.c| 12
> drivers/gpu/drm/drm_atomic_helper.c | 7 +++
> drivers/gpu/drm/drm_mode_config.c | 14 ++
> include/drm/drm_mode_config.h | 10 ++
> include/drm/drm_plane.h | 10 ++
> 5 files changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index b76d49218cf1d..4a06ff2fd1a5e 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -717,6 +717,8 @@ static int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> {
> struct drm_device *dev = plane->dev;
> struct drm_mode_config *config = &dev->mode_config;
> + bool replaced = false;
> + int ret;
>
> if (property == config->prop_fb_id) {
> struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL,
> val);
> @@ -762,6 +764,12 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> } else if (plane->funcs->atomic_set_property) {
> return plane->funcs->atomic_set_property(plane, state,
> property, val);
> + } else if (property == config->plane_degamma_lut_property) {
> + ret = drm_atomic_replace_property_blob_from_id(dev,
> + &state->degamma_lut,
> + val, -1, &replaced);
> + state->color_mgmt_changed |= replaced;
> + return ret;
> } else {
> return -EINVAL;
> }
> @@ -820,6 +828,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->zpos;
> } else if (plane->funcs->atomic_get_property) {
> return plane->funcs->atomic_get_property(plane, state,
> property, val);
> + } else if (property == config->plane_degamma_lut_property) {
> + *val = (state->degamma_lut) ?
> + state->degamma_lut->base.id : 0;
> } else {
> return -EINVAL;
> }
> @@ -944,6 +955,7 @@ static void drm_atomic_plane_print_state(struct
> drm_printer *p,
> drm_printf(p, "\tcrtc-pos=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&dest));
> drm_printf(p, "\tsrc-pos=" DRM_RECT_FP_FMT "\n", DRM_RECT_FP_ARG(&src));
> drm_printf(p, "\trotation=%x\n", state->rotation);
> + drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
>
> if (plane->funcs->atomic_print_state)
> plane->funcs->atomic_print_state(p, state);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index ab4032167094c..d3eaf4d397681 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3488,7 +3488,12 @@ void __drm_atomic_helper_plane_duplicate_state(struct
> drm_plane *plane,
> drm_framebuffer_get(state->fb);
>
> state->fence = NULL;
> +
> state->commit = NULL;
> +
> + if (state->degamma_lut)
> + drm_property_reference_blob(state->degamma_lut);
> + state->color_mgmt_changed = false;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>
> @@ -3533,6 +3538,8 @@ void __drm_atomic_helper_plane_destroy_state(struct
> drm_plane_state *state)
>
> if (state->commit)
> drm_crtc_commit_put(state->commit);
> +
> + drm_property_unreference_blob(state->degamma_lut);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index e5c653357024d..7d8e74715b565 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -348,6 +348,20 @@ static int drm_mode_create_standard_properties(struct
> drm_device *dev)
> return -ENOMEM;
> dev->mode_config.modifiers_property = prop;
>
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB,
> + "PLANE_DEGAMMA_LUT", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.plane_degamma_lut_property = prop;
> +
> + prop = drm_property_create_range(dev,
> + DRM_MODE_PROP_IMMUTABLE,
> + "PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.plane_degamma_lut_size_property = prop;
> +
> return 0;
> }
>
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 2cb6f02df64ab..dcec93d062b4d 100644
> --- a/include/drm/drm_mode_config.h
> +++
Re: [PATCH 00/10] drm: Add plane color matrix on rockchip
On Thu, Feb 15, 2018 at 12:32:50AM -0500, Daniele Castagna wrote: > Hello, > this patch series adds a per plane color matrix property as well as an > implementation for rockchip. > > This patch series builds on and includes previous work done by uma.shankar: > https://lists.freedesktop.org/archives/dri-devel/2017-September/153347.html > > The first rockchip patch of this series sets a BT.601 color conversion > matrix for YUV formats and is included because it adds YUV2YUV registers that > the rest of the color matrix rockchip implementation depends upon. > > The next patches are the ones originally provided by uma.shankar, > rebased and containing an additional small fix. > > The final set of rockchip patches implement, and finally enable, the plane CTM > property on rockchip. > > These patches have been rebased on drm-next (80daf42f97b1a0503d029f3c1325), > Unfortunately I haven't been able to test them after rebasing because the > display doesn't come up when I push a kernel built from drm-next on a device. Link to the open source userspace making use of these new properties is missing. See https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements -Daniel > > Daniele Castagna (5): > drm/rockchip: YUV overlays BT.601 color conversion. > drm/rockchip: Add yuv2yuv registers to vop_lit > drm/rockchip: Add R2R registers > drm/rockchip: Implement drm plane->ctm property. > drm/rockchip: Enable 'PLANE_CTM' drm property. > > uma.shankar at intel.com (Uma Shankar) (5): > drm: Add Plane Degamma properties > drm: Add Plane CTM property > drm: Add Plane Gamma properties > drm: Define helper function for plane color enabling > drm: Define helper to set legacy gamma table size > > drivers/gpu/drm/drm_atomic.c| 30 +++ > drivers/gpu/drm/drm_atomic_helper.c | 14 > drivers/gpu/drm/drm_color_mgmt.c| 41 ++ > drivers/gpu/drm/drm_mode_config.c | 34 > drivers/gpu/drm/drm_plane.c | 48 +++ > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 90 - > drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 16 > drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 88 +++- > include/drm/drm_color_mgmt.h| 8 ++ > include/drm/drm_mode_config.h | 27 +++ > include/drm/drm_plane.h | 31 +++ > 11 files changed, 424 insertions(+), 3 deletions(-) > > -- > 2.16.1.291.g4437f3f132-goog > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/meson: fix vsync buffer update
On Thu, Feb 15, 2018 at 11:19:36AM +0100, Neil Armstrong wrote:
> The plane buffer address/stride/height was incorrectly updated in the
> plane_atomic_update operation instead of the vsync irq.
> This patch delays this operation in the vsync irq along with the
> other plane delayed setup.
>
> This issue was masked using legacy framebuffer and X11 modesetting, but
> is clearly visible using gbm rendering when buffer is submitted late after
> vblank, like using software decoding and OpenGL rendering in Kodi.
> With this patch, tearing and other artifacts disappears completely.
You mean the frontbuffer rendering nature of X11 with all it's tearing
made the issue not noticeable? I'm also not clear what you mean with
legacy framebuffer ...
Either way looks reasonable:
Acked-by: Daniel Vetter
> Cc: Michal Lazo
> Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
> Signed-off-by: Neil Armstrong
> ---
> drivers/gpu/drm/meson/meson_crtc.c | 6 ++
> drivers/gpu/drm/meson/meson_drv.h | 3 +++
> drivers/gpu/drm/meson/meson_plane.c | 7 +++
> 3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_crtc.c
> b/drivers/gpu/drm/meson/meson_crtc.c
> index 5155f01..0552020 100644
> --- a/drivers/gpu/drm/meson/meson_crtc.c
> +++ b/drivers/gpu/drm/meson/meson_crtc.c
> @@ -36,6 +36,7 @@
> #include "meson_venc.h"
> #include "meson_vpp.h"
> #include "meson_viu.h"
> +#include "meson_canvas.h"
> #include "meson_registers.h"
>
> /* CRTC definition */
> @@ -192,6 +193,11 @@ void meson_crtc_irq(struct meson_drm *priv)
> } else
> meson_vpp_disable_interlace_vscaler_osd1(priv);
>
> + meson_canvas_setup(priv, MESON_CANVAS_ID_OSD1,
> +priv->viu.osd1_addr, priv->viu.osd1_stride,
> +priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE,
> +MESON_CANVAS_BLKMODE_LINEAR);
> +
> /* Enable OSD1 */
> writel_bits_relaxed(VPP_OSD1_POSTBLEND, VPP_OSD1_POSTBLEND,
> priv->io_base + _REG(VPP_MISC));
> diff --git a/drivers/gpu/drm/meson/meson_drv.h
> b/drivers/gpu/drm/meson/meson_drv.h
> index 5e8b392..8450d6ac 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -43,6 +43,9 @@ struct meson_drm {
> bool osd1_commit;
> uint32_t osd1_ctrl_stat;
> uint32_t osd1_blk0_cfg[5];
> + uint32_t osd1_addr;
> + uint32_t osd1_stride;
> + uint32_t osd1_height;
> } viu;
>
> struct {
> diff --git a/drivers/gpu/drm/meson/meson_plane.c
> b/drivers/gpu/drm/meson/meson_plane.c
> index d0a6ac8..27bd350 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -164,10 +164,9 @@ static void meson_plane_atomic_update(struct drm_plane
> *plane,
> /* Update Canvas with buffer address */
> gem = drm_fb_cma_get_gem_obj(fb, 0);
>
> - meson_canvas_setup(priv, MESON_CANVAS_ID_OSD1,
> -gem->paddr, fb->pitches[0],
> -fb->height, MESON_CANVAS_WRAP_NONE,
> -MESON_CANVAS_BLKMODE_LINEAR);
> + priv->viu.osd1_addr = gem->paddr;
> + priv->viu.osd1_stride = fb->pitches[0];
> + priv->viu.osd1_height = fb->height;
>
> spin_unlock_irqrestore(&priv->drm->event_lock, flags);
> }
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 198123] Console is the wrong color at boot with radeon 6670
https://bugzilla.kernel.org/show_bug.cgi?id=198123 --- Comment #32 from Deposite Pirate (dpir...@metalpunks.info) --- Hi, Yeah, I wasn't sure because I never enabled backtraces before. But it didn't seem to me that there were many backtraces either. Though I did double check the patch was applied. I have X automatically started by systemd/lightdm at boot but the booting process is slow enough that I can see the wrong colors while the kernel prints it's stuff for a few seconds. And yes the colors are correct after X starts when I switch back to consoles with CTRL+ALT+F?. The failure to compile on HEAD of back then had nothing to do with the patch. I don't remember exactly where it failed to compile but it was a completely unrelated source file. It's not like regressions even with stable kernel releases happen rarely. Right now, I have another Athlon 64 system with an onboard GeForce card that won't even start X because it can't find /dev/card/dri0 with kernel 4.15.4 and another more recent AMD A8-something ThinkPad that OOPSes at boot because of something related to SATA with 4.15.2. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] locking/ww_mutex: cleanup lock->ctx usage in amdgpu
On Thu, Feb 15, 2018 at 03:19:42PM +0100, Christian König wrote:
> amdgpu needs to verify if userspace sends us valid addresses and the simplest
> way of doing this is to check if the buffer object is locked with the ticket
> of the current submission.
>
> Clean up the access to the ww_mutex internals by providing a function
> for this and extend the check to the thread owning the underlying mutex.
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++-
> include/linux/ww_mutex.h | 17 +
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index eaa3cb0c3ad1..4c04b560e358 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1594,7 +1594,8 @@ int amdgpu_cs_find_mapping(struct amdgpu_cs_parser
> *parser,
> *map = mapping;
>
> /* Double check that the BO is reserved by this CS */
> - if (READ_ONCE((*bo)->tbo.resv->lock.ctx) != &parser->ticket)
> + if (!ww_mutex_is_owned_by(&(*bo)->tbo.resv->lock, current,
> + &parser->ticket))
> return -EINVAL;
>
> if (!((*bo)->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)) {
> diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
> index 39fda195bf78..dd580db289e8 100644
> --- a/include/linux/ww_mutex.h
> +++ b/include/linux/ww_mutex.h
> @@ -358,4 +358,21 @@ static inline bool ww_mutex_is_locked(struct ww_mutex
> *lock)
> return mutex_is_locked(&lock->base);
> }
>
> +/**
> + * ww_mutex_is_owned_by - is the w/w mutex locked by this task in that
> context
> + * @lock: the mutex to be queried
> + * @task: the task structure to check
> + * @ctx: the w/w acquire context to test
> + *
> + * Returns true if the mutex is locked in the context by the given task,
> false
> + * otherwise.
> + */
> +static inline bool ww_mutex_is_owned_by(struct ww_mutex *lock,
> + struct task_struct *task,
> + struct ww_acquire_ctx *ctx)
> +{
> + return likely(__mutex_owner(&lock->base) == task) &&
> + READ_ONCE(lock->ctx) == ctx;
Just comparing the context should be good enough. If you ever pass a
ww_acquire_ctx which does not belong to your own thread your seriously
wreaking things much worse already (and if we do catch that, should
probably lock the ctx to a given task when ww-mutex debugging is enabled).
That also simplifies the function signature.
Of course that means if you don't have a ctx, you can't test ownership of
a ww_mute, but I think that's not a really valid use-case. And not needed
for cmd submission, where you need the ctx anyway.
Besides this interface nit looks all good. With the task check¶meter
removed:
Reviewed-by: Daniel Vetter
-Daniel
> +}
> +
> #endif
> --
> 2.14.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 4/6] drm/panel: simple: Add ability to override typical timing
On Thu, Feb 08, 2018 at 12:48:51PM -0500, Sean Paul wrote:
> This patch adds the ability to override the typical display timing for a
> given panel. This is useful for devices which have timing constraints
> that do not apply across the entire display driver (eg: to avoid
> crosstalk between panel and digitizer on certain laptops). The rules are
> as follows:
>
> - panel must not specify fixed mode (since the override mode will
> either be the same as the fixed mode, or we'll be unable to
> check the bounds of the overried)
> - panel must specify at least one display_timing range which will be
> used to ensure the override mode fits within its bounds
>
> Changes in v2:
> - Parse the full display-timings node (using the native-mode) (Rob)
> Changes in v3:
> - No longer parse display-timings subnode, use panel-timing (Rob)
>
> Cc: Doug Anderson
> Cc: Eric Anholt
> Cc: Heiko Stuebner
> Cc: Jeffy Chen
> Cc: Rob Herring
> Cc: Stéphane Marchesin
> Cc: Thierry Reding
> Cc: devicet...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Sean Paul
> ---
> drivers/gpu/drm/panel/panel-simple.c | 67
> +++-
> 1 file changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 5591984a392b..87488392bca1 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -34,6 +34,7 @@
> #include
>
> #include
> +#include
> #include
>
> struct panel_desc {
> @@ -87,6 +88,8 @@ struct panel_simple {
> struct i2c_adapter *ddc;
>
> struct gpio_desc *enable_gpio;
> +
> + struct drm_display_mode override_mode;
> };
>
> static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
> @@ -99,11 +102,22 @@ static int panel_simple_get_fixed_modes(struct
> panel_simple *panel)
> struct drm_connector *connector = panel->base.connector;
> struct drm_device *drm = panel->base.drm;
> struct drm_display_mode *mode;
> + bool has_override = panel->override_mode.type;
> unsigned int i, num = 0;
>
> if (!panel->desc)
> return 0;
>
> + if (has_override) {
> + mode = drm_mode_duplicate(drm, &panel->override_mode);
> + if (mode) {
> + drm_mode_probed_add(connector, mode);
> + num++;
> + } else {
> + dev_err(drm->dev, "failed to add override mode\n");
> + }
> + }
Do we really want to continue after this? Shouldn't the override mode
simply override everything else? That is, if users do override the mode,
do we want to give them additional modes to potentially choose from?
Presumably the reason why the user had to override is because the fixed
one didn't work.
Actually, we should only ever have either the display timings specified
or a fixed mode. Anything else is rather bogus.
> +
> for (i = 0; i < panel->desc->num_timings; i++) {
> const struct display_timing *dt = &panel->desc->timings[i];
> struct videomode vm;
> @@ -120,7 +134,7 @@ static int panel_simple_get_fixed_modes(struct
> panel_simple *panel)
>
> mode->type |= DRM_MODE_TYPE_DRIVER;
>
> - if (panel->desc->num_timings == 1)
> + if (panel->desc->num_timings == 1 && !has_override)
> mode->type |= DRM_MODE_TYPE_PREFERRED;
>
> drm_mode_probed_add(connector, mode);
> @@ -291,10 +305,58 @@ static const struct drm_panel_funcs panel_simple_funcs
> = {
> .get_timings = panel_simple_get_timings,
> };
>
> +#define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \
> + (to_check->field.typ >= bounds->field.min && \
> + to_check->field.typ <= bounds->field.max)
> +static void panel_simple_add_override_mode(struct device *dev,
> +struct panel_simple *panel,
> +const struct display_timing *ot)
> +{
> + const struct panel_desc *desc = panel->desc;
> + struct videomode vm;
> + int i;
unsigned int, please.
> +
> + if (desc->num_modes) {
> + dev_err(dev, "Reject override mode: panel has a fixed mode\n");
> + return;
> + }
> + if (!desc->num_timings) {
> + dev_err(dev, "Reject override mode: no timings specified\n");
> + return;
> + }
Perhaps these should be WARN_ON() to be annoying, but let the driver
continue with the override mode? Again, this is something that we should
catch during review (when a new compatible is added, or a new driver, we
should make sure that it always comes with a timing).
WARN_ON() is probably enough to let people know when they test that they
forgot something.
> +
> + for (i = 0; i < panel->desc->num_timings; i++) {
> + const struct display_timing *dt = &panel->desc->ti
Re: [PATCH] drm/doc: nerved -> nerfed in drm_ioctl.c
On Thu, Feb 15, 2018 at 03:33:17PM -0500, Alex Deucher wrote: > On Thu, Feb 15, 2018 at 2:56 PM, Harry Wentland > wrote: > > On 2018-02-15 11:40 AM, Daniel Stone wrote: > >> Hi Harry, > >> > >> On 15 February 2018 at 16:28, Harry Wentland > >> wrote: > >>> This threw me for a loop when I read the docs. I imagine this is the > >>> intended definition: > >>> http://www.dictionary.com/browse/nerf > >> > >> Yeah. I'm quite sure it was intended to be 'nerfed', but replacing it > >> with something more clear (and especially more accessible to > >> non-native speakers) would be great if you could do that instead > >> please. :) > > > > Good point. > > > > danvet, I'm not sure exactly what nerfed really means in this context? Does > > it mean 'dropped', 'deprecated', 'no longer supported'? > > > > I think in this context, it means broken. For slang, urban dictionary helps: https://www.urbandictionary.com/define.php?term=nerf i.e. I meant "removed to make it harmless". So dropped/no longer supported is accurate I think. And 'deprecated' for the intent to remove it in the future. -Daniel > > Alex > > > I also see a reference to drm version 1.4 but I only see version 1.3 in > > libdrm? > > > > Harry > > > >> > >> Cheers, > >> Daniel > >> > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
