[Intel-gfx] [PATCH] drm/i915: vlv: fix stuck primary plane due to SR watermarks
Blanking/unblanking the console in a loop on an Asus T100 sometimes leaves the console blank. After some digging I found that applying commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595 Author: Egbert Eich Date: Mon Mar 4 09:24:38 2013 -0500 DRM/i915: On G45 enable cursor plane briefly after enabling the display plane. fixed VLV too. At least in my case the problem seems to happen already during the previous crtc disabling, and it goes away if we disable SR watermarks before disabling the primary plane. I also noticed that the problem only happens if the C7S CPU idle state is entered, disabling that also gets rid of the problem. As an alternative I also tried to increase the plane SR watermark close to its maximum but that didn't solve the issue. Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 9 + drivers/gpu/drm/i915/intel_pm.c | 7 +++ 3 files changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bea9ab40..afee677 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2591,6 +2591,7 @@ extern void gen6_set_rps(struct drm_device *dev, u8 val); extern void valleyview_set_rps(struct drm_device *dev, u8 val); extern int valleyview_rps_max_freq(struct drm_i915_private *dev_priv); extern int valleyview_rps_min_freq(struct drm_i915_private *dev_priv); +extern void valleyview_disable_sr_watermarks(struct drm_i915_private *dev_priv); extern void intel_detect_pch(struct drm_device *dev); extern int intel_trans_dp_port_sel(struct drm_crtc *crtc); extern int intel_enable_rc6(const struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 54095d4..11611e1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4767,6 +4767,15 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) if (IS_GEN2(dev)) intel_set_cpu_fifo_underrun_reporting(dev, pipe, false); + /* +* Having the SR WMs enabled when disabling the primary plane may +* leave the primary plane in a stuck state, where it wouldn't +* start fetching pixels after a subsequent crtc enable. +* The WMs will be set to their proper values again when calling +* intel_update_watermarks() next. +*/ + if (IS_VALLEYVIEW(dev)) + valleyview_disable_sr_watermarks(dev_priv); intel_crtc_disable_planes(crtc); for_each_encoder_on_crtc(dev, crtc, encoder) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1840d15..01962aa 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1306,6 +1306,13 @@ static void vlv_update_drain_latency(struct drm_device *dev) } } +void valleyview_disable_sr_watermarks(struct drm_i915_private *dev_priv) +{ + I915_WRITE(FW_BLC_SELF_VLV, + I915_READ(FW_BLC_SELF_VLV) & ~FW_CSPWRDWNEN); + POSTING_READ(FW_BLC_SELF_VLV); +} + #define single_plane_enabled(mask) is_power_of_2(mask) static void valleyview_update_wm(struct drm_crtc *crtc) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] intel_fb_initilal_config encoder->crtc warn
Hi, Just wondering what the point of the WARN(!encoder->crtc) in that function is, I hit this with MST and I can't see what it should matter, where I hit it is if I dock MST while X is running and VT switch, I get it, I first wondered when encoder would ever be false in non-MST world anyways, but I've added a check to find a valid MST encoder at this point, but none of them have a crtc assigned as of yet as they haven't been configured, even though I expect X has configured them, fbdev would turn them off on vt switch I assume. Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 2/2] drm/i915: State readout and cross-checking for dp_m2_n2
Hi Daniel, Please let me know if this patch (http://lists.freedesktop.org/archives/intel-gfx/2014-May/045877.html) and the patch http://lists.freedesktop.org/archives/intel-gfx/2014-May/045804.html require any other changes.. I have included all your review comments till date.. Thanks, Vandana On May-22-2014 12:18 PM, Kannan, Vandana wrote: > Adding relevant read out comparison code, in check_crtc_state, for the new > member of crtc_config, dp_m2_n2, which was introduced to store link_m_n > values for a DP downclock mode (if available). Suggested by Daniel. > > v2: Changed patch title. > Daniel's review comments incorporated. > Added relevant state readout code for M2_N2. dp_m2_n2 comparison to be done > only when high RR is not in use (This is because alternate m_n register > programming will be done only when low RR is being used). > > v3: Modified call to get_m2_n2 which had dp_m_n as param by mistake. > Compare dp_m_n and dp_m2_n2 for gen 7 and below. compare the structures > based on DRRS state for gen 8 and above. > Save and restore M2 N2 registers for gen 7 and below > > v4: For Gen>=8, check M_N registers against dp_m_n and dp_m2_n2 as there is > only one set of M_N registers > > v5: Removed the chunk which saves and restores M2_N2 registers. Modified > get_m_n() to get M2_N2 registers as well. Modified the macro which compares > hw.dp_m_n against sw.dp_m2_n2/sw.dp_m_n for gen > 8. > > Signed-off-by: Vandana Kannan > Cc: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 68 > +++- > 1 file changed, 60 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index cf3ad87..d593897 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6945,7 +6945,8 @@ static void intel_pch_transcoder_get_m_n(struct > intel_crtc *crtc, > > static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc, >enum transcoder transcoder, > - struct intel_link_m_n *m_n) > + struct intel_link_m_n *m_n, > + struct intel_link_m_n *m2_n2) > { > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -6959,6 +6960,15 @@ static void intel_cpu_transcoder_get_m_n(struct > intel_crtc *crtc, > m_n->gmch_n = I915_READ(PIPE_DATA_N1(transcoder)); > m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder)) > & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1; > + if (m2_n2 && INTEL_INFO(dev)->gen < 8) { > + m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder)); > + m2_n2->link_n = I915_READ(PIPE_LINK_N2(transcoder)); > + m2_n2->gmch_m = I915_READ(PIPE_DATA_M2(transcoder)) > + & ~TU_SIZE_MASK; > + m2_n2->gmch_n = I915_READ(PIPE_DATA_N2(transcoder)); > + m2_n2->tu = ((I915_READ(PIPE_DATA_M2(transcoder)) > + & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1; > + } > } else { > m_n->link_m = I915_READ(PIPE_LINK_M_G4X(pipe)); > m_n->link_n = I915_READ(PIPE_LINK_N_G4X(pipe)); > @@ -6977,14 +6987,15 @@ void intel_dp_get_m_n(struct intel_crtc *crtc, > intel_pch_transcoder_get_m_n(crtc, &pipe_config->dp_m_n); > else > intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder, > - &pipe_config->dp_m_n); > + &pipe_config->dp_m_n, > + &pipe_config->dp_m2_n2); > } > > static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc, > struct intel_crtc_config *pipe_config) > { > intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder, > - &pipe_config->fdi_m_n); > + &pipe_config->fdi_m_n, NULL); > } > > static void ironlake_get_pfit_config(struct intel_crtc *crtc, > @@ -9485,6 +9496,15 @@ static void intel_dump_pipe_config(struct intel_crtc > *crtc, > pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n, > pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n, > pipe_config->dp_m_n.tu); > + > + DRM_DEBUG_KMS("dp: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2: > %u, tu2: %u\n", > + pipe_config->has_dp_encoder, > + pipe_config->dp_m2_n2.gmch_m, > + pipe_config->dp_m2_n2.gmch_n, > + pipe_config->dp_m2_n2.link_m, > + pipe_config->dp_m2_n2.link_n, > + pipe_config->dp_m
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Allow vblank interrupts during modeset and eliminate some vblank races
On 29.05.2014 19:56, Daniel Vetter wrote: > On Thu, May 29, 2014 at 6:11 AM, Michel Dänzer wrote: >> >>> We could rename the off/on to pre/post_modeset if you like, but then >>> someone gets to audit all the other drivers. That someone isn't >>> going to be me. >> >> I appreciate your caution wrt other drivers, but I'm worried that having >> a second mechanism for keeping the DRM vblank counter consistent might >> cause subtle problems for drivers using the existing mechanism anyway. > > I think that risk is rather low. Only i915 has been stupid enough to > use both drm_vblank_off + pre/post_modeset in the normal crtc > enable/disable functions. Everyone else uses either or the other, > except for tegra which uses pre/post in the ->prepare/commit hooks and > off in the crtc->disable callback. So should still get consistent > results (since after ->disable vblank consistency stops mattering). > > The reason we do all this is that we want to kill the delayed vblank > put for i915, which is possible if you have a hw vblank counter. That should also be possible with pre/post_modeset. (Not sure eliminating it completely is a good idea for the reasons you mentioned before, but at least decreasing it significantly should be no problem. I think even dropping the default a lot for all drivers should be rather low risk) > There's apparently more stuff wrong here still, so while we wrestle > around probably best to leave other drivers out. I guess once this is > all done I have to roll it out across all drivers so that we have > consistent behaviour again ... Since AFAICT radeon isn't affected by the problem drm_vblank_off() was supposed to solve originally, I'd prefer if it didn't start using that and potentially suffer from all the trouble it seems to cause. -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] ACPI: export target system state for use by drivers
On Thursday, May 29, 2014 02:48:44 PM Jesse Barnes wrote: > From: Kristen Carlson Accardi > > Needed in ->freeze routines to figure out the target system state and > take appropriate action. > > v2: split out ACPI patch > > Signed-off-by: Kristen Carlson Accardi > Signed-off-by: Jesse Barnes This is in my linux-next branch already. I can put out a separate branch with it for you to pull from if necessary. Rafael > --- > drivers/acpi/sleep.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c > index c40fb2e..807f333 100644 > --- a/drivers/acpi/sleep.c > +++ b/drivers/acpi/sleep.c > @@ -89,6 +89,7 @@ u32 acpi_target_system_state(void) > { > return acpi_target_sleep_state; > } > +EXPORT_SYMBOL(acpi_target_system_state); > > static bool pwr_btn_event_pending; > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: Split connection_mutex out of mode_config.mutex (v2)
After the split-out of crtc locks from the big mode_config.mutex there's still two major areas it protects: - Various connector probe states, like connector->status, EDID properties, probed mode lists and similar information. - The links from connector->encoder and encoder->crtc and other modeset-relevant connector state (e.g. properties which control the panel fitter). The later is used by modeset operations. But they don't really care about the former since it's allowed to e.g. enable a disconnected VGA output or with a mode not in the probed list. Thus far this hasn't been a problem, but for the atomic modeset conversion Rob Clark needs to convert all modeset relevant locks into w/w locks. This is required because the order of acquisition is determined by how userspace supplies the atomic modeset data. This has run into troubles in the detect path since the i915 load detect code needs _both_ protections offered by the mode_config.mutex: It updates probe state and it needs to change the modeset configuration to enable the temporary load detect pipe. The big deal here is that for the probe/detect users of this lock a plain mutex fits best, but for atomic modesets we really want a w/w mutex. To fix this lets split out a new connection_mutex lock for the modeset relevant parts. For simplicity I've decided to only add one additional lock for all connector/encoder links and modeset configuration states. We have piles of different modeset objects in addition to those (like bridges or panels), so adding per-object locks would be much more effort. Also, we're guaranteed (at least for now) to do a full modeset if we need to acquire this lock. Which means that fine-grained locking is fairly irrelevant compared to the amount of time the full modeset will take. I've done a full audit, and there's just a few things that justify special focus: - Locking in drm_sysfs.c is almost completely absent. We should sprinkle mode_config.connection_mutex over this file a bit, but since it already lacks mode_config.mutex this patch wont make the situation any worse. This is material for a follow-up patch. - omap has a omap_framebuffer_flush function which walks the connector->encoder->crtc links and is called from many contexts. Some look like they don't acquire mode_config.mutex, so this is already racy. Again fixing this is material for a separate patch. - The radeon hot_plug function to retrain DP links looks at connector->dpms. Currently this happens without any locking, so is already racy. I think radeon_hotplug_work_func should gain mutex_lock/unlock calls for the mode_config.connection_mutex. - Same applies to i915's intel_dp_hot_plug. But again, this is already racy. - i915 load_detect code needs to acquire this lock. Which means the w/w dance due to Rob's work will be nicely contained to _just_ this function. I've added fixme comments everywhere where it looks suspicious but in the sysfs code. After a quick irc discussion with Dave Airlie it sounds like the lack of locking in there is due to sysfs cleanup fun at module unload. v1: original (only compile tested) v2: missing mutex_init(), etc (from Rob Clark) v3: i915 needs more care in the conversion: - Protect the edp pp logic with the connection_mutex. - Use connection_mutex in the backlight code due to get_pipe_from_connector. - Use drm_modeset_lock_all in suspend/resume paths. - Update lock checks in the overlay code. Cc: Alex Deucher Cc: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 8 drivers/gpu/drm/drm_crtc_helper.c | 2 ++ drivers/gpu/drm/drm_edid.c | 2 ++ drivers/gpu/drm/drm_plane_helper.c | 7 +++ drivers/gpu/drm/i915/i915_debugfs.c| 4 ++-- drivers/gpu/drm/i915/i915_drv.c| 9 +++-- drivers/gpu/drm/i915/intel_display.c | 16 drivers/gpu/drm/i915/intel_dp.c| 15 --- drivers/gpu/drm/i915/intel_opregion.c | 4 ++-- drivers/gpu/drm/i915/intel_overlay.c | 4 ++-- drivers/gpu/drm/i915/intel_panel.c | 8 drivers/gpu/drm/omapdrm/omap_fb.c | 1 + drivers/gpu/drm/radeon/radeon_connectors.c | 1 + include/drm/drm_crtc.h | 1 + 14 files changed, 55 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 37a3e0791ddf..ddd9d78e9000 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -54,6 +54,8 @@ void drm_modeset_lock_all(struct drm_device *dev) mutex_lock(&dev->mode_config.mutex); + mutex_lock(&dev->mode_config.connection_mutex); + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) mutex_lock_nest_lock(&crtc->mutex, &dev->mode_config.mutex); } @@ -72,6 +74,8 @@ void drm_modeset_unlock_all(struct drm_device *dev) list_for_each_entry(crtc, &dev->mode_config.crtc_list
[Intel-gfx] [PATCH 3/5] ACPI: export target system state for use by drivers
From: Kristen Carlson Accardi Needed in ->freeze routines to figure out the target system state and take appropriate action. v2: split out ACPI patch Signed-off-by: Kristen Carlson Accardi Signed-off-by: Jesse Barnes --- drivers/acpi/sleep.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index c40fb2e..807f333 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -89,6 +89,7 @@ u32 acpi_target_system_state(void) { return acpi_target_sleep_state; } +EXPORT_SYMBOL(acpi_target_system_state); static bool pwr_btn_event_pending; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/i915: send proper opregion notifications on suspend/resume
This indicates to the firmware that it can power down various other components or bring them back up, depending on the target system state. Signed-off-by: Kristen Carlson Accardi Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 433bdfa..b6211d7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -28,6 +28,7 @@ */ #include +#include #include #include #include "i915_drv.h" @@ -491,6 +492,7 @@ static int i915_drm_freeze(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc; + pci_power_t opregion_target_state; intel_runtime_pm_get(dev_priv); @@ -540,6 +542,12 @@ static int i915_drm_freeze(struct drm_device *dev) i915_save_state(dev); + if (acpi_target_system_state() >= ACPI_STATE_S3) + opregion_target_state = PCI_D3cold; + else + opregion_target_state = PCI_D1; + intel_opregion_notify_adapter(dev, opregion_target_state); + intel_opregion_fini(dev); console_lock(); @@ -671,6 +679,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) dev_priv->modeset_restore = MODESET_DONE; mutex_unlock(&dev_priv->modeset_restore_lock); + intel_opregion_notify_adapter(dev, PCI_D0); + intel_runtime_pm_put(dev_priv); return 0; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915: use VBT to determine whether to enumerate the VGA port
On Fri, Apr 04, 2014 at 04:12:07PM -0700, Jesse Barnes wrote: > Some platforms may not have it, and enumerating it is both confusing and > time consuming due to the hotplug and DDC probing. > > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/intel_display.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 3697433..6a6406f 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10678,7 +10678,7 @@ static void intel_setup_outputs(struct drm_device > *dev) > > intel_lvds_init(dev); > > - if (!IS_ULT(dev)) > + if (!IS_ULT(dev) && dev_priv->vbt.int_crt_support) > intel_crt_init(dev); > > if (HAS_DDI(dev)) { For the love of god, can we please rename "bits X" in the struct bdb_general_features? That caused me endless pain. It should be "byte 0" Also, I guess there is a problem when the VBT is missing, and we have a dont have a CRT on non-ULT (since the default seems to be true). This however is no worse than the current situation AFAICT. Reviewed-by: Ben Widawsky -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: enable PPGTT on VLV
Working for real this time. i915_ppgtt_info has all sorts of good stuff in it and X is running nicely on top. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_drv.h | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bea9ab40..8631fb3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1936,10 +1936,8 @@ struct drm_i915_cmd_table { #define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)->need_gfx_hws) #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) -#define HAS_ALIASING_PPGTT(dev)(INTEL_INFO(dev)->gen >= 6 && \ -(!IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))) -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ -&& !IS_GEN8(dev)) +#define HAS_ALIASING_PPGTT(dev)(INTEL_INFO(dev)->gen >= 6) +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_GEN8(dev)) #define USES_PPGTT(dev)intel_enable_ppgtt(dev, false) #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Drop locking around fbdev-fb in debugfs
All the date we print is invariant for the lifetime of the driver. And none of it would be protected by the mode_config.mutex anyway. So drop it. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_debugfs.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 538b83559593..5858cbb5232f 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1675,9 +1675,6 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data) #ifdef CONFIG_DRM_I915_FBDEV struct drm_i915_private *dev_priv = dev->dev_private; - int ret = mutex_lock_interruptible(&dev->mode_config.mutex); - if (ret) - return ret; ifbdev = dev_priv->fbdev; fb = to_intel_framebuffer(ifbdev->helper.fb); @@ -1690,7 +1687,6 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data) atomic_read(&fb->base.refcount.refcount)); describe_obj(m, fb->obj); seq_putc(m, '\n'); - mutex_unlock(&dev->mode_config.mutex); #endif mutex_lock(&dev->mode_config.fb_lock); -- 1.9.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix context locking in debugfs
This goes all the way back to the introduction of this debugfs file, even though back then no locking really was required. None of the intermediate patches fixed this. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_debugfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 2e5f76a585ef..538b83559593 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1721,7 +1721,7 @@ static int i915_context_status(struct seq_file *m, void *unused) struct intel_context *ctx; int ret, i; - ret = mutex_lock_interruptible(&dev->mode_config.mutex); + ret = mutex_lock_interruptible(&dev->struct_mutex); if (ret) return ret; @@ -1751,7 +1751,7 @@ static int i915_context_status(struct seq_file *m, void *unused) seq_putc(m, '\n'); } - mutex_unlock(&dev->mode_config.mutex); + mutex_unlock(&dev->struct_mutex); return 0; } -- 1.9.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Breaking suspend/resume by the Pipe A quirk
Hi Daniel, hi folks, according to my knowledge, the pipe A quirk is unconditionally enabled on the 830 to allow resume to work properly. Unfortunately, it does quite the opposite on the S6010, it breaks resume completely. If the pipe A quirk is disabled, then the boot console works correctly. Resume does not, the display is dead, but it is possible to remotely connect to the machine, from there POST the video card (via vbetool post), stop X, then restart X, then the display is back. If the pipe A quirk is enabled, then try to resume from suspend, then the machine is dead completely. You can ping it, but not log in. I currently have not yet tried to figure out where it hangs, but I would suspect the problem is somewhere in the i915 kernel module. The display just stays black. Thus, in addition to the watermark fix I proposed, please *disable* the unconditional pipe A quirk for the 830GM since it really breaks things, not only the boot console. Greetings, Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: enable PPGTT
On Thu, May 29, 2014 at 08:44:51AM -0700, Jesse Barnes wrote: > On Thu, 29 May 2014 08:30:10 -0700 > "Volkin, Bradley D" wrote: > > > On Wed, May 28, 2014 at 03:02:24PM -0700, Jesse Barnes wrote: > > > Need testing and possibly disabling on earlier steppings, but looks ok > > > here on my B3. > > > > > > Signed-off-by: Jesse Barnes > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h > > > index ec5f6fb..4b0e58c 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1971,7 +1971,7 @@ struct drm_i915_cmd_table { > > > > > > #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) > > > #define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && > > > !IS_VALLEYVIEW(dev)) > > > -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && > > > !IS_VALLEYVIEW(dev) \ > > > +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ > > >&& !IS_BROADWELL(dev)) > > > > Which branch is this against? In -nightly, these are implemented > > differently, so I don't know if all comments will apply. But... > > > > Since IS_VALLEYVIEW() is also true for chv, should we make the condition > > !IS_CHERRYVIEW() in this patch instead? Or do we intend to have PPGTT > > enabled for chv? > > > > I think we need the corresponding change to HAS_ALIASING_PPGTT() in order > > to actually enable PPGTT. Otherwise it looks like sanitize_enable_ppgtt() > > will disable PPGTT. > > I knew this was too easy, maybe that's why it worked so well for Ville > and I... unless I was somehow testing full PPGTT. I'll check it out. #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) -#define HAS_ALIASING_PPGTT(dev)(INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) \ +#define HAS_ALIASING_PPGTT(dev)(INTEL_INFO(dev)->gen >= 6) +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ && !IS_BROADWELL(dev)) is what I see in my branch, so I'm pretty sure I actually tested aliasing ppgtt. And IIRC I tried full ppgtt too (such as it was ~2 months ago). -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time
From: Kristen Carlson Accardi This allows the system to enter the lowest power mode during system freeze. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_drv.c | 3 --- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 16 +++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 66c6ffb..433bdfa 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -521,8 +521,6 @@ static int i915_drm_freeze(struct drm_device *dev) drm_irq_uninstall(dev); dev_priv->enable_hotplug_processing = false; - intel_disable_gt_powersave(dev); - /* * Disable CRTCs directly since we want to preserve sw state * for _thaw. @@ -543,7 +541,6 @@ static int i915_drm_freeze(struct drm_device *dev) i915_save_state(dev); intel_opregion_fini(dev); - intel_uncore_fini(dev); console_lock(); intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index c597b0d..bf90e7d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -956,6 +956,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv); void intel_init_gt_powersave(struct drm_device *dev); void intel_cleanup_gt_powersave(struct drm_device *dev); void intel_enable_gt_powersave(struct drm_device *dev); +void intel_enable_gt_powersave_sync(struct drm_device *dev); void intel_disable_gt_powersave(struct drm_device *dev); void intel_reset_gt_powersave(struct drm_device *dev); void ironlake_teardown_rc6(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1840d15..8d9e036 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4891,12 +4891,9 @@ void intel_disable_gt_powersave(struct drm_device *dev) } } -static void intel_gen6_powersave_work(struct work_struct *work) +void intel_enable_gt_powersave_sync(struct drm_device *dev) { - struct drm_i915_private *dev_priv = - container_of(work, struct drm_i915_private, -rps.delayed_resume_work.work); - struct drm_device *dev = dev_priv->dev; + struct drm_i915_private *dev_priv = dev->dev_private; mutex_lock(&dev_priv->rps.hw_lock); @@ -4917,6 +4914,15 @@ static void intel_gen6_powersave_work(struct work_struct *work) intel_runtime_pm_put(dev_priv); } +static void intel_gen6_powersave_work(struct work_struct *work) +{ + struct drm_i915_private *dev_priv = + container_of(work, struct drm_i915_private, +rps.delayed_resume_work.work); + + intel_enable_gt_powersave_sync(dev_priv->dev); +} + void intel_enable_gt_powersave(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: disable power wells on suspend
From: Kristen Carlson Accardi We want to make sure everything is disabled and at its lowest power when freezing. Signed-off-by: Kristen Carlson Accardi Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index e2bfdda..66c6ffb 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -551,6 +551,8 @@ static int i915_drm_freeze(struct drm_device *dev) dev_priv->suspend_count++; + intel_display_set_init_power(dev_priv, false); + return 0; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: send proper opregion notifications on suspend/resume
From: Kristen Carlson Accardi This indicates to the firmware that it can power down various other components or bring them back up, depending on the target system state. Signed-off-by: Kristen Carlson Accardi Signed-off-by: Jesse Barnes --- drivers/acpi/sleep.c| 1 + drivers/gpu/drm/i915/i915_drv.c | 10 ++ 2 files changed, 11 insertions(+) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index c40fb2e..807f333 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -89,6 +89,7 @@ u32 acpi_target_system_state(void) { return acpi_target_sleep_state; } +EXPORT_SYMBOL(acpi_target_system_state); static bool pwr_btn_event_pending; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 433bdfa..b6211d7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -28,6 +28,7 @@ */ #include +#include #include #include #include "i915_drv.h" @@ -491,6 +492,7 @@ static int i915_drm_freeze(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc; + pci_power_t opregion_target_state; intel_runtime_pm_get(dev_priv); @@ -540,6 +542,12 @@ static int i915_drm_freeze(struct drm_device *dev) i915_save_state(dev); + if (acpi_target_system_state() >= ACPI_STATE_S3) + opregion_target_state = PCI_D3cold; + else + opregion_target_state = PCI_D1; + intel_opregion_notify_adapter(dev, opregion_target_state); + intel_opregion_fini(dev); console_lock(); @@ -671,6 +679,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) dev_priv->modeset_restore = MODESET_DONE; mutex_unlock(&dev_priv->modeset_restore_lock); + intel_opregion_notify_adapter(dev, PCI_D0); + intel_runtime_pm_put(dev_priv); return 0; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume
From: Kristen Carlson Accardi This matches the runtime suspend paths and allows the system to enter the lowest power mode at freeze time. Signed-off-by: Kristen Carlson Accardi Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_drv.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b6211d7..24dc856 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); + if (IS_HASWELL(dev) || IS_BROADWELL(dev)) + hsw_enable_pc8(dev_priv); + return 0; } @@ -618,6 +621,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) { struct drm_i915_private *dev_priv = dev->dev_private; + if (IS_HASWELL(dev) || IS_BROADWELL(dev)) + hsw_disable_pc8(dev_priv); + if (drm_core_check_feature(dev, DRIVER_MODESET) && restore_gtt_mappings) { mutex_lock(&dev->struct_mutex); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] connector debugfs properties
The following series adds support for exposing various connector features using debugfs. The first patch refactors the sysfs connector add and remove functions into generic functions to register and unregister connectors. The remaining patches add an interface for each connector to debugfs and expose the force connector attribute and allow the edid value to be overridden. Thomas Wood (3): drm: add register and unregister functions for connectors drm/debugfs: add a "force" file per connector drm/debugfs: add an "edid_override" file per connector Documentation/DocBook/drm.tmpl| 6 +- drivers/gpu/drm/armada/armada_output.c| 4 +- drivers/gpu/drm/ast/ast_mode.c| 4 +- drivers/gpu/drm/bridge/ptn3460.c | 2 +- drivers/gpu/drm/drm_crtc.c| 49 +++- drivers/gpu/drm/drm_debugfs.c | 157 ++ drivers/gpu/drm/drm_probe_helper.c| 9 +- drivers/gpu/drm/drm_sysfs.c | 2 - drivers/gpu/drm/exynos/exynos_dp_core.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_connector.c | 6 +- drivers/gpu/drm/exynos/exynos_drm_dpi.c | 4 +- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 2 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- drivers/gpu/drm/gma500/cdv_intel_crt.c| 4 +- drivers/gpu/drm/gma500/cdv_intel_dp.c | 4 +- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 4 +- drivers/gpu/drm/gma500/cdv_intel_lvds.c | 4 +- drivers/gpu/drm/gma500/mdfld_dsi_output.c | 4 +- drivers/gpu/drm/gma500/oaktrail_hdmi.c| 2 +- drivers/gpu/drm/gma500/oaktrail_lvds.c| 2 +- drivers/gpu/drm/gma500/psb_intel_lvds.c | 4 +- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 4 +- drivers/gpu/drm/i915/intel_crt.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_dp.c | 4 +- drivers/gpu/drm/i915/intel_dsi.c | 2 +- drivers/gpu/drm/i915/intel_dvo.c | 2 +- drivers/gpu/drm/i915/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/intel_lvds.c | 2 +- drivers/gpu/drm/i915/intel_sdvo.c | 10 +- drivers/gpu/drm/i915/intel_tv.c | 2 +- drivers/gpu/drm/mgag200/mgag200_mode.c| 2 +- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 4 +- drivers/gpu/drm/nouveau/nouveau_connector.c | 4 +- drivers/gpu/drm/omapdrm/omap_connector.c | 4 +- drivers/gpu/drm/qxl/qxl_display.c | 4 +- drivers/gpu/drm/radeon/radeon_connectors.c| 6 +- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 4 +- drivers/gpu/drm/rcar-du/rcar_du_vgacon.c | 4 +- drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 6 +- drivers/gpu/drm/tegra/output.c| 4 +- drivers/gpu/drm/tilcdc/tilcdc_panel.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_tfp410.c| 2 +- drivers/gpu/drm/udl/udl_connector.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +- drivers/staging/imx-drm/imx-drm-core.c| 6 +- include/drm/drmP.h| 11 ++ include/drm/drm_crtc.h| 5 + 52 files changed, 308 insertions(+), 83 deletions(-) -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm: add register and unregister functions for connectors
Introduce generic functions to register and unregister connectors. This provides a common place to add and remove associated user space interfaces. Signed-off-by: Thomas Wood --- Documentation/DocBook/drm.tmpl| 6 +++--- drivers/gpu/drm/armada/armada_output.c| 4 ++-- drivers/gpu/drm/ast/ast_mode.c| 4 ++-- drivers/gpu/drm/bridge/ptn3460.c | 2 +- drivers/gpu/drm/drm_crtc.c| 30 ++- drivers/gpu/drm/drm_sysfs.c | 2 -- drivers/gpu/drm/exynos/exynos_dp_core.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_connector.c | 6 +++--- drivers/gpu/drm/exynos/exynos_drm_dpi.c | 4 ++-- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 2 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- drivers/gpu/drm/gma500/cdv_intel_crt.c| 4 ++-- drivers/gpu/drm/gma500/cdv_intel_dp.c | 4 ++-- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 4 ++-- drivers/gpu/drm/gma500/cdv_intel_lvds.c | 4 ++-- drivers/gpu/drm/gma500/mdfld_dsi_output.c | 4 ++-- drivers/gpu/drm/gma500/oaktrail_hdmi.c| 2 +- drivers/gpu/drm/gma500/oaktrail_lvds.c| 2 +- drivers/gpu/drm/gma500/psb_intel_lvds.c | 4 ++-- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 4 ++-- drivers/gpu/drm/i915/intel_crt.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_dp.c | 4 ++-- drivers/gpu/drm/i915/intel_dsi.c | 2 +- drivers/gpu/drm/i915/intel_dvo.c | 2 +- drivers/gpu/drm/i915/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/intel_lvds.c | 2 +- drivers/gpu/drm/i915/intel_sdvo.c | 10 - drivers/gpu/drm/i915/intel_tv.c | 2 +- drivers/gpu/drm/mgag200/mgag200_mode.c| 2 +- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 4 ++-- drivers/gpu/drm/nouveau/nouveau_connector.c | 4 ++-- drivers/gpu/drm/omapdrm/omap_connector.c | 4 ++-- drivers/gpu/drm/qxl/qxl_display.c | 4 ++-- drivers/gpu/drm/radeon/radeon_connectors.c| 6 +++--- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 4 ++-- drivers/gpu/drm/rcar-du/rcar_du_vgacon.c | 4 ++-- drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 6 +++--- drivers/gpu/drm/tegra/output.c| 4 ++-- drivers/gpu/drm/tilcdc/tilcdc_panel.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_tfp410.c| 2 +- drivers/gpu/drm/udl/udl_connector.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +- drivers/staging/imx-drm/imx-drm-core.c| 6 +++--- include/drm/drm_crtc.h| 2 ++ 49 files changed, 110 insertions(+), 82 deletions(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 00f1c25..0f96b25 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -1574,7 +1574,7 @@ int max_width, max_height; The connector is then registered with a call to drm_connector_init with a pointer to the connector functions and a connector type, and exposed through sysfs with a call to - drm_sysfs_connector_add. + drm_connector_register. Supported connector types are @@ -1732,7 +1732,7 @@ int max_width, max_height; (drm_encoder_cleanup) and connectors (drm_connector_cleanup). Furthermore, connectors that have been added to sysfs must be removed by a call to - drm_sysfs_connector_remove before calling + drm_connector_unregister before calling drm_connector_cleanup. @@ -1777,7 +1777,7 @@ void intel_crt_init(struct drm_device *dev) drm_encoder_helper_add(&intel_output->enc, &intel_crt_helper_funcs); drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs); - drm_sysfs_connector_add(connector); + drm_connector_register(connector); }]]> In the example above (taken from the i915 driver), a CRTC, connector and diff --git a/drivers/gpu/drm/armada/armada_output.c b/drivers/gpu/drm/armada/armada_output.c index d685a54..abbc309 100644 --- a/drivers/gpu/drm/armada/armada_output.c +++ b/drivers/gpu/drm/armada/armada_output.c @@ -48,7 +48,7 @@ static void armada_drm_connector_destroy(struct drm_connector *conn) { struct armada_connector *dconn = drm_to_armada_conn(conn); - drm_sysfs_connector_remove(conn); + drm_connector_unregister(conn); drm_connector_cleanup(conn); kfree(dconn); } @@ -141,7 +141,7 @@ int armada_output_create(struct drm_device *dev, if (ret) goto err_conn;
[Intel-gfx] [PATCH 2/3] drm/debugfs: add a "force" file per connector
Add a file to debugfs for each connector to enable modification of the "force" connector attribute. This allows connectors to be enabled or disabled for testing and debugging purposes. Signed-off-by: Thomas Wood --- drivers/gpu/drm/drm_crtc.c| 17 ++- drivers/gpu/drm/drm_debugfs.c | 101 ++ include/drm/drmP.h| 11 + include/drm/drm_crtc.h| 2 + 4 files changed, 130 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 998663c..59a2784 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -841,6 +841,8 @@ int drm_connector_init(struct drm_device *dev, drm_object_attach_property(&connector->base, dev->mode_config.dpms_property, 0); + connector->debugfs_entry = NULL; + out_put: if (ret) drm_mode_object_put(dev, &connector->base); @@ -891,7 +893,19 @@ EXPORT_SYMBOL(drm_connector_cleanup); */ int drm_connector_register(struct drm_connector *connector) { - return drm_sysfs_connector_add(connector); + int ret; + + ret = drm_sysfs_connector_add(connector); + if (ret != 0) + return ret; + + ret = drm_debugfs_connector_add(connector); + if (ret != 0) { + drm_sysfs_connector_remove(connector); + return ret; + } + + return 0; } EXPORT_SYMBOL(drm_connector_register); @@ -904,6 +918,7 @@ EXPORT_SYMBOL(drm_connector_register); void drm_connector_unregister(struct drm_connector *connector) { drm_sysfs_connector_remove(connector); + drm_debugfs_connector_remove(connector); } EXPORT_SYMBOL(drm_connector_unregister); diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index b4b51d4..b57b614 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -237,5 +237,106 @@ int drm_debugfs_cleanup(struct drm_minor *minor) return 0; } +static int connector_show(struct seq_file *m, void *data) +{ + struct drm_connector *connector = m->private; + const char *status; + + switch (connector->force) { + case DRM_FORCE_ON: + status = "on\n"; + break; + + case DRM_FORCE_ON_DIGITAL: + status = "digital\n"; + break; + + case DRM_FORCE_OFF: + status = "off\n"; + break; + + case DRM_FORCE_UNSPECIFIED: + status = "unspecified\n"; + break; + + default: + return 0; + } + + seq_puts(m, status); + + return 0; +} + +static int connector_open(struct inode *inode, struct file *file) +{ + struct drm_connector *dev = inode->i_private; + + return single_open(file, connector_show, dev); +} + +static ssize_t connector_write(struct file *file, const char __user *ubuf, + size_t len, loff_t *offp) +{ + struct seq_file *m = file->private_data; + struct drm_connector *connector = m->private; + + if (strncmp(ubuf, "on", len) == 0) + connector->force = DRM_FORCE_ON; + else if (strncmp(ubuf, "digital", len) == 0) + connector->force = DRM_FORCE_ON_DIGITAL; + else if (strncmp(ubuf, "off", len) == 0) + connector->force = DRM_FORCE_OFF; + else if (strncmp(ubuf, "unspecified", len) == 0) + connector->force = DRM_FORCE_UNSPECIFIED; + else + return -EINVAL; + + return len; +} + +static const struct file_operations drm_connector_fops = { + .owner = THIS_MODULE, + .open = connector_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, + .write = connector_write +}; + +int drm_debugfs_connector_add(struct drm_connector *connector) +{ + struct drm_minor *minor = connector->dev->primary; + struct dentry *root, *ent; + + if (!minor->debugfs_root) + return -1; + + root = debugfs_create_dir(drm_get_connector_name(connector), + minor->debugfs_root); + if (!root) + return -ENOMEM; + + connector->debugfs_entry = root; + + /* force */ + ent = debugfs_create_file("force", S_IRUGO, root, connector, + &drm_connector_fops); + if (!ent) + return -ENOMEM; + + return 0; +} + +void drm_debugfs_connector_remove(struct drm_connector *connector) +{ + if (!connector->debugfs_entry) + return; + + debugfs_remove_recursive(connector->debugfs_entry); + + connector->debugfs_entry = NULL; +} + #endif /* CONFIG_DEBUG_FS */ diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 76ccaab..c6c85f7 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1425,6 +1425,8 @@ extern int drm_debugfs_create
[Intel-gfx] [PATCH 3/3] drm/debugfs: add an "edid_override" file per connector
Add a file to debugfs for each connector that allows the edid data to be overridden. Signed-off-by: Thomas Wood --- drivers/gpu/drm/drm_crtc.c | 4 +++ drivers/gpu/drm/drm_debugfs.c | 56 ++ drivers/gpu/drm/drm_probe_helper.c | 9 +- include/drm/drm_crtc.h | 1 + 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 59a2784..8543eac 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3701,6 +3701,10 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, struct drm_device *dev = connector->dev; int ret, size; + /* ignore requests to set edid when overridden */ + if (connector->override_edid) + return 0; + if (connector->edid_blob_ptr) drm_property_destroy_blob(dev, connector->edid_blob_ptr); diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index b57b614..2c666ba 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -35,6 +35,7 @@ #include #include #include +#include #if defined(CONFIG_DEBUG_FS) @@ -295,6 +296,55 @@ static ssize_t connector_write(struct file *file, const char __user *ubuf, return len; } +static int edid_show(struct seq_file *m, void *data) +{ + struct drm_connector *connector = m->private; + struct drm_property_blob *edid = connector->edid_blob_ptr; + + if (connector->override_edid && edid) + seq_write(m, edid->data, edid->length); + + return 0; +} + +static int edid_open(struct inode *inode, struct file *file) +{ + struct drm_connector *dev = inode->i_private; + + return single_open(file, edid_show, dev); +} + +static ssize_t edid_write(struct file *file, const char __user *ubuf, + size_t len, loff_t *offp) +{ + struct seq_file *m = file->private_data; + struct drm_connector *connector = m->private; + + if (len >= EDID_LENGTH) { + drm_mode_connector_update_edid_property(connector, + (struct edid *) ubuf); + connector->override_edid = true; + } else { + if (connector->override_edid) { + connector->override_edid = false; + drm_mode_connector_update_edid_property(connector, + NULL); + } + } + + return len; +} + +static const struct file_operations drm_edid_fops = { + .owner = THIS_MODULE, + .open = edid_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, + .write = edid_write +}; + + static const struct file_operations drm_connector_fops = { .owner = THIS_MODULE, .open = connector_open, @@ -325,6 +375,12 @@ int drm_debugfs_connector_add(struct drm_connector *connector) if (!ent) return -ENOMEM; + /* edid */ + ent = debugfs_create_file("edid_override", S_IRUGO, root, connector, + &drm_edid_fops); + if (!ent) + return -ENOMEM; + return 0; } diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 79f07f2..1f0dc77 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -130,7 +130,14 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect count = drm_load_edid_firmware(connector); if (count == 0) #endif - count = (*connector_funcs->get_modes)(connector); + { + if (connector->override_edid) { + struct edid *edid = (struct edid *) connector->edid_blob_ptr->data; + + count = drm_add_edid_modes(connector, edid); + } else + count = (*connector_funcs->get_modes)(connector); + } if (count == 0 && connector->status == connector_status_connected) count = drm_add_modes_noedid(connector, 1024, 768); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index f04f4b9..11fce43 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -513,6 +513,7 @@ struct drm_connector { /* forced on connector */ enum drm_connector_force force; + bool override_edid; uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER]; struct drm_encoder *encoder; /* currently active encoder */ -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: enable PPGTT
On Thu, 29 May 2014 08:30:10 -0700 "Volkin, Bradley D" wrote: > On Wed, May 28, 2014 at 03:02:24PM -0700, Jesse Barnes wrote: > > Need testing and possibly disabling on earlier steppings, but looks ok > > here on my B3. > > > > Signed-off-by: Jesse Barnes > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index ec5f6fb..4b0e58c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1971,7 +1971,7 @@ struct drm_i915_cmd_table { > > > > #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) > > #define HAS_ALIASING_PPGTT(dev)(INTEL_INFO(dev)->gen >= 6 && > > !IS_VALLEYVIEW(dev)) > > -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && > > !IS_VALLEYVIEW(dev) \ > > +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ > > && !IS_BROADWELL(dev)) > > Which branch is this against? In -nightly, these are implemented > differently, so I don't know if all comments will apply. But... > > Since IS_VALLEYVIEW() is also true for chv, should we make the condition > !IS_CHERRYVIEW() in this patch instead? Or do we intend to have PPGTT > enabled for chv? > > I think we need the corresponding change to HAS_ALIASING_PPGTT() in order > to actually enable PPGTT. Otherwise it looks like sanitize_enable_ppgtt() > will disable PPGTT. I knew this was too easy, maybe that's why it worked so well for Ville and I... unless I was somehow testing full PPGTT. I'll check it out. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: enable PPGTT
On Wed, May 28, 2014 at 03:02:24PM -0700, Jesse Barnes wrote: > Need testing and possibly disabling on earlier steppings, but looks ok > here on my B3. > > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index ec5f6fb..4b0e58c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1971,7 +1971,7 @@ struct drm_i915_cmd_table { > > #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) > #define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && > !IS_VALLEYVIEW(dev)) > -#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && > !IS_VALLEYVIEW(dev) \ > +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 \ >&& !IS_BROADWELL(dev)) Which branch is this against? In -nightly, these are implemented differently, so I don't know if all comments will apply. But... Since IS_VALLEYVIEW() is also true for chv, should we make the condition !IS_CHERRYVIEW() in this patch instead? Or do we intend to have PPGTT enabled for chv? I think we need the corresponding change to HAS_ALIASING_PPGTT() in order to actually enable PPGTT. Otherwise it looks like sanitize_enable_ppgtt() will disable PPGTT. Brad > #define USES_PPGTT(dev) intel_enable_ppgtt(dev, false) > #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true) > -- > 1.8.4.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/5] kms_plane: Update for universal plane changes
Recent changes in igt_kms to support universal planes have removed the plane list order guarantees that kms_plane depended upon. Ensure that we loop over the entire plane list properly and then selectively skip non-overlay planes. While we're at it, update a couple igt_output_get_plane() calls to use plane enums rather than integer values to make it clear what we're actually doing. Signed-off-by: Matt Roper --- tests/kms_plane.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/kms_plane.c b/tests/kms_plane.c index 5db0947..1d334ab 100644 --- a/tests/kms_plane.c +++ b/tests/kms_plane.c @@ -94,7 +94,7 @@ test_position_init(test_position_t *test, igt_output_t *output, enum pipe pipe) test->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO); igt_output_set_pipe(output, pipe); - primary = igt_output_get_plane(output, 0); + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); mode = igt_output_get_mode(output); igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay, @@ -146,9 +146,14 @@ test_plane_position_with_output(data_t *data, test_position_init(&test, output, pipe); mode = igt_output_get_mode(output); - primary = igt_output_get_plane(output, 0); + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY); sprite = igt_output_get_plane(output, plane); + if (sprite->is_primary) { + test_position_fini(&test, output); + igt_skip("Skipping primary plane\n"); + } + create_fb_for_mode__position(data, mode, 100, 100, 64, 64, &primary_fb); igt_plane_set_fb(primary, &primary_fb); @@ -213,7 +218,7 @@ run_tests_for_pipe(data_t *data, enum pipe pipe) { int plane; - for (plane = 1; plane < IGT_MAX_PLANES; plane++) + for (plane = 0; plane < IGT_MAX_PLANES; plane++) run_tests_for_pipe_plane(data, pipe, plane); } -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/5] kms: Add igt_drm_plane_try_commit()
For some of our tests, we want to make sure that bogus plane programming attempts fail with the expected error code. Add igt_drm_plane_try_commit() that will just return the error code on failure rather than failing the current subtest. This lets us test the return value against the expected error code. Signed-off-by: Matt Roper --- lib/igt_kms.c | 21 ++--- lib/igt_kms.h | 1 + 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 97e329b..ce6ea87 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -810,7 +810,11 @@ static int igt_cursor_commit(igt_plane_t *plane, igt_output_t *output) return 0; } -static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output) +/* + * Attempt to commit a plane; if the DRM call to program the plane fails, + * just return an error code (but don't fail the current subtest). + */ +int igt_drm_plane_try_commit(igt_plane_t *plane, igt_output_t *output) { igt_display_t *display = output->display; igt_pipe_t *pipe; @@ -842,7 +846,8 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output) IGT_FIXED(0,0), /* src_w */ IGT_FIXED(0,0) /* src_h */); - igt_assert(ret == 0); + if (ret) + return ret; plane->fb_changed = false; } else if (plane->fb_changed || plane->position_changed) { @@ -866,7 +871,8 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output) IGT_FIXED(plane->fb->width,0), /* src_w */ IGT_FIXED(plane->fb->height,0) /* src_h */); - igt_assert(ret == 0); + if (ret) + return ret; plane->fb_changed = false; plane->position_changed = false; @@ -876,6 +882,15 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output) return 0; } +static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output) +{ + int ret = igt_drm_plane_try_commit(plane, output); + + igt_assert(ret == 0); + + return 0; +} + static int igt_plane_commit(igt_plane_t *plane, igt_output_t *output) { struct igt_display *display = plane->pipe->display; diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 4955fc8..2f72a1c 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -157,6 +157,7 @@ void igt_display_use_universal_commits(igt_display_t *display, bool use_universal); int igt_display_commit(igt_display_t *display); int igt_display_get_n_pipes(igt_display_t *display); +int igt_drm_plane_try_commit(igt_plane_t *plane, igt_output_t *output); const char *igt_output_name(igt_output_t *output); drmModeModeInfo *igt_output_get_mode(igt_output_t *output); -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/5] kms: Add universal plane support
Add support for universal planes. This involves revamping the existing plane handling a bit to allow primary & cursor planes to come from the DRM plane list, rather than always being manually added. Also, eliminate the hard-coded position of primary & cursor in the plane list since the DRM could return them in any order. To minimize impact for existing tests, we add a new igt_display_use_universal_commits() call to toggle between universal and legacy API's for programming the primary and cursor planes. Signed-off-by: Matt Roper --- lib/igt_kms.c | 132 +- lib/igt_kms.h | 5 +++ 2 files changed, 107 insertions(+), 30 deletions(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index d00250d..97e329b 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -500,27 +500,24 @@ void igt_display_init(igt_display_t *display, int drm_fd) */ display->n_pipes = resources->count_crtcs; + drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1); plane_resources = drmModeGetPlaneResources(display->drm_fd); igt_assert(plane_resources); for (i = 0; i < display->n_pipes; i++) { igt_pipe_t *pipe = &display->pipes[i]; - igt_plane_t *plane; - int p, j; + igt_plane_t *plane = NULL; + int p = 0, j; pipe->display = display; pipe->pipe = i; - /* primary plane */ - p = IGT_PLANE_PRIMARY; - plane = &pipe->planes[p]; - plane->pipe = pipe; - plane->index = p; - plane->is_primary = true; - /* add the planes that can be used with that pipe */ for (j = 0; j < plane_resources->count_planes; j++) { drmModePlane *drm_plane; + drmModeObjectPropertiesPtr proplist; + drmModePropertyPtr prop; + int k; drm_plane = drmModeGetPlane(display->drm_fd, plane_resources->planes[j]); @@ -531,21 +528,67 @@ void igt_display_init(igt_display_t *display, int drm_fd) continue; } - p++; plane = &pipe->planes[p]; plane->pipe = pipe; - plane->index = p; + plane->index = p++; plane->drm_plane = drm_plane; + + prop = NULL; + proplist = drmModeObjectGetProperties(display->drm_fd, + plane_resources->planes[j], + DRM_MODE_OBJECT_PLANE); + for (k = 0; k < proplist->count_props; k++) { + prop = drmModeGetProperty(display->drm_fd, proplist->props[k]); + if (!prop) + continue; + + if (strcmp(prop->name, "type") != 0) { + drmModeFreeProperty(prop); + continue; + } + + switch (proplist->prop_values[k]) { + case DRM_PLANE_TYPE_PRIMARY: + plane->is_primary = 1; + display->has_universal_planes = 1; + break; + case DRM_PLANE_TYPE_CURSOR: + plane->is_cursor = 1; + pipe->has_cursor = 1; + display->has_universal_planes = 1; + break; + } + + drmModeFreeProperty(prop); + break; + } + } - /* cursor plane */ - p++; - plane = &pipe->planes[p]; - plane->pipe = pipe; - plane->index = p; - plane->is_cursor = true; + /* +* Add a drm_plane-less primary plane for kernels without +* universal plane support. +*/ + if (!display->has_universal_planes) { + plane = &pipe->planes[p]; + plane->pipe = pipe; + plane->index = p++; + plane->is_primary = true; + } - pipe->n_planes = ++p; + /* +* Add drm_plane-less cursor plane for kernels that don't +* expose a universal cursor plane. +*/ +
[Intel-gfx] [PATCH i-g-t 4/5] kms_universal_plane: Universal plane testing (v5)
Add a simple test to exercise universal plane support. v5: - Check that we don't have more than one primary or cursor. This will catch accidental calls to drm_plane_init() in the kernel where drm_universal_plane_init() was intended (these don't cause a compile warning due to type compatibility between enum and bool). v4: - Test disabling the primary plane explicitly when it has previously been implicitly disabled due to clipping. - Skip test if igt_pipe_crc_new() fails v3: - For testing while crtc is off, switch between several different primary plane fb's before reenabling the crtc. This will help catch pin/unpin mistakes. v2: - Test that pageflips error out gracefully when the primary plane is disabled before the ioctl, or between ioctl and pageflip execution. - Test that nothing blows up if we try to disable the primary plane immediately after a pageflip (presumably before the pageflip actually completes). - Test that we can setup primary + sprite planes with the CRTC off and then have them show up properly when we enable the CRTC (drmModeSetCrtc with fb = -1). - Test that we can modeset properly after having disabled the primary plane - Test that proper error codes are returned for invalid plane programming attempts. Signed-off-by: Matt Roper --- tests/.gitignore| 1 + tests/Makefile.sources | 1 + tests/kms_universal_plane.c | 598 3 files changed, 600 insertions(+) create mode 100644 tests/kms_universal_plane.c diff --git a/tests/.gitignore b/tests/.gitignore index d7ad054..d1b01f7 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -124,6 +124,7 @@ kms_pipe_crc_basic kms_plane kms_render kms_setmode +kms_universal_plane multi-tests.txt pm_lpsp pm_rpm diff --git a/tests/Makefile.sources b/tests/Makefile.sources index eca4af9..0132f8e 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -70,6 +70,7 @@ TESTS_progs_M = \ kms_plane \ kms_render \ kms_setmode \ + kms_universal_plane \ pm_lpsp \ pm_rpm \ pm_rps \ diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c new file mode 100644 index 000..df48b50 --- /dev/null +++ b/tests/kms_universal_plane.c @@ -0,0 +1,598 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include +#include +#include +#include + +#include "drmtest.h" +#include "igt_debugfs.h" +#include "igt_kms.h" + +typedef struct { + int drm_fd; + igt_display_t display; +} data_t; + +typedef struct { + data_t *data; + igt_pipe_crc_t *pipe_crc; + igt_crc_t crc_1, crc_2, crc_3, crc_4, crc_5, crc_6, crc_7, crc_8, + crc_9, crc_10; + struct igt_fb red_fb, blue_fb, black_fb, yellow_fb; + drmModeModeInfo *mode; +} functional_test_t; + +typedef struct { + data_t *data; + drmModeResPtr moderes; + struct igt_fb blue_fb, oversized_fb, undersized_fb; +} sanity_test_t; + +typedef struct { + data_t *data; + struct igt_fb red_fb, blue_fb; +} pageflip_test_t; + +static void +functional_test_init(functional_test_t *test, igt_output_t *output, enum pipe pipe) +{ + data_t *data = test->data; + drmModeModeInfo *mode; + + test->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO); + if (!test->pipe_crc) + igt_skip("auto crc not supported on this connector with pipe %i\n", + pipe); + + + igt_output_set_pipe(output, pipe); + + mode = igt_output_get_mode(output); + igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay, + DRM_FORMAT_XRGB, + false, /* tiled */ + 0.0, 0.0, 0.0, +
[Intel-gfx] [PATCH i-g-t 5/5] kms_cursor_crc: Combine data_t and test_data_t
If a subtest fails, cleanup_crtc() never gets called and then the test_data_t structure for the test is lost, including the CRC file descriptor that we never got a chance to release; this causes all subsequent tests to fail with -EBUSY at igt_pipe_crc_new(). The split between permanent data_t and temporary test_data_t doesn't seem to serve a purpose, so just combine the fields from both into data_t. This will prevent us from losing the CRC filedescriptor so that we can properly close and reopen it after a failed test. Signed-off-by: Matt Roper --- tests/kms_cursor_crc.c | 206 +++-- 1 file changed, 97 insertions(+), 109 deletions(-) diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c index 06625ee..92d9a3c 100644 --- a/tests/kms_cursor_crc.c +++ b/tests/kms_cursor_crc.c @@ -44,10 +44,6 @@ typedef struct { igt_display_t display; struct igt_fb primary_fb; struct igt_fb fb; -} data_t; - -typedef struct { - data_t *data; igt_output_t *output; enum pipe pipe; igt_crc_t ref_crc; @@ -55,7 +51,7 @@ typedef struct { int screenw, screenh; int curw, curh; /* cursor size */ igt_pipe_crc_t *pipe_crc; -} test_data_t; +} data_t; static void draw_cursor(cairo_t *cr, int x, int y, int w) { @@ -71,11 +67,10 @@ static void draw_cursor(cairo_t *cr, int x, int y, int w) igt_paint_color_alpha(cr, x + w, y + w, w, w, 0.5, 0.5, 0.5, 1.0); } -static void cursor_enable(test_data_t *test_data) +static void cursor_enable(data_t *data) { - data_t *data = test_data->data; igt_display_t *display = &data->display; - igt_output_t *output = test_data->output; + igt_output_t *output = data->output; igt_plane_t *cursor; cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR); @@ -83,11 +78,10 @@ static void cursor_enable(test_data_t *test_data) igt_display_commit(display); } -static void cursor_disable(test_data_t *test_data) +static void cursor_disable(data_t *data) { - data_t *data = test_data->data; igt_display_t *display = &data->display; - igt_output_t *output = test_data->output; + igt_output_t *output = data->output; igt_plane_t *cursor; cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR); @@ -96,11 +90,10 @@ static void cursor_disable(test_data_t *test_data) } -static void do_single_test(test_data_t *test_data, int x, int y) +static void do_single_test(data_t *data, int x, int y) { - data_t *data = test_data->data; igt_display_t *display = &data->display; - igt_pipe_crc_t *pipe_crc = test_data->pipe_crc; + igt_pipe_crc_t *pipe_crc = data->pipe_crc; igt_crc_t crc, ref_crc; igt_plane_t *cursor; cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb); @@ -108,93 +101,93 @@ static void do_single_test(test_data_t *test_data, int x, int y) igt_info("."); fflush(stdout); /* Hardware test */ - igt_paint_test_pattern(cr, test_data->screenw, test_data->screenh); - cursor_enable(test_data); - cursor = igt_output_get_plane(test_data->output, IGT_PLANE_CURSOR); + igt_paint_test_pattern(cr, data->screenw, data->screenh); + cursor_enable(data); + cursor = igt_output_get_plane(data->output, IGT_PLANE_CURSOR); igt_plane_set_position(cursor, x, y); igt_display_commit(display); - igt_wait_for_vblank(data->drm_fd, test_data->pipe); + igt_wait_for_vblank(data->drm_fd, data->pipe); igt_pipe_crc_collect_crc(pipe_crc, &crc); - cursor_disable(test_data); + cursor_disable(data); /* Now render the same in software and collect crc */ - draw_cursor(cr, x, y, test_data->curw); + draw_cursor(cr, x, y, data->curw); igt_display_commit(display); - igt_wait_for_vblank(data->drm_fd, test_data->pipe); + igt_wait_for_vblank(data->drm_fd, data->pipe); igt_pipe_crc_collect_crc(pipe_crc, &ref_crc); /* Clear screen afterwards */ - igt_paint_color(cr, 0, 0, test_data->screenw, test_data->screenh, - 0.0, 0.0, 0.0); + igt_paint_color(cr, 0, 0, data->screenw, data->screenh, + 0.0, 0.0, 0.0); igt_assert(igt_crc_equal(&crc, &ref_crc)); } -static void do_test(test_data_t *test_data, +static void do_test(data_t *data, int left, int right, int top, int bottom) { - do_single_test(test_data, left, top); - do_single_test(test_data, right, top); - do_single_test(test_data, right, bottom); - do_single_test(test_data, left, bottom); + do_single_test(data, left, top); + do_single_test(data, right, top); + do_single_test(data, right, bottom); + do_single_test(data, left, bottom); } -static void test_crc_onscreen(test_data_t *test_data) +static void test_crc_onscreen(data_t *d
[Intel-gfx] [PATCH 1/4] drm: Check CRTC compatibility in setplane
The DRM core setplane code should check that the plane is usable on the specified CRTC before calling into the driver. Prior to this patch, a plane's possible_crtcs field was purely informational for userspace and was never actually verified at the kernel level (aside from the primary plane helper). Cc: dri-de...@lists.freedesktop.org Reviewed-by: Chon Ming Lee Signed-off-by: Matt Roper --- drivers/gpu/drm/drm_crtc.c | 7 +++ drivers/gpu/drm/drm_plane_helper.c | 6 -- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 37a3e07..1cfd1e3 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2153,6 +2153,13 @@ int drm_mode_setplane(struct drm_device *dev, void *data, } crtc = obj_to_crtc(obj); + /* Check whether this plane is usable on this CRTC */ + if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) { + DRM_DEBUG_KMS("Invalid crtc for plane\n"); + ret = -EINVAL; + goto out; + } + fb = drm_framebuffer_lookup(dev, plane_req->fb_id); if (!fb) { DRM_DEBUG_KMS("Unknown framebuffer ID %d\n", diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index d966afa..b601233 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -137,12 +137,6 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, return -EINVAL; } - /* Primary planes are locked to their owning CRTC */ - if (plane->possible_crtcs != drm_crtc_mask(crtc)) { - DRM_DEBUG_KMS("Cannot change primary plane CRTC\n"); - return -EINVAL; - } - /* Disallow scaling */ src_w >>= 16; src_h >>= 16; -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Intel-specific primary plane handling (v8)
Intel hardware allows the primary plane to be disabled independently of the CRTC. Provide custom primary plane handling to allow this. v8: - Pin/unpin properly when clipping causes the primary plane to be disabled when it has previously been enabled. - s/drm_primary_helper_check_update/drm_plane_helper_check_update/ v7: - Clip primary plane to invisible when crtc is disabled since intel_crtc->config.pipe_src_{w,h} may be garbage otherwise. - Unpin old fb before pinning new one in the "just pin and return" case that is used when the crtc is disabled. - Don't treat implicit disabling of the primary plane (caused by clipping) the same way as explicit disabling (caused by fb=0). For implicit disables, we should leave the fb set and pinned, whereas for explicit disables we need to unpin the fb before primary->fb is cleared. v6: - Pass rectangles to primary helper check function and get plane visibility back. - Wait for pending pageflips on primary plane update/disable. - Allow primary plane to be updated while the crtc is disabled (changes will take effect when the crtc is re-enabled if modeset passes -1 for the fb id). - Drop WARN() if we try to disable the primary plane when it's already been disabled. This will happen if the crtc gets disabled after the primary plane has already been disabled independently. v5: - Use new drm_primary_helper_check_update() helper function to check setplane parameter validity. - Swap primary plane's pipe for pre-gen4 FBC (caught by Ville Syrjälä) - Cleanup primary plane properly on crtc init failure v4: - Don't add a primary_plane field to intel_crtc; that was left over from a much earlier iteration of this patch series, but is no longer needed/used now that the DRM core primary plane support has been merged. v3: - Provide gen-specific primary plane format lists (suggested by Daniel Vetter). - If the primary plane is already enabled, go ahead and just call the primary plane helper to do the update (suggested by Daniel Vetter). - Don't try to disable the primary plane on destruction; the DRM layer should have already taken care of this for us. v2: - Unpin fb properly on primary plane disable - Provide an Intel-specific set of primary plane formats - Additional sanity checks on setplane (in line with the checks currently being done by the DRM core primary plane helper) Reviewed-by: Chon Ming Lee Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_display.c | 232 ++- 1 file changed, 229 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c2aedac..0ddffac 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -39,10 +39,37 @@ #include "i915_trace.h" #include #include +#include +#include #include +/* Primary plane formats supported by all gen */ +#define COMMON_PRIMARY_FORMATS \ + DRM_FORMAT_C8, \ + DRM_FORMAT_RGB565, \ + DRM_FORMAT_XRGB, \ + DRM_FORMAT_ARGB + +/* Primary plane formats for gen <= 3 */ +static const uint32_t intel_primary_formats_gen2[] = { + COMMON_PRIMARY_FORMATS, + DRM_FORMAT_XRGB1555, + DRM_FORMAT_ARGB1555, +}; + +/* Primary plane formats for gen >= 4 */ +static const uint32_t intel_primary_formats_gen4[] = { + COMMON_PRIMARY_FORMATS, \ + DRM_FORMAT_XBGR, + DRM_FORMAT_ABGR, + DRM_FORMAT_XRGB2101010, + DRM_FORMAT_ARGB2101010, + DRM_FORMAT_XBGR2101010, + DRM_FORMAT_ABGR2101010, +}; + #define DIV_ROUND_CLOSEST_ULL(ll, d) \ - ({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; }) +({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; }) static void intel_increase_pllclock(struct drm_crtc *crtc); static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on); @@ -10959,17 +10986,216 @@ static void intel_shared_dpll_init(struct drm_device *dev) BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS); } +static int +intel_primary_plane_disable(struct drm_plane *plane) +{ + 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_crtc *intel_crtc; + + if (!plane->fb) + return 0; + + BUG_ON(!plane->crtc); + + intel_crtc = to_intel_crtc(plane->crtc); + + /* +* Even though we checked plane->fb above, it's still possible that +* the primary plane has been implicitly disabled because the crtc +* coordinates given weren't visible, or because we detected +* that it was 100% covered by a sprite plane. Or, the CRTC may be +* off and we've set a fb, but haven't actually turned on the CRTC yet. +* In either case, we need to unpin the FB and let the fb pointer get +*
[Intel-gfx] [PATCH 3/4] drm/i915: don't force full modeset if primary plane is disabled (v2)
In a future patch, we'll allow the primary plane to be disabled by userspace via the universal plane API. If a modeset is requested while the primary plane is disabled, crtc->primary->fb will be NULL which generally triggers a full modeset (except in fastboot situations). If we detect that the crtc is active, but there's no primary plane fb, we should still allow a simple plane update rather than a full modeset if the mode isn't actually changing (after re-enabling the primary plane of course). v2: - Enable plane after set_base to avoid enabling the plane if set_base fails, and to make flip+enable atomic (suggested by Ville) - Drop BUG to WARN if we somehow enter the 'fb_changed' modeset case with the crtc disabled (suggested by Ville) Reviewed-by: Chon Ming Lee Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_display.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 731cd01..c2aedac 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10544,12 +10544,17 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, if (is_crtc_connector_off(set)) { config->mode_changed = true; } else if (set->crtc->primary->fb != set->fb) { - /* If we have no fb then treat it as a full mode set */ + /* +* If we have no fb, we can only flip as long as the crtc is +* active, otherwise we need a full mode set. The crtc may +* be active if we've only disabled the primary plane, or +* in fastboot situations. +*/ if (set->crtc->primary->fb == NULL) { struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc); - if (intel_crtc->active && i915.fastboot) { + if (intel_crtc->active) { DRM_DEBUG_KMS("crtc has no fb, will flip\n"); config->fb_changed = true; } else { @@ -10787,10 +10792,24 @@ static int intel_crtc_set_config(struct drm_mode_set *set) ret = intel_set_mode(set->crtc, set->mode, set->x, set->y, set->fb); } else if (config->fb_changed) { + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc); + intel_crtc_wait_for_pending_flips(set->crtc); ret = intel_pipe_set_base(set->crtc, set->x, set->y, set->fb); + + /* +* We need to make sure the primary plane is re-enabled if it +* has previously been turned off. +*/ + if (!intel_crtc->primary_enabled && ret == 0) { + WARN_ON(!intel_crtc->active); + intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane, + intel_crtc->pipe); + } + /* * In the fastboot case this may be our only check of the * state after boot. It would be better to only do it on -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/plane-helper: Add drm_plane_helper_check_update() (v3)
Pull the parameter checking from drm_primary_helper_update() out into its own function; drivers that provide their own setplane() implementations rather than using the helper may still want to share this parameter checking logic. A few of the checks here were also updated based on suggestions by Ville Syrjälä. v3: - s/primary_helper/plane_helper/ --- this checking logic may be useful for other types of planes as well. - Fix visibility check (need to dereference visibility pointer) v2: - Pass src/dest/clip rects and min/max scaling down to helper to avoid duplication of effort between helper and drivers (suggested by Ville). - Allow caller to specify whether the primary plane should be updatable while the crtc is disabled. Cc: dri-de...@lists.freedesktop.org Reviewed-by: Chon Ming Lee Signed-off-by: Matt Roper --- drivers/gpu/drm/drm_plane_helper.c | 124 - include/drm/drm_plane_helper.h | 22 +++ 2 files changed, 116 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index b601233..fd401fc 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -66,6 +66,79 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc, } /** + * drm_plane_helper_check_update() - Check plane update for validity + * @plane: plane object to update + * @crtc: owning CRTC of owning plane + * @fb: framebuffer to flip onto plane + * @src: source coordinates in 16.16 fixed point + * @dest: integer destination coordinates + * @clip: integer clipping coordinates + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point + * @can_position: is it legal to position the plane such that it + *doesn't cover the entire crtc? This will generally + *only be false for primary planes. + * @can_update_disabled: can the plane be updated while the crtc + * is disabled? + * @visible: output parameter indicating whether plane is still visible after + * clipping + * + * Checks that a desired plane update is valid. Drivers that provide + * their own plane handling rather than helper-provided implementations may + * still wish to call this function to avoid duplication of error checking + * code. + * + * RETURNS: + * Zero if update appears valid, error code on failure + */ +int drm_plane_helper_check_update(struct drm_plane *plane, + struct drm_crtc *crtc, + struct drm_framebuffer *fb, + struct drm_rect *src, + struct drm_rect *dest, + const struct drm_rect *clip, + int min_scale, + int max_scale, + bool can_position, + bool can_update_disabled, + bool *visible) +{ + int hscale, vscale; + + if (!crtc->enabled && !can_update_disabled) { + DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n"); + return -EINVAL; + } + + /* Check scaling */ + hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale); + vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale); + if (hscale < 0 || vscale < 0) { + DRM_DEBUG_KMS("Invalid scaling of plane\n"); + return -ERANGE; + } + + *visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale); + if (!*visible) + /* +* Plane isn't visible; some drivers can handle this +* so we just return success here. Drivers that can't +* (including those that use the primary plane helper's +* update function) will return an error from their +* update_plane handler. +*/ + return 0; + + if (!can_position && !drm_rect_equals(dest, clip)) { + DRM_DEBUG_KMS("Plane must cover entire CRTC\n"); + return -EINVAL; + } + + return 0; +} +EXPORT_SYMBOL(drm_plane_helper_check_update); + +/** * drm_primary_helper_update() - Helper for primary plane update * @plane: plane object to update * @crtc: owning CRTC of owning plane @@ -113,51 +186,42 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, .x = src_x >> 16, .y = src_y >> 16, }; + struct drm_rect src = { + .x1 = src_x, + .y1 = src_y, + .x2 = src_x + src_w, + .y2 = src_y + src_h, + }; struct drm_rect dest = { .x1 = crtc_x, .y1 = crtc_y, .x2 = crtc_x + crtc
[Intel-gfx] [PATCH 0/4] Intel primary plane support (v7)
Re-posting the final versions of these patches now that they all have a r-b, as requested by Daniel. Corresponding i-g-t tests will be sent shortly. Previous patch review thread and comments are here: http://lists.freedesktop.org/archives/intel-gfx/2014-April/044527.html Matt Roper (4): drm: Check CRTC compatibility in setplane drm/plane-helper: Add drm_plane_helper_check_update() (v3) drm/i915: don't force full modeset if primary plane is disabled (v2) drm/i915: Intel-specific primary plane handling (v8) drivers/gpu/drm/drm_crtc.c | 7 + drivers/gpu/drm/drm_plane_helper.c | 130 +- drivers/gpu/drm/i915/intel_display.c | 255 ++- include/drm/drm_plane_helper.h | 22 +++ 4 files changed, 373 insertions(+), 41 deletions(-) -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] 830GM still woes
Hi Daniel, hi folks, still a couple of observations from my side on this. The 1024x786x24 mode here uses a clock of 65MHz (65000kHz), if that is inserted into the watermark computation, it computes from that a prefetch of 40 entries, and thus a watermark level of four, which is much much too high. To get a stable display, the watermark level can be at most eight. However, if I program the FW_BLC register by hand, I can set the watermark down to 32, i.e. 15(!) entries, and I still get a wonderful stable display, scrolling and everything included. If I compare that with what is required by intel_calculate_wm() to get the same value, I find that the input is, in some place, off by about a factor of two. Which could mean: a) the latency is too high by a factor of two. Even with a latency of 2500ns, I do get a good watermark level and a stable display. The limit is around 1500ns. b) the I830_FIFO_LINE_SIZE is off by a factor of two. Is it really 32 bytes? Is it *measured* in bytes? With a line size twice as large, the result would fit again. c) Is the I830 FIFO_SIZE really 47? This is the value I get when I debug i9xx_update_wm(). It seems the code splits the totally available fifo (unified fifo of the 830GM) of 95 entries approximately in half for pipe A and pipe B. Is the *unit* correct? Is the fifo size measured in *entries*? What makes me wonder is that there is really approximately a factor of two between the *real* limit and the value computed by the code, which looks to me that at some point a division or multiplication by two is missing. Finally, a regression with the 3.15.0 code: I already had the phenomenon that the boot console is vertically shifted, which is caused by the pipe-a quirk (without that quirk, the display is correct), but it now also happens from time to time that the DVO is again not clocked correctly. The screen then goes dead in the boot console, but as soon as X starts up, I get again a display. This display is sometimes a bit broken (flickers, as if the frequency is about 40Hz, not 60Hz) or no display at all. Switching to the boot console and back to X resolves the issue. As said, disable the pipe_A quirk and we are good. A second observation is that the boot console now reports PIPE_A (and sometimes also) PIPE_B underruns during the bootstrap, only once. The system recovers from this (if you call a dead boot console as above "recovery"), so something is likely broken with the quirk. (No news, of course). Greetings, Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Support pf CRC source on haswell transcoder edp
The always-on power well pixel path on haswell is routed such that it bypasses the panel fitter when we use is. Which means the pfit CRC source won't work in that configuration. Add a new disallow-bypass flags to the pfit pipe config state and set it when we want to use the pf CRC. Results in a bit of flicker, but should get the job done. We'll also undo do it afterwards to make sure other tests arent' negatively affected. Totally untested due to lack of hsw laptops around here. v2: s/disallow_bypass/force_power_well_on/ to avoid a double negative (Damien). v3: force_thru because roadsigns. v4: Don't forget the power wells! Also note that until the runtime pm for DPMS series is fully merged the simple disable/enable trick won't work since the ->crtc_mode_set callback is still required to do nasty things. This stuff is tricky, but I think by both fixing up get_crtc_power_domains and the debugfs wa code we should always grab/drop the additional power well correctly. v5: Wrap in () as suggested by Damien to avoid setting reserved values for the edp transcoder path on bdw+ References: https://bugs.freedesktop.org/show_bug.cgi?id=72864 Cc: Damien Lespiau Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_debugfs.c | 64 ++-- drivers/gpu/drm/i915/intel_ddi.c | 4 ++- drivers/gpu/drm/i915/intel_display.c | 4 +-- drivers/gpu/drm/i915/intel_drv.h | 1 + 4 files changed, 67 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 2e5f76a585ef..e2d5d24e060a 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2847,7 +2847,60 @@ static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, return 0; } -static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, +static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *crtc = + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_A]); + + drm_modeset_lock_all(dev); + /* +* If we use the eDP transcoder we need to make sure that we don't +* bypass the pfit, since otherwise the pipe CRC source won't work. Only +* relevant on hsw with pipe A when using the always-on power well +* routing. +*/ + if (crtc->config.cpu_transcoder == TRANSCODER_EDP && + !crtc->config.pch_pfit.enabled) { + crtc->config.pch_pfit.force_thru = true; + + intel_display_power_get(dev_priv, + POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A)); + + dev_priv->display.crtc_disable(&crtc->base); + dev_priv->display.crtc_enable(&crtc->base); + } + drm_modeset_unlock_all(dev); +} + +static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *crtc = + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_A]); + + drm_modeset_lock_all(dev); + /* +* If we use the eDP transcoder we need to make sure that we don't +* bypass the pfit, since otherwise the pipe CRC source won't work. Only +* relevant on hsw with pipe A when using the always-on power well +* routing. +*/ + if (crtc->config.pch_pfit.force_thru) { + crtc->config.pch_pfit.force_thru = false; + + dev_priv->display.crtc_disable(&crtc->base); + dev_priv->display.crtc_enable(&crtc->base); + + intel_display_power_put(dev_priv, + POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A)); + } + drm_modeset_unlock_all(dev); +} + +static int ivb_pipe_crc_ctl_reg(struct drm_device *dev, + enum pipe pipe, + enum intel_pipe_crc_source *source, uint32_t *val) { if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) @@ -2861,6 +2914,9 @@ static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB; break; case INTEL_PIPE_CRC_SOURCE_PF: + if (IS_HASWELL(dev) && pipe == PIPE_A) + hsw_trans_edp_pipe_A_crc_wa(dev); + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB; break; case INTEL_PIPE_CRC_SOURCE_NONE: @@ -2893,11 +2949,11 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, else if (INTEL_INFO(dev)->gen < 5) ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val); else if (IS_VALLEYVIEW(dev)) - ret = vlv_pipe_crc_ctl_reg(dev,pipe, &source, &val); + ret = vlv_pipe_crc_ctl_reg(dev, pipe, &source, &val); else
Re: [Intel-gfx] [PATCH] drm/i915: Support pf CRC source on haswell transcoder edp
On Thu, May 29, 2014 at 12:41:44PM +0200, Daniel Vetter wrote: > On Thu, May 29, 2014 at 1:21 AM, Damien Lespiau > wrote: > > On Thu, May 29, 2014 at 12:27:55AM +0200, Daniel Vetter wrote: > >> - if (IS_HASWELL(dev) && > >> intel_crtc->config.pch_pfit.enabled) > >> + if (IS_HASWELL(dev) && > >> intel_crtc->config.pch_pfit.enabled || > >> + intel_crtc->config.pch_pfit.force_thru) > >> temp |= TRANS_DDI_EDP_INPUT_A_ONOFF; > > > > My gcc warns here, suggesting the addition of (). And indeed, it seems > > that we want: > > > > if (IS_HASWELL(dev) && (intel_crtc->config.pch_pfit.enabled || > > intel_crtc->config.pch_pfit.force_thru)) > > temp |= TRANS_DDI_EDP_INPUT_A_ONOFF; > > It doesn't actually matter since force_thru is set only on HSW. We can > set them whereever we want, and I think actually wrapping it like > (IS_HSW && pfit.enabled) || pfit.force_thru reads saner. Otoh only HSW > has this peculiarity. Either way is fine, but I like mine a bit better because when looking at these line in isolation (without looking where we set force_thru) it doesn't smell fishy. The pf changed on BDW and TRANS_DDI_EDP_INPUT_A_ONOFF is reserved, which is why I propose that split. (IS_HSW && pfit.enabled) || pfit.force_thru reads like force_thru could be set on BDW and we end up setting a reserved value. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Allow vblank interrupts during modeset and eliminate some vblank races
On Thu, May 29, 2014 at 6:11 AM, Michel Dänzer wrote: > >> We could rename the off/on to pre/post_modeset if you like, but then >> someone gets to audit all the other drivers. That someone isn't >> going to be me. > > I appreciate your caution wrt other drivers, but I'm worried that having > a second mechanism for keeping the DRM vblank counter consistent might > cause subtle problems for drivers using the existing mechanism anyway. I think that risk is rather low. Only i915 has been stupid enough to use both drm_vblank_off + pre/post_modeset in the normal crtc enable/disable functions. Everyone else uses either or the other, except for tegra which uses pre/post in the ->prepare/commit hooks and off in the crtc->disable callback. So should still get consistent results (since after ->disable vblank consistency stops mattering). The reason we do all this is that we want to kill the delayed vblank put for i915, which is possible if you have a hw vblank counter. There's apparently more stuff wrong here still, so while we wrestle around probably best to leave other drivers out. I guess once this is all done I have to roll it out across all drivers so that we have consistent behaviour again ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Support pf CRC source on haswell transcoder edp
On Thu, May 29, 2014 at 1:21 AM, Damien Lespiau wrote: > On Thu, May 29, 2014 at 12:27:55AM +0200, Daniel Vetter wrote: >> - if (IS_HASWELL(dev) && >> intel_crtc->config.pch_pfit.enabled) >> + if (IS_HASWELL(dev) && >> intel_crtc->config.pch_pfit.enabled || >> + intel_crtc->config.pch_pfit.force_thru) >> temp |= TRANS_DDI_EDP_INPUT_A_ONOFF; > > My gcc warns here, suggesting the addition of (). And indeed, it seems > that we want: > > if (IS_HASWELL(dev) && (intel_crtc->config.pch_pfit.enabled || > intel_crtc->config.pch_pfit.force_thru)) > temp |= TRANS_DDI_EDP_INPUT_A_ONOFF; It doesn't actually matter since force_thru is set only on HSW. We can set them whereever we want, and I think actually wrapping it like (IS_HSW && pfit.enabled) || pfit.force_thru reads saner. Otoh only HSW has this peculiarity. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Selection of MMIO vs CS flip at page flip time
From: Sourab Gupta This patch enables the selection of MMIO flip vs CS flip at page flip time. Earlier, this selection was done only at the init time, so, once .queue_flip was set, it was used forever. This patch enables this selection of flip mechanism at a time when page flips is being issued. Signed-off-by: Sourab Gupta --- drivers/gpu/drm/i915/intel_display.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c75a925..9dda965 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9427,7 +9427,13 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, work->gtt_offset = i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset; - ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, ring, page_flip_flags); + if (intel_use_mmio_flip(dev)) + ret = intel_queue_mmio_flip(dev, crtc, + fb, obj, ring, page_flip_flags); + else + ret = dev_priv->display.queue_flip(dev, crtc, + fb, obj, ring, page_flip_flags); + if (ret) goto cleanup_unpin; @@ -11652,9 +11658,6 @@ static void intel_init_display(struct drm_device *dev) break; } - if (intel_use_mmio_flip(dev)) - dev_priv->display.queue_flip = intel_queue_mmio_flip; - intel_panel_init_backlight_funcs(dev); } -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Make module param for MMIO flip selection as tristate
From: Sourab Gupta This patch enhances the module parameter, 'use_mmio_flip' which enables MMIO flips, to make it tristate. The values being- 0: Force CS flip 1: Force MMIO flip (Gen5+) >1: Driver discretion is applied while selecting CS vs MMIO flip. For Valleyview, this driver selection happens based on the idleness of Blitter and Video engines. The Blitter and Video engines are in the same power well. So, if both are idle, we can use MMIO flips, Otherwise, we can use the BCS flips. This usecase can be modified and/or enhanced to cover more platforms. Signed-off-by: Sourab Gupta --- drivers/gpu/drm/i915/i915_params.c | 4 +++- drivers/gpu/drm/i915/intel_display.c | 42 ++-- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index e0d44df..9becd1e 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -159,4 +159,6 @@ MODULE_PARM_DESC(enable_cmd_parser, "Enable command parsing (1=enabled [default], 0=disabled)"); module_param_named(use_mmio_flip, i915.use_mmio_flip, bool, 0600); -MODULE_PARM_DESC(use_mmio_flip, "use MMIO flips (default: false)"); +MODULE_PARM_DESC(use_mmio_flip, "use MMIO page flips" + "(0 force CS, 1:force mmio, >1: driver selection)" + "(default: 0)"); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9dda965..a3d38d2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9185,22 +9185,50 @@ static int intel_gen7_queue_flip(struct drm_device *dev, static bool intel_use_mmio_flip(struct drm_device *dev) { - /* If module parameter is disabled, use CS flips. -* Otherwise, use MMIO flips starting from Gen5. + struct drm_i915_private *dev_priv = dev->dev_private; + bool use_mmio_flip = false; + + /* If module parameter is 0, force CS flip. +* If module parameter is 1, force MMIO flip starting from Gen5. * This is not being used for older platforms, because * non-availability of flip done interrupt forces us to use * CS flips. Older platforms derive flip done using some clever * tricks involving the flip_pending status bits and vblank irqs. * So using MMIO flips there would disrupt this mechanism. +* If module parameter is > 1, driver discretion is applied for +* selection of CS vs MMIO flip. */ if (i915.use_mmio_flip == 0) - return false; + use_mmio_flip = false; - if (INTEL_INFO(dev)->gen >= 5) - return true; - else - return false; + if (i915.use_mmio_flip == 1) { + if (INTEL_INFO(dev)->gen >= 5) + use_mmio_flip = true; + else + use_mmio_flip = false; + } + + if (i915.use_mmio_flip > 1) { + /* For Valleyview, Blitter and Video engines are in the same +* power well. So, if both are idle, we can use MMIO flips, +* Otherwise, we can use the BCS flips. +* We use the parameter 'request_list' to determine the idleness +* of the engine. +*/ + if (IS_VALLEYVIEW(dev)) { + struct intel_engine_cs *bcs_ring = &dev_priv->ring[BCS]; + struct intel_engine_cs *vcs_ring = &dev_priv->ring[VCS]; + + if (list_empty(&bcs_ring->request_list) && + list_empty(&vcs_ring->request_list)) + use_mmio_flip = true; + else + use_mmio_flip = false; + } else + use_mmio_flip = false; + } + return use_mmio_flip; } static void intel_do_mmio_flip(struct intel_crtc *intel_crtc) -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v9 1/3] drm/i915: Replaced Blitter ring based flips with MMIO flips
From: Sourab Gupta This patch enables the framework for using MMIO based flip calls, in contrast with the CS based flip calls which are being used currently. MMIO based flip calls can be enabled on architectures where Render and Blitter engines reside in different power wells. The decision to use MMIO flips can be made based on workloads to give 100% residency for Media power well. v2: The MMIO flips now use the interrupt driven mechanism for issuing the flips when target seqno is reached. (Incorporating Ville's idea) v3: Rebasing on latest code. Code restructuring after incorporating Damien's comments v4: Addressing Ville's review comments -general cleanup -updating only base addr instead of calling update_primary_plane -extending patch for gen5+ platforms v5: Addressed Ville's review comments -Making mmio flip vs cs flip selection based on module parameter -Adding check for DRIVER_MODESET feature in notify_ring before calling notify mmio flip. -Other changes mostly in function arguments v6: -Having a seperate function to check condition for using mmio flips (Ville) -propogating error code from i915_gem_check_olr (Ville) v7: -Adding __must_check with i915_gem_check_olr (Chris) -Renaming mmio_flip_data to mmio_flip (Chris) -Rebasing on latest nightly v8: -Rebasing on latest code -squash 3rd patch in series(mmio setbase vs page flip race) with this patch -Added new tiling mode update in intel_do_mmio_flip (Chris) v9: -check for obj->last_write_seqno being 0 instead of obj->ring being NULL in intel_postpone_flip, as this is a more restrictive condition (Chris) Signed-off-by: Sourab Gupta Signed-off-by: Akash Goel --- drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 8 ++ drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_irq.c | 3 + drivers/gpu/drm/i915/i915_params.c | 4 + drivers/gpu/drm/i915/intel_display.c | 140 +++ drivers/gpu/drm/i915/intel_drv.h | 6 ++ 7 files changed, 163 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index b9159ad..532733a 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1572,6 +1572,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) spin_lock_init(&dev_priv->backlight_lock); spin_lock_init(&dev_priv->uncore.lock); spin_lock_init(&dev_priv->mm.object_stat_lock); + spin_lock_init(&dev_priv->mmio_flip_lock); mutex_init(&dev_priv->dpio_lock); mutex_init(&dev_priv->modeset_restore_lock); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bea9ab40..36e1b2a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1367,6 +1367,9 @@ struct drm_i915_private { /* protects the irq masks */ spinlock_t irq_lock; + /* protects the mmio flip data */ + spinlock_t mmio_flip_lock; + bool display_irqs_enabled; /* To control wakeup latency, e.g. for irq-driven dp aux transfers. */ @@ -2036,6 +2039,7 @@ struct i915_params { bool reset; bool disable_display; bool disable_vtd_wa; + bool use_mmio_flip; }; extern struct i915_params i915 __read_mostly; @@ -2231,6 +2235,8 @@ bool i915_gem_retire_requests(struct drm_device *dev); void i915_gem_retire_requests_ring(struct intel_engine_cs *ring); int __must_check i915_gem_check_wedge(struct i915_gpu_error *error, bool interruptible); +int __must_check i915_gem_check_olr(struct intel_engine_cs *ring, u32 seqno); + static inline bool i915_reset_in_progress(struct i915_gpu_error *error) { return unlikely(atomic_read(&error->reset_counter) @@ -2601,6 +2607,8 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data, int i915_get_reset_stats_ioctl(struct drm_device *dev, void *data, struct drm_file *file); +void intel_notify_mmio_flip(struct intel_engine_cs *ring); + /* overlay */ extern struct intel_overlay_error_state *intel_overlay_capture_error_state(struct drm_device *dev); extern void intel_overlay_print_error_state(struct drm_i915_error_state_buf *e, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 70b4f41..ab663ca 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1096,7 +1096,7 @@ i915_gem_check_wedge(struct i915_gpu_error *error, * Compare seqno against outstanding lazy request. Emit a request if they are * equal. */ -static int +__must_check int i915_gem_check_olr(struct intel_engine_cs *ring, u32 seqno) { int ret; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4ef6423..e0edb1f 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c
[Intel-gfx] [PATCH v4 0/3] Replace Blitter ring based flips with MMIO flips
From: Sourab Gupta This patch series enables the framework for using MMIO flips in place of Blitter ring based flips. This is useful for Media power well residency optimization. These may be enabled on architectures where Render and Blitter engines reside in different power wells. The blitter ring is currently being used just for command streamer based flip calls. The decision to use MMIO flips can be made based on workloads to give 100% residency for Media power well. Sourab Gupta (3): drm/i915: Replaced Blitter ring based flips with MMIO flips drm/i915: Selection of MMIO vs CS flip at page flip time drm/i915: Make module param for MMIO flip selection as tristate drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 8 ++ drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_irq.c | 3 + drivers/gpu/drm/i915/i915_params.c | 6 ++ drivers/gpu/drm/i915/intel_display.c | 173 ++- drivers/gpu/drm/i915/intel_drv.h | 6 ++ 7 files changed, 197 insertions(+), 2 deletions(-) -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx