Re: [Intel-gfx] [PATCH v2 0/7] xfree86: Handle drm race condition

[Maarten Lankhorst]

Hey,

Op 19-03-13 22:13, Chris Wilson schreef:
 On Tue, Mar 19, 2013 at 11:50:47AM +0100, Maarten Lankhorst wrote:
 The drmSetMaster call is needed, but the spinning is really just waiting for 
 the workqueue to run.

 bryce's patch never worked, it just caused it to try drmsetinterfaceversion 
 for a few seconds before timing out. That call
 was failing because his patch series never tried to obtain drm master.
 You missed that the series Bryce posted did contain the drmSetMaster()
 call inside the loop to retry drmSetVersion(). :)


Oh I must have missed that.

Is the drmSetInterfaceVersion call really needed here? If I look at 
DRM_IOCTL_GET_UNIQUE,
I don't see any requirement of drm master or anything, so it looks to me like 
for this specific race
the drmSetInterfaceVersion call can be skipped entirely without any side 
effects.
This would end up with cleaner code here, and drop the master requirement 
entirely.

Of course there's still a race that needs to be investigated, and is currently 
not completely understood, I think.

~Maarten

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

[Chris Wilson]

On Tue, Mar 19, 2013 at 08:19:56PM -0700, Ben Widawsky wrote:
 Change the gen6+ max delay if the pcode read was successful (not the
 inverse).
 
 The previous code was all sorts of wrong and has existed since I broke
 it:
 commit 42c0526c930523425ff6edc95b7235ce7ab9308d
 Author: Ben Widawsky b...@bwidawsk.net
 Date:   Wed Sep 26 10:34:00 2012 -0700
 
 drm/i915: Extract PCU communication
 
 I added some parentheses for clarity,

(Clarity)! ((They) ((were) (not)) (even) (connected) (to) ((the) (mistake))).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/10] drm/i915: Support PCH no display

[Ville Syrjälä]

On Fri, Mar 15, 2013 at 11:17:47AM -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 b...@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);

Wouldn't it be better to skip intel_setup_outputs() entirely? You could
then drop the intel_crt_init() hunk too.

 + } 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 

Re: [Intel-gfx] [PATCH v2 0/7] xfree86: Handle drm race condition

[Maarten Lankhorst]

Op 20-03-13 09:40, Maarten Lankhorst schreef:
 Hey,

 Op 19-03-13 22:13, Chris Wilson schreef:
 On Tue, Mar 19, 2013 at 11:50:47AM +0100, Maarten Lankhorst wrote:
 The drmSetMaster call is needed, but the spinning is really just waiting 
 for the workqueue to run.

 bryce's patch never worked, it just caused it to try drmsetinterfaceversion 
 for a few seconds before timing out. That call
 was failing because his patch series never tried to obtain drm master.
 You missed that the series Bryce posted did contain the drmSetMaster()
 call inside the loop to retry drmSetVersion(). :)


 Oh I must have missed that.

 Is the drmSetInterfaceVersion call really needed here? If I look at 
 DRM_IOCTL_GET_UNIQUE,
 I don't see any requirement of drm master or anything, so it looks to me like 
 for this specific race
 the drmSetInterfaceVersion call can be skipped entirely without any side 
 effects.
 This would end up with cleaner code here, and drop the master requirement 
 entirely.

 Of course there's still a race that needs to be investigated, and is 
 currently not completely understood, I think.

Or worse, is that drmGetBusId call there even useful? From digging at the 
kernel it seems it's a per master value.
So if a device is hotplugged, it wouldn't be set yet. If someone else holds 
master, it wouldn't be set either.
In fact it would only be ever set from DRIOpenDRMMaster, but that call only 
happens a lot later, if it even happens at all.

It seems to me like opening the fd there should be removed entirely, and the 
bus id should be retrieved from the udev event instead.

I'll try to get something working for this.

~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PULL] drm-intel-fixes

[Daniel Vetter]

Hi Dave,

Bunch of fixes, all pretty high-priority
- Fix execbuf argument checking (Kees Cook)
- Optionally obfuscate kernel addresses in dumps (Kees Cook)
- Two patches from Takashi Iwai to fix DP link training regressions he's
  seen.
- intel-gfx is no longer subscribers-only (well, just no longer moderated
  in an annoying way for non-subscribers), update MAINTAINERS
- gm45 gmbus irq fallout fix (Jiri Kosina)

Cheers, Daniel


The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9:

  Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~danvet/drm-intel drm-intel-fixes

for you to fetch changes up to c12aba5aa0e60b7947bc8b6ea25ef55c4acf81a4:

  drm/i915: stop using GMBUS IRQs on Gen4 chips (2013-03-20 00:03:16 +0100)


Daniel Vetter (1):
  MAINTAINERS: intel-gfx is no longer subscribers-only

Jiri Kosina (1):
  drm/i915: stop using GMBUS IRQs on Gen4 chips

Kees Cook (2):
  drm/i915: restrict kernel address leak in debugfs
  drm/i915: bounds check execbuffer relocation count

Takashi Iwai (2):
  Revert drm/i915: try to train DP even harder
  drm/i915: Use the fixed pixel clock for eDP in intel_dp_set_m_n()

 MAINTAINERS|2 +-
 drivers/gpu/drm/i915/i915_debugfs.c|2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   11 ---
 drivers/gpu/drm/i915/intel_dp.c|   14 --
 drivers/gpu/drm/i915/intel_i2c.c   |   11 ++-
 5 files changed, 32 insertions(+), 8 deletions(-)
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: fixup pd vs pt confusion in gen6 ppgtt code

[Paul Menzel]

Dear Daniel,


Am Dienstag, den 19.03.2013, 23:51 +0100 schrieb Daniel Vetter:
 The index variable points at a page table, not a page directory or a
 pde. Ben Widawsky fix this up correctly in his ppgtt cleanup, but I've

fix*ed*

 botched the job and copypasted the old confusion from the original
 gen6 ppgtt code in
 
 commit def886c3768d24c4e0aa56ff98b5a468c2b5c9bf
 Author: Daniel Vetter daniel.vet...@ffwll.ch
 Date:   Thu Jan 24 14:44:56 2013 -0800
 
 drm/i915: vfuncs for ppgtt
 
 Cc: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_gem_gtt.c |   14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

Acked-by: Paul Menzel paulepan...@users.sourceforge.net


Thanks,

Paul


signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: don't clear pfit at startup any more

[Mika Kuoppala]

Daniel Vetter daniel.vet...@ffwll.ch writes:

 Since

 commit 24a1f16de97c4cf0029d9acd04be06db32208726
 Author: Mika Kuoppala mika.kuopp...@linux.intel.com
 Date:   Fri Feb 8 16:35:37 2013 +0200

 drm/i915: disable shared panel fitter for pipe

 We clear the single panel fitter when disabling the pipe it's attached to, so 
 no
 need to additionally clear it when there's no lvds port detected. Since that
 alone isn't good enough e.g. when an external monitor is connected and the 
 bios
 uses the panel fitter on that output.

 v2: Remove the now unused has_lvds variable and drop the bool return
 value from intel_lvds_init, both suggest by Mika Kuoppala.

 Cc: Mika Kuoppala mika.kuopp...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com

 ---
  drivers/gpu/drm/i915/intel_display.c |7 +--
  drivers/gpu/drm/i915/intel_drv.h |2 +-
  drivers/gpu/drm/i915/intel_lvds.c|   20 ++--
  3 files changed, 12 insertions(+), 17 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index f73fc3d..00f3d0b 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -8335,13 +8335,8 @@ static void intel_setup_outputs(struct drm_device *dev)
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_encoder *encoder;
   bool dpd_is_edp = false;
 - bool has_lvds;
  
 - has_lvds = intel_lvds_init(dev);
 - if (!has_lvds  !HAS_PCH_SPLIT(dev)) {
 - /* disable the panel fitter on everything but LVDS */
 - I915_WRITE(PFIT_CONTROL, 0);
 - }
 + intel_lvds_init(dev);
  
   if (!(HAS_DDI(dev)  (I915_READ(DDI_BUF_CTL(PORT_A))  DDI_A_4_LANES)))
   intel_crt_init(dev);
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index e6f84d0..9e29223 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -459,7 +459,7 @@ extern void intel_tv_init(struct drm_device *dev);
  extern void intel_mark_busy(struct drm_device *dev);
  extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj);
  extern void intel_mark_idle(struct drm_device *dev);
 -extern bool intel_lvds_init(struct drm_device *dev);
 +extern void intel_lvds_init(struct drm_device *dev);
  extern bool intel_is_dual_link_lvds(struct drm_device *dev);
  extern void intel_dp_init(struct drm_device *dev, int output_reg,
 enum port port);
 diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
 b/drivers/gpu/drm/i915/intel_lvds.c
 index 6ff145f..5eb74de 100644
 --- a/drivers/gpu/drm/i915/intel_lvds.c
 +++ b/drivers/gpu/drm/i915/intel_lvds.c
 @@ -1037,7 +1037,7 @@ static bool intel_lvds_supported(struct drm_device *dev)
   * Create the connector, register the LVDS DDC bus, and try to figure out 
 what
   * modes we can display on the LVDS panel (if present).
   */
 -bool intel_lvds_init(struct drm_device *dev)
 +void intel_lvds_init(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_lvds_encoder *lvds_encoder;
 @@ -1055,35 +1055,35 @@ bool intel_lvds_init(struct drm_device *dev)
   u8 pin;
  
   if (!intel_lvds_supported(dev))
 - return false;
 + return;
  
   /* Skip init on machines we know falsely report LVDS */
   if (dmi_check_system(intel_no_lvds))
 - return false;
 + return;
  
   pin = GMBUS_PORT_PANEL;
   if (!lvds_is_present_in_vbt(dev, pin)) {
   DRM_DEBUG_KMS(LVDS is not present in VBT\n);
 - return false;
 + return;
   }
  
   if (HAS_PCH_SPLIT(dev)) {
   if ((I915_READ(PCH_LVDS)  LVDS_DETECTED) == 0)
 - return false;
 + return;
   if (dev_priv-edp.support) {
   DRM_DEBUG_KMS(disable LVDS for eDP support\n);
 - return false;
 + return;
   }
   }
  
   lvds_encoder = kzalloc(sizeof(struct intel_lvds_encoder), GFP_KERNEL);
   if (!lvds_encoder)
 - return false;
 + return;
  
   lvds_connector = kzalloc(sizeof(struct intel_lvds_connector), 
 GFP_KERNEL);
   if (!lvds_connector) {
   kfree(lvds_encoder);
 - return false;
 + return;
   }
  
   lvds_encoder-attached_connector = lvds_connector;
 @@ -1257,7 +1257,7 @@ out:
   intel_panel_init(intel_connector-panel, fixed_mode);
   intel_panel_setup_backlight(connector);
  
 - return true;
 + return;
  
  failed:
   DRM_DEBUG_KMS(No LVDS modes found, disabling.\n);
 @@ -1267,5 +1267,5 @@ failed:
   drm_mode_destroy(dev, fixed_mode);
   kfree(lvds_encoder);
   kfree(lvds_connector);
 - return false;
 + return;
  }
 -- 
 1.7.10.4

