Re: [Intel-gfx] [PATCH 09/12] Do more checks for proposed flip pixmaps
Eric Anholt e...@anholt.net writes: Keith Packard kei...@keithp.com writes: Make sure the pitch and tiling are correct. Make sure there's a BO we can get at. I thought we couldn't change these parameters, but now I can't find what prevents them from changing. Can you cite sources? Looks like we *can* change tiling format. That actually makes me kinda happy as that explains why we were able to allocate a linear frame buffer for the X front buffer (due to a bug) and page flip to DRI3 buffers which are always tiled. However, we can't change the pitch. From the kernel driver: /* * TILEOFF/LINOFF registers can't be changed via MI display flips. * Note that pitch changes could also affect these register. */ if (INTEL_INFO(dev)-gen 3 (fb-offsets[0] != crtc-primary-fb-offsets[0] || fb-pitches[0] != crtc-primary-fb-pitches[0])) return -EINVAL; I'll remove the tiling check. -- keith.pack...@intel.com pgpuPa0GqaUNw.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] Add glamor back into the driver
Eric Anholt e...@anholt.net writes: Keith Packard kei...@keithp.com writes: This adds glamor support back into the driver, but instad of going through UXA, this uses it directly instead. This is hard to read with the conditionalizing all of the UXA code in the same commit as adding the glamor code. Then there are a bunch of unrelated whitespace changes, or flattening of a bunch of nested conditionals. I'm not sure how to make the patch easier to review; I guess I could make UXA conditional first? That would be 'crazy' in that the driver would fail to ever work if you didn't ask for UXA, but might make the patch easier to read? The only substantive problem I see is intel_glamor_set_pixmap_bo(). The extra enum and temporary variable introduced here seems pretty pointless (even if that pattern had happened before). Agreed. The problem is that 'DEFAULT_ACCEL_METHOD' is defined as 'GLAMOR', 'UXA' or 'NONE' by configure.ac. This seemed like the cleanest solution in some ways. I also liked having the accel_type enum *not* define acceleration types which weren't compiled into the driver; that caught a few missing #ifdefs I don't think this will work -- glamor_egl uses a different fd from intel-bufmgr, so you're attaching some unrelated BO, if you're even that lucky. This API uses the same FD as the intel bufmgr. From intel_glamor.c: if (!glamor_egl_init(scrn, intel-drmSubFD)) { From glamor_egl.c: Bool glamor_egl_init(ScrnInfoPtr scrn, int fd) ... glamor_egl-fd = fd; ... glamor_egl-has_gem = glamor_egl_check_has_gem(fd); ... Bool glamor_egl_create_textured_pixmap(PixmapPtr pixmap, int handle, int stride) ... if (glamor_egl-has_gem) { if (!glamor_get_flink_name(glamor_egl-fd, handle, name)) { So, you pass in a GEM handle for the intel driver bufmgr and glamor does the flink to get a global name. We should be using an FD passing mechanism here instead, to avoid creating more global names... No need to check for initiailization -- double-calling it is safe (as long as the size is consistent). Thanks. Will fix. void @@ -258,18 +240,52 @@ intel_glamor_flush(intel_screen_private * intel) ScreenPtr screen; screen = xf86ScrnToScreen(intel-scrn); -if (intel-uxa_flags UXA_USE_GLAMOR) -glamor_block_handler(screen); +glamor_block_handler(screen); } glamor_block_handler() is pretty awfully named. We should do something about that. Suggestions welcome :-) Thanks for the careful review. -- keith.pack...@intel.com pgpCJ7Bv7BEdF.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/12] Rework intel 2D driver glamor support
Eric Anholt e...@anholt.net writes: Keith Packard kei...@keithp.com writes: I spent the day just cleaning up this patch series and testing. I think it's ready for others to use and review. I've been running it on two machines for a couple of days now and it's been solid. Patches 2, 4 are: Reviewed-by: Eric Anholt e...@anholt.net Patch 5 is: Acked-by: Eric Anholt e...@anholt.net Thanks. -- keith.pack...@intel.com pgp7IBtW8_yBv.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: Allow i915_gem_setup_global_gtt to fail
On Wed, Jul 30, 2014 at 09:42:01PM +0200, Daniel Vetter wrote: We already needs this just as a safety check in case the preallocation reservation dance fails. But we definitely need this to be able to move tha aliasing ppgtt setup back out of the context code to this place, where it belongs. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem.c | 7 ++- drivers/gpu/drm/i915/i915_gem_gtt.c | 16 ++-- drivers/gpu/drm/i915/i915_gem_gtt.h | 4 ++-- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f4e57fe05c6a..d8399ee622b9 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4762,7 +4762,12 @@ int i915_gem_init(struct drm_device *dev) DRM_DEBUG_DRIVER(allow wake ack timed out\n); } - i915_gem_init_userptr(dev); + ret = i915_gem_init_userptr(dev); + if (ret) { Just throw an error and keep on initialising. Optional features like this and stolen, we can keep going - so we should drop the error code return entirely for a better error message when we disable the feature. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Only refcount ppgtt if it actually is one
On Wed, Jul 30, 2014 at 09:41:59PM +0200, Daniel Vetter wrote: This essentially unbreaks non-ppgtt operation where we'd scribble over random memory. While at it give the vm_to_ppgtt function a proper prefix and make it a bit more paranoid. Wrong direction. If you make ggtt/ppgtt more similar we can loose having to write fragile code. -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 00/12] Rework intel 2D driver glamor support
Eric Anholt e...@anholt.net writes: Keith Packard kei...@keithp.com writes: I spent the day just cleaning up this patch series and testing. I think it's ready for others to use and review. I've been running it on two machines for a couple of days now and it's been solid. Patches 2, 4 are: Reviewed-by: Eric Anholt e...@anholt.net Patch 5 is: Acked-by: Eric Anholt e...@anholt.net I've pushed a new version of the tree with your review marked and a few changes: * Split the GetScratchPixmapHeader change into two pieces; the first just removes the redundant calls to drm_intel_bo_disable_reuse and the second contains the scratch pixmap changes and the other misc stuff. Yes, I could split the misc changes out into another cleanup patch if you want. * Removed the tiling check from intel_present when flipping. The kernel doesn't appear to ever require matching tiling. * Removed call to dixPrivateKeyRegistered in the 'Add glamor back' patch. What I didn't do is clean up the 'remove glamor' patch so that it would compile even with --enable-glamor. Fixing that seems like noise to me; all of the changes needed to make it work would be immediately un-done when adding glamor back. -- keith.pack...@intel.com pgpeiubNu3VUv.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/hdmi: call intel_hdmi_prepare for CHV
Hi Daniel, Do you mean the patch 95eb62c8c? If yes, I will have a test on the patch. BTW: I'm not familiar with gfx driver. But I see intel_hdmi_prepare() is called in the pre_pll_enable() for chv and vlv, but in others, it is called in pre_enable(). Could you tell me what's the difference between the two functions? I'm not sure if there is some impact on the audio function. Regards, Libin -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, July 30, 2014 6:46 PM To: Yang, Libin Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/hdmi: call intel_hdmi_prepare for CHV On Wed, Jul 30, 2014 at 10:43:48AM +0800, libin.y...@intel.com wrote: From: Libin Yang libin.y...@intel.com call the intel_hdmi_prepare() in chv_hdmi_pre_enable() for hdmi audio. Signed-off-by: Libin Yang libin.y...@intel.com I've just merged a similar patch from Ville, please check that it works. -Daniel --- drivers/gpu/drm/i915/intel_hdmi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 5f8f4ca..5a65e0c 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1374,6 +1374,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder) int data, i; u32 val; + intel_hdmi_prepare(encoder); mutex_lock(dev_priv-dpio_lock); /* Deassert soft data lane reset*/ -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()
On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com wrote: Daniel, the only way intel_dp-is_mst can get reset is inside this path. Ok, so that one should be safe. Then I guess we can just push the locking down into the respective non-mst leafs (since atm we do link-retraining without any locking, which isn't good). And it needs to be dev-mode_config.mutex, not connection mutex. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Only refcount ppgtt if it actually is one
On Thu, Jul 31, 2014 at 8:52 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Jul 30, 2014 at 09:41:59PM +0200, Daniel Vetter wrote: This essentially unbreaks non-ppgtt operation where we'd scribble over random memory. While at it give the vm_to_ppgtt function a proper prefix and make it a bit more paranoid. Wrong direction. If you make ggtt/ppgtt more similar we can loose having to write fragile code. So moving the kref into the address space? I guess that would work too, but I'm off for my extended w/e now ;-) -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 7/8] drm/irq: Implement a generic vblank_wait function
On Thu, Jul 31, 2014 at 3:14 AM, Michel Dänzer mic...@daenzer.net wrote: I think it would be better to refactor drm_wait_vblank() than to reinvent it. That's the ioctl implementation which spends most of its time decoding ioctl structures. If we take that out then there's about half a line which would be shared (since a lot of the stuff in there is ums gunk that we don't want to carry over to a kms-only internal interface). So imo that doesn't make sense. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Only refcount ppgtt if it actually is one
On Thu, Jul 31, 2014 at 9:39 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote: On Thu, Jul 31, 2014 at 8:52 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Jul 30, 2014 at 09:41:59PM +0200, Daniel Vetter wrote: This essentially unbreaks non-ppgtt operation where we'd scribble over random memory. While at it give the vm_to_ppgtt function a proper prefix and make it a bit more paranoid. Wrong direction. If you make ggtt/ppgtt more similar we can loose having to write fragile code. So moving the kref into the address space? I guess that would work too, but I'm off for my extended w/e now ;-) Ok got bored and tried to implement this, but it doesn't work out too well. It needs a fair bit of reorg so that we can handle the global gtt and the ppgtt with the same refcounting scheme and clean them up properly. I think as a stop-gap this one here is better and the refcounting unification needs to be done on top. -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: Remove context pointer from ppgtt struct
On Wed, Jul 30, 2014 at 5:52 PM, Daniel Vetter dan...@ffwll.ch wrote: I think I'll take a look at this myself and do some untangling ... Ok, I've taken a look and also spotted an issue in your patch. I've submitted the patches to intel-gfx and also uploaded them to http://cgit.freedesktop.org/~danvet/drm/log/?h=ctx-cleanup Can you please review them all? 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 7/8] drm/irq: Implement a generic vblank_wait function
On 31.07.2014 16:54, Daniel Vetter wrote: On Thu, Jul 31, 2014 at 3:14 AM, Michel Dänzer mic...@daenzer.net wrote: I think it would be better to refactor drm_wait_vblank() than to reinvent it. That's the ioctl implementation which spends most of its time decoding ioctl structures. If we take that out then there's about half a line which would be shared (since a lot of the stuff in there is ums gunk that we don't want to carry over to a kms-only internal interface). So imo that doesn't make sense. I'm referring to the core logic of waiting for a number of vblank periods or until the vblank period with a given sequence number, dealing with wraparound etc. The issues you guys were discussing for a new function were ironed out there long ago. -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Initialize the aliasing ppgtt as part of global gtt
On Thu, Jul 31, 2014 at 5:47 AM, Ben Widawsky b...@bwidawsk.net wrote: On Wed, Jul 30, 2014 at 08:46:11PM -0700, Ben Widawsky wrote: 1386367941-7131-76-git-send-email-benjamin.widaw...@intel.com and 1386367941-7131-81-git-send-email-benjamin.widaw...@intel.com Both of these are merged, so I really don't follow what you're trying to tell here. So this can't be the usual I've written this long ago, why was I ignored complaint. Slightly more context instead of a bunch of old msg-ids highly appreciated. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915: Track file_priv, not ctx in the ppgtt structure
-Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Wednesday, July 30, 2014 8:42 PM To: Intel Graphics Development Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 1/7] drm/i915: Track file_priv, not ctx in the ppgtt structure Hardware contexts reference a ppgtt, not the other way round. And the only user of this (in debugfs) actually only cares about which file the ppgtt is associated with. So give it what it wants. While at it give the ppgtt create function a proper nameplace. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_gem_context.c | 22 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 21 + drivers/gpu/drm/i915/i915_gem_gtt.h | 6 +- 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9e737b771c40..3bf1d20c598b 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -333,7 +333,7 @@ static int per_file_stats(int id, void *ptr, void *data) } ppgtt = container_of(vma-vm, struct i915_hw_ppgtt, base); - if (ppgtt-ctx ppgtt-ctx-file_priv != stats- file_priv) + if (ppgtt-file_priv != stats-file_priv) continue; if (obj-ring) /* XXX per-vma statistic */ diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 899c6a7a5920..3b8367aa8404 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -181,26 +181,6 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size) return obj; } -static struct i915_hw_ppgtt * -create_vm_for_ctx(struct drm_device *dev, struct intel_context *ctx) -{ - struct i915_hw_ppgtt *ppgtt; - int ret; - - ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL); - if (!ppgtt) - return ERR_PTR(-ENOMEM); - - ret = i915_ppgtt_init(dev, ppgtt); - if (ret) { - kfree(ppgtt); - return ERR_PTR(ret); - } - - ppgtt-ctx = ctx; - return ppgtt; -} - static struct intel_context * __create_hw_context(struct drm_device *dev, struct drm_i915_file_private *file_priv) @@ -287,7 +267,7 @@ i915_gem_create_context(struct drm_device *dev, } if (create_vm) { - struct i915_hw_ppgtt *ppgtt = create_vm_for_ctx(dev, ctx); + struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv); if (IS_ERR_OR_NULL(ppgtt)) { DRM_DEBUG_DRIVER(PPGTT setup failed (%ld)\n, diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index baa94199239b..83ee41e5c1c7 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1222,6 +1222,27 @@ int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) return ret; } +struct i915_hw_ppgtt * +i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv) +{ + struct i915_hw_ppgtt *ppgtt; + int ret; + + ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL); + if (!ppgtt) + return ERR_PTR(-ENOMEM); + + ret = i915_ppgtt_init(dev, ppgtt); + if (ret) { + kfree(ppgtt); + return ERR_PTR(ret); + } + + ppgtt-file_priv = fpriv; + + return ppgtt; +} + void i915_ppgtt_release(struct kref *kref) { struct i915_hw_ppgtt *ppgtt = diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 380e034c66f8..0b04ef6167f8 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -34,6 +34,8 @@ #ifndef __I915_GEM_GTT_H__ #define __I915_GEM_GTT_H__ +struct drm_i915_file_private; + typedef uint32_t gen6_gtt_pte_t; typedef uint64_t gen8_gtt_pte_t; typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; @@ -258,7 +260,7 @@ struct i915_hw_ppgtt { dma_addr_t *gen8_pt_dma_addr[4]; }; - struct intel_context *ctx; + struct drm_i915_file_private *file_priv; int (*enable)(struct i915_hw_ppgtt *ppgtt); int (*switch_mm)(struct i915_hw_ppgtt *ppgtt, @@ -276,6 +278,8 @@ bool intel_enable_ppgtt(struct drm_device *dev, bool full); int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); void i915_ppgtt_release(struct kref *kref); +struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_device *dev, + struct drm_i915_file_private *fpriv); static inline void i915_ppgtt_get(struct i915_hw_ppgtt *ppgtt) { if (ppgtt) -- 1.9.3
Re: [Intel-gfx] [PATCH 7/8] drm/irq: Implement a generic vblank_wait function
On Thu, Jul 31, 2014 at 10:56 AM, Michel Dänzer mic...@daenzer.net wrote: On 31.07.2014 16:54, Daniel Vetter wrote: On Thu, Jul 31, 2014 at 3:14 AM, Michel Dänzer mic...@daenzer.net wrote: I think it would be better to refactor drm_wait_vblank() than to reinvent it. That's the ioctl implementation which spends most of its time decoding ioctl structures. If we take that out then there's about half a line which would be shared (since a lot of the stuff in there is ums gunk that we don't want to carry over to a kms-only internal interface). So imo that doesn't make sense. I'm referring to the core logic of waiting for a number of vblank periods or until the vblank period with a given sequence number, dealing with wraparound etc. The issues you guys were discussing for a new function were ironed out there long ago. I'm referering to the same, but that logic is gunked up with special-cases for UMS and absolute vblank waits and all kinds of other stuff, so that sharing this with a kms helper to wait a few vblanks (so relative only) really doesn't buy us all that much. Actually I think you'll be left with nothing shared since for the kms driver-internal functions really shouldn't have all these hacks to paper over races and other issues (like ums shutting down the pipe while we didn't look). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/hdmi: call intel_hdmi_prepare for CHV
On Thu, Jul 31, 2014 at 9:16 AM, Yang, Libin libin.y...@intel.com wrote: BTW: I'm not familiar with gfx driver. But I see intel_hdmi_prepare() is called in the pre_pll_enable() for chv and vlv, but in others, it is called in pre_enable(). Could you tell me what's the difference between the two functions? I'm not sure if there is some impact on the audio function. Depending upon the platform the exact modeset sequence is not always the same. So we mixmatch and put the different stages at the right step. Which means the hdmi_prepare step isn't called on all platforms from exactly the same functions - platforms other than vlv/chv don't even have a pre_pll_enable step in the hdmi encoder. It's a bit complicated, but this way we can support pretty much all intel gfx ever shipped in one driver without massive amounts of duplicated code. I hope that explains the idea a bit. -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: Add a bit of locking to intel_dp_hpd_pulse()
On 31 July 2014 17:37, Daniel Vetter dan...@ffwll.ch wrote: On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com wrote: Daniel, the only way intel_dp-is_mst can get reset is inside this path. Ok, so that one should be safe. Then I guess we can just push the locking down into the respective non-mst leafs (since atm we do link-retraining without any locking, which isn't good). And it needs to be dev-mode_config.mutex, not connection mutex. I'd like to know why we do link training at this point though as well, adding locking is required of course, I was just going to wrap the short irq call to the link status check with the lock, but I think it should be possible to push it down further, I'm not sure how vague the spec is on what should happen on HPDs, but if we drop the port clock we obviously will lose the link, but we should also know not to be retraining it at that point anyways. Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 2/2] drm/i915: fix VDD state tracking after system resume
Just like during booting the BIOS can leave the VDD bit enabled after system resume. So apply the same state sanitization there too. This fixes a problem where after resume the port power domain refcount gets unbalanced. v2: - unchanged v3: - call edp sanitizing from the encoder reset handler (Daniel) Reported-and-tested-by: Jarkko Nikula jarkko.nik...@linux.intel.com Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 71294b5..8741439 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4002,6 +4002,11 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) kfree(intel_dig_port); } +static void intel_dp_encoder_reset(struct drm_encoder *encoder) +{ + intel_edp_panel_vdd_sanitize(to_intel_encoder(encoder)); +} + static const struct drm_connector_funcs intel_dp_connector_funcs = { .dpms = intel_connector_dpms, .detect = intel_dp_detect, @@ -4017,6 +4022,7 @@ static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = }; static const struct drm_encoder_funcs intel_dp_enc_funcs = { + .reset = intel_dp_encoder_reset, .destroy = intel_dp_encoder_destroy, }; -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [BUG?] 3.16-rc6 ... at drivers/gpu/drm/i915/intel_pm.c:5997 intel_display_power_put+0x12d/0x160()
On Wed, 2014-07-30 at 22:52 +0200, Ian Kumlien wrote: Sorry for the delay, it's been damned hot - vacation is over and overtime has been all the rage at work... No problem, thanks for the feedback. On fre, 2014-07-25 at 12:28 +0300, Imre Deak wrote: On Thu, 2014-07-24 at 01:33 +0200, Ian Kumlien wrote: Try four, now including CC lists for the intel driver... Could you give a try to the 2 patches at: https://patchwork.kernel.org/patch/4437061/ Didn't quite get that it was two separate patches at first, but when i did i also spotted a v2 of the patch set. I applied: https://patchwork.kernel.org/patch/4648961/ https://patchwork.kernel.org/patch/4648951/ On to 3.16-rc7 (there was some fuzz but it applied fine) I didn't see any OOPS:es (didn't scroll around too much) but otoh the screen never turned off? (it's one of those silly mac things, the apple is still lit) and the machine doesn't suspend/sleep anymore. AFAIR it does, after some coaxing, on the unpatched kernel (ie, not the first time but the second time i turn down the lid, i tried three times and play:ed with brightness as i assume you can see in the log) Hm, I can't see how these patches could prevent system suspend. Also according to the dmesg you sent suspend didn't even start, so I guess you're seeing a separate issue. Maybe the lid notification isn't properly handled, but I can't really help tracking that down. In any case to reproduce the particular bug in question (or see if the fix works) you need to get the machine to suspend/resume somehow. One way is to 'echo mem /sys/power/state' as root and resume by pressing power button or similar; could you still try this, again sending the dmesg? Thanks, Imre 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 3/7] drm/i915: Add proper prefix to obj_to_ggtt
-Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Wednesday, July 30, 2014 8:42 PM To: Intel Graphics Development Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 3/7] drm/i915: Add proper prefix to obj_to_ggtt Stuff in headers really aught to have this. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.h | 11 ++- drivers/gpu/drm/i915/i915_gem.c | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) Reviewed-by: Michel Thierry michel.thie...@intel.com smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
Hi Jeff, These patches look like they solve the problem well. I've added some comments in amongst the code. Thanks, Alistair. -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of jeff.mc...@intel.com Sent: Thursday, July 31, 2014 3:00 AM To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM From: Jeff McGee jeff.mc...@intel.com Define a struct to capture information on the device's Slice/Subslice/EU (SSEU) configuration. Add this struct to the main device info struct. Define a packed bitfield form for the SSEU info and share it with userspace via a new GETPARAM option. Starting with Cherryview, devices may have a varying number of EU for a given ID due to creative fusing. The surest way to determine the configuration is by reading fuses which is best done in the kernel and communicated to userspace. The immediate need from userspace is to determine the number of threads of compute work that can be safely submitted. The definition of SSEU as a new drm/i915 component, with its own header file and soon-to-be source file, is in anticipation of lots of upcoming code for its management, particularly the power gating functionality. Signed-off-by: Jeff McGee jeff.mc...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 3 +++ drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_sseu.h | 40 +++ include/uapi/drm/i915_drm.h | 18 ++ 4 files changed, 64 insertions(+) create mode 100644 drivers/gpu/drm/i915/intel_sseu.h diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2e7f03a..f581848 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_CMD_PARSER_VERSION: value = i915_cmd_parser_get_version(); break; + case I915_PARAM_SSEU_INFO: + value = INTEL_INFO(dev)-sseu.gp_sseu_info; + break; default: DRM_DEBUG(Unknown parameter %d\n, param-param); return -EINVAL; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 18c9ad8..01adafd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -45,6 +45,7 @@ #include linux/intel-iommu.h #include linux/kref.h #include linux/pm_qos.h +#include intel_sseu.h /* General customization: */ @@ -562,6 +563,8 @@ struct intel_device_info { int trans_offsets[I915_MAX_TRANSCODERS]; int palette_offsets[I915_MAX_PIPES]; int cursor_offsets[I915_MAX_PIPES]; + /* Slice/Subslice/EU info */ + struct intel_sseu_info sseu; }; #undef DEFINE_FLAG diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h new file mode 100644 index 000..7db7175 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_sseu.h @@ -0,0 +1,40 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person +obtaining a + * copy of this software and associated documentation files (the +Software), + * to deal in the Software without restriction, including without +limitation + * the rights to use, copy, modify, merge, publish, distribute, +sublicense, + * and/or sell copies of the Software, and to permit persons to whom +the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the +next + * paragraph) shall be included in all copies or substantial portions +of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR +OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS + * IN THE SOFTWARE. + * + */ +#ifndef _INTEL_SSEU_H_ +#define _INTEL_SSEU_H_ + +struct intel_sseu_info { + /* Total slice count */ + unsigned int slice_cnt; + /* Total subslice count */ + unsigned int subslice_cnt; + /* Total execution unit count */ + unsigned int eu_cnt; + /* Thread count per EU */ + unsigned int threads_per_eu; + /* Bit field representation for I915_PARAM_SSEU_INFO */ + u32 gp_sseu_info; +}; + +#endif diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index ff57f07..b99c1a2 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -171,6 +171,23 @@ typedef struct _drm_i915_sarea { #define
Re: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
Hi Jeff, Some more comments in the code. Thanks, Alistair. -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of jeff.mc...@intel.com Sent: Thursday, July 31, 2014 3:00 AM To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV From: Jeff McGee jeff.mc...@intel.com Cherryview can have different SSEU configurations within a given PCI ID, so we collect the info from the fuse register. I don't currently have access to a CHV, much less one with an interesting fuse config. So I have compile-checked this only! Signed-off-by: Jeff McGee jeff.mc...@intel.com --- drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/i915_dma.c | 2 ++ drivers/gpu/drm/i915/i915_reg.h | 13 drivers/gpu/drm/i915/intel_sseu.c | 64 +++ drivers/gpu/drm/i915/intel_sseu.h | 2 ++ 5 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_sseu.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..9a0f411 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \ i915_irq.o \ i915_trace_points.o \ intel_ringbuffer.o \ - intel_uncore.o + intel_uncore.o \ + intel_sseu.o # autogenerated null render state i915-y += intel_renderstate_gen6.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f581848..384ef65 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_gem_unload; } + intel_sseu_init(dev); + intel_power_domains_init(dev_priv); if (drm_core_check_feature(dev, DRIVER_MODESET)) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..24a2d56 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5624,6 +5624,19 @@ enum punit_power_well { #define GEN7_MISCCPCTL (0x9424) #define GEN7_DOP_CLOCK_GATE_ENABLE (10) +/* Fuse readout registers for GT */ +#define CHV_FUSE_GT0x182168 +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 10 +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS1 11 These should be: #define CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 (1 10) #define CHV_FUSE_GT_SUBSLICE_DISABLE_SS1 (1 11) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK (0xf16) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT16 Why not use the shift define here? #define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK (0xfCHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT) and for the others. +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK (0xf20) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT20 +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK (0xf24) +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT24 +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK (0xf28) +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT28 + /* IVYBRIDGE DPF */ #define GEN7_L3CDERRST10xB008 /* L3CD Error Status 1 */ #define HSW_L3CDERRST110xB208 /* L3CD Error Status register 1 slice 1 */ diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c new file mode 100644 index 000..6ba4830 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_sseu.c @@ -0,0 +1,64 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person +obtaining a + * copy of this software and associated documentation files (the +Software), + * to deal in the Software without restriction, including without +limitation + * the rights to use, copy, modify, merge, publish, distribute, +sublicense, + * and/or sell copies of the Software, and to permit persons to whom +the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the +next + * paragraph) shall be included in all copies or substantial portions +of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR +OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS + * IN THE SOFTWARE. + * + */ +#include linux/bitops.h +#include
Re: [Intel-gfx] [PATCH 26/40] drm/i915: Parametrize VLV_DDL registers
On Wed, Jul 30, 2014 at 05:43:10PM -0300, Paulo Zanoni wrote: 2014-06-27 20:04 GMT-03:00 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com The VLV/CHV DDL registers are uniform, and neatly enough the register offsets are sane so we can easily unify them to a single set of defines and just pass the pipe as the parameter to compute the register offset. What the commit message doesn't tell is that now we will call vlv_compute_drain_latency() for pipe C on CHV since I see CHV is defined with num_pipes=3. I think this is quite an important detail, since it's the only way this patch changes the behavior of the code. If that is intentional and correct, then I suggest amending the commit message, even maybe the patch title. Then you can add: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com. One of the following patches will add a proper cherryview_update_wm() function which also fills out the actual watermarks for pipe C. Ideally I probably should have reordered these patches. But I'll add a note of some sort here to avoid bigger reordering pains now. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 26/40] drm/i915: Parametrize VLV_DDL registers
From: Ville Syrjälä ville.syrj...@linux.intel.com The VLV/CHV DDL registers are uniform, and neatly enough the register offsets are sane so we can easily unify them to a single set of defines and just pass the pipe as the parameter to compute the register offset. Note that we now fill out the drain latency for pipe C on CHV which we didn't do before. The rest of the pipe C watermarks are still untouched but that will be remedied later by adding a proper cherryview_update_wm() function. v2: Add a note about CHV pipe C changes (Paulo) Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 54 ++--- drivers/gpu/drm/i915/intel_pm.c | 52 ++- 2 files changed, 36 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9fab647..60dd19c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4018,47 +4018,19 @@ enum punit_power_well { /* drain latency register values*/ #define DRAIN_LATENCY_PRECISION_32 32 #define DRAIN_LATENCY_PRECISION_64 64 -#define VLV_DDL1 (VLV_DISPLAY_BASE + 0x70050) -#define DDL_CURSORA_PRECISION_64 (131) -#define DDL_CURSORA_PRECISION_32 (031) -#define DDL_CURSORA_SHIFT 24 -#define DDL_SPRITEB_PRECISION_64 (123) -#define DDL_SPRITEB_PRECISION_32 (023) -#define DDL_SPRITEB_SHIFT 16 -#define DDL_SPRITEA_PRECISION_64 (115) -#define DDL_SPRITEA_PRECISION_32 (015) -#define DDL_SPRITEA_SHIFT 8 -#define DDL_PLANEA_PRECISION_64(17) -#define DDL_PLANEA_PRECISION_32(07) -#define DDL_PLANEA_SHIFT 0 - -#define VLV_DDL2 (VLV_DISPLAY_BASE + 0x70054) -#define DDL_CURSORB_PRECISION_64 (131) -#define DDL_CURSORB_PRECISION_32 (031) -#define DDL_CURSORB_SHIFT 24 -#define DDL_SPRITED_PRECISION_64 (123) -#define DDL_SPRITED_PRECISION_32 (023) -#define DDL_SPRITED_SHIFT 16 -#define DDL_SPRITEC_PRECISION_64 (115) -#define DDL_SPRITEC_PRECISION_32 (015) -#define DDL_SPRITEC_SHIFT 8 -#define DDL_PLANEB_PRECISION_64(17) -#define DDL_PLANEB_PRECISION_32(07) -#define DDL_PLANEB_SHIFT 0 - -#define VLV_DDL3 (VLV_DISPLAY_BASE + 0x70058) -#define DDL_CURSORC_PRECISION_64 (131) -#define DDL_CURSORC_PRECISION_32 (031) -#define DDL_CURSORC_SHIFT 24 -#define DDL_SPRITEF_PRECISION_64 (123) -#define DDL_SPRITEF_PRECISION_32 (023) -#define DDL_SPRITEF_SHIFT 16 -#define DDL_SPRITEE_PRECISION_64 (115) -#define DDL_SPRITEE_PRECISION_32 (015) -#define DDL_SPRITEE_SHIFT 8 -#define DDL_PLANEC_PRECISION_64(17) -#define DDL_PLANEC_PRECISION_32(07) -#define DDL_PLANEC_SHIFT 0 +#define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4 * (pipe)) +#define DDL_CURSOR_PRECISION_64(131) +#define DDL_CURSOR_PRECISION_32(031) +#define DDL_CURSOR_SHIFT 24 +#define DDL_SPRITE1_PRECISION_64 (123) +#define DDL_SPRITE1_PRECISION_32 (023) +#define DDL_SPRITE1_SHIFT 16 +#define DDL_SPRITE0_PRECISION_64 (115) +#define DDL_SPRITE0_PRECISION_32 (015) +#define DDL_SPRITE0_SHIFT 8 +#define DDL_PLANE_PRECISION_64 (17) +#define DDL_PLANE_PRECISION_32 (07) +#define DDL_PLANE_SHIFT0 /* FIFO watermark sizes etc */ #define G4X_FIFO_LINE_SIZE 64 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index dc858b5..f0516a7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1275,35 +1275,29 @@ static bool vlv_compute_drain_latency(struct drm_device *dev, static void vlv_update_drain_latency(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; - int planea_prec, planea_dl, planeb_prec, planeb_dl; - int cursora_prec, cursora_dl, cursorb_prec, cursorb_dl; - int plane_prec_mult, cursor_prec_mult; /* Precision multiplier is - either 16 or 32 */ - - /* For plane A, Cursor A */ - if (vlv_compute_drain_latency(dev, 0, plane_prec_mult, planea_dl, - cursor_prec_mult, cursora_dl)) { - cursora_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_32) ? - DDL_CURSORA_PRECISION_32 : DDL_CURSORA_PRECISION_64; - planea_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_32) ? - DDL_PLANEA_PRECISION_32 : DDL_PLANEA_PRECISION_64; - -
[Intel-gfx] [RFC 1/1] FOR_UPSTREAM [VPG]: drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
From: Sagar Kamble sagar.a.kam...@intel.com Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of gunit registers need to be followed in PM suspend and resume path similar to runtime suspend and resume. v2: 1. Keeping GT access, wake, gunit save/restore related helpers static. 2. Moved GT access check, Wake Control, Gunit state save to end of i915_drm_freeze. 3. Reusing the sequence in runtime_suspend/resume path at macro level. Issue: GMIN-2507 Change-Id: I15cfdeeec9c976d9839bb281f809664f4a0c78a2 Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 39 +-- drivers/gpu/drm/i915/i915_drv.h | 1 + 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6c4b25c..385dc74 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -490,11 +490,16 @@ bool i915_semaphore_is_enabled(struct drm_device *dev) return true; } +static int vlv_runtime_suspend(struct drm_i915_private *dev_priv); +static int vlv_runtime_resume(struct drm_i915_private *dev_priv, + bool resume_from_s0ix); + static int i915_drm_freeze(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc; pci_power_t opregion_target_state; + int ret = 0; /* ignore lid events during suspend */ mutex_lock(dev_priv-modeset_restore_lock); @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure +* changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); + + return ret; } int i915_suspend(struct drm_device *dev, pm_message_t state) @@ -610,6 +620,12 @@ void intel_console_resume(struct work_struct *work) static int i915_drm_thaw_early(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; + int ret = 0; + + /* Restore Gunit State and allow wake - Need to make sure +* changes in vlv_runtime_resume path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_resume(dev_priv, true); if (IS_HASWELL(dev) || IS_BROADWELL(dev)) hsw_disable_pc8(dev_priv); @@ -618,7 +634,7 @@ static int i915_drm_thaw_early(struct drm_device *dev) intel_uncore_sanitize(dev); intel_power_domains_init_hw(dev_priv); - return 0; + return ret; } static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) @@ -1098,6 +1114,7 @@ static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv) s-gu_ctl0 = I915_READ(VLV_GU_CTL0); s-gu_ctl1 = I915_READ(VLV_GU_CTL1); s-clock_gate_dis2 = I915_READ(VLV_GUNIT_CLOCK_GATE2); + s-dpio_cfg_data= I915_READ(DPIO_CTL); /* * Not saving any of: @@ -1192,6 +1209,7 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv) I915_WRITE(VLV_GU_CTL0, s-gu_ctl0); I915_WRITE(VLV_GU_CTL1, s-gu_ctl1); I915_WRITE(VLV_GUNIT_CLOCK_GATE2, s-clock_gate_dis2); + I915_WRITE(DPIO_CTL,s-dpio_cfg_data); } int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on) @@ -1291,6 +1309,8 @@ static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv) I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR); } +/* This function is used in system suspend path as well to utilize + * Gfx clock, Wake control, Gunit state save related functionaility */ static int vlv_runtime_suspend(struct drm_i915_private *dev_priv) { u32 mask; @@ -1331,7 +1351,12 @@ err1: return err; } -static int vlv_runtime_resume(struct drm_i915_private *dev_priv) +/* This function is used in system resume path as well to utilize + * Gfx clock, Wake control, Gunit state restore related functionaility. + * GEM and other initialization will differ which will be controlled by + * resume_from_s0ix variable */ +static int vlv_runtime_resume(struct drm_i915_private *dev_priv, + bool resume_from_s0ix) { struct drm_device *dev = dev_priv-dev; int err; @@ -1356,8 +1381,10 @@ static int vlv_runtime_resume(struct drm_i915_private *dev_priv) vlv_check_no_gt_access(dev_priv); - intel_init_clock_gating(dev); - i915_gem_restore_fences(dev); + if (!resume_from_s0ix) { + intel_init_clock_gating(dev); + i915_gem_restore_fences(dev); + } return ret; } @@ -1462,7 +1489,7 @@ static int
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Update DDL only for current CRTC
On Wed, 2014-07-16 at 18:24 +0530, Gajanan Bhat wrote: Instead of looping through all CRTCs, update DDL for current CRTC for which watermark is being updated. CHV is confirmed to have precision of 32/64 which is same as VLV. Signed-off-by: Gajanan Bhat gajanan.b...@intel.com Looks ok to me: Reviewed-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 25 + 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b881639..90df1e8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1307,24 +1307,17 @@ static bool vlv_compute_drain_latency(struct drm_device *dev, * latency value. */ -static void vlv_update_drain_latency(struct drm_device *dev) +static void vlv_update_drain_latency(struct drm_crtc *crtc) { + struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; - enum pipe pipe; - - for_each_pipe(pipe) { - int plane_prec, plane_dl; - int cursor_prec, cursor_dl; - int plane_prec_mult, cursor_prec_mult; + enum pipe pipe = to_intel_crtc(crtc)-pipe; + int plane_prec, plane_dl; + int cursor_prec, cursor_dl; + int plane_prec_mult, cursor_prec_mult; - if (!vlv_compute_drain_latency(dev, pipe, plane_prec_mult, plane_dl, -cursor_prec_mult, cursor_dl)) - continue; - - /* - * FIXME CHV spec still lists 16 and 32 as the precision - * values. Need to figure out if spec is outdated or what. - */ + if (vlv_compute_drain_latency(dev, pipe, plane_prec_mult, plane_dl, + cursor_prec_mult, cursor_dl)) { cursor_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_64) ? DDL_CURSOR_PRECISION_64 : DDL_CURSOR_PRECISION_32; plane_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_64) ? @@ -1349,7 +1342,7 @@ static void valleyview_update_wm(struct drm_crtc *crtc) unsigned int enabled = 0; bool cxsr_enabled; - vlv_update_drain_latency(dev); + vlv_update_drain_latency(crtc); if (g4x_compute_wm0(dev, PIPE_A, valleyview_wm_info, latency_ns, 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 2/3] drm/i915: Generalize drain latency computation
On Wed, 2014-07-16 at 18:24 +0530, Gajanan Bhat wrote: Modify drain latency computation to use it for any plane. Same function can be used for primary, cursor and sprite planes. Signed-off-by: Gajanan Bhat gajanan.b...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_pm.c | 82 ++- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d2a220b..a1260a2 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3877,6 +3877,7 @@ enum punit_power_well { #define DDL_PLANE_PRECISION_64 (17) #define DDL_PLANE_PRECISION_32 (07) #define DDL_PLANE_SHIFT 0 +#define DRAIN_LATENCY_MAX0x7f _MASK is the standard postfix in the driver. /* FIFO watermark sizes etc */ #define G4X_FIFO_LINE_SIZE 64 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 90df1e8..f3a3e90 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1268,33 +1268,21 @@ static bool g4x_compute_srwm(struct drm_device *dev, display, cursor); } -static bool vlv_compute_drain_latency(struct drm_device *dev, - int plane, - int *plane_prec_mult, - int *plane_dl, - int *cursor_prec_mult, - int *cursor_dl) +static bool vlv_compute_drain_latency(struct drm_crtc *crtc, + int pixel_size, + int *prec_mult, + int *drain_latency) { - struct drm_crtc *crtc; - int clock, pixel_size; int entries; + int clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock; - crtc = intel_get_crtc_for_plane(dev, plane); - if (!intel_crtc_active(crtc)) + if (clock == 0 || pixel_size == 0) This would mean a driver bug, so needs to be wrapped in a WARN(). return false; - clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock; - pixel_size = crtc-primary-fb-bits_per_pixel / 8; /* BPP */ - - entries = (clock / 1000) * pixel_size; - *plane_prec_mult = (entries 128) ? - DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; - *plane_dl = (64 * (*plane_prec_mult) * 4) / entries; - - entries = (clock / 1000) * 4; /* BPP is always 4 for cursor */ - *cursor_prec_mult = (entries 128) ? - DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; - *cursor_dl = (64 * (*cursor_prec_mult) * 4) / entries; + entries = DIV_ROUND_UP(clock, 1000) * pixel_size; + *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_64 : +DRAIN_LATENCY_PRECISION_32; + *drain_latency = (64 * (*prec_mult) * 4) / entries; return true; } @@ -1309,24 +1297,46 @@ static bool vlv_compute_drain_latency(struct drm_device *dev, static void vlv_update_drain_latency(struct drm_crtc *crtc) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; + struct drm_i915_private *dev_priv = crtc-dev-dev_private; + int pixel_size; + int drain_latency; enum pipe pipe = to_intel_crtc(crtc)-pipe; - int plane_prec, plane_dl; - int cursor_prec, cursor_dl; - int plane_prec_mult, cursor_prec_mult; + int plane_prec, prec_mult, plane_dl; - if (vlv_compute_drain_latency(dev, pipe, plane_prec_mult, plane_dl, - cursor_prec_mult, cursor_dl)) { - cursor_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_64) ? - DDL_CURSOR_PRECISION_64 : DDL_CURSOR_PRECISION_32; - plane_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_64) ? - DDL_PLANE_PRECISION_64 : DDL_PLANE_PRECISION_32; + plane_dl = I915_READ(VLV_DDL(pipe)) ~DDL_PLANE_PRECISION_64 +~DRAIN_LATENCY_MAX ~DDL_CURSOR_PRECISION_64 +~(DRAIN_LATENCY_MAX DDL_CURSOR_SHIFT); A tad simpler and more standard way for masking is ~(X | Y | Z). - I915_WRITE(VLV_DDL(pipe), cursor_prec | -(cursor_dl DDL_CURSOR_SHIFT) | -plane_prec | (plane_dl DDL_PLANE_SHIFT)); + if (!intel_crtc_active(crtc)) { + I915_WRITE(VLV_DDL(pipe), plane_dl); + return; } + + /* Primary plane Drain Latency */ + pixel_size = crtc-primary-fb-bits_per_pixel / 8; /* BPP */ + if (vlv_compute_drain_latency(crtc, pixel_size, prec_mult, drain_latency)) { + plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
Re: [Intel-gfx] [PATCH] drm/i915: Skip Stolen Memory first page.
On Wed, Jul 30, 2014 at 10:47:46AM -0700, Rodrigo Vivi wrote: WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle How does this interact with the BIOS FB takeover? Do BDW systems even place the BIOS FB at the start if stolen due to this W/A? We have no code to verify if that's true or not. The takeover code just blindly assumes that it's true. If the BIOS places the FB at the start of stolen, then I had an idea of copying the first page over just past the end of the BIOS FB and fixing up the the GTT to account for that. The CS is then free to clobber the first page of stolen without corrupting the FB. But I'm not sure our stolen object code can deal with 1 entry sg lists currently. Also we'd probably need to copy the page first to some temporary place (maybe just end of stolen), and only once we know the size of the BIOS FB it could be copied to the final place. Or I guess we could just leave it at the end of stolen since the GTT will remap any access anyway. As long as we don't access the stolen memory directly this should work just fine. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_gem_stolen.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 21c025a..3acefb3 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -290,6 +290,7 @@ int i915_gem_init_stolen(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; int bios_reserved = 0; + int initial_reserved = 0; #ifdef CONFIG_INTEL_IOMMU if (intel_iommu_gfx_mapped INTEL_INFO(dev)-gen 8) { @@ -314,9 +315,13 @@ int i915_gem_init_stolen(struct drm_device *dev) if (WARN_ON(bios_reserved dev_priv-gtt.stolen_size)) return 0; + /* WaSkipStolenMemoryFirstPage */ + if (INTEL_INFO(dev)-gen = 8) + initial_reserved = 4096; + /* Basic memrange allocator for stolen space */ - drm_mm_init(dev_priv-mm.stolen, 0, dev_priv-gtt.stolen_size - - bios_reserved); + drm_mm_init(dev_priv-mm.stolen, initial_reserved, + dev_priv-gtt.stolen_size - bios_reserved); return 0; } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add sprite watermark programming for VLV and CHV
On Wed, 2014-07-16 at 18:24 +0530, Gajanan Bhat wrote: Program DDL register as part sprite watermark programming for CHV and VLV. Signed-off-by: Gajanan Bhat gajanan.b...@intel.com This looks ok, but could you confirm, ideally referencing some document, that we don't need to program any of the sprite watermark level registers along with the DDL values? Specifically I mean the FW7, FW8 registers. --Imre --- drivers/gpu/drm/i915/intel_pm.c | 44 +++ 1 file changed, 44 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f3a3e90..0f439f7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1405,6 +1405,48 @@ static void valleyview_update_wm(struct drm_crtc *crtc) intel_set_memory_cxsr(dev_priv, true); } +static void valleyview_update_sprite_wm(struct drm_plane *plane, + struct drm_crtc *crtc, + uint32_t sprite_width, + uint32_t sprite_height, + int pixel_size, + bool enabled, bool scaled) +{ + struct drm_device *dev = crtc-dev; + struct drm_i915_private *dev_priv = dev-dev_private; + int pipe = to_intel_plane(plane)-pipe; + int drain_latency; + int plane_prec; + int sprite_dl; + int prec_mult; + + if (to_intel_plane(plane)-plane == 0) + sprite_dl = I915_READ(VLV_DDL(pipe)) ~DDL_SPRITE0_PRECISION_64 + ~(DRAIN_LATENCY_MAX DDL_SPRITE0_SHIFT); + else + sprite_dl = I915_READ(VLV_DDL(pipe)) ~DDL_SPRITE1_PRECISION_64 + ~(DRAIN_LATENCY_MAX DDL_SPRITE1_SHIFT); + + if (enabled vlv_compute_drain_latency(crtc, pixel_size, prec_mult, + drain_latency)) { + if (to_intel_plane(plane)-plane == 0) { + plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ? +DDL_SPRITE0_PRECISION_64 : +DDL_SPRITE0_PRECISION_32; + sprite_dl = sprite_dl | plane_prec | + drain_latency DDL_SPRITE0_SHIFT; + } else { + plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ? +DDL_SPRITE1_PRECISION_64 : +DDL_SPRITE1_PRECISION_32; + sprite_dl = sprite_dl | plane_prec | + drain_latency DDL_SPRITE1_SHIFT; + } + } + + I915_WRITE(VLV_DDL(pipe), sprite_dl); +} + static void g4x_update_wm(struct drm_crtc *crtc) { struct drm_device *dev = crtc-dev; @@ -6851,10 +6893,12 @@ void intel_init_pm(struct drm_device *dev) dev_priv-display.init_clock_gating = gen8_init_clock_gating; } else if (IS_CHERRYVIEW(dev)) { dev_priv-display.update_wm = valleyview_update_wm; + dev_priv-display.update_sprite_wm = valleyview_update_sprite_wm; dev_priv-display.init_clock_gating = cherryview_init_clock_gating; } else if (IS_VALLEYVIEW(dev)) { dev_priv-display.update_wm = valleyview_update_wm; + dev_priv-display.update_sprite_wm = valleyview_update_sprite_wm; dev_priv-display.init_clock_gating = valleyview_init_clock_gating; } else if (IS_PINEVIEW(dev)) { 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] [Xen-devel] [Announcement] Updates to XenGT - a Mediated Graphics Passthrough Solution from Intel
On mer, 2014-07-30 at 17:39 +0800, Jike Song wrote: On 07/29/2014 06:09 PM, Dario Faggioli wrote: Perhaps the info is available somewhere already (in which case, sorry), but what's the (if any) upstreaming plan/status/ETA? I think this info could well be part of these updates. :-) Thanks for your opinion :-) :-) We plan to start the upstreaming work in this quarter. For your information, Wei's IOREQ server enhancement(aka Extend ioreq-server to support page write protection) is actually part of the upstreaming effort. Patches for different components will be sent out later. That is fine... Actually, that is quite great to hear! :-) As I said in the original email, I'd add a few words about the upstreaming activity in the following updates. I think, at least in this mailing list, that is a relevant bit of information. :-D Thanks and Regards, Dario -- This happens because I choose it to happen! (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems RD Ltd., Cambridge (UK) 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 09/12] Do more checks for proposed flip pixmaps
On Wed, Jul 30, 2014 at 11:01:48PM -0700, Keith Packard wrote: Eric Anholt e...@anholt.net writes: Keith Packard kei...@keithp.com writes: Make sure the pitch and tiling are correct. Make sure there's a BO we can get at. I thought we couldn't change these parameters, but now I can't find what prevents them from changing. Can you cite sources? Looks like we *can* change tiling format. That actually makes me kinda happy as that explains why we were able to allocate a linear frame buffer for the X front buffer (due to a bug) and page flip to DRI3 buffers which are always tiled. However, we can't change the pitch. From the kernel driver: /* * TILEOFF/LINOFF registers can't be changed via MI display flips. * Note that pitch changes could also affect these register. */ if (INTEL_INFO(dev)-gen 3 (fb-offsets[0] != crtc-primary-fb-offsets[0] || fb-pitches[0] != crtc-primary-fb-pitches[0])) return -EINVAL; I'll remove the tiling check. Now that we have mmio flips in the kernel we can start to relax that restriction. That still needs a bit more work in the mmio flip code but I believe some people working on just that. We could even change the pixel format, except a check was added to drm_mode_page_flip_ioctl() to prevent that, so I guess it was deemed that the API isn't meant to allow that. -- 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 23/40] drm/i915: Fix drain latency precision multipler for VLV
2014-06-27 20:04 GMT-03:00 ville.syrj...@linux.intel.com: From: Zhenyu Wang zhen...@linux.intel.com I guess this affects both VLV and CHV, but my CHV docs still contain 16/32 instead of 32/64. I didn't check any VLV docs. Any pointers, or an explanation on the commit message? Signed-off-by: Zhenyu Wang zhen...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 50 - drivers/gpu/drm/i915/intel_pm.c | 12 +- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 191df9e..7ab5a03 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3909,47 +3909,47 @@ enum punit_power_well { /* drain latency register values*/ #define DRAIN_LATENCY_PRECISION_32 32 -#define DRAIN_LATENCY_PRECISION_16 16 +#define DRAIN_LATENCY_PRECISION_64 64 #define VLV_DDL1 (VLV_DISPLAY_BASE + 0x70050) -#define DDL_CURSORA_PRECISION_32 (131) -#define DDL_CURSORA_PRECISION_16 (031) +#define DDL_CURSORA_PRECISION_64 (131) +#define DDL_CURSORA_PRECISION_32 (031) #define DDL_CURSORA_SHIFT 24 -#define DDL_SPRITEB_PRECISION_32 (123) -#define DDL_SPRITEB_PRECISION_16 (023) +#define DDL_SPRITEB_PRECISION_64 (123) +#define DDL_SPRITEB_PRECISION_32 (023) #define DDL_SPRITEB_SHIFT 16 -#define DDL_SPRITEA_PRECISION_32 (115) -#define DDL_SPRITEA_PRECISION_16 (015) +#define DDL_SPRITEA_PRECISION_64 (115) +#define DDL_SPRITEA_PRECISION_32 (015) #define DDL_SPRITEA_SHIFT 8 -#define DDL_PLANEA_PRECISION_32(17) -#define DDL_PLANEA_PRECISION_16(07) +#define DDL_PLANEA_PRECISION_64(17) +#define DDL_PLANEA_PRECISION_32(07) #define DDL_PLANEA_SHIFT 0 #define VLV_DDL2 (VLV_DISPLAY_BASE + 0x70054) -#define DDL_CURSORB_PRECISION_32 (131) -#define DDL_CURSORB_PRECISION_16 (031) +#define DDL_CURSORB_PRECISION_64 (131) +#define DDL_CURSORB_PRECISION_32 (031) #define DDL_CURSORB_SHIFT 24 -#define DDL_SPRITED_PRECISION_32 (123) -#define DDL_SPRITED_PRECISION_16 (023) +#define DDL_SPRITED_PRECISION_64 (123) +#define DDL_SPRITED_PRECISION_32 (023) #define DDL_SPRITED_SHIFT 16 -#define DDL_SPRITEC_PRECISION_32 (115) -#define DDL_SPRITEC_PRECISION_16 (015) +#define DDL_SPRITEC_PRECISION_64 (115) +#define DDL_SPRITEC_PRECISION_32 (015) #define DDL_SPRITEC_SHIFT 8 -#define DDL_PLANEB_PRECISION_32(17) -#define DDL_PLANEB_PRECISION_16(07) +#define DDL_PLANEB_PRECISION_64(17) +#define DDL_PLANEB_PRECISION_32(07) #define DDL_PLANEB_SHIFT 0 #define VLV_DDL3 (VLV_DISPLAY_BASE + 0x70058) -#define DDL_CURSORC_PRECISION_32 (131) -#define DDL_CURSORC_PRECISION_16 (031) +#define DDL_CURSORC_PRECISION_64 (131) +#define DDL_CURSORC_PRECISION_32 (031) #define DDL_CURSORC_SHIFT 24 -#define DDL_SPRITEF_PRECISION_32 (123) -#define DDL_SPRITEF_PRECISION_16 (023) +#define DDL_SPRITEF_PRECISION_64 (123) +#define DDL_SPRITEF_PRECISION_32 (023) #define DDL_SPRITEF_SHIFT 16 -#define DDL_SPRITEE_PRECISION_32 (115) -#define DDL_SPRITEE_PRECISION_16 (015) +#define DDL_SPRITEE_PRECISION_64 (115) +#define DDL_SPRITEE_PRECISION_32 (015) #define DDL_SPRITEE_SHIFT 8 -#define DDL_PLANEC_PRECISION_32(17) -#define DDL_PLANEC_PRECISION_16(07) +#define DDL_PLANEC_PRECISION_64(17) +#define DDL_PLANEC_PRECISION_32(07) #define DDL_PLANEC_SHIFT 0 /* FIFO watermark sizes etc */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 55f3e6b..9413184 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1253,13 +1253,13 @@ static bool vlv_compute_drain_latency(struct drm_device *dev, entries = (clock / 1000) * pixel_size; *plane_prec_mult = (entries 256) ? - DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_16; + DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; *plane_dl = (64 * (*plane_prec_mult) * 4) / ((clock / 1000) * pixel_size); entries = (clock / 1000) * 4; /* BPP is always 4 for cursor */ *cursor_prec_mult = (entries 256) ? - DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_16; + DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; *cursor_dl = (64 *
Re: [Intel-gfx] [PATCH 23/40] drm/i915: Fix drain latency precision multipler for VLV
On Thu, Jul 31, 2014 at 12:08:09PM -0300, Paulo Zanoni wrote: 2014-06-27 20:04 GMT-03:00 ville.syrj...@linux.intel.com: From: Zhenyu Wang zhen...@linux.intel.com I guess this affects both VLV and CHV, but my CHV docs still contain 16/32 instead of 32/64. I didn't check any VLV docs. Any pointers, or an explanation on the commit message? I added a FIXME about that in patch 26. According to this http://patchwork.freedesktop.org/patch/29860/ CHV has been confirmed to use the 32/64 values too. Hopefully we'll get the spec updated too... Signed-off-by: Zhenyu Wang zhen...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 50 - drivers/gpu/drm/i915/intel_pm.c | 12 +- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 191df9e..7ab5a03 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3909,47 +3909,47 @@ enum punit_power_well { /* drain latency register values*/ #define DRAIN_LATENCY_PRECISION_32 32 -#define DRAIN_LATENCY_PRECISION_16 16 +#define DRAIN_LATENCY_PRECISION_64 64 #define VLV_DDL1 (VLV_DISPLAY_BASE + 0x70050) -#define DDL_CURSORA_PRECISION_32 (131) -#define DDL_CURSORA_PRECISION_16 (031) +#define DDL_CURSORA_PRECISION_64 (131) +#define DDL_CURSORA_PRECISION_32 (031) #define DDL_CURSORA_SHIFT 24 -#define DDL_SPRITEB_PRECISION_32 (123) -#define DDL_SPRITEB_PRECISION_16 (023) +#define DDL_SPRITEB_PRECISION_64 (123) +#define DDL_SPRITEB_PRECISION_32 (023) #define DDL_SPRITEB_SHIFT 16 -#define DDL_SPRITEA_PRECISION_32 (115) -#define DDL_SPRITEA_PRECISION_16 (015) +#define DDL_SPRITEA_PRECISION_64 (115) +#define DDL_SPRITEA_PRECISION_32 (015) #define DDL_SPRITEA_SHIFT 8 -#define DDL_PLANEA_PRECISION_32(17) -#define DDL_PLANEA_PRECISION_16(07) +#define DDL_PLANEA_PRECISION_64(17) +#define DDL_PLANEA_PRECISION_32(07) #define DDL_PLANEA_SHIFT 0 #define VLV_DDL2 (VLV_DISPLAY_BASE + 0x70054) -#define DDL_CURSORB_PRECISION_32 (131) -#define DDL_CURSORB_PRECISION_16 (031) +#define DDL_CURSORB_PRECISION_64 (131) +#define DDL_CURSORB_PRECISION_32 (031) #define DDL_CURSORB_SHIFT 24 -#define DDL_SPRITED_PRECISION_32 (123) -#define DDL_SPRITED_PRECISION_16 (023) +#define DDL_SPRITED_PRECISION_64 (123) +#define DDL_SPRITED_PRECISION_32 (023) #define DDL_SPRITED_SHIFT 16 -#define DDL_SPRITEC_PRECISION_32 (115) -#define DDL_SPRITEC_PRECISION_16 (015) +#define DDL_SPRITEC_PRECISION_64 (115) +#define DDL_SPRITEC_PRECISION_32 (015) #define DDL_SPRITEC_SHIFT 8 -#define DDL_PLANEB_PRECISION_32(17) -#define DDL_PLANEB_PRECISION_16(07) +#define DDL_PLANEB_PRECISION_64(17) +#define DDL_PLANEB_PRECISION_32(07) #define DDL_PLANEB_SHIFT 0 #define VLV_DDL3 (VLV_DISPLAY_BASE + 0x70058) -#define DDL_CURSORC_PRECISION_32 (131) -#define DDL_CURSORC_PRECISION_16 (031) +#define DDL_CURSORC_PRECISION_64 (131) +#define DDL_CURSORC_PRECISION_32 (031) #define DDL_CURSORC_SHIFT 24 -#define DDL_SPRITEF_PRECISION_32 (123) -#define DDL_SPRITEF_PRECISION_16 (023) +#define DDL_SPRITEF_PRECISION_64 (123) +#define DDL_SPRITEF_PRECISION_32 (023) #define DDL_SPRITEF_SHIFT 16 -#define DDL_SPRITEE_PRECISION_32 (115) -#define DDL_SPRITEE_PRECISION_16 (015) +#define DDL_SPRITEE_PRECISION_64 (115) +#define DDL_SPRITEE_PRECISION_32 (015) #define DDL_SPRITEE_SHIFT 8 -#define DDL_PLANEC_PRECISION_32(17) -#define DDL_PLANEC_PRECISION_16(07) +#define DDL_PLANEC_PRECISION_64(17) +#define DDL_PLANEC_PRECISION_32(07) #define DDL_PLANEC_SHIFT 0 /* FIFO watermark sizes etc */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 55f3e6b..9413184 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1253,13 +1253,13 @@ static bool vlv_compute_drain_latency(struct drm_device *dev, entries = (clock / 1000) * pixel_size; *plane_prec_mult = (entries 256) ? - DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_16; + DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; *plane_dl = (64 * (*plane_prec_mult) * 4) /
Re: [Intel-gfx] [PATCH 09/12] Do more checks for proposed flip pixmaps
Ville Syrjälä ville.syrj...@linux.intel.com writes: Now that we have mmio flips in the kernel we can start to relax that restriction. That still needs a bit more work in the mmio flip code but I believe some people working on just that. I couldn't find any tiling restrictions in the current (ring-based) flip code; did I just miss them? We could even change the pixel format, except a check was added to drm_mode_page_flip_ioctl() to prevent that, so I guess it was deemed that the API isn't meant to allow that. Yeah, not sure I care about this; 32bpp is pretty much the only format I want. -- keith.pack...@intel.com pgp6ASLSyQNPR.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] testdisplay: ignore invalid subtest options
testdisplay does not have any subtests and should therefore exit with the appropriate exit code if the --list-subtests or --run-subtest options are used. Signed-off-by: Thomas Wood thomas.w...@intel.com --- tests/testdisplay.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/testdisplay.c b/tests/testdisplay.c index a187c16..f26d942 100644 --- a/tests/testdisplay.c +++ b/tests/testdisplay.c @@ -71,6 +71,8 @@ #include stdlib.h #include signal.h +#define SUBTEST_OPTS 1 + static int tio_fd; struct termios saved_tio; @@ -748,12 +750,17 @@ int main(int argc, char **argv) GMainLoop *mainloop; float force_clock; bool opt_dump_info = false; + struct option long_opts[] = { + {list-subtests, 0, 0, SUBTEST_OPTS}, + {run-subtest, 1, 0, SUBTEST_OPTS}, + { 0, 0, 0, 0 } + }; igt_skip_on_simulation(); enter_exec_path( argv ); - while ((c = getopt(argc, argv, optstr)) != -1) { + while ((c = getopt_long(argc, argv, optstr, long_opts, NULL)) != -1) { switch (c) { case '3': test_stereo_modes = 1; @@ -804,6 +811,10 @@ int main(int argc, char **argv) case 'o': sscanf(optarg, %d,%d, specified_disp_id, specified_mode_num); break; + case SUBTEST_OPTS: + /* invalid subtest options */ + exit(IGT_EXIT_INVALID); + break; default: /* fall through */ case 'h': -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Initialize the aliasing ppgtt as part of global gtt
-Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Wednesday, July 30, 2014 8:42 PM To: Intel Graphics Development Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 5/7] drm/i915: Initialize the aliasing ppgtt as part of global gtt Stuffing this into the context setup code doesn't make a lot of sense. Also reusing the real ppgtt setup code makes even less sense since the aliasing ppgtt isn't a real address space. Leaving all that stuff unitialized will make sure that we catch any abusers promptly. This is also a prep work to clean up the context-ppgtt link. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem_context.c | 13 + drivers/gpu/drm/i915/i915_gem_gtt.c | 31 +-- 2 files changed, 26 insertions(+), 18 deletions(-) @@ -1766,6 +1771,20 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, /* And finally clear the reserved guard page */ ggtt_vm-clear_range(ggtt_vm, end - PAGE_SIZE, PAGE_SIZE, true); + if (HAS_ALIASING_PPGTT(dev) USES_FULL_PPGTT(dev)) { + struct i915_hw_ppgtt *ppgtt; + + ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL); + if (!ppgtt) + return -ENOMEM; + + ret = __hw_ppgtt_init(dev, ppgtt); + if (!ret) __hw_ppgtt_init will return '0' if successful. + return ret; + + dev_priv-mm.aliasing_ppgtt = ppgtt; + } + return 0; } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/12] Do more checks for proposed flip pixmaps
On Thu, Jul 31, 2014 at 08:20:20AM -0700, Keith Packard wrote: Ville Syrjälä ville.syrj...@linux.intel.com writes: Now that we have mmio flips in the kernel we can start to relax that restriction. That still needs a bit more work in the mmio flip code but I believe some people working on just that. I couldn't find any tiling restrictions in the current (ring-based) flip code; did I just miss them? No, changing tiling is supposed to work via cs flips. Except it doesn't actually work on VLV for some reason. We now fall back to mmio flip on VLV for that, so given a recent enough kernel it should just work (tm) on all platforms. I was referring to the relaxing the restrictions on stride changes. We could even change the pixel format, except a check was added to drm_mode_page_flip_ioctl() to prevent that, so I guess it was deemed that the API isn't meant to allow that. Yeah, not sure I care about this; 32bpp is pretty much the only format I want. You just need to be careful with the X vs. A because the kernel considers those distinct pixel formats. I now regret adding that distinciton to the pixel formats, but sadly I don't own a time machine so I can't undo it. -- 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 5/7] drm/i915: Initialize the aliasing ppgtt as part of global gtt
On Wed, Jul 30, 2014 at 09:42:02PM +0200, Daniel Vetter wrote: Stuffing this into the context setup code doesn't make a lot of sense. Also reusing the real ppgtt setup code makes even less sense since the aliasing ppgtt isn't a real address space. Leaving all that stuff unitialized will make sure that we catch any abusers promptly. This is also a prep work to clean up the context-ppgtt link. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem_context.c | 13 + drivers/gpu/drm/i915/i915_gem_gtt.c | 31 +-- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 3b8367aa8404..7a455fcee3a7 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -276,17 +276,6 @@ i915_gem_create_context(struct drm_device *dev, goto err_unpin; } else ctx-vm = ppgtt-base; - - /* This case is reserved for the global default context and - * should only happen once. */ - if (is_global_default_ctx) { - if (WARN_ON(dev_priv-mm.aliasing_ppgtt)) { - ret = -EEXIST; - goto err_unpin; - } - - dev_priv-mm.aliasing_ppgtt = ppgtt; - } } else if (USES_PPGTT(dev)) { /* For platforms which only have aliasing PPGTT, we fake the * address space and refcounting. */ @@ -361,7 +350,7 @@ int i915_gem_context_init(struct drm_device *dev) } } - ctx = i915_gem_create_context(dev, NULL, USES_PPGTT(dev)); + ctx = i915_gem_create_context(dev, NULL, USES_FULL_PPGTT(dev)); if (IS_ERR(ctx)) { DRM_ERROR(Failed to create default global context (error %ld)\n, PTR_ERR(ctx)); diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 4fa7807ed4d5..cb9bb13ff20a 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1191,23 +1191,27 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) return 0; } -int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) +int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) { struct drm_i915_private *dev_priv = dev-dev_private; - int ret = 0; ppgtt-base.dev = dev; ppgtt-base.scratch = dev_priv-gtt.base.scratch; if (INTEL_INFO(dev)-gen 8) - ret = gen6_ppgtt_init(ppgtt); + return gen6_ppgtt_init(ppgtt); else if (IS_GEN8(dev)) - ret = gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total); + return gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total); else BUG(); +} +int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + int ret = 0; + ret = __hw_ppgtt_init(dev, ppgtt); if (!ret) { - struct drm_i915_private *dev_priv = dev-dev_private; kref_init(ppgtt-ref); drm_mm_init(ppgtt-base.mm, ppgtt-base.start, ppgtt-base.total); @@ -1728,6 +1732,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, struct drm_mm_node *entry; struct drm_i915_gem_object *obj; unsigned long hole_start, hole_end; + int ret; BUG_ON(mappable_end end); @@ -1739,7 +1744,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, /* Mark any preallocated objects as occupied */ list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) { struct i915_vma *vma = i915_gem_obj_to_vma(obj, ggtt_vm); - int ret; + DRM_DEBUG_KMS(reserving preallocated space: %lx + %zx\n, i915_gem_obj_ggtt_offset(obj), obj-base.size); @@ -1766,6 +1771,20 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, /* And finally clear the reserved guard page */ ggtt_vm-clear_range(ggtt_vm, end - PAGE_SIZE, PAGE_SIZE, true); + if (HAS_ALIASING_PPGTT(dev) USES_FULL_PPGTT(dev)) { Should that be !USES_FULL_PPGTT ? + struct i915_hw_ppgtt *ppgtt; + + ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL); + if (!ppgtt) + return -ENOMEM; + + ret = __hw_ppgtt_init(dev, ppgtt); + if (!ret) + return ret; + + dev_priv-mm.aliasing_ppgtt = ppgtt; + } + return 0; } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match driver load thaw
Hi Daniel, Something more like this then? (and revert the change to intel_ring_begin(), putting it back to how it was ) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 991b663..b811ff2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1217,6 +1217,9 @@ struct i915_gpu_error { /* For missed irq/seqno simulation. */ unsigned int test_irq_rings; + + /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset */ + bool reload_in_progress; }; enum modeset_restore { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b38e086..a25d3b5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error *error, if (i915_terminally_wedged(error)) return -EIO; - return -EAGAIN; + /* Check if GPU Reset is in progress */ + if (!error-reload_in_reset) + return -EAGAIN; } return 0; @@ -2579,6 +2581,8 @@ void i915_gem_reset(struct drm_device *dev) struct intel_engine_cs *ring; int i; + /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset */ + dev_priv-gpu_error.reload_in_reset = true; /* * Before we free the objects from the requests, we need to inspect * them for finding the guilty party. As the requests only borrow @@ -2591,6 +2595,8 @@ void i915_gem_reset(struct drm_device *dev) i915_gem_reset_ring_cleanup(dev_priv, ring); i915_gem_restore_fences(dev); + + dev_priv-gpu_error.reload_in_reset = false; } -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, July 30, 2014 10:01 PM To: Mcaulay, Alistair Cc: Daniel Vetter; Chris Wilson; Ben Widawsky; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match driver load thaw On Wed, Jul 30, 2014 at 04:59:33PM +, Mcaulay, Alistair wrote: Hi Daniel, could you please be clearer on the change you mean. I think you mean something functionally equivalent to the code below, but done in a less hacky way. (This slight change has made no change to test results) Or is the idea to return at a different point to this? I couldn't find dev_priv-mm.reload_in_reset or similar in the code. The only thing I can find is error-reset_counter, which is used in check_wedge(). Bottom bit set means RESET_IN_PROGRESS, top bit means WEDGED Well I've meant that you have to add a new dev_prive-mm.realod_in_reset. And the below won't work since in all other places but when doing a gpu reset we want the -EAGAIN to reach callers. Actually it's really important that if we have an -EGAIN we don't eat it. And I guess the check for mm.reload_in_reset should actually be in gem_check_wedged. -Daniel --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1832,7 +1832,9 @@ int intel_ring_begin(struct intel_engine_cs *ring, ret = i915_gem_check_wedge(dev_priv-gpu_error, dev_priv-mm.interruptible); -if (ret) + +/* -EAGAIN means a reset is in progress, it is Ok to return */ +if (ret == -EAGAIN) +return 0; +if (ret) +return ret; ret = __intel_ring_prepare(ring, num_dwords * sizeof(uint32_t)); Alistair. -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Tuesday, July 29, 2014 11:33 AM To: Chris Wilson; Daniel Vetter; Ben Widawsky; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match driver load thaw On Tue, Jul 29, 2014 at 08:36:33AM +0100, Chris Wilson wrote: On Mon, Jul 28, 2014 at 11:26:38AM +0200, Daniel Vetter wrote: Oh, I guess that's the tricky bit why the old approach never worked - because reset_in_progress is set we failed the context/ppgtt loading through the rings and screwed up. Problem with your approach is that we want to bail out here if a reset is in progress, so we can't just eat the EAGAIN. If we do that we potentially deadlock or overflow the ring. I think we need a different hack here, and a few layers down (i.e. at the place where we actually generate that offending -EAGAIN). - Around the re-init sequence in the reset function we set dev_priv-mm.reload_in_reset or similar . Since we hold dev-struct_mutex no one will see that, as long as we never leak it out of the critical section. - In the ring_begin code that checks for gpu hangs we ignore reset_in_progress if this bit is set. - Both
Re: [Intel-gfx] [PATCH 23/40] drm/i915: Fix drain latency precision multipler for VLV
2014-07-31 12:16 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com: On Thu, Jul 31, 2014 at 12:08:09PM -0300, Paulo Zanoni wrote: 2014-06-27 20:04 GMT-03:00 ville.syrj...@linux.intel.com: From: Zhenyu Wang zhen...@linux.intel.com I guess this affects both VLV and CHV, but my CHV docs still contain 16/32 instead of 32/64. I didn't check any VLV docs. Any pointers, or an explanation on the commit message? I added a FIXME about that in patch 26. According to this http://patchwork.freedesktop.org/patch/29860/ CHV has been confirmed to use the 32/64 values too. Hopefully we'll get the spec updated too... Ok, but on this case it's quite hard to give a reviewed-by stamp to the patch, since there's no way to review. I guess this is one of the cases where we just have to believe the authors and merge the patch? Signed-off-by: Zhenyu Wang zhen...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 50 - drivers/gpu/drm/i915/intel_pm.c | 12 +- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 191df9e..7ab5a03 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3909,47 +3909,47 @@ enum punit_power_well { /* drain latency register values*/ #define DRAIN_LATENCY_PRECISION_32 32 -#define DRAIN_LATENCY_PRECISION_16 16 +#define DRAIN_LATENCY_PRECISION_64 64 #define VLV_DDL1 (VLV_DISPLAY_BASE + 0x70050) -#define DDL_CURSORA_PRECISION_32 (131) -#define DDL_CURSORA_PRECISION_16 (031) +#define DDL_CURSORA_PRECISION_64 (131) +#define DDL_CURSORA_PRECISION_32 (031) #define DDL_CURSORA_SHIFT 24 -#define DDL_SPRITEB_PRECISION_32 (123) -#define DDL_SPRITEB_PRECISION_16 (023) +#define DDL_SPRITEB_PRECISION_64 (123) +#define DDL_SPRITEB_PRECISION_32 (023) #define DDL_SPRITEB_SHIFT 16 -#define DDL_SPRITEA_PRECISION_32 (115) -#define DDL_SPRITEA_PRECISION_16 (015) +#define DDL_SPRITEA_PRECISION_64 (115) +#define DDL_SPRITEA_PRECISION_32 (015) #define DDL_SPRITEA_SHIFT 8 -#define DDL_PLANEA_PRECISION_32(17) -#define DDL_PLANEA_PRECISION_16(07) +#define DDL_PLANEA_PRECISION_64(17) +#define DDL_PLANEA_PRECISION_32(07) #define DDL_PLANEA_SHIFT 0 #define VLV_DDL2 (VLV_DISPLAY_BASE + 0x70054) -#define DDL_CURSORB_PRECISION_32 (131) -#define DDL_CURSORB_PRECISION_16 (031) +#define DDL_CURSORB_PRECISION_64 (131) +#define DDL_CURSORB_PRECISION_32 (031) #define DDL_CURSORB_SHIFT 24 -#define DDL_SPRITED_PRECISION_32 (123) -#define DDL_SPRITED_PRECISION_16 (023) +#define DDL_SPRITED_PRECISION_64 (123) +#define DDL_SPRITED_PRECISION_32 (023) #define DDL_SPRITED_SHIFT 16 -#define DDL_SPRITEC_PRECISION_32 (115) -#define DDL_SPRITEC_PRECISION_16 (015) +#define DDL_SPRITEC_PRECISION_64 (115) +#define DDL_SPRITEC_PRECISION_32 (015) #define DDL_SPRITEC_SHIFT 8 -#define DDL_PLANEB_PRECISION_32(17) -#define DDL_PLANEB_PRECISION_16(07) +#define DDL_PLANEB_PRECISION_64(17) +#define DDL_PLANEB_PRECISION_32(07) #define DDL_PLANEB_SHIFT 0 #define VLV_DDL3 (VLV_DISPLAY_BASE + 0x70058) -#define DDL_CURSORC_PRECISION_32 (131) -#define DDL_CURSORC_PRECISION_16 (031) +#define DDL_CURSORC_PRECISION_64 (131) +#define DDL_CURSORC_PRECISION_32 (031) #define DDL_CURSORC_SHIFT 24 -#define DDL_SPRITEF_PRECISION_32 (123) -#define DDL_SPRITEF_PRECISION_16 (023) +#define DDL_SPRITEF_PRECISION_64 (123) +#define DDL_SPRITEF_PRECISION_32 (023) #define DDL_SPRITEF_SHIFT 16 -#define DDL_SPRITEE_PRECISION_32 (115) -#define DDL_SPRITEE_PRECISION_16 (015) +#define DDL_SPRITEE_PRECISION_64 (115) +#define DDL_SPRITEE_PRECISION_32 (015) #define DDL_SPRITEE_SHIFT 8 -#define DDL_PLANEC_PRECISION_32(17) -#define DDL_PLANEC_PRECISION_16(07) +#define DDL_PLANEC_PRECISION_64(17) +#define DDL_PLANEC_PRECISION_32(07) #define DDL_PLANEC_SHIFT 0 /* FIFO watermark sizes etc */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 55f3e6b..9413184 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1253,13 +1253,13 @@ static bool vlv_compute_drain_latency(struct drm_device *dev, entries =
Re: [Intel-gfx] [PATCH 23/40] drm/i915: Fix drain latency precision multipler for VLV
On Thu, Jul 31, 2014 at 02:05:49PM -0300, Paulo Zanoni wrote: 2014-07-31 12:16 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com: On Thu, Jul 31, 2014 at 12:08:09PM -0300, Paulo Zanoni wrote: 2014-06-27 20:04 GMT-03:00 ville.syrj...@linux.intel.com: From: Zhenyu Wang zhen...@linux.intel.com I guess this affects both VLV and CHV, but my CHV docs still contain 16/32 instead of 32/64. I didn't check any VLV docs. Any pointers, or an explanation on the commit message? I added a FIXME about that in patch 26. According to this http://patchwork.freedesktop.org/patch/29860/ CHV has been confirmed to use the 32/64 values too. Hopefully we'll get the spec updated too... Ok, but on this case it's quite hard to give a reviewed-by stamp to the patch, since there's no way to review. I guess this is one of the cases where we just have to believe the authors and merge the patch? The VLV docs have the new 32/64 values. Signed-off-by: Zhenyu Wang zhen...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 50 - drivers/gpu/drm/i915/intel_pm.c | 12 +- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 191df9e..7ab5a03 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3909,47 +3909,47 @@ enum punit_power_well { /* drain latency register values*/ #define DRAIN_LATENCY_PRECISION_32 32 -#define DRAIN_LATENCY_PRECISION_16 16 +#define DRAIN_LATENCY_PRECISION_64 64 #define VLV_DDL1 (VLV_DISPLAY_BASE + 0x70050) -#define DDL_CURSORA_PRECISION_32 (131) -#define DDL_CURSORA_PRECISION_16 (031) +#define DDL_CURSORA_PRECISION_64 (131) +#define DDL_CURSORA_PRECISION_32 (031) #define DDL_CURSORA_SHIFT 24 -#define DDL_SPRITEB_PRECISION_32 (123) -#define DDL_SPRITEB_PRECISION_16 (023) +#define DDL_SPRITEB_PRECISION_64 (123) +#define DDL_SPRITEB_PRECISION_32 (023) #define DDL_SPRITEB_SHIFT 16 -#define DDL_SPRITEA_PRECISION_32 (115) -#define DDL_SPRITEA_PRECISION_16 (015) +#define DDL_SPRITEA_PRECISION_64 (115) +#define DDL_SPRITEA_PRECISION_32 (015) #define DDL_SPRITEA_SHIFT 8 -#define DDL_PLANEA_PRECISION_32(17) -#define DDL_PLANEA_PRECISION_16(07) +#define DDL_PLANEA_PRECISION_64(17) +#define DDL_PLANEA_PRECISION_32(07) #define DDL_PLANEA_SHIFT 0 #define VLV_DDL2 (VLV_DISPLAY_BASE + 0x70054) -#define DDL_CURSORB_PRECISION_32 (131) -#define DDL_CURSORB_PRECISION_16 (031) +#define DDL_CURSORB_PRECISION_64 (131) +#define DDL_CURSORB_PRECISION_32 (031) #define DDL_CURSORB_SHIFT 24 -#define DDL_SPRITED_PRECISION_32 (123) -#define DDL_SPRITED_PRECISION_16 (023) +#define DDL_SPRITED_PRECISION_64 (123) +#define DDL_SPRITED_PRECISION_32 (023) #define DDL_SPRITED_SHIFT 16 -#define DDL_SPRITEC_PRECISION_32 (115) -#define DDL_SPRITEC_PRECISION_16 (015) +#define DDL_SPRITEC_PRECISION_64 (115) +#define DDL_SPRITEC_PRECISION_32 (015) #define DDL_SPRITEC_SHIFT 8 -#define DDL_PLANEB_PRECISION_32(17) -#define DDL_PLANEB_PRECISION_16(07) +#define DDL_PLANEB_PRECISION_64(17) +#define DDL_PLANEB_PRECISION_32(07) #define DDL_PLANEB_SHIFT 0 #define VLV_DDL3 (VLV_DISPLAY_BASE + 0x70058) -#define DDL_CURSORC_PRECISION_32 (131) -#define DDL_CURSORC_PRECISION_16 (031) +#define DDL_CURSORC_PRECISION_64 (131) +#define DDL_CURSORC_PRECISION_32 (031) #define DDL_CURSORC_SHIFT 24 -#define DDL_SPRITEF_PRECISION_32 (123) -#define DDL_SPRITEF_PRECISION_16 (023) +#define DDL_SPRITEF_PRECISION_64 (123) +#define DDL_SPRITEF_PRECISION_32 (023) #define DDL_SPRITEF_SHIFT 16 -#define DDL_SPRITEE_PRECISION_32 (115) -#define DDL_SPRITEE_PRECISION_16 (015) +#define DDL_SPRITEE_PRECISION_64 (115) +#define DDL_SPRITEE_PRECISION_32 (015) #define DDL_SPRITEE_SHIFT 8 -#define DDL_PLANEC_PRECISION_32(17) -#define DDL_PLANEC_PRECISION_16(07) +#define DDL_PLANEC_PRECISION_64(17) +#define DDL_PLANEC_PRECISION_32(07) #define DDL_PLANEC_SHIFT 0 /* FIFO watermark sizes etc */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index
Re: [Intel-gfx] [PATCH 23/40] drm/i915: Fix drain latency precision multipler for VLV
2014-07-31 14:13 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com: On Thu, Jul 31, 2014 at 02:05:49PM -0300, Paulo Zanoni wrote: 2014-07-31 12:16 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com: On Thu, Jul 31, 2014 at 12:08:09PM -0300, Paulo Zanoni wrote: 2014-06-27 20:04 GMT-03:00 ville.syrj...@linux.intel.com: From: Zhenyu Wang zhen...@linux.intel.com I guess this affects both VLV and CHV, but my CHV docs still contain 16/32 instead of 32/64. I didn't check any VLV docs. Any pointers, or an explanation on the commit message? I added a FIXME about that in patch 26. According to this http://patchwork.freedesktop.org/patch/29860/ CHV has been confirmed to use the 32/64 values too. Hopefully we'll get the spec updated too... Ok, but on this case it's quite hard to give a reviewed-by stamp to the patch, since there's no way to review. I guess this is one of the cases where we just have to believe the authors and merge the patch? The VLV docs have the new 32/64 values. Ok, found it :) If we assume CHV uses 32/64 just like VLV: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Zhenyu Wang zhen...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 50 - drivers/gpu/drm/i915/intel_pm.c | 12 +- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 191df9e..7ab5a03 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3909,47 +3909,47 @@ enum punit_power_well { /* drain latency register values*/ #define DRAIN_LATENCY_PRECISION_32 32 -#define DRAIN_LATENCY_PRECISION_16 16 +#define DRAIN_LATENCY_PRECISION_64 64 #define VLV_DDL1 (VLV_DISPLAY_BASE + 0x70050) -#define DDL_CURSORA_PRECISION_32 (131) -#define DDL_CURSORA_PRECISION_16 (031) +#define DDL_CURSORA_PRECISION_64 (131) +#define DDL_CURSORA_PRECISION_32 (031) #define DDL_CURSORA_SHIFT 24 -#define DDL_SPRITEB_PRECISION_32 (123) -#define DDL_SPRITEB_PRECISION_16 (023) +#define DDL_SPRITEB_PRECISION_64 (123) +#define DDL_SPRITEB_PRECISION_32 (023) #define DDL_SPRITEB_SHIFT 16 -#define DDL_SPRITEA_PRECISION_32 (115) -#define DDL_SPRITEA_PRECISION_16 (015) +#define DDL_SPRITEA_PRECISION_64 (115) +#define DDL_SPRITEA_PRECISION_32 (015) #define DDL_SPRITEA_SHIFT 8 -#define DDL_PLANEA_PRECISION_32(17) -#define DDL_PLANEA_PRECISION_16(07) +#define DDL_PLANEA_PRECISION_64(17) +#define DDL_PLANEA_PRECISION_32(07) #define DDL_PLANEA_SHIFT 0 #define VLV_DDL2 (VLV_DISPLAY_BASE + 0x70054) -#define DDL_CURSORB_PRECISION_32 (131) -#define DDL_CURSORB_PRECISION_16 (031) +#define DDL_CURSORB_PRECISION_64 (131) +#define DDL_CURSORB_PRECISION_32 (031) #define DDL_CURSORB_SHIFT 24 -#define DDL_SPRITED_PRECISION_32 (123) -#define DDL_SPRITED_PRECISION_16 (023) +#define DDL_SPRITED_PRECISION_64 (123) +#define DDL_SPRITED_PRECISION_32 (023) #define DDL_SPRITED_SHIFT 16 -#define DDL_SPRITEC_PRECISION_32 (115) -#define DDL_SPRITEC_PRECISION_16 (015) +#define DDL_SPRITEC_PRECISION_64 (115) +#define DDL_SPRITEC_PRECISION_32 (015) #define DDL_SPRITEC_SHIFT 8 -#define DDL_PLANEB_PRECISION_32(17) -#define DDL_PLANEB_PRECISION_16(07) +#define DDL_PLANEB_PRECISION_64(17) +#define DDL_PLANEB_PRECISION_32(07) #define DDL_PLANEB_SHIFT 0 #define VLV_DDL3 (VLV_DISPLAY_BASE + 0x70058) -#define DDL_CURSORC_PRECISION_32 (131) -#define DDL_CURSORC_PRECISION_16 (031) +#define DDL_CURSORC_PRECISION_64 (131) +#define DDL_CURSORC_PRECISION_32 (031) #define DDL_CURSORC_SHIFT 24 -#define DDL_SPRITEF_PRECISION_32 (123) -#define DDL_SPRITEF_PRECISION_16 (023) +#define DDL_SPRITEF_PRECISION_64 (123) +#define DDL_SPRITEF_PRECISION_32 (023) #define DDL_SPRITEF_SHIFT 16 -#define DDL_SPRITEE_PRECISION_32 (115) -#define DDL_SPRITEE_PRECISION_16 (015) +#define DDL_SPRITEE_PRECISION_64 (115) +#define DDL_SPRITEE_PRECISION_32 (015) #define DDL_SPRITEE_SHIFT 8 -#define DDL_PLANEC_PRECISION_32(17) -#define DDL_PLANEC_PRECISION_16(07) +#define DDL_PLANEC_PRECISION_64(17) +#define DDL_PLANEC_PRECISION_32
Re: [Intel-gfx] [PATCH 24/40] drm/i915: Fix threshold for choosing 32 vs. 64 precisions for VLV DDL values
2014-06-27 20:04 GMT-03:00 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com The DDL registers can hold 7bit numbers. Make the most of those seven bits by adjusting the threshold where we switch between the 64 vs. 32 precision multipliers. Also we compute 'entries' to make the decision about precision, and then we recompute the same value to calculate the actual drain latency. Just use the already calculate 'entries' there. Just an addition: don't we also want to WARN in case entires 64 (or in case the final result exceeds 7 bits, which is equivalent)? Could be a separate patch too. With or without that: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 9413184..3aa7959 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1252,15 +1252,14 @@ static bool vlv_compute_drain_latency(struct drm_device *dev, pixel_size = crtc-primary-fb-bits_per_pixel / 8; /* BPP */ entries = (clock / 1000) * pixel_size; - *plane_prec_mult = (entries 256) ? + *plane_prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; - *plane_dl = (64 * (*plane_prec_mult) * 4) / ((clock / 1000) * -pixel_size); + *plane_dl = (64 * (*plane_prec_mult) * 4) / entries; entries = (clock / 1000) * 4; /* BPP is always 4 for cursor */ - *cursor_prec_mult = (entries 256) ? + *cursor_prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; - *cursor_dl = (64 * (*cursor_prec_mult) * 4) / ((clock / 1000) * 4); + *cursor_dl = (64 * (*cursor_prec_mult) * 4) / entries; return true; } -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] Displayport compliance testing
Paulo Zanoni mailto:przan...@gmail.com Tuesday, July 29, 2014 2:53 PM 2014-07-22 18:11 GMT-03:00 Jesse Barnesjbar...@virtuousgeek.org: On Tue, 22 Jul 2014 22:53:44 +0200 Daniel Vetterdan...@ffwll.ch wrote: On Tue, Jul 22, 2014 at 10:48 PM, Jesse Barnesjbar...@virtuousgeek.org wrote: Are you saying you'll reject this approach entirely? I'm saying that I don't see terrible lot of value in adding a bunch of code for a sticker, and that we should look into making it actually useful by testing the paths that end-users end up using. And we have to keep this working once it's merged. But if it doesn't make sense to make this sticker useful while still being able to get it then I'll reconsider. Yeah I think it depends on the test. We're supposed to go through existing paths for testing e.g. link training with different params (though with a fixed fb and mode), so getting coverage there is something we want regardless. But getting something like probing covered as part of the compliance testing may be something else entirely... I was finally able to take some time to read the spec, and I agree that the hybrid approach looks like the way to go. Some tests require specifically-crafted FBs, while some other tests cause real hotplug events to be sent from the sink. If there's an unknown/unspecified user-space running when the tests are happening, who knows how it is going to react? Of course, for tests that can be implemented directly inside the Kernel still using the standard code paths, we should do it in the Kernel. There's no question that this is going to require considerable support within the kernel. The tests that can be placed inside the kernel are mostly going to be ones that simply can't be accomplished from userspace (the 400us AUX transaction retry, for instance) or are elements that are necessary to enable the userspace tests to execute properly. The tests themselves though should definitely be handled out in userspace where possible. One possible approach that I thought would be the following: - Each DP encoder provides its own debugfs file for DP test compiance (e.g., /sys/kernel/debug/dri/0/i915_ddi_b_dp_test_compliance). - If the file is not open, any requests for tests that require special actions from our driver - outside of the normal behavior - will be NACKed. There's a couple options here that I've considered. This is a perfectly valid option but it's also possible to just use one debugfs file for the compliance work. It's unlikely that multiple Displayport connectors will ever be testing simultaneously, so the need to adjust the parameters on a per-connector basis is arguable. Only the connector for the sink that issued the test request would be required to read the debugfs file and adhere to its parameters. For general debugging of Displayport though, this is a better option as it allows for considerably more flexibility and utility. Personally I prefer the individual file approach, even though it might end up being somewhat more complicated to implement. The single-file approach does have merit though, so if anyone has a solid argument in favor of that, it's worth the discussion. - If the file is open, we ACK test requests and print special strings to the debugfs file telling the user-space app what it's supposed to do. We could use simple strings like set the preferred mode, set failsafe mode, set mode using FB test pattern Y, etc. A stringly typed protocol :) Probably better to use clearly-defined constants instead of strings, but yes, that's the idea. :) - The user-space app needs to be the DRM master, open the debugfs file, parse the operations it prints and act accordingly, and listen to the hotplug events sent by the Kernel. Agreed. - If some special corner quirky case needs to be done (e.g., train link with a specific number of lanes), the Kernel should store this information at struct intel_dp, and then when a modeset is done on this encoder, we check if the debugfs file is open (i.e., we're doing compliance testing) and then we use the specified configuration. With this, we can probably avoid special uevents or debug-only connector/encoder properties. I would argue here that it's better to leave as much of the active configuration undisturbed as possible and only update the fields necessary to complete the test(s). Additionally, any fields that are changed should be saved and subsequently restored upon completion of the test(s) - The user-space app could be part of intel-gpu-tools. Cool, this is what I was planning on doing. One thing I was looking into was potentially launching the test app from a uevent in the kernel. But none of the solutions I could find were all that good and some were downright scary. So having the app as a separate entity that needs to be launched by the tester isn't such a bad idea. Anyway, this is just an alternate idea to Daniel's suggestion, and many other possible implementation ideas would work for
Re: [Intel-gfx] [PATCH 07/12] drm/i915: Update EDID automated test function for Displayport compliance
Paulo Zanoni mailto:przan...@gmail.com Tuesday, July 29, 2014 3:37 PM 2014-07-14 16:10 GMT-03:00 Todd Previtetprev...@gmail.com: Implements an updated version of the automated testing function that handles Displayport compliance for EDID operations. Both the commit message and the code should mention the name of the specification that defines these tests, and also mention which specific tests are implemented by this patch/function. I see that there are multiple tests being implemented here, but reading the 232 pages of the spec will require too much time, so knowing which ones are implemented really helps the reviewers :) Heh fair enough. I can put all the test information in the commit message. I think it's already in the code, but I'll double-check that as well. Also, you should tell us what happens before and after this patch when you run your own tests. How many tests were we previously passing? How many tests are we passing now? I see there are some FIXME lines below, which leads to the question: is the code you provided enough and complete, or do we still need more adjustments to pass everything we can with what you built? In other words: is this patch, alone, already an improvement to the situation? Makes sense. I'll add the additional info in the commit messages. Signed-off-by: Todd Previtetprev...@gmail.com --- drivers/gpu/drm/i915/intel_dp.c | 77 - 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 33b6dc9..88f1bbe 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3408,7 +3408,82 @@ intel_dp_autotest_video_pattern(struct intel_dp *intel_dp) static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp) { - uint8_t test_result = DP_TEST_NAK; + struct drm_connector *connector =intel_dp-attached_connector-base; + struct i2c_adapter *adapter =intel_dp-aux.ddc; + struct edid *edid_read = NULL; + uint8_t *edid_data = NULL; + uint8_t test_result = DP_TEST_NAK, checksum = 0; + uint32_t i = 0, ret = 0; + struct drm_display_mode *use_mode = NULL; + int mode_count = 0; + struct drm_mode_set modeset; You have initialized every single variable you defined. This creates the problem that unused variables are not pointed by the compiler unless you enable the flags to warn set-but-not-used variables. For example, i is unused, and ret is set but never used. I also don't really see the point in, for example, NULL-initializing stuff like edid_data and edid_read. I always initialize all the variables I declare as a matter of course. I've always been of the opinion that I'd rather use up a little extra stack space than to have to track down the weird problems that can result from using uninitialized variables. Compilers complain about using uninitialized variables as well, so this is more of a difference of opinion than anything else. However, since there are unused variables, I'll remove those in the next iteration of this patch. :) + + DRM_DEBUG_KMS(Displayport: EDID automated test\n); + + /* Reset the NACK/DEFER counters */ As I said before, this is a great example of the comment says what the code already says problem. Fair enough. It will be removed. + intel_dp-aux.i2c_nack_count = 0; + intel_dp-aux.i2c_defer_count = 0; + /* Now read out the EDID */ + edid_read = drm_get_edid(connector, adapter); + + if (edid_read == NULL) { + /* Check for NACKs/DEFERs, goto failsafe if detected + (DP CTS 1.2 Core Rev 1.1, 4.2.2.4, 4.2.2.5) */ Our coding standard is to put aligned '*'s on each line of a multi-line comment. So this should be \t\t * (DP CTS 1.2 etc... instead of what is above. In theory, the /* and */ strings should be on their own lines, alone, but we are inconsistent regarding this (even though Daniel randomly complained about this to me a few times in the past). As usual, check Documentation/CodingStyle for better explanations. I'll adjust the code to conform to the coding standard, whatever it happens to be. + if (intel_dp-aux.i2c_nack_count 0 || + intel_dp-aux.i2c_defer_count 0) We tend to align the contents of the extra line with the '(' char on the line above. I think this is a problem with tab sizes (as evidenced below). My editor is set for 4-space tabs whereas the kernel standard is 8-space tabs. I've resisted changing it because it spaces the code out way too much for my liking while I'm editing. But apparently this is going to be an issue moving forward, so I'm going to have to change it. + DRM_DEBUG_KMS(Displayport: EDID read generated %d I2C NACKs, %d DEFERs\n, + intel_dp-aux.i2c_nack_count, +
[Intel-gfx] [RFC] Sync points/fences for i915
This hasn't seen any testing yet, but I'm still interested in any bugs people see in review, I'll fix them up. If there are no major objections, I'll add some tests and a man page to libdrm for this and we can move forward into the brave new world of fences, giving userspace a lot more rope to hang itself with, and/or tie cool knots. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 25/40] drm/i915: Fill out the FWx watermark register defines
2014-06-27 20:04 GMT-03:00 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com Add defines for all the watermark registers on modernish gmch platforms. VLV has increased the number of bits available for certain watermaks so expand the masks appropriately. Also vlv and chv have added some extra FW registers. Not sure what happened on chv because a new register called FW9 is now at the offset where FW7 was on vlv, while FW7 and FW8 (another new register) have been moved off somewhere else. Oh well, well just need two defines for FW7 then. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 138 +++- drivers/gpu/drm/i915/intel_pm.c | 11 ++-- 2 files changed, 130 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 7ab5a03..9fab647 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3884,28 +3884,136 @@ enum punit_power_well { #define DSPARB_BEND_SHIFT9 /* on 855 */ #define DSPARB_AEND_SHIFT0 +/* pnv/gen4/g4x/vlv/chv */ #define DSPFW1 (dev_priv-info.display_mmio_offset + 0x70034) -#define DSPFW_SR_SHIFT 23 -#define DSPFW_SR_MASK(0x1ff23) -#define DSPFW_CURSORB_SHIFT 16 -#define DSPFW_CURSORB_MASK (0x3f16) -#define DSPFW_PLANEB_SHIFT 8 -#define DSPFW_PLANEB_MASK(0x7f8) -#define DSPFW_PLANEA_MASK(0x7f) +#define DSPFW_SR_SHIFT 23 +#define DSPFW_SR_MASK(0x1ff23) +#define DSPFW_CURSORB_SHIFT 16 +#define DSPFW_CURSORB_MASK (0x3f16) +#define DSPFW_PLANEB_SHIFT 8 +#define DSPFW_PLANEB_MASK(0x7f8) +#define DSPFW_PLANEB_MASK_VLV(0xff8) /* vlv/chv */ +#define DSPFW_PLANEA_SHIFT 0 +#define DSPFW_PLANEA_MASK(0x7f0) +#define DSPFW_PLANEA_MASK_VLV(0xff0) /* vlv/chv */ #define DSPFW2 (dev_priv-info.display_mmio_offset + 0x70038) -#define DSPFW_CURSORA_MASK 0x3f00 -#define DSPFW_CURSORA_SHIFT 8 -#define DSPFW_PLANEC_MASK(0x7f) +#define DSPFW_FBC_SR_EN (131) /* g4x */ +#define DSPFW_FBC_SR_SHIFT 28 +#define DSPFW_FBC_SR_MASK(0x728) /* g4x */ +#define DSPFW_FBC_HPLL_SR_SHIFT 24 +#define DSPFW_FBC_HPLL_SR_MASK (0xf24) /* g4x */ +#define DSPFW_SPRITEB_SHIFT (16) +#define DSPFW_SPRITEB_MASK (0x7f16) /* g4x */ +#define DSPFW_SPRITEB_MASK_VLV (0xff16) /* vlv/chv */ +#define DSPFW_CURSORA_SHIFT 8 +#define DSPFW_CURSORA_MASK (0x3f8) +#define DSPFW_PLANEC_SHIFT_OLD 0 +#define DSPFW_PLANEC_MASK_OLD(0x7f0) /* pre-gen4 sprite C */ +#define DSPFW_SPRITEA_SHIFT 0 +#define DSPFW_SPRITEA_MASK (0x7f0) /* g4x */ +#define DSPFW_SPRITEA_MASK_VLV (0xff0) /* vlv/chv */ #define DSPFW3 (dev_priv-info.display_mmio_offset + 0x7003c) -#define DSPFW_HPLL_SR_EN (131) -#define DSPFW_CURSOR_SR_SHIFT24 +#define DSPFW_HPLL_SR_EN (131) #define PINEVIEW_SELF_REFRESH_EN (130) +#define DSPFW_CURSOR_SR_SHIFT24 #define DSPFW_CURSOR_SR_MASK (0x3f24) #define DSPFW_HPLL_CURSOR_SHIFT 16 #define DSPFW_HPLL_CURSOR_MASK (0x3f16) -#define DSPFW_HPLL_SR_MASK (0x1ff) -#define DSPFW4 (dev_priv-info.display_mmio_offset + 0x70070) -#define DSPFW7 (dev_priv-info.display_mmio_offset + 0x7007c) +#define DSPFW_HPLL_SR_SHIFT 0 +#define DSPFW_HPLL_SR_MASK (0x1ff0) + +/* vlv/chv */ +#define DSPFW4 (VLV_DISPLAY_BASE + 0x70070) +#define DSPFW_SPRITEB_WM1_SHIFT 16 +#define DSPFW_SPRITEB_WM1_MASK (0xff16) +#define DSPFW_CURSORA_WM1_SHIFT 8 +#define DSPFW_CURSORA_WM1_MASK (0x3f8) +#define DSPFW_SPRITEA_WM1_SHIFT 0 +#define DSPFW_SPRITEA_WM1_MASK (0xff0) +#define DSPFW5 (VLV_DISPLAY_BASE + 0x70074) +#define DSPFW_PLANEB_WM1_SHIFT 24 +#define DSPFW_PLANEB_WM1_MASK(0xff24) +#define DSPFW_PLANEA_WM1_SHIFT 16 +#define DSPFW_PLANEA_WM1_MASK(0xff16) +#define DSPFW_CURSORB_WM1_SHIFT 8 +#define DSPFW_CURSORB_WM1_MASK (0x3f8) +#define DSPFW_CURSOR_SR_WM1_SHIFT0 +#define DSPFW_CURSOR_SR_WM1_MASK (0x3f0) +#define DSPFW6 (VLV_DISPLAY_BASE + 0x70078) +#define DSPFW_SR_WM1_SHIFT 0 +#define DSPFW_SR_WM1_MASK(0x1ff0) +#define DSPFW7 (VLV_DISPLAY_BASE + 0x7007c) +#define DSPFW7_CHV (VLV_DISPLAY_BASE + 0x700b4) /* wtf #1? */ +#define DSPFW_SPRITED_WM1_SHIFT 24 +#define
Re: [Intel-gfx] [PATCH 28/40] drm/i915: Add cherryview_update_wm()
2014-06-27 20:04 GMT-03:00 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com CHV has a third pipe so we need to compute the watermarks for its planes. Add cherryview_update_wm() to do just that. Ok, so basically the only real difference between this code and VLV's code is when you enable CXSR: on VLV you just enable CXSR after the other WM registers are already written. I wonder if this is to prevent any intermediate situations where the previous WM values did not allow CXSR, so enabling it first would result in errors/underruns. On this case, the CHV function would need to do the same thing as VLV, right? Do you have any specific reason for keeping the CXSR code different on CHV? Also, instead of adding a new function, you could probably just rewrite vlv_update_wm to use for_each_pipe() instead of the current method. You'd define plane_wm[num_pipes] arrays instead of one variable per pipe, then you would be able to use the same function for both VLV and CHV. Anyway, I don't think we should block your patch based on this suggestion, so if you just provide a good explanation for the CXSR question - or a new patch - I'll give a R-B tag. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 77 - 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index cb0b4b4..346dced 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1364,6 +1364,81 @@ static void valleyview_update_wm(struct drm_crtc *crtc) (cursor_sr DSPFW_CURSOR_SR_SHIFT)); } +static void cherryview_update_wm(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc-dev; + static const int sr_latency_ns = 12000; + struct drm_i915_private *dev_priv = dev-dev_private; + int planea_wm, planeb_wm, planec_wm; + int cursora_wm, cursorb_wm, cursorc_wm; + int plane_sr, cursor_sr; + int ignore_plane_sr, ignore_cursor_sr; + unsigned int enabled = 0; + + vlv_update_drain_latency(dev); + + if (g4x_compute_wm0(dev, PIPE_A, + valleyview_wm_info, latency_ns, + valleyview_cursor_wm_info, latency_ns, + planea_wm, cursora_wm)) + enabled |= 1 PIPE_A; + + if (g4x_compute_wm0(dev, PIPE_B, + valleyview_wm_info, latency_ns, + valleyview_cursor_wm_info, latency_ns, + planeb_wm, cursorb_wm)) + enabled |= 1 PIPE_B; + + if (g4x_compute_wm0(dev, PIPE_C, + valleyview_wm_info, latency_ns, + valleyview_cursor_wm_info, latency_ns, + planec_wm, cursorc_wm)) + enabled |= 1 PIPE_C; + + if (single_plane_enabled(enabled) + g4x_compute_srwm(dev, ffs(enabled) - 1, +sr_latency_ns, +valleyview_wm_info, +valleyview_cursor_wm_info, +plane_sr, ignore_cursor_sr) + g4x_compute_srwm(dev, ffs(enabled) - 1, +2*sr_latency_ns, +valleyview_wm_info, +valleyview_cursor_wm_info, +ignore_plane_sr, cursor_sr)) { + I915_WRITE(FW_BLC_SELF_VLV, FW_CSPWRDWNEN); + } else { + I915_WRITE(FW_BLC_SELF_VLV, + I915_READ(FW_BLC_SELF_VLV) ~FW_CSPWRDWNEN); + plane_sr = cursor_sr = 0; + } + + DRM_DEBUG_KMS(Setting FIFO watermarks - A: plane=%d, cursor=%d, + B: plane=%d, cursor=%d, C: plane=%d, cursor=%d, + SR: plane=%d, cursor=%d\n, + planea_wm, cursora_wm, + planeb_wm, cursorb_wm, + planec_wm, cursorc_wm, + plane_sr, cursor_sr); + + I915_WRITE(DSPFW1, + (plane_sr DSPFW_SR_SHIFT) | + (cursorb_wm DSPFW_CURSORB_SHIFT) | + (planeb_wm DSPFW_PLANEB_SHIFT) | + (planea_wm DSPFW_PLANEA_SHIFT)); + I915_WRITE(DSPFW2, + (I915_READ(DSPFW2) ~DSPFW_CURSORA_MASK) | + (cursora_wm DSPFW_CURSORA_SHIFT)); + I915_WRITE(DSPFW3, + (I915_READ(DSPFW3) ~DSPFW_CURSOR_SR_MASK) | + (cursor_sr DSPFW_CURSOR_SR_SHIFT)); + I915_WRITE(DSPFW9_CHV, + (I915_READ(DSPFW9_CHV) ~(DSPFW_PLANEC_MASK | + DSPFW_CURSORC_MASK)) | + (planec_wm DSPFW_PLANEC_SHIFT) | +
Re: [Intel-gfx] [PATCH 1/1] Documentation: drm: describing drm properties exposed by various drivers
On 05/12/14 11:04, Randy Dunlap wrote: On 05/12/2014 08:54 AM, Daniel Vetter wrote: On Mon, May 12, 2014 at 08:23:45AM -0700, Randy Dunlap wrote: On 05/12/2014 01:58 AM, Daniel Vetter wrote: On Mon, May 12, 2014 at 06:24:57PM +1000, Dave Airlie wrote: If we decide to go for property documentation inside the source code then I believe we'll have to create our own format, as creating a properties table from kerneldoc information extracted from comments is probably not possible. Can comeone pick up the ball here and figure out what needs to be done? The reason why I want a central place for the documentation is to force people to collaborate outside their own sandbox when adding properties. Whether that's docbook or some text file I don't care so much at this point. The fact that it's a central place should mandate that the patches changing it will go through dri-devel and so everyone should se them, and when adding new properties it would make the patch author more likely to look around a bit before adding another slighty incompatible version of the same property. If someone has a better suggestion how to encforce this I'm all ears. Of course this idea can still fail if our esteemed maintainer merges stuff without checking for violations of this policy. Dave, any thoughts on the subject? Yeah I'm happy to block merging stuff, if we can spot new properties when stuff is posted on dri-devel, so much the better, most drivers still send everything via dri-devel anyways, its only really Intel I have to worry about so far, I'll enforce that all prop stuff gets cc: dri-devel and that it has updates for the prop docs. But we should definitely add it to the new driver review checklist as well. I'm also on the side of this patch is ugly and makes my eyes burn, please please get a plan to use something else ASAP, I'm willing to merge this but I'm tempted to give it a lifetime of a kernel or two before I burn it. Ok, I'll try to move make kerneldoc suck less up the task list and maybe find someone to do it for me internally ;-) -Daniel I certainly have no objections to making kerneldoc suck less. There was already an attempt to use asciidoc (like git uses) for kernel-doc (a few years ago, by Sam Ravnborg). I support(ed) that effort. Hm, do you have pointers to those? My google-fu seems lacking ... I googled for /kernel doc asciidoc ravnborg/ and found several hits for them. Ok, let's move this to the top and start discussions. The past few months I've written piles of kerneldoc comments for the DRM DocBook (all pulled in as kerneldoc, docbook .tmpl has just the chapter structure). DOC: sections are really useful to pull all the actual documentation out of the docbook xml into kerneldoc. But I've also done piles of docs for intel-gpu-tools, which is using gtkdoc. And there are some clear deficiencies: - No markdown for inline coments. Lack of lists and tables is hurting especially badly. If we add this (and I don't care one iota whether it's Yes, I've tried to add lists to kernel-doc notation but have failed so far. markdown or asciidoc or something else as long as it's readable plain text in comments) we should be able to move all the existing docbook xml paragraphs/lists/tables into kerneldoc comments. - Automatic cross-referencing of functions. If you place e.g. function() or #struct anywhere in a documentation comment gtk-doc automatically inserts a hyperlink to the relevant documentation page across the entire project. Really powerful and makes overview sections much more useful entry points for beginners since they can easily jump backforth betweeen the high-level overview and low-level per-function documentation. That's a nice-to-have IMO, but a really nice one. - In a really perfect world it would help if kerneldoc could collect structure member kerneldoc from per-member comments. Especially for large structures with lots of comments this would bring the kerneldoc and struct member much closer together. So that's my wishlist. OTOH, I would only want to add another way to do kernel-doc if it can be a full replacement for all of our docbook usage, i.e., it should provide a way that we can eliminate docbook and stop using it completely. Hm, I don't mind docbook at all, as long as all the real content is embedded into source files as kerneldoc and the docbook template just pulls in all the right bits and pieces. Even gtkdoc allos you to do that and pull in the different libararies (== header files with declarations for C) in the order you want. So imo the docbook toolchain is good enough for my needs. Or what do you mean by getting rid of all docbook usage? I meant no docbook style sheets, no 'xmlto', the whole ball of wax. But primarily I don't want to see drivers/video/ using one set of doc tools and drivers/media/ using another set and drivers/xyz/ using its own
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
On Thu, Jul 31, 2014 at 04:53:34AM -0700, Mcaulay, Alistair wrote: Hi Jeff, These patches look like they solve the problem well. I've added some comments in amongst the code. Thanks, Alistair. -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of jeff.mc...@intel.com Sent: Thursday, July 31, 2014 3:00 AM To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM From: Jeff McGee jeff.mc...@intel.com Define a struct to capture information on the device's Slice/Subslice/EU (SSEU) configuration. Add this struct to the main device info struct. Define a packed bitfield form for the SSEU info and share it with userspace via a new GETPARAM option. Starting with Cherryview, devices may have a varying number of EU for a given ID due to creative fusing. The surest way to determine the configuration is by reading fuses which is best done in the kernel and communicated to userspace. The immediate need from userspace is to determine the number of threads of compute work that can be safely submitted. The definition of SSEU as a new drm/i915 component, with its own header file and soon-to-be source file, is in anticipation of lots of upcoming code for its management, particularly the power gating functionality. Signed-off-by: Jeff McGee jeff.mc...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 3 +++ drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_sseu.h | 40 +++ include/uapi/drm/i915_drm.h | 18 ++ 4 files changed, 64 insertions(+) create mode 100644 drivers/gpu/drm/i915/intel_sseu.h diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2e7f03a..f581848 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_CMD_PARSER_VERSION: value = i915_cmd_parser_get_version(); break; + case I915_PARAM_SSEU_INFO: + value = INTEL_INFO(dev)-sseu.gp_sseu_info; + break; default: DRM_DEBUG(Unknown parameter %d\n, param-param); return -EINVAL; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 18c9ad8..01adafd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -45,6 +45,7 @@ #include linux/intel-iommu.h #include linux/kref.h #include linux/pm_qos.h +#include intel_sseu.h /* General customization: */ @@ -562,6 +563,8 @@ struct intel_device_info { int trans_offsets[I915_MAX_TRANSCODERS]; int palette_offsets[I915_MAX_PIPES]; int cursor_offsets[I915_MAX_PIPES]; + /* Slice/Subslice/EU info */ + struct intel_sseu_info sseu; }; #undef DEFINE_FLAG diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h new file mode 100644 index 000..7db7175 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_sseu.h @@ -0,0 +1,40 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person +obtaining a + * copy of this software and associated documentation files (the +Software), + * to deal in the Software without restriction, including without +limitation + * the rights to use, copy, modify, merge, publish, distribute, +sublicense, + * and/or sell copies of the Software, and to permit persons to whom +the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the +next + * paragraph) shall be included in all copies or substantial portions +of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR +OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS + * IN THE SOFTWARE. + * + */ +#ifndef _INTEL_SSEU_H_ +#define _INTEL_SSEU_H_ + +struct intel_sseu_info { + /* Total slice count */ + unsigned int slice_cnt; + /* Total subslice count */ + unsigned int subslice_cnt; + /* Total execution unit count */ + unsigned int eu_cnt; + /* Thread count per EU */ + unsigned int threads_per_eu; + /* Bit field representation for I915_PARAM_SSEU_INFO */ + u32 gp_sseu_info; +}; + +#endif diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index ff57f07..b99c1a2
Re: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
On Thu, Jul 31, 2014 at 04:55:58AM -0700, Mcaulay, Alistair wrote: Hi Jeff, Some more comments in the code. Thanks, Alistair. -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of jeff.mc...@intel.com Sent: Thursday, July 31, 2014 3:00 AM To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV From: Jeff McGee jeff.mc...@intel.com Cherryview can have different SSEU configurations within a given PCI ID, so we collect the info from the fuse register. I don't currently have access to a CHV, much less one with an interesting fuse config. So I have compile-checked this only! Signed-off-by: Jeff McGee jeff.mc...@intel.com --- drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/i915_dma.c | 2 ++ drivers/gpu/drm/i915/i915_reg.h | 13 drivers/gpu/drm/i915/intel_sseu.c | 64 +++ drivers/gpu/drm/i915/intel_sseu.h | 2 ++ 5 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_sseu.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..9a0f411 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \ i915_irq.o \ i915_trace_points.o \ intel_ringbuffer.o \ - intel_uncore.o + intel_uncore.o \ + intel_sseu.o # autogenerated null render state i915-y += intel_renderstate_gen6.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f581848..384ef65 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_gem_unload; } + intel_sseu_init(dev); + intel_power_domains_init(dev_priv); if (drm_core_check_feature(dev, DRIVER_MODESET)) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..24a2d56 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5624,6 +5624,19 @@ enum punit_power_well { #define GEN7_MISCCPCTL (0x9424) #define GEN7_DOP_CLOCK_GATE_ENABLE (10) +/* Fuse readout registers for GT */ +#define CHV_FUSE_GT 0x182168 +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 10 +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS1 11 These should be: #define CHV_FUSE_GT_SUBSLICE_DISABLE_SS0(1 10) #define CHV_FUSE_GT_SUBSLICE_DISABLE_SS1(1 11) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK (0xf16) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT 16 Why not use the shift define here? #define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK (0xfCHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT) and for the others. See answer in previous patch -Jeff +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK (0xf20) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT 20 +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK (0xf24) +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT 24 +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK (0xf28) +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT 28 + /* IVYBRIDGE DPF */ #define GEN7_L3CDERRST1 0xB008 /* L3CD Error Status 1 */ #define HSW_L3CDERRST11 0xB208 /* L3CD Error Status register 1 slice 1 */ diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c new file mode 100644 index 000..6ba4830 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_sseu.c @@ -0,0 +1,64 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person +obtaining a + * copy of this software and associated documentation files (the +Software), + * to deal in the Software without restriction, including without +limitation + * the rights to use, copy, modify, merge, publish, distribute, +sublicense, + * and/or sell copies of the Software, and to permit persons to whom +the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the +next + * paragraph) shall be included in all copies or substantial portions +of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR +OTHER + *
Re: [Intel-gfx] [PATCH 1/2] drm/i915/hdmi: call intel_hdmi_prepare for CHV
Hi Daniel, Get it. Thanks. Regards, Libin -Original Message- From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Thursday, July 31, 2014 5:51 PM To: Yang, Libin Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/hdmi: call intel_hdmi_prepare for CHV On Thu, Jul 31, 2014 at 9:16 AM, Yang, Libin libin.y...@intel.com wrote: BTW: I'm not familiar with gfx driver. But I see intel_hdmi_prepare() is called in the pre_pll_enable() for chv and vlv, but in others, it is called in pre_enable(). Could you tell me what's the difference between the two functions? I'm not sure if there is some impact on the audio function. Depending upon the platform the exact modeset sequence is not always the same. So we mixmatch and put the different stages at the right step. Which means the hdmi_prepare step isn't called on all platforms from exactly the same functions - platforms other than vlv/chv don't even have a pre_pll_enable step in the hdmi encoder. It's a bit complicated, but this way we can support pretty much all intel gfx ever shipped in one driver without massive amounts of duplicated code. I hope that explains the idea a bit. -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: Introduce FBC False Color for debug purposes.
With this bit enabled, HW changes the color when compressing frames for debug purposes. ALthough the simple way to enable a single bit is over intel_reg_write, this value is overwriten on next update_fbc so depending on the workload it is not possible to set this bit with intel-gpu-tools. So this patch introduces a persistent way to enable false color over debugfs. v2: Use DEFINE_SIMPLE_ATTRIBUTE as Daniel suggested v3: (Ville) only do false color for IVB+ since according to spec bit is MBZ before IVB. v4: We don't have FBC on valleyview nor on cherryview (Ben) Reviewed-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 42 + drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 3 +++ 4 files changed, 48 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9e737b7..2147b41 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1433,6 +1433,47 @@ static int i915_fbc_status(struct seq_file *m, void *unused) return 0; } +static int i915_fbc_fc_get(void *data, u64 *val) +{ + struct drm_device *dev = data; + struct drm_i915_private *dev_priv = dev-dev_private; + + if (INTEL_INFO(dev)-gen 7 || !HAS_PCH_SPLIT(dev)) + return -ENODEV; + + drm_modeset_lock_all(dev); + *val = dev_priv-fbc.false_color; + drm_modeset_unlock_all(dev); + + return 0; +} + +static int i915_fbc_fc_set(void *data, u64 val) +{ + struct drm_device *dev = data; + struct drm_i915_private *dev_priv = dev-dev_private; + u32 reg; + + if (INTEL_INFO(dev)-gen 7 || !HAS_PCH_SPLIT(dev)) + return -ENODEV; + + drm_modeset_lock_all(dev); + + reg = I915_READ(ILK_DPFC_CONTROL); + dev_priv-fbc.false_color = val; + + I915_WRITE(ILK_DPFC_CONTROL, val ? + (reg | FBC_CTL_FALSE_COLOR) : + (reg ~FBC_CTL_FALSE_COLOR)); + + drm_modeset_unlock_all(dev); + return 0; +} + +DEFINE_SIMPLE_ATTRIBUTE(i915_fbc_fc_fops, + i915_fbc_fc_get, i915_fbc_fc_set, + %llu\n); + static int i915_ips_status(struct seq_file *m, void *unused) { struct drm_info_node *node = m-private; @@ -3957,6 +3998,7 @@ static const struct i915_debugfs_files { {i915_pri_wm_latency, i915_pri_wm_latency_fops}, {i915_spr_wm_latency, i915_spr_wm_latency_fops}, {i915_cur_wm_latency, i915_cur_wm_latency_fops}, + {i915_fbc_false_color, i915_fbc_fc_fops}, }; void intel_display_crc_init(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d604f4f..3a29f9e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -636,6 +636,8 @@ struct i915_fbc { struct drm_mm_node compressed_fb; struct drm_mm_node *compressed_llb; + bool false_color; + struct intel_fbc_work { struct delayed_work work; struct drm_crtc *crtc; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..b5d295a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1540,6 +1540,7 @@ enum punit_power_well { /* Framebuffer compression for Ironlake */ #define ILK_DPFC_CB_BASE 0x43200 #define ILK_DPFC_CONTROL 0x43208 +#define FBC_CTL_FALSE_COLOR (110) /* The bit 28-8 is reserved */ #define DPFC_RESERVED(0x1F00) #define ILK_DPFC_RECOMP_CTL0x4320c diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1ddd4df..338a80b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -309,6 +309,9 @@ static void gen7_enable_fbc(struct drm_crtc *crtc) dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN; + if (dev_priv-fbc.false_color) + dpfc_ctl |= FBC_CTL_FALSE_COLOR; + I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); if (IS_IVYBRIDGE(dev)) { -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Skip Stolen Memory first page.
WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle v2: Improve variable names and fix allocated size. Reviewed-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 21c025a..82035b0 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -289,7 +289,8 @@ void i915_gem_cleanup_stolen(struct drm_device *dev) int i915_gem_init_stolen(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; - int bios_reserved = 0; + int start_rsvd = 0; + int end_rsvd = 0; #ifdef CONFIG_INTEL_IOMMU if (intel_iommu_gfx_mapped INTEL_INFO(dev)-gen 8) { @@ -308,15 +309,19 @@ int i915_gem_init_stolen(struct drm_device *dev) DRM_DEBUG_KMS(found %zd bytes of stolen memory at %08lx\n, dev_priv-gtt.stolen_size, dev_priv-mm.stolen_base); + /* WaSkipStolenMemoryFirstPage */ + if (INTEL_INFO(dev)-gen = 8) + start_rsvd = 4096; + if (IS_VALLEYVIEW(dev)) - bios_reserved = 1024*1024; /* top 1M on VLV/BYT */ + end_rsvd = 1024*1024; /* top 1M on VLV/BYT */ - if (WARN_ON(bios_reserved dev_priv-gtt.stolen_size)) + if (WARN_ON((start_rsvd + end_rsvd) dev_priv-gtt.stolen_size)) return 0; /* Basic memrange allocator for stolen space */ - drm_mm_init(dev_priv-mm.stolen, 0, dev_priv-gtt.stolen_size - - bios_reserved); + drm_mm_init(dev_priv-mm.stolen, start_rsvd, + dev_priv-gtt.stolen_size - start_rsvd - end_rsvd); return 0; } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW
According to spec FBC on BDW and HSW are identical without any gaps. So let's copy the nuke and let FBC really start compressing stuff. Without this patch we can verify with false color that nothing is being compressed. With the nuke in place and false color it is possible to see false color debugs. v2: Fix rebase conflict. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2908896..4ba3db1 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -406,6 +406,7 @@ gen8_render_ring_flush(struct intel_engine_cs *ring, { u32 flags = 0; u32 scratch_addr = ring-scratch.gtt_offset + 2 * CACHELINE_BYTES; + int ret; flags |= PIPE_CONTROL_CS_STALL; @@ -424,7 +425,14 @@ gen8_render_ring_flush(struct intel_engine_cs *ring, flags |= PIPE_CONTROL_GLOBAL_GTT_IVB; } - return gen8_emit_pipe_control(ring, flags, scratch_addr); + ret = gen8_emit_pipe_control(ring, flags, scratch_addr); + if (ret) + return ret; + + if (!invalidate_domains flush_domains) + return gen7_ring_fbc_flush(ring, FBC_REND_NUKE); + + return 0; } static void ring_write_tail(struct intel_engine_cs *ring, @@ -2065,7 +2073,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring, } intel_ring_advance(ring); - if (IS_GEN7(dev) !invalidate flush) + if (INTEL_INFO(dev)-gen = 7 !invalidate flush) return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN); return 0; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] intel-gpu-tools: Change type of variable from unsigned to uint64_t in gem_stress
To run gem_stress with the correct number of buffers even if aperture size = 4GB. Signed-off-by: Pavel Popov pavel.e.po...@intel.com --- tests/gem_stress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/gem_stress.c b/tests/gem_stress.c index 3bbe487..313a82a 100644 --- a/tests/gem_stress.c +++ b/tests/gem_stress.c @@ -737,7 +737,7 @@ static int parse_options(int opt, int opt_index) static void init(void) { int i; - unsigned tmp; + uint64_t tmp; if (options.num_buffers == 0) { tmp = gem_aperture_size(drm_fd); -- 1.9.1 Closed Joint Stock Company Intel A/O Registered legal address: Krylatsky Hills Business Park, 17 Krylatskaya Str., Bldg 4, Moscow 121614, Russian Federation This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx