[Bug 87196] [radeonsi][dota2] small freezes happens while playing with a drm-next-3.19 linux kernel
https://bugs.freedesktop.org/show_bug.cgi?id=87196 Sylvain BERTRAND changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |WORKSFORME --- Comment #6 from Sylvain BERTRAND --- ok... I cannot reproduce the mini-freezes. They seems gone. Driver would have been the wrong culprit. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/ff4fe40d/attachment.html>
[Bug 82201] [HAWAII] GPU doesn't reclock, poor 3D performance
https://bugs.freedesktop.org/show_bug.cgi?id=82201 Michel Dänzer changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #52 from Michel Dänzer --- Kai's bug is fixed. Sebastian, please file your own report. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/7dae965b/attachment-0001.html>
[PATCH RESEND 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls
The ioctl DRM_EXYNOS_GEM_MAP_OFFSET and DRM_EXYNOS_GEM_MMAP are removed from
the linux kernel. This patch modifies libdrm and libkms to use drm generic
ioctls instead of the removed ioctls.
Signed-off-by: Hyungwon Hwang
Signed-off-by: Inki Dae
---
exynos/exynos_drm.c | 24 +---
libkms/exynos.c | 7 ---
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/exynos/exynos_drm.c b/exynos/exynos_drm.c
index 4c7dd13..4cb6a6d 100644
--- a/exynos/exynos_drm.c
+++ b/exynos/exynos_drm.c
@@ -283,20 +283,22 @@ drm_public void *exynos_bo_map(struct exynos_bo *bo)
{
if (!bo->vaddr) {
struct exynos_device *dev = bo->dev;
- struct drm_exynos_gem_mmap req = {
- .handle = bo->handle,
- .size = bo->size,
- };
+ struct drm_mode_map_dumb arg;
+ void *map = NULL;
int ret;
- ret = drmIoctl(dev->fd, DRM_IOCTL_EXYNOS_GEM_MMAP, &req);
- if (ret) {
- fprintf(stderr, "failed to mmap[%s].\n",
- strerror(errno));
- return NULL;
- }
+ memset(&arg, 0, sizeof(arg));
+ arg.handle = bo->handle;
+
+ ret = drmIoctl(dev->fd, DRM_IOCTL_MODE_MAP_DUMB, &arg);
+ if (ret)
+ return ret;
- bo->vaddr = (void *)(uintptr_t)req.mapped;
+ map = drm_mmap(0, bo->size, PROT_READ | PROT_WRITE, MAP_SHARED,
+ dev->fd, arg.offset);
+
+ if (map == MAP_FAILED)
+ return NULL;
}
return bo->vaddr;
diff --git a/libkms/exynos.c b/libkms/exynos.c
index 92e329c..1123482 100644
--- a/libkms/exynos.c
+++ b/libkms/exynos.c
@@ -25,6 +25,7 @@
#include
#include "xf86drm.h"
+#include "libdrm.h"
#include "exynos_drm.h"
struct exynos_bo
@@ -124,7 +125,7 @@ static int
exynos_bo_map(struct kms_bo *_bo, void **out)
{
struct exynos_bo *bo = (struct exynos_bo *)_bo;
- struct drm_exynos_gem_map_off arg;
+ struct drm_mode_map_dumb arg;
void *map = NULL;
int ret;
@@ -137,11 +138,11 @@ exynos_bo_map(struct kms_bo *_bo, void **out)
memset(&arg, 0, sizeof(arg));
arg.handle = bo->base.handle;
- ret = drmCommandWriteRead(bo->base.kms->fd, DRM_EXYNOS_GEM_MAP_OFFSET,
&arg, sizeof(arg));
+ ret = drmIoctl(bo->base.kms->fd, DRM_IOCTL_MODE_MAP_DUMB, &arg);
if (ret)
return ret;
- map = mmap(0, bo->base.size, PROT_READ | PROT_WRITE, MAP_SHARED,
bo->base.kms->fd, arg.offset);
+ map = drm_mmap(0, bo->base.size, PROT_READ | PROT_WRITE, MAP_SHARED,
bo->base.kms->fd, arg.offset);
if (map == MAP_FAILED)
return -errno;
--
1.9.1
[PATCH RESEND 2/2] exynos: remove DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls
This patch removes the ioctls which are removed from the linux kernel.
Signed-off-by: Hyungwon Hwang
Signed-off-by: Inki Dae
---
exynos/exynos_drm.h | 40
1 file changed, 40 deletions(-)
diff --git a/exynos/exynos_drm.h b/exynos/exynos_drm.h
index c3c6579..256c02f 100644
--- a/exynos/exynos_drm.h
+++ b/exynos/exynos_drm.h
@@ -47,38 +47,6 @@ struct drm_exynos_gem_create {
};
/**
- * A structure for getting buffer offset.
- *
- * @handle: a pointer to gem object created.
- * @pad: just padding to be 64-bit aligned.
- * @offset: relatived offset value of the memory region allocated.
- * - this value should be set by user.
- */
-struct drm_exynos_gem_map_off {
- unsigned int handle;
- unsigned int pad;
- uint64_t offset;
-};
-
-/**
- * A structure for mapping buffer.
- *
- * @handle: a handle to gem object created.
- * @pad: just padding to be 64-bit aligned.
- * @size: memory size to be mapped.
- * @mapped: having user virtual address mmaped.
- * - this variable would be filled by exynos gem module
- * of kernel side with user virtual address which is allocated
- * by do_mmap().
- */
-struct drm_exynos_gem_mmap {
- unsigned int handle;
- unsigned int pad;
- uint64_t size;
- uint64_t mapped;
-};
-
-/**
* A structure to gem information.
*
* @handle: a handle to gem object created.
@@ -164,8 +132,6 @@ struct drm_exynos_g2d_exec {
};
#define DRM_EXYNOS_GEM_CREATE 0x00
-#define DRM_EXYNOS_GEM_MAP_OFFSET 0x01
-#define DRM_EXYNOS_GEM_MMAP0x02
/* Reserved 0x04 ~ 0x05 for exynos specific gem ioctl */
#define DRM_EXYNOS_GEM_GET 0x04
#define DRM_EXYNOS_VIDI_CONNECTION 0x07
@@ -178,12 +144,6 @@ struct drm_exynos_g2d_exec {
#define DRM_IOCTL_EXYNOS_GEM_CREATEDRM_IOWR(DRM_COMMAND_BASE + \
DRM_EXYNOS_GEM_CREATE, struct drm_exynos_gem_create)
-#define DRM_IOCTL_EXYNOS_GEM_MAP_OFFSETDRM_IOWR(DRM_COMMAND_BASE + \
- DRM_EXYNOS_GEM_MAP_OFFSET, struct drm_exynos_gem_map_off)
-
-#define DRM_IOCTL_EXYNOS_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + \
- DRM_EXYNOS_GEM_MMAP, struct drm_exynos_gem_mmap)
-
#define DRM_IOCTL_EXYNOS_GEM_GET DRM_IOWR(DRM_COMMAND_BASE + \
DRM_EXYNOS_GEM_GET, struct drm_exynos_gem_info)
--
1.9.1
[RFC] drm/i915: tame the chattermouth
On Thu, Dec 11, 2014 at 06:18:12PM -0500, Rob Clark wrote:
> Many distro's have mechanism in place to collect and automatically file
> bugs for failed WARN()s. And since every third line in i915 is a WARN()
> it generates quite a lot of noise which is somewhat disconcerting to the
> end user.
>
> Separate out the internal hw-is-in-the-state-I-expected checks into
> I915_WARN()s and allow configuration via i915.verbose_checks module
> param about whether this will generate a full blown stacktrace or just
> DRM_ERROR().
>
> Signed-off-by: Rob Clark
Yeah I guess makes sense, although I still claim that these are as much
"we've lost track of shit" bugs as when a refcount underflows or a pointer
is NULL when it shouldn't. But I also agree that we've done a stellar job
this year at not locking at these bugs, so meh.
Since this is all about hw cross checking, ack if I do an
s/I915_WARN/I915_HW_WARN/ on top?
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 29 +++
> drivers/gpu/drm/i915/i915_params.c | 5 ++
> drivers/gpu/drm/i915/intel_display.c| 134
>
> drivers/gpu/drm/i915/intel_dp.c | 4 +-
> drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
> 5 files changed, 104 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bb1892d..9fabaff 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -60,6 +60,34 @@
> #undef WARN_ON
> #define WARN_ON(x) WARN(x, "WARN_ON(" #x ")")
>
> +/* Use I915_WARN(x) and I915_WARN_ON() (rather than WARN() and WARN_ON())
> + * to check for unexpected conditions which may not necessarily be a user
> + * visible problem. This will either WARN() or DRM_ERROR() depending on
> + * the verbose_checks moduleparam, enabling distros and users to tailor
> + * their preferred amount of i915 abrt spam.
> + */
> +#define I915_WARN(condition, format...) ({ \
> + int __ret_warn_on = !!(condition); \
> + if (unlikely(__ret_warn_on)) { \
> + if (i915.verbose_checks)\
> + __WARN_printf(format); \
> + else\
> + DRM_ERROR(format); \
> + } \
> + unlikely(__ret_warn_on);\
> +})
> +
> +#define I915_WARN_ON(condition) ({ \
> + int __ret_warn_on = !!(condition); \
> + if (unlikely(__ret_warn_on)) { \
> + if (i915.verbose_checks)\
> + __WARN_printf("WARN_ON(" #condition ")\n"); \
> + else\
> + DRM_ERROR("WARN_ON(" #condition ")\n"); \
> + } \
> + unlikely(__ret_warn_on);\
> +})
> +
> enum pipe {
> INVALID_PIPE = -1,
> PIPE_A = 0,
> @@ -2311,6 +2339,7 @@ struct i915_params {
> bool disable_vtd_wa;
> int use_mmio_flip;
> bool mmio_debug;
> + bool verbose_checks;
> };
> extern struct i915_params i915 __read_mostly;
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index c91cb20..72777da 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -51,6 +51,7 @@ struct i915_params i915 __read_mostly = {
> .disable_vtd_wa = 0,
> .use_mmio_flip = 0,
> .mmio_debug = 0,
> + .verbose_checks = 1,
> };
>
> module_param_named(modeset, i915.modeset, int, 0400);
> @@ -173,3 +174,7 @@ module_param_named(mmio_debug, i915.mmio_debug, bool,
> 0600);
> MODULE_PARM_DESC(mmio_debug,
> "Enable the MMIO debug code (default: false). This may negatively "
> "affect performance.");
> +
> +module_param_named(verbose_checks, i915.verbose_checks, bool, 0600);
> +MODULE_PARM_DESC(verbose_checks,
> + "Enable verbose logs (ie. WARN_ON()) in case of unexpected
> conditions.");
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 63247c6..7b4ed18 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1024,7 +1024,7 @@ void assert_pll(struct drm_i915_private *dev_priv,
> reg = DPLL(pipe);
> val = I915_READ(reg);
> cur_state = !!(val & DPLL_VCO_ENABLE);
> - WARN(cur_state != state,
> + I915_WARN(cur_state != state,
>"PLL state assertion failure (expected %s, current %s)\n",
>stat
[RFC] drm/i915: tame the chattermouth
On Fri, Dec 12, 2014 at 08:17:23AM +0100, Daniel Vetter wrote: > On Thu, Dec 11, 2014 at 06:18:12PM -0500, Rob Clark wrote: > > Many distro's have mechanism in place to collect and automatically file > > bugs for failed WARN()s. And since every third line in i915 is a WARN() > > it generates quite a lot of noise which is somewhat disconcerting to the > > end user. > > > > Separate out the internal hw-is-in-the-state-I-expected checks into > > I915_WARN()s and allow configuration via i915.verbose_checks module > > param about whether this will generate a full blown stacktrace or just > > DRM_ERROR(). > > > > Signed-off-by: Rob Clark > > Yeah I guess makes sense, although I still claim that these are as much > "we've lost track of shit" bugs as when a refcount underflows or a pointer > is NULL when it shouldn't. But I also agree that we've done a stellar job > this year at not locking at these bugs, so meh. How about making the state checker WARNs conditional on drm.debug&2? The premise is that they are often tantalising unhelpful without the debug log, so only show them when we have both? -Chris -- Chris Wilson, Intel Open Source Technology Centre
[RFC 04/15] regulator: add restrack support
On 12/11/2014 02:43 PM, Russell King - ARM Linux wrote: > On Thu, Dec 11, 2014 at 12:58:37PM +, Mark Brown wrote: >> I'd expect someone reading the change in the regulator API to have at >> least some idea how this fits in with the rest of the API and how to use >> it, and probably more importantly I'd expect to be able to understand >> why this is DT only. > > Yep. > > This is a repetitive problem, and I fully agree with your concern about > stuff which is supposed to be arch-independent being designed with only > DT in mind. > > New core kernel features should *not* be designed with only DT in mind - > DT is not the only firmware description language which the kernel > supports. Folk need to understand that if they design a new arch > independent kernel feature where the sole use case is with DT, that new > feature is probably going to get rejected, especially when it's > something as generic as resource tracking. > > The world is not DT only. > OK. I will post next version of patchset with resource/provider lookup left to frameworks (regulators, clock, etc), this way it will be fully firmware agnostic. I will add also better description of the framework. Regards Andrzej
[Bug 86663] [radeonsi] Black squares in Sanctum 2
https://bugs.freedesktop.org/show_bug.cgi?id=86663 --- Comment #10 from Daniel Scharrer --- Created attachment 110770 --> https://bugs.freedesktop.org/attachment.cgi?id=110770&action=edit Deadfall Adventures menu screen with DX10_CLAMP patch Setting the DX10_CLAMP bit [1] also fixes the black squares in Sanctum 2. In Deadfall Adventures however there are still much smaller black squares as can be seen in the attached screenshot. [1] http://lists.freedesktop.org/archives/mesa-dev/2014-December/072402.html -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/a8745075/attachment.html>
[Bug 85421] radeon stalled, GPU lockup, reset and failed on resume; crashed by firefox.
https://bugzilla.kernel.org/show_bug.cgi?id=85421 --- Comment #19 from Jorn Amundsen --- (In reply to Hin-Tak Leung from comment #18) > (In reply to Alex Deucher from comment #14) > > Please make sure your version of mesa has this patch: > > http://cgit.freedesktop.org/mesa/mesa/commit/ > > ?id=ae4536b4f71cbe76230ea7edc7eb4d6041e651b4 > > This seems no good /insufficient. I just had a lock-up with 10.3.5, which ... > Seeing as the patch does not work/insufficient, and my best experience so > far is 10.2.9 (lasted 3 weeks, without the patch), my worst experience is > 10.3.3 (less than a day), and 10.3.4/10.3.5 (patch included) lasted a week, > I am going back to 10.2.9, and adding the patch to it. If the patch improves > 10.2.9 the way it did from 10.3.3 -> 10.3.4/10.3.5, i.e. make 10.2.9 lasts a > few months, I'd be happy enough. Hi Hin-Tak, I just would like to add that I have been running for one week without problems after patching Mesa 10.3.3 with the patch in Comment #14. Without the patch, I hung every 10-15 minute. --joern -- You are receiving this mail because: You are watching the assignee of the bug.
[RFC] drm/i915: tame the chattermouth
On Fri, 12 Dec 2014, Chris Wilson wrote: > On Fri, Dec 12, 2014 at 08:17:23AM +0100, Daniel Vetter wrote: >> On Thu, Dec 11, 2014 at 06:18:12PM -0500, Rob Clark wrote: >> > Many distro's have mechanism in place to collect and automatically file >> > bugs for failed WARN()s. And since every third line in i915 is a WARN() Come on Rob, that's not necessary. >> > it generates quite a lot of noise which is somewhat disconcerting to the >> > end user. >> > >> > Separate out the internal hw-is-in-the-state-I-expected checks into >> > I915_WARN()s and allow configuration via i915.verbose_checks module >> > param about whether this will generate a full blown stacktrace or just >> > DRM_ERROR(). >> > >> > Signed-off-by: Rob Clark >> >> Yeah I guess makes sense, although I still claim that these are as much >> "we've lost track of shit" bugs as when a refcount underflows or a pointer >> is NULL when it shouldn't. But I also agree that we've done a stellar job >> this year at not locking at these bugs, so meh. > > How about making the state checker WARNs conditional on drm.debug&2? The > premise is that they are often tantalising unhelpful without the debug > log, so only show them when we have both? As I suggested in an internal mail just days ago, make the checks emit a DRM_ERROR normally (but do log something!), and a WARN if drm.debug & DRM_UT_KMS (or DRM_UT_DRIVER like Chris suggests) holds. I object to adding new kernel parameters for this. We have enough, and our bug fixing efforts really don't need another round of "oh please *also* enable this new parameter we added". BR, Jani. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center
[Bug 88481] Kernel oops leaves system without graphical output [radeon]
https://bugzilla.kernel.org/show_bug.cgi?id=88481 Cherry Ontop changed: What|Removed |Added CC||amerikapsn at web.de --- Comment #13 from Cherry Ontop --- Does this bug still persist in 3.17.6-1 or is it patched there? -- You are receiving this mail because: You are watching the assignee of the bug.
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
Hey, On 10 December 2014 at 17:17, Rob Clark wrote: > > In DRM/KMS we are lacking a good way to deal with tiled/compressed > formats. Especially in the case of dmabuf/prime buffer sharing, where > we cannot always rely on under-the-hood flags passed to driver specific > gem-create ioctl to pass around these extra flags. > > The proposal is to add a per-plane format modifier. This allows to, if > necessary, use different tiling patters for sub-sampled planes, etc. > The format modifiers are added at the end of the ioctl struct, so for > legacy userspace it will be zero padded. > Cool, thanks a lot for looking at this! My one comment is that we could maybe go even further: keep the current modifier as a strict pixel-layout modifier (e.g. tiled, compressed - anything that affects how you actually determine pixel location), and then support an extra (perhaps non-vendor-namespaced) argument for optional pixel _interpretation_ modifiers, e.g. the hints in EGL_EXT_image_dma_buf_import. V4L2 is starting to properly attack things like chroma siting, and being able to specify narrow/wide YUV range is pretty important for STB/DTV in particular. And they're actually starting to move to KMS, too ... It might be useful to make the interpretation modifiers bitmaskable, so they can be combined (e.g. wide-range/unclamped YUV | whatever chroma siting), but I can't think of a usecase for combining multiple layout modifiers (e.g. this tiling | that compression). > TODO how best to deal with assignment of modifier token values? The > rough idea was to namespace things with an 8bit vendor-id, and then > beyond that it is treated as an opaque value. But that was a relatively > arbitrary choice. There are cases where same tiling pattern and/or > compression is supported by various different vendors. So we should > standardize to use the vendor-id and value of the first one who > documents the format? Yeah, I'd second all of danvet's comments here, as well as adding a new ADDFB2_MODIFIERS cap + query for supported modifiers. Makes life much easier for generic userspace. Cheers, Daniel -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/f28f729d/attachment-0001.html>
[PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy
[adding arm-soc maintainers to cc]
Hello Kukjin,
On 12/02/2014 09:39 AM, Jingoo Han wrote:
> On Tuesday, December 02, 2014 5:17 PM, Javier Martinez Canillas wrote:
>> On Mon, Nov 24, 2014 at 6:41 AM, Vivek Gautam
>> wrote:
>> > DP PHY now require pmu-system-controller to handle PMU register
>> > to control PHY's power isolation. Adding the same to dp-phy
>> > node.
>> >
>> > Signed-off-by: Vivek Gautam
>> > Reviewed-by: Jingoo Han
>> > Tested-by: Javier Martinez Canillas
>> > Cc: Kukjin Kim
>>
>> Any opinions about $subject?
>>
>> This patch is -rc material since is needed after commit a5ec598 ("phy:
>> exynos-dp-video: Use syscon
>> support to control pmu register") which landed in 3.18. That means
>> that display for Exynos is currently broken in 3.18.
>>
>> I think it's too late for the 3.18 -rc cycle but at least it would be
>> great to have this merged for 3.19 and backport to stable kernels to
>> have display working again.
>
> I agree with this suggestion.
>
Sorry for being nagging with this but 3.18 has been released and the Exynos
DP video PHY is not working because this patch was not merged :(
So, it would be good if this can be pushed before the merge window for 3.19
closes or we may end with another kernel release with a non-working display.
>>
>> Thierry had concerns that this change breaks DT backward compability
>> but actually it was already been broken by a5ec598 which changed the
>> DT binding for the phy-exynos-dp-video driver so we should either
>> apply this patch now or revert a5ec598.
>
> I think that very few people might use old properties for Exynos DP.
> Actually, DT backward compatibility will not be the considerable problem
> in my opinion.
>
> But, in order to keep the DT backward compatibility, we should revert
> a5ec598, and send another patch for keeping the DT backward compatibility.
>
I'm not sure if is worth it to revert a5ec598 and maintain DT backward
compatibility in this case since it seems there aren't real mainline
users of Exynos DP, otherwise someone would had cared that 3.18 is broken.
IMHO just $subject has to be picked to make the DTS use the new DT binding
of the phy-exynos-dp-video driver. Maybe arm-soc maintainers can pick
$subject directly since Kukjin seems to be busy?
Thanks a lot and best regards,
Javier
[PATCH V8 00/14] drm/exynos: few patches to enhance bridge chip support
Hello, On 11/18/2014 07:20 AM, Ajay kumar wrote: > On Sat, Nov 15, 2014 at 3:24 PM, Ajay Kumar > wrote: >> This series is based on master branch of Linus tree at: >> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git >> This series has been in the mailing lists for months and have been tested by many people. What else is missing before it can be merged? It would be great to have proper display support on platforms that have a eDP to LVDS bridge chip (e.g: Snow, Peach Pit and Spring Chromebooks) and everything is in place but this series which had been missing many kernel releases already. >> >> Changes since V7: >> -- Address comments from Tomi and Laurent: >> -- Use videoports and endpoints to represent the connection >> between >>encoder, bridge and the panel, instead of using phandles. >> -- Address comments from Daniel: >> -- Make the patch description more descriptive. >> -- remove device pointer from drm_bridge, and just use >>device_node instead. >> -- don't pass encoder pointer to bridge_attach. >> -- Address comments from Sean Paul: >> -- Remove bridge from mode_config, and pull out drm_bridge >>functions from drm_crtc.c to drm_bridge.c >> Tomi and Laurent, You asked Ajay to change his series to use the video port and enpoints DT bindings instead of phandles, could you please review his latest version? I guess is now too late for 3.19 since we are in the middle of the merge window but it would be great if this series can at least made it to 3.20. Thanks a lot and best regards, Javier
[drm:hsw_unclaimed_reg_detect] *ERROR* Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.
A syslog entry of a newly installed ThinkPad T440s advices me : Dec 12 00:17:59 t44 kernel: [drm:hsw_unclaimed_reg_detect] *ERROR* Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem. after ei s2disk'ed and resumed it. So I did it and recompiled the kernel with CONFIG_MMIOTRACE=y, s2disk'ed the sytem again and got this (it is a hardened 64 bit Gentoo kernel): Dec 12 13:22:12 t44 kernel: ACPI: Preparing to enter system sleep state S4 Dec 12 13:22:12 t44 kernel: PM: Saving platform NVS memory Dec 12 13:22:12 t44 kernel: Disabling non-boot CPUs ... Dec 12 13:22:12 t44 kernel: intel_pstate CPU 1 exiting Dec 12 13:22:12 t44 kernel: kvm: disabling virtualization on CPU1 Dec 12 13:22:12 t44 kernel: smpboot: CPU 1 is now offline Dec 12 13:22:12 t44 kernel: intel_pstate CPU 2 exiting Dec 12 13:22:12 t44 kernel: kvm: disabling virtualization on CPU2 Dec 12 13:22:12 t44 kernel: smpboot: CPU 2 is now offline Dec 12 13:22:12 t44 kernel: intel_pstate CPU 3 exiting Dec 12 13:22:12 t44 kernel: kvm: disabling virtualization on CPU3 Dec 12 13:22:12 t44 kernel: smpboot: CPU 3 is now offline Dec 12 13:22:12 t44 kernel: PM: Creating hibernation image: Dec 12 13:22:13 t44 kernel: PM: Need to copy 135989 pages Dec 12 13:22:12 t44 kernel: PM: Restoring platform NVS memory Dec 12 13:22:12 t44 kernel: Enabling non-boot CPUs ... Dec 12 13:22:12 t44 kernel: x86: Booting SMP configuration: Dec 12 13:22:12 t44 kernel: smpboot: Booting Node 0 Processor 1 APIC 0x1 Dec 12 13:22:12 t44 kernel: PAX: PCID detected Dec 12 13:22:12 t44 kernel: PAX: strong UDEREF enabled Dec 12 13:22:12 t44 kernel: PAX: INVPCID detected Dec 12 13:22:12 t44 kernel: kvm: enabling virtualization on CPU1 Dec 12 13:22:12 t44 kernel: CPU1 is up Dec 12 13:22:12 t44 kernel: smpboot: Booting Node 0 Processor 2 APIC 0x2 Dec 12 13:22:13 t44 kernel: PAX: PCID detected Dec 12 13:22:13 t44 kernel: PAX: strong UDEREF enabled Dec 12 13:22:13 t44 kernel: PAX: INVPCID detected Dec 12 13:22:13 t44 kernel: kvm: enabling virtualization on CPU2 Dec 12 13:22:13 t44 kernel: CPU2 is up Dec 12 13:22:13 t44 kernel: smpboot: Booting Node 0 Processor 3 APIC 0x3 Dec 12 13:22:13 t44 kernel: PAX: PCID detected Dec 12 13:22:13 t44 kernel: PAX: strong UDEREF enabled Dec 12 13:22:13 t44 kernel: PAX: INVPCID detected Dec 12 13:22:13 t44 kernel: kvm: enabling virtualization on CPU3 Dec 12 13:22:13 t44 kernel: CPU3 is up Dec 12 13:22:13 t44 kernel: ACPI: Waking up from system sleep state S4 Dec 12 13:22:13 t44 kernel: thinkpad_acpi: EC reports that Thermal Table has changed Dec 12 13:22:13 t44 kernel: acpi LNXPOWER:02: Turning OFF Dec 12 13:22:13 t44 kernel: PM: noirq restore of devices complete after 11.039 msecs Dec 12 13:22:13 t44 kernel: [ cut here ] Dec 12 13:22:13 t44 kernel: WARNING: CPU: 3 PID: 2750 at drivers/gpu/drm/i915/intel_uncore.c:528 hsw_unclaimed_reg_debug.isra.11+0x8d/0xa0 [i915]() Dec 12 13:22:13 t44 kernel: Unclaimed register detected before reading register 0x130040 Dec 12 13:22:13 t44 kernel: Modules linked in: ctr ccm ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_multiport nf_log_ipv4 nf_log_common xt_LOG xt_limit ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_tcpudp xt_recent xt_conntrack iptable_filter ip_tables x_tables nf_conntrack_ftp nf_conntrack af_packet hid_generic hid_cherry usbhid hid uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_core v4l2_common videodev arc4 snd_hda_codec_generic x86_pkg_temp_thermal coretemp kvm_intel evdev iTCO_wdt kvm iwlmvm aesni_intel aes_x86_64 mac80211 glue_helper lrw gf128mul ablk_helper cryptd psmouse iwlwifi atkbd cfg80211 thermal wmi i915 cfbfillrect fbcon thinkpad_acpi cfbimgblt bitblit softcursor nvram font i2c_algo_bit rfkill cfbcopyarea tpm_tis tpm battery ac drm_kms_helper snd_hda_intel video snd_hda_controller drm Dec 12 13:22:13 t44 kernel: intel_gtt e1000e i2c_i801 agpgart ehci_pci xhci_hcd snd_hda_codec ptp ehci_hcd button snd_pcm fb i2c_core fbdev pps_core processor lpc_ich snd_timer usbcore mfd_core snd thermal_sys usb_common soundcore hwmon [last unloaded: microcode] Dec 12 13:22:13 t44 kernel: CPU: 3 PID: 2750 Comm: kworker/u8:6 Not tainted 3.17.6-hardened #7 Dec 12 13:22:13 t44 kernel: Hardware name: LENOVO 20AQCTO1WW/20AQCTO1WW, BIOS GJET80WW (2.30 ) 10/20/2014 Dec 12 13:22:13 t44 kernel: Workqueue: events_unbound 81077030 Dec 12 13:22:13 t44 kernel: 818147c7 0009 c9002a6cbb68 Dec 12 13:22:13 t44 kernel: 815092b5 c9002a6cbbb0 c9002a6cbba0 810527bd Dec 12 13:22:13 t44 kernel: c0614c50 0210 c0614c78 88032cc80060 Dec 12 13:22:13 t44 kernel: Call Trace: Dec 12 13:22:13 t44 kernel: [] dump_stack+0x45/0x5c Dec 12 13:22:13 t44 kernel: [] warn_slowpath_common+0x7d/0xa0 Dec 12 13:22:13 t44 kernel: [] ? i915_ioctls+0x4db0/0x16b28 [i915] Dec 12 13:22:13 t44 kernel: [] ? i915_ioctls+0x4dd8/0x16b28 [i915] Dec 12 13:22:13 t44 k
[RFC] drm/i915: tame the chattermouth
On Fri, Dec 12, 2014 at 5:57 AM, Jani Nikula wrote: > On Fri, 12 Dec 2014, Chris Wilson wrote: >> On Fri, Dec 12, 2014 at 08:17:23AM +0100, Daniel Vetter wrote: >>> On Thu, Dec 11, 2014 at 06:18:12PM -0500, Rob Clark wrote: >>> > Many distro's have mechanism in place to collect and automatically file >>> > bugs for failed WARN()s. And since every third line in i915 is a WARN() > > Come on Rob, that's not necessary. sorry, I didn't intend that to be mean, so much as a bit tongue-in-cheek :-P i915 currently leads the rhel7 and fedora abrt leaderboard, but I realized if you divide the # of abrts by the # of WARN's in the driver code, i915 looks a lot better :-P >>> > it generates quite a lot of noise which is somewhat disconcerting to the >>> > end user. >>> > >>> > Separate out the internal hw-is-in-the-state-I-expected checks into >>> > I915_WARN()s and allow configuration via i915.verbose_checks module >>> > param about whether this will generate a full blown stacktrace or just >>> > DRM_ERROR(). >>> > >>> > Signed-off-by: Rob Clark >>> >>> Yeah I guess makes sense, although I still claim that these are as much >>> "we've lost track of shit" bugs as when a refcount underflows or a pointer >>> is NULL when it shouldn't. But I also agree that we've done a stellar job >>> this year at not locking at these bugs, so meh. >> >> How about making the state checker WARNs conditional on drm.debug&2? The >> premise is that they are often tantalising unhelpful without the debug >> log, so only show them when we have both? > > As I suggested in an internal mail just days ago, make the checks emit a > DRM_ERROR normally (but do log something!), and a WARN if drm.debug & > DRM_UT_KMS (or DRM_UT_DRIVER like Chris suggests) holds. > > I object to adding new kernel parameters for this. We have enough, and > our bug fixing efforts really don't need another round of "oh please > *also* enable this new parameter we added". I did kinda want to keep it as a separate param (or at least a separate drm.debug bit, but I preferred a param as we could more easily keep the default to 'true'), since for rawhide and people running their own kernels I do still want to get this "things-are-not-quite-in-the-state-I-expected" feedback to you. But at any rate, as long as we separate out the hw-state-check WARNs from the actually something-is-about-to-catch-fire WARNs, it is easy enough to patch the macro definitions in a distro kernel. I had some half-baked idea that in the DRM_ERROR case, the message dumped out should contain some string we can search for, so that we could eventually have some "enable sending anonymous driver feedback" type option in the distro, which would still scan the kernel logs for this string and upload feedback via some non-abrt mechanism. (And so, in case the user is actually seeing a problem, when we ask them to attach logs, we still easily see this information. I think in most cases the full callstack doesn't add too much value, so much as knowing what assumptions were broken.) No real idea how that would work from the infrastructure side, so I didn't try to add anything yet, but seems like it could be useful. BR, -R > BR, > Jani. > > >> -Chris >> >> -- >> Chris Wilson, Intel Open Source Technology Centre >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Jani Nikula, Intel Open Source Technology Center
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 6:27 AM, Daniel Stone wrote: > Hey, > > On 10 December 2014 at 17:17, Rob Clark wrote: >> >> In DRM/KMS we are lacking a good way to deal with tiled/compressed >> formats. Especially in the case of dmabuf/prime buffer sharing, where >> we cannot always rely on under-the-hood flags passed to driver specific >> gem-create ioctl to pass around these extra flags. >> >> The proposal is to add a per-plane format modifier. This allows to, if >> necessary, use different tiling patters for sub-sampled planes, etc. >> The format modifiers are added at the end of the ioctl struct, so for >> legacy userspace it will be zero padded. > > > Cool, thanks a lot for looking at this! My one comment is that we could > maybe go even further: keep the current modifier as a strict pixel-layout > modifier (e.g. tiled, compressed - anything that affects how you actually > determine pixel location), and then support an extra (perhaps > non-vendor-namespaced) argument for optional pixel _interpretation_ > modifiers, e.g. the hints in EGL_EXT_image_dma_buf_import. V4L2 is starting > to properly attack things like chroma siting, and being able to specify > narrow/wide YUV range is pretty important for STB/DTV in particular. And > they're actually starting to move to KMS, too ... Up until now I had sort of thought of things like YUV range as plane properties which could be updated (potentially on flip as part of atomic ioctl). But they are also additional metadata about how to properly interpret the pixel data contained in the buffer. I guess chroma siting and YUV range would at least be one value that applies across all the bos/planes of the fb, rather than per plane? It does make me briefly think of just letting us set properties on fb objects :-P (but maybe that is a bit overkill) I suppose the semi-custom plane property approach is a bit easier to extend-as-we-go, and we already have a mechanism so userspace can query about what is actually supported. But this does feel a bit more like attributes of the fb. I'm interested if anyone has particularly good arguments one way or another. > It might be useful to make the interpretation modifiers bitmaskable, so they > can be combined (e.g. wide-range/unclamped YUV | whatever chroma siting), > but I can't think of a usecase for combining multiple layout modifiers (e.g. > this tiling | that compression). > Yeah, I think the vendor-range part of the token, the vendor would probably want to define as a bitmask or set of bitfields so that they could have things like tiled+compressed (otoh, if you try to organize it too nicely now, eventually enough hw generations in the future that scheme will break down.. so maybe a big switch of #define cases is better than trying to interpret the modifier token) >> >> TODO how best to deal with assignment of modifier token values? The >> rough idea was to namespace things with an 8bit vendor-id, and then >> beyond that it is treated as an opaque value. But that was a relatively >> arbitrary choice. There are cases where same tiling pattern and/or >> compression is supported by various different vendors. So we should >> standardize to use the vendor-id and value of the first one who >> documents the format? > > > Yeah, I'd second all of danvet's comments here, as well as adding a new > ADDFB2_MODIFIERS cap + query for supported modifiers. Makes life much easier > for generic userspace. I've locally made a few tweaks (64b and move some stuff to drm_fourcc.h).. I was kicking around the idea of letting plane specify an array of supported format modifiers, and adding this to getplane ioctl, as an alternative to a cap. That plus wiring up some checking to disallow addfb2 for a format + modifiers not supported by at least one plane. Although some hw could only support certain tiling patterns for certain layers of an fb (ie. luma vs chroma). So I may scrap that idea and just go back to cap. BR, -R > > Cheers, > Daniel
[Bug 88481] Kernel oops leaves system without graphical output [radeon]
https://bugzilla.kernel.org/show_bug.cgi?id=88481 --- Comment #14 from Alex Deucher --- It should be fixed in .5 and .6. http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-3.17.y&id=68814ef48cc23572b14b494216598d3116f71096 -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 87265] display turned off during normal work, radeon GPU lockup
0: fence driver on ring 1 use gpu addr 0x8c04 and cpu addr 0x88022f587c04 Dec 12 13:15:43 titan kernel: radeon :01:00.0: fence driver on ring 2 use gpu addr 0x8c08 and cpu addr 0x88022f587c08 Dec 12 13:15:43 titan kernel: radeon :01:00.0: fence driver on ring 3 use gpu addr 0x8c0c and cpu addr 0x88022f587c0c Dec 12 13:15:43 titan kernel: radeon :01:00.0: fence driver on ring 4 use gpu addr 0x8c10 and cpu addr 0x88022f587c10 Dec 12 13:15:43 titan kernel: radeon :01:00.0: fence driver on ring 5 use gpu addr 0x00075a18 and cpu addr 0xc90005635a18 Dec 12 13:15:43 titan kernel: [drm] ring test on 0 succeeded in 4 usecs Dec 12 13:15:43 titan kernel: [drm] ring test on 1 succeeded in 1 usecs Dec 12 13:15:43 titan kernel: [drm] ring test on 2 succeeded in 1 usecs Dec 12 13:15:43 titan kernel: [drm] ring test on 3 succeeded in 5 usecs Dec 12 13:15:43 titan kernel: [drm] ring test on 4 succeeded in 5 usecs Dec 12 13:15:43 titan kernel: [drm] ring test on 5 succeeded in 2 usecs Dec 12 13:15:43 titan kernel: [drm] UVD initialized successfully. Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) [mi] EQ overflow continuing. 300 events have been dropped. Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) Backtrace: Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 0: /usr/libexec/Xorg.bin (QueueKeyboardEvents+0x52) [0x450302] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 1: /usr/libexec/Xorg.bin (xf86PostKeyboardEvent+0x44) [0x488004] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 2: /usr/lib64/xorg/modules/input/evdev_drv.so (_init+0x2e87) [0x7f16eebd1697] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 3: /usr/lib64/xorg/modules/input/evdev_drv.so (_init+0x3505) [0x7f16eebd29a5] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 4: /usr/libexec/Xorg.bin (DPMSSupported+0xe8) [0x4772d8] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 5: /usr/libexec/Xorg.bin (xf86SerialModemClearBits+0x277) [0x4a1cd7] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 6: /lib64/libc.so.6 (__restore_rt+0x0) [0x7f16f437494f] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 7: /usr/libexec/Xorg.bin (OsCleanup+0x20) [0x59c360] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 8: /lib64/libc.so.6 (__restore_rt+0x0) [0x7f16f437494f] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 9: /lib64/libpthread.so.0 (pthread_cond_wait+0xc0) [0x7f16f4130590] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 10: /usr/lib64/dri/radeonsi_dri.so (nouveau_drm_screen_create+0x10aad3) [0x7f16ed512f03] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 11: /usr/lib64/dri/radeonsi_dri.so (nouveau_drm_screen_create+0x10b840) [0x7f16ed514730] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 12: /usr/lib64/dri/radeonsi_dri.so (radeon_drm_winsys_create+0xbe524) [0x7f16ed587084] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 13: /usr/lib64/dri/radeonsi_dri.so (__driDriverGetExtensions_vmwgfx+0x150294) [0x7f16ed279134] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 14: /usr/lib64/xorg/modules/libglamoregl.so (glamor_block_handler+0x51) [0x7f16ee62ace1] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 15: /usr/libexec/Xorg.bin (_CallCallbacks+0x34) [0x43e454] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 16: /usr/libexec/Xorg.bin (FlushAllOutput+0x2b) [0x59b4fb] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 17: /usr/libexec/Xorg.bin (SendErrorToClient+0x16f) [0x438eaf] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 18: /usr/libexec/Xorg.bin (remove_fs_handlers+0x416) [0x43d196] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 19: /lib64/libc.so.6 (__libc_start_main+0xf0) [0x7f16f435ffe0] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 20: /usr/libexec/Xorg.bin (_start+0x29) [0x42761e] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) 21: ? (?+0x29) [0x29] Dec 12 13:16:21 titan gdm-Xorg-:0[741]: (EE) Dec 12 13:16:27 titan kernel: SysRq : Keyboard mode set to system default ... (and then nothing important, just reboot using sysrq) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/95a81b3f/attachment-0001.html>
[pull] radeon drm-next-3.19
Hi Dave, Just three small bug fixes for 3.19 The following changes since commit b59f78228ca3d81ecb491fb17d348b07002dbe03: Merge tag 'drm-intel-next-fixes-2014-12-11' of git://anongit.freedesktop.org/drm-intel into drm-next (2014-12-12 11:39:49 +1000) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-next-3.19 for you to fetch changes up to ac609761ad39b8a12c0601f0a3f7bec42004c9aa: drm/radeon: properly filter DP1.2 4k modes on non-DP1.2 hw (2014-12-12 09:25:29 -0500) Alex Deucher (3): drm/radeon: KV has three PPLLs (v2) drm/radeon: fix sad_count check for dce3 drm/radeon: properly filter DP1.2 4k modes on non-DP1.2 hw drivers/gpu/drm/radeon/atombios_crtc.c | 8 drivers/gpu/drm/radeon/atombios_dp.c | 4 drivers/gpu/drm/radeon/dce3_1_afmt.c | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-)
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 08:50:18AM -0500, Rob Clark wrote: > On Fri, Dec 12, 2014 at 6:27 AM, Daniel Stone wrote: > > Hey, > > > > On 10 December 2014 at 17:17, Rob Clark wrote: > >> > >> In DRM/KMS we are lacking a good way to deal with tiled/compressed > >> formats. Especially in the case of dmabuf/prime buffer sharing, where > >> we cannot always rely on under-the-hood flags passed to driver specific > >> gem-create ioctl to pass around these extra flags. > >> > >> The proposal is to add a per-plane format modifier. This allows to, if > >> necessary, use different tiling patters for sub-sampled planes, etc. > >> The format modifiers are added at the end of the ioctl struct, so for > >> legacy userspace it will be zero padded. > > > > > > Cool, thanks a lot for looking at this! My one comment is that we could > > maybe go even further: keep the current modifier as a strict pixel-layout > > modifier (e.g. tiled, compressed - anything that affects how you actually > > determine pixel location), and then support an extra (perhaps > > non-vendor-namespaced) argument for optional pixel _interpretation_ > > modifiers, e.g. the hints in EGL_EXT_image_dma_buf_import. V4L2 is starting > > to properly attack things like chroma siting, and being able to specify > > narrow/wide YUV range is pretty important for STB/DTV in particular. And > > they're actually starting to move to KMS, too ... > > Up until now I had sort of thought of things like YUV range as plane > properties which could be updated (potentially on flip as part of > atomic ioctl). But they are also additional metadata about how to > properly interpret the pixel data contained in the buffer. > > I guess chroma siting and YUV range would at least be one value that > applies across all the bos/planes of the fb, rather than per plane? There is the DV case where the chroma is sampled at different points for Cb and Cr. So we could in theory specify chroma siting per-plane. But it seems to me that it'd be enough to have it for the entire fb. I had some ideas posted years ago. Here's [1] one at least. [1] http://lists.freedesktop.org/archives/dri-devel/2011-November/016379.html > > It does make me briefly think of just letting us set properties on fb > objects :-P (but maybe that is a bit overkill) Yeah I had the same idea at some point. But then I decided that we could just have these as properties on the plane. > > I suppose the semi-custom plane property approach is a bit easier to > extend-as-we-go, and we already have a mechanism so userspace can > query about what is actually supported. But this does feel a bit more > like attributes of the fb. I'm interested if anyone has particularly > good arguments one way or another. I guess we could have just specified offset/size/stride as part of the fb and let pixel format and such as properties. That would be a fairly natural line IMO since it would be enough data to do a blit, but not enough to actually interpret the pixel data. But we already went beyond that with pixel formats. So I'm not sure how far we want to go. Also all this chroma siting and colorspace stuff definitely runs into hardware specific limitations so having some way to tell userspace what is possible would be nice as you said. Properties seem a decent match for that. > > > It might be useful to make the interpretation modifiers bitmaskable, so they > > can be combined (e.g. wide-range/unclamped YUV | whatever chroma siting), > > but I can't think of a usecase for combining multiple layout modifiers (e.g. > > this tiling | that compression). > > > > Yeah, I think the vendor-range part of the token, the vendor would > probably want to define as a bitmask or set of bitfields so that they > could have things like tiled+compressed > > (otoh, if you try to organize it too nicely now, eventually enough hw > generations in the future that scheme will break down.. so maybe a big > switch of #define cases is better than trying to interpret the > modifier token) > > >> > >> TODO how best to deal with assignment of modifier token values? The > >> rough idea was to namespace things with an 8bit vendor-id, and then > >> beyond that it is treated as an opaque value. But that was a relatively > >> arbitrary choice. There are cases where same tiling pattern and/or > >> compression is supported by various different vendors. So we should > >> standardize to use the vendor-id and value of the first one who > >> documents the format? > > > > > > Yeah, I'd second all of danvet's comments here, as well as adding a new > > ADDFB2_MODIFIERS cap + query for supported modifiers. Makes life much easier > > for generic userspace. > > I've locally made a few tweaks (64b and move some stuff to drm_fourcc.h).. > > I was kicking around the idea of letting plane specify an array of > supported format modifiers, and adding this to getplane ioctl, as an > alternative to a cap. That plus wiring up some checking to disallow > addfb2 for a format +
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
t; > > Yeah, I think the vendor-range part of the token, the vendor would > > probably want to define as a bitmask or set of bitfields so that they > > could have things like tiled+compressed > > > > (otoh, if you try to organize it too nicely now, eventually enough hw > > generations in the future that scheme will break down.. so maybe a big > > switch of #define cases is better than trying to interpret the > > modifier token) > > > > >> > > >> TODO how best to deal with assignment of modifier token values? The > > >> rough idea was to namespace things with an 8bit vendor-id, and then > > >> beyond that it is treated as an opaque value. But that was a > relatively > > >> arbitrary choice. There are cases where same tiling pattern and/or > > >> compression is supported by various different vendors. So we should > > >> standardize to use the vendor-id and value of the first one who > > >> documents the format? > > > > > > > > > Yeah, I'd second all of danvet's comments here, as well as adding a new > > > ADDFB2_MODIFIERS cap + query for supported modifiers. Makes life much > easier > > > for generic userspace. > > > > I've locally made a few tweaks (64b and move some stuff to > drm_fourcc.h).. > > > > I was kicking around the idea of letting plane specify an array of > > supported format modifiers, and adding this to getplane ioctl, as an > > alternative to a cap. That plus wiring up some checking to disallow > > addfb2 for a format + modifiers not supported by at least one plane. > > Although some hw could only support certain tiling patterns for > > certain layers of an fb (ie. luma vs chroma). So I may scrap that > > idea and just go back to cap. > > Indeed the format specific limitations are problem. Properties can't > handle that. We'd need to have some kind of caps for each plane+format > combination if we want to deal with that. But I don't think we can > still make it handle all the hw limitations, so I'm not sure it's worth > going down this path. > > -- > Ville Syrjälä > Intel OTC > -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/eb565cf6/attachment-0001.html>
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 03:11:02PM +, Daniel Stone wrote: > Hi, > > On 12 December 2014 at 14:56, Ville Syrjälä linux.intel.com> > wrote: > > > > On Fri, Dec 12, 2014 at 08:50:18AM -0500, Rob Clark wrote: > > > > It does make me briefly think of just letting us set properties on fb > > > > objects :-P (but maybe that is a bit overkill) > > > > Yeah I had the same idea at some point. But then I decided that we could > > just have these as properties on the plane. > > > Mm, it does seem a bit weird. Yes, they are relative to how the plane > interprets things, but then again so is the format surely. Not to mention > another thing to go wrong, if someone forgets to set and/or clear it when > changing the plane. Keeping it in the fb eliminates that possibility. > > > > > I suppose the semi-custom plane property approach is a bit easier to > > > extend-as-we-go, and we already have a mechanism so userspace can > > > query about what is actually supported. But this does feel a bit more > > > like attributes of the fb. I'm interested if anyone has particularly > > > good arguments one way or another. > > > > I guess we could have just specified offset/size/stride as part of the > > fb and let pixel format and such as properties. That would be a fairly > > natural line IMO since it would be enough data to do a blit, but not > > enough to actually interpret the pixel data. But we already went beyond > > that with pixel formats. So I'm not sure how far we want to go. > > > > Also all this chroma siting and colorspace stuff definitely runs into > > hardware specific limitations so having some way to tell userspace what > > is possible would be nice as you said. Properties seem a decent match for > > that. > > > Yeah, that's a good idea actually, especially since different planes do > have different capabilities. > > > > > > It might be useful to make the interpretation modifiers bitmaskable, > > so they > > > > can be combined (e.g. wide-range/unclamped YUV | whatever chroma > > siting), > > > > but I can't think of a usecase for combining multiple layout modifiers > > (e.g. > > > > this tiling | that compression). > > > > > > Yeah, I think the vendor-range part of the token, the vendor would > > > probably want to define as a bitmask or set of bitfields so that they > > > could have things like tiled+compressed > > > > > > (otoh, if you try to organize it too nicely now, eventually enough hw > > > generations in the future that scheme will break down.. so maybe a big > > > switch of #define cases is better than trying to interpret the > > > modifier token) > > > > Having them separated is still kinda nice though, for the same rationale as > the EGLImage import extension having them as hints. If your hardware > doesn't support the tiling/compression format you use, then you're going to > be showing absolute garbage. But if it doesn't support your exact > chroma-siting or YUV range request, it'll still be totally viewable, just > not entirely perfect. So I don't see the logic in failing these. Well, it will look nasty when switching between GL and display composition the GL path does the right thing an display path doesn't/ And we already have that problem with the fuzzy alignment/scaling restriction stuff. So I think we will want some kind of strict flag somewhere to allow the user to specify that they'd rather fail the whole thing and fall back to GL rather than annoy the user. But for some simpler cases like Xv it would seem perfectly OK to use the less strict rules. Well, unless someone implements Xv in a way that can also transparently switch between display planes and GL/software rendering. > > > > > >> TODO how best to deal with assignment of modifier token values? The > > > >> rough idea was to namespace things with an 8bit vendor-id, and then > > > >> beyond that it is treated as an opaque value. But that was a > > relatively > > > >> arbitrary choice. There are cases where same tiling pattern and/or > > > >> compression is supported by various different vendors. So we should > > > >> standardize to use the vendor-id and value of the first one who > > > >> documents the format? > > > > > > > > > > > > Yeah, I'd second all of danvet's comments here, as well as adding a new > > > > ADDFB2_MODIFIERS cap + query for supported modifiers. Makes life much > > easier > > > > for generic userspace. > > > > > > I've locally made a few tweaks (64b and move some stuff to > > drm_fourcc.h).. > > > > > > I was kicking around the idea of letting plane specify an array of > > > supported format modifiers, and adding this to getplane ioctl, as an > > > alternative to a cap. That plus wiring up some checking to disallow > > > addfb2 for a format + modifiers not supported by at least one plane. > > > Although some hw could only support certain tiling patterns for > > > certain layers of an fb (ie. luma vs chroma). So I may scrap that > > > idea and just go back to cap. > > > > Indeed the format spec
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 10:11 AM, Daniel Stone wrote: > Hi, > > On 12 December 2014 at 14:56, Ville Syrjälä linux.intel.com> > wrote: >> >> On Fri, Dec 12, 2014 at 08:50:18AM -0500, Rob Clark wrote: >> >> > It does make me briefly think of just letting us set properties on fb >> >> > objects :-P (but maybe that is a bit overkill) >> >> Yeah I had the same idea at some point. But then I decided that we could >> just have these as properties on the plane. > > > Mm, it does seem a bit weird. Yes, they are relative to how the plane > interprets things, but then again so is the format surely. Not to mention > another thing to go wrong, if someone forgets to set and/or clear it when > changing the plane. Keeping it in the fb eliminates that possibility. > yeah.. logically it seems nicer for them to be prop's on fb's. The drawback is having to invent some bit of infrastructure to support that. Avoidance of inheriting someone else's plane prop's might be enough justification to invent that infrastructure. But fb prop's don't really help w/ the whole not-all-planes-are-the-same thing.. >> >> > I suppose the semi-custom plane property approach is a bit easier to >> > extend-as-we-go, and we already have a mechanism so userspace can >> > query about what is actually supported. But this does feel a bit more >> > like attributes of the fb. I'm interested if anyone has particularly >> > good arguments one way or another. >> >> I guess we could have just specified offset/size/stride as part of the >> fb and let pixel format and such as properties. That would be a fairly >> natural line IMO since it would be enough data to do a blit, but not >> enough to actually interpret the pixel data. But we already went beyond >> that with pixel formats. So I'm not sure how far we want to go. >> >> Also all this chroma siting and colorspace stuff definitely runs into >> hardware specific limitations so having some way to tell userspace what >> is possible would be nice as you said. Properties seem a decent match for >> that. > > > Yeah, that's a good idea actually, especially since different planes do have > different capabilities. > >> >> > > It might be useful to make the interpretation modifiers bitmaskable, >> > > so they >> > > can be combined (e.g. wide-range/unclamped YUV | whatever chroma >> > > siting), >> > > but I can't think of a usecase for combining multiple layout modifiers >> > > (e.g. >> > > this tiling | that compression). >> > >> > Yeah, I think the vendor-range part of the token, the vendor would >> > probably want to define as a bitmask or set of bitfields so that they >> > could have things like tiled+compressed >> > >> > (otoh, if you try to organize it too nicely now, eventually enough hw >> > generations in the future that scheme will break down.. so maybe a big >> > switch of #define cases is better than trying to interpret the >> > modifier token) > > > Having them separated is still kinda nice though, for the same rationale as > the EGLImage import extension having them as hints. If your hardware doesn't > support the tiling/compression format you use, then you're going to be > showing absolute garbage. But if it doesn't support your exact chroma-siting > or YUV range request, it'll still be totally viewable, just not entirely > perfect. So I don't see the logic in failing these. oh, sorry, I was just referring to the 'modifier token' stuff.. chroma-siting and YUV range are common enough that I think they should be something separate from the per-plane 'modifer token' >> >> > >> TODO how best to deal with assignment of modifier token values? The >> > >> rough idea was to namespace things with an 8bit vendor-id, and then >> > >> beyond that it is treated as an opaque value. But that was a >> > >> relatively >> > >> arbitrary choice. There are cases where same tiling pattern and/or >> > >> compression is supported by various different vendors. So we should >> > >> standardize to use the vendor-id and value of the first one who >> > >> documents the format? >> > > >> > > >> > > Yeah, I'd second all of danvet's comments here, as well as adding a >> > > new >> > > ADDFB2_MODIFIERS cap + query for supported modifiers. Makes life much >> > > easier >> > > for generic userspace. >> > >> > I've locally made a few tweaks (64b and move some stuff to >> > drm_fourcc.h).. >> > >> > I was kicking around the idea of letting plane specify an array of >> > supported format modifiers, and adding this to getplane ioctl, as an >> > alternative to a cap. That plus wiring up some checking to disallow >> > addfb2 for a format + modifiers not supported by at least one plane. >> > Although some hw could only support certain tiling patterns for >> > certain layers of an fb (ie. luma vs chroma). So I may scrap that >> > idea and just go back to cap. >> >> Indeed the format specific limitations are problem. Properties can't >> handle that. We'd need to have some kind of caps for each plane+format >> combination if
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 10:30 AM, Ville Syrjälä wrote: >> >> Having them separated is still kinda nice though, for the same rationale as >> the EGLImage import extension having them as hints. If your hardware >> doesn't support the tiling/compression format you use, then you're going to >> be showing absolute garbage. But if it doesn't support your exact >> chroma-siting or YUV range request, it'll still be totally viewable, just >> not entirely perfect. So I don't see the logic in failing these. > > Well, it will look nasty when switching between GL and display > composition the GL path does the right thing an display path doesn't/ > And we already have that problem with the fuzzy alignment/scaling > restriction stuff. So I think we will want some kind of strict flag > somewhere to allow the user to specify that they'd rather fail the whole > thing and fall back to GL rather than annoy the user. another argument in favor of plane properties, I think. This way userspace can query what is actually possibly and we don't implicitly give userspace the idea that display hw can handle something that it doesn't.. BR, -R
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 11:00:29AM -0500, Rob Clark wrote: > On Fri, Dec 12, 2014 at 10:30 AM, Ville Syrjälä > wrote: > >> > >> Having them separated is still kinda nice though, for the same rationale as > >> the EGLImage import extension having them as hints. If your hardware > >> doesn't support the tiling/compression format you use, then you're going to > >> be showing absolute garbage. But if it doesn't support your exact > >> chroma-siting or YUV range request, it'll still be totally viewable, just > >> not entirely perfect. So I don't see the logic in failing these. > > > > Well, it will look nasty when switching between GL and display > > composition the GL path does the right thing an display path doesn't/ > > And we already have that problem with the fuzzy alignment/scaling > > restriction stuff. So I think we will want some kind of strict flag > > somewhere to allow the user to specify that they'd rather fail the whole > > thing and fall back to GL rather than annoy the user. > > > another argument in favor of plane properties, I think. This way > userspace can query what is actually possibly and we don't implicitly > give userspace the idea that display hw can handle something that it > doesn't.. Well, we don't have properties to describe a lot of the limitations. I'm not sure we want to add tons of read-only properties for that. And as stated, sometimes the limitations depend on other properties/pixel format/etc. so seems rather hard to describe in a sane way that would actually be useful to userspace. One idea that came up again just yesterday would be to have the kernel assign the planes on behalf of userspace. But that would then mean we need some kind of virtual plane layer on top so that the virtual plane state gets tracked correctly, or userspace would need to pass in the entire state for every display update. Also soon it may start to look like we're implementing some kind of compositor in the kernel. Another other approach might be to implement this plane assignment stuff in libdrm and duplicate some hw specific knowledge there. -- Ville Syrjälä Intel OTC
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 11:14 AM, Ville Syrjälä wrote: > On Fri, Dec 12, 2014 at 11:00:29AM -0500, Rob Clark wrote: >> On Fri, Dec 12, 2014 at 10:30 AM, Ville Syrjälä >> wrote: >> >> >> >> Having them separated is still kinda nice though, for the same rationale >> >> as >> >> the EGLImage import extension having them as hints. If your hardware >> >> doesn't support the tiling/compression format you use, then you're going >> >> to >> >> be showing absolute garbage. But if it doesn't support your exact >> >> chroma-siting or YUV range request, it'll still be totally viewable, just >> >> not entirely perfect. So I don't see the logic in failing these. >> > >> > Well, it will look nasty when switching between GL and display >> > composition the GL path does the right thing an display path doesn't/ >> > And we already have that problem with the fuzzy alignment/scaling >> > restriction stuff. So I think we will want some kind of strict flag >> > somewhere to allow the user to specify that they'd rather fail the whole >> > thing and fall back to GL rather than annoy the user. >> >> >> another argument in favor of plane properties, I think. This way >> userspace can query what is actually possibly and we don't implicitly >> give userspace the idea that display hw can handle something that it >> doesn't.. > > Well, we don't have properties to describe a lot of the limitations. I'm > not sure we want to add tons of read-only properties for that. And as > stated, sometimes the limitations depend on other properties/pixel > format/etc. so seems rather hard to describe in a sane way that would > actually be useful to userspace. sorry, wasn't quite what I meant.. What I meant was that YUV range and siting properties would probably be enum properties, so userspace could see which enum values are supported. r/o props could be a way to deal w/ some limits. Other limits, it could just be a matter of expressing the correct range as we convert things to properties for atomic. > One idea that came up again just yesterday would be to have the kernel > assign the planes on behalf of userspace. But that would then mean we > need some kind of virtual plane layer on top so that the virtual plane > state gets tracked correctly, or userspace would need to pass in the > entire state for every display update. Also soon it may start to look > like we're implementing some kind of compositor in the kernel. Another > other approach might be to implement this plane assignment stuff in > libdrm and duplicate some hw specific knowledge there. I kinda lean towards userspace. I don't want to preclude the case of a smart userspace (which has some driver specific userspace piece).. could be an interesting idea to have something in libdrm. BR, -R > -- > Ville Syrjälä > Intel OTC
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
Hi, On 12 December 2014 at 15:30, Ville Syrjälä wrote: > > On Fri, Dec 12, 2014 at 03:11:02PM +, Daniel Stone wrote: > > On 12 December 2014 at 14:56, Ville Syrjälä < > ville.syrjala at linux.intel.com> > > wrote: > > Having them separated is still kinda nice though, for the same rationale > as > > the EGLImage import extension having them as hints. If your hardware > > doesn't support the tiling/compression format you use, then you're going > to > > be showing absolute garbage. But if it doesn't support your exact > > chroma-siting or YUV range request, it'll still be totally viewable, just > > not entirely perfect. So I don't see the logic in failing these. > > Well, it will look nasty when switching between GL and display > composition the GL path does the right thing an display path doesn't/ > And we already have that problem with the fuzzy alignment/scaling > restriction stuff. So I think we will want some kind of strict flag > somewhere to allow the user to specify that they'd rather fail the whole > thing and fall back to GL rather than annoy the user. > If you're doing it through GL, you've already lost. Either you're doing some magic behind the user's back to bind multi-planar dmabuf-EGLImages to TEXTURE_2D with RGB sampling, or you're binding to TEXTURE_EXTERNAL_OES, which forces you to use linear/nearest filtering. Even if you do use TEXTURE_2D binding, the EGLImage import spec does exactly the same as what's suggested here, and treats them as hints, which the implementation can use or ignore. So far I don't know of any implementation which doesn't ignore them. FWIW, i965 completely disallows multi-planar EGLImage imports in the first place. Mali as shipped on ChromeOS forces you to use TEXTURE_EXTERNAL_OES. S ... > But for some simpler cases like Xv it would seem perfectly OK to use the > less strict rules. Well, unless someone implements Xv in a way that can > also transparently switch between display planes and GL/software rendering. Well sure, if you absolutely want to ensure it works, you're going to need some kind of query. Maybe, if the range/chroma-siting ones were part of a bitmask, you could steal the top bit to mark that the hints are actually requirements, and to fail if you can't respect the hints. > > Well, you don't have to solve literally everything at once. Just having a > > list of formats which could possibly be supported if you did the right > > thing, would be a hell of a lot better than punting to userspace, which > > either a) has to have hardware-specific knowledge in every component > > (compositor, media library, etc etc), or b) brute-force it. The lack of > any > > format query in EGLImage dmabuf import is a serious, serious, serious, > pain > > when trying to do generic userspace (e.g. compositor feeds GStreamer a > list > > of formats which are supported by the hardware). I get that there are > > combinations that could fail, but that's true of everything. At least > > narrowing down the problem space a bit is an enormous help. > > We alredy have a list of supported formats. The problem is when specific > formats impose additonal constraints (eg. more restricted scaling factor > limits). Where's the list of supported formats in this proposal? It just adds a modifier: there's no way to determine which modifiers are supported by a specific plane, which is what I really need to know. Right now, the only way is just brute-forcing your way through every single combination until you find one which succeeds. Like I said, I completely get that there are going to be specific/weird/arbitrary restrictions. There already are, such as scaling factors, maximum one non-primary plane per scanline, global bandwidth limits, etc etc. Those are not something KMS has ever attempted to solve, and I'm not suggesting that the modifier mechanism attempts to solve them. Cheers, Daniel -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/28ffaeaf/attachment.html>
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 05:11:50PM +, Daniel Stone wrote: > Hi, > > On 12 December 2014 at 15:30, Ville Syrjälä linux.intel.com> > wrote: > > > > On Fri, Dec 12, 2014 at 03:11:02PM +, Daniel Stone wrote: > > > On 12 December 2014 at 14:56, Ville Syrjälä < > > ville.syrjala at linux.intel.com> > > > wrote: > > > Having them separated is still kinda nice though, for the same rationale > > as > > > the EGLImage import extension having them as hints. If your hardware > > > doesn't support the tiling/compression format you use, then you're going > > to > > > be showing absolute garbage. But if it doesn't support your exact > > > chroma-siting or YUV range request, it'll still be totally viewable, just > > > not entirely perfect. So I don't see the logic in failing these. > > > > Well, it will look nasty when switching between GL and display > > composition the GL path does the right thing an display path doesn't/ > > And we already have that problem with the fuzzy alignment/scaling > > restriction stuff. So I think we will want some kind of strict flag > > somewhere to allow the user to specify that they'd rather fail the whole > > thing and fall back to GL rather than annoy the user. > > > > If you're doing it through GL, you've already lost. Either you're doing > some magic behind the user's back to bind multi-planar dmabuf-EGLImages to > TEXTURE_2D with RGB sampling, or you're binding to TEXTURE_EXTERNAL_OES, > which forces you to use linear/nearest filtering. Even if you do use > TEXTURE_2D binding, the EGLImage import spec does exactly the same as > what's suggested here, and treats them as hints, which the implementation > can use or ignore. So far I don't know of any implementation which doesn't > ignore them. Well anyone who is serious about quality ought to handle that stuff. Or at least make sure both GL/whatever and planes ignore the hints in the same way. So if you GL implementation is lax then you anyway need to have some driver/hardware specific knowledge to know which way to go when using the plane path to get matching output. > > FWIW, i965 completely disallows multi-planar EGLImage imports in the first > place. Mali as shipped on ChromeOS forces you to use TEXTURE_EXTERNAL_OES. > S ... Without the strict flag you probably need to patch the kernel too then to make sure the planes ignore the hint the same way as your GL implementation. Or vice versa. > > But for some simpler cases like Xv it would seem perfectly OK to use the > > less strict rules. Well, unless someone implements Xv in a way that can > > also transparently switch between display planes and GL/software rendering. > > > Well sure, if you absolutely want to ensure it works, you're going to need > some kind of query. Maybe, if the range/chroma-siting ones were part of a > bitmask, you could steal the top bit to mark that the hints are actually > requirements, and to fail if you can't respect the hints. I was more thinking of some global "I want exactly what I said" kind of knob. Maybe as a client cap type of thingy. Another idea I had was to have such a flag in the atomic ioctl. But it seems impossible to handle that in a sane way unless you require the caller to specify the entire state every time the ioctl is called with the flag set. -- Ville Syrjälä Intel OTC
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
Hi, On 12 December 2014 at 18:00, Ville Syrjälä wrote: > > On Fri, Dec 12, 2014 at 05:11:50PM +, Daniel Stone wrote: > > If you're doing it through GL, you've already lost. Either you're doing > > some magic behind the user's back to bind multi-planar dmabuf-EGLImages > to > > TEXTURE_2D with RGB sampling, or you're binding to TEXTURE_EXTERNAL_OES, > > which forces you to use linear/nearest filtering. Even if you do use > > TEXTURE_2D binding, the EGLImage import spec does exactly the same as > > what's suggested here, and treats them as hints, which the implementation > > can use or ignore. So far I don't know of any implementation which > doesn't > > ignore them. > > Well anyone who is serious about quality ought to handle that stuff. > Or at least make sure both GL/whatever and planes ignore the hints in > the same way. So if you GL implementation is lax then you anyway need > to have some driver/hardware specific knowledge to know which way to go > when using the plane path to get matching output. > Anyone who's serious about quality and is also using GL for video, is not serious about quality. Or accurate timing. > > > But for some simpler cases like Xv it would seem perfectly OK to use > the > > > less strict rules. Well, unless someone implements Xv in a way that can > > > also transparently switch between display planes and GL/software > rendering. > > > > Well sure, if you absolutely want to ensure it works, you're going to > need > > some kind of query. Maybe, if the range/chroma-siting ones were part of a > > bitmask, you could steal the top bit to mark that the hints are actually > > requirements, and to fail if you can't respect the hints. > > I was more thinking of some global "I want exactly what I said" kind > of knob. Maybe as a client cap type of thingy. > I like the idea of keeping it local to the chroma-siting/range hints, because it makes it far more clear exactly what it affects. Cheers, Daniel -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/e08e6c44/attachment-0001.html>
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 06:05:49PM +, Daniel Stone wrote: > Hi, > > On 12 December 2014 at 18:00, Ville Syrjälä linux.intel.com> > wrote: > > > > On Fri, Dec 12, 2014 at 05:11:50PM +, Daniel Stone wrote: > > > If you're doing it through GL, you've already lost. Either you're doing > > > some magic behind the user's back to bind multi-planar dmabuf-EGLImages > > to > > > TEXTURE_2D with RGB sampling, or you're binding to TEXTURE_EXTERNAL_OES, > > > which forces you to use linear/nearest filtering. Even if you do use > > > TEXTURE_2D binding, the EGLImage import spec does exactly the same as > > > what's suggested here, and treats them as hints, which the implementation > > > can use or ignore. So far I don't know of any implementation which > > doesn't > > > ignore them. > > > > Well anyone who is serious about quality ought to handle that stuff. > > Or at least make sure both GL/whatever and planes ignore the hints in > > the same way. So if you GL implementation is lax then you anyway need > > to have some driver/hardware specific knowledge to know which way to go > > when using the plane path to get matching output. > > > > Anyone who's serious about quality and is also using GL for video, is not > serious about quality. Or accurate timing. You're too hung up on the "GL" there. It doesn't actually matter what you use to render the video when not using the display hardware. The same problem remains. > > > > > > But for some simpler cases like Xv it would seem perfectly OK to use > > the > > > > less strict rules. Well, unless someone implements Xv in a way that can > > > > also transparently switch between display planes and GL/software > > rendering. > > > > > > Well sure, if you absolutely want to ensure it works, you're going to > > need > > > some kind of query. Maybe, if the range/chroma-siting ones were part of a > > > bitmask, you could steal the top bit to mark that the hints are actually > > > requirements, and to fail if you can't respect the hints. > > > > I was more thinking of some global "I want exactly what I said" kind > > of knob. Maybe as a client cap type of thingy. > > > > I like the idea of keeping it local to the chroma-siting/range hints, > because it makes it far more clear exactly what it affects. You're not thinking wide enough. We would need to add similar hints to pretty much every property. -- Ville Syrjälä Intel OTC
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Wednesday 10 December 2014 18:30:10 Daniel Vetter wrote: > On Wed, Dec 10, 2014 at 12:17:51PM -0500, Rob Clark wrote: > > In DRM/KMS we are lacking a good way to deal with tiled/compressed > > formats. Especially in the case of dmabuf/prime buffer sharing, where > > we cannot always rely on under-the-hood flags passed to driver specific > > gem-create ioctl to pass around these extra flags. > > > > The proposal is to add a per-plane format modifier. This allows to, if > > necessary, use different tiling patters for sub-sampled planes, etc. > > The format modifiers are added at the end of the ioctl struct, so for > > legacy userspace it will be zero padded. > > > > TODO how best to deal with assignment of modifier token values? The > > rough idea was to namespace things with an 8bit vendor-id, and then > > beyond that it is treated as an opaque value. But that was a relatively > > arbitrary choice. There are cases where same tiling pattern and/or > > compression is supported by various different vendors. So we should > > standardize to use the vendor-id and value of the first one who > > documents the format? > > 8bits should be enough, will take a while until we have more than 250 gpu > drivers in the linux kernel ;-) I'm leaning a bit towards using 64bits > though to make sure that there's enough space in the bitfiel to encode > substrides and stuff like that, in case anyone needs it. For vendor ids > I'd just go with first come and starting at 1 (i.e. rename yours). That > way we make it clear that until a patch is merged upstream the id isn't > reserved yet. drm-next should be sufficient as registry imo. > > > TODO move definition of tokens to drm_fourcc.h? > > Seems orthogonal imo. Another todo is to add checking to all drivers to > reject it if it's not 0 with -EINVAL. Otherwise we have yet another case > of an ioctl with fields that can't actually be used everywhere. Could we please add the check in core code instead of drivers ? > But yeah I like this and in i915 we definitely need this. skl adds a bunch > of framebuffer layouts where we need to spec tiling in more detail. > > Overall I like. > > > Signed-off-by: Rob Clark > > --- > > > > include/uapi/drm/drm_mode.h | 30 ++ > > 1 file changed, 30 insertions(+) -- Regards, Laurent Pinchart
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
Hi Rob,
On Wednesday 10 December 2014 12:17:51 Rob Clark wrote:
> In DRM/KMS we are lacking a good way to deal with tiled/compressed
> formats. Especially in the case of dmabuf/prime buffer sharing, where
> we cannot always rely on under-the-hood flags passed to driver specific
> gem-create ioctl to pass around these extra flags.
>
> The proposal is to add a per-plane format modifier. This allows to, if
> necessary, use different tiling patters for sub-sampled planes, etc.
> The format modifiers are added at the end of the ioctl struct, so for
> legacy userspace it will be zero padded.
But it will change the size of the structure, and thus the ioctl value. You
can't extend existing structures used in ioctls I'm afraid.
By the way, is thus calls for an addfb3, I would add reserved fields at the
end of the structure to make future extensions possible without a new ioctl.
> TODO how best to deal with assignment of modifier token values? The
> rough idea was to namespace things with an 8bit vendor-id, and then
> beyond that it is treated as an opaque value. But that was a relatively
> arbitrary choice. There are cases where same tiling pattern and/or
> compression is supported by various different vendors. So we should
> standardize to use the vendor-id and value of the first one who
> documents the format?
>
> TODO move definition of tokens to drm_fourcc.h?
>
> Signed-off-by: Rob Clark
> ---
> include/uapi/drm/drm_mode.h | 30 ++
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index aae71cb..c43648c 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -330,6 +330,28 @@ struct drm_mode_fb_cmd {
>
> #define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */
>
> +/*
> + * Format Modifiers:
> + *
> + * Format modifiers describe, typically, a re-ordering or modification
> + * of the data in a plane of an FB. This can be used to express tiled/
> + * swizzled formats, or compression, or a combination of the two.
> + *
> + * The upper 8 bits of the format modifier are a vendor-id as assigned
> + * below. The lower 24 bits are assigned as vendor sees fit.
> + */
> +
> +/* Vendor Ids: */
> +#define DRM_FOURCC_MOD_VENDOR_SAMSUNG 0x03
> +/* ... more */
> +
> +#define DRM_FOURCC_MOD(vendor, name) (((DRM_FOURCC_MOD_VENDOR_## vendor) <<
> 24) | val)
> +
> +/* Modifier values: */
> +/* 64x32 macroblock, ie NV12MT: */
> +#define DRM_FOURCC_MOD_SAMSUNG_64_32_TILE DRM_FOURCC_MOD(SAMSUNG, 123)
> +/* ... more */
> +
> struct drm_mode_fb_cmd2 {
> __u32 fb_id;
> __u32 width, height;
> @@ -349,10 +371,18 @@ struct drm_mode_fb_cmd2 {
>* So it would consist of Y as offsets[0] and UV as
>* offsets[1]. Note that offsets[0] will generally
>* be 0 (but this is not required).
> + *
> + * To accommodate tiled, compressed, etc formats, a per-plane
> + * modifier can be specified. The default value of zero
> + * indicates "native" format as specified by the fourcc.
> + * Vendor specific modifier token. This allows, for example,
> + * different tiling/swizzling pattern on different planes.
> + * See discussion above of DRM_FOURCC_MOD_xxx.
>*/
> __u32 handles[4];
> __u32 pitches[4]; /* pitch for each plane */
> __u32 offsets[4]; /* offset of each plane */
> + __u32 modifier[4]; /* ie, tiling, compressed (per plane) */
> };
>
> #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01
--
Regards,
Laurent Pinchart
[Bug 73338] Fan speed in idle at 40% with radeonsi and at 18% with catalyst
https://bugs.freedesktop.org/show_bug.cgi?id=73338 --- Comment #67 from Chernovsky Oleg --- Alex, more comments, now on SI impl >#defineCG_FDO_CTRL00x754 >#defineCG_FDO_CTRL10x758 >#defineCG_FDO_CTRL20x75C Are you sure they're 0x75* and not 0x76*? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/86cdfe0f/attachment.html>
[Bug 73338] Fan speed in idle at 40% with radeonsi and at 18% with catalyst
https://bugs.freedesktop.org/show_bug.cgi?id=73338 --- Comment #68 from Chernovsky Oleg --- (In reply to Martin Peres from comment #66) Martin, there're things I think I didn't get correctly: - What should I return if pwm1_enable is 0 and I'm trying to push 50 into pwm1? - Can pwm1_enabled value be 2 after I push 0 to it? Radeon default fan value comes from fuse and it's not 100% -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/4c683c78/attachment.html>
[RFC] drm: add support for tiled/compressed/etc modifier in addfb2
On Fri, Dec 12, 2014 at 3:57 PM, Laurent Pinchart
wrote:
> Hi Rob,
>
> On Wednesday 10 December 2014 12:17:51 Rob Clark wrote:
>> In DRM/KMS we are lacking a good way to deal with tiled/compressed
>> formats. Especially in the case of dmabuf/prime buffer sharing, where
>> we cannot always rely on under-the-hood flags passed to driver specific
>> gem-create ioctl to pass around these extra flags.
>>
>> The proposal is to add a per-plane format modifier. This allows to, if
>> necessary, use different tiling patters for sub-sampled planes, etc.
>> The format modifiers are added at the end of the ioctl struct, so for
>> legacy userspace it will be zero padded.
>
> But it will change the size of the structure, and thus the ioctl value. You
> can't extend existing structures used in ioctls I'm afraid.
Actually, that is why it will work. Old userspace passes smaller
size, drm_ioctl() zero pads the difference..
The issue is (potentially) in the other direction (new userspace, old
kernel) since the old kernel will ignore the new fields. But that can
be sorted w/ a cap/query
BR,
-R
> By the way, is thus calls for an addfb3, I would add reserved fields at the
> end of the structure to make future extensions possible without a new ioctl.
>
>> TODO how best to deal with assignment of modifier token values? The
>> rough idea was to namespace things with an 8bit vendor-id, and then
>> beyond that it is treated as an opaque value. But that was a relatively
>> arbitrary choice. There are cases where same tiling pattern and/or
>> compression is supported by various different vendors. So we should
>> standardize to use the vendor-id and value of the first one who
>> documents the format?
>>
>> TODO move definition of tokens to drm_fourcc.h?
>>
>> Signed-off-by: Rob Clark
>> ---
>> include/uapi/drm/drm_mode.h | 30 ++
>> 1 file changed, 30 insertions(+)
>>
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index aae71cb..c43648c 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -330,6 +330,28 @@ struct drm_mode_fb_cmd {
>>
>> #define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers
>> */
>>
>> +/*
>> + * Format Modifiers:
>> + *
>> + * Format modifiers describe, typically, a re-ordering or modification
>> + * of the data in a plane of an FB. This can be used to express tiled/
>> + * swizzled formats, or compression, or a combination of the two.
>> + *
>> + * The upper 8 bits of the format modifier are a vendor-id as assigned
>> + * below. The lower 24 bits are assigned as vendor sees fit.
>> + */
>> +
>> +/* Vendor Ids: */
>> +#define DRM_FOURCC_MOD_VENDOR_SAMSUNG 0x03
>> +/* ... more */
>> +
>> +#define DRM_FOURCC_MOD(vendor, name) (((DRM_FOURCC_MOD_VENDOR_## vendor) <<
>> 24) | val)
>> +
>> +/* Modifier values: */
>> +/* 64x32 macroblock, ie NV12MT: */
>> +#define DRM_FOURCC_MOD_SAMSUNG_64_32_TILE DRM_FOURCC_MOD(SAMSUNG, 123)
>> +/* ... more */
>> +
>> struct drm_mode_fb_cmd2 {
>> __u32 fb_id;
>> __u32 width, height;
>> @@ -349,10 +371,18 @@ struct drm_mode_fb_cmd2 {
>>* So it would consist of Y as offsets[0] and UV as
>>* offsets[1]. Note that offsets[0] will generally
>>* be 0 (but this is not required).
>> + *
>> + * To accommodate tiled, compressed, etc formats, a per-plane
>> + * modifier can be specified. The default value of zero
>> + * indicates "native" format as specified by the fourcc.
>> + * Vendor specific modifier token. This allows, for example,
>> + * different tiling/swizzling pattern on different planes.
>> + * See discussion above of DRM_FOURCC_MOD_xxx.
>>*/
>> __u32 handles[4];
>> __u32 pitches[4]; /* pitch for each plane */
>> __u32 offsets[4]; /* offset of each plane */
>> + __u32 modifier[4]; /* ie, tiling, compressed (per plane) */
>> };
>>
>> #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01
>
> --
> Regards,
>
> Laurent Pinchart
>
[Bug 73338] Fan speed in idle at 40% with radeonsi and at 18% with catalyst
https://bugs.freedesktop.org/show_bug.cgi?id=73338 --- Comment #69 from Alex Deucher --- (In reply to Chernovsky Oleg from comment #67) > Alex, more comments, now on SI impl > > >#define CG_FDO_CTRL00x754 > >#define CG_FDO_CTRL10x758 > >#define CG_FDO_CTRL20x75C > > Are you sure they're 0x75* and not 0x76*? Yes they are at 0x75*, I just double checked the register database. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/4289caf6/attachment.html>
[Bug 78951] gl_PrimitiveID is zero if no geometry shader is present
https://bugs.freedesktop.org/show_bug.cgi?id=78951 pavol at klacansky.com changed: What|Removed |Added Priority|medium |highest --- Comment #4 from pavol at klacansky.com --- I do not have radeonsi supported device. How does Intel driver support this? I think this is really useful for picking algorithm. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/da2f4edd/attachment-0001.html>
[Bug 73338] Fan speed in idle at 40% with radeonsi and at 18% with catalyst
https://bugs.freedesktop.org/show_bug.cgi?id=73338 --- Comment #70 from Chernovsky Oleg --- (In reply to Alex Deucher from comment #69) > Yes they are at 0x75*, I just double checked the register database. Hmm ok... strange though, for CI the offset from CG_MULT_THERMAL_STATUS to CG_FDO_CTRL0 is 0x50, and from CG_FDO_CTRL0 to CG_TACH_CTRL is 0xC, but for SI they's 0x40 and (again!) 0xC accordingly... -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/1d25eb93/attachment.html>
[Bug 73338] Fan speed in idle at 40% with radeonsi and at 18% with catalyst
https://bugs.freedesktop.org/show_bug.cgi?id=73338 --- Comment #71 from Chernovsky Oleg --- Never mind last comment, just thought about additional registers in the middle of range. I'll have a SI part at the end of December, hope I'll be able to perform some tests on it. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/cf3ff7d3/attachment.html>
[PATCH 1/2] drm/msm: Initial add eDP support in msm drm driver (V3)
This change adds a new eDP connector in msm drm driver. With this
change, eDP panel can work with msm platform under drm framework.
V1: Initial change
V2: Address Rob's comments
Use generated header file for register definitions
Change to devm_* APIs
V3: Address Thierry's comments and rebase on top of atomic changes
Remove edp_bridge_mode_fixup
Remove backlight control code and rely on pwm-backlight
Remove continuous splash screen support for now
Change to gpiod_* APIs
Signed-off-by: Hai Li
---
drivers/gpu/drm/msm/Makefile|6 +
drivers/gpu/drm/msm/edp/edp.c | 210 +
drivers/gpu/drm/msm/edp/edp.h | 84 ++
drivers/gpu/drm/msm/edp/edp_aux.c | 268 ++
drivers/gpu/drm/msm/edp/edp_bridge.c| 124 +++
drivers/gpu/drm/msm/edp/edp_connector.c | 161
drivers/gpu/drm/msm/edp/edp_ctrl.c | 1390 +++
drivers/gpu/drm/msm/edp/edp_phy.c | 106 +++
drivers/gpu/drm/msm/msm_drv.h |6 +
9 files changed, 2355 insertions(+)
create mode 100644 drivers/gpu/drm/msm/edp/edp.c
create mode 100644 drivers/gpu/drm/msm/edp/edp.h
create mode 100644 drivers/gpu/drm/msm/edp/edp_aux.c
create mode 100644 drivers/gpu/drm/msm/edp/edp_bridge.c
create mode 100644 drivers/gpu/drm/msm/edp/edp_connector.c
create mode 100644 drivers/gpu/drm/msm/edp/edp_ctrl.c
create mode 100644 drivers/gpu/drm/msm/edp/edp_phy.c
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 143d988..e5464a0 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -16,6 +16,12 @@ msm-y := \
hdmi/hdmi_phy_8960.o \
hdmi/hdmi_phy_8x60.o \
hdmi/hdmi_phy_8x74.o \
+ edp/edp.o \
+ edp/edp_aux.o \
+ edp/edp_bridge.o \
+ edp/edp_connector.o \
+ edp/edp_ctrl.o \
+ edp/edp_phy.o \
mdp/mdp_format.o \
mdp/mdp_kms.o \
mdp/mdp4/mdp4_crtc.o \
diff --git a/drivers/gpu/drm/msm/edp/edp.c b/drivers/gpu/drm/msm/edp/edp.c
new file mode 100644
index 000..16b8984
--- /dev/null
+++ b/drivers/gpu/drm/msm/edp/edp.c
@@ -0,0 +1,210 @@
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include "edp.h"
+
+static irqreturn_t edp_irq(int irq, void *dev_id)
+{
+ struct msm_edp *edp = dev_id;
+
+ /* Process eDP irq */
+ return msm_edp_ctrl_irq(edp->ctrl);
+}
+
+static void edp_destroy(struct platform_device *pdev)
+{
+ struct msm_edp *edp = platform_get_drvdata(pdev);
+
+ if (!edp)
+ return;
+
+ if (edp->ctrl) {
+ msm_edp_ctrl_destroy(edp->ctrl);
+ edp->ctrl = NULL;
+ }
+
+ platform_set_drvdata(pdev, NULL);
+}
+
+/* construct eDP at bind/probe time, grab all the resources. */
+static struct msm_edp *edp_init(struct platform_device *pdev)
+{
+ struct msm_edp *edp = NULL;
+ int ret;
+
+ if (!pdev) {
+ pr_err("no eDP device\n");
+ ret = -ENXIO;
+ goto fail;
+ }
+
+ edp = devm_kzalloc(&pdev->dev, sizeof(*edp), GFP_KERNEL);
+ if (!edp) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+ DBG("eDP probed=%p", edp);
+
+ edp->pdev = pdev;
+ platform_set_drvdata(pdev, edp);
+
+ ret = msm_edp_ctrl_init(edp);
+ if (ret)
+ goto fail;
+
+ return edp;
+
+fail:
+ if (edp)
+ edp_destroy(pdev);
+
+ return ERR_PTR(ret);
+}
+
+static int edp_bind(struct device *dev, struct device *master, void *data)
+{
+ struct drm_device *drm = dev_get_drvdata(master);
+ struct msm_drm_private *priv = drm->dev_private;
+ struct msm_edp *edp;
+
+ DBG("");
+ edp = edp_init(to_platform_device(dev));
+ if (IS_ERR(edp))
+ return PTR_ERR(edp);
+ priv->edp = edp;
+
+ return 0;
+}
+
+static void edp_unbind(struct device *dev, struct device *master, void *data)
+{
+ struct drm_device *drm = dev_get_drvdata(master);
+ struct msm_drm_private *priv = drm->dev_private;
+
+ DBG("");
+ if (priv->edp) {
+ edp_destroy(to_platform_device(dev));
+ priv->edp = NULL;
+ }
+}
+
+static const struct component_ops edp_ops = {
+ .bind = edp_bind,
+ .unbind = edp_unbind,
+};
+
+static int edp_dev_probe(struct platform_device *pdev)
+{
+ DBG("");
+ return component_add(&pd
[PATCH 2/2] drm/msm: Add the eDP connector in msm drm driver (V2)
Modified the hard-coded hdmi connector/encoder implementations in msm drm
driver to support both edp and hdmi.
V1: Initial change
V2: Address Thierry's change
Signed-off-by: Hai Li
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 38 +++--
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 43 -
drivers/gpu/drm/msm/msm_drv.c | 2 ++
drivers/gpu/drm/msm/msm_drv.h | 6
4 files changed, 74 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
index 3ce82be..dd2e5fa 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
@@ -1,4 +1,5 @@
/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
* Copyright (C) 2013 Red Hat
* Author: Rob Clark
*
@@ -162,11 +163,13 @@ static void mdp5_encoder_mode_set(struct drm_encoder
*encoder,
{
struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
struct mdp5_kms *mdp5_kms = get_kms(encoder);
+ struct drm_device *dev = encoder->dev;
+ struct drm_connector *connector;
int intf = mdp5_encoder->intf;
uint32_t dtv_hsync_skew, vsync_period, vsync_len, ctrl_pol;
uint32_t display_v_start, display_v_end;
uint32_t hsync_start_x, hsync_end_x;
- uint32_t format;
+ uint32_t format = 0x2100;
unsigned long flags;
mode = adjusted_mode;
@@ -188,7 +191,28 @@ static void mdp5_encoder_mode_set(struct drm_encoder
*encoder,
/* probably need to get DATA_EN polarity from panel.. */
dtv_hsync_skew = 0; /* get this from panel? */
- format = 0x213f; /* get this from panel? */
+
+ /* Get color format from panel, default is 8bpc */
+ list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ if (connector->encoder == encoder) {
+ switch (connector->display_info.bpc) {
+ case 4:
+ format |= 0;
+ break;
+ case 5:
+ format |= 0x15;
+ break;
+ case 6:
+ format |= 0x2A;
+ break;
+ case 8:
+ default:
+ format |= 0x3F;
+ break;
+ }
+ break;
+ }
+ }
hsync_start_x = (mode->htotal - mode->hsync_start);
hsync_end_x = mode->htotal - (mode->hsync_start - mode->hdisplay) - 1;
@@ -198,6 +222,16 @@ static void mdp5_encoder_mode_set(struct drm_encoder
*encoder,
display_v_start = (mode->vtotal - mode->vsync_start) * mode->htotal +
dtv_hsync_skew;
display_v_end = vsync_period - ((mode->vsync_start - mode->vdisplay) *
mode->htotal) + dtv_hsync_skew - 1;
+ /*
+* For edp only:
+* DISPLAY_V_START = (VBP * HCYCLE) + HBP
+* DISPLAY_V_END = (VBP + VACTIVE) * HCYCLE - 1 - HFP
+*/
+ if (mdp5_encoder->intf_id == INTF_eDP) {
+ display_v_start += mode->htotal - mode->hsync_start;
+ display_v_end -= mode->hsync_start - mode->hdisplay;
+ }
+
spin_lock_irqsave(&mdp5_encoder->intf_lock, flags);
mdp5_write(mdp5_kms, REG_MDP5_INTF_HSYNC_CTL(intf),
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index d6f7e42..5b50f06 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -210,14 +210,6 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
}
}
- /* Construct encoder for HDMI: */
- encoder = mdp5_encoder_init(dev, 3, INTF_HDMI);
- if (IS_ERR(encoder)) {
- dev_err(dev->dev, "failed to construct encoder\n");
- ret = PTR_ERR(encoder);
- goto fail;
- }
-
/* NOTE: the vsync and error irq's are actually associated with
* the INTF/encoder.. the easiest way to deal with this (ie. what
* we do now) is assume a fixed relationship between crtc's and
@@ -226,13 +218,18 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
* care of error and vblank irq's that the crtc has registered,
* and also update user-requested vblank_mask.
*/
- encoder->possible_crtcs = BIT(0);
- mdp5_crtc_set_intf(priv->crtcs[0], 3, INTF_HDMI);
+ if (priv->hdmi) {
+ /* Construct encoder for HDMI: */
+ encoder = mdp5_encoder_init(dev, 3, INTF_HDMI);
+ if (IS_ERR(encoder)) {
+ dev_err(dev->dev, "failed to construct encoder\n");
+ ret = PTR_ERR(encoder);
+ goto fail;
+
[RFC 01/15] drivers/base: add track framework
On Wed, Dec 10, 2014 at 04:48:19PM +0100, Andrzej Hajda wrote:
> track is a generic framework for safe tracking presence of any kernel objects
> having an id. There are no special requirements about type of object or its
> id. Id shall be unique.
This is pretty confusing, when it talks about "kernel objects" that
sounds like it means struct kobject but in fact there's no connection
with that or any of the other kernel object stuff. Perhaps it makes
sense but it should be addressed.
> Typical usage of the framework by consumer looks as follow:
> 1. Consumer registers notifier callback for objects with given id.
This is also a custom thing not connected with the notifier mechanism
implemented by struct notifier_block. Again this should probably be
addressed, even if it's just used internally it seems like there should
be some opportunity for code reuse here.
> + case track_task_up:
> + node = track_get_node(track, task->type, task->id, true);
> +
> + node->up = true;
> + node->data = task->data;
> + list_for_each_entry_safe(itb, node->itb_next, &node->itb_head,
> + list)
> + itb->callback(itb, node->data, true);
> + return;
> + case track_task_down:
I'm not sure the up and down naming is the most obvious naming for
users. It's obviously inspired by semaphores but it's not entirely
obvious that this is going to make things clear and meaningful for
someone trying to understand the interface.
> +static int track_process_queue(struct tracker *track)
> +{
> + struct track_task *task, *ptask = NULL;
> + unsigned long flags;
> + bool empty;
> +
> + /* Queue non-emptiness is used as a sentinel to prevent processing
> + * by multiple threads, so we cannot delete entry from the queue
> + * until it is processed.
> + */
> + while (true) {
> + spin_lock_irqsave(&track->queue_lock, flags);
> +
> + if (ptask)
> + list_del(&ptask->list);
> + task = list_first_entry(&track->queue,
> + struct track_task, list);
> +
> + empty = list_empty(&track->queue);
> + if (empty)
> + complete_all(&track->queue_empty);
> +
> + spin_unlock_irqrestore(&track->queue_lock, flags);
Here we get a task from the head of the list and drop the lock, leaving
the task on the list...
> + kfree(ptask);
> +
> + if (empty)
> + break;
> +
> + track_process_task(track, task);
...we then go and do some other stuff, including processing that task,
without the lock or or any other means I can see of excluding other
users before going round and removing the task. This seems to leave us
vulnerable to double execution. I *think* this is supposed to be
handled by your comment "Provider should take care of calling
notifications synchronously in proper order" in the changelog but that's
a bit obscure, it's not specific about what the requirements are (or
what the limits are supposed to be on the notification callbacks).
I'm also unclear what is supposed to happen if adding a notification
races with removing the thing being watched.
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/01edde99/attachment.sig>
[RFC 02/15] drivers/base: add restrack framework
On Wed, Dec 10, 2014 at 04:48:20PM +0100, Andrzej Hajda wrote:
> restrack framework allows tracking presence of resources with dynamic life
> time. Typical example of such resources are all resources provided by device
I don't know about anyone else but I'm having a hard time reading the
restrack name, it looks like a misspelling of restack to me.
At a high level my biggest questions here are the relationship between
this and the component code and usability. The usability concern I have
is that I see no diagnostics or trace here at all. This means that if a
user looks at their system, sees that the device model claims the driver
for a device bound to the device but can't see any sign of the device
doing anything they don't have any way of telling why that is other than
to look in the driver code, see what resources it was trying to depend
on and then go back to the running system to try to understand which of
those resources hasn't appeared.
> +int restrack_up(unsigned long type, const void *id, void *data)
> +int restrack_down(unsigned long type, const void *id, void *data)
Again I'm not sure that the up and down naming is meaningful in the
context of this interface.
> +static void restrack_itb_cb(struct track_block *itb, void *data, bool on)
> +{
itb_cb?
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20141212/10951c43/attachment.sig>
