[PATCH v2] drm/fences: add DOC: for explicit fencing
From: Gustavo Padovan
Document IN_FENCE_FD and OUT_FENCE_PTR properties.
v2: incorporate comments from Daniel Vetter
Signed-off-by: Gustavo Padovan
---
Documentation/gpu/drm-kms.rst | 6 +
drivers/gpu/drm/drm_atomic.c | 52 +++
2 files changed, 58 insertions(+)
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 568f3c2..cdc9539 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -287,6 +287,12 @@ Tile Group Property
.. kernel-doc:: drivers/gpu/drm/drm_connector.c
:doc: Tile group
+Explicit Fencing Properties
+---
+
+.. kernel-doc:: drivers/gpu/drm/drm_atomic.c
+ :doc: explicit fencing properties
+
Existing KMS Properties
---
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index b476ec5..f548e68 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1809,6 +1809,58 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_atomic_clean_old_fb);
+/**
+ * DOC: explicit fencing properties
+ *
+ * Explicit fencing allows userspace to control the buffer synchronization
+ * between devices. A Fence or a group of fences are transfered to/from
+ * userspace using Sync File fds and there are two DRM properties for that.
+ * IN_FENCE_FD on each DRM Plane to send fences to the kernel and
+ * OUT_FENCE_PTR on each DRM CRTC to receive fences from the kernel.
+ *
+ * As a contrast, with implicit fencing the kernel keeps track of any
+ * ongoing rendering, and automatically ensures that the atomic update waits
+ * for any pending rendering to complete. For shared buffers represented with
+ * a struct &dma_buf this is tracked in &reservation_object structures.
+ * Implicit syncing is how Linux traditionally worked (e.g. DRI2/3 on X.org),
+ * whereas explicit fencing is what Android wants.
+ *
+ * "IN_FENCE_FDâ:
+ * Use this property to pass a fence that DRM should wait on before
+ * proceeding with the Atomic Commit request and show the framebuffer for
+ * the plane on the screen. The fence can be either a normal fence or a
+ * merged one, the sync_file framework will handle both cases and use a
+ * fence_array if a merged fence is received. Passing -1 here means no
+ * fences to wait on.
+ *
+ * If the Atomic Commit request has the DRM_MODE_ATOMIC_TEST_ONLY flag
+ * it will only check if the Sync File is a valid one.
+ *
+ * On the driver side the fence is stored on the @fence parameter of
+ * struct &drm_plane_state. Drivers which also support implicit fencing
+ * should set the implicit fence using drm_atomic_set_fence_for_plane(),
+ * to make sure there's consistent behaviour between drivers in precedence
+ * of implicit vs. explicit fencing.
+ *
+ * "OUT_FENCE_PTRâ:
+ * Use this property to pass a file descriptor pointer to DRM. Once the
+ * Atomic Commit request call returns OUT_FENCE_PTR will be filled with
+ * the file descriptor number of a Sync File. This Sync File contains the
+ * CRTC fence that will be signaled when all framebuffers present on the
+ * Atomic Commit * request for that given CRTC are scanned out on the
+ * screen.
+ *
+ * The Atomic Commit request fails if a invalid pointer is passed. If the
+ * Atomic Commit request fails for any other reason the out fence fd
+ * returned will be -1. On a Atomic Commit with the
+ * DRM_MODE_ATOMIC_TEST_ONLY flag the out fence will also be set to -1.
+ *
+ * Note that out-fences don't have a special interface to drivers and are
+ * internally represented by a struct &drm_pending_vblank_event in struct
+ * &drm_crtc_state, which is also used by the async atomic commit helpers
+ * and for the DRM event handling for existing userspace.
+ */
+
static struct dma_fence *get_crtc_fence(struct drm_crtc *crtc)
{
struct dma_fence *fence;
--
2.5.5
[Bug 98005] VCE dual instance encoding inconsistent since st/va: enable dual instances encode by sync surface
https://bugs.freedesktop.org/show_bug.cgi?id=98005 --- Comment #21 from Andy Furniss --- Still the same issue with latest patches. I can sometimes get vbr to be affected as well. I made a smaller test file, it's re-coded with software to 24 fps as the real master is /1001 and so would be wrong without an additional patch anyway. I am just doing repeated decode/encode from/to ram and changing cpu/gpu settings to get unlucky timing - I guess on a different box you could luck out of seeing it, just initial testing with raw video input from ram seemed to work for me. This transcode seems to be mostly wrong with my cpu on perf and gpu on auto. for x in $(seq 1 20); do gst-launch-1.0 -f filesrc location=/mnt/ramdisk/in-24fps.mp4 ! qtdemux ! vaapidecode ! queue ! vaapih264enc rate-control=cbr bitrate=5000 ! video/x-h264,profile=baseline ! filesink location=/mnt/ramdisk/cbr-$x.264; done in-24fps.mp4 - https://drive.google.com/file/d/0BxP5-S1t9VEEaWNkZXdmb01GYmM/view?usp=sharing -- 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/20161122/76f2c080/attachment.html>
[Bug 98761] [regression][radeonsi][polaris]"radeonsi: set IF_THRESHOLD to 3" breaks Witcher2's ground
https://bugs.freedesktop.org/show_bug.cgi?id=98761 --- Comment #12 from Marek Olšák --- Created attachment 128128 --> https://bugs.freedesktop.org/attachment.cgi?id=128128&action=edit affected shader (good asm) -- 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/20161122/18229192/attachment.html>
[Bug 110421] Reading debugfs file /sys/kernel/debug/dri/0/amdgpu_regs hangs the machine
https://bugzilla.kernel.org/show_bug.cgi?id=110421 Vedran MiletiÄ changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |WILL_NOT_FIX -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 98761] [regression][radeonsi][polaris]"radeonsi: set IF_THRESHOLD to 3" breaks Witcher2's ground
https://bugs.freedesktop.org/show_bug.cgi?id=98761 --- Comment #13 from Marek Olšák --- Created attachment 128129 --> https://bugs.freedesktop.org/attachment.cgi?id=128129&action=edit affected shader (good->bad diff) -- 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/20161122/0462e2b3/attachment.html>
[Bug 188091] Resume with two monitors, second monitor is not resumed until VT switch
https://bugzilla.kernel.org/show_bug.cgi?id=188091 --- Comment #5 from Greg White --- I should also note this is easily reproducible but still intermittent. I'd say 80% of the time, I see this. It's also not always one monitor - it can be either of them. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 98761] [regression][radeonsi][polaris]"radeonsi: set IF_THRESHOLD to 3" breaks Witcher2's ground
https://bugs.freedesktop.org/show_bug.cgi?id=98761 --- Comment #14 from Marek Olšák --- Created attachment 128130 --> https://bugs.freedesktop.org/attachment.cgi?id=128130&action=edit affected shader (bad asm) -- 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/20161122/604f7048/attachment.html>
[Bug 188091] Resume with two monitors, second monitor is not resumed until VT switch
https://bugzilla.kernel.org/show_bug.cgi?id=188091 --- Comment #6 from Vedran MiletiÄ --- (In reply to Greg White from comment #5) > I should also note this is easily reproducible but still intermittent. I'd > say 80% of the time, I see this. It's also not always one monitor - it can > be either of them. Interesting. I have a very similar issue on Fiji (bug 176311) with two DP monitors, but in case of Fiji it is always one monitor that will be blank until the VT switch happens. And the error message is different. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 98761] [regression][radeonsi][polaris]"radeonsi: set IF_THRESHOLD to 3" breaks Witcher2's ground
https://bugs.freedesktop.org/show_bug.cgi?id=98761 --- Comment #15 from Marek Olšák --- It looks like m0 isn't restored for v_interp instructions. -- 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/20161122/04d21508/attachment.html>
[Bug 98005] VCE dual instance encoding inconsistent since st/va: enable dual instances encode by sync surface
https://bugs.freedesktop.org/show_bug.cgi?id=98005 --- Comment #22 from Andy Furniss --- FWIW the reference decoder on a bad result looks like below when it hits the first reversal it bails. Seems to always be the same place - this with the default gop = 30. 0(IDR)0 020 4:2:0 119 1( P )2 120 4:2:0 40 2( P )4 221 4:2:0 41 3( P )6 321 4:2:0 41 4( P )8 422 4:2:0 38 5( P ) 10 522 4:2:0 46 6( P ) 12 622 4:2:0 46 7( P ) 14 722 4:2:0 46 8( P ) 16 822 4:2:0 47 9( P ) 18 922 4:2:0 47 00010( P ) 201021 4:2:0 49 00011( P ) 221121 4:2:0 49 00012( P ) 241221 4:2:0 49 00013( P ) 261321 4:2:0 49 00014( P ) 281421 4:2:0 49 00015( P ) 301521 4:2:0 49 00016( P ) 321621 4:2:0 50 00017( P ) 341721 4:2:0 49 00018( P ) 361821 4:2:0 49 00019( P ) 381921 4:2:0 49 00020( P ) 402021 4:2:0 49 00021( P ) 422121 4:2:0 36 00022( P ) 442221 4:2:0 49 00023( P ) 462321 4:2:0 49 00024( P ) 482421 4:2:0 48 00025( P ) 502521 4:2:0 49 00026( P ) 522621 4:2:0 50 00027( P ) 542721 4:2:0 50 00028( P ) 562821 4:2:0 52 An unintentional loss of pictures occurs! Exit -- 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/20161122/215ace40/attachment-0001.html>
[Bug 98005] VCE dual instance encoding inconsistent since st/va: enable dual instances encode by sync surface
https://bugs.freedesktop.org/show_bug.cgi?id=98005 --- Comment #23 from Andy Furniss --- To clarify "Seems to always be the same place" I don't mean in the video - that's variable. I mean with gop = 30 after 00028 it will bail. -- 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/20161122/fe41ac0e/attachment.html>
[PATCH] drm/atomic: Unconfuse the old_state mess in commmit_tail
2016-11-21 Daniel Vetter :
> I totally butcherd the job on typing the kernel-doc for these, and no
> one realized. Noticed by Russell. Maarten has a more complete approach
> to this confusion, by making it more explicit what the new/old state
> is, instead of this magic switching behaviour.
>
> v2:
> - Liviu pointed out that wait_for_fences is even more magic. Leave
> that as @state, and document @pre_swap better.
> - While at it, patch in header for the reference section.
> - Fix spelling issues Russell noticed.
>
> v3: Fix up the @pre_swap note (Liviu): Also s/synchronous/blocking/,
> since async flip is something else than non-blocking.
>
> Cc: Liviu Dudau
> Reported-by: Russell King - ARM Linux
> Cc: Russell King - ARM Linux
> Fixes: 9f2a7950e77a ("drm/atomic-helper: nonblocking commit support")
> Cc: Gustavo Padovan
> Cc: Maarten Lankhorst
> Cc: Tomeu Vizoso
> Cc: Daniel Stone
> Signed-off-by: Daniel Vetter
> ---
> Documentation/gpu/drm-kms-helpers.rst| 3 ++
> drivers/gpu/drm/drm_atomic_helper.c | 78
> ++--
> include/drm/drm_modeset_helper_vtables.h | 12 +++--
> 3 files changed, 54 insertions(+), 39 deletions(-)
>
> diff --git a/Documentation/gpu/drm-kms-helpers.rst
> b/Documentation/gpu/drm-kms-helpers.rst
> index 4ca77f675967..03040aa14fe8 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -63,6 +63,9 @@ Atomic State Reset and Initialization
> .. kernel-doc:: drivers/gpu/drm/drm_atomic_helper.c
> :doc: atomic state reset and initialization
>
> +Helper Functions Reference
> +--
> +
> .. kernel-doc:: include/drm/drm_atomic_helper.h
> :internal:
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 0b16587cdc62..494680c9056e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1006,13 +1006,21 @@
> EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables);
> * drm_atomic_helper_wait_for_fences - wait for fences stashed in plane state
> * @dev: DRM device
> * @state: atomic state object with old state structures
> - * @pre_swap: if true, do an interruptible wait
> + * @pre_swap: If true, do an interruptible wait, and @state is the new state.
> + * Otherwise @state is the old state.
> *
> * For implicit sync, driver should fish the exclusive fence out from the
> * incoming fb's and stash it in the drm_plane_state. This is called after
> * drm_atomic_helper_swap_state() so it uses the current plane state (and
> * just uses the atomic state to find the changed planes)
> *
> + * Note that @pre_swap is needed since the point where we block for fences
> moves
> + * around depending upon whether an atomic commit is blocking or
> + * non-blocking. For async commit all waiting needs to happen after
> + * drm_atomic_helper_swap_state() is called, but for synchronous commits we
> want
> + * to wait **before** we do anything that can't be easily rolled back. That
> is
> + * before we call drm_atomic_helper_swap_state().
> + *
> * Returns zero if success or < 0 if dma_fence_wait() fails.
> */
> int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
> @@ -1147,7 +1155,7 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
>
> /**
> * drm_atomic_helper_commit_tail - commit atomic update to hardware
> - * @state: new modeset state to be committed
> + * @old_state: atomic state object with old state structures
> *
> * This is the default implemenation for the ->atomic_commit_tail() hook of
> the
> * &drm_mode_config_helper_funcs vtable.
> @@ -1158,53 +1166,53 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
> *
> * For drivers supporting runtime PM the recommended sequence is instead ::
> *
> - * drm_atomic_helper_commit_modeset_disables(dev, state);
> + * drm_atomic_helper_commit_modeset_disables(dev, old_state);
> *
> - * drm_atomic_helper_commit_modeset_enables(dev, state);
> + * drm_atomic_helper_commit_modeset_enables(dev, old_state);
> *
> - * drm_atomic_helper_commit_planes(dev, state,
> + * drm_atomic_helper_commit_planes(dev, old_state,
> * DRM_PLANE_COMMIT_ACTIVE_ONLY);
> *
> * for committing the atomic update to hardware. See the kerneldoc entries
> for
> * these three functions for more details.
> */
> -void drm_atomic_helper_commit_tail(struct drm_atomic_state *state)
> +void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state)
I thought we would commit the new state. Why the rename to old_state?
> {
> - struct drm_device *dev = state->dev;
> + struct drm_device *dev = old_state->dev;
>
> - drm_atomic_helper_commit_modeset_disables(dev, state);
> + drm_atomic_helper_commit_modeset_disables(dev, old_state);
>
> - drm_atomic_helper_commit_planes(dev, state, 0);
> + drm_atomic_helper_commit_plan
linux-next: build warning after merge of the drm-misc tree
Hi all,
After merging the drm-misc tree, today's linux-next build (arm
multi_v7_defconfig) produced this warning:
In file included from include/linux/pci.h:30:0,
from drivers/gpu/vga/vgaarb.c:40:
drivers/gpu/vga/vgaarb.c: In function 'vga_arb_device_init':
include/linux/device.h:1214:36: warning: 'dev' may be used uninitialized in
this function [-Wmaybe-uninitialized]
#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
^
drivers/gpu/vga/vgaarb.c:1410:17: note: 'dev' was declared here
struct device *dev;
^
Introduced by commit
a75d68f62106 ("vgaarb: Use dev_printk() when possible")
--
Cheers,
Stephen Rothwell
[GIT PULL] exynos-drm-fixes
Hi Dave, No critial patch but it make sure to unmap the region if HDMI probing failed, and it includes two trivial fixups. Please kindly let me know if there is any problem. Thanks, Inki Dae The following changes since commit c2ee69d83b2b14d68ad7ee1773fc1d40e97f201d: Merge tag 'drm-intel-fixes-2016-11-17' of ssh://git.freedesktop.org/git/drm-intel into drm-fixes (2016-11-18 10:33:28 +1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos exynos-drm-fixes for you to fetch changes up to 0260e20f4bf3f4f70240fed70726c7eb19a99fd5: drm/exynos: gsc: fix spelling mistakes (2016-11-21 14:03:49 +0900) Arvind Yadav (1): gpu/drm/exynos/exynos_hdmi - Unmap region obtained by of_iomap Colin Ian King (1): drm/exynos: gsc: fix spelling mistakes Shuah Khan (1): exynos-drm: Fix error messages to print flags and size drivers/gpu/drm/exynos/exynos_drm_fimc.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 4 ++-- drivers/gpu/drm/exynos/exynos_drm_gsc.c | 2 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 5 + 4 files changed, 9 insertions(+), 4 deletions(-)
[BUG] hdlcd gets confused about base address
On Mon, Nov 21, 2016 at 02:55:28PM +, Russell King - ARM Linux wrote: > On Mon, Nov 21, 2016 at 02:30:53PM +, Russell King - ARM Linux wrote: > > On Mon, Nov 21, 2016 at 01:24:19PM +, Russell King - ARM Linux wrote: > > > On Mon, Nov 21, 2016 at 12:56:53PM +, Liviu Dudau wrote: > > > > That is mostly due to the check in hdlcd_crtc_disable() which I should > > > > remove, I've added it because I was getting a ->disable() hook call > > > > before any ->enable() was called at startup time. I need to revisit > > > > this as I remember Daniel was commenting that this was not needed. > > > > > > Removing that test results in: > > > > > > [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0] > > > flip_done timed out > > > > > > and the kernel hanging, seemingly in an IRQs-off region. > > > > Annoyingly, enabling DRM debug prevents the kernel hanging... > > I've been trying to trace through what's happening with this flip_done > stuff, but I'm finding it _extremely_ difficult to follow the atomic > code. > > (Sorry, I'm going to go over my usual 72 column limit for this due to > the damn long DRM function names.) > > I can see that drm_atomic_helper_commit() calls > drm_atomic_helper_setup_commit() > which sets up commit->flip_done for each CRTC, and sets up an event for > each. > > drm_atomic_helper_commit() continues on to eventually call > drm_atomic_helper_swap_state() > which then swaps the state for the CRTCs, but then ends up dropping > the event reference: > > state->crtcs[i].commit->event = NULL; > > What I can't see is why this isn't a leaked pointer - I don't see > anything inbetween taking charge of that structure. The _commit_ > hasn't been swapped from what I can see, it's just state->crtcs[i].state > that have been swapped. The event is also stored in crtc_state->event, which after swap_states land in drm_crtc->state->event, which is the place drivers are supposed to pick it up from for delivery. > So I can't see who's responsible for generating this event, or how the > backend DRM drivers get to know about this event, and that they should > complete the flip. > > What I also don't get is why DRM is wanting to wait for a flip event > when we're disabling the CRTC. None of this makes sense to me, like > much of the atomic modeset code... The DRM event has two uses: - high-precision timestamp for when the new frame starts displaying. - confirmation that the old buffers are no longer being used by the hw. This is used on Android's drm_hwcomposer in the new hwc2 mode. Note that the crtc_state->event has 3 uses in total, all hidden behind the abstraction: - flip_done, for the atomic helpers - drm event, for current userspace (also needed to emulate legacy flips) - and out-fences, needed by android. The trouble with ->event delivery was that many drivers didn't bother to implement this at all, since driver submitters never even tested pageflippping. And for those maintainers that did test pageflipping, they only ever tested the legacy page_flip paths, which e.g. doesn't ever ask for an event when disabling the CRTC (since you can't do that). But atomic allows all this, and review wasn't enough to fight the influx of bad drivers. Hence I opted to make the nonblocking support in the atomic helpers enforce this part of the abi contract, even for blocking modesets. If you're stuck on a flip_done, then your driver doesn't send out events when disabling the CRTC. All the waits have a 10s timeout, and none of them are in atomic contexts, so no idea why this takes down your box. I suspect it's something unrelated. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v2] drm/fences: add DOC: for explicit fencing
On Tue, Nov 22, 2016 at 09:11:28AM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Document IN_FENCE_FD and OUT_FENCE_PTR properties.
>
> v2: incorporate comments from Daniel Vetter
>
> Signed-off-by: Gustavo Padovan
> ---
> Documentation/gpu/drm-kms.rst | 6 +
> drivers/gpu/drm/drm_atomic.c | 52
> +++
> 2 files changed, 58 insertions(+)
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 568f3c2..cdc9539 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -287,6 +287,12 @@ Tile Group Property
> .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> :doc: Tile group
>
> +Explicit Fencing Properties
> +---
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_atomic.c
> + :doc: explicit fencing properties
> +
> Existing KMS Properties
> ---
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index b476ec5..f548e68 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1809,6 +1809,58 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>
> +/**
> + * DOC: explicit fencing properties
> + *
> + * Explicit fencing allows userspace to control the buffer synchronization
> + * between devices. A Fence or a group of fences are transfered to/from
> + * userspace using Sync File fds and there are two DRM properties for that.
> + * IN_FENCE_FD on each DRM Plane to send fences to the kernel and
> + * OUT_FENCE_PTR on each DRM CRTC to receive fences from the kernel.
> + *
> + * As a contrast, with implicit fencing the kernel keeps track of any
> + * ongoing rendering, and automatically ensures that the atomic update waits
> + * for any pending rendering to complete. For shared buffers represented with
> + * a struct &dma_buf this is tracked in &reservation_object structures.
> + * Implicit syncing is how Linux traditionally worked (e.g. DRI2/3 on X.org),
> + * whereas explicit fencing is what Android wants.
> + *
> + * "IN_FENCE_FDâ:
> + * Use this property to pass a fence that DRM should wait on before
> + * proceeding with the Atomic Commit request and show the framebuffer for
> + * the plane on the screen. The fence can be either a normal fence or a
> + * merged one, the sync_file framework will handle both cases and use a
> + * fence_array if a merged fence is received. Passing -1 here means no
> + * fences to wait on.
> + *
> + * If the Atomic Commit request has the DRM_MODE_ATOMIC_TEST_ONLY flag
> + * it will only check if the Sync File is a valid one.
> + *
> + * On the driver side the fence is stored on the @fence parameter of
> + * struct &drm_plane_state. Drivers which also support implicit fencing
> + * should set the implicit fence using drm_atomic_set_fence_for_plane(),
> + * to make sure there's consistent behaviour between drivers in precedence
> + * of implicit vs. explicit fencing.
> + *
> + * "OUT_FENCE_PTRâ:
> + * Use this property to pass a file descriptor pointer to DRM. Once the
> + * Atomic Commit request call returns OUT_FENCE_PTR will be filled with
> + * the file descriptor number of a Sync File. This Sync File contains the
> + * CRTC fence that will be signaled when all framebuffers present on the
> + * Atomic Commit * request for that given CRTC are scanned out on the
> + * screen.
> + *
> + * The Atomic Commit request fails if a invalid pointer is passed. If the
> + * Atomic Commit request fails for any other reason the out fence fd
> + * returned will be -1. On a Atomic Commit with the
> + * DRM_MODE_ATOMIC_TEST_ONLY flag the out fence will also be set to -1.
> + *
> + * Note that out-fences don't have a special interface to drivers and are
> + * internally represented by a struct &drm_pending_vblank_event in struct
> + * &drm_crtc_state, which is also used by the async atomic commit helpers
I've done an s/async/nonblocking/ here for consistency, since async commit
== tearing commit (but it might still block), and applied the patch to
drm-misc.
Thanks a lot for typing this!
-Daniel
> + * and for the DRM event handling for existing userspace.
> + */
> +
> static struct dma_fence *get_crtc_fence(struct drm_crtc *crtc)
> {
> struct dma_fence *fence;
> --
> 2.5.5
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[RFC][PATCH 1/3] drm/bridge: adv7511: Rework adv7511_power_on/off() so they can be reused internally
Hi John,
Thank you for the patch.
On Monday 21 Nov 2016 16:37:30 John Stultz wrote:
> In chasing down issues with EDID probing, I found some
> duplicated but incomplete logic used to power the chip on and
> off.
>
> This patch refactors the adv7511_power_on/off functions, so
> they can be used for internal needs, and replaces duplicative
> logic that powers the chip on and off around the EDID probing
> with the common logic.
>
> Cc: David Airlie
> Cc: Archit Taneja
> Cc: Wolfram Sang
> Cc: Lars-Peter Clausen
> Cc: Laurent Pinchart
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: John Stultz
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 30 +++--
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8dba729..b240e05
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -325,7 +325,7 @@ static void adv7511_set_link_config(struct adv7511
> *adv7511, adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB;
> }
>
> -static void adv7511_power_on(struct adv7511 *adv7511)
> +static void __adv7511_power_on(struct adv7511 *adv7511)
> {
> adv7511->current_edid_segment = -1;
>
> @@ -343,6 +343,7 @@ static void adv7511_power_on(struct adv7511 *adv7511)
>ADV7511_INT1_DDC_ERROR);
> }
>
> +
This isn't needed.
> /*
>* Per spec it is allowed to pulse the HPD signal to indicate that the
>* EDID information has changed. Some monitors do this when they
wakeup
> @@ -362,11 +363,15 @@ static void adv7511_power_on(struct adv7511 *adv7511)
>
> if (adv7511->type == ADV7533)
> adv7533_dsi_power_on(adv7511);
> +}
>
> +static void adv7511_power_on(struct adv7511 *adv7511)
> +{
> + __adv7511_power_on(adv7511);
> adv7511->powered = true;
> }
>
> -static void adv7511_power_off(struct adv7511 *adv7511)
> +static void __adv7511_power_off(struct adv7511 *adv7511)
> {
> /* TODO: setup additional power down modes */
> regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> @@ -376,7 +381,11 @@ static void adv7511_power_off(struct adv7511 *adv7511)
>
> if (adv7511->type == ADV7533)
> adv7533_dsi_power_off(adv7511);
> +}
>
> +static void adv7511_power_off(struct adv7511 *adv7511)
> +{
> + __adv7511_power_off(adv7511);
> adv7511->powered = false;
> }
>
> @@ -545,24 +554,13 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
> unsigned int count;
>
> /* Reading the EDID only works if the device is powered */
> - if (!adv7511->powered) {
> - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> -ADV7511_POWER_POWER_DOWN, 0);
> - if (adv7511->i2c_main->irq) {
> - regmap_write(adv7511->regmap,
ADV7511_REG_INT_ENABLE(0),
> - ADV7511_INT0_EDID_READY);
> - regmap_write(adv7511->regmap,
ADV7511_REG_INT_ENABLE(1),
> - ADV7511_INT1_DDC_ERROR);
> - }
> - adv7511->current_edid_segment = -1;
> - }
> + if (!adv7511->powered)
> + __adv7511_power_on(adv7511);
The __adv7511_power_on() function does more than the above, in particular it
performs an expensive regcache_sync() and calls adv7533_dsi_power_on() for the
ADV7533. Don't those operations have side effects that are either not wanted
or not needed here ? In any case this patch modifies the behaviour of the
driver, which needs to be documented in the kernel message.
> edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511);
>
> if (!adv7511->powered)
> - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> -ADV7511_POWER_POWER_DOWN,
> -ADV7511_POWER_POWER_DOWN);
> + __adv7511_power_off(adv7511);
>
> kfree(adv7511->edid);
> adv7511->edid = edid;
--
Regards,
Laurent Pinchart
[RFC][PATCH 1/3] drm/bridge: adv7511: Rework adv7511_power_on/off() so they can be reused internally
On Tue, Nov 22, 2016 at 12:14 AM, Laurent Pinchart
wrote:
> Hi John,
>
> Thank you for the patch.
>
> On Monday 21 Nov 2016 16:37:30 John Stultz wrote:
>> In chasing down issues with EDID probing, I found some
>> duplicated but incomplete logic used to power the chip on and
>> off.
>>
>> This patch refactors the adv7511_power_on/off functions, so
>> they can be used for internal needs, and replaces duplicative
>> logic that powers the chip on and off around the EDID probing
>> with the common logic.
>>
>> Cc: David Airlie
>> Cc: Archit Taneja
>> Cc: Wolfram Sang
>> Cc: Lars-Peter Clausen
>> Cc: Laurent Pinchart
>> Cc: dri-devel at lists.freedesktop.org
>> Signed-off-by: John Stultz
>> ---
>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 30 +++--
>> 1 file changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8dba729..b240e05
>> 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -325,7 +325,7 @@ static void adv7511_set_link_config(struct adv7511
>> *adv7511, adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB;
>> }
>>
>> -static void adv7511_power_on(struct adv7511 *adv7511)
>> +static void __adv7511_power_on(struct adv7511 *adv7511)
>> {
>> adv7511->current_edid_segment = -1;
>>
>> @@ -343,6 +343,7 @@ static void adv7511_power_on(struct adv7511 *adv7511)
>>ADV7511_INT1_DDC_ERROR);
>> }
>>
>> +
>
> This isn't needed.
Apologies. I saw this right after I sent it!
>> /*
>>* Per spec it is allowed to pulse the HPD signal to indicate that the
>>* EDID information has changed. Some monitors do this when they
> wakeup
>> @@ -362,11 +363,15 @@ static void adv7511_power_on(struct adv7511 *adv7511)
>>
>> if (adv7511->type == ADV7533)
>> adv7533_dsi_power_on(adv7511);
>> +}
>>
>> +static void adv7511_power_on(struct adv7511 *adv7511)
>> +{
>> + __adv7511_power_on(adv7511);
>> adv7511->powered = true;
>> }
>>
>> -static void adv7511_power_off(struct adv7511 *adv7511)
>> +static void __adv7511_power_off(struct adv7511 *adv7511)
>> {
>> /* TODO: setup additional power down modes */
>> regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
>> @@ -376,7 +381,11 @@ static void adv7511_power_off(struct adv7511 *adv7511)
>>
>> if (adv7511->type == ADV7533)
>> adv7533_dsi_power_off(adv7511);
>> +}
>>
>> +static void adv7511_power_off(struct adv7511 *adv7511)
>> +{
>> + __adv7511_power_off(adv7511);
>> adv7511->powered = false;
>> }
>>
>> @@ -545,24 +554,13 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>> unsigned int count;
>>
>> /* Reading the EDID only works if the device is powered */
>> - if (!adv7511->powered) {
>> - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
>> -ADV7511_POWER_POWER_DOWN, 0);
>> - if (adv7511->i2c_main->irq) {
>> - regmap_write(adv7511->regmap,
> ADV7511_REG_INT_ENABLE(0),
>> - ADV7511_INT0_EDID_READY);
>> - regmap_write(adv7511->regmap,
> ADV7511_REG_INT_ENABLE(1),
>> - ADV7511_INT1_DDC_ERROR);
>> - }
>> - adv7511->current_edid_segment = -1;
>> - }
>> + if (!adv7511->powered)
>> + __adv7511_power_on(adv7511);
>
> The __adv7511_power_on() function does more than the above, in particular it
> performs an expensive regcache_sync() and calls adv7533_dsi_power_on() for the
> ADV7533. Don't those operations have side effects that are either not wanted
> or not needed here ? In any case this patch modifies the behaviour of the
> driver, which needs to be documented in the kernel message.
Sorry, what do you mean by kernel message? Commit message, maybe?
Fair point, I'll review the adv7533_dsi_power_on bits and see if they
should move out to the external function rather then the internal one.
Similarly for the regcache_sync.
Thanks so much for the review!
thanks
-john
[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
Hi John, Thank you for the patch. On Monday 21 Nov 2016 16:37:31 John Stultz wrote: > Secton 4.1 of the adv7511 programming guide advises one waits > 200ms after powering on the chip before trying to communicate > with it via i2c. Not doing so can cause reliability issues when > probing the EDID. > > See: > http://www.analog.com/media/en/technical-documentation/user-guides/ADV7511_P > rogramming_Guide.pdf > > So this patch simply adds a 200ms sleep at the end of the > power_on path. This greatly improves EDID probing reliabilty > on hotplug with the HiKey device. > > Cc: David Airlie > Cc: Archit Taneja > Cc: Wolfram Sang > Cc: Lars-Peter Clausen > Cc: Laurent Pinchart > Cc: dri-devel at lists.freedesktop.org > Signed-off-by: John Stultz > --- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index b240e05..2114a4c > 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -361,6 +361,8 @@ static void __adv7511_power_on(struct adv7511 *adv7511) >*/ > regcache_sync(adv7511->regmap); > > + msleep(200); > + The documentation states that "The user should wait 200ms for the address to be decided, after the power supplies are high, before attempting to communicate with the ADV7511W using I2C." The hardware user's guide further states that "When initially powered up, there is a 200ms period before the device is ready to be addressed." Not only the delay you add comes after lots of I2C communication, but the driver doesn't handle regulators, and thus doesn't power down the device at the hardware level. The initial power up should thus be long gone when this code is reached. Could it be that, on the HiKey board, the power supply is controlled through another mean that doesn't comply with the 200ms rule ? > if (adv7511->type == ADV7533) > adv7533_dsi_power_on(adv7511); > } -- Regards, Laurent Pinchart
[RFC][PATCH 1/3] drm/bridge: adv7511: Rework adv7511_power_on/off() so they can be reused internally
Hi John,
On Tuesday 22 Nov 2016 00:16:55 John Stultz wrote:
> On Tue, Nov 22, 2016 at 12:14 AM, Laurent Pinchart wrote:
> > On Monday 21 Nov 2016 16:37:30 John Stultz wrote:
> >> In chasing down issues with EDID probing, I found some
> >> duplicated but incomplete logic used to power the chip on and
> >> off.
> >>
> >> This patch refactors the adv7511_power_on/off functions, so
> >> they can be used for internal needs, and replaces duplicative
> >> logic that powers the chip on and off around the EDID probing
> >> with the common logic.
> >>
> >> Cc: David Airlie
> >> Cc: Archit Taneja
> >> Cc: Wolfram Sang
> >> Cc: Lars-Peter Clausen
> >> Cc: Laurent Pinchart
> >> Cc: dri-devel at lists.freedesktop.org
> >> Signed-off-by: John Stultz
> >> ---
> >>
> >> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 30 +-
> >> 1 file changed, 14 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> >> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8dba729..b240e05
> >> 100644
> >> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> >> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> >> @@ -325,7 +325,7 @@ static void adv7511_set_link_config(struct adv7511
> >> *adv7511, adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB;
> >>
> >> }
> >>
> >> -static void adv7511_power_on(struct adv7511 *adv7511)
> >> +static void __adv7511_power_on(struct adv7511 *adv7511)
> >> {
> >> adv7511->current_edid_segment = -1;
> >>
> >> @@ -343,6 +343,7 @@ static void adv7511_power_on(struct adv7511 *adv7511)
> >>ADV7511_INT1_DDC_ERROR);
> >> }
> >>
> >> +
> >
> > This isn't needed.
>
> Apologies. I saw this right after I sent it!
>
> >> /*
> >>* Per spec it is allowed to pulse the HPD signal to indicate that
> >>the
> >>* EDID information has changed. Some monitors do this when they
> >> wakeup
> >> @@ -362,11 +363,15 @@ static void adv7511_power_on(struct adv7511
> >> *adv7511)
> >>
> >> if (adv7511->type == ADV7533)
> >> adv7533_dsi_power_on(adv7511);
> >> +}
> >>
> >> +static void adv7511_power_on(struct adv7511 *adv7511)
> >> +{
> >> + __adv7511_power_on(adv7511);
> >> adv7511->powered = true;
> >> }
> >>
> >> -static void adv7511_power_off(struct adv7511 *adv7511)
> >> +static void __adv7511_power_off(struct adv7511 *adv7511)
> >> {
> >> /* TODO: setup additional power down modes */
> >> regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> >> @@ -376,7 +381,11 @@ static void adv7511_power_off(struct adv7511
> >> *adv7511)
> >>
> >> if (adv7511->type == ADV7533)
> >> adv7533_dsi_power_off(adv7511);
> >> +}
> >>
> >> +static void adv7511_power_off(struct adv7511 *adv7511)
> >> +{
> >> + __adv7511_power_off(adv7511);
> >> adv7511->powered = false;
> >> }
> >>
> >> @@ -545,24 +554,13 @@ static int adv7511_get_modes(struct adv7511
> >> *adv7511,
> >> unsigned int count;
> >>
> >> /* Reading the EDID only works if the device is powered */
> >> - if (!adv7511->powered) {
> >> - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> >> -ADV7511_POWER_POWER_DOWN, 0);
> >> - if (adv7511->i2c_main->irq) {
> >> - regmap_write(adv7511->regmap,
> >
> > ADV7511_REG_INT_ENABLE(0),
> >
> >> - ADV7511_INT0_EDID_READY);
> >> - regmap_write(adv7511->regmap,
> >> ADV7511_REG_INT_ENABLE(1),
> >> - ADV7511_INT1_DDC_ERROR);
> >> - }
> >> - adv7511->current_edid_segment = -1;
> >> - }
> >> + if (!adv7511->powered)
> >> + __adv7511_power_on(adv7511);
> >
> > The __adv7511_power_on() function does more than the above, in particular
> > it performs an expensive regcache_sync() and calls adv7533_dsi_power_on()
> > for the ADV7533. Don't those operations have side effects that are either
> > not wanted or not needed here ? In any case this patch modifies the
> > behaviour of the driver, which needs to be documented in the kernel
> > message.
>
> Sorry, what do you mean by kernel message? Commit message, maybe?
Sorry, yes, I meant commit message.
> Fair point, I'll review the adv7533_dsi_power_on bits and see if they
> should move out to the external function rather then the internal one.
> Similarly for the regcache_sync.
>
> Thanks so much for the review!
--
Regards,
Laurent Pinchart
[RFC][PATCH 3/3] drm/bridge: adv7511: Enable HPD interrupts to support hotplug and improve monitor detection
Hi John, Thank you for the patch. On Monday 21 Nov 2016 16:37:32 John Stultz wrote: > From: Archit Taneja > > On some adv7511 implementations, we can get some spurious > disconnect signals which can cause monitor probing to fail. > > This patch enables HPD (hot plug detect) interrupt support > which allows the monitor to be properly re-initialized when > the spurious disconnect signal goes away. > > This also enables proper hotplug support. > > Cc: David Airlie > Cc: Archit Taneja > Cc: Wolfram Sang > Cc: Lars-Peter Clausen > Cc: Laurent Pinchart > Cc: dri-devel at lists.freedesktop.org > Originally-by: Archit Taneja > [jstultz: Added proper commit message] > Signed-off-by: John Stultz This looks good to me. Acked-by: Laurent Pinchart > --- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 2114a4c..889cf36 > 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -338,7 +338,7 @@ static void __adv7511_power_on(struct adv7511 *adv7511) >* Still, let's be safe and stick to the documentation. >*/ > regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), > - ADV7511_INT0_EDID_READY); > + ADV7511_INT0_EDID_READY | ADV7511_INT0_HPD); > regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), >ADV7511_INT1_DDC_ERROR); > } > @@ -825,6 +825,10 @@ static int adv7511_bridge_attach(struct drm_bridge > *bridge) if (adv->type == ADV7533) > ret = adv7533_attach_dsi(adv); > > + if (adv->i2c_main->irq) > + regmap_write(adv->regmap, ADV7511_REG_INT_ENABLE(0), > + ADV7511_INT0_HPD); > + > return ret; > } -- Regards, Laurent Pinchart
[PATCH] drm/radeon: Ensure vblank interrupt is enabled on DPMS transition to on
From: Michel Dänzer Fixes the vblank interrupt being disabled when it should be on, which can cause at least the following symptoms: * Hangs when running 'xset dpms force off' in a GNOME session with gnome-shell using DRI2. * RandR 1.4 slave outputs freezing with garbage displayed using xf86-video-ati 7.8.0 or newer. NOTE: This patch only applies to 4.5.y or older kernels. With newer kernels, this problem cannot happen because the driver now uses drm_crtc_vblank_on/off instead of drm_vblank_pre/post_modeset. I consider this patch safer for older kernels than backporting the API change, because drm_crtc_vblank_on/off had various issues in older kernels, and I'm not sure all fixes for those have been backported to all stable branches where this patch could be applied. Reported-and-Tested-by: Max Staudt Signed-off-by: Michel Dänzer --- drivers/gpu/drm/radeon/atombios_crtc.c | 2 ++ drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c index dac78ad..115b4a4 100644 --- a/drivers/gpu/drm/radeon/atombios_crtc.c +++ b/drivers/gpu/drm/radeon/atombios_crtc.c @@ -275,6 +275,8 @@ void atombios_crtc_dpms(struct drm_crtc *crtc, int mode) atombios_enable_crtc_memreq(crtc, ATOM_ENABLE); atombios_blank_crtc(crtc, ATOM_DISABLE); drm_vblank_post_modeset(dev, radeon_crtc->crtc_id); + /* Make sure vblank interrupt is still enabled if needed */ + radeon_irq_set(rdev); radeon_crtc_load_lut(crtc); break; case DRM_MODE_DPMS_STANDBY: diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c index 678b438..89f22bd 100644 --- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c +++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c @@ -331,6 +331,8 @@ static void radeon_crtc_dpms(struct drm_crtc *crtc, int mode) WREG32_P(RADEON_CRTC_EXT_CNTL, crtc_ext_cntl, ~(mask | crtc_ext_cntl)); } drm_vblank_post_modeset(dev, radeon_crtc->crtc_id); + /* Make sure vblank interrupt is still enabled if needed */ + radeon_irq_set(rdev); radeon_crtc_load_lut(crtc); break; case DRM_MODE_DPMS_STANDBY: -- 2.10.2
[PATCH v2 01/13] devicetree/bindings: display: Document common panel properties
Hi Rob, On Monday 21 Nov 2016 10:48:15 Rob Herring wrote: > On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote: > > Document properties common to several display panels in a central > > location that can be referenced by the panel device tree bindings. > > Looks good. Just one comment... > > [...] > > > +Connectivity > > + > > + > > +- ports: Panels receive video data through one or multiple connections. > > While > > + the nature of those connections is specific to the panel type, the > > + connectivity is expressed in a standard fashion using ports as > > specified in > > + the device graph bindings defined in > > + Documentation/devicetree/bindings/graph.txt. > > We allow panels to either use graph binding or be a child of the display > controller. I knew that some display controllers use a phandle to the panel (see the fsl,panel and nvidia,panel properties), but I didn't know we had panels as children of display controller nodes. I don't think we should allow that for anything but DSI panels, as the DT hierarchy is based on control buses. Are you sure we have other panels instantiated through that mechanism ? > Using the graph is preferred, but in the simple cases just a child node is > sufficient. This should be described here or somewhere in this doc. -- Regards, Laurent Pinchart
[PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
Hi Frank,
On Tuesday 22 November 2016 07:13 AM, Frank Rowand wrote:
> On 11/21/16 08:33, Sekhar Nori wrote:
>> On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
>>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>> +{
>>> + const struct da8xx_ddrctl_config_knob *knob;
>>> + const struct da8xx_ddrctl_setting *setting;
>>> + struct device_node *node;
>>> + struct resource *res;
>>> + void __iomem *ddrctl;
>>> + struct device *dev;
>>> + u32 reg;
>>> +
>>> + dev = &pdev->dev;
>>> + node = dev->of_node;
>>> +
>>> + setting = da8xx_ddrctl_get_board_settings();
>>> + if (!setting) {
>>> + dev_err(dev, "no settings for board '%s'\n",
>>> + of_flat_dt_get_machine_name());
>>> + return -EINVAL;
>>> + }
>>
>> This causes a section mismatch because of_flat_dt_get_machine_name()
>> has an __init annotation. I did not notice that before, sorry.
>>
>> It can be fixed with a patch like below:
>>
>> ---8<---
>> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
>> index a20e7bbbcbe0..9ca5aab3ac54 100644
>> --- a/drivers/memory/da8xx-ddrctl.c
>> +++ b/drivers/memory/da8xx-ddrctl.c
>> @@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting
>> *da8xx_ddrctl_get_board_settings(void)
>> return NULL;
>> }
>>
>> +static const char* da8xx_ddrctl_get_machine_name(void)
>> +{
>> +const char *str;
>> +int ret;
>> +
>> +ret = of_property_read_string(of_root, "model", &str);
>> +if (ret)
>> +ret = of_property_read_string(of_root, "compatible", &str);
>> +
>> +return str;
>> +}
>> +
>> static int da8xx_ddrctl_probe(struct platform_device *pdev)
>> {
>> const struct da8xx_ddrctl_config_knob *knob;
>> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device
>> *pdev)
>> setting = da8xx_ddrctl_get_board_settings();
>> if (!setting) {
>> dev_err(dev, "no settings for board '%s'\n",
>> -of_flat_dt_get_machine_name());
>
> da8xx_ddrctl_get_board_settings() tries to match based on the "compatible"
> property in the root node. The "model" property in the root node has
> nothing to do with the failure to match. So creating and then using
> da8xx_ddrctl_get_machine_name() to potentially report model is not useful.
>
> It should be sufficient to simply report that no compatible matched.
I agree with you on this. Even if model name is printed, you will have
to go back and check the compatible anyway. But I think it will be
useful to print the compatible instead of just reporting that nothing
matched.
Bartosz, if you agree too, could you send a fix patch just printing the
compatible?
Thanks,
Sekhar
[PATCH] drm: tilcdc: fix a DT property parsing
The DT binding for tildc is not consistent with the driver code - the
option in the binding is called 'max-width' while the code expects
'ti,max-width'.
Make the driver code consistent with the binding.
Signed-off-by: Bartosz Golaszewski
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index a7c91f7..4d3adf8e 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -302,7 +302,7 @@ static int tilcdc_init(struct drm_driver *ddrv, struct
device *dev)
DBG("Maximum Bandwidth Value %d", priv->max_bandwidth);
- if (of_property_read_u32(node, "ti,max-width", &priv->max_width))
+ if (of_property_read_u32(node, "max-width", &priv->max_width))
priv->max_width = TILCDC_DEFAULT_MAX_WIDTH;
DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width);
--
2.9.3
[PATCH] ARM: dts: da850: specify max width for display node
It has been determined that the highest resolution supported correctly by LCDC rev1 is 800x600 on da850 due to memory bandwidth constraints. Set the max_width property in da850.dtsi to 800. Signed-off-by: Bartosz Golaszewski --- arch/arm/boot/dts/da850.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi index 36066fa..0876238 100644 --- a/arch/arm/boot/dts/da850.dtsi +++ b/arch/arm/boot/dts/da850.dtsi @@ -441,6 +441,7 @@ compatible = "ti,da850-tilcdc"; reg = <0x213000 0x1000>; interrupts = <52>; + max-width = <800>; status = "disabled"; }; }; -- 2.9.3
[PATCH] drm: update MAINTAINERS for qemu drivers (bochs, cirrus, qxl, virtio-gpu)
Changes: * add myself as maintainer, so patches land in my inbox. * add qemu-devel mailing list. * add drm-qemu git repo. * flip bochs and qxl status to "Maintained". Signed-off-by: Gerd Hoffmann --- MAINTAINERS | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index ad9b965..84dc747 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4027,11 +4027,16 @@ F: drivers/gpu/drm/ast/ DRM DRIVER FOR BOCHS VIRTUAL GPU M: Gerd Hoffmann -S: Odd Fixes +L: qemu-devel at nongnu.org +T: git git://git.kraxel.org/linux drm-qemu +S: Maintained F: drivers/gpu/drm/bochs/ DRM DRIVER FOR QEMU'S CIRRUS DEVICE M: Dave Airlie +M: Gerd Hoffmann +L: qemu-devel at nongnu.org +T: git git://git.kraxel.org/linux drm-qemu S: Odd Fixes F: drivers/gpu/drm/cirrus/ @@ -4204,7 +4209,10 @@ F: Documentation/devicetree/bindings/display/renesas,du.txt DRM DRIVER FOR QXL VIRTUAL GPU M: Dave Airlie -S: Odd Fixes +M: Gerd Hoffmann +L: qemu-devel at nongnu.org +T: git git://git.kraxel.org/linux drm-qemu +S: Maintained F: drivers/gpu/drm/qxl/ F: include/uapi/drm/qxl_drm.h @@ -12825,6 +12833,8 @@ M: David Airlie M: Gerd Hoffmann L: dri-devel at lists.freedesktop.org L: virtualization at lists.linux-foundation.org +L: qemu-devel at nongnu.org +T: git git://git.kraxel.org/linux drm-qemu S: Maintained F: drivers/gpu/drm/virtio/ F: include/uapi/linux/virtio_gpu.h -- 1.8.3.1
[PATCH] drm: flip cirrus driver status to "obsolete".
Also update Kconfig help text, explaining things: Cirrus is obsolete, the hardware was designed in the 90ies and can't keep up with todays needs. More background: https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/ Better alternatives are: - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+) - qxl (DRM_QXL, qemu -vga qxl, works best with spice) - virtio (VIRTIO_GPU), qemu -vga virtio) Signed-off-by: Gerd Hoffmann --- MAINTAINERS| 3 ++- drivers/gpu/drm/cirrus/Kconfig | 9 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 84dc747..feb34a3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4037,7 +4037,8 @@ M:Dave Airlie M: Gerd Hoffmann L: qemu-devel at nongnu.org T: git git://git.kraxel.org/linux drm-qemu -S: Odd Fixes +S: Obsolete +W: https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/ F: drivers/gpu/drm/cirrus/ RADEON and AMDGPU DRM DRIVERS diff --git a/drivers/gpu/drm/cirrus/Kconfig b/drivers/gpu/drm/cirrus/Kconfig index 04b3c16..ca5c0f2 100644 --- a/drivers/gpu/drm/cirrus/Kconfig +++ b/drivers/gpu/drm/cirrus/Kconfig @@ -7,3 +7,12 @@ config DRM_CIRRUS_QEMU This is a KMS driver for emulated cirrus device in qemu. It is *NOT* intended for real cirrus devices. This requires the modesetting userspace X.org driver. + +Cirrus is obsolete, the hardware was designed in the 90ies +and can't keep up with todays needs. More background: + https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/ + +Better alternatives are: + - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+) + - qxl (DRM_QXL, qemu -vga qxl, works best with spice) + - virtio (VIRTIO_GPU), qemu -vga virtio) -- 1.8.3.1
[PATCH] drm/radeon: Ensure vblank interrupt is enabled on DPMS transition to on
On Tue, Nov 22, 2016 at 05:35:12PM +0900, Michel Dänzer wrote: > From: Michel Dänzer > > Fixes the vblank interrupt being disabled when it should be on, which > can cause at least the following symptoms: > > * Hangs when running 'xset dpms force off' in a GNOME session with > gnome-shell using DRI2. > * RandR 1.4 slave outputs freezing with garbage displayed using > xf86-video-ati 7.8.0 or newer. > > NOTE: This patch only applies to 4.5.y or older kernels. With newer > kernels, this problem cannot happen because the driver now uses > drm_crtc_vblank_on/off instead of drm_vblank_pre/post_modeset. I > consider this patch safer for older kernels than backporting the API > change, because drm_crtc_vblank_on/off had various issues in older > kernels, and I'm not sure all fixes for those have been backported to > all stable branches where this patch could be applied. Yeah, makes sense to opt for the conservative option for backporting. I guess for stable this needs some reference to the corresponding upstream fix, which is: commit 777e3cbc791f131806d9bf24b3325637c7fc228d Author: Daniel Vetter Date: Thu Jan 21 11:08:57 2016 +0100 drm/radeon: Switch to drm_vblank_on/off Reviewed-by: Daniel Vetter > > Reported-and-Tested-by: Max Staudt > Signed-off-by: Michel Dänzer > --- > drivers/gpu/drm/radeon/atombios_crtc.c | 2 ++ > drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c > b/drivers/gpu/drm/radeon/atombios_crtc.c > index dac78ad..115b4a4 100644 > --- a/drivers/gpu/drm/radeon/atombios_crtc.c > +++ b/drivers/gpu/drm/radeon/atombios_crtc.c > @@ -275,6 +275,8 @@ void atombios_crtc_dpms(struct drm_crtc *crtc, int mode) > atombios_enable_crtc_memreq(crtc, ATOM_ENABLE); > atombios_blank_crtc(crtc, ATOM_DISABLE); > drm_vblank_post_modeset(dev, radeon_crtc->crtc_id); > + /* Make sure vblank interrupt is still enabled if needed */ > + radeon_irq_set(rdev); > radeon_crtc_load_lut(crtc); > break; > case DRM_MODE_DPMS_STANDBY: > diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c > b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c > index 678b438..89f22bd 100644 > --- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c > +++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c > @@ -331,6 +331,8 @@ static void radeon_crtc_dpms(struct drm_crtc *crtc, int > mode) > WREG32_P(RADEON_CRTC_EXT_CNTL, crtc_ext_cntl, ~(mask | > crtc_ext_cntl)); > } > drm_vblank_post_modeset(dev, radeon_crtc->crtc_id); > + /* Make sure vblank interrupt is still enabled if needed */ > + radeon_irq_set(rdev); > radeon_crtc_load_lut(crtc); > break; > case DRM_MODE_DPMS_STANDBY: > -- > 2.10.2 > > ___ > 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/atomic: Unconfuse the old_state mess in commmit_tail
On Mon, Nov 21, 2016 at 06:18:02PM +0100, Daniel Vetter wrote:
> I totally butcherd the job on typing the kernel-doc for these, and no
> one realized. Noticed by Russell. Maarten has a more complete approach
> to this confusion, by making it more explicit what the new/old state
> is, instead of this magic switching behaviour.
>
> v2:
> - Liviu pointed out that wait_for_fences is even more magic. Leave
> that as @state, and document @pre_swap better.
> - While at it, patch in header for the reference section.
> - Fix spelling issues Russell noticed.
>
> v3: Fix up the @pre_swap note (Liviu): Also s/synchronous/blocking/,
> since async flip is something else than non-blocking.
>
> Cc: Liviu Dudau
> Reported-by: Russell King - ARM Linux
> Cc: Russell King - ARM Linux
> Fixes: 9f2a7950e77a ("drm/atomic-helper: nonblocking commit support")
> Cc: Gustavo Padovan
> Cc: Maarten Lankhorst
> Cc: Tomeu Vizoso
> Cc: Daniel Stone
> Signed-off-by: Daniel Vetter
Applied with Liviu's irc-r-b.
-Daniel
> ---
> Documentation/gpu/drm-kms-helpers.rst| 3 ++
> drivers/gpu/drm/drm_atomic_helper.c | 78
> ++--
> include/drm/drm_modeset_helper_vtables.h | 12 +++--
> 3 files changed, 54 insertions(+), 39 deletions(-)
>
> diff --git a/Documentation/gpu/drm-kms-helpers.rst
> b/Documentation/gpu/drm-kms-helpers.rst
> index 4ca77f675967..03040aa14fe8 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -63,6 +63,9 @@ Atomic State Reset and Initialization
> .. kernel-doc:: drivers/gpu/drm/drm_atomic_helper.c
> :doc: atomic state reset and initialization
>
> +Helper Functions Reference
> +--
> +
> .. kernel-doc:: include/drm/drm_atomic_helper.h
> :internal:
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 0b16587cdc62..494680c9056e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1006,13 +1006,21 @@
> EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables);
> * drm_atomic_helper_wait_for_fences - wait for fences stashed in plane state
> * @dev: DRM device
> * @state: atomic state object with old state structures
> - * @pre_swap: if true, do an interruptible wait
> + * @pre_swap: If true, do an interruptible wait, and @state is the new state.
> + * Otherwise @state is the old state.
> *
> * For implicit sync, driver should fish the exclusive fence out from the
> * incoming fb's and stash it in the drm_plane_state. This is called after
> * drm_atomic_helper_swap_state() so it uses the current plane state (and
> * just uses the atomic state to find the changed planes)
> *
> + * Note that @pre_swap is needed since the point where we block for fences
> moves
> + * around depending upon whether an atomic commit is blocking or
> + * non-blocking. For async commit all waiting needs to happen after
> + * drm_atomic_helper_swap_state() is called, but for synchronous commits we
> want
> + * to wait **before** we do anything that can't be easily rolled back. That
> is
> + * before we call drm_atomic_helper_swap_state().
> + *
> * Returns zero if success or < 0 if dma_fence_wait() fails.
> */
> int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
> @@ -1147,7 +1155,7 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
>
> /**
> * drm_atomic_helper_commit_tail - commit atomic update to hardware
> - * @state: new modeset state to be committed
> + * @old_state: atomic state object with old state structures
> *
> * This is the default implemenation for the ->atomic_commit_tail() hook of
> the
> * &drm_mode_config_helper_funcs vtable.
> @@ -1158,53 +1166,53 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
> *
> * For drivers supporting runtime PM the recommended sequence is instead ::
> *
> - * drm_atomic_helper_commit_modeset_disables(dev, state);
> + * drm_atomic_helper_commit_modeset_disables(dev, old_state);
> *
> - * drm_atomic_helper_commit_modeset_enables(dev, state);
> + * drm_atomic_helper_commit_modeset_enables(dev, old_state);
> *
> - * drm_atomic_helper_commit_planes(dev, state,
> + * drm_atomic_helper_commit_planes(dev, old_state,
> * DRM_PLANE_COMMIT_ACTIVE_ONLY);
> *
> * for committing the atomic update to hardware. See the kerneldoc entries
> for
> * these three functions for more details.
> */
> -void drm_atomic_helper_commit_tail(struct drm_atomic_state *state)
> +void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state)
> {
> - struct drm_device *dev = state->dev;
> + struct drm_device *dev = old_state->dev;
>
> - drm_atomic_helper_commit_modeset_disables(dev, state);
> + drm_atomic_helper_commit_modeset_disables(dev, old_state);
>
> - drm_atomic_helper_commit_planes(dev, state, 0);
> + drm_atomic_helper_commit_p
[PATCH] ARM: dts: da850: specify max width for display node
On 22/11/16 11:42, Bartosz Golaszewski wrote: > It has been determined that the highest resolution supported correctly > by LCDC rev1 is 800x600 on da850 due to memory bandwidth constraints. > > Set the max_width property in da850.dtsi to 800. > > Signed-off-by: Bartosz Golaszewski > --- > arch/arm/boot/dts/da850.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi > index 36066fa..0876238 100644 > --- a/arch/arm/boot/dts/da850.dtsi > +++ b/arch/arm/boot/dts/da850.dtsi > @@ -441,6 +441,7 @@ > compatible = "ti,da850-tilcdc"; > reg = <0x213000 0x1000>; > interrupts = <52>; > + max-width = <800>; > status = "disabled"; > }; > }; > Does 1024x768 at 10 work? The max-width should be the hardware's maximum supported width, not used for bandwidth. tilcdc has max-bandwidth property for that. 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/20161122/6e711402/attachment.sig>
[PATCH] ARM: dts: da850: specify max width for display node
2016-11-22 11:27 GMT+01:00 Tomi Valkeinen : > On 22/11/16 11:42, Bartosz Golaszewski wrote: >> It has been determined that the highest resolution supported correctly >> by LCDC rev1 is 800x600 on da850 due to memory bandwidth constraints. >> >> Set the max_width property in da850.dtsi to 800. >> >> Signed-off-by: Bartosz Golaszewski >> --- >> arch/arm/boot/dts/da850.dtsi | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi >> index 36066fa..0876238 100644 >> --- a/arch/arm/boot/dts/da850.dtsi >> +++ b/arch/arm/boot/dts/da850.dtsi >> @@ -441,6 +441,7 @@ >> compatible = "ti,da850-tilcdc"; >> reg = <0x213000 0x1000>; >> interrupts = <52>; >> + max-width = <800>; >> status = "disabled"; >> }; >> }; >> > > Does 1024x768 at 10 work? > > The max-width should be the hardware's maximum supported width, not used > for bandwidth. tilcdc has max-bandwidth property for that. > > Tomi > Eeek I misread Jyri's answer. Will fix that in v2. Thanks, Bartosz
[PATCH 0/3] ARM: da8xx: fix section mismatch in new drivers
Sekhar noticed there's a section mismatch in the da8xx-mstpri and da8xx-ddrctl drivers. This is caused by calling of_flat_dt_get_machine_name() which has an __init annotation. This series addresses this issue by introducing a new function that allows to retrieve the compatible property of the root node and using it instead of of_flat_dt_get_machine_name() in the new drivers. Bartosz Golaszewski (3): of: base: add support to get machine compatible string bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name() memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name() drivers/bus/da8xx-mstpri.c| 2 +- drivers/memory/da8xx-ddrctl.c | 2 +- drivers/of/base.c | 22 ++ include/linux/of.h| 6 ++ 4 files changed, 30 insertions(+), 2 deletions(-) -- 2.9.3
[PATCH 1/3] of: base: add support to get machine compatible string
Add a function allowing to retrieve the compatible string of the root
node of the device tree.
Signed-off-by: Bartosz Golaszewski
---
drivers/of/base.c | 22 ++
include/linux/of.h | 6 ++
2 files changed, 28 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index a0bccb5..bbfe5e9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -546,6 +546,28 @@ int of_machine_is_compatible(const char *compat)
EXPORT_SYMBOL(of_machine_is_compatible);
/**
+ * of_machine_get_compatible - Get the compatible property of the root node
+ *
+ * Returns a NULL-terminated string containing the compatible if it could
+ * be found, NULL otherwise.
+ */
+const char *of_machine_get_compatible(void)
+{
+ struct device_node *root;
+ const char *compatible;
+ int ret = -1;
+
+ root = of_find_node_by_path("/");
+ if (root) {
+ ret = of_property_read_string(root, "compatible", &compatible);
+ of_node_put(root);
+ }
+
+ return ret ? NULL : compatible;
+}
+EXPORT_SYMBOL(of_machine_get_compatible);
+
+/**
* __of_device_is_available - check if a device is available for use
*
* @device: Node to check for availability, with locks already held
diff --git a/include/linux/of.h b/include/linux/of.h
index 299aeb1..664b734 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -367,6 +367,7 @@ extern int of_alias_get_id(struct device_node *np, const
char *stem);
extern int of_alias_get_highest_id(const char *stem);
extern int of_machine_is_compatible(const char *compat);
+extern const char *of_machine_get_compatible(void);
extern int of_add_property(struct device_node *np, struct property *prop);
extern int of_remove_property(struct device_node *np, struct property *prop);
@@ -788,6 +789,11 @@ static inline int of_machine_is_compatible(const char
*compat)
return 0;
}
+static inline const char *of_machine_get_compatible(void)
+{
+ return NULL;
+}
+
static inline bool of_console_check(const struct device_node *dn, const char
*name, int index)
{
return false;
--
2.9.3
[PATCH 2/3] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name()
In order to avoid a section mismatch use of_machine_get_compatible()
instead of of_flat_dt_get_machine_name() when printing the error
message.
Signed-off-by: Bartosz Golaszewski
---
drivers/bus/da8xx-mstpri.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bus/da8xx-mstpri.c b/drivers/bus/da8xx-mstpri.c
index 85f0b53..41cbbe6 100644
--- a/drivers/bus/da8xx-mstpri.c
+++ b/drivers/bus/da8xx-mstpri.c
@@ -227,7 +227,7 @@ static int da8xx_mstpri_probe(struct platform_device *pdev)
prio_list = da8xx_mstpri_get_board_prio();
if (!prio_list) {
dev_err(dev, "no master priotities defined for board '%s'\n",
- of_flat_dt_get_machine_name());
+ of_machine_get_compatible());
return -EINVAL;
}
--
2.9.3
[PATCH 3/3] memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name()
In order to avoid a section mismatch use of_machine_get_compatible()
instead of of_flat_dt_get_machine_name() when printing the error
message.
Signed-off-by: Bartosz Golaszewski
---
drivers/memory/da8xx-ddrctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
index a20e7bb..179f505 100644
--- a/drivers/memory/da8xx-ddrctl.c
+++ b/drivers/memory/da8xx-ddrctl.c
@@ -118,7 +118,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
setting = da8xx_ddrctl_get_board_settings();
if (!setting) {
dev_err(dev, "no settings for board '%s'\n",
- of_flat_dt_get_machine_name());
+ of_machine_get_compatible());
return -EINVAL;
}
--
2.9.3
[PATCH] drm/etnaviv: implement dma-buf mmap
This adds the required boilerplate to allow direct mmap of exported
etnaviv BOs.
Signed-off-by: Lucas Stach
Tested-by: Philipp Zabel
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 +
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 2 ++
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 +
3 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index aa687669e22b..38720adfc62f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -505,6 +505,7 @@ static struct drm_driver etnaviv_drm_driver = {
.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
.gem_prime_vmap = etnaviv_gem_prime_vmap,
.gem_prime_vunmap = etnaviv_gem_prime_vunmap,
+ .gem_prime_mmap = etnaviv_gem_prime_mmap,
#ifdef CONFIG_DEBUG_FS
.debugfs_init = etnaviv_debugfs_init,
.debugfs_cleanup= etnaviv_debugfs_cleanup,
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 65e057639653..c255eda40526 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -78,6 +78,8 @@ int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64
*offset);
struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
void *etnaviv_gem_prime_vmap(struct drm_gem_object *obj);
void etnaviv_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
+int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
+ struct vm_area_struct *vma);
struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device
*dev,
struct dma_buf_attachment *attach, struct sg_table *sg);
int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index b93618c1aa69..7e8fdb1859dd 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -39,6 +39,19 @@ void etnaviv_gem_prime_vunmap(struct drm_gem_object *obj,
void *vaddr)
/* TODO msm_gem_vunmap() */
}
+int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
+ struct vm_area_struct *vma)
+{
+ struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
+ int ret;
+
+ ret = drm_gem_mmap_obj(obj, obj->size, vma);
+ if (ret < 0)
+ return ret;
+
+ return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
+}
+
int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
{
if (!obj->import_attach) {
--
2.10.2
[Intel-gfx] [PATCH 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
Hi Gustavo,
A little late to the party here, but I was blocked by our internal
contributions process...
I didn't see these end up in my checkout yet though, so I guess they
aren't picked up yet.
On Mon, Nov 14, 2016 at 06:59:21PM +0900, Gustavo Padovan wrote:
>From: Gustavo Padovan
>
>Add support for the OUT_FENCE_PTR property to enable setting out fences for
>atomic commits.
>
>Signed-off-by: Gustavo Padovan
>---
> lib/igt_kms.c | 20 +++-
> lib/igt_kms.h | 3 +++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>index 4748c0a..f25e1eb 100644
>--- a/lib/igt_kms.c
>+++ b/lib/igt_kms.c
>@@ -175,7 +175,8 @@ const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> "DEGAMMA_LUT",
> "GAMMA_LUT",
> "MODE_ID",
>- "ACTIVE"
>+ "ACTIVE",
>+ "OUT_FENCE_PTR"
> };
>
> const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>@@ -2103,6 +2104,9 @@ static void igt_atomic_prepare_crtc_commit(igt_pipe_t
>*pipe_obj, drmModeAtomicRe
> igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE,
> !!output);
> }
>
>+ if (pipe_obj->out_fence_ptr)
>+ igt_atomic_populate_crtc_req(req, pipe_obj,
>IGT_CRTC_OUT_FENCE_PTR, pipe_obj->out_fence_ptr);
>+
> /*
>* TODO: Add all crtc level properties here
>*/
>@@ -2683,6 +2687,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr,
>size_t length)
> }
>
> /**
>+ * igt_pipe_set_out_fence_ptr:
>+ * @pipe: pipe pointer to which background color to be set
>+ * @fence_ptr: out fence pointer
I don't think fence_ptr can be int *. It needs to be a pointer to a
64-bit type.
>+ *
>+ * Sets the out fence pointer that will be passed to the kernel in
>+ * the atomic ioctl. When the kernel returns the out fence pointer
>+ * will contain the fd number of the out fence created by KMS.
>+ */
>+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr)
>+{
>+ pipe->out_fence_ptr = (uint64_t) fence_ptr;
>+}
>+
>+/**
> * igt_crtc_set_background:
> * @pipe: pipe pointer to which background color to be set
> * @background: background color value in BGR 16bpc
>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>index 344f931..02d7bd1 100644
>--- a/lib/igt_kms.h
>+++ b/lib/igt_kms.h
>@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
>IGT_CRTC_GAMMA_LUT,
>IGT_CRTC_MODE_ID,
>IGT_CRTC_ACTIVE,
>+ IGT_CRTC_OUT_FENCE_PTR,
>IGT_NUM_CRTC_PROPS
> };
>
>@@ -298,6 +299,7 @@ struct igt_pipe {
>
> uint64_t mode_blob;
> bool mode_changed;
>+ uint64_t out_fence_ptr;
IMO this should be:
int64_t *out_fence_ptr;
That way there can be no confusion about the type. You can convert it
to u64 just before giving it to the kernel.
Thanks,
-Brian
> };
>
> typedef struct {
>@@ -351,6 +353,7 @@ static inline bool igt_plane_supports_rotation(igt_plane_t
>*plane)
> void igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
> void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length);
> void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
>+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr);
>
> void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
> void igt_plane_set_fence_fd(igt_plane_t *plane, uint32_t fence_fd);
>--
>2.5.5
>
>___
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[PATCH 1/3] of: base: add support to get machine compatible string
2016-11-22 11:53 GMT+01:00 Sudeep Holla : > > > On 22/11/16 10:41, Bartosz Golaszewski wrote: >> >> Add a function allowing to retrieve the compatible string of the root >> node of the device tree. >> > > Rob has queued [1] and it's in -next today. You can reuse that if you > are planning to target this for v4.11 or just use open coding in your > driver for v4.10 and target this move for v4.11 to avoid cross tree > dependencies as I already mentioned in your previous thread. > > -- > Regards, > Sudeep > > [1] http://www.mail-archive.com/linux-kernel at > vger.kernel.org/msg1274549.html Rob's patch checks the model first - I'm not sure this is the behavior we want here as Sekhar suggested we print the machine compatible. Thanks, Bartosz Golaszewski
[PATCH 1/3] of: base: add support to get machine compatible string
2016-11-22 11:57 GMT+01:00 Bartosz Golaszewski : > 2016-11-22 11:53 GMT+01:00 Sudeep Holla : >> >> >> On 22/11/16 10:41, Bartosz Golaszewski wrote: >>> >>> Add a function allowing to retrieve the compatible string of the root >>> node of the device tree. >>> >> >> Rob has queued [1] and it's in -next today. You can reuse that if you >> are planning to target this for v4.11 or just use open coding in your >> driver for v4.10 and target this move for v4.11 to avoid cross tree >> dependencies as I already mentioned in your previous thread. >> >> -- >> Regards, >> Sudeep >> >> [1] http://www.mail-archive.com/linux-kernel at >> vger.kernel.org/msg1274549.html > > Rob's patch checks the model first - I'm not sure this is the behavior > we want here as Sekhar suggested we print the machine compatible. > I meant your patch of course. Thanks, Bartosz
[PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels
On Sat, Nov 19, 2016 at 05:28:02AM +0200, Laurent Pinchart wrote: > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. > Multiple incompatible data link layers have been used over time to > transmit image data to LVDS panels. This binding supports display panels > compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG > specifications. > > Signed-off-by: Laurent Pinchart > --- > .../bindings/display/panel/panel-lvds.txt | 120 > + > 1 file changed, 120 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-lvds.txt > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > new file mode 100644 > index ..b938269f841e > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > @@ -0,0 +1,120 @@ > +LVDS Display Panel > +== > + > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. > Multiple > +incompatible data link layers have been used over time to transmit image data > +to LVDS panels. This bindings supports display panels compatible with the > +following specifications. > + > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February > +1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA) > +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National > +Semiconductor > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video > +Electronics Standards Association (VESA) > + > +Device compatible with those specifications have been marketed under the > +FPD-Link and FlatLink brands. > + > + > +Required properties: > + > +- compatible: Shall contain "panel-lvds" in addition to a mandatory > + panel-specific compatible string defined in individual panel bindings. The > + "panel-lvds" value shall never be used on its own. What good is it if it shall never be used on its own? The above sounds to me like the panel-specific compatible string implies the LVDS binding, in a way that many compatible strings imply the simple binding. Note that initially we did the very same thing with "panel-simple", only to realize that it's completely redundant because it is never used. > +- width-mm: See panel-common.txt. > +- height-mm: See panel-common.txt. > +- data-mapping: The color signals mapping order, "jeida-18", "jeida-24" > + or "vesa-24". > + > +Optional properties: > + > +- label: See panel-common.txt. > +- gpios: See panel-common.txt. > +- backlight: See panel-common.txt. > +- data-mirror: If set, reverse the bit order described in the data mappings > + below on all data lanes, transmitting bits for slots 6 to 0 instead of > + 0 to 6. > + > +Required nodes: > + > +- panel-timing: See panel-common.txt. > +- ports: See panel-common.txt. These bindings require a single port subnode > + corresponding to the panel LVDS input. Looks like I should go read the patch that introduces panel-common.txt first... Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161122/a11ae106/attachment.sig>
[PATCH v2 01/13] devicetree/bindings: display: Document common panel properties
On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote: > Document properties common to several display panels in a central > location that can be referenced by the panel device tree bindings. > > Signed-off-by: Laurent Pinchart > --- > .../bindings/display/panel/panel-common.txt| 91 > ++ > 1 file changed, 91 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-common.txt > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt > b/Documentation/devicetree/bindings/display/panel/panel-common.txt > new file mode 100644 > index ..ec52c472c845 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt > @@ -0,0 +1,91 @@ > +Common Properties for Display Panel > +=== > + > +This document defines device tree properties common to several classes of > +display panels. It doesn't constitue a device tree binding specification by > +itself but is meant to be referenced by device tree bindings. > + > +When referenced from panel device tree bindings the properties defined in > this > +document are defined as follows. The panel device tree bindings are > +responsible for defining whether each property is required or optional. > + > + > +Descriptive Properties > +-- > + > +- width-mm, > +- height-mm: The width-mm and height-mm specify the width and height of the > + physical area where images are displayed. These properties are expressed in > + millimeters and rounded to the closest unit. Erm... this is already implied by the compatible string. Having this in device tree is completely redundant. > +- label: The label property specifies a symbolic name for the panel as a > + string suitable for use by humans. It typically contains a name inscribed > on > + the system (e.g. as an affixed label) or specified in the system's > + documentation (e.g. in the user's manual). > + > + If no such name exists, and unless the property is mandatory according to > + device tree bindings, it shall rather be omitted than constructed of > + non-descriptive information. For instance an LCD panel in a system that > + contains a single panel shall not be labelled "LCD" if that name is not > + inscribed on the system or used in a descriptive fashion in system > + documentation. > + > + > +Display Timings > +--- > + > +- panel-timing: Most display panels are restricted to a single resolution and > + require specific display timings. The panel-timing subnode expresses those > + timings as specified in the timing subnode section of the display timing > + bindings defined in > + Documentation/devicetree/bindings/display/display-timing.txt. Why? That's also implied by the compatible string. Honestly, I thought by now we had been over this often enough... Thierry -- next part ------ A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161122/ed0ff2df/attachment.sig>
[Intel-gfx] [PATCH 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
On Tue, Nov 22, 2016 at 10:53:51AM +, Brian Starkey wrote:
> Hi Gustavo,
>
> A little late to the party here, but I was blocked by our internal
> contributions process...
>
> I didn't see these end up in my checkout yet though, so I guess they
> aren't picked up yet.
>
> On Mon, Nov 14, 2016 at 06:59:21PM +0900, Gustavo Padovan wrote:
> >From: Gustavo Padovan
> >
> >Add support for the OUT_FENCE_PTR property to enable setting out fences for
> >atomic commits.
> >
> >Signed-off-by: Gustavo Padovan
> >---
> >lib/igt_kms.c | 20 +++-
> >lib/igt_kms.h | 3 +++
> >2 files changed, 22 insertions(+), 1 deletion(-)
> >
> >diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> >index 4748c0a..f25e1eb 100644
> >--- a/lib/igt_kms.c
> >+++ b/lib/igt_kms.c
> >@@ -175,7 +175,8 @@ const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> > "DEGAMMA_LUT",
> > "GAMMA_LUT",
> > "MODE_ID",
> >-"ACTIVE"
> >+"ACTIVE",
> >+"OUT_FENCE_PTR"
> >};
> >
> >const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> >@@ -2103,6 +2104,9 @@ static void igt_atomic_prepare_crtc_commit(igt_pipe_t
> >*pipe_obj, drmModeAtomicRe
> > igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE,
> > !!output);
> > }
> >
> >+if (pipe_obj->out_fence_ptr)
> >+igt_atomic_populate_crtc_req(req, pipe_obj,
> >IGT_CRTC_OUT_FENCE_PTR, pipe_obj->out_fence_ptr);
> >+
> > /*
> > * TODO: Add all crtc level properties here
> > */
> >@@ -2683,6 +2687,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr,
> >size_t length)
> >}
> >
> >/**
> >+ * igt_pipe_set_out_fence_ptr:
> >+ * @pipe: pipe pointer to which background color to be set
> >+ * @fence_ptr: out fence pointer
>
> I don't think fence_ptr can be int *. It needs to be a pointer to a
> 64-bit type.
>
> >+ *
> >+ * Sets the out fence pointer that will be passed to the kernel in
> >+ * the atomic ioctl. When the kernel returns the out fence pointer
> >+ * will contain the fd number of the out fence created by KMS.
> >+ */
> >+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr)
> >+{
> >+pipe->out_fence_ptr = (uint64_t) fence_ptr;
> >+}
> >+
> >+/**
> > * igt_crtc_set_background:
> > * @pipe: pipe pointer to which background color to be set
> > * @background: background color value in BGR 16bpc
> >diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> >index 344f931..02d7bd1 100644
> >--- a/lib/igt_kms.h
> >+++ b/lib/igt_kms.h
> >@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
> > IGT_CRTC_GAMMA_LUT,
> > IGT_CRTC_MODE_ID,
> > IGT_CRTC_ACTIVE,
> >+ IGT_CRTC_OUT_FENCE_PTR,
> > IGT_NUM_CRTC_PROPS
> >};
> >
> >@@ -298,6 +299,7 @@ struct igt_pipe {
> >
> > uint64_t mode_blob;
> > bool mode_changed;
> >+uint64_t out_fence_ptr;
>
> IMO this should be:
>
> int64_t *out_fence_ptr;
In userspace, fences are *fd*, a plain int. It is only the uabi that we
pass pointers as u64 to the kernel, and indeed that should be limited to
the uabi wrapper.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH v2 06/13] drm: panels: Add LVDS panel driver
On Sat, Nov 19, 2016 at 05:28:06AM +0200, Laurent Pinchart wrote: > This driver supports LVDS panels that don't require device-specific > handling of power supplies or control signals. It implements automatic > backlight handling if the panel is attached to a backlight controller. > > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/panel/Kconfig | 10 ++ > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-lvds.c | 284 > + > 3 files changed, 295 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-lvds.c The bulk of this is a duplicate of panel-simple.c and it adds all the things on top that I've been repeatedly rejecting in the past. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161122/88e3057c/attachment.sig>
[PATCH 06/12] lib/igt_kms: Add support for the IN_FENCE_FD property
Hi,
On Mon, Nov 14, 2016 at 06:59:20PM +0900, Gustavo Padovan wrote:
>From: Robert Foss
>
>Add support dor the IN_FENCE_FD property to enable setting in fences for atomic
>commits.
>
>Signed-off-by: Robert Foss
>---
> lib/igt_kms.c | 20
> lib/igt_kms.h | 5 +
> 2 files changed, 25 insertions(+)
>
>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>index 8aaff5b..4748c0a 100644
>--- a/lib/igt_kms.c
>+++ b/lib/igt_kms.c
>@@ -164,6 +164,7 @@ const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> "CRTC_H",
> "FB_ID",
> "CRTC_ID",
>+ "IN_FENCE_FD",
> "type",
> "rotation"
> };
>@@ -1426,6 +1427,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> n_planes++;
> plane->pipe = pipe;
> plane->drm_plane = drm_plane;
>+ plane->fence_fd = -1;
>
> if (is_atomic == 0) {
> display->is_atomic = 1;
>@@ -1712,6 +1714,11 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane,
>igt_pipe_t *pipe,
> plane->index,
> fb_id);
>
>+ if (plane->fence_fd >= 0) {
>+ uint32_t fence_fd = plane->fence_fd;
Assigning to uint32_t here will make the top bytes zero when it gets
cast to uint64_t. I guess that works out fine because the cast back to
int in the kernel will be 32-bits, but IMO better to cast to int64_t
here to get proper sign-extension to 64-bits.
>+ igt_atomic_populate_plane_req(req, plane,
>IGT_PLANE_IN_FENCE_FD, fence_fd);
>+ }
>+
> if (plane->fb_changed) {
> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID,
> fb_id ? crtc_id : 0);
> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID,
> fb_id);
>@@ -2522,6 +2529,19 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb
>*fb)
> plane->size_changed = true;
> }
>
>+/**
>+ * igt_plane_set_fence_fd:
>+ * @plane: plane
>+ * @fence_fd: fence fd, disable fence_fd by setting it to 0
Should be -1 to disable.
Cheers,
Brian
>+ *
>+ * This function sets a fence fd to enable a commit to wait for some event to
>+ * occur before completing.
>+ */
>+void igt_plane_set_fence_fd(igt_plane_t *plane, uint32_t fence_fd)
>+{
>+ plane->fence_fd = fence_fd;
>+}
>+
> void igt_plane_set_position(igt_plane_t *plane, int x, int y)
> {
> igt_pipe_t *pipe = plane->pipe;
>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>index ae2b505..344f931 100644
>--- a/lib/igt_kms.h
>+++ b/lib/igt_kms.h
>@@ -213,6 +213,7 @@ enum igt_atomic_plane_properties {
>
>IGT_PLANE_FB_ID,
>IGT_PLANE_CRTC_ID,
>+ IGT_PLANE_IN_FENCE_FD,
>IGT_PLANE_TYPE,
>IGT_PLANE_ROTATION,
>IGT_NUM_PLANE_PROPS
>@@ -266,6 +267,9 @@ typedef struct {
> uint32_t src_h;
>
> igt_rotation_t rotation;
>+
>+ /* in fence fd */
>+ int32_t fence_fd;
> uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS];
> } igt_plane_t;
>
>@@ -349,6 +353,7 @@ void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr,
>size_t length);
> void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
>
> void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
>+void igt_plane_set_fence_fd(igt_plane_t *plane, uint32_t fence_fd);
> void igt_plane_set_position(igt_plane_t *plane, int x, int y);
> void igt_plane_set_size(igt_plane_t *plane, int w, int h);
> void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
>--
>2.5.5
>
>___
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] [PATCH 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
On Tue, Nov 22, 2016 at 11:06:00AM +, Chris Wilson wrote:
>On Tue, Nov 22, 2016 at 10:53:51AM +, Brian Starkey wrote:
>> Hi Gustavo,
>>
>> A little late to the party here, but I was blocked by our internal
>> contributions process...
>>
>> I didn't see these end up in my checkout yet though, so I guess they
>> aren't picked up yet.
>>
>> On Mon, Nov 14, 2016 at 06:59:21PM +0900, Gustavo Padovan wrote:
>> >From: Gustavo Padovan
>> >
>> >Add support for the OUT_FENCE_PTR property to enable setting out fences for
>> >atomic commits.
>> >
>> >Signed-off-by: Gustavo Padovan
>> >---
>> >lib/igt_kms.c | 20 +++-
>> >lib/igt_kms.h | 3 +++
>> >2 files changed, 22 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> >index 4748c0a..f25e1eb 100644
>> >--- a/lib/igt_kms.c
>> >+++ b/lib/igt_kms.c
>> >@@ -175,7 +175,8 @@ const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
>> >"DEGAMMA_LUT",
>> >"GAMMA_LUT",
>> >"MODE_ID",
>> >- "ACTIVE"
>> >+ "ACTIVE",
>> >+ "OUT_FENCE_PTR"
>> >};
>> >
>> >const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>> >@@ -2103,6 +2104,9 @@ static void igt_atomic_prepare_crtc_commit(igt_pipe_t
>> >*pipe_obj, drmModeAtomicRe
>> >igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE,
>> > !!output);
>> >}
>> >
>> >+ if (pipe_obj->out_fence_ptr)
>> >+ igt_atomic_populate_crtc_req(req, pipe_obj,
>> >IGT_CRTC_OUT_FENCE_PTR, pipe_obj->out_fence_ptr);
>> >+
>> >/*
>> > * TODO: Add all crtc level properties here
>> > */
>> >@@ -2683,6 +2687,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr,
>> >size_t length)
>> >}
>> >
>> >/**
>> >+ * igt_pipe_set_out_fence_ptr:
>> >+ * @pipe: pipe pointer to which background color to be set
>> >+ * @fence_ptr: out fence pointer
>>
>> I don't think fence_ptr can be int *. It needs to be a pointer to a
>> 64-bit type.
>>
>> >+ *
>> >+ * Sets the out fence pointer that will be passed to the kernel in
>> >+ * the atomic ioctl. When the kernel returns the out fence pointer
>> >+ * will contain the fd number of the out fence created by KMS.
>> >+ */
>> >+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr)
>> >+{
>> >+ pipe->out_fence_ptr = (uint64_t) fence_ptr;
>> >+}
>> >+
>> >+/**
>> > * igt_crtc_set_background:
>> > * @pipe: pipe pointer to which background color to be set
>> > * @background: background color value in BGR 16bpc
>> >diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> >index 344f931..02d7bd1 100644
>> >--- a/lib/igt_kms.h
>> >+++ b/lib/igt_kms.h
>> >@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
>> > IGT_CRTC_GAMMA_LUT,
>> > IGT_CRTC_MODE_ID,
>> > IGT_CRTC_ACTIVE,
>> >+ IGT_CRTC_OUT_FENCE_PTR,
>> > IGT_NUM_CRTC_PROPS
>> >};
>> >
>> >@@ -298,6 +299,7 @@ struct igt_pipe {
>> >
>> >uint64_t mode_blob;
>> >bool mode_changed;
>> >+ uint64_t out_fence_ptr;
>>
>> IMO this should be:
>>
>> int64_t *out_fence_ptr;
>
>In userspace, fences are *fd*, a plain int. It is only the uabi that we
>pass pointers as u64 to the kernel, and indeed that should be limited to
>the uabi wrapper.
>-Chris
Where's the uabi wrapper in this case?
Wherever it is, afaik someone needs to have 64-bit type for the kernel
to stash its fd in - on the kernel side out_fence_ptr is
(s64 __user *), so if there's not a 64-bit variable on the other end
of it then someone's going to have a bad day.
-Brian
>
>--
>Chris Wilson, Intel Open Source Technology Centre
[Intel-gfx] [PATCH 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
On Tue, Nov 22, 2016 at 11:54:57AM +, Brian Starkey wrote:
> On Tue, Nov 22, 2016 at 11:06:00AM +, Chris Wilson wrote:
> >On Tue, Nov 22, 2016 at 10:53:51AM +, Brian Starkey wrote:
> >>Hi Gustavo,
> >>
> >>A little late to the party here, but I was blocked by our internal
> >>contributions process...
> >>
> >>I didn't see these end up in my checkout yet though, so I guess they
> >>aren't picked up yet.
> >>
> >>On Mon, Nov 14, 2016 at 06:59:21PM +0900, Gustavo Padovan wrote:
> >>>From: Gustavo Padovan
> >>>
> >>>Add support for the OUT_FENCE_PTR property to enable setting out fences for
> >>>atomic commits.
> >>>
> >>>Signed-off-by: Gustavo Padovan
> >>>---
> >>>lib/igt_kms.c | 20 +++-
> >>>lib/igt_kms.h | 3 +++
> >>>2 files changed, 22 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> >>>index 4748c0a..f25e1eb 100644
> >>>--- a/lib/igt_kms.c
> >>>+++ b/lib/igt_kms.c
> >>>@@ -175,7 +175,8 @@ const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> >>> "DEGAMMA_LUT",
> >>> "GAMMA_LUT",
> >>> "MODE_ID",
> >>>- "ACTIVE"
> >>>+ "ACTIVE",
> >>>+ "OUT_FENCE_PTR"
> >>>};
> >>>
> >>>const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> >>>@@ -2103,6 +2104,9 @@ static void
> >>>igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
> >>> igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE,
> >>> !!output);
> >>> }
> >>>
> >>>+ if (pipe_obj->out_fence_ptr)
> >>>+ igt_atomic_populate_crtc_req(req, pipe_obj,
> >>>IGT_CRTC_OUT_FENCE_PTR, pipe_obj->out_fence_ptr);
> >>>+
> >>> /*
> >>>* TODO: Add all crtc level properties here
> >>>*/
> >>>@@ -2683,6 +2687,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr,
> >>>size_t length)
> >>>}
> >>>
> >>>/**
> >>>+ * igt_pipe_set_out_fence_ptr:
> >>>+ * @pipe: pipe pointer to which background color to be set
> >>>+ * @fence_ptr: out fence pointer
> >>
> >>I don't think fence_ptr can be int *. It needs to be a pointer to a
> >>64-bit type.
> >>
> >>>+ *
> >>>+ * Sets the out fence pointer that will be passed to the kernel in
> >>>+ * the atomic ioctl. When the kernel returns the out fence pointer
> >>>+ * will contain the fd number of the out fence created by KMS.
> >>>+ */
> >>>+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr)
> >>>+{
> >>>+ pipe->out_fence_ptr = (uint64_t) fence_ptr;
> >>>+}
> >>>+
> >>>+/**
> >>> * igt_crtc_set_background:
> >>> * @pipe: pipe pointer to which background color to be set
> >>> * @background: background color value in BGR 16bpc
> >>>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> >>>index 344f931..02d7bd1 100644
> >>>--- a/lib/igt_kms.h
> >>>+++ b/lib/igt_kms.h
> >>>@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
> >>> IGT_CRTC_GAMMA_LUT,
> >>> IGT_CRTC_MODE_ID,
> >>> IGT_CRTC_ACTIVE,
> >>>+ IGT_CRTC_OUT_FENCE_PTR,
> >>> IGT_NUM_CRTC_PROPS
> >>>};
> >>>
> >>>@@ -298,6 +299,7 @@ struct igt_pipe {
> >>>
> >>> uint64_t mode_blob;
> >>> bool mode_changed;
> >>>+ uint64_t out_fence_ptr;
> >>
> >>IMO this should be:
> >>
> >>int64_t *out_fence_ptr;
> >
> >In userspace, fences are *fd*, a plain int. It is only the uabi that we
> >pass pointers as u64 to the kernel, and indeed that should be limited to
> >the uabi wrapper.
> >-Chris
>
> Where's the uabi wrapper in this case?
>
> Wherever it is, afaik someone needs to have 64-bit type for the kernel
> to stash its fd in - on the kernel side out_fence_ptr is
> (s64 __user *), so if there's not a 64-bit variable on the other end
> of it then someone's going to have a bad day.
We do not have pointers in the uabi because they are different sizes on
different platforms. The uabi must be a u64 representation of a user
address to store the result - that is what we pass to the crtc set
property ioctl. That it has been futher managled not to pass around fd
is an interesting twist, but ideally that sillyness should not make
itself into our API.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[Intel-gfx] [PATCH 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
On Tue, Nov 22, 2016 at 12:10:52PM +, Chris Wilson wrote:
>On Tue, Nov 22, 2016 at 11:54:57AM +, Brian Starkey wrote:
>> On Tue, Nov 22, 2016 at 11:06:00AM +, Chris Wilson wrote:
>> >On Tue, Nov 22, 2016 at 10:53:51AM +, Brian Starkey wrote:
>> >>Hi Gustavo,
>> >>
>> >>A little late to the party here, but I was blocked by our internal
>> >>contributions process...
>> >>
>> >>I didn't see these end up in my checkout yet though, so I guess they
>> >>aren't picked up yet.
>> >>
>> >>On Mon, Nov 14, 2016 at 06:59:21PM +0900, Gustavo Padovan wrote:
>> >>>From: Gustavo Padovan
>> >>>
>> >>>Add support for the OUT_FENCE_PTR property to enable setting out fences
>> >>>for
>> >>>atomic commits.
>> >>>
>> >>>Signed-off-by: Gustavo Padovan
>> >>>---
>> >>>lib/igt_kms.c | 20 +++-
>> >>>lib/igt_kms.h | 3 +++
>> >>>2 files changed, 22 insertions(+), 1 deletion(-)
>> >>>
>> >>>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> >>>index 4748c0a..f25e1eb 100644
>> >>>--- a/lib/igt_kms.c
>> >>>+++ b/lib/igt_kms.c
>> >>>@@ -175,7 +175,8 @@ const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] =
>> >>>{
>> >>> "DEGAMMA_LUT",
>> >>> "GAMMA_LUT",
>> >>> "MODE_ID",
>> >>>- "ACTIVE"
>> >>>+ "ACTIVE",
>> >>>+ "OUT_FENCE_PTR"
>> >>>};
>> >>>
>> >>>const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>> >>>@@ -2103,6 +2104,9 @@ static void
>> >>>igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
>> >>> igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE,
>> >>> !!output);
>> >>> }
>> >>>
>> >>>+ if (pipe_obj->out_fence_ptr)
>> >>>+ igt_atomic_populate_crtc_req(req, pipe_obj,
>> >>>IGT_CRTC_OUT_FENCE_PTR, pipe_obj->out_fence_ptr);
>> >>>+
>> >>> /*
>> >>> * TODO: Add all crtc level properties here
>> >>> */
>> >>>@@ -2683,6 +2687,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void
>> >>>*ptr, size_t length)
>> >>>}
>> >>>
>> >>>/**
>> >>>+ * igt_pipe_set_out_fence_ptr:
>> >>>+ * @pipe: pipe pointer to which background color to be set
>> >>>+ * @fence_ptr: out fence pointer
>> >>
>> >>I don't think fence_ptr can be int *. It needs to be a pointer to a
>> >>64-bit type.
>> >>
>> >>>+ *
>> >>>+ * Sets the out fence pointer that will be passed to the kernel in
>> >>>+ * the atomic ioctl. When the kernel returns the out fence pointer
>> >>>+ * will contain the fd number of the out fence created by KMS.
>> >>>+ */
>> >>>+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr)
>> >>>+{
>> >>>+ pipe->out_fence_ptr = (uint64_t) fence_ptr;
>> >>>+}
>> >>>+
>> >>>+/**
>> >>> * igt_crtc_set_background:
>> >>> * @pipe: pipe pointer to which background color to be set
>> >>> * @background: background color value in BGR 16bpc
>> >>>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> >>>index 344f931..02d7bd1 100644
>> >>>--- a/lib/igt_kms.h
>> >>>+++ b/lib/igt_kms.h
>> >>>@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
>> >>> IGT_CRTC_GAMMA_LUT,
>> >>> IGT_CRTC_MODE_ID,
>> >>> IGT_CRTC_ACTIVE,
>> >>>+ IGT_CRTC_OUT_FENCE_PTR,
>> >>> IGT_NUM_CRTC_PROPS
>> >>>};
>> >>>
>> >>>@@ -298,6 +299,7 @@ struct igt_pipe {
>> >>>
>> >>> uint64_t mode_blob;
>> >>> bool mode_changed;
>> >>>+ uint64_t out_fence_ptr;
>> >>
>> >>IMO this should be:
>> >>
>> >> int64_t *out_fence_ptr;
>> >
>> >In userspace, fences are *fd*, a plain int. It is only the uabi that we
>> >pass pointers as u64 to the kernel, and indeed that should be limited to
>> >the uabi wrapper.
>> >-Chris
>>
>> Where's the uabi wrapper in this case?
>>
>> Wherever it is, afaik someone needs to have 64-bit type for the kernel
>> to stash its fd in - on the kernel side out_fence_ptr is
>> (s64 __user *), so if there's not a 64-bit variable on the other end
>> of it then someone's going to have a bad day.
>
>We do not have pointers in the uabi because they are different sizes on
>different platforms. The uabi must be a u64 representation of a user
>address to store the result - that is what we pass to the crtc set
>property ioctl.
Sure, but igt_pipe is not a uabi structure. By storing a uint64_t here
we're making it needlessly opaque what the value is actually meant to
be - which is the address of a 64-bit signed integer.
Regardless, tests cannot set out_fence_ptr to the address of an int, I
hope we can agree on that. Where that detail gets taken care of I
don't much mind - but this code as-is is incorrect.
By making igt_pipe.out_fence_ptr an (int64_t *) I thought we'd be
letting the compiler warn anyone else away from incorrect code.
>That it has been futher managled not to pass around fd
>is an interesting twist, but ideally that sillyness should not make
>itself into our API.
Allowing the kernel and userspace to have different ideas about how
big an int is doesn't sound so silly to me. It may not be a
theoretical problem forever.
-Brian
>-Chris
>
>--
>Chris Wilson, Intel Open Source Technology Centre
[Bug 98761] [regression][radeonsi][polaris]"radeonsi: set IF_THRESHOLD to 3" breaks Witcher2's ground
https://bugs.freedesktop.org/show_bug.cgi?id=98761 --- Comment #16 from Grazvydas Ignotas --- Possible duplicates: bug 98776 bug 98783 bug 98785 llvm bug: https://llvm.org/bugs/show_bug.cgi?id=31019 -- 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/20161122/5e585c27/attachment.html>
[PATCH v2 0/2] da8xx: fix section mismatch in new drivers
Sekhar noticed there's a section mismatch in the da8xx-mstpri and da8xx-ddrctl drivers. This is caused by calling of_flat_dt_get_machine_name() which has an __init annotation. This series addresses this issue by open coding routines that return the machine compatible string in both drivers. Once a general function for that in of/base is merged, we'll remove them. v1 -> v2: - drop patch [1/3] from v1 - introduce internal routines in the drivers instead of a general function in of/base.c Bartosz Golaszewski (2): bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name() memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name() drivers/bus/da8xx-mstpri.c| 22 -- drivers/memory/da8xx-ddrctl.c | 22 -- 2 files changed, 40 insertions(+), 4 deletions(-) -- 2.9.3
[PATCH v2 1/2] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name()
In order to avoid a section mismatch use a locally implemented routine
instead of of_flat_dt_get_machine_name() when printing the error
message.
Signed-off-by: Bartosz Golaszewski
---
drivers/bus/da8xx-mstpri.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/da8xx-mstpri.c b/drivers/bus/da8xx-mstpri.c
index 85f0b53..bd38170 100644
--- a/drivers/bus/da8xx-mstpri.c
+++ b/drivers/bus/da8xx-mstpri.c
@@ -16,7 +16,6 @@
#include
#include
#include
-#include
/*
* REVISIT: Linux doesn't have a good framework for the kind of performance
@@ -190,6 +189,25 @@ static const struct da8xx_mstpri_board_priorities
da8xx_mstpri_board_confs[] = {
},
};
+/*
+ * FIXME Remove this function once of/base gets a general routine for getting
+ * the machine model/compatible string.
+ */
+static const char *da8xx_mstpri_machine_get_compatible(void)
+{
+ struct device_node *root;
+ const char *compatible;
+ int ret = -1;
+
+ root = of_find_node_by_path("/");
+ if (root) {
+ ret = of_property_read_string(root, "compatible", &compatible);
+ of_node_put(root);
+ }
+
+ return ret ? NULL : compatible;
+}
+
static const struct da8xx_mstpri_board_priorities *
da8xx_mstpri_get_board_prio(void)
{
@@ -227,7 +245,7 @@ static int da8xx_mstpri_probe(struct platform_device *pdev)
prio_list = da8xx_mstpri_get_board_prio();
if (!prio_list) {
dev_err(dev, "no master priotities defined for board '%s'\n",
- of_flat_dt_get_machine_name());
+ da8xx_mstpri_machine_get_compatible());
return -EINVAL;
}
--
2.9.3
[PATCH v2 2/2] memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name()
In order to avoid a section mismatch use a locally implemented routine
instead of of_flat_dt_get_machine_name() when printing the error
message.
Signed-off-by: Bartosz Golaszewski
---
drivers/memory/da8xx-ddrctl.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
index a20e7bb..ee0c266 100644
--- a/drivers/memory/da8xx-ddrctl.c
+++ b/drivers/memory/da8xx-ddrctl.c
@@ -14,7 +14,6 @@
#include
#include
#include
-#include
#include
#include
@@ -71,6 +70,25 @@ static const struct da8xx_ddrctl_board_settings
da8xx_ddrctl_board_confs[] = {
},
};
+/*
+ * FIXME Remove this function once of/base gets a general routine for getting
+ * the machine model/compatible string.
+ */
+static const char *da8xx_ddrctl_machine_get_compatible(void)
+{
+ struct device_node *root;
+ const char *compatible;
+ int ret = -1;
+
+ root = of_find_node_by_path("/");
+ if (root) {
+ ret = of_property_read_string(root, "compatible", &compatible);
+ of_node_put(root);
+ }
+
+ return ret ? NULL : compatible;
+}
+
static const struct da8xx_ddrctl_config_knob *
da8xx_ddrctl_match_knob(const struct da8xx_ddrctl_setting *setting)
{
@@ -118,7 +136,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
setting = da8xx_ddrctl_get_board_settings();
if (!setting) {
dev_err(dev, "no settings for board '%s'\n",
- of_flat_dt_get_machine_name());
+ da8xx_ddrctl_machine_get_compatible());
return -EINVAL;
}
--
2.9.3
[Intel-gfx] [PATCH 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
On Tue, Nov 22, 2016 at 12:37:47PM +, Brian Starkey wrote:
> On Tue, Nov 22, 2016 at 12:10:52PM +, Chris Wilson wrote:
> > On Tue, Nov 22, 2016 at 11:54:57AM +, Brian Starkey wrote:
> > > On Tue, Nov 22, 2016 at 11:06:00AM +, Chris Wilson wrote:
> > > >On Tue, Nov 22, 2016 at 10:53:51AM +, Brian Starkey wrote:
> > > >>Hi Gustavo,
> > > >>
> > > >>A little late to the party here, but I was blocked by our internal
> > > >>contributions process...
> > > >>
> > > >>I didn't see these end up in my checkout yet though, so I guess they
> > > >>aren't picked up yet.
> > > >>
> > > >>On Mon, Nov 14, 2016 at 06:59:21PM +0900, Gustavo Padovan wrote:
> > > >>>From: Gustavo Padovan
> > > >>>
> > > >>>Add support for the OUT_FENCE_PTR property to enable setting out
> > > >>>fences for
> > > >>>atomic commits.
> > > >>>
> > > >>>Signed-off-by: Gustavo Padovan
> > > >>>---
> > > >>>lib/igt_kms.c | 20 +++-
> > > >>>lib/igt_kms.h | 3 +++
> > > >>>2 files changed, 22 insertions(+), 1 deletion(-)
> > > >>>
> > > >>>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > >>>index 4748c0a..f25e1eb 100644
> > > >>>--- a/lib/igt_kms.c
> > > >>>+++ b/lib/igt_kms.c
> > > >>>@@ -175,7 +175,8 @@ const char
> > > >>>*igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> > > >>> "DEGAMMA_LUT",
> > > >>> "GAMMA_LUT",
> > > >>> "MODE_ID",
> > > >>>- "ACTIVE"
> > > >>>+ "ACTIVE",
> > > >>>+ "OUT_FENCE_PTR"
> > > >>>};
> > > >>>
> > > >>>const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> > > >>>@@ -2103,6 +2104,9 @@ static void
> > > >>>igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
> > > >>> igt_atomic_populate_crtc_req(req, pipe_obj,
> > > >>> IGT_CRTC_ACTIVE, !!output);
> > > >>> }
> > > >>>
> > > >>>+ if (pipe_obj->out_fence_ptr)
> > > >>>+ igt_atomic_populate_crtc_req(req, pipe_obj,
> > > >>>IGT_CRTC_OUT_FENCE_PTR, pipe_obj->out_fence_ptr);
> > > >>>+
> > > >>> /*
> > > >>>* TODO: Add all crtc level properties here
> > > >>>*/
> > > >>>@@ -2683,6 +2687,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void
> > > >>>*ptr, size_t length)
> > > >>>}
> > > >>>
> > > >>>/**
> > > >>>+ * igt_pipe_set_out_fence_ptr:
> > > >>>+ * @pipe: pipe pointer to which background color to be set
> > > >>>+ * @fence_ptr: out fence pointer
> > > >>
> > > >>I don't think fence_ptr can be int *. It needs to be a pointer to a
> > > >>64-bit type.
> > > >>
> > > >>>+ *
> > > >>>+ * Sets the out fence pointer that will be passed to the kernel in
> > > >>>+ * the atomic ioctl. When the kernel returns the out fence pointer
> > > >>>+ * will contain the fd number of the out fence created by KMS.
> > > >>>+ */
> > > >>>+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr)
> > > >>>+{
> > > >>>+ pipe->out_fence_ptr = (uint64_t) fence_ptr;
> > > >>>+}
> > > >>>+
> > > >>>+/**
> > > >>> * igt_crtc_set_background:
> > > >>> * @pipe: pipe pointer to which background color to be set
> > > >>> * @background: background color value in BGR 16bpc
> > > >>>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > >>>index 344f931..02d7bd1 100644
> > > >>>--- a/lib/igt_kms.h
> > > >>>+++ b/lib/igt_kms.h
> > > >>>@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
> > > >>> IGT_CRTC_GAMMA_LUT,
> > > >>> IGT_CRTC_MODE_ID,
> > > >>> IGT_CRTC_ACTIVE,
> > > >>>+ IGT_CRTC_OUT_FENCE_PTR,
> > > >>> IGT_NUM_CRTC_PROPS
> > > >>>};
> > > >>>
> > > >>>@@ -298,6 +299,7 @@ struct igt_pipe {
> > > >>>
> > > >>> uint64_t mode_blob;
> > > >>> bool mode_changed;
> > > >>>+ uint64_t out_fence_ptr;
> > > >>
> > > >>IMO this should be:
> > > >>
> > > >>int64_t *out_fence_ptr;
> > > >
> > > >In userspace, fences are *fd*, a plain int. It is only the uabi that we
> > > >pass pointers as u64 to the kernel, and indeed that should be limited to
> > > >the uabi wrapper.
> > > >-Chris
> > >
> > > Where's the uabi wrapper in this case?
> > >
> > > Wherever it is, afaik someone needs to have 64-bit type for the kernel
> > > to stash its fd in - on the kernel side out_fence_ptr is
> > > (s64 __user *), so if there's not a 64-bit variable on the other end
> > > of it then someone's going to have a bad day.
> >
> > We do not have pointers in the uabi because they are different sizes on
> > different platforms. The uabi must be a u64 representation of a user
> > address to store the result - that is what we pass to the crtc set
> > property ioctl.
>
> Sure, but igt_pipe is not a uabi structure. By storing a uint64_t here
> we're making it needlessly opaque what the value is actually meant to
> be - which is the address of a 64-bit signed integer.
>
> Regardless, tests cannot set out_fence_ptr to the address of an int, I
> hope we can agree on that. Where that detail gets taken care of I
> don't much mind - but this code as-is is incorrect.
>
> By making igt_pipe.out_fence_ptr a
[PATCH v2 01/13] devicetree/bindings: display: Document common panel properties
Hi Thierry, On Tuesday 22 Nov 2016 12:05:48 Thierry Reding wrote: > On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote: > > Document properties common to several display panels in a central > > location that can be referenced by the panel device tree bindings. > > > > Signed-off-by: Laurent Pinchart > > > > --- > > > > .../bindings/display/panel/panel-common.txt| 91 + > > 1 file changed, 91 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/panel/panel-common.txt> > > diff --git > > a/Documentation/devicetree/bindings/display/panel/panel-common.txt > > b/Documentation/devicetree/bindings/display/panel/panel-common.txt > > new file mode 100644 > > index ..ec52c472c845 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt > > @@ -0,0 +1,91 @@ > > +Common Properties for Display Panel > > +=== > > + > > +This document defines device tree properties common to several classes of > > +display panels. It doesn't constitue a device tree binding specification > > by > > +itself but is meant to be referenced by device tree bindings. > > + > > +When referenced from panel device tree bindings the properties defined in > > this > > +document are defined as follows. The panel device tree bindings are > > +responsible for defining whether each property is required or optional. > > + > > + > > +Descriptive Properties > > +-- > > + > > +- width-mm, > > +- height-mm: The width-mm and height-mm specify the width and height of > > the > > + physical area where images are displayed. These properties are > > expressed in > > + millimeters and rounded to the closest unit. > > Erm... this is already implied by the compatible string. Having this in > device tree is completely redundant. Nothing new under the sun here, we already have plenty of properties that could be implied by compatible strings. For instance for SoC IP cores many vendors use both an SoC-specific compatible string and a generic compatible string (e.g. "renesas,gpio-r8a7795" and "renesas,gpio-rcar"). The SoC- compatible string implies register addresses, clocks and interrupts, but we still describe them in DT. At the end of the day information about devices and their integration in the system needs to be available, either from DT or from C code. DT bindings should be designed to strike a good balance there, avoiding redundant information in DT (and thus keeping the bindings simple) while still providing enough information to allow for a reasonable level of genericity in OS implementations. > > +- label: The label property specifies a symbolic name for the panel as a > > + string suitable for use by humans. It typically contains a name > > inscribed on > > + the system (e.g. as an affixed label) or specified in the system's > > + documentation (e.g. in the user's manual). > > + > > + If no such name exists, and unless the property is mandatory according > > to > > + device tree bindings, it shall rather be omitted than constructed of > > + non-descriptive information. For instance an LCD panel in a system that > > + contains a single panel shall not be labelled "LCD" if that name is not > > + inscribed on the system or used in a descriptive fashion in system > > + documentation. > > + > > + > > +Display Timings > > +--- > > + > > +- panel-timing: Most display panels are restricted to a single resolution > > and > > + require specific display timings. The panel-timing subnode expresses > > those > > + timings as specified in the timing subnode section of the display > > timing > > + bindings defined in > > + Documentation/devicetree/bindings/display/display-timing.txt. > > Why? That's also implied by the compatible string. Honestly, I thought > by now we had been over this often enough... Same argument as above. I won't try to change your mind and fix the simple panel driver, but I still stand firm on my belief that expressing the size and timings in DT is the right solution in a wide variety of cases (and yes I've read http://sietch-tagr.blogspot.fi/2016/04/display-panels-are-not-special.html, and while I agree with the title, I still believe size and timings in DT are not wrong). -- Regards, Laurent Pinchart
[PATCH v2 06/13] drm: panels: Add LVDS panel driver
Hi Thierry, On Tuesday 22 Nov 2016 12:14:57 Thierry Reding wrote: > On Sat, Nov 19, 2016 at 05:28:06AM +0200, Laurent Pinchart wrote: > > This driver supports LVDS panels that don't require device-specific > > handling of power supplies or control signals. It implements automatic > > backlight handling if the panel is attached to a backlight controller. > > > > Signed-off-by: Laurent Pinchart > > > > --- > > > > drivers/gpu/drm/panel/Kconfig | 10 ++ > > drivers/gpu/drm/panel/Makefile | 1 + > > drivers/gpu/drm/panel/panel-lvds.c | 284 > > 3 files changed, 295 insertions(+) > > create mode 100644 drivers/gpu/drm/panel/panel-lvds.c > > The bulk of this is a duplicate of panel-simple.c Implementing the panel API should obviously be expected to produce some quantity of similar code. > and it adds all the things on top that I've been repeatedly rejecting in the > past. It's a good thing I haven't tried to add them to panel-simple.c then :-) -- Regards, Laurent Pinchart
[PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels
Hi Thierry, On Tuesday 22 Nov 2016 12:02:41 Thierry Reding wrote: > On Sat, Nov 19, 2016 at 05:28:02AM +0200, Laurent Pinchart wrote: > > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. > > Multiple incompatible data link layers have been used over time to > > transmit image data to LVDS panels. This binding supports display panels > > compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG > > specifications. > > > > Signed-off-by: Laurent Pinchart > > > > --- > > > > .../bindings/display/panel/panel-lvds.txt | 120 > > 1 file changed, 120 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/panel/panel-lvds.txt> > > diff --git > > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt new file > > mode 100644 > > index ..b938269f841e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt > > @@ -0,0 +1,120 @@ > > +LVDS Display Panel > > +== > > + > > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. > > Multiple +incompatible data link layers have been used over time to > > transmit image data +to LVDS panels. This bindings supports display > > panels compatible with the +following specifications. > > + > > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, > > February > > +1999 (Version 1.0), Japan Electronic Industry Development Association > > (JEIDA) +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), > > National +Semiconductor > > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video > > +Electronics Standards Association (VESA) > > + > > +Device compatible with those specifications have been marketed under the > > +FPD-Link and FlatLink brands. > > + > > + > > +Required properties: > > + > > +- compatible: Shall contain "panel-lvds" in addition to a mandatory > > + panel-specific compatible string defined in individual panel bindings. > > The > > + "panel-lvds" value shall never be used on its own. > > What good is it if it shall never be used on its own? The above sounds > to me like the panel-specific compatible string implies the LVDS > binding, in a way that many compatible strings imply the simple binding. > Note that initially we did the very same thing with "panel-simple", only > to realize that it's completely redundant because it is never used. DT allows specifying multiple compatible strings in decreasing order of genericity to make generic OS implementations possible while retaining the ability to later introduce device-specific code if/when the need arises (mostly because of information that were overlooked, misunderstood or just not available at implementation time - we unfortunately can't produce 100% perfect solutions all the time, I very much regret that). This is exactly what the LVDS panel bindings mandate. > > +- width-mm: See panel-common.txt. > > +- height-mm: See panel-common.txt. > > +- data-mapping: The color signals mapping order, "jeida-18", "jeida-24" > > + or "vesa-24". > > + > > +Optional properties: > > + > > +- label: See panel-common.txt. > > +- gpios: See panel-common.txt. > > +- backlight: See panel-common.txt. > > +- data-mirror: If set, reverse the bit order described in the data > > mappings > > + below on all data lanes, transmitting bits for slots 6 to 0 instead of > > + 0 to 6. > > + > > +Required nodes: > > + > > +- panel-timing: See panel-common.txt. > > +- ports: See panel-common.txt. These bindings require a single port > > subnode > > + corresponding to the panel LVDS input. > > Looks like I should go read the patch that introduces panel-common.txt > first... -- Regards, Laurent Pinchart
[Intel-gfx] [PATCH v9 01/11] drm/i915: Add i915 perf infrastructure
On Wed, Nov 09, 2016 at 08:00:06PM +, Matthew Auld wrote:
> On 7 November 2016 at 19:49, Robert Bragg wrote:
> > Adds base i915 perf infrastructure for Gen performance metrics.
> >
> > This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an array of uint64
> > properties to configure a stream of metrics and returns a new fd usable
> > with standard VFS system calls including read() to read typed and sized
> > records; ioctl() to enable or disable capture and poll() to wait for
> > data.
> >
> > A stream is opened something like:
> >
> > uint64_t properties[] = {
> > /* Single context sampling */
> > DRM_I915_PERF_PROP_CTX_HANDLE,ctx_handle,
> >
> > /* Include OA reports in samples */
> > DRM_I915_PERF_PROP_SAMPLE_OA, true,
> >
> > /* OA unit configuration */
> > DRM_I915_PERF_PROP_OA_METRICS_SET,metrics_set_id,
> > DRM_I915_PERF_PROP_OA_FORMAT, report_format,
> > DRM_I915_PERF_PROP_OA_EXPONENT, period_exponent,
> >};
> >struct drm_i915_perf_open_param parm = {
> > .flags = I915_PERF_FLAG_FD_CLOEXEC |
> >I915_PERF_FLAG_FD_NONBLOCK |
> >I915_PERF_FLAG_DISABLED,
> > .properties_ptr = (uint64_t)properties,
> > .num_properties = sizeof(properties) / 16,
> >};
> >int fd = drmIoctl(drm_fd, DRM_IOCTL_I915_PERF_OPEN, ¶m);
> >
> > Records read all start with a common { type, size } header with
> > DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample records
> > contain an extensible number of fields and it's the
> > DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening that
> > determine what's included in every sample.
> >
> > No specific streams are supported yet so any attempt to open a stream
> > will return an error.
> >
> > v2:
> > use i915_gem_context_get() - Chris Wilson
> > v3:
> > update read() interface to avoid passing state struct - Chris Wilson
> > fix some rebase fallout, with i915-perf init/deinit
> > v4:
> > s/DRM_IORW/DRM_IOW/ - Emil Velikov
> >
> > Signed-off-by: Robert Bragg
> > Reviewed-by: Matthew Auld
> > Reviewed-by: Sourab Gupta
> Minor nit, there are a fair few DRM_ERROR's missing a new line.
Also, DRM_ERROR for userspace-triggerable failures is no good. igt
testcase are supposed to exercise all the invalid stuff, and would then
fail if you spam dmesg. Why was this not caught?
Fixup patch totally fine, but if this wasn't caught due to missing igt
that needs to be fixed, too.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH v9 01/11] drm/i915: Add i915 perf infrastructure
On Tue, Nov 22, 2016 at 02:29:18PM +0100, Daniel Vetter wrote:
> On Wed, Nov 09, 2016 at 08:00:06PM +, Matthew Auld wrote:
> > On 7 November 2016 at 19:49, Robert Bragg wrote:
> > > Adds base i915 perf infrastructure for Gen performance metrics.
> > >
> > > This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an array of uint64
> > > properties to configure a stream of metrics and returns a new fd usable
> > > with standard VFS system calls including read() to read typed and sized
> > > records; ioctl() to enable or disable capture and poll() to wait for
> > > data.
> > >
> > > A stream is opened something like:
> > >
> > > uint64_t properties[] = {
> > > /* Single context sampling */
> > > DRM_I915_PERF_PROP_CTX_HANDLE,ctx_handle,
> > >
> > > /* Include OA reports in samples */
> > > DRM_I915_PERF_PROP_SAMPLE_OA, true,
> > >
> > > /* OA unit configuration */
> > > DRM_I915_PERF_PROP_OA_METRICS_SET,metrics_set_id,
> > > DRM_I915_PERF_PROP_OA_FORMAT, report_format,
> > > DRM_I915_PERF_PROP_OA_EXPONENT, period_exponent,
> > >};
> > >struct drm_i915_perf_open_param parm = {
> > > .flags = I915_PERF_FLAG_FD_CLOEXEC |
> > >I915_PERF_FLAG_FD_NONBLOCK |
> > >I915_PERF_FLAG_DISABLED,
> > > .properties_ptr = (uint64_t)properties,
> > > .num_properties = sizeof(properties) / 16,
> > >};
> > >int fd = drmIoctl(drm_fd, DRM_IOCTL_I915_PERF_OPEN, ¶m);
> > >
> > > Records read all start with a common { type, size } header with
> > > DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample records
> > > contain an extensible number of fields and it's the
> > > DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening that
> > > determine what's included in every sample.
> > >
> > > No specific streams are supported yet so any attempt to open a stream
> > > will return an error.
> > >
> > > v2:
> > > use i915_gem_context_get() - Chris Wilson
> > > v3:
> > > update read() interface to avoid passing state struct - Chris Wilson
> > > fix some rebase fallout, with i915-perf init/deinit
> > > v4:
> > > s/DRM_IORW/DRM_IOW/ - Emil Velikov
> > >
> > > Signed-off-by: Robert Bragg
> > > Reviewed-by: Matthew Auld
> > > Reviewed-by: Sourab Gupta
> > Minor nit, there are a fair few DRM_ERROR's missing a new line.
>
> Also, DRM_ERROR for userspace-triggerable failures is no good. igt
> testcase are supposed to exercise all the invalid stuff, and would then
> fail if you spam dmesg. Why was this not caught?
>
> Fixup patch totally fine, but if this wasn't caught due to missing igt
> that needs to be fixed, too.
Another nitpick for the future: Enabling new features first and then
fixing up the fallout is the wrong way round, if someone bisects over this
range mesa might blow up in really bad ways.
Oh well, this has been out there for way too long, so meh.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH v2] drm/i915: don't whitelist oacontrol in cmd parser
On Tue, Nov 08, 2016 at 12:51:48PM +, Robert Bragg wrote:
> This v2 patch bumps the command parser version so it can be referenced in
> corresponding i-g-t gem_exec_parse changes.
>
> --- >8 ---
Scissors cut everything below, not everything above, hence next time
around pls switch around your comment and the commit message, as-is not
much left ;-)
Fixed up while applying.
-Daniel
>
> Being able to program OACONTROL from a non-privileged batch buffer is
> not sufficient to be able to configure the OA unit. This was originally
> allowed to help enable Mesa to expose OA counters via the
> INTEL_performance_query extension, but the current implementation based
> on programming OACONTROL via a batch buffer isn't able to report useable
> data without a more complete OA unit configuration. Mesa handles the
> possibility that writes to OACONTROL may not be allowed and so only
> advertises the extension after explicitly testing that a write to
> OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
> should be ok for userspace.
>
> Removing this simplifies adding a new kernel api for configuring the OA
> unit without needing to consider the possibility that userspace might
> trample on OACONTROL state which we'd like to start managing within
> the kernel instead. In particular running any Mesa based GL application
> currently results in clearing OACONTROL when initializing which would
> disable the capturing of metrics.
>
> v2:
> This bumps the command parser version from 8 to 9, as the change is
> visible to userspace.
>
> Signed-off-by: Robert Bragg
> Reviewed-by: Matthew Auld
> Reviewed-by: Sourab Gupta
> ---
> drivers/gpu/drm/i915/i915_cmd_parser.c | 42
> --
> 1 file changed, 5 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
> b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index c9d2ecd..f5762cd 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -450,7 +450,6 @@ static const struct drm_i915_reg_descriptor
> gen7_render_regs[] = {
> REG64(PS_INVOCATION_COUNT),
> REG64(PS_DEPTH_COUNT),
> REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
> - REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below. */
> REG64(MI_PREDICATE_SRC0),
> REG64(MI_PREDICATE_SRC1),
> REG32(GEN7_3DPRIM_END_OFFSET),
> @@ -1060,8 +1059,7 @@ bool intel_engine_needs_cmd_parser(struct
> intel_engine_cs *engine)
> static bool check_cmd(const struct intel_engine_cs *engine,
> const struct drm_i915_cmd_descriptor *desc,
> const u32 *cmd, u32 length,
> - const bool is_master,
> - bool *oacontrol_set)
> + const bool is_master)
> {
> if (desc->flags & CMD_DESC_SKIP)
> return true;
> @@ -1099,31 +1097,6 @@ static bool check_cmd(const struct intel_engine_cs
> *engine,
> }
>
> /*
> - * OACONTROL requires some special handling for
> - * writes. We want to make sure that any batch which
> - * enables OA also disables it before the end of the
> - * batch. The goal is to prevent one process from
> - * snooping on the perf data from another process. To do
> - * that, we need to check the value that will be written
> - * to the register. Hence, limit OACONTROL writes to
> - * only MI_LOAD_REGISTER_IMM commands.
> - */
> - if (reg_addr == i915_mmio_reg_offset(GEN7_OACONTROL)) {
> - if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
> - DRM_DEBUG_DRIVER("CMD: Rejected LRM to
> OACONTROL\n");
> - return false;
> - }
> -
> - if (desc->cmd.value == MI_LOAD_REGISTER_REG) {
> - DRM_DEBUG_DRIVER("CMD: Rejected LRR to
> OACONTROL\n");
> - return false;
> - }
> -
> - if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1))
> - *oacontrol_set = (cmd[offset + 1] != 0);
> - }
> -
> - /*
>* Check the value written to the register against the
>* allowed mask/value pair given in the whitelist entry.
>*/
> @@ -1214,7 +1187,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs
> *engine,
> u32 *cmd, *batch_end;
> struct drm_i915_cmd_descriptor default_desc = noop_desc;
> const struct drm_i915_cmd_descriptor *desc = &default_desc;
> - bool oacontrol_set = false; /*
[PATCH v2 36/37] drm: Add mode_config .get_format_info() hook
From: Ville Syrjälä
Allow drivers to return a custom drm_format_info structure for special
fb layouts. We'll use this for the compression control surface in i915.
v2: Fix drm_get_format_info() kernel doc (Laurent)
Don't pass 'dev' to the new hook (Laurent)
Cc: Laurent Pinchart
Cc: Ben Widawsky
Cc: intel-gfx at lists.freedesktop.org
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
drivers/gpu/drm/drm_fourcc.c | 25 +
drivers/gpu/drm/drm_framebuffer.c| 9 +++--
drivers/gpu/drm/drm_modeset_helper.c | 2 +-
include/drm/drm_fourcc.h | 6 ++
include/drm/drm_mode_config.h| 14 ++
6 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
b/drivers/gpu/drm/drm_fb_cma_helper.c
index aab4465307ed..d7f8876cf5e9 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -186,7 +186,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct
drm_device *dev,
int ret;
int i;
- info = drm_format_info(mode_cmd->pixel_format);
+ info = drm_get_format_info(dev, mode_cmd);
if (!info)
return ERR_PTR(-EINVAL);
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 90d2cc8da8eb..f9b6445e846a 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -199,6 +199,31 @@ const struct drm_format_info *drm_format_info(u32 format)
EXPORT_SYMBOL(drm_format_info);
/**
+ * drm_get_format_info - query information for a given framebuffer
configuration
+ * @dev: DRM device
+ * @mode_cmd: metadata from the userspace fb creation request
+ *
+ * Returns:
+ * The instance of struct drm_format_info that describes the pixel format, or
+ * NULL if the format is unsupported.
+ */
+const struct drm_format_info *
+drm_get_format_info(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+ const struct drm_format_info *info = NULL;
+
+ if (dev->mode_config.funcs->get_format_info)
+ info = dev->mode_config.funcs->get_format_info(mode_cmd);
+
+ if (!info)
+ info = drm_format_info(mode_cmd->pixel_format);
+
+ return info;
+}
+EXPORT_SYMBOL(drm_get_format_info);
+
+/**
* drm_format_num_planes - get the number of planes for format
* @format: pixel format (DRM_FORMAT_*)
*
diff --git a/drivers/gpu/drm/drm_framebuffer.c
b/drivers/gpu/drm/drm_framebuffer.c
index 94ddab41f24f..292930a5dcc2 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -126,11 +126,13 @@ int drm_mode_addfb(struct drm_device *dev,
return 0;
}
-static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
+static int framebuffer_check(struct drm_device *dev,
+const struct drm_mode_fb_cmd2 *r)
{
const struct drm_format_info *info;
int i;
+ /* check if the format is supported at all */
info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
if (!info) {
struct drm_format_name_buf format_name;
@@ -140,6 +142,9 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2
*r)
return -EINVAL;
}
+ /* now let the driver pick its own format info */
+ info = drm_get_format_info(dev, r);
+
if (r->width == 0 || r->width % info->hsub) {
DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
return -EINVAL;
@@ -263,7 +268,7 @@ drm_internal_framebuffer_create(struct drm_device *dev,
return ERR_PTR(-EINVAL);
}
- ret = framebuffer_check(r);
+ ret = framebuffer_check(dev, r);
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/drm_modeset_helper.c
b/drivers/gpu/drm/drm_modeset_helper.c
index 5b051859b8d3..f78df06a940d 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -75,7 +75,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device *dev,
int i;
fb->dev = dev;
- fb->format = drm_format_info(mode_cmd->pixel_format);
+ fb->format = drm_get_format_info(dev, mode_cmd);
fb->width = mode_cmd->width;
fb->height = mode_cmd->height;
for (i = 0; i < 4; i++) {
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index fcc08da850c8..6942e84b6edd 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -25,6 +25,9 @@
#include
#include
+struct drm_device;
+struct drm_mode_fb_cmd2;
+
/**
* struct drm_format_info - information about a DRM format
* @format: 4CC format identifier (DRM_FORMAT_*)
@@ -55,6 +58,9 @@ struct drm_format_name_buf {
const struct drm_format_info *__drm_format_info(u32 format);
const struct drm_format_info *drm_format_info(u32 format);
+const struct drm_format_info *
+drm_get_forma
[PATCH v9 11/11] drm/i915: Add a kerneldoc summary for i915_perf.c
On Mon, Nov 07, 2016 at 07:49:57PM +, Robert Bragg wrote: > In particular this tries to capture for posterity some of the early > challenges we had with using the core perf infrastructure in case we > ever want to revisit adapting perf for device metrics. > > Cc: Chris Wilson > Signed-off-by: Robert Bragg > Reviewed-by: Matthew Auld > --- > drivers/gpu/drm/i915/i915_perf.c | 163 > +++ Missing the include stanza in Documentation/gpu/i915.rst, which means this won't show up in the docs build. Also function/struct docs are not included either it seems. Please check out Documentation/kernel-documentation.rst and build it all with $ make DOCBOOKS="" htmldocs Again fixup patch is fine. -Daniel > 1 file changed, 163 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c > b/drivers/gpu/drm/i915/i915_perf.c > index 1a87fe9..9551282 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -24,6 +24,169 @@ > * Robert Bragg > */ > > + > +/** > + * DOC: i915 Perf, streaming API for GPU metrics > + * > + * Gen graphics supports a large number of performance counters that can help > + * driver and application developers understand and optimize their use of the > + * GPU. > + * > + * This i915 perf interface enables userspace to configure and open a file > + * descriptor representing a stream of GPU metrics which can then be read() > as > + * a stream of sample records. > + * > + * The interface is particularly suited to exposing buffered metrics that are > + * captured by DMA from the GPU, unsynchronized with and unrelated to the > CPU. > + * > + * Streams representing a single context are accessible to applications with > a > + * corresponding drm file descriptor, such that OpenGL can use the interface > + * without special privileges. Access to system-wide metrics requires root > + * privileges by default, unless changed via the dev.i915.perf_event_paranoid > + * sysctl option. > + * > + * > + * The interface was initially inspired by the core Perf infrastructure but > + * some notable differences are: > + * > + * i915 perf file descriptors represent a "stream" instead of an "event"; > where > + * a perf event primarily corresponds to a single 64bit value, while a stream > + * might sample sets of tightly-coupled counters, depending on the > + * configuration. For example the Gen OA unit isn't designed to support > + * orthogonal configurations of individual counters; it's configured for a > set > + * of related counters. Samples for an i915 perf stream capturing OA metrics > + * will include a set of counter values packed in a compact HW specific > format. > + * The OA unit supports a number of different packing formats which can be > + * selected by the user opening the stream. Perf has support for grouping > + * events, but each event in the group is configured, validated and > + * authenticated individually with separate system calls. > + * > + * i915 perf stream configurations are provided as an array of u64 > (key,value) > + * pairs, instead of a fixed struct with multiple miscellaneous config > members, > + * interleaved with event-type specific members. > + * > + * i915 perf doesn't support exposing metrics via an mmap'd circular buffer. > + * The supported metrics are being written to memory by the GPU > unsynchronized > + * with the CPU, using HW specific packing formats for counter sets. > Sometimes > + * the constraints on HW configuration require reports to be filtered before > it > + * would be acceptable to expose them to unprivileged applications - to hide > + * the metrics of other processes/contexts. For these use cases a read() > based > + * interface is a good fit, and provides an opportunity to filter data as it > + * gets copied from the GPU mapped buffers to userspace buffers. > + * > + * > + * Some notes regarding Linux Perf: > + * > + * > + * The first prototype of this driver was based on the core perf > + * infrastructure, and while we did make that mostly work, with some changes > to > + * perf, we found we were breaking or working around too many assumptions > baked > + * into perf's currently cpu centric design. > + * > + * In the end we didn't see a clear benefit to making perf's implementation > and > + * interface more complex by changing design assumptions while we knew we > still > + * wouldn't be able to use any existing perf based userspace tools. > + * > + * Also considering the Gen specific nature of the Observability hardware and > + * how userspace will sometimes need to combine i915 perf OA metrics with > + * side-band OA data captured via MI_REPORT_PERF_COUNT commands; we're > + * expecting the interface to be used by a platform specific userspace such > as > + * OpenGL or tools. This is to say; we aren't inherently missing out on > having > + * a standard vendor/architecture agnostic interface by not using perf. > +
[PATCH 09/37] drm/arm: Add local 'fb' variables
On Mon, Nov 21, 2016 at 11:51:21AM +, Liviu Dudau wrote:
> On Fri, Nov 18, 2016 at 09:52:45PM +0200, ville.syrjala at linux.intel.com
> wrote:
> > From: Ville Syrjälä
> >
> > Add a local 'fb' variable to a few places to get rid of the
> > 'crtc->primary->fb' stuff. Looks neater and helps me with my ppor
> > coccinelle skills later.
> >
> > In some places the local variable was already there, just not used
> > consistently.
> >
> > Cc: Liviu Dudau
>
> Acked-by: Liviu Dudau
>
> Are you going to take the series through drm-misc or you want each
> sub-maintainer
> to cherry pick the patches?
Probably easier to suck it all in one go. Unless drm-misc maintainers
disagree?
>
> Best regards,
> Liviu
>
> > Cc: Brian Starkey
> > Cc: Mali DP Maintainers
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/arm/hdlcd_crtc.c| 18 ++
> > drivers/gpu/drm/arm/malidp_planes.c | 6 +++---
> > 2 files changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c
> > b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > index bbaa55add2d2..8a0fee03aa39 100644
> > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > @@ -60,11 +60,12 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
> > {
> > unsigned int btpp;
> > struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
> > + const struct drm_framebuffer *fb = crtc->primary->state->fb;
> > uint32_t pixel_format;
> > struct simplefb_format *format = NULL;
> > int i;
> >
> > - pixel_format = crtc->primary->state->fb->pixel_format;
> > + pixel_format = fb->pixel_format;
> >
> > for (i = 0; i < ARRAY_SIZE(supported_formats); i++) {
> > if (supported_formats[i].fourcc == pixel_format)
> > @@ -221,27 +222,28 @@ static int hdlcd_plane_atomic_check(struct drm_plane
> > *plane,
> > static void hdlcd_plane_atomic_update(struct drm_plane *plane,
> > struct drm_plane_state *state)
> > {
> > + struct drm_framebuffer *fb = plane->state->fb;
> > struct hdlcd_drm_private *hdlcd;
> > struct drm_gem_cma_object *gem;
> > u32 src_w, src_h, dest_w, dest_h;
> > dma_addr_t scanout_start;
> >
> > - if (!plane->state->fb)
> > + if (!fb)
> > return;
> >
> > src_w = plane->state->src_w >> 16;
> > src_h = plane->state->src_h >> 16;
> > dest_w = plane->state->crtc_w;
> > dest_h = plane->state->crtc_h;
> > - gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0);
> > - scanout_start = gem->paddr + plane->state->fb->offsets[0] +
> > - plane->state->crtc_y * plane->state->fb->pitches[0] +
> > + gem = drm_fb_cma_get_gem_obj(fb, 0);
> > + scanout_start = gem->paddr + fb->offsets[0] +
> > + plane->state->crtc_y * fb->pitches[0] +
> > plane->state->crtc_x *
> > - drm_format_plane_cpp(plane->state->fb->pixel_format, 0);
> > + drm_format_plane_cpp(fb->pixel_format, 0);
> >
> > hdlcd = plane->dev->dev_private;
> > - hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH,
> > plane->state->fb->pitches[0]);
> > - hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_PITCH,
> > plane->state->fb->pitches[0]);
> > + hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, fb->pitches[0]);
> > + hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_PITCH, fb->pitches[0]);
> > hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_COUNT, dest_h - 1);
> > hdlcd_write(hdlcd, HDLCD_REG_FB_BASE, scanout_start);
> > }
> > diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> > b/drivers/gpu/drm/arm/malidp_planes.c
> > index 63eec8f37cfc..ee7f7663a307 100644
> > --- a/drivers/gpu/drm/arm/malidp_planes.c
> > +++ b/drivers/gpu/drm/arm/malidp_planes.c
> > @@ -137,8 +137,8 @@ static int malidp_de_plane_check(struct drm_plane
> > *plane,
> >
> > /* packed RGB888 / BGR888 can't be rotated or flipped */
> > if (state->rotation != DRM_ROTATE_0 &&
> > - (state->fb->pixel_format == DRM_FORMAT_RGB888 ||
> > -state->fb->pixel_format == DRM_FORMAT_BGR888))
> > + (fb->pixel_format == DRM_FORMAT_RGB888 ||
> > +fb->pixel_format == DRM_FORMAT_BGR888))
> > return -EINVAL;
> >
> > ms->rotmem_size = 0;
> > @@ -147,7 +147,7 @@ static int malidp_de_plane_check(struct drm_plane
> > *plane,
> >
> > val = mp->hwdev->rotmem_required(mp->hwdev, state->crtc_h,
> > state->crtc_w,
> > -state->fb->pixel_format);
> > +fb->pixel_format);
> > if (val < 0)
> > return val;
> >
> > --
> > 2.7.4
> >
>
> --
>
> | I would like to |
> | fix the world, |
> | but they're not |
> | giving me the |
> \ source code! /
> ---
> ¯\_(ã)_/¯
--
Ville Syrjälä
Intel OTC
[Intel-gfx] [PATCH 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
On Tue, Nov 22, 2016 at 02:12:59PM +0100, Daniel Vetter wrote:
>On Tue, Nov 22, 2016 at 12:37:47PM +, Brian Starkey wrote:
>> On Tue, Nov 22, 2016 at 12:10:52PM +, Chris Wilson wrote:
>> > On Tue, Nov 22, 2016 at 11:54:57AM +, Brian Starkey wrote:
>> > > On Tue, Nov 22, 2016 at 11:06:00AM +, Chris Wilson wrote:
>> > > >On Tue, Nov 22, 2016 at 10:53:51AM +, Brian Starkey wrote:
>> > > >>Hi Gustavo,
>> > > >>
>> > > >>A little late to the party here, but I was blocked by our internal
>> > > >>contributions process...
>> > > >>
>> > > >>I didn't see these end up in my checkout yet though, so I guess they
>> > > >>aren't picked up yet.
>> > > >>
>> > > >>On Mon, Nov 14, 2016 at 06:59:21PM +0900, Gustavo Padovan wrote:
>> > > >>>From: Gustavo Padovan
>> > > >>>
>> > > >>>Add support for the OUT_FENCE_PTR property to enable setting out
>> > > >>>fences for
>> > > >>>atomic commits.
>> > > >>>
>> > > >>>Signed-off-by: Gustavo Padovan
>> > > >>>---
>> > > >>>lib/igt_kms.c | 20 +++-
>> > > >>>lib/igt_kms.h | 3 +++
>> > > >>>2 files changed, 22 insertions(+), 1 deletion(-)
>> > > >>>
>> > > >>>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> > > >>>index 4748c0a..f25e1eb 100644
>> > > >>>--- a/lib/igt_kms.c
>> > > >>>+++ b/lib/igt_kms.c
>> > > >>>@@ -175,7 +175,8 @@ const char
>> > > >>>*igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
>> > > >>> "DEGAMMA_LUT",
>> > > >>> "GAMMA_LUT",
>> > > >>> "MODE_ID",
>> > > >>>- "ACTIVE"
>> > > >>>+ "ACTIVE",
>> > > >>>+ "OUT_FENCE_PTR"
>> > > >>>};
>> > > >>>
>> > > >>>const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>> > > >>>@@ -2103,6 +2104,9 @@ static void
>> > > >>>igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
>> > > >>> igt_atomic_populate_crtc_req(req, pipe_obj,
>> > > >>> IGT_CRTC_ACTIVE, !!output);
>> > > >>> }
>> > > >>>
>> > > >>>+ if (pipe_obj->out_fence_ptr)
>> > > >>>+ igt_atomic_populate_crtc_req(req, pipe_obj,
>> > > >>>IGT_CRTC_OUT_FENCE_PTR, pipe_obj->out_fence_ptr);
>> > > >>>+
>> > > >>> /*
>> > > >>> * TODO: Add all crtc level properties here
>> > > >>> */
>> > > >>>@@ -2683,6 +2687,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void
>> > > >>>*ptr, size_t length)
>> > > >>>}
>> > > >>>
>> > > >>>/**
>> > > >>>+ * igt_pipe_set_out_fence_ptr:
>> > > >>>+ * @pipe: pipe pointer to which background color to be set
>> > > >>>+ * @fence_ptr: out fence pointer
>> > > >>
>> > > >>I don't think fence_ptr can be int *. It needs to be a pointer to a
>> > > >>64-bit type.
>> > > >>
>> > > >>>+ *
>> > > >>>+ * Sets the out fence pointer that will be passed to the kernel in
>> > > >>>+ * the atomic ioctl. When the kernel returns the out fence pointer
>> > > >>>+ * will contain the fd number of the out fence created by KMS.
>> > > >>>+ */
>> > > >>>+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr)
>> > > >>>+{
>> > > >>>+ pipe->out_fence_ptr = (uint64_t) fence_ptr;
>> > > >>>+}
>> > > >>>+
>> > > >>>+/**
>> > > >>> * igt_crtc_set_background:
>> > > >>> * @pipe: pipe pointer to which background color to be set
>> > > >>> * @background: background color value in BGR 16bpc
>> > > >>>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> > > >>>index 344f931..02d7bd1 100644
>> > > >>>--- a/lib/igt_kms.h
>> > > >>>+++ b/lib/igt_kms.h
>> > > >>>@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
>> > > >>> IGT_CRTC_GAMMA_LUT,
>> > > >>> IGT_CRTC_MODE_ID,
>> > > >>> IGT_CRTC_ACTIVE,
>> > > >>>+ IGT_CRTC_OUT_FENCE_PTR,
>> > > >>> IGT_NUM_CRTC_PROPS
>> > > >>>};
>> > > >>>
>> > > >>>@@ -298,6 +299,7 @@ struct igt_pipe {
>> > > >>>
>> > > >>> uint64_t mode_blob;
>> > > >>> bool mode_changed;
>> > > >>>+ uint64_t out_fence_ptr;
>> > > >>
>> > > >>IMO this should be:
>> > > >>
>> > > >> int64_t *out_fence_ptr;
>> > > >
>> > > >In userspace, fences are *fd*, a plain int. It is only the uabi that we
>> > > >pass pointers as u64 to the kernel, and indeed that should be limited to
>> > > >the uabi wrapper.
>> > > >-Chris
>> > >
>> > > Where's the uabi wrapper in this case?
>> > >
>> > > Wherever it is, afaik someone needs to have 64-bit type for the kernel
>> > > to stash its fd in - on the kernel side out_fence_ptr is
>> > > (s64 __user *), so if there's not a 64-bit variable on the other end
>> > > of it then someone's going to have a bad day.
>> >
>> > We do not have pointers in the uabi because they are different sizes on
>> > different platforms. The uabi must be a u64 representation of a user
>> > address to store the result - that is what we pass to the crtc set
>> > property ioctl.
>>
>> Sure, but igt_pipe is not a uabi structure. By storing a uint64_t here
>> we're making it needlessly opaque what the value is actually meant to
>> be - which is the address of a 64-bit signed integer.
>>
>> Regardless, tests cannot set out_fence_ptr to the address of an int,
[PATCH v2 12/37] drm/vmwgfx: Populate fb->dev before drm_framebuffer_init()
From: Ville Syrjälä drm_framebuffer_init() will start to check that fb->dev is already populated, so let's do that manually since vmwgfx isn't using drm_helper_mode_fill_fb_struct(). v2: s/to/do/ (Sinclair) Cc: linux-graphics-maintainer at vmware.com Cc: Sinclair Yeh Cc: Thomas Hellstrom Signed-off-by: Ville Syrjälä Reviewed-by: Sinclair Yeh --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index e3f68cc9bb4b..7d92ab56961b 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -581,6 +581,7 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv, goto out_err1; } + vfbs->base.base.dev = dev; /* XXX get the first 3 from the surface info */ vfbs->base.base.bits_per_pixel = mode_cmd->bpp; vfbs->base.base.pitches[0] = mode_cmd->pitch; @@ -875,6 +876,7 @@ static int vmw_kms_new_framebuffer_dmabuf(struct vmw_private *dev_priv, goto out_err1; } + vfbd->base.base.dev = dev; vfbd->base.base.bits_per_pixel = mode_cmd->bpp; vfbd->base.base.pitches[0] = mode_cmd->pitch; vfbd->base.base.depth = mode_cmd->depth; -- 2.7.4
[Intel-gfx] [PATCH 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
On Tue, Nov 22, 2016 at 01:50:53PM +, Brian Starkey wrote:
> On Tue, Nov 22, 2016 at 02:12:59PM +0100, Daniel Vetter wrote:
> > On Tue, Nov 22, 2016 at 12:37:47PM +, Brian Starkey wrote:
> > > On Tue, Nov 22, 2016 at 12:10:52PM +, Chris Wilson wrote:
> > > > On Tue, Nov 22, 2016 at 11:54:57AM +, Brian Starkey wrote:
> > > > > On Tue, Nov 22, 2016 at 11:06:00AM +, Chris Wilson wrote:
> > > > > >On Tue, Nov 22, 2016 at 10:53:51AM +, Brian Starkey wrote:
> > > > > >>Hi Gustavo,
> > > > > >>
> > > > > >>A little late to the party here, but I was blocked by our internal
> > > > > >>contributions process...
> > > > > >>
> > > > > >>I didn't see these end up in my checkout yet though, so I guess they
> > > > > >>aren't picked up yet.
> > > > > >>
> > > > > >>On Mon, Nov 14, 2016 at 06:59:21PM +0900, Gustavo Padovan wrote:
> > > > > >>>From: Gustavo Padovan
> > > > > >>>
> > > > > >>>Add support for the OUT_FENCE_PTR property to enable setting out
> > > > > >>>fences for
> > > > > >>>atomic commits.
> > > > > >>>
> > > > > >>>Signed-off-by: Gustavo Padovan
> > > > > >>>---
> > > > > >>>lib/igt_kms.c | 20 +++-
> > > > > >>>lib/igt_kms.h | 3 +++
> > > > > >>>2 files changed, 22 insertions(+), 1 deletion(-)
> > > > > >>>
> > > > > >>>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > > > >>>index 4748c0a..f25e1eb 100644
> > > > > >>>--- a/lib/igt_kms.c
> > > > > >>>+++ b/lib/igt_kms.c
> > > > > >>>@@ -175,7 +175,8 @@ const char
> > > > > >>>*igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> > > > > >>> "DEGAMMA_LUT",
> > > > > >>> "GAMMA_LUT",
> > > > > >>> "MODE_ID",
> > > > > >>>- "ACTIVE"
> > > > > >>>+ "ACTIVE",
> > > > > >>>+ "OUT_FENCE_PTR"
> > > > > >>>};
> > > > > >>>
> > > > > >>>const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> > > > > >>>@@ -2103,6 +2104,9 @@ static void
> > > > > >>>igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj,
> > > > > >>>drmModeAtomicRe
> > > > > >>> igt_atomic_populate_crtc_req(req, pipe_obj,
> > > > > >>> IGT_CRTC_ACTIVE, !!output);
> > > > > >>> }
> > > > > >>>
> > > > > >>>+ if (pipe_obj->out_fence_ptr)
> > > > > >>>+ igt_atomic_populate_crtc_req(req, pipe_obj,
> > > > > >>>IGT_CRTC_OUT_FENCE_PTR, pipe_obj->out_fence_ptr);
> > > > > >>>+
> > > > > >>> /*
> > > > > >>>* TODO: Add all crtc level properties here
> > > > > >>>*/
> > > > > >>>@@ -2683,6 +2687,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe,
> > > > > >>>void *ptr, size_t length)
> > > > > >>>}
> > > > > >>>
> > > > > >>>/**
> > > > > >>>+ * igt_pipe_set_out_fence_ptr:
> > > > > >>>+ * @pipe: pipe pointer to which background color to be set
> > > > > >>>+ * @fence_ptr: out fence pointer
> > > > > >>
> > > > > >>I don't think fence_ptr can be int *. It needs to be a pointer to a
> > > > > >>64-bit type.
> > > > > >>
> > > > > >>>+ *
> > > > > >>>+ * Sets the out fence pointer that will be passed to the kernel in
> > > > > >>>+ * the atomic ioctl. When the kernel returns the out fence pointer
> > > > > >>>+ * will contain the fd number of the out fence created by KMS.
> > > > > >>>+ */
> > > > > >>>+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr)
> > > > > >>>+{
> > > > > >>>+ pipe->out_fence_ptr = (uint64_t) fence_ptr;
> > > > > >>>+}
> > > > > >>>+
> > > > > >>>+/**
> > > > > >>> * igt_crtc_set_background:
> > > > > >>> * @pipe: pipe pointer to which background color to be set
> > > > > >>> * @background: background color value in BGR 16bpc
> > > > > >>>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > > > >>>index 344f931..02d7bd1 100644
> > > > > >>>--- a/lib/igt_kms.h
> > > > > >>>+++ b/lib/igt_kms.h
> > > > > >>>@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
> > > > > >>> IGT_CRTC_GAMMA_LUT,
> > > > > >>> IGT_CRTC_MODE_ID,
> > > > > >>> IGT_CRTC_ACTIVE,
> > > > > >>>+ IGT_CRTC_OUT_FENCE_PTR,
> > > > > >>> IGT_NUM_CRTC_PROPS
> > > > > >>>};
> > > > > >>>
> > > > > >>>@@ -298,6 +299,7 @@ struct igt_pipe {
> > > > > >>>
> > > > > >>> uint64_t mode_blob;
> > > > > >>> bool mode_changed;
> > > > > >>>+ uint64_t out_fence_ptr;
> > > > > >>
> > > > > >>IMO this should be:
> > > > > >>
> > > > > >>int64_t *out_fence_ptr;
> > > > > >
> > > > > >In userspace, fences are *fd*, a plain int. It is only the uabi that
> > > > > >we
> > > > > >pass pointers as u64 to the kernel, and indeed that should be
> > > > > >limited to
> > > > > >the uabi wrapper.
> > > > > >-Chris
> > > > >
> > > > > Where's the uabi wrapper in this case?
> > > > >
> > > > > Wherever it is, afaik someone needs to have 64-bit type for the kernel
> > > > > to stash its fd in - on the kernel side out_fence_ptr is
> > > > > (s64 __user *), so if there's not a 64-bit variable on the other end
> > > > > of it then someone's going to have a bad day.
> > > >
> > > > We do not have pointers in the uabi because they are different sizes on
> > > > different platforms. The uabi must be a u64 re
[PATCH 09/37] drm/arm: Add local 'fb' variables
On Tue, Nov 22, 2016 at 03:48:50PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 21, 2016 at 11:51:21AM +, Liviu Dudau wrote:
> > On Fri, Nov 18, 2016 at 09:52:45PM +0200, ville.syrjala at linux.intel.com
> > wrote:
> > > From: Ville Syrjälä
> > >
> > > Add a local 'fb' variable to a few places to get rid of the
> > > 'crtc->primary->fb' stuff. Looks neater and helps me with my ppor
> > > coccinelle skills later.
> > >
> > > In some places the local variable was already there, just not used
> > > consistently.
> > >
> > > Cc: Liviu Dudau
> >
> > Acked-by: Liviu Dudau
> >
> > Are you going to take the series through drm-misc or you want each
> > sub-maintainer
> > to cherry pick the patches?
>
> Probably easier to suck it all in one go. Unless drm-misc maintainers
> disagree?
Seems like exactly the thing drm-misc is for.
-Daniel
>
> >
> > Best regards,
> > Liviu
> >
> > > Cc: Brian Starkey
> > > Cc: Mali DP Maintainers
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/arm/hdlcd_crtc.c| 18 ++
> > > drivers/gpu/drm/arm/malidp_planes.c | 6 +++---
> > > 2 files changed, 13 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c
> > > b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > > index bbaa55add2d2..8a0fee03aa39 100644
> > > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> > > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > > @@ -60,11 +60,12 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
> > > {
> > > unsigned int btpp;
> > > struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
> > > + const struct drm_framebuffer *fb = crtc->primary->state->fb;
> > > uint32_t pixel_format;
> > > struct simplefb_format *format = NULL;
> > > int i;
> > >
> > > - pixel_format = crtc->primary->state->fb->pixel_format;
> > > + pixel_format = fb->pixel_format;
> > >
> > > for (i = 0; i < ARRAY_SIZE(supported_formats); i++) {
> > > if (supported_formats[i].fourcc == pixel_format)
> > > @@ -221,27 +222,28 @@ static int hdlcd_plane_atomic_check(struct
> > > drm_plane *plane,
> > > static void hdlcd_plane_atomic_update(struct drm_plane *plane,
> > > struct drm_plane_state *state)
> > > {
> > > + struct drm_framebuffer *fb = plane->state->fb;
> > > struct hdlcd_drm_private *hdlcd;
> > > struct drm_gem_cma_object *gem;
> > > u32 src_w, src_h, dest_w, dest_h;
> > > dma_addr_t scanout_start;
> > >
> > > - if (!plane->state->fb)
> > > + if (!fb)
> > > return;
> > >
> > > src_w = plane->state->src_w >> 16;
> > > src_h = plane->state->src_h >> 16;
> > > dest_w = plane->state->crtc_w;
> > > dest_h = plane->state->crtc_h;
> > > - gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0);
> > > - scanout_start = gem->paddr + plane->state->fb->offsets[0] +
> > > - plane->state->crtc_y * plane->state->fb->pitches[0] +
> > > + gem = drm_fb_cma_get_gem_obj(fb, 0);
> > > + scanout_start = gem->paddr + fb->offsets[0] +
> > > + plane->state->crtc_y * fb->pitches[0] +
> > > plane->state->crtc_x *
> > > - drm_format_plane_cpp(plane->state->fb->pixel_format, 0);
> > > + drm_format_plane_cpp(fb->pixel_format, 0);
> > >
> > > hdlcd = plane->dev->dev_private;
> > > - hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH,
> > > plane->state->fb->pitches[0]);
> > > - hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_PITCH,
> > > plane->state->fb->pitches[0]);
> > > + hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, fb->pitches[0]);
> > > + hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_PITCH, fb->pitches[0]);
> > > hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_COUNT, dest_h - 1);
> > > hdlcd_write(hdlcd, HDLCD_REG_FB_BASE, scanout_start);
> > > }
> > > diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> > > b/drivers/gpu/drm/arm/malidp_planes.c
> > > index 63eec8f37cfc..ee7f7663a307 100644
> > > --- a/drivers/gpu/drm/arm/malidp_planes.c
> > > +++ b/drivers/gpu/drm/arm/malidp_planes.c
> > > @@ -137,8 +137,8 @@ static int malidp_de_plane_check(struct drm_plane
> > > *plane,
> > >
> > > /* packed RGB888 / BGR888 can't be rotated or flipped */
> > > if (state->rotation != DRM_ROTATE_0 &&
> > > - (state->fb->pixel_format == DRM_FORMAT_RGB888 ||
> > > - state->fb->pixel_format == DRM_FORMAT_BGR888))
> > > + (fb->pixel_format == DRM_FORMAT_RGB888 ||
> > > + fb->pixel_format == DRM_FORMAT_BGR888))
> > > return -EINVAL;
> > >
> > > ms->rotmem_size = 0;
> > > @@ -147,7 +147,7 @@ static int malidp_de_plane_check(struct drm_plane
> > > *plane,
> > >
> > > val = mp->hwdev->rotmem_required(mp->hwdev, state->crtc_h,
> > >state->crtc_w,
> > > - state->fb->pixel_format);
> > > + fb->pixel_format);
> > > if (val < 0)
> > > return val;
> > >
> > > --
> > > 2.7.4
> > >
> >
> > --
> > ===
[Intel-gfx] [PATCH v9 01/11] drm/i915: Add i915 perf infrastructure
On Tue, Nov 22, 2016 at 1:31 PM, Daniel Vetter wrote:
> On Tue, Nov 22, 2016 at 02:29:18PM +0100, Daniel Vetter wrote:
> > On Wed, Nov 09, 2016 at 08:00:06PM +, Matthew Auld wrote:
> > > On 7 November 2016 at 19:49, Robert Bragg
> wrote:
> > > > Adds base i915 perf infrastructure for Gen performance metrics.
> > > >
> > > > This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an array of
> uint64
> > > > properties to configure a stream of metrics and returns a new fd
> usable
> > > > with standard VFS system calls including read() to read typed and
> sized
> > > > records; ioctl() to enable or disable capture and poll() to wait for
> > > > data.
> > > >
> > > > A stream is opened something like:
> > > >
> > > > uint64_t properties[] = {
> > > > /* Single context sampling */
> > > > DRM_I915_PERF_PROP_CTX_HANDLE,ctx_handle,
> > > >
> > > > /* Include OA reports in samples */
> > > > DRM_I915_PERF_PROP_SAMPLE_OA, true,
> > > >
> > > > /* OA unit configuration */
> > > > DRM_I915_PERF_PROP_OA_METRICS_SET,metrics_set_id,
> > > > DRM_I915_PERF_PROP_OA_FORMAT, report_format,
> > > > DRM_I915_PERF_PROP_OA_EXPONENT, period_exponent,
> > > >};
> > > >struct drm_i915_perf_open_param parm = {
> > > > .flags = I915_PERF_FLAG_FD_CLOEXEC |
> > > >I915_PERF_FLAG_FD_NONBLOCK |
> > > >I915_PERF_FLAG_DISABLED,
> > > > .properties_ptr = (uint64_t)properties,
> > > > .num_properties = sizeof(properties) / 16,
> > > >};
> > > >int fd = drmIoctl(drm_fd, DRM_IOCTL_I915_PERF_OPEN, ¶m);
> > > >
> > > > Records read all start with a common { type, size } header with
> > > > DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample records
> > > > contain an extensible number of fields and it's the
> > > > DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening that
> > > > determine what's included in every sample.
> > > >
> > > > No specific streams are supported yet so any attempt to open a stream
> > > > will return an error.
> > > >
> > > > v2:
> > > > use i915_gem_context_get() - Chris Wilson
> > > > v3:
> > > > update read() interface to avoid passing state struct - Chris
> Wilson
> > > > fix some rebase fallout, with i915-perf init/deinit
> > > > v4:
> > > > s/DRM_IORW/DRM_IOW/ - Emil Velikov
> > > >
> > > > Signed-off-by: Robert Bragg
> > > > Reviewed-by: Matthew Auld
> > > > Reviewed-by: Sourab Gupta
> > > Minor nit, there are a fair few DRM_ERROR's missing a new line.
> >
> > Also, DRM_ERROR for userspace-triggerable failures is no good. igt
> > testcase are supposed to exercise all the invalid stuff, and would then
> > fail if you spam dmesg. Why was this not caught?
> >
> > Fixup patch totally fine, but if this wasn't caught due to missing igt
> > that needs to be fixed, too.
>
> Another nitpick for the future: Enabling new features first and then
> fixing up the fallout is the wrong way round, if someone bisects over this
> range mesa might blow up in really bad ways.
>
> Oh well, this has been out there for way too long, so meh.
>
Fwiw I'm aware of this, and think I've ordered the patches correctly to
avoid bisect problems in Mesa / userspace. This infrastructure patch should
have no fallout to fix for userspace. The command parser changes that
affect userspace were done before adding oacontrol usage to i915-perf and
the cmd parser's EINVAL reporting for access failures was changed *before*
removing oacontrol from the whitelist.
Did I overlook something in particular?
- Robert
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
-- next part --
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161122/a04d5653/attachment.html>
[Intel-gfx] [PATCH 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
On Tue, Nov 22, 2016 at 02:56:49PM +0100, Daniel Vetter wrote:
>On Tue, Nov 22, 2016 at 01:50:53PM +, Brian Starkey wrote:
>> On Tue, Nov 22, 2016 at 02:12:59PM +0100, Daniel Vetter wrote:
>> > On Tue, Nov 22, 2016 at 12:37:47PM +, Brian Starkey wrote:
>> > > On Tue, Nov 22, 2016 at 12:10:52PM +, Chris Wilson wrote:
>> > > > On Tue, Nov 22, 2016 at 11:54:57AM +, Brian Starkey wrote:
>> > > > > On Tue, Nov 22, 2016 at 11:06:00AM +, Chris Wilson wrote:
>> > > > > >On Tue, Nov 22, 2016 at 10:53:51AM +, Brian Starkey wrote:
>> > > > > >>Hi Gustavo,
>> > > > > >>
>> > > > > >>A little late to the party here, but I was blocked by our internal
>> > > > > >>contributions process...
>> > > > > >>
>> > > > > >>I didn't see these end up in my checkout yet though, so I guess
>> > > > > >>they
>> > > > > >>aren't picked up yet.
>> > > > > >>
>> > > > > >>On Mon, Nov 14, 2016 at 06:59:21PM +0900, Gustavo Padovan wrote:
>> > > > > >>>From: Gustavo Padovan
>> > > > > >>>
>> > > > > >>>Add support for the OUT_FENCE_PTR property to enable setting out
>> > > > > >>>fences for
>> > > > > >>>atomic commits.
>> > > > > >>>
>> > > > > >>>Signed-off-by: Gustavo Padovan > > > > > >>>collabora.co.uk>
>> > > > > >>>---
>> > > > > >>>lib/igt_kms.c | 20 +++-
>> > > > > >>>lib/igt_kms.h | 3 +++
>> > > > > >>>2 files changed, 22 insertions(+), 1 deletion(-)
>> > > > > >>>
>> > > > > >>>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> > > > > >>>index 4748c0a..f25e1eb 100644
>> > > > > >>>--- a/lib/igt_kms.c
>> > > > > >>>+++ b/lib/igt_kms.c
>> > > > > >>>@@ -175,7 +175,8 @@ const char
>> > > > > >>>*igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
>> > > > > >>> "DEGAMMA_LUT",
>> > > > > >>> "GAMMA_LUT",
>> > > > > >>> "MODE_ID",
>> > > > > >>>- "ACTIVE"
>> > > > > >>>+ "ACTIVE",
>> > > > > >>>+ "OUT_FENCE_PTR"
>> > > > > >>>};
>> > > > > >>>
>> > > > > >>>const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>> > > > > >>>@@ -2103,6 +2104,9 @@ static void
>> > > > > >>>igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj,
>> > > > > >>>drmModeAtomicRe
>> > > > > >>> igt_atomic_populate_crtc_req(req, pipe_obj,
>> > > > > >>> IGT_CRTC_ACTIVE, !!output);
>> > > > > >>> }
>> > > > > >>>
>> > > > > >>>+ if (pipe_obj->out_fence_ptr)
>> > > > > >>>+ igt_atomic_populate_crtc_req(req, pipe_obj,
>> > > > > >>>IGT_CRTC_OUT_FENCE_PTR, pipe_obj->out_fence_ptr);
>> > > > > >>>+
>> > > > > >>> /*
>> > > > > >>> * TODO: Add all crtc level properties here
>> > > > > >>> */
>> > > > > >>>@@ -2683,6 +2687,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe,
>> > > > > >>>void *ptr, size_t length)
>> > > > > >>>}
>> > > > > >>>
>> > > > > >>>/**
>> > > > > >>>+ * igt_pipe_set_out_fence_ptr:
>> > > > > >>>+ * @pipe: pipe pointer to which background color to be set
>> > > > > >>>+ * @fence_ptr: out fence pointer
>> > > > > >>
>> > > > > >>I don't think fence_ptr can be int *. It needs to be a pointer to a
>> > > > > >>64-bit type.
>> > > > > >>
>> > > > > >>>+ *
>> > > > > >>>+ * Sets the out fence pointer that will be passed to the kernel
>> > > > > >>>in
>> > > > > >>>+ * the atomic ioctl. When the kernel returns the out fence
>> > > > > >>>pointer
>> > > > > >>>+ * will contain the fd number of the out fence created by KMS.
>> > > > > >>>+ */
>> > > > > >>>+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int *fence_ptr)
>> > > > > >>>+{
>> > > > > >>>+ pipe->out_fence_ptr = (uint64_t) fence_ptr;
>> > > > > >>>+}
>> > > > > >>>+
>> > > > > >>>+/**
>> > > > > >>> * igt_crtc_set_background:
>> > > > > >>> * @pipe: pipe pointer to which background color to be set
>> > > > > >>> * @background: background color value in BGR 16bpc
>> > > > > >>>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> > > > > >>>index 344f931..02d7bd1 100644
>> > > > > >>>--- a/lib/igt_kms.h
>> > > > > >>>+++ b/lib/igt_kms.h
>> > > > > >>>@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
>> > > > > >>> IGT_CRTC_GAMMA_LUT,
>> > > > > >>> IGT_CRTC_MODE_ID,
>> > > > > >>> IGT_CRTC_ACTIVE,
>> > > > > >>>+ IGT_CRTC_OUT_FENCE_PTR,
>> > > > > >>> IGT_NUM_CRTC_PROPS
>> > > > > >>>};
>> > > > > >>>
>> > > > > >>>@@ -298,6 +299,7 @@ struct igt_pipe {
>> > > > > >>>
>> > > > > >>> uint64_t mode_blob;
>> > > > > >>> bool mode_changed;
>> > > > > >>>+ uint64_t out_fence_ptr;
>> > > > > >>
>> > > > > >>IMO this should be:
>> > > > > >>
>> > > > > >> int64_t *out_fence_ptr;
>> > > > > >
>> > > > > >In userspace, fences are *fd*, a plain int. It is only the uabi
>> > > > > >that we
>> > > > > >pass pointers as u64 to the kernel, and indeed that should be
>> > > > > >limited to
>> > > > > >the uabi wrapper.
>> > > > > >-Chris
>> > > > >
>> > > > > Where's the uabi wrapper in this case?
>> > > > >
>> > > > > Wherever it is, afaik someone needs to have 64-bit type for the
>> > > > > kernel
>> > > > > to stash its fd in - on the kernel side out_fence_ptr is
>> > > > > (s64 __user *), so i
[Intel-gfx] [PATCH v2] drm/i915: don't whitelist oacontrol in cmd parser
}
> > -
> > - if (desc->cmd.value ==
> MI_LOAD_REGISTER_REG) {
> > - DRM_DEBUG_DRIVER("CMD: Rejected
> LRR to OACONTROL\n");
> > - return false;
> > - }
> > -
> > - if (desc->cmd.value ==
> MI_LOAD_REGISTER_IMM(1))
> > - *oacontrol_set = (cmd[offset + 1]
> != 0);
> > - }
> > -
> > - /*
> >* Check the value written to the register against
> the
> >* allowed mask/value pair given in the whitelist
> entry.
> >*/
> > @@ -1214,7 +1187,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs
> *engine,
> > u32 *cmd, *batch_end;
> > struct drm_i915_cmd_descriptor default_desc = noop_desc;
> > const struct drm_i915_cmd_descriptor *desc = &default_desc;
> > - bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd()
> */
> > bool needs_clflush_after = false;
> > int ret = 0;
> >
> > @@ -1270,8 +1242,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs
> *engine,
> > break;
> > }
> >
> > - if (!check_cmd(engine, desc, cmd, length, is_master,
> > -&oacontrol_set)) {
> > + if (!check_cmd(engine, desc, cmd, length, is_master)) {
> > ret = -EACCES;
> > break;
> > }
> > @@ -1279,11 +1250,6 @@ int intel_engine_cmd_parser(struct
> intel_engine_cs *engine,
> > cmd += length;
> > }
> >
> > - if (oacontrol_set) {
> > - DRM_DEBUG_DRIVER("CMD: batch set OACONTROL but did not
> clear it\n");
> > - ret = -EINVAL;
> > - }
> > -
> > if (cmd >= batch_end) {
> > DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a
> BBE cmd!\n");
> > ret = -EINVAL;
> > @@ -1336,6 +1302,8 @@ int i915_cmd_parser_get_version(struct
> drm_i915_private *dev_priv)
> >* 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.
> > + * 9. Don't whitelist or handle oacontrol specially, as ownership
> > + *for oacontrol state is moving to i915-perf.
> >*/
> > - return 8;
> > + return 9;
> > }
> > --
> > 2.10.1
> >
> > ___
> > 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
>
-- next part --
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161122/c29c546c/attachment.html>
[PATCH libdrm] xf86drm: introduce drmGetDeviceNameFromFd2
On 16 November 2016 at 11:14, Nicolai Hähnle wrote: > On 10.11.2016 18:43, Emil Velikov wrote: >> >> From: Emil Velikov >> >> The original version considered only card devices, while this will pick >> the device/node name regardless - card, control, renderD, other... >> >> Current implementation is "linux" specific, in such that it relies on >> sysfs/uevent file. At the same time this gives us the flexibility to >> support any nodes even future ones, as long as they're within DRM_MAJOR. >> >> Shamelessly copied from mesa, latter by: Gary Wong >> Signed-off-by: Emil Velikov > > > Makes sense to me. > > Reviewed-by: Nicolai Hähnle > Great, ty. Next step - the revision (-less) patches. Since the kernel thread reached conclusion :-) -Emil
[PATCH libdrm] libdrm: random typo fixes
On 21 November 2016 at 19:55, Alex Deucher wrote: > On Sun, Nov 20, 2016 at 1:25 PM, Grazvydas Ignotas > wrote: >> Just some trivial boring typo fixes all over the tree. >> READMEs and comments only. >> >> Signed-off-by: Grazvydas Ignotas > > Reviewed-by: Alex Deucher > Thanks gents. Just pushed both patches to master. -Emil
[PATCH] vgaarb: use valid dev pointer in vgaarb_info()
We now pass the device to the debug messages, but on non-x86,
this is an invalid pointer in vga_arb_device_init:
drivers/gpu/vga/vgaarb.c: In function 'vga_arb_device_init':
drivers/gpu/vga/vgaarb.c:1467:4: error: 'dev' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
This moves the initialization of the dev pointer outside of the
architecture #ifdef.
Fixes: a75d68f62106 ("vgaarb: Use dev_printk() when possible")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/vga/vgaarb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index b3d27182edd9..0f5b2dd24507 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -1407,7 +1407,6 @@ static int __init vga_arb_device_init(void)
int rc;
struct pci_dev *pdev;
struct vga_device *vgadev;
- struct device *dev;
rc = misc_register(&vga_arb_device);
if (rc < 0)
@@ -1424,6 +1423,7 @@ static int __init vga_arb_device_init(void)
vga_arbiter_add_pci_device(pdev);
list_for_each_entry(vgadev, &vga_list, list) {
+ struct device *dev = &vgadev->pdev->dev;
#if defined(CONFIG_X86) || defined(CONFIG_IA64)
/*
* Override vga_arbiter_add_pci_device()'s I/O based detection
@@ -1438,7 +1438,6 @@ static int __init vga_arb_device_init(void)
int i;
limit = screen_info.lfb_base + screen_info.lfb_size;
- dev = &vgadev->pdev->dev;
/* Does firmware framebuffer belong to us? */
for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
--
2.9.0
[PATCH v4 0/4] drm/tilcdc: Add bridge support and sync-lost flood recovery
Changes since v3: - "drm/tilcdc: Enable sync lost error and recovery handling for rev 1 LCDC" - Fix broken irq enable/disble code for LCDC rev 1 - Add: "dt-bindings: Move "ti,tfp410.txt" from display/ti to display/bridge" - "drm/bridge: Add ti-tfp410 DVI transmitter driver" - Don't fail if either i2c or platform driver register succeeds - ftp410 -> tfp410 - Merge the old display/ti,tfp410.txt document with my addition Changes since v2: - "drm/tilcdc: Recover from sync lost error flood by resetting the LCDC" - no change - "drm/bridge: Add ti-tfp410 DVI transmitter driver" - Fix deveice-tree document - "driver node" -> "device node" - remove "(the current implementation does not yet support this)" - Add dummy i2c support. The driver probe works also if placed under i2c controller node, but there is no actual i2c probing. - "drm/tilcdc: Add drm bridge support for attaching drm bridge drivers" - no change Changes since first version of the series: - "drm/tilcdc: Recover from sync lost error flood by resetting the LCDC" - no change - "drm/bridge: Add ti-tfp410 DVI transmitter driver" - HDMI -> DVI - DT Binding document - Prepare for tfp410 connected trough i2c by optional reg property - Require two port nodes - Implementation - Implement connector node functionality with in tfp410 bridge drive, but follow generic connector binding by pulling the ddc-i2c-bus property from the connector node. - "drm/tilcdc: Add drm bridge support for attaching drm bridge drivers" - Remove earlier change in TD binding document. There is no need to mention DRM implementation details, like bridge support, in DT binding. The first patch is an independent on and I've been testing it for quite a while now. The tfp410 bridge driver and the tilcdc bridge support are tested with BeagleBone DVI-D Cape Rev A3. The tfp410 bridge driver is missing a lot of features, because the DVI-D cape does not have too many wires connected. The missing features can be added later when they are needed. Jyri Sarha (4): drm/tilcdc: Recover from sync lost error flood by resetting the LCDC dt-bindings: Move "ti,tfp410.txt" from display/ti to display/bridge drm/bridge: Add ti-tfp410 DVI transmitter driver drm/tilcdc: Add drm bridge support for attaching drm bridge drivers .../bindings/display/bridge/ti,tfp410.txt | 46 +++ .../devicetree/bindings/display/ti/ti,tfp410.txt | 41 --- drivers/gpu/drm/bridge/Kconfig | 7 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/ti-tfp410.c | 311 + drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 26 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c| 7 +- drivers/gpu/drm/tilcdc/tilcdc_drv.h| 2 + drivers/gpu/drm/tilcdc/tilcdc_external.c | 140 -- drivers/gpu/drm/tilcdc/tilcdc_external.h | 1 + 10 files changed, 521 insertions(+), 61 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt delete mode 100644 Documentation/devicetree/bindings/display/ti/ti,tfp410.txt create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c -- 1.9.1
[PATCH v4 2/4] dt-bindings: Move "ti, tfp410.txt" from display/ti to display/bridge
Move "ti,tfp410.txt" from display/ti to display/bridge before adding
generic (non omapdrm/dss specific) implementation and new features.
Signed-off-by: Jyri Sarha
---
.../bindings/display/bridge/ti,tfp410.txt | 41 ++
.../devicetree/bindings/display/ti/ti,tfp410.txt | 41 --
2 files changed, 41 insertions(+), 41 deletions(-)
create mode 100644
Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
delete mode 100644 Documentation/devicetree/bindings/display/ti/ti,tfp410.txt
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
new file mode 100644
index 000..2cbe32a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
@@ -0,0 +1,41 @@
+TFP410 DPI to DVI encoder
+=
+
+Required properties:
+- compatible: "ti,tfp410"
+
+Optional properties:
+- powerdown-gpios: power-down gpio
+
+Required nodes:
+- Video port 0 for DPI input
+- Video port 1 for DVI output
+
+Example
+---
+
+tfp410: encoder at 0 {
+ compatible = "ti,tfp410";
+ powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port at 0 {
+ reg = <0>;
+
+ tfp410_in: endpoint at 0 {
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+
+ port at 1 {
+ reg = <1>;
+
+ tfp410_out: endpoint at 0 {
+ remote-endpoint = <&dvi_connector_in>;
+ };
+ };
+ };
+};
diff --git a/Documentation/devicetree/bindings/display/ti/ti,tfp410.txt
b/Documentation/devicetree/bindings/display/ti/ti,tfp410.txt
deleted file mode 100644
index 2cbe32a..000
--- a/Documentation/devicetree/bindings/display/ti/ti,tfp410.txt
+++ /dev/null
@@ -1,41 +0,0 @@
-TFP410 DPI to DVI encoder
-=
-
-Required properties:
-- compatible: "ti,tfp410"
-
-Optional properties:
-- powerdown-gpios: power-down gpio
-
-Required nodes:
-- Video port 0 for DPI input
-- Video port 1 for DVI output
-
-Example
-
-tfp410: encoder at 0 {
- compatible = "ti,tfp410";
- powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>;
-
- ports {
- #address-cells = <1>;
- #size-cells = <0>;
-
- port at 0 {
- reg = <0>;
-
- tfp410_in: endpoint at 0 {
- remote-endpoint = <&dpi_out>;
- };
- };
-
- port at 1 {
- reg = <1>;
-
- tfp410_out: endpoint at 0 {
- remote-endpoint = <&dvi_connector_in>;
- };
- };
- };
-};
--
1.9.1
[PATCH v4 4/4] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers
Adds drm bride support for attaching drm bridge drivers to tilcdc. The
decision whether a video port leads to an external encoder or bridge
is made simply based on remote device's compatible string. The code
has been tested with BeagleBone-Black with and without BeagleBone
DVI-D Cape Rev A3 using ti-tfp410 driver.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 7 +-
drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 +
drivers/gpu/drm/tilcdc/tilcdc_external.c | 140 +++
drivers/gpu/drm/tilcdc/tilcdc_external.h | 1 +
4 files changed, 131 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 3d2cea0..af959df 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -384,9 +384,14 @@ static int tilcdc_init(struct drm_driver *ddrv, struct
device *dev)
ret = tilcdc_add_external_encoders(ddev);
if (ret < 0)
goto init_failed;
+ } else {
+ ret = tilcdc_attach_remote_device(ddev);
+ if (ret)
+ goto init_failed;
}
- if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) {
+ if (!priv->remote_encoder &&
+ ((priv->num_encoders == 0) || (priv->num_connectors == 0))) {
dev_err(dev, "no encoders/connectors found\n");
ret = -ENXIO;
goto init_failed;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index d31fe5d..283ff28 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -90,6 +90,8 @@ struct tilcdc_drm_private {
struct drm_connector *connectors[8];
const struct drm_connector_helper_funcs *connector_funcs[8];
+ struct drm_encoder *remote_encoder;
+
bool is_registered;
bool is_componentized;
};
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c
b/drivers/gpu/drm/tilcdc/tilcdc_external.c
index 06a4c58..e1576ba 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
@@ -28,6 +28,18 @@
.raster_order = 0,
};
+static const struct tilcdc_panel_info panel_info_default = {
+ .ac_bias= 255,
+ .ac_bias_intrpt = 0,
+ .dma_burst_sz = 16,
+ .bpp= 16,
+ .fdd= 0x80,
+ .tft_alt_mode = 0,
+ .sync_edge = 0,
+ .sync_ctrl = 1,
+ .raster_order = 0,
+};
+
static int tilcdc_external_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
{
@@ -130,6 +142,101 @@ void tilcdc_remove_external_encoders(struct drm_device
*dev)
priv->connector_funcs[i]);
}
+static const struct drm_encoder_funcs tilcdc_remote_encoder_funcs = {
+ .destroy= drm_encoder_cleanup,
+};
+
+static
+int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
+{
+ struct tilcdc_drm_private *priv = ddev->dev_private;
+ int ret;
+
+ priv->remote_encoder->possible_crtcs = BIT(0);
+ priv->remote_encoder->bridge = bridge;
+ bridge->encoder = priv->remote_encoder;
+
+ ret = drm_bridge_attach(ddev, bridge);
+ if (ret) {
+ dev_err(ddev->dev, "drm_bridge_attach() failed %d\n", ret);
+ return ret;
+ }
+
+ tilcdc_crtc_set_panel_info(priv->crtc, &panel_info_default);
+
+ return 0;
+}
+
+static int tilcdc_node_has_port(struct device_node *dev_node)
+{
+ struct device_node *node;
+
+ node = of_get_child_by_name(dev_node, "ports");
+ if (!node)
+ node = of_get_child_by_name(dev_node, "port");
+ if (!node)
+ return 0;
+ of_node_put(node);
+
+ return 1;
+}
+
+static
+struct device_node *tilcdc_get_remote_node(struct device_node *node)
+{
+ struct device_node *ep;
+ struct device_node *parent;
+
+ if (!tilcdc_node_has_port(node))
+ return NULL;
+
+ ep = of_graph_get_next_endpoint(node, NULL);
+ if (!ep)
+ return NULL;
+
+ parent = of_graph_get_remote_port_parent(ep);
+ of_node_put(ep);
+
+ return parent;
+}
+
+int tilcdc_attach_remote_device(struct drm_device *ddev)
+{
+ struct tilcdc_drm_private *priv = ddev->dev_private;
+ struct device_node *remote_node;
+ struct drm_bridge *bridge;
+ int ret;
+
+ remote_node = tilcdc_get_remote_node(ddev->dev->of_node);
+ if (!remote_node)
+ return 0;
+
+ bridge = of_drm_find_bridge(remote_node);
+ of_node_put(remote_node);
+ if (!bridge
[PATCH v4 1/4] drm/tilcdc: Recover from sync lost error flood by resetting the LCDC
Recover from sync lost error flood by resetting the LCDC instead of
turning off the SYNC_LOST error IRQ. When LCDC starves on limited
memory bandwidth it may sometimes result an error situation when the
picture may have shifted couple of pixels to right and SYNC_LOST
interrupt is generated on every frame. LCDC main reset recovers from
this situation and causes a brief blanking on the screen.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 0d09acc..c787349 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -55,6 +55,7 @@ struct tilcdc_crtc {
int sync_lost_count;
bool frame_intact;
+ struct work_struct recover_work;
};
#define to_tilcdc_crtc(x) container_of(x, struct tilcdc_crtc, base)
@@ -252,6 +253,25 @@ static bool tilcdc_crtc_is_on(struct drm_crtc *crtc)
return crtc->state && crtc->state->enable && crtc->state->active;
}
+static void tilcdc_crtc_recover_work(struct work_struct *work)
+{
+ struct tilcdc_crtc *tilcdc_crtc =
+ container_of(work, struct tilcdc_crtc, recover_work);
+ struct drm_crtc *crtc = &tilcdc_crtc->base;
+
+ dev_info(crtc->dev->dev, "%s: Reset CRTC", __func__);
+
+ drm_modeset_lock_crtc(crtc, NULL);
+
+ if (!tilcdc_crtc_is_on(crtc))
+ goto out;
+
+ tilcdc_crtc_disable(crtc);
+ tilcdc_crtc_enable(crtc);
+out:
+ drm_modeset_unlock_crtc(crtc);
+}
+
static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
{
struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
@@ -838,9 +858,12 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
tilcdc_crtc->frame_intact = false;
if (tilcdc_crtc->sync_lost_count++ >
SYNC_LOST_COUNT_LIMIT) {
- dev_err(dev->dev, "%s(0x%08x): Sync lost flood
detected, disabling the interrupt", __func__, stat);
+ dev_err(dev->dev, "%s(0x%08x): Sync lost flood
detected, recovering", __func__, stat);
+ queue_work(system_wq,
+ &tilcdc_crtc->recover_work);
tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
LCDC_SYNC_LOST);
+ tilcdc_crtc->sync_lost_count = 0;
}
}
@@ -880,6 +903,7 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
"unref", unref_worker);
spin_lock_init(&tilcdc_crtc->irq_lock);
+ INIT_WORK(&tilcdc_crtc->recover_work, tilcdc_crtc_recover_work);
ret = drm_crtc_init_with_planes(dev, crtc,
&tilcdc_crtc->primary,
--
1.9.1
[PATCH v4 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver
Add very basic ti-tfp410 DVI transmitter driver. The only feature
separating this from a completely dummy bridge is the EDID read
support trough DDC I2C. Even that functionality should be in a
separate generic connector driver. However, because of missing DRM
infrastructure support the connector is implemented within the bridge
driver. Some tfp410 HW specific features may be added later if needed,
because there is a set of registers behind i2c if it is connected.
This implementation is tested against my new tilcdc bridge support
and it works with BeagleBone DVI-D Cape Rev A3. A DT binding document
is also updated.
Signed-off-by: Jyri Sarha
---
.../bindings/display/bridge/ti,tfp410.txt | 9 +-
drivers/gpu/drm/bridge/Kconfig | 7 +
drivers/gpu/drm/bridge/Makefile| 1 +
drivers/gpu/drm/bridge/ti-tfp410.c | 311 +
4 files changed, 326 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
index 2cbe32a..54d7e31 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
@@ -6,10 +6,15 @@ Required properties:
Optional properties:
- powerdown-gpios: power-down gpio
+- reg: I2C address. If and only if present the device node
+ should be placed into the i2c controller node where the
+ tfp410 i2c is connected to.
Required nodes:
-- Video port 0 for DPI input
-- Video port 1 for DVI output
+- Video port 0 for DPI input [1].
+- Video port 1 for DVI output [1].
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
Example
---
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index bd6acc8..a424e03 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -81,6 +81,13 @@ config DRM_TOSHIBA_TC358767
---help---
Toshiba TC358767 eDP bridge chip driver.
+config DRM_TI_TFP410
+ tristate "TI TFP410 DVI/HDMI bridge"
+ depends on OF
+ select DRM_KMS_HELPER
+ ---help---
+ Texas Instruments TFP410 DVI/HDMI Transmitter driver
+
source "drivers/gpu/drm/bridge/analogix/Kconfig"
source "drivers/gpu/drm/bridge/adv7511/Kconfig"
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 97ed1a5..8b065d9 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_DRM_SII902X) += sii902x.o
obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
+obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c
b/drivers/gpu/drm/bridge/ti-tfp410.c
new file mode 100644
index 000..58e26cc
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -0,0 +1,311 @@
+/*
+ * Copyright (C) 2016 Texas Instruments
+ * Author: Jyri Sarha
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+struct tfp410 {
+ struct drm_bridge bridge;
+ struct drm_connectorconnector;
+
+ struct i2c_adapter *ddc;
+
+ struct device *dev;
+};
+
+static inline struct tfp410 *
+drm_bridge_to_tfp410(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct tfp410, bridge);
+}
+
+static inline struct tfp410 *
+drm_connector_to_tfp410(struct drm_connector *connector)
+{
+ return container_of(connector, struct tfp410, connector);
+}
+
+static int tfp410_get_modes(struct drm_connector *connector)
+{
+ struct tfp410 *dvi = drm_connector_to_tfp410(connector);
+ struct edid *edid;
+ int ret;
+
+ if (!dvi->ddc)
+ goto fallback;
+
+ edid = drm_get_edid(connector, dvi->ddc);
+ if (!edid) {
+ DRM_INFO("EDID read failed. Fallback to standard modes\n");
+ goto fallback;
+ }
+
+ drm_mode_connector_update_edid_property(connector, edid);
+
+ return drm_add_edid_modes(connector, edid);
+fallback:
+ /* No EDID, fallback on the XGA standard modes */
+ ret = drm_add_modes_noedid(connector, 1920, 1200);
+
+ /* And prefer a mode pretty much anything can handle */
+ drm_set_preferred_mode(connector, 1024, 768);
+
+ return ret;
+}
+
+static const struct drm_connector_helper_funcs tfp410_con_helper_funcs = {
+ .get_modes = tfp410_get_modes,
+};
+
+static enum drm_connector_status
+tfp410_connector_detect(struct drm_connector *connector, bool force)
+{
+ struct tfp410 *dvi = drm_conne
[PATCH] drm/radeon: Ensure vblank interrupt is enabled on DPMS transition to on
On Tue, Nov 22, 2016 at 3:35 AM, Michel Dänzer wrote: > From: Michel Dänzer > > Fixes the vblank interrupt being disabled when it should be on, which > can cause at least the following symptoms: > > * Hangs when running 'xset dpms force off' in a GNOME session with > gnome-shell using DRI2. > * RandR 1.4 slave outputs freezing with garbage displayed using > xf86-video-ati 7.8.0 or newer. > > NOTE: This patch only applies to 4.5.y or older kernels. With newer > kernels, this problem cannot happen because the driver now uses > drm_crtc_vblank_on/off instead of drm_vblank_pre/post_modeset. I > consider this patch safer for older kernels than backporting the API > change, because drm_crtc_vblank_on/off had various issues in older > kernels, and I'm not sure all fixes for those have been backported to > all stable branches where this patch could be applied. > > Reported-and-Tested-by: Max Staudt > Signed-off-by: Michel Dänzer Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/radeon/atombios_crtc.c | 2 ++ > drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c > b/drivers/gpu/drm/radeon/atombios_crtc.c > index dac78ad..115b4a4 100644 > --- a/drivers/gpu/drm/radeon/atombios_crtc.c > +++ b/drivers/gpu/drm/radeon/atombios_crtc.c > @@ -275,6 +275,8 @@ void atombios_crtc_dpms(struct drm_crtc *crtc, int mode) > atombios_enable_crtc_memreq(crtc, ATOM_ENABLE); > atombios_blank_crtc(crtc, ATOM_DISABLE); > drm_vblank_post_modeset(dev, radeon_crtc->crtc_id); > + /* Make sure vblank interrupt is still enabled if needed */ > + radeon_irq_set(rdev); > radeon_crtc_load_lut(crtc); > break; > case DRM_MODE_DPMS_STANDBY: > diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c > b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c > index 678b438..89f22bd 100644 > --- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c > +++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c > @@ -331,6 +331,8 @@ static void radeon_crtc_dpms(struct drm_crtc *crtc, int > mode) > WREG32_P(RADEON_CRTC_EXT_CNTL, crtc_ext_cntl, ~(mask > | crtc_ext_cntl)); > } > drm_vblank_post_modeset(dev, radeon_crtc->crtc_id); > + /* Make sure vblank interrupt is still enabled if needed */ > + radeon_irq_set(rdev); > radeon_crtc_load_lut(crtc); > break; > case DRM_MODE_DPMS_STANDBY: > -- > 2.10.2 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm-intel:drm-intel-next-queued 6/11] undefined reference to `__udivdi3'
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued head: 7abbd8d670bb928366aa94332a173aa3d394ebfe commit: d79651522e89c4ffa8992b48dfe449f0c583f809 [6/11] drm/i915: Enable i915 perf stream for Haswell OA unit config: i386-defconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: git checkout d79651522e89c4ffa8992b48dfe449f0c583f809 # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/built-in.o: In function `i915_perf_open_ioctl': >> (.text+0x19a6f8): undefined reference to `__udivdi3' drivers/built-in.o: In function `i915_perf_open_ioctl': (.text+0x19a714): undefined reference to `__udivdi3' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- next part -- A non-text attachment was scrubbed... Name: .config.gz Type: application/gzip Size: 25159 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161122/7b21c6e5/attachment-0001.gz>
[Intel-gfx] [PATCH v9 01/11] drm/i915: Add i915 perf infrastructure
On Tue, Nov 22, 2016 at 02:02:38PM +, Robert Bragg wrote:
> On Tue, Nov 22, 2016 at 1:31 PM, Daniel Vetter wrote:
>
> > On Tue, Nov 22, 2016 at 02:29:18PM +0100, Daniel Vetter wrote:
> > > On Wed, Nov 09, 2016 at 08:00:06PM +, Matthew Auld wrote:
> > > > On 7 November 2016 at 19:49, Robert Bragg
> > wrote:
> > > > > Adds base i915 perf infrastructure for Gen performance metrics.
> > > > >
> > > > > This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an array of
> > uint64
> > > > > properties to configure a stream of metrics and returns a new fd
> > usable
> > > > > with standard VFS system calls including read() to read typed and
> > sized
> > > > > records; ioctl() to enable or disable capture and poll() to wait for
> > > > > data.
> > > > >
> > > > > A stream is opened something like:
> > > > >
> > > > > uint64_t properties[] = {
> > > > > /* Single context sampling */
> > > > > DRM_I915_PERF_PROP_CTX_HANDLE,ctx_handle,
> > > > >
> > > > > /* Include OA reports in samples */
> > > > > DRM_I915_PERF_PROP_SAMPLE_OA, true,
> > > > >
> > > > > /* OA unit configuration */
> > > > > DRM_I915_PERF_PROP_OA_METRICS_SET,metrics_set_id,
> > > > > DRM_I915_PERF_PROP_OA_FORMAT, report_format,
> > > > > DRM_I915_PERF_PROP_OA_EXPONENT, period_exponent,
> > > > >};
> > > > >struct drm_i915_perf_open_param parm = {
> > > > > .flags = I915_PERF_FLAG_FD_CLOEXEC |
> > > > >I915_PERF_FLAG_FD_NONBLOCK |
> > > > >I915_PERF_FLAG_DISABLED,
> > > > > .properties_ptr = (uint64_t)properties,
> > > > > .num_properties = sizeof(properties) / 16,
> > > > >};
> > > > >int fd = drmIoctl(drm_fd, DRM_IOCTL_I915_PERF_OPEN, ¶m);
> > > > >
> > > > > Records read all start with a common { type, size } header with
> > > > > DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample records
> > > > > contain an extensible number of fields and it's the
> > > > > DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening that
> > > > > determine what's included in every sample.
> > > > >
> > > > > No specific streams are supported yet so any attempt to open a stream
> > > > > will return an error.
> > > > >
> > > > > v2:
> > > > > use i915_gem_context_get() - Chris Wilson
> > > > > v3:
> > > > > update read() interface to avoid passing state struct - Chris
> > Wilson
> > > > > fix some rebase fallout, with i915-perf init/deinit
> > > > > v4:
> > > > > s/DRM_IORW/DRM_IOW/ - Emil Velikov
> > > > >
> > > > > Signed-off-by: Robert Bragg
> > > > > Reviewed-by: Matthew Auld
> > > > > Reviewed-by: Sourab Gupta
> > > > Minor nit, there are a fair few DRM_ERROR's missing a new line.
> > >
> > > Also, DRM_ERROR for userspace-triggerable failures is no good. igt
> > > testcase are supposed to exercise all the invalid stuff, and would then
> > > fail if you spam dmesg. Why was this not caught?
> > >
> > > Fixup patch totally fine, but if this wasn't caught due to missing igt
> > > that needs to be fixed, too.
> >
> > Another nitpick for the future: Enabling new features first and then
> > fixing up the fallout is the wrong way round, if someone bisects over this
> > range mesa might blow up in really bad ways.
> >
> > Oh well, this has been out there for way too long, so meh.
> >
>
> Fwiw I'm aware of this, and think I've ordered the patches correctly to
> avoid bisect problems in Mesa / userspace. This infrastructure patch should
> have no fallout to fix for userspace. The command parser changes that
> affect userspace were done before adding oacontrol usage to i915-perf and
> the cmd parser's EINVAL reporting for access failures was changed *before*
> removing oacontrol from the whitelist.
>
> Did I overlook something in particular?
Ah, if you key the userspace side off the cmd parser change then I think
it should be all fine. I only looked at this ioctl here, and that alone
looked like it was unsafe. Ordering of the patches later on seemed correct
indeed.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH v2] drm/i915: don't whitelist oacontrol in cmd parser
On Tue, Nov 22, 2016 at 02:09:08PM +, Robert Bragg wrote:
> On Tue, Nov 22, 2016 at 1:34 PM, Daniel Vetter wrote:
>
> > On Tue, Nov 08, 2016 at 12:51:48PM +, Robert Bragg wrote:
> > > This v2 patch bumps the command parser version so it can be referenced in
> > > corresponding i-g-t gem_exec_parse changes.
> > >
> > > --- >8 ---
> >
> > Scissors cut everything below, not everything above, hence next time
> > around pls switch around your comment and the commit message, as-is not
> > much left ;-)
> >
>
> Hmm, they cut away what's above and keep what's below in my experience -
> what command are you seeing the opposite with?
>
> I just double checked this with git am --scissors
Plain git am works the other way round, that's why I never noticed there's
a special option. TIL ;-)
-Daniel
>
> - Robert
>
>
>
> >
> > Fixed up while applying.
> > -Daniel
> >
> > >
> > > Being able to program OACONTROL from a non-privileged batch buffer is
> > > not sufficient to be able to configure the OA unit. This was originally
> > > allowed to help enable Mesa to expose OA counters via the
> > > INTEL_performance_query extension, but the current implementation based
> > > on programming OACONTROL via a batch buffer isn't able to report useable
> > > data without a more complete OA unit configuration. Mesa handles the
> > > possibility that writes to OACONTROL may not be allowed and so only
> > > advertises the extension after explicitly testing that a write to
> > > OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
> > > should be ok for userspace.
> > >
> > > Removing this simplifies adding a new kernel api for configuring the OA
> > > unit without needing to consider the possibility that userspace might
> > > trample on OACONTROL state which we'd like to start managing within
> > > the kernel instead. In particular running any Mesa based GL application
> > > currently results in clearing OACONTROL when initializing which would
> > > disable the capturing of metrics.
> > >
> > > v2:
> > > This bumps the command parser version from 8 to 9, as the change is
> > > visible to userspace.
> > >
> > > Signed-off-by: Robert Bragg
> > > Reviewed-by: Matthew Auld
> > > Reviewed-by: Sourab Gupta
> > > ---
> > > drivers/gpu/drm/i915/i915_cmd_parser.c | 42
> > --
> > > 1 file changed, 5 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > > index c9d2ecd..f5762cd 100644
> > > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > > @@ -450,7 +450,6 @@ static const struct drm_i915_reg_descriptor
> > gen7_render_regs[] = {
> > > REG64(PS_INVOCATION_COUNT),
> > > REG64(PS_DEPTH_COUNT),
> > > REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
> > > - REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below.
> > */
> > > REG64(MI_PREDICATE_SRC0),
> > > REG64(MI_PREDICATE_SRC1),
> > > REG32(GEN7_3DPRIM_END_OFFSET),
> > > @@ -1060,8 +1059,7 @@ bool intel_engine_needs_cmd_parser(struct
> > intel_engine_cs *engine)
> > > static bool check_cmd(const struct intel_engine_cs *engine,
> > > const struct drm_i915_cmd_descriptor *desc,
> > > const u32 *cmd, u32 length,
> > > - const bool is_master,
> > > - bool *oacontrol_set)
> > > + const bool is_master)
> > > {
> > > if (desc->flags & CMD_DESC_SKIP)
> > > return true;
> > > @@ -1099,31 +1097,6 @@ static bool check_cmd(const struct
> > intel_engine_cs *engine,
> > > }
> > >
> > > /*
> > > - * OACONTROL requires some special handling for
> > > - * writes. We want to make sure that any batch
> > which
> > > - * enables OA also disables it before the end of
> > the
> > > - * batch. The goal is to prevent one process from
> > > - * snooping on the perf data from another process.
> > To do
> > > - * that, we need to check the value that will be
> > written
> > > - * to the register. Hence, limit OACONTROL writes
> > to
> > > - * only MI_LOAD_REGISTER_IMM commands.
> > > - */
> > > - if (reg_addr ==
> > > i915_mmio_reg_offset(GEN7_OACONTROL))
> > {
> > > - if (desc->cmd.value ==
> > MI_LOAD_REGISTER_MEM) {
> > > - DRM_DEBUG_DRIVER("CMD: Rejected
> > LRM to OACONTROL\n");
> > > - return false;
> > > - }
> > > -
> > > - if (desc->cmd.value ==
> > MI_LOAD_REGISTER_REG) {
> > > - DRM_DEBUG_DRIVER("CM
[PATCH] drm/sun4i: Only count TCON endpoints as valid outputs
Hi, On Fri, Nov 18, 2016 at 10:22:40AM +0800, Chen-Yu Tsai wrote: > On Fri, Nov 18, 2016 at 3:02 AM, Maxime Ripard > wrote: > > On Wed, Nov 16, 2016 at 05:37:31PM +0800, Chen-Yu Tsai wrote: > >> The sun4i DRM driver counts the number of endpoints it found and > >> registers the whole DRM pipeline if any endpoints are found. > >> > >> However, if the TCON and its child endpoints (LCD panels, TV encoder, > >> HDMI encoder, MIPI DSI encoder, etc.) aren't found, that means we > >> don't have any usable CRTCs, and the display pipeline is incomplete > >> and useless. > > > > If some node set as available is not probed, then yes, it does, but > > I'm not really sure how it's a problem. Quite the opposite actually. > > Actually the problem occurs when the TCON is _not_ available, but > the other endpoints preceding it are. By preceding, you mean the display engine or the HDMI or TV encoders? > >> The debug message "Queued %d outputs on pipeline %d\n" is also telling. > >> > >> This patch makes the driver only count enabled TCON endpoints. If > >> none are found, the DRM pipeline is not used. This avoids screwing > >> up the simple framebuffer provided by the bootloader in cases where > >> we aren't able to support the display with the DRM subsystem, due > >> to lack of panel or bridge drivers, or just lack of progress. > > > > The framebuffer is removed only at bind time, which means that all the > > drivers have probed already. Lack of progress isn't an issue here, > > since the node simply won't be there, and we wouldn't have it in the > > component lists. And lack of drivers shouldn't be an issue either, > > since in order for bind to be called, all the drivers would have > > gone through their probe. > > > > So I'm not really sure what it fixes. > > To recap, on sun6i I had enabled the display engine node by default > in the dtsi, along with the backend and drc. The tcon is disabled > by default, so it doesn't get added to the list of components. > The available components get probed, binded, and simplefb gets > pushed out. > > I suppose disabling the display engine by default would be better? > At least simplefb still works. Yep, that works for me. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161122/65608cd9/attachment.sig>
[PATCH] vgaarb: use valid dev pointer in vgaarb_info()
On Tue, Nov 22, 2016 at 03:34:19PM +0100, Arnd Bergmann wrote:
> We now pass the device to the debug messages, but on non-x86,
> this is an invalid pointer in vga_arb_device_init:
>
> drivers/gpu/vga/vgaarb.c: In function 'vga_arb_device_init':
> drivers/gpu/vga/vgaarb.c:1467:4: error: 'dev' may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
>
> This moves the initialization of the dev pointer outside of the
> architecture #ifdef.
>
> Fixes: a75d68f62106 ("vgaarb: Use dev_printk() when possible")
> Signed-off-by: Arnd Bergmann
Applied, thx.
-Daniel
> ---
> drivers/gpu/vga/vgaarb.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index b3d27182edd9..0f5b2dd24507 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -1407,7 +1407,6 @@ static int __init vga_arb_device_init(void)
> int rc;
> struct pci_dev *pdev;
> struct vga_device *vgadev;
> - struct device *dev;
>
> rc = misc_register(&vga_arb_device);
> if (rc < 0)
> @@ -1424,6 +1423,7 @@ static int __init vga_arb_device_init(void)
> vga_arbiter_add_pci_device(pdev);
>
> list_for_each_entry(vgadev, &vga_list, list) {
> + struct device *dev = &vgadev->pdev->dev;
> #if defined(CONFIG_X86) || defined(CONFIG_IA64)
> /*
>* Override vga_arbiter_add_pci_device()'s I/O based detection
> @@ -1438,7 +1438,6 @@ static int __init vga_arb_device_init(void)
> int i;
>
> limit = screen_info.lfb_base + screen_info.lfb_size;
> - dev = &vgadev->pdev->dev;
>
> /* Does firmware framebuffer belong to us? */
> for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> --
> 2.9.0
>
> ___
> 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 0/3] drm/msm cleanups
Here are a few foundational patches to clean up some of the MSM code and get ready for new and awesome things down the line. Jordan Crouse (3): drm/msm: gpu: Cut down the list of "generic" registers to the ones we use drm/msm: gpu: Return error on hw_init failure drm/msm: gpu Add new gpu register read/write functions
[PATCH 1/3] drm/msm: gpu: Cut down the list of "generic" registers to the ones we use
There are very few register accesses in the common code. Cut down
the list of common registers to just those that are used. This
saves const space and saves us the effort of maintaining registers
for A3XX and A4XX that don't exist or are unused.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 80 -
drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 76 ---
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 59
3 files changed, 215 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index fd266ed..5eda847 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -419,91 +419,11 @@ static void a3xx_dump(struct msm_gpu *gpu)
}
/* Register offset defines for A3XX */
static const unsigned int a3xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
- REG_ADRENO_DEFINE(REG_ADRENO_CP_DEBUG, REG_AXXX_CP_DEBUG),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_ME_RAM_WADDR, REG_AXXX_CP_ME_RAM_WADDR),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_ME_RAM_DATA, REG_AXXX_CP_ME_RAM_DATA),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_PFP_UCODE_DATA,
- REG_A3XX_CP_PFP_UCODE_DATA),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_PFP_UCODE_ADDR,
- REG_A3XX_CP_PFP_UCODE_ADDR),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_WFI_PEND_CTR, REG_A3XX_CP_WFI_PEND_CTR),
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_AXXX_CP_RB_BASE),
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_RPTR_ADDR, REG_AXXX_CP_RB_RPTR_ADDR),
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_RPTR, REG_AXXX_CP_RB_RPTR),
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_WPTR, REG_AXXX_CP_RB_WPTR),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_PROTECT_CTRL, REG_A3XX_CP_PROTECT_CTRL),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_ME_CNTL, REG_AXXX_CP_ME_CNTL),
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_CNTL, REG_AXXX_CP_RB_CNTL),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_IB1_BASE, REG_AXXX_CP_IB1_BASE),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_IB1_BUFSZ, REG_AXXX_CP_IB1_BUFSZ),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_IB2_BASE, REG_AXXX_CP_IB2_BASE),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_IB2_BUFSZ, REG_AXXX_CP_IB2_BUFSZ),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_TIMESTAMP, REG_AXXX_CP_SCRATCH_REG0),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_ME_RAM_RADDR, REG_AXXX_CP_ME_RAM_RADDR),
- REG_ADRENO_DEFINE(REG_ADRENO_SCRATCH_ADDR, REG_AXXX_SCRATCH_ADDR),
- REG_ADRENO_DEFINE(REG_ADRENO_SCRATCH_UMSK, REG_AXXX_SCRATCH_UMSK),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_ROQ_ADDR, REG_A3XX_CP_ROQ_ADDR),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_ROQ_DATA, REG_A3XX_CP_ROQ_DATA),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_MERCIU_ADDR, REG_A3XX_CP_MERCIU_ADDR),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_MERCIU_DATA, REG_A3XX_CP_MERCIU_DATA),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_MERCIU_DATA2, REG_A3XX_CP_MERCIU_DATA2),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_MEQ_ADDR, REG_A3XX_CP_MEQ_ADDR),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_MEQ_DATA, REG_A3XX_CP_MEQ_DATA),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_HW_FAULT, REG_A3XX_CP_HW_FAULT),
- REG_ADRENO_DEFINE(REG_ADRENO_CP_PROTECT_STATUS,
- REG_A3XX_CP_PROTECT_STATUS),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_STATUS, REG_A3XX_RBBM_STATUS),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_PERFCTR_CTL,
- REG_A3XX_RBBM_PERFCTR_CTL),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_PERFCTR_LOAD_CMD0,
- REG_A3XX_RBBM_PERFCTR_LOAD_CMD0),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_PERFCTR_LOAD_CMD1,
- REG_A3XX_RBBM_PERFCTR_LOAD_CMD1),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_PERFCTR_PWR_1_LO,
- REG_A3XX_RBBM_PERFCTR_PWR_1_LO),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_INT_0_MASK, REG_A3XX_RBBM_INT_0_MASK),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_INT_0_STATUS,
- REG_A3XX_RBBM_INT_0_STATUS),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_AHB_ERROR_STATUS,
- REG_A3XX_RBBM_AHB_ERROR_STATUS),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_AHB_CMD, REG_A3XX_RBBM_AHB_CMD),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_INT_CLEAR_CMD,
- REG_A3XX_RBBM_INT_CLEAR_CMD),
- REG_ADRENO_DEFINE(REG_ADRENO_RBBM_CLOCK_CTL, REG_A3XX_RBBM_CLOCK_CTL),
- REG_ADRENO_DEFINE(REG_ADRENO_VPC_DEBUG_RAM_SEL,
- REG_A3XX_VPC_VPC_DEBUG_RAM_SEL),
- REG_ADRENO_DEFINE(REG_ADRENO_VPC_DEBUG_RAM_READ,
- REG_A3XX_VPC_VPC_DEBUG_RAM_READ),
- REG_ADRENO_DEFINE(REG_ADRENO_VSC_SIZE_ADDRESS,
- REG_A3XX_VSC_SIZE_ADDRESS),
- REG_ADRENO_DEFINE(REG_ADRENO_VFD_CONTROL_0, REG_A3XX_VFD_CONTROL_0),
- REG_ADRENO_DEFINE(REG_ADRENO_VFD_INDEX_MAX, REG_A3XX_VFD_INDEX_MAX),
- REG_ADRENO_DEFINE(REG_ADRENO_SP_VS_PVT_MEM_ADDR_REG,
- REG_A3XX_SP_VS
[PATCH 2/3] drm/msm: gpu: Return error on hw_init failure
When the GPU hardware init function fails (like say, ME_INIT timed
out) return error instead of blindly continuing on. This gives us
a small chance of saving the system before it goes boom.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 21 -
drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 20 +++-
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 11 +--
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 2 +-
drivers/gpu/drm/msm/msm_gpu.h | 2 +-
5 files changed, 30 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index 5eda847..13989d9 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -41,7 +41,7 @@ extern bool hang_debug;
static void a3xx_dump(struct msm_gpu *gpu);
-static void a3xx_me_init(struct msm_gpu *gpu)
+static bool a3xx_me_init(struct msm_gpu *gpu)
{
struct msm_ringbuffer *ring = gpu->rb;
@@ -65,7 +65,7 @@ static void a3xx_me_init(struct msm_gpu *gpu)
OUT_RING(ring, 0x);
gpu->funcs->flush(gpu);
- gpu->funcs->idle(gpu);
+ return gpu->funcs->idle(gpu);
}
static int a3xx_hw_init(struct msm_gpu *gpu)
@@ -294,9 +294,7 @@ static int a3xx_hw_init(struct msm_gpu *gpu)
/* clear ME_HALT to start micro engine */
gpu_write(gpu, REG_AXXX_CP_ME_CNTL, 0);
- a3xx_me_init(gpu);
-
- return 0;
+ return a3xx_me_init(gpu) ? 0 : -EINVAL;
}
static void a3xx_recover(struct msm_gpu *gpu)
@@ -330,17 +328,22 @@ static void a3xx_destroy(struct msm_gpu *gpu)
kfree(a3xx_gpu);
}
-static void a3xx_idle(struct msm_gpu *gpu)
+static bool a3xx_idle(struct msm_gpu *gpu)
{
/* wait for ringbuffer to drain: */
- adreno_idle(gpu);
+ if (!adreno_idle(gpu))
+ return false;
/* then wait for GPU to finish: */
if (spin_until(!(gpu_read(gpu, REG_A3XX_RBBM_STATUS) &
- A3XX_RBBM_STATUS_GPU_BUSY)))
+ A3XX_RBBM_STATUS_GPU_BUSY))) {
DRM_ERROR("%s: timeout waiting for GPU to idle!\n", gpu->name);
- /* TODO maybe we need to reset GPU here to recover from hang? */
+ /* TODO maybe we need to reset GPU here to recover from hang? */
+ return false;
+ }
+
+ return true;
}
static irqreturn_t a3xx_irq(struct msm_gpu *gpu)
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index 0354be2..ba16507 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -113,7 +113,7 @@ static void a4xx_enable_hwcg(struct msm_gpu *gpu)
}
-static void a4xx_me_init(struct msm_gpu *gpu)
+static bool a4xx_me_init(struct msm_gpu *gpu)
{
struct msm_ringbuffer *ring = gpu->rb;
@@ -137,7 +137,7 @@ static void a4xx_me_init(struct msm_gpu *gpu)
OUT_RING(ring, 0x);
gpu->funcs->flush(gpu);
- gpu->funcs->idle(gpu);
+ return gpu->funcs->idle(gpu);
}
static int a4xx_hw_init(struct msm_gpu *gpu)
@@ -292,9 +292,7 @@ static int a4xx_hw_init(struct msm_gpu *gpu)
/* clear ME_HALT to start micro engine */
gpu_write(gpu, REG_A4XX_CP_ME_CNTL, 0);
- a4xx_me_init(gpu);
-
- return 0;
+ return a4xx_me_init(gpu) ? 0 : -EINVAL;
}
static void a4xx_recover(struct msm_gpu *gpu)
@@ -328,17 +326,21 @@ static void a4xx_destroy(struct msm_gpu *gpu)
kfree(a4xx_gpu);
}
-static void a4xx_idle(struct msm_gpu *gpu)
+static bool a4xx_idle(struct msm_gpu *gpu)
{
/* wait for ringbuffer to drain: */
- adreno_idle(gpu);
+ if (!adreno_idle(gpu))
+ return false;
/* then wait for GPU to finish: */
if (spin_until(!(gpu_read(gpu, REG_A4XX_RBBM_STATUS) &
- A4XX_RBBM_STATUS_GPU_BUSY)))
+ A4XX_RBBM_STATUS_GPU_BUSY))) {
DRM_ERROR("%s: timeout waiting for GPU to idle!\n", gpu->name);
+ /* TODO maybe we need to reset GPU here to recover from hang? */
+ return false;
+ }
- /* TODO maybe we need to reset GPU here to recover from hang? */
+ return true;
}
static irqreturn_t a4xx_irq(struct msm_gpu *gpu)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index f386f46..8b2201c 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -218,19 +218,18 @@ void adreno_flush(struct msm_gpu *gpu)
adreno_gpu_write(adreno_gpu, REG_ADRENO_CP_RB_WPTR, wptr);
}
-void adreno_idle(struct msm_gpu *gpu)
+bool adreno_idle(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
uint32_t wptr = get_wptr(gpu->rb);
- int ret;
/* wait for CP to drain ringbuffer: */
- ret = spin_until(get_
[PATCH 3/3] drm/msm: gpu Add new gpu register read/write functions
Add some new functions to manipulate GPU registers. gpu_read64 and
gpu_write64 can read/write a 64 bit value to two 32 bit registers.
For 4XX and older these are normally perfcounter registers, but
future targets will use 64 bit addressing so there will be many
more spots where a 64 bit read and write are needed.
gpu_rmw() does a read/modify/write on a 32 bit register given a mask
and bits to OR in.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 12 ++-
drivers/gpu/drm/msm/msm_gpu.h | 39 +++
2 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index ba16507..b82210c 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -513,16 +513,8 @@ static int a4xx_pm_suspend(struct msm_gpu *gpu) {
static int a4xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
{
- uint32_t hi, lo, tmp;
-
- tmp = gpu_read(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_HI);
- do {
- hi = tmp;
- lo = gpu_read(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO);
- tmp = gpu_read(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_HI);
- } while (tmp != hi);
-
- *value = (((uint64_t)hi) << 32) | lo;
+ *value = gpu_read64(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO,
+ REG_A4XX_RBBM_PERFCTR_CP_0_HI);
return 0;
}
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 4ee95ca..f8e6657 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -154,6 +154,45 @@ static inline u32 gpu_read(struct msm_gpu *gpu, u32 reg)
return msm_readl(gpu->mmio + (reg << 2));
}
+static inline void gpu_rmw(struct msm_gpu *gpu, u32 reg, u32 mask, u32 or)
+{
+ uint32_t val = gpu_read(gpu, reg);
+
+ val &= ~mask;
+ gpu_write(gpu, reg, val | or);
+}
+
+static inline u64 gpu_read64(struct msm_gpu *gpu, u32 lo, u32 hi)
+{
+ u64 val;
+
+ /*
+* Why not a readq here? Two reasons: 1) many of the LO registers are
+* not quad word aligned and 2) the GPU hardware designers have a bit
+* of a history of putting registers where they fit, especially in
+* spins. The longer a GPU family goes the higher the chance that
+* we'll get burned. We could do a series of validity checks if we
+* wanted to, but really is a readq() that much better? Nah.
+*/
+
+ /*
+* For some lo/hi registers (like perfcounters), the hi value is latched
+* when the lo is read, so make sure to read the lo first to trigger
+* that
+*/
+ val = (u64) msm_readl(gpu->mmio + (lo << 2));
+ val |= ((u64) msm_readl(gpu->mmio + (hi << 2)) << 32);
+
+ return val;
+}
+
+static inline void gpu_write64(struct msm_gpu *gpu, u32 lo, u32 hi, u64 val)
+{
+ /* Why not a writeq here? Read the screed above */
+ msm_writel(lower_32_bits(val), gpu->mmio + (lo << 2));
+ msm_writel(upper_32_bits(val), gpu->mmio + (hi << 2));
+}
+
int msm_gpu_pm_suspend(struct msm_gpu *gpu);
int msm_gpu_pm_resume(struct msm_gpu *gpu);
--
1.9.1
[PATCH] drm/sun4i: Fix a return value in case of error
On Fri, Nov 18, 2016 at 07:18:47PM +0100, Christophe JAILLET wrote: > If 'sun4i_backend_drm_format_to_layer()' does not return 0, then 'val' is > left unmodified. > As it is not initialized either, the return value can be anything. > > It is likely that returning the error code was expected here. > > As the only caller of 'sun4i_backend_update_layer_formats()' does not check > the return value, this fix is purely theorical. > > Signed-off-by: Christophe JAILLET Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161122/a28ed215/attachment.sig>
[PATCH] drm: check for NULL parameter in exported drm_get_format_name() function.
drm_get_format_name() de-references the buf parameter without checking
if the pointer was not NULL. Given that the function is EXPORT-ed, lets
sanitise the parameters before proceeding.
Fixes: b3c11ac267d461d3d5 ("drm: move allocation out of drm_get_format_name())
Cc: Eric Engestrom
Cc: Rob Clark
Cc: Jani Nikula
Cc: Daniel Vetter
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/drm_fourcc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 90d2cc8..0a3ff0b 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -85,6 +85,9 @@ EXPORT_SYMBOL(drm_mode_legacy_fb_format);
*/
const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf
*buf)
{
+ if (!buf)
+ return NULL;
+
snprintf(buf->str, sizeof(buf->str),
"%c%c%c%c %s-endian (0x%08x)",
printable_char(format & 0xff),
--
2.10.2
[PATCH] drm: check for NULL parameter in exported drm_get_format_name() function.
On Tue, Nov 22, 2016 at 04:41:06PM +, Liviu Dudau wrote:
> drm_get_format_name() de-references the buf parameter without checking
> if the pointer was not NULL. Given that the function is EXPORT-ed, lets
> sanitise the parameters before proceeding.
>
> Fixes: b3c11ac267d461d3d5 ("drm: move allocation out of drm_get_format_name())
> Cc: Eric Engestrom
> Cc: Rob Clark
> Cc: Jani Nikula
> Cc: Daniel Vetter
>
> Signed-off-by: Liviu Dudau
> ---
> drivers/gpu/drm/drm_fourcc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 90d2cc8..0a3ff0b 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -85,6 +85,9 @@ EXPORT_SYMBOL(drm_mode_legacy_fb_format);
> */
> const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf
> *buf)
> {
> + if (!buf)
> + return NULL;
> +
Seems rather pointless to me. Why would you ever pass NULL to this guy?
> snprintf(buf->str, sizeof(buf->str),
>"%c%c%c%c %s-endian (0x%08x)",
>printable_char(format & 0xff),
> --
> 2.10.2
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
[pull] amdgpu drm-fixes-4.9
Hi Dave, Just one small fix for amdgpu. The following changes since commit 1da2c326e43b0834105993d13610647337bbad67: drm/amdgpu:fix vpost_needed routine (2016-11-15 14:06:07 -0500) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-fixes-4.9 for you to fetch changes up to da7800a88c5a3b798f763d6f9f343e9a49860c4f: drm/amd/powerplay: avoid out of bounds access on array ps. (2016-11-16 14:26:17 -0500) Rex Zhu (1): drm/amd/powerplay: avoid out of bounds access on array ps. drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-)
[PATCH v3 02/11] drm/tilcdc: implement palette loading for rev1
From: Bartosz Golaszewski
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
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 88 +++-
1 file changed, 87 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 5260eb2..0bfd7dd 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -21,11 +21,15 @@
#include
#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;
@@ -56,6 +60,10 @@ struct tilcdc_crtc {
int sync_lost_count;
bool frame_intact;
struct work_struct recover_work;
+
+ 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)
@@ -105,6 +113,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;
@@ -160,6 +217,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));
mutex_lock(&tilcdc_crtc->enable_lock);
@@ -172,6 +230,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);
@@ -213,6 +274,13 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool
shutdown)
__func__);
}
+ /*
+* LCDC will not retain the palette when reset. Make sure it gets
+* reloaded on tilcdc_crtc_enable().
+*/
+ if (priv->rev == 1)
+ reinit_completion(&tilcdc_crtc->palette_loaded);
+
drm_crtc_vblank_off(crtc);
tilcdc_crtc_disable_irqs(dev);
@@ -846,6 +914,14 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
dev_err_ratelimited(dev->dev, "%s(0x%08x): FIF
[PATCH v3 01/11] drm/tilcdc: Enable sync lost error and recovery handling for rev 1 LCDC
Revision 1 LCDC support also sync lost errors and can benefit from
sync lost recovery routine.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 33 +
drivers/gpu/drm/tilcdc/tilcdc_regs.h | 1 +
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index c787349..5260eb2 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -113,6 +113,7 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
if (priv->rev == 1) {
tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
+ LCDC_V1_SYNC_LOST_ENA |
LCDC_V1_UNDERFLOW_INT_ENA);
tilcdc_set(dev, LCDC_DMA_CTRL_REG,
LCDC_V1_END_OF_FRAME_INT_ENA);
@@ -130,7 +131,7 @@ static void tilcdc_crtc_disable_irqs(struct drm_device *dev)
/* disable irqs that we might have enabled: */
if (priv->rev == 1) {
- tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
+ tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_SYNC_LOST_ENA |
LCDC_V1_UNDERFLOW_INT_ENA | LCDC_V1_PL_INT_ENA);
tilcdc_clear(dev, LCDC_DMA_CTRL_REG,
LCDC_V1_END_OF_FRAME_INT_ENA);
@@ -845,6 +846,21 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underflow",
__func__, stat);
+ if (stat & LCDC_SYNC_LOST) {
+ dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
+ __func__, stat);
+ tilcdc_crtc->frame_intact = false;
+ if (tilcdc_crtc->sync_lost_count++ >
+ SYNC_LOST_COUNT_LIMIT) {
+ dev_err(dev->dev, "%s(0x%08x): Sync lost flood
detected, recovering", __func__, stat);
+ queue_work(system_wq,
+ &tilcdc_crtc->recover_work);
+ tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
+LCDC_SYNC_LOST);
+ tilcdc_crtc->sync_lost_count = 0;
+ }
+ }
+
/* For revision 2 only */
if (priv->rev == 2) {
if (stat & LCDC_FRAME_DONE) {
@@ -852,21 +868,6 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
wake_up(&tilcdc_crtc->frame_done_wq);
}
- if (stat & LCDC_SYNC_LOST) {
- dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
- __func__, stat);
- tilcdc_crtc->frame_intact = false;
- if (tilcdc_crtc->sync_lost_count++ >
- SYNC_LOST_COUNT_LIMIT) {
- dev_err(dev->dev, "%s(0x%08x): Sync lost flood
detected, recovering", __func__, stat);
- queue_work(system_wq,
- &tilcdc_crtc->recover_work);
- tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
-LCDC_SYNC_LOST);
- tilcdc_crtc->sync_lost_count = 0;
- }
- }
-
/* Indicate to LCDC that the interrupt service routine has
* completed, see 13.3.6.1.6 in AM335x TRM.
*/
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index f57c0d6..beb8c21 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -61,6 +61,7 @@
#define LCDC_V2_UNDERFLOW_INT_ENABIT(5)
#define LCDC_V1_PL_INT_ENA BIT(4)
#define LCDC_V2_PL_INT_ENA BIT(6)
+#define LCDC_V1_SYNC_LOST_ENABIT(5)
#define LCDC_MONOCHROME_MODE BIT(1)
#define LCDC_RASTER_ENABLE BIT(0)
#define LCDC_TFT_ALT_ENABLE BIT(23)
--
1.9.1
[PATCH v3 00/11] drm/tilcdc: LCDC Revision 1 related fixes
The git branch bellow is updated. Changes since v2: - Add: "drm/tilcdc: Fix load mode bit-field setting in tilcdc_crtc_enable()" - Drop: "drm/tilcdc: Free palette dma memory in tilcdc_crtc_destroy()" - Add: "drm/tilcdc: Add timeout wait for palette loading to complete" - Add: "drm/tilcdc: Call reset() before loading the palette" - Add: "drm/tilcdc: Use complete_all() to indicate completed palette loading" - Add "drm/tilcdc: Enable frame done irq and functionality for LCDC rev 1" - Bartosz: Please test if this works! The symptom for not working is "timeout waiting for framedone" message when screen is blanked. Changes since first version of the series: - Move tilcdc_regs.h changes from "drm/tilcdc: Enable palette loading for revision 2 LCDC too" to "drm/tilcdc: Add tilcdc_write_mask() to tilcdc_regs.h" These patches are inspired by this series form Bartosz Golaszewski: https://www.spinics.net/lists/arm-kernel/msg539629.html The patches are based on drm-next plus the earlier patches that I plan to send in a pull request for 4.10. The base + these patches are pushed here: https://github.com/jsarha/linux drm-next-tilcdc-for-4.10-wip Bartosz, please test if this branch works for rev1 LCDC, with your dts file! Bartosz Golaszewski (1): drm/tilcdc: implement palette loading for rev1 Jyri Sarha (10): drm/tilcdc: Enable sync lost error and recovery handling for rev 1 LCDC drm/tilcdc: Fix tilcdc_crtc_create() return value handling drm/tilcdc: Add tilcdc_write_mask() to tilcdc_regs.h drm/tilcdc: Fix load mode bit-field setting in tilcdc_crtc_enable() drm/tilcdc: Enable palette loading for revision 2 LCDC too drm/tilcdc: Add timeout wait for palette loading to complete drm/tilcdc: Call reset() before loading the palette drm/tilcdc: Use complete_all() to indicate completed palette loading drm/tilcdc: Load palette at the end of mode_set_nofb() drm/tilcdc: Enable frame done irq and functionality for LCDC rev 1 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 163 +++ drivers/gpu/drm/tilcdc/tilcdc_drv.c | 23 +++-- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 3 +- drivers/gpu/drm/tilcdc/tilcdc_regs.h | 15 4 files changed, 159 insertions(+), 45 deletions(-) -- 1.9.1
[PATCH v3 05/11] drm/tilcdc: Fix load mode bit-field setting in tilcdc_crtc_enable()
Set LCDC_PALETTE_LOAD_MODE bit-field with new tilcdc_write_mask() instead of tilcdc_set(). Setting a bit-fields with tilcdc_set() is fundamentally broken. Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index b3f35dc..0f89422 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -236,7 +236,9 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc) tilcdc_crtc_enable_irqs(dev); tilcdc_clear(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE); - tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_PALETTE_LOAD_MODE(DATA_ONLY)); + tilcdc_write_mask(dev, LCDC_RASTER_CTRL_REG, + LCDC_PALETTE_LOAD_MODE(DATA_ONLY), + LCDC_PALETTE_LOAD_MODE_MASK); tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE); drm_crtc_vblank_on(crtc); -- 1.9.1
[PATCH v3 10/11] drm/tilcdc: Load palette at the end of mode_set_nofb()
Load palette at the end of mode_set_nofb() and only if the palette has
not been loaded since last runtime resume. Moving the palette loading
to mode_set_nofb() saves us from storing and restoring of LCDC dma
addresses that were just recently updated.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 33 +
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 12
drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 +
3 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index fd3654d..1a1ff8d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -113,6 +113,13 @@ static void set_scanout(struct drm_crtc *crtc, struct
drm_framebuffer *fb)
tilcdc_crtc->curr_fb = fb;
}
+void tilcdc_crtc_reload_palette(struct drm_crtc *crtc)
+{
+ struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+
+ reinit_completion(&tilcdc_crtc->palette_loaded);
+}
+
static void reset(struct drm_crtc *crtc);
/*
* The driver currently only supports only true color formats. For
@@ -122,15 +129,13 @@ static void set_scanout(struct drm_crtc *crtc, struct
drm_framebuffer *fb)
*/
static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
{
- u32 dma_fb_base, dma_fb_ceiling, raster_ctl;
struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct tilcdc_drm_private *priv = dev->dev_private;
int ret;
- 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);
+ if (completion_done(&tilcdc_crtc->palette_loaded))
+ return;
/* SW reset before turning DMA on (see section 13.3.8 in AM335x TRM)*/
reset(crtc);
@@ -168,11 +173,6 @@ static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
else
tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, LCDC_V2_PL_INT_ENA);
-
- /* Restore the registers. */
- 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)
@@ -242,9 +242,6 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
reset(crtc);
- if (!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);
@@ -288,12 +285,6 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool
shutdown)
__func__);
}
- /*
-* LCDC will not retain the palette when reset. Make sure it gets
-* reloaded on tilcdc_crtc_enable().
-*/
- reinit_completion(&tilcdc_crtc->palette_loaded);
-
drm_crtc_vblank_off(crtc);
tilcdc_crtc_disable_irqs(dev);
@@ -681,10 +672,12 @@ static void tilcdc_crtc_mode_set_nofb(struct drm_crtc
*crtc)
drm_framebuffer_reference(fb);
- set_scanout(crtc, fb);
-
tilcdc_crtc_set_clk(crtc);
+ tilcdc_crtc_load_palette(crtc);
+
+ set_scanout(crtc, fb);
+
crtc->hwmode = crtc->state->adjusted_mode;
}
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 28e97d5..a7c91f7 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -637,8 +637,20 @@ static int tilcdc_pm_resume(struct device *dev)
}
#endif
+static int tilcdc_pm_runtime_resume(struct device *dev)
+{
+ struct drm_device *ddev = dev_get_drvdata(dev);
+ struct tilcdc_drm_private *priv = ddev->dev_private;
+
+ if (priv->crtc)
+ tilcdc_crtc_reload_palette(priv->crtc);
+
+ return 0;
+}
+
static const struct dev_pm_ops tilcdc_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(tilcdc_pm_suspend, tilcdc_pm_resume)
+ .runtime_resume = tilcdc_pm_runtime_resume,
};
/*
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 7913b0e..5803848 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -180,6 +180,7 @@ void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc
*crtc,
int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
struct drm_pending_vblank_event *event);
+void tilcdc_crtc_reload_palette(struct drm_crtc *crtc);
int tilcdc_plane_init(struct drm_device *dev, struct drm_plane *plane);
--
1.9.1
[PATCH v3 11/11] drm/tilcdc: Enable frame done irq and functionality for LCDC rev 1
We should wait for the last frame to complete before shutting things
down also on LCDC rev 1.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 34 +-
drivers/gpu/drm/tilcdc/tilcdc_regs.h | 1 +
2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 1a1ff8d..f251546 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -183,7 +183,7 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
if (priv->rev == 1) {
tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
- LCDC_V1_SYNC_LOST_ENA |
+ LCDC_V1_SYNC_LOST_ENA | LCDC_V1_FRAME_DONE_ENA |
LCDC_V1_UNDERFLOW_INT_ENA);
tilcdc_set(dev, LCDC_DMA_CTRL_REG,
LCDC_V1_END_OF_FRAME_INT_ENA);
@@ -201,7 +201,8 @@ static void tilcdc_crtc_disable_irqs(struct drm_device *dev)
/* disable irqs that we might have enabled: */
if (priv->rev == 1) {
- tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_SYNC_LOST_ENA |
+ tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
+ LCDC_V1_SYNC_LOST_ENA | LCDC_V1_FRAME_DONE_ENA |
LCDC_V1_UNDERFLOW_INT_ENA | LCDC_V1_PL_INT_ENA);
tilcdc_clear(dev, LCDC_DMA_CTRL_REG,
LCDC_V1_END_OF_FRAME_INT_ENA);
@@ -261,6 +262,7 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool
shutdown)
struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct tilcdc_drm_private *priv = dev->dev_private;
+ int ret;
mutex_lock(&tilcdc_crtc->enable_lock);
if (shutdown)
@@ -273,17 +275,15 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool
shutdown)
tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
/*
-* if necessary wait for framedone irq which will still come
-* before putting things to sleep..
+* Wait for framedone irq which will still come before putting
+* things to sleep..
*/
- if (priv->rev == 2) {
- int ret = wait_event_timeout(tilcdc_crtc->frame_done_wq,
-tilcdc_crtc->frame_done,
-msecs_to_jiffies(500));
- if (ret == 0)
- dev_err(dev->dev, "%s: timeout waiting for framedone\n",
- __func__);
- }
+ ret = wait_event_timeout(tilcdc_crtc->frame_done_wq,
+tilcdc_crtc->frame_done,
+msecs_to_jiffies(500));
+ if (ret == 0)
+ dev_err(dev->dev, "%s: timeout waiting for framedone\n",
+ __func__);
drm_crtc_vblank_off(crtc);
@@ -938,13 +938,13 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
}
}
+ if (stat & LCDC_FRAME_DONE) {
+ tilcdc_crtc->frame_done = true;
+ wake_up(&tilcdc_crtc->frame_done_wq);
+ }
+
/* For revision 2 only */
if (priv->rev == 2) {
- if (stat & LCDC_FRAME_DONE) {
- tilcdc_crtc->frame_done = true;
- wake_up(&tilcdc_crtc->frame_done_wq);
- }
-
/* Indicate to LCDC that the interrupt service routine has
* completed, see 13.3.6.1.6 in AM335x TRM.
*/
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index 56dbfbd..4e6975a 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -67,6 +67,7 @@
#define LCDC_V1_PL_INT_ENA BIT(4)
#define LCDC_V2_PL_INT_ENA BIT(6)
#define LCDC_V1_SYNC_LOST_ENABIT(5)
+#define LCDC_V1_FRAME_DONE_ENA BIT(3)
#define LCDC_MONOCHROME_MODE BIT(1)
#define LCDC_RASTER_ENABLE BIT(0)
#define LCDC_TFT_ALT_ENABLE BIT(23)
--
1.9.1
[PATCH v3 04/11] drm/tilcdc: Add tilcdc_write_mask() to tilcdc_regs.h
Add tilcdc_write_mask() for handling register field wider than one bit
and mask values for those fields.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/tilcdc/tilcdc_regs.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index beb8c21..56dbfbd 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -34,11 +34,14 @@
/* LCDC DMA Control Register */
#define LCDC_DMA_BURST_SIZE(x) ((x) << 4)
+#define LCDC_DMA_BURST_SIZE_MASK ((0x7) << 4)
#define LCDC_DMA_BURST_1 0x0
#define LCDC_DMA_BURST_2 0x1
#define LCDC_DMA_BURST_4 0x2
#define LCDC_DMA_BURST_8 0x3
#define LCDC_DMA_BURST_160x4
+#define LCDC_DMA_FIFO_THRESHOLD(x) ((x) << 8)
+#define LCDC_DMA_FIFO_THRESHOLD_MASK ((0x3) << 8)
#define LCDC_V1_END_OF_FRAME_INT_ENA BIT(2)
#define LCDC_V2_END_OF_FRAME0_INT_ENABIT(8)
#define LCDC_V2_END_OF_FRAME1_INT_ENABIT(9)
@@ -46,10 +49,12 @@
/* LCDC Control Register */
#define LCDC_CLK_DIVISOR(x) ((x) << 8)
+#define LCDC_CLK_DIVISOR_MASK((0xFF) << 8)
#define LCDC_RASTER_MODE 0x01
/* LCDC Raster Control Register */
#define LCDC_PALETTE_LOAD_MODE(x)((x) << 20)
+#define LCDC_PALETTE_LOAD_MODE_MASK ((0x3) << 20)
#define PALETTE_AND_DATA 0x00
#define PALETTE_ONLY 0x01
#define DATA_ONLY0x02
@@ -75,7 +80,9 @@
/* LCDC Raster Timing 2 Register */
#define LCDC_AC_BIAS_TRANSITIONS_PER_INT(x) ((x) << 16)
+#define LCDC_AC_BIAS_TRANSITIONS_PER_INT_MASK((0xF) << 16)
#define LCDC_AC_BIAS_FREQUENCY(x)((x) << 8)
+#define LCDC_AC_BIAS_FREQUENCY_MASK ((0xFF) << 8)
#define LCDC_SYNC_CTRL BIT(25)
#define LCDC_SYNC_EDGE BIT(24)
#define LCDC_INVERT_PIXEL_CLOCK BIT(22)
@@ -140,6 +147,12 @@ static inline u32 tilcdc_read(struct drm_device *dev, u32
reg)
return ioread32(priv->mmio + reg);
}
+static inline void tilcdc_write_mask(struct drm_device *dev, u32 reg,
+u32 val, u32 mask)
+{
+ tilcdc_write(dev, reg, (tilcdc_read(dev, reg) & ~mask) | (val & mask));
+}
+
static inline void tilcdc_set(struct drm_device *dev, u32 reg, u32 mask)
{
tilcdc_write(dev, reg, tilcdc_read(dev, reg) | mask);
--
1.9.1
[PATCH v3 09/11] drm/tilcdc: Use complete_all() to indicate completed palette loading
We need to use complete_all() to indicate completed palette loading in
stead of plain complete() if we want to test if the palette has
already been loaded with completion_done().
indicated with.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 963e0a0..fd3654d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -928,7 +928,7 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
__func__, stat);
if (stat & LCDC_PL_LOAD_DONE)
- complete(&tilcdc_crtc->palette_loaded);
+ complete_all(&tilcdc_crtc->palette_loaded);
if (stat & LCDC_SYNC_LOST) {
dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
--
1.9.1
[PATCH v3 08/11] drm/tilcdc: Call reset() before loading the palette
The palette loading does not work reliably without LCDC SW reset before it. The AM335x TRM suggests this [1] after L3 clock domain has been shut down. We have no knowledge of such events so we do it always. The software reset will clear all the frame information in the FIFO. Upon a restart, the L3 DMA will fetch from the fb0_base address [2]. [1] Section 13.3.8 in AM335x TRM (http://www.ti.com/lit/pdf/sprz360) [2] Section 13.4.6 in AM335x TRM Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index fbb41b1..963e0a0 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -113,6 +113,7 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb) tilcdc_crtc->curr_fb = fb; } +static void reset(struct drm_crtc *crtc); /* * The driver currently only supports only true color formats. For * true color the palette block is bypassed, but a 32 byte palette @@ -131,6 +132,9 @@ static void tilcdc_crtc_load_palette(struct drm_crtc *crtc) dma_fb_ceiling = tilcdc_read(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG); raster_ctl = tilcdc_read(dev, LCDC_RASTER_CTRL_REG); + /* SW reset before turning DMA on (see section 13.3.8 in AM335x TRM)*/ + reset(crtc); + /* Tell the LCDC where the palette is located. */ tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, tilcdc_crtc->palette_dma_handle); -- 1.9.1