Re: [Intel-gfx] More fastboot bits

[Imre Deak]

On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
 This one adds some extra checks on top of Chris's last set:
   - check for panel fit modes when inheriting from the BIOS
   - update pfit state at pipe_set_base time

I missed this version of the patchset and reviewed the previous one :/
Sending it with a v2 subject prefix with a proper In-reply-to header
would've been nice, consider the high traffic on this list.

Some of the comments from that review are still valid, so I'll copy them
over.

 It also changes the mode set vs flip checking to include the non-fb case
 (e.g. if the BIOS fb was too small for the native mode), since we might
 still be able to flip in that case.
 
 Finally, it includes a clock_get routine for ilk+.  I'd appreciate if
 someone could test this out on a machine where VBIOS supports the native
 panel mode, so the kernel can boot from the boot loader in the native
 mode.  In that case, it should actually fastboot and avoid the whole
 mode set/panel power sequence.

My ilk doesn't seem to start with a native mode, so can't test it there.
Would it be possible to test this with reloading the module?

--Imre

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine

[Imre Deak]

On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
 From: Chris Wilson ch...@chris-wilson.co.uk
 
 This will be shared with wrapping the BIOS framebuffer into the fbdev
 later. In the meantime, we can tidy the code slightly and improve the
 error path handling.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/intel_display.c |7 --
  drivers/gpu/drm/i915/intel_drv.h |7 ++
  drivers/gpu/drm/i915/intel_fb.c  |  154 
 ++
  3 files changed, 91 insertions(+), 77 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index f20555e..dc58b01 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -6422,13 +6422,6 @@ intel_framebuffer_create(struct drm_device *dev,
  }
  
  static u32
 -intel_framebuffer_pitch_for_width(int width, int bpp)
 -{
 - u32 pitch = DIV_ROUND_UP(width * bpp, 8);
 - return ALIGN(pitch, 64);
 -}
 -
 -static u32
  intel_framebuffer_size_for_mode(struct drm_display_mode *mode, int bpp)
  {
   u32 pitch = intel_framebuffer_pitch_for_width(mode-hdisplay, bpp);
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 005a91f..f93653d 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -134,6 +134,13 @@ struct intel_framebuffer {
   struct drm_i915_gem_object *obj;
  };
  
 +inline static u32
 +intel_framebuffer_pitch_for_width(int width, int bpp)
 +{
 + u32 pitch = DIV_ROUND_UP(width * bpp, 8);
 + return ALIGN(pitch, 64);
 +}
 +
  struct intel_fbdev {
   struct drm_fb_helper helper;
   struct intel_framebuffer ifb;
 diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
 index 1c510da..5afc31b 100644
 --- a/drivers/gpu/drm/i915/intel_fb.c
 +++ b/drivers/gpu/drm/i915/intel_fb.c
 @@ -57,29 +57,96 @@ static struct fb_ops intelfb_ops = {
   .fb_debug_leave = drm_fb_helper_debug_leave,
  };
  
 +static struct fb_info *intelfb_create_info(struct intel_fbdev *ifbdev)
 +{
 + struct drm_framebuffer *fb = ifbdev-ifb.base;
 + struct drm_device *dev = fb-dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + struct fb_info *info;
 + u32 gtt_offset, size;
 + int ret;
 +
 + info = framebuffer_alloc(0, dev-pdev-dev);
 + if (!info)
 + return NULL;
 +
 + info-par = ifbdev;
 + ifbdev-helper.fb = fb;
 + ifbdev-helper.fbdev = info;
 +
 + strcpy(info-fix.id, inteldrmfb);
 +
 + info-flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 + info-fbops = intelfb_ops;
 +
 + ret = fb_alloc_cmap(info-cmap, 256, 0);
 + if (ret)
 + goto err_info;
 +
 + /* setup aperture base/size for vesafb takeover */
 + info-apertures = alloc_apertures(1);
 + if (!info-apertures)
 + goto err_cmap;
 +
 + info-apertures-ranges[0].base = dev-mode_config.fb_base;
 + info-apertures-ranges[0].size = dev_priv-gtt.mappable_end;
 +
 + gtt_offset = ifbdev-ifb.obj-gtt_offset;
 + size = ifbdev-ifb.obj-base.size;
 +
 + info-fix.smem_start = dev-mode_config.fb_base + gtt_offset;
 + info-fix.smem_len = size;
 +
 + info-screen_size = size;
 + info-screen_base = ioremap_wc(dev_priv-gtt.mappable_base + gtt_offset,
 +size);
 + if (!info-screen_base)

kfree(info-apertures) is missing. The same goes for
intel_fbdev_destroy().

 + goto err_cmap;
 +
 + /* If the object is shmemfs backed, it will have given us zeroed pages.
 +  * If the object is stolen however, it will be full of whatever
 +  * garbage was left in there.
 +  */
 + if (ifbdev-ifb.obj-stolen)
 + memset_io(info-screen_base, 0, info-screen_size);
 +
 + /* Use default scratch pixmap (info-pixmap.flags = FB_PIXMAP_SYSTEM) */
 +
 + drm_fb_helper_fill_fix(info, fb-pitches[0], fb-depth);
 + drm_fb_helper_fill_var(info, ifbdev-helper, fb-width, fb-height);
 +
 + return info;
 +
 +err_cmap:
 + if (info-cmap.len)
 + fb_dealloc_cmap(info-cmap);

Should be fine to call w/o checking cmap.len.

 +err_info:
 + framebuffer_release(info);
 + return NULL;
 +}
 +
  static int intelfb_create(struct intel_fbdev *ifbdev,
 struct drm_fb_helper_surface_size *sizes)
  {
   struct drm_device *dev = ifbdev-helper.dev;
 - struct drm_i915_private *dev_priv = dev-dev_private;
 - struct fb_info *info;
 - struct drm_framebuffer *fb;
 - struct drm_mode_fb_cmd2 mode_cmd = {};
 + struct drm_mode_fb_cmd2 mode_cmd = { 0 };
   struct drm_i915_gem_object *obj;
 - struct device *device = dev-pdev-dev;
 + struct fb_info *info;
   int size, ret;
  
   /* we don't do packed 24bpp */
   if (sizes-surface_bpp == 24)
   sizes-surface_bpp = 32;
  
 - mode_cmd.width = 

Re: [Intel-gfx] [PATCH 05/13] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon

[Imre Deak]

On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
 From: Chris Wilson ch...@chris-wilson.co.uk
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/i915_dma.c  |8 +-
  drivers/gpu/drm/i915/i915_drv.h  |2 +-
  drivers/gpu/drm/i915/intel_display.c |   14 +-
  drivers/gpu/drm/i915/intel_drv.h |4 +
  drivers/gpu/drm/i915/intel_fb.c  |  305 
 +++---
  5 files changed, 306 insertions(+), 27 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 4fa6beb..f2b7db7 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1273,6 +1273,7 @@ static const struct vga_switcheroo_client_ops 
 i915_switcheroo_ops = {
  static int i915_load_modeset_init(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 + bool was_vga_enabled;
   int ret;
  
   ret = intel_parse_bios(dev);
 @@ -1309,7 +1310,11 @@ static int i915_load_modeset_init(struct drm_device 
 *dev)
  
   /* Important: The output setup functions called by modeset_init need
* working irqs for e.g. gmbus and dp aux transfers. */
 - intel_modeset_init(dev);
 + intel_modeset_init(dev, was_vga_enabled);
 +
 + /* Wrap existing BIOS mode configuration prior to GEM takeover */
 + if (!was_vga_enabled)
 + intel_fbdev_init_bios(dev);
  
   ret = i915_gem_init(dev);
   if (ret)
 @@ -1323,6 +1328,7 @@ static int i915_load_modeset_init(struct drm_device 
 *dev)
   /* FIXME: do pre/post-mode set stuff in core KMS code */
   dev-vblank_disable_allowed = 1;
  
 + /* Install a default KMS/GEM fbcon if we failed to wrap the BIOS fb */
   ret = intel_fbdev_init(dev);
   if (ret)
   goto cleanup_gem;
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 9b5478f..30cf7e6 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1811,7 +1811,7 @@ static inline void intel_unregister_dsm_handler(void) { 
 return; }
  
  /* modesetting */
  extern void intel_modeset_init_hw(struct drm_device *dev);
 -extern void intel_modeset_init(struct drm_device *dev);
 +extern void intel_modeset_init(struct drm_device *dev, bool 
 *was_vga_enabled);
  extern void intel_modeset_gem_init(struct drm_device *dev);
  extern void intel_modeset_cleanup(struct drm_device *dev);
  extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index dc58b01..9793e66 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -8735,12 +8735,17 @@ static void intel_init_quirks(struct drm_device *dev)
  }
  
  /* Disable the VGA plane that we never use */
 -static void i915_disable_vga(struct drm_device *dev)
 +static bool i915_disable_vga(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 + bool was_enabled;
   u8 sr1;
   u32 vga_reg = i915_vgacntrl_reg(dev);
  
 + was_enabled = !(I915_READ(vga_reg)  VGA_DISP_DISABLE);
 + DRM_DEBUG_KMS(VGA output is currently %s\n,
 +   was_enabled ? enabled : disabled);
 +
   vga_get_uninterruptible(dev-pdev, VGA_RSRC_LEGACY_IO);
   outb(SR01, VGA_SR_INDEX);
   sr1 = inb(VGA_SR_DATA);
 @@ -8750,6 +8755,8 @@ static void i915_disable_vga(struct drm_device *dev)
  
   I915_WRITE(vga_reg, VGA_DISP_DISABLE);
   POSTING_READ(vga_reg);
 +
 + return was_enabled;
  }
  
  void intel_modeset_init_hw(struct drm_device *dev)
 @@ -8765,7 +8772,8 @@ void intel_modeset_init_hw(struct drm_device *dev)
   mutex_unlock(dev-struct_mutex);
  }
  
 -void intel_modeset_init(struct drm_device *dev)
 +void intel_modeset_init(struct drm_device *dev,
 + bool *was_vga_enabled)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
   int i, ret;
 @@ -8812,7 +8820,7 @@ void intel_modeset_init(struct drm_device *dev)
   intel_pch_pll_init(dev);
  
   /* Just disable it once at startup */
 - i915_disable_vga(dev);
 + *was_vga_enabled = i915_disable_vga(dev);
   intel_setup_outputs(dev);
  
   /* Just in case the BIOS is doing something questionable. */
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index f93653d..9cf794f 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -146,6 +146,8 @@ struct intel_fbdev {
   struct intel_framebuffer ifb;
   struct list_head fbdev_list;
   struct drm_display_mode *our_mode;
 + bool stolen;
 + int preferred_bpp;
  };
  
  struct intel_encoder {
 @@ -212,6 +214,7 @@ struct intel_crtc {
   enum plane plane;
   enum transcoder cpu_transcoder;
   u8 lut_r[256], 

Re: [Intel-gfx] [PATCH 06/13] drm/i915: Retrieve the current mode upon KMS takeover

[Imre Deak]

On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
 From: Chris Wilson ch...@chris-wilson.co.uk
 
 Read the current hardware state to retrieve the active mode and populate
 our CRTC config if that mode matches our presumptions.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/i915_drv.h  |2 +
  drivers/gpu/drm/i915/intel_crt.c |   27 +++-
  drivers/gpu/drm/i915/intel_display.c |  119 
 ++
  drivers/gpu/drm/i915/intel_dp.c  |   22 +++
  drivers/gpu/drm/i915/intel_drv.h |7 +-
  drivers/gpu/drm/i915/intel_dvo.c |   36 ++
  drivers/gpu/drm/i915/intel_hdmi.c|   22 +++
  drivers/gpu/drm/i915/intel_lvds.c|   27 +++-
  drivers/gpu/drm/i915/intel_sdvo.c|   23 +++
  9 files changed, 242 insertions(+), 43 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 30cf7e6..8473db4 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -283,6 +283,8 @@ struct drm_i915_display_funcs {
   void (*update_linetime_wm)(struct drm_device *dev, int pipe,
struct drm_display_mode *mode);
   void (*modeset_global_resources)(struct drm_device *dev);
 + bool (*crtc_get_mode)(struct drm_crtc *crtc,
 +  struct drm_display_mode *mode);
   int (*crtc_mode_set)(struct drm_crtc *crtc,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
 diff --git a/drivers/gpu/drm/i915/intel_crt.c 
 b/drivers/gpu/drm/i915/intel_crt.c
 index cfc9687..f1d68e8 100644
 --- a/drivers/gpu/drm/i915/intel_crt.c
 +++ b/drivers/gpu/drm/i915/intel_crt.c
 @@ -81,6 +81,27 @@ static bool intel_crt_get_hw_state(struct intel_encoder 
 *encoder,
   return true;
  }
  
 +static unsigned intel_crt_get_mode_flags(struct intel_encoder *encoder)
 +{
 + struct drm_i915_private *dev_priv = encoder-base.dev-dev_private;
 + struct intel_crt *crt = intel_encoder_to_crt(encoder);
 + u32 tmp, flags = 0;
 +
 + tmp = I915_READ(crt-adpa_reg);
 +
 + if (tmp  ADPA_HSYNC_ACTIVE_HIGH)
 + flags |= DRM_MODE_FLAG_PHSYNC;
 + else
 + flags |= DRM_MODE_FLAG_NHSYNC;
 +
 + if (tmp  ADPA_VSYNC_ACTIVE_HIGH)
 + flags |= DRM_MODE_FLAG_PVSYNC;
 + else
 + flags |= DRM_MODE_FLAG_NVSYNC;
 +
 + return flags;
 +}
 +
  static void intel_disable_crt(struct intel_encoder *encoder)
  {
   struct drm_i915_private *dev_priv = encoder-base.dev-dev_private;
 @@ -777,10 +798,12 @@ void intel_crt_init(struct drm_device *dev)
  
   crt-base.disable = intel_disable_crt;
   crt-base.enable = intel_enable_crt;
 - if (HAS_DDI(dev))
 + if (HAS_DDI(dev)) {
   crt-base.get_hw_state = intel_ddi_get_hw_state;
 - else
 + } else {
   crt-base.get_hw_state = intel_crt_get_hw_state;
 + crt-base.get_mode_flags = intel_crt_get_mode_flags;
 + }
   intel_connector-get_hw_state = intel_connector_get_hw_state;
  
   drm_encoder_helper_add(crt-base.base, crt_encoder_funcs);
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 9793e66..e19b637 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -6618,11 +6618,12 @@ void intel_release_load_detect_pipe(struct 
 drm_connector *connector,
  }
  
  /* Returns the clock of the currently programmed mode of the given pipe. */
 -static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc 
 *crtc)
 +static int i9xx_crtc_clock_get(struct drm_crtc *crtc)
  {
 + struct drm_device *dev = crtc-dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 - int pipe = intel_crtc-pipe;
 + enum pipe pipe = intel_crtc-pipe;
   u32 dpll = I915_READ(DPLL(pipe));
   u32 fp;
   intel_clock_t clock;
 @@ -6705,35 +6706,84 @@ static int intel_crtc_clock_get(struct drm_device 
 *dev, struct drm_crtc *crtc)
  }
  
  /** Returns the currently programmed mode of the given pipe. */
 -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 -  struct drm_crtc *crtc)
 +static bool i9xx_crtc_get_mode(struct drm_crtc *crtc,
 +struct drm_display_mode *mode)
  {
 - struct drm_i915_private *dev_priv = dev-dev_private;
 + struct drm_i915_private *dev_priv = crtc-dev-dev_private;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
   enum transcoder cpu_transcoder = intel_crtc-cpu_transcoder;
 - struct drm_display_mode *mode;
 - int htot = I915_READ(HTOTAL(cpu_transcoder));
 - int hsync = I915_READ(HSYNC(cpu_transcoder));
 - int vtot = I915_READ(VTOTAL(cpu_transcoder));
 - int vsync = 

Re: [Intel-gfx] [PATCH 09/13] drm/i915: fix build in intel_display.c

[Imre Deak]

On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
 Missing a curly brace.

Should be merged into 6/13.

 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_display.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 0a2279e..595590c 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -9163,7 +9163,7 @@ void intel_modeset_setup_hw_state(struct drm_device 
 *dev,
   if (crtc-mode_valid  encoder-get_mode_flags)
   crtc-base.mode.flags |= 
 encoder-get_mode_flags(encoder);
   encoder-base.crtc = crtc-base;
 - } else
 + } else {
   encoder-base.crtc = NULL;
   }
  


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/13] drm/i915: check panel fit status at update_plane time

[Imre Deak]

On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
 We may need to disable the panel when flipping to a new buffer, so check
 the state here and zero it out if needed, otherwise leave it alone.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_display.c |   19 +++
  1 file changed, 19 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 595590c..91660b1 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2304,6 +2304,25 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int 
 y,
   if (crtc-fb)
   intel_finish_fb(crtc-fb);
  
 + I915_WRITE(PIPESRC(intel_crtc-pipe),
 +((crtc-mode.hdisplay - 1)  16) |
 +(crtc-mode.vdisplay - 1));
 + if (!dev_priv-pch_pf_size 
 + (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
 +  intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
 + /* Force use of hard-coded filter coefficients
 +  * as some pre-programmed values are broken,
 +  * e.g. x201.
 +  */

Nitpick: multiline comments should start with an empty /* line.

 + if (IS_IVYBRIDGE(dev))
 + I915_WRITE(PF_CTL(intel_crtc-pipe), 0);
 + else
 + I915_WRITE(PF_CTL(intel_crtc-pipe), 0);

No difference in the branches?

 + I915_WRITE(PF_WIN_POS(intel_crtc-pipe), 0);
 + I915_WRITE(PF_WIN_SZ(intel_crtc-pipe), 0);
 + }
 +
 +
   ret = dev_priv-display.update_plane(crtc, fb, x, y);
   if (ret) {
   intel_unpin_fb_obj(to_intel_framebuffer(fb)-obj);


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/13] drm/i915: add clock_get for ironlake+

[Imre Deak]

On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
 Turns out it's easy to get the clock, though it may correspond to a
 potential pfit mode.  In that case, we may still be able to flip if
 we can get the native mode params somehow.

This should be merged to 6/13.

 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_display.c |   26 +++---
  1 file changed, 23 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 91660b1..861af1a 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -6771,6 +6771,27 @@ static bool i9xx_crtc_get_mode(struct drm_crtc *crtc,
   return true;
  }
  
 +static int ironlake_crtc_clock_get(struct drm_crtc *crtc)
 +{
 + struct drm_i915_private *dev_priv = crtc-dev-dev_private;
 + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 + enum transcoder cpu_transcoder = intel_crtc-cpu_transcoder;
 + int clock;
 + u32 link_m;
 +
 + /*
 +  * PCH platforms make this easy: we can just use the LINK_M1 reg.
 +  * Note: this may be the pixel clock for a fitted mode, in which
 +  * case it won't match the native mode clock.  That means we won't be
 +  * able to do a simple flip in the fastboot case.
 +  */
 + link_m = I915_READ(PIPE_LINK_M1(cpu_transcoder));
 +
 + clock = link_m;
 +
 + return clock;
 +}

Could be simply return I915_READ(PIPE_LINK_M1(cpu_transcoder));

 +
  static bool ironlake_crtc_get_mode(struct drm_crtc *crtc,
  struct drm_display_mode *mode)
  {
 @@ -6797,12 +6818,11 @@ static bool ironlake_crtc_get_mode(struct drm_crtc 
 *crtc,
   mode-vsync_start = (tmp  0x) + 1;
   mode-vsync_end = ((tmp  0x)  16) + 1;
  
 - //mode-clock = i9xx_crtc_clock_get(crtc);
 - //mode-clock = 69300;
 + mode-clock = ironlake_crtc_clock_get(crtc);
  
   drm_mode_set_name(mode);
  
 - return false; /* XXX mode-clock unset */
 + return true;
  }
  
  static __maybe_unused bool no_crtc_get_mode(struct drm_crtc *crtc,


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 13/13] drm/i915: check for non-native modes when inheriting a BIOS fb

[Imre Deak]

On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
 If the mode is non-native using the panel fitter, don't try to re-use
 the fb the BIOS allocated for it.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_fb.c |   12 
  1 file changed, 12 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
 index b60f277..9ff12aa 100644
 --- a/drivers/gpu/drm/i915/intel_fb.c
 +++ b/drivers/gpu/drm/i915/intel_fb.c
 @@ -438,6 +438,18 @@ void intel_fbdev_init_bios(struct drm_device *dev)
   width = ((val  16)  0xfff) + 1;
   height = ((val  0)  0xfff) + 1;
  
 + /* Don't bother inheriting panel fitted modes */
 + val = I915_READ(HTOTAL(pipe));
 + if (((val  0x) + 1) != width) {
 + DRM_ERROR(BIOS fb not native width (%d vs %d), 
 skipping\n, width, (val  0x) + 1);
 + continue;
 + }
 + val = I915_READ(VTOTAL(pipe));
 + if (((val  0x) + 1) != height) {
 + DRM_ERROR(BIOS fb not native width (%d vs %d), 
 skipping\n, height, (val  0x) + 1);

s/width/height/

 + continue;
 + }
 +
   DRM_DEBUG_KMS(Found active pipe [%d/%d]: size=%dx%d@%d, 
 offset=%x\n,
 pipe, plane, width, height, bpp, offset);
  

On the series with my comments and the note that I couldn't yet test it:
Reviewed-by: Imre Deak imre.d...@intel.com


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] Issue after drm/i915: prefer wide slow to fast narrow in DP configs.

[Michal Srb]

Hi,

I am debugging an issue with IBM POS machine (4852-570, Truman), with 
8086:0102 Intel Sandybridge graphic card and internal monitor connected over 
display port.

The issue is that sporadically, after mode change, the monitor remains blank.
There is no difference in drm.debug=0xe level log or content of debugfs/dri 
between the state when monitor displays image and the state when it remains 
blank.

(The monitor supports only 1024x768, so every mode change is into this 
resolution. It happens when the intel driver switches resolution during boot, 
when X starts, X ends, modetest starts or modetest ends.)

I've bisected the cause to this commit:

  commit 2514bc510d0c3aadcc5204056bb440fa36845147
  Author: Jesse Barnes jbar...@virtuousgeek.org
  Date:   Thu Jun 21 15:13:50 2012 -0700

  drm/i915: prefer wide  slow to fast  narrow in DP configs

  High frequency link configurations have the potential to cause trouble
  with long and/or cheap cables, so prefer slow and wide configurations
  instead.  This patch has the potential to cause trouble for eDP
  configurations that lie about available lanes, so if we run into that we
  can make it conditional on eDP.


The machine reports to have 2 lanes available and that seems to be correct.
One lane was used before the commit and the screen used to show picture after 
every mode change:

  [drm:intel_dp_mode_fixup], DP ink computation with max lane count 2 max bw 
0a pixel clock 65000KHz
  [drm:intel_dp_mode_fixup], DP link bw 0a lane count 1 clock 27 bpp 24
  [drm:intel_dp_mode_fixup], DP link bw required 156000 available 216000

Two lanes are used after the commit and the screen occasionally remains blank 
after mode change:

  [drm:intel_dp_mode_fixup], DP link computation with max lane count 2 max bw 
0a pixel clock 65000KHz
  [drm:intel_dp_mode_fixup], DP link bw 06 lane count 2 clock 162000 bpp 24
  [drm:intel_dp_mode_fixup], DP link bw required 156000 available 259200


Could you advise me where to look further? Could this be a hardware bug? 
Reverting the commit works as a workaround, but doesn't look like correct way 
to fix this...

Thank you,
Michal Srb

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 0/7] xfree86: Handle drm race condition

[Chris Wilson]

On Wed, Mar 20, 2013 at 09:40:04AM +0100, Maarten Lankhorst wrote:
 Is the drmSetInterfaceVersion call really needed here? If I look at 
 DRM_IOCTL_GET_UNIQUE,
 I don't see any requirement of drm master or anything, so it looks to me like 
 for this specific race
 the drmSetInterfaceVersion call can be skipped entirely without any side 
 effects.
 This would end up with cleaner code here, and drop the master requirement 
 entirely.

Indeed, it does look like drmSetVersion() at that point is overkill.
Instead we will hit the race later in the drivers. For the purposes of
clearer code, we could happily lose that drmSetVersion().
 
 Of course there's still a race that needs to be investigated, and is 
 currently not completely understood, I think.

We are all in agreement. Ultimately we want to root cause the race, in
the meantime we need a fallback to make sure that no desktop is left
behind!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Kill a strange comment about DPMS functions

[ville . syrjala]

From: Ville Syrjälä ville.syrj...@linux.intel.com

This comment looks like some historical leftover. Get rid of it.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b3b22d7..ec26a85 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8600,7 +8600,6 @@ static void intel_init_display(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
 
-   /* We always want a DPMS function */
if (HAS_DDI(dev)) {
dev_priv-display.crtc_mode_set = haswell_crtc_mode_set;
dev_priv-display.crtc_enable = haswell_crtc_enable;
-- 
1.8.1.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] More fastboot bits

[Jesse Barnes]

On Wed, 20 Mar 2013 14:15:32 +0200
Imre Deak imre.d...@intel.com wrote:

 On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
  This one adds some extra checks on top of Chris's last set:
- check for panel fit modes when inheriting from the BIOS
- update pfit state at pipe_set_base time
 
 I missed this version of the patchset and reviewed the previous one :/
 Sending it with a v2 subject prefix with a proper In-reply-to header
 would've been nice, consider the high traffic on this list.
 
 Some of the comments from that review are still valid, so I'll copy them
 over.

Great, thanks.  Sorry about the confusion, reposts like this are a bad
habit from a lower volume mailing list. :)  I'll endeavor to improve
myself.

 
  It also changes the mode set vs flip checking to include the non-fb case
  (e.g. if the BIOS fb was too small for the native mode), since we might
  still be able to flip in that case.
  
  Finally, it includes a clock_get routine for ilk+.  I'd appreciate if
  someone could test this out on a machine where VBIOS supports the native
  panel mode, so the kernel can boot from the boot loader in the native
  mode.  In that case, it should actually fastboot and avoid the whole
  mode set/panel power sequence.
 
 My ilk doesn't seem to start with a native mode, so can't test it there.
 Would it be possible to test this with reloading the module?

Yeah, that should also work, but I definitely haven't tested it as much.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Issue after drm/i915: prefer wide slow to fast narrow in DP configs.

[Jesse Barnes]

On Wed, 20 Mar 2013 14:45:52 +0100
Michal Srb m...@suse.com wrote:

 Hi,
 
 I am debugging an issue with IBM POS machine (4852-570, Truman), with 
 8086:0102 Intel Sandybridge graphic card and internal monitor connected over 
 display port.
 
 The issue is that sporadically, after mode change, the monitor remains blank.
 There is no difference in drm.debug=0xe level log or content of debugfs/dri 
 between the state when monitor displays image and the state when it remains 
 blank.
 
 (The monitor supports only 1024x768, so every mode change is into this 
 resolution. It happens when the intel driver switches resolution during boot, 
 when X starts, X ends, modetest starts or modetest ends.)
 
 I've bisected the cause to this commit:
 
   commit 2514bc510d0c3aadcc5204056bb440fa36845147
   Author: Jesse Barnes jbar...@virtuousgeek.org
   Date:   Thu Jun 21 15:13:50 2012 -0700
 
   drm/i915: prefer wide  slow to fast  narrow in DP configs
 
   High frequency link configurations have the potential to cause trouble
   with long and/or cheap cables, so prefer slow and wide configurations
   instead.  This patch has the potential to cause trouble for eDP
   configurations that lie about available lanes, so if we run into that we
   can make it conditional on eDP.
 
 
 The machine reports to have 2 lanes available and that seems to be correct.
 One lane was used before the commit and the screen used to show picture after 
 every mode change:
 
   [drm:intel_dp_mode_fixup], DP ink computation with max lane count 2 max bw 
 0a pixel clock 65000KHz
   [drm:intel_dp_mode_fixup], DP link bw 0a lane count 1 clock 27 bpp 24
   [drm:intel_dp_mode_fixup], DP link bw required 156000 available 216000
 
 Two lanes are used after the commit and the screen occasionally remains blank 
 after mode change:
 
   [drm:intel_dp_mode_fixup], DP link computation with max lane count 2 max bw 
 0a pixel clock 65000KHz
   [drm:intel_dp_mode_fixup], DP link bw 06 lane count 2 clock 162000 bpp 24
   [drm:intel_dp_mode_fixup], DP link bw required 156000 available 259200
 
 
 Could you advise me where to look further? Could this be a hardware bug? 
 Reverting the commit works as a workaround, but doesn't look like correct way 
 to fix this...

There's nothing else in the log about aux failures?  Just that the
training apparently succeeded but you ended up with a blank screen?

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Set the VIC in AVI infoframe for SDVO

[ville . syrjala]

From: Ville Syrjälä ville.syrj...@linux.intel.com

We do this for HDMI already, so I don't know why we wouldn't do
it for SDVO as well.

This is completely untested due to lack of hardware.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_sdvo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c 
b/drivers/gpu/drm/i915/intel_sdvo.c
index 5300526..678c47c 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -965,6 +965,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo 
*intel_sdvo,
avi_if.body.avi.ITC_EC_Q_SC |= 
DIP_AVI_RGB_QUANT_RANGE_FULL;
}
 
+   avi_if.body.avi.VIC = drm_match_cea_mode(adjusted_mode);
+
intel_dip_infoframe_csum(avi_if);
 
/* sdvo spec says that the ecc is handled by the hw, and it looks like
-- 
1.8.1.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

[Jesse Barnes]

On Tue, 19 Mar 2013 20:19:56 -0700
Ben Widawsky b...@bwidawsk.net wrote:

 Change the gen6+ max delay if the pcode read was successful (not the
 inverse).
 
 The previous code was all sorts of wrong and has existed since I broke
 it:
 commit 42c0526c930523425ff6edc95b7235ce7ab9308d
 Author: Ben Widawsky b...@bwidawsk.net
 Date:   Wed Sep 26 10:34:00 2012 -0700
 
 drm/i915: Extract PCU communication
 
 I added some parentheses for clarity, and I also corrected the debug
 message message to use the mask (wrong before I came along) and added a
 print to show the value we're changing from.
 
 Looking over the code, I'm not actually sure what we're trying to do. I
 introduced the bug simply by extracting the function not implementing
 anything new. We already set max_delay based on the capabilities
 register (which is what we use elsewhere to determine min and max).
 This would potentially increase it, I suppose? Jesse, I can't find the
 document which explains the definitions of the pcode commands, maybe you
 have it around.
 
 Based on Jesse's response, this could potentially be for -fixes, or
 stable, or maybe lead to us dropping it entirely. As the current code is
 is, things won't completely break because of the aforementioned
 capabilities register, and in my experimentation, enabling this has no
 effect, it goes from 1100-1100.
 
 I found this while reviewing Jesse's VLV patches.
 
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 ---
  drivers/gpu/drm/i915/intel_pm.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index c30e89a..86729b1 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device *dev)
   if (!ret) {
   pcu_mbox = 0;
   ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, 
 pcu_mbox);
 - if (ret  pcu_mbox  (131)) { /* OC supported */
 + if (!ret  (pcu_mbox  (131))) { /* OC supported */
 + DRM_DEBUG_DRIVER(overclocking supported, adjusting 
 frequency max from %dMHz to %dMHz\n,
 +  ((dev_priv-rps.max_delay  0xff) * 
 50),
 +  ((pcu_mbox  0xff) * 50));
   dev_priv-rps.max_delay = pcu_mbox  0xff;
 - DRM_DEBUG_DRIVER(overclocking supported, adjusting 
 frequency max to %dMHz\n, pcu_mbox * 50);
   }
   } else {
   DRM_DEBUG_DRIVER(Failed to set the min frequency\n);

Yeah looks ok.  I don't think I've seen a system where this flag gets
set, but according to the docs this is the right thing to do.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/20] drm/i915: add constant alpha support to sprite ioctl

[Ville Syrjälä]

On Tue, Mar 19, 2013 at 10:57:25AM -0700, Jesse Barnes wrote:
 On Tue, 19 Mar 2013 09:42:56 +0100
 Daniel Vetter dan...@ffwll.ch wrote:
   --- a/include/uapi/drm/i915_drm.h
   +++ b/include/uapi/drm/i915_drm.h
   @@ -949,6 +949,7 @@ struct drm_intel_overlay_attrs {
#define I915_SET_COLORKEY_NONE   (10) /* disable color key 
   matching */
#define I915_SET_COLORKEY_DESTINATION(11)
#define I915_SET_COLORKEY_SOURCE (12)
   +#define I915_SET_COLORKEY_ALPHA  (13)
  
  We've added this driver-private ioctl before attributes everywhere was
  possible. I think we need to convert that before adding more stuff ...
  
  Also some discussion on dri-devel about a somewhat standardized set of
  plane blending properties can't hurt.
  
  Yes, I know that this will lead to a massive dri-devel bikeshed ;-)
 
 Yeah we need some properties for this to fit in with the atomic bits.
 This patch actually pre-dates that by a bit (it's very old)...
 
 I'm sure Ville has ideas on what he'd like to see.

Yeah, I've been pondering about this kind of stuff.

One idea was that we might want to make the props for
color key/mask, background color, const alpha etc. always use 16bpc,
and then we can just drop the least significant bits if the HW uses
less precision. And also fix the channel order to some common standard.
That might make it a bit easier to write generic user space code.

But I'm not sure how to deal w/ RGB vs. YCbCr. Sometimes you may
need to feed hardware RGB values, sometimes YCbCr.

Options:
1. use one prop but the driver will interpret the values as RGB or
   YCbCr as needed. Userspace just has to figure out what to stuff
   into prop somehow.
2. one prop for RGB, another one for YCbCr. If the hw has need for
   both, it'll pick the right one. Again userspace just has to figure
   out which one it should use at a given time
3. standardize on RGB and convert to YCbCr in the driver as needed


As for the per-pixel alpha, I'm not sure if relying on the pixel
format alone is the best idea. At least we need a way to tell the
premult and non-premult cases apart. But maybe we want to even allow
disabling per-pixel alpha for ARGB formats, so that we don't need
to create an XRGB fb for the same data. So maybe an enum prop w/
no per-pixel alpha, pre-mult, non-premult options.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 15/20] drm/i915: turbo RC6 support for VLV v2

[Ben Widawsky]

On Tue, Mar 19, 2013 at 03:35:55PM -0700, Jesse Barnes wrote:
 On Tue, 19 Mar 2013 15:27:36 -0700
 Ben Widawsky b...@bwidawsk.net wrote:
 
  On Fri, Mar 08, 2013 at 10:45:58AM -0800, Jesse Barnes wrote:
   From: Ben Widawsky b...@bwidawsk.net
   
   Uses slightly different interfaces than other platforms.
   
   v2: track actual set freq, not requested (Rohit)
   fix debug prints in init code (Jesse)
   
   Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
  
  One important question is how is our punit code going to interact with
  the other potential users? It seems a bunch of power management drivers
  would also want to touch this uC.
 
 Pretty sure the PUnit has to deal with concurrent access... if not
 we'll have to add common routines for all drivers to use and do proper
 locking.

I don't see how it could unless there are extra data/address/command
registers for simultaneous access. Anyway, I don't think you need to
change anything, just something we need to notify the other people
writing code about. Also something we need to keep in mind if things
mysteriously blow up at some point.

 
   +
   + valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, pval);
   + if ((pval  8) != val)
   + DRM_DEBUG_DRIVER(punit overrode freq: %d requested, but got 
   %d\n,
   +   val, pval  8);
  I'm debating whether this is a useful thing to do here... You either get
  the frequency or you don't. Really it would seem more useful to me to
  check things are sane when you first enter the function (ie. did the
  last request do what you want). But I don't care what you end up with.
 
 It's not really a matter of sanity, it's more about what state the
 platform is in.  If the Punit decides things are getting too hot for
 example, it may clamp your freq down.  That's totally ok and normal
 though, and may change in future calls.

I agree, but I wasn't referring tot his part of the hunk, and you
snipped the part I was referring to, so adding it back:

+   valleyview_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+
+   do {
+   valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, pval);
+   if (time_after(jiffies, timeout)) {
+   DRM_DEBUG_DRIVER(timed out waiting for Punit\n);
+   break;
+   }
+   udelay(10);
+   } while (pval  1);

This is what seems unnecessary to me.

 
  I suppose if you wanted to make things a bit cleaner you could extract
  just the frequency setting part, since most of this function is
  identical to gen6 set rps.
 
 I was afraid more changes would creep in over time, but yeah that's a
 possibility.  I have a couple more changes here before I consider that.
 

It won't effect my adding the r-b, just that I noticed it because it
would have made review a bit easier ;-)

   +
   + /* Make sure we continue to get interrupts
   +  * until we hit the minimum or maximum frequencies.
   +  */
   + I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, limits);
   +
   + dev_priv-rps.cur_delay = pval  8;
   +
   + trace_intel_gpu_freq_change(val * 50);
  Based on our IRC discussion, I believe the value in this trace is wrong.
 
 Ah yeah, correct.  I can just trace val here maybe, or Ill have to put
 this off until I post the real frequencies.
 
   +static void valleyview_enable_rps(struct drm_device *dev)
   +{
   + struct drm_i915_private *dev_priv = dev-dev_private;
   + struct intel_ring_buffer *ring;
   + u32 gtfifodbg, val;
   + int i;
   +
   + WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
  
  Should we check FB_GFX_TURBO_EN_FUSE here?
 
 I guess it wouldn't hurt.
 
   +
   + if ((gtfifodbg = I915_READ(GTFIFODBG))) {
   + DRM_ERROR(GT fifo had a previous error %x\n, gtfifodbg);
   + I915_WRITE(GTFIFODBG, gtfifodbg);
   + }
   +
   + gen6_gt_force_wake_get(dev_priv);
   +
   + I915_WRITE(GEN6_RC_SLEEP, 0);
  You've dropped GEN6_RC1_WAKE_RATE_LIMIT. Intentional?
 
 Uh I should have commented that.  I *think* it was intentional since
 RC1 doesn't exist on VLV, but I'll have to check.

I was just confused because you set GEN6_RC1e_THRESHOLD below. Is that
one valid?

 
   + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 0x0028);
   + I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000);
   + I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 0x19);
  
  Don't use 0x19, use 25 since we use 25 in the gen6 path
 
 Ok.
 
   +
   + for_each_ring(ring, dev_priv, i)
   + I915_WRITE(RING_MAX_IDLE(ring-mmio_base), 10);
   +
   + I915_WRITE(GEN6_RC1e_THRESHOLD, 1000);
   + I915_WRITE(GEN6_RC6_THRESHOLD, 0xc350);
   +
   + I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
   + I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000);
   + I915_WRITE(GEN6_RP_UP_EI, 66000);
   + I915_WRITE(GEN6_RP_DOWN_EI, 35);
   +
   + I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
  
  I can't find most of the above values, but, I'll assume they're correct.
  I'd also suggest converging on either all decimal, or all hex, just to
  

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

