[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
[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 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
[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 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 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(&connector->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(&connector->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(&encoder->crtc->hwmode, &to_intel_crtc(encoder->crtc)->config); - modes[i] = &encoder->crtc->hwmode; + modes[i] = &encoder->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 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 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(&dev->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 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
[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
[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
[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 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
[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
[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 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
[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
Re: [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 ___ 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 wrote: > On Thu, Sep 5, 2013 at 3:41 AM, Dave Airlie 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
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 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 > > > --- > > > 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
[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 = &dev->mode_config; - struct intel_encoder *encoder; - - mutex_lock(&mode_config->mutex); - DRM_DEBUG_KMS("running encoder hotplug functions\n"); - - list_for_each_entry(encoder, &mode_config->encoder_list, base.head) - if (encoder->hot_plug) - encoder->hot_plug(encoder); - - mutex_unlock(&mode_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 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 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
[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
[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(&fb_helper->kernel_fb_list)) { >>> list_del(&fb_helper->kernel_fb_list); >>> if (list_empty(&kernel_fb_helper_list)) { >>> - pr_info("drm: unregistered panic notifier\n"); >>> - atomic_notifier_chain_unregister(&panic_notifier_list, >>> -&paniced); >>> unregister_sysrq_key('v', >>> &sysrq_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(&kernel_fb_helper_list)) { >>> - dev_info(fb_helper->dev->dev, "registered panic notifier\n"); >>> - atomic_notifier_chain_register(&panic_notifier_list, >>> - &paniced); >>> register_sysrq_key('v', &sysrq_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 v2 12/22] drm/i915: Preserve SSC earlier
it will work even if the VBT > - * 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
[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 = > >> >> &dp_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
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
[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] 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
[drm-intel:drm-intel-fixes 15/18] drivers/gpu/drm/i915/intel_ringbuffer.c:1405:24: error: 'I915_GEM_SCRATCH_INDEX' undeclared
On Wed, 3 Oct 2012 17:12:18 +0200 Daniel Vetter wrote: > On Wed, Oct 03, 2012 at 10:15:38PM +0800, Fengguang Wu wrote: > > Hi Jesse, > > > > FYI, kernel build failed on > > > > tree: git://people.freedesktop.org/~danvet/drm-intel.git drm-intel-fixes > > head: 2cb47a9c6d49f2961dcf57f69ff968e65d48e876 > > commit: 307daa1b856699f1d4acc3972374017c3e99d331 [15/18] drm/i915: TLB > > invalidation with MI_FLUSH_DW requires a post-sync op > > config: x86_64-allyesconfig > > Thanks for the report, should be fixed now - somehow I've failed to notice > that my .config has gone awry. No, this is because this patch depends on the earlier patch to add the GEM_SCRATCH_INDEX to i915_drv.h, which wasn't included in this series. Did you pull it in from the earlier series? Or fix it up some other way that caused the breakage you saw? Jesse
[PATCH] drm: reduce default drm vblank off delay to 50ms
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 --- drivers/gpu/drm/drm_stub.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index c236fd2..a1cbc0e 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -40,7 +40,7 @@ unsigned int drm_debug = 0;/* 1 to enable debug output */ EXPORT_SYMBOL(drm_debug); -unsigned int drm_vblank_offdelay = 5000;/* Default to 5000 msecs. */ +unsigned int drm_vblank_offdelay = 50;/* Default to 50 msecs. */ EXPORT_SYMBOL(drm_vblank_offdelay); unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */ -- 1.7.9.5
[PATCH] drm: reduce default drm vblank off delay to 50ms
Cc'ing Mario in case he wants a different value than 50ms. On Tue, 30 Oct 2012 14:09:12 -0500 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 > --- > drivers/gpu/drm/drm_stub.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c > index c236fd2..a1cbc0e 100644 > --- a/drivers/gpu/drm/drm_stub.c > +++ b/drivers/gpu/drm/drm_stub.c > @@ -40,7 +40,7 @@ > unsigned int drm_debug = 0; /* 1 to enable debug output */ > EXPORT_SYMBOL(drm_debug); > > -unsigned int drm_vblank_offdelay = 5000;/* Default to 5000 msecs. */ > +unsigned int drm_vblank_offdelay = 50;/* Default to 50 msecs. */ > EXPORT_SYMBOL(drm_vblank_offdelay); > > unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */ -- Jesse Barnes, Intel Open Source Technology Center
[PATCH] drm: reduce default drm vblank off delay to 50ms
On Tue, 30 Oct 2012 20:20:44 +0100 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 > > Nacked-by: Daniel Vetter > > Also discussed with Jesse on irc, we've had fun ;-) That's ridiculous. Just because we have a race we can't fix wrt reading hw regs, doesn't mean we can't reduce the timeout. I nack your nack. -- Jesse Barnes, Intel Open Source Technology Center
[PATCH] drm/fb helper: don't call drm_helper_connector_dpms directly
On Fri, 7 Sep 2012 10:14:52 +0200 Daniel Vetter wrote: > Yet again a case where the fb helper is too intimate with the crtc > helper and calls a crtc helepr function directly instead of going > through the interface vtable. > > This fixes console blanking in drm/i915 with the new i915-specific > modeset code. > > Reported-by: Jesse Barnes > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_fb_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index eb79515..b5d05f5 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -330,7 +330,7 @@ static void drm_fb_helper_dpms(struct fb_info *info, int > dpms_mode) > /* Walk the connectors & encoders on this fb turning them > on/off */ > for (j = 0; j < fb_helper->connector_count; j++) { > connector = fb_helper->connector_info[j]->connector; > - drm_helper_connector_dpms(connector, dpms_mode); > + connector->funcs->dpms(connector, dpms_mode); > drm_connector_property_set_value(connector, > dev->mode_config.dpms_property, dpms_mode); > } Tested-by: Jesse Barnes -- Jesse Barnes, Intel Open Source Technology Center
[RFC 1/9] drm: add atomic fxns
On Sun, 9 Sep 2012 22:03:14 -0500 Rob Clark wrote: > From: Rob Clark > > The 'atomic' mechanism allows for multiple properties to be updated, > checked, and commited atomically. This will be the basis of atomic- > modeset and nuclear-pageflip. > > The basic flow is: > >state = dev->atomic_begin(); >for (... one or more ...) > obj->set_property(obj, state, prop, value); >if (dev->atomic_check(state)) > dev->atomic_commit(state, event); >dev->atomic_end(state); I think the above is more suited to drm_crtc_helper code. I think the top level callback should contain the arrays and be a single "multi flip" hook (or maybe a check then set double callback). For some drivers that'll end up being a lot simpler, rather than sprinkling atomic handling code in all the set_property callbacks. Having a transactional API just seems a little messier with both the atomic state and per-property state to track and rollback in the case of failure. -- Jesse Barnes, Intel Open Source Technology Center
[RFC 1/9] drm: add atomic fxns
On Wed, 12 Sep 2012 12:35:01 -0500 Rob Clark wrote: > On Wed, Sep 12, 2012 at 11:57 AM, Jesse Barnes > wrote: > > On Sun, 9 Sep 2012 22:03:14 -0500 > > Rob Clark wrote: > > > >> From: Rob Clark > >> > >> The 'atomic' mechanism allows for multiple properties to be updated, > >> checked, and commited atomically. This will be the basis of atomic- > >> modeset and nuclear-pageflip. > >> > >> The basic flow is: > >> > >>state = dev->atomic_begin(); > >>for (... one or more ...) > >> obj->set_property(obj, state, prop, value); > >>if (dev->atomic_check(state)) > >> dev->atomic_commit(state, event); > >>dev->atomic_end(state); > > > > I think the above is more suited to drm_crtc_helper code. I think the > > top level callback should contain the arrays and be a single "multi > > flip" hook (or maybe a check then set double callback). For some > > drivers that'll end up being a lot simpler, rather than sprinkling > > atomic handling code in all the set_property callbacks. > > well, there are a few other places in drm_crtc.c where I want to use > the new API, to avoid drivers having to support both atomic API and > old set_plane/page_flip stuff.. the transactional API makes that a bit > easier, I think.. or at least I don't have to construct an array on > the stack. > > But I'm not entirely convinced that it is a problem.. with > drm_{crtc,plane,etc}_state, it is just building up a set of values in > a state struct, and that state struct gets atomically committed. Yeah I know it can work this way, it just seemed like the begin, end, and set_property callbacks might be unnecessary if the props were all part of the state. The driver call roll things back (or just not touch hw) if the check or commit fails... I guess ultimately, given the choice, I'd rather have the drivers calling into helper functions where possible, rather than having the core impose a specific set of semi-fine grained hooks. > > Having a transactional API just seems a little messier with both the > > atomic state and per-property state to track and rollback in the case > > of failure. > > For the rollback, I think I'm just going to move the array of property > values into drm_{crtc,plane,etc}_state. That way, userspace doesn't > see updated property values if they are not committed. It makes the > property value rollback automatic. Ok that seems reasonable. -- Jesse Barnes, Intel Open Source Technology Center
[dri-devel] [RFC] Try a bit harder to get output on the screen at panic time
This set of 3 patches makes it a little more likely we'll get panic output onto the screen even when X is running, assuming a KMS enabled stack anyway. It gets me from a blank or very sparsely populated black screen at panic time, to one including the full backtrace and panic output at panic time (tested with "echo c > /proc/sysrq-trigger" from an X session). It doesn't cover every case; for instance I think it'll fail when X has disabled the display, but those cases need to be handled with separate patches anyway (need to add atomic DPMS paths for instance). Anyway, please test these out and let me know if they work for you. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[dri-devel] [PATCH] drm: add locked variant of drm_fb_helper_force_kernel_mode
Needed for panic and kdb, since we need to avoid taking the mode_config mutex. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/drm_fb_helper.c | 42 +- 1 files changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 6929f5b..962eadb 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -242,18 +242,22 @@ static int drm_fb_helper_parse_command_line(struct drm_fb_helper *fb_helper) return 0; } -bool drm_fb_helper_force_kernel_mode(void) +bool drm_fb_helper_force_kernel_mode_locked(void) { int i = 0; bool ret, error = false; struct drm_fb_helper *helper; - - if (list_empty(&kernel_fb_helper_list)) - return false; + struct drm_mode_set *mode_set; + struct drm_crtc *crtc; list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) { for (i = 0; i < helper->crtc_count; i++) { - struct drm_mode_set *mode_set = &helper->crtc_info[i].mode_set; + mode_set = &helper->crtc_info[i].mode_set; + crtc = helper->crtc_info[i].mode_set.crtc; + + if (!crtc->enabled) + continue; + ret = drm_crtc_helper_set_config(mode_set); if (ret) error = true; @@ -262,11 +266,37 @@ bool drm_fb_helper_force_kernel_mode(void) return error; } +bool drm_fb_helper_force_kernel_mode(void) +{ + bool ret; + struct drm_device *dev; + struct drm_fb_helper *helper; + struct drm_mode_set *mode_set; + + if (list_empty(&kernel_fb_helper_list)) { + DRM_DEBUG_KMS("no fb helper list??\n"); + return false; + } + + /* Get the DRM device */ + helper = list_first_entry(&kernel_fb_helper_list, + struct drm_fb_helper, + kernel_fb_list); + mode_set = &helper->crtc_info[0].mode_set; + dev = mode_set->crtc->dev; + + mutex_lock(&dev->mode_config.mutex); + ret = drm_fb_helper_force_kernel_mode_locked(); + mutex_unlock(&dev->mode_config.mutex); + + return ret; +} + int drm_fb_helper_panic(struct notifier_block *n, unsigned long ununsed, void *panic_str) { DRM_ERROR("panic occurred, switching back to text console\n"); - return drm_fb_helper_force_kernel_mode(); + drm_fb_helper_force_kernel_mode_locked(); return 0; } EXPORT_SYMBOL(drm_fb_helper_panic); -- 1.6.6.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[dri-devel] [PATCH] fbcon: assume console is active if panicing
This allows us to draw to the fbcon buffer in a panic situation, in case the low level driver can flip to it at panic time. Signed-off-by: Jesse Barnes --- drivers/video/console/fbcon.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c index b0a3fa0..6ca1051 100644 --- a/drivers/video/console/fbcon.c +++ b/drivers/video/console/fbcon.c @@ -283,7 +283,7 @@ static inline int fbcon_is_inactive(struct vc_data *vc, struct fb_info *info) struct fbcon_ops *ops = info->fbcon_par; return (info->state != FBINFO_STATE_RUNNING || - vc->vc_mode != KD_TEXT || ops->graphics); + vc->vc_mode != KD_TEXT || ops->graphics) && !oops_in_progress; } static inline int get_color(struct vc_data *vc, struct fb_info *info, -- 1.6.6.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[dri-devel] [PATCH] vt: try harder to print output when panicing
At panic time (i.e. when oops_in_progress is set) we should try a bit harder to update the screen and make sure output gets to the VT, since some drivers are capable of flipping back to it. So make sure we try to unblank and update the display if called from a panic context. Signed-off-by: Jesse Barnes --- drivers/char/vt.c | 12 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/char/vt.c b/drivers/char/vt.c index bd1d116..29ec1c9 100644 --- a/drivers/char/vt.c +++ b/drivers/char/vt.c @@ -698,7 +698,10 @@ void redraw_screen(struct vc_data *vc, int is_switch) update_attr(vc); clear_buffer_attributes(vc); } - if (update && vc->vc_mode != KD_GRAPHICS) + + /* Forcibly update if we're panicing */ + if ((update && vc->vc_mode != KD_GRAPHICS) || + oops_in_progress) do_update_region(vc, vc->vc_origin, vc->vc_screenbuf_size / 2); } set_cursor(vc); @@ -2498,7 +2501,7 @@ static void vt_console_print(struct console *co, const char *b, unsigned count) goto quit; } - if (vc->vc_mode != KD_TEXT) + if (vc->vc_mode != KD_TEXT && !oops_in_progress) goto quit; /* undraw cursor first */ @@ -3703,7 +3706,8 @@ void do_unblank_screen(int leaving_gfx) return; } vc = vc_cons[fg_console].d; - if (vc->vc_mode != KD_TEXT) + /* Try to unblank in oops case too */ + if (vc->vc_mode != KD_TEXT && !oops_in_progress) return; /* but leave console_blanked != 0 */ if (blankinterval) { @@ -3712,7 +3716,7 @@ void do_unblank_screen(int leaving_gfx) } console_blanked = 0; - if (vc->vc_sw->con_blank(vc, 0, leaving_gfx)) + if (vc->vc_sw->con_blank(vc, 0, leaving_gfx) || oops_in_progress) /* Low-level driver cannot restore -> do it ourselves */ update_screen(vc); if (console_blank_hook) -- 1.6.6.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add locked variant of drm_fb_helper_force_kernel_mode
On Mon, 12 Apr 2010 10:05:00 +1000 Dave Airlie wrote: > On Sat, Apr 10, 2010 at 8:11 AM, Jesse Barnes > wrote: > > Needed for panic and kdb, since we need to avoid taking the mode_config > > mutex. > > One comment below. > > > > > Signed-off-by: Jesse Barnes > > --- > > drivers/gpu/drm/drm_fb_helper.c | 42 > > +- > > 1 files changed, 36 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > > b/drivers/gpu/drm/drm_fb_helper.c > > index 6929f5b..962eadb 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -242,18 +242,22 @@ static int drm_fb_helper_parse_command_line(struct > > drm_fb_helper *fb_helper) > > return 0; > > } > > > > -bool drm_fb_helper_force_kernel_mode(void) > > +bool drm_fb_helper_force_kernel_mode_locked(void) > > { > > int i = 0; > > bool ret, error = false; > > struct drm_fb_helper *helper; > > - > > - if (list_empty(&kernel_fb_helper_list)) > > - return false; > > + struct drm_mode_set *mode_set; > > + struct drm_crtc *crtc; > > > > list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) { > > for (i = 0; i < helper->crtc_count; i++) { > > - struct drm_mode_set *mode_set = > > &helper->crtc_info[i].mode_set; > > + mode_set = &helper->crtc_info[i].mode_set; > > + crtc = helper->crtc_info[i].mode_set.crtc; > > + > > + if (!crtc->enabled) > > + continue; > > + > > ret = drm_crtc_helper_set_config(mode_set); > > if (ret) > > error = true; > > @@ -262,11 +266,37 @@ bool drm_fb_helper_force_kernel_mode(void) > > return error; > > } > > > > +bool drm_fb_helper_force_kernel_mode(void) > > +{ > > + bool ret; > > + struct drm_device *dev; > > + struct drm_fb_helper *helper; > > + struct drm_mode_set *mode_set; > > + > > + if (list_empty(&kernel_fb_helper_list)) { > > + DRM_DEBUG_KMS("no fb helper list??\n"); > > + return false; > > + } > > + > > + /* Get the DRM device */ > > + helper = list_first_entry(&kernel_fb_helper_list, > > + struct drm_fb_helper, > > + kernel_fb_list); > > + mode_set = &helper->crtc_info[0].mode_set; > > + dev = mode_set->crtc->dev; > > + > > + mutex_lock(&dev->mode_config.mutex); > > + ret = drm_fb_helper_force_kernel_mode_locked(); > > + mutex_unlock(&dev->mode_config.mutex); > > + > > + return ret; > > +} > > + > > int drm_fb_helper_panic(struct notifier_block *n, unsigned long ununsed, > > void *panic_str) > > { > > DRM_ERROR("panic occurred, switching back to text console\n"); > > - return drm_fb_helper_force_kernel_mode(); > > + drm_fb_helper_force_kernel_mode_locked(); > > Any reason to drop the return here? > > not just remove the return 0? Oh no; I don't think the return value is checked anywhere but we may as well return it anyway. -- 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: add locked variant of drm_fb_helper_force_kernel_mode
On Mon, 12 Apr 2010 10:05:00 +1000 Dave Airlie wrote: > On Sat, Apr 10, 2010 at 8:11 AM, Jesse Barnes > wrote: > > Needed for panic and kdb, since we need to avoid taking the mode_config > > mutex. > > One comment below. Updated patch below. -- Jesse Barnes, Intel Open Source Technology Center >From 7e972aa6a2f432b8fd04cb5d1a51f4df2fddea62 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Thu, 8 Apr 2010 14:40:27 -0700 Subject: [PATCH] drm: add locked variant of drm_fb_helper_force_kernel_mode Needed for panic and kdb, since we need to avoid taking the mode_config mutex. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/drm_fb_helper.c | 43 -- 1 files changed, 36 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 6929f5b..be0b95a 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -242,18 +242,22 @@ static int drm_fb_helper_parse_command_line(struct drm_fb_helper *fb_helper) return 0; } -bool drm_fb_helper_force_kernel_mode(void) +bool drm_fb_helper_force_kernel_mode_locked(void) { int i = 0; bool ret, error = false; struct drm_fb_helper *helper; - - if (list_empty(&kernel_fb_helper_list)) - return false; + struct drm_mode_set *mode_set; + struct drm_crtc *crtc; list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) { for (i = 0; i < helper->crtc_count; i++) { - struct drm_mode_set *mode_set = &helper->crtc_info[i].mode_set; + mode_set = &helper->crtc_info[i].mode_set; + crtc = helper->crtc_info[i].mode_set.crtc; + + if (!crtc->enabled) + continue; + ret = drm_crtc_helper_set_config(mode_set); if (ret) error = true; @@ -262,12 +266,37 @@ bool drm_fb_helper_force_kernel_mode(void) return error; } +bool drm_fb_helper_force_kernel_mode(void) +{ + bool ret; + struct drm_device *dev; + struct drm_fb_helper *helper; + struct drm_mode_set *mode_set; + + if (list_empty(&kernel_fb_helper_list)) { + DRM_DEBUG_KMS("no fb helper list??\n"); + return false; + } + + /* Get the DRM device */ + helper = list_first_entry(&kernel_fb_helper_list, + struct drm_fb_helper, + kernel_fb_list); + mode_set = &helper->crtc_info[0].mode_set; + dev = mode_set->crtc->dev; + + mutex_lock(&dev->mode_config.mutex); + ret = drm_fb_helper_force_kernel_mode_locked(); + mutex_unlock(&dev->mode_config.mutex); + + return ret; +} + int drm_fb_helper_panic(struct notifier_block *n, unsigned long ununsed, void *panic_str) { DRM_ERROR("panic occurred, switching back to text console\n"); - return drm_fb_helper_force_kernel_mode(); - return 0; + return drm_fb_helper_force_kernel_mode_locked(); } EXPORT_SYMBOL(drm_fb_helper_panic); -- 1.6.6.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [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 ___ 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
iv->dpio_lock)); > > - spin_lock_irqsave(&dev_priv->dpio_lock, flags); > if (wait_for_atomic_us((I915_READ(DPIO_PKT) & 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(&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 -- 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 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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fb helper: don't call drm_helper_connector_dpms directly
On Fri, 7 Sep 2012 10:14:52 +0200 Daniel Vetter wrote: > Yet again a case where the fb helper is too intimate with the crtc > helper and calls a crtc helepr function directly instead of going > through the interface vtable. > > This fixes console blanking in drm/i915 with the new i915-specific > modeset code. > > Reported-by: Jesse Barnes > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_fb_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index eb79515..b5d05f5 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -330,7 +330,7 @@ static void drm_fb_helper_dpms(struct fb_info *info, int > dpms_mode) > /* Walk the connectors & encoders on this fb turning them > on/off */ > for (j = 0; j < fb_helper->connector_count; j++) { > connector = fb_helper->connector_info[j]->connector; > - drm_helper_connector_dpms(connector, dpms_mode); > + connector->funcs->dpms(connector, dpms_mode); > drm_connector_property_set_value(connector, > dev->mode_config.dpms_property, dpms_mode); > } Tested-by: Jesse Barnes -- 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: [RFC 1/9] drm: add atomic fxns
On Sun, 9 Sep 2012 22:03:14 -0500 Rob Clark wrote: > From: Rob Clark > > The 'atomic' mechanism allows for multiple properties to be updated, > checked, and commited atomically. This will be the basis of atomic- > modeset and nuclear-pageflip. > > The basic flow is: > >state = dev->atomic_begin(); >for (... one or more ...) > obj->set_property(obj, state, prop, value); >if (dev->atomic_check(state)) > dev->atomic_commit(state, event); >dev->atomic_end(state); I think the above is more suited to drm_crtc_helper code. I think the top level callback should contain the arrays and be a single "multi flip" hook (or maybe a check then set double callback). For some drivers that'll end up being a lot simpler, rather than sprinkling atomic handling code in all the set_property callbacks. Having a transactional API just seems a little messier with both the atomic state and per-property state to track and rollback in the case of failure. -- 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: [RFC 1/9] drm: add atomic fxns
On Wed, 12 Sep 2012 12:35:01 -0500 Rob Clark wrote: > On Wed, Sep 12, 2012 at 11:57 AM, Jesse Barnes > wrote: > > On Sun, 9 Sep 2012 22:03:14 -0500 > > Rob Clark wrote: > > > >> From: Rob Clark > >> > >> The 'atomic' mechanism allows for multiple properties to be updated, > >> checked, and commited atomically. This will be the basis of atomic- > >> modeset and nuclear-pageflip. > >> > >> The basic flow is: > >> > >>state = dev->atomic_begin(); > >>for (... one or more ...) > >> obj->set_property(obj, state, prop, value); > >>if (dev->atomic_check(state)) > >> dev->atomic_commit(state, event); > >>dev->atomic_end(state); > > > > I think the above is more suited to drm_crtc_helper code. I think the > > top level callback should contain the arrays and be a single "multi > > flip" hook (or maybe a check then set double callback). For some > > drivers that'll end up being a lot simpler, rather than sprinkling > > atomic handling code in all the set_property callbacks. > > well, there are a few other places in drm_crtc.c where I want to use > the new API, to avoid drivers having to support both atomic API and > old set_plane/page_flip stuff.. the transactional API makes that a bit > easier, I think.. or at least I don't have to construct an array on > the stack. > > But I'm not entirely convinced that it is a problem.. with > drm_{crtc,plane,etc}_state, it is just building up a set of values in > a state struct, and that state struct gets atomically committed. Yeah I know it can work this way, it just seemed like the begin, end, and set_property callbacks might be unnecessary if the props were all part of the state. The driver call roll things back (or just not touch hw) if the check or commit fails... I guess ultimately, given the choice, I'd rather have the drivers calling into helper functions where possible, rather than having the core impose a specific set of semi-fine grained hooks. > > Having a transactional API just seems a little messier with both the > > atomic state and per-property state to track and rollback in the case > > of failure. > > For the rollback, I think I'm just going to move the array of property > values into drm_{crtc,plane,etc}_state. That way, userspace doesn't > see updated property values if they are not committed. It makes the > property value rollback automatic. Ok that seems reasonable. -- 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: [RFC 0/9] nuclear pageflip
On Wed, 12 Sep 2012 21:58:31 +0300 Ville Syrjälä wrote: > On Wed, Sep 12, 2012 at 01:00:19PM -0500, Clark, Rob wrote: > > On Wed, Sep 12, 2012 at 12:27 PM, Ville Syrjälä > > wrote: > > > On Wed, Sep 12, 2012 at 10:48:16AM -0500, Rob Clark wrote: > > >> But I think we could still do this w/ one ioctl per crtc for > > >> atomic-pageflip. > > > > > > We could, if we want to sacrifice the synced multi display case. I just > > > think it might be a real use case at some point. IVI feels like the most > > > likely short term cadidate to me, but perhaps someone would finally > > > introduce some new style phone/tablet thingy. I have seen > > > concepts/prototypes of such multi display gadgets in the past, but the > > > industry apparently got a bit stuck on the rectangular slab with > > > touchscreen on one side design. > > > > I could be wrong, but I think IVI the screens would normally be too > > far apart to matter? > > I was thinking of something like a display on the dash that normally > sits low with only a small sliver visible, and extends upwards when > you fire up a movie player for example. Internally it could be made > up of two displays for power savings purposes. > > > Anyways, it is really only a problem if you can't do two ioctl()s > > within one vblank period. If it actually turns out to be a real > > problem, > > Well exactly that's the problem this whole atomic pageflip stuff is > trying to tackle, no? ;) > > > we could always add later an ioctl that takes an array of > > 'struct drm_mode_crtc_atomic_page_flip's. I'm not sure if this is > > really useful or not.. but maybe I'm thinking too much about how > > weston does it's rendering of different output's independently. > > I'm just now thinking of surfaceflinger's way of doing things, with > its prepare and commit phases. If you need to issue two ioctls to handle > cloned displays, then you can end up in a somewhat funky situation. > > Let's say you have a video overlay in use (one each display naturally), > and you increase the downscaling factor enough so that you now have > enough memory bandwith to support only one overlay. With independent > check ioctls for each display, you never have the full device state > available in the kernel, so each check succeeds during the prepare > phase. So you decide that you can keep using the video overlays. > > You then proceed to commit the state, but after the first display has > been commited you get an error when trying to commit the second one. > What can you do now? The only option is to keep displaying the old > frame on the other displays for some time longer, and then on the > next frame you can switch to GPU composition. But on the next frame you > perhaps no longer need to use GPU composition, but since you can't > verify that in the prepare phase, you have no other option but to use > GPU composition. > > So when you run into a configuration that can be supported only > partially, you get animation stalls on some displays due to skipped > frames, and you always have to fall back to GPU composition for the > next frame. > > If on the other hand you would check the whole state in one ioctl, > you'd get the error in the first prepare phase, and could fall back > to GPU composition immediately. > > Am I too much of a perfectionst in considering such things? I don't > think so, but perhaps other people disagree. I don't think there's any harm in having multiple ioctls for different things. I was initially hoping the nuclear page flip would be very simple. Intended for simply updating buffers of several planes associated with a single display. That makes the inner loop of something like Wayland or SF simple, but obviously doesn't cover every case (in fact I had avoided dealing with moving planes initially). Rob's patchset goes further than that, but obviously not as far as you propose. OTOH, keeping things really simple and not very featureful means there are fewer points of failure, which is what I think callers would expect from a flip API... So where does that leave us? I'd propose we have a very simple, stripped down, single crtc flip ioctl, along with a big atomic mode set ioctl, and then perhaps a fancier multi-crtc flip ioctl. -- 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: Destroy the planes prior to destroying the associated CRTC
On Mon, 17 Sep 2012 10:38:03 +0100 Chris Wilson wrote: > As during the plane cleanup, we wish to disable the hardware and > so may modify state on the associated CRTC, that CRTC must continue to > exist until we are finished. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54101 > Signed-off-by: Chris Wilson > Cc: Jesse Barnes > Cc: sta...@vger.kernel.org > --- > drivers/gpu/drm/drm_crtc.c |8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 6fbfc24..af81f77 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1034,15 +1034,15 @@ void drm_mode_config_cleanup(struct drm_device *dev) > fb->funcs->destroy(fb); > } > > - list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list, head) { > - crtc->funcs->destroy(crtc); > - } > - > list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list, >head) { > plane->funcs->destroy(plane); > } > > + list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list, head) { > + crtc->funcs->destroy(crtc); > + } > + > idr_remove_all(&dev->mode_config.crtc_idr); > idr_destroy(&dev->mode_config.crtc_idr); > } Yeah, looks good. Did we used to not touch the crtc in the plane cleanup? Either way this is safer. Reviewed-by: Jesse Barnes -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Adopt a libdrm man page
I just pushed some basic man page stuff to the libdrm repo, but won't have time to do any more pages for the next week or two. But regardless, it would be cool if people could adopt some pages and push them out. The goal is to document every structure and function in the API header files, xf86drm.h and xf86drmMode.h, along with the chipset specific functions for each chipset under include/libdrm/. I've based the man page template on stuff from X, but it may make sense to create our own man page section or put the functions in a different section, I'm definitely not attached to the current structure. 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: Adopt a libdrm man page
On Mon, 17 Sep 2012 23:13:57 +0200 David Herrmann wrote: > Hi Jesse > > On Mon, Sep 17, 2012 at 5:12 PM, Jesse Barnes > wrote: > > I just pushed some basic man page stuff to the libdrm repo, but won't > > have time to do any more pages for the next week or two. > > > > But regardless, it would be cool if people could adopt some pages and > > push them out. The goal is to document every structure and function in > > the API header files, xf86drm.h and xf86drmMode.h, along with the > > chipset specific functions for each chipset under include/libdrm/. > > > > I've based the man page template on stuff from X, but it may make sense > > to create our own man page section or put the functions in a different > > section, I'm definitely not attached to the current structure. > > I just finished writing a short introduction into libdrm modesetting > in form of a working example+comments [1] and then started writing a > libdrm-API reference documentation. However, I think I will convert my > drafts into man-pages and send them to the list. Thanks for starting > it. > > Regards > David > > [1] http://dvdhrm.wordpress.com/2012/09/13/linux-drm-mode-setting-api/ Perfect, I was hoping someone would do an overview page or two for the man pages. Maybe a top level 'drm' man page with references to a high level 'drm kms' page and another for ttm + gem? 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 libdrm 1/4] man: use automake man_MANS to allow multiple suffixes
On Sun, 23 Sep 2012 16:40:04 +0200 David Herrmann wrote: > Current man-page infrastructure supports only man3 but we wanto overview > files to be placed in man7. So use the new automake support for man_MANS > which installs the files automatically in the right location. > > The current man-pages are simply moved and their header line is adjusted to > the > new man-page headers. > > Signed-off-by: David Herrmann > --- Yeah looks nice. Reviewed-by: Jesse Barnes -- 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 libdrm 2/4] man: add man/drm.7 overview page
On Sun, 23 Sep 2012 16:40:05 +0200 David Herrmann wrote: > This man-page is supposed to provide an overview of the whole DRM system. > It is targeted to users that have never worked with DRM and forwards the > reader to the correct other man-pages. > > Detailed information on each system are available in separate man-pages > which are currently under construction. > > Signed-off-by: David Herrmann > --- > man/Makefile.am | 1 + > man/drm.7 | 99 > + > 2 files changed, 100 insertions(+) > create mode 100644 man/drm.7 > > diff --git a/man/Makefile.am b/man/Makefile.am > index f003101..6193a95 100644 > --- a/man/Makefile.am > +++ b/man/Makefile.am > @@ -1,4 +1,5 @@ > man_MANS = \ > + drm.7 \ > drmAvailable.3 \ > drmHandleEvent.3 \ > drmModeGetResources.3 > diff --git a/man/drm.7 b/man/drm.7 > new file mode 100644 > index 000..d8d6f38 > --- /dev/null > +++ b/man/drm.7 > @@ -0,0 +1,99 @@ > +.\" > +.\" Written 2012 by David Herrmann > +.\" Dedicated to the Public Domain > +.\" > +.TH "DRM" 7 "September 2012" "libdrm" "Direct Rendering Manager" > +.SH NAME > +DRM \- Direct Rendering Manager > + > +.SH SYNOPSIS > +.B #include > + > +.SH DESCRIPTION > +The > +.B Direct Rendering Manager > +(DRM) is a framework to manage > +.B Graphics Processing Units > +(GPUs). It is designed to support the needs of complex graphics devices, > usually > +containing programmable pipelines well suited to 3D graphics acceleration. > +Furthermore, it is responsible for memory management, interrupt handling and > DMA > +to provide a uniform interface to applications. > + > +In earlier days, the kernel framework was solely used to provide raw hardware > +access to priviledged user-space processes which implement all the hardware > +abstraction layers. But more and more tasks where moved into the kernel. All > +these interfaces are based on > +.BR ioctl (2) > +commands on the DRM character device. The > +.B libdrm > +library provides wrappers for these system-calls and many helpers to simplify > +the API. > + > +When a GPU is detected, the DRM system loads a driver for the detected > hardware > +type. Each connected GPU is then presented to user-space via a > character-device > +that is usually available as > +.I /dev/dri/card0 > +and can be accessed with > +.BR open (2) > +and > +.BR close (2). > +However, it still depends on the grapics driver which interfaces are > available > +on these devices. If an interface is not available, the syscalls will fail > with > +EINVAL. > + > +.SS Authentication > +All DRM devices provide authentication mechanisms. Only a DRM-Master is > allowed > +to perform mode-setting or modify core state and only one user can be > DRM-Master > +at a time. See > +.BR drmSetMaster (3) > +for information on how to become DRM-Master and what the limitations are. > +Other DRM users can be authenticated to the DRM-Master via > +.BR drmAuthMagic (3) > +so they can perform buffer allocations and rendering. > + > +.SS Mode-Setting > +Managing connected monitors and displays and changing the current modes is > +called > +.BR Mode-Setting . > +This is restricted to the current DRM-Master. Historically, this was > implemented > +in user-space, but new DRM drivers implement a kernel interface to perform > +mode-setting called > +.B Kernel Mode Setting > +(KMS). If your hardware-driver supports it, you can use the KMS API provided > by > +DRM. This includes allocating framebuffers, selecting modes and managing > CRTCs > +and encoders. See > +.BR drm-kms (7) > +for more. > + > +.SS Memory Management > +The most sophisticated tasks for GPUs today is managing memory objects. > +Textures, framebuffers, command-buffers and all other kinds of commands for > the > +GPU have to be stored in memory. The DRM driver takes care of managing all > +memory objects, flushing caches, synchronizing access and providing CPU > access > +to GPU memory. > +.br > +All memory management is hardware driver dependent. However, two generic > +frameworks are available that are used by most DRM drivers. These are the > +.B Translation Table Manager > +(TTM) and the > +.B Graphics Execution Manager > +(GEM). They provide generic APIs to create, destroy and access buffers from > +user-space. However, there are still many differences between the drivers so > +driver-depedent code is still needed. Many helpers are provided in > +.B libgbm > +(Graphics Buffer Manager) from the > +.IR mesa-project . > +For mo
Re: [PATCH libdrm 3/4] man: add KMS overview page
drmModeSetCursor (3) > +and move it on the screen with > +.BR drmModeMoveCursor "(3)." > +This allows to move the cursor on the screen without rerendering. If no > hardware > +cursors are supported, you need to rerender for each frame the cursor is > moved. > + > +.SH EXAMPLES > +Some examples of how basic mode-setting can be done. See the man-page of each > +DRM function for more information. > + > +.SS CRTC/Encoder Selection > +If you retrieved all display configuration information via > +.BR drmModeGetResources (3) > +as > +.BR drmModeRes " *" res "," > +selected a connector from the list in > +.BR "res" "->" "connectors" > +and retrieved the connector-information as > +.BR "drmModeConnector" " *" "conn" " via " drmModeGetConnector (3) > +then this example shows, how you can find a suitable CRTC id to drive this > +connector. This function takes a file-descriptor to the DRM device > +.RB "(see " drmOpen "(3)) as " "fd" "," > +a pointer to the retrieved resources as > +.B res > +and a pointer to the selected connector as > +.BR "conn" "." > +It returns an integer smaller than 0 on failure, otherwise, a valid CRTC id > is > +returned. > + > +.in +4n > +.nf > +static int modeset_find_crtc(int fd, drmModeRes *res, drmModeConnector *conn) > +{ > + drmModeEncoder *enc; > + unsigned int i, j; > + > + /* iterate all encoders of this connector */ > + for (i = 0; i < conn->count_encoders; ++i) { > + enc = drmModeGetEncoder(fd, conn->encoders[i]); > + if (!enc) { > + /* cannot retrieve encoder, ignoring... */ > + continue; > + } > + > + /* iterate all global CRTCs */ > + for (j = 0; j < res->count_crtcs; ++j) { > + /* check whether this CRTC works with the encoder */ > + if (!(enc->possible_crtcs & (1 << j))) > + continue; > + > + > + /* Here you need to check that no other connector > + * currently uses the CRTC with id "crtc". If you intend > + * to drive one connector only, then you can skip this > + * step. Otherwise, simply scan your list of configured > + * connectors and CRTCs whether this CRTC is already > + * used. If it is, then simply continue the search > here. */ > + if (res->crtcs[j] "is unused") { > + drmModeFreeEncoder(enc); > + return res->crtcs[j]; > + } > + } > + > + drmModeFreeEncoder(enc); > + } > + > + /* cannot find a suitable CRTC */ > + return -ENOENT; > +} > +.fi > +.in > + > +.SH REPORTING BUGS > +Bugs in this manual should be reported to http://bugs.freedesktop.org under > +the "Mesa" product, with "Other" or "libdrm" as the component. > + > +.SH "SEE ALSO" > +.BR drm (7), > +.BR drm-memory (7), > +.BR drmModeGetResources (3), > +.BR drmModeGetConnector (3), > +.BR drmModeGetEncoder (3), > +.BR drmModeGetCrtc (3), > +.BR drmModeSetCrtc (3), > +.BR drmModeGetFB (3), > +.BR drmModeAddFB (3), > +.BR drmModeAddFB2 (3), > +.BR drmModeDirtyFB (3), > +.BR drmModeRmFB (3), > +.BR drmModePageFlip (3), > +.BR drmModeGetPlaneResources (3), > +.BR drmModeGetPlane (3), > +.BR drmModeSetPlane (3), > +.BR drmModeSetCursor (3), > +.BR drmModeMoveCursor (3), > +.BR drmSetMaster (3), > +.BR drmAvailable (3), > +.BR drmCheckModesettingSupported (3), > +.BR drmOpen (3) Could probably include a bit more sample code here, but that's no reason not to push. Reviewed-by: Jesse Barnes -- 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 libdrm 4/4] man: add drm-memory man-page
wever, the > +.BR drm-prime (7) > +infrastructure and the generic gem API as described here allow > display-managers > +to handle graphics-buffers and render-clients without any deeper knowledge of > +the GPU that is used. Moreover, it allows to move objects between GPUs and > +implement complex display-servers that don't do any rendering on their own. > See > +its man-page for more information. > + > +.SH EXAMPLES > +This section includes examples for basic memory-management tasks. > + > +.SS Dumb-Buffers > +This examples shows how to create a dumb-buffer via the generic DRM API. > This is > +driver-independent (as long as the driver supports dumb-buffers) and provides > +memory-mapped buffers that can be used for scanout. > +.br > +This example creates a full-HD 1920x1080 buffer with 32 bits-per-pixel and a > +color-depth of 24 bits. The buffer is then bound to a framebuffer which can > be > +used for scanout with the KMS API > +.RB "(see " drm-kms "(7))." > + > +.in +4n > +.nf > + struct drm_mode_create_dumb creq; > + struct drm_mode_destroy_dumb dreq; > + struct drm_mode_map_dumb mreq; > + uint32_t fb; > + int ret; > + void *map; > + > + /* create dumb buffer */ > + memset(&creq, 0, sizeof(creq)); > + creq.width = 1920; > + creq.height = 1080; > + creq.bpp = 32; > + ret = drmIoctl(fd, DRM_IOCTL_MODE_CREATE_DUMB, &creq); > + if (ret < 0) { > + /* buffer creation failed; see "errno" for more error codes */ > + ... > + } > + /* creq.pitch, creq.handle and creq.size are filled by this ioctl with > + * the requested values and can be used now. */ > + > + /* create framebuffer object for the dumb-buffer */ > + ret = drmModeAddFB(fd, 1920, 1080, 24, 32, creq.pitch, creq.handle, > &fb); > + if (ret) { > + /* frame buffer creation failed; see "errno" */ > + ... > + } > + /* the framebuffer "fb" can now used for scanout with KMS */ > + > + /* prepare buffer for memory mapping */ > + memset(&mreq, 0, sizeof(mreq)); > + mreq.handle = creq.handle; > + ret = drmIoctl(fd, DRM_IOCTL_MODE_MAP_DUMB, &mreq); > + if (ret) { > + /* DRM buffer preparation failed; see "errno" */ > + ... > + } > + /* mreq.offset now contains the new offset that can be used with mmap() > */ > + > + /* perform actual memory mapping */ > + map = mmap(0, creq.size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, > mreq.offset); > + if (map == MAP_FAILED) { > + /* memory-mapping failed; see "errno" */ > + ... > + } > + > + /* clear the framebuffer to 0 */ > + memset(map, 0, creq.size); > +.fi > +.in > + > +.SH REPORTING BUGS > +Bugs in this manual should be reported to http://bugs.freedesktop.org under > +the "Mesa" product, with "Other" or "libdrm" as the component. > + > +.SH "SEE ALSO" > +.BR drm (7), > +.BR drm-kms (7), > +.BR drmAvailable (3), > +.BR drmOpen (3), > +.BR drm-intel (7), > +.BR drm-radeon (7), > +.BR drm-nouveau (7), > +.BR drm-prime (7) > diff --git a/man/drm-mm.7 b/man/drm-mm.7 > new file mode 100644 > index 000..258b5a3 > --- /dev/null > +++ b/man/drm-mm.7 > @@ -0,0 +1 @@ > +.so man7/drm-memory.7 > diff --git a/man/drm-ttm.7 b/man/drm-ttm.7 > new file mode 100644 > index 000..258b5a3 > --- /dev/null > +++ b/man/drm-ttm.7 > @@ -0,0 +1 @@ > +.so man7/drm-memory.7 Heh, this one highlights the lack of documentation we have for other libs like Mesa and libgbm. :) Reviewed-by: Jesse Barnes -- 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: FOSDEM2013: DevRoom or not?
On Fri, 28 Sep 2012 17:44:27 +0200 Luc Verhaegen wrote: > On Fri, Sep 21, 2012 at 11:35:12AM +0200, Luc Verhaegen wrote: > > On Sun, Aug 12, 2012 at 03:50:16PM +0200, Luc Verhaegen wrote: > > > Hi, > > > > > > The FOSDEM organizers have sent out a call for devrooms. FOSDEM this > > > year is on the weekend of the 2nd and 3rd of February 2013. > > > > > > After the success of this formula last year, where, for the first time > > > ever, we had a properly filled devroom schedule when the deadline hit, i > > > am going to re-apply the same formula: > > > * By the 28th of september, i need 6 committed speakers, otherwise i > > > will not apply for a DevRoom. 6 people need to apply for a talk slot > > > who _definitely_ will come to FOSDEM on February 2nd and/or 3rd 2013. > > > This "definitely" means: > > > * Don't knowingly plan anything else for this weekend. > > > * Come to FOSDEM even if your corporation does not fund you (though > > > feel free to contact the board individually for funding) > > > * Make sure that you are allowed to travel to the shengen area in > > > february. > > > * Catastrophies excluded of course. Such catastrophies include > > > things like train-derailments and such, but explicitely excludes > > > hangovers :p > > > * Scheduling based on timeliness of application: the earlier you apply, > > > the better slot you get. > > > * FOSDEMs final deadline for the full schedule is likely around 15th of > > > january 2013. But do not count on that deadline, we will only do > > > hourly slots, to keep people from running around between devrooms like > > > headless chickens. Only 12-16 slots will be available, first come, > > > first serve. > > > > > > I will set up a wiki page tonight, at http://wiki.x.org/wiki/fosdem2013 > > > > > > I hope we get a nice devroom like we had last time. That new building we > > > were in was really amazing, even though it took a while before all > > > FOSDEM visitors found it. > > > > > > Thanks, > > > > > > Luc Verhaegen. > > > > Just a heads up guys, we have a week left and not a single speaker yet! > > > > Given the timing of XDC and the FOSDEM deadines, this is not too > > surprising, but still, with XDC2012 in its final day it is high time > > that we start thinking about FOSDEM. I will later on shout at people > > here in the room too :) > > > > All we need now is people who will definitely come to FOSDEM, and who > > are willing to talk in the DevRoom. If a vague idea of a topic is > > already known, then great, but this is not necessary yet. I now put up a > > preliminary wiki page at wiki.x.org/wiki/fosdem2013, add yourself to the > > list there. > > > > Thanks, > > > > Luc Verhaegen. > > Final reminder: the deadline is tonight. > > So far there are three speakers who lined up, and my feeling is that > Matthieu and Marc lined up just so that the deadline and requirement > will be met. So only a single person is intending to come to fosdem and > has a topic he wants to talk about. > > Come on guys. Surely we can do better than that. I could come up with something, maybe people would be interested in hearing about some of our recent SoC work? I'd have to see what I could get approval for, but I could probably find *something* that's not still secret. :) -- 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: [drm-intel:drm-intel-fixes 15/18] drivers/gpu/drm/i915/intel_ringbuffer.c:1405:24: error: 'I915_GEM_SCRATCH_INDEX' undeclared
On Wed, 3 Oct 2012 17:12:18 +0200 Daniel Vetter wrote: > On Wed, Oct 03, 2012 at 10:15:38PM +0800, Fengguang Wu wrote: > > Hi Jesse, > > > > FYI, kernel build failed on > > > > tree: git://people.freedesktop.org/~danvet/drm-intel.git drm-intel-fixes > > head: 2cb47a9c6d49f2961dcf57f69ff968e65d48e876 > > commit: 307daa1b856699f1d4acc3972374017c3e99d331 [15/18] drm/i915: TLB > > invalidation with MI_FLUSH_DW requires a post-sync op > > config: x86_64-allyesconfig > > Thanks for the report, should be fixed now - somehow I've failed to notice > that my .config has gone awry. No, this is because this patch depends on the earlier patch to add the GEM_SCRATCH_INDEX to i915_drv.h, which wasn't included in this series. Did you pull it in from the earlier series? Or fix it up some other way that caused the breakage you saw? Jesse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: reduce default drm vblank off delay to 50ms
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 --- drivers/gpu/drm/drm_stub.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index c236fd2..a1cbc0e 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -40,7 +40,7 @@ unsigned int drm_debug = 0;/* 1 to enable debug output */ EXPORT_SYMBOL(drm_debug); -unsigned int drm_vblank_offdelay = 5000;/* Default to 5000 msecs. */ +unsigned int drm_vblank_offdelay = 50;/* Default to 50 msecs. */ EXPORT_SYMBOL(drm_vblank_offdelay); unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */ -- 1.7.9.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: reduce default drm vblank off delay to 50ms
Cc'ing Mario in case he wants a different value than 50ms. On Tue, 30 Oct 2012 14:09:12 -0500 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 > --- > drivers/gpu/drm/drm_stub.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c > index c236fd2..a1cbc0e 100644 > --- a/drivers/gpu/drm/drm_stub.c > +++ b/drivers/gpu/drm/drm_stub.c > @@ -40,7 +40,7 @@ > unsigned int drm_debug = 0; /* 1 to enable debug output */ > EXPORT_SYMBOL(drm_debug); > > -unsigned int drm_vblank_offdelay = 5000;/* Default to 5000 msecs. */ > +unsigned int drm_vblank_offdelay = 50;/* Default to 50 msecs. */ > EXPORT_SYMBOL(drm_vblank_offdelay); > > unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */ -- 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: reduce default drm vblank off delay to 50ms
On Tue, 30 Oct 2012 20:20:44 +0100 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 > > Nacked-by: Daniel Vetter > > Also discussed with Jesse on irc, we've had fun ;-) That's ridiculous. Just because we have a race we can't fix wrt reading hw regs, doesn't mean we can't reduce the timeout. I nack your nack. -- 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 46/51] drm/i915: Add support for atomic modesetting completion events
On Thu, 1 Nov 2012 12:12:35 +0100 Daniel Vetter wrote: > On Thu, Oct 25, 2012 at 8:05 PM, wrote: > > From: Ville Syrjälä > > > > Send completion events when the atomic modesetting operations has > > finished succesfully. > > > > Signed-off-by: Ville Syrjälä > > I have to admit I'm not on top of the latest ioctl/interface > discussion, but one new requirement for the modeset (not the pageflip > part) of the all this atomic stuff I've discovered is that the kernel > needs to be able to select the crtcs for each output completely > unrestricted. I think userspace should simply pass in abstract crtc > ids (e.g. 0, 1, 2, ...) and the kernel then passes back the actual > crtcs it has selected. > > We can't do that remapping internally because the crtc abstraction is leaky: > - wait_for_vblank requires the pipe id, which could then change on every > modeset > - similarly userspace doing MI_WAIT for scanlines needs to know the > actual hw pipe in use by a crtc. > And current userspace presumes that the mapping between crtc->pipe > doesn't change. > > An example why the kernel needs to pick the crtc for userspace: > - on ivb only pipe A has a 7x5 panel fitter, so usually you want to > put the panel on the first crtc > - but if you run in a 3 pipe configuration and have an eDP panel, it's > better to put the eDP output on pipe C, since then pipes A&B will have > full 4-lane fdi links to the pch ports (instead of otherwise only 2 > lanes each on links B&C) > - similarly when we have a 3 pipe configuration with all encoders on > the pch, we need to move the mode with the highest dotclock to pipe A > (since that's the only one which will have 4 lanes, pipe B&C will only > have 2 each). > - afaik these kind of asymmetric constraints won't disappear anytime > soon, haswell definitely still has some. Yeah that's a good point... adding a virtual crtc layer would solve this and let us preserve the existing ABI. -- 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 46/51] drm/i915: Add support for atomic modesetting completion events
On Thu, 1 Nov 2012 19:07:02 +0200 Ville Syrjälä wrote: > On Thu, Nov 01, 2012 at 07:39:12AM -0700, Jesse Barnes wrote: > > On Thu, 1 Nov 2012 12:12:35 +0100 > > Daniel Vetter wrote: > > > > > On Thu, Oct 25, 2012 at 8:05 PM, wrote: > > > > From: Ville Syrjälä > > > > > > > > Send completion events when the atomic modesetting operations has > > > > finished succesfully. > > > > > > > > Signed-off-by: Ville Syrjälä > > > > > > I have to admit I'm not on top of the latest ioctl/interface > > > discussion, but one new requirement for the modeset (not the pageflip > > > part) of the all this atomic stuff I've discovered is that the kernel > > > needs to be able to select the crtcs for each output completely > > > unrestricted. I think userspace should simply pass in abstract crtc > > > ids (e.g. 0, 1, 2, ...) and the kernel then passes back the actual > > > crtcs it has selected. > > > > > > We can't do that remapping internally because the crtc abstraction is > > > leaky: > > > - wait_for_vblank requires the pipe id, which could then change on every > > > modeset > > > - similarly userspace doing MI_WAIT for scanlines needs to know the > > > actual hw pipe in use by a crtc. > > > And current userspace presumes that the mapping between crtc->pipe > > > doesn't change. > > > > > > An example why the kernel needs to pick the crtc for userspace: > > > - on ivb only pipe A has a 7x5 panel fitter, so usually you want to > > > put the panel on the first crtc > > > - but if you run in a 3 pipe configuration and have an eDP panel, it's > > > better to put the eDP output on pipe C, since then pipes A&B will have > > > full 4-lane fdi links to the pch ports (instead of otherwise only 2 > > > lanes each on links B&C) > > > - similarly when we have a 3 pipe configuration with all encoders on > > > the pch, we need to move the mode with the highest dotclock to pipe A > > > (since that's the only one which will have 4 lanes, pipe B&C will only > > > have 2 each). > > > - afaik these kind of asymmetric constraints won't disappear anytime > > > soon, haswell definitely still has some. > > > > Yeah that's a good point... adding a virtual crtc layer would solve > > this and let us preserve the existing ABI. > > How would the state handling work? I mean if drm_crtc X currently has > some state, drm_crtc Y has some other state, and then we do a modeset > which would require swapping the roles of the crtcs, what would happen > to the bits of state that we didn't specify? > > If we'd do the remapping below the drm crtc layer, the state would > always be tied to the drm crtc. But that would definitely require > mostly uniform hardware "crtcs" so that the capabilities of the > drm_crtcs wouldn't keep changing whenever the remap happens. > > Well, I suppose we could tie the state to the virtual crtc instead, > and doing an operation on a real drm_crtc would also change the > state of the currently bound virtual crtc. And then changing the > virtual<->real mapping would just copy the state over from the virtual > crtc to the real drm_crtc. > > And if we do it for crtcs, then we'd need to do it for planes as well, > because the plane<->crtc mapping can be fixed or otherwise limited > in some fashion. > > Either way it sounds rather messy to me. > > Another option would be just leave it up to userspace to make the > correct choice between crtcs and planes. But then user space needs > to be equipped with more hardware specific knowledge, so it's not > a very appealing idea either. Yeah it gets ugly one way or another. From a userspace perspective, keeping the ugliness in the kernel is nice, and if we have to do it somewhere I guess I'd prefer that. However, we can't always hide hw details like that in the kernel through generic interfaces (e.g. for sprites and all their restrictions) so userspace will have to have some knowledge one way or another, and maybe it's not that bad to push some of the other limitation knowledge into our userspace code. Ambivalent. -- 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: reduce default drm vblank off delay to 50ms
ded, but i > don't know, maybe i just misunderstood his comment? I asked for > clarification, but he didn't respond to my followup e-mails. Maybe he > was just too busy to reply, maybe i'm just too pushy and annoying. > > I will try to make some friendship with piglit and see if i can write > some tests when i find some time to catch such bugs earlier in the future. > > Also, i take from Chris comments about patch [1/3] above that properly > implemented triple-buffering with correct timestamping and > synchronisation is impossible with the current X-Servers, because the > DRI2 invalidate event mechanism is racy and broken to the point of not > being fixable? Basically we would have to wait for DRI3? > > Sorry for getting a bit off-topic here, but i think it's important to > keep all layers working in a meaningful way if you want to support some > rather serious applications of computer graphics. Agreed, and sorry for the breakage! piglit tests would definitely help if you find time. 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: [Intel-gfx] [PATCH 01/21] drm/i915: Enable digital port hotplug on PCH systems
On Fri, 30 Sep 2011 18:17:32 +0200 Daniel Vetter wrote: > On Thu, Sep 29, 2011 at 06:09:33PM -0700, Keith Packard wrote: > > We were relying on the BIOS to set these bits, which doesn't always > > happen. > > > > Signed-off-by: Keith Packard > > Ok, I'll try to increase our review-bandwidth for modesetting stuff by > jumping into this maze myself. For the moment I'll shortly explain what > I've actually checked so you know how much weight/little to attach to my > r-b's. > > Checked with docs, noticed that the public one's lack register addresses > for PCH regs ... > Reviewed-by: Daniel Vetter Yep, I like it too: Reviewed-by: Jesse Barnes -- 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/21] drm/i915: Only use VBT panel mode on eDP if no EDID is found
On Fri, 30 Sep 2011 10:58:26 -0700 Keith Packard wrote: > On Fri, 30 Sep 2011 18:32:35 +0200, Daniel Vetter wrote: > > > Ok, this is way over my head, just checked whether the patch does what it > > claims to - nice exercise in reading dp modeset code ;-) > > Yeah, it's purely heuristic -- the VBT contains a mode which was > originally for the LVDS. It's unclear whether it should ever be applied > to an eDP panel, but absent other information, it seems like we should > at least consider it. > > Many LVDS panels don't bother to include an EDID rom, or the vendor > didn't bother to hook up the DDC wire; presumably it's cheaper for them > to stick more data in the VBIOS than add hardware. However, there are > some LVDS panels with EDID roms which contain *incorrect* mode data for > the panel (amazing, I know), and so the driver prefers to use the VBT > data when both are present. > > eDP, on the other hand, doesn't require any additional wiring (at least) > to connect up the DDC channel, and eDP panels are required to provide > EDID data. So far, in my (very small) sample set, I've got one machine > which provides accurate VBT *and* EDID data (an HP 2540p) and one > machine which provides inaccurate VBT data but accurate EDID data (a > MacBook air). So, I just decided to prefer the EDID data. > > One option would be to extract the current mode from the hardware when > the driver starts and use that if present. But, that might mean that > you'd get different modes depending on whether the machine booted with > the lid closed or open, which seems like a bad plan. More than that, I think eDP *requires* an EDID, and it sounds like even the Air has one (and if any machine didn't, you know it would be an Apple). So I'm definitely in favor of this change. -- 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 05/21] drm/i915: Check eDP power when doing aux channel communications
On Thu, 29 Sep 2011 18:09:37 -0700 Keith Packard wrote: > Verify that the eDP VDD is on, either with the panel being on or with > the VDD force-on bit being set. > > This demonstrates that in many instances, VDD is not on when needed, > which leads to failed EDID communications. > > Signed-off-by: Keith Packard > --- > drivers/gpu/drm/i915/intel_dp.c | 22 ++ > 1 files changed, 22 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 8ab2a88..3b29a6f 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -279,6 +279,24 @@ intel_hrawclk(struct drm_device *dev) > } > } > > +static void > +intel_dp_check_edp(struct intel_dp *intel_dp) > +{ > + struct drm_device *dev = intel_dp->base.base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + u32 pp_status, pp_control; > + if (!is_edp(intel_dp)) > + return; > + pp_status = I915_READ(PCH_PP_STATUS); > + pp_control = I915_READ(PCH_PP_CONTROL); > + if ((pp_status & PP_ON) == 0 && (pp_control & EDP_FORCE_VDD) == 0) { > + WARN(1, "eDP powered off while attempting aux channel > communication.\n"); > + DRM_DEBUG_KMS("Status 0x%08x Control 0x%08x\n", > + pp_status, > + I915_READ(PCH_PP_CONTROL)); > + } > +} I'd call it "intel_dp_assert_dp_power" or something more descriptive (or just assert_panel_power to match the other asserts in intel_display.c), but other than that this is a nice sanity check. -- 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 16/21] drm/i915: edp_panel_on does not need to return a bool
On Thu, 29 Sep 2011 18:09:48 -0700 Keith Packard wrote: > The return value was unused, so just stop doing that. > > Signed-off-by: Keith Packard > --- > drivers/gpu/drm/i915/intel_dp.c |6 ++ > 1 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 02b5162..56146a2 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -886,14 +886,14 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp > *intel_dp) > } > > /* Returns true if the panel was already on when called */ > -static bool ironlake_edp_panel_on (struct intel_dp *intel_dp) > +static void ironlake_edp_panel_on (struct intel_dp *intel_dp) > { Remove the comment too? -- 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 10/21] drm/i915: Wrap DP EDID fetch functions to enable eDP panel power
On Thu, 29 Sep 2011 18:09:42 -0700 Keith Packard wrote: > Talking to the eDP DDC channel requires that the panel be powered > up. Wrap both the EDID and modes fetch code with calls to turn the vdd > power on and back off. > These VDD AUX changes look good, ack on all of them. -- 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 00/24] MacBook Air patch sequence (v2)
On Fri, 30 Sep 2011 11:17:38 -0700 Keith Packard wrote: > On Fri, 30 Sep 2011 09:20:55 -0400, Ted Ts'o wrote: > > > What kind of battery life do you get with these patches applied, out > > of curiosity? > > 4-5 hours when doing the usual web-surfing, etc. Seems pretty much the > same as people are getting under Mac OS. As long as you enable RC6. :) -- 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 9/9] drm/i915: Initialize PCH refclks at modeset init time
On Tue, 27 Sep 2011 11:11:57 -0700 Keith Packard wrote: > On Tue, 27 Sep 2011 17:56:39 +0100, Chris Wilson > wrote: > > > Ah, now I see why we moved from using the active configuration earlier. ;-) > > My evil plan is revealed! > > > Doesn't this prevent us from ever using SSC though, as virtually every > > single PCH machine has HDMI encoders that haven't been masked out through > > the chicken fuses or VBT? > > That wasn't my intent -- the SSC source gets modulated whenever the VBT > table says it can, so when the panel uses the SSC source, it will get > SSC. Did I mess something up here? Or is it just some interaction with > the mode setting code that I didn't get right? Assuming we're selecting the proper reference clock in the PLL selection code anyway... Doing it all up front seems nicer; did you get confirmation that the "wavy VGA" bug was fixed with this series? Overall seems like a good improvement over our old PCH refclk code... -- 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] PCH reference clock cleanups
On Wed, 28 Sep 2011 15:22:48 -0300 Paulo Zanoni wrote: > 2011/9/27 Keith Packard : > > Here's a patch sequence which cleans up a bunch of PCH refclk related > > bits. > > For the series: Tested-by: Paulo Zanoni > > Tested all the patches on Ironlake (LVDS + VGA). Fixes fd.o bug #38750 for me. > > I also tested the patch you sent today 1 hour ago (inline in one of > the emails) and things still work with it. I'll keep using these > patches since they fix my laptop. Any problem will be reported. > > Maybe my email client/server is ruining things, but I believe patch 7 > includes whitespace errors. Yay excellent. Now... is keeping the various refclks enabled costing us any power? IOW, should we be trying to disable them when everything has been DPMS'd off too? -- 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] PCH reference clock cleanups
On Mon, 03 Oct 2011 16:18:48 -0700 Keith Packard wrote: > On Mon, 3 Oct 2011 14:14:23 -0700, Jesse Barnes > wrote: > > > Now... is keeping the various refclks enabled costing us any power? > > IOW, should we be trying to disable them when everything has been > > DPMS'd off too? > > That's the same as tracking usage and enabling/disabling on the fly as > modes are set. I think it's possible, but I'd like to have the 'simpler' > fix present before we try a power saving move. Agreed; fortunately shutting everything off when no outputs are active should be simpler than trying flip the bits on & off every mode set. :) -- 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 v2 4/5] DRI2: Expose API to set drawable swap limit.
On Thu, 15 Sep 2011 01:31:00 +0200 Mario Kleiner wrote: > On Sep 15, 2011, at 12:54 AM, Francisco Jerez wrote: > > > Mario Kleiner writes: > > > >> On Sep 14, 2011, at 6:02 PM, Keith Packard wrote: > >> > >>> On Wed, 14 Sep 2011 10:05:29 -0500, Jesse Barnes > >>> wrote: > >>> > >>>> Ah thanks Mario, I blame Keith. :p I agree we should integrate > >>>> this > >>>> patch as it would allow us to do more fun stuff with swapping... > >>> > >>> That patch changes a ton of stuff; would be nice to see it split > >>> into > >>> smaller chunks that could be reviewed easily. > >>> > >> > >> For reference, we're talking about this series, right? > >> > >> <http://www.mail-archive.com/xorg-devel@lists.x.org/msg14336.html> > >> > >> As far as i can see, they were already split up in chunks and > >> reviewed > >> by Jesse, me and Franzisco Jerez - assuming they still apply to the > >> latest server version? At least 1/5, 4/5 and 5/5 looked simple > >> enough > >> and 4/5 and 5/5, the swaplimit api, seem to be independent from the > >> rest of the series? > >> > > Note that my r-b only goes to 4/5, I had some objections to 5/5 and > > I'm > > not sure if 1/5-3/5 still make sense. > > I haven't checked if the rest still makes sense. 5/5 was about why a > driver who requests a dri2 swaplimit change should be called back to > confirm it is ok with the swaplimit change, which you said seems > totally redundant, right? > > Ok, so we talk specifically about 4/5, which was reviewed by all of > us, i think non-controversial, and a simple addition of a dri2 > swaplimit api. > > Keith, what about that one for a start? > > > BTW, is there any reason this is being discussed outside of the > > mailing > > list? > > No. It just started as a private conversation with Jesse and > "drifted" into this. cc'ing dri-devel, all that was said is in this > mail. What's the latest here? I still think we need the swap limit API... -- Jesse Barnes, Intel Open Source Technology Center signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
DRM planes and new fb creation ioctl
I've given up waiting for someone to implement support for these ioctls on another platform before they're merged, but I have received a lot of feedback on the interfaces, and it sounds like they're ok. I've also fixed all the remaining issues I'm aware of on SNB platforms and things are working well, so I'm just going to push them out. (Note IVB support is still missing a few bits for scaling and such; I'll fix those up when I get back home and can test on IVB again.) One change you may notice from the last set is that I've removed the 'zpos' parameter. Plane blending and z ordering is very chipset specific (it even varies between Intel chipsets), so exposing it through a device specific ioctl is probably a better plan. By default, planes should just overlay the primary plane; a device specific ioctl (none available yet, but I have some planned for i915) can provide more flexibility. To recap previous posts, this patchset provides a few new interfaces: - addfb2 - a new FB creation ioctl that lets you specify a surface format, as defined by a fourcc code from the video4linux headers (libdrm will wrap these in DRM_ macros for portability) - planes - ioctls for fetching plane info and attaching an fb to a plane; note there's no separate flip ioctl for planes, just use setplane to update the fb The testdisplay.c program in intel-gpu-tools has support for testing these interfaces, and I'll be fixing up and pushing the xf86-video-intel support soon as well, so you can use either as a reference for how the new interfaces work. Thanks, Jesse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/11] drm: add plane support
Planes are a bit like half-CRTCs. They have a location and fb, but don't drive outputs directly. Add support for handling them to the core KMS code. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/drm_crtc.c | 236 +++- drivers/gpu/drm/drm_drv.c |3 + include/drm/drm.h |3 + include/drm/drm_crtc.h | 71 +- include/drm/drm_mode.h | 33 ++ 5 files changed, 343 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index fe738f0..0e129b1 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -535,6 +535,48 @@ void drm_encoder_cleanup(struct drm_encoder *encoder) } EXPORT_SYMBOL(drm_encoder_cleanup); +void drm_plane_init(struct drm_device *dev, struct drm_plane *plane, + unsigned long possible_crtcs, + const struct drm_plane_funcs *funcs, + uint32_t *formats, uint32_t format_count) +{ + mutex_lock(&dev->mode_config.mutex); + + plane->dev = dev; + drm_mode_object_get(dev, &plane->base, DRM_MODE_OBJECT_PLANE); + plane->funcs = funcs; + plane->format_types = kmalloc(sizeof(uint32_t) * format_count, + GFP_KERNEL); + if (!plane->format_types) { + DRM_DEBUG_KMS("out of memory when allocating plane\n"); + drm_mode_object_put(dev, &plane->base); + return; + } + + memcpy(plane->format_types, formats, format_count); + plane->format_count = format_count; + plane->possible_crtcs = possible_crtcs; + + list_add_tail(&plane->head, &dev->mode_config.plane_list); + dev->mode_config.num_plane++; + + mutex_unlock(&dev->mode_config.mutex); +} +EXPORT_SYMBOL(drm_plane_init); + +void drm_plane_cleanup(struct drm_plane *plane) +{ + struct drm_device *dev = plane->dev; + + mutex_lock(&dev->mode_config.mutex); + kfree(plane->format_types); + drm_mode_object_put(dev, &plane->base); + list_del(&plane->head); + dev->mode_config.num_plane--; + mutex_unlock(&dev->mode_config.mutex); +} +EXPORT_SYMBOL(drm_plane_cleanup); + /** * drm_mode_create - create a new display mode * @dev: DRM device @@ -866,6 +908,7 @@ void drm_mode_config_init(struct drm_device *dev) INIT_LIST_HEAD(&dev->mode_config.encoder_list); INIT_LIST_HEAD(&dev->mode_config.property_list); INIT_LIST_HEAD(&dev->mode_config.property_blob_list); + INIT_LIST_HEAD(&dev->mode_config.plane_list); idr_init(&dev->mode_config.crtc_idr); mutex_lock(&dev->mode_config.mutex); @@ -1466,6 +1509,193 @@ out: } /** + * drm_mode_getplane_res - get plane info + * @dev: DRM device + * @data: ioctl data + * @file_priv: DRM file info + * + * Return an plane count and set of IDs. + */ +int drm_mode_getplane_res(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_mode_get_plane_res *plane_resp = data; + struct drm_mode_config *config; + struct drm_plane *plane; + uint32_t __user *plane_ptr; + int copied = 0, ret = 0; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + mutex_lock(&dev->mode_config.mutex); + config = &dev->mode_config; + + /* +* This ioctl is called twice, once to determine how much space is +* needed, and the 2nd time to fill it. +*/ + if (config->num_plane && + (plane_resp->count_planes >= config->num_plane)) { + plane_ptr = (uint32_t *)(unsigned long)plane_resp->plane_id_ptr; + + list_for_each_entry(plane, &config->plane_list, head) { + if (put_user(plane->base.id, plane_ptr + copied)) { + ret = -EFAULT; + goto out; + } + copied++; + } + } + plane_resp->count_planes = config->num_plane; + +out: + mutex_unlock(&dev->mode_config.mutex); + return ret; +} + +/** + * drm_mode_getplane - get plane info + * @dev: DRM device + * @data: ioctl data + * @file_priv: DRM file info + * + * Return plane info, including formats supported, gamma size, any + * current fb, etc. + */ +int drm_mode_getplane(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_mode_get_plane *plane_resp = data; + struct drm_mode_object *obj; + struct drm_plane *plane; + uint32_t __user *format_ptr; + int ret = 0; + + if (!drm_core_check_feature(dev, DRIVER_MODESE
[PATCH 02/11] drm: add an fb creation ioctl that takes a pixel format
To properly support the various plane formats supported by different hardware, the kernel must know the pixel format of a framebuffer object. So add a new ioctl taking a format argument corresponding to a fourcc name from videodev2.h. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/drm_crtc.c| 71 - drivers/gpu/drm/drm_crtc_helper.c |3 +- drivers/gpu/drm/drm_drv.c |1 + drivers/gpu/drm/i915/intel_display.c |9 ++-- drivers/gpu/drm/i915/intel_drv.h |2 +- drivers/gpu/drm/i915/intel_fb.c |3 +- drivers/gpu/drm/nouveau/nouveau_display.c |4 +- drivers/gpu/drm/radeon/radeon_display.c |4 +- drivers/gpu/drm/radeon/radeon_fb.c|5 +- drivers/gpu/drm/radeon/radeon_mode.h |2 +- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |2 +- drivers/staging/gma500/framebuffer.c |2 +- include/drm/drm.h |1 + include/drm/drm_crtc.h|6 ++- include/drm/drm_crtc_helper.h |2 +- include/drm/drm_mode.h| 14 ++ 16 files changed, 112 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0e129b1..a30b9d4 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1909,7 +1909,76 @@ out: int drm_mode_addfb(struct drm_device *dev, void *data, struct drm_file *file_priv) { - struct drm_mode_fb_cmd *r = data; + struct drm_mode_fb_cmd *or = data; + struct drm_mode_fb_cmd2 r; + struct drm_mode_config *config = &dev->mode_config; + struct drm_framebuffer *fb; + int ret = 0; + + /* Use new struct with format internally */ + r.fb_id = or->fb_id; + r.width = or->width; + r.height = or->height; + r.pitch = or->pitch; + r.bpp = or->bpp; + r.depth = or->depth; + r.pixel_format = 0; + r.handle = or->handle; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + if ((config->min_width > r.width) || (r.width > config->max_width)) { + DRM_ERROR("mode new framebuffer width not within limits\n"); + return -EINVAL; + } + if ((config->min_height > r.height) || (r.height > config->max_height)) { + DRM_ERROR("mode new framebuffer height not within limits\n"); + return -EINVAL; + } + + mutex_lock(&dev->mode_config.mutex); + + /* TODO check buffer is sufficiently large */ + /* TODO setup destructor callback */ + + fb = dev->mode_config.funcs->fb_create(dev, file_priv, &r); + if (IS_ERR(fb)) { + DRM_ERROR("could not create framebuffer\n"); + ret = PTR_ERR(fb); + goto out; + } + + or->fb_id = fb->base.id; + list_add(&fb->filp_head, &file_priv->fbs); + DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id); + +out: + mutex_unlock(&dev->mode_config.mutex); + return ret; +} + +/** + * drm_mode_addfb2 - add an FB to the graphics configuration + * @inode: inode from the ioctl + * @filp: file * from the ioctl + * @cmd: cmd from ioctl + * @arg: arg from ioctl + * + * LOCKING: + * Takes mode config lock. + * + * Add a new FB to the specified CRTC, given a user request with format. + * + * Called by the user via ioctl. + * + * RETURNS: + * Zero on success, errno on failure. + */ +int drm_mode_addfb2(struct drm_device *dev, + void *data, struct drm_file *file_priv) +{ + struct drm_mode_fb_cmd2 *r = data; struct drm_mode_config *config = &dev->mode_config; struct drm_framebuffer *fb; int ret = 0; diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index f88a9b2..77c7293 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -806,13 +806,14 @@ void drm_helper_connector_dpms(struct drm_connector *connector, int mode) EXPORT_SYMBOL(drm_helper_connector_dpms); int drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb, - struct drm_mode_fb_cmd *mode_cmd) + struct drm_mode_fb_cmd2 *mode_cmd) { fb->width = mode_cmd->width; fb->height = mode_cmd->height; fb->pitch = mode_cmd->pitch; fb->bits_per_pixel = mode_cmd->bpp; fb->depth = mode_cmd->depth; + fb->pixel_format = mode_cmd->pixel_format; return 0; } diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 15da618..f24b9b6 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -152,6 +152,7 @@ static struct drm_ioctl_desc drm_ioctls[]
[PATCH 05/11] drm/i915: move pin & fence for plane past potential error paths
This avoids the need to unpin on the error path. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/intel_overlay2.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index 2e38b15..e583bd0 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -62,9 +62,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, old_obj = intel_plane->obj; mutex_lock(&dev->struct_mutex); - ret = intel_pin_and_fence_fb_obj(dev, obj, NULL); - if (ret) - goto out_unlock; dvscntr = I915_READ(reg); @@ -104,6 +101,10 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, goto out_unlock; } + ret = intel_pin_and_fence_fb_obj(dev, obj, NULL); + if (ret) + goto out_unlock; + intel_plane->obj = obj; dvscntr |= DVS_TILED; -- 1.7.4.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/11] drm/i915: rename existing overlay support to "legacy"
The old overlay block has all sorts of quirks and is very different than ILK+ video sprites. So rename it to legacy to make that clear and clash less with core overlay support. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_debugfs.c |2 +- drivers/gpu/drm/i915/i915_drv.h | 12 ++-- drivers/gpu/drm/i915/i915_irq.c |2 +- drivers/gpu/drm/i915/intel_display.c |2 +- drivers/gpu/drm/i915/intel_drv.h |4 +- drivers/gpu/drm/i915/intel_overlay.c | 126 +- 6 files changed, 74 insertions(+), 74 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 8e95d66..b6d0bbc 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -825,7 +825,7 @@ static int i915_error_state(struct seq_file *m, void *unused) } if (error->overlay) - intel_overlay_print_error_state(m, error->overlay); + intel_legacy_overlay_print_error_state(m, error->overlay); if (error->display) intel_display_print_error_state(m, dev, error->display); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 06a37f4..b96c174 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -118,8 +118,8 @@ struct intel_opregion { }; #define OPREGION_SIZE(8*1024) -struct intel_overlay; -struct intel_overlay_error_state; +struct intel_legacy_overlay; +struct intel_legacy_overlay_error_state; struct drm_i915_master_private { drm_local_map_t *sarea; @@ -191,7 +191,7 @@ struct drm_i915_error_state { u32 cache_level:2; } *active_bo, *pinned_bo; u32 active_bo_count, pinned_bo_count; - struct intel_overlay_error_state *overlay; + struct intel_legacy_overlay_error_state *overlay; struct intel_display_error_state *display; }; @@ -343,7 +343,7 @@ typedef struct drm_i915_private { struct intel_opregion opregion; /* overlay */ - struct intel_overlay *overlay; + struct intel_legacy_overlay *overlay; /* LVDS info */ int backlight_level; /* restore backlight to this value */ @@ -1309,8 +1309,8 @@ extern int intel_trans_dp_port_sel(struct drm_crtc *crtc); /* overlay */ #ifdef CONFIG_DEBUG_FS -extern struct intel_overlay_error_state *intel_overlay_capture_error_state(struct drm_device *dev); -extern void intel_overlay_print_error_state(struct seq_file *m, struct intel_overlay_error_state *error); +extern struct intel_legacy_overlay_error_state *intel_legacy_overlay_capture_error_state(struct drm_device *dev); +extern void intel_legacy_overlay_print_error_state(struct seq_file *m, struct intel_legacy_overlay_error_state *error); extern struct intel_display_error_state *intel_display_capture_error_state(struct drm_device *dev); extern void intel_display_print_error_state(struct seq_file *m, diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9ee2729..36f2837 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -998,7 +998,7 @@ static void i915_capture_error_state(struct drm_device *dev) do_gettimeofday(&error->time); - error->overlay = intel_overlay_capture_error_state(dev); + error->overlay = intel_legacy_overlay_capture_error_state(dev); error->display = intel_display_capture_error_state(dev); spin_lock_irqsave(&dev_priv->error_lock, flags); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b1ae70b..e748338 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3180,7 +3180,7 @@ static void intel_crtc_dpms_overlay(struct intel_crtc *intel_crtc, bool enable) mutex_lock(&dev->struct_mutex); dev_priv->mm.interruptible = false; - (void) intel_overlay_switch_off(intel_crtc->overlay); + (void) intel_legacy_overlay_switch_off(intel_crtc->overlay); dev_priv->mm.interruptible = true; mutex_unlock(&dev->struct_mutex); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 23c5622..467fb4a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -161,7 +161,7 @@ struct intel_crtc { bool busy; /* is scanout buffer being updated frequently? */ struct timer_list idle_timer; bool lowfreq_avail; - struct intel_overlay *overlay; + struct intel_legacy_overlay *overlay; struct intel_unpin_work *unpin_work; int fdi_lanes; @@ -370,7 +370,7 @@ extern void intel_finish_page_flip_plane(struct drm_device *dev, int plane); extern void intel_setup_overlay(struct drm_device *dev); extern
[PATCH 04/11] drm/i915: add SNB video sprite support
The video sprites support various video surface formats natively and can handle scaling as well. So add support for them using the new DRM core overlay support functions. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/i915_reg.h | 52 + drivers/gpu/drm/i915/intel_display.c | 25 +++- drivers/gpu/drm/i915/intel_drv.h | 14 +++ drivers/gpu/drm/i915/intel_fb.c |6 + drivers/gpu/drm/i915/intel_overlay2.c | 203 + 6 files changed, 294 insertions(+), 7 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_overlay2.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0ae6a7c..6193471 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -28,6 +28,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \ intel_dvo.o \ intel_ringbuffer.o \ intel_overlay.o \ + intel_overlay2.o \ intel_opregion.o \ dvo_ch7xxx.o \ dvo_ch7017.o \ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5a09416..7b128d4 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2666,6 +2666,58 @@ #define _DSPBSURF 0x7119C #define _DSPBTILEOFF 0x711A4 +/* Sprite A control */ +#define _DVSACNTR 0x72180 +#define DVS_ENABLE (1<<31) +#define DVS_GAMMA_ENABLE (1<<30) +#define DVS_PIXFORMAT_MASK (3<<25) +#define DVS_FORMAT_YUV422(0<<25) +#define DVS_FORMAT_RGBX101010(1<<25) +#define DVS_FORMAT_RGBX888 (2<<25) +#define DVS_FORMAT_RGBX161616(3<<25) +#define DVS_SOURCE_KEY (1<<22) +#define DVS_RGB_ORDER_RGBX (1<<20) +#define DVS_YUV_BYTE_ORDER_MASK (3<<16) +#define DVS_YUV_ORDER_YUYV (0<<16) +#define DVS_YUV_ORDER_UYVY (1<<16) +#define DVS_YUV_ORDER_YVYU (2<<16) +#define DVS_YUV_ORDER_VYUY (3<<16) +#define DVS_DEST_KEY (1<<2) +#define DVS_TRICKLE_FEED_DISABLE (1<<14) +#define DVS_TILED(1<<10) +#define _DVSASTRIDE0x72188 +#define _DVSAPOS 0x7218c +#define _DVSASIZE 0x72190 +#define _DVSAKEYVAL0x72194 +#define _DVSAKEYMSK0x72198 +#define _DVSASURF 0x7219c +#define _DVSAKEYMAXVAL 0x721a0 +#define _DVSATILEOFF 0x721a4 +#define _DVSASURFLIVE 0x721ac +#define _DVSASCALE 0x72204 +#define _DVSAGAMC 0x72300 + +#define _DVSBCNTR 0x73180 +#define _DVSBSTRIDE0x73188 +#define _DVSBPOS 0x7318c +#define _DVSBSIZE 0x73190 +#define _DVSBKEYVAL0x73194 +#define _DVSBKEYMSK0x73198 +#define _DVSBSURF 0x7319c +#define _DVSBKEYMAXVAL 0x731a0 +#define _DVSBTILEOFF 0x731a4 +#define _DVSBSURFLIVE 0x731ac +#define _DVSBSCALE 0x73204 +#define _DVSBGAMC 0x73300 + +#define DVSCNTR(pipe) _PIPE(pipe, _DVSACNTR, _DVSBCNTR) +#define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE) +#define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS) +#define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF) +#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE) +#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE) +#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF) + /* VBIOS regs */ #define VGACNTRL 0x71400 # define VGA_DISP_DISABLE (1 << 31) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e748338..4f599ce 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -915,8 +915,8 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv, pipe_name(pipe)); } -static void assert_pipe(struct drm_i915_private *dev_priv, - enum pipe pipe, bool state) +void assert_pipe(struct drm_i915_private *dev_priv, +enum pipe pipe, bool state) { int reg; u32 val; @@ -929,8 +929,6 @@ static void assert_pipe(struct drm_i915_private *dev_priv, "pipe %c assertion failure (expected %s, current %s)\n", pipe_name(pipe), state_string(state), state_string(cur_state)); } -#define assert_pipe_enabled(d, p) assert_pipe(d, p, true) -#define assert_pipe_disabled(d, p) assert_pipe(d, p, false) static void assert_plane_enabled(struct drm_i915_private *dev_priv, enum plane plane) @@ -4439,7 +4437,8 @@ static void ironlake_update_wm(struct drm_device *dev) ILK_LP0_CURSOR_LATENCY, &plane_wm, &cursor_wm)) { I915_WRITE(W
[PATCH 08/11] drm/i915: overlay watermark hack
--- drivers/gpu/drm/i915/intel_display.c | 11 --- 1 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4f599ce..cd7e04d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4521,7 +4521,8 @@ static void sandybridge_update_wm(struct drm_device *dev) &sandybridge_cursor_wm_info, latency, &plane_wm, &cursor_wm)) { I915_WRITE(WM0_PIPEA_ILK, - (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm); + (plane_wm << WM0_PIPE_PLANE_SHIFT) | + (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm); DRM_DEBUG_KMS("FIFO watermarks For pipe A -" " plane %d, " "cursor: %d\n", plane_wm, cursor_wm); @@ -4533,7 +4534,8 @@ static void sandybridge_update_wm(struct drm_device *dev) &sandybridge_cursor_wm_info, latency, &plane_wm, &cursor_wm)) { I915_WRITE(WM0_PIPEB_ILK, - (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm); + (plane_wm << WM0_PIPE_PLANE_SHIFT) | + (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm); DRM_DEBUG_KMS("FIFO watermarks For pipe B -" " plane %d, cursor: %d\n", plane_wm, cursor_wm); @@ -4587,11 +4589,6 @@ static void sandybridge_update_wm(struct drm_device *dev) (plane_wm << WM1_LP_SR_SHIFT) | cursor_wm); -#if 0 - I915_WRITE(WM1S_LP_ILK, - WM1S_LP_EN | -#endif - /* WM2 */ if (!ironlake_compute_srwm(dev, 2, enabled, SNB_READ_WM2_LATENCY() * 500, -- 1.7.4.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/11] drm/i915: plane teardown fixes
Make sure the object exists (it may not if the plane was previously disabled) and make sure we zero it out in the disable path to avoid trouble later. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/intel_overlay2.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index e583bd0..861e09e 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -149,12 +149,18 @@ intel_disable_plane(struct drm_plane *plane) mutex_lock(&dev->struct_mutex); + if (!intel_plane->obj) + goto out_unlock; + ret = i915_gem_object_finish_gpu(intel_plane->obj); if (ret) goto out_unlock; + i915_gem_object_unpin(intel_plane->obj); out_unlock: + intel_plane->obj = NULL; + I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE); I915_WRITE(DVSSURF(pipe), 0); POSTING_READ(DVSSURF(pipe)); -- 1.7.4.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/11] drm/i915: enable new overlay code on IVB too
Split things out a little and add the IVB reg definitions. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_reg.h | 59 drivers/gpu/drm/i915/intel_overlay2.c | 168 ++-- 2 files changed, 216 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 7b128d4..71496b8 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2718,6 +2718,65 @@ #define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE) #define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF) +#define _SPRA_CTL 0x70280 +#define SPRITE_ENABLE(1<<31) +#define SPRITE_GAMMA_ENABLE (1<<30) +#define SPRITE_PIXFORMAT_MASK(7<<25) +#define SPRITE_FORMAT_YUV422 (0<<25) +#define SPRITE_FORMAT_RGBX101010 (1<<25) +#define SPRITE_FORMAT_RGBX888(2<<25) +#define SPRITE_FORMAT_RGBX161616 (3<<25) +#define SPRITE_FORMAT_YUV444 (4<<25) +#define SPRITE_FORMAT_XBGR101010 (5<<25) +#define SPRITE_CSC_ENABLE(1<<24) +#define SPRITE_SOURCE_KEY(1<<22) +#define SPRITE_RGB_ORDER_RGBX(1<<20) /* only for 888 and 161616 */ +#define SPRITE_YUV_TO_RGB_CSC_DISABLE(1<<19) +#define SPRITE_YUV_CSC_FORMAT_BT709 (1<<18) /* 0 is BT601 */ +#define SPRITE_YUV_BYTE_ORDER_MASK (3<<16) +#define SPRITE_YUV_ORDER_YUYV(0<<16) +#define SPRITE_YUV_ORDER_UYVY(1<<16) +#define SPRITE_YUV_ORDER_YVYU(2<<16) +#define SPRITE_YUV_ORDER_VYUY(3<<16) +#define SPRITE_TRICKLE_FEED_DISABLE (1<<14) +#define SPRITE_INT_GAMMA_ENABLE (1<<13) +#define SPRITE_TILED (1<<10) +#define SPRITE_DEST_KEY (1<<2) +#define _SPRA_STRIDE 0x70288 +#define _SPRA_POS 0x7028c +#define _SPRA_SIZE 0x70290 +#define _SPRA_KEYVAL 0x70294 +#define _SPRA_KEYMSK 0x70298 +#define _SPRA_SURF 0x7029c +#define _SPRA_KEYMAX 0x702a0 +#define _SPRA_TILEOFF 0x702a4 +#define _SPRA_SCALE0x70304 +#define _SPRA_GAMC 0x70400 + +#define _SPRB_CTL 0x70280 +#define _SPRB_STRIDE 0x70288 +#define _SPRB_POS 0x7028c +#define _SPRB_SIZE 0x70290 +#define _SPRB_KEYVAL 0x70294 +#define _SPRB_KEYMSK 0x70298 +#define _SPRB_SURF 0x7029c +#define _SPRB_KEYMAX 0x702a0 +#define _SPRB_TILEOFF 0x702a4 +#define _SPRB_SCALE0x70304 +#define _SPRB_GAMC 0x70400 + +#define SPRCTL(pipe) _PIPE(pipe, _SPRA_CTL, _SPRB_CTL) +#define SPRSTRIDE(pipe) _PIPE(pipe, _SPRA_STRIDE, _SPRB_STRIDE) +#define SPRPOS(pipe) _PIPE(pipe, _SPRA_POS, _SPRB_POS) +#define SPRSIZE(pipe) _PIPE(pipe, _SPRA_SIZE, _SPRB_SIZE) +#define SPRKEYVAL(pipe) _PIPE(pipe, _SPRA_KEYVAL, _SPRB_KEYVAL) +#define SPRKEYMSK(pipe) _PIPE(pipe, _SPRA_KEYMSK, _SPRB_KEYMSK) +#define SPRSURF(pipe) _PIPE(pipe, _SPRA_SURF, _SPRB_SURF) +#define SPRKEYMAX(pipe) _PIPE(pipe, _SPRA_KEYMAX, _SPRB_KEYMAX) +#define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF) +#define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE) +#define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC) + /* VBIOS regs */ #define VGACNTRL 0x71400 # define VGA_DISP_DISABLE (1 << 31) diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index 861e09e..61b1a2f 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -36,7 +36,116 @@ #include "i915_drv.h" static int -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, +ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) +{ + struct drm_device *dev = plane->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_plane *intel_plane = to_intel_plane(plane); + struct intel_framebuffer *intel_fb; + struct drm_i915_gem_object *obj, *old_obj; + int pipe = intel_plane->pipe; + unsigned long start, offset; + u32 sprctl; + u32 reg = SPRCTL(pipe); + int ret = 0; + int x = src_x >> 16, y = src_y >> 16; + + assert_pipe_enabled(dev_priv, pipe); + + intel_fb = to_intel_framebuffer(fb); + obj = intel_fb->obj; + + old_obj = intel_plane->obj; + + mutex_lock(&dev-&g
[PATCH 09/11] drm/i915: fix overlay fb object handling
To avoid the object being destroyed before our disable hook is called, take a private reference on it. This will guarantee that we can still access the object at disable time. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/intel_overlay2.c | 27 ++- 1 files changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index 61b1a2f..8876857 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -35,6 +35,18 @@ #include "i915_drm.h" #include "i915_drv.h" +/* + * Note on refcounting: + * When the user creates an fb for the GEM object to be used for the plane, + * a ref is taken on the object. However, if the application exits before + * disabling the plane, the DRM close handling will free all the fbs and + * unless we take a ref on the object, it will be destroyed before the + * plane disable hook is called, causing obvious trouble with our efforts + * to look up and unpin the object. So we take a ref after we move the + * object to the display plane so it won't be destroyed until our disable + * hook is called and we drop our private reference. + */ + static int ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_framebuffer *fb, int crtc_x, int crtc_y, @@ -106,6 +118,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) goto out_unlock; + drm_gem_object_reference(&obj->base); + intel_plane->obj = obj; sprctl |= SPRITE_TILED; @@ -117,9 +131,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, start = obj->gtt_offset; offset = y * fb->pitch + x * (fb->bits_per_pixel / 8); - DRM_ERROR("enabling sprite, pos %d,%d, size %dx%d\n", - crtc_x, crtc_y, fb->width, fb->height); - I915_WRITE(SPRSTRIDE(pipe), fb->pitch); I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x); I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x); @@ -215,6 +226,8 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) goto out_unlock; + drm_gem_object_reference(&obj->base); + intel_plane->obj = obj; dvscntr |= DVS_TILED; @@ -260,13 +273,15 @@ ivb_disable_plane(struct drm_plane *plane) if (!intel_plane->obj) goto out_unlock; -#if 0 + ret = i915_gem_object_finish_gpu(intel_plane->obj); if (ret) goto out_unlock; i915_gem_object_unpin(intel_plane->obj); -#endif + + drm_gem_object_reference(&intel_plane->obj->base); + out_unlock: intel_plane->obj = NULL; @@ -299,6 +314,8 @@ snb_disable_plane(struct drm_plane *plane) i915_gem_object_unpin(intel_plane->obj); + drm_gem_object_reference(&intel_plane->obj->base); + out_unlock: intel_plane->obj = NULL; -- 1.7.4.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 11/11] drm/i915: add sprite scaling support
If the source and destination size are different, try to scale the sprite on the corresponding CRTC. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_reg.h |5 + drivers/gpu/drm/i915/intel_overlay2.c | 14 -- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 71496b8..8cbda0b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2695,6 +2695,11 @@ #define _DVSATILEOFF 0x721a4 #define _DVSASURFLIVE 0x721ac #define _DVSASCALE 0x72204 +#define DVS_SCALE_ENABLE (1<<31) +#define DVS_FILTER_MASK (3<<29) +#define DVS_FILTER_MEDIUM(0<<29) +#define DVS_FILTER_ENHANCING (1<<29) +#define DVS_FILTER_SOFTENING (2<<29) #define _DVSAGAMC 0x72300 #define _DVSBCNTR 0x73180 diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index 90c4f59..61594b6 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -169,7 +169,7 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_i915_gem_object *obj, *old_obj; int pipe = intel_plane->pipe; unsigned long start, offset; - u32 dvscntr; + u32 dvscntr, dvsscale = 0; u32 reg = DVSCNTR(pipe); int ret = 0; int x = src_x >> 16, y = src_y >> 16; @@ -185,6 +185,13 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (crtc_x >= active_w || crtc_y >= active_h) return -EINVAL; + /* +* We can take a larger source and scale it down, but +* only so much... 16x is the max on SNB. +*/ + if (((src_w * src_h) / (crtc_w * crtc_h)) > 16) + return -EINVAL; + /* Clamp the width & height into the visible area */ if (crtc_x + crtc_w > active_w) crtc_w = active_w - crtc_x - 1; @@ -249,11 +256,14 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, start = obj->gtt_offset; offset = y * fb->pitch + x * (fb->bits_per_pixel / 8); + if (crtc_w != src_w || crtc_h != src_h) + dvsscale = DVS_SCALE_ENABLE | (src_h << 16) | src_w; + I915_WRITE(DVSSTRIDE(pipe), fb->pitch); I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x); I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x); I915_WRITE(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); - I915_WRITE(DVSSCALE(pipe), 0); + I915_WRITE(DVSSCALE(pipe), dvsscale); I915_WRITE(reg, dvscntr); I915_WRITE(DVSSURF(pipe), start); POSTING_READ(DVSSURF(pipe)); -- 1.7.4.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/11] drm/i915: clamp sprite to viewable area
If we try to scan a sprite outside of the parent CRTC area, the display engine will underflow and potentially blank the framebuffer. So clamp the position + size to the viewable area. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/intel_overlay2.c | 12 +++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index 8876857..90c4f59 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -173,6 +173,7 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, u32 reg = DVSCNTR(pipe); int ret = 0; int x = src_x >> 16, y = src_y >> 16; + int active_w = crtc->mode.hdisplay, active_h = crtc->mode.vdisplay; assert_pipe_enabled(dev_priv, pipe); @@ -181,6 +182,15 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, old_obj = intel_plane->obj; + if (crtc_x >= active_w || crtc_y >= active_h) + return -EINVAL; + + /* Clamp the width & height into the visible area */ + if (crtc_x + crtc_w > active_w) + crtc_w = active_w - crtc_x - 1; + if (crtc_y + crtc_h > active_h) + crtc_h = active_h - crtc_y - 1; + mutex_lock(&dev->struct_mutex); dvscntr = I915_READ(reg); @@ -242,7 +252,7 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, I915_WRITE(DVSSTRIDE(pipe), fb->pitch); I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x); I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x); - I915_WRITE(DVSSIZE(pipe), (fb->height << 16) | fb->width); + I915_WRITE(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); I915_WRITE(DVSSCALE(pipe), 0); I915_WRITE(reg, dvscntr); I915_WRITE(DVSSURF(pipe), start); -- 1.7.4.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM planes and new fb creation ioctl
On Tue, 25 Oct 2011 19:47:13 +0900 Joonyoung Shim wrote: > 10/25/2011 06:46 PM, Jesse Barnes 쓴 글: > > I've given up waiting for someone to implement support for these > > ioctls on another platform before they're merged, but I have > > received a lot of feedback on the interfaces, and it sounds like > > they're ok. I've also fixed all the remaining issues I'm aware of > > on SNB platforms and things are working well, so I'm just going to > > push them out. (Note IVB support is still missing a few bits for > > scaling and such; I'll fix those up when I get back home and can > > test on IVB again.) > > > > One change you may notice from the last set is that I've removed the > > 'zpos' parameter. Plane blending and z ordering is very chipset > > specific (it even varies between Intel chipsets), so exposing it > > through a device specific ioctl is probably a better plan. > > But i think zpos is essential parameter of plane. If plane doesn't > support it, drm driver cannot know user wants to use which overlay, > so i wonder what it meant DRM_IOCTL_MODE_SETPLANE zpos is absent . Setplane is just for attaching a new fb. The order, keying, or whatever else your plane blender can support can be set with a device specific ioctl before or after the setplane call (probably before to avoid any flashing or bad frames). > If use device specific ioctl, should implement device specific ioctl > for DRM_IOCTL_MODE_SETPLANE? You could if you needed, but I don't think it's strictly necessary. > >By default, planes > > should just overlay the primary plane; a device specific ioctl (none > > available yet, but I have some planned for i915) can provide more > > flexibility. > > Could you explain what is the primary plane? Is it same as the overlay > handled by crtc? It confuses a bit when one overlay is handled by crtc > and plane at the same time. Yeah, it is a little confusing. When I refer to the primary, I'm referring to the plane bound to the CRTC. I'm fine if someone wants to break that out, I think it would make sense. I just didn't want to write the compat code that would be required for that scheme. :) But these patches definitely don't preclude it, and I don't think these interfaces will need changes if we ever move to a pipe/plane split at the userland level. Thanks, Jesse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/11] drm: add plane support
On Tue, 25 Oct 2011 19:53:02 +0900 Joonyoung Shim wrote: > > +/** > > + * drm_plane - central DRM plane control structure > > + * @dev: DRM device this plane belongs to > > + * @kdev: kernel device > > + * @attr: kdev attributes > > + * @head: for list management > > + * @base: base mode object > > + * @crtc_x: x position of plane (relative to pipe base) > > + * @crtc_y: y position of plane > > + * @x: x offset into fb > > + * @y: y offset into fb > Above 4 members don't be used. Oops yeah, I'll fix up the comments. > > + > > +struct drm_mode_get_plane { > > + __u64 format_type_ptr; > > + __u32 plane_id; > > + > > + __u32 crtc_id; > > + __u32 fb_id; > > + > > + __u32 possible_crtcs; > > + __u32 gamma_size; > > + > > + __u32 count_format_types; > > +}; > > I wonder why doesn't give to user crtc_x, crtc_y, crtc_w, crtc_h > information? It could, but the caller should already know was my thinking. Would you like those bits returned? Jesse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] DRM planes and new fb creation ioctl
On Tue, 25 Oct 2011 11:46:55 +0200 Jesse Barnes wrote: > I've given up waiting for someone to implement support for these > ioctls on another platform before they're merged, but I have received > a lot of feedback on the interfaces, and it sounds like they're ok. > I've also fixed all the remaining issues I'm aware of on SNB > platforms and things are working well, so I'm just going to push them > out. (Note IVB support is still missing a few bits for scaling and > such; I'll fix those up when I get back home and can test on IVB > again.) Btw, ignore 3-11, I forgot to collapse them when I collapsed the core fixes. Updated intel specific patch will be coming shortly (with a fix for the unpin vs disable ordering danvet pointed out). Jesse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: add SNB video sprite support
The video sprites support various video surface formats natively and can handle scaling as well. So add support for them using the new DRM core overlay support functions. v2: collapse patches and fix plane disable vs unpin ordering bug Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/i915_reg.h | 116 ++ drivers/gpu/drm/i915/intel_display.c | 26 ++- drivers/gpu/drm/i915/intel_drv.h | 14 ++ drivers/gpu/drm/i915/intel_fb.c |6 + drivers/gpu/drm/i915/intel_overlay2.c | 393 + 6 files changed, 547 insertions(+), 9 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_overlay2.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0ae6a7c..6193471 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -28,6 +28,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \ intel_dvo.o \ intel_ringbuffer.o \ intel_overlay.o \ + intel_overlay2.o \ intel_opregion.o \ dvo_ch7xxx.o \ dvo_ch7017.o \ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5a09416..8cbda0b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2666,6 +2666,122 @@ #define _DSPBSURF 0x7119C #define _DSPBTILEOFF 0x711A4 +/* Sprite A control */ +#define _DVSACNTR 0x72180 +#define DVS_ENABLE (1<<31) +#define DVS_GAMMA_ENABLE (1<<30) +#define DVS_PIXFORMAT_MASK (3<<25) +#define DVS_FORMAT_YUV422(0<<25) +#define DVS_FORMAT_RGBX101010(1<<25) +#define DVS_FORMAT_RGBX888 (2<<25) +#define DVS_FORMAT_RGBX161616(3<<25) +#define DVS_SOURCE_KEY (1<<22) +#define DVS_RGB_ORDER_RGBX (1<<20) +#define DVS_YUV_BYTE_ORDER_MASK (3<<16) +#define DVS_YUV_ORDER_YUYV (0<<16) +#define DVS_YUV_ORDER_UYVY (1<<16) +#define DVS_YUV_ORDER_YVYU (2<<16) +#define DVS_YUV_ORDER_VYUY (3<<16) +#define DVS_DEST_KEY (1<<2) +#define DVS_TRICKLE_FEED_DISABLE (1<<14) +#define DVS_TILED(1<<10) +#define _DVSASTRIDE0x72188 +#define _DVSAPOS 0x7218c +#define _DVSASIZE 0x72190 +#define _DVSAKEYVAL0x72194 +#define _DVSAKEYMSK0x72198 +#define _DVSASURF 0x7219c +#define _DVSAKEYMAXVAL 0x721a0 +#define _DVSATILEOFF 0x721a4 +#define _DVSASURFLIVE 0x721ac +#define _DVSASCALE 0x72204 +#define DVS_SCALE_ENABLE (1<<31) +#define DVS_FILTER_MASK (3<<29) +#define DVS_FILTER_MEDIUM(0<<29) +#define DVS_FILTER_ENHANCING (1<<29) +#define DVS_FILTER_SOFTENING (2<<29) +#define _DVSAGAMC 0x72300 + +#define _DVSBCNTR 0x73180 +#define _DVSBSTRIDE0x73188 +#define _DVSBPOS 0x7318c +#define _DVSBSIZE 0x73190 +#define _DVSBKEYVAL0x73194 +#define _DVSBKEYMSK0x73198 +#define _DVSBSURF 0x7319c +#define _DVSBKEYMAXVAL 0x731a0 +#define _DVSBTILEOFF 0x731a4 +#define _DVSBSURFLIVE 0x731ac +#define _DVSBSCALE 0x73204 +#define _DVSBGAMC 0x73300 + +#define DVSCNTR(pipe) _PIPE(pipe, _DVSACNTR, _DVSBCNTR) +#define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE) +#define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS) +#define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF) +#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE) +#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE) +#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF) + +#define _SPRA_CTL 0x70280 +#define SPRITE_ENABLE(1<<31) +#define SPRITE_GAMMA_ENABLE (1<<30) +#define SPRITE_PIXFORMAT_MASK(7<<25) +#define SPRITE_FORMAT_YUV422 (0<<25) +#define SPRITE_FORMAT_RGBX101010 (1<<25) +#define SPRITE_FORMAT_RGBX888(2<<25) +#define SPRITE_FORMAT_RGBX161616 (3<<25) +#define SPRITE_FORMAT_YUV444 (4<<25) +#define SPRITE_FORMAT_XBGR101010 (5<<25) +#define SPRITE_CSC_ENABLE(1<<24) +#define SPRITE_SOURCE_KEY(1<<22) +#define SPRITE_RGB_ORDER_RGBX(1<<20) /* only for 888 and 161616 */ +#define SPRITE_YUV_TO_RGB_CSC_DISABLE(1<<19) +#define SPRITE_YUV_CSC_FORMAT_BT709 (1<<18) /* 0 is BT601 */ +#define SPRITE_YUV_BYTE_ORDER_MASK (3<<16) +#define SPRITE_YUV_ORDER_YUYV(0<<16) +#define SPRITE_YUV_ORDER_UYVY(1<<16) +#define SPRITE_YUV_ORDER_YVYU
[PATCH] drm/i915: add SNB video sprite support
From 13efc0a405d522aad8250fce2dbd05fefb8b8ab0 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Fri, 22 Apr 2011 14:55:33 -0700 Subject: [PATCH] drm/i915: add SNB video sprite support The video sprites support various video surface formats natively and can handle scaling as well. So add support for them using the new DRM core overlay support functions. v2: collapse patches v3: no really, fix disable ordering Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/i915_reg.h | 116 ++ drivers/gpu/drm/i915/intel_display.c | 26 ++- drivers/gpu/drm/i915/intel_drv.h | 14 ++ drivers/gpu/drm/i915/intel_fb.c |6 + drivers/gpu/drm/i915/intel_overlay2.c | 395 + 6 files changed, 549 insertions(+), 9 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_overlay2.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0ae6a7c..6193471 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -28,6 +28,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \ intel_dvo.o \ intel_ringbuffer.o \ intel_overlay.o \ + intel_overlay2.o \ intel_opregion.o \ dvo_ch7xxx.o \ dvo_ch7017.o \ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5a09416..8cbda0b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2666,6 +2666,122 @@ #define _DSPBSURF 0x7119C #define _DSPBTILEOFF 0x711A4 +/* Sprite A control */ +#define _DVSACNTR 0x72180 +#define DVS_ENABLE (1<<31) +#define DVS_GAMMA_ENABLE (1<<30) +#define DVS_PIXFORMAT_MASK (3<<25) +#define DVS_FORMAT_YUV422(0<<25) +#define DVS_FORMAT_RGBX101010(1<<25) +#define DVS_FORMAT_RGBX888 (2<<25) +#define DVS_FORMAT_RGBX161616(3<<25) +#define DVS_SOURCE_KEY (1<<22) +#define DVS_RGB_ORDER_RGBX (1<<20) +#define DVS_YUV_BYTE_ORDER_MASK (3<<16) +#define DVS_YUV_ORDER_YUYV (0<<16) +#define DVS_YUV_ORDER_UYVY (1<<16) +#define DVS_YUV_ORDER_YVYU (2<<16) +#define DVS_YUV_ORDER_VYUY (3<<16) +#define DVS_DEST_KEY (1<<2) +#define DVS_TRICKLE_FEED_DISABLE (1<<14) +#define DVS_TILED(1<<10) +#define _DVSASTRIDE0x72188 +#define _DVSAPOS 0x7218c +#define _DVSASIZE 0x72190 +#define _DVSAKEYVAL0x72194 +#define _DVSAKEYMSK0x72198 +#define _DVSASURF 0x7219c +#define _DVSAKEYMAXVAL 0x721a0 +#define _DVSATILEOFF 0x721a4 +#define _DVSASURFLIVE 0x721ac +#define _DVSASCALE 0x72204 +#define DVS_SCALE_ENABLE (1<<31) +#define DVS_FILTER_MASK (3<<29) +#define DVS_FILTER_MEDIUM(0<<29) +#define DVS_FILTER_ENHANCING (1<<29) +#define DVS_FILTER_SOFTENING (2<<29) +#define _DVSAGAMC 0x72300 + +#define _DVSBCNTR 0x73180 +#define _DVSBSTRIDE0x73188 +#define _DVSBPOS 0x7318c +#define _DVSBSIZE 0x73190 +#define _DVSBKEYVAL0x73194 +#define _DVSBKEYMSK0x73198 +#define _DVSBSURF 0x7319c +#define _DVSBKEYMAXVAL 0x731a0 +#define _DVSBTILEOFF 0x731a4 +#define _DVSBSURFLIVE 0x731ac +#define _DVSBSCALE 0x73204 +#define _DVSBGAMC 0x73300 + +#define DVSCNTR(pipe) _PIPE(pipe, _DVSACNTR, _DVSBCNTR) +#define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE) +#define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS) +#define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF) +#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE) +#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE) +#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF) + +#define _SPRA_CTL 0x70280 +#define SPRITE_ENABLE(1<<31) +#define SPRITE_GAMMA_ENABLE (1<<30) +#define SPRITE_PIXFORMAT_MASK(7<<25) +#define SPRITE_FORMAT_YUV422 (0<<25) +#define SPRITE_FORMAT_RGBX101010 (1<<25) +#define SPRITE_FORMAT_RGBX888(2<<25) +#define SPRITE_FORMAT_RGBX161616 (3<<25) +#define SPRITE_FORMAT_YUV444 (4<<25) +#define SPRITE_FORMAT_XBGR101010 (5<<25) +#define SPRITE_CSC_ENABLE(1<<24) +#define SPRITE_SOURCE_KEY(1<<22) +#define SPRITE_RGB_ORDER_RGBX(1<<20) /* only for 888 and 161616 */ +#define SPRITE_YUV_TO_RGB_CSC_DISABLE(1<<19) +#define SPRITE_YUV_CSC_FORMAT_BT709 (1<<18) /* 0 is BT601 */ +#define SPRITE_YUV_BYTE_ORDER_MASK (3<<
Re: [PATCH 01/11] drm: add plane support
On Tue, 25 Oct 2011 13:58:55 +0200 Daniel Vetter wrote: > On Tue, Oct 25, 2011 at 11:46:56AM +0200, Jesse Barnes wrote: > > Planes are a bit like half-CRTCs. They have a location and fb, but > > don't drive outputs directly. Add support for handling them to the > > core KMS code. > > > > Signed-off-by: Jesse Barnes > > As discussed with Jesse on irc, drm fb handling is fragile. Current > rules: > - fbs are not reference counted, hence when destroying we need to > disable all crtcs (and now also planes) that use them. > drm_framebuffer_cleanup does that atm > - drivers that hold onto fbs after the kms core drops the > corresponding pointer needs to hold a ref onto the underlying backing > storage (like e.g. for pageflip on the to-be-flipped-out fb as long > as it might still be scanned out). > > We need proper refcounting for these ... But for now this patch is > missing the plane cleanup in drm_framebuffer_cleanup. Ah yeah that's a better place for the disable plane call I currently have in the intel specific code... I'll fix up and test. > Otherwise I think going with just the src and dst rect for set_plane > is about the only sensible thing given the crazy hw out there. But I > lack the knowledge about that kind of hw (and video stuff in > general), so I'll refrain from slapping my r-b on these two. Yeah, I think the drivers just need to be able to calculate the scaling level from the params and program them into whatever regs they happen to have (on Intel fortunately the scaling is figured out by hw when we program in the source & dest values, subject to some restrictions). Thanks, Jesse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/11] drm: add plane support
On Tue, 25 Oct 2011 14:26:07 +0100 Alan Cox wrote: > > As discussed with Jesse on irc, drm fb handling is fragile. Current > > rules: > > - fbs are not reference counted, hence when destroying we need to > > disable all crtcs (and now also planes) that use them. > > drm_framebuffer_cleanup does that atm > > - drivers that hold onto fbs after the kms core drops the > > corresponding pointer needs to hold a ref onto the underlying > > backing storage (like e.g. for pageflip on the to-be-flipped-out fb > > as long as it might still be scanned out). > > > > We need proper refcounting for these ... But for now this patch is > > missing the plane cleanup in drm_framebuffer_cleanup. > > I'd rather we fixed the framebuffer kref stuff as part of doing this > rather than have a poorer API because of something we have to fix > anyway. > > It shouldn't be *that* hard to fix, at least for this kind of use > case. Resize locking, fb moving etc are ugly issues, refcount > shouldn't be, and the tty layer also refcounts so we can only have > the fb objects themselves to worry about as we can defer fb > destruction to tty close or drm last unref even for those with > consoles on them. Oh it doesn't affect the userspace API so I don't think it's urgent. But I agree, it would be nice to clean up fb management a bit... Any volunteers? :) Thanks, Jesse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel