Re: [Intel-gfx] [PATCH v4 00/21] Add support for GuC-based SLPC

[O'Rourke, Tom]

On Wed, Apr 27, 2016 at 06:10:44PM -0700, tom.orou...@intel.com wrote:
> From: Tom O'Rourke 
> 
> SLPC (Single Loop Power Controller) is a replacement for
> some host-based power management features.  The SLPC
> implemenation runs in firmware on GuC.
> 
> This series has been tested with SKL guc firmware
> version 6.1.
> 
> The graphics power management features in SLPC in those
> versions are called GTPERF, BALANCER, and DCC.
> 
> GTPERF is a combination of DFPS (Dynamic FPS) and Turbo.
> DFPS adjusts requested graphics frequency to maintain
> target framerate.  Turbo adjusts requested graphics
> frequency to maintain target GT busyness; this includes
> an adaptive boost turbo method.
> 
> BALANCER adjusts balance between power budgets for IA
> and GT in power limited scenarios.  BALANCER is only
> active when all display pipes are in "game" mode.
> 
> DCC (Duty Cycle Control) adjusts requested graphics
> frequency and stalls guc-scheduler to maintain actual
> graphics frequency in efficient range.
> 
> The v3 series can be found in the archive at
> "[Intel-gfx] [PATCH v3 00/25] Add support for GuC-based SLPC"
> https://lists.freedesktop.org/archives/intel-gfx/2016-April/091771.html
> 
> This v4 series incorporates feedback from internal code 
> reviews for Android and Yocto projects.  This series also 
> drops the Broxton patches; the Broxton firmware has not 
> been published yet.  Broxton support can be added later 
> when the Broxton firmware is available. 
> 
> Also, the "DO NOT MERGE" patches to enable SLPC and guc 
> submission by default have been dropped.  These can be 
> added later after SLPC has been shown to outperform 
> host-based power management; this may require a newer 
> version of the GuC firmware.
> 
> With SLPC disabled by default, this series should be 
> safe to merge now. 
> 
> VIZ-6773, VIZ-6889

Thank you to Chris, Daniel and Imre for your comments.  
I agree that some of the suggested changes should be made.  

Whether those changes should be made before merging or with 
later patches will be someone else's problem.  

I won't be sending another version of this series.

Thanks,
Tom O'Rourke

> 
> Sagar Arun Kamble (4):
>   drm/i915/slpc: Add Display mode event related data structures
>   drm/i915/slpc: Notification of Display mode change
>   drm/i915/slpc: Notification of Refresh Rate change
>   drm/i915/slpc: Fail intel_runtime_suspend if SLPC or RPS not active
> 
> Tom O'Rourke (17):
>   drm/i915/slpc: Expose guc functions for use with SLPC
>   drm/i915/slpc: Add has_slpc capability flag
>   drm/i915/slpc: Add slpc_version_check
>   drm/i915/slpc: Add enable_slpc module parameter
>   drm/i915/slpc: Use intel_slpc_* functions if supported
>   drm/i915/slpc: Enable SLPC in guc if supported
>   drm/i915/slpc: If using SLPC, do not set frequency
>   drm/i915/slpc: Allocate/Release/Initialize SLPC shared data
>   drm/i915/slpc: Setup rps frequency values during SLPC init
>   drm/i915/slpc: Update current requested frequency
>   drm/i915/slpc: Send reset event
>   drm/i915/slpc: Send shutdown event
>   drm/i915/slpc: Add slpc_status enum values
>   drm/i915/slpc: Add parameter unset/set/get functions
>   drm/i915/slpc: Add slpc support for max/min freq
>   drm/i915/slpc: Add enable/disable debugfs for slpc
>   drm/i915/slpc: Add i915_slpc_info to debugfs
> 
>  drivers/gpu/drm/i915/Makefile  |   5 +-
>  drivers/gpu/drm/i915/i915_debugfs.c| 456 +
>  drivers/gpu/drm/i915/i915_drv.c|   4 +-
>  drivers/gpu/drm/i915/i915_drv.h|   7 +
>  drivers/gpu/drm/i915/i915_guc_submission.c |   6 +-
>  drivers/gpu/drm/i915/i915_params.c |   6 +
>  drivers/gpu/drm/i915/i915_params.h |   1 +
>  drivers/gpu/drm/i915/i915_reg.h|   1 +
>  drivers/gpu/drm/i915/i915_sysfs.c  |  21 ++
>  drivers/gpu/drm/i915/intel_display.c   |   2 +
>  drivers/gpu/drm/i915/intel_dp.c|   2 +
>  drivers/gpu/drm/i915/intel_drv.h   |  11 +
>  drivers/gpu/drm/i915/intel_guc.h   |  13 +
>  drivers/gpu/drm/i915/intel_guc_loader.c|  36 ++
>  drivers/gpu/drm/i915/intel_pm.c|  42 ++-
>  drivers/gpu/drm/i915/intel_slpc.c  | 516 
> +
>  drivers/gpu/drm/i915/intel_slpc.h  | 217 
>  17 files changed, 1329 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_slpc.c
>  create mode 100644 drivers/gpu/drm/i915/intel_slpc.h
> 
> -- 
> 1.9.1
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/guc: Use major_minor version for filename

[O'Rourke, Tom]

Hello,
These errors are unrelated to the major_minor version change.
Thanks,
Tom

>-Original Message-
>From: Patchwork [mailto:patchw...@emeril.freedesktop.org]
>Sent: Monday, April 25, 2016 10:18 PM
>To: O'Rourke, Tom <tom.orou...@intel.com>
>Cc: intel-gfx@lists.freedesktop.org
>Subject: ✗ Fi.CI.BAT: failure for drm/i915/guc: Use major_minor version for
>filename
>
>== Series Details ==
>
>Series: drm/i915/guc: Use major_minor version for filename
>URL   : https://patchwork.freedesktop.org/series/6293/
>State : failure
>
>== Summary ==
>
>Series 6293v1 drm/i915/guc: Use major_minor version for filename
>http://patchwork.freedesktop.org/api/1.0/series/6293/revisions/1/mbox/
>
>Test drv_hangman:
>Subgroup error-state-basic:
>incomplete -> PASS   (snb-dellxps)
>Test gem_busy:
>Subgroup basic-bsd1:
>pass   -> DMESG-WARN (skl-nuci5)
>Test gem_sync:
>Subgroup basic-bsd:
>pass   -> DMESG-WARN (skl-nuci5)
>Test kms_pipe_crc_basic:
>Subgroup hang-read-crc-pipe-a:
>pass   -> FAIL   (skl-nuci5)
>Subgroup read-crc-pipe-b-frame-sequence:
>skip   -> PASS   (bdw-nuci7)
>Subgroup read-crc-pipe-c-frame-sequence:
>pass   -> DMESG-WARN (skl-nuci5)
>
>bdw-nuci7total:200  pass:188  dwarn:0   dfail:0   fail:0   skip:12
>bsw-nuc-2total:199  pass:158  dwarn:0   dfail:0   fail:0   skip:41
>hsw-brixbox  total:200  pass:174  dwarn:0   dfail:0   fail:0   skip:26
>skl-i7k-2total:200  pass:173  dwarn:0   dfail:0   fail:0   skip:27
>skl-nuci5total:200  pass:185  dwarn:3   dfail:0   fail:1   skip:11
>snb-dellxps  total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:38
>bdw-ultra failed to connect after reboot ilk-hp8440p failed to connect after
>reboot ivb-t430s failed to connect after reboot
>
>Results at /archive/results/CI_IGT_test/Patchwork_2065/
>
>f814551aa7232ed36d71244dd148b48660b53a78 drm-intel-nightly: 2016y-04m-
>25d-11h-36m-27s UTC integration manifest
>bc7fd31 drm/i915/guc: Use major_minor version for filename

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

Re: [Intel-gfx] [PATCH 4/3] drm/i915: Read RPS frequencies earlier on non-VLV/CHV

[O'Rourke, Tom]

On Mon, Mar 07, 2016 at 08:57:09PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Read out the RPS frequencies already in intel_init_gt_powersave()
> on all the platforms. So far we only did that on VLV/CHV, and the
> rest of the platforms read them out at rps enable time, which happens
> asynchronously from a workqueue. Reading them out earlier prevents
> userspace from reading out invalid (zero) values via the relevant
> sysfs files before the rps enable work has been executed.
> 
> This used to be prevented by the flush_delayed_work() + locking
> in the sysfs code, but now that we no longer do that, we run the
> risk of letting userspace see the initial zeroed values.
> 
> Note that it's still possible to read out cur_freq as 0, since that
> only gets initialized from the delayed rps enable. Should that pose
> a real problem, I guess we could always add the flush+locking back
> for the cur_freq read.
> 
> Signed-off-by: Ville Syrjälä 
> ---

Hello,

The flush_delayed_work() calls were added in:
"commit 5c9669cee534cbb834d51aae115267f5e561b622
Author: Tom O'Rourke 
Date:   Mon Sep 16 14:56:43 2013 -0700

drm/i915: Finish enabling rps before use by sysfs or debugfs"

This change was made to address use before initialization
problems in min/max/cur frequency values.  The work queue
being flushed was added in:
"commit 1a01ab3b2dc4394c46c4c3230805748f632f6f74
Author: Jesse Barnes 
Date:   Fri Nov 2 11:14:00 2012 -0700

drm/i915: put ring frequency and turbo setup into a work queue v5

Communicating via the mailbox registers with the PCU can take quite
awhile.  And updating the ring frequency or enabling turbo is not
something that needs to happen synchronously, so take it out of our init
and resume paths to speed things up (~200ms on my T420)."

This change was made to reduce driver load times.

The two concerns I have are:
1) Similar changes would be needed in debugfs files.
2) This change may increase driver load times due to pcode
mailbox command used in gen6_rps_init_frequencies() that
is now in the init path.

I am not objecting to these changes but we should
be aware of the impact to driver load latency.

Thanks,
Tom

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

Re: [Intel-gfx] [RFC 00/22] Add support for GuC-based SLPC

[O'Rourke, Tom]

On Tue, Jan 26, 2016 at 09:17:51AM -0800, Jesse Barnes wrote:
> On 01/26/2016 09:00 AM, Daniel Vetter wrote:
> > On Tue, Jan 26, 2016 at 07:45:42AM -0800, Jesse Barnes wrote:
> >> On 01/22/2016 09:00 AM, Daniel Vetter wrote:
> >>> On Wed, Jan 20, 2016 at 06:26:02PM -0800, tom.orou...@intel.com wrote:
>  From: Tom O'Rourke 
> 
>  SLPC (Single Loop Power Controller) is a replacement for
>  some host-based power management features.  The SLPC
>  implemenation runs in firmware on GuC.
> 
>  This series is a first request for comments.  This series
>  is not expected to be merged.  After changes based on
>  comments, a later patch series will be sent for merging.
>   
>  This series has been tested with SKL guc firmware
>  versions 4.3 and 4.7.  The graphics power management
>  features in SLPC in those versions are DFPS (Dynamic FPS),
>  Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
>  requested graphics frequency to maintain target framerate.
>  Turbo adjusts requested graphics frequency to maintain
>  target GT busyness.  DCC adjusts requested graphics
>  frequency and stalls guc-scheduler to maintain actual
>  graphics frequency in efficient range.
> >>>
> >>> Either it's been forever long ago or I missed that meeting, so I'll drop
> >>> my big arch concerns here. We probably need to discuss this internally, at
> >>> least the benchmark data. Two big items:
> >>>
> >>> - How does GuC measure fps rendered to the screen? More specifically, how
> >>>   does it figure out that we missed a frame and kick the throttle up?
> >>
> >> Yeah, this has been covered before, both in the design review and with
> >> the GuC team; I don't think the DFPS feature is ready for Linux usage
> >> yet, or at least not generally, since afaik it doesn't have a way to
> >> monitor offscreen rendering at all, so may end up keeping the GPU freq
> >> lower than it needs to be when several clients are rendering to
> >> offscreen buffers and passing them to the compositor (depending on the
> >> compositor behavior at least).
> > 
> > There's also all kinds of issues with the current design, like:
> > - kernel knows when exactly we missed the vblank to display the next
> >   frame, guc can only control for average fps.
> > 
> > - all the fun you mention about multiple clients.
> > 
> > - what if we have more than 1 display running at different fps?
> 
> Yep; I think a userspace solution with a kernel interface would do a
> better job here (I outlined one a few years ago, but the lazyweb hasn't
> implemented it for me yet).

[TOR:] Patch 14/22 in this series sends display configuration info to SLPC.
If there are multiple displays, SLPC can take that into consideration.
> 
> > I'd say we need to keep the boost-deboost stuff alive, e.g. by manually
> > telling guc that the we want different limits, then resetting those limits
> > again after the boost is done. Same for fast idling - kernel simply has a
> > better idea if anyone is about to submit more work (we have execbuf hints
> > for specific workloads like libva).

[TOR:] Could those hints be sent to GuC as well?
> > 
> > Of course this assumes that guc slpc actually obeys our new limit requests
> > fast enough, so we'd still need to benchmark to make sure it's not slower
> > than what we have.
> 
> I definitely want to see benchmarking here too.  Maybe the GuC does
> boosting differently, but it may be just as good as what we have for all
> practical purposes.
> 
> Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for Add support for GuC-based SLPC

[O'Rourke, Tom]

On Thu, Jan 21, 2016 at 01:50:34PM +, Patchwork wrote:
> == Summary ==
> 
> Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 
> 2016y-01m-21d-11h-02m-42s UTC integration manifest
> 
> Test drv_getparams_basic:
> Subgroup basic-eu-total:
> pass   -> DMESG-FAIL (skl-i5k-2)
> Subgroup basic-subslice-total:
> pass   -> DMESG-FAIL (skl-i5k-2)
[TOR:] ...snip many failures on skl-i5k-2...

The results show an error in dmesg:
"[drm:intel_lr_context_deferred_alloc [i915]] *ERROR* ring create req: -5"

This error happens when attempting to use guc submission without guc
firmware available on system.  Skylake guc firmware is available for download
at https://01.org/linuxgraphics/downloads/sklgucver43 and is also at
git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git

How can we get CI test machines updated to use latest published firmware?

Thanks,
Tom

> 
> bdw-nuci7total:140  pass:131  dwarn:0   dfail:0   fail:0   skip:9  
> bdw-ultratotal:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> bsw-nuc-2total:143  pass:119  dwarn:0   dfail:0   fail:0   skip:24 
> byt-nuc  total:143  pass:128  dwarn:0   dfail:0   fail:0   skip:15 
> hsw-brixbox  total:143  pass:136  dwarn:0   dfail:0   fail:0   skip:7  
> hsw-gt2  total:143  pass:139  dwarn:0   dfail:0   fail:0   skip:4  
> ilk-hp8440p  total:143  pass:103  dwarn:2   dfail:0   fail:0   skip:38 
> ivb-t430stotal:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> skl-i5k-2total:143  pass:2dwarn:0   dfail:140 fail:0   skip:1  
> snb-dellxps  total:143  pass:129  dwarn:0   dfail:0   fail:0   skip:14 
> snb-x220ttotal:143  pass:129  dwarn:0   dfail:0   fail:1   skip:13 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1236/
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Update Promotion timer for RC6 TO Mode

[O'Rourke, Tom]

On Thu, Oct 01, 2015 at 07:59:27AM -0700, Kamble, Sagar A wrote:
> When using RC6 timeout mode, the timeout value
> should be written to GEN6_RC6_THRESHOLD.
> 
> v2: Updated commit message. (Tom)
> 
> v3: Rebase over whitespace differences. (Daniel)
> 
> Cc: Tom O'Rourke 
> Signed-off-by: Sagar Arun Kamble 
[TOR:] Still looks good.

Reviewed-by: Tom O'Rourke 

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c98eee6..c16f496 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4791,7 +4791,6 @@ static void gen9_enable_rc6(struct drm_device *dev)
>   I915_WRITE(GUC_MAX_IDLE_COUNT, 0xA);
>  
>   I915_WRITE(GEN6_RC_SLEEP, 0);
> - I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
>  
>   /* 2c: Program Coarse Power Gating Policies. */
>   I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 25);
> @@ -4802,16 +4801,19 @@ static void gen9_enable_rc6(struct drm_device *dev)
>   rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
>   DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ?
>   "on" : "off");
> -
> + /* WaRsUseTimeoutMode */
>   if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_D0) ||
> - (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_A0))
> + (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_A0)) {
> + I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us */
>   I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>  GEN7_RC_CTL_TO_MODE |
>  rc6_mask);
> - else
> + } else {
> + I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
>   I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>  GEN6_RC_CTL_EI_MODE(1) |
>  rc6_mask);
> + }
>  
>   /*
>* 3b: Enable Coarse Power Gating only when RC6 is enabled.
> -- 
> 1.9.1
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/guc: Don't forward flip interrupts to GuC

[O'Rourke, Tom]

On Wed, Sep 30, 2015 at 09:57:37AM -0700, yu@intel.com wrote:
> From: Sagar Arun Kamble 
> 
> Due to flip interrupts GuC stays awake always and GT does not enter
> RC6. Do not route those interrupts to GuC for now. Driver won't touch
> DE_GUCRMR register and leave it as what default value.
> 
> Signed-off-by: Sagar Arun Kamble 
> Signed-off-by: Alex Dai 
[TOR:] This patch was previously sent.  Still looks good to me.

Reviewed-by: Tom O'Rourke 

> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 10 --
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index ae85366..934b003 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -90,9 +90,6 @@ static void direct_interrupts_to_host(struct 
> drm_i915_private *dev_priv)
>   for_each_ring(ring, dev_priv, i)
>   I915_WRITE(RING_MODE_GEN7(ring), irqs);
>  
> - /* tell DE to send nothing to GuC */
> - I915_WRITE(DE_GUCRMR, ~0);
> -
>   /* route all GT interrupts to the host */
>   I915_WRITE(GUC_BCS_RCS_IER, 0);
>   I915_WRITE(GUC_VCS2_VCS1_IER, 0);
> @@ -110,13 +107,6 @@ static void direct_interrupts_to_guc(struct 
> drm_i915_private *dev_priv)
>   for_each_ring(ring, dev_priv, i)
>   I915_WRITE(RING_MODE_GEN7(ring), irqs);
>  
> - /* tell DE to send (all) flip_done to GuC */
> - irqs = DERRMR_PIPEA_PRI_FLIP_DONE | DERRMR_PIPEA_SPR_FLIP_DONE |
> -DERRMR_PIPEB_PRI_FLIP_DONE | DERRMR_PIPEB_SPR_FLIP_DONE |
> -DERRMR_PIPEC_PRI_FLIP_DONE | DERRMR_PIPEC_SPR_FLIP_DONE;
> - /* Unmasked bits will cause GuC response message to be sent */
> - I915_WRITE(DE_GUCRMR, ~irqs);
> -
>   /* route USER_INTERRUPT to Host, all others are sent to GuC. */
>   irqs = GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
>  GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/guc: Add host2guc notification for suspend and resume

[O'Rourke, Tom]

On Wed, Sep 30, 2015 at 08:59:21AM -0700, Yu Dai wrote:
> 
> 
> On 09/30/2015 03:46 AM, Kamble, Sagar A wrote:
> >Thanks for the updated patch. Minor comment below.
> >
> >Thanks
> >Sagar
> >
> >On 9/26/2015 12:16 AM, yu@intel.com wrote:
> >> From: Alex Dai 
> >>
> >> Add host2guc interfaces to nofigy GuC power state changes when
[TOR:] Is "nofigy" a typo?

Thanks,
Tom

> >> enter or resume from power saving state.
> >>
> >> v2: Add GuC suspend/resume to runtime suspend/resume too
> >>
> >> v1: Change to a more flexible way when fill host to GuC scratch
> >> data in order to remove hard coding.
> >>
> >> Signed-off-by: Alex Dai 
> >> ---
> >>   drivers/gpu/drm/i915/i915_drv.c|  5 +++
> >>   drivers/gpu/drm/i915/i915_gem.c|  2 ++
> >>   drivers/gpu/drm/i915/i915_guc_submission.c | 50 
> >> ++
> >>   drivers/gpu/drm/i915/intel_guc.h   |  2 ++
> >>   drivers/gpu/drm/i915/intel_guc_fwif.h  |  8 +
> >>   drivers/gpu/drm/i915/intel_guc_loader.c|  4 ++-
> >>   6 files changed, 70 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> >> b/drivers/gpu/drm/i915/i915_drv.c
> >> index e6d7a69..842eb13 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -737,6 +737,8 @@ static int i915_drm_resume(struct drm_device *dev)
> >>}
> >>mutex_unlock(>struct_mutex);
> >>
> >> +  intel_guc_resume(dev);
> >> +
> >>intel_modeset_init_hw(dev);
> >>
> >>spin_lock_irq(_priv->irq_lock);
> >> @@ -1476,6 +1478,7 @@ static int intel_runtime_suspend(struct device 
> >> *device)
> >>i915_gem_release_all_mmaps(dev_priv);
> >>mutex_unlock(>struct_mutex);
> >>
> >> +  intel_guc_suspend(dev);
> >>intel_suspend_gt_powersave(dev);
> >>intel_runtime_pm_disable_interrupts(dev_priv);
> >>
> >> @@ -1535,6 +1538,8 @@ static int intel_runtime_resume(struct device 
> >> *device)
> >>intel_opregion_notify_adapter(dev, PCI_D0);
> >>dev_priv->pm.suspended = false;
> >>
> >> +  intel_guc_resume(dev);
> >> +
> >>if (IS_GEN6(dev_priv))
> >>intel_init_pch_refclk(dev);
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> >> b/drivers/gpu/drm/i915/i915_gem.c
> >> index bf5ef7a..679ed55 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> @@ -4460,6 +4460,8 @@ i915_gem_suspend(struct drm_device *dev)
> >>i915_gem_stop_ringbuffers(dev);
> >>mutex_unlock(>struct_mutex);
> >>
> >> +  intel_guc_suspend(dev);
> >> +
> >Should this be called as part of i915_drm_suspend for consistency
> >instead of i915_gem_suspend?
> >
> 
> Yes, I will change it accordingly. i915_gem_suspend is called during
> unload, where we won't need to suspend guc - it will be unloaded
> anyway.
> 
> Thanks for the review,
> Alex
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Update Promotion timer for RC6 TO Mode

[O'Rourke, Tom]

On Wed, Sep 30, 2015 at 04:13:43PM +0530, Sagar Arun Kamble wrote:
> When using RC6 timeout mode, the timeout value
> should be written to GEN6_RC6_THRESHOLD.
> 
> v2: Updated commit message. (Tom)
> 
> Signed-off-by: Sagar Arun Kamble 

Reviewed-by: Tom O'Rourke 

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a878147..ebde43d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4842,7 +4842,6 @@ static void gen9_enable_rc6(struct drm_device *dev)
>   for_each_ring(ring, dev_priv, unused)
>   I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
>   I915_WRITE(GEN6_RC_SLEEP, 0);
> - I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
>  
>   /* 2c: Program Coarse Power Gating Policies. */
>   I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 25);
> @@ -4854,15 +4853,19 @@ static void gen9_enable_rc6(struct drm_device *dev)
>   DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ?
>   "on" : "off");
>  
> + /* WaRsUseTimeoutMode */
>   if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_D0) ||
> - (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_A0))
> + (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_A0)) {
> + I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us */
>  I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>  GEN7_RC_CTL_TO_MODE |
>  rc6_mask);
> -else
> + } else {
> + I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
>  I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>  GEN6_RC_CTL_EI_MODE(1) |
>  rc6_mask);
> + }
>  
>   /*
>* 3b: Enable Coarse Power Gating only when RC6 is enabled.
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/guc: Media domain bit needed when notify GuC rc6 state

[O'Rourke, Tom]

On Fri, Sep 25, 2015 at 11:46:56AM -0700, yu@intel.com wrote:
> From: Alex Dai 
> 
> GuC expects two bits for Render and Media domain separately when
> driver sends data via host2guc SAMPLE_FORCEWAKE. Bit 0 is for
> Render and bit 1 is for Media domain.
> 
> v2: Keep sync with code for WaRsDoubleRc6WrlWithCoarsePowerGating
> 
> v1: Add parameters definition to avoid magic value
> 
> Signed-off-by: Alex Dai 

Reviewed-by: Tom O'Rourke 

> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c | 13 +++--
>  drivers/gpu/drm/i915/intel_guc_fwif.h  |  3 +++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 38b6ef4..036b42b 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -155,12 +155,21 @@ static int host2guc_sample_forcewake(struct intel_guc 
> *guc,
>struct i915_guc_client *client)
>  {
>   struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + struct drm_device *dev = dev_priv->dev;
>   u32 data[2];
>  
>   data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE;
> - data[1] = (intel_enable_rc6(dev_priv->dev)) ? 1 : 0;
> + /* WaRsDisableCoarsePowerGating:skl,bxt */
> + if (!intel_enable_rc6(dev_priv->dev) ||
> + (IS_BROXTON(dev) && (INTEL_REVID(dev) < BXT_REVID_B0)) ||
> + (IS_SKL_GT3(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0)) ||
> + (IS_SKL_GT4(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0)))
> + data[1] = 0;
> + else
> + /* bit 0 and 1 are for Render and Media domain separately */
> + data[1] = GUC_FORCEWAKE_RENDER | GUC_FORCEWAKE_MEDIA;
>  
> - return host2guc_action(guc, data, 2);
> + return host2guc_action(guc, data, ARRAY_SIZE(data));
>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 4029478..04ca777 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -296,6 +296,9 @@ struct guc_context_desc {
>  #define GUC_POWER_D2 3
>  #define GUC_POWER_D3 4
>  
> +#define GUC_FORCEWAKE_RENDER (1 << 0)
> +#define GUC_FORCEWAKE_MEDIA  (1 << 1)
> +
>  /* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */
>  enum host2guc_action {
>   HOST2GUC_ACTION_DEFAULT = 0x0,
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/1] drm/i915: Update Promotion timer for RC6 TO Mode

[O'Rourke, Tom]

This change looks good and is necessary, but the
commit message should have more detail.

I would add:
"When using RC6 timeout mode, the timeout value
should be written to GEN6_RC6_THRESHOLD."

With that,
Reviewed-by: Tom O'Rourke 

On Wed, Sep 23, 2015 at 03:06:42PM +0530, Sagar Arun Kamble wrote:
> Signed-off-by: Sagar Arun Kamble 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a878147..ebde43d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4842,7 +4842,6 @@ static void gen9_enable_rc6(struct drm_device *dev)
>   for_each_ring(ring, dev_priv, unused)
>   I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
>   I915_WRITE(GEN6_RC_SLEEP, 0);
> - I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
>  
>   /* 2c: Program Coarse Power Gating Policies. */
>   I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 25);
> @@ -4854,15 +4853,19 @@ static void gen9_enable_rc6(struct drm_device *dev)
>   DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ?
>   "on" : "off");
>  
> + /* WaRsUseTimeoutMode */
>   if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_D0) ||
> - (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_A0))
> + (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_A0)) {
> + I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us */
>  I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>  GEN7_RC_CTL_TO_MODE |
>  rc6_mask);
> -else
> + } else {
> + I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
>  I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>  GEN6_RC_CTL_EI_MODE(1) |
>  rc6_mask);
> + }
>  
>   /*
>* 3b: Enable Coarse Power Gating only when RC6 is enabled.
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 5/6] drm/i915/guc: Media domain bit needed when notify GuC rc6 state

[O'Rourke, Tom]

On Tue, Sep 22, 2015 at 01:48:44PM -0700, yu@intel.com wrote:
> From: Alex Dai 
> 
> GuC expects two bits for Render and Media domain separately when
> driver sends data via host2guc SAMPLE_FORCEWAKE. Bit 0 is for
> Render and bit 1 is for Media domain.
> 
> v1: Add parameters definition to avoid magic value
> 
> Signed-off-by: Alex Dai 

Reviewed-by: Tom O'Rourke 

> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c | 6 --
>  drivers/gpu/drm/i915/intel_guc_fwif.h  | 3 +++
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 38b6ef4..7dbc108 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -158,9 +158,11 @@ static int host2guc_sample_forcewake(struct intel_guc 
> *guc,
>   u32 data[2];
>  
>   data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE;
> - data[1] = (intel_enable_rc6(dev_priv->dev)) ? 1 : 0;
> + /* bit 0 and 1 are for Render and Media domain separately */
> + data[1] = intel_enable_rc6(dev_priv->dev) ?
> + GUC_FORCEWAKE_RENDER | GUC_FORCEWAKE_MEDIA : 0;
>  
> - return host2guc_action(guc, data, 2);
> + return host2guc_action(guc, data, ARRAY_SIZE(data));
>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index f6d0aa4..ecea053 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -260,6 +260,9 @@ struct guc_context_desc {
>  #define GUC_POWER_D2 3
>  #define GUC_POWER_D3 4
>  
> +#define GUC_FORCEWAKE_RENDER (1 << 0)
> +#define GUC_FORCEWAKE_MEDIA  (1 << 1)
> +
>  /* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */
>  enum host2guc_action {
>   HOST2GUC_ACTION_DEFAULT = 0x0,
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/guc: Don't send flips to GuC

[O'Rourke, Tom]

On Tue, Sep 22, 2015 at 04:11:48PM -0700, yu@intel.com wrote:
> From: Sagar Arun Kamble 
> 
> Due to flip interrupts GuC stays awake always and GT does not enter
> RC6. Do not route those interrupts to GuC for now. Driver won't touch
> DE_GUCRMR register and leave it as what default value.
> 
> Signed-off-by: Sagar Arun Kamble 
> Signed-off-by: Alex Dai 

Looks good to me.

Reviewed-by: Tom O'Rourke 

> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 10 --
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 740bfb3..d513673 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -90,9 +90,6 @@ static void direct_interrupts_to_host(struct 
> drm_i915_private *dev_priv)
>   for_each_ring(ring, dev_priv, i)
>   I915_WRITE(RING_MODE_GEN7(ring), irqs);
>  
> - /* tell DE to send nothing to GuC */
> - I915_WRITE(DE_GUCRMR, ~0);
> -
>   /* route all GT interrupts to the host */
>   I915_WRITE(GUC_BCS_RCS_IER, 0);
>   I915_WRITE(GUC_VCS2_VCS1_IER, 0);
> @@ -110,13 +107,6 @@ static void direct_interrupts_to_guc(struct 
> drm_i915_private *dev_priv)
>   for_each_ring(ring, dev_priv, i)
>   I915_WRITE(RING_MODE_GEN7(ring), irqs);
>  
> - /* tell DE to send (all) flip_done to GuC */
> - irqs = DERRMR_PIPEA_PRI_FLIP_DONE | DERRMR_PIPEA_SPR_FLIP_DONE |
> -DERRMR_PIPEB_PRI_FLIP_DONE | DERRMR_PIPEB_SPR_FLIP_DONE |
> -DERRMR_PIPEC_PRI_FLIP_DONE | DERRMR_PIPEC_SPR_FLIP_DONE;
> - /* Unmasked bits will cause GuC response message to be sent */
> - I915_WRITE(DE_GUCRMR, ~irqs);
> -
>   /* route USER_INTERRUPT to Host, all others are sent to GuC. */
>   irqs = GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
>  GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Direct all DE interrupts to host.

[O'Rourke, Tom]

On Fri, Sep 11, 2015 at 05:44:32AM -0700, Kamble, Sagar A wrote:
> Due to flip interrupts routed to GuC, GuC stays awake always and GT does not 
> enter RC6.
> GuC firmware should re-direct to GuC those interrupts that it can handle.
> 
> v2: Commit message change and routing all interrupts to host. (Tom)
> 
> Cc: Alex Dai 
> Cc: Tom O'Rourke 
> Cc: Akash Goel 
> Signed-off-by: Sagar Arun Kamble 
> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 5eafd31..0b047c4 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -110,12 +110,8 @@ static void direct_interrupts_to_guc(struct 
> drm_i915_private *dev_priv)
>   for_each_ring(ring, dev_priv, i)
>   I915_WRITE(RING_MODE_GEN7(ring), irqs);
>  
> - /* tell DE to send (all) flip_done to GuC */
> - irqs = DERRMR_PIPEA_PRI_FLIP_DONE | DERRMR_PIPEA_SPR_FLIP_DONE |
> -DERRMR_PIPEB_PRI_FLIP_DONE | DERRMR_PIPEB_SPR_FLIP_DONE |
> -DERRMR_PIPEC_PRI_FLIP_DONE | DERRMR_PIPEC_SPR_FLIP_DONE;
> - /* Unmasked bits will cause GuC response message to be sent */
> - I915_WRITE(DE_GUCRMR, ~irqs);
> + /* tell DE to send nothing to GuC */
> + I915_WRITE(DE_GUCRMR, ~0);

[TOR:] Should the host driver be writing these bits in
DE_GUCRMR at all?  An alternative approach would let GuC
set/clear these bits based on whether or not GuC wants to
handle the resulting interrupts; the host driver should
not touch these bits since the host driver does not know
what GuC wants to do (or may have already done with
DE_GUCRMR register).

Thanks,
Tom


>  
>   /* route USER_INTERRUPT to Host, all others are sent to GuC. */
>   irqs = GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
> -- 
> 1.9.1
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 3/7] drm/i915: WaRsUseTimeoutMode

[O'Rourke, Tom]

Hello,
This change looks good but incomplete.

When changing RC6 from EI mode to TO mode,
should the time value in GEN6_RC6_THRESHOLD
be changed to hold the timeout value instead
of the evaluation interval period?

Should the workaround name be included in a comment?
While this workaround is unnamed for Broadwell, it is
called WaRsUseTimeoutMode for Skylake.

If possible, I would like to see those changes
squashed into this patch.  If not, then putting
those changes in a followup patch would be OK.

Thanks,
Tom

On Mon, Sep 21, 2015 at 11:49:58AM -0700, Yu Dai wrote:
> Looks good to me.
> Reviewed-by: Alex Dai 
> 
> On 09/11/2015 09:47 PM, Sagar Arun Kamble wrote:
> >Enable TO mode for RC6 for SKL till D0 and BXT till A0.
> >
> >Cc: Tom O'Rourke 
> >Cc: Akash Goel 
> >Signed-off-by: Sagar Arun Kamble 
> >---
> >  drivers/gpu/drm/i915/intel_pm.c | 13 ++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> >b/drivers/gpu/drm/i915/intel_pm.c
> >index c93d3a7..6e4818d 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -4847,9 +4847,16 @@ static void gen9_enable_rc6(struct drm_device *dev)
> > rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
> > DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ?
> > "on" : "off");
> >-I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> >-   GEN6_RC_CTL_EI_MODE(1) |
> >-   rc6_mask);
> >+
> >+if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_D0) ||
> >+(IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_A0))
> >+I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> >+GEN7_RC_CTL_TO_MODE |
> >+rc6_mask);
> >+else
> >+I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> >+GEN6_RC_CTL_EI_MODE(1) |
> >+rc6_mask);
> > /*
> >  * 3b: Enable Coarse Power Gating only when RC6 is enabled.
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/guc: Support GuC version 4.3

[O'Rourke, Tom]

On Tue, Aug 18, 2015 at 02:32:35PM -0700, yu@intel.com wrote:
 From: Alex Dai yu@intel.com
 
 The firmware layout changes that now it only has css header +
 uCode + RSA signature. Plus, other trivial changes to support
 GuC V4.3.
 
 Signed-off-by: Alex Dai yu@intel.com

Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com

This patch has the changes required to enable use
of the guc firmware posted on 01.org.  

 ---
  drivers/gpu/drm/i915/intel_guc_fwif.h   | 11 ---
  drivers/gpu/drm/i915/intel_guc_loader.c | 17 ++---
  2 files changed, 14 insertions(+), 14 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
 b/drivers/gpu/drm/i915/intel_guc_fwif.h
 index 950c7e7..e1f47ba 100644
 --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
 +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
 @@ -41,7 +41,7 @@
  #define GUC_CTX_PRIORITY_NORMAL  3
  
  #define GUC_MAX_GPU_CONTEXTS 1024
 -#define  GUC_INVALID_CTX_ID  (GUC_MAX_GPU_CONTEXTS + 1)
 +#define  GUC_INVALID_CTX_ID  GUC_MAX_GPU_CONTEXTS
  
  /* Work queue item header definitions */
  #define WQ_STATUS_ACTIVE 1
 @@ -75,6 +75,7 @@
  #define GUC_CTX_DESC_ATTR_RESET  (1  4)
  #define GUC_CTX_DESC_ATTR_WQLOCKED   (1  5)
  #define GUC_CTX_DESC_ATTR_PCH(1  6)
 +#define GUC_CTX_DESC_ATTR_TERMINATED (1  7)
  
  /* The guc control data is 10 DWORDs */
  #define GUC_CTL_CTXINFO  0
 @@ -107,6 +108,7 @@
  #define   GUC_CTL_DISABLE_SCHEDULER  (1  4)
  #define   GUC_CTL_PREEMPTION_LOG (1  5)
  #define   GUC_CTL_ENABLE_SLPC(1  7)
 +#define   GUC_CTL_RESET_ON_PREMPT_FAILURE(1  8)
  #define GUC_CTL_DEBUG8
  #define   GUC_LOG_VERBOSITY_SHIFT0
  #define   GUC_LOG_VERBOSITY_LOW  (0  GUC_LOG_VERBOSITY_SHIFT)
 @@ -116,8 +118,9 @@
  /* Verbosity range-check limits, without the shift */
  #defineGUC_LOG_VERBOSITY_MIN 0
  #defineGUC_LOG_VERBOSITY_MAX 3
 +#define GUC_CTL_RSRVD9
  
 -#define GUC_CTL_MAX_DWORDS   (GUC_CTL_DEBUG + 1)
 +#define GUC_CTL_MAX_DWORDS   (GUC_CTL_RSRVD + 1)

[TOR:] The change to this constant is needed but
this patch also adds some other definitions that
are not used yet.  I would prefer to leave those
new and unused constants to a later patch where
they are actually used.  This is a minor point
and should not delay merging this patch.

  
  struct guc_doorbell_info {
   u32 db_status;
 @@ -207,7 +210,9 @@ struct guc_context_desc {
  
   u32 engine_presence;
  
 - u32 reserved0[1];
 + u8 engine_suspended;
 +
 + u8 reserved0[3];
   u64 reserved1[1];
  
   u64 desc_private;
 diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
 b/drivers/gpu/drm/i915/intel_guc_loader.c
 index d46195c..7521eac 100644
 --- a/drivers/gpu/drm/i915/intel_guc_loader.c
 +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
 @@ -59,7 +59,7 @@
   *
   */
  
 -#define I915_SKL_GUC_UCODE i915/skl_guc_ver3.bin
 +#define I915_SKL_GUC_UCODE i915/skl_guc_ver4.bin
  MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
  
  /* User-friendly representation of an enum */
 @@ -226,10 +226,6 @@ static inline bool guc_ucode_response(struct 
 drm_i915_private *dev_priv,
   * +---+  
   * | RSA signature |  256B
   * +---+  
 - * | RSA public Key|  256B
 - * +---+  
 - * |   Public key modulus  |4B
 - * +---+  
   *
   * Architecturally, the DMA engine is bidirectional, and can potentially even
   * transfer between GTT locations. This functionality is left out of the API
 @@ -244,7 +240,6 @@ static inline bool guc_ucode_response(struct 
 drm_i915_private *dev_priv,
  #define UOS_VER_MAJOR_OFFSET 0x46
  #define UOS_CSS_HEADER_SIZE  0x80
  #define UOS_RSA_SIG_SIZE 0x100
 -#define UOS_CSS_SIGNING_SIZE 0x204
  
  static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
  {
 @@ -256,7 +251,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private 
 *dev_priv)
   int i, ret = 0;
  
   /* uCode size, also is where RSA signature starts */
 - offset = ucode_size = guc_fw-guc_fw_size - UOS_CSS_SIGNING_SIZE;
 + offset = ucode_size = guc_fw-guc_fw_size - UOS_RSA_SIG_SIZE;
   I915_WRITE(DMA_COPY_SIZE, ucode_size);
  
   /* Copy RSA signature from the fw image to HW for verification */
 @@ -471,8 +466,8 @@ static void guc_fw_fetch(struct drm_device *dev, struct 
 intel_guc_fw *guc_fw)
   struct drm_i915_gem_object *obj;
   const struct firmware *fw;
   const u8 *css_header;
 - const size_t minsize = UOS_CSS_HEADER_SIZE + UOS_CSS_SIGNING_SIZE;
 - const size_t maxsize = GUC_WOPCM_SIZE_VALUE + UOS_CSS_SIGNING_SIZE
 + const size_t minsize = UOS_CSS_HEADER_SIZE + UOS_RSA_SIG_SIZE;
 +  

Re: [Intel-gfx] [PATCH] drm/i915: Notify GuC rc6 state

[O'Rourke, Tom]

On Tue, Aug 18, 2015 at 02:34:47PM -0700, yu@intel.com wrote:
 From: Alex Dai yu@intel.com
 
 If rc6 is enabled, notify GuC so it can do proper forcewake before
 command submission.
 
 Signed-off-by: Alex Dai yu@intel.com

Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com

 ---
  drivers/gpu/drm/i915/i915_guc_submission.c | 15 +++
  1 file changed, 15 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
 b/drivers/gpu/drm/i915/i915_guc_submission.c
 index ec70393..792d0b9 100644
 --- a/drivers/gpu/drm/i915/i915_guc_submission.c
 +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
 @@ -151,6 +151,18 @@ static int host2guc_release_doorbell(struct intel_guc 
 *guc,
   return host2guc_action(guc, data, 2);
  }
  
 +static int host2guc_sample_forcewake(struct intel_guc *guc,
 +  struct i915_guc_client *client)
 +{
 + struct drm_i915_private *dev_priv = guc_to_i915(guc);
 + u32 data[2];
 +
 + data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE;
 + data[1] = (intel_enable_rc6(dev_priv-dev)) ? 1 : 0;
 +
 + return host2guc_action(guc, data, 2);
 +}
 +
  /*
   * Initialise, update, or clear doorbell data shared with the GuC
   *
 @@ -874,6 +886,9 @@ int i915_guc_submission_enable(struct drm_device *dev)
   }
  
   guc-execbuf_client = client;
 +
 + host2guc_sample_forcewake(guc, client);
 +
   return 0;
  }
  
 -- 
 1.9.1
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915: Notify Coarse Power Gating changes to GuC

[O'Rourke, Tom]

On Sun, Aug 23, 2015 at 05:52:51PM +0530, Sagar Arun Kamble wrote:
 From: Alex Dai yu@intel.com

[TOR:] This commit message is inadequate.  The 
needed information is in the cover letter but 
is lacking here.  Please rebase with Alex's 
previous patch drm/i915: Notify GuC rc6 state
 
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Sagar Arun Kamble sagar.a.kam...@intel.com
 ---
  drivers/gpu/drm/i915/i915_guc_submission.c | 18 ++
  drivers/gpu/drm/i915/intel_guc.h   |  1 +
  drivers/gpu/drm/i915/intel_pm.c| 16 
  3 files changed, 31 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
 b/drivers/gpu/drm/i915/i915_guc_submission.c
 index ec70393..462c679 100644
 --- a/drivers/gpu/drm/i915/i915_guc_submission.c
 +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
 @@ -877,6 +877,24 @@ int i915_guc_submission_enable(struct drm_device *dev)
   return 0;
  }
  
 +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data)
 +{
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + struct intel_guc *guc = dev_priv-guc;
 + struct intel_guc_fw *guc_fw = dev_priv-guc.guc_fw;
 +
 + /* Notify GuC about CPG changes. */
 + if (guc_fw-guc_fw_fetch_status == GUC_FIRMWARE_SUCCESS) {
 + u32 data[2];
 +
 + data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE;
 + data[1] = fw_data;
 +
 + if (host2guc_action(guc, data, 2))
 + DRM_ERROR(Unable to notify GuC of CPG change\n);
 + }
 +}
 +
  void i915_guc_submission_disable(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 diff --git a/drivers/gpu/drm/i915/intel_guc.h 
 b/drivers/gpu/drm/i915/intel_guc.h
 index 4ec2d27..691574d 100644
 --- a/drivers/gpu/drm/i915/intel_guc.h
 +++ b/drivers/gpu/drm/i915/intel_guc.h
 @@ -118,5 +118,6 @@ int i915_guc_submit(struct i915_guc_client *client,
   struct drm_i915_gem_request *rq);
  void i915_guc_submission_disable(struct drm_device *dev);
  void i915_guc_submission_fini(struct drm_device *dev);
 +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data);
  
  #endif
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index c0345d2..4a0483c 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -4616,6 +4616,9 @@ static void gen9_disable_rps(struct drm_device *dev)
   struct drm_i915_private *dev_priv = dev-dev_private;
  
   I915_WRITE(GEN6_RC_CONTROL, 0);
 +
 + i915_guc_sample_forcewake(dev, 0);
 +
   I915_WRITE(GEN9_PG_ENABLE, 0);
  }
  
 @@ -4804,6 +4807,7 @@ static void gen9_enable_rc6(struct drm_device *dev)
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_engine_cs *ring;
   uint32_t rc6_mask = 0;
 + uint32_t cpg_data = 0;
   int unused;
  
   /* 1a: Software RC state - RC0 */
 @@ -4843,11 +4847,15 @@ static void gen9_enable_rc6(struct drm_device *dev)
* WaRsDisableCoarsePowerGating:skl,bxt - Render/Media PG need to be 
 disabled with RC6.
*/
   if ((IS_BROXTON(dev)  (INTEL_REVID(dev)  BXT_REVID_B0)) ||
 - (IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_E0)))
 + (IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_E0))) {
 + i915_guc_sample_forcewake(dev, 0);
   I915_WRITE(GEN9_PG_ENABLE, 0);
 - else
 - I915_WRITE(GEN9_PG_ENABLE, (rc6_mask  GEN6_RC_CTL_RC6_ENABLE) ?
 - (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE) 
 : 0);
 + } else {
 + cpg_data = (rc6_mask  GEN6_RC_CTL_RC6_ENABLE)?
 + (GEN9_RENDER_PG_ENABLE | 
 GEN9_MEDIA_PG_ENABLE):0;
 + i915_guc_sample_forcewake(dev, cpg_data);
 + I915_WRITE(GEN9_PG_ENABLE, cpg_data);
 + }
  
   intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
  
 -- 
 1.9.1
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/9 v6] Batch submission via GuC

