Re: [Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/10/2013 07:01 AM, Matthew Garrett wrote: Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems. Signed-off-by: Matthew Garrett matthew.garr...@nebula.com --- drivers/gpu/drm/i915/i915_dma.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 3b315ba..23b6292 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) /* Must be done after probing outputs */ intel_opregion_init(dev); acpi_video_register(); + /* Don't use ACPI backlight functions on Windows 8 platforms */ + if (acpi_osi_version() = ACPI_OSI_WIN_8) + acpi_video_backlight_unregister(); What about the platform driver created backlight interface? Oh right, thinkpad_acpi will not create backlight interface for them since they are not in DMI table, so acpi_video_backlight_support() will return true and thinkspad_acpi thinks ACPI video driver is controlling backlight so it will not create backlight interface for them. Then we will need to remember not to add any of those systems into the DMI table of video_detect.c, or thinkpad_acpi module will create backlight interface for them and according to reporter, that interface does not work either. What about a priority based solution? We can introduce a new field named priority to backlight_device and instead of calling another module's function like the unregister one here(which cause unnecessary module dependency), we only need to boost priority for its own interface. This field will be exported to sysfs, so user can change it during runtime too. And we can also introduce a new kernel command line as backlight.force_interface=raw/firmware/platform, to overcome the limited functionality provided by acpi_backlight=video/vendor, which does not involve GPU's interface. And we can place the quirk code in backlight layer instead of individual backlight functionality provider module. Suppose we have a backlight manager there, for all win8 systems, we can boost the raw type's priority on its registration, so no need to add code in intel/amd/etc./'s GPU driver code. With priority based solution, all backlight control interfaces stay, the priority field is an indication given by kernel to user space. For a complete description on backlight control background and the proposed solution, please take a look at: https://gist.github.com/aaronlu/5779828 It would be good to know your opinions, thanks. -Aaron ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove extra ring from error message
On Thu, Jun 13, 2013 at 09:33:33PM -0700, Ben Widawsky wrote: The ring names already have ring in it. CC: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Ben Widawsky b...@bwidawsk.net Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Kill useless Enable panel fitter comments
On Thu, Jun 13, 2013 at 06:19:39PM -0700, Jesse Barnes wrote: On Fri, 14 Jun 2013 00:51:23 +0200 Daniel Vetter daniel.vet...@ffwll.ch wrote: Now that we have this all nicely abstract into separate functions with self-documenting names this is pointless. And as Yuly Novikov spotted in the case of ilk-ivb also wrong since we use the pfit both for lvds and eDP Reported-By: Yuly Novikov ynovi...@chromium.org Cc: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e1184eb..2338279 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3209,7 +3209,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) assert_fdi_rx_disabled(dev_priv, pipe); } - /* Enable panel fitting for LVDS */ ironlake_pfit_enable(intel_crtc); /* @@ -3315,7 +3314,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_ddi_enable_pipe_clock(intel_crtc); - /* Enable panel fitting for eDP */ ironlake_pfit_enable(intel_crtc); /* @@ -3611,7 +3609,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); - /* Enable panel fitting for eDP */ i9xx_pfit_enable(intel_crtc); intel_crtc_load_lut(crtc); @@ -3649,7 +3646,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) i9xx_enable_pll(intel_crtc); - /* Enable panel fitting for LVDS */ i9xx_pfit_enable(intel_crtc); intel_crtc_load_lut(crtc); Acked-by: Jesse Barnes jbar...@virtuousgeek.org And now merged to dinq. -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 2/2] drm/i915: s/LFP/LPF in DPIO PLL register names
From: Ville Syrjälä ville.syrj...@linux.intel.com LPF is short for low pass filter. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 8 drivers/gpu/drm/i915/i915_reg.h | 6 +++--- drivers/gpu/drm/i915/intel_display.c | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d4e78b6..cc20637 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1862,10 +1862,10 @@ static int i915_dpio_info(struct seq_file *m, void *data) seq_printf(m, DPIO_CORE_CLK_B: 0x%08x\n, vlv_dpio_read(dev_priv, _DPIO_CORE_CLK_B)); - seq_printf(m, DPIO_LFP_COEFF_A: 0x%08x\n, - vlv_dpio_read(dev_priv, _DPIO_LFP_COEFF_A)); - seq_printf(m, DPIO_LFP_COEFF_B: 0x%08x\n, - vlv_dpio_read(dev_priv, _DPIO_LFP_COEFF_B)); + seq_printf(m, DPIO_LPF_COEFF_A: 0x%08x\n, + vlv_dpio_read(dev_priv, _DPIO_LPF_COEFF_A)); + seq_printf(m, DPIO_LPF_COEFF_B: 0x%08x\n, + vlv_dpio_read(dev_priv, _DPIO_LPF_COEFF_B)); seq_printf(m, DPIO_FASTCLK_DISABLE: 0x%08x\n, vlv_dpio_read(dev_priv, DPIO_FASTCLK_DISABLE)); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9cb6236..828a6ed 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -448,9 +448,9 @@ #define _DPIO_PLL_CML_B0x806c #define DPIO_PLL_CML(pipe) _PIPE(pipe, _DPIO_PLL_CML_A, _DPIO_PLL_CML_B) -#define _DPIO_LFP_COEFF_A 0x8048 -#define _DPIO_LFP_COEFF_B 0x8068 -#define DPIO_LFP_COEFF(pipe) _PIPE(pipe, _DPIO_LFP_COEFF_A, _DPIO_LFP_COEFF_B) +#define _DPIO_LPF_COEFF_A 0x8048 +#define _DPIO_LPF_COEFF_B 0x8068 +#define DPIO_LPF_COEFF(pipe) _PIPE(pipe, _DPIO_LPF_COEFF_A, _DPIO_LPF_COEFF_B) #define DPIO_CALIBRATION 0x80ac diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 465d6bc..f6eaba2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4407,10 +4407,10 @@ static void vlv_update_pll(struct intel_crtc *crtc) if (crtc-config.port_clock == 162000 || intel_pipe_has_type(crtc-base, INTEL_OUTPUT_ANALOG) || intel_pipe_has_type(crtc-base, INTEL_OUTPUT_HDMI)) - vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), + vlv_dpio_write(dev_priv, DPIO_LPF_COEFF(pipe), 0x005f0021); else - vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), + vlv_dpio_write(dev_priv, DPIO_LPF_COEFF(pipe), 0x00df); if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_EDP) || -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/10 v2] drm/i915: We implement WaFbcAsynchFlipDisableFbcQueue on ilk and snb
v2: Put the comment a bit closer to the actual write (Paulo Zanoni)
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 57e99b1..a3c4e06 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4422,6 +4422,7 @@ static void ironlake_init_clock_gating(struct drm_device
*dev)
* The bit 7,8,9 of 0x42020.
*/
if (IS_IRONLAKE_M(dev)) {
+ /* WaFbcAsynchFlipDisableFbcQueue:ilk */
I915_WRITE(ILK_DISPLAY_CHICKEN1,
I915_READ(ILK_DISPLAY_CHICKEN1) |
ILK_FBCQ_DIS);
@@ -4557,6 +4558,8 @@ static void gen6_init_clock_gating(struct drm_device *dev)
* The bit5 and bit7 of 0x42020
* The bit14 of 0x70180
* The bit14 of 0x71180
+*
+* WaFbcAsynchFlipDisableFbcQueue:snb
*/
I915_WRITE(ILK_DISPLAY_CHICKEN1,
I915_READ(ILK_DISPLAY_CHICKEN1) |
--
1.8.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] Haswell Display audio routing bug fix
This patchset used to fix some display audio issues on Haswell platform. I tested this patch on Haswell ult board, C0 stepping, with eDP pannel, HDMI monitor, DP monitor. The fixed issues: 1) when only HDMI or DP monitor connected, hear sound on ALL three HDMI devices. 2) when HDMI + DP monitors connected, hear sound on HDMI and DP at the same time no matter you're playing audio on DP or HDMI device. 3) When DP + HDMI + eDP connected, no sound could be heard. 4) After bootup, play audio on HDMI device 7(use second pin), no sound. If play audio on device 3 at first(use first pin), it has sound again. There's quite different hardware changes for haswell display audio, compared with previous Ivybridge/Sandybridge. The common hd-a driver and hdmi driver need make improvement to support. Haswell chip supports Dp1.2 feature(see ref 1). This is very cool feture windows had supported. We get confirm linux gfx team has plan to support Dp1.2 in future too. We're ready to support dp1.2 feature in audio side, as the converter selection dependency is fixed in this patchset. Also we need some improvement in HDA driver side because Haswell added new Verb to support DP1.2. here’s a video show about DP1.2 feature http://www.club-3d.com/index.php/products/reader.en/product/mst-hub-1-3.html Please apply the patches based on Daniel's drm-intel-next-queued branch: http://cgit.freedesktop.org/~danvet/drm-intel/log/?h=drm-intel-next-queued last commit 80e83831a64b9a5d49e844691037b2d4be0f14f9 Please feel free to let me know the issues you meet during test. Wang Xingchao (4): ALSA: hda - Haswell converter power state D0 verify ALSA: hda - Return error when open empty hdmi device drm/i915: Add display audio routing APIs for ALSA ALSA: hda - Add display audio routing API for haswell drivers/gpu/drm/i915/i915_drv.h | 18 + drivers/gpu/drm/i915/intel_ddi.c | 131 +-- drivers/gpu/drm/i915/intel_display.c | 7 +- drivers/gpu/drm/i915/intel_drv.h | 1 + include/drm/i915_powerwell.h | 5 ++ sound/pci/hda/hda_i915.c | 83 ++ sound/pci/hda/hda_i915.h | 4 ++ sound/pci/hda/patch_hdmi.c | 37 ++ 8 files changed, 281 insertions(+), 5 deletions(-) -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] ALSA: hda - Return error when open empty hdmi device
when user open HDMI device 3/7/8, if it has no physical device connected, return error. The bug is from Haswell platform, All pins choose converter 0 by default in hardware level, maybe only pin 1 had valid monitor connected. if user play audio on pin 0/2, pin 1 can get audio data too. Signed-off-by: Wang Xingchao xingchao.w...@linux.intel.com --- sound/pci/hda/patch_hdmi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 8db5eb6..d766f40 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1139,6 +1139,9 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo, per_pin = get_pin(spec, pin_idx); eld = per_pin-sink_eld; + if (!eld-monitor_present || !eld-eld_valid) + return -EIO; + if (codec-vendor_id == 0x80862807) hsw_verify_cvt_D0(spec, codec); -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Add display audio routing APIs for ALSA
ALSA audio driver need know current audio routing infomation.
i.e. Route map between codec pins(DDI ports) and Transcoders(Pipe).
Also the new API let audio driver disable unused audio pin's output.
This fixed the bug when three pins *ALL* have monitors connected, playing
audio on one pin would cause audio output to all minitors.
Signed-off-by: Wang Xingchao xingchao.w...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 18 +
drivers/gpu/drm/i915/intel_ddi.c | 131 +--
drivers/gpu/drm/i915/intel_display.c | 7 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
include/drm/i915_powerwell.h | 5 ++
5 files changed, 157 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 10a56c9..8248048 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -87,6 +87,7 @@ enum port {
I915_MAX_PORTS
};
#define port_name(p) ((p) + 'A')
+#define pin2port(p) ((p) + PORT_B)
enum intel_display_power_domain {
POWER_DOMAIN_PIPE_A,
@@ -785,6 +786,20 @@ struct i915_power_well {
int i915_request;
};
+#define DEFAULT_PIPE -1
+/* Dp1.2 mode, one DDI port can choose multiple pipes */
+struct dp12_port {
+ int pipes[I915_MAX_PIPES];
+ int count;
+};
+
+/* audio routing info for haswell */
+struct i915_audio {
+ /* route map between pipe and DDI port */
+ struct dp12_port active_pipes[I915_MAX_PORTS];
+ int pin_eld;
+};
+
struct i915_dri1_state {
unsigned allow_batchbuffer : 1;
u32 __iomem *gfx_hws_cpu_addr;
@@ -1147,6 +1162,9 @@ typedef struct drm_i915_private {
/* Haswell power well */
struct i915_power_well power_well;
+ /* Haswell audio routing */
+ struct i915_audio audio_route;
+
enum no_fbc_reason no_fbc_reason;
struct drm_mm_node *compressed_fb;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 224ce25..82823b8 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -286,8 +286,11 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *adjusted_mode)
{
struct drm_crtc *crtc = encoder-crtc;
+ struct drm_i915_private *dev_priv = crtc-dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
+ struct i915_audio *hsw_audio = dev_priv-audio_route;
+ struct dp12_port *hsw_port;
int port = intel_ddi_get_encoder_port(intel_encoder);
int pipe = intel_crtc-pipe;
int type = intel_encoder-type;
@@ -296,6 +299,12 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder,
port_name(port), pipe_name(pipe));
intel_crtc-eld_vld = false;
+
+ /* store pipe routing info */
+ hsw_port = hsw_audio-active_pipes[port];
+ hsw_port-pipes[0] = pipe;
+ hsw_port-count = 1;
+
if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
struct intel_digital_port *intel_dig_port =
@@ -306,8 +315,8 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder,
intel_dp-DP |= DDI_PORT_WIDTH(intel_dp-lane_count);
if (intel_dp-has_audio) {
- DRM_DEBUG_DRIVER(DP audio on pipe %c on DDI\n,
-pipe_name(intel_crtc-pipe));
+ DRM_DEBUG_DRIVER(DP audio on pipe %c on DDI %c\n,
+pipe_name(intel_crtc-pipe),
port_name(port));
/* write eld */
DRM_DEBUG_DRIVER(DP audio: write eld information\n);
@@ -324,8 +333,8 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder,
* and a new set of registers, so we leave it for future
* patch bombing.
*/
- DRM_DEBUG_DRIVER(HDMI audio on pipe %c on DDI\n,
-pipe_name(intel_crtc-pipe));
+ DRM_DEBUG_DRIVER(HDMI audio on pipe %c on DDI %c \n,
+pipe_name(intel_crtc-pipe),
port_name(port));
/* write eld */
DRM_DEBUG_DRIVER(HDMI audio: write eld information\n);
@@ -1135,6 +1144,9 @@ static void intel_disable_ddi(struct intel_encoder
*intel_encoder)
int type = intel_encoder-type;
struct drm_device *dev = encoder-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+ struct i915_audio *hsw_audio = dev_priv-audio_route;
+ struct dp12_port *hsw_port;
+ enum port port = intel_ddi_get_encoder_port(intel_encoder);
uint32_t tmp;
[Intel-gfx] [PATCH 4/4] ALSA: hda - Add display audio routing API for haswell
ALSA side use these apis to know display audio routing map
in gfx side. And use the API to disable unused pin's audio output.
Signed-off-by: Wang Xingchao xingchao.w...@linux.intel.com
---
sound/pci/hda/hda_i915.c | 83 ++
sound/pci/hda/hda_i915.h | 4 +++
sound/pci/hda/patch_hdmi.c | 20 +--
3 files changed, 104 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c
index 76c13d5..7ac446f 100644
--- a/sound/pci/hda/hda_i915.c
+++ b/sound/pci/hda/hda_i915.c
@@ -22,9 +22,73 @@
#include drm/i915_powerwell.h
#include hda_i915.h
+/* Haswell power well */
static void (*get_power)(void);
static void (*put_power)(void);
+/* Haswell audio routing */
+static int (*get_using_pipe)(int);
+static int (*disable_unused_pipe)(int, int *);
+static int (*restore_eld)(void);
+
+#define i915_pipe_name(p) ((p) + 'A')
+
+static int busy_pins[3] = {0, 0, 0};
+
+int hdmi_disable_unused_pipe(int pin_idx, int pipe_idx)
+{
+ busy_pins[pin_idx] = 1;
+ if (disable_unused_pipe)
+ disable_unused_pipe(pipe_idx, busy_pins);
+
+ return 0;
+}
+EXPORT_SYMBOL(hdmi_disable_unused_pipe);
+
+void hdmi_restore_pineld(int pin_idx)
+{
+ busy_pins[pin_idx] = 0;
+ if (restore_eld)
+ restore_eld();
+}
+EXPORT_SYMBOL(hdmi_restore_pineld);
+
+int hdmi_get_using_pipe(int pin_idx)
+{
+ int pipe = -1;
+
+ if (get_using_pipe)
+ pipe = get_using_pipe(pin_idx);
+
+ if (pipe != -1)
+ snd_printd(HDMI: pin %d get using pipe %c\n, pin_idx,
i915_pipe_name(pipe));
+
+ return pipe;
+}
+EXPORT_SYMBOL(hdmi_get_using_pipe);
+
+static int init_audio_routing(void)
+{
+ get_using_pipe = symbol_request(i915_using_pipe);
+ if (!get_using_pipe)
+ return -ENODEV;
+
+ disable_unused_pipe = symbol_request(i915_disable_pipe);
+ if (!disable_unused_pipe) {
+ get_using_pipe = NULL;
+ return -ENODEV;
+ }
+
+ restore_eld = symbol_request(i915_restore_pineld);
+ if (!restore_eld) {
+ restore_eld = NULL;
+ get_using_pipe = NULL;
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
void hda_display_power(bool enable)
{
if (!get_power || !put_power)
@@ -57,6 +121,10 @@ int hda_i915_init(void)
snd_printd(HDA driver get symbol successfully from i915 module\n);
+ err = init_audio_routing();
+ if (err 0)
+ snd_printd(HDA driver get audior routing APIs failed!\n);
+
return err;
}
@@ -71,5 +139,20 @@ int hda_i915_exit(void)
put_power = NULL;
}
+ if (get_using_pipe) {
+ symbol_put(get_using_pipe);
+ get_using_pipe = NULL;
+ }
+
+ if (disable_unused_pipe) {
+ symbol_put(disable_unused_pipe);
+ disable_unused_pipe = NULL;
+ }
+
+ if (restore_eld) {
+ symbol_put(restore_eld);
+ restore_eld = NULL;
+ }
+
return 0;
}
diff --git a/sound/pci/hda/hda_i915.h b/sound/pci/hda/hda_i915.h
index 5a63da2..52d6f09 100644
--- a/sound/pci/hda/hda_i915.h
+++ b/sound/pci/hda/hda_i915.h
@@ -32,4 +32,8 @@ static inline int hda_i915_exit(void)
}
#endif
+extern int hdmi_get_using_pipe(int pin_idx);
+extern int hdmi_disable_unused_pipe(int pin_idx, int pipe_idx);
+extern void hdmi_restore_pineld(int pin_idx);
+
#endif
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index d766f40..2a1e977 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -39,6 +39,7 @@
#include hda_codec.h
#include hda_local.h
#include hda_jack.h
+#include hda_i915.h
static bool static_hdmi_pcm;
module_param(static_hdmi_pcm, bool, 0644);
@@ -1131,6 +1132,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
struct hdmi_spec_per_pin *per_pin;
struct hdmi_eld *eld;
struct hdmi_spec_per_cvt *per_cvt = NULL;
+ int pipe_idx;
/* Validate hinfo */
pin_idx = hinfo_to_pin_index(spec, hinfo);
@@ -1139,12 +1141,21 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
per_pin = get_pin(spec, pin_idx);
eld = per_pin-sink_eld;
+ if (codec-vendor_id == 0x80862807) {
+ hsw_verify_cvt_D0(spec, codec);
+
+ pipe_idx = hdmi_get_using_pipe(pin_idx);
+ if (pipe_idx 0)
+ snd_printdd(HDMI: Pin %d has no valid pipe in use\n,
pin_idx);
+ else {
+ hdmi_disable_unused_pipe(pin_idx, pipe_idx);
+ msleep(10);
+ }
+ }
+
if (!eld-monitor_present || !eld-eld_valid)
return -EIO;
- if (codec-vendor_id == 0x80862807)
- hsw_verify_cvt_D0(spec, codec);
-
/* Dynamically assign converter to
Re: [Intel-gfx] [PATCH 27/31] drm/i915: move i9xx dpll enabling into crtc enable function
On Wed, 2013-06-05 at 13:34 +0200, Daniel Vetter wrote:
Now that we have the proper pipe config to track this, we don't need
to write any registers any more.
v2: Drop a few now unnecessary local variables and switch the enable
function to take a struct intel_crtc * to simply arguments.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 102
+--
1 file changed, 37 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b9be047..b6f5e48 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1298,32 +1298,48 @@ static void vlv_enable_pll(struct drm_i915_private
*dev_priv, enum pipe pipe)
udelay(150); /* wait for warmup */
}
-static void i9xx_enable_pll(struct drm_i915_private *dev_priv, enum pipe
pipe)
+static void i9xx_enable_pll(struct intel_crtc *crtc)
{
- int reg;
- u32 val;
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int reg = DPLL(crtc-pipe);
+ u32 dpll = crtc-config.dpll_hw_state.dpll;
- assert_pipe_disabled(dev_priv, pipe);
+ assert_pipe_disabled(dev_priv, crtc-pipe);
/* No really, not for ILK+ */
- BUG_ON(!IS_VALLEYVIEW(dev_priv-dev) dev_priv-info-gen = 5);
+ BUG_ON(!IS_VALLEYVIEW(dev) dev_priv-info-gen = 5);
/* PLL is protected by panel, make sure we can write it */
- if (IS_MOBILE(dev_priv-dev) !IS_I830(dev_priv-dev))
- assert_panel_unlocked(dev_priv, pipe);
+ if (IS_MOBILE(dev) !IS_I830(dev))
+ assert_panel_unlocked(dev_priv, crtc-pipe);
- reg = DPLL(pipe);
- val = I915_READ(reg);
- val |= DPLL_VCO_ENABLE;
+ I915_WRITE(reg, dpll);
+
+ /* Wait for the clocks to stabilize. */
+ POSTING_READ(reg);
+ udelay(150);
+
+ if (INTEL_INFO(dev)-gen = 4) {
+ I915_WRITE(DPLL_MD(crtc-pipe),
+crtc-config.dpll_hw_state.dpll_md);
+ } else {
+ /* The pixel multiplier can only be updated once the
+ * DPLL is enabled and the clocks are stable.
+ *
+ * So write it again.
+ */
+ I915_WRITE(reg, dpll);
It's ok but isn't really needed any more as now we write dpll right
after this with the same value.
+ }
/* We do this three times for luck */
- I915_WRITE(reg, val);
+ I915_WRITE(reg, dpll);
POSTING_READ(reg);
udelay(150); /* wait for warmup */
- I915_WRITE(reg, val);
+ I915_WRITE(reg, dpll);
POSTING_READ(reg);
udelay(150); /* wait for warmup */
- I915_WRITE(reg, val);
+ I915_WRITE(reg, dpll);
POSTING_READ(reg);
udelay(150); /* wait for warmup */
}
@@ -3591,7 +3607,11 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
intel_crtc-active = true;
intel_update_watermarks(dev);
- i9xx_enable_pll(dev_priv, pipe);
+ for_each_encoder_on_crtc(dev, crtc, encoder)
+ if (encoder-pre_pll_enable)
+ encoder-pre_pll_enable(encoder);
+
+ i9xx_enable_pll(intel_crtc);
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder-pre_enable)
@@ -4429,8 +4449,6 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
{
struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
- struct intel_encoder *encoder;
- int pipe = crtc-pipe;
u32 dpll;
bool is_sdvo;
struct dpll *clock = crtc-config.dpll;
@@ -4494,37 +4512,14 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
dpll |= DPLL_VCO_ENABLE;
crtc-config.dpll_hw_state.dpll = dpll;
- I915_WRITE(DPLL(pipe), dpll ~DPLL_VCO_ENABLE);
- POSTING_READ(DPLL(pipe));
- udelay(150);
-
- for_each_encoder_on_crtc(dev, crtc-base, encoder)
- if (encoder-pre_pll_enable)
- encoder-pre_pll_enable(encoder);
-
- if (crtc-config.has_dp_encoder)
- intel_dp_set_m_n(crtc);
-
- I915_WRITE(DPLL(pipe), dpll);
-
- /* Wait for the clocks to stabilize. */
- POSTING_READ(DPLL(pipe));
- udelay(150);
-
if (INTEL_INFO(dev)-gen = 4) {
u32 dpll_md = (crtc-config.pixel_multiplier - 1)
DPLL_MD_UDI_MULTIPLIER_SHIFT;
crtc-config.dpll_hw_state.dpll_md = dpll_md;
-
- I915_WRITE(DPLL_MD(pipe), dpll_md);
- } else {
- /* The pixel multiplier can only be updated once the
- * DPLL is enabled and the clocks are stable.
- *
- * So write it again.
- */
- I915_WRITE(DPLL(pipe), dpll);
}
+
+ if (crtc-config.has_dp_encoder)
+
Re: [Intel-gfx] [PATCH 01/13] drm: Added SDP and VSC structures for handling PSR for eDP
2013/6/12 Rodrigo Vivi rodrigo.v...@gmail.com:
From: Shobhit Kumar shobhit.ku...@intel.com
v2: Modified and corrected the structures to be more in line for
kernel coding guidelines and rebased the code on Paulo's DP patchset
v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE
v4: moving them to include/drm/drm_dp_helper.h and also already
icluding EDP_PSR_RECEIVER_CAP_SIZE to add everything needed
for PSR at once at drm_dp_helper.h
v5: Fix SDP VSC header and identation by (Paulo Zanoni) and
remove i915 from title (Daniel Vetter)
CC: Paulo Zanoni paulo.r.zan...@intel.com
Signed-off-by: Sateesh Kavuri sateesh.kav...@intel.com
Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
include/drm/drm_dp_helper.h | 33 -
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index e8e1417..4062c9e 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -342,13 +342,44 @@ u8 drm_dp_get_adjust_request_voltage(u8
link_status[DP_LINK_STATUS_SIZE],
u8 drm_dp_get_adjust_request_pre_emphasis(u8
link_status[DP_LINK_STATUS_SIZE],
int lane);
-#define DP_RECEIVER_CAP_SIZE 0xf
+#define DP_RECEIVER_CAP_SIZE 0xf
+#define EDP_PSR_RECEIVER_CAP_SIZE 2
+
void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
u8 drm_dp_link_rate_to_bw_code(int link_rate);
int drm_dp_bw_code_to_link_rate(u8 link_bw);
+/* SDP header as per eDP 1.3 spec, section 3.6 */
On my version of the spec, it's section 3.5, same for the comment
below. Maybe we should just include the chapter name (PSR Secondary
Data Packet Support) or just don't add anything. These numbers might
change in the future.
Everything else looks correct, so with or without the comment fixed:
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
+struct edp_sdp_header {
+ u8 HB0; /* Secondary Data Packet ID */
+ u8 HB1; /* Secondary Data Packet Type */
+ u8 HB2; /* 7:5 reserved, 4:0 revision number */
+ u8 HB3; /* 7:5 reserved, 4:0 number of valid data bytes */
+} __packed;
+
+#define EDP_SDP_HEADER_REVISION_MASK 0x1F
+#define EDP_SDP_HEADER_VALID_PAYLOAD_BYTES 0x1F
+
+/* SDP VSC header as per eDP 1.3 spec, section 3.6 */
+struct edp_vsc_psr {
+ struct edp_sdp_header sdp_header;
+ u8 DB0; /* Stereo Interface */
+ u8 DB1; /* 0 - PSR State; 1 - Update RFB; 2 - CRC Valid */
+ u8 DB2; /* CRC value bits 7:0 of the R or Cr component */
+ u8 DB3; /* CRC value bits 15:8 of the R or Cr component */
+ u8 DB4; /* CRC value bits 7:0 of the G or Y component */
+ u8 DB5; /* CRC value bits 15:8 of the G or Y component */
+ u8 DB6; /* CRC value bits 7:0 of the B or Cb component */
+ u8 DB7; /* CRC value bits 15:8 of the B or Cb component */
+ u8 DB8_31[24]; /* Reserved */
+} __packed;
+
+#define EDP_VSC_PSR_STATE_ACTIVE (10)
+#define EDP_VSC_PSR_UPDATE_RFB (11)
+#define EDP_VSC_PSR_CRC_VALUES_VALID (12)
+
static inline int
drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
{
--
1.7.11.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3]
On Thu, Jun 13, 2013 at 10:22:05AM +0200, Daniel Vetter wrote: On Thu, Jun 06, 2013 at 04:59:26PM +0300, Jani Nikula wrote: With Greg's address fixed. Please drop the old one from any replies. Sorry for the noise. Oops, replied with the old one still there. Greg, Andrew: Imo it's best to merge all three patches through the same tree, so: Acked-by: Daniel Vetter daniel.vet...@ffwll.ch on the i915 parts of it. If you want I can also slurp them in through the intel tree, including the new dmi match code. Please feel free to take them through your tree, I don't need to take them. thanks, greg k-h ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/13] drm/i915: Read the EDP DPCD and PSR Capability
2013/6/12 Rodrigo Vivi rodrigo.v...@gmail.com:
From: Shobhit Kumar shobhit.ku...@intel.com
v2: reuse of just created is_edp_psr and put it at right place.
v3: move is_edp_psr above intel_edp_disable
Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
Reviewed-by: Jani Nikula jani.nik...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 13 +
drivers/gpu/drm/i915/intel_drv.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 759a1c5..5332186 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1342,6 +1342,12 @@ static void intel_dp_get_config(struct intel_encoder
*encoder,
pipe_config-adjusted_mode.flags |= flags;
}
+static bool is_edp_psr(struct intel_dp *intel_dp)
+{
+ return (is_edp(intel_dp)
+ (intel_dp-psr_dpcd[0] DP_PSR_IS_SUPPORTED));
+}
My only concern about this is: what will happen when they define PSR
version 02? Will that DP_PSR_IS_SUPPORTED bit still be 1? Anyway, if
things change in the future we should fix the DP_PSR_IS_SUPPORTED
definition and all its users, so for now I think the code is fine.
The patch looks good and I think that we could try to merge patches 1
and 2 now because they allow us to at least properly detect which
panels support PSR and which panels don't.
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
+
static void intel_disable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
@@ -2255,6 +2261,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
if (intel_dp-dpcd[DP_DPCD_REV] == 0)
return false; /* DPCD not present */
+ /* Check if the panel supports PSR */
+ memset(intel_dp-psr_dpcd, 0, sizeof(intel_dp-psr_dpcd));
+ intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
+ intel_dp-psr_dpcd,
+ sizeof(intel_dp-psr_dpcd));
+ if (is_edp_psr(intel_dp))
+ DRM_DEBUG_KMS(Detected EDP PSR Panel.\n);
if (!(intel_dp-dpcd[DP_DOWNSTREAMPORT_PRESENT]
DP_DWN_STRM_PORT_PRESENT))
return true; /* native DP sink */
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 0445d8c..18d9dea 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -470,6 +470,7 @@ struct intel_dp {
uint8_t link_bw;
uint8_t lane_count;
uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
+ uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
struct i2c_adapter adapter;
struct i2c_algo_dp_aux_data algo;
--
1.7.11.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/13] drm/i915: split aux_clock_divider logic in a separated function for reuse.
2013/6/12 Rodrigo Vivi rodrigo.v...@gmail.com:
Prep patch for reuse aux_clock_divider with EDP_PSR_AUX_CTL setup.
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
I've seen this patch on this mailing list a few times already, and
it's just a prep-work for PSR, so I guess we could merge it now so we
spare Rodrigo the pain of rebasing it again and again. And I like tiny
functions like these, even if they have just 1 caller.
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 58
+++--
1 file changed, 33 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5332186..02b2865 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -271,29 +271,12 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool
has_aux_irq)
return status;
}
-static int
-intel_dp_aux_ch(struct intel_dp *intel_dp,
- uint8_t *send, int send_bytes,
- uint8_t *recv, int recv_size)
+static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port-base.base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
- uint32_t ch_ctl = intel_dp-aux_ch_ctl_reg;
- uint32_t ch_data = ch_ctl + 4;
- int i, ret, recv_bytes;
- uint32_t status;
- uint32_t aux_clock_divider;
- int try, precharge;
- bool has_aux_irq = INTEL_INFO(dev)-gen = 5 !IS_VALLEYVIEW(dev);
- /* dp aux is extremely sensitive to irq latency, hence request the
-* lowest possible wakeup latency and so prevent the cpu from going
into
-* deep sleep states.
-*/
- pm_qos_update_request(dev_priv-pm_qos, 0);
-
- intel_dp_check_edp(intel_dp);
/* The clock divider is based off the hrawclk,
* and would like to run at 2MHz. So, take the
* hrawclk value and divide by 2 and use that
@@ -302,23 +285,48 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
* clock divider.
*/
if (IS_VALLEYVIEW(dev)) {
- aux_clock_divider = 100;
+ return 100;
} else if (intel_dig_port-port == PORT_A) {
if (HAS_DDI(dev))
- aux_clock_divider = DIV_ROUND_CLOSEST(
+ return DIV_ROUND_CLOSEST(
intel_ddi_get_cdclk_freq(dev_priv), 2000);
else if (IS_GEN6(dev) || IS_GEN7(dev))
- aux_clock_divider = 200; /* SNB IVB eDP input clock
at 400Mhz */
+ return 200; /* SNB IVB eDP input clock at 400Mhz */
else
- aux_clock_divider = 225; /* eDP input clock at 450Mhz
*/
+ return 225; /* eDP input clock at 450Mhz */
} else if (dev_priv-pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
/* Workaround for non-ULT HSW */
- aux_clock_divider = 74;
+ return 74;
} else if (HAS_PCH_SPLIT(dev)) {
- aux_clock_divider = DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
+ return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
} else {
- aux_clock_divider = intel_hrawclk(dev) / 2;
+ return intel_hrawclk(dev) / 2;
}
+}
+
+static int
+intel_dp_aux_ch(struct intel_dp *intel_dp,
+ uint8_t *send, int send_bytes,
+ uint8_t *recv, int recv_size)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ uint32_t ch_ctl = intel_dp-aux_ch_ctl_reg;
+ uint32_t ch_data = ch_ctl + 4;
+ int i, ret, recv_bytes;
+ uint32_t status;
+ uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp);
+ int try, precharge;
+ bool has_aux_irq = INTEL_INFO(dev)-gen = 5 !IS_VALLEYVIEW(dev);
+
+ /* dp aux is extremely sensitive to irq latency, hence request the
+* lowest possible wakeup latency and so prevent the cpu from going
into
+* deep sleep states.
+*/
+ pm_qos_update_request(dev_priv-pm_qos, 0);
+
+ intel_dp_check_edp(intel_dp);
if (IS_GEN6(dev))
precharge = 3;
--
1.7.11.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On Fri, 2013-06-14 at 14:47 +0800, Aaron Lu wrote: What about a priority based solution? We can introduce a new field named priority to backlight_device and instead of calling another module's function like the unregister one here(which cause unnecessary module dependency), we only need to boost priority for its own interface. This field will be exported to sysfs, so user can change it during runtime too. And we can also introduce a new kernel command line as backlight.force_interface=raw/firmware/platform, to overcome the limited functionality provided by acpi_backlight=video/vendor, which does not involve GPU's interface. How would that work with existing userspace? And we can place the quirk code in backlight layer instead of individual backlight functionality provider module. Suppose we have a backlight manager there, for all win8 systems, we can boost the raw type's priority on its registration, so no need to add code in intel/amd/etc./'s GPU driver code. But we'd need to add code to every piece of userspace that currently uses the backlight, right? With priority based solution, all backlight control interfaces stay, the priority field is an indication given by kernel to user space. We shouldn't export interfaces if we don't expect them to work. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: repin bound framebuffers on resume
On Wed, Jun 12, 2013 at 3:06 PM, Jesse Barnes jbar...@virtuousgeek.org wrote: On Wed, 12 Jun 2013 00:48:25 +0100 Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 04:01:21PM -0700, Stéphane Marchesin wrote: On Tue, Jun 11, 2013 at 3:57 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 03:49:27PM -0700, Stéphane Marchesin wrote: During suspend all fences are reset, including their pin_count which is reset to 0. However a framebuffer can be bound across suspend/resume, which means that when the buffer is unbound after resume, the pin count for the buffer will be negative. Since the fence pin count is now negative when available and zero when in use, the buffer's fence will get recycled when the fence is in use which is the opposite of what we want. The visible effect is that since the fence is recycled the tiling mode goes away while the buffer is being displayed and we get lines/screens of garbage. To fix this, we repin the fences for all bound fbs on resume, which ensures the pin count is right. Yikes. So why do we not just keep the fences alive during suspend (not touching their pin_count), and then just iterate over the list of fences rewriting the register as required upon resume? That would seem less error prone than trying to reconstruct the lost pin_count. I suspect they'd need to be saved/restored at the hw level as well, which AFAICS isn't happening today... Ugh, I introduced this bug 30 months ago - saved by the VT switch on resume. But we can restore the fences from dev_priv-fence_regs... Actually we have a very similar problem after a GPU reset where we should restore fences for pinned objects (i.e. the scanout). The patch to fix both looks fairly straightforward. To be clear, this only affects gen3 right? For gen4+ we don't need the fences for scanout since we have a bit in the plane control... Yup I've only ever seen the issue on gen3. Anyway, what should we do about this? Should I make another patch where I save/restore the fence regs instead? Stéphane Or are we failing to fault on a previously mapped scanout too? If so, we'd need to cover more than just scanout here. -- 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 1/2] drm/i915: tune the RC6 threshold for stability
On Wed, Jun 12, 2013 at 2:41 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 03:49:26PM -0700, Stéphane Marchesin wrote: It's basically the same deal as the RC6+ issues on ivy bridge except this time with RC6 on sandy bridge. Like last time the core of the issue is that the timings don't work 100% with our voltage regulator. So from time to time, the kernel will print a warning message about the GPU not getting out of RC6. In particular, I found this fairly easy to reproduce during suspend/resume. Changing the threshold to 15 instead of 5 seems to fix the issue. I also measured the idle power usage before/after this patch and didn't see a difference on a sandy bridge laptop. Signed-off-by: Stéphane Marchesin marc...@chromium.org One magic number for another with no idea what is blowing up - I fear we are just changing the frequency of the hang. I've pinged a number of snb rc6 bug reports to see if we get a bite. Yup, if only Intel documented those registers :) Stéphane FWIW, Acked-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Add display audio routing APIs for ALSA
On Fri, Jun 14, 2013 at 5:20 PM, Wang Xingchao xingchao.w...@linux.intel.com wrote: ALSA audio driver need know current audio routing infomation. i.e. Route map between codec pins(DDI ports) and Transcoders(Pipe). Also the new API let audio driver disable unused audio pin's output. This fixed the bug when three pins *ALL* have monitors connected, playing audio on one pin would cause audio output to all minitors. Signed-off-by: Wang Xingchao xingchao.w...@linux.intel.com So I've started to have a real look at audio on haswell and audio on i915 in general, and I'm seriously confused. Random observations, but I fear this isn't the real story by far yet: - On haswell we now have a hooping 3 places that set up these audio routing register: haswell_write_eld (called from the connector enable callbacks), intel_enable/disable_ddi and now the new hooks you're adding here. That's 2 places too many. If we really need all three places those need to be refactored so that the bit-frobbing logic is all in one place. But I seriously doubt that we need them all. - I've quickly read through the haswell audio modeset sequence. On a glance I could see no reason why we need to have 3 different places to set up the gfx side of the audio support, since it's much simpler apparently: Enable sequence: 1. gfx side sets up registers 2. gfx side sets the audio enable bit, which generates the interrup 3. audio side completes the setup on its side. Disable sequence is just the inverse. So I don't think we need 3 different places for this. - Both on ibx/cpt and also on haswell here we seem to rely on BIOS preset values way too often. Or at least I didn't figure out where these registes get initialized (pretty sure nowhere in i915.ko). We have countless bugs where the BIOS tried to be fancy and set up something we don't actually support. So please, if there's no really good reason why we need to do things differently, have explicit register setups. If there's something we need to preserve from the BIOS, it needs to be done explicitly and must have a comment. - No global state or stuffing random things into dev_priv/crtc any more. Our modeset infrastructure has a transactional state machine now: First we compute state parameters in the various compute_config functions and store all that into a struct intel_crtc_config pipe_config. Then the modeset functions apply that state. Finally at the end the hw state is cross-checked with the sw state. We need this to properly support atomic modeset and fastboot. Yes, this means that recent additions for haswell audio support like crtc-eld_vld need to go away and be moved into pipe_config. - Changing the OUTPUT_ENABLE bits will result in interrupts on the audio side. But these functions here are only called from the audio side, so we have a really complicated feedback loop. Given that your patches need much better explanation of what's going on (preferably with time/state diagrams). Also I think I need a more detailed explanation of what exactly is broken currently on Haswell audio and how these patches fix this. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: repin bound framebuffers on resume
On Fri, Jun 14, 2013 at 9:12 PM, Stéphane Marchesin marc...@chromium.org wrote: On Wed, Jun 12, 2013 at 3:06 PM, Jesse Barnes jbar...@virtuousgeek.org wrote: On Wed, 12 Jun 2013 00:48:25 +0100 Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 04:01:21PM -0700, Stéphane Marchesin wrote: On Tue, Jun 11, 2013 at 3:57 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 03:49:27PM -0700, Stéphane Marchesin wrote: During suspend all fences are reset, including their pin_count which is reset to 0. However a framebuffer can be bound across suspend/resume, which means that when the buffer is unbound after resume, the pin count for the buffer will be negative. Since the fence pin count is now negative when available and zero when in use, the buffer's fence will get recycled when the fence is in use which is the opposite of what we want. The visible effect is that since the fence is recycled the tiling mode goes away while the buffer is being displayed and we get lines/screens of garbage. To fix this, we repin the fences for all bound fbs on resume, which ensures the pin count is right. Yikes. So why do we not just keep the fences alive during suspend (not touching their pin_count), and then just iterate over the list of fences rewriting the register as required upon resume? That would seem less error prone than trying to reconstruct the lost pin_count. I suspect they'd need to be saved/restored at the hw level as well, which AFAICS isn't happening today... Ugh, I introduced this bug 30 months ago - saved by the VT switch on resume. But we can restore the fences from dev_priv-fence_regs... Actually we have a very similar problem after a GPU reset where we should restore fences for pinned objects (i.e. the scanout). The patch to fix both looks fairly straightforward. To be clear, this only affects gen3 right? For gen4+ we don't need the fences for scanout since we have a bit in the plane control... Yup I've only ever seen the issue on gen3. Anyway, what should we do about this? Should I make another patch where I save/restore the fence regs instead? drm-intel-fixes has already the improved patch from Chris, drm-intel-next-queued has a patch to add a WARN so we'll catch this much quicker next time around. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: tune the RC6 threshold for stability
On Fri, Jun 14, 2013 at 9:13 PM, Stéphane Marchesin marc...@chromium.org wrote: On Wed, Jun 12, 2013 at 2:41 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 03:49:26PM -0700, Stéphane Marchesin wrote: It's basically the same deal as the RC6+ issues on ivy bridge except this time with RC6 on sandy bridge. Like last time the core of the issue is that the timings don't work 100% with our voltage regulator. So from time to time, the kernel will print a warning message about the GPU not getting out of RC6. In particular, I found this fairly easy to reproduce during suspend/resume. Changing the threshold to 15 instead of 5 seems to fix the issue. I also measured the idle power usage before/after this patch and didn't see a difference on a sandy bridge laptop. Signed-off-by: Stéphane Marchesin marc...@chromium.org One magic number for another with no idea what is blowing up - I fear we are just changing the frequency of the hang. I've pinged a number of snb rc6 bug reports to see if we get a bite. Yup, if only Intel documented those registers :) We've spammed rc6 bugs in bugzilla, one reporter says that this patch breaks rc6 from sometimes it doesn't work after resume to always broken: https://bugs.freedesktop.org/show_bug.cgi?id=54089#c63 So I guess I can't merge this :( -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3]
On Fri, Jun 14, 2013 at 6:23 PM, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Thu, Jun 13, 2013 at 10:22:05AM +0200, Daniel Vetter wrote: On Thu, Jun 06, 2013 at 04:59:26PM +0300, Jani Nikula wrote: With Greg's address fixed. Please drop the old one from any replies. Sorry for the noise. Oops, replied with the old one still there. Greg, Andrew: Imo it's best to merge all three patches through the same tree, so: Acked-by: Daniel Vetter daniel.vet...@ffwll.ch on the i915 parts of it. If you want I can also slurp them in through the intel tree, including the new dmi match code. Please feel free to take them through your tree, I don't need to take them. Andrew already merged them, so I think we're good. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/15/2013 01:29 AM, Matthew Garrett wrote: On Fri, 2013-06-14 at 14:47 +0800, Aaron Lu wrote: What about a priority based solution? We can introduce a new field named priority to backlight_device and instead of calling another module's function like the unregister one here(which cause unnecessary module dependency), we only need to boost priority for its own interface. This field will be exported to sysfs, so user can change it during runtime too. And we can also introduce a new kernel command line as backlight.force_interface=raw/firmware/platform, to overcome the limited functionality provided by acpi_backlight=video/vendor, which does not involve GPU's interface. How would that work with existing userspace? User space tool will need to be updated to use this as stated in the gist page, I've patches for gsd-backlight-helper and xorg-x11-drv-intel, for others we can add I think if the priority based solution is deemed useful. And we can place the quirk code in backlight layer instead of individual backlight functionality provider module. Suppose we have a backlight manager there, for all win8 systems, we can boost the raw type's priority on its registration, so no need to add code in intel/amd/etc./'s GPU driver code. But we'd need to add code to every piece of userspace that currently uses the backlight, right? Yes that's the case. With priority based solution, all backlight control interfaces stay, the priority field is an indication given by kernel to user space. We shouldn't export interfaces if we don't expect them to work. It's not easy to decide if they work or not sometimes, e.g. I came across a system that claims win8 in ACPI table and has an Intel GPU, while its ACPI video interface also works. With this patch, the working ACPI video interface is removed, while with the priority based solution, the GPU's interface priority gets higher, but the ACPI video interface still stays. Thanks, Aaron ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On Sat, 2013-06-15 at 09:26 +0800, Aaron Lu wrote: On 06/15/2013 01:29 AM, Matthew Garrett wrote: How would that work with existing userspace? User space tool will need to be updated to use this as stated in the gist page, I've patches for gsd-backlight-helper and xorg-x11-drv-intel, for others we can add I think if the priority based solution is deemed useful. Right, that's not a great solution. We shouldn't export interfaces if we don't expect them to work. It's not easy to decide if they work or not sometimes, e.g. I came across a system that claims win8 in ACPI table and has an Intel GPU, while its ACPI video interface also works. With this patch, the working ACPI video interface is removed, while with the priority based solution, the GPU's interface priority gets higher, but the ACPI video interface still stays. Well, Windows 8 will only use the ACPI backlight interface if the GPU driver decides to, right? So the logic for deciding whether to remove the ACPI backlight control or not should be left up to the GPU. There's no harm in refusing to expose a working method if there's another working method, but there is harm in exposing a broken one and expecting userspace to know the difference. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/15/2013 09:38 AM, Matthew Garrett wrote: On Sat, 2013-06-15 at 09:26 +0800, Aaron Lu wrote: It's not easy to decide if they work or not sometimes, e.g. I came across a system that claims win8 in ACPI table and has an Intel GPU, while its ACPI video interface also works. With this patch, the working ACPI video interface is removed, while with the priority based solution, the GPU's interface priority gets higher, but the ACPI video interface still stays. Well, Windows 8 will only use the ACPI backlight interface if the GPU driver decides to, right? So the logic for deciding whether to remove the ACPI backlight control or not should be left up to the GPU. There's I don't know this. From the document I googled, Microsoft suggests under win8, backlight should be controlled by the graphics driver for smooth brightness level change, instead of ACPI or other methods. So it is possible that OEM will not test the ACPI interface well and thus the interface is likely broken. I don't see why GPU driver has any better knowledge on which systems the firmware interface is broken or not. no harm in refusing to expose a working method if there's another working method, but there is harm in exposing a broken one and expecting userspace to know the difference. BTW, the proposed solution is not meant to solve win8 problems alone, it should make solving other problems easy and make individual backlight control interface provider module independent with each other such as the platform drivers will not need to check if ACPI video driver will control backlight or not and can always create backlight interface(its default priority is lower that ACPI video driver's so will not be taken by user space by default, showing the same behavior of the current code). The current acpi_backlight=video/vendor kernel command line is pretty misleading, for laptops that do not have vendor backlight interface, adding acpi_backlight=vendor actually makes the system using the GPU's interface. Some laptops are using this switch to work around problems in ACPI video driver and users think they are using vendor interface. That's why I think we need a new command line as the backlight.force_interface=raw/firmware/platform. Instead of letting individual driver to make decisions on which backlight interface this system should use(either in platform driver as we currently did, see acer-wmi and asus-wmi, or GPU driver as this case), I think we need a better and clear way to handle such things. For example, suppose an acer laptop: vendor does not support backlight, ACPI video's backlight interface is broken and GPU's works, to make it work, user will need to select acer-wmi module while this module does not have anything to do with the functionality, but simply because it serves as the backlight manager for acer laptops. The above information and idea is formed while solving bugs reported in kernel bugzilla video component, the proposed solutin may not be good enough, but I hope we can find a better way to handle backlight problems. Thanks, Aaron ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On Sat, Jun 15, 2013 at 12:14:42PM +0800, Aaron Lu wrote: On 06/15/2013 09:38 AM, Matthew Garrett wrote: Well, Windows 8 will only use the ACPI backlight interface if the GPU driver decides to, right? So the logic for deciding whether to remove the ACPI backlight control or not should be left up to the GPU. There's I don't know this. From the document I googled, Microsoft suggests under win8, backlight should be controlled by the graphics driver for smooth brightness level change, instead of ACPI or other methods. So it is possible that OEM will not test the ACPI interface well and thus the interface is likely broken. I don't see why GPU driver has any better knowledge on which systems the firmware interface is broken or not. The vendor will presumably have tested that backlight control works - if the GPU driver uses the ACPI interface and backlight control is broken, then the vendor would fix it. no harm in refusing to expose a working method if there's another working method, but there is harm in exposing a broken one and expecting userspace to know the difference. BTW, the proposed solution is not meant to solve win8 problems alone, it should make solving other problems easy and make individual backlight control interface provider module independent with each other such as the platform drivers will not need to check if ACPI video driver will control backlight or not and can always create backlight interface(its default priority is lower that ACPI video driver's so will not be taken by user space by default, showing the same behavior of the current code). Sure, but it still requires the replacement of existing userspace. We need to fix the problem with existing userspace, too. The current acpi_backlight=video/vendor kernel command line is pretty misleading, for laptops that do not have vendor backlight interface, adding acpi_backlight=vendor actually makes the system using the GPU's interface. Some laptops are using this switch to work around problems in ACPI video driver and users think they are using vendor interface. That's why I think we need a new command line as the backlight.force_interface=raw/firmware/platform. No, I think we need to fix the bugs that currently require users to pass options. Instead of letting individual driver to make decisions on which backlight interface this system should use(either in platform driver as we currently did, see acer-wmi and asus-wmi, or GPU driver as this case), I think we need a better and clear way to handle such things. For example, suppose an acer laptop: vendor does not support backlight, ACPI video's backlight interface is broken and GPU's works, to make it work, user will need to select acer-wmi module while this module does not have anything to do with the functionality, but simply because it serves as the backlight manager for acer laptops. How do these machines work under Windows? Until we know the answer to that, we don't know what the correct way to handle the problem is under Linux. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
