Re: [Intel-gfx] [PATCH 1/2] drm: expose subpixel order name routine
On Mon, Jan 20, 2014 at 03:54:59PM -0800, Jesse Barnes wrote: > Just like we have for connector type etc. Then we might as well take a similar defensive approach if we want to expand the number of contexts we call it from. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/2] drm/i915: move module parameters into a struct, in a new file
With 20+ module parameters, I think referring to them via a struct improves clarity over just having a bunch of globals. While at it, move the parameter initialization and definitions into a new file i915_params.c to reduce clutter in i915_drv.c. Apart from the ill-named i915_enable_rc6, i915_enable_fbc and i915_enable_ppgtt parameters, for which we lose the "i915_" prefix internally, the module parameters now look the same both on the kernel command line and in code. For example, "i915.modeset". The downsides of the change are losing static on a couple of variables and not having the initialization and module_param_named() right next to each other. On the other hand, all module parameters are now defined in one place at i915_params.c. Plus you can do this to find all module parameter references: $ git grep "i915\." -- drivers/gpu/drm/i915 v2: - move the definitions into a new file - s/i915_params/i915/ - make i915_try_reset i915.reset, for consistency Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/i915_drv.c| 130 ++- drivers/gpu/drm/i915/i915_drv.h| 50 + drivers/gpu/drm/i915/i915_gem.c|4 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +- drivers/gpu/drm/i915/i915_gem_gtt.c|2 +- drivers/gpu/drm/i915/i915_irq.c|4 +- drivers/gpu/drm/i915/i915_params.c | 155 drivers/gpu/drm/i915/intel_bios.c |4 +- drivers/gpu/drm/i915/intel_display.c | 26 ++--- drivers/gpu/drm/i915/intel_dp.c|2 +- drivers/gpu/drm/i915/intel_lvds.c |6 +- drivers/gpu/drm/i915/intel_panel.c | 15 +-- drivers/gpu/drm/i915/intel_pm.c| 12 +-- 14 files changed, 228 insertions(+), 185 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_params.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index da682cbcb806..4850494bd548 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -14,6 +14,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ i915_gem_gtt.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ + i915_params.o \ i915_sysfs.o \ i915_trace_points.o \ i915_ums.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a543667140fa..73964ccdf3cb 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -38,120 +38,6 @@ #include #include -static int i915_modeset __read_mostly = -1; -module_param_named(modeset, i915_modeset, int, 0400); -MODULE_PARM_DESC(modeset, - "Use kernel modesetting [KMS] (0=DRM_I915_KMS from .config, " - "1=on, -1=force vga console preference [default])"); - -int i915_panel_ignore_lid __read_mostly = 1; -module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600); -MODULE_PARM_DESC(panel_ignore_lid, - "Override lid status (0=autodetect, 1=autodetect disabled [default], " - "-1=force lid closed, -2=force lid open)"); - -unsigned int i915_powersave __read_mostly = 1; -module_param_named(powersave, i915_powersave, int, 0600); -MODULE_PARM_DESC(powersave, - "Enable powersavings, fbc, downclocking, etc. (default: true)"); - -int i915_semaphores __read_mostly = -1; -module_param_named(semaphores, i915_semaphores, int, 0400); -MODULE_PARM_DESC(semaphores, - "Use semaphores for inter-ring sync (default: -1 (use per-chip defaults))"); - -int i915_enable_rc6 __read_mostly = -1; -module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0400); -MODULE_PARM_DESC(i915_enable_rc6, - "Enable power-saving render C-state 6. " - "Different stages can be selected via bitmask values " - "(0 = disable; 1 = enable rc6; 2 = enable deep rc6; 4 = enable deepest rc6). " - "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " - "default: -1 (use per-chip default)"); - -int i915_enable_fbc __read_mostly = -1; -module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600); -MODULE_PARM_DESC(i915_enable_fbc, - "Enable frame buffer compression for power savings " - "(default: -1 (use per-chip default))"); - -unsigned int i915_lvds_downclock __read_mostly = 0; -module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400); -MODULE_PARM_DESC(lvds_downclock, - "Use panel (LVDS/eDP) downclocking for power savings " - "(default: false)"); - -int i915_lvds_channel_mode __read_mostly; -module_param_named(lvds_channel_mode, i915_lvds_channel_mode, int, 0600); -MODULE_PARM_DESC(lvds_channel_mode, -"Specify LVDS channel mode " -"(0=probe BIOS [default], 1=single-channel, 2=dual-channel)"); - -int i915_panel_use_s
[Intel-gfx] [PATCH v2 1/2] drm/i915: drop the i915.fbpercrtc module parameter
It's unused, and nowadays specifying unknown parameters no longer
prevents modules from being loaded.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.c |3 ---
drivers/gpu/drm/i915/i915_drv.h |1 -
2 files changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 62c0f16b869d..a543667140fa 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -44,9 +44,6 @@ MODULE_PARM_DESC(modeset,
"Use kernel modesetting [KMS] (0=DRM_I915_KMS from .config, "
"1=on, -1=force vga console preference [default])");
-unsigned int i915_fbpercrtc __always_unused = 0;
-module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);
-
int i915_panel_ignore_lid __read_mostly = 1;
module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600);
MODULE_PARM_DESC(panel_ignore_lid,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f888fea9cb5d..56c720b789bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1896,7 +1896,6 @@ struct drm_i915_file_private {
extern const struct drm_ioctl_desc i915_ioctls[];
extern int i915_max_ioctl;
-extern unsigned int i915_fbpercrtc __always_unused;
extern int i915_panel_ignore_lid __read_mostly;
extern unsigned int i915_powersave __read_mostly;
extern int i915_semaphores __read_mostly;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] straggling drm patches
Hi Dave, A few drm core patches that haven't shown up in drm-next yet. All reviewed and the first 4 tested for a few weeks in drm-intel-nightly - I've quickly slapped the gem object init patch on top for convenience just today. Please pull or pick up the patches from the m-l. Cheers, Daniel The following changes since commit cfd72a4c2089aa3938f37281a34d6eb3306d5fd8: Merge branch 'drm-intel-next' of git://people.freedesktop.org/~danvet/drm-intel into drm-next (2014-01-20 10:21:54 +1000) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel topic/core-stuff for you to fetch changes up to 6ab11a2635ce988ebc2e798947beb72cf7324119: drm/gem: Always initialize the gem object in object_init (2014-01-21 10:19:58 +0100) Damien Lespiau (1): drm: Make the connector mode_valid() vfunc return a drm_mode_status enum Daniel Vetter (1): drm/gem: Always initialize the gem object in object_init Thomas Wood (2): drm/edid: split VIC display mode lookup into a separate function drm/edid: parse the list of additional 3D modes Vandana Kannan (1): drm/edid: Populate picture aspect ratio for CEA modes drivers/gpu/drm/drm_edid.c| 289 +- drivers/gpu/drm/drm_gem.c | 3 +- include/drm/drm_crtc.h| 2 + include/drm/drm_crtc_helper.h | 4 +- 4 files changed, 182 insertions(+), 116 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
On Mon, Jan 20, 2014 at 10:19:39AM -0700, Todd Previte wrote:
> For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that
> support it. The
> sink device must report that is supports Displayport 1.2 and the HBR2 bit
> rate in the
> DPCD in order to use HBR2.
>
> Signed-off-by: Todd Previte
As mentioned on irc, please add a per-patch changelog before the sob
section in the future. Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_dp.c | 31 +--
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 2 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7df5085..7fb02f6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -96,13 +96,18 @@ static int
> intel_dp_max_link_bw(struct intel_dp *intel_dp)
> {
> int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
> + struct drm_device *dev = intel_dp->attached_connector->base.dev;
>
> switch (max_link_bw) {
> case DP_LINK_BW_1_62:
> case DP_LINK_BW_2_7:
> break;
> case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
> - max_link_bw = DP_LINK_BW_2_7;
> + if ((IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8) &&
> + intel_dp->dpcd[DP_DPCD_REV] >= 0x12)
> + max_link_bw = DP_LINK_BW_5_4;
> + else
> + max_link_bw = DP_LINK_BW_2_7;
> break;
> default:
> WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
> @@ -805,9 +810,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> struct intel_connector *intel_connector = intel_dp->attached_connector;
> int lane_count, clock;
> int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> - int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 :
> 0;
> + /* Conveniently, the link BW constants become indices with a shift...*/
> + int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
> int bpp, mode_rate;
> - static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
> + static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
> int link_avail, link_clock;
>
> if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
> @@ -2621,10 +2627,15 @@ intel_dp_complete_link_train(struct intel_dp
> *intel_dp)
> bool channel_eq = false;
> int tries, cr_tries;
> uint32_t DP = intel_dp->DP;
> + uint32_t training_pattern = DP_TRAINING_PATTERN_2;
> +
> + /* Training Pattern 3 for HBR2 ot 1.2 devices that support it*/
> + if (intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3)
> + training_pattern = DP_TRAINING_PATTERN_3;
>
> /* channel equalization */
> if (!intel_dp_set_link_train(intel_dp, &DP,
> - DP_TRAINING_PATTERN_2 |
> + training_pattern |
>DP_LINK_SCRAMBLING_DISABLE)) {
> DRM_ERROR("failed to start channel equalization\n");
> return;
> @@ -2652,7 +2663,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
> if (!drm_dp_clock_recovery_ok(link_status,
> intel_dp->lane_count)) {
> intel_dp_start_link_train(intel_dp);
> intel_dp_set_link_train(intel_dp, &DP,
> - DP_TRAINING_PATTERN_2 |
> + training_pattern |
> DP_LINK_SCRAMBLING_DISABLE);
> cr_tries++;
> continue;
> @@ -2668,7 +2679,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
> intel_dp_link_down(intel_dp);
> intel_dp_start_link_train(intel_dp);
> intel_dp_set_link_train(intel_dp, &DP,
> - DP_TRAINING_PATTERN_2 |
> + training_pattern |
> DP_LINK_SCRAMBLING_DISABLE);
> tries = 0;
> cr_tries++;
> @@ -2810,6 +2821,14 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> }
> }
>
> + /* Training Pattern 3 support */
> + if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
> + intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED) {
> + intel_dp->use_tps3 = true;
> + DRM_DEBUG_KMS("Displayport TPS3 supported");
> + } else
> + intel_dp->use_tps3 = false;
> +
> if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DWN_STRM_PORT_PRESENT))
> return true; /* native DP sink */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: drop the i915.fbpercrtc module parameter
On Tue, Jan 21, 2014 at 11:24:24AM +0200, Jani Nikula wrote:
> It's unused, and nowadays specifying unknown parameters no longer
> prevents modules from being loaded.
>
> Signed-off-by: Jani Nikula
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c |3 ---
> drivers/gpu/drm/i915/i915_drv.h |1 -
> 2 files changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 62c0f16b869d..a543667140fa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -44,9 +44,6 @@ MODULE_PARM_DESC(modeset,
> "Use kernel modesetting [KMS] (0=DRM_I915_KMS from .config, "
> "1=on, -1=force vga console preference [default])");
>
> -unsigned int i915_fbpercrtc __always_unused = 0;
> -module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);
> -
> int i915_panel_ignore_lid __read_mostly = 1;
> module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600);
> MODULE_PARM_DESC(panel_ignore_lid,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f888fea9cb5d..56c720b789bc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1896,7 +1896,6 @@ struct drm_i915_file_private {
>
> extern const struct drm_ioctl_desc i915_ioctls[];
> extern int i915_max_ioctl;
> -extern unsigned int i915_fbpercrtc __always_unused;
> extern int i915_panel_ignore_lid __read_mostly;
> extern unsigned int i915_powersave __read_mostly;
> extern int i915_semaphores __read_mostly;
> --
> 1.7.9.5
>
--
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: Wait for completion of pending flips when starved of fences
On Mon, Jan 20, 2014 at 9:30 PM, Chris Wilson wrote: > On Mon, Jan 20, 2014 at 10:17:36AM +, Chris Wilson wrote: >> On older generations (gen2, gen3) the GPU requires fences for many >> operations, such as blits. The display hardware also requires fences for >> scanouts and this leads to a situation where an arbitrary number of >> fences may be pinned by old scanouts following a pageflip but before we >> have executed the unpin workqueue. This is unpredictable by userspace >> and leads to random EDEADLK when submitting an otherwise benign >> execbuffer. However, we can detect when we have an outstanding flip and >> so cause userspace to wait upon their completion before finally >> declaring that the system is starved of fences. This is really no worse >> than forcing the GPU to stall waiting for older execbuffer to retire and >> release their fences before we can reallocate them for the next >> execbuffer. >> >> v2: move the test for a pending fb unpin to a common routine for >> later reuse during eviction >> >> Reported-and-tested-by: di...@gmx.net >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73696 > Testcase: i-g-t/kms_flip/flip-vs-fences >> Signed-off-by: Chris Wilson > > The testcase is only for the trivial reproduction scenario, not the more > sporadic situation involving a slightly hungry workqueue, but it is > enough to demonstrate the issue and the effectiveness of the simple > workaround (for typical userspace). Testcase convinced me, this is better than status quo ;-) I guess I'll add a short comment though when merging that busy-looping through EAGAIN is a but un-neat. Jon, since this ties into the evict_something fun we've had last year can you please review this patch plus the follow-up one? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Wait for completion of pending flips when starved of fences
On Tue, Jan 21, 2014 at 10:32:50AM +0100, Daniel Vetter wrote: > On Mon, Jan 20, 2014 at 9:30 PM, Chris Wilson > wrote: > > The testcase is only for the trivial reproduction scenario, not the more > > sporadic situation involving a slightly hungry workqueue, but it is > > enough to demonstrate the issue and the effectiveness of the simple > > workaround (for typical userspace). > > Testcase convinced me, this is better than status quo ;-) I guess I'll > add a short comment though when merging that busy-looping through > EAGAIN is a but un-neat. On the other hand, it does reduce the RT problem into a typical priority inversion issue - all the lock dropping and complex error case handling is built in and does not need introduction of more fragile code. So reducing it to a known problem is itself neat. :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Turn get_aux_clock_divider() into per-product vfuncs
On Mon, 20 Jan 2014, Damien Lespiau wrote:
> A tiny clean-up to allow better code separation between platforms.
Should it also say "per-platform" instead of "per-product" in the
subject?
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/intel_dp.c | 79
>
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 2 files changed, 58 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 885d271..08e8e34 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -353,31 +353,49 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool
> has_aux_irq)
> return status;
> }
>
> -static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp,
> - int index)
> +/*
> + * The clock divider is based off the hrawclk, and would like to run at 2MHz.
> + * So, take the hrawclk value and divide by 2 and use that
This part of the comment clearly belongs above
i9xx_get_aux_clock_divider.
> + *
> + * Note that PCH attached eDP panels should use a 125MHz input clock divider.
> + */
But I'm not sure where this should go. Is this even valid anymore?
With that comment moved and possible fixed, or just nuked,
Reviewed-by: Jani Nikula
> +
> +static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int
> index)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = intel_dig_port->base.base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
>
> - /* The clock divider is based off the hrawclk,
> - * and would like to run at 2MHz. So, take the
> - * hrawclk value and divide by 2 and use that
> - *
> - * Note that PCH attached eDP panels should use a 125MHz input
> - * clock divider.
> - */
> - if (IS_VALLEYVIEW(dev)) {
> - return index ? 0 : 100;
> - } else if (intel_dig_port->port == PORT_A) {
> - if (index)
> - return 0;
> - if (HAS_DDI(dev))
> - return
> DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv), 2000);
> - else if (IS_GEN6(dev) || IS_GEN7(dev))
> + return index ? 0 : intel_hrawclk(dev) / 2;
> +}
> +
> +static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int
> index)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> +
> + if (index)
> + return 0;
> +
> + if (intel_dig_port->port == PORT_A) {
> + if (IS_GEN6(dev) || IS_GEN7(dev))
> return 200; /* SNB & IVB eDP input clock at 400Mhz */
> else
> return 225; /* eDP input clock at 450Mhz */
> + } else {
> + return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> + }
> +}
> +
> +static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int
> index)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (intel_dig_port->port == PORT_A) {
> + if (index)
> + return 0;
> + return DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv),
> 2000);
> } else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
> /* Workaround for non-ULT HSW */
> switch (index) {
> @@ -385,13 +403,16 @@ static uint32_t get_aux_clock_divider(struct intel_dp
> *intel_dp,
> case 1: return 72;
> default: return 0;
> }
> - } else if (HAS_PCH_SPLIT(dev)) {
> + } else {
> return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> - } else {
> - return index ? 0 :intel_hrawclk(dev) / 2;
> }
> }
>
> +static uint32_t vlv_get_aux_clock_divider(struct intel_dp *intel_dp, int
> index)
> +{
> + return index ? 0 : 100;
> +}
> +
> static int
> intel_dp_aux_ch(struct intel_dp *intel_dp,
> uint8_t *send, int send_bytes,
> @@ -450,7 +471,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> goto out;
> }
>
> - while ((aux_clock_divider = get_aux_clock_divider(intel_dp, clock++))) {
> + while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp,
> clock++))) {
> /* Must try at least 3 times according to DP spec */
> for (try = 0; try < 5; try++) {
> /* Load the send data into the aux channel data
> registers */
> @@ -1613,10 +1634,12 @@ static void intel_edp_psr_enable_sink(struct intel_dp
> *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> -
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Factor out a function returning the AUX_CTL value to start a send
On Mon, 20 Jan 2014, Damien Lespiau wrote:
> Also, move that computation outside of the for loop that tries 5 times,
> this value doesn't change between tries.
Some general bikeshedding...
I really wish everyone would write commit messages that work independent
of the patch subject. Just like a newspaper article should make sense
and tell a story also in the absence of a headline.
I know, Daniel usually churns out commit messages that are prime
examples of the *opposite* of this, so *sigh*. And,
Reviewed-by: Jani Nikula
on the patch.
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/intel_dp.c | 59
> ++---
> 1 file changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 08e8e34..a9a05d2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -413,6 +413,36 @@ static uint32_t vlv_get_aux_clock_divider(struct
> intel_dp *intel_dp, int index)
> return index ? 0 : 100;
> }
>
> +static uint32_t i9xx_get_aux_send_ctl(struct intel_dp *intel_dp,
> + bool has_aux_irq,
> + int send_bytes,
> + uint32_t aux_clock_divider)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + uint32_t precharge, timeout;
> +
> + if (IS_GEN6(dev))
> + precharge = 3;
> + else
> + precharge = 5;
> +
> + if (IS_BROADWELL(dev) && intel_dp->aux_ch_ctl_reg == DPA_AUX_CH_CTL)
> + timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> + else
> + timeout = DP_AUX_CH_CTL_TIME_OUT_400us;
> +
> + return DP_AUX_CH_CTL_SEND_BUSY |
> +(has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> +timeout |
> +(send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> +(precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> +(aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
> +DP_AUX_CH_CTL_DONE |
> +DP_AUX_CH_CTL_TIME_OUT_ERROR |
> +DP_AUX_CH_CTL_RECEIVE_ERROR;
> +}
> +
> static int
> intel_dp_aux_ch(struct intel_dp *intel_dp,
> uint8_t *send, int send_bytes,
> @@ -426,9 +456,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> uint32_t aux_clock_divider;
> int i, ret, recv_bytes;
> uint32_t status;
> - int try, precharge, clock = 0;
> + int try, clock = 0;
> bool has_aux_irq = true;
> - uint32_t timeout;
>
> /* dp aux is extremely sensitive to irq latency, hence request the
>* lowest possible wakeup latency and so prevent the cpu from going into
> @@ -438,16 +467,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>
> intel_dp_check_edp(intel_dp);
>
> - if (IS_GEN6(dev))
> - precharge = 3;
> - else
> - precharge = 5;
> -
> - if (IS_BROADWELL(dev) && ch_ctl == DPA_AUX_CH_CTL)
> - timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> - else
> - timeout = DP_AUX_CH_CTL_TIME_OUT_400us;
> -
> intel_aux_display_runtime_get(dev_priv);
>
> /* Try to wait for any previous AUX channel activity */
> @@ -472,6 +491,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> }
>
> while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp,
> clock++))) {
> + u32 send_ctl = i9xx_get_aux_send_ctl(intel_dp,
> + has_aux_irq,
> + send_bytes,
> + aux_clock_divider);
> +
> /* Must try at least 3 times according to DP spec */
> for (try = 0; try < 5; try++) {
> /* Load the send data into the aux channel data
> registers */
> @@ -480,16 +504,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> pack_aux(send + i, send_bytes - i));
>
> /* Send the command and wait for it to complete */
> - I915_WRITE(ch_ctl,
> -DP_AUX_CH_CTL_SEND_BUSY |
> -(has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> -timeout |
> -(send_bytes <<
> DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> -(precharge <<
> DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> -(aux_clock_divider <<
> DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
> -DP_AUX_CH_CTL_DONE |
> -DP_AUX_CH_CTL_TIME_OUT_ERROR |
> -DP_AUX_CH_CTL_RECEIVE_ERROR);
> + I915_WRITE(ch_ctl, send_ctl);
>
>
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Reorder the AUX_CTL bits in descending order
On Mon, 20 Jan 2014, Damien Lespiau wrote: > So it's easier to compare what we program with the documentation, not > having to jump at all. This could be squashed into the previous patch just as well, but either way, Reviewed-by: Jani Nikula > Signed-off-by: Damien Lespiau > --- > drivers/gpu/drm/i915/intel_dp.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index a9a05d2..ae9902a 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -433,14 +433,14 @@ static uint32_t i9xx_get_aux_send_ctl(struct intel_dp > *intel_dp, > timeout = DP_AUX_CH_CTL_TIME_OUT_400us; > > return DP_AUX_CH_CTL_SEND_BUSY | > +DP_AUX_CH_CTL_DONE | > (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) | > +DP_AUX_CH_CTL_TIME_OUT_ERROR | > timeout | > +DP_AUX_CH_CTL_RECEIVE_ERROR | > (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) | > (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) | > -(aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) | > -DP_AUX_CH_CTL_DONE | > -DP_AUX_CH_CTL_TIME_OUT_ERROR | > -DP_AUX_CH_CTL_RECEIVE_ERROR; > +(aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT); > } > > static int > -- > 1.8.3.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Introduce a get_aux_send_ctl() vfunc
On Mon, 20 Jan 2014, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau
Might be worth a mention this is prep work for something, as alone this
looks like a vfunc for the sake of vfunc...
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_dp.c | 10 ++
> drivers/gpu/drm/i915/intel_drv.h | 8
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ae9902a..95147ac 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -491,10 +491,10 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> }
>
> while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp,
> clock++))) {
> - u32 send_ctl = i9xx_get_aux_send_ctl(intel_dp,
> - has_aux_irq,
> - send_bytes,
> - aux_clock_divider);
> + u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> + has_aux_irq,
> + send_bytes,
> + aux_clock_divider);
>
> /* Must try at least 3 times according to DP spec */
> for (try = 0; try < 5; try++) {
> @@ -3685,6 +3685,8 @@ intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> else
> intel_dp->get_aux_clock_divider = i9xx_get_aux_clock_divider;
>
> + intel_dp->get_aux_send_ctl = i9xx_get_aux_send_ctl;
> +
> /* Preserve the current hw state. */
> intel_dp->DP = I915_READ(intel_dp->output_reg);
> intel_dp->attached_connector = intel_connector;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 6aeb62a..85bda0e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -494,6 +494,14 @@ struct intel_dp {
> struct intel_connector *attached_connector;
>
> uint32_t (*get_aux_clock_divider)(struct intel_dp *dp, int index);
> + /*
> + * This function returns the value we have to program the AUX_CTL
> + * register with to kick off an AUX transaction.
> + */
> + uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> + bool has_aux_irq,
> + int send_bytes,
> + uint32_t aux_clock_divider);
> };
>
> struct intel_digital_port {
> --
> 1.8.3.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Wait for completion of pending flips when starved of fences
> -Original Message- > From: intel-gfx-boun...@lists.freedesktop.org [mailto:intel-gfx- > boun...@lists.freedesktop.org] On Behalf Of Chris Wilson > Sent: Monday, January 20, 2014 10:18 AM > To: intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Wait for completion of pending > flips when starved of fences > > On older generations (gen2, gen3) the GPU requires fences for many > operations, such as blits. The display hardware also requires fences for > scanouts and this leads to a situation where an arbitrary number of fences > may be pinned by old scanouts following a pageflip but before we have > executed the unpin workqueue. This is unpredictable by userspace and leads > to random EDEADLK when submitting an otherwise benign execbuffer. > However, we can detect when we have an outstanding flip and so cause > userspace to wait upon their completion before finally declaring that the > system is starved of fences. This is really no worse than forcing the GPU to > stall waiting for older execbuffer to retire and release their fences before > we > can reallocate them for the next execbuffer. > > v2: move the test for a pending fb unpin to a common routine for later reuse > during eviction > > Reported-and-tested-by: di...@gmx.net > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73696 > Signed-off-by: Chris Wilson Review-by: Jon Bloomfield ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Repeat evictions whilst pageflip completions are outstanding
> From: intel-gfx-boun...@lists.freedesktop.org [mailto:intel-gfx- > boun...@lists.freedesktop.org] On Behalf Of Chris Wilson > Sent: Monday, January 20, 2014 10:18 AM > To: intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH 2/2] drm/i915: Repeat evictions whilst pageflip > completions are outstanding > > Since an old pageflip will keep its scanout buffer object pinned until it has > executed its unpin task on the common workqueue, we can clog up our > GGTT with stale pinned objects. As we cannot flush those workqueues > without dropping our locks, we have to resort to falling back to userspace > and telling them to repeat the operation in order to have a chance to run our > workqueues and free up the required memory. If we fail, then we are forced > to report ENOSPC back to userspace causing the operation to fail and best- > case scenario is that it introduces temporary corruption. > > Signed-off-by: Chris Wilson Reviewed-by: Jon Bloomfield ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Reorder the AUX_CTL bits in descending order
On Tue, Jan 21, 2014 at 12:09:18PM +0200, Jani Nikula wrote: > On Mon, 20 Jan 2014, Damien Lespiau wrote: > > So it's easier to compare what we program with the documentation, not > > having to jump at all. > > This could be squashed into the previous patch just as well, but either > way, I think it's a bit better to have separate patches to factor out code, and modify it. It generates easier diffs to review, rather than having one moving and changing the code at the same time. -- Damien ___ 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: clock readout support for DDI v2
On Mon, Jan 20, 2014 at 02:18:03PM -0800, Jesse Barnes wrote:
> Read out and calculate the port and pixel clocks on DDI configs as well.
> This means we have to grab the DP divider values and look at the port
> mapping to figure out which clock select reg to read out.
>
> v2: do the work from ddi_get_config (Ville)
>
> Signed-off-by: Jesse Barnes
> ---
> drivers/gpu/drm/i915/i915_reg.h | 6
> drivers/gpu/drm/i915/intel_ddi.c | 61
>
> drivers/gpu/drm/i915/intel_display.c | 2 ++
> 3 files changed, 69 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a699efd..644e4f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5318,8 +5318,13 @@
> #define WRPLL_PLL_SELECT_LCPLL_2700 (0x03<<28)
> /* WRPLL divider programming */
> #define WRPLL_DIVIDER_REFERENCE(x) ((x)<<0)
> +#define WRPLL_DIVIDER_REF_MASK (0xff)
> #define WRPLL_DIVIDER_POST(x) ((x)<<8)
> +#define WRPLL_DIVIDER_POST_MASK (0x3f<<8)
> +#define WRPLL_DIVIDER_POST_SHIFT8
> #define WRPLL_DIVIDER_FEEDBACK(x) ((x)<<16)
> +#define WRPLL_DIVIDER_FB_SHIFT 16
> +#define WRPLL_DIVIDER_FB_MASK (0xff<<16)
>
> /* Port clock selection */
> #define PORT_CLK_SEL_A 0x46100
> @@ -5332,6 +5337,7 @@
> #define PORT_CLK_SEL_WRPLL1 (4<<29)
> #define PORT_CLK_SEL_WRPLL2 (5<<29)
> #define PORT_CLK_SEL_NONE (7<<29)
> +#define PORT_CLK_SEL_MASK (7<<29)
>
> /* Transcoder clock selection */
> #define TRANS_CLK_SEL_A 0x46140
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 1488b28..d836c16 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -633,6 +633,65 @@ static void wrpll_update_rnp(uint64_t freq2k, unsigned
> budget,
> /* Otherwise a < c && b >= d, do nothing */
> }
>
> +static int intel_ddi_calc_wrpll_link(u32 wrpll)
> +{
> + int n, p, r;
> +
> + r = wrpll & WRPLL_DIVIDER_REF_MASK;
> + p = (wrpll & WRPLL_DIVIDER_POST_MASK) >> WRPLL_DIVIDER_POST_SHIFT;
> + n = (wrpll & WRPLL_DIVIDER_FB_MASK) >> WRPLL_DIVIDER_FB_SHIFT;
> +
> + return (LC_FREQ * n) / (p * r);
This is assuming the WRPLL will use the LCPLL as reference. Ideally we
should read out the ref clock settings too.
> +}
> +
> +static void haswell_ddi_clock_get(struct intel_encoder *encoder,
> + struct intel_crtc_config *pipe_config)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + enum port port = intel_ddi_get_encoder_port(encoder);
> + int link_clock;
> + u32 val, pll;
> +
> + val = I915_READ(PORT_CLK_SEL(port));
> + switch (val & PORT_CLK_SEL_MASK) {
> + case PORT_CLK_SEL_LCPLL_810:
> + link_clock = 81000;
> + break;
> + case PORT_CLK_SEL_LCPLL_1350:
> + link_clock = 135000;
> + break;
> + case PORT_CLK_SEL_LCPLL_2700:
> + link_clock = 27;
> + break;
> + case PORT_CLK_SEL_WRPLL1:
> + pll = I915_READ(WRPLL_CTL1);
> + link_clock = intel_ddi_calc_wrpll_link(pll);
> + break;
> + case PORT_CLK_SEL_WRPLL2:
> + pll = I915_READ(WRPLL_CTL2);
> + link_clock = intel_ddi_calc_wrpll_link(pll);
> + break;
> + case PORT_CLK_SEL_SPLL:
> + link_clock = 135000;
SPLL could also output 810 MHz.
> + break;
> + default:
> + WARN(1, "bad port clock sel\n");
> + return;
> + }
Could do the port_clock = link_clock * 2; here, and then pass port clock
to intel_dotclock_calculate() and avoid having to multiply crtc_clock by
2 afterwards.
As a side note, I must say it's a bit annoying that the DDI PLL code is
different to the rest of our PLL code. Hz vs. kHz etc. Makes it a bit
harder to figure out what it's doing.
> +
> + if (pipe_config->has_pch_encoder)
> + pipe_config->adjusted_mode.crtc_clock =
> + intel_dotclock_calculate(link_clock,
> + &pipe_config->fdi_m_n);
> + else
else if (has_dp_encoder)
> + pipe_config->adjusted_mode.crtc_clock =
> + intel_dotclock_calculate(link_clock,
> + &pipe_config->dp_m_n);
else
.crtc_clock = link_clock; // or port_clock if you take my suggestion
above
> +
> + pipe_config->port_clock = link_clock * 2;
> + pipe_config->adjusted_mode.crtc_clock *= 2;
> +}
> +
> static void
> intel_ddi_calculate_wrpll(int clock /* in Hz */,
> unsigned *r2_out, unsigned *n2_out, unsigned *p_out)
> @@ -1504,6 +1563,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Factor out a function returning the AUX_CTL value to start a send
On Tue, Jan 21, 2014 at 12:07:23PM +0200, Jani Nikula wrote: > > On Mon, 20 Jan 2014, Damien Lespiau wrote: > > Also, move that computation outside of the for loop that tries 5 times, > > this value doesn't change between tries. > > Some general bikeshedding... > > I really wish everyone would write commit messages that work independent > of the patch subject. Just like a newspaper article should make sense > and tell a story also in the absence of a headline. Will keep this in mind next time (one could argue we're in deep bikeshedding water here!). -- Damien ___ 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: Factor out a function returning the AUX_CTL value to start a send
On Tue, 21 Jan 2014, Damien Lespiau wrote: > On Tue, Jan 21, 2014 at 12:07:23PM +0200, Jani Nikula wrote: >> Some general bikeshedding... >> >> I really wish everyone would write commit messages that work independent >> of the patch subject. Just like a newspaper article should make sense >> and tell a story also in the absence of a headline. > > Will keep this in mind next time (one could argue we're in deep > bikeshedding water here!). Well, yeah. Nothing personal, Damien, times_this_has_annoyed_me++ just grew beyond some threshold at this patch. ;) BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] ACPI / video: Add systems that should favor native backlight interface
On Tue, 2014-01-21 at 13:32 +0800, Aaron Lu wrote: > On 01/21/2014 11:17 AM, Matthew Garrett wrote: > > We know that Windows 8 graphics drivers don't use the ACPI interface, > > and that systems change their behaviour as a result, in some cases with > > absolutely no way for the ACPI interface could possibly work. I haven't > > seen any cases where that's obviously true for any non-Windows 8 > > Perhaps I'm not clear, I didn't mean non-Windows 8 systems will all favor > GPU's interface, I just meant for one specific win7 laptop I could re-use > the existing code to make the GPU's interface as the only one left. And to > achieve this, the Win8 OSI check in acpi_video_verify_backlight_support > has to be gone. We could do that, but why do we think that's the correct fix? The plan is to remove the native list entirely and do this for all Windows 8 systems, so the Win8 OSI check is the right thing to do. -- Matthew Garrett ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Repeat evictions whilst pageflip completions are outstanding
On Tue, Jan 21, 2014 at 11:03:43AM +, Bloomfield, Jon wrote: > > From: intel-gfx-boun...@lists.freedesktop.org [mailto:intel-gfx- > > boun...@lists.freedesktop.org] On Behalf Of Chris Wilson > > Sent: Monday, January 20, 2014 10:18 AM > > To: intel-gfx@lists.freedesktop.org > > Subject: [Intel-gfx] [PATCH 2/2] drm/i915: Repeat evictions whilst pageflip > > completions are outstanding > > > > Since an old pageflip will keep its scanout buffer object pinned until it > > has > > executed its unpin task on the common workqueue, we can clog up our > > GGTT with stale pinned objects. As we cannot flush those workqueues > > without dropping our locks, we have to resort to falling back to userspace > > and telling them to repeat the operation in order to have a chance to run > > our > > workqueues and free up the required memory. If we fail, then we are forced > > to report ENOSPC back to userspace causing the operation to fail and best- > > case scenario is that it introduces temporary corruption. > > > > Signed-off-by: Chris Wilson > > Reviewed-by: Jon Bloomfield Both merged, with some frobbery to make the 2nd patch apply. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update
Dear audio and gfx stakeholders, We hope to add a new interface between audio and gfx driver, for gfx driver to notify audio about HDMI/DP hot-plug and ELD update. Would you please share some comments on the proposal below? Background of this issue: On Intel Haswell/Broadwell platforms, there is a HW restriction that after the display HD-Audio controller is in D3, it cannot be waken up by HDMI/DP hot-plug. Consequently, although the gfx driver can still detect the HDMI/DP hot-plug, audio driver has no idea about this and cannot notify user space whether the external HDMI/DP monitor is available for audio playback, because the audio controller cannot wake up to D0 and receive HW unsolicited event about hot-plug from the audio codec. This limitation will affect user space to decide whether we can output audio over HDMI/DP. To solve the above limitation, Takashi suggested to add a new communication interface between audio and gfx driver: create a common object containing the ops registered by both graphics and audio drivers, then communicate through it, something like vga_switcheroo. Is it okay to create this kernel object in i915 driver? I915 can export an API like "display_register_audio_client" for audio driver to register a client and hot-plug notification ops. I915 can also call some API like "display_register_gfx_client" itself and register ops for audio driver to query monitor presence and ELD info on a specific port. It would be faster for audio driver than quering ELD by command/response over the HD-A bus, thus avoid delay in i915 mode set. This will also avoid waking up the audio devices unnecessarily if the user space does not really want to use HDMI/DP for audio playback. Whenever i195 enables/disables audio on a port in modeset, it can call some API like "display_set_audio_state()" on this kernel object and trigger notifications to the audio driver. When the audio driver is probed (in the delayed probe stage), it can request i915 API symbol to register the audio client for this communication kernel object. Since the 1st i915 mode set may happen before audio driver registers the ops, we'll let audio driver check ELD once after registering the audio client ops. And for the platforms which uses this communication interface, we can disable unsolicited event for HDMI/DP hot-plug in the audio driver. We hope to hear your feedback and start to work out more details. Thanks & Best Regards Mengdong ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: quirk invert brightness for Acer Aspire 5336
On Mon, Jan 13, 2014 at 05:30:34PM +0200, Jani Nikula wrote:
> Since
> commit ee1452d7458451a7508e0663553ce88d63958157
> Author: Jani Nikula
> Date: Fri Sep 20 15:05:30 2013 +0300
>
> drm/i915: assume all GM45 Acer laptops use inverted backlight PWM
>
> failed and was later reverted in
> commit be505f643925e257087247b996cd8ece787c12af
> Author: Alexander van Heukelum
> Date: Sat Dec 28 21:00:39 2013 +0100
>
> Revert "drm/i915: assume all GM45 Acer laptops use inverted backlight PWM"
>
> fix the individual broken machine instead.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=54171
> Reference: http://mid.gmane.org/dub115-w7628c7c710ea51aa110cd4a5...@phx.gbl
> CC: sta...@vger.kernel.org
> Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
>
> ---
>
> Daniel, this is on top of -fixes.
> ---
> drivers/gpu/drm/i915/intel_display.c |3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 2bde35d34eb9..8712b5e93d7a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10556,6 +10556,9 @@ static struct intel_quirk intel_quirks[] = {
> /* Acer Aspire 4736Z */
> { 0x2a42, 0x1025, 0x0260, quirk_invert_brightness },
>
> + /* Acer Aspire 5336 */
> + { 0x2a42, 0x1025, 0x048a, quirk_invert_brightness },
> +
> /* Dell XPS13 HD Sandy Bridge */
> { 0x0116, 0x1028, 0x052e, quirk_no_pcm_pwm_enable },
> /* Dell XPS13 HD and XPS13 FHD Ivy Bridge */
> --
> 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] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update
On Tue, Jan 21, 2014 at 1:35 PM, Lin, Mengdong wrote: > Dear audio and gfx stakeholders, > > > > We hope to add a new interface between audio and gfx driver, for gfx driver > to notify audio about HDMI/DP hot-plug and ELD update. > > Would you please share some comments on the proposal below? > > > > Background of this issue: On Intel Haswell/Broadwell platforms, there is a > HW restriction that after the display HD-Audio controller is in D3, > > it cannot be waken up by HDMI/DP hot-plug. Consequently, although the gfx > driver can still detect the HDMI/DP hot-plug, > > audio driver has no idea about this and cannot notify user space whether the > external HDMI/DP monitor is available for audio playback, > > because the audio controller cannot wake up to D0 and receive HW unsolicited > event about hot-plug from the audio codec. > > This limitation will affect user space to decide whether we can output audio > over HDMI/DP. > > > > To solve the above limitation, Takashi suggested to add a new communication > interface between audio and gfx driver: create a common object > > containing the ops registered by both graphics and audio drivers, then > communicate through it, something like vga_switcheroo. > > > > Is it okay to create this kernel object in i915 driver? > > > > I915 can export an API like “display_register_audio_client” for audio driver > to register a client and hot-plug notification ops. > > > > I915 can also call some API like “display_register_gfx_client” itself and > register ops for audio driver to query monitor presence and ELD info on a > specific port. > > It would be faster for audio driver than quering ELD by command/response > over the HD-A bus, thus avoid delay in i915 mode set. > > This will also avoid waking up the audio devices unnecessarily if the user > space does not really want to use HDMI/DP for audio playback. > > > > Whenever i195 enables/disables audio on a port in modeset, it can call some > API like “display_set_audio_state()” on this kernel object and trigger > notifications to the audio driver. > > > > When the audio driver is probed (in the delayed probe stage), it can request > i915 API symbol to register the audio client for this communication kernel > object. > > Since the 1st i915 mode set may happen before audio driver registers the > ops, we’ll let audio driver check ELD once after registering the audio > client ops. > > And for the platforms which uses this communication interface, we can > disable unsolicited event for HDMI/DP hot-plug in the audio driver. > > > > We hope to hear your feedback and start to work out more details. Yeah, I've discussed this at KS with Takashi and we've agreed that some common object to facilitate driver interactions. A few things though: - This should be common infrastructure useable by all alsa and drm drivers, not just i915 and snd-hda. Especially on embedded platforms this issue is fairly rampant ... - While at it it should also encompass power management handling of the shared hw imo so that we can get rid of the hsw specific hacks for the power well code. Or at least we need to rework the power well code to reuse this new infrastructure, I don't really want to maintain a few copies of the lazy symbol_get logic this kind of stuff requires. - I think the biggest problem is figuring out who should register these device nodes. I think it makes the most sense if we do this in the gfx driver, but that requires some trickery on the alsa side (probably with using -EPROBE_DEFER or something like that. - I agree that passing ELD and all the other information through this new structure makes lot more sense than the current mess we have with passing the ELD through some hardware buffer. - Finally I think we should assign some identifier to this link which will get exposed both on the drm side and in alsa, so that userspace can figure out which display connects to which output. With that media player could do the Right Thing and automatically place the audio stream on the right pin in alsa. Adding dri-devel since that's where we imo need to have this discussion. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update
_Actually_ add dri-devel ... On Tue, Jan 21, 2014 at 2:10 PM, Daniel Vetter wrote: > On Tue, Jan 21, 2014 at 1:35 PM, Lin, Mengdong wrote: >> Dear audio and gfx stakeholders, >> >> >> >> We hope to add a new interface between audio and gfx driver, for gfx driver >> to notify audio about HDMI/DP hot-plug and ELD update. >> >> Would you please share some comments on the proposal below? >> >> >> >> Background of this issue: On Intel Haswell/Broadwell platforms, there is a >> HW restriction that after the display HD-Audio controller is in D3, >> >> it cannot be waken up by HDMI/DP hot-plug. Consequently, although the gfx >> driver can still detect the HDMI/DP hot-plug, >> >> audio driver has no idea about this and cannot notify user space whether the >> external HDMI/DP monitor is available for audio playback, >> >> because the audio controller cannot wake up to D0 and receive HW unsolicited >> event about hot-plug from the audio codec. >> >> This limitation will affect user space to decide whether we can output audio >> over HDMI/DP. >> >> >> >> To solve the above limitation, Takashi suggested to add a new communication >> interface between audio and gfx driver: create a common object >> >> containing the ops registered by both graphics and audio drivers, then >> communicate through it, something like vga_switcheroo. >> >> >> >> Is it okay to create this kernel object in i915 driver? >> >> >> >> I915 can export an API like “display_register_audio_client” for audio driver >> to register a client and hot-plug notification ops. >> >> >> >> I915 can also call some API like “display_register_gfx_client” itself and >> register ops for audio driver to query monitor presence and ELD info on a >> specific port. >> >> It would be faster for audio driver than quering ELD by command/response >> over the HD-A bus, thus avoid delay in i915 mode set. >> >> This will also avoid waking up the audio devices unnecessarily if the user >> space does not really want to use HDMI/DP for audio playback. >> >> >> >> Whenever i195 enables/disables audio on a port in modeset, it can call some >> API like “display_set_audio_state()” on this kernel object and trigger >> notifications to the audio driver. >> >> >> >> When the audio driver is probed (in the delayed probe stage), it can request >> i915 API symbol to register the audio client for this communication kernel >> object. >> >> Since the 1st i915 mode set may happen before audio driver registers the >> ops, we’ll let audio driver check ELD once after registering the audio >> client ops. >> >> And for the platforms which uses this communication interface, we can >> disable unsolicited event for HDMI/DP hot-plug in the audio driver. >> >> >> >> We hope to hear your feedback and start to work out more details. > > Yeah, I've discussed this at KS with Takashi and we've agreed that > some common object to facilitate driver interactions. A few things > though: > - This should be common infrastructure useable by all alsa and drm > drivers, not just i915 and snd-hda. Especially on embedded platforms > this issue is fairly rampant ... > - While at it it should also encompass power management handling of > the shared hw imo so that we can get rid of the hsw specific hacks for > the power well code. Or at least we need to rework the power well code > to reuse this new infrastructure, I don't really want to maintain a > few copies of the lazy symbol_get logic this kind of stuff requires. > - I think the biggest problem is figuring out who should register > these device nodes. I think it makes the most sense if we do this in > the gfx driver, but that requires some trickery on the alsa side > (probably with using -EPROBE_DEFER or something like that. > - I agree that passing ELD and all the other information through this > new structure makes lot more sense than the current mess we have with > passing the ELD through some hardware buffer. > - Finally I think we should assign some identifier to this link which > will get exposed both on the drm side and in alsa, so that userspace > can figure out which display connects to which output. With that media > player could do the Right Thing and automatically place the audio > stream on the right pin in alsa. > > Adding dri-devel since that's where we imo need to have this discussion. > > Cheers, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: deferring the HSW IPS enable at plane switch
On Sun, Jan 19, 2014 at 1:47 PM, Ramalingam C wrote: > To remove the wait_for_vblank from the plane switch execution path, > this change implements a function which will add a delayed work to > defer the IPS enable. > > The delay is nothing but frame length, which is calculated based on > vrefresh of the hwmode. i.e IPS enable will be scheduled after a frame. > If mode is not set during the plane switch (__unlikely__), delay will > fallback to a default value 100mSec (can handle the FPS >= 10). > > Signed-off-by: Ramalingam C We have a general need to do abitrary stuff in both irq and process context on future vblanks. Chris started with such infrastructure already: http://lists.freedesktop.org/archives/intel-gfx/2012-April/016557.html We've had a fair bit of discussion about where to go to with all this, but atm this is stalled. I also think we should have this bit of infrastructure generally available for drivers in drm_vblank.c. The other thing to keep in mind is to make this fit with Ville's plan for nuclear pageflips. I think in the end the modeset code should only fire up the pipe itself (i.e. we'll display a nice black) and enabling pipes, sprites and cursors should happen with the nuclear pageflip code. Otherwise we need to duplicate this logic, which usually means twice the bugs. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4 v2] drm/i915: Turn get_aux_clock_divider() into per-platform vfuncs
A tiny clean-up to allow better code separation between platforms.
v2: Fix comment placement (put in in i9xx_get_aux_clock_divider()) and
nuke the outdated PCH eDP comment (Jani Nikula)
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_dp.c | 74
drivers/gpu/drm/i915/intel_drv.h | 2 ++
2 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 885d271..8a4ed4a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -353,31 +353,46 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool
has_aux_irq)
return status;
}
-static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp,
- int index)
+static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int
index)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- /* The clock divider is based off the hrawclk,
-* and would like to run at 2MHz. So, take the
-* hrawclk value and divide by 2 and use that
-*
-* Note that PCH attached eDP panels should use a 125MHz input
-* clock divider.
+ /*
+* The clock divider is based off the hrawclk, and would like to run at
+* 2MHz. So, take the hrawclk value and divide by 2 and use that
*/
- if (IS_VALLEYVIEW(dev)) {
- return index ? 0 : 100;
- } else if (intel_dig_port->port == PORT_A) {
- if (index)
- return 0;
- if (HAS_DDI(dev))
- return
DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv), 2000);
- else if (IS_GEN6(dev) || IS_GEN7(dev))
+ return index ? 0 : intel_hrawclk(dev) / 2;
+}
+
+static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+
+ if (index)
+ return 0;
+
+ if (intel_dig_port->port == PORT_A) {
+ if (IS_GEN6(dev) || IS_GEN7(dev))
return 200; /* SNB & IVB eDP input clock at 400Mhz */
else
return 225; /* eDP input clock at 450Mhz */
+ } else {
+ return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
+ }
+}
+
+static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (intel_dig_port->port == PORT_A) {
+ if (index)
+ return 0;
+ return DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv),
2000);
} else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
/* Workaround for non-ULT HSW */
switch (index) {
@@ -385,13 +400,16 @@ static uint32_t get_aux_clock_divider(struct intel_dp
*intel_dp,
case 1: return 72;
default: return 0;
}
- } else if (HAS_PCH_SPLIT(dev)) {
+ } else {
return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
- } else {
- return index ? 0 :intel_hrawclk(dev) / 2;
}
}
+static uint32_t vlv_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
+{
+ return index ? 0 : 100;
+}
+
static int
intel_dp_aux_ch(struct intel_dp *intel_dp,
uint8_t *send, int send_bytes,
@@ -450,7 +468,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
goto out;
}
- while ((aux_clock_divider = get_aux_clock_divider(intel_dp, clock++))) {
+ while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp,
clock++))) {
/* Must try at least 3 times according to DP spec */
for (try = 0; try < 5; try++) {
/* Load the send data into the aux channel data
registers */
@@ -1613,10 +1631,12 @@ static void intel_edp_psr_enable_sink(struct intel_dp
*intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct drm_i915_private *dev_priv = dev->dev_private;
- uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp, 0);
+ uint32_t aux_clock_divider;
int precharge = 0x3;
int msg_size = 5; /* Header(4) + Message(1) */
+ aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
+
/* Enable PSR in sink */
if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT)
intel_dp_aux_native_write_1(intel_dp, DP
[Intel-gfx] [PATCH 4/4 v2] drm/i915: Introduce a get_aux_send_ctl() vfunc
We need a bit more flexibility here in the future, bits get shuffled
around.
v2: more descriptive commit message (Jani Nikula)
Reviewed-by: Jani Nikula
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_dp.c | 10 ++
drivers/gpu/drm/i915/intel_drv.h | 8
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7faf364..8936298 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -488,10 +488,10 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
}
while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp,
clock++))) {
- u32 send_ctl = i9xx_get_aux_send_ctl(intel_dp,
-has_aux_irq,
-send_bytes,
-aux_clock_divider);
+ u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
+ has_aux_irq,
+ send_bytes,
+ aux_clock_divider);
/* Must try at least 3 times according to DP spec */
for (try = 0; try < 5; try++) {
@@ -3682,6 +3682,8 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
else
intel_dp->get_aux_clock_divider = i9xx_get_aux_clock_divider;
+ intel_dp->get_aux_send_ctl = i9xx_get_aux_send_ctl;
+
/* Preserve the current hw state. */
intel_dp->DP = I915_READ(intel_dp->output_reg);
intel_dp->attached_connector = intel_connector;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6aeb62a..85bda0e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -494,6 +494,14 @@ struct intel_dp {
struct intel_connector *attached_connector;
uint32_t (*get_aux_clock_divider)(struct intel_dp *dp, int index);
+ /*
+* This function returns the value we have to program the AUX_CTL
+* register with to kick off an AUX transaction.
+*/
+ uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
+bool has_aux_irq,
+int send_bytes,
+uint32_t aux_clock_divider);
};
struct intel_digital_port {
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 3/5] drm/i915: Make sprite updates atomic
From: Ville Syrjälä
Add a mechanism by which we can evade the leading edge of vblank. This
guarantees that no two sprite register writes will straddle on either
side of the vblank start, and that means all the writes will be latched
together in one atomic operation.
We do the vblank evade by checking the scanline counter, and if it's too
close to the start of vblank (too close has been hardcoded to 100usec
for now), we will wait for the vblank start to pass. In order to
eliminate random delayes from the rest of the system, we operate with
interrupts disabled, except when waiting for the vblank obviously.
Note that we now go digging through pipe_to_crtc_mapping[] in the
vblank interrupt handler, which is a bit dangerous since we set up
interrupts before the crtcs. However in this case since it's the vblank
interrupt, we don't actually unmask it until some piece of code
requests it.
v2: preempt_check_resched() calls after local_irq_enable() (Jesse)
Hook up the vblank irq stuff on BDW as well
v3: Pass intel_crtc instead of drm_crtc (Daniel)
Warn if crtc.mutex isn't locked (Daniel)
Add an explicit compiler barrier and document the barriers (Daniel)
Note the irq vs. modeset setup madness in the commit message (Daniel)
Reviewed-by: Jesse Barnes
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_irq.c | 29 --
drivers/gpu/drm/i915/intel_display.c | 2 +
drivers/gpu/drm/i915/intel_drv.h | 3 +
drivers/gpu/drm/i915/intel_sprite.c | 103 +++
4 files changed, 133 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ffb56a9..8ef9edb 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1439,6 +1439,15 @@ static void gen6_rps_irq_handler(struct drm_i915_private
*dev_priv, u32 pm_iir)
}
}
+static void intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
+{
+ struct intel_crtc *intel_crtc =
+ to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
+
+ intel_crtc->vbl_received = true;
+ wake_up(&intel_crtc->vbl_wait);
+}
+
static irqreturn_t valleyview_irq_handler(int irq, void *arg)
{
struct drm_device *dev = (struct drm_device *) arg;
@@ -1479,8 +1488,10 @@ static irqreturn_t valleyview_irq_handler(int irq, void
*arg)
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
for_each_pipe(pipe) {
- if (pipe_stats[pipe] &
PIPE_START_VBLANK_INTERRUPT_STATUS)
+ if (pipe_stats[pipe] &
PIPE_START_VBLANK_INTERRUPT_STATUS) {
drm_handle_vblank(dev, pipe);
+ intel_pipe_handle_vblank(dev, pipe);
+ }
if (pipe_stats[pipe] & PLANE_FLIPDONE_INT_STATUS_VLV) {
intel_prepare_page_flip(dev, pipe);
@@ -1679,8 +1690,10 @@ static void ilk_display_irq_handler(struct drm_device
*dev, u32 de_iir)
DRM_ERROR("Poison interrupt\n");
for_each_pipe(pipe) {
- if (de_iir & DE_PIPE_VBLANK(pipe))
+ if (de_iir & DE_PIPE_VBLANK(pipe)) {
drm_handle_vblank(dev, pipe);
+ intel_pipe_handle_vblank(dev, pipe);
+ }
if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
if (intel_set_cpu_fifo_underrun_reporting(dev, pipe,
false))
@@ -1729,8 +1742,10 @@ static void ivb_display_irq_handler(struct drm_device
*dev, u32 de_iir)
intel_opregion_asle_intr(dev);
for_each_pipe(i) {
- if (de_iir & (DE_PIPE_VBLANK_IVB(i)))
+ if (de_iir & (DE_PIPE_VBLANK_IVB(i))) {
drm_handle_vblank(dev, i);
+ intel_pipe_handle_vblank(dev, i);
+ }
/* plane/pipes map 1:1 on ilk+ */
if (de_iir & DE_PLANE_FLIP_DONE_IVB(i)) {
@@ -1872,8 +1887,10 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
continue;
pipe_iir = I915_READ(GEN8_DE_PIPE_IIR(pipe));
- if (pipe_iir & GEN8_PIPE_VBLANK)
+ if (pipe_iir & GEN8_PIPE_VBLANK) {
drm_handle_vblank(dev, pipe);
+ intel_pipe_handle_vblank(dev, pipe);
+ }
if (pipe_iir & GEN8_PIPE_FLIP_DONE) {
intel_prepare_page_flip(dev, pipe);
@@ -3161,6 +3178,8 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
if (!drm_handle_vblank(dev, pipe))
return false;
+ intel_pipe_handle_vblank(dev, pipe);
+
if ((iir & flip_pending) == 0)
return false;
@@ -3344,6 +3363,8 @@ static bool i915_handle_vblank(struct drm_device *dev,
if (!drm_handle_vblank(dev, pipe))
return false
[Intel-gfx] [PATCH v2 4/5] drm/i915: Perform primary enable/disable atomically with sprite updates
From: Ville Syrjälä
Move the primary plane enable/disable to occur atomically with the
sprite update that caused the primary plane visibility to change.
FBC and IPS enable/disable is left to happen well before or after
the primary plane change.
v2: Pass intel_crtc instead of drm_crtc (Daniel)
Reviewed-by: Jesse Barnes
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_sprite.c | 94 ++---
1 file changed, 55 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 1ffa7ba..8bf3be8 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -110,6 +110,17 @@ static void intel_pipe_update_end(struct intel_crtc *crtc)
preempt_check_resched();
}
+static void intel_update_primary_plane(struct intel_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+ int reg = DSPCNTR(crtc->plane);
+
+ if (crtc->primary_enabled)
+ I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
+ else
+ I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
+}
+
static void
vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
struct drm_framebuffer *fb,
@@ -207,6 +218,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
intel_pipe_update_start(intel_crtc);
+ intel_update_primary_plane(intel_crtc);
+
I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
@@ -219,7 +232,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
I915_WRITE(SPCNTR(pipe, plane), sprctl);
I915_MODIFY_DISPBASE(SPSURF(pipe, plane), i915_gem_obj_ggtt_offset(obj)
+
sprsurf_offset);
- POSTING_READ(SPSURF(pipe, plane));
+
+ intel_flush_primary_plane(dev_priv, intel_crtc->plane);
intel_pipe_update_end(intel_crtc);
}
@@ -236,11 +250,14 @@ vlv_disable_plane(struct drm_plane *dplane, struct
drm_crtc *crtc)
intel_pipe_update_start(intel_crtc);
+ intel_update_primary_plane(intel_crtc);
+
I915_WRITE(SPCNTR(pipe, plane), I915_READ(SPCNTR(pipe, plane)) &
~SP_ENABLE);
/* Activate double buffered register update */
I915_MODIFY_DISPBASE(SPSURF(pipe, plane), 0);
- POSTING_READ(SPSURF(pipe, plane));
+
+ intel_flush_primary_plane(dev_priv, intel_crtc->plane);
intel_pipe_update_end(intel_crtc);
@@ -385,6 +402,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
intel_pipe_update_start(intel_crtc);
+ intel_update_primary_plane(intel_crtc);
+
I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
@@ -403,7 +422,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
I915_WRITE(SPRCTL(pipe), sprctl);
I915_MODIFY_DISPBASE(SPRSURF(pipe),
i915_gem_obj_ggtt_offset(obj) + sprsurf_offset);
- POSTING_READ(SPRSURF(pipe));
+
+ intel_flush_primary_plane(dev_priv, intel_crtc->plane);
intel_pipe_update_end(intel_crtc);
}
@@ -419,13 +439,16 @@ ivb_disable_plane(struct drm_plane *plane, struct
drm_crtc *crtc)
intel_pipe_update_start(intel_crtc);
+ intel_update_primary_plane(intel_crtc);
+
I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
/* Can't leave the scaler enabled... */
if (intel_plane->can_scale)
I915_WRITE(SPRSCALE(pipe), 0);
/* Activate double buffered register update */
I915_MODIFY_DISPBASE(SPRSURF(pipe), 0);
- POSTING_READ(SPRSURF(pipe));
+
+ intel_flush_primary_plane(dev_priv, intel_crtc->plane);
intel_pipe_update_end(intel_crtc);
@@ -574,6 +597,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
intel_pipe_update_start(intel_crtc);
+ intel_update_primary_plane(intel_crtc);
+
I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
@@ -587,7 +612,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
I915_WRITE(DVSCNTR(pipe), dvscntr);
I915_MODIFY_DISPBASE(DVSSURF(pipe),
i915_gem_obj_ggtt_offset(obj) + dvssurf_offset);
- POSTING_READ(DVSSURF(pipe));
+
+ intel_flush_primary_plane(dev_priv, intel_crtc->plane);
intel_pipe_update_end(intel_crtc);
}
@@ -603,12 +629,15 @@ ilk_disable_plane(struct drm_plane *plane, struct
drm_crtc *crtc)
intel_pipe_update_start(intel_crtc);
+ intel_update_primary_plane(intel_crtc);
+
I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE);
/* Disable the scaler */
I915_WRITE(DVSSCALE(pipe), 0);
[Intel-gfx] [PATCH v3 5/5] drm/i915: Add pipe update trace points
From: Ville Syrjälä
Add trace points for observing the atomic pipe update mechanism.
v2: Rebased due to earlier changes
v3: Pass intel_crtc instead of drm_crtc (Daniel)
Reviewed-by: Jesse Barnes
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_trace.h | 77 +
drivers/gpu/drm/i915/intel_sprite.c | 6 +++
2 files changed, 83 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_trace.h
b/drivers/gpu/drm/i915/i915_trace.h
index 6e580c9..b903f8e 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -7,6 +7,7 @@
#include
#include "i915_drv.h"
+#include "intel_drv.h"
#include "intel_ringbuffer.h"
#undef TRACE_SYSTEM
@@ -14,6 +15,82 @@
#define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
#define TRACE_INCLUDE_FILE i915_trace
+/* pipe updates */
+
+TRACE_EVENT(i915_pipe_update_start,
+ TP_PROTO(struct intel_crtc *crtc, u32 min, u32 max),
+ TP_ARGS(crtc, min, max),
+
+ TP_STRUCT__entry(
+__field(enum pipe, pipe)
+__field(u32, frame)
+__field(u32, scanline)
+__field(u32, min)
+__field(u32, max)
+),
+
+ TP_fast_assign(
+ __entry->pipe = crtc->pipe;
+ __entry->frame =
crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
+
crtc->pipe);
+ __entry->scanline =
intel_get_crtc_scanline(&crtc->base);
+ __entry->min = min;
+ __entry->max = max;
+ ),
+
+ TP_printk("pipe %c, frame=%u, scanline=%u, min=%u, max=%u",
+ pipe_name(__entry->pipe), __entry->frame,
+ __entry->scanline, __entry->min, __entry->max)
+);
+
+TRACE_EVENT(i915_pipe_update_vblank_evaded,
+ TP_PROTO(struct intel_crtc *crtc, u32 min, u32 max),
+ TP_ARGS(crtc, min, max),
+
+ TP_STRUCT__entry(
+__field(enum pipe, pipe)
+__field(u32, frame)
+__field(u32, scanline)
+__field(u32, min)
+__field(u32, max)
+),
+
+ TP_fast_assign(
+ __entry->pipe = crtc->pipe;
+ __entry->frame =
crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
+
crtc->pipe);
+ __entry->scanline =
intel_get_crtc_scanline(&crtc->base);
+ __entry->min = min;
+ __entry->max = max;
+ ),
+
+ TP_printk("pipe %c, frame=%u, scanline=%u, min=%u, max=%u",
+ pipe_name(__entry->pipe), __entry->frame,
+ __entry->scanline, __entry->min, __entry->max)
+);
+
+TRACE_EVENT(i915_pipe_update_end,
+ TP_PROTO(struct intel_crtc *crtc),
+ TP_ARGS(crtc),
+
+ TP_STRUCT__entry(
+__field(enum pipe, pipe)
+__field(u32, frame)
+__field(u32, scanline)
+),
+
+ TP_fast_assign(
+ __entry->pipe = crtc->pipe;
+ __entry->frame =
crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
+
crtc->pipe);
+ __entry->scanline =
intel_get_crtc_scanline(&crtc->base);
+ ),
+
+ TP_printk("pipe %c, frame=%u, scanline=%u",
+ pipe_name(__entry->pipe), __entry->frame,
+ __entry->scanline)
+);
+
/* object tracking */
TRACE_EVENT(i915_gem_object_create,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 8bf3be8..47e1721 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -64,6 +64,8 @@ static void intel_pipe_update_start(struct intel_crtc *crtc)
local_irq_disable();
+ trace_i915_pipe_update_start(crtc, min, max);
+
/*
* Explicit compiler barrier is used to make sure that vbl_received
* store happens before reading the current scanline, otherwise
@@ -102,10 +104,14 @@ static void intel_pipe_update_start(struct intel_crtc
*crtc)
}
drm_vblank_put(dev, pipe);
+
+ trace_i915_pipe_update_vblank_evaded(crtc, min, max);
}
static void intel_pipe_update_end(struct intel_crtc *crtc)
{
+ trace_i915_pipe_update_end(crtc);
+
Re: [Intel-gfx] [PATCH v4 1/3] drm/i915: Disable/Enable PM Intrrupts based on the current freq.
On Mon, Jan 20, 2014 at 06:40:24PM +0530, deepa...@intel.com wrote:
> From: Deepak S
>
> When current delay is already at max delay, Let's disable the PM UP
> THRESHOLD INTRRUPTS, so that we will not get further interrupts until
> current delay is less than max delay, Also request for the PM DOWN
> THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and
> viceversa for PM DOWN THRESHOLD INTRRUPTS.
>
> v2: Use bool variables (Daniel)
>
> v3: Fix Interrupt masking bit (Deepak)
>
> v4: Use existing symbolic constants in i915_reg.h (Daniel)
>
> Signed-off-by: Deepak S
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/gpu/drm/i915/i915_irq.c | 31 +--
> drivers/gpu/drm/i915/intel_pm.c | 3 +++
> 3 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f888fea..e89b9f4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
> u8 rp0_delay;
> u8 hw_max;
>
> + bool rp_up_masked;
> + bool rp_down_masked;
> +
> int last_adj;
> enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 160d65d..d0d87ed 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -993,7 +993,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
> adj *= 2;
> else
> adj = 1;
> - new_delay = dev_priv->rps.cur_delay + adj;
> +
> + if (dev_priv->rps.cur_delay >= dev_priv->rps.max_delay) {
> + I915_WRITE(GEN6_PMINTRMSK,
> + I915_READ(GEN6_PMINTRMSK) |
> GEN6_PM_RP_UP_THRESHOLD);
> + dev_priv->rps.rp_up_masked = true;
> + new_delay = dev_priv->rps.cur_delay;
> + } else
> + new_delay = dev_priv->rps.cur_delay + adj;
> +
> + if (dev_priv->rps.rp_down_masked) {
> + I915_WRITE(GEN6_PMINTRMSK,
> + I915_READ(GEN6_PMINTRMSK) &
> ~GEN6_PM_RP_DOWN_THRESHOLD);
> + dev_priv->rps.rp_down_masked = false;
> + }
At this point we've not yet computed the final new_delay. It would seem
better to me to put all this code to place where we have the final
new_delay.
Also I wonder if we should also mask the DOWN_TIMEOUT interrupt?
>
> /*
>* For better performance, jump directly
> @@ -1012,7 +1025,21 @@ static void gen6_pm_rps_work(struct work_struct *work)
> adj *= 2;
> else
> adj = -1;
> - new_delay = dev_priv->rps.cur_delay + adj;
> +
> + if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay) {
> + I915_WRITE(GEN6_PMINTRMSK,
> + I915_READ(GEN6_PMINTRMSK) |
> GEN6_PM_RP_DOWN_THRESHOLD);
> + dev_priv->rps.rp_down_masked = true;
> + new_delay = dev_priv->rps.cur_delay;
> + } else
> + new_delay = dev_priv->rps.cur_delay + adj;
> +
> + if (dev_priv->rps.rp_up_masked) {
> + I915_WRITE(GEN6_PMINTRMSK,
> + I915_READ(GEN6_PMINTRMSK) &
> ~GEN6_PM_RP_UP_THRESHOLD);
> + dev_priv->rps.rp_up_masked = false;
> + }
Same comments apply.
> +
> } else { /* unknown event */
> new_delay = dev_priv->rps.cur_delay;
> }
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b9b4fe4..d00a2cf 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3613,6 +3613,9 @@ static void valleyview_enable_rps(struct drm_device
> *dev)
>vlv_gpu_freq(dev_priv, dev_priv->rps.rpe_delay),
>dev_priv->rps.rpe_delay);
>
> + dev_priv->rps.rp_up_masked = false;
> + dev_priv->rps.rp_down_masked = false;
> +
> valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
>
> gen6_enable_rps_interrupts(dev);
> --
> 1.8.4.2
>
> ___
> 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 v3 2/3] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.
On Mon, Jan 20, 2014 at 06:40:25PM +0530, deepa...@intel.com wrote:
> From: Deepak S
>
> When we enter RC6 and GFX Clocks are off, the voltage remains higher
> than Vmin. When we try to set the freq to RPe, it might fail since the
> Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock up
> and set the freq to RPe then move GFx down.
>
> v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)
>
> v3: Fix the timeout during wait for gfx clock (Jesse)
>
> Signed-off-by: Deepak S
> ---
> drivers/gpu/drm/i915/i915_reg.h | 4
> drivers/gpu/drm/i915/intel_pm.c | 48
> -
> 2 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc2f3de..e1d5f31 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4944,6 +4944,10 @@
>GEN6_PM_RP_DOWN_THRESHOLD | \
>GEN6_PM_RP_DOWN_TIMEOUT)
>
> +#define VLV_GTLC_SURVIVABILITY_REG 0x130098
> +#define VLV_GFX_CLK_STATUS_BIT (1<<3)
> +#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2)
> +
> #define GEN6_GT_GFX_RC6_LOCKED 0x138104
> #define VLV_COUNTER_CONTROL 0x138104
> #define VLV_COUNT_RANGE_HIGH (1<<15)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d00a2cf..86d87e5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3035,6 +3035,51 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> trace_intel_gpu_freq_change(val * 50);
> }
>
> +/* vlv_set_rps_idle: Set the frequency to Rpe if Gfx clocks are down
> + *
> + * * If Gfx is Idle, then
> + * 1. Mask Turbo interrupts
> + * 2. Bring up Gfx clock
> + * 3. Change the freq to Rpe and wait till P-Unit updates freq
> + * 4. Clear the Force GFX CLK ON bit so that Gfx can down
> + * 5. Unmask Turbo interrupts
> +*/
> +static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> +{
> + /*
> + * When we are idle. Drop to min voltage state.
> + */
> +
> + if (dev_priv->rps.cur_delay == dev_priv->rps.rpe_delay)
> + return;
> +
> + /* Mask turbo interrupt so that they will not come in between */
> + I915_WRITE(GEN6_PMINTRMSK, 0x);
> +
> + /* Bring up the Gfx clock */
> + I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
> + I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
> + VLV_GFX_CLK_FORCE_ON_BIT);
> +
> + if (wait_for_atomic_us(((VLV_GFX_CLK_STATUS_BIT &
> + I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 500)) {
> + DRM_ERROR("GFX_CLK_ON request timed out\n");
> + return;
> + }
> +
> + valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
We're not actually waiting for Punit here. Should we?
Also valleyview_set_rps() won't actually do anything if cur_delay ==
rpe_delay. Are we required to actually poke the Punit here, or is it
enough that we had already previously requested <=rpe_delay? In that
case I think it might make sense to skip this if cur_delay <= rpe_delay.
But if we really need to poke Punit to make sure it changes the
frequency/voltage, then I guess we should force the issue by eg. setting
cur_delay=0 just before calling valleyview_set_rps().
> +
> + /* Release the Gfx clock */
> + I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
> + I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
> + ~VLV_GFX_CLK_FORCE_ON_BIT);
> +
> + /* Unmask Turbo interrupts */
> + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
> +}
> +
> +
> +
> void gen6_rps_idle(struct drm_i915_private *dev_priv)
> {
> struct drm_device *dev = dev_priv->dev;
> @@ -3042,7 +3087,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
> mutex_lock(&dev_priv->rps.hw_lock);
> if (dev_priv->rps.enabled) {
> if (IS_VALLEYVIEW(dev))
> - valleyview_set_rps(dev_priv->dev,
> dev_priv->rps.min_delay);
> + vlv_set_rps_idle(dev_priv);
> else
> gen6_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
> dev_priv->rps.last_adj = 0;
> @@ -4273,6 +4318,7 @@ void intel_gpu_ips_teardown(void)
> i915_mch_dev = NULL;
> spin_unlock_irq(&mchdev_lock);
> }
> +
> static void intel_init_emon(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> --
> 1.8.4.2
>
> ___
> 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.freedeskt
[Intel-gfx] [PATCH 2/3] drm/i915: Do not call retire_requests from wait_for_rendering
A common issue we have is that retiring requests causes recursion
through GTT manipulation or page table manipulation which we can only
handle at very specific points. However, to maintain internal
consistency (enforced through our sanity checks on write_domain at
various points in the GEM object lifecycle) we do need to retire the
object prior to marking it with a new write_domain, and also clear the
write_domain for the implicit flush following a batch.
Note that this then allows the unbound objects to still be on the active
lists, and so care must be taken when removing objects from unbound lists
(similar to the caveats we face processing the bound lists).
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem.c| 100 ++---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +
2 files changed, 64 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0e8c9bb6c3da..8912aaa7118a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -43,12 +43,15 @@ static void i915_gem_object_flush_cpu_write_domain(struct
drm_i915_gem_object *o
static __must_check int
i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
bool readonly);
+static void
+i915_gem_object_retire(struct drm_i915_gem_object *obj);
static __must_check int
i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
struct i915_address_space *vm,
unsigned alignment,
bool map_and_fenceable,
bool nonblocking);
+
static int i915_gem_phys_pwrite(struct drm_device *dev,
struct drm_i915_gem_object *obj,
struct drm_i915_gem_pwrite *args,
@@ -535,6 +538,8 @@ i915_gem_shmem_pread(struct drm_device *dev,
ret = i915_gem_object_wait_rendering(obj, true);
if (ret)
return ret;
+
+ i915_gem_object_retire(obj);
}
ret = i915_gem_object_get_pages(obj);
@@ -849,6 +854,8 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
ret = i915_gem_object_wait_rendering(obj, false);
if (ret)
return ret;
+
+ i915_gem_object_retire(obj);
}
/* Same trick applies to invalidate partially written cachelines read
* before writing. */
@@ -1238,7 +1245,8 @@ static int
i915_gem_object_wait_rendering__tail(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *ring)
{
- i915_gem_retire_requests_ring(ring);
+ if (!obj->active)
+ return 0;
/* Manually manage the write flush as we may have not yet
* retired the buffer.
@@ -1248,7 +1256,6 @@ i915_gem_object_wait_rendering__tail(struct
drm_i915_gem_object *obj,
* we know we have passed the last write.
*/
obj->last_write_seqno = 0;
- obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS;
return 0;
}
@@ -1856,58 +1863,58 @@ static unsigned long
__i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
bool purgeable_only)
{
- struct list_head still_bound_list;
- struct drm_i915_gem_object *obj, *next;
+ struct list_head still_in_list;
+ struct drm_i915_gem_object *obj;
unsigned long count = 0;
- list_for_each_entry_safe(obj, next,
-&dev_priv->mm.unbound_list,
-global_list) {
- if ((i915_gem_object_is_purgeable(obj) || !purgeable_only) &&
- i915_gem_object_put_pages(obj) == 0) {
- count += obj->base.size >> PAGE_SHIFT;
- if (count >= target)
- return count;
- }
- }
-
/*
-* As we may completely rewrite the bound list whilst unbinding
+* As we may completely rewrite the (un)bound list whilst unbinding
* (due to retiring requests) we have to strictly process only
* one element of the list at the time, and recheck the list
* on every iteration.
+*
+* In particular, we must hold a reference whilst removing the
+* object as we may end up waiting for and/or retiring the objects.
+* This might release the final reference (held by the active list)
+* and result in the object being freed from under us. This is
+* similar to the precautions the eviction code must take whilst
+* removing objects.
+*
+* Also note that although these lists do not hold a reference to
+* the object we can safely grab one here: The final object
+* unreferencing and the bound_list are both protected by the
+* dev->struct_mutex and so w
[Intel-gfx] [PATCH 3/3] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl
By exporting the ability to map user address and inserting PTEs
representing their backing pages into the GTT, we can exploit UMA in order
to utilize normal application data as a texture source or even as a
render target (depending upon the capabilities of the chipset). This has
a number of uses, with zero-copy downloads to the GPU and efficient
readback making the intermixed streaming of CPU and GPU operations
fairly efficient. This ability has many widespread implications from
faster rendering of client-side software rasterisers (chromium),
mitigation of stalls due to read back (firefox) and to faster pipelining
of texture data (such as pixel buffer objects in GL or data blobs in CL).
v2: Compile with CONFIG_MMU_NOTIFIER
v3: We can sleep while performing invalidate-range, which we can utilise
to drop our page references prior to the kernel manipulating the vma
(for either discard or cloning) and so protect normal users.
v4: Only run the invalidate notifier if the range intercepts the bo.
v5: Prevent userspace from attempting to GTT mmap non-page aligned buffers
v6: Recheck after reacquire mutex for lost mmu.
v7: Fix implicit padding of ioctl struct by rounding to next 64bit boundary.
v8: Fix rebasing error after forwarding porting the back port.
v9: Limit the userptr to page aligned entries. We now expect userspace
to handle all the offset-in-page adjustments itself.
v10: Prevent vma from being copied across fork to avoid issues with cow.
v11: Drop vma behaviour changes -- locking is nigh on impossible.
Use a worker to load user pages to avoid lock inversions.
v12: Use get_task_mm()/mmput() for correct refcounting of mm.
v13: Use a worker to release the mmu_notifier to avoid lock inversion
v14: Decouple mmu_notifier from struct_mutex using a custom mmu_notifer
with its own locking and tree of objects for each mm/mmu_notifier.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: "Gong, Zhipeng"
Cc: Akash Goel
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_dma.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 24 +-
drivers/gpu/drm/i915/i915_gem.c | 4 +
drivers/gpu/drm/i915/i915_gem_userptr.c | 553
drivers/gpu/drm/i915/i915_gpu_error.c | 2 +
include/uapi/drm/i915_drm.h | 16 +
7 files changed, 600 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 45071d18c730..1dc14e51f3bd 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -15,6 +15,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
i915_gem_gtt.o \
i915_gem_stolen.o \
i915_gem_tiling.o \
+ i915_gem_userptr.o \
i915_sysfs.o \
i915_trace_points.o \
i915_ums.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f053d1099d3b..c34f2e7ae6d9 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1919,6 +1919,7 @@ const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CREATE2, i915_gem_create2_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
};
int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 35a2e339c298..fe46be1c48cc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -163,6 +164,7 @@ enum hpd_pin {
if ((intel_encoder)->base.crtc == (__crtc))
struct drm_i915_private;
+struct i915_mmu_notifier;
enum intel_dpll_id {
DPLL_ID_PRIVATE = -1, /* non-shared dpll in use */
@@ -354,6 +356,7 @@ struct drm_i915_error_state {
u32 tiling:2;
u32 dirty:1;
u32 purgeable:1;
+ u32 userptr:1;
s32 ring:4;
u32 cache_level:3;
} **active_bo, **pinned_bo;
@@ -1465,6 +1468,9 @@ typedef struct drm_i915_private {
struct i915_gtt gtt; /* VMA representing the global address space */
struct i915_gem_mm mm;
+#if defined(CONFIG_MMU_NOTIFIER)
+ DECLARE_HASHTABLE(mmu_notifiers, 7);
+#endif
/* Kernel Modesetting */
@@ -1596,6 +1602,7 @@ struct drm_i915_gem_object_ops {
*/
int (*get_pages)(struct drm_i915_gem_object *);
void (*put_pages)(struct drm_i915_gem_object *);
+ void (*release)(struct drm_i915_gem_object *);
};
struct drm_i915_gem_object {
@@ -1709,9 +1716
Re: [Intel-gfx] [PATCH v4 1/3] drm/i915: Disable/Enable PM Intrrupts based on the current freq.
>At this point we've not yet computed the final new_delay. It would seem better
>to me to put all this code to place where we have the final new_delay.
I agree we can add this after we have the final new_delay. We can add this in
two places one in gen6_pm_rps_work before we call valleyview_set_rps or add
this inside valleyview_set_rps
Since the changes are specific to vlv. I think it is better to add it within
valleyview_set_rps before requesting the freq. Thoughts?
>Also I wonder if we should also mask the DOWN_TIMEOUT interrupt?
I think we need to do unmasking of DOWN_TIMEOUT, Idea, here is when we hit the
max_dealy we mask the UP_TIMEOUT and umask the DOWN_TIMEOUT and viceversa to
make sure we enable the interrupts based on whether we are going up or down and
avoid the interrupts that are not required..
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Tuesday, January 21, 2014 8:05 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v4 1/3] drm/i915: Disable/Enable PM Intrrupts
based on the current freq.
On Mon, Jan 20, 2014 at 06:40:24PM +0530, deepa...@intel.com wrote:
> From: Deepak S
>
> When current delay is already at max delay, Let's disable the PM UP
> THRESHOLD INTRRUPTS, so that we will not get further interrupts until
> current delay is less than max delay, Also request for the PM DOWN
> THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and
> viceversa for PM DOWN THRESHOLD INTRRUPTS.
>
> v2: Use bool variables (Daniel)
>
> v3: Fix Interrupt masking bit (Deepak)
>
> v4: Use existing symbolic constants in i915_reg.h (Daniel)
>
> Signed-off-by: Deepak S
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/gpu/drm/i915/i915_irq.c | 31 +--
> drivers/gpu/drm/i915/intel_pm.c | 3 +++
> 3 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index f888fea..e89b9f4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
> u8 rp0_delay;
> u8 hw_max;
>
> + bool rp_up_masked;
> + bool rp_down_masked;
> +
> int last_adj;
> enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c index 160d65d..d0d87ed 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -993,7 +993,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
> adj *= 2;
> else
> adj = 1;
> - new_delay = dev_priv->rps.cur_delay + adj;
> +
> + if (dev_priv->rps.cur_delay >= dev_priv->rps.max_delay) {
> + I915_WRITE(GEN6_PMINTRMSK,
> + I915_READ(GEN6_PMINTRMSK) |
> GEN6_PM_RP_UP_THRESHOLD);
> + dev_priv->rps.rp_up_masked = true;
> + new_delay = dev_priv->rps.cur_delay;
> + } else
> + new_delay = dev_priv->rps.cur_delay + adj;
> +
> + if (dev_priv->rps.rp_down_masked) {
> + I915_WRITE(GEN6_PMINTRMSK,
> + I915_READ(GEN6_PMINTRMSK) &
> ~GEN6_PM_RP_DOWN_THRESHOLD);
> + dev_priv->rps.rp_down_masked = false;
> + }
At this point we've not yet computed the final new_delay. It would seem better
to me to put all this code to place where we have the final new_delay.
Also I wonder if we should also mask the DOWN_TIMEOUT interrupt?
>
> /*
>* For better performance, jump directly @@ -1012,7 +1025,21 @@
> static void gen6_pm_rps_work(struct work_struct *work)
> adj *= 2;
> else
> adj = -1;
> - new_delay = dev_priv->rps.cur_delay + adj;
> +
> + if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay) {
> + I915_WRITE(GEN6_PMINTRMSK,
> + I915_READ(GEN6_PMINTRMSK) |
> GEN6_PM_RP_DOWN_THRESHOLD);
> + dev_priv->rps.rp_down_masked = true;
> + new_delay = dev_priv->rps.cur_delay;
> + } else
> + new_delay = dev_priv->rps.cur_delay + adj;
> +
> + if (dev_priv->rps.rp_up_masked) {
> + I915_WRITE(GEN6_PMINTRMSK,
> + I915_READ(GEN6_PMINTRMSK) &
> ~GEN6_PM_RP_UP_THRESHOLD);
> + dev_priv->rps.rp_up_masked = false;
> + }
Same comments apply.
> +
> } else { /* unknown event */
> new_delay = dev_priv->rps.cur_delay;
> }
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c index b9b4fe4..d00a2cf 100644
> ---
[Intel-gfx] [PATCH 1/3] lib: Always build and export interval_tree
lib/interval_tree.c provides a simple interface for an interval-tree (an augmented red-black tree) but is only built when testing the generic macros for building interval-trees. For drivers with modest needs, export the simple interval-tree library as is. Signed-off-by: Chris Wilson Cc: Michel Lespinasse Cc: Rik van Riel Cc: Peter Zijlstra Cc: Andrea Arcangeli Cc: David Woodhouse Cc: Andrew Morton --- lib/Makefile| 5 +++-- lib/interval_tree.c | 5 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/Makefile b/lib/Makefile index a459c31e8c6b..6b9cc0c4cc36 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -8,7 +8,8 @@ KBUILD_CFLAGS = $(subst -pg,,$(ORIG_CFLAGS)) endif lib-y := ctype.o string.o vsprintf.o cmdline.o \ -rbtree.o radix-tree.o dump_stack.o timerqueue.o\ +rbtree.o radix-tree.o interval_tree.o \ +dump_stack.o timerqueue.o\ idr.o int_sqrt.o extable.o \ sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \ proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \ @@ -152,7 +153,7 @@ lib-$(CONFIG_LIBFDT) += $(libfdt_files) obj-$(CONFIG_RBTREE_TEST) += rbtree_test.o obj-$(CONFIG_INTERVAL_TREE_TEST) += interval_tree_test.o -interval_tree_test-objs := interval_tree_test_main.o interval_tree.o +interval_tree_test-objs := interval_tree_test_main.o obj-$(CONFIG_PERCPU_TEST) += percpu_test.o diff --git a/lib/interval_tree.c b/lib/interval_tree.c index e6eb406f2d65..7b466e419740 100644 --- a/lib/interval_tree.c +++ b/lib/interval_tree.c @@ -8,3 +8,8 @@ INTERVAL_TREE_DEFINE(struct interval_tree_node, rb, unsigned long, __subtree_last, START, LAST,, interval_tree) + +EXPORT_SYMBOL(interval_tree_insert); +EXPORT_SYMBOL(interval_tree_remove); +EXPORT_SYMBOL(interval_tree_iter_first); +EXPORT_SYMBOL(interval_tree_iter_next); -- 1.8.5.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/vlv: WA for Turbo and RC6 to work together.
On Mon, Jan 20, 2014 at 06:40:26PM +0530, deepa...@intel.com wrote:
> From: Deepak S
>
> With RC6 enabled, BYT has an HW issue in determining the right
> Gfx busyness.
> WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide
> on increasing/decreasing the freq. This logic will monitor C0
> counters of render/media power-wells over EI period and takes
> necessary action based on these values
Do we have any idea what kind of performance impact this should
have?
>
> v2: resolved conflict in i915_reg.h
>
> Signed-off-by: Deepak S
> ---
> drivers/gpu/drm/i915/i915_drv.h | 13
> drivers/gpu/drm/i915/i915_irq.c | 151
> ++--
> drivers/gpu/drm/i915/i915_reg.h | 19 +
> drivers/gpu/drm/i915/intel_pm.c | 54 ++
> 4 files changed, 220 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e89b9f4..1d76461 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -942,10 +942,23 @@ struct intel_gen6_power_mgmt {
> u8 rp1_delay;
> u8 rp0_delay;
> u8 hw_max;
> + u8 hw_min;
>
> bool rp_up_masked;
> bool rp_down_masked;
>
> + u32 cz_freq;
> + u32 ei_interrupt_count;
> +
> + u32 cz_ts_up_ei;
> + u32 render_up_EI_C0;
> + u32 media_up_EI_C0;
> + u32 cz_ts_down_ei;
> + u32 render_down_EI_C0;
> + u32 media_down_EI_C0;
> +
> + bool use_RC0_residency_for_turbo;
> +
> int last_adj;
> enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d0d87ed..f4a3660 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -965,6 +965,123 @@ static void notify_ring(struct drm_device *dev,
> i915_queue_hangcheck(dev);
> }
>
> +/**
> + * vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU
> + * busy-ness calculated from C0 counters of render & media power wells
> + * @dev_priv: DRM device private
> + *
> + */
> +static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
> +{
> + u32 cz_ts = 0;
> + u32 render_count = 0, media_count = 0;
> + u32 elapsed_render = 0, elapsed_media = 0;
> + u32 elapsed_time = 0;
> + u32 residency_C0_up = 0, residency_C0_down = 0;
> + u8 new_delay;
> +
> + dev_priv->rps.ei_interrupt_count++;
> +
> + WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> +
> + cz_ts = vlv_punit_read(dev_priv, PUNIT_REG_CZ_TIMESTAMP);
> +
> + render_count = I915_READ(VLV_RENDER_C0_COUNT_REG);
> + media_count = I915_READ(VLV_MEDIA_C0_COUNT_REG);
> +
> + if (0 == dev_priv->rps.cz_ts_up_ei) {
People don't generally like this style.
> +
> + dev_priv->rps.cz_ts_up_ei = dev_priv->rps.cz_ts_down_ei = cz_ts;
> + dev_priv->rps.render_up_EI_C0 = dev_priv->rps.render_down_EI_C0
> + = render_count;
> + dev_priv->rps.media_up_EI_C0 = dev_priv->rps.media_down_EI_C0
> + = media_count;
> +
> + return dev_priv->rps.cur_delay;
> + }
> +
> + elapsed_time = cz_ts - dev_priv->rps.cz_ts_up_ei;
> + dev_priv->rps.cz_ts_up_ei = cz_ts;
> +
> + elapsed_render = render_count - dev_priv->rps.render_up_EI_C0;
> + dev_priv->rps.render_up_EI_C0 = render_count;
> +
> + elapsed_media = media_count - dev_priv->rps.media_up_EI_C0;
> + dev_priv->rps.media_up_EI_C0 = media_count;
> +
> + /* Convert all the counters into common unit of milli sec */
> + elapsed_time /= VLV_CZ_CLOCK_TO_MILLI_SEC;
> + elapsed_render /= (dev_priv->rps.cz_freq / 1000);
> + elapsed_media /= (dev_priv->rps.cz_freq / 1000);
We already have mem_freq, so cz_freq is duplicated information. Also
you're storing cz_freq in Hz and always throw away the last three
digits. I'd say either just use something like
DIV_ROUND_CLOSEST(mem_freq*1000, 4) or if the extra precision is useful,
change it so that mem_freq is in kHz from the start.
> +
> + /* Calculate overall C0 residency percentage only
> + * if elapsed time is non zero
> + */
Weird formatting. Happens more later.
> + if (elapsed_time) {
> + residency_C0_up = ((max(elapsed_render, elapsed_media)
> + * 100) / elapsed_time);
> + }
> +
> + /* To down throttle, C0 residency should be less than down threshold
> + * for continous EI intervals. So calculate down EI counters
> + * once in VLV_INT_COUNT_FOR_DOWN_EI
> + */
> + if (VLV_INT_COUNT_FOR_DOWN_EI == dev_priv->rps.ei_interrupt_count) {
Style issue again.
> +
> + dev_priv->rps.ei_interrupt_count = 0;
> +
> + elapsed_time = cz_ts - dev_priv->rps.cz_ts_down_ei;
> + dev_priv->rps.c
Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.
>We're not actually waiting for Punit here. Should we?
Ideally yes, we need to wait for the Punit to grant the freq. Based on your
suggestion on " vlv_update_rps_cur_delay" that the punit will recheck the
situation periodically, and it will try to use PUNIT_REG_GPU_FREQ_REQ. I
removed the wait form this patch.
I do believe we need to wait here. To make sure the requested freq is set
before bring down the Gfx clk.
>Also valleyview_set_rps() won't actually do anything if cur_delay ==
>rpe_delay. Are we required to actually poke the Punit here, or is it enough
>that we had already previously requested <=rpe_delay? In that case I think it
>might make sense to skip this if cur_delay <= rpe_delay.
>But if we really need to poke Punit to make sure it changes the
>frequency/voltage, then I guess we should force the issue by eg. setting
>cur_delay=0 just before calling valleyview_set_rps().
I agree, I think instead of valleyview_set_rps we can use " vlv_punit_write"
and request the freq and wait for punit to grant the freq if required.
Btw, still using outlook, I will send the next replay from new email client :)
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Tuesday, January 21, 2014 8:13 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/vlv: WA to fix Voltage not
getting dropped to Vmin when Gfx is power gated.
On Mon, Jan 20, 2014 at 06:40:25PM +0530, deepa...@intel.com wrote:
> From: Deepak S
>
> When we enter RC6 and GFX Clocks are off, the voltage remains higher
> than Vmin. When we try to set the freq to RPe, it might fail since the
> Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock
> up and set the freq to RPe then move GFx down.
>
> v2: remove vlv_update_rps_cur_delay function. Update commit message
> (Daniel)
>
> v3: Fix the timeout during wait for gfx clock (Jesse)
>
> Signed-off-by: Deepak S
> ---
> drivers/gpu/drm/i915/i915_reg.h | 4
> drivers/gpu/drm/i915/intel_pm.c | 48
> -
> 2 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index cc2f3de..e1d5f31 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4944,6 +4944,10 @@
>GEN6_PM_RP_DOWN_THRESHOLD | \
>GEN6_PM_RP_DOWN_TIMEOUT)
>
> +#define VLV_GTLC_SURVIVABILITY_REG 0x130098
> +#define VLV_GFX_CLK_STATUS_BIT (1<<3)
> +#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2)
> +
> #define GEN6_GT_GFX_RC6_LOCKED 0x138104
> #define VLV_COUNTER_CONTROL 0x138104
> #define VLV_COUNT_RANGE_HIGH (1<<15)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c index d00a2cf..86d87e5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3035,6 +3035,51 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> trace_intel_gpu_freq_change(val * 50); }
>
> +/* vlv_set_rps_idle: Set the frequency to Rpe if Gfx clocks are down
> + *
> + * * If Gfx is Idle, then
> + * 1. Mask Turbo interrupts
> + * 2. Bring up Gfx clock
> + * 3. Change the freq to Rpe and wait till P-Unit updates freq
> + * 4. Clear the Force GFX CLK ON bit so that Gfx can down
> + * 5. Unmask Turbo interrupts
> +*/
> +static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) {
> + /*
> + * When we are idle. Drop to min voltage state.
> + */
> +
> + if (dev_priv->rps.cur_delay == dev_priv->rps.rpe_delay)
> + return;
> +
> + /* Mask turbo interrupt so that they will not come in between */
> + I915_WRITE(GEN6_PMINTRMSK, 0x);
> +
> + /* Bring up the Gfx clock */
> + I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
> + I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
> + VLV_GFX_CLK_FORCE_ON_BIT);
> +
> + if (wait_for_atomic_us(((VLV_GFX_CLK_STATUS_BIT &
> + I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 500)) {
> + DRM_ERROR("GFX_CLK_ON request timed out\n");
> + return;
> + }
> +
> + valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
We're not actually waiting for Punit here. Should we?
Also valleyview_set_rps() won't actually do anything if cur_delay == rpe_delay.
Are we required to actually poke the Punit here, or is it enough that we had
already previously requested <=rpe_delay? In that case I think it might make
sense to skip this if cur_delay <= rpe_delay.
But if we really need to poke Punit to make sure it changes the
frequency/voltage, then I guess we should force the issue by eg. setting
cur_delay=0 just before calling valleyview_set_rps().
> +
> + /* Release the
[Intel-gfx] [PATCH] tests/gem_reset_stats: stop only one ring when submitting hang
If we stop all the rings, we can end up blaming the innocent
rings on hangcheck.
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=73652
Signed-off-by: Mika Kuoppala
---
tests/gem_reset_stats.c | 23 ---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index 034a298..331d954 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -237,20 +237,37 @@ static int exec_valid(int fd, int ctx)
return exec_valid_ring(fd, ctx, current_ring->exec);
}
-static void stop_rings(void)
+static void stop_rings(const int mask)
{
int fd;
+ char buf[80];
+ igt_assert((mask & ~((1 << NUM_RINGS) - 1)) == 0);
+ igt_assert(snprintf(buf, sizeof(buf), "0x%02x", mask) == 4);
fd = igt_debugfs_open(&dfs, "i915_ring_stop", O_WRONLY);
igt_assert(fd >= 0);
- igt_assert(write(fd, "0xff", 4) == 4);
+ igt_assert(write(fd, buf, 4) == 4);
close(fd);
}
#define BUFSIZE (4 * 1024)
#define ITEMS (BUFSIZE >> 2)
+static int ring_to_mask(int ring)
+{
+ for (unsigned i = 0; i < NUM_RINGS; i++) {
+ const struct target_ring *r = &rings[i];
+
+ if (r->exec == ring)
+ return (1 << i);
+ }
+
+ igt_assert(0);
+
+ return -1;
+}
+
static int inject_hang_ring(int fd, int ctx, int ring)
{
struct drm_i915_gem_execbuffer2 execbuf;
@@ -340,7 +357,7 @@ static int inject_hang_ring(int fd, int ctx, int ring)
free(buf);
- stop_rings();
+ stop_rings(ring_to_mask(ring));
return exec.handle;
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4 v2] drm/i915: Turn get_aux_clock_divider() into per-platform vfuncs
On Tue, 21 Jan 2014, Damien Lespiau wrote:
> A tiny clean-up to allow better code separation between platforms.
>
> v2: Fix comment placement (put in in i9xx_get_aux_clock_divider()) and
> nuke the outdated PCH eDP comment (Jani Nikula)
I'll take your word nothing else changed,
Reviewed-by: Jani Nikula
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/intel_dp.c | 74
>
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 2 files changed, 54 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 885d271..8a4ed4a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -353,31 +353,46 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool
> has_aux_irq)
> return status;
> }
>
> -static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp,
> - int index)
> +static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int
> index)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = intel_dig_port->base.base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
>
> - /* The clock divider is based off the hrawclk,
> - * and would like to run at 2MHz. So, take the
> - * hrawclk value and divide by 2 and use that
> - *
> - * Note that PCH attached eDP panels should use a 125MHz input
> - * clock divider.
> + /*
> + * The clock divider is based off the hrawclk, and would like to run at
> + * 2MHz. So, take the hrawclk value and divide by 2 and use that
>*/
> - if (IS_VALLEYVIEW(dev)) {
> - return index ? 0 : 100;
> - } else if (intel_dig_port->port == PORT_A) {
> - if (index)
> - return 0;
> - if (HAS_DDI(dev))
> - return
> DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv), 2000);
> - else if (IS_GEN6(dev) || IS_GEN7(dev))
> + return index ? 0 : intel_hrawclk(dev) / 2;
> +}
> +
> +static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int
> index)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> +
> + if (index)
> + return 0;
> +
> + if (intel_dig_port->port == PORT_A) {
> + if (IS_GEN6(dev) || IS_GEN7(dev))
> return 200; /* SNB & IVB eDP input clock at 400Mhz */
> else
> return 225; /* eDP input clock at 450Mhz */
> + } else {
> + return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> + }
> +}
> +
> +static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int
> index)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (intel_dig_port->port == PORT_A) {
> + if (index)
> + return 0;
> + return DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv),
> 2000);
> } else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
> /* Workaround for non-ULT HSW */
> switch (index) {
> @@ -385,13 +400,16 @@ static uint32_t get_aux_clock_divider(struct intel_dp
> *intel_dp,
> case 1: return 72;
> default: return 0;
> }
> - } else if (HAS_PCH_SPLIT(dev)) {
> + } else {
> return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> - } else {
> - return index ? 0 :intel_hrawclk(dev) / 2;
> }
> }
>
> +static uint32_t vlv_get_aux_clock_divider(struct intel_dp *intel_dp, int
> index)
> +{
> + return index ? 0 : 100;
> +}
> +
> static int
> intel_dp_aux_ch(struct intel_dp *intel_dp,
> uint8_t *send, int send_bytes,
> @@ -450,7 +468,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> goto out;
> }
>
> - while ((aux_clock_divider = get_aux_clock_divider(intel_dp, clock++))) {
> + while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp,
> clock++))) {
> /* Must try at least 3 times according to DP spec */
> for (try = 0; try < 5; try++) {
> /* Load the send data into the aux channel data
> registers */
> @@ -1613,10 +1631,12 @@ static void intel_edp_psr_enable_sink(struct intel_dp
> *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> - uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp, 0);
> + uint32_t aux_clock_divider;
> int precharge = 0x3;
> int msg_size = 5; /* Header(4) + Message(1)
[Intel-gfx] [PATCH] drm/i915: (VLV2) Fix the hotplug detection bits
These bits are in reverse order in the header from those defined in the specification. Change the bit positions for ports B and D to correctly match the spec. --- drivers/gpu/drm/i915/i915_reg.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 10ecf90..2d77b51 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2083,9 +2083,9 @@ * Please check the detailed lore in the commit message for for experimental * evidence. */ -#define PORTD_HOTPLUG_LIVE_STATUS (1 << 29) +#define PORTD_HOTPLUG_LIVE_STATUS (1 << 27) #define PORTC_HOTPLUG_LIVE_STATUS (1 << 28) -#define PORTB_HOTPLUG_LIVE_STATUS (1 << 27) +#define PORTB_HOTPLUG_LIVE_STATUS (1 << 29) #define PORTD_HOTPLUG_INT_STATUS (3 << 21) #define PORTC_HOTPLUG_INT_STATUS (3 << 19) #define PORTB_HOTPLUG_INT_STATUS (3 << 17) -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests/gem_reset_stats: stop only one ring when submitting hang
On Tue, Jan 21, 2014 at 05:40:08PM +0200, Mika Kuoppala wrote:
> If we stop all the rings, we can end up blaming the innocent
> rings on hangcheck.
>
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=73652
> Signed-off-by: Mika Kuoppala
Merged, thanks for the patch.
-Daniel
> ---
> tests/gem_reset_stats.c | 23 ---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
> index 034a298..331d954 100644
> --- a/tests/gem_reset_stats.c
> +++ b/tests/gem_reset_stats.c
> @@ -237,20 +237,37 @@ static int exec_valid(int fd, int ctx)
> return exec_valid_ring(fd, ctx, current_ring->exec);
> }
>
> -static void stop_rings(void)
> +static void stop_rings(const int mask)
> {
> int fd;
> + char buf[80];
>
> + igt_assert((mask & ~((1 << NUM_RINGS) - 1)) == 0);
> + igt_assert(snprintf(buf, sizeof(buf), "0x%02x", mask) == 4);
> fd = igt_debugfs_open(&dfs, "i915_ring_stop", O_WRONLY);
> igt_assert(fd >= 0);
>
> - igt_assert(write(fd, "0xff", 4) == 4);
> + igt_assert(write(fd, buf, 4) == 4);
> close(fd);
> }
>
> #define BUFSIZE (4 * 1024)
> #define ITEMS (BUFSIZE >> 2)
>
> +static int ring_to_mask(int ring)
> +{
> + for (unsigned i = 0; i < NUM_RINGS; i++) {
> + const struct target_ring *r = &rings[i];
> +
> + if (r->exec == ring)
> + return (1 << i);
> + }
> +
> + igt_assert(0);
> +
> + return -1;
> +}
> +
> static int inject_hang_ring(int fd, int ctx, int ring)
> {
> struct drm_i915_gem_execbuffer2 execbuf;
> @@ -340,7 +357,7 @@ static int inject_hang_ring(int fd, int ctx, int ring)
>
> free(buf);
>
> - stop_rings();
> + stop_rings(ring_to_mask(ring));
>
> return exec.handle;
> }
> --
> 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 4/4 v2] drm/i915: Introduce a get_aux_send_ctl() vfunc
On Tue, Jan 21, 2014 at 01:37:15PM +, Damien Lespiau wrote:
> We need a bit more flexibility here in the future, bits get shuffled
> around.
>
> v2: more descriptive commit message (Jani Nikula)
>
> Reviewed-by: Jani Nikula
> Signed-off-by: Damien Lespiau
Merged all to dinq, thanks for patches and review.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_dp.c | 10 ++
> drivers/gpu/drm/i915/intel_drv.h | 8
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7faf364..8936298 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -488,10 +488,10 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> }
>
> while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp,
> clock++))) {
> - u32 send_ctl = i9xx_get_aux_send_ctl(intel_dp,
> - has_aux_irq,
> - send_bytes,
> - aux_clock_divider);
> + u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> + has_aux_irq,
> + send_bytes,
> + aux_clock_divider);
>
> /* Must try at least 3 times according to DP spec */
> for (try = 0; try < 5; try++) {
> @@ -3682,6 +3682,8 @@ intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> else
> intel_dp->get_aux_clock_divider = i9xx_get_aux_clock_divider;
>
> + intel_dp->get_aux_send_ctl = i9xx_get_aux_send_ctl;
> +
> /* Preserve the current hw state. */
> intel_dp->DP = I915_READ(intel_dp->output_reg);
> intel_dp->attached_connector = intel_connector;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 6aeb62a..85bda0e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -494,6 +494,14 @@ struct intel_dp {
> struct intel_connector *attached_connector;
>
> uint32_t (*get_aux_clock_divider)(struct intel_dp *dp, int index);
> + /*
> + * This function returns the value we have to program the AUX_CTL
> + * register with to kick off an AUX transaction.
> + */
> + uint32_t (*get_aux_send_ctl)(struct intel_dp *dp,
> + bool has_aux_irq,
> + int send_bytes,
> + uint32_t aux_clock_divider);
> };
>
> struct intel_digital_port {
> --
> 1.8.3.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: (VLV2) Fix the hotplug detection bits
On Tue, 21 Jan 2014, Todd Previte wrote: > These bits are in reverse order in the header from those defined in > the specification. Change the bit positions for ports B and D to > correctly match the spec. Your signed-off-by is missing. The git commit -s option will add it for you. > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 10ecf90..2d77b51 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2083,9 +2083,9 @@ > * Please check the detailed lore in the commit message for for experimental > * evidence. > */ Please read the comment above, and check out commit 0ce99f749b3834edeb500e17d6ad17e86b60ff83 Author: Daniel Vetter Date: Fri Jul 26 11:27:49 2013 +0200 drm/i915: fix gen4 digital port hotplug definitions Since the definitions are used for both pre-PCH and VLV in g4x_dp_detect() we should try to figure out whether the bits are really different between them or not. Or if the VLV specs are confused too. Meanwhile, I'll go pat myself on the back for seeing this patch coming, and asking for Daniel to add that comment... BR, Jani. > -#define PORTD_HOTPLUG_LIVE_STATUS (1 << 29) > +#define PORTD_HOTPLUG_LIVE_STATUS (1 << 27) > #define PORTC_HOTPLUG_LIVE_STATUS (1 << 28) > -#define PORTB_HOTPLUG_LIVE_STATUS (1 << 27) > +#define PORTB_HOTPLUG_LIVE_STATUS (1 << 29) > #define PORTD_HOTPLUG_INT_STATUS (3 << 21) > #define PORTC_HOTPLUG_INT_STATUS (3 << 19) > #define PORTB_HOTPLUG_INT_STATUS (3 << 17) > -- > 1.8.3.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V2] drm/i915: (VLV2) Fix the hotplug detection bits
These bits are in reverse order in the header from those defined in the specification. Change the bit positions for ports B and D to correctly match the spec. - Added sign-off Signed-off-by: Todd Previte --- drivers/gpu/drm/i915/i915_reg.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 10ecf90..2d77b51 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2083,9 +2083,9 @@ * Please check the detailed lore in the commit message for for experimental * evidence. */ -#define PORTD_HOTPLUG_LIVE_STATUS (1 << 29) +#define PORTD_HOTPLUG_LIVE_STATUS (1 << 27) #define PORTC_HOTPLUG_LIVE_STATUS (1 << 28) -#define PORTB_HOTPLUG_LIVE_STATUS (1 << 27) +#define PORTB_HOTPLUG_LIVE_STATUS (1 << 29) #define PORTD_HOTPLUG_INT_STATUS (3 << 21) #define PORTC_HOTPLUG_INT_STATUS (3 << 19) #define PORTB_HOTPLUG_INT_STATUS (3 << 17) -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2] drm/i915: (VLV2) Fix the hotplug detection bits
On Tue, Jan 21, 2014 at 10:22:31AM -0700, Todd Previte wrote: > These bits are in reverse order in the header from those defined in > the specification. Change the bit positions for ports B and D to > correctly match the spec. > > - Added sign-off > > Signed-off-by: Todd Previte > > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 10ecf90..2d77b51 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2083,9 +2083,9 @@ > * Please check the detailed lore in the commit message for for experimental > * evidence. > */ > -#define PORTD_HOTPLUG_LIVE_STATUS (1 << 29) > +#define PORTD_HOTPLUG_LIVE_STATUS (1 << 27) > #define PORTC_HOTPLUG_LIVE_STATUS (1 << 28) > -#define PORTB_HOTPLUG_LIVE_STATUS (1 << 27) > +#define PORTB_HOTPLUG_LIVE_STATUS (1 << 29) As Jani explained, this will break g4x. And it might very well be that this particular lie has been carried forward until baytrail, so I think we must cross check Bspec first with the reality of shipping platforms. -Daniel > #define PORTD_HOTPLUG_INT_STATUS (3 << 21) > #define PORTC_HOTPLUG_INT_STATUS (3 << 19) > #define PORTB_HOTPLUG_INT_STATUS (3 << 17) > -- > 1.8.3.2 > > ___ > 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 V2] drm/i915: (VLV2) Fix the hotplug detection bits
On Tue, 21 Jan 2014, Todd Previte wrote: > These bits are in reverse order in the header from those defined in > the specification. Change the bit positions for ports B and D to > correctly match the spec. > > - Added sign-off And please also read the rest of my previous mail! :) Jani. > > Signed-off-by: Todd Previte > > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 10ecf90..2d77b51 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2083,9 +2083,9 @@ > * Please check the detailed lore in the commit message for for experimental > * evidence. > */ > -#define PORTD_HOTPLUG_LIVE_STATUS (1 << 29) > +#define PORTD_HOTPLUG_LIVE_STATUS (1 << 27) > #define PORTC_HOTPLUG_LIVE_STATUS (1 << 28) > -#define PORTB_HOTPLUG_LIVE_STATUS (1 << 27) > +#define PORTB_HOTPLUG_LIVE_STATUS (1 << 29) > #define PORTD_HOTPLUG_INT_STATUS (3 << 21) > #define PORTC_HOTPLUG_INT_STATUS (3 << 19) > #define PORTB_HOTPLUG_INT_STATUS (3 << 17) > -- > 1.8.3.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2] drm/i915: (VLV2) Fix the hotplug detection bits
On Tue, Jan 21, 2014 at 06:45:54PM +0100, Daniel Vetter wrote: > On Tue, Jan 21, 2014 at 10:22:31AM -0700, Todd Previte wrote: > > These bits are in reverse order in the header from those defined in > > the specification. Change the bit positions for ports B and D to > > correctly match the spec. > > > > - Added sign-off > > > > Signed-off-by: Todd Previte > > > > --- > > drivers/gpu/drm/i915/i915_reg.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 10ecf90..2d77b51 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -2083,9 +2083,9 @@ > > * Please check the detailed lore in the commit message for for > > experimental > > * evidence. > > */ > > -#define PORTD_HOTPLUG_LIVE_STATUS (1 << 29) > > +#define PORTD_HOTPLUG_LIVE_STATUS (1 << 27) > > #define PORTC_HOTPLUG_LIVE_STATUS (1 << 28) > > -#define PORTB_HOTPLUG_LIVE_STATUS (1 << 27) > > +#define PORTB_HOTPLUG_LIVE_STATUS (1 << 29) > > As Jani explained, this will break g4x. And it might very well be that > this particular lie has been carried forward until baytrail, so I think we > must cross check Bspec first with the reality of shipping platforms. FYI I just checked this on Jani's ELK, and it agrees with the spec. Port B is bit 29 and port C is bit 28. Both are HDMI/DVI ports on the board. No DP connectors present unfortunately. Which explains why things work since we don't check the live status on HDMI ports. -- 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 1/2] drm: share drm_add_fake_info_node
On Thu, Jan 16, 2014 at 12:42:22AM +0100, Daniel Vetter wrote:
> On Wed, Jan 15, 2014 at 12:08:19PM -0800, Ben Widawsky wrote:
> > On Wed, Jan 15, 2014 at 09:45:28AM +0100, Daniel Vetter wrote:
> > > On Wed, Jan 15, 2014 at 9:39 AM, Daniel Vetter wrote:
> > > > On Wed, Jan 15, 2014 at 12:40 AM, Russell King - ARM Linux
> > > > wrote:
> > > >> On Wed, Jan 15, 2014 at 12:25:10AM +0100, Daniel Vetter wrote:
> > > >>> On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote:
> > > >>> > Both i915 and Armada had the exact same implementation. For an
> > > >>> > upcoming
> > > >>> > patch, I'd like to call this function from two different source
> > > >>> > files in
> > > >>> > i915, and having it available externally helps there too.
> > > >>> >
> > > >>> > While moving, add 'debugfs_' to the name in order to match the
> > > >>> > other drm
> > > >>> > debugfs helper functions.
> > > >>> >
> > > >>> > Cc: linux-arm-ker...@lists.infradead.org
> > > >>> > Cc: intel-gfx@lists.freedesktop.org
> > > >>> > Signed-off-by: Ben Widawsky
> > > >>>
> > > >>> drm_debugfs_create_files in drm_debugfs.c has the almost same code
> > > >>> again.
> > > >>> Now the problem here is that the interface is a bit botched up, since
> > > >>> all
> > > >>> the users in i915 and armada actaully faile to clean up teh debugfs
> > > >>> dentry
> > > >>> if drm_add_fake_info_node.
> > > >>
> > > >> That's not correct - armada does clean up these, I think you need to
> > > >> take a closer look at the code.
> > > >>
> > > >> We do this by setting node->info_ent to the file operations structure,
> > > >> which is a unique key to the file being registered.
> > > >>
> > > >> Upon failure to create the fake node, we appropriately call
> > > >> drm_debugfs_remove_files() with the first argument being a pointer to
> > > >> the file operations. This causes drm_debugfs_remove_files() to match
> > > >> the fake entry, call debugfs_remove() on the dentry, and remove the
> > > >> node from the list, and free the node structure we allocated.
> > > >>
> > > >> Upon driver teardown, we also call drm_debugfs_remove_files() but with
> > > >> each fops which should be registered, thus cleaning up each fake node
> > > >> which was created.
> > > >>
> > > >> So, Armada does clean up these entries properly.
> > > >
> > > > Indeed I've missed that and it's just i915 that botches this. I still
> > > > think the helper would be saner if it cleans up all its leftovers in
> > > > the failure case.
> > >
> > > Ok, now I've actually page all the stuff back in - if
> > > drm_add_fake_info_node fails we don't set up a drm_info_node structure
> > > and link it anywhere. Which means drm_debugfs_remove_files won't ever
> > > find it and hence can't possibly call debugfs_remove. Which means the
> > > debugfs dentry is leaked. So I think the semantics of that new debugfs
> > > helper should get fixed to also allocate and clean up the debugfs
> > > node.
> > >
> > > I agree that i915 is even worse since it doesn't bother to clean up
> > > any debugfs files at all in the failure case.
> > > -Daniel
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> >
> > Perhaps I don't understand what you want here. The only failure path in
> > the fake entry creation does already call debugfs_remove.
> >
> > if (node == NULL) {
> > debugfs_remove(ent);
> > return -ENOMEM;
> > }
> >
> > So long as the function succeeds, the node will be findable and removable.
>
> Oh dear, I didn't see that. Still stand by my opinion though that we
> should shovel the debugfs_create_file into the helper - callers allocating
> something and then the helper freeing it (but only if it fails) is rather
> funky semantics.
> -Daniel
This actually falls apart for the one usage I originally cared about.
For CRC, we store our own info structure instead of fops as the key in
the drm debug list. Therefore, it doesn't align with the usage in ours
or other drivers. One option is to make the helper function take both a
fops as well as a key (an ugly interface if you ask me).
I've already written the patches as you wanted, so I can submit them -
it's just unfortunate that the duplication I was hoping to get rid of
will need to remain.
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 0/5] drm/dp: Introduce AUX channel infrastructure
Hi, This small series introduces some infrastructure to support AUX channels in a generic way. Drivers make use of it by embedding and filling in a struct drm_dp_aux. Various helpers can then be used to for example read from or write to the DPCD. Patch 1 adds the basic infrastructure as well as a couple of helpers to access the DPCD. The helper introduced in patch 2 can be used to obtain the link status as expected by various existing DP helpers. More convenience helpers are added in patch 3, which can come in handy during DP initialization. An AUX channel can also be used to implement I2C-over-AUX and patch 4 implements an I2C adapter that can be used with the DRM EDID helpers. Finally the Tegra eDP support in patch 5 shows how drivers can make use of this new infrastructure. Changes in v4: - fix a couple of typos in comments Changes in v3: - address comments by Jani Nikula: - keep debug and error messages in AUX helpers - read/write back-to-back registers in chunks - separate link power up and configuration - do not power up for DPCD prior to 1.1 - sleep after power up as per the spec - return number of bytes transferred - factor out some common code - reorder function arguments - fix typo in comment - address comments by Daniel Vetter: - embed i2c_adapter within struct drm_dp_aux - describe error codes Changes in v2: - reimplement I2C-over-AUX functionality to get rid of the additional layer - extract retry logic from existing drivers - add more kerneldoc comments Thierry Thierry Reding (5): drm/dp: Add AUX channel infrastructure drm/dp: Add drm_dp_dpcd_read_link_status() drm/dp: Add DisplayPort link helpers drm/dp: Allow registering AUX channels as I2C busses drm/tegra: Add eDP support .../bindings/gpu/nvidia,tegra20-host1x.txt | 42 + drivers/gpu/drm/drm_dp_helper.c| 403 +++ drivers/gpu/drm/tegra/Makefile |2 + drivers/gpu/drm/tegra/dc.h |1 + drivers/gpu/drm/tegra/dpaux.c | 558 ++ drivers/gpu/drm/tegra/dpaux.h | 87 ++ drivers/gpu/drm/tegra/drm.c| 19 +- drivers/gpu/drm/tegra/drm.h| 20 + drivers/gpu/drm/tegra/output.c |8 + drivers/gpu/drm/tegra/sor.c| 1106 drivers/gpu/drm/tegra/sor.h| 292 ++ include/drm/drm_dp_helper.h| 92 ++ 12 files changed, 2628 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/tegra/dpaux.c create mode 100644 drivers/gpu/drm/tegra/dpaux.h create mode 100644 drivers/gpu/drm/tegra/sor.c create mode 100644 drivers/gpu/drm/tegra/sor.h -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 2/5] drm/dp: Add drm_dp_dpcd_read_link_status()
The function reads the link status (6 bytes starting at offset 0x202)
from the DPCD so that it can be conveniently passed to other DPCD
helpers.
Reviewed-by: Alex Deucher
Reviewed-by: Jani Nikula
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/drm_dp_helper.c | 16
include/drm/drm_dp_helper.h | 3 +++
2 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 3e74ed93b19f..b26d46b00b05 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -456,3 +456,19 @@ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned
int offset,
size);
}
EXPORT_SYMBOL(drm_dp_dpcd_write);
+
+/**
+ * drm_dp_dpcd_read_link_status() - read DPCD link status (bytes 0x202-0x207)
+ * @aux: DisplayPort AUX channel
+ * @status: buffer to store the link status in (must be at least 6 bytes)
+ *
+ * Returns the number of bytes transferred on success or a negative error
+ * code on failure.
+ */
+int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
+u8 status[DP_LINK_STATUS_SIZE])
+{
+ return drm_dp_dpcd_read(aux, DP_LANE0_1_STATUS, status,
+ DP_LINK_STATUS_SIZE);
+}
+EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index c69c1dc1b741..8af695277a84 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -465,4 +465,7 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux
*aux,
return drm_dp_dpcd_write(aux, offset, &value, 1);
}
+int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
+u8 status[DP_LINK_STATUS_SIZE]);
+
#endif /* _DRM_DP_HELPER_H_ */
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 4/5] drm/dp: Allow registering AUX channels as I2C busses
Implements an I2C-over-AUX I2C adapter on top of the generic drm_dp_aux
infrastructure. It extracts the retry logic from existing drivers, which
should help in porting those drivers to this new helper.
Reviewed-by: Alex Deucher
Reviewed-by: Jani Nikula
Signed-off-by: Thierry Reding
---
Changes in v4:
- fix typo "bitrate" -> "bit rate"
Changes in v3:
- add back DRM_DEBUG_KMS and DRM_ERROR messages
- embed i2c_adapter within struct drm_dp_aux
- fix typo in comment
drivers/gpu/drm/drm_dp_helper.c | 183
include/drm/drm_dp_helper.h | 5 ++
2 files changed, 188 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index eea15b1414ed..dba4205de274 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -364,6 +364,13 @@ EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
*
* The .dev field should be set to a pointer to the device that implements
* the AUX channel.
+ *
+ * An AUX channel can also be used to transport I2C messages to a sink. A
+ * typical application of that is to access an EDID that's present in the
+ * sink device. The .transfer() function can also be used to execute such
+ * transactions. The drm_dp_aux_register_i2c_bus() function registers an
+ * I2C adapter that can be passed to drm_probe_ddc(). Upon removal, drivers
+ * should call drm_dp_aux_unregister_i2c_bus() to remove the I2C adapter.
*/
static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
@@ -566,3 +573,179 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct
drm_dp_link *link)
return 0;
}
+
+/*
+ * I2C-over-AUX implementation
+ */
+
+static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+ I2C_FUNC_SMBUS_READ_BLOCK_DATA |
+ I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
+ I2C_FUNC_10BIT_ADDR;
+}
+
+/*
+ * Transfer a single I2C-over-AUX message and handle various error conditions,
+ * retrying the transaction as appropriate.
+ */
+static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg
*msg)
+{
+ unsigned int retry;
+ int err;
+
+ /*
+* DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source device
+* is required to retry at least seven times upon receiving AUX_DEFER
+* before giving up the AUX transaction.
+*/
+ for (retry = 0; retry < 7; retry++) {
+ err = aux->transfer(aux, msg);
+ if (err < 0) {
+ if (err == -EBUSY)
+ continue;
+
+ DRM_DEBUG_KMS("transaction failed: %d\n", err);
+ return err;
+ }
+
+ switch (msg->reply & DP_AUX_NATIVE_REPLY_MASK) {
+ case DP_AUX_NATIVE_REPLY_ACK:
+ /*
+* For I2C-over-AUX transactions this isn't enough, we
+* need to check for the I2C ACK reply.
+*/
+ break;
+
+ case DP_AUX_NATIVE_REPLY_NACK:
+ DRM_DEBUG_KMS("native nack\n");
+ return -EREMOTEIO;
+
+ case DP_AUX_NATIVE_REPLY_DEFER:
+ DRM_DEBUG_KMS("native defer");
+ /*
+* We could check for I2C bit rate capabilities and if
+* available adjust this interval. We could also be
+* more careful with DP-to-legacy adapters where a
+* long legacy cable may force very low I2C bit rates.
+*
+* For now just defer for long enough to hopefully be
+* safe for all use-cases.
+*/
+ usleep_range(500, 600);
+ continue;
+
+ default:
+ DRM_ERROR("invalid native reply %#04x\n", msg->reply);
+ return -EREMOTEIO;
+ }
+
+ switch (msg->reply & DP_AUX_I2C_REPLY_MASK) {
+ case DP_AUX_I2C_REPLY_ACK:
+ /*
+* Both native ACK and I2C ACK replies received. We
+* can assume the transfer was successful.
+*/
+ return 0;
+
+ case DP_AUX_I2C_REPLY_NACK:
+ DRM_DEBUG_KMS("I2C nack\n");
+ return -EREMOTEIO;
+
+ case DP_AUX_I2C_REPLY_DEFER:
+ DRM_DEBUG_KMS("I2C defer\n");
+ usleep_range(400, 500);
+ continue;
+
+ default:
+ DRM_ERROR("invalid I2C reply %#04x\n", msg->reply);
+ return -EREMOTEIO;
+ }
+ }
+
+ DRM_ERRO
Re: [Intel-gfx] [PATCH v3 3/4] drm/dp: Add DisplayPort link helpers
On Fri, Jan 17, 2014 at 03:22:08PM +0200, Jani Nikula wrote:
> On Tue, 14 Jan 2014, Thierry Reding wrote:
[...]
> > +int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
> > +{
> > + u8 values[3];
> > + int err;
> > +
> > + memset(link, 0, sizeof(*link));
> > +
> > + err = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values));
> > + if (err < 0)
> > + return err;
> > +
> > + link->revision = values[0];
> > + link->rate = drm_dp_bw_code_to_link_rate(values[1]);
> > + link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
> > +
> > + if (values[2] & DP_ENHANCED_FRAME_CAP)
> > + link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
>
> Since DP_DPCD_REV == 0, you could use the #defines for the indexes (if
> you're going to send another version anyway). Ditto below for
> drm_dp_link_configure.
We write to DP_LINK_BW_SET in drm_dp_link_configure() so I don't think
we can apply the same trick there. Also I'm not sure if it's really
worth having that here. Given that it only works if we actually read
DP_DPCD_REV, the number of locations where it can be done is fairly
small and they will look asymmetric with respect to other functions
using the drm_dp_dpcd_read/write() helpers.
So unless you feel strongly I'd prefer not to do this.
> Other than that nitpick, the series looks good to me. If we face any
> issues migrating i915 on top of this, we can iron them out later on.
>
> On the series,
>
> Reviewed-by: Jani Nikula
Thanks!
Thierry
pgp5dOw9d19bF.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 3/5] drm/dp: Add DisplayPort link helpers
Add a helper to probe a DP link (read out the supported DPCD revision,
maximum rate, link count and capabilities) as well as power up the DP
link and configure it accordingly.
Reviewed-by: Alex Deucher
Reviewed-by: Jani Nikula
Signed-off-by: Thierry Reding
---
Changes in v4:
- fix a couple of typos in comments as pointed out by Alex Deucher
Changes in v3:
- split into drm_dp_link_power_up() and drm_dp_link_configure()
- do not change sink state for DPCD versions earlier than 1.1
- sleep for 1-2 ms after setting local sink to D0 state
- read and write consecutive registers where possible
- read DPCD revision when link is probed
- remove duplicate kerneldoc
drivers/gpu/drm/drm_dp_helper.c | 94 +
include/drm/drm_dp_helper.h | 17
2 files changed, 111 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index b26d46b00b05..eea15b1414ed 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -472,3 +472,97 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
DP_LINK_STATUS_SIZE);
}
EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
+
+/**
+ * drm_dp_link_probe() - probe a DisplayPort link for capabilities
+ * @aux: DisplayPort AUX channel
+ * @link: pointer to structure in which to return link capabilities
+ *
+ * The structure filled in by this function can usually be passed directly
+ * into drm_dp_link_power_up() and drm_dp_link_configure() to power up and
+ * configure the link based on the link's capabilities.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
+{
+ u8 values[3];
+ int err;
+
+ memset(link, 0, sizeof(*link));
+
+ err = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values));
+ if (err < 0)
+ return err;
+
+ link->revision = values[0];
+ link->rate = drm_dp_bw_code_to_link_rate(values[1]);
+ link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
+
+ if (values[2] & DP_ENHANCED_FRAME_CAP)
+ link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
+
+ return 0;
+}
+
+/**
+ * drm_dp_link_power_up() - power up a DisplayPort link
+ * @aux: DisplayPort AUX channel
+ * @link: pointer to a structure containing the link configuration
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link)
+{
+ u8 value;
+ int err;
+
+ /* DP_SET_POWER register is only available on DPCD v1.1 and later */
+ if (link->revision < 0x11)
+ return 0;
+
+ err = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value);
+ if (err < 0)
+ return err;
+
+ value &= ~DP_SET_POWER_MASK;
+ value |= DP_SET_POWER_D0;
+
+ err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
+ if (err < 0)
+ return err;
+
+ /*
+* According to the DP 1.1 specification, a "Sink Device must exit the
+* power saving state within 1 ms" (Section 2.5.3.1, Table 5-52, "Sink
+* Control Field" (register 0x600).
+*/
+ usleep_range(1000, 2000);
+
+ return 0;
+}
+
+/**
+ * drm_dp_link_configure() - configure a DisplayPort link
+ * @aux: DisplayPort AUX channel
+ * @link: pointer to a structure containing the link configuration
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
+{
+ u8 values[2];
+ int err;
+
+ values[0] = drm_dp_link_rate_to_bw_code(link->rate);
+ values[1] = link->num_lanes;
+
+ if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
+ values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
+
+ err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, values, sizeof(values));
+ if (err < 0)
+ return err;
+
+ return 0;
+}
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8af695277a84..c7b3c736c40a 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -291,6 +291,7 @@
#define DP_SET_POWER0x600
# define DP_SET_POWER_D00x1
# define DP_SET_POWER_D30x2
+# define DP_SET_POWER_MASK 0x3
#define DP_PSR_ERROR_STATUS 0x2006 /* XXX 1.2? */
# define DP_PSR_LINK_CRC_ERROR (1 << 0)
@@ -468,4 +469,20 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux
*aux,
int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
u8 status[DP_LINK_STATUS_SIZE]);
+/*
+ * DisplayPort link
+ */
+#define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)
+
+struct drm_dp_link {
+ unsigned char revision;
+ unsigned int rate;
+ unsigned int num_lanes;
+ uns
[Intel-gfx] [PATCH v4 1/5] drm/dp: Add AUX channel infrastructure
This is a superset of the current i2c_dp_aux bus functionality and can
be used to transfer native AUX in addition to I2C-over-AUX messages.
Helpers are provided to read and write the DPCD, either blockwise or
byte-wise. Many of the existing helpers for DisplayPort take a copy of a
portion of the DPCD and operate on that, without a way to write data
back to the DPCD (e.g. for configuration of the link).
Subsequent patches will build upon this infrastructure to provide common
functionality in a generic way.
Reviewed-by: Alex Deucher
Reviewed-by: Jani Nikula
Signed-off-by: Thierry Reding
---
Changes in v4:
- fix a typo in a comment
Changes in v3:
- reorder drm_dp_dpcd_writeb() arguments to be more intuitive
- return number of bytes transferred in drm_dp_dpcd_write()
- factor out drm_dp_dpcd_access()
- describe error codes
drivers/gpu/drm/drm_dp_helper.c | 110
include/drm/drm_dp_helper.h | 67
2 files changed, 177 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 9e978aae8972..3e74ed93b19f 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -346,3 +346,113 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw)
}
}
EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
+
+/**
+ * DOC: dp helpers
+ *
+ * The DisplayPort AUX channel is an abstraction to allow generic, driver-
+ * independent access to AUX functionality. Drivers can take advantage of
+ * this by filling in the fields of the drm_dp_aux structure.
+ *
+ * The .transfer() function is the hardware-specific implementation of how
+ * a transaction is executed on the AUX channel. A pointer to a struct
+ * drm_dp_aux_msg describing the transaction is passed into this function.
+ * Upon success, the implementation should return the number of bytes that
+ * were transferred, or a negative error-code on failure. Helpers propagate
+ * errors from the .transfer() function, with the exception of the -EBUSY
+ * error, which causes a transaction to be retried.
+ *
+ * The .dev field should be set to a pointer to the device that implements
+ * the AUX channel.
+ */
+
+static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
+ unsigned int offset, void *buffer, size_t size)
+{
+ struct drm_dp_aux_msg msg;
+ unsigned int retry;
+ int err;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.address = offset;
+ msg.request = request;
+ msg.buffer = buffer;
+ msg.size = size;
+
+ /*
+* The specification doesn't give any recommendation on how often to
+* retry native transactions, so retry 7 times like for I2C-over-AUX
+* transactions.
+*/
+ for (retry = 0; retry < 7; retry++) {
+ err = aux->transfer(aux, &msg);
+ if (err < 0) {
+ if (err == -EBUSY)
+ continue;
+
+ return err;
+ }
+
+ if (err == 0)
+ return -EPROTO;
+
+ switch (msg.reply & DP_AUX_NATIVE_REPLY_MASK) {
+ case DP_AUX_NATIVE_REPLY_ACK:
+ return err;
+
+ case DP_AUX_NATIVE_REPLY_NACK:
+ return -EIO;
+
+ case DP_AUX_NATIVE_REPLY_DEFER:
+ usleep_range(400, 500);
+ break;
+ }
+ }
+
+ DRM_ERROR("too many retries, giving up\n");
+ return -EIO;
+}
+
+/**
+ * drm_dp_dpcd_read() - read a series of bytes from the DPCD
+ * @aux: DisplayPort AUX channel
+ * @offset: address of the (first) register to read
+ * @buffer: buffer to store the register values
+ * @size: number of bytes in @buffer
+ *
+ * Returns the number of bytes transferred on success, or a negative error
+ * code on failure. -EIO is returned if the request was NAKed by the sink or
+ * if the retry count was exceeded. If no bytes were transferred, -EPROTO is
+ * returned. Errors from the underlying AUX channel transfer function, with
+ * the exception of -EBUSY (upon which the transaction will be retried), are
+ * propagated to the caller.
+ */
+ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
+void *buffer, size_t size)
+{
+ return drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, buffer,
+ size);
+}
+EXPORT_SYMBOL(drm_dp_dpcd_read);
+
+/**
+ * drm_dp_dpcd_write() - write a series of bytes to the DPCD
+ * @aux: DisplayPort AUX channel
+ * @offset: address of the (first) register to write
+ * @buffer: buffer containing the values to write
+ * @size: number of bytes in @buffer
+ *
+ * Returns the number of bytes transferred on success, or a negative error
+ * code on failure. -EIO is returned if the request was NAKed by the sink or
+ * if the retry count was exceeded. If no bytes wer
[Intel-gfx] [PATCH 5/6] drm/i915: Move forcewake debugfs setup also
The forcewake setup is eerily similar, except it needs special mode
flags. Inserting that info into our structure is trivial, and with that
forcewake easily converts to using the new interface as well.
Notice that CRC is lacking from this patch (CRC being very similar to
forcewake in code). CRC is targeted for a new file, so there is no
reason to move it.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_debugfs.c | 48 +++--
1 file changed, 14 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 72b8388..d092631 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3206,21 +3206,6 @@ static const struct file_operations i915_forcewake_fops
= {
.release = i915_forcewake_release,
};
-static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor)
-{
- struct drm_device *dev = minor->dev;
- struct dentry *ent;
-
- ent = debugfs_create_file("i915_forcewake_user",
- S_IRUSR,
- root, dev,
- &i915_forcewake_fops);
- if (!ent)
- return -ENOMEM;
-
- return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops);
-}
-
static const struct drm_info_list i915_debugfs_list[] = {
{"i915_capabilities", i915_capabilities, 0},
{"i915_gem_objects", i915_gem_object_info, 0},
@@ -3267,18 +3252,20 @@ static const struct drm_info_list i915_debugfs_list[] =
{
static const struct i915_debugfs_files {
const char *name;
const struct file_operations *fops;
+ umode_t mode;
} i915_debugfs_files[] = {
- {"i915_wedged", &i915_wedged_fops},
- {"i915_max_freq", &i915_max_freq_fops},
- {"i915_min_freq", &i915_min_freq_fops},
- {"i915_cache_sharing", &i915_cache_sharing_fops},
- {"i915_ring_stop", &i915_ring_stop_fops},
- {"i915_ring_missed_irq", &i915_ring_missed_irq_fops},
- {"i915_ring_test_irq", &i915_ring_test_irq_fops},
- {"i915_gem_drop_caches", &i915_drop_caches_fops},
- {"i915_error_state", &i915_error_state_fops},
- {"i915_next_seqno", &i915_next_seqno_fops},
- {"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
+ {"i915_wedged", &i915_wedged_fops, S_IRUGO | S_IWUSR},
+ {"i915_max_freq", &i915_max_freq_fops, S_IRUGO | S_IWUSR},
+ {"i915_min_freq", &i915_min_freq_fops, S_IRUGO | S_IWUSR},
+ {"i915_cache_sharing", &i915_cache_sharing_fops, S_IRUGO | S_IWUSR},
+ {"i915_ring_stop", &i915_ring_stop_fops, S_IRUGO | S_IWUSR},
+ {"i915_ring_missed_irq", &i915_ring_missed_irq_fops, S_IRUGO | S_IWUSR},
+ {"i915_ring_test_irq", &i915_ring_test_irq_fops, S_IRUGO | S_IWUSR},
+ {"i915_gem_drop_caches", &i915_drop_caches_fops, S_IRUGO | S_IWUSR},
+ {"i915_error_state", &i915_error_state_fops, S_IRUGO | S_IWUSR},
+ {"i915_next_seqno", &i915_next_seqno_fops, S_IRUGO | S_IWUSR},
+ {"i915_display_crc_ctl", &i915_display_crc_ctl_fops, S_IRUGO | S_IWUSR},
+ {"i915_forcewake_user", &i915_forcewake_fops, S_IRUGO},
};
void intel_display_crc_init(struct drm_device *dev)
@@ -3299,10 +3286,6 @@ int i915_debugfs_init(struct drm_minor *minor)
{
int ret, i;
- ret = i915_forcewake_create(minor->debugfs_root, minor);
- if (ret)
- return ret;
-
for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
ret = i915_pipe_crc_create(minor->debugfs_root, minor, i);
if (ret)
@@ -3313,7 +3296,7 @@ int i915_debugfs_init(struct drm_minor *minor)
ret = drm_debugfs_create_file(minor->debugfs_root, minor,
i915_debugfs_files[i].name,
i915_debugfs_files[i].fops,
- S_IRUGO | S_IWUSR);
+ i915_debugfs_files[i].mode);
if (ret)
return ret;
}
@@ -3330,9 +3313,6 @@ void i915_debugfs_cleanup(struct drm_minor *minor)
drm_debugfs_remove_files(i915_debugfs_list,
I915_DEBUGFS_ENTRIES, minor);
- drm_debugfs_remove_files((struct drm_info_list *) &i915_forcewake_fops,
-1, minor);
-
for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
struct drm_info_list *info_list =
(struct drm_info_list *)&i915_pipe_crc_data[i];
--
1.8.5.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915: Use new drm debugfs file helper
The debugfs helper duplicates the functionality used by Armada, so let's
just use that.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_debugfs.c | 25 -
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 473dda2..72b8388 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3221,24 +3221,6 @@ static int i915_forcewake_create(struct dentry *root,
struct drm_minor *minor)
return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops);
}
-static int i915_debugfs_create(struct dentry *root,
- struct drm_minor *minor,
- const char *name,
- const struct file_operations *fops)
-{
- struct drm_device *dev = minor->dev;
- struct dentry *ent;
-
- ent = debugfs_create_file(name,
- S_IRUGO | S_IWUSR,
- root, dev,
- fops);
- if (!ent)
- return -ENOMEM;
-
- return drm_add_fake_info_node(minor, ent, fops);
-}
-
static const struct drm_info_list i915_debugfs_list[] = {
{"i915_capabilities", i915_capabilities, 0},
{"i915_gem_objects", i915_gem_object_info, 0},
@@ -3328,9 +3310,10 @@ int i915_debugfs_init(struct drm_minor *minor)
}
for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
- ret = i915_debugfs_create(minor->debugfs_root, minor,
- i915_debugfs_files[i].name,
- i915_debugfs_files[i].fops);
+ ret = drm_debugfs_create_file(minor->debugfs_root, minor,
+ i915_debugfs_files[i].name,
+ i915_debugfs_files[i].fops,
+ S_IRUGO | S_IWUSR);
if (ret)
return ret;
}
--
1.8.5.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm: Create a debugfs file creation helper
DRM layer already provides a helper function to create a set of files
following a specific idiom primarily characterized by the access flags
of the file, and the file operations. This is great for writing concise
code to handle these very common cases.
This new helper function is provided to drivers that wish to change
either the access flags, or the fops for the file - in particular the
latter. This usage has become cropping up over many of the drivers.
Providing the helper is therefore here for the usual reasons.
Upcoming patches will update the callers. Currently the key is the same
as fops. This is what most callers want, the exception is when you want
to use 1 fops for multiple file nodes. I have found one case for this
(in i915), and don't feel it's a good idea to have the interface reflect
this one, currently fringe usage. In the future if more callers want to
have a separate fops and key, the interface should be extended.
v2: The original patch simply changes the way in which we wedge special
files into the normal drm node list. Daniel requested a fuller helper
function, which this patch addresses. His original claim that v1 was
buggy is unfounded.
Requested-by: Daniel Vetter
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/drm_debugfs.c | 49 +++
include/drm/drmP.h| 14 +
2 files changed, 63 insertions(+)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index b4b51d4..ea470b7 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -237,5 +237,54 @@ int drm_debugfs_cleanup(struct drm_minor *minor)
return 0;
}
+/**
+ * Helper function for DRM drivers to create a debugfs file.
+ *
+ * \param root parent directory entry for the new file
+ * \param minor minor device minor number
+ * \param name the new file's name
+ * \param fops file operations for the new file
+ * \param mode permissions for access
+ * \return zero on success, negative errno otherwise
+ *
+ * Instantiate a debugfs file with aforementioned data. The file will be added
+ * to the drm debugfs file list in the standard way, thus making cleanup
+ * possible through the normal drm_debugfs_remove_files() call.
+ *
+ * The primary motivation for this interface over the existing one is this
+ * interface provides explicit mode, and fops flags at the cost of some extra
+ * bookkeeping to be done by the driver.
+ */
+int drm_debugfs_create_file(struct dentry *root,
+ struct drm_minor *minor,
+ const char *name,
+ const struct file_operations *fops,
+ umode_t mode)
+{
+ struct drm_info_node *node;
+ struct dentry *ent;
+
+ ent = debugfs_create_file(name, mode, root, minor->dev, fops);
+ if (!ent)
+ return PTR_ERR(ent);
+
+ node = kmalloc(sizeof(*node), GFP_KERNEL);
+ if (node == NULL) {
+ debugfs_remove(ent);
+ return -ENOMEM;
+ }
+
+ node->minor = minor;
+ node->dent = ent;
+ node->info_ent = (void *)fops;
+
+ mutex_lock(&minor->debugfs_lock);
+ list_add(&node->list, &minor->debugfs_list);
+ mutex_unlock(&minor->debugfs_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_debugfs_create_file);
+
#endif /* CONFIG_DEBUG_FS */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 63eab2b..b8e2baf 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1458,6 +1458,11 @@ extern struct drm_local_map *drm_getsarea(struct
drm_device *dev);
#if defined(CONFIG_DEBUG_FS)
extern int drm_debugfs_init(struct drm_minor *minor, int minor_id,
struct dentry *root);
+extern int drm_debugfs_create_file(struct dentry *root,
+ struct drm_minor *minor,
+ const char *name,
+ const struct file_operations *fops,
+ umode_t mode);
extern int drm_debugfs_create_files(const struct drm_info_list *files,
int count, struct dentry *root,
struct drm_minor *minor);
@@ -1478,6 +1483,15 @@ static inline int drm_debugfs_create_files(const struct
drm_info_list *files,
return 0;
}
+static inline int drm_debugfs_create_file(struct dentry *root,
+ struct drm_minor *minor,
+ const char *name,
+ const struct file_operations *fops,
+ umode_t mode)
+{
+ return 0;
+}
+
static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
int count, struct drm_minor *minor)
{
--
1.8.5.3
___
Intel-gfx mailing list
Intel-gfx@list
[Intel-gfx] [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file
At almost 800 lines of code, with almost a function per platform, this
code was cluttering up the existing debugfs file, which has
historically had fairly small functions.
Patch should have no functional changes.
Unfortunately, the patch Daniel requested of me for the DRM helper
doesn't fit well with how we enable the CRC. Unlike most users of the
interface, the CRC debugfs uses a separate fops and key. Instead of
catering the interface to the one user, keep the CRC code the same as
before, but embed the function that we don't want others to use (unless
they have a good reason).
v2: Remove spurious #if 0 (Damien)
Rename to intel_display_test.c (Damien)
Inline the drm_info_fake_node_add, since the DRM helper function should
solve most users problem (Ben)
Cc: Damien Lespiau
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/Makefile | 2 +-
drivers/gpu/drm/i915/i915_debugfs.c | 790 -
drivers/gpu/drm/i915/i915_drv.h | 12 +
drivers/gpu/drm/i915/intel_display_test.c | 801 ++
4 files changed, 814 insertions(+), 791 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_display_test.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index da682cb..248ec41 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -54,7 +54,7 @@ i915-$(CONFIG_ACPI) += intel_acpi.o
i915-$(CONFIG_DRM_I915_FBDEV) += intel_fbdev.o
-i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
+i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_display_test.o
obj-$(CONFIG_DRM_I915) += i915.o
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index d092631..4c3382a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -27,8 +27,6 @@
*/
#include
-#include
-#include
#include
#include
#include
@@ -51,32 +49,6 @@ static const char *yesno(int v)
return v ? "yes" : "no";
}
-/* As the drm_debugfs_init() routines are called before dev->dev_private is
- * allocated we need to hook into the minor for release. */
-static int
-drm_add_fake_info_node(struct drm_minor *minor,
- struct dentry *ent,
- const void *key)
-{
- struct drm_info_node *node;
-
- node = kmalloc(sizeof(*node), GFP_KERNEL);
- if (node == NULL) {
- debugfs_remove(ent);
- return -ENOMEM;
- }
-
- node->minor = minor;
- node->dent = ent;
- node->info_ent = (void *) key;
-
- mutex_lock(&minor->debugfs_lock);
- list_add(&node->list, &minor->debugfs_list);
- mutex_unlock(&minor->debugfs_lock);
-
- return 0;
-}
-
static int i915_capabilities(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -2036,754 +2008,6 @@ static int i915_power_domain_info(struct seq_file *m,
void *unused)
return 0;
}
-struct pipe_crc_info {
- const char *name;
- struct drm_device *dev;
- enum pipe pipe;
-};
-
-static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
-{
- struct pipe_crc_info *info = inode->i_private;
- struct drm_i915_private *dev_priv = info->dev->dev_private;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- if (info->pipe >= INTEL_INFO(info->dev)->num_pipes)
- return -ENODEV;
-
- spin_lock_irq(&pipe_crc->lock);
-
- if (pipe_crc->opened) {
- spin_unlock_irq(&pipe_crc->lock);
- return -EBUSY; /* already open */
- }
-
- pipe_crc->opened = true;
- filep->private_data = inode->i_private;
-
- spin_unlock_irq(&pipe_crc->lock);
-
- return 0;
-}
-
-static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
-{
- struct pipe_crc_info *info = inode->i_private;
- struct drm_i915_private *dev_priv = info->dev->dev_private;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- spin_lock_irq(&pipe_crc->lock);
- pipe_crc->opened = false;
- spin_unlock_irq(&pipe_crc->lock);
-
- return 0;
-}
-
-/* (6 fields, 8 chars each, space separated (5) + '\n') */
-#define PIPE_CRC_LINE_LEN (6 * 8 + 5 + 1)
-/* account for \'0' */
-#define PIPE_CRC_BUFFER_LEN(PIPE_CRC_LINE_LEN + 1)
-
-static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc)
-{
- assert_spin_locked(&pipe_crc->lock);
- return CIRC_CNT(pipe_crc->head, pipe_crc->tail,
- INTEL_PIPE_CRC_ENTRIES_NR);
-}
-
-static ssize_t
-i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
- loff_t *pos)
-{
- struct pipe_crc_info *info = filep->private_data;
- struct drm_device *dev = info->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_pipe_crc *pipe_crc = &d
Re: [Intel-gfx] [PATCH 1/2] drm/i915: clock readout support for DDI v2
On Tue, 21 Jan 2014 13:36:56 +0200
Ville Syrjälä wrote:
> > +static int intel_ddi_calc_wrpll_link(u32 wrpll)
> > +{
> > + int n, p, r;
> > +
> > + r = wrpll & WRPLL_DIVIDER_REF_MASK;
> > + p = (wrpll & WRPLL_DIVIDER_POST_MASK) >> WRPLL_DIVIDER_POST_SHIFT;
> > + n = (wrpll & WRPLL_DIVIDER_FB_MASK) >> WRPLL_DIVIDER_FB_SHIFT;
> > +
> > + return (LC_FREQ * n) / (p * r);
>
> This is assuming the WRPLL will use the LCPLL as reference. Ideally we
> should read out the ref clock settings too.
I don't think I see that in this config, but I've added code to look
for that. Not sure if I got the ref freq right either; I think it's
135MHz in the PCH case...
> > + case PORT_CLK_SEL_SPLL:
> > + link_clock = 135000;
>
> SPLL could also output 810 MHz.
And even 2700 MHz. Fixed.
> > + break;
> > + default:
> > + WARN(1, "bad port clock sel\n");
> > + return;
> > + }
>
> Could do the port_clock = link_clock * 2; here, and then pass port clock
> to intel_dotclock_calculate() and avoid having to multiply crtc_clock by
> 2 afterwards.
Yeah that tidies things up nicely. Fixed.
> As a side note, I must say it's a bit annoying that the DDI PLL code is
> different to the rest of our PLL code. Hz vs. kHz etc. Makes it a bit
> harder to figure out what it's doing.
Yeah that could be converted, mixing them up definitely isn't ideal.
> > +
> > + if (pipe_config->has_pch_encoder)
> > + pipe_config->adjusted_mode.crtc_clock =
> > + intel_dotclock_calculate(link_clock,
> > +&pipe_config->fdi_m_n);
> > + else
>
> else if (has_dp_encoder)
>
> > + pipe_config->adjusted_mode.crtc_clock =
> > + intel_dotclock_calculate(link_clock,
> > +&pipe_config->dp_m_n);
>
> else
> .crtc_clock = link_clock; // or port_clock if you take my suggestion
> above
Fixed.
> > + intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
> > +&pipe_config->dp_m_n);
>
> Not needed. We already do intel_dp_get_m_n() in intel_ddi_get_config().
Ah right, just above. Fixed.
Thanks a lot for the review.
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: clock readout support for DDI v3
Read out and calculate the port and pixel clocks on DDI configs as well.
This means we have to grab the DP divider values and look at the port
mapping to figure out which clock select reg to read out.
v2: do the work from ddi_get_config (Ville)
v3: check WRPLL reference clock (Ville)
add additional SPLL freqs (Ville)
clean up port/crtc clock calc (Ville)
fix up crtc_clock conditionals (Ville)
drop superfluous dp_get_m_n from get_config (Ville)
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_reg.h | 10 +
drivers/gpu/drm/i915/intel_ddi.c | 92
2 files changed, 102 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a699efd..7bfc710 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5306,8 +5306,12 @@
#define SPLL_PLL_ENABLE (1<<31)
#define SPLL_PLL_SSC (1<<28)
#define SPLL_PLL_NON_SSC (2<<28)
+#define SPLL_PLL_LCPLL(3<<28)
+#define SPLL_PLL_REF_MASK (3<<28)
#define SPLL_PLL_FREQ_810MHz (0<<26)
#define SPLL_PLL_FREQ_1350MHz (1<<26)
+#define SPLL_PLL_FREQ_2700MHz (2<<26)
+#define SPLL_PLL_FREQ_MASK(3<<26)
/* WRPLL */
#define WRPLL_CTL1 0x46040
@@ -5318,8 +5322,13 @@
#define WRPLL_PLL_SELECT_LCPLL_2700 (0x03<<28)
/* WRPLL divider programming */
#define WRPLL_DIVIDER_REFERENCE(x)((x)<<0)
+#define WRPLL_DIVIDER_REF_MASK(0xff)
#define WRPLL_DIVIDER_POST(x) ((x)<<8)
+#define WRPLL_DIVIDER_POST_MASK (0x3f<<8)
+#define WRPLL_DIVIDER_POST_SHIFT 8
#define WRPLL_DIVIDER_FEEDBACK(x) ((x)<<16)
+#define WRPLL_DIVIDER_FB_SHIFT16
+#define WRPLL_DIVIDER_FB_MASK (0xff<<16)
/* Port clock selection */
#define PORT_CLK_SEL_A 0x46100
@@ -5332,6 +5341,7 @@
#define PORT_CLK_SEL_WRPLL1 (4<<29)
#define PORT_CLK_SEL_WRPLL2 (5<<29)
#define PORT_CLK_SEL_NONE (7<<29)
+#define PORT_CLK_SEL_MASK (7<<29)
/* Transcoder clock selection */
#define TRANS_CLK_SEL_A0x46140
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1488b28..b6171cf 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -633,6 +633,96 @@ static void wrpll_update_rnp(uint64_t freq2k, unsigned
budget,
/* Otherwise a < c && b >= d, do nothing */
}
+static int intel_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
+int reg)
+{
+ int refclk = LC_FREQ;
+ int n, p, r;
+ u32 wrpll;
+
+ wrpll = I915_READ(reg);
+ switch (wrpll & SPLL_PLL_REF_MASK) {
+ case SPLL_PLL_SSC:
+ case SPLL_PLL_NON_SSC:
+ /*
+* We could calculate spread here, but our checking
+* code only cares about 5% accuracy, and spread is a max of
+* 0.5% downspread.
+*/
+ refclk = 135;
+ break;
+ case SPLL_PLL_LCPLL:
+ refclk = LC_FREQ;
+ break;
+ default:
+ WARN(1, "bad wrpll refclk\n");
+ return 0;
+ }
+
+ r = wrpll & WRPLL_DIVIDER_REF_MASK;
+ p = (wrpll & WRPLL_DIVIDER_POST_MASK) >> WRPLL_DIVIDER_POST_SHIFT;
+ n = (wrpll & WRPLL_DIVIDER_FB_MASK) >> WRPLL_DIVIDER_FB_SHIFT;
+
+ return (LC_FREQ * n) / (p * r);
+}
+
+static void intel_ddi_clock_get(struct intel_encoder *encoder,
+ struct intel_crtc_config *pipe_config)
+{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ enum port port = intel_ddi_get_encoder_port(encoder);
+ int link_clock = 0;
+ u32 val, pll;
+
+ val = I915_READ(PORT_CLK_SEL(port));
+ switch (val & PORT_CLK_SEL_MASK) {
+ case PORT_CLK_SEL_LCPLL_810:
+ link_clock = 81000;
+ break;
+ case PORT_CLK_SEL_LCPLL_1350:
+ link_clock = 135000;
+ break;
+ case PORT_CLK_SEL_LCPLL_2700:
+ link_clock = 27;
+ break;
+ case PORT_CLK_SEL_WRPLL1:
+ link_clock = intel_ddi_calc_wrpll_link(dev_priv, WRPLL_CTL1);
+ break;
+ case PORT_CLK_SEL_WRPLL2:
+ link_clock = intel_ddi_calc_wrpll_link(dev_priv, WRPLL_CTL2);
+ break;
+ case PORT_CLK_SEL_SPLL:
+ pll = I915_READ(SPLL_CTL) & SPLL_PLL_FREQ_MASK;
+ if (pll == SPLL_PLL_FREQ_810MHz)
+ link_clock = 81000;
+ else if (pll == SPLL_PLL_FREQ_1350MHz)
+ link_clock = 135000;
+ else if (pll == SPLL_PLL_FREQ_2700MHz)
+ link_clock = 27;
+
Re: [Intel-gfx] [PATCH 1/2] drm: expose subpixel order name routine
On Tue, 21 Jan 2014 09:05:04 + Chris Wilson wrote: > On Mon, Jan 20, 2014 at 03:54:59PM -0800, Jesse Barnes wrote: > > Just like we have for connector type etc. > > Then we might as well take a similar defensive approach if we want to > expand the number of contexts we call it from. Since I'm not printing an identifier number into the string I can just return the one from the array, if that's what you mean. Fixed. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: expose subpixel order name routine v2
Just like we have for connector type etc.
v2: just return the string directly to avoid repeating the mistakes of
the past (Chris)
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_crtc.c | 16
include/drm/drm_crtc.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 266a01d..65f2937 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -215,6 +215,16 @@ static const struct drm_prop_enum_list
drm_encoder_enum_list[] =
{ DRM_MODE_ENCODER_DSI, "DSI" },
};
+static const struct drm_prop_enum_list drm_subpixel_enum_list[] =
+{
+ { SubPixelUnknown, "Unknown" },
+ { SubPixelHorizontalRGB, "Horizontal RGB" },
+ { SubPixelHorizontalBGR, "Horizontal BGR" },
+ { SubPixelVerticalRGB, "Vertical RGB" },
+ { SubPixelVerticalBGR, "Vertical BGR" },
+ { SubPixelNone, "None" },
+};
+
void drm_connector_ida_init(void)
{
int i;
@@ -264,6 +274,12 @@ const char *drm_get_connector_status_name(enum
drm_connector_status status)
}
EXPORT_SYMBOL(drm_get_connector_status_name);
+const char *drm_get_subpixel_order_name(enum subpixel_order order)
+{
+ return drm_subpixel_enum_list[order].name;
+}
+EXPORT_SYMBOL(drm_get_subpixel_order_name);
+
static char printable_char(int c)
{
return isascii(c) && isprint(c) ? c : '?';
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f32c5cd..271373f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -963,6 +963,7 @@ extern void drm_encoder_cleanup(struct drm_encoder
*encoder);
extern const char *drm_get_connector_name(const struct drm_connector
*connector);
extern const char *drm_get_connector_status_name(enum drm_connector_status
status);
+extern const char *drm_get_subpixel_order_name(enum subpixel_order order);
extern const char *drm_get_dpms_name(int val);
extern const char *drm_get_dvi_i_subconnector_name(int val);
extern const char *drm_get_dvi_i_select_name(int val);
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm: expose subpixel order name routine
On Tue, Jan 21, 2014 at 12:46:08PM -0800, Jesse Barnes wrote:
> On Tue, 21 Jan 2014 09:05:04 +
> Chris Wilson wrote:
>
> > On Mon, Jan 20, 2014 at 03:54:59PM -0800, Jesse Barnes wrote:
> > > Just like we have for connector type etc.
> >
> > Then we might as well take a similar defensive approach if we want to
> > expand the number of contexts we call it from.
>
> Since I'm not printing an identifier number into the string I can just
> return the one from the array, if that's what you mean. Fixed.
Heh, sounds useful as well. :)
I was thinking of the style encouraged for intel_output_name(), viz
const char *intel_subpixel_order_to_string(enum subpixel_order order) /* or
whatever it was called */
{
static const char *names[] = {
[SubPixelUnknown] = "unknown",
};
if ((unsigned)order >= ARRAY_SIZE(names) || !names[order])
return "unknown"
return names[order];
}
If we were consistent in using that approach, the code is robust against
any changes or bad input, without being too cumbersome.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file
On Tue, Jan 21, 2014 at 12:33:22PM -0800, Ben Widawsky wrote:
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1356,6 +1356,18 @@ struct intel_pipe_crc {
> wait_queue_head_t wq;
> };
>
> +struct pipe_crc_info {
> + const char *name;
> + struct drm_device *dev;
> + enum pipe pipe;
> +};
> +
> +extern struct pipe_crc_info i915_pipe_crc_data[I915_MAX_PIPES];
> +extern const struct file_operations i915_display_crc_ctl_fops;
> +
> +int i915_pipe_crc_create(struct dentry *root, struct drm_minor *minor,
> + enum pipe pipe);
> +
> typedef struct drm_i915_private {
> struct drm_device *dev;
> struct kmem_cache *slab;
Oops, I think I should have looked at that hunk before, hidden in that
big diff it wasn't obvious.
We try to put display related things into intel_drv.h, it was in
i915_drv.h because it was in debugfs.c. Also intel_drv.h and i915_drv.h
are neatly separated by file, so it'd be nice to have a separate entry
for intel_display_test.c.
Bikeshedding territory, I know, sorry. With that changed or not you have
my: Reviewed-by: Damien Lespiau
Who knows Daniel may not insist on this particular OCD.
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file
On Tue, Jan 21, 2014 at 09:44:51PM +, Damien Lespiau wrote:
> We try to put display related things into intel_drv.h, it was in
> i915_drv.h because it was in debugfs.c. Also intel_drv.h and i915_drv.h
> are neatly separated by file, so it'd be nice to have a separate entry
> for intel_display_test.c.
Also, there's another chunk in i915_drv.h you may want to move to
intel_drv.h if you decide the OCD is justifed:
#ifdef CONFIG_DEBUG_FS
void intel_display_crc_init(struct drm_device *dev);
#else
static inline void intel_display_crc_init(struct drm_device *dev) {}
#endif
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Allow reading the TIMESTAMP register on Gen8.
Nothing's changed here; we just need to bump the generation check.
Signed-off-by: Kenneth Graunke
---
drivers/gpu/drm/i915/intel_uncore.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 646fecf..c628414 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -805,7 +805,7 @@ static const struct register_whitelist {
uint32_t size;
uint32_t gen_bitmask; /* support gens, 0x10 for 4, 0x30 for 4 and 5,
etc. */
} whitelist[] = {
- { RING_TIMESTAMP(RENDER_RING_BASE), 8, 0xF0 },
+ { RING_TIMESTAMP(RENDER_RING_BASE), 8, 0x1F0 },
};
int i915_reg_read_ioctl(struct drm_device *dev,
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests: Move pm_rps to the right Makefile target
On Mon, Jan 20, 2014 at 05:14:23PM +0100, Daniel Vetter wrote: > On Mon, Jan 20, 2014 at 09:06:40AM -0600, Jeff McGee wrote: > > On Sun, Jan 19, 2014 at 02:49:19PM +0100, Daniel Vetter wrote: > > > If it's not in the multi-test target group testrunners won't pick up > > > on the fact that they need to enumerate subtests first. > > > > > > > OK. I haven't yet tried piglit, so missed this detail. Thanks > > Hm, how are you running testcases then on your side? Or are you just > running all the subtests in one go (which also works)? > -Daniel I'm just manually building and running the test binary that I'm interested in, which is just pm_rps for now. I'm working in Android for which overall support is still a little spotty I think. Or at least that was my initial perception and so I'm keeping things simple. Jeff ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] pm_rps: Remove repeat sysfs reads
From: Jeff McGee
Storing values avoids some unnecessary overhead but more importantly
allows all of our processing to be atomic.
Signed-off-by: Jeff McGee
---
tests/pm_rps.c | 98 ++
1 file changed, 51 insertions(+), 47 deletions(-)
diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index e5cbfd1..37f7020 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -35,7 +35,6 @@
#include "drmtest.h"
static bool verbose = false;
-static int origmin, origmax;
static const char sysfs_base_path[] = "/sys/class/drm/card%d/gt_%s_freq_mhz";
enum {
@@ -44,9 +43,12 @@ enum {
MAX,
RP0,
RP1,
- RPn
+ RPn,
+ NUMFREQ
};
+static int origfreqs[NUMFREQ];
+
struct junk {
const char *name;
const char *mode;
@@ -67,6 +69,14 @@ static int readval(FILE *filp)
return val;
}
+static void read_freqs(int *freqs)
+{
+ int i;
+
+ for (i = 0; i < NUMFREQ; i++)
+ freqs[i] = readval(stuff[i].filp);
+}
+
static int do_writeval(FILE *filp, int val, int lerrno)
{
int ret, orig;
@@ -90,16 +100,9 @@ static int do_writeval(FILE *filp, int val, int lerrno)
#define writeval(filp, val) do_writeval(filp, val, 0)
#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL)
-#define fcur (readval(stuff[CUR].filp))
-#define fmin (readval(stuff[MIN].filp))
-#define fmax (readval(stuff[MAX].filp))
-#define frp0 (readval(stuff[RP0].filp))
-#define frp1 (readval(stuff[RP1].filp))
-#define frpn (readval(stuff[RPn].filp))
-
static void setfreq(int val)
{
- if (val > fmax) {
+ if (val > readval(stuff[MAX].filp)) {
writeval(stuff[MAX].filp, val);
writeval(stuff[MIN].filp, val);
} else {
@@ -108,42 +111,41 @@ static void setfreq(int val)
}
}
-static void checkit(void)
+static void checkit(const int *freqs)
{
- igt_assert(fmin <= fmax);
- igt_assert(fcur <= fmax);
- igt_assert(fmin <= fcur);
- igt_assert(frpn <= fmin);
- igt_assert(fmax <= frp0);
- igt_assert(frp1 <= frp0);
- igt_assert(frpn <= frp1);
- igt_assert(frp0 != 0);
- igt_assert(frp1 != 0);
+ igt_assert(freqs[MIN] <= freqs[MAX]);
+ igt_assert(freqs[CUR] <= freqs[MAX]);
+ igt_assert(freqs[MIN] <= freqs[CUR]);
+ igt_assert(freqs[RPn] <= freqs[MIN]);
+ igt_assert(freqs[MAX] <= freqs[RP0]);
+ igt_assert(freqs[RP1] <= freqs[RP0]);
+ igt_assert(freqs[RPn] <= freqs[RP1]);
+ igt_assert(freqs[RP0] != 0);
+ igt_assert(freqs[RP1] != 0);
}
-static void dumpit(void)
+static void dumpit(const int *freqs)
{
- struct junk *junk = stuff;
- do {
- printf("gt frequency %s (MHz): %d\n", junk->name,
readval(junk->filp));
- junk++;
- } while(junk->name != NULL);
+ int i;
+
+ for (i = 0; i < NUMFREQ; i++)
+ printf("gt frequency %s (MHz): %d\n", stuff[i].name, freqs[i]);
printf("\n");
}
-#define dump() if (verbose) dumpit()
+#define dump(x) if (verbose) dumpit(x)
#define log(...) if (verbose) printf(__VA_ARGS__)
static void min_max_config(void (*check)(void))
{
- int fmid = (frpn + frp0) / 2;
+ int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
log("Check original min and max...\n");
check();
log("Set min=RPn and max=RP0...\n");
- writeval(stuff[MIN].filp, frpn);
- writeval(stuff[MAX].filp, frp0);
+ writeval(stuff[MIN].filp, origfreqs[RPn]);
+ writeval(stuff[MAX].filp, origfreqs[RP0]);
check();
log("Increase min to midpoint...\n");
@@ -151,15 +153,15 @@ static void min_max_config(void (*check)(void))
check();
log("Increase min to RP0...\n");
- writeval(stuff[MIN].filp, frp0);
+ writeval(stuff[MIN].filp, origfreqs[RP0]);
check();
log("Increase min above RP0 (invalid)...\n");
- writeval_inval(stuff[MIN].filp, frp0 + 1000);
+ writeval_inval(stuff[MIN].filp, origfreqs[RP0] + 1000);
check();
log("Decrease max to RPn (invalid)...\n");
- writeval_inval(stuff[MAX].filp, frpn);
+ writeval_inval(stuff[MAX].filp, origfreqs[RPn]);
check();
log("Decrease min to midpoint...\n");
@@ -167,7 +169,7 @@ static void min_max_config(void (*check)(void))
check();
log("Decrease min to RPn...\n");
- writeval(stuff[MIN].filp, frpn);
+ writeval(stuff[MIN].filp, origfreqs[RPn]);
check();
log("Decrease min below RPn (invalid)...\n");
@@ -179,7 +181,7 @@ static void min_max_config(void (*check)(void))
check();
log("Decrease max to RPn...\n");
- writeval(stuff[MAX].filp, frpn);
+ writeval(stuff[MAX].filp, origfreqs[RPn]);
check();
log("Decrease max below RPn (invalid)...\n");
@@ -187,7 +189,7 @@ static void min_max_config(void (*check)(void
[Intel-gfx] Expansion of pm_rps subtest min-max-config-at-idle
From: Jeff McGee Fill out the subtest with more min/max combinations and check that current frequency tracks with minimum after each perturbation. Jeff McGee (4): pm_rps: Expand on min and max config testing pm_rps: Remove repeat sysfs reads pm_rps: Make frequency logging more compact pm_rps: Require that cur reaches min at idle tests/pm_rps.c | 205 +++-- 1 file changed, 142 insertions(+), 63 deletions(-) -- 1.8.5.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] pm_rps: Require that cur reaches min at idle
From: Jeff McGee
The current frequency should reach the minimum frequency within a
reasonable time during idle. We hold forcewake to prevent interference
from sleep states.
Signed-off-by: Jeff McGee
---
tests/pm_rps.c | 34 ++
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 7ae0438..50c66ee 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -33,6 +33,7 @@
#include
#include
#include "drmtest.h"
+#include "igt_debugfs.h"
static bool verbose = false;
@@ -57,6 +58,9 @@ struct junk {
{ "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, {
"RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { NULL, NULL,
NULL }
};
+static igt_debugfs_t dfs;
+static FILE *forcewake;
+
static int readval(FILE *filp)
{
int val;
@@ -206,17 +210,33 @@ static void min_max_config(void (*check)(void))
check();
}
+#define IDLE_WAIT_TIMESTEP_MSEC 100
+#define IDLE_WAIT_TIMEOUT_MSEC 3000
static void idle_check(void)
{
int freqs[NUMFREQ];
-
- read_freqs(freqs);
- dump(freqs);
- checkit(freqs);
+ int wait = 0;
+
+ /* Monitor frequencies until cur settles down to min, which should
+* happen within the allotted time */
+ do {
+ read_freqs(freqs);
+ dump(freqs);
+ checkit(freqs);
+ if (freqs[CUR] == freqs[MIN])
+ break;
+ usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC);
+ wait += IDLE_WAIT_TIMESTEP_MSEC;
+ } while (wait < IDLE_WAIT_TIMEOUT_MSEC);
+
+ igt_assert(freqs[CUR] == freqs[MIN]);
+ log("Required %d msec to reach cur=min\n", wait);
}
static void pm_rps_exit_handler(int sig)
{
+ fclose(forcewake);
+
if (origfreqs[MIN] > readval(stuff[MAX].filp)) {
writeval(stuff[MAX].filp, origfreqs[MAX]);
writeval(stuff[MIN].filp, origfreqs[MIN]);
@@ -287,6 +307,12 @@ int main(int argc, char **argv)
read_freqs(origfreqs);
+ /* Hold forcewake throughout test to prevent sleep states from
+* interfering with evaluation of performance state management
*/
+ igt_require(igt_debugfs_init(&dfs) == 0);
+ forcewake = igt_debugfs_fopen(&dfs, "i915_forcewake_user", "r");
+ igt_require(forcewake);
+
igt_install_exit_handler(pm_rps_exit_handler);
}
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] pm_rps: Expand on min and max config testing
From: Jeff McGee
Add a function that methodically varies min and max to exercise
several valid and invalid combinations. Allow the caller to
define what is to be checked between each step.
Signed-off-by: Jeff McGee
---
tests/pm_rps.c | 106 +
1 file changed, 77 insertions(+), 29 deletions(-)
diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index f7f119c..e5cbfd1 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -134,6 +134,81 @@ static void dumpit(void)
#define dump() if (verbose) dumpit()
#define log(...) if (verbose) printf(__VA_ARGS__)
+static void min_max_config(void (*check)(void))
+{
+ int fmid = (frpn + frp0) / 2;
+
+ log("Check original min and max...\n");
+ check();
+
+ log("Set min=RPn and max=RP0...\n");
+ writeval(stuff[MIN].filp, frpn);
+ writeval(stuff[MAX].filp, frp0);
+ check();
+
+ log("Increase min to midpoint...\n");
+ writeval(stuff[MIN].filp, fmid);
+ check();
+
+ log("Increase min to RP0...\n");
+ writeval(stuff[MIN].filp, frp0);
+ check();
+
+ log("Increase min above RP0 (invalid)...\n");
+ writeval_inval(stuff[MIN].filp, frp0 + 1000);
+ check();
+
+ log("Decrease max to RPn (invalid)...\n");
+ writeval_inval(stuff[MAX].filp, frpn);
+ check();
+
+ log("Decrease min to midpoint...\n");
+ writeval(stuff[MIN].filp, fmid);
+ check();
+
+ log("Decrease min to RPn...\n");
+ writeval(stuff[MIN].filp, frpn);
+ check();
+
+ log("Decrease min below RPn (invalid)...\n");
+ writeval_inval(stuff[MIN].filp, 0);
+ check();
+
+ log("Decrease max to midpoint...\n");
+ writeval(stuff[MAX].filp, fmid);
+ check();
+
+ log("Decrease max to RPn...\n");
+ writeval(stuff[MAX].filp, frpn);
+ check();
+
+ log("Decrease max below RPn (invalid)...\n");
+ writeval_inval(stuff[MAX].filp, 0);
+ check();
+
+ log("Increase min to RP0 (invalid)...\n");
+ writeval_inval(stuff[MIN].filp, frp0);
+ check();
+
+ log("Increase max to midpoint...\n");
+ writeval(stuff[MAX].filp, fmid);
+ check();
+
+ log("Increase max to RP0...\n");
+ writeval(stuff[MAX].filp, frp0);
+ check();
+
+ log("Increase max above RP0 (invalid)...\n");
+ writeval_inval(stuff[MAX].filp, frp0 + 1000);
+ check();
+}
+
+static void idle_check(void)
+{
+ dump();
+ checkit();
+}
+
static void pm_rps_exit_handler(int sig)
{
if (origmin > fmax) {
@@ -210,35 +285,8 @@ int main(int argc, char **argv)
igt_install_exit_handler(pm_rps_exit_handler);
}
- igt_subtest("min-max-config-at-idle") {
- log("Original min = %d\nOriginal max = %d\n", origmin, origmax);
-
- dump();
-
- checkit();
- setfreq(origmin);
- dump();
- igt_assert(fcur == fmin);
- setfreq(origmax);
- dump();
- igt_assert(fcur == fmax);
- checkit();
-
- /* And some errors */
- writeval_inval(stuff[MIN].filp, frpn - 1);
- writeval_inval(stuff[MAX].filp, frp0 + 1000);
- checkit();
-
- writeval_inval(stuff[MIN].filp, fmax + 1000);
- writeval_inval(stuff[MAX].filp, fmin - 1);
- checkit();
-
- writeval_inval(stuff[MIN].filp, 0x1110);
- writeval_inval(stuff[MAX].filp, 0);
-
- writeval(stuff[MIN].filp, origmin);
- writeval(stuff[MAX].filp, origmax);
- }
+ igt_subtest("min-max-config-at-idle")
+ min_max_config(idle_check);
igt_exit();
}
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] pm_rps: Make frequency logging more compact
From: Jeff McGee
Signed-off-by: Jeff McGee
---
tests/pm_rps.c | 35 ++-
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 37f7020..7ae0438 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -128,8 +128,9 @@ static void dumpit(const int *freqs)
{
int i;
+ printf("gt freq (MHz):");
for (i = 0; i < NUMFREQ; i++)
- printf("gt frequency %s (MHz): %d\n", stuff[i].name, freqs[i]);
+ printf(" %s=%d", stuff[i].name, freqs[i]);
printf("\n");
}
@@ -140,67 +141,67 @@ static void min_max_config(void (*check)(void))
{
int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
- log("Check original min and max...\n");
+ log("\nCheck original min and max...\n");
check();
- log("Set min=RPn and max=RP0...\n");
+ log("\nSet min=RPn and max=RP0...\n");
writeval(stuff[MIN].filp, origfreqs[RPn]);
writeval(stuff[MAX].filp, origfreqs[RP0]);
check();
- log("Increase min to midpoint...\n");
+ log("\nIncrease min to midpoint...\n");
writeval(stuff[MIN].filp, fmid);
check();
- log("Increase min to RP0...\n");
+ log("\nIncrease min to RP0...\n");
writeval(stuff[MIN].filp, origfreqs[RP0]);
check();
- log("Increase min above RP0 (invalid)...\n");
+ log("\nIncrease min above RP0 (invalid)...\n");
writeval_inval(stuff[MIN].filp, origfreqs[RP0] + 1000);
check();
- log("Decrease max to RPn (invalid)...\n");
+ log("\nDecrease max to RPn (invalid)...\n");
writeval_inval(stuff[MAX].filp, origfreqs[RPn]);
check();
- log("Decrease min to midpoint...\n");
+ log("\nDecrease min to midpoint...\n");
writeval(stuff[MIN].filp, fmid);
check();
- log("Decrease min to RPn...\n");
+ log("\nDecrease min to RPn...\n");
writeval(stuff[MIN].filp, origfreqs[RPn]);
check();
- log("Decrease min below RPn (invalid)...\n");
+ log("\nDecrease min below RPn (invalid)...\n");
writeval_inval(stuff[MIN].filp, 0);
check();
- log("Decrease max to midpoint...\n");
+ log("\nDecrease max to midpoint...\n");
writeval(stuff[MAX].filp, fmid);
check();
- log("Decrease max to RPn...\n");
+ log("\nDecrease max to RPn...\n");
writeval(stuff[MAX].filp, origfreqs[RPn]);
check();
- log("Decrease max below RPn (invalid)...\n");
+ log("\nDecrease max below RPn (invalid)...\n");
writeval_inval(stuff[MAX].filp, 0);
check();
- log("Increase min to RP0 (invalid)...\n");
+ log("\nIncrease min to RP0 (invalid)...\n");
writeval_inval(stuff[MIN].filp, origfreqs[RP0]);
check();
- log("Increase max to midpoint...\n");
+ log("\nIncrease max to midpoint...\n");
writeval(stuff[MAX].filp, fmid);
check();
- log("Increase max to RP0...\n");
+ log("\nIncrease max to RP0...\n");
writeval(stuff[MAX].filp, origfreqs[RP0]);
check();
- log("Increase max above RP0 (invalid)...\n");
+ log("\nIncrease max above RP0 (invalid)...\n");
writeval_inval(stuff[MAX].filp, origfreqs[RP0] + 1000);
check();
}
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] [trivial] drm/i915: Remove incorrect comment about struct mutex
This statenment became false here:
commit 4fc688ce79772496503d22263d61b071a8fb596e
Author: Jesse Barnes
Date: Fri Nov 2 11:14:01 2012 -0700
drm/i915: protect RPS/RC6 related accesses (including PCU) with a new mutex
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f344842..d568de6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -935,8 +935,6 @@ struct intel_gen6_power_mgmt {
struct work_struct work;
u32 pm_iir;
- /* The below variables an all the rps hw state are protected by
-* dev->struct mutext. */
u8 cur_delay;
u8 min_delay;
u8 max_delay;
--
1.8.5.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] ACPI / video: Add systems that should favor native backlight interface
On 01/21/2014 08:11 PM, Matthew Garrett wrote: > On Tue, 2014-01-21 at 13:32 +0800, Aaron Lu wrote: >> On 01/21/2014 11:17 AM, Matthew Garrett wrote: >>> We know that Windows 8 graphics drivers don't use the ACPI interface, >>> and that systems change their behaviour as a result, in some cases with >>> absolutely no way for the ACPI interface could possibly work. I haven't >>> seen any cases where that's obviously true for any non-Windows 8 >> >> Perhaps I'm not clear, I didn't mean non-Windows 8 systems will all favor >> GPU's interface, I just meant for one specific win7 laptop I could re-use >> the existing code to make the GPU's interface as the only one left. And to >> achieve this, the Win8 OSI check in acpi_video_verify_backlight_support >> has to be gone. > > We could do that, but why do we think that's the correct fix? The plan > is to remove the native list entirely and do this for all Windows 8 What do you mean by native list, systems that are Win8 based but don't work with the GPU's backlight interface? If so, it doesn't seem we have such a native list now and if we don't make use_native_backlight=1 by default, we couldn't generate such a list to start with... BTW, it doesn't seem everyone agrees with this plan. May I ask, Rafael and Daniel, what's your opinion please? We need to agree with something to move things forward. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: manual merge of the drm-intel tree with the drm tree
Hi all,
Today's linux-next merge of the drm-intel tree got a conflict in
drivers/gpu/drm/i915/i915_irq.c between commit abca9e454498 ("drm: Pass
'flags' from the caller to .get_scanout_position()") from the drm tree
and commit d59a63ad8234 ("drm/i915: Add intel_get_crtc_scanline()") from
the drm-intel tree.
I fixed it up (I think - see below) and can carry the fix as necessary
(no action is required).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
diff --cc drivers/gpu/drm/i915/i915_irq.c
index 17d8fcb1b6f7,ffb56a9db9cc..
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@@ -649,8 -675,9 +649,9 @@@ static bool ilk_pipe_in_vblank_locked(s
}
static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
- int *vpos, int *hpos,
+ unsigned int flags, int *vpos, int *hpos,
- ktime_t *stime, ktime_t *etime)
+ ktime_t *stime, ktime_t *etime,
+ bool adjust)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
@@@ -788,6 -786,24 +791,24 @@@
return ret;
}
+ static int i915_get_scanout_position(struct drm_device *dev, int pipe,
+int *vpos, int *hpos,
+ktime_t *stime, ktime_t *etime)
+ {
- return i915_get_crtc_scanoutpos(dev, pipe, vpos, hpos,
++ return i915_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
+ stime, etime, true);
+ }
+
+ int intel_get_crtc_scanline(struct drm_crtc *crtc)
+ {
+ int vpos = 0, hpos = 0;
+
- i915_get_crtc_scanoutpos(crtc->dev, to_intel_crtc(crtc)->pipe,
++ i915_get_crtc_scanoutpos(crtc->dev, to_intel_crtc(crtc)->pipe, 0,
+&vpos, &hpos, NULL, NULL, false);
+
+ return vpos;
+ }
+
static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
int *max_error,
struct timeval *vblank_time,
pgpWxXZ5oi8Gw.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: manual merge of the drm-intel tree with the drm tree
Hi all,
Today's linux-next merge of the drm-intel tree got a conflict in
drivers/gpu/drm/i915/intel_display.c between commit c326c0a9c98c
("drm/i915: Call drm_calc_timestamping_constants() earlier") from the drm
tree and commit bbee18af2a25 ("drm/i915: Prepare to track new pipe config
per pipe") from the drm-intel tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
diff --cc drivers/gpu/drm/i915/intel_display.c
index 14b024becb91,e1d3ae1212a7..
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@@ -9660,14 -9705,7 +9703,15 @@@ static int __intel_set_mode(struct drm_
/* mode_set/enable/disable functions rely on a correct pipe
* config. */
to_intel_crtc(crtc)->config = *pipe_config;
+ to_intel_crtc(crtc)->new_config = &to_intel_crtc(crtc)->config;
+
+ /*
+ * Calculate and store various constants which
+ * are later needed by vblank and swap-completion
+ * timestamping. They are derived from true hwmode.
+ */
+ drm_calc_timestamping_constants(crtc,
+ &pipe_config->adjusted_mode);
}
/* Only after disabling all output pipelines that will be changed can we
pgpMdgFdRZ13Z.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915/vlv: Added 3 rendering specific Hw Workarounds in clock gating fn
From: Akash Goel Added 2 new rendering specific Workarounds 1. WaDisable_RenderCache_OperationalFlush Operational flush cannot be enabled on BWG A0 [Errata BWT006] 2. WaVSThreadDispatchOverride Performance optimization - Hw will decide which half slice the thread will dispatch, May not be really needed for VLV, as its single slice Modified the implementation of 1 workaround 1. WaDisableL3Bank2xClockGate Disabling L3 clock gating- MMIO 940c[25] = 1 Signed-off-by: Akash Goel --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 22 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a699efd..d829754 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -934,6 +934,9 @@ #define ECO_GATING_CX_ONLY (1<<3) #define ECO_FLIP_DONE(1<<0) +#define GEN7_CACHE_MODE_0 0x07000 /* IVB+ only */ +#define GEN7_RC_OP_FLUSH_ENABLE (1<<0) + #define CACHE_MODE_1 0x7004 /* IVB+ */ #define PIXEL_SUBSPAN_COLLECT_OPT_DISABLE (1<<6) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 469170c..4c36ff8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4955,6 +4955,12 @@ static void valleyview_init_clock_gating(struct drm_device *dev) I915_WRITE(GEN7_L3CNTLREG1, I915_READ(GEN7_L3CNTLREG1) | GEN7_L3AGDIS); I915_WRITE(GEN7_L3_CHICKEN_MODE_REGISTER, GEN7_WA_L3_CHICKEN_MODE); + /* WaDisable_RenderCache_OperationalFlush +* Clear bit 0, so we do a AND with the mask +* to keep other bits the same */ + I915_WRITE(GEN7_CACHE_MODE_0, (I915_READ(GEN7_CACHE_MODE_0) | + _MASKED_BIT_DISABLE(GEN7_RC_OP_FLUSH_ENABLE))); + /* WaForceL3Serialization:vlv */ I915_WRITE(GEN7_L3SQCREG4, I915_READ(GEN7_L3SQCREG4) & ~L3SQ_URB_READ_CAM_MATCH_DISABLE); @@ -4991,10 +4997,24 @@ static void valleyview_init_clock_gating(struct drm_device *dev) GEN6_RCPBUNIT_CLOCK_GATE_DISABLE | GEN6_RCCUNIT_CLOCK_GATE_DISABLE); - I915_WRITE(GEN7_UCGCTL4, GEN7_L3BANK2X_CLOCK_GATE_DISABLE); + /* WaDisableL3Bank2xClockGate +* Disabling L3 clock gating- MMIO 940c[25] = 1 +* Set bit 25, to disable L3_BANK_2x_CLK_GATING */ + I915_WRITE(GEN7_UCGCTL4, + I915_READ(GEN7_UCGCTL4) | GEN7_L3BANK2X_CLOCK_GATE_DISABLE); I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE); + /* WaVSThreadDispatchOverride +* Hw will decide which half slice the thread will dispatch. +* May not be needed for VLV, as its a single slice */ + I915_WRITE(GEN7_CACHE_MODE_0, + I915_READ(GEN7_FF_THREAD_MODE) & + (~GEN7_FF_VS_SCHED_LOAD_BALANCE)); + + /* WaDisable4x2SubspanOptimization, +* Disable combining of two 2x2 subspans into a 4x2 subspan +* Set chicken bit to disable subspan optimization */ I915_WRITE(CACHE_MODE_1, _MASKED_BIT_ENABLE(PIXEL_SUBSPAN_COLLECT_OPT_DISABLE)); -- 1.8.5.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/6] Rendering specific Hw workarounds for VLV
From: Akash Goel The following patches leads to stable behavior on VLV, especially when playing 3D Apps, benchmarks. Akash Goel (6): drm/i915/vlv: Added a rendering specific Hw WA 'WaTlbInvalidateStoreDataBefore' drm/i915/vlv: Added a rendering specific Hw WA 'WaReadAfterWriteHazard' drm/i915/vlv: Modified the programming of 2 regs in Ring initialisation drm/i915/vlv: Added 3 rendering specific Hw Workarounds in clock gating fn drm/i915/vlv: Removed 3 rendering specific Hw WA from clock gating fn drm/i915/vlv: Added a rendering specific Hw WA 'WaSendDummy3dPrimitveAfterSetContext' drivers/gpu/drm/i915/i915_gem_context.c | 64 +++- drivers/gpu/drm/i915/i915_reg.h | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 32 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 75 ++--- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 5 files changed, 160 insertions(+), 18 deletions(-) -- 1.8.5.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915/vlv: Added a rendering specific Hw WA 'WaReadAfterWriteHazard'
From: Akash Goel
Added a new rendering specific Workaround 'WaReadAfterWriteHazard'.
In this WA, need to add 12 MI Store Dword commands to ensure proper
flush of h/w pipeline.
Signed-off-by: Akash Goel
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 133d273..e8ec536 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2167,6 +2167,31 @@ intel_ring_flush_all_caches(struct intel_ring_buffer
*ring)
trace_i915_gem_ring_flush(ring, 0, I915_GEM_GPU_DOMAINS);
+ if (IS_VALLEYVIEW(ring->dev)) {
+ /*
+* WaReadAfterWriteHazard
+* Send a number of Store Data commands here to finish
+* flushing hardware pipeline.This is needed in the case
+* where the next workload tries reading from the same
+* surface that this batch writes to. Without these StoreDWs,
+* not all of the data will actually be flushd to the surface
+* by the time the next batch starts reading it, possibly
+* causing a small amount of corruption.
+*/
+ int i;
+ ret = intel_ring_begin(ring, 4 * 12);
+ if (ret)
+ return ret;
+ for (i = 0; i < 12; i++) {
+ intel_ring_emit(ring, MI_STORE_DWORD_INDEX);
+ intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_INDEX <<
+
MI_STORE_DWORD_INDEX_SHIFT);
+ intel_ring_emit(ring, 0);
+ intel_ring_emit(ring, MI_NOOP);
+ }
+ intel_ring_advance(ring);
+ }
+
ring->gpu_caches_dirty = false;
return 0;
}
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] drm/i915/vlv: Removed 3 rendering specific Hw WA from clock gating fn
From: Akash Goel Removed 3 workarounds as not needed for VLV+(B0 onwards) 1. WaDisableRHWOOptimizationForRenderHang 2. WaDisableL3CacheAging 3. WaDisableDopClockGating Signed-off-by: Akash Goel --- drivers/gpu/drm/i915/intel_pm.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4c36ff8..e4d220c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4947,12 +4947,6 @@ static void valleyview_init_clock_gating(struct drm_device *dev) _MASKED_BIT_ENABLE(GEN7_MAX_PS_THREAD_DEP | GEN7_PSD_SINGLE_PORT_DISPATCH_ENABLE)); - /* Apply the WaDisableRHWOOptimizationForRenderHang:vlv workaround. */ - I915_WRITE(GEN7_COMMON_SLICE_CHICKEN1, - GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC); - - /* WaApplyL3ControlAndL3ChickenMode:vlv */ - I915_WRITE(GEN7_L3CNTLREG1, I915_READ(GEN7_L3CNTLREG1) | GEN7_L3AGDIS); I915_WRITE(GEN7_L3_CHICKEN_MODE_REGISTER, GEN7_WA_L3_CHICKEN_MODE); /* WaDisable_RenderCache_OperationalFlush @@ -4965,10 +4959,6 @@ static void valleyview_init_clock_gating(struct drm_device *dev) I915_WRITE(GEN7_L3SQCREG4, I915_READ(GEN7_L3SQCREG4) & ~L3SQ_URB_READ_CAM_MATCH_DISABLE); - /* WaDisableDopClockGating:vlv */ - I915_WRITE(GEN7_ROW_CHICKEN2, - _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); - /* This is required by WaCatErrorRejectionIssue:vlv */ I915_WRITE(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG, I915_READ(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG) | -- 1.8.5.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915/vlv: Modified the programming of 2 regs in Ring initialisation
From: Akash Goel
Modified programming of following 2 regs in Render ring initialisation fn.
1. GFX_MODE_GEN7 (Enabling TLB invalidate)
2. MI_MODE (Enabling MI Flush)
Signed-off-by: Akash Goel
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e8ec536..8b99df2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -563,7 +563,9 @@ static int init_render_ring(struct intel_ring_buffer *ring)
int ret = init_ring_common(ring);
if (INTEL_INFO(dev)->gen > 3)
- I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH));
+ if (!IS_VALLEYVIEW(dev))
+ I915_WRITE(MI_MODE,
+ _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH));
/* We need to disable the AsyncFlip performance optimisations in order
* to use MI_WAIT_FOR_EVENT within the CS. It should already be
@@ -579,10 +581,17 @@ static int init_render_ring(struct intel_ring_buffer
*ring)
I915_WRITE(GFX_MODE,
_MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_ALWAYS));
- if (IS_GEN7(dev))
- I915_WRITE(GFX_MODE_GEN7,
- _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) |
- _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
+ if (IS_GEN7(dev)) {
+ if (IS_VALLEYVIEW(dev)) {
+ I915_WRITE(GFX_MODE_GEN7,
+ _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
+ I915_WRITE(MI_MODE, I915_READ(MI_MODE) |
+ _MASKED_BIT_ENABLE(MI_FLUSH_ENABLE));
+ } else
+ I915_WRITE(GFX_MODE_GEN7,
+ _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) |
+ _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
+ }
if (INTEL_INFO(dev)->gen >= 5) {
ret = init_pipe_control(ring);
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915/vlv: Added a rendering specific Hw WA 'WaSendDummy3dPrimitveAfterSetContext'
From: Akash Goel
This workaround is needed on VLV for the HW context feature.
It is used after adding the mi_set_context command in ring buffer
for Hw context switch. As per the spec
"The software must send a pipe_control with a CS stall and a post sync
operation and then a dummy DRAW after every MI_SET_CONTEXT and after any
PIPELINE_SELECT that is enabling 3D mode".
Signed-off-by: Akash Goel
---
drivers/gpu/drm/i915/i915_gem_context.c | 64 +++--
drivers/gpu/drm/i915/i915_reg.h | 3 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
4 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index ebe0f67..62a5362 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -532,6 +532,58 @@ i915_gem_context_get(struct drm_i915_file_private
*file_priv, u32 id)
return (struct i915_hw_context *)idr_find(&file_priv->context_idr, id);
}
+static inline void
+mi_set_context_dummy3d_prim_wa(struct intel_ring_buffer *ring)
+{
+ u32 scratch_addr;
+ u32 flags = 0;
+
+ /*
+* Check if we have the scratch page allocated needed
+* for the Pipe Control command, otherwise don't apply
+* the dummmy 3d primitive workaround & add NOOPs instead
+*/
+ if (get_pipe_control_scratch_addr(ring)) {
+ /* Actual scratch location is at 128 bytes offset */
+ scratch_addr = get_pipe_control_scratch_addr(ring) + 128;
+
+ /*
+* WaSendDummy3dPrimitveAfterSetContext
+* Software must send a pipe_control with a CS stall
+* and a post sync operation and then a dummy DRAW after
+* every MI_SET_CONTEXT and after any PIPELINE_SELECT that
+* is enabling 3D mode. A dummy draw is a 3DPRIMITIVE command
+* with Indirect Parameter Enable set to 0, UAV Coherency
+* Required set to 0, Predicate Enable set to 0,
+* End Offset Enable set to 0, and Vertex Count Per Instance
+* set to 0, All other parameters are a don't care.
+*/
+
+ /*
+* Add a pipe control with CS Stall and postsync op
+* before dummy 3D_PRIMITIVE
+*/
+ flags |= PIPE_CONTROL_QW_WRITE | PIPE_CONTROL_CS_STALL;
+ intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4));
+ intel_ring_emit(ring, flags);
+ intel_ring_emit(ring, scratch_addr | PIPE_CONTROL_GLOBAL_GTT);
+ intel_ring_emit(ring, 0);
+
+ /* Add a dummy 3D_PRIMITVE */
+ intel_ring_emit(ring, GFX_OP_3DPRIMITIVE());
+ intel_ring_emit(ring, 4); /* PrimTopoType*/
+ intel_ring_emit(ring, 0); /* VertexCountPerInstance */
+ intel_ring_emit(ring, 0); /* StartVertexLocation */
+ intel_ring_emit(ring, 0); /* InstanceCount */
+ intel_ring_emit(ring, 0); /* StartInstanceLocation */
+ intel_ring_emit(ring, 0); /* BaseVertexLocation */
+ } else {
+ int i;
+ for (i = 0; i < 11; i++)
+ intel_ring_emit(ring, MI_NOOP);
+ }
+}
+
static inline int
mi_set_context(struct intel_ring_buffer *ring,
struct i915_hw_context *new_context,
@@ -550,7 +602,10 @@ mi_set_context(struct intel_ring_buffer *ring,
return ret;
}
- ret = intel_ring_begin(ring, 6);
+ if (IS_VALLEYVIEW(ring->dev))
+ ret = intel_ring_begin(ring, 6+4+8);
+ else
+ ret = intel_ring_begin(ring, 6);
if (ret)
return ret;
@@ -571,7 +626,12 @@ mi_set_context(struct intel_ring_buffer *ring,
intel_ring_emit(ring, MI_NOOP);
if (IS_GEN7(ring->dev))
- intel_ring_emit(ring, MI_ARB_ON_OFF | MI_ARB_ENABLE);
+ if (IS_VALLEYVIEW(ring->dev)) {
+ mi_set_context_dummy3d_prim_wa(ring);
+ intel_ring_emit(ring, MI_ARB_ON_OFF | MI_ARB_ENABLE);
+ intel_ring_emit(ring, MI_NOOP);
+ } else
+ intel_ring_emit(ring, MI_ARB_ON_OFF | MI_ARB_ENABLE);
else
intel_ring_emit(ring, MI_NOOP);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d829754..649106d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -335,6 +335,9 @@
#define PIPE_CONTROL_DEPTH_CACHE_FLUSH (1<<0)
#define PIPE_CONTROL_GLOBAL_GTT (1<<2) /* in addr dword */
+#define GFX_OP_3DPRIMITIVE() \
+ ((0x3<<29)|(0x3<<27)|(0x3<<24)| \
+(0x0<<16)|(0x0<<10)|(0x0<<8)|(7-2))
/*
* Reset
[Intel-gfx] [PATCH 1/6] drm/i915/vlv: Added a rendering specific Hw WA 'WaTlbInvalidateStoreDataBefore'
From: Akash Goel
Added a new rendering specific Workaround 'WaTlbInvalidateStoreDataBefore'.
In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI
Store data commands.
Signed-off-by: Akash Goel
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 442c9a6..133d273 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2177,6 +2177,28 @@ intel_ring_invalidate_all_caches(struct
intel_ring_buffer *ring)
uint32_t flush_domains;
int ret;
+ if (IS_VALLEYVIEW(ring->dev)) {
+ /*
+* WaTlbInvalidateStoreDataBefore
+* Before pipecontrol with TLB invalidate set, need 2 store
+* data commands (such as MI_STORE_DATA_IMM or
MI_STORE_DATA_INDEX)
+* Without this, hardware cannot guarantee the command after the
+* PIPE_CONTROL with TLB inv will not use the old TLB values.
+*/
+ int i;
+ ret = intel_ring_begin(ring, 4 * 2);
+ if (ret)
+ return ret;
+ for (i = 0; i < 2; i++) {
+ intel_ring_emit(ring, MI_STORE_DWORD_INDEX);
+ intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_INDEX <<
+ MI_STORE_DWORD_INDEX_SHIFT);
+ intel_ring_emit(ring, 0);
+ intel_ring_emit(ring, MI_NOOP);
+ }
+ intel_ring_advance(ring);
+ }
+
flush_domains = 0;
if (ring->gpu_caches_dirty)
flush_domains = I915_GEM_GPU_DOMAINS;
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clean up display pipe register accesses
> -Original Message- > From: intel-gfx-boun...@lists.freedesktop.org [mailto:intel-gfx- > boun...@lists.freedesktop.org] On Behalf Of Antti Koskipaa > Sent: Wednesday, January 15, 2014 5:26 AM > To: intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH] drm/i915: Clean up display pipe register accesses > > RFCv2: Reorganize array indexing so that full offsets can be used as > is. It makes grepping for registers in i915_reg.h much easier. Also > move offset arrays to intel_device_info. > > PATCHv1: Fixed offsets for VLV, proper eDP handling > > Upcoming hardware will not have the various display pipe register > ranges evenly spaced in memory. Change register address calculations > into array lookups. > > Tested on SNB, VLV, IVB and HSW w/eDP. > > I left the UMS cruft untouched. > > Signed-off-by: Antti Koskipaa > --- > drivers/gpu/drm/i915/i915_drv.c | 44 + > drivers/gpu/drm/i915/i915_drv.h | 11 ++- > drivers/gpu/drm/i915/i915_reg.h | 191 +++ > - > 3 files changed, 164 insertions(+), 82 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c > index 62c0f16..3ac2ad2 100644 [snip] > +#define _HTOTAL_B0x61000 > +#define _HBLANK_B0x61004 > +#define _HSYNC_B 0x61008 > +#define _VTOTAL_B0x6100c > +#define _VBLANK_B0x61010 > +#define _VSYNC_B 0x61014 > +#define _PIPEBSRC0x6101c > +#define _BCLRPAT_B 0x61020 > +#define _VSYNCSHIFT_B0x61028 > + > +#define TRANSCODER_A_OFFSET 0x6 > +#define TRANSCODER_B_OFFSET 0x61000 > +#define TRANSCODER_C_OFFSET 0x62000 > +#define TRANSCODER_EDP_OFFSET 0x6f000 > + > +#define _TRANSCODER2(pipe, reg) (dev_priv->info->trans_offsets[(pipe)] - > \ > + dev_priv->info->trans_offsets[TRANSCODER_A] + (reg) + \ > + dev_priv->info->display_mmio_offset) > + > +#define HTOTAL(trans) _TRANSCODER2(trans, _HTOTAL_A) > +#define HBLANK(trans) _TRANSCODER2(trans, _HBLANK_A) > +#define HSYNC(trans) _TRANSCODER2(trans, _HSYNC_A) > +#define VTOTAL(trans) _TRANSCODER2(trans, _VTOTAL_A) > +#define VBLANK(trans) _TRANSCODER2(trans, _VBLANK_A) > +#define VSYNC(trans) _TRANSCODER2(trans, _VSYNC_A) > +#define BCLRPAT(pipe) _PIPE2(pipe, _BCLRPAT_A) BCLRPAT is in the transcoder, not the pipe. > +#define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A) > > /* HSW+ eDP PSR registers */ > #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : > 0x6f800) > @@ -3173,10 +3186,10 @@ > /* Display & cursor control */ > > /* Pipe A */ > -#define _PIPEADSL(dev_priv->info->display_mmio_offset + > 0x7) > +#define _PIPEADSL0x7 > #define DSL_LINEMASK_GEN2 0x0fff > #define DSL_LINEMASK_GEN3 0x1fff > -#define _PIPEACONF (dev_priv->info->display_mmio_offset + > 0x70008) > +#define _PIPEACONF 0x70008 > #define PIPECONF_ENABLE(1<<31) > #define PIPECONF_DISABLE 0 > #define PIPECONF_DOUBLE_WIDE (1<<30) > @@ -3219,7 +3232,7 @@ > #define PIPECONF_DITHER_TYPE_ST1 (1<<2) > #define PIPECONF_DITHER_TYPE_ST2 (2<<2) > #define PIPECONF_DITHER_TYPE_TEMP (3<<2) > -#define _PIPEASTAT (dev_priv->info->display_mmio_offset + > 0x70024) > +#define _PIPEASTAT 0x70024 > #define PIPE_FIFO_UNDERRUN_STATUS (1UL<<31) > #define SPRITE1_FLIPDONE_INT_EN_VLV(1UL<<30) > #define PIPE_CRC_ERROR_ENABLE (1UL<<29) > @@ -3257,12 +3270,21 @@ > #define PIPE_VBLANK_INTERRUPT_STATUS (1UL<<1) > #define PIPE_OVERLAY_UPDATED_STATUS(1UL<<0) > > -#define PIPESRC(pipe) _PIPE(pipe, _PIPEASRC, _PIPEBSRC) > -#define PIPECONF(tran) _TRANSCODER(tran, _PIPEACONF, _PIPEBCONF) > -#define PIPEDSL(pipe) _PIPE(pipe, _PIPEADSL, _PIPEBDSL) > -#define PIPEFRAME(pipe) _PIPE(pipe, _PIPEAFRAMEHIGH, > _PIPEBFRAMEHIGH) > -#define PIPEFRAMEPIXEL(pipe) _PIPE(pipe, _PIPEAFRAMEPIXEL, > _PIPEBFRAMEPIXEL) > -#define PIPESTAT(pipe) _PIPE(pipe, _PIPEASTAT, _PIPEBSTAT) > +#define PIPE_A_OFFSET0x7 > +#define PIPE_B_OFFSET0x71000 > +#define PIPE_C_OFFSET0x72000 > +#define PIPE_EDP_OFFSET 0x7f000 > + > +#define _PIPE2(pipe, reg) (dev_priv->info->pipe_offsets[pipe] - \ > + dev_priv->info->pipe_offsets[PIPE_A] + (reg) + \ > + dev_priv->info->display_mmio_offset) > + > +#define PIPESRC(pipe) _PIPE2(pipe, _PIPEASRC) PIPERSRC is in the transcoder not in the pipe. > +#define PIPECONF(tran) _TRANSCODER2(tran, _PIPEACONF) PIPECONF is in the pipe not the transcoder. > +#define PIPEDSL(pipe) _PIPE2(pipe, _PIPEADSL) > +#define PIPEFRAME(pipe) _PIPE2(pipe, _PIPEAFRAMEHIGH) > +#define PIPEFRAMEPIXEL(pipe) _PIPE2(pipe, _PIPEAFRAMEPIXEL) > +#define PIPESTAT(pipe) _PIPE2(pipe, _PIPEASTAT) > [snip] Missing from that patch is also all the DSP* registers (DSPCNTR,DSPADDR, etc...) those should use the new _PIPE2 macro. Thanks, Rafael __