[O'Rourke, Tom]

On Wed, Aug 12, 2015 at 07:57:37AM -0700, Gordon, David S wrote:
 On 12/08/15 15:43, Dave Gordon wrote:
  This patch series enables command submission via the GuC. In this mode,
  instead of the host CPU driving the execlist port directly, it hands
  over work items to the GuC, using a doorbell mechanism to tell the GuC
  that new items have been added to its work queue. The GuC then dispatches
  contexts to the various GPU engines, and manages the resulting context-
  switch interrupts. Completion of a batch is however still signalled to
  the CPU; the GuC is not involved in handling user interrupts.
 
  There are two subsequences within the patch series:
 
 drm/i915: GuC-specific firmware loader
 drm/i915: Debugfs interface to read GuC load status
 
  These two patches provide the GuC loader and a debugfs interface to
  verify the resulting state.  At this point in the sequence we can load
  and activate the GuC firmware, but not submit any batches through it.
  (This is nonetheless a potentially useful state, as the GuC could do
  other useful work even when not handling batch submissions).
 
 drm/i915: Expose one LRC function for GuC submission mode
 drm/i915: Prepare for GuC-based command submission
 drm/i915: Enable GuC firmware log
 drm/i915: Implementation of GuC submission client
 drm/i915: Interrupt routing for GuC submission
 drm/i915: Integrate GuC-based command submission
 drm/i915: Debugfs interface for GuC submission statistics
 
  In this second section, we implement the GuC submission mechanism, and
  link it into the (execlist-based) submission path. At this stage, it is
  not yet enabled by default; it is activated only if the kernel command
  line explicitly switches it on.
 
  The GuC firmware itself is not included in this patchset; it is or will
  be available for download from https://01.org/linuxgraphics/downloads/
  This driver works with and requires GuC firmware revision 3.x. It will
  not work with any firmware version 1.x, as the GuC protocol in those
  revisions was incompatible and is no longer supported.
 
  On platforms where there is no GuC, or where GuC submission is not
  specifically enabled, batch submission will continue to use the execlist
  (or ringbuffer) mechanisms.  On the other hand, if GuC submission is
  requested but the GuC firmware cannot be found or is invalid, the GPU
  will be unusable.
 
  It is expected that a subsequent patch will enable GuC submission by
  default once the signed firmware is published on 01.org.
 
  Ben Widawsky (0):
  Vinit Azad (0):
  Michael H. Nguyen (0):
 created the original versions on which some of these patches are based.
 
  Alex Dai (5):
 drm/i915: GuC-specific firmware loader
 drm/i915: Debugfs interface to read GuC load status
 drm/i915: Prepare for GuC-based command submission
 drm/i915: Enable GuC firmware log
 drm/i915: Integrate GuC-based command submission
 
  Dave Gordon (4):
 drm/i915: Expose one LRC function for GuC submission mode
 drm/i915: Implementation of GuC submission client
 drm/i915: Interrupt routing for GuC submission
 drm/i915: Debugfs interface for GuC submission statistics
 
Documentation/DocBook/drm.tmpl |  14 +
drivers/gpu/drm/i915/Makefile  |   4 +
drivers/gpu/drm/i915/i915_debugfs.c| 146 -
drivers/gpu/drm/i915/i915_dma.c|   9 +
drivers/gpu/drm/i915/i915_drv.h|  11 +
drivers/gpu/drm/i915/i915_gem.c|  16 +
drivers/gpu/drm/i915/i915_gpu_error.c  |  14 +-
drivers/gpu/drm/i915/i915_guc_reg.h|  17 +-
drivers/gpu/drm/i915/i915_guc_submission.c | 901 
  +
drivers/gpu/drm/i915/i915_reg.h|  15 +-
drivers/gpu/drm/i915/intel_guc.h   | 122 
drivers/gpu/drm/i915/intel_guc_fwif.h  |   9 +-
drivers/gpu/drm/i915/intel_guc_loader.c| 611 +++
drivers/gpu/drm/i915/intel_lrc.c   |  65 ++-
drivers/gpu/drm/i915/intel_lrc.h   |   8 +
15 files changed, 1918 insertions(+), 44 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.c
create mode 100644 drivers/gpu/drm/i915/intel_guc.h
create mode 100644 drivers/gpu/drm/i915/intel_guc_loader.c
 
 Tom O'Rourke has already R-B'ed the previous [v5] versions of these, and 
 there are no substantive changes to patches 2, 3, 4, 5, 7 and 8, so we 
 can carry that over; also, the change in patch 1 is just an update to a 
 comment noted in Tom's earlier reviews as being out-of-date.
 
 I haven't included patch 10 (enable-by-default) as we've decided to wait 
 until the signed firmware is publicly available on 01.org before
 making it the default.
 
 So, it's really just patches 6/9 and 9/9 that need a re-review from Tom.
 
 Thanks,
 .Dave.

[TOR:] These patches still look good.  Good catch in patch 6 with the offset.

For all 9 patches:

Re: [Intel-gfx] [PATCH 00/10 v5] Batch submission via GuC

[O'Rourke, Tom]

On Wed, Jul 29, 2015 at 06:48:28PM +0100, Dave Gordon wrote:
 This patch series enables command submission via the GuC. In this mode,
 instead of the host CPU driving the execlist port directly, it hands
 over work items to the GuC, using a doorbell mechanism to tell the GuC
 that new items have been added to its work queue. The GuC then dispatches
 contexts to the various GPU engines, and manages the resulting context-
 switch interrupts. Completion of a batch is however still signalled to
 the CPU; the GuC is not involved in handling user interrupts.
 
 There are two subsequences within the patch series:
 
   drm/i915: GuC-specific firmware loader
   drm/i915: Debugfs interface to read GuC load status
 
 These two patches provide the GuC loader and a debugfs interface to
 verify the resulting state.  At this point in the sequence we can load
 and activate the GuC firmware, but not submit any batches through it.
 (This is nonetheless a potentially useful state, as the GuC could do
 other useful work even when not handling batch submissions).
 
   drm/i915: Expose one LRC function for GuC submission mode
   drm/i915: Prepare for GuC-based command submission
   drm/i915: Enable GuC firmware log
   drm/i915: Implementation of GuC submission client
   drm/i915: Interrupt routing for GuC submission
   drm/i915: Integrate GuC-based command submission
   drm/i915: Debugfs interface for GuC submission statistics
   drm/i915: Enable GuC submission, where supported
 
 In this second section, we implement the GuC submission mechanism, link
 it into the (execlist-based) submission path, and finally enable it
 (on supported platforms).
 
 On platforms where there is no GuC, or if GuC submission is explicitly
 disabled, batch submission will revert to using the execlist mechanism
 directly.  On the other hand, if the GuC firmware cannot be found or is
 invalid, the GPU will be unusable.
 
 The GuC firmware itself is not included in this patchset; it is or will
 be available for download from https://01.org/linuxgraphics/downloads/
 This driver works with and requires GuC firmware revision 3.x. It will
 not work with any firmware version 1.x, as the GuC protocol in those
 revisions was incompatible and is no longer supported.
 
[TOR:] This patch set looks good.  There is one comment in
the first patch that should be updated.  Otherwise, I did
not see any changes required.

For the 10 patches:
Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com

However, the guc firmware version 3.x is still not available
on 01.org.  For that reason the 10th patch should not be
merged yet.  The first 9 patches can be merged now;
hopefully before there are difficult merge conflicts.

Also, the guc firmware going through the release process is
version 4.x.  Alex Dai is working on a few patches needed
for version 4.x.

Thanks,
Tom

 Ben Widawsky (0):
 Vinit Azad (0):
 Michael H. Nguyen (0):
   created the original versions on which some of these patches are based.
 
 Alex Dai (5):
   drm/i915: GuC-specific firmware loader
   drm/i915: Debugfs interface to read GuC load status
   drm/i915: Prepare for GuC-based command submission
   drm/i915: Enable GuC firmware log
   drm/i915: Integrate GuC-based command submission
 
 Dave Gordon (5):
   drm/i915: Expose one LRC function for GuC submission mode
   drm/i915: Implementation of GuC submission client
   drm/i915: Interrupt routing for GuC submission
   drm/i915: Debugfs interface for GuC submission statistics
   drm/i915: Enable GuC submission, where supported
 
  Documentation/DocBook/drm.tmpl |  14 +
  drivers/gpu/drm/i915/Makefile  |   4 +
  drivers/gpu/drm/i915/i915_debugfs.c| 122 +++-
  drivers/gpu/drm/i915/i915_dma.c|   9 +
  drivers/gpu/drm/i915/i915_drv.h|  11 +
  drivers/gpu/drm/i915/i915_gem.c|  16 +
  drivers/gpu/drm/i915/i915_gpu_error.c  |  13 +-
  drivers/gpu/drm/i915/i915_guc_reg.h|  17 +-
  drivers/gpu/drm/i915/i915_guc_submission.c | 887 
 +
  drivers/gpu/drm/i915/i915_params.c |   4 +-
  drivers/gpu/drm/i915/i915_reg.h|  15 +-
  drivers/gpu/drm/i915/intel_guc.h   | 122 
  drivers/gpu/drm/i915/intel_guc_fwif.h  |   3 +-
  drivers/gpu/drm/i915/intel_guc_loader.c| 613 
  drivers/gpu/drm/i915/intel_lrc.c   |  65 ++-
  drivers/gpu/drm/i915/intel_lrc.h   |   8 +
  16 files changed, 1881 insertions(+), 42 deletions(-)
  create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.c
  create mode 100644 drivers/gpu/drm/i915/intel_guc.h
  create mode 100644 drivers/gpu/drm/i915/intel_guc_loader.c
 
 -- 
 1.9.1
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/10 v5] drm/i915: GuC-specific firmware loader

[O'Rourke, Tom]

On Wed, Jul 29, 2015 at 06:48:29PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 This fetches the required firmware image from the filesystem,
 then loads it into the GuC's memory via a dedicated DMA engine.
 
 This patch is derived from GuC loading work originally done by
 Vinit Azad and Ben Widawsky.
 
 v2:
 Various improvements per review comments by Chris Wilson
 
 v3:
 Removed 'wait' parameter to intel_guc_ucode_load() as firmware
 prefetch is no longer supported in the common firmware loader,
   per Daniel Vetter's request.
 Firmware checker callback fn now returns errno rather than bool.
 
 v4:
 Squash uC-independent code into GuC-specifc loader [Daniel Vetter]
 Don't keep the driver working (by falling back to execlist mode)
 if GuC firmware loading fails [Daniel Vetter]
 
 v5:
 Clarify WOPCM-related #defines [Tom O'Rourke]
 Delete obsolete code no longer required with current h/w  f/w
 [Tom O'Rourke]
 Move the call to intel_guc_ucode_init() later, so that it can
 allocate GEM objects, and have it fetch the firmware; then
   intel_guc_ucode_load() doesn't need to fetch it later.
 [Daniel Vetter].
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
  drivers/gpu/drm/i915/Makefile   |   3 +
  drivers/gpu/drm/i915/i915_dma.c |   9 +
  drivers/gpu/drm/i915/i915_drv.h |  11 +
  drivers/gpu/drm/i915/i915_gem.c |  16 +
  drivers/gpu/drm/i915/i915_guc_reg.h |  17 +-
  drivers/gpu/drm/i915/i915_reg.h |   4 +-
  drivers/gpu/drm/i915/intel_guc.h|  67 
  drivers/gpu/drm/i915/intel_guc_fwif.h   |   3 +-
  drivers/gpu/drm/i915/intel_guc_loader.c | 531 
 
  9 files changed, 652 insertions(+), 9 deletions(-)
  create mode 100644 drivers/gpu/drm/i915/intel_guc.h
  create mode 100644 drivers/gpu/drm/i915/intel_guc_loader.c
 
 diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
 index 41fb8a9..cc359e0 100644
 --- a/drivers/gpu/drm/i915/Makefile
 +++ b/drivers/gpu/drm/i915/Makefile
 @@ -40,6 +40,9 @@ i915-y += i915_cmd_parser.o \
 intel_ringbuffer.o \
 intel_uncore.o
  
 +# general-purpose microcontroller (GuC) support
 +i915-y += intel_guc_loader.o
 +
  # autogenerated null render state
  i915-y += intel_renderstate_gen6.o \
 intel_renderstate_gen7.o \
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index ab37d11..2193cc2 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -435,6 +435,11 @@ static int i915_load_modeset_init(struct drm_device *dev)
* working irqs for e.g. gmbus and dp aux transfers. */
   intel_modeset_init(dev);
  
 + /* intel_guc_ucode_init() needs the mutex to allocate GEM objects */
 + mutex_lock(dev-struct_mutex);
 + intel_guc_ucode_init(dev);
 + mutex_unlock(dev-struct_mutex);
 +
   ret = i915_gem_init(dev);
   if (ret)
   goto cleanup_irq;
 @@ -476,6 +481,9 @@ cleanup_gem:
   i915_gem_context_fini(dev);
   mutex_unlock(dev-struct_mutex);
  cleanup_irq:
 + mutex_lock(dev-struct_mutex);
 + intel_guc_ucode_fini(dev);
 + mutex_unlock(dev-struct_mutex);
   drm_irq_uninstall(dev);
  cleanup_gem_stolen:
   i915_gem_cleanup_stolen(dev);
 @@ -1128,6 +1136,7 @@ int i915_driver_unload(struct drm_device *dev)
   flush_workqueue(dev_priv-wq);
  
   mutex_lock(dev-struct_mutex);
 + intel_guc_ucode_fini(dev);
   i915_gem_cleanup_ringbuffer(dev);
   i915_gem_context_fini(dev);
   mutex_unlock(dev-struct_mutex);
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 04aa34a..2c539df 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -50,6 +50,7 @@
  #include linux/intel-iommu.h
  #include linux/kref.h
  #include linux/pm_qos.h
 +#include intel_guc.h
  
  /* General customization:
   */
 @@ -1697,6 +1698,8 @@ struct drm_i915_private {
  
   struct i915_virtual_gpu vgpu;
  
 + struct intel_guc guc;
 +
   struct intel_csr csr;
  
   /* Display CSR-related protection */
 @@ -1941,6 +1944,11 @@ static inline struct drm_i915_private 
 *dev_to_i915(struct device *dev)
   return to_i915(dev_get_drvdata(dev));
  }
  
 +static inline struct drm_i915_private *guc_to_i915(struct intel_guc *guc)
 +{
 + return container_of(guc, struct drm_i915_private, guc);
 +}
 +
  /* Iterate over initialised rings */
  #define for_each_ring(ring__, dev_priv__, i__) \
   for ((i__) = 0; (i__)  I915_NUM_RINGS; (i__)++) \
 @@ -2544,6 +2552,9 @@ struct drm_i915_cmd_table {
  
  #define HAS_CSR(dev) (IS_SKYLAKE(dev))
  
 +#define HAS_GUC_UCODE(dev)   (IS_GEN9(dev))
 +#define HAS_GUC_SCHED(dev)   (IS_GEN9(dev))
 +
  #define HAS_RESOURCE_STREAMER(dev) (IS_HASWELL(dev) || \
 

Re: [Intel-gfx] [PATCH 07/13 v4] drm/i915: GuC submission setup, phase 1

[O'Rourke, Tom]

On Tue, Jul 28, 2015 at 08:40:58PM +0100, Dave Gordon wrote:
 On 28/07/15 16:16, Dave Gordon wrote:
 On 28/07/15 00:12, O'Rourke, Tom wrote:
 On Mon, Jul 27, 2015 at 03:41:31PM -0700, Yu Dai wrote:
 
 On 07/24/2015 03:31 PM, O'Rourke, Tom wrote:
 [TOR:] When I see phase 1 I also look for phase 2.
 A subject that better describes the change in this patch
 would help.
 
 On Thu, Jul 09, 2015 at 07:29:08PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 This adds the first of the data structures used to communicate with
 the GuC (the pool of guc_context structures).
 
 We create a GuC-specific wrapper round the GEM object allocator as all
 GEM objects shared with the GuC must be pinned into GGTT space at an
 address that is NOT in the range [0..WOPCM_SIZE), as that range of
 GGTT
 addresses is not accessible to the GuC (from the GuC's point of view,
 it's permanently reserved for other objects such as the BootROM 
 SRAM).
 
 [TOR:] I would like a clarfication on the excluded range.
 The excluded range should be 0 to size for guc within
 WOPCM area and not 0 to size of WOPCM area.
 
 Nope, GGTT range [0..WOPCM_SIZE) should be excluded from GuC usage.
 BSpec clearly says, from 0 to WOPCM_TOP-1 is for BootROM, SRAM and
 WOPCM. From WOPCM_TOP and above is GFX DRAM. Be note that, that GGTT
 space is still available to any gfx obj as long as it is not
 accessed by GuC (OK to pass through GuC).
 
 [TOR:] Should we take a closer look at the pin offset bias
 for guc objects?  GUC_WOPCM_SIZE_VALUE is not the full size
 of WOPCM area.
 
 I'm inclined to set the bias to GUC_WOPCM_TOP, and then define that as
 the sum of GUC_WOPCM_OFFSET_VALUE and GUC_WOPCM_SIZE_VALUE. That seems
 to be what the BSpec pages WriteOnceProtectedContentMemory (WOPCM)
 Management and WOPCM Memory Map suggest, although I think they're
 pretty unclear on the details :(
 
 Do you (both) agree this would be the right value?
 
 Actually I've changed my mind (again). On rereading this stuff, I
 now think that GUC_WOPCM_TOP is the same as the value put into the
 SIZE register. The (physical) range between the (real) WOPCM BASE
 and that plus the GUC WOPCM OFFSET isn't part of the GuC address
 space at all, so GuC address 0 maps (would map) to (real WOPCM
 BASE+GUC WOPCM OFFSET) in physical addresses, except that the bottom
 80k is shadowed by the bootrom and SRAM; and then the SIZE register
 defines the size of the range from (GuC address) 0 to GUC_WOPCM_TOP;
 and then higher addresses map through the GTT as expected.
 
 Or so I think. Does anyone know for sure? Please let me know ASAP as
 I want to submit an updated patchset tomorrow!
 
 Thanks,
 .Dave.

[TOR:] Hi Dave,  Sorry, I did not see your message earlier.
Please see my other reply on this thread.  I think you are
right here, but to be clear, I think by SIZE you mean
GUC_WOPCM_SIZE_VALUE.

Also, this should not matter here, but the SKL guc SRAM
shadow is 128k, not 80k.

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

Re: [Intel-gfx] [PATCH 07/13 v4] drm/i915: GuC submission setup, phase 1

[O'Rourke, Tom]

On Tue, Jul 28, 2015 at 04:16:03PM +0100, Dave Gordon wrote:
 On 28/07/15 00:12, O'Rourke, Tom wrote:
 On Mon, Jul 27, 2015 at 03:41:31PM -0700, Yu Dai wrote:
 
 On 07/24/2015 03:31 PM, O'Rourke, Tom wrote:
 [TOR:] When I see phase 1 I also look for phase 2.
 A subject that better describes the change in this patch
 would help.
 
 On Thu, Jul 09, 2015 at 07:29:08PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 This adds the first of the data structures used to communicate with the
 GuC (the pool of guc_context structures).
 
 We create a GuC-specific wrapper round the GEM object allocator as all
 GEM objects shared with the GuC must be pinned into GGTT space at an
 address that is NOT in the range [0..WOPCM_SIZE), as that range of GGTT
 addresses is not accessible to the GuC (from the GuC's point of view,
 it's permanently reserved for other objects such as the BootROM  SRAM).
 [TOR:] I would like a clarfication on the excluded range.
 The excluded range should be 0 to size for guc within
 WOPCM area and not 0 to size of WOPCM area.
 
 Nope, GGTT range [0..WOPCM_SIZE) should be excluded from GuC usage.
 BSpec clearly says, from 0 to WOPCM_TOP-1 is for BootROM, SRAM and
 WOPCM. From WOPCM_TOP and above is GFX DRAM. Be note that, that GGTT
 space is still available to any gfx obj as long as it is not
 accessed by GuC (OK to pass through GuC).
 
 [TOR:] Should we take a closer look at the pin offset bias
 for guc objects?  GUC_WOPCM_SIZE_VALUE is not the full size
 of WOPCM area.
 
 I'm inclined to set the bias to GUC_WOPCM_TOP, and then define that
 as the sum of GUC_WOPCM_OFFSET_VALUE and GUC_WOPCM_SIZE_VALUE. That
 seems to be what the BSpec pages WriteOnceProtectedContentMemory
 (WOPCM) Management and WOPCM Memory Map suggest, although I think
 they're pretty unclear on the details :(
 
 Do you (both) agree this would be the right value?

[TOR:] No, I do not think that is the right value.

I think the excluded range should be [0 ... GUC_WOPCM_SIZE_VALUE)
and that GUC_WOPCM_SIZE_VALUE should be used as the bias (as it
is now) for objects used by GuC.

The term WOPCM_SIZE is ambiguous since it could mean
GUC_WOPCM_SIZE (as in 0xc050) or it could mean size of
WOPCM area (as in 0x1082C0).  It gets used both ways
in the BSpec.

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

Re: [Intel-gfx] [PATCH 11/13 v4] drm/i915: Integrate GuC-based command submission

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:12PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 GuC-based submission is mostly the same as execlist mode, up to
 intel_logical_ring_advance_and_submit(), where the context being
 dispatched would be added to the execlist queue; at this point
 we submit the context to the GuC backend instead.
 
 There are, however, a few other changes also required, notably:
 1.  Contexts must be pinned at GGTT addresses accessible by the GuC
 i.e. NOT in the range [0..WOPCM_SIZE), so we have to add the
 PIN_OFFSET_BIAS flag to the relevant GGTT-pinning calls.
 
 2.  The GuC's TLB must be invalidated after a context is pinned at
 a new GGTT address.
 
 3.  GuC firmware uses the one page before Ring Context as shared data.
 Therefore, whenever driver wants to get base address of LRC, we
 will offset one page for it. LRC_PPHWSP_PN is defined as the page
 number of LRCA.
 
 4.  In the work queue used to pass requests to the GuC, the GuC
 firmware requires the ring-tail-offset to be represented as an
 11-bit value, expressed in QWords. Therefore, the ringbuffer
 size must be reduced to the representable range (4 pages).
 
 v2:
 Defer adding #defines until needed [Chris Wilson]
 Rationalise type declarations [Chris Wilson]
 
 v4:
 Squashed kerneldoc patch into here [Daniel Vetter]
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
  Documentation/DocBook/drm.tmpl | 14 
  drivers/gpu/drm/i915/i915_debugfs.c|  2 +-
  drivers/gpu/drm/i915/i915_guc_submission.c | 52 
 +++---
  drivers/gpu/drm/i915/intel_guc.h   |  1 +
  drivers/gpu/drm/i915/intel_lrc.c   | 51 -
  drivers/gpu/drm/i915/intel_lrc.h   |  6 
  6 files changed, 106 insertions(+), 20 deletions(-)
 
 diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
 index 596b11d..0ff5fd7 100644
 --- a/Documentation/DocBook/drm.tmpl
 +++ b/Documentation/DocBook/drm.tmpl
 @@ -4223,6 +4223,20 @@ int num_ioctls;/synopsis
/sect2
  /sect1
  sect1
 +  titleGuC-based Command Submission/title
 +  sect2
 +titleGuC/title
 +!Pdrivers/gpu/drm/i915/intel_guc_loader.c GuC-specific firmware loader
 +!Idrivers/gpu/drm/i915/intel_guc_loader.c
 +  /sect2
 +  sect2
 +titleGuC Client/title
 +!Pdrivers/gpu/drm/i915/intel_guc_submission.c GuC-based command submissison
 +!Idrivers/gpu/drm/i915/intel_guc_submission.c
 +  /sect2
 +/sect1
 +
 +sect1
title Tracing /title
para
  This sections covers all things related to the tracepoints implemented in
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 13e37d1..d93732a 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -1982,7 +1982,7 @@ static void i915_dump_lrc_obj(struct seq_file *m,
   return;
   }
  
 - page = i915_gem_object_get_page(ctx_obj, 1);
 + page = i915_gem_object_get_page(ctx_obj, LRC_STATE_PN);
   if (!WARN_ON(page == NULL)) {
   reg_state = kmap_atomic(page);
  
 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
 b/drivers/gpu/drm/i915/i915_guc_submission.c
 index 25d8807..c5c9fbf 100644
 --- a/drivers/gpu/drm/i915/i915_guc_submission.c
 +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
 @@ -346,18 +346,58 @@ static void guc_init_proc_desc(struct intel_guc *guc,
  static void guc_init_ctx_desc(struct intel_guc *guc,
 struct i915_guc_client *client)
  {
 + struct intel_context *ctx = client-owner;
   struct guc_context_desc desc;
   struct sg_table *sg;
 + int i;
  
   memset(desc, 0, sizeof(desc));
  
   desc.attribute = GUC_CTX_DESC_ATTR_ACTIVE | GUC_CTX_DESC_ATTR_KERNEL;
   desc.context_id = client-ctx_index;
   desc.priority = client-priority;
 - desc.engines_used = (1  RCS) | (1  VCS) | (1  BCS) |
 - (1  VECS) | (1  VCS2); /* all engines */
   desc.db_id = client-doorbell_id;
  
 + for (i = 0; i  I915_NUM_RINGS; i++) {
 + struct guc_execlist_context *lrc = desc.lrc[i];
 + struct intel_ringbuffer *ringbuf = ctx-engine[i].ringbuf;
 + struct intel_engine_cs *ring;
 + struct drm_i915_gem_object *obj;
 + uint64_t ctx_desc;
 +
 + /* TODO: We have a design issue to be solved here. Only when we
 +  * receive the first batch, we know which engine is used by the
 +  * user. But here GuC expects the lrc and ring to be pinned. It
 +  * is not an issue for default context, which is the only one
 +  * for now who owns a GuC client. But for future owner of GuC
 +  * client, need to make sure lrc is pinned prior to enter here.
 +  

Re: [Intel-gfx] [PATCH 10/13 v4] drm/i915: Interrupt routing for GuC submission

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:11PM +0100, Dave Gordon wrote:
 Turn on interrupt steering to route necessary interrupts to GuC.
 
 v4:
 Rebased
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
  drivers/gpu/drm/i915/i915_reg.h | 11 +--
  drivers/gpu/drm/i915/intel_guc_loader.c | 51 
 +
  2 files changed, 60 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 63728c1..1c2072b 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -1664,12 +1664,18 @@ enum skl_disp_power_wells {
  #define GFX_MODE_GEN70x0229c
  #define RING_MODE_GEN7(ring) ((ring)-mmio_base+0x29c)
  #define   GFX_RUN_LIST_ENABLE(115)
 +#define   GFX_INTERRUPT_STEERING (114)
  #define   GFX_TLB_INVALIDATE_EXPLICIT(113)
  #define   GFX_SURFACE_FAULT_ENABLE   (112)
  #define   GFX_REPLAY_MODE(111)
  #define   GFX_PSMI_GRANULARITY   (110)
  #define   GFX_PPGTT_ENABLE   (19)
  
 +#define   GFX_FORWARD_VBLANK_MASK(35)
 +#define   GFX_FORWARD_VBLANK_NEVER   (05)
 +#define   GFX_FORWARD_VBLANK_ALWAYS  (15)
 +#define   GFX_FORWARD_VBLANK_COND(25)
 +
  #define VLV_DISPLAY_BASE 0x18
  #define VLV_MIPI_BASE VLV_DISPLAY_BASE
  
 @@ -5683,11 +5689,12 @@ enum skl_disp_power_wells {
  #define GEN8_GT_IIR(which) (0x44308 + (0x10 * (which)))
  #define GEN8_GT_IER(which) (0x4430c + (0x10 * (which)))
  
 -#define GEN8_BCS_IRQ_SHIFT 16
  #define GEN8_RCS_IRQ_SHIFT 0
 -#define GEN8_VCS2_IRQ_SHIFT 16
 +#define GEN8_BCS_IRQ_SHIFT 16
  #define GEN8_VCS1_IRQ_SHIFT 0
 +#define GEN8_VCS2_IRQ_SHIFT 16
  #define GEN8_VECS_IRQ_SHIFT 0
 +#define GEN8_WD_IRQ_SHIFT 16
  
  #define GEN8_DE_PIPE_ISR(pipe) (0x44400 + (0x10 * (pipe)))
  #define GEN8_DE_PIPE_IMR(pipe) (0x44404 + (0x10 * (pipe)))
 diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
 b/drivers/gpu/drm/i915/intel_guc_loader.c
 index 25ba29f..2aa9227 100644
 --- a/drivers/gpu/drm/i915/intel_guc_loader.c
 +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
 @@ -79,6 +79,53 @@ const char *intel_guc_fw_status_repr(enum 
 intel_guc_fw_status status)
   }
  };
  
 +static void direct_interrupts_to_host(struct drm_i915_private *dev_priv)
 +{
 + struct intel_engine_cs *ring;
 + int i, irqs;
 +
 + /* tell all command streamers NOT to forward interrupts and vblank to 
 GuC */
 + irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER);
 + irqs |= _MASKED_BIT_DISABLE(GFX_INTERRUPT_STEERING);
 + for_each_ring(ring, dev_priv, i)
 + I915_WRITE(RING_MODE_GEN7(ring), irqs);
 +
 + /* tell DE to send nothing to GuC */
 + I915_WRITE(DE_GUCRMR, ~0);
 +
 + /* route all GT interrupts to the host */
 + I915_WRITE(GUC_BCS_RCS_IER, 0);
 + I915_WRITE(GUC_VCS2_VCS1_IER, 0);
 + I915_WRITE(GUC_WD_VECS_IER, 0);
 +}
 +
 +static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
 +{
 + struct intel_engine_cs *ring;
 + int i, irqs;
 +
 + /* tell all command streamers to forward interrupts and vblank to GuC */
 + irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, 
 GFX_FORWARD_VBLANK_ALWAYS);
 + irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
 + for_each_ring(ring, dev_priv, i)
 + I915_WRITE(RING_MODE_GEN7(ring), irqs);
 +
 + /* tell DE to send (all) flip_done to GuC */
 + irqs = DERRMR_PIPEA_PRI_FLIP_DONE | DERRMR_PIPEA_SPR_FLIP_DONE |
 +DERRMR_PIPEB_PRI_FLIP_DONE | DERRMR_PIPEB_SPR_FLIP_DONE |
 +DERRMR_PIPEC_PRI_FLIP_DONE | DERRMR_PIPEC_SPR_FLIP_DONE;
 + /* Unmasked bits will cause GuC response message to be sent */
 + I915_WRITE(DE_GUCRMR, ~irqs);
 +
 + /* route USER_INTERRUPT to Host, all others are sent to GuC. */
 + irqs = GT_RENDER_USER_INTERRUPT  GEN8_RCS_IRQ_SHIFT |
 +GT_RENDER_USER_INTERRUPT  GEN8_BCS_IRQ_SHIFT;
 + /* These three registers have the same bit definitions */
[TOR:] Reliance on the registers having the same bit
definitions does not seem safe.  Each of the three
registers has shift constants defined.  I would expect
the shift constants for the second and third registers
to be used when writing those registers.

Also, GT_RENDER_USER_INTERRUPT seems to have been defined
for use with a different register than this set.

On the other hand, this code does actually write the
correct values.

 + I915_WRITE(GUC_BCS_RCS_IER, ~irqs);
 + I915_WRITE(GUC_VCS2_VCS1_IER, ~irqs);
 + I915_WRITE(GUC_WD_VECS_IER, ~irqs);
 +}
 +
  static u32 get_gttype(struct drm_i915_private *dev_priv)
  {
   /* XXX: GT type based on PCI device ID? field seems unused by fw */
 @@ -427,6 +474,7 @@ int intel_guc_ucode_load(struct drm_device *dev)
   intel_guc_fw_status_repr(guc_fw-guc_fw_fetch_status),
   

Re: [Intel-gfx] [PATCH 12/13 v4] drm/i915: Debugfs interface for GuC submission statistics

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:13PM +0100, Dave Gordon wrote:
 This provides a means of reading status and counts relating
 to GuC actions and submissions.
 
 v2:
 Remove surplus blank line in output [Chris Wilson]
 
 v4:
 Rebased
 
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 Signed-off-by: Alex Dai yu@intel.com
 ---
Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com
  drivers/gpu/drm/i915/i915_debugfs.c | 40 
 +
  1 file changed, 40 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index d93732a..cebb93c 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -2397,6 +2397,45 @@ static int i915_guc_load_status_info(struct seq_file 
 *m, void *data)
   return 0;
  }
  
 +static int i915_guc_info(struct seq_file *m, void *data)
 +{
 + struct drm_info_node *node = m-private;
 + struct drm_device *dev = node-minor-dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + struct intel_guc guc;
 + struct i915_guc_client client = { .client_obj = 0 };
 +
 + if (!HAS_GUC_SCHED(dev_priv-dev))
 + return 0;
 +
 + /* Take a local copy of the GuC data, so we can dump it at leisure */
 + spin_lock(dev_priv-guc.host2guc_lock);
 + guc = dev_priv-guc;
 + if (guc.execbuf_client) {
 + spin_lock(guc.execbuf_client-wq_lock);
 + client = *guc.execbuf_client;
 + spin_unlock(guc.execbuf_client-wq_lock);
 + }
 + spin_unlock(dev_priv-guc.host2guc_lock);
 +
 + seq_printf(m, GuC total action count: %llu\n, guc.action_count);
 + seq_printf(m, GuC last action command: 0x%x\n, guc.action_cmd);
 + seq_printf(m, GuC last action status: 0x%x\n, guc.action_status);
 +
 + seq_printf(m, GuC action failure count: %u\n, guc.action_fail);
 + seq_printf(m, GuC last action error code: %d\n, guc.action_err);
 +
 + seq_printf(m, \nGuC execbuf client @ %p:\n, guc.execbuf_client);
 + seq_printf(m, \tTotal submissions: %llu\n, client.submissions);
 + seq_printf(m, \tFailed to queue: %u\n, client.q_fail);
 + seq_printf(m, \tFailed doorbell: %u\n, client.b_fail);
 + seq_printf(m, \tLast submission result: %d\n, client.retcode);
 +
 + /* Add more as required ... */
 +
 + return 0;
 +}
 +
  static int i915_guc_log_dump(struct seq_file *m, void *data)
  {
   struct drm_info_node *node = m-private;
 @@ -5139,6 +5178,7 @@ static const struct drm_info_list i915_debugfs_list[] = 
 {
   {i915_gem_hws_bsd, i915_hws_info, 0, (void *)VCS},
   {i915_gem_hws_vebox, i915_hws_info, 0, (void *)VECS},
   {i915_gem_batch_pool, i915_gem_batch_pool_info, 0},
 + {i915_guc_info, i915_guc_info, 0},
   {i915_guc_load_status, i915_guc_load_status_info, 0},
   {i915_guc_log_dump, i915_guc_log_dump, 0},
   {i915_frequency_info, i915_frequency_info, 0},
 -- 
 1.9.1
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/13 v4] drm/i915: GuC submission setup, phase 1

[O'Rourke, Tom]

On Mon, Jul 27, 2015 at 03:41:31PM -0700, Yu Dai wrote:
 
 
 On 07/24/2015 03:31 PM, O'Rourke, Tom wrote:
 [TOR:] When I see phase 1 I also look for phase 2.
 A subject that better describes the change in this patch
 would help.
 
 On Thu, Jul 09, 2015 at 07:29:08PM +0100, Dave Gordon wrote:
  From: Alex Dai yu@intel.com
 
  This adds the first of the data structures used to communicate with the
  GuC (the pool of guc_context structures).
 
  We create a GuC-specific wrapper round the GEM object allocator as all
  GEM objects shared with the GuC must be pinned into GGTT space at an
  address that is NOT in the range [0..WOPCM_SIZE), as that range of GGTT
  addresses is not accessible to the GuC (from the GuC's point of view,
  it's permanently reserved for other objects such as the BootROM  SRAM).
 [TOR:] I would like a clarfication on the excluded range.
 The excluded range should be 0 to size for guc within
 WOPCM area and not 0 to size of WOPCM area.
 
 Nope, GGTT range [0..WOPCM_SIZE) should be excluded from GuC usage.
 BSpec clearly says, from 0 to WOPCM_TOP-1 is for BootROM, SRAM and
 WOPCM. From WOPCM_TOP and above is GFX DRAM. Be note that, that GGTT
 space is still available to any gfx obj as long as it is not
 accessed by GuC (OK to pass through GuC).

[TOR:] Should we take a closer look at the pin offset bias
for guc objects?  GUC_WOPCM_SIZE_VALUE is not the full size
of WOPCM area.
 
 
  Later, we will need to allocate additional GuC-sharable objects for the
  submission client(s) and the GuC's debug log.
 
  v2:
  Remove redundant initialisation [Chris Wilson]
  Defer adding struct members until needed [Chris Wilson]
  Local functions should pass dev_priv rather than dev [Chris Wilson]
 
  v4:
  Rebased
 
  Issue: VIZ-4884
  Signed-off-by: Alex Dai yu@intel.com
  Signed-off-by: Dave Gordon david.s.gor...@intel.com
  ---
   drivers/gpu/drm/i915/Makefile  |   3 +-
   drivers/gpu/drm/i915/i915_guc_submission.c | 114 
  +
   drivers/gpu/drm/i915/intel_guc.h   |   7 ++
   drivers/gpu/drm/i915/intel_guc_loader.c|  19 +
   4 files changed, 142 insertions(+), 1 deletion(-)
   create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.c
 
  diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
  index e604cfe..23f5612 100644
  --- a/drivers/gpu/drm/i915/Makefile
  +++ b/drivers/gpu/drm/i915/Makefile
  @@ -40,7 +40,8 @@ i915-y += i915_cmd_parser.o \
   intel_uncore.o
 
   # general-purpose microcontroller (GuC) support
  -i915-y += intel_guc_loader.o
  +i915-y += intel_guc_loader.o \
  +i915_guc_submission.o
 
   # autogenerated null render state
   i915-y += intel_renderstate_gen6.o \
  diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
  b/drivers/gpu/drm/i915/i915_guc_submission.c
  new file mode 100644
  index 000..70a0405
  --- /dev/null
  +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
  @@ -0,0 +1,114 @@
  +/*
  + * 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/firmware.h
  +#include linux/circ_buf.h
  +#include i915_drv.h
  +#include intel_guc.h
  +
  +/**
  + * gem_allocate_guc_obj() - Allocate gem object for GuC usage
  + * @dev:  drm device
  + * @size: size of object
  + *
  + * This is a wrapper to create a gem obj. In order to use it inside GuC, 
  the
  + * object needs to be pinned lifetime. Also we must pin it to gtt space 
  other
  + * than [0, GUC_WOPCM_SIZE] because this range is reserved inside GuC.
  + *
  + * Return:A drm_i915_gem_object if successful, otherwise NULL.
  + */
  +static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device 
  *dev,
  +  u32 size)
  +{
  +  struct drm_i915_gem_object *obj

Re: [Intel-gfx] [PATCH 06/13 v4] drm/i915: Expose two LRC functions for GuC submission mode

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:07PM +0100, Dave Gordon wrote:
 GuC submission is basically execlist submission, but with the GuC
 handling the actual writes to the ELSP and the resulting context
 switch interrupts. So to prepare a context for submission via the
 GuC, we need some of the same functions used in execlist mode.
 This commit exposes two such functions, changing their names to
 better describe what they do (they're related to logical ring
 contexts rather than to execlists per se).
 
 v2:
 Replaces previous drm/i915: Move execlists defines from .c to .h
 
 v3:
 Incorporates a change to one of the functions exposed here that was
 previously part of an internal patch, but which was omitted from
 the version recently committed to drm-intel-nightly:
   7a01a0a drm/i915/lrc: Update PDPx registers with lri commands
 So we reinstate this change here.
 
 v4:
 Drop v3 change, update function parameters due to collision with
 8ee3615 drm/i915: Convert execlists_ctx_descriptor() for requests
 
 Issue: VIZ-4884
 Signed-off-by: Dave Gordon david.s.gor...@intel.com


Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com
 ---
  drivers/gpu/drm/i915/intel_lrc.c | 21 ++---
  drivers/gpu/drm/i915/intel_lrc.h |  3 +++
  2 files changed, 13 insertions(+), 11 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
 b/drivers/gpu/drm/i915/intel_lrc.c
 index d4f8b43..9e121d3 100644
 --- a/drivers/gpu/drm/i915/intel_lrc.c
 +++ b/drivers/gpu/drm/i915/intel_lrc.c
 @@ -261,11 +261,11 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object 
 *ctx_obj)
   return lrca  12;
  }
  
 -static uint64_t execlists_ctx_descriptor(struct drm_i915_gem_request *rq)
 +uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
 +  struct intel_engine_cs *ring)
  {
 - struct intel_engine_cs *ring = rq-ring;
   struct drm_device *dev = ring-dev;
 - struct drm_i915_gem_object *ctx_obj = rq-ctx-engine[ring-id].state;
 + struct drm_i915_gem_object *ctx_obj = ctx-engine[ring-id].state;
   uint64_t desc;
   uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj);
  
 @@ -303,13 +303,13 @@ static void execlists_elsp_write(struct 
 drm_i915_gem_request *rq0,
   uint64_t desc[2];
  
   if (rq1) {
 - desc[1] = execlists_ctx_descriptor(rq1);
 + desc[1] = intel_lr_context_descriptor(rq1-ctx, rq1-ring);
   rq1-elsp_submitted++;
   } else {
   desc[1] = 0;
   }
  
 - desc[0] = execlists_ctx_descriptor(rq0);
 + desc[0] = intel_lr_context_descriptor(rq0-ctx, rq0-ring);
   rq0-elsp_submitted++;
  
   /* You must always write both descriptors in the order below. */
 @@ -328,7 +328,8 @@ static void execlists_elsp_write(struct 
 drm_i915_gem_request *rq0,
   spin_unlock(dev_priv-uncore.lock);
  }
  
 -static int execlists_update_context(struct drm_i915_gem_request *rq)
 +/* Update the ringbuffer pointer and tail offset in a saved context image */
 +void intel_lr_context_update(struct drm_i915_gem_request *rq)
  {
   struct intel_engine_cs *ring = rq-ring;
   struct i915_hw_ppgtt *ppgtt = rq-ctx-ppgtt;
 @@ -358,17 +359,15 @@ static int execlists_update_context(struct 
 drm_i915_gem_request *rq)
   }
  
   kunmap_atomic(reg_state);
 -
 - return 0;
  }
  
  static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
 struct drm_i915_gem_request *rq1)
  {
 - execlists_update_context(rq0);
 + intel_lr_context_update(rq0);
  
   if (rq1)
 - execlists_update_context(rq1);
 + intel_lr_context_update(rq1);
  
   execlists_elsp_write(rq0, rq1);
  }
 @@ -2051,7 +2050,7 @@ populate_lr_context(struct intel_context *ctx, struct 
 drm_i915_gem_object *ctx_o
   reg_state[CTX_RING_TAIL+1] = 0;
   reg_state[CTX_RING_BUFFER_START] = RING_START(ring-mmio_base);
   /* Ring buffer start address is not known until the buffer is pinned.
 -  * It is written to the context image in execlists_update_context()
 +  * It is written to the context image in intel_lr_context_update()
*/
   reg_state[CTX_RING_BUFFER_CONTROL] = RING_CTL(ring-mmio_base);
   reg_state[CTX_RING_BUFFER_CONTROL+1] =
 diff --git a/drivers/gpu/drm/i915/intel_lrc.h 
 b/drivers/gpu/drm/i915/intel_lrc.h
 index e0299fb..6ecc0b3 100644
 --- a/drivers/gpu/drm/i915/intel_lrc.h
 +++ b/drivers/gpu/drm/i915/intel_lrc.h
 @@ -73,6 +73,9 @@ int intel_lr_context_deferred_create(struct intel_context 
 *ctx,
  void intel_lr_context_unpin(struct drm_i915_gem_request *req);
  void intel_lr_context_reset(struct drm_device *dev,
   struct intel_context *ctx);
 +void intel_lr_context_update(struct drm_i915_gem_request *rq);
 +uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
 +  struct intel_engine_cs *ring);
  
  /* Execlists */
  int 

Re: [Intel-gfx] [PATCH 08/13 v4] drm/i915: Enable GuC firmware log

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:09PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 Allocate a GEM object to hold GuC log data. A debugfs interface
 (i915_guc_log_dump) is provided to print out the log content.
 
 v2:
 Add struct members at point of use [Chris Wilson]
 
 v4:
 Rebased
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com

Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com

 ---
  drivers/gpu/drm/i915/i915_debugfs.c| 29 +++
  drivers/gpu/drm/i915/i915_guc_submission.c | 46 
 ++
  drivers/gpu/drm/i915/intel_guc.h   |  1 +
  3 files changed, 76 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 9ff5f17..13e37d1 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -2397,6 +2397,34 @@ static int i915_guc_load_status_info(struct seq_file 
 *m, void *data)
   return 0;
  }
  
 +static int i915_guc_log_dump(struct seq_file *m, void *data)
 +{
 + struct drm_info_node *node = m-private;
 + struct drm_device *dev = node-minor-dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + struct drm_i915_gem_object *log_obj = dev_priv-guc.log_obj;
 + u32 *log;
 + int i = 0, pg;
 +
 + if (!log_obj)
 + return 0;
 +
 + for (pg = 0; pg  log_obj-base.size / PAGE_SIZE; pg++) {
 + log = kmap_atomic(i915_gem_object_get_page(log_obj, pg));
 +
 + for (i = 0; i  PAGE_SIZE / sizeof(u32); i += 4)
 + seq_printf(m, 0x%08x 0x%08x 0x%08x 0x%08x\n,
 +*(log + i), *(log + i + 1),
 +*(log + i + 2), *(log + i + 3));
 +
 + kunmap_atomic(log);
 + }
 +
 + seq_putc(m, '\n');
 +
 + return 0;
 +}
 +
  static int i915_edp_psr_status(struct seq_file *m, void *data)
  {
   struct drm_info_node *node = m-private;
 @@ -5112,6 +5140,7 @@ static const struct drm_info_list i915_debugfs_list[] = 
 {
   {i915_gem_hws_vebox, i915_hws_info, 0, (void *)VECS},
   {i915_gem_batch_pool, i915_gem_batch_pool_info, 0},
   {i915_guc_load_status, i915_guc_load_status_info, 0},
 + {i915_guc_log_dump, i915_guc_log_dump, 0},
   {i915_frequency_info, i915_frequency_info, 0},
   {i915_hangcheck_info, i915_hangcheck_info, 0},
   {i915_drpc_info, i915_drpc_info, 0},
 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
 b/drivers/gpu/drm/i915/i915_guc_submission.c
 index 70a0405..e9d46d6 100644
 --- a/drivers/gpu/drm/i915/i915_guc_submission.c
 +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
 @@ -75,6 +75,47 @@ static void gem_release_guc_obj(struct drm_i915_gem_object 
 *obj)
   drm_gem_object_unreference(obj-base);
  }
  
 +static void guc_create_log(struct intel_guc *guc)
 +{
 + struct drm_i915_private *dev_priv = guc_to_i915(guc);
 + struct drm_i915_gem_object *obj;
 + unsigned long offset;
 + uint32_t size, flags;
 +
 + if (i915.guc_log_level  GUC_LOG_VERBOSITY_MIN)
 + return;
 +
 + if (i915.guc_log_level  GUC_LOG_VERBOSITY_MAX)
 + i915.guc_log_level = GUC_LOG_VERBOSITY_MAX;
 +
 + /* The first page is to save log buffer state. Allocate one
 +  * extra page for others in case for overlap */
 + size = (1 + GUC_LOG_DPC_PAGES + 1 +
 + GUC_LOG_ISR_PAGES + 1 +
 + GUC_LOG_CRASH_PAGES + 1)  PAGE_SHIFT;
 +
 + obj = guc-log_obj;
 + if (!obj) {
 + obj = gem_allocate_guc_obj(dev_priv-dev, size);
 + if (!obj) {
 + /* logging will be off */
 + i915.guc_log_level = -1;
 + return;
 + }
 +
 + guc-log_obj = obj;
 + }
 +
 + /* each allocated unit is a page */
 + flags = GUC_LOG_VALID | GUC_LOG_NOTIFY_ON_HALF_FULL |
 + (GUC_LOG_DPC_PAGES  GUC_LOG_DPC_SHIFT) |
 + (GUC_LOG_ISR_PAGES  GUC_LOG_ISR_SHIFT) |
 + (GUC_LOG_CRASH_PAGES  GUC_LOG_CRASH_SHIFT);
[TOR:] How does the log half full interrupt get handled?

 +
 + offset = i915_gem_obj_ggtt_offset(obj)  PAGE_SHIFT; /* in pages */
 + guc-log_flags = (offset  GUC_LOG_BUF_ADDR_SHIFT) | flags;
 +}
 +
  /*
   * Set up the memory resources to be shared with the GuC.  At this point,
   * we require just one object that can be mapped through the GGTT.
 @@ -99,6 +140,8 @@ int i915_guc_submission_init(struct drm_device *dev)
  
   ida_init(guc-ctx_ids);
  
 + guc_create_log(guc);
 +
   return 0;
  }
  
 @@ -107,6 +150,9 @@ void i915_guc_submission_fini(struct drm_device *dev)
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_guc *guc = dev_priv-guc;
  
 + gem_release_guc_obj(dev_priv-guc.log_obj);
 + guc-log_obj = NULL;
 +
   if (guc-ctx_pool_obj)
   

Re: [Intel-gfx] [PATCH 07/13 v4] drm/i915: GuC submission setup, phase 1

[O'Rourke, Tom]

[TOR:] When I see phase 1 I also look for phase 2.  
A subject that better describes the change in this patch 
would help.

On Thu, Jul 09, 2015 at 07:29:08PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 This adds the first of the data structures used to communicate with the
 GuC (the pool of guc_context structures).
 
 We create a GuC-specific wrapper round the GEM object allocator as all
 GEM objects shared with the GuC must be pinned into GGTT space at an
 address that is NOT in the range [0..WOPCM_SIZE), as that range of GGTT
 addresses is not accessible to the GuC (from the GuC's point of view,
 it's permanently reserved for other objects such as the BootROM  SRAM).
[TOR:] I would like a clarfication on the excluded range.
The excluded range should be 0 to size for guc within
WOPCM area and not 0 to size of WOPCM area.

 
 Later, we will need to allocate additional GuC-sharable objects for the
 submission client(s) and the GuC's debug log.
 
 v2:
 Remove redundant initialisation [Chris Wilson]
 Defer adding struct members until needed [Chris Wilson]
 Local functions should pass dev_priv rather than dev [Chris Wilson]
 
 v4:
 Rebased
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
  drivers/gpu/drm/i915/Makefile  |   3 +-
  drivers/gpu/drm/i915/i915_guc_submission.c | 114 
 +
  drivers/gpu/drm/i915/intel_guc.h   |   7 ++
  drivers/gpu/drm/i915/intel_guc_loader.c|  19 +
  4 files changed, 142 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.c
 
 diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
 index e604cfe..23f5612 100644
 --- a/drivers/gpu/drm/i915/Makefile
 +++ b/drivers/gpu/drm/i915/Makefile
 @@ -40,7 +40,8 @@ i915-y += i915_cmd_parser.o \
 intel_uncore.o
  
  # general-purpose microcontroller (GuC) support
 -i915-y += intel_guc_loader.o
 +i915-y += intel_guc_loader.o \
 +   i915_guc_submission.o
  
  # autogenerated null render state
  i915-y += intel_renderstate_gen6.o \
 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
 b/drivers/gpu/drm/i915/i915_guc_submission.c
 new file mode 100644
 index 000..70a0405
 --- /dev/null
 +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
 @@ -0,0 +1,114 @@
 +/*
 + * 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/firmware.h
 +#include linux/circ_buf.h
 +#include i915_drv.h
 +#include intel_guc.h
 +
 +/**
 + * gem_allocate_guc_obj() - Allocate gem object for GuC usage
 + * @dev: drm device
 + * @size:size of object
 + *
 + * This is a wrapper to create a gem obj. In order to use it inside GuC, the
 + * object needs to be pinned lifetime. Also we must pin it to gtt space other
 + * than [0, GUC_WOPCM_SIZE] because this range is reserved inside GuC.
 + *
 + * Return:   A drm_i915_gem_object if successful, otherwise NULL.
 + */
 +static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device 
 *dev,
 + u32 size)
 +{
 + struct drm_i915_gem_object *obj;
 +
 + obj = i915_gem_alloc_object(dev, size);
 + if (!obj)
 + return NULL;
 +
 + if (i915_gem_object_get_pages(obj)) {
 + drm_gem_object_unreference(obj-base);
 + return NULL;
 + }
 +
 + if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE,
 + PIN_OFFSET_BIAS | GUC_WOPCM_SIZE_VALUE)) {
 + drm_gem_object_unreference(obj-base);
 + return NULL;
 + }
 +
 + return obj;
 +}
 +
 +/**
 + * gem_release_guc_obj() - Release gem object allocated for GuC usage
 + * @obj: gem obj to be released
 +  */
 +static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 +{
 +

Re: [Intel-gfx] [PATCH 03/13 v4] drm/i915: Add GuC-related header files

[O'Rourke, Tom]

On Tue, Jul 21, 2015 at 08:38:35AM +0200, Daniel Vetter wrote:
 On Fri, Jul 17, 2015 at 05:38:05PM -0700, O'Rourke, Tom wrote:
  On Thu, Jul 09, 2015 at 07:29:04PM +0100, Dave Gordon wrote:
   intel_guc_fwif.h contains the subset of the GuC interface that we
   will need for submission of commands through the GuC. These MUST
   be kept in sync with the definitions used by the GuC firmware, and
   updates to this file will (or should) be autogenerated from the
   source files used to build the firmware. Editing this file is
   therefore not recommended.
   
   i915_guc_reg.h contains definitions of GuC-related hardware:
   registers, bitmasks, etc. These should match the BSpec.
   
   v2:
   Files renamed  resliced per review comments by Chris Wilson
   
   v4:
   Added DON'T-EDIT-ME warning [Tom O'Rourke]
   
   Issue: VIZ-4884
   Signed-off-by: Alex Dai yu@intel.com
   Signed-off-by: Dave Gordon david.s.gor...@intel.com
   ---
  Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com
 
 Merged up to this patch, thanks.
 -Daniel

[TOR:] Thank you. Please hold merging remaining patches in
this series until guc firmware v3 is available on 01.org.
Tom
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/13 v4] drm/i915: Implementation of GuC client

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:10PM +0100, Dave Gordon wrote:
 A GuC client has its own doorbell and workqueue. It maintains the
 doorbell cache line, process description object and work queue item.
 
 A default guc_client is created for the i915 driver to use for
 normal-priority in-order submission.
 
 Note that the created client is not yet ready for use; doorbell
 allocation will fail as we haven't yet linked the GuC's context
 descriptor to the default contexts for each ring (see later patch).
 
 v2:
 Defer adding structure members until needed [Chris Wilson]
 Rationalise type declarations [Chris Wilson]
 
 v4:
 Rebased
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com

[TOR:] I had some non-critical questions below.

Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com
 ---
  drivers/gpu/drm/i915/i915_guc_submission.c | 649 
 +
  drivers/gpu/drm/i915/intel_guc.h   |  42 ++
  drivers/gpu/drm/i915/intel_guc_loader.c|  12 +
  3 files changed, 703 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
 b/drivers/gpu/drm/i915/i915_guc_submission.c
 index e9d46d6..25d8807 100644
 --- a/drivers/gpu/drm/i915/i915_guc_submission.c
 +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
 @@ -27,6 +27,512 @@
  #include intel_guc.h
  
  /**
 + * DOC: GuC Client
 + *
 + * i915_guc_client:
 + * We use the term client to avoid confusion with contexts. A 
 i915_guc_client is
 + * equivalent to GuC object guc_context_desc. This context descriptor is
 + * allocated from a pool of 1024 entries. Kernel driver will allocate 
 doorbell
 + * and workqueue for it. Also the process descriptor (guc_process_desc), 
 which
 + * is mapped to client space. So the client can write Work Item then ring the
 + * doorbell.
 + *
 + * To simplify the implementation, we allocate one gem object that contains 
 all
 + * pages for doorbell, process descriptor and workqueue.
 + *
 + * The Scratch registers:
 + * There are 16 MMIO-based registers start from 0xC180. The kernel driver 
 writes
 + * a value to the action register (SOFT_SCRATCH_0) along with any data. It 
 then
 + * triggers an interrupt on the GuC via another register write (0xC4C8).
 + * Firmware writes a success/fail code back to the action register after
 + * processes the request. The kernel driver polls waiting for this update and
 + * then proceeds.
 + * See host2guc_action()
 + *
 + * Doorbells:
 + * Doorbells are interrupts to uKernel. A doorbell is a single cache line 
 (QW)
 + * mapped into process space.
 + *
 + * Work Items:
 + * There are several types of work items that the host may place into a
 + * workqueue, each with its own requirements and limitations. Currently only
 + * WQ_TYPE_INORDER is needed to support legacy submission via GuC, which
 + * represents in-order queue. The kernel driver packs ring tail pointer and 
 an
 + * ELSP context descriptor dword into Work Item.
 + * See guc_add_workqueue_item()
 + *
 + */
 +
 +/*
 + * Read GuC command/status register (SOFT_SCRATCH_0)
 + * Return true if it contains a response rather than a command
 + */
 +static inline bool host2guc_action_response(struct drm_i915_private 
 *dev_priv,
 + u32 *status)
 +{
 + u32 val = I915_READ(SOFT_SCRATCH(0));
 + *status = val;
 + return GUC2HOST_IS_RESPONSE(val);
 +}
 +
 +static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len)
 +{
 + struct drm_i915_private *dev_priv = guc_to_i915(guc);
 + u32 status;
 + int i;
 + int ret;
 +
 + if (WARN_ON(len  1 || len  15))
 + return -EINVAL;
 +

[TOR:] Would it be good for host2guc_action to take a 
forcewake?  There are several writes and polling reads 
for completion.  Taking a forcewake could avoid surplus 
forcewakes for each register access.

 + spin_lock(dev_priv-guc.host2guc_lock);
 +
 + dev_priv-guc.action_count += 1;
 + dev_priv-guc.action_cmd = data[0];
 +
 + for (i = 0; i  len; i++)
 + I915_WRITE(SOFT_SCRATCH(i), data[i]);
 +
 + POSTING_READ(SOFT_SCRATCH(i - 1));
 +
 + I915_WRITE(HOST2GUC_INTERRUPT, HOST2GUC_TRIGGER);
 +
 + ret = wait_for_atomic(host2guc_action_response(dev_priv, status), 10);
[TOR:] Why 10?

 + if (status != GUC2HOST_STATUS_SUCCESS) {
 + /* either GuC doesn't respond, which is a TIMEOUT,
 +  * or a failure code is returned. */
 + if (ret != -ETIMEDOUT)
 + ret = -EIO;
 +
 + DRM_ERROR(GUC: host2guc action 0x%X failed. ret=%d 
 + status=0x%08X response=0x%08X\n,
 + data[0], ret, status,
 + I915_READ(SOFT_SCRATCH(15)));
 +
 + dev_priv-guc.action_fail += 1;
 + dev_priv-guc.action_err = ret;
 + }
 + dev_priv-guc.action_status = status;
 +
 + 

Re: [Intel-gfx] [PATCH 04/13 v4] drm/i915: GuC-specific firmware loader

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:05PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 This fetches the required firmware image from the filesystem,
 then loads it into the GuC's memory via a dedicated DMA engine.
 
 This patch is derived from GuC loading work originally done by
 Vinit Azad and Ben Widawsky.
 
 v2:
 Various improvements per review comments by Chris Wilson
 
 v3:
 Removed 'wait' parameter to intel_guc_ucode_load() as firmware
 prefetch is no longer supported in the common firmware loader,
   per Daniel Vetter's request.
 Firmware checker callback fn now returns errno rather than bool.
 
 v4:
 Squash uC-independent code into GuC-specifc loader [Daniel Vetter]
 Don't keep the driver working (by falling back to execlist mode)
 if GuC firmware loading fails [Daniel Vetter]
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
  drivers/gpu/drm/i915/Makefile   |   3 +
  drivers/gpu/drm/i915/i915_dma.c |   4 +
  drivers/gpu/drm/i915/i915_drv.h |  11 +
  drivers/gpu/drm/i915/i915_gem.c |  13 +
  drivers/gpu/drm/i915/i915_reg.h |   4 +-
  drivers/gpu/drm/i915/intel_guc.h|  67 
  drivers/gpu/drm/i915/intel_guc_loader.c | 536 
 
  7 files changed, 637 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/i915/intel_guc.h
  create mode 100644 drivers/gpu/drm/i915/intel_guc_loader.c
 
 diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
 index de21965..e604cfe 100644
 --- a/drivers/gpu/drm/i915/Makefile
 +++ b/drivers/gpu/drm/i915/Makefile
 @@ -39,6 +39,9 @@ i915-y += i915_cmd_parser.o \
 intel_ringbuffer.o \
 intel_uncore.o
  
 +# general-purpose microcontroller (GuC) support
 +i915-y += intel_guc_loader.o
 +
  # autogenerated null render state
  i915-y += intel_renderstate_gen6.o \
 intel_renderstate_gen7.o \
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 066c34c..958ab4f 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -472,6 +472,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
  
  cleanup_gem:
   mutex_lock(dev-struct_mutex);
 + intel_guc_ucode_fini(dev);
   i915_gem_cleanup_ringbuffer(dev);
   i915_gem_context_fini(dev);
   mutex_unlock(dev-struct_mutex);
 @@ -869,6 +870,8 @@ int i915_driver_load(struct drm_device *dev, unsigned 
 long flags)
  
   intel_uncore_init(dev);
  
 + intel_guc_ucode_init(dev);
 +
   /* Load CSR Firmware for SKL */
   intel_csr_ucode_init(dev);
  
 @@ -1120,6 +1123,7 @@ int i915_driver_unload(struct drm_device *dev)
   flush_workqueue(dev_priv-wq);
  
   mutex_lock(dev-struct_mutex);
 + intel_guc_ucode_fini(dev);
   i915_gem_cleanup_ringbuffer(dev);
   i915_gem_context_fini(dev);
   mutex_unlock(dev-struct_mutex);
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 4a512da..15b9202 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -50,6 +50,7 @@
  #include linux/intel-iommu.h
  #include linux/kref.h
  #include linux/pm_qos.h
 +#include intel_guc.h
  
  /* General customization:
   */
 @@ -1694,6 +1695,8 @@ struct drm_i915_private {
  
   struct i915_virtual_gpu vgpu;
  
 + struct intel_guc guc;
 +
   struct intel_csr csr;
  
   /* Display CSR-related protection */
 @@ -1938,6 +1941,11 @@ static inline struct drm_i915_private 
 *dev_to_i915(struct device *dev)
   return to_i915(dev_get_drvdata(dev));
  }
  
 +static inline struct drm_i915_private *guc_to_i915(struct intel_guc *guc)
 +{
 + return container_of(guc, struct drm_i915_private, guc);
 +}
 +
  /* Iterate over initialised rings */
  #define for_each_ring(ring__, dev_priv__, i__) \
   for ((i__) = 0; (i__)  I915_NUM_RINGS; (i__)++) \
 @@ -2543,6 +2551,9 @@ struct drm_i915_cmd_table {
  
  #define HAS_CSR(dev) (IS_SKYLAKE(dev))
  
 +#define HAS_GUC_UCODE(dev)   (IS_GEN9(dev))
 +#define HAS_GUC_SCHED(dev)   (IS_GEN9(dev))
 +
  #define HAS_RESOURCE_STREAMER(dev) (IS_HASWELL(dev) || \
   INTEL_INFO(dev)-gen = 8)
  
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index dbbb649..e020309 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -5074,6 +5074,19 @@ i915_gem_init_hw(struct drm_device *dev)
   goto out;
   }
  
 + /* We can't enable contexts until all firmware is loaded */
 + ret = intel_guc_ucode_load(dev);
 +
 + /*
 +  * If we got an error and GuC submission is enabled, map
 +  * the error to -EIO so the GPU will be declared wedged.
 +  * OTOH, if we didn't intend to use the GuC anyway, just
 +  * discard the error and carry on.
 +  */
 + ret = 

Re: [Intel-gfx] [PATCH 01/13 v4] drm/i915: Add i915_gem_object_create_from_data()

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:02PM +0100, Dave Gordon wrote:
 i915_gem_object_create_from_data() is a generic function to save data
 from a plain linear buffer in a new pageable gem object that can later
 be accessed by the CPU and/or GPU.
 
 We will need this for the microcontroller firmware loading support code.
 
 Derived from i915_gem_object_write(), originally by Alex Dai
 
 v2:
 Change of function: now allocates  fills a new object, rather than
 writing to an existing object
 New name courtesy of Chris Wilson
 Explicit domain-setting and other improvements per review comments
 by Chris Wilson  Daniel Vetter
 
 v4:
 Rebased
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com
  drivers/gpu/drm/i915/i915_drv.h |  2 ++
  drivers/gpu/drm/i915/i915_gem.c | 40 
  2 files changed, 42 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 464b28d..3c91507 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -2755,6 +2755,8 @@ void i915_gem_object_init(struct drm_i915_gem_object 
 *obj,
const struct drm_i915_gem_object_ops *ops);
  struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
 size_t size);
 +struct drm_i915_gem_object *i915_gem_object_create_from_data(
 + struct drm_device *dev, const void *data, size_t size);
  void i915_init_vm(struct drm_i915_private *dev_priv,
 struct i915_address_space *vm);
  void i915_gem_free_object(struct drm_gem_object *obj);
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index a0bff41..dbbb649 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -5478,3 +5478,43 @@ bool i915_gem_obj_is_pinned(struct drm_i915_gem_object 
 *obj)
  
   return false;
  }
 +
 +/* Allocate a new GEM object and fill it with the supplied data */
 +struct drm_i915_gem_object *
 +i915_gem_object_create_from_data(struct drm_device *dev,
 +  const void *data, size_t size)
 +{
 + struct drm_i915_gem_object *obj;
 + struct sg_table *sg;
 + size_t bytes;
 + int ret;
 +
 + obj = i915_gem_alloc_object(dev, round_up(size, PAGE_SIZE));
 + if (IS_ERR_OR_NULL(obj))
 + return obj;
 +
 + ret = i915_gem_object_set_to_cpu_domain(obj, true);
 + if (ret)
 + goto fail;
 +
 + ret = i915_gem_object_get_pages(obj);
 + if (ret)
 + goto fail;
 +
 + i915_gem_object_pin_pages(obj);
 + sg = obj-pages;
 + bytes = sg_copy_from_buffer(sg-sgl, sg-nents, (void *)data, size);
 + i915_gem_object_unpin_pages(obj);
 +
 + if (WARN_ON(bytes != size)) {
 + DRM_ERROR(Incomplete copy, wrote %zu of %zu, bytes, size);
 + ret = -EFAULT;
 + goto fail;
 + }
 +
 + return obj;
 +
 +fail:
 + drm_gem_object_unreference(obj-base);
 + return ERR_PTR(ret);
 +}
 -- 
 1.9.1
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/13 v4] drm/i915: Add GuC-related header files

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:04PM +0100, Dave Gordon wrote:
 intel_guc_fwif.h contains the subset of the GuC interface that we
 will need for submission of commands through the GuC. These MUST
 be kept in sync with the definitions used by the GuC firmware, and
 updates to this file will (or should) be autogenerated from the
 source files used to build the firmware. Editing this file is
 therefore not recommended.
 
 i915_guc_reg.h contains definitions of GuC-related hardware:
 registers, bitmasks, etc. These should match the BSpec.
 
 v2:
 Files renamed  resliced per review comments by Chris Wilson
 
 v4:
 Added DON'T-EDIT-ME warning [Tom O'Rourke]
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com

  drivers/gpu/drm/i915/i915_guc_reg.h   | 102 ++
  drivers/gpu/drm/i915/intel_guc_fwif.h | 245 
 ++
  2 files changed, 347 insertions(+)
  create mode 100644 drivers/gpu/drm/i915/i915_guc_reg.h
  create mode 100644 drivers/gpu/drm/i915/intel_guc_fwif.h
 
 diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h 
 b/drivers/gpu/drm/i915/i915_guc_reg.h
 new file mode 100644
 index 000..ccdc6c8
 --- /dev/null
 +++ b/drivers/gpu/drm/i915/i915_guc_reg.h
 @@ -0,0 +1,102 @@
 +/*
 + * 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 _I915_GUC_REG_H_
 +#define _I915_GUC_REG_H_
 +
 +/* Definitions of GuC H/W registers, bits, etc */
 +
 +#define GUC_STATUS   0xc000
 +#define   GS_BOOTROM_SHIFT   1
 +#define   GS_BOOTROM_MASK  (0x7F  GS_BOOTROM_SHIFT)
 +#define   GS_BOOTROM_RSA_FAILED(0x50  GS_BOOTROM_SHIFT)
 +#define   GS_UKERNEL_SHIFT   8
 +#define   GS_UKERNEL_MASK  (0xFF  GS_UKERNEL_SHIFT)
 +#define   GS_UKERNEL_LAPIC_DONE(0x30  GS_UKERNEL_SHIFT)
 +#define   GS_UKERNEL_DPC_ERROR (0x60  GS_UKERNEL_SHIFT)
 +#define   GS_UKERNEL_READY (0xF0  GS_UKERNEL_SHIFT)
 +#define   GS_MIA_SHIFT   16
 +#define   GS_MIA_MASK  (0x07  GS_MIA_SHIFT)
 +
 +#define GUC_WOPCM_SIZE   0xc050
 +#define   GUC_WOPCM_SIZE_VALUE (0x80  12)  /* 512KB */
 +#define GUC_WOPCM_OFFSET 0x8 /* 512KB */
 +
 +#define SOFT_SCRATCH(n)  (0xc180 + ((n) * 4))
 +
 +#define UOS_RSA_SCRATCH_00xc200
 +#define DMA_ADDR_0_LOW   0xc300
 +#define DMA_ADDR_0_HIGH  0xc304
 +#define DMA_ADDR_1_LOW   0xc308
 +#define DMA_ADDR_1_HIGH  0xc30c
 +#define   DMA_ADDRESS_SPACE_WOPCM  (7  16)
 +#define   DMA_ADDRESS_SPACE_GTT(8  16)
 +#define DMA_COPY_SIZE0xc310
 +#define DMA_CTRL 0xc314
 +#define   UOS_MOVE (14)
 +#define   START_DMA(10)
 +#define DMA_GUC_WOPCM_OFFSET 0xc340
 +
 +#define GEN8_GT_PM_CONFIG0x138140
 +#define GEN9_GT_PM_CONFIG0x13816c
 +#define   GEN8_GT_DOORBELL_ENABLE  (10)
 +
 +#define GEN8_GTCR0x4274
 +#define   GEN8_GTCR_INVALIDATE (10)
 +
 +#define GUC_ARAT_C6DIS   0xA178
 +
 +#define GUC_SHIM_CONTROL 0xc064
 +#define   GUC_DISABLE_SRAM_INIT_TO_ZEROES(10)
 +#define   GUC_ENABLE_READ_CACHE_LOGIC(11)
 +#define   GUC_ENABLE_MIA_CACHING (12)
 +#define   GUC_GEN10_MSGCH_ENABLE (14)
 +#define   GUC_ENABLE_READ_CACHE_FOR_SRAM_DATA(19)
 +#define   GUC_ENABLE_READ_CACHE_FOR_WOPCM_DATA   (110)
 +#define   GUC_ENABLE_MIA_CLOCK_GATING(115)
 +#define   GUC_GEN10_SHIM_WC_ENABLE   

Re: [Intel-gfx] [PATCH 02/13 v4] drm/i915: Add GuC-related module parameters

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:03PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 Two new module parameters: enable_guc_submission which will turn
 on submission of batchbuffers via the GuC (when implemented), and
 guc_log_level which controls the level of debugging logged by the
 GuC and captured by the host.
 
 Signed-off-by: Alex Dai yu@intel.com
 
 v4:
 Mark enable_guc_submission unsafe [Daniel Vetter]
 
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com

  drivers/gpu/drm/i915/i915_drv.h| 2 ++
  drivers/gpu/drm/i915/i915_params.c | 9 +
  2 files changed, 11 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 3c91507..4a512da 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -2606,6 +2606,8 @@ struct i915_params {
   bool reset;
   bool disable_display;
   bool disable_vtd_wa;
 + bool enable_guc_submission;
 + int guc_log_level;
   int use_mmio_flip;
   int mmio_debug;
   bool verbose_state_checks;
 diff --git a/drivers/gpu/drm/i915/i915_params.c 
 b/drivers/gpu/drm/i915/i915_params.c
 index 7983fe4..2791b5a 100644
 --- a/drivers/gpu/drm/i915/i915_params.c
 +++ b/drivers/gpu/drm/i915/i915_params.c
 @@ -53,6 +53,8 @@ struct i915_params i915 __read_mostly = {
   .verbose_state_checks = 1,
   .nuclear_pageflip = 0,
   .edp_vswing = 0,
 + .enable_guc_submission = false,
 + .guc_log_level = -1,
  };
  
  module_param_named(modeset, i915.modeset, int, 0400);
 @@ -186,3 +188,10 @@ MODULE_PARM_DESC(edp_vswing,
Ignore/Override vswing pre-emph table selection from VBT 
(0=use value from vbt [default], 1=low power swing(200mV),
2=default swing(400mV)));
 +
 +module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, 
 bool, 0400);
 +MODULE_PARM_DESC(enable_guc_submission, Enable GuC submission 
 (default:false));
 +
 +module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
 +MODULE_PARM_DESC(guc_log_level,
 + GuC firmware logging level (-1:disabled (default), 0-3:enabled));
 -- 
 1.9.1
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/13 v4] Batch submission via GuC

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:01PM +0100, Dave Gordon wrote:
 This patch series enables command submission via the GuC. In this mode,
 instead of the host CPU driving the execlist port directly, it hands
 over work items to the GuC, using a doorbell mechanism to tell the GuC
 that new items have been added to its work queue. The GuC then dispatches
 contexts to the various GPU engines, and manages the resulting context-
 switch interrupts. Completion of a batch is however still signalled to
 the CPU; the GuC is not involved in handling user interrupts.
 
 There are two subsequences within the patch series:
 
   drm/i915: Add i915_gem_object_create_from_data()
   drm/i915: Add GuC-related module parameters
   drm/i915: Add GuC-related header files
   drm/i915: GuC-specific firmware loader
   drm/i915: Debugfs interface to read GuC load status
 
 These five patches make up the GuC loader and its prerequisites.  At this
 point in the sequence we can load and activate the GuC firmware, but not
 submit any batches through it. (This is nonetheless a potentially useful
 state, as the GuC could do other useful work even when not handling batch
 submissions).
 
   drm/i915: Expose two LRC functions for GuC submission mode
   drm/i915: GuC submission setup, phase 1
   drm/i915: Enable GuC firmware log
   drm/i915: Implementation of GuC client
   drm/i915: Interrupt routing for GuC submission
   drm/i915: Integrate GuC-based command submission
   drm/i915: Debugfs interface for GuC submission statistics
   drm/i915: Enable GuC submission, where supported
 
 In this second section, we implement the GuC submission mechanism, link
 it into the (execlist-based) submission path, and finally enable it
 (on supported platforms). On platforms where there is no GuC, or if
 GuC submission is explicitly disabled, batch submission will revert to
 using the execlist mechanism directly.
 
 On the other hand, if the GuC firmware cannot be found or is invalid,
 the GPU will be unusable.
 
 The GuC firmware itself is not included in this patchset; it is or will
 be available for download from https://01.org/linuxgraphics/downloads/
 This driver works with and requires GuC firmware revision 3.x. It will
 not work with any firmware version 1.x, as the GuC protocol in those
 revisions was incompatible and is no longer supported.

