[PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels
Hi Rob, Ping ? On Monday 17 Oct 2016 15:42:56 Laurent Pinchart wrote: > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote: > > On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote: > >> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote: > >>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote: > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. > Multiple incompatible data link layers have been used over time to > transmit image data to LVDS panels. This binding supports display > panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG > specifications. > > Signed-off-by: Laurent Pinchart > > --- > > .../bindings/display/panel/panel-lvds.txt | 119 > 1 file changed, 119 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-lvds.txt> > > diff --git > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > new file mode 100644 > index ..250861f2673e > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > @@ -0,0 +1,119 @@ > +Generic LVDS Panel > +== > + > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. > Multiple > +incompatible data link layers have been used over time to transmit > image data > +to LVDS panels. This bindings supports display panels compatible with > the > +following specifications. > + > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, > February > +1999 (Version 1.0), Japan Electronic Industry Development Association > (JEIDA) > +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), > National > +Semiconductor > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), > Video > +Electronics Standards Association (VESA) > + > +Device compatible with those specifications have been marketed under > the > +FPD-Link and FlatLink brands. > + > + > +Required properties: > +- compatible: shall contain "panel-lvds" > >>> > >>> Maybe as a fallback, but on its own, no way. > >> > >> Which brings an interesting question: when designing generic DT > >> bindings, what's the rule regarding > > Looks like I forgot part of the question. I meant to ask what is the rule > regarding usage of more precise compatible strings ? > > For instance (but perhaps not the best example), the > input/rotary-encoder.txt bindings define a "rotary-encoder" compatible > string, with no other bindings defining more precise compatible strings for > the exact rotary encoder model. When it comes to panels I believe it makes > sense to define model-specific compatible strings even if they're unused by > drivers. I'm however wondering what the rule is there, and where those > device-specific compatible strings should be defined. I'd like to avoid > using one file per panel model as done today for the simple-panel bindings. > > > Call it "simple" so I can easily NAK it. :) > > > > Define a generic structure, not a single binding trying to serve all. > > > >>> > +- width-mm: panel display width in millimeters > >>> > +- height-mm: panel display height in millimeters > >>> > >>> This is already documented for all panels IIRC. > >> > >> Note that this DT binding has nothing to do with the simple-panel > >> binding. It is instead similar to the panel-dpi and panel-dsi-cm bindings > >> (which currently don't but should specify the panel size in DT). The LVDS > >> panel driver will *not* include any panel-specific information such as > >> size or timings, these are specified in DT. > > > > The panel bindings aren't really different. The biggest difference was > > location in the tree, but we now generally allow panels to be either a > > child of the LCD controller or connected with OF graph. We probably > > need to work on restructuring the panel bindings a bit. We should have > > an inheritance with a base panel binding of things like size, label, > > graph, backlight, etc, then perhaps an interface specific bindings for > > LVDS, DSI, and parallel, then a panel specific binding. With this the > > panel specific binding is typically just a compatible string and which > > inherited properties apply to it. > > That sounds good to me, but we have multiple models for panel bindings. > > As you mentioned panels can be referenced through an LCD controller node > property containing a phandle to the panel node, or through OF graph. That's > a situation we have today, and we need to keep supporting both (at least > for existing panels, perhaps not for the new ones). > > Another difference is how to express panel data such as size and timings. > The simple-panel DT bind
[PATCH] ARM: shmobile: dts: Switch to panel-lvds bindings for Mitsubishi panels
The aa104xd12 and aa121td01 panels are LVDS panels, not DPI panels.
Use the correct DT bindings.
Signed-off-by: Laurent Pinchart
---
arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi | 3 ++-
arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
Hello,
This patch is an example of how the panel-lvds bindings should be used for
display panels. It isn't meant to be merged upstream at the moment as the
bindings haven't been accepted yet but can already be used as both an example
and a test base for LVDS mode selection.
diff --git a/arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi
b/arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi
index 65cb50f0c29f..238d14bb0ebe 100644
--- a/arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi
+++ b/arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi
@@ -10,10 +10,11 @@
/ {
panel {
- compatible = "mitsubishi,aa104xd12", "panel-dpi";
+ compatible = "mitsubishi,aa104xd12", "panel-lvds";
width-mm = <210>;
height-mm = <158>;
+ data-mapping = "jeida-18";
panel-timing {
/* 1024x768 @65Hz */
diff --git a/arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi
b/arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi
index a07ebf8f6938..04aafd479775 100644
--- a/arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi
+++ b/arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi
@@ -10,10 +10,11 @@
/ {
panel {
- compatible = "mitsubishi,aa121td01", "panel-dpi";
+ compatible = "mitsubishi,aa121td01", "panel-lvds";
width-mm = <261>;
height-mm = <163>;
+ data-mapping = "jeida-18";
panel-timing {
/* 1280x800 @60Hz */
--
Regards,
Laurent Pinchart
[Intel-gfx] [PATCH v2] drm/dp: Make space for null terminator in the DP device ID char array
Adding Cc's. On Mon, 2016-11-07 at 15:22 -0800, Dhinakaran Pandiyan wrote: > The DP device identification string read from the DPCD registers is 6 > characters long at max. and we store it in a char array of the same length > without space for the NULL terminator. Fix this by increasing the array > size to 7 and initialize it to an empty string. > > v2: Use %*pE format specifier (Jani) > > Signed-off-by: Dhinakaran Pandiyan > --- > drivers/gpu/drm/drm_dp_helper.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 3e6fe82..2d42760 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -544,7 +544,7 @@ void drm_dp_downstream_debug(struct seq_file *m, >DP_DETAILED_CAP_INFO_AVAILABLE; > int clk; > int bpc; > - char id[6]; > + char id[6] = ""; > int len; > uint8_t rev[2]; > int type = port_cap[0] & DP_DS_PORT_TYPE_MASK; > @@ -584,7 +584,8 @@ void drm_dp_downstream_debug(struct seq_file *m, > } > > drm_dp_downstream_id(aux, id); > - seq_printf(m, "\t\tID: %s\n", id); > + len = strnlen(id, 6); > + seq_printf(m, "\t\tID: %*pE\n", len, id); > > len = drm_dp_dpcd_read(aux, DP_BRANCH_HW_REV, &rev[0], 1); > if (len > 0)
[Bug 98724] garbled output using glDrawElementsIndirect
https://bugs.freedesktop.org/show_bug.cgi?id=98724 Aaron Paden changed: What|Removed |Added CC||aaronbpaden at gmail.com --- Comment #1 from Aaron Paden --- Created attachment 127970 --> https://bugs.freedesktop.org/attachment.cgi?id=127970&action=edit api traces Hi, I am the original reporter. At Logan's request, I'm uploading an archive with apitraces when I was using llvmpipe and when I was using radeonsi. In both cases, GLupeN64 was compiled with DEBUG=1 in the makefile. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/e4008476/attachment.html>
[PATCH v11 0/3] drm: add explicit fencing
From: Gustavo Padovan Hi, Hopefully the last version of the patches with the two comments from Brian in the previous version addressed. Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and added support to fences. Current patches can be seen here: https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1 He ran AOSP on top of padovan/fences kernel branch with full fence support on qemu/virgl and msm db410c. That means we already have a working open source userspace using the explicit fencing implementation. Also i-g-t testing are available with all tests suggested in v7 included: https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/ Please review! Gustavo [1] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1253822.html --- Gustavo Padovan (3): drm/fence: add in-fences support drm/fence: add fence timeline to drm_crtc drm/fence: add out-fences support drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_atomic.c| 255 +--- drivers/gpu/drm/drm_atomic_helper.c | 5 + drivers/gpu/drm/drm_crtc.c | 45 +++ drivers/gpu/drm/drm_plane.c | 1 + include/drm/drm_atomic.h| 1 + include/drm/drm_crtc.h | 56 7 files changed, 319 insertions(+), 45 deletions(-) -- 2.5.5
[PATCH v11 1/3] drm/fence: add in-fences support
From: Gustavo Padovan
There is now a new property called IN_FENCE_FD attached to every plane
state that receives sync_file fds from userspace via the atomic commit
IOCTL.
The fd is then translated to a fence (that may be a fence_array
subclass or just a normal fence) and then used by DRM to fence_wait() for
all fences in the sync_file to signal. So it only commits when all
framebuffers are ready to scanout.
v2: Comments by Daniel Vetter:
- remove set state->fence = NULL in destroy phase
- accept fence -1 as valid and just return 0
- do not call fence_get() - sync_file_fences_get() already calls it
- fence_put() if state->fence is already set, in case userspace
set the property more than once.
v3: WARN_ON if fence is set but state has no FB
v4: Comment from Maarten Lankhorst
- allow set fence with no related fb
v5: rename FENCE_FD to IN_FENCE_FD
v6: Comments by Daniel Vetter:
- rename plane_state->in_fence back to "fence"
- re-introduce WARN_ON if fence set but no fb
- rebase after fence -> dma_fence rename
v7: Comments by Brian Starkey
- set state->fence to NULL when duplicating the state
- fail if IN_FENCE_FD was already set
Signed-off-by: Gustavo Padovan
Reviewed-by: Brian Starkey
Reviewed-by: Sean Paul
Tested-by: Robert Foss
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/drm_atomic.c| 14 ++
drivers/gpu/drm/drm_atomic_helper.c | 5 +
drivers/gpu/drm/drm_crtc.c | 6 ++
drivers/gpu/drm/drm_plane.c | 1 +
include/drm/drm_crtc.h | 5 +
6 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 863cdca..95fc041 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -12,6 +12,7 @@ menuconfig DRM
select I2C
select I2C_ALGOBIT
select DMA_SHARED_BUFFER
+ select SYNC_FILE
help
Kernel-level support for the Direct Rendering Infrastructure (DRI)
introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 57e0a6e9..3ad780a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
@@ -712,6 +713,17 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
drm_atomic_set_fb_for_plane(state, fb);
if (fb)
drm_framebuffer_unreference(fb);
+ } else if (property == config->prop_in_fence_fd) {
+ if (state->fence)
+ return -EINVAL;
+
+ if (U642I64(val) == -1)
+ return 0;
+
+ state->fence = sync_file_get_fence(val);
+ if (!state->fence)
+ return -EINVAL;
+
} else if (property == config->prop_crtc_id) {
struct drm_crtc *crtc = drm_crtc_find(dev, val);
return drm_atomic_set_crtc_for_plane(state, crtc);
@@ -773,6 +785,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
if (property == config->prop_fb_id) {
*val = (state->fb) ? state->fb->base.id : 0;
+ } else if (property == config->prop_in_fence_fd) {
+ *val = -1;
} else if (property == config->prop_crtc_id) {
*val = (state->crtc) ? state->crtc->base.id : 0;
} else if (property == config->prop_crtc_x) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 5007796..0b16587 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3072,6 +3072,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct
drm_plane *plane,
if (state->fb)
drm_framebuffer_reference(state->fb);
+
+ state->fence = NULL;
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
@@ -3110,6 +3112,9 @@ void __drm_atomic_helper_plane_destroy_state(struct
drm_plane_state *state)
{
if (state->fb)
drm_framebuffer_unreference(state->fb);
+
+ if (state->fence)
+ dma_fence_put(state->fence);
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 5745464..f6d1b38 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -397,6 +397,12 @@ static int drm_mode_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_fb_id = prop;
+ prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC,
+ "IN_FENCE_FD", -1, INT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_in_fence_fd = prop;
+
prop = drm_property_create_object(dev, DRM_MOD
[PATCH v11 2/3] drm/fence: add fence timeline to drm_crtc
From: Gustavo Padovan
Create one timeline context for each CRTC to be able to handle out-fences
and signal them. It adds a few members to struct drm_crtc: fence_context,
where we store the context we get from fence_context_alloc(), the
fence seqno and the fence lock, that we pass in fence_init() to be
used by the fence.
v2: Comment by Daniel Stone:
- add BUG_ON() to fence_to_crtc() macro
v3: Comment by Ville Syrjälä
- Use more meaningful name as crtc timeline name
v4: Comments by Brian Starkey
- Use even more meaninful name for the crtc timeline
- add doc for timeline_name
Comment by Daniel Vetter
- use in-line style for comments
- rebase after fence -> dma_fence rename
v5: Comment by Daniel Vetter
- Add doc for drm_crtc_fence_ops
Signed-off-by: Gustavo Padovan
Reviewed-by: Daniel Vetter
Reviewed-by: Sean Paul
Tested-by: Robert Foss
---
drivers/gpu/drm/drm_crtc.c | 31 +++
include/drm/drm_crtc.h | 45 +
2 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f6d1b38..20ddaff 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -165,6 +165,32 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
#endif
}
+static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->dev->driver->name;
+}
+
+static const char *drm_crtc_fence_get_timeline_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->timeline_name;
+}
+
+static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
+{
+ return true;
+}
+
+const struct dma_fence_ops drm_crtc_fence_ops = {
+ .get_driver_name = drm_crtc_fence_get_driver_name,
+ .get_timeline_name = drm_crtc_fence_get_timeline_name,
+ .enable_signaling = drm_crtc_fence_enable_signaling,
+ .wait = dma_fence_default_wait,
+};
+
/**
* drm_crtc_init_with_planes - Initialise a new CRTC object with
*specified primary and cursor planes.
@@ -222,6 +248,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
struct drm_crtc *crtc,
return -ENOMEM;
}
+ crtc->fence_context = dma_fence_context_alloc(1);
+ spin_lock_init(&crtc->fence_lock);
+ snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
+"CRTC:%d-%s", crtc->base.id, crtc->name);
+
crtc->base.properties = &crtc->properties;
list_add_tail(&crtc->head, &config->crtc_list);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 11780a9..0870de1 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -32,6 +32,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -739,9 +741,52 @@ struct drm_crtc {
*/
struct drm_crtc_crc crc;
#endif
+
+ /**
+* @fence_context:
+*
+* timeline context used for fence operations.
+*/
+ unsigned int fence_context;
+
+ /**
+* @fence_lock:
+*
+* spinlock to protect the fences in the fence_context.
+*/
+
+ spinlock_t fence_lock;
+ /**
+* @fence_seqno:
+*
+* Seqno variable used as monotonic counter for the fences
+* created on the CRTC's timeline.
+*/
+ unsigned long fence_seqno;
+
+ /**
+* @timeline_name:
+*
+* The name of the CRTC's fence timeline.
+*/
+ char timeline_name[32];
};
/**
+ * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
+ *
+ * It contains the dma_fence_ops that should be called by the dma_fence
+ * code. CRTC core should use this ops when initializing fences.
+ */
+extern const struct dma_fence_ops drm_crtc_fence_ops;
+
+static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
+{
+ BUG_ON(fence->ops != &drm_crtc_fence_ops);
+ return container_of(fence->lock, struct drm_crtc, fence_lock);
+}
+
+/**
* struct drm_mode_set - new values for a CRTC config change
* @fb: framebuffer to use for new config
* @crtc: CRTC whose configuration we're about to change
--
2.5.5
[PATCH v11 3/3] drm/fence: add out-fences support
From: Gustavo Padovan
Support DRM out-fences by creating a sync_file with a fence for each CRTC
that sets the OUT_FENCE_PTR property.
We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
the sync_file fd back to userspace.
The sync_file and fd are allocated/created before commit, but the
fd_install operation only happens after we know that commit succeed.
v2: Comment by Rob Clark:
- Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
Comment by Daniel Vetter:
- Add clean up code for out_fences
v3: Comments by Daniel Vetter:
- create DRM_MODE_ATOMIC_EVENT_MASK
- userspace should fill out_fences_ptr with the crtc_ids for which
it wants fences back.
v4: Create OUT_FENCE_PTR properties and remove old approach.
v5: Comments by Brian Starkey:
- Remove extra fence_get() in atomic_ioctl()
- Check ret before iterating on the crtc_state
- check ret before fd_install
- set fence_state to NULL at the beginning
- check fence_state->out_fence_ptr before put_user()
- change order of fput() and put_unused_fd() on failure
- Add access_ok() check to the out_fence_ptr received
- Rebase after fence -> dma_fence rename
- Store out_fence_ptr in the drm_atomic_state
- Split crtc_setup_out_fence()
- return -1 as out_fence with TEST_ONLY flag
v6: Comments by Daniel Vetter
- Add prepare/unprepare_crtc_signaling()
- move struct drm_out_fence_state to drm_atomic.c
- mark get_crtc_fence() as static
Comments by Brian Starkey
- proper set fence_ptr fence_state array
- isolate fence_idx increment
- improve error handling
v7: Comments by Daniel Vetter
- remove prefix from internal functions
- make out_fence_ptr an s64 pointer
- degrade DRM_INFO to DRM_DEBUG_ATOMIC when put_user fail
- fix doc issues
- filter out OUT_FENCE_PTR == NULL and do not fail in this case
- add complete_crtc_signalling()
- krealloc fence_state on demand
Comment by Brian Starkey
- remove unused crtc_state arg from get_out_fence()
v8: Comment by Brian Starkey
- cancel events before check for !fence_state
- convert a few lefovers u64 types for out_fence_ptr
- fix memleak by assign fence_state earlier after realloc
- proper accout num_fences in case of error
v9: Comment by Brian Starkey
- memset last position of fence_state after krealloc
Comments by Sean Paul
- pass install_fds in complete_crtc_signaling() instead of ret
- put_user(-1, fence_ptr) when decoding props
v10: Comment by Brian Starkey
- remove unneeded num_fences increment on error path
- kfree fence_state after installing fences fd
Signed-off-by: Gustavo Padovan
Reviewed-by: Brian Starkey
Tested-by: Robert Foss
---
drivers/gpu/drm/drm_atomic.c | 241 +++
drivers/gpu/drm/drm_crtc.c | 8 ++
include/drm/drm_atomic.h | 1 +
include/drm/drm_crtc.h | 6 ++
4 files changed, 211 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3ad780a..d4a92a9 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -290,6 +290,23 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
}
EXPORT_SYMBOL(drm_atomic_get_crtc_state);
+static void set_out_fence_for_crtc(struct drm_atomic_state *state,
+ struct drm_crtc *crtc, s64 __user *fence_ptr)
+{
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
+}
+
+static s64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
+ struct drm_crtc *crtc)
+{
+ s64 __user *fence_ptr;
+
+ fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
+
+ return fence_ptr;
+}
+
/**
* drm_atomic_set_mode_for_crtc - set mode for CRTC
* @state: the CRTC whose incoming state to update
@@ -494,6 +511,16 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
+ } else if (property == config->prop_out_fence_ptr) {
+ s64 __user *fence_ptr = u64_to_user_ptr(val);
+
+ if (!fence_ptr)
+ return 0;
+
+ if (put_user(-1, fence_ptr))
+ return -EFAULT;
+
+ set_out_fence_for_crtc(state->state, crtc, fence_ptr);
} else if (crtc->funcs->atomic_set_property)
return crtc->funcs->atomic_set_property(crtc, state, property,
val);
else
@@ -536,6 +563,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = (state->ctm) ? state->ctm->base.id : 0;
[PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels
Hi Rob, On Monday 14 Nov 2016 19:40:26 Rob Herring wrote: > On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart wrote: > > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote: > >> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote: > >>> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote: > On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote: > > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. > > Multiple incompatible data link layers have been used over time to > > transmit image data to LVDS panels. This binding supports display > > panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG > > specifications. > > > > Signed-off-by: Laurent Pinchart > > > > --- > > > > .../bindings/display/panel/panel-lvds.txt | 119 > > 1 file changed, 119 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/panel/panel-lvds.txt> > > > > diff --git > > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > > new file mode 100644 > > index ..250861f2673e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > > @@ -0,0 +1,119 @@ > > +Generic LVDS Panel > > +== > > + > > +LVDS is a physical layer specification defined in > > ANSI/TIA/EIA-644-A. Multiple > > +incompatible data link layers have been used over time to transmit > > image data > > +to LVDS panels. This bindings supports display panels compatible > > with the > > +following specifications. > > + > > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, > > February > > +1999 (Version 1.0), Japan Electronic Industry Development > > Association (JEIDA) > > +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), > > National > > +Semiconductor > > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), > > Video > > +Electronics Standards Association (VESA) > > + > > +Device compatible with those specifications have been marketed under > > the > > +FPD-Link and FlatLink brands. > > + > > + > > +Required properties: > > +- compatible: shall contain "panel-lvds" > > Maybe as a fallback, but on its own, no way. > >>> > >>> Which brings an interesting question: when designing generic DT > >>> bindings, what's the rule regarding > > > > Looks like I forgot part of the question. I meant to ask what is the rule > > regarding usage of more precise compatible strings ? > > When in doubt, always have one. If there's any chance at all that s/w > will need to know or care, then we should have one. > > > For instance (but perhaps not the best example), the > > input/rotary-encoder.txt bindings define a "rotary-encoder" compatible > > string, with no other bindings defining more precise compatible strings > > for the exact rotary encoder model. When it comes to panels I believe it > > makes sense to define model-specific compatible strings even if they're > > unused by drivers. I'm however wondering what the rule is there, and > > where those device-specific compatible strings should be defined. I'd > > like to avoid using one file per panel model as done today for the > > simple-panel bindings. > > There's a few exceptions like this where there is not any sort of > model to base a compatible on. For example, a GPIO connected LED is > truly generic. The only way to have a more specific compatible would > be something with the board name in it. > > Your case here is in the middle. It seems like it's generic and > passive, but perhaps power control is not. Rather than trying to > decide, we can just cover our ass and put both a generic and specific > compatible in. That sounds good to me. I'll mention in the document that a more precise compatible is required. > >> Call it "simple" so I can easily NAK it. :) > >> > >> Define a generic structure, not a single binding trying to serve all. > >> > > +- width-mm: panel display width in millimeters > > +- height-mm: panel display height in millimeters > > This is already documented for all panels IIRC. > >>> > >>> Note that this DT binding has nothing to do with the simple-panel > >>> binding. It is instead similar to the panel-dpi and panel-dsi-cm > >>> bindings (which currently don't but should specify the panel size in > >>> DT). The LVDS panel driver will *not* include any panel-specific > >>> information such as size or timings, these are specified in DT. > >> > >> The panel bindings aren't really different. The biggest difference was > >> location in the tree, but we now generally allow panels to be either a > >> child of the LCD controller or connected with OF graph. We probably > >> need to work on re
[Bug 98417] TTM broken on 4.9-rc2
https://bugs.freedesktop.org/show_bug.cgi?id=98417
--- Comment #7 from Adam J. Richter ---
Agreeing with the previous comments that this is probably not a TTM problem, I
want to pass along that I have observed what is probably the same problem, but
with many kernel modules unrelated to TTM and graphics.
I think it might be possible to work around the problem by disabling
CONFIG_MODVERSIONS, but just have not had the time to try that yet.
I suspect that this has something to do with the changes in symbol exports that
occurred in linux 4.9-rc1, which you can see if you do something like:
diff -pruN linux-{4.8,4.9-rc1}/arch/x86/lib
The symbols that I have had trouble with, such as memset, are ones that have
had export declarations added to assembler sources (.S files). I see that the
entry for memset in the generated file Module.symvers is different.
In Linux 4.8, it looks like this:
0xfb578fc5 memset vmlinux EXPORT_SYMBOL
In Linux 4.9-rc1, it looks like this:
0x memset vmlinux EXPORT_SYMBOL
As you can see, the first field, which I believe is some sort of checksum of
the C function declaration, is all zeroes for memset in Linux 4.9-rc1.
I am still looking into this, but I am posting now because I may need to set
this task aside for a day or two and didn't want to delay in passing along
information that I think might be helpful in resolving your problem.
--
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/6e2c4fa3/attachment.html>
[Bug 98417] TTM broken on 4.9-rc2
https://bugs.freedesktop.org/show_bug.cgi?id=98417 --- Comment #8 from Adam J. Richter --- FYI, here is a very readable document that explains how kernel symbol versioning is implemented: http://tldp.org/HOWTO/Module-HOWTO/basekerncompat.html . -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/65a80e31/attachment.html>
[Bug 97403] AMDGPU/Iceland Strange warnings on drm-next-4.9-wip
https://bugs.freedesktop.org/show_bug.cgi?id=97403 --- Comment #5 from rezhu --- sorry, maybe the adaptor number is not 0 in your machine. can you just try the command: ./atiflash -ai on my end: the result is /home# ./atiflash -ai Adapter 0(BN=01, DN=00, PCIID=69011002, SSID=01341002) Asic Family: Iceland if you can get the adapter number, then try to save the atom bios by ./atiflash -s number file.name -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/7adfcb68/attachment.html>
[PATCH v2] drivers/gpu/vga: allocate vga_arb_write() buffer on stack
On Mon, Nov 14, 2016 at 10:11:47AM +0100, Dmitry Vyukov wrote:
> On Fri, Oct 14, 2016 at 3:22 PM, Dmitry Vyukov wrote:
> > Size of kmalloc() in vga_arb_write() is controlled by user.
> > Too large kmalloc() size triggers WARNING message on console.
> > Allocate the buffer on stack to avoid the WARNING.
> > The string must be small (e.g "target PCI:domain:bus:dev.fn").
> >
> > Signed-off-by: Dmitry Vyukov
> > Reviewed-by: Ville Syrjälä
> > Cc: Dave Airlie
> > Cc: Ville Syrjälä
> > Cc: dri-devel at lists.freedesktop.org
> > Cc: syzkaller at googlegroups.com
>
> Ping, this is still not merged.
> David, please take this to dri tree.
It's applied to drm-misc already.
-Daniel
>
>
> > ---
> >
> > Changes since v1:
> > - removed another kfree(kbuf)
> >
> > I've sent a patch that adds __GFP_NOWARN to the kmalloc a while ago:
> > https://groups.google.com/forum/#!msg/syzkaller/navdAwe4iCo/mZDhODsnAAAJ
> >
> > Ville suggested to allocate the buffer on stack as it must be small:
> >
> > "From the looks of things the longest command could be the
> > "target PCI:domain:bus:dev.fn" thing. Even assuming something silly like
> > having 10 characters for each domain,bus,dev,fn that would still be only
> > 55 bytes. So based on that even something like 64 bytes should be more
> > than enough AFAICS. "
> >
> > Example WARNING:
> >
> > WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999
> > __alloc_pages_nodemask+0x7d2/0x1760()
> > Modules linked in:
> > CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB 4.5.0-rc1+ #283
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> > 880069eff670 8299a06d
> > 8800658a4740 864985a0 880069eff6b0 8134fcf9
> > 8166de32 864985a0 0bb7 024040c0
> > Call Trace:
> > [< inline >] __dump_stack lib/dump_stack.c:15
> > [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
> > [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482
> > [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515
> > [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999
> > [] __alloc_pages_nodemask+0x7d2/0x1760
> > mm/page_alloc.c:3253
> > [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090
> > [< inline >] alloc_pages include/linux/gfp.h:459
> > [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433
> > [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008
> > [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019
> > [< inline >] kmalloc_large include/linux/slab.h:395
> > [] __kmalloc+0x2f4/0x340 mm/slub.c:3557
> > [< inline >] kmalloc include/linux/slab.h:468
> > [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926
> > [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719
> > [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849
> > [] vfs_writev+0x86/0xc0 fs/read_write.c:886
> > [< inline >] SYSC_writev fs/read_write.c:919
> > [] SyS_writev+0x111/0x2b0 fs/read_write.c:911
> > [] entry_SYSCALL_64_fastpath+0x16/0x7a
> > arch/x86/entry/entry_64.S:185
> > ---
> > drivers/gpu/vga/vgaarb.c | 15 ---
> > 1 file changed, 4 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> > index 1887f19..77657a8 100644
> > --- a/drivers/gpu/vga/vgaarb.c
> > +++ b/drivers/gpu/vga/vgaarb.c
> > @@ -1022,21 +1022,16 @@ static ssize_t vga_arb_write(struct file *file,
> > const char __user *buf,
> >
> > unsigned int io_state;
> >
> > - char *kbuf, *curr_pos;
> > + char kbuf[64], *curr_pos;
> > size_t remaining = count;
> >
> > int ret_val;
> > int i;
> >
> > -
> > - kbuf = kmalloc(count + 1, GFP_KERNEL);
> > - if (!kbuf)
> > - return -ENOMEM;
> > -
> > - if (copy_from_user(kbuf, buf, count)) {
> > - kfree(kbuf);
> > + if (count >= sizeof(kbuf))
> > + return -EINVAL;
> > + if (copy_from_user(kbuf, buf, count))
> > return -EFAULT;
> > - }
> > curr_pos = kbuf;
> > kbuf[count] = '\0'; /* Just to make sure... */
> >
> > @@ -1259,11 +1254,9 @@ static ssize_t vga_arb_write(struct file *file,
> > const char __user *buf,
> > goto done;
> > }
> > /* If we got here, the message written is not part of the protocol!
> > */
> > - kfree(kbuf);
> > return -EPROTO;
> >
> > done:
> > - kfree(kbuf);
> > return ret_val;
> > }
> >
> > --
> > 2.8.0.rc3.226.g39d4020
> >
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] dma-buf: Use fence_get_rcu_safe() for retrieving the exclusive fence
On Mon, Nov 14, 2016 at 04:37:09PM +0100, Christian König wrote: > Am 14.11.2016 um 12:55 schrieb Chris Wilson: > > The current code is subject to a race where we may try to acquire a > > reference on a stale fence: > > > > [13703.335118] WARNING: CPU: 1 PID: 14975 at ./include/linux/kref.h:46 > > i915_gem_object_wait+0x1a3/0x1c0 > > [13703.335184] Modules linked in: > > [13703.335202] CPU: 1 PID: 14975 Comm: gem_concurrent_ Not tainted > > 4.9.0-rc4+ #26 > > [13703.335216] Hardware name: /, BIOS > > PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 > > [13703.335233] c90002f5bcc8 812807de > > > > [13703.335257] c90002f5bd08 81073811 002e8000 > > 88026bf7c780 > > [13703.335279] 7fff 0001 88027045a550 > > 88026bf7c780 > > [13703.335301] Call Trace: > > [13703.335316] [] dump_stack+0x4d/0x6f > > [13703.335331] [] __warn+0xc1/0xe0 > > [13703.335343] [] warn_slowpath_null+0x18/0x20 > > [13703.335355] [] i915_gem_object_wait+0x1a3/0x1c0 > > [13703.335367] [] i915_gem_set_domain_ioctl+0xcc/0x330 > > [13703.335386] [] drm_ioctl+0x1cb/0x410 > > [13703.335400] [] ? > > i915_gem_obj_prepare_shmem_write+0x1d0/0x1d0 > > [13703.335416] [] ? drm_ioctl+0x2bb/0x410 > > [13703.335429] [] do_vfs_ioctl+0x8f/0x5c0 > > [13703.335442] [] SyS_ioctl+0x3c/0x70 > > [13703.335456] [] entry_SYSCALL_64_fastpath+0x17/0x98 > > [13703.335558] ---[ end trace fd24176416ba6981 ]--- > > [13703.382778] general protection fault: [#1] SMP > > [13703.382802] Modules linked in: > > [13703.382816] CPU: 1 PID: 14967 Comm: gem_concurrent_ Tainted: GW > > 4.9.0-rc4+ #26 > > [13703.382828] Hardware name: /, BIOS > > PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 > > [13703.382841] task: 880275458000 task.stack: c90002f18000 > > [13703.382849] RIP: 0010:[] [] > > i915_gem_request_retire+0x2b4/0x320 > > [13703.382870] RSP: 0018:c90002f1bbc8 EFLAGS: 00010293 > > [13703.382878] RAX: dead0200 RBX: 88026bf7dce8 RCX: > > dead0100 > > [13703.382887] RDX: dead0100 RSI: 88026bf7c930 RDI: > > 88026bf7dd00 > > [13703.382897] RBP: c90002f1bbf8 R08: R09: > > 88026b89a000 > > [13703.382905] R10: 0001 R11: 88026bbe8fe0 R12: > > 88026bf7c000 > > [13703.382913] R13: 880275af8000 R14: 88026bf7c180 R15: > > dead0200 > > [13703.382922] FS: 7f89e787d740() GS:88027fd0() > > knlGS: > > [13703.382934] CS: 0010 DS: ES: CR0: 80050033 > > [13703.382942] CR2: 7f9053d2e000 CR3: 00026d414000 CR4: > > 001006e0 > > [13703.382951] Stack: > > [13703.382958] 880275413000 c90002f1bde8 880275af8000 > > 880274e8a600 > > [13703.382976] 880276a06000 c90002f1bde8 c90002f1bc38 > > 813b48c5 > > [13703.382995] c90002f1bc00 c90002f1bde8 88026972a440 > > > > [13703.383021] Call Trace: > > [13703.383032] [] i915_gem_request_alloc+0xa5/0x350 > > [13703.383043] [] > > i915_gem_do_execbuffer.isra.41+0x7b3/0x18b0 > > [13703.383055] [] ? i915_gem_object_get_sg+0x25c/0x2b0 > > [13703.383065] [] ? i915_gem_object_get_page+0x1d/0x50 > > [13703.383076] [] ? i915_gem_pwrite_ioctl+0x66c/0x6d0 > > [13703.383086] [] i915_gem_execbuffer2+0x95/0x1e0 > > [13703.383096] [] drm_ioctl+0x1cb/0x410 > > [13703.383105] [] ? i915_gem_execbuffer+0x2d0/0x2d0 > > [13703.383117] [] ? hrtimer_start_range_ns+0x1a0/0x310 > > [13703.383128] [] do_vfs_ioctl+0x8f/0x5c0 > > [13703.383140] [] ? SyS_timer_settime+0x118/0x1a0 > > [13703.383150] [] SyS_ioctl+0x3c/0x70 > > [13703.383162] [] entry_SYSCALL_64_fastpath+0x17/0x98 > > [13703.383172] Code: 49 39 c6 48 8d 70 e8 48 8d 5f e8 75 16 eb 47 48 8d 43 > > 18 48 8b 53 18 48 89 de 49 39 c6 48 8d 5a e8 74 33 48 8b 56 08 48 8b 46 10 > > <48> 89 42 08 48 89 10 f6 46 38 01 48 89 4e 08 4c 89 7e 10 74 cf > > [13703.383557] RIP [] i915_gem_request_retire+0x2b4/0x320 > > [13703.383570] RSP > > [13703.383586] ---[ end trace fd24176416ba6982 ]--- > > > > This is fixed by using the kref_get_unless_zero() as a full memory > > barrier to validate the fence is still the current exclusive fence before > > returning it back to the caller. (Note the fix only requires using > > dma_fence_get_rcu() and correct handling, but we may as well use the > > helper rather than inline equivalent code.) > > > > Signed-off-by: Chris Wilson > > Cc: Sumit Semwal > Reviewed-by: Christian König . Applied to drm-misc, thanks. -Daniel > > > --- > > include/linux/reservation.h | 15 ++- > > 1 file changed, 6 insertions(+), 9 deletions(-) > > > > diff --git a/include/linux/reservation.h b/include/linux/reservation.h > > index 2e313cca08f0..d9706a6f5ae2 100644 > > --- a/include/linux/reservation.h > > +++ b/include/linux/reservation.h > > @@
[PATCH] drm: don't let crtc_ww_class leak out
On Mon, Nov 14, 2016 at 05:40:57PM -0500, Rob Clark wrote:
> kbuild spotted this error, with drm/msm patches that add a new
> modeset-lock in the driver and driver built as a module:
>
> ERROR: "crtc_ww_class" [drivers/gpu/drm/msm/msm.ko] undefined!
>
> Really the only reason for crtc_ww_class not being internal to
> drm_modeset_lock.c is that drm_modeset_lock_init() was static-inline
> (for no particularly good reason).
>
> Fix that, and move crtc_ww_class into drm_modeset_lock.c.
>
> Signed-off-by: Rob Clark
Applied to drm-misc, thx.
-Daniel
> ---
> This is an alternative to the "drm: export crtc_ww_class" patch.
>
> drivers/gpu/drm/drm_crtc.c | 2 --
> drivers/gpu/drm/drm_modeset_lock.c | 13 +
> include/drm/drm_modeset_lock.h | 12 +---
> 3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index cf2423d..5d994cc 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -102,8 +102,6 @@ int drm_crtc_force_disable_all(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_crtc_force_disable_all);
>
> -DEFINE_WW_CLASS(crtc_ww_class);
> -
> static unsigned int drm_num_crtcs(struct drm_device *dev)
> {
> unsigned int num = 0;
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c
> b/drivers/gpu/drm/drm_modeset_lock.c
> index 61146f5..9059fe3 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -60,6 +60,8 @@
> * lists and lookup data structures.
> */
>
> +static DEFINE_WW_CLASS(crtc_ww_class);
> +
> /**
> * drm_modeset_lock_all - take all modeset locks
> * @dev: DRM device
> @@ -398,6 +400,17 @@ int drm_modeset_backoff_interruptible(struct
> drm_modeset_acquire_ctx *ctx)
> EXPORT_SYMBOL(drm_modeset_backoff_interruptible);
>
> /**
> + * drm_modeset_lock_init - initialize lock
> + * @lock: lock to init
> + */
> +void drm_modeset_lock_init(struct drm_modeset_lock *lock)
> +{
> + ww_mutex_init(&lock->mutex, &crtc_ww_class);
> + INIT_LIST_HEAD(&lock->head);
> +}
> +EXPORT_SYMBOL(drm_modeset_lock_init);
> +
> +/**
> * drm_modeset_lock - take modeset lock
> * @lock: lock to take
> * @ctx: acquire ctx
> diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
> index c5576fb..d918ce4 100644
> --- a/include/drm/drm_modeset_lock.h
> +++ b/include/drm/drm_modeset_lock.h
> @@ -82,8 +82,6 @@ struct drm_modeset_lock {
> struct list_head head;
> };
>
> -extern struct ww_class crtc_ww_class;
> -
> void drm_modeset_acquire_init(struct drm_modeset_acquire_ctx *ctx,
> uint32_t flags);
> void drm_modeset_acquire_fini(struct drm_modeset_acquire_ctx *ctx);
> @@ -91,15 +89,7 @@ void drm_modeset_drop_locks(struct drm_modeset_acquire_ctx
> *ctx);
> void drm_modeset_backoff(struct drm_modeset_acquire_ctx *ctx);
> int drm_modeset_backoff_interruptible(struct drm_modeset_acquire_ctx *ctx);
>
> -/**
> - * drm_modeset_lock_init - initialize lock
> - * @lock: lock to init
> - */
> -static inline void drm_modeset_lock_init(struct drm_modeset_lock *lock)
> -{
> - ww_mutex_init(&lock->mutex, &crtc_ww_class);
> - INIT_LIST_HEAD(&lock->head);
> -}
> +void drm_modeset_lock_init(struct drm_modeset_lock *lock);
>
> /**
> * drm_modeset_lock_fini - cleanup lock
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH 2/5] drm: Set DRM connector link status property
On Mon, Nov 14, 2016 at 07:13:20PM -0800, Manasi Navare wrote:
> In the usual working scenarios, this property is "Good".
> If something fails during modeset, the DRM driver can
> set the link status to "Bad", prune the mode list based on the
> link rate/lane count fallback values and send hotplug uevent
> so that userspace that is aware of this property can take an
> appropriate action by reprobing connectors and re triggering
> a modeset to improve user experience and avoid black screens.
> In case of userspace that is not aware of this link status
> property, the user experience will be unchanged.
>
> The reason for adding the property is to handle link training failures,
> but it is not limited to DP or link training. For example, if we
> implement asynchronous setcrtc, we can use this to report any failures
> in that.
>
> Cc: dri-devel at lists.freedesktop.org
> Cc: Jani Nikula
> Cc: Daniel Vetter
> Cc: Ville Syrjala
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/drm_connector.c | 38 ++
> include/drm/drm_connector.h | 2 ++
> 2 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index d4e852f..09f4093 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -968,6 +968,44 @@ int drm_mode_connector_update_edid_property(struct
> drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_mode_connector_update_edid_property);
>
> +/**
> + * drm_mode_connector_set_link_status_property - Set the link status
> property of
> + * a connector to indicate status of link as a result of link training.
iirc this continuation upsets kernel-doc. Did you build the docs and
review them? You need to indent the 2nd line.
Also, might just shorten it to "set link status for a connector", details
are for the text below.
> + * @connector: drm connector
> + * @link_status: new value of link status property (0: Good, 1: Bad)
> + *
> + * In usual working scenario, this link status property will always be set to
> + * "GOOD".
Unecessary linebbreak. Either make a full paragraph (empty line) or
reflow, this here won't survivie kernel-doc formatting.
> + * If something fails during or after a mode set, the kernel driver can set
> this
> + * link status to "BAD", prune the mode list based on new information and
> send a
First need to prune the mode list, then set the property. Please reorder
in your text.
> + * hotplug uevent for userspace to have it re-check the valid modes through
> + * Get_connector and try again.
s/Get_connector/GET_CONNECTOR IOCTL/ is the more usual style.
> + *
> + * If userspace is not aware of this property, the user experience is the
> same
> + * as it currently is. If the userspace is aware of the property, it has a
> chance
> + * to improve user experience by handling link training failures, avoiding
> black
> + * screens. The DRM driver can chose to not modify property and keep link
> status
> + * as "GOOD" always to keep the user experience same as it currently is.
Imo this paragraph isn't needed. Maybe just mention that old userspace
exists:
"Note that a lot of existing userspace doesn't handle this property.
Drivers can therefore not rely on userspace to fix up everything and
should try to handle issues (like just re-training a link) without
userspace's intervention. This should only be used when the current mode
doesn't work any more, and userspace must select a different display
mode."
> + *
> + * The reason for adding this property is to handle link training failures,
> but
> + * it is not limited to DP or link training. For example, if we implement
> + * asynchronous setcrtc, this property can be used to reportany failures in
> that.
s/reportany/report/
> + *
> + * This function must be called from asynchronous work item.
This isn't true - it doesn't require an asynchronous work item, but the
locking rules mean that it.
> + * Returns zero on success and negative errrno on failure.
Hm, why can this ever fail? Intuitively this should never fail, and hence
we shouldn't need an error return value.
> + */
> +int drm_mode_connector_set_link_status_property(struct drm_connector
> *connector,
> + uint64_t link_status)
> +{
> + struct drm_device *dev = connector->dev;
> +
> + connector->link_status = link_status;
> + return drm_object_property_set_value(&connector->base,
> +
> dev->mode_config.link_status_property,
> + link_status);
This misses the hotplug_event call from my proposal. Intentionally? Why?
Also: With the current code you require that mode_config.mutex is held by
the caller. Every time you add a library/core function which requires
certain locks to be held, please check that with something like
lockdep_assert_held or similar. Leaking locking rules to callers like this
shoul
[PATCH 2/5] drm: Set DRM connector link status property
On Mon, Nov 14, 2016 at 07:23:33PM -0800, Manasi Navare wrote:
> None of the other functions that set the connector property hold the
> mode config locks while setting the connector property, I am following
> the same convention.
> Also we dont need to grab and release the locks in i915_modeset_work_func
> that first validates the modes and then sets this link status property.
Which other functions don't hold the mode_config.mutex?
-Daniel
>
> Manasi
>
> On Mon, Nov 14, 2016 at 07:13:20PM -0800, Manasi Navare wrote:
> > In the usual working scenarios, this property is "Good".
> > If something fails during modeset, the DRM driver can
> > set the link status to "Bad", prune the mode list based on the
> > link rate/lane count fallback values and send hotplug uevent
> > so that userspace that is aware of this property can take an
> > appropriate action by reprobing connectors and re triggering
> > a modeset to improve user experience and avoid black screens.
> > In case of userspace that is not aware of this link status
> > property, the user experience will be unchanged.
> >
> > The reason for adding the property is to handle link training failures,
> > but it is not limited to DP or link training. For example, if we
> > implement asynchronous setcrtc, we can use this to report any failures
> > in that.
> >
> > Cc: dri-devel at lists.freedesktop.org
> > Cc: Jani Nikula
> > Cc: Daniel Vetter
> > Cc: Ville Syrjala
> > Signed-off-by: Manasi Navare
> > ---
> > drivers/gpu/drm/drm_connector.c | 38 ++
> > include/drm/drm_connector.h | 2 ++
> > 2 files changed, 40 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index d4e852f..09f4093 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -968,6 +968,44 @@ int drm_mode_connector_update_edid_property(struct
> > drm_connector *connector,
> > }
> > EXPORT_SYMBOL(drm_mode_connector_update_edid_property);
> >
> > +/**
> > + * drm_mode_connector_set_link_status_property - Set the link status
> > property of
> > + * a connector to indicate status of link as a result of link training.
> > + * @connector: drm connector
> > + * @link_status: new value of link status property (0: Good, 1: Bad)
> > + *
> > + * In usual working scenario, this link status property will always be set
> > to
> > + * "GOOD".
> > + * If something fails during or after a mode set, the kernel driver can
> > set this
> > + * link status to "BAD", prune the mode list based on new information and
> > send a
> > + * hotplug uevent for userspace to have it re-check the valid modes through
> > + * Get_connector and try again.
> > + *
> > + * If userspace is not aware of this property, the user experience is the
> > same
> > + * as it currently is. If the userspace is aware of the property, it has a
> > chance
> > + * to improve user experience by handling link training failures, avoiding
> > black
> > + * screens. The DRM driver can chose to not modify property and keep link
> > status
> > + * as "GOOD" always to keep the user experience same as it currently is.
> > + *
> > + * The reason for adding this property is to handle link training
> > failures, but
> > + * it is not limited to DP or link training. For example, if we implement
> > + * asynchronous setcrtc, this property can be used to reportany failures
> > in that.
> > + *
> > + * This function must be called from asynchronous work item.
> > + * Returns zero on success and negative errrno on failure.
> > + */
> > +int drm_mode_connector_set_link_status_property(struct drm_connector
> > *connector,
> > + uint64_t link_status)
> > +{
> > + struct drm_device *dev = connector->dev;
> > +
> > + connector->link_status = link_status;
> > + return drm_object_property_set_value(&connector->base,
> > +
> > dev->mode_config.link_status_property,
> > +link_status);
> > +}
> > +EXPORT_SYMBOL(drm_mode_connector_set_link_status_property);
> > +
> > int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
> > struct drm_property *property,
> > uint64_t value)
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index ad5c8b0..ac76469 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -778,6 +778,8 @@ int drm_mode_connector_set_path_property(struct
> > drm_connector *connector,
> > int drm_mode_connector_set_tile_property(struct drm_connector *connector);
> > int drm_mode_connector_update_edid_property(struct drm_connector
> > *connector,
> > const struct edid *edid);
> > +int drm_mode_connector_set_link_status_property(struct drm_connector
> > *connector,
> > +
[Intel-gfx] [PATCH 2/5] drm: Set DRM connector link status property
On Mon, Nov 14, 2016 at 07:13:20PM -0800, Manasi Navare wrote:
> In the usual working scenarios, this property is "Good".
> If something fails during modeset, the DRM driver can
> set the link status to "Bad", prune the mode list based on the
> link rate/lane count fallback values and send hotplug uevent
> so that userspace that is aware of this property can take an
> appropriate action by reprobing connectors and re triggering
> a modeset to improve user experience and avoid black screens.
> In case of userspace that is not aware of this link status
> property, the user experience will be unchanged.
>
> The reason for adding the property is to handle link training failures,
> but it is not limited to DP or link training. For example, if we
> implement asynchronous setcrtc, we can use this to report any failures
> in that.
>
> Cc: dri-devel at lists.freedesktop.org
> Cc: Jani Nikula
> Cc: Daniel Vetter
> Cc: Ville Syrjala
> Signed-off-by: Manasi Navare
One more thing I've forgotten: Just adding the kernel-doc isn't enough
yet, we need to link this new property into the overall property
documentations.
We already have a section "KMS Properties" in
Documentation/gpu/drm-kms.rst, I think adding a new sub-section called
"Standard Connector Properties" there with a definition list pointing to
this function here would be best.
-Daniel
> ---
> drivers/gpu/drm/drm_connector.c | 38 ++
> include/drm/drm_connector.h | 2 ++
> 2 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index d4e852f..09f4093 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -968,6 +968,44 @@ int drm_mode_connector_update_edid_property(struct
> drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_mode_connector_update_edid_property);
>
> +/**
> + * drm_mode_connector_set_link_status_property - Set the link status
> property of
> + * a connector to indicate status of link as a result of link training.
> + * @connector: drm connector
> + * @link_status: new value of link status property (0: Good, 1: Bad)
> + *
> + * In usual working scenario, this link status property will always be set to
> + * "GOOD".
> + * If something fails during or after a mode set, the kernel driver can set
> this
> + * link status to "BAD", prune the mode list based on new information and
> send a
> + * hotplug uevent for userspace to have it re-check the valid modes through
> + * Get_connector and try again.
> + *
> + * If userspace is not aware of this property, the user experience is the
> same
> + * as it currently is. If the userspace is aware of the property, it has a
> chance
> + * to improve user experience by handling link training failures, avoiding
> black
> + * screens. The DRM driver can chose to not modify property and keep link
> status
> + * as "GOOD" always to keep the user experience same as it currently is.
> + *
> + * The reason for adding this property is to handle link training failures,
> but
> + * it is not limited to DP or link training. For example, if we implement
> + * asynchronous setcrtc, this property can be used to reportany failures in
> that.
> + *
> + * This function must be called from asynchronous work item.
> + * Returns zero on success and negative errrno on failure.
> + */
> +int drm_mode_connector_set_link_status_property(struct drm_connector
> *connector,
> + uint64_t link_status)
> +{
> + struct drm_device *dev = connector->dev;
> +
> + connector->link_status = link_status;
> + return drm_object_property_set_value(&connector->base,
> +
> dev->mode_config.link_status_property,
> + link_status);
> +}
> +EXPORT_SYMBOL(drm_mode_connector_set_link_status_property);
> +
> int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
> struct drm_property *property,
> uint64_t value)
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ad5c8b0..ac76469 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -778,6 +778,8 @@ int drm_mode_connector_set_path_property(struct
> drm_connector *connector,
> int drm_mode_connector_set_tile_property(struct drm_connector *connector);
> int drm_mode_connector_update_edid_property(struct drm_connector *connector,
> const struct edid *edid);
> +int drm_mode_connector_set_link_status_property(struct drm_connector
> *connector,
> + uint64_t link_status);
>
> /**
> * drm_for_each_connector - iterate over all connectors
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.o
[PATCH 01/12] tests/kms_atomic_transition: use select + read instead of blocking read
On Mon, Nov 14, 2016 at 06:59:14PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> If the event never arrives we can timeout with select and end the test.
>
> Signed-off-by: Gustavo Padovan
> ---
> tests/kms_atomic_transition.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> index 1977993..e693c88 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -296,6 +296,14 @@ static void commit_display(igt_display_t *display,
> unsigned event_mask, bool non
> struct drm_event *e = (void *)buf;
> struct drm_event_vblank *vblank = (void *)buf;
> uint32_t crtc_id, pipe = I915_MAX_PIPES;
> + struct timeval timeout = { .tv_sec = 3, .tv_usec = 0 };
> + fd_set fds;
> +
> + FD_ZERO(&fds);
> + FD_SET(0, &fds);
> + FD_SET(display->drm_fd, &fds);
> + ret = select(display->drm_fd + 1, &fds, NULL, NULL, &timeout);
> + igt_assert(ret > 0);
Hm, we have igt_timeout which sends a signal and kills the test if the
timeout expires. And drm event reading should be interruptible. That might
be an even simpler way to implement this.
-Daniel
>
> ret = read(display->drm_fd, buf, sizeof(buf));
> if (ret < 0 && (errno == EINTR || errno == EAGAIN))
> --
> 2.5.5
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH 02/12] tests/kms_atomic_transition: don't assume max pipes
On Mon, Nov 14, 2016 at 06:59:16PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Signed-off-by: Gustavo Padovan
> ---
> tests/kms_atomic_transition.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> index e693c88..8b26b53 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -404,7 +404,7 @@ static void run_modeset_tests(igt_display_t *display, int
> howmany, bool nonblock
> {
> struct igt_fb fbs[2];
> int i, j;
> - unsigned iter_max = 1 << I915_MAX_PIPES;
> + unsigned iter_max = 1 << display->n_pipes;
Didn't Tomeu have some patch series to fix these all up?
-Daniel
> igt_pipe_crc_t *pipe_crcs[I915_MAX_PIPES];
> igt_output_t *output;
> unsigned width = 0, height = 0;
> --
> 2.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v11 2/3] drm/fence: add fence timeline to drm_crtc
On Tue, Nov 15, 2016 at 10:57:35AM +0900, Gustavo Padovan wrote:
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 11780a9..0870de1 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -32,6 +32,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> @@ -739,9 +741,52 @@ struct drm_crtc {
>*/
> struct drm_crtc_crc crc;
> #endif
> +
> + /**
> + * @fence_context:
> + *
> + * timeline context used for fence operations.
> + */
> + unsigned int fence_context;
> +
> + /**
> + * @fence_lock:
> + *
> + * spinlock to protect the fences in the fence_context.
> + */
> +
> + spinlock_t fence_lock;
> + /**
> + * @fence_seqno:
> + *
> + * Seqno variable used as monotonic counter for the fences
> + * created on the CRTC's timeline.
> + */
> + unsigned long fence_seqno;
> +
> + /**
> + * @timeline_name:
> + *
> + * The name of the CRTC's fence timeline.
> + */
> + char timeline_name[32];
> };
>
> /**
> + * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
> + *
> + * It contains the dma_fence_ops that should be called by the dma_fence
> + * code. CRTC core should use this ops when initializing fences.
> + */
> +extern const struct dma_fence_ops drm_crtc_fence_ops;
> +
> +static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
> +{
> + BUG_ON(fence->ops != &drm_crtc_fence_ops);
> + return container_of(fence->lock, struct drm_crtc, fence_lock);
> +}
If you are planning to export this for use by drivers, you are missing
the EXPORT_SYMBOL(drm_crtc_fence_ops).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[Intel-gfx] [PATCH v11 3/4] drm/i915: Use new CRC debugfs API
On Tue, 15 Nov 2016, David Weinehall wrote:
> On Mon, Nov 14, 2016 at 12:44:25PM +0200, Jani Nikula wrote:
>> On Thu, 06 Oct 2016, Tomeu Vizoso wrote:
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > b/drivers/gpu/drm/i915/intel_display.c
>> > index 23a6c7213eca..7412a05fa5d9 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -14636,6 +14636,7 @@ static const struct drm_crtc_funcs
>> > intel_crtc_funcs = {
>> >.page_flip = intel_crtc_page_flip,
>> >.atomic_duplicate_state = intel_crtc_duplicate_state,
>> >.atomic_destroy_state = intel_crtc_destroy_state,
>> > + .set_crc_source = intel_crtc_set_crc_source,
>> > };
>> >
>> > /**
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> > b/drivers/gpu/drm/i915/intel_drv.h
>> > index 737261b09110..31894b7c6517 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -1844,6 +1844,14 @@ void intel_color_load_luts(struct drm_crtc_state
>> > *crtc_state);
>> > /* intel_pipe_crc.c */
>> > int intel_pipe_crc_create(struct drm_minor *minor);
>> > void intel_pipe_crc_cleanup(struct drm_minor *minor);
>> > +#ifdef CONFIG_DEBUG_FS
>> > +int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char
>> > *source_name,
>> > +size_t *values_cnt);
>> > +#else
>> > +static inline int intel_crtc_set_crc_source(struct drm_crtc *crtc,
>> > + const char *source_name,
>> > + size_t *values_cnt) { return 0; }
>> > +#endif
>>
>> "inline" here doesn't work because it's used as a function pointer.
>>
>> Is it better to have a function that returns 0 for .set_crc_source, or
>> to set .set_crc_source to NULL when CONFIG_DEBUG_FS=n?
>
> I'd say that whenever we have a function pointer we should have a dummy
> function without side-effects for this kind of things.
Whoever calls .set_crc_source could do smarter things depending on the
hook not being there vs. just silently plunging on.
BR,
Jani.
>
>
> Kind regards, David
--
Jani Nikula, Intel Open Source Technology Center
[drm-intel:topic/drm-misc 23/26] include/drm/drm_fb_cma_helper.h:45:13: warning: 'struct drm_plane_state' declared inside parameter list will not be visible outside of this definition or declaration
tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc head: 35cf03508d8466ecc5199c9d609e74e85bec785b commit: 14d7f96f90fb65c2ca0e0ac7df237e06ff001c29 [23/26] drm/fb_cma_helper: Add drm_fb_cma_prepare_fb() helper config: i386-allmodconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: git checkout 14d7f96f90fb65c2ca0e0ac7df237e06ff001c29 # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/vc4/vc4_drv.c:18:0: >> include/drm/drm_fb_cma_helper.h:45:13: warning: 'struct drm_plane_state' >> declared inside parameter list will not be visible outside of this >> definition or declaration struct drm_plane_state *state); ^~~ >> include/drm/drm_fb_cma_helper.h:44:34: warning: 'struct drm_plane' declared >> inside parameter list will not be visible outside of this definition or >> declaration int drm_fb_cma_prepare_fb(struct drm_plane *plane, ^ vim +45 include/drm/drm_fb_cma_helper.h 38 struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, 39 struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd); 40 41 struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, 42 unsigned int plane); 43 > 44 int drm_fb_cma_prepare_fb(struct drm_plane *plane, > 45struct drm_plane_state *state); 46 47 #ifdef CONFIG_DEBUG_FS 48 struct seq_file; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- next part -- A non-text attachment was scrubbed... Name: .config.gz Type: application/gzip Size: 56846 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/0fcc9071/attachment-0001.gz>
[Intel-gfx] [PATCH 04/12] lib/igt_kms: export properties names
On Mon, Nov 14, 2016 at 06:59:18PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Signed-off-by: Gustavo Padovan
gtkdoc for anything exported would always be nice ...
-Daniel
> ---
> lib/igt_kms.c | 6 +++---
> lib/igt_kms.h | 5 +
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index aa9fd16..8aaff5b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -153,7 +153,7 @@ const unsigned char* igt_kms_get_base_edid(void)
> #define EDID_NAME alt_edid
> #include "igt_edid_template.h"
>
> -static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> +const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> "SRC_X",
> "SRC_Y",
> "SRC_W",
> @@ -168,7 +168,7 @@ static const char
> *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> "rotation"
> };
>
> -static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> +const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> "background_color",
> "CTM",
> "DEGAMMA_LUT",
> @@ -177,7 +177,7 @@ static const char
> *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> "ACTIVE"
> };
>
> -static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> +const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> "scaling mode",
> "CRTC_ID"
> };
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 6422adc..ae2b505 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -113,12 +113,16 @@ enum igt_atomic_crtc_properties {
> IGT_NUM_CRTC_PROPS
> };
>
> +extern const char *igt_crtc_prop_names[];
> +
> enum igt_atomic_connector_properties {
> IGT_CONNECTOR_SCALING_MODE = 0,
> IGT_CONNECTOR_CRTC_ID,
> IGT_NUM_CONNECTOR_PROPS
> };
>
> +extern const char *igt_connector_prop_names[];
> +
> struct kmstest_connector_config {
> drmModeCrtc *crtc;
> drmModeConnector *connector;
> @@ -214,6 +218,7 @@ enum igt_atomic_plane_properties {
> IGT_NUM_PLANE_PROPS
> };
>
> +extern const char *igt_plane_prop_names[];
>
> typedef struct igt_display igt_display_t;
> typedef struct igt_pipe igt_pipe_t;
> --
> 2.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH 08/12] tests/kms_atomic: stress possible fence settings
On Mon, Nov 14, 2016 at 06:59:22PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Signed-off-by: Gustavo Padovan
> ---
> tests/kms_atomic.c | 124
> +
> 1 file changed, 115 insertions(+), 9 deletions(-)
>
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> index 8b648eb..58fc0dd 100644
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -44,6 +44,7 @@
> #include "drmtest.h"
> #include "igt.h"
> #include "igt_aux.h"
> +#include "sw_sync.h"
>
> #ifndef DRM_CLIENT_CAP_ATOMIC
> #define DRM_CLIENT_CAP_ATOMIC 3
> @@ -126,6 +127,7 @@ struct kms_atomic_plane_state {
> uint32_t fb_id; /* 0 to disable */
> uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */
> uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */
> + int32_t fence_fd;
> };
>
> struct kms_atomic_crtc_state {
> @@ -133,6 +135,7 @@ struct kms_atomic_crtc_state {
> uint32_t obj;
> int idx;
> bool active;
> + uint64_t out_fence_ptr;
> struct kms_atomic_blob mode;
> };
>
> @@ -190,11 +193,13 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig)
> crtc_populate_req(crtc, req); \
> plane_populate_req(plane, req); \
> do_atomic_commit((crtc)->state->desc->fd, req, flags); \
> - crtc_check_current_state(crtc, plane, relax); \
> - plane_check_current_state(plane, relax); \
> + if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \
> + crtc_check_current_state(crtc, plane, relax); \
> + plane_check_current_state(plane, relax); \
> + } \
> }
>
> -#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, relax,
> e) { \
> +#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, flags,
> relax, e) { \
> drmModeAtomicSetCursor(req, 0); \
> crtc_populate_req(crtc, req); \
> plane_populate_req(plane, req); \
> @@ -299,6 +304,9 @@ find_connector(struct kms_atomic_state *state,
> static void plane_populate_req(struct kms_atomic_plane_state *plane,
> drmModeAtomicReq *req)
> {
> + if (plane->fence_fd)
> + plane_set_prop(req, plane, IGT_PLANE_IN_FENCE_FD,
> plane->fence_fd);
> +
> plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id);
> plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id);
> plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x);
> @@ -424,6 +432,10 @@ find_plane(struct kms_atomic_state *state, enum
> plane_type type,
> static void crtc_populate_req(struct kms_atomic_crtc_state *crtc,
> drmModeAtomicReq *req)
> {
> + if (crtc->out_fence_ptr)
> + crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR,
> + crtc->out_fence_ptr);
> +
> crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id);
> crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active);
> }
> @@ -986,6 +998,7 @@ static void plane_invalid_params(struct
> kms_atomic_crtc_state *crtc,
> uint32_t format = plane_get_igt_format(&plane);
> drmModeAtomicReq *req = drmModeAtomicAlloc();
> struct igt_fb fb;
> + int timeline, fence_fd;
>
> /* Pass a series of invalid object IDs for the FB ID. */
> plane.fb_id = plane.obj;
> @@ -1024,6 +1037,23 @@ static void plane_invalid_params(struct
> kms_atomic_crtc_state *crtc,
> plane_commit_atomic_err(&plane, plane_old, req,
> ATOMIC_RELAX_NONE, EINVAL);
>
> + /* invalid fence fd */
> + plane.fence_fd = plane.state->desc->fd;
> + plane.crtc_id = plane_old->crtc_id;
> + plane_commit_atomic_err(&plane, plane_old, req,
> + ATOMIC_RELAX_NONE, EINVAL);
> +
> + /* Valid fence_fd but invalid CRTC */
> + timeline = sw_sync_timeline_create();
> + fence_fd = sw_sync_fence_create(timeline, 1);
> + plane.fence_fd = fence_fd;
> + plane.crtc_id = ~0U;
> + plane_commit_atomic_err(&plane, plane_old, req,
> + ATOMIC_RELAX_NONE, EINVAL);
> + close(fence_fd);
> + close(timeline);
> +
> + plane.fence_fd = -1;
> plane.crtc_id = plane_old->crtc_id;
> plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
>
> @@ -1058,27 +1088,98 @@ static void crtc_invalid_params(struct
> kms_atomic_crtc_state *crtc_old,
> {
> struct kms_atomic_crtc_state crtc = *crtc_old;
> drmModeAtomicReq *req = drmModeAtomicAlloc();
> + int timeline, fence_fd, *out_fence;
>
> igt_assert(req);
>
> /* Pass a series of invalid object IDs for the mode ID. */
> crtc.mode.id = plane->obj;
> - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
> ATOMIC_RELAX_NONE, EINVAL);
>
> crtc.mode.id = crtc.obj;
> - crtc_commit_atomic_err(&crtc, plane, crtc_old
[PATCH v11 2/3] drm/fence: add fence timeline to drm_crtc
On Tue, Nov 15, 2016 at 08:25:55AM +, Chris Wilson wrote:
> On Tue, Nov 15, 2016 at 10:57:35AM +0900, Gustavo Padovan wrote:
> > /**
> > + * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
> > + *
> > + * It contains the dma_fence_ops that should be called by the dma_fence
> > + * code. CRTC core should use this ops when initializing fences.
> > + */
> > +extern const struct dma_fence_ops drm_crtc_fence_ops;
> > +
> > +static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
> > +{
> > + BUG_ON(fence->ops != &drm_crtc_fence_ops);
> > + return container_of(fence->lock, struct drm_crtc, fence_lock);
> > +}
>
> If you are planning to export this for use by drivers, you are missing
> the EXPORT_SYMBOL(drm_crtc_fence_ops).
My suggestion would not to have the BUG_ON() here (saves one
checkpatch.pl complaint in exchange for a slightly more mysterious GPF).
Also please consider calling this dma_fence_to_crtc() as otherwise it
conflicts with those with plans to use struct fences on their
crtc/states.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH v11 2/3] drm/fence: add fence timeline to drm_crtc
2016-11-15 Chris Wilson :
> On Tue, Nov 15, 2016 at 10:57:35AM +0900, Gustavo Padovan wrote:
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 11780a9..0870de1 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -32,6 +32,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> > #include
> > #include
> > @@ -739,9 +741,52 @@ struct drm_crtc {
> > */
> > struct drm_crtc_crc crc;
> > #endif
> > +
> > + /**
> > +* @fence_context:
> > +*
> > +* timeline context used for fence operations.
> > +*/
> > + unsigned int fence_context;
> > +
> > + /**
> > +* @fence_lock:
> > +*
> > +* spinlock to protect the fences in the fence_context.
> > +*/
> > +
> > + spinlock_t fence_lock;
> > + /**
> > +* @fence_seqno:
> > +*
> > +* Seqno variable used as monotonic counter for the fences
> > +* created on the CRTC's timeline.
> > +*/
> > + unsigned long fence_seqno;
> > +
> > + /**
> > +* @timeline_name:
> > +*
> > +* The name of the CRTC's fence timeline.
> > +*/
> > + char timeline_name[32];
> > };
> >
> > /**
> > + * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
> > + *
> > + * It contains the dma_fence_ops that should be called by the dma_fence
> > + * code. CRTC core should use this ops when initializing fences.
> > + */
> > +extern const struct dma_fence_ops drm_crtc_fence_ops;
> > +
> > +static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
> > +{
> > + BUG_ON(fence->ops != &drm_crtc_fence_ops);
> > + return container_of(fence->lock, struct drm_crtc, fence_lock);
> > +}
>
> If you are planning to export this for use by drivers, you are missing
> the EXPORT_SYMBOL(drm_crtc_fence_ops).
Drivers should not be using this, at least for now.
Gustavo
BUG: 'list_empty(&vgdev->free_vbufs)' is true!
On Fr, 2016-11-11 at 17:28 +0100, Jiri Slaby wrote:
> On 11/09/2016, 09:01 AM, Gerd Hoffmann wrote:
> > On Di, 2016-11-08 at 22:37 +0200, Michael S. Tsirkin wrote:
> >> On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
> >>> Hi,
> >>>
> >>> I can relatively easily reproduce this bug:
> >
> > How?
>
> Run dmesg -w in the qemu window (virtio_gpu) to see a lot of output.
fbcon? Or xorg/wayland with terminal app?
> Run pps [1] without exit(0); on e.g. serial console.
> Wait a bit. The lot of output causes the BUG.
>
> [1] https://github.com/jirislaby/collected_sources/blob/master/pps.c
>
> >>> BUG: 'list_empty(&vgdev->free_vbufs)' is true!
> >
> >> The following might be helpful for debugging - if kernel still will
> >> not stop panicing, we are looking at some kind
> >> of memory corruption.
> >
> > Looking carefully through the code I think it isn't impossible to
> > trigger this, but you need for that:
> >
> > (1) command queue full (quite possible),
> > (2) cursor queue full too (unlikely), and
> > (3) multiple threads trying to submit commands and waiting for free
> > space in the command queue (possible with virgl enabled).
>
> I use -vga virtio with no -display option, so no virtgl, I suppose:
> [drm] virgl 3d acceleration not available
>
> > Do things improve if you allocate some extra bufs?
> >
> > int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev)
> > {
> > struct virtio_gpu_vbuffer *vbuf;
> > - int i, size, count = 0;
> > + int i, size, count = 16;
>
> This seems to help.
>
> thanks,
[drm-intel:topic/drm-misc 23/26] include/drm/drm_fb_cma_helper.h:45:13: warning: 'struct drm_plane_state' declared inside parameter list will not be visible outside of this definition or declaration
On Tue, Nov 15, 2016 at 04:29:04PM +0800, kbuild test robot wrote: > tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc > head: 35cf03508d8466ecc5199c9d609e74e85bec785b > commit: 14d7f96f90fb65c2ca0e0ac7df237e06ff001c29 [23/26] drm/fb_cma_helper: > Add drm_fb_cma_prepare_fb() helper > config: i386-allmodconfig (attached as .config) > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 > reproduce: > git checkout 14d7f96f90fb65c2ca0e0ac7df237e06ff001c29 > # save the attached .config to linux build tree > make ARCH=i386 > > All warnings (new ones prefixed by >>): > >In file included from drivers/gpu/drm/vc4/vc4_drv.c:18:0: > >> include/drm/drm_fb_cma_helper.h:45:13: warning: 'struct drm_plane_state' > >> declared inside parameter list will not be visible outside of this > >> definition or declaration > struct drm_plane_state *state); > ^~~ > >> include/drm/drm_fb_cma_helper.h:44:34: warning: 'struct drm_plane' > >> declared inside parameter list will not be visible outside of this > >> definition or declaration > int drm_fb_cma_prepare_fb(struct drm_plane *plane, > ^ Oops, didn't noticed this compiler warning before pushing. Since drm-misc is non-rebasing, can you pls supply a full fixup patch Marek? Thanks, Daniel > > vim +45 include/drm/drm_fb_cma_helper.h > > 38struct drm_framebuffer *drm_fb_cma_create(struct drm_device > *dev, > 39struct drm_file *file_priv, const struct > drm_mode_fb_cmd2 *mode_cmd); > 40 > 41struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct > drm_framebuffer *fb, > 42unsigned int plane); > 43 > > 44int drm_fb_cma_prepare_fb(struct drm_plane *plane, > > 45 struct drm_plane_state *state); > 46 > 47#ifdef CONFIG_DEBUG_FS > 48struct seq_file; > > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: > On 11/14/2016 10:15 PM, Ville Syrjälä wrote: > > On Mon, Nov 14, 2016 at 10:12:04PM +0530, Sharma, Shashank wrote: > > > Regards > > > > > > Shashank > > > > > > > > > On 11/14/2016 9:50 PM, Ville Syrjälä wrote: > > > > On Mon, Nov 14, 2016 at 09:37:18PM +0530, Sharma, Shashank wrote: > > > > > Regards > > > > > > > > > > Shashank > > > > > > > > > > > > > > > On 11/14/2016 9:19 PM, Ville Syrjälä wrote: > > > > > > On Mon, Nov 14, 2016 at 08:14:34PM +0530, Sharma, Shashank wrote: > > > > > > > Regards > > > > > > > Shashank > > > > > > > > the revert: > > > > > > > > > > > > > > > > HDMI2 connected 1920x1080+0+0 (normal left inverted right > > > > > > > > x axis y axis) 700mm x 390mm > > > > > > > > - 1920x1080 60.00*+ > > > > > > > > - 1920x1080i60.0050.00 > > > > > > > > + 1920x1080 60.00*+ 50.0059.9430.0025.00 > > > > > > > > 24.0029.9723.98 > > > > > > > > + 1920x1080i60.0050.0059.94 > > > > > > > > 1600x1200 60.00 > > > > > > > > 1680x1050 59.88 > > > > > > > > 1280x1024 75.0260.02 > > > > > > > > @@ -13,30 +13,29 @@ > > > > > > > > 1360x768 60.02 > > > > > > > > 1280x800 59.91 > > > > > > > > 1152x864 75.00 > > > > > > > > - 1280x720 60.0050.00 > > > > > > > > + 1280x720 60.0050.0059.94 > > > > > > > > 1024x768 75.0370.0760.00 > > > > > > > > 832x624 74.55 > > > > > > > > 800x600 72.1975.0060.32 > > > > > > > > - 640x480 75.0072.8166.6759.94 > > > > > > > > + 720x576 50.00 > > > > > > > > + 720x480 60.0059.94 > > > > > > > > + 640x480 75.0072.8166.6760.0059.94 > > > > > > > > 720x400 70.08 > > > > > > > None of these aspect ratios are new modes / new aspect ratios > > > > > > > from HDMI > > > > > > > 2.0/CEA-861-F > > > > > > > These are the existing modes, and should be independent of > > > > > > > reverted > > > > > > > patches. > > > > > > They're affected because your patches changed them by adding the > > > > > > aspect > > > > > > ratio flags to them. > > > > > Yes, But they are independent of reverted patch, which adds aspect > > > > > ratio > > > > > for HDMI 2.0 ratios (64:27 and 256:135) > > > > The second patch had to be reverted so that the first patch would revert > > > > cleanly. > > > > > > > > > > > > This was with sna, which does this: > > > > > > > > #define KNOWN_MODE_FLAGS ((1<<14)-1) > > > > > > > > if (mode->status == MODE_OK && kmode->flags & > > > > > > > > ~KNOWN_MODE_FLAGS) > > > > > > > > mode->status = MODE_BAD; /* unknown flags => unhandled > > > > > > > > */ > > > > > > > > so all the modes with an aspect ratio just vanished. > > > > > > > > > > > > > > > > -modesetting and -ati on the other hand just copy over the > > > > > > > > unknown > > > > > > > > bits into the xrandr mode structure, which sounds dubious at > > > > > > > > best: > > > > > > > > mode->Flags = kmode->flags; //& FLAG_BITS; > > > > > > > > I've not checked what damage it can actually cause. > > > > > > > > > > > > > > > > > > > > > > > > It looks like a few modes disappeared from the kernel's mode > > > > > > > > list > > > > > > > > as well, presumably because some cea modes in the list > > > > > > > > originated from > > > > > > > > DTDs and whanot so they don't have an aspect ratio and that > > > > > > > > causes > > > > > > > > add_alternate_cea_modes() to ignore them. So not populating an > > > > > > > > aspect > > > > > > > > ratio for cea modes originating from a source other than > > > > > > > > edid_cea_modes[] looks like another bug to me as well. > > > > > > > I am writing a patch series to cap the aspect ratio > > > > > > > implementation under > > > > > > > a drm_cap_hdmi2_aspect_ratios > > > > > > > This is how its going to work (inspired from the 2D/stereo series > > > > > > > from > > > > > > > damien L) > > > > > > > > > > > > > > - Add a new capability hdmi2_ar > > > > > > It should be just a generic "expose aspect ratio flags to > > > > > > userspace?" > > > > > Makes sense, in this way we can even revert the aspect_ratio property > > > > > for HDMI connector, as discussed during > > > > > the code review sessions of this patch series. In this way, when > > > > > kernel > > > > > will expose the aspect ratios, it will either > > > > > do the aspect ratios as per EDID, or wont. > > > > > > > - by default parsing the new hdmi 2.0 aspect ratio will be > > > > > > > disabled > > > > > > > under check of this cap > > > > > > > - during bootup time, while initializing the display, a userspace > > > > > > > can > > > > > > > get_cap on the hdmi2_aspect_ratio > > > > > > > - If it wants HDMI 2.0 aspect ratio support, it will set the cap, > > > > > > > and > > > >
[PATCH v11 2/3] drm/fence: add fence timeline to drm_crtc
On Tue, Nov 15, 2016 at 05:42:35PM +0900, Gustavo Padovan wrote:
> 2016-11-15 Chris Wilson :
>
> > On Tue, Nov 15, 2016 at 10:57:35AM +0900, Gustavo Padovan wrote:
> > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > index 11780a9..0870de1 100644
> > > --- a/include/drm/drm_crtc.h
> > > +++ b/include/drm/drm_crtc.h
> > > @@ -32,6 +32,8 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -739,9 +741,52 @@ struct drm_crtc {
> > >*/
> > > struct drm_crtc_crc crc;
> > > #endif
> > > +
> > > + /**
> > > + * @fence_context:
> > > + *
> > > + * timeline context used for fence operations.
> > > + */
> > > + unsigned int fence_context;
> > > +
> > > + /**
> > > + * @fence_lock:
> > > + *
> > > + * spinlock to protect the fences in the fence_context.
> > > + */
> > > +
> > > + spinlock_t fence_lock;
> > > + /**
> > > + * @fence_seqno:
> > > + *
> > > + * Seqno variable used as monotonic counter for the fences
> > > + * created on the CRTC's timeline.
> > > + */
> > > + unsigned long fence_seqno;
> > > +
> > > + /**
> > > + * @timeline_name:
> > > + *
> > > + * The name of the CRTC's fence timeline.
> > > + */
> > > + char timeline_name[32];
> > > };
> > >
> > > /**
> > > + * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
> > > + *
> > > + * It contains the dma_fence_ops that should be called by the dma_fence
> > > + * code. CRTC core should use this ops when initializing fences.
> > > + */
> > > +extern const struct dma_fence_ops drm_crtc_fence_ops;
> > > +
> > > +static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
> > > +{
> > > + BUG_ON(fence->ops != &drm_crtc_fence_ops);
> > > + return container_of(fence->lock, struct drm_crtc, fence_lock);
> > > +}
> >
> > If you are planning to export this for use by drivers, you are missing
> > the EXPORT_SYMBOL(drm_crtc_fence_ops).
>
> Drivers should not be using this, at least for now.
Then please put it into drm_crtc_internal.h. We should only allow drivers
to use functions they're supposed to be using, and hide everything else.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH for-4.9] drm/virtio: allocate some extra bufs
virtio-gpu guest driver appearently can run out of buffers.
allocate some extra buffers, as quick stopgap for 4.9.
analyzing root cause and fixing it properly is TBD.
Reported-by: Jiri Slaby
Tested-by: Jiri Slaby
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a0f8a7..974f941 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -75,7 +75,7 @@ void virtio_gpu_cursor_ack(struct virtqueue *vq)
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev)
{
struct virtio_gpu_vbuffer *vbuf;
- int i, size, count = 0;
+ int i, size, count = 16;
void *ptr;
INIT_LIST_HEAD(&vgdev->free_vbufs);
--
1.8.3.1
[PATCH v11 2/3] drm/fence: add fence timeline to drm_crtc
On Tue, Nov 15, 2016 at 05:42:35PM +0900, Gustavo Padovan wrote:
> 2016-11-15 Chris Wilson :
>
> > On Tue, Nov 15, 2016 at 10:57:35AM +0900, Gustavo Padovan wrote:
> > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > index 11780a9..0870de1 100644
> > > --- a/include/drm/drm_crtc.h
> > > +++ b/include/drm/drm_crtc.h
> > > @@ -32,6 +32,8 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -739,9 +741,52 @@ struct drm_crtc {
> > >*/
> > > struct drm_crtc_crc crc;
> > > #endif
> > > +
> > > + /**
> > > + * @fence_context:
> > > + *
> > > + * timeline context used for fence operations.
> > > + */
> > > + unsigned int fence_context;
> > > +
> > > + /**
> > > + * @fence_lock:
> > > + *
> > > + * spinlock to protect the fences in the fence_context.
> > > + */
> > > +
> > > + spinlock_t fence_lock;
> > > + /**
> > > + * @fence_seqno:
> > > + *
> > > + * Seqno variable used as monotonic counter for the fences
> > > + * created on the CRTC's timeline.
> > > + */
> > > + unsigned long fence_seqno;
> > > +
> > > + /**
> > > + * @timeline_name:
> > > + *
> > > + * The name of the CRTC's fence timeline.
> > > + */
> > > + char timeline_name[32];
> > > };
> > >
> > > /**
> > > + * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
> > > + *
> > > + * It contains the dma_fence_ops that should be called by the dma_fence
> > > + * code. CRTC core should use this ops when initializing fences.
> > > + */
> > > +extern const struct dma_fence_ops drm_crtc_fence_ops;
> > > +
> > > +static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
> > > +{
> > > + BUG_ON(fence->ops != &drm_crtc_fence_ops);
> > > + return container_of(fence->lock, struct drm_crtc, fence_lock);
> > > +}
> >
> > If you are planning to export this for use by drivers, you are missing
> > the EXPORT_SYMBOL(drm_crtc_fence_ops).
>
> Drivers should not be using this, at least for now.
You've put into a central header, with kerneldoc on the ops, just inviting
people to use it...
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
BUG: 'list_empty(&vgdev->free_vbufs)' is true!
On 11/15/2016, 09:46 AM, Gerd Hoffmann wrote: > On Fr, 2016-11-11 at 17:28 +0100, Jiri Slaby wrote: >> On 11/09/2016, 09:01 AM, Gerd Hoffmann wrote: >>> On Di, 2016-11-08 at 22:37 +0200, Michael S. Tsirkin wrote: On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote: > Hi, > > I can relatively easily reproduce this bug: >>> >>> How? >> >> Run dmesg -w in the qemu window (virtio_gpu) to see a lot of output. > > fbcon? Or xorg/wayland with terminal app? Ah, just console, so fbcon. No X server running. thanks, -- js suse labs
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
Regards Shashank On 11/15/2016 2:21 PM, Daniel Vetter wrote: > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: >> On 11/14/2016 10:15 PM, Ville Syrjälä wrote: >>> On Mon, Nov 14, 2016 at 10:12:04PM +0530, Sharma, Shashank wrote: Regards Shashank On 11/14/2016 9:50 PM, Ville Syrjälä wrote: > On Mon, Nov 14, 2016 at 09:37:18PM +0530, Sharma, Shashank wrote: >> Regards >> >> Shashank >> >> >> On 11/14/2016 9:19 PM, Ville Syrjälä wrote: >>> On Mon, Nov 14, 2016 at 08:14:34PM +0530, Sharma, Shashank wrote: Regards Shashank > the revert: > > HDMI2 connected 1920x1080+0+0 (normal left inverted right x > axis y axis) 700mm x 390mm > - 1920x1080 60.00*+ > - 1920x1080i60.0050.00 > + 1920x1080 60.00*+ 50.0059.9430.0025.0024.00 > 29.9723.98 > + 1920x1080i60.0050.0059.94 > 1600x1200 60.00 > 1680x1050 59.88 > 1280x1024 75.0260.02 > @@ -13,30 +13,29 @@ > 1360x768 60.02 > 1280x800 59.91 > 1152x864 75.00 > - 1280x720 60.0050.00 > + 1280x720 60.0050.0059.94 > 1024x768 75.0370.0760.00 > 832x624 74.55 > 800x600 72.1975.0060.32 > - 640x480 75.0072.8166.6759.94 > + 720x576 50.00 > + 720x480 60.0059.94 > + 640x480 75.0072.8166.6760.0059.94 > 720x400 70.08 None of these aspect ratios are new modes / new aspect ratios from HDMI 2.0/CEA-861-F These are the existing modes, and should be independent of reverted patches. >>> They're affected because your patches changed them by adding the aspect >>> ratio flags to them. >> Yes, But they are independent of reverted patch, which adds aspect ratio >> for HDMI 2.0 ratios (64:27 and 256:135) > The second patch had to be reverted so that the first patch would revert > cleanly. > > This was with sna, which does this: > #define KNOWN_MODE_FLAGS ((1<<14)-1) > if (mode->status == MODE_OK && kmode->flags & ~KNOWN_MODE_FLAGS) > mode->status = MODE_BAD; /* unknown flags => unhandled > */ > so all the modes with an aspect ratio just vanished. > > -modesetting and -ati on the other hand just copy over the unknown > bits into the xrandr mode structure, which sounds dubious at best: > mode->Flags = kmode->flags; //& FLAG_BITS; > I've not checked what damage it can actually cause. > > > It looks like a few modes disappeared from the kernel's mode list > as well, presumably because some cea modes in the list originated from > DTDs and whanot so they don't have an aspect ratio and that causes > add_alternate_cea_modes() to ignore them. So not populating an aspect > ratio for cea modes originating from a source other than > edid_cea_modes[] looks like another bug to me as well. I am writing a patch series to cap the aspect ratio implementation under a drm_cap_hdmi2_aspect_ratios This is how its going to work (inspired from the 2D/stereo series from damien L) - Add a new capability hdmi2_ar >>> It should be just a generic "expose aspect ratio flags to userspace?" >> Makes sense, in this way we can even revert the aspect_ratio property >> for HDMI connector, as discussed during >> the code review sessions of this patch series. In this way, when kernel >> will expose the aspect ratios, it will either >> do the aspect ratios as per EDID, or wont. - by default parsing the new hdmi 2.0 aspect ratio will be disabled under check of this cap - during bootup time, while initializing the display, a userspace can get_cap on the hdmi2_aspect_ratio - If it wants HDMI 2.0 aspect ratio support, it will set the cap, and kernel will expose these aspect ratios > Another bug I think might be the ordering of the modes with aspect > ratio > specified. IIRC the spec says that the preferred aspect ratio should > be > listed first in the EDID, but I don't think we preserve that ordering > in the final mode list. I guess we could fix that by somehow noting > which aspect ratio is preferred and sort based on that, or we try to > preserve the order from the EDID until we're ready to sort, and then > do > the sorting with a stable algorithm. >>
BUG: 'list_empty(&vgdev->free_vbufs)' is true!
On Di, 2016-11-15 at 09:55 +0100, Jiri Slaby wrote: > On 11/15/2016, 09:46 AM, Gerd Hoffmann wrote: > > On Fr, 2016-11-11 at 17:28 +0100, Jiri Slaby wrote: > >> On 11/09/2016, 09:01 AM, Gerd Hoffmann wrote: > >>> On Di, 2016-11-08 at 22:37 +0200, Michael S. Tsirkin wrote: > On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote: > > Hi, > > > > I can relatively easily reproduce this bug: > >>> > >>> How? > >> > >> Run dmesg -w in the qemu window (virtio_gpu) to see a lot of output. > > > > fbcon? Or xorg/wayland with terminal app? > > Ah, just console, so fbcon. No X server running. Hmm, /me looks puzzled. fbcon doesn't do cursor updates, so the cursor queue can hardly be full and there should be enough buffers even without allocating 16 extra bufs. I'll go try reproduce and analyze that one. The +16 patch submitted nevertheless as temporary stopgap. cheers, Gerd
[Bug 97403] AMDGPU/Iceland Strange warnings on drm-next-4.9-wip
https://bugs.freedesktop.org/show_bug.cgi?id=97403 --- Comment #6 from rezhu --- Created attachment 127977 --> https://bugs.freedesktop.org/attachment.cgi?id=127977&action=edit fix patch the attached patch is for the warning message. please help to verify. thanks. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/b2bf300a/attachment.html>
[Bug 97403] AMDGPU/Iceland Strange warnings on drm-next-4.9-wip
https://bugs.freedesktop.org/show_bug.cgi?id=97403 --- Comment #7 from rezhu --- when run Talos Principle can you cat the pm info by command: cat /sys/kernel/debug/dri/64/amdgpu_pm_info -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/7a35cb44/attachment.html>
[patch] drm: zte: checking for NULL instead of IS_ERR()
drm_dev_alloc() never returns NULL, it only returns error pointers on
error.
Fixes: 0a886f59528a ("drm: zte: add initial vou drm driver")
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
index abc8099..3e76f72 100644
--- a/drivers/gpu/drm/zte/zx_drm_drv.c
+++ b/drivers/gpu/drm/zte/zx_drm_drv.c
@@ -107,8 +107,8 @@ static int zx_drm_bind(struct device *dev)
return -ENOMEM;
drm = drm_dev_alloc(&zx_drm_driver, dev);
- if (!drm)
- return -ENOMEM;
+ if (IS_ERR(drm))
+ return PTR_ERR(drm);
drm->dev_private = priv;
dev_set_drvdata(dev, drm);
[Bug 98645] X Freeze while rendering video with multiple displays and TearFree enabled
https://bugs.freedesktop.org/show_bug.cgi?id=98645 Michel Dänzer changed: What|Removed |Added QA Contact|xorg-team at lists.x.org | Component|Driver/Radeon |DRM/Radeon Assignee|xorg-driver-ati at lists.x.org |dri-devel at lists.freedesktop ||.org Product|xorg|DRI -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/34993714/attachment.html>
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: > On 11/15/2016 2:21 PM, Daniel Vetter wrote: > > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: > > > In any case, I guess addition of a cap for aspect ratio should fix the > > > current objections for this implementation. > > > > > > And I will keep it 0 by default, so that no aspect ratio information is > > > added until userspace sets the cap to 1 on its own. > > Note that cap = needs new userspace. > > -Daniel > I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. > Is that what you mean ? Full stack solution, including enabling in an Xorg driver (or somewhere else, we also have drm_hwcomposer as an option). And because that's probably going to take forever I'm leaning towards revert again. Ville? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[patch] drm: zte: checking for NULL instead of IS_ERR()
On Tue, Nov 15, 2016 at 12:53:01PM +0300, Dan Carpenter wrote:
> drm_dev_alloc() never returns NULL, it only returns error pointers on
> error.
>
> Fixes: 0a886f59528a ("drm: zte: add initial vou drm driver")
> Signed-off-by: Dan Carpenter
Applied, thx.
-Daniel
>
> diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c
> b/drivers/gpu/drm/zte/zx_drm_drv.c
> index abc8099..3e76f72 100644
> --- a/drivers/gpu/drm/zte/zx_drm_drv.c
> +++ b/drivers/gpu/drm/zte/zx_drm_drv.c
> @@ -107,8 +107,8 @@ static int zx_drm_bind(struct device *dev)
> return -ENOMEM;
>
> drm = drm_dev_alloc(&zx_drm_driver, dev);
> - if (!drm)
> - return -ENOMEM;
> + if (IS_ERR(drm))
> + return PTR_ERR(drm);
>
> drm->dev_private = priv;
> dev_set_drvdata(dev, drm);
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[drm-intel:topic/drm-misc 23/26] include/drm/drm_fb_cma_helper.h:45:13: warning: 'struct drm_plane_state' declared inside parameter list will not be visible outside of this definition or declaration
On Tue, Nov 15, 2016 at 09:47:31AM +0100, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 04:29:04PM +0800, kbuild test robot wrote: > > tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc > > head: 35cf03508d8466ecc5199c9d609e74e85bec785b > > commit: 14d7f96f90fb65c2ca0e0ac7df237e06ff001c29 [23/26] drm/fb_cma_helper: > > Add drm_fb_cma_prepare_fb() helper > > config: i386-allmodconfig (attached as .config) > > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 > > reproduce: > > git checkout 14d7f96f90fb65c2ca0e0ac7df237e06ff001c29 > > # save the attached .config to linux build tree > > make ARCH=i386 > > > > All warnings (new ones prefixed by >>): > > > >In file included from drivers/gpu/drm/vc4/vc4_drv.c:18:0: > > >> include/drm/drm_fb_cma_helper.h:45:13: warning: 'struct drm_plane_state' > > >> declared inside parameter list will not be visible outside of this > > >> definition or declaration > > struct drm_plane_state *state); > > ^~~ > > >> include/drm/drm_fb_cma_helper.h:44:34: warning: 'struct drm_plane' > > >> declared inside parameter list will not be visible outside of this > > >> definition or declaration > > int drm_fb_cma_prepare_fb(struct drm_plane *plane, > > ^ > > Oops, didn't noticed this compiler warning before pushing. Since drm-misc > is non-rebasing, can you pls supply a full fixup patch Marek? Probably just need an #include in drm_fb_cma_helper.h. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
Regards Shashank On 11/15/2016 3:30 PM, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: >> On 11/15/2016 2:21 PM, Daniel Vetter wrote: >>> On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: In any case, I guess addition of a cap for aspect ratio should fix the current objections for this implementation. And I will keep it 0 by default, so that no aspect ratio information is added until userspace sets the cap to 1 on its own. >>> Note that cap = needs new userspace. >>> -Daniel >> I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. >> Is that what you mean ? > Full stack solution, including enabling in an Xorg driver (or somewhere > else, we also have drm_hwcomposer as an option). > > And because that's probably going to take forever I'm leaning towards > revert again. Ville? Not really, with a kernel cap implementation, the code will be as it was before this patch series ( as good as revert ) And then when we want to enable it, we can add the corresponding Xserver patch. I guess the current problem is if is breaks something in some userspace, but if I am loading the flags only when the cap is set, we should be good. Regards Shashank > -Daniel
[Intel-gfx] [PATCH 04/10] drm: Extract drm_drv.h
On Mon, Nov 14, 2016 at 12:58:19PM +0100, Daniel Vetter wrote:
> I want to move dumb buffer documentation into the right vfuncs, and
> for that I first need to be able to pull that into kerneldoc without
> having to clean up all of drmP.h. Also, header-splitting is nice.
>
> While at it shuffle all the function declarations for drm_drv.c into
> the right spots, and drop the kerneldoc for drm_minor_acquire/release
> since it's only used internally.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_drv.c | 18 +--
> drivers/gpu/drm/drm_internal.h | 4 +
> include/drm/drmP.h | 299 +---
> include/drm/drm_drv.h | 337
> +
> 4 files changed, 346 insertions(+), 312 deletions(-)
> create mode 100644 include/drm/drm_drv.h
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 98a083d4b81e..cc6c2530764b 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -32,7 +32,10 @@
> #include
> #include
> #include
> +
> +#include
> #include
> +
> #include "drm_crtc_internal.h"
> #include "drm_legacy.h"
> #include "drm_internal.h"
> @@ -257,10 +260,7 @@ static void drm_minor_unregister(struct drm_device *dev,
> unsigned int type)
> drm_debugfs_cleanup(minor);
> }
>
> -/**
> - * drm_minor_acquire - Acquire a DRM minor
> - * @minor_id: Minor ID of the DRM-minor
> - *
> +/*
> * Looks up the given minor-ID and returns the respective DRM-minor object.
> The
> * refence-count of the underlying device is increased so you must release
> this
> * object with drm_minor_release().
> @@ -268,10 +268,6 @@ static void drm_minor_unregister(struct drm_device *dev,
> unsigned int type)
> * As long as you hold this minor, it is guaranteed that the object and the
> * minor->dev pointer will stay valid! However, the device may get unplugged
> and
> * unregistered while you hold the minor.
> - *
> - * Returns:
> - * Pointer to minor-object with increased device-refcount, or PTR_ERR on
> - * failure.
> */
> struct drm_minor *drm_minor_acquire(unsigned int minor_id)
> {
> @@ -294,12 +290,6 @@ struct drm_minor *drm_minor_acquire(unsigned int
> minor_id)
> return minor;
> }
>
> -/**
> - * drm_minor_release - Release DRM minor
> - * @minor: Pointer to DRM minor object
> - *
> - * Release a minor that was previously acquired via drm_minor_acquire().
> - */
> void drm_minor_release(struct drm_minor *minor)
> {
> drm_dev_unref(minor->dev);
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 1e29cbc556d5..db80ec860e33 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -43,6 +43,10 @@ void drm_prime_destroy_file_private(struct
> drm_prime_file_private *prime_fpriv);
> void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private
> *prime_fpriv,
> struct dma_buf *dma_buf);
>
> +/* drm_drv.c */
> +struct drm_minor *drm_minor_acquire(unsigned int minor_id);
> +void drm_minor_release(struct drm_minor *minor);
> +
> /* drm_info.c */
> int drm_name_info(struct seq_file *m, void *data);
> int drm_clients_info(struct seq_file *m, void* data);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index cfa4b80f0628..b352a7b812e6 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -76,6 +76,7 @@
> #include
> #include
> #include
> +#include
>
> struct module;
>
> @@ -137,34 +138,10 @@ struct dma_buf_attachment;
> #define DRM_UT_VBL 0x20
> #define DRM_UT_STATE 0x40
>
> -extern __printf(6, 7)
> -void drm_dev_printk(const struct device *dev, const char *level,
> - unsigned int category, const char *function_name,
> - const char *prefix, const char *format, ...);
> -
> -extern __printf(3, 4)
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...);
> -
> /***/
> /** \name DRM template customization defaults */
> /*@{*/
>
> -/* driver capabilities and requirements mask */
> -#define DRIVER_USE_AGP 0x1
> -#define DRIVER_LEGACY0x2
> -#define DRIVER_PCI_DMA 0x8
> -#define DRIVER_SG0x10
> -#define DRIVER_HAVE_DMA 0x20
> -#define DRIVER_HAVE_IRQ 0x40
> -#define DRIVER_IRQ_SHARED0x80
> -#define DRIVER_GEM 0x1000
> -#define DRIVER_MODESET 0x2000
> -#define DRIVER_PRIME 0x4000
> -#define DRIVER_RENDER0x8000
> -#define DRIVER_ATOMIC0x1
> -#define DRIVER_KMS_LEGACY_CONTEXT0x2
> -
> /***/
> /** \name Macros to mak
[Intel-gfx] [PATCH 06/10] drm: Consolidate dumb buffer docs
On Mon, Nov 14, 2016 at 12:58:21PM +0100, Daniel Vetter wrote: > Put the callback docs into struct drm_driver, and the small overview > into a DOC comment. > > Signed-off-by: Daniel Vetter Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[Intel-gfx] [PATCH 08/10] drm: Extract drm_mode_config.[hc]
On Mon, Nov 14, 2016 at 12:58:23PM +0100, Daniel Vetter wrote: > And shuffle the kernel-doc structure a bit since drm_crtc.[hc] now > only contains CRTC-related functions and structures. > > Signed-off-by: Daniel Vetter > diff --git a/drivers/gpu/drm/drm_crtc_internal.h > b/drivers/gpu/drm/drm_crtc_internal.h > index 3d23a473ec35..dad212140d56 100644 > --- a/drivers/gpu/drm/drm_crtc_internal.h > +++ b/drivers/gpu/drm/drm_crtc_internal.h > @@ -40,18 +40,25 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc, > int x, int y, > const struct drm_display_mode *mode, > const struct drm_framebuffer *fb); > - > -void drm_fb_release(struct drm_file *file_priv); > +int drm_crtc_register_all(struct drm_device *dev); > +void drm_crtc_unregister_all(struct drm_device *dev); > > /* IOCTLs */ > -int drm_mode_getresources(struct drm_device *dev, > - void *data, struct drm_file *file_priv); > int drm_mode_getcrtc(struct drm_device *dev, >void *data, struct drm_file *file_priv); > int drm_mode_setcrtc(struct drm_device *dev, >void *data, struct drm_file *file_priv); > > + > +/* drm_mode_config.c */ > +/* IOCTLs */ > +int drm_mode_getresources(struct drm_device *dev, > + void *data, struct drm_file *file_priv); > + > + > /* drm_dumb_buffers.c */ > +int drm_modeset_register_all(struct drm_device *dev); > +void drm_modeset_unregister_all(struct drm_device *dev); > I was worried for a moment. Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH 10/10] drm: Drop externs from drm_crtc.h
On Mon, Nov 14, 2016 at 12:58:25PM +0100, Daniel Vetter wrote: > Just noise. > > Signed-off-by: Daniel Vetter Sometimes it is interesting. Practice across the kernel is mixed, but convergence on not putting extern. Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[Intel-gfx] [PATCH 05/10] drm: Clean up kerneldoc for struct drm_driver
On Mon, Nov 14, 2016 at 12:58:20PM +0100, Daniel Vetter wrote: > Just cleans up what's there, still plenty missing. > > Signed-off-by: Daniel Vetter I read it, seems to match my limited understanding of kerneldoc. Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[Intel-gfx] [PATCH 07/10] drm/print: Move kerneldoc next to definition
On Mon, Nov 14, 2016 at 12:58:22PM +0100, Daniel Vetter wrote:
> kerneldoc expects the comment next to definitions, otherwise it can't
> pick up exported vs. internal stuff.
>
> This fixes a warning from the doc build done with:
>
> $ make DOCBOOKS="" htmldocs
>
> Fixes: d8187177b0b1 ("drm: add helper for printing to log or seq_file")
> Cc: Rob Clark
> Cc: Sean Paul
> Signed-off-by: Daniel Vetter
Oh well, I liked the practice of having interface descriptions in the
headers. If they are in the body, I may as well just read the code.
Reviewed-by: Chris Wilson
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH 09/10] drm: Move tile group code into drm_connector.c
On Mon, Nov 14, 2016 at 12:58:24PM +0100, Daniel Vetter wrote: > And also put the overview section into the KMS Properties part of the > docs, instead of randomly-placed within the helpers - this is part of > the uabi. > > With this patch I think drm_crtc.[hc] is cleaned up and entirely > documented. > > Signed-off-by: Daniel Vetter Code motion looks ok, placement inside the rst I'll take you at your word. Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[Intel-gfx] [PATCH 01/10] drm: Extract drm_dumb_buffers.c
On Mon, Nov 14, 2016 at 12:58:16PM +0100, Daniel Vetter wrote:
> Just code movement, doc cleanup will follow up later.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/Makefile| 3 +-
> drivers/gpu/drm/drm_crtc.c | 109 -
> drivers/gpu/drm/drm_crtc_internal.h | 18 ++---
> drivers/gpu/drm/drm_dumb_buffers.c | 135
>
> 4 files changed, 147 insertions(+), 118 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_dumb_buffers.c
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index f217274754d4..adcfc8f922e3 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -15,7 +15,8 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
> drm_modeset_lock.o drm_atomic.o drm_bridge.o \
> drm_framebuffer.o drm_connector.o drm_blend.o \
> drm_encoder.o drm_mode_object.o drm_property.o \
> - drm_plane.o drm_color_mgmt.o drm_print.o
> + drm_plane.o drm_color_mgmt.o drm_print.o \
> + drm_dumb_buffers.o
>
> drm-$(CONFIG_COMPAT) += drm_ioc32.o
> drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 4f34d9a34190..0ece33cc0dc6 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -960,115 +960,6 @@ void drm_mode_config_reset(struct drm_device *dev)
> EXPORT_SYMBOL(drm_mode_config_reset);
>
> /**
> - * drm_mode_create_dumb_ioctl - create a dumb backing storage buffer
> - * @dev: DRM device
> - * @data: ioctl data
> - * @file_priv: DRM file info
> - *
> - * This creates a new dumb buffer in the driver's backing storage manager
> (GEM,
> - * TTM or something else entirely) and returns the resulting buffer handle.
> This
> - * handle can then be wrapped up into a framebuffer modeset object.
> - *
> - * Note that userspace is not allowed to use such objects for render
> - * acceleration - drivers must create their own private ioctls for such a use
> - * case.
> - *
> - * Called by the user via ioctl.
> - *
> - * Returns:
> - * Zero on success, negative errno on failure.
> - */
> -int drm_mode_create_dumb_ioctl(struct drm_device *dev,
> -void *data, struct drm_file *file_priv)
> -{
> - struct drm_mode_create_dumb *args = data;
> - u32 cpp, stride, size;
> -
> - if (!dev->driver->dumb_create)
> - return -ENOSYS;
> - if (!args->width || !args->height || !args->bpp)
> - return -EINVAL;
> -
> - /* overflow checks for 32bit size calculations */
> - /* NOTE: DIV_ROUND_UP() can overflow */
> - cpp = DIV_ROUND_UP(args->bpp, 8);
> - if (!cpp || cpp > 0xU / args->width)
> - return -EINVAL;
> - stride = cpp * args->width;
> - if (args->height > 0xU / stride)
> - return -EINVAL;
> -
> - /* test for wrap-around */
> - size = args->height * stride;
> - if (PAGE_ALIGN(size) == 0)
> - return -EINVAL;
> -
> - /*
> - * handle, pitch and size are output parameters. Zero them out to
> - * prevent drivers from accidentally using uninitialized data. Since
> - * not all existing userspace is clearing these fields properly we
> - * cannot reject IOCTL with garbage in them.
> - */
> - args->handle = 0;
> - args->pitch = 0;
> - args->size = 0;
> -
> - return dev->driver->dumb_create(file_priv, dev, args);
> -}
> -
> -/**
> - * drm_mode_mmap_dumb_ioctl - create an mmap offset for a dumb backing
> storage buffer
> - * @dev: DRM device
> - * @data: ioctl data
> - * @file_priv: DRM file info
> - *
> - * Allocate an offset in the drm device node's address space to be able to
> - * memory map a dumb buffer.
> - *
> - * Called by the user via ioctl.
> - *
> - * Returns:
> - * Zero on success, negative errno on failure.
> - */
> -int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
> - void *data, struct drm_file *file_priv)
> -{
> - struct drm_mode_map_dumb *args = data;
> -
> - /* call driver ioctl to get mmap offset */
> - if (!dev->driver->dumb_map_offset)
> - return -ENOSYS;
> -
> - return dev->driver->dumb_map_offset(file_priv, dev, args->handle,
> &args->offset);
> -}
> -
> -/**
> - * drm_mode_destroy_dumb_ioctl - destroy a dumb backing strage buffer
> - * @dev: DRM device
> - * @data: ioctl data
> - * @file_priv: DRM file info
> - *
> - * This destroys the userspace handle for the given dumb backing storage
> buffer.
> - * Since buffer objects must be reference counted in the kernel a buffer
> object
> - * won't be immediately freed if a framebuffer modeset object still uses it.
> - *
> - * Called by the user via ioctl.
> - *
> - * Returns:
> - * Zero on success, negative errno on failure.
> - */
> -int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
> -
[PATCH 02/10] drm/i915: Fixup kerneldoc includes
On Mon, Nov 14, 2016 at 12:58:17PM +0100, Daniel Vetter wrote:
> Would be great if everony could add
everyone
> $ make DOCBOOKS="" htmldocs
>
> to their build scripts to catch these. 0day should also report them,
> not sure why it failed to spot this.
"make IGNORE_DOCBOOKS=1 SPHINXOPTS=-W htmldocs" is that outdated?
I don't often run it since it is too slow when checking hundreds of
patches. Any chance of an incremental patch checker?
> Fixes: b42fe9ca0a1e ("drm/i915: Split out i915_vma.c")
> Cc: Tvrtko Ursulin
> Cc: Chris Wilson
> Cc: Joonas Lahtinen
> Signed-off-by: Daniel Vetter
Reviewed-by: Chris Wilson
(I'm not even going to ask how it appears three times in the docs and
how that all works :)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH 03/10] doc/dma-buf: Fix up include directives
On Mon, Nov 14, 2016 at 12:58:18PM +0100, Daniel Vetter wrote:
> Would be great if everony could add
>
> $ make DOCBOOKS="" htmldocs
>
> to their build scripts to catch these. 0day should also report them,
> not sure why it failed to spot this.
>
> Fixes: f54d1867005c ("dma-buf: Rename struct fence to dma_fence")
> Cc: Chris Wilson
> Cc: Gustavo Padovan
> Cc: Sumit Semwal
> Cc: Christian König
> Signed-off-by: Daniel Vetter
Weird, I caught some of the stale Documents/, obviously that didn't
trigger a warning that I needed to do a more careful check.
Reviewed: Chris Wilson
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 03:36:02PM +0530, Sharma, Shashank wrote: > On 11/15/2016 3:30 PM, Daniel Vetter wrote: > > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: > > > On 11/15/2016 2:21 PM, Daniel Vetter wrote: > > > > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: > > > > > In any case, I guess addition of a cap for aspect ratio should fix the > > > > > current objections for this implementation. > > > > > > > > > > And I will keep it 0 by default, so that no aspect ratio information > > > > > is > > > > > added until userspace sets the cap to 1 on its own. > > > > Note that cap = needs new userspace. > > > > -Daniel > > > I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. > > > Is that what you mean ? > > Full stack solution, including enabling in an Xorg driver (or somewhere > > else, we also have drm_hwcomposer as an option). > > > > And because that's probably going to take forever I'm leaning towards > > revert again. Ville? > Not really, with a kernel cap implementation, the code will be as it was > before this patch series ( as good as revert ) > And then when we want to enable it, we can add the corresponding Xserver > patch. > > I guess the current problem is if is breaks something in some userspace, but > if I am loading the flags only when the cap is set, we should be good. This is not how new uabi gets merged, see: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements Userspace must be ready, or it will not land. The entire point of my suggestion to just extend the display modes was to avoid the need for userspace, but since existing userspace tries to be too clever already, that didn't work out. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 02/10] drm/i915: Fixup kerneldoc includes
On Tue, Nov 15, 2016 at 10:44:29AM +, Chris Wilson wrote:
> On Mon, Nov 14, 2016 at 12:58:17PM +0100, Daniel Vetter wrote:
> > Would be great if everony could add
>
> everyone
>
> > $ make DOCBOOKS="" htmldocs
> >
> > to their build scripts to catch these. 0day should also report them,
> > not sure why it failed to spot this.
>
> "make IGNORE_DOCBOOKS=1 SPHINXOPTS=-W htmldocs" is that outdated?
IGNORE_DOCBOOKS=1 was renamed to DOCBOOKS="". And incremental builds (at
least in my experience here) are really fast with sphinx (a few seconds at
most). So should be good enough for a git rebase -x type checking.
> I don't often run it since it is too slow when checking hundreds of
> patches. Any chance of an incremental patch checker?
As long as you make sure you entirely avoid the old docbook horror show,
incremental builds should be real fast. The initial build can take a
while, but again if you avoid docbook it's reasonable fast imo. Full
kernel rebuilds are still much worse.
> > Fixes: b42fe9ca0a1e ("drm/i915: Split out i915_vma.c")
> > Cc: Tvrtko Ursulin
> > Cc: Chris Wilson
> > Cc: Joonas Lahtinen
> > Signed-off-by: Daniel Vetter
> Reviewed-by: Chris Wilson
>
> (I'm not even going to ask how it appears three times in the docs and
> how that all works :)
The follow-up paramaters are crucial: First one pulls in function
kernel-docs only, other 2 pull in specific DOC: comment sections.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes
Add the nodes for the MSTPRI configuration and DDR2/mDDR memory
controller drivers to da850.dtsi.
Signed-off-by: Bartosz Golaszewski
---
v1 -> v2:
- moved the priority controller node above the cfgchip node
- renamed added nodes to better reflect their purpose
arch/arm/boot/dts/da850.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 1bb1f6d..412eec6 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -210,6 +210,10 @@
};
};
+ prictrl: priority-controller at 14110 {
+ compatible = "ti,da850-mstpri";
+ reg = <0x14110 0x0c>;
+ };
cfgchip: chip-controller at 1417c {
compatible = "ti,da830-cfgchip", "syscon", "simple-mfd";
reg = <0x1417c 0x14>;
@@ -451,4 +455,8 @@
1 0 0x6800 0x8000>;
status = "disabled";
};
+ memctrl: memory-controller at b000 {
+ compatible = "ti,da850-ddr-controller";
+ reg = <0xb000 0xe8>;
+ };
};
--
2.9.3
[Bug 98629] OpenGL applications warns "MESA-LOADER: failed to retrieve device information"
https://bugs.freedesktop.org/show_bug.cgi?id=98629 Emil Velikov changed: What|Removed |Added Product|Mesa|DRI Resolution|--- |FIXED Assignee|mesa-dev at lists.freedesktop. |dri-devel at lists.freedesktop |org |.org Status|NEW |RESOLVED Component|Mesa core |libdrm Version|13.0|unspecified QA Contact|mesa-dev at lists.freedesktop. | |org | --- Comment #6 from Emil Velikov --- Fixed with the following commit, which is part of libdrm 2.4.73. Thanks for the report and testing ! commit f53d3542c1dfa2a1c1a5a7155d058df9a6bcce7b Author: Emil Velikov Date: Fri Nov 11 19:04:11 2016 + xd86drm: read more than 128 bytes of uevent in drmParsePciBusInfo -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/0d1db750/attachment.html>
[PATCH] drm/fb_cma_helper: Add missing forward declaration
On Tue, Nov 15, 2016 at 11:55:29AM +0100, Marek Vasut wrote: > Add missing forward declaration for struct drm_plane and drm_plane_state, > which causes the following warning in the VC4 driver (can be replicated > by building using bcm2835_defconfig): > > In file included from drivers/gpu/drm/vc4/vc4_drv.c:18:0: > include/drm/drm_fb_cma_helper.h:45:13: warning: âstruct drm_plane_stateâ > declared inside parameter list will not be visible outside of this definition > or declaration > struct drm_plane_state *state); > ^~~ > include/drm/drm_fb_cma_helper.h:44:34: warning: âstruct drm_planeâ > declared inside parameter list will not be visible outside of this definition > or declaration > int drm_fb_cma_prepare_fb(struct drm_plane *plane, > > Signed-off-by: Marek Vasut > Cc: Daniel Vetter Applied to drm-misc, thx for the quick fixup. -Daniel > --- > include/drm/drm_fb_cma_helper.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h > index cc82c73..3b00f64 100644 > --- a/include/drm/drm_fb_cma_helper.h > +++ b/include/drm/drm_fb_cma_helper.h > @@ -12,6 +12,8 @@ struct drm_fb_helper; > struct drm_device; > struct drm_file; > struct drm_mode_fb_cmd2; > +struct drm_plane; > +struct drm_plane_state; > > struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, > unsigned int preferred_bpp, unsigned int num_crtc, > -- > 2.10.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Intel-gfx] [PATCH 01/10] drm: Extract drm_dumb_buffers.c
On Tue, Nov 15, 2016 at 10:42:08AM +, Chris Wilson wrote: > On Mon, Nov 14, 2016 at 12:58:16PM +0100, Daniel Vetter wrote: > > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c > > b/drivers/gpu/drm/drm_dumb_buffers.c > > new file mode 100644 > > index ..4b4364b61c8d > > --- /dev/null > > +++ b/drivers/gpu/drm/drm_dumb_buffers.c > > @@ -0,0 +1,135 @@ > > +/* > > + * Copyright (c) 2016 Intel Corporation > > I've mentioned this elsewhere, but we should also retain the original > copyright statements for the code we are copying. Given that we're super-not-dutiful with updating them I figured point at git log with rename detection is good enough. But fixed (same for the later ones). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH libdrm] automake: make the build less chatty
Having the "Entering|Leaving directory X" messages it not required nor
useful in vast majority of the cases.
One can always have them printed by `make -w' or by overriding the
AM_MAKEFLAGS variable.
Signed-off-by: Emil Velikov
---
Makefile.am | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile.am b/Makefile.am
index 2e46bde..dfb8fcd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -22,6 +22,7 @@ include Makefile.sources
ACLOCAL_AMFLAGS = -I m4 ${ACLOCAL_FLAGS}
+AM_MAKEFLAGS = -s
AM_DISTCHECK_CONFIGURE_FLAGS = \
--enable-udev \
--enable-libkms \
--
2.10.2
[Intel-gfx] [PATCH 07/10] drm/print: Move kerneldoc next to definition
On Tue, Nov 15, 2016 at 10:37:26AM +, Chris Wilson wrote:
> On Mon, Nov 14, 2016 at 12:58:22PM +0100, Daniel Vetter wrote:
> > kerneldoc expects the comment next to definitions, otherwise it can't
> > pick up exported vs. internal stuff.
> >
> > This fixes a warning from the doc build done with:
> >
> > $ make DOCBOOKS="" htmldocs
> >
> > Fixes: d8187177b0b1 ("drm: add helper for printing to log or seq_file")
> > Cc: Rob Clark
> > Cc: Sean Paul
> > Signed-off-by: Daniel Vetter
>
> Oh well, I liked the practice of having interface descriptions in the
> headers. If they are in the body, I may as well just read the code.
I'm divided. On one hand it makes it more easily readable for users of the
code&functions. Otoh having it closer might increase the odds that it's
not forgotten when the semantics change. Personally I just have lots of
windows open, with both code and rendered docs ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Bug 185681] amdgpu: powerplay initialization failed
https://bugzilla.kernel.org/show_bug.cgi?id=185681 --- Comment #17 from René Linder --- For Me the new Patches works fine ... except the message it didn't found default frequency see dmesg log. But re clocking and everything i have on my notebook works fine. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 185681] amdgpu: powerplay initialization failed
https://bugzilla.kernel.org/show_bug.cgi?id=185681 --- Comment #18 from René Linder --- Created attachment 244601 --> https://bugzilla.kernel.org/attachment.cgi?id=244601&action=edit Teted with latest Patches on 4.9rc4 -- You are receiving this mail because: You are watching the assignee of the bug.
[Intel-gfx] [PATCH 07/10] drm/print: Move kerneldoc next to definition
On Tue, Nov 15, 2016 at 12:53:48PM +0100, Daniel Vetter wrote:
> On Tue, Nov 15, 2016 at 10:37:26AM +, Chris Wilson wrote:
> > On Mon, Nov 14, 2016 at 12:58:22PM +0100, Daniel Vetter wrote:
> > > kerneldoc expects the comment next to definitions, otherwise it can't
> > > pick up exported vs. internal stuff.
> > >
> > > This fixes a warning from the doc build done with:
> > >
> > > $ make DOCBOOKS="" htmldocs
> > >
> > > Fixes: d8187177b0b1 ("drm: add helper for printing to log or seq_file")
> > > Cc: Rob Clark
> > > Cc: Sean Paul
> > > Signed-off-by: Daniel Vetter
> >
> > Oh well, I liked the practice of having interface descriptions in the
> > headers. If they are in the body, I may as well just read the code.
>
> I'm divided. On one hand it makes it more easily readable for users of the
> code&functions. Otoh having it closer might increase the odds that it's
> not forgotten when the semantics change. Personally I just have lots of
> windows open, with both code and rendered docs ...
And merged up to this one, with addressing your feedback re copyright
notices. Somehow there's a nasty conflict with the extraction in patch 8
that I haven't figured out yet, needs more coffee. Thanks for your review.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH 01/10] drm: Extract drm_dumb_buffers.c
On Tue, Nov 15, 2016 at 12:47:31PM +0100, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 10:42:08AM +, Chris Wilson wrote: > > On Mon, Nov 14, 2016 at 12:58:16PM +0100, Daniel Vetter wrote: > > > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c > > > b/drivers/gpu/drm/drm_dumb_buffers.c > > > new file mode 100644 > > > index ..4b4364b61c8d > > > --- /dev/null > > > +++ b/drivers/gpu/drm/drm_dumb_buffers.c > > > @@ -0,0 +1,135 @@ > > > +/* > > > + * Copyright (c) 2016 Intel Corporation > > > > I've mentioned this elsewhere, but we should also retain the original > > copyright statements for the code we are copying. > > Given that we're super-not-dutiful with updating them I figured point at > git log with rename detection is good enough. But fixed (same for the > later ones). I agree that git gives more traceablity to authorship (if not necessarily to whom that author has transfered the copyright for the work), but I feel if we are adding a blanket copyright clause we should recognise the validity of the earlier ones as well. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[bug report] drm/amd/powerplay/smu7: fix checks in smu7_get_evv_voltages (v2)
Hello Alex Deucher,
This is a semi-automatic email about new static checker warnings.
The patch 0f12f73c5175: "drm/amd/powerplay/smu7: fix checks in
smu7_get_evv_voltages (v2)" from Nov 9, 2016, leads to the following
Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c:1486
smu7_get_evv_voltages()
warn: variable dereferenced before check 'table_info' (see line 1483)
drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c
1482 && !phm_get_sclk_for_voltage_evv(hwmgr,
1483
table_info->vddgfx_lookup_table, vv_id, &sclk)) {
^^^
1484 if
(phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
1485
PHM_PlatformCaps_ClockStretcher)) {
1486 if (table_info == NULL)
^^
We can't reach this check unless we have already dereferenced
"table_info" so something must be wrong.
1487 return -EINVAL;
1488 sclk_table =
table_info->vdd_dep_on_sclk;
regards,
dan carpenter
[PATCH] drm/amd/powerplay: check if table_info is NULL before dereferencing it
From: Colin Ian King
table_info is being dereferenced before a null check, which implies
a potential null pointer deference error. Fix this by moving the null
check of table_info to the start of smu7_get_evv_voltages to avoid
potential null pointer deferencing.
Found with static analysis by CoverityScan, CID 1377752
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 28e748d..6798067 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -1473,6 +1473,8 @@ static int smu7_get_evv_voltages(struct pp_hwmgr *hwmgr)
(struct phm_ppt_v1_information *)hwmgr->pptable;
struct phm_ppt_v1_clock_voltage_dependency_table *sclk_table = NULL;
+ if (table_info == NULL)
+ return -EINVAL;
for (i = 0; i < SMU7_MAX_LEAKAGE_COUNT; i++) {
vv_id = ATOM_VIRTUAL_VOLTAGE_ID0 + i;
@@ -1483,8 +1485,6 @@ static int smu7_get_evv_voltages(struct pp_hwmgr *hwmgr)
table_info->vddgfx_lookup_table, vv_id, &sclk)) {
if
(phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
PHM_PlatformCaps_ClockStretcher)) {
- if (table_info == NULL)
- return -EINVAL;
sclk_table =
table_info->vdd_dep_on_sclk;
for (j = 1; j < sclk_table->count; j++)
{
@@ -1517,8 +1517,6 @@ static int smu7_get_evv_voltages(struct pp_hwmgr *hwmgr)
table_info->vddc_lookup_table, vv_id,
&sclk)) {
if
(phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
PHM_PlatformCaps_ClockStretcher)) {
- if (table_info == NULL)
- return -EINVAL;
sclk_table =
table_info->vdd_dep_on_sclk;
for (j = 1; j < sclk_table->count; j++)
{
--
2.10.2
[PATCH v12 0/3] drm: add explicit fencing
From: Gustavo Padovan Hi, Yet another iteration, v12 now after working on the changes proposed by Chris Wilson. Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and added support to fences. Current patches can be seen here: https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1 He ran AOSP on top of padovan/fences kernel branch with full fence support on qemu/virgl and msm db410c. That means we already have a working open source userspace using the explicit fencing implementation. Also i-g-t testing are available with all tests suggested in v7 included: https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/ Please review! Gustavo [1] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1253822.html Gustavo Padovan (3): drm/fence: add in-fences support drm/fence: add fence timeline to drm_crtc drm/fence: add out-fences support drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_atomic.c| 255 +--- drivers/gpu/drm/drm_atomic_helper.c | 5 + drivers/gpu/drm/drm_crtc.c | 52 drivers/gpu/drm/drm_crtc_internal.h | 10 ++ drivers/gpu/drm/drm_plane.c | 1 + include/drm/drm_atomic.h| 1 + include/drm/drm_crtc.h | 40 ++ 8 files changed, 320 insertions(+), 45 deletions(-) -- 2.5.5
[PATCH v12 1/3] drm/fence: add in-fences support
From: Gustavo Padovan
There is now a new property called IN_FENCE_FD attached to every plane
state that receives sync_file fds from userspace via the atomic commit
IOCTL.
The fd is then translated to a fence (that may be a fence_array
subclass or just a normal fence) and then used by DRM to fence_wait() for
all fences in the sync_file to signal. So it only commits when all
framebuffers are ready to scanout.
v2: Comments by Daniel Vetter:
- remove set state->fence = NULL in destroy phase
- accept fence -1 as valid and just return 0
- do not call fence_get() - sync_file_fences_get() already calls it
- fence_put() if state->fence is already set, in case userspace
set the property more than once.
v3: WARN_ON if fence is set but state has no FB
v4: Comment from Maarten Lankhorst
- allow set fence with no related fb
v5: rename FENCE_FD to IN_FENCE_FD
v6: Comments by Daniel Vetter:
- rename plane_state->in_fence back to "fence"
- re-introduce WARN_ON if fence set but no fb
- rebase after fence -> dma_fence rename
v7: Comments by Brian Starkey
- set state->fence to NULL when duplicating the state
- fail if IN_FENCE_FD was already set
v8: rebase against latest drm-misc
Signed-off-by: Gustavo Padovan
Reviewed-by: Brian Starkey
Reviewed-by: Sean Paul
Tested-by: Robert Foss
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/drm_atomic.c| 14 ++
drivers/gpu/drm/drm_atomic_helper.c | 5 +
drivers/gpu/drm/drm_crtc.c | 6 ++
drivers/gpu/drm/drm_plane.c | 1 +
include/drm/drm_crtc.h | 5 +
6 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 863cdca..95fc041 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -12,6 +12,7 @@ menuconfig DRM
select I2C
select I2C_ALGOBIT
select DMA_SHARED_BUFFER
+ select SYNC_FILE
help
Kernel-level support for the Direct Rendering Infrastructure (DRI)
introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 57e0a6e9..3ad780a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
@@ -712,6 +713,17 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
drm_atomic_set_fb_for_plane(state, fb);
if (fb)
drm_framebuffer_unreference(fb);
+ } else if (property == config->prop_in_fence_fd) {
+ if (state->fence)
+ return -EINVAL;
+
+ if (U642I64(val) == -1)
+ return 0;
+
+ state->fence = sync_file_get_fence(val);
+ if (!state->fence)
+ return -EINVAL;
+
} else if (property == config->prop_crtc_id) {
struct drm_crtc *crtc = drm_crtc_find(dev, val);
return drm_atomic_set_crtc_for_plane(state, crtc);
@@ -773,6 +785,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
if (property == config->prop_fb_id) {
*val = (state->fb) ? state->fb->base.id : 0;
+ } else if (property == config->prop_in_fence_fd) {
+ *val = -1;
} else if (property == config->prop_crtc_id) {
*val = (state->crtc) ? state->crtc->base.id : 0;
} else if (property == config->prop_crtc_x) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 5007796..0b16587 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3072,6 +3072,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct
drm_plane *plane,
if (state->fb)
drm_framebuffer_reference(state->fb);
+
+ state->fence = NULL;
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
@@ -3110,6 +3112,9 @@ void __drm_atomic_helper_plane_destroy_state(struct
drm_plane_state *state)
{
if (state->fb)
drm_framebuffer_unreference(state->fb);
+
+ if (state->fence)
+ dma_fence_put(state->fence);
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b082763..c19ecc2 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -395,6 +395,12 @@ static int drm_mode_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_fb_id = prop;
+ prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC,
+ "IN_FENCE_FD", -1, INT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_in_fence_fd = prop;
+
prop = drm
[PATCH v12 2/3] drm/fence: add fence timeline to drm_crtc
From: Gustavo Padovan
Create one timeline context for each CRTC to be able to handle out-fences
and signal them. It adds a few members to struct drm_crtc: fence_context,
where we store the context we get from fence_context_alloc(), the
fence seqno and the fence lock, that we pass in fence_init() to be
used by the fence.
v2: Comment by Daniel Stone:
- add BUG_ON() to fence_to_crtc() macro
v3: Comment by Ville Syrjälä
- Use more meaningful name as crtc timeline name
v4: Comments by Brian Starkey
- Use even more meaninful name for the crtc timeline
- add doc for timeline_name
Comment by Daniel Vetter
- use in-line style for comments
- rebase after fence -> dma_fence rename
v5: Comment by Daniel Vetter
- Add doc for drm_crtc_fence_ops
v6: Comment by Chris Wilson
- Move fence_to_crtc to drm_crtc.c
- Move export of drm_crtc_fence_ops to drm_crtc_internal.h
- rebase against latest drm-misc
Signed-off-by: Gustavo Padovan
Reviewed-by: Daniel Vetter
Reviewed-by: Sean Paul
Tested-by: Robert Foss
---
drivers/gpu/drm/drm_crtc.c | 38 +
drivers/gpu/drm/drm_crtc_internal.h | 10 ++
include/drm/drm_crtc.h | 29
3 files changed, 77 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index c19ecc2..02e9451 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -163,6 +164,38 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
#endif
}
+static struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
+{
+ BUG_ON(fence->ops != &drm_crtc_fence_ops);
+ return container_of(fence->lock, struct drm_crtc, fence_lock);
+}
+
+static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->dev->driver->name;
+}
+
+static const char *drm_crtc_fence_get_timeline_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->timeline_name;
+}
+
+static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
+{
+ return true;
+}
+
+const struct dma_fence_ops drm_crtc_fence_ops = {
+ .get_driver_name = drm_crtc_fence_get_driver_name,
+ .get_timeline_name = drm_crtc_fence_get_timeline_name,
+ .enable_signaling = drm_crtc_fence_enable_signaling,
+ .wait = dma_fence_default_wait,
+};
+
/**
* drm_crtc_init_with_planes - Initialise a new CRTC object with
*specified primary and cursor planes.
@@ -220,6 +253,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
struct drm_crtc *crtc,
return -ENOMEM;
}
+ crtc->fence_context = dma_fence_context_alloc(1);
+ spin_lock_init(&crtc->fence_lock);
+ snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
+"CRTC:%d-%s", crtc->base.id, crtc->name);
+
crtc->base.properties = &crtc->properties;
list_add_tail(&crtc->head, &config->crtc_list);
diff --git a/drivers/gpu/drm/drm_crtc_internal.h
b/drivers/gpu/drm/drm_crtc_internal.h
index 64bb3eb..3130bc3 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -43,6 +43,16 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
void drm_fb_release(struct drm_file *file_priv);
+extern const struct dma_fence_ops drm_crtc_fence_ops;
+
+/* dumb buffer support IOCTLs */
+int drm_mode_create_dumb_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *file_priv);
+int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
+void *data, struct drm_file *file_priv);
+int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *file_priv);
+
/* IOCTLs */
int drm_mode_getresources(struct drm_device *dev,
void *data, struct drm_file *file_priv);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 11780a9..edd2d83 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -739,6 +739,35 @@ struct drm_crtc {
*/
struct drm_crtc_crc crc;
#endif
+
+ /**
+* @fence_context:
+*
+* timeline context used for fence operations.
+*/
+ unsigned int fence_context;
+
+ /**
+* @fence_lock:
+*
+* spinlock to protect the fences in the fence_context.
+*/
+
+ spinlock_t fence_lock;
+ /**
+* @fence_seqno:
+*
+* Seqno variable used as monotonic counter for the fences
+* created on the CRTC's timeline.
+*/
+ unsigned long fence_seqno;
+
+ /**
+* @timeline_name:
+*
+* The name of the CRTC's fence time
[PATCH v12 3/3] drm/fence: add out-fences support
From: Gustavo Padovan
Support DRM out-fences by creating a sync_file with a fence for each CRTC
that sets the OUT_FENCE_PTR property.
We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
the sync_file fd back to userspace.
The sync_file and fd are allocated/created before commit, but the
fd_install operation only happens after we know that commit succeed.
v2: Comment by Rob Clark:
- Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
Comment by Daniel Vetter:
- Add clean up code for out_fences
v3: Comments by Daniel Vetter:
- create DRM_MODE_ATOMIC_EVENT_MASK
- userspace should fill out_fences_ptr with the crtc_ids for which
it wants fences back.
v4: Create OUT_FENCE_PTR properties and remove old approach.
v5: Comments by Brian Starkey:
- Remove extra fence_get() in atomic_ioctl()
- Check ret before iterating on the crtc_state
- check ret before fd_install
- set fence_state to NULL at the beginning
- check fence_state->out_fence_ptr before put_user()
- change order of fput() and put_unused_fd() on failure
- Add access_ok() check to the out_fence_ptr received
- Rebase after fence -> dma_fence rename
- Store out_fence_ptr in the drm_atomic_state
- Split crtc_setup_out_fence()
- return -1 as out_fence with TEST_ONLY flag
v6: Comments by Daniel Vetter
- Add prepare/unprepare_crtc_signaling()
- move struct drm_out_fence_state to drm_atomic.c
- mark get_crtc_fence() as static
Comments by Brian Starkey
- proper set fence_ptr fence_state array
- isolate fence_idx increment
- improve error handling
v7: Comments by Daniel Vetter
- remove prefix from internal functions
- make out_fence_ptr an s64 pointer
- degrade DRM_INFO to DRM_DEBUG_ATOMIC when put_user fail
- fix doc issues
- filter out OUT_FENCE_PTR == NULL and do not fail in this case
- add complete_crtc_signalling()
- krealloc fence_state on demand
Comment by Brian Starkey
- remove unused crtc_state arg from get_out_fence()
v8: Comment by Brian Starkey
- cancel events before check for !fence_state
- convert a few lefovers u64 types for out_fence_ptr
- fix memleak by assign fence_state earlier after realloc
- proper accout num_fences in case of error
v9: Comment by Brian Starkey
- memset last position of fence_state after krealloc
Comments by Sean Paul
- pass install_fds in complete_crtc_signaling() instead of ret
- put_user(-1, fence_ptr) when decoding props
v10: Comment by Brian Starkey
- remove unneeded num_fences increment on error path
- kfree fence_state after installing fences fd
v11: rebase against latest drm-misc
Signed-off-by: Gustavo Padovan
Reviewed-by: Brian Starkey
Tested-by: Robert Foss
---
drivers/gpu/drm/drm_atomic.c | 241 +++
drivers/gpu/drm/drm_crtc.c | 8 ++
include/drm/drm_atomic.h | 1 +
include/drm/drm_crtc.h | 6 ++
4 files changed, 211 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3ad780a..d4a92a9 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -290,6 +290,23 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
}
EXPORT_SYMBOL(drm_atomic_get_crtc_state);
+static void set_out_fence_for_crtc(struct drm_atomic_state *state,
+ struct drm_crtc *crtc, s64 __user *fence_ptr)
+{
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
+}
+
+static s64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
+ struct drm_crtc *crtc)
+{
+ s64 __user *fence_ptr;
+
+ fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
+
+ return fence_ptr;
+}
+
/**
* drm_atomic_set_mode_for_crtc - set mode for CRTC
* @state: the CRTC whose incoming state to update
@@ -494,6 +511,16 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
+ } else if (property == config->prop_out_fence_ptr) {
+ s64 __user *fence_ptr = u64_to_user_ptr(val);
+
+ if (!fence_ptr)
+ return 0;
+
+ if (put_user(-1, fence_ptr))
+ return -EFAULT;
+
+ set_out_fence_for_crtc(state->state, crtc, fence_ptr);
} else if (crtc->funcs->atomic_set_property)
return crtc->funcs->atomic_set_property(crtc, state, property,
val);
else
@@ -536,6 +563,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = (s
[Bug 97980] [amdgpu] New kernel warning during shutdown
https://bugs.freedesktop.org/show_bug.cgi?id=97980 --- Comment #14 from Mike Lothian --- I've tested this again with the latest drm-next-4.10-wip branch and I still get the same errors -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/29745a90/attachment.html>
[Intel-gfx] [PATCH 02/12] tests/kms_atomic_transition: don't assume max pipes
On 15 November 2016 at 09:01, Daniel Vetter wrote:
> On Mon, Nov 14, 2016 at 06:59:16PM +0900, Gustavo Padovan wrote:
>> From: Gustavo Padovan
>>
>> Signed-off-by: Gustavo Padovan
>> ---
>> tests/kms_atomic_transition.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
>> index e693c88..8b26b53 100644
>> --- a/tests/kms_atomic_transition.c
>> +++ b/tests/kms_atomic_transition.c
>> @@ -404,7 +404,7 @@ static void run_modeset_tests(igt_display_t *display,
>> int howmany, bool nonblock
>> {
>> struct igt_fb fbs[2];
>> int i, j;
>> - unsigned iter_max = 1 << I915_MAX_PIPES;
>> + unsigned iter_max = 1 << display->n_pipes;
>
> Didn't Tomeu have some patch series to fix these all up?
Don't remember, and couldn't find any within my local branches. Maybe
Robert? But I'm adding it to my backlog anyway.
Regards,
Tomeu
> -Daniel
>
>> igt_pipe_crc_t *pipe_crcs[I915_MAX_PIPES];
>> igt_output_t *output;
>> unsigned width = 0, height = 0;
>> --
>> 2.5.5
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 11:00:04AM +0100, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: > > On 11/15/2016 2:21 PM, Daniel Vetter wrote: > > > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: > > > > In any case, I guess addition of a cap for aspect ratio should fix the > > > > current objections for this implementation. > > > > > > > > And I will keep it 0 by default, so that no aspect ratio information is > > > > added until userspace sets the cap to 1 on its own. > > > Note that cap = needs new userspace. > > > -Daniel > > I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. > > Is that what you mean ? > > Full stack solution, including enabling in an Xorg driver (or somewhere > else, we also have drm_hwcomposer as an option). > > And because that's probably going to take forever I'm leaning towards > revert again. Ville? Yeah I guess we'll need to push the revert to avoid the regression. Trying to put in new client caps and whatnot after -rc5 doesn't seem like a viable option to me. -- Ville Syrjälä Intel OTC
[Bug 98492] X-Plane 10 Core Dumping when using Real-Weather or any clouds
https://bugs.freedesktop.org/show_bug.cgi?id=98492 --- Comment #5 from Nicolai Hähnle --- So I'm confused, you're seeing hangs now? Judging by another X-Plane 10-related bug report, you should run with the MESA_EXTENSION_OVERRIDE=-GL_AMD_pinned_memory environment variable setting. If you're still seeing crashes, please install debug symbols for the radeonsi driver and run in gdb. I.e.: `MESA_EXTENSION_OVERRIDE=-GL_AMD_pinned_memory gdb X-Plane-x86_64`, and then `run --force_run`. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/d2a5fc78/attachment.html>
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 03:54:42PM +0200, Ville Syrjälä wrote: > On Tue, Nov 15, 2016 at 11:00:04AM +0100, Daniel Vetter wrote: > > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: > > > On 11/15/2016 2:21 PM, Daniel Vetter wrote: > > > > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: > > > > > In any case, I guess addition of a cap for aspect ratio should fix the > > > > > current objections for this implementation. > > > > > > > > > > And I will keep it 0 by default, so that no aspect ratio information > > > > > is > > > > > added until userspace sets the cap to 1 on its own. > > > > Note that cap = needs new userspace. > > > > -Daniel > > > I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. > > > Is that what you mean ? > > > > Full stack solution, including enabling in an Xorg driver (or somewhere > > else, we also have drm_hwcomposer as an option). > > > > And because that's probably going to take forever I'm leaning towards > > revert again. Ville? > > Yeah I guess we'll need to push the revert to avoid the regression. > Trying to put in new client caps and whatnot after -rc5 doesn't seem > like a viable option to me. Yeah, a few days left to get userspace in line is just not enough. Agreed and reverts applied. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 9:03 AM, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 03:54:42PM +0200, Ville Syrjälä wrote: >> On Tue, Nov 15, 2016 at 11:00:04AM +0100, Daniel Vetter wrote: >> > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: >> > > On 11/15/2016 2:21 PM, Daniel Vetter wrote: >> > > > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: >> > > > > In any case, I guess addition of a cap for aspect ratio should fix >> > > > > the >> > > > > current objections for this implementation. >> > > > > >> > > > > And I will keep it 0 by default, so that no aspect ratio information >> > > > > is >> > > > > added until userspace sets the cap to 1 on its own. >> > > > Note that cap = needs new userspace. >> > > > -Daniel >> > > I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. >> > > Is that what you mean ? >> > >> > Full stack solution, including enabling in an Xorg driver (or somewhere >> > else, we also have drm_hwcomposer as an option). >> > >> > And because that's probably going to take forever I'm leaning towards >> > revert again. Ville? >> >> Yeah I guess we'll need to push the revert to avoid the regression. >> Trying to put in new client caps and whatnot after -rc5 doesn't seem >> like a viable option to me. > > Yeah, a few days left to get userspace in line is just not enough. Agreed > and reverts applied. > Is there any way we can add the new CEA modes and worry about handling the aspect ratio stuff later? Alex
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 01:48:04PM +, Jose Abreu wrote: > Hi, > > > > On 15-11-2016 10:52, Daniel Vetter wrote: > > On Tue, Nov 15, 2016 at 03:36:02PM +0530, Sharma, Shashank wrote: > >> On 11/15/2016 3:30 PM, Daniel Vetter wrote: > >>> On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: > On 11/15/2016 2:21 PM, Daniel Vetter wrote: > > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: > >> In any case, I guess addition of a cap for aspect ratio should fix the > >> current objections for this implementation. > >> > >> And I will keep it 0 by default, so that no aspect ratio information is > >> added until userspace sets the cap to 1 on its own. > > Note that cap = needs new userspace. > > -Daniel > I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. > Is that what you mean ? > >>> Full stack solution, including enabling in an Xorg driver (or somewhere > >>> else, we also have drm_hwcomposer as an option). > >>> > >>> And because that's probably going to take forever I'm leaning towards > >>> revert again. Ville? > >> Not really, with a kernel cap implementation, the code will be as it was > >> before this patch series ( as good as revert ) > >> And then when we want to enable it, we can add the corresponding Xserver > >> patch. > >> > >> I guess the current problem is if is breaks something in some userspace, > >> but > >> if I am loading the flags only when the cap is set, we should be good. > > This is not how new uabi gets merged, see: > > > > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements > > > > Userspace must be ready, or it will not land. The entire point of my > > suggestion to just extend the display modes was to avoid the need for > > userspace, but since existing userspace tries to be too clever already, > > that didn't work out. > > Thanks, Daniel > > @Ville > > I gave my tested by to these patches because I was facing the > same scenario as Shashank: HDMI compliance. I believe we need to > find a better way to handle this instead of just reverting. The > HDMI spec is evolving so we also need to evolve. Of course that > if this is breaking user space then we can revert and wait until > we figure the best way to implement this. > > Anyway, this is a wild guess but I think the reason you are > loosing modes may be because of > drm_mode_equal_no_clocks_no_stereo() which is always expecting a > aspect ratio flag to be defined. If there isn't one defined then > it will unconditionally return false, even if the modes are > equal. I am well aware why we're losing modes. One reason is the mode equal checks in the kernel as you suspect, the other is simply userspace rejecting everything with the aspect ratio defined. > Can you please test this patch and see if it fixes on your > side? WARNING: Not compile tested I already did something like that earlier, except I rewrote the entire messy mode matching code to use a cleaner flag based approach: git://github.com/vsyrjala/linux.git cea_f_vics But that doesn't solve the userspace ABI issue, and there are still a lot of unanswered questions like: - Should we define aspect ratios for modes not directly coming from edid_cea_modes[]? I beleive the answer must be "yes" at least in the cases where the EDID lists the VIC even if we then skip adding the mode due to already having it on the list via from another source. Perhaps we want the aspect ratio even if the VIC wasn't directly specified? - Should we or should we not specify a VIC in the AVI infoframe when the userspace doesn't support aspect ratio flags? I would guess the answer is again "yes", and we should just pick the preferred aspect ratio for the mode. At least that's closer to what we've been doing up to now (except we just picked the first VIC, so we might have picked the non-preferred aspect ratio actually). At least having the VIC in the infoframe would seem like a safer option than not having it, in case there's some cheap ass hardware that can't do anything sensible without being hand fed a VIC. - How we should handle the aspect ratio in mode sorting? I think we want some sensible sorting order for these. Preferred aspect ratio first would seem like the obvious choice. I do realize that we don't sort based on 3D stereo flags either, but I thinking we probably should. -- Ville Syrjälä Intel OTC
[Bug 98645] X Freeze while rendering video with multiple displays and TearFree enabled
https://bugs.freedesktop.org/show_bug.cgi?id=98645 --- Comment #7 from Alex Deucher --- MSI problems tend to be platform problems. What system is this? What system chipset? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/46691c55/attachment.html>
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 09:18:03AM -0500, Alex Deucher wrote: > On Tue, Nov 15, 2016 at 9:03 AM, Daniel Vetter wrote: > > On Tue, Nov 15, 2016 at 03:54:42PM +0200, Ville Syrjälä wrote: > >> On Tue, Nov 15, 2016 at 11:00:04AM +0100, Daniel Vetter wrote: > >> > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: > >> > > On 11/15/2016 2:21 PM, Daniel Vetter wrote: > >> > > > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: > >> > > > > In any case, I guess addition of a cap for aspect ratio should fix > >> > > > > the > >> > > > > current objections for this implementation. > >> > > > > > >> > > > > And I will keep it 0 by default, so that no aspect ratio > >> > > > > information is > >> > > > > added until userspace sets the cap to 1 on its own. > >> > > > Note that cap = needs new userspace. > >> > > > -Daniel > >> > > I guess you mean a new libdrm, so yes, I will add this new cap in > >> > > libdrm. > >> > > Is that what you mean ? > >> > > >> > Full stack solution, including enabling in an Xorg driver (or somewhere > >> > else, we also have drm_hwcomposer as an option). > >> > > >> > And because that's probably going to take forever I'm leaning towards > >> > revert again. Ville? > >> > >> Yeah I guess we'll need to push the revert to avoid the regression. > >> Trying to put in new client caps and whatnot after -rc5 doesn't seem > >> like a viable option to me. > > > > Yeah, a few days left to get userspace in line is just not enough. Agreed > > and reverts applied. > > > > Is there any way we can add the new CEA modes and worry about handling > the aspect ratio stuff later? I don't think there's any dependency between the two. Or am I overlooking something? -- Ville Syrjälä Intel OTC
[PATCH v12 0/3] drm: add explicit fencing
On Tue, Nov 15, 2016 at 8:06 AM, Gustavo Padovan wrote: > From: Gustavo Padovan > > Hi, > > Yet another iteration, v12 now after working on the changes proposed by Chris > Wilson. > > Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and > added support to fences. Current patches can be seen here: > > https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1 > > He ran AOSP on top of padovan/fences kernel branch with full fence support on > qemu/virgl and msm db410c. That means we already have a working open source > userspace using the explicit fencing implementation. > > Also i-g-t testing are available with all tests suggested in v7 included: > > https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/ > > Please review! > > Gustavo > > [1] https://www.mail-archive.com/linux-kernel at > vger.kernel.org/msg1253822.html > > Gustavo Padovan (3): > drm/fence: add in-fences support > drm/fence: add fence timeline to drm_crtc > drm/fence: add out-fences support > Thanks Gustavo, everything looks good to me. For the series: Reviewed-by: Sean Paul > drivers/gpu/drm/Kconfig | 1 + > drivers/gpu/drm/drm_atomic.c| 255 > +--- > drivers/gpu/drm/drm_atomic_helper.c | 5 + > drivers/gpu/drm/drm_crtc.c | 52 > drivers/gpu/drm/drm_crtc_internal.h | 10 ++ > drivers/gpu/drm/drm_plane.c | 1 + > include/drm/drm_atomic.h| 1 + > include/drm/drm_crtc.h | 40 ++ > 8 files changed, 320 insertions(+), 45 deletions(-) > > -- > 2.5.5 >
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 09:18:03AM -0500, Alex Deucher wrote: > On Tue, Nov 15, 2016 at 9:03 AM, Daniel Vetter wrote: > > On Tue, Nov 15, 2016 at 03:54:42PM +0200, Ville Syrjälä wrote: > >> On Tue, Nov 15, 2016 at 11:00:04AM +0100, Daniel Vetter wrote: > >> > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: > >> > > On 11/15/2016 2:21 PM, Daniel Vetter wrote: > >> > > > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: > >> > > > > In any case, I guess addition of a cap for aspect ratio should fix > >> > > > > the > >> > > > > current objections for this implementation. > >> > > > > > >> > > > > And I will keep it 0 by default, so that no aspect ratio > >> > > > > information is > >> > > > > added until userspace sets the cap to 1 on its own. > >> > > > Note that cap = needs new userspace. > >> > > > -Daniel > >> > > I guess you mean a new libdrm, so yes, I will add this new cap in > >> > > libdrm. > >> > > Is that what you mean ? > >> > > >> > Full stack solution, including enabling in an Xorg driver (or somewhere > >> > else, we also have drm_hwcomposer as an option). > >> > > >> > And because that's probably going to take forever I'm leaning towards > >> > revert again. Ville? > >> > >> Yeah I guess we'll need to push the revert to avoid the regression. > >> Trying to put in new client caps and whatnot after -rc5 doesn't seem > >> like a viable option to me. > > > > Yeah, a few days left to get userspace in line is just not enough. Agreed > > and reverts applied. > > > > Is there any way we can add the new CEA modes and worry about handling > the aspect ratio stuff later? The two reverts from Ville are just for the aspect ratio pass-thru stuff, new cea modes should still be around. So exactly what you're asking for. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCHv2 2/2] v4l: vsp1: Provide a writeback video device
On Fri, Nov 4, 2016 at 6:53 PM, Kieran Bingham
wrote:
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> +static void vsp1_video_wb_process_buffer(struct vsp1_video *video)
> +{
> + if (buf) {
> + video->rwpf->mem = buf->mem;
> +
> + /*
> +* Store this buffer as pending. It will commence at the next
> +* frame start interrupt
> +*/
> + video->pending = buf;
> + list_del(&buf->queue);
> + } else {
> + /* Disable writeback with no buffer */
> + video->rwpf->mem = (struct vsp1_rwpf_memory) { 0 };
drivers/media/platform/vsp1/vsp1_video.c:946:30: warning: missing
braces around initializer [-Wmissing-braces]
video->rwpf->mem = (struct vsp1_rwpf_memory) { 0 };
> +static void vsp1_video_wb_stop_streaming(struct vb2_queue *vq)
> +{
> + struct vsp1_video *video = vb2_get_drv_priv(vq);
> + struct vsp1_rwpf *rwpf = video->rwpf;
> + struct vsp1_pipeline *pipe = rwpf->pipe;
> + struct vsp1_vb2_buffer *buffer;
> + unsigned long flags;
> +
> + /*
> +* Disable the completion interrupts, and clear the WPF memory to
> +* prevent writing out frames
> +*/
> + spin_lock_irqsave(&video->irqlock, flags);
> + video->frame_end = NULL;
> + rwpf->mem = (struct vsp1_rwpf_memory) { 0 };
drivers/media/platform/vsp1/vsp1_video.c:1008:22: warning: missing
braces around initializer [-Wmissing-braces]
rwpf->mem = (struct vsp1_rwpf_memory) { 0 };
Either drop the "0":
mem = (struct vsp1_rwpf_memory) { };
or add an additional pair of braces:
mem = (struct vsp1_rwpf_memory) { { 0 } };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at
linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH 02/10] drm/i915: Fixup kerneldoc includes
On Tue, Nov 15, 2016 at 10:44:29AM +, Chris Wilson wrote:
> On Mon, Nov 14, 2016 at 12:58:17PM +0100, Daniel Vetter wrote:
> > Would be great if everony could add
>
> everyone
>
> > $ make DOCBOOKS="" htmldocs
> >
> > to their build scripts to catch these. 0day should also report them,
> > not sure why it failed to spot this.
>
> "make IGNORE_DOCBOOKS=1 SPHINXOPTS=-W htmldocs" is that outdated?
IGNORE_DOCBOOKS=1 was renamed to DOCBOOKS="". And incremental builds (at
least in my experience here) are really fast with sphinx (a few seconds at
most). So should be good enough for a git rebase -x type checking.
> I don't often run it since it is too slow when checking hundreds of
> patches. Any chance of an incremental patch checker?
As long as you make sure you entirely avoid the old docbook horror show,
incremental builds should be real fast. The initial build can take a
while, but again if you avoid docbook it's reasonable fast imo. Full
kernel rebuilds are still much worse.
-Daniel
> > Fixes: b42fe9ca0a1e ("drm/i915: Split out i915_vma.c")
> > Cc: Tvrtko Ursulin
> > Cc: Chris Wilson
> > Cc: Joonas Lahtinen
> > Signed-off-by: Daniel Vetter
> Reviewed-by: Chris Wilson
>
> (I'm not even going to ask how it appears three times in the docs and
> how that all works :)
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH 08/10] drm: Extract drm_mode_config.[hc]
On Tue, Nov 15, 2016 at 10:32:14AM +, Chris Wilson wrote: > On Mon, Nov 14, 2016 at 12:58:23PM +0100, Daniel Vetter wrote: > > And shuffle the kernel-doc structure a bit since drm_crtc.[hc] now > > only contains CRTC-related functions and structures. > > > > Signed-off-by: Daniel Vetter > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h > > b/drivers/gpu/drm/drm_crtc_internal.h > > index 3d23a473ec35..dad212140d56 100644 > > --- a/drivers/gpu/drm/drm_crtc_internal.h > > +++ b/drivers/gpu/drm/drm_crtc_internal.h > > @@ -40,18 +40,25 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc, > > int x, int y, > > const struct drm_display_mode *mode, > > const struct drm_framebuffer *fb); > > - > > -void drm_fb_release(struct drm_file *file_priv); > > +int drm_crtc_register_all(struct drm_device *dev); > > +void drm_crtc_unregister_all(struct drm_device *dev); > > > > /* IOCTLs */ > > -int drm_mode_getresources(struct drm_device *dev, > > - void *data, struct drm_file *file_priv); > > int drm_mode_getcrtc(struct drm_device *dev, > > void *data, struct drm_file *file_priv); > > int drm_mode_setcrtc(struct drm_device *dev, > > void *data, struct drm_file *file_priv); > > > > + > > +/* drm_mode_config.c */ > > +/* IOCTLs */ > > +int drm_mode_getresources(struct drm_device *dev, > > + void *data, struct drm_file *file_priv); > > + > > + > > /* drm_dumb_buffers.c */ > > +int drm_modeset_register_all(struct drm_device *dev); > > +void drm_modeset_unregister_all(struct drm_device *dev); > > > > I was worried for a moment. Oops, fixed up while applying. Also noticed that I've forgotten to move drm_mode_config_cleanup (besides moving it in headers). Chris was still happy with the revised patch after checking some silliness on irc. -Daniel > > Reviewed-by: Chris Wilson > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/fence: add fence timeline to drm_crtc
From: Gustavo Padovan
Create one timeline context for each CRTC to be able to handle out-fences
and signal them. It adds a few members to struct drm_crtc: fence_context,
where we store the context we get from fence_context_alloc(), the
fence seqno and the fence lock, that we pass in fence_init() to be
used by the fence.
v2: Comment by Daniel Stone:
- add BUG_ON() to fence_to_crtc() macro
v3: Comment by Ville Syrjälä
- Use more meaningful name as crtc timeline name
v4: Comments by Brian Starkey
- Use even more meaninful name for the crtc timeline
- add doc for timeline_name
Comment by Daniel Vetter
- use in-line style for comments
- rebase after fence -> dma_fence rename
v5: Comment by Daniel Vetter
- Add doc for drm_crtc_fence_ops
v6: Comment by Chris Wilson
- Move fence_to_crtc to drm_crtc.c
- Move export of drm_crtc_fence_ops to drm_crtc_internal.h
- rebase against latest drm-misc
Signed-off-by: Gustavo Padovan
Reviewed-by: Daniel Vetter (v5)
Reviewed-by: Sean Paul (v5)
Tested-by: Robert Foss (v5)
---
drivers/gpu/drm/drm_crtc.c | 38 +
drivers/gpu/drm/drm_crtc_internal.h | 2 ++
include/drm/drm_crtc.h | 29
3 files changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index c19ecc2..02e9451 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -163,6 +164,38 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
#endif
}
+static struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
+{
+ BUG_ON(fence->ops != &drm_crtc_fence_ops);
+ return container_of(fence->lock, struct drm_crtc, fence_lock);
+}
+
+static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->dev->driver->name;
+}
+
+static const char *drm_crtc_fence_get_timeline_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->timeline_name;
+}
+
+static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
+{
+ return true;
+}
+
+const struct dma_fence_ops drm_crtc_fence_ops = {
+ .get_driver_name = drm_crtc_fence_get_driver_name,
+ .get_timeline_name = drm_crtc_fence_get_timeline_name,
+ .enable_signaling = drm_crtc_fence_enable_signaling,
+ .wait = dma_fence_default_wait,
+};
+
/**
* drm_crtc_init_with_planes - Initialise a new CRTC object with
*specified primary and cursor planes.
@@ -220,6 +253,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
struct drm_crtc *crtc,
return -ENOMEM;
}
+ crtc->fence_context = dma_fence_context_alloc(1);
+ spin_lock_init(&crtc->fence_lock);
+ snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
+"CRTC:%d-%s", crtc->base.id, crtc->name);
+
crtc->base.properties = &crtc->properties;
list_add_tail(&crtc->head, &config->crtc_list);
diff --git a/drivers/gpu/drm/drm_crtc_internal.h
b/drivers/gpu/drm/drm_crtc_internal.h
index 64bb3eb..036b972 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -43,6 +43,8 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
void drm_fb_release(struct drm_file *file_priv);
+extern const struct dma_fence_ops drm_crtc_fence_ops;
+
/* IOCTLs */
int drm_mode_getresources(struct drm_device *dev,
void *data, struct drm_file *file_priv);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 11780a9..edd2d83 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -739,6 +739,35 @@ struct drm_crtc {
*/
struct drm_crtc_crc crc;
#endif
+
+ /**
+* @fence_context:
+*
+* timeline context used for fence operations.
+*/
+ unsigned int fence_context;
+
+ /**
+* @fence_lock:
+*
+* spinlock to protect the fences in the fence_context.
+*/
+
+ spinlock_t fence_lock;
+ /**
+* @fence_seqno:
+*
+* Seqno variable used as monotonic counter for the fences
+* created on the CRTC's timeline.
+*/
+ unsigned long fence_seqno;
+
+ /**
+* @timeline_name:
+*
+* The name of the CRTC's fence timeline.
+*/
+ char timeline_name[32];
};
/**
--
2.5.5
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
Regards Shashank On 11/15/2016 7:48 PM, Ville Syrjälä wrote: > On Tue, Nov 15, 2016 at 01:48:04PM +, Jose Abreu wrote: >> Hi, >> >> >> >> On 15-11-2016 10:52, Daniel Vetter wrote: >>> On Tue, Nov 15, 2016 at 03:36:02PM +0530, Sharma, Shashank wrote: On 11/15/2016 3:30 PM, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: >> On 11/15/2016 2:21 PM, Daniel Vetter wrote: >>> On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: In any case, I guess addition of a cap for aspect ratio should fix the current objections for this implementation. And I will keep it 0 by default, so that no aspect ratio information is added until userspace sets the cap to 1 on its own. >>> Note that cap = needs new userspace. >>> -Daniel >> I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. >> Is that what you mean ? > Full stack solution, including enabling in an Xorg driver (or somewhere > else, we also have drm_hwcomposer as an option). > > And because that's probably going to take forever I'm leaning towards > revert again. Ville? Not really, with a kernel cap implementation, the code will be as it was before this patch series ( as good as revert ) And then when we want to enable it, we can add the corresponding Xserver patch. I guess the current problem is if is breaks something in some userspace, but if I am loading the flags only when the cap is set, we should be good. >>> This is not how new uabi gets merged, see: >>> >>> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements >>> >>> Userspace must be ready, or it will not land. The entire point of my >>> suggestion to just extend the display modes was to avoid the need for >>> userspace, but since existing userspace tries to be too clever already, >>> that didn't work out. >>> Thanks, Daniel >> @Ville >> >> I gave my tested by to these patches because I was facing the >> same scenario as Shashank: HDMI compliance. I believe we need to >> find a better way to handle this instead of just reverting. The >> HDMI spec is evolving so we also need to evolve. Of course that >> if this is breaking user space then we can revert and wait until >> we figure the best way to implement this. >> >> Anyway, this is a wild guess but I think the reason you are >> loosing modes may be because of >> drm_mode_equal_no_clocks_no_stereo() which is always expecting a >> aspect ratio flag to be defined. If there isn't one defined then >> it will unconditionally return false, even if the modes are >> equal. > I am well aware why we're losing modes. One reason is the mode equal > checks in the kernel as you suspect, the other is simply userspace > rejecting everything with the aspect ratio defined. > >> Can you please test this patch and see if it fixes on your >> side? WARNING: Not compile tested > I already did something like that earlier, except I rewrote the > entire messy mode matching code to use a cleaner flag based approach: > > git://github.com/vsyrjala/linux.git cea_f_vics > > But that doesn't solve the userspace ABI issue, and there are still a > lot of unanswered questions like: > > - Should we define aspect ratios for modes not directly coming from >edid_cea_modes[]? > >I beleive the answer must be "yes" at least in the cases where the >EDID lists the VIC even if we then skip adding the mode due to >already having it on the list via from another source. Perhaps we >want the aspect ratio even if the VIC wasn't directly specified? Wouldn't this break the whole concept of VIC, which is supposed to point to a unique mode ? > - Should we or should we not specify a VIC in the AVI infoframe >when the userspace doesn't support aspect ratio flags? This is a requirement of HDMI compliance tests, if userspace supports/handles aspect ratio, it should set cap, and it will be set in kernel flags and we should load that in AV IF. Are you planning to suggest a better way, in which this can be done ? >I would guess the answer is again "yes", and we should just pick the >preferred aspect ratio for the mode. At least that's closer to what >we've been doing up to now (except we just picked the first VIC, so >we might have picked the non-preferred aspect ratio actually). At >least having the VIC in the infoframe would seem like a safer option >than not having it, in case there's some cheap ass hardware that >can't do anything sensible without being hand fed a VIC. > > - How we should handle the aspect ratio in mode sorting? > >I think we want some sensible sorting order for these. Preferred >aspect ratio first would seem like the obvious choice. I do realize >that we don't sort based on 3D stereo flags either, but I thinking we >probably should. What is the need of this sorting ? None of t
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
I guess Daniel has already reverted the patches. Now many I know who is going to define on what should be the right way to handle aspect ratios ? Regards Shashank On 11/15/2016 7:48 PM, Ville Syrjälä wrote: > On Tue, Nov 15, 2016 at 01:48:04PM +, Jose Abreu wrote: >> Hi, >> >> >> >> On 15-11-2016 10:52, Daniel Vetter wrote: >>> On Tue, Nov 15, 2016 at 03:36:02PM +0530, Sharma, Shashank wrote: On 11/15/2016 3:30 PM, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: >> On 11/15/2016 2:21 PM, Daniel Vetter wrote: >>> On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: In any case, I guess addition of a cap for aspect ratio should fix the current objections for this implementation. And I will keep it 0 by default, so that no aspect ratio information is added until userspace sets the cap to 1 on its own. >>> Note that cap = needs new userspace. >>> -Daniel >> I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. >> Is that what you mean ? > Full stack solution, including enabling in an Xorg driver (or somewhere > else, we also have drm_hwcomposer as an option). > > And because that's probably going to take forever I'm leaning towards > revert again. Ville? Not really, with a kernel cap implementation, the code will be as it was before this patch series ( as good as revert ) And then when we want to enable it, we can add the corresponding Xserver patch. I guess the current problem is if is breaks something in some userspace, but if I am loading the flags only when the cap is set, we should be good. >>> This is not how new uabi gets merged, see: >>> >>> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements >>> >>> Userspace must be ready, or it will not land. The entire point of my >>> suggestion to just extend the display modes was to avoid the need for >>> userspace, but since existing userspace tries to be too clever already, >>> that didn't work out. >>> Thanks, Daniel >> @Ville >> >> I gave my tested by to these patches because I was facing the >> same scenario as Shashank: HDMI compliance. I believe we need to >> find a better way to handle this instead of just reverting. The >> HDMI spec is evolving so we also need to evolve. Of course that >> if this is breaking user space then we can revert and wait until >> we figure the best way to implement this. >> >> Anyway, this is a wild guess but I think the reason you are >> loosing modes may be because of >> drm_mode_equal_no_clocks_no_stereo() which is always expecting a >> aspect ratio flag to be defined. If there isn't one defined then >> it will unconditionally return false, even if the modes are >> equal. > I am well aware why we're losing modes. One reason is the mode equal > checks in the kernel as you suspect, the other is simply userspace > rejecting everything with the aspect ratio defined. > >> Can you please test this patch and see if it fixes on your >> side? WARNING: Not compile tested > I already did something like that earlier, except I rewrote the > entire messy mode matching code to use a cleaner flag based approach: > > git://github.com/vsyrjala/linux.git cea_f_vics > > But that doesn't solve the userspace ABI issue, and there are still a > lot of unanswered questions like: > > - Should we define aspect ratios for modes not directly coming from >edid_cea_modes[]? > >I beleive the answer must be "yes" at least in the cases where the >EDID lists the VIC even if we then skip adding the mode due to >already having it on the list via from another source. Perhaps we >want the aspect ratio even if the VIC wasn't directly specified? > > - Should we or should we not specify a VIC in the AVI infoframe >when the userspace doesn't support aspect ratio flags? > >I would guess the answer is again "yes", and we should just pick the >preferred aspect ratio for the mode. At least that's closer to what >we've been doing up to now (except we just picked the first VIC, so >we might have picked the non-preferred aspect ratio actually). At >least having the VIC in the infoframe would seem like a safer option >than not having it, in case there's some cheap ass hardware that >can't do anything sensible without being hand fed a VIC. > > - How we should handle the aspect ratio in mode sorting? > >I think we want some sensible sorting order for these. Preferred >aspect ratio first would seem like the obvious choice. I do realize >that we don't sort based on 3D stereo flags either, but I thinking we >probably should. >
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 08:40:01PM +0530, Sharma, Shashank wrote: > Regards > Shashank > On 11/15/2016 7:48 PM, Ville Syrjälä wrote: > > On Tue, Nov 15, 2016 at 01:48:04PM +, Jose Abreu wrote: > >> Hi, > >> > >> > >> > >> On 15-11-2016 10:52, Daniel Vetter wrote: > >>> On Tue, Nov 15, 2016 at 03:36:02PM +0530, Sharma, Shashank wrote: > On 11/15/2016 3:30 PM, Daniel Vetter wrote: > > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: > >> On 11/15/2016 2:21 PM, Daniel Vetter wrote: > >>> On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: > In any case, I guess addition of a cap for aspect ratio should fix > the > current objections for this implementation. > > And I will keep it 0 by default, so that no aspect ratio information > is > added until userspace sets the cap to 1 on its own. > >>> Note that cap = needs new userspace. > >>> -Daniel > >> I guess you mean a new libdrm, so yes, I will add this new cap in > >> libdrm. > >> Is that what you mean ? > > Full stack solution, including enabling in an Xorg driver (or somewhere > > else, we also have drm_hwcomposer as an option). > > > > And because that's probably going to take forever I'm leaning towards > > revert again. Ville? > Not really, with a kernel cap implementation, the code will be as it was > before this patch series ( as good as revert ) > And then when we want to enable it, we can add the corresponding Xserver > patch. > > I guess the current problem is if is breaks something in some userspace, > but > if I am loading the flags only when the cap is set, we should be good. > >>> This is not how new uabi gets merged, see: > >>> > >>> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements > >>> > >>> Userspace must be ready, or it will not land. The entire point of my > >>> suggestion to just extend the display modes was to avoid the need for > >>> userspace, but since existing userspace tries to be too clever already, > >>> that didn't work out. > >>> Thanks, Daniel > >> @Ville > >> > >> I gave my tested by to these patches because I was facing the > >> same scenario as Shashank: HDMI compliance. I believe we need to > >> find a better way to handle this instead of just reverting. The > >> HDMI spec is evolving so we also need to evolve. Of course that > >> if this is breaking user space then we can revert and wait until > >> we figure the best way to implement this. > >> > >> Anyway, this is a wild guess but I think the reason you are > >> loosing modes may be because of > >> drm_mode_equal_no_clocks_no_stereo() which is always expecting a > >> aspect ratio flag to be defined. If there isn't one defined then > >> it will unconditionally return false, even if the modes are > >> equal. > > I am well aware why we're losing modes. One reason is the mode equal > > checks in the kernel as you suspect, the other is simply userspace > > rejecting everything with the aspect ratio defined. > > > >> Can you please test this patch and see if it fixes on your > >> side? WARNING: Not compile tested > > I already did something like that earlier, except I rewrote the > > entire messy mode matching code to use a cleaner flag based approach: > > > > git://github.com/vsyrjala/linux.git cea_f_vics > > > > But that doesn't solve the userspace ABI issue, and there are still a > > lot of unanswered questions like: > > > > - Should we define aspect ratios for modes not directly coming from > >edid_cea_modes[]? > > > >I beleive the answer must be "yes" at least in the cases where the > >EDID lists the VIC even if we then skip adding the mode due to > >already having it on the list via from another source. Perhaps we > >want the aspect ratio even if the VIC wasn't directly specified? > Wouldn't this break the whole concept of VIC, which is supposed to point > to a unique mode ? Not sure what you're asking. We're already filtering out duplicates. > > - Should we or should we not specify a VIC in the AVI infoframe > >when the userspace doesn't support aspect ratio flags? > This is a requirement of HDMI compliance tests, if userspace > supports/handles aspect ratio, it should set cap, and it will be set in > kernel flags > and we should load that in AV IF. Are you planning to suggest a better > way, in which this can be done ? I was asking for the case where userspace doesn't specify the aspect ratio on account of not understanding what aspect ratios are. > >I would guess the answer is again "yes", and we should just pick the > >preferred aspect ratio for the mode. At least that's closer to what > >we've been doing up to now (except we just picked the first VIC, so > >we might have picked the non-preferred aspect ratio actually). At > >least having the VIC in the infoframe would seem like
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On Tue, Nov 15, 2016 at 9:26 AM, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 09:18:03AM -0500, Alex Deucher wrote: >> On Tue, Nov 15, 2016 at 9:03 AM, Daniel Vetter wrote: >> > On Tue, Nov 15, 2016 at 03:54:42PM +0200, Ville Syrjälä wrote: >> >> On Tue, Nov 15, 2016 at 11:00:04AM +0100, Daniel Vetter wrote: >> >> > On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: >> >> > > On 11/15/2016 2:21 PM, Daniel Vetter wrote: >> >> > > > On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: >> >> > > > > In any case, I guess addition of a cap for aspect ratio should >> >> > > > > fix the >> >> > > > > current objections for this implementation. >> >> > > > > >> >> > > > > And I will keep it 0 by default, so that no aspect ratio >> >> > > > > information is >> >> > > > > added until userspace sets the cap to 1 on its own. >> >> > > > Note that cap = needs new userspace. >> >> > > > -Daniel >> >> > > I guess you mean a new libdrm, so yes, I will add this new cap in >> >> > > libdrm. >> >> > > Is that what you mean ? >> >> > >> >> > Full stack solution, including enabling in an Xorg driver (or somewhere >> >> > else, we also have drm_hwcomposer as an option). >> >> > >> >> > And because that's probably going to take forever I'm leaning towards >> >> > revert again. Ville? >> >> >> >> Yeah I guess we'll need to push the revert to avoid the regression. >> >> Trying to put in new client caps and whatnot after -rc5 doesn't seem >> >> like a viable option to me. >> > >> > Yeah, a few days left to get userspace in line is just not enough. Agreed >> > and reverts applied. >> > >> >> Is there any way we can add the new CEA modes and worry about handling >> the aspect ratio stuff later? > > The two reverts from Ville are just for the aspect ratio pass-thru stuff, > new cea modes should still be around. So exactly what you're asking for. > -Daniel Great. Thanks! Alex
[Intel-gfx] [PATCH 02/12] tests/kms_atomic_transition: don't assume max pipes
On Tue, 2016-11-15 at 14:25 +0100, Tomeu Vizoso wrote:
> On 15 November 2016 at 09:01, Daniel Vetter wrote:
> >
> > On Mon, Nov 14, 2016 at 06:59:16PM +0900, Gustavo Padovan wrote:
> > >
> > > From: Gustavo Padovan
> > >
> > > Signed-off-by: Gustavo Padovan
> > > ---
> > > Â tests/kms_atomic_transition.c | 2 +-
> > > Â 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tests/kms_atomic_transition.c
> > > b/tests/kms_atomic_transition.c
> > > index e693c88..8b26b53 100644
> > > --- a/tests/kms_atomic_transition.c
> > > +++ b/tests/kms_atomic_transition.c
> > > @@ -404,7 +404,7 @@ static void run_modeset_tests(igt_display_t
> > > *display, int howmany, bool nonblock
> > > Â {
> > > Â Â Â Â Â Â struct igt_fb fbs[2];
> > > Â Â Â Â Â Â int i, j;
> > > -Â Â Â Â Â unsigned iter_max = 1 << I915_MAX_PIPES;
> > > +Â Â Â Â Â unsigned iter_max = 1 << display->n_pipes;
> > Didn't Tomeu have some patch series to fix these all up?
> Don't remember, and couldn't find any within my local branches. Maybe
> Robert? But I'm adding it to my backlog anyway.
>
I don't recognize it, but thanks tomeu!
Rob.
>
> >
> > -Daniel
> >
> > >
> > > Â Â Â Â Â Â igt_pipe_crc_t *pipe_crcs[I915_MAX_PIPES];
> > > Â Â Â Â Â Â igt_output_t *output;
> > > Â Â Â Â Â Â unsigned width = 0, height = 0;
> > > --
> > > 2.5.5
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx at 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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/color: document NULL values and default settings better
Sorry for missing this. Reviewed-by: Lionel Landwerlin On 26/09/16 10:04, Daniel Vetter wrote: > Brought up in a discussion for enabling gamma on fsl-dcu. > > Cc: Ville Syrjälä > Cc: Meng Yi > Cc: Lionel Landwerlin > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_color_mgmt.c | 12 > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c > b/drivers/gpu/drm/drm_color_mgmt.c > index d28ffdd2b929..6543ebde501a 100644 > --- a/drivers/gpu/drm/drm_color_mgmt.c > +++ b/drivers/gpu/drm/drm_color_mgmt.c > @@ -41,6 +41,10 @@ >* nor use all the elements of the LUT (for example the hardware might >* choose to interpolate between LUT[0] and LUT[4]). >* > + * Setting this to NULL (blob property value set to 0) means a > + * linear/pass-thru gamma table should be used. This is generally the > + * driver boot-up state too. > + * >* âDEGAMMA_LUT_SIZEâ: >* Unsinged range property to give the size of the lookup table to be set >* on the DEGAMMA_LUT property (the size depends on the underlying > @@ -54,6 +58,10 @@ >* lookup through the gamma LUT. The data is interpreted as a struct >* &drm_color_ctm. >* > + * Setting this to NULL (blob property value set to 0) means a > + * unit/pass-thru matrix should be used. This is generally the driver > + * boot-up state too. > + * >* âGAMMA_LUTâ: >* Blob property to set the gamma lookup table (LUT) mapping pixel data >* after the transformation matrix to data sent to the connector. The > @@ -62,6 +70,10 @@ >* nor use all the elements of the LUT (for example the hardware might >* choose to interpolate between LUT[0] and LUT[4]). >* > + * Setting this to NULL (blob property value set to 0) means a > + * linear/pass-thru gamma table should be used. This is generally the > + * driver boot-up state too. > + * >* âGAMMA_LUT_SIZEâ: >* Unsigned range property to give the size of the lookup table to be set >* on the GAMMA_LUT property (the size depends on the underlying hardware).
[PATCH] dma-buf: Provide wrappers for reservation's lock
Joonas complained that writing ww_mutex_lock(&resv->lock, ctx) was too
intrusive compared to reservation_object_lock(resv, ctx);
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Joonas Lahtinen
---
include/linux/reservation.h | 34 ++
1 file changed, 34 insertions(+)
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index ed238961e1bf..9cfc0d857862 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -129,6 +129,40 @@ reservation_object_fini(struct reservation_object *obj)
}
/**
+ * reservation_object_lock - lock the reservation object
+ * @obj: the reservation object
+ * @ctx: the locking context
+ *
+ * Locks the reservation object for exclusive access and modification. Note,
+ * that the lock is only against other writers, readers will run concurrently
+ * with a writer under RCU. The seqlock is used to notify readers if they
+ * overlap with a writer.
+ *
+ * As the reservation object may be locked by multiple parties in an
+ * undefined order, a #ww_acquire_ctx is passed to unwind if a cycle
+ * is detected. See ww_mutex_lock() and ww_acquire_init(). A reservation
+ * object may be locked by itself by passing NULL as @ctx.
+ */
+static inline int
+reservation_object_lock(struct reservation_object *obj,
+ struct ww_acquire_ctx *ctx)
+{
+ return ww_mutex_lock(&obj->lock, ctx);
+}
+
+/**
+ * reservation_object_unlock - unlock the reservation object
+ * @obj: the reservation object
+ *
+ * Unlocks the reservation object following exclusive access.
+ */
+static inline void
+reservation_object_unlock(struct reservation_object *obj)
+{
+ ww_mutex_unlock(&obj->lock);
+}
+
+/**
* reservation_object_get_excl - get the reservation object's
* exclusive fence, with update-side lock held
* @obj: the reservation object
--
2.10.2
[PATCH] drm/amd/powerplay: check if table_info is NULL before dereferencing it
> -Original Message-
> From: Colin King [mailto:colin.king at canonical.com]
> Sent: Tuesday, November 15, 2016 7:55 AM
> To: Deucher, Alexander; Koenig, Christian; David Airlie; Zhu, Rex; StDenis,
> Tom; Huang, Ray; Nils Wallménius; Baoyou Xie; dri-
> devel at lists.freedesktop.org
> Cc: linux-kernel at vger.kernel.org
> Subject: [PATCH] drm/amd/powerplay: check if table_info is NULL before
> dereferencing it
>
> From: Colin Ian King
>
> table_info is being dereferenced before a null check, which implies
> a potential null pointer deference error. Fix this by moving the null
> check of table_info to the start of smu7_get_evv_voltages to avoid
> potential null pointer deferencing.
>
> Found with static analysis by CoverityScan, CID 1377752
>
> Signed-off-by: Colin Ian King
NACK, this effectively reverts the patch. This causes the function to exit
early on asics where the table it NULL which is not what we want. Whether the
table exists or not is dependent on the table version and the feature caps for
the chip which are checked before the table is dereferenced. The NULL check in
the top if clause is not strictly necessary and could be removed.
Alex
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 28e748d..6798067 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -1473,6 +1473,8 @@ static int smu7_get_evv_voltages(struct pp_hwmgr
> *hwmgr)
> (struct phm_ppt_v1_information *)hwmgr->pptable;
> struct phm_ppt_v1_clock_voltage_dependency_table *sclk_table =
> NULL;
>
> + if (table_info == NULL)
> + return -EINVAL;
>
> for (i = 0; i < SMU7_MAX_LEAKAGE_COUNT; i++) {
> vv_id = ATOM_VIRTUAL_VOLTAGE_ID0 + i;
> @@ -1483,8 +1485,6 @@ static int smu7_get_evv_voltages(struct pp_hwmgr
> *hwmgr)
> table_info-
> >vddgfx_lookup_table, vv_id, &sclk)) {
> if (phm_cap_enabled(hwmgr-
> >platform_descriptor.platformCaps,
>
> PHM_PlatformCaps_ClockStretcher)) {
> - if (table_info == NULL)
> - return -EINVAL;
> sclk_table = table_info-
> >vdd_dep_on_sclk;
>
> for (j = 1; j < sclk_table->count; j++)
> {
> @@ -1517,8 +1517,6 @@ static int smu7_get_evv_voltages(struct pp_hwmgr
> *hwmgr)
> table_info->vddc_lookup_table,
> vv_id, &sclk)) {
> if (phm_cap_enabled(hwmgr-
> >platform_descriptor.platformCaps,
>
> PHM_PlatformCaps_ClockStretcher)) {
> - if (table_info == NULL)
> - return -EINVAL;
> sclk_table = table_info-
> >vdd_dep_on_sclk;
>
> for (j = 1; j < sclk_table->count; j++)
> {
> --
> 2.10.2
[PATCH] drm/amd/powerplay: check if table_info is NULL before dereferencing it
On 15/11/16 15:49, Deucher, Alexander wrote:
>> -Original Message-
>> From: Colin King [mailto:colin.king at canonical.com]
>> Sent: Tuesday, November 15, 2016 7:55 AM
>> To: Deucher, Alexander; Koenig, Christian; David Airlie; Zhu, Rex; StDenis,
>> Tom; Huang, Ray; Nils Wallménius; Baoyou Xie; dri-
>> devel at lists.freedesktop.org
>> Cc: linux-kernel at vger.kernel.org
>> Subject: [PATCH] drm/amd/powerplay: check if table_info is NULL before
>> dereferencing it
>>
>> From: Colin Ian King
>>
>> table_info is being dereferenced before a null check, which implies
>> a potential null pointer deference error. Fix this by moving the null
>> check of table_info to the start of smu7_get_evv_voltages to avoid
>> potential null pointer deferencing.
>>
>> Found with static analysis by CoverityScan, CID 1377752
>>
>> Signed-off-by: Colin Ian King
>
> NACK, this effectively reverts the patch. This causes the function to exit
> early on asics where the table it NULL which is not what we want. Whether
> the table exists or not is dependent on the table version and the feature
> caps for the chip which are checked before the table is dereferenced. The
> NULL check in the top if clause is not strictly necessary and could be
> removed.
>
> Alex
OK, understood. The part I'm missing is that we dereference table_info
at the following statement:
if ((hwmgr->pp_table_version == PP_TABLE_V1)
&& !phm_get_sclk_for_voltage_evv(hwmgr,
table_info->vddgfx_lookup_table, vv_id, &sclk))
and later check if it is NULL. So, I can't see where table_info is being
set other than the start of the function, so, either it can be null and
hence we have a null ptr deference, or it's never null, in which case
the check for NULL is redundant.
Colin
>
>> ---
>> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 6 ++
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> index 28e748d..6798067 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> @@ -1473,6 +1473,8 @@ static int smu7_get_evv_voltages(struct pp_hwmgr
>> *hwmgr)
>> (struct phm_ppt_v1_information *)hwmgr->pptable;
>> struct phm_ppt_v1_clock_voltage_dependency_table *sclk_table =
>> NULL;
>>
>> +if (table_info == NULL)
>> +return -EINVAL;
>>
>> for (i = 0; i < SMU7_MAX_LEAKAGE_COUNT; i++) {
>> vv_id = ATOM_VIRTUAL_VOLTAGE_ID0 + i;
>> @@ -1483,8 +1485,6 @@ static int smu7_get_evv_voltages(struct pp_hwmgr
>> *hwmgr)
>> table_info-
>>> vddgfx_lookup_table, vv_id, &sclk)) {
>> if (phm_cap_enabled(hwmgr-
>>> platform_descriptor.platformCaps,
>>
>> PHM_PlatformCaps_ClockStretcher)) {
>> -if (table_info == NULL)
>> -return -EINVAL;
>> sclk_table = table_info-
>>> vdd_dep_on_sclk;
>>
>> for (j = 1; j < sclk_table->count; j++)
>> {
>> @@ -1517,8 +1517,6 @@ static int smu7_get_evv_voltages(struct pp_hwmgr
>> *hwmgr)
>> table_info->vddc_lookup_table,
>> vv_id, &sclk)) {
>> if (phm_cap_enabled(hwmgr-
>>> platform_descriptor.platformCaps,
>>
>> PHM_PlatformCaps_ClockStretcher)) {
>> -if (table_info == NULL)
>> -return -EINVAL;
>> sclk_table = table_info-
>>> vdd_dep_on_sclk;
>>
>> for (j = 1; j < sclk_table->count; j++)
>> {
>> --
>> 2.10.2
>
[PATCH] drm/amd/powerplay: check if table_info is NULL before dereferencing it
> -Original Message-
> From: Colin Ian King [mailto:colin.king at canonical.com]
> Sent: Tuesday, November 15, 2016 11:09 AM
> To: Deucher, Alexander; Koenig, Christian; David Airlie; Zhu, Rex; StDenis,
> Tom; Huang, Ray; Nils Wallménius; Baoyou Xie; dri-
> devel at lists.freedesktop.org
> Cc: linux-kernel at vger.kernel.org
> Subject: Re: [PATCH] drm/amd/powerplay: check if table_info is NULL before
> dereferencing it
>
> On 15/11/16 15:49, Deucher, Alexander wrote:
> >> -Original Message-
> >> From: Colin King [mailto:colin.king at canonical.com]
> >> Sent: Tuesday, November 15, 2016 7:55 AM
> >> To: Deucher, Alexander; Koenig, Christian; David Airlie; Zhu, Rex; StDenis,
> >> Tom; Huang, Ray; Nils Wallménius; Baoyou Xie; dri-
> >> devel at lists.freedesktop.org
> >> Cc: linux-kernel at vger.kernel.org
> >> Subject: [PATCH] drm/amd/powerplay: check if table_info is NULL before
> >> dereferencing it
> >>
> >> From: Colin Ian King
> >>
> >> table_info is being dereferenced before a null check, which implies
> >> a potential null pointer deference error. Fix this by moving the null
> >> check of table_info to the start of smu7_get_evv_voltages to avoid
> >> potential null pointer deferencing.
> >>
> >> Found with static analysis by CoverityScan, CID 1377752
> >>
> >> Signed-off-by: Colin Ian King
> >
> > NACK, this effectively reverts the patch. This causes the function to exit
> early on asics where the table it NULL which is not what we want. Whether
> the table exists or not is dependent on the table version and the feature
> caps for the chip which are checked before the table is dereferenced. The
> NULL check in the top if clause is not strictly necessary and could be
> removed.
> >
> > Alex
>
> OK, understood. The part I'm missing is that we dereference table_info
> at the following statement:
>
> if ((hwmgr->pp_table_version == PP_TABLE_V1)
> && !phm_get_sclk_for_voltage_evv(hwmgr,
> table_info->vddgfx_lookup_table, vv_id, &sclk))
>
> and later check if it is NULL. So, I can't see where table_info is being
> set other than the start of the function, so, either it can be null and
> hence we have a null ptr deference, or it's never null, in which case
> the check for NULL is redundant.
Yes, that check is redundant. That is was I was referring to above.
Alex
>
> Colin
> >
> >> ---
> >> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 6 ++
> >> 1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> >> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> >> index 28e748d..6798067 100644
> >> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> >> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> >> @@ -1473,6 +1473,8 @@ static int smu7_get_evv_voltages(struct
> pp_hwmgr
> >> *hwmgr)
> >>(struct phm_ppt_v1_information *)hwmgr->pptable;
> >>struct phm_ppt_v1_clock_voltage_dependency_table *sclk_table =
> >> NULL;
> >>
> >> + if (table_info == NULL)
> >> + return -EINVAL;
> >>
> >>for (i = 0; i < SMU7_MAX_LEAKAGE_COUNT; i++) {
> >>vv_id = ATOM_VIRTUAL_VOLTAGE_ID0 + i;
> >> @@ -1483,8 +1485,6 @@ static int smu7_get_evv_voltages(struct
> pp_hwmgr
> >> *hwmgr)
> >>table_info-
> >>> vddgfx_lookup_table, vv_id, &sclk)) {
> >>if (phm_cap_enabled(hwmgr-
> >>> platform_descriptor.platformCaps,
> >>
> >>PHM_PlatformCaps_ClockStretcher)) {
> >> - if (table_info == NULL)
> >> - return -EINVAL;
> >>sclk_table = table_info-
> >>> vdd_dep_on_sclk;
> >>
> >>for (j = 1; j < sclk_table->count; j++)
> >> {
> >> @@ -1517,8 +1517,6 @@ static int smu7_get_evv_voltages(struct
> pp_hwmgr
> >> *hwmgr)
> >>table_info->vddc_lookup_table,
> >> vv_id, &sclk)) {
> >>if (phm_cap_enabled(hwmgr-
> >>> platform_descriptor.platformCaps,
> >>
> >>PHM_PlatformCaps_ClockStretcher)) {
> >> - if (table_info == NULL)
> >> - return -EINVAL;
> >>sclk_table = table_info-
> >>> vdd_dep_on_sclk;
> >>
> >>for (j = 1; j < sclk_table->count; j++)
> >> {
> >> --
> >> 2.10.2
> >
[Bug 97403] AMDGPU/Iceland Strange warnings on drm-next-4.9-wip
https://bugs.freedesktop.org/show_bug.cgi?id=97403 --- Comment #8 from Armin K --- (In reply to rezhu from comment #5) > sorry, maybe the adaptor number is not 0 in your machine. > can you just try the command: > ./atiflash -ai > > on my end: the result is > /home# ./atiflash -ai > Adapter 0(BN=01, DN=00, PCIID=69011002, SSID=01341002) > Asic Family: Iceland > > if you can get the adapter number, > then try to save the atom bios by > ./atiflash -s number file.name Hm, I'm currently on 4.8.6, does that matter? 4.9 is unusable right now, see #98417 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/9c7ef853/attachment.html>
[Bug 97403] AMDGPU/Iceland Strange warnings on drm-next-4.9-wip
https://bugs.freedesktop.org/show_bug.cgi?id=97403 --- Comment #9 from Armin K --- (In reply to rezhu from comment #5) > sorry, maybe the adaptor number is not 0 in your machine. > can you just try the command: > ./atiflash -ai > > on my end: the result is > /home# ./atiflash -ai > Adapter 0(BN=01, DN=00, PCIID=69011002, SSID=01341002) > Asic Family: Iceland > > if you can get the adapter number, > then try to save the atom bios by > ./atiflash -s number file.name OK, now I feel stupid. Your command is ran as root, I wasn't doing that. However, there's another problem now: # ./atiflash -ai Adapter 0(BN=01, DN=00, PCIID=69001002, SSID=811C103C) Asic Family: Iceland Flash Type : R600 SPI(64 KB) No VBIOS # ./atiflash -s 0 hpatombios.bin Failed to read ROM -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161115/e43d14f9/attachment.html>
[PATCH] drm/bridge: analogix_dp: return error if transfer none byte
On Thu, Nov 3, 2016 at 3:17 AM, Jianqun Xu wrote:
> Reference from drm_dp_aux description (about transfer):
> Upon success, the implementation should return the number of payload bytes
> that were transferred, or a negative error-code on failure. Helpers
> propagate errors from the .transfer() function, with the exception of
> the -EBUSY error, which causes a transaction to be retried. On a short,
> helpers will return -EPROTO to make it simpler to check for failure.
>
> The analogix_dp_transfer will return num_transferred, but if there is none
> byte been transferred, the return value will be 0, which means success, we
> should return error-code if transfer none byte.
>
> for (retry = 0; retry < 32; retry++) {
> err = aux->transfer(aux, &msg);
> if (err < 0) {
> if (err == -EBUSY)
> continue;
>
> goto unlock;
> }
> }
>
> Cc: zain wang
> Cc: Sean Paul
> Signed-off-by: Jianqun Xu
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index cd37ac0..303083a 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1162,5 +1162,5 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device
> *dp,
> (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ)
> msg->reply = DP_AUX_NATIVE_REPLY_ACK;
>
> - return num_transferred;
> + return num_transferred > 0 ? num_transferred : -EBUSY;
> }
> --
> 1.9.1
>
>
