Re: [Intel-gfx] [PATCH] drm/i915: Fix cursor visibility check with negative coordinates
On Mon, Sep 02, 2013 at 09:16:50PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When the cursor x coordinate is exactly -cursor_width, the cursor is invisible. And obviously the same holds for the y coordinate and cursor_height. And similarly for the earlier x == fb-width checks. -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 03/12] drm/i915: Use adjusted_mode in HDMI 12bpc clock check
On Mon, Sep 02, 2013 at 10:05:26PM +0300, Ville Syrjälä wrote: On Mon, Sep 02, 2013 at 08:39:45PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 09:13:30PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The pixel clock should come from adjusted_mode not requested_mode. In this case the two should be the same as we don't currently overwrite the clock in the case of HDMI. But let's make the code safe against such things happening in the future. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Atm only the encoder overrides the adjusted_mode at all, so I think I prefer the current code flow as clearer ... And I'd atually prefer to kill requested_mode. It's confusing. Only hdisplay/vdisplay are always valid. Well even that's not true as can be seen from my double wide series. The clock and timings can be trusted only at the very beginning of the compute config step. And at that point adjusted_mode contains the exact same information. Also apart from the clock, we never use the other timings from requested_mode, so keeping the whole thing around seems more or less pointless. Ok, count me slightly convinced. I agree that after the pipe config compute stage the only thing we can rely on is h/vdisplay (it's also the only thing we actually cross-check). So I guess ditching config-requested_mode is a good goal. If you throw a patch on top to add a bit of documentation for the adjusted_mode and the pipe_src_h|w fields I'll even be happy ;-) Cheers, 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 11/12] drm/i915: Add explicit pipe src size to pipe config
On Mon, Sep 02, 2013 at 10:11:43PM +0300, Ville Syrjälä wrote: On Mon, Sep 02, 2013 at 08:46:19PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 09:13:38PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Rather that mess about with hdisplay/vdisplay from requested_mode, add explicit pipe src size information to pipe config. Now requested_mode is only really relevant for dvo/sdvo output timings. For everything else either adjusted_mode or pipe src size should be used. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Not sold on this - imo it makes more sense to keep track of this in a new plane config structure or something similar which ties the fb + metadata to the crtc. Then we could move a bunch of things we currently have in the pipe config or someplaces else even (fbc, ips, ...) over there. This size isn't the plane size. It's the pipe size. Two different things. Oops, I guess I'm confused about this. In that case this makes tons of sense. Now just one of our residential watermarks experts (definitely not me) needs to double-check that we're always using the right thing ... -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/5] drm/i915: Check pixel clock limits on pre-gen4
On Mon, Sep 02, 2013 at 10:24:18PM +0300, Ville Syrjälä wrote: On Mon, Sep 02, 2013 at 08:51:41PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 09:24:26PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com We don't want to try to push the hardware beyond it's capabilities, so check the pixel clock against the display core clock limit. Do it for pre-gen4 for now since that's where we alread have the double wide pixel clock limit check. Let's assume that when double wide mode is enabled the max pixel clock limit is also doubled. FIXME: panel fitter downscaling probably affects the limit on non-pch platforms too, so we'd need another version of ilk_pipe_pixel_rate() to figure that out. FIXME: should check the limits on all platforms. Also sprites affect the max allowed pixel rate on some platforms, so we need to eventually tie all the planes and pipes into one check in the future. But we need plane state pre-compute before that can happen. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ea33468..040e0ef 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4140,6 +4140,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, return -EINVAL; } + /* FIXME should check pixel clock limits on all platforms */ We do by failing to find the pll. The fun is to move all that code into the pipe computation stage. Are you 100% sure the PLL algorithm can never give you a clock that exceeds the display core clock? Looking at the functions to calculate the core clock, there is an awful lot of variation there already, and we don't even have those functions filled out for modern platforms. The second part of the fun is that on newer platforms dotclock limits are all encoder specific, so we need to shovel them into encoder callbacks anyway. I don't know whether there are any additional pixel clock limits on top of what the plls can handle, but at least I haven't spotted them yet ... The display core clock is the big one. We have to check it for other platforms too eventually since the limit depends on a bunch of factors. For example on ILK/SNB we need to check whether there's one or two sprites enabled, are they using RGB or YUV, are they scaled, and if so by how much? Obviouly we can't do all that until we can pre-compute the state of all planes. So actually we may need some more steps in the process. Something like this: 1) adjust timings 2) calculate PLL settings to get the real clock 3) check double wide etc. that depends on the clock 4) compute plane config 5) check core clock and other plane related global limits But I don't mind dropping this one now, and revisiting the issue with a bigger hammer in the future. I think this one here is good, I've only taking issues with the FIXME comment since I've thought most of this is encoder specific. But it sounds like there's an entire set of constraints on the pixel value construction side of things that we're currently completely missing. So I guess this is ok. -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 2/2] drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2
On Wed, Sep 04, 2013 at 01:30:38AM +0800, Chon Ming Lee wrote: For DP pll settings, there is only two golden configs. Instead of running through the algorithm to determine it, hardcode the value and get it determine in intel_dp_set_clock. v2: Rework on the intel_limit compiler warning. (Jani) Signed-off-by: Chon Ming Lee chon.ming@intel.com lgtm, both patches merged. Note that checkpatch.pl was a bit unhappy about the structure initalization layout, so I've fixed this up while applying. I'm not too insisting on checkpatch clean code (especially the 80 char limit for debug output often results in ugly code so is better ignored in those cases). So please run it before submitting patches and fix up reported issues (if it doesn't result in ugly code ofc). Thanks, Daniel --- drivers/gpu/drm/i915/intel_display.c | 20 +++- drivers/gpu/drm/i915/intel_dp.c | 11 ++- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f526ea9..1a567d2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -339,19 +339,6 @@ static const intel_limit_t intel_limits_vlv_hdmi = { .p2_slow = 2, .p2_fast = 20 }, }; -static const intel_limit_t intel_limits_vlv_dp = { - .dot = { .min = 25000, .max = 27 }, - .vco = { .min = 400, .max = 600 }, - .n = { .min = 1, .max = 7 }, - .m = { .min = 22, .max = 450 }, - .m1 = { .min = 2, .max = 3 }, - .m2 = { .min = 11, .max = 156 }, - .p = { .min = 10, .max = 30 }, - .p1 = { .min = 1, .max = 3 }, - .p2 = { .dot_limit = 27, - .p2_slow = 2, .p2_fast = 20 }, -}; - static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc, int refclk) { @@ -414,10 +401,8 @@ static const intel_limit_t *intel_limit(struct drm_crtc *crtc, int refclk) } else if (IS_VALLEYVIEW(dev)) { if (intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG)) limit = intel_limits_vlv_dac; - else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI)) - limit = intel_limits_vlv_hdmi; else - limit = intel_limits_vlv_dp; + limit = intel_limits_vlv_hdmi; } else if (!IS_GEN2(dev)) { if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) limit = intel_limits_i9xx_lvds; @@ -4890,7 +4875,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, refclk = i9xx_get_refclk(crtc, num_connectors); - if (!is_dsi) { + if (!is_dsi !intel_crtc-config.clock_set) { /* * Returns a set of divisors for the desired target clock with * the given refclk, or FALSE. The returned values represent @@ -4917,6 +4902,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, * by using the FP0/FP1. In such case we will disable the LVDS * downclock feature. */ + limit = intel_limit(crtc, refclk); has_reduced_clock = dev_priv-display.find_dpll(limit, crtc, dev_priv-lvds_downclock, diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index fd09058..76b4372 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -59,6 +59,14 @@ static const struct dp_link_dpll pch_dpll[] = { .p1 = 1, .p2 = 10, .n = 2, .m1 = 14, .m2 = 8 }} }; +static const struct dp_link_dpll vlv_dpll[] = +{ + { DP_LINK_BW_1_62, + { .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 }}, + { DP_LINK_BW_2_7, + { .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 }} +}; + /** * is_edp - is the given port attached to an eDP panel (either CPU or PCH) * @intel_dp: DP struct @@ -682,7 +690,8 @@ intel_dp_set_clock(struct intel_encoder *encoder, divisor = pch_dpll; count = ARRAY_SIZE(pch_dpll); } else if (IS_VALLEYVIEW(dev)) { - /* FIXME: Need to figure out optimized DP clocks for vlv. */ + divisor = vlv_dpll; + count = ARRAY_SIZE(vlv_dpll); } if (divisor count) { -- 1.7.7.6 ___ 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
[Intel-gfx] [PATCH] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges
From: Ville Syrjälä ville.syrj...@linux.intel.com First of all we should not be looking at fb-{width,height} as those do not tell us what the actual pipe size is. Second of all we need to use = for the comparison. So fix the comparison, and make use of the new pipe_src_{w,h} to determine the real pipe source dimensions. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bbaa689..5055d2f8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6847,19 +6847,16 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, int pipe = intel_crtc-pipe; int x = intel_crtc-cursor_x; int y = intel_crtc-cursor_y; - u32 base, pos; + u32 base = 0, pos = 0; bool visible; - pos = 0; - - if (on crtc-enabled crtc-fb) { + if (on) base = intel_crtc-cursor_addr; - if (x (int) crtc-fb-width) - base = 0; - if (y (int) crtc-fb-height) - base = 0; - } else + if (x = intel_crtc-config.pipe_src_w) + base = 0; + + if (y = intel_crtc-config.pipe_src_h) base = 0; if (x 0) { -- 1.8.1.5 ___ 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 the relocate_entry refactoring
On Mon, Sep 02, 2013 at 08:58:03PM +0200, Daniel Vetter wrote: Somehow we've lost the error handling in the patch split-up between the internal and external patch. This regression has been introduced in commit 5032d871f7d300aee10c309ea004eb4f851553fe Author: Rafael Barbalho rafael.barba...@intel.com Date: Wed Aug 21 17:10:51 2013 +0100 drm/i915: Cleaning up the relocate entry function This bug is exercised by igt/gem_reloc_vs_gpu/interruptible. I've smashed the patch onto dinq meanwhile. -Daniel Cc: Rafael Barbalho rafael.barba...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index ea4bacb..e519f9f 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -349,6 +349,9 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, else ret = relocate_entry_gtt(obj, reloc); + if (ret) + return ret; + /* and update the user's relocation entry */ reloc-presumed_offset = target_offset; -- 1.8.4.rc3 -- 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 cursor visibility checks also for the right/bottom screen edges
On Tue, Sep 03, 2013 at 11:04:30AM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com First of all we should not be looking at fb-{width,height} as those do not tell us what the actual pipe size is. Second of all we need to use = for the comparison. So fix the comparison, and make use of the new pipe_src_{w,h} to determine the real pipe source dimensions. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Whoops. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk And r-b for its sibling as well. I am pretty sure there are real-world bugs out there, but they are going to pretty be rare and the same user is never likely to see it twice... And just maybe more recent hw isn't quite so hang-happy! -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 11/12] drm/i915: Add explicit pipe src size to pipe config
On Tue, Sep 03, 2013 at 09:52:40AM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 10:11:43PM +0300, Ville Syrjälä wrote: On Mon, Sep 02, 2013 at 08:46:19PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 09:13:38PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Rather that mess about with hdisplay/vdisplay from requested_mode, add explicit pipe src size information to pipe config. Now requested_mode is only really relevant for dvo/sdvo output timings. For everything else either adjusted_mode or pipe src size should be used. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Not sold on this - imo it makes more sense to keep track of this in a new plane config structure or something similar which ties the fb + metadata to the crtc. Then we could move a bunch of things we currently have in the pipe config or someplaces else even (fbc, ips, ...) over there. This size isn't the plane size. It's the pipe size. Two different things. Oops, I guess I'm confused about this. In that case this makes tons of sense. Now just one of our residential watermarks experts (definitely not me) needs to double-check that we're always using the right thing ... Actually we're now using pipe_src_{w,h} always. I was considering adding primary_{w,h} to pipe config already, but then decided that I could wait until we split the primary from the crtc. The reason I need to use pipe_src_{w,h} is the pipe_src_w roudning for double wide co. We need to clip the primary to the pipe dimensions, so the same value works as long as primary planes are fullscreen only. But if you'd prefer I could still add the explicit primary dimensions to pipe config. -- 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: Document the use of requested_mode, adjusted_mode, and pipe_src_{w, h}
From: Ville Syrjälä ville.syrj...@linux.intel.com Try to clarify the purpose of the two different modes we keep, and the pipe source size. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_drv.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a71b47c..75e86da 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -211,9 +211,19 @@ struct intel_crtc_config { #define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (10) /* unreliable sync mode.flags */ unsigned long quirks; + /* User requested mode, only valid as a starting point to +* compute adjusted_mode, except in the case of (S)DVO where +* it's also for the output timings of the (S)DVO chip. +* adjusted_mode will then correspond to the S(DVO) chip's +* preferred input timings. */ struct drm_display_mode requested_mode; + /* Actual pipe timings ie. what we +* program into the pipe timing registers. */ struct drm_display_mode adjusted_mode; + /* Pipe source size (ie. panel fitter input size) +* All planes will be positioned inside this space, +* and get clipped at the edges. */ int pipe_src_w, pipe_src_h; /* Whether to set up the PCH/FDI. Note that we never allow sharing -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/12] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode
On Mon, Sep 02, 2013 at 08:38:16PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 09:13:28PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com intel_crtc_compute_config() and i9xx_set_pipeconf() attempt to get the current pixel clock from requested_mode. requested_mode.clock may be totally bogus, so the clock should come from adjusted_mode. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ecb8b52..cab1319 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4124,8 +4124,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, if (HAS_PCH_SPLIT(dev)) { /* FDI link clock is fixed at 2.7G */ - if (pipe_config-requested_mode.clock * 3 -IRONLAKE_FDI_FREQ * 4) + if (adjusted_mode-clock * 3 IRONLAKE_FDI_FREQ * 4) Note quite: The fdi dotclock is the adjusted mode's clock but with the pixel multiplier _not_ taken into account. See ironlake_fdi_compute_config. Maybe we need a fdi_dotclock_from_pipe_config helper function? Dang those pixel multipliers. I need to study on the topic a bit more. I'm confused whether the pipe is actually pushing out pixels at the non-multiplied rate or the multiplied rate. That's an important detail when we consider the CDCLK vs. pipe pixel rate limitations. -- 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: Kill IRONLAKE_FDI_FREQ check
From: Ville Syrjälä ville.syrj...@linux.intel.com ironlake_fdi_compute_config() already checks that we have enough FDI bandwidth. And it doesn't just use a hardcoded value but takes into account factors such as the actual FDI frequency, shared FDI B/C lanes, etc. Suggested-by: Daniel Vetter dan...@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ecb8b52..8808b17 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -69,9 +69,6 @@ struct intel_limit { intel_p2_t p2; }; -/* FDI */ -#define IRONLAKE_FDI_FREQ 270 /* in kHz for mode-clock */ - int intel_pch_rawclk(struct drm_device *dev) { @@ -4122,13 +4119,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, struct drm_device *dev = crtc-base.dev; struct drm_display_mode *adjusted_mode = pipe_config-adjusted_mode; - if (HAS_PCH_SPLIT(dev)) { - /* FDI link clock is fixed at 2.7G */ - if (pipe_config-requested_mode.clock * 3 -IRONLAKE_FDI_FREQ * 4) - return -EINVAL; - } - /* Cantiga+ cannot handle modes with a hsync front porch of 0. * WaPruneModeWithIncorrectHsyncOffset:ctg,elk,ilk,snb,ivb,vlv,hsw. */ -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Kill IRONLAKE_FDI_FREQ check
On Tue, Sep 03, 2013 at 01:31:38PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com ironlake_fdi_compute_config() already checks that we have enough FDI bandwidth. And it doesn't just use a hardcoded value but takes into account factors such as the actual FDI frequency, shared FDI B/C lanes, etc. Suggested-by: Daniel Vetter dan...@ffwll.ch Signed-off-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
[Intel-gfx] [RFC PATCH 1/3] drm/i915: move backlight enable later in vlv enable sequence
Follow-up to commit 5004945f1d6c0282c0288afa89ad85d7f2bea4d5 Author: Jani Nikula jani.nik...@intel.com Date: Tue Jul 30 12:20:32 2013 +0300 drm/i915: move encoder-enable callback later in VLV crtc enable Reference: http://mid.gmane.org/cakmk7ufs9emvmw8bns24e5unm1d7jrfvg3sd5sdftveamgf...@mail.gmail.com Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_dp.c |8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c192dbb..adbe7bc 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1717,11 +1717,17 @@ static void intel_enable_dp(struct intel_encoder *encoder) ironlake_edp_panel_vdd_off(intel_dp, true); intel_dp_complete_link_train(intel_dp); intel_dp_stop_link_train(intel_dp); - ironlake_edp_backlight_on(intel_dp); + + /* this function is called from vlv_pre_enable_dp() */ + if (!IS_VALLEYVIEW(dev)) + ironlake_edp_backlight_on(intel_dp); } static void vlv_enable_dp(struct intel_encoder *encoder) { + struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); + + ironlake_edp_backlight_on(intel_dp); } static void intel_pre_enable_dp(struct intel_encoder *encoder) -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 3/3] drm/i915: add support for per-pipe power sequencing on vlv
VLV has per-pipe PP registers. Set up power sequencing on mode set. The connector init time setup is problematic, since we don't have a pipe at that time. Cook up something. Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 139 +++ 1 file changed, 97 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0ba72f1..95d67fc 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -237,24 +237,47 @@ intel_hrawclk(struct drm_device *dev) } } +static void +intel_dp_init_panel_power_sequencer(struct drm_device *dev, + struct intel_dp *intel_dp, + struct edp_power_seq *out); +static void +intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, + struct intel_dp *intel_dp, + struct edp_power_seq *out); + +static u32 _pp_ctrl_reg(struct intel_dp *intel_dp) +{ + struct drm_device *dev = intel_dp_to_dev(intel_dp); + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + int pipe = to_intel_crtc(intel_dig_port-base.base.crtc)-pipe; + + return HAS_PCH_SPLIT(dev) ? PCH_PP_CONTROL : VLV_PIPE_PP_CONTROL(pipe); +} + +static u32 _pp_stat_reg(struct intel_dp *intel_dp) +{ + struct drm_device *dev = intel_dp_to_dev(intel_dp); + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + int pipe = to_intel_crtc(intel_dig_port-base.base.crtc)-pipe; + + return HAS_PCH_SPLIT(dev) ? PCH_PP_STATUS : VLV_PIPE_PP_STATUS(pipe); +} + static bool ironlake_edp_have_panel_power(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_i915_private *dev_priv = dev-dev_private; - u32 pp_stat_reg; - pp_stat_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_STATUS : PCH_PP_STATUS; - return (I915_READ(pp_stat_reg) PP_ON) != 0; + return (I915_READ(_pp_stat_reg(intel_dp)) PP_ON) != 0; } static bool ironlake_edp_have_panel_vdd(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_i915_private *dev_priv = dev-dev_private; - u32 pp_ctrl_reg; - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL; - return (I915_READ(pp_ctrl_reg) EDP_FORCE_VDD) != 0; + return (I915_READ(_pp_ctrl_reg(intel_dp)) EDP_FORCE_VDD) != 0; } static void @@ -262,19 +285,15 @@ intel_dp_check_edp(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_i915_private *dev_priv = dev-dev_private; - u32 pp_stat_reg, pp_ctrl_reg; if (!is_edp(intel_dp)) return; - pp_stat_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_STATUS : PCH_PP_STATUS; - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL; - if (!ironlake_edp_have_panel_power(intel_dp) !ironlake_edp_have_panel_vdd(intel_dp)) { WARN(1, eDP powered off while attempting aux channel communication.\n); DRM_DEBUG_KMS(Status 0x%08x Control 0x%08x\n, - I915_READ(pp_stat_reg), - I915_READ(pp_ctrl_reg)); + I915_READ(_pp_stat_reg(intel_dp)), + I915_READ(_pp_ctrl_reg(intel_dp))); } } @@ -948,8 +967,8 @@ static void ironlake_wait_panel_status(struct intel_dp *intel_dp, struct drm_i915_private *dev_priv = dev-dev_private; u32 pp_stat_reg, pp_ctrl_reg; - pp_stat_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_STATUS : PCH_PP_STATUS; - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL; + pp_stat_reg = _pp_stat_reg(intel_dp); + pp_ctrl_reg = _pp_ctrl_reg(intel_dp); DRM_DEBUG_KMS(mask %08x value %08x status %08x control %08x\n, mask, value, @@ -991,11 +1010,8 @@ static u32 ironlake_get_pp_control(struct intel_dp *intel_dp) struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_i915_private *dev_priv = dev-dev_private; u32 control; - u32 pp_ctrl_reg; - - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL; - control = I915_READ(pp_ctrl_reg); + control = I915_READ(_pp_ctrl_reg(intel_dp)); control = ~PANEL_UNLOCK_MASK; control |= PANEL_UNLOCK_REGS; return control; @@ -1028,8 +1044,8 @@ void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp) pp = ironlake_get_pp_control(intel_dp); pp |= EDP_FORCE_VDD; - pp_stat_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_STATUS : PCH_PP_STATUS; - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL; + pp_stat_reg = _pp_stat_reg(intel_dp); +
[Intel-gfx] [PATCH 1/2] drm/i915: do not update cursor in crtc mode set
The cursor is disabled before crtc mode set in crtc disable, and enabled afterwards in crtc enable. Do not update it in crtc mode set. Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_display.c |9 - 1 file changed, 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 30e5946..120a004 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4888,9 +4888,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, } } - /* Ensure that the cursor is valid for the new mode before changing... */ - intel_crtc_update_cursor(crtc, true); - if (!is_dsi is_lvds dev_priv-lvds_downclock_avail) { /* * Ensure we match the reduced clock's P to the target clock. @@ -5782,9 +5779,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, intel_crtc-config.dpll.p2 = clock.p2; } - /* Ensure that the cursor is valid for the new mode before changing... */ - intel_crtc_update_cursor(crtc, true); - /* CPU eDP is the only output that doesn't need a PCH PLL of its own. */ if (intel_crtc-config.has_pch_encoder) { fp = i9xx_dpll_compute_fp(intel_crtc-config.dpll); @@ -6273,9 +6267,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, if (!intel_ddi_pll_mode_set(crtc)) return -EINVAL; - /* Ensure that the cursor is valid for the new mode before changing... */ - intel_crtc_update_cursor(crtc, true); - if (intel_crtc-config.has_dp_encoder) intel_dp_set_m_n(intel_crtc); -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: clean up i9xx_crtc_mode_set wrt DSI and intel_crtc-config.clock_set
Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 44 +- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 120a004..f079475 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4869,9 +4869,12 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, num_connectors++; } - refclk = i9xx_get_refclk(crtc, num_connectors); + if (is_dsi) + goto skip_dpll; + + if (!intel_crtc-config.clock_set) { + refclk = i9xx_get_refclk(crtc, num_connectors); - if (!is_dsi !intel_crtc-config.clock_set) { /* * Returns a set of divisors for the desired target clock with * the given refclk, or FALSE. The returned values represent @@ -4882,28 +4885,25 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, ok = dev_priv-display.find_dpll(limit, crtc, intel_crtc-config.port_clock, refclk, NULL, clock); - if (!ok !intel_crtc-config.clock_set) { + if (!ok) { DRM_ERROR(Couldn't find PLL settings for mode!\n); return -EINVAL; } - } - if (!is_dsi is_lvds dev_priv-lvds_downclock_avail) { - /* -* Ensure we match the reduced clock's P to the target clock. -* If the clocks don't match, we can't switch the display clock -* by using the FP0/FP1. In such case we will disable the LVDS -* downclock feature. - */ - limit = intel_limit(crtc, refclk); - has_reduced_clock = - dev_priv-display.find_dpll(limit, crtc, - dev_priv-lvds_downclock, - refclk, clock, - reduced_clock); - } - /* Compat-code for transition, will disappear. */ - if (!intel_crtc-config.clock_set) { + if (is_lvds dev_priv-lvds_downclock_avail) { + /* +* Ensure we match the reduced clock's P to the target +* clock. If the clocks don't match, we can't switch +* the display clock by using the FP0/FP1. In such case +* we will disable the LVDS downclock feature. +*/ + has_reduced_clock = + dev_priv-display.find_dpll(limit, crtc, + dev_priv-lvds_downclock, + refclk, clock, + reduced_clock); + } + /* Compat-code for transition, will disappear. */ intel_crtc-config.dpll.n = clock.n; intel_crtc-config.dpll.m1 = clock.m1; intel_crtc-config.dpll.m2 = clock.m2; @@ -4916,14 +4916,14 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, has_reduced_clock ? reduced_clock : NULL, num_connectors); } else if (IS_VALLEYVIEW(dev)) { - if (!is_dsi) - vlv_update_pll(intel_crtc); + vlv_update_pll(intel_crtc); } else { i9xx_update_pll(intel_crtc, has_reduced_clock ? reduced_clock : NULL, num_connectors); } +skip_dpll: /* Set up the display plane register */ dspcntr = DISPPLANE_GAMMA_ENABLE; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: do not update cursor in crtc mode set
On Tue, Sep 03, 2013 at 02:48:13PM +0300, Jani Nikula wrote: The cursor is disabled before crtc mode set in crtc disable, and enabled afterwards in crtc enable. Do not update it in crtc mode set. Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Jani Nikula jani.nik...@intel.com Ok, but this would be better replaced with a set of asserts. -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 01/12] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode
On Tue, Sep 03, 2013 at 01:23:23PM +0200, Daniel Vetter wrote: On Tue, Sep 03, 2013 at 01:01:14PM +0300, Ville Syrjälä wrote: On Mon, Sep 02, 2013 at 08:38:16PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 09:13:28PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com intel_crtc_compute_config() and i9xx_set_pipeconf() attempt to get the current pixel clock from requested_mode. requested_mode.clock may be totally bogus, so the clock should come from adjusted_mode. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ecb8b52..cab1319 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4124,8 +4124,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, if (HAS_PCH_SPLIT(dev)) { /* FDI link clock is fixed at 2.7G */ - if (pipe_config-requested_mode.clock * 3 -IRONLAKE_FDI_FREQ * 4) + if (adjusted_mode-clock * 3 IRONLAKE_FDI_FREQ * 4) Note quite: The fdi dotclock is the adjusted mode's clock but with the pixel multiplier _not_ taken into account. See ironlake_fdi_compute_config. Maybe we need a fdi_dotclock_from_pipe_config helper function? Dang those pixel multipliers. I need to study on the topic a bit more. I'm confused whether the pipe is actually pushing out pixels at the non-multiplied rate or the multiplied rate. That's an important detail when we consider the CDCLK vs. pipe pixel rate limitations. On pch ports the pixel multiplier is in the pch dpll, so I think the data pushed over the fdi link isn't multiplied. Iirc I've even bothered with some tests, but not sure any more ... I vaguely remember that I've broken Chris' ilk+sdvo machine a few times in the process of getting this fleshed out ;-) Right, so at least on SDVO the multiplier is added to keep the SDVO clock on the above 100 MHz. So the multiplier won't actually affect the pixel clock. So could we just make port_clock be the multiplied clock for SDVO and make adjusted_mode.clock be the actual pixel clock? -- 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 0/2] drm/i915: adjusted_mode.clock vs. port_clock vs. pixel_multiplier
As I mentioned before, I'd like adjusted_mode.clock to be the pipe pixel clock. So to me it looks like all we need to do to store the pixel multiplied clock into port_clock, and then adjusted_mode.clock can stay as the non-multiplied pixel clock. Seems simple enough. But a big warning here, I didn't test this at all yet. Also I don't have any SDVO hardware to make sure I didn't fumble something on that side. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Don't factor in pixel multplier when deriving dotclock from link clock and M/N values
From: Ville Syrjälä ville.syrj...@linux.intel.com We feed the non-multiplied clock to intel_link_compute_m_n(), so the opposite operation should use the same order of operations. So we just multiply by pixel_multiplier in the end now. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dae5267..1ed042d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7372,20 +7372,18 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc, struct drm_device *dev = crtc-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; enum transcoder cpu_transcoder = pipe_config-cpu_transcoder; - int link_freq, repeat; + int link_freq; u64 clock; u32 link_m, link_n; - repeat = pipe_config-pixel_multiplier; - /* * The calculation for the data clock is: -* pixel_clock = ((m/n)*(link_clock * nr_lanes * repeat))/bpp +* pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp * But we want to avoid losing precison if possible, so: -* pixel_clock = ((m * link_clock * nr_lanes * repeat)/(n*bpp)) +* pixel_clock = ((m * link_clock * nr_lanes)/(n*bpp)) * * and the link clock is simpler: -* link_clock = (m * link_clock * repeat) / n +* link_clock = (m * link_clock) / n */ /* @@ -7407,10 +7405,11 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc, if (!link_m || !link_n) return; - clock = ((u64)link_m * (u64)link_freq * (u64)repeat); + clock = ((u64)link_m * (u64)link_freq); do_div(clock, link_n); - pipe_config-adjusted_mode.clock = clock; + pipe_config-adjusted_mode.clock = clock * + pipe_config-pixel_multiplier; } /** Returns the currently programmed mode of the given pipe. */ -- 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/2] drm/i915: Make adjusted_mode.clock non-pixel multiplied
From: Ville Syrjälä ville.syrj...@linux.intel.com It would be easier if adjusted_mode.clock would be the pipe pixel clock, and it actually is, except for the cases where pixel_multiplier 1. So let's change intel_sdvo to use port_clock as the multiplied clock, and then we can leave adjusted_mode.clock as pipe pixel clock. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 10 -- drivers/gpu/drm/i915/intel_sdvo.c| 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1ed042d..3bd9a72 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4095,7 +4095,6 @@ retry: link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10; fdi_dotclock = adjusted_mode-clock; - fdi_dotclock /= pipe_config-pixel_multiplier; lane = ironlake_get_lanes_required(fdi_dotclock, link_bw, pipe_config-pipe_bpp); @@ -7362,8 +7361,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc, } } - pipe_config-adjusted_mode.clock = clock.dot * - pipe_config-pixel_multiplier; + pipe_config-adjusted_mode.clock = clock.dot; } static void ironlake_crtc_clock_get(struct intel_crtc *crtc, @@ -7408,8 +7406,7 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc, clock = ((u64)link_m * (u64)link_freq); do_div(clock, link_n); - pipe_config-adjusted_mode.clock = clock * - pipe_config-pixel_multiplier; + pipe_config-adjusted_mode.clock = clock; } /** Returns the currently programmed mode of the given pipe. */ @@ -8360,7 +8357,8 @@ encoder_retry: /* Set default port clock if not overwritten by the encoder. Needs to be * done afterwards in case the encoder adjusts the mode. */ if (!pipe_config-port_clock) - pipe_config-port_clock = pipe_config-adjusted_mode.clock; + pipe_config-port_clock = pipe_config-adjusted_mode.clock * + pipe_config-pixel_multiplier; ret = intel_crtc_compute_config(to_intel_crtc(crtc), pipe_config); if (ret 0) { diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 317e058..3381a52 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1059,7 +1059,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo, static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_config *pipe_config) { - unsigned dotclock = pipe_config-adjusted_mode.clock; + unsigned dotclock = pipe_config-port_clock; struct dpll *clock = pipe_config-dpll; /* SDVO TV has fixed PLL values depend on its clock range, @@ -1124,7 +1124,6 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder, */ pipe_config-pixel_multiplier = intel_sdvo_get_pixel_multiplier(adjusted_mode); - adjusted_mode-clock *= pipe_config-pixel_multiplier; if (intel_sdvo-color_range_auto) { /* See CEA-861-E - 5.1 Default Encoding Parameters */ -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges
On Tue, Sep 3, 2013 at 10:13 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Sep 03, 2013 at 11:04:30AM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com First of all we should not be looking at fb-{width,height} as those do not tell us what the actual pipe size is. Second of all we need to use = for the comparison. So fix the comparison, and make use of the new pipe_src_{w,h} to determine the real pipe source dimensions. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Whoops. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk And r-b for its sibling as well. I am pretty sure there are real-world bugs out there, but they are going to pretty be rare and the same user is never likely to see it twice... And just maybe more recent hw isn't quite so hang-happy! Hm, can we have an igt for this? We kinda don't any sprite/cursor test at all right now, but we need to start somewhere ... -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 01/12] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode
On Tue, Sep 03, 2013 at 03:08:50PM +0300, Ville Syrjälä wrote: On Tue, Sep 03, 2013 at 01:23:23PM +0200, Daniel Vetter wrote: On Tue, Sep 03, 2013 at 01:01:14PM +0300, Ville Syrjälä wrote: On Mon, Sep 02, 2013 at 08:38:16PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 09:13:28PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com intel_crtc_compute_config() and i9xx_set_pipeconf() attempt to get the current pixel clock from requested_mode. requested_mode.clock may be totally bogus, so the clock should come from adjusted_mode. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ecb8b52..cab1319 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4124,8 +4124,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, if (HAS_PCH_SPLIT(dev)) { /* FDI link clock is fixed at 2.7G */ - if (pipe_config-requested_mode.clock * 3 - IRONLAKE_FDI_FREQ * 4) + if (adjusted_mode-clock * 3 IRONLAKE_FDI_FREQ * 4) Note quite: The fdi dotclock is the adjusted mode's clock but with the pixel multiplier _not_ taken into account. See ironlake_fdi_compute_config. Maybe we need a fdi_dotclock_from_pipe_config helper function? Dang those pixel multipliers. I need to study on the topic a bit more. I'm confused whether the pipe is actually pushing out pixels at the non-multiplied rate or the multiplied rate. That's an important detail when we consider the CDCLK vs. pipe pixel rate limitations. On pch ports the pixel multiplier is in the pch dpll, so I think the data pushed over the fdi link isn't multiplied. Iirc I've even bothered with some tests, but not sure any more ... I vaguely remember that I've broken Chris' ilk+sdvo machine a few times in the process of getting this fleshed out ;-) Right, so at least on SDVO the multiplier is added to keep the SDVO clock on the above 100 MHz. So the multiplier won't actually affect the pixel clock. So could we just make port_clock be the multiplied clock for SDVO and make adjusted_mode.clock be the actual pixel clock? There's also the pixel multiplier thing for hdmi and I'm not sure how it works there. But generally I think using the port clock with the pixel multiplier but excluding it from the adjusted mode is a sensible approach. -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 2/2] drm/i915: Make adjusted_mode.clock non-pixel multiplied
On Tue, Sep 03, 2013 at 04:21:04PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com It would be easier if adjusted_mode.clock would be the pipe pixel clock, and it actually is, except for the cases where pixel_multiplier 1. So let's change intel_sdvo to use port_clock as the multiplied clock, and then we can leave adjusted_mode.clock as pipe pixel clock. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Yeah I think this makes sense. Comment updates for port_clock in intel_drv.h to also mention the pixel multiplier would be good. Plus adding a fuzzy port_clock check to the config comparator - teaching intel_fuzzy_clock_check to just compare two clocks and adding a PIPE_CONF_FUZZY_CLOCK_CHECK macro for both adjusted_mode.clock and port_clock would be great. Of course we still need to make those checks conditional on !HSW :( Cheers, Daniel --- drivers/gpu/drm/i915/intel_display.c | 10 -- drivers/gpu/drm/i915/intel_sdvo.c| 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1ed042d..3bd9a72 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4095,7 +4095,6 @@ retry: link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10; fdi_dotclock = adjusted_mode-clock; - fdi_dotclock /= pipe_config-pixel_multiplier; lane = ironlake_get_lanes_required(fdi_dotclock, link_bw, pipe_config-pipe_bpp); @@ -7362,8 +7361,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc, } } - pipe_config-adjusted_mode.clock = clock.dot * - pipe_config-pixel_multiplier; + pipe_config-adjusted_mode.clock = clock.dot; } static void ironlake_crtc_clock_get(struct intel_crtc *crtc, @@ -7408,8 +7406,7 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc, clock = ((u64)link_m * (u64)link_freq); do_div(clock, link_n); - pipe_config-adjusted_mode.clock = clock * - pipe_config-pixel_multiplier; + pipe_config-adjusted_mode.clock = clock; } /** Returns the currently programmed mode of the given pipe. */ @@ -8360,7 +8357,8 @@ encoder_retry: /* Set default port clock if not overwritten by the encoder. Needs to be * done afterwards in case the encoder adjusts the mode. */ if (!pipe_config-port_clock) - pipe_config-port_clock = pipe_config-adjusted_mode.clock; + pipe_config-port_clock = pipe_config-adjusted_mode.clock * + pipe_config-pixel_multiplier; ret = intel_crtc_compute_config(to_intel_crtc(crtc), pipe_config); if (ret 0) { diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 317e058..3381a52 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1059,7 +1059,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo, static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_config *pipe_config) { - unsigned dotclock = pipe_config-adjusted_mode.clock; + unsigned dotclock = pipe_config-port_clock; struct dpll *clock = pipe_config-dpll; /* SDVO TV has fixed PLL values depend on its clock range, @@ -1124,7 +1124,6 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder, */ pipe_config-pixel_multiplier = intel_sdvo_get_pixel_multiplier(adjusted_mode); - adjusted_mode-clock *= pipe_config-pixel_multiplier; if (intel_sdvo-color_range_auto) { /* See CEA-861-E - 5.1 Default Encoding Parameters */ -- 1.8.1.5 ___ 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
[Intel-gfx] [PATCH 1/3] drm/i915: Make intel_fuzzy_clock_check() take in arbitrary clocks
From: Ville Syrjälä ville.syrj...@linux.intel.com We want to do fuzzy clock checks for other things besides adjusted_mode.clock, so just pass two two clocks to compare to intel_fuzzy_clock_check(). Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3bd9a72..1fea189 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8545,13 +8545,9 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes) } -static bool intel_fuzzy_clock_check(struct intel_crtc_config *cur, - struct intel_crtc_config *new) +static bool intel_fuzzy_clock_check(int clock1, int clock2) { - int clock1, clock2, diff; - - clock1 = cur-adjusted_mode.clock; - clock2 = new-adjusted_mode.clock; + int diff; if (clock1 == clock2) return true; @@ -8673,7 +8669,8 @@ intel_pipe_config_compare(struct drm_device *dev, #undef PIPE_CONF_QUIRK if (!IS_HASWELL(dev)) { - if (!intel_fuzzy_clock_check(current_config, pipe_config)) { + if (!intel_fuzzy_clock_check(current_config-adjusted_mode.clock, +pipe_config-adjusted_mode.clock)) { DRM_ERROR(mismatch in clock (expected %d, found %d)\n, current_config-adjusted_mode.clock, pipe_config-adjusted_mode.clock); -- 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: Add PIPE_CONF_CHECK_CLOCK_FUZZY()
From: Ville Syrjälä ville.syrj...@linux.intel.com Add a new pipe config check macro PIPE_CONF_CHECK_CLOCK_FUZZY() to make it trivial and error proof to compare clocks in a fuzzy manner. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1fea189..bfa531a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8601,6 +8601,15 @@ intel_pipe_config_compare(struct drm_device *dev, return false; \ } +#define PIPE_CONF_CHECK_CLOCK_FUZZY(name) \ + if (!intel_fuzzy_clock_check(current_config-name, pipe_config-name)) { \ + DRM_ERROR(mismatch in #name \ + (expected %i, found %i)\n, \ + current_config-name, \ + pipe_config-name); \ + return false; \ + } + #define PIPE_CONF_QUIRK(quirk) \ ((current_config-quirks | pipe_config-quirks) (quirk)) @@ -8663,21 +8672,16 @@ intel_pipe_config_compare(struct drm_device *dev, PIPE_CONF_CHECK_X(dpll_hw_state.fp0); PIPE_CONF_CHECK_X(dpll_hw_state.fp1); + if (!IS_HASWELL(dev)) { + PIPE_CONF_CHECK_CLOCK_FUZZY(adjusted_mode.clock); + } + #undef PIPE_CONF_CHECK_X #undef PIPE_CONF_CHECK_I #undef PIPE_CONF_CHECK_FLAGS +#undef PIPE_CONF_CHECK_CLOCK_FUZZY #undef PIPE_CONF_QUIRK - if (!IS_HASWELL(dev)) { - if (!intel_fuzzy_clock_check(current_config-adjusted_mode.clock, -pipe_config-adjusted_mode.clock)) { - DRM_ERROR(mismatch in clock (expected %d, found %d)\n, - current_config-adjusted_mode.clock, - pipe_config-adjusted_mode.clock); - return false; - } - } - return true; } -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Add fuzzy clock check for port_clock
From: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bfa531a..05ff91d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8674,6 +8674,7 @@ intel_pipe_config_compare(struct drm_device *dev, if (!IS_HASWELL(dev)) { PIPE_CONF_CHECK_CLOCK_FUZZY(adjusted_mode.clock); + PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock); } #undef PIPE_CONF_CHECK_X -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] drm/i915: Fuzzy clock check for port_clock
As requested by Daniel. Totally untested as of now. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] No fastboot on Debian 64 bits despite Linux 3.10 xserver-xorg-video-intel 2.21.15 (IvyBridge)
Hi, I've read carefully the xf86-video-intel 2.21.11 announce on that mailing list with its fastboot promise. However i don't have it on my IvyBridge system with xserver-xorg-video-intel 2.21.15 - see my boot sequence video which resizes here : http://libre-ouvert.toile-libre.org/data/documents/MVI_3575.AVI (48,8 Mo, 26 sec) Please tell me if i have to report a bug against Intel driver or against Debian. Thanks (Note that i actually don't have tty when i suspend to disk / wake up my system as promised with Linux 3.10 - and it's great :) - Some details on my system : Intel Core i3-3225 CPU (Ivybridge) + Intel H77 chipset Debian GNU/Linux Sid 64 bits $ cat /proc/version Linux version 3.10-2-amd64 (debian-ker...@lists.debian.org) (gcc version 4.7.3 (Debian 4.7.3-6) ) #1 SMP Debian 3.10.7-1 (2013-08-17) $ dpkg -s xserver-xorg-video-intel Package: xserver-xorg-video-intel Status: install ok installed Priority: optional Section: x11 Installed-Size: 2468 Maintainer: Debian X Strike Force debia...@lists.debian.org Architecture: amd64 Version: 2:2.21.15-1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: add some assertions about VBT DDI port types
Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com On Wed, Aug 28, 2013 at 12:45 PM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Our code makes a lot of assumptions regarding what each DDI port actually supports, and the VBT should tell us what is really happening in the hardware. So parse the information provided by the VBT and check if any of our assumptions is wrong. Our driver also has a history of not really trusting the VBT, so a WARN here could mean that: a) our coding assumptions are wrong b) the VBT is wrong c) we're incorrectly parsing the VBT d) the checks are wrong But I really hope we won't ever trigger any of those WARNs. v2: Don't check the redundant Capabilities field from byte 24 since it doesn't seem to be used. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_bios.c | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index a6700ab..4003dbf 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -575,7 +575,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, struct ddi_vbt_port_info *info = dev_priv-vbt.ddi_port_info[port]; uint8_t hdmi_level_shift; int i, j; - bool is_dvi, is_dp; + bool is_dvi, is_hdmi, is_dp, is_edp, is_crt; uint8_t aux_channel; int dvo_ports[][2] = { {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0}, @@ -604,6 +604,24 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, is_dvi = child-common.device_type (1 4); is_dp = child-common.device_type (1 2); + is_crt = child-common.device_type (1 0); + is_hdmi = is_dvi (child-common.device_type (1 11)) == 0; + is_edp = is_dp (child-common.device_type (1 12)); + + DRM_DEBUG_KMS(Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n, + port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt); + + WARN(is_edp is_dvi, Internal DP port %c is TMDS compatible\n, +port_name(port)); + WARN(is_crt port != PORT_E, Port %c is analog\n, port_name(port)); + WARN(is_crt (is_dvi || is_dp), +Analog port %c is also DP or TMDS compatible\n, port_name(port)); + WARN(is_dvi (port == PORT_A || port == PORT_E), +Port %c is TMDS compabile\n, port_name(port)); + WARN(!is_dvi !is_dp !is_crt, +Port %c is not DP/TMDS/CRT compatible\n, port_name(port)); + WARN(is_edp (port == PORT_B || port == PORT_C || port == PORT_E), +Port %c is internal DP\n, port_name(port)); if (is_dvi) { WARN(child-common.ddc_pin == 0x05 port != PORT_B, -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 6/8] drm/i915: Add bind/unbind object functions to VM
On Fri, Aug 30, 2013 at 08:40:36PM -0700, Ben Widawsky wrote: On Sat, Aug 31, 2013 at 01:12:55AM +0100, Chris Wilson wrote: On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote: From: Ben Widawsky b...@bwidawsk.net As we plumb the code with more VM information, it has become more obvious that the easiest way to deal with bind and unbind is to simply put the function pointers in the vm, and let those choose the correct way to handle the page table updates. This change allows many places in the code to simply be vm-bind, and not have to worry about distinguishing PPGTT vs GGTT. Notice that this patch has no impact on functionality. I've decided to save the actual change until the next patch because I think it's easier to review that way. I'm happy to squash the two, or let Daniel do it on merge. v2: Make ggtt handle the quirky aliasing ppgtt Add flags to bind object to support above Don't ever call bind/unbind directly for PPGTT until we have real, full PPGTT (use NULLs to assert this) Make sure we rebind the ggtt if there already is a ggtt binding. This happens on set cache levels Use VMA for bind/unbind (Daniel, Ben) Signed-off-by: Ben Widawsky b...@bwidawsk.net You like pokkadot paint for its inconsistency? Other than interesting alternation of styles, I see nothing wrong with the logic. -Chris To what are you referring? I'm probably more than willing to change whatever displeases you. You were alternating between using temporary variables like const unsigned long entry = vma-node.start PAGE_SHIFT; and doing the computation inline, with no consistency as far as I could see. It was so very minor, but it looks like cut'n'paste code from multiple sources. -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/3] drm/i915: check the DDC and AUX bits of the VBT on DDI machines
Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com On Wed, Aug 28, 2013 at 12:45 PM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Our code currently assumes that port X will use the DP AUX channel X and the DDC pin X. The VBT should tell us how things are mapped, so add some WARNs in case we discover our assumptions are wrong (or in case the VBT is just wrong, which is also perfectly possible). Why would someone wire port B to AUX C and DDC D? Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_bios.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 79bb651..a6700ab 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -575,6 +575,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, struct ddi_vbt_port_info *info = dev_priv-vbt.ddi_port_info[port]; uint8_t hdmi_level_shift; int i, j; + bool is_dvi, is_dp; + uint8_t aux_channel; int dvo_ports[][2] = { {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0}, }; @@ -598,6 +600,31 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, if (!child) return; + aux_channel = child-raw[25]; + + is_dvi = child-common.device_type (1 4); + is_dp = child-common.device_type (1 2); + + if (is_dvi) { + WARN(child-common.ddc_pin == 0x05 port != PORT_B, +Unexpected DDC pin for port B\n); + WARN(child-common.ddc_pin == 0x04 port != PORT_C, +Unexpected DDC pin for port C\n); + WARN(child-common.ddc_pin == 0x06 port != PORT_D, +Unexpected DDC pin for port D\n); + } + + if (is_dp) { + WARN(aux_channel == 0x40 port != PORT_A, +Unexpected AUX channel for port A\n); + WARN(aux_channel == 0x10 port != PORT_B, +Unexpected AUX channel for port B\n); + WARN(aux_channel == 0x20 port != PORT_C, +Unexpected AUX channel for port C\n); + WARN(aux_channel == 0x30 port != PORT_D, +Unexpected AUX channel for port D\n); + } + if (bdb-version = 158) { /* The VBT HDMI level shift values match the table we have. */ hdmi_level_shift = child-raw[7] 0xF; -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 3/3] drm/i915: don't init DP or HDMI when not supported by DDI port
Do we really trust VBT that much? Anyways, Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com On Wed, Aug 28, 2013 at 12:45 PM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com There's no reason to init a DP connector if the encoder just supports HDMI: we'll just waste hundreds and hundreds of cycles trying to do DP AUX transactions to detect if there's something there. Same goes for a DP connector that doesn't support HDMI, but I'm not sure these actually exist. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_bios.c | 13 - drivers/gpu/drm/i915/intel_ddi.c | 30 ++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 645dd74..8720f31 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1041,6 +1041,9 @@ enum modeset_restore { struct ddi_vbt_port_info { uint8_t hdmi_level_shift; + bool supports_dvi; + bool supports_hdmi; + bool supports_dp; }; struct intel_vbt_data { diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 4003dbf..cd823b9 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -608,6 +608,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, is_hdmi = is_dvi (child-common.device_type (1 11)) == 0; is_edp = is_dp (child-common.device_type (1 12)); + info-supports_dvi = is_dvi; + info-supports_hdmi = is_hdmi; + info-supports_dp = is_dp; + DRM_DEBUG_KMS(Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n, port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt); @@ -762,8 +766,15 @@ init_vbt_defaults(struct drm_i915_private *dev_priv) enum port port; for (port = PORT_A; port = PORT_E; port++) { + struct ddi_vbt_port_info *info = + dev_priv-vbt.ddi_port_info[port]; + /* Recommended BSpec default: 800mV 0dB. */ - dev_priv-vbt.ddi_port_info[port].hdmi_level_shift = 6; + info-hdmi_level_shift = 6; + + info-supports_dvi = (port != PORT_A port != PORT_E); + info-supports_hdmi = info-supports_dvi; + info-supports_dp = (port != PORT_E); } } diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index ece226d..d344977 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1325,6 +1325,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port) struct drm_encoder *encoder; struct intel_connector *hdmi_connector = NULL; struct intel_connector *dp_connector = NULL; + bool init_hdmi, init_dp; + + init_hdmi = (dev_priv-vbt.ddi_port_info[port].supports_dvi || +dev_priv-vbt.ddi_port_info[port].supports_hdmi); + init_dp = dev_priv-vbt.ddi_port_info[port].supports_dp; + if (!init_dp !init_hdmi) { + DRM_ERROR(VBT says port %c is not DVI/HDMI/DP compatible\n, + port_name(port)); + init_hdmi = true; + init_dp = true; + } intel_dig_port = kzalloc(sizeof(struct intel_digital_port), GFP_KERNEL); if (!intel_dig_port) @@ -1362,19 +1373,22 @@ void intel_ddi_init(struct drm_device *dev, enum port port) intel_encoder-cloneable = false; intel_encoder-hot_plug = intel_ddi_hot_plug; - if (!intel_dp_init_connector(intel_dig_port, dp_connector)) { - drm_encoder_cleanup(encoder); - kfree(intel_dig_port); - kfree(dp_connector); - return; + if (init_dp) { + if (!intel_dp_init_connector(intel_dig_port, dp_connector)) { + drm_encoder_cleanup(encoder); + kfree(intel_dig_port); + kfree(dp_connector); + return; + } } - if (intel_encoder-type != INTEL_OUTPUT_EDP) { + /* In theory we don't need the encoder-type check, but leave it just in +* case we have some really bad VBTs... */ + if (intel_encoder-type != INTEL_OUTPUT_EDP init_hdmi) { hdmi_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL); - if (!hdmi_connector) { + if (!hdmi_connector) return; - } intel_dig_port-hdmi.hdmi_reg = DDI_BUF_CTL(port);
Re: [Intel-gfx] [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT
On Wed, Aug 28, 2013 at 12:39 PM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We currently use the recommended values from BSpec, but the VBT specifies the correct value to use for the hardware we have, so use it. We also fall back to the recommended value in case we can't find the VBT. In addition, this code also provides some infrastructure to parse more information about the DDI ports. There's a lot more information we could extract and use in the future. v2: - Move some code to init_vbt_defaults. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 6 drivers/gpu/drm/i915/intel_bios.c | 69 +++ drivers/gpu/drm/i915/intel_ddi.c | 24 -- 3 files changed, 97 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 08fe1b6..645dd74 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1039,6 +1039,10 @@ enum modeset_restore { MODESET_SUSPENDED, }; +struct ddi_vbt_port_info { + uint8_t hdmi_level_shift; +}; + struct intel_vbt_data { struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */ struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */ @@ -1068,6 +1072,8 @@ struct intel_vbt_data { int child_dev_num; union child_device_config *child_dev; + + struct ddi_vbt_port_info ddi_port_info[5]; }; enum intel_ddb_partitioning { diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 8a27925..79bb651 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -568,6 +568,63 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb) } } +static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, + struct bdb_header *bdb) +{ + union child_device_config *it, *child = NULL; + struct ddi_vbt_port_info *info = dev_priv-vbt.ddi_port_info[port]; + uint8_t hdmi_level_shift; + int i, j; + int dvo_ports[][2] = { + {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0}, + }; + int n_dvo_ports[] = {2, 2, 2, 2, 1}; I hadn't get this until you explain on irc, but I have no better idea how to make it more clear, so just ignore me ;) + + /* Find the child device to use, abort if more than one found. */ + for (i = 0; i dev_priv-vbt.child_dev_num; i++) { + it = dev_priv-vbt.child_dev + i; + + for (j = 0; j n_dvo_ports[port]; j++) { + if (it-common.dvo_port == dvo_ports[port][j]) { + if (child) { + DRM_ERROR(More than one child device for port %c in VBT.\n, + port_name(port)); + return; + } + child = it; + } + } + } + if (!child) + return; + + if (bdb-version = 158) { 0x8 is available since 157... is it useful to include it? + /* The VBT HDMI level shift values match the table we have. */ + hdmi_level_shift = child-raw[7] 0xF; + if (hdmi_level_shift 0xC) { + DRM_DEBUG_KMS(VBT HDMI level shift for port %c: %d\n, + port_name(port), + hdmi_level_shift); + info-hdmi_level_shift = hdmi_level_shift; + } + } +} + +static void parse_ddi_ports(struct drm_i915_private *dev_priv, + struct bdb_header *bdb) +{ + enum port port; + + if (!dev_priv-vbt.child_dev_num) + return; + + if (bdb-version 155) + return; Some time in the past I had thought about a similar check, but the version itself was added after 155... So not sure how effective is this... + + for (port = PORT_A; port = PORT_E; port++) + parse_ddi_port(dev_priv, port, bdb); +} + static void parse_device_mapping(struct drm_i915_private *dev_priv, struct bdb_header *bdb) @@ -655,6 +712,16 @@ init_vbt_defaults(struct drm_i915_private *dev_priv) dev_priv-vbt.lvds_use_ssc = 1; dev_priv-vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1); DRM_DEBUG_KMS(Set default to SSC at %dMHz\n, dev_priv-vbt.lvds_ssc_freq); + + if (HAS_DDI(dev)) { + enum port port; + + for (port = PORT_A; port = PORT_E; port++) { + /* Recommended BSpec default: 800mV 0dB. */ +
Re: [Intel-gfx] [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT
On Tue, Sep 03, 2013 at 11:37:27AM -0300, Rodrigo Vivi wrote: On Wed, Aug 28, 2013 at 12:39 PM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We currently use the recommended values from BSpec, but the VBT specifies the correct value to use for the hardware we have, so use it. We also fall back to the recommended value in case we can't find the VBT. In addition, this code also provides some infrastructure to parse more information about the DDI ports. There's a lot more information we could extract and use in the future. v2: - Move some code to init_vbt_defaults. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 6 drivers/gpu/drm/i915/intel_bios.c | 69 +++ drivers/gpu/drm/i915/intel_ddi.c | 24 -- 3 files changed, 97 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 08fe1b6..645dd74 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1039,6 +1039,10 @@ enum modeset_restore { MODESET_SUSPENDED, }; +struct ddi_vbt_port_info { + uint8_t hdmi_level_shift; +}; + struct intel_vbt_data { struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */ struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */ @@ -1068,6 +1072,8 @@ struct intel_vbt_data { int child_dev_num; union child_device_config *child_dev; + + struct ddi_vbt_port_info ddi_port_info[5]; }; enum intel_ddb_partitioning { diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 8a27925..79bb651 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -568,6 +568,63 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb) } } +static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, + struct bdb_header *bdb) +{ + union child_device_config *it, *child = NULL; + struct ddi_vbt_port_info *info = dev_priv-vbt.ddi_port_info[port]; + uint8_t hdmi_level_shift; + int i, j; + int dvo_ports[][2] = { + {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0}, + }; + int n_dvo_ports[] = {2, 2, 2, 2, 1}; I hadn't get this until you explain on irc, but I have no better idea how to make it more clear, so just ignore me ;) If you use -1 for the unused slot (and check for that value in the loop) you could ditch the n_dvo_ports array. Also I think the magic values in dvo_ports need a comment or so ... -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 1/8] drm/i915: Synchronize pread/pwrite with wait_rendering
On Mon, Sep 02, 2013 at 04:12:36PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 02:14:12PM +0100, Chris Wilson wrote: On Mon, Sep 02, 2013 at 08:32:24AM +0200, Daniel Vetter wrote: On Fri, Aug 30, 2013 at 08:39:46PM -0700, Ben Widawsky wrote: On Sat, Aug 31, 2013 at 12:50:30AM +0100, Chris Wilson wrote: On Fri, Aug 30, 2013 at 04:43:54PM -0700, Ben Widawsky wrote: lifted from Daniel: pread/pwrite isn't about the object's domain at all, but purely about synchronizing for outstanding rendering. Replacing the call to set_to_gtt_domain with a wait_rendering would imo improve code readability. Furthermore we could pimp pread to only block for outstanding writes and not for reads. Since you're not the first one to trip over this: Can I volunteer you for a follow-up patch to fix this? Recommended-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Ben Widawsky b...@bwidawsk.net This should fail i-g-t... -Chris Daniel, how have I failed your plan? It should work ... Since the enclosing if-block checks for !cpu domain (for either reads or writes) that implies that going into the gtt domain is a noop (or better should be) wrt clflushing and we only wait for outstanding gpu rendering. wait_rendering is an interface that's been added afterwards. Unfortunately I've failed to explain this trickery in either a comment or the commit message. Bad me ;-) The issue is that in the patch pwrite is not waiting for any outstanding GPU reads. Oh right, silly me didn't spot the s/true/false/ switch Ben sneaked in. This /should/ have been caught by the gem_concurrent_blt subtests that exercise pwrites ... Ben can you please check that this indeed blew up on igt? Should fail on any platform, no special caching mode required. Actually it won't blow up since you always set readonly = false. But it'll kill the neat read-read optimization ... -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] No fastboot on Debian 64 bits despite Linux 3.10 xserver-xorg-video-intel 2.21.15 (IvyBridge)
On Mon, Sep 02, 2013 at 12:02:54PM +0200, thibaut bethune wrote: Hi, I've read carefully the xf86-video-intel 2.21.11 announce on that mailing list with its fastboot promise. However i don't have it on my IvyBridge system with xserver-xorg-video-intel 2.21.15 - see my boot sequence video which resizes here : http://libre-ouvert.toile-libre.org/data/documents/MVI_3575.AVI (48,8 Mo, 26 sec) Please tell me if i have to report a bug against Intel driver or against Debian. All we mean is that we avoid extraneous mode changes. That your system doesn't boot to desktop under 2 seconds from after POST is a distribution bug. -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 3/3] drm/i915: don't init DP or HDMI when not supported by DDI port
2013/9/3 Rodrigo Vivi rodrigo.v...@gmail.com: Do we really trust VBT that much? We already trust it for some very important things, like eDP. Also, if the VBT is wrong we'll start seeing all sorts of WARNs due to the previous patches. Anyway, we can always revert... Thanks for the reviews! Anyways, Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com On Wed, Aug 28, 2013 at 12:45 PM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com There's no reason to init a DP connector if the encoder just supports HDMI: we'll just waste hundreds and hundreds of cycles trying to do DP AUX transactions to detect if there's something there. Same goes for a DP connector that doesn't support HDMI, but I'm not sure these actually exist. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_bios.c | 13 - drivers/gpu/drm/i915/intel_ddi.c | 30 ++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 645dd74..8720f31 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1041,6 +1041,9 @@ enum modeset_restore { struct ddi_vbt_port_info { uint8_t hdmi_level_shift; + bool supports_dvi; + bool supports_hdmi; + bool supports_dp; }; struct intel_vbt_data { diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 4003dbf..cd823b9 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -608,6 +608,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, is_hdmi = is_dvi (child-common.device_type (1 11)) == 0; is_edp = is_dp (child-common.device_type (1 12)); + info-supports_dvi = is_dvi; + info-supports_hdmi = is_hdmi; + info-supports_dp = is_dp; + DRM_DEBUG_KMS(Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n, port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt); @@ -762,8 +766,15 @@ init_vbt_defaults(struct drm_i915_private *dev_priv) enum port port; for (port = PORT_A; port = PORT_E; port++) { + struct ddi_vbt_port_info *info = + dev_priv-vbt.ddi_port_info[port]; + /* Recommended BSpec default: 800mV 0dB. */ - dev_priv-vbt.ddi_port_info[port].hdmi_level_shift = 6; + info-hdmi_level_shift = 6; + + info-supports_dvi = (port != PORT_A port != PORT_E); + info-supports_hdmi = info-supports_dvi; + info-supports_dp = (port != PORT_E); } } diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index ece226d..d344977 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1325,6 +1325,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port) struct drm_encoder *encoder; struct intel_connector *hdmi_connector = NULL; struct intel_connector *dp_connector = NULL; + bool init_hdmi, init_dp; + + init_hdmi = (dev_priv-vbt.ddi_port_info[port].supports_dvi || +dev_priv-vbt.ddi_port_info[port].supports_hdmi); + init_dp = dev_priv-vbt.ddi_port_info[port].supports_dp; + if (!init_dp !init_hdmi) { + DRM_ERROR(VBT says port %c is not DVI/HDMI/DP compatible\n, + port_name(port)); + init_hdmi = true; + init_dp = true; + } intel_dig_port = kzalloc(sizeof(struct intel_digital_port), GFP_KERNEL); if (!intel_dig_port) @@ -1362,19 +1373,22 @@ void intel_ddi_init(struct drm_device *dev, enum port port) intel_encoder-cloneable = false; intel_encoder-hot_plug = intel_ddi_hot_plug; - if (!intel_dp_init_connector(intel_dig_port, dp_connector)) { - drm_encoder_cleanup(encoder); - kfree(intel_dig_port); - kfree(dp_connector); - return; + if (init_dp) { + if (!intel_dp_init_connector(intel_dig_port, dp_connector)) { + drm_encoder_cleanup(encoder); + kfree(intel_dig_port); + kfree(dp_connector); + return; + } } - if (intel_encoder-type != INTEL_OUTPUT_EDP) { + /* In theory we don't need the encoder-type check, but leave it just in +* case we have some really bad VBTs... */ + if (intel_encoder-type != INTEL_OUTPUT_EDP init_hdmi) { hdmi_connector = kzalloc(sizeof(struct
Re: [Intel-gfx] [PATCH] drm/i915: i915.disable_pch_pwm overrides PCH_PWM_ENABLE quirk
On Tue, Sep 3, 2013 at 7:37 PM, Kamal Mostafa ka...@canonical.com wrote: BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=47941 Some BIOS configurations of Dell XPS13 are adversely affected by e85843b (drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight) so provide a boot param to inhibit the quirk, or force it on. i915.disable_pch_pwm can be set to -1: auto (default; allows the disabling of pch_pwm by dmi quirk table match) 0: inhibits the disabling of pch_pwm (overrides dmi quirk table match) 1: forces the disabling of pch_pwm Signed-off-by: Kamal Mostafa ka...@canonical.com Nack. Piling quirk over quirk isn't the right approach and I think I should just revert the pch_pwm enable quirk again. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: i915.disable_pch_pwm overrides PCH_PWM_ENABLE quirk
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=47941 Some BIOS configurations of Dell XPS13 are adversely affected by e85843b (drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight) so provide a boot param to inhibit the quirk, or force it on. i915.disable_pch_pwm can be set to -1: auto (default; allows the disabling of pch_pwm by dmi quirk table match) 0: inhibits the disabling of pch_pwm (overrides dmi quirk table match) 1: forces the disabling of pch_pwm Signed-off-by: Kamal Mostafa ka...@canonical.com --- drivers/gpu/drm/i915/i915_drv.c | 4 drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 11 --- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 72e2be7..fee05df 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -154,6 +154,10 @@ module_param_named(prefault_disable, i915_prefault_disable, bool, 0600); MODULE_PARM_DESC(prefault_disable, Disable page prefaulting for pread/pwrite/reloc (default:false). For developers only.); +int i915_disable_pch_pwm __read_mostly = -1; +module_param_named(disable_pch_pwm, i915_disable_pch_pwm, int, 0600); +MODULE_PARM_DESC(disable_pch_pwm, disable PCH_PWM (default: -1 (auto))); + static struct drm_driver driver; extern int intel_agp_enabled; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 769c138..e6f2a34 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1726,6 +1726,7 @@ extern bool i915_fastboot __read_mostly; extern int i915_enable_pc8 __read_mostly; extern int i915_pc8_timeout __read_mostly; extern bool i915_prefault_disable __read_mostly; +extern int i915_disable_pch_pwm __read_mostly; extern int i915_suspend(struct drm_device *dev, pm_message_t state); extern int i915_resume(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 30e5946..86fa722 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9948,9 +9948,8 @@ static void quirk_invert_brightness(struct drm_device *dev) */ static void quirk_no_pcm_pwm_enable(struct drm_device *dev) { - struct drm_i915_private *dev_priv = dev-dev_private; - dev_priv-quirks |= QUIRK_NO_PCH_PWM_ENABLE; - DRM_INFO(applying no-PCH_PWM_ENABLE quirk\n); + if (i915_disable_pch_pwm 0) + i915_disable_pch_pwm = 1; } struct intel_quirk { @@ -10048,6 +10047,12 @@ static void intel_init_quirks(struct drm_device *dev) if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0) intel_dmi_quirks[i].hook(dev); } + + if (i915_disable_pch_pwm == 1) { + struct drm_i915_private *dev_priv = dev-dev_private; + dev_priv-quirks |= QUIRK_NO_PCH_PWM_ENABLE; + DRM_INFO(applying no-PCH_PWM_ENABLE quirk\n); + } } /* Disable the VGA plane that we never use */ -- 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 1/2] drm/i915: handle sdvo input pixel multiplier correctly again
On Tue, Sep 03, 2013 at 08:40:36PM +0200, Daniel Vetter wrote: The sdvo input timing needs to be the actual mode, the sdvo encoder automatically adjusts for the need of pixel doubling or quadrupling. This was lost in pipe config conversion of the pixel multiplier in commit 6cc5f341b5830541a1b6945435ca90c69b1b8b21 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Wed Mar 27 00:44:53 2013 +0100 drm/i915: add pipe_config-pixel_multiplier While at it ditch the intel_ prefix from the crtc in intel_sdvo_mode_set. Cc: Jesse Barnes jbar...@virtuousgeek.org Cc: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch For both patches: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com -- 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: i915.disable_pch_pwm overrides PCH_PWM_ENABLE quirk
On Tue, 2013-09-03 at 19:50 +0200, Daniel Vetter wrote: On Tue, Sep 3, 2013 at 7:37 PM, Kamal Mostafa ka...@canonical.com wrote: BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=47941 Some BIOS configurations of Dell XPS13 are adversely affected by e85843b (drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight) so provide a boot param to inhibit the quirk, or force it on. i915.disable_pch_pwm can be set to -1: auto (default; allows the disabling of pch_pwm by dmi quirk table match) 0: inhibits the disabling of pch_pwm (overrides dmi quirk table match) 1: forces the disabling of pch_pwm Signed-off-by: Kamal Mostafa ka...@canonical.com Nack. Piling quirk over quirk isn't the right approach I understand your reluctance, but this isn't actually any new quirk functionality, just a way to manually enable/disable the original PCH_PWM_ENABLE quirk. I think this is the least crazy approach, because: Most XPS13 configurations do need the quirk (and maybe some other yet to be identified machines also), but dmi matching cannot discern the one particular XPS13 configuration (Ivy Bridge booting UEFI mode without Legacy Option ROM) that is adversely affected by it. We could alternately consider trying to detect that specific configuration with code in i915, but that seemed a lot crazier (and less generally useful) than just providing an override switch for rare or yet-to-be-discovered configurations. Hmmm. What if we had a pair of boot params i915.quirks_set and i915.quirks_mask boot params that could be used to manually set or mask _all_ the bits in dev_priv-quirks? Such params would surely come in handy for cases just like this one, and would be useful for testing future machines easily. (Would you take that if I submitted it?) and I think I should just revert the pch_pwm enable quirk again. -Daniel But reverting the original quirk would break ALL the XPS13 configurations, which nobody is requesting. Please don't revert the quirk. At most, you might want to disable the Ivy Bridge dmi match (but I don't recommend this either): /* Dell XPS13 HD and XPS13 FHD Ivy Bridge */ { 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable }, -Kamal signature.asc Description: This is a digitally signed message part ___ 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: handle sdvo input pixel multiplier correctly again
On Tue, Sep 03, 2013 at 09:53:15PM +0300, Ville Syrjälä wrote: On Tue, Sep 03, 2013 at 08:40:36PM +0200, Daniel Vetter wrote: The sdvo input timing needs to be the actual mode, the sdvo encoder automatically adjusts for the need of pixel doubling or quadrupling. This was lost in pipe config conversion of the pixel multiplier in commit 6cc5f341b5830541a1b6945435ca90c69b1b8b21 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Wed Mar 27 00:44:53 2013 +0100 drm/i915: add pipe_config-pixel_multiplier While at it ditch the intel_ prefix from the crtc in intel_sdvo_mode_set. Cc: Jesse Barnes jbar...@virtuousgeek.org Cc: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch For both patches: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Picked up for -fixes, thanks for the review. I'll also rebase dinq so that we can slurp in your patches on top of this. -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 1/2] drm/i915: handle sdvo input pixel multiplier correctly again
The sdvo input timing needs to be the actual mode, the sdvo encoder automatically adjusts for the need of pixel doubling or quadrupling. This was lost in pipe config conversion of the pixel multiplier in commit 6cc5f341b5830541a1b6945435ca90c69b1b8b21 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Wed Mar 27 00:44:53 2013 +0100 drm/i915: add pipe_config-pixel_multiplier While at it ditch the intel_ prefix from the crtc in intel_sdvo_mode_set. Cc: Jesse Barnes jbar...@virtuousgeek.org Cc: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_sdvo.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 317e058..85037b9 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1151,11 +1151,10 @@ static void intel_sdvo_mode_set(struct intel_encoder *intel_encoder) { struct drm_device *dev = intel_encoder-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; - struct drm_crtc *crtc = intel_encoder-base.crtc; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc *crtc = to_intel_crtc(intel_encoder-base.crtc); struct drm_display_mode *adjusted_mode = - intel_crtc-config.adjusted_mode; - struct drm_display_mode *mode = intel_crtc-config.requested_mode; + crtc-config.adjusted_mode; + struct drm_display_mode *mode = crtc-config.requested_mode; struct intel_sdvo *intel_sdvo = to_sdvo(intel_encoder); u32 sdvox; struct intel_sdvo_in_out_map in_out; @@ -1213,13 +1212,15 @@ static void intel_sdvo_mode_set(struct intel_encoder *intel_encoder) * adjusted_mode. */ intel_sdvo_get_dtd_from_mode(input_dtd, adjusted_mode); + input_dtd.part1.clock /= crtc-config.pixel_multiplier; + if (intel_sdvo-is_tv || intel_sdvo-is_lvds) input_dtd.part2.sdvo_flags = intel_sdvo-dtd_sdvo_flags; if (!intel_sdvo_set_input_timing(intel_sdvo, input_dtd)) DRM_INFO(Setting input timings on %s failed\n, SDVO_NAME(intel_sdvo)); - switch (intel_crtc-config.pixel_multiplier) { + switch (crtc-config.pixel_multiplier) { default: WARN(1, unknown pixel mutlipler specified\n); case 1: rate = SDVO_CLOCK_RATE_MULT_1X; break; @@ -1252,9 +1253,9 @@ static void intel_sdvo_mode_set(struct intel_encoder *intel_encoder) } if (INTEL_PCH_TYPE(dev) = PCH_CPT) - sdvox |= SDVO_PIPE_SEL_CPT(intel_crtc-pipe); + sdvox |= SDVO_PIPE_SEL_CPT(crtc-pipe); else - sdvox |= SDVO_PIPE_SEL(intel_crtc-pipe); + sdvox |= SDVO_PIPE_SEL(crtc-pipe); if (intel_sdvo-has_hdmi_audio) sdvox |= SDVO_AUDIO_ENABLE; @@ -1264,7 +1265,7 @@ static void intel_sdvo_mode_set(struct intel_encoder *intel_encoder) } else if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev)) { /* done in crtc_mode_set as it lives inside the dpll register */ } else { - sdvox |= (intel_crtc-config.pixel_multiplier - 1) + sdvox |= (crtc-config.pixel_multiplier - 1) SDVO_PORT_MULTIPLY_SHIFT; } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: fix i9xx_crtc_clock_get for multiplied pixels
The dpll actually runs at the port clock so we don't need to multiply it again with the pixel multiplier to get the adjusted_mode.clock. This is in contrast to the ironlake pixel clock readout code which uses the fdi dotclock: That one does _not_ run with multiplied pixels. This issue goes back to the original clock readout code added in commit f1f644dc66cbaf5a4c7dcde683361536b41885b9 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Thu Jun 27 00:39:25 2013 +0300 drm/i915: get mode clock when reading the pipe config v9 Cc: Jesse Barnes jbar...@virtuousgeek.org Cc: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bcb62fe..1da200e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7309,8 +7309,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc, } } - pipe_config-adjusted_mode.clock = clock.dot * - pipe_config-pixel_multiplier; + pipe_config-adjusted_mode.clock = clock.dot; } static void ironlake_crtc_clock_get(struct intel_crtc *crtc, -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linus-next: update to the drm-intel-fixes tree
Hi Daniel, This morning after fetching the drm-intel-fixes tree, I have discovered that you have just added a whole set of patches on top of Dave's drm tree and made that the drm-intel-fixes tree. I understood that this tree was for fixes to Linus' tree for the current release cycle. Given that Dave's tree has not been merged by Linus (yet), this is a bit inconsistent. Not only that, but now if I merge your tree early (which is what I do with the -fixes trees), I will also merge all of Dave's tree. That in turn would mean that I would have missed the (what would have been automatically applied) merge for I am carrying for Dave's tree. :-( I am going to have to drop your -fixes tree for today. If these are fixes for stuff in Linus' tree, then they should have been based on Linus' tree - if they are fixes for Dave's tree, then they should have been in your drm-intel tree, right? (fetches more trees ...) And now, I discover that they are the latter :-) So your -fixes tree will be dropped, but all those patches will still be merged via you drm-intel tree. -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpvp_Gay9CY8.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/8] drm/i915: Synchronize pread/pwrite with wait_rendering
On Tue, Sep 03, 2013 at 06:08:19PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 04:12:36PM +0200, Daniel Vetter wrote: On Mon, Sep 02, 2013 at 02:14:12PM +0100, Chris Wilson wrote: On Mon, Sep 02, 2013 at 08:32:24AM +0200, Daniel Vetter wrote: On Fri, Aug 30, 2013 at 08:39:46PM -0700, Ben Widawsky wrote: On Sat, Aug 31, 2013 at 12:50:30AM +0100, Chris Wilson wrote: On Fri, Aug 30, 2013 at 04:43:54PM -0700, Ben Widawsky wrote: lifted from Daniel: pread/pwrite isn't about the object's domain at all, but purely about synchronizing for outstanding rendering. Replacing the call to set_to_gtt_domain with a wait_rendering would imo improve code readability. Furthermore we could pimp pread to only block for outstanding writes and not for reads. Since you're not the first one to trip over this: Can I volunteer you for a follow-up patch to fix this? Recommended-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Ben Widawsky b...@bwidawsk.net This should fail i-g-t... -Chris Daniel, how have I failed your plan? It should work ... Since the enclosing if-block checks for !cpu domain (for either reads or writes) that implies that going into the gtt domain is a noop (or better should be) wrt clflushing and we only wait for outstanding gpu rendering. wait_rendering is an interface that's been added afterwards. Unfortunately I've failed to explain this trickery in either a comment or the commit message. Bad me ;-) The issue is that in the patch pwrite is not waiting for any outstanding GPU reads. Oh right, silly me didn't spot the s/true/false/ switch Ben sneaked in. This /should/ have been caught by the gem_concurrent_blt subtests that exercise pwrites ... Ben can you please check that this indeed blew up on igt? Should fail on any platform, no special caching mode required. Actually it won't blow up since you always set readonly = false. But it'll kill the neat read-read optimization ... -Daniel Doh! Sorry about this. Fixed locally. -- 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 6/8] drm/i915: Add bind/unbind object functions to VM
On Mon, Sep 02, 2013 at 03:46:52PM +0300, Ville Syrjälä wrote: On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote: From: Ben Widawsky b...@bwidawsk.net As we plumb the code with more VM information, it has become more obvious that the easiest way to deal with bind and unbind is to simply put the function pointers in the vm, and let those choose the correct way to handle the page table updates. This change allows many places in the code to simply be vm-bind, and not have to worry about distinguishing PPGTT vs GGTT. Notice that this patch has no impact on functionality. I've decided to save the actual change until the next patch because I think it's easier to review that way. I'm happy to squash the two, or let Daniel do it on merge. v2: Make ggtt handle the quirky aliasing ppgtt Add flags to bind object to support above Don't ever call bind/unbind directly for PPGTT until we have real, full PPGTT (use NULLs to assert this) Make sure we rebind the ggtt if there already is a ggtt binding. This happens on set cache levels Use VMA for bind/unbind (Daniel, Ben) Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_drv.h | 69 +--- drivers/gpu/drm/i915/i915_gem_gtt.c | 101 2 files changed, 140 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c9ed77a..377ca63 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -461,6 +461,36 @@ enum i915_cache_level { typedef uint32_t gen6_gtt_pte_t; +/** + * A VMA represents a GEM BO that is bound into an address space. Therefore, a + * VMA's presence cannot be guaranteed before binding, or after unbinding the + * object into/from the address space. + * + * To make things as simple as possible (ie. no refcounting), a VMA's lifetime + * will always be = an objects lifetime. So object refcounting should cover us. + */ +struct i915_vma { + struct drm_mm_node node; + struct drm_i915_gem_object *obj; + struct i915_address_space *vm; + + /** This object's place on the active/inactive lists */ + struct list_head mm_list; + + struct list_head vma_link; /* Link in the object's VMA list */ + + /** This vma's place in the batchbuffer or on the eviction list */ + struct list_head exec_list; + + /** +* Used for performing relocations during execbuffer insertion. +*/ + struct hlist_node exec_node; + unsigned long exec_handle; + struct drm_i915_gem_exec_object2 *exec_entry; + +}; + struct i915_address_space { struct drm_mm mm; struct drm_device *dev; @@ -499,9 +529,18 @@ struct i915_address_space { /* FIXME: Need a more generic return type */ gen6_gtt_pte_t (*pte_encode)(dma_addr_t addr, enum i915_cache_level level); + + /** Unmap an object from an address space. This usually consists of +* setting the valid PTE entries to a reserved scratch page. */ + void (*unbind_vma)(struct i915_vma *vma); void (*clear_range)(struct i915_address_space *vm, unsigned int first_entry, unsigned int num_entries); + /* Map an object into an address space with the given cache flags. */ +#define GLOBAL_BIND (10) + void (*bind_vma)(struct i915_vma *vma, +enum i915_cache_level cache_level, +u32 flags); void (*insert_entries)(struct i915_address_space *vm, struct sg_table *st, unsigned int first_entry, @@ -548,36 +587,6 @@ struct i915_hw_ppgtt { int (*enable)(struct drm_device *dev); }; -/** - * A VMA represents a GEM BO that is bound into an address space. Therefore, a - * VMA's presence cannot be guaranteed before binding, or after unbinding the - * object into/from the address space. - * - * To make things as simple as possible (ie. no refcounting), a VMA's lifetime - * will always be = an objects lifetime. So object refcounting should cover us. - */ -struct i915_vma { - struct drm_mm_node node; - struct drm_i915_gem_object *obj; - struct i915_address_space *vm; - - /** This object's place on the active/inactive lists */ - struct list_head mm_list; - - struct list_head vma_link; /* Link in the object's VMA list */ - - /** This vma's place in the batchbuffer or on the eviction list */ - struct list_head exec_list; - - /** -* Used for performing relocations during execbuffer insertion. -*/ - struct hlist_node exec_node; - unsigned long exec_handle; - struct drm_i915_gem_exec_object2 *exec_entry; - -}; - struct i915_ctx_hang_stats { /* This context had