[Ben Widawsky]

On Wed, Mar 20, 2013 at 09:21:47AM -0700, Jesse Barnes wrote:
 On Tue, 19 Mar 2013 20:19:56 -0700
 Ben Widawsky b...@bwidawsk.net wrote:
 
  Change the gen6+ max delay if the pcode read was successful (not the
  inverse).
  
  The previous code was all sorts of wrong and has existed since I broke
  it:
  commit 42c0526c930523425ff6edc95b7235ce7ab9308d
  Author: Ben Widawsky b...@bwidawsk.net
  Date:   Wed Sep 26 10:34:00 2012 -0700
  
  drm/i915: Extract PCU communication
  
  I added some parentheses for clarity, and I also corrected the debug
  message message to use the mask (wrong before I came along) and added a
  print to show the value we're changing from.
  
  Looking over the code, I'm not actually sure what we're trying to do. I
  introduced the bug simply by extracting the function not implementing
  anything new. We already set max_delay based on the capabilities
  register (which is what we use elsewhere to determine min and max).
  This would potentially increase it, I suppose? Jesse, I can't find the
  document which explains the definitions of the pcode commands, maybe you
  have it around.
  
  Based on Jesse's response, this could potentially be for -fixes, or
  stable, or maybe lead to us dropping it entirely. As the current code is
  is, things won't completely break because of the aforementioned
  capabilities register, and in my experimentation, enabling this has no
  effect, it goes from 1100-1100.
  
  I found this while reviewing Jesse's VLV patches.
  
  Cc: Jesse Barnes jbar...@virtuousgeek.org
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
  ---
   drivers/gpu/drm/i915/intel_pm.c | 6 --
   1 file changed, 4 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index c30e89a..86729b1 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device *dev)
  if (!ret) {
  pcu_mbox = 0;
  ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, 
  pcu_mbox);
  -   if (ret  pcu_mbox  (131)) { /* OC supported */
  +   if (!ret  (pcu_mbox  (131))) { /* OC supported */
  +   DRM_DEBUG_DRIVER(overclocking supported, adjusting 
  frequency max from %dMHz to %dMHz\n,
  +((dev_priv-rps.max_delay  0xff) * 
  50),
  +((pcu_mbox  0xff) * 50));
  dev_priv-rps.max_delay = pcu_mbox  0xff;
  -   DRM_DEBUG_DRIVER(overclocking supported, adjusting 
  frequency max to %dMHz\n, pcu_mbox * 50);
  }
  } else {
  DRM_DEBUG_DRIVER(Failed to set the min frequency\n);
 
 Yeah looks ok.  I don't think I've seen a system where this flag gets
 set, but according to the docs this is the right thing to do.
 
 Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
 
 -- 
 Jesse Barnes, Intel Open Source Technology Center

So from what I gather, we shouldn't bother with -fixes, or stable,
correct?

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Don't wait for PCH on reset

[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 b...@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) | 0xf);
 
