[Intel-gfx] [PATCH] Fix compute error while enable self-refresh
mask & (mask - 1) == 0 to make sure we have only one bit set.
use fls instead of ffs to find the right pipe.
Signed-off-by: Yuanhan Liu
---
drivers/gpu/drm/i915/intel_display.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 3106c0d..dc52dc1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3885,7 +3885,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
static inline bool single_plane_enabled(unsigned int mask)
{
- return mask && (mask & -mask) == 0;
+ return mask && (mask & (mask - 1)) == 0;
}
static void g4x_update_wm(struct drm_device *dev)
@@ -3910,7 +3910,7 @@ static void g4x_update_wm(struct drm_device *dev)
plane_sr = cursor_sr = 0;
if (single_plane_enabled(enabled) &&
- g4x_compute_srwm(dev, ffs(enabled) - 1,
+ g4x_compute_srwm(dev, fls(enabled) - 1,
sr_latency_ns,
&g4x_wm_info,
&g4x_cursor_wm_info,
@@ -4335,7 +4335,7 @@ static void ironlake_update_wm(struct drm_device *dev)
if (!single_plane_enabled(enabled))
return;
- enabled = ffs(enabled) - 1;
+ enabled = fls(enabled) - 1;
/* WM1 */
if (!ironlake_compute_srwm(dev, 1, enabled,
@@ -4421,7 +4421,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
if (!single_plane_enabled(enabled))
return;
- enabled = ffs(enabled) - 1;
+ enabled = fls(enabled) - 1;
/* WM1 */
if (!ironlake_compute_srwm(dev, 1, enabled,
--
1.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] intel-gen4asm: have a C-like binary output
On Thu, Mar 17, 2011 at 07:29:03PM -0700, Ben Widawsky wrote:
> From: Ben Widawsky
>
> Have the assembler support creating a byte array for binary blob-like
> inclusion. In my case, to write some exception handler which is not
> jit'd.
I just noticed that the spacing is a little messed up in this one. If someone
wants to push this, but doesn't want to fix it, send me an email and I'll
resubmit.
Here's sample output. It should be shorter lines, with proper spacing...
static const char gen4_bytes[] = {
0x05, 0x02, 0x00, 0x00,0x00, 0x1c, 0x00, 0x30,0x00, 0x10, 0x00,
0x00,0xf9, 0xff, 0xff, 0xff,
0x08, 0x02, 0x00, 0x00,0x1d, 0x1d, 0x04, 0x20,0x00, 0x0e, 0x00,
0x00,0x06, 0x00, 0x00, 0x00,
0x40, 0x02, 0x00, 0x00,0xbd, 0x43, 0x04, 0x20,0x04, 0x00, 0x00,
0x00,0x00, 0x0e, 0x00, 0x00,
0x41, 0x02, 0x00, 0x00,0xbd, 0x1f, 0x04, 0x20,0x04, 0x00, 0x00,
0x00,0x00, 0x10, 0x00, 0x00,
0x01, 0x02, 0x60, 0x00,0xbe, 0x03, 0x00, 0x20,0x00, 0x00, 0x00,
0x00,0x00, 0x00, 0x00, 0x00,
0x01, 0x22, 0x80, 0x00,0xbe, 0x03, 0x00, 0x20,0x00, 0x00, 0x00,
0x00,0x00, 0x00, 0x00, 0x00,
0x40, 0x02, 0x00, 0x00,0xbe, 0x1f, 0x02, 0x20,0x04, 0x00, 0x00,
0x00,0x00, 0x00, 0x00, 0x00,
0x31, 0x00, 0x60, 0x00,0x1e, 0x1c, 0x00, 0x20,0x00, 0x00, 0x00,
0x50,0x00, 0x00, 0x18, 0x10,
0x01, 0x00, 0x60, 0x00,0xbe, 0x03, 0xe0, 0x20,0x00, 0x00, 0x00,
0x00,0x00, 0x00, 0x00, 0x00,
0x40, 0x02, 0x00, 0x00,0xbe, 0x1f, 0xe2, 0x20,0x04, 0x00, 0x00,
0x00,0x00, 0x01, 0x00, 0x00,
0x31, 0x00, 0x60, 0x00,0x1e, 0x1c, 0xe0, 0x20,0x00, 0x00, 0x00,
0x50,0x00, 0x00, 0x18, 0x12,
};
Ben
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] intel-gen4asm: have a C-like binary output
From: Ben Widawsky
Have the assembler support creating a byte array for binary blob-like
inclusion. In my case, to write some exception handler which is not
jit'd.
I don't have push access, so if anyone thinks this is useful, please
push
Signed-off-by: Ben Widawsky
---
src/main.c | 53 +++--
1 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/src/main.c b/src/main.c
index def2655..785cce8 100644
--- a/src/main.c
+++ b/src/main.c
@@ -40,10 +40,13 @@ extern int errors;
long int gen_level = 4;
int advanced_flag = 0; /* 0: in unit of type, 1: in unit of data element size
*/
+int binary_like_output = 0; /* 0: default type, 1: nice c like output */
int need_export = 0;
char *input_filename = "";
char *export_filename = NULL;
+const char const *binary_prepend = "static const char gen4_bytes[] = {\n";
+
struct brw_program compiled_program;
struct program_defaults program_defaults;
@@ -53,6 +56,7 @@ static struct declared_register
*declared_register_table[HASHSZ];
static const struct option longopts[] = {
{"advanced", no_argument, 0, 'a'},
+ {"binary", no_argument, 0, 'b'},
{"export", required_argument, 0, 'e'},
{"input_list", required_argument, 0, 'l'},
{"output", required_argument, 0, 'o'},
@@ -65,6 +69,7 @@ static void usage(void)
fprintf(stderr, "usage: intel-gen4asm [options] inputfile\n");
fprintf(stderr, "OPTIONS:\n");
fprintf(stderr, "\t-a, --advanced Set advanced
flag\n");
+ fprintf(stderr, "\t-b, --binary C style binary
output\n");
fprintf(stderr, "\t-e, --export {exportfile}Export label
file\n");
fprintf(stderr, "\t-l, --input_list {entrytablefile}Input
entry_table_list file\n");
fprintf(stderr, "\t-o, --output {outputfile}Specify output
file\n");
@@ -168,6 +173,38 @@ static int is_entry_point(char *s)
return 0;
}
+static void
+print_instruction(FILE *output, struct brw_program_instruction *entry)
+{
+ if (binary_like_output) {
+ fprintf(output, "\t0x%02x, 0x%02x, 0x%02x, 0x%02x,"
+ "0x%02x, 0x%02x, 0x%02x, 0x%02x,"
+ "0x%02x, 0x%02x, 0x%02x, 0x%02x,"
+ "0x%02x, 0x%02x, 0x%02x, 0x%02x,\n",
+ ((unsigned char *)(&entry->instruction))[0],
+ ((unsigned char *)(&entry->instruction))[1],
+ ((unsigned char *)(&entry->instruction))[2],
+ ((unsigned char *)(&entry->instruction))[3],
+ ((unsigned char *)(&entry->instruction))[4],
+ ((unsigned char *)(&entry->instruction))[5],
+ ((unsigned char *)(&entry->instruction))[6],
+ ((unsigned char *)(&entry->instruction))[7],
+ ((unsigned char *)(&entry->instruction))[8],
+ ((unsigned char *)(&entry->instruction))[9],
+ ((unsigned char *)(&entry->instruction))[10],
+ ((unsigned char *)(&entry->instruction))[11],
+ ((unsigned char *)(&entry->instruction))[12],
+ ((unsigned char *)(&entry->instruction))[13],
+ ((unsigned char *)(&entry->instruction))[14],
+ ((unsigned char *)(&entry->instruction))[15]);
+ } else {
+ fprintf(output, " { 0x%08x, 0x%08x, 0x%08x, 0x%08x },\n",
+ ((int *)(&entry->instruction))[0],
+ ((int *)(&entry->instruction))[1],
+ ((int *)(&entry->instruction))[2],
+ ((int *)(&entry->instruction))[3]);
+ }
+}
int main(int argc, char **argv)
{
char *output_file = NULL;
@@ -177,7 +214,7 @@ int main(int argc, char **argv)
struct brw_program_instruction *entry, *entry1, *tmp_entry;
int err, inst_offset;
char o;
- while ((o = getopt_long(argc, argv, "e:l:o:g:a", longopts, NULL)) !=
-1) {
+ while ((o = getopt_long(argc, argv, "e:l:o:g:ab", longopts, NULL)) !=
-1) {
switch (o) {
case 'o':
if (strcmp(optarg, "-") != 0)
@@ -198,6 +235,9 @@ int main(int argc, char **argv)
case 'a':
advanced_flag = 1;
break;
+ case 'b':
+ binary_like_output = 1;
+ break;
case 'e':
need_export = 1;
@@ -317,20 +357,21 @@ int main(int argc, char **argv)
}
+ if (binary_like_output)
+ fprintf(output, "%s", binary_prepend);
+
for (entry = compiled_program.first;
entry != NULL;
entry = entry1) {
entry1 = entry->next;
Re: [Intel-gfx] Oops at i915_gem_retire_requests_ring
On Thu, 17 Mar 2011 13:54:45 -0300, Herton Ronaldo Krzesinski wrote: > On Thu, Mar 17, 2011 at 01:46:34PM +, Chris Wilson wrote: > > This is the single chunk required. I had thought that the actual > > insertion/deletion was serialised under the struct mutex and the intention > > of the spinlock was to protect the unlocked list traversal during > > throttling. However, I missed that i915_gem_release() is also called > > without struct mutex and so we do need the double check for > > i915_gem_request_remove_from_client(). > > Ok. I just still have one doubt though, if in i915_add_request > file/file_priv is NULL, wouldn't be possible to have an oops also in > i915_gem_release without the check? As in this case, > request->client_list wouldn't have mm.request_list added to it, and if > an error occurs and i915_reset is called, which ends up calling > i915_gem_release, we would try to do a list_del on request->client_list > without items. If the file_priv is NULL, then the request is not added to the client mm.request_list and so it is not seen during i915_gem_release. The list is file_priv->mm.request_list, the nodes within that are request->client_list. > If the check really isn't needed in i915_gem_release, then please > consider this patch: Done, thanks, -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] slow graphics after Suspend to RAM
* ch...@chris-wilson.co.uk (Mar 17, 2011 @ 07:51+): > On Wed, 16 Mar 2011 17:59:20 -0600, John Harrigan > wrote: > > I have a Lenovo X201 laptop with an i7-620M processor. After I resume > > from a Suspend to RAM the graphics are very slow. Suspend to Disk does > > not cause the same problem and doing a Suspend to Disk after a Suspend > > to RAM fixes the problem. > > Known problem. The PAT lose the WC bits for the GTT aperture on resume. > There is a workaround that we can do: recreate the ioremapping upon > resume. (But the root cause is not a gfx driver bug.) Thanks. Is there an existing bug report somewhere where I should supply logs and debug info? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] [drm/i915] - LVDS mode setting fixes
Having now looked at that post (sorry I wasn't even aware of this list until a few days ago), I agree. I think you were getting had by the same issues, previously fixed in the old userspace driver 3 years ago. I also find this very interesting: http://lists.freedesktop.org/archives/intel-gfx/2011-February/009378.html However it only deals with video timing of the LVDS output. The patches I just posted also allow override of LVDS pixel format and channel setup, aspects unique to LVDS. -Mike On Thu, 17 Mar 2011, Oliver Seitz wrote: > > > I'd very much appreciate it if these could please be merged into > > drm/i915. The changes are not complex and should be easy to follow. > > I have the feeling that these patches may also help on this topic: > http://lists.freedesktop.org/archives/intel-gfx/2011-February/009376.html > Chris Wilson supplied a patch on that one, but I was not able to test it due > to me lacking skills. As you wrote some documentation about using your > patches, I may be able to try *them*. It won't be before next week, though. > > Greets, > Kiste > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] [drm/i915] - LVDS mode setting fixes
On Thu, 17 Mar 2011, Chris Wilson wrote: > On Thu, 17 Mar 2011 08:57:43 -0500 (CDT), Mike Isely wrote: > > This patch series (3 of them) basically implement the same fixes as was > > previously done for the userspace driver back in 2008. The fixes are > > not a direct port; I coded the changes and obviously tested again. > > This is also why there are 3 patches not 2; the third one is a fix for > > another problem uncovered while debugging the fixed mode change. > > Mike, thanks for the patches! > > I think before we proceed, the question I want to ask is whether it is > preferable to add one super LVDS option (i.e. to parse a parameter string > ala intelfb or video=) which we can use to program the panel configuration > data (channels, bit depth, fixed mode, etc) or to add them ad hoc as > individual parameters? I'd actually prefer a single "super LVDS" option, but I think we need this as the first step. We could build a little parser for that option, maybe even make possible direct specification of the LVDS fixed mode. I did look into making possible direct specification of the fixed mode, but I'm really not fluent in the DRM "landscape" and after spending a day studying the DRM's video mode parser I felt it was less risky in the short term to just have a way to disable the scaling, same as what I had down for the user space driver back in 2008. I'd be happy to help build a better overall solution here with a single "super LVDS" option. But I don't want to step on any effort there that might already be underway. In the mean time we needed *something* here... And these patches really are pretty simple and low risk. If the desire is to go to a single LVDS option to cover everything, then let's please get that implemented. If not, then let's merge these patches that already exist and work. What I don't want to see happen is nothing done at all... > > One concern I have is the one Dave raised: we must be careful when > allowing the user to override panel configuration. The ultimate question > being do we trust the hardware configuration data more than the user > cut'n'pasting from a random forum? > -Chris I agree one must be careful, but I think it's also very important that the user, with suitable warnings in the documentation (where would this be documented?), be able to override anything here. If the decision is made to trust the hardware over the user, then I'm dead here because the i915 module right now is assuming that the video BIOS did things right, which is simply impossible when the LVDS display device is not integral to the processor board where the video BIOS resides. That entire issue after all is what drove these patches in the first place. IMHO, the user should always be able to override detected settings, especially when it's known that the detection can get things wrong. These fixes were merged into the userspace driver in 2008; things seemed to work out well there... Being able to detect as much as possible is always a good thing. But that's different than not providing any means of override. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] [drm/i915] - LVDS mode setting fixes
On Thu, 17 Mar 2011 08:57:43 -0500 (CDT), Mike Isely wrote: > This patch series (3 of them) basically implement the same fixes as was > previously done for the userspace driver back in 2008. The fixes are > not a direct port; I coded the changes and obviously tested again. > This is also why there are 3 patches not 2; the third one is a fix for > another problem uncovered while debugging the fixed mode change. Mike, thanks for the patches! I think before we proceed, the question I want to ask is whether it is preferable to add one super LVDS option (i.e. to parse a parameter string ala intelfb or video=) which we can use to program the panel configuration data (channels, bit depth, fixed mode, etc) or to add them ad hoc as individual parameters? One concern I have is the one Dave raised: we must be careful when allowing the user to override panel configuration. The ultimate question being do we trust the hardware configuration data more than the user cut'n'pasting from a random forum? -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 0/3] [drm/i915] - LVDS mode setting fixes
I'd very much appreciate it if these could please be merged into drm/i915. The changes are not complex and should be easy to follow. I have the feeling that these patches may also help on this topic: http://lists.freedesktop.org/archives/intel-gfx/2011-February/009376.html Chris Wilson supplied a patch on that one, but I was not able to test it due to me lacking skills. As you wrote some documentation about using your patches, I may be able to try *them*. It won't be before next week, though. Greets, Kiste ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] [drm/i915] - Implement manual override of LVDS single/dual channel mode
The logic for LVDS setup in the Intel driver needs to know whether the
LVDS port should be in single or dual channel mode when calculating
video timing. It had been answering this question by probing the
current hardware configuration, under the assumption that the video
BIOS would have already set it up. But the video BIOS would actually
have to know how to set up the LVDS port for the connected device,
which is a bad assumption if the display device is not integral to the
processor board - a situation that can exist for embedded situation.
This is yet one more case where the Intel driver had been implicitly
relying on the video BIOS for display configuration.
This changes creates a new kernel option, lvds_channels, which can be
used to override the probe. Setting this allows the user to specify
the number of channels in use, avoiding the possibly erroneous probe
of the hardware. Almost nobody should ever need to touch this option,
and its default value of zero is interpreted to preserve existing
probe-the-hardware behavior. But if the video BIOS gets this "wrong",
then it can be overridden by setting lvds_channels to 1 or 2,
indicating single or dual channel LVDS mode, respectively.
---
drivers/gpu/drm/i915/i915_drv.c |4
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/intel_display.c | 34 +-
3 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index
004880aa3a948669b8b4e23d9ad73d132cff81d0..1d88f059a27321ecb681e2b7927bb69029fcb49a
100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -49,6 +49,10 @@ module_param_named(powersave, i915_powersave, int, 0600);
unsigned int i915_lvds_fixed = 1;
module_param_named(lvds_fixed, i915_lvds_fixed, int, 0600);
+unsigned int i915_lvds_channels = 0;
+module_param_named(lvds_channels, i915_lvds_channels, int, 0600);
+MODULE_PARM_DESC(lvds_channels, "LVDS channels in use: 0=(default) probe
hardware 1=single 2=dual");
+
unsigned int i915_lvds_downclock = 0;
module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index
3fa8681459aa596e12e885568e5b48f0c9a60719..a6aab43e5f39f2d5b92a69a284bf8f72a254ea7c
100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -886,6 +886,7 @@ extern int i915_max_ioctl;
extern unsigned int i915_fbpercrtc;
extern unsigned int i915_powersave;
extern unsigned int i915_lvds_fixed;
+extern unsigned int i915_lvds_channels;
extern unsigned int i915_lvds_downclock;
extern unsigned int i915_lvds_24bit;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index
09f57f29c30c371c213944be473090a780a287db..4dc91400edd8935be45a229cf91292339bca0ce8
100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -642,6 +642,20 @@ static const intel_limit_t
intel_limits_ironlake_display_port = {
.find_pll = intel_find_pll_ironlake_dp,
};
+static int intel_is_dual_channel_mode(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ if (i915_lvds_channels) {
+ /* User has specified desired channel mode */
+ return (i915_lvds_channels == 2);
+ }
+
+ /* User has not specified mode so let's see
+ what the hardware is doing. */
+ return ((I915_READ(LVDS) & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP);
+}
+
static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -653,8 +667,7 @@ static const intel_limit_t *intel_ironlake_limit(struct
drm_crtc *crtc)
if (dev_priv->lvds_use_ssc && dev_priv->lvds_ssc_freq == 100)
refclk = 100;
- if ((I915_READ(PCH_LVDS) & LVDS_CLKB_POWER_MASK) ==
- LVDS_CLKB_POWER_UP) {
+ if (intel_is_dual_channel_mode(crtc)) {
/* LVDS dual channel */
if (refclk == 100)
limit = &intel_limits_ironlake_dual_lvds_100m;
@@ -677,18 +690,16 @@ static const intel_limit_t *intel_ironlake_limit(struct
drm_crtc *crtc)
static const intel_limit_t *intel_g4x_limit(struct drm_crtc *crtc)
{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
const intel_limit_t *limit;
if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
- if ((I915_READ(LVDS) & LVDS_CLKB_POWER_MASK) ==
- LVDS_CLKB_POWER_UP)
+ if (intel_is_dual_channel_mode(crtc)) {
/* LVDS with dual channel */
limit = &intel_limits_g4x_dual_channel_lvds;
- else
-
[Intel-gfx] [PATCH 0/3] [drm/i915] - LVDS mode setting fixes
Back in 2008, I generated these bug reports: https://bugs.freedesktop.org/show_bug.cgi?id=15200 https://bugs.freedesktop.org/show_bug.cgi?id=15201 These were entered due to a project I've been involved in where we pair up a special purpose display device with unusual video timings, via LVDS, to a commercial x86 single board computer with an Intel GPU. The SBC's video BIOS can't have any implicit knowledge of the nature of the otherwise unrelated display device, and since the xorg driver had been trying to probe the hardware / use the video BIOS, this put our display into an unusable state. I later submitted patches to fix these issues and they were merged into the userspace xorg Intel driver. These fixes allowed for some additional LVDS manual configuration and made it possible to directly set the LVDS output video timings via a normal xorg modeline. Fast forward to 2011 and this same project, due to the SBC being EOL'ed, is updating to later hardware with a more modern Intel GPU (GMA-4500), which then forced an update to a later Linux kernel, later X server, dri2, and most significantly kernel mode setting. With KMS being used now for Intel GPUs, the previous mode setting logic in the userspace xorg Intel driver had since been removed, and along with it the fixes I had made. The KMS implementation must have forked from the userspace code before I had submitted those patches because the same problems are in fact happening there and the corresponding fixes never made it into the drm/i915 module. This patch series (3 of them) basically implement the same fixes as was previously done for the userspace driver back in 2008. The fixes are not a direct port; I coded the changes and obviously tested again. This is also why there are 3 patches not 2; the third one is a fix for another problem uncovered while debugging the fixed mode change. I had also opened a new bug report for the 24 bit LVDS issue. Patch 1/3 in this series is the same as the patch posted to that bug report, found here: https://bugs.freedesktop.org/show_bug.cgi?id=34682 Note that these patches all have default behavior that preserves what the i915 module already does. So there's no impact to users until the relevant kernel module option is tweaked to enable the new behavior implemented by the patches. (This is just like what I did in 2008 except now it's a module otion instead of an xorg driver option.) I'd very much appreciate it if these could please be merged into drm/i915. The changes are not complex and should be easy to follow. Thank you, Signed-off-by: Mike Isely -Mike Isely is...@pobox.com (or is...@isely.net) -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] [drm/i915] - Implement ability to disabled LVDS fixed mode
An LVDS connected display device typically has a "fixed" mode to
which its video timings are locked, and that mode is not controllable
by the user - user-specified timings are always ignored. If the fixed
mode's expected resolution does not match the user-specified frame
buffer's resolution, then the port is programmed to scale the image
(this is how multiple resolutions are handled on an LCD panel and it's
why you get fuzzy LCD resolutions when the chosen resolution doesn't
match the native resolution).
Unfortunately this setup makes it impossible for the user to control
the display's video timings. This new option implements a new kernel
option, lvds_fixed, which defaults to true. But when it is set to
false (zero), fixed mode is disabled; all scaling is gone and the
driver will then use the user-specified video timings. Obviously the
user has to get the timings correct, but this is really not much
different than the bad old days of CRT video modes.
And while we normally don't want to disable fixed mode, the fact of
the matter is that sometimes there's no choice if the driver otherwise
gets the fixed mode "wrong". This can happen in some embedded systems
where the video BIOS really has no knowledge of the display device.
Warning: With fixed mode disabled, and if there is no other mode data
supplied anywhere (e.g. KMS trying to run a framebuffer for fbcon on
an LVDS console), then the LVDS port will likely simply be disabled.
However that's easily remedied by supplying video mode information on
the kernel command line, for example "video=LVDS-1:800x600-24MR@100"
for a 100Hz 800x600 24bpp resolution.
Signed-off-by: Mike Isely
---
drivers/gpu/drm/i915/i915_drv.c |3 +++
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/intel_lvds.c | 31 ++-
3 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index
c89f71c251acf230db613229eca90d24584b9729..004880aa3a948669b8b4e23d9ad73d132cff81d0
100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -46,6 +46,9 @@ module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);
unsigned int i915_powersave = 1;
module_param_named(powersave, i915_powersave, int, 0600);
+unsigned int i915_lvds_fixed = 1;
+module_param_named(lvds_fixed, i915_lvds_fixed, int, 0600);
+
unsigned int i915_lvds_downclock = 0;
module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index
526cef2972ab9afce1c17f57ef39ce9cc4dc736f..3fa8681459aa596e12e885568e5b48f0c9a60719
100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -885,6 +885,7 @@ extern struct drm_ioctl_desc i915_ioctls[];
extern int i915_max_ioctl;
extern unsigned int i915_fbpercrtc;
extern unsigned int i915_powersave;
+extern unsigned int i915_lvds_fixed;
extern unsigned int i915_lvds_downclock;
extern unsigned int i915_lvds_24bit;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c
b/drivers/gpu/drm/i915/intel_lvds.c
index
fe779b33899af536eb2e76429c9b05c363ce7721..303d60f71ca6dcf4ce7b59fe625ed5377af24bc4
100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -157,10 +157,12 @@ static int intel_lvds_mode_valid(struct drm_connector
*connector,
struct intel_lvds *intel_lvds = intel_attached_lvds(connector);
struct drm_display_mode *fixed_mode = intel_lvds->fixed_mode;
- if (mode->hdisplay > fixed_mode->hdisplay)
- return MODE_PANEL;
- if (mode->vdisplay > fixed_mode->vdisplay)
- return MODE_PANEL;
+ if (fixed_mode) {
+ if (mode->hdisplay > fixed_mode->hdisplay)
+ return MODE_PANEL;
+ if (mode->vdisplay > fixed_mode->vdisplay)
+ return MODE_PANEL;
+ }
return MODE_OK;
}
@@ -253,7 +255,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder
*encoder,
* with the panel scaling set up to source from the H/VDisplay
* of the original mode.
*/
- intel_fixed_panel_mode(intel_lvds->fixed_mode, adjusted_mode);
+ if (intel_lvds->fixed_mode)
+ intel_fixed_panel_mode(intel_lvds->fixed_mode, adjusted_mode);
if (HAS_PCH_SPLIT(dev)) {
intel_pch_panel_fitting(dev, intel_lvds->fitting_mode,
@@ -488,11 +491,13 @@ static int intel_lvds_get_modes(struct drm_connector
*connector)
if (intel_lvds->edid)
return drm_add_edid_modes(connector, intel_lvds->edid);
- mode = drm_mode_duplicate(dev, intel_lvds->fixed_mode);
- if (mode == 0)
- return 0;
+ if (intel_lvds->fixed_mode) {
+ mode = drm_mode_duplicate(dev, intel_lvds->fixed_mode);
+ if (mode == 0)
+ return 0;
- drm_mode_probed_add(conn
[Intel-gfx] [PATCH 1/3] [drm/i915] - Implement direct support for 24 bit LVDS pixel format
LVDS digital data can be formatted as 18 bits/pixel or one of two
24 bits/pixel possibilities. All are incompatible with one another,
so the GPU's LVDS output has to be configured to match the format
expected by the display device.
In many (most) cases this is generally not a problem because the LVDS
device is integral to the processor board (e.g. a laptop) and thus the
video BIOS already sets this up correctly as part of the boot
process. Then the i915 drm simply works with what has been already
set up.
But there are cases where the LVDS-driven display and the GPU are
discrete components - this can happen in embedded environments where
the processor board is a COTS device with its own BIOS and the display
is added to it later. In that situation the video BIOS on the
processor board can't know anything about the LVDS display which leads
to problems if the pixel format is not 18 bit (usually 18 bit is the
default).
This patch implements a new kernel option for the i915 kernel module:
"lvds_24bit". The default value of zero preserves the previous "don't
change anything" behavior. If it is set to "1" or "2" then 24 bit
format is enabled - the choice between "1" and "2" selects the
particular 24 bit format. If it is set to "3" then 24 format is
specifically disabled, which should be an extremely rare case but is
included for completeness.
There was a similar patch back in 2008 to support 24 bit LVDS with the
user-mode xorg intel driver, using a new driver option in the
xorg.conf file. However that patch was a casualty of the move to
kernel mode switching. This patch implements the same sort of
solution, just now it's in the kernel drm driver for i915 driven GPUs.
Signed-Off-By: Mike Isely
---
drivers/gpu/drm/i915/i915_drv.c |4 +++
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/i915_reg.h |7 ++
drivers/gpu/drm/i915/intel_display.c | 38 ++---
4 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index
da769bc12ecd6d63965df571b7cf3e95c474cd46..c89f71c251acf230db613229eca90d24584b9729
100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -49,6 +49,10 @@ module_param_named(powersave, i915_powersave, int, 0600);
unsigned int i915_lvds_downclock = 0;
module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
+unsigned int i915_lvds_24bit = 0;
+module_param_named(lvds_24bit, i915_lvds_24bit, int, 0600);
+MODULE_PARM_DESC(lvds_24bit, "LVDS 24 bit pixel format: 0=leave untouched
(default), 1=24 bit '2.0' format, 2=24 bit '2.1' format, 3=force older 18 bit
format");
+
static struct drm_driver driver;
extern int intel_agp_enabled;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index
d2896ebaba9f81100efc16f1d0cfbe7845d7a997..526cef2972ab9afce1c17f57ef39ce9cc4dc736f
100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -886,6 +886,7 @@ extern int i915_max_ioctl;
extern unsigned int i915_fbpercrtc;
extern unsigned int i915_powersave;
extern unsigned int i915_lvds_downclock;
+extern unsigned int i915_lvds_24bit;
extern int i915_suspend(struct drm_device *dev, pm_message_t state);
extern int i915_resume(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index
0a1b276418991f48b81f72f648aa6dc980a618b4..ffe7f459440f10612ac4ab957d4b4eb75205e3ce
100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1320,6 +1320,13 @@
#define LVDS_PIPEB_SELECT(1 << 30)
/* LVDS dithering flag on 965/g4x platform */
#define LVDS_ENABLE_DITHER (1 << 25)
+/*
+ * Selects between .0 and .1 formats:
+ *
+ * 0 = 1x18.0, 2x18.0, 1x24.0 or 2x24.0
+ * 1 = 1x24.1 or 2x24.1
+ */
+#define LVDS_DATA_FORMAT_DOT_ONE (1 << 24)
/* Enable border for unscaled (or aspect-scaled) display */
#define LVDS_BORDER_ENABLE (1 << 15)
/*
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index
3abd904ce435a79fb273c0242a18049b55050b9c..09f57f29c30c371c213944be473090a780a287db
100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4029,10 +4029,40 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
else
temp &= ~(LVDS_B0B3_POWER_UP | LVDS_CLKB_POWER_UP);
- /* It would be nice to set 24 vs 18-bit mode (LVDS_A3_POWER_UP)
-* appropriately here, but we need to look more thoroughly into
how
-* panels behave in the two modes.
-*/
+ /* Control the output pixel format. */
+ switch (i915_lvds_24bit) {
+ default:
+ case 0:
+ /* 0 means don't mess with 18 vs 24 bit LVDS pixel
+*
Re: [Intel-gfx] Oops at i915_gem_retire_requests_ring
On Thu, 17 Mar 2011 09:39:07 -0300, Herton Ronaldo Krzesinski
wrote:
> I don't know if it's the most correct fix, but perhaps the simple fix
> is needed in the code. It's against latest Linus tree. We may have an
> already removed client_list, or we didn't add any item to client_list
> (file_priv NULL in i915_add_request)
>
> Signed-off-by: Herton Ronaldo Krzesinski
> ---
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 36e66cc..6077c0d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1749,8 +1749,10 @@ i915_gem_request_remove_from_client(struct
> drm_i915_gem_request *request)
> return;
>
> spin_lock(&file_priv->mm.lock);
> - list_del(&request->client_list);
> - request->file_priv = NULL;
> + if (request->file_priv) {
> + list_del(&request->client_list);
> + request->file_priv = NULL;
> + }
> spin_unlock(&file_priv->mm.lock);
> }
This is the single chunk required. I had thought that the actual
insertion/deletion was serialised under the struct mutex and the intention
of the spinlock was to protect the unlocked list traversal during
throttling. However, I missed that i915_gem_release() is also called
without struct mutex and so we do need the double check for
i915_gem_request_remove_from_client().
-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] slow graphics after Suspend to RAM
On Wed, 16 Mar 2011 17:59:20 -0600, John Harrigan wrote: > I have a Lenovo X201 laptop with an i7-620M processor. After I resume > from a Suspend to RAM the graphics are very slow. Suspend to Disk does > not cause the same problem and doing a Suspend to Disk after a Suspend > to RAM fixes the problem. Known problem. The PAT lose the WC bits for the GTT aperture on resume. There is a workaround that we can do: recreate the ioremapping upon resume. (But the root cause is not a gfx driver bug.) > I see the same slow-down regardless of whether X11 is running or not. > I'm not doing 3D, I notice the slow-down in regular 2D stuff like > scrolling a lot of text through a terminal window. That's not regular 2D stuff, that's CPU fallback! You will only see a slight performance difference (an order of magnitude and more, with comparable decrease in power consumption whilst drawing) by switching to the XRender paths in your terminal. -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
