[Bug 178421] [regression] Radeon Oops on shutdown
https://bugzilla.kernel.org/show_bug.cgi?id=178421 Kevin changed: What|Removed |Added CC||kvbevsauce at gmail.com --- Comment #2 from Kevin --- Will these fixes be added to kernel 4.8 in a future minor version? or will I have to wait till 4.9? I cant shutdown since update to 4.8. reboot is ok though. radeon 7870 arch linux at 4.8.3-ck -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 83531] radeon 6650m cannot boot without radeon.runpm=0
https://bugzilla.kernel.org/show_bug.cgi?id=83531 Kevin changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |INVALID --- Comment #2 from Kevin --- cleaning. (2 years sorry) assumed bad hardware, works with bios disabled discrete card. -- You are receiving this mail because: You are watching the assignee of the bug.
[Intel-gfx] linux-next: Tree for Oct 20 (gpu/drm/i915)
On 2016.10.20 21:25:03 +0300, Jani Nikula wrote:
> On Thu, 20 Oct 2016, Daniel Vetter wrote:
> > On Thu, Oct 20, 2016 at 7:37 PM, Randy Dunlap
> > wrote:
> >> On 10/19/16 20:20, Stephen Rothwell wrote:
> >>> Hi all,
> >>>
> >>> Changes since 20161019:
> >>>
> >>
> >> on i386: when CONFIG_ACPI is not enabled:
> >
> > Adding Zhenyu. Might be good to have a fix just for this that I
> > directly pick up, since I want to tag the first 4.10 pull for Dave
> > Airlie this w/e.
>
> How about just this?
>
I'd like to not depend on acpi function any more, so just change that
for memremap. GVT-g driver currently only supports 64BIT kernel so will
fix that dependence. I'll send fix pull for Daniel later.
thanks
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 6aedc96aa412..94914381e8ef 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -85,7 +85,7 @@ config DRM_I915_USERPTR
>
> config DRM_I915_GVT
> bool "Enable Intel GVT-g graphics virtualization host support"
> -depends on DRM_I915
> +depends on DRM_I915 && ACPI
> default n
> help
> Choose this option if you want to enable Intel GVT-g graphics
>
>
>
> > -Daniel
> >
> >> ../drivers/gpu/drm/i915/gvt/opregion.c: In function
> >> 'intel_gvt_init_opregion':
> >> ../drivers/gpu/drm/i915/gvt/opregion.c:183:2: error: implicit declaration
> >> of function 'acpi_os_ioremap' [-Werror=implicit-function-declaration]
> >> gvt->opregion.opregion_va = acpi_os_ioremap(gvt->opregion.opregion_pa,
> >> ^
> >> ../drivers/gpu/drm/i915/gvt/opregion.c:183:28: warning: assignment makes
> >> pointer from integer without a cast [enabled by default]
> >> gvt->opregion.opregion_va = acpi_os_ioremap(gvt->opregion.opregion_pa,
> >> ^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function 'read_pte64':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:277:2: warning: left shift count >=
> >> width of type [enabled by default]
> >> pte |= ioread32(addr + 4) << 32;
> >> ^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function 'gen8_gtt_get_pfn':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:360:3: warning: left shift count >=
> >> width of type [enabled by default]
> >>pfn = (e->val64 & ADDR_4K_MASK) >> 12;
> >>^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function 'gen8_gtt_set_pfn':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:373:3: warning: left shift count >=
> >> width of type [enabled by default]
> >>e->val64 &= ~ADDR_4K_MASK;
> >>^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:374:3: warning: left shift count >=
> >> width of type [enabled by default]
> >>pfn &= (ADDR_4K_MASK >> 12);
> >>^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function 'gen8_gma_to_pml4_index':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:436:1: warning: right shift count >=
> >> width of type [enabled by default]
> >> DEFINE_PPGTT_GMA_TO_INDEX(gen8, pml4, (gma >> 39 & 0x1ff));
> >> ^
> >> CC drivers/gpu/drm/radeon/si_smc.o
> >> In file included from ../drivers/gpu/drm/i915/i915_drv.h:46:0,
> >> from ../drivers/gpu/drm/i915/gvt/gtt.c:36:
> >> ../drivers/gpu/drm/i915/gvt/gtt.c: In function
> >> 'intel_gvt_create_scratch_page':
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:1945:47: warning: cast from pointer to
> >> integer of different size [-Wpointer-to-int-cast]
> >>gvt_err("fail to translate vaddr:0x%llx\n", (u64)vaddr);
> >>^
> >> ../include/drm/drmP.h:201:43: note: in definition of macro 'DRM_ERROR'
> >> drm_printk(KERN_ERR, DRM_UT_NONE, fmt, ##__VA_ARGS__)
> >> ^
> >> ../drivers/gpu/drm/i915/gvt/gtt.c:1945:3: note: in expansion of macro
> >> 'gvt_err'
> >>gvt_err("fail to translate vaddr:0x%llx\n", (u64)vaddr);
> >>^
> >>
> >>
> >>
> >> --
> >> ~Randy
> >> ___
> >> Intel-gfx mailing list
> >> Intel-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/2339ae3c/attachment.sig>
[git pull] drm fixes for 4.9-rc2 (part 2)
Hi Linus, Second part of the fixes for rc2, main one being some vmwgfx fixes, also some armada, etnaviv and fsl-dcu fixes. There is a pretty large regression in -rc1 that affects radeon/amdgpu/nouveau and possibly other ttm using drivers with real VRAM on PAT systems, this was due to a change in mm, that uncovered an assumption in the drivers, I've started patches for it, and I'll hopefully line them up for submission next week, they hit outside drm a little. Dave. The following changes since commit 6f33d6458e35d6ba53c2635ee4b8a3177cbd912d: Merge tag 'pm-4.9-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm (2016-10-20 15:32:51 -0700) are available in the git repository at: git://people.freedesktop.org/~airlied/linux tags/drm-fixes-for-v4.9-rc2-part2 for you to fetch changes up to 26beaee9bb07be20cc641c1251152e280e80f54e: Merge branch 'drm-etnaviv-fixes' of git://git.pengutronix.de/lst/linux into drm-fixes (2016-10-21 13:27:55 +1000) drm fixes for 4.9 rc2 - vmware, fsl-dcu, armada and etnaviv Baole Ni (1): drm/vmwgfx: Replace numeric parameter like 0444 with macro Charmaine Lee (1): drm/vmwgfx: Enable SVGA_3D_CMD_DX_TRANSFER_FROM_BUFFER command Chris Wilson (1): drm/vmwgfx: Remove call to reservation_object_test_signaled_rcu before wait Dave Airlie (4): Merge branch 'fixes-for-v4.9-rc2' of http://git.agner.ch/git/linux-drm-fsl-dcu into drm-fixes Merge branch 'drm-armada-fixes' of git://git.armlinux.org.uk/~rmk/linux-arm into drm-fixes Merge branch 'drm-vmwgfx-fixes' of ssh://people.freedesktop.org/~syeh/repos_linux into drm-fixes Merge branch 'drm-etnaviv-fixes' of git://git.pengutronix.de/lst/linux into drm-fixes Lucas Stach (2): drm/etnaviv: ensure write caches are flushed at end of user cmdstream drm/etnaviv: block 64K of address space behind each cmdstream Markus Elfring (3): drm/vmwgfx: Use kmalloc_array() in vmw_surface_define_ioctl() drm/vmwgfx: Use memdup_user() rather than duplicating its implementation drm/vmwgfx: Adjust checks for null pointers in 13 functions Russell King (1): drm/armada: fix clock counts Stefan Agner (4): drm/fsl-dcu: enable TCON bypass mode by default drm/fsl-dcu: do not transfer registers on plane init drm/fsl-dcu: do not transfer registers in mode_set_nofb drm/fsl-dcu: enable pixel clock when enabling CRTC Thomas Hellstrom (4): drm/vmwgfx: Allow resource relocations on byte boundaries drm/vmwgfx: Remove a leftover debug printout drm/vmwgfx: Limit the user-space command buffer size drm/vmwgfx: Avoid validating views on view destruction drivers/gpu/drm/armada/armada_crtc.c| 18 ++-- drivers/gpu/drm/etnaviv/etnaviv_buffer.c| 24 - drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 3 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 4 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 23 + drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 5 - drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 39 +--- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 145 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c| 6 +- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 56 +-- 12 files changed, 187 insertions(+), 148 deletions(-)
[PATCH 2/8] drm: bridge: vga-dac: Add adi,adv7123 compatible string
On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
> The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
> controlled through a power save pin, and requires a power supply.
> However, on most boards where the device is used neither the power save
> signal nor the power supply are controllable.
>
> To avoid developing a separate device-specific driver add an
> "adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
> allow supporting most ADV7123-based boards easily, while allowing future
> development of an adv7123 driver when needed without breaking backward
> compatibility.
Shouldn't we have a DT binding doc for ADV7123, even if it's sharing
the dumb vga driver for now?
Same query for the Thine LVDS encoder.
Thanks,
Archit
>
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Laurent Pinchart
> ---
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232185a7..b33e3f829e4f 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>
> static const struct of_device_id dumb_vga_match[] = {
> { .compatible = "dumb-vga-dac" },
> + { .compatible = "adi,adv7123" },
> {},
> };
> MODULE_DEVICE_TABLE(of, dumb_vga_match);
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH 1/8] drm: bridge: Add LVDS encoder driver
On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
> The LVDS encoder driver is a DRM bridge driver that supports the
> parallel to LVDS encoders that don't require any configuration. The
> driver thus doesn't interact with the device, but creates an LVDS
> connector for the panel and exposes its size and timing based on
> information retrieved from DT.
>
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Laurent Pinchart
> ---
> .../bindings/display/bridge/lvds-transmitter.txt | 64 +++
> drivers/gpu/drm/bridge/Kconfig | 8 +
> drivers/gpu/drm/bridge/Makefile| 1 +
> drivers/gpu/drm/bridge/lvds-encoder.c | 203
> +
> 4 files changed, 276 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c
>
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> new file mode 100644
> index ..fd39ad34c383
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> @@ -0,0 +1,64 @@
> +Parallel to LVDS Encoder
> +
> +
> +This binding supports the parallel to LVDS encoders that don't require any
> +configuration.
> +
> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> Multiple
> +incompatible data link layers have been used over time to transmit image data
> +to LVDS panels. This binding targets devices compatible with the following
> +specifications only.
> +
> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
> +1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> +Semiconductor
> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> +Electronics Standards Association (VESA)
> +
> +Those devices have been marketed under the FPD-Link and FlatLink brand names
> +among others.
> +
> +
> +Required properties:
> +
> +- compatible: Must be "lvds-encoder"
> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modeled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for parallel input
> +- Video port 1 for LVDS output
> +
> +
> +Example
> +---
> +
> +lvds-encoder {
> + compatible = "lvds-encoder";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> +
> + lvds_enc_in: endpoint {
> + remote-endpoint = <&display_out_rgb>;
> + };
> + };
> +
> + port at 1 {
> + reg = <1>;
> +
> + lvds_enc_out: endpoint {
> + remote-endpoint = <&lvds_panel_in>;
> + };
> + };
> + };
> +};
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 10e12e74fc9f..5dafad7817ad 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -24,6 +24,14 @@ config DRM_DUMB_VGA_DAC
> help
> Support for RGB to VGA DAC based bridges
>
> +config DRM_LVDS_ENCODER
> + tristate "Transparent parallel to LVDS encoder support"
> + depends on OF
> + select DRM_KMS_HELPER
> + help
> + Support for transparent parallel to LVDS encoders that don't require
> + any configuration.
> +
> config DRM_DW_HDMI
> tristate
> select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index cdf3a3cf765d..bbaf583581ac 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -2,6 +2,7 @@ ccflags-y := -Iinclude/drm
>
> obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> +obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
> obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
> obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
> obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c
> b/drivers/gpu/drm/bridge/lvds-encoder.c
> new file mode 100644
> index ..33e8025c8a6d
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/lvds-encoder.c
> @@ -0,0 +1,203 @@
> +/*
> + * Copyright (C) 2016 Laurent Pinchart
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include
> +#
[Bug 98146] DRI_PRIME=1 and fullscreen issues
https://bugs.freedesktop.org/show_bug.cgi?id=98146 --- Comment #15 from Darek Deoniziak --- Thanks, it was my fault after all. As I did mention earlier I had also file called 10-monitor.conf and in that file I had already added section for devices, that file was primarly loaded and overloaded settings from other files. I moved DRI settings to that file because I was sure it is being loaded: ... Section "Device" Identifier "Device0" Driver "intel" Option "DRI" "3" EndSection Section "Device" Identifier "Device1" Driver "radeon" Option "DRI" "3" EndSection Now output of cat /var/log/Xorg.0.log | grep DRI: [ 5.164] (**) intel(0): Option "DRI" "3" [ 5.304] (**) RADEON(G0): Option "DRI" "3" [ 5.390] (II) glamor: EGL version 1.4 (DRI2): [ 5.394] (II) RADEON(G0): [DRI2] Setup complete [ 5.394] (II) RADEON(G0): [DRI2] DRI driver: radeonsi [ 5.394] (II) RADEON(G0): [DRI2] VDPAU driver: radeonsi [ 5.395] (**) RADEON(G0): DRI3 enabled [ 5.513] (II) intel(0): [DRI2] Setup complete [ 5.513] (II) intel(0): [DRI2] DRI driver: i965 [ 5.513] (II) intel(0): [DRI2] VDPAU driver: va_gl [ 5.513] (II) intel(0): direct rendering: DRI2 DRI3 enabled [ 5.528] (II) GLX: Initialized DRI2 GL provider for screen 0 Checked problem: 1 not drawing content until resizing windowed mode applications with DRI_PRIME=1 is solved, 2 did not check yet running fullscreen games 3 programs are still flickering but slightly different and much less, 4 did not check yet running fullscreen games 5 had no chance yet to plug in my external monitor yet I will be doing some more tests later today and this weekend to see if other problems are solved too, will write info asap. Big thanks for all help so far. -- You are receiving this mail because: You are the assignee for the bug. ------ next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/15cb48e4/attachment.html>
[PATCH 10/10] drm/imx: ipuv3-plane: use drm_plane_helper_check_state, clipped coordinates
On Thu, Oct 20, 2016 at 9:29 PM, Philipp Zabel wrote: > Am Donnerstag, den 20.10.2016, 16:51 +0800 schrieb Ying Liu: >> >> Does the clip thing potentially change the user's request by force? >> >> For example, the user request an unreasonable big resolution. >> > >> > The user is allowed to ask for destination coordinates extending outside >> > the crtc dimensions. This will chop off the parts that aren't visible, >> > and it will chop off the corresponding areas of the source as well. >> >> How about returning -EINVAL in this case which stands for >> an atomic check failure? > > Say the user requests to display a 640x480+0,0 source framebuffer at > destination offset -320,0 on a 320x240 screen, unscaled. The expectation > would be to see the upper right quarter of the framebuffer on the > screen, at least if the hardware was actually able to position overlays > partially offscreen. > If we can also fulfill that expectation by clipping the source rectangle > to 320,240+320,0 and changing the destination rectangle to 320x240+0,0, > why should -EINVAL be returned? Well, IIUC, there are two kinds of clipping. 1) Clipping a rectangle from a fb according to src_x/y and src_w/h. 2) Clipping done by drm_plane_helper_check_state(), which potentially changes src/dst->x1/2 and src/dst->y1/2(in other words, src_x/y, src_w/h and crtc_x/y/w/h, though not directly). 1) is fine, no problem. I doubt 2) is wrong as the users' original request could be changed. That's why I mentioned returning -EINVAL. Moreover, before and after applying the patch, I think the ->atomic_check behavior consistency is broken. For example, negative crtc_x or crtc_y for overlay are changed from unacceptable to potentially acceptable just because 2) may change their equivalent dst_>x/y1. Regards, Liu Ying > > regards > Philipp >
[patch] drm/i915/gvt: cleanup a type issue in submit_context()
submit_context() should return negative error codes and zero on success
but by mistake we made it a bool. I've changed it to int. Also I made
it static because this isn't referenced in any other files.
This doesn't affect runtime because the return is eventually propogated
to elsp_mmio_write() where it is ignored.
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/i915/gvt/execlist.c
b/drivers/gpu/drm/i915/gvt/execlist.c
index c50a3d1a5131..bc69ba1842ea 100644
--- a/drivers/gpu/drm/i915/gvt/execlist.c
+++ b/drivers/gpu/drm/i915/gvt/execlist.c
@@ -616,7 +616,7 @@ static int prepare_mm(struct intel_vgpu_workload *workload)
(list_empty(q) ? NULL : container_of(q->prev, \
struct intel_vgpu_workload, list))
-bool submit_context(struct intel_vgpu *vgpu, int ring_id,
+static int submit_context(struct intel_vgpu *vgpu, int ring_id,
struct execlist_ctx_descriptor_format *desc,
bool emulate_schedule_in)
{
[PATCH 10/10] drm/imx: ipuv3-plane: use drm_plane_helper_check_state, clipped coordinates
Am Freitag, den 21.10.2016, 13:45 +0800 schrieb Ying Liu: > On Thu, Oct 20, 2016 at 9:29 PM, Philipp Zabel > wrote: > > Am Donnerstag, den 20.10.2016, 16:51 +0800 schrieb Ying Liu: > >> >> Does the clip thing potentially change the user's request by force? > >> >> For example, the user request an unreasonable big resolution. > >> > > >> > The user is allowed to ask for destination coordinates extending outside > >> > the crtc dimensions. This will chop off the parts that aren't visible, > >> > and it will chop off the corresponding areas of the source as well. > >> > >> How about returning -EINVAL in this case which stands for > >> an atomic check failure? > > > > Say the user requests to display a 640x480+0,0 source framebuffer at > > destination offset -320,0 on a 320x240 screen, unscaled. The expectation > > would be to see the upper right quarter of the framebuffer on the > > screen, at least if the hardware was actually able to position overlays > > partially offscreen. > > If we can also fulfill that expectation by clipping the source rectangle > > to 320,240+320,0 and changing the destination rectangle to 320x240+0,0, > > why should -EINVAL be returned? > > Well, IIUC, there are two kinds of clipping. > 1) Clipping a rectangle from a fb according to src_x/y and src_w/h. > 2) Clipping done by drm_plane_helper_check_state(), which potentially > changes src/dst->x1/2 and src/dst->y1/2(in other words, src_x/y, > src_w/h and crtc_x/y/w/h, though not directly). > > 1) is fine, no problem. > I doubt 2) is wrong as the users' original request could be changed. > That's why I mentioned returning -EINVAL. > > Moreover, before and after applying the patch, I think the > ->atomic_check behavior consistency is broken. For example, > negative crtc_x or crtc_y for overlay are changed from unacceptable > to potentially acceptable just because 2) may change their equivalent > dst_>x/y1. I fail to see what's wrong with 2) as long as we can keep the observable behaviour exactly the same as if the user request was unchanged. regards Philipp
[PATCH] drm: Pass along the hotplug connector to the uevent
If we know which connector was plugged/unplugged or
connected/disconnected, we can pass that information along to userspace
inside the uevent to reduce the amount of work userspace has to perform
after the event (i.e. instead of looking over all connectors, it can
just reprobe the affected one).
Signed-off-by: Chris Wilson
Cc: Villle Syrjälä
Cc: Manasi Navare
---
drivers/gpu/drm/drm_probe_helper.c | 10 ++
drivers/gpu/drm/drm_sysfs.c| 19 +++
drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
drivers/gpu/drm/i915/intel_dp.c| 3 ++-
drivers/gpu/drm/i915/intel_dp_mst.c| 2 +-
drivers/gpu/drm/i915/intel_hotplug.c | 6 +-
drivers/gpu/drm/qxl/qxl_display.c | 2 +-
drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_vq.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c| 2 +-
include/drm/drmP.h | 3 ++-
include/drm/drm_crtc_helper.h | 3 ++-
13 files changed, 35 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index f6b64d7d3528..8790ee35a7cd 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -347,6 +347,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
/**
* drm_kms_helper_hotplug_event - fire off KMS hotplug events
* @dev: drm_device whose connector state changed
+ * @connector: connector on which the status changed, if any
*
* This function fires off the uevent for userspace and also calls the
* output_poll_changed function, which is most commonly used to inform the
fbdev
@@ -360,10 +361,11 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
* This function must be called from process context with no mode
* setting locks held.
*/
-void drm_kms_helper_hotplug_event(struct drm_device *dev)
+void drm_kms_helper_hotplug_event(struct drm_device *dev,
+ struct drm_connector *connector)
{
/* send a uevent + call fbdev */
- drm_sysfs_hotplug_event(dev);
+ drm_sysfs_hotplug_event(dev, connector);
if (dev->mode_config.funcs->output_poll_changed)
dev->mode_config.funcs->output_poll_changed(dev);
}
@@ -444,7 +446,7 @@ static void output_poll_execute(struct work_struct *work)
out:
if (changed)
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_hotplug_event(dev, NULL);
if (repoll)
schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
@@ -578,7 +580,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
mutex_unlock(&dev->mode_config.mutex);
if (changed)
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_hotplug_event(dev, NULL);
return changed;
}
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 9a37196c1bf1..c8f46055e2d0 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -280,7 +280,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
}
/* Let userspace know we have a new connector */
- drm_sysfs_hotplug_event(dev);
+ drm_sysfs_hotplug_event(dev, connector);
return 0;
}
@@ -312,19 +312,30 @@ void drm_sysfs_connector_remove(struct drm_connector
*connector)
/**
* drm_sysfs_hotplug_event - generate a DRM uevent
* @dev: DRM device
+ * @connector: the DRM connector, if any
*
* Send a uevent for the DRM device specified by @dev. Currently we only
* set HOTPLUG=1 in the uevent environment, but this could be expanded to
* deal with other types of events.
*/
-void drm_sysfs_hotplug_event(struct drm_device *dev)
+void drm_sysfs_hotplug_event(struct drm_device *dev,
+struct drm_connector *connector)
{
- char *event_string = "HOTPLUG=1";
- char *envp[] = { event_string, NULL };
+ char *envp[3] = { "HOTPLUG=1" };
+ char *connector_event = NULL;
+
+ if (connector)
+ connector_event = kasprintf(GFP_KERNEL,
+ "CONNECTOR=%d",
+ connector->base.id);
+ if (connector_event)
+ envp[1] = connector_event;
DRM_DEBUG("generating hotplug event\n");
kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
+
+ kfree(connector_event);
}
EXPORT_SYMBOL(drm_sysfs_hotplug_event);
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 9798d400d817..9fd873c87686 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -617,7 +617,7 @@ static void tda998x_detect_work(struct work_struct *work)
struct drm_device *dev = priv->encoder.dev;
if (dev)
- drm_kms_helper_hotplug_event(dev);
+ drm
[patch] drm/i915/gvt: cleanup a type issue in submit_context()
On 2016.10.21 11:06:01 +0300, Dan Carpenter wrote:
> submit_context() should return negative error codes and zero on success
> but by mistake we made it a bool. I've changed it to int. Also I made
> it static because this isn't referenced in any other files.
>
> This doesn't affect runtime because the return is eventually propogated
> to elsp_mmio_write() where it is ignored.
>
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/gpu/drm/i915/gvt/execlist.c
> b/drivers/gpu/drm/i915/gvt/execlist.c
> index c50a3d1a5131..bc69ba1842ea 100644
> --- a/drivers/gpu/drm/i915/gvt/execlist.c
> +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> @@ -616,7 +616,7 @@ static int prepare_mm(struct intel_vgpu_workload
> *workload)
> (list_empty(q) ? NULL : container_of(q->prev, \
> struct intel_vgpu_workload, list))
>
> -bool submit_context(struct intel_vgpu *vgpu, int ring_id,
> +static int submit_context(struct intel_vgpu *vgpu, int ring_id,
> struct execlist_ctx_descriptor_format *desc,
> bool emulate_schedule_in)
> {
Dan, this has been fixed in our recent pull, should hit linux-next very soon.
thanks
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/eca5866d/attachment.sig>
[Intel-gfx] [patch] drm/i915/gvt: cleanup a type issue in submit_context()
On Fri, Oct 21, 2016 at 11:06:01AM +0300, Dan Carpenter wrote: > submit_context() should return negative error codes and zero on success > but by mistake we made it a bool. I've changed it to int. Also I made > it static because this isn't referenced in any other files. > > This doesn't affect runtime because the return is eventually propogated > to elsp_mmio_write() where it is ignored. Thanks Dan, your work in finding these is always appreciated (as is smatch!) Should be fixed by commit 76a79d59ada00fa22e5f8cd94b36296f395c3406 Author: Du, Changbin Date: Thu Oct 20 14:08:48 2016 +0800 drm/i915/gvt: fix spare warnings on odd constant _Bool cast The function return values should has type int if it return a integer value. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH 10/10] drm/imx: ipuv3-plane: use drm_plane_helper_check_state, clipped coordinates
On Fri, Oct 21, 2016 at 4:18 PM, Philipp Zabel wrote: > Am Freitag, den 21.10.2016, 13:45 +0800 schrieb Ying Liu: >> On Thu, Oct 20, 2016 at 9:29 PM, Philipp Zabel >> wrote: >> > Am Donnerstag, den 20.10.2016, 16:51 +0800 schrieb Ying Liu: >> >> >> Does the clip thing potentially change the user's request by force? >> >> >> For example, the user request an unreasonable big resolution. >> >> > >> >> > The user is allowed to ask for destination coordinates extending outside >> >> > the crtc dimensions. This will chop off the parts that aren't visible, >> >> > and it will chop off the corresponding areas of the source as well. >> >> >> >> How about returning -EINVAL in this case which stands for >> >> an atomic check failure? >> > >> > Say the user requests to display a 640x480+0,0 source framebuffer at >> > destination offset -320,0 on a 320x240 screen, unscaled. The expectation >> > would be to see the upper right quarter of the framebuffer on the >> > screen, at least if the hardware was actually able to position overlays >> > partially offscreen. >> > If we can also fulfill that expectation by clipping the source rectangle >> > to 320,240+320,0 and changing the destination rectangle to 320x240+0,0, >> > why should -EINVAL be returned? >> >> Well, IIUC, there are two kinds of clipping. >> 1) Clipping a rectangle from a fb according to src_x/y and src_w/h. >> 2) Clipping done by drm_plane_helper_check_state(), which potentially >> changes src/dst->x1/2 and src/dst->y1/2(in other words, src_x/y, >> src_w/h and crtc_x/y/w/h, though not directly). >> >> 1) is fine, no problem. >> I doubt 2) is wrong as the users' original request could be changed. >> That's why I mentioned returning -EINVAL. >> >> Moreover, before and after applying the patch, I think the >> ->atomic_check behavior consistency is broken. For example, >> negative crtc_x or crtc_y for overlay are changed from unacceptable >> to potentially acceptable just because 2) may change their equivalent >> dst_>x/y1. > > I fail to see what's wrong with 2) as long as we can keep the observable > behaviour exactly the same as if the user request was unchanged. It seems the behavior could change - negative crtc_x or crtc_y for overlay make ->atomic_check return -EINVAL before(overlay hw state machine has nothing changed), and potentially successful after(overlay hw state machine changes). Regards, Liu Ying > > regards > Philipp >
[RFC] drm: add hint to userspace about whether a plane can scale
Hi Rob,
On Thu, Oct 20, 2016 at 04:17:14PM -0400, Rob Clark wrote:
>When you have a mix of planes that can scale and those that cannot
>scale, userspace really wants to have some hint to know which planes
>can definitely not scale so it knows to assign them to unscaled layers.
>I don't think it is fully possible to describe scaling constraints in
>a generic way, so I don't think it is even worth trying, so this is
>not a substitute for atomic TESTONLY step, but it does reduce the
>search-space for userspace. In the common case, most layers will not
>be scaled so knowing the best planes to pick for those layers is
>useful.
Somewhat related to what Daniel mentioned on IRC about driver
consistency - how about making it "cannot_scale". This is then opt-in
for drivers, and should mean userspace can always trust it if it's
set.
i.e. if cannot_scale == false, userspace can give it a shot, but if
cannot_scale == true it should never bother.
Either way, even with a device-specific planner we would want this
hint to manage different HW versions so I'm in favour. But...
>---
> drivers/gpu/drm/drm_crtc.c| 1 +
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 1 +
> include/drm/drm_crtc.h| 2 ++
> include/uapi/drm/drm_mode.h | 3 +++
> 4 files changed, 7 insertions(+)
>
>diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>index b4b973f..d7e0e0d 100644
>--- a/drivers/gpu/drm/drm_crtc.c
>+++ b/drivers/gpu/drm/drm_crtc.c
>@@ -2389,6 +2389,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
> plane_resp->plane_id = plane->base.id;
> plane_resp->possible_crtcs = plane->possible_crtcs;
> plane_resp->gamma_size = 0;
>+ plane_resp->can_scale = plane->can_scale;
>
> /*
>* This ioctl is called twice, once to determine how much space is
>diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>index 692c888..2061c83 100644
>--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>@@ -908,6 +908,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
> mdp5_plane->pipe = pipe;
> mdp5_plane->name = pipe2name(pipe);
> mdp5_plane->caps = caps;
>+ plane->can_scale = !!(caps & MDP_PIPE_CAP_SCALE);
>
> mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats,
> ARRAY_SIZE(mdp5_plane->formats),
>diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>index d74d47a..6e290b6 100644
>--- a/include/drm/drm_crtc.h
>+++ b/include/drm/drm_crtc.h
>@@ -1679,6 +1679,7 @@ enum drm_plane_type {
> * @format_types: array of formats supported by this plane
> * @format_count: number of formats supported
> * @format_default: driver hasn't supplied supported formats for the plane
>+ * @can_scale: a hint to userspace that this plane can (or cannot) scale
> * @crtc: currently bound CRTC
> * @fb: currently bound fb
> * @old_fb: Temporary tracking of the old fb while a modeset is ongoing. Used
> by
>@@ -1710,6 +1711,7 @@ struct drm_plane {
> uint32_t *format_types;
> unsigned int format_count;
> bool format_default;
>+ bool can_scale;
>
> struct drm_crtc *crtc;
> struct drm_framebuffer *fb;
>diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>index ce71ad5..5bf9361 100644
>--- a/include/uapi/drm/drm_mode.h
>+++ b/include/uapi/drm/drm_mode.h
>@@ -191,6 +191,9 @@ struct drm_mode_get_plane {
>
> __u32 count_format_types;
> __u64 format_type_ptr;
>+
>+ __u32 can_scale;
... 32 bits for a bool does seem a bit wasteful.
Thanks,
Brian
>+ __u32 pad;
> };
>
> struct drm_mode_get_plane_res {
>--
>2.7.4
>
>___
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: Pass along the hotplug connector to the uevent
If we know which connector was plugged/unplugged or
connected/disconnected, we can pass that information along to userspace
inside the uevent to reduce the amount of work userspace has to perform
after the event (i.e. instead of looking over all connectors, it can
just reprobe the affected one).
v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
need the force.
Signed-off-by: Chris Wilson
Cc: Villle Syrjälä
Cc: Manasi Navare
---
drivers/gpu/drm/drm_probe_helper.c | 10 ++
drivers/gpu/drm/drm_sysfs.c| 19 +++
drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
drivers/gpu/drm/i915/intel_dp.c| 3 ++-
drivers/gpu/drm/i915/intel_dp_mst.c| 2 +-
drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
drivers/gpu/drm/qxl/qxl_display.c | 2 +-
drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_vq.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c| 2 +-
include/drm/drmP.h | 3 ++-
include/drm/drm_crtc_helper.h | 3 ++-
13 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index f6b64d7d3528..8790ee35a7cd 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -347,6 +347,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
/**
* drm_kms_helper_hotplug_event - fire off KMS hotplug events
* @dev: drm_device whose connector state changed
+ * @connector: connector on which the status changed, if any
*
* This function fires off the uevent for userspace and also calls the
* output_poll_changed function, which is most commonly used to inform the
fbdev
@@ -360,10 +361,11 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
* This function must be called from process context with no mode
* setting locks held.
*/
-void drm_kms_helper_hotplug_event(struct drm_device *dev)
+void drm_kms_helper_hotplug_event(struct drm_device *dev,
+ struct drm_connector *connector)
{
/* send a uevent + call fbdev */
- drm_sysfs_hotplug_event(dev);
+ drm_sysfs_hotplug_event(dev, connector);
if (dev->mode_config.funcs->output_poll_changed)
dev->mode_config.funcs->output_poll_changed(dev);
}
@@ -444,7 +446,7 @@ static void output_poll_execute(struct work_struct *work)
out:
if (changed)
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_hotplug_event(dev, NULL);
if (repoll)
schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
@@ -578,7 +580,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
mutex_unlock(&dev->mode_config.mutex);
if (changed)
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_hotplug_event(dev, NULL);
return changed;
}
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 9a37196c1bf1..c8f46055e2d0 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -280,7 +280,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
}
/* Let userspace know we have a new connector */
- drm_sysfs_hotplug_event(dev);
+ drm_sysfs_hotplug_event(dev, connector);
return 0;
}
@@ -312,19 +312,30 @@ void drm_sysfs_connector_remove(struct drm_connector
*connector)
/**
* drm_sysfs_hotplug_event - generate a DRM uevent
* @dev: DRM device
+ * @connector: the DRM connector, if any
*
* Send a uevent for the DRM device specified by @dev. Currently we only
* set HOTPLUG=1 in the uevent environment, but this could be expanded to
* deal with other types of events.
*/
-void drm_sysfs_hotplug_event(struct drm_device *dev)
+void drm_sysfs_hotplug_event(struct drm_device *dev,
+struct drm_connector *connector)
{
- char *event_string = "HOTPLUG=1";
- char *envp[] = { event_string, NULL };
+ char *envp[3] = { "HOTPLUG=1" };
+ char *connector_event = NULL;
+
+ if (connector)
+ connector_event = kasprintf(GFP_KERNEL,
+ "CONNECTOR=%d",
+ connector->base.id);
+ if (connector_event)
+ envp[1] = connector_event;
DRM_DEBUG("generating hotplug event\n");
kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
+
+ kfree(connector_event);
}
EXPORT_SYMBOL(drm_sysfs_hotplug_event);
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 9798d400d817..9fd873c87686 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -617,7 +617,7 @@ static void tda998x_detect_work(struct work_struct *work)
struct drm_device *dev = priv->encoder.dev;
[PATCH 2/3] ARM: bus: da8xx-syscfg: new driver
On 20/10/16 22:39, Kevin Hilman wrote: > However, after our discussion on IRC, we'll respin this without the DT > bindings at all. Next version will just use static configuration data > in the drivers/bus driver based on SoC compatible string, since for the > use-cases I'm aware of, the settings are boot-time only. If it's static boot time config, why not do it in the u-boot? I'm fine either way, but this sounds like quite low level memory bus config. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/09458b88/attachment.sig>
[Intel-gfx] [PATCH v2] drm: Pass along the hotplug connector to the uevent
On Fri, 21 Oct 2016, Chris Wilson wrote:
> If we know which connector was plugged/unplugged or
> connected/disconnected, we can pass that information along to userspace
> inside the uevent to reduce the amount of work userspace has to perform
> after the event (i.e. instead of looking over all connectors, it can
> just reprobe the affected one).
>
> v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> need the force.
>
> Signed-off-by: Chris Wilson
> Cc: Villle Syrjälä
> Cc: Manasi Navare
> ---
> drivers/gpu/drm/drm_probe_helper.c | 10 ++
> drivers/gpu/drm/drm_sysfs.c| 19 +++
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/i915/intel_dp.c| 3 ++-
> drivers/gpu/drm/i915/intel_dp_mst.c| 2 +-
> drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
> drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c| 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c| 2 +-
> include/drm/drmP.h | 3 ++-
> include/drm/drm_crtc_helper.h | 3 ++-
> 13 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index f6b64d7d3528..8790ee35a7cd 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -347,6 +347,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
> /**
> * drm_kms_helper_hotplug_event - fire off KMS hotplug events
> * @dev: drm_device whose connector state changed
> + * @connector: connector on which the status changed, if any
> *
> * This function fires off the uevent for userspace and also calls the
> * output_poll_changed function, which is most commonly used to inform the
> fbdev
> @@ -360,10 +361,11 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
> * This function must be called from process context with no mode
> * setting locks held.
> */
> -void drm_kms_helper_hotplug_event(struct drm_device *dev)
> +void drm_kms_helper_hotplug_event(struct drm_device *dev,
> + struct drm_connector *connector)
> {
> /* send a uevent + call fbdev */
> - drm_sysfs_hotplug_event(dev);
> + drm_sysfs_hotplug_event(dev, connector);
> if (dev->mode_config.funcs->output_poll_changed)
> dev->mode_config.funcs->output_poll_changed(dev);
> }
> @@ -444,7 +446,7 @@ static void output_poll_execute(struct work_struct *work)
>
> out:
> if (changed)
> - drm_kms_helper_hotplug_event(dev);
> + drm_kms_helper_hotplug_event(dev, NULL);
>
> if (repoll)
> schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
> @@ -578,7 +580,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
> mutex_unlock(&dev->mode_config.mutex);
>
> if (changed)
> - drm_kms_helper_hotplug_event(dev);
> + drm_kms_helper_hotplug_event(dev, NULL);
>
> return changed;
> }
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 9a37196c1bf1..c8f46055e2d0 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -280,7 +280,7 @@ int drm_sysfs_connector_add(struct drm_connector
> *connector)
> }
>
> /* Let userspace know we have a new connector */
> - drm_sysfs_hotplug_event(dev);
> + drm_sysfs_hotplug_event(dev, connector);
>
> return 0;
> }
> @@ -312,19 +312,30 @@ void drm_sysfs_connector_remove(struct drm_connector
> *connector)
> /**
> * drm_sysfs_hotplug_event - generate a DRM uevent
> * @dev: DRM device
> + * @connector: the DRM connector, if any
> *
> * Send a uevent for the DRM device specified by @dev. Currently we only
> * set HOTPLUG=1 in the uevent environment, but this could be expanded to
> * deal with other types of events.
> */
> -void drm_sysfs_hotplug_event(struct drm_device *dev)
> +void drm_sysfs_hotplug_event(struct drm_device *dev,
> + struct drm_connector *connector)
> {
> - char *event_string = "HOTPLUG=1";
> - char *envp[] = { event_string, NULL };
> + char *envp[3] = { "HOTPLUG=1" };
> + char *connector_event = NULL;
> +
> + if (connector)
> + connector_event = kasprintf(GFP_KERNEL,
GFP_TEMPORARY?
Otherwise, on a light reading of the patch, and assuming existing
userspace doesn't fall over because of this,
Reviewed-by: Jani Nikula
> + "CONNECTOR=%d",
> + connector->base.id);
> + if (connector_event)
> + envp[1] = connector_event;
>
> DRM_DEBUG("generating hotplug event\n");
>
> kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
> +
> + kfree(connector_event);
> }
> EX
[Bug 98352] [ctg bat] igt@kms_flip@basic-flip-vs-wf_vblank
https://bugs.freedesktop.org/show_bug.cgi?id=98352 yann changed: What|Removed |Added Assignee|intel-gfx-bugs at lists.freede |dri-devel at lists.freedesktop |sktop.org |.org QA Contact|intel-gfx-bugs at lists.freede | |sktop.org | Component|DRM/Intel |IGT --- Comment #1 from yann --- Reference to Chris' patch: https://patchwork.freedesktop.org/series/14114/ -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/73df223e/attachment.html>
[PATCH v2] drm: Pass along the hotplug connector to the uevent
On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote:
> If we know which connector was plugged/unplugged or
> connected/disconnected, we can pass that information along to userspace
> inside the uevent to reduce the amount of work userspace has to perform
> after the event (i.e. instead of looking over all connectors, it can
> just reprobe the affected one).
>
> v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> need the force.
>
> Signed-off-by: Chris Wilson
> Cc: Villle Syrjälä
> Cc: Manasi Navare
> ---
> drivers/gpu/drm/drm_probe_helper.c | 10 ++
> drivers/gpu/drm/drm_sysfs.c| 19 +++
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/i915/intel_dp.c| 3 ++-
> drivers/gpu/drm/i915/intel_dp_mst.c| 2 +-
> drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
> drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c| 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c| 2 +-
> include/drm/drmP.h | 3 ++-
> include/drm/drm_crtc_helper.h | 3 ++-
> 13 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 3ffbd69e4551..2bd48a987934 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -493,7 +493,7 @@ static void intel_dp_mst_hotplug(struct
> drm_dp_mst_topology_mgr *mgr)
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = intel_dig_port->base.base.dev;
>
> - drm_kms_helper_hotplug_event(dev);
> + drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
> }
>
> static const struct drm_dp_mst_topology_cbs mst_cbs = {
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 334d47b5811a..da0649aff734 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -340,7 +340,7 @@ static void i915_hotplug_work_func(struct work_struct
> *work)
> mutex_unlock(&mode_config->mutex);
>
> if (changed)
> - drm_kms_helper_hotplug_event(dev);
> + drm_kms_helper_hotplug_event(dev, NULL);
> }
So basically this patch deals with all the weird cases but doesn't do
anything for the normal case of a connector being connected/disconnected?
--
Ville Syrjälä
Intel OTC
[PATCH v6 03/11] drm/i915: return EACCES for check_cmd() failures
On Thu, Oct 20, 2016 at 10:19:02PM +0100, Robert Bragg wrote: > @@ -1333,6 +1333,9 @@ int i915_cmd_parser_get_version(struct drm_i915_private > *dev_priv) >* 5. GPGPU dispatch compute indirect registers. >* 6. TIMESTAMP register and Haswell CS GPR registers >* 7. Allow MI_LOAD_REGISTER_REG between whitelisted registers. > + * 8. Don't report cmd_check() failures as EINVAL errors to userspace; > + *rely on the HW to NOOP disallowed commands as it would without > + *the parser enabled. I added a test case to exercise this and at least for OACONTROL we are fine. That test can supercede the negative BAT in gem_exec_parse. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH 2/3] ARM: bus: da8xx-syscfg: new driver
On 21/10/16 12:53, Sekhar Nori wrote: > On Friday 21 October 2016 02:55 PM, Tomi Valkeinen wrote: >> On 20/10/16 22:39, Kevin Hilman wrote: >> >>> However, after our discussion on IRC, we'll respin this without the DT >>> bindings at all. Next version will just use static configuration data >>> in the drivers/bus driver based on SoC compatible string, since for the >>> use-cases I'm aware of, the settings are boot-time only. >> >> If it's static boot time config, why not do it in the u-boot? > > Hardware initialization dependencies with boot-loader are tough to track > and debug. The bootloader thats currently ships with the board may not > have these settings, for example. This forces everyone to update their > bootloader when shifting to mainline kernel. Yep, true... We need something similar for AM335x too. And perhaps for other SoCs too (AM4 comes to my mind). Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/4f710899/attachment.sig>
[PATCH v2] drm: Pass along the hotplug connector to the uevent
On Fri, Oct 21, 2016 at 12:46:53PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote:
> > If we know which connector was plugged/unplugged or
> > connected/disconnected, we can pass that information along to userspace
> > inside the uevent to reduce the amount of work userspace has to perform
> > after the event (i.e. instead of looking over all connectors, it can
> > just reprobe the affected one).
> >
> > v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> > need the force.
> >
> > Signed-off-by: Chris Wilson
> > Cc: Villle Syrjälä
> > Cc: Manasi Navare
> > ---
> > drivers/gpu/drm/drm_probe_helper.c | 10 ++
> > drivers/gpu/drm/drm_sysfs.c| 19 +++
> > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> > drivers/gpu/drm/i915/intel_dp.c| 3 ++-
> > drivers/gpu/drm/i915/intel_dp_mst.c| 2 +-
> > drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
> > drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> > drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
> > drivers/gpu/drm/virtio/virtgpu_vq.c| 2 +-
> > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 2 +-
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c| 2 +-
> > include/drm/drmP.h | 3 ++-
> > include/drm/drm_crtc_helper.h | 3 ++-
> > 13 files changed, 35 insertions(+), 19 deletions(-)
> >
>
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 3ffbd69e4551..2bd48a987934 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -493,7 +493,7 @@ static void intel_dp_mst_hotplug(struct
> > drm_dp_mst_topology_mgr *mgr)
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > struct drm_device *dev = intel_dig_port->base.base.dev;
> >
> > - drm_kms_helper_hotplug_event(dev);
> > + drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
> > }
> >
> > static const struct drm_dp_mst_topology_cbs mst_cbs = {
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 334d47b5811a..da0649aff734 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -340,7 +340,7 @@ static void i915_hotplug_work_func(struct work_struct
> > *work)
> > mutex_unlock(&mode_config->mutex);
> >
> > if (changed)
> > - drm_kms_helper_hotplug_event(dev);
> > + drm_kms_helper_hotplug_event(dev, NULL);
> > }
>
> So basically this patch deals with all the weird cases but doesn't do
> anything for the normal case of a connector being connected/disconnected?
Yes. We can expand it to the lightweight hotplug events, but where it
was just checking the current connector->status I didn't feel it merited
the extra work in sending per-connector hotplug events.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH 1/8] drm: bridge: Add LVDS encoder driver
Hi Archit,
On Friday 21 Oct 2016 10:51:59 Archit Taneja wrote:
> On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
> > The LVDS encoder driver is a DRM bridge driver that supports the
> > parallel to LVDS encoders that don't require any configuration. The
> > driver thus doesn't interact with the device, but creates an LVDS
> > connector for the panel and exposes its size and timing based on
> > information retrieved from DT.
> >
> > Cc: devicetree at vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> >
> > ---
> >
> > .../bindings/display/bridge/lvds-transmitter.txt | 64 +++
> > drivers/gpu/drm/bridge/Kconfig | 8 +
> > drivers/gpu/drm/bridge/Makefile| 1 +
> > drivers/gpu/drm/bridge/lvds-encoder.c | 203
> > 4 files changed, 276 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> > create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> > b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> > new file mode 100644
> > index ..fd39ad34c383
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> > @@ -0,0 +1,64 @@
> > +Parallel to LVDS Encoder
> > +
> > +
> > +This binding supports the parallel to LVDS encoders that don't require
> > any
> > +configuration.
> > +
> > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> > Multiple +incompatible data link layers have been used over time to
> > transmit image data +to LVDS panels. This binding targets devices
> > compatible with the following +specifications only.
> > +
> > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> > February
> > +1999 (Version 1.0), Japan Electronic Industry Development Association
> > (JEIDA) +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> > National +Semiconductor
> > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> > +Electronics Standards Association (VESA)
> > +
> > +Those devices have been marketed under the FPD-Link and FlatLink brand
> > names +among others.
> > +
> > +
> > +Required properties:
> > +
> > +- compatible: Must be "lvds-encoder"
> > +
> > +Required nodes:
> > +
> > +This device has two video ports. Their connections are modeled using the
> > OF +graph bindings specified in
> > Documentation/devicetree/bindings/graph.txt. +
> > +- Video port 0 for parallel input
> > +- Video port 1 for LVDS output
> > +
> > +
> > +Example
> > +---
> > +
> > +lvds-encoder {
> > + compatible = "lvds-encoder";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port at 0 {
> > + reg = <0>;
> > +
> > + lvds_enc_in: endpoint {
> > + remote-endpoint = <&display_out_rgb>;
> > + };
> > + };
> > +
> > + port at 1 {
> > + reg = <1>;
> > +
> > + lvds_enc_out: endpoint {
> > + remote-endpoint = <&lvds_panel_in>;
> > + };
> > + };
> > + };
> > +};
> > diff --git a/drivers/gpu/drm/bridge/Kconfig
> > b/drivers/gpu/drm/bridge/Kconfig index 10e12e74fc9f..5dafad7817ad 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -24,6 +24,14 @@ config DRM_DUMB_VGA_DAC
> >
> > help
> >
> > Support for RGB to VGA DAC based bridges
> >
> > +config DRM_LVDS_ENCODER
> > + tristate "Transparent parallel to LVDS encoder support"
> > + depends on OF
> > + select DRM_KMS_HELPER
> > + help
> > + Support for transparent parallel to LVDS encoders that don't require
> > + any configuration.
> > +
> >
> > config DRM_DW_HDMI
> >
> > tristate
> > select DRM_KMS_HELPER
> >
> > diff --git a/drivers/gpu/drm/bridge/Makefile
> > b/drivers/gpu/drm/bridge/Makefile index cdf3a3cf765d..bbaf583581ac 100644
> > --- a/drivers/gpu/drm/bridge/Makefile
> > +++ b/drivers/gpu/drm/bridge/Makefile
> > @@ -2,6 +2,7 @@ ccflags-y := -Iinclude/drm
> >
> > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> > obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> >
> > +obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
> >
> > obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
> > obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
> > obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> >
> > diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c
> > b/drivers/gpu/drm/bridge/lvds-encoder.c new file mode 100644
> > index ..33e8025c8a6d
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/lvds-encoder.c
> > @@ -0,0 +1,203 @@
> > +/*
> > + * Copyright (C) 201
[PATCH 2/8] drm: bridge: vga-dac: Add adi, adv7123 compatible string
Hi Archit,
On Friday 21 Oct 2016 10:43:34 Archit Taneja wrote:
> On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
> > The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
> > controlled through a power save pin, and requires a power supply.
> > However, on most boards where the device is used neither the power save
> > signal nor the power supply are controllable.
> >
> > To avoid developing a separate device-specific driver add an
> > "adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
> > allow supporting most ADV7123-based boards easily, while allowing future
> > development of an adv7123 driver when needed without breaking backward
> > compatibility.
>
> Shouldn't we have a DT binding doc for ADV7123, even if it's sharing
> the dumb vga driver for now?
Documentation/devicetree/bindings/display/bridge/adi,adv7123.txt
> Same query for the Thine LVDS encoder.
Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt
> > Cc: devicetree at vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> >
> > ---
> >
> > drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > b/drivers/gpu/drm/bridge/dumb-vga-dac.c index afec232185a7..b33e3f829e4f
> > 100644
> > --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device
> > *pdev)>
> > static const struct of_device_id dumb_vga_match[] = {
> >
> > { .compatible = "dumb-vga-dac" },
> > + { .compatible = "adi,adv7123" },
> > {},
> > };
> > MODULE_DEVICE_TABLE(of, dumb_vga_match);
--
Regards,
Laurent Pinchart
[PATCH 2/8] drm: bridge: vga-dac: Add adi,adv7123 compatible string
On 10/21/2016 03:52 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> On Friday 21 Oct 2016 10:43:34 Archit Taneja wrote:
>> On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
>>> The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
>>> controlled through a power save pin, and requires a power supply.
>>> However, on most boards where the device is used neither the power save
>>> signal nor the power supply are controllable.
>>>
>>> To avoid developing a separate device-specific driver add an
>>> "adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
>>> allow supporting most ADV7123-based boards easily, while allowing future
>>> development of an adv7123 driver when needed without breaking backward
>>> compatibility.
>>
>> Shouldn't we have a DT binding doc for ADV7123, even if it's sharing
>> the dumb vga driver for now?
>
> Documentation/devicetree/bindings/display/bridge/adi,adv7123.txt
>
>> Same query for the Thine LVDS encoder.
>
> Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt
Cool, didn't know these already existed :)
Thanks,
Archit
>
>>> Cc: devicetree at vger.kernel.org
>>> Signed-off-by: Laurent Pinchart
>>>
>>> ---
>>>
>>> drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index afec232185a7..b33e3f829e4f
>>> 100644
>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device
>>> *pdev)>
>>> static const struct of_device_id dumb_vga_match[] = {
>>>
>>> { .compatible = "dumb-vga-dac" },
>>> + { .compatible = "adi,adv7123" },
>>> {},
>>> };
>>> MODULE_DEVICE_TABLE(of, dumb_vga_match);
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v5 4/4] drm/fence: add out-fences support
Hi Gustavo,
On Thu, Oct 20, 2016 at 06:30:17PM -0200, Gustavo Padovan wrote:
>Hi Brian,
>
>2016-10-20 Brian Starkey :
>
>> Hi Gustavo,
>>
>> I notice your branch has the sync_file refcount change in, but this
>> doesn't seem to take account for that. Will you be dropping that
>> change to match the semantics of fence_array?
>
>I will drop the fence_get() in the out-fence patch because we already
>hold the ref when we create the fence. The sync_file refcount patch will
>remain.
>
OK, I'll work on that basis, thanks.
>>
>> Couple more comments below.
>>
>> On Thu, Oct 20, 2016 at 12:50:05PM -0200, Gustavo Padovan wrote:
>> > From: Gustavo Padovan
>> >
>> > Support DRM out-fences by creating a sync_file with a fence for each CRTC
>> > that sets the OUT_FENCE_PTR property.
>> >
>> > We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
>> > the sync_file fd back to userspace.
>> >
>> > The sync_file and fd are allocated/created before commit, but the
>> > fd_install operation only happens after we know that commit succeed.
>> >
>> > In case of error userspace should receive a fd number of -1.
>> >
>> > v2: Comment by Rob Clark:
>> >- Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
>> >
>> >Comment by Daniel Vetter:
>> >- Add clean up code for out_fences
>> >
>> > v3: Comments by Daniel Vetter:
>> >- create DRM_MODE_ATOMIC_EVENT_MASK
>> >- userspace should fill out_fences_ptr with the crtc_ids for which
>> >it wants fences back.
>> >
>> > v4: Create OUT_FENCE_PTR properties and remove old approach.
>> >
>> > Signed-off-by: Gustavo Padovan
>> > ---
>> > drivers/gpu/drm/drm_atomic.c | 116
>> > ++-
>> > drivers/gpu/drm/drm_crtc.c | 8 +++
>> > include/drm/drm_crtc.h | 19 +++
>> > 3 files changed, 119 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> > index b3284b2..07775fc 100644
>> > --- a/drivers/gpu/drm/drm_atomic.c
>> > +++ b/drivers/gpu/drm/drm_atomic.c
>> > @@ -490,6 +490,8 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>> >&replaced);
>> >state->color_mgmt_changed |= replaced;
>> >return ret;
>> > + } else if (property == config->prop_out_fence_ptr) {
>> > + state->out_fence_ptr = u64_to_user_ptr(val);
>> >} else if (crtc->funcs->atomic_set_property)
>> >return crtc->funcs->atomic_set_property(crtc, state, property,
>> > val);
>> >else
>> > @@ -532,6 +534,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>> >*val = (state->ctm) ? state->ctm->base.id : 0;
>> >else if (property == config->gamma_lut_property)
>> >*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>> > + else if (property == config->prop_out_fence_ptr)
>> > + *val = 0;
>> >else if (crtc->funcs->atomic_get_property)
>> >return crtc->funcs->atomic_get_property(crtc, state, property,
>> > val);
>> >else
>> > @@ -1474,11 +1478,9 @@ EXPORT_SYMBOL(drm_atomic_nonblocking_commit);
>> > */
>> >
>> > static struct drm_pending_vblank_event *create_vblank_event(
>> > - struct drm_device *dev, struct drm_file *file_priv,
>> > - struct fence *fence, uint64_t user_data)
>> > + struct drm_device *dev, uint64_t user_data)
>> > {
>> >struct drm_pending_vblank_event *e = NULL;
>> > - int ret;
>> >
>> >e = kzalloc(sizeof *e, GFP_KERNEL);
>> >if (!e)
>> > @@ -1488,17 +1490,6 @@ static struct drm_pending_vblank_event
>> > *create_vblank_event(
>> >e->event.base.length = sizeof(e->event);
>> >e->event.user_data = user_data;
>> >
>> > - if (file_priv) {
>> > - ret = drm_event_reserve_init(dev, file_priv, &e->base,
>> > - &e->event.base);
>> > - if (ret) {
>> > - kfree(e);
>> > - return NULL;
>> > - }
>> > - }
>> > -
>> > - e->base.fence = fence;
>> > -
>> >return e;
>> > }
>> >
>> > @@ -1603,6 +1594,40 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
>> > }
>> > EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>> >
>> > +static int crtc_setup_out_fence(struct drm_crtc *crtc,
>> > + struct drm_crtc_state *crtc_state,
>> > + struct drm_out_fence_state *fence_state)
>> > +{
>> > + struct fence *fence;
>> > +
>> > + fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
>> > + if (fence_state->fd < 0) {
>> > + return fence_state->fd;
>> > + }
>> > +
>> > + fence_state->out_fence_ptr = crtc_state->out_fence_ptr;
>> > + crtc_state->out_fence_ptr = NULL;
>> > +
>> > + if (put_user(fence_state->fd, fence_state->out_fence_ptr))
>> > + return -EFAULT;
>> > +
>> > + fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>> > + if (!fence)
>> > + return -ENOMEM;
>> > +
>> > + fence_init(fence, &drm_crtc_f
[Bug 93826] 2560x1440 @144Hz graphic glitches and bad refresh rate
https://bugs.freedesktop.org/show_bug.cgi?id=93826 --- Comment #39 from Christian König --- (In reply to Yves W. from comment #38) > means i have to try manually each step and recompile kernel? But that will > take ages :) I don't have that much time. No git bisect will do this automatically for you. Additional to that you don't need to compile every commit in the range. It simply does a binary search, reducing the number of compiles to something close to square root of the number of commits. That's usually quite handleable. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/d6517c9c/attachment.html>
[Bug 98361] [BDW /BXT/SKL/SNB] [Regression] drv_hangman subtests parser fail
https://bugs.freedesktop.org/show_bug.cgi?id=98361 Chris Wilson changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from Chris Wilson --- commit c2fb6b80c951ba45012560e0a6e43aa7e169cb2f Author: Chris Wilson Date: Fri Oct 21 12:55:09 2016 +0100 igt/drv_hangman: Skip format expectations for compressed output -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/99ca608e/attachment.html>
[PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to "tristate".
On Thu, Oct 20, 2016 at 04:44:49PM +0300, Jani Nikula wrote: > On Thu, 20 Oct 2016, Andrzej Hajda wrote: > > Hi Jani, > > > > Forgive me late response. > > > > On 12.10.2016 16:28, Jani Nikula wrote: > >> On Wed, 12 Oct 2016, Emil Velikov wrote: > >>> On 11 October 2016 at 10:33, Jani Nikula > >>> wrote: > On Tue, 11 Oct 2016, "Sun, Jing A" wrote: > > It's needed that DRM Driver module could be removed and reloaded after > > kernel booting on the projects that I have been working on, and I hope > > such module type change could be accepted. Looks like Iwai has similar > > change request as well. Would you please review it and let us know if > > any concerns? > Looking at the Kconfig, selecting CONFIG_DRM_MIPI_DSI is against the > recommendations of Documentation/kbuild/kconfig-language.txt: > > select should be used with care. select will force > a symbol to a value without visiting the dependencies. > By abusing select you are able to select a symbol FOO even > if FOO depends on BAR that is not set. > In general use select only for non-visible symbols > (no prompts anywhere) and for symbols with no dependencies. > That will limit the usefulness but on the other hand avoid > the illegal configurations all over. > > Indeed, you may end up with CONFIG_DRM_MIPI_DSI=y and CONFIG_DRM=m, > which violates DRM_MIPI_DSI dependency on CONFIG_DRM. This is broken and > should be fixed. The suggested patch does *not* fix this issue. > > >>> Jani, git log suggests you as the unfortunate author of the select > >>> DRM_MIPI_DSI/select DRM_PANEL hunks in i915 ;-) > >> /o\ > >> > >> As much as my present self would like to scold my past self for all his > >> mistakes, I have to remind myself that it is the mistakes that have > >> given me invaluable experience that my past self didn't have. I can only > >> hope my future self will have time to fix even a fraction of the > >> mistakes. > >> > >> Anyway, as Andrzej pointed out, all configs that select DRM_MIPI_DSI > >> also depend on DRM, so this problem can't currently occur. Once dsi bus > >> un-registration gets addressed, we can turn DRM_MIPI_DSI into a tristate > >> config (i.e. a loadable module). > > > > There is already patch adding bus unregistration [1]. It was published > > together > > with 'tristate' patch [2], both were created by Takashi few months ago. > > Right. So make that "Once dis bus un-registration gets merged > upstream". ;) Someone volunteering for some review pls? -Daniel > > BR, > Jani. > > > > > > [1]: > > https://lists.freedesktop.org/archives/dri-devel/2016-August/114870.html > > [2]: > > https://lists.freedesktop.org/archives/dri-devel/2016-August/114497.html > > > > Regards > > Andrzej > > > >> > >> BR, > >> Jani. > >> > > > > -- > Jani Nikula, Intel Open Source Technology Center > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] dma-buf: Rename struct fence to dma_fence
On Thu, Oct 20, 2016 at 10:45:34PM +0530, Sumit Semwal wrote: > Hi Chris, > > Thanks for the patch! > > On 20 October 2016 at 17:38, Gustavo Padovan wrote: > > 2016-10-20 Chris Wilson : > > > >> I plan to usurp the short name of struct fence for a core kernel struct, > >> and so I need to rename the specialised fence/timeline for DMA > >> operations to make room. > >> > >> A consensus was reached in > >> https://lists.freedesktop.org/archives/dri-devel/2016-July/113083.html > >> that making clear this fence applies to DMA operations was a good thing. > >> Since then the patch has grown a bit as usage increases, so hopefully it > >> ( > >> ... > >> ) > >> > >> Signed-off-by: Chris Wilson > > Acked-by: Sumit Semwal > > Daniel, > As we agreed, we should perhaps take it via the drm tree? Yeah, see my other reply. After -rc2 is out I'll send a drm-misc and drm-intel pull to Dave, and then I'll ask Chris to regen the patch on top of Dave's drm-next and stuff that into a topic branch. Since we don't yet have users outside of drm that's probably the easiest (and only) way to land this. -Daniel > > > > > Reviewed-by: Gustavo Padovan > > > > Gustavo > > > > Best regards, > Sumit. > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Intel-gfx] i915 monitor hotplug on 4.1/4.4 PREEMPT
On Thu, Oct 20, 2016 at 05:09:52PM +0300, Nicolae Rosia wrote: > Hi, > > On Thu, Oct 20, 2016 at 3:57 PM, David Weinehall wrote: > >> > I get an udev event for unplugging, but there's no event generated for > >> > plugging the monitor back in. > >> > >> Does it work on non-RT? Does it work on v4.8 or v4.9-rc1? > > > > The second one is relevant though. 4.1 is pre-historic, 4.4 is ancient. > I just tested this on v4.8.2-rt2 and it works. > > Any tips on what I should do next, excluding the upgrade :-) ? Very, very likely the answer is "upgrade", because: $ git log --oneline v4.4..v4.8 -- drivers/gpu/drm/i915/ | wc -l 1931 That's not even counting the drm core changes, which also might be relevant. Normally a revert bisect (to figure out the bugfix, hoping that it's just 1 patch and not an entire series) would be an option. But since -rt is still an add-on you'd need to rebase the entire -rt patchset every time. That's impossible. If you have way too much time, and know what you're doing another option might be to backport all of drm (to avoid conflicts between drm core and i915) from 4.8 to 4.4. No guarantee at all it'll work, and we generally keep a few people busy for a while every time we have to do that internally for some product team. So yeah, "upgrade" is very likely is. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/fb-helper: Don't call dirty callback for untouched clips
On Thu, Oct 20, 2016 at 05:00:35PM +0200, Takashi Iwai wrote:
> On Thu, 20 Oct 2016 16:56:04 +0200,
> Ville Syrjälä wrote:
> >
> > On Thu, Oct 20, 2016 at 04:39:52PM +0200, Takashi Iwai wrote:
> > > Since 4.7 kernel, we've seen the error messages like
> > >
> > > kernel: [TTM] Buffer eviction failed
> > > kernel: qxl :00:02.0: object_init failed for (4026540032, 0x0001)
> > > kernel: [drm:qxl_alloc_bo_reserved [qxl]] *ERROR* failed to allocate
> > > VRAM BO
> > >
> > > on QXL when switching and accessing on VT. The culprit was the
> > > generic deferred_io code (qxl driver switched to it since 4.7).
> > > There is a race between the dirty clip update and the call of
> > > callback.
> > >
> > > In drm_fb_helper_dirty(), the dirty clip is updated in the spinlock,
> > > while it kicks off the update worker outside the spinlock. Meanwhile
> > > the update worker clears the dirty clip in the spinlock, too. Thus,
> > > when drm_fb_helper_dirty() is called concurrently, schedule_work() is
> > > called after the clip is cleared in the first worker call.
> > >
> > > This patch addresses it by validating the clip before calling the
> > > dirty fb callback.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98322
> > > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1003298
> > > Fixes: eaa434defaca ('drm/fb-helper: Add fb_deferred_io support')
> > > Cc:
> > > Signed-off-by: Takashi Iwai
> > > ---
> > > drivers/gpu/drm/drm_fb_helper.c | 13 +
> > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > b/drivers/gpu/drm/drm_fb_helper.c
> > > index 03414bde1f15..d790d205129e 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -636,15 +636,20 @@ static void drm_fb_helper_dirty_work(struct
> > > work_struct *work)
> > > dirty_work);
> > > struct drm_clip_rect *clip = &helper->dirty_clip;
> > > struct drm_clip_rect clip_copy;
> > > + bool dirty;
> > > unsigned long flags;
> > >
> > > spin_lock_irqsave(&helper->dirty_lock, flags);
> > > - clip_copy = *clip;
> > > - clip->x1 = clip->y1 = ~0;
> > > - clip->x2 = clip->y2 = 0;
> > > + dirty = (clip->x1 < clip->x2 && clip->y1 < clip->y2);
> > > + if (dirty) {
> > > + clip_copy = *clip;
> > > + clip->x1 = clip->y1 = ~0;
> > > + clip->x2 = clip->y2 = 0;
> > > + }
> > > spin_unlock_irqrestore(&helper->dirty_lock, flags);
> > >
> > > - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> > > + if (dirty)
> >
> > Could do it the other way too, ie. just make the copy, and then check the
> > copy (can be done after dropping the lock even). Would avoid having to
> > add the 'dirty' variable.
>
> Sounds good. Let me try...
Another thing: How do we prevent userspace from doing the same, i.e.
submitting an empty rectangle? Do we need to patch up
drm_mode_dirtyfb_ioctl too? Not much point if we fix this bug in the fb
helpers and leave the barn door wide open for userspace to oops drivers
;-)
-Daniel
>
>
> Takashi
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[patch] drm/i915/gvt: cleanup a type issue in submit_context()
On 2016.10.21 13:27:22 +0300, Dan Carpenter wrote:
> On Fri, Oct 21, 2016 at 04:27:38PM +0800, Zhenyu Wang wrote:
> > On 2016.10.21 11:06:01 +0300, Dan Carpenter wrote:
> > > submit_context() should return negative error codes and zero on success
> > > but by mistake we made it a bool. I've changed it to int. Also I made
> > > it static because this isn't referenced in any other files.
> > >
> > > This doesn't affect runtime because the return is eventually propogated
> > > to elsp_mmio_write() where it is ignored.
> > >
> > > Signed-off-by: Dan Carpenter
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/execlist.c
> > > b/drivers/gpu/drm/i915/gvt/execlist.c
> > > index c50a3d1a5131..bc69ba1842ea 100644
> > > --- a/drivers/gpu/drm/i915/gvt/execlist.c
> > > +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> > > @@ -616,7 +616,7 @@ static int prepare_mm(struct intel_vgpu_workload
> > > *workload)
> > > (list_empty(q) ? NULL : container_of(q->prev, \
> > > struct intel_vgpu_workload, list))
> > >
> > > -bool submit_context(struct intel_vgpu *vgpu, int ring_id,
> > > +static int submit_context(struct intel_vgpu *vgpu, int ring_id,
> > > struct execlist_ctx_descriptor_format *desc,
> > > bool emulate_schedule_in)
> > > {
> >
> > Dan, this has been fixed in our recent pull, should hit linux-next very
> > soon.
> >
>
> Could you review elsp_mmio_write() as well? Should we be doing
> something with that return value?
>
yeah, ignore return value is not good, will fix that. Thanks!
Zhi, could you double check error path for elsp write?
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/8409966d/attachment.sig>
[RFC] drm: add hint to userspace about whether a plane can scale
On Fri, Oct 21, 2016 at 09:58:45AM +0100, Brian Starkey wrote:
> Hi Rob,
>
> On Thu, Oct 20, 2016 at 04:17:14PM -0400, Rob Clark wrote:
> > When you have a mix of planes that can scale and those that cannot
> > scale, userspace really wants to have some hint to know which planes
> > can definitely not scale so it knows to assign them to unscaled layers.
> > I don't think it is fully possible to describe scaling constraints in
> > a generic way, so I don't think it is even worth trying, so this is
> > not a substitute for atomic TESTONLY step, but it does reduce the
> > search-space for userspace. In the common case, most layers will not
> > be scaled so knowing the best planes to pick for those layers is
> > useful.
>
> Somewhat related to what Daniel mentioned on IRC about driver
> consistency - how about making it "cannot_scale". This is then opt-in
> for drivers, and should mean userspace can always trust it if it's
> set.
>
> i.e. if cannot_scale == false, userspace can give it a shot, but if
> cannot_scale == true it should never bother.
>
> Either way, even with a device-specific planner we would want this
> hint to manage different HW versions so I'm in favour. But...
I think first thing we should do is add some backoff heuristics to
drm_hwcomposer to try the same plane also with a non-scaled surface (after
having tried it with a scaled one). That would probably be as effective as
adding "can_scale", but with the upshot that we're not at the mercy of
drivers exposing it correctly.
I'm always vary when we have the same limit checks 2 (in this case once in
atomic_check, and once in the code that registers the property). And I'd
like to avoid that as much as possible. We could avoid this by making the
can_scale property mandatory, and enforcing it in the drm core. I.e. if
it's not set, we reject scaled planes. That should give some decent
motivation for driver writers to update them correctly. But without such a
self-consistency check I don't really like this. It would be akin to
adding "can_rotate" besides the "rotation" prop, and allowing drivers to
botch things up and not register stuff consistently.
-Daniel
>
> > ---
> > drivers/gpu/drm/drm_crtc.c| 1 +
> > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 1 +
> > include/drm/drm_crtc.h| 2 ++
> > include/uapi/drm/drm_mode.h | 3 +++
> > 4 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index b4b973f..d7e0e0d 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -2389,6 +2389,7 @@ int drm_mode_getplane(struct drm_device *dev, void
> > *data,
> > plane_resp->plane_id = plane->base.id;
> > plane_resp->possible_crtcs = plane->possible_crtcs;
> > plane_resp->gamma_size = 0;
> > + plane_resp->can_scale = plane->can_scale;
> >
> > /*
> > * This ioctl is called twice, once to determine how much space is
> > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> > b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> > index 692c888..2061c83 100644
> > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> > @@ -908,6 +908,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device
> > *dev,
> > mdp5_plane->pipe = pipe;
> > mdp5_plane->name = pipe2name(pipe);
> > mdp5_plane->caps = caps;
> > + plane->can_scale = !!(caps & MDP_PIPE_CAP_SCALE);
> >
> > mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats,
> > ARRAY_SIZE(mdp5_plane->formats),
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index d74d47a..6e290b6 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -1679,6 +1679,7 @@ enum drm_plane_type {
> > * @format_types: array of formats supported by this plane
> > * @format_count: number of formats supported
> > * @format_default: driver hasn't supplied supported formats for the plane
> > + * @can_scale: a hint to userspace that this plane can (or cannot) scale
> > * @crtc: currently bound CRTC
> > * @fb: currently bound fb
> > * @old_fb: Temporary tracking of the old fb while a modeset is ongoing.
> > Used by
> > @@ -1710,6 +1711,7 @@ struct drm_plane {
> > uint32_t *format_types;
> > unsigned int format_count;
> > bool format_default;
> > + bool can_scale;
> >
> > struct drm_crtc *crtc;
> > struct drm_framebuffer *fb;
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index ce71ad5..5bf9361 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -191,6 +191,9 @@ struct drm_mode_get_plane {
> >
> > __u32 count_format_types;
> > __u64 format_type_ptr;
> > +
> > + __u32 can_scale;
>
> ... 32 bits for a bool does seem a bit wasteful.
>
> Thanks,
> Brian
>
> > + __u32 pad;
> > };
> >
> > struct drm_mode_get_plane_res {
> > --
> > 2.7.4
> >
> > ___
[Intel-gfx] [PATCH v2] drm: Pass along the hotplug connector to the uevent
On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote:
> If we know which connector was plugged/unplugged or
> connected/disconnected, we can pass that information along to userspace
> inside the uevent to reduce the amount of work userspace has to perform
> after the event (i.e. instead of looking over all connectors, it can
> just reprobe the affected one).
>
> v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> need the force.
>
> Signed-off-by: Chris Wilson
> Cc: Villle Syrjälä
> Cc: Manasi Navare
Nothing is preventing multiple connectors to change state at the same
time. Or at least almost the same time, e.g. yank an entire dp mst chain.
I think we need something that works per-connector, so either send out
uevents for one that changed individually. Or go with the epoche counter
idea. The later has the upside that it disconnects probing from reporting:
Sometimes only deep down in the driver's probe function do we realize that
something changed (e.g. different edid, or different dpcd). With a helper
to increment the epoche there would be no need to wire the hotplug status
through the entire callchain.
To give us the same speed-up benefits like this here the only thing we'd
need to do is make sure reading a read-only (i.e. not userspace setable
prop) doesn't take any heavyweight locks. That should be easy to achieve.
Of course that leaves us with num_connector ioctl calls, but that should
be acceptable.
And it's an uabi change either way.
-Daniel
> ---
> drivers/gpu/drm/drm_probe_helper.c | 10 ++
> drivers/gpu/drm/drm_sysfs.c| 19 +++
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/i915/intel_dp.c| 3 ++-
> drivers/gpu/drm/i915/intel_dp_mst.c| 2 +-
> drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
> drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c| 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c| 2 +-
> include/drm/drmP.h | 3 ++-
> include/drm/drm_crtc_helper.h | 3 ++-
> 13 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index f6b64d7d3528..8790ee35a7cd 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -347,6 +347,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
> /**
> * drm_kms_helper_hotplug_event - fire off KMS hotplug events
> * @dev: drm_device whose connector state changed
> + * @connector: connector on which the status changed, if any
> *
> * This function fires off the uevent for userspace and also calls the
> * output_poll_changed function, which is most commonly used to inform the
> fbdev
> @@ -360,10 +361,11 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
> * This function must be called from process context with no mode
> * setting locks held.
> */
> -void drm_kms_helper_hotplug_event(struct drm_device *dev)
> +void drm_kms_helper_hotplug_event(struct drm_device *dev,
> + struct drm_connector *connector)
> {
> /* send a uevent + call fbdev */
> - drm_sysfs_hotplug_event(dev);
> + drm_sysfs_hotplug_event(dev, connector);
> if (dev->mode_config.funcs->output_poll_changed)
> dev->mode_config.funcs->output_poll_changed(dev);
> }
> @@ -444,7 +446,7 @@ static void output_poll_execute(struct work_struct *work)
>
> out:
> if (changed)
> - drm_kms_helper_hotplug_event(dev);
> + drm_kms_helper_hotplug_event(dev, NULL);
>
> if (repoll)
> schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
> @@ -578,7 +580,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
> mutex_unlock(&dev->mode_config.mutex);
>
> if (changed)
> - drm_kms_helper_hotplug_event(dev);
> + drm_kms_helper_hotplug_event(dev, NULL);
>
> return changed;
> }
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 9a37196c1bf1..c8f46055e2d0 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -280,7 +280,7 @@ int drm_sysfs_connector_add(struct drm_connector
> *connector)
> }
>
> /* Let userspace know we have a new connector */
> - drm_sysfs_hotplug_event(dev);
> + drm_sysfs_hotplug_event(dev, connector);
>
> return 0;
> }
> @@ -312,19 +312,30 @@ void drm_sysfs_connector_remove(struct drm_connector
> *connector)
> /**
> * drm_sysfs_hotplug_event - generate a DRM uevent
> * @dev: DRM device
> + * @connector: the DRM connector, if any
> *
> * Send a uevent for the DRM device specified by @dev. Currently we only
> * set HOTPLUG=1 in the uevent environment, but this could be expanded to
> * deal w
[PATCH] drm/fb-helper: Don't call dirty callback for untouched clips
On Fri, 21 Oct 2016 14:30:32 +0200,
Daniel Vetter wrote:
>
> On Thu, Oct 20, 2016 at 05:00:35PM +0200, Takashi Iwai wrote:
> > On Thu, 20 Oct 2016 16:56:04 +0200,
> > Ville Syrjälä wrote:
> > >
> > > On Thu, Oct 20, 2016 at 04:39:52PM +0200, Takashi Iwai wrote:
> > > > Since 4.7 kernel, we've seen the error messages like
> > > >
> > > > kernel: [TTM] Buffer eviction failed
> > > > kernel: qxl :00:02.0: object_init failed for (4026540032,
> > > > 0x0001)
> > > > kernel: [drm:qxl_alloc_bo_reserved [qxl]] *ERROR* failed to allocate
> > > > VRAM BO
> > > >
> > > > on QXL when switching and accessing on VT. The culprit was the
> > > > generic deferred_io code (qxl driver switched to it since 4.7).
> > > > There is a race between the dirty clip update and the call of
> > > > callback.
> > > >
> > > > In drm_fb_helper_dirty(), the dirty clip is updated in the spinlock,
> > > > while it kicks off the update worker outside the spinlock. Meanwhile
> > > > the update worker clears the dirty clip in the spinlock, too. Thus,
> > > > when drm_fb_helper_dirty() is called concurrently, schedule_work() is
> > > > called after the clip is cleared in the first worker call.
> > > >
> > > > This patch addresses it by validating the clip before calling the
> > > > dirty fb callback.
> > > >
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98322
> > > > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1003298
> > > > Fixes: eaa434defaca ('drm/fb-helper: Add fb_deferred_io support')
> > > > Cc:
> > > > Signed-off-by: Takashi Iwai
> > > > ---
> > > > drivers/gpu/drm/drm_fb_helper.c | 13 +
> > > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > > b/drivers/gpu/drm/drm_fb_helper.c
> > > > index 03414bde1f15..d790d205129e 100644
> > > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > > @@ -636,15 +636,20 @@ static void drm_fb_helper_dirty_work(struct
> > > > work_struct *work)
> > > > dirty_work);
> > > > struct drm_clip_rect *clip = &helper->dirty_clip;
> > > > struct drm_clip_rect clip_copy;
> > > > + bool dirty;
> > > > unsigned long flags;
> > > >
> > > > spin_lock_irqsave(&helper->dirty_lock, flags);
> > > > - clip_copy = *clip;
> > > > - clip->x1 = clip->y1 = ~0;
> > > > - clip->x2 = clip->y2 = 0;
> > > > + dirty = (clip->x1 < clip->x2 && clip->y1 < clip->y2);
> > > > + if (dirty) {
> > > > + clip_copy = *clip;
> > > > + clip->x1 = clip->y1 = ~0;
> > > > + clip->x2 = clip->y2 = 0;
> > > > + }
> > > > spin_unlock_irqrestore(&helper->dirty_lock, flags);
> > > >
> > > > - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> > > > + if (dirty)
> > >
> > > Could do it the other way too, ie. just make the copy, and then check the
> > > copy (can be done after dropping the lock even). Would avoid having to
> > > add the 'dirty' variable.
> >
> > Sounds good. Let me try...
>
> Another thing: How do we prevent userspace from doing the same, i.e.
> submitting an empty rectangle? Do we need to patch up
> drm_mode_dirtyfb_ioctl too? Not much point if we fix this bug in the fb
> helpers and leave the barn door wide open for userspace to oops drivers
> ;-)
OK, then how about adding a helper to call the dirty callback with a
sanity check like below?
Takashi
---
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 03414bde1f15..7bef4d642511 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -644,7 +644,7 @@ static void drm_fb_helper_dirty_work(struct work_struct
*work)
clip->x2 = clip->y2 = 0;
spin_unlock_irqrestore(&helper->dirty_lock, flags);
- helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
+ drm_framebuffer_update_dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
}
/**
diff --git a/drivers/gpu/drm/drm_framebuffer.c
b/drivers/gpu/drm/drm_framebuffer.c
index 398efd67cb93..0817a0b607c5 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -529,6 +529,25 @@ int drm_mode_getfb(struct drm_device *dev,
}
/**
+ * blah blah
+ */
+int drm_framebuffer_update_dirty(struct drm_framebuffer *fb,
+struct drm_file *file_priv,
+unsigned flags, unsigned color,
+struct drm_clip_rect *clips,
+unsigned num_clips)
+{
+ if (!fb->funcs->dirty)
+ return -ENOSYS;
+ if (!num_clips)
+ return 0;
+ if (clips->x1 >= clips->x2 || clips->y1 >= clips->y2)
+ return 0;
+ return fb->funcs->dirty(fb, file_priv, flags, color, clips, num_clips);
+}
+EXPORT_SYMBOL
[PATCH v5 1/4] drm/fence: add in-fences support
On Thu, Oct 20, 2016 at 12:50:02PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> There is now a new property called FENCE_FD attached to every plane
> state that receives the sync_file fd from userspace via the atomic commit
> IOCTL.
>
> The fd is then translated to a fence (that may be a fence_collection
> subclass or just a normal fence) and then used by DRM to fence_wait() for
> all fences in the sync_file to signal. So it only commits when all
> framebuffers are ready to scanout.
>
> v2: Comments by Daniel Vetter:
> - remove set state->fence = NULL in destroy phase
> - accept fence -1 as valid and just return 0
> - do not call fence_get() - sync_file_fences_get() already calls it
> - fence_put() if state->fence is already set, in case userspace
> set the property more than once.
>
> v3: WARN_ON if fence is set but state has no FB
>
> v4: Comment from Maarten Lankhorst
> - allow set fence with no related fb
>
> v5: rename FENCE_FD to IN_FENCE_FD
>
> Signed-off-by: Gustavo Padovan
Since you rename plane_state->fence to in_fence (why exactly?) this breaks
compilation for imx and msm. Also there's the question of which fence
should win, if there's both explicit and implicit fences. I think the best
way to solve that is by adding a helper to set the implicit fence, which
drivers are supposed to call. That helper only sets the fence if there's
not yet an explicit fence attached. New sequence would be:
- patch1: roll out that helper (e.g. drm_atomic_plane_state_set_fence or
whatever) for all drivers using it.
- patch2 (this one here): add explicit fences and change the helper to
cooporate correctly.
> ---
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/drm_atomic.c| 14 ++
> drivers/gpu/drm/drm_atomic_helper.c | 13 +++--
> drivers/gpu/drm/drm_crtc.c | 6 ++
> drivers/gpu/drm/drm_plane.c | 1 +
> include/drm/drm_crtc.h | 5 +
> include/drm/drm_plane.h | 4 ++--
> 7 files changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 483059a..43cb33d 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -12,6 +12,7 @@ menuconfig DRM
> select I2C
> select I2C_ALGOBIT
> select DMA_SHARED_BUFFER
> + select SYNC_FILE
> help
> Kernel-level support for the Direct Rendering Infrastructure (DRI)
> introduced in XFree86 4.0. If you say Y here, you need to select
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5dd7054..b3284b2 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
>
> #include "drm_crtc_internal.h"
>
> @@ -686,6 +687,17 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> drm_atomic_set_fb_for_plane(state, fb);
> if (fb)
> drm_framebuffer_unreference(fb);
> + } else if (property == config->prop_in_fence_fd) {
> + if (U642I64(val) == -1)
> + return 0;
> +
> + if (state->in_fence)
> + fence_put(state->in_fence);
> +
> + state->in_fence = sync_file_get_fence(val);
> + if (!state->in_fence)
> + return -EINVAL;
> +
> } else if (property == config->prop_crtc_id) {
> struct drm_crtc *crtc = drm_crtc_find(dev, val);
> return drm_atomic_set_crtc_for_plane(state, crtc);
> @@ -745,6 +757,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>
> if (property == config->prop_fb_id) {
> *val = (state->fb) ? state->fb->base.id : 0;
> + } else if (property == config->prop_in_fence_fd) {
> + *val = -1;
> } else if (property == config->prop_crtc_id) {
> *val = (state->crtc) ? state->crtc->base.id : 0;
> } else if (property == config->prop_crtc_x) {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 07b432f..73464b1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1031,22 +1031,20 @@ int drm_atomic_helper_wait_for_fences(struct
> drm_device *dev,
> if (!pre_swap)
> plane_state = plane->state;
>
> - if (!plane_state->fence)
> + if (!plane_state->in_fence)
> continue;
>
> - WARN_ON(!plane_state->fb);
> -
> /*
>* If waiting for fences pre-swap (ie: nonblock), userspace can
>* still interrupt the operation. Instead of blocking until the
>* timer expires, make the wait interruptible.
>*/
> - ret = fence_wait(plane_state->fence, pre_swap);
> +
[PATCH v2] drm/fb-helper: Don't call dirty callback for untouched clips
On Thu, Oct 20, 2016 at 05:05:30PM +0200, Takashi Iwai wrote:
> Since 4.7 kernel, we've seen the error messages like
>
> kernel: [TTM] Buffer eviction failed
> kernel: qxl :00:02.0: object_init failed for (4026540032, 0x0001)
> kernel: [drm:qxl_alloc_bo_reserved [qxl]] *ERROR* failed to allocate VRAM BO
>
> on QXL when switching and accessing on VT. The culprit was the
> generic deferred_io code (qxl driver switched to it since 4.7).
> There is a race between the dirty clip update and the call of
> callback.
>
> In drm_fb_helper_dirty(), the dirty clip is updated in the spinlock,
> while it kicks off the update worker outside the spinlock. Meanwhile
> the update worker clears the dirty clip in the spinlock, too. Thus,
> when drm_fb_helper_dirty() is called concurrently, schedule_work() is
> called after the clip is cleared in the first worker call.
>
> This patch addresses it by validating the clip before calling the
> dirty fb callback.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98322
> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1003298
> Fixes: eaa434defaca ('drm/fb-helper: Add fb_deferred_io support')
> Cc:
> Signed-off-by: Takashi Iwai
Reviewed-by: Ville Syrjälä
> ---
> v1->v2: simplified the code as suggested by Ville
>
> drivers/gpu/drm/drm_fb_helper.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 03414bde1f15..aae7df01864d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -644,7 +644,9 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
> clip->x2 = clip->y2 = 0;
> spin_unlock_irqrestore(&helper->dirty_lock, flags);
>
> - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> + /* call dirty callback only when it has been really touched */
> + if (clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2)
> + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> }
>
> /**
> --
> 2.10.1
--
Ville Syrjälä
Intel OTC
[PATCH v5 2/4] drm/fence: release fence reference when canceling event
On Thu, Oct 20, 2016 at 12:50:03PM -0200, Gustavo Padovan wrote: > From: Gustavo Padovan > > If the event gets canceled we also need to put away the fence > reference it holds. > > Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter I've broken my local dim scripts right now, so can't apply ;-) -Daniel > --- > drivers/gpu/drm/drm_fops.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c > index e84faec..8bed5f4 100644 > --- a/drivers/gpu/drm/drm_fops.c > +++ b/drivers/gpu/drm/drm_fops.c > @@ -663,6 +663,10 @@ void drm_event_cancel_free(struct drm_device *dev, > list_del(&p->pending_link); > } > spin_unlock_irqrestore(&dev->event_lock, flags); > + > + if (p->fence) > + fence_put(p->fence); > + > kfree(p); > } > EXPORT_SYMBOL(drm_event_cancel_free); > -- > 2.5.5 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v5 3/4] drm/fence: add fence timeline to drm_crtc
On Thu, Oct 20, 2016 at 12:50:04PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Create one timeline context for each CRTC to be able to handle out-fences
> and signal them. It adds a few members to struct drm_crtc: fence_context,
> where we store the context we get from fence_context_alloc(), the
> fence seqno and the fence lock, that we pass in fence_init() to be
> used by the fence.
>
> v2: Comment by Daniel Stone:
> - add BUG_ON() to fence_to_crtc() macro
>
> v3: Comment by Ville Syrjälä
> - Use more meaningful name as crtc timeline name
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/gpu/drm/drm_crtc.c | 31 +++
> include/drm/drm_crtc.h | 19 +++
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index fcb6453..b99090f 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -165,6 +165,32 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
> #endif
> }
>
> +static const char *drm_crtc_fence_get_driver_name(struct fence *fence)
> +{
> + struct drm_crtc *crtc = fence_to_crtc(fence);
> +
> + return crtc->dev->driver->name;
> +}
> +
> +static const char *drm_crtc_fence_get_timeline_name(struct fence *fence)
> +{
> + struct drm_crtc *crtc = fence_to_crtc(fence);
> +
> + return crtc->timeline_name;
> +}
> +
> +static bool drm_crtc_fence_enable_signaling(struct fence *fence)
> +{
> + return true;
> +}
> +
> +const struct fence_ops drm_crtc_fence_ops = {
> + .get_driver_name = drm_crtc_fence_get_driver_name,
> + .get_timeline_name = drm_crtc_fence_get_timeline_name,
> + .enable_signaling = drm_crtc_fence_enable_signaling,
> + .wait = fence_default_wait,
> +};
> +
> /**
> * drm_crtc_init_with_planes - Initialise a new CRTC object with
> *specified primary and cursor planes.
> @@ -222,6 +248,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
> struct drm_crtc *crtc,
> return -ENOMEM;
> }
>
> + crtc->fence_context = fence_context_alloc(1);
> + spin_lock_init(&crtc->fence_lock);
> + snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
> + "drm_crtc-%d", crtc->base.id);
> +
> crtc->base.properties = &crtc->properties;
>
> list_add_tail(&crtc->head, &config->crtc_list);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 279132e..657a33a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -32,6 +32,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> @@ -618,6 +620,9 @@ struct drm_crtc_funcs {
> * @gamma_store: gamma ramp values
> * @helper_private: mid-layer private data
> * @properties: property tracking for this CRTC
> + * @fence_context: context for fence signalling
> + * @fence_lock: fence lock for the fence context
> + * @fence_seqno: seqno variable to create fences
For new stuff I much prefer in-line kerneldoc with structures, keeps the
comments much closer to the code ...
> *
> * Each CRTC may have one or more connectors associated with it. This
> structure
> * allows the CRTC to be controlled.
> @@ -726,8 +731,22 @@ struct drm_crtc {
>*/
> struct drm_crtc_crc crc;
> #endif
> +
> + /* fence timelines info for DRM out-fences */
... and avoids duplicated comments like this one here.
Otherwise lgtm.
-Daniel
> + unsigned int fence_context;
> + spinlock_t fence_lock;
> + unsigned long fence_seqno;
> + char timeline_name[32];
> };
>
> +extern const struct fence_ops drm_crtc_fence_ops;
> +
> +static inline struct drm_crtc *fence_to_crtc(struct fence *fence)
> +{
> + BUG_ON(fence->ops != &drm_crtc_fence_ops);
> + return container_of(fence->lock, struct drm_crtc, fence_lock);
> +}
> +
> /**
> * struct drm_mode_set - new values for a CRTC config change
> * @fb: framebuffer to use for new config
> --
> 2.5.5
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v5 4/4] drm/fence: add out-fences support
On Thu, Oct 20, 2016 at 10:15:20PM +0300, Ville Syrjälä wrote: > On Thu, Oct 20, 2016 at 07:34:44PM +0300, Ville Syrjälä wrote: > > On Thu, Oct 20, 2016 at 01:55:38PM -0200, Gustavo Padovan wrote: > > > 2016-10-20 Ville Syrjälä : > > > > > > > On Thu, Oct 20, 2016 at 12:50:05PM -0200, Gustavo Padovan wrote: > > > > > From: Gustavo Padovan > > > > > > > > > > Support DRM out-fences by creating a sync_file with a fence for each > > > > > CRTC > > > > > that sets the OUT_FENCE_PTR property. > > > > > > > > I still maintain the out fence should also be per fb (well, per plane > > > > since we can't add it to fbs). Otherwise it's not going to be at all > > > > useful if you do fps>vrefresh and don't include the same set of planes > > > > in every atomic ioctl, eg. if you only ever include ones that are > > > > somehow dirty. > > > > > > How would the kernel signal these dirty planes then? Right now we signal > > > at the vblank. > > > > So if I think about it in terms of the previous fbs something like this > > comes to mind: > > > > starting point: > > plane a, fb 0 > > plane b, fb 1 > > > > ioctl: > > plane a, fb 2, fence A > > plane b, fb 3, fence B > > > > ioctl: > > plane a, fb 4, fence C > > -> fb 2 can be reused, so fence C should signal immediately? > > > > vblank: > > -> fb 0,1 can be reused, so fence A,B signal? > > > > It feels a bit wonky since the fence is effectively associated with the > > previous fb after the previous fb was already submitted to the kernel. > > One might assume fence A to be the one signalled early, but that would > > give the impression that fb 0 would be free for reuse when it's not. > > OK, so after hashing this out on irc with Gustavo and Brian, we came > to the conclusion that the per-crtc out fence should be sufficient > after all. > > The way it can work is that the first ioctl will return the fence, > doesn't really matter how many of the planes are involved in this > ioctl. Subsequent ioctls that manage to get in before the next > vblank will get back a -1 as the fence, even if the set if planes > involved is not the same as in the first ioctl. From the -1 > userspace can tell what happened, and can then consult its own > records to see which still pending flips were overwritten, and > thus knows which buffers can be reused immediately. > > If userspace plans on sending out the now free buffers to someone > else accompanied by a fence, it can just create an already signalled > fence to send instead of sending the fence it got back from the > atomic ioctl since that one is still associated with the original > scanout buffers. > > The fence returned by the atomic ioctl will only signal after the > vblank, at which point userspace will obviously know that the > orginal scanout buffers are now also ready for reuse, and that > the last buffer submitted for each plane is now being actively > scanned out. So if userspace wants to send out some of the > original buffers to someone else, it can send along the fence > returned from the atomic ioctl. > > So requires a bit more work from userspace perhaps. But obviously > it only has to do this if it plans on submitting multiple operations > per frame. > > So a slightly expanded version of my previous example might look like: > starting point: > plane a, fb 0 > plane b, fb 1 > > ioctl: > crtc: fence A > plane a, fb 2 > plane b, fb 3 > > ioctl: > crtc: fence -1 > plane a, fb 4 > -> the previously submitted fb for plane a (fb 2) can be reused > > ioctl: > crtc: fence -1 > plane a, fb 5 > -> the previously submitted fb for plane a (fb 4) can be reused > > vblank: > -> fence A signals, fb 0,1 can be reused > fb 3 and 5 being scanned out now > > > We also had a quick side track w.r.t. variable refresh rate displays, > and I think the conclusion was that there the vblank may just happen > sooner or later. Nothing else should change. What's unclear is how > the refresh rate would be controlled. The two obvious options are > explicit control, and automagic based on the submit rate. But that's > a separate topic entirely. Thanks a lot for doing this discussion and the detailed write-up. Imo we should bake this into the kerneldoc, as uabi documentation examples. Gustavo, can you pls include this? I'd put it into the kerneldoc for @out_fence, with inline struct comments we can be rather excessive in their lenght ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/fb-helper: Don't call dirty callback for untouched clips
On Fri, Oct 21, 2016 at 02:48:49PM +0200, Takashi Iwai wrote:
> On Fri, 21 Oct 2016 14:30:32 +0200,
> Daniel Vetter wrote:
> >
> > On Thu, Oct 20, 2016 at 05:00:35PM +0200, Takashi Iwai wrote:
> > > On Thu, 20 Oct 2016 16:56:04 +0200,
> > > Ville Syrjälä wrote:
> > > >
> > > > On Thu, Oct 20, 2016 at 04:39:52PM +0200, Takashi Iwai wrote:
> > > > > Since 4.7 kernel, we've seen the error messages like
> > > > >
> > > > > kernel: [TTM] Buffer eviction failed
> > > > > kernel: qxl :00:02.0: object_init failed for (4026540032,
> > > > > 0x0001)
> > > > > kernel: [drm:qxl_alloc_bo_reserved [qxl]] *ERROR* failed to allocate
> > > > > VRAM BO
> > > > >
> > > > > on QXL when switching and accessing on VT. The culprit was the
> > > > > generic deferred_io code (qxl driver switched to it since 4.7).
> > > > > There is a race between the dirty clip update and the call of
> > > > > callback.
> > > > >
> > > > > In drm_fb_helper_dirty(), the dirty clip is updated in the spinlock,
> > > > > while it kicks off the update worker outside the spinlock. Meanwhile
> > > > > the update worker clears the dirty clip in the spinlock, too. Thus,
> > > > > when drm_fb_helper_dirty() is called concurrently, schedule_work() is
> > > > > called after the clip is cleared in the first worker call.
> > > > >
> > > > > This patch addresses it by validating the clip before calling the
> > > > > dirty fb callback.
> > > > >
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98322
> > > > > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1003298
> > > > > Fixes: eaa434defaca ('drm/fb-helper: Add fb_deferred_io support')
> > > > > Cc:
> > > > > Signed-off-by: Takashi Iwai
> > > > > ---
> > > > > drivers/gpu/drm/drm_fb_helper.c | 13 +
> > > > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > > > b/drivers/gpu/drm/drm_fb_helper.c
> > > > > index 03414bde1f15..d790d205129e 100644
> > > > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > > > @@ -636,15 +636,20 @@ static void drm_fb_helper_dirty_work(struct
> > > > > work_struct *work)
> > > > > dirty_work);
> > > > > struct drm_clip_rect *clip = &helper->dirty_clip;
> > > > > struct drm_clip_rect clip_copy;
> > > > > + bool dirty;
> > > > > unsigned long flags;
> > > > >
> > > > > spin_lock_irqsave(&helper->dirty_lock, flags);
> > > > > - clip_copy = *clip;
> > > > > - clip->x1 = clip->y1 = ~0;
> > > > > - clip->x2 = clip->y2 = 0;
> > > > > + dirty = (clip->x1 < clip->x2 && clip->y1 < clip->y2);
> > > > > + if (dirty) {
> > > > > + clip_copy = *clip;
> > > > > + clip->x1 = clip->y1 = ~0;
> > > > > + clip->x2 = clip->y2 = 0;
> > > > > + }
> > > > > spin_unlock_irqrestore(&helper->dirty_lock, flags);
> > > > >
> > > > > - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> > > > > + if (dirty)
> > > >
> > > > Could do it the other way too, ie. just make the copy, and then check
> > > > the
> > > > copy (can be done after dropping the lock even). Would avoid having to
> > > > add the 'dirty' variable.
> > >
> > > Sounds good. Let me try...
> >
> > Another thing: How do we prevent userspace from doing the same, i.e.
> > submitting an empty rectangle? Do we need to patch up
> > drm_mode_dirtyfb_ioctl too? Not much point if we fix this bug in the fb
> > helpers and leave the barn door wide open for userspace to oops drivers
> > ;-)
>
> OK, then how about adding a helper to call the dirty callback with a
> sanity check like below?
>
>
> Takashi
>
> ---
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 03414bde1f15..7bef4d642511 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -644,7 +644,7 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
> clip->x2 = clip->y2 = 0;
> spin_unlock_irqrestore(&helper->dirty_lock, flags);
>
> - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> + drm_framebuffer_update_dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> }
>
> /**
> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> b/drivers/gpu/drm/drm_framebuffer.c
> index 398efd67cb93..0817a0b607c5 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -529,6 +529,25 @@ int drm_mode_getfb(struct drm_device *dev,
> }
>
> /**
> + * blah blah
> + */
> +int drm_framebuffer_update_dirty(struct drm_framebuffer *fb,
> + struct drm_file *file_priv,
> + unsigned flags, unsigned color,
> + struct drm_clip_rect *clips,
> + unsigned num_clips)
> +{
> + if (!fb->funcs->dirty)
> +
[Bug 98372] UBSAN in ../drivers/gpu/drm/drm_modes.c:325:49
https://bugs.freedesktop.org/show_bug.cgi?id=98372 Bug ID: 98372 Summary: UBSAN in ../drivers/gpu/drm/drm_modes.c:325:49 Product: DRI Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/other Assignee: dri-devel at lists.freedesktop.org Reporter: marxin.liska at gmail.com Running $uname -a Linux linux-h8g6 4.9.0-rc1-2-syzkaller #1 SMP PREEMPT Mon Oct 17 19:37:55 UTC 2016 (55c3dd5) x86_64 x86_64 x86_64 GNU/Linux with enabled UBSAN (built by GCC 7.0) in qemu, I reached following error: [ 48.723720] UBSAN: Undefined behaviour in ../drivers/gpu/drm/drm_modes.c:325:49 [ 48.726943] signed integer overflow: [ 48.728503] 2240 * 100 cannot be represented in type 'int' -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/da7aa9ff/attachment.html>
[Bug 98372] UBSAN in ../drivers/gpu/drm/drm_modes.c:325:49
] -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/5e9efde8/attachment.html>
[PATCH v5 4/4] drm/fence: add out-fences support
On Fri, Oct 21, 2016 at 11:55:52AM +0100, Brian Starkey wrote:
> On Thu, Oct 20, 2016 at 06:30:17PM -0200, Gustavo Padovan wrote:
> > 2016-10-20 Brian Starkey :
> > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > > index 657a33a..b898604 100644
> > > > --- a/include/drm/drm_crtc.h
> > > > +++ b/include/drm/drm_crtc.h
> > > > @@ -165,6 +165,8 @@ struct drm_crtc_state {
> > > > struct drm_property_blob *ctm;
> > > > struct drm_property_blob *gamma_lut;
> > > >
> > > > + u64 __user *out_fence_ptr;
> > > > +
> > >
> > > I'm somewhat not convinced about stashing a __user pointer in the
> > > CRTC atomic state. I know it gets cleared before the driver sees it,
> > > but if anything I think that hints that this isn't the right place to
> > > store it.
> > >
> > > I've been trying to think of other ways to get from a property to
> > > something drm_mode_atomic_ioctl can use - maybe we can store some
> > > stuff in drm_atomic_state which only lives for the length of the ioctl
> > > call and put this pointer in there.
> >
> > The drm_atomic_state is still visible by the drivers. Between there and
> > crtc_state, I would keep in the crtc_state for now.
> >
>
> Mm, yeah I suppose they could get to it if they went looking for it
> in ->atomic_set_property or something, but I was thinking of freeing
> it before the drm_atomic_commit.
>
> Anyway, this way is certainly simpler, and setting it to NULL should
> be enough to limit the damage a driver can do :-)
+1 on moving this out of drm_crtc_state. drm_atomic_state already has
per-crtc structs, so easy to extend them with this. And yes drivers can
still see it, but mostly they're not supposed to touch drm_atomic_state
internals - the book-keeping is all done by the core.
The per-object state structs otoh are meant to be massively mangled by
drivers.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[RFC] drm: add hint to userspace about whether a plane can scale
On Fri, Oct 21, 2016 at 4:58 AM, Brian Starkey wrote:
> Hi Rob,
>
> On Thu, Oct 20, 2016 at 04:17:14PM -0400, Rob Clark wrote:
>>
>> When you have a mix of planes that can scale and those that cannot
>> scale, userspace really wants to have some hint to know which planes
>> can definitely not scale so it knows to assign them to unscaled layers.
>> I don't think it is fully possible to describe scaling constraints in
>> a generic way, so I don't think it is even worth trying, so this is
>> not a substitute for atomic TESTONLY step, but it does reduce the
>> search-space for userspace. In the common case, most layers will not
>> be scaled so knowing the best planes to pick for those layers is
>> useful.
>
>
> Somewhat related to what Daniel mentioned on IRC about driver
> consistency - how about making it "cannot_scale". This is then opt-in
> for drivers, and should mean userspace can always trust it if it's
> set.
yeah, I thought about it.. but a negative-cap sounded funny. I could
just initialize plane->can_scale to true in drm_universal_plane_init()
if needed.
> i.e. if cannot_scale == false, userspace can give it a shot, but if
> cannot_scale == true it should never bother.
>
> Either way, even with a device-specific planner we would want this
> hint to manage different HW versions so I'm in favour. But...
>
>
>> ---
>> drivers/gpu/drm/drm_crtc.c| 1 +
>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 1 +
>> include/drm/drm_crtc.h| 2 ++
>> include/uapi/drm/drm_mode.h | 3 +++
>> 4 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index b4b973f..d7e0e0d 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -2389,6 +2389,7 @@ int drm_mode_getplane(struct drm_device *dev, void
>> *data,
>> plane_resp->plane_id = plane->base.id;
>> plane_resp->possible_crtcs = plane->possible_crtcs;
>> plane_resp->gamma_size = 0;
>> + plane_resp->can_scale = plane->can_scale;
>>
>> /*
>> * This ioctl is called twice, once to determine how much space is
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>> index 692c888..2061c83 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>> @@ -908,6 +908,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device
>> *dev,
>> mdp5_plane->pipe = pipe;
>> mdp5_plane->name = pipe2name(pipe);
>> mdp5_plane->caps = caps;
>> + plane->can_scale = !!(caps & MDP_PIPE_CAP_SCALE);
>>
>> mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats,
>> ARRAY_SIZE(mdp5_plane->formats),
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index d74d47a..6e290b6 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -1679,6 +1679,7 @@ enum drm_plane_type {
>> * @format_types: array of formats supported by this plane
>> * @format_count: number of formats supported
>> * @format_default: driver hasn't supplied supported formats for the plane
>> + * @can_scale: a hint to userspace that this plane can (or cannot) scale
>> * @crtc: currently bound CRTC
>> * @fb: currently bound fb
>> * @old_fb: Temporary tracking of the old fb while a modeset is ongoing.
>> Used by
>> @@ -1710,6 +1711,7 @@ struct drm_plane {
>> uint32_t *format_types;
>> unsigned int format_count;
>> bool format_default;
>> + bool can_scale;
>>
>> struct drm_crtc *crtc;
>> struct drm_framebuffer *fb;
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index ce71ad5..5bf9361 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -191,6 +191,9 @@ struct drm_mode_get_plane {
>>
>> __u32 count_format_types;
>> __u64 format_type_ptr;
>> +
>> + __u32 can_scale;
>
>
> ... 32 bits for a bool does seem a bit wasteful.
alternative is to make it a bitmask of caps, although not sure how
much else we would add. And isn't like this ioctl is critical path,
or anything like that.
BR,
-R
> Thanks,
> Brian
>
>> + __u32 pad;
>> };
>>
>> struct drm_mode_get_plane_res {
>> --
>> 2.7.4
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 0/4] drm: add explicit fencing
On Thu, Oct 20, 2016 at 12:50:01PM -0200, Gustavo Padovan wrote: > From: Gustavo Padovan > > Hi, > > Currently the Linux Kernel only have an implicit fencing mechanism, through > reservation objects, in which the fences are attached directly to buffers > operations and userspace is unaware of what is happening. On the other hand > explicit fencing exposes fences to the userspace to handle fencing between > producer/consumer explicitely. > > To support explicit fencing in the mainline kernel we de-staged the the needed > parts of the Android Sync Framework[1] to be able to send and received fences > to/from userspace. It has the concept of sync_file that exposes the driver's > fences to userspace as file descriptors. > > With explicit fencing we have a global mechanism that optimizes the flow of > buffers between consumers and producers, avoiding a lot of waiting and some > potential desktop freeze. So instead of waiting for a buffer to be processed > by > the GPU before sending it to DRM in an Atomic IOCTL we can get a sync_file fd > from the GPU driver at the moment we submit the buffer processing. The > compositor then passes these fds to DRM in an Atomic IOCTL, that will > not be displayed until the fences signal, i.e, the GPU finished processing the > buffer and it is ready to display. In DRM the fences we wait on before > displaying a buffer are called in-fences and they are a per-plane deal. > > Vice-versa, we have out-fences to sychronize the return of buffers to GPU > (producer) to be processed again. When DRM receives an atomic request with a > the OUT_FENCE_PTR property it create a fence attach it to a per-crtc > sync_file. It then returns the sync_file fds to userspace. With the fences > available userspace can forward these fences to the GPU, where it will wait > the > fence to signal before starting to process on buffer again. Out-fences are > per-crtc. > > While these are out-fences in DRM (the consumer) they become in-fences once > they get to the GPU (the producer). > > DRM explicit fences are opt-in, as the default will still be implicit fencing. > > In-fences > - > > In the first discussions on #dri-devel on IRC we decided to hide the Sync > Framework from DRM drivers to reduce complexity, so as soon we get the fd > via IN_FENCE_FD plane property we convert the sync_file fd to a struct fence. > However a sync_file might contain more than one fence, so we created the > fence_array concept. struct fence_array is a subclass of struct > fence and stores a group of fences that needs to be waited together. > > Then we just use the already in place fence waiting support to wait on those > fences. Once the producer calls fence_signal() for all fences on wait we can > proceed with the atomic commit and display the framebuffers. > > Out-fences > -- > > Passing a pointer to OUT_FENCE_PTR property in an atomic request enables > out-fences. The kernel then creates a fence, attach it to a sync_file and > install this file on a unused fd for each crtc. The kernel writes the fd in > the memory pointed by the out_fence_ptr provided. In case of error it writes > -1. > > DRM core use the already in place drm_event infrastructure to help signal > fences, we've added a fence pointer to struct drm_pending_event. We signal > signal fences when all the buffer related to that CRTC are *on* the screen. > > Kernel tree > --- > > For those who want all patches on this RFC are in my tree: > > https://git.kernel.org/cgit/linux/kernel/git/padovan/linux.git/log/?h=fences > > > v5 changes > -- > > The changes from v5 to v4 are in the patches description. > > > Userspace > - > > Fences support on drm_hwcomposer is currently a working in progress. Where are the igts? There's some fairly tricky error recovery and handling code in there, I think we should have testcases for all of them. E.g. out_fence property set, but then some invalid property later on to exercise the error handling paths. Another one would be an atomic update (maybe of a primary plane) which should work, except the fb is way too small. That's checked by core code, but only at ->atomic_check phase, and so could be used to exercise the even later error code. Other things are mixing up in_fences, out_fences and events in different ways, and making sure it all works out. And then maybe also mix in nonblocking commit vs. blocking commit. If you need something to generate fences: vgem has them. Chris Wilson can point you at the code he's done in igt for testing the implicit fence support in i915. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v5 4/4] drm/fence: add out-fences support
2016-10-21 Daniel Vetter : > On Thu, Oct 20, 2016 at 10:15:20PM +0300, Ville Syrjälä wrote: > > On Thu, Oct 20, 2016 at 07:34:44PM +0300, Ville Syrjälä wrote: > > > On Thu, Oct 20, 2016 at 01:55:38PM -0200, Gustavo Padovan wrote: > > > > 2016-10-20 Ville Syrjälä : > > > > > > > > > On Thu, Oct 20, 2016 at 12:50:05PM -0200, Gustavo Padovan wrote: > > > > > > From: Gustavo Padovan > > > > > > > > > > > > Support DRM out-fences by creating a sync_file with a fence for > > > > > > each CRTC > > > > > > that sets the OUT_FENCE_PTR property. > > > > > > > > > > I still maintain the out fence should also be per fb (well, per plane > > > > > since we can't add it to fbs). Otherwise it's not going to be at all > > > > > useful if you do fps>vrefresh and don't include the same set of planes > > > > > in every atomic ioctl, eg. if you only ever include ones that are > > > > > somehow dirty. > > > > > > > > How would the kernel signal these dirty planes then? Right now we signal > > > > at the vblank. > > > > > > So if I think about it in terms of the previous fbs something like this > > > comes to mind: > > > > > > starting point: > > > plane a, fb 0 > > > plane b, fb 1 > > > > > > ioctl: > > > plane a, fb 2, fence A > > > plane b, fb 3, fence B > > > > > > ioctl: > > > plane a, fb 4, fence C > > > -> fb 2 can be reused, so fence C should signal immediately? > > > > > > vblank: > > > -> fb 0,1 can be reused, so fence A,B signal? > > > > > > It feels a bit wonky since the fence is effectively associated with the > > > previous fb after the previous fb was already submitted to the kernel. > > > One might assume fence A to be the one signalled early, but that would > > > give the impression that fb 0 would be free for reuse when it's not. > > > > OK, so after hashing this out on irc with Gustavo and Brian, we came > > to the conclusion that the per-crtc out fence should be sufficient > > after all. > > > > The way it can work is that the first ioctl will return the fence, > > doesn't really matter how many of the planes are involved in this > > ioctl. Subsequent ioctls that manage to get in before the next > > vblank will get back a -1 as the fence, even if the set if planes > > involved is not the same as in the first ioctl. From the -1 > > userspace can tell what happened, and can then consult its own > > records to see which still pending flips were overwritten, and > > thus knows which buffers can be reused immediately. > > > > If userspace plans on sending out the now free buffers to someone > > else accompanied by a fence, it can just create an already signalled > > fence to send instead of sending the fence it got back from the > > atomic ioctl since that one is still associated with the original > > scanout buffers. > > > > The fence returned by the atomic ioctl will only signal after the > > vblank, at which point userspace will obviously know that the > > orginal scanout buffers are now also ready for reuse, and that > > the last buffer submitted for each plane is now being actively > > scanned out. So if userspace wants to send out some of the > > original buffers to someone else, it can send along the fence > > returned from the atomic ioctl. > > > > So requires a bit more work from userspace perhaps. But obviously > > it only has to do this if it plans on submitting multiple operations > > per frame. > > > > So a slightly expanded version of my previous example might look like: > > starting point: > > plane a, fb 0 > > plane b, fb 1 > > > > ioctl: > > crtc: fence A > > plane a, fb 2 > > plane b, fb 3 > > > > ioctl: > > crtc: fence -1 > > plane a, fb 4 > > -> the previously submitted fb for plane a (fb 2) can be reused > > > > ioctl: > > crtc: fence -1 > > plane a, fb 5 > > -> the previously submitted fb for plane a (fb 4) can be reused > > > > vblank: > > -> fence A signals, fb 0,1 can be reused > > fb 3 and 5 being scanned out now > > > > > > We also had a quick side track w.r.t. variable refresh rate displays, > > and I think the conclusion was that there the vblank may just happen > > sooner or later. Nothing else should change. What's unclear is how > > the refresh rate would be controlled. The two obvious options are > > explicit control, and automagic based on the submit rate. But that's > > a separate topic entirely. > > Thanks a lot for doing this discussion and the detailed write-up. Imo we > should bake this into the kerneldoc, as uabi documentation examples. > Gustavo, can you pls include this? I'd put it into the kerneldoc for > @out_fence, with inline struct comments we can be rather excessive in > their lenght ;-) Sure, I'll work on kernel doc for this. Gustavo
[Intel-gfx] [PATCH v2] drm: Pass along the hotplug connector to the uevent
On Fri, Oct 21, 2016 at 02:45:41PM +0200, Daniel Vetter wrote: > On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote: > > If we know which connector was plugged/unplugged or > > connected/disconnected, we can pass that information along to userspace > > inside the uevent to reduce the amount of work userspace has to perform > > after the event (i.e. instead of looking over all connectors, it can > > just reprobe the affected one). > > > > v2: Don't convert intel_hotplug.c, it does a light probe and doesn't > > need the force. > > > > Signed-off-by: Chris Wilson > > Cc: Villle Syrjälä > > Cc: Manasi Navare > > Nothing is preventing multiple connectors to change state at the same > time. Or at least almost the same time, e.g. yank an entire dp mst chain. Or from sending multiple uevents in the time it takes userspace to respond. > I think we need something that works per-connector, so either send out > uevents for one that changed individually. Or go with the epoche counter > idea. The later has the upside that it disconnects probing from reporting: > Sometimes only deep down in the driver's probe function do we realize that > something changed (e.g. different edid, or different dpcd). With a helper > to increment the epoche there would be no need to wire the hotplug status > through the entire callchain. > > To give us the same speed-up benefits like this here the only thing we'd > need to do is make sure reading a read-only (i.e. not userspace setable > prop) doesn't take any heavyweight locks. That should be easy to achieve. > Of course that leaves us with num_connector ioctl calls, but that should > be acceptable. > > And it's an uabi change either way. This is a very simple extension to the current abi that provides a small additional hint. (And it is purely an optional hint.) I agree it is not as impactful as being able to categorically detect whether or not connector state has changed using an epoch counter, but it can provide a meaningful improvement right now. :) The simplest way to add the epoch would either be an extension to GETCONNECTOR or GETPROPBLOB, as there is no way to query a connector property otherwise (at least not after a cursory double check). getconnector->pad has never been set, so it would require GETCONNECTORv2 (just to add a new 64bit field to the struct, the simplest being a u64 nsec timestamp of last change). A simple extension either way. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[Intel-gfx] i915 monitor hotplug on 4.1/4.4 PREEMPT
Hello, On Fri, Oct 21, 2016 at 3:27 PM, Daniel Vetter wrote: > So yeah, "upgrade" is very likely is. I see... i915.disable_power_well=0 seems to do the trick. Is it safe to use? The kernel gets tainted because of this. Regards, Nicolae
[RFC] drm: add hint to userspace about whether a plane can scale
On Fri, Oct 21, 2016 at 8:35 AM, Daniel Vetter wrote:
> On Fri, Oct 21, 2016 at 09:58:45AM +0100, Brian Starkey wrote:
>> Hi Rob,
>>
>> On Thu, Oct 20, 2016 at 04:17:14PM -0400, Rob Clark wrote:
>> > When you have a mix of planes that can scale and those that cannot
>> > scale, userspace really wants to have some hint to know which planes
>> > can definitely not scale so it knows to assign them to unscaled layers.
>> > I don't think it is fully possible to describe scaling constraints in
>> > a generic way, so I don't think it is even worth trying, so this is
>> > not a substitute for atomic TESTONLY step, but it does reduce the
>> > search-space for userspace. In the common case, most layers will not
>> > be scaled so knowing the best planes to pick for those layers is
>> > useful.
>>
>> Somewhat related to what Daniel mentioned on IRC about driver
>> consistency - how about making it "cannot_scale". This is then opt-in
>> for drivers, and should mean userspace can always trust it if it's
>> set.
>>
>> i.e. if cannot_scale == false, userspace can give it a shot, but if
>> cannot_scale == true it should never bother.
>>
>> Either way, even with a device-specific planner we would want this
>> hint to manage different HW versions so I'm in favour. But...
>
> I think first thing we should do is add some backoff heuristics to
> drm_hwcomposer to try the same plane also with a non-scaled surface (after
> having tried it with a scaled one). That would probably be as effective as
> adding "can_scale", but with the upshot that we're not at the mercy of
> drivers exposing it correctly.
well, ignoring the fact that drm-hwc2 just tries one thing then falls
back to gl (which should be fixed but it is a big pile of c++11 code
so that will be someone elses job ;-))...
I did think about this approach, but two many changing variables so
making userspace guess about this sort of thing just seems evil.
> I'm always vary when we have the same limit checks 2 (in this case once in
> atomic_check, and once in the code that registers the property). And I'd
> like to avoid that as much as possible. We could avoid this by making the
> can_scale property mandatory, and enforcing it in the drm core. I.e. if
> it's not set, we reject scaled planes. That should give some decent
> motivation for driver writers to update them correctly. But without such a
> self-consistency check I don't really like this. It would be akin to
> adding "can_rotate" besides the "rotation" prop, and allowing drivers to
> botch things up and not register stuff consistently.
Fair enough, I'll add a check in drm core. That is simple enough. I
guess remaining question should can_scale default to true?
I guess the first time someone tries to bring up drm-hwc or similar
(ie. something more complex than single fullscreen layer) on their hw,
they are going to run into a few bugs in their driver, so I guess I'd
be ok for it to default to false and let people fix it when the time
comes.
BR,
-R
> -Daniel
>
>>
>> > ---
>> > drivers/gpu/drm/drm_crtc.c| 1 +
>> > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 1 +
>> > include/drm/drm_crtc.h| 2 ++
>> > include/uapi/drm/drm_mode.h | 3 +++
>> > 4 files changed, 7 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> > index b4b973f..d7e0e0d 100644
>> > --- a/drivers/gpu/drm/drm_crtc.c
>> > +++ b/drivers/gpu/drm/drm_crtc.c
>> > @@ -2389,6 +2389,7 @@ int drm_mode_getplane(struct drm_device *dev, void
>> > *data,
>> > plane_resp->plane_id = plane->base.id;
>> > plane_resp->possible_crtcs = plane->possible_crtcs;
>> > plane_resp->gamma_size = 0;
>> > + plane_resp->can_scale = plane->can_scale;
>> >
>> > /*
>> > * This ioctl is called twice, once to determine how much space is
>> > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>> > b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>> > index 692c888..2061c83 100644
>> > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>> > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>> > @@ -908,6 +908,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device
>> > *dev,
>> > mdp5_plane->pipe = pipe;
>> > mdp5_plane->name = pipe2name(pipe);
>> > mdp5_plane->caps = caps;
>> > + plane->can_scale = !!(caps & MDP_PIPE_CAP_SCALE);
>> >
>> > mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats,
>> > ARRAY_SIZE(mdp5_plane->formats),
>> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> > index d74d47a..6e290b6 100644
>> > --- a/include/drm/drm_crtc.h
>> > +++ b/include/drm/drm_crtc.h
>> > @@ -1679,6 +1679,7 @@ enum drm_plane_type {
>> > * @format_types: array of formats supported by this plane
>> > * @format_count: number of formats supported
>> > * @format_default: driver hasn't supplied supported formats for the plane
>> > + * @can_scale: a hint to userspace that this plane can (or cannot) scale
>> > * @crtc: currently
[PATCH] drm/fb-helper: Don't call dirty callback for untouched clips
On Fri, Oct 21, 2016 at 02:30:32PM +0200, Daniel Vetter wrote:
> On Thu, Oct 20, 2016 at 05:00:35PM +0200, Takashi Iwai wrote:
> > On Thu, 20 Oct 2016 16:56:04 +0200,
> > Ville Syrjälä wrote:
> > >
> > > On Thu, Oct 20, 2016 at 04:39:52PM +0200, Takashi Iwai wrote:
> > > > Since 4.7 kernel, we've seen the error messages like
> > > >
> > > > kernel: [TTM] Buffer eviction failed
> > > > kernel: qxl :00:02.0: object_init failed for (4026540032,
> > > > 0x0001)
> > > > kernel: [drm:qxl_alloc_bo_reserved [qxl]] *ERROR* failed to allocate
> > > > VRAM BO
> > > >
> > > > on QXL when switching and accessing on VT. The culprit was the
> > > > generic deferred_io code (qxl driver switched to it since 4.7).
> > > > There is a race between the dirty clip update and the call of
> > > > callback.
> > > >
> > > > In drm_fb_helper_dirty(), the dirty clip is updated in the spinlock,
> > > > while it kicks off the update worker outside the spinlock. Meanwhile
> > > > the update worker clears the dirty clip in the spinlock, too. Thus,
> > > > when drm_fb_helper_dirty() is called concurrently, schedule_work() is
> > > > called after the clip is cleared in the first worker call.
> > > >
> > > > This patch addresses it by validating the clip before calling the
> > > > dirty fb callback.
> > > >
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98322
> > > > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1003298
> > > > Fixes: eaa434defaca ('drm/fb-helper: Add fb_deferred_io support')
> > > > Cc:
> > > > Signed-off-by: Takashi Iwai
> > > > ---
> > > > drivers/gpu/drm/drm_fb_helper.c | 13 +
> > > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > > b/drivers/gpu/drm/drm_fb_helper.c
> > > > index 03414bde1f15..d790d205129e 100644
> > > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > > @@ -636,15 +636,20 @@ static void drm_fb_helper_dirty_work(struct
> > > > work_struct *work)
> > > > dirty_work);
> > > > struct drm_clip_rect *clip = &helper->dirty_clip;
> > > > struct drm_clip_rect clip_copy;
> > > > + bool dirty;
> > > > unsigned long flags;
> > > >
> > > > spin_lock_irqsave(&helper->dirty_lock, flags);
> > > > - clip_copy = *clip;
> > > > - clip->x1 = clip->y1 = ~0;
> > > > - clip->x2 = clip->y2 = 0;
> > > > + dirty = (clip->x1 < clip->x2 && clip->y1 < clip->y2);
> > > > + if (dirty) {
> > > > + clip_copy = *clip;
> > > > + clip->x1 = clip->y1 = ~0;
> > > > + clip->x2 = clip->y2 = 0;
> > > > + }
> > > > spin_unlock_irqrestore(&helper->dirty_lock, flags);
> > > >
> > > > - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> > > > + if (dirty)
> > >
> > > Could do it the other way too, ie. just make the copy, and then check the
> > > copy (can be done after dropping the lock even). Would avoid having to
> > > add the 'dirty' variable.
> >
> > Sounds good. Let me try...
>
> Another thing: How do we prevent userspace from doing the same, i.e.
> submitting an empty rectangle? Do we need to patch up
> drm_mode_dirtyfb_ioctl too? Not much point if we fix this bug in the fb
> helpers and leave the barn door wide open for userspace to oops drivers
> ;-)
I think of a use for sending an empty clip: where you don't want to
push any new pixel data, but you do want to be sure that the pipeline
has been flushed.
The change I would suggest here would be
dirty = clip->x1 <= clip->x2 && clip->y1 <= clip->y2
as the bug is not the empty rectangle but the invalid one. However, that
may be overkill, and none of the backends care about the empty rect!
But, indeed, we do not validate the incoming dirtyfb either.
diff --git a/drivers/gpu/drm/drm_framebuffer.c
b/drivers/gpu/drm/drm_framebuffer.c
index 49fd7db758e0..ada6a5517945 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -504,10 +504,13 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
}
if (num_clips && clips_ptr) {
+ int i;
+
if (num_clips < 0 || num_clips > DRM_MODE_FB_DIRTY_MAX_CLIPS) {
ret = -EINVAL;
goto out_err1;
}
+
clips = kcalloc(num_clips, sizeof(*clips), GFP_KERNEL);
if (!clips) {
ret = -ENOMEM;
@@ -520,6 +523,14 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
ret = -EFAULT;
goto out_err2;
}
+
+ for (i = 0; i < num_clips; i++) {
+ if (clips[i].x2 < clips[i].x1 ||
+ clips[i].y2 < clips[i].y1) {
+ ret = -EINVAL;
+
[PATCH 1/6] video: of: Constify node argument to display timing functions
On 04/10/16 15:31, Laurent Pinchart wrote: > The node pointer passed to the display timing functions is never > modified, make it const. > > Signed-off-by: Laurent Pinchart > --- > drivers/video/of_display_timing.c | 6 +++--- > include/video/of_display_timing.h | 15 --- > 2 files changed, 11 insertions(+), 10 deletions(-) > > Cc: David Airlie > Cc: Tomi Valkeinen Reviewed-by: Tomi Valkeinen And ack for merging via drm. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/edfcd1cb/attachment.sig>
[PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl
After hotplug notification, userspace has to reprobe all connectors to
detect any changes. This can be expensive (some outputs may require time
consuming load detection, some outputs may simply be slow to respond)
and not all outputs need to be checked everytime. The kernel is usually
in a very good position to know which outputs need to be reprobed (since
it has just responded to the hardware notification) and can convey this
information to userspace by tracking the timestamp of the last connector
change onto the GETCONNECTOR query.
Signed-off-by: Chris Wilson
---
Adding an epoch/timestamp field would look something like this. There are
probably a few more places where the hardware indicates a real change
(more than just status).
That update_timestamp() should probably be
atomic64_set(×tamp, ktime_get_ns());
---
drivers/gpu/drm/drm_connector.c | 4 +++-
drivers/gpu/drm/drm_crtc.c | 4 +++-
drivers/gpu/drm/drm_probe_helper.c | 6 +-
drivers/gpu/drm/gma500/mdfld_dsi_dpi.c | 1 +
drivers/gpu/drm/gma500/mdfld_dsi_output.c | 1 +
drivers/gpu/drm/i915/intel_dp.c | 4
drivers/gpu/drm/i915/intel_dp_mst.c | 2 ++
drivers/gpu/drm/i915/intel_hotplug.c| 5 -
drivers/gpu/drm/i915/intel_lvds.c | 1 +
drivers/gpu/drm/nouveau/nouveau_connector.c | 1 +
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 1 +
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 1 +
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 1 +
include/drm/drm_connector.h | 8
include/uapi/drm/drm.h | 2 +-
include/uapi/drm/drm_mode.h | 26 ++
16 files changed, 63 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2db7fb510b6c..cd3d85e94b91 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -227,6 +227,7 @@ int drm_connector_init(struct drm_device *dev,
INIT_LIST_HEAD(&connector->modes);
connector->edid_blob_ptr = NULL;
connector->status = connector_status_unknown;
+ drm_connector_update_timestamp(connector);
drm_connector_get_cmdline_mode(connector);
@@ -1010,7 +1011,7 @@ static bool drm_mode_expose_to_userspace(const struct
drm_display_mode *mode,
int drm_mode_getconnector(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
- struct drm_mode_get_connector *out_resp = data;
+ struct drm_mode_get_connector_v2 *out_resp = data;
struct drm_connector *connector;
struct drm_encoder *encoder;
struct drm_display_mode *mode;
@@ -1058,6 +1059,7 @@ int drm_mode_getconnector(struct drm_device *dev, void
*data,
out_resp->mm_height = connector->display_info.height_mm;
out_resp->subpixel = connector->display_info.subpixel_order;
out_resp->connection = connector->status;
+ out_resp->timestamp = ktime_to_ns(connector->timestamp);
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
encoder = drm_connector_get_encoder(connector);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 788d6cdc49a0..8668e95bb5b1 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -949,9 +949,11 @@ void drm_mode_config_reset(struct drm_device *dev)
encoder->funcs->reset(encoder);
mutex_lock(&dev->mode_config.mutex);
- drm_for_each_connector(connector, dev)
+ drm_for_each_connector(connector, dev) {
+ drm_connector_update_timestamp(connector);
if (connector->funcs->reset)
connector->funcs->reset(connector);
+ }
mutex_unlock(&dev->mode_config.mutex);
}
EXPORT_SYMBOL(drm_mode_config_reset);
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index 8790ee35a7cd..4b6882edc3fb 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -249,6 +249,7 @@ int drm_helper_probe_single_connector_modes(struct
drm_connector *connector,
* check here, and if anything changed start the hotplug code.
*/
if (old_status != connector->status) {
+ drm_connector_update_timestamp(connector);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
%s\n",
connector->base.id,
connector->name,
@@ -438,6 +439,7 @@ static void output_poll_execute(struct work_struct *work)
connector->name,
old, new);
+ drm_connector_update_timestamp(connector);
changed = true;
}
}
@@ -573,8 +575,10 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
conn
[PATCH] drm: Use u64 for intermediate dotclock calculations
We have reached the era where monitor bandwidths now exceed 31bits in
frequency calculations, though as we stored them in kHz units we are
safe from overflow in the modelines for some time.
[ 48.723720] UBSAN: Undefined behaviour in
../drivers/gpu/drm/drm_modes.c:325:49
[ 48.726943] signed integer overflow:
[ 48.728503] 2240 * 100 cannot be represented in type 'int'
Reported-by: Martin Liška
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98372
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/drm_modes.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 173b7d335834..f64ac86deb84 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -165,6 +165,7 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device
*dev, int hdisplay,
unsigned int vfieldrate, hperiod;
int hdisplay_rnd, hmargin, vdisplay_rnd, vmargin, vsync;
int interlace;
+ u64 tmp;
/* allocate the drm_display_mode structure. If failure, we will
* return directly
@@ -322,8 +323,11 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device
*dev, int hdisplay,
drm_mode->vsync_end = drm_mode->vsync_start + vsync;
}
/* 15/13. Find pixel clock frequency (kHz for xf86) */
- drm_mode->clock = drm_mode->htotal * HV_FACTOR * 1000 / hperiod;
- drm_mode->clock -= drm_mode->clock % CVT_CLOCK_STEP;
+ tmp = drm_mode->htotal; /* perform intermediate calcs in u64 */
+ tmp *= HV_FACTOR * 1000;
+ do_div(tmp, hperiod);
+ tmp -= drm_mode->clock % CVT_CLOCK_STEP;
+ drm_mode->clock = tmp;
/* 18/16. Find actual vertical frame frequency */
/* ignore - just set the mode flag for interlaced */
if (interlaced) {
--
2.9.3
[PATCH] drm: Use u64 for intermediate dotclock calculations
On Fri, Oct 21, 2016 at 10:15 AM, Chris Wilson
wrote:
> We have reached the era where monitor bandwidths now exceed 31bits in
> frequency calculations, though as we stored them in kHz units we are
> safe from overflow in the modelines for some time.
>
> [ 48.723720] UBSAN: Undefined behaviour in
> ../drivers/gpu/drm/drm_modes.c:325:49
> [ 48.726943] signed integer overflow:
> [ 48.728503] 2240 * 100 cannot be represented in type 'int'
>
> Reported-by: Martin Liška
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98372
> Signed-off-by: Chris Wilson
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/drm_modes.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 173b7d335834..f64ac86deb84 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -165,6 +165,7 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device
> *dev, int hdisplay,
> unsigned int vfieldrate, hperiod;
> int hdisplay_rnd, hmargin, vdisplay_rnd, vmargin, vsync;
> int interlace;
> + u64 tmp;
>
> /* allocate the drm_display_mode structure. If failure, we will
> * return directly
> @@ -322,8 +323,11 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device
> *dev, int hdisplay,
> drm_mode->vsync_end = drm_mode->vsync_start + vsync;
> }
> /* 15/13. Find pixel clock frequency (kHz for xf86) */
> - drm_mode->clock = drm_mode->htotal * HV_FACTOR * 1000 / hperiod;
> - drm_mode->clock -= drm_mode->clock % CVT_CLOCK_STEP;
> + tmp = drm_mode->htotal; /* perform intermediate calcs in u64 */
> + tmp *= HV_FACTOR * 1000;
> + do_div(tmp, hperiod);
> + tmp -= drm_mode->clock % CVT_CLOCK_STEP;
> + drm_mode->clock = tmp;
> /* 18/16. Find actual vertical frame frequency */
> /* ignore - just set the mode flag for interlaced */
> if (interlaced) {
> --
> 2.9.3
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[patch] drm/i915/gvt: cleanup a type issue in submit_context()
On Fri, Oct 21, 2016 at 04:27:38PM +0800, Zhenyu Wang wrote:
> On 2016.10.21 11:06:01 +0300, Dan Carpenter wrote:
> > submit_context() should return negative error codes and zero on success
> > but by mistake we made it a bool. I've changed it to int. Also I made
> > it static because this isn't referenced in any other files.
> >
> > This doesn't affect runtime because the return is eventually propogated
> > to elsp_mmio_write() where it is ignored.
> >
> > Signed-off-by: Dan Carpenter
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/execlist.c
> > b/drivers/gpu/drm/i915/gvt/execlist.c
> > index c50a3d1a5131..bc69ba1842ea 100644
> > --- a/drivers/gpu/drm/i915/gvt/execlist.c
> > +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> > @@ -616,7 +616,7 @@ static int prepare_mm(struct intel_vgpu_workload
> > *workload)
> > (list_empty(q) ? NULL : container_of(q->prev, \
> > struct intel_vgpu_workload, list))
> >
> > -bool submit_context(struct intel_vgpu *vgpu, int ring_id,
> > +static int submit_context(struct intel_vgpu *vgpu, int ring_id,
> > struct execlist_ctx_descriptor_format *desc,
> > bool emulate_schedule_in)
> > {
>
> Dan, this has been fixed in our recent pull, should hit linux-next very soon.
>
Could you review elsp_mmio_write() as well? Should we be doing
something with that return value?
regards,
dan carpenter
[PATCH] dt-bindings: video: exynos7-decon: Remove obsolete samsung,power-domain property
On 10/21/2016 04:05 PM, Krzysztof Kozlowski wrote:
> The samsung,power-domain property is obsolete since commit 0da658704136
> ("ARM: dts: convert to generic power domain bindings for exynos DT").
> Replace it with generic one.
>
> Signed-off-by: Krzysztof Kozlowski
Reviewed-by: Sylwester Nawrocki
[PATCH 1/2] dma-buf/sync_file: hold reference to fence when creating sync_file
On Wed, Oct 19, 2016 at 1:48 PM, Gustavo Padovan wrote: > From: Gustavo Padovan > > fence referencing was out of balance. It was not taking any ref to the > fence at creating time, but it was putting a reference when freeing the > sync file. > > This patch fixes the balancing issue by getting a reference for the fence > when creating the sync_file. > > Signed-off-by: Gustavo Padovan Applied to drm-misc, thanks > --- > drivers/dma-buf/sync_file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c > index b29a9e8..235f8ac 100644 > --- a/drivers/dma-buf/sync_file.c > +++ b/drivers/dma-buf/sync_file.c > @@ -79,7 +79,7 @@ struct sync_file *sync_file_create(struct fence *fence) > if (!sync_file) > return NULL; > > - sync_file->fence = fence; > + sync_file->fence = fence_get(fence); > > snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%llu-%d", > fence->ops->get_driver_name(fence), > -- > 2.5.5 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/i915/gvt: fix compilation
Two functions in the newly added gvt render code are obviously
broken, as they reference a variable without initialization and
don't reference another variable at all:
drivers/gpu/drm/i915/gvt/render.c: In function âintel_gvt_load_render_mmioâ:
drivers/gpu/drm/i915/gvt/render.c:148:13: error: âoffset.regâ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
drivers/gpu/drm/i915/gvt/render.c: In function
âintel_gvt_restore_render_mmioâ:
drivers/gpu/drm/i915/gvt/render.c:185:13: error: âoffset.regâ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
This is probably not a correct fix, but it gets us a clean build
by removing the unused arrays and initializing the offset variable
to something that potentially might be correct.
Fixes: 178657139307 ("drm/i915/gvt: vGPU context switch")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/i915/gvt/render.c | 25 +++--
1 file changed, 3 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/render.c
b/drivers/gpu/drm/i915/gvt/render.c
index feebb65ba641..79e112288065 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -147,29 +147,20 @@ static void load_mocs(struct intel_vgpu *vgpu, int
ring_id)
{
struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
i915_reg_t offset, l3_offset;
- u32 regs[] = {
- [RCS] = 0xc800,
- [VCS] = 0xc900,
- [VCS2] = 0xca00,
- [BCS] = 0xcc00,
- [VECS] = 0xcb00,
- };
int i;
- if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
- return;
-
if (!IS_SKYLAKE(dev_priv))
return;
for (i = 0; i < 64; i++) {
+ offset.reg = i * 4;
gen9_render_mocs[ring_id][i] = I915_READ(offset);
I915_WRITE(offset, vgpu_vreg(vgpu, offset));
POSTING_READ(offset);
- offset.reg += 4;
}
if (ring_id == RCS) {
+ offset.reg = 64 * 4;
l3_offset.reg = 0xb020;
for (i = 0; i < 32; i++) {
gen9_render_mocs_L3[i] = I915_READ(l3_offset);
@@ -184,26 +175,16 @@ static void restore_mocs(struct intel_vgpu *vgpu, int
ring_id)
{
struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
i915_reg_t offset, l3_offset;
- u32 regs[] = {
- [RCS] = 0xc800,
- [VCS] = 0xc900,
- [VCS2] = 0xca00,
- [BCS] = 0xcc00,
- [VECS] = 0xcb00,
- };
int i;
- if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
- return;
-
if (!IS_SKYLAKE(dev_priv))
return;
for (i = 0; i < 64; i++) {
+ offset.reg = i * 4;
vgpu_vreg(vgpu, offset) = I915_READ(offset);
I915_WRITE(offset, gen9_render_mocs[ring_id][i]);
POSTING_READ(offset);
- offset.reg += 4;
}
if (ring_id == RCS) {
--
2.9.0
[PATCH] dt-bindings: video: exynos7-decon: Remove obsolete samsung,power-domain property
On 10/21/2016 11:36 AM, Sylwester Nawrocki wrote:
> On 10/21/2016 04:05 PM, Krzysztof Kozlowski wrote:
>> The samsung,power-domain property is obsolete since commit 0da658704136
>> ("ARM: dts: convert to generic power domain bindings for exynos DT").
>> Replace it with generic one.
>>
>> Signed-off-by: Krzysztof Kozlowski
>
> Reviewed-by: Sylwester Nawrocki
>
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
[PATCH 1/2] drm/i915/gvt: add ACPI and 64BIT dependencies
The newly added gvt code produces lots of serious warnings and errors
when either built on 32-bit x86, or built with ACPI disabled, e.g.
drivers/gpu/drm/i915/gvt/gtt.c: In function âread_pte64â:
drivers/gpu/drm/i915/gvt/gtt.c:277:2: error: left shift count >= width of type
[-Werror]
drivers/gpu/drm/i915/gvt/gtt.c: In function âgen8_gtt_get_pfnâ:
drivers/gpu/drm/i915/gvt/gtt.c:360:3: error: left shift count >= width of type
[-Werror]
drivers/gpu/drm/i915/gvt/opregion.c: In function âintel_gvt_init_opregionâ:
drivers/gpu/drm/i915/gvt/opregion.c:183:2: error: implicit declaration of
function âacpi_os_ioremapâ [-Werror=implicit-function-declaration]
This avoids the problems by simply disallowing those configurations
in Kconfig. I'm sure it's possible to make the code more portable
and support building GVT without those options, but it might not be
useful to do so.
Fixes: 4d60c5fd3f87 ("drm/i915/gvt: vGPU PCI configuration space
virtualization")
Signed-off-by: Arnd Bergmann
---
If the code is meant to work on 32-bit and non-ACPI kernels, please
treat this as a bug report and disregard the patch.
---
drivers/gpu/drm/i915/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 6d4194288d11..1b9308284dde 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -84,6 +84,7 @@ config DRM_I915_USERPTR
config DRM_I915_GVT
bool "Enable Intel GVT-g graphics virtualization host support"
depends on DRM_I915
+ depends on 64BIT && ACPI
default n
help
Choose this option if you want to enable Intel GVT-g graphics
--
2.9.0
[Bug 178281] wine-staging apps freezes the machine with RX460
https://bugzilla.kernel.org/show_bug.cgi?id=178281 --- Comment #9 from fin4478 at hotmail.com --- Now when I am playing TR 2013 with wine-1.9.21 (Staging) csmt enabled, gaming will will hang my machine when having fast actions. 10-20 minutes gaming is possible without rebooting. Using 64-bit ubuntu server kernel from here: http://www.yourownlinux.com/2016/10/how-to-install-linux-kernel-4-9-rc1-in-linux.html Playing the game at 1920x1200 normal settings. I have played the game many times with A8-7600 and amdgpu kernel driver without hanging. The hardware and software has been the same except now I have carrizo cpu and polaris gpu. My hardware: XFX RX460 2GB DDR5 Amd X4 845 Carrizo cpu Asus A88XM-E motherboard 8GB 2133MHz ddr3 SanDisk SDSSDA240G OS: Debian testing Xfce with Padoka ppa Mesa 12.1.0-devel. No events in logs, because hanging happens so fast. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v5 4/4] drm/fence: add out-fences support
Hi,
Sorry, I hit another couple of bugs that originated from my hastebin
patch - see below.
On Thu, Oct 20, 2016 at 12:50:05PM -0200, Gustavo Padovan wrote:
>From: Gustavo Padovan
>
>Support DRM out-fences by creating a sync_file with a fence for each CRTC
>that sets the OUT_FENCE_PTR property.
>
>We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
>the sync_file fd back to userspace.
>
>The sync_file and fd are allocated/created before commit, but the
>fd_install operation only happens after we know that commit succeed.
>
>In case of error userspace should receive a fd number of -1.
>
>v2: Comment by Rob Clark:
> - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
>
>Comment by Daniel Vetter:
> - Add clean up code for out_fences
>
>v3: Comments by Daniel Vetter:
> - create DRM_MODE_ATOMIC_EVENT_MASK
> - userspace should fill out_fences_ptr with the crtc_ids for which
> it wants fences back.
>
>v4: Create OUT_FENCE_PTR properties and remove old approach.
>
>Signed-off-by: Gustavo Padovan
>---
> drivers/gpu/drm/drm_atomic.c | 116 ++-
> drivers/gpu/drm/drm_crtc.c | 8 +++
> include/drm/drm_crtc.h | 19 +++
> 3 files changed, 119 insertions(+), 24 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index b3284b2..07775fc 100644
>--- a/drivers/gpu/drm/drm_atomic.c
>+++ b/drivers/gpu/drm/drm_atomic.c
>@@ -490,6 +490,8 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> &replaced);
> state->color_mgmt_changed |= replaced;
> return ret;
>+ } else if (property == config->prop_out_fence_ptr) {
>+ state->out_fence_ptr = u64_to_user_ptr(val);
> } else if (crtc->funcs->atomic_set_property)
> return crtc->funcs->atomic_set_property(crtc, state, property,
> val);
> else
>@@ -532,6 +534,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> *val = (state->ctm) ? state->ctm->base.id : 0;
> else if (property == config->gamma_lut_property)
> *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>+ else if (property == config->prop_out_fence_ptr)
>+ *val = 0;
> else if (crtc->funcs->atomic_get_property)
> return crtc->funcs->atomic_get_property(crtc, state, property,
> val);
> else
>@@ -1474,11 +1478,9 @@ EXPORT_SYMBOL(drm_atomic_nonblocking_commit);
> */
>
> static struct drm_pending_vblank_event *create_vblank_event(
>- struct drm_device *dev, struct drm_file *file_priv,
>- struct fence *fence, uint64_t user_data)
>+ struct drm_device *dev, uint64_t user_data)
> {
> struct drm_pending_vblank_event *e = NULL;
>- int ret;
>
> e = kzalloc(sizeof *e, GFP_KERNEL);
> if (!e)
>@@ -1488,17 +1490,6 @@ static struct drm_pending_vblank_event
>*create_vblank_event(
> e->event.base.length = sizeof(e->event);
> e->event.user_data = user_data;
>
>- if (file_priv) {
>- ret = drm_event_reserve_init(dev, file_priv, &e->base,
>- &e->event.base);
>- if (ret) {
>- kfree(e);
>- return NULL;
>- }
>- }
>-
>- e->base.fence = fence;
>-
> return e;
> }
>
>@@ -1603,6 +1594,40 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>
>+static int crtc_setup_out_fence(struct drm_crtc *crtc,
>+ struct drm_crtc_state *crtc_state,
>+ struct drm_out_fence_state *fence_state)
>+{
>+ struct fence *fence;
>+
>+ fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
>+ if (fence_state->fd < 0) {
>+ return fence_state->fd;
>+ }
>+
>+ fence_state->out_fence_ptr = crtc_state->out_fence_ptr;
>+ crtc_state->out_fence_ptr = NULL;
>+
>+ if (put_user(fence_state->fd, fence_state->out_fence_ptr))
>+ return -EFAULT;
>+
>+ fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>+ if (!fence)
>+ return -ENOMEM;
>+
>+ fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
>+ crtc->fence_context, ++crtc->fence_seqno);
>+
>+ fence_state->sync_file = sync_file_create(fence);
>+ if(!fence_state->sync_file) {
>+ fence_put(fence);
>+ return -ENOMEM;
>+ }
>+
>+ crtc_state->event->base.fence = fence_get(fence);
>+ return 0;
>+}
>+
> int drm_mode_atomic_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv)
> {
>@@ -1617,9 +1642,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> struct drm_plane *plane;
> struct drm_crtc *crtc;
> struct drm_crtc_state *crtc_state;
>+ struct drm_out_fence_stat
[PATCH v6 06/11] drm/i915: Enable i915 perf stream for Haswell OA unit
On Thu, Oct 20, 2016 at 11:10 PM, Chris Wilson wrote: > On Thu, Oct 20, 2016 at 10:19:05PM +0100, Robert Bragg wrote: > > +int i915_gem_context_pin_legacy_rcs_state(struct drm_i915_private > *dev_priv, > > + struct i915_gem_context *ctx, > > + u64 flags) > > This is still no. > Okay, but it's a little frustrating for me to go in circles here :-/ I didn't originally do it this way; I originally looked at pinning the context when opening the stream so I didn't have to consider it being relocated. The feedback from Daniel Vetter was to look at doing it this way I think because of some concern to do with some shrinker corner cases. ... just dug up the archive: https://lists.freedesktop.org/archives/intel-gfx/2014-November/055385.html Can you maybe please explain what's wrong with the current approach and provide some justification for a different approach with some reassurance that Daniel's original concern with the shrinker unpinning contexts isn't actually a problem? I don't currently understand the concern with this, and this approach seems to have been working well for quite a long time now. - Robert -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/2f58bebf/attachment.html>
[Intel-gfx] [PATCH v3 03/15] drm: Add support for optional per-plane rotation property
On Tue, Sep 27, 2016 at 01:22:48PM +0300, Ville Syrjälä wrote: > On Tue, Sep 27, 2016 at 12:54:46PM +0300, Joonas Lahtinen wrote: > > On ma, 2016-09-26 at 19:30 +0300, ville.syrjala at linux.intel.com wrote: > > > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c > > > > > > index 85172a977bf3..e52aece30900 100644 > > > --- a/drivers/gpu/drm/drm_blend.c > > > +++ b/drivers/gpu/drm/drm_blend.c > > > > Now that I'm thinking, *_zpos_* fits to drm_blend.c nicely, but not > > sure if rotation is related to blending so much. Maybe it should be > > like a new file drm_transform.c (or drm_plane.c to be overly generic), > > not sure. > > The next guy who moves it gets to rebase all of my ~500 branches! Or > alternatively gets to make git smarter so it'll detect the pointless > shuffling automagically. My idea behind drm_blend is to cover _everything_ between the raw framebuffer data and the final crtc picture. So scaling, rotation, blending and anything like that. I should probably have called it compositing, but oh well that fail is now done. -Daniel > > > > > Anyway, > > > > Reviewed-by: Joonas Lahtinen > > > > Regards, Joonas > > -- > > Joonas Lahtinen > > Open Source Technology Center > > Intel Corporation > > -- > Ville Syrjälä > Intel OTC > ___ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v2 08/15] drm/msm/mdp5: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0
On Mon, Sep 26, 2016 at 07:30:53PM +0300, ville.syrjala at linux.intel.com wrote: > From: Ville Syrjälä > > 0 isn't a valid rotation property value, so let's set the initial value > of the property to BIT(DRM_ROTATE_0) instead. > > In the same vein, we must always have at leat one angle as part of > set of supported rotation bits, so let's include DRM_ROTATE_0 > in there. > > v2: Drop the BIT() > > Cc: Rob Clark > Cc: Jilai Wang > Cc: Archit Taneja > Signed-off-by: Ville Syrjälä This one here had a conflict, so didn't apply. Not sure if still needed. -Daniel > --- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > index ba8f43278a44..b0ecf5357bfd 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > @@ -78,12 +78,12 @@ static void mdp5_plane_install_rotation_property(struct > drm_device *dev, > if (!dev->mode_config.rotation_property) > dev->mode_config.rotation_property = > drm_mode_create_rotation_property(dev, > - DRM_REFLECT_X | DRM_REFLECT_Y); > + DRM_ROTATE_0 | DRM_REFLECT_X | DRM_REFLECT_Y); > > if (dev->mode_config.rotation_property) > drm_object_attach_property(&plane->base, > dev->mode_config.rotation_property, > - 0); > + DRM_ROTATE_0); > } > > /* helper to install properties which are common to planes and crtcs */ > -- > 2.7.4 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Intel-gfx] [PATCH v3 11/15] drm/i915: Use the per-plane rotation property
On Tue, Sep 27, 2016 at 12:58:49PM +0300, Joonas Lahtinen wrote: > On ma, 2016-09-26 at 19:30 +0300, ville.syrjala at linux.intel.com wrote: > > > From: Ville Syrjälä > > > > On certain platforms not all planes support the same set of > > rotations/reflections, so let's use the per-plane property > > for this. > > > > This is already a problem on SKL when we use the legay cursor plane > > as it only supports 0|180 whereas the universal planes support > > 0|90|180|270, and it will be a problem on CHV soon. > > > > v2: Use drm_plane_create_rotation_property() helper > > v3: Drop the BIT(), use INTEL_GEN() > > > > Signed-off-by: Ville Syrjälä > > Reviewed-by: Chris Wilson (v1) > > Reviewed-by: Joonas Lahtinen (v2) > > Reviewed-by: Joonas Lahtinen Applied up to this one because I hit a small snag in msm. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[pull] radeon and amdgpu drm-fixes-4.9
Hi Dave, Misc bug fixes for radeon and amdgpu. The following changes since commit 26beaee9bb07be20cc641c1251152e280e80f54e: Merge branch 'drm-etnaviv-fixes' of git://git.pengutronix.de/lst/linux into drm-fixes (2016-10-21 13:27:55 +1000) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-fixes-4.9 for you to fetch changes up to 8861a8209782faffedb6d64572fa968ee9c1c375: drm/amd/powerplay: don't give up if DPM is already running (2016-10-21 11:19:18 -0400) Alex Deucher (5): drm/amdgpu/powerplay/smu7: fix static checker warning drm/amdgpu: drop atom scratch save/restore in gpu reset drm/amdgpu: move atom scratch register save/restore to common code drm/amdgpu/st: move ATC CG golden init from gfx to mc drm/amdgpu: explicitly set pg_flags for ST Evan Quan (1): drm/amd/amdgpu: expose max engine and memory clock for powerplay enabled case Grazvydas Ignotas (1): drm/amd/powerplay: don't give up if DPM is already running Rex Zhu (1): drm/amd/powerplay: fix static checker warning in process_pptables_v1_0.c Tom St Denis (1): drm/radeon/si_dpm: Limit clocks on HD86xx part jimqu (1): drm/amdgpu: avoid drm error log during S3 on RHEL7.3 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 - drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 ++ drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 6 -- drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 6 -- drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 6 -- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 6 -- drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 1 - drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 1 + drivers/gpu/drm/amd/amdgpu/vi.c | 2 +- .../gpu/drm/amd/powerplay/hwmgr/process_pptables_v1_0.c | 9 ++--- drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c| 17 ++--- drivers/gpu/drm/radeon/si_dpm.c | 6 ++ 13 files changed, 32 insertions(+), 46 deletions(-)
[PATCH v2 08/15] drm/msm/mdp5: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0
On Fri, Oct 21, 2016 at 06:26:54PM +0200, Daniel Vetter wrote: > On Mon, Sep 26, 2016 at 07:30:53PM +0300, ville.syrjala at linux.intel.com > wrote: > > From: Ville Syrjälä > > > > 0 isn't a valid rotation property value, so let's set the initial value > > of the property to BIT(DRM_ROTATE_0) instead. > > > > In the same vein, we must always have at leat one angle as part of > > set of supported rotation bits, so let's include DRM_ROTATE_0 > > in there. > > > > v2: Drop the BIT() > > > > Cc: Rob Clark > > Cc: Jilai Wang > > Cc: Archit Taneja > > Signed-off-by: Ville Syrjälä > > This one here had a conflict, so didn't apply. Not sure if still needed. I think Rob picked it up already. > -Daniel > > > --- > > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > index ba8f43278a44..b0ecf5357bfd 100644 > > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > @@ -78,12 +78,12 @@ static void mdp5_plane_install_rotation_property(struct > > drm_device *dev, > > if (!dev->mode_config.rotation_property) > > dev->mode_config.rotation_property = > > drm_mode_create_rotation_property(dev, > > - DRM_REFLECT_X | DRM_REFLECT_Y); > > + DRM_ROTATE_0 | DRM_REFLECT_X | DRM_REFLECT_Y); > > > > if (dev->mode_config.rotation_property) > > drm_object_attach_property(&plane->base, > > dev->mode_config.rotation_property, > > - 0); > > + DRM_ROTATE_0); > > } > > > > /* helper to install properties which are common to planes and crtcs */ > > -- > > 2.7.4 > > > > ___ > > dri-devel mailing list > > dri-devel at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel OTC
[PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl
On Fri, Oct 21, 2016 at 02:42:12PM +0100, Chris Wilson wrote:
> After hotplug notification, userspace has to reprobe all connectors to
> detect any changes. This can be expensive (some outputs may require time
> consuming load detection, some outputs may simply be slow to respond)
> and not all outputs need to be checked everytime. The kernel is usually
> in a very good position to know which outputs need to be reprobed (since
> it has just responded to the hardware notification) and can convey this
> information to userspace by tracking the timestamp of the last connector
> change onto the GETCONNECTOR query.
>
> Signed-off-by: Chris Wilson
> ---
> Adding an epoch/timestamp field would look something like this. There are
> probably a few more places where the hardware indicates a real change
> (more than just status).
>
> That update_timestamp() should probably be
> atomic64_set(×tamp, ktime_get_ns());
Hm, I thought we'd just add a new epoch property, attach it to every
connector, and userspace would inquire about it using
DRM_IOCTL_MODE_OBJ_GETPROPERTIES.
The only nitpick/polish would be to audit
drm_mode_obj_get_properties_ioctl and figure out how much locking it
really needs (probably almost nothing at all).
> ---
> drivers/gpu/drm/drm_connector.c | 4 +++-
> drivers/gpu/drm/drm_crtc.c | 4 +++-
> drivers/gpu/drm/drm_probe_helper.c | 6 +-
> drivers/gpu/drm/gma500/mdfld_dsi_dpi.c | 1 +
> drivers/gpu/drm/gma500/mdfld_dsi_output.c | 1 +
> drivers/gpu/drm/i915/intel_dp.c | 4
> drivers/gpu/drm/i915/intel_dp_mst.c | 2 ++
> drivers/gpu/drm/i915/intel_hotplug.c| 5 -
> drivers/gpu/drm/i915/intel_lvds.c | 1 +
> drivers/gpu/drm/nouveau/nouveau_connector.c | 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 1 +
> include/drm/drm_connector.h | 8
> include/uapi/drm/drm.h | 2 +-
> include/uapi/drm/drm_mode.h | 26 ++
> 16 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2db7fb510b6c..cd3d85e94b91 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -227,6 +227,7 @@ int drm_connector_init(struct drm_device *dev,
> INIT_LIST_HEAD(&connector->modes);
> connector->edid_blob_ptr = NULL;
> connector->status = connector_status_unknown;
> + drm_connector_update_timestamp(connector);
>
> drm_connector_get_cmdline_mode(connector);
>
> @@ -1010,7 +1011,7 @@ static bool drm_mode_expose_to_userspace(const struct
> drm_display_mode *mode,
> int drm_mode_getconnector(struct drm_device *dev, void *data,
> struct drm_file *file_priv)
> {
> - struct drm_mode_get_connector *out_resp = data;
> + struct drm_mode_get_connector_v2 *out_resp = data;
> struct drm_connector *connector;
> struct drm_encoder *encoder;
> struct drm_display_mode *mode;
> @@ -1058,6 +1059,7 @@ int drm_mode_getconnector(struct drm_device *dev, void
> *data,
> out_resp->mm_height = connector->display_info.height_mm;
> out_resp->subpixel = connector->display_info.subpixel_order;
> out_resp->connection = connector->status;
> + out_resp->timestamp = ktime_to_ns(connector->timestamp);
>
> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> encoder = drm_connector_get_encoder(connector);
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 788d6cdc49a0..8668e95bb5b1 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -949,9 +949,11 @@ void drm_mode_config_reset(struct drm_device *dev)
> encoder->funcs->reset(encoder);
>
> mutex_lock(&dev->mode_config.mutex);
> - drm_for_each_connector(connector, dev)
> + drm_for_each_connector(connector, dev) {
> + drm_connector_update_timestamp(connector);
> if (connector->funcs->reset)
> connector->funcs->reset(connector);
> + }
> mutex_unlock(&dev->mode_config.mutex);
> }
> EXPORT_SYMBOL(drm_mode_config_reset);
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index 8790ee35a7cd..4b6882edc3fb 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -249,6 +249,7 @@ int drm_helper_probe_single_connector_modes(struct
> drm_connector *connector,
>* check here, and if anything changed start the hotplug code.
>*/
> if (old_status != connector->status) {
> + drm_connector_update_timestamp(connector);
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
> %s\n",
>
[PATCH libdrm 1/2] Return an -ENODEV from drmGetDevice() when no device was found.
From: Rob Herring Fixes crashes in Mesa on platform device, which expected *device to have a device when 0 was returned. (code from a paste by Rob, commit message by anholt) Signed-off-by: Eric Anholt --- xf86drm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xf86drm.c b/xf86drm.c index 9cfca49ddfda..9b52889e4cef 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -3181,6 +3181,8 @@ int drmGetDevice(int fd, drmDevicePtr *device) closedir(sysdir); free(local_devices); +if (*device == NULL) + return -ENODEV; return 0; free_devices: -- 2.9.3
[PATCH] drm: tilcdc: implement palette loading for rev1
Revision 1 of the IP doesn't work if we don't load the palette (even
if it's not used, which is the case for the RGB565 format).
Add a function called from tilcdc_crtc_enable() which performs all
required actions if we're dealing with a rev1 chip.
Signed-off-by: Bartosz Golaszewski
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 82 +++-
1 file changed, 81 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 87cfde9..cc94cbb 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -21,11 +21,14 @@
#include
#include
#include
+#include
#include "tilcdc_drv.h"
#include "tilcdc_regs.h"
-#define TILCDC_VBLANK_SAFETY_THRESHOLD_US 1000
+#define TILCDC_VBLANK_SAFETY_THRESHOLD_US 1000
+#define TILCDC_REV1_PALETTE_SIZE 32
+#define TILCDC_REV1_PALETTE_FIRST_ENTRY0x4000
struct tilcdc_crtc {
struct drm_crtc base;
@@ -53,6 +56,10 @@ struct tilcdc_crtc {
int sync_lost_count;
bool frame_intact;
+
+ dma_addr_t palette_dma_handle;
+ void *palette_base;
+ struct completion palette_loaded;
};
#define to_tilcdc_crtc(x) container_of(x, struct tilcdc_crtc, base)
@@ -100,6 +107,55 @@ static void set_scanout(struct drm_crtc *crtc, struct
drm_framebuffer *fb)
tilcdc_crtc->curr_fb = fb;
}
+/*
+ * The driver currently only supports the RGB565 format for revision 1. For
+ * 16 bits-per-pixel the palette block is bypassed, but the first 32 bytes of
+ * the framebuffer are still considered palette. The first 16-bit entry must
+ * be 0x4000 while all other entries must be zeroed.
+ */
+static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
+{
+ u32 dma_fb_base, dma_fb_ceiling, raster_ctl;
+ struct tilcdc_crtc *tilcdc_crtc;
+ struct drm_device *dev;
+ u16 *first_entry;
+
+ dev = crtc->dev;
+ tilcdc_crtc = to_tilcdc_crtc(crtc);
+ first_entry = tilcdc_crtc->palette_base;
+
+ *first_entry = TILCDC_REV1_PALETTE_FIRST_ENTRY;
+
+ dma_fb_base = tilcdc_read(dev, LCDC_DMA_FB_BASE_ADDR_0_REG);
+ dma_fb_ceiling = tilcdc_read(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG);
+ raster_ctl = tilcdc_read(dev, LCDC_RASTER_CTRL_REG);
+
+ /* Tell the LCDC where the palette is located. */
+ tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG,
+tilcdc_crtc->palette_dma_handle);
+ tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG,
+(u32)tilcdc_crtc->palette_dma_handle
+ + TILCDC_REV1_PALETTE_SIZE - 1);
+
+ /* Load it. */
+ tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
+LCDC_PALETTE_LOAD_MODE(DATA_ONLY));
+ tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
+ LCDC_PALETTE_LOAD_MODE(PALETTE_ONLY));
+
+ /* Enable the LCDC and wait for palette to be loaded. */
+ tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
+ tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+
+ wait_for_completion(&tilcdc_crtc->palette_loaded);
+
+ /* Restore the registers. */
+ tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+ tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, dma_fb_base);
+ tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG, dma_fb_ceiling);
+ tilcdc_write(dev, LCDC_RASTER_CTRL_REG, raster_ctl);
+}
+
static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
{
struct tilcdc_drm_private *priv = dev->dev_private;
@@ -154,6 +210,7 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+ struct tilcdc_drm_private *priv = dev->dev_private;
WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
@@ -164,6 +221,9 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
reset(crtc);
+ if (priv->rev == 1 && !completion_done(&tilcdc_crtc->palette_loaded))
+ tilcdc_crtc_load_palette(crtc);
+
tilcdc_crtc_enable_irqs(dev);
tilcdc_clear(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE);
@@ -245,6 +305,10 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
of_node_put(crtc->port);
drm_crtc_cleanup(crtc);
drm_flip_work_cleanup(&tilcdc_crtc->unref_work);
+
+ dma_free_coherent(crtc->dev->dev, TILCDC_REV1_PALETTE_SIZE,
+ tilcdc_crtc->palette_base,
+ tilcdc_crtc->palette_dma_handle);
}
int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
@@ -804,6 +868,14 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underfow",
__func__, stat);
+ if (priv->rev == 1) {
+ if (stat & LCDC_PL_LOAD_DONE) {
+ co
[PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl
On Fri, Oct 21, 2016 at 02:42:12PM +0100, Chris Wilson wrote:
> After hotplug notification, userspace has to reprobe all connectors to
> detect any changes. This can be expensive (some outputs may require time
> consuming load detection, some outputs may simply be slow to respond)
> and not all outputs need to be checked everytime. The kernel is usually
> in a very good position to know which outputs need to be reprobed (since
> it has just responded to the hardware notification) and can convey this
> information to userspace by tracking the timestamp of the last connector
> change onto the GETCONNECTOR query.
>
> Signed-off-by: Chris Wilson
> ---
> Adding an epoch/timestamp field would look something like this. There are
> probably a few more places where the hardware indicates a real change
> (more than just status).
>
> That update_timestamp() should probably be
> atomic64_set(×tamp, ktime_get_ns());
> ---
> drivers/gpu/drm/drm_connector.c | 4 +++-
> drivers/gpu/drm/drm_crtc.c | 4 +++-
> drivers/gpu/drm/drm_probe_helper.c | 6 +-
> drivers/gpu/drm/gma500/mdfld_dsi_dpi.c | 1 +
> drivers/gpu/drm/gma500/mdfld_dsi_output.c | 1 +
> drivers/gpu/drm/i915/intel_dp.c | 4
> drivers/gpu/drm/i915/intel_dp_mst.c | 2 ++
> drivers/gpu/drm/i915/intel_hotplug.c| 5 -
> drivers/gpu/drm/i915/intel_lvds.c | 1 +
> drivers/gpu/drm/nouveau/nouveau_connector.c | 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 1 +
> include/drm/drm_connector.h | 8
> include/uapi/drm/drm.h | 2 +-
> include/uapi/drm/drm_mode.h | 26 ++
> 16 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2db7fb510b6c..cd3d85e94b91 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -227,6 +227,7 @@ int drm_connector_init(struct drm_device *dev,
> INIT_LIST_HEAD(&connector->modes);
> connector->edid_blob_ptr = NULL;
> connector->status = connector_status_unknown;
> + drm_connector_update_timestamp(connector);
>
> drm_connector_get_cmdline_mode(connector);
>
> @@ -1010,7 +1011,7 @@ static bool drm_mode_expose_to_userspace(const struct
> drm_display_mode *mode,
> int drm_mode_getconnector(struct drm_device *dev, void *data,
> struct drm_file *file_priv)
> {
> - struct drm_mode_get_connector *out_resp = data;
> + struct drm_mode_get_connector_v2 *out_resp = data;
> struct drm_connector *connector;
> struct drm_encoder *encoder;
> struct drm_display_mode *mode;
> @@ -1058,6 +1059,7 @@ int drm_mode_getconnector(struct drm_device *dev, void
> *data,
> out_resp->mm_height = connector->display_info.height_mm;
> out_resp->subpixel = connector->display_info.subpixel_order;
> out_resp->connection = connector->status;
> + out_resp->timestamp = ktime_to_ns(connector->timestamp);
Is there benefit to making it a timestamp as opposed to just an
incrementing counter?
--
Ville Syrjälä
Intel OTC
[PATCH libdrm 1/2] Return an -ENODEV from drmGetDevice() when no device was found.
On Fri, Oct 21, 2016 at 1:12 PM, Eric Anholt wrote: > From: Rob Herring > > Fixes crashes in Mesa on platform device, which expected *device to > have a device when 0 was returned. > > (code from a paste by Rob, commit message by anholt) > > Signed-off-by: Eric Anholt Reviewed-by: Alex Deucher > --- > xf86drm.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/xf86drm.c b/xf86drm.c > index 9cfca49ddfda..9b52889e4cef 100644 > --- a/xf86drm.c > +++ b/xf86drm.c > @@ -3181,6 +3181,8 @@ int drmGetDevice(int fd, drmDevicePtr *device) > > closedir(sysdir); > free(local_devices); > +if (*device == NULL) > + return -ENODEV; > return 0; > > free_devices: > -- > 2.9.3 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 2/2] Silence runtime complaints on platform devices
glxgears was spamming this 12 times at startup because of Mesa's probing of the DRM device code, which doesn't support platform devices. Signed-off-by: Eric Anholt --- xf86drm.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/xf86drm.c b/xf86drm.c index 9b52889e4cef..52add5e441d7 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -3149,7 +3149,6 @@ int drmGetDevice(int fd, drmDevicePtr *device) break; default: -fprintf(stderr, "The subsystem type is not supported yet\n"); continue; } @@ -3259,7 +3258,6 @@ int drmGetDevices(drmDevicePtr devices[], int max_devices) break; default: -fprintf(stderr, "The subsystem type is not supported yet\n"); continue; } -- 2.9.3
[PATCH libdrm 1/2] Return an -ENODEV from drmGetDevice() when no device was found.
On 21 October 2016 at 18:12, Eric Anholt wrote: > From: Rob Herring > > Fixes crashes in Mesa on platform device, which expected *device to > have a device when 0 was returned. > > (code from a paste by Rob, commit message by anholt) > > Signed-off-by: Eric Anholt Reviewed-by: Emil Velikov Thanks Emil
[PATCH libdrm 2/2] Silence runtime complaints on platform devices
On Fri, Oct 21, 2016 at 1:12 PM, Eric Anholt wrote: > glxgears was spamming this 12 times at startup because of Mesa's > probing of the DRM device code, which doesn't support platform > devices. > > Signed-off-by: Eric Anholt Reviewed-by: Alex Deucher > --- > xf86drm.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/xf86drm.c b/xf86drm.c > index 9b52889e4cef..52add5e441d7 100644 > --- a/xf86drm.c > +++ b/xf86drm.c > @@ -3149,7 +3149,6 @@ int drmGetDevice(int fd, drmDevicePtr *device) > > break; > default: > -fprintf(stderr, "The subsystem type is not supported yet\n"); > continue; > } > > @@ -3259,7 +3258,6 @@ int drmGetDevices(drmDevicePtr devices[], int > max_devices) > > break; > default: > -fprintf(stderr, "The subsystem type is not supported yet\n"); > continue; > } > > -- > 2.9.3 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/i2c: tda998x: Remove obsolete drm_connector_register() call
On Thu, 20 Oct 2016 16:56:44 +0530 Archit Taneja wrote: > > Please show _technically_ how this would work. I want to see code or > > pseudo-code illustrating how a "foreign" DRM encoder could be used with > > either dw-hdmi or tda998x, because right now I can't see any way that > > could work. > > This is something we already do with the adv7511 bridge driver on msm, > rcar and arc (for 4.9) drivers. > > I've shared pseudo code on the kms driver and encoder chip's driver > side. I've also shared a diff that converts the tda998x driver to use > drm_bridge(uncompiled/untested). > > 1) Kms driver side: > > /* > * Create an encoder instance. Depending on the hardware represented > * by the KMS driver, the encoder can ops can either have some > * functionality, or be nops. In the case of tilcdc, the encoder > * funcs would be mostly nops. > */ > drm_encoder_helper_add(&kms_priv->encoder, &kms_encoder_helper_funcs); > drm_encoder_init(kms_pirv->drm, &kms_priv->encoder, &kms_encoder_funcs, >type, NULL); Then, how does this 'kms_priv' know the type of the encoder, this one being tied to the connector type at the end of the bridge chain? -- Ken ar c'hentañ| ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/
[Intel-gfx] i915 monitor hotplug on 4.1/4.4 PREEMPT
On Fri, Oct 21, 2016 at 04:21:53PM +0300, Nicolae Rosia wrote: > Hello, > > On Fri, Oct 21, 2016 at 3:27 PM, Daniel Vetter wrote: > > So yeah, "upgrade" is very likely is. > I see... > > i915.disable_power_well=0 seems to do the trick. > Is it safe to use? The kernel gets tainted because of this. We taint the kernel on principle if you touch any of these debug options, i.e. don't send us bug reports if it blows up. Because it'll be running untested codepaths. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/fb-helper: Don't call dirty callback for untouched clips
On Fri, Oct 21, 2016 at 01:57:28PM +0100, Chris Wilson wrote:
> On Fri, Oct 21, 2016 at 02:30:32PM +0200, Daniel Vetter wrote:
> > On Thu, Oct 20, 2016 at 05:00:35PM +0200, Takashi Iwai wrote:
> > > On Thu, 20 Oct 2016 16:56:04 +0200,
> > > Ville Syrjälä wrote:
> > > >
> > > > On Thu, Oct 20, 2016 at 04:39:52PM +0200, Takashi Iwai wrote:
> > > > > Since 4.7 kernel, we've seen the error messages like
> > > > >
> > > > > kernel: [TTM] Buffer eviction failed
> > > > > kernel: qxl :00:02.0: object_init failed for (4026540032,
> > > > > 0x0001)
> > > > > kernel: [drm:qxl_alloc_bo_reserved [qxl]] *ERROR* failed to allocate
> > > > > VRAM BO
> > > > >
> > > > > on QXL when switching and accessing on VT. The culprit was the
> > > > > generic deferred_io code (qxl driver switched to it since 4.7).
> > > > > There is a race between the dirty clip update and the call of
> > > > > callback.
> > > > >
> > > > > In drm_fb_helper_dirty(), the dirty clip is updated in the spinlock,
> > > > > while it kicks off the update worker outside the spinlock. Meanwhile
> > > > > the update worker clears the dirty clip in the spinlock, too. Thus,
> > > > > when drm_fb_helper_dirty() is called concurrently, schedule_work() is
> > > > > called after the clip is cleared in the first worker call.
> > > > >
> > > > > This patch addresses it by validating the clip before calling the
> > > > > dirty fb callback.
> > > > >
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98322
> > > > > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1003298
> > > > > Fixes: eaa434defaca ('drm/fb-helper: Add fb_deferred_io support')
> > > > > Cc:
> > > > > Signed-off-by: Takashi Iwai
> > > > > ---
> > > > > drivers/gpu/drm/drm_fb_helper.c | 13 +
> > > > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > > > b/drivers/gpu/drm/drm_fb_helper.c
> > > > > index 03414bde1f15..d790d205129e 100644
> > > > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > > > @@ -636,15 +636,20 @@ static void drm_fb_helper_dirty_work(struct
> > > > > work_struct *work)
> > > > > dirty_work);
> > > > > struct drm_clip_rect *clip = &helper->dirty_clip;
> > > > > struct drm_clip_rect clip_copy;
> > > > > + bool dirty;
> > > > > unsigned long flags;
> > > > >
> > > > > spin_lock_irqsave(&helper->dirty_lock, flags);
> > > > > - clip_copy = *clip;
> > > > > - clip->x1 = clip->y1 = ~0;
> > > > > - clip->x2 = clip->y2 = 0;
> > > > > + dirty = (clip->x1 < clip->x2 && clip->y1 < clip->y2);
> > > > > + if (dirty) {
> > > > > + clip_copy = *clip;
> > > > > + clip->x1 = clip->y1 = ~0;
> > > > > + clip->x2 = clip->y2 = 0;
> > > > > + }
> > > > > spin_unlock_irqrestore(&helper->dirty_lock, flags);
> > > > >
> > > > > - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
> > > > > + if (dirty)
> > > >
> > > > Could do it the other way too, ie. just make the copy, and then check
> > > > the
> > > > copy (can be done after dropping the lock even). Would avoid having to
> > > > add the 'dirty' variable.
> > >
> > > Sounds good. Let me try...
> >
> > Another thing: How do we prevent userspace from doing the same, i.e.
> > submitting an empty rectangle? Do we need to patch up
> > drm_mode_dirtyfb_ioctl too? Not much point if we fix this bug in the fb
> > helpers and leave the barn door wide open for userspace to oops drivers
> > ;-)
>
> I think of a use for sending an empty clip: where you don't want to
> push any new pixel data, but you do want to be sure that the pipeline
> has been flushed.
What exactly should an empty rectangle flush out? It's a bit unclear, but
for speed I guess drivers should be allowed to make dirty async ...
-Daniel
>
> The change I would suggest here would be
>
> dirty = clip->x1 <= clip->x2 && clip->y1 <= clip->y2
>
> as the bug is not the empty rectangle but the invalid one. However, that
> may be overkill, and none of the backends care about the empty rect!
>
> But, indeed, we do not validate the incoming dirtyfb either.
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> b/drivers/gpu/drm/drm_framebuffer.c
> index 49fd7db758e0..ada6a5517945 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -504,10 +504,13 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
> }
>
> if (num_clips && clips_ptr) {
> + int i;
> +
> if (num_clips < 0 || num_clips > DRM_MODE_FB_DIRTY_MAX_CLIPS)
> {
> ret = -EINVAL;
> goto out_err1;
> }
> +
> clips = kcalloc(num_clips, sizeof(*clips), GFP_KERNEL);
> if (!clips) {
> ret = -ENO
[PATCH v6 06/11] drm/i915: Enable i915 perf stream for Haswell OA unit
On Thu, Oct 20, 2016 at 11:10 PM, Chris Wilson
wrote:
> On Thu, Oct 20, 2016 at 10:19:05PM +0100, Robert Bragg wrote:
> > +int i915_gem_context_pin_legacy_rcs_state(struct drm_i915_private
> *dev_priv,
> > + struct i915_gem_context *ctx,
> > + u64 flags)
>
> This is still no.
>
> > +static int alloc_oa_buffer(struct drm_i915_private *dev_priv)
> > +{
> > + struct drm_i915_gem_object *bo;
> > + enum i915_map_type map;
> > + struct i915_vma *vma;
> > + int ret;
> > +
> > + BUG_ON(dev_priv->perf.oa.oa_buffer.obj);
> > +
> > + ret = i915_mutex_lock_interruptible(&dev_priv->drm);
> > + if (ret)
> > + return ret;
> > +
> > + BUILD_BUG_ON_NOT_POWER_OF_2(OA_BUFFER_SIZE);
> > + BUILD_BUG_ON(OA_BUFFER_SIZE < SZ_128K || OA_BUFFER_SIZE > SZ_16M);
> > +
> > + bo = i915_gem_object_create(&dev_priv->drm, OA_BUFFER_SIZE);
> > + if (IS_ERR(bo)) {
> > + DRM_ERROR("Failed to allocate OA buffer\n");
> > + ret = PTR_ERR(bo);
> > + goto unlock;
> > + }
> > + dev_priv->perf.oa.oa_buffer.obj = bo;
> > +
> > + ret = i915_gem_object_set_cache_level(bo, I915_CACHE_LLC);
> > + if (ret)
> > + goto err_unref;
> > +
> > + /* PreHSW required 512K alignment, HSW requires 16M */
> > + vma = i915_gem_object_ggtt_pin(bo, NULL, 0, SZ_16M, PIN_MAPPABLE);
> > + if (IS_ERR(vma)) {
> > + ret = PTR_ERR(vma);
> > + goto err_unref;
> > + }
> > + dev_priv->perf.oa.oa_buffer.vma = vma;
> > +
> > + map = HAS_LLC(dev_priv) ? I915_MAP_WB : I915_MAP_WC;
>
> You set the hw up to do coherent writes into the CPU cache, and then you
> request WC access to the pages? With set_cache_level(LLC) you can use
> MAP_WB on both llc and snoop based architectures. Fortunately this is
> only HSW!
>
hmm, yeah it looks like I unwittingly added this recently as part of a
rebase, I think from lazily copying some similar code from
intel_ringbuffer.c when I hit a conflict, without thinking more carefully,
sorry.
>
> > + dev_priv->perf.oa.oa_buffer.gtt_offset = i915_ggtt_offset(vma);
>
> I haven't spotted the advantage of storing both the ggtt_offset in
> addition to the vma (or the bo as well as the vma).
>
right, it looks like this can be cleaned up.
>
> > + dev_priv->perf.oa.oa_buffer.addr = i915_gem_object_pin_map(bo,
> map);
> > + if (IS_ERR(dev_priv->perf.oa.oa_buffer.addr)) {
> > + ret = PTR_ERR(dev_priv->perf.oa.oa_buffer.addr);
> > + goto err_unpin;
> > + }
>
> --
> Chris Wilson, Intel Open Source Technology Centre
>
Thanks,
- Robert
-- next part --
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/f35d133f/attachment.html>
[RFC] drm: add hint to userspace about whether a plane can scale
On Fri, Oct 21, 2016 at 09:26:22AM -0400, Rob Clark wrote: > On Fri, Oct 21, 2016 at 8:35 AM, Daniel Vetter wrote: > > On Fri, Oct 21, 2016 at 09:58:45AM +0100, Brian Starkey wrote: > >> Hi Rob, > >> > >> On Thu, Oct 20, 2016 at 04:17:14PM -0400, Rob Clark wrote: > >> > When you have a mix of planes that can scale and those that cannot > >> > scale, userspace really wants to have some hint to know which planes > >> > can definitely not scale so it knows to assign them to unscaled layers. > >> > I don't think it is fully possible to describe scaling constraints in > >> > a generic way, so I don't think it is even worth trying, so this is > >> > not a substitute for atomic TESTONLY step, but it does reduce the > >> > search-space for userspace. In the common case, most layers will not > >> > be scaled so knowing the best planes to pick for those layers is > >> > useful. > >> > >> Somewhat related to what Daniel mentioned on IRC about driver > >> consistency - how about making it "cannot_scale". This is then opt-in > >> for drivers, and should mean userspace can always trust it if it's > >> set. > >> > >> i.e. if cannot_scale == false, userspace can give it a shot, but if > >> cannot_scale == true it should never bother. > >> > >> Either way, even with a device-specific planner we would want this > >> hint to manage different HW versions so I'm in favour. But... > > > > I think first thing we should do is add some backoff heuristics to > > drm_hwcomposer to try the same plane also with a non-scaled surface (after > > having tried it with a scaled one). That would probably be as effective as > > adding "can_scale", but with the upshot that we're not at the mercy of > > drivers exposing it correctly. > > well, ignoring the fact that drm-hwc2 just tries one thing then falls > back to gl (which should be fixed but it is a big pile of c++11 code > so that will be someone elses job ;-))... > > I did think about this approach, but two many changing variables so > making userspace guess about this sort of thing just seems evil. Imo drm-hwc2 needs to be fixed. Adding new uabi because the current userspace makes a few too many assumption about how hw works (the only thing which is supposed to always work is one full-screen unscaled primary buffer) feels wrong. Imo the proper fix is to fix userspace to not scale in the absolute last-resort fallback path. Because that won't work on many i915 platforms either (or on many other chips fwiw). > > I'm always vary when we have the same limit checks 2 (in this case once in > > atomic_check, and once in the code that registers the property). And I'd > > like to avoid that as much as possible. We could avoid this by making the > > can_scale property mandatory, and enforcing it in the drm core. I.e. if > > it's not set, we reject scaled planes. That should give some decent > > motivation for driver writers to update them correctly. But without such a > > self-consistency check I don't really like this. It would be akin to > > adding "can_rotate" besides the "rotation" prop, and allowing drivers to > > botch things up and not register stuff consistently. > > Fair enough, I'll add a check in drm core. That is simple enough. I > guess remaining question should can_scale default to true? What's the point in making it true by default, i.e. lying by default? > I guess the first time someone tries to bring up drm-hwc or similar > (ie. something more complex than single fullscreen layer) on their hw, > they are going to run into a few bugs in their driver, so I guess I'd > be ok for it to default to false and let people fix it when the time > comes. Still not convinced that can_scale is worth it. I'd like to fix userspace first, before we roll out the duct-tape on the kernel ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm: Use u64 for intermediate dotclock calculations
On Fri, Oct 21, 2016 at 10:22:04AM -0400, Alex Deucher wrote:
> On Fri, Oct 21, 2016 at 10:15 AM, Chris Wilson
> wrote:
> > We have reached the era where monitor bandwidths now exceed 31bits in
> > frequency calculations, though as we stored them in kHz units we are
> > safe from overflow in the modelines for some time.
> >
> > [ 48.723720] UBSAN: Undefined behaviour in
> > ../drivers/gpu/drm/drm_modes.c:325:49
> > [ 48.726943] signed integer overflow:
> > [ 48.728503] 2240 * 100 cannot be represented in type 'int'
> >
> > Reported-by: Martin Liška
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98372
> > Signed-off-by: Chris Wilson
>
> Reviewed-by: Alex Deucher
Applied to drm-misc.
-Daniel
>
> > ---
> > drivers/gpu/drm/drm_modes.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 173b7d335834..f64ac86deb84 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -165,6 +165,7 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device
> > *dev, int hdisplay,
> > unsigned int vfieldrate, hperiod;
> > int hdisplay_rnd, hmargin, vdisplay_rnd, vmargin, vsync;
> > int interlace;
> > + u64 tmp;
> >
> > /* allocate the drm_display_mode structure. If failure, we will
> > * return directly
> > @@ -322,8 +323,11 @@ struct drm_display_mode *drm_cvt_mode(struct
> > drm_device *dev, int hdisplay,
> > drm_mode->vsync_end = drm_mode->vsync_start + vsync;
> > }
> > /* 15/13. Find pixel clock frequency (kHz for xf86) */
> > - drm_mode->clock = drm_mode->htotal * HV_FACTOR * 1000 / hperiod;
> > - drm_mode->clock -= drm_mode->clock % CVT_CLOCK_STEP;
> > + tmp = drm_mode->htotal; /* perform intermediate calcs in u64 */
> > + tmp *= HV_FACTOR * 1000;
> > + do_div(tmp, hperiod);
> > + tmp -= drm_mode->clock % CVT_CLOCK_STEP;
> > + drm_mode->clock = tmp;
> > /* 18/16. Find actual vertical frame frequency */
> > /* ignore - just set the mode flag for interlaced */
> > if (interlaced) {
> > --
> > 2.9.3
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/i915: Enable i915 perf stream for Haswell OA unit
Gen graphics hardware can be set up to periodically write snapshots of
performance counters into a circular buffer via its Observation
Architecture and this patch exposes that capability to userspace via the
i915 perf interface.
v2:
Make sure to initialize ->specific_ctx_id when opening, without
relying on _pin_notify hook, in case ctx already pinned.
Cc: Chris Wilson
Signed-off-by: Robert Bragg
Signed-off-by: Zhenyu Wang
---
drivers/gpu/drm/i915/i915_drv.h | 70 ++-
drivers/gpu/drm/i915/i915_gem_context.c | 22 +-
drivers/gpu/drm/i915/i915_perf.c| 1028 ++-
drivers/gpu/drm/i915/i915_reg.h | 338 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +-
include/uapi/drm/i915_drm.h | 70 ++-
6 files changed, 1507 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 28f3f77..b155ab0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1760,6 +1760,11 @@ struct intel_wm_config {
bool sprites_scaled;
};
+struct i915_oa_format {
+ u32 format;
+ int size;
+};
+
struct i915_oa_reg {
i915_reg_t addr;
u32 value;
@@ -1780,11 +1785,6 @@ struct i915_perf_stream_ops {
*/
void (*disable)(struct i915_perf_stream *stream);
- /* Return: true if any i915 perf records are ready to read()
-* for this stream.
-*/
- bool (*can_read)(struct i915_perf_stream *stream);
-
/* Call poll_wait, passing a wait queue that will be woken
* once there is something ready to read() for the stream
*/
@@ -1794,9 +1794,7 @@ struct i915_perf_stream_ops {
/* For handling a blocking read, wait until there is something
* to ready to read() for the stream. E.g. wait on the same
-* wait queue that would be passed to poll_wait() until
-* ->can_read() returns true (if its safe to call ->can_read()
-* without the i915 perf lock held).
+* wait queue that would be passed to poll_wait().
*/
int (*wait_unlocked)(struct i915_perf_stream *stream);
@@ -1836,11 +1834,28 @@ struct i915_perf_stream {
struct list_head link;
u32 sample_flags;
+ int sample_size;
struct i915_gem_context *ctx;
bool enabled;
- struct i915_perf_stream_ops *ops;
+ const struct i915_perf_stream_ops *ops;
+};
+
+struct i915_oa_ops {
+ void (*init_oa_buffer)(struct drm_i915_private *dev_priv);
+ int (*enable_metric_set)(struct drm_i915_private *dev_priv);
+ void (*disable_metric_set)(struct drm_i915_private *dev_priv);
+ void (*oa_enable)(struct drm_i915_private *dev_priv);
+ void (*oa_disable)(struct drm_i915_private *dev_priv);
+ void (*update_oacontrol)(struct drm_i915_private *dev_priv);
+ void (*update_hw_ctx_id_locked)(struct drm_i915_private *dev_priv,
+ u32 ctx_id);
+ int (*read)(struct i915_perf_stream *stream,
+ char __user *buf,
+ size_t count,
+ size_t *offset);
+ bool (*oa_buffer_is_empty)(struct drm_i915_private *dev_priv);
};
struct drm_i915_private {
@@ -2145,16 +2160,46 @@ struct drm_i915_private {
struct {
bool initialized;
+
struct mutex lock;
struct list_head streams;
+ spinlock_t hook_lock;
+
struct {
- u32 metrics_set;
+ struct i915_perf_stream *exclusive_stream;
+
+ u32 specific_ctx_id;
+
+ struct hrtimer poll_check_timer;
+ wait_queue_head_t poll_wq;
+ atomic_t pollin;
+
+ bool periodic;
+ int period_exponent;
+ int timestamp_frequency;
+
+ int tail_margin;
+
+ int metrics_set;
const struct i915_oa_reg *mux_regs;
int mux_regs_len;
const struct i915_oa_reg *b_counter_regs;
int b_counter_regs_len;
+
+ struct {
+ struct i915_vma *vma;
+ u8 *vaddr;
+ int format;
+ int format_size;
+ } oa_buffer;
+
+ u32 gen7_latched_oastatus1;
+
+ struct i915_oa_ops ops;
+ const struct i915_oa_format *oa_formats;
+ int n_builtin_sets;
} oa;
} perf;
@@ -3525,6 +3570,9 @@ struct drm_i915_gem_object *
i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
struct i915_gem_context *
i915_gem_context_create_gvt(struct drm_device *dev);
[PATCH 2/4] drm/i2c: tda998x: Remove obsolete drm_connector_register() call
On Wed, Oct 19, 2016 at 12:19:30PM +0300, Laurent Pinchart wrote: (sorry, I lost your original mail) > >>> DRM bridges indeed don't create encoders. That task is left to the display > >>> driver. The reason is the same as above: bridges can be chained (including > >>> with an internal encoder that is not modelled as a bridge, and that's a > >>> case > >>> we have today), while the KMS model exposes a single encoder to userspace. > >>> Exposing DRM encoder objects as part of the KMS UABI was probably a > >>> mistake. > >>> Better solutions would have been to expose no encoder at all or all > >>> encoders > >>> in the chain. We are however stuck with this model as we can't break the > >>> UABI. > >>> For that reason a DRM encoder object doesn't represent an encoder but a > >>> chain > >>> of encoders. As a DRM bridge models a single encoder, the DRM encoder > >>> object > >>> must be created at a higher level, in the display driver. I wonder why you created 'bridge's instead of simply adding links to the encoders? (that's what ASoC did: the audio CODECs are linked) This way, in simple cases (most cases), there would have been crtc -> (encoder -> connector) instead of crtc -> (bridge + encoder) -> (bridge + connector) without any changes in the actual (encoder + connector)s. -- Ken ar c'hentañ| ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/
[Bug 93649] [radeonsi] Graphics lockup while playing tf2
https://bugs.freedesktop.org/show_bug.cgi?id=93649 --- Comment #44 from Rosco P. Coltrane --- I don't know if it can be of any help, but I've been playing "7 days to die" during the last weeks, regularly for the last days, and I didn't encounter any kind of bug. Until yesterday evening where at my great surprise I had the same bug (freeze, sound loop) which totally crashed my machine once and only froze it (with a recovery after a few seconds) twice. I checked that no update occurred on the game files, on the steam runtime and on my OS between the days when it worked flawlessly and yesterday when it crashed 3 time in 15 minutes. So if it's not only related to files, could it be related to the hardware? Could it be a faulty card (HD7970), or maybe a mix between a faulty hardware and some software instruction? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161021/e60cc92d/attachment.html>
[PATCH v3 0/6] drm: Per-plane rotation etc.
From: Ville Syrjälä Remainder of the patches from the per-plane rotation series [1]. Rebased and also Rob's r-b picked up from [2]. [1] https://lists.freedesktop.org/archives/dri-devel/2016-September/119451.html [2] https://lists.freedesktop.org/archives/dri-devel/2016-September/119468.html Ville Syrjälä (6): drm/msm/mdp5: Use per-plane rotation property drm/msm/mdp5: Advertize 180 degree rotation drm: RIP mode_config->rotation_property drm/i915: Use & instead if == to check for rotations drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup drm/i915: Add horizontal mirroring support for CHV pipe B planes drivers/gpu/drm/drm_atomic.c | 6 ++--- drivers/gpu/drm/drm_blend.c | 32 - drivers/gpu/drm/drm_fb_helper.c | 7 +- drivers/gpu/drm/i915/intel_atomic_plane.c | 9 +++ drivers/gpu/drm/i915/intel_display.c | 39 --- drivers/gpu/drm/i915/intel_sprite.c | 39 --- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 35 --- include/drm/drm_blend.h | 2 -- include/drm/drm_crtc.h| 5 9 files changed, 88 insertions(+), 86 deletions(-) -- 2.7.4
[PATCH v2 1/6] drm/msm/mdp5: Use per-plane rotation property
From: Ville Syrjälä The global mode_config.rotation_property is going away, switch over to per-plane rotation_property. v2: Drop the BIT() Cc: Rob Clark Cc: Jilai Wang Cc: Archit Taneja Signed-off-by: Ville Syrjälä Reviewed-by: Rob Clark --- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index 951c002b05df..2653ad893ebc 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c @@ -75,15 +75,11 @@ static void mdp5_plane_install_rotation_property(struct drm_device *dev, !(mdp5_plane->caps & MDP_PIPE_CAP_VFLIP)) return; - if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = - drm_mode_create_rotation_property(dev, - DRM_ROTATE_0 | DRM_REFLECT_X | DRM_REFLECT_Y); - - if (dev->mode_config.rotation_property) - drm_object_attach_property(&plane->base, - dev->mode_config.rotation_property, - DRM_ROTATE_0); + drm_plane_create_rotation_property(plane, + DRM_ROTATE_0, + DRM_ROTATE_0 | + DRM_REFLECT_X | + DRM_REFLECT_Y); } /* helper to install properties which are common to planes and crtcs */ -- 2.7.4
[PATCH v2 2/6] drm/msm/mdp5: Advertize 180 degree rotation
From: Ville Syrjälä
Since the hardware can apparently do both X and Y reflection, we
can advertize also 180 degree rotation as thats just X+Y reflection.
v2: Drop the BIT()
Cc: Rob Clark
Cc: Jilai Wang
Cc: Archit Taneja
Signed-off-by: Ville Syrjälä
Reviewed-by: Rob Clark
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 2653ad893ebc..cf50d3ec8d1b 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -78,6 +78,7 @@ static void mdp5_plane_install_rotation_property(struct
drm_device *dev,
drm_plane_create_rotation_property(plane,
DRM_ROTATE_0,
DRM_ROTATE_0 |
+ DRM_ROTATE_180 |
DRM_REFLECT_X |
DRM_REFLECT_Y);
}
@@ -285,6 +286,8 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
plane_enabled(old_state), plane_enabled(state));
if (plane_enabled(state)) {
+ unsigned int rotation;
+
format = to_mdp_format(msm_framebuffer_format(state->fb));
if (MDP_FORMAT_IS_YUV(format) &&
!pipe_supports_yuv(mdp5_plane->caps)) {
@@ -305,8 +308,13 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}
- hflip = !!(state->rotation & DRM_REFLECT_X);
- vflip = !!(state->rotation & DRM_REFLECT_Y);
+ rotation = drm_rotation_simplify(state->rotation,
+DRM_ROTATE_0 |
+DRM_REFLECT_X |
+DRM_REFLECT_Y);
+ hflip = !!(rotation & DRM_REFLECT_X);
+ vflip = !!(rotation & DRM_REFLECT_Y);
+
if ((vflip && !(mdp5_plane->caps & MDP_PIPE_CAP_VFLIP)) ||
(hflip && !(mdp5_plane->caps & MDP_PIPE_CAP_HFLIP))) {
dev_err(plane->dev->dev,
@@ -677,6 +685,7 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
int pe_top[COMP_MAX], pe_bottom[COMP_MAX];
uint32_t hdecm = 0, vdecm = 0;
uint32_t pix_format;
+ unsigned int rotation;
bool vflip, hflip;
unsigned long flags;
int ret;
@@ -739,8 +748,12 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
config |= get_scale_config(format, src_h, crtc_h, false);
DBG("scale config = %x", config);
- hflip = !!(pstate->rotation & DRM_REFLECT_X);
- vflip = !!(pstate->rotation & DRM_REFLECT_Y);
+ rotation = drm_rotation_simplify(pstate->rotation,
+DRM_ROTATE_0 |
+DRM_REFLECT_X |
+DRM_REFLECT_Y);
+ hflip = !!(rotation & DRM_REFLECT_X);
+ vflip = !!(rotation & DRM_REFLECT_Y);
spin_lock_irqsave(&mdp5_plane->pipe_lock, flags);
--
2.7.4
[PATCH v2 4/6] drm/i915: Use & instead if == to check for rotations
From: Ville Syrjälä
Using == to check for 180 degree rotation only works as long as the
reflection bits aren't set. That will change soon enough for CHV, so
let's stop doing things the wrong way.
v2: Drop the BIT()
Signed-off-by: Ville Syrjälä
Reviewed-by: Chris Wilson
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/intel_display.c | 8
drivers/gpu/drm/i915/intel_sprite.c | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a94f7d195db4..7aaea7a44c0a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3087,7 +3087,7 @@ static void i9xx_update_primary_plane(struct drm_plane
*primary,
intel_crtc->dspaddr_offset =
intel_compute_tile_offset(&x, &y, plane_state, 0);
- if (rotation == DRM_ROTATE_180) {
+ if (rotation & DRM_ROTATE_180) {
dspcntr |= DISPPLANE_ROTATE_180;
x += (crtc_state->pipe_src_w - 1);
@@ -3188,7 +3188,7 @@ static void ironlake_update_primary_plane(struct
drm_plane *primary,
intel_crtc->dspaddr_offset =
intel_compute_tile_offset(&x, &y, plane_state, 0);
- if (rotation == DRM_ROTATE_180) {
+ if (rotation & DRM_ROTATE_180) {
dspcntr |= DISPPLANE_ROTATE_180;
if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
@@ -10871,7 +10871,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc,
u32 base,
if (HAS_DDI(dev_priv))
cntl |= CURSOR_PIPE_CSC_ENABLE;
- if (plane_state->base.rotation == DRM_ROTATE_180)
+ if (plane_state->base.rotation & DRM_ROTATE_180)
cntl |= CURSOR_ROTATE_180;
}
@@ -10917,7 +10917,7 @@ static void intel_crtc_update_cursor(struct drm_crtc
*crtc,
/* ILK+ do this automagically */
if (HAS_GMCH_DISPLAY(dev_priv) &&
- plane_state->base.rotation == DRM_ROTATE_180) {
+ plane_state->base.rotation & DRM_ROTATE_180) {
base += (plane_state->base.crtc_h *
plane_state->base.crtc_w - 1) * 4;
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 43d0350856e7..3821cf2a8209 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -436,7 +436,7 @@ vlv_update_plane(struct drm_plane *dplane,
intel_add_fb_offsets(&x, &y, plane_state, 0);
sprsurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
- if (rotation == DRM_ROTATE_180) {
+ if (rotation & DRM_ROTATE_180) {
sprctl |= SP_ROTATE_180;
x += src_w;
@@ -566,7 +566,7 @@ ivb_update_plane(struct drm_plane *plane,
intel_add_fb_offsets(&x, &y, plane_state, 0);
sprsurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
- if (rotation == DRM_ROTATE_180) {
+ if (rotation & DRM_ROTATE_180) {
sprctl |= SPRITE_ROTATE_180;
/* HSW and BDW does this automagically in hardware */
@@ -700,7 +700,7 @@ ilk_update_plane(struct drm_plane *plane,
intel_add_fb_offsets(&x, &y, plane_state, 0);
dvssurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
- if (rotation == DRM_ROTATE_180) {
+ if (rotation & DRM_ROTATE_180) {
dvscntr |= DVS_ROTATE_180;
x += src_w;
--
2.7.4
[PATCH v3 6/6] drm/i915: Add horizontal mirroring support for CHV pipe B planes
From: Ville Syrjälä
The primary and sprite planes on CHV pipe B support horizontal
mirroring. Expose it to the world.
Sadly the hardware ignores the mirror bit when the rotate bit is
set, so we'll have to reject the 180+X case.
v2: Drop the BIT()
v3: Pass dev_priv instead of dev to IS_CHERRYVIEW()
Signed-off-by: Ville Syrjälä
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 9 +
drivers/gpu/drm/i915/intel_display.c | 9 +
drivers/gpu/drm/i915/intel_sprite.c | 9 +
3 files changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index c762ae549a1c..157248c6288a 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -108,6 +108,7 @@ intel_plane_destroy_state(struct drm_plane *plane,
static int intel_plane_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)
{
+ struct drm_i915_private *dev_priv = to_i915(plane->dev);
struct drm_crtc *crtc = state->crtc;
struct intel_crtc *intel_crtc;
struct intel_crtc_state *crtc_state;
@@ -169,6 +170,14 @@ static int intel_plane_atomic_check(struct drm_plane
*plane,
}
}
+ /* CHV ignores the mirror bit when the rotate bit is set :( */
+ if (IS_CHERRYVIEW(dev_priv) &&
+ state->rotation & DRM_ROTATE_180 &&
+ state->rotation & DRM_REFLECT_X) {
+ DRM_DEBUG_KMS("Cannot rotate and reflect at the same time\n");
+ return -EINVAL;
+ }
+
intel_state->base.visible = false;
ret = intel_plane->check_plane(plane, crtc_state, intel_state);
if (ret)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 533442ac713c..190dfee17955 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3081,6 +3081,9 @@ static void i9xx_update_primary_plane(struct drm_plane
*primary,
if (rotation & DRM_ROTATE_180)
dspcntr |= DISPPLANE_ROTATE_180;
+ if (rotation & DRM_REFLECT_X)
+ dspcntr |= DISPPLANE_MIRROR;
+
if (IS_G4X(dev_priv))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
@@ -3093,6 +3096,8 @@ static void i9xx_update_primary_plane(struct drm_plane
*primary,
if (rotation & DRM_ROTATE_180) {
x += crtc_state->pipe_src_w - 1;
y += crtc_state->pipe_src_h - 1;
+ } else if (rotation & DRM_REFLECT_X) {
+ x += crtc_state->pipe_src_w - 1;
}
linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
@@ -15080,6 +15085,10 @@ static struct drm_plane
*intel_primary_plane_create(struct drm_device *dev,
supported_rotations =
DRM_ROTATE_0 | DRM_ROTATE_90 |
DRM_ROTATE_180 | DRM_ROTATE_270;
+ } else if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) {
+ supported_rotations =
+ DRM_ROTATE_0 | DRM_ROTATE_180 |
+ DRM_REFLECT_X;
} else if (INTEL_GEN(dev_priv) >= 4) {
supported_rotations =
DRM_ROTATE_0 | DRM_ROTATE_180;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 9e002811e697..d4c332115082 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -430,6 +430,9 @@ vlv_update_plane(struct drm_plane *dplane,
if (rotation & DRM_ROTATE_180)
sprctl |= SP_ROTATE_180;
+ if (rotation & DRM_REFLECT_X)
+ sprctl |= SP_MIRROR;
+
/* Sizes are 0 based */
src_w--;
src_h--;
@@ -442,6 +445,8 @@ vlv_update_plane(struct drm_plane *dplane,
if (rotation & DRM_ROTATE_180) {
x += src_w;
y += src_h;
+ } else if (rotation & DRM_REFLECT_X) {
+ x += src_w;
}
linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
@@ -1132,6 +1137,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe,
int plane)
supported_rotations =
DRM_ROTATE_0 | DRM_ROTATE_90 |
DRM_ROTATE_180 | DRM_ROTATE_270;
+ } else if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) {
+ supported_rotations =
+ DRM_ROTATE_0 | DRM_ROTATE_180 |
+ DRM_REFLECT_X;
} else {
supported_rotations =
DRM_ROTATE_0 | DRM_ROTATE_180;
--
2.7.4
[PATCH v2 5/6] drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup
From: Ville Syrjälä
Move the plane control register rotation setup away from the
coordinate munging code. This will result in neater looking
code once we add reflection support for CHV.
v2: Drop the BIT(), drop some usless parens,
Signed-off-by: Ville Syrjälä
Reviewed-by: Chris Wilson
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/intel_display.c | 24 +---
drivers/gpu/drm/i915/intel_sprite.c | 26 ++
2 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7aaea7a44c0a..533442ac713c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3078,6 +3078,9 @@ static void i9xx_update_primary_plane(struct drm_plane
*primary,
fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
dspcntr |= DISPPLANE_TILED;
+ if (rotation & DRM_ROTATE_180)
+ dspcntr |= DISPPLANE_ROTATE_180;
+
if (IS_G4X(dev_priv))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
@@ -3088,10 +3091,8 @@ static void i9xx_update_primary_plane(struct drm_plane
*primary,
intel_compute_tile_offset(&x, &y, plane_state, 0);
if (rotation & DRM_ROTATE_180) {
- dspcntr |= DISPPLANE_ROTATE_180;
-
- x += (crtc_state->pipe_src_w - 1);
- y += (crtc_state->pipe_src_h - 1);
+ x += crtc_state->pipe_src_w - 1;
+ y += crtc_state->pipe_src_h - 1;
}
linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
@@ -3180,6 +3181,9 @@ static void ironlake_update_primary_plane(struct
drm_plane *primary,
if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
dspcntr |= DISPPLANE_TILED;
+ if (rotation & DRM_ROTATE_180)
+ dspcntr |= DISPPLANE_ROTATE_180;
+
if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
@@ -3188,13 +3192,11 @@ static void ironlake_update_primary_plane(struct
drm_plane *primary,
intel_crtc->dspaddr_offset =
intel_compute_tile_offset(&x, &y, plane_state, 0);
- if (rotation & DRM_ROTATE_180) {
- dspcntr |= DISPPLANE_ROTATE_180;
-
- if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
- x += (crtc_state->pipe_src_w - 1);
- y += (crtc_state->pipe_src_h - 1);
- }
+ /* HSW+ does this automagically in hardware */
+ if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv) &&
+ rotation & DRM_ROTATE_180) {
+ x += crtc_state->pipe_src_w - 1;
+ y += crtc_state->pipe_src_h - 1;
}
linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 3821cf2a8209..9e002811e697 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -427,6 +427,9 @@ vlv_update_plane(struct drm_plane *dplane,
if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
sprctl |= SP_TILED;
+ if (rotation & DRM_ROTATE_180)
+ sprctl |= SP_ROTATE_180;
+
/* Sizes are 0 based */
src_w--;
src_h--;
@@ -437,8 +440,6 @@ vlv_update_plane(struct drm_plane *dplane,
sprsurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
if (rotation & DRM_ROTATE_180) {
- sprctl |= SP_ROTATE_180;
-
x += src_w;
y += src_h;
}
@@ -546,6 +547,9 @@ ivb_update_plane(struct drm_plane *plane,
if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
sprctl |= SPRITE_TILED;
+ if (rotation & DRM_ROTATE_180)
+ sprctl |= SPRITE_ROTATE_180;
+
if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
sprctl &= ~SPRITE_TRICKLE_FEED_DISABLE;
else
@@ -566,14 +570,11 @@ ivb_update_plane(struct drm_plane *plane,
intel_add_fb_offsets(&x, &y, plane_state, 0);
sprsurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
- if (rotation & DRM_ROTATE_180) {
- sprctl |= SPRITE_ROTATE_180;
-
- /* HSW and BDW does this automagically in hardware */
- if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
- x += src_w;
- y += src_h;
- }
+ /* HSW+ does this automagically in hardware */
+ if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv) &&
+ rotation & DRM_ROTATE_180) {
+ x += src_w;
+ y += src_h;
}
linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
@@ -684,6 +685,9 @@ ilk_update_plane(struct drm_plane *plane,
if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
[PATCH v2 3/6] drm: RIP mode_config->rotation_property
From: Ville Syrjälä
Now that all drivers have been converted over to the per-plane rotation
property, we can just nuke the global rotation property.
v2: Rebase due to BIT(),__builtin_ffs() & co.
Deal with superfluous code shuffling
Signed-off-by: Ville Syrjälä
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/drm_atomic.c| 6 ++
drivers/gpu/drm/drm_blend.c | 32
drivers/gpu/drm/drm_fb_helper.c | 7 +--
include/drm/drm_blend.h | 2 --
include/drm/drm_crtc.h | 5 -
5 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f81706387889..1b5a32df9a9a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -705,8 +705,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
state->src_w = val;
} else if (property == config->prop_src_h) {
state->src_h = val;
- } else if (property == config->rotation_property ||
- property == plane->rotation_property) {
+ } else if (property == plane->rotation_property) {
if (!is_power_of_2(val & DRM_ROTATE_MASK))
return -EINVAL;
state->rotation = val;
@@ -766,8 +765,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_w;
} else if (property == config->prop_src_h) {
*val = state->src_h;
- } else if (property == config->rotation_property ||
- property == plane->rotation_property) {
+ } else if (property == plane->rotation_property) {
*val = state->rotation;
} else if (property == plane->zpos_property) {
*val = state->zpos;
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index e52aece30900..1f2412c7ccfd 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -89,7 +89,7 @@
* On top of this basic transformation additional properties can be exposed by
* the driver:
*
- * - Rotation is set up with drm_mode_create_rotation_property(). It adds a
+ * - Rotation is set up with drm_plane_create_rotation_property(). It adds a
* rotation and reflection step between the source and destination
rectangles.
* Without this property the rectangle is only scaled, but not rotated or
* reflected.
@@ -105,18 +105,12 @@
*/
/**
- * drm_mode_create_rotation_property - create a new rotation property
- * @dev: DRM device
+ * drm_plane_create_rotation_property - create a new rotation property
+ * @plane: drm plane
+ * @rotation: initial value of the rotation property
* @supported_rotations: bitmask of supported rotations and reflections
*
* This creates a new property with the selected support for transformations.
- * The resulting property should be stored in @rotation_property in
- * &drm_mode_config. It then must be attached to each plane which supports
- * rotations using drm_object_attach_property().
- *
- * FIXME: Probably better if the rotation property is created on each plane,
- * like the zpos property. Otherwise it's not possible to allow different
- * rotation modes on different planes.
*
* Since a rotation by 180° degress is the same as reflecting both along the x
* and the y axis the rotation property is somewhat redundant. Drivers can use
@@ -144,24 +138,6 @@
* rotation. After reflection, the rotation is applied to the image sampled
from
* the source rectangle, before scaling it to fit the destination rectangle.
*/
-struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
- unsigned int
supported_rotations)
-{
- static const struct drm_prop_enum_list props[] = {
- { __builtin_ffs(DRM_ROTATE_0) - 1, "rotate-0" },
- { __builtin_ffs(DRM_ROTATE_90) - 1, "rotate-90" },
- { __builtin_ffs(DRM_ROTATE_180) - 1, "rotate-180" },
- { __builtin_ffs(DRM_ROTATE_270) - 1, "rotate-270" },
- { __builtin_ffs(DRM_REFLECT_X) - 1, "reflect-x" },
- { __builtin_ffs(DRM_REFLECT_Y) - 1, "reflect-y" },
- };
-
- return drm_property_create_bitmask(dev, 0, "rotation",
- props, ARRAY_SIZE(props),
- supported_rotations);
-}
-EXPORT_SYMBOL(drm_mode_create_rotation_property);
-
int drm_plane_create_rotation_property(struct drm_plane *plane,
unsigned int rotation,
unsigned int supported_rotations)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index e0d428f9d1cb..83dbae0fabcf 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -392,15 +392,10 @@ static int restore_fbdev_mode(struct drm_fb_helper
[PATCH 00/18] dim: split out drm-misc/tip.git
Hi all, So here's all the bits I think we need for: - split out drm-misc.git repo - rolling model in drm-misc to avoid the merge window, like for drm-intel branches. - split out rerere stuff into a shared drm-tip.git repo - sharing .git metadata using git worktrees. Conversion howto: 1. Update to latest dim, but store it somewhere outside of the maintainer-tools branch. 2. Remove the drm-intel-rerere, drm-intel-nightly and maintainter-tools branches. With the new dim .git repos can be shared (assuming your git has worktree support). 3. Run dim setup. I'll probably complain about a few missing remotes. Add them if needed - dim doesn't do this automatically to allow you to bikeshed the remote names ;-) 4. Profit! If possible I'd like to roll this all out officially next week or so, so pls scream if you see any trouble. Cheers, Daniel Chris Wilson (1): dim-update-next: Update DRIVER_TIMESTAMP Daniel Vetter (17): dim: Extract TODO dim: Autocheck for up-to-dateness dim: echoerr helper for printing to stderr dim: autodetect remotes, first part for dim_setup dim: support git worktree for aux checkouts dim: Nuke nightly-forget dim: autodetect branches in rebuild-nightly dim: remove integration-tree remotes dim: Split out drm-nightly.git dim: s/drm-nightly/drm-tip dim: use git branch --list dim: add revert-rerere dim: use get_maintainers.pl in dim fixes dim: support multiple remotes for branches dim: remove DIM_DRM_UPSTREAM_REMOTE config var dim: Adapat create/remove-branch dim: Make update_linux_next multi-repo compliant TODO | 25 dim| 433 - dim.rst| 37 +++-- dimrc.sample | 6 +- drm-intel-flow.dot | 18 +-- drm-intel.rst | 20 +-- qf | 11 -- 7 files changed, 333 insertions(+), 217 deletions(-) create mode 100644 TODO -- 2.9.3
[PATCH 01/18] dim: Extract TODO
Just maybe a bit more visibility, the scripts are growing. Signed-off-by: Daniel Vetter --- TODO | 27 +++ dim | 17 - qf | 11 --- 3 files changed, 27 insertions(+), 28 deletions(-) create mode 100644 TODO diff --git a/TODO b/TODO new file mode 100644 index ..ac0f27763dfa --- /dev/null +++ b/TODO @@ -0,0 +1,27 @@ +dim: +- extract the integration tree logic and make it generally useful, maybe for a + drm-integration tree ... +- Improve nightly-forget to forget a specific merge instead of just the first + dinq/dif merge. +- add option to check-patch to check stdin +- integrate ninja-check? Or too much checkers considered harmful? + https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000554.html +- add patchwork Link: also after manually resolving conflicts in drm + apply-resolved +- pull in dim extract-tags tool from Ville +- allow dim rebuild-nightly to pull branches from local trees in dry-run mode. + This is useful to confirm a backmerge is indeed correct, by comparing the + resulting -nightly with the old one. Current the branch must be pushed out + first for rebuild-nightly to pick it up, which means the merge can't be + fixed any more. + +qf: +- get better at preventing and cleaning up a mess when switching branches + while there's still applied quilt patches around ... +- combine quilt annotate and git blame into one tool +- use the index a bit more to e.g. stage all applied quilt patches, then use + the output of git diff to refresh a quilt patch +- use git commit-tree and git write-tree in the setup code instead of the + current high-level hacks +- track/restore the topmost patch maybe? +- synchronize quilt notes in qf push and qf fetch diff --git a/dim b/dim index 5fb3a0fee7ff..57ad4fcf9767 100755 --- a/dim +++ b/dim @@ -27,23 +27,6 @@ # drm-intel-next maintainer script -# TODO -# - extract the integration tree logic and make it generally useful, maybe for a -# drm-integration tree ... -# - Improve nightly-forget to forget a specific merge instead of just the first -# dinq/dif merge. -# - add option to check-patch to check stdin -# - integrate ninja-check? Or too much checkers considered harmful? -# https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000554.html -# - add patchwork Link: also after manually resolving conflicts in drm -# apply-resolved -# - pull in dim extract-tags tool from Ville -# - allow dim rebuild-nightly to pull branches from local trees in dry-run mode. -# This is useful to confirm a backmerge is indeed correct, by comparing the -# resulting -nightly with the old one. Current the branch must be pushed out -# first for rebuild-nightly to pick it up, which means the merge can't be -# fixed any more. - # fail on any goof-up set -e diff --git a/qf b/qf index 4e9cb03f31fe..31b9f3bae0a2 100755 --- a/qf +++ b/qf @@ -26,17 +26,6 @@ # quilt git flow script -# TODO -# - get better at preventing and cleaning up a mess when switching branches -# while there's still applied quilt patches around ... -# - combine quilt annotate and git blame into one tool -# - use the index a bit more to e.g. stage all applied quilt patches, then use -# the output of git diff to refresh a quilt patch -# - use git commit-tree and git write-tree in the setup code instead of the -# current high-level hacks -# - track/restore the topmost patch maybe? -# - synchronize quilt notes in qf push and qf fetch - # config QUILT_PREFIX=quilt/ -- 2.9.3
[PATCH 02/18] dim: Autocheck for up-to-dateness
Exits script to annoy people roughly every 100th time ...
Also switch to the magic @{upstream} reference, in case the remote is
not called origin (which is pretty normal in case of using git
worktree).
Signed-off-by: Daniel Vetter
---
dim | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/dim b/dim
index 57ad4fcf9767..192d6ee10838 100755
--- a/dim
+++ b/dim
@@ -176,13 +176,17 @@ function dim_uptodate
exit 1
fi
- if ! git --git-dir=$DIM_PREFIX/maintainer-tools/.git show
origin/maintainer-tools:dim |\
+ if ! git --git-dir=$DIM_PREFIX/maintainer-tools/.git show
@{upstream}:dim |\
diff "$using" - >& /dev/null; then
echo "$dim: not running upstream version of the script." >&2
exit 1
fi
}
+if [[ "$((`date +%s` % 100))" -eq "0" ]] ; then
+dim_uptodate
+fi
+
# get message id from file
# $1 = file
message_get_id ()
--
2.9.3
[PATCH 03/18] dim: echoerr helper for printing to stderr
Signed-off-by: Daniel Vetter
---
dim | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/dim b/dim
index 192d6ee10838..2601bb7dbbad 100755
--- a/dim
+++ b/dim
@@ -102,12 +102,17 @@ DRY=
FORCE=
HELP=
+function echoerr
+{
+ echo "$@" >&2
+}
+
function warn_or_fail
{
if [[ $FORCE ]] ; then
- echo WARNING: $1, but continuing
+ echoerr WARNING: $1, but continuing
else
- echo ERROR: $1, aborting
+ echoerr ERROR: $1, aborting
exit 1
fi
}
@@ -128,7 +133,7 @@ while getopts hdfi opt; do
HELP=1
;;
*)
- echo "See '$dim help' for more information." >&2
+ echoerr "See '$dim help' for more information."
exit
esac
done
@@ -167,18 +172,18 @@ function dim_uptodate
local using="${BASH_SOURCE[0]}"
if [[ ! -e "$using" ]]; then
- echo "$dim: could not figure out the version being used
($using)." >&2
+ echoerr "$dim: could not figure out the version being used
($using)."
exit 1
fi
if [[ ! -e "$DIM_PREFIX/maintainer-tools/.git" ]]; then
- echo "$dim: could not find the upstream repo for $dim." >&2
+ echoerr "$dim: could not find the upstream repo for $dim."
exit 1
fi
if ! git --git-dir=$DIM_PREFIX/maintainer-tools/.git show
@{upstream}:dim |\
diff "$using" - >& /dev/null; then
- echo "$dim: not running upstream version of the script." >&2
+ echoerr "$dim: not running upstream version of the script."
exit 1
fi
}
@@ -1280,6 +1285,6 @@ subcmd_func=dim_${subcmd//-/_}
if declare -f $subcmd_func >/dev/null; then
$subcmd_func "$@"
else
- echo "$dim: '$subcommand' is not a dim command." >&2
+ echoerr "$dim: '$subcommand' is not a dim command."
dim_usage
fi
--
2.9.3
[PATCH 05/18] dim: support git worktree for aux checkouts
If available by default. This saves quite a pile of disk-space that imo making it the default is justified. Note that this will break the rebuild-nightly script for now, follow-up patches will fix that. Signed-off-by: Daniel Vetter --- dim | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/dim b/dim index 90eb553c6575..80baf24e4ad7 100755 --- a/dim +++ b/dim @@ -1072,15 +1072,24 @@ function setup_aux_checkout # name remote echo "Setting up $dir ..." if [ ! -d $dir ]; then - git clone --reference=$DIM_PREFIX/$DIM_DRM_INTEL/.git $remote_url $dir - cd $dir - git config remote.origin.url $remote_url - echo "$DIM_PREFIX/$DIM_DRM_INTEL/.git/objects" > .git/objects/info/alternates - git repack -a -d -l - remote=origin + if git help worktree &> /dev/null ; then + cd $DIM_PREFIX/$DIM_DRM_INTEL + remote=`get_remote_name $remote_url` + if ! git branch | grep $name > /dev/null ; then + git branch --track $name $remote/$name + fi + git worktree add ../$dir $name + else + git clone --reference=$DIM_PREFIX/$DIM_DRM_INTEL/.git $remote_url $dir + cd $dir + git config remote.origin.url $remote_url + echo "$DIM_PREFIX/$DIM_DRM_INTEL/.git/objects" > .git/objects/info/alternates + git repack -a -d -l + remote=origin + fi else cd $dir - remote=`get_remote_name $drm_intel_ssh` + remote=`get_remote_name $remote_url` fi if ! git branch | grep $name > /dev/null ; then git checkout -t $remote/$name -- 2.9.3