+   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   (113)
 #define  DISP_FBC_WM_DIS   (115)
+#define GEN7_MSG_CTL   0x45010
+#define  WAIT_FOR_PCH_RESET_ACK(11)
+#define  WAIT_FOR_PCH_FLR_ACK  (10)
 
 /* GEN7 chicken */
 #define GEN7_COMMON_SLICE_CHICKEN1 0x7010
-- 
1.8.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Don't wait for PCH on reset

[Jesse Barnes]

On Wed, 20 Mar 2013 09:43:00 -0700
Ben Widawsky b...@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 b...@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) | 0xf);
  
 + 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 (113)
  #define  DISP_FBC_WM_DIS (115)
 +#define GEN7_MSG_CTL 0x45010
 +#define  WAIT_FOR_PCH_RESET_ACK  (11)
 +#define  WAIT_FOR_PCH_FLR_ACK(10)
  
  /* GEN7 chicken */
  #define GEN7_COMMON_SLICE_CHICKEN1   0x7010

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

[Jesse Barnes]

On Wed, 20 Mar 2013 09:33:28 -0700
Ben Widawsky b...@bwidawsk.net wrote:

 On Wed, Mar 20, 2013 at 09:21:47AM -0700, Jesse Barnes wrote:
  On Tue, 19 Mar 2013 20:19:56 -0700
  Ben Widawsky b...@bwidawsk.net wrote:
  
   Change the gen6+ max delay if the pcode read was successful (not the
   inverse).
   
   The previous code was all sorts of wrong and has existed since I broke
   it:
   commit 42c0526c930523425ff6edc95b7235ce7ab9308d
   Author: Ben Widawsky b...@bwidawsk.net
   Date:   Wed Sep 26 10:34:00 2012 -0700
   
   drm/i915: Extract PCU communication
   
   I added some parentheses for clarity, and I also corrected the debug
   message message to use the mask (wrong before I came along) and added a
   print to show the value we're changing from.
   
   Looking over the code, I'm not actually sure what we're trying to do. I
   introduced the bug simply by extracting the function not implementing
   anything new. We already set max_delay based on the capabilities
   register (which is what we use elsewhere to determine min and max).
   This would potentially increase it, I suppose? Jesse, I can't find the
   document which explains the definitions of the pcode commands, maybe you
   have it around.
   
   Based on Jesse's response, this could potentially be for -fixes, or
   stable, or maybe lead to us dropping it entirely. As the current code is
   is, things won't completely break because of the aforementioned
   capabilities register, and in my experimentation, enabling this has no
   effect, it goes from 1100-1100.
   
   I found this while reviewing Jesse's VLV patches.
   
   Cc: Jesse Barnes jbar...@virtuousgeek.org
   Signed-off-by: Ben Widawsky b...@bwidawsk.net
   ---