[TOR:] I finished reviewing the first 5 patches for GuC
firmware loading.  These patches look ready to go.
Should we wait until the GuC version 3 firmware is
available from 01.org before merging?

I am still working on the second section for GuC submission.

Thanks,
Tom
 
 Ben Widawsky (0):
 Vinit Azad (0):
 Michael H. Nguyen (0):
   created the original versions on which some of these patches are based.
 
 Alex Dai (6):
   drm/i915: Add GuC-related module parameters
   drm/i915: GuC-specific firmware loader
   drm/i915: Debugfs interface to read GuC load status
   drm/i915: GuC submission setup, phase 1
   drm/i915: Enable GuC firmware log
   drm/i915: Integrate GuC-based command submission
 
 Dave Gordon (7):
   drm/i915: Add i915_gem_object_create_from_data()
   drm/i915: Add GuC-related header files
   drm/i915: Expose two LRC functions for GuC submission mode
   drm/i915: Implementation of GuC client
   drm/i915: Interrupt routing for GuC submission
   drm/i915: Debugfs interface for GuC submission statistics
   drm/i915: Enable GuC submission, where supported
 
  Documentation/DocBook/drm.tmpl |  14 +
  drivers/gpu/drm/i915/Makefile  |   4 +
  drivers/gpu/drm/i915/i915_debugfs.c| 110 +++-
  drivers/gpu/drm/i915/i915_dma.c|   4 +
  drivers/gpu/drm/i915/i915_drv.h|  15 +
  drivers/gpu/drm/i915/i915_gem.c|  53 ++
  drivers/gpu/drm/i915/i915_guc_reg.h| 102 
  drivers/gpu/drm/i915/i915_guc_submission.c | 853 
 +
  drivers/gpu/drm/i915/i915_params.c |   9 +
  drivers/gpu/drm/i915/i915_reg.h|  15 +-
  drivers/gpu/drm/i915/intel_guc.h   | 118 
  drivers/gpu/drm/i915/intel_guc_fwif.h  | 245 +
  drivers/gpu/drm/i915/intel_guc_loader.c| 618 +
  drivers/gpu/drm/i915/intel_lrc.c   |  72 ++-
  drivers/gpu/drm/i915/intel_lrc.h   |   9 +
  15 files changed, 2211 insertions(+), 30 deletions(-)
  create mode 100644 drivers/gpu/drm/i915/i915_guc_reg.h
  create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.c
  create mode 100644 drivers/gpu/drm/i915/intel_guc.h
  create mode 100644 drivers/gpu/drm/i915/intel_guc_fwif.h
  create mode 100644 drivers/gpu/drm/i915/intel_guc_loader.c
 
 -- 
 1.9.1
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/13 v4] drm/i915: Debugfs interface to read GuC load status

[O'Rourke, Tom]

On Thu, Jul 09, 2015 at 07:29:06PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 The new node provides access to the status of the GuC-specific loader;
 also the scratch registers used for communication between the i915
 driver and the GuC firmware.
 
 v2:
 Changes to output formats per Chris Wilson's suggestions
 
 v4:
 Rebased
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com

  drivers/gpu/drm/i915/i915_debugfs.c | 39 
 +
  1 file changed, 39 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 98fd3c9..9ff5f17 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -2359,6 +2359,44 @@ static int i915_llc(struct seq_file *m, void *data)
   return 0;
  }
  
 +static int i915_guc_load_status_info(struct seq_file *m, void *data)
 +{
 + struct drm_info_node *node = m-private;
 + struct drm_i915_private *dev_priv = node-minor-dev-dev_private;
 + struct intel_guc_fw *guc_fw = dev_priv-guc.guc_fw;
 + u32 tmp, i;
 +
 + if (!HAS_GUC_UCODE(dev_priv-dev))
 + return 0;
 +
 + seq_printf(m, GuC firmware status:\n);
 + seq_printf(m, \tpath: %s\n,
 + guc_fw-guc_fw_path);
 + seq_printf(m, \tfetch: %s\n,
 + intel_guc_fw_status_repr(guc_fw-guc_fw_fetch_status));
 + seq_printf(m, \tload: %s\n,
 + intel_guc_fw_status_repr(guc_fw-guc_fw_load_status));
 + seq_printf(m, \tversion wanted: %d.%d\n,
 + guc_fw-guc_fw_major_wanted, guc_fw-guc_fw_minor_wanted);
 + seq_printf(m, \tversion found: %d.%d\n,
 + guc_fw-guc_fw_major_found, guc_fw-guc_fw_minor_found);
 +
 + tmp = I915_READ(GUC_STATUS);
 +
 + seq_printf(m, \nGuC status 0x%08x:\n, tmp);
 + seq_printf(m, \tBootrom status = 0x%x\n,
 + (tmp  GS_BOOTROM_MASK)  GS_BOOTROM_SHIFT);
 + seq_printf(m, \tuKernel status = 0x%x\n,
 + (tmp  GS_UKERNEL_MASK)  GS_UKERNEL_SHIFT);
 + seq_printf(m, \tMIA Core status = 0x%x\n,
 + (tmp  GS_MIA_MASK)  GS_MIA_SHIFT);
 + seq_puts(m, \nScratch registers:\n);
 + for (i = 0; i  16; i++)
 + seq_printf(m, \t%2d: \t0x%x\n, i, I915_READ(SOFT_SCRATCH(i)));
 +
 + return 0;
 +}
 +
  static int i915_edp_psr_status(struct seq_file *m, void *data)
  {
   struct drm_info_node *node = m-private;
 @@ -5073,6 +5111,7 @@ static const struct drm_info_list i915_debugfs_list[] = 
 {
   {i915_gem_hws_bsd, i915_hws_info, 0, (void *)VCS},
   {i915_gem_hws_vebox, i915_hws_info, 0, (void *)VECS},
   {i915_gem_batch_pool, i915_gem_batch_pool_info, 0},
 + {i915_guc_load_status, i915_guc_load_status_info, 0},
   {i915_frequency_info, i915_frequency_info, 0},
   {i915_hangcheck_info, i915_hangcheck_info, 0},
   {i915_drpc_info, i915_drpc_info, 0},
 -- 
 1.9.1
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/15 v3] Batch submission via GuC

[O'Rourke, Tom]

On Mon, Jul 06, 2015 at 04:29:47PM +0200, Daniel Vetter wrote:
 On Fri, Jul 03, 2015 at 01:30:22PM +0100, Dave Gordon wrote:
  This patch series enables command submission via the GuC. In this mode,
[TOR:] ...
created the original versions on which some of these patches are based.
 
 A few minor things still (I guess you expected my comment on the firmware
 loader) but looks good overall with those addressed and the detailed
 review done on top.
 -Daniel
 
[TOR:] This patch series has conflicts with Mika's recently
merged Convert parts of intel_lrc.c to requests series.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/17 v2] drm/i915: Add GuC-related header files

[O'Rourke, Tom]

On Thu, Jun 25, 2015 at 03:40:00PM +0100, Dave Gordon wrote:
 intel_guc_fwif.h contains the subset of the GuC interface that we
 will need for submission of commands through the GuC. These MUST
 be kept in sync with the definitions used by the GuC firmware, and
 updates to this file will (or should) be autogenerated from the
 source files used to build the firmware. Editing this file is
 therefore not recommended.

[TOR:] That recommendation should be repeated in the file itself.

 
 i915_guc_reg.h contains definitions of GuC-related hardware:
 registers, bitmasks, etc. These should match the BSpec.
 
 v2:
 Files renamed  resliced per review comments by Chris Wilson
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/5] drm/i915/skl: Retrieve the Rpe value from Pcode

[O'Rourke, Tom]

  +
  +   dev_priv-rps.efficient_freq *=
  +   (IS_SKYLAKE(dev) ? GEN9_FREQ_SCALER : 1);

This line seems awkward.  I suppose a good compiler could
optimize out the multiply by one.

I would prefer something like:

if(IS_SKYLAKE(dev))
dev_priv-rps.efficient_freq *= GEN9_FREQ_SCALER;

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

Re: [Intel-gfx] i915: WARN_ON(val dev_priv-rps.max_freq_softlimit)

[O'Rourke, Tom]

On Wed, Feb 11, 2015 at 08:30:44AM +0100, Daniel Vetter wrote:
 On Tue, Feb 10, 2015 at 10:26:02PM -0800, O'Rourke, Tom wrote:
  On Thu, Jan 29, 2015 at 08:56:06PM -0500, Michael Auchter wrote:
   On Thu, Jan 29, 2015 at 06:12:31PM +0100, Daniel Vetter wrote:
On Wed, Jan 28, 2015 at 10:36:02PM -0800, O'Rourke, Tom wrote:
 On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote:
  On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote:
   On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote:
Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) 
resulted in
this WARN at boot (and pretty frequently afterwards):

WARNING: CPU: 0 PID: 989 at 
drivers/gpu/drm/i915/intel_pm.c:4377 gen6_set_rps+0x371/0x3c0()
WARN_ON(val  dev_priv-rps.max_freq_softlimit)
   
   [snip]

I'm not at all familiar with this hardware, but I took a quick 
look into
what changed with this commit for my laptop. Before the commit,
rps.min_freq_softlimit is 4 (from rps.min_freq) and
rps.max_freq_softlimit is 22.

After the commit, rps.min_freq_softlimit is set to the
rps.efficient_freq value read from pcode, which is 34 on my 
laptop.
So later when gen6_set_rps() is called with 
rps.min_freq_softlimit that
warning is hit.

Any thoughts? It certainly seems fishy that this commit causes
rps.min_freq_softlimit to be greater than 
rps.max_freq_softlimit.
   
   Very fishy indeed. Moral of this story, never trust hw.
   
   diff --git a/drivers/gpu/drm/i915/intel_pm.c 
   b/drivers/gpu/drm/i915/intel_pm.c
   index 3e630feb18e4..bbedd2901c54 100644
   --- a/drivers/gpu/drm/i915/intel_pm.c
   +++ b/drivers/gpu/drm/i915/intel_pm.c
   @@ -4007,7 +4007,10 @@ static void 
   gen6_init_rps_frequencies(struct drm_device *dev)
   ddcc_status);
   if (0 == ret)
   dev_priv-rps.efficient_freq =
   -   (ddcc_status  8)  0xff;
   +   clamp_t(u8,
   +   (ddcc_status  8)  0xff,
   +   dev_priv-rps.min_freq,
   +   dev_priv-rps.max_freq);
  
  Maybe better to fall back to rp1_freq if this is bogus?
 
 [TOR:] Michael, Thank you for bringing this problem to our attention.
 
 Yes, this function needs some range checking to maintain
 RPn = RPe = RP0.
 
 A value of 34 seems too high for RPe.  
 What values does the Carbon X1 (Haswell) have for RPn and RP0?

4  22, already in Micheal's original bug report.

Tom, can you pls polish the clamping into a proper patch with m-l
references?

Micheal, can you please test the first hunk from Chris (the one that 
adds
the clamp) to make sure it does indeed address the WARN_ON you're 
seeing?
   
   The clamp suggested by Chris does indeed fix the WARN_ON.
   
   In the case where RPe is greater than RP0, RPe will now be clamped to
   RP0. Is this likely to result in increased power consumption?
   
   At a quick glance on my laptop it does not (idling around 5W before and
   after) but Ville had suggested earlier to fall back to RP1, which would
   be more consistent with previous kernels.
   
   Thanks again for the quick responses,
Michael
  
  [TOR:] Michael,  I discussed this report with a pcode architect here.
  
  The RPe value is clamped to the [RPn, RP0] range by pcode before
  returning the value to the driver on Broadwell but not on Haswell.
  
  On Haswell, an efficient frequency value above RP0 is not a garbage
  value and could result from a relatively flat efficiency curve.  In
  this situation, leakage power would dominate the efficiency curve
  such that running at lower frequencies may not save power overall.
  Higher leakage power may result from a higher package temperature.
  
  Running at RP0 may actually save power compared to RP1 by allowing
  more time in RC6.  
 
 Hm, I'd just go with clamping since that's what bdw does too. So Chris
 diff essentially.
 -Daniel

[TOR:] Yes, we should go with clamping, essentially the diff from Chris.
I am trying to provide an explanation of why that is the right thing to do.

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

Re: [Intel-gfx] i915: WARN_ON(val dev_priv-rps.max_freq_softlimit)

[O'Rourke, Tom]

On Thu, Jan 29, 2015 at 08:56:06PM -0500, Michael Auchter wrote:
 On Thu, Jan 29, 2015 at 06:12:31PM +0100, Daniel Vetter wrote:
  On Wed, Jan 28, 2015 at 10:36:02PM -0800, O'Rourke, Tom wrote:
   On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote:
On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote:
 On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote:
  Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) 
  resulted in
  this WARN at boot (and pretty frequently afterwards):
  
  WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 
  gen6_set_rps+0x371/0x3c0()
  WARN_ON(val  dev_priv-rps.max_freq_softlimit)
 
 [snip]
  
  I'm not at all familiar with this hardware, but I took a quick look 
  into
  what changed with this commit for my laptop. Before the commit,
  rps.min_freq_softlimit is 4 (from rps.min_freq) and
  rps.max_freq_softlimit is 22.
  
  After the commit, rps.min_freq_softlimit is set to the
  rps.efficient_freq value read from pcode, which is 34 on my laptop.
  So later when gen6_set_rps() is called with rps.min_freq_softlimit 
  that
  warning is hit.
  
  Any thoughts? It certainly seems fishy that this commit causes
  rps.min_freq_softlimit to be greater than rps.max_freq_softlimit.
 
 Very fishy indeed. Moral of this story, never trust hw.
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c 
 b/drivers/gpu/drm/i915/intel_pm.c
 index 3e630feb18e4..bbedd2901c54 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct 
 drm_device *dev)
 ddcc_status);
 if (0 == ret)
 dev_priv-rps.efficient_freq =
 -   (ddcc_status  8)  0xff;
 +   clamp_t(u8,
 +   (ddcc_status  8)  0xff,
 +   dev_priv-rps.min_freq,
 +   dev_priv-rps.max_freq);

