Re: [Intel-gfx] [PATCH v5 0/4] Fix Win8 backlight issue
On 10/28/2013 04:09 PM, Jani Nikula wrote:
On Mon, 28 Oct 2013, Aaron Lu aaron...@intel.com wrote:
+static int __init video_set_use_native_backlight(const struct dmi_system_id
*d)
+{
+use_native_backlight = true;
+return 0;
+}
Hi Aaron, it might be beneficial to make use_native_backlight support
three values: force true, force false, and use platform default based on
DMI.
Makes sense. I modified the patch a little bit so that if user has
specified the cmdline option use_native_backlight=0/1, it will always
take effect no matter if the system is in DMI table or not.
From: Aaron Lu aaron...@intel.com
Subject: [PATCH] ACPI / video: Add systems that should favor native backlight
interface
Some system's ACPI video backlight control interface is broken and the
native backlight control interface should be used by default. This patch
sets the use_native_backlight parameter to true for those systems so
that video backlight control interface will not be created. To be
specific, the ThinkPad T430s/X230, Lenovo Yoga 13 and Dell Inspiron 7520
are added here.
Note that the user specified kernel cmdline option will always have the
highest priority, i.e. if use_native_backlight=0 is specified and the
system is in the DMI table, the video module will not skip register
backlight interface for it.
Thinkpad T430s:
Reported-by: Theodore Tso ty...@mit.edu
Reported-and-tested-by: Peter Weber b...@ttyhoney.com
Thinkpad X230:
Reported-and-tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
Lenovo Yoga 13:
Reported-by: Lennart Poettering lenn...@poettering.net
Reported-and-tested-by: Kevin Smith thirdwig...@gmail.com
Dell Inspiron 7520:
Reported-by: Rinat Ibragimov ibragimovri...@mail.ru
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/acpi/video.c | 57 +++-
1 file changed, 52 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 38c3a28..41bd4b4 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -89,11 +89,12 @@ static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
/*
- * For Windows 8 systems: if set ture and the GPU driver has
- * registered a backlight interface, skip registering ACPI video's.
+ * For Windows 8 systems: used to decide if video module
+ * should skip registering backlight interface of its own.
*/
-static bool use_native_backlight = false;
-module_param(use_native_backlight, bool, 0644);
+static int use_native_backlight_param = -1;
+module_param_named(use_native_backlight, use_native_backlight_param, int,
0444);
+static bool use_native_backlight_dmi = false;
static int register_count;
static struct mutex video_list_lock;
@@ -239,9 +240,17 @@ static int acpi_video_get_next_level(struct
acpi_video_device *device,
static int acpi_video_switch_brightness(struct acpi_video_device *device,
int event);
+static bool acpi_video_use_native_backlight(void)
+{
+ if (use_native_backlight_param != -1)
+ return !!use_native_backlight_param;
+ else
+ return use_native_backlight_dmi;
+}
+
static bool acpi_video_verify_backlight_support(void)
{
- if (acpi_osi_is_win8() use_native_backlight
+ if (acpi_osi_is_win8() acpi_video_use_native_backlight()
backlight_device_registered(BACKLIGHT_RAW))
return false;
return acpi_video_backlight_support();
@@ -412,6 +421,12 @@ static int video_ignore_initial_backlight(const struct
dmi_system_id *d)
return 0;
}
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d)
+{
+ use_native_backlight_dmi = true;
+ return 0;
+}
+
static struct dmi_system_id video_dmi_table[] __initdata = {
/*
* Broken _BQC workaround
http://bugzilla.kernel.org/show_bug.cgi?id=13121
@@ -512,6 +527,38 @@ static struct dmi_system_id video_dmi_table[] __initdata =
{
DMI_MATCH(DMI_PRODUCT_NAME, HP 250 G1 Notebook PC),
},
},
+ {
+.callback = video_set_use_native_backlight,
+.ident = ThinkPad T430s,
+.matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, LENOVO),
+ DMI_MATCH(DMI_PRODUCT_VERSION, ThinkPad T430s),
+ },
+ },
+ {
+.callback = video_set_use_native_backlight,
+.ident = ThinkPad X230,
+.matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, LENOVO),
+ DMI_MATCH(DMI_PRODUCT_VERSION, ThinkPad X230),
+ },
+ },
+ {
+.callback = video_set_use_native_backlight,
+.ident = Lenovo Yoga 13,
+.matches = {
+DMI_MATCH(DMI_SYS_VENDOR, LENOVO),
+DMI_MATCH(DMI_PRODUCT_VERSION, Lenovo IdeaPad Yoga 13),
+ },
+ },
+ {
+
Re: [Intel-gfx] [PATCH 3/3] drm/i915/vlv: update czclk freq if needed for high bandwidth modes
On Mon, Oct 28, 2013 at 01:46:39PM -0700, Jesse Barnes wrote:
Needed to support large panel resolutions.
Tested-by: Josh Triplett j...@joshtriplett.org
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_display.c | 64
2 files changed, 66 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6a6eb8b..89109c4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1447,6 +1447,8 @@
#define CZCLK_FREQ_MASK0xf
#define GMBUSFREQ_VLV(VLV_DISPLAY_BASE + 0x6510)
+#define CZCLK_CDCLK_FREQ_RATIO (VLV_DISPLAY_BASE + 0x6508)
+
/*
* Palette regs
*/
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 152d6a8..1bf811a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3851,6 +3851,67 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
I915_WRITE(BCLRPAT(crtc-pipe), 0);
}
+static void valleyview_adjust_czclk(struct drm_device *dev, bool up)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 val, divider;
+
+ switch (dev_priv-mem_freq) {
+ default:
+ case 800:
+ if (up)
+ divider = 3;
+ else
+ divider = 4;
+ break;
+ case 1066:
+ if (up)
+ divider = 7;
+ else
+ divider = 9;
+ break;
+ case 1333:
+ if (up)
+ divider = 9;
+ else
+ divider = 11;
+ break;
+ }
+
+ /* adjust czclk ratio */
+ mutex_lock(dev_priv-dpio_lock);
+ val = vlv_cck_read(dev_priv, 0x6b);
+ val = ~0xf;
+ val |= divider;
+ vlv_cck_write(dev_priv, 0x6b, val);
+
+ /* adjust self-refresh exit latency value */
+ val = vlv_bunit_read(dev_priv, 0x11);
+ val = ~0x7f;
+ if (up)
+ val |= 0x12;
+ else
+ val |= 0xc;
+ vlv_bunit_write(dev_priv, 0x11, val);
+ mutex_unlock(dev_priv-dpio_lock);
+}
+
+static void valleyview_modeset_global_resources(struct drm_device *dev)
+{
+ struct intel_crtc *crtc;
+ bool need_czclk_increase = false;
+
+ list_for_each_entry(crtc, dev-mode_config.crtc_list, base.head) {
+ if (!crtc-base.enabled)
+ continue;
+
+ if (crtc-config.adjusted_mode.clock = 3600)
All our clocks are in kHz, so this is _really_ slow. To make debugging
this a bit easier I think we need a DRM_DEBUG_KMS line here that tells us
why we need the higher clock, e.g.
DRM_DEBUG_KMS(Increasing czclk due to pipe %s with
dotclock %i\n,
pipe_name(crtc-pipe),
crtc-config.adjusted_mode.clock);
Also, a quick comment either in the code or in the commit message that
explains quickly what this clock controls and why you've picked the right
one of the three clocks we have in the config would be good.
-Daniel
+ need_czclk_increase = true;
+ }
+
+ valleyview_adjust_czclk(dev, need_czclk_increase);
+}
+
static void valleyview_crtc_enable(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
@@ -10218,6 +10279,9 @@ static void intel_init_display(struct drm_device *dev)
}
} else if (IS_G4X(dev)) {
dev_priv-display.write_eld = g4x_write_eld;
+ } else if (IS_VALLEYVIEW(dev)) {
+ dev_priv-display.modeset_global_resources =
+ valleyview_modeset_global_resources;
}
/* Default just returns -ENODEV to indicate unsupported */
--
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
[Intel-gfx] i810 forgets configured rows columns on ttys on startx shutdown
Embedded Dell motherboard/video BIOS has very limited VESA support!!! (F00:80x25=VGA, F01:80x50=VGA, F02:80x43=VGA, F03:80x28=VGA, F05:80x30=VGA, F07:80x60=VGA, 309:132x25=VESA, 30A:132x43=VESA, 30B:132x50=VESA, 30C:132x60=VESA, (8)120:132x25=BIOS, (8)121:132x43=BIOS, (8)122:132x50=BIOS, (8)123:132x60=BIOS) Pre-KMS it used to be that common vga= modes were useless, so I took to using 0x121 to escape from 80x25 screens. Using 0x8121 required specifying an appropriate font via config file. Even after KMS began, vga=0x8121 continued to work, and still does at least through kernel 3.4.47. Now with kernel 3.11.x (openSUSE 13.1) and 3.12.rc6 (Mageia 4): boot: ttys OK, whatever font, rows and columns kernel determines unless specified otherwise on cmdline via vga=. video= and resolution= on cmdline have no effect. Vga= still works too if font is correctly specified by config file, but only for 0x12# BIOS modes. After startx is started, before being stopped, ttys remain functional. Once startx is stopped, ttys may or may not remain functional. Sometimes after startx has been run exactly once since booting they continue to be useful. Other times, the rows and columns are no longer remembered, switching to fonts appropriate to an 80x25 screen, but outputting more than 80 columns and 25 rows, scrambling the output in part via line breaks absent when needed at column 80. -- The wise are known for their understanding, and pleasant words are persuasive. Proverbs 16:21 (New Living Translation) Team OS/2 ** Reg. Linux User #211409 ** a11y rocks! Felix Miata *** http://fm.no-ip.com/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i810 forgets configured rows columns on ttys on startx shutdown
On Tue, Oct 29, 2013 at 9:37 AM, Felix Miata mrma...@earthlink.net wrote: Pre-KMS it used to be that common vga= modes were useless, so I took to using 0x121 to escape from 80x25 screens. Using 0x8121 required specifying an appropriate font via config file. Even after KMS began, vga=0x8121 continued to work, and still does at least through kernel 3.4.47. This is still pre-kms, there's no kms driver for i810 chipsets. So whatever bug there is might be just bad luck or the userspace X driver not restoring stuff correctly. But I'm pretty sure it's not a kernel bug, at least no in the i810 driver. -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] [PULL] drm-intel-fixes
On Mon, Oct 28, 2013 at 08:49:45AM +0100, Daniel Vetter wrote: Hi Dave, Please do _not_ pull this. The pipe bpp readout stuff this crucially relies on is only partially backported from -next to -fixes and apparently missing bits on Haswell. Ok, updated pull request. I wanted to wait a bit for an ivb 3 pipe regression fix, but my brain was too fuzzy thus far to write a correct patch ... So I guess that one will go through the merge window with a cc: stable. On top of the edp bpp fix from Jani just a no-lvds quirk. The two patches from Ville are both from -next and provide infrastructure for Jani's fix. I could have done a more minimal port, but I'd like to avoid divergent code for backports as much as possible. Cheers, Daniel The following changes since commit 959f58544b7f20c92d5eb43d1232c96c15c01bfb: Linux 3.12-rc7 (2013-10-27 16:12:03 -0700) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-fixes-2013-10-29 for you to fetch changes up to 645378d85ee524e429aa4cf52806047b56cdc596: drm/i915: No LVDS hardware on Intel D410PT and D425KT (2013-10-28 17:48:30 +0100) Jani Nikula (1): drm/i915/dp: workaround BIOS eDP bpp clamping issue Rob Pearce (1): drm/i915: No LVDS hardware on Intel D410PT and D425KT Ville Syrjälä (2): drm/i915: Add support for pipe_bpp readout drm/i915: Add HSW CRT output readout support Thanks, Daniel On Fri, Oct 25, 2013 at 12:50:12PM +0200, Daniel Vetter wrote: Hi Dave, Just the edp bpp fix from Jani plus the pipe bpp readout code from Ville to make it work. There's a 3 pipe ivb regression fix pending from me, but Ville's review convinced me that my first stab is broken. Cheers, Daniel The following changes since commit 828c79087cec61eaf4c76bb32c222fbe35ac3930: drm/i915: Disable GGTT PTEs on GEN6+ suspend (2013-10-18 15:44:47 +0200) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-fixes-2013-10-25 for you to fetch changes up to 52e1e223456e3aa747e9932f95948381f04b3b26: drm/i915/dp: workaround BIOS eDP bpp clamping issue (2013-10-21 09:57:02 +0200) Jani Nikula (1): drm/i915/dp: workaround BIOS eDP bpp clamping issue Ville Syrjälä (1): drm/i915: Add support for pipe_bpp readout drivers/gpu/drm/i915/intel_ddi.c | 17 + drivers/gpu/drm/i915/intel_display.c | 36 drivers/gpu/drm/i915/intel_dp.c | 20 3 files changed, 73 insertions(+) -- 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 -- 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] lib/drmtest: Scream harder when igt_exit isn't called for subtest tests
We really need this since otherwise the magic return value handling for running testcases with piglit (or on QA's validation infrastructure) doesn't work properly. We need to be careful though to only install this check on success. See also the previous commits to sprinkle igt_exit() calls over all the tests that missed it. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- lib/drmtest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/drmtest.c b/lib/drmtest.c index 8164ef9..576b4ab 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -812,8 +812,6 @@ int igt_subtest_init_parse_opts(int argc, char **argv, case 'l': if (!run_single_subtest) list_subtests = true; - else - igt_install_exit_handler(check_igt_exit); break; case 'r': if (!list_subtests) @@ -842,6 +840,8 @@ int igt_subtest_init_parse_opts(int argc, char **argv, } } + igt_install_exit_handler(check_igt_exit); + out: return ret; } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix the PPT fdi lane bifurcate state handling on ivb
Originally I've thought that this is leftover hw state dirt from the
BIOS. But after way too much helpless flailing around on my part I've
noticed that the actual bug is when we change the state of an already
active pipe.
For example when we change the fdi lines from 2 to 3 without switching
off outputs in-between we'll never see the crucial on-off transition
in the -modeset_global_resources hook the current logic relies on.
Patch version 2 got this right by instead also checking whether the
pipe is indeed active. But that in turn broke things when pipes have
been turned off through dpms since the bifurcate enabling is done in
the -crtc_mode_set callback.
To address this issues discussed with Ville in the patch review move
the setting of the bifurcate bit into the -crtc_enable hook. That way
we won't wreak havoc with this state when userspace puts all other
outputs into dpms off state. This also moves us forward with our
overall goal to unify the modeset and dpms on paths (which we need to
have to allow runtime pm in the dpms off state).
Unfortunately this requires us to move the bifurcate helpers around a
bit.
Also update the commit message, I've misanalyzed the bug rather badly.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70507
Tested-by: Jan-Michael Brummer jan.brum...@tabos.org
Cc: sta...@vger.kernel.org
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 95 ++--
1 file changed, 48 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8c3bf8a89cb7..509762c85d2e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2421,9 +2421,10 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc)
FDI_FE_ERRC_ENABLE);
}
-static bool pipe_has_enabled_pch(struct intel_crtc *intel_crtc)
+static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
{
- return intel_crtc-base.enabled intel_crtc-config.has_pch_encoder;
+ return crtc-base.enabled crtc-active
+ crtc-config.has_pch_encoder;
}
static void ivb_modeset_global_resources(struct drm_device *dev)
@@ -3074,6 +3075,48 @@ static void ironlake_pch_transcoder_set_timings(struct
intel_crtc *crtc,
I915_READ(VSYNCSHIFT(cpu_transcoder)));
}
+static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ uint32_t temp;
+
+ temp = I915_READ(SOUTH_CHICKEN1);
+ if (temp FDI_BC_BIFURCATION_SELECT)
+ return;
+
+ WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) FDI_RX_ENABLE);
+ WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) FDI_RX_ENABLE);
+
+ temp |= FDI_BC_BIFURCATION_SELECT;
+ DRM_DEBUG_KMS(enabling fdi C rx\n);
+ I915_WRITE(SOUTH_CHICKEN1, temp);
+ POSTING_READ(SOUTH_CHICKEN1);
+}
+
+static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
+{
+ struct drm_device *dev = intel_crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ switch (intel_crtc-pipe) {
+ case PIPE_A:
+ break;
+ case PIPE_B:
+ if (intel_crtc-config.fdi_lanes 2)
+ WARN_ON(I915_READ(SOUTH_CHICKEN1)
FDI_BC_BIFURCATION_SELECT);
+ else
+ cpt_enable_fdi_bc_bifurcation(dev);
+
+ break;
+ case PIPE_C:
+ cpt_enable_fdi_bc_bifurcation(dev);
+
+ break;
+ default:
+ BUG();
+ }
+}
+
/*
* Enable PCH resources required for PCH ports:
* - PCH PLLs
@@ -3092,6 +3135,9 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
assert_pch_transcoder_disabled(dev_priv, pipe);
+ if (IS_IVYBRIDGE(dev))
+ ivybridge_update_fdi_bc_bifurcation(intel_crtc);
+
/* Write the TU size bits before fdi link training, so that error
* detection works. */
I915_WRITE(FDI_RX_TUSIZE1(pipe),
@@ -5849,48 +5895,6 @@ static bool ironlake_compute_clocks(struct drm_crtc
*crtc,
return true;
}
-static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev-dev_private;
- uint32_t temp;
-
- temp = I915_READ(SOUTH_CHICKEN1);
- if (temp FDI_BC_BIFURCATION_SELECT)
- return;
-
- WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) FDI_RX_ENABLE);
- WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) FDI_RX_ENABLE);
-
- temp |= FDI_BC_BIFURCATION_SELECT;
- DRM_DEBUG_KMS(enabling fdi C rx\n);
- I915_WRITE(SOUTH_CHICKEN1, temp);
- POSTING_READ(SOUTH_CHICKEN1);
-}
-
-static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
-{
- struct drm_device *dev = intel_crtc-base.dev;
-
Re: [Intel-gfx] i810 forgets configured rows columns on ttys on startx shutdown
On 2013-10-29 09:51 (GMT+0100) Daniel Vetter composed: Felix Miata wrote: Pre-KMS it used to be that common vga= modes were useless, so I took to using 0x121 to escape from 80x25 screens. Using 0x8121 required specifying an appropriate font via config file. Even after KMS began, vga=0x8121 continued to work, and still does at least through kernel 3.4.47. This is still pre-kms, there's no kms driver for i810 chipsets. So whatever bug there is might be just bad luck or the userspace X driver not restoring stuff correctly. But I'm pretty sure it's not a kernel bug, at least no in the i810 driver. I was hoping for a (what appears to be a not our bug) response that was more helpful WRT whose bug it likely is. To me it's clearly a bug report that either can be found or needs to be made at bugs.freedesktop.org, but in what component? https://bugs.freedesktop.org/describecomponents.cgi?product=xorg has a long list, among which there is only one that includes the string intel, Driver/intel, which is what this list seems to be about. Does it look like Server/General, Server/DDX/Xorg, or something else? As it manifest in more than one distro, it surely isn't likely to be downstream. -- The wise are known for their understanding, and pleasant words are persuasive. Proverbs 16:21 (New Living Translation) Team OS/2 ** Reg. Linux User #211409 ** a11y rocks! Felix Miata *** http://fm.no-ip.com/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix the PPT fdi lane bifurcate state handling on ivb
On Tue, Oct 29, 2013 at 12:04:08PM +0100, Daniel Vetter wrote:
Originally I've thought that this is leftover hw state dirt from the
BIOS. But after way too much helpless flailing around on my part I've
noticed that the actual bug is when we change the state of an already
active pipe.
For example when we change the fdi lines from 2 to 3 without switching
off outputs in-between we'll never see the crucial on-off transition
in the -modeset_global_resources hook the current logic relies on.
Patch version 2 got this right by instead also checking whether the
pipe is indeed active. But that in turn broke things when pipes have
been turned off through dpms since the bifurcate enabling is done in
the -crtc_mode_set callback.
To address this issues discussed with Ville in the patch review move
the setting of the bifurcate bit into the -crtc_enable hook. That way
we won't wreak havoc with this state when userspace puts all other
outputs into dpms off state. This also moves us forward with our
overall goal to unify the modeset and dpms on paths (which we need to
have to allow runtime pm in the dpms off state).
Unfortunately this requires us to move the bifurcate helpers around a
bit.
Also update the commit message, I've misanalyzed the bug rather badly.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70507
Tested-by: Jan-Michael Brummer jan.brum...@tabos.org
Cc: sta...@vger.kernel.org
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
And since it now calls cpt_enable_fdi_bc_bifurcation() only when
has_pch_encoder==true it should also fix the following scenario:
1. setcrtc pipe B - PCH w/ fdi_lanes2
2. setcrtc pipe C - eDP
Previously the pipe C .mode_set() whould have called
cpt_enable_fdi_bc_bifurcation() unconditionally, which
would have killed pipe B.
---
drivers/gpu/drm/i915/intel_display.c | 95
++--
1 file changed, 48 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8c3bf8a89cb7..509762c85d2e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2421,9 +2421,10 @@ static void intel_fdi_normal_train(struct drm_crtc
*crtc)
FDI_FE_ERRC_ENABLE);
}
-static bool pipe_has_enabled_pch(struct intel_crtc *intel_crtc)
+static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
{
- return intel_crtc-base.enabled intel_crtc-config.has_pch_encoder;
+ return crtc-base.enabled crtc-active
+ crtc-config.has_pch_encoder;
}
static void ivb_modeset_global_resources(struct drm_device *dev)
@@ -3074,6 +3075,48 @@ static void ironlake_pch_transcoder_set_timings(struct
intel_crtc *crtc,
I915_READ(VSYNCSHIFT(cpu_transcoder)));
}
+static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ uint32_t temp;
+
+ temp = I915_READ(SOUTH_CHICKEN1);
+ if (temp FDI_BC_BIFURCATION_SELECT)
+ return;
+
+ WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) FDI_RX_ENABLE);
+ WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) FDI_RX_ENABLE);
+
+ temp |= FDI_BC_BIFURCATION_SELECT;
+ DRM_DEBUG_KMS(enabling fdi C rx\n);
+ I915_WRITE(SOUTH_CHICKEN1, temp);
+ POSTING_READ(SOUTH_CHICKEN1);
+}
+
+static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc
*intel_crtc)
+{
+ struct drm_device *dev = intel_crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ switch (intel_crtc-pipe) {
+ case PIPE_A:
+ break;
+ case PIPE_B:
+ if (intel_crtc-config.fdi_lanes 2)
+ WARN_ON(I915_READ(SOUTH_CHICKEN1)
FDI_BC_BIFURCATION_SELECT);
+ else
+ cpt_enable_fdi_bc_bifurcation(dev);
+
+ break;
+ case PIPE_C:
+ cpt_enable_fdi_bc_bifurcation(dev);
+
+ break;
+ default:
+ BUG();
+ }
+}
+
/*
* Enable PCH resources required for PCH ports:
* - PCH PLLs
@@ -3092,6 +3135,9 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
assert_pch_transcoder_disabled(dev_priv, pipe);
+ if (IS_IVYBRIDGE(dev))
+ ivybridge_update_fdi_bc_bifurcation(intel_crtc);
+
/* Write the TU size bits before fdi link training, so that error
* detection works. */
I915_WRITE(FDI_RX_TUSIZE1(pipe),
@@ -5849,48 +5895,6 @@ static bool ironlake_compute_clocks(struct drm_crtc
*crtc,
return true;
}
-static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev-dev_private;
- uint32_t temp;
-
- temp = I915_READ(SOUTH_CHICKEN1);
- if (temp
Re: [Intel-gfx] [PATCH] drm/i915: Fix the PPT fdi lane bifurcate state handling on ivb
On Tue, Oct 29, 2013 at 02:46:10PM +0200, Ville Syrjälä wrote:
On Tue, Oct 29, 2013 at 12:04:08PM +0100, Daniel Vetter wrote:
Originally I've thought that this is leftover hw state dirt from the
BIOS. But after way too much helpless flailing around on my part I've
noticed that the actual bug is when we change the state of an already
active pipe.
For example when we change the fdi lines from 2 to 3 without switching
off outputs in-between we'll never see the crucial on-off transition
in the -modeset_global_resources hook the current logic relies on.
Patch version 2 got this right by instead also checking whether the
pipe is indeed active. But that in turn broke things when pipes have
been turned off through dpms since the bifurcate enabling is done in
the -crtc_mode_set callback.
To address this issues discussed with Ville in the patch review move
the setting of the bifurcate bit into the -crtc_enable hook. That way
we won't wreak havoc with this state when userspace puts all other
outputs into dpms off state. This also moves us forward with our
overall goal to unify the modeset and dpms on paths (which we need to
have to allow runtime pm in the dpms off state).
Unfortunately this requires us to move the bifurcate helpers around a
bit.
Also update the commit message, I've misanalyzed the bug rather badly.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70507
Tested-by: Jan-Michael Brummer jan.brum...@tabos.org
Cc: sta...@vger.kernel.org
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
And since it now calls cpt_enable_fdi_bc_bifurcation() only when
has_pch_encoder==true it should also fix the following scenario:
1. setcrtc pipe B - PCH w/ fdi_lanes2
2. setcrtc pipe C - eDP
Previously the pipe C .mode_set() whould have called
cpt_enable_fdi_bc_bifurcation() unconditionally, which
would have killed pipe B.
Geez, did that take a while for me to sink in. Somehow I've thought I've
fixed this a long time ago so that cpu edp on pipe C allows us to fully
use the fdi links on pipes A/B. But reading through the code again I've
only updated the logic in the compute_config stage, not here.
Thanks for your review, patch merged to -fixes.
-Daniel
---
drivers/gpu/drm/i915/intel_display.c | 95
++--
1 file changed, 48 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8c3bf8a89cb7..509762c85d2e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2421,9 +2421,10 @@ static void intel_fdi_normal_train(struct drm_crtc
*crtc)
FDI_FE_ERRC_ENABLE);
}
-static bool pipe_has_enabled_pch(struct intel_crtc *intel_crtc)
+static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
{
- return intel_crtc-base.enabled intel_crtc-config.has_pch_encoder;
+ return crtc-base.enabled crtc-active
+ crtc-config.has_pch_encoder;
}
static void ivb_modeset_global_resources(struct drm_device *dev)
@@ -3074,6 +3075,48 @@ static void
ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
I915_READ(VSYNCSHIFT(cpu_transcoder)));
}
+static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ uint32_t temp;
+
+ temp = I915_READ(SOUTH_CHICKEN1);
+ if (temp FDI_BC_BIFURCATION_SELECT)
+ return;
+
+ WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) FDI_RX_ENABLE);
+ WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) FDI_RX_ENABLE);
+
+ temp |= FDI_BC_BIFURCATION_SELECT;
+ DRM_DEBUG_KMS(enabling fdi C rx\n);
+ I915_WRITE(SOUTH_CHICKEN1, temp);
+ POSTING_READ(SOUTH_CHICKEN1);
+}
+
+static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc
*intel_crtc)
+{
+ struct drm_device *dev = intel_crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ switch (intel_crtc-pipe) {
+ case PIPE_A:
+ break;
+ case PIPE_B:
+ if (intel_crtc-config.fdi_lanes 2)
+ WARN_ON(I915_READ(SOUTH_CHICKEN1)
FDI_BC_BIFURCATION_SELECT);
+ else
+ cpt_enable_fdi_bc_bifurcation(dev);
+
+ break;
+ case PIPE_C:
+ cpt_enable_fdi_bc_bifurcation(dev);
+
+ break;
+ default:
+ BUG();
+ }
+}
+
/*
* Enable PCH resources required for PCH ports:
* - PCH PLLs
@@ -3092,6 +3135,9 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
assert_pch_transcoder_disabled(dev_priv, pipe);
+ if (IS_IVYBRIDGE(dev))
+ ivybridge_update_fdi_bc_bifurcation(intel_crtc);
+
/*
Re: [Intel-gfx] i810 forgets configured rows columns on ttys on startx shutdown
On Tue, Oct 29, 2013 at 08:00:20AM -0400, Felix Miata wrote: On 2013-10-29 09:51 (GMT+0100) Daniel Vetter composed: Felix Miata wrote: Pre-KMS it used to be that common vga= modes were useless, so I took to using 0x121 to escape from 80x25 screens. Using 0x8121 required specifying an appropriate font via config file. Even after KMS began, vga=0x8121 continued to work, and still does at least through kernel 3.4.47. This is still pre-kms, there's no kms driver for i810 chipsets. So whatever bug there is might be just bad luck or the userspace X driver not restoring stuff correctly. But I'm pretty sure it's not a kernel bug, at least no in the i810 driver. I was hoping for a (what appears to be a not our bug) response that was more helpful WRT whose bug it likely is. To me it's clearly a bug report that either can be found or needs to be made at bugs.freedesktop.org, but in what component? https://bugs.freedesktop.org/describecomponents.cgi?product=xorg has a long list, among which there is only one that includes the string intel, Driver/intel, which is what this list seems to be about. Does it look like Server/General, Server/DDX/Xorg, or something else? As it manifest in more than one distro, it surely isn't likely to be downstream. Userspace modesetting is known to be racy and broken, so people don't really want to waste time on digging into bugs which are most likely flukes. But if you can bisect this issue to a specific patch a fix should be possible. -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] [PULL] drm-intel-fixes
On Tue, Oct 29, 2013 at 11:29:29AM +0100, Daniel Vetter wrote: On Mon, Oct 28, 2013 at 08:49:45AM +0100, Daniel Vetter wrote: Hi Dave, Please do _not_ pull this. The pipe bpp readout stuff this crucially relies on is only partially backported from -next to -fixes and apparently missing bits on Haswell. Ok, updated pull request. I wanted to wait a bit for an ivb 3 pipe regression fix, but my brain was too fuzzy thus far to write a correct patch ... So I guess that one will go through the merge window with a cc: stable. On top of the edp bpp fix from Jani just a no-lvds quirk. The two patches from Ville are both from -next and provide infrastructure for Jani's fix. I could have done a more minimal port, but I'd like to avoid divergent code for backports as much as possible. Ok, clue came back in the nick of time, so I've figured I'll smash the 3 pipe fix (testedreviewed) into the pull request, too. I like my kernel release free of known regressions ;-) Cheers, Daniel The following changes since commit 959f58544b7f20c92d5eb43d1232c96c15c01bfb: Linux 3.12-rc7 (2013-10-27 16:12:03 -0700) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-fixes-2013-10-29 for you to fetch changes up to 1fbc0d789d12fec313c91912fc11733fdfbab863: drm/i915: Fix the PPT fdi lane bifurcate state handling on ivb (2013-10-29 13:52:56 +0100) Daniel Vetter (1): drm/i915: Fix the PPT fdi lane bifurcate state handling on ivb Jani Nikula (1): drm/i915/dp: workaround BIOS eDP bpp clamping issue Rob Pearce (1): drm/i915: No LVDS hardware on Intel D410PT and D425KT Ville Syrjälä (2): drm/i915: Add support for pipe_bpp readout drm/i915: Add HSW CRT output readout support -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: add back checking for i915_disable_power_well
In
commit 6efdf354ddb186c6604d1692075421e8d2c740e9
Author: Imre Deak imre.d...@intel.com
Date: Wed Oct 16 17:25:52 2013 +0300
the check for i915_disable_power_well flag was removed by overlook,
so add it back now.
Reported-by: Paulo Zanoni paulo.zan...@intel.com
Signed-off-by: Imre Deak imre.d...@intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a0c907f..09ac9e7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5599,7 +5599,7 @@ static void __intel_power_well_put(struct drm_device *dev,
struct i915_power_well *power_well)
{
WARN_ON(!power_well-count);
- if (!--power_well-count)
+ if (!--power_well-count i915_disable_power_well)
__intel_set_power_well(dev, false);
}
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: rename i915_init_power_well to init_power_domains_init
On Mon, Oct 28, 2013 at 04:51:52PM -0200, Paulo Zanoni wrote:
2013/10/28 Imre Deak imre.d...@intel.com:
Similarly rename the other related functions in the power domain
interface.
Higher level driver code calling these functions knows only about power
domains, not the underlying power wells which may be different on
different platforms. Also these functions really init/cleanup/resume
power domains and only through that all related power wells, so rename
them accordingly.
Note that I left i915_{request,release}_power_well as is, since that
really changes the state only of a single power well (and is HSW
specific). It should also get a better name once we make it more
generic by controlling things through a new audio power domain.
v4:
- use intel prefix instead of i915 everywhere (Paulo)
- use a $prefix_$block_$action format (Daniel)
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/14] drm: Simplify the math in drm_calc_timestamping_constants()
From: Ville Syrjälä ville.syrj...@linux.intel.com
drm_calc_timestamping_constants() makes the math more complex
than necessary.
- multipying the dotclock by 1000 is pointless, just makes all the
numbers bigger
- div64_u64() is also pointless, div_u64 is enough
- pixeldur_ns doesn't need any 64bit math
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_irq.c | 26 +++---
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 48bf91f..d6ef034 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -451,10 +451,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
const struct drm_display_mode *mode)
{
s64 linedur_ns = 0, pixeldur_ns = 0, framedur_ns = 0;
- u64 dotclock;
-
- /* Dot clock in Hz: */
- dotclock = (u64) mode-clock * 1000;
+ int dotclock = mode-clock;
/* Fields of interlaced scanout modes are only halve a frame duration.
* Double the dotclock to get halve the frame-/line-/pixelduration.
@@ -464,17 +461,16 @@ void drm_calc_timestamping_constants(struct drm_crtc
*crtc,
/* Valid dotclock? */
if (dotclock 0) {
- int frame_size;
- /* Convert scanline length in pixels and video dot clock to
-* line duration, frame duration and pixel duration in
-* nanoseconds:
+ int frame_size = mode-crtc_htotal * mode-crtc_vtotal;
+
+ /*
+* Convert scanline length in pixels and video
+* dot clock to line duration, frame duration
+* and pixel duration in nanoseconds:
*/
- pixeldur_ns = (s64) div64_u64(10, dotclock);
- linedur_ns = (s64) div64_u64(((u64) mode-crtc_htotal *
- 10), dotclock);
- frame_size = mode-crtc_htotal * mode-crtc_vtotal;
- framedur_ns = (s64) div64_u64((u64) frame_size * 10,
- dotclock);
+ pixeldur_ns = 100 / dotclock;
+ linedur_ns = div_u64((u64) mode-crtc_htotal * 100,
dotclock);
+ framedur_ns = div_u64((u64) frame_size * 100, dotclock);
} else
DRM_ERROR(crtc %d: Can't calculate constants, dotclock = 0!\n,
crtc-base.id);
@@ -487,7 +483,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
crtc-base.id, mode-crtc_htotal,
mode-crtc_vtotal, mode-crtc_vdisplay);
DRM_DEBUG(crtc %d: clock %d kHz framedur %d linedur %d, pixeldur %d\n,
- crtc-base.id, (int) dotclock/1000, (int) framedur_ns,
+ crtc-base.id, dotclock, (int) framedur_ns,
(int) linedur_ns, (int) pixeldur_ns);
}
EXPORT_SYMBOL(drm_calc_timestamping_constants);
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/14] drm: Pass the display mode to drm_calc_timestamping_constants()
From: Ville Syrjälä ville.syrj...@linux.intel.com
We don't really use hwmode anymore in i915, so eliminating its use
from the core code seems prudent. Just pass the appropriate mode
to drm_calc_timestamping_constants().
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_crtc_helper.c| 2 +-
drivers/gpu/drm/drm_irq.c| 17 +
drivers/gpu/drm/i915/intel_display.c | 3 ++-
include/drm/drmP.h | 3 ++-
4 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index dbcd687..fce1cff 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -533,7 +533,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
* are later needed by vblank and swap-completion
* timestamping. They are derived from true hwmode.
*/
- drm_calc_timestamping_constants(crtc);
+ drm_calc_timestamping_constants(crtc, crtc-hwmode);
/* FIXME: add subpixel order */
done:
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 2250724..679417d 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -445,20 +445,22 @@ int drm_control(struct drm_device *dev, void *data,
* adjustments into account.
*
* @crtc drm_crtc whose timestamp constants should be updated.
+ * @mode display mode containing the scanout timings
*
*/
-void drm_calc_timestamping_constants(struct drm_crtc *crtc)
+void drm_calc_timestamping_constants(struct drm_crtc *crtc,
+const struct drm_display_mode *mode)
{
s64 linedur_ns = 0, pixeldur_ns = 0, framedur_ns = 0;
u64 dotclock;
/* Dot clock in Hz: */
- dotclock = (u64) crtc-hwmode.clock * 1000;
+ dotclock = (u64) mode-clock * 1000;
/* Fields of interlaced scanout modes are only halve a frame duration.
* Double the dotclock to get halve the frame-/line-/pixelduration.
*/
- if (crtc-hwmode.flags DRM_MODE_FLAG_INTERLACE)
+ if (mode-flags DRM_MODE_FLAG_INTERLACE)
dotclock *= 2;
/* Valid dotclock? */
@@ -469,10 +471,9 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc)
* nanoseconds:
*/
pixeldur_ns = (s64) div64_u64(10, dotclock);
- linedur_ns = (s64) div64_u64(((u64) crtc-hwmode.crtc_htotal *
+ linedur_ns = (s64) div64_u64(((u64) mode-crtc_htotal *
10), dotclock);
- frame_size = crtc-hwmode.crtc_htotal *
- crtc-hwmode.crtc_vtotal;
+ frame_size = mode-crtc_htotal * mode-crtc_vtotal;
framedur_ns = (s64) div64_u64((u64) frame_size * 10,
dotclock);
} else
@@ -484,8 +485,8 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc)
crtc-framedur_ns = framedur_ns;
DRM_DEBUG(crtc %d: hwmode: htotal %d, vtotal %d, vdisplay %d\n,
- crtc-base.id, crtc-hwmode.crtc_htotal,
- crtc-hwmode.crtc_vtotal, crtc-hwmode.crtc_vdisplay);
+ crtc-base.id, mode-crtc_htotal,
+ mode-crtc_vtotal, mode-crtc_vdisplay);
DRM_DEBUG(crtc %d: clock %d kHz framedur %d linedur %d, pixeldur %d\n,
crtc-base.id, (int) dotclock/1000, (int) framedur_ns,
(int) linedur_ns, (int) pixeldur_ns);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0c2e83c..7ea20b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9364,7 +9364,8 @@ static int __intel_set_mode(struct drm_crtc *crtc,
* are later needed by vblank and swap-completion
* timestamping. They are derived from true hwmode.
*/
- drm_calc_timestamping_constants(crtc);
+ drm_calc_timestamping_constants(crtc,
+ pipe_config-adjusted_mode);
}
/* FIXME: add subpixel order */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8d4e10d..99f49ea 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1419,7 +1419,8 @@ extern int drm_calc_vbltimestamp_from_scanoutpos(struct
drm_device *dev,
struct timeval *vblank_time,
unsigned flags,
struct drm_crtc *refcrtc);
-extern void drm_calc_timestamping_constants(struct drm_crtc *crtc);
+extern void drm_calc_timestamping_constants(struct drm_crtc *crtc,
+ const struct drm_display_mode
*mode);
extern bool
[Intel-gfx] [PATCH 10/14] drm/i915: Fix scanoutpos calculations for interlaced modes
From: Ville Syrjälä ville.syrj...@linux.intel.com
The scanline counter counts lines in the current field, not the entire
frame. But the crtc_ timings are the values for the entire frame. Divide
the vertical timings by 2 to make them match the scanline counter.
The rounding was carefully chosen to make it do the right thing wrt. the
observed scanline counter and ISR vblank bit behaviour.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2b19989..f6b3206 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -680,6 +680,12 @@ static int i915_get_crtc_scanoutpos(struct drm_device
*dev, int pipe,
vbl_start = mode-crtc_vblank_start;
vbl_end = mode-crtc_vblank_end;
+ if (mode-flags DRM_MODE_FLAG_INTERLACE) {
+ vbl_start = DIV_ROUND_UP(vbl_start, 2);
+ vbl_end /= 2;
+ vtotal /= 2;
+ }
+
ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE;
/* Lock uncore.lock, as we will do multiple timing critical raw
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/14] drm/i915: Call drm_calc_timestamping_constants() earlier
From: Ville Syrjälä ville.syrj...@linux.intel.com
Update the pixel/line/frame duration information when we switch to the
new pipe config. This will keep the timestamping constants in better
sync with the real hardware state.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2b35b29..1b8ed0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9331,6 +9331,14 @@ static int __intel_set_mode(struct drm_crtc *crtc,
/* mode_set/enable/disable functions rely on a correct pipe
* config. */
to_intel_crtc(crtc)-config = *pipe_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
@@ -9354,15 +9362,6 @@ static int __intel_set_mode(struct drm_crtc *crtc,
for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc)
dev_priv-display.crtc_enable(intel_crtc-base);
- if (modeset_pipes) {
- /* 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);
- }
-
/* FIXME: add subpixel order */
done:
if (ret crtc-enabled)
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/14] drm: Improve drm_calc_timestamping_constants() documentation
From: Ville Syrjälä ville.syrj...@linux.intel.com Move the long blurp to into the body of the comment, leaving only a short summary line at the top. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/drm_irq.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 7702614..48bf91f 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -436,17 +436,16 @@ int drm_control(struct drm_device *dev, void *data, } /** - * drm_calc_timestamping_constants - Calculate and - * store various constants which are later needed by - * vblank and swap-completion timestamping, e.g, by - * drm_calc_vbltimestamp_from_scanoutpos(). - * They are derived from crtc's true scanout timing, - * so they take things like panel scaling or other - * adjustments into account. + * drm_calc_timestamping_constants - Calculate vblank timestamp constants * * @crtc drm_crtc whose timestamp constants should be updated. * @mode display mode containing the scanout timings * + * Calculate and store various constants which are later + * needed by vblank and swap-completion timestamping, e.g, + * by drm_calc_vbltimestamp_from_scanoutpos(). They are + * derived from crtc's true scanout timing, so they take + * things like panel scaling or other adjustments into account. */ void drm_calc_timestamping_constants(struct drm_crtc *crtc, const struct drm_display_mode *mode) -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/14] drm/radeon: Populate crtc_clock in radeon_atom_get_tv_timings()
From: Ville Syrjälä ville.syrj...@linux.intel.com
crtc_clock is now supposed to be the actual pixel clock corresponding to
the other crtc_ timing values. Populate crtc_clock appropriately in
radeon_atom_get_tv_timings().
This was the only obvious place where we frob with the crtc_ timigns
directly instead of calling drm_mode_set_crtcinfo() which would also
update crtc_clock.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/radeon/radeon_atombios.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c
b/drivers/gpu/drm/radeon/radeon_atombios.c
index f79ee18..229ef65 100644
--- a/drivers/gpu/drm/radeon/radeon_atombios.c
+++ b/drivers/gpu/drm/radeon/radeon_atombios.c
@@ -1809,7 +1809,8 @@ bool radeon_atom_get_tv_timings(struct radeon_device
*rdev, int index,
if (misc ATOM_DOUBLE_CLOCK_MODE)
mode-flags |= DRM_MODE_FLAG_DBLSCAN;
- mode-clock =
le16_to_cpu(tv_info-aModeTimings[index].usPixelClock) * 10;
+ mode-crtc_clock = mode-clock =
+ le16_to_cpu(tv_info-aModeTimings[index].usPixelClock)
* 10;
if (index == 1) {
/* PAL timings appear to have wrong values for totals */
@@ -1852,7 +1853,8 @@ bool radeon_atom_get_tv_timings(struct radeon_device
*rdev, int index,
if (misc ATOM_DOUBLE_CLOCK_MODE)
mode-flags |= DRM_MODE_FLAG_DBLSCAN;
- mode-clock = le16_to_cpu(dtd_timings-usPixClk) * 10;
+ mode-crtc_clock = mode-clock =
+ le16_to_cpu(dtd_timings-usPixClk) * 10;
break;
}
return true;
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/14] drm: Fix vblank timestamping constants for interlaced modes
From: Ville Syrjälä ville.syrj...@linux.intel.com
We're currently miscalculating the line and pixel durations for
interlaced modes. crtc_htotal and crtc_vtotal are the full frame
timings, and so is crtc_clock, so we can compute the line
and pixel durations from those w/o any extra adjustments. But
we actually want framedur_ns to be the field, not frame, duration,
so we must divide it by two.
This should make the scanout based vblank timestamp corrections
work correctly with interlaced modes, at least for i915. It all
depends whether we keep the field or frame timings in the display
mode crtc_ timings.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_irq.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index b21a226..b5c4d42 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -453,12 +453,6 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
int linedur_ns = 0, pixeldur_ns = 0, framedur_ns = 0;
int dotclock = mode-crtc_clock;
- /* Fields of interlaced scanout modes are only halve a frame duration.
-* Double the dotclock to get halve the frame-/line-/pixelduration.
-*/
- if (mode-flags DRM_MODE_FLAG_INTERLACE)
- dotclock *= 2;
-
/* Valid dotclock? */
if (dotclock 0) {
int frame_size = mode-crtc_htotal * mode-crtc_vtotal;
@@ -471,6 +465,12 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
pixeldur_ns = 100 / dotclock;
linedur_ns = div_u64((u64) mode-crtc_htotal * 100,
dotclock);
framedur_ns = div_u64((u64) frame_size * 100, dotclock);
+
+ /*
+* Fields of interlaced scanout modes are only halve a frame
duration.
+*/
+ if (mode-flags DRM_MODE_FLAG_INTERLACE)
+ framedur_ns /= 2;
} else
DRM_ERROR(crtc %d: Can't calculate constants, dotclock = 0!\n,
crtc-base.id);
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/14] drm: Use crtc_clock in drm_calc_timestamping_constants()
From: Ville Syrjälä ville.syrj...@linux.intel.com
drm_calc_timestamping_constants() computes the pixel/line/frame
durations based on the crtc_ timing values. The corresponding pixel
clock is in mode-crtc_clock, so we need to use that instead of
mode-clock.
This should fix drm_calc_timestamping_constants() for frame packing
stereo modes.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index d6ef034..77be2fb 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -451,7 +451,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
const struct drm_display_mode *mode)
{
s64 linedur_ns = 0, pixeldur_ns = 0, framedur_ns = 0;
- int dotclock = mode-clock;
+ int dotclock = mode-crtc_clock;
/* Fields of interlaced scanout modes are only halve a frame duration.
* Double the dotclock to get halve the frame-/line-/pixelduration.
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/14] drm/i915: Add a kludge for DSL incrementing too late and ISR not working
From: Ville Syrjälä ville.syrj...@linux.intel.com
On pre-PCH platforms ISR doesn't seem to be an actual ISR, at least as
far as display interrupts are concerned. Instead it sort of looks like
some ISR bits just directly reflect the corresponding bit from PIPESTAT.
The bit appears in the ISR only if the PIPESTAT interrupt is enabled. So
in that sense it sort of looks a bit like the south interrupt scheme on
PCH platforms. So it goes something a bit like this:
PIPESTAT.status PIPESTAT.enable - ISR - IMR - IIR - IER - actual
interrupt
In any case that means the intel_pipe_in_vblank_locked() doesn't actually
work for pre-PCH platforms. As a last resort, add a similar kludge as radeon
has that fixes things up if we got called from the vblank interrupt,
but the scanline counter value indicates that we're not quite there yet.
We know that the scanline counter increments at hsync but is otherwise
accurate, so we can limit the kludge to the line just prior to vblank
start, instead of the relative distance that radeon uses.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 79 -
1 file changed, 39 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 70daf3c..b93f39c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -603,36 +603,15 @@ static u32 gm45_get_vblank_counter(struct drm_device
*dev, int pipe)
#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)-regs +
(reg__))
#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)-regs +
(reg__))
-static bool intel_pipe_in_vblank_locked(struct drm_device *dev, enum pipe pipe)
+static bool ilk_pipe_in_vblank_locked(struct drm_device *dev, enum pipe pipe)
{
struct drm_i915_private *dev_priv = dev-dev_private;
uint32_t status;
- int reg;
- if (IS_VALLEYVIEW(dev)) {
- status = pipe == PIPE_A ?
- I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
- I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
-
- reg = VLV_ISR;
- } else if (IS_GEN2(dev)) {
- status = pipe == PIPE_A ?
- I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
- I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
-
- reg = ISR;
- } else if (INTEL_INFO(dev)-gen 5) {
- status = pipe == PIPE_A ?
- I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
- I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
-
- reg = ISR;
- } else if (INTEL_INFO(dev)-gen 7) {
+ if (INTEL_INFO(dev)-gen 7) {
status = pipe == PIPE_A ?
DE_PIPEA_VBLANK :
DE_PIPEB_VBLANK;
-
- reg = DEISR;
} else {
switch (pipe) {
default:
@@ -646,14 +625,9 @@ static bool intel_pipe_in_vblank_locked(struct drm_device
*dev, enum pipe pipe)
status = DE_PIPEC_VBLANK_IVB;
break;
}
-
- reg = DEISR;
}
- if (IS_GEN2(dev))
- return __raw_i915_read16(dev_priv, reg) status;
- else
- return __raw_i915_read32(dev_priv, reg) status;
+ return __raw_i915_read32(dev_priv, DEISR) status;
}
static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
@@ -710,17 +684,42 @@ static int i915_get_crtc_scanoutpos(struct drm_device
*dev, int pipe,
else
position = __raw_i915_read32(dev_priv, PIPEDSL(pipe))
DSL_LINEMASK_GEN3;
- /*
-* The scanline counter increments at the leading edge
-* of hsync, ie. it completely misses the active portion
-* of the line. Fix up the counter at both edges of vblank
-* to get a more accurate picture whether we're in vblank
-* or not.
-*/
- in_vbl = intel_pipe_in_vblank_locked(dev, pipe);
- if ((in_vbl position == vbl_start - 1) ||
- (!in_vbl position == vbl_end - 1))
- position = (position + 1) % vtotal;
+ if (HAS_PCH_SPLIT(dev)) {
+ /*
+* The scanline counter increments at the leading edge
+* of hsync, ie. it completely misses the active portion
+* of the line. Fix up the counter at both edges of
vblank
+* to get a more accurate picture whether we're in
vblank
+* or not.
+*/
+ in_vbl = ilk_pipe_in_vblank_locked(dev, pipe);
+ if ((in_vbl position == vbl_start - 1) ||
+ (!in_vbl position == vbl_end - 1))
+
[Intel-gfx] [PATCH 12/14] drm: Pass 'flags' from the caller to .get_scanout_position()
From: Ville Syrjälä ville.syrj...@linux.intel.com
Preparation for moving the early vblank IRQ logic into
radeon_get_crtc_scanoutpos().
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_irq.c | 2 +-
drivers/gpu/drm/i915/i915_irq.c | 3 ++-
drivers/gpu/drm/radeon/radeon_display.c | 7 ---
drivers/gpu/drm/radeon/radeon_mode.h| 1 +
drivers/gpu/drm/radeon/radeon_pm.c | 2 +-
include/drm/drmP.h | 2 ++
6 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index b5c4d42..b39255f 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -585,7 +585,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device
*dev, int crtc,
/* Get vertical and horizontal scanout position vpos, hpos,
* and bounding timestamps stime, etime, pre/post query.
*/
- vbl_status = dev-driver-get_scanout_position(dev, crtc, vpos,
+ vbl_status = dev-driver-get_scanout_position(dev, crtc,
flags, vpos,
hpos, stime,
etime);
/* Get correction for CLOCK_MONOTONIC - CLOCK_REALTIME if
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f6b3206..70daf3c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -657,7 +657,8 @@ static bool intel_pipe_in_vblank_locked(struct drm_device
*dev, enum pipe pipe)
}
static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
-int *vpos, int *hpos, ktime_t *stime, ktime_t
*etime)
+ unsigned int flags, int *vpos, int *hpos,
+ ktime_t *stime, ktime_t *etime)
{
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc = dev_priv-pipe_to_crtc_mapping[pipe];
diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c
index ccd8751..3581570 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -305,7 +305,7 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev,
int crtc_id)
* to complete in this vblank?
*/
if (update_pending
- (DRM_SCANOUTPOS_VALID radeon_get_crtc_scanoutpos(rdev-ddev,
crtc_id,
+ (DRM_SCANOUTPOS_VALID radeon_get_crtc_scanoutpos(rdev-ddev,
crtc_id, 0,
vpos, hpos,
NULL, NULL))
((vpos = (99 *
rdev-mode_info.crtcs[crtc_id]-base.hwmode.crtc_vdisplay)/100) ||
(vpos 0 !ASIC_IS_AVIVO(rdev {
@@ -1544,6 +1544,7 @@ bool radeon_crtc_scaling_mode_fixup(struct drm_crtc *crtc,
*
* \param dev Device to query.
* \param crtc Crtc to query.
+ * \param flags Flags from caller (DRM_CALLED_FROM_VBLIRQ or 0).
* \param *vpos Location where vertical scanout position should be stored.
* \param *hpos Location where horizontal scanout position should go.
* \param *stime Target location for timestamp taken immediately before
@@ -1565,8 +1566,8 @@ bool radeon_crtc_scaling_mode_fixup(struct drm_crtc *crtc,
* unknown small number of scanlines wrt. real scanout position.
*
*/
-int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, int *vpos,
int *hpos,
- ktime_t *stime, ktime_t *etime)
+int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int
flags,
+ int *vpos, int *hpos, ktime_t *stime, ktime_t
*etime)
{
u32 stat_crtc = 0, vbl = 0, position = 0;
int vbl_start, vbl_end, vtotal, ret = 0;
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
b/drivers/gpu/drm/radeon/radeon_mode.h
index 3bfa910..c4016dc 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -758,6 +758,7 @@ extern int radeon_crtc_cursor_move(struct drm_crtc *crtc,
int x, int y);
extern int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc,
+ unsigned int flags,
int *vpos, int *hpos, ktime_t *stime,
ktime_t *etime);
diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
b/drivers/gpu/drm/radeon/radeon_pm.c
index 98bf63b..a394049 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -1468,7 +1468,7 @@ static bool radeon_pm_in_vbl(struct radeon_device *rdev)
*/
for (crtc = 0; (crtc rdev-num_crtc) in_vbl; crtc++) {
if (rdev-pm.active_crtcs (1 crtc)) {
- vbl_status = radeon_get_crtc_scanoutpos(rdev-ddev,
crtc, vpos, hpos, NULL, NULL);
+ vbl_status =
[Intel-gfx] [PATCH 13/14] drm/radeon: Move the early vblank IRQ fixup to radeon_get_crtc_scanoutpos()
From: Ville Syrjälä ville.syrj...@linux.intel.com
i915 doesn't need this kludge for most platforms. Although we do
appear to need something similar on certain platforms, but we can
be more accurate when we apply the adjustment since we know exactly
why the scanline counter doesn't always quite match the vblank
status.
Also the current code doesn't handle interlaced modes correctly,
and we already deal with interlaced modes in i915 code.
So let's just move the current code to radeon_get_crtc_scanoutpos()
since that's why it was added. For i915 we'll add a more finely
targeted variant.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_irq.c | 25 ++---
drivers/gpu/drm/radeon/radeon_display.c | 22 ++
2 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index b39255f..a1cc1a3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -542,7 +542,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device
*dev, int crtc,
{
ktime_t stime, etime, mono_time_offset;
struct timeval tv_etime;
- int vbl_status, vtotal, vdisplay;
+ int vbl_status;
int vpos, hpos, i;
int framedur_ns, linedur_ns, pixeldur_ns, delta_ns, duration_ns;
bool invbl;
@@ -558,9 +558,6 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device
*dev, int crtc,
return -EIO;
}
- vtotal = mode-crtc_vtotal;
- vdisplay = mode-crtc_vdisplay;
-
/* Durations of frames, lines, pixels in nanoseconds. */
framedur_ns = refcrtc-framedur_ns;
linedur_ns = refcrtc-linedur_ns;
@@ -569,7 +566,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device
*dev, int crtc,
/* If mode timing undefined, just return as no-op:
* Happens during initial modesetting of a crtc.
*/
- if (vtotal = 0 || vdisplay = 0 || framedur_ns == 0) {
+ if (framedur_ns == 0) {
DRM_DEBUG(crtc %d: Noop due to uninitialized mode.\n, crtc);
return -EAGAIN;
}
@@ -631,24 +628,6 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct
drm_device *dev, int crtc,
*/
delta_ns = vpos * linedur_ns + hpos * pixeldur_ns;
- /* Is vpos outside nominal vblank area, but less than
-* 1/100 of a frame height away from start of vblank?
-* If so, assume this isn't a massively delayed vblank
-* interrupt, but a vblank interrupt that fired a few
-* microseconds before true start of vblank. Compensate
-* by adding a full frame duration to the final timestamp.
-* Happens, e.g., on ATI R500, R600.
-*
-* We only do this if DRM_CALLED_FROM_VBLIRQ.
-*/
- if ((flags DRM_CALLED_FROM_VBLIRQ) !invbl
- ((vdisplay - vpos) vtotal / 100)) {
- delta_ns = delta_ns - framedur_ns;
-
- /* Signal this correction as applied. */
- vbl_status |= 0x8;
- }
-
if (!drm_timestamp_monotonic)
etime = ktime_sub(etime, mono_time_offset);
diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c
index 3581570..9d02fa7 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1709,5 +1709,27 @@ int radeon_get_crtc_scanoutpos(struct drm_device *dev,
int crtc, unsigned int fl
if (in_vbl)
ret |= DRM_SCANOUTPOS_INVBL;
+ /* Is vpos outside nominal vblank area, but less than
+* 1/100 of a frame height away from start of vblank?
+* If so, assume this isn't a massively delayed vblank
+* interrupt, but a vblank interrupt that fired a few
+* microseconds before true start of vblank. Compensate
+* by adding a full frame duration to the final timestamp.
+* Happens, e.g., on ATI R500, R600.
+*
+* We only do this if DRM_CALLED_FROM_VBLIRQ.
+*/
+ if ((flags DRM_CALLED_FROM_VBLIRQ) !in_vbl) {
+ vbl_start =
rdev-mode_info.crtcs[crtc]-base.hwmode.crtc_vdisplay;
+ vtotal = rdev-mode_info.crtcs[crtc]-base.hwmode.crtc_vtotal;
+
+ if (vbl_start - *vpos vtotal / 100) {
+ vpos -= vtotal;
+
+ /* Signal this correction as applied. */
+ ret |= 0x8;
+ }
+ }
+
return ret;
}
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/14] drm: Change {pixel, line, frame}dur_ns from s64 to int
From: Ville Syrjälä ville.syrj...@linux.intel.com
Using s64 for the timestamping constants is wasteful. Signed 32bit
integers get us a range of over +-2 seconds. Presuming that no-one
wants to a vrefresh rate less than 0.5, we can switch to using int
for the timestamping constants. We save a few bytes in drm_crtc and
avoid a bunch of 64bit math.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_irq.c | 18 +-
include/drm/drm_crtc.h| 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 77be2fb..b21a226 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -450,7 +450,7 @@ int drm_control(struct drm_device *dev, void *data,
void drm_calc_timestamping_constants(struct drm_crtc *crtc,
const struct drm_display_mode *mode)
{
- s64 linedur_ns = 0, pixeldur_ns = 0, framedur_ns = 0;
+ int linedur_ns = 0, pixeldur_ns = 0, framedur_ns = 0;
int dotclock = mode-crtc_clock;
/* Fields of interlaced scanout modes are only halve a frame duration.
@@ -483,8 +483,8 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
crtc-base.id, mode-crtc_htotal,
mode-crtc_vtotal, mode-crtc_vdisplay);
DRM_DEBUG(crtc %d: clock %d kHz framedur %d linedur %d, pixeldur %d\n,
- crtc-base.id, dotclock, (int) framedur_ns,
- (int) linedur_ns, (int) pixeldur_ns);
+ crtc-base.id, dotclock, framedur_ns,
+ linedur_ns, pixeldur_ns);
}
EXPORT_SYMBOL(drm_calc_timestamping_constants);
@@ -544,7 +544,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device
*dev, int crtc,
struct timeval tv_etime;
int vbl_status, vtotal, vdisplay;
int vpos, hpos, i;
- s64 framedur_ns, linedur_ns, pixeldur_ns, delta_ns, duration_ns;
+ int framedur_ns, linedur_ns, pixeldur_ns, delta_ns, duration_ns;
bool invbl;
if (crtc 0 || crtc = dev-num_crtcs) {
@@ -605,18 +605,18 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct
drm_device *dev, int crtc,
duration_ns = ktime_to_ns(etime) - ktime_to_ns(stime);
/* Accept result with max_error nsecs timing uncertainty. */
- if (duration_ns = (s64) *max_error)
+ if (duration_ns = *max_error)
break;
}
/* Noisy system timing? */
if (i == DRM_TIMESTAMP_MAXRETRIES) {
DRM_DEBUG(crtc %d: Noisy timestamp %d us %d us [%d reps].\n,
- crtc, (int) duration_ns/1000, *max_error/1000, i);
+ crtc, duration_ns/1000, *max_error/1000, i);
}
/* Return upper bound of timestamp precision error. */
- *max_error = (int) duration_ns;
+ *max_error = duration_ns;
/* Check if in vblank area:
* vpos is =0 in video scanout area, but negative
@@ -629,7 +629,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device
*dev, int crtc,
* since start of scanout at first display scanline. delta_ns
* can be negative if start of scanout hasn't happened yet.
*/
- delta_ns = (s64) vpos * linedur_ns + (s64) hpos * pixeldur_ns;
+ delta_ns = vpos * linedur_ns + hpos * pixeldur_ns;
/* Is vpos outside nominal vblank area, but less than
* 1/100 of a frame height away from start of vblank?
@@ -667,7 +667,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device
*dev, int crtc,
crtc, (int)vbl_status, hpos, vpos,
(long)tv_etime.tv_sec, (long)tv_etime.tv_usec,
(long)vblank_time-tv_sec, (long)vblank_time-tv_usec,
- (int)duration_ns/1000, i);
+ duration_ns/1000, i);
vbl_status = DRM_VBLANKTIME_SCANOUTPOS_METHOD;
if (invbl)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d3a91ad..d94c25f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -447,7 +447,7 @@ struct drm_crtc {
uint16_t *gamma_store;
/* Constants needed for precise vblank and swap timestamping. */
- s64 framedur_ns, linedur_ns, pixeldur_ns;
+ int framedur_ns, linedur_ns, pixeldur_ns;
/* if you are using the helper */
void *helper_private;
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [Draft] Testing Requirements for drm/i915 Patches
Hi all, So in the past half year we've had tons of sometimes rather heated discussions about getting patches merged. Often these discussions have been in the context of specific patch series, which meant that people are already invested. Which contributed to the boiling emotions. I'd like to avoid that here by making this a free-standing discussion. There's a bunch of smaller process tuning going on, but the big thing I'd like to instate henceforth is that automated test coverage is a primary consideration for anything going upstream. In this write up I'll detail my reasons, considerations and expectations. My plan is to solicit feedback over the next few days and then publish an edited and polished version to my blog. After that I'll put down my foot on this process so that we can go back to coding and stop blowing through so much time and energy on waging flamewars. Feedback and critique highly welcome. Cheers, Daniel Testing Requirements for Upstreaming (Draft) I want to make automated test coverage an integral part of our feature and bufix development process. For features this means that starting with the design phase testability needs to be considered an integral part of any feature. This needs to go up through the entire development process until when the implementation is submitted together with the proposed tests. For bugfixes that means the fix is only complete once the automated testcase for it is also done, if we need a new one. This specifically excludes testing with humans somewhere in the loop. We are extremely limited in our validation resources, every time we put something new onto the manual testing plate something else _will_ fall off. Why? - More predictability. Right now test coverage often only comes up as a topic when I drop my maintainer review onto a patch series. Which is too late, since it'll delay the otherwise working patches and so massively frustrates people. I hope by making test requirements clear and up-front we can make the upstreaming process more predictable. Also, if we have good tests from the get-go there should be much less need for me to drop patches from my trees after having them merged. - Less bikeshedding. In my opinion test cases are an excellent means to settle bikesheds - we've had in the past seen cases of endless backforths where writing a simple testcase would have shown that _all_ proposed color flavours are actually broken. The even more important thing is that fully automated tests allow us to legitimately postpone cleanups. If the only testing we have is manual testing then we have only one shot at a feature tested, namely when the developer tests it. So it better be perfect. But with automated tests we can postpone cleanups with too high risks of regressions until a really clear need is established. And since that need often never materializes we'll save work. - Better review. For me it's often helps a lot to review tests than the actual code in-depth. This is especially true for reviewing userspace interface additions. - Actionable regression reports. Only if we have a fully automated testcase do we have a good chance that QA reports a regression within just a few days. Everything else can easily take weeks (for platforms and features which are explicitly tested) to months (for stuff only users from the community notice). And especially now that much more shipping products depend upon a working i915.ko driver we just can't do this any more. - Better tests. A lot of our code is really hard to test in an automated fashion, and pushing the frontier of what is testable often requires a lot of work. I hope that by making tests an integral part of any feature work and so forcing more people to work on them and think about testing we'll advance the state of the art at a brisker pace. Risks and Buts -- - Bikeshedding on tests. This plan is obviously not too useful if we just replace massive bikeshedding on patches with massive bikeshedding on testcases. But right now we do almost no review on i-g-t patches so the risk is small. Long-term the review requirements for testcases will certainly increase, but as with everything else we simply need to strive for a good balance to strike for just the right amount of review. Also if we really start discussing tests _before_ having written massive patch series we'll do the bikeshedding while there's no real rebase pain. So even if the bikeshedding just shifts we'll benefit I think, especially for really big features. - Technical debt in test coverage. We have a lot of old code which still completely lacks testcases. Which means that even small feature work might be on the hook for a big pile of debt restructuring. I think this is inevitable occasionally. But I think that doing an assement of the current state of test coverage of the existing code _before_
[Intel-gfx] [PATCH] drm/i915: Capture batchbuffer state upon GPU hang
The bbstate contains useful bits of debugging information such as
whether the batch is being read from GTT or PPGTT, or whether it is
allowed to execute privileged instructions.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gpu_error.c | 2 ++
drivers/gpu/drm/i915/i915_reg.h | 1 +
3 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3d65b511e320..ca1b223356eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -301,6 +301,7 @@ struct drm_i915_error_state {
u32 cpu_ring_tail[I915_NUM_RINGS];
u32 error; /* gen6+ */
u32 err_int; /* gen7 */
+ u32 bbstate[I915_NUM_RINGS];
u32 instpm[I915_NUM_RINGS];
u32 instps[I915_NUM_RINGS];
u32 extra_instdone[I915_NUM_INSTDONE_REG];
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 918d978dce4a..cdd9121a28bb 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -251,6 +251,7 @@ static void i915_ring_error_state(struct
drm_i915_error_state_buf *m,
if (ring == RCS INTEL_INFO(dev)-gen = 4)
err_printf(m, BBADDR: 0x%08llx\n, error-bbaddr);
+ err_printf(m, BB_STATE: 0x%08x\n, error-bbstate[ring]);
if (INTEL_INFO(dev)-gen = 4)
err_printf(m, INSTPS: 0x%08x\n, error-instps[ring]);
err_printf(m, INSTPM: 0x%08x\n, error-instpm[ring]);
@@ -735,6 +736,7 @@ static void i915_record_ring_state(struct drm_device *dev,
}
error-waiting[ring-id] = waitqueue_active(ring-irq_queue);
+ error-bbstate[ring-id] = I915_READ(RING_BBSTATE(ring-mmio_base));
error-instpm[ring-id] = I915_READ(RING_INSTPM(ring-mmio_base));
error-seqno[ring-id] = ring-get_seqno(ring, false);
error-acthd[ring-id] = intel_ring_get_active_head(ring);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dbad19f4a983..9d79d28decb6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -722,6 +722,7 @@
#define NOPID 0x02094
#define HWSTAM 0x02098
#define DMA_FADD_I8XX 0x020d0
+#define RING_BBSTATE(base) ((base)+0x110)
#define ERROR_GEN6 0x040a0
#define GEN7_ERR_INT 0x44040
--
1.8.4.rc3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/7] drm/i915: add i915_get_reset_stats_ioctl
On 10/27/2013 05:30 AM, Daniel Vetter wrote:
On Fri, Oct 25, 2013 at 06:42:35PM -0700, Ian Romanick wrote:
Since the Mesa merge window is closing soon, I'm finally getting back on
this. I've pushed a rebase of my old Mesa branch to my fd.o repo
http://cgit.freedesktop.org/~idr/mesa/log/?h=robustness3
I have a couple questions...
1. Has any of this landed an a kernel tree anywhere?
Afaik everything but the actual ioctl and i-g-t testcase has landed.
And that stuff will land once my patches hit the Mesa list or ... ?
2. Has any support code landed in a libdrm tree anywhere?
Dunno whether Mika has libdrm patches. Since mesa is the only expected
user I'd just go with putting the ioctl wrapper (using the drmIoctl
helper) into mesa itself, that get rids of a dep for merging this support.
What's the right way to get the ctx_id out of the drm_intel_context?
That struct is private to libdrm, but the ioctl needs it.
3. What method should I use to detect that the kernel has support? In
early discussions, reset notification was only going to be available on
some GPUs, so there was a getparam to detect actual availability. I
guess now it's just based on kernel version?
Usually we add a new feature flag to get get_param ioctl if there's no
natural way otherwise for userspace to figure this out (usually by calling
the new ioctl and disabling the feature if that doesn't work).
-Daniel
It looks like I should just need to update df87cdd and 61dad8e in my
Mesa tree.
On 07/03/2013 07:22 AM, Mika Kuoppala wrote:
This ioctl returns reset stats for specified context.
The struct returned contains context loss counters.
reset_count:all resets across all contexts
batch_active: active batches lost on resets
batch_pending: pending batches lost on resets
v2: get rid of state tracking completely and deliver only counts. Idea
from Chris Wilson.
v3: fix commit message
v4: default context handled inside i915_gem_contest_get_hang_stats
v5: reset_count only for priviledged process
v6: ctx=0 needs CAP_SYS_ADMIN for batch_* counters (Chris Wilson)
v7: context hang stats never returns NULL
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
Cc: Ian Romanick i...@freedesktop.org
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_dma.c |1 +
drivers/gpu/drm/i915/i915_drv.c | 34 ++
drivers/gpu/drm/i915/i915_drv.h |2 ++
include/uapi/drm/i915_drm.h | 17 +
4 files changed, 54 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index 0e22142..d1a006f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1889,6 +1889,7 @@ struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE,
i915_gem_context_create_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY,
i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),
+ DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl,
DRM_UNLOCKED),
};
int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index 33cb973..0d4e3a8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1350,3 +1350,37 @@ int i915_reg_read_ioctl(struct drm_device *dev,
return 0;
}
+
+int i915_get_reset_stats_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *file)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_i915_reset_stats *args = data;
+ struct i915_ctx_hang_stats *hs;
+ int ret;
+
+ if (args-ctx_id == 0 !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ ret = mutex_lock_interruptible(dev-struct_mutex);
+ if (ret)
+ return ret;
+
+ hs = i915_gem_context_get_hang_stats(dev, file, args-ctx_id);
+ if (IS_ERR(hs)) {
+ mutex_unlock(dev-struct_mutex);
+ return PTR_ERR(hs);
+ }
+
+ if (capable(CAP_SYS_ADMIN))
+ args-reset_count = i915_reset_count(dev_priv-gpu_error);
+ else
+ args-reset_count = 0;
+
+ args-batch_active = hs-batch_active;
+ args-batch_pending = hs-batch_pending;
+
+ mutex_unlock(dev-struct_mutex);
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 1def049..0ca98fa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2021,6 +2021,8 @@ extern int intel_enable_rc6(const struct drm_device
*dev);
extern bool i915_semaphore_is_enabled(struct drm_device *dev);
int i915_reg_read_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
+int i915_get_reset_stats_ioctl(struct
Re: [Intel-gfx] [PATCH 00/66] [v1] Full PPGTT minus soft pin
Daniel Vetter dan...@ffwll.ch writes: Hi Ben So first things first: I rather like what the code looks like overall at the end. I've done a light read-through (by far not a full review) and besides a few bikesheds (all mentioned by mail already) the big thing is the 1:1 context:ppgtt address space relationship. We've discussed this at length in private irc and agreed that we need to changes this two a n:1 relationship, so I'll just reiterate the reasons for that on the list: - Current userspace expects that different contexts created on the same fd all use the same address space (since there's really only one). So if we want to no add a new ABI (and for testing I really think we want to enable ppgtt on current unchanged userspace) we must keep that promise. Hence we need to be able to point the different contexts created on an fd all at the same (per-fd) address space. I'm not coming up with anything in userland requiring this. Can you clarify? For the GL context reset stuff, it is required that we have more than one address space per fd, because the fd is global to all contexts, not just a share group. pgpRgg39_COzQ.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i810 forgets configured rows columns on ttys on startx shutdown
On 2013-10-29 13:54 (GMT+0100) Daniel Vetter composed: Felix Miata wrote: On 2013-10-29 09:51 (GMT+0100) Daniel Vetter composed: Felix Miata wrote: Pre-KMS it used to be that common vga= modes were useless, so I took to using 0x121 to escape from 80x25 screens. Using 0x8121 required specifying an appropriate font via config file. Even after KMS began, vga=0x8121 continued to work, and still does at least through kernel 3.4.47. This is still pre-kms, there's no kms driver for i810 chipsets. So whatever bug there is might be just bad luck or the userspace X driver not restoring stuff correctly. But I'm pretty sure it's not a kernel bug, at least no in the i810 driver. I was hoping for a (what appears to be a not our bug) response that was more helpful WRT whose bug it likely is. To me it's clearly a bug report that either can be found or needs to be made at bugs.freedesktop.org, but in what component? https://bugs.freedesktop.org/describecomponents.cgi?product=xorg has a long list, among which there is only one that includes the string intel, Driver/intel, which is what this list seems to be about. Does it look like Server/General, Server/DDX/Xorg, or something else? As it manifest in more than one distro, it surely isn't likely to be downstream. Userspace modesetting is known to be racy and broken, so people don't really want to waste time on digging into bugs which are most likely flukes. But if you can bisect this issue to a specific patch a fix should be possible. Again I got no clear answer to the question asked, so I guess now it's fair to assume this must be the right place for discussion after all. I've installed openSUSE 12.3. Its 3.7.10-1.16-default and server 1.13.2 behave nicely. Beyond installing Mageia 3 to get 3.8.13 and 1.13.4, and as one who does not build software even via proxy, I'm open to suggestions how to get kernels and/or other xorg versions needed to perform bisection. -- The wise are known for their understanding, and pleasant words are persuasive. Proverbs 16:21 (New Living Translation) Team OS/2 ** Reg. Linux User #211409 ** a11y rocks! Felix Miata *** http://fm.no-ip.com/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/66] [v1] Full PPGTT minus soft pin
On Tue, 29 Oct 2013 16:08:24 -0700 Eric Anholt e...@anholt.net wrote: Daniel Vetter dan...@ffwll.ch writes: Hi Ben So first things first: I rather like what the code looks like overall at the end. I've done a light read-through (by far not a full review) and besides a few bikesheds (all mentioned by mail already) the big thing is the 1:1 context:ppgtt address space relationship. We've discussed this at length in private irc and agreed that we need to changes this two a n:1 relationship, so I'll just reiterate the reasons for that on the list: - Current userspace expects that different contexts created on the same fd all use the same address space (since there's really only one). So if we want to no add a new ABI (and for testing I really think we want to enable ppgtt on current unchanged userspace) we must keep that promise. Hence we need to be able to point the different contexts created on an fd all at the same (per-fd) address space. I'm not coming up with anything in userland requiring this. Can you clarify? For the GL context reset stuff, it is required that we have more than one address space per fd, because the fd is global to all contexts, not just a share group. I think Daniel was just worried about the potential semantic change? But if userspace doesn't rely on it, we can go the easier route of simply creating one address space per context. But overall, do we need to allow creating multiple contexts in the same address space for GL share groups or any other feature? If so, we'd need to track contexts and address spaces separately and refcount them like Ben has done with the per-fd work, though we could go back to sharing a single fd and exposing the feature through the context create ioctl instead, or possibly a new one if we need the notion of an ASID as a separate entity. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Draft] Testing Requirements for drm/i915 Patches
Since a number of people internally are also involved in i915 development but not on the mailing list, I think we'll need to have an internal meeting or two to cover this stuff and get buy in. Overall, developing tests along with code is a good goal. A few comments below. On Tue, 29 Oct 2013 20:00:49 +0100 Daniel Vetter dan...@ffwll.ch wrote: - Tests must fully cover userspace interfaces. By this I mean exercising all the [snip] - Tests need to provide a reasonable baseline coverage of the internal driver state. The idea here isn't to aim for full coverage, that's an impossible and [snip] What you've described here is basically full validation. Something that most groups at Intel have large teams to dedicate all their time to. I'm not sure how far we can go down this path with just the development resources we have today (though maybe we'll get some help from the validation teams in the product groups) Finally the short lists of excuses that don't count as proper test coverage for a feature. - Manual testing. We are ridiculously limited on our QA manpower. Every time we drop something onto the manual testing plate something else _will_ drop off. Which means in the end that we don't really have any test coverage. So if patches don't come with automated tests, in-kernel cross-checking or some other form of validation attached they need to have really good reasons for doing so. Some things are only testable manually at this point, since we don't have a sophisticated webcam structure set up for everything (and in fact, the webcam tests we do have are fairly manual at this point, in that they have to be set up specially each time). - Testing by product teams. The entire point of Intel OTC's upstream first strategy is to have a common codebase for everyone. If we break product trees every time we feed an update into them because we can't properly regression test a given feature then the value of upstreaming features is greatly diminished in my opinion and could potentially doom collaborations with product teams. We just can't have that. This means that when products teams submit patches upstream they also need to submit the relevant testcases to i-g-t. So what I'm hearing here is that even if someone submits a tested patch, with tests available (and passing) somewhere other than i-g-t, you'll reject them until they port/write a new test for i-g-t. Is that what you meant? I think a more reasonable criteria would be that tests from non-i-g-t test suites are available and run by our QA, or run against upstream kernels by groups other than our QA. That should keep a lid on regressions just as well. One thing you didn't mention here is that our test suite is starting to see as much churn (and more breakage) than upstream. If you look at recent results from QA, you'd think SNB was totally broken based on i-g-t results. But despite that, desktops come up fine and things generally work. So we need to take care that our tests are simple and that our test library code doesn't see massive churn causing false positive breakage all the time. In other words, tests are just as likely to be broken (reporting false breakage or false passing) as the code they're testing. The best way to avoid that is to keep the tests very small, simple, and targeted. Converting and refactoring code in i-g-t to allow that will be a big chunk of work. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 0/4] Fix Win8 backlight issue
On Wednesday, October 30, 2013 12:40:13 AM Matthew Garrett wrote: On Wed, 2013-10-16 at 01:33 +0200, Rafael J. Wysocki wrote: Since the next step will be to introduce a list of systems that need video.use_native_backlight=1 *and* don't break in that configuration, I don't see much point adding another Kconfig option for the default. I'd still really prefer not to add such a list, because it almost inevitably means that we'll never fix this problem properly. We have this list in blacklist.c anyway. Thanks! -- I speak only for myself. Rafael J. Wysocki, 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 v5 0/4] Fix Win8 backlight issue
On Wed, 2013-10-16 at 01:33 +0200, Rafael J. Wysocki wrote: Since the next step will be to introduce a list of systems that need video.use_native_backlight=1 *and* don't break in that configuration, I don't see much point adding another Kconfig option for the default. I'd still really prefer not to add such a list, because it almost inevitably means that we'll never fix this problem properly. -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/vlv: Fix typo in the DPIO register define.
Incorrect definition DPIO_TX3_SWING_CTL4. Signed-off-by: Chon Ming Lee chon.ming@intel.com --- drivers/gpu/drm/i915/i915_reg.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 6799d53..f7ecad2 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -444,7 +444,7 @@ #define _DPIO_TX3_SWING_CTL4_A 0x690 #define _DPIO_TX3_SWING_CTL4_B 0x2a90 -#define DPIO_TX3_SWING_CTL4(pipe) _PIPE(pipe, _DPIO_TX_SWING_CTL4_A, \ +#define DPIO_TX3_SWING_CTL4(pipe) _PIPE(pipe, _DPIO_TX3_SWING_CTL4_A, \ _DPIO_TX3_SWING_CTL4_B) /* -- 1.7.7.6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915/vlv: Rename VLV DPIO register to be more structure to match configdb document.
Some VLV PHY/PLL DPIO registers have group/lane/channel access. Current DPIO register definition doesn't have a structure way to break them down. As a result it is not easy to match the PHY/PLL registers with the configdb document. Rename those registers based on the configdb for easy cross references, and without the need to check the offset in the header file. New format is as following. platform name_DPIO componentoptional lane #_DWdword # in the doc_optional channel # For example, VLV_PCS_DW0 - Group access to PCS for lane 0 to 3 for PCS DWORD 0. VLV_PCS01_DW0_CH0 - PCS access to lane 0/1, channel 0 for PCS DWORD 0. Another example is VLV_TX_DW0 - Group access to TX lane 0 to 3 for TX DWORD 0 VLV_TX0_DW0 - Refer to TX Lane 0 access only for TX DWORD 0. There is no functional change on this patch. Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Chon Ming Lee chon.ming@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 40 drivers/gpu/drm/i915/i915_reg.h | 189 -- drivers/gpu/drm/i915/intel_display.c | 48 +- drivers/gpu/drm/i915/intel_dp.c | 32 +++--- drivers/gpu/drm/i915/intel_hdmi.c| 54 -- 5 files changed, 171 insertions(+), 192 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 5c45e9e..f6c4486 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1650,28 +1650,28 @@ static int i915_dpio_info(struct seq_file *m, void *data) seq_printf(m, DPIO_CTL: 0x%08x\n, I915_READ(DPIO_CTL)); - seq_printf(m, DPIO_DIV_A: 0x%08x\n, - vlv_dpio_read(dev_priv, PIPE_A, _DPIO_DIV_A)); - seq_printf(m, DPIO_DIV_B: 0x%08x\n, - vlv_dpio_read(dev_priv, PIPE_A, _DPIO_DIV_B)); - - seq_printf(m, DPIO_REFSFR_A: 0x%08x\n, - vlv_dpio_read(dev_priv, PIPE_A, _DPIO_REFSFR_A)); - seq_printf(m, DPIO_REFSFR_B: 0x%08x\n, - vlv_dpio_read(dev_priv, PIPE_A, _DPIO_REFSFR_B)); - - seq_printf(m, DPIO_CORE_CLK_A: 0x%08x\n, - vlv_dpio_read(dev_priv, PIPE_A, _DPIO_CORE_CLK_A)); - seq_printf(m, DPIO_CORE_CLK_B: 0x%08x\n, - vlv_dpio_read(dev_priv, PIPE_A, _DPIO_CORE_CLK_B)); - - seq_printf(m, DPIO_LPF_COEFF_A: 0x%08x\n, - vlv_dpio_read(dev_priv, PIPE_A, _DPIO_LPF_COEFF_A)); - seq_printf(m, DPIO_LPF_COEFF_B: 0x%08x\n, - vlv_dpio_read(dev_priv, PIPE_A, _DPIO_LPF_COEFF_B)); + seq_printf(m, DPIO PLL DW3 CH0 : 0x%08x\n, + vlv_dpio_read(dev_priv, PIPE_A, VLV_PLL_DW3(0))); + seq_printf(m, DPIO PLL DW3 CH1: 0x%08x\n, + vlv_dpio_read(dev_priv, PIPE_A, VLV_PLL_DW3(1))); + + seq_printf(m, DPIO PLL DW5 CH0: 0x%08x\n, + vlv_dpio_read(dev_priv, PIPE_A, VLV_PLL_DW5(0))); + seq_printf(m, DPIO PLL DW5 CH1: 0x%08x\n, + vlv_dpio_read(dev_priv, PIPE_A, VLV_PLL_DW5(1))); + + seq_printf(m, DPIO PLL DW7 CH0: 0x%08x\n, + vlv_dpio_read(dev_priv, PIPE_A, VLV_PLL_DW7(0))); + seq_printf(m, DPIO PLL DW7 CH1: 0x%08x\n, + vlv_dpio_read(dev_priv, PIPE_A, VLV_PLL_DW7(1))); + + seq_printf(m, DPIO PLL DW12 CH0: 0x%08x\n, + vlv_dpio_read(dev_priv, PIPE_A, VLV_PLL_DW12(0))); + seq_printf(m, DPIO PLL DW12 CH1: 0x%08x\n, + vlv_dpio_read(dev_priv, PIPE_A, VLV_PLL_DW12(1))); seq_printf(m, DPIO_FASTCLK_DISABLE: 0x%08x\n, - vlv_dpio_read(dev_priv, PIPE_A, DPIO_FASTCLK_DISABLE)); + vlv_dpio_read(dev_priv, PIPE_A, VLV_CMN_DW0)); mutex_unlock(dev_priv-dpio_lock); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index dd8ff3b..98d0c78 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -442,17 +442,13 @@ #define DPIO_SFR_BYPASS (11) #define DPIO_CMNRST (10) -#define _DPIO_TX3_SWING_CTL4_A 0x690 -#define _DPIO_TX3_SWING_CTL4_B 0x2a90 -#define DPIO_TX3_SWING_CTL4(pipe) _PIPE(pipe, _DPIO_TX3_SWING_CTL4_A, \ - _DPIO_TX3_SWING_CTL4_B) #define DPIO_PHY_PORT(pipe)(dev_priv-vlv_phy[pipe 1]) /* * Per pipe/PLL DPIO regs */ -#define _DPIO_DIV_A0x800c +#define _VLV_PLL_DW3_CH0 0x800c #define DPIO_POST_DIV_SHIFT (28) /* 3 bits */ #define DPIO_POST_DIV_DAC0 #define DPIO_POST_DIV_HDMIDP 1 /* DAC 225-400M rate */ @@ -465,10 +461,10 @@ #define DPIO_ENABLE_CALIBRATION (111) #define DPIO_M1DIV_SHIFT (8) /* 3 bits */ #define DPIO_M2DIV_MASK 0xff -#define _DPIO_DIV_B0x802c -#define DPIO_DIV(pipe) _PIPE(pipe, _DPIO_DIV_A, _DPIO_DIV_B) +#define _VLV_PLL_DW3_CH1
[Intel-gfx] [PATCH 1/2] drm/i915/vlv: Make the vlv_dpio_read/vlv_dpio_write more PHY centric
vlv_dpio_read/write should be describe more in PHY centric instead of
display controller centric.
Create a enum dpio_channel for channel index and enum dpio_phy for PHY
index. This should better to gather for upcoming platform.
v2: Rebase the code based on
drm/i915/vlv: Fix typo in the DPIO register define.
Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Chon Ming Lee chon.ming@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 13 +
drivers/gpu/drm/i915/i915_reg.h |2 ++
drivers/gpu/drm/i915/intel_display.c | 16
drivers/gpu/drm/i915/intel_dp.c |9 -
drivers/gpu/drm/i915/intel_drv.h |7 ---
drivers/gpu/drm/i915/intel_hdmi.c |9 -
drivers/gpu/drm/i915/intel_sideband.c | 13 ++---
7 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2731fbb..b1609ae 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -88,6 +88,18 @@ enum port {
};
#define port_name(p) ((p) + 'A')
+#define I915_NUM_PHYS_VLV 1
+
+enum dpio_channel {
+ DPIO_CH0,
+ DPIO_CH1
+};
+
+enum dpio_phy {
+ DPIO_PHY0,
+ DPIO_PHY1
+};
+
enum intel_display_power_domain {
POWER_DOMAIN_PIPE_A,
POWER_DOMAIN_PIPE_B,
@@ -1401,6 +1413,7 @@ typedef struct drm_i915_private {
int num_shared_dpll;
struct intel_shared_dpll shared_dplls[I915_NUM_PLLS];
struct intel_ddi_plls ddi_plls;
+ int vlv_phy[I915_NUM_PHYS_VLV];
/* Reclocking support */
bool render_reclock_avail;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f7ecad2..dd8ff3b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -447,6 +447,8 @@
#define DPIO_TX3_SWING_CTL4(pipe) _PIPE(pipe, _DPIO_TX3_SWING_CTL4_A, \
_DPIO_TX3_SWING_CTL4_B)
+#define DPIO_PHY_PORT(pipe)(dev_priv-vlv_phy[pipe 1])
+
/*
* Per pipe/PLL DPIO regs
*/
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8f40ae3..c08f9f8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1361,6 +1361,7 @@ static void intel_init_dpio(struct drm_device *dev)
if (!IS_VALLEYVIEW(dev))
return;
+ dev_priv-vlv_phy[DPIO_PHY0] = IOSF_PORT_DPIO;
/*
* From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
* 6. De-assert cmn_reset/side_reset. Same as VLV X0.
@@ -1494,18 +1495,25 @@ static void vlv_disable_pll(struct drm_i915_private
*dev_priv, enum pipe pipe)
POSTING_READ(DPLL(pipe));
}
-void vlv_wait_port_ready(struct drm_i915_private *dev_priv, int port)
+void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
+ struct intel_digital_port *dport)
{
u32 port_mask;
- if (!port)
+ switch (dport-port) {
+ case PORT_B:
port_mask = DPLL_PORTB_READY_MASK;
- else
+ break;
+ case PORT_C:
port_mask = DPLL_PORTC_READY_MASK;
+ break;
+ default:
+ BUG();
+ }
if (wait_for((I915_READ(DPLL(0)) port_mask) == 0, 1000))
WARN(1, timed out waiting for port %c ready: 0x%08x\n,
-'B' + port, I915_READ(DPLL(0)));
+'B' + dport-port, I915_READ(DPLL(0)));
}
/**
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b3cc333..5d00c83 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1831,7 +1831,7 @@ static void vlv_pre_enable_dp(struct intel_encoder
*encoder)
struct drm_device *dev = encoder-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(encoder-base.crtc);
- int port = vlv_dport_to_channel(dport);
+ enum dpio_channel port = vlv_dport_to_channel(dport);
int pipe = intel_crtc-pipe;
struct edp_power_seq power_seq;
u32 val;
@@ -1839,7 +1839,6 @@ static void vlv_pre_enable_dp(struct intel_encoder
*encoder)
mutex_lock(dev_priv-dpio_lock);
val = vlv_dpio_read(dev_priv, pipe, DPIO_DATA_LANE_A(port));
- val = 0;
if (pipe)
val |= (121);
else
@@ -1858,7 +1857,7 @@ static void vlv_pre_enable_dp(struct intel_encoder
*encoder)
intel_enable_dp(encoder);
- vlv_wait_port_ready(dev_priv, port);
+ vlv_wait_port_ready(dev_priv, dport);
}
static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder)
@@ -1868,7 +1867,7 @@ static void vlv_dp_pre_pll_enable(struct intel_encoder
*encoder)
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc =
[Intel-gfx] [PATCH 2/4] drm: Push latency sensitive bits of vblank scanoutpos timestamping into kms drivers.
A change in locking of some kms drivers (currently intel-kms) make
the old approach too inaccurate and also incompatible with the
PREEMPT_RT realtime kernel patchset.
The driver-get_scanout_position() method of intel-kms now needs
to aquire a spinlock, which clashes badly with the former
preempt_disable() calls in the drm, and it also introduces larger
delays and timing uncertainty on a contended lock than acceptable.
This patch changes the prototype of driver-get_scanout_position()
to require/allow kms drivers to perform the ktime_get() system time
queries which go along with actual scanout position readout in a way
that provides maximum precision and to return those timestamps to
the drm. kms drivers implementations of get_scanout_position() are
asked to implement timestamping and scanoutpos readout in a way
that is as precise as possible and compatible with preempt_disable()
on a PREMPT_RT kernel. A driver should follow this pattern in
get_scanout_position() for precision and compatibility:
spin_lock...(...);
preempt_disable_rt(); // On a PREEMPT_RT kernel, otherwise omit.
if (stime) *stime = ktime_get();
... Minimum amount of MMIO register reads to get scanout position ...
... no taking of locks allowed here! ...
if (etime) *etime = ktime_get();
preempt_enable_rt(); // On PREEMPT_RT kernel, otherwise omit.
spin_unlock...(...);
v2: Fix formatting of new multi-line code comments.
Signed-off-by: Mario Kleiner mario.kleiner...@gmail.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Reviewed-by: Alex Deucher alexander.deuc...@amd.com
---
drivers/gpu/drm/drm_irq.c | 20
include/drm/drmP.h| 10 --
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 33ee515..d80d952 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -219,7 +219,7 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs)
for (i = 0; i num_crtcs; i++)
init_waitqueue_head(dev-vblank[i].queue);
- DRM_INFO(Supports vblank timestamp caching Rev 1 (10.10.2010).\n);
+ DRM_INFO(Supports vblank timestamp caching Rev 2 (21.10.2013).\n);
/* Driver specific high-precision vblank timestamping supported? */
if (dev-driver-get_vblank_timestamp)
@@ -586,14 +586,17 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct
drm_device *dev, int crtc,
* code gets preempted or delayed for some reason.
*/
for (i = 0; i DRM_TIMESTAMP_MAXRETRIES; i++) {
- /* Get system timestamp before query. */
- stime = ktime_get();
-
- /* Get vertical and horizontal scanout pos. vpos, hpos. */
- vbl_status = dev-driver-get_scanout_position(dev, crtc,
vpos, hpos);
+ /*
+* Get vertical and horizontal scanout position vpos, hpos,
+* and bounding timestamps stime, etime, pre/post query.
+*/
+ vbl_status = dev-driver-get_scanout_position(dev, crtc, vpos,
+ hpos, stime,
etime);
- /* Get system timestamp after query. */
- etime = ktime_get();
+ /*
+* Get correction for CLOCK_MONOTONIC - CLOCK_REALTIME if
+* CLOCK_REALTIME is requested.
+*/
if (!drm_timestamp_monotonic)
mono_time_offset = ktime_get_monotonic_offset();
@@ -604,6 +607,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device
*dev, int crtc,
return -EIO;
}
+ /* Compute uncertainty in timestamp of scanout position query.
*/
duration_ns = ktime_to_ns(etime) - ktime_to_ns(stime);
/* Accept result with max_error nsecs timing uncertainty. */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2b954ad..48d15f0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -835,12 +835,17 @@ struct drm_driver {
/**
* Called by vblank timestamping code.
*
-* Return the current display scanout position from a crtc.
+* Return the current display scanout position from a crtc, and an
+* optional accurate ktime_get timestamp of when position was measured.
*
* \param dev DRM device.
* \param crtc Id of the crtc to query.
* \param *vpos Target location for current vertical scanout position.
* \param *hpos Target location for current horizontal scanout position.
+* \param *stime Target location for timestamp taken immediately before
+* scanout position query. Can be NULL to skip timestamp.
+* \param *etime Target location for timestamp taken immediately after
+* scanout position query. Can be NULL to skip timestamp.
*
[Intel-gfx] Vblank timestamping improvements/fixes for Linux drm. [v2]
Hi Dave, this is v2 of the patch set for improving/restoring accuracy and robustness of vblank timestamping and for fixing incompatibilities with the PREEMPT_RT patches. Could you please merge this for the next kernel? Would be good to have the old accuracy restored as soon as possible. Thanks. v2: Added the reviewed-by's of Ville and Alex, thanks for the review! Fixed multi-line code formatting as suggested by Ville. Successfully tested on Intel and AMD Radeon hardware. thanks, -mario ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/intel: Push get_scanout_position() timestamping into kms driver.
Move the ktime_get() clock readouts and potential preempt_disable()
calls from drm core into kms driver to make it compatible with the
api changes in the drm core.
The intel-kms driver needs to take the uncore.lock inside
i915_get_crtc_scanoutpos() and intel_pipe_in_vblank().
This is incompatible with the preempt_disable() on a
PREEMPT_RT patched kernel, as regular spin locks must not
be taken within a preempt_disable'd section. Lock contention
on the uncore.lock also introduced too much uncertainty in vblank
timestamps.
Push the ktime_get() timestamping for scanoutpos queries and
potential preempt_disable_rt() into i915_get_crtc_scanoutpos(),
so these problems can be avoided:
1. First lock the uncore.lock (might sleep on a PREEMPT_RT kernel).
2. preempt_disable_rt() (will be added by the rt-linux folks).
3. ktime_get() a timestamp before scanout pos query.
4. Do all mmio reads as fast as possible without grabbing any new locks!
5. ktime_get() a post-query timestamp.
6. preempt_enable_rt()
7. Unlock the uncore.lock.
This reduces timestamp uncertainty on a low-end HP Atom Mini netbook
with Intel GMA-950 nicely:
Before: 3-8 usecs with spikes 20 usecs, triggering query retries.
After : Typically 1 usec (98% of all samples), occassionally 2 usecs
(2% of all samples), with maximum of 3 usecs (a handful).
v2: Fix formatting of new multi-line code comments.
Signed-off-by: Mario Kleiner mario.kleiner...@gmail.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Reviewed-by: Alex Deucher alexander.deuc...@amd.com
---
drivers/gpu/drm/i915/i915_irq.c | 54 +++
1 file changed, 43 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 156a1a4..7cafe64 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -599,35 +599,40 @@ static u32 gm45_get_vblank_counter(struct drm_device
*dev, int pipe)
return I915_READ(reg);
}
-static bool intel_pipe_in_vblank(struct drm_device *dev, enum pipe pipe)
+/* raw reads, only for fast reads of display block, no need for forcewake etc.
*/
+#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)-regs +
(reg__))
+#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)-regs +
(reg__))
+
+static bool intel_pipe_in_vblank_locked(struct drm_device *dev, enum pipe pipe)
{
struct drm_i915_private *dev_priv = dev-dev_private;
uint32_t status;
+ int reg;
if (IS_VALLEYVIEW(dev)) {
status = pipe == PIPE_A ?
I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
- return I915_READ(VLV_ISR) status;
+ reg = VLV_ISR;
} else if (IS_GEN2(dev)) {
status = pipe == PIPE_A ?
I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
- return I915_READ16(ISR) status;
+ reg = ISR;
} else if (INTEL_INFO(dev)-gen 5) {
status = pipe == PIPE_A ?
I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT :
I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
- return I915_READ(ISR) status;
+ reg = ISR;
} else if (INTEL_INFO(dev)-gen 7) {
status = pipe == PIPE_A ?
DE_PIPEA_VBLANK :
DE_PIPEB_VBLANK;
- return I915_READ(DEISR) status;
+ reg = DEISR;
} else {
switch (pipe) {
default:
@@ -642,12 +647,17 @@ static bool intel_pipe_in_vblank(struct drm_device *dev,
enum pipe pipe)
break;
}
- return I915_READ(DEISR) status;
+ reg = DEISR;
}
+
+ if (IS_GEN2(dev))
+ return __raw_i915_read16(dev_priv, reg) status;
+ else
+ return __raw_i915_read32(dev_priv, reg) status;
}
static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
-int *vpos, int *hpos)
+int *vpos, int *hpos, ktime_t *stime, ktime_t
*etime)
{
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc = dev_priv-pipe_to_crtc_mapping[pipe];
@@ -657,6 +667,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev,
int pipe,
int vbl_start, vbl_end, htotal, vtotal;
bool in_vbl = true;
int ret = 0;
+ unsigned long irqflags;
if (!intel_crtc-active) {
DRM_DEBUG_DRIVER(trying to get scanoutpos for disabled
@@ -671,14 +682,27 @@ static int i915_get_crtc_scanoutpos(struct drm_device
*dev, int pipe,
ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE;
+ /*
+* Lock uncore.lock, as we will do multiple
[Intel-gfx] [PATCH 3/4] drm/radeon: Push get_scanout_position() timestamping into kms driver.
Move the ktime_get() clock readouts and potential preempt_disable()
calls from drm core into kms driver to make it compatible with the
api changes in the drm core.
This should not introduce any change in functionality or behaviour
in radeon-kms, just a reshuffling of code.
Signed-off-by: Mario Kleiner mario.kleiner...@gmail.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Reviewed-by: Alex Deucher alexander.deuc...@amd.com
---
drivers/gpu/drm/radeon/radeon_display.c | 24 +---
drivers/gpu/drm/radeon/radeon_drv.c |3 ++-
drivers/gpu/drm/radeon/radeon_mode.h|3 ++-
drivers/gpu/drm/radeon/radeon_pm.c |2 +-
4 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c
index 0d1aa05..ccd8751 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -306,7 +306,7 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev,
int crtc_id)
*/
if (update_pending
(DRM_SCANOUTPOS_VALID radeon_get_crtc_scanoutpos(rdev-ddev,
crtc_id,
- vpos, hpos))
+ vpos, hpos,
NULL, NULL))
((vpos = (99 *
rdev-mode_info.crtcs[crtc_id]-base.hwmode.crtc_vdisplay)/100) ||
(vpos 0 !ASIC_IS_AVIVO(rdev {
/* crtc didn't flip in this target vblank interval,
@@ -1539,12 +1539,17 @@ bool radeon_crtc_scaling_mode_fixup(struct drm_crtc
*crtc,
}
/*
- * Retrieve current video scanout position of crtc on a given gpu.
+ * Retrieve current video scanout position of crtc on a given gpu, and
+ * an optional accurate timestamp of when query happened.
*
* \param dev Device to query.
* \param crtc Crtc to query.
* \param *vpos Location where vertical scanout position should be stored.
* \param *hpos Location where horizontal scanout position should go.
+ * \param *stime Target location for timestamp taken immediately before
+ * scanout position query. Can be NULL to skip timestamp.
+ * \param *etime Target location for timestamp taken immediately after
+ * scanout position query. Can be NULL to skip timestamp.
*
* Returns vpos as a positive number while in active scanout area.
* Returns vpos as a negative number inside vblank, counting the number
@@ -1560,7 +1565,8 @@ bool radeon_crtc_scaling_mode_fixup(struct drm_crtc *crtc,
* unknown small number of scanlines wrt. real scanout position.
*
*/
-int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, int *vpos,
int *hpos)
+int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, int *vpos,
int *hpos,
+ ktime_t *stime, ktime_t *etime)
{
u32 stat_crtc = 0, vbl = 0, position = 0;
int vbl_start, vbl_end, vtotal, ret = 0;
@@ -1568,6 +1574,12 @@ int radeon_get_crtc_scanoutpos(struct drm_device *dev,
int crtc, int *vpos, int
struct radeon_device *rdev = dev-dev_private;
+ /* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
+
+ /* Get optional system timestamp before query. */
+ if (stime)
+ *stime = ktime_get();
+
if (ASIC_IS_DCE4(rdev)) {
if (crtc == 0) {
vbl = RREG32(EVERGREEN_CRTC_V_BLANK_START_END +
@@ -1650,6 +1662,12 @@ int radeon_get_crtc_scanoutpos(struct drm_device *dev,
int crtc, int *vpos, int
}
}
+ /* Get optional system timestamp after query. */
+ if (etime)
+ *etime = ktime_get();
+
+ /* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
+
/* Decode into vertical and horizontal scanout position. */
*vpos = position 0x1fff;
*hpos = (position 16) 0x1fff;
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index 22f6858..101e7c0 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -106,7 +106,8 @@ int radeon_gem_object_open(struct drm_gem_object *obj,
void radeon_gem_object_close(struct drm_gem_object *obj,
struct drm_file *file_priv);
extern int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc,
- int *vpos, int *hpos);
+ int *vpos, int *hpos, ktime_t *stime,
+ ktime_t *etime);
extern const struct drm_ioctl_desc radeon_ioctls_kms[];
extern int radeon_max_kms_ioctl;
int radeon_mmap(struct file *filp, struct vm_area_struct *vma);
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
b/drivers/gpu/drm/radeon/radeon_mode.h
index ef63d3f..3bfa910 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -758,7 +758,8 @@ extern
[Intel-gfx] [PATCH 1/4] drm: Remove preempt_disable() from vblank timestamping code.
Preemption handling will get pushed into the kms
drivers in followup patches, to make timestamping
more robust and PREEMPT_RT friendly.
Signed-off-by: Mario Kleiner mario.kleiner...@gmail.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Reviewed-by: Alex Deucher alexander.deuc...@amd.com
---
drivers/gpu/drm/drm_irq.c |7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index f9af048..33ee515 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -586,11 +586,6 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct
drm_device *dev, int crtc,
* code gets preempted or delayed for some reason.
*/
for (i = 0; i DRM_TIMESTAMP_MAXRETRIES; i++) {
- /* Disable preemption to make it very likely to
-* succeed in the first iteration even on PREEMPT_RT kernel.
-*/
- preempt_disable();
-
/* Get system timestamp before query. */
stime = ktime_get();
@@ -602,8 +597,6 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device
*dev, int crtc,
if (!drm_timestamp_monotonic)
mono_time_offset = ktime_get_monotonic_offset();
- preempt_enable();
-
/* Return as no-op if scanout query unsupported or failed. */
if (!(vbl_status DRM_SCANOUTPOS_VALID)) {
DRM_DEBUG(crtc %d : scanoutpos query failed [%d].\n,
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