drivers/gpu/drm/i915/intel_pm.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
   
   diff --git a/drivers/gpu/drm/i915/intel_pm.c 
   b/drivers/gpu/drm/i915/intel_pm.c
   index c30e89a..86729b1 100644
   --- a/drivers/gpu/drm/i915/intel_pm.c
   +++ b/drivers/gpu/drm/i915/intel_pm.c
   @@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device *dev)
 if (!ret) {
 pcu_mbox = 0;
 ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, 
   pcu_mbox);
   - if (ret  pcu_mbox  (131)) { /* OC supported */
   + if (!ret  (pcu_mbox  (131))) { /* OC supported */
   + DRM_DEBUG_DRIVER(overclocking supported, adjusting 
   frequency max from %dMHz to %dMHz\n,
   +  ((dev_priv-rps.max_delay  0xff) * 
   50),
   +  ((pcu_mbox  0xff) * 50));
 dev_priv-rps.max_delay = pcu_mbox  0xff;
   - DRM_DEBUG_DRIVER(overclocking supported, adjusting 
   frequency max to %dMHz\n, pcu_mbox * 50);
 }
 } else {
 DRM_DEBUG_DRIVER(Failed to set the min frequency\n);
  
  Yeah looks ok.  I don't think I've seen a system where this flag gets
  set, but according to the docs this is the right thing to do.
  
  Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
  
  -- 
  Jesse Barnes, Intel Open Source Technology Center
 
 So from what I gather, we shouldn't bother with -fixes, or stable,
 correct?

Correct, unless someone complains about their gfx suddenly being a bit
slower, it's not worth the risk.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/20] drm/i915: add constant alpha support to sprite ioctl

[Daniel Vetter]

On Wed, Mar 20, 2013 at 06:32:00PM +0200, Ville Syrjälä wrote:
 On Tue, Mar 19, 2013 at 10:57:25AM -0700, Jesse Barnes wrote:
  On Tue, 19 Mar 2013 09:42:56 +0100
  Daniel Vetter dan...@ffwll.ch wrote:
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -949,6 +949,7 @@ struct drm_intel_overlay_attrs {
 #define I915_SET_COLORKEY_NONE (10) /* disable color key 
matching */
 #define I915_SET_COLORKEY_DESTINATION  (11)
 #define I915_SET_COLORKEY_SOURCE   (12)
+#define I915_SET_COLORKEY_ALPHA(13)
   
   We've added this driver-private ioctl before attributes everywhere was
   possible. I think we need to convert that before adding more stuff ...
   
   Also some discussion on dri-devel about a somewhat standardized set of
   plane blending properties can't hurt.
   
   Yes, I know that this will lead to a massive dri-devel bikeshed ;-)
  
  Yeah we need some properties for this to fit in with the atomic bits.
  This patch actually pre-dates that by a bit (it's very old)...
  
  I'm sure Ville has ideas on what he'd like to see.
 
 Yeah, I've been pondering about this kind of stuff.
 
 One idea was that we might want to make the props for
 color key/mask, background color, const alpha etc. always use 16bpc,
 and then we can just drop the least significant bits if the HW uses
 less precision. And also fix the channel order to some common standard.
 That might make it a bit easier to write generic user space code.
 
 But I'm not sure how to deal w/ RGB vs. YCbCr. Sometimes you may
 need to feed hardware RGB values, sometimes YCbCr.
 
 Options:
 1. use one prop but the driver will interpret the values as RGB or
YCbCr as needed. Userspace just has to figure out what to stuff
into prop somehow.
 2. one prop for RGB, another one for YCbCr. If the hw has need for
both, it'll pick the right one. Again userspace just has to figure
out which one it should use at a given time
 3. standardize on RGB and convert to YCbCr in the driver as needed

I'm leaning towards 3 for now, since currently no driver supports ycbcr
passthrough. If conversion would be too imprecise (e.g. for color keys) we
could add new attributes later on (i.e. do 2).

 As for the per-pixel alpha, I'm not sure if relying on the pixel
 format alone is the best idea. At least we need a way to tell the
 premult and non-premult cases apart. But maybe we want to even allow
 disabling per-pixel alpha for ARGB formats, so that we don't need
 to create an XRGB fb for the same data. So maybe an enum prop w/
 no per-pixel alpha, pre-mult, non-premult options.

Creating different fb views for xrgb and argb with the same backing
storage feels like the right way for me. But yeah, we need some agreement
on how to expose constant alpha and whether the argb data is
pre-multiplied or not.

I kinda wonder whether we shouldn't just copypaste the gl1 texture blend
state stuff ... Although iirc that passes on colorkeys.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/15] drm/i915: there's no DSPADDR register on Haswell

[Paulo Zanoni]

Hi

2013/3/17 Daniel Vetter dan...@ffwll.ch:
 On Fri, Mar 15, 2013 at 12:10:02PM -0700, Ben Widawsky wrote:
 On Wed, Mar 06, 2013 at 08:03:14PM -0300, Paulo Zanoni wrote:
  From: Paulo Zanoni paulo.r.zan...@intel.com
 
  So don't read it when we hang the GPU. This solves unclaimed
  register messages.
 
  Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
 It would be nice if you could make this a bit more future proof, but
 looks correct to me:

 Done.

Done for intel_display_capture_error_state but not for
intel_display_print_error_state. Do you want to fix this or do you
want me to send a patch on top of dinq?

Thanks for the reviews!
Paulo


 Reviewed-by: Ben Widawsky b...@bwidawsk.net
 Queued for -next, thanks for the patch.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 13/20] drm/i915: add Punit read/write routines for VLV