Maybe better to fall back to rp1_freq if this is bogus?
   
   [TOR:] Michael, Thank you for bringing this problem to our attention.
   
   Yes, this function needs some range checking to maintain
   RPn = RPe = RP0.
   
   A value of 34 seems too high for RPe.  
   What values does the Carbon X1 (Haswell) have for RPn and RP0?
  
  4  22, already in Micheal's original bug report.
  
  Tom, can you pls polish the clamping into a proper patch with m-l
  references?
  
  Micheal, can you please test the first hunk from Chris (the one that adds
  the clamp) to make sure it does indeed address the WARN_ON you're seeing?
 
 The clamp suggested by Chris does indeed fix the WARN_ON.
 
 In the case where RPe is greater than RP0, RPe will now be clamped to
 RP0. Is this likely to result in increased power consumption?
 
 At a quick glance on my laptop it does not (idling around 5W before and
 after) but Ville had suggested earlier to fall back to RP1, which would
 be more consistent with previous kernels.
 
 Thanks again for the quick responses,
  Michael

[TOR:] Michael,  I discussed this report with a pcode architect here.

The RPe value is clamped to the [RPn, RP0] range by pcode before
returning the value to the driver on Broadwell but not on Haswell.

On Haswell, an efficient frequency value above RP0 is not a garbage
value and could result from a relatively flat efficiency curve.  In
this situation, leakage power would dominate the efficiency curve
such that running at lower frequencies may not save power overall.
Higher leakage power may result from a higher package temperature.

Running at RP0 may actually save power compared to RP1 by allowing
more time in RC6.  

Thanks,
Tom O'Rourke
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] i915: WARN_ON(val dev_priv-rps.max_freq_softlimit)

