Re: [Intel-gfx] [PATCH] drm/edid: Populate picture aspect ratio for CEA modes
On Mon, 20 Jan 2014 19:07:23 +0200 Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Mon, Dec 23, 2013 at 11:27:40AM +0530, Vandana Kannan wrote: Adding picture aspect ratio for CEA modes based on CEA-861D Table 3 or CEA-861E Table 4. This is useful for filling up the detail in AVI infoframe. v2: Ville's inputs incorporated. Added picture aspect ratio as part of edid_cea_modes instead of DRM_MODE Signed-off-by: Vandana Kannan vandana.kan...@intel.com Reviewed-by: Alex Deucher alexander.deuc...@amd.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Note this one is needed for the patch I just reviewed to populate the PAR bits. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915/vlv: use min brightness from VBT
On Mon, 31 Mar 2014 21:07:04 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Mon, Mar 31, 2014 at 11:13:57AM -0700, Jesse Barnes wrote: Going below the minimum value may affect the BLC_EN line, so try to use the VBT provided minimum where possible, otherwise use an experimentally derived value to prevent the panel from coming up. to prevent the panel form failing to come up I hope? Yes I suppose that makes more sense. I'm not trying to deliberately sabotage things, even though it may appear that way at times. :) -- 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 1/3] drm/i915: Program VSYNCSHIFT in a more consistent manner
On Fri, 28 Mar 2014 23:29:30 +0200 ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When interlaced sdvo output is used, vsyncshift should supposedly be (htotal-1)/2. In reality PIPECONF/TRANSCONF will override it by using the legacy vsyncshift interlace mode which causes the hardware to ignore the VSYNCSHIFT register. The only odd thing here is that on PCH platforms we program the VSYNCSHIFT on both CPU and PCH, and it's not entirely clear if both sides have to agree on the value or not. On the CPU side there's no way to override the value via PIPECONF anymore, so if we want to make the CPU side agree with the PCH side, we should probably program the approriate value into VSYNCSHIFT manually. So let's do that, but for now leave the PCH side to still use the legacy interlace mode in TRANSCONF. We can also drop the gen2 check since gen2 doesn't support interlaced modes at all. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d633139..a9a4f6a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5377,21 +5377,23 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc) enum transcoder cpu_transcoder = intel_crtc-config.cpu_transcoder; struct drm_display_mode *adjusted_mode = intel_crtc-config.adjusted_mode; - uint32_t vsyncshift, crtc_vtotal, crtc_vblank_end; + uint32_t vsyncshift = 0, crtc_vtotal, crtc_vblank_end; /* We need to be careful not to changed the adjusted mode, for otherwise * the hw state checker will get angry at the mismatch. */ crtc_vtotal = adjusted_mode-crtc_vtotal; crtc_vblank_end = adjusted_mode-crtc_vblank_end; - if (!IS_GEN2(dev) adjusted_mode-flags DRM_MODE_FLAG_INTERLACE) { + if (adjusted_mode-flags DRM_MODE_FLAG_INTERLACE) { /* the chip adds 2 halflines automatically */ crtc_vtotal -= 1; crtc_vblank_end -= 1; - vsyncshift = adjusted_mode-crtc_hsync_start - - adjusted_mode-crtc_htotal / 2; - } else { - vsyncshift = 0; + + if (intel_pipe_has_type(intel_crtc-base, INTEL_OUTPUT_SDVO)) + vsyncshift = (adjusted_mode-crtc_htotal - 1) / 2; + else + vsyncshift = adjusted_mode-crtc_hsync_start - + adjusted_mode-crtc_htotal / 2; } if (INTEL_INFO(dev)-gen 3) My only concern here is that some chip might try to use a nonzero vsyncshift for a non-interlaced mode. But that should be easy to bisect to if so, so Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 2/3] drm/i915: Fix the interlace mode selection for gmch platforms
On Fri, 28 Mar 2014 23:29:31 +0200 ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com PIPECONF_INTERLACE_W_FIELD_INDICATION is only meant to be used for sdvo since it implies a slightly weird vsync shift of htotal/2. For everything else we should use PIPECONF_INTERLACE_W_SYNC_SHIFT and let the value in the VSYNCSHIFT register take effect. The only exception is gen3 simply because VSYNCSHIFT didn't exist yet. Gen2 doesn't support interlaced modes at all, so we can drop the explicit gen2 checks. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a9a4f6a..3ab40e3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5543,13 +5543,13 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc) } } - if (IS_VALLEYVIEW(dev) - intel_crtc-config.adjusted_mode.flags DRM_MODE_FLAG_INTERLACE) - pipeconf |= PIPECONF_INTERLACE_W_SYNC_SHIFT; - else if (!IS_GEN2(dev) - intel_crtc-config.adjusted_mode.flags DRM_MODE_FLAG_INTERLACE) - pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION; - else + if (intel_crtc-config.adjusted_mode.flags DRM_MODE_FLAG_INTERLACE) { + if (INTEL_INFO(dev)-gen 4 || + intel_pipe_has_type(intel_crtc-base, INTEL_OUTPUT_SDVO)) + pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION; + else + pipeconf |= PIPECONF_INTERLACE_W_SYNC_SHIFT; + } else pipeconf |= PIPECONF_PROGRESSIVE; if (IS_VALLEYVIEW(dev) intel_crtc-config.limited_color_range) Hooray for SDVO. I really hope no one tries to do that on VLV... (afaik it's unsupported but who knows the hw might work if someone solders such a board together). Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 3/3] drm/i915: Make sure vsyncshift is positive
On Fri, 28 Mar 2014 23:29:32 +0200 ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If vsyncshift comes out as negative, add one htotal to it to get the corresponding positive value. This is rather theoretical as it would require a mode where the hsync+back porch is very long and the active+front porch very short. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3ab40e3..7d3b18b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5377,7 +5377,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc) enum transcoder cpu_transcoder = intel_crtc-config.cpu_transcoder; struct drm_display_mode *adjusted_mode = intel_crtc-config.adjusted_mode; - uint32_t vsyncshift = 0, crtc_vtotal, crtc_vblank_end; + uint32_t crtc_vtotal, crtc_vblank_end; + int vsyncshift = 0; /* We need to be careful not to changed the adjusted mode, for otherwise * the hw state checker will get angry at the mismatch. */ @@ -5394,6 +5395,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc) else vsyncshift = adjusted_mode-crtc_hsync_start - adjusted_mode-crtc_htotal / 2; + if (vsyncshift 0) + vsyncshift += adjusted_mode-crtc_htotal; } if (INTEL_INFO(dev)-gen 3) Funky indeed. I wonder if we should congratulate the user if we detect this configuration. Achievement unlocked: funky mode timings!. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- Jesse Barnes, 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/vlv: use W_SYNC_SHIFT for interlaced modes on VLV
This makes HDMI testers happier on VLV platforms. It may be that we need it for any non-SVO platform, but I don't have any tests to back that up, so I'm leaving other pre-ILK platforms alone for now. Tested-by: Clint Taylor clinton.a.tay...@intel.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e0a87aa..d633139 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5541,8 +5541,11 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc) } } - if (!IS_GEN2(dev) + if (IS_VALLEYVIEW(dev) intel_crtc-config.adjusted_mode.flags DRM_MODE_FLAG_INTERLACE) + pipeconf |= PIPECONF_INTERLACE_W_SYNC_SHIFT; + else if (!IS_GEN2(dev) +intel_crtc-config.adjusted_mode.flags DRM_MODE_FLAG_INTERLACE) pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION; else pipeconf |= PIPECONF_PROGRESSIVE; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] [v4] drm/i915/bdw: Ensure a context is loaded before RC6
On Thu, 20 Mar 2014 14:42:32 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Wed, Mar 19, 2014 at 05:41:37PM -0700, Ben Widawsky wrote: I can't say it's completely unexpected that this would be your response, but I do feel like you've ignored my argument that this is better than the current situation. Not merging this patch only keeps things bad. So I'd like you to re-consider merging this patch instead of waiting for the perfect solution. This patch requires a lot less review than the real fix. It has been tested by several people (I can ask them to put their reviewed by on it). It enables rc6 for people today, and this includes pc7, and deeper package and C states. It's very easy to revert if/when we get a real fix. Real users benefit from this patch. Real users are not hurt by this patch because if userspace is submitting bad state setup, they'll have the same or worse experience than failing RC6. As an aside, this needs to come before I enable rc6 anyway. So the order way bad. I fully agree with your assessment on technical reasons. The problem is that I've just been forced through an exercise of merge this now because it works, people have tested it and we really, really, really need it to move forward and it didn't go down well. Which means for the foreseeable future I'll reject patches when it looks like a few too many rolls of ducttape have been involved in their construction. I'd prefer if we could move more towards a merge early or at least integration-test early model, but currently that's not a viable model for me. Note that this is not an iron rule at all, e.g. with psr I've just told Rodrigo that I want the current state of affairs finalized for merging instead of trying to hunt for the perfect patches. The reason for that was that I think the remaining issues in psr support are well-understood and we have solid test-coverage to make sure we don't fumble things. Also one issue with the current psr patches is that they're way too conservative in a few cases (i.e. wasting power), but for something tricky leaning towards correctness is actually a feature. And bad power numbers tend to grab our project managements attention. Overall the risks are a fairly clear quantity. In this area of rc6 and contexts though we have track record of not really understanding issues. Which means that the risks here are unknown and might be fairly big. Do you think this patch falls into that class of issues? It seems like it's a general improvement, even if it doesn't address the more general behavior we'd like (sooner than later really). But blocking it until we have the full primitive emission seems like it's going to keep power consumption in a terrible state on BDW for the forseeable future... moreover I guess this is something we need going back forever for RC6 stability, at least according to the hw team. So rather than blocking this, maybe we should commit it for all platforms! -- 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 11/11] [v4] drm/i915/bdw: Ensure a context is loaded before RC6
On Thu, 20 Mar 2014 10:30:32 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: On Thu, 20 Mar 2014 14:42:32 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Wed, Mar 19, 2014 at 05:41:37PM -0700, Ben Widawsky wrote: I can't say it's completely unexpected that this would be your response, but I do feel like you've ignored my argument that this is better than the current situation. Not merging this patch only keeps things bad. So I'd like you to re-consider merging this patch instead of waiting for the perfect solution. This patch requires a lot less review than the real fix. It has been tested by several people (I can ask them to put their reviewed by on it). It enables rc6 for people today, and this includes pc7, and deeper package and C states. It's very easy to revert if/when we get a real fix. Real users benefit from this patch. Real users are not hurt by this patch because if userspace is submitting bad state setup, they'll have the same or worse experience than failing RC6. As an aside, this needs to come before I enable rc6 anyway. So the order way bad. I fully agree with your assessment on technical reasons. The problem is that I've just been forced through an exercise of merge this now because it works, people have tested it and we really, really, really need it to move forward and it didn't go down well. Which means for the foreseeable future I'll reject patches when it looks like a few too many rolls of ducttape have been involved in their construction. I'd prefer if we could move more towards a merge early or at least integration-test early model, but currently that's not a viable model for me. Note that this is not an iron rule at all, e.g. with psr I've just told Rodrigo that I want the current state of affairs finalized for merging instead of trying to hunt for the perfect patches. The reason for that was that I think the remaining issues in psr support are well-understood and we have solid test-coverage to make sure we don't fumble things. Also one issue with the current psr patches is that they're way too conservative in a few cases (i.e. wasting power), but for something tricky leaning towards correctness is actually a feature. And bad power numbers tend to grab our project managements attention. Overall the risks are a fairly clear quantity. In this area of rc6 and contexts though we have track record of not really understanding issues. Which means that the risks here are unknown and might be fairly big. Do you think this patch falls into that class of issues? It seems like it's a general improvement, even if it doesn't address the more general behavior we'd like (sooner than later really). But blocking it until we have the full primitive emission seems like it's going to keep power consumption in a terrible state on BDW for the forseeable future... moreover I guess this is something we need going back forever for RC6 stability, at least according to the hw team. So rather than blocking this, maybe we should commit it for all platforms! Summarizing IRC discussion a bit: speaking generally of when some future work blocks an existing fix or feature, we really need to make sure someone is working on the broader task and make sure we track it so it doesn't get lost, otherwise everyone loses. The user loses because a fix or feature isn't available, the author loses because something gets blocked indefinitely, and upstream loses because either a fix doesn't land or it does and the larger feature never gets implemented because the pressure is off. So with that, who wants to volunteer to update the 3D state emission patch to include BDW and push it upstream? Daniel promised to file a JIRA task for this so our PM can track it, so someone will be volunteered in the next week or so if we don't get it done before then. -- 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: no MCHBAR on VLV
Junxiao, can you add you reviewed-by to this patch? Thanks, Jesse On Mon, 3 Mar 2014 14:27:57 -0800 Jesse Barnes jbar...@virtuousgeek.org wrote: So don't try to allocate and program it, we're only fooling ourselves. Reported-by: Chang, Junxiao junxiao.ch...@intel.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_dma.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 7688abc..22f839b 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1187,6 +1187,9 @@ intel_setup_mchbar(struct drm_device *dev) u32 temp; bool enabled; + if (IS_VALLEYVIEW(dev)) + return; + dev_priv-mchbar_need_disable = false; if (IS_I915G(dev) || IS_I915GM(dev)) { -- 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 12/26] drm/i915: Page table helpers, and define renames
On Tue, 18 Mar 2014 09:05:58 + Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Mar 17, 2014 at 10:48:44PM -0700, Ben Widawsky wrote: --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -1,8 +1,11 @@ #ifndef _I915_GEM_GTT_H #define _I915_GEM_GTT_H -#define GEN6_PPGTT_PD_ENTRIES 512 -#define I915_PPGTT_PT_ENTRIES (PAGE_SIZE / sizeof(gen6_gtt_pte_t)) +/* GEN Agnostic defines */ +#define I915_PDES_PER_PD 512 +#define I915_PTE_MASK (PAGE_SHIFT-1) That looks decidely fishy. PAGE_SHIFT is 12 - PTE_MASK = 0xb +#define I915_PDE_MASK (I915_PDES_PER_PD-1) + typedef uint32_t gen6_gtt_pte_t; typedef uint64_t gen8_gtt_pte_t; typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; @@ -23,6 +26,98 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; #define GEN6_PTE_ADDR_ENCODE(addr) GEN6_GTT_ADDR_ENCODE(addr) #define HSW_PTE_ADDR_ENCODE(addr) HSW_GTT_ADDR_ENCODE(addr) + +/* GEN6 PPGTT resembles a 2 level page table: + * 31:22 | 21:12 | 11:0 + * PDE | PTE | offset + */ +#define GEN6_PDE_SHIFT 22 +#define GEN6_PTES_PER_PT (PAGE_SIZE / sizeof(gen6_gtt_pte_t)) + +static inline uint32_t i915_pte_index(uint64_t address, uint32_t pde_shift) +{ + const uint32_t mask = (1 (pde_shift - PAGE_SHIFT)) - 1; + return (address PAGE_SHIFT) mask; +} + +/* Helper to counts the number of PTEs within the given length. This count does + * not cross a page table boundary, so the max value would be + * GEN6_PTES_PER_PT for GEN6, and GEN8_PTES_PER_PT for GEN8. + */ +static inline size_t i915_pte_count(uint64_t addr, size_t length, + uint32_t pde_shift) +{ + const uint64_t pd_mask = ~((1 pde_shift) - 1); + uint64_t end; + + if (WARN_ON(!length)) + return 0; + + if (WARN_ON(addr % PAGE_SIZE)) + addr = round_down(addr, PAGE_SIZE); + + if (WARN_ON(length % PAGE_SIZE)) + length = round_up(length, PAGE_SIZE); Oh oh. I think these fixups are very suspect, so just BUG_ON(length == 0); BUG_ON(offset_in_page(addr|length)); + + end = addr + length; + + if ((addr pd_mask) != (end pd_mask)) + return (1 (pde_shift - PAGE_SHIFT)) - #define NUM_PTE(pde_shift) (1 (pde_shift - PAGE_SHIFT)) here and for computing the pd_mask. + i915_pte_index(addr, pde_shift); + + return i915_pte_index(end, pde_shift) - i915_pte_index(addr, pde_shift); +} Otherwise the helpers look a useful improvement in readability. -Chris Can we use GTT_PAGE_SIZE here too? I'm worried the kernel PAGE_SIZE will change at some point and blow us up. At least in places where we're doing our own thing rather than using the x86 bits... -- 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: Add a DRM property psr
On Tue, 18 Mar 2014 12:51:07 -0700 Siva Chandra sivachan...@chromium.org wrote: This property helps one turn PSR on and off via xrandr. The default value is same as that of the module param i915.enable_psr. Signed-off-by: Siva Chandra sivachan...@google.com --- So are you using this in Chromium for disabling PSR in cases where it doesn't work? Or to optimize power consumption when the kernel driver gets it wrong? Or just for debug? Seems potentially useful, just curious what motivated you guys. Thanks, -- Jesse Barnes, 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 1/4] drm/i915: preserve SSC if previously set v2
Some machines may have a broken VBT or no VBT at all, but we still want to use SSC there. So check for it and keep it enabled if we see it already on. Based on an earlier fix from Kristian. v2: honor modparam if set too (Daniel) read out at init time and store for panel_use_ssc() use (Jesse) Reported-by: Kristian Høgsberg hoegsb...@gmail.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_drv.h |2 ++ drivers/gpu/drm/i915/intel_display.c | 11 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 70fbe90..c64f770 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1507,6 +1507,8 @@ typedef struct drm_i915_private { struct intel_opregion opregion; struct intel_vbt_data vbt; + bool bios_ssc; /* BIOS had SSC enabled at boot? */ + /* overlay */ struct intel_overlay *overlay; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2bccc68..4b3e1c0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4986,7 +4986,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) { if (i915.panel_use_ssc = 0) return i915.panel_use_ssc != 0; - return dev_priv-vbt.lvds_use_ssc + return (dev_priv-vbt.lvds_use_ssc || dev_priv-bios_ssc) !(dev_priv-quirks QUIRK_LVDS_SSC_DISABLE); } @@ -11732,9 +11732,18 @@ void intel_modeset_setup_hw_state(struct drm_device *dev, void intel_modeset_gem_init(struct drm_device *dev) { + struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *c; struct intel_framebuffer *fb; + /* +* There may be no VBT; and if the BIOS enabled SSC we can +* just keep using it to avoid unnecessary flicker. +*/ + if ((HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) + (I915_READ(PCH_DREF_CONTROL) DREF_SSC1_ENABLE)) + dev_priv-bios_ssc = true; + intel_modeset_init_hw(dev); intel_setup_overlay(dev); -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: enable fastboot by default
Let them eat cake. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_params.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index a66ffb6..5f81047 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), .disable_power_well = 1, .enable_ips = 1, - .fastboot = 0, + .fastboot = 42, .enable_pc8 = 1, .pc8_timeout = 5000, .prefault_disable = 0, -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: use current mode if the size matches the preferred mode
From: Kristian Høgsberg hoegsb...@gmail.com The BIOS may set a native mode that doesn't quite match the preferred mode timings. It should be ok to use however if it uses the same size, so try to avoid a mode set in that case. Signed-off-by: Kristian Høgsberg hoegsb...@gmail.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/drm_modes.c|8 drivers/gpu/drm/i915/intel_fbdev.c | 37 ++-- include/drm/drm_crtc.h |2 ++ 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index b073315..7d2dda4 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -894,6 +894,14 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, } EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo); +bool drm_mode_same_size(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return mode1-vdisplay == mode2-vdisplay + mode1-hdisplay == mode2-hdisplay; +} +EXPORT_SYMBOL(drm_mode_same_size); + /** * drm_mode_validate_size - make sure modes adhere to size constraints * @dev: DRM device diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index d6d78c8..f81e3db 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -369,31 +369,30 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, /* go for command line mode first */ modes[i] = drm_pick_cmdline_mode(fb_conn, width, height); - /* try for preferred next */ + /* try for preferred next or match current */ if (!modes[i]) { + struct drm_display_mode *preferred; + DRM_DEBUG_KMS(looking for preferred mode on connector %d\n, fb_conn-connector-base.id); - modes[i] = drm_has_preferred_mode(fb_conn, width, - height); - } - - /* last resort: use current mode */ - if (!modes[i]) { - /* -* IMPORTANT: We want to use the adjusted mode (i.e. -* after the panel fitter upscaling) as the initial -* config, not the input mode, which is what crtc-mode -* usually contains. But since our current fastboot -* code puts a mode derived from the post-pfit timings -* into crtc-mode this works out correctly. We don't -* use hwmode anywhere right now, so use it for this -* since the fb helper layer wants a pointer to -* something we own. -*/ + preferred = drm_has_preferred_mode(fb_conn, width, + height); intel_mode_from_pipe_config(encoder-crtc-hwmode, to_intel_crtc(encoder-crtc)-config); - modes[i] = encoder-crtc-hwmode; + modes[i] = encoder-crtc-hwmode; + + if (preferred + !drm_mode_same_size(preferred, modes[i])) { + DRM_DEBUG_KMS(using preferred mode %s + instead of current mode %s + on connector %d\n, + preferred-name, + modes[i]-name, + fb_conn-connector-base.id); + modes[i] = preferred; + } } + crtcs[i] = new_crtc; DRM_DEBUG_KMS(connector %s on crtc %d: %s\n, diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index f764654..d5ebe3b 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1020,6 +1020,8 @@ extern bool drm_mode_equal(const struct drm_display_mode *mode1, const struct dr extern bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); extern int drm_mode_width(const struct drm_display_mode *mode); extern int drm_mode_height(const struct drm_display_mode *mode); +extern bool drm_mode_same_size(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2); /* for us by fb module */ extern struct drm_display_mode *drm_mode_create(struct drm_device *dev); -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: don't bother enabling swizzle bits on gen7+ v2
As of IVB, the memory controller does internal swizzling already, so we shouldn't need to enable these. Based on an earlier fix from Kristian. v2: preserve swizzling if BIOS had it set (Daniel) Reported-by: Kristian Høgsberg hoegsb...@gmail.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_drv.h|1 + drivers/gpu/drm/i915/i915_gem.c|6 ++ drivers/gpu/drm/i915/i915_gem_tiling.c |3 ++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c64f770..29cd977 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1508,6 +1508,7 @@ typedef struct drm_i915_private { struct intel_vbt_data vbt; bool bios_ssc; /* BIOS had SSC enabled at boot? */ + bool bios_swizzle; /* overlay */ struct intel_overlay *overlay; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 92b0b41..87e34bc 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4313,6 +4313,9 @@ void i915_gem_init_swizzling(struct drm_device *dev) dev_priv-mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE) return; + if (INTEL_INFO(dev)-gen = 7 !dev_priv-bios_swizzle) + return; + I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) | DISP_TILE_SURFACE_SWIZZLING); @@ -4454,6 +4457,9 @@ int i915_gem_init(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; int ret; + if (I915_READ(DISP_ARB_CTL) DISP_TILE_SURFACE_SWIZZLING) + dev_priv-bios_swizzle = true; + mutex_lock(dev-struct_mutex); if (IS_VALLEYVIEW(dev)) { diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c index eb99358..c6447de 100644 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c @@ -91,7 +91,8 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev) uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; - if (IS_VALLEYVIEW(dev)) { + if (INTEL_INFO(dev)-gen = 7 + !(I915_READ(TILECTL) TILECTL_SWZCTL)) { swizzle_x = I915_BIT_6_SWIZZLE_NONE; swizzle_y = I915_BIT_6_SWIZZLE_NONE; } else if (INTEL_INFO(dev)-gen = 6) { -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Documentation requirements for drm/i915 feature work
On Tue, 11 Mar 2014 12:21:32 +0100 Daniel Vetter dan...@ffwll.ch wrote: Hi all, So I guess people have seen the writing on the wall since a while ;-) So with my latest drm kerneldoc series we'll have fairly nice interface docs for most of the still relevant drm core subsystems. Which means we can finally start to look at our own driver. I've already started with a very basic skeleton as part of my latest kerneldoc series, see http://people.freedesktop.org/~danvet/drm/drmI915.html Now we only need to flesh this out! I see three areas where decent documentation provides good value: 1) High level overviews of a feature, i.e. what I've done thus far in my blog posts. 2) Detailed in-driver api documentation as kerneldoc. 3) Documentating userspace ABIs like ioctls structuresflags, properties and so on. I have no idea how to do 3) well, see e.g. the discussion on documenting drm properties. And the drm core is completely undocumented in that area anyway afaik. So I think we can postpone this for now. IMO (3) very much belongs in libdrm as man page updates. We need to be good about catching this on review for new stuff. For older stuff I think there was a bit of momentum awhile back, but it seems to have dissipated. We could try to extract it from kernel source somehow, but for user API stuff, I think we really want man pages in libdrm, in addition to whatever web based documentation we make available. -- 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] [RFC] Documentation requirements for drm/i915 feature work
On Fri, 14 Mar 2014 19:16:01 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Fri, Mar 14, 2014 at 7:03 PM, Jesse Barnes jbar...@virtuousgeek.org wrote: Yeah just saying a man page should be required as part of any new ioctl. Yeah I agree and long-term we'll get there. Otherwise I wouldn't have added it. But imo for a documentation requirement for merging features we need a few things ready first: a) Have a somewhat useful skeleton. For drm core Laurent made this happen and then the details (mostly api docs) have been slowly filled out over the past 1-2 years). Now we're ready to crawl into drivers. b) Have someone with good experience with the tooling. I've written and reviewed lots of kerneldoc api patches for drm, so I think we're covered. Those are also the reasons why I'm writing piles of igt docs just now - we need a bit a baseline so that people have lots of examples to follow and I'm learning the tooling to figure out what works and what doesn't. For ioctls we have a bit of manpages, but only for the libdrm functions and not the ioctls themselves, and only for drm core stuff. Hence why I think ioctl docs aren't for the masses yet. But if someone digs in and lays that groundwork and is willing to review patches a bit at the beginnning I'll happily support that by rejecting new ioctls without such docs. We have the groundwork in libdrm already, along with a couple of pages, that's where I'd expect them to land. -- 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: Fix runtime pm inbalance due to reg I/O forcewake dance
On Fri, 14 Mar 2014 09:52:47 +0100 Daniel Vetter daniel.vet...@ffwll.ch wrote: On Fri, Mar 14, 2014 at 9:34 AM, Ville Syrjälä ville.syrj...@linux.intel.com wrote: Heh, I went the other way round, dropped the runtime pm put from the timer and flushed the forcewaker timer when we suspended the device... That's what I meant. No delayed runtime_pm_put. Well I've figured we want to keep this ... have things changed sufficiently meanwhile? I've lost a bit track in all that recent shuffling ... Can we pull the forcewake bits out of the reg functions and just do a check and WARN instead? -- 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 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2
On Sat, 8 Mar 2014 11:33:15 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Fri, Mar 07, 2014 at 08:57:55AM -0800, Jesse Barnes wrote: By stuffing the fb allocation into the crtc, we get mode set lifetime refcounting for free, but have to handle the initial pin fence slightly differently. It also means we can move the shared fb handling into the core rather than leaving it out in the fbdev code. v2: null out crtc-fb on error (Daniel) take fbdev fb ref and remove unused error path (Daniel) Requested-by: Daniel Vetter dan...@ffwll.ch Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Ok, I've merged patches 1-4 and this one here from the series. Not terribly happy that you didn't squash in this fixup, since now people might stumble over the strange lifetime rules of the previous patches. But meh ;-) Yeah even though there's a handoff from the core to the fbdev side, I still think they're correct as far as fbs go. The underlying obj ref may not be correct though; I think that was papering over an earlier bug. Either way those should manifest as a leaked object (the stolen fb will stick around forever) rather than a crash or something. Whereas this last patch is more likely to cause serious trouble I think since it's a bit more invasive and relies on some other bits more... Anyway thanks, looking forward to seeing the perf data on the swizzle stuff. -- 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 6/8] drm/i915: don't bother enabling swizzle bits on gen7+
On Sat, 8 Mar 2014 11:36:24 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Fri, Mar 07, 2014 at 08:57:53AM -0800, Jesse Barnes wrote: As of IVB, the memory controller does internal swizzling already, so we shouldn't need to enable these. Based on an earlier fix from Kristian. Reported-by: Kristian Høgsberg hoegsb...@gmail.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Imo the right approach here is to check whether any of the preserved/inherited framebuffers has tiling enabled, and if so we need to preserve the swizzling mode the bios has set. Also this should be done on gen6+ since those are the machines where swizzling can be changed. Ah yeah good point... haven't checked to see if any BIOSes enable this automatically. -- 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 v8 3/5] drm/i915: Make sprite updates atomic
+596,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, pixel_size, fb-pitches[0]); linear_offset -= dvssurf_offset; + atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count); + I915_WRITE(DVSSTRIDE(pipe), fb-pitches[0]); I915_WRITE(DVSPOS(pipe), (crtc_y 16) | crtc_x); @@ -492,6 +612,9 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, I915_WRITE(DVSSURF(pipe), i915_gem_obj_ggtt_offset(obj) + dvssurf_offset); POSTING_READ(DVSSURF(pipe)); + + if (atomic_update) + intel_pipe_update_end(intel_crtc, start_vbl_count); } static void @@ -500,7 +623,12 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) 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 = to_intel_crtc(crtc); int pipe = intel_plane-pipe; + u32 start_vbl_count; + bool atomic_update; + + atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count); I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) ~DVS_ENABLE); /* Disable the scaler */ @@ -509,6 +637,9 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) I915_WRITE(DVSSURF(pipe), 0); POSTING_READ(DVSSURF(pipe)); + if (atomic_update) + intel_pipe_update_end(intel_crtc, start_vbl_count); + /* * Avoid underruns when disabling the sprite. * FIXME remove once watermark updates are done properly. Yeah looks like this will work ok. I don't understand the prepare_to_wait() comment, since we're both holding the crtc mutex and prepare_to_wait() will take the crtc vbl_wait queue lock, but since things look safe as-is I guess it's not a big deal. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- Jesse Barnes, 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 1/8] drm/i915: add plane_config fetching infrastructure v2
Early at init time, we can try to read out the plane config structure and try to preserve it if possible. v2: alloc fb obj at init time after fetching plane config Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/intel_display.c | 92 drivers/gpu/drm/i915/intel_drv.h | 9 3 files changed, 104 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 29da39f..c9aad90 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -405,6 +405,7 @@ struct drm_i915_error_state { struct intel_connector; struct intel_crtc_config; +struct intel_plane_config; struct intel_crtc; struct intel_limit; struct dpll; @@ -443,6 +444,8 @@ struct drm_i915_display_funcs { * fills out the pipe-config with the hw state. */ bool (*get_pipe_config)(struct intel_crtc *, struct intel_crtc_config *); + void (*get_plane_config)(struct intel_crtc *, +struct intel_plane_config *); int (*crtc_mode_set)(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7e8bfd8..e793c2a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2047,6 +2047,70 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y, } } +int intel_format_to_fourcc(int format) +{ + switch (format) { + case DISPPLANE_8BPP: + return DRM_FORMAT_C8; + case DISPPLANE_BGRX555: + return DRM_FORMAT_XRGB1555; + case DISPPLANE_BGRX565: + return DRM_FORMAT_RGB565; + default: + case DISPPLANE_BGRX888: + return DRM_FORMAT_XRGB; + case DISPPLANE_RGBX888: + return DRM_FORMAT_XBGR; + case DISPPLANE_BGRX101010: + return DRM_FORMAT_XRGB2101010; + case DISPPLANE_RGBX101010: + return DRM_FORMAT_XBGR2101010; + } +} + +static void intel_alloc_plane_obj(struct intel_crtc *crtc, + struct intel_plane_config *plane_config) +{ + struct drm_device *dev = crtc-base.dev; + struct drm_i915_gem_object *obj = NULL; + struct drm_mode_fb_cmd2 mode_cmd = { 0 }; + u32 base = plane_config-base; + + if (!plane_config-fb) + return; + + obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base, + plane_config-size); + if (!obj) + return; + + if (plane_config-tiled) { + obj-tiling_mode = I915_TILING_X; + obj-stride = plane_config-fb-base.pitches[0]; + } + + mode_cmd.pixel_format = plane_config-fb-base.pixel_format; + mode_cmd.width = plane_config-fb-base.width; + mode_cmd.height = plane_config-fb-base.height; + mode_cmd.pitches[0] = plane_config-fb-base.pitches[0]; + + mutex_lock(dev-struct_mutex); + + if (intel_framebuffer_init(dev, plane_config-fb, mode_cmd, obj)) { + DRM_DEBUG_KMS(intel fb init failed\n); + goto out_unref_obj; + } + + mutex_unlock(dev-struct_mutex); + DRM_DEBUG_KMS(plane fb obj %p\n, plane_config-fb-obj); + return; + +out_unref_obj: + drm_gem_object_unreference(obj-base); + mutex_unlock(dev-struct_mutex); + +} + static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, int x, int y) { @@ -10997,6 +11061,7 @@ void intel_modeset_init(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; int sprite, ret; enum pipe pipe; + struct intel_crtc *crtc; drm_mode_config_init(dev); @@ -11059,6 +11124,33 @@ void intel_modeset_init(struct drm_device *dev) mutex_lock(dev-mode_config.mutex); intel_modeset_setup_hw_state(dev, false); mutex_unlock(dev-mode_config.mutex); + + list_for_each_entry(crtc, dev-mode_config.crtc_list, + base.head) { + if (!crtc-active) + continue; + +#if IS_ENABLED(CONFIG_FB) + /* +* We don't have a good way of freeing the buffer w/o the FB +* layer owning it... +* Note that reserving the BIOS fb up front prevents us +* from stuffing other stolen allocations like the ring +* on top. This prevents some ugliness at boot time, and +* can even allow for smooth boot transitions if the BIOS +* fb is large enough for the active pipe configuration
[Intel-gfx] [PATCH 3/8] drm/i915: get_plane_config support for ILK+ v3
This should allow BIOS fb inheritance to work on ILK+ machines too. v2: handle tiled BIOS fbs (Kristian) split out common bits (Jesse) v3: alloc fb obj out in _init Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c | 62 1 file changed, 62 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index da7bac5..b933a92 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6631,6 +6631,66 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc, } } +static void ironlake_get_plane_config(struct intel_crtc *crtc, + struct intel_plane_config *plane_config) +{ + struct drm_device *dev = crtc-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + u32 val, base, offset; + int pipe = crtc-pipe, plane = crtc-plane; + int fourcc, pixel_format; + int aligned_height; + + plane_config-fb = kzalloc(sizeof(*plane_config-fb), GFP_KERNEL); + if (!plane_config-fb) { + DRM_DEBUG_KMS(failed to alloc fb\n); + return; + } + + val = I915_READ(DSPCNTR(plane)); + + if (INTEL_INFO(dev)-gen = 4) + if (val DISPPLANE_TILED) + plane_config-tiled = true; + + pixel_format = val DISPPLANE_PIXFORMAT_MASK; + fourcc = intel_format_to_fourcc(pixel_format); + plane_config-fb-base.pixel_format = fourcc; + plane_config-fb-base.bits_per_pixel = + drm_format_plane_cpp(fourcc, 0) * 8; + + base = I915_READ(DSPSURF(plane)) 0xf000; + if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { + offset = I915_READ(DSPOFFSET(plane)); + } else { + if (plane_config-tiled) + offset = I915_READ(DSPTILEOFF(plane)); + else + offset = I915_READ(DSPLINOFF(plane)); + } + plane_config-base = base; + + val = I915_READ(PIPESRC(pipe)); + plane_config-fb-base.width = ((val 16) 0xfff) + 1; + plane_config-fb-base.height = ((val 0) 0xfff) + 1; + + val = I915_READ(DSPSTRIDE(pipe)); + plane_config-fb-base.pitches[0] = val 0xff80; + + aligned_height = intel_align_height(dev, plane_config-fb-base.height, + plane_config-tiled); + + plane_config-size = ALIGN(plane_config-fb-base.pitches[0] * + aligned_height, PAGE_SIZE); + + DRM_DEBUG_KMS(pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n, + pipe, plane, plane_config-fb-base.width, + plane_config-fb-base.height, + plane_config-fb-base.bits_per_pixel, base, + plane_config-fb-base.pitches[0], + plane_config-size); +} + static bool ironlake_get_pipe_config(struct intel_crtc *crtc, struct intel_crtc_config *pipe_config) { @@ -10839,6 +10899,7 @@ static void intel_init_display(struct drm_device *dev) if (HAS_DDI(dev)) { dev_priv-display.get_pipe_config = haswell_get_pipe_config; + dev_priv-display.get_plane_config = ironlake_get_plane_config; dev_priv-display.crtc_mode_set = haswell_crtc_mode_set; dev_priv-display.crtc_enable = haswell_crtc_enable; dev_priv-display.crtc_disable = haswell_crtc_disable; @@ -10846,6 +10907,7 @@ static void intel_init_display(struct drm_device *dev) dev_priv-display.update_plane = ironlake_update_plane; } else if (HAS_PCH_SPLIT(dev)) { dev_priv-display.get_pipe_config = ironlake_get_pipe_config; + dev_priv-display.get_plane_config = ironlake_get_plane_config; dev_priv-display.crtc_mode_set = ironlake_crtc_mode_set; dev_priv-display.crtc_enable = ironlake_crtc_enable; dev_priv-display.crtc_disable = ironlake_crtc_disable; -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/8] drm/i915: don't bother enabling swizzle bits on gen7+
As of IVB, the memory controller does internal swizzling already, so we shouldn't need to enable these. Based on an earlier fix from Kristian. Reported-by: Kristian Høgsberg hoegsb...@gmail.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_gem.c| 7 +++ drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 18ea6bc..dcf4b01 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4296,6 +4296,9 @@ void i915_gem_init_swizzling(struct drm_device *dev) dev_priv-mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE) return; + if (INTEL_INFO(dev)-gen = 7) + return; + I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) | DISP_TILE_SURFACE_SWIZZLING); @@ -4305,10 +4308,6 @@ void i915_gem_init_swizzling(struct drm_device *dev) I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_SWZCTL); if (IS_GEN6(dev)) I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_SNB)); - else if (IS_GEN7(dev)) - I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_IVB)); - else if (IS_GEN8(dev)) - I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_BDW)); else BUG(); } diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c index eb99358..05c5d98 100644 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c @@ -91,7 +91,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev) uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; - if (IS_VALLEYVIEW(dev)) { + if (INTEL_INFO(dev)-gen = 7) { swizzle_x = I915_BIT_6_SWIZZLE_NONE; swizzle_y = I915_BIT_6_SWIZZLE_NONE; } else if (INTEL_INFO(dev)-gen = 6) { -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2
By stuffing the fb allocation into the crtc, we get mode set lifetime refcounting for free, but have to handle the initial pin fence slightly differently. It also means we can move the shared fb handling into the core rather than leaving it out in the fbdev code. v2: null out crtc-fb on error (Daniel) take fbdev fb ref and remove unused error path (Daniel) Requested-by: Daniel Vetter dan...@ffwll.ch Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c | 145 --- drivers/gpu/drm/i915/intel_drv.h | 1 - drivers/gpu/drm/i915/intel_fbdev.c | 38 + 3 files changed, 105 insertions(+), 79 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d450ab6..718cc73 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2068,7 +2068,7 @@ int intel_format_to_fourcc(int format) } } -static void intel_alloc_plane_obj(struct intel_crtc *crtc, +static bool intel_alloc_plane_obj(struct intel_crtc *crtc, struct intel_plane_config *plane_config) { struct drm_device *dev = crtc-base.dev; @@ -2076,38 +2076,76 @@ static void intel_alloc_plane_obj(struct intel_crtc *crtc, struct drm_mode_fb_cmd2 mode_cmd = { 0 }; u32 base = plane_config-base; - if (!plane_config-fb) - return; - obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base, plane_config-size); if (!obj) - return; + return false; if (plane_config-tiled) { obj-tiling_mode = I915_TILING_X; - obj-stride = plane_config-fb-base.pitches[0]; + obj-stride = crtc-base.fb-pitches[0]; } - mode_cmd.pixel_format = plane_config-fb-base.pixel_format; - mode_cmd.width = plane_config-fb-base.width; - mode_cmd.height = plane_config-fb-base.height; - mode_cmd.pitches[0] = plane_config-fb-base.pitches[0]; + mode_cmd.pixel_format = crtc-base.fb-pixel_format; + mode_cmd.width = crtc-base.fb-width; + mode_cmd.height = crtc-base.fb-height; + mode_cmd.pitches[0] = crtc-base.fb-pitches[0]; mutex_lock(dev-struct_mutex); - if (intel_framebuffer_init(dev, plane_config-fb, mode_cmd, obj)) { + if (intel_framebuffer_init(dev, to_intel_framebuffer(crtc-base.fb), + mode_cmd, obj)) { DRM_DEBUG_KMS(intel fb init failed\n); goto out_unref_obj; } mutex_unlock(dev-struct_mutex); - DRM_DEBUG_KMS(plane fb obj %p\n, plane_config-fb-obj); - return; + + DRM_DEBUG_KMS(plane fb obj %p\n, obj); + return true; out_unref_obj: drm_gem_object_unreference(obj-base); mutex_unlock(dev-struct_mutex); + return false; +} + +static void intel_find_plane_obj(struct intel_crtc *intel_crtc, +struct intel_plane_config *plane_config) +{ + struct drm_device *dev = intel_crtc-base.dev; + struct drm_crtc *c; + struct intel_crtc *i; + struct intel_framebuffer *fb; + + if (!intel_crtc-base.fb) + return; + + if (intel_alloc_plane_obj(intel_crtc, plane_config)) + return; + + kfree(intel_crtc-base.fb); + + /* +* Failed to alloc the obj, check to see if we should share +* an fb with another CRTC instead +*/ + list_for_each_entry(c, dev-mode_config.crtc_list, head) { + i = to_intel_crtc(c); + + if (c == intel_crtc-base) + continue; + + if (!i-active || !c-fb) + continue; + + fb = to_intel_framebuffer(c-fb); + if (i915_gem_obj_ggtt_offset(fb-obj) == plane_config-base) { + drm_framebuffer_reference(c-fb); + intel_crtc-base.fb = c-fb; + break; + } + } } static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, @@ -5636,8 +5674,8 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc, int fourcc, pixel_format; int aligned_height; - plane_config-fb = kzalloc(sizeof(*plane_config-fb), GFP_KERNEL); - if (!plane_config-fb) { + crtc-base.fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL); + if (!crtc-base.fb) { DRM_DEBUG_KMS(failed to alloc fb\n); return; } @@ -5650,8 +5688,8 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc, pixel_format = val DISPPLANE_PIXFORMAT_MASK; fourcc = intel_format_to_fourcc(pixel_format); - plane_config-fb-base.pixel_format = fourcc; - plane_config-fb
[Intel-gfx] [PATCH 4/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v12
Retrieve current framebuffer config info from the regs and create an fb object for the buffer the BIOS or boot loader left us. This should allow for smooth transitions to userspace apps once we finish the initial configuration construction. v2: check for non-native modes and adjust (Jesse) fixup aperture and cmap frees (Imre) use unlocked unref if init_bios fails (Jesse) fix curly brace around DSPADDR check (Imre) comment failure path for pin_and_fence (Imre) v3: fixup fixup of aperture frees (Chris) v4: update to current bits (locking pin_and_fence hack) (Jesse) v5: move fb config fetch to display code (Jesse) re-order hw state readout on initial load to suit fb inherit (Jesse) re-add pin_and_fence in fbdev code to make sure we refcount properly (Je v6: rename to plane_config (Daniel) check for valid object when initializing BIOS fb (Jesse) split from plane_config readout and other display changes (Jesse) drop use_bios_fb option (Chris) update comments (Jesse) rework fbdev_init_bios for clarity (Jesse) drop fb obj ref under lock (Chris) v7: use fb object from plane_config instead (Ville) take ref on fb object (Jesse) v8: put under i915_fastboot option (Jesse) fix fb ptr checking (Jesse) inform drm_fb_helper if we fail to enable a connector (Jesse) drop unnecessary enabled[] modifications in failure cases (Chris) split from BIOS connector config readout (Daniel) don't memset the fb buffer if preallocated (Chris) alloc ifbdev up front and pass to init_bios (Chris) check for bad ifbdev in restore_mode too (Chris) v9: fix up !fastboot bpp setting (Jesse) fix up !fastboot helper alloc (Jesse) make sure BIOS fb is sufficient for biggest active pipe (Jesse) v10:fix up size calculation for proposed fbs (Chris) go back to two pass pipe fb assignment (Chris) add warning for active pipes w/o fbs (Chris) clean up num_pipes checks in fbdev_init and fbdev_restore_mode (Chris) move i915.fastboot into fbdev_init (Chris) v11:make BIOS connector config usage unconditional (Daniel) v12:fix up fb vs pipe size checking (Chris) Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c | 1 - drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_fbdev.c | 174 +-- 3 files changed, 166 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b933a92..479de3b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2108,7 +2108,6 @@ static void intel_alloc_plane_obj(struct intel_crtc *crtc, out_unref_obj: drm_gem_object_unreference(obj-base); mutex_unlock(dev-struct_mutex); - } static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 95dd15d..3d404ab 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -113,6 +113,7 @@ struct intel_fbdev { struct intel_framebuffer *fb; struct list_head fbdev_list; struct drm_display_mode *our_mode; + int preferred_bpp; }; struct intel_encoder { diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 6b5beed..32a05ed 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -128,6 +128,7 @@ static int intelfb_create(struct drm_fb_helper *helper, struct drm_framebuffer *fb; struct drm_i915_gem_object *obj; int size, ret; + bool prealloc = false; mutex_lock(dev-struct_mutex); @@ -139,6 +140,7 @@ static int intelfb_create(struct drm_fb_helper *helper, intel_fb = ifbdev-fb; } else { DRM_DEBUG_KMS(re-using BIOS fb\n); + prealloc = true; sizes-fb_width = intel_fb-base.width; sizes-fb_height = intel_fb-base.height; } @@ -200,7 +202,7 @@ static int intelfb_create(struct drm_fb_helper *helper, * If the object is stolen however, it will be full of whatever * garbage was left in there. */ - if (ifbdev-fb-obj-stolen) + if (ifbdev-fb-obj-stolen !prealloc) memset_io(info-screen_base, 0, info-screen_size); /* Use default scratch pixmap (info-pixmap.flags = FB_PIXMAP_SYSTEM) */ @@ -454,27 +456,179 @@ static void intel_fbdev_destroy(struct drm_device *dev, drm_framebuffer_remove(ifbdev-fb-base); } +/* + * Build an intel_fbdev struct using a BIOS allocated framebuffer, if possible. + * The core display code will have read out the current plane configuration, + * so we use that to figure out if there's an object for us to use as the + * fb, and if so, we re-use it for the fbdev configuration. + * + * Note we only support a single fb
[Intel-gfx] [PATCH 2/8] drm/i915: get_plane_config for i9xx v13
Read out the current plane configuration at init time into a new plane_config structure. This allows us to track any existing framebuffers attached to the plane and potentially re-use them in our fbdev code for a smooth handoff. v2: update for new pitch_for_width function (Jesse) comment how get_plane_config works with shared fbs (Jesse) v3: s/ARGB/XRGB (Ville) use pipesrc width/height (Ville) fix fourcc comment (Bob) use drm_format_plane_cpp (Ville) v4: use fb for tracking fb data object (Ville) v5: fix up gen2 pitch limits (Ville) v6: read out stride as well (Daniel) v7: split out init ordering changes (Daniel) don't fetch config if !CONFIG_FB v8: use proper height in get_plane_config (Chris) v9: fix CONFIG_FB check for modular configs (Jani) v10: add comment about stolen allocation stomping v11: drop hw state readout hunk (Daniel) v12: handle tiled BIOS fbs (Kristian) pull out common bits (Jesse) v13: move fb obj alloc out to _init Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c | 63 1 file changed, 63 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e793c2a..da7bac5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5627,6 +5627,67 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc, pipe_config-port_clock = clock.dot / 5; } +static void i9xx_get_plane_config(struct intel_crtc *crtc, + struct intel_plane_config *plane_config) +{ + struct drm_device *dev = crtc-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + u32 val, base, offset; + int pipe = crtc-pipe, plane = crtc-plane; + int fourcc, pixel_format; + int aligned_height; + + plane_config-fb = kzalloc(sizeof(*plane_config-fb), GFP_KERNEL); + if (!plane_config-fb) { + DRM_DEBUG_KMS(failed to alloc fb\n); + return; + } + + val = I915_READ(DSPCNTR(plane)); + + if (INTEL_INFO(dev)-gen = 4) + if (val DISPPLANE_TILED) + plane_config-tiled = true; + + pixel_format = val DISPPLANE_PIXFORMAT_MASK; + fourcc = intel_format_to_fourcc(pixel_format); + plane_config-fb-base.pixel_format = fourcc; + plane_config-fb-base.bits_per_pixel = + drm_format_plane_cpp(fourcc, 0) * 8; + + if (INTEL_INFO(dev)-gen = 4) { + if (plane_config-tiled) + offset = I915_READ(DSPTILEOFF(plane)); + else + offset = I915_READ(DSPLINOFF(plane)); + base = I915_READ(DSPSURF(plane)) 0xf000; + } else { + base = I915_READ(DSPADDR(plane)); + } + plane_config-base = base; + + val = I915_READ(PIPESRC(pipe)); + plane_config-fb-base.width = ((val 16) 0xfff) + 1; + plane_config-fb-base.height = ((val 0) 0xfff) + 1; + + val = I915_READ(DSPSTRIDE(pipe)); + plane_config-fb-base.pitches[0] = val 0xff80; + + aligned_height = intel_align_height(dev, plane_config-fb-base.height, + plane_config-tiled); + + plane_config-size = ALIGN(plane_config-fb-base.pitches[0] * + aligned_height, PAGE_SIZE); + + DRM_DEBUG_KMS(pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n, + pipe, plane, plane_config-fb-base.width, + plane_config-fb-base.height, + plane_config-fb-base.bits_per_pixel, base, + plane_config-fb-base.pitches[0], + plane_config-size); + +} + static bool i9xx_get_pipe_config(struct intel_crtc *crtc, struct intel_crtc_config *pipe_config) { @@ -10792,6 +10853,7 @@ static void intel_init_display(struct drm_device *dev) dev_priv-display.update_plane = ironlake_update_plane; } else if (IS_VALLEYVIEW(dev)) { dev_priv-display.get_pipe_config = i9xx_get_pipe_config; + dev_priv-display.get_plane_config = i9xx_get_plane_config; dev_priv-display.crtc_mode_set = i9xx_crtc_mode_set; dev_priv-display.crtc_enable = valleyview_crtc_enable; dev_priv-display.crtc_disable = i9xx_crtc_disable; @@ -10799,6 +10861,7 @@ static void intel_init_display(struct drm_device *dev) dev_priv-display.update_plane = i9xx_update_plane; } else { dev_priv-display.get_pipe_config = i9xx_get_pipe_config; + dev_priv-display.get_plane_config = i9xx_get_plane_config; dev_priv-display.crtc_mode_set = i9xx_crtc_mode_set; dev_priv-display.crtc_enable = i9xx_crtc_enable; dev_priv
[Intel-gfx] [PATCH 7/8] drm/i915: use current mode if the size matches the preferred mode
From: Kristian Høgsberg hoegsb...@gmail.com The BIOS may set a native mode that doesn't quite match the preferred mode timings. It should be ok to use however if it uses the same size, so try to avoid a mode set in that case. Signed-off-by: Kristian Høgsberg hoegsb...@gmail.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/drm_modes.c| 8 drivers/gpu/drm/i915/intel_fbdev.c | 37 ++--- include/drm/drm_crtc.h | 2 ++ 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index b073315..7d2dda4 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -894,6 +894,14 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, } EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo); +bool drm_mode_same_size(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return mode1-vdisplay == mode2-vdisplay + mode1-hdisplay == mode2-hdisplay; +} +EXPORT_SYMBOL(drm_mode_same_size); + /** * drm_mode_validate_size - make sure modes adhere to size constraints * @dev: DRM device diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 32a05ed..950469c 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -369,31 +369,30 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, /* go for command line mode first */ modes[i] = drm_pick_cmdline_mode(fb_conn, width, height); - /* try for preferred next */ + /* try for preferred next or match current */ if (!modes[i]) { + struct drm_display_mode *preferred; + DRM_DEBUG_KMS(looking for preferred mode on connector %d\n, fb_conn-connector-base.id); - modes[i] = drm_has_preferred_mode(fb_conn, width, - height); - } - - /* last resort: use current mode */ - if (!modes[i]) { - /* -* IMPORTANT: We want to use the adjusted mode (i.e. -* after the panel fitter upscaling) as the initial -* config, not the input mode, which is what crtc-mode -* usually contains. But since our current fastboot -* code puts a mode derived from the post-pfit timings -* into crtc-mode this works out correctly. We don't -* use hwmode anywhere right now, so use it for this -* since the fb helper layer wants a pointer to -* something we own. -*/ + preferred = drm_has_preferred_mode(fb_conn, width, + height); intel_mode_from_pipe_config(encoder-crtc-hwmode, to_intel_crtc(encoder-crtc)-config); - modes[i] = encoder-crtc-hwmode; + modes[i] = encoder-crtc-hwmode; + + if (preferred + !drm_mode_same_size(preferred, modes[i])) { + DRM_DEBUG_KMS(using preferred mode %s + instead of current mode %s + on connector %d\n, + preferred-name, + modes[i]-name, + fb_conn-connector-base.id); + modes[i] = preferred; + } } + crtcs[i] = new_crtc; DRM_DEBUG_KMS(connector %s on crtc %d: %s\n, diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index f764654..d5ebe3b 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1020,6 +1020,8 @@ extern bool drm_mode_equal(const struct drm_display_mode *mode1, const struct dr extern bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); extern int drm_mode_width(const struct drm_display_mode *mode); extern int drm_mode_height(const struct drm_display_mode *mode); +extern bool drm_mode_same_size(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2); /* for us by fb module */ extern struct drm_display_mode *drm_mode_create(struct drm_device *dev); -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org
[Intel-gfx] [PATCH 5/8] drm/i915: preserve SSC if previously set
Some machines may have a broken VBT or no VBT at all, but we still want to use SSC there. So check for it and keep it enabled if we see it already on. Based on an earlier fix from Kristian. Reported-by: Kristian Høgsberg hoegsb...@gmail.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 479de3b..d450ab6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5806,6 +5806,10 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) */ val = I915_READ(PCH_DREF_CONTROL); + /* Preserve SSC if the BIOS set it */ + if (val DREF_SSC1_ENABLE) + i915.panel_use_ssc = 1; + /* As we must carefully and slowly disable/enable each source in turn, * compute the final state we want first and check if we need to * make any changes at all. -- 1.8.4.2 ___ 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: make CRTC enable/disable asynchronous
On Thu, 6 Mar 2014 09:35:23 + Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Mar 05, 2014 at 02:48:26PM -0800, Jesse Barnes wrote: @@ -7554,6 +7610,8 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc, goto fail; } + intel_sync_crtc(crtc); + /* we only need to pin inside GTT if cursor is non-phy */ mutex_lock(dev-struct_mutex); if (!INTEL_INFO(dev)-cursor_needs_physical) { @@ -7639,6 +7697,8 @@ static int intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) intel_crtc-cursor_x = clamp_t(int, x, SHRT_MIN, SHRT_MAX); intel_crtc-cursor_y = clamp_t(int, y, SHRT_MIN, SHRT_MAX); + intel_sync_crtc(crtc); + if (intel_crtc-active) intel_crtc_update_cursor(crtc, intel_crtc-cursor_bo != NULL); Hmm. Would be much nicer if touching the cursor didn't incur a delay. And it would seem to quite easy to capture the state change and queue it for when the CRTC is re-enabled. Do you think that's worthwhile? I guess we'll block userspace a bit here, but presumably the cursor won't be visible until the mode set completes anyway... But queuing this stuff is another option. @@ -8682,6 +8742,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, if (work == NULL) return -ENOMEM; + intel_sync_crtc(crtc); + work-event = event; work-crtc = crtc; work-old_fb_obj = to_intel_framebuffer(old_fb)-obj; @@ -9677,8 +9739,10 @@ static int __intel_set_mode(struct drm_crtc *crtc, intel_crtc_disable(intel_crtc-base); for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) { - if (intel_crtc-base.enabled) - dev_priv-display.crtc_disable(intel_crtc-base); + if (intel_crtc-base.enabled) { + intel_queue_crtc_disable(intel_crtc-base); + intel_sync_crtc(intel_crtc-base); + } } /* crtc-mode is already used by the -mode_set callbacks, hence we need @@ -9719,7 +9783,7 @@ static int __intel_set_mode(struct drm_crtc *crtc, /* Now enable the clocks, plane, pipe, and connectors that we set up. */ for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) - dev_priv-display.crtc_enable(intel_crtc-base); + intel_queue_crtc_enable(intel_crtc-base); /* FIXME: add subpixel order */ done: @@ -9740,8 +9804,9 @@ static int intel_set_mode(struct drm_crtc *crtc, ret = __intel_set_mode(crtc, mode, x, y, fb); - if (ret == 0) - intel_modeset_check_state(crtc-dev); + /* FIXME: check after async crtc enable/disable */ +// if (ret == 0) +// intel_modeset_check_state(crtc-dev); return ret; } @@ -10306,6 +10371,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) dev_priv-pipe_to_crtc_mapping[intel_crtc-pipe] = intel_crtc-base; drm_crtc_helper_add(intel_crtc-base, intel_helper_funcs); + + INIT_WORK(intel_crtc-enable_work, intel_crtc_enable_work); + INIT_WORK(intel_crtc-disable_work, intel_crtc_disable_work); I feel using independent work items (remember the global wq is really a pool of many wq) is horribly prone to deadlocks. We have the usual caveat that this has an implicit API change in that setcrtc can now return before the change is complete - and so userspace may write to a still currently visible scanout. Its not a huge issue (and is a change I am in favour of), it is just a change in behaviour we have to be wary of (which also means stating it in the changelog for future reference). Yeah that's a good point, and if we're not careful it could result in some visible ugliness. -- 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 2/6] drm/i915: make fbdev initialization asynchronous
On Thu, 6 Mar 2014 09:12:40 + Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Mar 05, 2014 at 02:48:27PM -0800, Jesse Barnes wrote: This gets us out of our init code and out to userspace quite a bit faster, but does open us up to some bugs given the state of our init time locking. Why are we hand rolling an async task for this? See http://lists.freedesktop.org/archives/intel-gfx/2010-August/007642.html And the locking issue was the main reason why we haven't been able to proceed so far... In looking at the async domains it didn't appear that they would actually save me much if any code in most of these cases. The locking is worrisome, but I added some extra WARNs and things are solid across multiple boots, reloads, and suspend/resume cycles. I haven't tried lockdep though... -- 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 5/6] drm/i915/dp: push eDP caching out into a work queue
On Thu, 6 Mar 2014 11:28:13 +0200 Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Wed, Mar 05, 2014 at 02:48:30PM -0800, Jesse Barnes wrote: It takes awhile to fetch the DPCD and EDID for caching, so take it out of the critical path to improve init time. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_dp.c | 113 +--- 1 file changed, 82 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 738c4e6..763f235 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3001,6 +3001,20 @@ intel_dp_handle_test_request(struct intel_dp *intel_dp) intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK); } +static void intel_flush_edp_cache_work(struct intel_dp *intel_dp) +{ + struct drm_device *dev = intel_dp-attached_connector-base.dev; + + WARN_ON(!mutex_is_locked(dev-mode_config.mutex)); + + if (!is_edp(intel_dp)) + return; + + mutex_unlock(dev-mode_config.mutex); + flush_work(intel_dp-edp_cache_work); + mutex_lock(dev-mode_config.mutex); This feels like deadlock land to me. If we drop mode_config.mutex someone else might grab it and then get blocked on the crtc mutex we're already holding, and then we try to re-grab mode_config.mutex... Yeah I could use unlock_all here instead, or be more careful about dropping the specific crtc mutex we need. I did that on the crtc side of things but obviously missed it here for cases where we'll hold the crtc lock in this path. -- 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] Why Baytrail Gfx driver not always uses pipe A when it's free?
No special reason. It shouldn't matter on BYT either, really, since pipe A doesn't have special characteristics like it does on HSW. Jesse On Thu, 6 Mar 2014 17:19:22 +0800 Lin, Mengdong mengdong@intel.com wrote: Hi Jesse, Could you tell us why Baytrail Gfx driver does not always use pipe A when it's free? We have a Baytrail-M which only have HDMI (port B) and DisplayPort (port C) output. When I connect HDMI or DP only, the Gfx driver always uses pipe B for port B/C, although pipe A is free. This behavior does not change no matter I enable X or not. Is there any special consideration? This is different from what we observe on HSW/BDW, where pipe A is always used if it’s free. Thanks Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Disable full ppgtt by default
On Thu, 6 Mar 2014 12:14:21 +0100 Daniel Vetter daniel.vet...@ffwll.ch wrote: There are too many oustanding issues: - Fence handling in the current code is broken. There's a patch series from me, but it's blocked on and extended review (which includes writing the testcases). - IOMMU mapping handling is broken, we need to properly refcount it - currently it gets destroyed when the first vma is unbound, so way too early. - There's a pending reset issue on snb. Since Mika's reset work and full ppgtt have been pulled in in separate branches and ended up intermittingly breaking each another it's unclear who's the exact culprit here. - We still have persistent evidince of crazy recursion bugs through vma_unbind and ppgtt_relase, e.g. https://bugs.freedesktop.org/show_bug.cgi?id=73383 This issue (and a few others meanwhile resolved) have blocked our performance measuring/tuning group since 3 months. - Secure batch dispatching is broken. This is blocking Brad Volkin's command checker work since 3 months. Do we have bugs and/or tasks filed for all these, or just the one? We need to make sure people are signed up to fix/review them, or it'll end up staying disabled forever, and then we'll be stuck without a command checker and some advanced features coming up... -- 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 v2 01/21] drm/i915: use drm_i915_private everywhere in the power domain api
On Tue, 4 Mar 2014 19:22:50 +0200 Imre Deak imre.d...@intel.com wrote: The power domains framework is internal to the i915 driver, so pass drm_i915_private instead of drm_device to its functions. Also remove a dangling intel_set_power_well() declaration. No functional change. Signed-off-by: Imre Deak imre.d...@intel.com Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_dma.c | 14 drivers/gpu/drm/i915/i915_drv.c | 4 +-- drivers/gpu/drm/i915/i915_drv.h | 4 +-- drivers/gpu/drm/i915/intel_display.c | 27 +++ drivers/gpu/drm/i915/intel_drv.h | 17 +- drivers/gpu/drm/i915/intel_pm.c | 65 ++-- 6 files changed, 58 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 7688abc..8177c17 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1325,7 +1325,7 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_gem_stolen; - intel_power_domains_init_hw(dev); + intel_power_domains_init_hw(dev_priv); /* Important: The output setup functions called by modeset_init need * working irqs for e.g. gmbus and dp aux transfers. */ @@ -1343,7 +1343,7 @@ static int i915_load_modeset_init(struct drm_device *dev) /* FIXME: do pre/post-mode set stuff in core KMS code */ dev-vblank_disable_allowed = true; if (INTEL_INFO(dev)-num_pipes == 0) { - intel_display_power_put(dev, POWER_DOMAIN_VGA); + intel_display_power_put(dev_priv, POWER_DOMAIN_VGA); return 0; } @@ -1381,7 +1381,7 @@ cleanup_gem: WARN_ON(dev_priv-mm.aliasing_ppgtt); drm_mm_takedown(dev_priv-gtt.base.mm); cleanup_power: - intel_display_power_put(dev, POWER_DOMAIN_VGA); + intel_display_power_put(dev_priv, POWER_DOMAIN_VGA); drm_irq_uninstall(dev); cleanup_gem_stolen: i915_gem_cleanup_stolen(dev); @@ -1702,7 +1702,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_gem_unload; } - intel_power_domains_init(dev); + intel_power_domains_init(dev_priv); if (drm_core_check_feature(dev, DRIVER_MODESET)) { ret = i915_load_modeset_init(dev); @@ -1731,7 +1731,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) return 0; out_power_well: - intel_power_domains_remove(dev); + intel_power_domains_remove(dev_priv); drm_vblank_cleanup(dev); out_gem_unload: if (dev_priv-mm.inactive_shrinker.scan_objects) @@ -1781,8 +1781,8 @@ int i915_driver_unload(struct drm_device *dev) /* The i915.ko module is still not prepared to be loaded when * the power well is not enabled, so just enable it in case * we're going to unload/reload. */ - intel_display_set_init_power(dev, true); - intel_power_domains_remove(dev); + intel_display_set_init_power(dev_priv, true); + intel_power_domains_remove(dev_priv); i915_teardown_sysfs(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 08052f3d..ce898af 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -429,7 +429,7 @@ static int i915_drm_freeze(struct drm_device *dev) /* We do a lot of poking in a lot of registers, make sure they work * properly. */ hsw_disable_package_c8(dev_priv); - intel_display_set_init_power(dev, true); + intel_display_set_init_power(dev_priv, true); drm_kms_helper_poll_disable(dev); @@ -551,7 +551,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) mutex_unlock(dev-struct_mutex); } - intel_power_domains_init_hw(dev); + intel_power_domains_init_hw(dev_priv); i915_restore_state(dev); intel_opregion_setup(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 05cfcc1..53b0512 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1024,9 +1024,9 @@ struct i915_power_well { int count; unsigned long domains; void *data; - void (*set)(struct drm_device *dev, struct i915_power_well *power_well, + void (*set)(struct drm_i915_private *dev_priv, struct i915_power_well *power_well, bool enable); - bool (*is_enabled)(struct drm_device *dev, + bool (*is_enabled)(struct drm_i915_private *dev_priv, struct i915_power_well *power_well); }; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6f15627..5cc116b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915
Re: [Intel-gfx] [PATCH v2 04/21] drm/i915: move power domain macros to intel_pm.c
On Tue, 4 Mar 2014 19:22:53 +0200 Imre Deak imre.d...@intel.com wrote: These macros are used only locally, so move them to the .c file. No functional change. v2: - add init power domain to always-on power wells in the following - separate - patch (Paulo) Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 10 -- drivers/gpu/drm/i915/intel_pm.c | 20 ++-- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 53b0512..0794bbd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -121,8 +121,6 @@ enum intel_display_power_domain { POWER_DOMAIN_NUM, }; -#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1) - #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A) #define POWER_DOMAIN_PIPE_PANEL_FITTER(pipe) \ ((pipe) + POWER_DOMAIN_PIPE_A_PANEL_FITTER) @@ -130,14 +128,6 @@ enum intel_display_power_domain { ((tran) == TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \ (tran) + POWER_DOMAIN_TRANSCODER_A) -#define HSW_ALWAYS_ON_POWER_DOMAINS (\ - BIT(POWER_DOMAIN_PIPE_A) | \ - BIT(POWER_DOMAIN_TRANSCODER_EDP)) -#define BDW_ALWAYS_ON_POWER_DOMAINS (\ - BIT(POWER_DOMAIN_PIPE_A) | \ - BIT(POWER_DOMAIN_TRANSCODER_EDP) | \ - BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER)) - enum hpd_pin { HPD_NONE = 0, HPD_PORT_A = HPD_NONE, /* PORT_A is internal */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index db48d55..ebbd0ed 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5384,6 +5384,22 @@ void i915_release_power_well(void) } EXPORT_SYMBOL_GPL(i915_release_power_well); +#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1) + +#define HSW_ALWAYS_ON_POWER_DOMAINS (\ + BIT(POWER_DOMAIN_PIPE_A) | \ + BIT(POWER_DOMAIN_TRANSCODER_EDP)) +#define HSW_DISPLAY_POWER_DOMAINS ( \ + (POWER_DOMAIN_MASK ~HSW_ALWAYS_ON_POWER_DOMAINS) |\ + BIT(POWER_DOMAIN_INIT)) + +#define BDW_ALWAYS_ON_POWER_DOMAINS (\ + HSW_ALWAYS_ON_POWER_DOMAINS | \ + BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER)) +#define BDW_DISPLAY_POWER_DOMAINS ( \ + (POWER_DOMAIN_MASK ~BDW_ALWAYS_ON_POWER_DOMAINS) |\ + BIT(POWER_DOMAIN_INIT)) + static struct i915_power_well i9xx_always_on_power_well[] = { { .name = always-on, @@ -5400,7 +5416,7 @@ static struct i915_power_well hsw_power_wells[] = { }, { .name = display, - .domains = POWER_DOMAIN_MASK ~HSW_ALWAYS_ON_POWER_DOMAINS, + .domains = HSW_DISPLAY_POWER_DOMAINS, .is_enabled = hsw_power_well_enabled, .set = hsw_set_power_well, }, @@ -5414,7 +5430,7 @@ static struct i915_power_well bdw_power_wells[] = { }, { .name = display, - .domains = POWER_DOMAIN_MASK ~BDW_ALWAYS_ON_POWER_DOMAINS, + .domains = BDW_DISPLAY_POWER_DOMAINS, .is_enabled = hsw_power_well_enabled, .set = hsw_set_power_well, }, Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v2 05/21] drm/i915: add init power domain to always-on power wells
On Tue, 4 Mar 2014 19:22:54 +0200 Imre Deak imre.d...@intel.com wrote: Whenever we request a power domain it has to guarantee that all HW resources are enabled that are needed to access a HW register associated with that power domain. In case a register is on an always-on power well this won't result in turning on a power well, but it may require enabling some other HW resource. One such resource is the HSW/BDW device D0 state that is required for all register accesses and thus for all power wells/power domains. So far the init power domain (guaranteeing access to all HW registers) was part of the default i9xx always-on power well, but not the HSW/BDW always-on power wells. Add the domain to the latter power wells too. Atm, all the always-on power wells have noop handlers, so this doesn't change the functionality. v2: - clarify semantics of always-on power wells (Paulo) Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ebbd0ed..9a608f1 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5388,7 +5388,8 @@ EXPORT_SYMBOL_GPL(i915_release_power_well); #define HSW_ALWAYS_ON_POWER_DOMAINS (\ BIT(POWER_DOMAIN_PIPE_A) | \ - BIT(POWER_DOMAIN_TRANSCODER_EDP)) + BIT(POWER_DOMAIN_TRANSCODER_EDP) | \ + BIT(POWER_DOMAIN_INIT)) #define HSW_DISPLAY_POWER_DOMAINS ( \ (POWER_DOMAIN_MASK ~HSW_ALWAYS_ON_POWER_DOMAINS) |\ BIT(POWER_DOMAIN_INIT)) Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v2 07/21] drm/i915: add noop power well handlers instead of NULL checking them
On Tue, 4 Mar 2014 19:22:56 +0200 Imre Deak imre.d...@intel.com wrote: Reading code free of special cases wins over the small overhead of calling a noop handler. Suggested by Jesse. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 29 + 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 7866426..a94f5dd 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5332,6 +5332,17 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv, hsw_enable_package_c8(dev_priv); } +static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv, +struct i915_power_well *power_well) +{ +} + +static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well) +{ + return true; +} + void intel_display_power_get(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain) { @@ -5344,7 +5355,7 @@ void intel_display_power_get(struct drm_i915_private *dev_priv, mutex_lock(power_domains-lock); for_each_power_well(i, power_well, BIT(domain), power_domains) - if (!power_well-count++ power_well-ops-enable) + if (!power_well-count++) power_well-ops-enable(dev_priv, power_well); power_domains-domain_use_count[domain]++; @@ -5369,8 +5380,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, for_each_power_well_rev(i, power_well, BIT(domain), power_domains) { WARN_ON(!power_well-count); - if (!--power_well-count power_well-ops-disable - i915.disable_power_well) + if (!--power_well-count i915.disable_power_well) power_well-ops-disable(dev_priv, power_well); } @@ -5424,7 +5434,12 @@ EXPORT_SYMBOL_GPL(i915_release_power_well); (POWER_DOMAIN_MASK ~BDW_ALWAYS_ON_POWER_DOMAINS) |\ BIT(POWER_DOMAIN_INIT)) -static const struct i915_power_well_ops i9xx_always_on_power_well_ops = { }; +static const struct i915_power_well_ops i9xx_always_on_power_well_ops = { + .sync_hw = i9xx_always_on_power_well_noop, + .enable = i9xx_always_on_power_well_noop, + .disable = i9xx_always_on_power_well_noop, + .is_enabled = i9xx_always_on_power_well_enabled, +}; static struct i915_power_well i9xx_always_on_power_well[] = { { @@ -5510,10 +5525,8 @@ static void intel_power_domains_resume(struct drm_i915_private *dev_priv) int i; mutex_lock(power_domains-lock); - for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) { - if (power_well-ops-sync_hw) - power_well-ops-sync_hw(dev_priv, power_well); - } + for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) + power_well-ops-sync_hw(dev_priv, power_well); mutex_unlock(power_domains-lock); } Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v3 11/21] drm/i915: check pipe power domain when reading its hw state
On Wed, 5 Mar 2014 16:20:55 +0200 Imre Deak imre.d...@intel.com wrote: We can read out the pipe HW state only if the required power domain is on. If not we consider the pipe to be off. v2: - no change v3: - push down the power domain checks into the specific crtc get_pipe_config handlers (Daniel) Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 32407ea..cde6d02 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5611,6 +5611,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc, struct drm_i915_private *dev_priv = dev-dev_private; uint32_t tmp; + if (!intel_display_power_enabled(dev_priv, + POWER_DOMAIN_PIPE(crtc-pipe))) + return false; + pipe_config-cpu_transcoder = (enum transcoder) crtc-pipe; pipe_config-shared_dpll = DPLL_ID_PRIVATE; @@ -6981,6 +6985,10 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, enum intel_display_power_domain pfit_domain; uint32_t tmp; + if (!intel_display_power_enabled(dev_priv, + POWER_DOMAIN_PIPE(crtc-pipe))) + return false; + pipe_config-cpu_transcoder = (enum transcoder) crtc-pipe; pipe_config-shared_dpll = DPLL_ID_PRIVATE; Same goes here, though I suppose there's more room for additional, specific domains down at this level... Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v2 12/21] drm/i915: vlv: keep first level vblank IRQs masked
On Tue, 4 Mar 2014 19:23:01 +0200 Imre Deak imre.d...@intel.com wrote: This is a left-over from commit b7e634cc8dcd320123199a18bae0937b40dc28b8 Author: Imre Deak imre.d...@intel.com Date: Tue Feb 4 21:35:45 2014 +0200 drm/i915: vlv: don't unmask IIR[DISPLAY_PIPE_A/B_VBLANK] interrupt where we stopped unmasking the vblank IRQs, but left them enabled in the IER register. Disable them in IER too. v2: - remove comment becoming stale after this change (Ville) Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 331f89c..471d8f9 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3026,17 +3026,9 @@ static int valleyview_irq_postinstall(struct drm_device *dev) enable_mask = I915_DISPLAY_PORT_INTERRUPT; enable_mask |= I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | - I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT | - I915_DISPLAY_PIPE_B_EVENT_INTERRUPT | - I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT; + I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; - /* - *Leave vblank interrupts masked initially. enable/disable will - * toggle them based on usage. - */ - dev_priv-irq_mask = (~enable_mask) | - I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT | - I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT; + dev_priv-irq_mask = ~enable_mask; I915_WRITE(PORT_HOTPLUG_EN, 0); POSTING_READ(PORT_HOTPLUG_EN); Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v2 14/21] drm/i915: factor out reset_vblank_counter
On Tue, 4 Mar 2014 19:23:03 +0200 Imre Deak imre.d...@intel.com wrote: We need to do the same for other platforms in upcoming patches. v2: - s/p/pipe (Ville) - Call the new helper with the vbl_lock already held. The part it protects is short, so releasing it between pipes only makes proving correctness more difficult. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 78e8989..28fae53 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5249,11 +5249,18 @@ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv) } } +static void reset_vblank_counter(struct drm_device *dev, enum pipe pipe) +{ + assert_spin_locked(dev-vbl_lock); + + dev-vblank[pipe].last = 0; +} + static void hsw_power_well_post_disable(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; - enum pipe p; unsigned long irqflags; + enum pipe pipe; /* * After this, the registers on the pipes that are part of the power @@ -5263,9 +5270,9 @@ static void hsw_power_well_post_disable(struct drm_i915_private *dev_priv) * FIXME: Should we do this in general in drm_vblank_post_modeset? */ spin_lock_irqsave(dev-vbl_lock, irqflags); - for_each_pipe(p) - if (p != PIPE_A) - dev-vblank[p].last = 0; + for_each_pipe(pipe) + if (pipe != PIPE_A) + reset_vblank_counter(dev, pipe); spin_unlock_irqrestore(dev-vbl_lock, irqflags); } Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v2 17/21] drm/i915: sanity check power well sw state against hw state
On Tue, 4 Mar 2014 19:23:06 +0200 Imre Deak imre.d...@intel.com wrote: Suggested by Daniel. v2: - sanitize the state checking condition, the original was rather confusing (partly due to the unfortunate naming of i915.disable_power_well) (Ville) - simpler message+backtrace generation by using WARN instead of WARN_ON (Ville) - check if always-on power wells are truly on all the time Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 38 +++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 28fae53..c034842 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5350,6 +5350,29 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv, return true; } +static void check_power_well_state(struct drm_i915_private *dev_priv, +struct i915_power_well *power_well) +{ + bool enabled = power_well-ops-is_enabled(dev_priv, power_well); + + if (power_well-always_on || !i915.disable_power_well) { + if (!enabled) + goto mismatch; + + return; + } + + if (enabled != (power_well-count 0)) + goto mismatch; + + return; + +mismatch: + WARN(1, state mismatch for '%s' (always_on %d hw state %d use-count %d disable_power_well %d\n, + power_well-name, power_well-always_on, enabled, + power_well-count, i915.disable_power_well); +} + void intel_display_power_get(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain) { @@ -5361,9 +5384,14 @@ void intel_display_power_get(struct drm_i915_private *dev_priv, mutex_lock(power_domains-lock); - for_each_power_well(i, power_well, BIT(domain), power_domains) - if (!power_well-count++) + for_each_power_well(i, power_well, BIT(domain), power_domains) { + if (!power_well-count++) { + DRM_DEBUG_KMS(enabling %s\n, power_well-name); power_well-ops-enable(dev_priv, power_well); + } + + check_power_well_state(dev_priv, power_well); + } power_domains-domain_use_count[domain]++; @@ -5387,8 +5415,12 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, for_each_power_well_rev(i, power_well, BIT(domain), power_domains) { WARN_ON(!power_well-count); - if (!--power_well-count i915.disable_power_well) + if (!--power_well-count i915.disable_power_well) { + DRM_DEBUG_KMS(disabling %s\n, power_well-name); power_well-ops-disable(dev_priv, power_well); + } + + check_power_well_state(dev_priv, power_well); } mutex_unlock(power_domains-lock); Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v3 09/21] drm/i915: get port power domain in connector detect handlers
); + struct intel_encoder *intel_encoder = intel_attached_encoder(connector); + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(intel_encoder-base); struct drm_i915_private *dev_priv = connector-dev-dev_private; + enum intel_display_power_domain power_domain; struct edid *edid; bool has_audio = false; + power_domain = intel_display_port_power_domain(intel_encoder); + intel_display_power_get(dev_priv, power_domain); + edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, intel_hdmi-ddc_bus)); @@ -975,6 +996,8 @@ intel_hdmi_detect_audio(struct drm_connector *connector) kfree(edid); } + intel_display_power_put(dev_priv, power_domain); + return has_audio; } Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v3 10/21] drm/i915: check port power domain when reading the encoder hw state
and making things fairly opaque, I'm not sure Daniel's suggestion buys us anything. So to me the wrapper seemed nicer... but either way works I guess. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v2 18/21] drm/i915: vlv: factor out valleyview_display_irq_install
On Tue, 4 Mar 2014 19:23:07 +0200 Imre Deak imre.d...@intel.com wrote: +void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv) +{ + assert_spin_locked(dev_priv-irq_lock); + + if (dev_priv-display_irqs_enabled) + return; + + dev_priv-display_irqs_enabled = true; + + if (dev_priv-dev-irq_enabled) + valleyview_display_irqs_install(dev_priv); +} This made me do a double take, then I saw you were checking the actual drm_device irq enabled state rather than checking the new field again... Looks good. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v2 19/21] drm/i915: move hsw power domain comment to its right place
On Tue, 4 Mar 2014 19:23:08 +0200 Imre Deak imre.d...@intel.com wrote: Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c034842..39acffd 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5213,6 +5213,12 @@ bool intel_display_power_enabled(struct drm_i915_private *dev_priv, return is_enabled; } +/* + * Starting with Haswell, we have a Power Down Well that can be turned off + * when not needed anymore. We have 4 registers that can request the power well + * to be enabled, and it will only be disabled if none of the registers is + * requesting it to be enabled. + */ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; @@ -5578,12 +5584,6 @@ static void intel_power_domains_resume(struct drm_i915_private *dev_priv) mutex_unlock(power_domains-lock); } -/* - * Starting with Haswell, we have a Power Down Well that can be turned off - * when not needed anymore. We have 4 registers that can request the power well - * to be enabled, and it will only be disabled if none of the registers is - * requesting it to be enabled. - */ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv) { /* For now, we need the power well to be always enabled. */ Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v2 20/21] drm/i915: factor out intel_set_cpu_fifo_underrun_reporting_nolock
On Tue, 4 Mar 2014 19:23:09 +0200 Imre Deak imre.d...@intel.com wrote: Needed by the next patch, wanting to set the underrun reporting as part of a bigger dev_priv-irq_lock'ed sequence. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9d9309c..bc94cfb 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -387,17 +387,14 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, * * Returns the previous state of underrun reporting. */ -bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev, -enum pipe pipe, bool enable) +bool intel_set_cpu_fifo_underrun_reporting_nolock(struct drm_device *dev, + enum pipe pipe, bool enable) { struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc = dev_priv-pipe_to_crtc_mapping[pipe]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - unsigned long flags; bool ret; - spin_lock_irqsave(dev_priv-irq_lock, flags); - ret = !intel_crtc-cpu_fifo_underrun_disabled; if (enable == ret) @@ -415,7 +412,20 @@ bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev, broadwell_set_fifo_underrun_reporting(dev, pipe, enable); done: + return ret; +} + +bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev, +enum pipe pipe, bool enable) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + unsigned long flags; + bool ret; + + spin_lock_irqsave(dev_priv-irq_lock, flags); + ret = intel_set_cpu_fifo_underrun_reporting_nolock(dev, pipe, enable); spin_unlock_irqrestore(dev_priv-irq_lock, flags); + return ret; } Funky how diff left the spin_unlock line alone. :) Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 v2 21/21] drm/i915: power domains: add vlv power wells
struct i915_power_well_ops vlv_dpio_power_well_ops = { + .sync_hw = vlv_power_well_sync_hw, + .enable = vlv_power_well_enable, + .disable = vlv_power_well_disable, + .is_enabled = vlv_power_well_enabled, +}; + +static struct i915_power_well vlv_power_wells[] = { + { + .name = always-on, + .always_on = 1, + .domains = VLV_ALWAYS_ON_POWER_DOMAINS, + .ops = i9xx_always_on_power_well_ops, + }, + { + .name = display, + .domains = VLV_DISPLAY_POWER_DOMAINS, + .data = PUNIT_POWER_WELL_DISP2D, + .ops = vlv_display_power_well_ops, + }, + { + .name = dpio-common, + .domains = VLV_DPIO_CMN_BC_POWER_DOMAINS, + .data = PUNIT_POWER_WELL_DPIO_CMN_BC, + .ops = vlv_dpio_power_well_ops, + }, + { + .name = dpio-tx-b-01, + .domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS | +VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS | +VLV_DPIO_TX_C_LANES_01_POWER_DOMAINS | +VLV_DPIO_TX_C_LANES_23_POWER_DOMAINS, + .ops = vlv_dpio_power_well_ops, + .data = PUNIT_POWER_WELL_DPIO_TX_B_LANES_01, + }, + { + .name = dpio-tx-b-23, + .domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS | +VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS | +VLV_DPIO_TX_C_LANES_01_POWER_DOMAINS | +VLV_DPIO_TX_C_LANES_23_POWER_DOMAINS, + .ops = vlv_dpio_power_well_ops, + .data = PUNIT_POWER_WELL_DPIO_TX_B_LANES_23, + }, + { + .name = dpio-tx-c-01, + .domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS | +VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS | +VLV_DPIO_TX_C_LANES_01_POWER_DOMAINS | +VLV_DPIO_TX_C_LANES_23_POWER_DOMAINS, + .ops = vlv_dpio_power_well_ops, + .data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_01, + }, + { + .name = dpio-tx-c-23, + .domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS | +VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS | +VLV_DPIO_TX_C_LANES_01_POWER_DOMAINS | +VLV_DPIO_TX_C_LANES_23_POWER_DOMAINS, + .ops = vlv_dpio_power_well_ops, + .data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_23, + }, +}; + #define set_power_wells(power_domains, __power_wells) ({ \ (power_domains)-power_wells = (__power_wells); \ (power_domains)-power_well_count = ARRAY_SIZE(__power_wells); \ @@ -5560,6 +5795,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv) } else if (IS_BROADWELL(dev_priv-dev)) { set_power_wells(power_domains, bdw_power_wells); hsw_pwr = power_domains; + } else if (IS_VALLEYVIEW(dev_priv-dev)) { + set_power_wells(power_domains, vlv_power_wells); } else { set_power_wells(power_domains, i9xx_always_on_power_well); } Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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: print connector mode list in display_info
On Wed, 5 Mar 2014 13:55:00 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Thu, Feb 20, 2014 at 08:50:59PM +, Chris Wilson wrote: On Thu, Feb 20, 2014 at 12:39:57PM -0800, Jesse Barnes wrote: Useful for bug reports. Hey, this would be useful for error state as well :) I seem to have pissed of Jesse yesterday, so not going to insist here ;-) Queued for -next, thanks for the patch. Move along, nothing to see here. :) But yeah, adding this to error state might be useful just so things get bundled up I guess? It's a bunch more refactoring to get the printouts into a buffer for use in either case though. Not sure it's worth it since we can just ask for both files. -- 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 2/2] drm/i915: ignore bios output config if not all outputs are on
On Tue, 4 Mar 2014 22:08:12 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Tue, Mar 04, 2014 at 12:33:01PM -0800, Jesse Barnes wrote: On Tue, 4 Mar 2014 21:08:42 +0100 Daniel Vetter daniel.vet...@ffwll.ch wrote: Both Ville and QA rather immediately complained that with the new initial_config logic from Jesse not all outputs get enabled. Since the fbdev emulation pretty much tries to always enable as many outputs as possible (it even has hotplug handling and all that) fall back if more outputs could have been enabled. v2: Fix up my confusion about what enabled means - it's passed from the fbdev helper, we need to check for a non-zero connector-encoder link. Spotted by Ville. Cc: Jesse Barnes jbar...@virtuousgeek.org Cc: Ville Syrjälä ville.syrj...@linux.intel.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75552 Tested-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_fbdev.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index df00e6b01f0d..c1a20c3babde 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -290,6 +290,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, int i, j; bool *save_enabled; bool fallback = true; + int num_connectors_enabled = 0; + int num_connectors_detected = 0; /* * If the user specified any force options, just bail here @@ -324,6 +326,10 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, fb_conn = fb_helper-connector_info[i]; connector = fb_conn-connector; + + if (connector-status == connector_status_connected) + num_connectors_detected++; + if (!enabled[i]) { DRM_DEBUG_KMS(connector %d not enabled, skipping\n, connector-base.id); @@ -338,6 +344,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, continue; } + num_connectors_enabled++; + new_crtc = intel_fb_helper_crtc(fb_helper, encoder-crtc); /* @@ -393,6 +401,15 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, fallback = false; } + /* + * If the BIOS didn't enable everything it could, fall back to have the + * same user experiencing of lighting up as much as possible like the + * fbdev helper library. + */ + if (num_connectors_enabled != num_connectors_detected + num_connectors_enabled INTEL_INFO(dev)-num_pipes) + fallback = true; I think we need a debug message in here so people can figure out why their fastboot failed with this patch included. E.g. some connected outputs weren't enabled, falling back to old behavior. Also note that this will probably always happen in certain configs, and the fallback behavior won't be any better since we may not be able to light up everything that's attached. Excellent suggestion, I've gone ahead and added debug output for all cases where we fall back. With those caveats: Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Thinking about this some more last night, I think it would be better to count the pipes and the connectors, and bail out if we have detected connectors available but not enabled and some free pipes. That would prevent unnecessary fastboot breakage I think. -- 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 2/2] drm/i915: ignore bios output config if not all outputs are on
On Wed, 5 Mar 2014 19:34:45 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Wed, Mar 05, 2014 at 08:27:08AM -0800, Jesse Barnes wrote: On Tue, 4 Mar 2014 22:08:12 +0100 Daniel Vetter dan...@ffwll.ch wrote: On Tue, Mar 04, 2014 at 12:33:01PM -0800, Jesse Barnes wrote: On Tue, 4 Mar 2014 21:08:42 +0100 Daniel Vetter daniel.vet...@ffwll.ch wrote: Both Ville and QA rather immediately complained that with the new initial_config logic from Jesse not all outputs get enabled. Since the fbdev emulation pretty much tries to always enable as many outputs as possible (it even has hotplug handling and all that) fall back if more outputs could have been enabled. v2: Fix up my confusion about what enabled means - it's passed from the fbdev helper, we need to check for a non-zero connector-encoder link. Spotted by Ville. Cc: Jesse Barnes jbar...@virtuousgeek.org Cc: Ville Syrjälä ville.syrj...@linux.intel.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75552 Tested-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_fbdev.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index df00e6b01f0d..c1a20c3babde 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -290,6 +290,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, int i, j; bool *save_enabled; bool fallback = true; + int num_connectors_enabled = 0; + int num_connectors_detected = 0; /* * If the user specified any force options, just bail here @@ -324,6 +326,10 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, fb_conn = fb_helper-connector_info[i]; connector = fb_conn-connector; + + if (connector-status == connector_status_connected) + num_connectors_detected++; + if (!enabled[i]) { DRM_DEBUG_KMS(connector %d not enabled, skipping\n, connector-base.id); @@ -338,6 +344,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, continue; } + num_connectors_enabled++; + new_crtc = intel_fb_helper_crtc(fb_helper, encoder-crtc); /* @@ -393,6 +401,15 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, fallback = false; } + /* + * If the BIOS didn't enable everything it could, fall back to have the + * same user experiencing of lighting up as much as possible like the + * fbdev helper library. + */ + if (num_connectors_enabled != num_connectors_detected + num_connectors_enabled INTEL_INFO(dev)-num_pipes) + fallback = true; I think we need a debug message in here so people can figure out why their fastboot failed with this patch included. E.g. some connected outputs weren't enabled, falling back to old behavior. Also note that this will probably always happen in certain configs, and the fallback behavior won't be any better since we may not be able to light up everything that's attached. Excellent suggestion, I've gone ahead and added debug output for all cases where we fall back. With those caveats: Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Thinking about this some more last night, I think it would be better to count the pipes and the connectors, and bail out if we have detected connectors available but not enabled and some free pipes. That would prevent unnecessary fastboot breakage I think. I do take pipes into account and only bail out if we'd have a free one. I don't see what more we could do (beside trying to keep the crtcs for the already enabled connectors)? Ah ok I missed the check against num_pipes... yeah looks like it should be fine. Thanks, -- Jesse Barnes, 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 4/6] drm: take modeset locks around initial fb helper probing
Drivers ought to complain otherwise. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/drm_fb_helper.c | 2 ++ drivers/gpu/drm/i915/intel_dp.c | 4 drivers/gpu/drm/i915/intel_drv.h | 3 +++ 3 files changed, 9 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index ca100d6..b946217 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1533,9 +1533,11 @@ bool drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel) drm_fb_helper_parse_command_line(fb_helper); + drm_modeset_lock_all(dev); count = drm_fb_helper_probe_connector_modes(fb_helper, dev-mode_config.max_width, dev-mode_config.max_height); + drm_modeset_unlock_all(dev); /* * we shouldn't end up with no modes here. */ diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0d5a311..738c4e6 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2880,6 +2880,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) char dpcd_hex_dump[sizeof(intel_dp-dpcd) * 3]; + /* We cache the DPCD for eDP panels */ + if (intel_dp-dpcd_valid) + return true; + if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp-dpcd, sizeof(intel_dp-dpcd)) == 0) return false; /* aux transfer failed */ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a01fcf0..9ee412d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -503,8 +503,11 @@ struct intel_dp { unsigned long last_backlight_off; bool psr_setup_done; bool use_tps3; + bool dpcd_valid; /* for eDP DPCD caching */ struct intel_connector *attached_connector; + struct work_struct edp_cache_work; struct edp_power_seq power_seq; + const char *i2c_name; uint32_t (*get_aux_clock_divider)(struct intel_dp *dp, int index); /* -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915: make CRTC enable/disable asynchronous
This lets us return to userspace more quickly and should improve init and suspend/resume times as well, allowing us to return to userspace sooner. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 4 +- drivers/gpu/drm/i915/intel_display.c | 106 --- drivers/gpu/drm/i915/intel_drv.h | 4 ++ 4 files changed, 94 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 08052f3d..29cc079 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -456,7 +456,7 @@ static int i915_drm_freeze(struct drm_device *dev) */ mutex_lock(dev-mode_config.mutex); list_for_each_entry(crtc, dev-mode_config.crtc_list, head) - dev_priv-display.crtc_disable(crtc); + dev_priv-display._crtc_disable(crtc); mutex_unlock(dev-mode_config.mutex); intel_modeset_suspend_hw(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 42f83f2..4c39bb5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -438,8 +438,8 @@ struct drm_i915_display_funcs { int (*crtc_mode_set)(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb); - void (*crtc_enable)(struct drm_crtc *crtc); - void (*crtc_disable)(struct drm_crtc *crtc); + void (*_crtc_enable)(struct drm_crtc *crtc); + void (*_crtc_disable)(struct drm_crtc *crtc); void (*off)(struct drm_crtc *crtc); void (*write_eld)(struct drm_connector *connector, struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 46ce940..c066a7d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1747,6 +1747,63 @@ static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv) I915_WRITE(_TRANSA_CHICKEN2, val); } +static void intel_sync_crtc(struct drm_crtc *crtc) +{ + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_device *dev = crtc-dev; + + WARN(!mutex_is_locked(intel_crtc-base.mutex), need crtc mutex\n); + + mutex_unlock(dev-mode_config.mutex); + mutex_unlock(intel_crtc-base.mutex); + flush_work(intel_crtc-disable_work); + flush_work(intel_crtc-enable_work); + mutex_lock(intel_crtc-base.mutex); + mutex_lock(dev-mode_config.mutex); +} + +static void intel_crtc_disable_work(struct work_struct *work) +{ + struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc, +disable_work); + struct drm_i915_private *dev_priv = intel_crtc-base.dev-dev_private; + struct drm_device *dev = dev_priv-dev; + + mutex_lock(dev-mode_config.mutex); + mutex_lock(intel_crtc-base.mutex); + dev_priv-display._crtc_disable(intel_crtc-base); + mutex_unlock(intel_crtc-base.mutex); + mutex_unlock(dev-mode_config.mutex); +} + +void intel_queue_crtc_disable(struct drm_crtc *crtc) +{ + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + schedule_work(intel_crtc-disable_work); +} + +static void intel_crtc_enable_work(struct work_struct *work) +{ + struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc, +enable_work); + struct drm_i915_private *dev_priv = intel_crtc-base.dev-dev_private; + struct drm_device *dev = dev_priv-dev; + + mutex_lock(dev-mode_config.mutex); + mutex_lock(intel_crtc-base.mutex); + dev_priv-display._crtc_enable(intel_crtc-base); + mutex_unlock(intel_crtc-base.mutex); + mutex_unlock(dev-mode_config.mutex); +} + +static void intel_queue_crtc_enable(struct drm_crtc *crtc) +{ + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + schedule_work(intel_crtc-enable_work); +} + /** * intel_enable_pipe - enable a pipe, asserting requirements * @crtc: crtc responsible for the pipe @@ -4309,7 +4366,6 @@ static void intel_crtc_update_sarea(struct drm_crtc *crtc, void intel_crtc_update_dpms(struct drm_crtc *crtc) { struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; struct intel_encoder *intel_encoder; bool enable = false; @@ -4317,9 +4373,9 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc) enable |= intel_encoder-connectors_active; if (enable) - dev_priv-display.crtc_enable(crtc); + intel_queue_crtc_enable(crtc); else - dev_priv-display.crtc_disable(crtc
[Intel-gfx] [PATCH 6/6] drm/i915/dp: make sure VDD is on around link status checking
In the hotplug case, nothing was grabbing VDD, leading to some warnings. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_dp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 763f235..78c883e 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3037,10 +3037,13 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) if (WARN_ON(!intel_encoder-base.crtc)) return; + edp_panel_vdd_on(intel_dp); /* Try to read receiver status if the link appears to be up */ if (!intel_dp_get_link_status(intel_dp, link_status)) { + edp_panel_vdd_off(intel_dp, false); return; } + edp_panel_vdd_off(intel_dp, false); intel_flush_edp_cache_work(intel_dp); -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Make init and mode set more asynchronous
I'm worried about the locking in this... I've also commented out the state checker, but that can be re-added as a check after any queued CRTC changes as another queued item, so should be easy to fix. This set drastically improves the init time of the i915 module (based on initcall_debug timing), and should allow suspend and resume to speed up significantly too, but I'm still working on that. Open items on the suspend/resume path include: - synchronizing CRTC state against DP up/down (right now I synchronize which could be put off) - speeding up our DP hot plug on resume, it currently takes forever mostly due to VDD toggling. I think Paulo's stuff should allow us to make this faster, but we still have some big delays here - get rid of the CRTC sync in the mode_set prepare_pipes; the sync just needs to happen before the enable CRTCs are actually enabled... so maybe queuing things in order is sufficient Anyway I'd appreciate some eyes and feedback on this. Thanks, Jesse ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915: make fbdev initialization asynchronous
This gets us out of our init code and out to userspace quite a bit faster, but does open us up to some bugs given the state of our init time locking. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_dma.c| 3 ++- drivers/gpu/drm/i915/i915_drv.h| 1 + drivers/gpu/drm/i915/intel_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_fbdev.c | 6 -- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 7688abc..7c0a61a 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1364,7 +1364,8 @@ static int i915_load_modeset_init(struct drm_device *dev) * scanning against hotplug events. Hence do this first and ignore the * tiny window where we will loose hotplug notifactions. */ - intel_fbdev_initial_config(dev); + INIT_WORK(dev_priv-fbdev_work, intel_fbdev_initial_config); + schedule_work(dev_priv-fbdev_work); /* Only enable hotplug handling once the fbdev is fully set up. */ dev_priv-enable_hotplug_processing = true; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4c39bb5..9024809 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1551,6 +1551,7 @@ typedef struct drm_i915_private { /* list of fbdev register on this device */ struct intel_fbdev *fbdev; #endif + struct work_struct fbdev_work; /* * The console may be contended at resume, but we don't diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index b90f1f5..5eeca0f 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -775,7 +775,7 @@ void intel_dvo_init(struct drm_device *dev); /* legacy fbdev emulation in intel_fbdev.c */ #ifdef CONFIG_DRM_I915_FBDEV extern int intel_fbdev_init(struct drm_device *dev); -extern void intel_fbdev_initial_config(struct drm_device *dev); +extern void intel_fbdev_initial_config(struct work_struct *work); extern void intel_fbdev_fini(struct drm_device *dev); extern void intel_fbdev_set_suspend(struct drm_device *dev, int state); extern void intel_fbdev_output_poll_changed(struct drm_device *dev); @@ -786,7 +786,7 @@ static inline int intel_fbdev_init(struct drm_device *dev) return 0; } -static inline void intel_fbdev_initial_config(struct drm_device *dev) +static inline void intel_fbdev_initial_config(struct work_struct *work) { } diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 4e4b461..4418c8a 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -457,9 +457,10 @@ int intel_fbdev_init(struct drm_device *dev) return 0; } -void intel_fbdev_initial_config(struct drm_device *dev) +void intel_fbdev_initial_config(struct work_struct *work) { - struct drm_i915_private *dev_priv = dev-dev_private; + struct drm_i915_private *dev_priv = + container_of(work, struct drm_i915_private, fbdev_work); /* Due to peculiar init order wrt to hpd handling this is separate. */ drm_fb_helper_initial_config(dev_priv-fbdev-helper, 32); @@ -471,6 +472,7 @@ void intel_fbdev_fini(struct drm_device *dev) if (!dev_priv-fbdev) return; + cancel_work_sync(dev_priv-fbdev_work); intel_fbdev_destroy(dev, dev_priv-fbdev); kfree(dev_priv-fbdev); dev_priv-fbdev = NULL; -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915/dp: put power sequence info into intel_dp
Reduces params in a few places and makes workqueueing the eDP caching work easier. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_dp.c | 23 +-- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c512d78..0d5a311 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -246,12 +246,10 @@ intel_hrawclk(struct drm_device *dev) static void intel_dp_init_panel_power_sequencer(struct drm_device *dev, - struct intel_dp *intel_dp, - struct edp_power_seq *out); + struct intel_dp *intel_dp); static void intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, - struct intel_dp *intel_dp, - struct edp_power_seq *out); + struct intel_dp *intel_dp); static enum pipe vlv_power_sequencer_pipe(struct intel_dp *intel_dp) @@ -1942,7 +1940,6 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder) struct intel_crtc *intel_crtc = to_intel_crtc(encoder-base.crtc); enum dpio_channel port = vlv_dport_to_channel(dport); int pipe = intel_crtc-pipe; - struct edp_power_seq power_seq; u32 val; mutex_lock(dev_priv-dpio_lock); @@ -1962,9 +1959,8 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder) if (is_edp(intel_dp)) { /* init power sequencer on this pipe and port */ - intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, - power_seq); + intel_dp_init_panel_power_sequencer(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); } intel_enable_dp(encoder); @@ -3529,11 +3525,11 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) static void intel_dp_init_panel_power_sequencer(struct drm_device *dev, - struct intel_dp *intel_dp, - struct edp_power_seq *out) + struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dev-dev_private; struct edp_power_seq cur, vbt, spec, final; + struct edp_power_seq *out = intel_dp-power_seq; u32 pp_on, pp_off, pp_div, pp; int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg; @@ -3629,10 +3625,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, static void intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, - struct intel_dp *intel_dp, - struct edp_power_seq *seq) + struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dev-dev_private; + struct edp_power_seq *seq = intel_dp-power_seq; u32 pp_on, pp_off, pp_div, port_sel = 0; int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev); int pp_on_reg, pp_off_reg, pp_div_reg; @@ -3726,7 +3722,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, } /* We now know it's not a ghost, init power sequence regs. */ - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, power_seq); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); edid = drm_get_edid(connector, intel_dp-adapter); if (edid) { @@ -3775,7 +3771,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, struct drm_device *dev = intel_encoder-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; enum port port = intel_dig_port-port; - struct edp_power_seq power_seq = { 0 }; const char *name = NULL; int type, error; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 5eeca0f..a01fcf0 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -504,6 +504,7 @@ struct intel_dp { bool psr_setup_done; bool use_tps3; struct intel_connector *attached_connector; + struct edp_power_seq power_seq; uint32_t (*get_aux_clock_divider)(struct intel_dp *dp, int index); /* -- 1.8.4.2 ___ 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: make CRTC enable/disable asynchronous
On Thu, 06 Mar 2014 01:29:14 +0200 Imre Deak imre.d...@intel.com wrote: On Wed, 2014-03-05 at 14:48 -0800, Jesse Barnes wrote: This lets us return to userspace more quickly and should improve init and suspend/resume times as well, allowing us to return to userspace sooner. IMHO this is a good move towards a full command queue based solution for kms commands, where eventually we have to think less of concurrency. That is if we can queue all the other kms commands too (flip, set_plane). But I don't see why that wouldn't be possible. Btw, why do you have a separate disable and enable queue? As opposed to a dedicated work queue for both combined? I had a separate queue in an earlier patch, but dropped it while debugging some other stuff. We should bring it back to ensure ordering. That would remove the need for a few of the syncs, and would also let us queue a check at the appropriate time on the same queue. -- 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: Reject changes of fb base when we have a flip pending
On Tue, 4 Mar 2014 13:15:08 + Chris Wilson ch...@chris-wilson.co.uk wrote: This should be impossible due to the wait for outstanding flips that the caller is meant to perform prior to updating the scanout base. Paranoia tells me to check anyway. References: https://bugs.freedesktop.org/show_bug.cgi?id=75502 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_display.c | 43 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 25c486d5fb6a..6dc93bd6594f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2349,6 +2349,25 @@ intel_finish_fb(struct drm_framebuffer *old_fb) return ret; } +static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc-dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + unsigned long flags; + bool pending; + + if (i915_reset_in_progress(dev_priv-gpu_error) || + intel_crtc-reset_counter != atomic_read(dev_priv-gpu_error.reset_counter)) + return false; + + spin_lock_irqsave(dev-event_lock, flags); + pending = to_intel_crtc(crtc)-unpin_work != NULL; + spin_unlock_irqrestore(dev-event_lock, flags); + + return pending; +} + static int intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *fb) @@ -2359,6 +2378,11 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb; int ret; + if (intel_crtc_has_pending_flip(crtc)) { + DRM_ERROR(pipe is still busy with an old pageflip\n); + return -EBUSY; + } + /* no fb bound */ if (!fb) { DRM_ERROR(No FB bound\n); @@ -2984,25 +3008,6 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc) udelay(100); } -static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) -{ - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - unsigned long flags; - bool pending; - - if (i915_reset_in_progress(dev_priv-gpu_error) || - intel_crtc-reset_counter != atomic_read(dev_priv-gpu_error.reset_counter)) - return false; - - spin_lock_irqsave(dev-event_lock, flags); - pending = to_intel_crtc(crtc)-unpin_work != NULL; - spin_unlock_irqrestore(dev-event_lock, flags); - - return pending; -} - bool intel_has_pending_fb_unpin(struct drm_device *dev) { struct intel_crtc *crtc; Looks fine, my only comment is do we want this to be a DRM_ERROR? It would be easy for userspace to trigger this by queueing a flip on a busy ring, then doing a mode set that ends up doing just a pipe base update, right? Otherwise, Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 1/2] drm/i915: s/any_enabled/!fallback/ in fbdev_initial_config
On Tue, 4 Mar 2014 21:08:41 +0100 Daniel Vetter daniel.vet...@ffwll.ch wrote: It started as a simple check whether anything is lit up, but now is't used to driver the general fallback logic to the default output configuration selector in the helper library. So rename it for more clarity. Cc: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_fbdev.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 4e4b461e0a70..df00e6b01f0d 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -289,7 +289,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, struct drm_device *dev = fb_helper-dev; int i, j; bool *save_enabled; - bool any_enabled = false; + bool fallback = true; /* * If the user specified any force options, just bail here @@ -347,7 +347,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, */ for (j = 0; j fb_helper-connector_count; j++) { if (crtcs[j] == new_crtc) { - any_enabled = false; + fallback = true; goto out; } } @@ -390,11 +390,11 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, encoder-crtc-base.id, modes[i]-name); - any_enabled = true; + fallback = false; } out: - if (!any_enabled) { + if (fallback) { memcpy(enabled, save_enabled, dev-mode_config.num_connector); kfree(save_enabled); return false; Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 2/2] drm/i915: ignore bios output config if not all outputs are on
On Tue, 4 Mar 2014 21:08:42 +0100 Daniel Vetter daniel.vet...@ffwll.ch wrote: Both Ville and QA rather immediately complained that with the new initial_config logic from Jesse not all outputs get enabled. Since the fbdev emulation pretty much tries to always enable as many outputs as possible (it even has hotplug handling and all that) fall back if more outputs could have been enabled. v2: Fix up my confusion about what enabled means - it's passed from the fbdev helper, we need to check for a non-zero connector-encoder link. Spotted by Ville. Cc: Jesse Barnes jbar...@virtuousgeek.org Cc: Ville Syrjälä ville.syrj...@linux.intel.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75552 Tested-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_fbdev.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index df00e6b01f0d..c1a20c3babde 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -290,6 +290,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, int i, j; bool *save_enabled; bool fallback = true; + int num_connectors_enabled = 0; + int num_connectors_detected = 0; /* * If the user specified any force options, just bail here @@ -324,6 +326,10 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, fb_conn = fb_helper-connector_info[i]; connector = fb_conn-connector; + + if (connector-status == connector_status_connected) + num_connectors_detected++; + if (!enabled[i]) { DRM_DEBUG_KMS(connector %d not enabled, skipping\n, connector-base.id); @@ -338,6 +344,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, continue; } + num_connectors_enabled++; + new_crtc = intel_fb_helper_crtc(fb_helper, encoder-crtc); /* @@ -393,6 +401,15 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, fallback = false; } + /* + * If the BIOS didn't enable everything it could, fall back to have the + * same user experiencing of lighting up as much as possible like the + * fbdev helper library. + */ + if (num_connectors_enabled != num_connectors_detected + num_connectors_enabled INTEL_INFO(dev)-num_pipes) + fallback = true; I think we need a debug message in here so people can figure out why their fastboot failed with this patch included. E.g. some connected outputs weren't enabled, falling back to old behavior. Also note that this will probably always happen in certain configs, and the fallback behavior won't be any better since we may not be able to light up everything that's attached. With those caveats: Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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: reverse dp link param selection, prefer fast over wide again
On Mon, 3 Mar 2014 11:18:10 +0100 Daniel Vetter daniel.vet...@ffwll.ch wrote: ... it's this time of the year again. Originally we've frobbed this to fix up some regressions, but maybe our DP code improved sufficiently now that we can dare to do again what the spec recommends. This reverts commit 2514bc510d0c3aadcc5204056bb440fa36845147 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Thu Jun 21 15:13:50 2012 -0700 drm/i915: prefer wide slow to fast narrow in DP configs I'm pretty sure I'll regret this patch, but otoh I expect we won't make progress here without poking the devil occasionally. Acked-by: Jesse Barnes jbar...@virtuousgeek.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Resolving the memory region conflict for Stolen area
On Thu, 27 Feb 2014 11:01:08 +0200 Jani Nikula jani.nik...@linux.intel.com wrote: On Thu, 27 Feb 2014, Jani Nikula jani.nik...@linux.intel.com wrote: On Wed, 26 Feb 2014, Jesse Barnes jbar...@virtuousgeek.org wrote: On Mon, 13 Jan 2014 16:25:21 +0530 akash.g...@intel.com wrote: From: Akash Goel akash.g...@intel.com There is a conflict seen when requesting the kernel to reserve the physical space used for the stolen area. This is because some BIOS are wrapping the stolen area in the root PCI bus, but have an off-by-one error. As a workaround we retry the reservation with an offset of 1 instead of 0. v2: updated commit message the comment in source file (Daniel) Signed-off-by: Akash Goel akash.g...@intel.com --- drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 1a24e84..114a806 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -82,9 +82,23 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev) r = devm_request_mem_region(dev-dev, base, dev_priv-gtt.stolen_size, Graphics Stolen Memory); if (r == NULL) { - DRM_ERROR(conflict detected with stolen region: [0x%08x - 0x%08x]\n, - base, base + (uint32_t)dev_priv-gtt.stolen_size); - base = 0; + /* + * One more attempt but this time requesting region from + * base + 1, as we have seen that this resolves the region + * conflict with the PCI Bus. + * This is a BIOS w/a: Some BIOS wrap stolen in the root + * PCI bus, but have an off-by-one error. Hence retry the + * reservation starting from 1 instead of 0. + */ + r = devm_request_mem_region(dev-dev, base + 1, + dev_priv-gtt.stolen_size - 1, + Graphics Stolen Memory); + if (r == NULL) { + DRM_ERROR(conflict detected with stolen region:\ + [0x%08x - 0x%08x]\n, + base, base + (uint32_t)dev_priv-gtt.stolen_size); + base = 0; + } } return base; Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Tested-by: Arjan van de Ven ar...@linux.intel.com Pushed to -fixes, thanks for the patch, review, and testing. Oh, forgot to add that I fixed the DRM_ERROR string split up that checkpatch complained about while applying. Note I think we should do this at the x86 quirk level as well since otherwise the kernel may put MMIO space on top of our stolen range. -- Jesse Barnes, 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/vlv: no MCHBAR on VLV
So don't try to allocate and program it, we're only fooling ourselves. Reported-by: Chang, Junxiao junxiao.ch...@intel.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_dma.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 7688abc..22f839b 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1187,6 +1187,9 @@ intel_setup_mchbar(struct drm_device *dev) u32 temp; bool enabled; + if (IS_VALLEYVIEW(dev)) + return; + dev_priv-mchbar_need_disable = false; if (IS_I915G(dev) || IS_I915GM(dev)) { -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Resolving the memory region conflict for Stolen area
On Mon, 3 Mar 2014 11:14:09 -0800 Jesse Barnes jbar...@virtuousgeek.org wrote: On Thu, 27 Feb 2014 11:01:08 +0200 Jani Nikula jani.nik...@linux.intel.com wrote: On Thu, 27 Feb 2014, Jani Nikula jani.nik...@linux.intel.com wrote: On Wed, 26 Feb 2014, Jesse Barnes jbar...@virtuousgeek.org wrote: On Mon, 13 Jan 2014 16:25:21 +0530 akash.g...@intel.com wrote: From: Akash Goel akash.g...@intel.com There is a conflict seen when requesting the kernel to reserve the physical space used for the stolen area. This is because some BIOS are wrapping the stolen area in the root PCI bus, but have an off-by-one error. As a workaround we retry the reservation with an offset of 1 instead of 0. v2: updated commit message the comment in source file (Daniel) Signed-off-by: Akash Goel akash.g...@intel.com --- drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 1a24e84..114a806 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -82,9 +82,23 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev) r = devm_request_mem_region(dev-dev, base, dev_priv-gtt.stolen_size, Graphics Stolen Memory); if (r == NULL) { - DRM_ERROR(conflict detected with stolen region: [0x%08x - 0x%08x]\n, - base, base + (uint32_t)dev_priv-gtt.stolen_size); - base = 0; + /* +* One more attempt but this time requesting region from +* base + 1, as we have seen that this resolves the region +* conflict with the PCI Bus. +* This is a BIOS w/a: Some BIOS wrap stolen in the root +* PCI bus, but have an off-by-one error. Hence retry the +* reservation starting from 1 instead of 0. +*/ + r = devm_request_mem_region(dev-dev, base + 1, + dev_priv-gtt.stolen_size - 1, + Graphics Stolen Memory); + if (r == NULL) { + DRM_ERROR(conflict detected with stolen region:\ + [0x%08x - 0x%08x]\n, + base, base + (uint32_t)dev_priv-gtt.stolen_size); + base = 0; + } } return base; Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Tested-by: Arjan van de Ven ar...@linux.intel.com Pushed to -fixes, thanks for the patch, review, and testing. Oh, forgot to add that I fixed the DRM_ERROR string split up that checkpatch complained about while applying. Note I think we should do this at the x86 quirk level as well since otherwise the kernel may put MMIO space on top of our stolen range. Nevermind, I think the stolen core code will do the right thing and mark this region reserved... but it's worth checking on an affected system. -- 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 3/9] drm/i915: make crtc enable/disable asynchronous
On Fri, 7 Feb 2014 18:37:01 -0200 Rodrigo Vivi rodrigo.v...@gmail.com wrote: From: Jesse Barnes jbar...@virtuousgeek.org The intent is to get back to userspace as quickly as possible so it can start doing drawing or whatever. It should also allow our suspend/resume/init time to improve a lot. Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/i915_irq.c | 10 +- drivers/gpu/drm/i915/intel_display.c | 27 --- drivers/gpu/drm/i915/intel_drv.h | 4 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e9c94c9..749c20f 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -917,8 +917,16 @@ static void i915_hotplug_work_func(struct work_struct *work) intel_connector = to_intel_connector(connector); intel_encoder = intel_connector-encoder; if (hpd_event_bits (1 intel_encoder-hpd_pin)) { - if (intel_encoder-hot_plug) + if (intel_encoder-hot_plug) { + struct drm_crtc *crtc = + intel_encoder-base.crtc; + if (crtc) { + mutex_lock(crtc-mutex); + intel_sync_crtc(intel_encoder-base.crtc); + mutex_unlock(crtc-mutex); + } intel_encoder-hot_plug(intel_encoder); + } if (intel_hpd_irq_event(dev, connector)) changed = true; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 21a950d..6ecd9da 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1746,6 +1746,11 @@ static void intel_crtc_disable_work(struct work_struct *work) { struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc, disable_work); + struct drm_i915_private *dev_priv = intel_crtc-base.dev-dev_private; + + mutex_lock(intel_crtc-base.mutex); + dev_priv-display._crtc_disable(intel_crtc-base); + mutex_unlock(intel_crtc-base.mutex); } void intel_queue_crtc_disable(struct drm_crtc *crtc) @@ -1761,6 +1766,11 @@ static void intel_crtc_enable_work(struct work_struct *work) { struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc, enable_work); + struct drm_i915_private *dev_priv = intel_crtc-base.dev-dev_private; + + mutex_lock(intel_crtc-base.mutex); + dev_priv-display._crtc_enable(intel_crtc-base); + mutex_unlock(intel_crtc-base.mutex); } static void intel_queue_crtc_enable(struct drm_crtc *crtc) @@ -1772,14 +1782,16 @@ static void intel_queue_crtc_enable(struct drm_crtc *crtc) queue_work(dev_priv-wq, intel_crtc-enable_work); } -static void intel_sync_crtc(struct drm_crtc *crtc) +void intel_sync_crtc(struct drm_crtc *crtc) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + WARN(!mutex_is_locked(intel_crtc-base.mutex), need crtc mutex\n); + + mutex_unlock(intel_crtc-base.mutex); flush_work(intel_crtc-disable_work); flush_work(intel_crtc-enable_work); + mutex_lock(intel_crtc-base.mutex); } /** @@ -9781,8 +9793,9 @@ static int intel_set_mode(struct drm_crtc *crtc, ret = __intel_set_mode(crtc, mode, x, y, fb); - if (ret == 0) - intel_modeset_check_state(crtc-dev); + /* FIXME: need to check after the CRTC changes have been applied */ +// if (ret == 0) +// intel_modeset_check_state(crtc-dev); return ret; } @@ -10348,8 +10361,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) drm_crtc_helper_add(intel_crtc-base, intel_helper_funcs); - INIT_WORK(intel_crtc-enable_work, intel_crtc_enable_work); - INIT_WORK(intel_crtc-disable_work, intel_crtc_disable_work); + INIT_WORK(intel_crtc-enable_work, intel_crtc_enable_work); + INIT_WORK(intel_crtc-disable_work, intel_crtc_disable_work); } enum pipe intel_get_pipe_from_connector(struct intel_connector *connector) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 5b5b51e..f104fe1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -376,6 +376,9 @@ struct intel_crtc { /* watermarks currently being used */ struct intel_pipe_wm active; } wm; + + struct work_struct enable_work
Re: [Intel-gfx] [PATCH 12/23] drm/i915: get runtime PM references when the GPU is idle/busy
On Thu, 27 Feb 2014 19:26:39 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com ... instead of PC8 references. Now that both are the same thing and we are killing PC8, just get the runtime PM reference. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0d82241..0de1aa7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8102,7 +8102,7 @@ void intel_mark_busy(struct drm_device *dev) if (dev_priv-mm.busy) return; - hsw_disable_package_c8(dev_priv); + intel_runtime_pm_get(dev_priv); i915_update_gfx_val(dev_priv); dev_priv-mm.busy = true; } @@ -8131,7 +8131,7 @@ void intel_mark_idle(struct drm_device *dev) gen6_rps_idle(dev-dev_private); out: - hsw_enable_package_c8(dev_priv); + intel_runtime_pm_put(dev_priv); } void intel_mark_fb_busy(struct drm_i915_gem_object *obj, Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 14/23] drm/i915: remove an indirection level on PC8 functions
On Thu, 27 Feb 2014 19:26:41 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com After the latest changes, the indirection is useless. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 16 ++-- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3b57257..ef0bcf8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6673,12 +6673,6 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv) hsw_disable_lcpll(dev_priv, true, true); } -static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv) -{ - WARN_ON(!mutex_is_locked(dev_priv-pc8.lock)); - intel_runtime_pm_put(dev_priv); -} - void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; @@ -6706,19 +6700,13 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) dev_priv-pc8.enabled = false; } -static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv) -{ - WARN_ON(!mutex_is_locked(dev_priv-pc8.lock)); - intel_runtime_pm_get(dev_priv); -} - void hsw_enable_package_c8(struct drm_i915_private *dev_priv) { if (!HAS_PC8(dev_priv-dev)) return; mutex_lock(dev_priv-pc8.lock); - __hsw_enable_package_c8(dev_priv); + intel_runtime_pm_put(dev_priv); mutex_unlock(dev_priv-pc8.lock); } @@ -6728,7 +6716,7 @@ void hsw_disable_package_c8(struct drm_i915_private *dev_priv) return; mutex_lock(dev_priv-pc8.lock); - __hsw_disable_package_c8(dev_priv); + intel_runtime_pm_get(dev_priv); mutex_unlock(dev_priv-pc8.lock); } Oh here it is, the next patch in the series. :) Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 13/23] drm/i915: kill pc8.disable_count
On Thu, 27 Feb 2014 19:26:40 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Since after the latest patches it's only being used to prevent getting/putting the runtime PM refcount. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 1 - drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_display.c | 14 -- drivers/gpu/drm/i915/intel_pm.c | 1 - 4 files changed, 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 36daaa8..ad6c209 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1998,7 +1998,6 @@ static int i915_pc8_status(struct seq_file *m, void *unused) mutex_lock(dev_priv-pc8.lock); seq_printf(m, GPU idle: %s\n, yesno(!dev_priv-mm.busy)); - seq_printf(m, Disable count: %d\n, dev_priv-pc8.disable_count); seq_printf(m, IRQs disabled: %s\n, yesno(dev_priv-pc8.irqs_disabled)); seq_printf(m, Enabled: %s\n, yesno(dev_priv-pc8.enabled)); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6ea5c4b..8b66c14 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1353,7 +1353,6 @@ struct i915_package_c8 { bool irqs_disabled; /* Only true after the delayed work task actually enables it. */ bool enabled; - int disable_count; struct mutex lock; struct { diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0de1aa7..3b57257 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6676,13 +6676,6 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv) static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv) { WARN_ON(!mutex_is_locked(dev_priv-pc8.lock)); - WARN(dev_priv-pc8.disable_count 1, - pc8.disable_count: %d\n, dev_priv-pc8.disable_count); - - dev_priv-pc8.disable_count--; - if (dev_priv-pc8.disable_count != 0) - return; - intel_runtime_pm_put(dev_priv); } @@ -6716,13 +6709,6 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv) { WARN_ON(!mutex_is_locked(dev_priv-pc8.lock)); - WARN(dev_priv-pc8.disable_count 0, - pc8.disable_count: %d\n, dev_priv-pc8.disable_count); - - dev_priv-pc8.disable_count++; - if (dev_priv-pc8.disable_count != 1) - return; - intel_runtime_pm_get(dev_priv); } diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 962e0d1..86e6835 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5793,7 +5793,6 @@ void intel_pm_setup(struct drm_device *dev) mutex_init(dev_priv-pc8.lock); dev_priv-pc8.irqs_disabled = false; dev_priv-pc8.enabled = false; - dev_priv-pc8.disable_count = 0; INIT_DELAYED_WORK(dev_priv-rps.delayed_resume_work, intel_gen6_powersave_work); } So now it looks like these functions are just documentation around the runtime PM bits. I don't see them get remove totally in favor of the runtime_pm_get|put calls later on, is that possible or desirable? Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 16/23] drm/i915: get/put runtime PM references for GMBUS and DP AUX
On Thu, 27 Feb 2014 19:26:43 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We had these intel_aux_display_runtime_{get,put} abstractions that would just get/put PC8 references, but now that runtime PM and PC8 are the same stuff, we just need the runtime PM references, so just get/put runtime PM directly, because that's what the rest of our code does. Another way to solve this problem would be to add DP_AUX and GMBUS power domains, and then use intel_display_power_{get,put}, but every power domain already gets/puts runtime PM references, so this would just make things more complex. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 4 ++-- drivers/gpu/drm/i915/intel_drv.h | 2 -- drivers/gpu/drm/i915/intel_i2c.c | 4 ++-- drivers/gpu/drm/i915/intel_pm.c | 11 --- 4 files changed, 4 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c512d78..79d4844 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -469,7 +469,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, intel_dp_check_edp(intel_dp); - intel_aux_display_runtime_get(dev_priv); + intel_runtime_pm_get(dev_priv); /* Try to wait for any previous AUX channel activity */ for (try = 0; try 3; try++) { @@ -563,7 +563,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, ret = recv_bytes; out: pm_qos_update_request(dev_priv-pm_qos, PM_QOS_DEFAULT_VALUE); - intel_aux_display_runtime_put(dev_priv); + intel_runtime_pm_put(dev_priv); return ret; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ea24eae..a2e0cd7 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -890,8 +890,6 @@ void ironlake_teardown_rc6(struct drm_device *dev); void gen6_update_ring_freq(struct drm_device *dev); void gen6_rps_idle(struct drm_i915_private *dev_priv); void gen6_rps_boost(struct drm_i915_private *dev_priv); -void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv); -void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv); void intel_runtime_pm_get(struct drm_i915_private *dev_priv); void intel_runtime_pm_put(struct drm_i915_private *dev_priv); void intel_init_runtime_pm(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index d33b61d..3d403ce 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -446,7 +446,7 @@ gmbus_xfer(struct i2c_adapter *adapter, int i, reg_offset; int ret = 0; - intel_aux_display_runtime_get(dev_priv); + intel_runtime_pm_get(dev_priv); mutex_lock(dev_priv-gmbus_mutex); if (bus-force_bit) { @@ -546,7 +546,7 @@ timeout: out: mutex_unlock(dev_priv-gmbus_mutex); - intel_aux_display_runtime_put(dev_priv); + intel_runtime_pm_put(dev_priv); return ret; } diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 86e6835..1e3580f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5516,17 +5516,6 @@ void intel_power_domains_init_hw(struct drm_device *dev) I915_WRITE(HSW_PWR_WELL_BIOS, 0); } -/* Disables PC8 so we can use the GMBUS and DP AUX interrupts. */ -void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv) -{ - hsw_disable_package_c8(dev_priv); -} - -void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv) -{ - hsw_enable_package_c8(dev_priv); -} - void intel_runtime_pm_get(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; But OTOH, in cases where we have a separate, explicit power well for display, doesn't the aux_display_runtime_get|put make sense? We don't want just global runtime get/put everywhere since we can be finer grained in may cases, right? -- 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 18/23] drm/i915: remove dev_priv-pc8.enabled
On Thu, 27 Feb 2014 19:26:45 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com It was just being used on debugfs and on a WARN inside hsw_set_power_well. But now that we PC8 is part of runtime PM and we get/put runtime PM when we get/put any power domain, we shouldn't need the WARN anymore. v2: - Rebase. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 1 - drivers/gpu/drm/i915/i915_drv.h | 2 -- drivers/gpu/drm/i915/intel_display.c | 3 --- drivers/gpu/drm/i915/intel_pm.c | 3 --- 4 files changed, 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index ad6c209..93d6065 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2000,7 +2000,6 @@ static int i915_pc8_status(struct seq_file *m, void *unused) seq_printf(m, GPU idle: %s\n, yesno(!dev_priv-mm.busy)); seq_printf(m, IRQs disabled: %s\n, yesno(dev_priv-pc8.irqs_disabled)); - seq_printf(m, Enabled: %s\n, yesno(dev_priv-pc8.enabled)); mutex_unlock(dev_priv-pc8.lock); return 0; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8b66c14..16e344e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1351,8 +1351,6 @@ struct ilk_wm_values { */ struct i915_package_c8 { bool irqs_disabled; - /* Only true after the delayed work task actually enables it. */ - bool enabled; struct mutex lock; struct { diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d9ea9b89..271dd66 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6660,8 +6660,6 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv) DRM_DEBUG_KMS(Enabling package C8+\n); - dev_priv-pc8.enabled = true; - if (dev_priv-pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { val = I915_READ(SOUTH_DSPCLK_GATE_D); val = ~PCH_LP_PARTITION_LEVEL_DISABLE; @@ -6697,7 +6695,6 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) mutex_lock(dev_priv-rps.hw_lock); gen6_update_ring_freq(dev); mutex_unlock(dev_priv-rps.hw_lock); - dev_priv-pc8.enabled = false; } #define for_each_power_domain(domain, mask) \ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 78f56e8..c85507a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5278,8 +5278,6 @@ static void hsw_set_power_well(struct drm_device *dev, bool is_enabled, enable_requested; uint32_t tmp; - WARN_ON(dev_priv-pc8.enabled); - tmp = I915_READ(HSW_PWR_WELL_DRIVER); is_enabled = tmp HSW_PWR_WELL_STATE_ENABLED; enable_requested = tmp HSW_PWR_WELL_ENABLE_REQUEST; @@ -5773,7 +5771,6 @@ void intel_pm_setup(struct drm_device *dev) mutex_init(dev_priv-pc8.lock); dev_priv-pc8.irqs_disabled = false; - dev_priv-pc8.enabled = false; INIT_DELAYED_WORK(dev_priv-rps.delayed_resume_work, intel_gen6_powersave_work); } Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 20/23] drm/i915: kill struct i915_package_c8
On Thu, 27 Feb 2014 19:26:47 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com The only remaining field of the struct was the lock, which was useless. v2: - Rebase. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 2 -- drivers/gpu/drm/i915/i915_drv.h | 6 -- drivers/gpu/drm/i915/intel_pm.c | 1 - 3 files changed, 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 213b093..ff81b90 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1996,11 +1996,9 @@ static int i915_pc8_status(struct seq_file *m, void *unused) return 0; } - mutex_lock(dev_priv-pc8.lock); seq_printf(m, GPU idle: %s\n, yesno(!dev_priv-mm.busy)); seq_printf(m, IRQs disabled: %s\n, yesno(dev_priv-pm.irqs_disabled)); - mutex_unlock(dev_priv-pc8.lock); return 0; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 74fba1c..08b22e3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1349,10 +1349,6 @@ struct ilk_wm_values { * * For more, read Display Sequences for Package C8 on our documentation. */ -struct i915_package_c8 { - struct mutex lock; -}; - struct i915_runtime_pm { bool suspended; bool irqs_disabled; @@ -1590,8 +1586,6 @@ typedef struct drm_i915_private { struct ilk_wm_values hw; } wm; - struct i915_package_c8 pc8; - struct i915_runtime_pm pm; /* Old dri1 support infrastructure, beware the dragons ya fools entering diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5ff4b59..3bd6e8f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5769,7 +5769,6 @@ void intel_pm_setup(struct drm_device *dev) mutex_init(dev_priv-rps.hw_lock); - mutex_init(dev_priv-pc8.lock); INIT_DELAYED_WORK(dev_priv-rps.delayed_resume_work, intel_gen6_powersave_work); Yay, this is a nice simplification overall. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 19/23] drm/i915: move pc8.irqs_disabled to pm.irqs_disabled
hsw_runtime_pm_restore_interrupts(struct drm_device *dev); /* intel_crt.c */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c85507a..5ff4b59 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5770,7 +5770,8 @@ void intel_pm_setup(struct drm_device *dev) mutex_init(dev_priv-rps.hw_lock); mutex_init(dev_priv-pc8.lock); - dev_priv-pc8.irqs_disabled = false; INIT_DELAYED_WORK(dev_priv-rps.delayed_resume_work, intel_gen6_powersave_work); + + dev_priv-pm.irqs_disabled = false; } I wonder if we should eventually not bother with saving/restoring the interrupt state and do a full re-init like we'll have to in the display power well case. But that's a separate issue really... Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 21/23] drm/i915: rename __hsw_do_{en, dis}able_pc8
On Thu, 27 Feb 2014 19:26:48 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com After we removed all the intermediate abstractions, we can rename these functions to just hsw_{en,dis}able_pc8. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 4 ++-- drivers/gpu/drm/i915/intel_display.c | 4 ++-- drivers/gpu/drm/i915/intel_drv.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7a8b86e..88ea6ce 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -846,7 +846,7 @@ static int i915_runtime_suspend(struct device *device) DRM_DEBUG_KMS(Suspending device\n); if (HAS_PC8(dev)) - __hsw_do_enable_pc8(dev_priv); + hsw_enable_pc8(dev_priv); i915_gem_release_all_mmaps(dev_priv); @@ -880,7 +880,7 @@ static int i915_runtime_resume(struct device *device) dev_priv-pm.suspended = false; if (HAS_PC8(dev)) - __hsw_do_disable_pc8(dev_priv); + hsw_disable_pc8(dev_priv); DRM_DEBUG_KMS(Device resumed\n); return 0; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 420fad5..c981e63 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6651,7 +6651,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv) gen6_gt_force_wake_put_no_rpm(dev_priv, FORCEWAKE_ALL); } -void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv) +void hsw_enable_pc8(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; uint32_t val; @@ -6671,7 +6671,7 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv) hsw_disable_lcpll(dev_priv, true, true); } -void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) +void hsw_disable_pc8(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; uint32_t val; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d0434e8..288f531 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -720,8 +720,8 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y, unsigned int bpp, unsigned int pitch); void intel_display_handle_reset(struct drm_device *dev); -void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv); -void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv); +void hsw_enable_pc8(struct drm_i915_private *dev_priv); +void hsw_disable_pc8(struct drm_i915_private *dev_priv); void intel_dp_get_m_n(struct intel_crtc *crtc, struct intel_crtc_config *pipe_config); int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n); Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 15/23] drm/i915: don't get/put PC8 reference on freeze/thaw
On Thu, 27 Feb 2014 19:26:42 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We already get runtime PM references, and PC8 is now part of runtime PM, so this is enough. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 983ab56..7a8b86e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -433,7 +433,6 @@ static int i915_drm_freeze(struct drm_device *dev) /* We do a lot of poking in a lot of registers, make sure they work * properly. */ - hsw_disable_package_c8(dev_priv); intel_display_set_init_power(dev, true); drm_kms_helper_poll_disable(dev); @@ -606,10 +605,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) schedule_work(dev_priv-console_resume_work); } - /* Undo what we did at i915_drm_freeze so the refcount goes back to the - * expected level. */ - hsw_enable_package_c8(dev_priv); - mutex_lock(dev_priv-modeset_restore_lock); dev_priv-modeset_restore = MODESET_DONE; mutex_unlock(dev_priv-modeset_restore_lock); Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 22/23] drm/i915: update the PC8 and runtime PM documentation
On Thu, 27 Feb 2014 19:26:49 -0300 Paulo Zanoni przan...@gmail.com wrote: + * Our driver uses the autosuspend delay feature, which means we'll only really + * suspend if we stay with zero refcount for a certain amount of time. The + * default value is currently very conservative (see intel_init_runtime_pm), but + * it can be changed with the standard runtime PM files frmo sysfs. typo from Otherwise, Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 17/23] drm/i915: don't get/put PC8 when getting/putting power wells
On Thu, 27 Feb 2014 19:26:44 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Because we already get/put runtime PM every time we get/put any power domain, and now PC8 and runtime PM are the same thing. With this, we can also now kill the hsw_{en,dis}able_package_c8 functions. v2: - Rebase. v3: - Rebase. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 20 drivers/gpu/drm/i915/intel_drv.h | 2 -- drivers/gpu/drm/i915/intel_pm.c | 12 ++-- 3 files changed, 2 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ef0bcf8..d9ea9b89 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6700,26 +6700,6 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) dev_priv-pc8.enabled = false; } -void hsw_enable_package_c8(struct drm_i915_private *dev_priv) -{ - if (!HAS_PC8(dev_priv-dev)) - return; - - mutex_lock(dev_priv-pc8.lock); - intel_runtime_pm_put(dev_priv); - mutex_unlock(dev_priv-pc8.lock); -} - -void hsw_disable_package_c8(struct drm_i915_private *dev_priv) -{ - if (!HAS_PC8(dev_priv-dev)) - return; - - mutex_lock(dev_priv-pc8.lock); - intel_runtime_pm_get(dev_priv); - mutex_unlock(dev_priv-pc8.lock); -} - #define for_each_power_domain(domain, mask) \ for ((domain) = 0; (domain) POWER_DOMAIN_NUM; (domain)++) \ if ((1 (domain)) (mask)) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a2e0cd7..2c02d68 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -722,8 +722,6 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y, void intel_display_handle_reset(struct drm_device *dev); void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv); void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv); -void hsw_enable_package_c8(struct drm_i915_private *dev_priv); -void hsw_disable_package_c8(struct drm_i915_private *dev_priv); void intel_dp_get_m_n(struct intel_crtc *crtc, struct intel_crtc_config *pipe_config); int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1e3580f..78f56e8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5311,26 +5311,18 @@ static void hsw_set_power_well(struct drm_device *dev, static void __intel_power_well_get(struct drm_device *dev, struct i915_power_well *power_well) { - struct drm_i915_private *dev_priv = dev-dev_private; - - if (!power_well-count++ power_well-set) { - hsw_disable_package_c8(dev_priv); + if (!power_well-count++ power_well-set) power_well-set(dev, power_well, true); - } } static void __intel_power_well_put(struct drm_device *dev, struct i915_power_well *power_well) { - struct drm_i915_private *dev_priv = dev-dev_private; - WARN_ON(!power_well-count); if (!--power_well-count power_well-set - i915.disable_power_well) { + i915.disable_power_well) power_well-set(dev, power_well, false); - hsw_enable_package_c8(dev_priv); - } } void intel_display_power_get(struct drm_device *dev, Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 23/23] drm/i915: init pm.suspended earlier
On Thu, 27 Feb 2014 19:26:50 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Function intel_init_runtime_pm is supposed to start allowing runtime PM from that point, but it's called very late on the driver initialization code, to prevent the driver from trying to suspend while still initializing. The problem is that variables are accessed earlier than that, so initalize them at intel_pm_setup, which is supposed to be the correct place. Notice that this shouldn't fix any specific bugs because dev_priv is zeroed when allocated, so the value is already correct right from the start. v2: - Rebase. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3bd6e8f..88b434b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5535,8 +5535,6 @@ void intel_init_runtime_pm(struct drm_i915_private *dev_priv) struct drm_device *dev = dev_priv-dev; struct device *device = dev-pdev-dev; - dev_priv-pm.suspended = false; - if (!HAS_RUNTIME_PM(dev)) return; @@ -5772,5 +5770,6 @@ void intel_pm_setup(struct drm_device *dev) INIT_DELAYED_WORK(dev_priv-rps.delayed_resume_work, intel_gen6_powersave_work); + dev_priv-pm.suspended = false; dev_priv-pm.irqs_disabled = false; } Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Though my earlier comments about getting rid of the init special case still apply... I think it would be a little easier to understand in that case (though maybe not, I guess we'd have to see the patches and resulting code). -- 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 16/23] drm/i915: get/put runtime PM references for GMBUS and DP AUX
On Fri, 28 Feb 2014 19:38:17 +0200 Imre Deak imre.d...@intel.com wrote: On Fri, 2014-02-28 at 09:13 -0800, Jesse Barnes wrote: On Thu, 27 Feb 2014 19:26:43 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We had these intel_aux_display_runtime_{get,put} abstractions that would just get/put PC8 references, but now that runtime PM and PC8 are the same stuff, we just need the runtime PM references, so just get/put runtime PM directly, because that's what the rest of our code does. Another way to solve this problem would be to add DP_AUX and GMBUS power domains, and then use intel_display_power_{get,put}, but every power domain already gets/puts runtime PM references, so this would just make things more complex. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 4 ++-- drivers/gpu/drm/i915/intel_drv.h | 2 -- drivers/gpu/drm/i915/intel_i2c.c | 4 ++-- drivers/gpu/drm/i915/intel_pm.c | 11 --- 4 files changed, 4 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c512d78..79d4844 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -469,7 +469,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, intel_dp_check_edp(intel_dp); - intel_aux_display_runtime_get(dev_priv); + intel_runtime_pm_get(dev_priv); /* Try to wait for any previous AUX channel activity */ for (try = 0; try 3; try++) { @@ -563,7 +563,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, ret = recv_bytes; out: pm_qos_update_request(dev_priv-pm_qos, PM_QOS_DEFAULT_VALUE); - intel_aux_display_runtime_put(dev_priv); + intel_runtime_pm_put(dev_priv); return ret; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ea24eae..a2e0cd7 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -890,8 +890,6 @@ void ironlake_teardown_rc6(struct drm_device *dev); void gen6_update_ring_freq(struct drm_device *dev); void gen6_rps_idle(struct drm_i915_private *dev_priv); void gen6_rps_boost(struct drm_i915_private *dev_priv); -void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv); -void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv); void intel_runtime_pm_get(struct drm_i915_private *dev_priv); void intel_runtime_pm_put(struct drm_i915_private *dev_priv); void intel_init_runtime_pm(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index d33b61d..3d403ce 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -446,7 +446,7 @@ gmbus_xfer(struct i2c_adapter *adapter, int i, reg_offset; int ret = 0; - intel_aux_display_runtime_get(dev_priv); + intel_runtime_pm_get(dev_priv); mutex_lock(dev_priv-gmbus_mutex); if (bus-force_bit) { @@ -546,7 +546,7 @@ timeout: out: mutex_unlock(dev_priv-gmbus_mutex); - intel_aux_display_runtime_put(dev_priv); + intel_runtime_pm_put(dev_priv); return ret; } diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 86e6835..1e3580f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5516,17 +5516,6 @@ void intel_power_domains_init_hw(struct drm_device *dev) I915_WRITE(HSW_PWR_WELL_BIOS, 0); } -/* Disables PC8 so we can use the GMBUS and DP AUX interrupts. */ -void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv) -{ - hsw_disable_package_c8(dev_priv); -} - -void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv) -{ - hsw_enable_package_c8(dev_priv); -} - void intel_runtime_pm_get(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; But OTOH, in cases where we have a separate, explicit power well for display, doesn't the aux_display_runtime_get|put make sense? We don't want just global runtime get/put everywhere since we can be finer grained in may cases, right? I think here we should just depend on connector-detect and -get_modes getting the needed power domains, which will also adjust the RPM refcount accordingly. That should work too I think, do we have any paths outside those where we would do aux poking? Maybe audio or DDC brightness controls in the future? I think audio is ok today, and we can worry about new stuff as it comes along... -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Resolving the memory region conflict for Stolen area
On Mon, 13 Jan 2014 16:25:21 +0530 akash.g...@intel.com wrote: From: Akash Goel akash.g...@intel.com There is a conflict seen when requesting the kernel to reserve the physical space used for the stolen area. This is because some BIOS are wrapping the stolen area in the root PCI bus, but have an off-by-one error. As a workaround we retry the reservation with an offset of 1 instead of 0. v2: updated commit message the comment in source file (Daniel) Signed-off-by: Akash Goel akash.g...@intel.com --- drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 1a24e84..114a806 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -82,9 +82,23 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev) r = devm_request_mem_region(dev-dev, base, dev_priv-gtt.stolen_size, Graphics Stolen Memory); if (r == NULL) { - DRM_ERROR(conflict detected with stolen region: [0x%08x - 0x%08x]\n, - base, base + (uint32_t)dev_priv-gtt.stolen_size); - base = 0; + /* + * One more attempt but this time requesting region from + * base + 1, as we have seen that this resolves the region + * conflict with the PCI Bus. + * This is a BIOS w/a: Some BIOS wrap stolen in the root + * PCI bus, but have an off-by-one error. Hence retry the + * reservation starting from 1 instead of 0. + */ + r = devm_request_mem_region(dev-dev, base + 1, + dev_priv-gtt.stolen_size - 1, + Graphics Stolen Memory); + if (r == NULL) { + DRM_ERROR(conflict detected with stolen region:\ + [0x%08x - 0x%08x]\n, + base, base + (uint32_t)dev_priv-gtt.stolen_size); + base = 0; + } } return base; Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Tested-by: Arjan van de Ven ar...@linux.intel.com Thanks, -- 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 1/5] drm: Use correct spinlock flavor in drm_vblank_get()
On Fri, 21 Feb 2014 21:03:31 +0200 ville.syrj...@linux.intel.com wrote: From: Peter Hurley pe...@hurleysoftware.com The irq flags state is already established by the outer spin_lock_irqsave(); re-disabling irqs is redundant. Signed-off-by: Peter Hurley pe...@hurleysoftware.com --- drivers/gpu/drm/drm_irq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c2676b5..baa12e7 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -882,13 +882,13 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) */ int drm_vblank_get(struct drm_device *dev, int crtc) { - unsigned long irqflags, irqflags2; + unsigned long irqflags; int ret = 0; spin_lock_irqsave(dev-vbl_lock, irqflags); /* Going from 0-1 means we have to enable interrupts again */ if (atomic_add_return(1, dev-vblank[crtc].refcount) == 1) { - spin_lock_irqsave(dev-vblank_time_lock, irqflags2); + spin_lock(dev-vblank_time_lock); if (!dev-vblank[crtc].enabled) { /* Enable vblank irqs under vblank_time_lock protection. * All vblank count timestamp updates are held off @@ -906,7 +906,7 @@ int drm_vblank_get(struct drm_device *dev, int crtc) drm_update_vblank_count(dev, crtc); } } - spin_unlock_irqrestore(dev-vblank_time_lock, irqflags2); + spin_unlock(dev-vblank_time_lock); } else { if (!dev-vblank[crtc].enabled) { atomic_dec(dev-vblank[crtc].refcount); Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 2/5] drm: Make the vblank disable timer per-crtc
; /* Last vblank seqno waited per CRTC */ unsigned int inmodeset; /* Display driver is setting mode */ + int crtc; /* crtc index */ bool enabled; /* so we don't call enable more than once per disable */ }; @@ -1157,7 +1160,6 @@ struct drm_device { spinlock_t vblank_time_lock;/** Protects vblank count and time updates during vblank enable/disable */ spinlock_t vbl_lock; - struct timer_list vblank_disable_timer; u32 max_vblank_count; /** size of vblank counter register */ Yeah this looks like a good fix. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 5/5] drm/i915: Allow vblank interrupts during modeset and eliminate some vblank races
On Tue, 25 Feb 2014 11:58:26 +0900 Michel Dänzer mic...@daenzer.net wrote: On Mon, 2014-02-24 at 14:11 +0200, Ville Syrjälä wrote: On Mon, Feb 24, 2014 at 12:48:55PM +0900, Michel Dänzer wrote: On Fre, 2014-02-21 at 21:03 +0200, ville.syrj...@linux.intel.com wrote: We can kill of the drm_vblank_{pre,post}_modeset() calls since those are there simply to make drm_vblank_get() fail during a modeset. Actually, their original purpose was to keep the DRM vblank counter consistent across modesets, assuming the modeset resets the hardware vblank counter. I see. Well, actually I really don't. The code is too funky for me to tell what it actually ends up doing. The obvious way would be to resample the hardware counter at drm_vblank_post_modeset(), which the code certainly doesn't do. But maybe it did something sensible in the past. When the pre/post-modeset hooks were originally added, it worked like this: the pre-modeset hook enabled the vblank interrupt, which updated the DRM vblank counter from the driver/HW counter. The post-modeset hook disabled the vblank interrupt again, which recorded the post-modeset driver/HW counter value. But the vblank code has changed a lot since then, not sure it still works like that. Yeah it's changed a bit. I think Rob added the latest bits there. They were originally in place to support both UMS and KMS drivers, which is as ugly as it sounds. As long as nothing breaks (vblank vs dpms, vblank vs modeset, vblank vs high precision timestamps, vblank vs refcount) I think your code is ok. Cc'ing Mario for another opinion too. This makes me nervous but it seems ok. I think you should update the docbook (with examples) as well so other driver writers will know how to use this stuff. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 3/5] drm: Allow the driver to reject vblank requests only when it really has the vblank interrupts disabled
) REG_WRITE(VGACNTRL, VGA_DISP_DISABLE); /* Turn off vblank interrupts */ - drm_vblank_off(dev, pipe); + drm_vblank_off(dev, pipe, false); /* Wait for vblank for the disable to take effect */ gma_wait_for_vblank(dev); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f19e6ea..bab0d08 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3643,7 +3643,7 @@ static void haswell_crtc_disable_planes(struct drm_crtc *crtc) int plane = intel_crtc-plane; intel_crtc_wait_for_pending_flips(crtc); - drm_vblank_off(dev, pipe); + drm_vblank_off(dev, pipe, false); /* FBC must be disabled before disabling the plane on HSW. */ if (dev_priv-fbc.plane == plane) @@ -3774,7 +3774,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) encoder-disable(encoder); intel_crtc_wait_for_pending_flips(crtc); - drm_vblank_off(dev, pipe); + drm_vblank_off(dev, pipe, false); if (dev_priv-fbc.plane == plane) intel_disable_fbc(dev); @@ -4239,7 +4239,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) /* Give the overlay scaler a chance to disable if it's on this pipe */ intel_crtc_wait_for_pending_flips(crtc); - drm_vblank_off(dev, pipe); + drm_vblank_off(dev, pipe, false); if (dev_priv-fbc.plane == plane) intel_disable_fbc(dev); diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 9336006..480bfec 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -324,7 +324,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc) } } - drm_vblank_off(drm, dc-pipe); + drm_vblank_off(drm, dc-pipe, false); } static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc, diff --git a/include/drm/drmP.h b/include/drm/drmP.h index f974da9..ee40483 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1090,6 +1090,7 @@ struct drm_vblank_crtc { int crtc; /* crtc index */ bool enabled; /* so we don't call enable more than once per disable */ + bool reject;/* reject drm_vblank_get()? */ }; /** @@ -1400,7 +1401,8 @@ extern void drm_send_vblank_event(struct drm_device *dev, int crtc, extern bool drm_handle_vblank(struct drm_device *dev, int crtc); extern int drm_vblank_get(struct drm_device *dev, int crtc); extern void drm_vblank_put(struct drm_device *dev, int crtc); -extern void drm_vblank_off(struct drm_device *dev, int crtc); +extern void drm_vblank_off(struct drm_device *dev, int crtc, bool reject); +extern void drm_vblank_on(struct drm_device *dev, int crtc); extern void drm_vblank_cleanup(struct drm_device *dev); extern u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, struct timeval *tvblank, unsigned flags); Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 19/19] drm/i915: power domains: add vlv power wells
On Wed, 26 Feb 2014 20:02:19 +0200 Imre Deak imre.d...@intel.com wrote: On Thu, 2014-02-20 at 11:58 -0800, Jesse Barnes wrote: On Wed, 19 Feb 2014 14:29:44 +0200 Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Tue, Feb 18, 2014 at 12:02:20AM +0200, Imre Deak wrote: Based on an early draft from Jesse. Add support for powering on/off the dynamic power wells on VLV by registering its display and dpio dynamic power wells with the power domain framework. For now power on all PHY TX lanes regardless of the actual lane configuration. Later this can be optimized when the PHY side setup enables only the required lanes. Atm, it enables all lanes in all cases. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 1 - drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_display.c | 1 + drivers/gpu/drm/i915/intel_pm.c | 228 +++ 4 files changed, 230 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index dca4dc3..f8f7a59 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1668,7 +1668,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_mtrrfree; } - dev_priv-display_irqs_enabled = true; intel_irq_init(dev); intel_uncore_sanitize(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 227c349..804334e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1053,7 +1053,7 @@ struct i915_power_well { /* power well enable/disable usage count */ int count; unsigned long domains; - void *data; + unsigned long data; const struct i915_power_well_ops *ops; }; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ea00878..d6661c4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4224,6 +4224,7 @@ static void valleyview_modeset_global_resources(struct drm_device *dev) if (req_cdclk != cur_cdclk) valleyview_set_cdclk(dev, req_cdclk); + modeset_update_power_wells(dev); } static void valleyview_crtc_enable(struct drm_crtc *crtc) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 68f58e5..e4416a7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5344,6 +5344,133 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv, hsw_enable_package_c8(dev_priv); } +static void vlv_set_power_well(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well, bool enable) +{ + enum punit_power_well power_well_id = power_well-data; + u32 mask; + u32 state; + u32 ctrl; + + mask = PUNIT_PWRGT_MASK(power_well_id); + state = enable ? PUNIT_PWRGT_PWR_ON(power_well_id) : +PUNIT_PWRGT_PWR_GATE(power_well_id); + + mutex_lock(dev_priv-rps.hw_lock); + +#define COND \ + ((vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS) mask) == state) + + if (COND) + goto out; + + ctrl = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_CTRL); + ctrl = ~mask; + ctrl |= state; + vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, ctrl); + + if (wait_for(COND, 100)) + DRM_ERROR(timout setting power well state %08x (%08x)\n, + state, + vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_CTRL)); #undef COND somewhere to avoid suprises further down in the code? + +out: + mutex_unlock(dev_priv-rps.hw_lock); +} + snip I'd like to see the code for re-enabling the display state land eventually too, so we can get savings when userspace uses DPMS instead of NULL mode sets for things. But to do that nicely requires some more work in the mode set code to pull out more PLL bits (also needed for atomic mode setting). I guess you meant here the drm connector-funcs-dpms() callback, that's called when setting the connector's dpms property. But I'm not sure what you meant by re-enabling the display state. I was thinking that we need to get/put the power domains around setting DPMS off/on via the above property, but we actually don't need that. Internally the dpms callback just calls
Re: [Intel-gfx] 3.13 i915 brightness settings broken when going from docked - undocked
On Sun, 23 Feb 2014 10:50:59 -0500 Josh Boyer jwbo...@fedoraproject.org wrote: On Thu, Feb 20, 2014 at 07:31:41PM -0500, Josh Boyer wrote: On Wed, Feb 19, 2014 at 9:20 PM, Josh Boyer jwbo...@fedoraproject.org wrote: Hi All, We've had a rather weird report[1] of the brightness adjustments being broken in a specific case with Thinkpad x220 hardware (SandyBridge based). If you boot the machine with it in a dock and then undock, the brightness adjustments do not work. That is with either the FN keys or the GNOME brightness slider. I can see that the value of /sys/class/backlight/acpi_video0/brightness increases/decreases but /sys/class/backlight/intel_backlight/brightness doesn't reflect any changes. With 3.12 this works, and oddly with 3.14-rc1 it works (specifically, it starts working around v3.13-10231-g53d8ab2 which is right after the first DRM merge for 3.14). With 3.13, if I undock and echo a higher value in the intel_backlight_brightness sysfs entry, the brightness will actually increase so it can be done manually, but it does not work as you'd expect. I'm in the middle of trying to do a reverse bisect for which patch fixes it in the 3.14-rcX series, but that's taking a while. I thought I'd email and see if anyone already knows about this situation, what patch in 3.13 broke this, and which one then fixed it again. Thus far all I've gathered is that backlight handling is confusing. The reverse bisect between 3.13 and 3.14-rc1 didn't prove fruitful, either because I messed it up or there's a combination of things that fix the issue. So instead I did a regular git bisect between 3.12 and 3.13 to see which commit _broke_ things and caused the above behavior. That landed me at: Author: Jesse Barnes jbar...@virtuousgeek.org Date: Thu Oct 31 18:55:49 2013 +0200 drm/i915: make backlight functions take a connector I have no idea if that makes sense or not, but it's at least something that seems to be in a relevant area of code. Does anyone involved in that know why it would cause the above symptoms on 3.13, and which commit(s) fix it in 3.14-rc1? Since nobody is replying I poked around a bit myself. The one commit that looks somewhat relevant in 3.14-rc1 seems to be: commit c91c9f32843a1b433de5a1ead4789a6bc8d3d914 Author: Jani Nikula jani.nik...@intel.com Date: Fri Nov 8 16:48:55 2013 +0200 drm/i915: make asle notifications update backlight on all connectors That doesn't apply cleanly on 3.13 because of the other reworks that went in first, so I came up with the patch below. It seems to fix it for my machine, but I'm waiting for confirmation from the original bug reporter still. Maybe this will elicit some comments? josh Backport of upstream commit c91c9f328 --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_opregion.c | 31 ++- drivers/gpu/drm/i915/intel_panel.c| 4 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 221ac62..d6d4349 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1371,6 +1371,7 @@ typedef struct drm_i915_private { /* backlight */ struct { + bool present; int level; bool enabled; spinlock_t lock; /* bl registers and the above bl fields */ diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index 6d69a9b..b2a51ae 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -414,38 +414,19 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp) return ASLC_BACKLIGHT_FAILED; mutex_lock(dev-mode_config.mutex); - /* - * Could match the OpRegion connector here instead, but we'd also need - * to verify the connector could handle a backlight call. - */ - list_for_each_entry(encoder, dev-mode_config.encoder_list, head) - if (encoder-crtc == crtc) { - found = true; - break; - } - - if (!found) { - ret = ASLC_BACKLIGHT_FAILED; - goto out; - } - list_for_each_entry(connector, dev-mode_config.connector_list, head) - if (connector-encoder == encoder) - intel_connector = to_intel_connector(connector); - - if (!intel_connector) { - ret = ASLC_BACKLIGHT_FAILED; - goto out; + DRM_DEBUG_KMS(updating opregion backlight %d/255\n, bclp); + list_for_each_entry(connector, dev-mode_config.connector_list, head) { + intel_connector = to_intel_connector(connector); + if (dev_priv-backlight.present) + intel_panel_set_backlight(intel_connector, bclp, 255
Re: [Intel-gfx] [PATCH 03/11] drm/i915: put runtime PM only when we actually release force_wake
On Fri, 21 Feb 2014 13:52:20 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com When we call gen6_gt_force_wake_put we don't actually put force_wake, we just schedule gen6_force_wake_work through mod_delayed_work, and that will eventually release force_wake. The problem is that we call intel_runtime_pm_put directly at gen6_gt_force_wake_put, so most of the times we put our runtime PM reference before the delayed work happens, so we may runtime suspend while force_wake is still supposed to be enabled if the graphics autosuspend_delay_ms is too small. Now the nice thing about the current code is that after it triggers the delayed work function it gets a refcount, and it only triggers the delayed work function if refcount is zero. This guarantees that when we schedule the funciton, it will run before we try to schedule it again, which simplifies the problem and allows for the current solution to work properly (hopefully!). Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_uncore.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index c628414..1f7226f 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -299,6 +299,8 @@ static void gen6_force_wake_work(struct work_struct *work) if (--dev_priv-uncore.forcewake_count == 0) dev_priv-uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL); spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags); + + intel_runtime_pm_put(dev_priv); } static void intel_uncore_forcewake_reset(struct drm_device *dev) @@ -393,6 +395,7 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine) void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine) { unsigned long irqflags; + bool delayed = false; if (!dev_priv-uncore.funcs.force_wake_put) return; @@ -405,13 +408,15 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine) spin_lock_irqsave(dev_priv-uncore.lock, irqflags); if (--dev_priv-uncore.forcewake_count == 0) { dev_priv-uncore.forcewake_count++; + delayed = true; mod_delayed_work(dev_priv-wq, dev_priv-uncore.force_wake_work, 1); } spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags); - intel_runtime_pm_put(dev_priv); + if (!delayed) + intel_runtime_pm_put(dev_priv); } /* We give fast paths for the really cool registers */ Do we need this for the VLV path too? It's a little confusing that we do this delayed thing, incrementing the count and then decrementing again in the work queue, but what you have looks correct for both cases. So with the VLV thing addressed: Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 02/11] drm/i915: put runtime PM only at the end of intel_mark_idle
On Fri, 21 Feb 2014 13:52:19 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Because intel_mark_idle still touches some registers: it needs the machine to be awake. If you set both the autosuspend and PC8 delays to zero, you can get a Device suspended WARN when gen6_rps_idle touches registers. This is not easy to reproduce, but happens once in a while when running pm_pc8. Testcase: igt/pm_pc8 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dd416f2..10ec401 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8210,10 +8210,8 @@ void intel_mark_idle(struct drm_device *dev) dev_priv-mm.busy = false; - hsw_package_c8_gpu_idle(dev_priv); - if (!i915.powersave) - return; + goto out; list_for_each_entry(crtc, dev-mode_config.crtc_list, head) { if (!crtc-fb) @@ -8224,6 +8222,9 @@ void intel_mark_idle(struct drm_device *dev) if (INTEL_INFO(dev)-gen = 6) gen6_rps_idle(dev-dev_private); + +out: + hsw_package_c8_gpu_idle(dev_priv); } void intel_mark_fb_busy(struct drm_i915_gem_object *obj, Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 04/11] drm/i915: get runtime PM at intel_set_mode
On Fri, 21 Feb 2014 13:52:21 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Otherwise, when we run intel_modeset_check_state we may already be runtime suspended, and our state checking code will read registers while the device is suspended. This can only happen if your autosuspend_delay_ms is low (not the default 10s). Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 10ec401..c64fb7f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9746,13 +9746,18 @@ static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, int x, int y, struct drm_framebuffer *fb) { + struct drm_device *dev = crtc-dev; + struct drm_i915_private *dev_priv = dev-dev_private; int ret; + intel_runtime_pm_get(dev_priv); + ret = __intel_set_mode(crtc, mode, x, y, fb); if (ret == 0) intel_modeset_check_state(crtc-dev); + intel_runtime_pm_put(dev_priv); return ret; } Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 05/11] drm/i915: get runtime PM while trying to detect CRT
On Fri, 21 Feb 2014 13:52:22 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Otherwise we'll read registers that return 0x, trigger some WARNs, think CRT is actually connected (because certain bits are 1), and fail the drm-resources-equal testcase! Tested on a SNB machine with runtime PM support (which is not upstream yet, but is already on my public tree at freedesktop.org, and will hopefully eventually become upstream). Testcase: igt/pm_pc8/drm-resources-equal Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_crt.c | 26 +++--- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 9864aa1..4c1230c 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -630,10 +630,13 @@ static enum drm_connector_status intel_crt_detect(struct drm_connector *connector, bool force) { struct drm_device *dev = connector-dev; + struct drm_i915_private *dev_priv = dev-dev_private; struct intel_crt *crt = intel_attached_crt(connector); enum drm_connector_status status; struct intel_load_detect_pipe tmp; + intel_runtime_pm_get(dev_priv); + DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n, connector-base.id, drm_get_connector_name(connector), force); @@ -645,23 +648,30 @@ intel_crt_detect(struct drm_connector *connector, bool force) */ if (intel_crt_detect_hotplug(connector)) { DRM_DEBUG_KMS(CRT detected via hotplug\n); - return connector_status_connected; + status = connector_status_connected; + goto out; } else DRM_DEBUG_KMS(CRT not detected via hotplug\n); } - if (intel_crt_detect_ddc(connector)) - return connector_status_connected; + if (intel_crt_detect_ddc(connector)) { + status = connector_status_connected; + goto out; + } /* Load detection is broken on HPD capable machines. Whoever wants a * broken monitor (without edid) to work behind a broken kvm (that fails * to have the right resistors for HP detection) needs to fix this up. * For now just bail out. */ - if (I915_HAS_HOTPLUG(dev)) - return connector_status_disconnected; + if (I915_HAS_HOTPLUG(dev)) { + status = connector_status_disconnected; + goto out; + } - if (!force) - return connector-status; + if (!force) { + status = connector-status; + goto out; + } /* for pre-945g platforms use load detect */ if (intel_get_load_detect_pipe(connector, NULL, tmp)) { @@ -673,6 +683,8 @@ intel_crt_detect(struct drm_connector *connector, bool force) } else status = connector_status_unknown; +out: + intel_runtime_pm_put(dev_priv); return status; } Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 06/11] drm/i915: get/put runtime PM in more places at i915_debugfs.c
On Fri, 21 Feb 2014 13:52:23 -0300 Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com These are places where we read (not write) registers while we're runtime suspended. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d90a707..34e347f 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1348,6 +1348,8 @@ static int i915_fbc_status(struct seq_file *m, void *unused) return 0; } + intel_runtime_pm_get(dev_priv); + if (intel_fbc_enabled(dev)) { seq_puts(m, FBC enabled\n); } else { @@ -1391,6 +1393,9 @@ static int i915_fbc_status(struct seq_file *m, void *unused) } seq_putc(m, '\n'); } + + intel_runtime_pm_put(dev_priv); + return 0; } @@ -1405,11 +1410,15 @@ static int i915_ips_status(struct seq_file *m, void *unused) return 0; } + intel_runtime_pm_get(dev_priv); + if (IS_BROADWELL(dev) || I915_READ(IPS_CTL) IPS_ENABLE) seq_puts(m, enabled\n); else seq_puts(m, disabled\n); + intel_runtime_pm_put(dev_priv); + return 0; } @@ -1420,6 +1429,8 @@ static int i915_sr_status(struct seq_file *m, void *unused) drm_i915_private_t *dev_priv = dev-dev_private; bool sr_enabled = false; + intel_runtime_pm_get(dev_priv); + if (HAS_PCH_SPLIT(dev)) sr_enabled = I915_READ(WM1_LP_ILK) WM1_LP_SR_EN; else if (IS_CRESTLINE(dev) || IS_I945G(dev) || IS_I945GM(dev)) @@ -1429,6 +1440,8 @@ static int i915_sr_status(struct seq_file *m, void *unused) else if (IS_PINEVIEW(dev)) sr_enabled = I915_READ(DSPFW3) PINEVIEW_SELF_REFRESH_EN; + intel_runtime_pm_put(dev_priv); + seq_printf(m, self-refresh: %s\n, sr_enabled ? enabled : disabled); @@ -1972,12 +1985,16 @@ static int i915_energy_uJ(struct seq_file *m, void *data) if (INTEL_INFO(dev)-gen 6) return -ENODEV; + intel_runtime_pm_get(dev_priv); + rdmsrl(MSR_RAPL_POWER_UNIT, power); power = (power 0x1f00) 8; units = 100 / (1 power); /* convert to uJ */ power = I915_READ(MCH_SECP_NRG_STTS); power *= units; + intel_runtime_pm_put(dev_priv); + seq_printf(m, %llu, (long long unsigned)power); return 0; Looks like there are more places we need this too.. wonder if it would be better to put the get into some wrapper around our sysfs files... But these bits look correct, if not sufficient, so: Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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] [RFC 1/6] drm/i915: cover ioctls with runtime_get/put
On Fri, 24 Jan 2014 19:23:54 +0200 Imre Deak imre.d...@intel.com wrote: On Fri, 2014-01-24 at 20:35 +0530, Naresh Kumar Kachhi wrote: On 01/22/2014 06:53 PM, Imre Deak wrote: On Wed, 2014-01-22 at 13:51 +0100, Daniel Vetter wrote: On Wed, Jan 22, 2014 at 05:34:17PM +0530, naresh.kumar.kac...@intel.com wrote: From: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com With runtime PM enabled, we need to make sure that all HW access are valid (i.e. Gfx is in D0). Invalid accesses might end up in HW hangs. Ex. A hang is seen if display register is accessed on BYT while display power island is power gated. This patch is covering all the IOCTLs with get/put. TODO: limit runtime_get/put to IOCTLs that accesses HW Signed-off-by: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com Nack on the concept. We need to have get/put calls for the individual functional blocks of the hw, which then in turn (if it's not the top-level power domain) need to grab references to the next up power domain. Splattering unconditional get/puts over all driver entry points is bad style and imo also too fragile. Also, with Paulos final runtime pm/pc8 unification patches and Imre's display power well patches for byt we should have full coverage already. Have you looked at the work of these too? I'm still in debt with the BYT specific power domain patches, I want to post them (this week) after I sorted out spurious pipe stat IRQs we'd receive atm with the power well off. Until then there is only the WIP version at: https://github.com/ideak/linux/commits/powerwells But in practice, as you point out the plan was to only call modeset_update_power_wells() during modeset time and that will end up doing the proper RPM get/put as necessary. Similarly on some other code paths like connector detect and HW state read-out code, we'd ask only for the needed power domains which would end up doing the RPM get/put. --Imre Hi Imre, I tried to go through your and Paulo's patches (http://patchwork.freedesktop.org/patch/16952/). As per my understanding we are doing disp power well gate/ungate independent of we are in runtime_suspend/resume state we might face problem here as on BYT interrupts are routed through display power well, so we might have a situation where display power island is power gated, but render workload is still active. As we won't be receiving any interrupt __wait_seq_no will stall and we might end up in a TDR. We will not see the issue until display power gating is enabled. I will try to include a test to cover this test Hm, which exact interrupt routing are you referring to? At least on my BYT I manage to power off only the display power well and keep the render well on, while still being able to run for example igt/tests/gem_render_copy, gem_render_linear_blits. I can see that through /proc/interrupts that GT interrupts are properly generated. The display side interrupt routing is of course off, for example the PIPESTAT registers will read all 0x, but DEIIR, DEIMR, DEISR are all seem to work ok. I think Naresh is referring to full D3 state shutdown, where the Gunit goes away too. But in that case the GT will be shut down as well, so I don't think the display vs GT thing by itself is a problem. -- Jesse Barnes, 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: re-add locking around hw state readout
To silence locking complaints. This was a rebase failure on my part in commit fa9fa083d0606cb323f6105c17702460ea0a6780 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Tue Feb 11 15:28:56 2014 -0800 drm/i915: read out hw state earlier v2 Reported-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 13ad064..69cade7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11282,7 +11282,9 @@ void intel_modeset_init(struct drm_device *dev) /* Just in case the BIOS is doing something questionable. */ intel_disable_fbc(dev); + mutex_lock(dev-mode_config.mutex); intel_modeset_setup_hw_state(dev, false); + mutex_unlock(dev-mode_config.mutex); list_for_each_entry(crtc, dev-mode_config.crtc_list, base.head) { -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Revert workaround for disabling L3 cache aging on BYT
On Wed, 19 Feb 2014 13:09:31 -0800 Sinclair Yeh sinclair@intel.com wrote: V2: edit the commit message to contain more info The W/A spreadsheet says this is still required, but the b-spec says it's not for BYT-T. So the documentation is not clear. However, our experience with the other SKUs of BYT-I/M on Android and Linux suggests that setting this bit actually causes GPU hang for certain OGL benchmark applications. Removing this bit completely resolves the GPU hangs. --- drivers/gpu/drm/i915/intel_pm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a6b877a..3ba037e 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5004,9 +5004,6 @@ static void valleyview_init_clock_gating(struct drm_device *dev) _MASKED_BIT_ENABLE(GEN7_MAX_PS_THREAD_DEP | GEN7_PSD_SINGLE_PORT_DISPATCH_ENABLE)); - /* WaDisableL3CacheAging:vlv */ - I915_WRITE(GEN7_L3CNTLREG1, I915_READ(GEN7_L3CNTLREG1) | GEN7_L3AGDIS); - /* WaForceL3Serialization:vlv */ I915_WRITE(GEN7_L3SQCREG4, I915_READ(GEN7_L3SQCREG4) ~L3SQ_URB_READ_CAM_MATCH_DISABLE); I don't think we have good docs on this, but since it works empirically: Acked-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 02/19] drm/i915: fold in __intel_power_well_get/put functions
On Tue, 18 Feb 2014 00:02:03 +0200 Imre Deak imre.d...@intel.com wrote: These functions are used only by a single call site and are simple enough to just fold them in. No functional change. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 37 + 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index aa9c2df..db48d55 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5305,27 +5305,6 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv, } } -static void __intel_power_well_get(struct drm_i915_private *dev_priv, -struct i915_power_well *power_well) -{ - if (!power_well-count++ power_well-set) { - hsw_disable_package_c8(dev_priv); - power_well-set(dev_priv, power_well, true); - } -} - -static void __intel_power_well_put(struct drm_i915_private *dev_priv, -struct i915_power_well *power_well) -{ - WARN_ON(!power_well-count); - - if (!--power_well-count power_well-set - i915.disable_power_well) { - power_well-set(dev_priv, power_well, false); - hsw_enable_package_c8(dev_priv); - } -} - void intel_display_power_get(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain) { @@ -5338,7 +5317,10 @@ void intel_display_power_get(struct drm_i915_private *dev_priv, mutex_lock(power_domains-lock); for_each_power_well(i, power_well, BIT(domain), power_domains) - __intel_power_well_get(dev_priv, power_well); + if (!power_well-count++ power_well-set) { + hsw_disable_package_c8(dev_priv); + power_well-set(dev_priv, power_well, true); + } power_domains-domain_use_count[domain]++; @@ -5359,8 +5341,15 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, WARN_ON(!power_domains-domain_use_count[domain]); power_domains-domain_use_count[domain]--; - for_each_power_well_rev(i, power_well, BIT(domain), power_domains) - __intel_power_well_put(dev_priv, power_well); + for_each_power_well_rev(i, power_well, BIT(domain), power_domains) { + WARN_ON(!power_well-count); + + if (!--power_well-count power_well-set + i915.disable_power_well) { + power_well-set(dev_priv, power_well, false); + hsw_enable_package_c8(dev_priv); + } + } mutex_unlock(power_domains-lock); } Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 03/19] drm/i915: move modeset_update_power_wells earlier
- * any unnecessary toggling of the power wells. - */ - list_for_each_entry(crtc, dev-mode_config.crtc_list, base.head) { - enum intel_display_power_domain domain; - - if (!crtc-base.enabled) - continue; - - pipe_domains[crtc-pipe] = get_pipe_power_domains(dev, - crtc-pipe, - crtc-config.pch_pfit.enabled); - - for_each_power_domain(domain, pipe_domains[crtc-pipe]) - intel_display_power_get(dev_priv, domain); - } - - list_for_each_entry(crtc, dev-mode_config.crtc_list, base.head) { - enum intel_display_power_domain domain; - - for_each_power_domain(domain, crtc-enabled_power_domains) - intel_display_power_put(dev_priv, domain); - - crtc-enabled_power_domains = pipe_domains[crtc-pipe]; - } - - intel_display_set_init_power(dev_priv, false); -} - static void haswell_modeset_global_resources(struct drm_device *dev) { modeset_update_power_wells(dev); Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 01/19] drm/i915: use drm_i915_private everywhere in the power domain api
to be always enabled. */ - intel_display_set_init_power(dev, true); - intel_power_domains_resume(dev); + intel_display_set_init_power(dev_priv, true); + intel_power_domains_resume(dev_priv); - if (!(IS_HASWELL(dev) || IS_BROADWELL(dev))) + if (!(IS_HASWELL(dev_priv-dev) || IS_BROADWELL(dev_priv-dev))) return; /* We're taking over the BIOS, so clear any requests made by it since Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 04/19] drm/i915: move power domain macros to intel_pm.c
On Tue, 18 Feb 2014 00:02:05 +0200 Imre Deak imre.d...@intel.com wrote: These macros are used only locally, so move them to the .c file. Also since logically the init power domain should be part of all power wells add it to the always-on power wells too for consistency. Since always-on power wells have noop handlers, this doesn't change the functionality. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 10 -- drivers/gpu/drm/i915/intel_pm.c | 21 +++-- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 796f971..de0c0e0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -121,8 +121,6 @@ enum intel_display_power_domain { POWER_DOMAIN_NUM, }; -#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1) - #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A) #define POWER_DOMAIN_PIPE_PANEL_FITTER(pipe) \ ((pipe) + POWER_DOMAIN_PIPE_A_PANEL_FITTER) @@ -130,14 +128,6 @@ enum intel_display_power_domain { ((tran) == TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \ (tran) + POWER_DOMAIN_TRANSCODER_A) -#define HSW_ALWAYS_ON_POWER_DOMAINS (\ - BIT(POWER_DOMAIN_PIPE_A) | \ - BIT(POWER_DOMAIN_TRANSCODER_EDP)) -#define BDW_ALWAYS_ON_POWER_DOMAINS (\ - BIT(POWER_DOMAIN_PIPE_A) | \ - BIT(POWER_DOMAIN_TRANSCODER_EDP) | \ - BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER)) - enum hpd_pin { HPD_NONE = 0, HPD_PORT_A = HPD_NONE, /* PORT_A is internal */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index db48d55..9a608f1 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5384,6 +5384,23 @@ void i915_release_power_well(void) } EXPORT_SYMBOL_GPL(i915_release_power_well); +#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1) + +#define HSW_ALWAYS_ON_POWER_DOMAINS (\ + BIT(POWER_DOMAIN_PIPE_A) | \ + BIT(POWER_DOMAIN_TRANSCODER_EDP) | \ + BIT(POWER_DOMAIN_INIT)) +#define HSW_DISPLAY_POWER_DOMAINS ( \ + (POWER_DOMAIN_MASK ~HSW_ALWAYS_ON_POWER_DOMAINS) |\ + BIT(POWER_DOMAIN_INIT)) + +#define BDW_ALWAYS_ON_POWER_DOMAINS (\ + HSW_ALWAYS_ON_POWER_DOMAINS | \ + BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER)) +#define BDW_DISPLAY_POWER_DOMAINS ( \ + (POWER_DOMAIN_MASK ~BDW_ALWAYS_ON_POWER_DOMAINS) |\ + BIT(POWER_DOMAIN_INIT)) + static struct i915_power_well i9xx_always_on_power_well[] = { { .name = always-on, @@ -5400,7 +5417,7 @@ static struct i915_power_well hsw_power_wells[] = { }, { .name = display, - .domains = POWER_DOMAIN_MASK ~HSW_ALWAYS_ON_POWER_DOMAINS, + .domains = HSW_DISPLAY_POWER_DOMAINS, .is_enabled = hsw_power_well_enabled, .set = hsw_set_power_well, }, @@ -5414,7 +5431,7 @@ static struct i915_power_well bdw_power_wells[] = { }, { .name = display, - .domains = POWER_DOMAIN_MASK ~BDW_ALWAYS_ON_POWER_DOMAINS, + .domains = BDW_DISPLAY_POWER_DOMAINS, .is_enabled = hsw_power_well_enabled, .set = hsw_set_power_well, }, Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- 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 07/19] drm/i915: add port power domains
; + } + case INTEL_OUTPUT_ANALOG: + return POWER_DOMAIN_PORT_CRT; + case INTEL_OUTPUT_DSI: + return POWER_DOMAIN_PORT_DSI; + default: + return POWER_DOMAIN_PORT_OTHER; + } +} + +static unsigned long get_crtc_power_domains(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc-dev; + struct intel_encoder *intel_encoder; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + enum pipe pipe = intel_crtc-pipe; + bool pfit_enabled = intel_crtc-config.pch_pfit.enabled; unsigned long mask; enum transcoder transcoder; @@ -3970,6 +4010,9 @@ static unsigned long get_pipe_power_domains(struct drm_device *dev, if (pfit_enabled) mask |= BIT(POWER_DOMAIN_PIPE_PANEL_FITTER(pipe)); + for_each_encoder_on_crtc(dev, crtc, intel_encoder) + mask |= BIT(intel_display_port_power_domain(intel_encoder)); + return mask; } @@ -4003,9 +4046,7 @@ static void modeset_update_power_wells(struct drm_device *dev) if (!crtc-base.enabled) continue; - pipe_domains[crtc-pipe] = get_pipe_power_domains(dev, - crtc-pipe, - crtc-config.pch_pfit.enabled); + pipe_domains[crtc-pipe] = get_crtc_power_domains(crtc-base); for_each_power_domain(domain, pipe_domains[crtc-pipe]) intel_display_power_get(dev_priv, domain); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 6042797..e31eb1e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -733,6 +733,8 @@ bool intel_crtc_active(struct drm_crtc *crtc); void hsw_enable_ips(struct intel_crtc *crtc); void hsw_disable_ips(struct intel_crtc *crtc); void intel_display_set_init_power(struct drm_i915_private *dev, bool enable); +enum intel_display_power_domain +intel_display_port_power_domain(struct intel_encoder *intel_encoder); int valleyview_get_vco(struct drm_i915_private *dev_priv); void intel_mode_from_pipe_config(struct drm_display_mode *mode, struct intel_crtc_config *pipe_config); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index faaa4ec..9cb7ed6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5412,6 +5412,15 @@ EXPORT_SYMBOL_GPL(i915_release_power_well); #define HSW_ALWAYS_ON_POWER_DOMAINS (\ BIT(POWER_DOMAIN_PIPE_A) | \ BIT(POWER_DOMAIN_TRANSCODER_EDP) | \ + BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) | \ + BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) | \ + BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) | \ + BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) | \ + BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) | \ + BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) | \ + BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) | \ + BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) | \ + BIT(POWER_DOMAIN_PORT_CRT) |\ BIT(POWER_DOMAIN_INIT)) #define HSW_DISPLAY_POWER_DOMAINS ( \ (POWER_DOMAIN_MASK ~HSW_ALWAYS_ON_POWER_DOMAINS) |\ Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org I wonder if we want a way to parameterize the lane count instead, in case we end up doing 1 lane for example in the future, or have some other power domain that can provide multiple levels of power. But I suppose we can deal with that if/when we come to it. -- 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 05/19] drm/i915: power domains: add power well ops
++ power_well-ops-enable) + power_well-ops-enable(dev_priv, power_well); power_domains-domain_use_count[domain]++; @@ -5344,11 +5369,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, for_each_power_well_rev(i, power_well, BIT(domain), power_domains) { WARN_ON(!power_well-count); - if (!--power_well-count power_well-set - i915.disable_power_well) { - power_well-set(dev_priv, power_well, false); - hsw_enable_package_c8(dev_priv); - } + if (!--power_well-count power_well-ops-disable + i915.disable_power_well) + power_well-ops-disable(dev_priv, power_well); } mutex_unlock(power_domains-lock); @@ -5401,25 +5424,35 @@ EXPORT_SYMBOL_GPL(i915_release_power_well); (POWER_DOMAIN_MASK ~BDW_ALWAYS_ON_POWER_DOMAINS) |\ BIT(POWER_DOMAIN_INIT)) +static const struct i915_power_well_ops i9xx_always_on_power_well_ops = { }; + static struct i915_power_well i9xx_always_on_power_well[] = { { .name = always-on, .always_on = 1, .domains = POWER_DOMAIN_MASK, + .ops = i9xx_always_on_power_well_ops, }, }; +static const struct i915_power_well_ops hsw_power_well_ops = { + .sync_hw = hsw_power_well_sync_hw, + .enable = hsw_power_well_enable, + .disable = hsw_power_well_disable, + .is_enabled = hsw_power_well_enabled, +}; + static struct i915_power_well hsw_power_wells[] = { { .name = always-on, .always_on = 1, .domains = HSW_ALWAYS_ON_POWER_DOMAINS, + .ops = i9xx_always_on_power_well_ops, }, { .name = display, .domains = HSW_DISPLAY_POWER_DOMAINS, - .is_enabled = hsw_power_well_enabled, - .set = hsw_set_power_well, + .ops = hsw_power_well_ops, }, }; @@ -5428,12 +5461,12 @@ static struct i915_power_well bdw_power_wells[] = { .name = always-on, .always_on = 1, .domains = BDW_ALWAYS_ON_POWER_DOMAINS, + .ops = i9xx_always_on_power_well_ops, }, { .name = display, .domains = BDW_DISPLAY_POWER_DOMAINS, - .is_enabled = hsw_power_well_enabled, - .set = hsw_set_power_well, + .ops = hsw_power_well_ops, }, }; @@ -5478,8 +5511,8 @@ static void intel_power_domains_resume(struct drm_i915_private *dev_priv) mutex_lock(power_domains-lock); for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) { - if (power_well-set) - power_well-set(dev_priv, power_well, power_well-count 0); + if (power_well-ops-sync_hw) + power_well-ops-sync_hw(dev_priv, power_well); } mutex_unlock(power_domains-lock); } @@ -5495,14 +5528,6 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv) /* For now, we need the power well to be always enabled. */ intel_display_set_init_power(dev_priv, true); intel_power_domains_resume(dev_priv); - - if (!(IS_HASWELL(dev_priv-dev) || IS_BROADWELL(dev_priv-dev))) - return; - - /* We're taking over the BIOS, so clear any requests made by it since - * the driver is in charge now. */ - if (I915_READ(HSW_PWR_WELL_BIOS) HSW_PWR_WELL_ENABLE_REQUEST) - I915_WRITE(HSW_PWR_WELL_BIOS, 0); } /* Disables PC8 so we can use the GMBUS and DP AUX interrupts. */ Looks good. Only nitpick might be to have no-op ops for the 9xx case and get rid of the checks for whether the ops function exists. Going forward that might force us to think about things a bit at least. But that's really just a style issue, the rest looks like a good change and pulls some of the HSW bits out of the generic routines. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx