[PATCH libdrm] tests: remove missleading comments
On 12/18/2015 12:14 AM, Stefan Agner wrote: > The comment has been copied from modetest and is not applicable > for vbltest. > > Signed-off-by: Stefan Agner > --- > tests/vbltest/vbltest.c | 15 +-- > 1 file changed, 1 insertion(+), 14 deletions(-) > > diff --git a/tests/vbltest/vbltest.c b/tests/vbltest/vbltest.c > index 531196c..5c61b43 100644 > --- a/tests/vbltest/vbltest.c > +++ b/tests/vbltest/vbltest.c > @@ -1,5 +1,5 @@ > /* > - * DRM based mode setting test program > + * DRM based vblank test program > * Copyright 2008 Tungsten Graphics > * Jakob Bornecrantz > * Copyright 2008 Intel Corporation > @@ -24,19 +24,6 @@ > * IN THE SOFTWARE. > */ > > -/* > - * This fairly simple test program dumps output in a similar format to the > - * "xrandr" tool everyone knows & loves. It's necessarily slightly different > - * since the kernel separates outputs into encoder and connector structures, > - * each with their own unique ID. The program also allows test testing of > the > - * memory management and mode setting APIs by allowing the user to specify a > - * connector and mode to use for mode setting. If all works as expected, a > - * blue background should be painted on the monitor attached to the specified > - * connector after the selected mode is set. > - * > - * TODO: use cairo to write the mode info on the selected output once > - * the mode has been programmed, along with possible test patterns. > - */ > #ifdef HAVE_CONFIG_H > #include "config.h" > #endif Yep, thanks. Reviewed-by: Jesse Barnes
[PATCH] intel: merge latest i915_drm.h
On 12/12/2015 07:16 AM, Emil Velikov wrote: > On 11 December 2015 at 21:55, Jesse Barnes > wrote: >> Pick up context flags, softpin, etc. >> >> Signed-off-by: Jesse Barnes >> --- >> include/drm/i915_drm.h | 57 >> ++ >> 1 file changed, 48 insertions(+), 9 deletions(-) >> > Any objections if we do this (and pretty much every other outdated > header) in a single go, as the header cleanups hit Linus' tree ? > Dave already has then in drm-next :-) No objection here. Feel free to push it with my ack! Thanks, Jesse
[PATCH] intel: merge latest i915_drm.h
Pick up context flags, softpin, etc.
Signed-off-by: Jesse Barnes
---
include/drm/i915_drm.h | 57 ++
1 file changed, 48 insertions(+), 9 deletions(-)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index ded43b1..4ce1fe9 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -171,8 +171,12 @@ typedef struct _drm_i915_sarea {
#define I915_BOX_TEXTURE_LOAD 0x8
#define I915_BOX_LOST_CONTEXT 0x10
-/* I915 specific ioctls
- * The device specific ioctl range is 0x40 to 0x79.
+/*
+ * i915 specific ioctls.
+ *
+ * The device specific ioctl range is [DRM_COMMAND_BASE, DRM_COMMAND_END) ie
+ * [0x40, 0xa0) (a0 is excluded). The numbers below are defined as offset
+ * against DRM_COMMAND_BASE and should be between [0x0, 0x60).
*/
#define DRM_I915_INIT 0x00
#define DRM_I915_FLUSH 0x01
@@ -270,7 +274,7 @@ typedef struct _drm_i915_sarea {
#define DRM_IOCTL_I915_OVERLAY_PUT_IMAGE DRM_IOW(DRM_COMMAND_BASE +
DRM_I915_OVERLAY_PUT_IMAGE, struct drm_intel_overlay_put_image)
#define DRM_IOCTL_I915_OVERLAY_ATTRS DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs)
#define DRM_IOCTL_I915_SET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
-#define DRM_IOCTL_I915_GET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
+#define DRM_IOCTL_I915_GET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_GET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
#define DRM_IOCTL_I915_GEM_WAITDRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_GEM_WAIT, struct drm_i915_gem_wait)
#define DRM_IOCTL_I915_GEM_CONTEXT_CREATE DRM_IOWR (DRM_COMMAND_BASE +
DRM_I915_GEM_CONTEXT_CREATE, struct drm_i915_gem_context_create)
#define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE +
DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy)
@@ -350,9 +354,16 @@ typedef struct drm_i915_irq_wait {
#define I915_PARAM_REVISION 32
#define I915_PARAM_SUBSLICE_TOTAL 33
#define I915_PARAM_EU_TOTAL 34
+#define I915_PARAM_HAS_GPU_RESET35
+#define I915_PARAM_HAS_RESOURCE_STREAMER 36
+#define I915_PARAM_HAS_EXEC_SOFTPIN 37
typedef struct drm_i915_getparam {
- int param;
+ __s32 param;
+ /*
+* WARNING: Using pointers instead of fixed-size u64 means we need to
write
+* compat32 code. Don't repeat this mistake.
+*/
int *value;
} drm_i915_getparam_t;
@@ -672,15 +683,21 @@ struct drm_i915_gem_exec_object2 {
__u64 alignment;
/**
-* Returned value of the updated offset of the object, for future
-* presumed_offset writes.
+* When the EXEC_OBJECT_PINNED flag is specified this is populated by
+* the user with the GTT offset at which this object will be pinned.
+* When the I915_EXEC_NO_RELOC flag is specified this must contain the
+* presumed_offset of the object.
+* During execbuffer2 the kernel populates it with the value of the
+* current GTT offset of the object, for future presumed_offset writes.
*/
__u64 offset;
#define EXEC_OBJECT_NEEDS_FENCE (1<<0)
#define EXEC_OBJECT_NEEDS_GTT (1<<1)
#define EXEC_OBJECT_WRITE (1<<2)
-#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_WRITE<<1)
+#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
+#define EXEC_OBJECT_PINNED (1<<4)
+#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_PINNED<<1)
__u64 flags;
__u64 rsvd1;
@@ -760,7 +777,12 @@ struct drm_i915_gem_execbuffer2 {
#define I915_EXEC_BSD_RING1(1<<13)
#define I915_EXEC_BSD_RING2(2<<13)
-#define __I915_EXEC_UNKNOWN_FLAGS -(1<<15)
+/** Tell the kernel that the batchbuffer is processed by
+ * the resource streamer.
+ */
+#define I915_EXEC_RESOURCE_STREAMER (1<<15)
+
+#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER<<1)
#define I915_EXEC_CONTEXT_ID_MASK (0x)
#define i915_execbuffer2_set_context_id(eb2, context) \
@@ -996,6 +1018,7 @@ struct drm_intel_overlay_put_image {
/* flags */
#define I915_OVERLAY_UPDATE_ATTRS (1<<0)
#define I915_OVERLAY_UPDATE_GAMMA (1<<1)
+#define I915_OVERLAY_DISABLE_DEST_COLORKEY (1<<2)
struct drm_intel_overlay_attrs {
__u32 flags;
__u32 color_key;
@@ -1062,9 +1085,23 @@ struct drm_i915_gem_context_destroy {
};
struct drm_i915_reg_read {
+ /*
+* Register offset.
+* For 64bit wide registers where the upper 32bits don't immediately
+* follow the lower 32bits, the offset of the lower 32bits must
+* be specified
+*/
__u64 offset;
__u64 val; /* Return value */
};
+/* Known registers:
+ *
+ * Re
[Intel-gfx] [git pull] drm for 4.3
Cc'ing Maarten and Matt; I'm guessing this may be related to one of their recent patches. Jesse On 09/21/2015 11:48 AM, Dave Jones wrote: > On Mon, Sep 07, 2015 at 02:45:59PM -0400, Dave Jones wrote: > > On Fri, Sep 04, 2015 at 11:40:53PM +0100, Dave Airlie wrote: > > > > > > Hi Linus, > > > > > > This is the main pull request for the drm for 4.3. Nouveau is probably > the biggest > > > amount of changes in here, since it missed 4.2. Highlights below, along > with the usual > > > bunch of fixes. There are a few minor conflicts with your tree but > nothing > > > you can't handle. All stuff outside drm should have applicable acks. > > > > > > Highlights: > > > > > > ... > > > i915: > > > Skylake support enabled by default > > > legacy modesetting using atomic infrastructure > > > Skylake fixes > > > GEN9 workarounds > > > > Since this merge, I'm seeing this twice during boot.. > > And still there in -rc2. Several other people reported this too, > and they also got no reponse. > > I'll start bisecting when I get home tonight. It shouldn't be too hard, > as 4.2 was fine. > > Dave > > > [ cut here ] > > WARNING: CPU: 0 PID: 6 at drivers/gpu/drm/i915/intel_display.c:1377 > assert_planes_disabled+0xdf/0x140() > > plane A assertion failure, should be disabled but not > > CPU: 0 PID: 6 Comm: kworker/u8:0 Not tainted 4.2.0-think+ #9 > > Workqueue: events_unbound async_run_entry_fn > > 0561 88050392b6f8 8d7dccce 88050392b740 > > 88050392b730 8d079ee2 880500a6 > > 8805008e99c8 88050392b790 > > Call Trace: > > [] dump_stack+0x4e/0x79 > > [] warn_slowpath_common+0x82/0xc0 > > [] warn_slowpath_fmt+0x4c/0x50 > > [] assert_planes_disabled+0xdf/0x140 > > [] intel_disable_pipe+0x4b/0x2c0 > > [] haswell_crtc_disable+0x8a/0x2e0 > > [] intel_atomic_commit+0xff/0x1320 > > [] ? drm_atomic_check_only+0x21e/0x550 > > [] drm_atomic_commit+0x37/0x60 > > [] drm_atomic_helper_set_config+0x1c5/0x430 > > [] drm_mode_set_config_internal+0x65/0x110 > > [] restore_fbdev_mode+0xbe/0xe0 > > [] drm_fb_helper_restore_fbdev_mode_unlocked+0x25/0x70 > > [] drm_fb_helper_set_par+0x2d/0x50 > > [] intel_fbdev_set_par+0x1a/0x60 > > [] fbcon_init+0x545/0x5d0 > > [] visual_init+0xca/0x130 > > [] do_bind_con_driver+0x1c5/0x3b0 > > [] do_take_over_console+0x149/0x1a0 > > [] do_fbcon_takeover+0x57/0xb0 > > [] fbcon_event_notify+0x66c/0x760 > > [] notifier_call_chain+0x3e/0xb0 > > [] __blocking_notifier_call_chain+0x4d/0x70 > > [] blocking_notifier_call_chain+0x16/0x20 > > [] fb_notifier_call_chain+0x1b/0x20 > > [] register_framebuffer+0x1e7/0x300 > > [] drm_fb_helper_initial_config+0x252/0x3e0 > > [] intel_fbdev_initial_config+0x1b/0x20 > > [] async_run_entry_fn+0x4a/0x140 > > [] process_one_work+0x1fd/0x670 > > [] ? process_one_work+0x16c/0x670 > > [] worker_thread+0x4e/0x450 > > [] ? process_one_work+0x670/0x670 > > [] kthread+0x101/0x120 > > [] ? kthread_create_on_node+0x250/0x250 > > [] ret_from_fork+0x3f/0x70 > > [] ? kthread_create_on_node+0x250/0x250 > > ---[ end trace 54cab2e0c772d5d9 ]--- > > > > > > 00:02.0 VGA compatible controller: Intel Corporation Xeon E3-1200 v3 > Processor Integrated Graphics Controller (rev 06) > > ___ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
[Intel-gfx] [PATCH v2 12/22] drm/i915: Preserve SSC earlier
> - * indicates as much. > - */ > - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) > - dev_priv->vbt.lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) & > - DREF_SSC1_ENABLE); > - > intel_modeset_init_hw(dev); > > intel_setup_overlay(dev); > Yeah looks good (and I'm having deja vu here; I thought I ran into the same ordering issue at one point in a report from krh, but I guess I never fixed it; didn't have test hw at the time). Reviewed-by: Jesse Barnes Thanks, Jesse
[RFC PATCH] drm/fb: drop panic handling
On 07/16/2015 01:27 AM, Daniel Vetter wrote:
> On Thu, Jul 09, 2015 at 11:14:43PM +0200, Daniel Vetter wrote:
>> On Thu, Jul 09, 2015 at 01:15:34PM +1000, Dave Airlie wrote:
>>> From: Dave Airlie
>>>
>>> This really doesn't seem to have much chance of working anymore,
>>>
>>> esp for irq context, qxl at least tries to talk to the hw,
>>> and waits for irqs, and fails.
>>>
>>> with runtime pm and other stuff I think we should just
>>> bail on this for now.
>>>
>>> Signed-off-by: Dave Airlie
>>
>> Yeah concurred that this has become hopeless. Also this would allow us to
>> drop an pretension in i915 to still support this which means we can stop
>> checking drm_can_sleep in our wait_for macros. Which has papered over some
>> pretty serious bugs.
>>
>> Reviewed-by: Daniel Vetter
>
> Well I applied this to drm-misc.
>
>> There's one more though, we can get into the fbdev callbacks through a
>> pretty impressive chain:
>>
>> panic() -> bust_spinlock() -> console_unblank() -> pass the trylock ->
>> c->unblank() -> unblank_screen() (now in vt/vt.c) ->
>> vc_sw->con_blank() -> fbcon_blank()
>>
>> To make this really complete I think we also need to sprinkle
>>
>> if (oops_in_progress)
>> return;
>>
>> over all the fbdev entry points we have in drm_fbdev_helper.c plus all the
>> ones in drivers which have their own (qxl, udl, i915 are the ones I know
>> of).
>
> I'll do a patch for this. Just realized that we have some cargo-culted
> checks already, but they don't work everywhere so mostly about unifying
> everything.
> -Daniel
>
>>
>> Cheers, Daniel
>>
>>> ---
>>> drivers/gpu/drm/drm_fb_helper.c | 26 --
>>> 1 file changed, 26 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>>> b/drivers/gpu/drm/drm_fb_helper.c
>>> index cac4229..eaf652b 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -429,24 +429,6 @@ static bool drm_fb_helper_force_kernel_mode(void)
>>> return error;
>>> }
>>>
>>> -static int drm_fb_helper_panic(struct notifier_block *n, unsigned long
>>> ununsed,
>>> - void *panic_str)
>>> -{
>>> - /*
>>> -* It's a waste of time and effort to switch back to text console
>>> -* if the kernel should reboot before panic messages can be seen.
>>> -*/
>>> - if (panic_timeout < 0)
>>> - return 0;
>>> -
>>> - pr_err("panic occurred, switching back to text console\n");
>>> - return drm_fb_helper_force_kernel_mode();
>>> -}
>>> -
>>> -static struct notifier_block paniced = {
>>> - .notifier_call = drm_fb_helper_panic,
>>> -};
>>> -
>>> static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
>>> {
>>> struct drm_device *dev = fb_helper->dev;
>>> @@ -672,9 +654,6 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
>>> if (!list_empty(_helper->kernel_fb_list)) {
>>> list_del(_helper->kernel_fb_list);
>>> if (list_empty(_fb_helper_list)) {
>>> - pr_info("drm: unregistered panic notifier\n");
>>> - atomic_notifier_chain_unregister(_notifier_list,
>>> -);
>>> unregister_sysrq_key('v',
>>> _drm_fb_helper_restore_op);
>>> }
>>> }
>>> @@ -1109,12 +1088,7 @@ static int drm_fb_helper_single_fb_probe(struct
>>> drm_fb_helper *fb_helper,
>>> dev_info(fb_helper->dev->dev, "fb%d: %s frame buffer device\n",
>>> info->node, info->fix.id);
>>>
>>> - /* Switch back to kernel console on panic */
>>> - /* multi card linked list maybe */
>>> if (list_empty(_fb_helper_list)) {
>>> - dev_info(fb_helper->dev->dev, "registered panic notifier\n");
>>> - atomic_notifier_chain_register(_notifier_list,
>>> - );
>>> register_sysrq_key('v', _drm_fb_helper_restore_op);
>>> }
I missed this; Daniel just told me about it tonight. It's too bad we
have to drop it; I think PPC has had it since forever, so losing it from
KMS is too bad. Hopefully we can bring it back somehow with David's new
kernel console stuff at some point...
Jesse
[Intel-gfx] [PATCH] drm/i915: fix definition of the DRM_IOCTL_I915_GET_SPRITE_COLORKEY ioctl
On 03/27/2015 01:04 AM, Daniel Vetter wrote: > On Fri, Mar 27, 2015 at 08:39:56AM +0200, Jani Nikula wrote: >> On Thu, 26 Mar 2015, Tommi Rantala wrote: >>> Fix definition of the DRM_IOCTL_I915_GET_SPRITE_COLORKEY ioctl, so that it >>> is different from the DRM_IOCTL_I915_SET_SPRITE_COLORKEY ioctl. >>> >>> Signed-off-by: Tommi Rantala >> >> Whoa. Broken since its introduction in >> >> commit 8ea30864229e54b01ac0e9fe88c4b733a940ec4e >> Author: Jesse Barnes >> Date: Tue Jan 3 08:05:39 2012 -0800 >> >> drm/i915: add color key support v4 >> >> Cc: stable at vger.kernel.org > > Nope, we'll just rip it out and replace it all with the drm_noop ioctl. At > least I didn't find any userspace anywhere and only broken definitions. > Proof enough this isn't useful. > > Even more so since that means we don't have to convert the get ioctl over > to atomic, yay! I'll send patches. Can we just skip the noop part and delete it altogether? It was only added speculatively and obviously never used at all...
[PATCH] drm/i915: fix definition of the DRM_IOCTL_I915_GET_SPRITE_COLORKEY ioctl
On 03/26/2015 11:39 PM, Jani Nikula wrote:
> On Thu, 26 Mar 2015, Tommi Rantala wrote:
>> Fix definition of the DRM_IOCTL_I915_GET_SPRITE_COLORKEY ioctl, so that it
>> is different from the DRM_IOCTL_I915_SET_SPRITE_COLORKEY ioctl.
>>
>> Signed-off-by: Tommi Rantala
>
> Whoa. Broken since its introduction in
>
> commit 8ea30864229e54b01ac0e9fe88c4b733a940ec4e
> Author: Jesse Barnes
> Date: Tue Jan 3 08:05:39 2012 -0800
>
> drm/i915: add color key support v4
>
> Cc: stable at vger.kernel.org
>
> BR,
> Jani.
>
>
>> ---
>> include/uapi/drm/i915_drm.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index 6eed16b..0e9c835 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -270,7 +270,7 @@ typedef struct _drm_i915_sarea {
>> #define DRM_IOCTL_I915_OVERLAY_PUT_IMAGEDRM_IOW(DRM_COMMAND_BASE +
>> DRM_I915_OVERLAY_PUT_IMAGE, struct drm_intel_overlay_put_image)
>> #define DRM_IOCTL_I915_OVERLAY_ATTRSDRM_IOWR(DRM_COMMAND_BASE +
>> DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs)
>> #define DRM_IOCTL_I915_SET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE +
>> DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
>> -#define DRM_IOCTL_I915_GET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE +
>> DRM_I915_SET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
>> +#define DRM_IOCTL_I915_GET_SPRITE_COLORKEY DRM_IOWR(DRM_COMMAND_BASE +
>> DRM_I915_GET_SPRITE_COLORKEY, struct drm_intel_sprite_colorkey)
>> #define DRM_IOCTL_I915_GEM_WAIT DRM_IOWR(DRM_COMMAND_BASE +
>> DRM_I915_GEM_WAIT, struct drm_i915_gem_wait)
>> #define DRM_IOCTL_I915_GEM_CONTEXT_CREATE DRM_IOWR (DRM_COMMAND_BASE +
>> DRM_I915_GEM_CONTEXT_CREATE, struct drm_i915_gem_context_create)
>> #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE +
>> DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy)
Hah, I guess that must be a well used ioctl! Not even the test program
does a get call!
Jesse
[Intel-gfx] [PATCH 2/4] drm/cache: Try to be smarter about clflushing on x86
On 12/14/2014 4:59 AM, Chris Wilson wrote: > One of the things wbinvd is considered evil for is that it blocks the > CPU for an indeterminate amount of time - upsetting latency critcial > aspects of the OS. For example, the x86/mm has similar code to use > wbinvd for large clflushes that caused a bit of controversy with RT: > > http://linux-kernel.2935.n7.nabble.com/PATCH-x86-Use-clflush-instead-of-wbinvd-whenever-possible-when-changing-mapping-td493751.html > > and also the use of wbinvd in the nvidia driver has also been noted as > evil by RT folks. > > However as the wbinvd still exists, it can't be all that bad... Yeah there are definitely tradeoffs here. In this particular case, we're trying to flush out a ~140M object on every frame, which just seems silly. There's definitely room for optimization in Mesa too; avoiding a mapping that marks a large bo as dirty would be good, but if we improve the kernel in this area, even sloppy apps and existing binaries will speed up. Maybe we could apply this only on !llc systems or something? I wonder how much wbinvd performance varies across microarchitectures; maybe Thomas's issue isn't really relevant anymore (at least one can hope). When digging into this, I found that an optimization to remove the IPI for wbinvd was clobbered during a merge; maybe that should be resurrected too. Surely a single, global wbinvd is sufficient; we don't need to do n_cpus^2 wbinvd + the associated invalidation bus signals here... Alternately, we could insert some delays into this path just to make it extra clear to userspace that they really shouldn't be hitting this in the common case (and provide some additional interfaces to let them avoid it by allowing flushing and dirty management in userspace). Jesse
FOSDEM15: Graphics DevRoom: call for speakers.
On Tue, 9 Dec 2014 15:39:26 +0100 Luc Verhaegen wrote: > On Thu, Oct 02, 2014 at 07:44:57PM +0200, Luc Verhaegen wrote: > > Hi, > > > > At FOSDEM on the 31st of january and the 1st of February 2015, there > > will be another graphics DevRoom. URL: https://fosdem.org/2015/ > > > Slots will be handed out on a first come, first serve basis. The best > > slots will go to those who apply the earliest. The amount of slots is > > currently not known yet, but i expect there to be around 16 available (8 > > on each day), so act quickly. > > > As for deadlines, i hope to have a pretty much complete schedule between > > christmas and the new year. The rockhard printed schedule deadline is > > probably January 9th, after that you will not be featured in the booklet > > and you will have a lot less visitors. I will hopefully be able to lock > > down entries and descriptions after that date. > > It's been more than 2 months since the original email, it's less than > two months away from the event, and one month away from what usually is > the deadline for the booklet. File your talk now, while there are still > some useful slots available. > > Also, for those who have filed already but who have left their abstracts > open, please get those filed in ASAP. Your talk will be only be ordered > in when at least the basics are provided. Hey Luc, thanks for the reminder. I just submitted a talk myself; hopefully there's still room on the schedule! :) -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [RFC] dpms handling on atomic drivers
On Thu, 6 Nov 2014 19:35:51 -0500 Rob Clark wrote: > On Thu, Nov 6, 2014 at 6:29 PM, Daniel Vetter wrote: > > > > That aside I guess I need to elaborate on what makes dpms special in > > i915, and why there's a real difference between crtc->enable == true > > && ->active == false and crtc->enable == false in i915. For complex > > configs we do resource checking (shared dplls) and that's done in > > the modeset. For a pipe which has been disabled just with dpms we > > then guarantee that we'll keep these resources reserve and so will > > always be able to enable the pipe again. If you disable the pipe > > completely (i.e. set crtc->enable to false) we'll release these > > resources. E.g. in the i915 share dpll code we have both an active > > refcount (does the ppl need to be running) and a reference mask > > (which crtc is referencing this pll, even when the crtc is disabled > > with dpms). > > ahh, ok, "reserved but not enabled" makes a lot more sense.. that was > the distinction that I was missing. That probably deserves to be in > headerdoc somewhere.. A rename would be nice too; it's very misleading. Though with a move to a boolean DPMS internal state, it should be possible to drop it and just re-alloc all the resources on DPMS on (iow treat both DPMS off and on as mode sets). But that's not something that should block these changes by any means. Jesse
[Intel-gfx] [PATCH] drm/i915: don't try using training pattern 3 on pre-haswell
On Wed, 29 Oct 2014 11:07:32 +0200
Jani Nikula wrote:
> On Wed, 29 Oct 2014, Ville Syrjälä wrote:
> > On Wed, Oct 29, 2014 at 10:23:50AM +0200, Jani Nikula wrote:
> >> On Wed, 29 Oct 2014, Ville Syrjälä
> >> wrote:
> >> > On Wed, Oct 29, 2014 at 05:02:50PM +1000, Dave Airlie wrote:
> >> >> From: Dave Airlie
> >> >>
> >> >> Ivybridge + 30" monitor prints a drm error on every modeset, since
> >> >> IVB doesn't support DP3 we should even bother trying to use it.
> >> >>
> >> >> Reviewed-by: Daniel Vetter (on irc)
> >> >> Signed-off-by: Dave Airlie
> >> >> ---
> >> >> drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> >> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >> >> b/drivers/gpu/drm/i915/intel_dp.c
> >> >> index f6a3fdd..87cfb92 100644
> >> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> >> @@ -3547,13 +3547,15 @@ intel_dp_start_link_train(struct intel_dp
> >> >> *intel_dp)
> >> >> void
> >> >> intel_dp_complete_link_train(struct intel_dp *intel_dp)
> >> >> {
> >> >> + struct drm_encoder *encoder =
> >> >> _to_dig_port(intel_dp)->base.base;
> >> >> + struct drm_device *dev = encoder->dev;
> >> >> bool channel_eq = false;
> >> >> int tries, cr_tries;
> >> >> uint32_t DP = intel_dp->DP;
> >> >> uint32_t training_pattern = DP_TRAINING_PATTERN_2;
> >> >>
> >> >> /* Training Pattern 3 for HBR2 ot 1.2 devices that support it*/
> >> >> - if (intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3)
> >> >> + if ((intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3)
> >> >> && !HAS_DDI(dev))
> >> >
> >> > CHV has pattern 3.
> >>
> >> Is "supports tps3" the same set of platforms as "supports 5.4 Gbps"? We
> >> should abstract the check from intel_dp_max_link_bw.
> >
> > Not quite. HSW-ULX supports pattern 3 even though it doesn't do 5.4 Gbps.
>
> How about [1] instead? I forgot --in-reply-to, sorry.
>
> BR,
> Jani.
>
>
> [1] http://mid.gmane.org/1414573406-17071-1-git-send-email-jani.nikula at
> intel.com
Looks like we need something like that at least, assuming we're not
hitting the link_bw == DP_LINK_BW_5_4 case.
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] v3.17, i915 vs nouveau: possible recursive locking detected
On Thu, 16 Oct 2014 13:47:38 +0200 Daniel Vetter wrote: > We need ww mutexes and need to rewrite i915 a bit fo fix this all. > I.e. known issue. As long as your userspace isn't nasty nothing bad > will ever happen though. So do we already have a bug open with a good description of the issue? Eventually the streets will run with blood and we'll enter the 6th republic of i915, so it would be good to have it on the hit list for when the time comes... Thanks, Jesse
[Intel-gfx] [PATCH] drm: Implement O_NONBLOCK support on /dev/dri/cardN
On Tue, 7 Oct 2014 14:13:51 +0100
Chris Wilson wrote:
> The implmentation is simple in the extreme: we only want to wait for
> events if the device was opened in blocking mode, otherwise we grab
> what is available and report an error if there was none.
>
> Signed-off-by: Chris Wilson
> Cc: dri-devel at lists.freedesktop.org
> ---
> drivers/gpu/drm/drm_fops.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index ed7bc68f7e87..91e1105f2800 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -515,10 +515,12 @@ ssize_t drm_read(struct file *filp, char __user
> *buffer, size_t total;
> ssize_t ret;
>
> - ret = wait_event_interruptible(file_priv->event_wait,
> -!list_empty(_priv->event_list));
> - if (ret < 0)
> - return ret;
> + if ((filp->f_flags & O_NONBLOCK) == 0) {
> + ret = wait_event_interruptible(file_priv->event_wait,
> +
> !list_empty(_priv->event_list));
> + if (ret < 0)
> + return ret;
> + }
>
> total = 0;
> while (drm_dequeue_event(file_priv, total, count, )) {
> @@ -532,7 +534,7 @@ ssize_t drm_read(struct file *filp, char __user
> *buffer, e->destroy(e);
> }
>
> - return total;
> + return total ?: -EAGAIN;
> }
> EXPORT_SYMBOL(drm_read);
I'd prefer "total" to be spelled out after the ? (is this just a GNU
thing or does recent C implicitly use the first operand too?), but
that's no biggie. Looks fine.
Reviewed-by: Jesse Barnes
Question on UAPI for fences
On Fri, 12 Sep 2014 18:08:23 +0200 Christian K?nig wrote: > > As Daniel said using fd is most likely the way we want to do it but this > > remains vague. > Separating the discussion if it should be an fd or not. Using an fd > sounds fine to me in general, but I have some concerns as well. > > For example what was the maximum number of opened FDs per process again? > Could that become a problem? etc... You can check out the i915 patches I posted if you want to see examples. Max fds may be an issue if userspace doesn't clean up its fences. The implementation is pretty easy with the stuff Maarten has done recently. The changes I still need to make to mine: - sit on top of Chris's request/seqno changes (driver internals really) - switch over to execbuf as the main API on the render side (like you're doing) - add support for display and other timelines As far as compat goes, I don't think it should be too hard. Even with GPU scheduling, a given context's buffers should all be in-order with respect to one another, so we ought to be able to mix & match clients using explicit fencing and implicit fencing. Though in Mesa I still haven't looked at how to handle server vs client side arb_sync with the scheduler and explicit fencing in place; might need some extra work there... -- Jesse Barnes, Intel Open Source Technology Center
GEM memory DOS (WAS Re: [PATCH 3/3] drm/ttm: under memory pressure minimize the size of memory pool)
On Wed, 13 Aug 2014 17:13:56 +0200 Thomas Hellstrom wrote: > On 08/13/2014 03:01 PM, Daniel Vetter wrote: > > On Wed, Aug 13, 2014 at 02:35:52PM +0200, Thomas Hellstrom wrote: > >> On 08/13/2014 12:42 PM, Daniel Vetter wrote: > >>> On Wed, Aug 13, 2014 at 11:06:25AM +0200, Thomas Hellstrom wrote: > >>>> On 08/13/2014 05:52 AM, J?r?me Glisse wrote: > >>>>> From: J?r?me Glisse > >>>>> > >>>>> When experiencing memory pressure we want to minimize pool size so that > >>>>> memory we just shrinked is not added back again just as the next thing. > >>>>> > >>>>> This will divide by 2 the maximum pool size for each device each time > >>>>> the pool have to shrink. The limit is bumped again is next allocation > >>>>> happen after one second since the last shrink. The one second delay is > >>>>> obviously an arbitrary choice. > >>>> J?r?me, > >>>> > >>>> I don't like this patch. It adds extra complexity and its usefulness is > >>>> highly questionable. > >>>> There are a number of caches in the system, and if all of them added > >>>> some sort of voluntary shrink heuristics like this, we'd end up with > >>>> impossible-to-debug unpredictable performance issues. > >>>> > >>>> We should let the memory subsystem decide when to reclaim pages from > >>>> caches and what caches to reclaim them from. > >>> Yeah, artificially limiting your cache from growing when your shrinker > >>> gets called will just break the equal-memory pressure the core mm uses to > >>> rebalance between all caches when workload changes. In i915 we let > >>> everything grow without artificial bounds and only rely upon the shrinker > >>> callbacks to ensure we don't consume more than our fair share of available > >>> memory overall. > >>> -Daniel > >> Now when you bring i915 memory usage up, Daniel, > >> I can't refrain from bringing up the old user-space unreclaimable kernel > >> memory issue, for which gem open is a good example ;) Each time > >> user-space opens a gem handle, some un-reclaimable kernel memory is > >> allocated, for which there is no accounting, so theoretically I think a > >> user can bring a system to unusability this way. > >> > >> Typically there are various limits on unreclaimable objects like this, > >> like open file descriptors, and IIRC the kernel even has an internal > >> limit on the number of struct files you initialize, based on the > >> available system memory, so dma-buf / prime should already have some > >> sort of protection. > > Oh yeah, we have zero cgroups limits or similar stuff for gem allocations, > > so there's not really a way to isolate gpu memory usage in a sane way for > > specific processes. But there's also zero limits on actual gpu usage > > itself (timeslices or whatever) so I guess no one asked for this yet. > > In its simplest form (like in TTM if correctly implemented by drivers) > this type of accounting stops non-privileged malicious GPU-users from > exhausting all system physical memory causing grief for other kernel > systems but not from causing grief for other GPU users. I think that's > the minimum level that's intended also for example also for the struct > file accounting. > > > My comment really was about balancing mm users under the assumption that > > they're all unlimited. > > Yeah, sorry for stealing the thread. I usually bring this up now and > again but nowadays with an exponential backoff. Yeah I agree we're missing some good limits stuff in i915 and DRM in general probably. Chris started looking at this awhile back, but I haven't seen anything recently. Tying into the ulimits/rlimits might make sense, and at the very least we need to account for things properly so we can add new limits where needed. -- Jesse Barnes, Intel Open Source Technology Center
[PATCH 00/83] AMD HSA kernel driver
On Sun, 13 Jul 2014 12:40:32 -0400 j.glisse at gmail.com (Jerome Glisse) wrote: > On Sun, Jul 13, 2014 at 11:42:58AM +0200, Daniel Vetter wrote: > > On Sat, Jul 12, 2014 at 6:49 PM, Jerome Glisse > > wrote: > > >> Hm, so the hsa part is a completely new driver/subsystem, not just an > > >> additional ioctl tacked onto radoen? The history of drm is littered with > > >> "generic" ioctls that turned out to be useful for exactly one driver. > > >> Which is why _all_ the command submission is now done with driver-private > > >> ioctls. > > >> > > >> I'd be quite a bit surprised if that suddenly works differently, so > > >> before > > >> we bless a generic hsa interface I really want to see some implementation > > >> from a different vendor (i.e. nvdidia or intel) using the same ioctls. > > >> Otherwise we just repeat history and I'm not terribly inclined to keep on > > >> cleanup up cruft forever - one drm legacy is enough ;-) > > >> > > >> Jesse is the guy from our side to talk to about this. > > >> -Daniel > > > > > > I am not worried about that side, the hsa foundation has pretty strict > > > guidelines on what is hsa compliant hardware ie the hw needs to understand > > > the pm4 packet format of radeon (well small subset of it). But of course > > > this require hsa compliant hardware and from member i am guessing ARM > > > Mali, > > > ImgTech, Qualcomm, ... so unless Intel and NVidia joins hsa you will not > > > see it for those hardware. > > > > > > So yes for once same ioctl would apply to different hardware. The only > > > things > > > that is different is the shader isa. The hsafoundation site has some pdf > > > explaining all that but someone thought that slideshare would be a good > > > idea > > > personnaly i would not register to any of the website just to get the pdf. > > > > > > So to sumup i am ok with having a new device file that present uniform set > > > of ioctl. It would actualy be lot easier for userspace, just open this fix > > > device file and ask for list of compliant hardware. > > > > > > Then radeon kernel driver would register itself as a provider. So all > > > ioctl > > > decoding marshalling would be share which makes sense. > > > > There's also the other side namely that preparing the cp ring in > > userspace and submitting the entire pile through a doorbell to the hw > > scheduler isn't really hsa exclusive. And for a solid platform with > > seamless gpu/cpu integration that means we need standard ways to set > > gpu context priorities and get at useful stats like gpu time used by a > > given context. > > > > To get there I guess intel/nvidia need to reuse the hsa subsystem with > > the command submission adjusted a bit. Kinda like drm where kms and > > buffer sharing is common and cs driver specific. > > HSA module would be for HSA compliant hardware and thus hardware would > need to follow HSA specification which again is pretty clear on what > the hardware need to provide. So if Intel and NVidia wants to join HSA > i am sure they would be welcome, the more the merrier :) > > So i would not block HSA kernel ioctl design in order to please non HSA > hardware especialy if at this point in time nor Intel or NVidia can > share anything concret on the design and how this things should be setup > for there hardware. > > When Intel or NVidia present their own API they should provide their > own set of ioctl through their own platform. Yeah things are different enough that a uniform ioctl doesn't make sense. If/when all the vendors decide on a single standard, we can use that, but until then I don't see a nice way to share our doorbell & submission scheme with HSA, and I assume nvidia is the same. Using HSA as a basis for non-HSA systems seems like it would add a lot of complexity, since non-HSA hardware would have to intercept the queue writes and manage the submission requests etc as bytecodes in the kernel driver, or maybe as a shim layer library that wraps that stuff. Probably not worth the effort given that the command sets themselves are all custom as well, driven by specific user level drivers like GL, CL, and libva. -- Jesse Barnes, Intel Open Source Technology Center
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, 21 Jul 2014 19:05:46 +0200 daniel at ffwll.ch (Daniel Vetter) wrote: > On Mon, Jul 21, 2014 at 11:58:52AM -0400, Jerome Glisse wrote: > > On Mon, Jul 21, 2014 at 05:25:11PM +0200, Daniel Vetter wrote: > > > On Mon, Jul 21, 2014 at 03:39:09PM +0200, Christian K?nig wrote: > > > > Am 21.07.2014 14:36, schrieb Oded Gabbay: > > > > >On 20/07/14 20:46, Jerome Glisse wrote: [snip!!] > > > > > > > > The main questions here are if it's avoid able to pin down the memory > > > > and if > > > > the memory is pinned down at driver load, by request from userspace or > > > > by > > > > anything else. > > > > > > > > As far as I can see only the "mqd per userspace queue" might be a bit > > > > questionable, everything else sounds reasonable. > > > > > > Aside, i915 perspective again (i.e. how we solved this): When scheduling > > > away from contexts we unpin them and put them into the lru. And in the > > > shrinker we have a last-ditch callback to switch to a default context > > > (since you can't ever have no context once you've started) which means we > > > can evict any context object if it's getting in the way. > > > > So Intel hardware report through some interrupt or some channel when it is > > not using a context ? ie kernel side get notification when some user context > > is done executing ? > > Yes, as long as we do the scheduling with the cpu we get interrupts for > context switches. The mechanic is already published in the execlist > patches currently floating around. We get a special context switch > interrupt. > > But we have this unpin logic already on the current code where we switch > contexts through in-line cs commands from the kernel. There we obviously > use the normal batch completion events. Yeah and we can continue that going forward. And of course if your hw can do page faulting, you don't need to pin the normal data buffers. Usually there are some special buffers that need to be pinned for longer periods though, anytime the context could be active. Sounds like in this case the userland queues, which makes some sense. But maybe for smaller systems the size limit could be clamped to something smaller than 128M. Or tie it into the rlimit somehow, just like we do for mlock() stuff. > > The issue with radeon hardware AFAICT is that the hardware do not report any > > thing about the userspace context running ie you do not get notification > > when > > a context is not use. Well AFAICT. Maybe hardware do provide that. > > I'm not sure whether we can do the same trick with the hw scheduler. But > then unpinning hw contexts will drain the pipeline anyway, so I guess we > can just stop feeding the hw scheduler until it runs dry. And then unpin > and evict. Yeah we should have an idea which contexts have been fed to the scheduler, at least with kernel based submission. With userspace submission we'll be in a tougher spot... but as you say we can always idle things and unpin everything under pressure. That's a really big hammer to apply though. > > Like the VMID is a limited resources so you have to dynamicly bind them so > > maybe we can only allocate pinned buffer for each VMID and then when binding > > a PASID to a VMID it also copy back pinned buffer to pasid unpinned copy. > > Yeah, pasid assignment will be fun. Not sure whether Jesse's patches will > do this already. We _do_ already have fun with ctx id assigments though > since we move them around (and the hw id is the ggtt address afaik). So we > need to remap them already. Not sure on the details for pasid mapping, > iirc it's a separate field somewhere in the context struct. Jesse knows > the details. The PASID space is a bit bigger, 20 bits iirc. So we probably won't run out quickly or often. But when we do I thought we could apply the same trick Linux uses for ASID management on SPARC and ia64 (iirc on sparc anyway, maybe MIPS too): "allocate" a PASID everytime you need one, but don't tie it to the process at all, just use it as a counter that lets you know when you need to do a full TLB flush, then start the allocation process over. This lets you minimize TLB flushing and gracefully handles oversubscription. My current code doesn't bother though; context creation will fail if we run out of PASIDs on a given device. -- Jesse Barnes, Intel Open Source Technology Center
[Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences
On Wed, 23 Jul 2014 11:47:15 +0200 Daniel Vetter wrote: > On Tue, Jul 22, 2014 at 9:14 PM, Jesse Barnes > wrote: > >> We don't have the code yet ready, but that's the direction i915 will > >> move towards for the near future. Jesse is working on some patches > >> already. > > > > Yeah I'd like to get some feedback from Maarten on my bits so I can get > > them ready for upstream. I still need to add documentation and tests, > > but I'd like to make sure the interfaces and internals get acked first. > > Review works better if you supply a pointer to the patches ;-) I asked > Maarten whether he looked at it and he said he didn't know where ... Oh I provided it in IRC earlier, figured Maarten was just busy. :) Tree is android-dma-buf-i915-fences in my fdo linux repo. -- Jesse Barnes, Intel Open Source Technology Center
[PATCH 09/17] drm/radeon: use common fence implementation for fences
On Tue, 22 Jul 2014 17:48:18 +0200 Daniel Vetter wrote: > On Tue, Jul 22, 2014 at 5:42 PM, Alex Deucher > wrote: > >> Fence-based syncing between userspace queues submitted stuff through > >> doorbells and anything submitted by the general simply wont work. > >> Which is why I think the doorbell is a stupid interface since I just > >> don't see cameras and v4l devices implementing all that complexity to > >> get a pure userspace side sync solution. > >> > > > > Like it or not this is what a lot of application writers want (look at > > mantle and metal and similar new APIs or android synpts). Having > > queues and fences in userspace allows the application to structure > > things to best fit their own task graphs. The app can decide how to > > deal with dependencies and synchronization explicitly instead of > > blocking the queues in the kernel for everyone. Anyway, this is > > getting off topic. > > Well there's explicit fences as used in opencl and android syncpts. My > plan is actually to support that in i915 using Maarten's struct fence > stuff (and there's just a very trivial patch for the android stuff in > merging needed to get there). What doesn't work is fences created > behind the kernel's back purely in userspace by giving shared memory > locations special meaning. Those get the kernel completely out of the > picture (as opposed to android syncpts, which just make sync > explicit). > > I guess long-term we might need something like gpu futexes to make > that pure userspace syncing integrate a bit better, but imo that's (at > least for now) out of scope. For fences here I have the goal of one Yeah, with a little kernel help you could have a mostly kernel independent sync mechanism using just shared mem in userspace. The kernel would just need to signal any interested clients when something happened (even if it didn't know what) and let userspace sort out the rest. I think that would be a nice thing to provide at some point, as it could allow for some fine grained CPU/GPU algorithms that use lightweight synchronization with and without busy looping on the CPU side. But all of that is definitely a lower priority than getting explicit fencing exported to userspace to work right, both for intra-driver sync and inter-driver sync. > internally representation used by both implicit syncing (dma-buf on > classic linux, e.g. prime) and explicit fencing on android or opencl > or something like that. > > We don't have the code yet ready, but that's the direction i915 will > move towards for the near future. Jesse is working on some patches > already. Yeah I'd like to get some feedback from Maarten on my bits so I can get them ready for upstream. I still need to add documentation and tests, but I'd like to make sure the interfaces and internals get acked first. Thanks, -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 1/3] drm/crtc: Add property for aspect ratio
Daniel, looks like this series has some r-bs; iirc this fixed some Asus
HDMI monitors too (and who knows how many TVs).
Jesse
On Thu, 22 May 2014 16:50:48 +0530
Vandana Kannan wrote:
> Added a property to enable user space to set aspect ratio.
> This patch contains declaration of the property and code to create the
> property.
>
> Signed-off-by: Vandana Kannan
> Cc: dri-devel at lists.freedesktop.org
> ---
> drivers/gpu/drm/drm_crtc.c | 31 +++
> include/drm/drm_crtc.h | 2 ++
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 37a3e07..84d359e 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -139,6 +139,12 @@ static const struct drm_prop_enum_list
> drm_scaling_mode_enum_list[] =
> { DRM_MODE_SCALE_ASPECT, "Full aspect" },
> };
>
> +static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
> + { HDMI_PICTURE_ASPECT_NONE, "Automatic" },
> + { HDMI_PICTURE_ASPECT_4_3, "4:3" },
> + { HDMI_PICTURE_ASPECT_16_9, "16:9" },
> +};
> +
> /*
> * Non-global properties, but "required" for certain connectors.
> */
> @@ -1344,6 +1350,31 @@ int drm_mode_create_scaling_mode_property(struct
> drm_device *dev)
> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>
> /**
> + * drm_mode_create_aspect_ratio_property - create aspect ratio property
> + * @dev: DRM device
> + *
> + * Called by a driver the first time it's needed, must be attached to desired
> + * connectors.
> + */
> +int drm_mode_create_aspect_ratio_property(struct drm_device *dev)
> +{
> + struct drm_property *aspect_ratio;
> +
> + if (dev->mode_config.aspect_ratio_property)
> + return 0;
> +
> + aspect_ratio =
> + drm_property_create_enum(dev, 0, "aspect ratio",
> + drm_aspect_ratio_enum_list,
> + ARRAY_SIZE(drm_aspect_ratio_enum_list));
> +
> + dev->mode_config.aspect_ratio_property = aspect_ratio;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
> +
> +/**
> * drm_mode_create_dirty_property - create dirty property
> * @dev: DRM device
> *
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5c1c31c..1149617 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -801,6 +801,7 @@ struct drm_mode_config {
>
> /* Optional properties */
> struct drm_property *scaling_mode_property;
> + struct drm_property *aspect_ratio_property;
> struct drm_property *dirty_info_property;
>
> /* dumb ioctl parameters */
> @@ -971,6 +972,7 @@ extern int drm_mode_create_dvi_i_properties(struct
> drm_device *dev);
> extern int drm_mode_create_tv_properties(struct drm_device *dev, int
> num_formats,
>char *formats[]);
> extern int drm_mode_create_scaling_mode_property(struct drm_device *dev);
> +extern int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
> extern int drm_mode_create_dirty_info_property(struct drm_device *dev);
> extern const char *drm_get_encoder_name(const struct drm_encoder *encoder);
>
--
Jesse Barnes, Intel Open Source Technology Center
[PATCH] drm: reduce default drm vblank off delay to 50ms
On Tue, 8 Jul 2014 15:56:04 +0200 Daniel Vetter wrote: > On Wed, Jul 02, 2014 at 01:42:38PM -0700, Jesse Barnes wrote: > > On Wed, 2 Jul 2014 13:35:19 -0700 > > St?phane Marchesin wrote: > > > > > On Tue, Oct 30, 2012 at 12:20 PM, Daniel Vetter > > > wrote: > > > > On Tue, Oct 30, 2012 at 8:09 PM, Jesse Barnes > > > virtuousgeek.org> wrote: > > > >> People keep whining about this, but no one seems to send a patch. This > > > >> *ought* to be safe now that we've dealt with the hw races in Mario's > > > >> updated code, and fixed the bugs we know about in VT switch, DPMS, and > > > >> multi-head configuraions. > > > >> > > > >> Signed-off-by: Jesse Barnes > > > > > > > > Afaik the fundamental race of enabling the vblank is still there, so > > > > this is just duct-tape. And our hw has the required registers (on > > > > gen5+ at least) to close this race for real and abolish all "disable > > > > vblank irq later to paper over races and smooth things out). Hence I > > > > think we should dtrt and so > > > > > > [digging an old thread] > > > > > > So I'm looking at this machine where we can't get good PSR residency > > > because the vblank_offdelay is so long. Therefore, I'm suddenly very > > > interested in solving this issue :) Of course I can't seem to find > > > logs of the fun IRC discussion you guys had, can you describe what the > > > race is, and also what are the registers you're talking about? > > > > Beyond that I don't see why this obvious and simple improvement should > > be blocked on some other work. Maybe it's a bit late now since Ville > > may already have patches for what Daniel mentions above, but I still > > find the nack to be totally misguided. > > > > Dave, please just pick this up so everyone can benefit while we thrash > > through an i915 fix (doubtless introducing some bugs) that lets us > > disable immediately. > > This needs an ack from Mario. > > And I really don't see why we _now_ need to suddenly rush then when we > have patches from Ville to address this properly. The blocker is only that > it's not yet reviewed but meh. > > And people with product ship dates looming over their head can always just > apply this themselves. > > Us sucking at reviewing is imo no reason at all to rush patches in. This is just the most recent version: http://lists.freedesktop.org/archives/dri-devel/2012-October/029648.html IIRC there was one posted back in 2010 too. So hardly rushing. -- Jesse Barnes, Intel Open Source Technology Center
[PATCH] drm: reduce default drm vblank off delay to 50ms
On Wed, 2 Jul 2014 13:35:19 -0700 St?phane Marchesin wrote: > On Tue, Oct 30, 2012 at 12:20 PM, Daniel Vetter wrote: > > On Tue, Oct 30, 2012 at 8:09 PM, Jesse Barnes > > wrote: > >> People keep whining about this, but no one seems to send a patch. This > >> *ought* to be safe now that we've dealt with the hw races in Mario's > >> updated code, and fixed the bugs we know about in VT switch, DPMS, and > >> multi-head configuraions. > >> > >> Signed-off-by: Jesse Barnes > > > > Afaik the fundamental race of enabling the vblank is still there, so > > this is just duct-tape. And our hw has the required registers (on > > gen5+ at least) to close this race for real and abolish all "disable > > vblank irq later to paper over races and smooth things out). Hence I > > think we should dtrt and so > > [digging an old thread] > > So I'm looking at this machine where we can't get good PSR residency > because the vblank_offdelay is so long. Therefore, I'm suddenly very > interested in solving this issue :) Of course I can't seem to find > logs of the fun IRC discussion you guys had, can you describe what the > race is, and also what are the registers you're talking about? Beyond that I don't see why this obvious and simple improvement should be blocked on some other work. Maybe it's a bit late now since Ville may already have patches for what Daniel mentions above, but I still find the nack to be totally misguided. Dave, please just pick this up so everyone can benefit while we thrash through an i915 fix (doubtless introducing some bugs) that lets us disable immediately. -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 0/9] drm: More vblank on/off work
On Mon, 26 May 2014 14:46:23 +0300 ville.syrjala at linux.intel.com wrote: > From: Ville Syrj?l? > > Another vblank series with the following features: > - Plug a race between drm_vblank_off() and marking the crtc inactive > - Don't send zeroed vblank evens to userspace at drm_vblank_off() > - Have the user visible vblank counter account the entire time > when the crtc was active, regardless of how long vblank interrupts > were enabled > - Avoid random jumps in the user visible vblank counter if the hardware > counter gets reset > - Allow disabling vblank interrupts immediately at drm_vblank_put() > - Some polish via coccinelle > > While setting drm_vblank_offdelay to 0 is now possible, I'm not sure > if we should set it 0 automatically in the i915 driver. If there are > multiple GPUs in the system that setting will affect them all, which > might have bad consequences if the other GPU doesn't have a hardware > frame counter, or if it's just buggy. So perhaps we should move that > option to be per-driver? > > Ville Syrj?l? (9): > drm: Always reject drm_vblank_get() after drm_vblank_off() > drm/i915: Warn if drm_vblank_get() still works after drm_vblank_off() > drm: Don't clear vblank timestamps when vblank interrupt is disabled > drm: Move drm_update_vblank_count() > drm: Have the vblank counter account for the time between vblank irq > disable and drm_vblank_off() > drm: Avoid random vblank counter jumps if the hardware counter has > been reset > drm: Disable vblank interrupt immediately when drm_vblank_offdelay==0 > drm: Reduce the amount of dev->vblank[crtc] in the code > drm/i915: Leave interrupts enabled while disabling crtcs during > suspend Here's one that may be fixed by this series, needs testing though: https://bugs.freedesktop.org/show_bug.cgi?id=79054 -- Jesse Barnes, Intel Open Source Technology Center
[PATCH 2/5] drm/i915: preserve swizzle settings if necessary v3
On Wed, 11 Jun 2014 17:39:29 +0200 Daniel Vetter wrote: > On Wed, Jun 11, 2014 at 5:13 PM, Jesse Barnes > wrote: > >> - If you have a machine which uses tiled framebuffers and enables > >> swizzling in the BIOS your code will a) drop the swizzle setup in > >> gem_init_hw, breaking resume b) not set the swizzle settings correctly > >> in swizzle_detect, breaking swap in/out and pwrite/pread. Not sure such > >> a machine exists, but still. > > > > This would affect krh's MBA, which is why I wanted testing here... > > anyway I'll spin a new one and ask krh to test again. > > Hm, I've thought the issue with the MBA is that it used tiled fbs, but > non-swizzled. And then a mess ensued when we've enabled it. But yeah, > unfortunately with the new logic we need to retest :( Ah yeah I think you're right, either way, need more testing. Maybe we should have just gone with the first patch to never enable swizzling based on Art's assertion that it didn't matter. -- Jesse Barnes, Intel Open Source Technology Center
[PATCH 2/5] drm/i915: preserve swizzle settings if necessary v3
On Wed, 11 Jun 2014 11:23:26 +0200
Daniel Vetter wrote:
> On Tue, Jun 10, 2014 at 12:45:38PM -0700, Jesse Barnes wrote:
> > On Tue, 10 Jun 2014 21:33:27 +0200
> > Daniel Vetter wrote:
> >
> > > On Tue, Jun 10, 2014 at 7:27 PM, Jesse Barnes > > virtuousgeek.org> wrote:
> > > > Yes, that's what I do below... I even added it to the changelog:
> > > > http://patchwork.freedesktop.org/patch/27223/
> > > >
> > > > Did you miss the later hunk in intel_display.c?
> > > >
> > > > What we try to do here is enable swizzling if possible, which we can do
> > > > if no inherited fbs are tiled.
> > > >
> > > > So I think I've done exactly what you repeated above, and documented
> > > > it. So you're going to need to repeat it with different words so I can
> > > > understand, if I'm still missing something.
> > >
> > > In swizzle_detect:
> > >
> > > ...
> > >
> > > if (GEN6+) {
> > > if (preserve_bios_swizzle) {
> > > if (I915_READ(DISP_ARB_CTL) & DISP_TILE_SURFACE_SWIZZLING) {
> > > swizzle_x = I915_BIT_6_SWIZZLE_9_10;
> > > ...
> > > } else {
> > > swizzle_x = I915_BIT_6_SWIZZLE_NONE;
> > > ...
> > > }
> > > } else {
> > > /* existing/old logic to decide about swizzling */
> > > }
> > > }
> > >
> > > ...
> > >
> > > Plus no shortcut in i915_gem_init_swizzling. Personally I'd also just use
> > > a small helper function to compute preserve_bios_swizzle instead of
> > > storing it in dev_priv (since we will only use it at exactly one place),
> > > but that's a pure style preference.
> >
> > Doesn't this amount to the same thing? I.e. we enable it if possible,
> > otherwise just report it as unswizzled? So you're just wanting a style
> > change here?
>
> So presuming I read your code correctly there's two issues:
>
> - The first thing you check in swizzle_detect is the hw swizzle bit in
> DSP_ARB. If that's not set you force unswizzled. Since most BIOS don't
> bother to set this (they use an untiled buffer after all) this means
> you've effectively disabled swizzling on almost all machines. The goal
> of keeping the old logic was that we actually want to enable swizzling
> in some situations.
Ah damn, I had been thinking that bit_6_swizzle was the runtime call,
and that the init_swizzle call would go ahead and set it up properly.
I see that's too late though.
> - If you have a machine which uses tiled framebuffers and enables
> swizzling in the BIOS your code will a) drop the swizzle setup in
> gem_init_hw, breaking resume b) not set the swizzle settings correctly
> in swizzle_detect, breaking swap in/out and pwrite/pread. Not sure such
> a machine exists, but still.
This would affect krh's MBA, which is why I wanted testing here...
anyway I'll spin a new one and ask krh to test again.
--
Jesse Barnes, Intel Open Source Technology Center
[PATCH 2/5] drm/i915: preserve swizzle settings if necessary v3
On Tue, 10 Jun 2014 21:33:27 +0200
Daniel Vetter wrote:
> On Tue, Jun 10, 2014 at 7:27 PM, Jesse Barnes
> wrote:
> > Yes, that's what I do below... I even added it to the changelog:
> > http://patchwork.freedesktop.org/patch/27223/
> >
> > Did you miss the later hunk in intel_display.c?
> >
> > What we try to do here is enable swizzling if possible, which we can do
> > if no inherited fbs are tiled.
> >
> > So I think I've done exactly what you repeated above, and documented
> > it. So you're going to need to repeat it with different words so I can
> > understand, if I'm still missing something.
>
> In swizzle_detect:
>
> ...
>
> if (GEN6+) {
> if (preserve_bios_swizzle) {
> if (I915_READ(DISP_ARB_CTL) & DISP_TILE_SURFACE_SWIZZLING) {
> swizzle_x = I915_BIT_6_SWIZZLE_9_10;
> ...
> } else {
> swizzle_x = I915_BIT_6_SWIZZLE_NONE;
> ...
> }
> } else {
> /* existing/old logic to decide about swizzling */
> }
> }
>
> ...
>
> Plus no shortcut in i915_gem_init_swizzling. Personally I'd also just use
> a small helper function to compute preserve_bios_swizzle instead of
> storing it in dev_priv (since we will only use it at exactly one place),
> but that's a pure style preference.
Doesn't this amount to the same thing? I.e. we enable it if possible,
otherwise just report it as unswizzled? So you're just wanting a style
change here?
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 5/5] drm/i915: enable fastboot by default
On Tue, 10 Jun 2014 11:01:06 -0700
St?phane Marchesin wrote:
> On Tue, Jun 10, 2014 at 10:31 AM, Jesse Barnes
> wrote:
> > On Tue, 10 Jun 2014 16:07:44 +0200
> > Daniel Vetter wrote:
> >
> >> On Thu, Jun 05, 2014 at 11:24:31AM -0700, Jesse Barnes wrote:
> >> > Let them eat mincemeat pie.
> >> >
> >> > Signed-off-by: Jesse Barnes
> >> > ---
> >> > drivers/gpu/drm/i915/i915_params.c | 4 ++--
> >> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_params.c
> >> > b/drivers/gpu/drm/i915/i915_params.c
> >> > index d05a2af..081ab2f 100644
> >> > --- a/drivers/gpu/drm/i915/i915_params.c
> >> > +++ b/drivers/gpu/drm/i915/i915_params.c
> >> > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = {
> >> > .preliminary_hw_support =
> >> > IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
> >> > .disable_power_well = 1,
> >> > .enable_ips = 1,
> >> > - .fastboot = 0,
> >> > + .fastboot = 42,
> >> > .prefault_disable = 0,
> >> > .reset = true,
> >> > .invert_brightness = 0,
> >> > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default:
> >> > true)");
> >> >
> >> > module_param_named(fastboot, i915.fastboot, bool, 0600);
> >> > MODULE_PARM_DESC(fastboot,
> >> > - "Try to skip unnecessary mode sets at boot time (default: false)");
> >> > + "Try to skip unnecessary mode sets at boot time (default: true)");
> >>
> >> Nah, that wasn't the intention of this option. It was meant as a hack to
> >> experiment around with fastboot and get things going, but imo we need to
> >> really do the full modeset and short-circuit if the state matches.
> >>
> >> And there's still a bunch of things we don't track like infoframes which
> >> we either need to fix up (similar to the pfit fixup) or quirk to disallow
> >> fastboot.
> >
> > Hm that contradicts our earlier discussions w/Damien when we decided
> > the infoframes stuff were too esoteric to matter...
>
> My 2 cents is that I've seen some really bad TVs which didn't work
> because infoframes were missing (IIRC it relied on the VIC to detect
> the video mode).
Yeah so we'd still leave them in place in this case, and apply them on
the next mode set as well, but we wouldn't be explicitly cross checking
for them, at least not yet.
It's a good thing to add, I just didn't think it was a blocker based on
our last discussion about this.
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 5/5] drm/i915: enable fastboot by default
On Tue, 10 Jun 2014 16:07:44 +0200
Daniel Vetter wrote:
> On Thu, Jun 05, 2014 at 11:24:31AM -0700, Jesse Barnes wrote:
> > Let them eat mincemeat pie.
> >
> > Signed-off-by: Jesse Barnes
> > ---
> > drivers/gpu/drm/i915/i915_params.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_params.c
> > b/drivers/gpu/drm/i915/i915_params.c
> > index d05a2af..081ab2f 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = {
> > .preliminary_hw_support =
> > IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
> > .disable_power_well = 1,
> > .enable_ips = 1,
> > - .fastboot = 0,
> > + .fastboot = 42,
> > .prefault_disable = 0,
> > .reset = true,
> > .invert_brightness = 0,
> > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default:
> > true)");
> >
> > module_param_named(fastboot, i915.fastboot, bool, 0600);
> > MODULE_PARM_DESC(fastboot,
> > - "Try to skip unnecessary mode sets at boot time (default: false)");
> > + "Try to skip unnecessary mode sets at boot time (default: true)");
>
> Nah, that wasn't the intention of this option. It was meant as a hack to
> experiment around with fastboot and get things going, but imo we need to
> really do the full modeset and short-circuit if the state matches.
>
> And there's still a bunch of things we don't track like infoframes which
> we either need to fix up (similar to the pfit fixup) or quirk to disallow
> fastboot.
Hm that contradicts our earlier discussions w/Damien when we decided
the infoframes stuff were too esoteric to matter...
Also do you want the mod_parm_desc updated to be more accurate? Not
sure what the request is here.
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 4/5] drm/i915: use current mode if the size matches the preferred mode
On Tue, 10 Jun 2014 16:05:36 +0200 Daniel Vetter wrote: > On Thu, Jun 05, 2014 at 11:24:30AM -0700, Jesse Barnes wrote: > > From: Kristian H?gsberg > > > > The BIOS may set a native mode that doesn't quite match the preferred > > mode timings. It should be ok to use however if it uses the same size, > > so try to avoid a mode set in that case. > > > > Signed-off-by: Kristian H?gsberg > > Signed-off-by: Jesse Barnes > > Not sure we want this since this seems to override the cmdline options to > force a specific edid. Also not sure whether we shouldn't just add this as > the preferred mode when probing (before the preferred mode the vbt/edid > provides ofc). > > What exactly is the mismatch here? It could be DRRS or something fancy, > too. > > Not sure what to do here really. AFAICT it's just slightly different timings for fun. I don't think they go low enough to reduce the DP lane count... maybe there's just a mismatch between their hard coded panel timings and the ones reported by the EDID. Not sure which to trust... Kristian can you post the timings you see here? Both the BIOS timings and the EDID ones? So I'm stuck here too, I think it's a rare case though. -- Jesse Barnes, Intel Open Source Technology Center
[PATCH 2/5] drm/i915: preserve swizzle settings if necessary v3
On Tue, 10 Jun 2014 16:02:51 +0200
Daniel Vetter wrote:
> On Thu, Jun 05, 2014 at 11:24:28AM -0700, Jesse Barnes wrote:
> > Some machines (like MBAs) might use a tiled framebuffer but not enable
> > display swizzling at boot time. We want to preserve that configuration
> > if possible to prevent a boot time mode set. On IVB+ it shouldn't
> > affect performance anyway since the memory controller does internal
> > swizzling anyway.
> >
> > For most other configs we'll be able to enable swizzling at boot time,
> > since the initial framebuffer won't be tiled, thus we won't see any
> > corruption when we enable it.
> >
> > v2: preserve swizzling if BIOS had it set (Daniel)
> > v3: preserve swizzling only if we inherited a tiled framebuffer (Daniel)
> > check display swizzle setting in detect_bit_6_swizzle (Daniel)
> > use gen6 as cutoff point (Daniel)
> >
> > Reported-by: Kristian H?gsberg
> > Signed-off-by: Jesse Barnes
> > ---
> > drivers/gpu/drm/i915/i915_drv.h| 1 +
> > drivers/gpu/drm/i915/i915_gem.c| 3 +++
> > drivers/gpu/drm/i915/i915_gem_tiling.c | 38
> > +++---
> > drivers/gpu/drm/i915/intel_display.c | 3 +++
> > 4 files changed, 28 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index f57b752..f49fdcb 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1405,6 +1405,7 @@ struct drm_i915_private {
> > struct intel_vbt_data vbt;
> >
> > bool bios_ssc; /* BIOS had SSC enabled at boot? */
> > + bool preserve_bios_swizzle;
> >
> > /* overlay */
> > struct intel_overlay *overlay;
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index bfc7af0..0b168fb 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4580,6 +4580,9 @@ void i915_gem_init_swizzling(struct drm_device *dev)
> > dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
> > return;
> >
> > + if (INTEL_INFO(dev)->gen >= 6 && dev_priv->preserve_bios_swizzle)
> > + return;
> > +
>
> Above two hunks shouldnt be needed if the setup in
> i915_gem_detect_bit_6_swizzle works correctly.
>
> > I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
> > DISP_TILE_SURFACE_SWIZZLING);
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
> > b/drivers/gpu/drm/i915/i915_gem_tiling.c
> > index cb150e8..73005ad 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> > @@ -91,26 +91,30 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
> > uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
> > uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
> >
> > - if (IS_VALLEYVIEW(dev)) {
> > - swizzle_x = I915_BIT_6_SWIZZLE_NONE;
> > - swizzle_y = I915_BIT_6_SWIZZLE_NONE;
> > - } else if (INTEL_INFO(dev)->gen >= 6) {
> > + if (INTEL_INFO(dev)->gen >= 6) {
> > uint32_t dimm_c0, dimm_c1;
> > - dimm_c0 = I915_READ(MAD_DIMM_C0);
> > - dimm_c1 = I915_READ(MAD_DIMM_C1);
> > - dimm_c0 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
> > - dimm_c1 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
> > - /* Enable swizzling when the channels are populated with
> > -* identically sized dimms. We don't need to check the 3rd
> > -* channel because no cpu with gpu attached ships in that
> > -* configuration. Also, swizzling only makes sense for 2
> > -* channels anyway. */
> > - if (dimm_c0 == dimm_c1) {
> > - swizzle_x = I915_BIT_6_SWIZZLE_9_10;
> > - swizzle_y = I915_BIT_6_SWIZZLE_9;
> > - } else {
> > +
> > + /* Make sure to honor boot time display configuration */
> > + if (!(I915_READ(DISP_ARB_CTL) & DISP_TILE_SURFACE_SWIZZLING)) {
>
> Not quite what I had in mind. Imo we need to check for whether any
> inherited fbs are tiled and if so also inherit the swizzle setting
> unconditionally, whether it is swizzled or unswizzled. See
>
> http://patchwork.freedesktop.org/patch/22204/
Yes, that's what I do below... I even added it to the changelog:
http://patchwork.freedesktop.org/patch/27223/
Did you miss the later hunk in intel_display.c?
What we try to do here is enable swizzling if possible, which we can do
if no inherited fbs are tiled.
So I think I've done exactly what you repeated above, and documented
it. So you're going to need to repeat it with different words so I can
understand, if I'm still missing something.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
[PATCH 5/5] drm/i915: enable fastboot by default
Let them eat mincemeat pie.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_params.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index d05a2af..081ab2f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = {
.preliminary_hw_support =
IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
.disable_power_well = 1,
.enable_ips = 1,
- .fastboot = 0,
+ .fastboot = 42,
.prefault_disable = 0,
.reset = true,
.invert_brightness = 0,
@@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
module_param_named(fastboot, i915.fastboot, bool, 0600);
MODULE_PARM_DESC(fastboot,
- "Try to skip unnecessary mode sets at boot time (default: false)");
+ "Try to skip unnecessary mode sets at boot time (default: true)");
module_param_named(prefault_disable, i915.prefault_disable, bool, 0600);
MODULE_PARM_DESC(prefault_disable,
--
1.8.3.2
[PATCH 4/5] drm/i915: use current mode if the size matches the preferred mode
From: Kristian H?gsberg <hoegsb...@gmail.com>
The BIOS may set a native mode that doesn't quite match the preferred
mode timings. It should be ok to use however if it uses the same size,
so try to avoid a mode set in that case.
Signed-off-by: Kristian H?gsberg
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/intel_fbdev.c | 47 +++---
1 file changed, 18 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index 088fe93..09819ae 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -379,42 +379,31 @@ static bool intel_fb_initial_config(struct drm_fb_helper
*fb_helper,
/* go for command line mode first */
modes[i] = drm_pick_cmdline_mode(fb_conn, width, height);
- /* try for preferred next */
+ /* try for preferred next or match current */
if (!modes[i]) {
- DRM_DEBUG_KMS("looking for preferred mode on connector
%s\n",
- connector->name);
- modes[i] = drm_has_preferred_mode(fb_conn, width,
- height);
- }
+ struct drm_display_mode *preferred;
- /* No preferred mode marked by the EDID? Are there any modes? */
- if (!modes[i] && !list_empty(>modes)) {
- DRM_DEBUG_KMS("using first mode listed on connector
%s\n",
+ DRM_DEBUG_KMS("looking for preferred mode on connector
%s\n",
connector->name);
- modes[i] = list_first_entry(>modes,
- struct drm_display_mode,
- head);
- }
+ preferred = drm_has_preferred_mode(fb_conn, width,
+ height);
- /* last resort: use current mode */
- if (!modes[i]) {
- /*
-* IMPORTANT: We want to use the adjusted mode (i.e.
-* after the panel fitter upscaling) as the initial
-* config, not the input mode, which is what crtc->mode
-* usually contains. But since our current fastboot
-* code puts a mode derived from the post-pfit timings
-* into crtc->mode this works out correctly. We don't
-* use hwmode anywhere right now, so use it for this
-* since the fb helper layer wants a pointer to
-* something we own.
-*/
- DRM_DEBUG_KMS("looking for current mode on connector
%s\n",
- connector->name);
intel_mode_from_pipe_config(>crtc->hwmode,
_intel_crtc(encoder->crtc)->config);
- modes[i] = >crtc->hwmode;
+ modes[i] = >crtc->hwmode;
+
+ if (preferred &&
+ !drm_mode_same_size(preferred, modes[i])) {
+ DRM_DEBUG_KMS("using preferred mode %s "
+ "instead of current mode %s "
+ "on connector %d\n",
+ preferred->name,
+ modes[i]->name,
+ fb_conn->connector->base.id);
+ modes[i] = preferred;
+ }
}
+
crtcs[i] = new_crtc;
DRM_DEBUG_KMS("connector %s on pipe %d [CRTC:%d]: %dx%d%s\n",
--
1.8.3.2
[PATCH 3/5] drm: add drm_mode_same_size function
From: Kristian H?gsberg <hoegsb...@gmail.com>
Like mode_equal but w/o the clock checks. Useful for checking if modes
are close enough to re-use to avoid a boot time mode set for example.
Signed-off-by: Kristian H?gsberg
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_modes.c | 8
include/drm/drm_modes.h | 2 ++
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index bedf189..92ae7f3 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -905,6 +905,14 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct
drm_display_mode *mode1,
}
EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
+bool drm_mode_same_size(const struct drm_display_mode *mode1,
+ const struct drm_display_mode *mode2)
+{
+ return mode1->vdisplay == mode2->vdisplay &&
+ mode1->hdisplay == mode2->hdisplay;
+}
+EXPORT_SYMBOL(drm_mode_same_size);
+
/**
* drm_mode_validate_size - make sure modes adhere to size constraints
* @dev: DRM device
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 91d0582..7272309 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -215,6 +215,8 @@ bool drm_mode_equal(const struct drm_display_mode *mode1,
const struct drm_display_mode *mode2);
bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
const struct drm_display_mode *mode2);
+bool drm_mode_same_size(const struct drm_display_mode *mode1,
+ const struct drm_display_mode *mode2);
/* for use by the crtc helper probe functions */
void drm_mode_validate_size(struct drm_device *dev,
--
1.8.3.2
[PATCH 2/5] drm/i915: preserve swizzle settings if necessary v3
Some machines (like MBAs) might use a tiled framebuffer but not enable
display swizzling at boot time. We want to preserve that configuration
if possible to prevent a boot time mode set. On IVB+ it shouldn't
affect performance anyway since the memory controller does internal
swizzling anyway.
For most other configs we'll be able to enable swizzling at boot time,
since the initial framebuffer won't be tiled, thus we won't see any
corruption when we enable it.
v2: preserve swizzling if BIOS had it set (Daniel)
v3: preserve swizzling only if we inherited a tiled framebuffer (Daniel)
check display swizzle setting in detect_bit_6_swizzle (Daniel)
use gen6 as cutoff point (Daniel)
Reported-by: Kristian H?gsberg
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_drv.h| 1 +
drivers/gpu/drm/i915/i915_gem.c| 3 +++
drivers/gpu/drm/i915/i915_gem_tiling.c | 38 +++---
drivers/gpu/drm/i915/intel_display.c | 3 +++
4 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f57b752..f49fdcb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1405,6 +1405,7 @@ struct drm_i915_private {
struct intel_vbt_data vbt;
bool bios_ssc; /* BIOS had SSC enabled at boot? */
+ bool preserve_bios_swizzle;
/* overlay */
struct intel_overlay *overlay;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bfc7af0..0b168fb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4580,6 +4580,9 @@ void i915_gem_init_swizzling(struct drm_device *dev)
dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
return;
+ if (INTEL_INFO(dev)->gen >= 6 && dev_priv->preserve_bios_swizzle)
+ return;
+
I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
DISP_TILE_SURFACE_SWIZZLING);
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index cb150e8..73005ad 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -91,26 +91,30 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
- if (IS_VALLEYVIEW(dev)) {
- swizzle_x = I915_BIT_6_SWIZZLE_NONE;
- swizzle_y = I915_BIT_6_SWIZZLE_NONE;
- } else if (INTEL_INFO(dev)->gen >= 6) {
+ if (INTEL_INFO(dev)->gen >= 6) {
uint32_t dimm_c0, dimm_c1;
- dimm_c0 = I915_READ(MAD_DIMM_C0);
- dimm_c1 = I915_READ(MAD_DIMM_C1);
- dimm_c0 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
- dimm_c1 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
- /* Enable swizzling when the channels are populated with
-* identically sized dimms. We don't need to check the 3rd
-* channel because no cpu with gpu attached ships in that
-* configuration. Also, swizzling only makes sense for 2
-* channels anyway. */
- if (dimm_c0 == dimm_c1) {
- swizzle_x = I915_BIT_6_SWIZZLE_9_10;
- swizzle_y = I915_BIT_6_SWIZZLE_9;
- } else {
+
+ /* Make sure to honor boot time display configuration */
+ if (!(I915_READ(DISP_ARB_CTL) & DISP_TILE_SURFACE_SWIZZLING)) {
swizzle_x = I915_BIT_6_SWIZZLE_NONE;
swizzle_y = I915_BIT_6_SWIZZLE_NONE;
+ } else {
+ dimm_c0 = I915_READ(MAD_DIMM_C0);
+ dimm_c1 = I915_READ(MAD_DIMM_C1);
+ dimm_c0 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
+ dimm_c1 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
+ /* Enable swizzling when the channels are populated with
+* identically sized dimms. We don't need to check the
+* 3rd channel because no cpu with gpu attached ships
+* in that configuration. Also, swizzling only makes
+* sense for 2 channels anyway. */
+ if (dimm_c0 == dimm_c1) {
+ swizzle_x = I915_BIT_6_SWIZZLE_9_10;
+ swizzle_y = I915_BIT_6_SWIZZLE_9;
+ } else {
+ swizzle_x = I915_BIT_6_SWIZZLE_NONE;
+ swizzle_y = I915_BIT_6_SWIZZLE_NONE;
+ }
}
} else if (IS_GEN5(dev)) {
/* On Ironlake whatever DRAM config, GPU always do
diff --g
[PATCH 1/5] drm/i915: preserve SSC if previously set v2
Some machines may have a broken VBT or no VBT at all, but we still want
to use SSC there. So check for it and keep it enabled if we see it
already on. Based on an earlier fix from Kristian.
v2: honor modparam if set too (Daniel)
read out at init time and store for panel_use_ssc() use (Jesse)
Reported-by: Kristian H?gsberg
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_display.c | 11 ++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8631fb3..f57b752 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1404,6 +1404,8 @@ struct drm_i915_private {
struct intel_opregion opregion;
struct intel_vbt_data vbt;
+ bool bios_ssc; /* BIOS had SSC enabled at boot? */
+
/* overlay */
struct intel_overlay *overlay;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b5cbb28..0e8c9bc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5355,7 +5355,7 @@ static inline bool intel_panel_use_ssc(struct
drm_i915_private *dev_priv)
{
if (i915.panel_use_ssc >= 0)
return i915.panel_use_ssc != 0;
- return dev_priv->vbt.lvds_use_ssc
+ return (dev_priv->vbt.lvds_use_ssc || dev_priv->bios_ssc)
&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
}
@@ -12478,6 +12478,7 @@ void intel_modeset_setup_hw_state(struct drm_device
*dev,
void intel_modeset_gem_init(struct drm_device *dev)
{
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *c;
struct intel_framebuffer *fb;
@@ -12485,6 +12486,14 @@ void intel_modeset_gem_init(struct drm_device *dev)
intel_init_gt_powersave(dev);
mutex_unlock(>struct_mutex);
+ /*
+* There may be no VBT; and if the BIOS enabled SSC we can
+* just keep using it to avoid unnecessary flicker.
+*/
+ if ((HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) &&
+ (I915_READ(PCH_DREF_CONTROL) & DREF_SSC1_ENABLE))
+ dev_priv->bios_ssc = true;
+
intel_modeset_init_hw(dev);
intel_setup_overlay(dev);
--
1.8.3.2
[PATCH 3/3] drm/i915: use async hpd_irq_event function on resume
On Wed, 21 May 2014 08:52:34 +0200 Daniel Vetter wrote: > On Tue, May 20, 2014 at 03:25:35PM -0700, Jesse Barnes wrote: > > Gets the detect code (which may take awhile) out of the resume path, > > speeding things up a bit. > > > > Signed-off-by: Jesse Barnes > > --- > > drivers/gpu/drm/i915/i915_drv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 302495f..571f688 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -606,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool > > restore_gtt_mappings) > > intel_hpd_init(dev); > > dev_priv->enable_hotplug_processing = true; > > /* Config may have changed between suspend and resume */ > > - drm_helper_hpd_irq_event(dev); > > + async_schedule(drm_helper_hpd_irq_event_async, dev); > > Does that really help all that much? I've thought the driver core > sychnronizes all the async workers again once resume is done. I'm better > to schedule this as a fully async work with e.g. a 1s delay, like we do > with the rps resume work. That might be better, I'll check on the synchronization. I thought async_schedule was the new hotness we were supposed to use everywhere... -- Jesse Barnes, Intel Open Source Technology Center
[PATCH 3/3] drm/i915: use async hpd_irq_event function on resume
Gets the detect code (which may take awhile) out of the resume path, speeding things up a bit. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 302495f..571f688 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -606,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) intel_hpd_init(dev); dev_priv->enable_hotplug_processing = true; /* Config may have changed between suspend and resume */ - drm_helper_hpd_irq_event(dev); + async_schedule(drm_helper_hpd_irq_event_async, dev); } intel_opregion_init(dev); -- 1.8.4.2
[PATCH 2/3] drm: add async version of hpd_irq_event
In some cases, the callers of this function may not need the return
value and delaying the uevent is ok. So add an async version of the
function for use in those cases.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_probe_helper.c | 8
include/drm/drm_crtc_helper.h | 2 ++
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index 79f07f2..f3aee4a 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -446,3 +446,11 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
return changed;
}
EXPORT_SYMBOL(drm_helper_hpd_irq_event);
+
+void drm_helper_hpd_irq_event_async(void *data, async_cookie_t cookie)
+{
+ struct drm_device *dev = data;
+
+ drm_helper_hpd_irq_event(dev);
+}
+EXPORT_SYMBOL(drm_helper_hpd_irq_event_async);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index a3d75fe..4f4ed9c 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -33,6 +33,7 @@
#ifndef __DRM_CRTC_HELPER_H__
#define __DRM_CRTC_HELPER_H__
+#include
#include
#include
#include
@@ -172,6 +173,7 @@ extern int
drm_helper_probe_single_connector_modes_nomerge(struct drm_connector
extern void drm_kms_helper_poll_init(struct drm_device *dev);
extern void drm_kms_helper_poll_fini(struct drm_device *dev);
extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
+extern void drm_helper_hpd_irq_event_async(void *data, async_cookie_t cookie);
extern void drm_kms_helper_hotplug_event(struct drm_device *dev);
extern void drm_kms_helper_poll_disable(struct drm_device *dev);
--
1.8.4.2
[PATCH 1/3] drm/i915: drop encoder hot_plug calls at resume
We really just want to go detect displays again and fire off a hotplug
event if things have changed, not go through full hotplug processing.
Requested-by: Daniel Vetter
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_drv.c | 20 +---
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b948c71..302495f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -551,24 +551,6 @@ void intel_console_resume(struct work_struct *work)
console_unlock();
}
-static void intel_resume_hotplug(struct drm_device *dev)
-{
- struct drm_mode_config *mode_config = >mode_config;
- struct intel_encoder *encoder;
-
- mutex_lock(_config->mutex);
- DRM_DEBUG_KMS("running encoder hotplug functions\n");
-
- list_for_each_entry(encoder, _config->encoder_list, base.head)
- if (encoder->hot_plug)
- encoder->hot_plug(encoder);
-
- mutex_unlock(_config->mutex);
-
- /* Just fire off a uevent and let userspace tell us what to do */
- drm_helper_hpd_irq_event(dev);
-}
-
static int i915_drm_thaw_early(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -624,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
intel_hpd_init(dev);
dev_priv->enable_hotplug_processing = true;
/* Config may have changed between suspend and resume */
- intel_resume_hotplug(dev);
+ drm_helper_hpd_irq_event(dev);
}
intel_opregion_init(dev);
--
1.8.4.2
[PATCH] drm: Perform cmdline mode parsing during connector initialisation
On Thu, 15 May 2014 10:54:36 +0100
Chris Wilson wrote:
> i915.ko has a custom fbdev initialisation routine that aims to preserve
> the current mode set by the BIOS, unless overruled by the user. The
> user's wishes are determined by what, if any, mode is specified on the
> command line (via the video= parameter). However, that command line mode
> is first parsed by drm_fb_helper_initial_config() which is called after
> i915.ko's custom initial_config() as a fallback method. So in order for
> us to honour it, we need to move the cmdline parser earlier. If we
> perform the connector cmdline parsing as soon as we initialise the
> connector, that cmdline mode and forced status is then available even if
> the fbdev helper is not compiled in or never called.
>
> We also then expose the cmdline user mode in the connector mode lists.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73154
> Signed-off-by: Chris Wilson
> Cc: Jesse Barnes
> Cc: Ville Syrj?l?
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc.c | 56 +++
> drivers/gpu/drm/drm_fb_helper.c| 64
> ++--
> drivers/gpu/drm/drm_modes.c|1 +
> drivers/gpu/drm/drm_probe_helper.c | 17 ++
> drivers/gpu/drm/i915/intel_drv.h |5 ---
> include/drm/drm_crtc.h |1 +
> include/drm/drm_fb_helper.h|1 -
> 7 files changed, 78 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 461d19b..93c9198 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -799,6 +799,60 @@ static void drm_mode_remove(struct drm_connector
> *connector,
> }
>
> /**
> + * drm_connector_get_cmdline_mode - reads the user's cmdline mode
> + * @connector: connector to quwery
> + * @mode: returned mode
> + *
> + * The kernel supports per-connector configration of its consoles through
> + * use of the video= parameter. This function parses that option and
> + * extracts the user's specified mode (or enable/disable status) for a
> + * particular connector. This is typically only used during the early fbdev
> + * setup.
> + */
> +static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
> +{
> + struct drm_cmdline_mode *mode = >cmdline_mode;
> + char *option = NULL;
> +
> + if (fb_get_options(drm_get_connector_name(connector), ))
> + return;
> +
> + if (!drm_mode_parse_command_line_for_connector(option,
> +connector,
> +mode))
> + return;
> +
> + if (mode->force) {
> + const char *s;
> +
> + switch (mode->force) {
> + case DRM_FORCE_OFF:
> + s = "OFF";
> + break;
> + case DRM_FORCE_ON_DIGITAL:
> + s = "ON - dig";
> + break;
> + default:
> + case DRM_FORCE_ON:
> + s = "ON";
> + break;
> + }
> +
> + DRM_INFO("forcing %s connector %s\n",
> + drm_get_connector_name(connector), s);
> + connector->force = mode->force;
> + }
> +
> + DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
> + drm_get_connector_name(connector),
> + mode->xres, mode->yres,
> + mode->refresh_specified ? mode->refresh : 60,
> + mode->rb ? " reduced blanking" : "",
> + mode->margins ? " with margins" : "",
> + mode->interlace ? " interlaced" : "");
> +}
> +
> +/**
> * drm_connector_init - Init a preallocated connector
> * @dev: DRM device
> * @connector: the connector to init
> @@ -842,6 +896,8 @@ int drm_connector_init(struct drm_device *dev,
> connector->edid_blob_ptr = NULL;
> connector->status = connector_status_unknown;
>
> + drm_connector_get_cmdline_mode(connector);
> +
> list_add_tail(>head, >mode_config.connector_list);
> dev->mode_config.num_connector++;
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index e95ed58..c3b53f9 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -105,60 +105,6 @@ fail:
> }
> EXPORT_SYMBOL(drm_fb_helper_single_add
[PATCH] drm/mm: Don't WARN if drm_mm_reserve_node
On Mon, 7 Apr 2014 23:25:20 +0200 Daniel Vetter wrote: > Jesse's BIOS fb reconstruction code actually relies on the -ENOSPC > return value to detect overlapping framebuffers (which the bios uses > always when lighting up more than one screen). All this fanciness > happens in intel_alloc_plane_obj in intel_display.c. > > Since no one else uses this we can savely remove the WARN without > repercursions. > > Reported-by: Ben Widawsky > Cc: Ben Widawsky > Cc: Jesse Barnes > Cc: Dave Airlie > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_mm.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c > index a2d45b748f86..e4dfd5c3b15e 100644 > --- a/drivers/gpu/drm/drm_mm.c > +++ b/drivers/gpu/drm/drm_mm.c > @@ -192,8 +192,6 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct > drm_mm_node *node) > return 0; > } > > - WARN(1, "no hole found for node 0x%lx + 0x%lx\n", > - node->start, node->size); > return -ENOSPC; > } > EXPORT_SYMBOL(drm_mm_reserve_node); Yeah thanks, pushing this has been on my list for weeks now...
[Intel-gfx] [PATCH] drm/edid: Populate picture aspect ratio for CEA modes
On Mon, 20 Jan 2014 19:07:23 +0200 Ville Syrj?l? wrote: > On Mon, Dec 23, 2013 at 11:27:40AM +0530, Vandana Kannan wrote: > > Adding picture aspect ratio for CEA modes based on CEA-861D Table 3 or > > CEA-861E Table 4. This is useful for filling up the detail in AVI > > infoframe. > > > > v2: Ville's inputs incorporated. Added picture aspect ratio as part of > > edid_cea_modes instead of DRM_MODE > > > > Signed-off-by: Vandana Kannan > > Reviewed-by: Alex Deucher > > Reviewed-by: Ville Syrj?l? Note this one is needed for the patch I just reviewed to populate the PAR bits. -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH] drm/edid: Fill PAR in AVI infoframe based on CEA mode list
On Fri, 21 Mar 2014 08:31:29 +0530
Vandana Kannan wrote:
> Populate PAR in infoframe structure. If there is a user setting for PAR, then
> that value is set. Else, value is taken from CEA mode list if VIC is found.
> Else, PAR is calculated from resolution. If none of these conditions are
> satisfied, PAR is NONE as per initialization.
>
> As a next step, create a property that would enable a user space app to set
> aspect ratio. (will be pushed as a separate patch)
>
> Signed-off-by: Vandana Kannan
> Cc: Jesse Barnes
> Cc: Vijay Purushothaman
> Cc: Ville Syrj?l?
> Cc: intel-gfx at lists.freedesktop.org
> ---
> drivers/gpu/drm/drm_edid.c | 34 ++
> include/drm/drm_crtc.h |1 +
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index d4e3f9d..3db693f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2452,6 +2452,21 @@ u8 drm_match_cea_mode(const struct drm_display_mode
> *to_match)
> }
> EXPORT_SYMBOL(drm_match_cea_mode);
>
> +/**
> + * drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to
> + * the input VIC from the CEA mode list
> + *
> + * Returns picture aspect ratio
> + */
> +enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code)
> +{
> + /* return picture aspect ratio for video_code - 1 to access the
> + * right array element
> + */
> + return edid_cea_modes[video_code-1].picture_aspect_ratio;
> +}
> +EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
> +
> /*
> * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
> * specific block).
> @@ -3613,6 +3628,25 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
> hdmi_avi_infoframe *frame,
> frame->video_code = drm_match_cea_mode(mode);
>
> frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
> +
> + /* Populate picture aspect ratio from either CEA mode list or
> + * user input
> + */
> + if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
> + mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
> + frame->picture_aspect = mode->picture_aspect_ratio;
> + else if (frame->video_code > 0)
> + frame->picture_aspect = drm_get_cea_aspect_ratio(
> + frame->video_code);
> + else {
> + if (!(mode->vdisplay % 3) &&
> + (((mode->vdisplay * 4) / 3) == mode->hdisplay))
> + frame->picture_aspect = HDMI_PICTURE_ASPECT_4_3;
> + else if (!(mode->vdisplay % 9) &&
> + (((mode->vdisplay * 16) / 9) == mode->hdisplay))
> + frame->picture_aspect = HDMI_PICTURE_ASPECT_16_9;
> + }
> +
> frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
> frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 27f828c..50dc55a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -983,6 +983,7 @@ extern int drm_mode_gamma_get_ioctl(struct drm_device
> *dev,
> extern int drm_mode_gamma_set_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv);
> extern u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
> +extern enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8
> video_code);
> extern bool drm_detect_hdmi_monitor(struct edid *edid);
> extern bool drm_detect_monitor_audio(struct edid *edid);
> extern bool drm_rgb_quant_range_selectable(struct edid *edid);
Reviewed-by: Jesse Barnes
[PATCH 4/4] drm/i915: enable fastboot by default
Let them eat cake.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_params.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index a66ffb6..5f81047 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = {
.preliminary_hw_support =
IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
.disable_power_well = 1,
.enable_ips = 1,
- .fastboot = 0,
+ .fastboot = 42,
.enable_pc8 = 1,
.pc8_timeout = 5000,
.prefault_disable = 0,
--
1.7.9.5
[PATCH 3/4] drm/i915: use current mode if the size matches the preferred mode
From: Kristian H?gsberg <hoegsb...@gmail.com>
The BIOS may set a native mode that doesn't quite match the preferred
mode timings. It should be ok to use however if it uses the same size,
so try to avoid a mode set in that case.
Signed-off-by: Kristian H?gsberg
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_modes.c|8
drivers/gpu/drm/i915/intel_fbdev.c | 37 ++--
include/drm/drm_crtc.h |2 ++
3 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index b073315..7d2dda4 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -894,6 +894,14 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct
drm_display_mode *mode1,
}
EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
+bool drm_mode_same_size(const struct drm_display_mode *mode1,
+ const struct drm_display_mode *mode2)
+{
+ return mode1->vdisplay == mode2->vdisplay &&
+ mode1->hdisplay == mode2->hdisplay;
+}
+EXPORT_SYMBOL(drm_mode_same_size);
+
/**
* drm_mode_validate_size - make sure modes adhere to size constraints
* @dev: DRM device
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index d6d78c8..f81e3db 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -369,31 +369,30 @@ static bool intel_fb_initial_config(struct drm_fb_helper
*fb_helper,
/* go for command line mode first */
modes[i] = drm_pick_cmdline_mode(fb_conn, width, height);
- /* try for preferred next */
+ /* try for preferred next or match current */
if (!modes[i]) {
+ struct drm_display_mode *preferred;
+
DRM_DEBUG_KMS("looking for preferred mode on connector
%d\n",
fb_conn->connector->base.id);
- modes[i] = drm_has_preferred_mode(fb_conn, width,
- height);
- }
-
- /* last resort: use current mode */
- if (!modes[i]) {
- /*
-* IMPORTANT: We want to use the adjusted mode (i.e.
-* after the panel fitter upscaling) as the initial
-* config, not the input mode, which is what crtc->mode
-* usually contains. But since our current fastboot
-* code puts a mode derived from the post-pfit timings
-* into crtc->mode this works out correctly. We don't
-* use hwmode anywhere right now, so use it for this
-* since the fb helper layer wants a pointer to
-* something we own.
-*/
+ preferred = drm_has_preferred_mode(fb_conn, width,
+ height);
intel_mode_from_pipe_config(>crtc->hwmode,
_intel_crtc(encoder->crtc)->config);
- modes[i] = >crtc->hwmode;
+ modes[i] = >crtc->hwmode;
+
+ if (preferred &&
+ !drm_mode_same_size(preferred, modes[i])) {
+ DRM_DEBUG_KMS("using preferred mode %s "
+ "instead of current mode %s "
+ "on connector %d\n",
+ preferred->name,
+ modes[i]->name,
+ fb_conn->connector->base.id);
+ modes[i] = preferred;
+ }
}
+
crtcs[i] = new_crtc;
DRM_DEBUG_KMS("connector %s on crtc %d: %s\n",
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f764654..d5ebe3b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1020,6 +1020,8 @@ extern bool drm_mode_equal(const struct drm_display_mode
*mode1, const struct dr
extern bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode
*mode1, const struct drm_display_mode *mode2);
extern int drm_mode_width(const struct drm_display_mode *mode);
extern int drm_mode_height(const struct drm_display_mode *mode);
+extern bool drm_mode_same_size(const struct drm_display_mode *mode1,
+ const struct drm_display_mode *mode2);
/* for us by fb module */
extern struct drm_display_mode *drm_mode_create(struct drm_device *dev);
--
1.7.9.5
[PATCH 2/4] drm/i915: don't bother enabling swizzle bits on gen7+ v2
As of IVB, the memory controller does internal swizzling already, so we
shouldn't need to enable these. Based on an earlier fix from Kristian.
v2: preserve swizzling if BIOS had it set (Daniel)
Reported-by: Kristian H?gsberg
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_drv.h|1 +
drivers/gpu/drm/i915/i915_gem.c|6 ++
drivers/gpu/drm/i915/i915_gem_tiling.c |3 ++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c64f770..29cd977 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1508,6 +1508,7 @@ typedef struct drm_i915_private {
struct intel_vbt_data vbt;
bool bios_ssc; /* BIOS had SSC enabled at boot? */
+ bool bios_swizzle;
/* overlay */
struct intel_overlay *overlay;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 92b0b41..87e34bc 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4313,6 +4313,9 @@ void i915_gem_init_swizzling(struct drm_device *dev)
dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
return;
+ if (INTEL_INFO(dev)->gen >= 7 && !dev_priv->bios_swizzle)
+ return;
+
I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
DISP_TILE_SURFACE_SWIZZLING);
@@ -4454,6 +4457,9 @@ int i915_gem_init(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
+ if (I915_READ(DISP_ARB_CTL) & DISP_TILE_SURFACE_SWIZZLING)
+ dev_priv->bios_swizzle = true;
+
mutex_lock(>struct_mutex);
if (IS_VALLEYVIEW(dev)) {
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index eb99358..c6447de 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -91,7 +91,8 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
- if (IS_VALLEYVIEW(dev)) {
+ if (INTEL_INFO(dev)->gen >= 7 &&
+ !(I915_READ(TILECTL) & TILECTL_SWZCTL)) {
swizzle_x = I915_BIT_6_SWIZZLE_NONE;
swizzle_y = I915_BIT_6_SWIZZLE_NONE;
} else if (INTEL_INFO(dev)->gen >= 6) {
--
1.7.9.5
[PATCH 1/4] drm/i915: preserve SSC if previously set v2
Some machines may have a broken VBT or no VBT at all, but we still want
to use SSC there. So check for it and keep it enabled if we see it
already on. Based on an earlier fix from Kristian.
v2: honor modparam if set too (Daniel)
read out at init time and store for panel_use_ssc() use (Jesse)
Reported-by: Kristian H?gsberg
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_drv.h |2 ++
drivers/gpu/drm/i915/intel_display.c | 11 ++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 70fbe90..c64f770 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1507,6 +1507,8 @@ typedef struct drm_i915_private {
struct intel_opregion opregion;
struct intel_vbt_data vbt;
+ bool bios_ssc; /* BIOS had SSC enabled at boot? */
+
/* overlay */
struct intel_overlay *overlay;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2bccc68..4b3e1c0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4986,7 +4986,7 @@ static inline bool intel_panel_use_ssc(struct
drm_i915_private *dev_priv)
{
if (i915.panel_use_ssc >= 0)
return i915.panel_use_ssc != 0;
- return dev_priv->vbt.lvds_use_ssc
+ return (dev_priv->vbt.lvds_use_ssc || dev_priv->bios_ssc)
&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
}
@@ -11732,9 +11732,18 @@ void intel_modeset_setup_hw_state(struct drm_device
*dev,
void intel_modeset_gem_init(struct drm_device *dev)
{
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *c;
struct intel_framebuffer *fb;
+ /*
+* There may be no VBT; and if the BIOS enabled SSC we can
+* just keep using it to avoid unnecessary flicker.
+*/
+ if ((HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) &&
+ (I915_READ(PCH_DREF_CONTROL) & DREF_SSC1_ENABLE))
+ dev_priv->bios_ssc = true;
+
intel_modeset_init_hw(dev);
intel_setup_overlay(dev);
--
1.7.9.5
[PATCH] drm: Set the plane's crtc before calling disable_plane.
On Mon, 3 Mar 2014 13:38:36 -0800
St?phane Marchesin wrote:
> Some drivers like exynos need the crtc to be able to disable the plane,
> so set it before calling disable_plane.
>
> Signed-off-by: St?phane Marchesin
> ---
> drivers/gpu/drm/drm_crtc.c | 21 +++--
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 3b7d32d..0943316 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1947,10 +1947,21 @@ int drm_mode_setplane(struct drm_device *dev, void
> *data,
> }
> plane = obj_to_plane(obj);
>
> + obj = drm_mode_object_find(dev, plane_req->crtc_id,
> +DRM_MODE_OBJECT_CRTC);
> + if (!obj) {
> + DRM_DEBUG_KMS("Unknown crtc ID %d\n",
> + plane_req->crtc_id);
> + ret = -ENOENT;
> + goto out;
> + }
> + crtc = obj_to_crtc(obj);
> +
> /* No fb means shut it down */
> if (!plane_req->fb_id) {
> drm_modeset_lock_all(dev);
> old_fb = plane->fb;
> + plane->crtc = crtc;
> plane->funcs->disable_plane(plane);
> plane->crtc = NULL;
> plane->fb = NULL;
> @@ -1958,16 +1969,6 @@ int drm_mode_setplane(struct drm_device *dev, void
> *data,
> goto out;
> }
>
> - obj = drm_mode_object_find(dev, plane_req->crtc_id,
> -DRM_MODE_OBJECT_CRTC);
> - if (!obj) {
> - DRM_DEBUG_KMS("Unknown crtc ID %d\n",
> - plane_req->crtc_id);
> - ret = -ENOENT;
> - goto out;
> - }
> - crtc = obj_to_crtc(obj);
> -
> fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
> if (!fb) {
> DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
I'm pretty sure this is ok since we don't have much userspace using
this that might fail to pass in a crtc when shutting down a plane...
Reviewed-by: Jesse Barnes
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 5/5] drm/i915: Allow vblank interrupts during modeset and eliminate some vblank races
On Tue, 25 Feb 2014 11:58:26 +0900
Michel D?nzer wrote:
> On Mon, 2014-02-24 at 14:11 +0200, Ville Syrj?l? wrote:
> > On Mon, Feb 24, 2014 at 12:48:55PM +0900, Michel D?nzer wrote:
> > > On Fre, 2014-02-21 at 21:03 +0200, ville.syrjala at linux.intel.com wrote:
> > > >
> > > > We can kill of the drm_vblank_{pre,post}_modeset() calls since those are
> > > > there simply to make drm_vblank_get() fail during a modeset.
> > >
> > > Actually, their original purpose was to keep the DRM vblank counter
> > > consistent across modesets, assuming the modeset resets the hardware
> > > vblank counter.
> >
> > I see. Well, actually I really don't. The code is too funky for me to
> > tell what it actually ends up doing. The obvious way would be to
> > resample the hardware counter at drm_vblank_post_modeset(), which the
> > code certainly doesn't do. But maybe it did something sensible in the
> > past.
>
> When the pre/post-modeset hooks were originally added, it worked like
> this: the pre-modeset hook enabled the vblank interrupt, which updated
> the DRM vblank counter from the driver/HW counter. The post-modeset hook
> disabled the vblank interrupt again, which recorded the post-modeset
> driver/HW counter value.
>
> But the vblank code has changed a lot since then, not sure it still
> works like that.
Yeah it's changed a bit. I think Rob added the latest bits there.
They were originally in place to support both UMS and KMS drivers,
which is as ugly as it sounds.
As long as nothing breaks (vblank vs dpms, vblank vs modeset, vblank vs
high precision timestamps, vblank vs refcount) I think your code is ok.
Cc'ing Mario for another opinion too. This makes me nervous but it
seems ok.
I think you should update the docbook (with examples) as well so other
driver writers will know how to use this stuff.
Reviewed-by: Jesse Barnes
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 4/5] drm: Allow reenabling of vblank interrupts even if refcount>0
On Fri, 21 Feb 2014 21:03:34 +0200
ville.syrjala at linux.intel.com wrote:
> From: Ville Syrj?l?
>
> If someone holds a vblank reference across the modeset, and after/during
> the modeset someone tries to grab a vblank reference, the current code
> won't re-enable the vblank interrupts. That's not good, so instead allow
> the driver to choose whether drm_vblank_get() should always enable the
> interrupts regardless of the refcount.
>
> Combined with the drm_vblank_off/drm_vblank_on reject mechanism, this
> can also be used to allow drivers to use vblank interrupts during
> modeset, whether or not someone is currently holding a vblank reference.
>
> Signed-off-by: Ville Syrj?l?
> ---
> drivers/gpu/drm/drm_irq.c | 3 ++-
> include/drm/drmP.h| 6 ++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 6e5d820..d613b6f 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -897,7 +897,8 @@ int drm_vblank_get(struct drm_device *dev, int crtc)
> }
>
> /* Going from 0->1 means we have to enable interrupts again */
> - if (atomic_add_return(1, >vblank[crtc].refcount) == 1) {
> + if (atomic_add_return(1, >vblank[crtc].refcount) == 1 ||
> + dev->vblank_always_enable_on_get) {
> spin_lock(>vblank_time_lock);
> if (!dev->vblank[crtc].enabled) {
> /* Enable vblank irqs under vblank_time_lock protection.
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index ee40483..3eca0ee 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1156,6 +1156,12 @@ struct drm_device {
>*/
> bool vblank_disable_allowed;
>
> + /*
> + * Should a non-rejected drm_vblank_get() always enable the
> + * vblank interrupt regardless of the current refcount?
> + */
> + bool vblank_always_enable_on_get;
> +
> /* array of size num_crtcs */
> struct drm_vblank_crtc *vblank;
>
This seems like the sort of thing it would be good to have a test
for... I'm surprised we haven't hit it yet. But in looking at the code
I don't see where we'd re-enable things properly in this situation, so
I guess it's a real bug.
Reviewed-by: Jesse Barnes
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 3/5] drm: Allow the driver to reject vblank requests only when it really has the vblank interrupts disabled
off(crtc->dev, exynos_crtc->pipe);
> + drm_vblank_off(crtc->dev, exynos_crtc->pipe, false);
> }
>
> exynos_drm_fn_encoder(crtc, , exynos_drm_encoder_crtc_dpms);
> diff --git a/drivers/gpu/drm/gma500/gma_display.c
> b/drivers/gpu/drm/gma500/gma_display.c
> index 386de2c..ff18220 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -281,7 +281,7 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode)
> REG_WRITE(VGACNTRL, VGA_DISP_DISABLE);
>
> /* Turn off vblank interrupts */
> - drm_vblank_off(dev, pipe);
> + drm_vblank_off(dev, pipe, false);
>
> /* Wait for vblank for the disable to take effect */
> gma_wait_for_vblank(dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f19e6ea..bab0d08 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3643,7 +3643,7 @@ static void haswell_crtc_disable_planes(struct drm_crtc
> *crtc)
> int plane = intel_crtc->plane;
>
> intel_crtc_wait_for_pending_flips(crtc);
> - drm_vblank_off(dev, pipe);
> + drm_vblank_off(dev, pipe, false);
>
> /* FBC must be disabled before disabling the plane on HSW. */
> if (dev_priv->fbc.plane == plane)
> @@ -3774,7 +3774,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> encoder->disable(encoder);
>
> intel_crtc_wait_for_pending_flips(crtc);
> - drm_vblank_off(dev, pipe);
> + drm_vblank_off(dev, pipe, false);
>
> if (dev_priv->fbc.plane == plane)
> intel_disable_fbc(dev);
> @@ -4239,7 +4239,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>
> /* Give the overlay scaler a chance to disable if it's on this pipe */
> intel_crtc_wait_for_pending_flips(crtc);
> - drm_vblank_off(dev, pipe);
> + drm_vblank_off(dev, pipe, false);
>
> if (dev_priv->fbc.plane == plane)
> intel_disable_fbc(dev);
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 9336006..480bfec 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -324,7 +324,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
> }
> }
>
> - drm_vblank_off(drm, dc->pipe);
> + drm_vblank_off(drm, dc->pipe, false);
> }
>
> static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index f974da9..ee40483 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1090,6 +1090,7 @@ struct drm_vblank_crtc {
> int crtc; /* crtc index */
> bool enabled; /* so we don't call enable more than
> once per disable */
> + bool reject;/* reject drm_vblank_get()? */
> };
>
> /**
> @@ -1400,7 +1401,8 @@ extern void drm_send_vblank_event(struct drm_device
> *dev, int crtc,
> extern bool drm_handle_vblank(struct drm_device *dev, int crtc);
> extern int drm_vblank_get(struct drm_device *dev, int crtc);
> extern void drm_vblank_put(struct drm_device *dev, int crtc);
> -extern void drm_vblank_off(struct drm_device *dev, int crtc);
> +extern void drm_vblank_off(struct drm_device *dev, int crtc, bool reject);
> +extern void drm_vblank_on(struct drm_device *dev, int crtc);
> extern void drm_vblank_cleanup(struct drm_device *dev);
> extern u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
>struct timeval *tvblank, unsigned flags);
Reviewed-by: Jesse Barnes
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 2/5] drm: Make the vblank disable timer per-crtc
/**< VBLANK wait queue */
> struct timeval time[DRM_VBLANKTIME_RBSIZE]; /**< timestamp of
> current count */
> + struct timer_list disable_timer;/* delayed disable
> timer */
> atomic_t count; /**< number of VBLANK interrupts */
> atomic_t refcount; /* number of users of vblank
> interruptsper crtc */
> u32 last; /* protected by dev->vbl_lock, used */
> /* for wraparound handling */
> u32 last_wait; /* Last vblank seqno waited per CRTC */
> unsigned int inmodeset; /* Display driver is setting mode */
> + int crtc; /* crtc index */
> bool enabled; /* so we don't call enable more than
> once per disable */
> };
> @@ -1157,7 +1160,6 @@ struct drm_device {
>
> spinlock_t vblank_time_lock;/**< Protects vblank count and time
> updates during vblank enable/disable */
> spinlock_t vbl_lock;
> - struct timer_list vblank_disable_timer;
>
> u32 max_vblank_count; /**< size of vblank counter register */
>
Yeah this looks like a good fix.
Reviewed-by: Jesse Barnes
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 1/5] drm: Use correct spinlock flavor in drm_vblank_get()
On Fri, 21 Feb 2014 21:03:31 +0200
ville.syrjala at linux.intel.com wrote:
> From: Peter Hurley
>
> The irq flags state is already established by the outer
> spin_lock_irqsave(); re-disabling irqs is redundant.
>
> Signed-off-by: Peter Hurley
> ---
> drivers/gpu/drm/drm_irq.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index c2676b5..baa12e7 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -882,13 +882,13 @@ static void drm_update_vblank_count(struct drm_device
> *dev, int crtc)
> */
> int drm_vblank_get(struct drm_device *dev, int crtc)
> {
> - unsigned long irqflags, irqflags2;
> + unsigned long irqflags;
> int ret = 0;
>
> spin_lock_irqsave(>vbl_lock, irqflags);
> /* Going from 0->1 means we have to enable interrupts again */
> if (atomic_add_return(1, >vblank[crtc].refcount) == 1) {
> - spin_lock_irqsave(>vblank_time_lock, irqflags2);
> + spin_lock(>vblank_time_lock);
> if (!dev->vblank[crtc].enabled) {
> /* Enable vblank irqs under vblank_time_lock protection.
>* All vblank count & timestamp updates are held off
> @@ -906,7 +906,7 @@ int drm_vblank_get(struct drm_device *dev, int crtc)
> drm_update_vblank_count(dev, crtc);
> }
> }
> - spin_unlock_irqrestore(>vblank_time_lock, irqflags2);
> + spin_unlock(>vblank_time_lock);
> } else {
> if (!dev->vblank[crtc].enabled) {
> atomic_dec(>vblank[crtc].refcount);
Reviewed-by: Jesse Barnes
--
Jesse Barnes, Intel Open Source Technology Center
3.13 i915 brightness settings broken when going from docked -> undocked
On Sun, 23 Feb 2014 10:50:59 -0500
Josh Boyer wrote:
> On Thu, Feb 20, 2014 at 07:31:41PM -0500, Josh Boyer wrote:
> >On Wed, Feb 19, 2014 at 9:20 PM, Josh Boyer
> >wrote:
> >> Hi All,
> >>
> >> We've had a rather weird report[1] of the brightness adjustments being
> >> broken in a specific case with Thinkpad x220 hardware (SandyBridge
> >> based). If you boot the machine with it in a dock and then undock,
> >> the brightness adjustments do not work. That is with either the FN
> >> keys or the GNOME brightness slider.
> >>
> >> I can see that the value of
> >> /sys/class/backlight/acpi_video0/brightness increases/decreases but
> >> /sys/class/backlight/intel_backlight/brightness doesn't reflect any
> >> changes. With 3.12 this works, and oddly with 3.14-rc1 it works
> >> (specifically, it starts working around v3.13-10231-g53d8ab2 which is
> >> right after the first DRM merge for 3.14). With 3.13, if I undock and
> >> echo a higher value in the intel_backlight_brightness sysfs entry, the
> >> brightness will actually increase so it can be done manually, but it
> >> does not work as you'd expect.
> >>
> >> I'm in the middle of trying to do a reverse bisect for which patch
> >> fixes it in the 3.14-rcX series, but that's taking a while. I thought
> >> I'd email and see if anyone already knows about this situation, what
> >> patch in 3.13 broke this, and which one then fixed it again. Thus far
> >> all I've gathered is that backlight handling is confusing.
> >
> >The reverse bisect between 3.13 and 3.14-rc1 didn't prove fruitful,
> >either because I messed it up or there's a combination of things that
> >fix the issue. So instead I did a regular git bisect between 3.12 and
> >3.13 to see which commit _broke_ things and caused the above behavior.
> > That landed me at:
> >
> >Author: Jesse Barnes
> >Date: Thu Oct 31 18:55:49 2013 +0200
> >
> >drm/i915: make backlight functions take a connector
> >
> >I have no idea if that makes sense or not, but it's at least something
> >that seems to be in a relevant area of code. Does anyone involved in
> >that know why it would cause the above symptoms on 3.13, and which
> >commit(s) fix it in 3.14-rc1?
>
> Since nobody is replying I poked around a bit myself. The one commit
> that looks somewhat relevant in 3.14-rc1 seems to be:
>
> commit c91c9f32843a1b433de5a1ead4789a6bc8d3d914
> Author: Jani Nikula
> Date: Fri Nov 8 16:48:55 2013 +0200
>
> drm/i915: make asle notifications update backlight on all connectors
>
> That doesn't apply cleanly on 3.13 because of the other reworks that
> went in first, so I came up with the patch below. It seems to fix it
> for my machine, but I'm waiting for confirmation from the original bug
> reporter still. Maybe this will elicit some comments?
>
> josh
>
> Backport of upstream commit c91c9f328
>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_opregion.c | 31 ++-
> drivers/gpu/drm/i915/intel_panel.c| 4
> 3 files changed, 11 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 221ac62..d6d4349 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1371,6 +1371,7 @@ typedef struct drm_i915_private {
>
> /* backlight */
> struct {
> + bool present;
> int level;
> bool enabled;
> spinlock_t lock; /* bl registers and the above bl fields */
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> b/drivers/gpu/drm/i915/intel_opregion.c
> index 6d69a9b..b2a51ae 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -414,38 +414,19 @@ static u32 asle_set_backlight(struct drm_device *dev,
> u32 bclp)
> return ASLC_BACKLIGHT_FAILED;
>
> mutex_lock(>mode_config.mutex);
> - /*
> - * Could match the OpRegion connector here instead, but we'd also need
> - * to verify the connector could handle a backlight call.
> - */
> - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> - if (encoder->crtc == crtc) {
> - found = true;
> - break;
> - }
> -
> - if (!found) {
> - ret = ASLC_BACKLIGHT_FAILED;
> - goto out;
> - }
>
> - list_for_each_entry(connector, >mod
[PATCH] drm: export cmdline and preferred mode functions from fb helper
This allows drivers to use them in custom initial_config functions.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_fb_helper.c |6 --
include/drm/drm_fb_helper.h |6 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 98a0363..d99df15 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1136,7 +1136,7 @@ static int drm_fb_helper_probe_connector_modes(struct
drm_fb_helper *fb_helper,
return count;
}
-static struct drm_display_mode *drm_has_preferred_mode(struct
drm_fb_helper_connector *fb_connector, int width, int height)
+struct drm_display_mode *drm_has_preferred_mode(struct drm_fb_helper_connector
*fb_connector, int width, int height)
{
struct drm_display_mode *mode;
@@ -1149,6 +1149,7 @@ static struct drm_display_mode
*drm_has_preferred_mode(struct drm_fb_helper_conn
}
return NULL;
}
+EXPORT_SYMBOL(drm_has_preferred_mode);
static bool drm_has_cmdline_mode(struct drm_fb_helper_connector *fb_connector)
{
@@ -1157,7 +1158,7 @@ static bool drm_has_cmdline_mode(struct
drm_fb_helper_connector *fb_connector)
return cmdline_mode->specified;
}
-static struct drm_display_mode *drm_pick_cmdline_mode(struct
drm_fb_helper_connector *fb_helper_conn,
+struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector
*fb_helper_conn,
int width, int height)
{
struct drm_cmdline_mode *cmdline_mode;
@@ -1197,6 +1198,7 @@ create_mode:
list_add(>head, _helper_conn->connector->modes);
return mode;
}
+EXPORT_SYMBOL(drm_pick_cmdline_mode);
static bool drm_connector_enabled(struct drm_connector *connector, bool strict)
{
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 471f276..2d659dc 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -121,5 +121,11 @@ bool drm_fb_helper_initial_config(struct drm_fb_helper
*fb_helper, int bpp_sel);
int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper);
int drm_fb_helper_debug_enter(struct fb_info *info);
int drm_fb_helper_debug_leave(struct fb_info *info);
+struct drm_display_mode *
+drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
+ int width, int height);
+struct drm_display_mode *
+drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
+ int width, int height);
#endif
--
1.7.9.5
Fw: [Intel-gfx] [PATCH 1/6] drm: export cmdline and preferred mode functions from fb helper
Begin forwarded message:
Date: Wed, 12 Feb 2014 12:26:24 -0800
From: Jesse Barnes <jbar...@virtuousgeek.org>
To: intel-gfx at lists.freedesktop.org
Subject: [Intel-gfx] [PATCH 1/6] drm: export cmdline and preferred mode
functions from fb helper
This allows drivers to use them in custom initial_config functions.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_fb_helper.c |6 --
include/drm/drm_fb_helper.h |6 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c
b/drivers/gpu/drm/drm_fb_helper.c index 98a0363..d99df15 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1136,7 +1136,7 @@ static int
drm_fb_helper_probe_connector_modes(struct drm_fb_helper *fb_helper,
return count; }
-static struct drm_display_mode *drm_has_preferred_mode(struct
drm_fb_helper_connector *fb_connector, int width, int height) +struct
drm_display_mode *drm_has_preferred_mode(struct drm_fb_helper_connector
*fb_connector, int width, int height) { struct drm_display_mode *mode;
@@ -1149,6 +1149,7 @@ static struct drm_display_mode
*drm_has_preferred_mode(struct drm_fb_helper_conn }
return NULL;
}
+EXPORT_SYMBOL(drm_has_preferred_mode);
static bool drm_has_cmdline_mode(struct drm_fb_helper_connector
*fb_connector) {
@@ -1157,7 +1158,7 @@ static bool drm_has_cmdline_mode(struct
drm_fb_helper_connector *fb_connector) return cmdline_mode->specified;
}
-static struct drm_display_mode *drm_pick_cmdline_mode(struct
drm_fb_helper_connector *fb_helper_conn, +struct drm_display_mode
*drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
int width, int height) {
struct drm_cmdline_mode *cmdline_mode;
@@ -1197,6 +1198,7 @@ create_mode:
list_add(>head, _helper_conn->connector->modes);
return mode;
}
+EXPORT_SYMBOL(drm_pick_cmdline_mode);
static bool drm_connector_enabled(struct drm_connector *connector,
bool strict) {
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 471f276..2d659dc 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -121,5 +121,11 @@ bool drm_fb_helper_initial_config(struct
drm_fb_helper *fb_helper, int bpp_sel); int
drm_fb_helper_single_add_all_connectors(struct drm_fb_helper
*fb_helper); int drm_fb_helper_debug_enter(struct fb_info *info); int
drm_fb_helper_debug_leave(struct fb_info *info); +struct
drm_display_mode * +drm_has_preferred_mode(struct
drm_fb_helper_connector *fb_connector,
+ int width, int height);
+struct drm_display_mode *
+drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
+ int width, int height);
#endif
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 1/6] drm: export cmdline and preferred mode functions from fb helper
This allows drivers to use them in custom initial_config functions.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_fb_helper.c |6 --
include/drm/drm_fb_helper.h |6 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 98a0363..d99df15 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1136,7 +1136,7 @@ static int drm_fb_helper_probe_connector_modes(struct
drm_fb_helper *fb_helper,
return count;
}
-static struct drm_display_mode *drm_has_preferred_mode(struct
drm_fb_helper_connector *fb_connector, int width, int height)
+struct drm_display_mode *drm_has_preferred_mode(struct drm_fb_helper_connector
*fb_connector, int width, int height)
{
struct drm_display_mode *mode;
@@ -1149,6 +1149,7 @@ static struct drm_display_mode
*drm_has_preferred_mode(struct drm_fb_helper_conn
}
return NULL;
}
+EXPORT_SYMBOL(drm_has_preferred_mode);
static bool drm_has_cmdline_mode(struct drm_fb_helper_connector *fb_connector)
{
@@ -1157,7 +1158,7 @@ static bool drm_has_cmdline_mode(struct
drm_fb_helper_connector *fb_connector)
return cmdline_mode->specified;
}
-static struct drm_display_mode *drm_pick_cmdline_mode(struct
drm_fb_helper_connector *fb_helper_conn,
+struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector
*fb_helper_conn,
int width, int height)
{
struct drm_cmdline_mode *cmdline_mode;
@@ -1197,6 +1198,7 @@ create_mode:
list_add(>head, _helper_conn->connector->modes);
return mode;
}
+EXPORT_SYMBOL(drm_pick_cmdline_mode);
static bool drm_connector_enabled(struct drm_connector *connector, bool strict)
{
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 471f276..2d659dc 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -121,5 +121,11 @@ bool drm_fb_helper_initial_config(struct drm_fb_helper
*fb_helper, int bpp_sel);
int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper);
int drm_fb_helper_debug_enter(struct fb_info *info);
int drm_fb_helper_debug_leave(struct fb_info *info);
+struct drm_display_mode *
+drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
+ int width, int height);
+struct drm_display_mode *
+drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn,
+ int width, int height);
#endif
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[PATCH] drm: expose subpixel order name routine v3
Just like we have for connector type etc.
v2: drop static array (Chris)
v3: add kdoc (Daniel)
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_crtc.c | 23 +++
include/drm/drm_crtc.h |1 +
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3b7d32d..35ea15d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -215,6 +215,16 @@ static const struct drm_prop_enum_list
drm_encoder_enum_list[] =
{ DRM_MODE_ENCODER_DSI, "DSI" },
};
+static const struct drm_prop_enum_list drm_subpixel_enum_list[] =
+{
+ { SubPixelUnknown, "Unknown" },
+ { SubPixelHorizontalRGB, "Horizontal RGB" },
+ { SubPixelHorizontalBGR, "Horizontal BGR" },
+ { SubPixelVerticalRGB, "Vertical RGB" },
+ { SubPixelVerticalBGR, "Vertical BGR" },
+ { SubPixelNone, "None" },
+};
+
void drm_connector_ida_init(void)
{
int i;
@@ -264,6 +274,19 @@ const char *drm_get_connector_status_name(enum
drm_connector_status status)
}
EXPORT_SYMBOL(drm_get_connector_status_name);
+/**
+ * drm_get_subpixel_order_name - return a string for a given subpixel enum
+ * @order: enum of subpixel_order
+ *
+ * Note you could abuse this and return something out of bounds, but that
+ * would be a caller error. No unscrubbed user data should make it here.
+ */
+const char *drm_get_subpixel_order_name(enum subpixel_order order)
+{
+ return drm_subpixel_enum_list[order].name;
+}
+EXPORT_SYMBOL(drm_get_subpixel_order_name);
+
static char printable_char(int c)
{
return isascii(c) && isprint(c) ? c : '?';
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 71727b6..ce9ee60 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -991,6 +991,7 @@ extern void drm_encoder_cleanup(struct drm_encoder
*encoder);
extern const char *drm_get_connector_name(const struct drm_connector
*connector);
extern const char *drm_get_connector_status_name(enum drm_connector_status
status);
+extern const char *drm_get_subpixel_order_name(enum subpixel_order order);
extern const char *drm_get_dpms_name(int val);
extern const char *drm_get_dvi_i_subconnector_name(int val);
extern const char *drm_get_dvi_i_select_name(int val);
--
1.7.9.5
[Intel-gfx] [PATCH] drm/dp: Clarify automated test constant and add constant for FAUX test pattern
On Mon, 07 Oct 2013 11:05:57 +0300 Jani Nikula wrote: > On Fri, 04 Oct 2013, Todd Previte wrote: > > - DP_TEST_LINK_PATTERN is ambiguous, rename to DP_TEST_LINK_VIDEO_PATTERN > > to clarify > > - Added DP_TEST_LINK_FAUX_PATTERN to support automated testing of Fast AUX > > > > Signed-off-by: Todd Previte > > Reviewed-by: Jani Nikula Did this ever go in? -- Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH 17/20] drm/i915: Use adjusted_mode in the fastboot hack to disable pfit
On Fri, 20 Sep 2013 16:33:47 +0100
Damien Lespiau damien.lesp...@intel.com wrote:
On Fri, Sep 20, 2013 at 05:54:55PM +0300, Ville Syrjälä wrote:
On Thu, Sep 19, 2013 at 05:40:32PM +0100, Damien Lespiau wrote:
When booting with i915.fastboot=1, we always take tha code path and end
up undoing what we're trying to do with adjusted_mode.
Hopefully, as the fastboot hardware readout code is using adjusted_mode
as well, it should be equivalent.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f868266..2b9f80b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2288,9 +2288,12 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x,
int y,
/* Update pipe size and adjust fitter if needed */
if (i915_fastboot) {
+ const struct drm_display_mode *adjusted_mode =
+ intel_crtc-config.adjusted_mode;
+
I915_WRITE(PIPESRC(intel_crtc-pipe),
-((crtc-mode.hdisplay - 1) 16) |
-(crtc-mode.vdisplay - 1));
+((adjusted_mode-crtc_hdisplay - 1) 16) |
+(adjusted_mode-crtc_vdisplay - 1));
if (!intel_crtc-config.pch_pfit.enabled
(intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
OK, I'm offically confused by this thing. Maybe it got a bit broken
by the pfit.enabled change?
I must assume that the original intention of this was to turn off the
panel fitter in case the BIOS had left it enabled w/ 0x0 size, but
I'm not sure how that would even work. Anyways, now it will turn it
off if it's already off, which doesn't make much sense.
And I guess the PIPESRC write is there because we assume the BIOS left
it wrong for the non-pfit case. We have explicit readout for it now,
so we could actually check if that's the case.
Well, I didn't even read beyond the PIPESRC write, but yes, now that you
mention it, it looks dodgy.
Jesse, do you remember what was the original intention? neither the
commit introducing the change nor the comment are very verbose.
Just for the record, I'll capture what we discussed on IRC.
The reason for this is that in compute_mode_changes we check the native
mode (not the pfit mode) to see if we can flip rather than do a full
mode set. In the fastboot case, we'll flip, but if we don't update the
pipesrc and pfit state, we'll end up with a big fb scanned out into the
wrong sized surface.
To fix this properly, we need to hoist the checks up into
compute_mode_changes (or above), check the actual pfit state and
whether the platform allows pfit disable with pipe active, and only
then update the pipesrc and pfit state, even on the flip path.
On top of that, other state like info frames and audio state needs to
be tracked and preserved for fastboot as applicable. Then we can
remove the parameter hacks.
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm tree for 3.12-rc1
On Thu, 5 Sep 2013 12:18:32 -0700 Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Sep 5, 2013 at 3:41 AM, Dave Airlie airl...@linux.ie wrote: i915: Haswell PC8+ support and eLLC support, HDMI 4K support, initial per-process VMA pieces, watermark reworks, convert to generic hdmi infoframes, encoder reworking, fastboot support, Hmm. The first time I booted this, I just got a black screen on my Haswell desktop when X11 started up. I could ctrl-alt-BS and ctrl-alt-del to reboot the machine, and neither the Xorg.0.log nor the dmesg contained anything interesting. Did the console come back after ctl-alt-bs? Or was it just a blind reboot? Troubling that it doesn't happen again... I was about to try to bisect it, but decided to see how repeatable it was, and it didn't happen again. But it also hasn't ever happened before, so I'm a bit worried. This is with the DP cable, which has made my other Haswell issues go away. I'm also a bit bummed that hw acceleration of video doesn't seem to work on Haswell, meaning that full-screen is now a jerky mess. I fear that that is user-space libraries/X.org, but I thought I'd mention it in the hope of getting a oh, it's working for us, you'll get a fix for it soon. AFAIK we have libva support out there for HSW. The trick is getting your stack to actually use it. Gwenole or Sean may be able to help. Because my shiny new 65W haswell is really nice and does a make allmodconfig in half the time of my old machine, but the GPU side has been something of a step backwards... Well we definitely don't want that... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: add fast boot support for Haswell
On Thu, 1 Aug 2013 14:12:22 -0700
Furquan Shaikh wrote:
> @@ -1282,6 +1283,13 @@ static void intel_ddi_get_config(struct intel_encoder
> *encoder,
> flags |= DRM_MODE_FLAG_NVSYNC;
>
> pipe_config->adjusted_mode.flags |= flags;
> +
> + if (port == PORT_A) {
> + if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
> + pipe_config->port_clock = 162000;
> + else
> + pipe_config->port_clock = 27;
> + }
Sorry Furquan, I should have checked this earlier, I knew it was too
good to be true. :)
On HSW, DP_A is actually DDI_BUF_CTL, and it has a different layout
than the old DP_A reg. Like Daniel said, doing it the old way is
invalid on HSW. It might work in your configs, but I think that's just
coincidence, since bit 16 is the port reversal bit on HSW, not the
clock freq.
To get the clock freq, you need to look at 0x45020 to find the refclk,
then look at the WRPLL_CTL for the pipe to get the dividers. That's
what Daniel meant when he asked for a full "clock_get" function. It's
only a little more complicated, but you'll need docs for it. Charlie
Huang ought to be able to get you the NDA docs that should have the
info you need.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH] drm/i915: add fast boot support for Haswell
On Thu, 1 Aug 2013 14:12:22 -0700
Furquan Shaikh furq...@google.com wrote:
@@ -1282,6 +1283,13 @@ static void intel_ddi_get_config(struct intel_encoder
*encoder,
flags |= DRM_MODE_FLAG_NVSYNC;
pipe_config-adjusted_mode.flags |= flags;
+
+ if (port == PORT_A) {
+ if ((I915_READ(DP_A) DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
+ pipe_config-port_clock = 162000;
+ else
+ pipe_config-port_clock = 27;
+ }
Sorry Furquan, I should have checked this earlier, I knew it was too
good to be true. :)
On HSW, DP_A is actually DDI_BUF_CTL, and it has a different layout
than the old DP_A reg. Like Daniel said, doing it the old way is
invalid on HSW. It might work in your configs, but I think that's just
coincidence, since bit 16 is the port reversal bit on HSW, not the
clock freq.
To get the clock freq, you need to look at 0x45020 to find the refclk,
then look at the WRPLL_CTL for the pipe to get the dividers. That's
what Daniel meant when he asked for a full clock_get function. It's
only a little more complicated, but you'll need docs for it. Charlie
Huang ought to be able to get you the NDA docs that should have the
info you need.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/i915: fix long-standing SNB regression in power consumption after resume
On Wed, 17 Jul 2013 10:22:58 +0400
Konstantin Khlebnikov wrote:
> This patch fixes regression in power consumtion of sandy bridge gpu, which
> exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that
> it's extremely busy. After that it never reaches rc6 state.
>
> Bug exists since kernel v3.6, commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0
> ("drm/i915: load boot context at driver init time").
>
> For some reason RC6 is already enabled at the beginning of resuming process.
> Following initliaztion breaks some internal state and confuses RPS engine.
> This patch disables RC6 at the beginnig of resume and initialization.
>
> I've rearranged initialization sequence, because intel_disable_gt_powersave()
> needs initialized force_wake_get/put and some locks from the dev_priv.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=54089
> References: https://bugzilla.kernel.org/show_bug.cgi?id=58971
> Signed-off-by: Konstantin Khlebnikov
> Cc: Daniel Vetter
> Cc: Chris Wilson
> Cc: Jesse Barnes
> ---
My hero!
So the later init change didn't work?
Either way, great to have this fix in the tree... thanks again.
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 22:43:49 +0200 Daniel Vetter wrote: > On Tue, Jul 16, 2013 at 01:19:25PM -0700, Jesse Barnes wrote: > > On Tue, 16 Jul 2013 10:06:54 -0700 > > Jesse Barnes wrote: > > > > > On Tue, 16 Jul 2013 11:34:25 +0400 > > > Konstantin Khlebnikov wrote: > > > > I've tested that patch and it really works for me. If you want change > > > > something for other hardware or > > > > extend range where forcewake is held prease do it in a separate patch. > > > > This will be good for bisecting new bugs in the future. > > > > > > Thanks a ton for finding this Konstantin, it puts us on the right > > > track. > > > > > > Can I ask you to test this patch? The theory is that having RC6 > > > enabled messes with the initial programming sequence, so it's probably > > > best to just shut it off at init until we're done, rather than trying > > > to forcewake around everywhere we need it. > > > > Oops, last one triggers a warn about IRQs. This one doesn't and still > > works for me. > > > > Testing welcome. > > > > Thanks, > > -- > > Jesse Barnes, Intel Open Source Technology Center > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index c9d9d20..d962ec0 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) > > drm_i915_private_t *dev_priv = dev->dev_private; > > int ret; > > > > + /* BIOS often leaves RC6 enabled, but disable it for hw init */ > > + intel_disable_gt_powersave(dev); > > I think it'd be better to have an explicit gen >= 6 check here and a > disable_rps call. disable_gt_powersave also calls the ironlake version, > which restores probably bogus values (since we haven't read them out yet > in the enable code) into the hw. Yeah the ilk "restore to initial freq" needs fixing, but I thought we wanted this on all gens? It shouldn't hurt anything, and may help with other issues as well. -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 10:06:54 -0700 Jesse Barnes wrote: > On Tue, 16 Jul 2013 11:34:25 +0400 > Konstantin Khlebnikov wrote: > > I've tested that patch and it really works for me. If you want change > > something for other hardware or > > extend range where forcewake is held prease do it in a separate patch. > > This will be good for bisecting new bugs in the future. > > Thanks a ton for finding this Konstantin, it puts us on the right > track. > > Can I ask you to test this patch? The theory is that having RC6 > enabled messes with the initial programming sequence, so it's probably > best to just shut it off at init until we're done, rather than trying > to forcewake around everywhere we need it. Oops, last one triggers a warn about IRQs. This one doesn't and still works for me. Testing welcome. Thanks, -- Jesse Barnes, Intel Open Source Technology Center diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c9d9d20..d962ec0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) drm_i915_private_t *dev_priv = dev->dev_private; int ret; + /* BIOS often leaves RC6 enabled, but disable it for hw init */ + intel_disable_gt_powersave(dev); + if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt()) return -EIO;
[Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 11:34:25 +0400
Konstantin Khlebnikov wrote:
> I've tested that patch and it really works for me. If you want change
> something for other hardware or
> extend range where forcewake is held prease do it in a separate patch.
> This will be good for bisecting new bugs in the future.
Thanks a ton for finding this Konstantin, it puts us on the right
track.
Can I ask you to test this patch? The theory is that having RC6
enabled messes with the initial programming sequence, so it's probably
best to just shut it off at init until we're done, rather than trying
to forcewake around everywhere we need it.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_d
index 12ea1a9..9152cba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9648,6 +9648,9 @@ static void i915_disable_vga(struct drm_device *dev)
void intel_modeset_init_hw(struct drm_device *dev)
{
+ /* BIOS often leaves RC6 enabled, but disable it for hw init */
+ intel_disable_gt_powersave(dev);
+
intel_init_power_well(dev);
intel_prepare_ddi(dev);
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 11:34:25 +0400
Konstantin Khlebnikov khlebni...@openvz.org wrote:
I've tested that patch and it really works for me. If you want change
something for other hardware or
extend range where forcewake is held prease do it in a separate patch.
This will be good for bisecting new bugs in the future.
Thanks a ton for finding this Konstantin, it puts us on the right
track.
Can I ask you to test this patch? The theory is that having RC6
enabled messes with the initial programming sequence, so it's probably
best to just shut it off at init until we're done, rather than trying
to forcewake around everywhere we need it.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_d
index 12ea1a9..9152cba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9648,6 +9648,9 @@ static void i915_disable_vga(struct drm_device *dev)
void intel_modeset_init_hw(struct drm_device *dev)
{
+ /* BIOS often leaves RC6 enabled, but disable it for hw init */
+ intel_disable_gt_powersave(dev);
+
intel_init_power_well(dev);
intel_prepare_ddi(dev);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 10:06:54 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: On Tue, 16 Jul 2013 11:34:25 +0400 Konstantin Khlebnikov khlebni...@openvz.org wrote: I've tested that patch and it really works for me. If you want change something for other hardware or extend range where forcewake is held prease do it in a separate patch. This will be good for bisecting new bugs in the future. Thanks a ton for finding this Konstantin, it puts us on the right track. Can I ask you to test this patch? The theory is that having RC6 enabled messes with the initial programming sequence, so it's probably best to just shut it off at init until we're done, rather than trying to forcewake around everywhere we need it. Oops, last one triggers a warn about IRQs. This one doesn't and still works for me. Testing welcome. Thanks, -- Jesse Barnes, Intel Open Source Technology Center diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c9d9d20..d962ec0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) drm_i915_private_t *dev_priv = dev-dev_private; int ret; + /* BIOS often leaves RC6 enabled, but disable it for hw init */ + intel_disable_gt_powersave(dev); + if (INTEL_INFO(dev)-gen 6 !intel_enable_gtt()) return -EIO; ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 22:43:49 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Tue, Jul 16, 2013 at 01:19:25PM -0700, Jesse Barnes wrote: On Tue, 16 Jul 2013 10:06:54 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: On Tue, 16 Jul 2013 11:34:25 +0400 Konstantin Khlebnikov khlebni...@openvz.org wrote: I've tested that patch and it really works for me. If you want change something for other hardware or extend range where forcewake is held prease do it in a separate patch. This will be good for bisecting new bugs in the future. Thanks a ton for finding this Konstantin, it puts us on the right track. Can I ask you to test this patch? The theory is that having RC6 enabled messes with the initial programming sequence, so it's probably best to just shut it off at init until we're done, rather than trying to forcewake around everywhere we need it. Oops, last one triggers a warn about IRQs. This one doesn't and still works for me. Testing welcome. Thanks, -- Jesse Barnes, Intel Open Source Technology Center diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c9d9d20..d962ec0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) drm_i915_private_t *dev_priv = dev-dev_private; int ret; + /* BIOS often leaves RC6 enabled, but disable it for hw init */ + intel_disable_gt_powersave(dev); I think it'd be better to have an explicit gen = 6 check here and a disable_rps call. disable_gt_powersave also calls the ironlake version, which restores probably bogus values (since we haven't read them out yet in the enable code) into the hw. Yeah the ilk restore to initial freq needs fixing, but I thought we wanted this on all gens? It shouldn't hurt anything, and may help with other issues as well. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
'Timed out waiting for forcewake old ack to clear' and hangup on IvyBridge system
On Sat, 22 Jun 2013 13:04:09 -0700 Guenter Roeck wrote: > On Sat, Jun 22, 2013 at 12:16:46PM -0700, Jesse Barnes wrote: > > On Fri, 21 Jun 2013 23:58:08 -0700 > > Guenter Roeck wrote: > > > > > Hi all, > > > > > > after upgrading one of my servers to 3.8, then 3.9.7 and 3.10-rc6, I > > > started to > > > see lots of "Timed out waiting for forcewake old ack to clear" error > > > messages, > > > including hang-ups especially if the system was highly loaded. With 3.5.24 > > > everything was fine. > > > > > > After backing out commit 36ec8f877 (drm/i915: unconditionally use mt > > > forcewake > > > on hsw/ivb), everything is back to normal. The log message is still > > > there, but > > > only once during boot, and the system runs stable. > > > > > > CPU is "Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz", mainboard is Supermicro > > > C7H61, BIOS version 2.00 dated 11/02/2012. Configuration file is whatever > > > comes with Ubuntu; I'll be happy to provide a copy if anyone thinks it > > > might > > > help. > > > > > > Any idea what else I can do besides using a special kernel with the > > > backed out > > > commit ? Is it possible that others have the same problem ? > > > > Ouch, so a BIOS that uses the other forcewake mechanism seems to have > > escaped. Is there a newer one available for your system? I'm hoping > > it'll fix the issue, otherwise we may have to introduce both methods > > for IVB again... > > > I installed the latest BIOS version (2.00b), but it did not fix the problem. > > Is there some info (such as an Intel document describing what needs to be > done) > which I could pass on to Supermicro ? > > I think it would be helpful if the condition was detected and reported, if > that > is possible. I spent two days so far tracking this down. It would be nice > if others would not have to go through the same experience. I don't think there's anything public to share, but it's not a big deal to simply revert the patch in question. That seems like the right thing to do anyway since we'd like stuff to work "out of the box" as much as possible. Daniel? -- Jesse Barnes, Intel Open Source Technology Center
Re: 'Timed out waiting for forcewake old ack to clear' and hangup on IvyBridge system
On Sat, 22 Jun 2013 13:04:09 -0700 Guenter Roeck li...@roeck-us.net wrote: On Sat, Jun 22, 2013 at 12:16:46PM -0700, Jesse Barnes wrote: On Fri, 21 Jun 2013 23:58:08 -0700 Guenter Roeck li...@roeck-us.net wrote: Hi all, after upgrading one of my servers to 3.8, then 3.9.7 and 3.10-rc6, I started to see lots of Timed out waiting for forcewake old ack to clear error messages, including hang-ups especially if the system was highly loaded. With 3.5.24 everything was fine. After backing out commit 36ec8f877 (drm/i915: unconditionally use mt forcewake on hsw/ivb), everything is back to normal. The log message is still there, but only once during boot, and the system runs stable. CPU is Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz, mainboard is Supermicro C7H61, BIOS version 2.00 dated 11/02/2012. Configuration file is whatever comes with Ubuntu; I'll be happy to provide a copy if anyone thinks it might help. Any idea what else I can do besides using a special kernel with the backed out commit ? Is it possible that others have the same problem ? Ouch, so a BIOS that uses the other forcewake mechanism seems to have escaped. Is there a newer one available for your system? I'm hoping it'll fix the issue, otherwise we may have to introduce both methods for IVB again... I installed the latest BIOS version (2.00b), but it did not fix the problem. Is there some info (such as an Intel document describing what needs to be done) which I could pass on to Supermicro ? I think it would be helpful if the condition was detected and reported, if that is possible. I spent two days so far tracking this down. It would be nice if others would not have to go through the same experience. I don't think there's anything public to share, but it's not a big deal to simply revert the patch in question. That seems like the right thing to do anyway since we'd like stuff to work out of the box as much as possible. Daniel? -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
'Timed out waiting for forcewake old ack to clear' and hangup on IvyBridge system
On Fri, 21 Jun 2013 23:58:08 -0700 Guenter Roeck wrote: > Hi all, > > after upgrading one of my servers to 3.8, then 3.9.7 and 3.10-rc6, I started > to > see lots of "Timed out waiting for forcewake old ack to clear" error messages, > including hang-ups especially if the system was highly loaded. With 3.5.24 > everything was fine. > > After backing out commit 36ec8f877 (drm/i915: unconditionally use mt forcewake > on hsw/ivb), everything is back to normal. The log message is still there, but > only once during boot, and the system runs stable. > > CPU is "Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz", mainboard is Supermicro > C7H61, BIOS version 2.00 dated 11/02/2012. Configuration file is whatever > comes with Ubuntu; I'll be happy to provide a copy if anyone thinks it might > help. > > Any idea what else I can do besides using a special kernel with the backed out > commit ? Is it possible that others have the same problem ? Ouch, so a BIOS that uses the other forcewake mechanism seems to have escaped. Is there a newer one available for your system? I'm hoping it'll fix the issue, otherwise we may have to introduce both methods for IVB again... -- Jesse Barnes, Intel Open Source Technology Center
Re: 'Timed out waiting for forcewake old ack to clear' and hangup on IvyBridge system
On Fri, 21 Jun 2013 23:58:08 -0700 Guenter Roeck li...@roeck-us.net wrote: Hi all, after upgrading one of my servers to 3.8, then 3.9.7 and 3.10-rc6, I started to see lots of Timed out waiting for forcewake old ack to clear error messages, including hang-ups especially if the system was highly loaded. With 3.5.24 everything was fine. After backing out commit 36ec8f877 (drm/i915: unconditionally use mt forcewake on hsw/ivb), everything is back to normal. The log message is still there, but only once during boot, and the system runs stable. CPU is Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz, mainboard is Supermicro C7H61, BIOS version 2.00 dated 11/02/2012. Configuration file is whatever comes with Ubuntu; I'll be happy to provide a copy if anyone thinks it might help. Any idea what else I can do besides using a special kernel with the backed out commit ? Is it possible that others have the same problem ? Ouch, so a BIOS that uses the other forcewake mechanism seems to have escaped. Is there a newer one available for your system? I'm hoping it'll fix the issue, otherwise we may have to introduce both methods for IVB again... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] compat/compat-drivers/linux-next: fb skip_vt_switch
On Thu, 28 Mar 2013 05:04:26 -0700 "Luis R. Rodriguez" wrote: > The new commit by Jesse that extended the fb_info with a skip_vt_switch > element is the simplest example of a data structure expansion. We'd backport > this by adding a static inline to compat so that new kernels muck with the > new element and for older kernels this would be a no-op. This reduces the > size of the backport and unclutters the required patch with #idefs, and > insteads leaves only a replacement of the usage of the new elements with > a static inline, this however would still be required on our end: > > - info->skip_vt_switch = true; > + fb_enable_skip_vt_switch(info); > > So we'd then have to just add this static inline change for each new driver... > There may be a way to get SmPL to do this for us... Yeah I'm not attached to the direct structure reference; a couple of inlines are just as easy to read. So no argument from me. Thanks, -- Jesse Barnes, Intel Open Source Technology Center
[PATCH] drm: Fix error message in drmWaitVBlank
On Thu, 28 Mar 2013 14:05:40 +0800
Daniel Kurtz wrote:
> If clock_gettime did fail, it would return -1 and set errno.
> What we really want to strerror() is the errno.
>
> Signed-off-by: Daniel Kurtz
> ---
> xf86drm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 2a74c80..4791a05 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -1946,7 +1946,7 @@ int drmWaitVBlank(int fd, drmVBlankPtr vbl)
>
> ret = clock_gettime(CLOCK_MONOTONIC, );
> if (ret < 0) {
> - fprintf(stderr, "clock_gettime failed: %s\n", strerror(ret));
> + fprintf(stderr, "clock_gettime failed: %s\n", strerror(errno));
> goto out;
> }
> timeout.tv_sec++;
Applied, thanks.
--
Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH] drm: Fix error message in drmWaitVBlank
On Thu, 28 Mar 2013 14:05:40 +0800
Daniel Kurtz djku...@chromium.org wrote:
If clock_gettime did fail, it would return -1 and set errno.
What we really want to strerror() is the errno.
Signed-off-by: Daniel Kurtz djku...@chromium.org
---
xf86drm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xf86drm.c b/xf86drm.c
index 2a74c80..4791a05 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -1946,7 +1946,7 @@ int drmWaitVBlank(int fd, drmVBlankPtr vbl)
ret = clock_gettime(CLOCK_MONOTONIC, timeout);
if (ret 0) {
- fprintf(stderr, clock_gettime failed: %s\n, strerror(ret));
+ fprintf(stderr, clock_gettime failed: %s\n, strerror(errno));
goto out;
}
timeout.tv_sec++;
Applied, thanks.
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] compat/compat-drivers/linux-next: fb skip_vt_switch
On Thu, 28 Mar 2013 05:04:26 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: The new commit by Jesse that extended the fb_info with a skip_vt_switch element is the simplest example of a data structure expansion. We'd backport this by adding a static inline to compat so that new kernels muck with the new element and for older kernels this would be a no-op. This reduces the size of the backport and unclutters the required patch with #idefs, and insteads leaves only a replacement of the usage of the new elements with a static inline, this however would still be required on our end: - info-skip_vt_switch = true; + fb_enable_skip_vt_switch(info); So we'd then have to just add this static inline change for each new driver... There may be a way to get SmPL to do this for us... Yeah I'm not attached to the direct structure reference; a couple of inlines are just as easy to read. So no argument from me. Thanks, -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] [PATCH 6/8] drm/i915: Enable/Disable PSR
On Tue, 26 Feb 2013 14:48:33 +0200
Ville Syrj?l? wrote:
> On Mon, Feb 25, 2013 at 07:55:20PM -0300, Rodrigo Vivi wrote:
> > Adding Enable and Disable PSR functionalities. This includes setting the
> > PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
> > enabling PSR in the sink via DPCD register and finally enabling PSR on
> > the host.
> >
> > This patch is heavily based on initial PSR code by Sateesh Kavuri and
> > Kumar Shobhit but in a different implementation.
> >
> > Credits-by: Sateesh Kavuri
> > Credits-by: Shobhit Kumar
> > Signed-off-by: Rodrigo Vivi
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 40 +
> > drivers/gpu/drm/i915/intel_dp.c | 183
> > +++
> > drivers/gpu/drm/i915/intel_drv.h | 3 +
> > 3 files changed, 226 insertions(+)
> >
>
> > +void intel_edp_write_vsc_psr(struct intel_dp* intel_dp,
> > +struct edp_vsc_psr *vsc_psr)
> > +{
> > + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct intel_crtc *intel_crtc =
> > to_intel_crtc(intel_dp_to_crtc(intel_dp));
> > + u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> > + u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(intel_crtc->cpu_transcoder);
> > + uint32_t *data = (uint32_t *)vsc_psr;
> > + unsigned int i;
> > + u32 val = I915_READ(ctl_reg);
> > +
> > + if (data_reg == 0)
> > + return;
> > +
> > + /* As per eDP spec, wait for vblank to send SDP VSC packet */
> > + intel_wait_for_vblank(dev, intel_crtc->pipe);
> > +
> > + mmiowb();
>
> I was curious about these mmiowb()s and apparently they were added to
> all infoframe writes "just in case". But AFAICS on x86 mmiowb() ends
> up as a compiler barrier, so this stuff seems to be a nop.
>
> And if these writes can get reordered somewhere, why not everything
> else too? I'm sure we have places where we write a bunch of registers,
> and the final write enables something which requires the earlier
> writes to have landed beforehand.
Yeah we shouldn't need mmiowb() anywhere in our driver (until our
on-chip topology gets really complicated anyway).
--
Jesse Barnes, Intel Open Source Technology Center
[PATCH] drm: Don't set the plane->fb to NULL on successfull set_plane
On Fri, 15 Feb 2013 21:21:37 +0100
Daniel Vetter wrote:
> We need to clear the local variable to get the refcounting right
> (since the reference drm_mode_setplane holds is transferred to the
> plane->fb pointer). But should be done _after_ we update the pointer.
>
> Breakage introduced in
>
> commit 6c2a75325c800de286166c693e0cd33c3a1c5ec8
> Author: Daniel Vetter
> Date: Tue Dec 11 00:59:24 2012 +0100
>
> drm: refcounting for sprite framebuffers
>
> Reported-by: Jesse Barnes
> Cc: Jesse Barnes
> Cc: Rob Clark
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 826a5ca..1960418 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1982,9 +1982,9 @@ int drm_mode_setplane(struct drm_device *dev, void
> *data,
>plane_req->src_w, plane_req->src_h);
> if (!ret) {
> old_fb = plane->fb;
> - fb = NULL;
> plane->crtc = crtc;
> plane->fb = fb;
> + fb = NULL;
> }
> drm_modeset_unlock_all(dev);
>
Reviewed-by: Jesse Barnes
This will allow us to restore the sprite config in no-VT switch resumes.
--
Jesse Barnes, Intel Open Source Technology Center
linux-next: build failure after merge of the drm-intel tree
On Fri, 15 Feb 2013 10:30:16 +0100 Daniel Vetter wrote: > On Fri, Feb 15, 2013 at 3:37 AM, Stephen Rothwell > wrote: > > Hi all, > > > > After merging the drm-intel tree, today's linux-next build (x86_64 > > allmodconfig) failed like this: > > > > ERROR: "pm_vt_switch_unregister" [drivers/video/fb.ko] undefined! > > > > I have dropped the tree for today. > > Meh, that fail was already reported from Wu's kernel builder a few > days ago, but no patch yet showed up to fix things. Since the i915 > side of that work isn't ready yet either I've dropped the offending > patches. I sent a patch yesterday for this. I'll bounce it over again. -- Jesse Barnes, Intel Open Source Technology Center
Re: linux-next: build failure after merge of the drm-intel tree
On Fri, 15 Feb 2013 10:30:16 +0100 Daniel Vetter daniel.vet...@ffwll.ch wrote: On Fri, Feb 15, 2013 at 3:37 AM, Stephen Rothwell s...@canb.auug.org.au wrote: Hi all, After merging the drm-intel tree, today's linux-next build (x86_64 allmodconfig) failed like this: ERROR: pm_vt_switch_unregister [drivers/video/fb.ko] undefined! I have dropped the tree for today. Meh, that fail was already reported from Wu's kernel builder a few days ago, but no patch yet showed up to fix things. Since the i915 side of that work isn't ready yet either I've dropped the offending patches. I sent a patch yesterday for this. I'll bounce it over again. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Don't set the plane-fb to NULL on successfull set_plane
On Fri, 15 Feb 2013 21:21:37 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:
We need to clear the local variable to get the refcounting right
(since the reference drm_mode_setplane holds is transferred to the
plane-fb pointer). But should be done _after_ we update the pointer.
Breakage introduced in
commit 6c2a75325c800de286166c693e0cd33c3a1c5ec8
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date: Tue Dec 11 00:59:24 2012 +0100
drm: refcounting for sprite framebuffers
Reported-by: Jesse Barnes jbar...@virtuousgeek.org
Cc: Jesse Barnes jbar...@virtuousgeek.org
Cc: Rob Clark r...@ti.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_crtc.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 826a5ca..1960418 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1982,9 +1982,9 @@ int drm_mode_setplane(struct drm_device *dev, void
*data,
plane_req-src_w, plane_req-src_h);
if (!ret) {
old_fb = plane-fb;
- fb = NULL;
plane-crtc = crtc;
plane-fb = fb;
+ fb = NULL;
}
drm_modeset_unlock_all(dev);
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
This will allow us to restore the sprite config in no-VT switch resumes.
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] man: Fix typo and use $() for make expressions
On Fri, 25 Jan 2013 16:54:11 +0100
David Herrmann wrote:
> Hi Jesse
>
> On Fri, Jan 18, 2013 at 5:54 PM, Jesse Barnes
> wrote:
> > On Fri, 18 Jan 2013 17:01:59 +0100
> > David Herrmann wrote:
> >
> >> On Fri, Jan 18, 2013 at 5:00 PM, David Herrmann
> >> wrote:
> >> > Hi Thierry
> >> >
> >> > On Fri, Jan 18, 2013 at 1:22 PM, Thierry Reding
> >> > wrote:
> >> >> Due to the typo, none of the .xml files would end up in the release
> >> >> tarball and cause make distcheck as well as builds from the tarball to
> >> >> fail.
> >> >>
> >> >> Using $() isn't strictly necessary but other variables and expressions
> >> >> use that variant already so it makes the usage consistent.
> >> >
> >> > That's weird. "make distcheck" should not be able to build the
> >> > manpages if the XML files are not available. Also ${} is pretty
> >> > standard in makefiles, isn't it? I wonder what the problem here is. At
> >> > least distcheck runs fine on my machine.
> >>
> >> Ah sorry, I now saw the "subs" => "subst" typo. Still I wonder why
> >> distcheck works here. But the patch looks fine. Thanks!
> >
> > Works here too. Pushed with David's reviewed-by. Thanks Thierry.
>
> Did you forget to push this patch? I cannot see it in upstream fdo
> libdrm repository. Or maybe I am just looking at the wrong place.
Yeah failed to push, sorry. Should be there now.
--
Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH] man: Fix typo and use $() for make expressions
On Fri, 25 Jan 2013 16:54:11 +0100
David Herrmann dh.herrm...@googlemail.com wrote:
Hi Jesse
On Fri, Jan 18, 2013 at 5:54 PM, Jesse Barnes jbar...@virtuousgeek.org
wrote:
On Fri, 18 Jan 2013 17:01:59 +0100
David Herrmann dh.herrm...@googlemail.com wrote:
On Fri, Jan 18, 2013 at 5:00 PM, David Herrmann
dh.herrm...@googlemail.com wrote:
Hi Thierry
On Fri, Jan 18, 2013 at 1:22 PM, Thierry Reding
thierry.red...@avionic-design.de wrote:
Due to the typo, none of the .xml files would end up in the release
tarball and cause make distcheck as well as builds from the tarball to
fail.
Using $() isn't strictly necessary but other variables and expressions
use that variant already so it makes the usage consistent.
That's weird. make distcheck should not be able to build the
manpages if the XML files are not available. Also ${} is pretty
standard in makefiles, isn't it? I wonder what the problem here is. At
least distcheck runs fine on my machine.
Ah sorry, I now saw the subs = subst typo. Still I wonder why
distcheck works here. But the patch looks fine. Thanks!
Works here too. Pushed with David's reviewed-by. Thanks Thierry.
Did you forget to push this patch? I cannot see it in upstream fdo
libdrm repository. Or maybe I am just looking at the wrong place.
Yeah failed to push, sorry. Should be there now.
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] man: Fix typo and use $() for make expressions
On Fri, 18 Jan 2013 17:01:59 +0100
David Herrmann wrote:
> On Fri, Jan 18, 2013 at 5:00 PM, David Herrmann
> wrote:
> > Hi Thierry
> >
> > On Fri, Jan 18, 2013 at 1:22 PM, Thierry Reding
> > wrote:
> >> Due to the typo, none of the .xml files would end up in the release
> >> tarball and cause make distcheck as well as builds from the tarball to
> >> fail.
> >>
> >> Using $() isn't strictly necessary but other variables and expressions
> >> use that variant already so it makes the usage consistent.
> >
> > That's weird. "make distcheck" should not be able to build the
> > manpages if the XML files are not available. Also ${} is pretty
> > standard in makefiles, isn't it? I wonder what the problem here is. At
> > least distcheck runs fine on my machine.
>
> Ah sorry, I now saw the "subs" => "subst" typo. Still I wonder why
> distcheck works here. But the patch looks fine. Thanks!
Works here too. Pushed with David's reviewed-by. Thanks Thierry.
--
Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH] man: Fix typo and use $() for make expressions
On Fri, 18 Jan 2013 17:01:59 +0100
David Herrmann dh.herrm...@googlemail.com wrote:
On Fri, Jan 18, 2013 at 5:00 PM, David Herrmann
dh.herrm...@googlemail.com wrote:
Hi Thierry
On Fri, Jan 18, 2013 at 1:22 PM, Thierry Reding
thierry.red...@avionic-design.de wrote:
Due to the typo, none of the .xml files would end up in the release
tarball and cause make distcheck as well as builds from the tarball to
fail.
Using $() isn't strictly necessary but other variables and expressions
use that variant already so it makes the usage consistent.
That's weird. make distcheck should not be able to build the
manpages if the XML files are not available. Also ${} is pretty
standard in makefiles, isn't it? I wonder what the problem here is. At
least distcheck runs fine on my machine.
Ah sorry, I now saw the subs = subst typo. Still I wonder why
distcheck works here. But the patch looks fine. Thanks!
Works here too. Pushed with David's reviewed-by. Thanks Thierry.
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] man: fix manpage build instructions
On Wed, 16 Jan 2013 19:35:25 +0100 David Herrmann wrote: > This fixes all the out-of-tree build-failures with manpages and uses a > .man_fixup file to avoid overriding man-pages on every build. > > Manpages are only built if xsltproc is found and the stylesheets are > available locally. You can disable building manpages with > --disable-manpages so the quite expensive xsltproc procedure can be > skipped. > > Signed-off-by: David Herrmann > --- Seems to work here, pushed. Thanks David. -- Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH] man: fix manpage build instructions
On Wed, 16 Jan 2013 19:35:25 +0100 David Herrmann dh.herrm...@googlemail.com wrote: This fixes all the out-of-tree build-failures with manpages and uses a .man_fixup file to avoid overriding man-pages on every build. Manpages are only built if xsltproc is found and the stylesheets are available locally. You can disable building manpages with --disable-manpages so the quite expensive xsltproc procedure can be skipped. Signed-off-by: David Herrmann dh.herrm...@googlemail.com --- Seems to work here, pushed. Thanks David. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 0/4] Manpages for libdrm
On Thu, 10 Jan 2013 22:00:20 +0100 David Herrmann wrote: > Hi Jesse > > On Thu, Jan 10, 2013 at 1:22 AM, Jesse Barnes > wrote: > > On Fri, 28 Sep 2012 23:44:18 +0200 > > David Herrmann wrote: > > > >> Hi > >> > >> This is revision 2 of the manpages for libdrm. I converted everything to > >> docbook > >> XML. This makes it easier to write them (troff is really hard to read) and > >> allows us to integrate it into other documentation. > >> > >> Other than that I only fixed typos and the small corrections you guys > >> mentioned. > >> Thanks for reviewing! > > > > I went ahead and pushed these finally. > > Thanks! > > > Can you just apply for an fdo account though so we can let you push > > things in the future? :) > > I don't have a signed ssh key but I guess I can get it signed by some > people at FOSDEM13. I will then apply for an account afterwards. > > I also have some cleanup patches for the man-page build here that > should be applied. I actually don't know why it fails on your machine > but my rework should build them properly. I will send them shortly. Great, thanks. See you there! -- Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH libdrm 0/4] Manpages for libdrm
On Thu, 10 Jan 2013 22:00:20 +0100 David Herrmann dh.herrm...@googlemail.com wrote: Hi Jesse On Thu, Jan 10, 2013 at 1:22 AM, Jesse Barnes jbar...@virtuousgeek.org wrote: On Fri, 28 Sep 2012 23:44:18 +0200 David Herrmann dh.herrm...@googlemail.com wrote: Hi This is revision 2 of the manpages for libdrm. I converted everything to docbook XML. This makes it easier to write them (troff is really hard to read) and allows us to integrate it into other documentation. Other than that I only fixed typos and the small corrections you guys mentioned. Thanks for reviewing! I went ahead and pushed these finally. Thanks! Can you just apply for an fdo account though so we can let you push things in the future? :) I don't have a signed ssh key but I guess I can get it signed by some people at FOSDEM13. I will then apply for an account afterwards. I also have some cleanup patches for the man-page build here that should be applied. I actually don't know why it fails on your machine but my rework should build them properly. I will send them shortly. Great, thanks. See you there! -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 0/4] Manpages for libdrm
On Fri, 28 Sep 2012 23:44:18 +0200 David Herrmann wrote: > Hi > > This is revision 2 of the manpages for libdrm. I converted everything to > docbook > XML. This makes it easier to write them (troff is really hard to read) and > allows us to integrate it into other documentation. > > Other than that I only fixed typos and the small corrections you guys > mentioned. > Thanks for reviewing! I went ahead and pushed these finally. Can you just apply for an fdo account though so we can let you push things in the future? :) -- Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH libdrm 0/4] Manpages for libdrm
On Fri, 28 Sep 2012 23:44:18 +0200 David Herrmann dh.herrm...@googlemail.com wrote: Hi This is revision 2 of the manpages for libdrm. I converted everything to docbook XML. This makes it easier to write them (troff is really hard to read) and allows us to integrate it into other documentation. Other than that I only fixed typos and the small corrections you guys mentioned. Thanks for reviewing! I went ahead and pushed these finally. Can you just apply for an fdo account though so we can let you push things in the future? :) -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] [PATCH 04/37] drm/i915: rework locking for intel_dpio|sbi_read|write
On Wed, 12 Dec 2012 23:00:34 +0100 Daniel Vetter wrote: > On Wed, Dec 12, 2012 at 12:54:47PM -0800, Jesse Barnes wrote: > > On Wed, 12 Dec 2012 14:06:44 +0100 > > Daniel Vetter wrote: > > > > > Spinning for up to 200 us with interrupts locked out is not good. So > > > let's just spin (and even that seems to be excessive). > > > > > > And we don't call these functions from interrupt context, so this is > > > not required. Besides that doing anything in interrupt contexts which > > > might take a few hundred us is a no-go. So just convert the entire > > > thing to a mutex. Also move the mutex-grabbing out of the read/write > > > functions (add a WARN_ON(!is_locked)) instead) since all callers are > > > nicely grouped together. > > > > > > Finally the real motivation for this change: Dont grab the modeset > > > mutex in the dpio debugfs file, we don't need that consistency. And > > > correctness of the dpio interface is ensured with the dpio_lock. > > > > > > Signed-off-by: Daniel Vetter > > > > Looks fine, I guess you could convert the wait_for_atomic_us to the > > non-atomic variant now that you have a mutex. Either way: > > I like that _us purely to document the correct timeout ... Yeah we'd need a non-atomic _us variant to clean it up. No biggie. -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 04/37] drm/i915: rework locking for intel_dpio|sbi_read|write
KT) & DPIO_BUSY) == 0, 100)) {
> DRM_ERROR("DPIO idle wait timed out\n");
> - goto out_unlock;
> + return;
> }
>
> I915_WRITE(DPIO_DATA, val);
> @@ -456,9 +450,6 @@ static void intel_dpio_write(struct drm_i915_private
> *dev_priv, int reg,
> DPIO_BYTE);
> if (wait_for_atomic_us((I915_READ(DPIO_PKT) & DPIO_BUSY) == 0, 100))
> DRM_ERROR("DPIO write wait timed out\n");
> -
> -out_unlock:
> - spin_unlock_irqrestore(_priv->dpio_lock, flags);
> }
>
> static void vlv_init_dpio(struct drm_device *dev)
> @@ -1455,13 +1446,12 @@ static void intel_disable_pll(struct drm_i915_private
> *dev_priv, enum pipe pipe)
> static void
> intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value)
> {
> - unsigned long flags;
> + WARN_ON(!mutex_is_locked(_priv->dpio_lock));
>
> - spin_lock_irqsave(_priv->dpio_lock, flags);
> if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
> 100)) {
> DRM_ERROR("timeout waiting for SBI to become ready\n");
> - goto out_unlock;
> + return;
> }
>
> I915_WRITE(SBI_ADDR,
> @@ -1475,24 +1465,19 @@ intel_sbi_write(struct drm_i915_private *dev_priv,
> u16 reg, u32 value)
> if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL))
> == 0,
> 100)) {
> DRM_ERROR("timeout waiting for SBI to complete write
> transaction\n");
> - goto out_unlock;
> + return;
> }
> -
> -out_unlock:
> - spin_unlock_irqrestore(_priv->dpio_lock, flags);
> }
>
> static u32
> intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg)
> {
> - unsigned long flags;
> - u32 value = 0;
> + WARN_ON(!mutex_is_locked(_priv->dpio_lock));
>
> - spin_lock_irqsave(_priv->dpio_lock, flags);
> if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
> 100)) {
> DRM_ERROR("timeout waiting for SBI to become ready\n");
> - goto out_unlock;
> + return 0;
> }
>
> I915_WRITE(SBI_ADDR,
> @@ -1504,14 +1489,10 @@ intel_sbi_read(struct drm_i915_private *dev_priv, u16
> reg)
> if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL))
> == 0,
> 100)) {
> DRM_ERROR("timeout waiting for SBI to complete read
> transaction\n");
> - goto out_unlock;
> + return 0;
> }
>
> - value = I915_READ(SBI_DATA);
> -
> -out_unlock:
> - spin_unlock_irqrestore(_priv->dpio_lock, flags);
> - return value;
> + return I915_READ(SBI_DATA);
> }
>
> /**
> @@ -2960,6 +2941,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
> u32 divsel, phaseinc, auxdiv, phasedir = 0;
> u32 temp;
>
> + mutex_lock(_priv->dpio_lock);
> +
> /* It is necessary to ungate the pixclk gate prior to programming
>* the divisors, and gate it back when it is done.
>*/
> @@ -3041,6 +3024,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
> udelay(24);
>
> I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_UNGATE);
> +
> + mutex_unlock(_priv->dpio_lock);
> }
>
> /*
> @@ -4268,6 +4253,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> bool is_sdvo;
> u32 temp;
>
> + mutex_lock(_priv->dpio_lock);
> +
> is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
>
> @@ -4351,6 +4338,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> temp |= (1 << 21);
> intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL2, temp);
> }
> +
> + mutex_unlock(_priv->dpio_lock);
> }
>
> static void i9xx_update_pll(struct drm_crtc *crtc,
Looks fine, I guess you could convert the wait_for_atomic_us to the
non-atomic variant now that you have a mutex. Either way:
Reviewed-by: Jesse Barnes
--
Jesse Barnes, Intel Open Source Technology Center
Re: [Intel-gfx] [PATCH 04/37] drm/i915: rework locking for intel_dpio|sbi_read|write
(dev_priv-dpio_lock, flags);
}
static void vlv_init_dpio(struct drm_device *dev)
@@ -1455,13 +1446,12 @@ static void intel_disable_pll(struct drm_i915_private
*dev_priv, enum pipe pipe)
static void
intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value)
{
- unsigned long flags;
+ WARN_ON(!mutex_is_locked(dev_priv-dpio_lock));
- spin_lock_irqsave(dev_priv-dpio_lock, flags);
if (wait_for((I915_READ(SBI_CTL_STAT) SBI_BUSY) == 0,
100)) {
DRM_ERROR(timeout waiting for SBI to become ready\n);
- goto out_unlock;
+ return;
}
I915_WRITE(SBI_ADDR,
@@ -1475,24 +1465,19 @@ intel_sbi_write(struct drm_i915_private *dev_priv,
u16 reg, u32 value)
if (wait_for((I915_READ(SBI_CTL_STAT) (SBI_BUSY | SBI_RESPONSE_FAIL))
== 0,
100)) {
DRM_ERROR(timeout waiting for SBI to complete write
transaction\n);
- goto out_unlock;
+ return;
}
-
-out_unlock:
- spin_unlock_irqrestore(dev_priv-dpio_lock, flags);
}
static u32
intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg)
{
- unsigned long flags;
- u32 value = 0;
+ WARN_ON(!mutex_is_locked(dev_priv-dpio_lock));
- spin_lock_irqsave(dev_priv-dpio_lock, flags);
if (wait_for((I915_READ(SBI_CTL_STAT) SBI_BUSY) == 0,
100)) {
DRM_ERROR(timeout waiting for SBI to become ready\n);
- goto out_unlock;
+ return 0;
}
I915_WRITE(SBI_ADDR,
@@ -1504,14 +1489,10 @@ intel_sbi_read(struct drm_i915_private *dev_priv, u16
reg)
if (wait_for((I915_READ(SBI_CTL_STAT) (SBI_BUSY | SBI_RESPONSE_FAIL))
== 0,
100)) {
DRM_ERROR(timeout waiting for SBI to complete read
transaction\n);
- goto out_unlock;
+ return 0;
}
- value = I915_READ(SBI_DATA);
-
-out_unlock:
- spin_unlock_irqrestore(dev_priv-dpio_lock, flags);
- return value;
+ return I915_READ(SBI_DATA);
}
/**
@@ -2960,6 +2941,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
u32 divsel, phaseinc, auxdiv, phasedir = 0;
u32 temp;
+ mutex_lock(dev_priv-dpio_lock);
+
/* It is necessary to ungate the pixclk gate prior to programming
* the divisors, and gate it back when it is done.
*/
@@ -3041,6 +3024,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
udelay(24);
I915_WRITE(PIXCLK_GATE, PIXCLK_GATE_UNGATE);
+
+ mutex_unlock(dev_priv-dpio_lock);
}
/*
@@ -4268,6 +4253,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
bool is_sdvo;
u32 temp;
+ mutex_lock(dev_priv-dpio_lock);
+
is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
@@ -4351,6 +4338,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
temp |= (1 21);
intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL2, temp);
}
+
+ mutex_unlock(dev_priv-dpio_lock);
}
static void i9xx_update_pll(struct drm_crtc *crtc,
Looks fine, I guess you could convert the wait_for_atomic_us to the
non-atomic variant now that you have a mutex. Either way:
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 04/37] drm/i915: rework locking for intel_dpio|sbi_read|write
On Wed, 12 Dec 2012 23:00:34 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Wed, Dec 12, 2012 at 12:54:47PM -0800, Jesse Barnes wrote: On Wed, 12 Dec 2012 14:06:44 +0100 Daniel Vetter daniel.vet...@ffwll.ch wrote: Spinning for up to 200 us with interrupts locked out is not good. So let's just spin (and even that seems to be excessive). And we don't call these functions from interrupt context, so this is not required. Besides that doing anything in interrupt contexts which might take a few hundred us is a no-go. So just convert the entire thing to a mutex. Also move the mutex-grabbing out of the read/write functions (add a WARN_ON(!is_locked)) instead) since all callers are nicely grouped together. Finally the real motivation for this change: Dont grab the modeset mutex in the dpio debugfs file, we don't need that consistency. And correctness of the dpio interface is ensured with the dpio_lock. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Looks fine, I guess you could convert the wait_for_atomic_us to the non-atomic variant now that you have a mutex. Either way: I like that _us purely to document the correct timeout ... Yeah we'd need a non-atomic _us variant to clean it up. No biggie. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: Make the HPD status updates debug logs more readable
On Mon, 10 Dec 2012 20:24:23 +
Damien Lespiau wrote:
> From: Damien Lespiau
>
> Instead of just printing "status updated from 1 to 2", make those enum
> numbers immediately readable.
>
> v2: Also patch output_poll_execute() (Daniel Vetter)
>
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/drm_crtc_helper.c | 22 ++
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c
> index 1fe719f..524f0d3 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -950,6 +950,18 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
>
> +static const char *connector_status_str(enum drm_connector_status status)
> +{
> + switch (status) {
> + case connector_status_connected:
> + return "connected";
> + case connector_status_disconnected:
> + return "disconnected";
> + default:
> + return "unknown";
> + }
> +}
> +
> #define DRM_OUTPUT_POLL_PERIOD (10*HZ)
> static void output_poll_execute(struct work_struct *work)
> {
> @@ -984,10 +996,11 @@ static void output_poll_execute(struct work_struct
> *work)
> continue;
>
> connector->status = connector->funcs->detect(connector, false);
> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to
> %d\n",
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
> %s\n",
> connector->base.id,
> drm_get_connector_name(connector),
> - old_status, connector->status);
> + connector_status_str(old_status),
> + connector_status_str(connector->status));
> if (old_status != connector->status)
> changed = true;
> }
> @@ -1062,10 +1075,11 @@ void drm_helper_hpd_irq_event(struct drm_device *dev)
> old_status = connector->status;
>
> connector->status = connector->funcs->detect(connector, false);
> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to
> %d\n",
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
> %s\n",
> connector->base.id,
> drm_get_connector_name(connector),
> - old_status, connector->status);
> + connector_status_str(old_status),
> + connector_status_str(connector->status));
> if (old_status != connector->status)
> changed = true;
> }
Yeah, thanks.
Reviewed-by: Jesse Barnes
--
Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH 1/2] drm: Make the HPD status updates debug logs more readable
On Mon, 10 Dec 2012 20:24:23 +
Damien Lespiau damien.lesp...@gmail.com wrote:
From: Damien Lespiau damien.lesp...@intel.com
Instead of just printing status updated from 1 to 2, make those enum
numbers immediately readable.
v2: Also patch output_poll_execute() (Daniel Vetter)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/drm_crtc_helper.c | 22 ++
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index 1fe719f..524f0d3 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -950,6 +950,18 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
+static const char *connector_status_str(enum drm_connector_status status)
+{
+ switch (status) {
+ case connector_status_connected:
+ return connected;
+ case connector_status_disconnected:
+ return disconnected;
+ default:
+ return unknown;
+ }
+}
+
#define DRM_OUTPUT_POLL_PERIOD (10*HZ)
static void output_poll_execute(struct work_struct *work)
{
@@ -984,10 +996,11 @@ static void output_poll_execute(struct work_struct
*work)
continue;
connector-status = connector-funcs-detect(connector, false);
- DRM_DEBUG_KMS([CONNECTOR:%d:%s] status updated from %d to
%d\n,
+ DRM_DEBUG_KMS([CONNECTOR:%d:%s] status updated from %s to
%s\n,
connector-base.id,
drm_get_connector_name(connector),
- old_status, connector-status);
+ connector_status_str(old_status),
+ connector_status_str(connector-status));
if (old_status != connector-status)
changed = true;
}
@@ -1062,10 +1075,11 @@ void drm_helper_hpd_irq_event(struct drm_device *dev)
old_status = connector-status;
connector-status = connector-funcs-detect(connector, false);
- DRM_DEBUG_KMS([CONNECTOR:%d:%s] status updated from %d to
%d\n,
+ DRM_DEBUG_KMS([CONNECTOR:%d:%s] status updated from %s to
%s\n,
connector-base.id,
drm_get_connector_name(connector),
- old_status, connector-status);
+ connector_status_str(old_status),
+ connector_status_str(connector-status));
if (old_status != connector-status)
changed = true;
}
Yeah, thanks.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