[O'Rourke, Tom]

On Thu, Jan 29, 2015 at 08:56:06PM -0500, Michael Auchter wrote:
 On Thu, Jan 29, 2015 at 06:12:31PM +0100, Daniel Vetter wrote:
  On Wed, Jan 28, 2015 at 10:36:02PM -0800, O'Rourke, Tom wrote:
   On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote:
On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote:
 On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote:
  Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) 
  resulted in
  this WARN at boot (and pretty frequently afterwards):
  
  WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 
  gen6_set_rps+0x371/0x3c0()
  WARN_ON(val  dev_priv-rps.max_freq_softlimit)
 
 [snip]
  
  I'm not at all familiar with this hardware, but I took a quick look 
  into
  what changed with this commit for my laptop. Before the commit,
  rps.min_freq_softlimit is 4 (from rps.min_freq) and
  rps.max_freq_softlimit is 22.
  
  After the commit, rps.min_freq_softlimit is set to the
  rps.efficient_freq value read from pcode, which is 34 on my laptop.
  So later when gen6_set_rps() is called with rps.min_freq_softlimit 
  that
  warning is hit.
  
  Any thoughts? It certainly seems fishy that this commit causes
  rps.min_freq_softlimit to be greater than rps.max_freq_softlimit.
 
 Very fishy indeed. Moral of this story, never trust hw.
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c 
 b/drivers/gpu/drm/i915/intel_pm.c
 index 3e630feb18e4..bbedd2901c54 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct 
 drm_device *dev)
 ddcc_status);
 if (0 == ret)
 dev_priv-rps.efficient_freq =
 -   (ddcc_status  8)  0xff;
 +   clamp_t(u8,
 +   (ddcc_status  8)  0xff,
 +   dev_priv-rps.min_freq,
 +   dev_priv-rps.max_freq);

Maybe better to fall back to rp1_freq if this is bogus?
   
   [TOR:] Michael, Thank you for bringing this problem to our attention.
   
   Yes, this function needs some range checking to maintain
   RPn = RPe = RP0.
   
   A value of 34 seems too high for RPe.  
   What values does the Carbon X1 (Haswell) have for RPn and RP0?
  
  4  22, already in Micheal's original bug report.
  
  Tom, can you pls polish the clamping into a proper patch with m-l
  references?
[TOR:] Yes, I will submit a polished patch with proper range checking.

  
  Micheal, can you please test the first hunk from Chris (the one that adds
  the clamp) to make sure it does indeed address the WARN_ON you're seeing?
 
 The clamp suggested by Chris does indeed fix the WARN_ON.
 
 In the case where RPe is greater than RP0, RPe will now be clamped to
 RP0. Is this likely to result in increased power consumption? 
 
 At a quick glance on my laptop it does not (idling around 5W before and
 after) but Ville had suggested earlier to fall back to RP1, which would
 be more consistent with previous kernels.
 
 Thanks again for the quick responses,
  Michael
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] i915: WARN_ON(val dev_priv-rps.max_freq_softlimit)

[O'Rourke, Tom]

On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote:
 On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote:
  On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote:
   Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted in
   this WARN at boot (and pretty frequently afterwards):
   
   WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 
   gen6_set_rps+0x371/0x3c0()
   WARN_ON(val  dev_priv-rps.max_freq_softlimit)
  
  [snip]
   
   I'm not at all familiar with this hardware, but I took a quick look into
   what changed with this commit for my laptop. Before the commit,
   rps.min_freq_softlimit is 4 (from rps.min_freq) and
   rps.max_freq_softlimit is 22.
   
   After the commit, rps.min_freq_softlimit is set to the
   rps.efficient_freq value read from pcode, which is 34 on my laptop.
   So later when gen6_set_rps() is called with rps.min_freq_softlimit that
   warning is hit.
   
   Any thoughts? It certainly seems fishy that this commit causes
   rps.min_freq_softlimit to be greater than rps.max_freq_softlimit.
  
  Very fishy indeed. Moral of this story, never trust hw.
  
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 3e630feb18e4..bbedd2901c54 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct 
  drm_device *dev)
  ddcc_status);
  if (0 == ret)
  dev_priv-rps.efficient_freq =
  -   (ddcc_status  8)  0xff;
  +   clamp_t(u8,
  +   (ddcc_status  8)  0xff,
  +   dev_priv-rps.min_freq,
  +   dev_priv-rps.max_freq);
 
 Maybe better to fall back to rp1_freq if this is bogus?

[TOR:] Michael, Thank you for bringing this problem to our attention.

Yes, this function needs some range checking to maintain
RPn = RPe = RP0.

A value of 34 seems too high for RPe.  
What values does the Carbon X1 (Haswell) have for RPn and RP0?

I agree with Ville that a bogus value from the pcode read should 
not be used.  Simple clamping would set the min_freq to RP0; 
probably incorrect.

Tom O'Rourke
 
  }
   
  /* Preserve min/max settings in case of re-init */
  
  But really it is probably just best to disable the query for hsw:
  
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 3e630feb18e4..01bd508e81f6 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -4001,7 +4001,7 @@ static void gen6_init_rps_frequencies(struct 
  drm_device *dev)
  dev_priv-rps.max_freq  = dev_priv-rps.rp0_freq;
   
  dev_priv-rps.efficient_freq = dev_priv-rps.rp1_freq;
  -   if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
  +   if (IS_BROADWELL(dev)) {
  ret = sandybridge_pcode_read(dev_priv,
  
  HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
  ddcc_status);
  
  Paranoia says we do both.
  -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
 
 -- 
 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 2/4] drm/i915: Add gt_act_freq_mhz sysfs file

[O'Rourke, Tom]

On Sun, Jan 25, 2015 at 09:34:33AM +, Chris Wilson wrote:
 On Fri, Jan 23, 2015 at 09:04:24PM +0200, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  Currently the 'gt_cur_freq_mhz' file shows the actual GPU frequency on
  VLV/CHV, and the last requested frequency on other platforms. Change the
  meaning of the file on VLV/CHV to follow the the other platforms, and
  introduce a new file 'gt_act_freq_mhz' which shows the actual frequency
  on all platforms.
  
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
 
   drivers/gpu/drm/i915/i915_sysfs.c | 35 ++-
   1 file changed, 34 insertions(+), 1 deletion(-)
  
  +static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
  +   struct device_attribute *attr, char *buf)
  +{
  +   struct drm_minor *minor = dev_to_drm_minor(kdev);
  +   struct drm_device *dev = minor-dev;
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +   int ret;
  +
  +   flush_delayed_work(dev_priv-rps.delayed_resume_work);
 
 Is this required for querying the current value?
 
 Though probably better to keep it similar to the others.
 -Chris

[TOR:] flush_delayed_work is needed to make sure rps.cur_freq
is valid for non VLV/CHV platforms.

Tom O'Rourke

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

Re: [Intel-gfx] [PATCH] [v2] intel_frequency: A tool to manipulate Intel GPU frequency

[O'Rourke, Tom]

On Tue, Jan 13, 2015 at 02:36:56PM -0800, Ben Widawsky wrote:
 On Tue, Jan 13, 2015 at 09:19:04PM +, O'Rourke, Tom wrote:
  Sent: Sunday, January 11, 2015 7:48 PM
  To: Widawsky, Benjamin
  Cc: Intel GFX
  Subject: Re: [Intel-gfx] [PATCH] [v2] intel_frequency: A tool to 
  manipulate Intel
  GPU frequency
  
  On Sun, Jan 11, 2015 at 07:35:21PM -0800, Ben Widawsky wrote:
   WARNING: very minimally tested
  
   In general you should not need this tool. It's primary purpose is for
   benchmarking, and for debugging performance issues.
  
  I noticed the it's vs its on v1, but forgot to fix it. IT'S fixed 
  locally
  though.
  
  
   For many kernel releases now sysfs has supported reading and writing the 
   GPU
   frequency. Therefore, this tool provides no new functionality. What it 
   does
   provide is an easy to package (for distros) tool that handles the most 
   common
   scenarios.
  [TOR:] This is a nice tool.  
  I am concerned that this tool may be confusing RP1 frequency with RPe 
  (Efficient) frequency.  On many platforms, these are not the same thing. 
  Thanks,
  Tom O'Rourke
  
 
 Any platform other than BYT/CHV?

[TOR:] I am thinking about Haswell and Broadwell.  The RP1
value can be read from RP_STATE_CAP while the RPe value can
be read from PCU mailbox.  Do we need to add a sysfs entry
for RPe?

  +  case 'e': /* efficient */
  +  if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)) {
  +  /* the LP parts have special efficient  
  frequencies
 */
  +  fprintf(stderr,
  +  FIXME: Warning efficient frequency 
  information is incorrect.\n);
  +  exit(EXIT_FAILURE);
  +  }
 
  
   v2:
   Get rid of -f from the usage message (Jordan)
   Add space before [-s (Jordan)
   Add a -c/--custom example (Jordan)
   Add a setting for resetting to hardware default (Ken)
   Replicate examples in commit message in the source code. (me)
  
   Signed-off-by: Ben Widawsky b...@bwidawsk.net
   Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
   Cc: Kenneth Graunke kenn...@whitecape.org
  
   Here are some sample usages:
   $ sudo intel_frequency --get=cur,min,max,eff
   cur: 200 MHz
   min: 200 MHz
   RP1: 200 MHz
   max: 1200 MHz
  
   $ sudo intel_frequency -g
   cur: 200 MHz
   min: 200 MHz
   RP1: 200 MHz
   max: 1200 MHz
  
   $ sudo intel_frequency -geff
   RP1: 200 MHz
  
   $ sudo intel_frequency --set min=300
   $ sudo intel_frequency --get min
   cur: 300 MHz
   min: 300 MHz
   RP1: 200 MHz
   max: 1200 MHz
  
   $ sudo intel_frequency --custom max=900
   $ sudo intel_frequency --get max
   cur: 300 MHz
   min: 300 MHz
   RP1: 200 MHz
   max: 900 MHz
  
   $ sudo intel_frequency --max
   $ sudo intel_frequency -g
   cur: 1200 MHz
   min: 1200 MHz
   RP1: 200 MHz
   max: 1200 MHz
  
   $ sudo intel_frequency -e
   $ sudo intel_frequency -g
   cur: 200 MHz
   min: 200 MHz
   RP1: 200 MHz
   max: 200 MHz
  
   $ sudo intel_frequency --max
   $ sudo intel_frequency -g
   cur: 1200 MHz
   min: 1200 MHz
   RP1: 200 MHz
   max: 1200 MHz
  
   $ sudo intel_frequency --min
   $ sudo intel_frequency -g
   cur: 200 MHz
   min: 200 MHz
   RP1: 200 MHz
   max: 200 MHz
   ---
tools/Makefile.sources  |   1 +
tools/intel_frequency.c | 363
  
2 files changed, 364 insertions(+)
create mode 100644 tools/intel_frequency.c
  
   diff --git a/tools/Makefile.sources b/tools/Makefile.sources
   index b85a6b8..2bea389 100644
   --- a/tools/Makefile.sources
   +++ b/tools/Makefile.sources
   @@ -14,6 +14,7 @@ bin_PROGRAMS = \
intel_dump_decode   \
intel_error_decode  \
intel_forcewaked\
   +intel_frequency \
intel_framebuffer_dump  \
intel_gpu_time  \
intel_gpu_top   \
   diff --git a/tools/intel_frequency.c b/tools/intel_frequency.c
   new file mode 100644
   index 000..59f3814
   --- /dev/null
   +++ b/tools/intel_frequency.c
   @@ -0,0 +1,363 @@
   +/*
   + * Copyright © 2015 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

Re: [Intel-gfx] [PATCH] [v2] intel_frequency: A tool to manipulate Intel GPU frequency

[O'Rourke, Tom]

Sent: Sunday, January 11, 2015 7:48 PM
To: Widawsky, Benjamin
Cc: Intel GFX
Subject: Re: [Intel-gfx] [PATCH] [v2] intel_frequency: A tool to manipulate 
Intel
GPU frequency

On Sun, Jan 11, 2015 at 07:35:21PM -0800, Ben Widawsky wrote:
 WARNING: very minimally tested

 In general you should not need this tool. It's primary purpose is for
 benchmarking, and for debugging performance issues.

I noticed the it's vs its on v1, but forgot to fix it. IT'S fixed locally
though.


 For many kernel releases now sysfs has supported reading and writing the GPU
 frequency. Therefore, this tool provides no new functionality. What it does
 provide is an easy to package (for distros) tool that handles the most common
 scenarios.
[TOR:] This is a nice tool.  
I am concerned that this tool may be confusing RP1 frequency with RPe 
(Efficient) frequency.  On many platforms, these are not the same thing. 
Thanks,
Tom O'Rourke


 v2:
 Get rid of -f from the usage message (Jordan)
 Add space before [-s (Jordan)
 Add a -c/--custom example (Jordan)
 Add a setting for resetting to hardware default (Ken)
 Replicate examples in commit message in the source code. (me)

 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
 Cc: Kenneth Graunke kenn...@whitecape.org

 Here are some sample usages:
 $ sudo intel_frequency --get=cur,min,max,eff
 cur: 200 MHz
 min: 200 MHz
 RP1: 200 MHz
 max: 1200 MHz

 $ sudo intel_frequency -g
 cur: 200 MHz
 min: 200 MHz
 RP1: 200 MHz
 max: 1200 MHz

 $ sudo intel_frequency -geff
 RP1: 200 MHz

 $ sudo intel_frequency --set min=300
 $ sudo intel_frequency --get min
 cur: 300 MHz
 min: 300 MHz
 RP1: 200 MHz
 max: 1200 MHz

 $ sudo intel_frequency --custom max=900
 $ sudo intel_frequency --get max
 cur: 300 MHz
 min: 300 MHz
 RP1: 200 MHz
 max: 900 MHz

 $ sudo intel_frequency --max
 $ sudo intel_frequency -g
 cur: 1200 MHz
 min: 1200 MHz
 RP1: 200 MHz
 max: 1200 MHz

 $ sudo intel_frequency -e
 $ sudo intel_frequency -g
 cur: 200 MHz
 min: 200 MHz
 RP1: 200 MHz
 max: 200 MHz

 $ sudo intel_frequency --max
 $ sudo intel_frequency -g
 cur: 1200 MHz
 min: 1200 MHz
 RP1: 200 MHz
 max: 1200 MHz

 $ sudo intel_frequency --min
 $ sudo intel_frequency -g
 cur: 200 MHz
 min: 200 MHz
 RP1: 200 MHz
 max: 200 MHz
 ---
  tools/Makefile.sources  |   1 +
  tools/intel_frequency.c | 363

  2 files changed, 364 insertions(+)
  create mode 100644 tools/intel_frequency.c

 diff --git a/tools/Makefile.sources b/tools/Makefile.sources
 index b85a6b8..2bea389 100644
 --- a/tools/Makefile.sources
 +++ b/tools/Makefile.sources
 @@ -14,6 +14,7 @@ bin_PROGRAMS = \
  intel_dump_decode   \
  intel_error_decode  \
  intel_forcewaked\
 +intel_frequency \
  intel_framebuffer_dump  \
  intel_gpu_time  \
  intel_gpu_top   \
 diff --git a/tools/intel_frequency.c b/tools/intel_frequency.c
 new file mode 100644
 index 000..59f3814
 --- /dev/null
 +++ b/tools/intel_frequency.c
 @@ -0,0 +1,363 @@
 +/*
 + * Copyright © 2015 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.
 + *
 + * Example:
 + * Get all frequencies:
 + * intel_frequency --get=cur,min,max,eff
 + *
 + * Same as above:
 + * intel_frequency -g
 + *
 + * Get the efficient frequency:
 + * intel_frequency -geff
 + *
 + * Lock the GPU frequency to 300MHz:
 + * intel_frequency --set min=300
 + *
 + * Set the maximum frequency to 900MHz:
 + * intel_frequency --custom max=900
 + *
 + * Lock the GPU frequency to its maximum frequency:
 + * intel_frequency --max
 + *
 + * Lock the GPU frequency to its most efficient frequency:
 + * intel_frequency -e
 + *
 + * Lock The GPU frequency to its minimum frequency:
 + * intel_frequency --min
 + *
 + * 

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Use efficient frequency for HSW/BDW

[O'Rourke, Tom]

On Fri, Nov 07, 2014 at 10:50:02AM +0100, Daniel Vetter wrote:
 On Wed, Nov 05, 2014 at 05:31:34PM -0800, Tom.O'rou...@intel.com wrote:
  From: Tom O'Rourke Tom.O'rou...@intel.com
  
  Updated gen6|8_enable_rps() for Haswell and Broadwell
  to use the efficient frequency read from pcode.
  
  Added hsw_use_efficient_freq() to read efficient
  frequency (aka RPe) from pcode.  The efficiency is
  based on the frequency/power ratio (MHz/W); this is
  considering GT power and not package power.  The
  efficent frequency is the highest frequency for which
  the frequency/power ratio is within some threshold of
  the highest frequency/power ratio.
  
  Also set the min_freq_softlimit to the efficient
  frequency.  A fixed decrease in frequency results in
  smaller decrease in power at frequencies less than RPe
  than at frequencies above RPe.
  
  Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com
  ---
   drivers/gpu/drm/i915/i915_reg.h |1 +
   drivers/gpu/drm/i915/intel_pm.c |   22 ++
   2 files changed, 23 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index d43fa0e..6fbfdec 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -6010,6 +6010,7 @@ enum punit_power_well {
   #define   GEN6_ENCODE_RC6_VID(mv)  (((mv) - 245) / 5)
   #define   GEN6_DECODE_RC6_VID(vids)(((vids) * 5) + 245)
   #define   DISPLAY_IPS_CONTROL  0x19
  +#define  HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL  0x1A
   #define GEN6_PCODE_DATA0x138128
   #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT   8
   #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 300d7e5..e4347d9 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -4706,6 +4706,18 @@ static void parse_rp_state_cap(struct 
  drm_i915_private *dev_priv, u32 rp_state_c
  dev_priv-rps.min_freq_softlimit = dev_priv-rps.min_freq;
   }
   
  +static void hsw_use_efficient_freq(struct drm_i915_private *dev_priv)
  +{
  +   u32 ddcc_status = 0;
  +   int ret;
  +
  +   ret = sandybridge_pcode_read(dev_priv,
  +   HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
  +   ddcc_status);
  +   if (0 == ret)
  +   dev_priv-rps.efficient_freq = (ddcc_status  8)  0xff;
 
 Control flow in the error case looks funny here - shouldn't we put the
 adjustment of the softlimit into the (ret == 0) case here to avoid putting
 garbage into it in case the pcode read falls over?
 
If the pcode read fails, the efficient_freq will not 
be set here and will still have the non-garbage value 
set in parse_rp_state_cap.

 That would make the function name match reality a bit more too, since atm
 it just reads the efficient_freq but doesn't use it. So as-is a bit
 misnamed.
 -Daniel

I agree the function is misnamed as-is.  If there are 
no objections, I will rewrite the patch to follow 
Daniel's recommendation and move adjusting the 
softlimit to hsw_use_efficient_freq.
 
  +}
  +
   static void gen9_enable_rps(struct drm_device *dev)
   {
  struct drm_i915_private *dev_priv = dev-dev_private;
  @@ -4765,6 +4777,11 @@ static void gen8_enable_rps(struct drm_device *dev)
  rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
  parse_rp_state_cap(dev_priv, rp_state_cap);
   
  +   if (IS_BROADWELL(dev)) {
  +   hsw_use_efficient_freq(dev_priv);
  +   dev_priv-rps.min_freq_softlimit = dev_priv-rps.efficient_freq;
  +   }
  +
  /* 2b: Program RC6 thresholds.*/
  I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40  16);
  I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); /* 12500 * 1280ns */
  @@ -4860,6 +4877,11 @@ static void gen6_enable_rps(struct drm_device *dev)
   
  parse_rp_state_cap(dev_priv, rp_state_cap);
   
  +   if (IS_HASWELL(dev)) {
  +   hsw_use_efficient_freq(dev_priv);
  +   dev_priv-rps.min_freq_softlimit = dev_priv-rps.efficient_freq;
  +   }
  +
  /* disable the counters and set deterministic thresholds */
  I915_WRITE(GEN6_RC_CONTROL, 0);
   
  -- 
  1.7.9.5
  
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 -- 
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Use efficient frequency for HSW/BDW

