[Intel-gfx] [PATCH 00/43] [RFC] modeset rework, part 1
Hi all, This patch bomb is just the prep work for the actual rework ;-) The goal of this little adventure is to move away from the crtc helper code, which has the fundamental assumption that encoders and crtc can be enabled/disabled in any order, as long as we take care of depencies. Our hw works differently. We already have tons of ugly cases where crtc code enable encoder hw (or encoder-mode_set enables stuff that should only be enabled in enocder-commit) to work around these issues. But on the disable side we can't pull off similar tricks - there we actually need to rework the modeset sequence that controls all this. Quick summary of the changes in here - the patches that actually introduce a new concept all have rather extensive commit messages to explain things: - Add new encoder-enable/disable functions which are directly called from the crtc-enable/disable function. This ensures that the encoder's can be enabled/disabled at a very specific in the modeset sequence, controlled by our platform specific code. - Rework the dpms code - our code has mostly 1:1 connector:encoder mappings and does support cloning on on a few encoders, so we can simplify things quite a bit. Also only ever disable/enable the entire output pipeline - this ensures that we obey the right sequence of enabling/disabling things. For cloneable encoders this requires a bit of special handling to ensure that outputs can still be disabled individually, but it simplifies the common case quite a bit. - Add infrastructure to read out the current hw state. No amount of careful ordering will help us if we brick the hw on the first modesetup. Which could happen if we just randomly disable things, oblivious to the state set up by the bios. Hence we need to be able to read that out. As a benefit, we grow a few generic functions useful to cross-check our modeset code with actual hw state. - A few little changes/cleanups in a few encoders to make the above less onerous. Things that are still needed: - Reworking the modeset code - the current copypasted code still pretty much works like the crtc helper and doesn't disable things correctly yet. - Once the above is in place, add new hooks to enable special connectors at the right place. I'm thinking of encoder-pre_enable and encoder-post_disable, which would each run after/before the plane/pipe/plls. - Actually fix the cpu edp bugs (and port the hsw dp code) with the new hooks. And further out, i.e. auxiliary benefits: - Fix up our confusion in the resume path - we enable a few encoders by accident when restoring registers. - Clean things up by moving a few of the existing hacks around, e.g. the edp pll enabling or the lvds/pll enabling. - fastboot - the hw state readout code is neat prep work for that, the patch itself lists some of the missing bits. - Fixing all the bugs turned up by the more stringent cross-checking the new code allows - the last patch in this series gives a glimpse, but I plan to add much more after the modeset sequence has saner semantics. Patch series lightly tested, atm I'm beating on it with my machines. Due to lack of hw, I'm looking for people with TV out and dvo encoders ... Comments, flames, ideas and rants highly welcome. I hope that I can send out and RFC with the new modeset code in a few days at most. Cheers, Daniel Daniel Vetter (43): drm/i915: introduce for_each_encoder_on_crtc drm/i915: add crtc-enable/disable vfuncs insted of dpms drm/i915: rip out crtc prepare/commit indirection drm/i915: add direct encoder disable/enable infrastructure drm/i915: add missing gen2 pipe A quirk entries drm/i915: rip out the overlay pipe A workaround drm/i915: prepare load-detect pipe code for dpms changes drm/i915/hdmi: convert to encoder-disable/enable drm/i915/tv: convert to encoder enable/disable drm/i915/lvds: ditch -prepare special case drm/i915/lvds: convert to encoder disable/enable drm/i915/dp: convert to encoder disable/enable drm/i915: create VLV_DSIPLAY_BASE #define drm/i915: group ADPA #defines together drm/i915: add inte_crt-adpa_reg drm/i915/crt: convert to encoder disable/enable drm/i915/sdvo: convert to encoder disabl/enable drm/i915: simplify dvo dpms interface drm/i915: simplify possible_clones computation drm/i915: add port parameter to intel_hdmi_init drm/i915: convert dpms functions of dvo/sdvo/crt drm/i915: rip out encoder-disable/enable checks drm/i915: clean up encoder_prepare/commit drm/fb helper: don't call drm_crtc_helper_set_config drm: remove the list_head from drm_mode_set drm/i915: copypaste drm_crtc_helper_set_config drm/i915: call set_base directly drm/i915: inline intel_best_encoder drm/i915: copypaste drm_crtc_helper_set_mode drm/i915: simplify intel_crtc_prepare_encoders drm/i915: rip out encoder-prepare/commit drm/i915: call crtc functions directly drm/i915: WARN when trying to enabled an unused crtc drm/i915: Add
[Intel-gfx] [PATCH 01/43] drm/i915: introduce for_each_encoder_on_crtc
We already have this pattern at quite a few places, and moving part of the modeset helper stuff into the driver will add more. v2: Don't clobber the crtc struct name with the macro parameter ... Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.h |4 +++ drivers/gpu/drm/i915/intel_display.c | 38 - drivers/gpu/drm/i915/intel_dp.c | 22 +-- 3 files changed, 19 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a0c15ab..aa24fc1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -79,6 +79,10 @@ enum port { #define for_each_pipe(p) for ((p) = 0; (p) dev_priv-num_pipe; (p)++) +#define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \ + list_for_each_entry((intel_encoder), (dev)-mode_config.encoder_list, base.head) \ + if ((intel_encoder)-base.crtc == (__crtc)) + struct intel_pch_pll { int refcount; /* count of number of CRTCs sharing this PLL */ int active; /* count of number of active CRTCs (i.e. DPMS on) */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3fbc802..72b73f8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -627,11 +627,10 @@ static void intel_clock(struct drm_device *dev, int refclk, intel_clock_t *clock bool intel_pipe_has_type(struct drm_crtc *crtc, int type) { struct drm_device *dev = crtc-dev; - struct drm_mode_config *mode_config = dev-mode_config; struct intel_encoder *encoder; - list_for_each_entry(encoder, mode_config-encoder_list, base.head) - if (encoder-base.crtc == crtc encoder-type == type) + for_each_encoder_on_crtc(dev, crtc, encoder) + if (encoder-type == type) return true; return false; @@ -2805,16 +2804,13 @@ static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) static bool intel_crtc_driving_pch(struct drm_crtc *crtc) { struct drm_device *dev = crtc-dev; - struct drm_mode_config *mode_config = dev-mode_config; struct intel_encoder *encoder; /* * If there's a non-PCH eDP on this crtc, it must be DP_A, and that * must be driven by its own crtc; no sharing is possible. */ - list_for_each_entry(encoder, mode_config-encoder_list, base.head) { - if (encoder-base.crtc != crtc) - continue; + for_each_encoder_on_crtc(dev, crtc, encoder) { /* On Haswell, LPT PCH handles the VGA connection via FDI, and Haswell * CPU handles all others */ @@ -3703,16 +3699,12 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc, { struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; - struct drm_encoder *encoder; struct drm_connector *connector; + struct intel_encoder *intel_encoder; unsigned int display_bpc = UINT_MAX, bpc; /* Walk the encoders connectors on this crtc, get min bpc */ - list_for_each_entry(encoder, dev-mode_config.encoder_list, head) { - struct intel_encoder *intel_encoder = to_intel_encoder(encoder); - - if (encoder-crtc != crtc) - continue; + for_each_encoder_on_crtc(dev, crtc, intel_encoder) { if (intel_encoder-type == INTEL_OUTPUT_LVDS) { unsigned int lvds_bpc; @@ -3744,7 +3736,7 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc, /* Not one of the known troublemakers, check the EDID */ list_for_each_entry(connector, dev-mode_config.connector_list, head) { - if (connector-encoder != encoder) + if (connector-encoder != intel_encoder-base) continue; /* Don't use an invalid EDID bpc value */ @@ -4213,15 +4205,11 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, u32 dspcntr, pipeconf, vsyncshift; bool ok, has_reduced_clock = false, is_sdvo = false; bool is_lvds = false, is_tv = false, is_dp = false; - struct drm_mode_config *mode_config = dev-mode_config; struct intel_encoder *encoder; const intel_limit_t *limit; int ret; - list_for_each_entry(encoder, mode_config-encoder_list, base.head) { - if (encoder-base.crtc != crtc) - continue; - + for_each_encoder_on_crtc(dev, crtc, encoder) { switch (encoder-type) { case INTEL_OUTPUT_LVDS: is_lvds = true; @@ -4524,15 +4512,11 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
[Intel-gfx] [PATCH 02/43] drm/i915: add crtc-enable/disable vfuncs insted of dpms
Because that's what we're essentially calling. This is the first step in untangling the crtc_helper induced dpms handling mess we have - at the crtc level we only have 2 states and the magic is just in selectin which one (and atm there isn't even much magic, but on recent platforms where not even the crt output has more than 2 states we could do better). Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.h |3 +- drivers/gpu/drm/i915/intel_display.c | 62 ++ 2 files changed, 20 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index aa24fc1..111d1e5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -238,7 +238,6 @@ struct drm_i915_error_state { }; struct drm_i915_display_funcs { - void (*dpms)(struct drm_crtc *crtc, int mode); bool (*fbc_enabled)(struct drm_device *dev); void (*enable_fbc)(struct drm_crtc *crtc, unsigned long interval); void (*disable_fbc)(struct drm_device *dev); @@ -255,6 +254,8 @@ struct drm_i915_display_funcs { struct drm_display_mode *adjusted_mode, int x, int y, struct drm_framebuffer *old_fb); + void (*crtc_enable)(struct drm_crtc *crtc); + void (*crtc_disable)(struct drm_crtc *crtc); void (*off)(struct drm_crtc *crtc); void (*write_eld)(struct drm_connector *connector, struct drm_crtc *crtc); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 72b73f8..072f94d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3299,30 +3299,6 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) mutex_unlock(dev-struct_mutex); } -static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode) -{ - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pipe = intel_crtc-pipe; - int plane = intel_crtc-plane; - - /* XXX: When our outputs are all unaware of DPMS modes other than off -* and on, we should map those modes to DRM_MODE_DPMS_OFF in the CRTC. -*/ - switch (mode) { - case DRM_MODE_DPMS_ON: - case DRM_MODE_DPMS_STANDBY: - case DRM_MODE_DPMS_SUSPEND: - DRM_DEBUG_KMS(crtc %d/%d dpms on\n, pipe, plane); - ironlake_crtc_enable(crtc); - break; - - case DRM_MODE_DPMS_OFF: - DRM_DEBUG_KMS(crtc %d/%d dpms off\n, pipe, plane); - ironlake_crtc_disable(crtc); - break; - } -} - static void ironlake_crtc_off(struct drm_crtc *crtc) { struct intel_crtc *intel_crtc = to_intel_crtc(crtc); @@ -3402,23 +3378,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) intel_update_watermarks(dev); } -static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode) -{ - /* XXX: When our outputs are all unaware of DPMS modes other than off -* and on, we should map those modes to DRM_MODE_DPMS_OFF in the CRTC. -*/ - switch (mode) { - case DRM_MODE_DPMS_ON: - case DRM_MODE_DPMS_STANDBY: - case DRM_MODE_DPMS_SUSPEND: - i9xx_crtc_enable(crtc); - break; - case DRM_MODE_DPMS_OFF: - i9xx_crtc_disable(crtc); - break; - } -} - static void i9xx_crtc_off(struct drm_crtc *crtc) { } @@ -3440,7 +3399,20 @@ static void intel_crtc_dpms(struct drm_crtc *crtc, int mode) intel_crtc-dpms_mode = mode; - dev_priv-display.dpms(crtc, mode); + /* XXX: When our outputs are all unaware of DPMS modes other than off +* and on, we should map those modes to DRM_MODE_DPMS_OFF in the CRTC. +*/ + switch (mode) { + case DRM_MODE_DPMS_ON: + case DRM_MODE_DPMS_STANDBY: + case DRM_MODE_DPMS_SUSPEND: + dev_priv-display.crtc_enable(crtc); + break; + + case DRM_MODE_DPMS_OFF: + dev_priv-display.crtc_disable(crtc); + break; + } if (!dev-primary-master) return; @@ -6940,13 +6912,15 @@ static void intel_init_display(struct drm_device *dev) /* We always want a DPMS function */ if (HAS_PCH_SPLIT(dev)) { - dev_priv-display.dpms = ironlake_crtc_dpms; dev_priv-display.crtc_mode_set = ironlake_crtc_mode_set; + dev_priv-display.crtc_enable = ironlake_crtc_enable; + dev_priv-display.crtc_disable = ironlake_crtc_disable; dev_priv-display.off = ironlake_crtc_off; dev_priv-display.update_plane = ironlake_update_plane; } else { - dev_priv-display.dpms = i9xx_crtc_dpms; dev_priv-display.crtc_mode_set = i9xx_crtc_mode_set; +
[Intel-gfx] [PATCH 03/43] drm/i915: rip out crtc prepare/commit indirection
Just impendance matching with the the crtc helper stuff. ... and somehow the design of this all ended up in this commit here, too ;-) The big plan is that a new set of display_funcs that take crtc take full responsibility of a modeset (and call down into object-specific callbacks and functions). The platform-specific callbacks simply know best what the proper order is. This has the drawback that we can't do minimal change-overs any more if a modeset just disables one encoder in a cloned configuration (because we will only expose a disabl/enable action that takes down/sets up the entire crtc including all encoders). Imo that's the only sane way to do it though: - The use-case for this is pretty minimal, even when presenting (at least sane people) should use a dual-screen output so that you can see your notes on your panel. Clone mode is imo BS. - With all the clone mode constrains, shared resources, and special ordering requirements (which differ even on the same platform sometimes for different outputs) there's no way we'd get this right for all cases. Especially since this is a under-used feature. - And to top it off: On haswell even dp link re-training requires us to take down the entire display pipe - otherwise the chip dies. So the only sane way is to do a full modeset on every crtc where the output config changes in any way. To support atomic modeset we'd then add one more function to allocate global and shared objects in the best ways (e.g. fdi links, pch plls, ...). The crtc functions would then simply use the pre-allocated stuff (and shouldn't be able to fail, ever). We could even do all the object pinning in there (and maybe try to defragment the global gtt if we fail)! Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 37 + 1 files changed, 2 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 072f94d..608046f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3457,34 +3457,6 @@ static void intel_crtc_disable(struct drm_crtc *crtc) } } -/* Prepare for a mode set. - * - * Note we could be a lot smarter here. We need to figure out which outputs - * will be enabled, which disabled (in short, how the config will changes) - * and perform the minimum necessary steps to accomplish that, e.g. updating - * watermarks, FBC configuration, making sure PLLs are programmed correctly, - * panel fitting is in the proper state, etc. - */ -static void i9xx_crtc_prepare(struct drm_crtc *crtc) -{ - i9xx_crtc_disable(crtc); -} - -static void i9xx_crtc_commit(struct drm_crtc *crtc) -{ - i9xx_crtc_enable(crtc); -} - -static void ironlake_crtc_prepare(struct drm_crtc *crtc) -{ - ironlake_crtc_disable(crtc); -} - -static void ironlake_crtc_commit(struct drm_crtc *crtc) -{ - ironlake_crtc_enable(crtc); -} - void intel_encoder_prepare(struct drm_encoder *encoder) { struct drm_encoder_helper_funcs *encoder_funcs = encoder-helper_private; @@ -6598,13 +6570,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) intel_crtc-active = true; /* force the pipe off on setup_init_config */ intel_crtc-bpp = 24; /* default for pre-Ironlake */ - if (HAS_PCH_SPLIT(dev)) { - intel_helper_funcs.prepare = ironlake_crtc_prepare; - intel_helper_funcs.commit = ironlake_crtc_commit; - } else { - intel_helper_funcs.prepare = i9xx_crtc_prepare; - intel_helper_funcs.commit = i9xx_crtc_commit; - } + intel_helper_funcs.prepare = dev_priv-display.crtc_disable; + intel_helper_funcs.commit = dev_priv-display.crtc_enable; drm_crtc_helper_add(intel_crtc-base, intel_helper_funcs); -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/43] drm/i915: add direct encoder disable/enable infrastructure
Just prep work, not yet put to some use. Note that because we're still using the crtc helper to switch modes (and their complicated way to do partial modesets), we need to call the encoder's disable function unconditionally. But once this is cleaned up we should call the encoder's disable function unconditionally, because then we know that we'll only call it if the encoder is actually enabled. Also note that we then need to be careful about which crtc we're filtering the encoder list on: We want to filter on the crtc of the _current_ mode, not the one we're about to set up. For the enabling side we need to do the same trick. And again, we should be able to simplify this quite a bit when things have settled into place. Also note that this simply does not take cloning into account, so dpms needs to be handled specially for the few outputs where we even bother with it. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 38 - drivers/gpu/drm/i915/intel_drv.h |2 + 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 608046f..58fda88 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3147,13 +3147,16 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_encoder *encoder; int pipe = intel_crtc-pipe; int plane = intel_crtc-plane; u32 temp; bool is_pch_port; + /* XXX: For compatability with the crtc helper code, call the encoder's +* enable function unconditionally for now. */ if (intel_crtc-active) - return; + goto encoders; intel_crtc-active = true; intel_update_watermarks(dev); @@ -3200,6 +3203,12 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) mutex_unlock(dev-struct_mutex); intel_crtc_update_cursor(crtc, true); + +encoders: + for_each_encoder_on_crtc(dev, crtc, encoder) { + if (encoder-enable) + encoder-enable(encoder); + } } static void ironlake_crtc_disable(struct drm_crtc *crtc) @@ -3207,10 +3216,18 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_encoder *encoder; int pipe = intel_crtc-pipe; int plane = intel_crtc-plane; u32 reg, temp; + /* XXX: For compatability with the crtc helper code, call the encoder's +* disable function unconditionally for now. */ + for_each_encoder_on_crtc(dev, crtc, encoder) { + if (encoder-disable) + encoder-disable(encoder); + } + if (!intel_crtc-active) return; @@ -3328,11 +3345,14 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_encoder *encoder; int pipe = intel_crtc-pipe; int plane = intel_crtc-plane; + /* XXX: For compatability with the crtc helper code, call the encoder's +* enable function unconditionally for now. */ if (intel_crtc-active) - return; + goto encoders; intel_crtc-active = true; intel_update_watermarks(dev); @@ -3347,6 +3367,12 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) /* Give the overlay scaler a chance to enable if it's on this pipe */ intel_crtc_dpms_overlay(intel_crtc, true); intel_crtc_update_cursor(crtc, true); + +encoders: + for_each_encoder_on_crtc(dev, crtc, encoder) { + if (encoder-enable) + encoder-enable(encoder); + } } static void i9xx_crtc_disable(struct drm_crtc *crtc) @@ -3354,9 +3380,17 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_encoder *encoder; int pipe = intel_crtc-pipe; int plane = intel_crtc-plane; + /* XXX: For compatability with the crtc helper code, call the encoder's +* disable function unconditionally for now. */ + for_each_encoder_on_crtc(dev, crtc, encoder) { + if (encoder-disable) + encoder-disable(encoder); + } + if (!intel_crtc-active) return; diff --git
[Intel-gfx] [PATCH 05/43] drm/i915: add missing gen2 pipe A quirk entries
For some odd reason we've missed i830 and a i855 variant. Also kill the two now redundant i830 entries. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c |8 ++-- 1 files changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 58fda88..52b70ea 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7046,21 +7046,17 @@ static struct intel_quirk intel_quirks[] = { /* HP Mini needs pipe A force quirk (LP: #322104) */ { 0x27ae, 0x103c, 0x361a, quirk_pipea_force }, - /* Thinkpad R31 needs pipe A force quirk */ - { 0x3577, 0x1014, 0x0505, quirk_pipea_force }, /* Toshiba Protege R-205, S-209 needs pipe A force quirk */ { 0x2592, 0x1179, 0x0001, quirk_pipea_force }, - /* ThinkPad X30 needs pipe A force quirk (LP: #304614) */ - { 0x3577, 0x1014, 0x0513, quirk_pipea_force }, - /* ThinkPad X40 needs pipe A force quirk */ - /* ThinkPad T60 needs pipe A force quirk (bug #16494) */ { 0x2782, 0x17aa, 0x201a, quirk_pipea_force }, /* 855 before need to leave pipe A dpll A up */ { 0x3582, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force }, { 0x2562, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force }, + { 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force }, + { 0x358e, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force }, /* Lenovo U160 cannot use SSC on LVDS */ { 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable }, -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/43] drm/i915: rip out the overlay pipe A workaround
Now that all affected i830M systems have the pipe A quirk set, we don't need to do any special dances in the overlay code any longer. And reading through the code I'm rather dubios that it actually does what it claims to do ... As a nice benefit this rips out a users of the crtc helper dpms callback. v2: As suggested by Chris Wilson, replace the code by an appropriate WARN to ensure that the pipe A is indeed running. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_overlay.c | 58 +- 1 files changed, 1 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c index 830d0dd..c0f4858 100644 --- a/drivers/gpu/drm/i915/intel_overlay.c +++ b/drivers/gpu/drm/i915/intel_overlay.c @@ -235,54 +235,6 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay, return 0; } -/* Workaround for i830 bug where pipe a must be enable to change control regs */ -static int -i830_activate_pipe_a(struct drm_device *dev) -{ - drm_i915_private_t *dev_priv = dev-dev_private; - struct intel_crtc *crtc; - struct drm_crtc_helper_funcs *crtc_funcs; - struct drm_display_mode vesa_640x480 = { - DRM_MODE(640x480, DRM_MODE_TYPE_DRIVER, 25175, 640, 656, -752, 800, 0, 480, 489, 492, 525, 0, -DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) - }, *mode; - - crtc = to_intel_crtc(dev_priv-pipe_to_crtc_mapping[0]); - if (crtc-dpms_mode == DRM_MODE_DPMS_ON) - return 0; - - /* most i8xx have pipe a forced on, so don't trust dpms mode */ - if (I915_READ(_PIPEACONF) PIPECONF_ENABLE) - return 0; - - crtc_funcs = crtc-base.helper_private; - if (crtc_funcs-dpms == NULL) - return 0; - - DRM_DEBUG_DRIVER(Enabling pipe A in order to enable overlay\n); - - mode = drm_mode_duplicate(dev, vesa_640x480); - - if (!drm_crtc_helper_set_mode(crtc-base, mode, - crtc-base.x, crtc-base.y, - crtc-base.fb)) - return 0; - - crtc_funcs-dpms(crtc-base, DRM_MODE_DPMS_ON); - return 1; -} - -static void -i830_deactivate_pipe_a(struct drm_device *dev) -{ - drm_i915_private_t *dev_priv = dev-dev_private; - struct drm_crtc *crtc = dev_priv-pipe_to_crtc_mapping[0]; - struct drm_crtc_helper_funcs *crtc_funcs = crtc-helper_private; - - crtc_funcs-dpms(crtc, DRM_MODE_DPMS_OFF); -} - /* overlay needs to be disable in OCMD reg */ static int intel_overlay_on(struct intel_overlay *overlay) { @@ -290,17 +242,12 @@ static int intel_overlay_on(struct intel_overlay *overlay) struct drm_i915_private *dev_priv = dev-dev_private; struct intel_ring_buffer *ring = dev_priv-ring[RCS]; struct drm_i915_gem_request *request; - int pipe_a_quirk = 0; int ret; BUG_ON(overlay-active); overlay-active = 1; - if (IS_I830(dev)) { - pipe_a_quirk = i830_activate_pipe_a(dev); - if (pipe_a_quirk 0) - return pipe_a_quirk; - } + WARN_ON(IS_I830(dev) !(dev_priv-quirks QUIRK_PIPEA_FORCE)); request = kzalloc(sizeof(*request), GFP_KERNEL); if (request == NULL) { @@ -322,9 +269,6 @@ static int intel_overlay_on(struct intel_overlay *overlay) ret = intel_overlay_do_wait_request(overlay, request, NULL); out: - if (pipe_a_quirk) - i830_deactivate_pipe_a(dev); - return ret; } -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/43] drm/i915: prepare load-detect pipe code for dpms changes
A few things need adjustement: - Change the dpms state by calling the dpms connector function and not some crtc helper internal callbacks. Otherwise this will break once we switch to our own dpms handling. - Instead of tracking and restoring intel_crtc-dpms_mode use the connector's dpms variable - the former relies on the dpms compuation rules used by the crtc helper. And it would break when the encoder is cloned and the other output has a different dpms state. But luckily no one is crazy enough for that. - Properly clear the connector - encoder - crtc linking, even when failing (note that the crtc helper removes the encoder - crtc link in disabled_unused_functions for us). Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 38 - 1 files changed, 14 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 52b70ea..562ad86 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5570,21 +5570,12 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, if (encoder-crtc) { crtc = encoder-crtc; - intel_crtc = to_intel_crtc(crtc); - old-dpms_mode = intel_crtc-dpms_mode; + old-dpms_mode = connector-dpms; old-load_detect_temp = false; /* Make sure the crtc and connector are running */ - if (intel_crtc-dpms_mode != DRM_MODE_DPMS_ON) { - struct drm_encoder_helper_funcs *encoder_funcs; - struct drm_crtc_helper_funcs *crtc_funcs; - - crtc_funcs = crtc-helper_private; - crtc_funcs-dpms(crtc, DRM_MODE_DPMS_ON); - - encoder_funcs = encoder-helper_private; - encoder_funcs-dpms(encoder, DRM_MODE_DPMS_ON); - } + if (connector-dpms != DRM_MODE_DPMS_ON) + connector-funcs-dpms(connector, DRM_MODE_DPMS_ON); return true; } @@ -5612,7 +5603,7 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, connector-encoder = encoder; intel_crtc = to_intel_crtc(crtc); - old-dpms_mode = intel_crtc-dpms_mode; + old-dpms_mode = connector-dpms; old-load_detect_temp = true; old-release_fb = NULL; @@ -5637,22 +5628,25 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, DRM_DEBUG_KMS(reusing fbdev for load-detection framebuffer\n); if (IS_ERR(crtc-fb)) { DRM_DEBUG_KMS(failed to allocate framebuffer for load-detection\n); - crtc-fb = old_fb; - return false; + goto fail; } if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, old_fb)) { DRM_DEBUG_KMS(failed to set mode on load-detect pipe\n); if (old-release_fb) old-release_fb-funcs-destroy(old-release_fb); - crtc-fb = old_fb; - return false; + goto fail; } /* let the connector get through one full cycle before testing */ intel_wait_for_vblank(dev, intel_crtc-pipe); return true; +fail: + connector-encoder = NULL; + encoder-crtc = NULL; + crtc-fb = old_fb; + return false; } void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder, @@ -5661,9 +5655,6 @@ void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder, { struct drm_encoder *encoder = intel_encoder-base; struct drm_device *dev = encoder-dev; - struct drm_crtc *crtc = encoder-crtc; - struct drm_encoder_helper_funcs *encoder_funcs = encoder-helper_private; - struct drm_crtc_helper_funcs *crtc_funcs = crtc-helper_private; DRM_DEBUG_KMS([CONNECTOR:%d:%s], [ENCODER:%d:%s]\n, connector-base.id, drm_get_connector_name(connector), @@ -5671,6 +5662,7 @@ void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder, if (old-load_detect_temp) { connector-encoder = NULL; + encoder-crtc = NULL; drm_helper_disable_unused_functions(dev); if (old-release_fb) @@ -5680,10 +5672,8 @@ void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder, } /* Switch crtc and encoder back off if necessary */ - if (old-dpms_mode != DRM_MODE_DPMS_ON) { - encoder_funcs-dpms(encoder, old-dpms_mode); - crtc_funcs-dpms(crtc, old-dpms_mode); - } + if (old-dpms_mode != DRM_MODE_DPMS_ON) + connector-funcs-dpms(connector, old-dpms_mode); } /* Returns the clock of the currently programmed mode of the given pipe. */ -- 1.7.7.6
[Intel-gfx] [PATCH 08/43] drm/i915/hdmi: convert to encoder-disable/enable
I've picked hdmi as the first encoder to convert because it's rather simple: - no cloning possible - no differences between prepare/commit and dpms off/on switching. A few changes are required to do so: - Split up the dpms code into an enable/disable function and wire it up with the intel encoder. - Noop out the existing encoder prepare/commit functions used by the crtc helper - our crtc enable/disable code now calls back into the encoder enable/disable code at the right spot. - Create new helper functions to handle dpms changes. - Add intel_encoder-connectors_active to better track dpms state. Atm this is unused, but it will be useful to correctly disable the entire display pipe for cloned configurations. Also note that for now this is only useful in the dpms code - thanks to the crtc helper's dpms confusion across a modeset operation we can't (yet) rely on this having a sensible value in all circumstances. - Rip out the encoder helper dpms callback, if this is still getting called somewhere we have a bug. The slight issue with that is that the crtc helper abuses dpms off to disable unused functions. Hence we also need to implement a default encoder disable function to do just that with the new encoder-disable callback. - Note that we drop the cpt modeset verification in the commit callback, too. The right place to do this would be in the crtc's enable function, _after_ all the encoders are set up. But because not all encoders are converted yet, we can't do that. Hence disable this check temporarily as a minor concession to bisectability. v2: Squash the dpms mode to only the supported values - connector-dpms is for internal tracking only, we can hence avoid needless state-changes a bit whithout causing harm. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_ddi.c | 28 +--- drivers/gpu/drm/i915/intel_display.c | 49 + drivers/gpu/drm/i915/intel_drv.h |8 ++- drivers/gpu/drm/i915/intel_hdmi.c| 126 +++--- 4 files changed, 160 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index f33fe1a..b71303c 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -729,21 +729,16 @@ void intel_ddi_mode_set(struct drm_encoder *encoder, intel_hdmi-set_infoframes(encoder, adjusted_mode); } -void intel_ddi_dpms(struct drm_encoder *encoder, int mode) +void intel_enable_ddi(struct intel_encoder *encoder) { - struct drm_device *dev = encoder-dev; + struct drm_device *dev = encoder-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder-base); int port = intel_hdmi-ddi_port; u32 temp; temp = I915_READ(DDI_BUF_CTL(port)); - - if (mode != DRM_MODE_DPMS_ON) { - temp = ~DDI_BUF_CTL_ENABLE; - } else { - temp |= DDI_BUF_CTL_ENABLE; - } + temp |= DDI_BUF_CTL_ENABLE; /* Enable DDI_BUF_CTL. In HDMI/DVI mode, the port width, * and swing/emphasis values are ignored so nothing special needs @@ -752,3 +747,18 @@ void intel_ddi_dpms(struct drm_encoder *encoder, int mode) I915_WRITE(DDI_BUF_CTL(port), temp); } + +void intel_disable_ddi(struct intel_encoder *encoder) +{ + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder-base); + int port = intel_hdmi-ddi_port; + u32 temp; + + temp = I915_READ(DDI_BUF_CTL(port)); + temp = ~DDI_BUF_CTL_ENABLE; + + I915_WRITE(DDI_BUF_CTL(port), + temp); +} diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 562ad86..62acf98 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3511,6 +3511,17 @@ void intel_encoder_commit(struct drm_encoder *encoder) intel_cpt_verify_modeset(dev, intel_crtc-pipe); } +void intel_encoder_noop(struct drm_encoder *encoder) +{ +} + +void intel_encoder_disable(struct drm_encoder *encoder) +{ + struct intel_encoder *intel_encoder = to_intel_encoder(encoder); + + intel_encoder-disable(intel_encoder); +} + void intel_encoder_destroy(struct drm_encoder *encoder) { struct intel_encoder *intel_encoder = to_intel_encoder(encoder); @@ -3519,6 +3530,44 @@ void intel_encoder_destroy(struct drm_encoder *encoder) kfree(intel_encoder); } +/* Simple dpms helper for encodres with just one connector, no cloning and only + * one kind of off state. It clamps all !ON modes to fully OFF and changes the + * state of the entire output pipe. */ +void
[Intel-gfx] [PATCH 09/43] drm/i915/tv: convert to encoder enable/disable
Like hdmi tv outputs are simple: They only have 2 states and can't be cloned. Hence give it the same treatment. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_tv.c | 35 +++ 1 files changed, 19 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index a233a51..e2169d2 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -837,21 +837,21 @@ static struct intel_tv *intel_attached_tv(struct drm_connector *connector) } static void -intel_tv_dpms(struct drm_encoder *encoder, int mode) +intel_enable_tv(struct intel_encoder *encoder) { - struct drm_device *dev = encoder-dev; + struct drm_device *dev = encoder-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; - switch (mode) { - case DRM_MODE_DPMS_ON: - I915_WRITE(TV_CTL, I915_READ(TV_CTL) | TV_ENC_ENABLE); - break; - case DRM_MODE_DPMS_STANDBY: - case DRM_MODE_DPMS_SUSPEND: - case DRM_MODE_DPMS_OFF: - I915_WRITE(TV_CTL, I915_READ(TV_CTL) ~TV_ENC_ENABLE); - break; - } + I915_WRITE(TV_CTL, I915_READ(TV_CTL) | TV_ENC_ENABLE); +} + +static void +intel_disable_tv(struct intel_encoder *encoder) +{ + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + + I915_WRITE(TV_CTL, I915_READ(TV_CTL) ~TV_ENC_ENABLE); } static const struct tv_mode * @@ -1484,15 +1484,15 @@ out: } static const struct drm_encoder_helper_funcs intel_tv_helper_funcs = { - .dpms = intel_tv_dpms, .mode_fixup = intel_tv_mode_fixup, - .prepare = intel_encoder_prepare, + .prepare = intel_encoder_noop, .mode_set = intel_tv_mode_set, - .commit = intel_encoder_commit, + .commit = intel_encoder_noop, + .disable = intel_encoder_disable, }; static const struct drm_connector_funcs intel_tv_connector_funcs = { - .dpms = drm_helper_connector_dpms, + .dpms = intel_connector_dpms, .detect = intel_tv_detect, .destroy = intel_tv_destroy, .set_property = intel_tv_set_property, @@ -1622,6 +1622,9 @@ intel_tv_init(struct drm_device *dev) drm_encoder_init(dev, intel_encoder-base, intel_tv_enc_funcs, DRM_MODE_ENCODER_TVDAC); + intel_encoder-enable = intel_enable_tv; + intel_encoder-disable = intel_disable_tv; + intel_connector_attach_encoder(intel_connector, intel_encoder); intel_encoder-type = INTEL_OUTPUT_TVOUT; intel_encoder-crtc_mask = (1 0) | (1 1); -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/43] drm/i915/lvds: convert to encoder disable/enable
With the previous patch LVDS is also a simple case. Treat it accordingly. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_lvds.c | 52 ++--- 1 files changed, 14 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 4edbba4..9ae7fc9 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -68,10 +68,11 @@ static struct intel_lvds *intel_attached_lvds(struct drm_connector *connector) /** * Sets the power state for the panel. */ -static void intel_lvds_enable(struct intel_lvds *intel_lvds) +static void intel_enable_lvds(struct intel_encoder *encoder) { - struct drm_device *dev = intel_lvds-base.base.dev; - struct intel_crtc *intel_crtc = to_intel_crtc(intel_lvds-base.base.crtc); + struct drm_device *dev = encoder-base.dev; + struct intel_lvds *intel_lvds = to_intel_lvds(encoder-base); + struct intel_crtc *intel_crtc = to_intel_crtc(encoder-base.crtc); struct drm_i915_private *dev_priv = dev-dev_private; u32 ctl_reg, lvds_reg, stat_reg; @@ -111,9 +112,10 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds) intel_panel_enable_backlight(dev, intel_crtc-pipe); } -static void intel_lvds_disable(struct intel_lvds *intel_lvds) +static void intel_disable_lvds(struct intel_encoder *encoder) { - struct drm_device *dev = intel_lvds-base.base.dev; + struct drm_device *dev = encoder-base.dev; + struct intel_lvds *intel_lvds = to_intel_lvds(encoder-base); struct drm_i915_private *dev_priv = dev-dev_private; u32 ctl_reg, lvds_reg, stat_reg; @@ -142,18 +144,6 @@ static void intel_lvds_disable(struct intel_lvds *intel_lvds) POSTING_READ(lvds_reg); } -static void intel_lvds_dpms(struct drm_encoder *encoder, int mode) -{ - struct intel_lvds *intel_lvds = to_intel_lvds(encoder); - - if (mode == DRM_MODE_DPMS_ON) - intel_lvds_enable(intel_lvds); - else - intel_lvds_disable(intel_lvds); - - /* XXX: We never power down the LVDS pairs. */ -} - static int intel_lvds_mode_valid(struct drm_connector *connector, struct drm_display_mode *mode) { @@ -405,23 +395,6 @@ out: return true; } -static void intel_lvds_prepare(struct drm_encoder *encoder) -{ - struct intel_lvds *intel_lvds = to_intel_lvds(encoder); - - intel_lvds_disable(intel_lvds); -} - -static void intel_lvds_commit(struct drm_encoder *encoder) -{ - struct intel_lvds *intel_lvds = to_intel_lvds(encoder); - - /* Always do a full power on as we do not know what state -* we were left in. -*/ - intel_lvds_enable(intel_lvds); -} - static void intel_lvds_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) @@ -596,11 +569,11 @@ static int intel_lvds_set_property(struct drm_connector *connector, } static const struct drm_encoder_helper_funcs intel_lvds_helper_funcs = { - .dpms = intel_lvds_dpms, .mode_fixup = intel_lvds_mode_fixup, - .prepare = intel_lvds_prepare, + .prepare = intel_encoder_noop, .mode_set = intel_lvds_mode_set, - .commit = intel_lvds_commit, + .commit = intel_encoder_noop, + .disable = intel_encoder_disable, }; static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs = { @@ -610,7 +583,7 @@ static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs }; static const struct drm_connector_funcs intel_lvds_connector_funcs = { - .dpms = drm_helper_connector_dpms, + .dpms = intel_connector_dpms, .detect = intel_lvds_detect, .fill_modes = drm_helper_probe_single_connector_modes, .set_property = intel_lvds_set_property, @@ -964,6 +937,9 @@ bool intel_lvds_init(struct drm_device *dev) drm_encoder_init(dev, intel_encoder-base, intel_lvds_enc_funcs, DRM_MODE_ENCODER_LVDS); + intel_encoder-enable = intel_enable_lvds; + intel_encoder-disable = intel_disable_lvds; + intel_connector_attach_encoder(intel_connector, intel_encoder); intel_encoder-type = INTEL_OUTPUT_LVDS; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/43] drm/i915/lvds: ditch -prepare special case
LVDS is the first output where dpms on/off and prepare/commit don't perfectly match. Now the idea behind this special case seems to be that for simple resolution changes on the LVDS we don't need to stop the pipe, because (at least on newer chips) we can adjust the panel fitter on the fly. There are a few problems with the current code though: - We still stop and restart the pipe unconditionally, because the crtc helper code isn't flexible enough. - We show some ugly flickering, especially when changing crtcs (this the crtc helper would actually take into account, but we don't implement the encoder-get_crtc callback required to make this work properly). So it doesn't even work as advertised. I agree that it would be nice to do resolution changes on LVDS (and also eDP) whithout blacking the screen where the panel fitter allows to do that. But imo we should implement this as a special case a few layers up in the mode set code, akin to how we already detect simple framebuffer changes (and only update that with -mode_set_base). Until this is all in place, make our lives easier and just rip it out. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_lvds.c |8 +--- 1 files changed, 1 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 05fcadb..4edbba4 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -409,13 +409,7 @@ static void intel_lvds_prepare(struct drm_encoder *encoder) { struct intel_lvds *intel_lvds = to_intel_lvds(encoder); - /* -* Prior to Ironlake, we must disable the pipe if we want to adjust -* the panel fitter. However at all other times we can just reset -* the registers regardless. -*/ - if (!HAS_PCH_SPLIT(encoder-dev) intel_lvds-pfit_dirty) - intel_lvds_disable(intel_lvds); + intel_lvds_disable(intel_lvds); } static void intel_lvds_commit(struct drm_encoder *encoder) -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/43] drm/i915/dp: convert to encoder disable/enable
DP is the first encoder which isn't simple. As commit d240f20f545fa4ed78ce48d1eb62ab529f2b1467 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Fri Aug 13 15:43:26 2010 -0700 drm/i915: make sure eDP PLL is enabled at the right time discovered, we need to enable the eDP PLL for the cpu port _before_ we enable the pipes and planes. After a few more commits the current solution is to enable the PLL in the dp mode_set function (because this is the only encoder callback the crtc helper code calls before it calls the crtc's commit function). Now I suspect that we actually should enable/disable the entire cpu eDP port before/after planes, but thanks to how the crtc helper code assumes that you can disable an encoder without disabling it's crtc right away, this won't work. The result is that the current prepare/commit hooks don't touch the eDP PLL, but instead it get's frobbed in dp_mode_set and in the dp dpms function. Hence we need to keep things (at least for now) bug-for-bug compatible by using our own special dp dpms function and keep everything else more-or-less as-is (just using our own infrastrucutre now). This mess can only be cleaned once we control the entire modeset sequence and can move around things freely. v2: Squash unsupported dpms modes to OFF at the beginning of the DP dpms function. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_dp.c | 88 +++--- 1 files changed, 44 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 096947d..b8abbff 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1277,10 +1277,9 @@ static void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) } } -static void intel_dp_prepare(struct drm_encoder *encoder) +static void intel_disable_dp(struct intel_encoder *encoder) { - struct intel_dp *intel_dp = enc_to_intel_dp(encoder); - + struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); /* Make sure the panel is off before trying to change the mode. But also * ensure that we have vdd while we switch off the panel. */ @@ -1293,62 +1292,60 @@ static void intel_dp_prepare(struct drm_encoder *encoder) ironlake_edp_panel_vdd_off(intel_dp, false); } -static void intel_dp_commit(struct drm_encoder *encoder) +static void intel_enable_dp(struct intel_encoder *encoder) { - struct intel_dp *intel_dp = enc_to_intel_dp(encoder); - struct drm_device *dev = encoder-dev; - struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp-base.base.crtc); + struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + uint32_t dp_reg = I915_READ(intel_dp-output_reg); ironlake_edp_panel_vdd_on(intel_dp); intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); - intel_dp_start_link_train(intel_dp); - ironlake_edp_panel_on(intel_dp); - ironlake_edp_panel_vdd_off(intel_dp, true); - intel_dp_complete_link_train(intel_dp); + if (!(dp_reg DP_PORT_EN)) { + intel_dp_start_link_train(intel_dp); + ironlake_edp_panel_on(intel_dp); + ironlake_edp_panel_vdd_off(intel_dp, true); + intel_dp_complete_link_train(intel_dp); + } else + ironlake_edp_panel_vdd_off(intel_dp, false); ironlake_edp_backlight_on(intel_dp); intel_dp-dpms_mode = DRM_MODE_DPMS_ON; - - if (HAS_PCH_CPT(dev)) - intel_cpt_verify_modeset(dev, intel_crtc-pipe); } static void -intel_dp_dpms(struct drm_encoder *encoder, int mode) +intel_dp_dpms(struct drm_connector *connector, int mode) { - struct intel_dp *intel_dp = enc_to_intel_dp(encoder); - struct drm_device *dev = encoder-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - uint32_t dp_reg = I915_READ(intel_dp-output_reg); + struct intel_dp *intel_dp = intel_attached_dp(connector); - if (mode != DRM_MODE_DPMS_ON) { - /* Switching the panel off requires vdd. */ - ironlake_edp_panel_vdd_on(intel_dp); - ironlake_edp_backlight_off(intel_dp); - ironlake_edp_panel_off(intel_dp); + /* DP supports only 2 dpms states. */ + if (mode != DRM_MODE_DPMS_ON) + mode = DRM_MODE_DPMS_OFF; - intel_dp_sink_dpms(intel_dp, mode); - intel_dp_link_down(intel_dp); - ironlake_edp_panel_vdd_off(intel_dp, false); + if (mode == connector-dpms) + return; + + connector-dpms = mode; + + /* Only need to change hw state when actually enabled */ + if (!intel_dp-base.base.crtc) { + intel_dp-base.connectors_active = false; + return; + } + + if (mode
[Intel-gfx] [PATCH 13/43] drm/i915: create VLV_DSIPLAY_BASE #define
Will be used more in the next patch. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.c |2 +- drivers/gpu/drm/i915/i915_reg.h |2 ++ 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 79be879..57381eb 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1169,7 +1169,7 @@ static bool IS_DISPLAYREG(u32 reg) * This should make it easier to transition modules over to the * new register block scheme, since we can do it incrementally. */ - if (reg = 0x18) + if (reg = VLV_DISPLAY_BASE) return false; if (reg = RENDER_RING_BASE diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ac65e96..524c70e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -528,6 +528,8 @@ #define GFX_PSMI_GRANULARITY (110) #define GFX_PPGTT_ENABLE (19) +#define VLV_DISPLAY_BASE 0x18 + #define SCPD0 0x0209c /* 915+ only */ #define IER0x020a0 #define IIR0x020a4 -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/43] drm/i915: group ADPA #defines together
Splitting them up between pch and gmch variants just makes it harder to find things. Especially since the hotplug bits are actually valid on earlier chips, too. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_reg.h | 47 ++ 1 files changed, 22 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 524c70e..052d470 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1542,12 +1542,34 @@ /* VGA port control */ #define ADPA 0x61100 +#define PCH_ADPA0xe1100 + #define ADPA_DAC_ENABLE (131) #define ADPA_DAC_DISABLE 0 #define ADPA_PIPE_SELECT_MASK(130) #define ADPA_PIPE_A_SELECT 0 #define ADPA_PIPE_B_SELECT (130) #define ADPA_PIPE_SELECT(pipe) ((pipe) 30) +/* CPT uses bits 29:31 for pch transcoder select */ +#define ADPA_CRT_HOTPLUG_MASK 0x03ff /* bit 25-16 */ +#define ADPA_CRT_HOTPLUG_MONITOR_NONE (024) +#define ADPA_CRT_HOTPLUG_MONITOR_MASK (324) +#define ADPA_CRT_HOTPLUG_MONITOR_COLOR (324) +#define ADPA_CRT_HOTPLUG_MONITOR_MONO (224) +#define ADPA_CRT_HOTPLUG_ENABLE(123) +#define ADPA_CRT_HOTPLUG_PERIOD_64 (022) +#define ADPA_CRT_HOTPLUG_PERIOD_128(122) +#define ADPA_CRT_HOTPLUG_WARMUP_5MS(021) +#define ADPA_CRT_HOTPLUG_WARMUP_10MS (121) +#define ADPA_CRT_HOTPLUG_SAMPLE_2S (020) +#define ADPA_CRT_HOTPLUG_SAMPLE_4S (120) +#define ADPA_CRT_HOTPLUG_VOLTAGE_40(018) +#define ADPA_CRT_HOTPLUG_VOLTAGE_50(118) +#define ADPA_CRT_HOTPLUG_VOLTAGE_60(218) +#define ADPA_CRT_HOTPLUG_VOLTAGE_70(318) +#define ADPA_CRT_HOTPLUG_VOLREF_325MV (017) +#define ADPA_CRT_HOTPLUG_VOLREF_475MV (117) +#define ADPA_CRT_HOTPLUG_FORCE_TRIGGER (116) #define ADPA_USE_VGA_HVPOLARITY (115) #define ADPA_SETS_HVPOLARITY 0 #define ADPA_VSYNC_CNTL_DISABLE (111) @@ -3878,31 +3900,6 @@ #define FDI_PLL_CTL_1 0xfe000 #define FDI_PLL_CTL_2 0xfe004 -/* CRT */ -#define PCH_ADPA0xe1100 -#define ADPA_TRANS_SELECT_MASK (130) -#define ADPA_TRANS_A_SELECT0 -#define ADPA_TRANS_B_SELECT(130) -#define ADPA_CRT_HOTPLUG_MASK 0x03ff /* bit 25-16 */ -#define ADPA_CRT_HOTPLUG_MONITOR_NONE (024) -#define ADPA_CRT_HOTPLUG_MONITOR_MASK (324) -#define ADPA_CRT_HOTPLUG_MONITOR_COLOR (324) -#define ADPA_CRT_HOTPLUG_MONITOR_MONO (224) -#define ADPA_CRT_HOTPLUG_ENABLE(123) -#define ADPA_CRT_HOTPLUG_PERIOD_64 (022) -#define ADPA_CRT_HOTPLUG_PERIOD_128(122) -#define ADPA_CRT_HOTPLUG_WARMUP_5MS(021) -#define ADPA_CRT_HOTPLUG_WARMUP_10MS (121) -#define ADPA_CRT_HOTPLUG_SAMPLE_2S (020) -#define ADPA_CRT_HOTPLUG_SAMPLE_4S (120) -#define ADPA_CRT_HOTPLUG_VOLTAGE_40(018) -#define ADPA_CRT_HOTPLUG_VOLTAGE_50(118) -#define ADPA_CRT_HOTPLUG_VOLTAGE_60(218) -#define ADPA_CRT_HOTPLUG_VOLTAGE_70(318) -#define ADPA_CRT_HOTPLUG_VOLREF_325MV (017) -#define ADPA_CRT_HOTPLUG_VOLREF_475MV (117) -#define ADPA_CRT_HOTPLUG_FORCE_TRIGGER (116) - /* or SDVOB */ #define HDMIB 0xe1140 #define PORT_ENABLE(1 31) -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/43] drm/i915: add inte_crt-adpa_reg
With the base addresses shifting around, this is easier to handle. Also move to the real reg offset on vlv. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_crt.c | 23 --- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 052d470..3b9c65e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1543,6 +1543,7 @@ /* VGA port control */ #define ADPA 0x61100 #define PCH_ADPA0xe1100 +#define VLV_ADPA (VLV_DISPLAY_BASE + ADPA) #define ADPA_DAC_ENABLE (131) #define ADPA_DAC_DISABLE 0 diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 61d55d3..8babc10 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -47,6 +47,7 @@ struct intel_crt { struct intel_encoder base; bool force_hotplug_required; + u32 adpa_reg; }; static struct intel_crt *intel_attached_crt(struct drm_connector *connector) @@ -55,6 +56,11 @@ static struct intel_crt *intel_attached_crt(struct drm_connector *connector) struct intel_crt, base); } +static struct intel_crt *intel_encoder_to_crt(struct intel_encoder *encoder) +{ + return container_of(encoder, struct intel_crt, base); +} + static void pch_crt_dpms(struct drm_encoder *encoder, int mode) { struct drm_device *dev = encoder-dev; @@ -145,19 +151,15 @@ static void intel_crt_mode_set(struct drm_encoder *encoder, struct drm_device *dev = encoder-dev; struct drm_crtc *crtc = encoder-crtc; + struct intel_crt *crt = + intel_encoder_to_crt(to_intel_encoder(encoder)); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_i915_private *dev_priv = dev-dev_private; int dpll_md_reg; u32 adpa, dpll_md; - u32 adpa_reg; dpll_md_reg = DPLL_MD(intel_crtc-pipe); - if (HAS_PCH_SPLIT(dev)) - adpa_reg = PCH_ADPA; - else - adpa_reg = ADPA; - /* * Disable separate mode multiplier used when cloning SDVO to CRT * XXX this needs to be adjusted when we really are cloning @@ -185,7 +187,7 @@ static void intel_crt_mode_set(struct drm_encoder *encoder, if (!HAS_PCH_SPLIT(dev)) I915_WRITE(BCLRPAT(intel_crtc-pipe), 0); - I915_WRITE(adpa_reg, adpa); + I915_WRITE(crt-adpa_reg, adpa); } static bool intel_ironlake_crt_detect_hotplug(struct drm_connector *connector) @@ -682,6 +684,13 @@ void intel_crt_init(struct drm_device *dev) else encoder_helper_funcs = gmch_encoder_funcs; + if (HAS_PCH_SPLIT(dev)) + crt-adpa_reg = PCH_ADPA; + else if (IS_VALLEYVIEW(dev)) + crt-adpa_reg = VLV_ADPA; + else + crt-adpa_reg = ADPA; + drm_encoder_helper_add(crt-base.base, encoder_helper_funcs); drm_connector_helper_add(connector, intel_crt_connector_helper_funcs); -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 16/43] drm/i915/crt: convert to encoder disable/enable
CRT is the first output which can be cloned, hence we cannot (yet) move the dpms handling over to disable/enable. This requires some more smarts in intel_crtc_dpms first to set the display pipe status depening upon encoder-connectors_active of all connected encoders. Because that will happen in a separate step, don't touch the dpms functions, yet. v2: Be careful about clearing the _DISABLE flags for intermediate dpms modes - otherwise we might clobber the crt state when another (cloned) connector gets enabled. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_crt.c | 36 1 files changed, 32 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 8babc10..9525822 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -61,6 +61,29 @@ static struct intel_crt *intel_encoder_to_crt(struct intel_encoder *encoder) return container_of(encoder, struct intel_crt, base); } +static void intel_disable_crt(struct intel_encoder *encoder) +{ + struct drm_i915_private *dev_priv = encoder-base.dev-dev_private; + struct intel_crt *crt = intel_encoder_to_crt(encoder); + u32 temp; + + temp = I915_READ(crt-adpa_reg); + temp = ~(ADPA_HSYNC_CNTL_DISABLE | ADPA_VSYNC_CNTL_DISABLE); + temp = ~ADPA_DAC_ENABLE; + I915_WRITE(crt-adpa_reg, temp); +} + +static void intel_enable_crt(struct intel_encoder *encoder) +{ + struct drm_i915_private *dev_priv = encoder-base.dev-dev_private; + struct intel_crt *crt = intel_encoder_to_crt(encoder); + u32 temp; + + temp = I915_READ(crt-adpa_reg); + temp |= ADPA_DAC_ENABLE; + I915_WRITE(crt-adpa_reg, temp); +} + static void pch_crt_dpms(struct drm_encoder *encoder, int mode) { struct drm_device *dev = encoder-dev; @@ -582,18 +605,20 @@ static void intel_crt_reset(struct drm_connector *connector) static const struct drm_encoder_helper_funcs pch_encoder_funcs = { .mode_fixup = intel_crt_mode_fixup, - .prepare = intel_encoder_prepare, - .commit = intel_encoder_commit, + .prepare = intel_encoder_noop, + .commit = intel_encoder_noop, .mode_set = intel_crt_mode_set, .dpms = pch_crt_dpms, + .disable = intel_encoder_disable, }; static const struct drm_encoder_helper_funcs gmch_encoder_funcs = { .mode_fixup = intel_crt_mode_fixup, - .prepare = intel_encoder_prepare, - .commit = intel_encoder_commit, + .prepare = intel_encoder_noop, + .commit = intel_encoder_noop, .mode_set = intel_crt_mode_set, .dpms = gmch_crt_dpms, + .disable = intel_encoder_disable, }; static const struct drm_connector_funcs intel_crt_connector_funcs = { @@ -691,6 +716,9 @@ void intel_crt_init(struct drm_device *dev) else crt-adpa_reg = ADPA; + crt-base.disable = intel_disable_crt; + crt-base.enable = intel_enable_crt; + drm_encoder_helper_add(crt-base.base, encoder_helper_funcs); drm_connector_helper_add(connector, intel_crt_connector_helper_funcs); -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 17/43] drm/i915/sdvo: convert to encoder disabl/enable
Similar to crt, this doesn't convert the dpms functions. Also similar to crt, we don't switch of the display pipe for the intermediate modes, only DPMS_OFF is truely off. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_sdvo.c | 58 +++- 1 files changed, 56 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 2f5106a..d630db8 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1139,6 +1139,56 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder, intel_sdvo_write_sdvox(intel_sdvo, sdvox); } +static void intel_disable_sdvo(struct intel_encoder *encoder) +{ + struct drm_i915_private *dev_priv = encoder-base.dev-dev_private; + struct intel_sdvo *intel_sdvo = to_intel_sdvo(encoder-base); + u32 temp; + + intel_sdvo_set_active_outputs(intel_sdvo, 0); + if (0) + intel_sdvo_set_encoder_power_state(intel_sdvo, + DRM_MODE_DPMS_OFF); + + temp = I915_READ(intel_sdvo-sdvo_reg); + if ((temp SDVO_ENABLE) != 0) { + intel_sdvo_write_sdvox(intel_sdvo, temp ~SDVO_ENABLE); + } +} + +static void intel_enable_sdvo(struct intel_encoder *encoder) +{ + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_sdvo *intel_sdvo = to_intel_sdvo(encoder-base); + struct intel_crtc *intel_crtc = to_intel_crtc(encoder-base.crtc); + u32 temp; + bool input1, input2; + int i; + u8 status; + + temp = I915_READ(intel_sdvo-sdvo_reg); + if ((temp SDVO_ENABLE) == 0) + intel_sdvo_write_sdvox(intel_sdvo, temp | SDVO_ENABLE); + for (i = 0; i 2; i++) + intel_wait_for_vblank(dev, intel_crtc-pipe); + + status = intel_sdvo_get_trained_inputs(intel_sdvo, input1, input2); + /* Warn if the device reported failure to sync. +* A lot of SDVO devices fail to notify of sync, but it's +* a given it the status is a success, we succeeded. +*/ + if (status == SDVO_CMD_STATUS_SUCCESS !input1) { + DRM_DEBUG_KMS(First %s output reported failure to + sync\n, SDVO_NAME(intel_sdvo)); + } + + if (0) + intel_sdvo_set_encoder_power_state(intel_sdvo, + DRM_MODE_DPMS_ON); + intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo-attached_output); +} + static void intel_sdvo_dpms(struct drm_encoder *encoder, int mode) { struct drm_device *dev = encoder-dev; @@ -1844,9 +1894,10 @@ done: static const struct drm_encoder_helper_funcs intel_sdvo_helper_funcs = { .dpms = intel_sdvo_dpms, .mode_fixup = intel_sdvo_mode_fixup, - .prepare = intel_encoder_prepare, + .prepare = intel_encoder_noop, .mode_set = intel_sdvo_mode_set, - .commit = intel_encoder_commit, + .commit = intel_encoder_noop, + .disable = intel_encoder_disable }; static const struct drm_connector_funcs intel_sdvo_connector_funcs = { @@ -2573,6 +2624,9 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob) drm_encoder_helper_add(intel_encoder-base, intel_sdvo_helper_funcs); + intel_encoder-disable = intel_disable_sdvo; + intel_encoder-enable = intel_enable_sdvo; + /* In default case sdvo lvds is false */ if (!intel_sdvo_get_capabilities(intel_sdvo, intel_sdvo-caps)) goto err; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 18/43] drm/i915: simplify dvo dpms interface
All dvo drivers only support 2 dpms states, and our dvo driver even switches of the dvo port for anything else than DPMS_ON. Hence ditch this complexity and simply use bool enable. While reading through this code I've noticed that the mode_set function of ch7017 is a bit peculiar - it disable the lvds again, even though the crtc helper code should have done that ... This might be to work around an issue at driver load, we pretty much ignore the hw state when taking over. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/dvo.h|9 - drivers/gpu/drm/i915/dvo_ch7017.c |8 drivers/gpu/drm/i915/dvo_ch7xxx.c |4 ++-- drivers/gpu/drm/i915/dvo_ivch.c |8 drivers/gpu/drm/i915/dvo_sil164.c |4 ++-- drivers/gpu/drm/i915/dvo_tfp410.c |4 ++-- drivers/gpu/drm/i915/intel_dvo.c |4 ++-- 7 files changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/dvo.h b/drivers/gpu/drm/i915/dvo.h index 8c2ad01..8efec6d 100644 --- a/drivers/gpu/drm/i915/dvo.h +++ b/drivers/gpu/drm/i915/dvo.h @@ -58,13 +58,12 @@ struct intel_dvo_dev_ops { void (*create_resources)(struct intel_dvo_device *dvo); /* -* Turn on/off output or set intermediate power levels if available. +* Turn on/off output. * -* Unsupported intermediate modes drop to the lower power setting. -* If the mode is DPMSModeOff, the output must be disabled, -* as the DPLL may be disabled afterwards. +* Because none of our dvo drivers support an intermediate power levels, +* we don't expose this in the interfac. */ - void (*dpms)(struct intel_dvo_device *dvo, int mode); + void (*dpms)(struct intel_dvo_device *dvo, bool enable); /* * Callback for testing a video mode for a given output. diff --git a/drivers/gpu/drm/i915/dvo_ch7017.c b/drivers/gpu/drm/i915/dvo_ch7017.c index 1ca799a..71e7650 100644 --- a/drivers/gpu/drm/i915/dvo_ch7017.c +++ b/drivers/gpu/drm/i915/dvo_ch7017.c @@ -163,7 +163,7 @@ struct ch7017_priv { }; static void ch7017_dump_regs(struct intel_dvo_device *dvo); -static void ch7017_dpms(struct intel_dvo_device *dvo, int mode); +static void ch7017_dpms(struct intel_dvo_device *dvo, bool enable); static bool ch7017_read(struct intel_dvo_device *dvo, u8 addr, u8 *val) { @@ -309,7 +309,7 @@ static void ch7017_mode_set(struct intel_dvo_device *dvo, lvds_power_down = CH7017_LVDS_POWER_DOWN_DEFAULT_RESERVED | (mode-hdisplay 0x0700) 8; - ch7017_dpms(dvo, DRM_MODE_DPMS_OFF); + ch7017_dpms(dvo, false); ch7017_write(dvo, CH7017_HORIZONTAL_ACTIVE_PIXEL_INPUT, horizontal_active_pixel_input); ch7017_write(dvo, CH7017_HORIZONTAL_ACTIVE_PIXEL_OUTPUT, @@ -331,7 +331,7 @@ static void ch7017_mode_set(struct intel_dvo_device *dvo, } /* set the CH7017 power state */ -static void ch7017_dpms(struct intel_dvo_device *dvo, int mode) +static void ch7017_dpms(struct intel_dvo_device *dvo, bool enable) { uint8_t val; @@ -345,7 +345,7 @@ static void ch7017_dpms(struct intel_dvo_device *dvo, int mode) CH7017_DAC3_POWER_DOWN | CH7017_TV_POWER_DOWN_EN); - if (mode == DRM_MODE_DPMS_ON) { + if (enable) { /* Turn on the LVDS */ ch7017_write(dvo, CH7017_LVDS_POWER_DOWN, val ~CH7017_LVDS_POWER_DOWN_EN); diff --git a/drivers/gpu/drm/i915/dvo_ch7xxx.c b/drivers/gpu/drm/i915/dvo_ch7xxx.c index 4a03660..c1dea5b 100644 --- a/drivers/gpu/drm/i915/dvo_ch7xxx.c +++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c @@ -289,9 +289,9 @@ static void ch7xxx_mode_set(struct intel_dvo_device *dvo, } /* set the CH7xxx power state */ -static void ch7xxx_dpms(struct intel_dvo_device *dvo, int mode) +static void ch7xxx_dpms(struct intel_dvo_device *dvo, bool enable) { - if (mode == DRM_MODE_DPMS_ON) + if (enable) ch7xxx_writeb(dvo, CH7xxx_PM, CH7xxx_PM_DVIL | CH7xxx_PM_DVIP); else ch7xxx_writeb(dvo, CH7xxx_PM, CH7xxx_PM_FPD); diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c index 04f2893..fa8ff6b 100644 --- a/drivers/gpu/drm/i915/dvo_ivch.c +++ b/drivers/gpu/drm/i915/dvo_ivch.c @@ -288,7 +288,7 @@ static enum drm_mode_status ivch_mode_valid(struct intel_dvo_device *dvo, } /** Sets the power state of the panel connected to the ivch */ -static void ivch_dpms(struct intel_dvo_device *dvo, int mode) +static void ivch_dpms(struct intel_dvo_device *dvo, bool enable) { int i; uint16_t vr01, vr30, backlight; @@ -297,13 +297,13 @@ static void ivch_dpms(struct intel_dvo_device *dvo, int mode) if (!ivch_read(dvo, VR01, vr01)) return; - if (mode == DRM_MODE_DPMS_ON) + if (enable) backlight = 1; else
[Intel-gfx] [PATCH 19/43] drm/i915: simplify possible_clones computation
Intel hw only has one MUX for encoders, so outputs are not cloneable or all in the same group of cloneable outputs. This neatly simplifies the code and allows us to ditch some ugly if cascades in the dp and hdmi init code (well, we need these if cascades for other stuff still, but that can be taken care of in follow-up patches). Also explain why sdvo LVDS outputs can be cloned. Native LVDS (and also eDP) can't be cloned, because the panel fitter is before the ouput MUX (and for cpu eDP cloning isn't possible, anyway). Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_crt.c |4 +-- drivers/gpu/drm/i915/intel_display.c | 18 +++ drivers/gpu/drm/i915/intel_dp.c | 14 ++- drivers/gpu/drm/i915/intel_drv.h | 25 - drivers/gpu/drm/i915/intel_dvo.c | 39 +++-- drivers/gpu/drm/i915/intel_hdmi.c| 10 +--- drivers/gpu/drm/i915/intel_lvds.c|2 +- drivers/gpu/drm/i915/intel_sdvo.c| 14 ++-- drivers/gpu/drm/i915/intel_tv.c |2 +- 9 files changed, 65 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 9525822..c3f6680 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -690,9 +690,7 @@ void intel_crt_init(struct drm_device *dev) intel_connector_attach_encoder(intel_connector, crt-base); crt-base.type = INTEL_OUTPUT_ANALOG; - crt-base.clone_mask = (1 INTEL_SDVO_NON_TV_CLONE_BIT | - 1 INTEL_ANALOG_CLONE_BIT | - 1 INTEL_SDVO_LVDS_CLONE_BIT); + crt-base.cloneable = true; if (IS_HASWELL(dev)) crt-base.crtc_mask = (1 0); else diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 62acf98..11e1b10 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6678,15 +6678,23 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data, return 0; } -static int intel_encoder_clones(struct drm_device *dev, int type_mask) +static int intel_encoder_clones(struct intel_encoder *encoder) { - struct intel_encoder *encoder; + struct drm_device *dev = encoder-base.dev; + struct intel_encoder *source_encoder; int index_mask = 0; int entry = 0; - list_for_each_entry(encoder, dev-mode_config.encoder_list, base.head) { - if (type_mask encoder-clone_mask) + list_for_each_entry(source_encoder, + dev-mode_config.encoder_list, base.head) { + + if (encoder == source_encoder) index_mask |= (1 entry); + + /* Intel hw has only one MUX where enocoders could be cloned. */ + if (encoder-cloneable source_encoder-cloneable) + index_mask |= (1 entry); + entry++; } @@ -6845,7 +6853,7 @@ static void intel_setup_outputs(struct drm_device *dev) list_for_each_entry(encoder, dev-mode_config.encoder_list, base.head) { encoder-base.possible_crtcs = encoder-crtc_mask; encoder-base.possible_clones = - intel_encoder_clones(dev, encoder-clone_mask); + intel_encoder_clones(encoder); } /* disable all the possible outputs/crtcs before entering KMS mode */ diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index b8abbff..b7db872 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2479,18 +2479,10 @@ intel_dp_init(struct drm_device *dev, int output_reg) connector-polled = DRM_CONNECTOR_POLL_HPD; - if (output_reg == DP_B || output_reg == PCH_DP_B) - intel_encoder-clone_mask = (1 INTEL_DP_B_CLONE_BIT); - else if (output_reg == DP_C || output_reg == PCH_DP_C) - intel_encoder-clone_mask = (1 INTEL_DP_C_CLONE_BIT); - else if (output_reg == DP_D || output_reg == PCH_DP_D) - intel_encoder-clone_mask = (1 INTEL_DP_D_CLONE_BIT); + intel_encoder-cloneable = false; - if (is_edp(intel_dp)) { - intel_encoder-clone_mask = (1 INTEL_EDP_CLONE_BIT); - INIT_DELAYED_WORK(intel_dp-panel_vdd_work, - ironlake_panel_vdd_work); - } + INIT_DELAYED_WORK(intel_dp-panel_vdd_work, + ironlake_panel_vdd_work); intel_encoder-crtc_mask = (1 0) | (1 1) | (1 2); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 40f50c2..53a8b8d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -89,25 +89,6 @@ #define INTEL_OUTPUT_DISPLAYPORT 7 #define INTEL_OUTPUT_EDP 8 -/* Intel Pipe Clone
[Intel-gfx] [PATCH 20/43] drm/i915: add port parameter to intel_hdmi_init
Instead of having a giant if cascade to figure this out according to the passed-in register. We could do quite a bit more cleaning up and all by using the port at more places, but I think this should be part of a bigger rework to introduce a struct intel_digital_port which would keep track of all these things. I guess this will be part of some haswell-DP-induced refactoring. For now this rips out the big cascade, which is what annoyed me so much. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_ddi.c |2 +- drivers/gpu/drm/i915/intel_display.c | 14 +- drivers/gpu/drm/i915/intel_drv.h |2 +- drivers/gpu/drm/i915/intel_hdmi.c| 41 ++ 4 files changed, 21 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index b71303c..c2d8389 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -241,7 +241,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port) case PORT_B: case PORT_C: case PORT_D: - intel_hdmi_init(dev, DDI_BUF_CTL(port)); + intel_hdmi_init(dev, DDI_BUF_CTL(port), port); break; default: DRM_DEBUG_DRIVER(No handlers defined for port %d, skipping DDI initialization\n, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 11e1b10..8548bb2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6769,16 +6769,16 @@ static void intel_setup_outputs(struct drm_device *dev) /* PCH SDVOB multiplex with HDMIB */ found = intel_sdvo_init(dev, PCH_SDVOB, true); if (!found) - intel_hdmi_init(dev, HDMIB); + intel_hdmi_init(dev, HDMIB, PORT_B); if (!found (I915_READ(PCH_DP_B) DP_DETECTED)) intel_dp_init(dev, PCH_DP_B); } if (I915_READ(HDMIC) PORT_DETECTED) - intel_hdmi_init(dev, HDMIC); + intel_hdmi_init(dev, HDMIC, PORT_C); if (!dpd_is_edp I915_READ(HDMID) PORT_DETECTED) - intel_hdmi_init(dev, HDMID); + intel_hdmi_init(dev, HDMID, PORT_D); if (I915_READ(PCH_DP_C) DP_DETECTED) intel_dp_init(dev, PCH_DP_C); @@ -6792,13 +6792,13 @@ static void intel_setup_outputs(struct drm_device *dev) /* SDVOB multiplex with HDMIB */ found = intel_sdvo_init(dev, SDVOB, true); if (!found) - intel_hdmi_init(dev, SDVOB); + intel_hdmi_init(dev, SDVOB, PORT_B); if (!found (I915_READ(DP_B) DP_DETECTED)) intel_dp_init(dev, DP_B); } if (I915_READ(SDVOC) PORT_DETECTED) - intel_hdmi_init(dev, SDVOC); + intel_hdmi_init(dev, SDVOC, PORT_C); /* Shares lanes with HDMI on SDVOC */ if (I915_READ(DP_C) DP_DETECTED) @@ -6811,7 +6811,7 @@ static void intel_setup_outputs(struct drm_device *dev) found = intel_sdvo_init(dev, SDVOB, true); if (!found SUPPORTS_INTEGRATED_HDMI(dev)) { DRM_DEBUG_KMS(probing HDMI on SDVOB\n); - intel_hdmi_init(dev, SDVOB); + intel_hdmi_init(dev, SDVOB, PORT_B); } if (!found SUPPORTS_INTEGRATED_DP(dev)) { @@ -6831,7 +6831,7 @@ static void intel_setup_outputs(struct drm_device *dev) if (SUPPORTS_INTEGRATED_HDMI(dev)) { DRM_DEBUG_KMS(probing HDMI on SDVOC\n); - intel_hdmi_init(dev, SDVOC); + intel_hdmi_init(dev, SDVOC, PORT_C); } if (SUPPORTS_INTEGRATED_DP(dev)) { DRM_DEBUG_KMS(probing DP_C\n); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 53a8b8d..5cc4b3c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -331,7 +331,7 @@ extern void intel_attach_force_audio_property(struct drm_connector *connector); extern void intel_attach_broadcast_rgb_property(struct drm_connector *connector); extern void intel_crt_init(struct drm_device *dev); -extern void intel_hdmi_init(struct drm_device *dev, int sdvox_reg); +extern void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port); extern struct intel_hdmi
[Intel-gfx] [PATCH 21/43] drm/i915: convert dpms functions of dvo/sdvo/crt
Yeah, big patch but I couldn't come up with a neat idea of how to split it up further, that wouldn't break dpms on cloned configs somehow. But the changes in dvo/sdvo/crt are all pretty much orthonogal, so it's not too bad a patch. These are the only encoders that support cloning, which requires a few special changes compared to the previous patches. - Compute the desired state of the display pipe by walking all connected encoders and checking whether any has active connectors. To make this clearer, drop the old mode parameter to the crtc dpms function and rename it to intel_crtc_update_dpms. - There's the curious case of intel_crtc-dpms_mode. With the previous patches to remove the overlay pipe A code and to rework the load detect pipe code, the big users are gone. We still keep it to avoid enabling the pipe twice, but we duplicate this logic with crtc-active, too. Still, leave this for now and just push a fake dpms mode into it that reflects the state of the display pipe. Changes in the encoder dpms functions: - We clamp the dpms state to the supported range right away. This is escpecially important for the VGA outputs, where only older hw supports the intermediate states. This (and the crt-adpa_reg patch) allows us to unify the crt dpms code again between all variants (gmch, vlv and pch). - We only enable/disable the output for dvo/sdvo and leave the encoder running. The encoder will be disabled/enabled when we switch the state of the entire output pipeline (which will happen right away for non-cloned setups). This way the duplication is reduced and strange interaction when disabling output ports at the wrong time avoided. The dpms code for all three types of connectors contains a bit of duplicated logic, but I think keeping these special cases separate is simpler: CRT is the only one that hanldes intermediate dpms state (which requires extra logic to enable/disable things in the right order), and introducing some abstraction just to share the code between dvo and sdvo smells like overkill. We can do that once someone bothers to implement cloning for the more modern outputs. But I doubt that this will ever happen. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_crt.c | 97 + drivers/gpu/drm/i915/intel_display.c | 37 ++--- drivers/gpu/drm/i915/intel_drv.h |1 + drivers/gpu/drm/i915/intel_dvo.c | 39 ++ drivers/gpu/drm/i915/intel_sdvo.c| 60 + 5 files changed, 122 insertions(+), 112 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index c3f6680..3cca437 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -84,13 +84,17 @@ static void intel_enable_crt(struct intel_encoder *encoder) I915_WRITE(crt-adpa_reg, temp); } -static void pch_crt_dpms(struct drm_encoder *encoder, int mode) +/* Note: The caller is required to filter out dpms modes not supported by the + * platform. */ +static void intel_crtc_set_dpms(struct intel_encoder *encoder, int mode) { - struct drm_device *dev = encoder-dev; + struct drm_device *dev = encoder-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_crt *crt = intel_encoder_to_crt(encoder); u32 temp; - temp = I915_READ(PCH_ADPA); + temp = I915_READ(crt-adpa_reg); + temp = ~(ADPA_HSYNC_CNTL_DISABLE | ADPA_VSYNC_CNTL_DISABLE); temp = ~ADPA_DAC_ENABLE; switch (mode) { @@ -98,44 +102,59 @@ static void pch_crt_dpms(struct drm_encoder *encoder, int mode) temp |= ADPA_DAC_ENABLE; break; case DRM_MODE_DPMS_STANDBY: + temp |= ADPA_DAC_ENABLE | ADPA_HSYNC_CNTL_DISABLE; + break; case DRM_MODE_DPMS_SUSPEND: + temp |= ADPA_DAC_ENABLE | ADPA_VSYNC_CNTL_DISABLE; + break; case DRM_MODE_DPMS_OFF: - /* Just leave port enable cleared */ + temp |= ADPA_HSYNC_CNTL_DISABLE | ADPA_VSYNC_CNTL_DISABLE; break; } - I915_WRITE(PCH_ADPA, temp); + I915_WRITE(crt-adpa_reg, temp); } -static void gmch_crt_dpms(struct drm_encoder *encoder, int mode) +static void intel_crt_dpms(struct drm_connector *connector, int mode) { - struct drm_device *dev = encoder-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - u32 temp; - - temp = I915_READ(ADPA); - temp = ~(ADPA_HSYNC_CNTL_DISABLE | ADPA_VSYNC_CNTL_DISABLE); - temp = ~ADPA_DAC_ENABLE; + struct drm_device *dev = connector-dev; + struct intel_encoder *encoder = intel_attached_encoder(connector); + struct drm_crtc *crtc; + int old_dpms; - if (IS_VALLEYVIEW(dev) mode != DRM_MODE_DPMS_ON) + /* PCH platforms and VLV only support on/off. */ + if
[Intel-gfx] [PATCH 22/43] drm/i915: rip out encoder-disable/enable checks
All encoders are now converted so there's no need for these checks any more. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 24 1 files changed, 8 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 222bb41..d30306a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3205,10 +3205,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) intel_crtc_update_cursor(crtc, true); encoders: - for_each_encoder_on_crtc(dev, crtc, encoder) { - if (encoder-enable) - encoder-enable(encoder); - } + for_each_encoder_on_crtc(dev, crtc, encoder) + encoder-enable(encoder); } static void ironlake_crtc_disable(struct drm_crtc *crtc) @@ -3223,10 +3221,8 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) /* XXX: For compatability with the crtc helper code, call the encoder's * disable function unconditionally for now. */ - for_each_encoder_on_crtc(dev, crtc, encoder) { - if (encoder-disable) - encoder-disable(encoder); - } + for_each_encoder_on_crtc(dev, crtc, encoder) + encoder-disable(encoder); if (!intel_crtc-active) return; @@ -3369,10 +3365,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_crtc_update_cursor(crtc, true); encoders: - for_each_encoder_on_crtc(dev, crtc, encoder) { - if (encoder-enable) - encoder-enable(encoder); - } + for_each_encoder_on_crtc(dev, crtc, encoder) + encoder-enable(encoder); } static void i9xx_crtc_disable(struct drm_crtc *crtc) @@ -3386,10 +3380,8 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) /* XXX: For compatability with the crtc helper code, call the encoder's * disable function unconditionally for now. */ - for_each_encoder_on_crtc(dev, crtc, encoder) { - if (encoder-disable) - encoder-disable(encoder); - } + for_each_encoder_on_crtc(dev, crtc, encoder) + encoder-disable(encoder); if (!intel_crtc-active) return; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 23/43] drm/i915: clean up encoder_prepare/commit
We no longer need them. And now that all encoders are converted, we can finally move the cpt modeset check to the right place - at the end of the crtc_enable function. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 23 +++ drivers/gpu/drm/i915/intel_drv.h |2 -- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d30306a..d716634 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3207,6 +3207,9 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) encoders: for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); + + if (HAS_PCH_CPT(dev)) + intel_cpt_verify_modeset(dev, intel_crtc-pipe); } static void ironlake_crtc_disable(struct drm_crtc *crtc) @@ -3481,26 +3484,6 @@ static void intel_crtc_disable(struct drm_crtc *crtc) } } -void intel_encoder_prepare(struct drm_encoder *encoder) -{ - struct drm_encoder_helper_funcs *encoder_funcs = encoder-helper_private; - /* lvds has its own version of prepare see intel_lvds_prepare */ - encoder_funcs-dpms(encoder, DRM_MODE_DPMS_OFF); -} - -void intel_encoder_commit(struct drm_encoder *encoder) -{ - struct drm_encoder_helper_funcs *encoder_funcs = encoder-helper_private; - struct drm_device *dev = encoder-dev; - struct intel_crtc *intel_crtc = to_intel_crtc(encoder-crtc); - - /* lvds has its own version of commit see intel_lvds_commit */ - encoder_funcs-dpms(encoder, DRM_MODE_DPMS_ON); - - if (HAS_PCH_CPT(dev)) - intel_cpt_verify_modeset(dev, intel_crtc-pipe); -} - void intel_encoder_noop(struct drm_encoder *encoder) { } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d081158..5099440 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -375,8 +375,6 @@ extern enum drm_connector_status intel_panel_detect(struct drm_device *dev); extern void intel_crtc_load_lut(struct drm_crtc *crtc); extern void intel_crtc_update_dpms(struct drm_crtc *crtc); -extern void intel_encoder_prepare(struct drm_encoder *encoder); -extern void intel_encoder_commit(struct drm_encoder *encoder); extern void intel_encoder_noop(struct drm_encoder *encoder); extern void intel_encoder_disable(struct drm_encoder *encoder); extern void intel_encoder_destroy(struct drm_encoder *encoder); -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 24/43] drm/fb helper: don't call drm_crtc_helper_set_config
Go through the interface vtable instead, because not everyone might be using the crtc helper code. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/drm_fb_helper.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 5683b7f..bf97c0a 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -228,7 +228,7 @@ bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) int i, ret; for (i = 0; i fb_helper-crtc_count; i++) { struct drm_mode_set *mode_set = fb_helper-crtc_info[i].mode_set; - ret = drm_crtc_helper_set_config(mode_set); + ret = mode_set-crtc-funcs-set_config(mode_set); if (ret) error = true; } -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 25/43] drm: remove the list_head from drm_mode_set
It's unused. At it confused me quite a bit until I've discovered that. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- include/drm/drm_crtc.h |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index bac55c2..a1a0386 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -676,8 +676,6 @@ struct drm_plane { * This is used to set modes. */ struct drm_mode_set { - struct list_head head; - struct drm_framebuffer *fb; struct drm_crtc *crtc; struct drm_display_mode *mode; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 26/43] drm/i915: copypaste drm_crtc_helper_set_config
And the following static functions required by it: drm_encoder_crtc_ok, drm_crtc_helper_disable No changes safe for the s/drm/intel prefix change. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 314 +- 1 files changed, 313 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d716634..72c415b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6552,12 +6552,324 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = { .disable = intel_crtc_disable, }; +static bool intel_encoder_crtc_ok(struct drm_encoder *encoder, + struct drm_crtc *crtc) +{ + struct drm_device *dev; + struct drm_crtc *tmp; + int crtc_mask = 1; + + WARN(!crtc, checking null crtc?\n); + + dev = crtc-dev; + + list_for_each_entry(tmp, dev-mode_config.crtc_list, head) { + if (tmp == crtc) + break; + crtc_mask = 1; + } + + if (encoder-possible_crtcs crtc_mask) + return true; + return false; +} + +static int +intel_crtc_helper_disable(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc-dev; + struct drm_connector *connector; + struct drm_encoder *encoder; + + /* Decouple all encoders and their attached connectors from this crtc */ + list_for_each_entry(encoder, dev-mode_config.encoder_list, head) { + if (encoder-crtc != crtc) + continue; + + list_for_each_entry(connector, dev-mode_config.connector_list, head) { + if (connector-encoder != encoder) + continue; + + connector-encoder = NULL; + } + } + + drm_helper_disable_unused_functions(dev); + return 0; +} + +static int intel_crtc_set_config(struct drm_mode_set *set) +{ + struct drm_device *dev; + struct drm_crtc *save_crtcs, *new_crtc, *crtc; + struct drm_encoder *save_encoders, *new_encoder, *encoder; + struct drm_framebuffer *old_fb = NULL; + bool mode_changed = false; /* if true do a full mode set */ + bool fb_changed = false; /* if true and !mode_changed just do a flip */ + struct drm_connector *save_connectors, *connector; + int count = 0, ro, fail = 0; + struct drm_crtc_helper_funcs *crtc_funcs; + struct drm_mode_set save_set; + int ret; + int i; + + DRM_DEBUG_KMS(\n); + + if (!set) + return -EINVAL; + + if (!set-crtc) + return -EINVAL; + + if (!set-crtc-helper_private) + return -EINVAL; + + crtc_funcs = set-crtc-helper_private; + + if (!set-mode) + set-fb = NULL; + + if (set-fb) { + DRM_DEBUG_KMS([CRTC:%d] [FB:%d] #connectors=%d (x y) (%i %i)\n, + set-crtc-base.id, set-fb-base.id, + (int)set-num_connectors, set-x, set-y); + } else { + DRM_DEBUG_KMS([CRTC:%d] [NOFB]\n, set-crtc-base.id); + return intel_crtc_helper_disable(set-crtc); + } + + dev = set-crtc-dev; + + /* Allocate space for the backup of all (non-pointer) crtc, encoder and +* connector data. */ + save_crtcs = kzalloc(dev-mode_config.num_crtc * +sizeof(struct drm_crtc), GFP_KERNEL); + if (!save_crtcs) + return -ENOMEM; + + save_encoders = kzalloc(dev-mode_config.num_encoder * + sizeof(struct drm_encoder), GFP_KERNEL); + if (!save_encoders) { + kfree(save_crtcs); + return -ENOMEM; + } + + save_connectors = kzalloc(dev-mode_config.num_connector * + sizeof(struct drm_connector), GFP_KERNEL); + if (!save_connectors) { + kfree(save_crtcs); + kfree(save_encoders); + return -ENOMEM; + } + + /* Copy data. Note that driver private data is not affected. +* Should anything bad happen only the expected state is +* restored, not the drivers personal bookkeeping. +*/ + count = 0; + list_for_each_entry(crtc, dev-mode_config.crtc_list, head) { + save_crtcs[count++] = *crtc; + } + + count = 0; + list_for_each_entry(encoder, dev-mode_config.encoder_list, head) { + save_encoders[count++] = *encoder; + } + + count = 0; + list_for_each_entry(connector, dev-mode_config.connector_list, head) { + save_connectors[count++] = *connector; + } + + save_set.crtc = set-crtc; + save_set.mode = set-crtc-mode; + save_set.x = set-crtc-x; +
[Intel-gfx] [PATCH 27/43] drm/i915: call set_base directly
And drop the check, we always have it. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 11 ++- 1 files changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 72c415b..0fd9085 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6546,7 +6546,6 @@ static void intel_crtc_reset(struct drm_crtc *crtc) static struct drm_crtc_helper_funcs intel_helper_funcs = { .mode_fixup = intel_crtc_mode_fixup, .mode_set = intel_crtc_mode_set, - .mode_set_base = intel_pipe_set_base, .mode_set_base_atomic = intel_pipe_set_base_atomic, .load_lut = intel_crtc_load_lut, .disable = intel_crtc_disable, @@ -6608,7 +6607,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set) bool fb_changed = false; /* if true and !mode_changed just do a flip */ struct drm_connector *save_connectors, *connector; int count = 0, ro, fail = 0; - struct drm_crtc_helper_funcs *crtc_funcs; struct drm_mode_set save_set; int ret; int i; @@ -6624,7 +6622,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set) if (!set-crtc-helper_private) return -EINVAL; - crtc_funcs = set-crtc-helper_private; if (!set-mode) set-fb = NULL; @@ -6786,10 +6783,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set) } } - /* mode_set_base is not a required function */ - if (fb_changed !crtc_funcs-mode_set_base) - mode_changed = true; - if (mode_changed) { set-crtc-enabled = drm_helper_crtc_in_use(set-crtc); if (set-crtc-enabled) { @@ -6822,8 +6815,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set) old_fb = set-crtc-fb; if (set-crtc-fb != set-fb) set-crtc-fb = set-fb; - ret = crtc_funcs-mode_set_base(set-crtc, - set-x, set-y, old_fb); + ret = intel_pipe_set_base(set-crtc, + set-x, set-y, old_fb); if (ret != 0) { set-crtc-fb = old_fb; goto fail; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 28/43] drm/i915: inline intel_best_encoder
Also kill the error-path, we have a fixed connector-encoder mapping. Unfortunately we can't rip out all the -best_encoder callbacks, these are all still used by the fb_helper. Neat helper layering violation there. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 18 +++--- 1 files changed, 3 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0fd9085..0c15960 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6606,7 +6606,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set) bool mode_changed = false; /* if true do a full mode set */ bool fb_changed = false; /* if true and !mode_changed just do a flip */ struct drm_connector *save_connectors, *connector; - int count = 0, ro, fail = 0; + int count = 0, ro; struct drm_mode_set save_set; int ret; int i; @@ -6622,7 +6622,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set) if (!set-crtc-helper_private) return -EINVAL; - if (!set-mode) set-fb = NULL; @@ -6715,17 +6714,11 @@ static int intel_crtc_set_config(struct drm_mode_set *set) /* a) traverse passed in connector list and get encoders for them */ count = 0; list_for_each_entry(connector, dev-mode_config.connector_list, head) { - struct drm_connector_helper_funcs *connector_funcs = - connector-helper_private; new_encoder = connector-encoder; for (ro = 0; ro set-num_connectors; ro++) { if (set-connectors[ro] == connector) { - new_encoder = connector_funcs-best_encoder(connector); - /* if we can't get an encoder for a connector - we are setting now - then fail */ - if (new_encoder == NULL) - /* don't break so fail path works correct */ - fail = 1; + new_encoder = + intel_attached_encoder(connector)-base; break; } } @@ -6742,11 +6735,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set) } } - if (fail) { - ret = -EINVAL; - goto fail; - } - count = 0; list_for_each_entry(connector, dev-mode_config.connector_list, head) { if (!connector-encoder) -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 29/43] drm/i915: copypaste drm_crtc_helper_set_mode
Together with the static helpers drm_crtc_prepare_encoders and drm_encoder_disable (which will be simplified in the next patch, but for now are 1:1 copies). Again, no changes beside new names for these functions. Also call our new set_mode instead of the crtc helper one now in all the places we've done so far. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 160 +- drivers/gpu/drm/i915/intel_dp.c |6 +- drivers/gpu/drm/i915/intel_drv.h |4 + drivers/gpu/drm/i915/intel_hdmi.c|6 +- drivers/gpu/drm/i915/intel_lvds.c|5 +- drivers/gpu/drm/i915/intel_sdvo.c|4 +- drivers/gpu/drm/i915/intel_tv.c |4 +- 7 files changed, 173 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0c15960..574a610 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5653,7 +5653,7 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, goto fail; } - if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, old_fb)) { + if (!intel_crtc_set_mode(crtc, mode, 0, 0, old_fb)) { DRM_DEBUG_KMS(failed to set mode on load-detect pipe\n); if (old-release_fb) old-release_fb-funcs-destroy(old-release_fb); @@ -6597,6 +6597,158 @@ intel_crtc_helper_disable(struct drm_crtc *crtc) return 0; } +static void +intel_encoder_disable_helper(struct drm_encoder *encoder) +{ + struct drm_encoder_helper_funcs *encoder_funcs = encoder-helper_private; + + if (encoder_funcs-disable) + (*encoder_funcs-disable)(encoder); + else + (*encoder_funcs-dpms)(encoder, DRM_MODE_DPMS_OFF); +} + +static void +intel_crtc_prepare_encoders(struct drm_device *dev) +{ + struct drm_encoder_helper_funcs *encoder_funcs; + struct drm_encoder *encoder; + + list_for_each_entry(encoder, dev-mode_config.encoder_list, head) { + encoder_funcs = encoder-helper_private; + /* Disable unused encoders */ + if (encoder-crtc == NULL) + intel_encoder_disable_helper(encoder); + /* Disable encoders whose CRTC is about to change */ + if (encoder_funcs-get_crtc + encoder-crtc != (*encoder_funcs-get_crtc)(encoder)) + intel_encoder_disable_helper(encoder); + } +} + +bool intel_crtc_set_mode(struct drm_crtc *crtc, +struct drm_display_mode *mode, +int x, int y, +struct drm_framebuffer *old_fb) +{ + struct drm_device *dev = crtc-dev; + struct drm_display_mode *adjusted_mode, saved_mode, saved_hwmode; + struct drm_crtc_helper_funcs *crtc_funcs = crtc-helper_private; + struct drm_encoder_helper_funcs *encoder_funcs; + int saved_x, saved_y; + struct drm_encoder *encoder; + bool ret = true; + + crtc-enabled = drm_helper_crtc_in_use(crtc); + if (!crtc-enabled) + return true; + + adjusted_mode = drm_mode_duplicate(dev, mode); + if (!adjusted_mode) + return false; + + saved_hwmode = crtc-hwmode; + saved_mode = crtc-mode; + saved_x = crtc-x; + saved_y = crtc-y; + + /* Update crtc values up front so the driver can rely on them for mode +* setting. +*/ + crtc-mode = *mode; + crtc-x = x; + crtc-y = y; + + /* Pass our mode to the connectors and the CRTC to give them a chance to +* adjust it according to limitations or connector properties, and also +* a chance to reject the mode entirely. +*/ + list_for_each_entry(encoder, dev-mode_config.encoder_list, head) { + + if (encoder-crtc != crtc) + continue; + encoder_funcs = encoder-helper_private; + if (!(ret = encoder_funcs-mode_fixup(encoder, mode, + adjusted_mode))) { + DRM_DEBUG_KMS(Encoder fixup failed\n); + goto done; + } + } + + if (!(ret = crtc_funcs-mode_fixup(crtc, mode, adjusted_mode))) { + DRM_DEBUG_KMS(CRTC fixup failed\n); + goto done; + } + DRM_DEBUG_KMS([CRTC:%d]\n, crtc-base.id); + + /* Prepare the encoders and CRTCs before setting the mode. */ + list_for_each_entry(encoder, dev-mode_config.encoder_list, head) { + + if (encoder-crtc != crtc) + continue; + encoder_funcs = encoder-helper_private; + /* Disable the encoders as the first thing we do. */ + encoder_funcs-prepare(encoder); + } + +
[Intel-gfx] [PATCH 30/43] drm/i915: simplify intel_crtc_prepare_encoders
- We don't have the -get_crtc callback. - Call intel_encoder-disable directly. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 25 - 1 files changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 574a610..fefa600 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6598,31 +6598,14 @@ intel_crtc_helper_disable(struct drm_crtc *crtc) } static void -intel_encoder_disable_helper(struct drm_encoder *encoder) -{ - struct drm_encoder_helper_funcs *encoder_funcs = encoder-helper_private; - - if (encoder_funcs-disable) - (*encoder_funcs-disable)(encoder); - else - (*encoder_funcs-dpms)(encoder, DRM_MODE_DPMS_OFF); -} - -static void intel_crtc_prepare_encoders(struct drm_device *dev) { - struct drm_encoder_helper_funcs *encoder_funcs; - struct drm_encoder *encoder; + struct intel_encoder *encoder; - list_for_each_entry(encoder, dev-mode_config.encoder_list, head) { - encoder_funcs = encoder-helper_private; + list_for_each_entry(encoder, dev-mode_config.encoder_list, base.head) { /* Disable unused encoders */ - if (encoder-crtc == NULL) - intel_encoder_disable_helper(encoder); - /* Disable encoders whose CRTC is about to change */ - if (encoder_funcs-get_crtc - encoder-crtc != (*encoder_funcs-get_crtc)(encoder)) - intel_encoder_disable_helper(encoder); + if (encoder-base.crtc == NULL) + encoder-disable(encoder); } } -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 32/43] drm/i915: call crtc functions directly
Instead of going through the crtc helper function tables. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 15 +-- 1 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4d31b30..0c4eca5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6540,8 +6540,6 @@ static void intel_crtc_reset(struct drm_crtc *crtc) } static struct drm_crtc_helper_funcs intel_helper_funcs = { - .mode_fixup = intel_crtc_mode_fixup, - .mode_set = intel_crtc_mode_set, .mode_set_base_atomic = intel_pipe_set_base_atomic, .load_lut = intel_crtc_load_lut, .disable = intel_crtc_disable, @@ -6611,8 +6609,8 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc, struct drm_framebuffer *old_fb) { struct drm_device *dev = crtc-dev; + drm_i915_private_t *dev_priv = dev-dev_private; struct drm_display_mode *adjusted_mode, saved_mode, saved_hwmode; - struct drm_crtc_helper_funcs *crtc_funcs = crtc-helper_private; struct drm_encoder_helper_funcs *encoder_funcs; int saved_x, saved_y; struct drm_encoder *encoder; @@ -6654,7 +6652,7 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc, } } - if (!(ret = crtc_funcs-mode_fixup(crtc, mode, adjusted_mode))) { + if (!(ret = intel_crtc_mode_fixup(crtc, mode, adjusted_mode))) { DRM_DEBUG_KMS(CRTC fixup failed\n); goto done; } @@ -6662,12 +6660,12 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc, intel_crtc_prepare_encoders(dev); - crtc_funcs-prepare(crtc); + dev_priv-display.crtc_disable(crtc); /* Set up the DPLL and any encoders state that needs to adjust or depend * on the DPLL. */ - ret = !crtc_funcs-mode_set(crtc, mode, adjusted_mode, x, y, old_fb); + ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, old_fb); if (!ret) goto done; @@ -6684,7 +6682,7 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc, } /* Now enable the clocks, plane, pipe, and connectors that we set up. */ - crtc_funcs-commit(crtc); + dev_priv-display.crtc_enable(crtc); /* Store real post-adjustment hardware mode. */ crtc-hwmode = *adjusted_mode; @@ -7019,9 +7017,6 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) intel_crtc-active = true; /* force the pipe off on setup_init_config */ intel_crtc-bpp = 24; /* default for pre-Ironlake */ - intel_helper_funcs.prepare = dev_priv-display.crtc_disable; - intel_helper_funcs.commit = dev_priv-display.crtc_enable; - drm_crtc_helper_add(intel_crtc-base, intel_helper_funcs); intel_crtc-busy = false; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 31/43] drm/i915: rip out encoder-prepare/commit
With the new infrastructure we're doing this when enabling/disabling the entire display pipe. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_crt.c |2 -- drivers/gpu/drm/i915/intel_display.c | 24 drivers/gpu/drm/i915/intel_dp.c |2 -- drivers/gpu/drm/i915/intel_drv.h |1 - drivers/gpu/drm/i915/intel_dvo.c |2 -- drivers/gpu/drm/i915/intel_hdmi.c|4 drivers/gpu/drm/i915/intel_lvds.c|2 -- drivers/gpu/drm/i915/intel_sdvo.c|2 -- drivers/gpu/drm/i915/intel_tv.c |2 -- 9 files changed, 0 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 3cca437..7ea986c 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -624,8 +624,6 @@ static void intel_crt_reset(struct drm_connector *connector) static const struct drm_encoder_helper_funcs crt_encoder_funcs = { .mode_fixup = intel_crt_mode_fixup, - .prepare = intel_encoder_noop, - .commit = intel_encoder_noop, .mode_set = intel_crt_mode_set, .disable = intel_encoder_disable, }; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index fefa600..4d31b30 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3484,10 +3484,6 @@ static void intel_crtc_disable(struct drm_crtc *crtc) } } -void intel_encoder_noop(struct drm_encoder *encoder) -{ -} - void intel_encoder_disable(struct drm_encoder *encoder) { struct intel_encoder *intel_encoder = to_intel_encoder(encoder); @@ -6664,16 +6660,6 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc, } DRM_DEBUG_KMS([CRTC:%d]\n, crtc-base.id); - /* Prepare the encoders and CRTCs before setting the mode. */ - list_for_each_entry(encoder, dev-mode_config.encoder_list, head) { - - if (encoder-crtc != crtc) - continue; - encoder_funcs = encoder-helper_private; - /* Disable the encoders as the first thing we do. */ - encoder_funcs-prepare(encoder); - } - intel_crtc_prepare_encoders(dev); crtc_funcs-prepare(crtc); @@ -6700,16 +6686,6 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc, /* Now enable the clocks, plane, pipe, and connectors that we set up. */ crtc_funcs-commit(crtc); - list_for_each_entry(encoder, dev-mode_config.encoder_list, head) { - - if (encoder-crtc != crtc) - continue; - - encoder_funcs = encoder-helper_private; - encoder_funcs-commit(encoder); - - } - /* Store real post-adjustment hardware mode. */ crtc-hwmode = *adjusted_mode; diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 162b000..5b4498f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2359,9 +2359,7 @@ static void intel_dp_encoder_destroy(struct drm_encoder *encoder) static const struct drm_encoder_helper_funcs intel_dp_helper_funcs = { .mode_fixup = intel_dp_mode_fixup, - .prepare = intel_encoder_noop, .mode_set = intel_dp_mode_set, - .commit = intel_encoder_noop, .disable = intel_encoder_disable, }; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index b5544f0..70a8e10 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -379,7 +379,6 @@ extern bool intel_crtc_set_mode(struct drm_crtc *crtc, struct drm_framebuffer *old_fb); extern void intel_crtc_load_lut(struct drm_crtc *crtc); extern void intel_crtc_update_dpms(struct drm_crtc *crtc); -extern void intel_encoder_noop(struct drm_encoder *encoder); extern void intel_encoder_disable(struct drm_encoder *encoder); extern void intel_encoder_destroy(struct drm_encoder *encoder); extern void intel_encoder_dpms(struct intel_encoder *encoder, int mode); diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c index 8a188f4..34cbe8c 100644 --- a/drivers/gpu/drm/i915/intel_dvo.c +++ b/drivers/gpu/drm/i915/intel_dvo.c @@ -310,9 +310,7 @@ static void intel_dvo_destroy(struct drm_connector *connector) static const struct drm_encoder_helper_funcs intel_dvo_helper_funcs = { .mode_fixup = intel_dvo_mode_fixup, - .prepare = intel_encoder_noop, .mode_set = intel_dvo_mode_set, - .commit = intel_encoder_noop, .disable = intel_encoder_disable, }; diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 4b99140..1782357 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -886,17 +886,13 @@ static void intel_hdmi_destroy(struct drm_connector *connector) static const
[Intel-gfx] [PATCH 33/43] drm/i915: WARN when trying to enabled an unused crtc
This is the first tiny step towards cross-checking the entire modeset state machine with WARNs. A crtc can only be enabled when it's actually in use, i.e. crtc-active imlies crtc-enabled. Unfortunately we can't (yet) check this when disabling the crtc, because the crtc helpers are a bit slopy with updating state and unconditionally update crtc-enabled before changing the hw state. Fixing that requires quite some more work. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c |4 drivers/gpu/drm/i915/intel_drv.h | 10 +- 2 files changed, 13 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0c4eca5..e01cd09 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3153,6 +3153,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) u32 temp; bool is_pch_port; + WARN_ON(!crtc-enabled); + /* XXX: For compatability with the crtc helper code, call the encoder's * enable function unconditionally for now. */ if (intel_crtc-active) @@ -3348,6 +3350,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) int pipe = intel_crtc-pipe; int plane = intel_crtc-plane; + WARN_ON(!crtc-enabled); + /* XXX: For compatability with the crtc helper code, call the encoder's * enable function unconditionally for now. */ if (intel_crtc-active) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 70a8e10..8714f00 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -156,7 +156,15 @@ struct intel_crtc { enum plane plane; u8 lut_r[256], lut_g[256], lut_b[256]; int dpms_mode; - bool active; /* is the crtc on? independent of the dpms mode */ + /* +* Whether the crtc and the connected output pipeline is active. Implies +* that crtc-enabled is set, i.e. the current mode configuration has +* some outputs connected to this crtc. +* +* Atm crtc-enabled is unconditionally updated _before_ the hw state is +* changed, hence we can only check this when enabling the crtc. +*/ + bool active; bool primary_disabled; /* is the crtc obscured by a plane? */ bool busy; /* is scanout buffer being updated frequently? */ struct timer_list idle_timer; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 34/43] drm/i915: Add interfaces to read out encoder/connector hw state
It is all glorious if we try really hard to only enable an entire display pipe to ensure that everyting happens in the right order. But if we don't know the output configuration when the driver takes over, this will all be for vain because we'll make the hw angry right on the first modeset. Hence we need to be able to read out the current hw state, so that we can properly tear down the current hw state on the first modeset. Obviously this is also a nice preparation for the fastboot work, where we try to avoid the modeset on driver load if it matches what the hw is currently using. Furthermore we'll be using these functions to cross-check the actual hw state with what we think it should be, to ensure that the modeset state machine actually works as advertised. This patch only contains the interface definitions and a little helper for the simple case. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 11 +++ drivers/gpu/drm/i915/intel_drv.h |8 2 files changed, 19 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e01cd09..d423b9a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3541,6 +3541,17 @@ void intel_connector_dpms(struct drm_connector *connector, int mode) encoder-connectors_active = false; } +/* Simple connector-get_hw_state implementation for encoders that support only + * one connector and no cloning and hence the encoder state determines the state + * of the connector. */ +bool intel_connector_get_hw_state(struct intel_connector *connector) +{ + enum pipe pipe; + struct intel_encoder *encoder = connector-encoder; + + return encoder-get_hw_state(encoder, pipe); +} + static bool intel_crtc_mode_fixup(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8714f00..45dd067 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -142,12 +142,19 @@ struct intel_encoder { void (*hot_plug)(struct intel_encoder *); void (*enable)(struct intel_encoder *); void (*disable)(struct intel_encoder *); + /* Read out the current hw state of this connector, returning true if +* the encoder is active. If the encoder is enabled it also set the pipe +* it is connected to in the pipe parameter. */ + bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe); int crtc_mask; }; struct intel_connector { struct drm_connector base; struct intel_encoder *encoder; + /* Reads out the current hw, returning true if the connector is enabled +* and active (i.e. dpms ON state). */ + bool (*get_hw_state)(struct intel_connector *); }; struct intel_crtc { @@ -391,6 +398,7 @@ extern void intel_encoder_disable(struct drm_encoder *encoder); extern void intel_encoder_destroy(struct drm_encoder *encoder); extern void intel_encoder_dpms(struct intel_encoder *encoder, int mode); extern void intel_connector_dpms(struct drm_connector *, int mode); +extern bool intel_connector_get_hw_state(struct intel_connector *connector); static inline struct intel_encoder *intel_attached_encoder(struct drm_connector *connector) { -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 35/43] drm/i915/dp: implement get_hw_state
Also add some macros to make the pipe computation a bit easier. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_reg.h |2 + drivers/gpu/drm/i915/intel_dp.c | 50 +++ 2 files changed, 52 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 3b9c65e..428c3f3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4008,6 +4008,8 @@ #define PORT_TRANS_C_SEL_CPT (229) #define PORT_TRANS_SEL_MASK (329) #define PORT_TRANS_SEL_CPT(pipe) ((pipe) 29) +#define PORT_TO_PIPE_CPT(val) (((val) (130)) 30) +#define PORT_TO_PIPE(val) (((val) PORT_TRANS_SEL_MASK) 29) #define TRANS_DP_CTL_A 0xe0300 #define TRANS_DP_CTL_B 0xe1300 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5b4498f..648c6e5 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1277,6 +1277,54 @@ static void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) } } +static bool intel_dp_get_hw_state(struct intel_encoder *encoder, + enum pipe *pipe) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + u32 tmp = I915_READ(intel_dp-output_reg); + + if (!(tmp DP_PORT_EN)) + return false; + + if (is_cpu_edp(intel_dp) IS_GEN7(dev)) { + *pipe = PORT_TO_PIPE_CPT(tmp); + } else if (!HAS_PCH_CPT(dev) || is_cpu_edp(intel_dp)) { + *pipe = PORT_TO_PIPE(tmp); + } else { + u32 trans_sel; + u32 trans_dp; + int i; + + switch (intel_dp-output_reg) { + case PCH_DP_B: + trans_sel = TRANS_DP_PORT_SEL_B; + break; + case PCH_DP_C: + trans_sel = TRANS_DP_PORT_SEL_C; + break; + case PCH_DP_D: + trans_sel = TRANS_DP_PORT_SEL_D; + break; + default: + return true; + } + + for_each_pipe(i) { + trans_dp = I915_READ(TRANS_DP_CTL(i)); + if ((trans_dp TRANS_DP_PORT_SEL_MASK) == trans_sel) { + *pipe = i; + return true; + } + } + } + + DRM_DEBUG_KMS(No pipe for dp port 0x%x found\n, intel_dp-output_reg); + + return true; +} + static void intel_disable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); @@ -2496,6 +2544,8 @@ intel_dp_init(struct drm_device *dev, int output_reg) intel_encoder-enable = intel_enable_dp; intel_encoder-disable = intel_disable_dp; + intel_encoder-get_hw_state = intel_dp_get_hw_state; + intel_connector-get_hw_state = intel_connector_get_hw_state; /* Set up the DDC bus. */ switch (output_reg) { -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 36/43] drm/i915/hdmi: implement get_hw_state
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_ddi.c | 29 + drivers/gpu/drm/i915/intel_drv.h |2 ++ drivers/gpu/drm/i915/intel_hdmi.c | 24 3 files changed, 55 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index c2d8389..08d5a1e 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -729,6 +729,35 @@ void intel_ddi_mode_set(struct drm_encoder *encoder, intel_hdmi-set_infoframes(encoder, adjusted_mode); } +bool intel_ddi_get_hw_state(struct intel_encoder *encoder, + enum pipe *pipe) +{ + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder-base); + u32 tmp; + int i; + + tmp = I915_READ(DDI_BUF_CTL(intel_hdmi-ddi_port)); + + if (!(tmp DDI_BUF_CTL_ENABLE)) + return false; + + for_each_pipe(i) { + tmp = I915_READ(DDI_FUNC_CTL(i)); + + if ((tmp PIPE_DDI_PORT_MASK) + == PIPE_DDI_SELECT_PORT(intel_hdmi-ddi_port)) { + *pipe = i; + return true; + } + } + + DRM_DEBUG_KMS(No pipe for ddi port %i found\n, intel_hdmi-ddi_port); + + return true; +} + void intel_enable_ddi(struct intel_encoder *encoder) { struct drm_device *dev = encoder-base.dev; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 45dd067..aad231e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -506,6 +506,8 @@ extern void intel_disable_gt_powersave(struct drm_device *dev); extern void intel_enable_ddi(struct intel_encoder *encoder); extern void intel_disable_ddi(struct intel_encoder *encoder); +extern bool intel_ddi_get_hw_state(struct intel_encoder *encoder, + enum pipe *pipe); extern void intel_ddi_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 1782357..3ccf7fd 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -601,6 +601,27 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder, intel_hdmi-set_infoframes(encoder, adjusted_mode); } +static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder, + enum pipe *pipe) +{ + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder-base); + u32 tmp; + + tmp = I915_READ(intel_hdmi-sdvox_reg); + + if (!(tmp SDVO_ENABLE)) + return false; + + if (HAS_PCH_CPT(dev)) + *pipe = PORT_TO_PIPE_CPT(tmp); + else + *pipe = PORT_TO_PIPE(tmp); + + return true; +} + static void intel_enable_hdmi(struct intel_encoder *encoder) { struct drm_device *dev = encoder-base.dev; @@ -999,14 +1020,17 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port) if (IS_HASWELL(dev)) { intel_encoder-enable = intel_enable_ddi; intel_encoder-disable = intel_disable_ddi; + intel_encoder-get_hw_state = intel_ddi_get_hw_state; drm_encoder_helper_add(intel_encoder-base, intel_hdmi_helper_funcs_hsw); } else { intel_encoder-enable = intel_enable_hdmi; intel_encoder-disable = intel_disable_hdmi; + intel_encoder-get_hw_state = intel_hdmi_get_hw_state; drm_encoder_helper_add(intel_encoder-base, intel_hdmi_helper_funcs); } + intel_connector-get_hw_state = intel_connector_get_hw_state; intel_hdmi_add_properties(intel_hdmi, connector); -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 37/43] drm/i915/tv: implement get_hw_state
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_tv.c | 17 + 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 2fce753..5920c5f 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -836,6 +836,21 @@ static struct intel_tv *intel_attached_tv(struct drm_connector *connector) base); } +static bool +intel_tv_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe) +{ + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + u32 tmp = I915_READ(TV_CTL); + + if (!(tmp TV_ENC_ENABLE)) + return false; + + *pipe = PORT_TO_PIPE(tmp); + + return true; +} + static void intel_enable_tv(struct intel_encoder *encoder) { @@ -1622,6 +1637,8 @@ intel_tv_init(struct drm_device *dev) intel_encoder-enable = intel_enable_tv; intel_encoder-disable = intel_disable_tv; + intel_encoder-get_hw_state = intel_tv_get_hw_state; + intel_connector-get_hw_state = intel_connector_get_hw_state; intel_connector_attach_encoder(intel_connector, intel_encoder); intel_encoder-type = INTEL_OUTPUT_TVOUT; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 38/43] drm/i915/lvds: implement get_hw_state
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_lvds.c | 28 1 files changed, 28 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 7fb4bc1..7bcd4b2 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -65,6 +65,32 @@ static struct intel_lvds *intel_attached_lvds(struct drm_connector *connector) struct intel_lvds, base); } +static bool intel_lvds_get_hw_state(struct intel_encoder *encoder, + enum pipe *pipe) +{ + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + u32 lvds_reg, tmp; + + if (HAS_PCH_SPLIT(dev)) { + lvds_reg = PCH_LVDS; + } else { + lvds_reg = LVDS; + } + + tmp = I915_READ(lvds_reg); + + if (!(tmp LVDS_PORT_EN)) + return false; + + if (HAS_PCH_CPT(dev)) + *pipe = PORT_TO_PIPE_CPT(tmp); + else + *pipe = PORT_TO_PIPE(tmp); + + return true; +} + /** * Sets the power state for the panel. */ @@ -938,6 +964,8 @@ bool intel_lvds_init(struct drm_device *dev) intel_encoder-enable = intel_enable_lvds; intel_encoder-disable = intel_disable_lvds; + intel_encoder-get_hw_state = intel_lvds_get_hw_state; + intel_connector-get_hw_state = intel_connector_get_hw_state; intel_connector_attach_encoder(intel_connector, intel_encoder); intel_encoder-type = INTEL_OUTPUT_LVDS; -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 39/43] drm/i915/crt: implement get_hw_state
Note that even though this connector is cloneable we have still can use the exact same test to check whether the connector is on or whether the output is enabled - both the dpms code and the encoder disable/enable frob the exact same hw state. For dvo/sdvo outputs, this will be different. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_crt.c | 23 +++ 1 files changed, 23 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 7ea986c..0a5b61a 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -61,6 +61,27 @@ static struct intel_crt *intel_encoder_to_crt(struct intel_encoder *encoder) return container_of(encoder, struct intel_crt, base); } +static bool intel_crt_get_hw_state(struct intel_encoder *encoder, + enum pipe *pipe) +{ + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_crt *crt = intel_encoder_to_crt(encoder); + u32 tmp; + + tmp = I915_READ(crt-adpa_reg); + + if (!(tmp ADPA_DAC_ENABLE)) + return false; + + if (HAS_PCH_CPT(dev)) + *pipe = PORT_TO_PIPE_CPT(tmp); + else + *pipe = PORT_TO_PIPE(tmp); + + return true; +} + static void intel_disable_crt(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = encoder-base.dev-dev_private; @@ -717,6 +738,8 @@ void intel_crt_init(struct drm_device *dev) crt-base.disable = intel_disable_crt; crt-base.enable = intel_enable_crt; + crt-base.get_hw_state = intel_crt_get_hw_state; + intel_connector-get_hw_state = intel_connector_get_hw_state; drm_encoder_helper_add(crt-base.base, crt_encoder_funcs); drm_connector_helper_add(connector, intel_crt_connector_helper_funcs); -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 40/43] drm/i915/sdvo: implement get_hw_state
SDVO is the first real special case - we support multiple outputs on the same encoder and the encoder dpms state isn't the same as when just disabling the outputs when the encoder is cloned. Hence we need a real connector get_hw_state function which inquires the sdvo encoder about its active outputs. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_sdvo.c | 46 + 1 files changed, 46 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 92763d5..65b52a1 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -625,6 +625,14 @@ static bool intel_sdvo_set_active_outputs(struct intel_sdvo *intel_sdvo, outputs, sizeof(outputs)); } +static bool intel_sdvo_get_active_outputs(struct intel_sdvo *intel_sdvo, + u16 *outputs) +{ + return intel_sdvo_get_value(intel_sdvo, + SDVO_CMD_GET_ACTIVE_OUTPUTS, + outputs, sizeof(*outputs)); +} + static bool intel_sdvo_set_encoder_power_state(struct intel_sdvo *intel_sdvo, int mode) { @@ -1139,6 +1147,42 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder, intel_sdvo_write_sdvox(intel_sdvo, sdvox); } +static bool intel_sdvo_connector_get_hw_state(struct intel_connector *connector) +{ + struct intel_sdvo_connector *intel_sdvo_connector = + to_intel_sdvo_connector(connector-base); + struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector-base); + u16 active_outputs; + + intel_sdvo_get_active_outputs(intel_sdvo, active_outputs); + + if (active_outputs intel_sdvo_connector-output_flag) + return true; + else + return false; +} + +static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder, + enum pipe *pipe) +{ + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_sdvo *intel_sdvo = to_intel_sdvo(encoder-base); + u32 tmp; + + tmp = I915_READ(intel_sdvo-sdvo_reg); + + if (!(tmp SDVO_ENABLE)) + return false; + + if (HAS_PCH_CPT(dev)) + *pipe = PORT_TO_PIPE_CPT(tmp); + else + *pipe = PORT_TO_PIPE(tmp); + + return true; +} + static void intel_disable_sdvo(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = encoder-base.dev-dev_private; @@ -2063,6 +2107,7 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector, connector-base.base.interlace_allowed = 1; connector-base.base.doublescan_allowed = 0; connector-base.base.display_info.subpixel_order = SubPixelHorizontalRGB; + connector-base.get_hw_state = intel_sdvo_connector_get_hw_state; intel_connector_attach_encoder(connector-base, encoder-base); drm_sysfs_connector_add(connector-base.base); @@ -2616,6 +2661,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob) intel_encoder-disable = intel_disable_sdvo; intel_encoder-enable = intel_enable_sdvo; + intel_encoder-get_hw_state = intel_sdvo_get_hw_state; /* In default case sdvo lvds is false */ if (!intel_sdvo_get_capabilities(intel_sdvo, intel_sdvo-caps)) -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 41/43] drm/i915/dvo: implement get_hw_state
Similar to the sdvo code we poke the dvo encoder whether the output is active. Safe that dvo encoders are not standardized, so this requires a new callback into the dvo chip driver. Hence implement that for all 5 dvo drivers. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/dvo.h|6 ++ drivers/gpu/drm/i915/dvo_ch7017.c | 13 + drivers/gpu/drm/i915/dvo_ch7xxx.c | 13 + drivers/gpu/drm/i915/dvo_ivch.c | 15 +++ drivers/gpu/drm/i915/dvo_sil164.c | 16 drivers/gpu/drm/i915/dvo_tfp410.c | 14 ++ drivers/gpu/drm/i915/intel_dvo.c | 27 +++ 7 files changed, 104 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/dvo.h b/drivers/gpu/drm/i915/dvo.h index 8efec6d..0ccda16 100644 --- a/drivers/gpu/drm/i915/dvo.h +++ b/drivers/gpu/drm/i915/dvo.h @@ -114,6 +114,12 @@ struct intel_dvo_dev_ops { */ enum drm_connector_status (*detect)(struct intel_dvo_device *dvo); + /* +* Probe the current hw status, returning true if the connected output +* is active. +*/ + bool (*get_hw_state)(struct intel_dvo_device *dev); + /** * Query the device for the modes it provides. * diff --git a/drivers/gpu/drm/i915/dvo_ch7017.c b/drivers/gpu/drm/i915/dvo_ch7017.c index 71e7650..86b27d1 100644 --- a/drivers/gpu/drm/i915/dvo_ch7017.c +++ b/drivers/gpu/drm/i915/dvo_ch7017.c @@ -359,6 +359,18 @@ static void ch7017_dpms(struct intel_dvo_device *dvo, bool enable) msleep(20); } +static bool ch7017_get_hw_state(struct intel_dvo_device *dvo) +{ + uint8_t val; + + ch7017_read(dvo, CH7017_LVDS_POWER_DOWN, val); + + if (val CH7017_LVDS_POWER_DOWN_EN) + return false; + else + return true; +} + static void ch7017_dump_regs(struct intel_dvo_device *dvo) { uint8_t val; @@ -396,6 +408,7 @@ struct intel_dvo_dev_ops ch7017_ops = { .mode_valid = ch7017_mode_valid, .mode_set = ch7017_mode_set, .dpms = ch7017_dpms, + .get_hw_state = ch7017_get_hw_state, .dump_regs = ch7017_dump_regs, .destroy = ch7017_destroy, }; diff --git a/drivers/gpu/drm/i915/dvo_ch7xxx.c b/drivers/gpu/drm/i915/dvo_ch7xxx.c index c1dea5b..38f3a6c 100644 --- a/drivers/gpu/drm/i915/dvo_ch7xxx.c +++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c @@ -297,6 +297,18 @@ static void ch7xxx_dpms(struct intel_dvo_device *dvo, bool enable) ch7xxx_writeb(dvo, CH7xxx_PM, CH7xxx_PM_FPD); } +static bool ch7xxx_get_hw_state(struct intel_dvo_device *dvo) +{ + u8 val; + + ch7xxx_readb(dvo, CH7xxx_PM, val); + + if (val CH7xxx_PM_FPD) + return false; + else + return true; +} + static void ch7xxx_dump_regs(struct intel_dvo_device *dvo) { int i; @@ -326,6 +338,7 @@ struct intel_dvo_dev_ops ch7xxx_ops = { .mode_valid = ch7xxx_mode_valid, .mode_set = ch7xxx_mode_set, .dpms = ch7xxx_dpms, + .get_hw_state = ch7xxx_get_hw_state, .dump_regs = ch7xxx_dump_regs, .destroy = ch7xxx_destroy, }; diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c index fa8ff6b..baaf65b 100644 --- a/drivers/gpu/drm/i915/dvo_ivch.c +++ b/drivers/gpu/drm/i915/dvo_ivch.c @@ -323,6 +323,20 @@ static void ivch_dpms(struct intel_dvo_device *dvo, bool enable) udelay(16 * 1000); } +static bool ivch_get_hw_state(struct intel_dvo_device *dvo) +{ + uint16_t vr01; + + /* Set the new power state of the panel. */ + if (!ivch_read(dvo, VR01, vr01)) + return false; + + if (vr01 VR01_LCD_ENABLE) + return true; + else + return false; +} + static void ivch_mode_set(struct intel_dvo_device *dvo, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) @@ -413,6 +427,7 @@ static void ivch_destroy(struct intel_dvo_device *dvo) struct intel_dvo_dev_ops ivch_ops = { .init = ivch_init, .dpms = ivch_dpms, + .get_hw_state = ivch_get_hw_state, .mode_valid = ivch_mode_valid, .mode_set = ivch_mode_set, .detect = ivch_detect, diff --git a/drivers/gpu/drm/i915/dvo_sil164.c b/drivers/gpu/drm/i915/dvo_sil164.c index cc24c1c..4debd32 100644 --- a/drivers/gpu/drm/i915/dvo_sil164.c +++ b/drivers/gpu/drm/i915/dvo_sil164.c @@ -226,6 +226,21 @@ static void sil164_dpms(struct intel_dvo_device *dvo, bool enable) return; } +static bool sil164_get_hw_state(struct intel_dvo_device *dvo) +{ + int ret; + unsigned char ch; + + ret = sil164_readb(dvo, SIL164_REG8, ch); + if (ret == false) + return false; + + if (ch SIL164_8_PD) + return true; + else + return false; +} + static void
[Intel-gfx] [PATCH 42/43] drm/i915: read out the modeset hw state at load and resume time
... instead of resetting a few things and hoping that this will work out. To properly disable the output pipelines at the initial modeset after resume or boot up we need to have an accurate picture of which outputs are enabled and connected to which crtcs. Otherwise we risk disabling things at the wrong time, which can lead to hangs (or at least royally confused panels), both requiring a walk to the reset button to fix. Hence read out the hw state with the freshly introduce get_hw_state functions and then sanitize it afterwards. For a full modeset readout (which would allow us to avoid the initial modeset at boot up) a few things are still missing: - Reading out the mode from the pipe, especially the dotclock computation is quite some fun. - Reading out the parameters for the stolen memory framebuffer and wrapping it up. - Reading out the pch pll connections - luckily the disable code simply bails out if the crtc doesn't have a pch pll attached (even for configurations that would need one). This patch here turned up tons of smelly stuff around resume: We restore tons of register in seemingly random way (well, not quite, but we're not too careful either), which leaves the hw in a rather ill-defined state: E.g. the port registers are sometimes unconditionally restore (lvds, crt), leaving us with an active encoder/connector but no active pipe connected to it. Luckily the hw state sanitizer detects this madness and fixes things up a bit. v2: When checking whether an encoder with active connectors has a crtc wire up to it, check for both the crtc _and_ it's active state. v3: - Extract intel_sanitize_encoder. - Manually disable active encoders without an active pipe. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.c |1 + drivers/gpu/drm/i915/i915_drv.h |1 + drivers/gpu/drm/i915/intel_display.c | 276 ++ 3 files changed, 212 insertions(+), 66 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 57381eb..5f26aea 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -650,6 +650,7 @@ static int i915_drm_thaw(struct drm_device *dev) mutex_unlock(dev-struct_mutex); intel_modeset_init_hw(dev); + intel_modeset_setup_hw_state(dev); drm_mode_config_reset(dev); drm_irq_install(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 111d1e5..0ee0e0a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1511,6 +1511,7 @@ extern void intel_modeset_init(struct drm_device *dev); extern void intel_modeset_gem_init(struct drm_device *dev); extern void intel_modeset_cleanup(struct drm_device *dev); extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state); +extern void intel_modeset_setup_hw_state(struct drm_device *dev); extern bool intel_fbc_enabled(struct drm_device *dev); extern void intel_disable_fbc(struct drm_device *dev); extern bool ironlake_set_drps(struct drm_device *dev, u8 val); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d423b9a..a8791f8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3546,7 +3546,7 @@ void intel_connector_dpms(struct drm_connector *connector, int mode) * of the connector. */ bool intel_connector_get_hw_state(struct intel_connector *connector) { - enum pipe pipe; + enum pipe pipe = 0; struct intel_encoder *encoder = connector-encoder; return encoder-get_hw_state(encoder, pipe); @@ -6495,65 +6495,6 @@ free_work: return ret; } -static void intel_sanitize_modesetting(struct drm_device *dev, - int pipe, int plane) -{ - struct drm_i915_private *dev_priv = dev-dev_private; - u32 reg, val; - int i; - - /* Clear any frame start delays used for debugging left by the BIOS */ - for_each_pipe(i) { - reg = PIPECONF(i); - I915_WRITE(reg, I915_READ(reg) ~PIPECONF_FRAME_START_DELAY_MASK); - } - - if (HAS_PCH_SPLIT(dev)) - return; - - /* Who knows what state these registers were left in by the BIOS or -* grub? -* -* If we leave the registers in a conflicting state (e.g. with the -* display plane reading from the other pipe than the one we intend -* to use) then when we attempt to teardown the active mode, we will -* not disable the pipes and planes in the correct order -- leaving -* a plane reading from a disabled pipe and possibly leading to -* undefined behaviour. -*/ - - reg = DSPCNTR(plane); - val = I915_READ(reg); - - if ((val DISPLAY_PLANE_ENABLE) == 0) - return; - if (!!(val
[Intel-gfx] [PATCH 43/43] drm/i915: check connector hw/sw state
Atm we can only check the connector state after a dpms call - while doing modeset with the copypasted crtc helper code things are too ill-defined for proper checking. But the idea is very much to call this check from the modeset code, too. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_crt.c |2 + drivers/gpu/drm/i915/intel_display.c | 36 ++ drivers/gpu/drm/i915/intel_dp.c |2 + drivers/gpu/drm/i915/intel_drv.h |1 + drivers/gpu/drm/i915/intel_dvo.c |2 + drivers/gpu/drm/i915/intel_sdvo.c|2 + 6 files changed, 45 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 0a5b61a..14a2f24 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -176,6 +176,8 @@ static void intel_crt_dpms(struct drm_connector *connector, int mode) intel_crtc_update_dpms(crtc); } + + intel_connector_check_state(to_intel_connector(connector)); } static int intel_crt_mode_valid(struct drm_connector *connector, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a8791f8..14abbb8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3519,6 +3519,40 @@ void intel_encoder_dpms(struct intel_encoder *encoder, int mode) } } +/* Cross check the actual hw state with our own modeset state tracking (and it's + * internal consistency). */ +void intel_connector_check_state(struct intel_connector *connector) +{ + if (connector-get_hw_state(connector)) { + struct intel_encoder *encoder = connector-encoder; + struct drm_crtc *crtc; + bool encoder_enabled; + enum pipe pipe; + + WARN(connector-base.dpms != DRM_MODE_DPMS_ON, +wrong connector dpms state\n); + WARN(connector-base.encoder != encoder-base, +active connector not linked to encoder\n); + WARN(!encoder-connectors_active, +encoder-connectors_active not set\n); + + encoder_enabled = encoder-get_hw_state(encoder, pipe); + WARN(!encoder_enabled, encoder not enabled\n); + if (WARN_ON(!encoder-base.crtc)) + return; + + crtc = encoder-base.crtc; + + WARN(!crtc-enabled, crtc not enabled\n); + WARN(!to_intel_crtc(crtc)-active, crtc not active\n); + } else { + /* We still allow dpms changes on unused connectors, hence no +* check for dpms here. */ + WARN_ON(!connector-base.encoder); + } + +} + /* Even simpler default implementation, if there's really no special case to * consider. */ void intel_connector_dpms(struct drm_connector *connector, int mode) @@ -3539,6 +3573,8 @@ void intel_connector_dpms(struct drm_connector *connector, int mode) intel_encoder_dpms(encoder, mode); else encoder-connectors_active = false; + + intel_connector_check_state(to_intel_connector(connector)); } /* Simple connector-get_hw_state implementation for encoders that support only diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 648c6e5..61356b7 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1394,6 +1394,8 @@ intel_dp_dpms(struct drm_connector *connector, int mode) intel_encoder_dpms(intel_dp-base, mode); WARN_ON(intel_dp-dpms_mode != DRM_MODE_DPMS_ON); } + + intel_connector_check_state(to_intel_connector(connector)); } /* diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index aad231e..2be4515 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -399,6 +399,7 @@ extern void intel_encoder_destroy(struct drm_encoder *encoder); extern void intel_encoder_dpms(struct intel_encoder *encoder, int mode); extern void intel_connector_dpms(struct drm_connector *, int mode); extern bool intel_connector_get_hw_state(struct intel_connector *connector); +extern void intel_connector_check_state(struct intel_connector *); static inline struct intel_encoder *intel_attached_encoder(struct drm_connector *connector) { diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c index 88dd005..b9df54c 100644 --- a/drivers/gpu/drm/i915/intel_dvo.c +++ b/drivers/gpu/drm/i915/intel_dvo.c @@ -180,6 +180,8 @@ static void intel_dvo_dpms(struct drm_connector *connector, int mode) intel_crtc_update_dpms(crtc); } + + intel_connector_check_state(to_intel_connector(connector)); } static int intel_dvo_mode_valid(struct drm_connector *connector, diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
Re: [Intel-gfx] [ANNOUNCE] libdrm 2.4.37
The link generated by the release script was incorrect. They should be http://dri.freedesktop.org/libdrm/libdrm-2.4.37.tar.gz http://dri.freedesktop.org/libdrm/libdrm-2.4.37.tar.bz2 On Fri, 29 Jun 2012 11:17:47 -0700 Ben Widawsky b...@bwidawsk.net wrote: I botched the 2.3.36 release quite royally. Here is 2.6.37 this time with the proper context APIs in place. Ben Widawsky (2): intel/context: create/destroy implementation configure: bump version for release Kristian Høgsberg (1): modetest: Dump bit field names git tag: libdrm-2.4.37 http://dri.freedesktop.org/www/libdrm/libdrm-2.4.37.tar.bz2 MD5: 9765919c28d4a54887576db3680137cc libdrm-2.4.37.tar.bz2 SHA1: fa8463e390eee9b589dc369abc4cbe3e4ef16d16 libdrm-2.4.37.tar.bz2 SHA256: e4ea39a901d4a8e59064f10f413bb037dad7790f7c16a5986e7cc1453b36488f libdrm-2.4.37.tar.bz2 http://dri.freedesktop.org/www/libdrm/libdrm-2.4.37.tar.gz MD5: 7f762bfa0bdaa7216c926d0dc9629e87 libdrm-2.4.37.tar.gz SHA1: b086dc3f64570ac9aa9eccd23a1e8156e9038995 libdrm-2.4.37.tar.gz SHA256: b530d71ff9a7f5252f450a386540fe8512bde033e8283fa6a1bbcd3c62fc91e4 libdrm-2.4.37.tar.gz ___ xorg-announce mailing list xorg-annou...@lists.x.org http://lists.x.org/mailman/listinfo/xorg-announce -- 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 1/4] drm/i915: don't return a spurious -EIO from intel_ring_begin
My experience with these patches is that they make it less likely that the hang is reported to the userspace in a timely fashion (filling the ring full leads to lots of lost rendering) and worse make it much more likely that i915_gem_fault() hits an EIO and goes bang. That is unacceptable and trivial to hit with these patches. I have not yet reproduced that issue using the same broken renderer without these patches. I do think the patches are a step in the right direction, but with the change in userspace behaviour it has to be a NAK for the time being. -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/4] drm/i915: don't return a spurious -EIO from intel_ring_begin
On Tue, Jul 3, 2012 at 5:59 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: My experience with these patches is that they make it less likely that the hang is reported to the userspace in a timely fashion (filling the ring full leads to lots of lost rendering) and worse make it much more likely that i915_gem_fault() hits an EIO and goes bang. That is unacceptable and trivial to hit with these patches. I have not yet reproduced that issue using the same broken renderer without these patches. Hm, I don't see how these patches allow much more rendering to be queued up until we stop everything - for single-threaded userspace that should be at most one additional batch (which might have been the one that could catch the spurious -EIO). All subsequent execbuf ioctl calls should stall when trying to grab the mutex. Same for the case that the gpu reset failed, userspace should be able to submit one more batch afaict until it gets an -EIO. So can you please dig into what exactly your seeing a bit more and unconfuse me? I do think the patches are a step in the right direction, but with the change in userspace behaviour it has to be a NAK for the time being. Ok, I guess I'll have to throw the -EIO sigbus eater into the mix, too. After all userspace is supposed to call set_domain(GTT) before accessing the gtt mmap, so it should still get notice when the gpu has died and rendering might be incomplete. Imo we should still return -EIO for all ioctls, userspace should be able to cope with these (In the future we might even add optional behaviour to signal potentially dropped rendering due to a gpu reset at wait_rendering for userspace that really cares). So would the sigbus eater be good enough or do we need more? Thanks, Daniel -- Daniel Vetter daniel.vet...@ffwll.ch - +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: add PCH_NONE to enum intel_pch
From: Paulo Zanoni paulo.r.zan...@intel.com And rely on the fact that it's 0 to assume that machines without a PCH will have PCH_NONE as dev_priv-pch_type. Just today I finally realized that HAS_PCH_IBX is true for machines without a PCH. IMHO this is totally counter-intuitive and I don't think it's a good idea to assume that we're going to check for HAS_PCH_IBX only after we check for HAS_PCH_SPLIT. I believe that in the future we'll have more PCH types and checks like: if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) will become more and more common. There's a good chance that we may break non-PCH machines by adding these checks in code that runs on all machines. I also believe that the HAS_PCH_SPLIT check will become less common as we add more and more different PCH types. Also: are we sure we don't already have any bugs triggered by checking for HAS_PCH_IBX on non-PCH machines? Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h |1 + 1 file changed, 1 insertion(+) Another alternative would have been to change HAS_PCH_IBX to also check for HAS_PCH_SPLIT, but I'm not exactly in favor of adding more conditionals... diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b7a1eaa..b12e79a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -333,6 +333,7 @@ enum no_fbc_reason { }; enum intel_pch { + PCH_NONE = 0, /* No PCH present */ PCH_IBX,/* Ibexpeak PCH */ PCH_CPT,/* Cougarpoint PCH */ PCH_LPT,/* Lynxpoint PCH */ -- 1.7.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: get rid of dev_priv-info-has_pch_split
From: Paulo Zanoni paulo.r.zan...@intel.com Previously we had has_pch_split to tell us whether we had a PCH or not and we also had dev_priv-pch_type to tell us which kind of PCH it was, but it could only be used if we were 100% sure we did have a PCH. Now that PCH_NONE was added to dev_priv-pch_type we don't need has_pch_split anymore: we can just check for pch_type != PCH_NONE. The HAS_PCH_{IBX,CPT,LPT} macros use dev_priv-pch_type, so they can only be called after intel_detect_pch. The HAS_PCH_SPLIT macro looks at dev_priv-info-has_pch_split, which is available earlier. Since the goal is to implement HAS_PCH_SPLIT using dev_priv-pch_type instead of dev_priv-info-has_pch_split, we need to make sure that intel_detect_pch is called before any calls to HAS_PCH_SPLIT are made. So we moved the intel_detect_pch call to an earlier stage. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_dma.c |5 +++-- drivers/gpu/drm/i915/i915_drv.c |8 drivers/gpu/drm/i915/i915_drv.h |3 +-- 3 files changed, 4 insertions(+), 12 deletions(-) This patch does not solve any real problem: it's just a suggestion of something we could do after the previous patch. Some people may argue that looking at the has_pch_split variable might make it easier for us to find out which machines actually have a pch split without running the machine. So I really won't complain if we don't accept this patch: patch 01 is the important one. diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2166519..f8bc9ea 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1547,6 +1547,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_mtrrfree; } + /* This must be called before any calls to HAS_PCH_* */ + intel_detect_pch(dev); + intel_irq_init(dev); /* Try to make sure MCHBAR is enabled before poking at it */ @@ -1599,8 +1602,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) /* Start out suspended */ dev_priv-mm.suspended = 1; - intel_detect_pch(dev); - if (drm_core_check_feature(dev, DRIVER_MODESET)) { ret = i915_load_modeset_init(dev); if (ret 0) { diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7d0eb82..1794833 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -225,7 +225,6 @@ static const struct intel_device_info intel_ironlake_d_info = { .gen = 5, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, - .has_pch_split = 1, }; static const struct intel_device_info intel_ironlake_m_info = { @@ -233,7 +232,6 @@ static const struct intel_device_info intel_ironlake_m_info = { .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 1, .has_bsd_ring = 1, - .has_pch_split = 1, }; static const struct intel_device_info intel_sandybridge_d_info = { @@ -242,7 +240,6 @@ static const struct intel_device_info intel_sandybridge_d_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -253,7 +250,6 @@ static const struct intel_device_info intel_sandybridge_m_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -263,7 +259,6 @@ static const struct intel_device_info intel_ivybridge_d_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -274,7 +269,6 @@ static const struct intel_device_info intel_ivybridge_m_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -302,7 +296,6 @@ static const struct intel_device_info intel_haswell_d_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -312,7 +305,6 @@ static const struct intel_device_info intel_haswell_m_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b12e79a..89025ab 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -285,7 +285,6 @@ struct intel_device_info { u8 is_crestline:1; u8 is_ivybridge:1; u8 is_valleyview:1; - u8 has_pch_split:1; u8 has_force_wake:1; u8 is_haswell:1; u8 has_fbc:1; @@ -1113,13 +1112,13 @@ struct drm_i915_file_private { #define HAS_PIPE_CXSR(dev) (INTEL_INFO(dev)-has_pipe_cxsr) #define I915_HAS_FBC(dev)
[Intel-gfx] [PATCH 3/3] drm/i915: don't ironlake_init_pch_refclk() on LPT
From: Paulo Zanoni paulo.r.zan...@intel.com This function is used to set the PCH_DREF_CONTROL register, which does not exist on LPT anymore. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.c |2 +- drivers/gpu/drm/i915/intel_display.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) My crystal ball tells me patches like this will become more and more common in the future... That's why patch 01 was proposed. This patch depends on patch 01, otherwise we'll start calling ironlake_init_pch_refclk on non-pch machines. diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 1794833..0630471 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -687,7 +687,7 @@ static int i915_drm_thaw(struct drm_device *dev) /* KMS EnterVT equivalent */ if (drm_core_check_feature(dev, DRIVER_MODESET)) { - if (HAS_PCH_SPLIT(dev)) + if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) ironlake_init_pch_refclk(dev); mutex_lock(dev-struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8cd5032..da33911 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6855,7 +6855,7 @@ static void intel_setup_outputs(struct drm_device *dev) /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(dev); - if (HAS_PCH_SPLIT(dev)) + if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) ironlake_init_pch_refclk(dev); } -- 1.7.10.2 ___ 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: add PCH_NONE to enum intel_pch
On Tue, Jul 03, 2012 at 03:57:31PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com And rely on the fact that it's 0 to assume that machines without a PCH will have PCH_NONE as dev_priv-pch_type. Just today I finally realized that HAS_PCH_IBX is true for machines without a PCH. IMHO this is totally counter-intuitive and I don't think it's a good idea to assume that we're going to check for HAS_PCH_IBX only after we check for HAS_PCH_SPLIT. I believe that in the future we'll have more PCH types and checks like: if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) will become more and more common. There's a good chance that we may break non-PCH machines by adding these checks in code that runs on all machines. I also believe that the HAS_PCH_SPLIT check will become less common as we add more and more different PCH types. Also: are we sure we don't already have any bugs triggered by checking for HAS_PCH_IBX on non-PCH machines? I think most of the HAS_PCH_xxx are implicitly guarded because we've split up the pch modeset into it's own functions. I think there might only be a few issues in the encoder functions maybe. Have your checked all the HAS_PCH_IBX checks there? If you want, I can go through the code, too. Otherwise I really like this. -Daniel Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h |1 + 1 file changed, 1 insertion(+) Another alternative would have been to change HAS_PCH_IBX to also check for HAS_PCH_SPLIT, but I'm not exactly in favor of adding more conditionals... diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b7a1eaa..b12e79a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -333,6 +333,7 @@ enum no_fbc_reason { }; enum intel_pch { + PCH_NONE = 0, /* No PCH present */ PCH_IBX,/* Ibexpeak PCH */ PCH_CPT,/* Cougarpoint PCH */ PCH_LPT,/* Lynxpoint PCH */ -- 1.7.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ 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: get rid of dev_priv-info-has_pch_split
On Tue, Jul 03, 2012 at 03:57:32PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Previously we had has_pch_split to tell us whether we had a PCH or not and we also had dev_priv-pch_type to tell us which kind of PCH it was, but it could only be used if we were 100% sure we did have a PCH. Now that PCH_NONE was added to dev_priv-pch_type we don't need has_pch_split anymore: we can just check for pch_type != PCH_NONE. The HAS_PCH_{IBX,CPT,LPT} macros use dev_priv-pch_type, so they can only be called after intel_detect_pch. The HAS_PCH_SPLIT macro looks at dev_priv-info-has_pch_split, which is available earlier. Since the goal is to implement HAS_PCH_SPLIT using dev_priv-pch_type instead of dev_priv-info-has_pch_split, we need to make sure that intel_detect_pch is called before any calls to HAS_PCH_SPLIT are made. So we moved the intel_detect_pch call to an earlier stage. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_dma.c |5 +++-- drivers/gpu/drm/i915/i915_drv.c |8 drivers/gpu/drm/i915/i915_drv.h |3 +-- 3 files changed, 4 insertions(+), 12 deletions(-) This patch does not solve any real problem: it's just a suggestion of something we could do after the previous patch. Some people may argue that looking at the has_pch_split variable might make it easier for us to find out which machines actually have a pch split without running the machine. So I really won't complain if we don't accept this patch: patch 01 is the important one. diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2166519..f8bc9ea 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1547,6 +1547,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_mtrrfree; } + /* This must be called before any calls to HAS_PCH_* */ + intel_detect_pch(dev); + Hm, what about defining PCH_NONE as -1, PCH_RESERVED as 0 and then adding a WARN_ON(dev_priv-pch_type == PCH_RESERVED)? detect_pch is called unconditionally, and that way we would catch this. Might be overkill otoh, so if you think this is not worth it, np. -Daniel intel_irq_init(dev); /* Try to make sure MCHBAR is enabled before poking at it */ @@ -1599,8 +1602,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) /* Start out suspended */ dev_priv-mm.suspended = 1; - intel_detect_pch(dev); - if (drm_core_check_feature(dev, DRIVER_MODESET)) { ret = i915_load_modeset_init(dev); if (ret 0) { diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7d0eb82..1794833 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -225,7 +225,6 @@ static const struct intel_device_info intel_ironlake_d_info = { .gen = 5, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, - .has_pch_split = 1, }; static const struct intel_device_info intel_ironlake_m_info = { @@ -233,7 +232,6 @@ static const struct intel_device_info intel_ironlake_m_info = { .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 1, .has_bsd_ring = 1, - .has_pch_split = 1, }; static const struct intel_device_info intel_sandybridge_d_info = { @@ -242,7 +240,6 @@ static const struct intel_device_info intel_sandybridge_d_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -253,7 +250,6 @@ static const struct intel_device_info intel_sandybridge_m_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -263,7 +259,6 @@ static const struct intel_device_info intel_ivybridge_d_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -274,7 +269,6 @@ static const struct intel_device_info intel_ivybridge_m_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -302,7 +296,6 @@ static const struct intel_device_info intel_haswell_d_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -312,7 +305,6 @@ static const struct intel_device_info intel_haswell_m_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b12e79a..89025ab 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -285,7 +285,6 @@ struct
Re: [Intel-gfx] Valleyview: 3DSTATE_URB_VS Minimum URB Entries
Cheah, Douglas douglas.ch...@intel.com wrote: Hello folks, I am seeing corruption when running spectex from mesa demos which looks like vertex being randomly clipped on Valleyview, however spectex works fine on Ivy Bridge. After tracing down the codes I realize that the current Mesa driver would program the maximum number or URB entries (512 Valleyview) whenever possible. This conflict with the 3D pipeline PRM where it states there is a programming restriction if the URB Entry Allocation Size is less than 9 URB entries then total entries should be program to 32. After modifying the codes to meet the restriction I was able to run spectex without corruption on Valleyview, pretty surprise that Ivy Bridge did not have this problem. Here is the code snippet that I have changed which is at this point a quick hack and not upstream worthy as you can see the macro is not even properly defined in brw_defeines.h. Appreciate if I could get opinions from folks who are more familiar with Mesa and more experience with the graphic core. static void gen7_upload_urb(struct brw_context *brw) { struct intel_context *intel = brw-intel; /* Total space for entries is URB size - 16kB for push constants */ int handle_region_size = (brw-urb.size - 16) * 1024; /* bytes */ /* CACHE_NEW_VS_PROG */ brw-urb.vs_size = MAX2(brw-vs.prog_data-urb_entry_size, 1); int nr_vs_entries = handle_region_size / (brw-urb.vs_size * 64); if (nr_vs_entries brw-urb.max_vs_entries) nr_vs_entries = brw-urb.max_vs_entries; /* If the number of URB Allocation Size is smaller than 9 512 bit * units set the number or URB to Entries to 32 */ #define GEN7_URB_VS_MIN_ENTRIES 32 if(brw-urb.vs_size 9) nr_vs_entries = GEN7_URB_VS_MIN_ENTRIES; Douglas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ooh this is a good fix. Can you bounce it over to the meas list for inclusion? -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/10] drm/i915: introduce haswell_init_clock_gating
On Mon, Jul 02, 2012 at 11:51:09AM -0300, Eugeni Dodonov wrote: This is based on Ivy Bridge clock gating for now, but is subject to changes in the future. Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com This copy of presumeably the ivb clock gate code is missing the changes introduce in: commit 208482232de3590cee4757dfabe5d8cee8c6e626 Author: Ben Widawsky b...@bwidawsk.net Date: Fri May 4 18:58:59 2012 -0700 drm/i915: set IDICOS to medium uncore resources I guess that's not quite intentional ... All the previous patches up to here are queued for -next, thanks. -Daniel --- drivers/gpu/drm/i915/intel_pm.c | 54 - 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5ea8319..f54196f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3415,6 +3415,58 @@ static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv) I915_WRITE(GEN7_FF_THREAD_MODE, reg); } +static void haswell_init_clock_gating(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + int pipe; + uint32_t dspclk_gate = VRHUNIT_CLOCK_GATE_DISABLE; + + I915_WRITE(PCH_DSPCLK_GATE_D, dspclk_gate); + + I915_WRITE(WM3_LP_ILK, 0); + I915_WRITE(WM2_LP_ILK, 0); + I915_WRITE(WM1_LP_ILK, 0); + + /* According to the spec, bit 13 (RCZUNIT) must be set on IVB. + * This implements the WaDisableRCZUnitClockGating workaround. + */ + I915_WRITE(GEN6_UCGCTL2, GEN6_RCZUNIT_CLOCK_GATE_DISABLE); + + I915_WRITE(ILK_DSPCLK_GATE, IVB_VRHUNIT_CLK_GATE); + + I915_WRITE(IVB_CHICKEN3, +CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE | +CHICKEN3_DGMG_DONE_FIX_DISABLE); + + /* Apply the WaDisableRHWOOptimizationForRenderHang workaround. */ + I915_WRITE(GEN7_COMMON_SLICE_CHICKEN1, +GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC); + + /* WaApplyL3ControlAndL3ChickenMode requires those two on Ivy Bridge */ + I915_WRITE(GEN7_L3CNTLREG1, + GEN7_WA_FOR_GEN7_L3_CONTROL); + I915_WRITE(GEN7_L3_CHICKEN_MODE_REGISTER, + GEN7_WA_L3_CHICKEN_MODE); + + /* This is required by WaCatErrorRejectionIssue */ + I915_WRITE(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG, + I915_READ(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG) | + GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB); + + for_each_pipe(pipe) { + I915_WRITE(DSPCNTR(pipe), +I915_READ(DSPCNTR(pipe)) | +DISPPLANE_TRICKLE_FEED_DISABLE); + intel_flush_display_plane(dev_priv, pipe); + } + + gen7_setup_fixed_func_scheduler(dev_priv); + + /* WaDisable4x2SubspanOptimization */ + I915_WRITE(CACHE_MODE_1, +_MASKED_BIT_ENABLE(PIXEL_SUBSPAN_COLLECT_OPT_DISABLE)); +} + static void ivybridge_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; @@ -3824,7 +3876,7 @@ void intel_init_pm(struct drm_device *dev) Disable CxSR\n); dev_priv-display.update_wm = NULL; } - dev_priv-display.init_clock_gating = ivybridge_init_clock_gating; + dev_priv-display.init_clock_gating = haswell_init_clock_gating; dev_priv-display.sanitize_pm = gen6_sanitize_pm; } else dev_priv-display.update_wm = NULL; -- 1.7.11.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ 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: add PCH_NONE to enum intel_pch
2012/7/3 Daniel Vetter dan...@ffwll.ch: I think most of the HAS_PCH_xxx are implicitly guarded because we've split up the pch modeset into it's own functions. I think there might only be a few issues in the encoder functions maybe. Have your checked all the HAS_PCH_IBX checks there? If you want, I can go through the code, too. I did check. At the moment we have just a few HAS_PCH_IBX calls in our driver. The only possible issues may be inside intel_hdmi.c and intel_dp.c (and they need more investigation). My biggest worry here is being future-proof: are we sure whenever someone suggests adding HAS_PCH_IBX we'll remember that machines without a PCH return true for HAS_PCH_IBX? This is highly counter-intuitive. I really think that in future hardware enablement code we'll replace a lot of the if (HAS_PCH_SPLIT) { foo(); } else { bar(); } code for if (HAS_PCH_NEW) { baz(); } else if (HAS_PCH_OLD) { foo(); } else { bar(); }. Thanks, Paulo Otherwise I really like this. -Daniel Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h |1 + 1 file changed, 1 insertion(+) Another alternative would have been to change HAS_PCH_IBX to also check for HAS_PCH_SPLIT, but I'm not exactly in favor of adding more conditionals... diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b7a1eaa..b12e79a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -333,6 +333,7 @@ enum no_fbc_reason { }; enum intel_pch { + PCH_NONE = 0, /* No PCH present */ PCH_IBX,/* Ibexpeak PCH */ PCH_CPT,/* Cougarpoint PCH */ PCH_LPT,/* Lynxpoint PCH */ -- 1.7.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- Paulo Zanoni ___ 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: add PCH_NONE to enum intel_pch
On Tue, Jul 3, 2012 at 10:29 PM, Paulo Zanoni przan...@gmail.com wrote: 2012/7/3 Daniel Vetter dan...@ffwll.ch: I think most of the HAS_PCH_xxx are implicitly guarded because we've split up the pch modeset into it's own functions. I think there might only be a few issues in the encoder functions maybe. Have your checked all the HAS_PCH_IBX checks there? If you want, I can go through the code, too. I did check. At the moment we have just a few HAS_PCH_IBX calls in our driver. The only possible issues may be inside intel_hdmi.c and intel_dp.c (and they need more investigation). My biggest worry here is being future-proof: are we sure whenever someone suggests adding HAS_PCH_IBX we'll remember that machines without a PCH return true for HAS_PCH_IBX? This is highly counter-intuitive. I really think that in future hardware enablement code we'll replace a lot of the if (HAS_PCH_SPLIT) { foo(); } else { bar(); } code for if (HAS_PCH_NEW) { baz(); } else if (HAS_PCH_OLD) { foo(); } else { bar(); }. Ok, I've quickly checked them. The one in intel_dp.c isn't an issue, because DP is a gen5+ feature. So the only thing accidentally affected is vlv, which isn't such a big deal ;-) The only other check that isn't guarded with a HAS_PCH_SPLIT check is in intel_hdmi.c for a ibx-only w/a. That one will also leak out into gm45 platforms (which support hdmi, too). Otherwise I haven't found anything. Can you please amend the commit message detailing the effects on these two places? Just in case a bisect hits this patch and someone is totally confused what's going on here ... -Daniel -- Daniel Vetter daniel.vet...@ffwll.ch - +41 (0) 79 364 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/3] drm/i915: get rid of dev_priv-info-has_pch_split
2012/7/3 Daniel Vetter dan...@ffwll.ch: Hm, what about defining PCH_NONE as -1, PCH_RESERVED as 0 and then adding a WARN_ON(dev_priv-pch_type == PCH_RESERVED)? detect_pch is called unconditionally, and that way we would catch this. Might be overkill otoh, so if you think this is not worth it, np. I actually thought about this idea before. But it would only make sense if we add these WARNs to the HAS_PCH_FOO macros. But these macros are supposed to be simple and cheap and fast... Do we really want to start adding assertions inside them? If we think the price is worth paying, then we might do as you suggested. Or maybe this could be inside some #ifdef DEBUG... On my local machines, I changed the HAS_PCH_FOO macros to print some stuff so I could check whether any of them was called before intel_detect_pch. At least on these machines (SNB, HSW and a netbook), everything was fine. -Daniel intel_irq_init(dev); /* Try to make sure MCHBAR is enabled before poking at it */ @@ -1599,8 +1602,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) /* Start out suspended */ dev_priv-mm.suspended = 1; - intel_detect_pch(dev); - if (drm_core_check_feature(dev, DRIVER_MODESET)) { ret = i915_load_modeset_init(dev); if (ret 0) { diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7d0eb82..1794833 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -225,7 +225,6 @@ static const struct intel_device_info intel_ironlake_d_info = { .gen = 5, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, - .has_pch_split = 1, }; static const struct intel_device_info intel_ironlake_m_info = { @@ -233,7 +232,6 @@ static const struct intel_device_info intel_ironlake_m_info = { .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 1, .has_bsd_ring = 1, - .has_pch_split = 1, }; static const struct intel_device_info intel_sandybridge_d_info = { @@ -242,7 +240,6 @@ static const struct intel_device_info intel_sandybridge_d_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -253,7 +250,6 @@ static const struct intel_device_info intel_sandybridge_m_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -263,7 +259,6 @@ static const struct intel_device_info intel_ivybridge_d_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -274,7 +269,6 @@ static const struct intel_device_info intel_ivybridge_m_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -302,7 +296,6 @@ static const struct intel_device_info intel_haswell_d_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; @@ -312,7 +305,6 @@ static const struct intel_device_info intel_haswell_m_info = { .has_bsd_ring = 1, .has_blt_ring = 1, .has_llc = 1, - .has_pch_split = 1, .has_force_wake = 1, }; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b12e79a..89025ab 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -285,7 +285,6 @@ struct intel_device_info { u8 is_crestline:1; u8 is_ivybridge:1; u8 is_valleyview:1; - u8 has_pch_split:1; u8 has_force_wake:1; u8 is_haswell:1; u8 has_fbc:1; @@ -1113,13 +1112,13 @@ struct drm_i915_file_private { #define HAS_PIPE_CXSR(dev) (INTEL_INFO(dev)-has_pipe_cxsr) #define I915_HAS_FBC(dev) (INTEL_INFO(dev)-has_fbc) -#define HAS_PCH_SPLIT(dev) (INTEL_INFO(dev)-has_pch_split) #define HAS_PIPE_CONTROL(dev) (INTEL_INFO(dev)-gen = 5) #define INTEL_PCH_TYPE(dev) (((struct drm_i915_private *)(dev)-dev_private)-pch_type) #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT) #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) +#define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE) #define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)-has_force_wake) -- 1.7.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Ajdust down threshold in intel_pm.
The up and down thresholds are very asymetric, so it is possible to have a case where a spike of rendering increases the GPU clock to the max (because the up threshold is low) and then a simple blinking cursor is enough to keep the clock at the maximum speed forever (because the down threshold is high). Lowering the down threshold allows the GPU clock to go back down even when there is a blinking cursor on the screen. Signed-off-by: Stéphane Marchesin marc...@chromium.org --- drivers/gpu/drm/i915/intel_pm.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d0ce2a5..eba882a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2432,7 +2432,7 @@ void gen6_enable_rps(struct drm_i915_private *dev_priv) dev_priv-max_delay 24 | dev_priv-min_delay 16); I915_WRITE(GEN6_RP_UP_THRESHOLD, 1); - I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 100); + I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 10); I915_WRITE(GEN6_RP_UP_EI, 10); I915_WRITE(GEN6_RP_DOWN_EI, 500); I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10); -- 1.7.5.3.367.ga9930 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: add PCH_NONE to enum intel_pch
From: Paulo Zanoni paulo.r.zan...@intel.com And rely on the fact that it's 0 to assume that machines without a PCH will have PCH_NONE as dev_priv-pch_type. Just today I finally realized that HAS_PCH_IBX is true for machines without a PCH. IMHO this is totally counter-intuitive and I don't think it's a good idea to assume that we're going to check for HAS_PCH_IBX only after we check for HAS_PCH_SPLIT. I believe that in the future we'll have more PCH types and checks like: if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) will become more and more common. There's a good chance that we may break non-PCH machines by adding these checks in code that runs on all machines. I also believe that the HAS_PCH_SPLIT check will become less common as we add more and more different PCH types. We'll probably start replacing checks like: if (HAS_PCH_SPLIT(dev)) foo(); else bar(); with: if (HAS_PCH_NEW(dev)) baz(); else if (HAS_PCH_OLD(dev) || HAS_PCH_IBX(dev)) foo(); else bar(); and this may break gen 2/3/4. As far as we have investigated, this patch will affect the behavior of intel_hdmi_dpms and intel_dp_link_down on gen 4. In both functions the code inside the HAS_PCH_IBX check is for IBX-specific workarounds, so we should be safe. If we start bisecting gen 2/3/4 bugs to this commit we should consider replacing the HAS_PCH_IBX checks with something else. V2: Improve commit message, list possible side effects and solution. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h |1 + 1 file changed, 1 insertion(+) Another alternative would have been to change HAS_PCH_IBX to also check for HAS_PCH_SPLIT, but I'm not exactly in favor of adding more conditionals... diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b7a1eaa..b12e79a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -333,6 +333,7 @@ enum no_fbc_reason { }; enum intel_pch { + PCH_NONE = 0, /* No PCH present */ PCH_IBX,/* Ibexpeak PCH */ PCH_CPT,/* Cougarpoint PCH */ PCH_LPT,/* Lynxpoint PCH */ -- 1.7.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/10] drm/i915: introduce haswell_init_clock_gating
On 07/03/12 17:24, Daniel Vetter wrote: On Mon, Jul 02, 2012 at 11:51:09AM -0300, Eugeni Dodonov wrote: This is based on Ivy Bridge clock gating for now, but is subject to changes in the future. Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com This copy of presumeably the ivb clock gate code is missing the changes introduce in: commit 208482232de3590cee4757dfabe5d8cee8c6e626 Author: Ben Widawsky b...@bwidawsk.net Date: Fri May 4 18:58:59 2012 -0700 drm/i915: set IDICOS to medium uncore resources I guess that's not quite intentional ... All the previous patches up to here are queued for -next, thanks. I thought that this one was specific for Ivy Bridge, so I just skipped it... I have not tried setting these settings on Haswell, so I don't know if it is useful here as well. I'll try later this week to see if there are any visible effects. Eugeni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx