[Bug 65911] radeon: garbled output/only noise through HDMI and GPU lockups
https://bugzilla.kernel.org/show_bug.cgi?id=65911 --- Comment #11 from tomka --- In another bug report (https://bugzilla.redhat.com/show_bug.cgi?id=990986) it was suggested to disable glx-tls during build configuration. So I replaced the line "--enable-glx-tls" with "--disable-glx-tls". However, the GPU lock ups still happen and I can't start X. Then I tried setting several environment variables like it was suggested in yet another bug report (https://bugs.freedesktop.org/show_bug.cgi?id=69728): Neither R600_DEBUG=nohyperz, R600_DEBUG=nodma, R600_DEBUG=nosb or R600_LLVM=0 did help. Not alone and not combined. Additionally, I removed the lines "--with-llvm-shared-libs" and "--enable-gallium-llvm" from the ./configure parameters, because I read in bug 69728 there might be some issues with LLVM. This also didn't change anything (the binaries got obisously much bigger, though). The next thing I wanted to try is to disable "the new DMA ring for ttm bo moves" like suggested by you (Alex) in another thread about GPU lockups (https://groups.google.com/forum/#!topic/fa.linux.kernel/1_KzqknQn_U). However, this change seems to be already in the mainline kernel. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] drm/edid: parse the list of additional 3D modes
On Fri, Nov 29, 2013 at 06:18:58PM +, Thomas Wood wrote:
> Parse 2D_VIC_order_X and 3D_Structure_X from the list at the end of the
> HDMI Vendor Specific Data Block.
>
> v2: Use an offset value depending on 3D_Multi_present and add
> detail_present. (Ville Syrj?l?)
> v3: Make sure the list is parsed even if 3D_Structure_ALL/MASK is not
> present. (Ville Syrj?l?)
> Fix one length check and remove another. (Ville Syrj?l?)
>
> Signed-off-by: Thomas Wood
For the series:
Reviewed-by: Ville Syrj?l?
> ---
> drivers/gpu/drm/drm_edid.c | 94
> +++---
> 1 file changed, 73 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index f1d6e1e..4c82a80 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2734,7 +2734,7 @@ static int
> do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
> const u8 *video_db, u8 video_len)
> {
> - int modes = 0, offset = 0, i, multi_present = 0;
> + int modes = 0, offset = 0, i, multi_present = 0, multi_len;
> u8 vic_len, hdmi_3d_len = 0;
> u16 mask;
> u16 structure_all;
> @@ -2780,32 +2780,84 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
> const u8 *db, u8 len,
> }
> offset += 1 + vic_len;
>
> - if (!(multi_present == 1 || multi_present == 2))
> - goto out;
> + if (multi_present == 1)
> + multi_len = 2;
> + else if (multi_present == 2)
> + multi_len = 4;
> + else
> + multi_len = 0;
>
> - if ((multi_present == 1 && len < (9 + offset)) ||
> - (multi_present == 2 && len < (11 + offset)))
> + if (len < (8 + offset + hdmi_3d_len - 1))
> goto out;
>
> - if ((multi_present == 1 && hdmi_3d_len < 2) ||
> - (multi_present == 2 && hdmi_3d_len < 4))
> + if (hdmi_3d_len < multi_len)
> goto out;
>
> - /* 3D_Structure_ALL */
> - structure_all = (db[8 + offset] << 8) | db[9 + offset];
> + if (multi_present == 1 || multi_present == 2) {
> + /* 3D_Structure_ALL */
> + structure_all = (db[8 + offset] << 8) | db[9 + offset];
>
> - /* check if 3D_MASK is present */
> - if (multi_present == 2)
> - mask = (db[10 + offset] << 8) | db[11 + offset];
> - else
> - mask = 0x;
> -
> - for (i = 0; i < 16; i++) {
> - if (mask & (1 << i))
> - modes += add_3d_struct_modes(connector,
> - structure_all,
> - video_db,
> - video_len, i);
> + /* check if 3D_MASK is present */
> + if (multi_present == 2)
> + mask = (db[10 + offset] << 8) | db[11 + offset];
> + else
> + mask = 0x;
> +
> + for (i = 0; i < 16; i++) {
> + if (mask & (1 << i))
> + modes += add_3d_struct_modes(connector,
> + structure_all,
> + video_db,
> + video_len, i);
> + }
> + }
> +
> + offset += multi_len;
> +
> + for (i = 0; i < (hdmi_3d_len - multi_len); i++) {
> + int vic_index;
> + struct drm_display_mode *newmode = NULL;
> + unsigned int newflag = 0;
> + bool detail_present;
> +
> + detail_present = ((db[8 + offset + i] & 0x0f) > 7);
> +
> + if (detail_present && (i + 1 == hdmi_3d_len - multi_len))
> + break;
> +
> + /* 2D_VIC_order_X */
> + vic_index = db[8 + offset + i] >> 4;
> +
> + /* 3D_Structure_X */
> + switch (db[8 + offset + i] & 0x0f) {
> + case 0:
> + newflag = DRM_MODE_FLAG_3D_FRAME_PACKING;
> + break;
> + case 6:
> + newflag = DRM_MODE_FLAG_3D_TOP_AND_BOTTOM;
> + break;
> + case 8:
> + /* 3D_Detail_X */
> + if ((db[9 + offset + i] >> 4) == 1)
> + newflag = DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF;
> + break;
> + }
> +
> + if (newflag != 0) {
> + newmode = drm_display_mode_from_vic_index(connector,
> + video_db,
> + video_len,
> + vic_index);
> +
> + if (newmode) {
> + newmode->flags |= newflag;
> + drm_mo
[Bug 71859] texelFetch segfault in libLLVM-3.3.so (on Cayman)
https://bugs.freedesktop.org/show_bug.cgi?id=71859 --- Comment #11 from Alexandre Demers --- Coredump with R600_LLVM=1 from running /home/dema1701/projects/display/piglit/bin/texelFetch vs isampler2DArray b0r1 -auto -fbo 65x32x5 Stack dump: 0.Running pass 'Function Pass Manager' on module 'tgsi'. 1.Running pass 'AMDGPU DAG->DAG Pattern Instruction Selection' on function '@main' Segmentation fault (core dumped) Coredump can be downloaded from: https://drive.google.com/file/d/0Bw_tZdWsNa4BemdsVUV4UUktWFk/edit?usp=sharing -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/958b4bb6/attachment.html>
[Bug 71859] texelFetch segfault in libLLVM-3.3.so (on Cayman)
https://bugs.freedesktop.org/show_bug.cgi?id=71859 --- Comment #10 from Alexandre Demers --- Disabling llvm (R600_LLVM=0) prevents the crash. So, I enabled it back, ran one of the test and had a coredump. I'll be attaching it in a moment. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/302e400f/attachment.html>
[Bug 69723] GPU lockups with kernel 3.11.0 / 3.12-rc1 when dpm=1 on r600g (Cayman)
https://bugs.freedesktop.org/show_bug.cgi?id=69723 --- Comment #52 from Alexandre Demers --- I disabled R600_LLVM and ran another piglit at high power level. It crashed anyway. But I got a different message in my journal: Nov 29 14:02:48 Xander kernel: glx-create-cont[14825]: segfault at 17c ip 7f474ede915e sp 7fff555e8ff0 error 6 in r600_dri.so[7f474e81d000+80e000] Nov 29 14:02:48 Xander systemd-coredump[14834]: Process 14825 (glx-create-cont) dumped core. Nov 29 14:09:06 Xander kernel: traps: shader_runner[20951] trap int3 ip:7fd47a14782f sp:7fffeb3ab520 error:0 Nov 29 14:09:06 Xander systemd-coredump[20968]: Process 20951 (shader_runner) dumped core. Nov 29 14:09:21 Xander kernel: traps: shader_runner[22965] trap int3 ip:7f9e6348982f sp:7fff739db970 error:0 Nov 29 14:09:21 Xander systemd-coredump[22983]: Process 22965 (shader_runner) dumped core. Nov 29 14:11:09 Xander dbus-daemon[3115]: dbus[3115]: [system] Activating via systemd: service name='org.freedesktop.hostname1' unit='dbus-org.freedesktop.hostname1.service' Nov 29 14:11:09 Xander dbus[3115]: [system] Activating via systemd: service name='org.freedesktop.hostname1' unit='dbus-org.freedesktop.hostname1.service' Nov 29 14:11:09 Xander systemd[1]: Starting Hostname Service... Nov 29 14:11:09 Xander dbus-daemon[3115]: dbus[3115]: [system] Successfully activated service 'org.freedesktop.hostname1' Nov 29 14:11:09 Xander dbus[3115]: [system] Successfully activated service 'org.freedesktop.hostname1' Nov 29 14:11:09 Xander systemd[1]: Started Hostname Service. Nov 29 14:12:19 Xander kernel: radeon_gem_object_create:62 alloc size 1365Mb bigger than 256Mb limit Nov 29 14:12:19 Xander kernel: radeon_gem_object_create:62 alloc size 1365Mb bigger than 256Mb limit Nov 29 14:12:41 Xander kernel: radeon_gem_object_create:62 alloc size 1024Mb bigger than 256Mb limit Nov 29 14:12:41 Xander kernel: radeon_gem_object_create:62 alloc size 1024Mb bigger than 256Mb limit Nov 29 14:13:47 Xander systemd[1]: Starting Cleanup of Temporary Directories... -- Reboot -- Still dumps and a GEM object allocation problem. Anything usefull? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/fba691fd/attachment.html>
[PATCH 3/3] drm/edid: parse the list of additional 3D modes
On Fri, Nov 29, 2013 at 03:33:28PM +, Thomas Wood wrote:
> Parse 2D_VIC_order_X and 3D_Structure_X from the list at the end of the
> HDMI Vendor Specific Data Block.
>
> v2: Use an offset value depending on 3D_Multi_present and add
> detail_present. (Ville Syrj?l?)
>
> Signed-off-by: Thomas Wood
> ---
> drivers/gpu/drm/drm_edid.c | 64
> ++
> 1 file changed, 59 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index f1d6e1e..9426563 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2734,7 +2734,7 @@ static int
> do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
> const u8 *video_db, u8 video_len)
> {
> - int modes = 0, offset = 0, i, multi_present = 0;
> + int modes = 0, offset = 0, i, multi_present = 0, multi_len;
> u8 vic_len, hdmi_3d_len = 0;
> u16 mask;
> u16 structure_all;
> @@ -2783,12 +2783,15 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
> const u8 *db, u8 len,
> if (!(multi_present == 1 || multi_present == 2))
> goto out;
We should actually skip the 3D_Structure_ALL/MASK stuff in
this case, but we should still go on and parse the 2D_VIC_order
stuff. So this check should be removed.
>
> - if ((multi_present == 1 && len < (9 + offset)) ||
> - (multi_present == 2 && len < (11 + offset)))
> + if (multi_present == 1)
> + multi_len = 2;
> + else
and this should then be 'else if (multi_present == 2)' and
initialize multi_len to 0 in the beginning.
> + multi_len = 4;
> +
> + if (len < (offset + hdmi_3d_len))
At this point db[8 + offset] we be the first 3d byte, so
I think this should actually be something like:
if (len < (8 + offset + hdmi_3d_len - 1))
> goto out;
>
> - if ((multi_present == 1 && hdmi_3d_len < 2) ||
> - (multi_present == 2 && hdmi_3d_len < 4))
> + if (hdmi_3d_len < multi_len)
> goto out;
>
> /* 3D_Structure_ALL */
> @@ -2808,6 +2811,57 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
> const u8 *db, u8 len,
>video_len, i);
> }
>
> + if (hdmi_3d_len <= multi_len)
> + goto out;
Already checked, except the == case, but the loop will check
that for us, so this check can be dropped.
> +
> + offset += multi_len;
> +
> + for (i = 0; i < (hdmi_3d_len - multi_len); i++) {
> + int vic_index;
> + struct drm_display_mode *newmode = NULL;
> + unsigned int newflag = 0;
> + bool detail_present;
> +
> + detail_present = ((db[8 + offset + i] & 0x0f) > 7);
> +
> + if (detail_present && (i + 1 == hdmi_3d_len - multi_len))
> + break;
> +
> + /* 2D_VIC_order_X */
> + vic_index = db[8 + offset + i] >> 4;
> +
> + /* 3D_Structure_X */
> + switch (db[8 + offset + i] & 0x0f) {
> + case 0:
> + newflag = DRM_MODE_FLAG_3D_FRAME_PACKING;
> + break;
> + case 6:
> + newflag = DRM_MODE_FLAG_3D_TOP_AND_BOTTOM;
> + break;
> + case 8:
> + /* 3D_Detail_X */
> + if ((db[9 + offset + i] >> 4) == 1)
> + newflag = DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF;
> + break;
> + }
> +
> + if (newflag != 0) {
> + newmode = drm_display_mode_from_vic_index(connector,
> + video_db,
> + video_len,
> + vic_index);
> +
> + if (newmode) {
> + newmode->flags |= newflag;
> + drm_mode_probed_add(connector, newmode);
> + modes++;
> + }
> + }
> +
> + if (detail_present)
> + i++;
> + }
> +
> out:
> return modes;
> }
> --
> 1.8.4.2
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrj?l?
Intel OTC
[PATCH 4/4] drm: exynos: hdmi: Add dt support for hdmiphy settings
Hi Shirish,
Please see my comments inline.
On Monday 25 of November 2013 14:24:39 Shirish S wrote:
> This patch adds dt support to hdmiphy config settings
> as it is board specific and depends on the signal pattern
> of board.
>
> Signed-off-by: Shirish S
> ---
> .../devicetree/bindings/video/exynos_hdmi.txt | 31
> drivers/gpu/drm/exynos/exynos_hdmi.c | 77
> +++-
> 2 files changed, 104 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> index 323983b..6eeb333 100644
> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> @@ -13,6 +13,30 @@ Required properties:
> b) pin number within the gpio controller.
> c) optional flags and pull up/down.
>
> +- hdmiphy-configs: following information about the hdmiphy config settings.
Is this node required or optional? If it's required, then it breaks
compatibility with already existing DTBs, which is not desirable.
> + a) "config: config" specifies the phy configuration settings,
> + where 'N' denotes the number of configuration, since every
> + pixel clock can have its unique configuration.
Node names should not have any semantic meaning for parsing code. I know
that there are already existing bindings which rely on presence of
particularly named nodes, but that's not right and new bindings should
not follow that.
Also what do you need the label of each config node for?
Generally from parsing perspective you shouldn't really care about node
names. All you seem to do in the driver is iterating over all specified
nodes and matching them with internal driver data using pixel clock
frequency.
> + "pixel-clock" specifies the pixel clock
Vendor-specific properties should have vendor prefix, so this one should
be called "samsung,pixel-clock".
> + "conifig-de-emphasis-level" provides fine control of TMDS data
Typo: s/conifig/config
Also it should be called "samsung,de-emphasis-level".
> + pre emphasis, below shown is example for
> + data de-emphasis register at address 0x145D0040.
> + hdmiphy at 38[16] for bits[3:0] permitted values are in
> + the range of 760 mVdiff to 1400 mVdiff at 20mVdiff
> + increments for every LSB
> + hdmiphy at 38[16] for bits[7:4] permitted values are in
> + the range of 0dB to -7.45dB at increments of -0.45dB
> + for every LSB.
> + "config-clock-level" provides fine control of TMDS data
"samsung,clock-level"
> + amplitude for each channel,
> + for example if 0x145D005C is the address of clock level
[snip]
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 32ce9a6..5f599e3 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
[snip]
> +static int drm_hdmi_dt_parse_phy_conf(struct platform_device *pdev,
> + struct hdmi_context *hdata)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *dev_np = dev->of_node;
> + struct device_node *phy_conf, *cfg_np;
> + int i, pixel_clock = 0;
> +
> + /* Initialize with default config */
> + hdata->confs = hdmiphy_v14_configs;
> + hdata->nr_confs = ARRAY_SIZE(hdmiphy_v14_configs);
> +
> + phy_conf = of_find_node_by_name(dev_np, "hdmiphy-configs");
of_find_node_by_name() does not do what you need here. Please refer to
its implementation to learn why.
What you need here is of_get_child_by_name().
> + if (phy_conf == NULL) {
> + hdata->nr_confs = ARRAY_SIZE(hdmiphy_v14_configs);
> + DRM_ERROR("Did not find hdmiphy-configs node\n");
> + return -ENODEV;
> + }
> +
> + for_each_child_of_node(phy_conf, cfg_np) {
> + if (!of_find_property(cfg_np, "pixel-clock", NULL))
> + continue;
This check is not needed. You can simply check the return value of
of_property_read_u32() below (as you already do anyway).
> +
> + if (of_property_read_u32(cfg_np, "pixel-clock",
> + &pixel_clock, 1)) {
> + DRM_ERROR("Failed to get pixel clock\n");
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(hdmiphy_v14_configs); i++) {
The code would be much cleaner if you simply used the loop to find the
config you need and then do the rest outside of the loop.
> + if (hdata->confs[i].pixel_clock == pixel_clock)
> + /* Update the data de-emphasis and data level */
> +
[PATCH] drm/edid: parse the list of additional 3D modes
Parse 2D_VIC_order_X and 3D_Structure_X from the list at the end of the
HDMI Vendor Specific Data Block.
v2: Use an offset value depending on 3D_Multi_present and add
detail_present. (Ville Syrj?l?)
v3: Make sure the list is parsed even if 3D_Structure_ALL/MASK is not
present. (Ville Syrj?l?)
Fix one length check and remove another. (Ville Syrj?l?)
Signed-off-by: Thomas Wood
---
drivers/gpu/drm/drm_edid.c | 94 +++---
1 file changed, 73 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f1d6e1e..4c82a80 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2734,7 +2734,7 @@ static int
do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
const u8 *video_db, u8 video_len)
{
- int modes = 0, offset = 0, i, multi_present = 0;
+ int modes = 0, offset = 0, i, multi_present = 0, multi_len;
u8 vic_len, hdmi_3d_len = 0;
u16 mask;
u16 structure_all;
@@ -2780,32 +2780,84 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
const u8 *db, u8 len,
}
offset += 1 + vic_len;
- if (!(multi_present == 1 || multi_present == 2))
- goto out;
+ if (multi_present == 1)
+ multi_len = 2;
+ else if (multi_present == 2)
+ multi_len = 4;
+ else
+ multi_len = 0;
- if ((multi_present == 1 && len < (9 + offset)) ||
- (multi_present == 2 && len < (11 + offset)))
+ if (len < (8 + offset + hdmi_3d_len - 1))
goto out;
- if ((multi_present == 1 && hdmi_3d_len < 2) ||
- (multi_present == 2 && hdmi_3d_len < 4))
+ if (hdmi_3d_len < multi_len)
goto out;
- /* 3D_Structure_ALL */
- structure_all = (db[8 + offset] << 8) | db[9 + offset];
+ if (multi_present == 1 || multi_present == 2) {
+ /* 3D_Structure_ALL */
+ structure_all = (db[8 + offset] << 8) | db[9 + offset];
- /* check if 3D_MASK is present */
- if (multi_present == 2)
- mask = (db[10 + offset] << 8) | db[11 + offset];
- else
- mask = 0x;
-
- for (i = 0; i < 16; i++) {
- if (mask & (1 << i))
- modes += add_3d_struct_modes(connector,
-structure_all,
-video_db,
-video_len, i);
+ /* check if 3D_MASK is present */
+ if (multi_present == 2)
+ mask = (db[10 + offset] << 8) | db[11 + offset];
+ else
+ mask = 0x;
+
+ for (i = 0; i < 16; i++) {
+ if (mask & (1 << i))
+ modes += add_3d_struct_modes(connector,
+ structure_all,
+ video_db,
+ video_len, i);
+ }
+ }
+
+ offset += multi_len;
+
+ for (i = 0; i < (hdmi_3d_len - multi_len); i++) {
+ int vic_index;
+ struct drm_display_mode *newmode = NULL;
+ unsigned int newflag = 0;
+ bool detail_present;
+
+ detail_present = ((db[8 + offset + i] & 0x0f) > 7);
+
+ if (detail_present && (i + 1 == hdmi_3d_len - multi_len))
+ break;
+
+ /* 2D_VIC_order_X */
+ vic_index = db[8 + offset + i] >> 4;
+
+ /* 3D_Structure_X */
+ switch (db[8 + offset + i] & 0x0f) {
+ case 0:
+ newflag = DRM_MODE_FLAG_3D_FRAME_PACKING;
+ break;
+ case 6:
+ newflag = DRM_MODE_FLAG_3D_TOP_AND_BOTTOM;
+ break;
+ case 8:
+ /* 3D_Detail_X */
+ if ((db[9 + offset + i] >> 4) == 1)
+ newflag = DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF;
+ break;
+ }
+
+ if (newflag != 0) {
+ newmode = drm_display_mode_from_vic_index(connector,
+ video_db,
+ video_len,
+ vic_index);
+
+ if (newmode) {
+ newmode->flags |= newflag;
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+ }
+
+ if (detail_present)
+ i++;
[PATCH v3 12/32] drm/exynos: Split manager/display/subdrv
On Friday 29 of November 2013 09:13:19 Rob Clark wrote: > On Fri, Nov 29, 2013 at 4:10 AM, Tomasz Figa wrote: > > I would mostly agree with you if we were discussing SoC-internal > > components here. Mostly, because the ARM world is more complex and you > > can see the same IP across completely different SoCs from different > > vendors. > > > > However, the topic here is about external devices, outside the SoC, such > > as different kind of bridges, like the PTN3460 eDP to LVDS bridge, which > > are likely to be reused across different platforms. Similar thing is with > > using different bridges on different boards using the same SoC platform. > > I don't think having an abstraction here would be any overabstraction at > > all. Anything less would be a huge "underabstraction" in fact. > > > I think no one is arguing that we don't eventually need some better > abstraction. But as long as it is one-bridge and one-user, if the > patches otherwise have merit, add functionality that was missing > before and don't regress, then lack of infrastructure to match up > bridge and driver isn't something I will care about too much yet. > Things are allowed to be in-progress. A missing abstraction for a 1:1 > relationship is fine. This is not just one-bridge, one-user. This is about users of Exynos DRM we already have in-tree, such as Trats, Trats2 or Arndale, that the DRM bridge infrastructure could be used on and finally allowing to have display support on them. Of course you could merge this as is and then let someone else completely rewrite it (most likely in the same release cycle), but since it's not really much more work, I don't think there is any sense. Moreover, let's stick to modern kernel driver coding standards. I don't think that "I want this patchset merged so badly" is really a good excuse to get around it. After all, there would be no GKH's staging tree, if nobody cared about quality in mainline. Best regards, Tomasz
linux-next: Tree for Nov 29 (i915/intel_opregion)
On 11/28/13 18:45, Stephen Rothwell wrote: > Hi all, > > Changes since 20131128: > on x86_64: CONFIG_ACPI is not enabled. CC drivers/gpu/drm/i915/intel_opregion.o In file included from drivers/gpu/drm/i915/intel_opregion.c:31:0: include/linux/acpi_io.h:7:45: error: unknown type name 'acpi_physical_address' include/linux/acpi_io.h:8:10: error: unknown type name 'acpi_size' include/linux/acpi_io.h:13:33: error: unknown type name 'acpi_physical_address' include/linux/acpi_io.h:15:40: warning: 'struct acpi_generic_address' declared inside parameter list [enabled by default] include/linux/acpi_io.h:15:40: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default] include/linux/acpi_io.h:16:43: warning: 'struct acpi_generic_address' declared inside parameter list [enabled by default] drivers/gpu/drm/i915/intel_opregion.c: In function 'intel_opregion_setup': drivers/gpu/drm/i915/intel_opregion.c:866:2: error: implicit declaration of function 'acpi_os_ioremap' [-Werror=implicit-function-declaration] drivers/gpu/drm/i915/intel_opregion.c:866:7: warning: assignment makes pointer from integer without a cast [enabled by default] cc1: some warnings being treated as errors make[5]: *** [drivers/gpu/drm/i915/intel_opregion.o] Error 1 -- ~Randy
[PATCH v3 31/32] drm/exynos: Move lvds bridge discovery into DP driver
Hi Sean, On Tuesday 29 of October 2013 12:13:17 Sean Paul wrote: > This patch moves the lvds bridge discovery and connector pre-emption > code from exynos common code into the dp driver (since the bridge is > only applicable for dp). > > Signed-off-by: Sean Paul > --- > > Changes in v3: > - Added to the patchset > > drivers/gpu/drm/exynos/exynos_dp_core.c | 41 > > drivers/gpu/drm/exynos/exynos_drm_core.c | 41 > > 2 files changed, 41 insertions(+), 41 deletions(-) > Well, DRM bridge infrastructure is useful for more than just DP. Currently there are several platforms that could use it with DSI as well, e.g. Trats, Trats2, Arndale. With a simple abstraction worth of one or at most two days of work, we could get something that would cover much more than just a single Chromebook. Best regards, Tomasz
[PATCH v3 32/32] drm/exynos: Remove the exynos_drm_connector shim
Hi Sean, On Tuesday 29 of October 2013 12:13:18 Sean Paul wrote: > This path removes the exynos_drm_connector code since it was just > passing hooks through display_ops. The individual device drivers are now > responsible for implementing drm_connector directly. > > Signed-off-by: Sean Paul > --- > > Changes in v3: > - Added to the patchset > > drivers/gpu/drm/exynos/Makefile | 2 +- > drivers/gpu/drm/exynos/exynos_drm_connector.c | 258 > -- > drivers/gpu/drm/exynos/exynos_drm_connector.h | 20 -- > drivers/gpu/drm/exynos/exynos_drm_core.c | 18 +- > drivers/gpu/drm/exynos/exynos_drm_drv.h | 11 -- > drivers/gpu/drm/exynos/exynos_drm_encoder.c | 1 - > 6 files changed, 4 insertions(+), 306 deletions(-) > delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_connector.c > delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_connector.h Reviewed-by: Tomasz Figa Best regards, Tomasz
[PATCH v3 30/32] drm/exynos: Implement drm_connector directly in vidi driver
Hi Sean, On Tuesday 29 of October 2013 12:13:16 Sean Paul wrote: > This patch implements drm_connector directly in the vidi > driver, this will allow us to move away from the exynos_drm_connector > layer. > > Signed-off-by: Sean Paul > --- > > Changes in v3: > - Added to the patchset > > drivers/gpu/drm/exynos/exynos_drm_vidi.c | 163 > --- > 1 file changed, 107 insertions(+), 56 deletions(-) Reviewed-by: Tomasz Figa Best regards, Tomasz
[PATCH v3 29/32] drm/exynos: Implement drm_connector directly in dp driver
Hi Sean, On Tuesday 29 of October 2013 12:13:15 Sean Paul wrote: > This patch implements drm_connector directly in the dp driver, this will > allow us to move away from the exynos_drm_connector layer. > > Signed-off-by: Sean Paul > --- > > Changes in v3: > - Added to the patchset > > drivers/gpu/drm/exynos/exynos_dp_core.c | 99 > ++--- > drivers/gpu/drm/exynos/exynos_dp_core.h | 4 ++ > 2 files changed, 94 insertions(+), 9 deletions(-) Reviewed-by: Tomasz Figa Best regards, Tomasz
Fixing nouveau for >4k PAGE_SIZE
On Thu, 2013-08-29 at 16:49 +1000, Ben Skeggs wrote:
> > Additionally the current code is broken in that the upper layer in
> > vm/base.c doesn't increment "pte" by the right amount.
> >
> > Now, if those two assertions can be made always true:
> >
> > - Those two functions (map_sg and map_sg_table) never deal with the
> > "big" case.
> >
> > - An object is always mapped at a card address that is a multiple
> > of PAGE_SIZE (ie, invividual PAGE_SIZE pages don't cross pde boundaries
> > when mapped)
> I think these two restrictions are reasonable to enforce, and we should do so.
>
> >
> > Then we can probably simplify the code significantly *and* go back to
> > handling PAGE_SIZE pages in the vmm->map_sg() instead of breaking them
> > up a level above like I do.
> Sounds good!
Ok so I experimented with that approach a bit. The code could probably
use further simplifications and cleanups but it seems to work. Note that
I haven't been able to test much more than the fbdev and the DDX without
3d accel since GLX is currently broken on nouveau big endian for other
reasons (no visuals) since the Gallium BE rework by Ajax (plus the
nv30/40 merge also introduced artifacts and instabilities on BE which I
haven't tracked down either).
The basic idea here is that the backend vmm->map_sg operates on system
PAGE_SIZE, which allows to continue passing a page array down as we do
today.
That means however that only the nv04 and nv41 backends handle that case
right now, the other ones will have to be fixed eventually but the fix
is rather easy.
Now it's possible that I've missed cases where card objects might be
allocated in vram that isn't system PAGE_SIZE aligned, in which case we
will have breakage due to having a single system PAGE crossing a PDE
boundary, you'll have to help me here figure that out though I haven't
hit any of my WARN_ON's so far :-)
Patch isn't S-O-B yet, first let me know what you think of the approach
(and maybe check I didn't break x86 :-)
diff --git a/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
b/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
index ef3133e..44dc050 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
@@ -82,55 +82,77 @@ void
nouveau_vm_map_sg_table(struct nouveau_vma *vma, u64 delta, u64 length,
struct nouveau_mem *mem)
{
+ /*
+* XXX Should the "12" in a couple of places here be replaced
+* with vmm->spg_shift for correctness ? Might help if we ever
+* support 64k card pages on 64k PAGE_SIZE systems
+*/
struct nouveau_vm *vm = vma->vm;
struct nouveau_vmmgr *vmm = vm->vmm;
- int big = vma->node->type != vmm->spg_shift;
u32 offset = vma->node->offset + (delta >> 12);
- u32 bits = vma->node->type - 12;
- u32 num = length >> vma->node->type;
+ u32 shift = vma->node->type;
+ u32 order = PAGE_SHIFT - shift;
+ u32 num = length >> PAGE_SHIFT;
u32 pde = (offset >> vmm->pgt_bits) - vm->fpde;
- u32 pte = (offset & ((1 << vmm->pgt_bits) - 1)) >> bits;
- u32 max = 1 << (vmm->pgt_bits - bits);
- unsigned m, sglen;
- u32 end, len;
+ u32 pte = offset & ((1 << vmm->pgt_bits) - 1);
+ u32 max = 1 << vmm->pgt_bits;
+ u32 end, len, cardlen;
int i;
struct scatterlist *sg;
- for_each_sg(mem->sg->sgl, sg, mem->sg->nents, i) {
- struct nouveau_gpuobj *pgt = vm->pgt[pde].obj[big];
- sglen = sg_dma_len(sg) >> PAGE_SHIFT;
+ /* We don't handle "big" pages here */
+ if (WARN_ON(shift != vmm->spg_shift || shift > PAGE_SHIFT))
+ return;
- end = pte + sglen;
- if (unlikely(end >= max))
- end = max;
- len = end - pte;
+ /* We dont' handle objects that aren't PAGE_SIZE aligned either */
+ if (WARN_ON((offset << 12) & ~PAGE_MASK))
+ return;
- for (m = 0; m < len; m++) {
- dma_addr_t addr = sg_dma_address(sg) + (m <<
PAGE_SHIFT);
+ /* Iterate sglist elements */
+ for_each_sg(mem->sg->sgl, sg, mem->sg->nents, i) {
+ struct nouveau_gpuobj *pgt = vm->pgt[pde].obj[0];
+ unsigned long m, sglen;
+ dma_addr_t addr;
- vmm->map_sg(vma, pgt, mem, pte, 1, &addr);
- num--;
- pte++;
+ /* Number of system pages and base address */
+ sglen = sg_dma_len(sg) >> PAGE_SHIFT;
+ addr = sg_dma_address(sg);
+
+ /* Iterate over system pages in the segment and
+* covered PDEs
+*/
+ while(sglen) {
+ /* Number of card PTEs */
+ cardlen = sglen << order;
+ end = pte + cardlen;
+ if (unlikel
[PATCH v3 28/32] drm/exynos: Implement drm_connector in hdmi directly
Hi Sean, On Tuesday 29 of October 2013 12:13:14 Sean Paul wrote: > This patch implements drm_connector in the hdmi driver directly, instead > of using exynos_drm_connector. > > Signed-off-by: Sean Paul > --- > > Changes in v3: > - Added to the patchset > > drivers/gpu/drm/exynos/exynos_hdmi.c | 126 > +++ > 1 file changed, 85 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > b/drivers/gpu/drm/exynos/exynos_hdmi.c > index c6561fe..b063610 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -43,9 +43,8 @@ > #include > #include > > -#define MAX_WIDTH1920 > -#define MAX_HEIGHT 1080 Hmm, you are removing these values, but they don't seem to be redefined in any way anywhere below. Are you removing some of the video mode checks? > #define get_hdmi_display(dev) > platform_get_drvdata(to_platform_device(dev)) > +#define ctx_from_connector(c)container_of(c, struct hdmi_context, > connector) > > /* AVI header and aspect ratio */ > #define HDMI_AVI_VERSION 0x02 [snip] > @@ -811,11 +816,60 @@ static int hdmi_check_mode(struct exynos_drm_display > *display, > > ret = mixer_check_mode(mode); > if (ret) > - return ret; > + return MODE_BAD; Is there a need to define custom return values, instead of returning 0 or a standard error code depending on whether the mode is correct? Best regards, Tomasz
[RFC v2 PATCH] mipi-dsi-bus: add MIPI DSI bus support
On 11/27/2013 11:54 AM, Thierry Reding wrote:
> On Mon, Nov 25, 2013 at 04:05:41PM +0100, Andrzej Hajda wrote:
> [...]
+/**
+ * mipi_dsi_device_register - register a DSI device
+ * @dev: DSI device we're registering
+ */
+int mipi_dsi_device_register(struct mipi_dsi_device *dev,
+struct mipi_dsi_bus *bus)
+{
+ device_initialize(&dev->dev);
+
+ dev->bus = bus;
+ dev->dev.bus = &mipi_dsi_bus_type;
+ dev->dev.parent = bus->dev;
+ dev->dev.type = &mipi_dsi_dev_type;
+
+ if (dev->id != -1)
>>> Does that case actually make sense in the context of DSI? There's a
>>> defined hierarchy in DSI, so shouldn't we construct the names in a more
>>> hierarchical way? I'm not sure if the ID is useful at all, perhaps it
>>> should really be the virtual channel?
>>>
>>> The patch that I proposed used the following to make up the device name:
>>>
>>> dev_set_name(&dsi->dev, "%s.%u", dev_name(host->dev), dsi->channel);
>>>
>>> That has the advantage of reflecting the bus topology.
>>>
>>> It seems like your code was copied mostly from platform_device, for
>>> which there's no well-defined topology and therefore having an ID makes
>>> sense to differentiate between multiple instances of the same device.
>>> But for DSI any host/bus can only have a single device with a given
>>> channel, so that makes for a pretty good for unique name.
>> Code was based on mipi_dbi_bus.c from CDF.
>> I am not sure about hardwiring devices to virtual channels.
>> There could be devices which uses more than one virtual channel,
>> in fact exynos-drm docs suggests that such hardware exists.
> In that case, why not make them two logically separate devices within
> the kernel. We need the channel number so that the device can be
> addressed in the first place, so I don't see what's wrong with using
> that number in the device's name.
>
> The whole point of this patch is to add MIPI DSI bus infrastructure, and
> the virtual channel is one of the fundamental aspects of that bus, so I
> think we need to make it an integral part of the implementation.
There are already devices which IMO do not fit to this model, for example
TC358710 [1] can work as DSI hub, which I suppose uses two or three virtual
channels of the main DSI-host and performs "automatic virtual channel
translation".
[1]
http://www.toshiba.com/taec/components/ProdBrief/10F02_TC358710_ProdBrief.pdf
>
>> I can also imagine scenarios when dynamic virtual channel (re-)association
>> can be useful and DSI specs are not against it AFAIK.
> How is that going to work? There's no hotplug support or similar in DSI,
> so why would anyone want to reassign virtual channels.
>
> Supposing even that we wanted to support this eventually, I think a more
> appropriate solution would be to completely remove the device and add a
> new one, because that also takes care of keeping the channel number
> embedded within the struct mipi_dsi_device up to date.
>
>> reg property means device is at fixed virtual channel.
>> DSI specs says nothing about it IMHO.
> Without that fixed virtual channel number we can't use the device in the
> first place. How are you going to address the device if you don't know
> its virtual channel?
>
+ ssize_t (*transfer)(struct mipi_dsi_bus *bus,
+ struct mipi_dsi_device *dev,
+ struct mipi_dsi_msg *msg);
>>> Why do we need the dev parameter? There's already a channel field in the
>>> mipi_dsi_msg structure. Isn't that all that the transfer function needs
>>> to know about the device?
>> For simple HSI transfers, yes. In case transfer would depend on dev state
>> it would be useful, but I have no use case yet, so it could be removed.
> I don't know what an HSI transfer is. The transfer function is something
> that will be called by the peripheral's device driver, and that driver
> will know the state of the device, so I don't think the DSI host should
> care.
It should be HS, ie high-speed. But as I wrote before we can remove it
for now.
It can be added again in the future if I will have a real use case.
>
+struct mipi_dsi_device {
+ char name[MIPI_DSI_NAME_SIZE];
+ int id;
+ struct device dev;
+
+ const struct mipi_dsi_device_id *id_entry;
+ struct mipi_dsi_bus *bus;
+ struct videomode vm;
>>> Why is the videomode part of this structure? What if a device supports
>>> more than a single mode?
>> This is the current video mode. If we want to change mode,
>> we call:
>> ops->set_stream(false);
>> dev->vm =...new mode
>> ops->set_stream(true);
> I don't think that needs to be part of the DSI device. Rather it seems
> to me that whatever object type is implemented by the DSI device driver
> should have subsystem-specific code to do this.
>
> For example, if a DSI device is a bridge, then it will implement a
> drm_bridge object, and in that case the drm_bridge_funcs.mode_set()
> wou
[PATCH v3 26/32] drm/exynos: Consolidate suspend/resume in drm_drv
Hi Sean,
On Tuesday 29 of October 2013 12:13:12 Sean Paul wrote:
> This patch removes all of the suspend/resume logic from the individual
> drivers and consolidates it in drm_drv. This consolidation reduces the
> number of functions which enable/disable the hardware to just one -- the
> dpms callback. This ensures that we always power up/down in a consistent
> manner.
>
> Signed-off-by: Sean Paul
> ---
>
> Changes in v2:
> - Added to the patchset
> Changes in v3:
> - Made appropriate changes to vidi as well (removed pm_ops)
>
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 97 +
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 91 ---
> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 119
> +--
> drivers/gpu/drm/exynos/exynos_hdmi.c | 82 +
> drivers/gpu/drm/exynos/exynos_mixer.c| 75 ---
> 5 files changed, 176 insertions(+), 288 deletions(-)
[snip]
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index ba12916..208e013 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -741,6 +741,8 @@ static int fimd_poweron(struct exynos_drm_manager *mgr)
>
> ctx->suspended = false;
>
> + pm_runtime_get_sync(ctx->dev);
> +
> ret = clk_prepare_enable(ctx->bus_clk);
> if (ret < 0) {
> DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret);
> @@ -794,32 +796,24 @@ static int fimd_poweroff(struct exynos_drm_manager *mgr)
> clk_disable_unprepare(ctx->lcd_clk);
> clk_disable_unprepare(ctx->bus_clk);
>
> + pm_runtime_put_sync(ctx->dev);
> +
> ctx->suspended = true;
> return 0;
> }
[snip]
> @@ -950,8 +944,14 @@ static int fimd_probe(struct platform_device *pdev)
> fimd_manager.ctx = ctx;
> exynos_drm_manager_register(&fimd_manager);
>
> + /*
> + * We need to runtime pm to enable/disable sysmmu since it is a child of
> + * this driver. Ideally, this would hang off the drm driver's runtime
> + * operations, but we're not quite there yet.
You also need runtime PM to control state of power domains. I don't think
we should be going away of runtime PM API. Instead DPMS callbacks could
simply call pm_runtime_get_sync/put() whenever the hardware is supposed
to go up or down, just as done above in fimd_poweron/off(). This would
allow any platform-specific PM logic placed outside of DRM subsystem (such
as power domains and IOMMU) to operate.
> + *
> + * Tracked in crbug.com/264312
> + */
> pm_runtime_enable(dev);
> - pm_runtime_get_sync(dev);
>
> for (win = 0; win < WINDOWS_NR; win++)
> fimd_clear_win(ctx, win);
[snip]
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 2c127b9..c6561fe 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
[snip]
> @@ -2030,8 +2024,6 @@ static int hdmi_probe(struct platform_device *pdev)
> hdmi_display.ctx = hdata;
> exynos_drm_display_register(&hdmi_display);
>
> - pm_runtime_enable(dev);
> -
That's plain wrong. You need runtime PM to enable LCD power domain in
which the HDMI is placed.
> return 0;
>
> err_hdmiphy:
> @@ -2047,88 +2039,18 @@ static int hdmi_remove(struct platform_device *pdev)
> struct exynos_drm_display *display = get_hdmi_display(dev);
> struct hdmi_context *hdata = display->ctx;
>
> - pm_runtime_disable(dev);
> -
> put_device(&hdata->hdmiphy_port->dev);
> put_device(&hdata->ddc_port->dev);
>
> return 0;
> }
[snip]
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 39aed3e..985391d 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
[snip]
> @@ -1239,6 +1239,13 @@ static int mixer_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, &mixer_manager);
> exynos_drm_manager_register(&mixer_manager);
>
> + /*
> + * We need to runtime pm to enable/disable sysmmu since it is a child of
> + * this driver. Ideally, this would hang off the drm driver's runtime
> + * operations, but we're not quite there yet.
Same comment as for FIMD and HDMI.
Best regards,
Tomasz
[Bug 69723] GPU lockups with kernel 3.11.0 / 3.12-rc1 when dpm=1 on r600g (Cayman)
https://bugs.freedesktop.org/show_bug.cgi?id=69723 --- Comment #51 from Alexandre Demers --- (In reply to comment #50) > (In reply to comment #49) > > (In reply to comment #48) > > > The display went black a couple of time and according to dmesg/journal, > > > it was > > > related to texelFetch segfaulting in llvm [...] > > > > 'The display went black a couple of time' sounds like GPU resets after > > lockups, which are unlikely to be directly related to segfaulting piglit > > tests FWIW. > > Maybe, but I have no other indication. Also, I'm pretty that, when in auto > power level, when I run my piglit tests, it locks at there. > > Another thing I noticed: if I run the quick piglit tests at a low power > level, the total number of passed tests is higher than when I run it at an > higher speed. The failing tests explaining this difference are mostly > texelFetch related. I'll send the piglit results from both runs once I'll > have set back mclk to its default value. Oops, "... I'm pretty that..." -> "... I'm pretty sure that..." -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/7cab042b/attachment.html>
[Bug 69723] GPU lockups with kernel 3.11.0 / 3.12-rc1 when dpm=1 on r600g (Cayman)
https://bugs.freedesktop.org/show_bug.cgi?id=69723 --- Comment #50 from Alexandre Demers --- (In reply to comment #49) > (In reply to comment #48) > > The display went black a couple of time and according to dmesg/journal, it > > was > > related to texelFetch segfaulting in llvm [...] > > 'The display went black a couple of time' sounds like GPU resets after > lockups, which are unlikely to be directly related to segfaulting piglit > tests FWIW. Maybe, but I have no other indication. Also, I'm pretty that, when in auto power level, when I run my piglit tests, it locks at there. Another thing I noticed: if I run the quick piglit tests at a low power level, the total number of passed tests is higher than when I run it at an higher speed. The failing tests explaining this difference are mostly texelFetch related. I'll send the piglit results from both runs once I'll have set back mclk to its default value. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/23b18831/attachment.html>
[PATCH 00/14] drm: Some more vblank timestampi changes
On Wed, Nov 06, 2013 at 01:46:41PM +1000, Dave Airlie wrote: > On Wed, Oct 30, 2013 at 4:06 AM, wrote: > > So I took another look at the vblank timestamping code, and got a bit > > excited. The result is this patchset. > > I'd like to merge this, I was hoping Mario could ack it at least as it > seems mostly sane to my eyes. So we missed that boat, but maybe we'll get the next one... Pinging Mario. Any chance you can take a look at this stuff at some point? Hmm. Do I have the wrong email addres for Mario? Adding the other one too just to make sure... -- Ville Syrj?l? Intel OTC
[PATCH 3/3] drm/edid: parse the list of additional 3D modes
Parse 2D_VIC_order_X and 3D_Structure_X from the list at the end of the
HDMI Vendor Specific Data Block.
v2: Use an offset value depending on 3D_Multi_present and add
detail_present. (Ville Syrj?l?)
Signed-off-by: Thomas Wood
---
drivers/gpu/drm/drm_edid.c | 64 ++
1 file changed, 59 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f1d6e1e..9426563 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2734,7 +2734,7 @@ static int
do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
const u8 *video_db, u8 video_len)
{
- int modes = 0, offset = 0, i, multi_present = 0;
+ int modes = 0, offset = 0, i, multi_present = 0, multi_len;
u8 vic_len, hdmi_3d_len = 0;
u16 mask;
u16 structure_all;
@@ -2783,12 +2783,15 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
const u8 *db, u8 len,
if (!(multi_present == 1 || multi_present == 2))
goto out;
- if ((multi_present == 1 && len < (9 + offset)) ||
- (multi_present == 2 && len < (11 + offset)))
+ if (multi_present == 1)
+ multi_len = 2;
+ else
+ multi_len = 4;
+
+ if (len < (offset + hdmi_3d_len))
goto out;
- if ((multi_present == 1 && hdmi_3d_len < 2) ||
- (multi_present == 2 && hdmi_3d_len < 4))
+ if (hdmi_3d_len < multi_len)
goto out;
/* 3D_Structure_ALL */
@@ -2808,6 +2811,57 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
const u8 *db, u8 len,
video_len, i);
}
+ if (hdmi_3d_len <= multi_len)
+ goto out;
+
+ offset += multi_len;
+
+ for (i = 0; i < (hdmi_3d_len - multi_len); i++) {
+ int vic_index;
+ struct drm_display_mode *newmode = NULL;
+ unsigned int newflag = 0;
+ bool detail_present;
+
+ detail_present = ((db[8 + offset + i] & 0x0f) > 7);
+
+ if (detail_present && (i + 1 == hdmi_3d_len - multi_len))
+ break;
+
+ /* 2D_VIC_order_X */
+ vic_index = db[8 + offset + i] >> 4;
+
+ /* 3D_Structure_X */
+ switch (db[8 + offset + i] & 0x0f) {
+ case 0:
+ newflag = DRM_MODE_FLAG_3D_FRAME_PACKING;
+ break;
+ case 6:
+ newflag = DRM_MODE_FLAG_3D_TOP_AND_BOTTOM;
+ break;
+ case 8:
+ /* 3D_Detail_X */
+ if ((db[9 + offset + i] >> 4) == 1)
+ newflag = DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF;
+ break;
+ }
+
+ if (newflag != 0) {
+ newmode = drm_display_mode_from_vic_index(connector,
+ video_db,
+ video_len,
+ vic_index);
+
+ if (newmode) {
+ newmode->flags |= newflag;
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+ }
+
+ if (detail_present)
+ i++;
+ }
+
out:
return modes;
}
--
1.8.4.2
[PATCH 2/3] drm/edid: split VIC display mode lookup into a separate function
Signed-off-by: Thomas Wood
---
drivers/gpu/drm/drm_edid.c | 67 +++---
1 file changed, 39 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0a1e4a5..f1d6e1e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2557,25 +2557,40 @@ add_alternate_cea_modes(struct drm_connector
*connector, struct edid *edid)
return modes;
}
-static int
-do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
+static struct drm_display_mode *
+drm_display_mode_from_vic_index(struct drm_connector *connector,
+ const u8 *video_db, u8 video_len,
+ u8 video_index)
{
struct drm_device *dev = connector->dev;
- const u8 *mode;
+ struct drm_display_mode *newmode;
u8 cea_mode;
- int modes = 0;
- for (mode = db; mode < db + len; mode++) {
- cea_mode = (*mode & 127) - 1; /* CEA modes are numbered 1..127
*/
- if (cea_mode < ARRAY_SIZE(edid_cea_modes)) {
- struct drm_display_mode *newmode;
- newmode = drm_mode_duplicate(dev,
-&edid_cea_modes[cea_mode]);
- if (newmode) {
- newmode->vrefresh = 0;
- drm_mode_probed_add(connector, newmode);
- modes++;
- }
+ if (video_db == NULL || video_index >= video_len)
+ return NULL;
+
+ /* CEA modes are numbered 1..127 */
+ cea_mode = (video_db[video_index] & 127) - 1;
+ if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
+ return NULL;
+
+ newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
+ newmode->vrefresh = 0;
+
+ return newmode;
+}
+
+static int
+do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
+{
+ int i, modes = 0;
+
+ for (i = 0; i < len; i++) {
+ struct drm_display_mode *mode;
+ mode = drm_display_mode_from_vic_index(connector, db, len, i);
+ if (mode) {
+ drm_mode_probed_add(connector, mode);
+ modes++;
}
}
@@ -2669,21 +2684,13 @@ static int add_hdmi_mode(struct drm_connector
*connector, u8 vic)
static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
const u8 *video_db, u8 video_len, u8 video_index)
{
- struct drm_device *dev = connector->dev;
struct drm_display_mode *newmode;
int modes = 0;
- u8 cea_mode;
-
- if (video_db == NULL || video_index >= video_len)
- return 0;
-
- /* CEA modes are numbered 1..127 */
- cea_mode = (video_db[video_index] & 127) - 1;
- if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
- return 0;
if (structure & (1 << 0)) {
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
+ newmode = drm_display_mode_from_vic_index(connector, video_db,
+ video_len,
+ video_index);
if (newmode) {
newmode->flags |= DRM_MODE_FLAG_3D_FRAME_PACKING;
drm_mode_probed_add(connector, newmode);
@@ -2691,7 +2698,9 @@ static int add_3d_struct_modes(struct drm_connector
*connector, u16 structure,
}
}
if (structure & (1 << 6)) {
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
+ newmode = drm_display_mode_from_vic_index(connector, video_db,
+ video_len,
+ video_index);
if (newmode) {
newmode->flags |= DRM_MODE_FLAG_3D_TOP_AND_BOTTOM;
drm_mode_probed_add(connector, newmode);
@@ -2699,7 +2708,9 @@ static int add_3d_struct_modes(struct drm_connector
*connector, u16 structure,
}
}
if (structure & (1 << 8)) {
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
+ newmode = drm_display_mode_from_vic_index(connector, video_db,
+ video_len,
+ video_index);
if (newmode) {
newmode->flags |= DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF;
drm_mode_probed_add(connector, newmode);
--
1.8.4.2
[PATCH 1/3] drm/edid: fix length check when adding extra 3D modes
Signed-off-by: Thomas Wood --- drivers/gpu/drm/drm_edid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 52e060e..0a1e4a5 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2674,7 +2674,7 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, int modes = 0; u8 cea_mode; - if (video_db == NULL || video_index > video_len) + if (video_db == NULL || video_index >= video_len) return 0; /* CEA modes are numbered 1..127 */ -- 1.8.4.2
Parse the list of additional 3D modes (v2)
Here's an updated series of patches taking into account Ville's review. Regards, Thomas
[PATCH 10/31] ARM: tegra: pass reset to tegra_powergate_sequence_power_up()
On Fri, Nov 29, 2013 at 02:45:33PM +0100, Thierry Reding wrote: > On Fri, Nov 15, 2013 at 01:54:05PM -0700, Stephen Warren wrote: > > From: Stephen Warren > > > > Tegra's clock driver now provides an implementation of the common > > reset API (include/linux/reset.h). Use this instead of the old Tegra- > > specific API; that will soon be removed. > > Ah, there it is! > > Reviewed-by: Thierry Reding And: Acked-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/7a886112/attachment.pgp>
[PATCH 10/31] ARM: tegra: pass reset to tegra_powergate_sequence_power_up()
On Fri, Nov 15, 2013 at 01:54:05PM -0700, Stephen Warren wrote: > From: Stephen Warren > > Tegra's clock driver now provides an implementation of the common > reset API (include/linux/reset.h). Use this instead of the old Tegra- > specific API; that will soon be removed. Ah, there it is! Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/2d85e249/attachment.pgp>
[PATCH 09/31] drm/tegra: use reset framework
On Fri, Nov 15, 2013 at 01:54:04PM -0700, Stephen Warren wrote: > From: Stephen Warren > > Tegra's clock driver now provides an implementation of the common > reset API (include/linux/reset.h). Use this instead of the old Tegra- > specific API; that will soon be removed. > > Cc: treding at nvidia.com > Cc: pdeschrijver at nvidia.com > Cc: linux-tegra at vger.kernel.org > Cc: linux-arm-kernel at lists.infradead.org > Cc: Terje Bergstr?m > Cc: David Airlie > Cc: dri-devel at lists.freedesktop.org > Signed-off-by: Stephen Warren > --- > This patch is part of a series with strong internal depdendencies. I'm > looking for an ack so that I can take the entire series through the Tegra > and arm-soc trees. The series will be part of a stable branch that can be > merged into other subsystems if needed to avoid/resolve dependencies. > --- > drivers/gpu/drm/tegra/Kconfig | 1 + > drivers/gpu/drm/tegra/dc.c| 9 - > drivers/gpu/drm/tegra/drm.h | 3 +++ > drivers/gpu/drm/tegra/gr3d.c | 16 > drivers/gpu/drm/tegra/hdmi.c | 14 +++--- > 5 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig > index 8961ba6a34b8..8db9b3bce001 100644 > --- a/drivers/gpu/drm/tegra/Kconfig > +++ b/drivers/gpu/drm/tegra/Kconfig > @@ -2,6 +2,7 @@ config DRM_TEGRA > bool "NVIDIA Tegra DRM" > depends on ARCH_TEGRA || ARCH_MULTIPLATFORM > depends on DRM > + depends on RESET_CONTROLLER Is this really needed? ARCH_TEGRA already selects RESET_CONTROLLER and we depend on ARCH_TEGRA. Or perhaps you need this because it might also be that ARCH_MULTIPLATFORM is selected without ARCH_TEGRA support? In either case I guess a good thing would be to add dummies for the reset API so that we don't have this additional compile-time dependency. > select TEGRA_HOST1X > select DRM_KMS_HELPER > select DRM_KMS_FB_HELPER > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c > index ae1cb31ead7e..c3be92879bea 100644 > --- a/drivers/gpu/drm/tegra/dc.c > +++ b/drivers/gpu/drm/tegra/dc.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include You should be able to drop linux/clk/tegra.h now. > diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c > index 4cec8f526af7..f629e38b00e4 100644 > --- a/drivers/gpu/drm/tegra/gr3d.c > +++ b/drivers/gpu/drm/tegra/gr3d.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include I was going to say "Same here", but interestingly this driver doesn't use the old Tegra-specific reset API. It's actually handled internally by the tegra_powergate_sequence_power_up() function and I think that'll be handled by a separate patch in your series if I recall correctly. > diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c > index 0cd9bc2056e8..f3aad49633d6 100644 > --- a/drivers/gpu/drm/tegra/hdmi.c > +++ b/drivers/gpu/drm/tegra/hdmi.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include But here the linux/clk/tegra.h include can again be dropped. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/f826b445/attachment.pgp>
[Intel-gfx] [PATCH 13/19] drm: do not steal the display if we have a master
On Wed, Nov 27, 2013 at 06:24:08PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni
>
> Sometimes we want to disable all the screens on a system, because that
> will allow the graphics card to be put into low-power states. The
> problem is that, for example, while all screens are disabled, if we
> get a hotplug interrupt, fbcon will decide to set a mode instead of
> keeping everything disabled, which will remove us from our low power
> states.
>
> Let's assume that if there's a DRM master, it will be able to do
> whatever is appropriate when we get the hotplug.
>
> This problem can be reproduced by the runtime PM test program from
> intel-gpu-tools: we disable all the screens so the graphics device can
> be put into D3, then something triggers a hotplug interrupt, fbcon
> sets a mode and breaks our test suite. The problem can be reproduced
> more easily by the "i2c" subtest.
>
> Other approaches considered for the problem:
> - Return "false" if "bound == 0" and the caller of
> drm_fb_helper_is_bound is a hotplug handler. This would break
> the case where the machine boots with no outputs connected, then
> the user plugs a monitor.
> - Add a new IOCTL to force fbcon to not set modes. This would keep
> all the current applications behaving the same, but adding a new
> IOCTL is not always the greatest idea.
> - Return false only if "dev->primary->master && bound == 0". This
> was my first implementation, but Chris suggested we should do
> the check irrespective of the "bound" variable.
>
> Thanks to Daniel Vetter for the investigation, ideas and the
> implementation of the hotplug alternative.
>
> v2: - Do the check first, irrespective of "bound".
> - Cc dri-devel
>
> Cc: dri-devel at lists.freedesktop.org
> Credits-to: Daniel Vetter
> Signed-off-by: Paulo Zanoni
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_helper.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0a19401..98a0363 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -359,6 +359,11 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper
> *fb_helper)
> struct drm_crtc *crtc;
> int bound = 0, crtcs_bound = 0;
>
> + /* Sometimes user space wants everything disabled, so don't steal the
> + * display if there's a master. */
> + if (dev->primary->master)
> + return false;
> +
> list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> if (crtc->fb)
> crtcs_bound++;
> @@ -368,6 +373,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper
> *fb_helper)
>
> if (bound < crtcs_bound)
> return false;
> +
> return true;
> }
>
> --
> 1.8.3.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH v3 12/32] drm/exynos: Split manager/display/subdrv
On Fri, Nov 29, 2013 at 12:05 PM, Tomasz Figa wrote: > On Friday 29 of November 2013 09:13:19 Rob Clark wrote: >> On Fri, Nov 29, 2013 at 4:10 AM, Tomasz Figa >> wrote: >> > I would mostly agree with you if we were discussing SoC-internal >> > components here. Mostly, because the ARM world is more complex and you >> > can see the same IP across completely different SoCs from different >> > vendors. >> > >> > However, the topic here is about external devices, outside the SoC, such >> > as different kind of bridges, like the PTN3460 eDP to LVDS bridge, which >> > are likely to be reused across different platforms. Similar thing is with >> > using different bridges on different boards using the same SoC platform. >> > I don't think having an abstraction here would be any overabstraction at >> > all. Anything less would be a huge "underabstraction" in fact. >> >> >> I think no one is arguing that we don't eventually need some better >> abstraction. But as long as it is one-bridge and one-user, if the >> patches otherwise have merit, add functionality that was missing >> before and don't regress, then lack of infrastructure to match up >> bridge and driver isn't something I will care about too much yet. >> Things are allowed to be in-progress. A missing abstraction for a 1:1 >> relationship is fine. > > This is not just one-bridge, one-user. This is about users of Exynos DRM > we already have in-tree, such as Trats, Trats2 or Arndale, that the DRM > bridge infrastructure could be used on and finally allowing to have > display support on them. Of course you could merge this as is and > then let someone else completely rewrite it (most likely in the same > release cycle), but since it's not really much more work, I don't > think there is any sense. well, I'm not quite sure where I there is a pending complete re-write.. it looks like the hard ptn3460 dependency is just a few lines in one function. Otherwise I'd agree with you. (and even the patch that touches the code calling ptn3460_init() is just moving it around from what I see) > Moreover, let's stick to modern kernel driver coding standards. I don't > think that "I want this patchset merged so badly" is really a good excuse > to get around it. After all, there would be no GKH's staging tree, if > nobody cared about quality in mainline. And with my quality hat on, I could say that I'm not too fond of unused (or 1:1 client to user) abstractions. That is something that should be introduced as we merge our 2nd or 3rd bridge. BR, -R > Best regards, > Tomasz >
[PATCH v3 12/32] drm/exynos: Split manager/display/subdrv
On Fri, Nov 29, 2013 at 10:10 AM, Tomasz Figa wrote: > I would mostly agree with you if we were discussing SoC-internal > components here. Mostly, because the ARM world is more complex and you > can see the same IP across completely different SoCs from different > vendors. Yeah, hence the restriction to IP blocks from the same vendor. Heck we have external IP blocks in our gpus, too. It's just that the driver to make that one work is in no shape for upstreaming :( So even for that case of smashing a random decode engine into your gpu we've seen this on intel. And I'd guess the radeon uvd block is a similarly alien piece (although at least not shared anywhere else iirc). > However, the topic here is about external devices, outside the SoC, such > as different kind of bridges, like the PTN3460 eDP to LVDS bridge, which > are likely to be reused across different platforms. Similar thing is with > using different bridges on different boards using the same SoC platform. > I don't think having an abstraction here would be any overabstraction at > all. Anything less would be a huge "underabstraction" in fact. Hm, I've thought Sean's rework was mostly about the driver core and not so much about off-chip blocks. But I admit to not have read through all the details of this thread, so please forgive me my ignorance ;-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH v3 13/32] drm/exynos: hdmi: remove the i2c drivers and use devtree
On Thu, Nov 28, 2013 at 02:30:24PM +0100, Tomasz Figa wrote:
> On Monday 11 of November 2013 09:44:27 Thierry Reding wrote:
> > On Sun, Nov 10, 2013 at 09:46:02PM +0100, Tomasz Figa wrote:
> > [...]
> > > On Tuesday 29 of October 2013 12:12:59 Sean Paul wrote:
> > [...]
> > > [snip]
> > > > @@ -1957,21 +1943,30 @@ static int hdmi_probe(struct platform_device
> > > > *pdev)
> > > > }
> > > >
> > > > /* DDC i2c driver */
> > > > - if (i2c_add_driver(&ddc_driver)) {
> > > > - DRM_ERROR("failed to register ddc i2c driver\n");
> > > > - return -ENOENT;
> > > > + ddc_node = of_find_node_by_name(NULL, "hdmiddc");
> > >
> > > This is wrong. You shall not reference a device tree node by its name,
> > > except some very specific well-defined cases, such as cpus or memory
> > > nodes.
> > >
> > > A solution closest to yours, but correct, would be to use the same match
> > > table as in the I2C driver you are removing and call
> > > of_find_matching_node().
> >
> > Isn't the correct solution to use a phandle? That might need the binding
> > to change in a backwards incompatible way.
>
> Yes, phandle is an even better option as it can point you precisely to the
> node you are interested in, but this will be incompatible, meaning that
> you would have to support both variants anyway.
Oh come on. If a phandle is the right way to do it, then we should just
do it. Will it really be so difficult to carry code for both variants?
If nothing else it will at least set a good example and reduce the risk
of people doing the same mistakes over and over again.
Adding the right binding also gives you a way to start deprecating the
wrong one and eventually remove it. The longer you wait, the more people
will start to use the existing, broken binding and removing it will only
become more difficult over time.
> > Then again, if something as
> > simple as specifying a DDC I2C bus causes the binding to change in a
> > backwards incompatible way then it can't have been a very good binding
> > in the first place, right? +1 for unstable DT bindings...
>
> Well, some of already existing bindings should have been definitely marked
> unstable, as they haven't been thought and reviewed well enough, if at all
> (especially reviewed, as we only started seriously reviewing DT bindings
> not so long ago).
>
> Honestly, I'm not quite sure about this binding in particular, especially
> how much it would be a problem if we broke compatibility. I mean, how much
> tied to old DTBs are existing boards using this binding. The affected
> boards are:
> - exynos5250-snow,
> - exynos5250-arndale,
> - exynos5250-smdk5250,
> - exynos5420-smdk5420.
> The last three are most likely to be used only with DTB appended, so
> I don't think that anyone would complain. However I'm not sure about the
> first one, which is supposed to be a Chromebook if I'm not mistaken.
Well, if it's a Chromebook it likely doesn't ship with a completely
mainline kernel. That frees it from the stability requirements, doesn't
it?
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/42198eb9/attachment.pgp>
[PATCH v3 12/32] drm/exynos: Split manager/display/subdrv
On Fri, Nov 29, 2013 at 12:04:04AM +0100, Tomasz Figa wrote: > On Tuesday 26 of November 2013 10:00:13 Olof Johansson wrote: > > On Tue, Nov 12, 2013 at 10:35 AM, Tomasz Figa > > wrote: [...] > > > Sorry, this might have been a bit too harsh, but just imagine myself doing > > > my regular patch review round, hoping (as usual) to see only code that is > > > not less than great, while suddenly stumbling upon a line of code that > > > I would expect at most in some vendor tree or maybe several years ago in > > > sources of a 2.4 kernel. > > > > More like code written in the same style as x86 DRM stuff, where > > they're not used to overabstracting things from day one to make things > > generic instead of supporting the only known chip combination to date. > > No, not really. They don't have any modularity on x86. Just a graphics > card with everything on it, so they can freely do such things, as they > don't have to account for all we need to account for on ARM platforms. I don't think there's such a clear difference. A particular GPU on x86 is equally modular as a specific GPU on an ARM SoC. The problem is that on x86 there's much less mix-and-matching going on. So theoretically they do need to account for the same craziness. That's not done, however, because there's no need for it. It doesn't happen in practice. > Also, I wouldn't call making a driver usable and useful for more than > just one fixed platform "overabstracting"... It is if there's never more than the single fixed platform. Since none of us can predict the future I think it's entirely reasonable if we do concentrate on solving the problems that we have now. It doesn't mean that we should write code in a way that makes future enhancements unnecessarily difficult, but I very much prefer to see code merged that supports one specific use-case that we have now rather than working on the code for years on end in an attempt to make it generic enough to support everything that the future can possibly hold. Chances are pretty good that we won't be able to predict one specific use-case and then rewrite everything anyway. Linux has been pretty successful so far in a large part because it has evolved organically. We're not doing ourselves any good by requiring everything to be perfect from the beginning. We all know what a disaster DT has been and still is. It makes it very difficult to get new features supported because we now have a component that we can no longer change at will and that cannot evolve organically anymore. That impedes progress. For this particular issue no DT is involved. There's no need for ABI stability because it's just in-tree code and we can change it to our heart's content. We can refactor things when an actual need arises. By then chances are pretty good we'll have a much better understanding of what exactly we need and therefore we can come up with a much better solution than at this point in time. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/f97dfc52/attachment.pgp>
i915: pipe state still does not match
On Thu, Nov 28, 2013 at 09:08:45PM +0100, Jan Engelhardt wrote: > On Wednesday 2013-11-27 12:08, Chris Wilson wrote: > >On Wed, Nov 27, 2013 at 11:59:56AM +0100, Jan Engelhardt wrote: > >> > >> Despite the i915/drm fixes added in v3.11.8, the X server still > >> terminates due to some pipe state bug in 3.11.9. > > > >X terminating is entirely unconnected with that *ERROR*. > > Are you sure? Whenever X crashed, that inteldrv kernel message > was showing up in dmesg. > Affected versions: > xorg-x11-server-1.14.3.901 > xf86-video-intel-2.99.906-4.1.x86_64 > Working versions: > xorg-x11-server-1.13.2 > xf86-video-intel-2.20.19 > > >Can you please attach your Xorg.0.log with the crash information? > > What I could collect so far: Thanks, I broke the handling of cropped XvImages along the fast paths. It should be fixed by: commit fd007d9d465b9b3ddbbaf769931ec921a6f5ecb8 Author: Chris Wilson Date: Thu Nov 28 21:13:33 2013 + sna/video: Correct handling of cropped images along packed fast path In particular, it was offseting the read from the source image, but not correcting the length to read - causing a read from beyond the end of the source and a segfault. Reported-by: Jan Engelhardt Signed-off-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH v3 12/32] drm/exynos: Split manager/display/subdrv
On Friday 29 of November 2013 08:52:22 Daniel Vetter wrote: > On Fri, Nov 29, 2013 at 12:04:04AM +0100, Tomasz Figa wrote: > > On Tuesday 26 of November 2013 10:00:13 Olof Johansson wrote: > > > More like code written in the same style as x86 DRM stuff, where > > > they're not used to overabstracting things from day one to make things > > > generic instead of supporting the only known chip combination to date. > > > > No, not really. They don't have any modularity on x86. Just a graphics > > card with everything on it, so they can freely do such things, as they > > don't have to account for all we need to account for on ARM platforms. > > I guess you didn't bother looking at x86 drivers then ... in i915 we have > different blocks spanning 7 major hw iterations, reused in funny (and > overlapping) ways. We have clock trees, nested power domains, interrupt > controllers and forwarders, different coherency domains, off-chip > functiosn in the PCH, the full shebang. > > In the code itself we have piles of vtables for the different parts of the > chip, mmio base offsets (since hw engineers just love to move things > around) and otherwise neatly abstracted helpers for different parts of the > chip to be able to reuse things across generations. > > The _only_ difference I see compared to an arm soc is that this entire > madness is neatly wrapped up into a fake pci device. And some of the > really remote mmio ranges are windowed into general hodgepodge mmio > register bar so that we don't have to hunt it down on different pci > devices. But occasionally we do even that. > > So as per the discussion at KS about soc gfx drivers where all the ip > blocks come from the same vendor I really don't see why arm drivers need > to be so much different than more traditional x86 drivers. Since nowadays > (at least with intel) that means we _are_ talking about an soc. I would mostly agree with you if we were discussing SoC-internal components here. Mostly, because the ARM world is more complex and you can see the same IP across completely different SoCs from different vendors. However, the topic here is about external devices, outside the SoC, such as different kind of bridges, like the PTN3460 eDP to LVDS bridge, which are likely to be reused across different platforms. Similar thing is with using different bridges on different boards using the same SoC platform. I don't think having an abstraction here would be any overabstraction at all. Anything less would be a huge "underabstraction" in fact. Best regards, Tomasz
[PATCH v3 12/32] drm/exynos: Split manager/display/subdrv
On Fri, Nov 29, 2013 at 4:10 AM, Tomasz Figa wrote: > I would mostly agree with you if we were discussing SoC-internal > components here. Mostly, because the ARM world is more complex and you > can see the same IP across completely different SoCs from different > vendors. > > However, the topic here is about external devices, outside the SoC, such > as different kind of bridges, like the PTN3460 eDP to LVDS bridge, which > are likely to be reused across different platforms. Similar thing is with > using different bridges on different boards using the same SoC platform. > I don't think having an abstraction here would be any overabstraction at > all. Anything less would be a huge "underabstraction" in fact. I think no one is arguing that we don't eventually need some better abstraction. But as long as it is one-bridge and one-user, if the patches otherwise have merit, add functionality that was missing before and don't regress, then lack of infrastructure to match up bridge and driver isn't something I will care about too much yet. Things are allowed to be in-progress. A missing abstraction for a 1:1 relationship is fine. Now as we start getting a few more bridge devices and users, then I'll start blocking patches until some sort of generic bridge loader is sorted out. BR, -R
i915: pipe state still does not match
On Thu, Nov 28, 2013 at 09:08:45PM +0100, Jan Engelhardt wrote: > On Wednesday 2013-11-27 12:08, Chris Wilson wrote: > >On Wed, Nov 27, 2013 at 11:59:56AM +0100, Jan Engelhardt wrote: > >> > >> Despite the i915/drm fixes added in v3.11.8, the X server still > >> terminates due to some pipe state bug in 3.11.9. > > > >X terminating is entirely unconnected with that *ERROR*. > > Are you sure? Whenever X crashed, that inteldrv kernel message > was showing up in dmesg. > Affected versions: > xorg-x11-server-1.14.3.901 > xf86-video-intel-2.99.906-4.1.x86_64 > Working versions: > xorg-x11-server-1.13.2 > xf86-video-intel-2.20.19 For the ERROR the kernel is the important part. I can be that it's triggered by userspace doing something funny (and likely, sinc there's a good chance you're hitting a less well tested path when crashing). But it's a kernel issue. To track the kernel issue down can you please boot with drm.debug=0xe added to your bootline, reproduce the issue and then attach the complete dmesg? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH v3 12/32] drm/exynos: Split manager/display/subdrv
On Fri, Nov 29, 2013 at 12:04:04AM +0100, Tomasz Figa wrote: > On Tuesday 26 of November 2013 10:00:13 Olof Johansson wrote: > > More like code written in the same style as x86 DRM stuff, where > > they're not used to overabstracting things from day one to make things > > generic instead of supporting the only known chip combination to date. > > No, not really. They don't have any modularity on x86. Just a graphics > card with everything on it, so they can freely do such things, as they > don't have to account for all we need to account for on ARM platforms. I guess you didn't bother looking at x86 drivers then ... in i915 we have different blocks spanning 7 major hw iterations, reused in funny (and overlapping) ways. We have clock trees, nested power domains, interrupt controllers and forwarders, different coherency domains, off-chip functiosn in the PCH, the full shebang. In the code itself we have piles of vtables for the different parts of the chip, mmio base offsets (since hw engineers just love to move things around) and otherwise neatly abstracted helpers for different parts of the chip to be able to reuse things across generations. The _only_ difference I see compared to an arm soc is that this entire madness is neatly wrapped up into a fake pci device. And some of the really remote mmio ranges are windowed into general hodgepodge mmio register bar so that we don't have to hunt it down on different pci devices. But occasionally we do even that. So as per the discussion at KS about soc gfx drivers where all the ip blocks come from the same vendor I really don't see why arm drivers need to be so much different than more traditional x86 drivers. Since nowadays (at least with intel) that means we _are_ talking about an soc. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 71930] Kernel Bug and X fails to start when using radeon.runpm=1
https://bugs.freedesktop.org/show_bug.cgi?id=71930 --- Comment #14 from sotiris papadimitriou --- (In reply to comment #13) > Same problem. > My graphics cards: > lspci | grep VGA > 01:05.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI > RS880M [Mobility Radeon HD 4225/4250] > 02:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI Park > [Mobility Radeon HD 5430/5450/5470] > If radeon.runpm=0 all o.k. > If manually turning off the dGPU using switcheroo then freeze > > I'm use Ubuntu 14.04 with kernel 3.13.rc1 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/d75c4009/attachment.html>
[Bug 71930] Kernel Bug and X fails to start when using radeon.runpm=1
https://bugs.freedesktop.org/show_bug.cgi?id=71930 --- Comment #13 from sotiris papadimitriou --- Same problem. My graphics cards: lspci | grep VGA 01:05.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI RS880M [Mobility Radeon HD 4225/4250] 02:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI Park [Mobility Radeon HD 5430/5450/5470] If radeon.runpm=0 all o.k. If manually turning off the dGPU using switcheroo then freeze Use Ubuntu 14.04 with 13.rc1 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/3e4f1b74/attachment.html>
[Bug 65911] radeon: garbled output/only noise through HDMI and GPU lockups
https://bugzilla.kernel.org/show_bug.cgi?id=65911 --- Comment #10 from tomka --- Oops, I figured the last dmesg log lines were truncated. Here they are again: [ 170.607701] radeon :00:01.0: GPU lockup CP stall for more than 1msec [ 170.607711] radeon :00:01.0: GPU lockup (waiting for 0x0007 last fence id 0x0002) [ 170.607717] [drm:r600_ib_test] *ERROR* radeon: fence wait failed (-35). [ 170.607723] [drm:radeon_ib_ring_tests] *ERROR* radeon: failed testing IB on GFX ring (-35). [ 170.607727] radeon :00:01.0: ib ring test failed (-35). -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 35457] [rs690m] Graphics corruption with ati x1200
https://bugs.freedesktop.org/show_bug.cgi?id=35457 --- Comment #77 from lct at mail.ru --- Hello Mr. Deucher, hello d4ddi0; sorry for not responding, unfortunately I can only test it on this weekend, sorry for polluting maillist with my ego status. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/3cd6bffe/attachment.html>
[Bug 65121] Possible memory leak in qxl drm driver
https://bugzilla.kernel.org/show_bug.cgi?id=65121 --- Comment #7 from Matthew Stapleton --- It looks like that fixes it. Thanks. Also, I'm only running on console, X isn't running. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 35457] [rs690m] Graphics corruption with ati x1200
https://bugs.freedesktop.org/show_bug.cgi?id=35457 --- Comment #76 from d4ddi0 --- Oh, wow! patch in attachment 89886 seems to fix this issue for me. Today is thanksgiving here in the USA. I am thankful for open source radeon drivers. I am thankful for your efforts, Alex Deucher. This would confirm that it is a bogus BIOS setting where the bios reports 384, but it is actually only 256. I tried something similar on my own earlier, but I did not understand the memory layout well enough to know to adjust the base address. Thank you for trying yet again to fix this, Alex. I am testing now. Anyone else on this bug able to test this proposed fix? On Tue, Nov 26, 2013 at 9:03 PM, wrote: > *Comment # 71 <https://bugs.freedesktop.org/show_bug.cgi?id=35457#c71> > on bug 35457 <https://bugs.freedesktop.org/show_bug.cgi?id=35457> from Alex > Deucher * > > Created attachment 89886 > <https://bugs.freedesktop.org/attachment.cgi?id=89886> [details] > <https://bugs.freedesktop.org/attachment.cgi?id=89886&action=edit> [review] > <https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=35457&attachment=89886> > possible fix > > Does this patch help? > > -- > You are receiving this mail because: > >- You are on the CC list for the bug. > > -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/f4392a44/attachment.html>
[Bug 69723] GPU lockups with kernel 3.11.0 / 3.12-rc1 when dpm=1 on r600g (Cayman)
https://bugs.freedesktop.org/show_bug.cgi?id=69723 --- Comment #49 from Michel D?nzer --- (In reply to comment #48) > The display went black a couple of time and according to dmesg/journal, it was > related to texelFetch segfaulting in llvm [...] 'The display went black a couple of time' sounds like GPU resets after lockups, which are unlikely to be directly related to segfaulting piglit tests FWIW. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/a6b252d9/attachment.html>
[Bug 69775] [r600g] RV730 AGP UVD hang (GPU lockup) with mplayer on dual DVI display with 3.12-rc2
https://bugs.freedesktop.org/show_bug.cgi?id=69775 Dieter N?tzel changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |WORKSFORME --- Comment #3 from Dieter N?tzel --- Retested with Kernel 3.13-rc1 + drm-radeon-fix-typo-in-fetching-mpll-params.patch radeon.agpmode=8 (dpm is on per default) Mesa 10.1.0-devel (git-fb5f5b8) Works, now. Good job! -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/693ee7be/attachment.html>
[Bug 69723] GPU lockups with kernel 3.11.0 / 3.12-rc1 when dpm=1 on r600g (Cayman)
https://bugs.freedesktop.org/show_bug.cgi?id=69723 --- Comment #48 from Alexandre Demers --- Went to 122500, still no total hang. Ran quick.test (piglit) while playing a movie. The display went black a couple of time and according to dmesg/journal, it was related to texelFetch segfaulting in llvm (another bug I've reported). So, for now, we can say it is still stable at this frequency. I'll push it a bit more. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131129/b2c1331f/attachment.html>
[PATCH v3 12/32] drm/exynos: Split manager/display/subdrv
On Tuesday 26 of November 2013 10:00:13 Olof Johansson wrote: > On Tue, Nov 12, 2013 at 10:35 AM, Tomasz Figa > wrote: > > On Tuesday 12 of November 2013 12:51:11 Sean Paul wrote: > >> On Sun, Nov 10, 2013 at 4:09 PM, Tomasz Figa > >> wrote: > >> > Hi Sean, > >> > > >> > On Tuesday 29 of October 2013 12:12:58 Sean Paul wrote: > >> >> This patch splits display and manager from subdrv. The result is that > >> >> crtc functions can directly call into manager callbacks and encoder > >> >> functions can directly call into display callbacks. This will allow > >> >> us to remove the exynos_drm_hdmi shim and support mixer/hdmi & fimd/dp > >> >> with common code. > >> >> > >> >> Signed-off-by: Sean Paul > >> >> --- > >> >> > >> >> Changes in v2: > >> >> - Pass display into display_ops instead of context > >> >> Changes in v3: > >> >> - Changed vidi args to exynos_drm_display instead of void > >> >> - Moved exynos_drm_hdmi.c removal into next patch > >> >> > >> >> drivers/gpu/drm/exynos/exynos_drm_connector.c | 50 ++--- > >> >> drivers/gpu/drm/exynos/exynos_drm_core.c | 181 +- > >> >> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 115 +--- > >> >> drivers/gpu/drm/exynos/exynos_drm_crtc.h | 20 +- > >> >> drivers/gpu/drm/exynos/exynos_drm_drv.c | 29 +-- > >> >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 106 +++ > >> >> drivers/gpu/drm/exynos/exynos_drm_encoder.c | 258 > >> >> -- > >> >> drivers/gpu/drm/exynos/exynos_drm_encoder.h | 18 +- > >> >> drivers/gpu/drm/exynos/exynos_drm_fb.c| 4 +- > >> >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 161 > >> >> drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 211 > >> >> + > >> >> drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 2 + > >> >> drivers/gpu/drm/exynos/exynos_drm_plane.c | 15 +- > >> >> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 129 ++--- > >> >> 14 files changed, 615 insertions(+), 684 deletions(-) > >> > > >> > This patch is really heavy, making it very hard to review. I wonder if it > >> > couldn't really be split into several smaller patches... > >> > > >> > >> I've distilled it down as much as possible. Unfortunately the > >> encoder/crtc were fused pretty tightly and the effects of that are > >> far-reaching. > > > > I was afraid of this. Well, I guess nothing can be done about it then. > > > >> > >> > Anyway, please see my comments below, for the points I haven't overlooked > >> > due to the complexity of this patch. > >> > > >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_core.c > >> >> b/drivers/gpu/drm/exynos/exynos_drm_core.c > >> >> index 08ca4f9..e76098d 100644 > >> >> --- a/drivers/gpu/drm/exynos/exynos_drm_core.c > >> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_core.c > >> >> @@ -16,11 +16,14 @@ > >> >> #include > >> >> #include > >> > > >> > Huh? Including a specific bridge chip inside a generic Exynos DRM core? > >> > I know this is not a part of this patch, but... this is _broken_. > >> > > >> > You may want to support multiple bridge chips in Exynos DRM core and you > >> > may also want to support PTN3460 in other DRM cores. It can't be done > >> > this > >> > way. > >> > > >> > This series is very nice, but the whole effect is destroyed by basing it > >> > on top of such insanity... Please rebase it on top of something more > >> > reasonable (preferably Inki's next branch directly). > >> > > >> > >> Well, that was blunt. Unfortunately, I don't know how else to do this > >> other than this broken, unreasonable and insane way. > > > > Sorry, this might have been a bit too harsh, but just imagine myself doing > > my regular patch review round, hoping (as usual) to see only code that is > > not less than great, while suddenly stumbling upon a line of code that > > I would expect at most in some vendor tree or maybe several years ago in > > sources of a 2.4 kernel. > > More like code written in the same style as x86 DRM stuff, where > they're not used to overabstracting things from day one to make things > generic instead of supporting the only known chip combination to date. No, not really. They don't have any modularity on x86. Just a graphics card with everything on it, so they can freely do such things, as they don't have to account for all we need to account for on ARM platforms. Also, I wouldn't call making a driver usable and useful for more than just one fixed platform "overabstracting"... > > > >> This has already been discussed at length in a few other places. > >> Without some help from the drm layer, I don't see any other way to do > >> this than to enumerate the possible bridges in each drm driver. > >> > >> I based the current set off the "Add some missing bits for > >> exynos5250-snow" series I sent up a little while ago. This set was > >> sent to the relevant dt people, acked by Olof, and there are no > >> outstanding review com