[O'Rourke, Tom]

On Fri, Nov 07, 2014 at 10:41:26AM +, Chris Wilson wrote:
 On Wed, Nov 05, 2014 at 05:31:34PM -0800, Tom.O'rou...@intel.com wrote:
  From: Tom O'Rourke Tom.O'rou...@intel.com
  
  Updated gen6|8_enable_rps() for Haswell and Broadwell
  to use the efficient frequency read from pcode.
  
  Added hsw_use_efficient_freq() to read efficient
  frequency (aka RPe) from pcode.  The efficiency is
  based on the frequency/power ratio (MHz/W); this is
  considering GT power and not package power.  The
  efficent frequency is the highest frequency for which
  the frequency/power ratio is within some threshold of
  the highest frequency/power ratio.
  
  Also set the min_freq_softlimit to the efficient
  frequency.  A fixed decrease in frequency results in
  smaller decrease in power at frequencies less than RPe
  than at frequencies above RPe.
  
  Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com
  ---
   drivers/gpu/drm/i915/i915_reg.h |1 +
   drivers/gpu/drm/i915/intel_pm.c |   22 ++
   2 files changed, 23 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index d43fa0e..6fbfdec 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -6010,6 +6010,7 @@ enum punit_power_well {
   #define   GEN6_ENCODE_RC6_VID(mv)  (((mv) - 245) / 5)
   #define   GEN6_DECODE_RC6_VID(vids)(((vids) * 5) + 245)
   #define   DISPLAY_IPS_CONTROL  0x19
  +#define  HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL  0x1A
   #define GEN6_PCODE_DATA0x138128
   #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT   8
   #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 300d7e5..e4347d9 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -4706,6 +4706,18 @@ static void parse_rp_state_cap(struct 
  drm_i915_private *dev_priv, u32 rp_state_c
  dev_priv-rps.min_freq_softlimit = dev_priv-rps.min_freq;
   }
   
  +static void hsw_use_efficient_freq(struct drm_i915_private *dev_priv)
 
 use_efficient_freq() ?
 
 Shouldn't this be:
 
 parse_rp_state_cap()
   {
   ...
   dev_priv-rps.efficient_freq = rps_rpe_freq(dev_priv);
   ...
   }
 
The parse_rp_state_cap() function is for parsing the 
RP_STATE_CAP register.  The pcode read to get the 
efficient frequency seems out of scope.

 
 static in rps_rpe_freq(dev_priv) 
   {
   /* By default, prefer the nominal frequency for efficiency */
   int rpe = dev_priv-rps.rp1_freq;
 
   if (INTEL_GEN(dev_priv) = 8 || IS_HASWELL(dev_priv)) {
   u32 ddcc_status = 0;
   int ret;
 
   /* Some useful and informative comment */
   if (sandybridge_pcode_read(dev_priv,
  HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
  ddcc_status) == 0)
   rpe = (ddcc_status  8)  0xff;
   }
 
   return rpe;
   }
 
 -- 
 Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Add haswell_pcode_write function

[O'Rourke, Tom]

On Thu, Nov 06, 2014 at 10:28:53AM +0200, Ville Syrjälä wrote:
 On Wed, Nov 05, 2014 at 05:32:44PM -0800, Tom.O'rou...@intel.com wrote:
  From: Tom O'Rourke Tom.O'rou...@intel.com
  
  Based on sandybridge_pcode_write, haswell_pcode_write has an
  additional field for address control.
 
 It's already there in snb.
If the address control field is already there for SNB,
then I would prefer to modify sandybridge_pcode_write
instead of adding a haswell_pcode_write.

I based this patch on the Haswell documentation for
PCU_CR_GTDRIVER_MAILBOX_INTERFACE_0_2_0_GTTMMADR on
page 101 of
https://01.org/linuxgraphics/sites/default/files/documentation/intel-gfx-prm-osrc-hsw-pcie-config-registers.pdf

Can you point me to the SNB documentation for the
address control field?   I do not have documentation
for address control for earlier than Haswell.

 
 Do you have an actual use case for this? If so I wonder if we should
 just change the mbox parameter to u32 and allow the caller to specify
 it all?
 
No, I do not have an actual use case for this yet.
I am working on something that will use it but that
is not ready yet.

Yes, we could change mbox to u32 and shift the work
to the caller.  The caller would need to combine the
u8 mailbox command with the address control field.
I do not think that would be better (or worse) but
I can make that change if others want it.

  
  Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com
  ---
   drivers/gpu/drm/i915/i915_drv.h |1 +
   drivers/gpu/drm/i915/i915_reg.h |1 +
   drivers/gpu/drm/i915/intel_pm.c |9 +++--
   3 files changed, 9 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index 0f00e58..fd8b550 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -2950,6 +2950,7 @@ void gen6_gt_force_wake_put(struct drm_i915_private 
  *dev_priv, int fw_engine);
   void assert_force_wake_inactive(struct drm_i915_private *dev_priv);
   
   int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 
  *val);
  +int haswell_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 
  val, u32 control);
   int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, 
  u32 val);
   
   /* intel_sideband.c */
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index 6fbfdec..b674050 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -6011,6 +6011,7 @@ enum punit_power_well {
   #define   GEN6_DECODE_RC6_VID(vids)(((vids) * 5) + 245)
   #define   DISPLAY_IPS_CONTROL  0x19
   #define  HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL  0x1A
  +#define   HSW_PCODE_ADDR_CNTL(cntl)((cntl  8)  
  0x1f00)
   #define GEN6_PCODE_DATA0x138128
   #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT   8
   #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 1244ff8..9c47bc8 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -7277,7 +7277,7 @@ int sandybridge_pcode_read(struct drm_i915_private 
  *dev_priv, u8 mbox, u32 *val)
  return 0;
   }
   
  -int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, 
  u32 val)
  +int haswell_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 
  val, u32 control)
   {
  WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
   
  @@ -7287,7 +7287,7 @@ int sandybridge_pcode_write(struct drm_i915_private 
  *dev_priv, u8 mbox, u32 val)
  }
   
  I915_WRITE(GEN6_PCODE_DATA, val);
  -   I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
  +   I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox | 
  HSW_PCODE_ADDR_CNTL(control));
   
  if (wait_for((I915_READ(GEN6_PCODE_MAILBOX)  GEN6_PCODE_READY) == 0,
   500)) {
  @@ -7300,6 +7300,11 @@ int sandybridge_pcode_write(struct drm_i915_private 
  *dev_priv, u8 mbox, u32 val)
  return 0;
   }
   
  +int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, 
  u32 val)
  +{
  +   return haswell_pcode_write(dev_priv, mbox, val, 0);
  +}
  +
   static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
   {
  int div;
  -- 
  1.7.9.5
  
  ___
  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] drm/i915: Add haswell_pcode_write function

[O'Rourke, Tom]

On Thu, Nov 06, 2014 at 08:18:46PM +0200, Ville Syrjälä wrote:
 On Thu, Nov 06, 2014 at 09:35:48AM -0800, O'Rourke, Tom wrote:
  On Thu, Nov 06, 2014 at 10:28:53AM +0200, Ville Syrjälä wrote:
   On Wed, Nov 05, 2014 at 05:32:44PM -0800, Tom.O'rou...@intel.com wrote:
From: Tom O'Rourke Tom.O'rou...@intel.com

Based on sandybridge_pcode_write, haswell_pcode_write has an
additional field for address control.
   
   It's already there in snb.
  If the address control field is already there for SNB,
  then I would prefer to modify sandybridge_pcode_write
  instead of adding a haswell_pcode_write.
 
 Agreed.
 
  
  I based this patch on the Haswell documentation for
  PCU_CR_GTDRIVER_MAILBOX_INTERFACE_0_2_0_GTTMMADR on
  page 101 of
  https://01.org/linuxgraphics/sites/default/files/documentation/intel-gfx-prm-osrc-hsw-pcie-config-registers.pdf
  
  Can you point me to the SNB documentation for the
  address control field?   I do not have documentation
  for address control for earlier than Haswell.
 
 I found it in BSpec. Not sure it can be found in the PRMs.

Thank you for the pointer.  I will rewrite the patch.
This current patch should be abandoned.

  
   
   Do you have an actual use case for this? If so I wonder if we should
   just change the mbox parameter to u32 and allow the caller to specify
   it all?
   
  No, I do not have an actual use case for this yet.
  I am working on something that will use it but that
  is not ready yet.
  
  Yes, we could change mbox to u32 and shift the work
  to the caller.  The caller would need to combine the
  u8 mailbox command with the address control field.
  I do not think that would be better (or worse) but
  I can make that change if others want it.
 
 Well, I don't really mind either way. So unless anyone else objects to
 one of the approaches feel free to pick either one.
 
  

Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h |1 +
 drivers/gpu/drm/i915/i915_reg.h |1 +
 drivers/gpu/drm/i915/intel_pm.c |9 +++--
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h 
b/drivers/gpu/drm/i915/i915_drv.h
index 0f00e58..fd8b550 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2950,6 +2950,7 @@ void gen6_gt_force_wake_put(struct 
drm_i915_private *dev_priv, int fw_engine);
 void assert_force_wake_inactive(struct drm_i915_private *dev_priv);
 
 int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, 
u32 *val);
+int haswell_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, 
u32 val, u32 control);
 int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 
mbox, u32 val);
 
 /* intel_sideband.c */
diff --git a/drivers/gpu/drm/i915/i915_reg.h 
b/drivers/gpu/drm/i915/i915_reg.h
index 6fbfdec..b674050 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6011,6 +6011,7 @@ enum punit_power_well {
 #define   GEN6_DECODE_RC6_VID(vids)(((vids) * 5) + 245)
 #define   DISPLAY_IPS_CONTROL  0x19
 #define  HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL  0x1A
+#define   HSW_PCODE_ADDR_CNTL(cntl)((cntl  8)  
0x1f00)
 
 I don't think we need the mask here. We generally don't have it, with some
 exceptions like the csc stuff I did recently which deals in signed values,
 and so needs it to avoid the sign bits from clobbering everything above.
 
 But I have been dreaming occasionally about having such macros defined
 in a way that would allow us to verify that we don't overflow anything
 by accident. Eg. something like this:
 
 #if CONFIG_I915_CHECK_REGISTERS
 #define _BITFIELD(value,offset,size) ({ WARN_ON((value)  ~((1  (size)) - 
 1)); ((value)  (shift)) })
 #else
 #define _BITFIELD(value,offset,size) ((value)  (shift))
 #endif
 
 #define PCODE_ADDR_CNTL(x) _BITFIELD((x), 8, 21)
 
 But converting everything would be a big task, especially as we don't
 have such macros for everything. Sometimes we just have a shift value,
 and sometimes not even that and we calculate a shift using some case
 specific magic, and sometimes the shift isn't strictly needed (eg.
 it's 0, or just happens to match naturally the data we stuff in there
 like in the case of page aligned addresses).
 
I like the CONFIG_I915_CHECK_REGISTERS idea.  Until we
have something like that, I would like to keep the mask.
In addition to adding a little safety, the mask makes it
clear in the code how big the field should be.

 #define GEN6_PCODE_DATA0x138128
 #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT   8
 #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16
diff --git a/drivers/gpu/drm/i915/intel_pm.c 
b/drivers/gpu/drm/i915/intel_pm.c
index 1244ff8..9c47bc8 100644

Re: [Intel-gfx] [PATCH 03/10] drm/i915/chv: Use timeout mode for RC6 on chv

[O'Rourke, Tom]

On Tue, Nov 04, 2014 at 04:51:41AM -0800, Rodrigo Vivi wrote:
 From: Deepak S deepa...@linux.intel.com
 
 Higher RC6 residency is observed using timeout mode
 instead of EI mode. It's Recommended to use TO Method for RC6.
 
[TOR:] My comments on the previous version of this patch are at
http://lists.freedesktop.org/archives/intel-gfx/2014-August/050203.html

Based on the understanding this patch will provide benefit
on some pre-production CHV steppings and no benefit on the
production CHV steppings, this patch should not be merged.

 Signed-off-by: Deepak S deepa...@linux.intel.com
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 7a69eba..0bfcd83 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -4348,7 +4348,7 @@ static void cherryview_enable_rps(struct drm_device 
 *dev)
   I915_WRITE(RING_MAX_IDLE(ring-mmio_base), 10);
   I915_WRITE(GEN6_RC_SLEEP, 0);
  
 - I915_WRITE(GEN6_RC6_THRESHOLD, 5); /* 50/125ms per EI */
 + I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
  
   /* allows RC6 residency counter to work */
   I915_WRITE(VLV_COUNTER_CONTROL,
 @@ -4364,7 +4364,7 @@ static void cherryview_enable_rps(struct drm_device 
 *dev)
   /* 3: Enable RC6 */
   if ((intel_enable_rc6(dev)  INTEL_RC6_ENABLE) 
   (pcbr  VLV_PCBR_ADDR_SHIFT))
 - rc6_mode = GEN6_RC_CTL_EI_MODE(1);
 + rc6_mode = GEN7_RC_CTL_TO_MODE;
  
   I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
  
 -- 
 1.9.3
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] Revert drm/i915/bdw: BDW Software Turbo

[O'Rourke, Tom]

On Mon, Sep 29, 2014 at 12:46:25PM -0700, Jesse Barnes wrote:
 On Mon, 29 Sep 2014 09:52:38 -0700
 Rodrigo Vivi rodrigo.v...@gmail.com wrote:
 
  On Mon, Sep 29, 2014 at 9:38 AM, Daniel Vetter dan...@ffwll.ch wrote:
   On Mon, Sep 29, 2014 at 08:48:53AM -0700, Jesse Barnes wrote:
   On Mon, 29 Sep 2014 15:11:51 +0200
   Daniel Vetter daniel.vet...@ffwll.ch wrote:
  
This reverts commit c76bb61a71083b2d90504cc6d0dda2047c5d63ca.
   
It's apparently too broken so that Rodrigo submitted a patch to add a
config option for it. Given that the design is also ... suboptimal and
that I've only merged this to get lead engineers and managers off my
back for one second let's just revert this.
   
/me puts on combat gear again
   
It was worth a shot ...
  
   I thought we had a fix for the runtime PM issue this created?  And
   Rodrigo's fix is just a simple only BDW needs this patch, so I guess
   I don't see the big issue?
  
  Well, actually the issue is to stay with high busted gt frequency even on 
  idle.
  Or this or an incompatibility with the actual rps interface...
  
  So my patch was actually a protect a feture under development with 
  parameter.
  
  
   Or is there another bug you didn't mention in the s-o-b section you're
   worried about?
  
  The only issue I know is:
  References: https://bugs.freedesktop.org/show_bug.cgi?id=77869
 
 Ah yeah, misread it thinking it was accidentally being enabled on
 non-bdw too or something.
 
 But yeah I expect turbo to be broken even with your patch, since we
 won't ramp up/down the freq (or at least not properly).  The only thing
 Daisy's patch addresses is making sure we at least try to ramp if we're
 getting regular page flips.  The more complete fix (which should
 address the test failure too) would be to peroidically poll the hw busy
 state and adjust the freq based on that, independent of the flip
 frequency.  That should allow GPGPU apps, offlien media encode/decode,
 and these tests to work as they should, but it's a little more work.
 We need to make sure the timer is shut down when we go into RC6 and
 only fires frequently when the GPU is busy.

I object to reverting Daisy's BDW Software Turbo patch 
without also having an acceptable replacement.

Yes, Daisy's patch is not a complete fix.  Yes, a different 
design may have been better.  But this patch did get turbo 
working on BDW systems with regular flips (such as those 
running Android).  Taking this patch out without a 
replacement is a step in the wrong direction.

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

Re: [Intel-gfx] [PATCH 15/15] drm/i915/chv: Use timeout mode for RC6 on chv

[O'Rourke, Tom]

On Tue, Aug 05, 2014 at 07:51:26AM -0700, Rodrigo Vivi wrote:
 From: Deepak S deepa...@linux.intel.com
 
 Higher RC6 residency is observed using timeout mode
 instead of EI mode. It's Recommended to use TO Method for RC6.
 
[TOR:] When I made the similar change for BDW, I understood timeout mode will 
provide benefit on some pre-production CHV steppings and no benefit on the 
production CHV steppings.  Is that understanding still correct? Do we want 
to merge a change that is not expected to benefit any production steppings?

 Signed-off-by: Deepak S deepa...@linux.intel.com
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index d3085b7..0cc8460 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -4083,7 +4083,7 @@ static void cherryview_enable_rps(struct drm_device 
 *dev)
   I915_WRITE(RING_MAX_IDLE(ring-mmio_base), 10);
   I915_WRITE(GEN6_RC_SLEEP, 0);
  
 - I915_WRITE(GEN6_RC6_THRESHOLD, 5); /* 50/125ms per EI */
 + I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
[TOR:] A comment to explain the meaning of 0x557 could be helpful.

  
   /* allows RC6 residency counter to work */
   I915_WRITE(VLV_COUNTER_CONTROL,
 @@ -4099,7 +4099,7 @@ static void cherryview_enable_rps(struct drm_device 
 *dev)
   /* 3: Enable RC6 */
   if ((intel_enable_rc6(dev)  INTEL_RC6_ENABLE) 
   (pcbr  VLV_PCBR_ADDR_SHIFT))
 - rc6_mode = GEN6_RC_CTL_EI_MODE(1);
 + rc6_mode = GEN7_RC_CTL_TO_MODE;
  
   I915_WRITE(GEN6_RC_CONTROL, rc6_mode);

[TOR:] If we want to change from EI mode to TO mode for CHV, 
this patch does that correctly.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on bdw

[O'Rourke, Tom]

From: Ben Widawsky [mailto:b...@bwidawsk.net]
Sent: Friday, June 20, 2014 9:43 AM
On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote:
 Higher RC6 residency is observed using timeout mode instead of EI
 mode.  This applies to Broadwell only.
 The difference is particularly noticeable with video playback.

 Issue: VIZ-3778
 Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d
 Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com


Now that CHV is out, does it apply there too?
Reviewed-by: Ben Widawsky b...@bwidawsk.net

[snip]


--
Ben Widawsky, Intel Open Source Technology Center
[TOR:] For CHV, we expect timeout mode will provide benefit on some pre-
production steppings and no benefit on the production stepping.

[TOR:] Can we get this patch merged now that Ben has reviewed?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on bdw

[O'Rourke, Tom]

From: Ben Widawsky [mailto:b...@bwidawsk.net]
Sent: Friday, June 20, 2014 9:43 AM
On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote:
 Higher RC6 residency is observed using timeout mode instead of EI
 mode.  This applies to Broadwell only.
 The difference is particularly noticeable with video playback.

 Issue: VIZ-3778
 Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d
 Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com


Now that CHV is out, does it apply there too?
Reviewed-by: Ben Widawsky b...@bwidawsk.net

[snip]


--
Ben Widawsky, Intel Open Source Technology Center
[TOR:] For CHV, we expect timeout mode will provide benefit on some
pre-production steppings and no benefit on the production stepping.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/bdw: remove erroneous chv specific workarounds from bdw code

[O'Rourke, Tom]

---

All, I intend to push this to drm-intel-fixes, any objections?

Jani.
---
[TOR:] I have no objections.  Looks good to me.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[O'Rourke, Tom]

 This is just a merge mishap in one the chv patches. Someone just needs
 to send a patch that moves the misapplied stuff to the appropriate chv
 function.

Right. So my first comment was correct, and my elaboration total bullcrap. This
is not present in 3.15, but we've queued the screwup for 3.16. Thanks for the
correction Ville.

BR,
Jani.




 --
 Ville Syrjälä
 Intel OTC

--
Jani Nikula, Intel Open Source Technology Center
[TOR:] Hi Ville,
Do you want to be the someone to send the patch that moves the misapplied stuff?
I could guess where it goes but I do not have a CHV for testing the fix.
Thanks,
Tom
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on bdw

[O'Rourke, Tom]

From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, June 03, 2014 12:38 AM
To: O'Rourke, Tom
Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Ben Widawsky; Kristen
Carlson Accardi
Subject: Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on
bdw

[TOR:]...snip
 
  [TOR:] Can we get this patch merged now that RC6 is working on
  drm-intel-
 nightly?
 
 Needs some review from bdw people. Also some relative residency
 improvement date should be added to the commit message (yes, we're
 allowed to do that now officially).
 -Daniel
 --

 [TOR:] Hello bdw people, please review this patch.

 Is relative performance data now required in the commit message?  A
 week ago this would have been prohibited.

You might need to double-check with your own manager, but we're now again
allowed to officially add it to the commit message. It was kinda always 
required,
just had to be washed down more.
-Daniel

[TOR:] Sorry, I am not allowed to add the performance improvement claims to the 
commit message.  
Tom
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on bdw

[O'Rourke, Tom]

From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, June 02, 2014 1:26 AM
To: O'Rourke, Tom
Cc: intel-gfx@lists.freedesktop.org; Ben Widawsky; Kristen Carlson Accardi
Subject: Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on
bdw

On Fri, May 30, 2014 at 11:30:18PM +, O'Rourke, Tom wrote:
 On Wed, Apr 30, 2014 at 02:14:02PM -0700, Kristen Carlson Accardi wrote:
  On Thu, 01 May 2014 00:03:15 +0300
  Imre Deak imre.d...@intel.com wrote:
 
   On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote:
On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi
wrote:
 On Tue, 29 Apr 2014 22:31:49 -0700 Ben Widawsky
 b...@bwidawsk.net wrote:

  On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote:
   Higher RC6 residency is observed using timeout mode
   instead of EI mode.  This applies to Broadwell only.
   The difference is particularly noticeable with video
   playback.
  
   Issue: VIZ-3778
   Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d
   Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com
 
  I've merged this one to my bdw-rc6 branch, and therefore my
  broadwell branch. Hopefully Kristen will see some improvement.

 Unfortunately, I built your bdw-rc6 branch along with the
 revert I need to get my panel to work, and I get zero rc6 residency.
 Do I have to explicitly enable it?
   
I'm not actually sure. You can try it and let me know. I
haven't had any time to verify the rebase. We can check my hack.
  
   Note that in -nightly you also have to update
   sanitize_rc6_option() along with intel_enable_gt_powersave() and
   intel_disable_gt_powersave() since atm these keep RC6 disabled on BDW.
  
   --Imre
  
 
 
  Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was
  able to see some rc6 residency.  With the idle workload, residency
  appears to be similar to before, so no regression.
 
 Thanks. I'll squash this in where appropriate.
 
 --
 Ben Widawsky, Intel Open Source Technology Center

 [TOR:] Can we get this patch merged now that RC6 is working on drm-intel-
nightly?

Needs some review from bdw people. Also some relative residency
improvement date should be added to the commit message (yes, we're allowed
to do that now officially).
-Daniel
--

[TOR:] Hello bdw people, please review this patch.

Is relative performance data now required in the commit message?  A week ago 
this would have been prohibited.

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

Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on bdw

[O'Rourke, Tom]

On Wed, Apr 30, 2014 at 02:14:02PM -0700, Kristen Carlson Accardi wrote:
 On Thu, 01 May 2014 00:03:15 +0300
 Imre Deak imre.d...@intel.com wrote:

  On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote:
   On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi wrote:
On Tue, 29 Apr 2014 22:31:49 -0700 Ben Widawsky
b...@bwidawsk.net wrote:
   
 On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote:
  Higher RC6 residency is observed using timeout mode instead
  of EI mode.  This applies to Broadwell only.
  The difference is particularly noticeable with video
  playback.
 
  Issue: VIZ-3778
  Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d
  Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com

 I've merged this one to my bdw-rc6 branch, and therefore my
 broadwell branch. Hopefully Kristen will see some improvement.
   
Unfortunately, I built your bdw-rc6 branch along with the revert
I need to get my panel to work, and I get zero rc6 residency.
Do I have to explicitly enable it?
  
   I'm not actually sure. You can try it and let me know. I haven't
   had any time to verify the rebase. We can check my hack.
 
  Note that in -nightly you also have to update sanitize_rc6_option()
  along with intel_enable_gt_powersave() and
  intel_disable_gt_powersave() since atm these keep RC6 disabled on BDW.
 
  --Imre
 


 Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was able
 to see some rc6 residency.  With the idle workload, residency appears
 to be similar to before, so no regression.

Thanks. I'll squash this in where appropriate.

--
Ben Widawsky, Intel Open Source Technology Center

[TOR:] Can we get this patch merged now that RC6 is working on 
drm-intel-nightly?
Thanks,
Tom
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/5] drm/i915/bdw: Implement a basic PM interrupt handler

[O'Rourke, Tom]

+static void gen8_disable_rps_interrupts(struct drm_device *dev) {
+  struct drm_i915_private *dev_priv = dev-dev_private;
+
+  I915_WRITE(GEN6_PMINTRMSK, 0x);

[TOR:] Please note that for Broadwell, bit 31 in GEN6_PMINTRMSK is not an 
interrupt disable bit.
In drm/i915: Enable PM Interrupts target via   Display Interface. this bit is 
defined as:
+#define GEN8_PMINTR_REDIRECT_TO_NON_DISP   (131)

Writing this bit here could have unintended consequences.

Thanks,
Tom

+  I915_WRITE(GEN8_GT_IER(2), I915_READ(GEN8_GT_IER(2)) 
+ ~dev_priv-pm_rps_events);
+  /* Complete PM interrupt masking here doesn't race with the rps work
+   * item again unmasking PM interrupts because that is using a different
+   * register (GEN8_GT_IMR(2)) to mask PM interrupts. The only risk is in
+   * leaving stale bits in GEN8_GT_IIR(2) and GEN8_GT_IMR(2) which
+   * gen8_enable_rps will clean up. */
+
+  spin_lock_irq(dev_priv-irq_lock);
+  dev_priv-rps.pm_iir = 0;
+  spin_unlock_irq(dev_priv-irq_lock);
+
+  I915_WRITE(GEN8_GT_IIR(2), dev_priv-pm_rps_events); }
+
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on bdw

[O'Rourke, Tom]

  Higher RC6 residency is observed using timeout mode instead of EI
  mode.  This applies to Broadwell only.
  The difference is particularly noticeable with video playback.
 
  Issue: VIZ-3778
  Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d
  Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com

 How recent a nightly branch have you used to obtain these results?
 Chris just fixed some serious bugs in the gpu booster logic which
 would have affected all intermediate workloads.
 -Daniel

He must not be using nightly if he has any BDW RC6 residency at all.

[TOR:] Ben is correct.  I was testing mostly with a kernel for Android.
I also tested with Ben's broadwell branch and saw similar improvement.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx