* [PATCH 0/9] displayless PCH
@ 2013-03-13 18:20 Ben Widawsky
2013-03-13 18:21 ` [PATCH 1/9] drm/i915: Move num_pipes to intel info Ben Widawsky
` (9 more replies)
0 siblings, 10 replies; 40+ messages in thread
From: Ben Widawsky @ 2013-03-13 18:20 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky, Art Runyan, Mcallister, Jeffrey
Certain fusing options allow customers to fuse off the South Display
which would result in hangs when reading/writing to those registers. In
addition, certain CPU display registers (like backlight) may actually
invoke writes to South Display.
This patch series enables our code to run displayless which can allow
the GEN hardware to be an offload GPU and not have a display hooked up.
This patch series hasn't been tested. A pre-cleaned version was tested,
and did work. I'm hoping Jeff (in the CC) can test this latest version.
patch 1: prep, could be separate
patch 2-7: Add some num_pipe == 0 checks
patch 8-9: Enable the actual platform we have
Ben Widawsky (9):
drm/i915: Move num_pipes to intel info
drm/i915: Support PCH no display
drm/i915: PCH_NOP
drm/i915: Don't touch South display when PCH_NOP
drm/i915: Don't initialize watermark stuff with PCH_NOP
drm/i915: PCH_NOP suspend/resume
drm/i915: Don't wait for PCH on reset
drm/i915: Set PCH_NOP
drm/i915: Add a pipeless ivybridge configuration
drivers/gpu/drm/i915/i915_dma.c | 27 ++++++------
drivers/gpu/drm/i915/i915_drv.c | 79 +++++++++++++++++++++++++-----------
drivers/gpu/drm/i915/i915_drv.h | 6 ++-
drivers/gpu/drm/i915/i915_gem.c | 3 ++
drivers/gpu/drm/i915/i915_irq.c | 23 +++++++----
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/i915_suspend.c | 57 ++++++++++++++++++--------
drivers/gpu/drm/i915/intel_bios.c | 3 ++
drivers/gpu/drm/i915/intel_crt.c | 3 ++
drivers/gpu/drm/i915/intel_display.c | 33 +++++++++------
drivers/gpu/drm/i915/intel_fb.c | 5 ++-
drivers/gpu/drm/i915/intel_i2c.c | 4 +-
drivers/gpu/drm/i915/intel_lvds.c | 4 ++
drivers/gpu/drm/i915/intel_overlay.c | 3 ++
drivers/gpu/drm/i915/intel_panel.c | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 5 ++-
16 files changed, 178 insertions(+), 80 deletions(-)
--
1.8.1.5
^ permalink raw reply [flat|nested] 40+ messages in thread* [PATCH 1/9] drm/i915: Move num_pipes to intel info 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky @ 2013-03-13 18:21 ` Ben Widawsky 2013-03-13 18:31 ` Chris Wilson ` (2 more replies) 2013-03-13 18:21 ` [PATCH 2/9] drm/i915: Support PCH no display Ben Widawsky ` (8 subsequent siblings) 9 siblings, 3 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:21 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter, Ben Widawsky, Mcallister, Jeffrey Requested by Daniel. Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_dma.c | 9 +------ drivers/gpu/drm/i915/i915_drv.c | 48 ++++++++++++++++++------------------ drivers/gpu/drm/i915/i915_drv.h | 4 +-- drivers/gpu/drm/i915/i915_irq.c | 3 +-- drivers/gpu/drm/i915/intel_display.c | 19 +++++++------- drivers/gpu/drm/i915/intel_fb.c | 2 +- drivers/gpu/drm/i915/intel_panel.c | 2 +- 7 files changed, 40 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index e16099b..ebcfe2e 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1630,14 +1630,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) mutex_init(&dev_priv->rps.hw_lock); mutex_init(&dev_priv->modeset_restore_lock); - if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) - dev_priv->num_pipe = 3; - else if (IS_MOBILE(dev) || !IS_GEN2(dev)) - dev_priv->num_pipe = 2; - else - dev_priv->num_pipe = 1; - - ret = drm_vblank_init(dev, dev_priv->num_pipe); + ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes); if (ret) goto out_gem_unload; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 1ebed96..0849651 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -136,74 +136,74 @@ extern int intel_agp_enabled; .driver_data = (unsigned long) info } static const struct intel_device_info intel_i830_info = { - .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, + .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 1, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_845g_info = { - .gen = 2, + .gen = 2, .num_pipes = 1, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i85x_info = { - .gen = 2, .is_i85x = 1, .is_mobile = 1, + .gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 1, .cursor_needs_physical = 1, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i865g_info = { - .gen = 2, + .gen = 2, .num_pipes = 1, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i915g_info = { - .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, + .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, .num_pipes = 1, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i915gm_info = { - .gen = 3, .is_mobile = 1, + .gen = 3, .is_mobile = 1, .num_pipes = 2, .cursor_needs_physical = 1, .has_overlay = 1, .overlay_needs_physical = 1, .supports_tv = 1, }; static const struct intel_device_info intel_i945g_info = { - .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, + .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, .num_pipes = 1, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i945gm_info = { - .gen = 3, .is_i945gm = 1, .is_mobile = 1, + .gen = 3, .is_i945gm = 1, .is_mobile = 1, .num_pipes = 2, .has_hotplug = 1, .cursor_needs_physical = 1, .has_overlay = 1, .overlay_needs_physical = 1, .supports_tv = 1, }; static const struct intel_device_info intel_i965g_info = { - .gen = 4, .is_broadwater = 1, + .gen = 4, .is_broadwater = 1, .num_pipes = 1, .has_hotplug = 1, .has_overlay = 1, }; static const struct intel_device_info intel_i965gm_info = { - .gen = 4, .is_crestline = 1, + .gen = 4, .is_crestline = 1, .num_pipes = 2, .is_mobile = 1, .has_fbc = 1, .has_hotplug = 1, .has_overlay = 1, .supports_tv = 1, }; static const struct intel_device_info intel_g33_info = { - .gen = 3, .is_g33 = 1, + .gen = 3, .is_g33 = 1, .num_pipes = 1, .need_gfx_hws = 1, .has_hotplug = 1, .has_overlay = 1, }; static const struct intel_device_info intel_g45_info = { - .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, + .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, .num_pipes = 1, .has_pipe_cxsr = 1, .has_hotplug = 1, .has_bsd_ring = 1, }; static const struct intel_device_info intel_gm45_info = { - .gen = 4, .is_g4x = 1, + .gen = 4, .is_g4x = 1, .num_pipes = 2, .is_mobile = 1, .need_gfx_hws = 1, .has_fbc = 1, .has_pipe_cxsr = 1, .has_hotplug = 1, .supports_tv = 1, @@ -211,26 +211,26 @@ static const struct intel_device_info intel_gm45_info = { }; static const struct intel_device_info intel_pineview_info = { - .gen = 3, .is_g33 = 1, .is_pineview = 1, .is_mobile = 1, + .gen = 3, .is_g33 = 1, .is_pineview = 1, .is_mobile = 1, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_overlay = 1, }; static const struct intel_device_info intel_ironlake_d_info = { - .gen = 5, + .gen = 5, .num_pipes = 1, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, }; static const struct intel_device_info intel_ironlake_m_info = { - .gen = 5, .is_mobile = 1, + .gen = 5, .is_mobile = 1, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 1, .has_bsd_ring = 1, }; static const struct intel_device_info intel_sandybridge_d_info = { - .gen = 6, + .gen = 6, .num_pipes = 1, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, .has_blt_ring = 1, @@ -239,7 +239,7 @@ static const struct intel_device_info intel_sandybridge_d_info = { }; static const struct intel_device_info intel_sandybridge_m_info = { - .gen = 6, .is_mobile = 1, + .gen = 6, .is_mobile = 1, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 1, .has_bsd_ring = 1, @@ -249,7 +249,7 @@ static const struct intel_device_info intel_sandybridge_m_info = { }; static const struct intel_device_info intel_ivybridge_d_info = { - .is_ivybridge = 1, .gen = 7, + .is_ivybridge = 1, .gen = 7, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, .has_blt_ring = 1, @@ -258,7 +258,7 @@ static const struct intel_device_info intel_ivybridge_d_info = { }; static const struct intel_device_info intel_ivybridge_m_info = { - .is_ivybridge = 1, .gen = 7, .is_mobile = 1, + .is_ivybridge = 1, .gen = 7, .is_mobile = 1, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 0, /* FBC is not enabled on Ivybridge mobile yet */ .has_bsd_ring = 1, @@ -268,7 +268,7 @@ static const struct intel_device_info intel_ivybridge_m_info = { }; static const struct intel_device_info intel_valleyview_m_info = { - .gen = 7, .is_mobile = 1, + .gen = 7, .is_mobile = 1, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 0, .has_bsd_ring = 1, @@ -279,7 +279,7 @@ static const struct intel_device_info intel_valleyview_m_info = { }; static const struct intel_device_info intel_valleyview_d_info = { - .gen = 7, + .gen = 7, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 0, .has_bsd_ring = 1, @@ -290,7 +290,7 @@ static const struct intel_device_info intel_valleyview_d_info = { }; static const struct intel_device_info intel_haswell_d_info = { - .is_haswell = 1, .gen = 7, + .is_haswell = 1, .gen = 7, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, .has_blt_ring = 1, @@ -299,7 +299,7 @@ static const struct intel_device_info intel_haswell_d_info = { }; static const struct intel_device_info intel_haswell_m_info = { - .is_haswell = 1, .gen = 7, .is_mobile = 1, + .is_haswell = 1, .gen = 7, .is_mobile = 1, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, .has_blt_ring = 1, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ca6b215..587dca0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -93,7 +93,7 @@ enum port { I915_GEM_DOMAIN_INSTRUCTION | \ I915_GEM_DOMAIN_VERTEX) -#define for_each_pipe(p) for ((p) = 0; (p) < dev_priv->num_pipe; (p)++) +#define for_each_pipe(p) for ((p) = 0; (p) < INTEL_INFO(dev)->num_pipes; (p)++) #define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \ list_for_each_entry((intel_encoder), &(dev)->mode_config.encoder_list, base.head) \ @@ -341,6 +341,7 @@ struct drm_i915_gt_funcs { struct intel_device_info { u32 display_mmio_offset; + u8 num_pipes:3; u8 gen; u8 is_mobile:1; u8 is_i85x:1; @@ -912,7 +913,6 @@ typedef struct drm_i915_private { struct work_struct hotplug_work; bool enable_hotplug_processing; - int num_pipe; int num_pch_pll; unsigned long cfb_size; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2139714..b860f0b 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -254,10 +254,9 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, struct timeval *vblank_time, unsigned flags) { - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc; - if (pipe < 0 || pipe >= dev_priv->num_pipe) { + if (pipe < 0 || pipe >= INTEL_INFO(dev)->num_pipes) { DRM_ERROR("Invalid crtc %d\n", pipe); return -EINVAL; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 502cb28..23379e7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2335,10 +2335,10 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, return 0; } - if(intel_crtc->plane > dev_priv->num_pipe) { + if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) { DRM_ERROR("no plane for crtc: plane %d, num_pipes %d\n", intel_crtc->plane, - dev_priv->num_pipe); + INTEL_INFO(dev)->num_pipes); return -EINVAL; } @@ -5359,7 +5359,7 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc) return false; } - if (dev_priv->num_pipe == 2) + if (INTEL_INFO(dev)->num_pipes == 2) return true; switch (intel_crtc->pipe) { @@ -8870,9 +8870,10 @@ void intel_modeset_init(struct drm_device *dev) dev->mode_config.fb_base = dev_priv->gtt.mappable_base; DRM_DEBUG_KMS("%d display pipe%s available.\n", - dev_priv->num_pipe, dev_priv->num_pipe > 1 ? "s" : ""); + INTEL_INFO(dev)->num_pipes, + INTEL_INFO(dev)->num_pipes > 1 ? "s" : ""); - for (i = 0; i < dev_priv->num_pipe; i++) { + for (i = 0; i < INTEL_INFO(dev)->num_pipes; i++) { intel_crtc_init(dev, i); ret = intel_plane_init(dev, i); if (ret) @@ -8929,10 +8930,11 @@ static void intel_enable_pipe_a(struct drm_device *dev) static bool intel_check_plane_mapping(struct intel_crtc *crtc) { - struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; u32 reg, val; - if (dev_priv->num_pipe == 1) + if (INTEL_INFO(dev)->num_pipes == 1) return true; reg = DSPCNTR(!crtc->plane); @@ -9366,10 +9368,9 @@ intel_display_print_error_state(struct seq_file *m, struct drm_device *dev, struct intel_display_error_state *error) { - drm_i915_private_t *dev_priv = dev->dev_private; int i; - seq_printf(m, "Num Pipes: %d\n", dev_priv->num_pipe); + seq_printf(m, "Num Pipes: %d\n", INTEL_INFO(dev)->num_pipes); for_each_pipe(i) { seq_printf(m, "Pipe [%d]:\n", i); seq_printf(m, " CONF: %08x\n", error->pipe[i].conf); diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c index 953ee73..dcdb1d3 100644 --- a/drivers/gpu/drm/i915/intel_fb.c +++ b/drivers/gpu/drm/i915/intel_fb.c @@ -240,7 +240,7 @@ int intel_fbdev_init(struct drm_device *dev) ifbdev->helper.funcs = &intel_fb_helper_funcs; ret = drm_fb_helper_init(dev, &ifbdev->helper, - dev_priv->num_pipe, + INTEL_INFO(dev)->num_pipes, INTELFB_CONN_LIMIT); if (ret) { kfree(ifbdev); diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index a3730e0..f1530f4 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -338,7 +338,7 @@ void intel_panel_enable_backlight(struct drm_device *dev, if (tmp & BLM_PWM_ENABLE) goto set_level; - if (dev_priv->num_pipe == 3) + if (INTEL_INFO(dev)->num_pipes == 3) tmp &= ~BLM_PIPE_SELECT_IVB; else tmp &= ~BLM_PIPE_SELECT; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 1/9] drm/i915: Move num_pipes to intel info 2013-03-13 18:21 ` [PATCH 1/9] drm/i915: Move num_pipes to intel info Ben Widawsky @ 2013-03-13 18:31 ` Chris Wilson 2013-03-13 18:36 ` Ben Widawsky 2013-03-13 21:05 ` [PATCH 01/10] [v2] " Ben Widawsky 2013-03-19 19:25 ` [PATCH 1/9] " Jesse Barnes 2 siblings, 1 reply; 40+ messages in thread From: Chris Wilson @ 2013-03-13 18:31 UTC (permalink / raw) To: Ben Widawsky; +Cc: Daniel Vetter, intel-gfx, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 11:21:00AM -0700, Ben Widawsky wrote: > Requested by Daniel. > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_dma.c | 9 +------ > drivers/gpu/drm/i915/i915_drv.c | 48 ++++++++++++++++++------------------ > drivers/gpu/drm/i915/i915_drv.h | 4 +-- > drivers/gpu/drm/i915/i915_irq.c | 3 +-- > drivers/gpu/drm/i915/intel_display.c | 19 +++++++------- > drivers/gpu/drm/i915/intel_fb.c | 2 +- > drivers/gpu/drm/i915/intel_panel.c | 2 +- > 7 files changed, 40 insertions(+), 47 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index e16099b..ebcfe2e 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1630,14 +1630,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > mutex_init(&dev_priv->rps.hw_lock); > mutex_init(&dev_priv->modeset_restore_lock); > > - if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) > - dev_priv->num_pipe = 3; > - else if (IS_MOBILE(dev) || !IS_GEN2(dev)) > - dev_priv->num_pipe = 2; > - else > - dev_priv->num_pipe = 1; > - > - ret = drm_vblank_init(dev, dev_priv->num_pipe); > + ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes); > if (ret) > goto out_gem_unload; > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 1ebed96..0849651 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -136,74 +136,74 @@ extern int intel_agp_enabled; > .driver_data = (unsigned long) info } > > static const struct intel_device_info intel_i830_info = { > - .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, > + .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 1, 2 pipes for 830gm > static const struct intel_device_info intel_i85x_info = { > - .gen = 2, .is_i85x = 1, .is_mobile = 1, > + .gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 1, 2 pipes for 855gm > static const struct intel_device_info intel_i915g_info = { > - .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, > + .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, .num_pipes = 1, 2 pipes for 915g > static const struct intel_device_info intel_i945g_info = { > - .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, > + .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, .num_pipes = 1, 2 pipes for 945g > static const struct intel_device_info intel_i965g_info = { > - .gen = 4, .is_broadwater = 1, > + .gen = 4, .is_broadwater = 1, .num_pipes = 1, 2 pipes for 965g > > static const struct intel_device_info intel_g33_info = { > - .gen = 3, .is_g33 = 1, > + .gen = 3, .is_g33 = 1, .num_pipes = 1, 2 pipes for g33 > .need_gfx_hws = 1, .has_hotplug = 1, > .has_overlay = 1, > }; > > static const struct intel_device_info intel_g45_info = { > - .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, > + .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, .num_pipes = 1, 2 pipes for g45 > static const struct intel_device_info intel_ironlake_d_info = { > - .gen = 5, > + .gen = 5, .num_pipes = 1, 2 pipes for ilk-d > static const struct intel_device_info intel_sandybridge_d_info = { > - .gen = 6, > + .gen = 6, .num_pipes = 1, 2 pipes for snb-d > static const struct intel_device_info intel_valleyview_d_info = { > - .gen = 7, > + .gen = 7, .num_pipes = 3, 2? > @@ -341,6 +341,7 @@ struct drm_i915_gt_funcs { > > struct intel_device_info { > u32 display_mmio_offset; > + u8 num_pipes:3; A hole! > u8 gen; > u8 is_mobile:1; > u8 is_i85x:1; > @@ -912,7 +913,6 @@ typedef struct drm_i915_private { > struct work_struct hotplug_work; > bool enable_hotplug_processing; > > - int num_pipe; > int num_pch_pll; > > unsigned long cfb_size; > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 2139714..b860f0b 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -254,10 +254,9 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, > struct timeval *vblank_time, > unsigned flags) > { > - struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_crtc *crtc; > > - if (pipe < 0 || pipe >= dev_priv->num_pipe) { > + if (pipe < 0 || pipe >= INTEL_INFO(dev)->num_pipes) { num_pipes is unsigned so pipe<0 is a redundant compare > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 502cb28..23379e7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2335,10 +2335,10 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > return 0; > } > > - if(intel_crtc->plane > dev_priv->num_pipe) { > + if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) { >= ? -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/9] drm/i915: Move num_pipes to intel info 2013-03-13 18:31 ` Chris Wilson @ 2013-03-13 18:36 ` Ben Widawsky 0 siblings, 0 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:36 UTC (permalink / raw) To: Chris Wilson, intel-gfx, Daniel Vetter, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 06:31:16PM +0000, Chris Wilson wrote: > On Wed, Mar 13, 2013 at 11:21:00AM -0700, Ben Widawsky wrote: > > Requested by Daniel. > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_dma.c | 9 +------ > > drivers/gpu/drm/i915/i915_drv.c | 48 ++++++++++++++++++------------------ > > drivers/gpu/drm/i915/i915_drv.h | 4 +-- > > drivers/gpu/drm/i915/i915_irq.c | 3 +-- > > drivers/gpu/drm/i915/intel_display.c | 19 +++++++------- > > drivers/gpu/drm/i915/intel_fb.c | 2 +- > > drivers/gpu/drm/i915/intel_panel.c | 2 +- > > 7 files changed, 40 insertions(+), 47 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > index e16099b..ebcfe2e 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -1630,14 +1630,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > > mutex_init(&dev_priv->rps.hw_lock); > > mutex_init(&dev_priv->modeset_restore_lock); > > > > - if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) > > - dev_priv->num_pipe = 3; > > - else if (IS_MOBILE(dev) || !IS_GEN2(dev)) > > - dev_priv->num_pipe = 2; > > - else > > - dev_priv->num_pipe = 1; > > - > > - ret = drm_vblank_init(dev, dev_priv->num_pipe); > > + ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes); > > if (ret) > > goto out_gem_unload; > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 1ebed96..0849651 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -136,74 +136,74 @@ extern int intel_agp_enabled; > > .driver_data = (unsigned long) info } > > > > static const struct intel_device_info intel_i830_info = { > > - .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, > > + .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 1, > 2 pipes for 830gm > > > static const struct intel_device_info intel_i85x_info = { > > - .gen = 2, .is_i85x = 1, .is_mobile = 1, > > + .gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 1, > 2 pipes for 855gm > > > static const struct intel_device_info intel_i915g_info = { > > - .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, > > + .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, .num_pipes = 1, > 2 pipes for 915g > > > static const struct intel_device_info intel_i945g_info = { > > - .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, > > + .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, .num_pipes = 1, > 2 pipes for 945g > > > static const struct intel_device_info intel_i965g_info = { > > - .gen = 4, .is_broadwater = 1, > > + .gen = 4, .is_broadwater = 1, .num_pipes = 1, > 2 pipes for 965g > > > > static const struct intel_device_info intel_g33_info = { > > - .gen = 3, .is_g33 = 1, > > + .gen = 3, .is_g33 = 1, .num_pipes = 1, > 2 pipes for g33 > > > .need_gfx_hws = 1, .has_hotplug = 1, > > .has_overlay = 1, > > }; > > > > static const struct intel_device_info intel_g45_info = { > > - .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, > > + .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, .num_pipes = 1, > 2 pipes for g45 > > > static const struct intel_device_info intel_ironlake_d_info = { > > - .gen = 5, > > + .gen = 5, .num_pipes = 1, > 2 pipes for ilk-d > > > static const struct intel_device_info intel_sandybridge_d_info = { > > - .gen = 6, > > + .gen = 6, .num_pipes = 1, > 2 pipes for snb-d > > > static const struct intel_device_info intel_valleyview_d_info = { > > - .gen = 7, > > + .gen = 7, .num_pipes = 3, > 2? > > > @@ -341,6 +341,7 @@ struct drm_i915_gt_funcs { > > > > struct intel_device_info { > > u32 display_mmio_offset; > > + u8 num_pipes:3; > A hole! > > > u8 gen; > > u8 is_mobile:1; > > u8 is_i85x:1; > > @@ -912,7 +913,6 @@ typedef struct drm_i915_private { > > struct work_struct hotplug_work; > > bool enable_hotplug_processing; > > > > - int num_pipe; > > int num_pch_pll; > > > > unsigned long cfb_size; > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index 2139714..b860f0b 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -254,10 +254,9 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, > > struct timeval *vblank_time, > > unsigned flags) > > { > > - struct drm_i915_private *dev_priv = dev->dev_private; > > struct drm_crtc *crtc; > > > > - if (pipe < 0 || pipe >= dev_priv->num_pipe) { > > + if (pipe < 0 || pipe >= INTEL_INFO(dev)->num_pipes) { > num_pipes is unsigned so pipe<0 is a redundant compare > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 502cb28..23379e7 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2335,10 +2335,10 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > > return 0; > > } > > > > - if(intel_crtc->plane > dev_priv->num_pipe) { > > + if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) { > > >= ? > > -- > Chris Wilson, Intel Open Source Technology Centre Thanks for detailed review. Sloppy code is a result of working while at a conference. I'll apply these fixes locally presently. -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 01/10] [v2] drm/i915: Move num_pipes to intel info 2013-03-13 18:21 ` [PATCH 1/9] drm/i915: Move num_pipes to intel info Ben Widawsky 2013-03-13 18:31 ` Chris Wilson @ 2013-03-13 21:05 ` Ben Widawsky 2013-03-19 19:25 ` [PATCH 1/9] " Jesse Barnes 2 siblings, 0 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 21:05 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter, Ben Widawsky, Mcallister, Jeffrey Requested by Daniel. v2: Fix incorrect num_pipe settings. (Chris) Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_dma.c | 9 +------ drivers/gpu/drm/i915/i915_drv.c | 48 ++++++++++++++++++------------------ drivers/gpu/drm/i915/i915_drv.h | 4 +-- drivers/gpu/drm/i915/i915_irq.c | 3 +-- drivers/gpu/drm/i915/intel_display.c | 19 +++++++------- drivers/gpu/drm/i915/intel_fb.c | 2 +- drivers/gpu/drm/i915/intel_panel.c | 2 +- 7 files changed, 40 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index e16099b..ebcfe2e 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1630,14 +1630,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) mutex_init(&dev_priv->rps.hw_lock); mutex_init(&dev_priv->modeset_restore_lock); - if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) - dev_priv->num_pipe = 3; - else if (IS_MOBILE(dev) || !IS_GEN2(dev)) - dev_priv->num_pipe = 2; - else - dev_priv->num_pipe = 1; - - ret = drm_vblank_init(dev, dev_priv->num_pipe); + ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes); if (ret) goto out_gem_unload; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 1ebed96..8ff83e9 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -136,74 +136,74 @@ extern int intel_agp_enabled; .driver_data = (unsigned long) info } static const struct intel_device_info intel_i830_info = { - .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, + .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_845g_info = { - .gen = 2, + .gen = 2, .num_pipes = 1, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i85x_info = { - .gen = 2, .is_i85x = 1, .is_mobile = 1, + .gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 2, .cursor_needs_physical = 1, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i865g_info = { - .gen = 2, + .gen = 2, .num_pipes = 1, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i915g_info = { - .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, + .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, .num_pipes = 2, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i915gm_info = { - .gen = 3, .is_mobile = 1, + .gen = 3, .is_mobile = 1, .num_pipes = 2, .cursor_needs_physical = 1, .has_overlay = 1, .overlay_needs_physical = 1, .supports_tv = 1, }; static const struct intel_device_info intel_i945g_info = { - .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, + .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, .num_pipes = 2, .has_overlay = 1, .overlay_needs_physical = 1, }; static const struct intel_device_info intel_i945gm_info = { - .gen = 3, .is_i945gm = 1, .is_mobile = 1, + .gen = 3, .is_i945gm = 1, .is_mobile = 1, .num_pipes = 2, .has_hotplug = 1, .cursor_needs_physical = 1, .has_overlay = 1, .overlay_needs_physical = 1, .supports_tv = 1, }; static const struct intel_device_info intel_i965g_info = { - .gen = 4, .is_broadwater = 1, + .gen = 4, .is_broadwater = 1, .num_pipes = 2, .has_hotplug = 1, .has_overlay = 1, }; static const struct intel_device_info intel_i965gm_info = { - .gen = 4, .is_crestline = 1, + .gen = 4, .is_crestline = 1, .num_pipes = 2, .is_mobile = 1, .has_fbc = 1, .has_hotplug = 1, .has_overlay = 1, .supports_tv = 1, }; static const struct intel_device_info intel_g33_info = { - .gen = 3, .is_g33 = 1, + .gen = 3, .is_g33 = 1, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_overlay = 1, }; static const struct intel_device_info intel_g45_info = { - .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, + .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, .num_pipes = 2, .has_pipe_cxsr = 1, .has_hotplug = 1, .has_bsd_ring = 1, }; static const struct intel_device_info intel_gm45_info = { - .gen = 4, .is_g4x = 1, + .gen = 4, .is_g4x = 1, .num_pipes = 2, .is_mobile = 1, .need_gfx_hws = 1, .has_fbc = 1, .has_pipe_cxsr = 1, .has_hotplug = 1, .supports_tv = 1, @@ -211,26 +211,26 @@ static const struct intel_device_info intel_gm45_info = { }; static const struct intel_device_info intel_pineview_info = { - .gen = 3, .is_g33 = 1, .is_pineview = 1, .is_mobile = 1, + .gen = 3, .is_g33 = 1, .is_pineview = 1, .is_mobile = 1, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_overlay = 1, }; static const struct intel_device_info intel_ironlake_d_info = { - .gen = 5, + .gen = 5, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, }; static const struct intel_device_info intel_ironlake_m_info = { - .gen = 5, .is_mobile = 1, + .gen = 5, .is_mobile = 1, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 1, .has_bsd_ring = 1, }; static const struct intel_device_info intel_sandybridge_d_info = { - .gen = 6, + .gen = 6, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, .has_blt_ring = 1, @@ -239,7 +239,7 @@ static const struct intel_device_info intel_sandybridge_d_info = { }; static const struct intel_device_info intel_sandybridge_m_info = { - .gen = 6, .is_mobile = 1, + .gen = 6, .is_mobile = 1, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 1, .has_bsd_ring = 1, @@ -249,7 +249,7 @@ static const struct intel_device_info intel_sandybridge_m_info = { }; static const struct intel_device_info intel_ivybridge_d_info = { - .is_ivybridge = 1, .gen = 7, + .is_ivybridge = 1, .gen = 7, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, .has_blt_ring = 1, @@ -258,7 +258,7 @@ static const struct intel_device_info intel_ivybridge_d_info = { }; static const struct intel_device_info intel_ivybridge_m_info = { - .is_ivybridge = 1, .gen = 7, .is_mobile = 1, + .is_ivybridge = 1, .gen = 7, .is_mobile = 1, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 0, /* FBC is not enabled on Ivybridge mobile yet */ .has_bsd_ring = 1, @@ -268,7 +268,7 @@ static const struct intel_device_info intel_ivybridge_m_info = { }; static const struct intel_device_info intel_valleyview_m_info = { - .gen = 7, .is_mobile = 1, + .gen = 7, .is_mobile = 1, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 0, .has_bsd_ring = 1, @@ -279,7 +279,7 @@ static const struct intel_device_info intel_valleyview_m_info = { }; static const struct intel_device_info intel_valleyview_d_info = { - .gen = 7, + .gen = 7, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, .has_fbc = 0, .has_bsd_ring = 1, @@ -290,7 +290,7 @@ static const struct intel_device_info intel_valleyview_d_info = { }; static const struct intel_device_info intel_haswell_d_info = { - .is_haswell = 1, .gen = 7, + .is_haswell = 1, .gen = 7, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, .has_blt_ring = 1, @@ -299,7 +299,7 @@ static const struct intel_device_info intel_haswell_d_info = { }; static const struct intel_device_info intel_haswell_m_info = { - .is_haswell = 1, .gen = 7, .is_mobile = 1, + .is_haswell = 1, .gen = 7, .is_mobile = 1, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, .has_bsd_ring = 1, .has_blt_ring = 1, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ca6b215..587dca0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -93,7 +93,7 @@ enum port { I915_GEM_DOMAIN_INSTRUCTION | \ I915_GEM_DOMAIN_VERTEX) -#define for_each_pipe(p) for ((p) = 0; (p) < dev_priv->num_pipe; (p)++) +#define for_each_pipe(p) for ((p) = 0; (p) < INTEL_INFO(dev)->num_pipes; (p)++) #define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \ list_for_each_entry((intel_encoder), &(dev)->mode_config.encoder_list, base.head) \ @@ -341,6 +341,7 @@ struct drm_i915_gt_funcs { struct intel_device_info { u32 display_mmio_offset; + u8 num_pipes:3; u8 gen; u8 is_mobile:1; u8 is_i85x:1; @@ -912,7 +913,6 @@ typedef struct drm_i915_private { struct work_struct hotplug_work; bool enable_hotplug_processing; - int num_pipe; int num_pch_pll; unsigned long cfb_size; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2139714..b860f0b 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -254,10 +254,9 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, struct timeval *vblank_time, unsigned flags) { - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc; - if (pipe < 0 || pipe >= dev_priv->num_pipe) { + if (pipe < 0 || pipe >= INTEL_INFO(dev)->num_pipes) { DRM_ERROR("Invalid crtc %d\n", pipe); return -EINVAL; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 502cb28..23379e7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2335,10 +2335,10 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, return 0; } - if(intel_crtc->plane > dev_priv->num_pipe) { + if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) { DRM_ERROR("no plane for crtc: plane %d, num_pipes %d\n", intel_crtc->plane, - dev_priv->num_pipe); + INTEL_INFO(dev)->num_pipes); return -EINVAL; } @@ -5359,7 +5359,7 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc) return false; } - if (dev_priv->num_pipe == 2) + if (INTEL_INFO(dev)->num_pipes == 2) return true; switch (intel_crtc->pipe) { @@ -8870,9 +8870,10 @@ void intel_modeset_init(struct drm_device *dev) dev->mode_config.fb_base = dev_priv->gtt.mappable_base; DRM_DEBUG_KMS("%d display pipe%s available.\n", - dev_priv->num_pipe, dev_priv->num_pipe > 1 ? "s" : ""); + INTEL_INFO(dev)->num_pipes, + INTEL_INFO(dev)->num_pipes > 1 ? "s" : ""); - for (i = 0; i < dev_priv->num_pipe; i++) { + for (i = 0; i < INTEL_INFO(dev)->num_pipes; i++) { intel_crtc_init(dev, i); ret = intel_plane_init(dev, i); if (ret) @@ -8929,10 +8930,11 @@ static void intel_enable_pipe_a(struct drm_device *dev) static bool intel_check_plane_mapping(struct intel_crtc *crtc) { - struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; u32 reg, val; - if (dev_priv->num_pipe == 1) + if (INTEL_INFO(dev)->num_pipes == 1) return true; reg = DSPCNTR(!crtc->plane); @@ -9366,10 +9368,9 @@ intel_display_print_error_state(struct seq_file *m, struct drm_device *dev, struct intel_display_error_state *error) { - drm_i915_private_t *dev_priv = dev->dev_private; int i; - seq_printf(m, "Num Pipes: %d\n", dev_priv->num_pipe); + seq_printf(m, "Num Pipes: %d\n", INTEL_INFO(dev)->num_pipes); for_each_pipe(i) { seq_printf(m, "Pipe [%d]:\n", i); seq_printf(m, " CONF: %08x\n", error->pipe[i].conf); diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c index 953ee73..dcdb1d3 100644 --- a/drivers/gpu/drm/i915/intel_fb.c +++ b/drivers/gpu/drm/i915/intel_fb.c @@ -240,7 +240,7 @@ int intel_fbdev_init(struct drm_device *dev) ifbdev->helper.funcs = &intel_fb_helper_funcs; ret = drm_fb_helper_init(dev, &ifbdev->helper, - dev_priv->num_pipe, + INTEL_INFO(dev)->num_pipes, INTELFB_CONN_LIMIT); if (ret) { kfree(ifbdev); diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index a3730e0..f1530f4 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -338,7 +338,7 @@ void intel_panel_enable_backlight(struct drm_device *dev, if (tmp & BLM_PWM_ENABLE) goto set_level; - if (dev_priv->num_pipe == 3) + if (INTEL_INFO(dev)->num_pipes == 3) tmp &= ~BLM_PIPE_SELECT_IVB; else tmp &= ~BLM_PIPE_SELECT; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 1/9] drm/i915: Move num_pipes to intel info 2013-03-13 18:21 ` [PATCH 1/9] drm/i915: Move num_pipes to intel info Ben Widawsky 2013-03-13 18:31 ` Chris Wilson 2013-03-13 21:05 ` [PATCH 01/10] [v2] " Ben Widawsky @ 2013-03-19 19:25 ` Jesse Barnes 2013-03-19 23:06 ` Daniel Vetter 2 siblings, 1 reply; 40+ messages in thread From: Jesse Barnes @ 2013-03-19 19:25 UTC (permalink / raw) To: Ben Widawsky; +Cc: Daniel Vetter, intel-gfx, Mcallister, Jeffrey On Wed, 13 Mar 2013 11:21:00 -0700 Ben Widawsky <ben@bwidawsk.net> wrote: > Requested by Daniel. > static const struct intel_device_info intel_i85x_info = { > - .gen = 2, .is_i85x = 1, .is_mobile = 1, > + .gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 1, > .cursor_needs_physical = 1, > .has_overlay = 1, .overlay_needs_physical = 1, > }; Most of these are wrong. All mobile devices have 2 pipes. All gen3+ devices have 2 or 3 pipes. Check the original code: - if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) - dev_priv->num_pipe = 3; - else if (IS_MOBILE(dev) || !IS_GEN2(dev)) - dev_priv->num_pipe = 2; - else - dev_priv->num_pipe = 1; > static const struct intel_device_info intel_valleyview_d_info = { > - .gen = 7, > + .gen = 7, .num_pipes = 3, > .need_gfx_hws = 1, .has_hotplug = 1, > .has_fbc = 0, > .has_bsd_ring = 1, > @@ -290,7 +290,7 @@ static const struct intel_device_info intel_valleyview_d_info = { > }; VLV only has 2 pipes on all variants. Looks good otherwise. With the pipe count fixes: Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/9] drm/i915: Move num_pipes to intel info 2013-03-19 19:25 ` [PATCH 1/9] " Jesse Barnes @ 2013-03-19 23:06 ` Daniel Vetter 0 siblings, 0 replies; 40+ messages in thread From: Daniel Vetter @ 2013-03-19 23:06 UTC (permalink / raw) To: Jesse Barnes; +Cc: Daniel Vetter, Ben Widawsky, intel-gfx, Mcallister, Jeffrey On Tue, Mar 19, 2013 at 12:25:23PM -0700, Jesse Barnes wrote: > On Wed, 13 Mar 2013 11:21:00 -0700 > Ben Widawsky <ben@bwidawsk.net> wrote: > > > Requested by Daniel. > > static const struct intel_device_info intel_i85x_info = { > > - .gen = 2, .is_i85x = 1, .is_mobile = 1, > > + .gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 1, > > .cursor_needs_physical = 1, > > .has_overlay = 1, .overlay_needs_physical = 1, > > }; > > Most of these are wrong. All mobile devices have 2 pipes. All gen3+ > devices have 2 or 3 pipes. Check the original code: > > - if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) > - dev_priv->num_pipe = 3; > - else if (IS_MOBILE(dev) || !IS_GEN2(dev)) > - dev_priv->num_pipe = 2; > - else > - dev_priv->num_pipe = 1; > > > static const struct intel_device_info intel_valleyview_d_info = { > > - .gen = 7, > > + .gen = 7, .num_pipes = 3, > > .need_gfx_hws = 1, .has_hotplug = 1, > > .has_fbc = 0, > > .has_bsd_ring = 1, > > @@ -290,7 +290,7 @@ static const struct intel_device_info intel_valleyview_d_info = { > > }; > > VLV only has 2 pipes on all variants. > > Looks good otherwise. With the pipe count fixes: > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> Merged v2 and smashed your r-b on top. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/9] drm/i915: Support PCH no display 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky 2013-03-13 18:21 ` [PATCH 1/9] drm/i915: Move num_pipes to intel info Ben Widawsky @ 2013-03-13 18:21 ` Ben Widawsky 2013-03-17 21:13 ` Daniel Vetter 2013-03-13 18:21 ` [PATCH 3/9] drm/i915: PCH_NOP Ben Widawsky ` (7 subsequent siblings) 9 siblings, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:21 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey GEN supports a fusing option which subtracts the PCH display (making the CPU display also useless). In this configuration MMIO which gets decoded to a certain range will hang the CPU. For us, this is sort of the equivalent of having no pipes, and we can easily modify some code to not do certain things with no pipes. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_dma.c | 20 ++++++++++++++------ drivers/gpu/drm/i915/intel_crt.c | 3 +++ drivers/gpu/drm/i915/intel_display.c | 10 ++++++++-- drivers/gpu/drm/i915/intel_fb.c | 3 +++ drivers/gpu/drm/i915/intel_overlay.c | 3 +++ 5 files changed, 31 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index ebcfe2e..d925504 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1322,6 +1322,10 @@ static int i915_load_modeset_init(struct drm_device *dev) /* Always safe in the mode setting case. */ /* FIXME: do pre/post-mode set stuff in core KMS code */ dev->vblank_disable_allowed = 1; + if (INTEL_INFO(dev)->num_pipes == 0) { + dev_priv->mm.suspended = 0; + return 0; + } ret = intel_fbdev_init(dev); if (ret) @@ -1630,9 +1634,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) mutex_init(&dev_priv->rps.hw_lock); mutex_init(&dev_priv->modeset_restore_lock); - ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes); - if (ret) - goto out_gem_unload; + if (INTEL_INFO(dev)->num_pipes) { + ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes); + if (ret) + goto out_gem_unload; + } /* Start out suspended */ dev_priv->mm.suspended = 1; @@ -1647,9 +1653,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) i915_setup_sysfs(dev); - /* Must be done after probing outputs */ - intel_opregion_init(dev); - acpi_video_register(); + if (INTEL_INFO(dev)->num_pipes) { + /* Must be done after probing outputs */ + intel_opregion_init(dev); + acpi_video_register(); + } if (IS_GEN5(dev)) intel_gpu_ips_init(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index cfc9687..e794c6c 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -736,6 +736,9 @@ void intel_crt_init(struct drm_device *dev) if (dmi_check_system(intel_no_crt)) return; + if (INTEL_INFO(dev)->num_pipes == 0) + return; + crt = kzalloc(sizeof(struct intel_crt), GFP_KERNEL); if (!crt) return; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 23379e7..d6dbffd 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7682,6 +7682,9 @@ intel_modeset_check_state(struct drm_device *dev) struct intel_encoder *encoder; struct intel_connector *connector; + if (INTEL_INFO(dev)->num_pipes == 0) + return; + list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) { /* This also checks the encoder/connector hw state with the @@ -8326,7 +8329,9 @@ static void intel_setup_outputs(struct drm_device *dev) if (!(HAS_DDI(dev) && (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES))) intel_crt_init(dev); - if (HAS_DDI(dev)) { + if (INTEL_INFO(dev)->num_pipes == 0) { + DRM_DEBUG_KMS("Skipping output detection\n"); + } else if (HAS_DDI(dev)) { int found; /* Haswell uses DDI functions to detect digital outputs */ @@ -8443,7 +8448,8 @@ static void intel_setup_outputs(struct drm_device *dev) intel_init_pch_refclk(dev); - drm_helper_move_panel_connectors_to_head(dev); + if (INTEL_INFO(dev)->num_pipes) + drm_helper_move_panel_connectors_to_head(dev); } static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb) diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c index dcdb1d3..5f825c2 100644 --- a/drivers/gpu/drm/i915/intel_fb.c +++ b/drivers/gpu/drm/i915/intel_fb.c @@ -295,6 +295,9 @@ void intel_fb_restore_mode(struct drm_device *dev) struct drm_mode_config *config = &dev->mode_config; struct drm_plane *plane; + if (!INTEL_INFO(dev)->num_pipes) + return; + drm_modeset_lock_all(dev); ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper); diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c index 67a2501..e7fbf2e 100644 --- a/drivers/gpu/drm/i915/intel_overlay.c +++ b/drivers/gpu/drm/i915/intel_overlay.c @@ -1323,6 +1323,9 @@ void intel_setup_overlay(struct drm_device *dev) if (!HAS_OVERLAY(dev)) return; + if (INTEL_INFO(dev)->num_pipes == 0) + return; + overlay = kzalloc(sizeof(struct intel_overlay), GFP_KERNEL); if (!overlay) return; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 2/9] drm/i915: Support PCH no display 2013-03-13 18:21 ` [PATCH 2/9] drm/i915: Support PCH no display Ben Widawsky @ 2013-03-17 21:13 ` Daniel Vetter 0 siblings, 0 replies; 40+ messages in thread From: Daniel Vetter @ 2013-03-17 21:13 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 11:21:01AM -0700, Ben Widawsky wrote: > GEN supports a fusing option which subtracts the PCH display (making the > CPU display also useless). In this configuration MMIO which gets decoded > to a certain range will hang the CPU. > > For us, this is sort of the equivalent of having no pipes, and we can > easily modify some code to not do certain things with no pipes. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_dma.c | 20 ++++++++++++++------ > drivers/gpu/drm/i915/intel_crt.c | 3 +++ > drivers/gpu/drm/i915/intel_display.c | 10 ++++++++-- > drivers/gpu/drm/i915/intel_fb.c | 3 +++ > drivers/gpu/drm/i915/intel_overlay.c | 3 +++ > 5 files changed, 31 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index ebcfe2e..d925504 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1322,6 +1322,10 @@ static int i915_load_modeset_init(struct drm_device *dev) > /* Always safe in the mode setting case. */ > /* FIXME: do pre/post-mode set stuff in core KMS code */ > dev->vblank_disable_allowed = 1; > + if (INTEL_INFO(dev)->num_pipes == 0) { > + dev_priv->mm.suspended = 0; > + return 0; > + } > > ret = intel_fbdev_init(dev); > if (ret) > @@ -1630,9 +1634,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > mutex_init(&dev_priv->rps.hw_lock); > mutex_init(&dev_priv->modeset_restore_lock); > > - ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes); > - if (ret) > - goto out_gem_unload; > + if (INTEL_INFO(dev)->num_pipes) { > + ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes); > + if (ret) > + goto out_gem_unload; > + } > > /* Start out suspended */ > dev_priv->mm.suspended = 1; > @@ -1647,9 +1653,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > > i915_setup_sysfs(dev); > > - /* Must be done after probing outputs */ > - intel_opregion_init(dev); > - acpi_video_register(); > + if (INTEL_INFO(dev)->num_pipes) { > + /* Must be done after probing outputs */ > + intel_opregion_init(dev); > + acpi_video_register(); > + } > > if (IS_GEN5(dev)) > intel_gpu_ips_init(dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c > index cfc9687..e794c6c 100644 > --- a/drivers/gpu/drm/i915/intel_crt.c > +++ b/drivers/gpu/drm/i915/intel_crt.c > @@ -736,6 +736,9 @@ void intel_crt_init(struct drm_device *dev) > if (dmi_check_system(intel_no_crt)) > return; > > + if (INTEL_INFO(dev)->num_pipes == 0) > + return; Imo better to move this up in the callchain since we should never ever bother to probe connectors with no pipes. > + > crt = kzalloc(sizeof(struct intel_crt), GFP_KERNEL); > if (!crt) > return; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 23379e7..d6dbffd 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7682,6 +7682,9 @@ intel_modeset_check_state(struct drm_device *dev) > struct intel_encoder *encoder; > struct intel_connector *connector; > > + if (INTEL_INFO(dev)->num_pipes == 0) > + return; Presuming our works correctly this should never be called if there's no pipes. Could be that the lid handler is offending us, but that one shouldn't be registered without a panel. Can you please check whether we really need this? > + > list_for_each_entry(connector, &dev->mode_config.connector_list, > base.head) { > /* This also checks the encoder/connector hw state with the > @@ -8326,7 +8329,9 @@ static void intel_setup_outputs(struct drm_device *dev) > if (!(HAS_DDI(dev) && (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES))) > intel_crt_init(dev); > > - if (HAS_DDI(dev)) { > + if (INTEL_INFO(dev)->num_pipes == 0) { > + DRM_DEBUG_KMS("Skipping output detection\n"); Move this up and add an early return, then all the other output checks should be redundant. There's some other stuff called from intel_modeset_init which might need to be guarded. > + } else if (HAS_DDI(dev)) { > int found; > > /* Haswell uses DDI functions to detect digital outputs */ > @@ -8443,7 +8448,8 @@ static void intel_setup_outputs(struct drm_device *dev) > > intel_init_pch_refclk(dev); If you drop everything in setup_outputs you also don't have to deal with the pch_refclock stuff. Cheers, Daniel > - drm_helper_move_panel_connectors_to_head(dev); > + if (INTEL_INFO(dev)->num_pipes) > + drm_helper_move_panel_connectors_to_head(dev); > } > > static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb) > diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c > index dcdb1d3..5f825c2 100644 > --- a/drivers/gpu/drm/i915/intel_fb.c > +++ b/drivers/gpu/drm/i915/intel_fb.c > @@ -295,6 +295,9 @@ void intel_fb_restore_mode(struct drm_device *dev) > struct drm_mode_config *config = &dev->mode_config; > struct drm_plane *plane; > > + if (!INTEL_INFO(dev)->num_pipes) > + return; > + > drm_modeset_lock_all(dev); > > ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper); > diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c > index 67a2501..e7fbf2e 100644 > --- a/drivers/gpu/drm/i915/intel_overlay.c > +++ b/drivers/gpu/drm/i915/intel_overlay.c > @@ -1323,6 +1323,9 @@ void intel_setup_overlay(struct drm_device *dev) > if (!HAS_OVERLAY(dev)) > return; > > + if (INTEL_INFO(dev)->num_pipes == 0) > + return; > + > overlay = kzalloc(sizeof(struct intel_overlay), GFP_KERNEL); > if (!overlay) > return; > -- > 1.8.1.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 3/9] drm/i915: PCH_NOP 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky 2013-03-13 18:21 ` [PATCH 1/9] drm/i915: Move num_pipes to intel info Ben Widawsky 2013-03-13 18:21 ` [PATCH 2/9] drm/i915: Support PCH no display Ben Widawsky @ 2013-03-13 18:21 ` Ben Widawsky 2013-03-17 21:05 ` Daniel Vetter 2013-03-13 18:21 ` [PATCH 4/9] drm/i915: Don't touch South display when PCH_NOP Ben Widawsky ` (6 subsequent siblings) 9 siblings, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:21 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey Given certain fusing options discussed in the previous patch, it's possible to end up with platforms that normally have PCH but that PCH doesn't actually exist. In many cases, this is easily remedied with setting 0 pipes. This covers the other corners. Requiring this is a symptom of improper code splitting (using HAS_PCH_SPLIT instead of proper GEN checking, basically). I do not want to fix this. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_display.c | 4 +++- drivers/gpu/drm/i915/intel_lvds.c | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 587dca0..ceed199 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -461,6 +461,7 @@ enum intel_pch { PCH_IBX, /* Ibexpeak PCH */ PCH_CPT, /* Cougarpoint PCH */ PCH_LPT, /* Lynxpoint PCH */ + PCH_NOP, }; enum intel_sbi_destination { @@ -1351,6 +1352,7 @@ struct drm_i915_file_private { #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_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) #define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE) #define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)->has_force_wake) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d6dbffd..1f0624e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5085,7 +5085,9 @@ static void lpt_init_pch_refclk(struct drm_device *dev) */ void intel_init_pch_refclk(struct drm_device *dev) { - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) + if (HAS_PCH_NOP(dev)) + return; + else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) ironlake_init_pch_refclk(dev); else if (HAS_PCH_LPT(dev)) lpt_init_pch_refclk(dev); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 6b24fc5..613ac43 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -1018,6 +1018,10 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder) static bool intel_lvds_supported(struct drm_device *dev) { + + if (HAS_PCH_NOP(dev)) + return false; + /* With the introduction of the PCH we gained a dedicated * LVDS presence pin, use it. */ if (HAS_PCH_SPLIT(dev)) -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 3/9] drm/i915: PCH_NOP 2013-03-13 18:21 ` [PATCH 3/9] drm/i915: PCH_NOP Ben Widawsky @ 2013-03-17 21:05 ` Daniel Vetter 2013-03-17 21:14 ` Daniel Vetter 0 siblings, 1 reply; 40+ messages in thread From: Daniel Vetter @ 2013-03-17 21:05 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 11:21:02AM -0700, Ben Widawsky wrote: > Given certain fusing options discussed in the previous patch, it's > possible to end up with platforms that normally have PCH but that PCH > doesn't actually exist. In many cases, this is easily remedied with > setting 0 pipes. This covers the other corners. > > Requiring this is a symptom of improper code splitting (using > HAS_PCH_SPLIT instead of proper GEN checking, basically). I do not want > to fix this. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_display.c | 4 +++- > drivers/gpu/drm/i915/intel_lvds.c | 4 ++++ > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 587dca0..ceed199 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -461,6 +461,7 @@ enum intel_pch { > PCH_IBX, /* Ibexpeak PCH */ > PCH_CPT, /* Cougarpoint PCH */ > PCH_LPT, /* Lynxpoint PCH */ > + PCH_NOP, > }; > > enum intel_sbi_destination { > @@ -1351,6 +1352,7 @@ struct drm_i915_file_private { > #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_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) > #define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE) > > #define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)->has_force_wake) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index d6dbffd..1f0624e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5085,7 +5085,9 @@ static void lpt_init_pch_refclk(struct drm_device *dev) > */ > void intel_init_pch_refclk(struct drm_device *dev) > { Since this function here only deals with display refclocks I think a num_pipes == 0 check should equally work. And also makes it clear what's exactly going on. > - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) > + if (HAS_PCH_NOP(dev)) > + return; > + else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) > ironlake_init_pch_refclk(dev); > else if (HAS_PCH_LPT(dev)) > lpt_init_pch_refclk(dev); > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 6b24fc5..613ac43 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -1018,6 +1018,10 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder) > > static bool intel_lvds_supported(struct drm_device *dev) > { > + > + if (HAS_PCH_NOP(dev)) > + return false; This hunk here looks superflous, imo we should stop any and all output probing much earlier in the callchain. -Daniel > + > /* With the introduction of the PCH we gained a dedicated > * LVDS presence pin, use it. */ > if (HAS_PCH_SPLIT(dev)) > -- > 1.8.1.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 3/9] drm/i915: PCH_NOP 2013-03-17 21:05 ` Daniel Vetter @ 2013-03-17 21:14 ` Daniel Vetter 0 siblings, 0 replies; 40+ messages in thread From: Daniel Vetter @ 2013-03-17 21:14 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Sun, Mar 17, 2013 at 10:05:41PM +0100, Daniel Vetter wrote: > On Wed, Mar 13, 2013 at 11:21:02AM -0700, Ben Widawsky wrote: > > Given certain fusing options discussed in the previous patch, it's > > possible to end up with platforms that normally have PCH but that PCH > > doesn't actually exist. In many cases, this is easily remedied with > > setting 0 pipes. This covers the other corners. > > > > Requiring this is a symptom of improper code splitting (using > > HAS_PCH_SPLIT instead of proper GEN checking, basically). I do not want > > to fix this. > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_display.c | 4 +++- > > drivers/gpu/drm/i915/intel_lvds.c | 4 ++++ > > 3 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 587dca0..ceed199 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -461,6 +461,7 @@ enum intel_pch { > > PCH_IBX, /* Ibexpeak PCH */ > > PCH_CPT, /* Cougarpoint PCH */ > > PCH_LPT, /* Lynxpoint PCH */ > > + PCH_NOP, > > }; > > > > enum intel_sbi_destination { > > @@ -1351,6 +1352,7 @@ struct drm_i915_file_private { > > #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_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) > > #define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE) > > > > #define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)->has_force_wake) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index d6dbffd..1f0624e 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5085,7 +5085,9 @@ static void lpt_init_pch_refclk(struct drm_device *dev) > > */ > > void intel_init_pch_refclk(struct drm_device *dev) > > { > > Since this function here only deals with display refclocks I think a > num_pipes == 0 check should equally work. And also makes it clear what's > exactly going on. See my comment on the first patch, I think if we never run the code in setup_outputs we should be fine here already and don't need anything more. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 4/9] drm/i915: Don't touch South display when PCH_NOP 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky ` (2 preceding siblings ...) 2013-03-13 18:21 ` [PATCH 3/9] drm/i915: PCH_NOP Ben Widawsky @ 2013-03-13 18:21 ` Ben Widawsky 2013-03-14 11:44 ` Jani Nikula 2013-03-13 18:21 ` [PATCH 5/9] drm/i915: Don't initialize watermark stuff with PCH_NOP Ben Widawsky ` (5 subsequent siblings) 9 siblings, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:21 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey Interrupts, clock gating, and gmbus are all within the, "this will hang the CPU" range when we have PCH_NOP. There is a bit of a hack in init clock gating. We want to do most of the block gating, but the part we skip will hang the system. It could probably be abstracted a bit better, but I don't feel it's too unsightly. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++++++------ drivers/gpu/drm/i915/intel_bios.c | 3 +++ drivers/gpu/drm/i915/intel_i2c.c | 4 +++- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b860f0b..ba8ada3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) } } - /* check event from PCH */ - if (de_iir & DE_PCH_EVENT_IVB) { - u32 pch_iir = I915_READ(SDEIIR); + if (HAS_PCH_NOP(dev)) { + /* check event from PCH */ + if (de_iir & DE_PCH_EVENT_IVB) { + u32 pch_iir = I915_READ(SDEIIR); - cpt_irq_handler(dev, pch_iir); + cpt_irq_handler(dev, pch_iir); - /* clear PCH hotplug event before clear CPU irq */ - I915_WRITE(SDEIIR, pch_iir); + /* clear PCH hotplug event before CPU irq */ + I915_WRITE(SDEIIR, pch_iir); + } } I915_WRITE(DEIIR, de_iir); @@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device *dev) I915_WRITE(GTIER, 0x0); POSTING_READ(GTIER); + if (HAS_PCH_NOP(dev)) + return; + /* south display irq */ I915_WRITE(SDEIMR, 0xffffffff); I915_WRITE(SDEIER, 0x0); @@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev) SDE_GMBUS_CPT | SDE_AUX_MASK_CPT; + if (HAS_PCH_NOP(dev)) + return; + I915_WRITE(SDEIIR, I915_READ(SDEIIR)); I915_WRITE(SDEIMR, ~mask); I915_WRITE(SDEIER, mask); diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 55ffba1..194df27 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev) struct bdb_header *bdb = NULL; u8 __iomem *bios = NULL; + if (HAS_PCH_NOP(dev)) + return -ENODEV; + init_vbt_defaults(dev_priv); /* XXX Should this validation be moved to intel_opregion.c? */ diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index acf8aec..fc19e49 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; int ret, i; - if (HAS_PCH_SPLIT(dev)) + if (HAS_PCH_NOP(dev)) + return 0; + else if (HAS_PCH_SPLIT(dev)) dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA; else if (IS_VALLEYVIEW(dev)) dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5479363..52203fd 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct drm_device *dev) snpcr |= GEN6_MBC_SNPCR_MED; I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr); - cpt_init_clock_gating(dev); + if (HAS_PCH_NOP(dev)) + cpt_init_clock_gating(dev); gen6_check_mch_setup(dev); } -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 4/9] drm/i915: Don't touch South display when PCH_NOP 2013-03-13 18:21 ` [PATCH 4/9] drm/i915: Don't touch South display when PCH_NOP Ben Widawsky @ 2013-03-14 11:44 ` Jani Nikula 2013-03-14 15:55 ` [PATCH 04/10] [v2] drm/i915: Don't touch South Display " Ben Widawsky 0 siblings, 1 reply; 40+ messages in thread From: Jani Nikula @ 2013-03-14 11:44 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey On Wed, 13 Mar 2013, Ben Widawsky <ben@bwidawsk.net> wrote: > Interrupts, clock gating, and gmbus are all within the, "this will hang > the CPU" range when we have PCH_NOP. > > There is a bit of a hack in init clock gating. We want to do most of the > block gating, but the part we skip will hang the system. It could > probably be abstracted a bit better, but I don't feel it's too > unsightly. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++++++------ > drivers/gpu/drm/i915/intel_bios.c | 3 +++ > drivers/gpu/drm/i915/intel_i2c.c | 4 +++- > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > 4 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index b860f0b..ba8ada3 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) > } > } > > - /* check event from PCH */ > - if (de_iir & DE_PCH_EVENT_IVB) { > - u32 pch_iir = I915_READ(SDEIIR); > + if (HAS_PCH_NOP(dev)) { !HAS_PCH_NOP() ? BR, Jani. > + /* check event from PCH */ > + if (de_iir & DE_PCH_EVENT_IVB) { > + u32 pch_iir = I915_READ(SDEIIR); > > - cpt_irq_handler(dev, pch_iir); > + cpt_irq_handler(dev, pch_iir); > > - /* clear PCH hotplug event before clear CPU irq */ > - I915_WRITE(SDEIIR, pch_iir); > + /* clear PCH hotplug event before CPU irq */ > + I915_WRITE(SDEIIR, pch_iir); > + } > } > > I915_WRITE(DEIIR, de_iir); > @@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device *dev) > I915_WRITE(GTIER, 0x0); > POSTING_READ(GTIER); > > + if (HAS_PCH_NOP(dev)) > + return; > + > /* south display irq */ > I915_WRITE(SDEIMR, 0xffffffff); > I915_WRITE(SDEIER, 0x0); > @@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev) > SDE_GMBUS_CPT | > SDE_AUX_MASK_CPT; > > + if (HAS_PCH_NOP(dev)) > + return; > + > I915_WRITE(SDEIIR, I915_READ(SDEIIR)); > I915_WRITE(SDEIMR, ~mask); > I915_WRITE(SDEIER, mask); > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 55ffba1..194df27 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev) > struct bdb_header *bdb = NULL; > u8 __iomem *bios = NULL; > > + if (HAS_PCH_NOP(dev)) > + return -ENODEV; > + > init_vbt_defaults(dev_priv); > > /* XXX Should this validation be moved to intel_opregion.c? */ > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c > index acf8aec..fc19e49 100644 > --- a/drivers/gpu/drm/i915/intel_i2c.c > +++ b/drivers/gpu/drm/i915/intel_i2c.c > @@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > int ret, i; > > - if (HAS_PCH_SPLIT(dev)) > + if (HAS_PCH_NOP(dev)) > + return 0; > + else if (HAS_PCH_SPLIT(dev)) > dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA; > else if (IS_VALLEYVIEW(dev)) > dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE; > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 5479363..52203fd 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct drm_device *dev) > snpcr |= GEN6_MBC_SNPCR_MED; > I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr); > > - cpt_init_clock_gating(dev); > + if (HAS_PCH_NOP(dev)) > + cpt_init_clock_gating(dev); > > gen6_check_mch_setup(dev); > } > -- > 1.8.1.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 04/10] [v2] drm/i915: Don't touch South Display when PCH_NOP 2013-03-14 11:44 ` Jani Nikula @ 2013-03-14 15:55 ` Ben Widawsky 2013-03-17 21:21 ` Daniel Vetter 0 siblings, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-14 15:55 UTC (permalink / raw) To: Intel GFX; +Cc: Ben Widawsky Interrupts, clock gating, and GMBUS are all within the, "this will hang the CPU" range when we have PCH_NOP. There is a bit of a hack in init clock gating. We want to do most of the clock gating, but the part we skip will hang the system. It could probably be abstracted a bit better, but I don't feel it's too unsightly. v2: Use inverse HAS_PCH_NOP check (Jani) Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++++++------ drivers/gpu/drm/i915/intel_bios.c | 3 +++ drivers/gpu/drm/i915/intel_i2c.c | 4 +++- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b860f0b..15b4668 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) } } - /* check event from PCH */ - if (de_iir & DE_PCH_EVENT_IVB) { - u32 pch_iir = I915_READ(SDEIIR); + if (!HAS_PCH_NOP(dev)) { + /* check event from PCH */ + if (de_iir & DE_PCH_EVENT_IVB) { + u32 pch_iir = I915_READ(SDEIIR); - cpt_irq_handler(dev, pch_iir); + cpt_irq_handler(dev, pch_iir); - /* clear PCH hotplug event before clear CPU irq */ - I915_WRITE(SDEIIR, pch_iir); + /* clear PCH hotplug event before CPU irq */ + I915_WRITE(SDEIIR, pch_iir); + } } I915_WRITE(DEIIR, de_iir); @@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device *dev) I915_WRITE(GTIER, 0x0); POSTING_READ(GTIER); + if (HAS_PCH_NOP(dev)) + return; + /* south display irq */ I915_WRITE(SDEIMR, 0xffffffff); I915_WRITE(SDEIER, 0x0); @@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev) SDE_GMBUS_CPT | SDE_AUX_MASK_CPT; + if (HAS_PCH_NOP(dev)) + return; + I915_WRITE(SDEIIR, I915_READ(SDEIIR)); I915_WRITE(SDEIMR, ~mask); I915_WRITE(SDEIER, mask); diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 55ffba1..194df27 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev) struct bdb_header *bdb = NULL; u8 __iomem *bios = NULL; + if (HAS_PCH_NOP(dev)) + return -ENODEV; + init_vbt_defaults(dev_priv); /* XXX Should this validation be moved to intel_opregion.c? */ diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index acf8aec..fc19e49 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; int ret, i; - if (HAS_PCH_SPLIT(dev)) + if (HAS_PCH_NOP(dev)) + return 0; + else if (HAS_PCH_SPLIT(dev)) dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA; else if (IS_VALLEYVIEW(dev)) dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5479363..52203fd 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct drm_device *dev) snpcr |= GEN6_MBC_SNPCR_MED; I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr); - cpt_init_clock_gating(dev); + if (HAS_PCH_NOP(dev)) + cpt_init_clock_gating(dev); gen6_check_mch_setup(dev); } -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 04/10] [v2] drm/i915: Don't touch South Display when PCH_NOP 2013-03-14 15:55 ` [PATCH 04/10] [v2] drm/i915: Don't touch South Display " Ben Widawsky @ 2013-03-17 21:21 ` Daniel Vetter 2013-03-18 23:05 ` Ben Widawsky 0 siblings, 1 reply; 40+ messages in thread From: Daniel Vetter @ 2013-03-17 21:21 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX On Thu, Mar 14, 2013 at 08:55:16AM -0700, Ben Widawsky wrote: > Interrupts, clock gating, and GMBUS are all within the, "this will hang > the CPU" range when we have PCH_NOP. > > There is a bit of a hack in init clock gating. We want to do most of the > clock gating, but the part we skip will hang the system. It could > probably be abstracted a bit better, but I don't feel it's too > unsightly. > > v2: Use inverse HAS_PCH_NOP check (Jani) > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++++++------ > drivers/gpu/drm/i915/intel_bios.c | 3 +++ > drivers/gpu/drm/i915/intel_i2c.c | 4 +++- > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > 4 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index b860f0b..15b4668 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) > } > } > > - /* check event from PCH */ > - if (de_iir & DE_PCH_EVENT_IVB) { > - u32 pch_iir = I915_READ(SDEIIR); > + if (!HAS_PCH_NOP(dev)) { > + /* check event from PCH */ > + if (de_iir & DE_PCH_EVENT_IVB) { && the two conditions to not require another indent level (it starts to look a bit ugly). And the PCH_NOP looks here better than a num_pipes check, so I'll merge that patch (i.e. disregard my other suggestion to replace PCH_NOP with num_pipes checks from a previous patch). Also note that Paulo's SDEIIR race fix added more reads/writes to that register in -fixes, so please rebase these patches on top of -nightly so that I can correctly apply them (I'll do the backmerge before slurping in your series). > + u32 pch_iir = I915_READ(SDEIIR); > > - cpt_irq_handler(dev, pch_iir); > + cpt_irq_handler(dev, pch_iir); > > - /* clear PCH hotplug event before clear CPU irq */ > - I915_WRITE(SDEIIR, pch_iir); > + /* clear PCH hotplug event before CPU irq */ > + I915_WRITE(SDEIIR, pch_iir); > + } > } > > I915_WRITE(DEIIR, de_iir); > @@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device *dev) > I915_WRITE(GTIER, 0x0); > POSTING_READ(GTIER); > > + if (HAS_PCH_NOP(dev)) > + return; > + > /* south display irq */ > I915_WRITE(SDEIMR, 0xffffffff); > I915_WRITE(SDEIER, 0x0); > @@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev) > SDE_GMBUS_CPT | > SDE_AUX_MASK_CPT; > > + if (HAS_PCH_NOP(dev)) > + return; > + > I915_WRITE(SDEIIR, I915_READ(SDEIIR)); > I915_WRITE(SDEIMR, ~mask); > I915_WRITE(SDEIER, mask); > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 55ffba1..194df27 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev) > struct bdb_header *bdb = NULL; > u8 __iomem *bios = NULL; > > + if (HAS_PCH_NOP(dev)) > + return -ENODEV; > + > init_vbt_defaults(dev_priv); > > /* XXX Should this validation be moved to intel_opregion.c? */ > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c > index acf8aec..fc19e49 100644 > --- a/drivers/gpu/drm/i915/intel_i2c.c > +++ b/drivers/gpu/drm/i915/intel_i2c.c > @@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > int ret, i; > > - if (HAS_PCH_SPLIT(dev)) > + if (HAS_PCH_NOP(dev)) > + return 0; > + else if (HAS_PCH_SPLIT(dev)) > dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA; > else if (IS_VALLEYVIEW(dev)) > dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE; > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 5479363..52203fd 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct drm_device *dev) > snpcr |= GEN6_MBC_SNPCR_MED; > I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr); > > - cpt_init_clock_gating(dev); > + if (HAS_PCH_NOP(dev)) > + cpt_init_clock_gating(dev); Inverted check? Cheers, Daniel > > gen6_check_mch_setup(dev); > } > -- > 1.8.1.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 04/10] [v2] drm/i915: Don't touch South Display when PCH_NOP 2013-03-17 21:21 ` Daniel Vetter @ 2013-03-18 23:05 ` Ben Widawsky 0 siblings, 0 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-18 23:05 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel GFX On Sun, Mar 17, 2013 at 10:21:30PM +0100, Daniel Vetter wrote: > On Thu, Mar 14, 2013 at 08:55:16AM -0700, Ben Widawsky wrote: > > Interrupts, clock gating, and GMBUS are all within the, "this will hang > > the CPU" range when we have PCH_NOP. > > > > There is a bit of a hack in init clock gating. We want to do most of the > > clock gating, but the part we skip will hang the system. It could > > probably be abstracted a bit better, but I don't feel it's too > > unsightly. > > > > v2: Use inverse HAS_PCH_NOP check (Jani) > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++++++------ > > drivers/gpu/drm/i915/intel_bios.c | 3 +++ > > drivers/gpu/drm/i915/intel_i2c.c | 4 +++- > > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > > 4 files changed, 22 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index b860f0b..15b4668 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) > > } > > } > > > > - /* check event from PCH */ > > - if (de_iir & DE_PCH_EVENT_IVB) { > > - u32 pch_iir = I915_READ(SDEIIR); > > + if (!HAS_PCH_NOP(dev)) { > > + /* check event from PCH */ > > + if (de_iir & DE_PCH_EVENT_IVB) { > > && the two conditions to not require another indent level (it starts to > look a bit ugly). And the PCH_NOP looks here better than a num_pipes > check, so I'll merge that patch (i.e. disregard my other suggestion to > replace PCH_NOP with num_pipes checks from a previous patch). > > Also note that Paulo's SDEIIR race fix added more reads/writes to that > register in -fixes, so please rebase these patches on top of -nightly so > that I can correctly apply them (I'll do the backmerge before slurping in > your series). Okay to both. > > > + u32 pch_iir = I915_READ(SDEIIR); > > > > - cpt_irq_handler(dev, pch_iir); > > + cpt_irq_handler(dev, pch_iir); > > > > - /* clear PCH hotplug event before clear CPU irq */ > > - I915_WRITE(SDEIIR, pch_iir); > > + /* clear PCH hotplug event before CPU irq */ > > + I915_WRITE(SDEIIR, pch_iir); > > + } > > } > > > > I915_WRITE(DEIIR, de_iir); > > @@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device *dev) > > I915_WRITE(GTIER, 0x0); > > POSTING_READ(GTIER); > > > > + if (HAS_PCH_NOP(dev)) > > + return; > > + > > /* south display irq */ > > I915_WRITE(SDEIMR, 0xffffffff); > > I915_WRITE(SDEIER, 0x0); > > @@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev) > > SDE_GMBUS_CPT | > > SDE_AUX_MASK_CPT; > > > > + if (HAS_PCH_NOP(dev)) > > + return; > > + > > I915_WRITE(SDEIIR, I915_READ(SDEIIR)); > > I915_WRITE(SDEIMR, ~mask); > > I915_WRITE(SDEIER, mask); > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > > index 55ffba1..194df27 100644 > > --- a/drivers/gpu/drm/i915/intel_bios.c > > +++ b/drivers/gpu/drm/i915/intel_bios.c > > @@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev) > > struct bdb_header *bdb = NULL; > > u8 __iomem *bios = NULL; > > > > + if (HAS_PCH_NOP(dev)) > > + return -ENODEV; > > + > > init_vbt_defaults(dev_priv); > > > > /* XXX Should this validation be moved to intel_opregion.c? */ > > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c > > index acf8aec..fc19e49 100644 > > --- a/drivers/gpu/drm/i915/intel_i2c.c > > +++ b/drivers/gpu/drm/i915/intel_i2c.c > > @@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev) > > struct drm_i915_private *dev_priv = dev->dev_private; > > int ret, i; > > > > - if (HAS_PCH_SPLIT(dev)) > > + if (HAS_PCH_NOP(dev)) > > + return 0; > > + else if (HAS_PCH_SPLIT(dev)) > > dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA; > > else if (IS_VALLEYVIEW(dev)) > > dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE; > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 5479363..52203fd 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct drm_device *dev) > > snpcr |= GEN6_MBC_SNPCR_MED; > > I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr); > > > > - cpt_init_clock_gating(dev); > > + if (HAS_PCH_NOP(dev)) > > + cpt_init_clock_gating(dev); > > Inverted check? Boy, how did I screw this up twice... Good thing you checked. > > Cheers, Daniel > > > > gen6_check_mch_setup(dev); > > } > > -- > > 1.8.1.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 5/9] drm/i915: Don't initialize watermark stuff with PCH_NOP 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky ` (3 preceding siblings ...) 2013-03-13 18:21 ` [PATCH 4/9] drm/i915: Don't touch South display when PCH_NOP Ben Widawsky @ 2013-03-13 18:21 ` Ben Widawsky 2013-03-13 18:47 ` Chris Wilson 2013-03-13 21:06 ` [PATCH 05/10] [v2] " Ben Widawsky 2013-03-13 18:21 ` [PATCH 6/9] drm/i915: PCH_NOP suspend/resume Ben Widawsky ` (4 subsequent siblings) 9 siblings, 2 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:21 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 52203fd..3166aa4 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4175,7 +4175,7 @@ void intel_init_pm(struct drm_device *dev) dev_priv->display.init_clock_gating = gen6_init_clock_gating; } else if (IS_IVYBRIDGE(dev)) { /* FIXME: detect B0+ stepping and use auto training */ - if (SNB_READ_WM0_LATENCY()) { + if (SNB_READ_WM0_LATENCY() && !HAS_PCH_NOP(dev)) { dev_priv->display.update_wm = ivybridge_update_wm; dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm; } else { -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 5/9] drm/i915: Don't initialize watermark stuff with PCH_NOP 2013-03-13 18:21 ` [PATCH 5/9] drm/i915: Don't initialize watermark stuff with PCH_NOP Ben Widawsky @ 2013-03-13 18:47 ` Chris Wilson 2013-03-13 21:06 ` [PATCH 05/10] [v2] " Ben Widawsky 1 sibling, 0 replies; 40+ messages in thread From: Chris Wilson @ 2013-03-13 18:47 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 11:21:04AM -0700, Ben Widawsky wrote: > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/intel_pm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 52203fd..3166aa4 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4175,7 +4175,7 @@ void intel_init_pm(struct drm_device *dev) > dev_priv->display.init_clock_gating = gen6_init_clock_gating; > } else if (IS_IVYBRIDGE(dev)) { > /* FIXME: detect B0+ stepping and use auto training */ > - if (SNB_READ_WM0_LATENCY()) { > + if (SNB_READ_WM0_LATENCY() && !HAS_PCH_NOP(dev)) { Check for platform support first? -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 05/10] [v2] drm/i915: Don't initialize watermark stuff with PCH_NOP 2013-03-13 18:21 ` [PATCH 5/9] drm/i915: Don't initialize watermark stuff with PCH_NOP Ben Widawsky 2013-03-13 18:47 ` Chris Wilson @ 2013-03-13 21:06 ` Ben Widawsky 2013-03-17 21:02 ` Daniel Vetter 1 sibling, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 21:06 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey v2: Move check to the top (Chris) Add BUG_ON for !ivybridge, since it's all we support for now (Ben) Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/intel_pm.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 52203fd..8e7908b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4153,7 +4153,12 @@ void intel_init_pm(struct drm_device *dev) i915_ironlake_get_mem_freq(dev); /* For FIFO watermark updates */ - if (HAS_PCH_SPLIT(dev)) { + if (HAS_PCH_NOP(dev)) { + BUG_ON(!IS_IVYBRIDGE(dev)); + dev_priv->display.init_clock_gating = ivybridge_init_clock_gating; + dev_priv->display.update_wm = NULL; + dev_priv->display.update_sprite_wm = NULL; + } else if (HAS_PCH_SPLIT(dev)) { if (IS_GEN5(dev)) { if (I915_READ(MLTR_ILK) & ILK_SRLT_MASK) dev_priv->display.update_wm = ironlake_update_wm; @@ -4175,7 +4180,7 @@ void intel_init_pm(struct drm_device *dev) dev_priv->display.init_clock_gating = gen6_init_clock_gating; } else if (IS_IVYBRIDGE(dev)) { /* FIXME: detect B0+ stepping and use auto training */ - if (SNB_READ_WM0_LATENCY()) { + if (SNB_READ_WM0_LATENCY() && !HAS_PCH_NOP(dev)) { dev_priv->display.update_wm = ivybridge_update_wm; dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm; } else { -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 05/10] [v2] drm/i915: Don't initialize watermark stuff with PCH_NOP 2013-03-13 21:06 ` [PATCH 05/10] [v2] " Ben Widawsky @ 2013-03-17 21:02 ` Daniel Vetter 2013-03-19 0:45 ` Ben Widawsky 0 siblings, 1 reply; 40+ messages in thread From: Daniel Vetter @ 2013-03-17 21:02 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 02:06:12PM -0700, Ben Widawsky wrote: > v2: Move check to the top (Chris) > Add BUG_ON for !ivybridge, since it's all we support for now (Ben) > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/intel_pm.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 52203fd..8e7908b 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4153,7 +4153,12 @@ void intel_init_pm(struct drm_device *dev) > i915_ironlake_get_mem_freq(dev); > > /* For FIFO watermark updates */ > - if (HAS_PCH_SPLIT(dev)) { > + if (HAS_PCH_NOP(dev)) { > + BUG_ON(!IS_IVYBRIDGE(dev)); > + dev_priv->display.init_clock_gating = ivybridge_init_clock_gating; > + dev_priv->display.update_wm = NULL; > + dev_priv->display.update_sprite_wm = NULL; > + } else if (HAS_PCH_SPLIT(dev)) { > if (IS_GEN5(dev)) { > if (I915_READ(MLTR_ILK) & ILK_SRLT_MASK) > dev_priv->display.update_wm = ironlake_update_wm; > @@ -4175,7 +4180,7 @@ void intel_init_pm(struct drm_device *dev) > dev_priv->display.init_clock_gating = gen6_init_clock_gating; > } else if (IS_IVYBRIDGE(dev)) { > /* FIXME: detect B0+ stepping and use auto training */ > - if (SNB_READ_WM0_LATENCY()) { > + if (SNB_READ_WM0_LATENCY() && !HAS_PCH_NOP(dev)) { > dev_priv->display.update_wm = ivybridge_update_wm; > dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm; > } else { I'm confused why we need this patch here - update_wm functions should only be called when we have a pipe. If there's any caller left I think we should fix those up, not paper over it here. And imo it's ok to have non-NULL vfuncs here (or anywhere else, e.g. pageflips) as long as we don't call them. After all the num_pips/PCH_NOP checks are here to make this as unobtrusive as possible. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 05/10] [v2] drm/i915: Don't initialize watermark stuff with PCH_NOP 2013-03-17 21:02 ` Daniel Vetter @ 2013-03-19 0:45 ` Ben Widawsky 0 siblings, 0 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-19 0:45 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx, Mcallister, Jeffrey On Sun, Mar 17, 2013 at 10:02:44PM +0100, Daniel Vetter wrote: > On Wed, Mar 13, 2013 at 02:06:12PM -0700, Ben Widawsky wrote: > > v2: Move check to the top (Chris) > > Add BUG_ON for !ivybridge, since it's all we support for now (Ben) > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 52203fd..8e7908b 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4153,7 +4153,12 @@ void intel_init_pm(struct drm_device *dev) > > i915_ironlake_get_mem_freq(dev); > > > > /* For FIFO watermark updates */ > > - if (HAS_PCH_SPLIT(dev)) { > > + if (HAS_PCH_NOP(dev)) { > > + BUG_ON(!IS_IVYBRIDGE(dev)); > > + dev_priv->display.init_clock_gating = ivybridge_init_clock_gating; > > + dev_priv->display.update_wm = NULL; > > + dev_priv->display.update_sprite_wm = NULL; > > + } else if (HAS_PCH_SPLIT(dev)) { > > if (IS_GEN5(dev)) { > > if (I915_READ(MLTR_ILK) & ILK_SRLT_MASK) > > dev_priv->display.update_wm = ironlake_update_wm; > > @@ -4175,7 +4180,7 @@ void intel_init_pm(struct drm_device *dev) > > dev_priv->display.init_clock_gating = gen6_init_clock_gating; > > } else if (IS_IVYBRIDGE(dev)) { > > /* FIXME: detect B0+ stepping and use auto training */ > > - if (SNB_READ_WM0_LATENCY()) { > > + if (SNB_READ_WM0_LATENCY() && !HAS_PCH_NOP(dev)) { > > dev_priv->display.update_wm = ivybridge_update_wm; > > dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm; > > } else { > > I'm confused why we need this patch here - update_wm functions should only > be called when we have a pipe. If there's any caller left I think we > should fix those up, not paper over it here. > > And imo it's ok to have non-NULL vfuncs here (or anywhere else, e.g. > pageflips) as long as we don't call them. After all the num_pips/PCH_NOP > checks are here to make this as unobtrusive as possible. > -Daniel I think you're right, so I've dropped this patch entirely. -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 6/9] drm/i915: PCH_NOP suspend/resume 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky ` (4 preceding siblings ...) 2013-03-13 18:21 ` [PATCH 5/9] drm/i915: Don't initialize watermark stuff with PCH_NOP Ben Widawsky @ 2013-03-13 18:21 ` Ben Widawsky 2013-03-17 21:28 ` Daniel Vetter 2013-03-13 18:21 ` [PATCH 7/9] drm/i915: Don't wait for PCH on reset Ben Widawsky ` (3 subsequent siblings) 9 siblings, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:21 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey More registers we can't write. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_suspend.c | 57 ++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c index c1e02b0..dd5766a 100644 --- a/drivers/gpu/drm/i915/i915_suspend.c +++ b/drivers/gpu/drm/i915/i915_suspend.c @@ -333,11 +333,19 @@ int i915_save_state(struct drm_device *dev) mutex_lock(&dev->struct_mutex); - i915_save_display(dev); + if (!HAS_PCH_NOP(dev)) + i915_save_display(dev); if (!drm_core_check_feature(dev, DRIVER_MODESET)) { /* Interrupt state */ - if (HAS_PCH_SPLIT(dev)) { + if (HAS_PCH_NOP(dev)) { + dev_priv->regfile.saveDEIER = I915_READ(DEIER); + dev_priv->regfile.saveDEIMR = I915_READ(DEIMR); + dev_priv->regfile.saveGTIER = I915_READ(GTIER); + dev_priv->regfile.saveGTIMR = I915_READ(GTIMR); + dev_priv->regfile.saveMCHBAR_RENDER_STANDBY = + I915_READ(RSTDBYCTL); + } else if (HAS_PCH_SPLIT(dev)) { dev_priv->regfile.saveDEIER = I915_READ(DEIER); dev_priv->regfile.saveDEIMR = I915_READ(DEIMR); dev_priv->regfile.saveGTIER = I915_READ(GTIER); @@ -361,13 +369,18 @@ int i915_save_state(struct drm_device *dev) /* Memory Arbitration state */ dev_priv->regfile.saveMI_ARB_STATE = I915_READ(MI_ARB_STATE); - /* Scratch space */ - for (i = 0; i < 16; i++) { - dev_priv->regfile.saveSWF0[i] = I915_READ(SWF00 + (i << 2)); - dev_priv->regfile.saveSWF1[i] = I915_READ(SWF10 + (i << 2)); + if (!HAS_PCH_NOP(dev)) { + /* Scratch space */ + for (i = 0; i < 16; i++) { + dev_priv->regfile.saveSWF0[i] = + I915_READ(SWF00 + (i << 2)); + dev_priv->regfile.saveSWF1[i] = + I915_READ(SWF10 + (i << 2)); + } + for (i = 0; i < 3; i++) + dev_priv->regfile.saveSWF2[i] = + I915_READ(SWF30 + (i << 2)); } - for (i = 0; i < 3; i++) - dev_priv->regfile.saveSWF2[i] = I915_READ(SWF30 + (i << 2)); mutex_unlock(&dev->struct_mutex); @@ -383,11 +396,17 @@ int i915_restore_state(struct drm_device *dev) mutex_lock(&dev->struct_mutex); - i915_restore_display(dev); + if (!HAS_PCH_NOP(dev)) + i915_restore_display(dev); if (!drm_core_check_feature(dev, DRIVER_MODESET)) { /* Interrupt state */ - if (HAS_PCH_SPLIT(dev)) { + if (HAS_PCH_NOP(dev)) { + I915_WRITE(DEIER, dev_priv->regfile.saveDEIER); + I915_WRITE(DEIMR, dev_priv->regfile.saveDEIMR); + I915_WRITE(GTIER, dev_priv->regfile.saveGTIER); + I915_WRITE(GTIMR, dev_priv->regfile.saveGTIMR); + } else if (HAS_PCH_SPLIT(dev)) { I915_WRITE(DEIER, dev_priv->regfile.saveDEIER); I915_WRITE(DEIMR, dev_priv->regfile.saveDEIMR); I915_WRITE(GTIER, dev_priv->regfile.saveGTIER); @@ -407,16 +426,22 @@ int i915_restore_state(struct drm_device *dev) /* Memory arbitration state */ I915_WRITE(MI_ARB_STATE, dev_priv->regfile.saveMI_ARB_STATE | 0xffff0000); - for (i = 0; i < 16; i++) { - I915_WRITE(SWF00 + (i << 2), dev_priv->regfile.saveSWF0[i]); - I915_WRITE(SWF10 + (i << 2), dev_priv->regfile.saveSWF1[i]); + if (!HAS_PCH_NOP(dev)) { + for (i = 0; i < 16; i++) { + I915_WRITE(SWF00 + (i << 2), + dev_priv->regfile.saveSWF0[i]); + I915_WRITE(SWF10 + (i << 2), + dev_priv->regfile.saveSWF1[i]); + } + for (i = 0; i < 3; i++) + I915_WRITE(SWF30 + (i << 2), + dev_priv->regfile.saveSWF2[i]); } - for (i = 0; i < 3; i++) - I915_WRITE(SWF30 + (i << 2), dev_priv->regfile.saveSWF2[i]); mutex_unlock(&dev->struct_mutex); - intel_i2c_reset(dev); + if (!HAS_PCH_NOP(dev)) + intel_i2c_reset(dev); return 0; } -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 6/9] drm/i915: PCH_NOP suspend/resume 2013-03-13 18:21 ` [PATCH 6/9] drm/i915: PCH_NOP suspend/resume Ben Widawsky @ 2013-03-17 21:28 ` Daniel Vetter 2013-03-19 0:51 ` Ben Widawsky 0 siblings, 1 reply; 40+ messages in thread From: Daniel Vetter @ 2013-03-17 21:28 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 11:21:05AM -0700, Ben Widawsky wrote: > More registers we can't write. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_suspend.c | 57 ++++++++++++++++++++++++++----------- > 1 file changed, 41 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c > index c1e02b0..dd5766a 100644 > --- a/drivers/gpu/drm/i915/i915_suspend.c > +++ b/drivers/gpu/drm/i915/i915_suspend.c > @@ -333,11 +333,19 @@ int i915_save_state(struct drm_device *dev) > > mutex_lock(&dev->struct_mutex); > > - i915_save_display(dev); > + if (!HAS_PCH_NOP(dev)) > + i915_save_display(dev); This here looks a bit funny - imo it's better to move this check to the only two places where we still touch registers in the kms case: lvds & pp restore. > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) { > /* Interrupt state */ > - if (HAS_PCH_SPLIT(dev)) { > + if (HAS_PCH_NOP(dev)) { > + dev_priv->regfile.saveDEIER = I915_READ(DEIER); > + dev_priv->regfile.saveDEIMR = I915_READ(DEIMR); > + dev_priv->regfile.saveGTIER = I915_READ(GTIER); > + dev_priv->regfile.saveGTIMR = I915_READ(GTIMR); > + dev_priv->regfile.saveMCHBAR_RENDER_STANDBY = > + I915_READ(RSTDBYCTL); > + } else if (HAS_PCH_SPLIT(dev)) { > dev_priv->regfile.saveDEIER = I915_READ(DEIER); > dev_priv->regfile.saveDEIMR = I915_READ(DEIMR); > dev_priv->regfile.saveGTIER = I915_READ(GTIER); > @@ -361,13 +369,18 @@ int i915_save_state(struct drm_device *dev) > /* Memory Arbitration state */ > dev_priv->regfile.saveMI_ARB_STATE = I915_READ(MI_ARB_STATE); > > - /* Scratch space */ > - for (i = 0; i < 16; i++) { > - dev_priv->regfile.saveSWF0[i] = I915_READ(SWF00 + (i << 2)); > - dev_priv->regfile.saveSWF1[i] = I915_READ(SWF10 + (i << 2)); > + if (!HAS_PCH_NOP(dev)) { > + /* Scratch space */ > + for (i = 0; i < 16; i++) { > + dev_priv->regfile.saveSWF0[i] = > + I915_READ(SWF00 + (i << 2)); > + dev_priv->regfile.saveSWF1[i] = > + I915_READ(SWF10 + (i << 2)); > + } > + for (i = 0; i < 3; i++) > + dev_priv->regfile.saveSWF2[i] = > + I915_READ(SWF30 + (i << 2)); Blergh, I hate those registers, and I have no idea where we actually need to restore them for kms. Can you please also add a big "XXX: Do we really need this for kms?" comment in the scratch space block? > } > - for (i = 0; i < 3; i++) > - dev_priv->regfile.saveSWF2[i] = I915_READ(SWF30 + (i << 2)); > > mutex_unlock(&dev->struct_mutex); > > @@ -383,11 +396,17 @@ int i915_restore_state(struct drm_device *dev) > > mutex_lock(&dev->struct_mutex); > > - i915_restore_display(dev); > + if (!HAS_PCH_NOP(dev)) > + i915_restore_display(dev); > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) { > /* Interrupt state */ > - if (HAS_PCH_SPLIT(dev)) { > + if (HAS_PCH_NOP(dev)) { > + I915_WRITE(DEIER, dev_priv->regfile.saveDEIER); > + I915_WRITE(DEIMR, dev_priv->regfile.saveDEIMR); > + I915_WRITE(GTIER, dev_priv->regfile.saveGTIER); > + I915_WRITE(GTIMR, dev_priv->regfile.saveGTIMR); > + } else if (HAS_PCH_SPLIT(dev)) { > I915_WRITE(DEIER, dev_priv->regfile.saveDEIER); > I915_WRITE(DEIMR, dev_priv->regfile.saveDEIMR); > I915_WRITE(GTIER, dev_priv->regfile.saveGTIER); > @@ -407,16 +426,22 @@ int i915_restore_state(struct drm_device *dev) > /* Memory arbitration state */ > I915_WRITE(MI_ARB_STATE, dev_priv->regfile.saveMI_ARB_STATE | 0xffff0000); > > - for (i = 0; i < 16; i++) { > - I915_WRITE(SWF00 + (i << 2), dev_priv->regfile.saveSWF0[i]); > - I915_WRITE(SWF10 + (i << 2), dev_priv->regfile.saveSWF1[i]); > + if (!HAS_PCH_NOP(dev)) { > + for (i = 0; i < 16; i++) { > + I915_WRITE(SWF00 + (i << 2), > + dev_priv->regfile.saveSWF0[i]); > + I915_WRITE(SWF10 + (i << 2), > + dev_priv->regfile.saveSWF1[i]); > + } > + for (i = 0; i < 3; i++) > + I915_WRITE(SWF30 + (i << 2), > + dev_priv->regfile.saveSWF2[i]); > } > - for (i = 0; i < 3; i++) > - I915_WRITE(SWF30 + (i << 2), dev_priv->regfile.saveSWF2[i]); > > mutex_unlock(&dev->struct_mutex); > > - intel_i2c_reset(dev); > + if (!HAS_PCH_NOP(dev)) > + intel_i2c_reset(dev); > > return 0; > } > -- > 1.8.1.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 6/9] drm/i915: PCH_NOP suspend/resume 2013-03-17 21:28 ` Daniel Vetter @ 2013-03-19 0:51 ` Ben Widawsky 2013-03-19 7:51 ` Daniel Vetter 0 siblings, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-19 0:51 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx, Mcallister, Jeffrey On Sun, Mar 17, 2013 at 10:28:55PM +0100, Daniel Vetter wrote: > On Wed, Mar 13, 2013 at 11:21:05AM -0700, Ben Widawsky wrote: > > More registers we can't write. > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_suspend.c | 57 ++++++++++++++++++++++++++----------- > > 1 file changed, 41 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c > > index c1e02b0..dd5766a 100644 > > --- a/drivers/gpu/drm/i915/i915_suspend.c > > +++ b/drivers/gpu/drm/i915/i915_suspend.c > > @@ -333,11 +333,19 @@ int i915_save_state(struct drm_device *dev) > > > > mutex_lock(&dev->struct_mutex); > > > > - i915_save_display(dev); > > + if (!HAS_PCH_NOP(dev)) > > + i915_save_display(dev); > > This here looks a bit funny - imo it's better to move this check to the > only two places where we still touch registers in the kms case: lvds & pp > restore. I had something like this originally, except I also can't touch the backlight registers (even though they're not in a bad range). In an earlier patch you asked me to move the check up in the callchain, and I liked that idea. It seems to me here the same logic applies, we never care about any of the display registers. If you feel strongly about it though, I will change it. Please correct me if I misunderstood your request. > > > > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) { > > /* Interrupt state */ > > - if (HAS_PCH_SPLIT(dev)) { > > + if (HAS_PCH_NOP(dev)) { > > + dev_priv->regfile.saveDEIER = I915_READ(DEIER); > > + dev_priv->regfile.saveDEIMR = I915_READ(DEIMR); > > + dev_priv->regfile.saveGTIER = I915_READ(GTIER); > > + dev_priv->regfile.saveGTIMR = I915_READ(GTIMR); > > + dev_priv->regfile.saveMCHBAR_RENDER_STANDBY = > > + I915_READ(RSTDBYCTL); > > + } else if (HAS_PCH_SPLIT(dev)) { > > dev_priv->regfile.saveDEIER = I915_READ(DEIER); > > dev_priv->regfile.saveDEIMR = I915_READ(DEIMR); > > dev_priv->regfile.saveGTIER = I915_READ(GTIER); > > @@ -361,13 +369,18 @@ int i915_save_state(struct drm_device *dev) > > /* Memory Arbitration state */ > > dev_priv->regfile.saveMI_ARB_STATE = I915_READ(MI_ARB_STATE); > > > > - /* Scratch space */ > > - for (i = 0; i < 16; i++) { > > - dev_priv->regfile.saveSWF0[i] = I915_READ(SWF00 + (i << 2)); > > - dev_priv->regfile.saveSWF1[i] = I915_READ(SWF10 + (i << 2)); > > + if (!HAS_PCH_NOP(dev)) { > > + /* Scratch space */ > > + for (i = 0; i < 16; i++) { > > + dev_priv->regfile.saveSWF0[i] = > > + I915_READ(SWF00 + (i << 2)); > > + dev_priv->regfile.saveSWF1[i] = > > + I915_READ(SWF10 + (i << 2)); > > + } > > + for (i = 0; i < 3; i++) > > + dev_priv->regfile.saveSWF2[i] = > > + I915_READ(SWF30 + (i << 2)); > > Blergh, I hate those registers, and I have no idea where we actually need > to restore them for kms. Can you please also add a big "XXX: Do we really > need this for kms?" comment in the scratch space block? > > > } > > - for (i = 0; i < 3; i++) > > - dev_priv->regfile.saveSWF2[i] = I915_READ(SWF30 + (i << 2)); > > > > mutex_unlock(&dev->struct_mutex); > > > > @@ -383,11 +396,17 @@ int i915_restore_state(struct drm_device *dev) > > > > mutex_lock(&dev->struct_mutex); > > > > - i915_restore_display(dev); > > + if (!HAS_PCH_NOP(dev)) > > + i915_restore_display(dev); > > > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) { > > /* Interrupt state */ > > - if (HAS_PCH_SPLIT(dev)) { > > + if (HAS_PCH_NOP(dev)) { > > + I915_WRITE(DEIER, dev_priv->regfile.saveDEIER); > > + I915_WRITE(DEIMR, dev_priv->regfile.saveDEIMR); > > + I915_WRITE(GTIER, dev_priv->regfile.saveGTIER); > > + I915_WRITE(GTIMR, dev_priv->regfile.saveGTIMR); > > + } else if (HAS_PCH_SPLIT(dev)) { > > I915_WRITE(DEIER, dev_priv->regfile.saveDEIER); > > I915_WRITE(DEIMR, dev_priv->regfile.saveDEIMR); > > I915_WRITE(GTIER, dev_priv->regfile.saveGTIER); > > @@ -407,16 +426,22 @@ int i915_restore_state(struct drm_device *dev) > > /* Memory arbitration state */ > > I915_WRITE(MI_ARB_STATE, dev_priv->regfile.saveMI_ARB_STATE | 0xffff0000); > > > > - for (i = 0; i < 16; i++) { > > - I915_WRITE(SWF00 + (i << 2), dev_priv->regfile.saveSWF0[i]); > > - I915_WRITE(SWF10 + (i << 2), dev_priv->regfile.saveSWF1[i]); > > + if (!HAS_PCH_NOP(dev)) { > > + for (i = 0; i < 16; i++) { > > + I915_WRITE(SWF00 + (i << 2), > > + dev_priv->regfile.saveSWF0[i]); > > + I915_WRITE(SWF10 + (i << 2), > > + dev_priv->regfile.saveSWF1[i]); > > + } > > + for (i = 0; i < 3; i++) > > + I915_WRITE(SWF30 + (i << 2), > > + dev_priv->regfile.saveSWF2[i]); > > } > > - for (i = 0; i < 3; i++) > > - I915_WRITE(SWF30 + (i << 2), dev_priv->regfile.saveSWF2[i]); > > > > mutex_unlock(&dev->struct_mutex); > > > > - intel_i2c_reset(dev); > > + if (!HAS_PCH_NOP(dev)) > > + intel_i2c_reset(dev); > > > > return 0; > > } > > -- > > 1.8.1.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 6/9] drm/i915: PCH_NOP suspend/resume 2013-03-19 0:51 ` Ben Widawsky @ 2013-03-19 7:51 ` Daniel Vetter 2013-03-19 17:53 ` Ben Widawsky 0 siblings, 1 reply; 40+ messages in thread From: Daniel Vetter @ 2013-03-19 7:51 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Tue, Mar 19, 2013 at 1:51 AM, Ben Widawsky <ben@bwidawsk.net> wrote: > On Sun, Mar 17, 2013 at 10:28:55PM +0100, Daniel Vetter wrote: >> On Wed, Mar 13, 2013 at 11:21:05AM -0700, Ben Widawsky wrote: >> > More registers we can't write. >> > >> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> >> > --- >> > drivers/gpu/drm/i915/i915_suspend.c | 57 ++++++++++++++++++++++++++----------- >> > 1 file changed, 41 insertions(+), 16 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c >> > index c1e02b0..dd5766a 100644 >> > --- a/drivers/gpu/drm/i915/i915_suspend.c >> > +++ b/drivers/gpu/drm/i915/i915_suspend.c >> > @@ -333,11 +333,19 @@ int i915_save_state(struct drm_device *dev) >> > >> > mutex_lock(&dev->struct_mutex); >> > >> > - i915_save_display(dev); >> > + if (!HAS_PCH_NOP(dev)) >> > + i915_save_display(dev); >> >> This here looks a bit funny - imo it's better to move this check to the >> only two places where we still touch registers in the kms case: lvds & pp >> restore. > > I had something like this originally, except I also can't touch the > backlight registers (even though they're not in a bad range). > > In an earlier patch you asked me to move the check up in the callchain, > and I liked that idea. It seems to me here the same logic applies, we > never care about any of the display registers. If you feel strongly > about it though, I will change it. Please correct me if I misunderstood > your request. Yes, in general I think it's good to move the checks up in the callchains and consolidate them that way. I'm voting for an exception here in the register save/restore code since we're slowly moving away from it for kms. Moving the PCH_NOP check into the few remaining places allows us to easily kill them once those are guarded with if (!kms) checks, too. I've just figured that that way we won't have a stray PCH_NOP check left behind for code which (eventually) isn't run at all in kms mode. I don't mind if you leave this as-is, really just a tiny bikeshed. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 6/9] drm/i915: PCH_NOP suspend/resume 2013-03-19 7:51 ` Daniel Vetter @ 2013-03-19 17:53 ` Ben Widawsky 0 siblings, 0 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-19 17:53 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx, Mcallister, Jeffrey On Tue, Mar 19, 2013 at 08:51:01AM +0100, Daniel Vetter wrote: > On Tue, Mar 19, 2013 at 1:51 AM, Ben Widawsky <ben@bwidawsk.net> wrote: > > On Sun, Mar 17, 2013 at 10:28:55PM +0100, Daniel Vetter wrote: > >> On Wed, Mar 13, 2013 at 11:21:05AM -0700, Ben Widawsky wrote: > >> > More registers we can't write. > >> > > >> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > >> > --- > >> > drivers/gpu/drm/i915/i915_suspend.c | 57 ++++++++++++++++++++++++++----------- > >> > 1 file changed, 41 insertions(+), 16 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c > >> > index c1e02b0..dd5766a 100644 > >> > --- a/drivers/gpu/drm/i915/i915_suspend.c > >> > +++ b/drivers/gpu/drm/i915/i915_suspend.c > >> > @@ -333,11 +333,19 @@ int i915_save_state(struct drm_device *dev) > >> > > >> > mutex_lock(&dev->struct_mutex); > >> > > >> > - i915_save_display(dev); > >> > + if (!HAS_PCH_NOP(dev)) > >> > + i915_save_display(dev); > >> > >> This here looks a bit funny - imo it's better to move this check to the > >> only two places where we still touch registers in the kms case: lvds & pp > >> restore. > > > > I had something like this originally, except I also can't touch the > > backlight registers (even though they're not in a bad range). > > > > In an earlier patch you asked me to move the check up in the callchain, > > and I liked that idea. It seems to me here the same logic applies, we > > never care about any of the display registers. If you feel strongly > > about it though, I will change it. Please correct me if I misunderstood > > your request. > > Yes, in general I think it's good to move the checks up in the > callchains and consolidate them that way. I'm voting for an exception > here in the register save/restore code since we're slowly moving away > from it for kms. Moving the PCH_NOP check into the few remaining > places allows us to easily kill them once those are guarded with if > (!kms) checks, too. I've just figured that that way we won't have a > stray PCH_NOP check left behind for code which (eventually) isn't run > at all in kms mode. > > I don't mind if you leave this as-is, really just a tiny bikeshed. > -Daniel I think having the check never hurts, and since I tend to introduce bugs whenever I change things, I'll punt on this if you don't mind. > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 7/9] drm/i915: Don't wait for PCH on reset 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky ` (5 preceding siblings ...) 2013-03-13 18:21 ` [PATCH 6/9] drm/i915: PCH_NOP suspend/resume Ben Widawsky @ 2013-03-13 18:21 ` Ben Widawsky 2013-03-19 19:36 ` Jesse Barnes 2013-03-13 18:21 ` [PATCH 8/9] drm/i915: Set PCH_NOP Ben Widawsky ` (2 subsequent siblings) 9 siblings, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:21 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey BIOS should be setting this, but in case it doesn't... Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_gem.c | 3 +++ drivers/gpu/drm/i915/i915_reg.h | 1 + 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 1417fc6..ee9c14c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3986,6 +3986,9 @@ i915_gem_init_hw(struct drm_device *dev) if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1)) I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000); + if (HAS_PCH_NOP(dev)) + I915_WRITE(GEN7_MSG_CTL, 0); + i915_gem_l3_remap(dev); i915_gem_init_swizzling(dev); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4cf3ece..00b6a55 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3481,6 +3481,7 @@ #define DISP_ARB_CTL 0x45000 #define DISP_TILE_SURFACE_SWIZZLING (1<<13) #define DISP_FBC_WM_DIS (1<<15) +#define GEN7_MSG_CTL 0x45010 /* GEN7 chicken */ #define GEN7_COMMON_SLICE_CHICKEN1 0x7010 -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 7/9] drm/i915: Don't wait for PCH on reset 2013-03-13 18:21 ` [PATCH 7/9] drm/i915: Don't wait for PCH on reset Ben Widawsky @ 2013-03-19 19:36 ` Jesse Barnes 2013-03-20 16:43 ` [PATCH] " Ben Widawsky 0 siblings, 1 reply; 40+ messages in thread From: Jesse Barnes @ 2013-03-19 19:36 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Wed, 13 Mar 2013 11:21:06 -0700 Ben Widawsky <ben@bwidawsk.net> wrote: > BIOS should be setting this, but in case it doesn't... > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_gem.c | 3 +++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 1417fc6..ee9c14c 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3986,6 +3986,9 @@ i915_gem_init_hw(struct drm_device *dev) > if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1)) > I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000); > > + if (HAS_PCH_NOP(dev)) > + I915_WRITE(GEN7_MSG_CTL, 0); > + > i915_gem_l3_remap(dev); > > i915_gem_init_swizzling(dev); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 4cf3ece..00b6a55 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3481,6 +3481,7 @@ > #define DISP_ARB_CTL 0x45000 > #define DISP_TILE_SURFACE_SWIZZLING (1<<13) > #define DISP_FBC_WM_DIS (1<<15) > +#define GEN7_MSG_CTL 0x45010 > > /* GEN7 chicken */ > #define GEN7_COMMON_SLICE_CHICKEN1 0x7010 I think this needs a comment or at least a define or two for the bits. And are we sure we want to clobber the upper bits? They're listed as "PBC", which may be related to PBR, but somehow I doubt the GT will spew beer if we write the bits. -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] drm/i915: Don't wait for PCH on reset 2013-03-19 19:36 ` Jesse Barnes @ 2013-03-20 16:43 ` Ben Widawsky 2013-03-20 16:58 ` Jesse Barnes 0 siblings, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-20 16:43 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx, Ben Widawsky BIOS should be setting this, but in case it doesn't... v2: Define the bits we actually want to clear (Jesse) Make it an RMW op (Jesse) Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_gem.c | 6 ++++++ drivers/gpu/drm/i915/i915_reg.h | 3 +++ 2 files changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8a2cbee..f2d4970 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3991,6 +3991,12 @@ i915_gem_init_hw(struct drm_device *dev) if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1)) I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000); + if (HAS_PCH_NOP(dev)) { + u32 temp = I915_READ(GEN7_MSG_CTL); + temp &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK); + I915_WRITE(GEN7_MSG_CTL, temp); + } + i915_gem_l3_remap(dev); i915_gem_init_swizzling(dev); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 176cf5c..a627756 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3481,6 +3481,9 @@ #define DISP_ARB_CTL 0x45000 #define DISP_TILE_SURFACE_SWIZZLING (1<<13) #define DISP_FBC_WM_DIS (1<<15) +#define GEN7_MSG_CTL 0x45010 +#define WAIT_FOR_PCH_RESET_ACK (1<<1) +#define WAIT_FOR_PCH_FLR_ACK (1<<0) /* GEN7 chicken */ #define GEN7_COMMON_SLICE_CHICKEN1 0x7010 -- 1.8.2 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH] drm/i915: Don't wait for PCH on reset 2013-03-20 16:43 ` [PATCH] " Ben Widawsky @ 2013-03-20 16:58 ` Jesse Barnes 0 siblings, 0 replies; 40+ messages in thread From: Jesse Barnes @ 2013-03-20 16:58 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx On Wed, 20 Mar 2013 09:43:00 -0700 Ben Widawsky <ben@bwidawsk.net> wrote: > BIOS should be setting this, but in case it doesn't... > > v2: Define the bits we actually want to clear (Jesse) > Make it an RMW op (Jesse) > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_gem.c | 6 ++++++ > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 8a2cbee..f2d4970 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3991,6 +3991,12 @@ i915_gem_init_hw(struct drm_device *dev) > if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1)) > I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000); > > + if (HAS_PCH_NOP(dev)) { > + u32 temp = I915_READ(GEN7_MSG_CTL); > + temp &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK); > + I915_WRITE(GEN7_MSG_CTL, temp); > + } > + > i915_gem_l3_remap(dev); > > i915_gem_init_swizzling(dev); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 176cf5c..a627756 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3481,6 +3481,9 @@ > #define DISP_ARB_CTL 0x45000 > #define DISP_TILE_SURFACE_SWIZZLING (1<<13) > #define DISP_FBC_WM_DIS (1<<15) > +#define GEN7_MSG_CTL 0x45010 > +#define WAIT_FOR_PCH_RESET_ACK (1<<1) > +#define WAIT_FOR_PCH_FLR_ACK (1<<0) > > /* GEN7 chicken */ > #define GEN7_COMMON_SLICE_CHICKEN1 0x7010 Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 8/9] drm/i915: Set PCH_NOP 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky ` (6 preceding siblings ...) 2013-03-13 18:21 ` [PATCH 7/9] drm/i915: Don't wait for PCH on reset Ben Widawsky @ 2013-03-13 18:21 ` Ben Widawsky 2013-03-17 21:36 ` Daniel Vetter 2013-03-13 18:21 ` [PATCH 9/9] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky 2013-03-13 18:58 ` [PATCH 0/9] displayless PCH Chris Wilson 9 siblings, 1 reply; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:21 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey Set up PCH_NOP when we match a certain platform. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 0849651..0fa55cb 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -398,11 +398,22 @@ static const struct pci_device_id pciidlist[] = { /* aka */ MODULE_DEVICE_TABLE(pci, pciidlist); #endif +static bool intel_pch_displayless(struct drm_device *dev) +{ + return false; +} + void intel_detect_pch(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct pci_dev *pch; + if (intel_pch_displayless(dev)) { + dev_priv->pch_type = PCH_NOP; + dev_priv->num_pch_pll = 0; + return; + } + /* * The reason to probe ISA bridge instead of Dev31:Fun0 is to * make graphics device passthrough work easy for VMM, that only -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 8/9] drm/i915: Set PCH_NOP 2013-03-13 18:21 ` [PATCH 8/9] drm/i915: Set PCH_NOP Ben Widawsky @ 2013-03-17 21:36 ` Daniel Vetter 0 siblings, 0 replies; 40+ messages in thread From: Daniel Vetter @ 2013-03-17 21:36 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 11:21:07AM -0700, Ben Widawsky wrote: > Set up PCH_NOP when we match a certain platform. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_drv.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 0849651..0fa55cb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -398,11 +398,22 @@ static const struct pci_device_id pciidlist[] = { /* aka */ > MODULE_DEVICE_TABLE(pci, pciidlist); > #endif > > +static bool intel_pch_displayless(struct drm_device *dev) > +{ > + return false; > +} > + > void intel_detect_pch(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct pci_dev *pch; > > + if (intel_pch_displayless(dev)) { Just checking for num_pipes == 0 + a comment not good enough? I know, a bit hapzardous, but I don't see how the indirection helps code clarity. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 9/9] drm/i915: Add a pipeless ivybridge configuration 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky ` (7 preceding siblings ...) 2013-03-13 18:21 ` [PATCH 8/9] drm/i915: Set PCH_NOP Ben Widawsky @ 2013-03-13 18:21 ` Ben Widawsky 2013-03-13 18:52 ` Chris Wilson 2013-03-13 21:08 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky 2013-03-13 18:58 ` [PATCH 0/9] displayless PCH Chris Wilson 9 siblings, 2 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 18:21 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey FIXME: This is based on some HW being used for a demo. We should probably wait until we have confirmation on the IDs before upstreaming this patch. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 0fa55cb..9b3bb1e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -135,6 +135,16 @@ extern int intel_agp_enabled; .subdevice = PCI_ANY_ID, \ .driver_data = (unsigned long) info } +#define INTEL_QUANTA_VGA_DEVICE(info) { \ + .class = PCI_BASE_CLASS_DISPLAY << 16, \ + .class_mask = 0xff0000, \ + .vendor = 0x8086, \ + .device = 0x16a, \ + .subvendor = 0x152d, \ + .subdevice = 0x8990, \ + .driver_data = (unsigned long) info } + + static const struct intel_device_info intel_i830_info = { .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 1, .has_overlay = 1, .overlay_needs_physical = 1, @@ -248,6 +258,15 @@ static const struct intel_device_info intel_sandybridge_m_info = { .has_force_wake = 1, }; +static const struct intel_device_info intel_ivybridge_q_info = { + .is_ivybridge = 1, .gen = 7, .num_pipes = 0, + .need_gfx_hws = 1, + .has_bsd_ring = 1, + .has_blt_ring = 1, + .has_llc = 1, + .has_force_wake = 1, +}; + static const struct intel_device_info intel_ivybridge_d_info = { .is_ivybridge = 1, .gen = 7, .num_pipes = 3, .need_gfx_hws = 1, .has_hotplug = 1, @@ -351,6 +370,7 @@ static const struct pci_device_id pciidlist[] = { /* aka */ INTEL_VGA_DEVICE(0x0152, &intel_ivybridge_d_info), /* GT1 desktop */ INTEL_VGA_DEVICE(0x0162, &intel_ivybridge_d_info), /* GT2 desktop */ INTEL_VGA_DEVICE(0x015a, &intel_ivybridge_d_info), /* GT1 server */ + INTEL_QUANTA_VGA_DEVICE(&intel_ivybridge_q_info), /* Quanta transcode */ INTEL_VGA_DEVICE(0x016a, &intel_ivybridge_d_info), /* GT2 server */ INTEL_VGA_DEVICE(0x0402, &intel_haswell_d_info), /* GT1 desktop */ INTEL_VGA_DEVICE(0x0412, &intel_haswell_d_info), /* GT2 desktop */ @@ -400,7 +420,7 @@ MODULE_DEVICE_TABLE(pci, pciidlist); static bool intel_pch_displayless(struct drm_device *dev) { - return false; + return INTEL_INFO(dev) == &intel_ivybridge_q_info; } void intel_detect_pch(struct drm_device *dev) -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 9/9] drm/i915: Add a pipeless ivybridge configuration 2013-03-13 18:21 ` [PATCH 9/9] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky @ 2013-03-13 18:52 ` Chris Wilson 2013-03-13 21:08 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky 1 sibling, 0 replies; 40+ messages in thread From: Chris Wilson @ 2013-03-13 18:52 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 11:21:08AM -0700, Ben Widawsky wrote: > FIXME: This is based on some HW being used for a demo. We should > probably wait until we have confirmation on the IDs before upstreaming > this patch. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_drv.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 0fa55cb..9b3bb1e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -135,6 +135,16 @@ extern int intel_agp_enabled; > .subdevice = PCI_ANY_ID, \ > .driver_data = (unsigned long) info } > > +#define INTEL_QUANTA_VGA_DEVICE(info) { \ > + .class = PCI_BASE_CLASS_DISPLAY << 16, \ > + .class_mask = 0xff0000, \ > + .vendor = 0x8086, \ > + .device = 0x16a, \ > + .subvendor = 0x152d, \ > + .subdevice = 0x8990, \ > + .driver_data = (unsigned long) info } > + > + > static const struct intel_device_info intel_i830_info = { > .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 1, > .has_overlay = 1, .overlay_needs_physical = 1, > @@ -248,6 +258,15 @@ static const struct intel_device_info intel_sandybridge_m_info = { > .has_force_wake = 1, > }; > > +static const struct intel_device_info intel_ivybridge_q_info = { > + .is_ivybridge = 1, .gen = 7, .num_pipes = 0, > + .need_gfx_hws = 1, > + .has_bsd_ring = 1, > + .has_blt_ring = 1, > + .has_llc = 1, > + .has_force_wake = 1, What we could start to introduce is #define IVB_FEATURES \ .is_ivybridge = 1, \ .gen = 7, \ .num_pipes = 3, \ .need_gfx_hws = 1,\ etc then static const struct intel_device_info intel_ivybridge_d_info = { IVB_FEATURES, }; static const struct intel_device_info intel_ivybridge_m_info = { IVB_FEATURES, .is_mobile = 1, }; static const struct intel_device_info intel_ivybridge_q_info = { IVB_FEATURES, .num_pipes = 0, /* legal, last one wins */ }; -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition 2013-03-13 18:21 ` [PATCH 9/9] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky 2013-03-13 18:52 ` Chris Wilson @ 2013-03-13 21:08 ` Ben Widawsky 2013-03-13 21:08 ` [PATCH 10/10] [v2] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky 2013-03-15 4:05 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky 1 sibling, 2 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 21:08 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey Recommended by Chris. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a63abd7..2ee89c2 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -248,23 +248,21 @@ static const struct intel_device_info intel_sandybridge_m_info = { .has_force_wake = 1, }; +#define IVB_FEATURES \ + .is_ivybridge = 1, .gen = 7, .num_pipes = 3, \ + .need_gfx_hws = 1, .has_hotplug = 1, \ + .has_bsd_ring = 1, \ + .has_blt_ring = 1, \ + .has_llc = 1, \ + .has_force_wake = 1 + static const struct intel_device_info intel_ivybridge_d_info = { - .is_ivybridge = 1, .gen = 7, .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, - .has_bsd_ring = 1, - .has_blt_ring = 1, - .has_llc = 1, - .has_force_wake = 1, + IVB_FEATURES, }; static const struct intel_device_info intel_ivybridge_m_info = { - .is_ivybridge = 1, .gen = 7, .is_mobile = 1, .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, - .has_fbc = 0, /* FBC is not enabled on Ivybridge mobile yet */ - .has_bsd_ring = 1, - .has_blt_ring = 1, - .has_llc = 1, - .has_force_wake = 1, + IVB_FEATURES, + .is_mobile = 1, }; static const struct intel_device_info intel_valleyview_m_info = { -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 10/10] [v2] drm/i915: Add a pipeless ivybridge configuration 2013-03-13 21:08 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky @ 2013-03-13 21:08 ` Ben Widawsky 2013-03-15 4:05 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky 1 sibling, 0 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 21:08 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky, Mcallister, Jeffrey FIXME: This is based on some HW being used for a demo. We should probably wait until we have confirmation on the IDs before upstreaming this patch. v2: Use IVB_FEATURES (Chris) Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 2ee89c2..c1a5967 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -135,6 +135,16 @@ extern int intel_agp_enabled; .subdevice = PCI_ANY_ID, \ .driver_data = (unsigned long) info } +#define INTEL_QUANTA_VGA_DEVICE(info) { \ + .class = PCI_BASE_CLASS_DISPLAY << 16, \ + .class_mask = 0xff0000, \ + .vendor = 0x8086, \ + .device = 0x16a, \ + .subvendor = 0x152d, \ + .subdevice = 0x8990, \ + .driver_data = (unsigned long) info } + + static const struct intel_device_info intel_i830_info = { .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2, .has_overlay = 1, .overlay_needs_physical = 1, @@ -265,6 +275,11 @@ static const struct intel_device_info intel_ivybridge_m_info = { .is_mobile = 1, }; +static const struct intel_device_info intel_ivybridge_q_info = { + IVB_FEATURES, + .num_pipes = 0, /* legal, last one wins */ +}; + static const struct intel_device_info intel_valleyview_m_info = { .gen = 7, .is_mobile = 1, .num_pipes = 2, .need_gfx_hws = 1, .has_hotplug = 1, @@ -349,6 +364,7 @@ static const struct pci_device_id pciidlist[] = { /* aka */ INTEL_VGA_DEVICE(0x0152, &intel_ivybridge_d_info), /* GT1 desktop */ INTEL_VGA_DEVICE(0x0162, &intel_ivybridge_d_info), /* GT2 desktop */ INTEL_VGA_DEVICE(0x015a, &intel_ivybridge_d_info), /* GT1 server */ + INTEL_QUANTA_VGA_DEVICE(&intel_ivybridge_q_info), /* Quanta transcode */ INTEL_VGA_DEVICE(0x016a, &intel_ivybridge_d_info), /* GT2 server */ INTEL_VGA_DEVICE(0x0402, &intel_haswell_d_info), /* GT1 desktop */ INTEL_VGA_DEVICE(0x0412, &intel_haswell_d_info), /* GT2 desktop */ @@ -398,7 +414,7 @@ MODULE_DEVICE_TABLE(pci, pciidlist); static bool intel_pch_displayless(struct drm_device *dev) { - return false; + return INTEL_INFO(dev) == &intel_ivybridge_q_info; } void intel_detect_pch(struct drm_device *dev) -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition 2013-03-13 21:08 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky 2013-03-13 21:08 ` [PATCH 10/10] [v2] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky @ 2013-03-15 4:05 ` Ben Widawsky 1 sibling, 0 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-15 4:05 UTC (permalink / raw) To: intel-gfx On Wed, 13 Mar 2013 14:08:28 -0700 Ben Widawsky <ben@bwidawsk.net> wrote: > Recommended by Chris. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_drv.c | 24 +++++++++++------------- > 1 file changed, 11 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c index a63abd7..2ee89c2 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -248,23 +248,21 @@ static const struct intel_device_info > intel_sandybridge_m_info = { .has_force_wake = 1, > }; > > +#define IVB_FEATURES \ > + .is_ivybridge = 1, .gen = 7, .num_pipes = 3, \ > + .need_gfx_hws = 1, .has_hotplug = 1, \ > + .has_bsd_ring = 1, \ > + .has_blt_ring = 1, \ > + .has_llc = 1, \ > + .has_force_wake = 1 > + > static const struct intel_device_info intel_ivybridge_d_info = { > - .is_ivybridge = 1, .gen = 7, .num_pipes = 3, > - .need_gfx_hws = 1, .has_hotplug = 1, > - .has_bsd_ring = 1, > - .has_blt_ring = 1, > - .has_llc = 1, > - .has_force_wake = 1, > + IVB_FEATURES, > }; > > static const struct intel_device_info intel_ivybridge_m_info = { > - .is_ivybridge = 1, .gen = 7, .is_mobile = 1, .num_pipes = 3, > - .need_gfx_hws = 1, .has_hotplug = 1, > - .has_fbc = 0, /* FBC is not enabled on Ivybridge > mobile yet */ > - .has_bsd_ring = 1, > - .has_blt_ring = 1, > - .has_llc = 1, > - .has_force_wake = 1, > + IVB_FEATURES, > + .is_mobile = 1, > }; > > static const struct intel_device_info intel_valleyview_m_info = { I noticed I can use this for VLV, and HSW as well. I've modified it locally. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 0/9] displayless PCH 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky ` (8 preceding siblings ...) 2013-03-13 18:21 ` [PATCH 9/9] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky @ 2013-03-13 18:58 ` Chris Wilson 2013-03-13 19:05 ` Ben Widawsky 9 siblings, 1 reply; 40+ messages in thread From: Chris Wilson @ 2013-03-13 18:58 UTC (permalink / raw) To: Ben Widawsky; +Cc: intel-gfx, Art Runyan, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 11:20:59AM -0700, Ben Widawsky wrote: > Certain fusing options allow customers to fuse off the South Display > which would result in hangs when reading/writing to those registers. In > addition, certain CPU display registers (like backlight) may actually > invoke writes to South Display. > > This patch series enables our code to run displayless which can allow > the GEN hardware to be an offload GPU and not have a display hooked up. > > This patch series hasn't been tested. A pre-cleaned version was tested, > and did work. I'm hoping Jeff (in the CC) can test this latest version. This mode of operation should be supported by xf86-video-intel-2.21.4. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 0/9] displayless PCH 2013-03-13 18:58 ` [PATCH 0/9] displayless PCH Chris Wilson @ 2013-03-13 19:05 ` Ben Widawsky 0 siblings, 0 replies; 40+ messages in thread From: Ben Widawsky @ 2013-03-13 19:05 UTC (permalink / raw) To: Chris Wilson, intel-gfx, Art Runyan, Mcallister, Jeffrey On Wed, Mar 13, 2013 at 06:58:06PM +0000, Chris Wilson wrote: > On Wed, Mar 13, 2013 at 11:20:59AM -0700, Ben Widawsky wrote: > > Certain fusing options allow customers to fuse off the South Display > > which would result in hangs when reading/writing to those registers. In > > addition, certain CPU display registers (like backlight) may actually > > invoke writes to South Display. > > > > This patch series enables our code to run displayless which can allow > > the GEN hardware to be an offload GPU and not have a display hooked up. > > > > This patch series hasn't been tested. A pre-cleaned version was tested, > > and did work. I'm hoping Jeff (in the CC) can test this latest version. > > This mode of operation should be supported by xf86-video-intel-2.21.4. > -Chris Yes, but for us it doesn't hard hang when we get it wrong, so I'd really like to get it tested on the real hardware. To a lesser extent, making sure the subvendor/subdevice matching works would also be nice. > > -- > Chris Wilson, Intel Open Source Technology Centre -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2013-03-20 16:58 UTC | newest] Thread overview: 40+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky 2013-03-13 18:21 ` [PATCH 1/9] drm/i915: Move num_pipes to intel info Ben Widawsky 2013-03-13 18:31 ` Chris Wilson 2013-03-13 18:36 ` Ben Widawsky 2013-03-13 21:05 ` [PATCH 01/10] [v2] " Ben Widawsky 2013-03-19 19:25 ` [PATCH 1/9] " Jesse Barnes 2013-03-19 23:06 ` Daniel Vetter 2013-03-13 18:21 ` [PATCH 2/9] drm/i915: Support PCH no display Ben Widawsky 2013-03-17 21:13 ` Daniel Vetter 2013-03-13 18:21 ` [PATCH 3/9] drm/i915: PCH_NOP Ben Widawsky 2013-03-17 21:05 ` Daniel Vetter 2013-03-17 21:14 ` Daniel Vetter 2013-03-13 18:21 ` [PATCH 4/9] drm/i915: Don't touch South display when PCH_NOP Ben Widawsky 2013-03-14 11:44 ` Jani Nikula 2013-03-14 15:55 ` [PATCH 04/10] [v2] drm/i915: Don't touch South Display " Ben Widawsky 2013-03-17 21:21 ` Daniel Vetter 2013-03-18 23:05 ` Ben Widawsky 2013-03-13 18:21 ` [PATCH 5/9] drm/i915: Don't initialize watermark stuff with PCH_NOP Ben Widawsky 2013-03-13 18:47 ` Chris Wilson 2013-03-13 21:06 ` [PATCH 05/10] [v2] " Ben Widawsky 2013-03-17 21:02 ` Daniel Vetter 2013-03-19 0:45 ` Ben Widawsky 2013-03-13 18:21 ` [PATCH 6/9] drm/i915: PCH_NOP suspend/resume Ben Widawsky 2013-03-17 21:28 ` Daniel Vetter 2013-03-19 0:51 ` Ben Widawsky 2013-03-19 7:51 ` Daniel Vetter 2013-03-19 17:53 ` Ben Widawsky 2013-03-13 18:21 ` [PATCH 7/9] drm/i915: Don't wait for PCH on reset Ben Widawsky 2013-03-19 19:36 ` Jesse Barnes 2013-03-20 16:43 ` [PATCH] " Ben Widawsky 2013-03-20 16:58 ` Jesse Barnes 2013-03-13 18:21 ` [PATCH 8/9] drm/i915: Set PCH_NOP Ben Widawsky 2013-03-17 21:36 ` Daniel Vetter 2013-03-13 18:21 ` [PATCH 9/9] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky 2013-03-13 18:52 ` Chris Wilson 2013-03-13 21:08 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky 2013-03-13 21:08 ` [PATCH 10/10] [v2] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky 2013-03-15 4:05 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky 2013-03-13 18:58 ` [PATCH 0/9] displayless PCH Chris Wilson 2013-03-13 19:05 ` Ben Widawsky
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.