[Ben Widawsky]

On Fri, Mar 08, 2013 at 10:45:56AM -0800, Jesse Barnes wrote:
 Slightly different than other platforms.
 
 v2 [Jani]: Fix IOSF_BYTE_ENABLES_SHIFT shift. Use common routine.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 Signed-off-by: Jani Nikula jani.nik...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h |2 ++
  drivers/gpu/drm/i915/i915_reg.h |   22 
  drivers/gpu/drm/i915/intel_pm.c |   53 
 +++
  3 files changed, 77 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 3061d73..592e944 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1849,6 +1849,8 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private 
 *dev_priv);
  
  int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 
 *val);
  int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 
 val);
 +int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 
 *val);
 +int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 
 val);
  
  #define __i915_read(x, y) \
   u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg);
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index cf291b6..1877d0e 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -4394,6 +4394,28 @@
  #define GEN6_PCODE_DATA  0x138128
  #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8
  
 +#define VLV_IOSF_DOORBELL_REQ0x182100
 +#define   IOSF_DEVFN_SHIFT   24
 +#define   IOSF_OPCODE_SHIFT  16
 +#define   IOSF_PORT_SHIFT8
 +#define   IOSF_BYTE_ENABLES_SHIFT4
 +#define   IOSF_BAR_SHIFT 1
 +#define   IOSF_SB_BUSY   (10)
 +#define   IOSF_PORT_PUNIT0x4

I'm not seeing where the 0x4 is defined, but I'll assume it's right.


 +#define VLV_IOSF_DATA0x182104
 +#define VLV_IOSF_ADDR0x182108
 +
 +#define PUNIT_REG_GPU_LFM0xd3
 +#define PUNIT_REG_GPU_FREQ_REQ   0xd4
 +#define PUNIT_REG_GPU_FREQ_STS   0xd8
 +#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ   0xdc
 +
 +#define PUNIT_OPCODE_REG_READ6
 +#define PUNIT_OPCODE_REG_WRITE   7
 +
 +#define PUNIT_FUSE_BUS2  0xf6 /* bits 47:40 */
 +#define PUNIT_FUSE_BUS1  0xf5 /* bits 55:48 */
 +
  #define GEN6_GT_CORE_STATUS  0x138060
  #define   GEN6_CORE_CPD_STATE_MASK   (74)
  #define   GEN6_RCn_MASK  7

Could probably do without polluting i915_reg.h for this, since I doubt
we'd ever use this outside of intel_pm.c (unless we have a shared driver
as I mentioned in patch 15).

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 3fae9ff..d2499eb 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -4512,3 +4512,56 @@ int sandybridge_pcode_write(struct drm_i915_private 
 *dev_priv, u8 mbox, u32 val)
  
   return 0;
  }
 +
 +static int vlv_punit_rw(struct drm_i915_private *dev_priv, u8 opcode,
 + u8 addr, u32 *val)
 +{
 + u32 cmd, devfn, port, be, bar;
 +
 + bar = 0;
 + be = 0xf;
 + port = IOSF_PORT_PUNIT;
 + devfn = 16;
 +
 + cmd = (devfn  IOSF_DEVFN_SHIFT) | (opcode  IOSF_OPCODE_SHIFT) |
 + (port  IOSF_PORT_SHIFT) | (be  IOSF_BYTE_ENABLES_SHIFT) |
 + (bar  IOSF_BAR_SHIFT);
 +
 + WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
 +
 + if (I915_READ(VLV_IOSF_DOORBELL_REQ)  IOSF_SB_BUSY) {
 + DRM_DEBUG_DRIVER(warning: pcode (%s) mailbox access failed\n,
 +  opcode == PUNIT_OPCODE_REG_READ ?
 +  read : write);
 + return -EAGAIN;
 + }
 +
 + I915_WRITE(VLV_IOSF_ADDR, addr);
 + if (opcode == PUNIT_OPCODE_REG_WRITE)
 + I915_WRITE(VLV_IOSF_DATA, *val);
 + I915_WRITE(VLV_IOSF_DOORBELL_REQ, cmd);

The doc says to write the data register first, but I doubt it matters.

 +
 + if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ)  IOSF_SB_BUSY) == 0,
 +  500)) {
 + DRM_ERROR(timeout waiting for pcode %s (%d) to finish\n,
 +   opcode == PUNIT_OPCODE_REG_READ ? read : write,
 +   addr);
 + return -ETIMEDOUT;
 + }
 +
 + if (opcode == PUNIT_OPCODE_REG_READ)
 + *val = I915_READ(VLV_IOSF_DATA);
 + I915_WRITE(VLV_IOSF_DATA, 0);

The ending write also shouldn't be necessary, but I doubt it matters.

 +
 + return 0;
 +}
 +
 +int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 

[Intel-gfx] [PATCH] drm/i915: Fix pipe enabled mask for pipe C in WM calculations

[ville . syrjala]

From: Ville Syrjälä ville.syrj...@linux.intel.com

Cc: sta...@vger.kernel.org
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 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 8a3d89e..89a2d6f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1943,7 +1943,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
DRM_DEBUG_KMS(FIFO watermarks For pipe C -
   plane %d, cursor: %d\n,
  plane_wm, cursor_wm);
-   enabled |= 3;
+   enabled |= 4;
}
 
/*
-- 
1.8.1.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/15] drm/i915: there's no DSPADDR register on Haswell

[Daniel Vetter]

On Wed, Mar 20, 2013 at 03:01:34PM -0300, Paulo Zanoni wrote:
 Hi
 
 2013/3/17 Daniel Vetter dan...@ffwll.ch:
  On Fri, Mar 15, 2013 at 12:10:02PM -0700, Ben Widawsky wrote:
  On Wed, Mar 06, 2013 at 08:03:14PM -0300, Paulo Zanoni wrote:
   From: Paulo Zanoni paulo.r.zan...@intel.com
  
   So don't read it when we hang the GPU. This solves unclaimed
   register messages.
  
   Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
  It would be nice if you could make this a bit more future proof, but
  looks correct to me:
 
  Done.
 
 Done for intel_display_capture_error_state but not for
 intel_display_print_error_state. Do you want to fix this or do you
 want me to send a patch on top of dinq?

Oh, I've failed again ;-) Yes, please submit a patch since due to the
backmerge it'll be a mess to rebase this one out.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

[Daniel Vetter]

On Wed, Mar 20, 2013 at 09:57:53AM -0700, Jesse Barnes wrote:
 On Wed, 20 Mar 2013 09:33:28 -0700
 Ben Widawsky b...@bwidawsk.net wrote:
 
  On Wed, Mar 20, 2013 at 09:21:47AM -0700, Jesse Barnes wrote:
   On Tue, 19 Mar 2013 20:19:56 -0700
   Ben Widawsky b...@bwidawsk.net wrote:
   
Change the gen6+ max delay if the pcode read was successful (not the
inverse).

The previous code was all sorts of wrong and has existed since I broke
it:
commit 42c0526c930523425ff6edc95b7235ce7ab9308d
Author: Ben Widawsky b...@bwidawsk.net
Date:   Wed Sep 26 10:34:00 2012 -0700

drm/i915: Extract PCU communication

I added some parentheses for clarity, and I also corrected the debug
message message to use the mask (wrong before I came along) and added a
print to show the value we're changing from.

Looking over the code, I'm not actually sure what we're trying to do. I
introduced the bug simply by extracting the function not implementing
anything new. We already set max_delay based on the capabilities
register (which is what we use elsewhere to determine min and max).
This would potentially increase it, I suppose? Jesse, I can't find the
document which explains the definitions of the pcode commands, maybe you
have it around.

Based on Jesse's response, this could potentially be for -fixes, or
stable, or maybe lead to us dropping it entirely. As the current code is
is, things won't completely break because of the aforementioned
capabilities register, and in my experimentation, enabling this has no
effect, it goes from 1100-1100.

I found this while reviewing Jesse's VLV patches.

Cc: Jesse Barnes jbar...@virtuousgeek.org
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/intel_pm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c 
b/drivers/gpu/drm/i915/intel_pm.c
index c30e89a..86729b1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device 
*dev)
if (!ret) {
pcu_mbox = 0;
ret = sandybridge_pcode_read(dev_priv, 
GEN6_READ_OC_PARAMS, pcu_mbox);
-   if (ret  pcu_mbox  (131)) { /* OC supported */
+   if (!ret  (pcu_mbox  (131))) { /* OC supported */
+   DRM_DEBUG_DRIVER(overclocking supported, 
adjusting frequency max from %dMHz to %dMHz\n,
+((dev_priv-rps.max_delay  
0xff) * 50),
+((pcu_mbox  0xff) * 50));
dev_priv-rps.max_delay = pcu_mbox  0xff;
-   DRM_DEBUG_DRIVER(overclocking supported, 
adjusting frequency max to %dMHz\n, pcu_mbox * 50);
}
} else {
DRM_DEBUG_DRIVER(Failed to set the min frequency\n);
   
   Yeah looks ok.  I don't think I've seen a system where this flag gets
   set, but according to the docs this is the right thing to do.
   
   Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
   
   -- 
   Jesse Barnes, Intel Open Source Technology Center
  
  So from what I gather, we shouldn't bother with -fixes, or stable,
  correct?
 
 Correct, unless someone complains about their gfx suddenly being a bit
 slower, it's not worth the risk.

Concurred and so merged to dinq, thanks for the patch. I've also applied
Chris' bikeshed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Kill a strange comment about DPMS functions

[Daniel Vetter]

On Wed, Mar 20, 2013 at 05:05:09PM +0200, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 This comment looks like some historical leftover. Get rid of it.
 
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Set the VIC in AVI infoframe for SDVO

[Daniel Vetter]

On Wed, Mar 20, 2013 at 06:10:07PM +0200, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 We do this for HDMI already, so I don't know why we wouldn't do
 it for SDVO as well.
 
 This is completely untested due to lack of hardware.
 
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Makes sense, and might fix the oddball sdvo hdmi fail we still have.
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Implement WaSwitchSolVfFArbitrationPriority

[Ben Widawsky]

Bspec mentions this for HSW+. I can't quite tell what the effects are,
and I don't easily have a way to test this.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/intel_pm.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 50dba38..bceca11 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -121,6 +121,7 @@
 
 #define GAM_ECOCHK 0x4090
 #define   ECOCHK_SNB_BIT   (110)
+#define   HSW_ECOCHK_ARB_PRIO_SOL  (16)
 #define   ECOCHK_PPGTT_CACHE64B(0x33)
 #define   ECOCHK_PPGTT_CACHE4B (0x03)
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8a3d89e..cc67591 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3768,6 +3768,9 @@ static void haswell_init_clock_gating(struct drm_device 
*dev)
I915_WRITE(GEN6_MBCTL, I915_READ(GEN6_MBCTL) |
   GEN6_MBCTL_ENABLE_BOOT_FETCH);
 
+   /* WaSwitchSolVfFArbitrationPriority */
+   I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
+
/* XXX: This is a workaround for early silicon revisions and should be
 * removed later.
 */
-- 
1.8.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Fix pipe enabled mask for pipe C in WM calculations

[Daniel Vetter]

On Wed, Mar 20, 2013 at 09:51:05PM +0200, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 Cc: sta...@vger.kernel.org

One of the stable rules is that patches should fix real issues. So can you
please hunt through bugzillas quickly and feed this to relevant bug
reports?
-Daniel
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  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 8a3d89e..89a2d6f 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -1943,7 +1943,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
   DRM_DEBUG_KMS(FIFO watermarks For pipe C -
  plane %d, cursor: %d\n,
 plane_wm, cursor_wm);
 - enabled |= 3;
 + enabled |= 4;
   }
  
   /*
 -- 
 1.8.1.5
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/15] drm/i915: only disable DDI sound if intel_crtc-eld_vld

[Paulo Zanoni]

Hi

2013/3/17 Daniel Vetter dan...@ffwll.ch:
 On Thu, Mar 07, 2013 at 11:31:23AM +0200, Ville Syrjälä wrote:
 On Wed, Mar 06, 2013 at 08:03:08PM -0300, Paulo Zanoni wrote:
  From: Paulo Zanoni paulo.r.zan...@intel.com
 
  We already have the same check on intel_enable_ddi. This patch
  prevents unclaimed register messages when the power well is
  disabled.
 
  Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
  ---
   drivers/gpu/drm/i915/intel_ddi.c |9 ++---
   1 file changed, 6 insertions(+), 3 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
  b/drivers/gpu/drm/i915/intel_ddi.c
  index 56bb7cb..cd2f519 100644
  --- a/drivers/gpu/drm/i915/intel_ddi.c
  +++ b/drivers/gpu/drm/i915/intel_ddi.c
  @@ -1347,9 +1347,12 @@ static void intel_disable_ddi(struct intel_encoder 
  *intel_encoder)
  ironlake_edp_backlight_off(intel_dp);
  }
 
  -   tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
  -   tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A)  (pipe * 4));
  -   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
  +   if (intel_crtc-eld_vld) {
  +   tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
  +   tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) 
  +(pipe * 4));
  +   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
  +   }

 We set eld_vld=false before disabling the crtc in intel_crtc_disable().
 I think you need to rearrange that so that we clear eld_vld only
 after -crtc_disable has been called.

 I've forgotten to drop my bikeshed on the patch itself:

 This looks a bit fishy since currently we assume that disabling something
 just works (especially clearing a few registers). And I don't really
 understand how we can hit unclaimed register issues since the pipe should
 be enabled when we call this function here ...

The audio registers are on the power well. My test case is: eDP panel
without sound. We don't hit unclaimed registers at the -enable
function because it's protected by eld_vld (which is false, because we
don't have sound), but then at the disable path we just
unconditionally touch the registers which are on the power down well,
so unclaimed register.


 So either transcoder eDP doesn't have audio, in which case I think it'd be
 better to check for that here (plus ensure that we yell at callers for
 integrated eDP in e.g. hsw_write_eld).

The eld_vld is the check for audio which you're asking for.

 Or it _does_ have audio, but the
 audio stuff is in the power well. In which case we need to add a check.

We need to patch haswell_modeset_global_resources to enable the power
well in case eDP has sound, but this is an unrelated bug with a
different patch. It's on my TODO list too.

Still, Ville's comment is valid, so I need to resend.


 Yes, I'm too lazy to check the docs myself, but tbh it's still w/e here so
 don't want to fire up the work machine ;-)

 Cheers, Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/15] drm/i915: add intel_power_well_is_down

[Paulo Zanoni]

Hi

2013/3/15 Ben Widawsky b...@bwidawsk.net:
 On Thu, Mar 07, 2013 at 12:26:23AM +0100, Daniel Vetter wrote:
 On Wed, Mar 06, 2013 at 08:03:10PM -0300, Paulo Zanoni wrote:
  From: Paulo Zanoni paulo.r.zan...@intel.com
 
  It returns true if we're not supposed to touch the registers on the
  power down well.
 
  For now there's just one caller, but I'm going to add more.
 
  Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
  ---
   drivers/gpu/drm/i915/intel_display.c |4 ++--
   drivers/gpu/drm/i915/intel_drv.h |1 +
   drivers/gpu/drm/i915/intel_pm.c  |   16 
   3 files changed, 19 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 502cb28..bd27336 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -1227,8 +1227,8 @@ void assert_pipe(struct drm_i915_private *dev_priv,
  if (pipe == PIPE_A  dev_priv-quirks  QUIRK_PIPEA_FORCE)
  state = true;
 
  -   if (IS_HASWELL(dev_priv-dev)  cpu_transcoder != TRANSCODER_EDP 
  -   !(I915_READ(HSW_PWR_WELL_DRIVER)  HSW_PWR_WELL_ENABLE)) {
  +   if (intel_power_well_is_down(dev_priv-dev) 

 The name here feels a bit too generic given that we already have on hsw
 different display c states with different requirements and different
 pieces of hw not working.

 Can't thinkg of anything better than intel_display_power_well_is_down
 though ...
 -Daniel

  +   cpu_transcoder != TRANSCODER_EDP) {
  cur_state = false;
  } else {
  reg = PIPECONF(cpu_transcoder);
  diff --git a/drivers/gpu/drm/i915/intel_drv.h 
  b/drivers/gpu/drm/i915/intel_drv.h
  index 010e998..28c4789 100644
  --- a/drivers/gpu/drm/i915/intel_drv.h
  +++ b/drivers/gpu/drm/i915/intel_drv.h
  @@ -671,6 +671,7 @@ extern void intel_update_fbc(struct drm_device *dev);
   extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
   extern void intel_gpu_ips_teardown(void);
 
  +extern bool intel_power_well_is_down(struct drm_device *dev);
   extern void intel_init_power_well(struct drm_device *dev);
   extern void intel_set_power_well(struct drm_device *dev, bool enable);
   extern void intel_enable_gt_powersave(struct drm_device *dev);
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 5479363..90562bc 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -4070,6 +4070,22 @@ void intel_init_clock_gating(struct drm_device *dev)
  dev_priv-display.init_clock_gating(dev);
   }
 
  +/**
  + * Returns true if we're not supposed to touch the registers on the power 
  down
  + * well. Notice that we don't check whether the power well is actually 
  off, we
  + * just check if our driver told the hardware that it doesn't need the 
  power
  + * well enabled.
  + */

 I agree with Denial that the name sucks because your comment clearly
 contradicts what the function is actually called. Can't think of
 anything better either.

