[PATCH] nouveau: bring back legacy mmap handler
On 16 December 2014 at 18:56, Daniel Vetter wrote:
> On Tue, Dec 16, 2014 at 04:34:39PM +1000, Dave Airlie wrote:
>> From: Dave Airlie
>>
>> nouveau userspace back at 1.0.1 used to call the X server
>> DRIOpenDRMMaster interface even for DRI2 (doh!), this attempts
>> to map the sarea and fails if it can't.
>>
>> Since 884c6dabb0eafe7227f099c9e78e514191efaf13 from Daniel,
>> this fails, but only ancient drivers would see it.
>>
>> Revert the nouveau bits of that fix, and hope it works.
>>
>> Signed-off-by: Dave Airlie
>
> Argh, sorry for missing that in my git history digging.
>
> Acked-by: Daniel Vetter
>
> Now curious question: Would return 0 also work? As long as userspace only
> cares about the errno of the ioctl call and doesn't actually look at the
> sarea itself we could keep the cake and eat it, too. I've used a similar
> trick in legacy context code, again because old nouveau had copypasta from
> dri1, but only cared about the errno and not whether the contexts actually
> did anything useful.
if (drmMap(tmp.drmFD, tmp.hLSAREA, sAreaSize,
(drmAddressPtr) (&tmp.pLSAREA)) < 0) {
DRIDrvMsg(-1, X_INFO, "[drm] Mapping SAREA for DRM lock failed.\n");
tmp.pLSAREA = NULL;
goto out_err;
}
memset(tmp.pLSAREA, 0, sAreaSize);
seems unlikely return 0 will work!
Dave.
drm/dsi: Add message to packet translator
Hello Thierry Reding,
This is a semi-automatic email about new static checker warnings.
The patch a52879e8d7cb: "drm/dsi: Add message to packet translator"
from Oct 16, 2014, leads to the following Smatch complaint:
drivers/gpu/drm/drm_mipi_dsi.c:328 mipi_dsi_create_packet()
warn: variable dereferenced before check 'msg' (see line 326)
drivers/gpu/drm/drm_mipi_dsi.c
325 {
326 const u8 *tx = msg->tx_buf;
^^^
Dereference.
327
328 if (!packet || !msg)
Check.
329 return -EINVAL;
330
regards,
dan carpenter
drm: rockchip: Add basic drm driver
Hello Dan,
Thanks for your review, I will fix it soon.
Do you use a script to find out these issues? I was interested in this
script.
Can you teach me how to find this script and use it?
regards,
Mark Yao
On 2014å¹´12æ16æ¥ 19:51, Dan Carpenter wrote:
> Hello Mark Yao,
>
> The patch 2048e3286f34: "drm: rockchip: Add basic drm driver" from
> Aug 22, 2014, leads to the following static checker warning:
>
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1383 vop_bind()
> warn: unsigned 'vop->irq' is never less than zero.
>
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>1381
>1382 vop->irq = platform_get_irq(pdev, 0);
>1383 if (vop->irq < 0) {
>
> Doesn't work.
right, we should use "int" but not "unsigned int" for irq type.
>1384 dev_err(dev, "cannot find irq for vop\n");
>1385 return vop->irq;
>1386 }
>1387
>
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1188 vop_create_crtc()
> warn: missing error code here? 'of_get_child_by_name()' failed.
>
>1184 port = of_get_child_by_name(dev->of_node, "port");
>1185 if (!port) {
>1186 DRM_ERROR("no port node found in %s\n",
>1187dev->of_node->full_name);
>
> Probably, "ret = -ENODEV;"
right, "ret = -ENODEV" is needed.
>
>1188 goto err_cleanup_crtc;
>1189 }
>1190
>
>
> regards,
> dan carpenter
>
>
>
[Bug 85421] radeon stalled, GPU lockup, reset and failed on resume; crashed by firefox.
https://bugzilla.kernel.org/show_bug.cgi?id=85421 --- Comment #21 from Michel Dänzer --- If there really is a significant difference in stability between Mesa 10.2.y and 10.3.y, it would be interesting if you could isolate which change between them made the difference. However, from your description so far, I'm afraid the difference might just be coincidence, because we don't understand yet what triggers the problem, so it happens 'randomly'. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 13/13] RFC: drm: Atomic modeset ioctl
On 17.12.2014 08:05, Rob Clark wrote:
> The atomic modeset ioctl can be used to push any number of new values
> for object properties. The driver can then check the full device
> configuration as single unit, and try to apply the changes atomically.
>
> The ioctl simply takes a list of object IDs and property IDs and their
> values.
[...]
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 86574b0..3459778 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -519,4 +519,25 @@ struct drm_mode_destroy_dumb {
> uint32_t handle;
> };
>
> +/* page-flip flags are valid, plus: */
> +#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> +#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> +
> +#define DRM_MODE_ATOMIC_FLAGS (\
> + DRM_MODE_PAGE_FLIP_EVENT |\
> + DRM_MODE_PAGE_FLIP_ASYNC |\
> + DRM_MODE_ATOMIC_TEST_ONLY |\
> + DRM_MODE_ATOMIC_NONBLOCK)
> +
> +struct drm_mode_atomic {
> + __u32 flags;
> + __u32 count_objs;
> + __u64 objs_ptr;
> + __u64 count_props_ptr;
> + __u64 props_ptr;
> + __u64 prop_values_ptr;
> + __u64 blob_values_ptr; /* remove? */
> + __u64 user_data;
> +};
> +
> #endif
>
The new ioctl(s) should take an explicit parameter specifying when the
changes should take effect. And since variable refresh rate displays are
becoming mainstream, that parameter should probably be a timestamp
rather than a frame counter.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[PATCH] drm/qxl: Use drm_vblank_count()
On 12/15/2014 04:56 PM, Thierry Reding wrote:
> From: Thierry Reding
>
> The QXL driver duplicates part of the core's drm_vblank_count(), so it
> might as well use the core's variant for the extra goodies.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 7 +--
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 1d9b80c91a15..497024461a3c 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -196,11 +196,6 @@ static int qxl_pm_restore(struct device *dev)
> return qxl_drm_resume(drm_dev, false);
> }
>
> -static u32 qxl_noop_get_vblank_counter(struct drm_device *dev, int crtc)
> -{
> - return dev->vblank[crtc].count.counter;
> -}
> -
> static int qxl_noop_enable_vblank(struct drm_device *dev, int crtc)
> {
> return 0;
> @@ -231,7 +226,7 @@ static struct drm_driver qxl_driver = {
> DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED,
> .load = qxl_driver_load,
> .unload = qxl_driver_unload,
> - .get_vblank_counter = qxl_noop_get_vblank_counter,
> + .get_vblank_counter = drm_vblank_count,
> .enable_vblank = qxl_noop_enable_vblank,
> .disable_vblank = qxl_noop_disable_vblank,
>
Hi
That doesn't really help, although it doesn't hurt either. Just wanted
to point out that both the old and new method implement a no-op. The
get_vblank_counter() driver function is meant to implement a hardware
vblank counter query. It's only use case atm. is to reinitialize the
dev->vblank[crtc].count.counter counter returned by drm_vblank_count().
The most honest implementation if there isn't any way to get a hw vblank
count would be to just "return 0;" - Same net effect, but at least a
marker in the code that there is future work to do.
I think a better solution would be if we wouldn't require
.get_vblank_counter to be non-NULL, don't fake implement it in
kms-drivers which can't do it, and make the drm core deal with lack of
hw counter queries, e.g., by not disabling vblank irqs.
-mario
[PATCH 08/13] drm/irq: Check for valid VBLANK before dereference
On 17.12.2014 01:53, Thierry Reding wrote:
> From: Thierry Reding
>
> When accessing the array of per-CRTC VBLANK structures we must always
> check that the index into the array is valid before dereferencing to
> avoid crashing.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/gpu/drm/drm_irq.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index a24658162284..cb207e047505 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1070,10 +1070,10 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
> {
> struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
>
> - if (WARN_ON(atomic_read(&vblank->refcount) == 0))
> + if (WARN_ON(crtc >= dev->num_crtcs))
> return;
>
> - if (WARN_ON(crtc >= dev->num_crtcs))
> + if (WARN_ON(atomic_read(&vblank->refcount) == 0))
> return;
>
> /* Last user schedules interrupt disable */
> @@ -1356,6 +1356,9 @@ void drm_vblank_post_modeset(struct drm_device *dev,
> int crtc)
> if (!dev->num_crtcs)
> return;
>
> + if (WARN_ON(crtc >= dev->num_crtcs))
> + return;
> +
> if (vblank->inmodeset) {
> spin_lock_irqsave(&dev->vbl_lock, irqflags);
> dev->vblank_disable_allowed = true;
>
It would probably be better to use WARN_ON_ONCE, otherwise any bugs
triggering these might flood dmesg.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[Bug 87278] Packet0 not allowed and GPU fault detected errors with Serious Engine games
https://bugs.freedesktop.org/show_bug.cgi?id=87278 --- Comment #5 from Alexandre Demers --- (In reply to Daniel Scharrer from comment #0) > Created attachment 110808 [details] > dmesg output with the GPU fault errors filtered out > > Running Serious Sam 3 or The Talos Principle spams dmesg with thousands of > these errors: > > [ 6001.212237] radeon :01:00.0: GPU fault detected: 147 0x02528801 > [ 6001.212243] radeon :01:00.0: VM_CONTEXT1_PROTECTION_FAULT_ADDR > 0x0FF02192 > [ 6001.212246] radeon :01:00.0: VM_CONTEXT1_PROTECTION_FAULT_STATUS > 0x12088001 > [ 6001.212249] VM fault (0x01, vmid 9) at page 267395474, read from TC (136) > > There are also a few "Packet0 not allowed" errors (followed by a hex dump): > > [15446.473341] radeon :01:00.0: Packet0 not allowed! > > So far it's only these errors in dmesg - I haven't observed any actual > rendering issues, crashes, GPU lockups because of this. > > I have only attached a filtered kernel log with the GPU fault errors removed > - the full log is available at http://constexpr.org/tmp/serious-dmesg.log > (140 MiB). > > Both of these games use the Serious Engine 3.5 (Serious Sam 3) or 4 (The > Talos Principle). This is also reproducible with The Talos Principle Public > Test which as of now is still available as a free download on Steam. > > Kernel: 3.18.0-gentoo > GPU: Radeon HD 7950 > Driver: radeonsi, Mesa 10.5.0-devel (git-ff96537) > > This might be related to bug 84500 - however those spurious Packet0 have > been gone for a while now with updated Mesa - now I got them again but only > while running Serious Engine games. I haven't had a look at the log when launching SS3, but for sure it crashes in no time. It crashes in no time once in a game. It could be related to your bug. However, I think the VM and the Packet 0 are different bugs. I'll have a look in the logs if I get something similar. -- 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/20141217/0b612a6a/attachment.html>
[PATCH 13/13] RFC: drm: Atomic modeset ioctl
On Wed, 17 Dec 2014 11:48:51 +0900
Michel Dänzer wrote:
> On 17.12.2014 08:05, Rob Clark wrote:
> > The atomic modeset ioctl can be used to push any number of new values
> > for object properties. The driver can then check the full device
> > configuration as single unit, and try to apply the changes atomically.
> >
> > The ioctl simply takes a list of object IDs and property IDs and their
> > values.
>
> [...]
>
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 86574b0..3459778 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -519,4 +519,25 @@ struct drm_mode_destroy_dumb {
> > uint32_t handle;
> > };
> >
> > +/* page-flip flags are valid, plus: */
> > +#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> > +#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> > +
> > +#define DRM_MODE_ATOMIC_FLAGS (\
> > + DRM_MODE_PAGE_FLIP_EVENT |\
> > + DRM_MODE_PAGE_FLIP_ASYNC |\
> > + DRM_MODE_ATOMIC_TEST_ONLY |\
> > + DRM_MODE_ATOMIC_NONBLOCK)
> > +
> > +struct drm_mode_atomic {
> > + __u32 flags;
> > + __u32 count_objs;
> > + __u64 objs_ptr;
> > + __u64 count_props_ptr;
> > + __u64 props_ptr;
> > + __u64 prop_values_ptr;
> > + __u64 blob_values_ptr; /* remove? */
> > + __u64 user_data;
> > +};
> > +
> > #endif
> >
>
> The new ioctl(s) should take an explicit parameter specifying when the
> changes should take effect. And since variable refresh rate displays are
> becoming mainstream, that parameter should probably be a timestamp
> rather than a frame counter.
That sounds cool to me, but also a rabbit hole. While having worked on
the Wayland Presentation queueing extension, I'd like to ask the
following questions:
- If you set the atomic kick to happen in the future, is there any way
to cancel it? I'd be ok with not being able to cancel initially, but
if one wants to add that later, we should already know how to
reference this atomic submission in the cancel request. What if user
space has a bug and schedules an update at one hour or three days from
now, how would we abort that?
- Can you VT-switch or drop DRM master if you have a pending atomic
update?
- Should one be able to set multiple pending atomic updates?
- If I schedule an atomic update for one CTRC, can I schedule another
update for another CRTC before the first one completes? Or am I
forced to gather all updates over all outputs in the same atomic
submission even if I don't care about or want inter-output sync and
the outputs might even be running at different refresh rates?
(Actually, this seems to be a valid question even without any target
time parameter.)
- If there can be multiple pending atomic updates on the same DRM
device, is there any way to guarantee that the
DRM_MODE_ATOMIC_TEST_ONLY results will be accurate also when the
atomic update actually kicks in? Another update may have changed the
configuration before this update kicks in, which means the overall
state isn't the same that was tested.
- Does a pending atomic update prevent immediate (old style) KMS
changes?
- Assuming hardware cannot do arbitrary time updates, how do you round
the given timestamp? Strictly "not before" given time? Round to
nearest possible time? The effect of required vs. unwanted sync to
vblank?
- How would user space match the page flip event to the atomic
submission it did?
I wonder if there is a way to postpone these hard(?) questions, so that
we could have atomic sooner and add scheduling later? I would imagine
solving everything above is quite some work.
Thanks,
pq
[Bug 89661] Kernel panic when trying use amdkfd driver on Kaveri
https://bugzilla.kernel.org/show_bug.cgi?id=89661 --- Comment #12 from Oded Gabbay --- As I said, there is definitely a bug when compiling both radeon and amdkfd inside the kernel. I'm working on fixing it, but that could take a few days. In the meantime, the only way to make it work without touching the code, is to either compile both drivers as modules or just radeon as module. No need for further experiments. -- You are receiving this mail because: You are watching the assignee of the bug.
drm: exynos: mixer: fix using usleep() in atomic context
On Mon, Dec 01, 2014 at 05:16:17PM +0100, Tobias Jakobi wrote: > On 2014-12-01 16:54, Thierry Reding wrote: > >On Sun, Nov 30, 2014 at 01:35:25AM +0100, tjakobi at math.uni-bielefeld.de > >wrote: > >>From: Tomasz Stanislawski > >> > >>This patch fixes calling usleep_range() after taking reg_slock > >>using spin_lock_irqsave(). The mdelay() is used instead. > >>Waiting in atomic context is not the best idea in general. > >>Hopefully, waiting occurs only when Video Processor fails > >>to reset correctly. > >> > >>Signed-off-by: Tomasz Stanislawski > >>--- > >> drivers/gpu/drm/exynos/exynos_mixer.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c > >>b/drivers/gpu/drm/exynos/exynos_mixer.c > >>index a41c84e..cc7 100644 > >>--- a/drivers/gpu/drm/exynos/exynos_mixer.c > >>+++ b/drivers/gpu/drm/exynos/exynos_mixer.c > >>@@ -632,7 +632,7 @@ static void vp_win_reset(struct mixer_context *ctx) > >>/* waiting until VP_SRESET_PROCESSING is 0 */ > >>if (~vp_reg_read(res, VP_SRESET) & VP_SRESET_PROCESSING) > >>break; > >>- usleep_range(1, 12000); > >>+ mdelay(10); > >>} > >>WARN(tries == 0, "failed to reset Video Processor\n"); > >> } > > > >I can't see a reason why you would need to hold the lock around this > >code. Perhaps a better way to fix this would be to drop the lock before > >calling vp_win_reset()? > > > >Thierry > > Hmm, I'm pretty new to spinlocks (only have worked with the usual mutex > stuff in userspace), but wouldn't that mean that it is then possible for > mixer_win_reset to execute while a (previous) vp_win_reset is still running? Indeed it would. I didn't look properly. Looking more closely it seems the call stack for this looks something like: vp_win_reset() mixer_win_reset() mixer_poweron() mixer_dpms() exynos_drm_crtc_dpms() Which can then be called from two places: exynos_drm_crtc_commit() drm_crtc_helper_set_mode() drm_crtc_helper_set_config() drm_helper_connector_dpms() drm_crtc_helper_set_config() drm_crtc_helper_set_config() itself must be called with the all modeset locks held, so I don't see a way how vp_win_reset() could be called concurrently. Anyway, even if you're still concerned about concurrent accesses to the register you'd better lock this section with a mutex to avoid excessive spinning. In fact I think a better option would be to extend the mutex in mixer_poweron() to encompass the whole function. This currently looks broken because one process could go to sleep in pm_runtime_get_sync() or clk_prepare_enable() and another process start running mixer_poweron() concurrently, getting to the second mutex_lock() sooner than the first process. So the lock being dropped between checking for ctx->powered and setting it doesn't actually prevent a race. Then again, nobody seems to have cared so far... Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141217/c3389dfd/attachment.sig>
[RFC PATCH v3 4/4] tests/drv_module_reload: add ipvr support
On Mon, Nov 24, 2014 at 02:14:48PM +0100, Daniel Vetter wrote: > On Mon, Nov 24, 2014 at 10:55:46AM +0100, Thierry Reding wrote: > > On Fri, Nov 21, 2014 at 09:36:33PM +0100, Daniel Vetter wrote: > > > On Fri, Nov 21, 2014 at 09:27:04PM +0100, Thierry Reding wrote: > > > > On Sat, Nov 22, 2014 at 03:10:01AM +0800, Yao Cheng wrote: > > > > > on vlv, if ipvr is installed, it need be manually unloaded before > > > > > i915, otherwise user might run into use-after-free issue. > > > > > > > > Huh? That doesn't sound right. What exactly is it that's going wrong? > > > > You should never have to do this. If you do you're almost certainly > > > > doing something wrong in the kernel module. > > > > > > It's the hilarity called platform devices. Removing them is somewhat racy, > > > so doing that upfront makes the entire thing a bit safer. The use after > > > free is on the text, since grabbing a module refcount for the platform > > > device doesn't work (it would pin the module forever). > > > > I don't understand what the issue is here. I've used platform devices > > quite extensively on ARM and I've never encountered a situation where > > they were insufficient (or racy for that matter). > > > > If I understand correctly what this commit tries to achieve, then it > > unloads one module before another module that it depends on so that the > > dependency can be removed subsequently without causing a crash. That > > sounds really brittle to me. How are you going to document this for > > users so that they don't accidentally go and unload the i915 module and > > crash their system? > > Module unloading taints your kernel and isn't an end-user supported > feature. That simple ;-) > > Also afaik the problem is that you actually can't unload i915 until you've > unloaded the subordinate driver, since i915 registering the platform > driver prevents unload. That doesn't sound at all like use-after-free, so if that's really the only problem then the commit description should be more accurate. > Or at least that was my understanding, I didn't test this myself. I > just asked whether the unload script still works and apparently it > breaks. > > I guess what's different with ARM is that DT creates all the platform > devices, and not modules themselves? No, I don't think that has anything to do with it. I'm pretty sure I've seen this work reliably with something like MFD where one module can create a number of platform devices, and remove them again, just as well. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141217/24ca9713/attachment-0001.sig>
[RFC PATCH v3 4/4] tests/drv_module_reload: add ipvr support
On Mon, Dec 01, 2014 at 03:06:08AM +, Cheng, Yao wrote: > > -Original Message- > > From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel > > Vetter > > Sent: Monday, November 24, 2014 21:15 > > To: Thierry Reding > > Cc: Daniel Vetter; Cheng, Yao; intel-gfx at lists.freedesktop.org; dri- > > devel at lists.freedesktop.org; daniel.vetter at ffwll.ch; Kelley, Sean V; > > Chehab, > > John; emil.l.velikov at gmail.com; Jiang, Fei > > Subject: Re: [RFC PATCH v3 4/4] tests/drv_module_reload: add ipvr support > > > > On Mon, Nov 24, 2014 at 10:55:46AM +0100, Thierry Reding wrote: > > > On Fri, Nov 21, 2014 at 09:36:33PM +0100, Daniel Vetter wrote: > > > > On Fri, Nov 21, 2014 at 09:27:04PM +0100, Thierry Reding wrote: > > > > > On Sat, Nov 22, 2014 at 03:10:01AM +0800, Yao Cheng wrote: > > > > > > on vlv, if ipvr is installed, it need be manually unloaded > > > > > > before i915, otherwise user might run into use-after-free issue. > > > > > > > > > > Huh? That doesn't sound right. What exactly is it that's going wrong? > > > > > You should never have to do this. If you do you're almost > > > > > certainly doing something wrong in the kernel module. > > > > > > > > It's the hilarity called platform devices. Removing them is somewhat > > > > racy, so doing that upfront makes the entire thing a bit safer. The > > > > use after free is on the text, since grabbing a module refcount for > > > > the platform device doesn't work (it would pin the module forever). > > > > > > I don't understand what the issue is here. I've used platform devices > > > quite extensively on ARM and I've never encountered a situation where > > > they were insufficient (or racy for that matter). > > > > > > If I understand correctly what this commit tries to achieve, then it > > > unloads one module before another module that it depends on so that > > > the dependency can be removed subsequently without causing a crash. > > > That sounds really brittle to me. How are you going to document this > > > for users so that they don't accidentally go and unload the i915 > > > module and crash their system? > > > > Module unloading taints your kernel and isn't an end-user supported feature. > > That simple ;-) > > > > Also afaik the problem is that you actually can't unload i915 until you've > > unloaded the subordinate driver, since i915 registering the platform driver > > prevents unload. Or at least that was my understanding, I didn't test this > > myself. I just asked whether the unload script still works and apparently it > > breaks. > > > > I guess what's different with ARM is that DT creates all the platform > > devices, > > and not modules themselves? > > -Daniel > > Thierry/Daniel, the actual symptom is, after "rmmod i915", though > drm_drv_release() is also called on the child device "ipvr", I still > see the module exist in the system (check it by "lsmod"). Which module? ipvr or i915? > This causes issue when I modprobe i915 and ipvr again later. What issue are you seeing? If your driver can't deal with a situation where it's probed again after being removed then you have a bug. > I don't understand why this happens but I believe what Daniel said: > "grabbing a module refcount for the platform device doesn't work (it > would pin the module forever)" What I'd expect to happen is this: # modprobe i915 i915 registers a platform devices # modprobe ipvr driver core probes ipvr device # modprobe -r i915 i915 removes the platform device (ipvr's ->remove() is called) I guess if you don't do anything else, then indeed the ipvr module will stay around, but the above should work idempotently, that is you should be able to repeat it an unlimited number of times and nothing should break. In fact you should be able to run the following in any permutation without causing a crash: # modprobe i915 # modprobe ipvr # modprobe -r ipvr # modprobe -r i915 If any permutation results in a crash you have a bug. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141217/beeacd74/attachment.sig>
[Bug 80584] XCOM: Enemy Unknown incorrect hair rendering
https://bugs.freedesktop.org/show_bug.cgi?id=80584 --- Comment #6 from dex+fdobugzilla at dragonslave.de --- I think this may be due to missing GL_ARB_tessellation_shader extension. Mesa lists this as Status "started" but no driver implementation yet. I'll try to evaluate the status of this and see if I can verify that its indeed this missing extension. (That would also mean that XCOM is in fact OpenGL) -- 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/20141217/e6f6e51a/attachment.html>
[Bug 80584] XCOM: Enemy Unknown incorrect hair rendering
https://bugs.freedesktop.org/show_bug.cgi?id=80584 --- Comment #7 from dex+fdobugzilla at dragonslave.de --- OpenGL 4.0... Why can't I edit even my own comments!? -- 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/20141217/743c9bdb/attachment.html>
[PATCH 13/13] RFC: drm: Atomic modeset ioctl
On 17.12.2014 16:20, Pekka Paalanen wrote:
> On Wed, 17 Dec 2014 11:48:51 +0900
> Michel Dänzer wrote:
>
>> On 17.12.2014 08:05, Rob Clark wrote:
>>> The atomic modeset ioctl can be used to push any number of new values
>>> for object properties. The driver can then check the full device
>>> configuration as single unit, and try to apply the changes atomically.
>>>
>>> The ioctl simply takes a list of object IDs and property IDs and their
>>> values.
>>
>> [...]
>>
>>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>>> index 86574b0..3459778 100644
>>> --- a/include/uapi/drm/drm_mode.h
>>> +++ b/include/uapi/drm/drm_mode.h
>>> @@ -519,4 +519,25 @@ struct drm_mode_destroy_dumb {
>>> uint32_t handle;
>>> };
>>>
>>> +/* page-flip flags are valid, plus: */
>>> +#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
>>> +#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
>>> +
>>> +#define DRM_MODE_ATOMIC_FLAGS (\
>>> + DRM_MODE_PAGE_FLIP_EVENT |\
>>> + DRM_MODE_PAGE_FLIP_ASYNC |\
>>> + DRM_MODE_ATOMIC_TEST_ONLY |\
>>> + DRM_MODE_ATOMIC_NONBLOCK)
>>> +
>>> +struct drm_mode_atomic {
>>> + __u32 flags;
>>> + __u32 count_objs;
>>> + __u64 objs_ptr;
>>> + __u64 count_props_ptr;
>>> + __u64 props_ptr;
>>> + __u64 prop_values_ptr;
>>> + __u64 blob_values_ptr; /* remove? */
>>> + __u64 user_data;
>>> +};
>>> +
>>> #endif
>>>
>>
>> The new ioctl(s) should take an explicit parameter specifying when the
>> changes should take effect. And since variable refresh rate displays are
>> becoming mainstream, that parameter should probably be a timestamp
>> rather than a frame counter.
>
> That sounds cool to me, but also a rabbit hole. While having worked on
> the Wayland Presentation queueing extension, I'd like to ask the
> following questions:
>
> - If you set the atomic kick to happen in the future, is there any way
> to cancel it? I'd be ok with not being able to cancel initially, but
> if one wants to add that later, we should already know how to
> reference this atomic submission in the cancel request. What if user
> space has a bug and schedules an update at one hour or three days from
> now, how would we abort that?
>
> - Can you VT-switch or drop DRM master if you have a pending atomic
> update?
>
> - Should one be able to set multiple pending atomic updates?
>
> - If I schedule an atomic update for one CTRC, can I schedule another
> update for another CRTC before the first one completes? Or am I
> forced to gather all updates over all outputs in the same atomic
> submission even if I don't care about or want inter-output sync and
> the outputs might even be running at different refresh rates?
> (Actually, this seems to be a valid question even without any target
> time parameter.)
>
> - If there can be multiple pending atomic updates on the same DRM
> device, is there any way to guarantee that the
> DRM_MODE_ATOMIC_TEST_ONLY results will be accurate also when the
> atomic update actually kicks in? Another update may have changed the
> configuration before this update kicks in, which means the overall
> state isn't the same that was tested.
>
> - Does a pending atomic update prevent immediate (old style) KMS
> changes?
>
> - Assuming hardware cannot do arbitrary time updates, how do you round
> the given timestamp? Strictly "not before" given time? Round to
> nearest possible time? The effect of required vs. unwanted sync to
> vblank?
>
> - How would user space match the page flip event to the atomic
> submission it did?
>
> I wonder if there is a way to postpone these hard(?) questions, so that
> we could have atomic sooner and add scheduling later? I would imagine
> solving everything above is quite some work.
I agree. The main reason I brought it up is because I'd like to avoid
getting into the same situation as with the current
DRM_IOCTL_MODE_PAGE_FLIP ioctl, which doesn't explicitly communicate
between userspace and kernel when the flip is supposed/expected to
occur. We recently had to jump through some hoops in the radeon kernel
driver to prevent flips from occurring sooner than expected by userspace.
But I also think it would be a shame at this point to add new ioctls
which aren't designed with variable refresh rate displays in mind.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[PATCH] drm/qxl: Use drm_vblank_count()
On Wed, Dec 17, 2014 at 03:57:51AM +0100, Mario Kleiner wrote:
> On 12/15/2014 04:56 PM, Thierry Reding wrote:
> > From: Thierry Reding
> >
> > The QXL driver duplicates part of the core's drm_vblank_count(), so it
> > might as well use the core's variant for the extra goodies.
> >
> > Signed-off-by: Thierry Reding
> > ---
> > drivers/gpu/drm/qxl/qxl_drv.c | 7 +--
> > 1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > index 1d9b80c91a15..497024461a3c 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > @@ -196,11 +196,6 @@ static int qxl_pm_restore(struct device *dev)
> > return qxl_drm_resume(drm_dev, false);
> > }
> >
> > -static u32 qxl_noop_get_vblank_counter(struct drm_device *dev, int crtc)
> > -{
> > - return dev->vblank[crtc].count.counter;
> > -}
> > -
> > static int qxl_noop_enable_vblank(struct drm_device *dev, int crtc)
> > {
> > return 0;
> > @@ -231,7 +226,7 @@ static struct drm_driver qxl_driver = {
> >DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED,
> > .load = qxl_driver_load,
> > .unload = qxl_driver_unload,
> > - .get_vblank_counter = qxl_noop_get_vblank_counter,
> > + .get_vblank_counter = drm_vblank_count,
> > .enable_vblank = qxl_noop_enable_vblank,
> > .disable_vblank = qxl_noop_disable_vblank,
> >
>
> Hi
>
> That doesn't really help, although it doesn't hurt either. Just wanted
> to point out that both the old and new method implement a no-op. The
> get_vblank_counter() driver function is meant to implement a hardware
> vblank counter query. It's only use case atm. is to reinitialize the
> dev->vblank[crtc].count.counter counter returned by drm_vblank_count().
>
> The most honest implementation if there isn't any way to get a hw vblank
> count would be to just "return 0;" - Same net effect, but at least a
> marker in the code that there is future work to do.
Yeah 'return 0' is what we do in i915 when there's no hw counter. I did
consider changing it to drm_vblank_count() since that seems to be the
current fad. I was hoping it might allow removing some code from
drm_irq.c, but after some more thought that might not be the case.
I'll probably need to take another look at it.
>
> I think a better solution would be if we wouldn't require
> .get_vblank_counter to be non-NULL, don't fake implement it in
> kms-drivers which can't do it, and make the drm core deal with lack of
> hw counter queries, e.g., by not disabling vblank irqs.
That seems a bit drastic. The current delayed disable
seems quite reasonable to me. The count will remain accurate as long
as the vblank irq is enabled, and if you wait for so long that the
irq gets disabled, well, I don't think a very precise answer was
needed anyway.
I was hunting some bugs in the vblank code recently, and while doing
that I thought that I might change the code to use the timestamp
difference between disable->enable to calculate an approximate number
of vblanks lost and bump the counter appropriately. Didn't try it
yet though, but seems like a reasonable idea when there's no hw
counter. Though some care will be needed when dealing with
drm_vblank_off/on.
--
Ville Syrjälä
Intel OTC
[Bug 87278] Packet0 not allowed and GPU fault detected errors with Serious Engine games
https://bugs.freedesktop.org/show_bug.cgi?id=87278 Michel Dänzer changed: What|Removed |Added CC||tstellar at gmail.com --- Comment #6 from Michel Dänzer --- (In reply to Christoph Haag from comment #2) > [47303.471211] radeon :01:00.0: VM_CONTEXT1_PROTECTION_FAULT_ADDR > 0x0FF03E4E The fact that bits 32-39 of the faulting addresses are FF indicates incorrect shader code generation resulting in those address bits being clobbered. Can you generate an apitrace which reproduces the Packet0 error and/or GPUVM faults? -- 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/20141217/4f9a57ad/attachment-0001.html>
[PATCH RFC 09/15] drm: imx: Add MIPI DSI host controller driver
Hi Thierry, Sorry for the late response. I tried to address almost all your comments locally first. More feedback below. On 12/10/2014 09:16 PM, Thierry Reding wrote: > On Wed, Dec 10, 2014 at 04:37:22PM +0800, Liu Ying wrote: >> This patch adds i.MX MIPI DSI host controller driver support. >> Currently, the driver supports the burst with sync pulses mode only. >> >> Signed-off-by: Liu Ying >> --- >> .../devicetree/bindings/drm/imx/mipi_dsi.txt | 81 ++ >> drivers/gpu/drm/imx/Kconfig|6 + >> drivers/gpu/drm/imx/Makefile |1 + >> drivers/gpu/drm/imx/imx-mipi-dsi.c | 1017 >> >> 4 files changed, 1105 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt >> create mode 100644 drivers/gpu/drm/imx/imx-mipi-dsi.c >> >> diff --git a/Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt >> b/Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt >> new file mode 100644 >> index 000..3d07fd7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/imx/mipi_dsi.txt >> @@ -0,0 +1,81 @@ >> +Device-Tree bindings for MIPI DSI host controller >> + >> +MIPI DSI host controller >> + >> + >> +The MIPI DSI host controller is a Synopsys DesignWare IP. >> +It is a digital core that implements all protocol functions defined >> +in the MIPI DSI specification, providing an interface between the >> +system and the MIPI DPHY, and allowing communication with a MIPI DSI >> +compliant display. >> + >> +Required properties: >> + - #address-cells : Should be <1>. >> + - #size-cells : Should be <0>. >> + - compatible : Should be "fsl,imx6q-mipi-dsi" for i.MX6q/sdl SoCs. >> + - reg : Physical base address of the controller and length of memory >> + mapped region. >> + - interrupts : The controller's interrupt number to the CPU(s). >> + - gpr : Should be <&gpr>. >> + The phandle points to the iomuxc-gpr region containing the >> + multiplexer control register for the controller. > > Side-note: Shouldn't this really be a pinmux, then? No. The muxing is inside the i.MX SoC. There is a DT binding documentation for the system controller node(gpr) at [1]. And, for i.MX DT sources, there are several existing use cases in which the gpr node is referred by other nodes. [1] Documentation/devicetree/bindings/mfd/syscon.txt. > >> + - clocks, clock-names : Phandles to the controller pllref, pllref_gate >> + and core_cfg clocks, as described in [1] and [2]. >> + - panel at 0 : A panel node which contains a display-timings child node as >> + defined in [3]. > > There's no need for these to be named panel@*. They could be bridges for > example. And no, they shouldn't contain a display-timings child node > either. Panels should have a proper driver and the driver being device > specific it should have the timings embedded. Ok, I'll move the timing to the panel driver. > >> + - port@[0-4] : Up to four port nodes with endpoint definitions as defined >> + in [4], corresponding to the four inputs to the controller multiplexer. >> + Note that each port node should contain the input-port property to >> + distinguish it from the panel node, as described in [5]. > > [4] says that you can group all port nodes under a ports parent node. I > think this is really what you want to do here to make it clear that the > ports aren't part of the DSI host binding part of the device. > Accepted. >> diff --git a/drivers/gpu/drm/imx/imx-mipi-dsi.c >> b/drivers/gpu/drm/imx/imx-mipi-dsi.c > [...] >> +/* >> + * i.MX drm driver - MIPI DSI Host Controller >> + * >> + * Copyright (C) 2011-2014 Freescale Semiconductor, Inc. >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * as published by the Free Software Foundation; either version 2 >> + * of the License, or (at your option) any later version. >> + * >> + * 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include > > Don't you want the more generic linux/math64.h here? I'll use linux/math64.h. > >> +#include >> +#include >> +#include > > I don't see any of the functions defined in that header used here. I'll remove this. > >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include "imx-drm.h" >> + >> +#define DRIVER_NAME "imx-mipi-dsi" >> + >> +#define DSI_VERSION 0x00 >> + >> +#define DSI_PWR_UP 0x04 >> +#define RESET
[PATCH RFC 10/15] drm: panel: Add support for Himax HX8369A MIPI DSI panel
On 12/10/2014 10:03 PM, Thierry Reding wrote:
> On Wed, Dec 10, 2014 at 04:37:23PM +0800, Liu Ying wrote:
>> This patch adds support for Himax HX8369A MIPI DSI panel.
>>
>> Signed-off-by: Liu Ying
>> ---
>> .../devicetree/bindings/panel/himax,hx8369a.txt| 86 +++
>> drivers/gpu/drm/panel/Kconfig | 6 +
>> drivers/gpu/drm/panel/Makefile | 1 +
>> drivers/gpu/drm/panel/panel-hx8369a.c | 627
>> +
>> 4 files changed, 720 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/panel/himax,hx8369a.txt
>> create mode 100644 drivers/gpu/drm/panel/panel-hx8369a.c
>>
>> diff --git a/Documentation/devicetree/bindings/panel/himax,hx8369a.txt
>> b/Documentation/devicetree/bindings/panel/himax,hx8369a.txt
>> new file mode 100644
>> index 000..6fe251e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/panel/himax,hx8369a.txt
>> @@ -0,0 +1,86 @@
>> +Himax HX8369A WVGA 16.7M color TFT single chip driver with internal GRAM
>> +
>> +Himax HX8369A is a WVGA resolution driving controller.
>> +It is designed to provide a single chip solution that combines a source
>> +driver and power supply circuits to drive a TFT dot matrix LCD with
>> +480RGBx864 dots at the maximum.
>> +
>> +The HX8369A supports several interface modes, including MPU MIPI DBI Type
>> +A/B mode, MIPI DPI/DBI Type C mode, MIPI DSI video mode, MIPI DSI command
>> +mode and MDDI mode. The interface mode is selected by the external hardware
>> +pins BS[3:0].
>> +
>> +Currently, only the MIPI DSI video mode is supported.
>> +
>> +Required properties:
>> + - compatible: "himax,hx8369a-dsi"
>> + - reg: the virtual channel number of a DSI peripheral
>> + - reset-gpios: a GPIO spec for the reset pin
>> + - data-lanes: the data lane number of a DSI peripheral
>
> This is implied by the compatible already.
Accepted.
>
>> + - display-timings: timings for the connected panel as described by [1]
>
> Also implied by the compatible value.
Accepted.
>
>> + - bs: the interface mode number described by the following table
>> +--
>> + | DBI_TYPE_A_8BIT | 0 |
>> + | DBI_TYPE_A_9BIT | 1 |
>> + | DBI_TYPE_A_16BIT| 2 |
>> + | DBI_TYPE_A_18BIT| 3 |
>> + | DBI_TYPE_B_8BIT | 4 |
>> + | DBI_TYPE_B_9BIT | 5 |
>> + | DBI_TYPE_B_16BIT| 6 |
>> + | DBI_TYPE_B_18BIT| 7 |
>> + | DSI_CMD_MODE| 8 |
>> + | DBI_TYPE_B_24BIT| 9 |
>> + | DSI_VIDEO_MODE | 10 |
>> + | MDDI| 11 |
>> + | DPI_DBI_TYPE_C_OPT1 | 12 |
>> + | DPI_DBI_TYPE_C_OPT2 | 13 |
>> + | DPI_DBI_TYPE_C_OPT3 | 14 |
>> +--
>
> Can this not be inferred by the driver? If it's a DSI driver can't it
> select between DSI_VIDEO_MODE or DSI_CMD_MODE based on its capabilities?
> That is, if the panel driver can setup command mode, shouldn't it be
> using command mode in that case? And use DSI_VIDEO_MODE otherwise?
I may remove this property.
But, I choose not to add any logic in the host and slave drivers to
handle the interface mode selection at present, since I only support
the DSI_VIDEO_MODE now. Is this acceptable?
>
>> +Optional properties:
>> + - power-on-delay: delay after turning regulators on [ms]
>> + - reset-delay: delay after reset sequence [ms]
>
> Surely these are constant for this panel?
Accepted.
>
>> + - panel-width-mm: physical panel width [mm]
>> + - panel-height-mm: physical panel height [mm]
>
> These are also implied by compatible.
>
Accepted.
>> +Example:
>> +panel at 0 {
>> +compatible = "himax,hx8369a-dsi";
>> +reg = <0>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <&pinctrl_mipi_panel>;
>> +reset-gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
>> +reset-delay = <120>;
>> +bs2-gpios = <&gpio6 14 GPIO_ACTIVE_HIGH>;
>> +data-lanes = <2>;
>> +panel-width-mm = <45>;
>> +panel-height-mm = <76>;
>> +bs = <10>;
>> +status = "okay";
>
> status = "okay" is the default, so it probably shouldn't be in this
> example.
>
Accepted.
>> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
>> index 024e98e..f1a5b58 100644
>> --- a/drivers/gpu/drm/panel/Kconfig
>> +++ b/drivers/gpu/drm/panel/Kconfig
>> @@ -40,4 +40,10 @@ config DRM_PANEL_SHARP_LQ101R1SX01
>>To compile this driver as a module, choose M here: the module
>>will be called panel-sharp-lq101r1sx01.
>>
>> +config DRM_PANEL_HX8369A
>> +tristate "HX8369A panel"
>> +depends on OF
>> +select DRM_MIPI_DSI
>> +select VIDEOMODE_HELPERS
>> +
>
> This should be sorted alphabetically. I think it would also be a good
> idea to use a HIMAX prefix here, just to reduce the potential for name
> clashes.
Accepted.
>
>> diff --gi
[Bug 78951] gl_PrimitiveID is zero if no geometry shader is present
https://bugs.freedesktop.org/show_bug.cgi?id=78951 --- Comment #6 from Marek Olšák --- The hw GS mode A should be able to do this, which is basically a VS which gets PrimitiveID on the input. -- 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/20141217/cc9dea5a/attachment.html>
[Bug 87278] Packet0 not allowed and GPU fault detected errors with Serious Engine games
https://bugs.freedesktop.org/show_bug.cgi?id=87278 --- Comment #7 from Daniel Scharrer --- (In reply to Michel Dänzer from comment #3) > Does the environment variable R600_DEBUG=nodma avoid this problem? No change here either. But now that you mention it: IIRC SS3 did sometimes lock up the GPU before http://cgit.freedesktop.org/mesa/mesa/commit/?id=ae4536b4f71cbe76230ea7edc7eb4d6041e651b4 (In reply to Michel Dänzer from comment #6) > Can you generate an apitrace which reproduces the Packet0 error and/or GPUVM > faults? Here is one with VM faults from starting The Talos Principle Public Test, just up to the main menu: http://constexpr.org/tmp/Talos_Demo.trace (149 MiB) Sometimes there is also a Packet0 error at the end. Didn't get it while recording, got it 2/3 times while replaying. -- 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/20141217/f7825aa7/attachment.html>
[PATCH RFC 12/15] ARM: dts: imx6qdl-sabresd: Add support for TRULY TFT480800-16-E MIPI DSI panel
2014-12-10 22:07 GMT+08:00 Thierry Reding :
>
> On Wed, Dec 10, 2014 at 04:37:25PM +0800, Liu Ying wrote:
> [...]
> > diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> [...]
> > +&mipi_dsi {
> > + status = "okay";
> > +
> > + panel at 0 {
> > + compatible = "himax,hx8369a-dsi";
> > + reg = <0>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_mipi_panel>;
> > + reset-gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
> > + reset-delay = <120>;
> > + bs2-gpios = <&gpio6 14 GPIO_ACTIVE_HIGH>;
> > + data-lanes = <2>;
> > + panel-width-mm = <45>;
> > + panel-height-mm = <76>;
> > + bs = <10>;
> > + status = "okay";
> > +
> > + display-timings {
> > + native-mode = <&timing1>;
> > + timing1: truly-tft480800-16-e {
>
> This is the only place where Truly is mentioned. The panel vendor is
> either Truly or it is Himax, it can't be a mix. From this example and
> your patch description it seems like Truly is the manufacturer of the
> panel, hence the panel compatible should really be:
>
> compatible = "truly,tft480800-16-e";
>
May I change the compatible string to be '"truly,tft480800-16-e-dsi"?
This way, the dsi driver may match the dsi device.
>
> That it uses an HX8369A chip internally is really secondary, at least
> regarding the binding. If we ever get need to support multiple panels
> with the same driver IC the proper solution would be to abstract that
> code out into a helper library that is used from the various panel
> drivers.
>
> Thierry
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20141217/74bc0699/attachment.html>
[PATCH RFC 09/15] drm: imx: Add MIPI DSI host controller driver
On Wed, Dec 17, 2014 at 05:44:33PM +0800, Liu Ying wrote:
> Hi Thierry,
>
> Sorry for the late response.
> I tried to address almost all your comments locally first.
> More feedback below.
>
> On 12/10/2014 09:16 PM, Thierry Reding wrote:
> >On Wed, Dec 10, 2014 at 04:37:22PM +0800, Liu Ying wrote:
> >>+static int check_status(struct imx_mipi_dsi *dsi, u32 reg, u32 status,
> >>+ int timeout, bool to_set)
> >>+{
> >>+ u32 val;
> >>+ bool out = false;
> >>+
> >>+ val = dsi_read(dsi, reg);
> >>+ for (;;) {
> >>+ out = to_set ? (val & status) : !(val & status);
> >>+ if (out)
> >>+ break;
> >>+
> >>+ if (!timeout--)
> >>+ return -EFAULT;
> >>+
> >>+ msleep(1);
> >>+ val = dsi_read(dsi, reg);
> >>+ }
> >>+ return 0;
> >>+}
> >
> >You should probably use a properly timed loop here. msleep() isn't
> >guaranteed to return after exactly one millisecond, so your timeout is
> >never going to be accurate. Something like the following would be better
> >in my opinion:
> >
> > timeout = jiffies + msecs_to_jiffies(timeout);
> >
> > while (time_before(jiffies, timeout)) {
> > ...
> > }
> >
> >Also timeout should be unsigned long in that case.
>
> Accepted.
Actually, that's a bad example: what we want to do is to assess success
after we wait, before we decide that something has failed. In other
words, we don't want to wait, and decide that we failed without first
checking for success.
In any case, returning -EFAULT is not sane: EFAULT doesn't mean "fault"
it means "Bad address", and it is returned to userspace to mean that
userspace passed the kernel a bad address. That definition does /not/
fit what's going on here.
timeout = jiffies + msecs_to_jiffies(timeout);
do {
val = dsi_read(dsi, reg);
out = to_set ? (val & status) : !(val & status);
if (out)
break;
if (time_is_after_jiffies(timeout))
return -ETIMEDOUT;
msleep(1);
} while (1);
return 0;
would be better: we only fail immediately after we have checked whether
we succeeded, and we also do the first check immediately.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
[PATCH 13/13] RFC: drm: Atomic modeset ioctl
On Wed, Dec 17, 2014 at 06:31:13PM +0900, Michel Dänzer wrote:
> On 17.12.2014 16:20, Pekka Paalanen wrote:
> > On Wed, 17 Dec 2014 11:48:51 +0900
> > Michel Dänzer wrote:
> >
> >> On 17.12.2014 08:05, Rob Clark wrote:
> >>> The atomic modeset ioctl can be used to push any number of new values
> >>> for object properties. The driver can then check the full device
> >>> configuration as single unit, and try to apply the changes atomically.
> >>>
> >>> The ioctl simply takes a list of object IDs and property IDs and their
> >>> values.
> >>
> >> [...]
> >>
> >>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> >>> index 86574b0..3459778 100644
> >>> --- a/include/uapi/drm/drm_mode.h
> >>> +++ b/include/uapi/drm/drm_mode.h
> >>> @@ -519,4 +519,25 @@ struct drm_mode_destroy_dumb {
> >>> uint32_t handle;
> >>> };
> >>>
> >>> +/* page-flip flags are valid, plus: */
> >>> +#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> >>> +#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> >>> +
> >>> +#define DRM_MODE_ATOMIC_FLAGS (\
> >>> + DRM_MODE_PAGE_FLIP_EVENT |\
> >>> + DRM_MODE_PAGE_FLIP_ASYNC |\
> >>> + DRM_MODE_ATOMIC_TEST_ONLY |\
> >>> + DRM_MODE_ATOMIC_NONBLOCK)
> >>> +
> >>> +struct drm_mode_atomic {
> >>> + __u32 flags;
> >>> + __u32 count_objs;
> >>> + __u64 objs_ptr;
> >>> + __u64 count_props_ptr;
> >>> + __u64 props_ptr;
> >>> + __u64 prop_values_ptr;
> >>> + __u64 blob_values_ptr; /* remove? */
> >>> + __u64 user_data;
> >>> +};
> >>> +
> >>> #endif
> >>>
> >>
> >> The new ioctl(s) should take an explicit parameter specifying when the
> >> changes should take effect. And since variable refresh rate displays are
> >> becoming mainstream, that parameter should probably be a timestamp
> >> rather than a frame counter.
> >
> > That sounds cool to me, but also a rabbit hole. While having worked on
> > the Wayland Presentation queueing extension, I'd like to ask the
> > following questions:
> >
> > - If you set the atomic kick to happen in the future, is there any way
> > to cancel it? I'd be ok with not being able to cancel initially, but
> > if one wants to add that later, we should already know how to
> > reference this atomic submission in the cancel request. What if user
> > space has a bug and schedules an update at one hour or three days from
> > now, how would we abort that?
> >
> > - Can you VT-switch or drop DRM master if you have a pending atomic
> > update?
> >
> > - Should one be able to set multiple pending atomic updates?
> >
> > - If I schedule an atomic update for one CTRC, can I schedule another
> > update for another CRTC before the first one completes? Or am I
> > forced to gather all updates over all outputs in the same atomic
> > submission even if I don't care about or want inter-output sync and
> > the outputs might even be running at different refresh rates?
> > (Actually, this seems to be a valid question even without any target
> > time parameter.)
> >
> > - If there can be multiple pending atomic updates on the same DRM
> > device, is there any way to guarantee that the
> > DRM_MODE_ATOMIC_TEST_ONLY results will be accurate also when the
> > atomic update actually kicks in? Another update may have changed the
> > configuration before this update kicks in, which means the overall
> > state isn't the same that was tested.
> >
> > - Does a pending atomic update prevent immediate (old style) KMS
> > changes?
> >
> > - Assuming hardware cannot do arbitrary time updates, how do you round
> > the given timestamp? Strictly "not before" given time? Round to
> > nearest possible time? The effect of required vs. unwanted sync to
> > vblank?
> >
> > - How would user space match the page flip event to the atomic
> > submission it did?
> >
> > I wonder if there is a way to postpone these hard(?) questions, so that
> > we could have atomic sooner and add scheduling later? I would imagine
> > solving everything above is quite some work.
>
> I agree. The main reason I brought it up is because I'd like to avoid
> getting into the same situation as with the current
> DRM_IOCTL_MODE_PAGE_FLIP ioctl, which doesn't explicitly communicate
> between userspace and kernel when the flip is supposed/expected to
> occur. We recently had to jump through some hoops in the radeon kernel
> driver to prevent flips from occurring sooner than expected by userspace.
The current approach is to ask for a vblank event 1 frame earlier in the
ddx and schedule the flip when you receive that. And hope for the best.
There shouldn't be anything really required in the kernel.
> But I also think it would be a shame at this point to add new ioctls
> which aren't designed with variable refresh rate displays in mind.
Let's please not aim for the kitchen sink, we won't get this in anytime
soon otherwise. And there is _lots_ more little features all over that we
want but don't yet have:
- update rectangle for manual/selective upd
[Bug 89661] Kernel panic when trying use amdkfd driver on Kaveri
https://bugzilla.kernel.org/show_bug.cgi?id=89661 Oded Gabbay changed: What|Removed |Added Attachment #160721|0 |1 is obsolete|| Attachment #160751|0 |1 is obsolete|| --- Comment #13 from Oded Gabbay --- Created attachment 160951 --> https://bugzilla.kernel.org/attachment.cgi?id=160951&action=edit workaround for the module order problem -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH V8 00/14] drm/exynos: few patches to enhance bridge chip support
Hi Javier, On Wednesday 17 December 2014 10:31:41 Javier Martinez Canillas wrote: > On 12/16/2014 12:37 AM, Laurent Pinchart wrote: > > Hi Javier, > > > >> 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. > > > > I don't have time to review the series in details right now, but I'm happy > > with the DT bindings, and have no big issue with the rest of the patches. > > I > > don't really like the of_drm_find_bridge() concept introduced in 03/14 but > > I won't nack it given lack of time to implement an alternative proposal. > > It's an internal API, it can always be reworked later anyway. > > Thanks a lot for taking the time to look at the DT bindings, You're welcome. Sorry for the long delay. > then I guess that the series are finally ready to be merged? >From my point of view, yes. > Ajay's series don't apply cleanly anymore because it has been a while since > he posted it but he can rebase on top of 3.19-rc1 once it is released and > re-resend. -- Regards, Laurent Pinchart
[Bug 89661] Kernel panic when trying use amdkfd driver on Kaveri
https://bugzilla.kernel.org/show_bug.cgi?id=89661 --- Comment #14 from Oded Gabbay --- I attached a new patch which should solve you the problem when compiling all the drivers into the kernel image. This is a hacky workaround, so this is not the final solution, but it will help you continue with your setup, I hope. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 89661] Kernel panic when trying use amdkfd driver on Kaveri
https://bugzilla.kernel.org/show_bug.cgi?id=89661 Oded Gabbay changed: What|Removed |Added Attachment #160951|0 |1 is obsolete|| --- Comment #15 from Oded Gabbay --- Created attachment 160961 --> https://bugzilla.kernel.org/attachment.cgi?id=160961&action=edit hacky workaround for module order problem -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 86011] [drm] "Memory manager not clean during takedown" after vga-switcheroo turn off nvidia card
https://bugzilla.kernel.org/show_bug.cgi?id=86011 --- Comment #7 from JoaquÃn AramendÃa --- Confirmed. It was also backported today to 3.17.y branch. So it's fixed for good -- You are receiving this mail because: You are watching someone on the CC list of the bug.
[PATCH 0/3] drm: Basic mode sanity checks
From: Ville Syrjälä We had a bug in i915 land recently where X passed in a zeroed mode with mode_valid=1 to setcrtc. That didn't go down so well and caused a div-by-zero in i915. For a long time I've been thinking that we need some real checks to validate the modes we feed into the hardware. I started to sketch out something for that but it quickly turned into a bit of a nightmare with the whole panel fitter, stereo 3D, SDVO etc. special cases we have in i915. So I gave up on that for now, and instead cooked up this small series to add some basic sanity checks to the mode validation, and also apply the same sanity checks to user provided modes. This is enough to prevent the div-by-zero in i915 with buggy X. The risk is that we start to reject some modes that more or less worked by accident before. Given how lax we've been in handling the panel fitter on i915 for instance, this could be a real problem if people have been useing custom modes that have been filled out only partially. But I'm hoping the number of users doing that is so small that we can risk it. The other concern is drivers which might also provide funky modes from their .get_modes(). I tried to look at all the drivers a bit, and most produce modes via EDID, CVT, DMT, or drm_display_mode_from_videomode(). All of those look safe, except mode->clock might be an issue with drm_display_mode_from_videomode(), but hopefully there's some kind of clock provided in most cases. I didn't dig through all the nooks and crannies in all drivers though, so may have missed something. Ville Syrjälä (3): drm: Reorganize probed mode validation drm: Perform basic sanity checks on probed modes drm: Do basic sanity checks for user modes drivers/gpu/drm/drm_crtc.c | 6 drivers/gpu/drm/drm_modes.c| 56 ++ drivers/gpu/drm/drm_probe_helper.c | 43 ++--- include/drm/drm_modes.h| 6 ++-- 4 files changed, 74 insertions(+), 37 deletions(-) -- 2.0.4
[PATCH 1/3] drm: Reorganize probed mode validation
From: Ville Syrjälä
Make drm_mode_validate_size() and drm_mode_validate_flag() deal with a
single mode instead of having each iterate through the mode list.
The hope is that in the future we might be able to share various mode
validation functions between modeset and get_modes.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_modes.c| 24 +++
drivers/gpu/drm/drm_probe_helper.c | 40 +-
include/drm/drm_modes.h| 5 ++---
3 files changed, 32 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index d1b7d20..61a4a9a 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -907,8 +907,7 @@ EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
/**
* drm_mode_validate_size - make sure modes adhere to size constraints
- * @dev: DRM device
- * @mode_list: list of modes to check
+ * @mode: mode to check
* @maxX: maximum width
* @maxY: maximum height
*
@@ -916,20 +915,21 @@ EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
* limitations of the DRM device/connector. If a mode is too big its status
* memeber is updated with the appropriate validation failure code. The list
* itself is not changed.
+ *
+ * Returns:
+ * The mode status
*/
-void drm_mode_validate_size(struct drm_device *dev,
- struct list_head *mode_list,
- int maxX, int maxY)
+enum drm_mode_status
+drm_mode_validate_size(const struct drm_display_mode *mode,
+ int maxX, int maxY)
{
- struct drm_display_mode *mode;
+ if (maxX > 0 && mode->hdisplay > maxX)
+ return MODE_VIRTUAL_X;
- list_for_each_entry(mode, mode_list, head) {
- if (maxX > 0 && mode->hdisplay > maxX)
- mode->status = MODE_VIRTUAL_X;
+ if (maxY > 0 && mode->vdisplay > maxY)
+ return MODE_VIRTUAL_Y;
- if (maxY > 0 && mode->vdisplay > maxY)
- mode->status = MODE_VIRTUAL_Y;
- }
+ return MODE_OK;
}
EXPORT_SYMBOL(drm_mode_validate_size);
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index 6857e9a..c637bd9 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -58,28 +58,23 @@
static bool drm_kms_helper_poll = true;
module_param_named(poll, drm_kms_helper_poll, bool, 0600);
-static void drm_mode_validate_flag(struct drm_connector *connector,
- int flags)
+static enum drm_mode_status
+drm_mode_validate_flag(const struct drm_display_mode *mode,
+ int flags)
{
- struct drm_display_mode *mode;
+ if ((mode->flags & DRM_MODE_FLAG_INTERLACE) &&
+ !(flags & DRM_MODE_FLAG_INTERLACE))
+ return MODE_NO_INTERLACE;
- if (flags == (DRM_MODE_FLAG_DBLSCAN | DRM_MODE_FLAG_INTERLACE |
- DRM_MODE_FLAG_3D_MASK))
- return;
+ if ((mode->flags & DRM_MODE_FLAG_DBLSCAN) &&
+ !(flags & DRM_MODE_FLAG_DBLSCAN))
+ return MODE_NO_DBLESCAN;
- list_for_each_entry(mode, &connector->modes, head) {
- if ((mode->flags & DRM_MODE_FLAG_INTERLACE) &&
- !(flags & DRM_MODE_FLAG_INTERLACE))
- mode->status = MODE_NO_INTERLACE;
- if ((mode->flags & DRM_MODE_FLAG_DBLSCAN) &&
- !(flags & DRM_MODE_FLAG_DBLSCAN))
- mode->status = MODE_NO_DBLESCAN;
- if ((mode->flags & DRM_MODE_FLAG_3D_MASK) &&
- !(flags & DRM_MODE_FLAG_3D_MASK))
- mode->status = MODE_NO_STEREO;
- }
+ if ((mode->flags & DRM_MODE_FLAG_3D_MASK) &&
+ !(flags & DRM_MODE_FLAG_3D_MASK))
+ return MODE_NO_STEREO;
- return;
+ return MODE_OK;
}
static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
@@ -163,18 +158,19 @@ static int
drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
drm_mode_connector_list_update(connector, merge_type_bits);
- if (maxX && maxY)
- drm_mode_validate_size(dev, &connector->modes, maxX, maxY);
-
if (connector->interlace_allowed)
mode_flags |= DRM_MODE_FLAG_INTERLACE;
if (connector->doublescan_allowed)
mode_flags |= DRM_MODE_FLAG_DBLSCAN;
if (connector->stereo_allowed)
mode_flags |= DRM_MODE_FLAG_3D_MASK;
- drm_mode_validate_flag(connector, mode_flags);
list_for_each_entry(mode, &connector->modes, head) {
+ mode->status = drm_mode_validate_size(mode, maxX, maxY);
+
+ if (mode->status == MODE_OK)
+ mode->status = drm_mode_validate_flag(mode, mode_flags);
+
if (mode->
[PATCH 2/3] drm: Perform basic sanity checks on probed modes
From: Ville Syrjälä
Make sure the timings of probed modes at least pass some very basic
sanity checks.
The checks include:
- clock,hdisplay,vdisplay are non zero
- sync pulse fits within the blanking period
- htotal,vtotal are big enough
I have not checked all the drivers to see if the modes the generate
might violate these constraints. I'm hoping not, because that would mean
either abandoning the idea of doing this from the core code, or fixing
the drivers.
I'm not entirely sure about limiting the sync pulse to the blanking
period. Intel hardware doesn't support such things, but some other
hardware might. However at least HDMI doesn't allow having sync pulse
edges within the active period, so I'm thinking the check is probably
OK to have in the common code.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_modes.c| 32
drivers/gpu/drm/drm_probe_helper.c | 5 -
include/drm/drm_modes.h| 1 +
3 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 61a4a9a..543f8c6 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -906,6 +906,38 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct
drm_display_mode *mode1,
EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
/**
+ * drm_mode_validate_basic - make sure the mode is somewhat sane
+ * @mode: mode to check
+ *
+ * Check that the mode timings are at least somewhat reasonable.
+ * Any hardware specific limits are left up for each driver to check.
+ *
+ * Returns:
+ * The mode status
+ */
+enum drm_mode_status
+drm_mode_validate_basic(const struct drm_display_mode *mode)
+{
+ if (mode->clock == 0)
+ return MODE_CLOCK_LOW;
+
+ if (mode->hdisplay == 0 ||
+ mode->hsync_start < mode->hdisplay ||
+ mode->hsync_end < mode->hsync_start ||
+ mode->htotal < mode->hsync_end)
+ return MODE_H_ILLEGAL;
+
+ if (mode->vdisplay == 0 ||
+ mode->vsync_start < mode->vdisplay ||
+ mode->vsync_end < mode->vsync_start ||
+ mode->vtotal < mode->vsync_end)
+ return MODE_V_ILLEGAL;
+
+ return MODE_OK;
+}
+EXPORT_SYMBOL(drm_mode_validate_basic);
+
+/**
* drm_mode_validate_size - make sure modes adhere to size constraints
* @mode: mode to check
* @maxX: maximum width
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index c637bd9..0b86ab7 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -166,7 +166,10 @@ static int
drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
mode_flags |= DRM_MODE_FLAG_3D_MASK;
list_for_each_entry(mode, &connector->modes, head) {
- mode->status = drm_mode_validate_size(mode, maxX, maxY);
+ mode->status = drm_mode_validate_basic(mode);
+
+ if (mode->status == MODE_OK)
+ mode->status = drm_mode_validate_size(mode, maxX, maxY);
if (mode->status == MODE_OK)
mode->status = drm_mode_validate_flag(mode, mode_flags);
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 6c53114..a36a5bf 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -217,6 +217,7 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct
drm_display_mode *mode1,
const struct drm_display_mode *mode2);
/* for use by the crtc helper probe functions */
+enum drm_mode_status drm_mode_validate_basic(const struct drm_display_mode
*mode);
enum drm_mode_status drm_mode_validate_size(const struct drm_display_mode
*mode,
int maxX, int maxY);
void drm_mode_prune_invalid(struct drm_device *dev,
--
2.0.4
[PATCH 3/3] drm: Do basic sanity checks for user modes
From: Ville Syrjälä
Currently userspace is allowed to pass in basiclly any kind of garbage
to setcrtc. Try to catch the cases where the timings make no sense
by passing the mode through drm_mode_validate_basic().
One concern here is that we now start to block some modes that have
worked in the past. It's at least possible with when using i915 with
LVDS/eDP. Previously we've just ignored everything but hdisplay/vdisplay
from the user mode and just overwritten the rest with the panel fixed
mode. So if someone has been passing a mode with just those populated
that would now stop working. If that is a real problem, we can't add
these checks to the core code and each driver would have to have its
own sanity checks. So fingers crossed...
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_crtc.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e79c8d3..9a1d0e8 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2612,6 +2612,12 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
goto out;
}
+ mode->status = drm_mode_validate_basic(mode);
+ if (mode->status != MODE_OK) {
+ ret = -EINVAL;
+ goto out;
+ }
+
drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
ret = drm_crtc_check_viewport(crtc, crtc_req->x, crtc_req->y,
--
2.0.4
[PULL] drm-intel-next-fixes
Hi Dave - Final i915 fixes pull before rc1, majority of them cc: stable. BR, Jani. The following changes since commit 9f49c37635d5c2a801f7670d5fbf0b25ec461f2c: drm/i915: save/restore GMBUS freq across suspend/resume on gen4 (2014-12-11 15:31:59 +0200) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-fixes-2014-12-17 for you to fetch changes up to 2c550183476dfa25641309ae9a28d30feed14379: drm/i915: Disable PSMI sleep messages on all rings around context switches (2014-12-16 15:07:53 +0200) Chris Wilson (3): drm/i915: Invalidate media caches on gen7 drm/i915: Force the CS stall for invalidate flushes drm/i915: Disable PSMI sleep messages on all rings around context switches Imre Deak (3): drm/i915: vlv: fix IRQ masking when uninstalling interrupts drm/i915: move RPS PM_IER enabling to gen6_enable_rps_interrupts drm/i915: sanitize RPS resetting during GPU reset drivers/gpu/drm/i915/i915_drv.c | 4 ++- drivers/gpu/drm/i915/i915_gem_context.c | 48 +++-- drivers/gpu/drm/i915/i915_irq.c | 18 ++--- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 28 --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ 6 files changed, 82 insertions(+), 22 deletions(-) -- Jani Nikula, Intel Open Source Technology Center
[PATCH 1/6] gpu: ipu-di: Add ipu_di_adjust_videomode()
Steve, On Mon, Dec 15, 2014 at 10:29 PM, wrote: > From: Jiada Wang > > On some monitors, high resolution modes are not working, exhibiting > pixel column truncation problems (for example, 1280x1024 displays as > 1280x1022). > > The function ipu_di_adjust_videomode() aims to fix these issues by > adjusting a passed videomode to IPU restrictions. The function can > be called from the drm_crtc_helper_funcs->mode_fixup() methods. > > Signed-off-by: Jiada Wang > Signed-off-by: Deepak Das > Signed-off-by: Steve Longerbeam Looks like you missed to add Philipp on Cc.
[PATCH 01/13] drm: allow property validation for refcnted props
On Tue, Dec 16, 2014 at 06:05:29PM -0500, Rob Clark wrote:
> We can already have object properties, which technically can be fb's.
> Switch the property validation logic over to a get/put style interface
> so it can take a ref to refcnt'd objects, and then drop that ref after
> the driver has a chance to take it's own ref.
>
> Signed-off-by: Rob Clark
Yeah I think of all the options we've looked at this is the best wrt to
interface safety.
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc.c | 58
> ++
> 1 file changed, 43 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5213da4..5ee4b88 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -4193,12 +4193,22 @@ int drm_mode_connector_update_edid_property(struct
> drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_mode_connector_update_edid_property);
>
> -static bool drm_property_change_is_valid(struct drm_property *property,
> - uint64_t value)
> +/* Some properties could refer to dynamic refcnt'd objects, or things that
> + * need special locking to handle lifetime issues (ie. to ensure the prop
> + * value doesn't become invalid part way through the property update due to
> + * race). The value returned by reference via 'obj' should be passed back
> + * to drm_property_change_valid_put() after the property is set (and the
> + * object to which the property is attached has a chance to take it's own
> + * reference).
> + */
> +static bool drm_property_change_valid_get(struct drm_property *property,
> + uint64_t value, struct drm_mode_object
> **ref)
> {
> if (property->flags & DRM_MODE_PROP_IMMUTABLE)
> return false;
>
> + *ref = NULL;
> +
> if (drm_property_type_is(property, DRM_MODE_PROP_RANGE)) {
> if (value < property->values[0] || value > property->values[1])
> return false;
> @@ -4219,19 +4229,23 @@ static bool drm_property_change_is_valid(struct
> drm_property *property,
> /* Only the driver knows */
> return true;
> } else if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
> - struct drm_mode_object *obj;
> /* a zero value for an object property translates to null: */
> if (value == 0)
> return true;
> - /*
> - * NOTE: use _object_find() directly to bypass restriction on
> - * looking up refcnt'd objects (ie. fb's). For a refcnt'd
> - * object this could race against object finalization, so it
> - * simply tells us that the object *was* valid. Which is good
> - * enough.
> - */
> - obj = _object_find(property->dev, value, property->values[0]);
> - return obj != NULL;
> +
> + /* handle refcnt'd objects specially: */
> + if (property->values[0] == DRM_MODE_OBJECT_FB) {
> + struct drm_framebuffer *fb;
> + fb = drm_framebuffer_lookup(property->dev, value);
> + if (fb) {
> + *ref = &fb->base;
> + return true;
> + } else {
> + return false;
> + }
> + } else {
> + return _object_find(property->dev, value,
> property->values[0]) != NULL;
> + }
> } else {
> int i;
> for (i = 0; i < property->num_values; i++)
> @@ -4241,6 +4255,18 @@ static bool drm_property_change_is_valid(struct
> drm_property *property,
> }
> }
>
> +static void drm_property_change_valid_put(struct drm_property *property,
> + struct drm_mode_object *ref)
> +{
> + if (!ref)
> + return;
> +
> + if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
> + if (property->values[0] == DRM_MODE_OBJECT_FB)
> + drm_framebuffer_unreference(obj_to_fb(ref));
> + }
> +}
> +
> /**
> * drm_mode_connector_property_set_ioctl - set the current value of a
> connector property
> * @dev: DRM device
> @@ -4429,8 +4455,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device
> *dev, void *data,
> struct drm_mode_object *arg_obj;
> struct drm_mode_object *prop_obj;
> struct drm_property *property;
> - int ret = -EINVAL;
> - int i;
> + int i, ret = -EINVAL;
> + struct drm_mode_object *ref;
>
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> return -EINVAL;
> @@ -4460,7 +4486,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device
> *dev, void *data,
> }
> property = obj_to_property(prop_obj);
>
> - if (!drm_property_change_is_valid(property, arg->va
drm: rockchip: Add basic drm driver
On Wed, Dec 17, 2014 at 08:56:25AM +0800, Mark yao wrote: > Hello Dan, > > Thanks for your review, I will fix it soon. > Do you use a script to find out these issues? I was interested in > this script. > Can you teach me how to find this script and use it? The first one is a smatch warning. http://smatch.sf.net. The other one is from a private Smatch check that I haven't pushed yet. regards, dan carpenter
[PATCH 04/13] drm: add atomic_set_property wrappers
On Tue, Dec 16, 2014 at 06:05:32PM -0500, Rob Clark wrote:
> As we add properties for all the standard plane/crtc/connector
> attributes (in preperation for the atomic ioctl), we are going to want
> to handle core state in core (rather than per driver). Intercepting the
> core properties will be easier if the atomic_set_property vfuncs are not
> called directly, but instead have a mandatory wrapper function (which
> will later serve as the point to intercept core properties).
>
> Signed-off-by: Rob Clark
Tiny copypasta nit in one comment.
> ---
> drivers/gpu/drm/drm_atomic.c| 78
> +
> drivers/gpu/drm/drm_atomic_helper.c | 12 +++---
> include/drm/drm_atomic.h| 9 +
> include/drm/drm_crtc.h | 3 ++
> 4 files changed, 96 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index ff5f034..1261ade 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -217,6 +217,28 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>
> /**
> + * drm_atomic_crtc_set_property - set property on connector
s/connector/crtc/
> + * @crtc: the drm CRTC to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling crtc->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> + struct drm_crtc_state *state, struct drm_property *property,
> + uint64_t val)
> +{
> + if (crtc->funcs->atomic_set_property)
> + return crtc->funcs->atomic_set_property(crtc, state, property,
> val);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_crtc_set_property);
> +
> +/**
> * drm_atomic_get_plane_state - get plane state
> * @state: global atomic state object
> * @plane: plane to get state object for
> @@ -272,6 +294,28 @@ drm_atomic_get_plane_state(struct drm_atomic_state
> *state,
> EXPORT_SYMBOL(drm_atomic_get_plane_state);
>
> /**
> + * drm_atomic_plane_set_property - set property on plane
> + * @plane: the drm plane to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling plane->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_plane_set_property(struct drm_plane *plane,
> + struct drm_plane_state *state, struct drm_property *property,
> + uint64_t val)
> +{
> + if (plane->funcs->atomic_set_property)
> + return plane->funcs->atomic_set_property(plane, state,
> property, val);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_plane_set_property);
> +
> +/**
> * drm_atomic_get_connector_state - get connector state
> * @state: global atomic state object
> * @connector: connector to get state object for
> @@ -343,6 +387,40 @@ drm_atomic_get_connector_state(struct drm_atomic_state
> *state,
> EXPORT_SYMBOL(drm_atomic_get_connector_state);
>
> /**
> + * drm_atomic_connector_set_property - set property on connector.
> + * @connector: the drm connector to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling connector->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_connector_set_property(struct drm_connector *connector,
> + struct drm_connector_state *state, struct drm_property
> *property,
> + uint64_t val)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (property == config->dpms_property) {
> + /* setting DPMS property requires special handling, which
> + * is done in legacy setprop path for us. Disallow (for
> + * now?) atomic writes to DPMS property:
> + */
> + return -EINVAL;
> + } else if (connector->funcs->atomic_set_property) {
> + return connector->funcs->atomic_set_property(connector,
> + state, property, val);
> + } else {
> + return -EINVAL;
> + }
> +}
> +EXPORT_SYMBOL(drm_atomic_connector_set_property);
> +
> +/**
> * drm_atomic_set_crtc_for_plane - set crtc for plane
> * @state: the incoming atomic state
> * @plane: the plane whose incoming state to update
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 4a78a77..d42fdb1 100644
> --- a/drivers/gpu/d
[PATCH 05/13] drm: add atomic_get_property
On Tue, Dec 16, 2014 at 06:05:33PM -0500, Rob Clark wrote:
> Since we won't be using the obj->properties->values[] array to shadow
> property values for atomic drivers, we are going to need a vfunc for
> getting prop values. Add that along w/ mandatory wrapper fxns.
>
> Signed-off-by: Rob Clark
Same nit here
> ---
> drivers/gpu/drm/drm_atomic.c | 76
>
> include/drm/drm_atomic.h | 9 ++
> include/drm/drm_crtc.h | 18 +++
> 3 files changed, 103 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 1261ade..4099b44 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -239,6 +239,28 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> EXPORT_SYMBOL(drm_atomic_crtc_set_property);
>
> /**
> + * drm_atomic_crtc_get_property - get property on connector
s/connector/crtc/
> + * @crtc: the drm CRTC to get a property on
> + * @state: the state object with the property value to read
> + * @property: the property to get
> + * @val: the property value (returned by reference)
> + *
> + * Use this instead of calling crtc->atomic_get_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> + const struct drm_crtc_state *state,
> + struct drm_property *property, uint64_t *val)
> +{
> + if (crtc->funcs->atomic_get_property)
> + return crtc->funcs->atomic_get_property(crtc, state, property,
> val);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_crtc_get_property);
> +
> +/**
> * drm_atomic_get_plane_state - get plane state
> * @state: global atomic state object
> * @plane: plane to get state object for
> @@ -316,6 +338,28 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> EXPORT_SYMBOL(drm_atomic_plane_set_property);
>
> /**
> + * drm_atomic_plane_get_property - get property on plane
> + * @plane: the drm plane to get a property on
> + * @state: the state object with the property value to read
> + * @property: the property to get
> + * @val: the property value (returned by reference)
> + *
> + * Use this instead of calling plane->atomic_get_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_plane_get_property(struct drm_plane *plane,
> + const struct drm_plane_state *state,
> + struct drm_property *property, uint64_t *val)
> +{
> + if (plane->funcs->atomic_get_property)
> + return plane->funcs->atomic_get_property(plane, state,
> property, val);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_plane_get_property);
> +
> +/**
> * drm_atomic_get_connector_state - get connector state
> * @state: global atomic state object
> * @connector: connector to get state object for
> @@ -421,6 +465,38 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> EXPORT_SYMBOL(drm_atomic_connector_set_property);
>
> /**
> + * drm_atomic_connector_get_property - get property on connector
> + * @connector: the drm connector to get a property on
> + * @state: the state object with the property value to read
> + * @property: the property to get
> + * @val: the property value (returned by reference)
> + *
> + * Use this instead of calling connector->atomic_get_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_connector_get_property(struct drm_connector *connector,
> + const struct drm_connector_state *state,
> + struct drm_property *property, uint64_t *val)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (property == config->dpms_property) {
> + *val = connector->dpms;
> + } else if (connector->funcs->atomic_get_property) {
> + return connector->funcs->atomic_get_property(connector,
> + state, property, val);
> + } else {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_connector_get_property);
> +
> +/**
> * drm_atomic_set_crtc_for_plane - set crtc for plane
> * @state: the incoming atomic state
> * @plane: the plane whose incoming state to update
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index b0834dc..b34224a 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -41,18 +41,27 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> struct drm_crtc_state *state, struct drm_property *property,
> uint64_t val);
> +int drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> + const struct drm_crtc_state *state,
> + struct drm_property *property,
[PATCH 00/13] Atomic Properties
On Tue, Dec 16, 2014 at 6:05 PM, Rob Clark wrote: > Now that we have core atomic helpers, let's expose that to userspace! > The first step here is to, for drivers supporting atomic, expose all > the core plane/CRTC/connector state as properties. Once this is in > place, all that remains is the atomic ioctl to allow userspace to > atomically set a bunch of properties on a bunch of objects (RFC patch > at the end). > > All of the core properties have their set/get/check's handled by the > drm_atomic core, so drivers needn't be bothered about that. > > NOTE: that there is one notable omission here. The 'MODE' property > on the CRTC. Without this, you can do atomic pageflip but not yet > atomic modeset. I am debating between three paths for handling MODE: > > 1) Add back blob property support to atomic ioctl > 2) Add ioctls to create/destroy blob properties, and then simply > pass blob property id to atomic ioctl. > 3) Stick our head in the sands and expose mode as a bunch of descrete > properties. > > Of these, at the moment I prefer #2, since it is more consistent with > how userspace currently reads blob properties, and it avoids making the > atomic ioctl function even more gnarly. > Sounds reasonable. > The whole patchset can also be found at: > > http://cgit.freedesktop.org/~robclark/linux/log/?h=atomic-properties > git://people.freedesktop.org/~robclark/linux atomic-properties > Unsurprisingly, since I took a pass last week, the set looks good to me (aside from 2 comment nits, which I trust you can fix up). Feel free to add my Rb to all 13 patches. Reviewed-by: Sean Paul > Rob Clark (13): > drm: allow property validation for refcnted props > drm: store property instead of id in obj attachment > drm: get rid of direct property value access > drm: add atomic_set_property wrappers > drm: add atomic_get_property > drm: add atomic hook to read property plus helper > drm: small property creation cleanup > drm: tweak getconnector locking > drm/atomic: atomic_check functions > drm/atomic: atomic plane properties > drm/atomic: atomic connector properties > drm/msm: atomic property support > RFC: drm: Atomic modeset ioctl > > Documentation/DocBook/drm.tmpl | 83 ++- > drivers/gpu/drm/drm_atomic.c | 613 > + > drivers/gpu/drm/drm_atomic_helper.c| 105 +++- > drivers/gpu/drm/drm_crtc.c | 230 ++-- > drivers/gpu/drm/drm_ioctl.c| 1 + > drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 1 + > drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c | 1 + > drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c | 1 + > drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 1 + > drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 1 + > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 2 + > drivers/gpu/drm/msm/msm_drv.c | 1 + > include/drm/drm_atomic.h | 22 + > include/drm/drm_atomic_helper.h| 2 + > include/drm/drm_crtc.h | 50 +- > include/uapi/drm/drm.h | 1 + > include/uapi/drm/drm_mode.h| 21 + > 17 files changed, 1063 insertions(+), 73 deletions(-) > > -- > 2.1.0 >
[PATCH 02/13] drm: store property instead of id in obj attachment
On Tue, Dec 16, 2014 at 06:05:30PM -0500, Rob Clark wrote:
> Keep property pointer, instead of id, in per mode-object attachments.
> This will simplify things in later patches.
>
> Signed-off-by: Rob Clark
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc.c | 17 -
> include/drm/drm_crtc.h | 7 ++-
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5ee4b88..2780a08 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2105,12 +2105,11 @@ int drm_mode_getconnector(struct drm_device *dev,
> void *data,
> prop_ptr = (uint32_t __user *)(unsigned
> long)(out_resp->props_ptr);
> prop_values = (uint64_t __user *)(unsigned
> long)(out_resp->prop_values_ptr);
> for (i = 0; i < connector->properties.count; i++) {
> - if (put_user(connector->properties.ids[i],
> - prop_ptr + copied)) {
> + struct drm_property *prop =
> connector->properties.properties[i];
> + if (put_user(prop->base.id, prop_ptr + copied)) {
> ret = -EFAULT;
> goto out;
> }
> -
> if (put_user(connector->properties.values[i],
>prop_values + copied)) {
> ret = -EFAULT;
> @@ -3822,7 +3821,7 @@ void drm_object_attach_property(struct drm_mode_object
> *obj,
> return;
> }
>
> - obj->properties->ids[count] = property->base.id;
> + obj->properties->properties[count] = property;
> obj->properties->values[count] = init_val;
> obj->properties->count++;
> }
> @@ -3847,7 +3846,7 @@ int drm_object_property_set_value(struct
> drm_mode_object *obj,
> int i;
>
> for (i = 0; i < obj->properties->count; i++) {
> - if (obj->properties->ids[i] == property->base.id) {
> + if (obj->properties->properties[i] == property) {
> obj->properties->values[i] = val;
> return 0;
> }
> @@ -3877,7 +3876,7 @@ int drm_object_property_get_value(struct
> drm_mode_object *obj,
> int i;
>
> for (i = 0; i < obj->properties->count; i++) {
> - if (obj->properties->ids[i] == property->base.id) {
> + if (obj->properties->properties[i] == property) {
> *val = obj->properties->values[i];
> return 0;
> }
> @@ -4413,8 +4412,8 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device
> *dev, void *data,
> prop_values_ptr = (uint64_t __user *)(unsigned long)
> (arg->prop_values_ptr);
> for (i = 0; i < props_count; i++) {
> - if (put_user(obj->properties->ids[i],
> - props_ptr + copied)) {
> + struct drm_property *prop =
> obj->properties->properties[i];
> + if (put_user(prop->base.id, props_ptr + copied)) {
> ret = -EFAULT;
> goto out;
> }
> @@ -4472,7 +4471,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device
> *dev, void *data,
> goto out;
>
> for (i = 0; i < arg_obj->properties->count; i++)
> - if (arg_obj->properties->ids[i] == arg->prop_id)
> + if (arg_obj->properties->properties[i]->base.id == arg->prop_id)
> break;
>
> if (i == arg_obj->properties->count)
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index b863298..02758e8 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -64,7 +64,12 @@ struct drm_mode_object {
> #define DRM_OBJECT_MAX_PROPERTY 24
> struct drm_object_properties {
> int count;
> - uint32_t ids[DRM_OBJECT_MAX_PROPERTY];
> + /* NOTE: if we ever start dynamically destroying properties (ie.
> + * not at drm_mode_config_cleanup() time), then we'd have to do
> + * a better job of detaching property from mode objects to avoid
> + * dangling property pointers:
> + */
> + struct drm_property *properties[DRM_OBJECT_MAX_PROPERTY];
> uint64_t values[DRM_OBJECT_MAX_PROPERTY];
> };
>
> --
> 2.1.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 03/13] drm: get rid of direct property value access
On Tue, Dec 16, 2014 at 06:05:31PM -0500, Rob Clark wrote:
> For atomic drivers, we won't use the values array but instead shunt
> things off to obj->atomic_get_property(). So to simplify things make
> all read/write of properties values go through the accessors.
>
> Signed-off-by: Rob Clark
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc.c | 19 +++
> include/drm/drm_crtc.h | 3 +++
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 2780a08..481bb25 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2106,12 +2106,17 @@ int drm_mode_getconnector(struct drm_device *dev,
> void *data,
> prop_values = (uint64_t __user *)(unsigned
> long)(out_resp->prop_values_ptr);
> for (i = 0; i < connector->properties.count; i++) {
> struct drm_property *prop =
> connector->properties.properties[i];
> + uint64_t val;
> +
> + ret = drm_object_property_get_value(&connector->base,
> prop, &val);
> + if (ret)
> + goto out;
> +
> if (put_user(prop->base.id, prop_ptr + copied)) {
> ret = -EFAULT;
> goto out;
> }
> - if (put_user(connector->properties.values[i],
> - prop_values + copied)) {
> + if (put_user(val, prop_values + copied)) {
> ret = -EFAULT;
> goto out;
> }
> @@ -4413,12 +4418,18 @@ int drm_mode_obj_get_properties_ioctl(struct
> drm_device *dev, void *data,
> (arg->prop_values_ptr);
> for (i = 0; i < props_count; i++) {
> struct drm_property *prop =
> obj->properties->properties[i];
> + uint64_t val;
> +
> + ret = drm_object_property_get_value(obj, prop, &val);
> + if (ret)
> + goto out;
> +
> if (put_user(prop->base.id, props_ptr + copied)) {
> ret = -EFAULT;
> goto out;
> }
> - if (put_user(obj->properties->values[i],
> - prop_values_ptr + copied)) {
> +
> + if (put_user(val, prop_values_ptr + copied)) {
> ret = -EFAULT;
> goto out;
> }
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 02758e8..61ab3e5 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -70,6 +70,9 @@ struct drm_object_properties {
>* dangling property pointers:
>*/
> struct drm_property *properties[DRM_OBJECT_MAX_PROPERTY];
> + /* do not read/write values directly, but use
> drm_object_property_get_value()
> + * and drm_object_property_set_value():
> + */
> uint64_t values[DRM_OBJECT_MAX_PROPERTY];
> };
>
> --
> 2.1.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 04/13] drm: add atomic_set_property wrappers
On Tue, Dec 16, 2014 at 06:05:32PM -0500, Rob Clark wrote:
> As we add properties for all the standard plane/crtc/connector
> attributes (in preperation for the atomic ioctl), we are going to want
> to handle core state in core (rather than per driver). Intercepting the
> core properties will be easier if the atomic_set_property vfuncs are not
> called directly, but instead have a mandatory wrapper function (which
> will later serve as the point to intercept core properties).
>
> Signed-off-by: Rob Clark
Two suggestions for comment improvements below. With that addressed:
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_atomic.c| 78
> +
> drivers/gpu/drm/drm_atomic_helper.c | 12 +++---
> include/drm/drm_atomic.h| 9 +
> include/drm/drm_crtc.h | 3 ++
> 4 files changed, 96 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index ff5f034..1261ade 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -217,6 +217,28 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>
> /**
> + * drm_atomic_crtc_set_property - set property on connector
> + * @crtc: the drm CRTC to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling crtc->atomic_set_property directly
Maybe clarify a bit more here. What about
"This function directly processes generic properties (both mandatory and
optional ones) and calls into the driver's ->atomic_set_property hook for
anything else. To esnure consistent behavior this wrapper must be used
instead of calling the driver hook directly, even for driver private
properties."
A bit much perhaps, but kerneldoc is cheap ;-)
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> + struct drm_crtc_state *state, struct drm_property *property,
> + uint64_t val)
> +{
> + if (crtc->funcs->atomic_set_property)
> + return crtc->funcs->atomic_set_property(crtc, state, property,
> val);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_crtc_set_property);
> +
> +/**
> * drm_atomic_get_plane_state - get plane state
> * @state: global atomic state object
> * @plane: plane to get state object for
> @@ -272,6 +294,28 @@ drm_atomic_get_plane_state(struct drm_atomic_state
> *state,
> EXPORT_SYMBOL(drm_atomic_get_plane_state);
>
> /**
> + * drm_atomic_plane_set_property - set property on plane
> + * @plane: the drm plane to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling plane->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_plane_set_property(struct drm_plane *plane,
> + struct drm_plane_state *state, struct drm_property *property,
> + uint64_t val)
> +{
> + if (plane->funcs->atomic_set_property)
> + return plane->funcs->atomic_set_property(plane, state,
> property, val);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_plane_set_property);
> +
> +/**
> * drm_atomic_get_connector_state - get connector state
> * @state: global atomic state object
> * @connector: connector to get state object for
> @@ -343,6 +387,40 @@ drm_atomic_get_connector_state(struct drm_atomic_state
> *state,
> EXPORT_SYMBOL(drm_atomic_get_connector_state);
>
> /**
> + * drm_atomic_connector_set_property - set property on connector.
> + * @connector: the drm connector to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling connector->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_connector_set_property(struct drm_connector *connector,
> + struct drm_connector_state *state, struct drm_property
> *property,
> + uint64_t val)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (property == config->dpms_property) {
> + /* setting DPMS property requires special handling, which
> + * is done in legacy setprop path for us. Disallow (for
> + * now?) atomic writes to DPMS property:
> + */
Argh, I still need to write those patches for dpms in atomic. But imo a
better comment would be:
/*
* Per-connector is a legacy-only feature, disallow for atomic
* updates.
[PATCH 05/13] drm: add atomic_get_property
On Tue, Dec 16, 2014 at 06:05:33PM -0500, Rob Clark wrote:
> Since we won't be using the obj->properties->values[] array to shadow
> property values for atomic drivers, we are going to need a vfunc for
> getting prop values. Add that along w/ mandatory wrapper fxns.
>
> Signed-off-by: Rob Clark
With Sean's nitpick: Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_atomic.c | 76
>
> include/drm/drm_atomic.h | 9 ++
> include/drm/drm_crtc.h | 18 +++
> 3 files changed, 103 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 1261ade..4099b44 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -239,6 +239,28 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> EXPORT_SYMBOL(drm_atomic_crtc_set_property);
>
> /**
> + * drm_atomic_crtc_get_property - get property on connector
> + * @crtc: the drm CRTC to get a property on
> + * @state: the state object with the property value to read
> + * @property: the property to get
> + * @val: the property value (returned by reference)
> + *
> + * Use this instead of calling crtc->atomic_get_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> + const struct drm_crtc_state *state,
> + struct drm_property *property, uint64_t *val)
> +{
> + if (crtc->funcs->atomic_get_property)
> + return crtc->funcs->atomic_get_property(crtc, state, property,
> val);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_crtc_get_property);
> +
> +/**
> * drm_atomic_get_plane_state - get plane state
> * @state: global atomic state object
> * @plane: plane to get state object for
> @@ -316,6 +338,28 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> EXPORT_SYMBOL(drm_atomic_plane_set_property);
>
> /**
> + * drm_atomic_plane_get_property - get property on plane
> + * @plane: the drm plane to get a property on
> + * @state: the state object with the property value to read
> + * @property: the property to get
> + * @val: the property value (returned by reference)
> + *
> + * Use this instead of calling plane->atomic_get_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_plane_get_property(struct drm_plane *plane,
> + const struct drm_plane_state *state,
> + struct drm_property *property, uint64_t *val)
> +{
> + if (plane->funcs->atomic_get_property)
> + return plane->funcs->atomic_get_property(plane, state,
> property, val);
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_plane_get_property);
> +
> +/**
> * drm_atomic_get_connector_state - get connector state
> * @state: global atomic state object
> * @connector: connector to get state object for
> @@ -421,6 +465,38 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> EXPORT_SYMBOL(drm_atomic_connector_set_property);
>
> /**
> + * drm_atomic_connector_get_property - get property on connector
> + * @connector: the drm connector to get a property on
> + * @state: the state object with the property value to read
> + * @property: the property to get
> + * @val: the property value (returned by reference)
> + *
> + * Use this instead of calling connector->atomic_get_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_connector_get_property(struct drm_connector *connector,
> + const struct drm_connector_state *state,
> + struct drm_property *property, uint64_t *val)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (property == config->dpms_property) {
> + *val = connector->dpms;
> + } else if (connector->funcs->atomic_get_property) {
> + return connector->funcs->atomic_get_property(connector,
> + state, property, val);
> + } else {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_connector_get_property);
> +
> +/**
> * drm_atomic_set_crtc_for_plane - set crtc for plane
> * @state: the incoming atomic state
> * @plane: the plane whose incoming state to update
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index b0834dc..b34224a 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -41,18 +41,27 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> struct drm_crtc_state *state, struct drm_property *property,
> uint64_t val);
> +int drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> + const struct drm_crtc_state *state,
> + struct drm_prope
[PATCH 06/13] drm: add atomic hook to read property plus helper
On Tue, Dec 16, 2014 at 06:05:34PM -0500, Rob Clark wrote:
> Once a driver is using atomic helpers for modeset, the next step is to
> switch over to atomic properties. To do this, plug in the helper
> function to your modeconfig funcs and be sure to implement the plane/
> crtc/connector atomic_{get,set}_property vfuncs for any of your mode-
> objects which have driver custom properties.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 46
> +
> drivers/gpu/drm/drm_crtc.c | 9
> include/drm/drm_atomic_helper.h | 2 ++
> include/drm/drm_crtc.h | 3 +++
> 4 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index d42fdb1..1a1ab34 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1703,6 +1703,52 @@ backoff:
> EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
>
> /**
> + * drm_atomic_helper_get_property - helper to read atomic property
> + * @obj: drm mode object whose property to read
> + * @property: the property to read
> + * @val: the read value, returned by reference
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_helper_get_property(struct drm_mode_object *obj,
> + struct drm_property *property, uint64_t *val)
> +{
> + struct drm_device *dev = property->dev;
> + int ret;
> +
> + switch (obj->type) {
> + case DRM_MODE_OBJECT_CONNECTOR: {
> + struct drm_connector *connector = obj_to_connector(obj);
> +
> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> + ret = connector->funcs->atomic_get_property(connector,
> + connector->state, property, val);
Shouldn't we use the get helpers introduced in previous patches here? For
legacy support we definitely want old core stuff like dpms to keep
working. Of course that means all the new atomic properties won't get
magically filtered out. But I think we need to hide them explicitly
anyway, e.g. with a new DRM_MODE_PROP_ATOMIC and checking for that in
legacy paths.
-Daniel
> + break;
> + }
> + case DRM_MODE_OBJECT_CRTC: {
> + struct drm_crtc *crtc = obj_to_crtc(obj);
> + WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
> + ret = crtc->funcs->atomic_get_property(crtc,
> + crtc->state, property, val);
> + break;
> + }
> + case DRM_MODE_OBJECT_PLANE: {
> + struct drm_plane *plane = obj_to_plane(obj);
> + WARN_ON(!drm_modeset_is_locked(&plane->mutex));
> + ret = plane->funcs->atomic_get_property(plane,
> + plane->state, property, val);
> + break;
> + }
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_get_property);
> +
> +/**
> * drm_atomic_helper_page_flip - execute a legacy page flip
> * @crtc: DRM crtc
> * @fb: DRM framebuffer
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 481bb25..57cd950 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3878,8 +3878,17 @@ EXPORT_SYMBOL(drm_object_property_set_value);
> int drm_object_property_get_value(struct drm_mode_object *obj,
> struct drm_property *property, uint64_t *val)
> {
> + struct drm_mode_config *config = &property->dev->mode_config;
> int i;
>
> + /* read-only properties bypass atomic mechanism and still store
> + * their value in obj->properties->values[].. mostly to avoid
> + * having to deal w/ EDID and similar props in atomic paths:
> + */
> + if (config->funcs->atomic_get_property &&
> + !(property->flags & DRM_MODE_PROP_IMMUTABLE))
> + return config->funcs->atomic_get_property(obj, property, val);
> +
> for (i = 0; i < obj->properties->count; i++) {
> if (obj->properties->properties[i] == property) {
> *val = obj->properties->values[i];
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index f956b41..5e72b82 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -74,6 +74,8 @@ int drm_atomic_helper_plane_set_property(struct drm_plane
> *plane,
> int drm_atomic_helper_connector_set_property(struct drm_connector *connector,
> struct drm_property *property,
> uint64_t val);
> +int drm_atomic_helper_get_property(struct drm_mode_object *obj,
> +struct drm_property *property, uint64_t *val);
> int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>
[PATCH 6/6] gpu: ipu-v3: Use videomode in struct ipu_di_signal_cfg
Hi Steve, I think this is a good idea. Two small comments below. Am Montag, den 15.12.2014, 16:29 -0800 schrieb slongerbeam at gmail.com: > From: Steve Longerbeam > > This patch changes struct ipu_di_signal_cfg to use struct videomode > to define video timings and flags. > > Signed-off-by: Steve Longerbeam > --- > drivers/gpu/drm/imx/ipuv3-crtc.c | 27 +++- > drivers/gpu/ipu-v3/ipu-di.c | 89 > -- > include/video/imx-ipu-v3.h | 19 ++-- > 3 files changed, 57 insertions(+), 78 deletions(-) > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c > b/drivers/gpu/drm/imx/ipuv3-crtc.c > index fb16026..0a50129 100644 > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > @@ -158,35 +158,20 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc, > > out_pixel_fmt = ipu_crtc->interface_pix_fmt; > > - if (mode->flags & DRM_MODE_FLAG_INTERLACE) > - sig_cfg.interlaced = 1; > - if (mode->flags & DRM_MODE_FLAG_PHSYNC) > - sig_cfg.Hsync_pol = 1; > - if (mode->flags & DRM_MODE_FLAG_PVSYNC) > - sig_cfg.Vsync_pol = 1; > - > sig_cfg.enable_pol = 1; > sig_cfg.clk_pol = 0; > - sig_cfg.width = mode->hdisplay; > - sig_cfg.height = mode->vdisplay; > sig_cfg.pixel_fmt = out_pixel_fmt; > - sig_cfg.h_start_width = mode->htotal - mode->hsync_end; > - sig_cfg.h_sync_width = mode->hsync_end - mode->hsync_start; > - sig_cfg.h_end_width = mode->hsync_start - mode->hdisplay; > - > - sig_cfg.v_start_width = mode->vtotal - mode->vsync_end; > - sig_cfg.v_sync_width = mode->vsync_end - mode->vsync_start; > - sig_cfg.v_end_width = mode->vsync_start - mode->vdisplay; > - sig_cfg.pixelclock = mode->clock * 1000; > sig_cfg.clkflags = ipu_crtc->di_clkflags; > - > sig_cfg.v_to_h_sync = 0; > - > sig_cfg.hsync_pin = ipu_crtc->di_hsync_pin; > sig_cfg.vsync_pin = ipu_crtc->di_vsync_pin; > > - ret = ipu_dc_init_sync(ipu_crtc->dc, ipu_crtc->di, sig_cfg.interlaced, > - out_pixel_fmt, mode->hdisplay); > + videomode_from_drm_display_mode(mode, &sig_cfg.mode); > + > + ret = ipu_dc_init_sync(ipu_crtc->dc, ipu_crtc->di, > +(mode->flags & DRM_MODE_FLAG_INTERLACE) ? > +true : false, The interlaced parameter to ipu_dc_init_sync is of type bool, so the ()?true:false is superfluous. [...] > @@ -433,10 +437,11 @@ static void ipu_di_config_clock(struct ipu_di *di, > unsigned long in_rate; > unsigned div; > > - clk_set_rate(clk, sig->pixelclock); > + clk_set_rate(clk, sig->mode.pixelclock); > > in_rate = clk_get_rate(clk); > - div = (in_rate + sig->pixelclock / 2) / sig->pixelclock; > + div = (in_rate + sig->mode.pixelclock / 2) / > + sig->mode.pixelclock; Let's use this opportunity to switch to DIV_ROUND_CLOSEST here ... > @@ -473,10 +479,11 @@ static void ipu_di_config_clock(struct ipu_di *di, > > clk = di->clk_di; > > - clk_set_rate(clk, sig->pixelclock); > + clk_set_rate(clk, sig->mode.pixelclock); > > in_rate = clk_get_rate(clk); > - div = (in_rate + sig->pixelclock / 2) / sig->pixelclock; > + div = (in_rate + sig->mode.pixelclock / 2) / > + sig->mode.pixelclock; ... and here. regards Philipp
[PATCH 5/6] imx-drm: encoder mode_set must use adjusted mode
Am Montag, den 15.12.2014, 16:29 -0800 schrieb slongerbeam at gmail.com:
> From: Steve Longerbeam
>
> The encoder ->mode_set() methods need to use the hw adjusted mode,
> not the original mode.
>
> Signed-off-by: Steve Longerbeam
> ---
> drivers/gpu/drm/imx/imx-hdmi.c |4 ++--
> drivers/gpu/drm/imx/imx-ldb.c |4 ++--
> drivers/gpu/drm/imx/imx-tve.c |4 ++--
> drivers/gpu/drm/imx/parallel-display.c |4 ++--
> 4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/imx-hdmi.c b/drivers/gpu/drm/imx/imx-hdmi.c
> index aaec6b2..32116cc 100644
> --- a/drivers/gpu/drm/imx/imx-hdmi.c
> +++ b/drivers/gpu/drm/imx/imx-hdmi.c
> @@ -1417,8 +1417,8 @@ static struct drm_encoder
> *imx_hdmi_connector_best_encoder(struct drm_connector
> }
>
> static void imx_hdmi_encoder_mode_set(struct drm_encoder *encoder,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + struct drm_display_mode *orig_mode,
> + struct drm_display_mode *mode)
> {
> struct imx_hdmi *hdmi = container_of(encoder, struct imx_hdmi, encoder);
>
> diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
> index 4662e00..a88c0e1 100644
> --- a/drivers/gpu/drm/imx/imx-ldb.c
> +++ b/drivers/gpu/drm/imx/imx-ldb.c
There's another line that should be changed
- struct drm_display_mode *mode = &encoder->crtc->mode;
+ struct drm_display_mode *mode = &encoder->crtc->hwmode;
in imx_ldb_encoder_prepare.
> @@ -246,8 +246,8 @@ static void imx_ldb_encoder_commit(struct drm_encoder
> *encoder)
> }
>
> static void imx_ldb_encoder_mode_set(struct drm_encoder *encoder,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + struct drm_display_mode *orig_mode,
> + struct drm_display_mode *mode)
> {
> struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
> struct imx_ldb *ldb = imx_ldb_ch->ldb;
regards
Philipp
[PATCH 13/13] RFC: drm: Atomic modeset ioctl
On Wed, Dec 17, 2014 at 4:31 AM, Michel Dänzer wrote:
> On 17.12.2014 16:20, Pekka Paalanen wrote:
>> On Wed, 17 Dec 2014 11:48:51 +0900
>> Michel Dänzer wrote:
>>
>>> On 17.12.2014 08:05, Rob Clark wrote:
The atomic modeset ioctl can be used to push any number of new values
for object properties. The driver can then check the full device
configuration as single unit, and try to apply the changes atomically.
The ioctl simply takes a list of object IDs and property IDs and their
values.
>>>
>>> [...]
>>>
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 86574b0..3459778 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -519,4 +519,25 @@ struct drm_mode_destroy_dumb {
uint32_t handle;
};
+/* page-flip flags are valid, plus: */
+#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
+#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
+
+#define DRM_MODE_ATOMIC_FLAGS (\
+ DRM_MODE_PAGE_FLIP_EVENT |\
+ DRM_MODE_PAGE_FLIP_ASYNC |\
+ DRM_MODE_ATOMIC_TEST_ONLY |\
+ DRM_MODE_ATOMIC_NONBLOCK)
+
+struct drm_mode_atomic {
+ __u32 flags;
+ __u32 count_objs;
+ __u64 objs_ptr;
+ __u64 count_props_ptr;
+ __u64 props_ptr;
+ __u64 prop_values_ptr;
+ __u64 blob_values_ptr; /* remove? */
+ __u64 user_data;
+};
+
#endif
>>>
>>> The new ioctl(s) should take an explicit parameter specifying when the
>>> changes should take effect. And since variable refresh rate displays are
>>> becoming mainstream, that parameter should probably be a timestamp
>>> rather than a frame counter.
>>
>> That sounds cool to me, but also a rabbit hole. While having worked on
>> the Wayland Presentation queueing extension, I'd like to ask the
>> following questions:
>>
>> - If you set the atomic kick to happen in the future, is there any way
>> to cancel it? I'd be ok with not being able to cancel initially, but
>> if one wants to add that later, we should already know how to
>> reference this atomic submission in the cancel request. What if user
>> space has a bug and schedules an update at one hour or three days from
>> now, how would we abort that?
>>
>> - Can you VT-switch or drop DRM master if you have a pending atomic
>> update?
>>
>> - Should one be able to set multiple pending atomic updates?
>>
>> - If I schedule an atomic update for one CTRC, can I schedule another
>> update for another CRTC before the first one completes? Or am I
>> forced to gather all updates over all outputs in the same atomic
>> submission even if I don't care about or want inter-output sync and
>> the outputs might even be running at different refresh rates?
>> (Actually, this seems to be a valid question even without any target
>> time parameter.)
>>
>> - If there can be multiple pending atomic updates on the same DRM
>> device, is there any way to guarantee that the
>> DRM_MODE_ATOMIC_TEST_ONLY results will be accurate also when the
>> atomic update actually kicks in? Another update may have changed the
>> configuration before this update kicks in, which means the overall
>> state isn't the same that was tested.
>>
>> - Does a pending atomic update prevent immediate (old style) KMS
>> changes?
>>
>> - Assuming hardware cannot do arbitrary time updates, how do you round
>> the given timestamp? Strictly "not before" given time? Round to
>> nearest possible time? The effect of required vs. unwanted sync to
>> vblank?
>>
>> - How would user space match the page flip event to the atomic
>> submission it did?
>>
>> I wonder if there is a way to postpone these hard(?) questions, so that
>> we could have atomic sooner and add scheduling later? I would imagine
>> solving everything above is quite some work.
>
> I agree. The main reason I brought it up is because I'd like to avoid
> getting into the same situation as with the current
> DRM_IOCTL_MODE_PAGE_FLIP ioctl, which doesn't explicitly communicate
> between userspace and kernel when the flip is supposed/expected to
> occur. We recently had to jump through some hoops in the radeon kernel
> driver to prevent flips from occurring sooner than expected by userspace.
>
>
> But I also think it would be a shame at this point to add new ioctls
> which aren't designed with variable refresh rate displays in mind.
>
So, I think this is something we can safely add later (and possibly
could be added to pageflip ioctl too in the same way.. but would need
to check the git history about whether pageflip ioctl handler was
always good enough at checking for invalid flags). My suggestion
would be to add a new field at end of ioctl along w/ a new arg->flags
bit to indicate that a timestamp is provided. New userspace on old
kernel would fail it due to invalid flag. Old userspace on new kernel
wouldn't set the bit.
Eit
[PATCH 06/13] drm: add atomic hook to read property plus helper
On Wed, Dec 17, 2014 at 03:02:41PM +0100, Daniel Vetter wrote:
> On Tue, Dec 16, 2014 at 06:05:34PM -0500, Rob Clark wrote:
> > Once a driver is using atomic helpers for modeset, the next step is to
> > switch over to atomic properties. To do this, plug in the helper
> > function to your modeconfig funcs and be sure to implement the plane/
> > crtc/connector atomic_{get,set}_property vfuncs for any of your mode-
> > objects which have driver custom properties.
> >
> > Signed-off-by: Rob Clark
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 46
> > +
> > drivers/gpu/drm/drm_crtc.c | 9
> > include/drm/drm_atomic_helper.h | 2 ++
> > include/drm/drm_crtc.h | 3 +++
> > 4 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index d42fdb1..1a1ab34 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1703,6 +1703,52 @@ backoff:
> > EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
> >
> > /**
> > + * drm_atomic_helper_get_property - helper to read atomic property
> > + * @obj: drm mode object whose property to read
> > + * @property: the property to read
> > + * @val: the read value, returned by reference
> > + *
> > + * RETURNS:
> > + * Zero on success, error code on failure
> > + */
> > +int drm_atomic_helper_get_property(struct drm_mode_object *obj,
> > + struct drm_property *property, uint64_t *val)
> > +{
> > + struct drm_device *dev = property->dev;
> > + int ret;
> > +
> > + switch (obj->type) {
> > + case DRM_MODE_OBJECT_CONNECTOR: {
> > + struct drm_connector *connector = obj_to_connector(obj);
> > +
> > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> > + ret = connector->funcs->atomic_get_property(connector,
> > + connector->state, property, val);
>
> Shouldn't we use the get helpers introduced in previous patches here? For
> legacy support we definitely want old core stuff like dpms to keep
> working. Of course that means all the new atomic properties won't get
> magically filtered out. But I think we need to hide them explicitly
> anyway, e.g. with a new DRM_MODE_PROP_ATOMIC and checking for that in
> legacy paths.
Also I'm not sure if there's a benefit to make this a vfunc+helper instead
of just core code called unconditionally. Meaning I can't think of any
case where drivers want to overwrite this, ever. They can do all the fancy
they want for their private properties in the per-object hooks. And for
core/shared stuff we better not allow them to play with fire.
Or do I miss an important case here?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[GIT PULL] drm/tegra: Fixes for v3.19-rc1
The following changes since commit 4e0cd68115620bc3236ff4e58e4c073948629b41: drm: sti: fix module compilation issue (2014-12-15 17:07:57 +1000) are available in the git repository at: git://people.freedesktop.org/~tagr/linux tags/drm/tegra/for-3.19-rc1-fixes for you to fetch changes up to 93396d0f9c027654eb09151d2e22fe78a39feedb: drm/tegra: dc: Select root window for event dispatch (2014-12-17 14:27:39 +0100) Thanks, Thierry drm/tegra: Fixes for v3.19-rc1 This is a set of fixes for two regressions and one bug in the IOMMU mapping code. It turns out that all of these issues turn up primarily on Tegra30 hardware. The IOMMU mapping bug only manifests on buffers that aren't multiples of the page size. I happened to be testing HDMI with 1080p while writing the code and framebuffers for that happen to fit exactly within 2025 pages of 4 KiB each. One of the regressions is caused by the IOMMU code allocating pages from shmem which can have associated cache lines. If the pages aren't flushed then these cache lines may be flushed later on and cause framebuffer corruption. I'm not sure why I didn't see this before. Perhaps the board that I was using had enough RAM so that the pages shmem would hand out had a better chance of being unused. Or maybe I didn't look too closely. The fix for this is to fake up an SG table so that it can be passed to the DMA API. Ideally this would use drm_clflush_*(), but implementing that for ARM causes DRM to fail to build as a module since some of the low-level cache maintenance functions aren't exported. Hopefully we can get a suitable API exported on ARM for the next release. The second regression is caused by a mismatch between the hardware pipe number and the CRTC's DRM index. These were used inconsistently, which could cause one code location to call drm_vblank_get() with a different pipe than the corresponding drm_vblank_put(), thereby causing the reference count to become unbalanced. Alexandre also reported a possible race condition related to this, which this series also fixes. Sean Paul (1): drm/tegra: dc: Select root window for event dispatch Thierry Reding (7): drm/irq: Add drm_crtc_send_vblank_event() drm/irq: Add drm_crtc_handle_vblank() drm/irq: Add drm_crtc_vblank_count() drm/tegra: dc: Consistently use the same pipe drm/tegra: dc: Fix a potential race on page-flip completion drm/tegra: gem: Flush buffer objects upon allocation drm/tegra: gem: Use the proper size for GEM objects drivers/gpu/drm/drm_irq.c | 60 + drivers/gpu/drm/tegra/dc.c | 48 +++- drivers/gpu/drm/tegra/drm.c | 16 +++- drivers/gpu/drm/tegra/gem.c | 52 ++- include/drm/drmP.h | 4 +++ 5 files changed, 150 insertions(+), 30 deletions(-)
[PATCH 07/13] drm: small property creation cleanup
On Tue, Dec 16, 2014 at 06:05:35PM -0500, Rob Clark wrote:
> Getting ready to add a lot more standard properties for atomic.
>
> Signed-off-by: Rob Clark
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc.c | 27 +--
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 57cd950..96965ec 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1330,33 +1330,40 @@ EXPORT_SYMBOL(drm_plane_force_disable);
>
> static int drm_mode_create_standard_connector_properties(struct drm_device
> *dev)
> {
> - struct drm_property *edid;
> - struct drm_property *dpms;
> - struct drm_property *dev_path;
> + struct drm_property *prop;
>
> /*
>* Standard properties (apply to all connectors)
>*/
> - edid = drm_property_create(dev, DRM_MODE_PROP_BLOB |
> + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
> DRM_MODE_PROP_IMMUTABLE,
> "EDID", 0);
> - dev->mode_config.edid_property = edid;
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.edid_property = prop;
>
> - dpms = drm_property_create_enum(dev, 0,
> + prop = drm_property_create_enum(dev, 0,
> "DPMS", drm_dpms_enum_list,
> ARRAY_SIZE(drm_dpms_enum_list));
> - dev->mode_config.dpms_property = dpms;
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.dpms_property = prop;
>
> - dev_path = drm_property_create(dev,
> + prop = drm_property_create(dev,
> DRM_MODE_PROP_BLOB |
> DRM_MODE_PROP_IMMUTABLE,
> "PATH", 0);
> - dev->mode_config.path_property = dev_path;
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.path_property = prop;
>
> - dev->mode_config.tile_property = drm_property_create(dev,
> + prop = drm_property_create(dev,
>DRM_MODE_PROP_BLOB
> |
>
> DRM_MODE_PROP_IMMUTABLE,
>"TILE", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.tile_property = prop;
>
> return 0;
> }
> --
> 2.1.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 06/13] drm: add atomic hook to read property plus helper
On Wed, Dec 17, 2014 at 9:02 AM, Daniel Vetter wrote:
> On Tue, Dec 16, 2014 at 06:05:34PM -0500, Rob Clark wrote:
>> Once a driver is using atomic helpers for modeset, the next step is to
>> switch over to atomic properties. To do this, plug in the helper
>> function to your modeconfig funcs and be sure to implement the plane/
>> crtc/connector atomic_{get,set}_property vfuncs for any of your mode-
>> objects which have driver custom properties.
>>
>> Signed-off-by: Rob Clark
>> ---
>> drivers/gpu/drm/drm_atomic_helper.c | 46
>> +
>> drivers/gpu/drm/drm_crtc.c | 9
>> include/drm/drm_atomic_helper.h | 2 ++
>> include/drm/drm_crtc.h | 3 +++
>> 4 files changed, 60 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> b/drivers/gpu/drm/drm_atomic_helper.c
>> index d42fdb1..1a1ab34 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1703,6 +1703,52 @@ backoff:
>> EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
>>
>> /**
>> + * drm_atomic_helper_get_property - helper to read atomic property
>> + * @obj: drm mode object whose property to read
>> + * @property: the property to read
>> + * @val: the read value, returned by reference
>> + *
>> + * RETURNS:
>> + * Zero on success, error code on failure
>> + */
>> +int drm_atomic_helper_get_property(struct drm_mode_object *obj,
>> + struct drm_property *property, uint64_t *val)
>> +{
>> + struct drm_device *dev = property->dev;
>> + int ret;
>> +
>> + switch (obj->type) {
>> + case DRM_MODE_OBJECT_CONNECTOR: {
>> + struct drm_connector *connector = obj_to_connector(obj);
>> +
>> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>> + ret = connector->funcs->atomic_get_property(connector,
>> + connector->state, property, val);
>
> Shouldn't we use the get helpers introduced in previous patches here? For
> legacy support we definitely want old core stuff like dpms to keep
> working. Of course that means all the new atomic properties won't get
> magically filtered out. But I think we need to hide them explicitly
> anyway, e.g. with a new DRM_MODE_PROP_ATOMIC and checking for that in
> legacy paths.
bleh, you are right.. and they did at one point use the helpers.. but
somehow in my squash/rebase/juggle act I lost that :-(
But I am still not a huge fan of hiding props for atomic drivers via
legacy getprop/setprop interfaces.. if nothing else it will prevent
them from showing up in modetest. And it seems kind of inconsistent
and unnecessary.
BR,
-R
> -Daniel
>
>> + break;
>> + }
>> + case DRM_MODE_OBJECT_CRTC: {
>> + struct drm_crtc *crtc = obj_to_crtc(obj);
>> + WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
>> + ret = crtc->funcs->atomic_get_property(crtc,
>> + crtc->state, property, val);
>> + break;
>> + }
>> + case DRM_MODE_OBJECT_PLANE: {
>> + struct drm_plane *plane = obj_to_plane(obj);
>> + WARN_ON(!drm_modeset_is_locked(&plane->mutex));
>> + ret = plane->funcs->atomic_get_property(plane,
>> + plane->state, property, val);
>> + break;
>> + }
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(drm_atomic_helper_get_property);
>> +
>> +/**
>> * drm_atomic_helper_page_flip - execute a legacy page flip
>> * @crtc: DRM crtc
>> * @fb: DRM framebuffer
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 481bb25..57cd950 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -3878,8 +3878,17 @@ EXPORT_SYMBOL(drm_object_property_set_value);
>> int drm_object_property_get_value(struct drm_mode_object *obj,
>> struct drm_property *property, uint64_t *val)
>> {
>> + struct drm_mode_config *config = &property->dev->mode_config;
>> int i;
>>
>> + /* read-only properties bypass atomic mechanism and still store
>> + * their value in obj->properties->values[].. mostly to avoid
>> + * having to deal w/ EDID and similar props in atomic paths:
>> + */
>> + if (config->funcs->atomic_get_property &&
>> + !(property->flags & DRM_MODE_PROP_IMMUTABLE))
>> + return config->funcs->atomic_get_property(obj, property, val);
>> +
>> for (i = 0; i < obj->properties->count; i++) {
>> if (obj->properties->properties[i] == property) {
>> *val = obj->properties->values[i];
>> diff --git a/include/drm/drm_atomic_helper.h
>> b/include/drm/drm_atomic_helper.h
>> index f956b41..5e72b82 100644
>> --- a/include/drm/drm_atomic_helper.h
>> +++ b/include/drm/drm_atomic_
[PATCH 08/13] drm: tweak getconnector locking
On Tue, Dec 16, 2014 at 06:05:36PM -0500, Rob Clark wrote:
> We need to hold connection_mutex as we read the properties. Easiest
> thing is just widen the scope where connection_mutex is held.
>
> Signed-off-by: Rob Clark
I'm not yet sure on the story we want to have for getprop locking (and
whether it's even relevant). But this gets us forward at least.
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 96965ec..62f5dc8 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2043,6 +2043,7 @@ int drm_mode_getconnector(struct drm_device *dev, void
> *data,
> DRM_DEBUG_KMS("[CONNECTOR:%d:?]\n", out_resp->connector_id);
>
> mutex_lock(&dev->mode_config.mutex);
> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>
> connector = drm_connector_find(dev, out_resp->connector_id);
> if (!connector) {
> @@ -2076,14 +2077,11 @@ int drm_mode_getconnector(struct drm_device *dev,
> void *data,
> out_resp->mm_height = connector->display_info.height_mm;
> out_resp->subpixel = connector->display_info.subpixel_order;
> out_resp->connection = connector->status;
> - drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> -
> encoder = drm_connector_get_encoder(connector);
> if (encoder)
> out_resp->encoder_id = encoder->base.id;
> else
> out_resp->encoder_id = 0;
> - drm_modeset_unlock(&dev->mode_config.connection_mutex);
>
> /*
>* This ioctl is called twice, once to determine how much space is
> @@ -2149,6 +2147,7 @@ int drm_mode_getconnector(struct drm_device *dev, void
> *data,
> out_resp->count_encoders = encoders_count;
>
> out:
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> mutex_unlock(&dev->mode_config.mutex);
>
> return ret;
> --
> 2.1.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 06/13] drm: add atomic hook to read property plus helper
On Wed, Dec 17, 2014 at 9:06 AM, Daniel Vetter wrote:
> On Wed, Dec 17, 2014 at 03:02:41PM +0100, Daniel Vetter wrote:
>> On Tue, Dec 16, 2014 at 06:05:34PM -0500, Rob Clark wrote:
>> > Once a driver is using atomic helpers for modeset, the next step is to
>> > switch over to atomic properties. To do this, plug in the helper
>> > function to your modeconfig funcs and be sure to implement the plane/
>> > crtc/connector atomic_{get,set}_property vfuncs for any of your mode-
>> > objects which have driver custom properties.
>> >
>> > Signed-off-by: Rob Clark
>> > ---
>> > drivers/gpu/drm/drm_atomic_helper.c | 46
>> > +
>> > drivers/gpu/drm/drm_crtc.c | 9
>> > include/drm/drm_atomic_helper.h | 2 ++
>> > include/drm/drm_crtc.h | 3 +++
>> > 4 files changed, 60 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> > b/drivers/gpu/drm/drm_atomic_helper.c
>> > index d42fdb1..1a1ab34 100644
>> > --- a/drivers/gpu/drm/drm_atomic_helper.c
>> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> > @@ -1703,6 +1703,52 @@ backoff:
>> > EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
>> >
>> > /**
>> > + * drm_atomic_helper_get_property - helper to read atomic property
>> > + * @obj: drm mode object whose property to read
>> > + * @property: the property to read
>> > + * @val: the read value, returned by reference
>> > + *
>> > + * RETURNS:
>> > + * Zero on success, error code on failure
>> > + */
>> > +int drm_atomic_helper_get_property(struct drm_mode_object *obj,
>> > + struct drm_property *property, uint64_t *val)
>> > +{
>> > + struct drm_device *dev = property->dev;
>> > + int ret;
>> > +
>> > + switch (obj->type) {
>> > + case DRM_MODE_OBJECT_CONNECTOR: {
>> > + struct drm_connector *connector = obj_to_connector(obj);
>> > +
>> > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>> > + ret = connector->funcs->atomic_get_property(connector,
>> > + connector->state, property, val);
>>
>> Shouldn't we use the get helpers introduced in previous patches here? For
>> legacy support we definitely want old core stuff like dpms to keep
>> working. Of course that means all the new atomic properties won't get
>> magically filtered out. But I think we need to hide them explicitly
>> anyway, e.g. with a new DRM_MODE_PROP_ATOMIC and checking for that in
>> legacy paths.
>
> Also I'm not sure if there's a benefit to make this a vfunc+helper instead
> of just core code called unconditionally. Meaning I can't think of any
> case where drivers want to overwrite this, ever. They can do all the fancy
> they want for their private properties in the per-object hooks. And for
> core/shared stuff we better not allow them to play with fire.
>
> Or do I miss an important case here?
drivers can have their own custom properties too :-)
umm, or are you talking about the vfunc in config->funcs? (In which
case, it was at least a convenient way to for core to know whether to
take legacy path or atomic path)
BR,
-R
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 0/6] imx-drm: ipuv3-crtc: Implement mode_fixup
Hi Steve, Please put me on Cc: for IPU and imx-drm related patches. Am Montag, den 15.12.2014, 16:29 -0800 schrieb slongerbeam at gmail.com: > From: Steve Longerbeam > > This patchset implements ->mode_fixup() in the imx ipuv3-crtc driver, > using a new support function ipu_di_adjust_videomode(). This new > function needs to be subsystem independent, so it accepts a video > mode as a 'struct videomode'. Hence ipu-crtc ->mode_fixup() needs > another support function to convert a drm_display_mode to a videomode > before passing the mode to ipu_di_adjust_videomode() for fixup. > > Also some related code cleanup: 'struct ipu_di_signal_cfg' should > use 'struct videomode' for mode timings. I have tested the video mode adjustment on an LVDS panel (that doesn't care about the actual front/back porches and allows a wide range of timings) and verified it fixes the problem of the display randomly turning off with some timings. regards Philipp
[PATCH 09/13] drm/atomic: atomic_check functions
On Tue, Dec 16, 2014 at 06:05:37PM -0500, Rob Clark wrote:
> Add functions to check core plane/crtc state.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/drm_atomic.c| 87
> +
> drivers/gpu/drm/drm_atomic_helper.c | 47 ++--
> include/drm/drm_atomic.h| 4 ++
> 3 files changed, 124 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 4099b44..afb830d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -261,6 +261,27 @@ int drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> EXPORT_SYMBOL(drm_atomic_crtc_get_property);
>
> /**
> + * drm_atomic_crtc_check - check crtc state
> + * @crtc: crtc to check
> + * @state: crtc state to check
> + *
> + * Provides core sanity checks for crtc state.
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_crtc_check(struct drm_crtc *crtc,
> + struct drm_crtc_state *state)
> +{
> + /* TODO anything to check? I think if drivers want to enforce that
> + * primary layer covers entire screen, they should do that in their
> + * own crtc->atomic_check() vfunc..
> + */
We could do things like mode sanity checking, like Ville's latest series
does. But for now there's nothing, so imo drop the comment. Maybe instead
/* TODO: Add generic modeset state checks once we support those. */
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_crtc_check);
> +
> +/**
> * drm_atomic_get_plane_state - get plane state
> * @state: global atomic state object
> * @plane: plane to get state object for
> @@ -360,6 +381,72 @@ int drm_atomic_plane_get_property(struct drm_plane
> *plane,
> EXPORT_SYMBOL(drm_atomic_plane_get_property);
>
> /**
> + * drm_atomic_plane_check - check plane state
> + * @plane: plane to check
> + * @state: plane state to check
> + *
> + * Provides core sanity checks for plane state.
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_plane_check(struct drm_plane *plane,
> + struct drm_plane_state *state)
> +{
> + unsigned int fb_width, fb_height;
> + unsigned int i;
The overflow checks on crtc_w/x/h/y from drm_mode_setplane seem to be
missing here. I think we should add them to the atomic paths, too.
> +
> + /* either *both* CRTC and FB must be set, or neither */
> + if (WARN_ON(state->crtc && !state->fb)) {
> + DRM_DEBUG_KMS("CRTC set but no FB\n");
> + return -EINVAL;
> + } else if (WARN_ON(state->fb && !state->crtc)) {
> + DRM_DEBUG_KMS("FB set but no CRTC\n");
> + return -EINVAL;
> + }
> +
> + /* if disabled, we don't care about the rest of the state: */
> + if (!state->crtc)
> + return 0;
> +
> + /* Check whether this plane is usable on this CRTC */
> + if (!(plane->possible_crtcs & drm_crtc_mask(state->crtc))) {
> + DRM_DEBUG_KMS("Invalid crtc for plane\n");
> + return -EINVAL;
> + }
> +
> + /* Check whether this plane supports the fb pixel format. */
> + for (i = 0; i < plane->format_count; i++)
> + if (state->fb->pixel_format == plane->format_types[i])
> + break;
> + if (i == plane->format_count) {
> + DRM_DEBUG_KMS("Invalid pixel format %s\n",
> + drm_get_format_name(state->fb->pixel_format));
> + return -EINVAL;
> + }
> +
> + fb_width = state->fb->width << 16;
> + fb_height = state->fb->height << 16;
> +
> + /* Make sure source coordinates are inside the fb. */
> + if (state->src_w > fb_width ||
> + state->src_x > fb_width - state->src_w ||
> + state->src_h > fb_height ||
> + state->src_y > fb_height - state->src_h) {
> + DRM_DEBUG_KMS("Invalid source coordinates "
> + "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
> + state->src_w >> 16, ((state->src_w & 0x) *
> 15625) >> 10,
> + state->src_h >> 16, ((state->src_h & 0x) *
> 15625) >> 10,
> + state->src_x >> 16, ((state->src_x & 0x) *
> 15625) >> 10,
> + state->src_y >> 16, ((state->src_y & 0x) *
> 15625) >> 10);
> + return -ENOSPC;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_plane_check);
> +
> +/**
> * drm_atomic_get_connector_state - get connector state
> * @state: global atomic state object
> * @connector: connector to get state object for
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 1a1ab34..55b6981 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -407,6 +407,37 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> return mo
[PATCH 06/13] drm: add atomic hook to read property plus helper
On Wed, Dec 17, 2014 at 09:15:12AM -0500, Rob Clark wrote:
> On Wed, Dec 17, 2014 at 9:06 AM, Daniel Vetter wrote:
> > On Wed, Dec 17, 2014 at 03:02:41PM +0100, Daniel Vetter wrote:
> >> On Tue, Dec 16, 2014 at 06:05:34PM -0500, Rob Clark wrote:
> >> > Once a driver is using atomic helpers for modeset, the next step is to
> >> > switch over to atomic properties. To do this, plug in the helper
> >> > function to your modeconfig funcs and be sure to implement the plane/
> >> > crtc/connector atomic_{get,set}_property vfuncs for any of your mode-
> >> > objects which have driver custom properties.
> >> >
> >> > Signed-off-by: Rob Clark
> >> > ---
> >> > drivers/gpu/drm/drm_atomic_helper.c | 46
> >> > +
> >> > drivers/gpu/drm/drm_crtc.c | 9
> >> > include/drm/drm_atomic_helper.h | 2 ++
> >> > include/drm/drm_crtc.h | 3 +++
> >> > 4 files changed, 60 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> >> > b/drivers/gpu/drm/drm_atomic_helper.c
> >> > index d42fdb1..1a1ab34 100644
> >> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> > @@ -1703,6 +1703,52 @@ backoff:
> >> > EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
> >> >
> >> > /**
> >> > + * drm_atomic_helper_get_property - helper to read atomic property
> >> > + * @obj: drm mode object whose property to read
> >> > + * @property: the property to read
> >> > + * @val: the read value, returned by reference
> >> > + *
> >> > + * RETURNS:
> >> > + * Zero on success, error code on failure
> >> > + */
> >> > +int drm_atomic_helper_get_property(struct drm_mode_object *obj,
> >> > + struct drm_property *property, uint64_t *val)
> >> > +{
> >> > + struct drm_device *dev = property->dev;
> >> > + int ret;
> >> > +
> >> > + switch (obj->type) {
> >> > + case DRM_MODE_OBJECT_CONNECTOR: {
> >> > + struct drm_connector *connector = obj_to_connector(obj);
> >> > +
> >> > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> >> > + ret = connector->funcs->atomic_get_property(connector,
> >> > + connector->state, property, val);
> >>
> >> Shouldn't we use the get helpers introduced in previous patches here? For
> >> legacy support we definitely want old core stuff like dpms to keep
> >> working. Of course that means all the new atomic properties won't get
> >> magically filtered out. But I think we need to hide them explicitly
> >> anyway, e.g. with a new DRM_MODE_PROP_ATOMIC and checking for that in
> >> legacy paths.
> >
> > Also I'm not sure if there's a benefit to make this a vfunc+helper instead
> > of just core code called unconditionally. Meaning I can't think of any
> > case where drivers want to overwrite this, ever. They can do all the fancy
> > they want for their private properties in the per-object hooks. And for
> > core/shared stuff we better not allow them to play with fire.
> >
> > Or do I miss an important case here?
>
> drivers can have their own custom properties too :-)
>
> umm, or are you talking about the vfunc in config->funcs? (In which
> case, it was at least a convenient way to for core to know whether to
> take legacy path or atomic path)
Yeah I only meant the config->func hook. Is there no other way to handle
this in the core? It looks funny if we use a func pointer essentially as a
boolean and don't allow drivers to set anything else than the defaults.
Essentially makes the helpers mandatory. Maybe just have
bool config->use_atomic_properties instead?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 06/13] drm: add atomic hook to read property plus helper
On Wed, Dec 17, 2014 at 09:12:31AM -0500, Rob Clark wrote: > But I am still not a huge fan of hiding props for atomic drivers via > legacy getprop/setprop interfaces.. if nothing else it will prevent > them from showing up in modetest. And it seems kind of inconsistent > and unnecessary. By default at least the intel X driver reads/writes _all_ props it finds. Maybe just on connectors, but even there we have the crtc prop. Which means with the current legacy set_prop helpers we'll end up doing tons of modesets, and that will piss of users. So I think we don't have a choice but hide them. In i915 all the legacy props have careful checks to short-circuit such noop changes out, exactly for this reasons. Adding all these short-circuit checks would pretty badly complicate helpers and hence is something I've explicitly left out (for now at least). And the PROP_ATOMIC flag is a useful hint to userspace to maybe not do stupid things with that property just for fun. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/tegra: Select root window for event dispatch
On Wed, Nov 19, 2014 at 01:04:49PM -0500, Sean Paul wrote: > In finish pageflip, the driver was not selecting the root > window when dispatching events. This exposed a race where > a plane update would change the window selection and cause > tegra_dc_finish_page_flip to check the wrong base address. > > This patch also protects access to the window selection register > as well as the registers affected by it. > > Signed-off-by: Sean Paul > --- > drivers/gpu/drm/tegra/dc.c | 20 +--- > 1 file changed, 17 insertions(+), 3 deletions(-) I had hoped to be done with the conversion to atomic modesetting by now at which point this patch would become significantly different. But I've applied it to the fixes branch now and rebased the atomic modesetting patches on top since it'll take a while longer for those to be ready. Note also that we should be able to get rid of this spinlock by using the per-window registers directly. As I understand it the window select register is the legacy way of programming windows. There is also a flattened address space available for them. According to the TRM these are located at: 0x0a00: window A 0x0c00: window B 0x0e00: window C Supporting that would require a larger rewrite, but I think it'd be a good idea to do that at some point to avoid the spinlock. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141217/c87d159f/attachment.sig>
[PATCH 10/13] drm/atomic: atomic plane properties
On Tue, Dec 16, 2014 at 06:05:38PM -0500, Rob Clark wrote:
> Expose the core plane state as properties, so they can be updated via
> atomic ioctl.
>
> Signed-off-by: Rob Clark
Just comments about the lack of PROP_ATOMIC and one suggestion for a
comment. With that addressed this is Reviewed-by: Daniel Vetter
> ---
> Documentation/DocBook/drm.tmpl | 74 --
> drivers/gpu/drm/drm_atomic.c | 69 ---
> drivers/gpu/drm/drm_crtc.c | 82
> +++---
> include/drm/drm_crtc.h | 10 ++
> 4 files changed, 224 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 4b592ff..282fa6b 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2564,7 +2564,7 @@ void intel_crt_init(struct drm_device *dev)
> Description/Restrictions
>
>
> - DRM
> + DRM
> Generic
> âEDIDâ
> BLOB | IMMUTABLE
> @@ -2594,7 +2594,7 @@ void intel_crt_init(struct drm_device *dev)
> Contains tiling information for a connector.
>
>
> - Plane
> + Plane
> âtypeâ
> ENUM | IMMUTABLE
> { "Overlay", "Primary", "Cursor" }
> @@ -2602,6 +2602,76 @@ void intel_crt_init(struct drm_device *dev)
> Plane type
>
>
> + âSRC_Xâ
> + RANGE
> + Min=0, Max=UINT_MAX
> + Plane
> + Scanout source x coordinate in 16.16 fixed point
> (atomic)
> +
> +
> + âSRC_Yâ
> + RANGE
> + Min=0, Max=UINT_MAX
> + Plane
> + Scanout source y coordinate in 16.16 fixed point
> (atomic)
> +
> +
> + âSRC_Wâ
> + RANGE
> + Min=0, Max=UINT_MAX
> + Plane
> + Scanout source width in 16.16 fixed point
> (atomic)
> +
> +
> + âSRC_Hâ
> + RANGE
> + Min=0, Max=UINT_MAX
> + Plane
> + Scanout source height in 16.16 fixed point
> (atomic)
> +
> +
> + âCRTC_Xâ
> + SIGNED_RANGE
> + Min=INT_MIN, Max=INT_MAX
> + Plane
> + Scanout CRTC (destination) x coordinate (atomic)
> +
> +
> + âCRTC_Yâ
> + SIGNED_RANGE
> + Min=INT_MIN, Max=INT_MAX
> + Plane
> + Scanout CRTC (destination) y coordinate (atomic)
> +
> +
> + âCRTC_Wâ
> + RANGE
> + Min=0, Max=UINT_MAX
> + Plane
> + Scanout CRTC (destination) width (atomic)
> +
> +
> + âCRTC_Hâ
> + RANGE
> + Min=0, Max=UINT_MAX
> + Plane
> + Scanout CRTC (destination) height (atomic)
> +
> +
> + âFB_IDâ
> + OBJECT
> + DRM_MODE_OBJECT_FB
> + Plane
> + Scanout framebuffer (atomic)
> +
> +
> + âCRTC_IDâ
> + OBJECT
> + DRM_MODE_OBJECT_CRTC
> + Plane
> + CRTC that plane is attached to (atomic)
> +
> +
> DVI-I
> âsubconnectorâ
> ENUM
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index afb830d..c09a05a 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -352,9 +352,41 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> struct drm_plane_state *state, struct drm_property *property,
> uint64_t val)
> {
> - if (plane->funcs->atomic_set_property)
> - return plane->funcs->atomic_set_property(plane, state,
> property, val);
> - return -EINVAL;
> + struct drm_device *dev = plane->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (property == config->prop_fb_id) {
> + struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, val);
> + drm_atomic_set_fb_for_plane(state, fb);
> + if (fb)
> + drm_framebuffer_unreference(fb);
> + } else if (property == config->prop_crtc_id) {
> + struct drm_crtc *crtc = drm_crtc_find(dev, val);
> + return drm_atomic_set_crtc_for_plane(state->state, plane, crtc);
> + } else if (property == config->prop_crtc_x) {
> + state->crtc_x = U642I64(val);
> + } else if (property == config->prop_crtc_y) {
> + state->crtc_y = U642I64(val);
> + } else if (property == config->prop_crtc_w) {
> + state->crtc_w = val;
> + } else if (property == config->prop_crtc_h) {
> + state->crtc_h = val;
> + } else if (property == config->prop_src_x) {
> + state->src_x = val;
> + } else if (property == config->prop_src_y) {
> + state->src_y = val;
> + } else if (property == config->prop_src_w) {
> + state->src_w = val;
> + } else if (property == config->prop_src_h) {
> + state->src_h = val;
We need to check for PROP_ATOMIC somewhere. Well more precisely
if ((prop->flags & PROP_ATOMIC) && !file_priv->atomic_kms)
[PATCH 11/13] drm/atomic: atomic connector properties
On Tue, Dec 16, 2014 at 06:05:39PM -0500, Rob Clark wrote:
> Expose the core connector state as properties so it can be updated via
> atomic ioctl.
>
> Signed-off-by: Rob Clark
Oh, didn't realize that you're sharing config->prop_crtc_id with plane and
connector. Please add a comment about that to drm_crtc.h.
Plus same comment about setting DRM_MODE_PROB_ATOMIC. With these two this
is Reviewed-by: Daniel Vetter
I wasn't to clear in my reply to the previous patch probably, but I think
the atomic filtering should be in an earlier patch. Best in the missing
one to add the modparam and file_priv->atomic_kms.
-Daniel
> ---
> Documentation/DocBook/drm.tmpl | 11 +--
> drivers/gpu/drm/drm_atomic.c | 9 +++--
> drivers/gpu/drm/drm_crtc.c | 13 +
> 3 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 282fa6b..15cb9b9 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2564,8 +2564,8 @@ void intel_crt_init(struct drm_device *dev)
> Description/Restrictions
>
>
> - DRM
> - Generic
> + DRM
> + Connector
> âEDIDâ
> BLOB | IMMUTABLE
> 0
> @@ -2594,6 +2594,13 @@ void intel_crt_init(struct drm_device *dev)
> Contains tiling information for a connector.
>
>
> + âCRTC_IDâ
> + OBJECT
> + DRM_MODE_OBJECT_CRTC
> + Connector
> + CRTC that connector is attached to (atomic)
> +
> +
> Plane
> âtypeâ
> ENUM | IMMUTABLE
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index c09a05a..71b48a0 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -595,7 +595,10 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> struct drm_device *dev = connector->dev;
> struct drm_mode_config *config = &dev->mode_config;
>
> - if (property == config->dpms_property) {
> + if (property == config->prop_crtc_id) {
> + struct drm_crtc *crtc = drm_crtc_find(dev, val);
> + return drm_atomic_set_crtc_for_connector(state, crtc);
> + } else if (property == config->dpms_property) {
> /* setting DPMS property requires special handling, which
>* is done in legacy setprop path for us. Disallow (for
>* now?) atomic writes to DPMS property:
> @@ -629,7 +632,9 @@ int drm_atomic_connector_get_property(struct
> drm_connector *connector,
> struct drm_device *dev = connector->dev;
> struct drm_mode_config *config = &dev->mode_config;
>
> - if (property == config->dpms_property) {
> + if (property == config->prop_crtc_id) {
> + *val = (state->crtc) ? state->crtc->base.id : 0;
> + } else if (property == config->dpms_property) {
> *val = connector->dpms;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index fd6f91d..39c3e06 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -844,6 +844,7 @@ int drm_connector_init(struct drm_device *dev,
> const struct drm_connector_funcs *funcs,
> int connector_type)
> {
> + struct drm_mode_config *config = &dev->mode_config;
> int ret;
> struct ida *connector_ida =
> &drm_connector_enum_list[connector_type].ida;
> @@ -882,16 +883,20 @@ int drm_connector_init(struct drm_device *dev,
>
> /* We should add connectors at the end to avoid upsetting the connector
>* index too much. */
> - list_add_tail(&connector->head, &dev->mode_config.connector_list);
> - dev->mode_config.num_connector++;
> + list_add_tail(&connector->head, &config->connector_list);
> + config->num_connector++;
>
> if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL)
> drm_object_attach_property(&connector->base,
> - dev->mode_config.edid_property,
> + config->edid_property,
> 0);
>
> drm_object_attach_property(&connector->base,
> - dev->mode_config.dpms_property, 0);
> + config->dpms_property, 0);
> +
> + if (has_atomic_properties(dev)) {
> + drm_object_attach_property(&connector->base,
> config->prop_crtc_id, 0);
> + }
>
> connector->debugfs_entry = NULL;
>
> --
> 2.1.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 13/13] RFC: drm: Atomic modeset ioctl
On Tue, Dec 16, 2014 at 06:05:41PM -0500, Rob Clark wrote:
> The atomic modeset ioctl can be used to push any number of new values
> for object properties. The driver can then check the full device
> configuration as single unit, and try to apply the changes atomically.
>
> The ioctl simply takes a list of object IDs and property IDs and their
> values.
>
> Originally based on a patch from Ville Syrjälä, although it has mutated
> (mutilated?) enough since then that you probably shouldn't blame it on
> him ;-)
>
> TODO:
> * hide behind moduleparam initially
Yeah we need this, and I think we really need this (plus
file_priv->atomic_kms to allow new userspace to enable atomic, similar to
file_priv->universal_planes) even for patches 6, 10&11.
> * either bring back blob property support, or add ioctls for create/
>destroy blob properties so they can be passed in by id in this
>ioctl. I'm leaning towards the latter approach as (a) it is more
>in line with how blob properties are read, and (b) the atomic
>ioctl function is big enough already.
The blob approach is a bit more work, since we need to copypaste all the
logic we have for per-filpriv framebuffers: cleanup lists & refcounting.
Not sure whether we should just go with lots more properties, they're
cheap ;-) But I'm ok with this blob approach, too. Adding blobs to the
atomic ioctl itself feels a bit wrong.
One thing I have to consider with my intel hat on is the bridge we need to
adf. Everyting plain 64bit props would be a bit easier to marshal through
adf into atomic interfaces. But only slightly, we can just add an in-line
set of blobs at the end and write them directly, too. So either approach
is fine really.
Below a few comments all over, but looks good overall.
Cheers, Daniel
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/drm_atomic.c | 308
> +++
> drivers/gpu/drm/drm_crtc.c | 4 +-
> drivers/gpu/drm/drm_ioctl.c | 1 +
> include/drm/drm_crtc.h | 6 +
> include/uapi/drm/drm.h | 1 +
> include/uapi/drm/drm_mode.h | 21 +++
> 6 files changed, 339 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 71b48a0..e7a45ec 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -960,3 +960,311 @@ int drm_atomic_async_commit(struct drm_atomic_state
> *state)
> return config->funcs->atomic_commit(state->dev, state, true);
> }
> EXPORT_SYMBOL(drm_atomic_async_commit);
> +
> +/*
> + * The big monstor ioctl
> + */
> +
> +static struct drm_pending_vblank_event *create_vblank_event(
> + struct drm_device *dev, struct drm_file *file_priv, uint64_t
> user_data)
> +{
> + struct drm_pending_vblank_event *e = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->event_lock, flags);
> + if (file_priv->event_space < sizeof e->event) {
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + goto out;
> + }
> + file_priv->event_space -= sizeof e->event;
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> + e = kzalloc(sizeof *e, GFP_KERNEL);
> + if (e == NULL) {
> + spin_lock_irqsave(&dev->event_lock, flags);
> + file_priv->event_space += sizeof e->event;
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + goto out;
> + }
> +
> + e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
> + e->event.base.length = sizeof e->event;
> + e->event.user_data = user_data;
> + e->base.event = &e->event.base;
> + e->base.file_priv = file_priv;
> + e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
> +
> +out:
> + return e;
> +}
> +
> +static void destroy_vblank_event(struct drm_device *dev,
> + struct drm_file *file_priv, struct drm_pending_vblank_event *e)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->event_lock, flags);
> + file_priv->event_space += sizeof e->event;
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + kfree(e);
> +}
I guess these two are the skeleton functions for the refactoring you've
started? Just an aside really.
> +int drm_mode_atomic_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv)
> +{
> + struct drm_mode_atomic *arg = data;
> + uint32_t __user *objs_ptr = (uint32_t __user *)(unsigned
> long)(arg->objs_ptr);
> + uint32_t __user *count_props_ptr = (uint32_t __user *)(unsigned
> long)(arg->count_props_ptr);
> + uint32_t __user *props_ptr = (uint32_t __user *)(unsigned
> long)(arg->props_ptr);
> + uint64_t __user *prop_values_ptr = (uint64_t __user *)(unsigned
> long)(arg->prop_values_ptr);
> + unsigned int copied_objs, copied_props;
> + struct drm_atomic_state *state;
> + struct drm_modeset_acquire_ctx ctx;
> + struct drm_plane *plane;
> + un
[Bug 89961] New: System Shutdown on High Load (Radeon)
https://bugzilla.kernel.org/show_bug.cgi?id=89961 Bug ID: 89961 Summary: System Shutdown on High Load (Radeon) Product: Drivers Version: 2.5 Kernel Version: >3.13 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri at kernel-bugs.osdl.org Reporter: swoorupj at gmail.com Regression: No Whenever playing any applications that requires high graphics performance especially games like Left For Dead 2, for a certain period of time, my system suddenly shutdowns down. It feels like power has been cut off all of a sudden. I am not sure as to what the cause of the problem exactly is. But it occurs with radeon module. It does not happen with catalyst, but I can't use catalyst it has problem with rendering. One thing I notice is that temperature is very high during this stage. Could it be due to a bug in which the frequency clocks are transitioned in the GPU? Or simply due to overheating? Note that this does not occur with windows driver. I have a AMD A6-4400M APU with Radeon HD 7520G] -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] drm/doc: Remove duplicate "by"
From: Thierry Reding Signed-off-by: Thierry Reding --- Documentation/DocBook/drm.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 8b6fe229dc9a..8a0b2a84f4bf 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -1377,7 +1377,7 @@ int max_width, max_height; DRM_PLANE_TYPE_PRIMARY represents a "main" plane for a CRTC. Primary -planes are the planes operated upon by by CRTC modesetting and flipping +planes are the planes operated upon by CRTC modesetting and flipping operations described in . -- 2.1.3
[PATCH] drm/fb-helper: Propagate errors from initial config failure
From: Thierry Reding
Make drm_fb_helper_initial_config() return an int rather than a bool so
that the error can be properly propagated. While at it, update drivers
to propagate errors further rather than just ignore them.
Cc: David Airlie
Cc: Daniel Vetter
Cc: Patrik Jakobsson
Cc: Rob Clark
Cc: Tomi Valkeinen
Cc: Alex Deucher
Cc: Christian König
Cc: Ben Skeggs
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/ast/ast_fb.c| 21 +++--
drivers/gpu/drm/bochs/bochs_fbdev.c | 14 --
drivers/gpu/drm/cirrus/cirrus_fbdev.c | 22 --
drivers/gpu/drm/drm_fb_helper.c | 2 +-
drivers/gpu/drm/gma500/framebuffer.c| 22 ++
drivers/gpu/drm/mgag200/mgag200_fb.c| 12 ++--
drivers/gpu/drm/msm/msm_fbdev.c | 10 --
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 21 +++--
drivers/gpu/drm/omapdrm/omap_fbdev.c| 10 --
drivers/gpu/drm/qxl/qxl_fb.c| 22 --
drivers/gpu/drm/radeon/radeon_fb.c | 21 +++--
drivers/gpu/drm/udl/udl_fb.c| 22 +++---
include/drm/drm_fb_helper.h | 2 +-
13 files changed, 150 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 5c60ae524c45..ff68eefae273 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -335,18 +335,27 @@ int ast_fbdev_init(struct drm_device *dev)
ret = drm_fb_helper_init(dev, &afbdev->helper,
1, 1);
- if (ret) {
- kfree(afbdev);
- return ret;
- }
+ if (ret)
+ goto free;
- drm_fb_helper_single_add_all_connectors(&afbdev->helper);
+ ret = drm_fb_helper_single_add_all_connectors(&afbdev->helper);
+ if (ret)
+ goto fini;
/* disable all the possible outputs/crtcs before entering KMS mode */
drm_helper_disable_unused_functions(dev);
- drm_fb_helper_initial_config(&afbdev->helper, 32);
+ ret = drm_fb_helper_initial_config(&afbdev->helper, 32);
+ if (ret)
+ goto fini;
+
return 0;
+
+fini:
+ drm_fb_helper_fini(&afbdev->helper);
+free:
+ kfree(afbdev);
+ return ret;
}
void ast_fbdev_fini(struct drm_device *dev)
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index 61dbf09dff5d..976d9798dc99 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -207,12 +207,22 @@ int bochs_fbdev_init(struct bochs_device *bochs)
if (ret)
return ret;
- drm_fb_helper_single_add_all_connectors(&bochs->fb.helper);
+ ret = drm_fb_helper_single_add_all_connectors(&bochs->fb.helper);
+ if (ret)
+ goto fini;
+
drm_helper_disable_unused_functions(bochs->dev);
- drm_fb_helper_initial_config(&bochs->fb.helper, 32);
+
+ ret = drm_fb_helper_initial_config(&bochs->fb.helper, 32);
+ if (ret)
+ goto fini;
bochs->fb.initialized = true;
return 0;
+
+fini:
+ drm_fb_helper_fini(&bochs->fb.helper);
+ return ret;
}
void bochs_fbdev_fini(struct bochs_device *bochs)
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 502a89eb54b5..0682210b068b 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -317,17 +317,27 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
ret = drm_fb_helper_init(cdev->dev, &gfbdev->helper,
cdev->num_crtc, CIRRUSFB_CONN_LIMIT);
- if (ret) {
- kfree(gfbdev);
- return ret;
- }
- drm_fb_helper_single_add_all_connectors(&gfbdev->helper);
+ if (ret)
+ goto free;
+
+ ret = drm_fb_helper_single_add_all_connectors(&gfbdev->helper);
+ if (ret)
+ goto fini;
/* disable all the possible outputs/crtcs before entering KMS mode */
drm_helper_disable_unused_functions(cdev->dev);
- drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel);
+
+ ret = drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel);
+ if (ret)
+ goto fini;
return 0;
+
+free:
+ kfree(gfbdev);
+fini:
+ drm_fb_helper_fini(&gfbdev->helper);
+ return ret;
}
void cirrus_fbdev_fini(struct cirrus_device *cdev)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 52ce26d6b4fb..876f1ef0acd1 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1688,7 +1688,7 @@ out:
* RETURNS:
* Zero if everything went ok, nonzero otherwise.
*/
-bool drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
+int drm_fb_helper_initial_config(struct drm_fb_h
[PATCH] drm/doc: Remove duplicate "by"
On Wed, Dec 17, 2014 at 04:13:17PM +0100, Thierry Reding wrote: > From: Thierry Reding > > Signed-off-by: Thierry Reding Applied to my drm misc tree. -Daniel > --- > Documentation/DocBook/drm.tmpl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 8b6fe229dc9a..8a0b2a84f4bf 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -1377,7 +1377,7 @@ int max_width, max_height; > > > DRM_PLANE_TYPE_PRIMARY represents a "main" plane for a CRTC. Primary > -planes are the planes operated upon by by CRTC modesetting and > flipping > +planes are the planes operated upon by CRTC modesetting and flipping > operations described in . > > > -- > 2.1.3 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/doc: Remove duplicate "by"
From: Thierry Reding Signed-off-by: Thierry Reding --- Documentation/DocBook/drm.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 8b6fe229dc9a..8a0b2a84f4bf 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -1377,7 +1377,7 @@ int max_width, max_height; DRM_PLANE_TYPE_PRIMARY represents a "main" plane for a CRTC. Primary -planes are the planes operated upon by by CRTC modesetting and flipping +planes are the planes operated upon by CRTC modesetting and flipping operations described in . -- 2.1.3
[PATCH 1/4] drm: Remove stale comment
From: Thierry Reding
The struct drm_connector_funcs kerneldoc refers to a part of struct
drm_crtc_funcs that no longer exists.
Signed-off-by: Thierry Reding
---
include/drm/drm_crtc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 768321ef76e8..3f607e78d361 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -463,7 +463,7 @@ struct drm_connector_state {
/**
* struct drm_connector_funcs - control connectors on a given device
- * @dpms: set power state (see drm_crtc_funcs above)
+ * @dpms: set power state
* @save: save connector state
* @restore: restore connector state
* @reset: reset connector after state has been invalidated (e.g. resume)
--
2.1.3
[PATCH 2/4] drm: Move IRQ related fields to proper section
From: Thierry Reding
The .irq and .irq_enabled fields are part of the VBLANK interrupt
handling infrastructure, so move them to the appropriate section within
the structure.
Signed-off-by: Thierry Reding
---
include/drm/drmP.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 227e67c52d5e..c05289d1d5f6 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -744,8 +744,6 @@ struct drm_device {
/** \name Context support */
/*@{ */
- bool irq_enabled; /**< True if irq handler is enabled */
- int irq;
__volatile__ long context_flag; /**< Context swapping flag */
int last_context; /**< Last current context */
@@ -753,6 +751,8 @@ struct drm_device {
/** \name VBLANK IRQ support */
/*@{ */
+ bool irq_enabled;
+ int irq;
/*
* At load time, disabling the vblank interrupt won't be allowed since
--
2.1.3
[PATCH 3/4] drm: Make drm_crtc_helper.h standalone includible
From: Thierry Reding
The file refers to a bunch of structure declared in drm_crtc.h, so
include it to make sure the drm_crtc_helper.h header can be included
standalone.
Signed-off-by: Thierry Reding
---
include/drm/drm_crtc_helper.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 7adbb65ea8ae..8608897ace10 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -39,6 +39,8 @@
#include
+#include
+
enum mode_set_atomic {
LEAVE_ATOMIC_MODE_SET,
ENTER_ATOMIC_MODE_SET,
--
2.1.3
[PATCH 4/4] drm: Include drm_crtc_helper.h in DocBook
From: Thierry Reding
There is already a section that describes the helpers implemented by
this module. Add the kerneldoc-generated structure descriptions to this
section.
While at it, add missing kerneldoc for the structures to avoid warnings
when generating the documentation.
Signed-off-by: Thierry Reding
---
Documentation/DocBook/drm.tmpl | 1 +
include/drm/drm_crtc_helper.h | 30 +-
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 4b592ffbafee..8b6fe229dc9a 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2362,6 +2362,7 @@ void intel_crt_init(struct drm_device *dev)
Modeset Helper Functions Reference
+!Iinclude/drm/drm_crtc_helper.h
!Edrivers/gpu/drm/drm_crtc_helper.c
!Pdrivers/gpu/drm/drm_crtc_helper.c overview
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 8608897ace10..e76828d81a8b 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -47,9 +47,20 @@ enum mode_set_atomic {
};
/**
- * drm_crtc_helper_funcs - helper operations for CRTCs
- * @mode_fixup: try to fixup proposed mode for this connector
+ * struct drm_crtc_helper_funcs - helper operations for CRTCs
+ * @dpms: set power state
+ * @prepare: prepare the CRTC, called before @mode_set
+ * @commit: commit changes to CRTC, called after @mode_set
+ * @mode_fixup: try to fixup proposed mode for this CRTC
* @mode_set: set this mode
+ * @mode_set_nofb: set mode only (no scanout buffer attached)
+ * @mode_set_base: update the scanout buffer
+ * @mode_set_base_atomic: non-blocking mode set (used for kgdb support)
+ * @load_lut: load color palette
+ * @disable: disable CRTC when no longer in use
+ * @atomic_check: check for validity of an atomic state
+ * @atomic_begin: begin atomic update
+ * @atomic_flush: flush atomic update
*
* The helper operations are called by the mid-layer CRTC helper.
*/
@@ -93,9 +104,17 @@ struct drm_crtc_helper_funcs {
};
/**
- * drm_encoder_helper_funcs - helper operations for encoders
+ * struct drm_encoder_helper_funcs - helper operations for encoders
+ * @dpms: set power state
+ * @save: save connector state
+ * @restore: restore connector state
* @mode_fixup: try to fixup proposed mode for this connector
+ * @prepare: part of the disable sequence, called before the CRTC modeset
+ * @commit: called after the CRTC modeset
* @mode_set: set this mode
+ * @get_crtc: return CRTC that the encoder is currently attached to
+ * @detect: connection status detection
+ * @disable: disable encoder when not in use (overrides DPMS off)
*
* The helper operations are called by the mid-layer CRTC helper.
*/
@@ -121,9 +140,10 @@ struct drm_encoder_helper_funcs {
};
/**
- * drm_connector_helper_funcs - helper operations for connectors
+ * struct drm_connector_helper_funcs - helper operations for connectors
* @get_modes: get mode list for this connector
- * @mode_valid (optional): is this mode valid on the given connector?
+ * @mode_valid: is this mode valid on the given connector? (optional)
+ * @best_encoder: return the preferred encoder for this connector
*
* The helper operations are called by the mid-layer CRTC helper.
*/
--
2.1.3
[Bug 89961] System Shutdown on High Load (Radeon)
https://bugzilla.kernel.org/show_bug.cgi?id=89961 Alex Deucher changed: What|Removed |Added CC||alexdeucher at gmail.com --- Comment #1 from Alex Deucher --- Please attach your xorg log, dmesg output, and lspci -vnn output. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 0/3] drm: Basic mode sanity checks
On Wed, Dec 17, 2014 at 6:56 AM, wrote: > From: Ville Syrjälä > > We had a bug in i915 land recently where X passed in a zeroed mode with > mode_valid=1 to setcrtc. That didn't go down so well and caused a > div-by-zero in i915. > > For a long time I've been thinking that we need some real checks to > validate the modes we feed into the hardware. I started to sketch out > something for that but it quickly turned into a bit of a nightmare > with the whole panel fitter, stereo 3D, SDVO etc. special cases we > have in i915. > > So I gave up on that for now, and instead cooked up this small series > to add some basic sanity checks to the mode validation, and also apply > the same sanity checks to user provided modes. This is enough to > prevent the div-by-zero in i915 with buggy X. > > The risk is that we start to reject some modes that more or less worked > by accident before. Given how lax we've been in handling the panel fitter > on i915 for instance, this could be a real problem if people have been > useing custom modes that have been filled out only partially. But I'm > hoping the number of users doing that is so small that we can risk it. > > The other concern is drivers which might also provide funky modes from > their .get_modes(). I tried to look at all the drivers a bit, and most > produce modes via EDID, CVT, DMT, or drm_display_mode_from_videomode(). > All of those look safe, except mode->clock might be an issue with > drm_display_mode_from_videomode(), but hopefully there's some kind of > clock provided in most cases. I didn't dig through all the nooks and > crannies in all drivers though, so may have missed something. For the series: Reviewed-by: Alex Deucher > > Ville Syrjälä (3): > drm: Reorganize probed mode validation > drm: Perform basic sanity checks on probed modes > drm: Do basic sanity checks for user modes > > drivers/gpu/drm/drm_crtc.c | 6 > drivers/gpu/drm/drm_modes.c| 56 > ++ > drivers/gpu/drm/drm_probe_helper.c | 43 ++--- > include/drm/drm_modes.h| 6 ++-- > 4 files changed, 74 insertions(+), 37 deletions(-) > > -- > 2.0.4 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] drm: Basic mode sanity checks
On Wed, Dec 17, 2014 at 12:09:47PM -0500, Alex Deucher wrote: > On Wed, Dec 17, 2014 at 6:56 AM, wrote: > > From: Ville Syrjälä > > > > We had a bug in i915 land recently where X passed in a zeroed mode with > > mode_valid=1 to setcrtc. That didn't go down so well and caused a > > div-by-zero in i915. > > > > For a long time I've been thinking that we need some real checks to > > validate the modes we feed into the hardware. I started to sketch out > > something for that but it quickly turned into a bit of a nightmare > > with the whole panel fitter, stereo 3D, SDVO etc. special cases we > > have in i915. > > > > So I gave up on that for now, and instead cooked up this small series > > to add some basic sanity checks to the mode validation, and also apply > > the same sanity checks to user provided modes. This is enough to > > prevent the div-by-zero in i915 with buggy X. > > > > The risk is that we start to reject some modes that more or less worked > > by accident before. Given how lax we've been in handling the panel fitter > > on i915 for instance, this could be a real problem if people have been > > useing custom modes that have been filled out only partially. But I'm > > hoping the number of users doing that is so small that we can risk it. > > > > The other concern is drivers which might also provide funky modes from > > their .get_modes(). I tried to look at all the drivers a bit, and most > > produce modes via EDID, CVT, DMT, or drm_display_mode_from_videomode(). > > All of those look safe, except mode->clock might be an issue with > > drm_display_mode_from_videomode(), but hopefully there's some kind of > > clock provided in most cases. I didn't dig through all the nooks and > > crannies in all drivers though, so may have missed something. > > For the series: > > Reviewed-by: Alex Deucher All pulled into my drm misc branch. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 1/4] drm: Remove stale comment
On Wed, Dec 17, 2014 at 04:41:40PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> The struct drm_connector_funcs kerneldoc refers to a part of struct
> drm_crtc_funcs that no longer exists.
>
> Signed-off-by: Thierry Reding
All four pulled into drm misc branch, thanks.
-Daniel
> ---
> include/drm/drm_crtc.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 768321ef76e8..3f607e78d361 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -463,7 +463,7 @@ struct drm_connector_state {
>
> /**
> * struct drm_connector_funcs - control connectors on a given device
> - * @dpms: set power state (see drm_crtc_funcs above)
> + * @dpms: set power state
> * @save: save connector state
> * @restore: restore connector state
> * @reset: reset connector after state has been invalidated (e.g. resume)
> --
> 2.1.3
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PULL] topic/atomic-fixes
Hi Dave, Yeah a pull for one patch is a bit overkill but I started to assemble the various patches for 3.20 in a branch for atomic props/ioctl and didn't realize that this bugfix here at the beginnning of the branch should be in 3.19 (because msm is using the helpers arleady). So if you'd merge we'd have it twice or or I need to shuffle branches again. Can do if you want. Cheers, Daniel The following changes since commit 4e0cd68115620bc3236ff4e58e4c073948629b41: drm: sti: fix module compilation issue (2014-12-15 17:07:57 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/atomic-fixes-2014-12-17 for you to fetch changes up to 4b08eae52f2f73723dbc4dd4d251eb60a7d8c0e1: drm/atomic: fix potential null ptr on plane enable (2014-12-17 18:39:57 +0100) Rob Clark (1): drm/atomic: fix potential null ptr on plane enable drivers/gpu/drm/drm_atomic_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[drm/atomic] BUG: unable to handle kernel NULL pointer dereference at 00000008
it_freeable+0x160/0x1e0
[1.190017] [<81682d82>] kernel_init+0x8/0xb8
[1.190017] [<8168dae0>] ret_from_kernel_thread+0x20/0x30
[1.190017] [<81682d7a>] ? rest_init+0xa2/0xa2
[1.190017] Code: ff 83 c8 04 00 00 8b 93 88 05 00 00 8b 45 f0 8b b0 c0 01
00 00 31 ff 57 56 8b 45 ec e8 1d fc ff ff 8b 93 fc 04 00 00 58 59 31 c0 <83> 7a
08 00 0f 84 bf 00 00 00 8b 93 b0 05 00 00 6a 00 6a 00 8b
[1.190017] EIP: [<81379138>] drm_universal_plane_init+0x143/0x214 SS:ESP
0068:8006bd90
[1.190017] CR2: 0008
[1.190017] ---[ end trace 3ffa9c8b438719d6 ]---
[1.190017] Kernel panic - not syncing: Fatal exception
git bisect start 161cea05a4f8f97dc75edda9c2546d7d1e1437de
b2776bf7149bddd1f4161f14f79520f17fc1d71d --
git bisect bad a6b151feb8a2681a73c316a37a8753622f243ee8 # 10:07 0-
20 Merge 'pm/linux-next' into devel-jaketown-smoke-201412170928
git bisect good a80479ce39f779468c5a07d9b350bda4a91b4bd6 # 10:23 20+
0 Merge 'fenghua/test' into devel-jaketown-smoke-201412170928
git bisect bad a32823272730eddc5198a77da0e6cc67e6586ff5 # 11:06 0-
9 Merge 'robclark/atomic-properties' into devel-jaketown-smoke-201412170928
git bisect good 49b70a31c166a6da12f60ec85da608c064555406 # 11:53 20+
0 bochs: fix bochsdrmfb mmap
git bisect good 547ad072838c48e18cab7bccb5c02cbfefe10da5 # 12:10 20+
0 drm/nouveau/kms: when pinning display-related buffers, force contig vram
git bisect good 064ca1d250b14b785e662b0a13d8d20cb84574e1 # 12:23 20+
0 drm/i915: Don't pin LRC in GGTT when dumping in debugfs
git bisect good 7608867d0c4d9da30e9efe6a53ff4ee1e6c4990b # 12:34 20+
0 Merge tag 'drm-intel-next-fixes-2014-12-04' of
git://anongit.freedesktop.org/drm-intel into drm-next
git bisect good eb929dc4d36db7881bbf90d5532b024615f64c0f # 12:47 20+
0 drm: sti: remove event lock while disabling vblank
git bisect good 731d754550b4076d39e06e656ba608612e3f63dd # 12:58 20+
0 Merge branch 'drm-sti-next-2014-12-11' of
http://git.linaro.org/people/benjamin.gaignard/kernel into drm-next
git bisect good 9f11ed76f492dcd7dcc7d159455c5d6b84ee8fe0 # 13:06 20+
0 drm: add atomic hook to read property plus helper
git bisect bad 77df8db029cf879929c155512ac7030dd2bed842 # 13:36 0-
20 drm/atomic: atomic plane properties
git bisect good a2c2b5a59c0d149654df7060753f0c5c459124be # 13:43 20+
0 drm: tweak getconnector locking
git bisect good 67021527bbd4649b8aee29c9b30ee7ed6fc0c23a # 14:16 20+
0 drm/atomic: atomic_check functions
# first bad commit: [77df8db029cf879929c155512ac7030dd2bed842] drm/atomic:
atomic plane properties
git bisect good 67021527bbd4649b8aee29c9b30ee7ed6fc0c23a # 14:18 60+
0 drm/atomic: atomic_check functions
# extra tests on HEAD of linux-devel/devel-jaketown-smoke-201412170928
git bisect bad 161cea05a4f8f97dc75edda9c2546d7d1e1437de # 14:18 0-
16 0day head guard for 'devel-jaketown-smoke-201412170928'
# extra tests on tree/branch robclark/atomic-properties
git bisect bad 820e68c3d3fb9c0ea036515c2e8535bc017c79fd # 15:20 0-
22 RFC: drm: Atomic modeset ioctl
# extra tests on tree/branch linus/master
git bisect good 603ba7e41bf5d405aba22294af5d075d8898176d # 15:29 60+
0 Merge branch 'for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
# extra tests on tree/branch next/master
git bisect good 5fef85d456eedf616809f7bf722b6f6a782d8a93 # 15:40 60+
0 Add linux-next specific files for 20141217
This script may reproduce the error.
#!/bin/bash
kernel=$1
initrd=yocto-minimal-i386.cgz
wget --no-clobber
https://github.com/fengguang/reproduce-kernel-bug/raw/master/initrd/$initrd
kvm=(
qemu-system-x86_64
-cpu kvm64
-enable-kvm
-kernel $kernel
-initrd $initrd
-m 320
-smp 1
-net nic,vlan=1,model=e1000
-net user,vlan=1
-boot order=nc
-no-reboot
-watchdog i6300esb
-rtc base=localtime
-serial stdio
-display none
-monitor null
)
append=(
hung_task_panic=1
earlyprintk=ttyS0,115200
debug
apic=debug
sysrq_always_enabled
rcupdate.rcu_cpu_stall_timeout=100
panic=-1
softlockup_panic=1
nmi_watchdog=panic
oops=panic
load_ramdisk=2
prompt_ramdisk=0
console=ttyS0,115200
console=tty0
vga=normal
root=/dev/ram0
rw
drbd.minor_count=8
)
"${kvm[@]}" --append "${append[*]}"
Thanks,
Fengguang
-- next part --
early console in setup code
[0.00] Ini
[PATCH] drm/qxl: Use drm_vblank_count()
On 12/17/2014 10:37 AM, Ville Syrjälä wrote:
> On Wed, Dec 17, 2014 at 03:57:51AM +0100, Mario Kleiner wrote:
>> On 12/15/2014 04:56 PM, Thierry Reding wrote:
>>> From: Thierry Reding
>>>
>>> The QXL driver duplicates part of the core's drm_vblank_count(), so it
>>> might as well use the core's variant for the extra goodies.
>>>
>>> Signed-off-by: Thierry Reding
>>> ---
>>>drivers/gpu/drm/qxl/qxl_drv.c | 7 +--
>>>1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
>>> index 1d9b80c91a15..497024461a3c 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_drv.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
>>> @@ -196,11 +196,6 @@ static int qxl_pm_restore(struct device *dev)
>>> return qxl_drm_resume(drm_dev, false);
>>>}
>>>
>>> -static u32 qxl_noop_get_vblank_counter(struct drm_device *dev, int crtc)
>>> -{
>>> - return dev->vblank[crtc].count.counter;
>>> -}
>>> -
>>>static int qxl_noop_enable_vblank(struct drm_device *dev, int crtc)
>>>{
>>> return 0;
>>> @@ -231,7 +226,7 @@ static struct drm_driver qxl_driver = {
>>>DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED,
>>> .load = qxl_driver_load,
>>> .unload = qxl_driver_unload,
>>> - .get_vblank_counter = qxl_noop_get_vblank_counter,
>>> + .get_vblank_counter = drm_vblank_count,
>>> .enable_vblank = qxl_noop_enable_vblank,
>>> .disable_vblank = qxl_noop_disable_vblank,
>>>
>> Hi
>>
>> That doesn't really help, although it doesn't hurt either. Just wanted
>> to point out that both the old and new method implement a no-op. The
>> get_vblank_counter() driver function is meant to implement a hardware
>> vblank counter query. It's only use case atm. is to reinitialize the
>> dev->vblank[crtc].count.counter counter returned by drm_vblank_count().
>>
>> The most honest implementation if there isn't any way to get a hw vblank
>> count would be to just "return 0;" - Same net effect, but at least a
>> marker in the code that there is future work to do.
> Yeah 'return 0' is what we do in i915 when there's no hw counter. I did
> consider changing it to drm_vblank_count() since that seems to be the
> current fad. I was hoping it might allow removing some code from
> drm_irq.c, but after some more thought that might not be the case.
> I'll probably need to take another look at it.
>
>> I think a better solution would be if we wouldn't require
>> .get_vblank_counter to be non-NULL, don't fake implement it in
>> kms-drivers which can't do it, and make the drm core deal with lack of
>> hw counter queries, e.g., by not disabling vblank irqs.
> That seems a bit drastic. The current delayed disable
> seems quite reasonable to me. The count will remain accurate as long
> as the vblank irq is enabled, and if you wait for so long that the
> irq gets disabled, well, I don't think a very precise answer was
> needed anyway.
Agreed. The 5 second timeout is imho reasonable in practice. I just
meant maybe we just check in the drm if that function is non-NULL, so
drivers are not forced to implement no-op stubs of that function if they
don't actually support it, just to avoid oopses.
> I was hunting some bugs in the vblank code recently, and while doing
> that I thought that I might change the code to use the timestamp
> difference between disable->enable to calculate an approximate number
> of vblanks lost and bump the counter appropriately. Didn't try it
> yet though, but seems like a reasonable idea when there's no hw
> counter. Though some care will be needed when dealing with
> drm_vblank_off/on.
>
Would be an option. Unsure if it is worth it or not in practice.
-mario
[Bug 85421] radeon stalled, GPU lockup, reset and failed on resume; crashed by firefox.
https://bugzilla.kernel.org/show_bug.cgi?id=85421 --- Comment #22 from Hin-Tak Leung --- Am just following the advice Alex gave in comment 2(try different versions of mesa) and reporting on my experience. It would appear that my problem is orthogonal to what the commit "radeonsi: Disable asynchronous DMA except for PIPE_BUFFER" was trying to address. That commit was in 10.3.4/10.3.5 and 10.4.0, and *not* in 10.2.9; but I have had crashes with 10.3.5 after ~5 days of use, 10.4.0 within a day, and 10.2.9 for nearly 4 weeks. 10.2.8: two crashes in 6 weeks. 10.2.9: crash after almost 4 weeks. 10.3.3: crash within first day 10.3.4: insufficient data - used it only for a day or two before 10.3.5 10.3.5: crashed after 5 days 10.4.0: crash within first day My crash-free days with 10.3.x/10.4.0 are measured in days if not hours, but with 10.2.x is in weeks. I'll continue to switch to a newer mesa as it comes out, and if I get burned, go back to the longest crash-free version until the another mesa version comes out. I think there is a bug with xv (so probably either mesa or glamor; does not seem to be sensitive to which version) because some videos plays skewed as in playing a square as: / / / / / / / / / / It happens only to certain specific videos (vdpau gl and x11 are fine), so I am not sure whether it is a bug in mplayer's use of xv, glamor's implementation of xv, or what. It seems to happens to videos with "Movie-Aspect is 1.xx:1 - prescaling to correct movie aspect." when played, but not all such videos are played badly. I am mentioning this, just in case digging further on that video playing problem might help fix the crash... -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] drm/fb-helper: Propagate errors from initial config failure
On Wed, Dec 17, 2014 at 10:39 AM, Thierry Reding
wrote:
> From: Thierry Reding
>
> Make drm_fb_helper_initial_config() return an int rather than a bool so
> that the error can be properly propagated. While at it, update drivers
> to propagate errors further rather than just ignore them.
>
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Patrik Jakobsson
> Cc: Rob Clark
> Cc: Tomi Valkeinen
> Cc: Alex Deucher
> Cc: Christian König
> Cc: Ben Skeggs
> Signed-off-by: Thierry Reding
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/ast/ast_fb.c| 21 +++--
> drivers/gpu/drm/bochs/bochs_fbdev.c | 14 --
> drivers/gpu/drm/cirrus/cirrus_fbdev.c | 22 --
> drivers/gpu/drm/drm_fb_helper.c | 2 +-
> drivers/gpu/drm/gma500/framebuffer.c| 22 ++
> drivers/gpu/drm/mgag200/mgag200_fb.c| 12 ++--
> drivers/gpu/drm/msm/msm_fbdev.c | 10 --
> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 21 +++--
> drivers/gpu/drm/omapdrm/omap_fbdev.c| 10 --
> drivers/gpu/drm/qxl/qxl_fb.c| 22 --
> drivers/gpu/drm/radeon/radeon_fb.c | 21 +++--
> drivers/gpu/drm/udl/udl_fb.c| 22 +++---
> include/drm/drm_fb_helper.h | 2 +-
> 13 files changed, 150 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> index 5c60ae524c45..ff68eefae273 100644
> --- a/drivers/gpu/drm/ast/ast_fb.c
> +++ b/drivers/gpu/drm/ast/ast_fb.c
> @@ -335,18 +335,27 @@ int ast_fbdev_init(struct drm_device *dev)
>
> ret = drm_fb_helper_init(dev, &afbdev->helper,
> 1, 1);
> - if (ret) {
> - kfree(afbdev);
> - return ret;
> - }
> + if (ret)
> + goto free;
>
> - drm_fb_helper_single_add_all_connectors(&afbdev->helper);
> + ret = drm_fb_helper_single_add_all_connectors(&afbdev->helper);
> + if (ret)
> + goto fini;
>
> /* disable all the possible outputs/crtcs before entering KMS mode */
> drm_helper_disable_unused_functions(dev);
>
> - drm_fb_helper_initial_config(&afbdev->helper, 32);
> + ret = drm_fb_helper_initial_config(&afbdev->helper, 32);
> + if (ret)
> + goto fini;
> +
> return 0;
> +
> +fini:
> + drm_fb_helper_fini(&afbdev->helper);
> +free:
> + kfree(afbdev);
> + return ret;
> }
>
> void ast_fbdev_fini(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
> b/drivers/gpu/drm/bochs/bochs_fbdev.c
> index 61dbf09dff5d..976d9798dc99 100644
> --- a/drivers/gpu/drm/bochs/bochs_fbdev.c
> +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
> @@ -207,12 +207,22 @@ int bochs_fbdev_init(struct bochs_device *bochs)
> if (ret)
> return ret;
>
> - drm_fb_helper_single_add_all_connectors(&bochs->fb.helper);
> + ret = drm_fb_helper_single_add_all_connectors(&bochs->fb.helper);
> + if (ret)
> + goto fini;
> +
> drm_helper_disable_unused_functions(bochs->dev);
> - drm_fb_helper_initial_config(&bochs->fb.helper, 32);
> +
> + ret = drm_fb_helper_initial_config(&bochs->fb.helper, 32);
> + if (ret)
> + goto fini;
>
> bochs->fb.initialized = true;
> return 0;
> +
> +fini:
> + drm_fb_helper_fini(&bochs->fb.helper);
> + return ret;
> }
>
> void bochs_fbdev_fini(struct bochs_device *bochs)
> diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> index 502a89eb54b5..0682210b068b 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> @@ -317,17 +317,27 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
>
> ret = drm_fb_helper_init(cdev->dev, &gfbdev->helper,
> cdev->num_crtc, CIRRUSFB_CONN_LIMIT);
> - if (ret) {
> - kfree(gfbdev);
> - return ret;
> - }
> - drm_fb_helper_single_add_all_connectors(&gfbdev->helper);
> + if (ret)
> + goto free;
> +
> + ret = drm_fb_helper_single_add_all_connectors(&gfbdev->helper);
> + if (ret)
> + goto fini;
>
> /* disable all the possible outputs/crtcs before entering KMS mode */
> drm_helper_disable_unused_functions(cdev->dev);
> - drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel);
> +
> + ret = drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel);
> + if (ret)
> + goto fini;
>
> return 0;
> +
> +free:
> + kfree(gfbdev);
> +fini:
> + drm_fb_helper_fini(&gfbdev->helper);
> + return ret;
> }
>
> void cirrus_fbdev_fini(struct cirrus_device *cdev)
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_he
[PATCH 4/4] drm: Include drm_crtc_helper.h in DocBook
On Wed, Dec 17, 2014 at 10:41 AM, Thierry Reding
wrote:
> From: Thierry Reding
>
> There is already a section that describes the helpers implemented by
> this module. Add the kerneldoc-generated structure descriptions to this
> section.
>
> While at it, add missing kerneldoc for the structures to avoid warnings
> when generating the documentation.
>
> Signed-off-by: Thierry Reding
Reviewed-by: Alex Deucher
> ---
> Documentation/DocBook/drm.tmpl | 1 +
> include/drm/drm_crtc_helper.h | 30 +-
> 2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 4b592ffbafee..8b6fe229dc9a 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2362,6 +2362,7 @@ void intel_crt_init(struct drm_device *dev)
>
>
>Modeset Helper Functions Reference
> +!Iinclude/drm/drm_crtc_helper.h
> !Edrivers/gpu/drm/drm_crtc_helper.c
> !Pdrivers/gpu/drm/drm_crtc_helper.c overview
>
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 8608897ace10..e76828d81a8b 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -47,9 +47,20 @@ enum mode_set_atomic {
> };
>
> /**
> - * drm_crtc_helper_funcs - helper operations for CRTCs
> - * @mode_fixup: try to fixup proposed mode for this connector
> + * struct drm_crtc_helper_funcs - helper operations for CRTCs
> + * @dpms: set power state
> + * @prepare: prepare the CRTC, called before @mode_set
> + * @commit: commit changes to CRTC, called after @mode_set
> + * @mode_fixup: try to fixup proposed mode for this CRTC
> * @mode_set: set this mode
> + * @mode_set_nofb: set mode only (no scanout buffer attached)
> + * @mode_set_base: update the scanout buffer
> + * @mode_set_base_atomic: non-blocking mode set (used for kgdb support)
> + * @load_lut: load color palette
> + * @disable: disable CRTC when no longer in use
> + * @atomic_check: check for validity of an atomic state
> + * @atomic_begin: begin atomic update
> + * @atomic_flush: flush atomic update
> *
> * The helper operations are called by the mid-layer CRTC helper.
> */
> @@ -93,9 +104,17 @@ struct drm_crtc_helper_funcs {
> };
>
> /**
> - * drm_encoder_helper_funcs - helper operations for encoders
> + * struct drm_encoder_helper_funcs - helper operations for encoders
> + * @dpms: set power state
> + * @save: save connector state
> + * @restore: restore connector state
> * @mode_fixup: try to fixup proposed mode for this connector
> + * @prepare: part of the disable sequence, called before the CRTC modeset
> + * @commit: called after the CRTC modeset
> * @mode_set: set this mode
> + * @get_crtc: return CRTC that the encoder is currently attached to
> + * @detect: connection status detection
> + * @disable: disable encoder when not in use (overrides DPMS off)
> *
> * The helper operations are called by the mid-layer CRTC helper.
> */
> @@ -121,9 +140,10 @@ struct drm_encoder_helper_funcs {
> };
>
> /**
> - * drm_connector_helper_funcs - helper operations for connectors
> + * struct drm_connector_helper_funcs - helper operations for connectors
> * @get_modes: get mode list for this connector
> - * @mode_valid (optional): is this mode valid on the given connector?
> + * @mode_valid: is this mode valid on the given connector? (optional)
> + * @best_encoder: return the preferred encoder for this connector
> *
> * The helper operations are called by the mid-layer CRTC helper.
> */
> --
> 2.1.3
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm: Move IRQ related fields to proper section
On Wed, Dec 17, 2014 at 10:41 AM, Thierry Reding
wrote:
> From: Thierry Reding
>
> The .irq and .irq_enabled fields are part of the VBLANK interrupt
> handling infrastructure, so move them to the appropriate section within
> the structure.
>
> Signed-off-by: Thierry Reding
Reviewed-by: Alex Deucher
> ---
> include/drm/drmP.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 227e67c52d5e..c05289d1d5f6 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -744,8 +744,6 @@ struct drm_device {
>
> /** \name Context support */
> /*@{ */
> - bool irq_enabled; /**< True if irq handler is enabled */
> - int irq;
>
> __volatile__ long context_flag; /**< Context swapping flag */
> int last_context; /**< Last current context */
> @@ -753,6 +751,8 @@ struct drm_device {
>
> /** \name VBLANK IRQ support */
> /*@{ */
> + bool irq_enabled;
> + int irq;
>
> /*
> * At load time, disabling the vblank interrupt won't be allowed since
> --
> 2.1.3
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/fb-helper: Propagate errors from initial config failure
On Wed, Dec 17, 2014 at 4:39 PM, Thierry Reding
wrote:
> From: Thierry Reding
>
> Make drm_fb_helper_initial_config() return an int rather than a bool so
> that the error can be properly propagated. While at it, update drivers
> to propagate errors further rather than just ignore them.
>
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Patrik Jakobsson
> Cc: Rob Clark
> Cc: Tomi Valkeinen
> Cc: Alex Deucher
> Cc: Christian König
> Cc: Ben Skeggs
> Signed-off-by: Thierry Reding
Looks good
Reviewed-by: Patrik Jakobsson
> ---
> drivers/gpu/drm/ast/ast_fb.c| 21 +++--
> drivers/gpu/drm/bochs/bochs_fbdev.c | 14 --
> drivers/gpu/drm/cirrus/cirrus_fbdev.c | 22 --
> drivers/gpu/drm/drm_fb_helper.c | 2 +-
> drivers/gpu/drm/gma500/framebuffer.c| 22 ++
> drivers/gpu/drm/mgag200/mgag200_fb.c| 12 ++--
> drivers/gpu/drm/msm/msm_fbdev.c | 10 --
> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 21 +++--
> drivers/gpu/drm/omapdrm/omap_fbdev.c| 10 --
> drivers/gpu/drm/qxl/qxl_fb.c| 22 --
> drivers/gpu/drm/radeon/radeon_fb.c | 21 +++--
> drivers/gpu/drm/udl/udl_fb.c| 22 +++---
> include/drm/drm_fb_helper.h | 2 +-
> 13 files changed, 150 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> index 5c60ae524c45..ff68eefae273 100644
> --- a/drivers/gpu/drm/ast/ast_fb.c
> +++ b/drivers/gpu/drm/ast/ast_fb.c
> @@ -335,18 +335,27 @@ int ast_fbdev_init(struct drm_device *dev)
>
> ret = drm_fb_helper_init(dev, &afbdev->helper,
> 1, 1);
> - if (ret) {
> - kfree(afbdev);
> - return ret;
> - }
> + if (ret)
> + goto free;
>
> - drm_fb_helper_single_add_all_connectors(&afbdev->helper);
> + ret = drm_fb_helper_single_add_all_connectors(&afbdev->helper);
> + if (ret)
> + goto fini;
>
> /* disable all the possible outputs/crtcs before entering KMS mode */
> drm_helper_disable_unused_functions(dev);
>
> - drm_fb_helper_initial_config(&afbdev->helper, 32);
> + ret = drm_fb_helper_initial_config(&afbdev->helper, 32);
> + if (ret)
> + goto fini;
> +
> return 0;
> +
> +fini:
> + drm_fb_helper_fini(&afbdev->helper);
> +free:
> + kfree(afbdev);
> + return ret;
> }
>
> void ast_fbdev_fini(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
> b/drivers/gpu/drm/bochs/bochs_fbdev.c
> index 61dbf09dff5d..976d9798dc99 100644
> --- a/drivers/gpu/drm/bochs/bochs_fbdev.c
> +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
> @@ -207,12 +207,22 @@ int bochs_fbdev_init(struct bochs_device *bochs)
> if (ret)
> return ret;
>
> - drm_fb_helper_single_add_all_connectors(&bochs->fb.helper);
> + ret = drm_fb_helper_single_add_all_connectors(&bochs->fb.helper);
> + if (ret)
> + goto fini;
> +
> drm_helper_disable_unused_functions(bochs->dev);
> - drm_fb_helper_initial_config(&bochs->fb.helper, 32);
> +
> + ret = drm_fb_helper_initial_config(&bochs->fb.helper, 32);
> + if (ret)
> + goto fini;
>
> bochs->fb.initialized = true;
> return 0;
> +
> +fini:
> + drm_fb_helper_fini(&bochs->fb.helper);
> + return ret;
> }
>
> void bochs_fbdev_fini(struct bochs_device *bochs)
> diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> index 502a89eb54b5..0682210b068b 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> @@ -317,17 +317,27 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
>
> ret = drm_fb_helper_init(cdev->dev, &gfbdev->helper,
> cdev->num_crtc, CIRRUSFB_CONN_LIMIT);
> - if (ret) {
> - kfree(gfbdev);
> - return ret;
> - }
> - drm_fb_helper_single_add_all_connectors(&gfbdev->helper);
> + if (ret)
> + goto free;
> +
> + ret = drm_fb_helper_single_add_all_connectors(&gfbdev->helper);
> + if (ret)
> + goto fini;
>
> /* disable all the possible outputs/crtcs before entering KMS mode */
> drm_helper_disable_unused_functions(cdev->dev);
> - drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel);
> +
> + ret = drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel);
> + if (ret)
> + goto fini;
>
> return 0;
> +
> +free:
> + kfree(gfbdev);
> +fini:
> + drm_fb_helper_fini(&gfbdev->helper);
> + return ret;
> }
>
> void cirrus_fbdev_fini(struct cirrus_device *cdev)
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gp
[Bug 89661] Kernel panic when trying use amdkfd driver on Kaveri
https://bugzilla.kernel.org/show_bug.cgi?id=89661 --- Comment #16 from Bernd Steinhauser --- Thanks, I'll give it a try. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 66761] ACPI "Firmware Bug" (IGPU, Z01I) causes trouble
https://bugzilla.kernel.org/show_bug.cgi?id=66761 KernelBug <3fdd1e5d at opayq.com> changed: What|Removed |Added CC||3fdd1e5d at opayq.com --- Comment #9 from KernelBug <3fdd1e5d at opayq.com> --- Hi, Not sure if my messages are related, but surfing the internet led me to here, with what appears to be the same types of messages in the logs. If this seems to have the same relation to this original post, please let me know if you'd like any logs... My Specs; Asus G75vw laptop Slackware 14.1 x86_64 Kernel 3.17.6 Nvidia Geforce GTX660M nvidia-driver-340.58 $dmesg -t | grep -i 'error\|warn\|exception' ACPI Warning: SystemIO range 0xf000-0xf01f conflicts with OpRegion 0xf000-0xf00f (\SMB0) (20140724/utaddress-258) ACPI Warning: SystemIO range 0xf000-0xf01f conflicts with OpRegion 0xf000-0xf00f (\_SB_.PCI0.SBUS.SMBI) (20140724/utaddress-258) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) ACPI Warning: \_SB_.PCI0.PEG0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20140724/nsarguments-95) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 89961] System Shutdown on High Load (Radeon)
https://bugzilla.kernel.org/show_bug.cgi?id=89961 --- Comment #2 from swoorupj at gmail.com --- Created attachment 161091 --> https://bugzilla.kernel.org/attachment.cgi?id=161091&action=edit dmesg output -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 89961] System Shutdown on High Load (Radeon)
https://bugzilla.kernel.org/show_bug.cgi?id=89961 --- Comment #3 from swoorupj at gmail.com --- Created attachment 161101 --> https://bugzilla.kernel.org/attachment.cgi?id=161101&action=edit lspci -vnn output -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 89961] System Shutdown on High Load (Radeon)
https://bugzilla.kernel.org/show_bug.cgi?id=89961 --- Comment #4 from swoorupj at gmail.com --- Created attachment 16 --> https://bugzilla.kernel.org/attachment.cgi?id=16&action=edit Xorg log file -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH V8 00/14] drm/exynos: few patches to enhance bridge chip support
Hello Laurent and Tomi, On 12/16/2014 12:37 AM, Laurent Pinchart wrote: > Hi Javier, >> 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. > > I don't have time to review the series in details right now, but I'm happy > with the DT bindings, and have no big issue with the rest of the patches. I > don't really like the of_drm_find_bridge() concept introduced in 03/14 but I > won't nack it given lack of time to implement an alternative proposal. It's > an > internal API, it can always be reworked later anyway. > Thanks a lot for taking the time to look at the DT bindings, then I guess that the series are finally ready to be merged? Ajay's series don't apply cleanly anymore because it has been a while since he posted it but he can rebase on top of 3.19-rc1 once it is released and re-resend. Best regards, Javier
drm/dsi: Add message to packet translator
On Wed, Dec 17, 2014 at 02:53:05AM +0300, Dan Carpenter wrote:
> Hello Thierry Reding,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch a52879e8d7cb: "drm/dsi: Add message to packet translator"
> from Oct 16, 2014, leads to the following Smatch complaint:
>
> drivers/gpu/drm/drm_mipi_dsi.c:328 mipi_dsi_create_packet()
>warn: variable dereferenced before check 'msg' (see line 326)
>
> drivers/gpu/drm/drm_mipi_dsi.c
>325{
>326const u8 *tx = msg->tx_buf;
>^^^
> Dereference.
>
>327
>328if (!packet || !msg)
>
> Check.
>
>329return -EINVAL;
>330
>
> regards,
> dan carpenter
I've had a patch in my tree for almost two weeks now, I wonder why this
is still there...
Ah, forgot to push it of course. Done now.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20141217/2e10b43f/attachment.sig>
[PATCH] drm/dsi: Do the 'msg' check prior to dereferencing it.
On Tue, Dec 16, 2014 at 10:04:34PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam
>
> Commit a52879e8d7cb ("drm/dsi: Add message to packet translator") caused the
> following Smatch complaint:
>
> drivers/gpu/drm/drm_mipi_dsi.c:328 mipi_dsi_create_packet()
> warn: variable dereferenced before check 'msg' (see line 326)
>
> Do the 'msg' check prior to dereferencing it.
>
> Reported-by: Dan Carpenter
> Signed-off-by: Fabio Estevam
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
I've had an equivalent patch in the drm/panel tree for almost two weeks
but had forgotten to push. Pushed now.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20141217/882e2e69/attachment.sig>
