Re: [Intel-gfx] [PATCH 1/2] drm/i915: Send a DPIO cmnreset during driver load or system resume.
On Wed, Sep 25, 2013 at 07:14:34AM +0800, Lee, Chon Ming wrote: On 09/24 18:18, Ville Syrjälä wrote: On Tue, Sep 24, 2013 at 03:14:27PM +0800, Chon Ming Lee wrote: Without the DPIO cmnreset, the PLL fail to lock. This should have done by BIOS. v2: Move this to intel_uncore_sanitize to allow it to get call during resume path. (Daniel) v3: Remove redundant write 0 to DPIO_CTL, and use DPIO_RESET instead of just 0x1 (Ville) Without BIOS, DPIO/render well/media well may still power gated. Turn it off. Signed-off-by: Chon Ming Lee chon.ming@intel.com --- drivers/gpu/drm/i915/i915_reg.h |9 + drivers/gpu/drm/i915/intel_uncore.c | 23 +++ 2 files changed, 32 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c4f9bef..c02f893 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -361,6 +361,15 @@ #define PUNIT_OPCODE_REG_READ6 #define PUNIT_OPCODE_REG_WRITE 7 +#define PUNIT_REG_PWRGT_CTRL 0x60 +#define PUNIT_REG_PWRGT_STATUS 0x61 +#definePUNIT_CLK_GATE1 +#definePUNIT_PWR_RESET 2 +#definePUNIT_PWR_GATE3 +#defineRENDER_PWRGT (PUNIT_PWR_GATE 0) +#defineMEDIA_PWRGT (PUNIT_PWR_GATE 2) +#defineDPIO_PWRGT(PUNIT_PWR_GATE 6) Subsys 6 seems to be one of four TX lanes, and there's also the common lane subsys, and the disp2d is one as well. RX supposedly is not relevant for display PHY, not sure why it has subsys allocated too. Anyways my point would be that shouldn't we check all subsys ie render + media + disp2d + common lane + all tx lanes? By default, the common lane + all tx lanes are not power gated during cold boot or system resume. Unless S0ix entry actually power gate it by driver, then, this will need to add to turn off it. OK. And as Imre pointed out to me the ' 6' isn't a TX lane as I claimed but the display subsystems (3). I assume that's the same thing as the disp2d block, ie. the pipes. So the DPIO in the name is wrong. It should be called display or disp2d I think. If you say the PHY side isn't power gated during cold boot, I think we can ignore it for now. So if you rename the DPIO thing, this patch should be OK. And should we maybe power gate the RX lanes always as those shouldn't be needed for display? Yes, you are correct. I believe there should be another patch to do it, to enable power gate the VLV correctly for SOix entry or exit. My plan is to power gate everything we can during runtime, not just s0ix. But that's a biger topic we can discuss once Imre gets some relevant patches ready. + #define PUNIT_REG_GPU_LFM0xd3 #define PUNIT_REG_GPU_FREQ_REQ 0xd4 #define PUNIT_REG_GPU_FREQ_STS 0xd8 diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 8649f1c..6923b4d 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -276,10 +276,33 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev) void intel_uncore_sanitize(struct drm_device *dev) { + struct drm_i915_private *dev_priv = dev-dev_private; + u32 reg_val; + intel_uncore_forcewake_reset(dev); /* BIOS often leaves RC6 enabled, but disable it for hw init */ intel_disable_gt_powersave(dev); + + /* Trigger DPIO CMN RESET and turn off power gate, require + * especially in BIOS less system + */ + if (IS_VALLEYVIEW(dev)) { + + mutex_lock(dev_priv-rps.hw_lock); + reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS); + + if (reg_val (RENDER_PWRGT | MEDIA_PWRGT | DPIO_PWRGT)) + vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0); + + mutex_unlock(dev_priv-rps.hw_lock); + + reg_val = I915_READ(DPIO_CTL); + if (!(reg_val DPIO_RESET)) { + I915_WRITE(DPIO_CTL, DPIO_RESET); + POSTING_READ(DPIO_CTL); + } + } } /* -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix VLV eDP timing
Fix the typo in previous commit for DP 1.62 divisor. drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2 Reported-by: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Chon Ming Lee chon.ming@intel.com --- drivers/gpu/drm/i915/intel_dp.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5e1de35..a8ceccf 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -59,7 +59,7 @@ static const struct dp_link_dpll pch_dpll[] = { static const struct dp_link_dpll vlv_dpll[] = { { DP_LINK_BW_1_62, - { .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 } }, + { .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 81 } }, { DP_LINK_BW_2_7, { .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 } } }; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix 1.62 DP DPLL settings for VLV
From: Ville Syrjälä ville.syrj...@linux.intel.com Use the recommended PLL dividers from VLV2_DPLL_mphy_hsdpll_frequency_table_ww6_rev1p1.xlsm. The previous values were really bogus. The 2.7 values look good however. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5e1de35..a5e4e61 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -59,7 +59,7 @@ static const struct dp_link_dpll pch_dpll[] = { static const struct dp_link_dpll vlv_dpll[] = { { DP_LINK_BW_1_62, - { .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 } }, + { .p1 = 3, .p2 = 2, .n = 5, .m1 = 3, .m2 = 81 } }, { DP_LINK_BW_2_7, { .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 } } }; -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix VLV eDP timing v2
Fix the typo in previous commit for DP 1.62 divisor. drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2 v2: sigh, the m1 div is 3. Reported-by: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Chon Ming Lee chon.ming@intel.com --- drivers/gpu/drm/i915/intel_dp.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5e1de35..a5e4e61 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -59,7 +59,7 @@ static const struct dp_link_dpll pch_dpll[] = { static const struct dp_link_dpll vlv_dpll[] = { { DP_LINK_BW_1_62, - { .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 } }, + { .p1 = 3, .p2 = 2, .n = 5, .m1 = 3, .m2 = 81 } }, { DP_LINK_BW_2_7, { .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 } } }; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7
On Wed, Sep 25, 2013 at 06:32:10AM +0200, Mario Kleiner wrote: On 23.09.13 10:38, Ville Syrjälä wrote: On Sat, Sep 21, 2013 at 12:07:36AM +0200, Mario Kleiner wrote: On 09/17/2013 10:55 PM, Daniel Vetter wrote: On Tue, Sep 17, 2013 at 9:50 PM, Peter Hurley pe...@hurleysoftware.com wrote: On 09/11/2013 03:31 PM, Peter Hurley wrote: [+cc dri-devel] On 09/11/2013 11:38 AM, Steven Rostedt wrote: On Wed, 11 Sep 2013 11:16:43 -0400 Peter Hurley pe...@hurleysoftware.com wrote: The funny part is, there's a comment there that shows that this was done even for PREEMPT_RT. Unfortunately, the call to get_scanout_position() can call functions that use the rt-mutex sleeping spin locks and it breaks there. I guess we need to ask the authors of the mainline patch exactly why that preempt_disable() is needed? The drm core associates a timestamp with each vertical blank frame #. Drm drivers can optionally support a 'high resolution' hw timestamp. The vblank frame #/timestamp tuple is user-space visible. The i915 drm driver supports a hw timestamp via this drm helper function which computes the timestamp from the crtc scan position (based on the pixel clock). For mainline, the preempt_disable/_enable() isn't actually necessary because every call tree that leads here already has preemption disabled. For -RT, the maybe i915 register spinlock (uncore.lock) should be raw? No, it should not. Note, any other lock that can be held when it is held would also need to be raw. By that, you mean any other lock that might be claimed would also need to be raw? Hopefully not any other lock already held? And by taking a quick audit of the code, I see this: spin_lock_irqsave(dev_priv-uncore.lock, irqflags); /* Reset the chip */ /* GEN6_GDRST is not in the gt power well, no need to check * for fifo space for the write or forcewake the chip for * the read */ __raw_i915_write32(dev_priv, GEN6_GDRST, GEN6_GRDOM_FULL); /* Spin waiting for the device to ack the reset request */ ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) GEN6_GRDOM_FULL) == 0, 500); That spin is unacceptable in RT with preemption and interrupts disabled. Yep. That would be bad. AFAICT the registers read in i915_get_crtc_scanoutpos() aren't included in the force-wake set, so raw reads of the registers would probably be acceptable (thus obviating the need for claiming the uncore.lock). Except that _ALL_ register access is disabled with the uncore.lock during a gpu reset. Not sure if that's meant to include crtc registers or not, or what other synchronization/serialization issues are being handled/hidden by forcing all register accesses to wait during a gpu reset. Hopefully an i915 expert can weigh in here? Daniel, Can you shed some light on whether the i915+ crtc registers (specifically those in i915_get_crtc_scanoutpos() and i915_/gm45_get_vblank_counter()) read as part of the vblank counter/timestamp handling need to be prevented during gpu reset? The depency here in the locking is a recent addition: commit a7cd1b8fea2f341b626b255d9898a5ca5fabbf0a Author: Chris Wilson ch...@chris-wilson.co.uk Date: Fri Jul 19 20:36:51 2013 +0100 drm/i915: Serialize almost all register access It's a (slightly) oversized hammer to work around a hardware issue - we could break it down to register blocks, which can be accessed concurrently, but that tends to be more fragile. But the chip really dies if you access (even just reads) the same block concurrently :( We could try break the spinlock protected section a bit in the reset handler - register access on a hung gpu tends to be ill-defined anyway. The implied wait with preemption and interrupts disabled is causing grief in -RT, but also a 4ms wait inside an irq handler seems like a bad idea. Oops, the magic code in wait_for which is just there to make the imo totally misguided kgdb support work papered over the aweful long wait in atomic context ever since we've added this in commit b6e45f866465f42b53d803b0c574da0fc508a0e9 Author: Keith Packard kei...@keithp.com Date: Fri Jan 6 11:34:04 2012 -0800 drm/i915: Move reset forcewake processing to gen6_do_reset Reverting this change should be enough (code moved obviously a bit). Cheers, Daniel Regards, Peter Hurley What's the real issue here? That the vblank timestamp needs to be an accurate measurement of a realtime event. Sleeping/servicing interrupts while reading the registers necessary to compute the timestamp would be bad too. (edit: which hopefully Mario Kleiner clarified in his reply) My point earlier was three-fold: 1. Don't need the preempt_disable() for mainline: all callers are already holding interrupt-disabling
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Skip register reads in i915_get_crtc_scanoutpos()
On Wed, Sep 25, 2013 at 6:45 AM, Mario Kleiner mario.klei...@tuebingen.mpg.de wrote: Mario, can you please take a look at these patches and ack them? I'd like to slurp them in for -next. Done in the other e-mails, with some comments. However, i'd like to test these a bit and it might take a week or two before i can get my hands on the machine with the intel card and the measurement equipment. Are we still quite far away from the next merge window? Yeah, we still have plenty time. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Fwd: linux-next: Tree for Jul 1 [ drm-intel-next: Several call-traces ]
On Wed, Sep 25, 2013 at 01:17:52PM +1000, Dave Airlie wrote: On Mon, Sep 23, 2013 at 12:14 AM, Michael S. Tsirkin m...@redhat.com wrote: On Mon, Aug 26, 2013 at 04:05:11PM +0300, Michael S. Tsirkin wrote: On Wed, Aug 21, 2013 at 11:22:58AM +0200, Sedat Dilek wrote: [ Re: [Intel-gfx] i915 producing warnings with kernel 3.11-rc5 ] Hi, saw your posting in [1]... can you try the patches below? Not sure if they apply. Did you try v3.11-rc6(+)... or drm-intel-nightly? Regards, - Sedat - [1] http://lists.freedesktop.org/archives/intel-gfx/2013-August/032154.html Same thing observed with v3.11-rc7. I still see this with 3.11. Following this message, my VGA monitor goes blank and shows an error suggesting a wrong resolution or frequency are set. Anyone? Daniel, Jesse? still ongoing I think. Yeah, I've dismissed it since the original issue in this thread is resolved. But it's something else. Note to bug reporters: Please don't me-too, but start a new thread/report - almost every gfx bug is different, even if it superficially looks the same. Michael, can you please boot with drm.debug=0xe, reproduce the problem and then attach the complete dmesg? Please make sure dmesg contains everything since boot-up, if not please increase the dmesg buffer size with log_buf_len=4M. Also please test the latest drm-intel-fixes release to check that we haven't just forgotten to send the patch to stable (there's at least one flags mismatch fix already in-flight to 3.11 stable kernels). Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix VLV eDP timing
On Wed, Sep 25, 2013 at 03:36:52PM +0800, Chon Ming Lee wrote: Fix the typo in previous commit for DP 1.62 divisor. drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2 Reported-by: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Chon Ming Lee chon.ming@intel.com Ville just submitted the same patch so I guess this is right ;-) Merged to dinq, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Improve the accuracy of get_scanout_pos on CTG+
On Wed, Sep 25, 2013 at 05:12:54AM +0200, Mario Kleiner wrote: On 23.09.13 12:02, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The DSL register increments at the start of horizontal sync, so it manages to miss the entire active portion of the current line. Improve the get_scanoutpos accuracy a bit when the scanout position is close to the start or end of vblank. We can do that by double checking the DSL value against the vblank status bit from ISR. Cc: Mario Kleiner mario.klei...@tuebingen.mpg.de Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_irq.c | 53 + 1 file changed, 53 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4f74f0c..14b42d9 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -567,6 +567,47 @@ static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe) return I915_READ(reg); } +static bool g4x_pipe_in_vblank(struct drm_device *dev, enum pipe pipe) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + uint32_t status; + + if (IS_VALLEYVIEW(dev)) { + status = pipe == PIPE_A ? + I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT : + I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT; + + return I915_READ(VLV_ISR) status; + } else if (IS_G4X(dev)) { + status = pipe == PIPE_A ? + I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT : + I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT; + + return I915_READ(ISR) status; + } else if (INTEL_INFO(dev)-gen 7) { + status = pipe == PIPE_A ? + DE_PIPEA_VBLANK : + DE_PIPEB_VBLANK; + + return I915_READ(DEISR) status; + } else { + switch (pipe) { + default: + case PIPE_A: + status = DE_PIPEA_VBLANK_IVB; + break; + case PIPE_B: + status = DE_PIPEB_VBLANK_IVB; + break; + case PIPE_C: + status = DE_PIPEC_VBLANK_IVB; + break; + } + + return I915_READ(DEISR) status; + } +} + static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, int *vpos, int *hpos) { @@ -616,6 +657,18 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, * scanout position from Display scan line register. */ position = I915_READ(PIPEDSL(pipe)) 0x1fff; + + /* +* The scanline counter increments at the leading edge +* of hsync, ie. it completely misses the active portion +* of the line. Fix up the counter at both edges of vblank +* to get a more accurate picture whether we're in vblank +* or not. +*/ + in_vbl = g4x_pipe_in_vblank(dev, pipe); + if ((in_vbl position == vbl_start - 1) || + (!in_vbl position == vbl_end - 1)) + position = (position + 1) % vtotal; } else { /* Have access to pixelcount since start of frame. * We can split this into vertical and horizontal This one i don't know. I think i can't follow the logic, but i don't know enough about the way the intel hw counts. Do you mean the counter increments when the scanline is over, instead of when it begins? Let me draw a picture of the scanline (not to scale): |X|-|___|---| horiz. active horiz. sync ^ ^ | | first pixel this is where the of the line scanline counter increments With this correction by +1 at the edges of vblank, the scanlines at vbl_start and vbl_end would be reported twice, for two successive scanline durations, that seems a bit weird and asymmetric to the rest of the scanline positions. Wouldn't it make more sense to simply always add 1 for a smaller overall error, given that hblank is shorter than the active scanout part of a scanline? Since the counter increments too late, drm_handle_vblank() may get the wrong idea ie. something like this may happen: 1. vblank irq triggered 2. drm_handle_vblank() gets called 3. i915_get_crtc_scanoutpos() returns vbl_start-1 as the scanline 4. delta_ns calculation gets confused and tries to correct for it Now, the correction you do for delta_ns should handle this, but I don't like having such kludges in common code, and we can handle it in the driver as I've demonstrated. But yeah, I suppose it can make the error slightly less stable. For some other uses (atomic page flip stuff) of the
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Skip register reads in i915_get_crtc_scanoutpos()
On Wed, Sep 25, 2013 at 04:35:56AM +0200, Mario Kleiner wrote: On 23.09.13 13:48, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com We have all the information we need in the mode structure, so going and reading it from the hardware is pointless, and slower. We never populated -get_vblank_timestamp() in the UMS case, and as that is the only way we'd ever call -get_scanout_position(), we can completely ignore UMS in i915_get_crtc_scanoutpos(). Also reorganize intel_irq_init() a bit to clarify the KMS vs. UMS situation. v2: Drop UMS code Cc: Mario Kleiner mario.klei...@tuebingen.mpg.de Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_irq.c | 43 - 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b356dc1..058f099 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -570,24 +570,29 @@ static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe) static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, int *vpos, int *hpos) { - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private; - u32 vbl = 0, position = 0; + 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); + const struct drm_display_mode *mode = intel_crtc-config.adjusted_mode; + u32 position; int vbl_start, vbl_end, htotal, vtotal; bool in_vbl = true; int ret = 0; - enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv, - pipe); - if (!i915_pipe_enabled(dev, pipe)) { + if (!intel_crtc-active) { DRM_DEBUG_DRIVER(trying to get scanoutpos for disabled pipe %c\n, pipe_name(pipe)); return 0; } - /* Get vtotal. */ - vtotal = 1 + ((I915_READ(VTOTAL(cpu_transcoder)) 16) 0x1fff); + htotal = mode-crtc_htotal; + vtotal = mode-crtc_vtotal; + vbl_start = mode-crtc_vblank_start; + vbl_end = mode-crtc_vblank_end; - if (INTEL_INFO(dev)-gen = 4) { + ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE; + + if (IS_G4X(dev) || INTEL_INFO(dev)-gen = 5) { /* No obvious pixelcount register. Only query vertical * scanout position from Display scan line register. */ @@ -605,29 +610,16 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, */ position = (I915_READ(PIPEFRAMEPIXEL(pipe)) PIPE_PIXEL_MASK) PIPE_PIXEL_SHIFT; - htotal = 1 + ((I915_READ(HTOTAL(cpu_transcoder)) 16) 0x1fff); *vpos = position / htotal; *hpos = position - (*vpos * htotal); } - /* Query vblank area. */ - vbl = I915_READ(VBLANK(cpu_transcoder)); - - /* Test position against vblank region. */ - vbl_start = vbl 0x1fff; - vbl_end = (vbl 16) 0x1fff; - - if ((*vpos vbl_start) || (*vpos vbl_end)) - in_vbl = false; + in_vbl = *vpos = vbl_start *vpos vbl_end; I think this should be a = instead of in *vpos vbl_end, if it is meant to be equal to the line it replaces (not is =), unless the original comparison was off-by-one? Yeah, I think the original was wrong, in more ways than one. It forgot to add +1 to vbl_start/end, and then it did the comparison wrong as well. + in_vbl = *vpos = vbl_start *vpos = vbl_end; Other than that, it looks good. Reviewed-by: mario.kleiner...@gmail.com /* Inside upper part of vblank area? Apply corrective offset: */ if (in_vbl (*vpos = vbl_start)) *vpos = *vpos - vtotal; - /* Readouts valid? */ - if (vbl 0) - ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE; - /* In vblank? */ if (in_vbl) ret |= DRM_SCANOUTPOS_INVBL; @@ -3148,11 +3140,10 @@ void intel_irq_init(struct drm_device *dev) dev-driver-get_vblank_counter = gm45_get_vblank_counter; } - if (drm_core_check_feature(dev, DRIVER_MODESET)) + if (drm_core_check_feature(dev, DRIVER_MODESET)) { dev-driver-get_vblank_timestamp = i915_get_vblank_timestamp; - else - dev-driver-get_vblank_timestamp = NULL; - dev-driver-get_scanout_position = i915_get_crtc_scanoutpos; + dev-driver-get_scanout_position = i915_get_crtc_scanoutpos; + } if (IS_VALLEYVIEW(dev)) { dev-driver-irq_handler = valleyview_irq_handler; -- Ville Syrjälä Intel OTC ___ Intel-gfx
Re: [Intel-gfx] Fwd: linux-next: Tree for Jul 1 [ drm-intel-next: Several call-traces ]
On Wed, Sep 25, 2013 at 09:56:52AM +0200, Daniel Vetter wrote: On Wed, Sep 25, 2013 at 01:17:52PM +1000, Dave Airlie wrote: On Mon, Sep 23, 2013 at 12:14 AM, Michael S. Tsirkin m...@redhat.com wrote: On Mon, Aug 26, 2013 at 04:05:11PM +0300, Michael S. Tsirkin wrote: On Wed, Aug 21, 2013 at 11:22:58AM +0200, Sedat Dilek wrote: [ Re: [Intel-gfx] i915 producing warnings with kernel 3.11-rc5 ] Hi, saw your posting in [1]... can you try the patches below? Not sure if they apply. Did you try v3.11-rc6(+)... or drm-intel-nightly? Regards, - Sedat - [1] http://lists.freedesktop.org/archives/intel-gfx/2013-August/032154.html Same thing observed with v3.11-rc7. I still see this with 3.11. Following this message, my VGA monitor goes blank and shows an error suggesting a wrong resolution or frequency are set. Anyone? Daniel, Jesse? still ongoing I think. Yeah, I've dismissed it since the original issue in this thread is resolved. But it's something else. Note to bug reporters: Please don't me-too, but start a new thread/report - almost every gfx bug is different, even if it superficially looks the same. Michael, can you please boot with drm.debug=0xe, reproduce the problem and then attach the complete dmesg? Please make sure dmesg contains everything since boot-up, if not please increase the dmesg buffer size with log_buf_len=4M. Also please test the latest drm-intel-fixes release to check that we haven't just forgotten to send the patch to stable (there's at least one flags mismatch fix already in-flight to 3.11 stable kernels). Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch Will do. Holidays starting now so expect some reports next week. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix VLV eDP timing v2
On Wed, Sep 25, 2013 at 03:47:51PM +0800, Chon Ming Lee wrote: Fix the typo in previous commit for DP 1.62 divisor. drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2 v2: sigh, the m1 div is 3. Reported-by: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Chon Ming Lee chon.ming@intel.com Ok, I'll try this one ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/hsw: add flag has_audio in crtc config
Hi Daniel, Thanks for your advice! Would you help working on this patch? Or can I continue after a few days, I have some Android support task these days. Regards Mengdong -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, September 24, 2013 8:48 PM To: Lin, Mengdong Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/hsw: add flag has_audio in crtc config On Tue, Sep 24, 2013 at 01:01:37AM -0400, mengdong@intel.com wrote: From: Mengdong Lin mengdong@intel.com This patch adds a flag has_audio for audio presence in intel_crtc-config. HMDI and DP encoders set this flag in their computer_config() if the external monitor supports audio. Later audio sequence will check this flag. Signed-off-by: Mengdong Lin mengdong@intel.com [snip] diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 26e162b..4bc125e 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -764,6 +764,8 @@ found: intel_dp_set_clock(encoder, pipe_config, intel_dp-link_bw); + pipe_config-has_audio = intel_dp-has_audio; + This should only be set when we actually have an audio-capable monitor and want to enable the audio output on the port. Furthermore you need to add hw state readout support for this boolean to haswell_get_pipe_config and also the relevant state check code to intel_pipe_config_compare (by adding PIPE_CONF_CHECK_I(has_audio)). Same applies to the intel_hdmi.c part ofc. Cheers, Daniel return true; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index b7d6e09..2bdc23c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -290,6 +290,7 @@ struct intel_crtc_config { struct intel_link_m_n fdi_m_n; bool ips_enabled; + bool has_audio; }; struct intel_crtc { @@ -303,7 +304,6 @@ struct intel_crtc { * some outputs connected to this crtc. */ bool active; - bool eld_vld; bool primary_disabled; /* is the crtc obscured by a plane? */ bool lowfreq_avail; struct intel_overlay *overlay; diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 2fd3fd5..09c9d69 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -864,6 +864,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, return false; } + pipe_config-has_audio = intel_hdmi-has_audio; + return true; } -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell
Hi, Is it okay to integrate this patch at first? We can check whether vblank wait can be removed safely later Thanks Mengdong -Original Message- From: Lin, Mengdong Sent: Tuesday, September 24, 2013 1:01 PM To: intel-gfx@lists.freedesktop.org Cc: Arora, MukeshX; Lin, Mengdong Subject: [PATCH v3 1/2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell From: Mukesh mukeshx.ar...@intel.com The code defines a new function intel_disable_audio() to implement the entire audio codec disable sequence, called by intel_disable_ddi() in DDI port disablement. This audio codec disbale sequence is implemented as per the recommendation of the Bspec. [Patch modified by Mendong to apply to both HDMI and DP port.] Signed-off-by: Mukesh Arora mukeshx.ar...@intel.com Signed-off-by: Mengdong Lin mengdong@intel.com Reviewed-by: Ben Widawsky b...@bwidawsk.net diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index b042ee5..bda9181 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1088,6 +1088,50 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder) I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); } +/* audio codec disable sequence, as per Bspec */ void +intel_disable_audio(struct intel_encoder *intel_encoder) { + int type = intel_encoder-type; + struct drm_encoder *encoder = intel_encoder-base; + struct drm_device *dev = encoder-dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(encoder-crtc); + int pipe = intel_crtc-pipe; + int aud_config = HSW_AUD_CFG(pipe); + u32 temp; + + /* need not disable sample fabrication because never enabled */ + + /* disable timestamps */ + temp = I915_READ(aud_config); + if (type == INTEL_OUTPUT_HDMI) + temp = ~AUD_CONFIG_N_VALUE_INDEX; + else if (type == INTEL_OUTPUT_DISPLAYPORT) + temp |= AUD_CONFIG_N_VALUE_INDEX; + else + return; + temp |= AUD_CONFIG_N_PROG_ENABLE; + temp = ~(AUD_CONFIG_UPPER_N_VALUE|AUD_CONFIG_LOWER_N_VALUE); + I915_WRITE(aud_config, temp); + POSTING_READ(aud_config); + + /* Disable ELDV and ELD buffer */ + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD); + temp = ~(AUDIO_ELD_VALID_A (pipe * 4)); + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp); + POSTING_READ(HSW_AUD_PIN_ELD_CP_VLD); + + /* Wait for 2 vertical blanks */ + intel_wait_for_vblank(dev, pipe); + intel_wait_for_vblank(dev, pipe); + + /* Disable audio PD. This is optional as per Bspec. */ + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD); + temp = ~(AUDIO_OUTPUT_ENABLE_A (pipe * 4)); + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp); + POSTING_READ(HSW_AUD_PIN_ELD_CP_VLD); +} + static void intel_enable_ddi(struct intel_encoder *intel_encoder) { struct drm_encoder *encoder = intel_encoder-base; @@ -1132,18 +1176,10 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder) struct drm_encoder *encoder = intel_encoder-base; struct drm_crtc *crtc = encoder-crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pipe = intel_crtc-pipe; int type = intel_encoder-type; - struct drm_device *dev = encoder-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - uint32_t tmp; - if (intel_crtc-eld_vld type != INTEL_OUTPUT_EDP) { - tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD); - tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) - (pipe * 4)); - I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp); - } + if (intel_crtc-eld_vld type != INTEL_OUTPUT_EDP) + intel_disable_audio(intel_encoder); if (type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); -- 1.8.1.2 ___ 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: Improve the accuracy of get_scanout_pos on CTG+
On Wed, Sep 25, 2013 at 11:11:30AM +0300, Ville Syrjälä wrote: On Wed, Sep 25, 2013 at 05:12:54AM +0200, Mario Kleiner wrote: This one i don't know. I think i can't follow the logic, but i don't know enough about the way the intel hw counts. Do you mean the counter increments when the scanline is over, instead of when it begins? Let me draw a picture of the scanline (not to scale): |X|-|___|---| horiz. active horiz. sync ^ ^ | | first pixel this is where the of the line scanline counter increments With this correction by +1 at the edges of vblank, the scanlines at vbl_start and vbl_end would be reported twice, for two successive scanline durations, that seems a bit weird and asymmetric to the rest of the scanline positions. Wouldn't it make more sense to simply always add 1 for a smaller overall error, given that hblank is shorter than the active scanout part of a scanline? Since the counter increments too late, drm_handle_vblank() may get the wrong idea ie. something like this may happen: 1. vblank irq triggered 2. drm_handle_vblank() gets called 3. i915_get_crtc_scanoutpos() returns vbl_start-1 as the scanline 4. delta_ns calculation gets confused and tries to correct for it Now, the correction you do for delta_ns should handle this, but I don't like having such kludges in common code, and we can handle it in the driver as I've demonstrated. But yeah, I suppose it can make the error slightly less stable. For some other uses (atomic page flip stuff) of the scanline position, I definitely want this correction since I need accurate information whether the position has passed vblank start. At least on modern platforms I think we should take a good look at the timestamp registers. With those the only thing we essentially need to do is to correct the spot where the timestamp is taken to make it suitable for OML_sync ... But doing such a switch is something for future work ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue
On Wed, Sep 25, 2013 at 10:29:37AM +0200, Jörg Otte wrote: Backlight can't be modified with this patch set - neither with function keys nor with the gui. It is a step backward to v3.11-rc1 Thanks for the test. Please check /sys/class/backlight, is there only one link file intel_backlight? If so, please cd inside and try modify the brightness file using echo with some values in the range of 0 - max_brightness, does the backlight level change when you echo a new value? I guess it doesn't, or you wouldn't notice problem. If indeed so, perhaps file a bug at http://bugzilla.kernel.org, Drivers/Video(DRI-Intel)? I suppose Jani and Daniel can help fix your problem. Thanks, Aaron Video driver: i915 FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012 acpi_backlight=video works. Jörg 2013/9/24 Igor Gnatenko i.gnatenko.br...@gmail.com On Tue, 2013-09-24 at 17:47 +0800, Aaron Lu wrote: v3: 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; 2 Remove unnecessary function acpi_video_unregister introduced in patch 2/3 as pointed out by Jani Nikula. v2: v1 has the subject of Rework ACPI video driver and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that. This patchset has three patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/3 for details. Then patch 2/3 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/3 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Technically, patch 2/3 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series. Aaron Lu (4): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists thinkpad-acpi: fix handle locate for video and query of _BCL drivers/acpi/internal.h | 5 +- drivers/acpi/video.c | 442 --- drivers/acpi/video_detect.c | 14 +- drivers/platform/x86/thinkpad_acpi.c | 31 ++- drivers/video/backlight/backlight.c | 31 +++ include/acpi/video.h | 2 + include/linux/backlight.h| 4 + 7 files changed, 324 insertions(+), 205 deletions(-) Excellent! I've tested this patch-set on ThinkPad X230 and backlight issue is exhausted. Thanks. -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.1-300.fc20.x86_64 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/13] drm/i915: Add a tracepoint for using a semaphore
On Mon, Sep 23, 2013 at 05:33:26PM -0300, Rodrigo Vivi wrote: From: Chris Wilson ch...@chris-wilson.co.uk So that we can find the callers who introduce a ring stall. A single ring stall is not too unwelcome, the right issue becomes when they start to interlock and prevent any concurrent work. That, however, is a little tricker to detect with a mere tracepoint! v2: Rebrand it as a ring event, rather than an object event. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com Just wondering if we would want to see the seqno(s) in the trace as well? But anyway, the patch looks fine. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem.c | 2 ++ drivers/gpu/drm/i915/i915_trace.h | 19 +++ 2 files changed, 21 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index d68cc5c..4a16491 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2614,6 +2614,8 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj, if (ret) return ret; + trace_i915_gem_ring_sync_to(from, to); + ret = to-sync_to(to, from, seqno); if (!ret) /* We use last_read_seqno because sync_to() diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 5c8e36a..48e8f07 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -233,6 +233,25 @@ TRACE_EVENT(i915_gem_evict_everything, TP_printk(dev=%d, __entry-dev) ); +TRACE_EVENT(i915_gem_ring_sync_to, + TP_PROTO(struct intel_ring_buffer *from, struct intel_ring_buffer *to), + TP_ARGS(from, to), + + TP_STRUCT__entry( + __field(u32, dev) + __field(u32, sync_from) + __field(u32, sync_to) + ), + + TP_fast_assign( +__entry-dev = from-dev-primary-index; +__entry-sync_from = from-id; +__entry-sync_to = to-id; +), + + TP_printk(dev=%u, sync-from=%u, sync-to=%u, __entry-dev, __entry-sync_from, __entry-sync_to) +); + TRACE_EVENT(i915_gem_ring_dispatch, TP_PROTO(struct intel_ring_buffer *ring, u32 flags), TP_ARGS(ring, flags), -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Show WT caching in debugfs
Add the missing cache-level to the describe_obj() function for debug and error reporting. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gpu_error.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 28c886d..e8ffd57 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -324,7 +324,7 @@ struct drm_i915_error_state { u32 dirty:1; u32 purgeable:1; s32 ring:4; - u32 cache_level:2; + u32 cache_level:3; } **active_bo, **pinned_bo; u32 *active_bo_count, *pinned_bo_count; struct intel_overlay_error_state *overlay; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index c3ff6bd..0a49b65 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1012,6 +1012,7 @@ const char *i915_cache_level_str(int type) case I915_CACHE_NONE: return uncached; case I915_CACHE_LLC: return snooped or LLC; case I915_CACHE_L3_LLC: return L3+LLC; + case I915_CACHE_WT: return WT; default: return ; } } -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/13] drm/i915: Add a tracepoint for using a semaphore
On Wed, Sep 25, 2013 at 12:34:37PM +0300, Ville Syrjälä wrote: On Mon, Sep 23, 2013 at 05:33:26PM -0300, Rodrigo Vivi wrote: From: Chris Wilson ch...@chris-wilson.co.uk So that we can find the callers who introduce a ring stall. A single ring stall is not too unwelcome, the right issue becomes when they start to interlock and prevent any concurrent work. That, however, is a little tricker to detect with a mere tracepoint! v2: Rebrand it as a ring event, rather than an object event. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com Just wondering if we would want to see the seqno(s) in the trace as well? Hm yeah, I guess the seqno we're syncing on the from ring would be useful to gauge how much parallelism there really is. Chris, care to respin? -Daniel But anyway, the patch looks fine. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem.c | 2 ++ drivers/gpu/drm/i915/i915_trace.h | 19 +++ 2 files changed, 21 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index d68cc5c..4a16491 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2614,6 +2614,8 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj, if (ret) return ret; + trace_i915_gem_ring_sync_to(from, to); + ret = to-sync_to(to, from, seqno); if (!ret) /* We use last_read_seqno because sync_to() diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 5c8e36a..48e8f07 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -233,6 +233,25 @@ TRACE_EVENT(i915_gem_evict_everything, TP_printk(dev=%d, __entry-dev) ); +TRACE_EVENT(i915_gem_ring_sync_to, + TP_PROTO(struct intel_ring_buffer *from, struct intel_ring_buffer *to), + TP_ARGS(from, to), + + TP_STRUCT__entry( +__field(u32, dev) +__field(u32, sync_from) +__field(u32, sync_to) +), + + TP_fast_assign( + __entry-dev = from-dev-primary-index; + __entry-sync_from = from-id; + __entry-sync_to = to-id; + ), + + TP_printk(dev=%u, sync-from=%u, sync-to=%u, __entry-dev, __entry-sync_from, __entry-sync_to) +); + TRACE_EVENT(i915_gem_ring_dispatch, TP_PROTO(struct intel_ring_buffer *ring, u32 flags), TP_ARGS(ring, flags), -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Show WT caching in debugfs
On Wed, Sep 25, 2013 at 10:23:19AM +0100, Chris Wilson wrote: Add the missing cache-level to the describe_obj() function for debug and error reporting. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Merged to dinq ... --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gpu_error.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 28c886d..e8ffd57 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -324,7 +324,7 @@ struct drm_i915_error_state { u32 dirty:1; u32 purgeable:1; s32 ring:4; - u32 cache_level:2; + u32 cache_level:3; ... but I wonder a bit whether we should just use the i915_cache_level enum here? Same for the ring id maybe. Otoh meh ;-) -Daniel } **active_bo, **pinned_bo; u32 *active_bo_count, *pinned_bo_count; struct intel_overlay_error_state *overlay; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index c3ff6bd..0a49b65 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1012,6 +1012,7 @@ const char *i915_cache_level_str(int type) case I915_CACHE_NONE: return uncached; case I915_CACHE_LLC: return snooped or LLC; case I915_CACHE_L3_LLC: return L3+LLC; + case I915_CACHE_WT: return WT; default: return ; } } -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Show WT caching in debugfs
On Wed, Sep 25, 2013 at 12:14:44PM +0200, Daniel Vetter wrote: On Wed, Sep 25, 2013 at 10:23:19AM +0100, Chris Wilson wrote: Add the missing cache-level to the describe_obj() function for debug and error reporting. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Merged to dinq ... --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gpu_error.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 28c886d..e8ffd57 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -324,7 +324,7 @@ struct drm_i915_error_state { u32 dirty:1; u32 purgeable:1; s32 ring:4; - u32 cache_level:2; + u32 cache_level:3; ... but I wonder a bit whether we should just use the i915_cache_level enum here? Same for the ring id maybe. Otoh meh ;-) Previous bytes semi-permanently allocated... A pittance compared to the overallocated of a libva batchbuffer, but still... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue
On Wed, 25 Sep 2013, Aaron Lu aaron...@intel.com wrote: On Wed, Sep 25, 2013 at 10:29:37AM +0200, Jörg Otte wrote: Backlight can't be modified with this patch set - neither with function keys nor with the gui. It is a step backward to v3.11-rc1 So both hotkeys and GUI work in v3.11-rc1? And v3.12-rc2? Thanks for the test. Please check /sys/class/backlight, is there only one link file intel_backlight? Indeed, are there others? fujitsu-laptop perhaps? If yes, try CONFIG_FUJITSU_LAPTOP=n or an appropriate module blacklist. Just checking, you do have CONFIG_BACKLIGHT_CLASS_DEVICE=y? Embarrassingly there was a bug in i915 fixed just recently where the backlight device wasn't registered for CONFIG_BACKLIGHT_CLASS_DEVICE=m... If so, please cd inside and try modify the brightness file using echo with some values in the range of 0 - max_brightness, does the backlight level change when you echo a new value? I guess it doesn't, or you wouldn't notice problem. If indeed so, perhaps file a bug at http://bugzilla.kernel.org, Drivers/Video(DRI-Intel)? I suppose Jani and Daniel can help fix your problem. Yup, please check the above, and report back. BR, Jani. Thanks, Aaron Video driver: i915 FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012 acpi_backlight=video works. Jörg 2013/9/24 Igor Gnatenko i.gnatenko.br...@gmail.com On Tue, 2013-09-24 at 17:47 +0800, Aaron Lu wrote: v3: 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; 2 Remove unnecessary function acpi_video_unregister introduced in patch 2/3 as pointed out by Jani Nikula. v2: v1 has the subject of Rework ACPI video driver and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that. This patchset has three patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/3 for details. Then patch 2/3 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/3 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Technically, patch 2/3 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series. Aaron Lu (4): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists thinkpad-acpi: fix handle locate for video and query of _BCL drivers/acpi/internal.h | 5 +- drivers/acpi/video.c | 442 --- drivers/acpi/video_detect.c | 14 +- drivers/platform/x86/thinkpad_acpi.c | 31 ++- drivers/video/backlight/backlight.c | 31 +++ include/acpi/video.h | 2 + include/linux/backlight.h| 4 + 7 files changed, 324 insertions(+), 205 deletions(-) Excellent! I've tested this patch-set on ThinkPad X230 and backlight issue is exhausted. Thanks. -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.1-300.fc20.x86_64 -- Jani Nikula, 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: Add a tracepoint for using a semaphore
So that we can find the callers who introduce a ring stall. A single ring stall is not too unwelcome, the right issue becomes when they start to interlock and prevent any concurrent work. That, however, is a little tricker to detect with a mere tracepoint! v2: Rebrand it as a ring event, rather than an object event. v3: Include the seqno in the tracepoint for posterity or something. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 1 + drivers/gpu/drm/i915/i915_trace.h | 26 ++ 2 files changed, 27 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 82bc029..fa3b373 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2625,6 +2625,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj, if (ret) return ret; + trace_i915_gem_ring_sync_to(from, to, seqno); ret = to-sync_to(to, from, seqno); if (!ret) /* We use last_read_seqno because sync_to() diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index daa6fdf..6e580c9 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -248,6 +248,32 @@ TRACE_EVENT(i915_gem_evict_vm, TP_printk(dev=%d, vm=%p, __entry-vm-dev-primary-index, __entry-vm) ); +TRACE_EVENT(i915_gem_ring_sync_to, + TP_PROTO(struct intel_ring_buffer *from, +struct intel_ring_buffer *to, +u32 seqno), + TP_ARGS(from, to, seqno), + + TP_STRUCT__entry( +__field(u32, dev) +__field(u32, sync_from) +__field(u32, sync_to) +__field(u32, seqno) +), + + TP_fast_assign( + __entry-dev = from-dev-primary-index; + __entry-sync_from = from-id; + __entry-sync_to = to-id; + __entry-seqno = seqno; + ), + + TP_printk(dev=%u, sync-from=%u, sync-to=%u, seqno=%u, + __entry-dev, + __entry-sync_from, __entry-sync_to, + __entry-seqno) +); + TRACE_EVENT(i915_gem_ring_dispatch, TP_PROTO(struct intel_ring_buffer *ring, u32 seqno, u32 flags), TP_ARGS(ring, seqno, flags), -- 1.8.4.rc3 ___ 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 tracepoint for using a semaphore
On Wed, Sep 25, 2013 at 11:43:28AM +0100, Chris Wilson wrote: So that we can find the callers who introduce a ring stall. A single ring stall is not too unwelcome, the right issue becomes when they start to interlock and prevent any concurrent work. That, however, is a little tricker to detect with a mere tracepoint! v2: Rebrand it as a ring event, rather than an object event. v3: Include the seqno in the tracepoint for posterity or something. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 1 + drivers/gpu/drm/i915/i915_trace.h | 26 ++ 2 files changed, 27 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 82bc029..fa3b373 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2625,6 +2625,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj, if (ret) return ret; + trace_i915_gem_ring_sync_to(from, to, seqno); ret = to-sync_to(to, from, seqno); Passing the rings in the same order to both might avoid some confusion, but I don't care enough to withhold my r-b so: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com if (!ret) /* We use last_read_seqno because sync_to() diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index daa6fdf..6e580c9 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -248,6 +248,32 @@ TRACE_EVENT(i915_gem_evict_vm, TP_printk(dev=%d, vm=%p, __entry-vm-dev-primary-index, __entry-vm) ); +TRACE_EVENT(i915_gem_ring_sync_to, + TP_PROTO(struct intel_ring_buffer *from, + struct intel_ring_buffer *to, + u32 seqno), + TP_ARGS(from, to, seqno), + + TP_STRUCT__entry( + __field(u32, dev) + __field(u32, sync_from) + __field(u32, sync_to) + __field(u32, seqno) + ), + + TP_fast_assign( +__entry-dev = from-dev-primary-index; +__entry-sync_from = from-id; +__entry-sync_to = to-id; +__entry-seqno = seqno; +), + + TP_printk(dev=%u, sync-from=%u, sync-to=%u, seqno=%u, + __entry-dev, + __entry-sync_from, __entry-sync_to, + __entry-seqno) +); + TRACE_EVENT(i915_gem_ring_dispatch, TP_PROTO(struct intel_ring_buffer *ring, u32 seqno, u32 flags), TP_ARGS(ring, seqno, flags), -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ 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 tracepoint for using a semaphore
On Wed, Sep 25, 2013 at 02:29:34PM +0300, Ville Syrjälä wrote: On Wed, Sep 25, 2013 at 11:43:28AM +0100, Chris Wilson wrote: So that we can find the callers who introduce a ring stall. A single ring stall is not too unwelcome, the right issue becomes when they start to interlock and prevent any concurrent work. That, however, is a little tricker to detect with a mere tracepoint! v2: Rebrand it as a ring event, rather than an object event. v3: Include the seqno in the tracepoint for posterity or something. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 1 + drivers/gpu/drm/i915/i915_trace.h | 26 ++ 2 files changed, 27 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 82bc029..fa3b373 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2625,6 +2625,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj, if (ret) return ret; + trace_i915_gem_ring_sync_to(from, to, seqno); ret = to-sync_to(to, from, seqno); Passing the rings in the same order to both might avoid some confusion, but I don't care enough to withhold my r-b so: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915: Use the new vm [un]bind functions
On Tue, Sep 24, 2013 at 09:58:00AM -0700, Ben Widawsky wrote: From: Ben Widawsky b...@bwidawsk.net Building on the last patch which created the new function pointers in the VM for bind/unbind, here we actually put those new function pointers to use. Split out as a separate patch to aid in review. I'm fine with squashing into the previous patch if people request it. v2: Updated to address the smart ggtt which can do aliasing as needed Make sure we bind to global gtt when mappable and fenceable. I thought we could get away without this initialy, but we cannot. v3: Make the global GTT binding explicitly use the ggtt VM for bind_vma(). While at it, use the new ggtt_vma helper (Chris) v4: Make the code support the secure dispatch flag, which requires special handling during execbuf. This was fixed (incorrectly) later in the series, but having it here earlier in the series should be perfectly acceptable. (Chris) Move do_switch over to the new, special ggtt_vma interface. v5: Don't use a local variable (or assertion) when setting the batch object to the global GTT during secure dispatch (Chris) v6: Caclulate the exec offset for the secure case (Bug fix missed on v4). (Chris) Remove redundant check for has_global_gtt_mapping, since it is done in bind_vma. v7: Remove now unused dev_priv in do_switch Don't pass the vm to ggtt_offset (error from v6 which I should have caught before sending). v8: Assert, and rework the SNB workaround (to make it a bit clearer) code to make sure the VM can't be anything but the GGTT. v9: Fixing more bugs which can't exist yet on the behest of Chris. Make sure that the batch object is properly bound, and added to the global VM's active list - for when we use non-global VMs. (Chris) Note that there is an ongoing confusion about what bugs existed, but the known bugs are fixed here. v10: Nitpicks on whitespacing etc. introduced in v9 (Chris) Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Ben Widawsky b...@bwidawsk.net Big rant here on two topics: First splitting a patch into a write new functions, but leave them as dead code (the previous patch) and kill old code, use new functions (this patch) is imo pointless. It hampers reviewing (since you can't compare old and new logic easily because it's split into two patches) and it adds zero value for bisecting: Compile-testing dead code isn't a useful additional checkpoint. Check out Ville's vlv dpll refactoring for a great split-up patch series. Second is the death of the -insert_entries/-clear_range vfuncs. We _need_ them in the internal tree and you really can't just kill them. If you want to, you need to follow three steps: 1. Create new interfaces in the public tree, use the on public platforms but keeep the old interfaces working. 2. Convert the -internal platforms over. 3. Remove old interface. Doing 3. before 2. is bonghits and will result in the internal tree being broken a bit in-between. Since I'm supposed to maintain that I'll undo the damage here to be able to do a rebase. Cheers, Daniel --- drivers/gpu/drm/i915/i915_drv.h| 9 - drivers/gpu/drm/i915/i915_gem.c| 33 +++- drivers/gpu/drm/i915/i915_gem_context.c| 6 ++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 61 -- drivers/gpu/drm/i915/i915_gem_gtt.c| 48 ++- 5 files changed, 62 insertions(+), 95 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9995cdb..e8ae8fd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2102,17 +2102,8 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, /* i915_gem_gtt.c */ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev); -void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt, - struct drm_i915_gem_object *obj, - enum i915_cache_level cache_level); -void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, - struct drm_i915_gem_object *obj); - void i915_gem_restore_gtt_mappings(struct drm_device *dev); int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj); -void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj, - enum i915_cache_level cache_level); -void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj); void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj); void i915_gem_init_global_gtt(struct drm_device *dev); void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f6c8b0e..378d4ef 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2693,12 +2693,8 @@ int i915_vma_unbind(struct i915_vma *vma)
[Intel-gfx] [PATCH] drm/i915: Fix up usage of SHRINK_STOP
In commit 81e49f811404f428a9d9a63295a0c267e802fa12 Author: Glauber Costa glom...@openvz.org Date: Wed Aug 28 10:18:13 2013 +1000 i915: bail out earlier when shrinker cannot acquire mutex SHRINK_STOP was added to tell the core shrinker code to bail out and go to the next shrinker since the i915 shrinker couldn't acquire required locks. But the SHRINK_STOP return code was added to the -count_objects callback and not the -scan_objects callback as it should have been, resulting in tons of dmesg noise like shrink_slab: i915_gem_inactive_scan+0x0/0x9c negative objects to delete nr=-x Fix discusssed with Dave Chinner. References: http://www.spinics.net/lists/intel-gfx/msg33597.html Reported-by: Knut Petersen knut_peter...@t-online.de Cc: Knut Petersen knut_peter...@t-online.de Cc: Dave Chinner da...@fromorbit.com Cc: Glauber Costa glom...@openvz.org Cc: Glauber Costa glom...@gmail.com Cc: Andrew Morton a...@linux-foundation.org Cc: Rik van Riel r...@redhat.com Cc: Mel Gorman mgor...@suse.de Cc: Johannes Weiner han...@cmpxchg.org Cc: Michal Hocko mho...@suse.cz Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index df9253d..cdfb9da 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4800,10 +4800,10 @@ i915_gem_inactive_count(struct shrinker *shrinker, struct shrink_control *sc) if (!mutex_trylock(dev-struct_mutex)) { if (!mutex_is_locked_by(dev-struct_mutex, current)) - return SHRINK_STOP; + return 0; if (dev_priv-mm.shrinker_no_lock_stealing) - return SHRINK_STOP; + return 0; unlock = false; } @@ -4901,10 +4901,10 @@ i915_gem_inactive_scan(struct shrinker *shrinker, struct shrink_control *sc) if (!mutex_trylock(dev-struct_mutex)) { if (!mutex_is_locked_by(dev-struct_mutex, current)) - return 0; + return SHRINK_STOP; if (dev_priv-mm.shrinker_no_lock_stealing) - return 0; + return SHRINK_STOP; unlock = false; } -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: we can't move_to_active before intel_ring_begin
Otherwise we can die in a fire of not-yet-allocated lazy requests when we expect them to be there: [ 4405.463113] [ cut here ] [ 4405.464769] kernel BUG at /home/user/src/linux/drivers/gpu/drm/i915/intel_ringbuffer.h:268! [ 4405.466392] invalid opcode: [#1] PREEMPT SMP [ 4405.468007] Modules linked in: snd_hda_codec_hdmi hid_generic snd_hda_codec_realtek usbhid hid iTCO_wdt snd_hda_intel psmouse i2c_i801 snd_hda_codec pcspkr snd_hwdep i915 snd_pcm snd_page_alloc snd_timer cfbfillrect snd cfbimgblt lpc_ich cfbcopyarea soundcore drm_kms_helper mfd_core evdev wmi [ 4405.471387] CPU: 5 PID: 9300 Comm: gem_storedw_bat Not tainted 3.12.0-rc2-drm_vbl+ #1 [ 4405.473047] Hardware name: /DZ77BH-55K, BIOS BHZ7710H.86A.0100.2013.0517.0942 05/17/2013 [ 4405.474712] task: 8800618d4b00 ti: 88010a806000 task.ti: 88010a806000 [ 4405.476370] RIP: 0010:[a009ffa9] [a009ffa9] i915_vma_move_to_active+0x1b9/0x1e0 [i915] [ 4405.478045] RSP: 0018:88010a807be8 EFLAGS: 00010246 [ 4405.479689] RAX: 88011a681000 RBX: 8800b364f9c0 RCX: 88011902b898 [ 4405.481321] RDX: 8800d4a1b6e0 RSI: 8800d4a1a8b8 RDI: 88011902b840 [ 4405.482932] RBP: 88010a807c08 R08: 0001 R09: [ 4405.484526] R10: 0001 R11: R12: 8800d4a1a8b8 [ 4405.486100] R13: R14: 8800d4a18000 R15: 8800b364f9c0 [ 4405.487664] FS: 7f36c1a738c0() GS:88011f34() knlGS: [ 4405.489216] CS: 0010 DS: ES: CR0: 80050033 [ 4405.490747] CR2: 7fff1b28ea30 CR3: 000119e0d000 CR4: 001407e0 [ 4405.492276] Stack: [ 4405.493774] 88010a807dd8 8800d4a1a8b8 8800d3c1c400 a00ac060 [ 4405.495276] 88010a807d28 a00aa0db 88010a807cb8 810aa4e4 [ 4405.496776] 0003 8801 8800618d50e8 81a9da00 [ 4405.498265] Call Trace: [ 4405.499735] [a00ac060] ? gen6_ppgtt_insert_entries+0x1a0/0x1a0 [i915] [ 4405.501218] [a00aa0db] i915_gem_do_execbuffer.clone.18+0x65b/0x12e0 [i915] [ 4405.502685] [810aa4e4] ? lock_release_non_nested+0xa4/0x360 [ 4405.504134] [a00ab298] i915_gem_execbuffer2+0xa8/0x290 [i915] [ 4405.505573] [813552b9] drm_ioctl+0x419/0x5c0 [ 4405.506991] [81128b12] ? handle_mm_fault+0x352/0xa00 [ 4405.508399] [a00ab1f0] ? i915_gem_execbuffer+0x490/0x490 [i915] [ 4405.509792] [8103cd2c] ? __do_page_fault+0x1fc/0x4b0 [ 4405.511170] [81165e66] do_vfs_ioctl+0x96/0x560 [ 4405.512533] [81512163] ? error_sti+0x5/0x6 [ 4405.513878] [81511d0d] ? retint_swapgs+0xe/0x13 [ 4405.515208] [811663d1] SyS_ioctl+0xa1/0xb0 [ 4405.516522] [8129ddee] ? trace_hardirqs_on_thunk+0x3a/0x3f [ 4405.517830] [81512792] system_call_fastpath+0x16/0x1b [ 4405.519119] Code: f6 83 40 01 00 00 01 0f 85 11 ff ff ff b8 01 00 00 00 f0 0f c1 03 ff c0 83 f8 01 7e 10 80 8b 40 01 00 00 01 e9 f5 fe ff ff 0f 0b 0f 0b 80 3d 61 55 08 00 01 74 e7 be 2f 00 00 00 48 c7 c7 18 03 [ 4405.520610] RIP [a009ffa9] i915_vma_move_to_active+0x1b9/0x1e0 [i915] [ 4405.522001] RSP 88010a807be8 [ 4405.523367] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x y) (0 0) [ 4405.524751] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=0 [ 4405.526152] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] to [CRTC:3] [ 4405.531096] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x y) (0 0) [ 4405.532491] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=0 [ 4405.533903] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] to [CRTC:3] [ 4405.53] ---[ end trace 53d1b708421bb5b3 ]--- This regression has been introduced in from Ben Widawsky's ppgtt/vma enabling patch drm/i915: Use the new vm [un]bind functions. This should be exercised by igt/gem_storedw_batches_loop/secure-dispatch. v2: Clarify the copypasta comment and update it to suit the new location of the move_to_active call for the batch vma. Reported-by: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Ben Widawsky b...@bwidawsk.net Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 34 -- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 88c924f..f540207 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -929,6 +929,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, drm_i915_private_t *dev_priv =
[Intel-gfx] [PATCH] drm/i915: we can't call move_to_active before intel_ring_begin
Otherwise we can die in a fire of not-yet-allocated lazy requests when we expect them to be there: [ 4405.463113] [ cut here ] [ 4405.464769] kernel BUG at /home/user/src/linux/drivers/gpu/drm/i915/intel_ringbuffer.h:268! [ 4405.466392] invalid opcode: [#1] PREEMPT SMP [ 4405.468007] Modules linked in: snd_hda_codec_hdmi hid_generic snd_hda_codec_realtek usbhid hid iTCO_wdt snd_hda_intel psmouse i2c_i801 snd_hda_codec pcspkr snd_hwdep i915 snd_pcm snd_page_alloc snd_timer cfbfillrect snd cfbimgblt lpc_ich cfbcopyarea soundcore drm_kms_helper mfd_core evdev wmi [ 4405.471387] CPU: 5 PID: 9300 Comm: gem_storedw_bat Not tainted 3.12.0-rc2-drm_vbl+ #1 [ 4405.473047] Hardware name: /DZ77BH-55K, BIOS BHZ7710H.86A.0100.2013.0517.0942 05/17/2013 [ 4405.474712] task: 8800618d4b00 ti: 88010a806000 task.ti: 88010a806000 [ 4405.476370] RIP: 0010:[a009ffa9] [a009ffa9] i915_vma_move_to_active+0x1b9/0x1e0 [i915] [ 4405.478045] RSP: 0018:88010a807be8 EFLAGS: 00010246 [ 4405.479689] RAX: 88011a681000 RBX: 8800b364f9c0 RCX: 88011902b898 [ 4405.481321] RDX: 8800d4a1b6e0 RSI: 8800d4a1a8b8 RDI: 88011902b840 [ 4405.482932] RBP: 88010a807c08 R08: 0001 R09: [ 4405.484526] R10: 0001 R11: R12: 8800d4a1a8b8 [ 4405.486100] R13: R14: 8800d4a18000 R15: 8800b364f9c0 [ 4405.487664] FS: 7f36c1a738c0() GS:88011f34() knlGS: [ 4405.489216] CS: 0010 DS: ES: CR0: 80050033 [ 4405.490747] CR2: 7fff1b28ea30 CR3: 000119e0d000 CR4: 001407e0 [ 4405.492276] Stack: [ 4405.493774] 88010a807dd8 8800d4a1a8b8 8800d3c1c400 a00ac060 [ 4405.495276] 88010a807d28 a00aa0db 88010a807cb8 810aa4e4 [ 4405.496776] 0003 8801 8800618d50e8 81a9da00 [ 4405.498265] Call Trace: [ 4405.499735] [a00ac060] ? gen6_ppgtt_insert_entries+0x1a0/0x1a0 [i915] [ 4405.501218] [a00aa0db] i915_gem_do_execbuffer.clone.18+0x65b/0x12e0 [i915] [ 4405.502685] [810aa4e4] ? lock_release_non_nested+0xa4/0x360 [ 4405.504134] [a00ab298] i915_gem_execbuffer2+0xa8/0x290 [i915] [ 4405.505573] [813552b9] drm_ioctl+0x419/0x5c0 [ 4405.506991] [81128b12] ? handle_mm_fault+0x352/0xa00 [ 4405.508399] [a00ab1f0] ? i915_gem_execbuffer+0x490/0x490 [i915] [ 4405.509792] [8103cd2c] ? __do_page_fault+0x1fc/0x4b0 [ 4405.511170] [81165e66] do_vfs_ioctl+0x96/0x560 [ 4405.512533] [81512163] ? error_sti+0x5/0x6 [ 4405.513878] [81511d0d] ? retint_swapgs+0xe/0x13 [ 4405.515208] [811663d1] SyS_ioctl+0xa1/0xb0 [ 4405.516522] [8129ddee] ? trace_hardirqs_on_thunk+0x3a/0x3f [ 4405.517830] [81512792] system_call_fastpath+0x16/0x1b [ 4405.519119] Code: f6 83 40 01 00 00 01 0f 85 11 ff ff ff b8 01 00 00 00 f0 0f c1 03 ff c0 83 f8 01 7e 10 80 8b 40 01 00 00 01 e9 f5 fe ff ff 0f 0b 0f 0b 80 3d 61 55 08 00 01 74 e7 be 2f 00 00 00 48 c7 c7 18 03 [ 4405.520610] RIP [a009ffa9] i915_vma_move_to_active+0x1b9/0x1e0 [i915] [ 4405.522001] RSP 88010a807be8 [ 4405.523367] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x y) (0 0) [ 4405.524751] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=0 [ 4405.526152] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] to [CRTC:3] [ 4405.531096] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x y) (0 0) [ 4405.532491] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=0 [ 4405.533903] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] to [CRTC:3] [ 4405.53] ---[ end trace 53d1b708421bb5b3 ]--- This regression has been introduced in from Ben Widawsky's ppgtt/vma enabling patch drm/i915: Use the new vm [un]bind functions. This should be exercised by igt/gem_storedw_batches_loop/secure-dispatch. Note that this won't fix the full ordeal for real ppgtt since the potential allocation for the batch vma could recurse into the shrinker and wreak utter havoc with our carefully reserved buffers. But for now this is good enough. The real fix involves some trickery to allocate the batch vma around the call to eb_lookup_vmas and do all the additional buffer space reserving for the batch in global gtt in the normal reservation step. But that requires quite some frobbing of various pieces of code, so definitely a 2nd step. v2: Clarify the copypasta comment and update it to suit the new location of the move_to_active call for the batch vma. v3: Pimp the commit message a bit to explain that this isn't the full fix. Reported-by: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Chris Wilson
Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue
Backlight can't be modified with this patch set - neither with function keys nor with the gui. It is a step backward to v3.11-rc1 Video driver: i915 FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012 acpi_backlight=video works. Jörg 2013/9/24 Igor Gnatenko i.gnatenko.br...@gmail.com On Tue, 2013-09-24 at 17:47 +0800, Aaron Lu wrote: v3: 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; 2 Remove unnecessary function acpi_video_unregister introduced in patch 2/3 as pointed out by Jani Nikula. v2: v1 has the subject of Rework ACPI video driver and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that. This patchset has three patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/3 for details. Then patch 2/3 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/3 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Technically, patch 2/3 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series. Aaron Lu (4): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists thinkpad-acpi: fix handle locate for video and query of _BCL drivers/acpi/internal.h | 5 +- drivers/acpi/video.c | 442 --- drivers/acpi/video_detect.c | 14 +- drivers/platform/x86/thinkpad_acpi.c | 31 ++- drivers/video/backlight/backlight.c | 31 +++ include/acpi/video.h | 2 + include/linux/backlight.h| 4 + 7 files changed, 324 insertions(+), 205 deletions(-) Excellent! I've tested this patch-set on ThinkPad X230 and backlight issue is exhausted. Thanks. -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.1-300.fc20.x86_64 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7
On Wed, 25 Sep 2013 06:32:10 +0200 Mario Kleiner mario.klei...@tuebingen.mpg.de wrote: I assume if a spin_lock_irqsave doesn't really disable interrupts on a RT kernel with normal spinlocks then local_irq_disable won't really disable interrupts either? That is incorrect. On PREEMPT_RT, you are right about spin_lock_irqsave() not disabling interrupts, but local_irq_disable() does indeed disable interrupts. Open coded local_irq_disable() usually ends up being a bug as it does nothing to synchronize interrupts from other CPUs. But most of those bugs have been removed, and there are some very legit reasons for using local_irq_disable(). PREEMPT_RT honors those. The reason PREEMPT_RT does not disable interrupts for spin_lock_irqsave(), is because that's usually used for when a interrupt uses the same spinlock. You need the irqsave() part in order to prevent a deadlock, if the code that has the spinlock gets preempted by the interrupt and that interrupt tries to grab the same lock. Because PREEMPT_RT runs interrupts as threads, we don't have that issue, because if the interrupt preempts the holder of the lock and it tries to grab the same lock, it will just block like any other thread trying to grab that lock. That is, spinlocks turn into mutexes on PREEMPT_RT. -- Steve ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7
On Wed, 25 Sep 2013 06:32:10 +0200 Mario Kleiner mario.klei...@tuebingen.mpg.de wrote: But given the new situation, your proposal is great! If we push the clock readouts into the get_scanoutpos routine, we can make this robust without causing grief for the rt people and without the need for a new separate lock for display regs in intel-kms. E.g., for intel-kms: i915_get_crtc_scanoutpos(..., ktime_t *stime, ktime_t *etime) { ... spin_lock_irqsave(...uncore.lock); preempt_disable(); *stime = ktime_get(); position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)); *etime = ktime_get(); preempt_enable(); spin_unlock_irqrestore(...uncore.lock) ... } With your patchset to reduce the amount of register reads needed in that function, and given that forcewake handling isn't needed for these registers, this should make it robust again and wouldn't need new locks. Unless ktime_get is also a bad thing to do in a preempt disabled section? ktime_get() works fine in preempt_disable sections, although it may add some latencies, but you shouldn't need to worry about it. I like this solution the best too, but if it does go in, I would ask to send us the patch for adding the preempt_disable() and we can add the preempt_disable_rt() to it. Why make mainline have a little more overhead? -- Steve ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7
Sorry for the late reply, I was at Linux Plumbers, and had a bunch of stuff to catch up on when I returned. On Sat, 21 Sep 2013 00:07:36 +0200 Mario Kleiner mario.klei...@tuebingen.mpg.de wrote: Steven, would it then be acceptable to convert that faster lock into a raw_spinlock_t or is this unacceptable? If so, the preempt_disable() could stay, right? If a spinlock is tight (not held for more than 2us on todays processors), and has little contention, than I would be fine with converting it to raw. And if that's the only lock held you could do the preempt_disable() call. In fact, if you want, you can leave the preempt_disable() out of mainline, and send a patch to us that uses preempt_disable_rt() and add a comment to it. In the -rt patch, preempt_disable_rt() is a nop when PREEMPT_RT is not set, and is preempt_disable() when it is. -- Steve ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7
On Wed, 25 Sep 2013 10:49:36 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: The preempt_disable/enable is not needed. The spinlock serves the same purpose already. As stated, that was only for the -rt patch, as spin_lock_irqsave does not disable preemption nor does it even disable interrupts. But I agree, the added preempt_disable() should be sent to us to keep in the -rt patch itself. We really appreciate that you are thinking about us :-) But something like this will just confuse the mainline folks. Having a preempt_disable_rt() would make a lot more sense (which exists in the -rt patch). As far as ktime_get(), I've used it from spinlocked/irq disabled sections and so far haven't seen it do bad things. But would be nice to get some official statement to that effect. It's just a read seqlock. It may do a few loops to get the correct time, but it's fine to have in a preempt/irq disabled section. -- Steve ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/22] drm: Move the GET_CAP macros next to the corresponding ioctl structure
It's a tiny bit more logical to find the different capabilities you can use with the GET_CAP ioctl next to the structure rather than putting them at the end of the file. v2: Tab align the litterals (David Herrmann) v3: Make it clearer that DRM_PRIME_CAP_EXPORT/IMPORT are flags of DRM_CAP_PRIME. v4: Rebase on top of latest bits (DRM_CAP_ASYNC_PAGE_FLIP was introduced) Reviewed-by: David Herrmann dh.herrm...@gmail.com (for v2) Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- include/uapi/drm/drm.h | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index ece8678..1e09e8f 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -611,6 +611,16 @@ struct drm_gem_open { __u64 size; }; +#define DRM_CAP_DUMB_BUFFER0x1 +#define DRM_CAP_VBLANK_HIGH_CRTC 0x2 +#define DRM_CAP_DUMB_PREFERRED_DEPTH 0x3 +#define DRM_CAP_DUMB_PREFER_SHADOW 0x4 +#define DRM_CAP_PRIME 0x5 +#define DRM_PRIME_CAP_IMPORT 0x1 +#define DRM_PRIME_CAP_EXPORT 0x2 +#define DRM_CAP_TIMESTAMP_MONOTONIC0x6 +#define DRM_CAP_ASYNC_PAGE_FLIP0x7 + /** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap { __u64 capability; @@ -774,17 +784,6 @@ struct drm_event_vblank { __u32 reserved; }; -#define DRM_CAP_DUMB_BUFFER 0x1 -#define DRM_CAP_VBLANK_HIGH_CRTC 0x2 -#define DRM_CAP_DUMB_PREFERRED_DEPTH 0x3 -#define DRM_CAP_DUMB_PREFER_SHADOW 0x4 -#define DRM_CAP_PRIME 0x5 -#define DRM_CAP_TIMESTAMP_MONOTONIC 0x6 -#define DRM_CAP_ASYNC_PAGE_FLIP 0x7 - -#define DRM_PRIME_CAP_IMPORT 0x1 -#define DRM_PRIME_CAP_EXPORT 0x2 - /* typedef area */ #ifndef __KERNEL__ typedef struct drm_clip_rect drm_clip_rect_t; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/22] drm: Add a SET_CLIENT_CAP ioctl
This ioctl can be used to turn some knobs in a DRM driver. The client can ask the DRM core for an alternate view of the reality: it can be useful to be able to instruct the core that the DRM client can handle new functionnality that would otherwise break current ABI. v2: Rename to ioctl from SET_CAP to SET_CLIENT_CAP (Chris Wilson) Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_drv.c | 1 + drivers/gpu/drm/drm_ioctl.c | 9 + include/drm/drmP.h | 2 ++ include/uapi/drm/drm.h | 7 +++ 4 files changed, 19 insertions(+) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index e572dd2..e79d8d9 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -69,6 +69,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SET_CLIENT_CAP, drm_setclientcap, 0), DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER), DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_setunique, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 07247e2..15da412 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -303,6 +303,15 @@ int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_priv) } /** + * Set device/driver capabilities + */ +int +drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) +{ + return -EINVAL; +} + +/** * Setversion ioctl. * * \param inode device inode. diff --git a/include/drm/drmP.h b/include/drm/drmP.h index b46fb45..dbc86b0 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1303,6 +1303,8 @@ extern int drm_getstats(struct drm_device *dev, void *data, struct drm_file *file_priv); extern int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_priv); +extern int drm_setclientcap(struct drm_device *dev, void *data, + struct drm_file *file_priv); extern int drm_setversion(struct drm_device *dev, void *data, struct drm_file *file_priv); extern int drm_noop(struct drm_device *dev, void *data, diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 1e09e8f..526baed 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -627,6 +627,12 @@ struct drm_get_cap { __u64 value; }; +/** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */ +struct drm_set_client_cap { + __u64 capability; + __u64 value; +}; + #define DRM_CLOEXEC O_CLOEXEC struct drm_prime_handle { __u32 handle; @@ -659,6 +665,7 @@ struct drm_prime_handle { #define DRM_IOCTL_GEM_FLINKDRM_IOWR(0x0a, struct drm_gem_flink) #define DRM_IOCTL_GEM_OPEN DRM_IOWR(0x0b, struct drm_gem_open) #define DRM_IOCTL_GET_CAP DRM_IOWR(0x0c, struct drm_get_cap) +#define DRM_IOCTL_SET_CLIENT_CAP DRM_IOW( 0x0d, struct drm_set_client_cap) #define DRM_IOCTL_SET_UNIQUE DRM_IOW( 0x10, struct drm_unique) #define DRM_IOCTL_AUTH_MAGIC DRM_IOW( 0x11, struct drm_auth) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/22] drm: Add HDMI stereo 3D flags to struct drm_mode_modeinfo
HDMI 1.4a defines a few layouts that we'd like to expose. This commits add new modeinfo flags that can be used to list the supported stereo layouts (when querying the list of modes) and to set a given stereo 3D mode (when setting a mode). v2: Add a drm_mode_is_stereo() helper Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- include/drm/drm_crtc.h | 14 ++ include/uapi/drm/drm_mode.h | 36 ++-- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 24f4995..825d6fa 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -180,6 +180,20 @@ struct drm_display_mode { int hsync; /* in kHz */ }; +#define DRM_MODE_FLAG_3D_MASK (DRM_MODE_FLAG_3D_FRAME_PACKING | \ +DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE | \ +DRM_MODE_FLAG_3D_LINE_ALTERNATIVE | \ +DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL | \ +DRM_MODE_FLAG_3D_L_DEPTH | \ +DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH | \ +DRM_MODE_FLAG_3D_TOP_AND_BOTTOM| \ +DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF) + +static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) +{ + return mode-flags DRM_MODE_FLAG_3D_MASK; +} + enum drm_connector_status { connector_status_connected = 1, connector_status_disconnected = 2, diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 113d324..bafe612 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -44,20 +44,28 @@ /* Video mode flags */ /* bit compatible with the xorg definitions. */ -#define DRM_MODE_FLAG_PHSYNC (10) -#define DRM_MODE_FLAG_NHSYNC (11) -#define DRM_MODE_FLAG_PVSYNC (12) -#define DRM_MODE_FLAG_NVSYNC (13) -#define DRM_MODE_FLAG_INTERLACE(14) -#define DRM_MODE_FLAG_DBLSCAN (15) -#define DRM_MODE_FLAG_CSYNC(16) -#define DRM_MODE_FLAG_PCSYNC (17) -#define DRM_MODE_FLAG_NCSYNC (18) -#define DRM_MODE_FLAG_HSKEW(19) /* hskew provided */ -#define DRM_MODE_FLAG_BCAST(110) -#define DRM_MODE_FLAG_PIXMUX (111) -#define DRM_MODE_FLAG_DBLCLK (112) -#define DRM_MODE_FLAG_CLKDIV2 (113) +#define DRM_MODE_FLAG_PHSYNC (10) +#define DRM_MODE_FLAG_NHSYNC (11) +#define DRM_MODE_FLAG_PVSYNC (12) +#define DRM_MODE_FLAG_NVSYNC (13) +#define DRM_MODE_FLAG_INTERLACE(14) +#define DRM_MODE_FLAG_DBLSCAN (15) +#define DRM_MODE_FLAG_CSYNC(16) +#define DRM_MODE_FLAG_PCSYNC (17) +#define DRM_MODE_FLAG_NCSYNC (18) +#define DRM_MODE_FLAG_HSKEW(19) /* hskew provided */ +#define DRM_MODE_FLAG_BCAST(110) +#define DRM_MODE_FLAG_PIXMUX (111) +#define DRM_MODE_FLAG_DBLCLK (112) +#define DRM_MODE_FLAG_CLKDIV2 (113) +#define DRM_MODE_FLAG_3D_FRAME_PACKING (114) +#define DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE (115) +#define DRM_MODE_FLAG_3D_LINE_ALTERNATIVE (116) +#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL (117) +#define DRM_MODE_FLAG_3D_L_DEPTH (118) +#define DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH (119) +#define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM(120) +#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (121) /* DPMS flags */ /* bit compatible with the xorg definitions. */ -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/22] drm: Add a STEREO_3D capability to the SET_CLIENT_CAP ioctl
This capability allows user space to control the delivery of modes with the 3D flags set. This is to not play games with current user space users not knowing anything about stereo 3D flags and that could try to set a mode with one or several of those bits set. So, the plan is to remove the stereo modes from the list of modes we give to DRM clients by default, and let them through if we are being told otherwise. stereo_allowed is bound to the drm_file structure to make it a per-client setting, not a global one. v2: Replace clearing 3D flags by discarding the stereo modes now that they are regular modes. v3: SET_CAP - SET_CLIENT_CAP rename (Chris Wilson) Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_crtc.c | 19 ++- drivers/gpu/drm/drm_ioctl.c | 14 +- include/drm/drmP.h | 3 +++ include/uapi/drm/drm.h | 9 + 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e79577c..454ac8a 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1581,6 +1581,19 @@ out: return ret; } +static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, +const struct drm_file *file_priv) +{ + /* +* If user-space hasn't configured the driver to expose the stereo 3D +* modes, don't expose them. +*/ + if (!file_priv-stereo_allowed drm_mode_is_stereo(mode)) + return false; + + return true; +} + /** * drm_mode_getconnector - get connector configuration * @dev: drm device for the ioctl @@ -1646,7 +1659,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, /* delayed so we get modes regardless of pre-fill_modes state */ list_for_each_entry(mode, connector-modes, head) - mode_count++; + if (drm_mode_expose_to_userspace(mode, file_priv)) + mode_count++; out_resp-connector_id = connector-base.id; out_resp-connector_type = connector-connector_type; @@ -1668,6 +1682,9 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, copied = 0; mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp-modes_ptr; list_for_each_entry(mode, connector-modes, head) { + if (!drm_mode_expose_to_userspace(mode, file_priv)) + continue; + drm_crtc_convert_to_umode(u_mode, mode); if (copy_to_user(mode_ptr + copied, u_mode, sizeof(u_mode))) { diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 15da412..dffc836 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -308,7 +308,19 @@ int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_priv) int drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) { - return -EINVAL; + struct drm_set_client_cap *req = data; + + switch (req-capability) { + case DRM_CLIENT_CAP_STEREO_3D: + if (req-value 1) + return -EINVAL; + file_priv-stereo_allowed = req-value; + break; + default: + return -EINVAL; + } + + return 0; } /** diff --git a/include/drm/drmP.h b/include/drm/drmP.h index dbc86b0..c65f496 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -433,6 +433,9 @@ struct drm_file { struct drm_master *master; /* master this node is currently associated with N.B. not always minor-master */ + /* true when the client has asked us to expose stereo 3D mode flags */ + bool stereo_allowed; + /** * fbs - List of framebuffers associated with this file. * diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 526baed..9b24d65 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -627,6 +627,15 @@ struct drm_get_cap { __u64 value; }; +/** + * DRM_CLIENT_CAP_STEREO_3D + * + * if set to 1, the DRM core will expose the stereo 3D capabilities of the + * monitor by advertising the supported 3D layouts in the flags of struct + * drm_mode_modeinfo. + */ +#define DRM_CLIENT_CAP_STEREO_3D 1 + /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */ struct drm_set_client_cap { __u64 capability; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/22] drm/edid: Expose mandatory stereo modes for HDMI sinks
For now, let's just look at the 3D_present flag of the CEA HDMI vendor block to detect if the sink supports a small list of then mandatory 3D formats. See the HDMI 1.4a 3D extraction for detail: http://www.hdmi.org/manufacturer/specification.aspx v2: Rename freq to vrefresh, make the mandatory structure a bit more compact, fix some white space issues and add a couple of const (Ville Syrjälä) Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_edid.c | 110 ++--- 1 file changed, 103 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1688ff5..52e6087 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2553,13 +2553,95 @@ do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) return modes; } +struct stereo_mandatory_mode { + int width, height, vrefresh; + unsigned int flags; +}; + +static const struct stereo_mandatory_mode stereo_mandatory_modes[] = { + { 1920, 1080, 24, + DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING }, + { 1920, 1080, 50, + DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF }, + { 1920, 1080, 60, + DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF }, + { 1280, 720, 50, + DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING }, + { 1280, 720, 60, + DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING } +}; + +static bool +stereo_match_mandatory(const struct drm_display_mode *mode, + const struct stereo_mandatory_mode *stereo_mode) +{ + unsigned int interlaced = mode-flags DRM_MODE_FLAG_INTERLACE; + + return mode-hdisplay == stereo_mode-width + mode-vdisplay == stereo_mode-height + interlaced == (stereo_mode-flags DRM_MODE_FLAG_INTERLACE) + drm_mode_vrefresh(mode) == stereo_mode-vrefresh; +} + +static const struct stereo_mandatory_mode * +hdmi_find_stereo_mandatory_mode(const struct drm_display_mode *mode) +{ + int i; + + for (i = 0; i ARRAY_SIZE(stereo_mandatory_modes); i++) + if (stereo_match_mandatory(mode, stereo_mandatory_modes[i])) + return stereo_mandatory_modes[i]; + + return NULL; +} + +static int add_hdmi_mandatory_stereo_modes(struct drm_connector *connector) +{ + struct drm_device *dev = connector-dev; + const struct drm_display_mode *mode; + struct list_head stereo_modes; + int modes = 0; + + INIT_LIST_HEAD(stereo_modes); + + list_for_each_entry(mode, connector-probed_modes, head) { + const struct stereo_mandatory_mode *mandatory; + u32 stereo_layouts, layout; + + mandatory = hdmi_find_stereo_mandatory_mode(mode); + if (!mandatory) + continue; + + stereo_layouts = mandatory-flags DRM_MODE_FLAG_3D_MASK; + do { + struct drm_display_mode *new_mode; + + layout = 1 (ffs(stereo_layouts) - 1); + stereo_layouts = ~layout; + + new_mode = drm_mode_duplicate(dev, mode); + if (!new_mode) + continue; + + new_mode-flags |= layout; + list_add_tail(new_mode-head, stereo_modes); + modes++; + } while (stereo_layouts); + } + + list_splice_tail(stereo_modes, connector-probed_modes); + + return modes; +} + /* * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block * @connector: connector corresponding to the HDMI sink * @db: start of the CEA vendor specific block * @len: length of the CEA block payload, ie. one can access up to db[len] * - * Parses the HDMI VSDB looking for modes to add to @connector. + * Parses the HDMI VSDB looking for modes to add to @connector. This function + * also adds the stereo 3d modes when applicable. */ static int do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len) @@ -2585,10 +2667,15 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len) /* the declared length is not long enough for the 2 first bytes * of additional video format capabilities */ - offset += 2; - if (len (8 + offset)) + if (len (8 + offset + 2)) goto out; + /* 3D_Present */ + offset++; + if (db[8 + offset] (1 7)) + modes += add_hdmi_mandatory_stereo_modes(connector); + + offset++; vic_len = db[8 + offset] 5; for (i = 0; i vic_len len = (9 + offset + i); i++) { @@ -2668,8 +2755,8 @@ static int add_cea_modes(struct drm_connector
[Intel-gfx] [PATCH 06/22] drm: Extract add_hdmi_mode() out of do_hdmi_vsdb_modes()
So we respect a nice design of having similar functions at the same level, in this case: do_hdmi_vsdb_modes() - add_hdmi_mandatory_stereo_modes() - add_hdmi_mode() Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_edid.c | 36 +--- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 52e6087..7366007 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2634,6 +2634,26 @@ static int add_hdmi_mandatory_stereo_modes(struct drm_connector *connector) return modes; } +static int add_hdmi_mode(struct drm_connector *connector, u8 vic) +{ + struct drm_device *dev = connector-dev; + struct drm_display_mode *newmode; + + vic--; /* VICs start at 1 */ + if (vic = ARRAY_SIZE(edid_4k_modes)) { + DRM_ERROR(Unknown HDMI VIC: %d\n, vic); + return 0; + } + + newmode = drm_mode_duplicate(dev, edid_4k_modes[vic]); + if (!newmode) + return 0; + + drm_mode_probed_add(connector, newmode); + + return 1; +} + /* * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block * @connector: connector corresponding to the HDMI sink @@ -2646,7 +2666,6 @@ static int add_hdmi_mandatory_stereo_modes(struct drm_connector *connector) static int do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len) { - struct drm_device *dev = connector-dev; int modes = 0, offset = 0, i; u8 vic_len; @@ -2679,23 +2698,10 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len) vic_len = db[8 + offset] 5; for (i = 0; i vic_len len = (9 + offset + i); i++) { - struct drm_display_mode *newmode; u8 vic; vic = db[9 + offset + i]; - - vic--; /* VICs start at 1 */ - if (vic = ARRAY_SIZE(edid_4k_modes)) { - DRM_ERROR(Unknown HDMI VIC: %d\n, vic); - continue; - } - - newmode = drm_mode_duplicate(dev, edid_4k_modes[vic]); - if (!newmode) - continue; - - drm_mode_probed_add(connector, newmode); - modes++; + modes += add_hdmi_mode(connector, vic); } out: -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/22] drm: Reject modes with more than 1 stereo flags set
When setting a stereo 3D mode, there can be only one bit set describing the layout of the frambuffer(s). So reject invalid modes early. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_crtc.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 454ac8a..090415f 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1319,6 +1319,10 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out, if (in-clock INT_MAX || in-vrefresh INT_MAX) return -ERANGE; + /* At most, 1 set bit describing the 3D layout of the mode */ + if (hweight32(in-flags DRM_MODE_FLAG_3D_MASK) 1) + return -EINVAL; + out-clock = in-clock; out-hdisplay = in-hdisplay; out-hsync_start = in-hsync_start; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/22] drm: Make drm_match_cea_mode() return the underlying 2D VIC for 3d modes
When scanning out a stereo mode, the AVI infoframe vic field has to be the underlyng 2D VIC. Before that commit, we weren't matching the CEA mode because of the extra stereo flag and then were setting the VIC field in the AVI infoframe to 0. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_edid.c | 4 ++-- drivers/gpu/drm/drm_modes.c | 18 -- include/drm/drm_crtc.h | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 0bae76d..48f1746 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2404,7 +2404,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock2)) - drm_mode_equal_no_clocks(to_match, cea_mode)) + drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode)) return mode + 1; } return 0; @@ -2453,7 +2453,7 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock2)) - drm_mode_equal_no_clocks(to_match, hdmi_mode)) + drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) return mode + 1; } return 0; diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index fc2adb6..c2cb2c8 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -830,12 +830,16 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ } else if (mode1-clock != mode2-clock) return false; - return drm_mode_equal_no_clocks(mode1, mode2); + if ((mode1-flags DRM_MODE_FLAG_3D_MASK) != + (mode2-flags DRM_MODE_FLAG_3D_MASK)) + return false; + + return drm_mode_equal_no_clocks_no_stereo(mode1, mode2); } EXPORT_SYMBOL(drm_mode_equal); /** - * drm_mode_equal_no_clocks - test modes for equality + * drm_mode_equal_no_clocks_no_stereo - test modes for equality * @mode1: first mode * @mode2: second mode * @@ -843,12 +847,13 @@ EXPORT_SYMBOL(drm_mode_equal); * None. * * Check to see if @mode1 and @mode2 are equivalent, but - * don't check the pixel clocks. + * don't check the pixel clocks nor the stereo layout. * * RETURNS: * True if the modes are equal, false otherwise. */ -bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2) +bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) { if (mode1-hdisplay == mode2-hdisplay mode1-hsync_start == mode2-hsync_start @@ -860,12 +865,13 @@ bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct mode1-vsync_end == mode2-vsync_end mode1-vtotal == mode2-vtotal mode1-vscan == mode2-vscan - mode1-flags == mode2-flags) + (mode1-flags ~DRM_MODE_FLAG_3D_MASK) == +(mode2-flags ~DRM_MODE_FLAG_3D_MASK)) return true; return false; } -EXPORT_SYMBOL(drm_mode_equal_no_clocks); +EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo); /** * drm_mode_validate_size - make sure modes adhere to size constraints diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 825d6fa..6b7f9c7 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -989,7 +989,7 @@ extern void drm_mode_config_reset(struct drm_device *dev); extern void drm_mode_config_cleanup(struct drm_device *dev); extern void drm_mode_set_name(struct drm_display_mode *mode); extern bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); -extern bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); +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); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/22] drm: Set the relevant infoframe field when scanning out a 3D mode
When scanning out a 3D mode on HDMI, we need to send an HDMI infoframe with the corresponding layout to the sink. v2: Make s3d_structure_from_display_mode() less subtle (Ville Syrjälä) Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_edid.c | 40 ++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7366007..0bae76d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3421,6 +3421,33 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, } EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); +static enum hdmi_3d_structure +s3d_structure_from_display_mode(const struct drm_display_mode *mode) +{ + u32 layout = mode-flags DRM_MODE_FLAG_3D_MASK; + + switch (layout) { + case DRM_MODE_FLAG_3D_FRAME_PACKING: + return HDMI_3D_STRUCTURE_FRAME_PACKING; + case DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE: + return HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE; + case DRM_MODE_FLAG_3D_LINE_ALTERNATIVE: + return HDMI_3D_STRUCTURE_LINE_ALTERNATIVE; + case DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL: + return HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL; + case DRM_MODE_FLAG_3D_L_DEPTH: + return HDMI_3D_STRUCTURE_L_DEPTH; + case DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH: + return HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH; + case DRM_MODE_FLAG_3D_TOP_AND_BOTTOM: + return HDMI_3D_STRUCTURE_TOP_AND_BOTTOM; + case DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF: + return HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF; + default: + return HDMI_3D_STRUCTURE_INVALID; + } +} + /** * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infoframe with * data from a DRM display mode @@ -3438,20 +3465,29 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, const struct drm_display_mode *mode) { int err; + u32 s3d_flags; u8 vic; if (!frame || !mode) return -EINVAL; vic = drm_match_hdmi_mode(mode); - if (!vic) + s3d_flags = mode-flags DRM_MODE_FLAG_3D_MASK; + + if (!vic !s3d_flags) + return -EINVAL; + + if (vic s3d_flags) return -EINVAL; err = hdmi_vendor_infoframe_init(frame); if (err 0) return err; - frame-vic = vic; + if (vic) + frame-vic = vic; + else + frame-s3d_struct = s3d_structure_from_display_mode(mode); return 0; } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/22] drm: Carry over the stereo flags when adding the alternate mode
This allows to expose the alternate clock versions of the stereo modes. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_edid.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 48f1746..c24af1d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2507,6 +2507,9 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) if (!newmode) continue; + /* Carry over the stereo flags */ + newmode-flags |= mode-flags DRM_MODE_FLAG_3D_MASK; + /* * The current mode could be either variant. Make * sure to pick the other clock for the new mode. -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/22] drm: Make exposing stereo modes a per-connector opt-in
Just like with interlaced or double scan modes, make stereo modes a per-connector opt-in to give a chance to driver authors to make it work before enabling it. Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_crtc_helper.c | 8 +++- include/drm/drm_crtc.h| 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index c722c3b..4280e37 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -76,7 +76,8 @@ static void drm_mode_validate_flag(struct drm_connector *connector, { struct drm_display_mode *mode; - if (flags == (DRM_MODE_FLAG_DBLSCAN | DRM_MODE_FLAG_INTERLACE)) + if (flags == (DRM_MODE_FLAG_DBLSCAN | DRM_MODE_FLAG_INTERLACE | + DRM_MODE_FLAG_3D_MASK)) return; list_for_each_entry(mode, connector-modes, head) { @@ -86,6 +87,9 @@ static void drm_mode_validate_flag(struct drm_connector *connector, if ((mode-flags DRM_MODE_FLAG_DBLSCAN) !(flags DRM_MODE_FLAG_DBLSCAN)) mode-status = MODE_NO_DBLESCAN; + if ((mode-flags DRM_MODE_FLAG_3D_MASK) + !(flags DRM_MODE_FLAG_3D_MASK)) + mode-status = MODE_NO_STEREO; } return; @@ -175,6 +179,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, mode_flags |= DRM_MODE_FLAG_INTERLACE; if (connector-doublescan_allowed) mode_flags |= DRM_MODE_FLAG_DBLSCAN; + if (connector-stereo_allowed) + mode_flags |= DRM_MODE_FLAG_3D_MASK; drm_mode_validate_flag(connector, mode_flags); list_for_each_entry(mode, connector-modes, head) { diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 6b7f9c7..1b69407 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -108,6 +108,7 @@ enum drm_mode_status { MODE_ONE_HEIGHT,/* only one height is supported */ MODE_ONE_SIZE, /* only one resolution is supported */ MODE_NO_REDUCED,/* monitor doesn't accept reduced blanking */ +MODE_NO_STEREO,/* stereo modes not supported */ MODE_UNVERIFIED = -3, /* mode needs to reverified */ MODE_BAD = -2, /* unspecified reason */ MODE_ERROR = -1/* error condition */ @@ -611,6 +612,7 @@ struct drm_connector { int connector_type_id; bool interlace_allowed; bool doublescan_allowed; + bool stereo_allowed; struct list_head modes; /* list of modes on this connector */ enum drm_connector_status status; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/22] drm: Remove synth_clock from struct drm_display_mode
This field is unused. Garbage collect it. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- include/drm/drm_crtc.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 011baaa..8e9716e 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -156,7 +156,6 @@ struct drm_display_mode { int height_mm; /* Actual mode we give to hw */ - int synth_clock; int crtc_hdisplay; int crtc_hblank_start; int crtc_hblank_end; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/22] drm: Factor out common CRTC viewport checking code
Both setcrtc and page_flip are checking that the framebuffer is big enough for the defined crtc viewport (x, y, hdisplay, vdisplay). Factor that code out in a single function. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_crtc.c | 70 -- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 090415f..db05864 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2063,6 +2063,37 @@ int drm_mode_set_config_internal(struct drm_mode_set *set) } EXPORT_SYMBOL(drm_mode_set_config_internal); +/* + * Checks that the framebuffer is big enough for the CRTC viewport + * (x, y, hdisplay, vdisplay) + */ +static int drm_crtc_check_viewport(const struct drm_crtc *crtc, + int x, int y, + const struct drm_display_mode *mode, + const struct drm_framebuffer *fb) + +{ + int hdisplay, vdisplay; + + hdisplay = mode-hdisplay; + vdisplay = mode-vdisplay; + + if (crtc-invert_dimensions) + swap(hdisplay, vdisplay); + + if (hdisplay fb-width || + vdisplay fb-height || + x fb-width - hdisplay || + y fb-height - vdisplay) { + DRM_DEBUG_KMS(Invalid fb size %ux%u for CRTC viewport %ux%u+%d+%d%s.\n, + fb-width, fb-height, hdisplay, vdisplay, x, y, + crtc-invert_dimensions ? (inverted) : ); + return -ENOSPC; + } + + return 0; +} + /** * drm_mode_setcrtc - set CRTC configuration * @dev: drm device for the ioctl @@ -2110,7 +2141,6 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, DRM_DEBUG_KMS([CRTC:%d]\n, crtc-base.id); if (crtc_req-mode_valid) { - int hdisplay, vdisplay; /* If we have a mode we need a framebuffer. */ /* If we pass -1, set the mode with the currently bound fb */ if (crtc_req-fb_id == -1) { @@ -2146,23 +2176,11 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V); - hdisplay = mode-hdisplay; - vdisplay = mode-vdisplay; - - if (crtc-invert_dimensions) - swap(hdisplay, vdisplay); - - if (hdisplay fb-width || - vdisplay fb-height || - crtc_req-x fb-width - hdisplay || - crtc_req-y fb-height - vdisplay) { - DRM_DEBUG_KMS(Invalid fb size %ux%u for CRTC viewport %ux%u+%d+%d%s.\n, - fb-width, fb-height, - hdisplay, vdisplay, crtc_req-x, crtc_req-y, - crtc-invert_dimensions ? (inverted) : ); - ret = -ENOSPC; + ret = drm_crtc_check_viewport(crtc, crtc_req-x, crtc_req-y, + mode, fb); + if (ret) goto out; - } + } if (crtc_req-count_connectors == 0 mode) { @@ -3579,7 +3597,6 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, struct drm_framebuffer *fb = NULL, *old_fb = NULL; struct drm_pending_vblank_event *e = NULL; unsigned long flags; - int hdisplay, vdisplay; int ret = -EINVAL; if (page_flip-flags ~DRM_MODE_PAGE_FLIP_FLAGS || @@ -3611,22 +3628,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, if (!fb) goto out; - hdisplay = crtc-mode.hdisplay; - vdisplay = crtc-mode.vdisplay; - - if (crtc-invert_dimensions) - swap(hdisplay, vdisplay); - - if (hdisplay fb-width || - vdisplay fb-height || - crtc-x fb-width - hdisplay || - crtc-y fb-height - vdisplay) { - DRM_DEBUG_KMS(Invalid fb size %ux%u for CRTC viewport %ux%u+%d+%d%s.\n, - fb-width, fb-height, hdisplay, vdisplay, crtc-x, crtc-y, - crtc-invert_dimensions ? (inverted) : ); - ret = -ENOSPC; + ret = drm_crtc_check_viewport(crtc, crtc-x, crtc-y, crtc-mode, fb); + if (ret) goto out; - } if (crtc-fb-pixel_format != fb-pixel_format) { DRM_DEBUG_KMS(Page flip is not allowed to change frame buffer format.\n); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/22] drm: Check the fb size against the adjusted v/hdisplay for stereo modes
Some stereo modes, like frame packing, need a larger CRTC viewport than the natural underlying 2D mode and thus drm_crtc_check_viewport() needs to query the adjusted mode to use the correct h/vdisplay. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_crtc.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index db05864..807309f 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2078,6 +2078,14 @@ static int drm_crtc_check_viewport(const struct drm_crtc *crtc, hdisplay = mode-hdisplay; vdisplay = mode-vdisplay; + if (drm_mode_is_stereo(mode)) { + struct drm_display_mode adjusted = *mode; + + drm_mode_set_crtcinfo(adjusted, CRTC_STEREO_DOUBLE); + hdisplay = adjusted.crtc_hdisplay; + vdisplay = adjusted.crtc_vdisplay; + } + if (crtc-invert_dimensions) swap(hdisplay, vdisplay); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/22] drm: Remove clock_index from struct drm_display_mode
This field was only accessed by the nouveau driver, but never set. So concluded we can rid of this one. Acked-by: Ben Skeggs bske...@redhat.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/nouveau/dispnv04/crtc.c | 2 -- include/drm/drm_crtc.h | 1 - 2 files changed, 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c index d4fbf11..0e3270c 100644 --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c @@ -326,8 +326,6 @@ nv_crtc_mode_set_vga(struct drm_crtc *crtc, struct drm_display_mode *mode) regp-MiscOutReg = 0x23;/* +hsync +vsync */ } - regp-MiscOutReg |= (mode-clock_index 0x03) 2; - /* * Time Sequencer */ diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 1b69407..011baaa 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -156,7 +156,6 @@ struct drm_display_mode { int height_mm; /* Actual mode we give to hw */ - int clock_index; int synth_clock; int crtc_hdisplay; int crtc_hblank_start; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 16/22] drm: Introduce a crtc_clock for struct drm_display_mode
Just like the various timings, make it possible to have a clock field what we can tweak before giving it to hardware. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_modes.c | 1 + include/drm/drm_crtc.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index c2cb2c8..ef26eb2 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -719,6 +719,7 @@ void drm_mode_set_crtcinfo(struct drm_display_mode *p, int adjust_flags) if ((p == NULL) || ((p-type DRM_MODE_TYPE_CRTC_C) == DRM_MODE_TYPE_BUILTIN)) return; + p-crtc_clock = p-clock; p-crtc_hdisplay = p-hdisplay; p-crtc_hsync_start = p-hsync_start; p-crtc_hsync_end = p-hsync_end; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8e9716e..73478bc 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -156,6 +156,7 @@ struct drm_display_mode { int height_mm; /* Actual mode we give to hw */ + int crtc_clock; /* in KHz */ int crtc_hdisplay; int crtc_hblank_start; int crtc_hblank_end; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 18/22] drm/i915: Use crtc_clock in intel_dump_crtc_timings()
We want to dump the parameters given to the hardware, so let's use crtc_clock here. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d12d563..88c641a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8310,7 +8310,7 @@ static void intel_dump_crtc_timings(const struct drm_display_mode *mode) { DRM_DEBUG_KMS(crtc timings: %d %d %d %d %d %d %d %d %d, type: 0x%x flags: 0x%x\n, - mode-clock, + mode-crtc_clock, mode-crtc_hdisplay, mode-crtc_hsync_start, mode-crtc_hsync_end, mode-crtc_htotal, mode-crtc_vdisplay, mode-crtc_vsync_start, -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 17/22] drm: Implement timings adjustments for frame packing
When using the frame packing and a single big framebuffer, some hardware requires that we do everything like if we were scanning out the big buffer itself. Let's instrument drm_mode_set_crtcinfo() to be able to do this adjustement if the driver is asking for it. v2: Use crtc_vtotal and multiply the clock by 2 instead of reconstructing it (Ville Syrjälä) Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/drm_modes.c | 22 +- include/drm/drm_crtc.h | 3 ++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index ef26eb2..b073315 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -707,12 +707,18 @@ EXPORT_SYMBOL(drm_mode_vrefresh); /** * drm_mode_set_crtcinfo - set CRTC modesetting parameters * @p: mode - * @adjust_flags: unused? (FIXME) + * @adjust_flags: a combination of adjustment flags * * LOCKING: * None. * * Setup the CRTC modesetting parameters for @p, adjusting if necessary. + * + * - The CRTC_INTERLACE_HALVE_V flag can be used to halve vertical timings of + * interlaced modes. + * - The CRTC_STEREO_DOUBLE flag can be used to compute the timings for + * buffers containing two eyes (only adjust the timings when needed, eg. for + * frame packing or side by side full). */ void drm_mode_set_crtcinfo(struct drm_display_mode *p, int adjust_flags) { @@ -753,6 +759,20 @@ void drm_mode_set_crtcinfo(struct drm_display_mode *p, int adjust_flags) p-crtc_vtotal *= p-vscan; } + if (adjust_flags CRTC_STEREO_DOUBLE) { + unsigned int layout = p-flags DRM_MODE_FLAG_3D_MASK; + + switch (layout) { + case DRM_MODE_FLAG_3D_FRAME_PACKING: + p-crtc_clock *= 2; + p-crtc_vdisplay += p-crtc_vtotal; + p-crtc_vsync_start += p-crtc_vtotal; + p-crtc_vsync_end += p-crtc_vtotal; + p-crtc_vtotal += p-crtc_vtotal; + break; + } + } + p-crtc_vblank_start = min(p-crtc_vsync_start, p-crtc_vdisplay); p-crtc_vblank_end = max(p-crtc_vsync_end, p-crtc_vtotal); p-crtc_hblank_start = min(p-crtc_hsync_start, p-crtc_hdisplay); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 73478bc..b2d08ca 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -125,7 +125,8 @@ enum drm_mode_status { .vscan = (vs), .flags = (f), \ .base.type = DRM_MODE_OBJECT_MODE -#define CRTC_INTERLACE_HALVE_V 0x1 /* halve V values for interlacing */ +#define CRTC_INTERLACE_HALVE_V (1 0) /* halve V values for interlacing */ +#define CRTC_STEREO_DOUBLE (1 1) /* adjust timings for stereo modes */ struct drm_display_mode { /* Header */ -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/22] drm/i915: Use crtc_clock with the adjusted mode
struct drm_mode_display now has a separate crtc_ version of the clock to be used when we're talking about the timings given to the harwadre (was far as the mode is concerned). This commit is really the result of a git grep adjusted_mode.*clock and replacing those by adjusted_mode.crtc_clock. No functional change. v2: Rebased on drm-intel-queued-next Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_crt.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 34 +- drivers/gpu/drm/i915/intel_dp.c | 11 +++ drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_dvo.c | 2 +- drivers/gpu/drm/i915/intel_hdmi.c| 6 +++--- drivers/gpu/drm/i915/intel_lvds.c| 2 +- drivers/gpu/drm/i915/intel_pm.c | 36 +++- drivers/gpu/drm/i915/intel_sdvo.c| 2 +- drivers/gpu/drm/i915/intel_tv.c | 2 +- 10 files changed, 56 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 019c4ce..0263629 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -117,7 +117,7 @@ static void intel_crt_get_config(struct intel_encoder *encoder, if (HAS_PCH_SPLIT(dev)) ironlake_check_encoder_dotclock(pipe_config, dotclock); - pipe_config-adjusted_mode.clock = dotclock; + pipe_config-adjusted_mode.crtc_clock = dotclock; } static void hsw_crt_get_config(struct intel_encoder *encoder, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 88c641a..3c982a4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -739,14 +739,14 @@ bool intel_crtc_active(struct drm_crtc *crtc) /* Be paranoid as we can arrive here with only partial * state retrieved from the hardware during setup. * -* We can ditch the adjusted_mode.clock check as soon +* We can ditch the adjusted_mode.crtc_clock check as soon * as Haswell has gained clock readout/fastboot support. * * We can ditch the crtc-fb check as soon as we can * properly reconstruct framebuffers. */ return intel_crtc-active crtc-fb - intel_crtc-config.adjusted_mode.clock; + intel_crtc-config.adjusted_mode.crtc_clock; } enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv, @@ -2913,7 +2913,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc) { struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; - int clock = to_intel_crtc(crtc)-config.adjusted_mode.clock; + int clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock; u32 divsel, phaseinc, auxdiv, phasedir = 0; u32 temp; @@ -2937,8 +2937,8 @@ static void lpt_program_iclkip(struct drm_crtc *crtc) phaseinc = 0x20; } else { /* The iCLK virtual clock root frequency is in MHz, -* but the adjusted_mode-clock in in KHz. To get the divisors, -* it is necessary to divide one by another, so we +* but the adjusted_mode-crtc_clock in in KHz. To get the +* divisors, it is necessary to divide one by another, so we * convert the virtual clock precision to KHz here for higher * precision. */ @@ -4148,7 +4148,7 @@ retry: */ link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10; - fdi_dotclock = adjusted_mode-clock; + fdi_dotclock = adjusted_mode-crtc_clock; lane = ironlake_get_lanes_required(fdi_dotclock, link_bw, pipe_config-pipe_bpp); @@ -4204,12 +4204,12 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, * otherwise pipe A only. */ if ((crtc-pipe == PIPE_A || IS_I915G(dev)) - adjusted_mode-clock clock_limit * 9 / 10) { + adjusted_mode-crtc_clock clock_limit * 9 / 10) { clock_limit *= 2; pipe_config-double_wide = true; } - if (adjusted_mode-clock clock_limit * 9 / 10) + if (adjusted_mode-crtc_clock clock_limit * 9 / 10) return -EINVAL; } @@ -4869,7 +4869,7 @@ static void intel_crtc_mode_from_pipe_config(struct intel_crtc *intel_crtc, crtc-mode.flags = pipe_config-adjusted_mode.flags; - crtc-mode.clock = pipe_config-adjusted_mode.clock; + crtc-mode.clock = pipe_config-adjusted_mode.crtc_clock; crtc-mode.flags |= pipe_config-adjusted_mode.flags; } @@ -7440,7 +7440,7 @@ static void
[Intel-gfx] [PATCH 20/22] drm/i915: Ask the DRM core do make stereo timings adjustements
When scanning out big stereo buffers that are actually bigger that their natural 2D counterpart, we need to blow up the crtc timings as well. Not that this is only done for frame packing as this is the only stereo mode currently exposed needing this kind of ajdustements. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3c982a4..c25622d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8435,7 +8435,7 @@ encoder_retry: pipe_config-pixel_multiplier = 1; /* Fill in default crtc timings, allow encoders to overwrite them. */ - drm_mode_set_crtcinfo(pipe_config-adjusted_mode, 0); + drm_mode_set_crtcinfo(pipe_config-adjusted_mode, CRTC_STEREO_DOUBLE); /* Pass our mode to the connectors and the CRTC to give them a chance to * adjust it according to limitations or connector properties, and also -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 22/22] drm/i915: Allow stereo modes on HDMI
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_hdmi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 1a57758..6004f9c 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1228,6 +1228,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, connector-interlace_allowed = 1; connector-doublescan_allowed = 0; + connector-stereo_allowed = 1; switch (port) { case PORT_B: -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 21/22] drm/i915: Prefer crtc_{h|v}display for pipe src dimensions
Now that we ask to adjust the crtc timings for stereo modes, the correct pipe_src_w and pipe_src_h can be found in crtc_vdisplay and crtc_hdisplay. v2: Add comment about why pipe_src_w/h need to be set afert set_crtcinfo() (Daniel Vetter) Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@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 c25622d..c94fe38 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8400,9 +8400,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, drm_mode_copy(pipe_config-adjusted_mode, mode); drm_mode_copy(pipe_config-requested_mode, mode); - pipe_config-pipe_src_w = mode-hdisplay; - pipe_config-pipe_src_h = mode-vdisplay; - pipe_config-cpu_transcoder = (enum transcoder) to_intel_crtc(crtc)-pipe; pipe_config-shared_dpll = DPLL_ID_PRIVATE; @@ -8437,6 +8434,10 @@ encoder_retry: /* Fill in default crtc timings, allow encoders to overwrite them. */ drm_mode_set_crtcinfo(pipe_config-adjusted_mode, CRTC_STEREO_DOUBLE); + /* set_crtcinfo() may have adjusted hdisplay/vdisplay */ + pipe_config-pipe_src_w = pipe_config-adjusted_mode.crtc_hdisplay; + pipe_config-pipe_src_h = pipe_config-adjusted_mode.crtc_vdisplay; + /* Pass our mode to the connectors and the CRTC to give them a chance to * adjust it according to limitations or connector properties, and also * a chance to reject the mode entirely. -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue
2013/9/25 Jani Nikula jani.nik...@linux.intel.com: On Wed, 25 Sep 2013, Aaron Lu aaron...@intel.com wrote: On Wed, Sep 25, 2013 at 10:29:37AM +0200, Jörg Otte wrote: Backlight can't be modified with this patch set - neither with function keys nor with the gui. It is a step backward to v3.11-rc1 So both hotkeys and GUI work in v3.11-rc1? And v3.12-rc2? In v3.11-rc1 it didn't work. Since a later rc (I don't remember exactly which) it did work. In v3.12-rc1/2 it does work. In v3.12-rc2 + Aaron's patch set it again doesn't work. Thanks for the test. Please check /sys/class/backlight, is there only one link file intel_backlight? Indeed, are there others? fujitsu-laptop perhaps? If yes, try CONFIG_FUJITSU_LAPTOP=n or an appropriate module blacklist. Just checking, you do have CONFIG_BACKLIGHT_CLASS_DEVICE=y? There is only one intel_backlight link. Yes, I have CONFIG_BACKLIGHT_CLASS_DEVICE=y Embarrassingly there was a bug in i915 fixed just recently where the backlight device wasn't registered for CONFIG_BACKLIGHT_CLASS_DEVICE=m... If so, please cd inside and try modify the brightness file using echo with some values in the range of 0 - max_brightness, does the backlight level change when you echo a new value? I guess it doesn't, or you wouldn't notice problem. If indeed so, perhaps file a bug at http://bugzilla.kernel.org, Drivers/Video(DRI-Intel)? I suppose Jani and Daniel can help fix your problem. Yup, please check the above, and report back. - echo 0..max_brightness brightness does not work. Video driver: i915 FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012 acpi_backlight=video works. Jörg ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 0/4] Fix Win8 backlight issue
On Wed, 25 Sep 2013, Jörg Otte jrg.o...@gmail.com wrote: 2013/9/25 Jani Nikula jani.nik...@linux.intel.com: On Wed, 25 Sep 2013, Aaron Lu aaron...@intel.com wrote: On Wed, Sep 25, 2013 at 10:29:37AM +0200, Jörg Otte wrote: Backlight can't be modified with this patch set - neither with function keys nor with the gui. It is a step backward to v3.11-rc1 So both hotkeys and GUI work in v3.11-rc1? And v3.12-rc2? In v3.11-rc1 it didn't work. Since a later rc (I don't remember exactly which) it did work. In v3.12-rc1/2 it does work. In v3.12-rc2 + Aaron's patch set it again doesn't work. Thanks for the test. Please check /sys/class/backlight, is there only one link file intel_backlight? Indeed, are there others? fujitsu-laptop perhaps? If yes, try CONFIG_FUJITSU_LAPTOP=n or an appropriate module blacklist. Just checking, you do have CONFIG_BACKLIGHT_CLASS_DEVICE=y? There is only one intel_backlight link. Yes, I have CONFIG_BACKLIGHT_CLASS_DEVICE=y Embarrassingly there was a bug in i915 fixed just recently where the backlight device wasn't registered for CONFIG_BACKLIGHT_CLASS_DEVICE=m... If so, please cd inside and try modify the brightness file using echo with some values in the range of 0 - max_brightness, does the backlight level change when you echo a new value? I guess it doesn't, or you wouldn't notice problem. If indeed so, perhaps file a bug at http://bugzilla.kernel.org, Drivers/Video(DRI-Intel)? I suppose Jani and Daniel can help fix your problem. Yup, please check the above, and report back. - echo 0..max_brightness brightness does not work. Thanks for the info. How about v3.12-rc2 without Aaron's patches? Does intel_backlight also not work there? How about the acpi_video0 (which I presume is present) sysfs interface? BR, Jani. Video driver: i915 FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012 acpi_backlight=video works. Jörg ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so
Still digging up the actual VBT info for this, but wanted to get this out there for testing, or in case others are also bugged by this. This can happen if you boot with an external display connected. In that case, the attached eDP backlight modulation frequency may not be programmed, so we need to use something (in this case the value my BIOS normally programs with just the internal display enabled). Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_panel.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 3bc89a6..a3536785 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -372,6 +372,9 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev) I915_WRITE(BLC_PWM_CTL2, dev_priv-regfile.saveBLC_PWM_CTL2); } + + if (IS_VALLEYVIEW(dev) !val) + val = 0x; } return val; @@ -629,10 +632,24 @@ set_level: spin_unlock_irqrestore(dev_priv-backlight.lock, flags); } +/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */ +static void intel_panel_init_backlight_regs(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + + if (IS_VALLEYVIEW(dev)) { + u32 cur_val = I915_READ(BLC_PWM_CTL) + ~BACKLIGHT_DUTY_CYCLE_MASK; + I915_WRITE(BLC_PWM_CTL, (0xf42 16) | cur_val); + } +} + static void intel_panel_init_backlight(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; + intel_panel_init_backlight_regs(dev); + dev_priv-backlight.level = intel_panel_get_backlight(dev); dev_priv-backlight.enabled = dev_priv-backlight.level != 0; } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Upstreaming the stereo v6 series
Hi, So this series looks like a good candidate to be merged in one tree. Beside the new 3d flags added to the mode structure, the other new API is the SET_CLIENT_CAP ioctl. It seems that this new ioctl could already be potentially useful for user space to tell us they want the primary plane explosed as a DRM plane. The i915 bits depend on the lastest drm-intel(-next-queued) so it'd be simpler to merge this series in drm-intel rather than drm-next. Options are: - merge it through drm-intel (yey!) - merge it through drm-next once the current drm-intel has been merged (will probably need a rebase because of the crtc_clock addition) - merge the drm patches through drm-next and the drm/i915 ones through drm-intel, but that'll likely need me to rebase the i915 patches as well. All in all, it'd be much easier to merge it through drm-intel (if people are happy with the current state of the series, of course). -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix pre-CTG vblank counter
From: Ville Syrjälä ville.syrj...@linux.intel.com The old style frame counter increments at the start of active video. However for i915_get_vblank_counter() we want a counter that increments at the start of vblank. Fortunately the low frame counter register also contains the pixel counter for the current frame. We can can compare that against the vblank start pixel count to determine if we need to increment the frame counter by 1 to get the correct answer. Also reorganize the function pointer assignments in intel_irq_init() a bit to avoid confusing people. Cc: Mario Kleiner mario.klei...@tuebingen.mpg.de Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- Just another small vblank related gem I forgot to polish up and send out until Imre started asking me questions about the vblank counter functions. drivers/gpu/drm/i915/i915_irq.c | 36 +++- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4b519c8..8a5eb9d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -526,7 +526,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe) drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private; unsigned long high_frame; unsigned long low_frame; - u32 high1, high2, low; + u32 high1, high2, low, pixel, vbl_start; if (!i915_pipe_enabled(dev, pipe)) { DRM_DEBUG_DRIVER(trying to get vblank count for disabled @@ -534,6 +534,24 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe) return 0; } + if (drm_core_check_feature(dev, DRIVER_MODESET)) { + struct intel_crtc *intel_crtc = + to_intel_crtc(dev_priv-pipe_to_crtc_mapping[pipe]); + const struct drm_display_mode *mode = + intel_crtc-config.adjusted_mode; + + vbl_start = mode-crtc_vblank_start * mode-crtc_htotal; + } else { + enum transcoder cpu_transcoder = + intel_pipe_to_cpu_transcoder(dev_priv, pipe); + u32 htotal; + + htotal = ((I915_READ(HTOTAL(cpu_transcoder)) 16) 0x1fff) + 1; + vbl_start = (I915_READ(VBLANK(cpu_transcoder)) 0x1fff) + 1; + + vbl_start *= htotal; + } + high_frame = PIPEFRAME(pipe); low_frame = PIPEFRAMEPIXEL(pipe); @@ -544,13 +562,20 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe) */ do { high1 = I915_READ(high_frame) PIPE_FRAME_HIGH_MASK; - low = I915_READ(low_frame) PIPE_FRAME_LOW_MASK; + low = I915_READ(low_frame); high2 = I915_READ(high_frame) PIPE_FRAME_HIGH_MASK; } while (high1 != high2); high1 = PIPE_FRAME_HIGH_SHIFT; + pixel = low PIPE_PIXEL_MASK; low = PIPE_FRAME_LOW_SHIFT; - return (high1 8) | low; + + /* +* The frame counter increments at beginning of active. +* Cook up a vblank counter by also checking the pixel +* counter against vblank start. +*/ + return ((high1 8) | low) + (pixel = vbl_start); } static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe) @@ -3197,11 +3222,12 @@ void intel_irq_init(struct drm_device *dev) pm_qos_add_request(dev_priv-pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE); - dev-driver-get_vblank_counter = i915_get_vblank_counter; - dev-max_vblank_count = 0xff; /* only 24 bits of frame count */ if (IS_G4X(dev) || INTEL_INFO(dev)-gen = 5) { dev-max_vblank_count = 0x; /* full 32 bit counter */ dev-driver-get_vblank_counter = gm45_get_vblank_counter; + } else { + dev-driver-get_vblank_counter = i915_get_vblank_counter; + dev-max_vblank_count = 0xff; /* only 24 bits of frame count */ } if (drm_core_check_feature(dev, DRIVER_MODESET)) { -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Boost RPS frequency for CPU stalls
If we encounter a situation where the CPU blocks waiting for results from the GPU, give the GPU a kick to boost its the frequency. This should work to reduce user interface stalls and to quickly promote mesa to high frequencies - but the cost is that our requested frequency stalls high (as we do not idle for long enough before rc6 to start reducing frequencies, nor are we aggressive at down clocking an underused GPU). However, this should be mitigated by rc6 itself powering off the GPU when idle, and that energy use is dependent upon the workload of the GPU in addition to its frequency (e.g. the math or sampler functions only consume power when used). Still, this is likely to adversely affect light workloads. In particular, this nearly eliminates the highly noticeable wake-up lag in animations from idle. For example, expose or workspace transitions. (However, given the situation where we fail to downclock, our requested frequency is almost always the maximum, except for Baytrail where we manually downclock upon idling. This often masks the latency of upclocking after being idle, so animations are typically smooth - at the cost of increased power consumption.) Stéphane raised the concern that this will punish good applications and reward bad applications - but due to the nature of how mesa performs its client throttling, I believe all mesa applications will be roughly equally affected. To address this concern, and to prevent applications like compositors from permanently boosting the RPS state, we ratelimit the frequency of the wait-boosts each client recieves. Unfortunately, this techinique is ineffective with Ironlake - which also has dynamic render power states and suffers just as dramatically. For Ironlake, the thermal/power headroom is shared with the CPU through Intelligent Power Sharing and the intel-ips module. This leaves us with no GPU boost frequencies available when coming out of idle, and due to hardware limitations we cannot change the arbitration between the CPU and GPU quickly enough to be effective. v2: Limit each client to receiving a single boost for each active period. Tested by QA to only marginally increase power, and to demonstrably increase throughput in games. No latency measurements yet. v3: Cater for front-buffer rendering with manual throttling. v4: Tidy up. v5: Sadly the compositor needs frequent boosts as it may never idle, but due to its picking mechanism (using ReadPixels) may require frequent waits. Those waits, along with the waits for the vrefresh swap, conspire to keep the GPU at low frequencies despite the interactive latency. To overcome this we ditch the one-boost-per-active-period and just ratelimit the number of wait-boosts each client can receive. Reported-and-tested-by: Paul Neumann paul1...@yahoo.de Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68716 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Kenneth Graunke kenn...@whitecape.org Cc: Stéphane Marchesin stephane.marche...@gmail.com Cc: Owen Taylor otay...@redhat.com Cc: Meng, Mengmeng mengmeng.m...@intel.com Cc: Zhuang, Lena lena.zhu...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 16 ++--- drivers/gpu/drm/i915/i915_drv.h | 19 +++-- drivers/gpu/drm/i915/i915_gem.c | 135 --- drivers/gpu/drm/i915/i915_irq.c | 11 --- drivers/gpu/drm/i915/intel_display.c | 3 + drivers/gpu/drm/i915/intel_drv.h | 3 + drivers/gpu/drm/i915/intel_pm.c | 42 ++- 7 files changed, 138 insertions(+), 91 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index d35de1b..57ca5af 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1816,19 +1816,11 @@ int i915_driver_unload(struct drm_device *dev) int i915_driver_open(struct drm_device *dev, struct drm_file *file) { - struct drm_i915_file_private *file_priv; - - DRM_DEBUG_DRIVER(\n); - file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL); - if (!file_priv) - return -ENOMEM; - - file-driver_priv = file_priv; - - spin_lock_init(file_priv-mm.lock); - INIT_LIST_HEAD(file_priv-mm.request_list); + int ret; - idr_init(file_priv-context_idr); + ret = i915_gem_open(dev, file); + if (ret) + return ret; return 0; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 906f4fb..ddc528b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -853,9 +853,6 @@ struct intel_gen6_power_mgmt { struct work_struct work; u32 pm_iir; - /* On vlv we need to manually drop to Vmin with a delayed work. */ - struct delayed_work vlv_work; - /* The below variables an all the rps hw state are protected by * dev-struct mutext. */ u8 cur_delay; @@ -974,6 +971,15 @@ struct i915_gem_mm { struct
Re: [Intel-gfx] [PATCH] drm/i915: we can't call move_to_active before intel_ring_begin
On Wed, Sep 25, 2013 at 05:05:15PM +0200, Daniel Vetter wrote: Otherwise we can die in a fire of not-yet-allocated lazy requests when we expect them to be there: And Chris accuses me of violating Rusty's rules. This is an extremely ugly caveat to put in an interface given that active tracking and ring dispatch should have little connection. Isn't it much simpler to just call intel_ring_alloc_seqno during move_to_active? [ 4405.463113] [ cut here ] [ 4405.464769] kernel BUG at /home/user/src/linux/drivers/gpu/drm/i915/intel_ringbuffer.h:268! [ 4405.466392] invalid opcode: [#1] PREEMPT SMP [ 4405.468007] Modules linked in: snd_hda_codec_hdmi hid_generic snd_hda_codec_realtek usbhid hid iTCO_wdt snd_hda_intel psmouse i2c_i801 snd_hda_codec pcspkr snd_hwdep i915 snd_pcm snd_page_alloc snd_timer cfbfillrect snd cfbimgblt lpc_ich cfbcopyarea soundcore drm_kms_helper mfd_core evdev wmi [ 4405.471387] CPU: 5 PID: 9300 Comm: gem_storedw_bat Not tainted 3.12.0-rc2-drm_vbl+ #1 [ 4405.473047] Hardware name: /DZ77BH-55K, BIOS BHZ7710H.86A.0100.2013.0517.0942 05/17/2013 [ 4405.474712] task: 8800618d4b00 ti: 88010a806000 task.ti: 88010a806000 [ 4405.476370] RIP: 0010:[a009ffa9] [a009ffa9] i915_vma_move_to_active+0x1b9/0x1e0 [i915] [ 4405.478045] RSP: 0018:88010a807be8 EFLAGS: 00010246 [ 4405.479689] RAX: 88011a681000 RBX: 8800b364f9c0 RCX: 88011902b898 [ 4405.481321] RDX: 8800d4a1b6e0 RSI: 8800d4a1a8b8 RDI: 88011902b840 [ 4405.482932] RBP: 88010a807c08 R08: 0001 R09: [ 4405.484526] R10: 0001 R11: R12: 8800d4a1a8b8 [ 4405.486100] R13: R14: 8800d4a18000 R15: 8800b364f9c0 [ 4405.487664] FS: 7f36c1a738c0() GS:88011f34() knlGS: [ 4405.489216] CS: 0010 DS: ES: CR0: 80050033 [ 4405.490747] CR2: 7fff1b28ea30 CR3: 000119e0d000 CR4: 001407e0 [ 4405.492276] Stack: [ 4405.493774] 88010a807dd8 8800d4a1a8b8 8800d3c1c400 a00ac060 [ 4405.495276] 88010a807d28 a00aa0db 88010a807cb8 810aa4e4 [ 4405.496776] 0003 8801 8800618d50e8 81a9da00 [ 4405.498265] Call Trace: [ 4405.499735] [a00ac060] ? gen6_ppgtt_insert_entries+0x1a0/0x1a0 [i915] [ 4405.501218] [a00aa0db] i915_gem_do_execbuffer.clone.18+0x65b/0x12e0 [i915] [ 4405.502685] [810aa4e4] ? lock_release_non_nested+0xa4/0x360 [ 4405.504134] [a00ab298] i915_gem_execbuffer2+0xa8/0x290 [i915] [ 4405.505573] [813552b9] drm_ioctl+0x419/0x5c0 [ 4405.506991] [81128b12] ? handle_mm_fault+0x352/0xa00 [ 4405.508399] [a00ab1f0] ? i915_gem_execbuffer+0x490/0x490 [i915] [ 4405.509792] [8103cd2c] ? __do_page_fault+0x1fc/0x4b0 [ 4405.511170] [81165e66] do_vfs_ioctl+0x96/0x560 [ 4405.512533] [81512163] ? error_sti+0x5/0x6 [ 4405.513878] [81511d0d] ? retint_swapgs+0xe/0x13 [ 4405.515208] [811663d1] SyS_ioctl+0xa1/0xb0 [ 4405.516522] [8129ddee] ? trace_hardirqs_on_thunk+0x3a/0x3f [ 4405.517830] [81512792] system_call_fastpath+0x16/0x1b [ 4405.519119] Code: f6 83 40 01 00 00 01 0f 85 11 ff ff ff b8 01 00 00 00 f0 0f c1 03 ff c0 83 f8 01 7e 10 80 8b 40 01 00 00 01 e9 f5 fe ff ff 0f 0b 0f 0b 80 3d 61 55 08 00 01 74 e7 be 2f 00 00 00 48 c7 c7 18 03 [ 4405.520610] RIP [a009ffa9] i915_vma_move_to_active+0x1b9/0x1e0 [i915] [ 4405.522001] RSP 88010a807be8 [ 4405.523367] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x y) (0 0) [ 4405.524751] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=0 [ 4405.526152] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] to [CRTC:3] [ 4405.531096] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x y) (0 0) [ 4405.532491] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=0 [ 4405.533903] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] to [CRTC:3] [ 4405.53] ---[ end trace 53d1b708421bb5b3 ]--- This regression has been introduced in from Ben Widawsky's ppgtt/vma enabling patch drm/i915: Use the new vm [un]bind functions. This should be exercised by igt/gem_storedw_batches_loop/secure-dispatch. Note that this won't fix the full ordeal for real ppgtt since the potential allocation for the batch vma could recurse into the shrinker and wreak utter havoc with our carefully reserved buffers. But for now this is good enough. The real fix involves some trickery to allocate the batch vma around the call to eb_lookup_vmas and do all the additional buffer space reserving for the batch in global gtt in the normal
[Intel-gfx] [PATCH 3/3] drm/i915: Tweak RPS thresholds to more aggressively downclock
After applying wait-boost we often find ourselves stuck at higher clocks than required. The current threshold value requires the GPU to be continuously and completely idle for 313ms before it is dropped by one bin. Conversely, we require the GPU to be busy for an average of 90% over a 84ms period before we upclock. So the current thresholds almost never downclock the GPU, and respond very slowly to sudden demands for more power. It is easy to observe that we currently lock into the wrong bin and both underperform in benchmarks and consume more power than optimal (just by repeating the task and measuring the different results). An alternative approach, as discussed in the bspec, is to use a continuous threshold for upclocking, and an average value for downclocking. This is good for quickly detecting and reacting to state changes within a frame, however it fails with the common throttling method of waiting upon the outstanding frame - at least it is difficult to choose a threshold that works well at 15,000fps and at 60fps. So continue to use average busy/idle loads to determine frequency change. v2: Use 3 power zones to keep frequencies low in steady-state mostly idle (e.g. scrolling, interactive 2D drawing), and frequencies high for demanding games. In between those end-states, we use a fast-reclocking algorithm to converge more quickly on the desired bin. v3: Bug fixes - make sure we reset adj after switching power zones. v4: Tune - drop the continuous busy thresholds as it prevents us from choosing the right frequency for glxgears style swap benchmarks. Instead the goal is to be able to find the right clocks irrespective of the wait-boost. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Kenneth Graunke kenn...@whitecape.org Cc: Stéphane Marchesin stephane.marche...@gmail.com Cc: Owen Taylor otay...@redhat.com Cc: Meng, Mengmeng mengmeng.m...@intel.com Cc: Zhuang, Lena lena.zhu...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 5 ++ drivers/gpu/drm/i915/i915_irq.c | 46 ++ drivers/gpu/drm/i915/i915_reg.h | 2 +- drivers/gpu/drm/i915/intel_pm.c | 137 ++-- 4 files changed, 143 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ddc528b..fd0a28d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -859,8 +859,13 @@ struct intel_gen6_power_mgmt { u8 min_delay; u8 max_delay; u8 rpe_delay; + u8 rp1_delay; + u8 rp0_delay; u8 hw_max; + int last_adj; + enum { LOW_POWER, BETWEEN, HIGH_POWER } power; + struct delayed_work delayed_resume_work; /* diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6ee5572..418ad64 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -818,7 +818,7 @@ static void gen6_pm_rps_work(struct work_struct *work) drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t, rps.work); u32 pm_iir; - u8 new_delay; + int new_delay, adj; spin_lock_irq(dev_priv-irq_lock); pm_iir = dev_priv-rps.pm_iir; @@ -835,29 +835,49 @@ static void gen6_pm_rps_work(struct work_struct *work) mutex_lock(dev_priv-rps.hw_lock); + adj = dev_priv-rps.last_adj; if (pm_iir GEN6_PM_RP_UP_THRESHOLD) { - new_delay = dev_priv-rps.cur_delay + 1; + if (adj 0) + adj *= 2; + else + adj = 1; + new_delay = dev_priv-rps.cur_delay + adj; /* * For better performance, jump directly * to RPe if we're below it. */ - if (IS_VALLEYVIEW(dev_priv-dev) - dev_priv-rps.cur_delay dev_priv-rps.rpe_delay) + if (new_delay dev_priv-rps.rpe_delay) + new_delay = dev_priv-rps.rpe_delay; + } else if (pm_iir GEN6_PM_RP_DOWN_TIMEOUT) { + if (dev_priv-rps.cur_delay dev_priv-rps.rpe_delay) new_delay = dev_priv-rps.rpe_delay; - } else - new_delay = dev_priv-rps.cur_delay - 1; + else + new_delay = dev_priv-rps.min_delay; + adj = 0; + } else if (pm_iir GEN6_PM_RP_DOWN_THRESHOLD) { + if (adj 0) + adj *= 2; + else + adj = -1; + new_delay = dev_priv-rps.cur_delay + adj; + } else { /* unknown event */ + new_delay = dev_priv-rps.cur_delay; + } /* sysfs frequency interfaces may have snuck in while servicing the * interrupt */ - if (new_delay = dev_priv-rps.min_delay - new_delay = dev_priv-rps.max_delay) { -
[Intel-gfx] [PATCH 1/3] drm/i915: Fix __wait_seqno to use true infinite timeouts
When we switched to always using a timeout in conjunction with wait_seqno, we lost the ability to detect missed interrupts. Since, we have had issues with interrupts on a number of generations, and they are required to be delivered in a timely fashion for a smooth UX, it is important that we do log errors found in the wild and prevent the display stalling for upwards of 1s every time the seqno interrupt is missed. Rather than continue to fix up the timeouts to work around the interface impedence in wait_event_*(), open code the combination of wait_event[_interruptible][_timeout], and use the exposed timer to poll for seqno should we detect a lost interrupt. v2: In order to satisfy the debug requirement of logging missed interrupts with the real world requirments of making machines work even if interrupts are hosed, we revert to polling after detecting a missed interrupt. v3: Throw in a debugfs interface to simulate broken hw not reporting interrupts. v4: s/EGAIN/EAGAIN/ (Imre) Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Reviewed-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 68 drivers/gpu/drm/i915/i915_drv.h | 6 ++ drivers/gpu/drm/i915/i915_gem.c | 114 -- drivers/gpu/drm/i915/i915_gpu_error.c | 1 + drivers/gpu/drm/i915/i915_irq.c | 11 ++-- 5 files changed, 149 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index fcfa9884..0c339f0 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1897,6 +1897,72 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_ring_stop_fops, i915_ring_stop_get, i915_ring_stop_set, 0x%08llx\n); +static int +i915_ring_missed_irq_get(void *data, u64 *val) +{ + struct drm_device *dev = data; + drm_i915_private_t *dev_priv = dev-dev_private; + + *val = dev_priv-gpu_error.missed_irq_rings; + return 0; +} + +static int +i915_ring_missed_irq_set(void *data, u64 val) +{ + struct drm_device *dev = data; + struct drm_i915_private *dev_priv = dev-dev_private; + int ret; + + /* Lock against concurrent debugfs callers */ + ret = mutex_lock_interruptible(dev-struct_mutex); + if (ret) + return ret; + dev_priv-gpu_error.missed_irq_rings = val; + mutex_unlock(dev-struct_mutex); + + return 0; +} + +DEFINE_SIMPLE_ATTRIBUTE(i915_ring_missed_irq_fops, + i915_ring_missed_irq_get, i915_ring_missed_irq_set, + 0x%08llx\n); + +static int +i915_ring_test_irq_get(void *data, u64 *val) +{ + struct drm_device *dev = data; + drm_i915_private_t *dev_priv = dev-dev_private; + + *val = dev_priv-gpu_error.test_irq_rings; + + return 0; +} + +static int +i915_ring_test_irq_set(void *data, u64 val) +{ + struct drm_device *dev = data; + struct drm_i915_private *dev_priv = dev-dev_private; + int ret; + + DRM_DEBUG_DRIVER(Masking interrupts on rings 0x%08llx\n, val); + + /* Lock against concurrent debugfs callers */ + ret = mutex_lock_interruptible(dev-struct_mutex); + if (ret) + return ret; + + dev_priv-gpu_error.test_irq_rings = val; + mutex_unlock(dev-struct_mutex); + + return 0; +} + +DEFINE_SIMPLE_ATTRIBUTE(i915_ring_test_irq_fops, + i915_ring_test_irq_get, i915_ring_test_irq_set, + 0x%08llx\n); + #define DROP_UNBOUND 0x1 #define DROP_BOUND 0x2 #define DROP_RETIRE 0x4 @@ -2290,6 +2356,8 @@ static struct i915_debugfs_files { {i915_min_freq, i915_min_freq_fops}, {i915_cache_sharing, i915_cache_sharing_fops}, {i915_ring_stop, i915_ring_stop_fops}, + {i915_ring_missed_irq, i915_ring_missed_irq_fops}, + {i915_ring_test_irq, i915_ring_test_irq_fops}, {i915_gem_drop_caches, i915_drop_caches_fops}, {i915_error_state, i915_error_state_fops}, {i915_next_seqno, i915_next_seqno_fops}, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e8ffd57..906f4fb 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1022,6 +1022,9 @@ struct i915_gpu_error { struct drm_i915_error_state *first_error; struct work_struct work; + + unsigned long missed_irq_rings; + /** * State variable and reset counter controlling the reset flow * @@ -1060,6 +1063,9 @@ struct i915_gpu_error { /* For gpu hang simulation. */ unsigned int stop_rings; + + /* For missed irq/seqno simulation. */ + unsigned int test_irq_rings; }; enum modeset_restore { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index fa3b373..40e1293 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++
Re: [Intel-gfx] [PATCH] drm/i915: we can't call move_to_active before intel_ring_begin
On Wed, Sep 25, 2013 at 10:11:35AM -0700, Ben Widawsky wrote: On Wed, Sep 25, 2013 at 05:05:15PM +0200, Daniel Vetter wrote: Otherwise we can die in a fire of not-yet-allocated lazy requests when we expect them to be there: And Chris accuses me of violating Rusty's rules. This is an extremely ugly caveat to put in an interface given that active tracking and ring dispatch should have little connection. Isn't it much simpler to just call intel_ring_alloc_seqno during move_to_active? Daniel keeps harping on about the potential allocation resulting in the shrinker stealing vma and pages. All because we cheated and dropped the pin early during reservation... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ 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: Use the new vm [un]bind functions
On Wed, Sep 25, 2013 at 01:42:21PM +0200, Daniel Vetter wrote: On Tue, Sep 24, 2013 at 09:58:00AM -0700, Ben Widawsky wrote: From: Ben Widawsky b...@bwidawsk.net Building on the last patch which created the new function pointers in the VM for bind/unbind, here we actually put those new function pointers to use. Split out as a separate patch to aid in review. I'm fine with squashing into the previous patch if people request it. v2: Updated to address the smart ggtt which can do aliasing as needed Make sure we bind to global gtt when mappable and fenceable. I thought we could get away without this initialy, but we cannot. v3: Make the global GTT binding explicitly use the ggtt VM for bind_vma(). While at it, use the new ggtt_vma helper (Chris) v4: Make the code support the secure dispatch flag, which requires special handling during execbuf. This was fixed (incorrectly) later in the series, but having it here earlier in the series should be perfectly acceptable. (Chris) Move do_switch over to the new, special ggtt_vma interface. v5: Don't use a local variable (or assertion) when setting the batch object to the global GTT during secure dispatch (Chris) v6: Caclulate the exec offset for the secure case (Bug fix missed on v4). (Chris) Remove redundant check for has_global_gtt_mapping, since it is done in bind_vma. v7: Remove now unused dev_priv in do_switch Don't pass the vm to ggtt_offset (error from v6 which I should have caught before sending). v8: Assert, and rework the SNB workaround (to make it a bit clearer) code to make sure the VM can't be anything but the GGTT. v9: Fixing more bugs which can't exist yet on the behest of Chris. Make sure that the batch object is properly bound, and added to the global VM's active list - for when we use non-global VMs. (Chris) Note that there is an ongoing confusion about what bugs existed, but the known bugs are fixed here. v10: Nitpicks on whitespacing etc. introduced in v9 (Chris) Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Ben Widawsky b...@bwidawsk.net Second is the death of the -insert_entries/-clear_range vfuncs. We _need_ them in the internal tree and you really can't just kill them. If you want to, you need to follow three steps: 1. Create new interfaces in the public tree, use the on public platforms but keeep the old interfaces working. 2. Convert the -internal platforms over. 3. Remove old interface. Doing 3. before 2. is bonghits and will result in the internal tree being broken a bit in-between. Since I'm supposed to maintain that I'll undo the damage here to be able to do a rebase. Cheers, Daniel As I've now stated multiple times over the course of the last 5 months - I am fine with you not merging this. It's the right thing to do, but it seems neither you or I have time to do it in the right way. Sometimes reality gets in the way what's desirable. --- drivers/gpu/drm/i915/i915_drv.h| 9 - drivers/gpu/drm/i915/i915_gem.c| 33 +++- drivers/gpu/drm/i915/i915_gem_context.c| 6 ++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 61 -- drivers/gpu/drm/i915/i915_gem_gtt.c| 48 ++- 5 files changed, 62 insertions(+), 95 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9995cdb..e8ae8fd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2102,17 +2102,8 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, /* i915_gem_gtt.c */ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev); -void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt, - struct drm_i915_gem_object *obj, - enum i915_cache_level cache_level); -void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, - struct drm_i915_gem_object *obj); - void i915_gem_restore_gtt_mappings(struct drm_device *dev); int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj); -void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj, - enum i915_cache_level cache_level); -void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj); void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj); void i915_gem_init_global_gtt(struct drm_device *dev); void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f6c8b0e..378d4ef 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2693,12 +2693,8 @@ int i915_vma_unbind(struct i915_vma *vma) trace_i915_vma_unbind(vma); - if
Re: [Intel-gfx] [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so
On Wed, 25 Sep 2013, Jesse Barnes jbar...@virtuousgeek.org wrote: Still digging up the actual VBT info for this, but wanted to get this out there for testing, or in case others are also bugged by this. I had a look at this a few weeks back. The VBT value for max backlight is in Hz (as is the value you get through opregion) and transforming that into the value the registers eat needs some digging. I tried, but none of the real world examples of VBT and PWM freq matched any of that, so I moved on... This can happen if you boot with an external display connected. In that case, the attached eDP backlight modulation frequency may not be programmed, so we need to use something (in this case the value my BIOS normally programs with just the internal display enabled). Something similar is required for non-vlv ChromeOS stuff too AFAIK. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_panel.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 3bc89a6..a3536785 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -372,6 +372,9 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev) I915_WRITE(BLC_PWM_CTL2, dev_priv-regfile.saveBLC_PWM_CTL2); } + + if (IS_VALLEYVIEW(dev) !val) + val = 0x; Huh, that's a lot... why don't you use the same value here and below? In fact, it should be sufficient to do the hack right here, as this gets called through intel_panel_setup_backlight(). Then again, this hole function is a kludge... :/ } return val; @@ -629,10 +632,24 @@ set_level: spin_unlock_irqrestore(dev_priv-backlight.lock, flags); } +/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */ +static void intel_panel_init_backlight_regs(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + + if (IS_VALLEYVIEW(dev)) { + u32 cur_val = I915_READ(BLC_PWM_CTL) + ~BACKLIGHT_DUTY_CYCLE_MASK; That should be without the NOT, right? + I915_WRITE(BLC_PWM_CTL, (0xf42 16) | cur_val); + } +} + static void intel_panel_init_backlight(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; + intel_panel_init_backlight_regs(dev); + dev_priv-backlight.level = intel_panel_get_backlight(dev); dev_priv-backlight.enabled = dev_priv-backlight.level != 0; } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so
On Wed, 25 Sep 2013 20:18:39 +0300 Jani Nikula jani.nik...@linux.intel.com wrote: On Wed, 25 Sep 2013, Jesse Barnes jbar...@virtuousgeek.org wrote: Still digging up the actual VBT info for this, but wanted to get this out there for testing, or in case others are also bugged by this. I had a look at this a few weeks back. The VBT value for max backlight is in Hz (as is the value you get through opregion) and transforming that into the value the registers eat needs some digging. I tried, but none of the real world examples of VBT and PWM freq matched any of that, so I moved on... This can happen if you boot with an external display connected. In that case, the attached eDP backlight modulation frequency may not be programmed, so we need to use something (in this case the value my BIOS normally programs with just the internal display enabled). Something similar is required for non-vlv ChromeOS stuff too AFAIK. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_panel.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 3bc89a6..a3536785 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -372,6 +372,9 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev) I915_WRITE(BLC_PWM_CTL2, dev_priv-regfile.saveBLC_PWM_CTL2); } + + if (IS_VALLEYVIEW(dev) !val) + val = 0x; Huh, that's a lot... why don't you use the same value here and below? In fact, it should be sufficient to do the hack right here, as this gets called through intel_panel_setup_backlight(). Then again, this hole function is a kludge... :/ Simply doing it here isn't enough, because we never actually set the modulation freq bits. But yes, we could use the same magic value in both places. Seems to work either way though. } return val; @@ -629,10 +632,24 @@ set_level: spin_unlock_irqrestore(dev_priv-backlight.lock, flags); } +/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */ +static void intel_panel_init_backlight_regs(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + + if (IS_VALLEYVIEW(dev)) { + u32 cur_val = I915_READ(BLC_PWM_CTL) + ~BACKLIGHT_DUTY_CYCLE_MASK; That should be without the NOT, right? No I'm explicitly ignoring the low 16 bits, trying to set the bits for the modulation frequency. Without those, setting a value just gives you a blinking display as the backlight goes on and off about once per 500ms. -- 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: add VLV specific clock_get function v3
On Mon, 23 Sep 2013 21:01:44 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Fri, Sep 20, 2013 at 11:29:32AM -0700, Jesse Barnes wrote: Calculation is a little different than other platforms. v2: update to use port_clock instead rebase on top of Ville's changes v3: update to new port_clock semantics - don't divide by pixel_multiplier (Ville) References: https://bugs.freedesktop.org/show_bug.cgi?id=67345 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c | 33 - 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7eecf37..e5c9c1c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5048,6 +5048,34 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc, I915_READ(LVDS) LVDS_BORDER_ENABLE; } +static void vlv_crtc_clock_get(struct intel_crtc *crtc, + struct intel_crtc_config *pipe_config) +{ + struct drm_device *dev = crtc-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + int pipe = pipe_config-cpu_transcoder; + intel_clock_t clock; + u32 mdiv; + int refclk = 10, fastclk, update_rate; + + mutex_lock(dev_priv-dpio_lock); + mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe)); + mutex_unlock(dev_priv-dpio_lock); + + clock.m1 = (mdiv DPIO_M1DIV_SHIFT) 7; + clock.m2 = mdiv DPIO_M2DIV_MASK; + clock.n = (mdiv DPIO_N_SHIFT) 0xf; + clock.p1 = (mdiv DPIO_P1_SHIFT) 7; + clock.p2 = (mdiv DPIO_P2_SHIFT) 0x1f; + + update_rate = refclk / clock.n; + clock.vco = update_rate * clock.m1 * clock.m2; + fastclk = clock.vco / clock.p1 / clock.p2; + clock.dot = (2 * fastclk); + + pipe_config-port_clock = clock.dot / 10; Looks like it should get roughly the right answer, but I don't see much point in all the intermediate results. If you want to keep some of them for clarity, then I think this should be enough: clock.vco = refclk * clock.m / clock.n; clock.dot = clock.vco / clock.p; /* fast clock */ pipe_config-port_clock = clock.dot / 5; Although calling the fast clock dot is a bit wrong, but I think it's fine here, especially as it matches what I have in mind for vlv_find_best_dpll(). I had to rewrite that sucker a bit to make it compatible with my brain ;) I'll send a patch for that ASAP. The other advantage of having it all spelled out like this is that it matches the freq calculation spreadsheet I sourced it from. So I'd prefer to keep this as is, if I can have your r-b anyway. :) 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 v3 3/4] ACPI / video: Do not register backlight if win8 and native interface exists
On Tuesday, September 24, 2013 05:47:31 PM Aaron Lu wrote: According to Matthew Garrett, Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows [8] doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems. So for Win8 systems, if there is native backlight control interface registered by GPU driver, ACPI video will not register its own. For users who prefer to keep ACPI video's backlight interface, the existing kernel cmdline option acpi_backlight=video can be used. I think the idea is to use the aggressive default for now and we can switch the default back to the current behavior before the merge window in case there are too many problems with it? Rafael Signed-off-by: Aaron Lu aaron...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com Tested-by: Yves-Alexis Perez cor...@debian.org --- drivers/acpi/internal.h | 5 ++--- drivers/acpi/video.c| 10 +- drivers/acpi/video_detect.c | 14 -- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 20f4233..453ae8d 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device *adev, Video -- */ #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) -bool acpi_video_backlight_quirks(void); -#else -static inline bool acpi_video_backlight_quirks(void) { return false; } +bool acpi_osi_is_win8(void); +bool acpi_video_verify_backlight_support(void); #endif #endif /* _ACPI_INTERNAL_H_ */ diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 3bd1eaa..343db59 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device *device, int event) unsigned long long level_current, level_next; int result = -EINVAL; - /* no warning message if acpi_backlight=vendor is used */ - if (!acpi_video_backlight_support()) + /* no warning message if acpi_backlight=vendor or a quirk is used */ + if (!acpi_video_verify_backlight_support()) return 0; if (!device-brightness) @@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video, static int acpi_video_bus_start_devices(struct acpi_video_bus *video) { return acpi_video_bus_DOS(video, 0, - acpi_video_backlight_quirks() ? 1 : 0); + acpi_osi_is_win8() ? 1 : 0); } static int acpi_video_bus_stop_devices(struct acpi_video_bus *video) { return acpi_video_bus_DOS(video, 0, - acpi_video_backlight_quirks() ? 0 : 1); + acpi_osi_is_win8() ? 0 : 1); } static void acpi_video_bus_notify(struct acpi_device *device, u32 event) @@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, void *context, static void acpi_video_dev_register_backlight(struct acpi_video_device *device) { - if (acpi_video_backlight_support()) { + if (acpi_video_verify_backlight_support()) { struct backlight_properties props; struct pci_dev *pdev; acpi_handle acpi_parent; diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 940edbf..23d7d26 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -37,6 +37,7 @@ #include linux/acpi.h #include linux/dmi.h #include linux/pci.h +#include linux/backlight.h #include internal.h @@ -233,11 +234,11 @@ static void acpi_video_caps_check(void) acpi_video_get_capabilities(NULL); } -bool acpi_video_backlight_quirks(void) +bool acpi_osi_is_win8(void) { return acpi_gbl_osi_data = ACPI_OSI_WIN_8; } -EXPORT_SYMBOL(acpi_video_backlight_quirks); +EXPORT_SYMBOL(acpi_osi_is_win8); /* Promote the vendor interface instead of the generic video module. * This function allow DMI blacklists to be implemented by externals @@ -283,6 +284,15 @@ int acpi_video_backlight_support(void) } EXPORT_SYMBOL(acpi_video_backlight_support); +bool acpi_video_verify_backlight_support(void) +{ + if (!(acpi_video_support ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO) + acpi_osi_is_win8() backlight_device_registered(BACKLIGHT_RAW)) + return false; + return acpi_video_backlight_support(); +} +EXPORT_SYMBOL(acpi_video_verify_backlight_support); + /* * Use acpi_backlight=vendor/video to
Re: [Intel-gfx] [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so
On Wed, 25 Sep 2013 20:18:39 +0300 Jani Nikula jani.nik...@linux.intel.com wrote: On Wed, 25 Sep 2013, Jesse Barnes jbar...@virtuousgeek.org wrote: Still digging up the actual VBT info for this, but wanted to get this out there for testing, or in case others are also bugged by this. I had a look at this a few weeks back. The VBT value for max backlight is in Hz (as is the value you get through opregion) and transforming that into the value the registers eat needs some digging. I tried, but none of the real world examples of VBT and PWM freq matched any of that, so I moved on... This can happen if you boot with an external display connected. In that case, the attached eDP backlight modulation frequency may not be programmed, so we need to use something (in this case the value my BIOS normally programs with just the internal display enabled). Something similar is required for non-vlv ChromeOS stuff too AFAIK. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_panel.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 3bc89a6..a3536785 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -372,6 +372,9 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev) I915_WRITE(BLC_PWM_CTL2, dev_priv-regfile.saveBLC_PWM_CTL2); } + + if (IS_VALLEYVIEW(dev) !val) + val = 0x; Huh, that's a lot... why don't you use the same value here and below? In fact, it should be sufficient to do the hack right here, as this gets called through intel_panel_setup_backlight(). Then again, this hole function is a kludge... :/ } return val; @@ -629,10 +632,24 @@ set_level: spin_unlock_irqrestore(dev_priv-backlight.lock, flags); } +/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */ +static void intel_panel_init_backlight_regs(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + + if (IS_VALLEYVIEW(dev)) { + u32 cur_val = I915_READ(BLC_PWM_CTL) + ~BACKLIGHT_DUTY_CYCLE_MASK; That should be without the NOT, right? Oops yes, rather than preserving the value I'm about to clobber... :) -- 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: hack to init backlight regs if BIOS fails to do so
On Wed, Sep 25, 2013 at 08:18:39PM +0300, Jani Nikula wrote: On Wed, 25 Sep 2013, Jesse Barnes jbar...@virtuousgeek.org wrote: Still digging up the actual VBT info for this, but wanted to get this out there for testing, or in case others are also bugged by this. I had a look at this a few weeks back. The VBT value for max backlight is in Hz (as is the value you get through opregion) and transforming that into the value the registers eat needs some digging. I tried, but none of the real world examples of VBT and PWM freq matched any of that, so I moved on... This can happen if you boot with an external display connected. In that case, the attached eDP backlight modulation frequency may not be programmed, so we need to use something (in this case the value my BIOS normally programs with just the internal display enabled). Something similar is required for non-vlv ChromeOS stuff too AFAIK. Afaik ChromeOS doesn't have a vbt, so I think we need to shovel some failsafe (yeah, failsafe and backlight doesn't compute, I know) default into the regs in case all else fails. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix pre-CTG vblank counter
On Wed, Sep 25, 2013 at 07:55:26PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The old style frame counter increments at the start of active video. However for i915_get_vblank_counter() we want a counter that increments at the start of vblank. Fortunately the low frame counter register also contains the pixel counter for the current frame. We can can compare that against the vblank start pixel count to determine if we need to increment the frame counter by 1 to get the correct answer. Also reorganize the function pointer assignments in intel_irq_init() a bit to avoid confusing people. Cc: Mario Kleiner mario.klei...@tuebingen.mpg.de Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- Just another small vblank related gem I forgot to polish up and send out until Imre started asking me questions about the vblank counter functions. Hm, I've thought the magic fixup code does take care of that for us? But I agree that we should do this explicitly ... This could explain some of the strange vblank timestamp off failures QA has reported (if there's too much delay and the fixup doesn't fire any more), care to attach this patch to the relevant bug reports? Searching for ts jitter + pre-gen5 should be good enough. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Mask LPSP to get PSR working even with Power Well in use by audio.
Power Well in use forces constantly PSR to exit. On recent Kernel I noticed that PSR Performance Counter was always 0 indicating that PSR was never really achieved. By masking LPSP, PSR can work normally and save power on Haswell. Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 79c14e2..2c555f9 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1467,7 +1467,7 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp) /* Avoid continuous PSR exit by masking memup and hpd */ I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP | - EDP_PSR_DEBUG_MASK_HPD); + EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP); intel_dp-psr_setup_done = true; } -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Mask LPSP to get PSR working even with Power Well in use by audio.
Hi Daniel, Please consider to accept this for -fixes, otherwise PSR will never work on Haswell on 3.12. Thanks, Rodrigo. On Wed, Sep 25, 2013 at 5:51 PM, Rodrigo Vivi rodrigo.v...@gmail.com wrote: Power Well in use forces constantly PSR to exit. On recent Kernel I noticed that PSR Performance Counter was always 0 indicating that PSR was never really achieved. By masking LPSP, PSR can work normally and save power on Haswell. Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 79c14e2..2c555f9 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1467,7 +1467,7 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp) /* Avoid continuous PSR exit by masking memup and hpd */ I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP | - EDP_PSR_DEBUG_MASK_HPD); + EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP); intel_dp-psr_setup_done = true; } -- 1.7.11.7 -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: add VLV specific clock_get function v3
On Wed, Sep 25, 2013 at 10:38:33AM -0700, Jesse Barnes wrote: On Mon, 23 Sep 2013 21:01:44 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Fri, Sep 20, 2013 at 11:29:32AM -0700, Jesse Barnes wrote: Calculation is a little different than other platforms. v2: update to use port_clock instead rebase on top of Ville's changes v3: update to new port_clock semantics - don't divide by pixel_multiplier (Ville) References: https://bugs.freedesktop.org/show_bug.cgi?id=67345 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c | 33 - 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7eecf37..e5c9c1c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5048,6 +5048,34 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc, I915_READ(LVDS) LVDS_BORDER_ENABLE; } +static void vlv_crtc_clock_get(struct intel_crtc *crtc, +struct intel_crtc_config *pipe_config) +{ + struct drm_device *dev = crtc-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + int pipe = pipe_config-cpu_transcoder; + intel_clock_t clock; + u32 mdiv; + int refclk = 10, fastclk, update_rate; + + mutex_lock(dev_priv-dpio_lock); + mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe)); + mutex_unlock(dev_priv-dpio_lock); + + clock.m1 = (mdiv DPIO_M1DIV_SHIFT) 7; + clock.m2 = mdiv DPIO_M2DIV_MASK; + clock.n = (mdiv DPIO_N_SHIFT) 0xf; + clock.p1 = (mdiv DPIO_P1_SHIFT) 7; + clock.p2 = (mdiv DPIO_P2_SHIFT) 0x1f; + + update_rate = refclk / clock.n; + clock.vco = update_rate * clock.m1 * clock.m2; + fastclk = clock.vco / clock.p1 / clock.p2; + clock.dot = (2 * fastclk); + + pipe_config-port_clock = clock.dot / 10; Looks like it should get roughly the right answer, but I don't see much point in all the intermediate results. If you want to keep some of them for clarity, then I think this should be enough: clock.vco = refclk * clock.m / clock.n; clock.dot = clock.vco / clock.p; /* fast clock */ pipe_config-port_clock = clock.dot / 5; Although calling the fast clock dot is a bit wrong, but I think it's fine here, especially as it matches what I have in mind for vlv_find_best_dpll(). I had to rewrite that sucker a bit to make it compatible with my brain ;) I'll send a patch for that ASAP. The other advantage of having it all spelled out like this is that it matches the freq calculation spreadsheet I sourced it from. So I'd prefer to keep this as is, if I can have your r-b anyway. :) Queued for -next with Ville's irc-r-b, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Mask LPSP to get PSR working even with Power Well in use by audio.
On Wed, Sep 25, 2013 at 10:59 PM, Rodrigo Vivi rodrigo.v...@gmail.com wrote: Please consider to accept this for -fixes, otherwise PSR will never work on Haswell on 3.12. You know the drill: A feature regressed and no one noticed, which means we are lacking a fully automated testcase. I guess we need to expose in debugfs somewhere if the edp panel can do psr (if we don't do that already) to be able to skip the test correctly and then check with a little igt testcase that we actually achieve psr residency. Also please poke QA to make sure they actually have a hsw platform with psr panel. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: fix up broken precision in vlv_crtc_clock_get
On Wed, Sep 25, 2013 at 02:24:01PM -0700, Jesse Barnes wrote: From: Chris Wilson ch...@chris-wilson.co.uk With some divider values we end up with the wrong result. So remove the intermediates (like Ville suggested in the first place) to get the right answer. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: add VLV specific clock_get function v3
On Wed, Sep 25, 2013 at 11:00:20PM +0200, Daniel Vetter wrote: On Wed, Sep 25, 2013 at 10:38:33AM -0700, Jesse Barnes wrote: On Mon, 23 Sep 2013 21:01:44 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Fri, Sep 20, 2013 at 11:29:32AM -0700, Jesse Barnes wrote: Calculation is a little different than other platforms. v2: update to use port_clock instead rebase on top of Ville's changes v3: update to new port_clock semantics - don't divide by pixel_multiplier (Ville) References: https://bugs.freedesktop.org/show_bug.cgi?id=67345 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_display.c | 33 - 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7eecf37..e5c9c1c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5048,6 +5048,34 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc, I915_READ(LVDS) LVDS_BORDER_ENABLE; } +static void vlv_crtc_clock_get(struct intel_crtc *crtc, + struct intel_crtc_config *pipe_config) +{ + struct drm_device *dev = crtc-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + int pipe = pipe_config-cpu_transcoder; + intel_clock_t clock; + u32 mdiv; + int refclk = 10, fastclk, update_rate; + + mutex_lock(dev_priv-dpio_lock); + mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe)); + mutex_unlock(dev_priv-dpio_lock); + + clock.m1 = (mdiv DPIO_M1DIV_SHIFT) 7; + clock.m2 = mdiv DPIO_M2DIV_MASK; + clock.n = (mdiv DPIO_N_SHIFT) 0xf; + clock.p1 = (mdiv DPIO_P1_SHIFT) 7; + clock.p2 = (mdiv DPIO_P2_SHIFT) 0x1f; + + update_rate = refclk / clock.n; + clock.vco = update_rate * clock.m1 * clock.m2; + fastclk = clock.vco / clock.p1 / clock.p2; + clock.dot = (2 * fastclk); + + pipe_config-port_clock = clock.dot / 10; Looks like it should get roughly the right answer, but I don't see much point in all the intermediate results. If you want to keep some of them for clarity, then I think this should be enough: clock.vco = refclk * clock.m / clock.n; clock.dot = clock.vco / clock.p; /* fast clock */ pipe_config-port_clock = clock.dot / 5; Although calling the fast clock dot is a bit wrong, but I think it's fine here, especially as it matches what I have in mind for vlv_find_best_dpll(). I had to rewrite that sucker a bit to make it compatible with my brain ;) I'll send a patch for that ASAP. The other advantage of having it all spelled out like this is that it matches the freq calculation spreadsheet I sourced it from. So I'd prefer to keep this as is, if I can have your r-b anyway. :) Queued for -next with Ville's irc-r-b, thanks for the patch. -Daniel Did Jesse post a delta update to this, as #67345 still warns with v3. For the record, this works - can't see why it would make a difference at the moment though (probably some type is too small or loss of precision): diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5741d48..5219fdc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5083,10 +5083,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc, { struct drm_device *dev = crtc-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; + const int refclk = 10; int pipe = pipe_config-cpu_transcoder; intel_clock_t clock; u32 mdiv; - int refclk = 10, fastclk, update_rate; mutex_lock(dev_priv-dpio_lock); mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe)); @@ -5098,12 +5098,16 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc, clock.p1 = (mdiv DPIO_P1_SHIFT) 7; clock.p2 = (mdiv DPIO_P2_SHIFT) 0x1f; - update_rate = refclk / clock.n; - clock.vco = update_rate * clock.m1 * clock.m2; - fastclk = clock.vco / clock.p1 / clock.p2; - clock.dot = (2 * fastclk); + clock.vco = refclk * clock.m1 * clock.m2 / clock.n; + clock.dot = 2 * clock.vco / (clock.p1 * clock.p2); pipe_config-port_clock = clock.dot / 10; } -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix up usage of SHRINK_STOP
On Wed, Sep 25, 2013 at 02:00:02PM +0200, Daniel Vetter wrote: In commit 81e49f811404f428a9d9a63295a0c267e802fa12 Author: Glauber Costa glom...@openvz.org Date: Wed Aug 28 10:18:13 2013 +1000 i915: bail out earlier when shrinker cannot acquire mutex SHRINK_STOP was added to tell the core shrinker code to bail out and go to the next shrinker since the i915 shrinker couldn't acquire required locks. But the SHRINK_STOP return code was added to the -count_objects callback and not the -scan_objects callback as it should have been, resulting in tons of dmesg noise like shrink_slab: i915_gem_inactive_scan+0x0/0x9c negative objects to delete nr=-x Fix discusssed with Dave Chinner. Acked-by: Dave Chinner dchin...@redhat.com Cheers, Dave. -- Dave Chinner da...@fromorbit.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Print ring min freq scaling
This allows us to keep track of the values being set if we want to tweak this code. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_pm.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d27eda6..31cf188 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3671,6 +3671,9 @@ void gen6_update_ring_freq(struct drm_device *dev) ring_freq = (gpu_freq * 5 + 3) / 4; ring_freq = max(min_ring_freq, ring_freq); /* leave ia_freq as the default, chosen by cpufreq */ + + DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT freq %dMHz\n, +ring_freq * 100, gpu_freq * 50); } else { /* On older processors, there is no separate ring * clock domain, so in order to boost the bandwidth @@ -3684,6 +3687,9 @@ void gen6_update_ring_freq(struct drm_device *dev) else ia_freq = max_ia_freq - ((diff * scaling_factor) / 2); ia_freq = DIV_ROUND_CLOSEST(ia_freq, 100); + + DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT freq %dMHz\n, +ia_freq * 100, gpu_freq * 50); } sandybridge_pcode_write(dev_priv, -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Use the real cpu max frequency for ring scaling
The policy's max frequency is not equal to the CPU's max frequency. The ring frequency is derived from the CPU frequency, and not the policy frequency. One example of how this may differ through sysfs. If the sysfs max frequency is modified, that will be used for the max ring frequency calculation. (/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq). As far as I know, no current governor uses anything but max as the default, but in theory, they could. Similarly distributions might set policy as part of their init process. It's ideal to use the real frequency because when we're currently scaled up on the GPU. In this case we likely want to race to idle, and using a less than max ring frequency is non-optimal for this situation. AFAIK, this patch should have no impact on a majority of people. This behavior hasn't been changed since it was first introduced: commit 23b2f8bb92feb83127679c53633def32d3108e70 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Tue Jun 28 13:04:16 2011 -0700 drm/i915: load a ring frequency scaling table v3 CC: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_pm.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 31cf188..3212d3b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3639,16 +3639,21 @@ void gen6_update_ring_freq(struct drm_device *dev) unsigned int gpu_freq; unsigned int max_ia_freq, min_ring_freq; int scaling_factor = 180; + struct cpufreq_policy *policy; WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock)); - max_ia_freq = cpufreq_quick_get_max(0); - /* -* Default to measured freq if none found, PCU will ensure we don't go -* over -*/ - if (!max_ia_freq) + policy = cpufreq_cpu_get(0); + if (policy) { + max_ia_freq = policy-cpuinfo.max_freq; + cpufreq_cpu_put(policy); + } else { + /* +* Default to measured freq if none found, PCU will ensure we +* don't go over +*/ max_ia_freq = tsc_khz; + } /* Convert from kHz to MHz */ max_ia_freq /= 1000; -- 1.8.4 ___ 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: Print ring min freq scaling
On Wed, Sep 25, 2013 at 04:35:32PM -0700, Ben Widawsky wrote: This allows us to keep track of the values being set if we want to tweak this code. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_pm.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d27eda6..31cf188 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3671,6 +3671,9 @@ void gen6_update_ring_freq(struct drm_device *dev) ring_freq = (gpu_freq * 5 + 3) / 4; ring_freq = max(min_ring_freq, ring_freq); /* leave ia_freq as the default, chosen by cpufreq */ + + DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT freq %dMHz\n, + ring_freq * 100, gpu_freq * 50); } else { /* On older processors, there is no separate ring * clock domain, so in order to boost the bandwidth @@ -3684,6 +3687,9 @@ void gen6_update_ring_freq(struct drm_device *dev) else ia_freq = max_ia_freq - ((diff * scaling_factor) / 2); ia_freq = DIV_ROUND_CLOSEST(ia_freq, 100); + + DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT freq %dMHz\n, + ia_freq * 100, gpu_freq * 50); This is not a ring freq, but a cpu core freq. Also would be wise to point out that this information is in /sys/kernel/debug/dri/0/i915_ring_freq_table -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ 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: Use the real cpu max frequency for ring scaling
On Wed, Sep 25, 2013 at 04:35:33PM -0700, Ben Widawsky wrote: The policy's max frequency is not equal to the CPU's max frequency. The ring frequency is derived from the CPU frequency, and not the policy frequency. One example of how this may differ through sysfs. If the sysfs max frequency is modified, that will be used for the max ring frequency calculation. (/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq). As far as I know, no current governor uses anything but max as the default, but in theory, they could. Similarly distributions might set policy as part of their init process. Actually this seems to differ based on policy as well. The pstate may disable turbo mode, in which case policy-max = policy-cpuinfo.max. But excluding user intervention, it seems that max is what we want, and it is possibly arguable that we do not want to push the core to turbo for a GPU bound workload - though I'm not sure if the ring frequency continues to scale with turbo cpu frequencies. It's ideal to use the real frequency because when we're currently scaled up on the GPU. In this case we likely want to race to idle, and using a less than max ring frequency is non-optimal for this situation. Also note that scaling the cpu frequency cuts into our thermal headroom, so for ULT devices it may not be clearly beneficial. AFAIK, this patch should have no impact on a majority of people. And also obsolete since HSW. This behavior hasn't been changed since it was first introduced: commit 23b2f8bb92feb83127679c53633def32d3108e70 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Tue Jun 28 13:04:16 2011 -0700 drm/i915: load a ring frequency scaling table v3 CC: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Ben Widawsky b...@bwidawsk.net FWIW, Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk I think using the real range of the CPU frequency rather than the adjusted policy maximum is sensible. Just the hw implementation is just a bit silly and it is not clear exactly what the best policy should be. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ 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: Print ring min freq scaling
On Thu, Sep 26, 2013 at 12:49:37AM +0100, Chris Wilson wrote: On Wed, Sep 25, 2013 at 04:35:32PM -0700, Ben Widawsky wrote: This allows us to keep track of the values being set if we want to tweak this code. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_pm.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d27eda6..31cf188 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3671,6 +3671,9 @@ void gen6_update_ring_freq(struct drm_device *dev) ring_freq = (gpu_freq * 5 + 3) / 4; ring_freq = max(min_ring_freq, ring_freq); /* leave ia_freq as the default, chosen by cpufreq */ + + DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT freq %dMHz\n, +ring_freq * 100, gpu_freq * 50); } else { /* On older processors, there is no separate ring * clock domain, so in order to boost the bandwidth @@ -3684,6 +3687,9 @@ void gen6_update_ring_freq(struct drm_device *dev) else ia_freq = max_ia_freq - ((diff * scaling_factor) / 2); ia_freq = DIV_ROUND_CLOSEST(ia_freq, 100); + + DRM_DEBUG_DRIVER(Setup min ring frequency %dMHz for GT freq %dMHz\n, +ia_freq * 100, gpu_freq * 50); This is not a ring freq, but a cpu core freq. Also would be wise to point out that this information is in /sys/kernel/debug/dri/0/i915_ring_freq_table -Chris -- Chris Wilson, Intel Open Source Technology Centre Ooops. I forgot about that interface. Ignore this patch. You are incorrect in saying it's _just_ the CPU core frequency. This is the ring and IA frequency on SNB + IVB. The frequency is tied to the CPU core on SNB + IVB. It is not tied to the cpu core on Haswell. To make the print more generic, and correct across the board it makes sense to say we're setting the ring frequency (to me). Even the programming guides typically call this, IA/ring frequency. Above is only for posterity. Again, forget the patch. -- Ben Widawsky, 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 4/4] thinkpad-acpi: fix handle locate for video and query of _BCL
On Wed, Sep 25, 2013 at 04:58:39PM -0300, Henrique de Moraes Holschuh wrote: On Tue, 24 Sep 2013, Aaron Lu wrote: locate handle for ACPI video by HID, the problem is, ACPI video node doesn't really have HID defined(i.e. no _HID control method is defined ACPI video is supposed to attach a virtual HID node (ACPI_VIDEO_HID) to ACPI video devices so as to keep the non-trivial video device detection logic in just one place instead of reinventing the wheel in every driver (which is always a recipe for disaster). When did that break? I checked the git log for the commit to use tpacpi_acpi_handle_locate to locate video controller's ACPI handle, it's: commit 122f26726b5e16174bf8a707df14be1d93c49d62 Author: Henrique de Moraes Holschuh h...@hmh.eng.br Date: Mon Aug 9 23:48:18 2010 -0300 thinkpad-acpi: find ACPI video device by synthetic HID So I checked out that commit and found that it shouldn't work either, since it has the same problem of the current code. The ACPI video controller device is given an id of ACPI_VIDEO_HID, but it's only known to Linux ACPI, not ACPICA, so the function provided by ACPICA acpi_get_devices will not work in this case, as that function will really check the control method of _HID under the handle, which does not exist no matter if Linux ACPI has added an id to its device structure or not. OTOH, the function provided by Linux ACPI acpi_device_hid will see the added id. In a word, the add of the HID will not affect the ASL namespace layout of the device node(thus, no _HID control method will be added to the device node). It seems that this problem has been found previously by: commit ff413195e830541afeae469fc866ecd0319abd7e Author: Alex Hung alex.h...@canonical.com Date: Tue Apr 24 16:40:52 2012 +0800 thinkpad-acpi: fix issuing duplicated key events for brightness up/down The tp_features.bright_acpimode will not be set correctly for brightness control because ACPI_VIDEO_HID will not be located in ACPI. As a result, a duplicated key event will always be sent. acpi_video_backlight_support() is sufficient to detect standard ACPI brightness control. Signed-off-by: Alex Hung alex.h...@canonical.com Signed-off-by: Matthew Garrett m...@redhat.com Thanks, Aaron ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/4] ACPI / video: Do not register backlight if win8 and native interface exists
On Wed, Sep 25, 2013 at 07:53:13PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 24, 2013 05:47:31 PM Aaron Lu wrote: According to Matthew Garrett, Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows [8] doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems. So for Win8 systems, if there is native backlight control interface registered by GPU driver, ACPI video will not register its own. For users who prefer to keep ACPI video's backlight interface, the existing kernel cmdline option acpi_backlight=video can be used. I think the idea is to use the aggressive default for now and we can switch the default back to the current behavior before the merge window in case there are too many problems with it? Yes I think so. Thanks, Aaron ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx