Re: [Intel-gfx] [PATCH] drm/i915: fix build warning on format specifier mismatch
On Fri, Jun 07, 2013 at 04:03:50PM +0300, Jani Nikula wrote:
> drivers/gpu/drm/i915/i915_gem.c: In function ‘i915_gem_object_bind_to_gtt’:
> drivers/gpu/drm/i915/i915_gem.c:3002:3: warning: format ‘%ld’ expects
> argument of type ‘long int’, but argument 5 has type ‘size_t’ [-Wformat]
>
> v2: Use %zu instead of %d. Two char patch, and 100% wrong. (Ville)
>
> Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_gem.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 58048d4..c2b60a4 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2999,7 +2999,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object
> *obj,
>* before evicting everything in a vain attempt to find space.
>*/
> if (obj->base.size > gtt_max) {
> - DRM_ERROR("Attempting to bind an object larger than the
> aperture: object=%zd > %s aperture=%ld\n",
> + DRM_ERROR("Attempting to bind an object larger than the
> aperture: object=%zd > %s aperture=%zu\n",
> obj->base.size,
> map_and_fenceable ? "mappable" : "total",
> gtt_max);
> --
> 1.7.9.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Backporting "drm/i915: force full modeset if the connector is in DPMS OFF mode"
On Sun, Jun 09, 2013 at 11:18:16AM +0200, Daniel Vetter wrote: > Hi stable maintainers, > > Please backport > > commit e3de42b68478a8c95dd27520e9adead2af9477a5 > Author: Imre Deak > Date: Fri May 3 19:44:07 2013 +0200 > > drm/i915: force full modeset if the connector is in DPMS OFF mode > > to all supported stable kernels. > > Additional bugzilla on top of all the others: > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65559 Is that a new record for the most number of Bugzilla: tags in a single patch? :) Now applied to the 3.9-stable queue, it wouldn't apply to 3.4 or 3.0. 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] drm/i915: propagate errors from intel_dp_init_connector
On Mon, Jun 10, 2013 at 9:59 PM, Chris Wilson wrote:
> On Mon, Jun 10, 2013 at 02:19:45PM -0300, Paulo Zanoni wrote:
>> - intel_dp_init_connector(intel_dig_port, dp_connector);
>> + /* May fail if it's an eDP connector that's disconnected. */
>> + if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
>> + intel_ddi_destroy(encoder);
>> + intel_dp_destroy(&dp_connector->base);
>> + return;
>> + }
>
> You should not need to export intel_dp_destroy() ever. Better would be
> for init to undo anything it allocates upon failure and here we just
> kfree(dp_connector). In particular, intel_dp_destroy() is wrong here as
> you have no idea if init returned at an appropriate juntion to make
> calling that function safe.
Good point, I guess we could extract an __dp_destroy function which
avoids the kfree, use that in appropriate places and do the kfree in
the two functions that allocated the struct. Patch dropped again.
-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: propagate errors from intel_dp_init_connector
On Mon, Jun 10, 2013 at 02:19:45PM -0300, Paulo Zanoni wrote:
> - intel_dp_init_connector(intel_dig_port, dp_connector);
> + /* May fail if it's an eDP connector that's disconnected. */
> + if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
> + intel_ddi_destroy(encoder);
> + intel_dp_destroy(&dp_connector->base);
> + return;
> + }
You should not need to export intel_dp_destroy() ever. Better would be
for init to undo anything it allocates upon failure and here we just
kfree(dp_connector). In particular, intel_dp_destroy() is wrong here as
you have no idea if init returned at an appropriate juntion to make
calling that function safe.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Fwd: [Bug 58971] High temperature after resuming from suspend to RAM (system idle).
Hi, It looks like an i915 commit causes problems with frequency scaling to happen to people. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.--- Begin Message --- https://bugzilla.kernel.org/show_bug.cgi?id=58971 --- Comment #42 from Alexander Kaltsas 2013-06-10 14:53:06 --- Ok, I managed to complete a git bisect. The result was: First bad commit: [15239099d7a7a9ecdc1ccb5b187ae4cda5488ff9] drm/i915: enable irqs earlier when resuming. http://o.cs.uvic.ca:20810/perl/cid.pl?cid=15239099d7a7a9ecdc1ccb5b187ae4cda5488ff9 I removed the above patch from kernel 3.9.4, 3.9.5 and it seems to work ok after suspend and resume. With Intel PSTATE enabled. The temperature is back to normal and the cpu frequency doesn't "lock" to the highest value. Just a side note. Don't know if related at all: I have noticed that when using Intel PSTATE. If the system starts with frequency scaling to powersave the frequency is around to 800~900 MHz. If I change it to performance it will go up. After changing it back to powersave the frequency will stay to ~2000 MHz. If I suspend and resume it will be around to 800~900 MHz again. This doesn't seem to effect the temperature of the system' -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. --- End Message --- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: propagate errors from intel_dp_init_connector
On Mon, Jun 10, 2013 at 02:19:45PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni
>
> In case we detect a "ghost eDP", intel_dp_init_connector was freeing
> both the connector and encoder and then returning. On Haswell,
> intel_ddi_init was trying to use the freed encoder on the HDMI
> initialization path since the following commit:
>
> commit 21a8e6a4853b2ed39fa4c5188a710f2cf1b92026
> Author: Daniel Vetter
> Date: Wed Apr 10 23:28:35 2013 +0200
> drm/i915: don't setup hdmi for port D edp in ddi_init
>
> So on this patch we change intel_dp_init_connector from void to bool,
> check its return value on its callers and leave the resource-freeing
> task to its callers (since the callers are the ones who allocate the
> resources). With this change it will be easier to see that
> intel_dp_init_connector can fail, so I hope we won't have similar
> regressions in the future.
>
> Also include a small change to remove extra "{" and "}" on nearby
> code.
>
> Signed-off-by: Paulo Zanoni
Oops, thanks for tracking this down and fixing it. Patch is queued for
-next.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 10 +++---
> drivers/gpu/drm/i915/intel_dp.c | 15 +--
> drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> 3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 224ce25..4e0cf37 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1356,14 +1356,18 @@ void intel_ddi_init(struct drm_device *dev, enum port
> port)
> intel_encoder->cloneable = false;
> intel_encoder->hot_plug = intel_ddi_hot_plug;
>
> - intel_dp_init_connector(intel_dig_port, dp_connector);
> + /* May fail if it's an eDP connector that's disconnected. */
> + if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
> + intel_ddi_destroy(encoder);
> + intel_dp_destroy(&dp_connector->base);
> + return;
> + }
>
> if (intel_encoder->type != INTEL_OUTPUT_EDP) {
> hdmi_connector = kzalloc(sizeof(struct intel_connector),
>GFP_KERNEL);
> - if (!hdmi_connector) {
> + if (!hdmi_connector)
> return;
> - }
>
> intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
> intel_hdmi_init_connector(intel_dig_port, hdmi_connector);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0de82bb..ec23b3d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2687,7 +2687,7 @@ done:
> return 0;
> }
>
> -static void
> +void
> intel_dp_destroy(struct drm_connector *connector)
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> @@ -2961,7 +2961,7 @@ intel_dp_init_panel_power_sequencer_registers(struct
> drm_device *dev,
> I915_READ(pp_div_reg));
> }
>
> -void
> +bool
> intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> struct intel_connector *intel_connector)
> {
> @@ -3095,9 +3095,7 @@ intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> } else {
> /* if this fails, presume the device is a ghost */
> DRM_INFO("failed to retrieve link info, disabling
> eDP\n");
> - intel_dp_encoder_destroy(&intel_encoder->base);
> - intel_dp_destroy(connector);
> - return;
> + return false;
> }
>
> /* We now know it's not a ghost, init power sequence regs. */
> @@ -3152,6 +3150,8 @@ intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> u32 temp = I915_READ(PEG_BAND_GAP_DATA);
> I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
> }
> +
> + return true;
> }
>
> void
> @@ -3197,5 +3197,8 @@ intel_dp_init(struct drm_device *dev, int output_reg,
> enum port port)
> intel_encoder->cloneable = false;
> intel_encoder->hot_plug = intel_dp_hot_plug;
>
> - intel_dp_init_connector(intel_dig_port, intel_connector);
> + if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
> + intel_dp_encoder_destroy(encoder);
> + intel_dp_destroy(&intel_connector->base);
> + }
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index b9e250c..8a7ad99 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -569,9 +569,10 @@ extern bool intel_lvds_init(struct drm_device *dev);
> extern bool intel_is_dual_link_lvds(struct drm_device *dev);
> extern void intel_dp_init(struct drm_device *dev, int output_reg,
> enum port port);
> -extern void intel_dp_init_connector(struct
Re: [Intel-gfx] [PATCH] drm/i915: Initialize active_outputs to never read unitialized values
On Mon, Jun 10, 2013 at 01:49:25PM +0100, Damien Lespiau wrote: > In case of intel_sdvo_get_active_outputs() failing, we end up reading a > value from the stack. > > Signed-off-by: Damien Lespiau 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: Fix old reference to i830_sdvo_get_capabilities()
On Mon, Jun 10, 2013 at 01:28:42PM +0100, Damien Lespiau wrote: > It's now intel_sdvo_get_capabilities(). > > Signed-off-by: Damien Lespiau 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 00/31] shared pch display pll rework
On Mon, Jun 10, 2013 at 06:57:23PM +0300, Ville Syrjälä wrote: > On Fri, Jun 07, 2013 at 08:46:45PM +0300, Ville Syrjälä wrote: > > I've just gone over patches 01-13. > > > > For patches 01,02,04,05,07,09-13 > > Reviewed-by: Ville Syrjälä > > > > For patches 03,06,08 I replied with some concerns/ideas. > > You can slap my r-b onto those three as well. Thanks for the review, I've merged patches 1-13 to dinq now. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] fb: Make fb_get_options() 'name' parameter const
On 18:43 Fri 07 Jun , ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The string isn't modified so make it const.
>
> Cc: Jean-Christophe Plagniol-Villard
> Cc: Tomi Valkeinen
> Cc: linux-fb...@vger.kernel.org
> Signed-off-by: Ville Syrjälä
Applied
Best Regards,
J.
> ---
> drivers/video/fbmem.c | 2 +-
> include/linux/fb.h| 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 098bfc6..d8d5779 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1881,7 +1881,7 @@ static int ofonly __read_mostly;
> *
> * NOTE: Needed to maintain backwards compatibility
> */
> -int fb_get_options(char *name, char **option)
> +int fb_get_options(const char *name, char **option)
> {
> char *opt, *options = NULL;
> int retval = 0;
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index d49c60f..ffac70a 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -624,7 +624,7 @@ extern void fb_pad_aligned_buffer(u8 *dst, u32 d_pitch,
> u8 *src, u32 s_pitch, u3
> extern void fb_set_suspend(struct fb_info *info, int state);
> extern int fb_get_color_depth(struct fb_var_screeninfo *var,
> struct fb_fix_screeninfo *fix);
> -extern int fb_get_options(char *name, char **option);
> +extern int fb_get_options(const char *name, char **option);
> extern int fb_new_modelist(struct fb_info *info);
>
> extern struct fb_info *registered_fb[FB_MAX];
> --
> 1.8.1.5
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Mon, 2013-06-10 at 13:59 +0200, Rafael J. Wysocki wrote: > I happen to know that alternative solution to this problem is being worked on, > so I'm going to wait until it is submitted and then we'll decide what to > merge. Sure. > I'm slightly concerned about unregistering ACPI backlight control for all > systems declaring win8 support, even though it may actually work for them. Right, that's why I think it's correct to leave it up to the graphics drivers. It seems like they're the ones that make the policy determination under Windows now. The policy as implemented here may not be correct, but doing better would probably involve Intel letting us know how their Windows driver behaves. -- 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
於 日，2013-06-09 於 19:01 -0400，Matthew Garrett 提到： > 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 > --- > 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(); > } > > if (IS_GEN5(dev)) This patch set works to me on Acer Aspire V3 notebook for unregister the backlight interface of acpi video driver when i915 loaded. Acer Aspire V3 has the Windows8 support in DSDT. Tested-by: Lee, Chun-Yi Thanks a lot! Joey Lee ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
Windows 8 introduced new policy for backlight control by pushing it out to graphics drivers. This appears to have coincided with a range of vendors adding Windows 8 checks to their backlight control code which trigger either awkward behaviour (Lenovo) or complete brokenness (some Dells). The simplest thing to do would be to just disable ACPI backlight control entirely if the firmware indicates Windows 8 support, but it's entirely possible that individual graphics drivers might still make use of the ACPI functionality in preference to native control. The first two patches in this series are picked from other patchesets aimed at solving similar problems. The last simply unregisters ACPI backlight control on Windows 8 systems when using an Intel GPU. Similar code could be added to other drivers, but I'm reluctant to do so without further investigation as to the behaviour of the vendor drivers under Windows. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] acpi: video: add function to support unregister backlight interface
From: "Lee, Chun-Yi"
There have some situation we unregister whole acpi/video driver by downstream
driver just want to remove backlight control interface of acpi/video. It caues
we lost other functions of acpi/video, e.g. transfer acpi event to input event.
So, this patch add a new function, find_video_unregister_backlight, it provide
the interface let downstream driver can tell acpi/video to unregister backlight
interface of all acpi video devices. Then we can keep functions of acpi/video
but only remove backlight support.
Reference: bko#35622
https://bugzilla.kernel.org/show_bug.cgi?id=35622
v2: Also unregister cooling devices.
Tested-by: Andrzej Krentosz
Cc: Zhang Rui
Cc: Len Brown
Cc: Rafael J. Wysocki
Cc: Carlos Corbacho
Cc: Matthew Garrett
Cc: Dmitry Torokhov
Cc: Corentin Chary
Cc: Aaron Lu
Cc: Thomas Renninger
Signed-off-by: Lee, Chun-Yi
---
drivers/acpi/video.c | 93
include/acpi/video.h | 2 ++
2 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 5b32e15..da08ff7 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -43,6 +43,7 @@
#include
#include
#include
+#include
#define PREFIX "ACPI: "
@@ -86,6 +87,9 @@ module_param(allow_duplicates, bool, 0644);
static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
+static bool backlight_disable;
+module_param(backlight_disable, bool, 0644);
+
static int register_count = 0;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device);
@@ -214,6 +218,8 @@ static const char device_decode[][30] = {
"UNKNOWN",
};
+static DEFINE_MUTEX(backlight_mutex);
+
static void acpi_video_device_notify(acpi_handle handle, u32 event, void
*data);
static void acpi_video_device_rebind(struct acpi_video_bus *video);
static void acpi_video_device_bind(struct acpi_video_bus *video,
@@ -882,7 +888,7 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
device->cap._DDC = 1;
}
- if (acpi_video_backlight_support()) {
+ if (acpi_video_backlight_support() || backlight_disable) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
@@ -1083,15 +1089,22 @@ acpi_video_bus_get_one_device(struct acpi_device
*device,
struct acpi_video_device *data;
struct acpi_video_device_attrib* attribute;
+ mutex_lock(&backlight_mutex);
+
status =
acpi_evaluate_integer(device->handle, "_ADR", NULL, &device_id);
/* Some device omits _ADR, we skip them instead of fail */
- if (ACPI_FAILURE(status))
- return 0;
+ if (ACPI_FAILURE(status)) {
+ status = 0;
+ goto out;
+ }
data = kzalloc(sizeof(struct acpi_video_device), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
+ if (!data) {
+ status = -ENOMEM;
+ goto out;
+ }
+
strcpy(acpi_device_name(device), ACPI_VIDEO_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS);
@@ -1156,6 +1169,8 @@ acpi_video_bus_get_one_device(struct acpi_device *device,
list_add_tail(&data->entry, &video->video_device_list);
mutex_unlock(&video->device_list_lock);
+out:
+ mutex_unlock(&backlight_mutex);
return status;
}
@@ -1336,7 +1351,7 @@ acpi_video_switch_brightness(struct acpi_video_device
*device, int event)
int result = -EINVAL;
/* no warning message if acpi_backlight=vendor is used */
- if (!acpi_video_backlight_support())
+ if (!acpi_video_backlight_support() || backlight_disable)
return 0;
if (!device->brightness)
@@ -1869,6 +1884,72 @@ static int __init intel_opregion_present(void)
return opregion;
}
+static acpi_status
+find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context,
+ void **rv)
+{
+ struct acpi_device *acpi_dev;
+ struct acpi_video_bus *video;
+ struct acpi_video_device *dev, *next;
+ acpi_status status = AE_OK;
+
+ mutex_lock(&backlight_mutex);
+
+ if (acpi_bus_get_device(handle, &acpi_dev))
+ goto out;
+
+ if (!acpi_match_device_ids(acpi_dev, video_device_ids)) {
+ video = acpi_driver_data(acpi_dev);
+
+ if (!video)
+ goto out;
+
+ acpi_video_bus_stop_devices(video);
+ mutex_lock(&video->device_list_lock);
+ list_for_each_entry_safe(dev, next, &video->video_device_list,
+ entry) {
+ if (dev->backlight) {
+ backlight_device_unregister(dev->backlight);
+
[Intel-gfx] [PATCH 2/3] ACPICA: Add interface for getting latest OS version requested via _OSI
Drivers may need to make policy decisions based on the OS that the firmware
believes it's interacting with. ACPI firmware will make a series of _OSI
calls, starting from the oldest OS version they support and ending with the
most recent. Add a function to return the last successful call so that
drivers know what the firmware's expecting.
Based on a patch by Seth Forshee
Signed-off-by: Matthew Garrett
Cc: Seth Forshee
---
drivers/acpi/acpica/aclocal.h | 13 -
drivers/acpi/acpica/utxface.c | 19 +++
include/acpi/acpixf.h | 22 ++
3 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h
index d5bfbd3..8a2f532 100644
--- a/drivers/acpi/acpica/aclocal.h
+++ b/drivers/acpi/acpica/aclocal.h
@@ -948,19 +948,6 @@ struct acpi_bit_register_info {
/* Structs and definitions for _OSI support and I/O port validation */
-#define ACPI_OSI_WIN_2000 0x01
-#define ACPI_OSI_WIN_XP 0x02
-#define ACPI_OSI_WIN_XP_SP1 0x03
-#define ACPI_OSI_WINSRV_20030x04
-#define ACPI_OSI_WIN_XP_SP2 0x05
-#define ACPI_OSI_WINSRV_2003_SP10x06
-#define ACPI_OSI_WIN_VISTA 0x07
-#define ACPI_OSI_WINSRV_20080x08
-#define ACPI_OSI_WIN_VISTA_SP1 0x09
-#define ACPI_OSI_WIN_VISTA_SP2 0x0A
-#define ACPI_OSI_WIN_7 0x0B
-#define ACPI_OSI_WIN_8 0x0C
-
#define ACPI_ALWAYS_ILLEGAL 0x00
struct acpi_interface_info {
diff --git a/drivers/acpi/acpica/utxface.c b/drivers/acpi/acpica/utxface.c
index 6505774..c1638c3 100644
--- a/drivers/acpi/acpica/utxface.c
+++ b/drivers/acpi/acpica/utxface.c
@@ -389,6 +389,25 @@ ACPI_EXPORT_SYMBOL(acpi_install_interface_handler)
/*
*
+ * FUNCTION:acpi_osi_version
+ *
+ * PARAMETERS: None
+ *
+ * RETURN: Last OS version requested via _OSI
+ *
+ * DESCRIPTION: Returns the argument to the most recent _OSI query performed
+ * by the firmware
+ *
+ /
+u8 acpi_osi_version(void)
+{
+ return acpi_gbl_osi_data;
+}
+
+ACPI_EXPORT_SYMBOL(acpi_osi_version)
+
+/*
+ *
* FUNCTION:acpi_check_address_range
*
* PARAMETERS: space_id- Address space ID
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 454881e..41d3ac1 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -82,6 +82,22 @@ extern u8 acpi_gbl_truncate_io_addresses;
extern u8 acpi_gbl_disable_auto_repair;
/*
+ * Values returned by acpi_osi_version()
+ */
+#define ACPI_OSI_WIN_2000 0x01
+#define ACPI_OSI_WIN_XP 0x02
+#define ACPI_OSI_WIN_XP_SP1 0x03
+#define ACPI_OSI_WINSRV_20030x04
+#define ACPI_OSI_WIN_XP_SP2 0x05
+#define ACPI_OSI_WINSRV_2003_SP10x06
+#define ACPI_OSI_WIN_VISTA 0x07
+#define ACPI_OSI_WINSRV_20080x08
+#define ACPI_OSI_WIN_VISTA_SP1 0x09
+#define ACPI_OSI_WIN_VISTA_SP2 0x0A
+#define ACPI_OSI_WIN_7 0x0B
+#define ACPI_OSI_WIN_8 0x0C
+
+/*
* Hardware-reduced prototypes. All interfaces that use these macros will
* be configured out of the ACPICA build if the ACPI_REDUCED_HARDWARE flag
* is set to TRUE.
@@ -307,6 +323,12 @@ acpi_status acpi_install_notify_handler(acpi_handle
device, u32 handler_type,
acpi_notify_handler handler,
void *context);
+#ifdef CONFIG_ACPI
+u8 acpi_osi_version(void);
+#else
+static inline u8 acpi_osi_version(void) { return 0; }
+#endif
+
acpi_status
acpi_remove_notify_handler(acpi_handle device,
u32 handler_type, acpi_notify_handler handler);
--
1.8.2.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
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 --- 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(); } if (IS_GEN5(dev)) -- 1.8.2.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: propagate errors from intel_dp_init_connector
From: Paulo Zanoni
In case we detect a "ghost eDP", intel_dp_init_connector was freeing
both the connector and encoder and then returning. On Haswell,
intel_ddi_init was trying to use the freed encoder on the HDMI
initialization path since the following commit:
commit 21a8e6a4853b2ed39fa4c5188a710f2cf1b92026
Author: Daniel Vetter
Date: Wed Apr 10 23:28:35 2013 +0200
drm/i915: don't setup hdmi for port D edp in ddi_init
So on this patch we change intel_dp_init_connector from void to bool,
check its return value on its callers and leave the resource-freeing
task to its callers (since the callers are the ones who allocate the
resources). With this change it will be easier to see that
intel_dp_init_connector can fail, so I hope we won't have similar
regressions in the future.
Also include a small change to remove extra "{" and "}" on nearby
code.
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_ddi.c | 10 +++---
drivers/gpu/drm/i915/intel_dp.c | 15 +--
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
3 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 224ce25..4e0cf37 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1356,14 +1356,18 @@ void intel_ddi_init(struct drm_device *dev, enum port
port)
intel_encoder->cloneable = false;
intel_encoder->hot_plug = intel_ddi_hot_plug;
- intel_dp_init_connector(intel_dig_port, dp_connector);
+ /* May fail if it's an eDP connector that's disconnected. */
+ if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
+ intel_ddi_destroy(encoder);
+ intel_dp_destroy(&dp_connector->base);
+ return;
+ }
if (intel_encoder->type != INTEL_OUTPUT_EDP) {
hdmi_connector = kzalloc(sizeof(struct intel_connector),
GFP_KERNEL);
- if (!hdmi_connector) {
+ if (!hdmi_connector)
return;
- }
intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
intel_hdmi_init_connector(intel_dig_port, hdmi_connector);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0de82bb..ec23b3d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2687,7 +2687,7 @@ done:
return 0;
}
-static void
+void
intel_dp_destroy(struct drm_connector *connector)
{
struct intel_dp *intel_dp = intel_attached_dp(connector);
@@ -2961,7 +2961,7 @@ intel_dp_init_panel_power_sequencer_registers(struct
drm_device *dev,
I915_READ(pp_div_reg));
}
-void
+bool
intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct intel_connector *intel_connector)
{
@@ -3095,9 +3095,7 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
} else {
/* if this fails, presume the device is a ghost */
DRM_INFO("failed to retrieve link info, disabling
eDP\n");
- intel_dp_encoder_destroy(&intel_encoder->base);
- intel_dp_destroy(connector);
- return;
+ return false;
}
/* We now know it's not a ghost, init power sequence regs. */
@@ -3152,6 +3150,8 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
u32 temp = I915_READ(PEG_BAND_GAP_DATA);
I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
}
+
+ return true;
}
void
@@ -3197,5 +3197,8 @@ intel_dp_init(struct drm_device *dev, int output_reg,
enum port port)
intel_encoder->cloneable = false;
intel_encoder->hot_plug = intel_dp_hot_plug;
- intel_dp_init_connector(intel_dig_port, intel_connector);
+ if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
+ intel_dp_encoder_destroy(encoder);
+ intel_dp_destroy(&intel_connector->base);
+ }
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b9e250c..8a7ad99 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -569,9 +569,10 @@ extern bool intel_lvds_init(struct drm_device *dev);
extern bool intel_is_dual_link_lvds(struct drm_device *dev);
extern void intel_dp_init(struct drm_device *dev, int output_reg,
enum port port);
-extern void intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
+extern bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct intel_connector *intel_connector);
extern void intel_dp_init_link_config(struct intel_dp *intel_dp);
+extern void intel_dp_destroy(struct drm_connector *
Re: [Intel-gfx] [PATCH 00/31] shared pch display pll rework
On Fri, Jun 07, 2013 at 08:46:45PM +0300, Ville Syrjälä wrote: > I've just gone over patches 01-13. > > For patches 01,02,04,05,07,09-13 > Reviewed-by: Ville Syrjälä > > For patches 03,06,08 I replied with some concerns/ideas. You can slap my r-b onto those three as well. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: lock down pch pll accouting some more
Before I start to make a complete mess out of this, crank up
the paranoia level a bit.
v2: Kill the has_pch_encoder check in put_shared_dpll - it's invalid
as spotted by Ville since we currently only put the dpll when we
already have the new pipe config. So a direct pch port -> cpu edp
transition will hit this.
v3: Now that I've lifted my blinders add the WARN_ON Ville requested.
Cc: Ville Syrjälä
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8fbcf31..4d50b0a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1414,6 +1414,7 @@ static void ironlake_enable_pch_pll(struct intel_crtc
*intel_crtc)
assert_pch_pll_enabled(dev_priv, pll, NULL);
return;
}
+ WARN_ON(pll->on);
DRM_DEBUG_KMS("enabling PCH PLL %x\n", pll->pll_reg);
@@ -1452,6 +1453,7 @@ static void intel_disable_pch_pll(struct intel_crtc
*intel_crtc)
}
assert_pch_pll_enabled(dev_priv, pll, NULL);
+ WARN_ON(!pll->on);
if (--pll->active)
return;
@@ -3051,7 +3053,11 @@ static void intel_put_pch_pll(struct intel_crtc
*intel_crtc)
return;
}
- --pll->refcount;
+ if (--pll->refcount == 0) {
+ WARN_ON(pll->on);
+ WARN_ON(pll->active);
+ }
+
intel_crtc->pch_pll = NULL;
}
--
1.7.11.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: disable sdvo pixel multiplier cross-check for HAS_PCH_SPLIT
We don't (yet) have proper pixel multiplier readout support on pch split platforms, so the cross check will naturally fail. v2: Fix spelling in the comment, spotted by Ville. Reported-by: Chris Wilson Cc: Chris Wilson Cc: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_sdvo.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 67710e1..34cd3fd 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1362,6 +1362,10 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder, encoder_pixel_multiplier = 4; break; } + + if(HAS_PCH_SPLIT(dev)) + return; /* no pixel multiplier readout support yet */ + WARN(encoder_pixel_multiplier != pipe_config->pixel_multiplier, "SDVO pixel multiplier mismatch, port: %i, encoder: %i\n", pipe_config->pixel_multiplier, encoder_pixel_multiplier); -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use FBINFO_STATE defines instead of 0 and 1
On Mon, Jun 10, 2013 at 04:03:58PM +0100, Chris Wilson wrote: > On Mon, Jun 10, 2013 at 03:48:09PM +0100, Damien Lespiau wrote: > > This makes, arguably, the condition on state easier to read. > > > > Suggested-by: Chris Wilson > > Signed-off-by: Damien Lespiau > Reviewed-by: Chris Wilson 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: Use FBINFO_STATE defines instead of 0 and 1
On Mon, Jun 10, 2013 at 03:48:09PM +0100, Damien Lespiau wrote: > This makes, arguably, the condition on state easier to read. > > Suggested-by: Chris Wilson > Signed-off-by: Damien Lespiau Reviewed-by: Chris Wilson -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] drm/i915: Workaround incoherence between fences and LLC across multiple CPUs
On Mon, Jun 10, 2013 at 4:47 PM, Bloomfield, Jon wrote: > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. Oh, the pinning isn't a suitable w/a for upstream nor for general consumption. But I guess for the realtime linux folks it is suitable (at least for now) since they tend to not care too much about gfx throughput anyway. [Aside: The patch is only for the -rt tree for now, so not something that will land upstream.] -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Use FBINFO_STATE defines instead of 0 and 1
This makes, arguably, the condition on state easier to read.
Suggested-by: Chris Wilson
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_drv.c | 6 +++---
drivers/gpu/drm/i915/intel_fb.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b23cd63..381c9dd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -546,7 +546,7 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_opregion_fini(dev);
console_lock();
- intel_fbdev_set_suspend(dev, 1);
+ intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
console_unlock();
return 0;
@@ -590,7 +590,7 @@ void intel_console_resume(struct work_struct *work)
struct drm_device *dev = dev_priv->dev;
console_lock();
- intel_fbdev_set_suspend(dev, 0);
+ intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING);
console_unlock();
}
@@ -659,7 +659,7 @@ static int __i915_drm_thaw(struct drm_device *dev)
* path of resume if possible.
*/
if (console_trylock()) {
- intel_fbdev_set_suspend(dev, 0);
+ intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING);
console_unlock();
} else {
schedule_work(&dev_priv->console_resume_work);
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 7a77d4c..244060a 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -275,7 +275,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int
state)
* been restored from swap. If the object is stolen however, it will be
* full of whatever garbage was left in there.
*/
- if (!state && ifbdev->ifb.obj->stolen)
+ if (state == FBINFO_STATE_RUNNING && ifbdev->ifb.obj->stolen)
memset_io(info->screen_base, 0, info->screen_size);
fb_set_suspend(info, state);
--
1.8.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Workaround incoherence between fences and LLC across multiple CPUs
On Mon, Jun 10, 2013 at 3:17 PM, Bloomfield, Jon
wrote:
>> -Original Message-
>> From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of
>> Daniel Vetter
>> Sent: Saturday, June 08, 2013 10:22 PM
>> To: Carsten Emde
>> Cc: Chris Wilson; Jesse Barnes; Intel Graphics Development; Bloomfield, Jon;
>> Steven Rostedt; Christoph Mathys; stable
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Workaround incoherence
>> between fences and LLC across multiple CPUs
>>
>> On Sat, Jun 8, 2013 at 10:41 PM, Carsten Emde wrote:
>> > On 04/08/2013 11:54 AM, Daniel Vetter wrote:
>> >>
>> >> On Fri, Apr 05, 2013 at 09:03:27AM -0700, Jesse Barnes wrote:
>> >>>
>> >>> On Thu, 4 Apr 2013 21:31:03 +0100
>> >>> Chris Wilson wrote:
>> >>>
>> In order to fully serialize access to the fenced region and the
>> update to the fence register we need to take extreme measures on
>> SNB+, and manually flush writes to memory prior to writing the
>> fence register in conjunction with the memory barriers placed around
>> the register write.
>>
>> Fixes i-g-t/gem_fence_thrash
>>
>> v2: Bring a bigger gun
>> v3: Switch the bigger gun for heavier bullets (Arjan van de Ven)
>> v4: Remove changes for working generations.
>> v5: Reduce to a per-cpu wbinvd() call prior to updating the fences.
>> v6: Rewrite comments to ellide forgotten history.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=62191
>> Signed-off-by: Chris Wilson
>> Cc: Jon Bloomfield
>> Tested-by: Jon Bloomfield (v2)
>> Cc: sta...@vger.kernel.org
>> ---
>> drivers/gpu/drm/i915/i915_gem.c | 28
>> +++-
>> 1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> b/drivers/gpu/drm/i915/i915_gem.c index fa4ea1a..8f7739e 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -2689,17 +2689,35 @@ static inline int fence_number(struct
>> drm_i915_private *dev_priv,
>> return fence - dev_priv->fence_regs;
>> }
>>
>> +static void i915_gem_write_fence__ipi(void *data) {
>> + wbinvd();
>> +}
>> +
>> static void i915_gem_object_update_fence(struct
>> drm_i915_gem_object *obj,
>> struct drm_i915_fence_reg
>> *fence,
>> bool enable)
>> {
>> - struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
>> - int reg = fence_number(dev_priv, fence);
>> -
>> - i915_gem_write_fence(obj->base.dev, reg, enable ? obj : NULL);
>> + struct drm_device *dev = obj->base.dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + int fence_reg = fence_number(dev_priv, fence);
>> +
>> + /* In order to fully serialize access to the fenced region and
>> +* the update to the fence register we need to take extreme
>> +* measures on SNB+. In theory, the write to the fence register
>> +* flushes all memory transactions before, and coupled with the
>> +* mb() placed around the register write we serialise all memory
>> +* operations with respect to the changes in the tiler. Yet, on
>> +* SNB+ we need to take a step further and emit an explicit
>> wbinvd()
>> +* on each processor in order to manually flush all memory
>> +* transactions before updating the fence register.
>> +*/
>> + if (HAS_LLC(obj->base.dev))
>> + on_each_cpu(i915_gem_write_fence__ipi, NULL, 1);
>> + i915_gem_write_fence(dev, fence_reg, enable ? obj : NULL);
>>
>> if (enable) {
>> - obj->fence_reg = reg;
>> + obj->fence_reg = fence_reg;
>> fence->obj = obj;
>> list_move_tail(&fence->lru_list,
>> &dev_priv->mm.fence_list);
>> } else {
>> >>>
>> >>>
>> >>> I almost wish x86 just gave up and went fully weakly ordered. At
>> >>> least then we'd know we need barriers everywhere, rather than just
>> >>> in mystical places.
>> >>>
>> >>> Reviewed-by: Jesse Barnes
>> >>
>> >>
>> >> Queued for -next, thanks for the patch. I am a bit unhappy that the
>> >> testcase doesn't work too well and can't reliably reproduce this on
>> >> all affected machines. But I've tried a bit to improve things and
>> >> it's very fickle. I guess we just have to accept this :(
>> >
>> >
>> > Under real-time conditions when we expect deterministic response to
>> > external and internal events at any time within a couple of
>> > microseconds, invalidating and flushing the entire cache in a running
>> > system is unacceptable. This is introducing latencies o
Re: [Intel-gfx] [PATCH] drm/i915: Workaround incoherence between fences and LLC across multiple CPUs
> -Original Message-
> From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Monday, June 10, 2013 3:38 PM
> To: Bloomfield, Jon
> Cc: Carsten Emde; Chris Wilson; Jesse Barnes; Intel Graphics Development;
> Steven Rostedt; Christoph Mathys; stable
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Workaround incoherence
> between fences and LLC across multiple CPUs
>
> On Mon, Jun 10, 2013 at 3:17 PM, Bloomfield, Jon
> wrote:
> >> -Original Message-
> >> From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On
> >> Behalf Of Daniel Vetter
> >> Sent: Saturday, June 08, 2013 10:22 PM
> >> To: Carsten Emde
> >> Cc: Chris Wilson; Jesse Barnes; Intel Graphics Development;
> >> Bloomfield, Jon; Steven Rostedt; Christoph Mathys; stable
> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Workaround incoherence
> >> between fences and LLC across multiple CPUs
> >>
> >> On Sat, Jun 8, 2013 at 10:41 PM, Carsten Emde
> wrote:
> >> > On 04/08/2013 11:54 AM, Daniel Vetter wrote:
> >> >>
> >> >> On Fri, Apr 05, 2013 at 09:03:27AM -0700, Jesse Barnes wrote:
> >> >>>
> >> >>> On Thu, 4 Apr 2013 21:31:03 +0100 Chris Wilson
> >> >>> wrote:
> >> >>>
> >> In order to fully serialize access to the fenced region and the
> >> update to the fence register we need to take extreme measures on
> >> SNB+, and manually flush writes to memory prior to writing the
> >> fence register in conjunction with the memory barriers placed
> >> around
> >> the register write.
> >>
> >> Fixes i-g-t/gem_fence_thrash
> >>
> >> v2: Bring a bigger gun
> >> v3: Switch the bigger gun for heavier bullets (Arjan van de Ven)
> >> v4: Remove changes for working generations.
> >> v5: Reduce to a per-cpu wbinvd() call prior to updating the fences.
> >> v6: Rewrite comments to ellide forgotten history.
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=62191
> >> Signed-off-by: Chris Wilson
> >> Cc: Jon Bloomfield
> >> Tested-by: Jon Bloomfield (v2)
> >> Cc: sta...@vger.kernel.org
> >> ---
> >> drivers/gpu/drm/i915/i915_gem.c | 28
> >> +++-
> >> 1 file changed, 23 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> >> b/drivers/gpu/drm/i915/i915_gem.c index fa4ea1a..8f7739e 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> @@ -2689,17 +2689,35 @@ static inline int fence_number(struct
> >> drm_i915_private *dev_priv,
> >> return fence - dev_priv->fence_regs;
> >> }
> >>
> >> +static void i915_gem_write_fence__ipi(void *data) {
> >> + wbinvd();
> >> +}
> >> +
> >> static void i915_gem_object_update_fence(struct
> >> drm_i915_gem_object *obj,
> >> struct
> >> drm_i915_fence_reg *fence,
> >> bool enable)
> >> {
> >> - struct drm_i915_private *dev_priv = obj->base.dev-
> >dev_private;
> >> - int reg = fence_number(dev_priv, fence);
> >> -
> >> - i915_gem_write_fence(obj->base.dev, reg, enable ? obj :
> NULL);
> >> + struct drm_device *dev = obj->base.dev;
> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> + int fence_reg = fence_number(dev_priv, fence);
> >> +
> >> + /* In order to fully serialize access to the fenced region and
> >> +* the update to the fence register we need to take extreme
> >> +* measures on SNB+. In theory, the write to the fence
> >> register
> >> +* flushes all memory transactions before, and coupled with
> >> the
> >> +* mb() placed around the register write we serialise all
> memory
> >> +* operations with respect to the changes in the tiler. Yet,
> >> on
> >> +* SNB+ we need to take a step further and emit an
> >> + explicit
> >> wbinvd()
> >> +* on each processor in order to manually flush all memory
> >> +* transactions before updating the fence register.
> >> +*/
> >> + if (HAS_LLC(obj->base.dev))
> >> + on_each_cpu(i915_gem_write_fence__ipi, NULL, 1);
> >> + i915_gem_write_fence(dev, fence_reg, enable ? obj :
> >> + NULL);
> >>
> >> if (enable) {
> >> - obj->fence_reg = reg;
> >> + obj->fence_reg = fence_reg;
> >> fence->obj = obj;
> >> list_move_tail(&fence->lru_list,
> >> &dev_priv->mm.fence_list);
> >> } else {
> >> >>>
> >> >>>
> >> >>> I almost wish x86 just gave up and went fully weakly ordered. At
> >> >>> least then we'd know we ne
Re: [Intel-gfx] [PATCH 03/31] drm/i915: lock down pch pll accouting some more
On Mon, Jun 10, 2013 at 04:34:05PM +0200, Daniel Vetter wrote:
> On Mon, Jun 10, 2013 at 12:11 PM, Ville Syrjälä
> wrote:
> > On Fri, Jun 07, 2013 at 11:13:32PM +0200, Daniel Vetter wrote:
> >> On Fri, Jun 07, 2013 at 11:46:08PM +0300, Ville Syrjälä wrote:
> >> > On Fri, Jun 07, 2013 at 10:03:20PM +0200, Daniel Vetter wrote:
> >> > > On Fri, Jun 07, 2013 at 07:32:56PM +0300, Ville Syrjälä wrote:
> >> > > > On Wed, Jun 05, 2013 at 01:34:05PM +0200, Daniel Vetter wrote:
> >> > > > > Before I start to make a complete mess out of this, crank up
> >> > > > > the paranoia level a bit.
> >> > > > >
> >> > > > > Signed-off-by: Daniel Vetter
> >> > > > > ---
> >> > > > > drivers/gpu/drm/i915/intel_display.c | 9 -
> >> > > > > 1 file changed, 8 insertions(+), 1 deletion(-)
> >> > > > >
> >> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> > > > > b/drivers/gpu/drm/i915/intel_display.c
> >> > > > > index 56fb6ed..39e977f 100644
> >> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> >> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> >> > > > > @@ -1440,6 +1440,7 @@ static void intel_disable_pch_pll(struct
> >> > > > > intel_crtc *intel_crtc)
> >> > > > > }
> >> > > > >
> >> > > > > assert_pch_pll_enabled(dev_priv, pll, NULL);
> >> > > > > + WARN_ON(!pll->on);
> >> > > > > if (--pll->active)
> >> > > > > return;
> >> > > >
> >> > > > Maybe a WARN_ON(pll->on) near the end of ironlake_enable_pch_pll()
> >> > > > too?
> >> > >
> >> > > At the very end we set on = true, and the only non-error early return
> >> > > (when the active refcount is > 0 to begin with) has alreay a
> >> > > WARN_ON(!pll->on). Shouldn't that be good enough?
> >> >
> >> > Well I was just thinking that since we have this dual bookeeping w/
> >> > active and on, maybe we want to warn if things go out of sync.
> >>
> >> Now I'm confused. I've tried to explain why I think we already have full
> >> checking of pll->on in enable_shared_dpll ... Can you maybe show in a diff
> >> where you'd want to add more?
> >
> > Something like this:
> >
> > if (pll->active++) {
> > WARN_ON(!pll->on);
> > assert_pch_pll_enabled(dev_priv, pll, NULL);
> > return;
> > }
> > + WARN_ON(pll->on);
>
> That one's gonna misfire every time since we set pll->on = true only
> at the end of the function in this case.
>
> > and maybe also:
> > + assert_pch_pll_disabled(dev_priv, pll, NULL);
>
> This one should already be in the platform-specific pll->enable hook.
> It's added in "drm/i915: enable/disable hooks for shared dplls"
>
> > Or maybe just kill 'pll->on' as it seems totally redundant.
>
> Yeah, I've considered that but independently checking pll->on with the
> hw state and pll->active with the number of crtc using it looked
> neated. I guess we could rip out pll->on as a follow-up though.
>
> > Also maybe we could move most of the asserts and WARNs to some
> > central location. Currently there are quite a few early return paths
> > from the pll enable/disable functions, and I don't think we perform the
> > same checks for all of the branches. So maybe we could just have one
> > function that would cross-check pll->on, pll->active and the real hardware
> > state. We could call said function just before and after
> > enable/disable_pch_pll().
>
> The totally paranoid hw state cross checker does that at the very end
> of each modeset. The checks in here are simply to assert a bunch of
> edge transitions. And like I've said, I think I pretty much have it
> all covered.
Before we set pll->on to true, pll->on should be false, no?
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/31] drm/i915: lock down pch pll accouting some more
On Mon, Jun 10, 2013 at 12:11 PM, Ville Syrjälä
wrote:
> On Fri, Jun 07, 2013 at 11:13:32PM +0200, Daniel Vetter wrote:
>> On Fri, Jun 07, 2013 at 11:46:08PM +0300, Ville Syrjälä wrote:
>> > On Fri, Jun 07, 2013 at 10:03:20PM +0200, Daniel Vetter wrote:
>> > > On Fri, Jun 07, 2013 at 07:32:56PM +0300, Ville Syrjälä wrote:
>> > > > On Wed, Jun 05, 2013 at 01:34:05PM +0200, Daniel Vetter wrote:
>> > > > > Before I start to make a complete mess out of this, crank up
>> > > > > the paranoia level a bit.
>> > > > >
>> > > > > Signed-off-by: Daniel Vetter
>> > > > > ---
>> > > > > drivers/gpu/drm/i915/intel_display.c | 9 -
>> > > > > 1 file changed, 8 insertions(+), 1 deletion(-)
>> > > > >
>> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > > > > b/drivers/gpu/drm/i915/intel_display.c
>> > > > > index 56fb6ed..39e977f 100644
>> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
>> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > > > > @@ -1440,6 +1440,7 @@ static void intel_disable_pch_pll(struct
>> > > > > intel_crtc *intel_crtc)
>> > > > > }
>> > > > >
>> > > > > assert_pch_pll_enabled(dev_priv, pll, NULL);
>> > > > > + WARN_ON(!pll->on);
>> > > > > if (--pll->active)
>> > > > > return;
>> > > >
>> > > > Maybe a WARN_ON(pll->on) near the end of ironlake_enable_pch_pll() too?
>> > >
>> > > At the very end we set on = true, and the only non-error early return
>> > > (when the active refcount is > 0 to begin with) has alreay a
>> > > WARN_ON(!pll->on). Shouldn't that be good enough?
>> >
>> > Well I was just thinking that since we have this dual bookeeping w/
>> > active and on, maybe we want to warn if things go out of sync.
>>
>> Now I'm confused. I've tried to explain why I think we already have full
>> checking of pll->on in enable_shared_dpll ... Can you maybe show in a diff
>> where you'd want to add more?
>
> Something like this:
>
> if (pll->active++) {
> WARN_ON(!pll->on);
> assert_pch_pll_enabled(dev_priv, pll, NULL);
> return;
> }
> + WARN_ON(pll->on);
That one's gonna misfire every time since we set pll->on = true only
at the end of the function in this case.
> and maybe also:
> + assert_pch_pll_disabled(dev_priv, pll, NULL);
This one should already be in the platform-specific pll->enable hook.
It's added in "drm/i915: enable/disable hooks for shared dplls"
> Or maybe just kill 'pll->on' as it seems totally redundant.
Yeah, I've considered that but independently checking pll->on with the
hw state and pll->active with the number of crtc using it looked
neated. I guess we could rip out pll->on as a follow-up though.
> Also maybe we could move most of the asserts and WARNs to some
> central location. Currently there are quite a few early return paths
> from the pll enable/disable functions, and I don't think we perform the
> same checks for all of the branches. So maybe we could just have one
> function that would cross-check pll->on, pll->active and the real hardware
> state. We could call said function just before and after
> enable/disable_pch_pll().
The totally paranoid hw state cross checker does that at the very end
of each modeset. The checks in here are simply to assert a bunch of
edge transitions. And like I've said, I think I pretty much have it
all covered.
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 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
Radeon probably needs something similar. See attached untested patch.
Alex
On Sun, Jun 9, 2013 at 7:01 PM, 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
> ---
> 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();
> }
>
> if (IS_GEN5(dev))
> --
> 1.8.2.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
From c6ead3429fd3977b4b2ee5748d83c72272592b29 Mon Sep 17 00:00:00 2001
From: Alex Deucher
Date: Mon, 10 Jun 2013 10:05:43 -0400
Subject: [PATCH] drm/radeon: don't provide ACPI backlight if firmware expects
Windows 8
Windows 8 leaves backlight control up to individual graphics drivers rather
than making ACPI calls itself.
Untested.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/radeon/atombios_encoders.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 4120d35..bc9e007 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -233,6 +233,10 @@ void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
DRM_INFO("radeon atom DIG backlight initialized\n");
+ /* Don't use ACPI backlight functions on Windows 8 platforms */
+ if (acpi_osi_version() >= ACPI_OSI_WIN_8)
+ acpi_video_backlight_unregister();
+
return;
error:
--
1.7.7.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Eliminate the addr/seqno from the hangcheck warning
Chris Wilson writes:
> This is of no value to the developer reading the report, let alone the
> bamboozled user.
>
> Signed-off-by: Chris Wilson
> Acked-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_irq.c |8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index cf8584c..39730b6 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2525,12 +2525,10 @@ void i915_hangcheck_elapsed(unsigned long data)
>
> for_each_ring(ring, dev_priv, i) {
> if (ring->hangcheck.score > FIRE) {
> - rings_hung++;
> - DRM_ERROR("%s: %s on %s 0x%x\n", ring->name,
> + DRM_ERROR("%s on %s ring\n",
^^
Noticed one redudant 'ring' in here as ring->name already contains it.
Patches 1-4:
Reviewed-by: Mika Kuoppala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Workaround incoherence between fences and LLC across multiple CPUs
> -Original Message-
> From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Saturday, June 08, 2013 10:22 PM
> To: Carsten Emde
> Cc: Chris Wilson; Jesse Barnes; Intel Graphics Development; Bloomfield, Jon;
> Steven Rostedt; Christoph Mathys; stable
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Workaround incoherence
> between fences and LLC across multiple CPUs
>
> On Sat, Jun 8, 2013 at 10:41 PM, Carsten Emde wrote:
> > On 04/08/2013 11:54 AM, Daniel Vetter wrote:
> >>
> >> On Fri, Apr 05, 2013 at 09:03:27AM -0700, Jesse Barnes wrote:
> >>>
> >>> On Thu, 4 Apr 2013 21:31:03 +0100
> >>> Chris Wilson wrote:
> >>>
> In order to fully serialize access to the fenced region and the
> update to the fence register we need to take extreme measures on
> SNB+, and manually flush writes to memory prior to writing the
> fence register in conjunction with the memory barriers placed around
> the register write.
>
> Fixes i-g-t/gem_fence_thrash
>
> v2: Bring a bigger gun
> v3: Switch the bigger gun for heavier bullets (Arjan van de Ven)
> v4: Remove changes for working generations.
> v5: Reduce to a per-cpu wbinvd() call prior to updating the fences.
> v6: Rewrite comments to ellide forgotten history.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=62191
> Signed-off-by: Chris Wilson
> Cc: Jon Bloomfield
> Tested-by: Jon Bloomfield (v2)
> Cc: sta...@vger.kernel.org
> ---
> drivers/gpu/drm/i915/i915_gem.c | 28
> +++-
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c index fa4ea1a..8f7739e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2689,17 +2689,35 @@ static inline int fence_number(struct
> drm_i915_private *dev_priv,
> return fence - dev_priv->fence_regs;
> }
>
> +static void i915_gem_write_fence__ipi(void *data) {
> + wbinvd();
> +}
> +
> static void i915_gem_object_update_fence(struct
> drm_i915_gem_object *obj,
> struct drm_i915_fence_reg
> *fence,
> bool enable)
> {
> - struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> - int reg = fence_number(dev_priv, fence);
> -
> - i915_gem_write_fence(obj->base.dev, reg, enable ? obj : NULL);
> + struct drm_device *dev = obj->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int fence_reg = fence_number(dev_priv, fence);
> +
> + /* In order to fully serialize access to the fenced region and
> +* the update to the fence register we need to take extreme
> +* measures on SNB+. In theory, the write to the fence register
> +* flushes all memory transactions before, and coupled with the
> +* mb() placed around the register write we serialise all memory
> +* operations with respect to the changes in the tiler. Yet, on
> +* SNB+ we need to take a step further and emit an explicit
> wbinvd()
> +* on each processor in order to manually flush all memory
> +* transactions before updating the fence register.
> +*/
> + if (HAS_LLC(obj->base.dev))
> + on_each_cpu(i915_gem_write_fence__ipi, NULL, 1);
> + i915_gem_write_fence(dev, fence_reg, enable ? obj : NULL);
>
> if (enable) {
> - obj->fence_reg = reg;
> + obj->fence_reg = fence_reg;
> fence->obj = obj;
> list_move_tail(&fence->lru_list,
> &dev_priv->mm.fence_list);
> } else {
> >>>
> >>>
> >>> I almost wish x86 just gave up and went fully weakly ordered. At
> >>> least then we'd know we need barriers everywhere, rather than just
> >>> in mystical places.
> >>>
> >>> Reviewed-by: Jesse Barnes
> >>
> >>
> >> Queued for -next, thanks for the patch. I am a bit unhappy that the
> >> testcase doesn't work too well and can't reliably reproduce this on
> >> all affected machines. But I've tried a bit to improve things and
> >> it's very fickle. I guess we just have to accept this :(
> >
> >
> > Under real-time conditions when we expect deterministic response to
> > external and internal events at any time within a couple of
> > microseconds, invalidating and flushing the entire cache in a running
> > system is unacceptable. This is introducing latencies of several
> > milliseconds which was clearly shown in RT regression tests on a
> > kernel with this patch applied (https://www.osadl.org/?id=1543#c7602).
> > We there
[Intel-gfx] [PATCH] drm/i915: Reorder ring reinitialisation round reset
In particular, we were reseting our GEM tracking before even attempting
(and failing) to reset the chip. Do it with the other bookkeeping after
successfully reseting. Note that we can simplify the reset to always do
the GPU reinitialisation as for UMS it will simply be redundant in the
case where X was switched away. And that we can safely re-establish all
the features that were setup during initialisation - any that were not
enabled for UMS/DRI1 will not be restored and should still not be enabled.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.c | 50 +--
1 file changed, 17 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ac145bb..16c273c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -897,16 +897,15 @@ int intel_gpu_reset(struct drm_device *dev)
int i915_reset(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
+ struct intel_ring_buffer *ring;
bool simulated;
- int ret;
+ int i, ret;
if (!i915_try_reset)
return 0;
mutex_lock(&dev->struct_mutex);
- i915_gem_reset(dev);
-
simulated = dev_priv->gpu_error.stop_rings != 0;
if (!simulated && get_seconds() - dev_priv->gpu_error.last_reset < 5) {
@@ -935,38 +934,23 @@ int i915_reset(struct drm_device *dev)
/* Ok, now get things going again... */
- /*
-* Everything depends on having the GTT running, so we need to start
-* there. Fortunately we don't need to do this unless we reset the
-* chip at a PCI level.
-*
-* Next we need to restore the context, but we don't use those
-* yet either...
-*
-* Ring buffer needs to be re-initialized in the KMS case, or if X
-* was running at the time of the reset (i.e. we weren't VT
-* switched away).
-*/
- if (drm_core_check_feature(dev, DRIVER_MODESET) ||
- !dev_priv->mm.suspended) {
- struct intel_ring_buffer *ring;
- int i;
-
- dev_priv->mm.suspended = 0;
+ i915_gem_init_swizzling(dev);
- i915_gem_init_swizzling(dev);
+ for_each_ring(ring, dev_priv, i) {
+ ret = ring->init(ring);
+ if (ret)
+ goto unlock;
+ }
- for_each_ring(ring, dev_priv, i) {
- ret = ring->init(ring);
- if (ret)
- goto unlock;
- }
+ i915_gem_reset(dev);
+ i915_gem_context_init(dev);
+ if (dev_priv->mm.aliasing_ppgtt) {
+ if (dev_priv->mm.aliasing_ppgtt->enable(dev))
+ i915_gem_cleanup_aliasing_ppgtt(dev);
+ }
- i915_gem_context_init(dev);
- if (dev_priv->mm.aliasing_ppgtt) {
- if (dev_priv->mm.aliasing_ppgtt->enable(dev))
- i915_gem_cleanup_aliasing_ppgtt(dev);
- }
+ if (drm_core_check_feature(dev, DRIVER_MODESET))
+ dev_priv->mm.suspended = 0;
/*
* It would make sense to re-init all the other hw state, at
@@ -974,13 +958,13 @@ int i915_reset(struct drm_device *dev)
* some unknown reason, this blows up my ilk, so don't.
*/
-unlock:
mutex_unlock(&dev->struct_mutex);
drm_irq_uninstall(dev);
drm_irq_install(dev);
intel_hpd_init(dev);
} else {
+unlock:
mutex_unlock(&dev->struct_mutex);
}
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Initialize active_outputs to never read unitialized values
In case of intel_sdvo_get_active_outputs() failing, we end up reading a value from the stack. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_sdvo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index ac923b7..040a259 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1277,7 +1277,7 @@ static bool intel_sdvo_connector_get_hw_state(struct intel_connector *connector) struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(&connector->base); struct intel_sdvo *intel_sdvo = intel_attached_sdvo(&connector->base); - u16 active_outputs; + u16 active_outputs = 0; intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs); @@ -1293,7 +1293,7 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder, struct drm_device *dev = encoder->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base); - u16 active_outputs; + u16 active_outputs = 0; u32 tmp; tmp = I915_READ(intel_sdvo->sdvo_reg); -- 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 old reference to i830_sdvo_get_capabilities()
It's now intel_sdvo_get_capabilities().
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_sdvo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo.c
index 7068195..221d415 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -80,7 +80,7 @@ struct intel_sdvo {
/*
* Capabilities of the SDVO device returned by
-* i830_sdvo_get_capabilities()
+* intel_sdvo_get_capabilities()
*/
struct intel_sdvo_caps caps;
--
1.8.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
Hi Matthew, On Sunday, June 09, 2013 07:01:36 PM Matthew Garrett wrote: > Windows 8 introduced new policy for backlight control by pushing it out to > graphics drivers. This appears to have coincided with a range of vendors > adding Windows 8 checks to their backlight control code which trigger either > awkward behaviour (Lenovo) or complete brokenness (some Dells). The simplest > thing to do would be to just disable ACPI backlight control entirely if the > firmware indicates Windows 8 support, but it's entirely possible that > individual graphics drivers might still make use of the ACPI functionality in > preference to native control. > > The first two patches in this series are picked from other patchesets aimed at > solving similar problems. The last simply unregisters ACPI backlight control > on Windows 8 systems when using an Intel GPU. Similar code could be added to > other drivers, but I'm reluctant to do so without further investigation as > to the behaviour of the vendor drivers under Windows. I happen to know that alternative solution to this problem is being worked on, so I'm going to wait until it is submitted and then we'll decide what to merge. I'm slightly concerned about unregistering ACPI backlight control for all systems declaring win8 support, even though it may actually work for them. Thanks, Rafael -- 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
[Intel-gfx] [PATCH 3/4] drm/i915: Don't count semaphore waits towards a stuck ring
If we detect a ring is in a valid wait for another, just let it be.
Eventually it will either begin to progress again, or the entire system
will come grinding to a halt and then hangcheck will fire as soon as the
deadlock is detected.
This error was foretold by Ben in
commit 05407ff889ceebe383aa5907219f86582ef96b72
Author: Mika Kuoppala
Date: Thu May 30 09:04:29 2013 +0300
drm/i915: detect hang using per ring hangcheck_score
"If ring B is waiting on ring A via semaphore, and ring A is making
progress, albeit slowly - the hangcheck will fire. The check will
determine that A is moving, however ring B will appear hung because
the ACTHD doesn't move. I honestly can't say if that's actually a
realistic problem to hit it probably implies the timeout value is too
low."
v2: Make sure we don't even incur the KICK cost whilst waiting.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65394
Signed-off-by: Chris Wilson
Cc: Mika Kuoppala
Cc: Ben Widawsky
---
drivers/gpu/drm/i915/i915_irq.c | 121 +++
drivers/gpu/drm/i915/intel_ringbuffer.h |1 +
2 files changed, 90 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 32b2465..cf8584c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2331,21 +2331,21 @@ ring_idle(struct intel_ring_buffer *ring, u32 seqno)
i915_seqno_passed(seqno, ring_last_seqno(ring)));
}
-static bool semaphore_passed(struct intel_ring_buffer *ring)
+static struct intel_ring_buffer *
+semaphore_waits_for(struct intel_ring_buffer *ring, u32 *seqno)
{
struct drm_i915_private *dev_priv = ring->dev->dev_private;
- u32 acthd = intel_ring_get_active_head(ring) & HEAD_ADDR;
- struct intel_ring_buffer *signaller;
- u32 cmd, ipehr, acthd_min;
+ u32 cmd, ipehr, acthd, acthd_min;
ipehr = I915_READ(RING_IPEHR(ring->mmio_base));
if ((ipehr & ~(0x3 << 16)) !=
(MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE | MI_SEMAPHORE_REGISTER))
- return false;
+ return NULL;
/* ACTHD is likely pointing to the dword after the actual command,
* so scan backwards until we find the MBOX.
*/
+ acthd = intel_ring_get_active_head(ring) & HEAD_ADDR;
acthd_min = max((int)acthd - 3 * 4, 0);
do {
cmd = ioread32(ring->virtual_start + acthd);
@@ -2354,22 +2354,53 @@ static bool semaphore_passed(struct intel_ring_buffer
*ring)
acthd -= 4;
if (acthd < acthd_min)
- return false;
+ return NULL;
} while (1);
- signaller = &dev_priv->ring[(ring->id + (((ipehr >> 17) & 1) + 1)) % 3];
- return i915_seqno_passed(signaller->get_seqno(signaller, false),
-ioread32(ring->virtual_start+acthd+4)+1);
+ *seqno = ioread32(ring->virtual_start+acthd+4)+1;
+ return &dev_priv->ring[(ring->id + (((ipehr >> 17) & 1) + 1)) % 3];
+}
+
+static int semaphore_passed(struct intel_ring_buffer *ring)
+{
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+ struct intel_ring_buffer *signaller;
+ u32 seqno, ctl;
+
+ ring->hangcheck.deadlock = true;
+
+ signaller = semaphore_waits_for(ring, &seqno);
+ if (signaller == NULL || signaller->hangcheck.deadlock)
+ return -1;
+
+ /* cursory check for an unkickable deadlock */
+ ctl = I915_READ_CTL(signaller);
+ if (ctl & RING_WAIT_SEMAPHORE && semaphore_passed(signaller) < 0)
+ return -1;
+
+ return i915_seqno_passed(signaller->get_seqno(signaller, false), seqno);
+}
+
+static void semaphore_clear_deadlocks(struct drm_i915_private *dev_priv)
+{
+ struct intel_ring_buffer *ring;
+ int i;
+
+ for_each_ring(ring, dev_priv, i)
+ ring->hangcheck.deadlock = false;
}
-static bool ring_hung(struct intel_ring_buffer *ring)
+static enum { wait, active, kick, hung } ring_stuck(struct intel_ring_buffer
*ring, u32 acthd)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;
+ if (ring->hangcheck.acthd != acthd)
+ return active;
+
if (IS_GEN2(dev))
- return true;
+ return hung;
/* Is the chip hanging on a WAIT_FOR_EVENT?
* If so we can simply poke the RB_WAIT bit
@@ -2381,19 +2412,24 @@ static bool ring_hung(struct intel_ring_buffer *ring)
DRM_ERROR("Kicking stuck wait on %s\n",
ring->name);
I915_WRITE_CTL(ring, tmp);
- return false;
- }
-
- if (INTEL_INFO(dev)->gen >= 6 &&
- tmp & RING_WAIT_SEMAPHORE &&
- semaphore_passed(ring)) {
- DRM_ERROR("Kicking stuck semaphore on %s\n",
-
[Intel-gfx] [PATCH 2/4] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
After kicking a ring, it should be free to make progress again and so
should not be accused of being stuck until hangcheck fires once more. In
order to catch a denial-of-service within a batch or across multiple
batches, we still do increment the hangcheck score - just not as
severely so that it takes multiple kicks to fail.
This should address part of Ben's justified criticism of
commit 05407ff889ceebe383aa5907219f86582ef96b72
Author: Mika Kuoppala
Date: Thu May 30 09:04:29 2013 +0300
drm/i915: detect hang using per ring hangcheck_score
"There's also another corner case on the kick. If the seqno = 2
(though not stuck), and on the 3rd hangcheck, the ring is stuck, and
we try to kick it... we don't actually try to find out if the kick
helped."
v2: Make sure we catch DoS attempts with batches full of invalid WAITs.
v3: Preserve the ability to detect loops by always charging the ring
if it is busy on the same request.
v4: Make sure we queue another check if on a new batch
References: https://bugs.freedesktop.org/show_bug.cgi?id=65394
Signed-off-by: Chris Wilson
Cc: Mika Kuoppala
Cc: Ben Widawsky
---
drivers/gpu/drm/i915/i915_irq.c | 110 +--
1 file changed, 58 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index dcb5209..32b2465 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2324,21 +2324,11 @@ ring_last_seqno(struct intel_ring_buffer *ring)
struct drm_i915_gem_request, list)->seqno;
}
-static bool i915_hangcheck_ring_idle(struct intel_ring_buffer *ring,
-u32 ring_seqno, bool *err)
-{
- if (list_empty(&ring->request_list) ||
- i915_seqno_passed(ring_seqno, ring_last_seqno(ring))) {
- /* Issue a wake-up to catch stuck h/w. */
- if (waitqueue_active(&ring->irq_queue)) {
- DRM_ERROR("Hangcheck timer elapsed... %s idle\n",
- ring->name);
- wake_up_all(&ring->irq_queue);
- *err = true;
- }
- return true;
- }
- return false;
+static bool
+ring_idle(struct intel_ring_buffer *ring, u32 seqno)
+{
+ return (list_empty(&ring->request_list) ||
+ i915_seqno_passed(seqno, ring_last_seqno(ring)));
}
static bool semaphore_passed(struct intel_ring_buffer *ring)
@@ -2372,16 +2362,26 @@ static bool semaphore_passed(struct intel_ring_buffer
*ring)
ioread32(ring->virtual_start+acthd+4)+1);
}
-static bool kick_ring(struct intel_ring_buffer *ring)
+static bool ring_hung(struct intel_ring_buffer *ring)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- u32 tmp = I915_READ_CTL(ring);
+ u32 tmp;
+
+ if (IS_GEN2(dev))
+ return true;
+
+ /* Is the chip hanging on a WAIT_FOR_EVENT?
+* If so we can simply poke the RB_WAIT bit
+* and break the hang. This should work on
+* all but the second generation chipsets.
+*/
+ tmp = I915_READ_CTL(ring);
if (tmp & RING_WAIT) {
DRM_ERROR("Kicking stuck wait on %s\n",
ring->name);
I915_WRITE_CTL(ring, tmp);
- return true;
+ return false;
}
if (INTEL_INFO(dev)->gen >= 6 &&
@@ -2390,22 +2390,10 @@ static bool kick_ring(struct intel_ring_buffer *ring)
DRM_ERROR("Kicking stuck semaphore on %s\n",
ring->name);
I915_WRITE_CTL(ring, tmp);
- return true;
- }
- return false;
-}
-
-static bool i915_hangcheck_ring_hung(struct intel_ring_buffer *ring)
-{
- if (IS_GEN2(ring->dev))
return false;
+ }
- /* Is the chip hanging on a WAIT_FOR_EVENT?
-* If so we can simply poke the RB_WAIT bit
-* and break the hang. This should work on
-* all but the second generation chipsets.
-*/
- return !kick_ring(ring);
+ return true;
}
/**
@@ -2423,45 +2411,63 @@ void i915_hangcheck_elapsed(unsigned long data)
struct intel_ring_buffer *ring;
int i;
int busy_count = 0, rings_hung = 0;
- bool stuck[I915_NUM_RINGS];
+ bool stuck[I915_NUM_RINGS] = { 0 };
+#define BUSY 1
+#define KICK 5
+#define HUNG 20
+#define FIRE 30
if (!i915_enable_hangcheck)
return;
for_each_ring(ring, dev_priv, i) {
u32 seqno, acthd;
- bool idle, err = false;
+ bool busy = true;
seqno = ring->get_seqno(ring, false);
acthd = intel_ring_get_active_head(ring);
- idle = i915_hangcheck_ring_idle(ring, seqno, &err);
- stuck[i]
[Intel-gfx] [PATCH 4/4] drm/i915: Eliminate the addr/seqno from the hangcheck warning
This is of no value to the developer reading the report, let alone the
bamboozled user.
Signed-off-by: Chris Wilson
Acked-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_irq.c |8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index cf8584c..39730b6 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2525,12 +2525,10 @@ void i915_hangcheck_elapsed(unsigned long data)
for_each_ring(ring, dev_priv, i) {
if (ring->hangcheck.score > FIRE) {
- rings_hung++;
- DRM_ERROR("%s: %s on %s 0x%x\n", ring->name,
+ DRM_ERROR("%s on %s ring\n",
stuck[i] ? "stuck" : "no progress",
- stuck[i] ? "addr" : "seqno",
- stuck[i] ? ring->hangcheck.acthd & HEAD_ADDR :
- ring->hangcheck.seqno);
+ ring->name);
+ rings_hung++;
}
}
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: Initialize ring->hangcheck upon ring init
When we reset and restart a ring, we also want to clear any existing hangcheck. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_ringbuffer.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 1ef081c..a3cfa35 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -453,6 +453,8 @@ static int init_ring_common(struct intel_ring_buffer *ring) ring->last_retired_head = -1; } + memset(&ring->hangcheck, 0, sizeof(ring->hangcheck)); + out: if (HAS_FORCE_WAKE(dev)) gen6_gt_force_wake_put(dev_priv); -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/31] drm/i915: lock down pch pll accouting some more
On Fri, Jun 07, 2013 at 11:13:32PM +0200, Daniel Vetter wrote:
> On Fri, Jun 07, 2013 at 11:46:08PM +0300, Ville Syrjälä wrote:
> > On Fri, Jun 07, 2013 at 10:03:20PM +0200, Daniel Vetter wrote:
> > > On Fri, Jun 07, 2013 at 07:32:56PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Jun 05, 2013 at 01:34:05PM +0200, Daniel Vetter wrote:
> > > > > Before I start to make a complete mess out of this, crank up
> > > > > the paranoia level a bit.
> > > > >
> > > > > Signed-off-by: Daniel Vetter
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_display.c | 9 -
> > > > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > index 56fb6ed..39e977f 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -1440,6 +1440,7 @@ static void intel_disable_pch_pll(struct
> > > > > intel_crtc *intel_crtc)
> > > > > }
> > > > >
> > > > > assert_pch_pll_enabled(dev_priv, pll, NULL);
> > > > > + WARN_ON(!pll->on);
> > > > > if (--pll->active)
> > > > > return;
> > > >
> > > > Maybe a WARN_ON(pll->on) near the end of ironlake_enable_pch_pll() too?
> > >
> > > At the very end we set on = true, and the only non-error early return
> > > (when the active refcount is > 0 to begin with) has alreay a
> > > WARN_ON(!pll->on). Shouldn't that be good enough?
> >
> > Well I was just thinking that since we have this dual bookeeping w/
> > active and on, maybe we want to warn if things go out of sync.
>
> Now I'm confused. I've tried to explain why I think we already have full
> checking of pll->on in enable_shared_dpll ... Can you maybe show in a diff
> where you'd want to add more?
Something like this:
if (pll->active++) {
WARN_ON(!pll->on);
assert_pch_pll_enabled(dev_priv, pll, NULL);
return;
}
+ WARN_ON(pll->on);
and maybe also:
+ assert_pch_pll_disabled(dev_priv, pll, NULL);
Or maybe just kill 'pll->on' as it seems totally redundant.
Also maybe we could move most of the asserts and WARNs to some
central location. Currently there are quite a few early return paths
from the pll enable/disable functions, and I don't think we perform the
same checks for all of the branches. So maybe we could just have one
function that would cross-check pll->on, pll->active and the real hardware
state. We could call said function just before and after
enable/disable_pch_pll().
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: disable sdvo pixel multiplier cross-check for HAS_PCH_SPLIT
On Mon, Jun 10, 2013 at 09:25:40AM +0200, Daniel Vetter wrote: > We don't (yet) have proper pixel multiplier readout support on pch > split platforms, so the cross check will naturally fail. > > Reported-by: Chris Wilson > Cc: Chris Wilson > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_sdvo.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c > b/drivers/gpu/drm/i915/intel_sdvo.c > index 67710e1..cf9dc6d 100644 > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -1362,6 +1362,10 @@ static void intel_sdvo_get_config(struct intel_encoder > *encoder, > encoder_pixel_multiplier = 4; > break; > } > + > + if(HAS_PCH_SPLIT(dev)) > + return; /* no pixel mutlierplier readout support yet */ > + > WARN(encoder_pixel_multiplier != pipe_config->pixel_multiplier, >"SDVO pixel multiplier mismatch, port: %i, encoder: %i\n", >pipe_config->pixel_multiplier, encoder_pixel_multiplier); > -- > 1.7.11.7 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: prefer VBT modes for SVDO-LVDS over EDID
On Mon, Jun 10, 2013 at 09:10:51AM +0100, Chris Wilson wrote: > On Mon, Jun 10, 2013 at 09:47:58AM +0200, Daniel Vetter wrote: > > In > > > > commit 53d3b4d7778daf15900867336c85d3f8dd70600c > > Author: Egbert Eich > > Date: Tue Jun 4 17:13:21 2013 +0200 > > > > drm/i915/sdvo: Use &intel_sdvo->ddc instead of intel_sdvo->i2c for DDC > > > > Egbert Eich fixed a long-standing bug where we simply used a > > non-working i2c controller to read the EDID for SDVO-LVDS panels. > > Unfortunately some machines seem to not be able to cope with the mode > > provided in the EDID (specifically they seem to not be able to cope > > with a 4x pixel mutliplier instead of a 2x one). > > > > Since it took forever to notice the breakage it's fairly safe to > > assume that at least for SDVO-LVDS panels the VBT contains fairly sane > > data. So just switch around the order and use VBT modes first. > > > > v2: Also add EDID modes just in case, and spell Egbert correctly. > > > > Cc: Egbert Eich > > Cc: Chris Wilson > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65524 > > Cc: sta...@vger.kernel.org > > Signed-off-by: Daniel Vetter > Tested-by: Chris Wilson Picked up for -fixes, thanks for testing. -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 v2] drm: Print pretty names for pixel formats
From: Ville Syrjälä
Rather than just printing the pixel format as a hex number, decode the
fourcc into human readable form, and also decode the LE vs. BE flag.
Keep printing the raw hex number too in case it contains non-printable
characters.
Some examples what the new drm_get_format_name() produces:
DRM_FORMAT_XRGB: "XR24 little-endian (0x34325258)"
DRM_FORMAT_YUYV: "YUYV little-endian (0x56595559)"
DRM_FORMAT_RGB565|DRM_FORMAT_BIG_ENDIAN: "RG16 big-endian (0xb6314752)"
Unprintable characters: "D??? big-endian (0xff7f0244)"
v2: Fix patch author
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_crtc.c | 29 +++--
include/drm/drm_crtc.h | 1 +
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e7e9242..079996a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -29,6 +29,7 @@
* Dave Airlie
* Jesse Barnes
*/
+#include
#include
#include
#include
@@ -252,6 +253,28 @@ char *drm_get_connector_status_name(enum
drm_connector_status status)
}
EXPORT_SYMBOL(drm_get_connector_status_name);
+static char printable_char(int c)
+{
+ return isascii(c) && isprint(c) ? c : '?';
+}
+
+char *drm_get_format_name(uint32_t format)
+{
+ static char buf[32];
+
+ snprintf(buf, sizeof(buf),
+"%c%c%c%c %s-endian (0x%08x)",
+printable_char(format & 0xff),
+printable_char((format >> 8) & 0xff),
+printable_char((format >> 16) & 0xff),
+printable_char((format >> 24) & 0x7f),
+format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
+format);
+
+ return buf;
+}
+EXPORT_SYMBOL(drm_get_format_name);
+
/**
* drm_mode_object_get - allocate a new modeset identifier
* @dev: DRM device
@@ -1834,7 +1857,8 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
if (fb->pixel_format == plane->format_types[i])
break;
if (i == plane->format_count) {
- DRM_DEBUG_KMS("Invalid pixel format 0x%08x\n",
fb->pixel_format);
+ DRM_DEBUG_KMS("Invalid pixel format %s\n",
+ drm_get_format_name(fb->pixel_format));
ret = -EINVAL;
goto out;
}
@@ -2312,7 +2336,8 @@ static int framebuffer_check(const struct
drm_mode_fb_cmd2 *r)
ret = format_check(r);
if (ret) {
- DRM_DEBUG_KMS("bad framebuffer format 0x%08x\n",
r->pixel_format);
+ DRM_DEBUG_KMS("bad framebuffer format %s\n",
+ drm_get_format_name(r->pixel_format));
return ret;
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index adb3f9b..2cbbfd4 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1094,5 +1094,6 @@ extern int drm_format_num_planes(uint32_t format);
extern int drm_format_plane_cpp(uint32_t format, int plane);
extern int drm_format_horz_chroma_subsampling(uint32_t format);
extern int drm_format_vert_chroma_subsampling(uint32_t format);
+extern char *drm_get_format_name(uint32_t format);
#endif /* __DRM_CRTC_H__ */
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: prefer VBT modes for SVDO-LVDS over EDID
On Mon, Jun 10, 2013 at 09:47:58AM +0200, Daniel Vetter wrote: > In > > commit 53d3b4d7778daf15900867336c85d3f8dd70600c > Author: Egbert Eich > Date: Tue Jun 4 17:13:21 2013 +0200 > > drm/i915/sdvo: Use &intel_sdvo->ddc instead of intel_sdvo->i2c for DDC > > Egbert Eich fixed a long-standing bug where we simply used a > non-working i2c controller to read the EDID for SDVO-LVDS panels. > Unfortunately some machines seem to not be able to cope with the mode > provided in the EDID (specifically they seem to not be able to cope > with a 4x pixel mutliplier instead of a 2x one). > > Since it took forever to notice the breakage it's fairly safe to > assume that at least for SDVO-LVDS panels the VBT contains fairly sane > data. So just switch around the order and use VBT modes first. > > v2: Also add EDID modes just in case, and spell Egbert correctly. > > Cc: Egbert Eich > Cc: Chris Wilson > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65524 > Cc: sta...@vger.kernel.org > Signed-off-by: Daniel Vetter Tested-by: Chris Wilson -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/3] drm/i915: Remove dead code from SDVO initialisation
On Sun, Jun 09, 2013 at 04:02:05PM +0100, Chris Wilson wrote:
> The hotplug_mask is no longer used as the hpd interrupt setup is now
> handled in the core.
>
> Signed-off-by: Chris Wilson
First two patches merged to -fixes with the missing regression notes
added, last one merged to dinq. Also cc'ing Egbert now.
Thanks for tracking this down.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_sdvo.c | 13 -
> 1 file changed, 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index a939591..89ee792 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2852,7 +2852,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t
> sdvo_reg, bool is_sdvob)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_encoder *intel_encoder;
> struct intel_sdvo *intel_sdvo;
> - u32 hotplug_mask;
> int i;
> intel_sdvo = kzalloc(sizeof(struct intel_sdvo), GFP_KERNEL);
> if (!intel_sdvo)
> @@ -2881,18 +2880,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t
> sdvo_reg, bool is_sdvob)
> }
> }
>
> - hotplug_mask = 0;
> - if (IS_G4X(dev)) {
> - hotplug_mask = intel_sdvo->is_sdvob ?
> - SDVOB_HOTPLUG_INT_STATUS_G4X :
> SDVOC_HOTPLUG_INT_STATUS_G4X;
> - } else if (IS_GEN4(dev)) {
> - hotplug_mask = intel_sdvo->is_sdvob ?
> - SDVOB_HOTPLUG_INT_STATUS_I965 :
> SDVOC_HOTPLUG_INT_STATUS_I965;
> - } else {
> - hotplug_mask = intel_sdvo->is_sdvob ?
> - SDVOB_HOTPLUG_INT_STATUS_I915 :
> SDVOC_HOTPLUG_INT_STATUS_I915;
> - }
> -
> intel_encoder->compute_config = intel_sdvo_compute_config;
> intel_encoder->disable = intel_disable_sdvo;
> intel_encoder->mode_set = intel_sdvo_mode_set;
> --
> 1.7.10.4
>
> ___
> 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] [PATCH 1/2] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
After kicking a ring, it should be free to make progress again and so
should not be accused of being stuck until hangcheck fires once more. In
order to catch a denial-of-service within a batch or across multiple
batches, we still do increment the hangcheck score - just not as
severely so that it takes multiple kicks to fail.
This should address part of Ben's justified criticism of
commit 05407ff889ceebe383aa5907219f86582ef96b72
Author: Mika Kuoppala
Date: Thu May 30 09:04:29 2013 +0300
drm/i915: detect hang using per ring hangcheck_score
"There's also another corner case on the kick. If the seqno = 2
(though not stuck), and on the 3rd hangcheck, the ring is stuck, and
we try to kick it... we don't actually try to find out if the kick
helped."
v2: Make sure we catch DoS attempts with batches full of invalid WAITs.
v3: Preserve the ability to detect loops by always charging the ring
if it is busy on the same request.
References: https://bugs.freedesktop.org/show_bug.cgi?id=65394
Signed-off-by: Chris Wilson
Cc: Mika Kuoppala
Cc: Ben Widawsky
---
drivers/gpu/drm/i915/i915_irq.c | 109 ---
1 file changed, 57 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index dcb5209..ffa6305 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2324,21 +2324,11 @@ ring_last_seqno(struct intel_ring_buffer *ring)
struct drm_i915_gem_request, list)->seqno;
}
-static bool i915_hangcheck_ring_idle(struct intel_ring_buffer *ring,
-u32 ring_seqno, bool *err)
-{
- if (list_empty(&ring->request_list) ||
- i915_seqno_passed(ring_seqno, ring_last_seqno(ring))) {
- /* Issue a wake-up to catch stuck h/w. */
- if (waitqueue_active(&ring->irq_queue)) {
- DRM_ERROR("Hangcheck timer elapsed... %s idle\n",
- ring->name);
- wake_up_all(&ring->irq_queue);
- *err = true;
- }
- return true;
- }
- return false;
+static bool
+ring_idle(struct intel_ring_buffer *ring, u32 seqno)
+{
+ return (list_empty(&ring->request_list) ||
+ i915_seqno_passed(seqno, ring_last_seqno(ring)));
}
static bool semaphore_passed(struct intel_ring_buffer *ring)
@@ -2372,16 +2362,26 @@ static bool semaphore_passed(struct intel_ring_buffer
*ring)
ioread32(ring->virtual_start+acthd+4)+1);
}
-static bool kick_ring(struct intel_ring_buffer *ring)
+static bool ring_hung(struct intel_ring_buffer *ring)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- u32 tmp = I915_READ_CTL(ring);
+ u32 tmp;
+
+ if (IS_GEN2(dev))
+ return true;
+
+ /* Is the chip hanging on a WAIT_FOR_EVENT?
+* If so we can simply poke the RB_WAIT bit
+* and break the hang. This should work on
+* all but the second generation chipsets.
+*/
+ tmp = I915_READ_CTL(ring);
if (tmp & RING_WAIT) {
DRM_ERROR("Kicking stuck wait on %s\n",
ring->name);
I915_WRITE_CTL(ring, tmp);
- return true;
+ return false;
}
if (INTEL_INFO(dev)->gen >= 6 &&
@@ -2390,22 +2390,10 @@ static bool kick_ring(struct intel_ring_buffer *ring)
DRM_ERROR("Kicking stuck semaphore on %s\n",
ring->name);
I915_WRITE_CTL(ring, tmp);
- return true;
- }
- return false;
-}
-
-static bool i915_hangcheck_ring_hung(struct intel_ring_buffer *ring)
-{
- if (IS_GEN2(ring->dev))
return false;
+ }
- /* Is the chip hanging on a WAIT_FOR_EVENT?
-* If so we can simply poke the RB_WAIT bit
-* and break the hang. This should work on
-* all but the second generation chipsets.
-*/
- return !kick_ring(ring);
+ return true;
}
/**
@@ -2423,37 +2411,54 @@ void i915_hangcheck_elapsed(unsigned long data)
struct intel_ring_buffer *ring;
int i;
int busy_count = 0, rings_hung = 0;
- bool stuck[I915_NUM_RINGS];
+ bool stuck[I915_NUM_RINGS] = { 0 };
+#define BUSY 1
+#define KICK 5
+#define HUNG 20
+#define FIRE 30
if (!i915_enable_hangcheck)
return;
for_each_ring(ring, dev_priv, i) {
u32 seqno, acthd;
- bool idle, err = false;
seqno = ring->get_seqno(ring, false);
acthd = intel_ring_get_active_head(ring);
- idle = i915_hangcheck_ring_idle(ring, seqno, &err);
- stuck[i] = ring->hangcheck.acthd == acthd;
-
- if (idle) {
-
[Intel-gfx] [PATCH 2/2] drm/i915: Don't count semaphore waits towards a stuck ring
If we detect a ring is in a valid wait for another, just let it be.
Eventually it will either begin to progress again, or the entire system
will come grinding to a halt and then hangcheck will fire as soon as the
deadlock is detected.
This error was foretold by Ben in
commit 05407ff889ceebe383aa5907219f86582ef96b72
Author: Mika Kuoppala
Date: Thu May 30 09:04:29 2013 +0300
drm/i915: detect hang using per ring hangcheck_score
"If ring B is waiting on ring A via semaphore, and ring A is making
progress, albeit slowly - the hangcheck will fire. The check will
determine that A is moving, however ring B will appear hung because
the ACTHD doesn't move. I honestly can't say if that's actually a
realistic problem to hit it probably implies the timeout value is too
low."
v2: Make sure we don't even incur the KICK cost whilst waiting.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65394
Signed-off-by: Chris Wilson
Cc: Mika Kuoppala
Cc: Ben Widawsky
---
drivers/gpu/drm/i915/i915_irq.c | 121 +++
drivers/gpu/drm/i915/intel_ringbuffer.h |1 +
2 files changed, 90 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ffa6305..dc77a78 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2331,21 +2331,21 @@ ring_idle(struct intel_ring_buffer *ring, u32 seqno)
i915_seqno_passed(seqno, ring_last_seqno(ring)));
}
-static bool semaphore_passed(struct intel_ring_buffer *ring)
+static struct intel_ring_buffer *
+semaphore_waits_for(struct intel_ring_buffer *ring, u32 *seqno)
{
struct drm_i915_private *dev_priv = ring->dev->dev_private;
- u32 acthd = intel_ring_get_active_head(ring) & HEAD_ADDR;
- struct intel_ring_buffer *signaller;
- u32 cmd, ipehr, acthd_min;
+ u32 cmd, ipehr, acthd, acthd_min;
ipehr = I915_READ(RING_IPEHR(ring->mmio_base));
if ((ipehr & ~(0x3 << 16)) !=
(MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE | MI_SEMAPHORE_REGISTER))
- return false;
+ return NULL;
/* ACTHD is likely pointing to the dword after the actual command,
* so scan backwards until we find the MBOX.
*/
+ acthd = intel_ring_get_active_head(ring) & HEAD_ADDR;
acthd_min = max((int)acthd - 3 * 4, 0);
do {
cmd = ioread32(ring->virtual_start + acthd);
@@ -2354,22 +2354,53 @@ static bool semaphore_passed(struct intel_ring_buffer
*ring)
acthd -= 4;
if (acthd < acthd_min)
- return false;
+ return NULL;
} while (1);
- signaller = &dev_priv->ring[(ring->id + (((ipehr >> 17) & 1) + 1)) % 3];
- return i915_seqno_passed(signaller->get_seqno(signaller, false),
-ioread32(ring->virtual_start+acthd+4)+1);
+ *seqno = ioread32(ring->virtual_start+acthd+4)+1;
+ return &dev_priv->ring[(ring->id + (((ipehr >> 17) & 1) + 1)) % 3];
+}
+
+static int semaphore_passed(struct intel_ring_buffer *ring)
+{
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+ struct intel_ring_buffer *signaller;
+ u32 seqno, ctl;
+
+ ring->hangcheck.deadlock = true;
+
+ signaller = semaphore_waits_for(ring, &seqno);
+ if (signaller == NULL || signaller->hangcheck.deadlock)
+ return -1;
+
+ /* cursory check for an unkickable deadlock */
+ ctl = I915_READ_CTL(signaller);
+ if (ctl & RING_WAIT_SEMAPHORE && semaphore_passed(signaller) < 0)
+ return -1;
+
+ return i915_seqno_passed(signaller->get_seqno(signaller, false), seqno);
+}
+
+static void semaphore_clear_deadlocks(struct drm_i915_private *dev_priv)
+{
+ struct intel_ring_buffer *ring;
+ int i;
+
+ for_each_ring(ring, dev_priv, i)
+ ring->hangcheck.deadlock = false;
}
-static bool ring_hung(struct intel_ring_buffer *ring)
+static enum { wait, busy, kick, hung } ring_stuck(struct intel_ring_buffer
*ring, u32 acthd)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;
+ if (ring->hangcheck.acthd != acthd)
+ return busy;
+
if (IS_GEN2(dev))
- return true;
+ return hung;
/* Is the chip hanging on a WAIT_FOR_EVENT?
* If so we can simply poke the RB_WAIT bit
@@ -2381,19 +2412,24 @@ static bool ring_hung(struct intel_ring_buffer *ring)
DRM_ERROR("Kicking stuck wait on %s\n",
ring->name);
I915_WRITE_CTL(ring, tmp);
- return false;
- }
-
- if (INTEL_INFO(dev)->gen >= 6 &&
- tmp & RING_WAIT_SEMAPHORE &&
- semaphore_passed(ring)) {
- DRM_ERROR("Kicking stuck semaphore on %s\n",
-
[Intel-gfx] [PATCH] drm/i915: prefer VBT modes for SVDO-LVDS over EDID
In
commit 53d3b4d7778daf15900867336c85d3f8dd70600c
Author: Egbert Eich
Date: Tue Jun 4 17:13:21 2013 +0200
drm/i915/sdvo: Use &intel_sdvo->ddc instead of intel_sdvo->i2c for DDC
Egbert Eich fixed a long-standing bug where we simply used a
non-working i2c controller to read the EDID for SDVO-LVDS panels.
Unfortunately some machines seem to not be able to cope with the mode
provided in the EDID (specifically they seem to not be able to cope
with a 4x pixel mutliplier instead of a 2x one).
Since it took forever to notice the breakage it's fairly safe to
assume that at least for SDVO-LVDS panels the VBT contains fairly sane
data. So just switch around the order and use VBT modes first.
v2: Also add EDID modes just in case, and spell Egbert correctly.
Cc: Egbert Eich
Cc: Chris Wilson
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65524
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_sdvo.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo.c
index 4c47b44..2a449d1 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1777,10 +1777,13 @@ static void intel_sdvo_get_lvds_modes(struct
drm_connector *connector)
* arranged in priority order.
*/
intel_ddc_get_modes(connector, &intel_sdvo->ddc);
- if (list_empty(&connector->probed_modes) == false)
- goto end;
- /* Fetch modes from VBT */
+ /*
+* Fetch modes from VBT. For SDVO prefer the VBT mode since some
+* SDVO->LVDS transcoders can't cope with the EDID mode. Since
+* drm_mode_probed_add adds the mode at the head of the list we add it
+* last.
+*/
if (dev_priv->sdvo_lvds_vbt_mode != NULL) {
newmode = drm_mode_duplicate(connector->dev,
dev_priv->sdvo_lvds_vbt_mode);
@@ -1792,7 +1795,6 @@ static void intel_sdvo_get_lvds_modes(struct
drm_connector *connector)
}
}
-end:
list_for_each_entry(newmode, &connector->probed_modes, head) {
if (newmode->type & DRM_MODE_TYPE_PREFERRED) {
intel_sdvo->sdvo_lvds_fixed_mode =
--
1.7.10.4
___
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 Sun, Jun 09, 2013 at 07:01:39PM -0400, 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 Make sense and I guess it's easier to merge this all through the acpi tree. So Acked-by: Daniel Vetter > --- > 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(); > } > > if (IS_GEN5(dev)) > -- > 1.8.2.1 > -- 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: disable sdvo pixel multiplier cross-check for HAS_PCH_SPLIT
We don't (yet) have proper pixel multiplier readout support on pch split platforms, so the cross check will naturally fail. Reported-by: Chris Wilson Cc: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_sdvo.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 67710e1..cf9dc6d 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1362,6 +1362,10 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder, encoder_pixel_multiplier = 4; break; } + + if(HAS_PCH_SPLIT(dev)) + return; /* no pixel mutlierplier readout support yet */ + WARN(encoder_pixel_multiplier != pipe_config->pixel_multiplier, "SDVO pixel multiplier mismatch, port: %i, encoder: %i\n", pipe_config->pixel_multiplier, encoder_pixel_multiplier); -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: prefer VBT modes for SVDO-LVDS over EDID
Chris Wilson writes: > On Sun, Jun 09, 2013 at 11:28:12PM +0200, Daniel Vetter wrote: > > On Sun, Jun 9, 2013 at 11:16 PM, Chris Wilson > > wrote: > > > On Sun, Jun 09, 2013 at 10:58:38PM +0200, Daniel Vetter wrote: > > >> In > > >> > > >> commit 53d3b4d7778daf15900867336c85d3f8dd70600c > > >> Author: Egbert Eich > > >> Date: Tue Jun 4 17:13:21 2013 +0200 > > >> > > >> drm/i915/sdvo: Use &intel_sdvo->ddc instead of intel_sdvo->i2c for > > >> DDC > > >> > > >> Ebgert Eich fixed a long-standing bug where we simply used a > > >> non-working i2c controller to read the EDID for SDVO-LVDS panels. > > >> Unfortunately some machines seem to not be able to cope with the mode > > >> provided in the EDID (specifically they seem to not be able to cope > > >> with a 4x pixel mutliplier instead of a 2x one). > > >> > > >> Since it took forever to notice the breakage it's fairly safe to > > >> assume that at least for SDVO-LVDS panels the VBT contains fairly sane > > >> data. So just switch around the order and use VBT modes first. > > >> > > >> Cc: Egbert Eich > > >> Cc: Chris Wilson > > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65524 > > >> Cc: sta...@vger.kernel.org > > >> Signed-off-by: Daniel Vetter > > > > > > I can accept the argument that we need to prefer the VBT mode here to > > > paper over the apparent regression, but to not pass on the full EDID > > > modes seems dubious. > > > > I'm not sure there's any value in additional modes. We can't really > > support frequency switching over sdvo (it'd very likely require a > > mutliplier change) and for everything else we only ever let the fixed > > mode through the fixup hook. > > I am trying not to generalise from the broken behaviour on this machine. > On another machine, there may be some value in the extra modes. > Select the sanest default we can, then let the user go nuts with the > extra information. While I'm not a fan of setting non-native modes on panels this seems to be a requirement in some environments - not sure if there are any real world use cases but at least many QA test scenarios seem to include such modes. So adding the EDID modes (unflagging the preferred bit!) would be what I'd opt for - admittedly for a very selfish reason: it will spare me of lengthy, pointless discussions with some partners' QA departments next time we update the Intel driver. Once again we go out of our ways to accomodate the most broken devices by imposing more limitations on all others as well. At one point we may even have to give in face the grim reality and start blacklisting some of the broken crap. And since we are so much into bikeshedding here - maybe you could fix my name in the comment ;) Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