intel_using_power_well?


 In the bikeshed realm, I think it makes more sense to do the IS_HASWELL
 check in your pipe assertion, but I'll assume that you have a good usage
 as you mention later.

I want to make all the power well functions work correctly on Gens
that don't have a power well. We already introduced bugs related to
this in the past, so let's be defensive.


 Reviewed-by: Ben Widawsky b...@bwidawsk.net

  +bool intel_power_well_is_down(struct drm_device *dev)
  +{
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +
  +   if (IS_HASWELL(dev))
  +   return !(I915_READ(HSW_PWR_WELL_DRIVER)  HSW_PWR_WELL_ENABLE);
  +   else
  +   return false;
  +}
  +
   void intel_set_power_well(struct drm_device *dev, bool enable)
   {
  struct drm_i915_private *dev_priv = dev-dev_private;
  --
  1.7.10.4
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx

 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

 --
 Ben Widawsky, Intel Open Source Technology Center



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/15] drm/i915: only disable DDI sound if intel_crtc-eld_vld

[Daniel Vetter]

On Wed, Mar 20, 2013 at 07:03:25PM -0300, Paulo Zanoni wrote:
 Hi
 
 2013/3/17 Daniel Vetter dan...@ffwll.ch:
  On Thu, Mar 07, 2013 at 11:31:23AM +0200, Ville Syrjälä wrote:
  On Wed, Mar 06, 2013 at 08:03:08PM -0300, Paulo Zanoni wrote:
   From: Paulo Zanoni paulo.r.zan...@intel.com
  
   We already have the same check on intel_enable_ddi. This patch
   prevents unclaimed register messages when the power well is
   disabled.
  
   Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
   ---
drivers/gpu/drm/i915/intel_ddi.c |9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
  
   diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
   b/drivers/gpu/drm/i915/intel_ddi.c
   index 56bb7cb..cd2f519 100644
   --- a/drivers/gpu/drm/i915/intel_ddi.c
   +++ b/drivers/gpu/drm/i915/intel_ddi.c
   @@ -1347,9 +1347,12 @@ static void intel_disable_ddi(struct 
   intel_encoder *intel_encoder)
   ironlake_edp_backlight_off(intel_dp);
   }
  
   -   tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
   -   tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A)  (pipe * 4));
   -   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
   +   if (intel_crtc-eld_vld) {
   +   tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
   +   tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) 
   +(pipe * 4));
   +   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
   +   }
 
  We set eld_vld=false before disabling the crtc in intel_crtc_disable().
  I think you need to rearrange that so that we clear eld_vld only
  after -crtc_disable has been called.
 
  I've forgotten to drop my bikeshed on the patch itself:
 
  This looks a bit fishy since currently we assume that disabling something
  just works (especially clearing a few registers). And I don't really
  understand how we can hit unclaimed register issues since the pipe should
  be enabled when we call this function here ...
 
 The audio registers are on the power well. My test case is: eDP panel
 without sound. We don't hit unclaimed registers at the -enable
 function because it's protected by eld_vld (which is false, because we
 don't have sound), but then at the disable path we just
 unconditionally touch the registers which are on the power down well,
 so unclaimed register.
 
 
  So either transcoder eDP doesn't have audio, in which case I think it'd be
  better to check for that here (plus ensure that we yell at callers for
  integrated eDP in e.g. hsw_write_eld).
 
 The eld_vld is the check for audio which you're asking for.

Ok, now I've been slightly less lazy and quickly checked the docs. If I'm
reading the diagrams and audio connections correctly, then there's _no_
audio support at all for the eDP transcoder. Which means we'd need to
protect both the enable and disable side in trancoder != TRANS_EDP checks.

That's what I've meant with proper check. The eld_vld thing is a bit
ad-hoc and imo due to our ddi encoder/connector split: Only the connector
has the edid and so knows whether the sync claims to support audio, but we
need that information in the ddi encoder functions.

But since our current ddi code is full of such little control inversions
where the ddi code has to dig out some piece of information about the
current connector from somewhere, I don't care and postpone this to a time
when hsw support has really settled.
-Daniel

 
  Or it _does_ have audio, but the
  audio stuff is in the power well. In which case we need to add a check.
 
 We need to patch haswell_modeset_global_resources to enable the power
 well in case eDP has sound, but this is an unrelated bug with a
 different patch. It's on my TODO list too.
 
 Still, Ville's comment is valid, so I need to resend.
 
 
  Yes, I'm too lazy to check the docs myself, but tbh it's still w/e here so
  don't want to fire up the work machine ;-)
 
  Cheers, Daniel
  --
  Daniel Vetter
  Software Engineer, Intel Corporation
  +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 
 
 
 -- 
 Paulo Zanoni

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Implement WaSwitchSolVfFArbitrationPriority

[Daniel Vetter]

On Wed, Mar 20, 2013 at 02:49:14PM -0700, Ben Widawsky wrote:
 Bspec mentions this for HSW+. I can't quite tell what the effects are,
 and I don't easily have a way to test this.
 
 Signed-off-by: Ben Widawsky b...@bwidawsk.net

Checked with Bspec, HSD and wadb. Oh dear Intel, why does everything need
to be splatter so much all over the place?

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Fix pipe enabled mask for pipe C in WM calculations

[Chris Wilson]

On Wed, Mar 20, 2013 at 11:05:37PM +0100, Daniel Vetter wrote:
 On Wed, Mar 20, 2013 at 09:51:05PM +0200, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  Cc: sta...@vger.kernel.org
 
 One of the stable rules is that patches should fix real issues. So can you
 please hunt through bugzillas quickly and feed this to relevant bug
 reports?

It would take an astute user to notice that his machine was not using a
lower power self-refresh FIFO mode. And the number of machines that only
set up the third pipe is going to be small, so the impact minor. It
will not fix any of the three pipe issues we have open, for example.

The patch is correct, though had I used enabled |= 1  PIPE_[ABC] it
would have probably prevented this bug.

Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Fix pipe enabled mask for pipe C in WM calculations

[Daniel Vetter]

On Wed, Mar 20, 2013 at 11:39 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Wed, Mar 20, 2013 at 11:05:37PM +0100, Daniel Vetter wrote:
 On Wed, Mar 20, 2013 at 09:51:05PM +0200, ville.syrj...@linux.intel.com 
 wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
 
  Cc: sta...@vger.kernel.org

 One of the stable rules is that patches should fix real issues. So can you
 please hunt through bugzillas quickly and feed this to relevant bug
 reports?

 It would take an astute user to notice that his machine was not using a
 lower power self-refresh FIFO mode. And the number of machines that only
 set up the third pipe is going to be small, so the impact minor. It
 will not fix any of the three pipe issues we have open, for example.

Ah, lazy me didn't read the docs. Low power fifo mode is indeed hard
to report in a bug ;-)

 The patch is correct, though had I used enabled |= 1  PIPE_[ABC] it
 would have probably prevented this bug.

This would be quite a bit less magic I think. Can I have it, please?
-Daniel

 Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk




-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Fix pipe enabled mask for pipe C in WM calculations

[Jesse Barnes]

On Wed, 20 Mar 2013 22:39:33 +
Chris Wilson ch...@chris-wilson.co.uk wrote:

 On Wed, Mar 20, 2013 at 11:05:37PM +0100, Daniel Vetter wrote:
  On Wed, Mar 20, 2013 at 09:51:05PM +0200, ville.syrj...@linux.intel.com 
  wrote:
   From: Ville Syrjälä ville.syrj...@linux.intel.com
   
   Cc: sta...@vger.kernel.org
  
  One of the stable rules is that patches should fix real issues. So can you
  please hunt through bugzillas quickly and feed this to relevant bug
  reports?
 
 It would take an astute user to notice that his machine was not using a
 lower power self-refresh FIFO mode. And the number of machines that only
 set up the third pipe is going to be small, so the impact minor. It
 will not fix any of the three pipe issues we have open, for example.
 
 The patch is correct, though had I used enabled |= 1  PIPE_[ABC] it
 would have probably prevented this bug.
 
 Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk

My fault; I had to merge the initial 3 pipe code after Chris changed
this and added the enabled bitfield.  I remember complaining about it
at the time, I must have been feeling passive-aggressive and snuck the
bug in. :)

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx