Re: [PATCH libdrm] modetest: add C8 support to generate SMPTE pattern
On Fri, Nov 17, 2017 at 11:56 PM, Ilia Mirkin wrote:
> Tested on nouveau with the CRTC only.
>
> Signed-off-by: Ilia Mirkin
> ---
>
> Please note that I have no clue what the proper way to operate the gamma
> interface is. This seemed OK, but the 256 seems awefully hardcoded. Perhaps
> that won't work on other HW?
>
> tests/modetest/buffers.c | 2 ++
> tests/modetest/modetest.c | 13 +
> tests/util/format.c | 2 ++
> tests/util/pattern.c | 73
> +++
> tests/util/pattern.h | 2 ++
> 5 files changed, 92 insertions(+)
>
> diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
> index 4fd310b9..58f88e85 100644
> --- a/tests/modetest/buffers.c
> +++ b/tests/modetest/buffers.c
> @@ -139,6 +139,7 @@ bo_create(int fd, unsigned int format,
> int ret;
>
> switch (format) {
> + case DRM_FORMAT_C8:
> case DRM_FORMAT_NV12:
> case DRM_FORMAT_NV21:
> case DRM_FORMAT_NV16:
> @@ -279,6 +280,7 @@ bo_create(int fd, unsigned int format,
> planes[2] = virtual + offsets[2];
> break;
>
> + case DRM_FORMAT_C8:
> case DRM_FORMAT_ARGB:
> case DRM_FORMAT_XRGB:
> case DRM_FORMAT_ABGR:
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 62d93327..049bdd5e 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -1228,6 +1228,19 @@ static void set_mode(struct device *dev, struct
> pipe_arg *pipes, unsigned int co
> fprintf(stderr, "failed to set mode: %s\n",
> strerror(errno));
> return;
> }
> +
> + if (pipes[0].fourcc == DRM_FORMAT_C8) {
> + uint16_t r[256], g[256], b[256];
> +
> + util_smpte_c8_gamma(256, r, g, b);
> + ret = drmModeCrtcSetGamma(
> + dev->fd, pipe->crtc->crtc->crtc_id,
> + 256, r, g, b);
> + if (ret) {
> + fprintf(stderr, "failed to set gamma: %s\n",
> strerror(errno));
> + return;
> + }
Of course once you start messing around with gamma, something has to
undo that. I've added
} else {
uint16_t r[256], g[256], b[256];
for (j = 0; j < 256; j++)
r[j] = g[j] = b[j] = j << 8;
drmModeCrtcSetGamma(
dev->fd, pipe->crtc->crtc->crtc_id,
256, r, g, b);
But that seems like a hack here. Where should it go? clear_mode?
> + }
> }
> }
>
> diff --git a/tests/util/format.c b/tests/util/format.c
> index 043cfe7f..3eefe224 100644
> --- a/tests/util/format.c
> +++ b/tests/util/format.c
> @@ -43,6 +43,8 @@
> .yuv = { (order), (xsub), (ysub), (chroma_stride) }
>
> static const struct util_format_info format_info[] = {
> + /* Indexed */
> + { DRM_FORMAT_C8, "C8" },
> /* YUV packed */
> { DRM_FORMAT_UYVY, "UYVY", MAKE_YUV_INFO(YUV_YCbCr | YUV_CY, 2, 2, 2)
> },
> { DRM_FORMAT_VYUY, "VYUY", MAKE_YUV_INFO(YUV_YCrCb | YUV_CY, 2, 2, 2)
> },
> diff --git a/tests/util/pattern.c b/tests/util/pattern.c
> index 00b08a8c..41fb541b 100644
> --- a/tests/util/pattern.c
> +++ b/tests/util/pattern.c
> @@ -461,6 +461,77 @@ static void fill_smpte_rgb32(const struct util_rgb_info
> *rgb, void *mem,
> }
> }
>
> +static void fill_smpte_c8(void *mem, unsigned int width, unsigned int height,
> + unsigned int stride)
> +{
> + unsigned int x;
> + unsigned int y;
> +
> + for (y = 0; y < height * 6 / 9; ++y) {
> + for (x = 0; x < width; ++x)
> + ((uint8_t *)mem)[x] = x * 7 / width;
> + mem += stride;
> + }
> +
> + for (; y < height * 7 / 9; ++y) {
> + for (x = 0; x < width; ++x)
> + ((uint8_t *)mem)[x] = 7 + (x * 7 / width);
> + mem += stride;
> + }
> +
> + for (; y < height; ++y) {
> + for (x = 0; x < width * 5 / 7; ++x)
> + ((uint8_t *)mem)[x] =
> + 14 + (x * 4 / (width * 5 / 7));
> + for (; x < width * 6 / 7; ++x)
> + ((uint8_t *)mem)[x] =
> + 14 + ((x - width * 5 / 7) * 3
> + / (width / 7) + 4);
> + for (; x < width; ++x)
> + ((uint8_t *)mem)[x] = 14 + 7;
> + mem += stride;
> + }
> +}
> +
> +void util_smpte_c8_gamma(unsigned size, uint16_t *r, uint16_t *g, uint16_t
> *b)
> +{
> + if (size < 7 + 7 + 8) {
> + printf("Error: gamma too small: %d < %d\n", size, 7 + 7 + 8);
> + return;
> + }
> +#define FILL_COLOR(idx, red, green, blue) \
> + r[idx] = red << 8; \
> + g[idx] =
[Bug 103788] Screen goes blank after screen sleep and will not come back on
https://bugs.freedesktop.org/show_bug.cgi?id=103788 coolo...@gmail.com changed: What|Removed |Added Summary|Screen goes blank after |Screen goes blank after |screen sleep and wioll not |screen sleep and will not |come back on|come back on -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103788] Screen goes blank after screen sleep and wioll not come back on
https://bugs.freedesktop.org/show_bug.cgi?id=103788 --- Comment #3 from coolo...@gmail.com --- Created attachment 135572 --> https://bugs.freedesktop.org/attachment.cgi?id=135572&action=edit dmesg after issue -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103788] Screen goes blank after screen sleep and wioll not come back on
https://bugs.freedesktop.org/show_bug.cgi?id=103788 --- Comment #2 from coolo...@gmail.com --- Created attachment 135571 --> https://bugs.freedesktop.org/attachment.cgi?id=135571&action=edit xorg log after issue -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] modetest: add C8 support to generate SMPTE pattern
Tested on nouveau with the CRTC only.
Signed-off-by: Ilia Mirkin
---
Please note that I have no clue what the proper way to operate the gamma
interface is. This seemed OK, but the 256 seems awefully hardcoded. Perhaps
that won't work on other HW?
tests/modetest/buffers.c | 2 ++
tests/modetest/modetest.c | 13 +
tests/util/format.c | 2 ++
tests/util/pattern.c | 73 +++
tests/util/pattern.h | 2 ++
5 files changed, 92 insertions(+)
diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index 4fd310b9..58f88e85 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -139,6 +139,7 @@ bo_create(int fd, unsigned int format,
int ret;
switch (format) {
+ case DRM_FORMAT_C8:
case DRM_FORMAT_NV12:
case DRM_FORMAT_NV21:
case DRM_FORMAT_NV16:
@@ -279,6 +280,7 @@ bo_create(int fd, unsigned int format,
planes[2] = virtual + offsets[2];
break;
+ case DRM_FORMAT_C8:
case DRM_FORMAT_ARGB:
case DRM_FORMAT_XRGB:
case DRM_FORMAT_ABGR:
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 62d93327..049bdd5e 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -1228,6 +1228,19 @@ static void set_mode(struct device *dev, struct pipe_arg
*pipes, unsigned int co
fprintf(stderr, "failed to set mode: %s\n",
strerror(errno));
return;
}
+
+ if (pipes[0].fourcc == DRM_FORMAT_C8) {
+ uint16_t r[256], g[256], b[256];
+
+ util_smpte_c8_gamma(256, r, g, b);
+ ret = drmModeCrtcSetGamma(
+ dev->fd, pipe->crtc->crtc->crtc_id,
+ 256, r, g, b);
+ if (ret) {
+ fprintf(stderr, "failed to set gamma: %s\n",
strerror(errno));
+ return;
+ }
+ }
}
}
diff --git a/tests/util/format.c b/tests/util/format.c
index 043cfe7f..3eefe224 100644
--- a/tests/util/format.c
+++ b/tests/util/format.c
@@ -43,6 +43,8 @@
.yuv = { (order), (xsub), (ysub), (chroma_stride) }
static const struct util_format_info format_info[] = {
+ /* Indexed */
+ { DRM_FORMAT_C8, "C8" },
/* YUV packed */
{ DRM_FORMAT_UYVY, "UYVY", MAKE_YUV_INFO(YUV_YCbCr | YUV_CY, 2, 2, 2) },
{ DRM_FORMAT_VYUY, "VYUY", MAKE_YUV_INFO(YUV_YCrCb | YUV_CY, 2, 2, 2) },
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 00b08a8c..41fb541b 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -461,6 +461,77 @@ static void fill_smpte_rgb32(const struct util_rgb_info
*rgb, void *mem,
}
}
+static void fill_smpte_c8(void *mem, unsigned int width, unsigned int height,
+ unsigned int stride)
+{
+ unsigned int x;
+ unsigned int y;
+
+ for (y = 0; y < height * 6 / 9; ++y) {
+ for (x = 0; x < width; ++x)
+ ((uint8_t *)mem)[x] = x * 7 / width;
+ mem += stride;
+ }
+
+ for (; y < height * 7 / 9; ++y) {
+ for (x = 0; x < width; ++x)
+ ((uint8_t *)mem)[x] = 7 + (x * 7 / width);
+ mem += stride;
+ }
+
+ for (; y < height; ++y) {
+ for (x = 0; x < width * 5 / 7; ++x)
+ ((uint8_t *)mem)[x] =
+ 14 + (x * 4 / (width * 5 / 7));
+ for (; x < width * 6 / 7; ++x)
+ ((uint8_t *)mem)[x] =
+ 14 + ((x - width * 5 / 7) * 3
+ / (width / 7) + 4);
+ for (; x < width; ++x)
+ ((uint8_t *)mem)[x] = 14 + 7;
+ mem += stride;
+ }
+}
+
+void util_smpte_c8_gamma(unsigned size, uint16_t *r, uint16_t *g, uint16_t *b)
+{
+ if (size < 7 + 7 + 8) {
+ printf("Error: gamma too small: %d < %d\n", size, 7 + 7 + 8);
+ return;
+ }
+#define FILL_COLOR(idx, red, green, blue) \
+ r[idx] = red << 8; \
+ g[idx] = green << 8; \
+ b[idx] = blue << 8
+
+ FILL_COLOR( 0, 192, 192, 192); /* grey */
+ FILL_COLOR( 1, 192, 192, 0 ); /* yellow */
+ FILL_COLOR( 2, 0, 192, 192); /* cyan */
+ FILL_COLOR( 3, 0, 192, 0 ); /* green */
+ FILL_COLOR( 4, 192, 0, 192); /* magenta */
+ FILL_COLOR( 5, 192, 0, 0 ); /* red */
+ FILL_COLOR( 6, 0, 0, 192); /* blue */
+
+ FILL_COLOR( 7, 0, 0, 192); /* blue */
+ FILL_COLOR( 8, 19, 19, 19 ); /* black */
+ FILL_COLOR( 9, 192, 0, 192); /* magenta */
+ FILL_COLOR(10, 19, 19, 19 ); /* black */
+ FILL_COLO
Re: -EPROBE_DEFER and failed DSI panel probe
Andrzej Hajda writes: > On 16.11.2017 21:27, Eric Anholt wrote: >> Andrzej Hajda writes: >> >>> On 15.11.2017 21:26, Eric Anholt wrote: I'm happy to have the DSI panel finally working on VC4 (just waiting on https://lists.freedesktop.org/archives/dri-devel/2017-October/156407.html), but now I've got another problem to solve. It would be great if I could include the DSI panel in our upstream DT, so that it automatically worked when you plugged one in. However, right now we return -EPROBE_DEFER during bind unless the panel has actually shown up. This means that if you don't have the panel actually connected, you get this sequence at startup: [ 10.719929] [drm] Initialized [ 10.829510] vc4_hdmi 3f902000.hdmi: vc4-hdmi-hifi <-> 3f902000.hdmi mapping ok [ 10.844043] vc4-drm soc:gpu: bound 3f902000.hdmi (ops vc4_hdmi_ops [vc4]) [ 10.848626] vc4-drm soc:gpu: bound 3f806000.vec (ops vc4_vec_ops [vc4]) [ 10.850214] vc4-drm soc:gpu: failed to bind 3f70.dsi (ops vc4_dsi_ops [vc4]): -517 [ 10.856559] vc4-drm soc:gpu: master bind failed: -517 [...] [ 10.967718] rpi_touchscreen 3-0045: Atmel I2C read failed: -6 Once the panel driver fails to probe, we never get asked to re-bind vc4, and drm_of_find_panel_or_bridge looks like it would just give us -EPROBE_DEFER again since the panel still wasn't registered. Does anyone have any suggestions for handling this? >>> I guess you should call component_add only when all required resources >>> are present(including panel), I suppose moving it to vc4_dsi_host_attach >>> should help. >> How can I decide when the panel driver has tried to probe and failed, >> versus not tried to probe yet? find_panel_or_bridge gives me >> -EPROBE_DEFER either way. >> >>> On the other side I am curious why EPROBE_DEFER from bind does not fail >>> probing of some component (the last one probed), with proper error >>> propagation it should cause defer_probing of one of the components or >>> master, and probe/bind should be retried after panel's probe. >> The panel probe failed, though, so there's no trigger to re-probe other >> drivers. > > OK, I misunderstood your problem. And after 2nd read I am not sure what > do you want to achieve exactly? > > Do you want to 'hotplug' DSI panel? Or just make it working with and > without the panel. In 2nd case other paths (HDMI) should still work, I > guess. Yeah, the second thing. I would like to use a single DT to describe a platform where the panel may or may not be present, so the panel driver (panel-raspberrypi-touchscreen.c) will throw an error from the I2C part of the probe or not instead of creating the DSI device and attaching the panel. > Lets assume that you are interested in the latter case. There could be > multiple solutions more or less hacky: > > 1. Use connector's HPD infrastructure to signal presence of the panel, > ie. in mipi_dsi::host_(attach|detach) callback you grab the panel and > change connector status to connected|disconnected and calls > drm_kms_helper_hotplug_event, it is done this way in exynos_dsi_host_attach. This is basically what I had before all the review reworks, and the regression from this state is keeping me from merging the current driver downstream. This option is unfortunate because we'll have forced *some* output on at boot time, and then when the panel driver shows up later we don't resize the fbcon to the panel's size. > 2. Check for presence of the panel's driver before registering the bus, > if not present defer probing (but I am not sure if driver's registration > will trigger deferred probing, should be checked). > > This way if the panel's driver is present during mipi bus registration > and after that panel should be bound to the drivers, otherwise it means > probe failed. This solutions requires discovering panels compatible in > DSI driver. Quite complicated and very hacky. Restating to see if I understand: Defer all of VC4, including its MIPI host registration, until we see the panel driver loaded (how would one do that?). Then once we get reprobed with the MIPI driver present (would that even happen?), register our MIPI bus and continue on to component bind. In component bind, don't defer if we don't find the panel, because we know the panel driver had a chance to probe and attach right away when our bus showed up. > 3. In panel's probe attach the panel to the bus before hardware probe > and detach it if the physical panel is not present. This way dsi host > callbacks will be called and can be used to discover failed probes. If the I2C driver's probe fails, what ends up freeing the panel or DSI host driver that the I2C driver allocated when the module is unloaded? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedeskto
[GIT PULL] drm/fsl-dcu: cleanup and fixes for v4.15
Hi Daniel, Some cleanup/fixes, some noticed during testing of Noralf Trønnes rework of the suspend/resume helper. He will rebase the patchset ontop of this. I did bsae this still on drm-misc-next-fixes, hope that is still fine. -- Stefan The following changes since commit a9386bb051931778436db3dd6e3a163f7db92b56: Merge tag 'drm-misc-next-fixes-2017-11-08' of git://anongit.freedesktop.org/drm/drm-misc into drm-next (2017-11-09 11:59:30 +1000) are available in the Git repository at: http://git.agner.ch/git/linux-drm-fsl-dcu.git tags/drm-fsl-dcu-fixes-for-v4.15 for you to fetch changes up to 9fd99f4f3f5e13ce959900ae57d64b1bdb51d823: drm/fsl-dcu: enable IRQ before drm_atomic_helper_resume() (2017-11-14 17:19:24 +0100) Laurent Pinchart (1): drm/fsl-dcu: Don't set connector DPMS property Stefan Agner (2): drm/fsl-dcu: avoid disabling pixel clock twice on suspend drm/fsl-dcu: enable IRQ before drm_atomic_helper_resume() drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 +-- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 5 - 2 files changed, 1 insertion(+), 7 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103804] igt/benchmark/gem_exec_nop does not permit to select execution ring
https://bugs.freedesktop.org/show_bug.cgi?id=103804 Dmitry Rogozhkin changed: What|Removed |Added Assignee|dri-devel@lists.freedesktop |ch...@chris-wilson.co.uk |.org| CC||ch...@chris-wilson.co.uk, ||rodrigo.v...@gmail.com, ||tvrtko.ursulin@linux.intel. ||com -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103804] igt/benchmark/gem_exec_nop does not permit to select execution ring
https://bugs.freedesktop.org/show_bug.cgi?id=103804
Bug ID: 103804
Summary: igt/benchmark/gem_exec_nop does not permit to select
execution ring
Product: DRI
Version: unspecified
Hardware: Other
OS: All
Status: NEW
Severity: normal
Priority: medium
Component: IGT
Assignee: dri-devel@lists.freedesktop.org
Reporter: dmitry.v.rogozh...@intel.com
Looking into the code igt/benchmark/gem_exec_nop should permit to select a RING
to load. However, this feature is not functional. For example, assuming that
i915 PMU patches https://patchwork.freedesktop.org/series/29735/ are applied
for the kernel, try:
# perf stat -e
i915/rcs0-busy/,i915/vcs0-busy/,i915/vcs1-busy/,i915/vecs0-busy/,i915/bcs0-busy/
-a ./gem_exec_nop -e rcs
4.433
Performance counter stats for 'system wide':
2,002,891,967 ns i915/rcs0-busy/
280,244 ns i915/vcs0-busy/
118,222 ns i915/vcs1-busy/
361,440 ns i915/vecs0-busy/
365,253 ns i915/bcs0-busy/
3.033127723 seconds time elapsed
# perf stat -e
i915/rcs0-busy/,i915/vcs0-busy/,i915/vcs1-busy/,i915/vecs0-busy/,i915/bcs0-busy/
-a ./gem_exec_nop -e vcs
4.531
Performance counter stats for 'system wide':
2,005,028,005 ns i915/rcs0-busy/
304,735 ns i915/vcs0-busy/
100,476 ns i915/vcs1-busy/
348,364 ns i915/vecs0-busy/
383,365 ns i915/bcs0-busy/
3.048972240 seconds time elapsed
# perf stat -e
i915/rcs0-busy/,i915/vcs0-busy/,i915/vcs1-busy/,i915/vecs0-busy/,i915/bcs0-busy/
-a ./gem_exec_nop -e bcs
4.548
Performance counter stats for 'system wide':
2,003,302,067 ns i915/rcs0-busy/
229,991 ns i915/vcs0-busy/
50,410 ns i915/vcs1-busy/
249,257 ns i915/vecs0-busy/
267,072 ns i915/bcs0-busy/
3.050740036 seconds time elapsed
# perf stat -e
i915/rcs0-busy/,i915/vcs0-busy/,i915/vcs1-busy/,i915/vecs0-busy/,i915/bcs0-busy/
-a ./gem_exec_nop -e vecs
4.547
Performance counter stats for 'system wide':
2,002,918,507 ns i915/rcs0-busy/
251,940 ns i915/vcs0-busy/
134,314 ns i915/vcs1-busy/
345,163 ns i915/vecs0-busy/
366,121 ns i915/bcs0-busy/
3.054508956 seconds time elapsed
# perf stat -e
i915/rcs0-busy/,i915/vcs0-busy/,i915/vcs1-busy/,i915/vecs0-busy/,i915/bcs0-busy/
-a ./gem_exec_nop -e all
4.488
Performance counter stats for 'system wide':
2,004,461,103 ns i915/rcs0-busy/
194,267 ns i915/vcs0-busy/
104,581 ns i915/vcs1-busy/
306,019 ns i915/vecs0-busy/
291,113 ns i915/bcs0-busy/
3.061850018 seconds time elapsed
So, you see that the load goes always to rcs0. The reason seems to be the
commit:
commit 05ca171aa9a6902614241f9685de2f62f30126d8
Author: Chris Wilson
Date: Fri Jun 3 10:43:09 2016 +0100
benchmarks/gem_exec_nop: Extend submission to check write inter-engine sync
Currently, we look at the throughput for submitting a read batch to a
single engine or any. The kernel optimises for this by allowing multiple
engine to read at the same time, but writes are exclusive to a single
engine. So lets try to measure the impact of inserting the barriers
between writes on different engines.
Signed-off-by: Chris Wilson
which actually shadowed the RING parameter in the loop function:
static int loop(unsigned ring, int reps, int ncpus, unsigned flags) {
all_nengine = 0;
for (ring = 1; ring < 16; ring++) {
execbuf.flags &= ~ENGINE_FLAGS;
execbuf.flags |= ring;
if (__gem_execbuf(fd, &execbuf) == 0)
all_engines[all_nengine++] = ring;
}
if (ring == -1) {
nengine = all_nengine;
memcpy(engines, all_engines, all_nengine*sizeof(engines[0]));
} else {
nengine = 1;
engines[0] = ring;
}
}
--
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 92248] [KBL/SKL/BYT/BXT/GLK] igt/kms_plane_scaling fail
https://bugs.freedesktop.org/show_bug.cgi?id=92248 --- Comment #45 from Hector Velazquez --- This test continue failing on GLK QA igt@kms_plane_scaling IGT-Version: 1.20-g88d6550 (x86_64) (Linux: 4.14.0-drm-tip-ww46-commit-1fc4fe8+ x86_64) fastfeedback-nov-ww46-thursday-07-03-33-code-179785857 Component: drm tag: libdrm-2.4.81-107-g18ffe48 commit: 18ffe485cdfa41d48b6f2d3080cb990d28c27d57 Component: cairo tag: 1.15.6-83-g0c8070f commit: 0c8070f5bc74c124e6393b433a61807a8e4bee5d Component: intel-gpu-tools tag: intel-gpu-tools-1.19-483-g88d6550 commit: 88d6550795fad3974d77e4db2f563c5e2e8872e1 Component: piglit tag: piglit-v1 commit: b6aee208234287380d2e55c17dc2d236931284fa -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103107] [CI] igt@gem_ctx_param@invalid-param-[get|set] - Failed assertion: __gem_context_get_param(fd, &arg) == -22
https://bugs.freedesktop.org/show_bug.cgi?id=103107 --- Comment #5 from Hector Velazquez --- This tests has the same failure on GLK QA igt@gem_ctx_param@invalid-param-get igt@gem_ctx_param@invalid-param-set IGT-Version: 1.20-g88d6550 (x86_64) (Linux: 4.14.0-drm-tip-ww46-commit-1fc4fe8+ x86_64) fastfeedback-nov-ww46-thursday-07-03-33-code-179785857 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103769] Unity based games do not start
https://bugs.freedesktop.org/show_bug.cgi?id=103769 --- Comment #3 from bartos.p...@gmail.com --- For additional info, I've compiled latest mesa git with llvm 4.0 (stock version of Fedora 26) and Unity games are working. But I've no idea whether the problem is in llvm 6.0 itself, or how mesa is using newest llvm library. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm: amd: Fix line continuation formats
On Thu, Nov 16, 2017 at 10:50 AM, Joe Perches wrote:
> On Thu, 2017-11-16 at 10:38 -0500, Harry Wentland wrote:
>> On 2017-11-16 10:27 AM, Joe Perches wrote:
>> > Line continuations with excess spacing causes unexpected output.
> []
>> > @@ -872,9 +870,8 @@ static bool perform_clock_recovery_sequence(
>> > if (retry_count >= LINK_TRAINING_MAX_CR_RETRY) {
>> > ASSERT(0);
>> > dm_logger_write(link->ctx->logger, LOG_ERROR,
>> > - "%s: Link Training Error, could not \
>> > -get CR after %d tries. \
>> > - Possibly voltage swing issue", __func__,
>> > + "%s: Link Training Error, could not get CR after %d
>> > tries. Possibly voltage swing issue",
>>
>> Would probably be good to add a '\n' here as well but that's not the main
>> intention of this patch.
>
> About 1/4 of the dm_logger_write calls are missing
> newlines and I think it should be a separate patch.
>
> I encourage you to fix them one day.
>
>> Reviewed-by: Harry Wentland
>
> cheers, Joe
Applied. Thanks!
Alex
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for v4.15
Am 17.11.2017 um 19:55 schrieb Linus Torvalds: On Fri, Nov 17, 2017 at 10:14 AM, Christian König wrote: Taking an example from the AMD headers why this automation is more tricky than it sounds in the first place: Look at the mmVM_CONTEXT*_PAGE_TABLE_BASE_ADDR registers for example. Register 0-7 are consecutive and so could be perfectly addressable with an index, but register 8-15 aren't and so we always end with logic like if(i<8) ... else The rational from the hardware guys is obvious that they initially had only 8 and on a later hardware generation extended that to 16 registers. Heh. I don't disagree, but at the same time, that case is actually a wonderful example. Let's take the gmc_6_0 case, because it shows your irregularity, but it also shows another horrid example of nasty nasty automation: mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR 0x054F mmVM_CONTEXT10_PAGE_TABLE_BASE_ADDR 0x0510 mmVM_CONTEXT11_PAGE_TABLE_BASE_ADDR 0x0511 mmVM_CONTEXT12_PAGE_TABLE_BASE_ADDR 0x0512 mmVM_CONTEXT13_PAGE_TABLE_BASE_ADDR 0x0513 mmVM_CONTEXT14_PAGE_TABLE_BASE_ADDR 0x0514 mmVM_CONTEXT15_PAGE_TABLE_BASE_ADDR 0x0515 mmVM_CONTEXT1_PAGE_TABLE_BASE_ADDR 0x0550 mmVM_CONTEXT2_PAGE_TABLE_BASE_ADDR 0x0551 mmVM_CONTEXT3_PAGE_TABLE_BASE_ADDR 0x0552 mmVM_CONTEXT4_PAGE_TABLE_BASE_ADDR 0x0553 mmVM_CONTEXT5_PAGE_TABLE_BASE_ADDR 0x0554 mmVM_CONTEXT6_PAGE_TABLE_BASE_ADDR 0x0555 mmVM_CONTEXT7_PAGE_TABLE_BASE_ADDR 0x0556 mmVM_CONTEXT8_PAGE_TABLE_BASE_ADDR 0x050E mmVM_CONTEXT9_PAGE_TABLE_BASE_ADDR 0x050F Oops. Those were clearly sorted automatically, and in entirely the wrong way. So automation has _really_ done something inexcusably stupid, and made the end result completely illegible in the process. Yeah, but that is already the input we get from the hardware teams. E.g. in this case a list of registers sorted by their name or address (or even sometimes some hardware internal magic). There isn't much we could do about that except for manual or semi manually cleaning up the mess. And yes, you'd be right that it's discontiguous at 8, but it's still arithmetic, ie you could easily have #define mmVM_PAGE_TABLE_BASE_ADDR(ctx) \ ((ctx)+0x054f-((ctx) & 8)*9-((ctx)&8)/8) and if "ctx" is a constant, then the end result is trivially a constant and can be used as such. And if it isn't, it's still a much cheaper operation than an "if" or "switch ()" statement (it's just a bitmask and two shifts). Interesting approach, but it is not so performance critical. So I would still go with the "if" or "?" operator just for the improved readability. Now, seeing those patterns is likely not something that automation should do (although it's definitely possible - superoptimizers do that all the time), but automation could still *verify* the patterns once a human has made them up. Well, this was just a rather simple example, the real problem is that some blocks have a dozen instances and >10k registers each. Manual intervention is just completely out of question when applied to the general problem. What we need is some automation, but as you wrote as well that is possible but far from easy. And it's quite possible that it would be a good idea to encode that pattern even in the original source code. In fact, it may *be* there somewhere (not as that arithmetic expression, but as the reverse decode logic, obviously). The obvious alternative which we are working on for a few years now is to improve the input data we get from the hardware people. In other words instead of getting a flat list of registers we want the information about where and how many times a hardware block was instantiated. But getting that proved much more difficult than we thought and yes we are working on that for multiple years now. Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for v4.15
On Fri, Nov 17, 2017 at 10:14 AM, Christian König wrote: > > Taking an example from the AMD headers why this automation is more tricky > than it sounds in the first place: Look at the > mmVM_CONTEXT*_PAGE_TABLE_BASE_ADDR registers for example. > > Register 0-7 are consecutive and so could be perfectly addressable with an > index, but register 8-15 aren't and so we always end with logic like if(i<8) > ... else > > The rational from the hardware guys is obvious that they initially had only > 8 and on a later hardware generation extended that to 16 registers. Heh. I don't disagree, but at the same time, that case is actually a wonderful example. Let's take the gmc_6_0 case, because it shows your irregularity, but it also shows another horrid example of nasty nasty automation: mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR 0x054F mmVM_CONTEXT10_PAGE_TABLE_BASE_ADDR 0x0510 mmVM_CONTEXT11_PAGE_TABLE_BASE_ADDR 0x0511 mmVM_CONTEXT12_PAGE_TABLE_BASE_ADDR 0x0512 mmVM_CONTEXT13_PAGE_TABLE_BASE_ADDR 0x0513 mmVM_CONTEXT14_PAGE_TABLE_BASE_ADDR 0x0514 mmVM_CONTEXT15_PAGE_TABLE_BASE_ADDR 0x0515 mmVM_CONTEXT1_PAGE_TABLE_BASE_ADDR 0x0550 mmVM_CONTEXT2_PAGE_TABLE_BASE_ADDR 0x0551 mmVM_CONTEXT3_PAGE_TABLE_BASE_ADDR 0x0552 mmVM_CONTEXT4_PAGE_TABLE_BASE_ADDR 0x0553 mmVM_CONTEXT5_PAGE_TABLE_BASE_ADDR 0x0554 mmVM_CONTEXT6_PAGE_TABLE_BASE_ADDR 0x0555 mmVM_CONTEXT7_PAGE_TABLE_BASE_ADDR 0x0556 mmVM_CONTEXT8_PAGE_TABLE_BASE_ADDR 0x050E mmVM_CONTEXT9_PAGE_TABLE_BASE_ADDR 0x050F Oops. Those were clearly sorted automatically, and in entirely the wrong way. So automation has _really_ done something inexcusably stupid, and made the end result completely illegible in the process. And yes, you'd be right that it's discontiguous at 8, but it's still arithmetic, ie you could easily have #define mmVM_PAGE_TABLE_BASE_ADDR(ctx) \ ((ctx)+0x054f-((ctx) & 8)*9-((ctx)&8)/8) and if "ctx" is a constant, then the end result is trivially a constant and can be used as such. And if it isn't, it's still a much cheaper operation than an "if" or "switch ()" statement (it's just a bitmask and two shifts). Now, seeing those patterns is likely not something that automation should do (although it's definitely possible - superoptimizers do that all the time), but automation could still *verify* the patterns once a human has made them up. And it's quite possible that it would be a good idea to encode that pattern even in the original source code. In fact, it may *be* there somewhere (not as that arithmetic expression, but as the reverse decode logic, obviously). Linus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] omapdrm: hdmi4_cec: fix unsigned int comparison with less than zero
From: Colin Ian King
The two comparisons of the unsigned int ret for -ve error returns is
always false because ret is unsigned. Fix this by making ret a signed
int.
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
index d86873f2abe6..e626eddf24d5 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
@@ -352,7 +352,7 @@ int hdmi4_cec_init(struct platform_device *pdev, struct
hdmi_core_data *core,
{
const u32 caps = CEC_CAP_TRANSMIT | CEC_CAP_LOG_ADDRS |
CEC_CAP_PASSTHROUGH | CEC_CAP_RC;
- unsigned int ret;
+ int ret;
core->adap = cec_allocate_adapter(&hdmi_cec_adap_ops, core,
"omap4", caps, CEC_MAX_LOG_ADDRS);
--
2.14.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for v4.15
First of all I can certainly agree with everything said so far, so just skipping to the technical problem. Am 17.11.2017 um 17:57 schrieb Linus Torvalds: People say "it's a ton of work to do it by hand". They'd be right. I'm not saying you should do it by hand. But "automation" does not always need to mean "completely mindlessly stupid" either. Taking an example from the AMD headers why this automation is more tricky than it sounds in the first place: Look at the mmVM_CONTEXT*_PAGE_TABLE_BASE_ADDR registers for example. Register 0-7 are consecutive and so could be perfectly addressable with an index, but register 8-15 aren't and so we always end with logic like if(i<8) ... else The rational from the hardware guys is obvious that they initially had only 8 and on a later hardware generation extended that to 16 registers. Patterns like that repeat all over the place and are not limited to AMD at all. We could write up hand written rules to sanitize all of that, but then we are back to "automatically" creating the headers by hand. When you have to maintain 10+ years of hardware generations where sometimes registers headers even need to be regenerated from the database then that is something which won't fly either. Certainly not saying that this is a bad idea, but we simply need better tools for that which also have 100% reproducible results. Ideas welcome, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for v4.15
On Fri, Nov 17, 2017 at 08:57:53AM -0800, Linus Torvalds wrote: > On Fri, Nov 17, 2017 at 4:51 AM, Nicolai Hähnle wrote: > To see the effects of this, I picked something at random from one of > those huge AMD header files. > > I swear. It was entirely at random, and the first thing I picked. Do this: > > git grep PCIE_UNCORR_ERR_MASK Oh that looks familiar, it's the Uncorrectable Error Mask Register (PCIe Base Spec r2.1 page 558). We already have those definitions in the tree in include/uapi/linux/pci_regs.h: git grep PCI_ERR_UNC_ > and tell me that there isn't any room for making these things smarter. ... or deduplicate them. :-) Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for v4.15
On Fri, Nov 17, 2017 at 9:19 AM, Lukas Wunner wrote: >> and tell me that there isn't any room for making these things smarter. > > ... or deduplicate them. :-) You could even - wait for it - have _automation_ that does it. Yeah, it's easier to write a stupid sed-script or whatever to generate those files. Taking patterns into account and making the output smarter is harder, but still doesn't have to be manual. But wouldn't it be good? I bet it would be good for all those other OS's that want the header file too. Linus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for v4.15
On Fri, Nov 17, 2017 at 4:51 AM, Nicolai Hähnle wrote:
>
> This raises the question of how people feel about putting the source
> database into the kernel (most likely as XML in our case) and
> auto-generating the headers from there instead.
I suspect that at least in some cases, the "source" database may be a
bit too sensitive. It may have various comments about errata, and may
even be part of the whole hardware build system (ie it might be munged
from verilog/vhdl)
> I've been pondering doing this in Mesa for radeonsi for quite some time now.
What personally annoys me most about a lot of generated header files
(not all, but a _lot_) is that exactly because they are generated,
they are often inexcusably stupid.
So say that you have a block of status registers, all of which have a
certain base pattern. Every single auto-generated header file I have
ever seen takes the brute-force approach of just enumerating them all
separately as independent things.
Which makes the header file a huge mess of repeated almost-identical
register defines.
But it's not even the _size_ of the header file that then annoys me,
it's that it's not just big, but inflexible. Often, on a source level,
you may actually be in the situation where you get an index to the
thing, and then you do
switch (index) {
case X:
access_using(REGISTERX_DEFINITION);
case Y:
access_using(REGISTERY_DEFINITION);
...
or similar. When a _smarter_ auto-generated file would actually take
advantage of the language features (whether preprocessor or dynamic),
and actually allow for
access_using(REGISTER_DEFINITION(index))
instead.
Or - a variation of the above - it's not that the register definitions
for _one_ core have that kind of regularity, but you have it for a
while family of products, and because you autogenerate, you
auto-generate the stupid "repat the exact same thing with different
names" model, and then you have (instead of the index thing) you have
the "chip family" thing that you switch on.
To see the effects of this, I picked something at random from one of
those huge AMD header files.
I swear. It was entirely at random, and the first thing I picked. Do this:
git grep PCIE_UNCORR_ERR_MASK
and tell me that there isn't any room for making these things smarter.
Btw, not a single of those defines are actually _used_. Again, that
pattern was entirely randomly picked from the largest header file we
have.
People say "it's a ton of work to do it by hand". They'd be right. I'm
not saying you should do it by hand. But "automation" does not always
need to mean "completely mindlessly stupid" either.
Linus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] OMAPFB: prevent buffer underflow in omapfb_parse_vram_param()
On Tuesday, November 14, 2017 09:12:28 AM Dan Carpenter wrote: > We cap the upper bound of "fbnum" but we also need to check for > negatives or make the type unsigned. > > Signed-off-by: Dan Carpenter Patch queued for 4.15, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: fbdev: sm501fb: fix potential null pointer dereference on fbi
On Friday, November 10, 2017 05:32:31 PM Colin King wrote:
> From: Colin Ian King
>
> The pointer fbi is dereferenced with par = fbi->par before there is a
> null check on fbi, hence there is a potential null pointer dereference
> on a null par. Fix this by moving the dereference after the null
> pointer check.
>
> Detected by CoverityScan, CID#1461301 ("Dereference before null check")
>
> Signed-off-by: Colin Ian King
Patch queued for 4.15, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display/dc/dce110/dce110_mem_input_v: use swap macro in program_size_and_rotation
On 2017-11-10 05:31 PM, Gustavo A. R. Silva wrote:
> Make use of the swap macro instead of _manually_ swapping values
> and remove unnecessary variable swap.
>
> This makes the code easier to read and maintain.
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva
Reviewed-by: Harry Wentland
Harry
> ---
> .../drm/amd/display/dc/dce110/dce110_mem_input_v.c | 28
> +++---
> 1 file changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input_v.c
> b/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input_v.c
> index a06c602..7bab8c6 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input_v.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input_v.c
> @@ -237,26 +237,14 @@ static void program_size_and_rotation(
> if (rotation == ROTATION_ANGLE_90 ||
> rotation == ROTATION_ANGLE_270) {
>
> - uint32_t swap;
> - swap = local_size.video.luma_size.x;
> - local_size.video.luma_size.x =
> - local_size.video.luma_size.y;
> - local_size.video.luma_size.y = swap;
> -
> - swap = local_size.video.luma_size.width;
> - local_size.video.luma_size.width =
> - local_size.video.luma_size.height;
> - local_size.video.luma_size.height = swap;
> -
> - swap = local_size.video.chroma_size.x;
> - local_size.video.chroma_size.x =
> - local_size.video.chroma_size.y;
> - local_size.video.chroma_size.y = swap;
> -
> - swap = local_size.video.chroma_size.width;
> - local_size.video.chroma_size.width =
> - local_size.video.chroma_size.height;
> - local_size.video.chroma_size.height = swap;
> + swap(local_size.video.luma_size.x,
> + local_size.video.luma_size.y);
> + swap(local_size.video.luma_size.width,
> + local_size.video.luma_size.height);
> + swap(local_size.video.chroma_size.x,
> + local_size.video.chroma_size.y);
> + swap(local_size.video.chroma_size.width,
> + local_size.video.chroma_size.height);
> }
>
> value = 0;
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display/dc/core/dc_resource: use swap macro in rect_swap_helper
On 2017-11-10 05:38 PM, Gustavo A. R. Silva wrote:
> Make use of the swap macro instead of _manually_ swapping values
> and remove unnecessary variable temp.
>
> This makes the code easier to read and maintain.
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva
Reviewed-by: Harry Wentland
Harry
> ---
> drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 11 ++-
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index d1cdf9f..ee216f2 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -426,15 +426,8 @@ static enum pixel_format
> convert_pixel_format_to_dalsurface(
>
> static void rect_swap_helper(struct rect *rect)
> {
> - uint32_t temp = 0;
> -
> - temp = rect->height;
> - rect->height = rect->width;
> - rect->width = temp;
> -
> - temp = rect->x;
> - rect->x = rect->y;
> - rect->y = temp;
> + swap(rect->height, rect->width);
> + swap(rect->x, rect->y);
> }
>
> static void calculate_viewport(struct pipe_ctx *pipe_ctx)
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103370] `vblank_mode=0 DRI_PRIME=1 glxgears` will introduce GPU lock up on Intel Graphics [8086:5917] + AMD Graphics [1002:6665] (rev c3)
https://bugs.freedesktop.org/show_bug.cgi?id=103370 --- Comment #31 from Michel Dänzer --- With vblank_mode=0, the only thing that can prevent tearing is luck. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103370] `vblank_mode=0 DRI_PRIME=1 glxgears` will introduce GPU lock up on Intel Graphics [8086:5917] + AMD Graphics [1002:6665] (rev c3)
https://bugs.freedesktop.org/show_bug.cgi?id=103370 --- Comment #30 from Shih-Yuan Lee --- Tearing won't happen on battery power, but it will only happen when plugged in AC power. Is this behavior also expected? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Android i.mx6 drm_hwcomposer
Hey Martin, On Fri, 2017-11-17 at 15:27 +0100, Martin Fuzzey wrote: > Hi, > > I'm trying to get Android graphics working on i.MX6 using upstream > versions: > > Kernel 4.14 > > Mesa: mesa-17.3.0-rc2 > > Libdrm: libdrm-2.4.88 > > drm_hwcomposer (git head) > > gbm_gralloc (git head from git://github.com/robherring) > > > I want to run on Android 5, which only has HWC1, the code for that > is > still in drm_hwcomposer but is not built anymore. > So I added that back and fixed up a few minor build errors. > > The first problem I ran into was > E/GRALLOC-GBM( 437): failed to create BO, size=1024x600, > fmt=2, > usage=5 > W/GraphicBufferAllocator( 437): alloc(1024, 600, 2, 1a00, > ...) > failed -13 (Permission denied) > > This was due to an GBM attempting to issue DRM_IOCTL_MODE_CREATE_DUMB > on > a render node. > > Setting gralloc.gbm.device /dev/dri/card1 (the imx-drm node) instead > of > the default /dev/dri/renderD128 fixed that but then HWC couldn't > become > master on the DRI node to issue KMS commands since gralloc had > already > opened the device. > > Adding a patch similar to: > > commit 4f4cb902517f5caacea075fb9724e4ce0c435e3d > Author: Robert Foss > Date: Wed May 17 18:08:14 2017 -0400 > > drm_hwcomposer: Get KMS FD from gbm_gralloc > > To make drm_hwc and gbm_gralloc use the same FD fixed that. > > > But the problem I now have that when HWC tries to import the first > client buffers (from bootanimation) there are errors like: > > 01-01 17:27:02.282 443 443 E hwc-platform-drm-generic: failed to > import prime fd 25 ret=-1 > > This is because the imx-drm kernel driver wants physically > contiguous > buffers for import (it uses drm_gem_cma_prime_import_sg_table()) but > the > buffer allocated by drm etnaviv isn't contiguous... > > Not sure what to do about that one. > > Any help appreciated. > > Martin > Remarkably this is an issue have not encountered. I would suggest poking the fine folks on the etnaviv ML, like Lucas/Wladimir/Christian. Rob. signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Operation context for TTM
On 17/11/17 11:49 AM, Christian König wrote: > Hi everyone, > > Michel already reviewed this back in April, but I didn't found time to > actually fully test it before now. > > So sending this one out once more because it's an interface change which > affects all driver using TTM. > > Please review and/or comment. This series is now also Tested-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103791] Tearing after screen wakeup/on
https://bugs.freedesktop.org/show_bug.cgi?id=103791 Michel Dänzer changed: What|Removed |Added Attachment #135554|text/x-log |text/plain mime type|| -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] gpu: gma500: remove unneeded DRIVER_LICENSE #define
There is no need to #define the license of the driver, just put it in
the MODULE_LICENSE() line directly as a text string.
This allows tools that check that the module license matches the source
code license to work properly, as there is no need to unwind the
unneeded dereference, especially when the string is defined in a .h file
far away from the .c file it is used in.
Cc: Patrik Jakobsson
Cc: David Airlie
Reported-by: Philippe Ombredanne
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
drivers/gpu/drm/gma500/psb_drv.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 37a3be71acd9..8f5cc1f471cd 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -527,4 +527,4 @@ module_exit(psb_exit);
MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
-MODULE_LICENSE(DRIVER_LICENSE);
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
index 821497dbd3fc..4918efc57b7a 100644
--- a/drivers/gpu/drm/gma500/psb_drv.h
+++ b/drivers/gpu/drm/gma500/psb_drv.h
@@ -36,7 +36,6 @@
#include "mmu.h"
#define DRIVER_AUTHOR "Alan Cox and others"
-#define DRIVER_LICENSE "GPL"
#define DRIVER_NAME "gma500"
#define DRIVER_DESC "DRM driver for the Intel GMA500, GMA600, GMA3600, GMA3650"
--
2.15.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH AUTOSEL for 4.9 36/56] drm/i915: Fix the level 0 max_wm hack on VLV/CHV
On Fri, Nov 17, 2017 at 01:13:27PM +, Emil Velikov wrote: > Hi Greg, all, > > Pardon for the silly question, but I'm struggling to find > documentation about this new 'autoselection' process? > Where can one read up on it - be that about the tooling or the heuristics > used? > > I think the above may be the core reason behind the discussion here. See my other email on this topic already (hint, it shouldn't matter how the patch is selected if at least one experienced developer feels it might be a valid stable patch...) thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL for 4.9 36/56] drm/i915: Fix the level 0 max_wm hack on VLV/CHV
On Fri, Nov 17, 2017 at 03:01:08PM +0200, Jani Nikula wrote: > On Fri, 17 Nov 2017, Greg KH wrote: > > On Fri, Nov 17, 2017 at 01:28:05PM +0200, Jani Nikula wrote: > >> > >> Cc: Greg > >> > >> On Wed, 15 Nov 2017, Ville Syrjälä wrote: > >> > On Wed, Nov 15, 2017 at 04:44:54PM +, alexander.le...@verizon.com > >> > wrote: > >> >> On Wed, Nov 15, 2017 at 01:08:05PM +0200, Ville Syrjälä wrote: > >> >> >On Wed, Nov 15, 2017 at 02:45:43AM +, alexander.le...@verizon.com > >> >> >wrote: > >> >> >> From: Ville Syrjälä > >> >> >> > >> >> >> [ Upstream commit 1be4d3793d5a93daddcd9be657c429b38ad750a3 ] > >> >> >> > >> >> >> The watermark should never exceed the FIFO size, so we need to > >> >> >> check against the current FIFO size instead of the theoretical > >> >> >> maximum when we clamp the level 0 watermark. > >> >> >> > >> >> >> Signed-off-by: Ville Syrjälä > >> >> >> Link: > >> >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.freedesktop.org_patch_msgid_1480354637-2D14209-2D4-2Dgit-2Dsend-2Demail-2Dville.syrjala-40linux.intel.com&d=DwIDAw&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=iuPtUar-VEGbH1jmVH_UTr4C02X8fmjHUfNYix-Yc0Y&s=ha_F0zP3A1Aztp5S5e6_bqdhiuuPXhn0dRWQ58vv3Is&e= > >> >> >> Reviewed-by: Maarten Lankhorst > >> >> >> Signed-off-by: Sasha Levin > >> >> > > >> >> >Why are these patches being proposed for stable? They're not straight > >> >> >up > >> >> >fixes for known issues, and there's always a chance that something will > >> >> >break. Who is doing the qa on this? > >> >> > >> >> Hi Ville, > >> >> > >> >> They were selected automatically as part of a new process we're trying > >> >> out. If you disagree with the selection I'd be happy to drop it. > >> > > >> > How does that automatic process decide that a patch should be backported? > >> > > >> > drm and i915 are very fast moving targets so unintended side effects from > >> > backported patches is a real possibility. So I would recommend against > >> > backporting anything that isn't fixing a real issue affecting users. We > >> > do try to add the cc:stable to such patches. > >> > >> Agreed. > >> > >> First, I think an automatic backport process is against the stable > >> kernel rules (e.g. "It must fix a real bug that bothers people"). > > > > It's finding lots of fixes that did bother people enough to submit a fix > > for. > > I still have no idea how this autoselect picks up patches that do *not* > have cc: stable nor Fixes: from us. What information do you have that we > don't for making that call? I'll let Sasha describe how he's doing this, but in the end, does it really matter _how_ it is done, vs. the fact that it seems to at least one human reviewer that this is a patch that _should_ be included based on the changelog text and the code patch? By having this review process that Sasha is providing, he's saying "Here's a patch that I think might be good for stable, do you object?" If you do, great, no harm done, all is fine, the patch is dropped. If you don't object, just ignore the email and the patch gets merged. If you don't want any of this to happen for your subsystem at all, then also fine, just let us know and we will ignore it entirely. thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 18/22] drm/zte: Use drm_fb_cma_fbdev_init/fini()
On Wed, Nov 15, 2017 at 03:19:57PM +0100, Noralf Trønnes wrote: > Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on > the fact that drm_device holds a pointer to the drm_fb_helper structure. > This means that the driver doesn't have to keep track of that. > Also use the drm_fb_helper functions directly. > > Cc: Shawn Guo Acked-by: Shawn Guo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] omapdrm: hdmi4: Correct the SoC revision matching
On 2017-11-17 14:55, Tomi Valkeinen wrote:
> On 17/11/17 10:00, Peter Ujfalusi wrote:
>> I believe the intention of the commit 2c9fc9bf45f8
>> ("drm: omapdrm: Move FEAT_HDMI_* features to hdmi4 driver")
>> was to identify omap4430 ES1.x, omap4430 ES2.x and other OMAP4 revisions,
>> like omap4460.
>>
>> By using family=OMAP4 in the match the code will treat omap4460 ES1.x in a
>> same way as it would treat omap4430 ES1.x
>>
>> This breaks HDMI audio on OMAP4460 devices (PandaES for example).
>>
>> Correct the match rule so we are not going to get false positive match.
>>
>> Fixes: 2c9fc9bf45f8 ("drm: omapdrm: Move FEAT_HDMI_* features to hdmi4
>> driver")
>>
>> CC: sta...@vger.kernel.org # 4.14
>> Signed-off-by: Peter Ujfalusi
>> ---
>> drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 17 ++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
>> b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
>> index 62e451162d96..07945a40c33a 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
>> @@ -902,9 +902,20 @@ static const struct hdmi4_features hdmi4_es3_features =
>> {
>> };
>>
>> static const struct soc_device_attribute hdmi4_soc_devices[] = {
>> -{ .family = "OMAP4", .revision = "ES1.?", .data = &hdmi4_es1_features },
>> -{ .family = "OMAP4", .revision = "ES2.?", .data = &hdmi4_es2_features },
>> -{ .family = "OMAP4", .data = &hdmi4_es3_features },
>> +{
>> +.machine = "OMAP4430",
>> +.revision = "ES1.?",
>> +.data = &hdmi4_es1_features,
>> +},
>> +{
>> +.machine = "OMAP4430",
>> +.revision = "ES2.?",
>> +.data = &hdmi4_es2_features,
>> +},
>> +{
>> +.family = "OMAP4",
>> +.data = &hdmi4_es3_features,
>> +},
>> { /* sentinel */ }
>> };
>>
>>
>
> Looks good to me.
>
> Was there are reason to change the format from single-line to multi-line?
in one line it would beyond 80 and just breaking the .data to new line
looked ugly.
> While at it, I think it would make sense to rename hdmi4_es3_features
> to... hdmi4_features? It's not "ES3" in any way.
Yes, that would make sense, but then the hdmi4_es1/2 is not really
correct either as it should be omap4430_es1/2 ...
I'll resend it on Monday with this change.
>
> Tomi
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH AUTOSEL for 4.9 36/56] drm/i915: Fix the level 0 max_wm hack on VLV/CHV
Hi Greg, all, Pardon for the silly question, but I'm struggling to find documentation about this new 'autoselection' process? Where can one read up on it - be that about the tooling or the heuristics used? I think the above may be the core reason behind the discussion here. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL for 4.9 36/56] drm/i915: Fix the level 0 max_wm hack on VLV/CHV
On Fri, Nov 17, 2017 at 02:53:43PM +0200, Ville Syrjälä wrote: > On Fri, Nov 17, 2017 at 01:41:23PM +0100, Greg KH wrote: > > On Fri, Nov 17, 2017 at 01:28:05PM +0200, Jani Nikula wrote: > > > > > > Cc: Greg > > > > > > On Wed, 15 Nov 2017, Ville Syrjälä wrote: > > > > On Wed, Nov 15, 2017 at 04:44:54PM +, alexander.le...@verizon.com > > > > wrote: > > > >> On Wed, Nov 15, 2017 at 01:08:05PM +0200, Ville Syrjälä wrote: > > > >> >On Wed, Nov 15, 2017 at 02:45:43AM +, alexander.le...@verizon.com > > > >> >wrote: > > > >> >> From: Ville Syrjälä > > > >> >> > > > >> >> [ Upstream commit 1be4d3793d5a93daddcd9be657c429b38ad750a3 ] > > > >> >> > > > >> >> The watermark should never exceed the FIFO size, so we need to > > > >> >> check against the current FIFO size instead of the theoretical > > > >> >> maximum when we clamp the level 0 watermark. > > > >> >> > > > >> >> Signed-off-by: Ville Syrjälä > > > >> >> Link: > > > >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.freedesktop.org_patch_msgid_1480354637-2D14209-2D4-2Dgit-2Dsend-2Demail-2Dville.syrjala-40linux.intel.com&d=DwIDAw&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=iuPtUar-VEGbH1jmVH_UTr4C02X8fmjHUfNYix-Yc0Y&s=ha_F0zP3A1Aztp5S5e6_bqdhiuuPXhn0dRWQ58vv3Is&e= > > > >> >> Reviewed-by: Maarten Lankhorst > > > >> >> Signed-off-by: Sasha Levin > > > >> > > > > >> >Why are these patches being proposed for stable? They're not straight > > > >> >up > > > >> >fixes for known issues, and there's always a chance that something > > > >> >will > > > >> >break. Who is doing the qa on this? > > > >> > > > >> Hi Ville, > > > >> > > > >> They were selected automatically as part of a new process we're trying > > > >> out. If you disagree with the selection I'd be happy to drop it. > > > > > > > > How does that automatic process decide that a patch should be > > > > backported? > > > > > > > > drm and i915 are very fast moving targets so unintended side effects > > > > from > > > > backported patches is a real possibility. So I would recommend against > > > > backporting anything that isn't fixing a real issue affecting users. We > > > > do try to add the cc:stable to such patches. > > > > > > Agreed. > > > > > > First, I think an automatic backport process is against the stable > > > kernel rules (e.g. "It must fix a real bug that bothers people"). > > > > It's finding lots of fixes that did bother people enough to submit a fix > > for. > > > > > Second, we can't and won't take any responsibility for backports we > > > didn't indicate with Cc: stable, a Fixes: tag, or a specific backport > > > request. > > > > Ok, you all are already totally messing with my normal stable workflow, > > so might as well just trust you all completely. So let's just only take > > patches that you all do send me in the normal way. It's easy for Sasha > > to filter out the drm/i915 patches from his results. > > > > Is that ok? > > > > > If you think there's a commit that should be backported and is known to > > > fix a user visible issue (as per the stable rules!), please check with > > > us first. > > > > Um, that is what he was doing with the cc: of you all on the patch > > itself that started this whole conversation... > > And what were the user visible issues fixed by those backports? We > can't judge the risk/benefit ratio of a backport without knowing what > is supposedly being fixed. Ok fine, if you all don't want any patches automatically picked up and backported, that's your choice, just let us know and we will add it to a blacklist or something to prevent it. Your development process must be so perfect that nothing falls through the cracks and forgets to get marked for stable... greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL for 4.9 36/56] drm/i915: Fix the level 0 max_wm hack on VLV/CHV
On Fri, 17 Nov 2017, Greg KH wrote:
> On Fri, Nov 17, 2017 at 01:28:05PM +0200, Jani Nikula wrote:
>>
>> Cc: Greg
>>
>> On Wed, 15 Nov 2017, Ville Syrjälä wrote:
>> > On Wed, Nov 15, 2017 at 04:44:54PM +, alexander.le...@verizon.com
>> > wrote:
>> >> On Wed, Nov 15, 2017 at 01:08:05PM +0200, Ville Syrjälä wrote:
>> >> >On Wed, Nov 15, 2017 at 02:45:43AM +, alexander.le...@verizon.com
>> >> >wrote:
>> >> >> From: Ville Syrjälä
>> >> >>
>> >> >> [ Upstream commit 1be4d3793d5a93daddcd9be657c429b38ad750a3 ]
>> >> >>
>> >> >> The watermark should never exceed the FIFO size, so we need to
>> >> >> check against the current FIFO size instead of the theoretical
>> >> >> maximum when we clamp the level 0 watermark.
>> >> >>
>> >> >> Signed-off-by: Ville Syrjälä
>> >> >> Link:
>> >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.freedesktop.org_patch_msgid_1480354637-2D14209-2D4-2Dgit-2Dsend-2Demail-2Dville.syrjala-40linux.intel.com&d=DwIDAw&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=iuPtUar-VEGbH1jmVH_UTr4C02X8fmjHUfNYix-Yc0Y&s=ha_F0zP3A1Aztp5S5e6_bqdhiuuPXhn0dRWQ58vv3Is&e=
>> >> >> Reviewed-by: Maarten Lankhorst
>> >> >> Signed-off-by: Sasha Levin
>> >> >
>> >> >Why are these patches being proposed for stable? They're not straight up
>> >> >fixes for known issues, and there's always a chance that something will
>> >> >break. Who is doing the qa on this?
>> >>
>> >> Hi Ville,
>> >>
>> >> They were selected automatically as part of a new process we're trying
>> >> out. If you disagree with the selection I'd be happy to drop it.
>> >
>> > How does that automatic process decide that a patch should be backported?
>> >
>> > drm and i915 are very fast moving targets so unintended side effects from
>> > backported patches is a real possibility. So I would recommend against
>> > backporting anything that isn't fixing a real issue affecting users. We
>> > do try to add the cc:stable to such patches.
>>
>> Agreed.
>>
>> First, I think an automatic backport process is against the stable
>> kernel rules (e.g. "It must fix a real bug that bothers people").
>
> It's finding lots of fixes that did bother people enough to submit a fix
> for.
I still have no idea how this autoselect picks up patches that do *not*
have cc: stable nor Fixes: from us. What information do you have that we
don't for making that call?
BR,
Jani.
>> Second, we can't and won't take any responsibility for backports we
>> didn't indicate with Cc: stable, a Fixes: tag, or a specific backport
>> request.
>
> Ok, you all are already totally messing with my normal stable workflow,
> so might as well just trust you all completely. So let's just only take
> patches that you all do send me in the normal way. It's easy for Sasha
> to filter out the drm/i915 patches from his results.
>
> Is that ok?
>
>> If you think there's a commit that should be backported and is known to
>> fix a user visible issue (as per the stable rules!), please check with
>> us first.
>
> Um, that is what he was doing with the cc: of you all on the patch
> itself that started this whole conversation...
>
> {sigh}
>
> greg k-h
--
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] omapdrm: hdmi4: Correct the SoC revision matching
On 17/11/17 10:00, Peter Ujfalusi wrote:
> I believe the intention of the commit 2c9fc9bf45f8
> ("drm: omapdrm: Move FEAT_HDMI_* features to hdmi4 driver")
> was to identify omap4430 ES1.x, omap4430 ES2.x and other OMAP4 revisions,
> like omap4460.
>
> By using family=OMAP4 in the match the code will treat omap4460 ES1.x in a
> same way as it would treat omap4430 ES1.x
>
> This breaks HDMI audio on OMAP4460 devices (PandaES for example).
>
> Correct the match rule so we are not going to get false positive match.
>
> Fixes: 2c9fc9bf45f8 ("drm: omapdrm: Move FEAT_HDMI_* features to hdmi4
> driver")
>
> CC: sta...@vger.kernel.org # 4.14
> Signed-off-by: Peter Ujfalusi
> ---
> drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 17 ++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> index 62e451162d96..07945a40c33a 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> @@ -902,9 +902,20 @@ static const struct hdmi4_features hdmi4_es3_features = {
> };
>
> static const struct soc_device_attribute hdmi4_soc_devices[] = {
> - { .family = "OMAP4", .revision = "ES1.?", .data = &hdmi4_es1_features },
> - { .family = "OMAP4", .revision = "ES2.?", .data = &hdmi4_es2_features },
> - { .family = "OMAP4", .data = &hdmi4_es3_features },
> + {
> + .machine = "OMAP4430",
> + .revision = "ES1.?",
> + .data = &hdmi4_es1_features,
> + },
> + {
> + .machine = "OMAP4430",
> + .revision = "ES2.?",
> + .data = &hdmi4_es2_features,
> + },
> + {
> + .family = "OMAP4",
> + .data = &hdmi4_es3_features,
> + },
> { /* sentinel */ }
> };
>
>
Looks good to me.
Was there are reason to change the format from single-line to multi-line?
While at it, I think it would make sense to rename hdmi4_es3_features
to... hdmi4_features? It's not "ES3" in any way.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL for 4.9 36/56] drm/i915: Fix the level 0 max_wm hack on VLV/CHV
On Fri, Nov 17, 2017 at 01:41:23PM +0100, Greg KH wrote: > On Fri, Nov 17, 2017 at 01:28:05PM +0200, Jani Nikula wrote: > > > > Cc: Greg > > > > On Wed, 15 Nov 2017, Ville Syrjälä wrote: > > > On Wed, Nov 15, 2017 at 04:44:54PM +, alexander.le...@verizon.com > > > wrote: > > >> On Wed, Nov 15, 2017 at 01:08:05PM +0200, Ville Syrjälä wrote: > > >> >On Wed, Nov 15, 2017 at 02:45:43AM +, alexander.le...@verizon.com > > >> >wrote: > > >> >> From: Ville Syrjälä > > >> >> > > >> >> [ Upstream commit 1be4d3793d5a93daddcd9be657c429b38ad750a3 ] > > >> >> > > >> >> The watermark should never exceed the FIFO size, so we need to > > >> >> check against the current FIFO size instead of the theoretical > > >> >> maximum when we clamp the level 0 watermark. > > >> >> > > >> >> Signed-off-by: Ville Syrjälä > > >> >> Link: > > >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.freedesktop.org_patch_msgid_1480354637-2D14209-2D4-2Dgit-2Dsend-2Demail-2Dville.syrjala-40linux.intel.com&d=DwIDAw&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=iuPtUar-VEGbH1jmVH_UTr4C02X8fmjHUfNYix-Yc0Y&s=ha_F0zP3A1Aztp5S5e6_bqdhiuuPXhn0dRWQ58vv3Is&e= > > >> >> Reviewed-by: Maarten Lankhorst > > >> >> Signed-off-by: Sasha Levin > > >> > > > >> >Why are these patches being proposed for stable? They're not straight up > > >> >fixes for known issues, and there's always a chance that something will > > >> >break. Who is doing the qa on this? > > >> > > >> Hi Ville, > > >> > > >> They were selected automatically as part of a new process we're trying > > >> out. If you disagree with the selection I'd be happy to drop it. > > > > > > How does that automatic process decide that a patch should be backported? > > > > > > drm and i915 are very fast moving targets so unintended side effects from > > > backported patches is a real possibility. So I would recommend against > > > backporting anything that isn't fixing a real issue affecting users. We > > > do try to add the cc:stable to such patches. > > > > Agreed. > > > > First, I think an automatic backport process is against the stable > > kernel rules (e.g. "It must fix a real bug that bothers people"). > > It's finding lots of fixes that did bother people enough to submit a fix > for. > > > Second, we can't and won't take any responsibility for backports we > > didn't indicate with Cc: stable, a Fixes: tag, or a specific backport > > request. > > Ok, you all are already totally messing with my normal stable workflow, > so might as well just trust you all completely. So let's just only take > patches that you all do send me in the normal way. It's easy for Sasha > to filter out the drm/i915 patches from his results. > > Is that ok? > > > If you think there's a commit that should be backported and is known to > > fix a user visible issue (as per the stable rules!), please check with > > us first. > > Um, that is what he was doing with the cc: of you all on the patch > itself that started this whole conversation... And what were the user visible issues fixed by those backports? We can't judge the risk/benefit ratio of a backport without knowing what is supposedly being fixed. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for v4.15
On 16.11.2017 21:57, Dave Airlie wrote: On 16 November 2017 at 14:59, Linus Torvalds wrote: On Wed, Nov 15, 2017 at 6:34 PM, Dave Airlie wrote: There is some code touched on sound/soc, but I think the sound tree should have the same commits from the same base,so this may luck different if you pulled it as I generated my pull request a couple of days ago. Otherwise the highlights are below. I'm more curious about (and disgusted by) this one: include/dt-bindings/msm/msm-bus-ids.h wtf? It's full of defines that aren't actually used anywhere. Which is just as well, since it doesn't seem to be included from anything either. There's something odd about drm people. You guys like these completely insane generated header files, and you seem to be populating the whole tree with this odd and diseased notion of "generated header files are cool". Is somebody getting paid by line of code? It would still cost less than transcribing each register and all it's fields by hand from pdfs generated from the same place. This raises the question of how people feel about putting the source database into the kernel (most likely as XML in our case) and auto-generating the headers from there instead. I've been pondering doing this in Mesa for radeonsi for quite some time now. Given that the Mesa header style is different from the kernel header style, this could help reduce our IP review load going forward, and would have some other neat benefits as well. Cheers, Nicolai -- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/10] drm/edid: Fix cea mode aspect ratio handling
On Fri, Nov 17, 2017 at 05:50:11PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 11/17/2017 5:05 PM, Ville Syrjälä wrote:
> > On Fri, Nov 17, 2017 at 08:49:49AM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 11/16/2017 9:53 PM, Ville Syrjälä wrote:
> >>> On Thu, Nov 16, 2017 at 08:21:44PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 11/13/2017 10:34 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > commit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer")
> > cause us to not send out any VICs in the AVI infoframes. That commit
> > was since reverted, but if and when we add aspect ratio handing back
> > we need to be more careful.
> >
> > Let's handle this by considering the aspect ratio as a requirement
> > for cea mode matching only if the passed in mode actually has a
> > non-zero aspect ratio field. This will keep userspace that doesn't
> > provide an aspect ratio working as before by matching it to the
> > first otherwise equal cea mode. And once userspace starts to
> > provide the aspect ratio it will be considerd a hard requirement
> > for the match.
> >
> > Also change the hdmi mode matching to use drm_mode_match() for
> > consistency, but we don't match on aspect ratio there since the
> > spec doesn't list a specific aspect ratio for those modes.
> >
> > Cc: Shashank Sharma
> > Cc: "Lin, Jia"
> > Cc: Akashdeep Sharma
> > Cc: Jim Bride
> > Cc: Jose Abreu
> > Cc: Daniel Vetter
> > Cc: Emil Velikov
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/drm_edid.c | 18 ++
> > 1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 7220b8f9a7e8..00aa98f3e55d 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -2903,11 +2903,15 @@ cea_mode_alternate_timings(u8 vic, struct
> > drm_display_mode *mode)
> > static u8 drm_match_cea_mode_clock_tolerance(const struct
> > drm_display_mode *to_match,
> > unsigned int
> > clock_tolerance)
> > {
> > + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS |
> > DRM_MODE_MATCH_FLAGS;
> > u8 vic;
> >
> > if (!to_match->clock)
> > return 0;
> >
> > + if (to_match->picture_aspect_ratio)
> > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
> This doesn't look right. This means we are expecting a CEA mode without
> a pic aspect ratio field,
> which is invalid.
> >>> No, it's perfectly valid. It's what we currently get from userspace.
> >> Yep, but that's due to missing Aspect ratio handling in the DRM layer.
> >> If that's fixed, as per the list of CEA modes,
> >> each CEA VIC contains an aspect ratio, which is a part of its unique
> >> identity.
> >>
> >> I guess once we have the aspect ratio handling in DRM layer, it
> >> would/should look like this:
> >> - EDID gives you all supported modes, including CEA modes with Aspect ratio
> >> - Userspcae gets the mode information, with aspect ratio (for CEA modes)
> >> If ( Userspace picks one of the CEA modes)
> >> - sends a modeset
> >> - we find a matching CEA VIC, found one from modedb
> >> - we load this VIC = nonzero information in AVI IF VIC field,
> >> else
> >> - sends a modeset
> >> - we could not find a matching CEA VIC, as aspect ratio is 0
> >> - we make VIC field in AVI IF as 0a
> > No. That would break current userspace.
> I guess I forgot to make it clear, that userspace will set the cap, only
> then we will provide aspect ratio information.
> So this should not break userspace, isn't it ?
> >> This is important, as HDMI compliance test 7-27 inspects if the VIC
> >> field in the AVI IF is accurate.
> > Complicance is secondary to not breaking things that work. Also I find
> > it hard to see what purpose there is in having a complicance test that
> > sets a CEA modes w/o aspect ratio and then expects the infoframe to have
> > VIC 0.
> Again, typically this is how these analyzers force modeset:
> - They send EDID with only one mode, which is the test mode, with aspect
> ratio.
> - They expect that VIC to be present in AVI IF
> >> - Shashank
> Ankit is going to publish the aspect ratio patch
> series again, with proper DRM cap and flags check. Would it be
> ok if we have a look that handling first ?
> >>> This patch will be needed by that work. Otherwise we're going to stop
> >>> sending a VIC for CEA modes with current userspace.
> >> I guess we should force userspaces to start bothering about aspect ratio
> >> field, right now we
> >> are doing this for Wayland b
Re: [PATCH AUTOSEL for 4.9 36/56] drm/i915: Fix the level 0 max_wm hack on VLV/CHV
On Fri, Nov 17, 2017 at 01:28:05PM +0200, Jani Nikula wrote:
>
> Cc: Greg
>
> On Wed, 15 Nov 2017, Ville Syrjälä wrote:
> > On Wed, Nov 15, 2017 at 04:44:54PM +, alexander.le...@verizon.com wrote:
> >> On Wed, Nov 15, 2017 at 01:08:05PM +0200, Ville Syrjälä wrote:
> >> >On Wed, Nov 15, 2017 at 02:45:43AM +, alexander.le...@verizon.com
> >> >wrote:
> >> >> From: Ville Syrjälä
> >> >>
> >> >> [ Upstream commit 1be4d3793d5a93daddcd9be657c429b38ad750a3 ]
> >> >>
> >> >> The watermark should never exceed the FIFO size, so we need to
> >> >> check against the current FIFO size instead of the theoretical
> >> >> maximum when we clamp the level 0 watermark.
> >> >>
> >> >> Signed-off-by: Ville Syrjälä
> >> >> Link:
> >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.freedesktop.org_patch_msgid_1480354637-2D14209-2D4-2Dgit-2Dsend-2Demail-2Dville.syrjala-40linux.intel.com&d=DwIDAw&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=iuPtUar-VEGbH1jmVH_UTr4C02X8fmjHUfNYix-Yc0Y&s=ha_F0zP3A1Aztp5S5e6_bqdhiuuPXhn0dRWQ58vv3Is&e=
> >> >> Reviewed-by: Maarten Lankhorst
> >> >> Signed-off-by: Sasha Levin
> >> >
> >> >Why are these patches being proposed for stable? They're not straight up
> >> >fixes for known issues, and there's always a chance that something will
> >> >break. Who is doing the qa on this?
> >>
> >> Hi Ville,
> >>
> >> They were selected automatically as part of a new process we're trying
> >> out. If you disagree with the selection I'd be happy to drop it.
> >
> > How does that automatic process decide that a patch should be backported?
> >
> > drm and i915 are very fast moving targets so unintended side effects from
> > backported patches is a real possibility. So I would recommend against
> > backporting anything that isn't fixing a real issue affecting users. We
> > do try to add the cc:stable to such patches.
>
> Agreed.
>
> First, I think an automatic backport process is against the stable
> kernel rules (e.g. "It must fix a real bug that bothers people").
It's finding lots of fixes that did bother people enough to submit a fix
for.
> Second, we can't and won't take any responsibility for backports we
> didn't indicate with Cc: stable, a Fixes: tag, or a specific backport
> request.
Ok, you all are already totally messing with my normal stable workflow,
so might as well just trust you all completely. So let's just only take
patches that you all do send me in the normal way. It's easy for Sasha
to filter out the drm/i915 patches from his results.
Is that ok?
> If you think there's a commit that should be backported and is known to
> fix a user visible issue (as per the stable rules!), please check with
> us first.
Um, that is what he was doing with the cc: of you all on the patch
itself that started this whole conversation...
{sigh}
greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] display: panel: Add Tianma tm070rvhg71 display support (800x480)
On Fri, Nov 17, 2017 at 01:17:12PM +0100, Lukasz Majewski wrote: > Hi Thierry, > > > On Fri, Nov 17, 2017 at 11:02:47AM +0100, Lukasz Majewski wrote: > > > Dear All, > > > > > > > On Tue, Nov 07, 2017 at 04:30:58PM +0100, Lukasz Majewski wrote: > > > > > Signed-off-by: Lukasz Majewski > > > > > > > > > > --- > > > > > Changes for v2: > > > > > - Provide more > > > > > detailed ./Documentation/devicetree/bindings/display/panel > > > > > entry to describe this panel device. --- > > > > > .../bindings/display/panel/tianma,tm070rvhg71.txt | 29 > > > > > ++ > > > > > drivers/gpu/drm/panel/panel-simple.c | 27 > > > > > 2 files changed, 56 insertions(+) create > > > > > mode 100644 > > > > > Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt > > > > > > > > > > > > > Acked-by: Rob Herring > > > > > > Is there a chance that this patch will find its way to v4.15-rc1 ? > > > > No, that's very unlikely. We're in the middle of the merge window, no > > new patches are accepted at that point. > > Ok. I see. I should have sent the ping earlier. Looking at the timeline of this patch even that wouldn't have been enough. You sent the original patch on October 21, which is one day after the final drm-misc feature pull request was tagged. So this wouldn't have made it into v4.15-rc1 no matter how early you would have pinged. =) Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/10] drm/edid: Fix cea mode aspect ratio handling
Regards
Shashank
On 11/17/2017 5:05 PM, Ville Syrjälä wrote:
On Fri, Nov 17, 2017 at 08:49:49AM +0530, Sharma, Shashank wrote:
Regards
Shashank
On 11/16/2017 9:53 PM, Ville Syrjälä wrote:
On Thu, Nov 16, 2017 at 08:21:44PM +0530, Sharma, Shashank wrote:
Regards
Shashank
On 11/13/2017 10:34 PM, Ville Syrjala wrote:
From: Ville Syrjälä
commit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer")
cause us to not send out any VICs in the AVI infoframes. That commit
was since reverted, but if and when we add aspect ratio handing back
we need to be more careful.
Let's handle this by considering the aspect ratio as a requirement
for cea mode matching only if the passed in mode actually has a
non-zero aspect ratio field. This will keep userspace that doesn't
provide an aspect ratio working as before by matching it to the
first otherwise equal cea mode. And once userspace starts to
provide the aspect ratio it will be considerd a hard requirement
for the match.
Also change the hdmi mode matching to use drm_mode_match() for
consistency, but we don't match on aspect ratio there since the
spec doesn't list a specific aspect ratio for those modes.
Cc: Shashank Sharma
Cc: "Lin, Jia"
Cc: Akashdeep Sharma
Cc: Jim Bride
Cc: Jose Abreu
Cc: Daniel Vetter
Cc: Emil Velikov
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7220b8f9a7e8..00aa98f3e55d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2903,11 +2903,15 @@ cea_mode_alternate_timings(u8 vic, struct
drm_display_mode *mode)
static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode
*to_match,
unsigned int clock_tolerance)
{
+ unsigned int match_flags = DRM_MODE_MATCH_TIMINGS |
DRM_MODE_MATCH_FLAGS;
u8 vic;
if (!to_match->clock)
return 0;
+ if (to_match->picture_aspect_ratio)
+ match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
This doesn't look right. This means we are expecting a CEA mode without
a pic aspect ratio field,
which is invalid.
No, it's perfectly valid. It's what we currently get from userspace.
Yep, but that's due to missing Aspect ratio handling in the DRM layer.
If that's fixed, as per the list of CEA modes,
each CEA VIC contains an aspect ratio, which is a part of its unique
identity.
I guess once we have the aspect ratio handling in DRM layer, it
would/should look like this:
- EDID gives you all supported modes, including CEA modes with Aspect ratio
- Userspcae gets the mode information, with aspect ratio (for CEA modes)
If ( Userspace picks one of the CEA modes)
- sends a modeset
- we find a matching CEA VIC, found one from modedb
- we load this VIC = nonzero information in AVI IF VIC field,
else
- sends a modeset
- we could not find a matching CEA VIC, as aspect ratio is 0
- we make VIC field in AVI IF as 0a
No. That would break current userspace.
I guess I forgot to make it clear, that userspace will set the cap, only
then we will provide aspect ratio information.
So this should not break userspace, isn't it ?
This is important, as HDMI compliance test 7-27 inspects if the VIC
field in the AVI IF is accurate.
Complicance is secondary to not breaking things that work. Also I find
it hard to see what purpose there is in having a complicance test that
sets a CEA modes w/o aspect ratio and then expects the infoframe to have
VIC 0.
Again, typically this is how these analyzers force modeset:
- They send EDID with only one mode, which is the test mode, with aspect
ratio.
- They expect that VIC to be present in AVI IF
- Shashank
Ankit is going to publish the aspect ratio patch
series again, with proper DRM cap and flags check. Would it be
ok if we have a look that handling first ?
This patch will be needed by that work. Otherwise we're going to stop
sending a VIC for CEA modes with current userspace.
I guess we should force userspaces to start bothering about aspect ratio
field, right now we
are doing this for Wayland based compositors, may be we should extend it
to X based too.
Yes, I've been saying that someone should look into extending the randr
protocol with the necessary bits. But that still doesn't allow us to
change the current behaviour as old userspace would anyway linger around
for a long time.
I think cap will cove this part
- Shashank
- Shashank
+
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
struct drm_display_mode cea_mode = edid_cea_modes[vic];
unsigned int clock1, clock2;
@@ -2921,7 +2925,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct
drm_display_mode *to_m
continue;
do {
- if (drm_mode_equal_no_clocks_no_stereo(t
[Bug 103791] Tearing after screen wakeup/on
https://bugs.freedesktop.org/show_bug.cgi?id=103791 --- Comment #3 from denisgolo...@yandex.ru --- Created attachment 135554 --> https://bugs.freedesktop.org/attachment.cgi?id=135554&action=edit Xorg.0.log -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103791] Tearing after screen wakeup/on
https://bugs.freedesktop.org/show_bug.cgi?id=103791 --- Comment #2 from denisgolo...@yandex.ru --- Created attachment 135553 --> https://bugs.freedesktop.org/attachment.cgi?id=135553&action=edit dmesg -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103791] Tearing after screen wakeup/on
https://bugs.freedesktop.org/show_bug.cgi?id=103791 --- Comment #1 from denisgolo...@yandex.ru --- Created attachment 135552 --> https://bugs.freedesktop.org/attachment.cgi?id=135552&action=edit xorg.conf -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103791] Tearing after screen wakeup/on
https://bugs.freedesktop.org/show_bug.cgi?id=103791 Bug ID: 103791 Summary: Tearing after screen wakeup/on Product: DRI Version: XOrg git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: denisgolo...@yandex.ru Hi Sometimes after screen wakeup/turning on I start to experience tearing. Tear free mode works fine before sleep/turning display off. My hardware is XFX Radeon RX580. Software: kernel-4.13.13+ck1, xorg-1.19.5, xf86-video-amdgpu (git 3a4f7422913093ed9e26b73ecd7f9e773478cb1e), libdrm-2.4.88 See dmesg, Xorg.0.log, xorg.conf attached. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] display: panel: Add Mitsubishi aa070mc01 display support (800x480)
On Sat, Oct 21, 2017 at 12:06:09AM +0200, Lukasz Majewski wrote: > This commit adds support for Mitsubishi aa070mc01 TFT panel working > with 8 bit ISP mode (pin 19 "mode" HIGH for 20 pin TFT connector). > > Signed-off-by: Lukasz Majewski > --- > Changes for v2: > - Place the code sorted alphabetically > - Add missing ./Documentation/devicetree/binding/display properties > description > --- > .../display/panel/mitsubishi,aa070mc01.txt | 7 + > drivers/gpu/drm/panel/panel-simple.c | 35 > ++ > 2 files changed, 42 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/mitsubishi,aa070mc01.txt Applied to drm-misc-next, thanks. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] display: panel: Add Tianma tm070rvhg71 display support (800x480)
On Fri, Nov 17, 2017 at 11:02:47AM +0100, Lukasz Majewski wrote: > Dear All, > > > On Tue, Nov 07, 2017 at 04:30:58PM +0100, Lukasz Majewski wrote: > > > Signed-off-by: Lukasz Majewski > > > > > > --- > > > Changes for v2: > > > - Provide more > > > detailed ./Documentation/devicetree/bindings/display/panel entry to > > > describe this panel device. --- > > > .../bindings/display/panel/tianma,tm070rvhg71.txt | 29 > > > ++ > > > drivers/gpu/drm/panel/panel-simple.c | 27 > > > 2 files changed, 56 insertions(+) create mode > > > 100644 > > > Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt > > > > Acked-by: Rob Herring > > Is there a chance that this patch will find its way to v4.15-rc1 ? No, that's very unlikely. We're in the middle of the merge window, no new patches are accepted at that point. If you want to make sure a patch goes into the next release, make sure it is posted and reviewed at least a few days before -rc6 of the prior release. -rc6 is a common cut-off point for subsystems (though not all) in the kernel. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/10] drm/edid: Don't send bogus aspect ratios in AVI infoframes
On Fri, Nov 17, 2017 at 08:53:54AM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 11/16/2017 9:56 PM, Ville Syrjälä wrote:
> > On Thu, Nov 16, 2017 at 08:31:36PM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 11/13/2017 10:34 PM, Ville Syrjala wrote:
> >>> From: Ville Syrjälä
> >>>
> >>> If the user mode would specify an aspect ratio other than 4:3 or 16:9
> >>> we now silently ignore it. Maybe a better apporoach is to return an
> >>> error? Let's try that.
> >>>
> >>> Also we must be careful that we don't try to send illegal picture
> >>> aspect in the infoframe as it's only capable of signalling none,
> >>> 4:3, and 16:9. Currently we're sending these bogus infoframes
> >>> whenever the cea mode specifies some other aspect ratio.
> >>>
> >>> Cc: Shashank Sharma
> >>> Cc: Sean Paul
> >>> Cc: Jose Abreu
> >>> Cc: Daniel Vetter
> >>> Cc: Emil Velikov
> >>> Signed-off-by: Ville Syrjälä
> >>> ---
> >>>drivers/gpu/drm/drm_edid.c | 23 +--
> >>>1 file changed, 17 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>> index 00aa98f3e55d..bafb3ee4ea97 100644
> >>> --- a/drivers/gpu/drm/drm_edid.c
> >>> +++ b/drivers/gpu/drm/drm_edid.c
> >>> @@ -4786,6 +4786,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
> >>> hdmi_avi_infoframe *frame,
> >>>const struct drm_display_mode
> >>> *mode,
> >>>bool is_hdmi2_sink)
> >>>{
> >>> + enum hdmi_picture_aspect picture_aspect;
> >>> int err;
> >>>
> >>> if (!frame || !mode)
> >>> @@ -4828,13 +4829,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
> >>> hdmi_avi_infoframe *frame,
> >>>* Populate picture aspect ratio from either
> >>>* user input (if specified) or from the CEA mode list.
> >>>*/
> >>> - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
> >>> - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
> >>> - frame->picture_aspect = mode->picture_aspect_ratio;
> >>> - else if (frame->video_code > 0)
> >>> - frame->picture_aspect = drm_get_cea_aspect_ratio(
> >>> - frame->video_code);
> >>> + picture_aspect = mode->picture_aspect_ratio;
> >>> + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE)
> >>> + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code);
> >> This is slightly going in the loop.
> >> - During the modeset the driver cant specify the aspect ratio
> >> information, as DRM layer lacks this support.
> >> - So we fill the VIC field, by comparing the mode with the
> >> DRM_CEA_MODES[] list. This will pick the first mode
> >> available in the list (regardless of its aspect ratio), and fill the
> >> VIC, as we don't consider aspect ratio while comparing timings.
> >> - Again, now while sending the aspect ratio, we are picking up the VIC,
> >> which may not be correct.
> >>
> >> So if we have 720x480(4:3) and 720x480(16:9) in the list, as 4:3 is
> >> first in list, we will always pick 4:3 aspect ratio.
> > Yes. The user didn't care about the aspect ratio (or rather couldn't
> > specify one) so we just pick one. Which is exactly what we've been
> > doing ever since we started sending the VIC in the infoframe.
> Correct, and we are hoping that this should be better (if not fixed)
> with the aspect ratio support
> patches + DRM cap. If the userspace doesn't set the cap, then anyways
> there is no aspect ratio
> field available, and VIC would be always 0, as this becomes a Non CEA mode.
>
> Or do you think it would be a better idea to send some VIC instead of No
> VIC, when userspace doesn't
> set the DRM cap for aspect ratio ?
Yes. That's the current behaviour.
IIRC some crappy amplifiers etc. with HDMI passthrough don't even work
correctly unless VIC is specified. Hence we do want to send it whenever
possible.
>
> - Shashank
> >> - Shashank
> >>>
> >>> + /*
> >>> + * The infoframe can't convey anything but none, 4:3
> >>> + * and 16:9, so if the user has asked for anything else
> >>> + * we can only satisfy it by specifying the right VIC.
> >>> + */
> >>> + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) {
> >>> + if (picture_aspect !=
> >>> + drm_get_cea_aspect_ratio(frame->video_code))
> >>> + return -EINVAL;
> >>> + picture_aspect = HDMI_PICTURE_ASPECT_NONE;
> >>> + }
> >>> +
> >>> + frame->picture_aspect = picture_aspect;
> >>> frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
> >>> frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
> >>>
--
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] display: panel: Add Tianma tm070rvhg71 display support (800x480)
On Tue, Nov 07, 2017 at 04:30:58PM +0100, Lukasz Majewski wrote:
> Signed-off-by: Lukasz Majewski
>
> ---
> Changes for v2:
> - Provide more detailed ./Documentation/devicetree/bindings/display/panel
> entry to describe this panel device.
> ---
> .../bindings/display/panel/tianma,tm070rvhg71.txt | 29
> ++
> drivers/gpu/drm/panel/panel-simple.c | 27
> 2 files changed, 56 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
I've applied this, though I had to make a few modifications. First, I
added a commit message. Commits should always have one.
Also, please send DT bindings and driver changes as separate patches in
the future.
Device tree bindings should have a subject prefixed with any of these:
dt-bindings:
dt-bindings: display:
dt-bindings: display: panel:
Though the latter two are fairly long by themselves, so you don't have a
lot of room for the important bits.
Please also prefix the subject of panel driver patches with a:
drm/panel:
Which makes it easier to identify relevant patches among loads and loads
of other email.
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
> b/Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
> new file mode 100644
> index ..02562867444d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
> @@ -0,0 +1,29 @@
> +Tianma Micro-electronics TM070RVHG71 7.0" WXGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "tianma,tm070rvhg71
Added a missing " at the end here.
> +- power-supply: single regulator to provide the supply voltage
> +- backlight: phandle of the backlight device attached to the panel
> +
> +Required nodes:
> +- port: LVDS port mapping to connect this display
> +
> +This panel needs single power supply voltage. Its backlight is conntrolled
> +via PWM signal.
> +
> +Example:
> +
> +
> +Example device-tree definition when connected to iMX6Q based board
> +
> + panel: panel-lvds0 {
> + compatible = "tianma,tm070rvhg71";
> + backlight = <&backlight_lvds>;
> + power-supply = <®_lvds>;
> +
> + port {
> + panel_in_lvds0: endpoint {
> + remote-endpoint = <&lvds0_out>;
> + };
> + };
> + };
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 3d2cb8bc4d94..07188dc084df 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1831,6 +1831,30 @@ static const struct panel_desc tianma_tm070jdhg30 = {
> .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> };
>
> +static const struct display_timing tianma_tm070rvhg71_timing = {
> + .pixelclock = { 2770, 2920, 3960 },
> + .hactive = { 800, 800, 800 },
> + .hfront_porch = { 12, 40, 212 },
> + .hback_porch = { 88, 88, 88 },
> + .hsync_len = { 1, 1, 40 },
> + .vactive = { 480, 480, 480 },
> + .vfront_porch = { 1, 13, 88 },
> + .vback_porch = { 32, 32, 32 },
> + .vsync_len = { 1, 1, 3 },
> + .flags = DISPLAY_FLAGS_DE_HIGH,
> +};
> +
> +static const struct panel_desc tianma_tm070rvhg71 = {
> + .timings = &tianma_tm070rvhg71_timing,
> + .num_timings = 1,
> + .bpc = 8,
> + .size = {
> + .width = 154,
> + .height = 86,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> +};
> +
> static const struct drm_display_mode tpk_f07a_0102_mode = {
> .clock = 33260,
> .hdisplay = 800,
> @@ -2113,6 +2137,9 @@ static const struct of_device_id platform_of_match[] = {
> .compatible = "tianma,tm070jdhg30",
> .data = &tianma_tm070jdhg30,
> }, {
> + .compatible = "tianma,tm070rvhg71",
> + .data = &tianma_tm070rvhg71,
> + }, {
> .compatible = "tpk,f07a-0102",
> .data = &tpk_f07a_0102,
> }, {
Looks like these are actually sorted correctly in your patch. However,
when applying these got added after the Toshiba panel that was recently
added, so I resorted again.
Thierry
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/10] drm/edid: Fix cea mode aspect ratio handling
On Fri, Nov 17, 2017 at 08:49:49AM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 11/16/2017 9:53 PM, Ville Syrjälä wrote:
> > On Thu, Nov 16, 2017 at 08:21:44PM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 11/13/2017 10:34 PM, Ville Syrjala wrote:
> >>> From: Ville Syrjälä
> >>>
> >>> commit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer")
> >>> cause us to not send out any VICs in the AVI infoframes. That commit
> >>> was since reverted, but if and when we add aspect ratio handing back
> >>> we need to be more careful.
> >>>
> >>> Let's handle this by considering the aspect ratio as a requirement
> >>> for cea mode matching only if the passed in mode actually has a
> >>> non-zero aspect ratio field. This will keep userspace that doesn't
> >>> provide an aspect ratio working as before by matching it to the
> >>> first otherwise equal cea mode. And once userspace starts to
> >>> provide the aspect ratio it will be considerd a hard requirement
> >>> for the match.
> >>>
> >>> Also change the hdmi mode matching to use drm_mode_match() for
> >>> consistency, but we don't match on aspect ratio there since the
> >>> spec doesn't list a specific aspect ratio for those modes.
> >>>
> >>> Cc: Shashank Sharma
> >>> Cc: "Lin, Jia"
> >>> Cc: Akashdeep Sharma
> >>> Cc: Jim Bride
> >>> Cc: Jose Abreu
> >>> Cc: Daniel Vetter
> >>> Cc: Emil Velikov
> >>> Signed-off-by: Ville Syrjälä
> >>> ---
> >>>drivers/gpu/drm/drm_edid.c | 18 ++
> >>>1 file changed, 14 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>> index 7220b8f9a7e8..00aa98f3e55d 100644
> >>> --- a/drivers/gpu/drm/drm_edid.c
> >>> +++ b/drivers/gpu/drm/drm_edid.c
> >>> @@ -2903,11 +2903,15 @@ cea_mode_alternate_timings(u8 vic, struct
> >>> drm_display_mode *mode)
> >>>static u8 drm_match_cea_mode_clock_tolerance(const struct
> >>> drm_display_mode *to_match,
> >>>unsigned int
> >>> clock_tolerance)
> >>>{
> >>> + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS |
> >>> DRM_MODE_MATCH_FLAGS;
> >>> u8 vic;
> >>>
> >>> if (!to_match->clock)
> >>> return 0;
> >>>
> >>> + if (to_match->picture_aspect_ratio)
> >>> + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
> >> This doesn't look right. This means we are expecting a CEA mode without
> >> a pic aspect ratio field,
> >> which is invalid.
> > No, it's perfectly valid. It's what we currently get from userspace.
> Yep, but that's due to missing Aspect ratio handling in the DRM layer.
> If that's fixed, as per the list of CEA modes,
> each CEA VIC contains an aspect ratio, which is a part of its unique
> identity.
>
> I guess once we have the aspect ratio handling in DRM layer, it
> would/should look like this:
> - EDID gives you all supported modes, including CEA modes with Aspect ratio
> - Userspcae gets the mode information, with aspect ratio (for CEA modes)
> If ( Userspace picks one of the CEA modes)
> - sends a modeset
> - we find a matching CEA VIC, found one from modedb
> - we load this VIC = nonzero information in AVI IF VIC field,
> else
> - sends a modeset
> - we could not find a matching CEA VIC, as aspect ratio is 0
> - we make VIC field in AVI IF as 0a
No. That would break current userspace.
>
> This is important, as HDMI compliance test 7-27 inspects if the VIC
> field in the AVI IF is accurate.
Complicance is secondary to not breaking things that work. Also I find
it hard to see what purpose there is in having a complicance test that
sets a CEA modes w/o aspect ratio and then expects the infoframe to have
VIC 0.
>
> - Shashank
> >> Ankit is going to publish the aspect ratio patch
> >> series again, with proper DRM cap and flags check. Would it be
> >> ok if we have a look that handling first ?
> > This patch will be needed by that work. Otherwise we're going to stop
> > sending a VIC for CEA modes with current userspace.
> I guess we should force userspaces to start bothering about aspect ratio
> field, right now we
> are doing this for Wayland based compositors, may be we should extend it
> to X based too.
Yes, I've been saying that someone should look into extending the randr
protocol with the necessary bits. But that still doesn't allow us to
change the current behaviour as old userspace would anyway linger around
for a long time.
>
> - Shashank
> >
> >>> +
> >>> for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> >>> struct drm_display_mode cea_mode = edid_cea_modes[vic];
> >>> unsigned int clock1, clock2;
> >>> @@ -2921,7 +2925,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const
> >>> struct drm_display_mode *to_m
> >>> continue;
> >>>
> >>> do {
> >>> - if (drm_
Re: [PATCH AUTOSEL for 4.9 36/56] drm/i915: Fix the level 0 max_wm hack on VLV/CHV
Cc: Greg On Wed, 15 Nov 2017, Ville Syrjälä wrote: > On Wed, Nov 15, 2017 at 04:44:54PM +, alexander.le...@verizon.com wrote: >> On Wed, Nov 15, 2017 at 01:08:05PM +0200, Ville Syrjälä wrote: >> >On Wed, Nov 15, 2017 at 02:45:43AM +, alexander.le...@verizon.com wrote: >> >> From: Ville Syrjälä >> >> >> >> [ Upstream commit 1be4d3793d5a93daddcd9be657c429b38ad750a3 ] >> >> >> >> The watermark should never exceed the FIFO size, so we need to >> >> check against the current FIFO size instead of the theoretical >> >> maximum when we clamp the level 0 watermark. >> >> >> >> Signed-off-by: Ville Syrjälä >> >> Link: >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.freedesktop.org_patch_msgid_1480354637-2D14209-2D4-2Dgit-2Dsend-2Demail-2Dville.syrjala-40linux.intel.com&d=DwIDAw&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=iuPtUar-VEGbH1jmVH_UTr4C02X8fmjHUfNYix-Yc0Y&s=ha_F0zP3A1Aztp5S5e6_bqdhiuuPXhn0dRWQ58vv3Is&e= >> >> Reviewed-by: Maarten Lankhorst >> >> Signed-off-by: Sasha Levin >> > >> >Why are these patches being proposed for stable? They're not straight up >> >fixes for known issues, and there's always a chance that something will >> >break. Who is doing the qa on this? >> >> Hi Ville, >> >> They were selected automatically as part of a new process we're trying >> out. If you disagree with the selection I'd be happy to drop it. > > How does that automatic process decide that a patch should be backported? > > drm and i915 are very fast moving targets so unintended side effects from > backported patches is a real possibility. So I would recommend against > backporting anything that isn't fixing a real issue affecting users. We > do try to add the cc:stable to such patches. Agreed. First, I think an automatic backport process is against the stable kernel rules (e.g. "It must fix a real bug that bothers people"). Second, we can't and won't take any responsibility for backports we didn't indicate with Cc: stable, a Fixes: tag, or a specific backport request. If you think there's a commit that should be backported and is known to fix a user visible issue (as per the stable rules!), please check with us first. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103370] `vblank_mode=0 DRI_PRIME=1 glxgears` will introduce GPU lock up on Intel Graphics [8086:5917] + AMD Graphics [1002:6665] (rev c3)
https://bugs.freedesktop.org/show_bug.cgi?id=103370 --- Comment #29 from Michel Dänzer --- Tearing is expected with vblank_mode=0. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103370] `vblank_mode=0 DRI_PRIME=1 glxgears` will introduce GPU lock up on Intel Graphics [8086:5917] + AMD Graphics [1002:6665] (rev c3)
https://bugs.freedesktop.org/show_bug.cgi?id=103370 Shih-Yuan Lee changed: What|Removed |Added Summary|`DRI_PRIME=1 glxgears |`vblank_mode=0 DRI_PRIME=1 |-info` halts the system |glxgears` will introduce |with Intel Graphics |GPU lock up on Intel |[8086:5917] + AMD Graphics |Graphics [8086:5917] + AMD |[1002:6665] (rev c3)|Graphics [1002:6665] (rev ||c3) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 00/22] drm/cma-helper: Remove drm_fbdev_cma* functions
Den 17.11.2017 10.10, skrev Alexey Brodkin: Hi Noralf, On Thu, 2017-11-16 at 21:11 +0100, Noralf Trønnes wrote: Den 16.11.2017 09.14, skrev Shawn Guo: On Wed, Nov 15, 2017 at 03:19:39PM +0100, Noralf Trønnes wrote: This patchset adds drm_fb_cma_fbdev_init/fini() functions that replaces drm_fbdev_cma_init/fini(). The reason for doing so is to get rid of struct drm_fbdev_cma and it's wrapper functions. The final piece will happen when tinydrm moves away from the cma helper and we can remove the struct. Note: Patches 19-22 depends on patchset: [v3] drm: Add simple modeset suspend/resume helpers Is there a git branch somewhere we can test? Here you go: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notro_linux_tree_drm-5Ffb-5Fcma-5Ffbdev-5Finit&d=DwIDaQ&c=DPL6_X_6JkXFx 7AXWqB0tg&r=OtZvQ4lNHIbjtyysXrNW8RbX6WFkigcev-xByzJ_fLk&m=McbBjcx46wmGkpM3GHmk9URB1xbd6ywS- Z5tpdWwDX8&s=BewulagwMNQa5xW19olMnlzV5DI5cZ_7eDSPyUpzMV8&e= Thanks for that this really helps to test your patches. That's nice to know so I can include it in future patchsets like this. And looks like something is broken for ARC PGU + ADV7511 with your tree: -->8 adv7511: probe of 1-0039 failed with error -2 -2 is -ENOENT There are some changes to adv7511 in drm-misc since 4.14. I suggest you try drm-misc-next directly to see if that works or not. Noralf. arcpgu e0017000.pgu: arc_pgu ID: 0x41440304 arcpgu e0017000.pgu: assigned reserved memory node frame_buffe r@9e00 [drm] Cannot find any crtc or sizes [drm] Cannot find any crtc or sizes [drm] Initialized arcpgu 1.0.0 20160219 for e0017000.pgu on minor 0 -->8 That's what I see on vanilla 4.14 kernel: -->8 arcpgu e0017000.pgu: arc_pgu ID: 0x41440304 arcpgu e0017000.pgu: assigned reserved memory node frame_buffer@9e00 Console: switching to colour frame buffer device 160x45 arcpgu e0017000.pgu: fb0: frame buffer device [drm] Initialized arcpgu 1.0.0 20160219 for e0017000.pgu on minor 0 -->8 Any thoughts? -Alexey ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103370] `DRI_PRIME=1 glxgears -info` halts the system with Intel Graphics [8086:5917] + AMD Graphics [1002:6665] (rev c3)
https://bugs.freedesktop.org/show_bug.cgi?id=103370 Shih-Yuan Lee changed: What|Removed |Added Summary|`DRI_PRIME=1 glxgears |`DRI_PRIME=1 glxgears |-info` halts the system |-info` halts the system |with Intel Graphics |with Intel Graphics |[8086:5917] + AMD Graphics |[8086:5917] + AMD Graphics |[1002:6665].|[1002:6665] (rev c3) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103370] `DRI_PRIME=1 glxgears -info` halts the system with Intel Graphics [8086:5917] + AMD Graphics [1002:6665].
https://bugs.freedesktop.org/show_bug.cgi?id=103370 --- Comment #28 from Shih-Yuan Lee --- `vblank_mode=0 DRI_PRIME=1 glxgears` will also introduce the GPU lock up. However when using radeon.dpm=0, it won't happen but it is tearing all the time. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/8] drm/amdgpu: forward operation context to ttm_bo_mem_space
This way we can finally use some more stats.
Signed-off-by: Christian König
Reviewed-by: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 30 --
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 7d0efc3be25e..17bf0ce1e04f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -466,12 +466,10 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo,
return r;
}
-static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo,
- bool evict, bool interruptible,
- bool no_wait_gpu,
+static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
+ struct ttm_operation_ctx *ctx,
struct ttm_mem_reg *new_mem)
{
- struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = &bo->mem;
struct ttm_mem_reg tmp_mem;
@@ -489,7 +487,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object
*bo,
placements.fpfn = 0;
placements.lpfn = 0;
placements.flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
- r = ttm_bo_mem_space(bo, &placement, &tmp_mem, &ctx);
+ r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx);
if (unlikely(r)) {
return r;
}
@@ -503,22 +501,20 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object
*bo,
if (unlikely(r)) {
goto out_cleanup;
}
- r = amdgpu_move_blit(bo, true, no_wait_gpu, &tmp_mem, old_mem);
+ r = amdgpu_move_blit(bo, true, ctx->no_wait_gpu, &tmp_mem, old_mem);
if (unlikely(r)) {
goto out_cleanup;
}
- r = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, new_mem);
+ r = ttm_bo_move_ttm(bo, ctx->interruptible, ctx->no_wait_gpu, new_mem);
out_cleanup:
ttm_bo_mem_put(bo, &tmp_mem);
return r;
}
-static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
- bool evict, bool interruptible,
- bool no_wait_gpu,
+static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo, bool evict,
+ struct ttm_operation_ctx *ctx,
struct ttm_mem_reg *new_mem)
{
- struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = &bo->mem;
struct ttm_mem_reg tmp_mem;
@@ -536,15 +532,15 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object
*bo,
placements.fpfn = 0;
placements.lpfn = 0;
placements.flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
- r = ttm_bo_mem_space(bo, &placement, &tmp_mem, &ctx);
+ r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx);
if (unlikely(r)) {
return r;
}
- r = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, &tmp_mem);
+ r = ttm_bo_move_ttm(bo, ctx->interruptible, ctx->no_wait_gpu, &tmp_mem);
if (unlikely(r)) {
goto out_cleanup;
}
- r = amdgpu_move_blit(bo, true, no_wait_gpu, new_mem, old_mem);
+ r = amdgpu_move_blit(bo, true, ctx->no_wait_gpu, new_mem, old_mem);
if (unlikely(r)) {
goto out_cleanup;
}
@@ -590,12 +586,10 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
bool evict,
if (old_mem->mem_type == TTM_PL_VRAM &&
new_mem->mem_type == TTM_PL_SYSTEM) {
- r = amdgpu_move_vram_ram(bo, evict, ctx->interruptible,
- ctx->no_wait_gpu, new_mem);
+ r = amdgpu_move_vram_ram(bo, evict, ctx, new_mem);
} else if (old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_VRAM) {
- r = amdgpu_move_ram_vram(bo, evict, ctx->interruptible,
- ctx->no_wait_gpu, new_mem);
+ r = amdgpu_move_ram_vram(bo, evict, ctx, new_mem);
} else {
r = amdgpu_move_blit(bo, evict, ctx->no_wait_gpu,
new_mem, old_mem);
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/8] drm/ttm: add number of bytes moved to the operation context
Add some statistics how many bytes we have moved.
Signed-off-by: Christian König
Reviewed-by: Michel Dänzer
---
drivers/gpu/drm/ttm/ttm_bo.c | 1 +
include/drm/ttm/ttm_bo_api.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d3448c38f00d..97c3da6d5f17 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -361,6 +361,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
else
bo->offset = 0;
+ ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
return 0;
out_err:
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index d0164d131982..368eb02b54a9 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -270,6 +270,7 @@ struct ttm_bo_kmap_obj {
struct ttm_operation_ctx {
bool interruptible;
bool no_wait_gpu;
+ uint64_t bytes_moved;
};
/**
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 8/8] drm/amdgpu: use the new TTM bytes moved counter v2
Instead of the global statistics use the per context bytes moved counter.
v2: rebased
Signed-off-by: Christian König
Reviewed-by: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 9 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 +++---
2 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 41994b87c76e..bea5bc64bf7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -344,7 +344,6 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
struct ttm_operation_ctx ctx = { true, false };
- u64 initial_bytes_moved, bytes_moved;
uint32_t domain;
int r;
@@ -374,15 +373,13 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser
*p,
retry:
amdgpu_ttm_placement_from_domain(bo, domain);
- initial_bytes_moved = atomic64_read(&adev->num_bytes_moved);
r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
- bytes_moved = atomic64_read(&adev->num_bytes_moved) -
- initial_bytes_moved;
- p->bytes_moved += bytes_moved;
+
+ p->bytes_moved += ctx.bytes_moved;
if (adev->mc.visible_vram_size < adev->mc.real_vram_size &&
bo->tbo.mem.mem_type == TTM_PL_VRAM &&
bo->tbo.mem.start < adev->mc.visible_vram_size >> PAGE_SHIFT)
- p->bytes_moved_vis += bytes_moved;
+ p->bytes_moved_vis += ctx.bytes_moved;
if (unlikely(r == -ENOMEM) && domain != bo->allowed_domains) {
domain = bo->allowed_domains;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 15027f751e07..dc0a8be98043 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -331,7 +331,6 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
struct amdgpu_bo *bo;
enum ttm_bo_type type;
unsigned long page_align;
- u64 initial_bytes_moved, bytes_moved;
size_t acc_size;
int r;
@@ -406,22 +405,19 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
bo->tbo.bdev = &adev->mman.bdev;
amdgpu_ttm_placement_from_domain(bo, domain);
- initial_bytes_moved = atomic64_read(&adev->num_bytes_moved);
- /* Kernel allocation are uninterruptible */
r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, type,
&bo->placement, page_align, &ctx, NULL,
acc_size, sg, resv, &amdgpu_ttm_bo_destroy);
if (unlikely(r != 0))
return r;
- bytes_moved = atomic64_read(&adev->num_bytes_moved) -
- initial_bytes_moved;
if (adev->mc.visible_vram_size < adev->mc.real_vram_size &&
bo->tbo.mem.mem_type == TTM_PL_VRAM &&
bo->tbo.mem.start < adev->mc.visible_vram_size >> PAGE_SHIFT)
- amdgpu_cs_report_moved_bytes(adev, bytes_moved, bytes_moved);
+ amdgpu_cs_report_moved_bytes(adev, ctx.bytes_moved,
+ctx.bytes_moved);
else
- amdgpu_cs_report_moved_bytes(adev, bytes_moved, 0);
+ amdgpu_cs_report_moved_bytes(adev, ctx.bytes_moved, 0);
if (kernel)
bo->tbo.priority = 1;
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/8] drm/ttm: use an operation ctx for ttm_bo_init_reserved
Instead of specifying if sleeping should be interruptible.
Signed-off-by: Christian König
Reviewed-by: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
drivers/gpu/drm/ttm/ttm_bo.c | 12 +---
include/drm/ttm/ttm_bo_api.h | 5 ++---
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index c2419bc6b3df..15027f751e07 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -327,6 +327,7 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
uint64_t init_value,
struct amdgpu_bo **bo_ptr)
{
+ struct ttm_operation_ctx ctx = { !kernel, false };
struct amdgpu_bo *bo;
enum ttm_bo_type type;
unsigned long page_align;
@@ -408,7 +409,7 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
initial_bytes_moved = atomic64_read(&adev->num_bytes_moved);
/* Kernel allocation are uninterruptible */
r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, type,
-&bo->placement, page_align, !kernel, NULL,
+&bo->placement, page_align, &ctx, NULL,
acc_size, sg, resv, &amdgpu_ttm_bo_destroy);
if (unlikely(r != 0))
return r;
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 5347c3f3e2f4..1f6957adc19e 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1132,7 +1132,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
enum ttm_bo_type type,
struct ttm_placement *placement,
uint32_t page_alignment,
-bool interruptible,
+struct ttm_operation_ctx *ctx,
struct file *persistent_swap_storage,
size_t acc_size,
struct sg_table *sg,
@@ -1218,11 +1218,8 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
WARN_ON(!locked);
}
- if (likely(!ret)) {
- struct ttm_operation_ctx ctx = { interruptible, false };
-
- ret = ttm_bo_validate(bo, placement, &ctx);
- }
+ if (likely(!ret))
+ ret = ttm_bo_validate(bo, placement, ctx);
if (unlikely(ret)) {
if (!resv)
@@ -1255,10 +1252,11 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
struct reservation_object *resv,
void (*destroy) (struct ttm_buffer_object *))
{
+ struct ttm_operation_ctx ctx = { interruptible, false };
int ret;
ret = ttm_bo_init_reserved(bdev, bo, size, type, placement,
- page_alignment, interruptible,
+ page_alignment, &ctx,
persistent_swap_storage, acc_size,
sg, resv, destroy);
if (ret)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 097951e999bc..d0164d131982 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -455,8 +455,7 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev,
* @type: Requested type of buffer object.
* @flags: Initial placement flags.
* @page_alignment: Data alignment in pages.
- * @interruptible: If needing to sleep to wait for GPU resources,
- * sleep interruptible.
+ * @ctx: TTM operation context for memory allocation.
* @persistent_swap_storage: Usually the swap storage is deleted for buffers
* pinned in physical memory. If this behaviour is not desired, this member
* holds a pointer to a persistent shmem object. Typically, this would
@@ -493,7 +492,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
enum ttm_bo_type type,
struct ttm_placement *placement,
uint32_t page_alignment,
-bool interrubtible,
+struct ttm_operation_ctx *ctx,
struct file *persistent_swap_storage,
size_t acc_size,
struct sg_table *sg,
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/8] drm/ttm: add context to driver move callback as well
Instead of passing the parameters manually.
Signed-off-by: Christian König
Reviewed-by: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 +++--
drivers/gpu/drm/nouveau/nouveau_bo.c| 27 ---
drivers/gpu/drm/qxl/qxl_ttm.c | 9 -
drivers/gpu/drm/radeon/radeon_ttm.c | 23 ---
drivers/gpu/drm/ttm/ttm_bo.c| 3 +--
drivers/gpu/drm/virtio/virtgpu_ttm.c| 7 +++
include/drm/ttm/ttm_bo_driver.h | 6 ++
7 files changed, 49 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 7aea403f5a10..7d0efc3be25e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -553,10 +553,9 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object
*bo,
return r;
}
-static int amdgpu_bo_move(struct ttm_buffer_object *bo,
- bool evict, bool interruptible,
- bool no_wait_gpu,
- struct ttm_mem_reg *new_mem)
+static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
+ struct ttm_operation_ctx *ctx,
+ struct ttm_mem_reg *new_mem)
{
struct amdgpu_device *adev;
struct amdgpu_bo *abo;
@@ -591,19 +590,21 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
if (old_mem->mem_type == TTM_PL_VRAM &&
new_mem->mem_type == TTM_PL_SYSTEM) {
- r = amdgpu_move_vram_ram(bo, evict, interruptible,
- no_wait_gpu, new_mem);
+ r = amdgpu_move_vram_ram(bo, evict, ctx->interruptible,
+ ctx->no_wait_gpu, new_mem);
} else if (old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_VRAM) {
- r = amdgpu_move_ram_vram(bo, evict, interruptible,
- no_wait_gpu, new_mem);
+ r = amdgpu_move_ram_vram(bo, evict, ctx->interruptible,
+ ctx->no_wait_gpu, new_mem);
} else {
- r = amdgpu_move_blit(bo, evict, no_wait_gpu, new_mem, old_mem);
+ r = amdgpu_move_blit(bo, evict, ctx->no_wait_gpu,
+new_mem, old_mem);
}
if (r) {
memcpy:
- r = ttm_bo_move_memcpy(bo, interruptible, no_wait_gpu, new_mem);
+ r = ttm_bo_move_memcpy(bo, ctx->interruptible,
+ ctx->no_wait_gpu, new_mem);
if (r) {
return r;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 04e975fe754c..f86a5b0a4114 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1257,8 +1257,9 @@ nouveau_bo_vm_cleanup(struct ttm_buffer_object *bo,
}
static int
-nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, bool intr,
- bool no_wait_gpu, struct ttm_mem_reg *new_reg)
+nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
+ struct ttm_operation_ctx *ctx,
+ struct ttm_mem_reg *new_reg)
{
struct nouveau_drm *drm = nouveau_bdev(bo->bdev);
struct nouveau_bo *nvbo = nouveau_bo(bo);
@@ -1266,7 +1267,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
bool intr,
struct nouveau_drm_tile *new_tile = NULL;
int ret = 0;
- ret = ttm_bo_wait(bo, intr, no_wait_gpu);
+ ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
if (ret)
return ret;
@@ -1290,22 +1291,26 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool
evict, bool intr,
/* Hardware assisted copy. */
if (drm->ttm.move) {
if (new_reg->mem_type == TTM_PL_SYSTEM)
- ret = nouveau_bo_move_flipd(bo, evict, intr,
- no_wait_gpu, new_reg);
+ ret = nouveau_bo_move_flipd(bo, evict,
+ ctx->interruptible,
+ ctx->no_wait_gpu, new_reg);
else if (old_reg->mem_type == TTM_PL_SYSTEM)
- ret = nouveau_bo_move_flips(bo, evict, intr,
- no_wait_gpu, new_reg);
+ ret = nouveau_bo_move_flips(bo, evict,
+ ctx->interruptible,
+ ctx->no_wait_gpu, new_reg);
else
- ret = nouveau_bo_move_m2mf(bo, evict, intr,
- no_wait_gpu, new_reg);
+ ret = nouveau_bo_mo
[PATCH 3/8] drm/ttm: use an operation context for ttm_bo_mem_space v2
Instead of specifying interruptible and no_wait_gpu manually.
v2: rebase
Signed-off-by: Christian König
Reviewed-by: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 11 ++-
drivers/gpu/drm/nouveau/nouveau_bo.c | 6 --
drivers/gpu/drm/radeon/radeon_ttm.c| 8
drivers/gpu/drm/ttm/ttm_bo.c | 22 +++---
include/drm/ttm/ttm_bo_driver.h| 3 +--
6 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 65fba5fb537e..942156f5d050 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -682,6 +682,7 @@ void amdgpu_fw_reserve_vram_fini(struct amdgpu_device *adev)
*/
int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev)
{
+ struct ttm_operation_ctx ctx = { false, false };
int r = 0;
int i;
u64 vram_size = adev->mc.visible_vram_size;
@@ -718,8 +719,8 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev)
}
ttm_bo_mem_put(&bo->tbo, &bo->tbo.mem);
- r = ttm_bo_mem_space(&bo->tbo, &bo->placement, &bo->tbo.mem,
-false, false);
+ r = ttm_bo_mem_space(&bo->tbo, &bo->placement,
+&bo->tbo.mem, &ctx);
if (r)
goto error_pin;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3e6817864af3..7aea403f5a10 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -471,6 +471,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object
*bo,
bool no_wait_gpu,
struct ttm_mem_reg *new_mem)
{
+ struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = &bo->mem;
struct ttm_mem_reg tmp_mem;
@@ -488,8 +489,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object
*bo,
placements.fpfn = 0;
placements.lpfn = 0;
placements.flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
- r = ttm_bo_mem_space(bo, &placement, &tmp_mem,
-interruptible, no_wait_gpu);
+ r = ttm_bo_mem_space(bo, &placement, &tmp_mem, &ctx);
if (unlikely(r)) {
return r;
}
@@ -518,6 +518,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object
*bo,
bool no_wait_gpu,
struct ttm_mem_reg *new_mem)
{
+ struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = &bo->mem;
struct ttm_mem_reg tmp_mem;
@@ -535,8 +536,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object
*bo,
placements.fpfn = 0;
placements.lpfn = 0;
placements.flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
- r = ttm_bo_mem_space(bo, &placement, &tmp_mem,
-interruptible, no_wait_gpu);
+ r = ttm_bo_mem_space(bo, &placement, &tmp_mem, &ctx);
if (unlikely(r)) {
return r;
}
@@ -878,6 +878,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
+ struct ttm_operation_ctx ctx = { false, false };
struct amdgpu_ttm_tt *gtt = (void*)bo->ttm;
struct ttm_mem_reg tmp;
struct ttm_placement placement;
@@ -900,7 +901,7 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
placements.flags = (bo->mem.placement & ~TTM_PL_MASK_MEM) |
TTM_PL_FLAG_TT;
- r = ttm_bo_mem_space(bo, &placement, &tmp, false, false);
+ r = ttm_bo_mem_space(bo, &placement, &tmp, &ctx);
if (unlikely(r))
return r;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 16caca2647b9..04e975fe754c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1129,6 +1129,7 @@ static int
nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr,
bool no_wait_gpu, struct ttm_mem_reg *new_reg)
{
+ struct ttm_operation_ctx ctx = { intr, no_wait_gpu };
struct ttm_place placement_memtype = {
.fpfn = 0,
.lpfn = 0,
@@ -1143,7 +1144,7 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool
evict, bool intr,
tmp_reg = *new_reg;
tmp_reg.mm_node = NULL;
- ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, intr, no_wait_gpu);
+ ret = ttm_bo_mem
[PATCH 1/8] drm/ttm: add operation ctx to ttm_bo_validate v2
Give moving a BO into place an operation context to work with.
v2: rebased
Signed-off-by: Christian König
Reviewed-by: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 14 --
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 6 --
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 3 ++-
drivers/gpu/drm/ast/ast_ttm.c | 9 ++---
drivers/gpu/drm/bochs/bochs_mm.c| 6 --
drivers/gpu/drm/cirrus/cirrus_ttm.c | 6 --
drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 6 --
drivers/gpu/drm/mgag200/mgag200_ttm.c | 9 ++---
drivers/gpu/drm/nouveau/nouveau_bo.c| 4 ++--
drivers/gpu/drm/qxl/qxl_ioctl.c | 4 ++--
drivers/gpu/drm/qxl/qxl_object.c| 6 --
drivers/gpu/drm/qxl/qxl_release.c | 4 ++--
drivers/gpu/drm/radeon/radeon_gem.c | 3 ++-
drivers/gpu/drm/radeon/radeon_mn.c | 3 ++-
drivers/gpu/drm/radeon/radeon_object.c | 14 +-
drivers/gpu/drm/radeon/radeon_vm.c | 3 ++-
drivers/gpu/drm/ttm/ttm_bo.c| 16 +---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 11 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c | 21 +
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 9 -
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c| 6 --
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 3 ++-
include/drm/ttm/ttm_bo_api.h| 20
26 files changed, 129 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index ee7736419411..41994b87c76e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -343,6 +343,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
struct amdgpu_bo *bo)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+ struct ttm_operation_ctx ctx = { true, false };
u64 initial_bytes_moved, bytes_moved;
uint32_t domain;
int r;
@@ -374,7 +375,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
retry:
amdgpu_ttm_placement_from_domain(bo, domain);
initial_bytes_moved = atomic64_read(&adev->num_bytes_moved);
- r = ttm_bo_validate(&bo->tbo, &bo->placement, true, false);
+ r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
bytes_moved = atomic64_read(&adev->num_bytes_moved) -
initial_bytes_moved;
p->bytes_moved += bytes_moved;
@@ -396,6 +397,7 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
struct amdgpu_bo *validated)
{
uint32_t domain = validated->allowed_domains;
+ struct ttm_operation_ctx ctx = { true, false };
int r;
if (!p->evictable)
@@ -433,7 +435,7 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
bo->tbo.mem.mem_type == TTM_PL_VRAM &&
bo->tbo.mem.start < adev->mc.visible_vram_size >>
PAGE_SHIFT;
initial_bytes_moved = atomic64_read(&adev->num_bytes_moved);
- r = ttm_bo_validate(&bo->tbo, &bo->placement, true, false);
+ r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
bytes_moved = atomic64_read(&adev->num_bytes_moved) -
initial_bytes_moved;
p->bytes_moved += bytes_moved;
@@ -472,6 +474,7 @@ static int amdgpu_cs_validate(void *param, struct amdgpu_bo
*bo)
static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
struct list_head *validated)
{
+ struct ttm_operation_ctx ctx = { true, false };
struct amdgpu_bo_list_entry *lobj;
int r;
@@ -489,8 +492,7 @@ static int amdgpu_cs_list_validate(struct amdgpu_cs_parser
*p,
lobj->user_pages) {
amdgpu_ttm_placement_from_domain(bo,
AMDGPU_GEM_DOMAIN_CPU);
- r = ttm_bo_validate(&bo->tbo, &bo->placement, true,
- false);
+ r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
if (r)
return r;
amdgpu_ttm_tt_set_user_pages(bo->tbo.ttm,
@@ -1573,6 +1575,7 @@ int amdgpu_cs_find_mapping(struct amdgpu_cs_parser
*parser,
struct amdgpu_bo_va_mapping **map)
{
struct amdgpu_fpriv *fpriv = parser->filp->driver_priv;
+ struct ttm_operation_ctx ctx = { false, false };
struct amdgpu_vm *vm = &fpriv->vm;
struct amdgpu_bo_va_mapping *mapping;
i
[PATCH 4/8] drm/ttm: use the operation context inside TTM
Instead of passing down the parameters manually to every function.
Signed-off-by: Christian König
Reviewed-by: Michel Dänzer
---
drivers/gpu/drm/ttm/ttm_bo.c | 67 +++-
1 file changed, 29 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 63c1a97b3589..4ed30ffa411f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -269,9 +269,8 @@ static int ttm_bo_add_ttm(struct ttm_buffer_object *bo,
bool zero_alloc)
}
static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
- struct ttm_mem_reg *mem,
- bool evict, bool interruptible,
- bool no_wait_gpu)
+ struct ttm_mem_reg *mem, bool evict,
+ struct ttm_operation_ctx *ctx)
{
struct ttm_bo_device *bdev = bo->bdev;
bool old_is_pci = ttm_mem_reg_is_pci(bdev, &bo->mem);
@@ -325,12 +324,14 @@ static int ttm_bo_handle_move_mem(struct
ttm_buffer_object *bo,
if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
- ret = ttm_bo_move_ttm(bo, interruptible, no_wait_gpu, mem);
+ ret = ttm_bo_move_ttm(bo, ctx->interruptible,
+ ctx->no_wait_gpu, mem);
else if (bdev->driver->move)
- ret = bdev->driver->move(bo, evict, interruptible,
-no_wait_gpu, mem);
+ ret = bdev->driver->move(bo, evict, ctx->interruptible,
+ctx->no_wait_gpu, mem);
else
- ret = ttm_bo_move_memcpy(bo, interruptible, no_wait_gpu, mem);
+ ret = ttm_bo_move_memcpy(bo, ctx->interruptible,
+ctx->no_wait_gpu, mem);
if (ret) {
if (bdev->driver->move_notify) {
@@ -653,10 +654,9 @@ void ttm_bo_unlock_delayed_workqueue(struct ttm_bo_device
*bdev, int resched)
}
EXPORT_SYMBOL(ttm_bo_unlock_delayed_workqueue);
-static int ttm_bo_evict(struct ttm_buffer_object *bo, bool interruptible,
- bool no_wait_gpu)
+static int ttm_bo_evict(struct ttm_buffer_object *bo,
+ struct ttm_operation_ctx *ctx)
{
- struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu };
struct ttm_bo_device *bdev = bo->bdev;
struct ttm_mem_reg evict_mem;
struct ttm_placement placement;
@@ -672,7 +672,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, bool
interruptible,
placement.num_placement = 0;
placement.num_busy_placement = 0;
bdev->driver->evict_flags(bo, &placement);
- ret = ttm_bo_mem_space(bo, &placement, &evict_mem, &ctx);
+ ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx);
if (ret) {
if (ret != -ERESTARTSYS) {
pr_err("Failed to find memory space for buffer 0x%p
eviction\n",
@@ -682,8 +682,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, bool
interruptible,
goto out;
}
- ret = ttm_bo_handle_move_mem(bo, &evict_mem, true,
-interruptible, no_wait_gpu);
+ ret = ttm_bo_handle_move_mem(bo, &evict_mem, true, ctx);
if (unlikely(ret)) {
if (ret != -ERESTARTSYS)
pr_err("Buffer eviction failed\n");
@@ -713,8 +712,7 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
struct reservation_object *resv,
uint32_t mem_type,
const struct ttm_place *place,
- bool interruptible,
- bool no_wait_gpu)
+ struct ttm_operation_ctx *ctx)
{
struct ttm_bo_global *glob = bdev->glob;
struct ttm_mem_type_manager *man = &bdev->man[mem_type];
@@ -759,8 +757,8 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
kref_get(&bo->list_kref);
if (!list_empty(&bo->ddestroy)) {
- ret = ttm_bo_cleanup_refs(bo, interruptible, no_wait_gpu,
- locked);
+ ret = ttm_bo_cleanup_refs(bo, ctx->interruptible,
+ ctx->no_wait_gpu, locked);
kref_put(&bo->list_kref, ttm_bo_release_list);
return ret;
}
@@ -768,7 +766,7 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
ttm_bo_del_from_lru(bo);
spin_unlock(&glob->lru_lock);
- ret = ttm_bo_evict(bo, interruptible, no_wait_gpu);
+ ret = ttm_bo_evict(bo, ctx);
if (locked) {
ttm_bo_unreserve(bo);
} else {
@@ -826,8 +824,7 @@ static int ttm_bo_mem_force_
Operation context for TTM
Hi everyone, Michel already reviewed this back in April, but I didn't found time to actually fully test it before now. So sending this one out once more because it's an interface change which affects all driver using TTM. Please review and/or comment. Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103370] `DRI_PRIME=1 glxgears -info` halts the system with Intel Graphics [8086:5917] + AMD Graphics [1002:6665].
https://bugs.freedesktop.org/show_bug.cgi?id=103370 --- Comment #27 from Michel Dänzer --- Thanks for bisecting, but I don't think that commit can be directly responsible for a GPU hang. Before that commit, the DRI3 code in Mesa would only use one back buffer for glxgears, which means that the GPU could only start rendering a new frame after the previous one had finished presenting. Maybe that somehow prevented the hang. A possible test for this theory is running vblank_mode=0 DRI_PRIME=1 glxgears with Mesa 12.0.3; does that also trigger the hang? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103370] `DRI_PRIME=1 glxgears -info` halts the system with Intel Graphics [8086:5917] + AMD Graphics [1002:6665].
https://bugs.freedesktop.org/show_bug.cgi?id=103370 Timo Aaltonen changed: What|Removed |Added CC||tjaal...@ubuntu.com --- Comment #26 from Timo Aaltonen --- this was tested to regress between mesa 12.0.3 and 12.0.5, and bisect points out commit d3d33918c79d9e87aedaf6f70ed39f75eed262a0 Author: Michel Dänzer Date: Wed Aug 17 17:02:04 2016 +0900 loader/dri3: Overhaul dri3_update_num_back as the first bad commit -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[REBASE 5/5] drm: Add and handle new aspect ratios in DRM layer
HDMI 2.0/CEA-861-F introduces two new aspect ratios:
- 64:27
- 256:135
This patch:
- Adds new DRM flags for to represent these new aspect ratios.
- Adds new cases to handle these aspect ratios while converting
from user->kernel mode or vise versa.
This patch was once reviewed and merged, and later reverted due
to lack of DRM client protection, while adding aspect ratio bits
in user modes. This is a re-spin of the series, with DRM client
cap protection.
The previous series can be found here:
https://pw-emeril.freedesktop.org/series/10850/
Signed-off-by: Shashank Sharma
Reviewed-by: Sean Paul (V2)
Reviewed-by: Jose Abreu (V2)
Cc: Ville Syrjala
Cc: Sean Paul
Cc: Jose Abreu
Cc: Ankit Nautiyal
---
drivers/gpu/drm/drm_modes.c | 12
include/uapi/drm/drm_mode.h | 6 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 2e8a11e..b9ec35b 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1559,6 +1559,12 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo
*out,
case HDMI_PICTURE_ASPECT_16_9:
out->flags |= DRM_MODE_FLAG_PIC_AR_16_9;
break;
+ case HDMI_PICTURE_ASPECT_64_27:
+ out->flags |= DRM_MODE_FLAG_PIC_AR_64_27;
+ break;
+ case DRM_MODE_PICTURE_ASPECT_256_135:
+ out->flags |= DRM_MODE_FLAG_PIC_AR_256_135;
+ break;
case HDMI_PICTURE_ASPECT_RESERVED:
default:
out->flags |= DRM_MODE_FLAG_PIC_AR_NONE;
@@ -1620,6 +1626,12 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
case DRM_MODE_FLAG_PIC_AR_16_9:
out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9;
break;
+ case DRM_MODE_FLAG_PIC_AR_64_27:
+ out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_64_27;
+ break;
+ case DRM_MODE_FLAG_PIC_AR_256_135:
+ out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_256_135;
+ break;
default:
out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
break;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 5597a87..64177d7 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -89,6 +89,8 @@ extern "C" {
#define DRM_MODE_PICTURE_ASPECT_NONE 0
#define DRM_MODE_PICTURE_ASPECT_4_31
#define DRM_MODE_PICTURE_ASPECT_16_9 2
+#define DRM_MODE_PICTURE_ASPECT_64_27 3
+#define DRM_MODE_PICTURE_ASPECT_256_1354
/* Aspect ratio flag bitmask (4 bits 22:19) */
#define DRM_MODE_FLAG_PIC_AR_MASK (0x0F<<19)
@@ -98,6 +100,10 @@ extern "C" {
(DRM_MODE_PICTURE_ASPECT_4_3<<19)
#define DRM_MODE_FLAG_PIC_AR_16_9 \
(DRM_MODE_PICTURE_ASPECT_16_9<<19)
+#define DRM_MODE_FLAG_PIC_AR_64_27 \
+ (DRM_MODE_PICTURE_ASPECT_64_27<<19)
+#define DRM_MODE_FLAG_PIC_AR_256_135 \
+ (DRM_MODE_PICTURE_ASPECT_256_135<<19)
/* DPMS flags */
/* bit compatible with the xorg definitions. */
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[REBASE 0/5] Aspect ratio support in DRM layer
This patch series is a re-attempt to enable aspect ratio support in DRM layer. Currently the aspect ratio information gets lost in translation during a user->kernel mode or vice versa. The old patch series (https://pw-emeril.freedesktop.org/series/10850/) had 4 patches, out of which 2 patches were reverted due to lack of drm client protection while loading the aspect information. This patch series, adds a DRM client option for aspect ratio, and loads aspect ratio flags, only when the client sets this cap. Also, to test this patch series there is a userspace implementation by Ankit Nautiyal in Wayland/weston layer: https://patchwork.freedesktop.org/patch/188125/ This, helps us in passing HDMI compliance test cases like 7-27, where the test equipment applies a CEA mode, and expects the exact VIC in the AVI infoframes. Ankit Nautiyal (1): drm: Handle aspect ratio in modeset paths Shashank Sharma (2): drm: Add aspect ratio parsing in DRM layer drm: Add and handle new aspect ratios in DRM layer aknautiy (2): drm: Add DRM client cap for aspect-ratio drm: Expose modes with aspect ratio, only if requested drivers/gpu/drm/drm_atomic.c| 40 +++--- drivers/gpu/drm/drm_connector.c | 7 +++ drivers/gpu/drm/drm_crtc.c | 19 ++ drivers/gpu/drm/drm_ioctl.c | 5 + drivers/gpu/drm/drm_modes.c | 43 + include/drm/drm_atomic.h| 2 ++ include/drm/drm_file.h | 7 +++ include/uapi/drm/drm.h | 7 +++ include/uapi/drm/drm_mode.h | 6 ++ 9 files changed, 129 insertions(+), 7 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[REBASE 3/5] drm: Expose modes with aspect ratio, only if requested
From: aknautiy We parse the EDID and add all the modes in the connector's modelist. This adds CEA modes with aspect ratio information too, regadless of if user space requested this information or not. This patch prunes the modes with aspect-ratio information, from a connector's modelist, if the user-space has not set the aspect ratio DRM client cap. Cc: Ville Syrjala Cc: Shashank Sharma Cc: Jose Abreu Signed-off-by: aknautiy --- drivers/gpu/drm/drm_connector.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 704fc89..a246bb5 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1285,6 +1285,13 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, */ if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode)) return false; + /* +* If user-space hasn't configured the driver to expose the modes +* with aspect-ratio, don't expose them. +*/ + if (!file_priv->aspect_ratio_allowed && + mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) + return false; return true; } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[REBASE 4/5] drm: Add aspect ratio parsing in DRM layer
Current DRM layer functions don't parse aspect ratio information
while converting a user mode->kernel mode or vice versa. This
causes modeset to pick mode with wrong aspect ratio, eventually
causing failures in HDMI compliance test cases, due to wrong VIC.
This patch adds aspect ratio information in DRM's mode conversion
and mode comparision functions, to make sure kernel picks mode
with right aspect ratio (as per the VIC).
Background:
This patch was once reviewed and merged, and later reverted due to
lack of DRM cap protection. This is a re-spin of this patch, this
time with DRM cap protection, to avoid aspect ratio information, when
the client doesn't request for it.
Review link: https://pw-emeril.freedesktop.org/patch/104068/
Background discussion: https://patchwork.kernel.org/patch/9379057/
Signed-off-by: Shashank Sharma
Signed-off-by: Lin, Jia
Signed-off-by: Akashdeep Sharma
Reviewed-by: Jim Bride (V2)
Reviewed-by: Jose Abreu (V4)
Cc: Ville Syrjala
Cc: Jim Bride
Cc: Jose Abreu
Cc: Ankit Nautiyal
---
drivers/gpu/drm/drm_modes.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 4a3f68a..2e8a11e 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1015,6 +1015,7 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct
drm_display_mode *mode1,
mode1->vsync_end == mode2->vsync_end &&
mode1->vtotal == mode2->vtotal &&
mode1->vscan == mode2->vscan &&
+ mode1->picture_aspect_ratio == mode2->picture_aspect_ratio &&
(mode1->flags & ~DRM_MODE_FLAG_3D_MASK) ==
(mode2->flags & ~DRM_MODE_FLAG_3D_MASK))
return true;
@@ -1549,6 +1550,21 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo
*out,
out->vrefresh = in->vrefresh;
out->flags = in->flags;
out->type = in->type;
+ out->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
+
+ switch (in->picture_aspect_ratio) {
+ case HDMI_PICTURE_ASPECT_4_3:
+ out->flags |= DRM_MODE_FLAG_PIC_AR_4_3;
+ break;
+ case HDMI_PICTURE_ASPECT_16_9:
+ out->flags |= DRM_MODE_FLAG_PIC_AR_16_9;
+ break;
+ case HDMI_PICTURE_ASPECT_RESERVED:
+ default:
+ out->flags |= DRM_MODE_FLAG_PIC_AR_NONE;
+ break;
+ }
+
strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
}
@@ -1594,6 +1610,21 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
+ /* Clearing picture aspect ratio bits from out flags */
+ out->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
+
+ switch (in->flags & DRM_MODE_FLAG_PIC_AR_MASK) {
+ case DRM_MODE_FLAG_PIC_AR_4_3:
+ out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3;
+ break;
+ case DRM_MODE_FLAG_PIC_AR_16_9:
+ out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9;
+ break;
+ default:
+ out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+ break;
+ }
+
out->status = drm_mode_validate_basic(out);
if (out->status != MODE_OK)
goto out;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[REBASE 2/5] drm: Handle aspect ratio in modeset paths
From: Ankit Nautiyal
If the user mode does not support aspect-ratio, and requests for
a modeset with aspect ratio flags, then the flag bits reprsenting
aspect ratio must be ignored.
Similarly, if user space doesn't set the aspect ratio client cap,
while preparing a usermode, the aspect-ratio info must not be given
into it.
This patch:
1. adds a new bit field aspect_ratio_allowed in drm_atomic_state,
which is set only if the aspect-ratio is supported by the user.
2. discards the aspect-ratio info from the user mode while
preparing kernel mode structure, during modeset, if the
user does not support aspect ratio.
3. avoids setting the aspect-ratio info in user-mode, while
converting user-mode from the kernel-mode.
Cc: Ville Syrjala
Cc: Shashank Sharma
Cc: Jose Abreu
Signed-off-by: Ankit Nautiyal
---
drivers/gpu/drm/drm_atomic.c | 40 +---
drivers/gpu/drm/drm_crtc.c | 19 +++
include/drm/drm_atomic.h | 2 ++
3 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 37445d5..b9743af 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -338,18 +338,30 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state
*state,
state->mode_blob = NULL;
if (mode) {
- drm_mode_convert_to_umode(&umode, mode);
+ /*
+* If the user has not asked for aspect ratio support,
+* take a copy of the 'mode' and set the aspect ratio as
+* None. This copy is used to prepare the user-mode with no
+* aspect-ratio info.
+*/
+ struct drm_display_mode ar_mode;
+ struct drm_atomic_state *atomic_state = state->state;
+
+ drm_mode_copy(&ar_mode, mode);
+ if (atomic_state && !atomic_state->allow_aspect_ratio)
+ ar_mode.picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+ drm_mode_convert_to_umode(&umode, &ar_mode);
state->mode_blob =
drm_property_create_blob(state->crtc->dev,
-sizeof(umode),
-&umode);
+sizeof(umode),
+&umode);
if (IS_ERR(state->mode_blob))
return PTR_ERR(state->mode_blob);
- drm_mode_copy(&state->mode, mode);
+ drm_mode_copy(&state->mode, &ar_mode);
state->enable = true;
DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
-mode->name, state);
+ar_mode.name, state);
} else {
memset(&state->mode, 0, sizeof(state->mode));
state->enable = false;
@@ -386,10 +398,23 @@ int drm_atomic_set_mode_prop_for_crtc(struct
drm_crtc_state *state,
memset(&state->mode, 0, sizeof(state->mode));
if (blob) {
+ struct drm_mode_modeinfo *ar_umode;
+ struct drm_atomic_state *atomic_state;
+
+ /*
+* If the user-space does not ask for aspect-ratio
+* the aspect ratio bits in the drmModeModeinfo from user,
+* does not mean anything. Therefore these bits must be
+* discarded.
+*/
+ ar_umode = (struct drm_mode_modeinfo *) blob->data;
+ atomic_state = state->state;
+ if (atomic_state && !atomic_state->allow_aspect_ratio)
+ ar_umode->flags &= (~DRM_MODE_FLAG_PIC_AR_MASK);
if (blob->length != sizeof(struct drm_mode_modeinfo) ||
drm_mode_convert_umode(&state->mode,
- (const struct drm_mode_modeinfo *)
- blob->data))
+ (const struct drm_mode_modeinfo *)
+ ar_umode))
return -EINVAL;
state->mode_blob = drm_property_blob_get(blob);
@@ -2229,6 +2254,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
state->acquire_ctx = &ctx;
state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
+ state->allow_aspect_ratio = file_priv->aspect_ratio_allowed;
retry:
plane_mask = 0;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f0556e6..a2d34fa 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -425,6 +425,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
drm_modeset_lock(&crtc->mutex, NULL);
if (crtc->state) {
if (crtc->state->enable) {
+ /*
+
[REBASE 1/5] drm: Add DRM client cap for aspect-ratio
From: aknautiy
A new drm client cap is required to enable user-space to advertise
if it supports modes with aspect-ratio. Based on this cap value, the
kernel will take a call on exposing the aspect ratio information in
modes or not.
This patch adds the client cap for aspect-ratio.
Cc: Ville Syrjala
Cc: Shashank Sharma
Signed-off-by: aknautiy
---
drivers/gpu/drm/drm_ioctl.c | 5 +
include/drm/drm_file.h | 7 +++
include/uapi/drm/drm.h | 7 +++
3 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 4aafe48..e092550 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -325,6 +325,11 @@ drm_setclientcap(struct drm_device *dev, void *data,
struct drm_file *file_priv)
file_priv->atomic = req->value;
file_priv->universal_planes = req->value;
break;
+ case DRM_CLIENT_CAP_ASPECT_RATIO:
+ if (req->value > 1)
+ return -EINVAL;
+ file_priv->aspect_ratio_allowed = req->value;
+ break;
default:
return -EINVAL;
}
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 0e0c868..6e0e435 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -182,6 +182,13 @@ struct drm_file {
unsigned atomic:1;
/**
+* @aspect_ratio_allowed:
+*
+* True if client has asked to expose the aspect-ratio flags with mode.
+*/
+ unsigned aspect_ratio_allowed:1;
+
+ /**
* @is_master:
*
* This client is the creator of @master. Protected by struct
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 6fdff59..fe5f531 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -680,6 +680,13 @@ struct drm_get_cap {
*/
#define DRM_CLIENT_CAP_ATOMIC 3
+/**
+ * DRM_CLIENT_CAP_ASPECT_RATIO
+ *
+ * If set to 1, the DRM core will expose aspect ratio flags to userspace.
+ */
+#define DRM_CLIENT_CAP_ASPECT_RATIO4
+
/** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
struct drm_set_client_cap {
__u64 capability;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103788] Screen goes blank after screen sleep and wioll not come back on
https://bugs.freedesktop.org/show_bug.cgi?id=103788 --- Comment #1 from Michel Dänzer --- Please attach the corresponding dmesg output and Xorg log file. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
> Hi Andrew,
>
>> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
>>
>> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>>>
Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
On 16/11/17 10:50, H. Nikolaus Schaller wrote:
> The vendor name was "toppoly" but other panels and the vendor list
> have defined it as "tpo". So let's fix it in driver and bindings.
>
> Signed-off-by: H. Nikolaus Schaller
> ---
> -MODULE_ALIAS("spi:toppoly,td028ttec1");
> +MODULE_ALIAS("spi:tpo,td028ttec1");
Doesn't this mean that the module won't load if you have old bindings?
>>>
>>> Hm.
>>>
>>> Well, I think it can load but doesn't automatically from DT strings which
>>> might
>>> be unexpected.
>>>
Can't we have two module aliases?
>>>
>>> I think we can. Just a random example:
>>> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>>>
>>> So we should keep both.
>>
>> Even better would be to drop both MODULE_ALIAS and let the
>> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
>
> Why would that be better?
>
MODULE_ALIAS is ugly, you already have a table (usually) of device names
that are supported by the driver, the module aliases should be generated
from that table. This also keeps supported device list in one place.
> As far as I see it will need more code and changes than adding one line of
> MODULE_ALIAS.
>
>> Although it doesn't look like this driver has an SPI id table, you
>> should probably add one, I be interested to see if this module is always
>> being matched through the "spi" or the "of" alias..
>
> Could you please propose how that code should look like, so that I can test?
>
Sure,
start with
$ udevadm monitor
and see what string the kernel is looking for when trying to find a
module for this device.
If it is only ever looking for "of:toppoly,td028ttec1", then you can
drop the MODULE_ALIAS and be done as it was never getting used anyway.
What I expect though is "spi:toppoly,td028ttec1", in which case you
should add
static const struct spi_device_id td028ttec1_ids[] = {
{ "toppoly,td028ttec1", 0 },
{ "tpo,td028ttec1", 0},
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
link to it in the td028ttec1_spi_driver struct:
.id_table = td028ttec1_ids,
Then test again to see that the module still loads with the new and old
DT string.
Andrew
> BR and thanks,
> Nikolaus Schaller
>
>>
>>>
>>> Should I submit a new version?
>>>
>>> BR,
>>> Nikolaus
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] treewide: Fix line continuation formats
On Thu, 2017-11-16 at 07:27 -0800, Joe Perches wrote: > Avoid using line continations in formats as that causes unexpected > output. Is having lines greater than 80 characters the preferred method? Could you add quotes before the backlash and before the first word on the next line instead? Mimi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [RFT] drm: adv7511/33: fix adv7511_cec_init() failure handling
On 15 November 2017 at 18:28, Hans Verkuil wrote:
> On 15/11/17 13:37, Arnd Bergmann wrote:
>> An otherwise correct cleanup patch from Dan Carpenter turned a broken
>> failure handling from a feature patch by Hans Verkuil into a kernel
>> Oops, so bisection points to commit 7af35b0addbc ("drm/kirin: Checking
>> for IS_ERR() instead of NULL") rather than 3b1b975003e4 ("drm:
>> adv7511/33: add HDMI CEC support").
>>
>> I've managed to piece together several partial problems, though
>> I'm still struggling with the bigger picture:
>>
>> adv7511_probe() registers a drm_bridge structure that was allocated
>> with devm_kzalloc(). It calls adv7511_cec_init(), which fails for an
>> unknown reason, which in turn triggers the registered structure to be
>> removed.
>>
>> Elsewhere, kirin_drm_platform_probe() gets called, which calls
>> of_graph_get_remote_node(), and that returns NULL. Before Dan's
>> patch we would go on with a NULL pointer here and register that,
>> now kirin_drm_platform_probe() fails with -ENODEV.
>>
>> In a third driver, dsi_parse_dt() calls drm_of_find_panel_or_bridge(),
>> which after not finding a panel goes on to call of_drm_find_bridge(),
>> and that crashes due to the earlier list corruption.
>>
>> This addresses the first issue by making sure that adv7511_probe()
>> does not leave behind any corrupted list entries. This should
>> get the system back to boot but needs testing. From my understanding,
>> there is at least one more bug that needs to be resolved to actually
>> get everything to work again.
>>
>> Reported-by: Naresh Kamboju
>> Cc: Xinliang Liu
>> Cc: Dan Carpenter
>> Cc: Sean Paul
>> Cc: Hans Verkuil
>> Cc: Archit Taneja
>> Link: https://bugs.linaro.org/show_bug.cgi?id=3345
>> Link: https://lkft.validation.linaro.org/scheduler/job/48017#L3551
>> Fixes: 7af35b0addbc ("drm/kirin: Checking for IS_ERR() instead of NULL")
>> Fixes: 3b1b975003e4 ("drm: adv7511/33: add HDMI CEC support")
>> Signed-off-by: Arnd Bergmann
>> ---
>> Untested so far, this is what I came up with after reading the
>> WARN_ON log from a modified kernel.
>>
>> Naresh, can you give this one a go?
Tested with this patch.
I did not notice kernel crash/WARNING in dmesg log on HiKey (arm64) board.
Ref test log:
Link: https://pastebin.com/t8iLEFwF
Tested-by: Naresh Kamboju
>>
>> Hans and others, can you review in the meantime?
>>
>> Signed-off-by: Arnd Bergmann
>> ---
>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 7 ++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> index 0e14f1572d05..93d1ecafe8fa 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -1204,7 +1204,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
>> struct i2c_device_id *id)
>> #ifdef CONFIG_DRM_I2C_ADV7511_CEC
>> ret = adv7511_cec_init(dev, adv7511, offset);
>> if (ret)
>> - goto err_unregister_cec;
>> + goto err_unregister_bridge;
>
> Rather than adding the err_unregister_bridge label, I think it is better to
> move
> this code up to just before the call to drm_bridge_add().
>
> I think I just didn't realize that doing it after would require additional
> cleanup.
> But it should be perfectly fine to move it up so we can avoid doing that.
>
> I can't test it until Monday as I don't have access to the hardware at the
> moment.
>
> Regards,
>
> Hans
>
>> #else
>> regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
>>ADV7511_CEC_CTRL_POWER_DOWN);
>> @@ -1212,6 +1212,11 @@ static int adv7511_probe(struct i2c_client *i2c,
>> const struct i2c_device_id *id)
>>
>> return 0;
>>
>> +#ifdef CONFIG_DRM_I2C_ADV7511_CEC
>> +err_unregister_bridge:
>> + adv7511_audio_exit(adv7511);
>> + drm_bridge_remove(&adv7511->bridge);
>> +#endif
>> err_unregister_cec:
>> i2c_unregister_device(adv7511->i2c_cec);
>> if (adv7511->cec_clk)
>>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
On 11/16/2017 12:18 PM, H. Nikolaus Schaller wrote:
>
>> Am 16.11.2017 um 18:08 schrieb Andrew F. Davis :
>>
>> On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
>>> Hi Andrew,
>>>
Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>
>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>>
>> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
>>> The vendor name was "toppoly" but other panels and the vendor list
>>> have defined it as "tpo". So let's fix it in driver and bindings.
>>>
>>> Signed-off-by: H. Nikolaus Schaller
>>> ---
>>
>>
>>> -MODULE_ALIAS("spi:toppoly,td028ttec1");
>>> +MODULE_ALIAS("spi:tpo,td028ttec1");
>>
>> Doesn't this mean that the module won't load if you have old bindings?
>
> Hm.
>
> Well, I think it can load but doesn't automatically from DT strings which
> might
> be unexpected.
>
>> Can't we have two module aliases?
>
> I think we can. Just a random example:
> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>
> So we should keep both.
Even better would be to drop both MODULE_ALIAS and let the
MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
>>>
>>> Why would that be better?
>>>
>>
>> MODULE_ALIAS is ugly, you already have a table (usually) of device names
>> that are supported by the driver, the module aliases should be generated
>> from that table. This also keeps supported device list in one place.
>>
>>> As far as I see it will need more code and changes than adding one line of
>>> MODULE_ALIAS.
>>>
Although it doesn't look like this driver has an SPI id table, you
should probably add one, I be interested to see if this module is always
being matched through the "spi" or the "of" alias..
>>>
>>> Could you please propose how that code should look like, so that I can test?
>>>
>>
>> Sure,
>>
>> start with
>> $ udevadm monitor
>> and see what string the kernel is looking for when trying to find a
>> module for this device.
>
> Well, the module is loaded automatically from DT at boot time well before
> I can start udevadm. So that is the most tricky part to setup the system
> to suppress this...
>
>>
>> If it is only ever looking for "of:toppoly,td028ttec1", then you can
>> drop the MODULE_ALIAS and be done as it was never getting used anyway.
>
> Since it is an SPI client, I am sure it looks for "spi:something.
>
>>
>> What I expect though is "spi:toppoly,td028ttec1", in which case you
>> should add
>>
>> static const struct spi_device_id td028ttec1_ids[] = {
>> { "toppoly,td028ttec1", 0 },
>> { "tpo,td028ttec1", 0},
>> { /* sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
>
> We already have a static const struct of_device_id td028ttec1_of_match[]
> table with the same information.
>
> So we still have two places to keep in sync.
>
> Or can we remove the td028ttec1_of_match[]? AFAIK not.
>
>>
>> link to it in the td028ttec1_spi_driver struct:
>> .id_table = td028ttec1_ids,
>>
>> Then test again to see that the module still loads with the new and old
>> DT string.
>
> In total I am not really convinced that adding 7 lines of code is better
> than one (the "tpo," alias) that is tested and works...
>
> And it looks like a lot of unplanned code testing for me which takes more
> than 5 minutes :)
>
> So I'd prefer to leave that exercise of fixing the MODULE_ALIAS/DEVICE_TABLE
> to someone else...
>
That's fine, someday I'll probably get some script to do this for all
the drivers that still have MODULE_ALIAS and an existing table.
> BR and thanks,
> Nikolaus
>
>>
>> Andrew
>>
>>> BR and thanks,
>>> Nikolaus Schaller
>>>
>
> Should I submit a new version?
>
> BR,
> Nikolaus
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
Hi Andrew,
> Am 16.11.2017 um 19:32 schrieb Andrew F. Davis :
>
> On 11/16/2017 12:18 PM, H. Nikolaus Schaller wrote:
>>
>>> Am 16.11.2017 um 18:08 schrieb Andrew F. Davis :
>>>
>>> On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
Hi Andrew,
> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
>
> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>>
>>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>>>
>>> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
The vendor name was "toppoly" but other panels and the vendor list
have defined it as "tpo". So let's fix it in driver and bindings.
Signed-off-by: H. Nikolaus Schaller
---
>>>
>>>
-MODULE_ALIAS("spi:toppoly,td028ttec1");
+MODULE_ALIAS("spi:tpo,td028ttec1");
>>>
>>> Doesn't this mean that the module won't load if you have old bindings?
>>
>> Hm.
>>
>> Well, I think it can load but doesn't automatically from DT strings
>> which might
>> be unexpected.
>>
>>> Can't we have two module aliases?
>>
>> I think we can. Just a random example:
>> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>>
>> So we should keep both.
>
> Even better would be to drop both MODULE_ALIAS and let the
> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
Why would that be better?
>>>
>>> MODULE_ALIAS is ugly, you already have a table (usually) of device names
>>> that are supported by the driver, the module aliases should be generated
>>> from that table. This also keeps supported device list in one place.
>>>
As far as I see it will need more code and changes than adding one line of
MODULE_ALIAS.
> Although it doesn't look like this driver has an SPI id table, you
> should probably add one, I be interested to see if this module is always
> being matched through the "spi" or the "of" alias..
Could you please propose how that code should look like, so that I can
test?
>>>
>>> Sure,
>>>
>>> start with
>>> $ udevadm monitor
>>> and see what string the kernel is looking for when trying to find a
>>> module for this device.
>>
>> Well, the module is loaded automatically from DT at boot time well before
>> I can start udevadm. So that is the most tricky part to setup the system
>> to suppress this...
>>
>>>
>>> If it is only ever looking for "of:toppoly,td028ttec1", then you can
>>> drop the MODULE_ALIAS and be done as it was never getting used anyway.
>>
>> Since it is an SPI client, I am sure it looks for "spi:something.
>>
>>>
>>> What I expect though is "spi:toppoly,td028ttec1", in which case you
>>> should add
>>>
>>> static const struct spi_device_id td028ttec1_ids[] = {
>>> { "toppoly,td028ttec1", 0 },
>>> { "tpo,td028ttec1", 0},
>>> { /* sentinel */ }
>>> };
>>> MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
>>
>> We already have a static const struct of_device_id td028ttec1_of_match[]
>> table with the same information.
>>
>> So we still have two places to keep in sync.
>>
>> Or can we remove the td028ttec1_of_match[]? AFAIK not.
>>
>>>
>>> link to it in the td028ttec1_spi_driver struct:
>>> .id_table = td028ttec1_ids,
>>>
>>> Then test again to see that the module still loads with the new and old
>>> DT string.
>>
>> In total I am not really convinced that adding 7 lines of code is better
>> than one (the "tpo," alias) that is tested and works...
>>
>> And it looks like a lot of unplanned code testing for me which takes more
>> than 5 minutes :)
>>
>> So I'd prefer to leave that exercise of fixing the MODULE_ALIAS/DEVICE_TABLE
>> to someone else...
>>
>
> That's fine, someday I'll probably get some script to do this for all
> the drivers that still have MODULE_ALIAS and an existing table.
That would be cool!
On a second thought, I think there is a quick experiment for this driver
not needing to monitor events.
1st attempt: remove ALIASES => if it still loads it would be fine
2nd attempt: add your id table => if it loads again, it is fine
if not, let's keep ALIASES.
Maybe I can try tomorrow.
BR and thanks,
Nikolaus
>
>> BR and thanks,
>> Nikolaus
>>
>>>
>>> Andrew
>>>
BR and thanks,
Nikolaus Schaller
>
>>
>> Should I submit a new version?
>>
>> BR,
>> Nikolaus
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> To unsubscribe from this list: send the line "u
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
> Am 16.11.2017 um 18:08 schrieb Andrew F. Davis :
>
> On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
>> Hi Andrew,
>>
>>> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
>>>
>>> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>
> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
>> The vendor name was "toppoly" but other panels and the vendor list
>> have defined it as "tpo". So let's fix it in driver and bindings.
>>
>> Signed-off-by: H. Nikolaus Schaller
>> ---
>
>
>> -MODULE_ALIAS("spi:toppoly,td028ttec1");
>> +MODULE_ALIAS("spi:tpo,td028ttec1");
>
> Doesn't this mean that the module won't load if you have old bindings?
Hm.
Well, I think it can load but doesn't automatically from DT strings which
might
be unexpected.
> Can't we have two module aliases?
I think we can. Just a random example:
https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
So we should keep both.
>>>
>>> Even better would be to drop both MODULE_ALIAS and let the
>>> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
>>
>> Why would that be better?
>>
>
> MODULE_ALIAS is ugly, you already have a table (usually) of device names
> that are supported by the driver, the module aliases should be generated
> from that table. This also keeps supported device list in one place.
>
>> As far as I see it will need more code and changes than adding one line of
>> MODULE_ALIAS.
>>
>>> Although it doesn't look like this driver has an SPI id table, you
>>> should probably add one, I be interested to see if this module is always
>>> being matched through the "spi" or the "of" alias..
>>
>> Could you please propose how that code should look like, so that I can test?
>>
>
> Sure,
>
> start with
> $ udevadm monitor
> and see what string the kernel is looking for when trying to find a
> module for this device.
Well, the module is loaded automatically from DT at boot time well before
I can start udevadm. So that is the most tricky part to setup the system
to suppress this...
>
> If it is only ever looking for "of:toppoly,td028ttec1", then you can
> drop the MODULE_ALIAS and be done as it was never getting used anyway.
Since it is an SPI client, I am sure it looks for "spi:something.
>
> What I expect though is "spi:toppoly,td028ttec1", in which case you
> should add
>
> static const struct spi_device_id td028ttec1_ids[] = {
> { "toppoly,td028ttec1", 0 },
> { "tpo,td028ttec1", 0},
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
We already have a static const struct of_device_id td028ttec1_of_match[]
table with the same information.
So we still have two places to keep in sync.
Or can we remove the td028ttec1_of_match[]? AFAIK not.
>
> link to it in the td028ttec1_spi_driver struct:
> .id_table = td028ttec1_ids,
>
> Then test again to see that the module still loads with the new and old
> DT string.
In total I am not really convinced that adding 7 lines of code is better
than one (the "tpo," alias) that is tested and works...
And it looks like a lot of unplanned code testing for me which takes more
than 5 minutes :)
So I'd prefer to leave that exercise of fixing the MODULE_ALIAS/DEVICE_TABLE
to someone else...
BR and thanks,
Nikolaus
>
> Andrew
>
>> BR and thanks,
>> Nikolaus Schaller
>>
>>>
Should I submit a new version?
BR,
Nikolaus
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/10] drm/tegra: Deliver job completion callback to client
On 05.11.2017 14:01, Mikko Perttunen wrote:
> To allow client drivers to free resources when jobs have completed,
> deliver job completion callbacks to them. This requires adding
> reference counting to context objects, as job completion can happen
> after the userspace application has closed the context. As such,
> also add kref-based refcounting for contexts.
>
> Signed-off-by: Mikko Perttunen
> ---
> drivers/gpu/drm/tegra/drm.c | 27 ---
> drivers/gpu/drm/tegra/drm.h | 4
> 2 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 2cdd054520bf..3e2a4a19412e 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -281,8 +281,11 @@ static int tegra_drm_open(struct drm_device *drm, struct
> drm_file *filp)
> return 0;
> }
>
> -static void tegra_drm_context_free(struct tegra_drm_context *context)
> +static void tegra_drm_context_free(struct kref *ref)
> {
> + struct tegra_drm_context *context =
> + container_of(ref, struct tegra_drm_context, ref);
> +
> context->client->ops->close_channel(context);
> kfree(context);
> }
> @@ -379,6 +382,16 @@ static int host1x_waitchk_copy_from_user(struct
> host1x_waitchk *dest,
> return 0;
> }
>
> +static void tegra_drm_job_done(struct host1x_job *job)
> +{
> + struct tegra_drm_context *context = job->callback_data;
> +
> + if (context->client->ops->submit_done)
> + context->client->ops->submit_done(context);
> +
> + kref_put(&context->ref, tegra_drm_context_free);
> +}
> +
> int tegra_drm_submit(struct tegra_drm_context *context,
>struct drm_tegra_submit *args, struct drm_device *drm,
>struct drm_file *file)
> @@ -560,6 +573,9 @@ int tegra_drm_submit(struct tegra_drm_context *context,
> job->syncpt_id = syncpt.id;
> job->timeout = 1;
>
> + job->done = tegra_drm_job_done;
> + job->callback_data = context;
> +
> if (args->timeout && args->timeout < 1)
> job->timeout = args->timeout;
>
> @@ -567,8 +583,11 @@ int tegra_drm_submit(struct tegra_drm_context *context,
> if (err)
> goto fail;
>
> + kref_get(&context->ref);
> +
> err = host1x_job_submit(job);
> if (err) {
> + kref_put(&context->ref, tegra_drm_context_free);
> host1x_job_unpin(job);
> goto fail;
> }
> @@ -717,6 +736,8 @@ static int tegra_open_channel(struct drm_device *drm,
> void *data,
> if (err < 0)
> kfree(context);
>
> + kref_init(&context->ref);
> +
> mutex_unlock(&fpriv->lock);
> return err;
> }
> @@ -738,7 +759,7 @@ static int tegra_close_channel(struct drm_device *drm,
> void *data,
> }
>
> idr_remove(&fpriv->contexts, context->id);
> - tegra_drm_context_free(context);
> + kref_put(&context->ref, tegra_drm_context_free);
>
> unlock:
> mutex_unlock(&fpriv->lock);
> @@ -1026,7 +1047,7 @@ static int tegra_drm_context_cleanup(int id, void *p,
> void *data)
> {
> struct tegra_drm_context *context = p;
>
> - tegra_drm_context_free(context);
> + kref_put(&context->ref, tegra_drm_context_free);
>
Probably won't hurt to add and use
tegra_drm_context_get()/tegra_drm_context_put().
> return 0;
> }
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 063f5d397526..079aebb3fb38 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -74,6 +75,8 @@ struct tegra_drm {
> struct tegra_drm_client;
>
> struct tegra_drm_context {
> + struct kref ref;
> +
> struct tegra_drm_client *client;
> struct host1x_channel *channel;
> unsigned int id;
> @@ -88,6 +91,7 @@ struct tegra_drm_client_ops {
> int (*submit)(struct tegra_drm_context *context,
> struct drm_tegra_submit *args, struct drm_device *drm,
> struct drm_file *file);
> + void (*submit_done)(struct tegra_drm_context *context);
> };
>
> int tegra_drm_submit(struct tegra_drm_context *context,
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/10] drm/tegra: Deliver job completion callback to client
On 05.11.2017 14:01, Mikko Perttunen wrote:
> To allow client drivers to free resources when jobs have completed,
> deliver job completion callbacks to them. This requires adding
> reference counting to context objects, as job completion can happen
> after the userspace application has closed the context. As such,
> also add kref-based refcounting for contexts.
>
It's very likely that we will need contexts kref'ing for other things as well,
would be nice if you could factor out kref addition into a standalone patch.
> Signed-off-by: Mikko Perttunen
> ---
> drivers/gpu/drm/tegra/drm.c | 27 ---
> drivers/gpu/drm/tegra/drm.h | 4
> 2 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 2cdd054520bf..3e2a4a19412e 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -281,8 +281,11 @@ static int tegra_drm_open(struct drm_device *drm, struct
> drm_file *filp)
> return 0;
> }
>
> -static void tegra_drm_context_free(struct tegra_drm_context *context)
> +static void tegra_drm_context_free(struct kref *ref)
> {
> + struct tegra_drm_context *context =
> + container_of(ref, struct tegra_drm_context, ref);
> +
> context->client->ops->close_channel(context);
> kfree(context);
> }
> @@ -379,6 +382,16 @@ static int host1x_waitchk_copy_from_user(struct
> host1x_waitchk *dest,
> return 0;
> }
>
> +static void tegra_drm_job_done(struct host1x_job *job)
> +{
> + struct tegra_drm_context *context = job->callback_data;
> +
> + if (context->client->ops->submit_done)
> + context->client->ops->submit_done(context);
> +
> + kref_put(&context->ref, tegra_drm_context_free);
> +}
> +
> int tegra_drm_submit(struct tegra_drm_context *context,
>struct drm_tegra_submit *args, struct drm_device *drm,
>struct drm_file *file)
> @@ -560,6 +573,9 @@ int tegra_drm_submit(struct tegra_drm_context *context,
> job->syncpt_id = syncpt.id;
> job->timeout = 1;
>
> + job->done = tegra_drm_job_done;
> + job->callback_data = context;
> +
> if (args->timeout && args->timeout < 1)
> job->timeout = args->timeout;
>
> @@ -567,8 +583,11 @@ int tegra_drm_submit(struct tegra_drm_context *context,
> if (err)
> goto fail;
>
> + kref_get(&context->ref);
> +
> err = host1x_job_submit(job);
> if (err) {
> + kref_put(&context->ref, tegra_drm_context_free);
> host1x_job_unpin(job);
> goto fail;
> }
> @@ -717,6 +736,8 @@ static int tegra_open_channel(struct drm_device *drm,
> void *data,
> if (err < 0)
> kfree(context);
>
> + kref_init(&context->ref);
> +
Would be a bit cleaner to move kref_init into tegra_client_open() where the rest
of context variables getting initialized.
> mutex_unlock(&fpriv->lock);
> return err;
> }
> @@ -738,7 +759,7 @@ static int tegra_close_channel(struct drm_device *drm,
> void *data,
> }
>
> idr_remove(&fpriv->contexts, context->id);
> - tegra_drm_context_free(context);
> + kref_put(&context->ref, tegra_drm_context_free);
>
> unlock:
> mutex_unlock(&fpriv->lock);
> @@ -1026,7 +1047,7 @@ static int tegra_drm_context_cleanup(int id, void *p,
> void *data)
> {
> struct tegra_drm_context *context = p;
>
> - tegra_drm_context_free(context);
> + kref_put(&context->ref, tegra_drm_context_free);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 063f5d397526..079aebb3fb38 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -74,6 +75,8 @@ struct tegra_drm {
> struct tegra_drm_client;
>
> struct tegra_drm_context {
> + struct kref ref;
> +
> struct tegra_drm_client *client;
> struct host1x_channel *channel;
> unsigned int id;
> @@ -88,6 +91,7 @@ struct tegra_drm_client_ops {
> int (*submit)(struct tegra_drm_context *context,
> struct drm_tegra_submit *args, struct drm_device *drm,
> struct drm_file *file);
> + void (*submit_done)(struct tegra_drm_context *context);
> };
>
> int tegra_drm_submit(struct tegra_drm_context *context,
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>
>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>>
>> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
>>> The vendor name was "toppoly" but other panels and the vendor list
>>> have defined it as "tpo". So let's fix it in driver and bindings.
>>>
>>> Signed-off-by: H. Nikolaus Schaller
>>> ---
>>
>>
>>> -MODULE_ALIAS("spi:toppoly,td028ttec1");
>>> +MODULE_ALIAS("spi:tpo,td028ttec1");
>>
>> Doesn't this mean that the module won't load if you have old bindings?
>
> Hm.
>
> Well, I think it can load but doesn't automatically from DT strings which
> might
> be unexpected.
>
>> Can't we have two module aliases?
>
> I think we can. Just a random example:
> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>
> So we should keep both.
Even better would be to drop both MODULE_ALIAS and let the
MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
Although it doesn't look like this driver has an SPI id table, you
should probably add one, I be interested to see if this module is always
being matched through the "spi" or the "of" alias..
>
> Should I submit a new version?
>
> BR,
> Nikolaus
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] treewide: Fix line continuation formats
On Thu, 2017-11-16 at 09:17 -0800, Joe Perches wrote: > On Thu, 2017-11-16 at 12:11 -0500, Mimi Zohar wrote: > > On Thu, 2017-11-16 at 07:27 -0800, Joe Perches wrote: > > > Avoid using line continations in formats as that causes unexpected > > > output. > > > > Is having lines greater than 80 characters the preferred method? > > yes. > > > Could you add quotes before the backlash and before the first word on > > the next line instead? > > coalesced formats are preferred. In the future, please reference the commit 6f76b6fcaa60 "CodingStyle: Document the exception of not splitting user-visible strings, for grepping" thanks, Mimi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
Hi Andrew,
> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis :
>
> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>>
>>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen :
>>>
>>> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
The vendor name was "toppoly" but other panels and the vendor list
have defined it as "tpo". So let's fix it in driver and bindings.
Signed-off-by: H. Nikolaus Schaller
---
>>>
>>>
-MODULE_ALIAS("spi:toppoly,td028ttec1");
+MODULE_ALIAS("spi:tpo,td028ttec1");
>>>
>>> Doesn't this mean that the module won't load if you have old bindings?
>>
>> Hm.
>>
>> Well, I think it can load but doesn't automatically from DT strings which
>> might
>> be unexpected.
>>
>>> Can't we have two module aliases?
>>
>> I think we can. Just a random example:
>> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>>
>> So we should keep both.
>
> Even better would be to drop both MODULE_ALIAS and let the
> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
Why would that be better?
As far as I see it will need more code and changes than adding one line of
MODULE_ALIAS.
> Although it doesn't look like this driver has an SPI id table, you
> should probably add one, I be interested to see if this module is always
> being matched through the "spi" or the "of" alias..
Could you please propose how that code should look like, so that I can test?
BR and thanks,
Nikolaus Schaller
>
>>
>> Should I submit a new version?
>>
>> BR,
>> Nikolaus
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 4.1 EOL
On 11/16/17, Jani Nikula wrote: > On Wed, 15 Nov 2017, Tuncer Ayaz wrote: > > I don't follow why you think it's a different platform and how I > > might have "more" definitely shown v4.1 to be good, but I'll trust > > your judgement as a drm dev and not argue :). > > You apparently have Sandy Bridge, the referenced reports are about > Broadwell and Skylake. Even if the symptoms you see are the same, > the root causes might be wildly different, needing a different fix. Thanks for taking time to explain and clear my confusion :). I checked the comments of the other reporter with a Sandy Bridge system, and they haven't provided a proper trace. Hence, you're absolutely right. > I've learned the hard way not to make assumptions without detailed > information, which in this case I don't have. As in, I don't even > know for sure if you have Sandy Bridge or not, although it's alluded > to in your message. I do (Sandy Bridge), sorry for not being clearer about that. > From my point of view, you're shouting regression while giving us > nothing to work with. You need to help us to help you. Like I said, will do. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC] [PATCH] drm: adv7511/33: Fix adv7511_cec_init() failure handling
If the device tree for a board did not specify a cec clock, then
adv7511_cec_init would return an error, which would cause adv7511_probe()
to fail and thus there is no HDMI output.
There is no need to have adv7511_probe() fail if the CEC initialization
fails, so just change adv7511_cec_init() to a void function. In addition,
adv7511_cec_init() should just return silently if the cec clock isn't
found and show a message for any other errors.
An otherwise correct cleanup patch from Dan Carpenter turned this broken
failure handling into a kernel Oops, so bisection points to commit
7af35b0addbc ("drm/kirin: Checking for IS_ERR() instead of NULL") rather
than 3b1b975003e4 ("drm: adv7511/33: add HDMI CEC support").
Based on earlier patches from Arnd and John.
Reported-by: Naresh Kamboju
Cc: Xinliang Liu
Cc: Dan Carpenter
Cc: Sean Paul
Cc: Hans Verkuil
Cc: Archit Taneja
Cc: John Stultz
Link: https://bugs.linaro.org/show_bug.cgi?id=3345
Link: https://lkft.validation.linaro.org/scheduler/job/48017#L3551
Fixes: 7af35b0addbc ("drm/kirin: Checking for IS_ERR() instead of NULL")
Fixes: 3b1b975003e4 ("drm: adv7511/33: add HDMI CEC support")
Signed-off-by: Hans Verkuil
---
This rework of Arnd and John's patches goes a bit further and makes
cec_init a void function and just silently exits if there is no cec clock
defined in the dts. I'm sure that's the reason why the kirin board failed
on this. BTW: if the kirin board DOES support cec, then it would be nice
if it can be hooked up in the dts!
I'll test this with my two adv7511/33 boards on Monday.
Regards,
Hans
---
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 543a5eb91624..bc17aa965e58 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -374,8 +374,8 @@ struct adv7511 {
};
#ifdef CONFIG_DRM_I2C_ADV7511_CEC
-int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511,
-unsigned int offset);
+void adv7511_cec_init(struct device *dev, struct adv7511 *adv7511,
+ unsigned int offset);
void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
#endif
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
index b33d730e4d73..c1cd471d31fa 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
@@ -300,18 +300,20 @@ static int adv7511_cec_parse_dt(struct device *dev,
struct adv7511 *adv7511)
return 0;
}
-int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511,
-unsigned int offset)
+void adv7511_cec_init(struct device *dev, struct adv7511 *adv7511,
+ unsigned int offset)
{
int ret = adv7511_cec_parse_dt(dev, adv7511);
if (ret)
- return ret;
+ goto disable_cec;
adv7511->cec_adap = cec_allocate_adapter(&adv7511_cec_adap_ops,
adv7511, dev_name(dev), CEC_CAP_DEFAULTS, ADV7511_MAX_ADDRS);
- if (IS_ERR(adv7511->cec_adap))
- return PTR_ERR(adv7511->cec_adap);
+ if (IS_ERR(adv7511->cec_adap)) {
+ ret = PTR_ERR(adv7511->cec_adap);
+ goto fail;
+ }
regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, 0);
/* cec soft reset */
@@ -329,9 +331,15 @@ int adv7511_cec_init(struct device *dev, struct adv7511
*adv7511,
((adv7511->cec_clk_freq / 75) - 1) << 2);
ret = cec_register_adapter(adv7511->cec_adap, dev);
- if (ret) {
- cec_delete_adapter(adv7511->cec_adap);
- adv7511->cec_adap = NULL;
- }
- return ret;
+ if (!ret)
+ return;
+ cec_delete_adapter(adv7511->cec_adap);
+ adv7511->cec_adap = NULL;
+
+fail:
+ dev_info(dev, "Initializing CEC failed with error %d, disabling CEC\n",
+ret);
+disable_cec:
+ regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
+ADV7511_CEC_CTRL_POWER_DOWN);
}
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 3a33075dbb22..56a6a1fa 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1202,9 +1202,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
struct i2c_device_id *id)
offset = adv7511->type == ADV7533 ? ADV7533_REG_CEC_OFFSET : 0;
#ifdef CONFIG_DRM_I2C_ADV7511_CEC
- ret = adv7511_cec_init(dev, adv7511, offset);
- if (ret)
- goto err_unregister_cec;
+ adv7511_cec_init(dev, adv7511, offset);
#else
regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
ADV7511_CEC_CTRL_POWER_DOWN);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http
[PATCH] omapdrm: hdmi4: Correct the SoC revision matching
I believe the intention of the commit 2c9fc9bf45f8
("drm: omapdrm: Move FEAT_HDMI_* features to hdmi4 driver")
was to identify omap4430 ES1.x, omap4430 ES2.x and other OMAP4 revisions,
like omap4460.
By using family=OMAP4 in the match the code will treat omap4460 ES1.x in a
same way as it would treat omap4430 ES1.x
This breaks HDMI audio on OMAP4460 devices (PandaES for example).
Correct the match rule so we are not going to get false positive match.
Fixes: 2c9fc9bf45f8 ("drm: omapdrm: Move FEAT_HDMI_* features to hdmi4 driver")
CC: sta...@vger.kernel.org # 4.14
Signed-off-by: Peter Ujfalusi
---
drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
index 62e451162d96..07945a40c33a 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
@@ -902,9 +902,20 @@ static const struct hdmi4_features hdmi4_es3_features = {
};
static const struct soc_device_attribute hdmi4_soc_devices[] = {
- { .family = "OMAP4", .revision = "ES1.?", .data = &hdmi4_es1_features },
- { .family = "OMAP4", .revision = "ES2.?", .data = &hdmi4_es2_features },
- { .family = "OMAP4", .data = &hdmi4_es3_features },
+ {
+ .machine = "OMAP4430",
+ .revision = "ES1.?",
+ .data = &hdmi4_es1_features,
+ },
+ {
+ .machine = "OMAP4430",
+ .revision = "ES2.?",
+ .data = &hdmi4_es2_features,
+ },
+ {
+ .family = "OMAP4",
+ .data = &hdmi4_es3_features,
+ },
{ /* sentinel */ }
};
--
Peter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
